fix(spec): the external-federation error family declares its HTTP status, so a write refusal stops leaking as a bare 500 (#7739) - #7791
Conversation
…#7739) A write to a read-only federated external object was refused correctly — `ExternalWriteForbiddenError` names the datasource, its `schemaMode` and both flags that would be required, and nothing is applied — but reached the client as a bare 500 INTERNAL_ERROR with no `code`, indistinguishable from a crash. The `code` was never the missing half: `EXTERNAL_WRITE_FORBIDDEN` is already registered in the ADR-0112 error-code ledger. The missing half was an HTTP status, which no exit can invent — so the refusal fell past every structured branch of `mapDataError` and left through the terminal `UNCLASSIFIED_FAULT`, which sanitises to 500 and ships no code the producer never declared. Fixed at the producer, for the whole `EXTERNAL_ERROR_CODES` family at once: new `EXTERNAL_ERROR_HTTP_STATUS` maps every code to its status, and each error class carries it as `status`. Every HTTP exit in the repo already resolves `status` then `statusCode` (`declaredHttpStatus`, `resolveErrorResponse`, `HttpDispatcher.errorFromThrown`, `dispatcher-plugin.errorResponseBase`, `endpoint-executor`, `domains/actions`, `plugin-hono-server`), so one table fixes every door; a branch in `rest-server.ts` would have fixed one. `rest-server.ts` is untouched. EXTERNAL_WRITE_FORBIDDEN 403 policy refusal, not malformed input EXTERNAL_SCHEMA_MODE_VIOLATION 403 same, for DDL EXTERNAL_SCHEMA_MISMATCH 503 a deployment state, not the caller `satisfies Record<ExternalErrorCode, number>` makes a future gate that adds a code without a status a compile error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0158ZQo7LiHSxGWpYKuPq1wu
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 106 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
|
CI is red on The failure is in This PR does not touch Identified from the file's commit history, not inferred from timing. Why it landed invisibly. The conformance scan lives in Not re-queueing. A re-queue cannot fix a failure that lives on the base branch, and each attempt rebuilds every PR behind it in the batch. This PR stays as-is until #7802 is resolved; when #7802 is not a "add Nothing is requested of this PR. Everything else here is green. Generated by Claude Code |
Fixes #7739
The defect
A write to a read-only federated external object was refused correctly on
the server.
ExternalWriteForbiddenErrornames the datasource, itsschemaMode, and both flags that would be required, and nothing is applied —ADR-0015's Gate 3 throws before the driver is reached at all. Only the envelope
was wrong: the refusal reached the client as a bare 500 INTERNAL_ERROR with
no
code, indistinguishable from the server falling over.Root cause — the status, not the code
The
codeexisted the whole way down.EXTERNAL_WRITE_FORBIDDENis registeredin the ADR-0112 error-code ledger (
error-code-ledger.zod.ts, under@objectstack/spec), so the wire vocabulary was ready.What the error did not carry was an HTTP status, and no exit can invent one.
So the refusal fell past every structured branch of
mapDataError, matched nomessage heuristic, and left through the terminal
UNCLASSIFIED_FAULT— whichsanitises to
500and, by design, ships nocodethe producer never declared.The discriminator the client needed was dropped at the boundary for want of a
status to carry it.
The fix: the family declares its status, at the producer
packages/spec/src/shared/external-errors.tsgains one table beside the codesit keys on, and each of the three error classes carries its value as
status:Why at the producer rather than in a REST error map. ADR-0112 says the
producer names the condition, and every HTTP exit in this repo already agrees on
how to read one —
statusthenstatusCode, band 400-599:mapDataError'sdeclaredHttpStatus(#7525),resolveErrorResponse(#5437/#5582),
HttpDispatcher.errorFromThrown(#3867),dispatcher-plugin.errorResponseBase,endpoint-executor,domains/actions,plugin-hono-server. Declaring the status on the error is therefore what routesthe family through one place for every door at once; a branch in
rest-server.tswould have fixed one door and left the runtime dispatcher, theendpoint executor and the CLI answering 500 for the same throw.
Same shape the repo already uses for this problem:
service-analytics'sdataset-refusal.ts(#5367) andstorage-service.ts'sstorageListRefusal—"one condition, one wire shape, chosen by the producer that knows".
rest-server.tsis untouched. #5949 records it at 8593 lines and a frequentconflict site; this change adds zero lines to it.
Family mapping — and the status convention followed
The card asked for the family, not the instance, and for the 4xx choice to be
justified against the surrounding conventions.
EXTERNAL_WRITE_FORBIDDENdatasource.external.allowWritesandobject.external.writableare both on, so 400/422 ("fix your request") would be a lie, and 409 ("conflict with current state") promises a retry that cannot help. 403 is whatmapDataErroralready answers for a capability a flag switched off for the object —FEEDS_DISABLED,FILES_DISABLED,CLONE_DISABLED,RECORD_NOT_ACCESSIBLE— and what the standard catalog spellsFORBIDDEN.EXTERNAL_SCHEMA_MODE_VIOLATIONschemaMode !== 'managed'forbids it, and no request the caller can rewrite changes that.EXTERNAL_SCHEMA_MISMATCHERR_DATASOURCE_UNAVAILABLEalready gets ("the deployment cannot serve this object right now"); both areisExpectedDataStatuslifecycle outcomes rather than crashes. The 5xx band withholds the message by design, which is right here: this gate aborts boot, so its structureddiffsaudience is already operator-side, and the client still gets acodeto branch on instead of a bare 500.So the dispatch is one mapping; only the values differ, and the one that
differs is the one that is genuinely not a client error.
satisfies Record< ExternalErrorCode, number >makes a future gate that adds acode without a status a compile error — which is the half that stops the
next QA run from refiling this card under a different code.
Verification
packages/rest/src/external-write-forbidden-envelope.test.ts(new, 9 cases)drives the real CRUD routes in process against a protocol whose data verbs
are a real
ObjectQLengine over an in-memory driver whose store the testcan read — not a rejecting mock.
That fixture choice is load-bearing. The card asks for two facts a
mockRejectedValuecannot pair: the refusal answers 403 with itscode, andnothing is applied. A mock that rejects makes the second vacuously true, so it
would stay green against a future "fix" that got the status right by letting the
write through. Here §3 measures the store against a driver that can write,
and a CONTROL case flips the two opt-in flags and writes through the very same
route to prove the fixture is not inert. §3 also asserts the driver was never
reached at all, so a create-then-rollback "fix" fails even with an empty table.
packages/spec/src/shared/external-errors.test.tsgains the family pin: the mapcovers exactly
EXTERNAL_ERROR_CODES, every status is inside the 400-599 bandeach exit reads, and each instance carries it as
status.Reverse verification
Direction predicted before running: RED on the unfixed producer, for the
reason the card describes. Measured by reverting
external-errors.tstoorigin/main, rebuilding@objectstack/spec(the test importsdist, soreverting the source alone would have proven nothing) and re-running:
500, andres.body.codeundefinedexpected true to be false— the500 did emit
[REST] Unhandled error, which is the operator-side half of thesame symptom
expected 500 to be 403the store assertions in it were green before and after
predicted: §3's "PATCH and DELETE reach the driver no more than POST does"
(nothing was ever applied — the behaviour was never the bug) and §3's CONTROL
case (the write-allowed arm, which this change does not touch). Recorded as
measured rather than reshaped: a template demanding before-red on every
assertion would have had those two cases lie about what they measure. They
are asserted anyway because they are the half that stops a status-only "fix"
from passing.
Scope
Not touched: #7663 (auth bodyless-500s), #7543 (raw
TypeErrorleak), #5437(5xx driver-throw passthrough) — same envelope-leak class, deliberately out of
scope. This change does not make them easier to close: each has a different
cause (a producer that writes no body, a native error name reaching a business
branch, a 5xx passthrough range), and none of them is a missing status
declaration.
.changeset/external-error-http-status.mdadded — the wire status of threecodes is user-visible.
content/docs/releases/untouched.Local runs
pnpm --filter @objectstack/spec test(full suite)pnpm --filter @objectstack/rest test(full suite)pnpm --filter @objectstack/rest test external-write-forbidden-envelopepnpm --filter @objectstack/objectql test external-write-gatepnpm --filter @objectstack/runtime test external-validationpnpm --filter @objectstack/spec typecheckpnpm --filter @objectstack/rest typecheckeslinton the three changed/added filespnpm check:api-surface/check:export-originspnpm check:error-code-casingpnpm check:nul-bytesGenerated by Claude Code