Skip to content

fix(service-datasource): refuse an importObject name override that violates the namespace prefix - #11241

Merged
os-sam merged 2 commits into
mainfrom
claude/issue-11061-import-object-namespace-prefix
Aug 23, 2026
Merged

fix(service-datasource): refuse an importObject name override that violates the namespace prefix#11241
os-sam merged 2 commits into
mainfrom
claude/issue-11061-import-object-namespace-prefix

Conversation

@os-sam

Copy link
Copy Markdown
Collaborator

Fixes#11061

What

ExternalDatasourceService.importObject took opts.name verbatim (opts.name ?? draft.name) and persisted it through metadata.register('object', ...) — the one runtime write path no namespace gate looks at (register applies the register contract and the writability gate, nothing else; the prefix rule runs only at defineStack() and the publish pre-flight). A POST /api/v1/datasources/:name/external/tables/:remote/import body of {"name": "customers"} therefore minted an unprefixed federated object that the platform's own authoring and publish paths would both have refused. Reachability re-verified against this head: the route forwards the request body as opts unfiltered — now at packages/rest/src/external-datasource-routes.ts:465 (the card's :462 had drifted by three lines; same code).

Triage ruled (comment 5383472206, binding): invariant restore — the prefix rule is the declared behavior; loud refusal or forced prefixing, implementer argues which in the PR. That argument follows.

The choice: B (loud refusal), argued against A's cost

A (force the prefix) keeps every request succeeding, which is its whole appeal — no caller breaks today. But it succeeds by silently rewriting a name the caller explicitly asked for: the request says customers, the 201 says wh_customers, and nothing failed loudly. Every caller that recorded its requested name — an AI-authored script, a saved config, a follow-up read of /api/v1/data/customers — now holds a reference to an object that does not exist, and discovers it later, far from the cause. That is the tolerant-consumer accommodation Prime Directive #12 forbids: off-contract input accepted and adjusted downstream, fossilizing "any name works, the platform fixes it" into a de-facto second contract. It would also make this seam the only place in the tree where the prefix rule edits input rather than refusing it — defineStack() refuses at compile time, the publish pre-flight refuses at runtime — a third behavior for one rule. (The derived path's applyNamespacePrefix is not a precedent for A: it adjusts a name the service itself invented; no caller intent is overridden there.)

B's cost, engaged honestly: a request that succeeds today returns 400 tomorrow. It is bounded three ways, all measured rather than assumed: the refusal takes an explicit name in the body (in-tree callers pass none — objectui's ImportObjectDialog posts no name, os datasource introspect does not import; the exposure is the raw route body), and a datasource whose owning package resolves a namespace, and a name that violates the rule — i.e. exactly the requests that were minting objects the platform's authoring path refuses. The 400 carries the rule's own prescription ("Rename it to 'wh_customers' (namespace = 'wh')"), so a broken caller — human or AI — self-corrects in one round trip. A silent rewrite hides the mistake where AI-generated metadata errors propagate; a loud refusal with the exact fix is the structurally safe failure. B also matches what the publish gate already does with the identical violation, keeping one rule at one behavior with one message everywhere it fires.

Shape

  • The gate runs in importObjectbefore the draft is generated: it resolves the namespace through the same injected read and the same normaliseNamespace the derived path uses, and judges opts.name with validateObjectNamespacePrefix — the tree's single source of the rule. Fail-fast placement is deliberate: the verdict needs only the injected namespace read, so a doomed request does not cost a live remote introspection round trip (pinned: the refusal case asserts zero introspect calls).
  • The refusal message is the validator's own authored text, verbatim — the same prescription the publish gate serves for this violation. No second dialect.
  • The throw carries its own status and code — the The direct-mount REST package door answers 500 INTERNAL_ERROR for coded 4xx errors the dispatcher twin maps correctly #8016 thrown-refusal declaration shape, the same pattern as this package's metadataIncompleteError. Stated in prose because the body sanitizer eats short angle-bracket fragments: the helper returns a plain Error object whose code property is the string EXTERNAL_IMPORT_ERROR (the ADR-0112 ledger's registered code for a refused federated import) and whose status property is the number 400. The REST import route answers 400 + EXTERNAL_IMPORT_ERROR with the thrown message for every importObject throw (already pinned in packages/rest/src/external-datasource-envelope.conformance.test.ts, "an import the service refuses"), so the declared envelope and the served envelope agree by construction. Consequently packages/rest/src/external-datasource-routes.ts — permitted only if the shape required it — was not touched.
  • No namespace resolvable, or a blank one: the rule is skipped and the override is accepted, exactly as the derived path and defineStack() behave. Nothing invents a prefix on the caller's behalf.
  • packages/spec untouched (zero ownership in this lane; the fix did not need it — the validator was already exported). The ImportObjectOpts.name doc there still reads "Override the auto-derived object name (snake_case)", which remains true; the refusal semantics are documented at the service seam.

