Uh oh!
There was an error while loading. Please reload this page.
fix(service-package): classify a publish driver fault as 5xx and stop returning driver text as caller data (#8131) - #8277
Conversation
… returning driver text (#8131) `POST /api/v1/packages/publish` answered `400 PACKAGE_PUBLISH_FAILED` with the raw driver message whenever the `INSERT INTO sys_packages` statement failed. Reproduced on a real SQLite engine before changing anything: 400 {"code":"PACKAGE_PUBLISH_FAILED","message":"no such table: sys_packages"} 400 {"code":"PACKAGE_PUBLISH_FAILED", "message":"NOT NULL constraint failed: sys_packages.tenant_ref"} Two independent defects: a server fault labelled a client error, and a driver dump handed back as caller-visible data. Fixed at the producer. `publish` no longer returns `(error as Error).message`; it returns a discriminated `driverFault` carrying a stable, non-interpolated sentence, and re-throws a refusal that declares its own status so the door's existing mapping answers it with that status and code. The door maps a returned driver fault to 500. The producer half is load-bearing, and measured to be: the 5xx withhold lives in `sendThrownError`, which a RETURNED failure never reaches at any status, and `looksLikeInternalErrorLeak('no such table: sys_packages')` is false — so reclassifying alone would have left the driver line on the wire. The discriminant is the STATUS channel only. Accepting a string `code` as a declaration was tried and reverted: every SQL driver populates it (ERR_SQLITE_ERROR, 42P01, ER_NO_SUCH_TABLE), so it re-threw genuine driver faults into a 500 whose message the heuristic does not withhold. Caller-facing 4xx is untouched, per the card's binding scope guard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARidKDYSCD56LaygrvDPnk
…e FROM/TO block (#8131) The ledger's subject is metadata; this change touches a runtime TS interface only. The consumer guidance stays — it is now prose plus the compiler error, which is the channel that actually reaches a caller of this service. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARidKDYSCD56LaygrvDPnk
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 2 package(s): 9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also reference the affected code. These are read-only:
|
…kage-door-producer-classify
… re-prove the fix without them (#8131) CI went red on three cases in this PR's own new suite, all reading `expected true to be false`. Cause: #8132 (f598aa8) landed on main after this branch point and taught `looksLikeInternalErrorLeak` the bare-SQLite and Postgres phrasings, so `no such table: sys_packages` is now recognised. The pins asserted it was NOT. That is the outcome those cases were written to signal, and their own instruction — and #8136's — was "delete or invert; do not repair to green". Inverted, not silently flipped: the prose that claimed a gap is rewritten, because the gap is closed. What this does NOT change is why the producer fix exists. That argument had two halves and only the second is retired: - structural (still true, and now the sole reason): the withhold lives in `sendThrownError`; a RETURNED failure reaches `sendError`, which consults no predicate at any status; - semantic (retired by #8132): the phrasing used to trip nothing. Re-measured against the WIDENED predicate, main's producer with only the status corrected to 500 still answers `500 {"code":"PACKAGE_PUBLISH_FAILED","message":"no such table: sys_packages"}` — the driver line on the wire while a predicate that recognises it perfectly is never asked. A new case pins exactly that, so nobody concludes #8132 made this fix redundant. Docblocks, the producer comment and the changeset are corrected to match; no behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARidKDYSCD56LaygrvDPnk
Uh oh!
There was an error while loading. Please reload this page.
Part of #8131
Part ofand notFixes. This closes the card's §1 (POST /packages/publish) only. Its §2 — theDELETEpartial-failure array, whose raw text rides indetails.failed[].error— is produced byprotocol.deletePackageinpackages/metadata-protocol, which is out of this card's declared file surface and is the surface of the in-flight #8136. Merging this must not auto-close a card with that half unaddressed; the PM should close#8131 deliberately once #8136 lands. Details in "What is NOT in this PR" below.The premise, verified before anything was changed
The card filed its
publishpath as read from source, not reproduced, and asked whoever took it to force a real failure first. Done: a real SQLite database behindobjectql.execute, running the realINSERT INTO sys_packages …statement, driven through the real handler. Two forced failures onorigin/main:The card's claim is exactly right, and both defects are real: raw driver text as caller-visible data, on a 400.
The load-bearing assumption was FALSE — and it stays false after #8132
The dispatch assumed that reclassifying this path to 5xx would put it inside the existing 5xx withhold "with no new rule".
The structural reason it is false, which is the whole argument. The withhold lives in
sendThrownError. A failure that is returned reachessendErrordirectly, andsendErrorconsults no predicate at any status. Classification alone turns a 400 into a 500 and leaves the driver line exactly where it was.Measured as a counterfactual —
main's producer with only the status corrected (400 → 500, producer untouched):Re-measured after #8132 landed, and the result is unchanged.#8132 (
f598aa8) taughtlooksLikeInternalErrorLeakthe bare-SQLite and Postgres phrasings, sono such table: sys_packagesis now recognised as a leak. The counterfactual above still produces the same body — a driver line on the wire while a predicate that recognises it perfectly is never asked. #8132 does not make this fix redundant, and a new case pins exactly that so nobody concludes otherwise.An earlier revision of this PR also argued a semantic half — that the phrasing tripped none of the heuristic's keywords, so even reaching the withhold would not have helped. #8132 retired that half. It is called out here rather than quietly dropped, because it was load-bearing in the original reasoning and is no longer true.
What changed
Producer —
packages/services/service-package/src/index.tspublishcaught everything, so every coded refusal reachable from this call path was flattened into one400 PACKAGE_PUBLISH_FAILED, losing both.driverFaultcarrying a constant sentence with no interpolation at all.Door —
packages/rest/src/package-routes.ts, the two lines of status/message mapping and nothing else:400→500, andresult.error→result.driverFault?.message.The discriminant is the STATUS channel, and that is a measured decision
An earlier draft accepted any non-empty string
codeas a declaration. It reads as reasonable and is wrong: every SQL driver populates that field —node:sqlitethrowsERR_SQLITE_ERROR, better-sqlite3SQLITE_ERROR, Postgres the SQLSTATE42P01, MySQLER_NO_SUCH_TABLE. The real-driver cases went red at once: every genuine driver fault was re-thrown as if it were a refusal and resolved to500 INTERNAL_ERRORwith the driver's own message. Pinned per dialect so it cannot come back.Caller-facing 4xx is untouched — the binding scope guard
Held and pinned. Missing manifest, invalid manifest, and any coded refusal thrown from below
publishall keep their status, code and self-correcting message; a409 DESTRUCTIVE_CHANGEis still a 409. The refusal pins assert the service was never called, not merely the status.Reverse verification — direction predicted before each run
expected 400 to be 500plus the message diffsendError, not pins on this differror TS2339: Property 'error' does not exist on type 'PackagePublishResult'Restoration after every lap was verified byte-identical to the commit.
The CI red, and what it was
Test Core (3/3)failed on three cases in this PR's own new suite, all readingexpected true to be false— the leak-predicate pins, turned red by #8132 landing after this branch point. Those cases were written with an explicit instruction for this exact event ("delete or invert it; do not repair it to green"), and they are inverted, not flipped: the prose claiming a gap is rewritten because the gap is closed, and the surviving argument is re-proved without them (lap C′). Not a flake, not environmental, and not a re-run candidate.Verification
@objectstack/service-package— 21/21,@objectstack/rest— 1800/1800, onmainmerged in.@objectstack/rest,@objectstack/runtime,@objectstack/cli(downstream direction, i.e. the packages that depend onservice-package).check:route-envelope,check:cross-package-test-inputs,check:test-source-alias,check:type-source-resolution,check:nul-bytes,check:error-code-casing,check:authz-resolver,check:filter-alias-parity,check:meta-type-normalized,check:docs-audit-scope,check:adr-0087-registration— all pass.What is NOT in this PR
DELETEpartial-failure array. Its producer isprotocol.deletePackageinpackages/metadata-protocol— outside this card's file surface, and the subject of [finding]metadata-protocolinterpolates raw driver text into client-facing messages — three downstream sanitizers each have a hole because of it (option C of #8086) #8136, which reasons about this exact array as part of its own case.packageService.delete'sPACKAGE_DELETE_FAILED, which answers400for a driver fault in the identical shape. It carries no message, so there is no disclosure — only the mislabelling. Filed as [finding]packageService.delete's driver fault is answered as400 PACKAGE_DELETE_FAILED— the same mislabelling #8131 fixed forpublish, on the sibling route #8275 rather than swept in.Generated by Claude Code