Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions .changeset/metadata-protocol-driver-text-disclosure.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
---
"@objectstack/metadata-protocol": patch
---

fix(metadata-protocol): stop interpolating raw driver text into client-facing messages (#8136)

Option C of #8086, at the producer. `packages/metadata-protocol` interpolated
raw driver/engine error text into messages and response payloads that reach API
clients. Measured on the uninstall path: `DELETE /api/v1/packages/:id` answered
`500 INTERNAL_ERROR` with the body message `SQLITE_ERROR: no such table:
sys_metadata` — a physical table name on the wire.

Three downstream sanitizers already existed for this class, and each had a hole
traceable to the producer. Two of those holes are structural, not accidental:

- The boundary belts run `looksLikeInternalErrorLeak`, a **heuristic over the
message**. It now knows the two dialects this repo runs (#8132 / #8263), but a
phrasing test can only ever know the dialects someone has met — MySQL, MSSQL
and Oracle each phrase "this table is missing" differently again, and all
three are measured invisible to it.
- `deletePackage`'s per-item `failed[]` and `cleanups[]` ride onto a
`PACKAGE_DELETE_PARTIAL` **400** inside `details`. That is data, not a
message, so no 5xx message withhold at any HTTP boundary ever sees it.

**The rule, now stated once at the producer.** A caught error's sentence is
quoted back to a caller only when that error **declared itself a client-facing
refusal** — a 4xx `status` in the ADR-0112 envelope. Anything undeclared (a bare
`Error` from a driver) or declared a server fault gets a stable sentence naming
the operation that failed, and the original error rides on `cause` so the
operator's log still receives it whole. This is a positive list rather than a
negative heuristic, so a dialect nobody here has run is handled correctly by
default.

Behaviour changes visible to an API client, all on failure paths:

- A driver failure on the uninstall's `sys_metadata` read is now refused with the
declared envelope this package already uses for that exact condition —
**503 `SERVICE_UNAVAILABLE`** with the "metadata store could not be read"
sentence — instead of an undeclared 500 carrying the driver's own text. It
remains a failure: an unreachable store is never reported as an uninstall that
removed nothing.
- `deleteMetaItem`'s two failure exits keep the `Failed to delete customization
overlay` prefix and their existing `status`, but no longer append the driver's
message.
- `deletePackage`'s `cleanups[].error` reports `cleanup failed` for a cleanup
that failed without declaring a refusal.

Self-correcting refusals are deliberately untouched: `[item_locked]`,
`[writable_package_required]`, `[no_draft]`, `[tenant_scope_required]` and the
rest declare a 4xx and still reach the caller verbatim, including inside
`failed[]` on a partial uninstall.
15 changes: 14 additions & 1 deletion packages/metadata-protocol/src/durable-package.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -96,11 +96,24 @@ describe('deletePackage — uninstall cleanups (#2747)', () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
try {
const { impl } = makeImpl();
// [#8136] `db down` is a BARE error — it declares no ADR-0112 envelope,
// so the producer no longer quotes it onto the response. `cleanups[]`
// rides onto a `PACKAGE_DELETE_PARTIAL` 400 inside `details`, where no
// HTTP boundary's 5xx message withhold can reach it, so a cleanup that
// failed on a driver fault used to ship the driver's words to the client.
//
// This case's SUBJECT is unchanged and still asserted: a throwing cleanup
// is reported as `success: false` rather than aborting the uninstall.
// Only the text moved, and the counterpart — a cleanup that DECLARES a
// 4xx refusal keeps its sentence verbatim — is pinned in
// `protocol.driver-text-disclosure.test.ts`, so "reported as failed" and
// "reported in the driver's words" cannot collapse into one another.
(impl as any).registerUninstallCleanup('boom', async () => { throw new Error('db down'); });
const res: any = await (impl as any).deletePackage({ packageId: 'com.example.orders', allTenants: true });
expect(res.cleanups).toEqual([
{ name: 'boom', success: false, removed: 0, error: 'db down' },
{ name: 'boom', success: false, removed: 0, error: 'cleanup failed' },
]);
expect(JSON.stringify(res)).not.toContain('db down');
} finally {
warn.mockRestore();
}
Expand Down
Loading
Loading