Tests (7 new cases, both directions)

Refusal pins (ADR-0112: code + status, never a bare toThrow):

  • unprefixed override under a namespace: rejects with status 400, code EXTERNAL_IMPORT_ERROR, message byte-equal to the validator's own return (computed via the imported validator, not a copied string), nothing persisted, nothing introspected;
  • legacy FQN form (wh__orders): same envelope, the validator's legacy-form message.

Preservation pins:

  • compliant override (wh_orders) accepted verbatim — no double prefixing;
  • sys_* override accepted (the platform-reserved carve-out every gate on this rule shares);
  • no namespace resolvable: unprefixed override stays accepted;
  • blank namespace treated as absent (same normalisation as the derived path);
  • derived path still prefixed under a namespace when no override is passed (wh_fact_orders).

Reverse verification, from the committed state: stood the base (dd84ddd796) service file up in the fixed tree (mutation confirmed on disk by anchored grep: zero hits of the new helper), re-ran the suite — exactly the two refusal pins went red (import succeeded instead of refusing), all 568 others stayed green; restored from the branch commit (disk-confirmed), 570/570 green. Predicted direction, observed direction: match.

Gates — derived union, measured at head 4a86656751, clean tree

node scripts/pm/dispatch-gates.mjs (no hand-supplied paths; derivation header names this repo at 4a86656751). All to real verdicts, exit codes captured before any pipe:

  • pnpm --filter @objectstack/service-datasource test — "Test Files 26 passed (26) · Tests 570 passed (570)"
  • pnpm --filter @objectstack/service-datasource typecheck — command-exit 0
  • path-matched (12): check:changeset-gate-self-tests, check:objectui-changeset, check:published-files, check:slot-lookup, check:test-source-alias ("72 packages with tests scanned... OK"), check:type-source-resolution, check-adr-0087-registration ("adds no declared-breaking changeset"), check-changeset-no-major, check-ci-filter-parity, check-empty-changeset ("1 declaring changeset(s) added"), check-plugin-teardown-shape, docs-audit/check-affected-docs — all exit 0
  • convention-triggered by the new test code (6): check:query-options-erasure, check:type-check-coverage ("OK — 65/78 workspace packages type-checked"), check:type-check-debt after building the workspace closure ("--re-measure: OK — 33 ledger entr(ies) re-measured, none above its recorded number"), check:engine-double-contract ("OK — 384 pinned"), check:cross-package-test-inputs ("OK: 13 package(s) read outside themselves, all declared"), check:where-matcher — all exit 0
  • standard clause: check:nul-bytes ("OK — scanned 6511 text file(s)... no raw ASCII control bytes")

The full repo farm runs in CI; local scope is the derived union above (declared narrowing: repo-wide scans belong to CI per lane policy).

Contract review (clause-②)

This PR is draft and stays draft: it changes contract accept/reject behavior on a request the API accepts today, so it waits on a contract-review seat at tier. No ready-flip, no auto-merge, and needs:contract-review stays on the card — this lane cannot clear any of those.

Out of scope, recorded


Generated by Claude Code

…olates the namespace prefix
The opts.name override was taken verbatim and persisted through
metadata.register — the one runtime write path no namespace gate looks at.
It now answers to the same ADR-0028 rule the derived name already obeys:
loud refusal (the publish pre-flight's treatment of the identical
violation) with validateObjectNamespacePrefix's own actionable message,
thrown in the #8016 declaration shape (status 400, code
EXTERNAL_IMPORT_ERROR). No namespace resolvable = rule skipped, mirroring
defineStack.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

4 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 38cf397ea9b1b5aea338805e7559f70576b88fb1packageMentionDocs.

Which tree this was computed on

This run read content/docs from 5863fd119c8f10958088328af50745bfe4d2cb17 — the merge of head 4a86656751cf7c9942210d50daa33a0162edb358 into base 38cf397ea9b1b5aea338805e7559f70576b88fb1, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5863fd119c8f10958088328af50745bfe4d2cb17 && git checkout 5863fd119c8f10958088328af50745bfe4d2cb17
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 38cf397ea9b1b5aea338805e7559f70576b88fb1 4a86656751cf7c9942210d50daa33a0162edb358 && git checkout -B drift-repro 38cf397ea9b1b5aea338805e7559f70576b88fb1 && git merge --no-ff 4a86656751cf7c9942210d50daa33a0162edb358
node scripts/docs-audit/affected-docs.mjs --json 38cf397ea9b1b5aea338805e7559f70576b88fb1

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 23, 2026
@claude

claudeBot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

合同审核已通过(PASS),详见卡 #11061 上的判定记录。needs:contract-review 已清除(卡侧;PR 侧本就未挂标)。此 PR 属治理外普通代码面,可按常规流程 merge。


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

importObject's opts.name override bypasses the namespace prefix and persists an unprefixed federated object

2 participants

@os-sam@claude