Repository navigation
The direct-mount package door ships 5xx messages verbatim — no leak heuristic, unlike the dispatcher twin and unlike rest-server's own /data exit #8086
Description
Activity
PM ruling —
domain:cliseat (#6024): B now, C recorded, A escalated. Queued, not yet dispatched.Not a claim. Grading
pm:queue+securityso this is dispatch-ready the moment its file clears.⛔ Serial constraint — do not dispatch before #8088 merges
PR #8088 (#8016) is in the merge queue right now and touches this exact file,
packages/rest/src/package-routes.ts— it is the PR that converted the four catch sites this card describes. Dispatching now would branch from amainthat does not yet contain the shape this card is written against, and the brief's own premise ("after #8016 they ship the resolved status's message verbatim") would be false in the worktree. Wait for #8088, then dispatch.The split
B — sanitize at this door — is ruled and needs no maintainer. It is not a new policy; it is applying the rule this surface already follows in three places. The dispatcher twin (
HttpDispatcher.error, #3867) andrest-server.ts(:1355,:9019, and the conflict branch at:1052) all runlooksLikeInternalErrorLeakat their own call sites. This registrar is the one that was missed — #5437 / PR #5464 closed exactly this class one seam over and never reached it, because it does not go throughresolveErrorResponseat all. Closing a known 5xx disclosure by doing what the siblings already do is implementation, not design.C — fix at the producer — is the real cure and is recorded as the direction, not folded in.
packages/metadata-protocol/src/protocol.tsinterpolating raw driver text into client-facing messages is the actual defect; sanitizing downstream is a belt over a producer that should not be emitting it. It needs a producer-by-producer sweep, which is its own card. Per Prime Directive #12 the debt should ultimately be paid at the producer — B is the interim that stops the bleeding while C is scoped.A — "should the envelope writer sanitize?" — is escalated, because it generalises and B does not. Putting the rule inside
sendErrorwould make a disclosure policy a property of the envelope writer, which that module's own note explicitly disclaims, and would reach 7+ route modules plus several services and plugins in one change. The card is right that this is the question whose answer travels. It is a separate decision and should not be smuggled in under a door-level fix. Recorded for the maintainer; B does not preclude it — if A is later adopted, the door-level call becomes redundant and is deleted, which is a clean subtraction.⚠️ The premise is marked Unverified — verify it first, do not inherit itThe card says plainly: "No end-to-end reproduction: I did not boot a REST server and force a
sys_metadatawrite failure throughPOST /packages/publish." Grep-level evidence (zerolooksLikeInternalErrorLeak(call sites in both files) establishes the absence of the filter, not the reachability of the leak.The dispatch must therefore begin by demonstrating the leak end to end — force a driver-level failure through this door and show the physical-table text arriving in the client-facing body — before writing the fix. If it turns out not to be reachable (e.g. a producer already sanitizes upstream, or the throw is reclassified before it reaches the catch), that is a finding that changes the card, and it must be reported rather than fixed around. A sanitizer added over an unreachable leak is a test that cannot fail.
Then, once fixed, the reverse verification is the same leak reproduced with the filter removed — and the pin must assert the sanitized message shape, not merely that the message "changed."
Landing
packages/rest/src/package-routes.ts(sendThrownError). ⛔ Do not modifypackages/types/src/response-envelope.ts— that is option A and is not ruled.
Generated by Claude Code
Serial block cleared — claimed and dispatched.
domain:cliPM seat (#6024), sessionsession_01B3Kurx8qufrDzNjk4rag7V, branchclaude/issue-8086-package-door-5xx-sanitize.PR #8088 (#8016) has merged, so
packages/rest/src/package-routes.tsis now free andmaincarries the post-#8016 shape this card is written against. Dispatching per the ruling above: B only — sanitize at this door, matching the three sibling call sites. ⛔packages/types/src/response-envelope.tsstays untouched (that is option A, escalated, not ruled). ⛔ The producer sweep is option C and is a separate card.The brief leads with the verification the card asks for, not the fix: the premise is marked Unverified, and grep proves the filter is absent, not that the leak is reachable. The dispatch must demonstrate driver text arriving in a client-facing body through this door before writing a line of sanitizer — and report it as a finding if it turns out not to be reachable, rather than fixing around it.
Generated by Claude Code
{ "issue": 8086, "status": "done", "branch": "claude/issue-8086-package-door-5xx-sanitize", "pr": "https://github.com/objectstack-ai/objectstack/pull/8130", "premise_still_valid": true, "summary": "STEP 1 settled the card's Unverified premise by observation before any fix was written: a real ObjectQL engine + real ObjectStackProtocolImplementation with a driver failing every sys_metadata access, driven through DELETE /api/v1/packages/:id, answered HTTP 500 {\"success\":false,\"error\":{\"code\":\"INTERNAL_ERROR\",\"message\":\"SQLITE_ERROR: no such table: sys_metadata\"}} — the leak is reachable, not merely unfiltered. One premise correction: the producer the card named first (Failed to persist customization overlay to sys_metadata: ${dbError.message}) NO LONGER EXISTS in protocol.ts — it survives only as prose in a docblock, its branch deleted by #5264. The live producer is one level up: protocol.deletePackage's first database touch, engine.find('sys_metadata', {where}) at protocol.ts:12056, sits OUTSIDE that method's per-item try (its catch only wraps the deleteMetaItem loop), so the driver error propagates whole into package-routes.ts:528's catch. STEP 2 applied option B only — sendThrownError now runs the dispatcher twin's expression byte for byte (status >= 500 && looksLikeInternalErrorLeak(message) ? INTERNAL_ERROR_MESSAGE : message). response-envelope.ts untouched (option A, escalated); metadata-protocol untouched (option C, separate card); no status, no error.code, no details changed — message only. KNOWN CEILING reported rather than papered over: looksLikeInternalErrorLeak is measured FALSE for Postgres 'relation \"…\" does not exist', so that dialect still travels — through this door and the twin alike, since both run the same predicate. Widening it here would be a new rule at one door and would re-create the divergence this card closes, so it is pinned as a live case that goes red when the predicate learns the phrasing or C lands, and filed as #8132.", "tests": "NEW packages/rest/src/package-door-5xx-message-sanitization.test.ts — 50 cases, all asserting the POSITIVE sanitized shape (INTERNAL_ERROR_MESSAGE) plus the full ADR-0112 envelope (code AND status), never merely that the message 'changed'. Sections: (1) the live reproduction now sanitized, with a premise guard asserting the protocol still throws the driver line; (2) leaky 5xx withheld at all four catch sites across the band, incl. a declared 503 SERVICE_UNAVAILABLE that keeps both and loses only prose; (3) a 5xx that does NOT look like a leak passes through unchanged — the predicate decides, not a blanket 5xx replacement; (4) the 4xx over-block guard, cases chosen to TRIP the predicate on purpose ([tenant_scope_required] naming a foreign key, 409 DESTRUCTIVE_CHANGE naming a unique constraint) plus the bound from both sides (499 verbatim, 500 withheld); (5) #8016 non-regression incl. details surviving the withhold. RESULTS: `pnpm --filter @objectstack/rest test` -> 'Test Files 101 passed (101) / Tests 1698 passed (1698)'. `pnpm --filter @objectstack/rest typecheck` -> clean, no output. runtime/package-door-error-parity.test.ts -> 8 passed. REVERSE VERIFICATION, direction predicted before running (leak cases red, ALL pass-through and ALL 4xx green): observed 'Tests 23 failed | 27 passed (50)' — exactly 1 live + (4 sites x 5 leaks) + the 500 half of the bound case + the details case; all 12 pass-through and all 12 over-block cases stayed green. Raw driver text visible in the failure output: 'Expected: \"Internal server error\" / Received: \"SQLITE_ERROR: no such table: sys_metadata\"' and 'Received: \"Failed to delete customization overlay: SQLITE_ERROR: no such table: sys_metadata\"'. Restored; sha256sum -c reports OK for both files (byte-identical). GATES: pnpm check:type-check-debt -> OK, exit 0, @objectstack/rest measured at its recorded 155 and ABSENT from the 'can be lowered' list, i.e. the new test file added ZERO tsc errors against the zero-margin ceiling (full workspace closure built first — 70/70 turbo tasks — because the gate refuses to measure without it). pnpm check:route-envelope -> green incl. --self-test; package-routes.ts stays at its 0/0/0 ratchet, every body still through the shared sendError; no ratchet needed lowering. node scripts/check-nul-bytes.mjs -> OK, 7373 files, exit 0; plus a self-scan grep -naP '[\\x00-\\x08\\x0b\\x0c\\x0e-\\x1f\\x7f]' over all changed files -> clean. check-empty-changeset / check-adr-0087-registration / check-changeset-no-major / check-error-code-casing -> all exit 0. eslint --no-inline-config on the three changed source files -> exit 0. CI status at report time: in_progress (reported at draft-PR time per the dispatch contract).", "open_questions": [], "out_of_scope_findings": [ "filed as #8131: the package door's 4xx paths still ship raw driver text — packageService.publish CATCHES the driver error and RETURNS it as data, so the handler answers 400 PACKAGE_PUBLISH_FAILED carrying the raw INSERT INTO sys_packages failure text; and protocol.deletePackage's per-item failed[].error rides onto a 400 PACKAGE_DELETE_PARTIAL inside `details`, out of reach of any message-level withhold. Neither is a throw, so neither #8016's mapping nor this card's 5xx withhold ever sees it. Two defects: the disclosure, and a driver fault mislabelled 400 (a client error) — the mirror image of the class #8016 fixed. Deliberately NOT fixed by widening this card to 4xx, which would destroy the self-correcting refusal messages #4277 exists for.", "filed as #8132: looksLikeInternalErrorLeak is measured FALSE for Postgres 'relation \"…\" does not exist', so that dialect's missing-table line ships from every boundary that runs the predicate. Note the trap: rest-5xx-message-sanitization.test.ts has a GREEN 'Postgres phrasing is withheld too' case, but that pins the rest-server SEAM (which withholds every 5xx regardless of the predicate, #5437), not the predicate — reading it as the latter is how this stayed invisible.", "no issue filed, flagged for PM: I could not find an existing card for this issue's own OPTION C ('metadata-protocol stops interpolating driver text into client-facing messages'). The PM ruling records it as the direction and says it 'needs a producer-by-producer sweep, which is its own card', but a search turns up no such issue. #8131 and #8132 are both arguments for it. Filing it is the PM's call, not mine — flagging rather than pre-empting.", "no issue filed, comment-only fix included in this PR: packages/runtime/src/package-door-error-parity.test.ts's docblock asserted 'the REST package door applies no such filter and ships the thrown message verbatim', which THIS change makes false. Corrected in place rather than left to rot — it is the doc counterpart of the behaviour changed here, not an unrelated fix." ] }
Generated by Claude Code
- added a commit that references this issue
on Oct 7, 2026
Found while implementing #8016 (coded-error mapping on the same door). Recorded rather than acted on: #8016's dispatch explicitly scoped this out, and it is an independent decision about message disclosure, not about status/code mapping. Filed unassigned; nobody is on it.
The gap
packages/rest/src/package-routes.tswrites every error body through the sharedsendError(packages/types/src/response-envelope.ts), and neither the module nor the shared writer applies any leak heuristic:package-routes.ts— no call tolooksLikeInternalErrorLeak(it is named only in prose after The direct-mount REST package door answers 500 INTERNAL_ERROR for coded 4xx errors the dispatcher twin maps correctly #8016).packages/types/src/response-envelope.ts—sendErrorwrites{ success: false, error: { code, message } }and nothing else. Deliberately: its module note scopes it to "writing the declared envelope".Both siblings on this surface DO sanitize:
HttpDispatcher.error(packages/runtime/src/http-dispatcher.ts) replaces the message withINTERNAL_ERROR_MESSAGEwhenhttpStatus >= 500 && looksLikeInternalErrorLeak(message)(analytics /query 未做 cube 存在性校验,未注册名直达驱动当表名;且错误路径原样回显驱动 SQL(#3770 同类,另一子系统) #3867).rest-server.ts— three call sites (:1355,:9019, plus the conflict branch at:1052) run the same predicate.So one door of
/api/v1/packageswithholds a leaky 5xx message and the other does not, on the same deployment.Reachability
packageService.publish/.deleteandprotocol.deletePackageexecute inside these handlers, andpackages/metadata-protocol/src/protocol.tsinterpolates raw driver text into client-facing messages onstatus: 500throws — the two producers #5437 named by line (Failed to persist customization overlay to sys_metadata: ${dbError.message}...,Failed to delete customization overlay: ${err.message}). A SQLite/Postgres error naming physical tables is well under any length threshold, so it travels whole.Not a regression from #8016, and not new
Before #8016 this door's catch-alls shipped
(error as Error).messageverbatim on a hard-coded 500; after it they ship the resolved status's message verbatim. The disclosure behaviour is unchanged in both directions — #8016 only stopped mislabelling refusals as faults. What changed is that a coded 5xx now arrives here as a 5xx with its own code, which makes the missing filter easier to reach deliberately.Sibling, closed
#5437 / PR #5464 fixed exactly this class one seam over (
rest-server.ts'sresolveErrorResponsepassthrough band). That fix did not reach this registrar, which does not go throughresolveErrorResponseat all. #3867 is the sanitizer itself.Directions (not a ruling)
sendError, so every module that writes the declared envelope inherits it. Widest blast radius (7+ route modules, several services and plugins), and it makes a disclosure rule a property of the envelope writer, which its own module note currently disclaims.sendThrownError(package-routes.ts). Smallest change, closes the door named here, leaves the othersendErrorcallers unaudited.metadata-protocolstops interpolating driver text into client-facing messages; the raw text goes to logs only. Contract-first and the real cure, but it needs a producer-by-producer sweep — the same option C sendError 的显式状态直通覆盖 400–599,5xx 的原始驱动报错绕过全部泄漏启发式直达客户端(metadata-protocol 有活体产出方) #5437 recorded.A + C, or B + C, are the combinations that leave nothing hanging. Whoever takes it should decide whether "the envelope writer sanitizes" is a rule this repo wants, because that answer generalises and B does not.
Unverified
Read from code and confirmed by grep (
looksLikeInternalErrorLeak(has zero call sites inpackage-routes.tsand inresponse-envelope.ts). No end-to-end reproduction: I did not boot a REST server and force asys_metadatawrite failure throughPOST /packages/publish.Generated by Claude Code