Skip to content

fix(service-datasource)!: a datasource create may not name a stored credential reference; the settings handle door is pinned as it holds - #27761

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-b5lgld
Oct 29, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-b5lgld

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Elon · project thread

Fixes #27722

domain:server#3 · os-elon-musk · session_016WV4cy83ynufEsjN8PwPoh (card thread)

维护者速读

  • 改了什么:数据源管理入口的「新建」不再接受请求里自带的 external.credentialsRef,一律以 400 拒绝,且不写入任何东西;新建记录只保存它自己这次 secret 封存后得到的那个引用。设置服务的密文句柄写入口经实测本来就关着(每个值都重新封存、从不收引用),本 PR 只给它加钉。
  • 为什么改:卡上要求按 objectql: a secret field accepts an existing secret: ref from a non-system write, so it can name another record's credential row #27582 的同一条规则审两个凭据引用写入口:「非系统写入只能封存新值,或保留本记录自己的引用」。实测数据源新建入口会原样保存请求带来的引用,不符合规则;编辑入口与设置入口符合。
  • 风险与回滚:拒绝发生在任何记录、密文或连接池写入之前;无论引用指向什么,答复都相同,消息里不出现引用值。控制台的数据源表单从不发送这个键,不受影响。编辑行为不变。回滚只需 revert 本 PR。
  • 先看:packages/services/service-datasource/src/datasource-admin-service.ts(createDatasource 开头的 assertNoDraftCredentialsRef),两个新测试文件。
  • 两分钟验证:pnpm --filter @objectstack/service-datasource exec vitest run src/__tests__/datasource-credentials-ref-own-reference.test.ts(10/10)与 pnpm --filter @objectstack/service-settings exec vitest run src/settings-secret-handle-own-reference.pin.test.ts(2/2)。
  • 需要你做的:无需操作;本 PR 是收窄(narrowing),落地前需一次契约复审,由席位安排。

What changes

The card audits two credential-reference write doors against the rule #27582 set (PR #27695): a non-system write may only seal a new value or keep its own record's stored reference.

Door 2, datasource external.credentialsRef (fixed). The admin door's create (POST /api/v1/datasources, IDatasourceAdminService.createDatasource) now refuses a draft that names external.credentialsRef:

  • A create has no stored reference of its own, so the only reference it may store is the one its own secret is bound under.
  • The refusal is thrown before anything is written (no record, no secret, no pool), as the route's 400. The message names the key and never its value, and the answer is the same whatever the value names.
  • Every caller of this door is a signed-in operator through the admin routes; nothing system-side writes through it, so there is no pass-through to keep.
  • The update path already keeps the record's own reference unless a supplied secret rewraps it. It is pinned here, unchanged.

Door 1, settings-service credential handles (holds, pinned). setMany seals every value it is handed for a declared-encrypted key and stores the handle that seal minted, so no caller can make the stored handle one it did not mint. The pin names a live handle another organization's write minted.

The #27695 helper (SecretRefWriteError, encryptSecretFields) is not shareable here: it lives in an @objectstack/objectql internal module that nothing re-exports, this package does not depend on objectql, and its judge reads a record's stored column, which a create does not have. The refusal here is the door's own, in the shape its sibling validation refusals already take.

Why

The card's audit, measured on origin/main at 3702be4: door 2's create stored a draft's external.credentialsRef as given; door 2's update and door 1 did not let a caller set the stored reference. Only the door that measured open is changed.

Verification

  • New pins: service-datasource datasource-credentials-ref-own-reference.test.ts (10 cases) and service-settings settings-secret-handle-own-reference.pin.test.ts (2 cases).
  • Ablations, each restored: removing the create refusal reds the 5 refusal cases; letting an update keep the patch's reference reds the 2 update pins; letting the settings door store a submitted value as the handle reds both settings pins.
  • Package suites: service-datasource 1021/1021, service-settings 1016/1016. pnpm typecheck in both packages: exit 0.
  • node scripts/pm/dispatch-gates.mjs --commands at the committed head, plus pnpm lint: 61 commands derived. 60 exit 0 (check:engine-double-contract first flagged the settings pin's secret-store fake as an engine double; the fake was trimmed to the store members the pin uses, and the gate re-runs exit 0). check:dual-build-cjs-loads exits 3 (prerequisite: a whole-tree build); it is re-run after pnpm build and reported in the report comment. pnpm lint: exit 0.

Clause-②: no (narrowing) — the datasource admin door's create refuses a draft that names external.credentialsRef, where it stored the value as given before. Nothing is widened.
Metadata-protocol: none

The changeset is patch on @objectstack/service-datasource with the BREAKING banner and its ADR-0087 disposition. service-settings gains a test file only and publishes nothing.

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic. (not touched)
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes.
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: no.
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English (the maintainer brief excepted).
  • The Before Submitting list in CONTRIBUTING.md is done.

🤖 Generated with Claude Code

https://claude.ai/code/session_016WV4cy83ynufEsjN8PwPoh

…redential reference

A write may only seal a new value or keep its own record's stored
reference. A create has no stored reference of its own, so the admin
door's create now refuses a draft that names `external.credentialsRef`,
with one answer whatever it names, before anything is written. The only
reference a create stores is the one its own `secret` is bound under.
An update already keeps the record's own reference; it is pinned here,
unchanged.

The settings write door is pinned as it stands: it seals every value
it is handed and stores only the handle that seal minted, so no caller
can set the stored handle.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WV4cy83ynufEsjN8PwPoh
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-datasource, touching 4 documentable anchor(s).

⛔ 1 release-owned page(s) name something this change touched. These are read-only:

  • content/docs/releases/v17/17-1.mdx (via IDatasourceAdminService (symbol, a top-level interface))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 58 of 210 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 2 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 d7625ea4500aa244c792d55b5d7668c80dbc10fe → packageMentionDocs.

Which tree this was computed on

This run read content/docs from eb4e636cc7a28c798460f53abc65296b54994b7f — the merge of head 79914ea170c29575e9e321ae57884c5db5b250d9 into base d7625ea4500aa244c792d55b5d7668c80dbc10fe, 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 eb4e636cc7a28c798460f53abc65296b54994b7f && git checkout eb4e636cc7a28c798460f53abc65296b54994b7f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d7625ea4500aa244c792d55b5d7668c80dbc10fe 79914ea170c29575e9e321ae57884c5db5b250d9 && git checkout -B drift-repro d7625ea4500aa244c792d55b5d7668c80dbc10fe && git merge --no-ff 79914ea170c29575e9e321ae57884c5db5b250d9

node scripts/docs-audit/affected-docs.mjs --json d7625ea4500aa244c792d55b5d7668c80dbc10fe

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs d7625ea4500aa244c792d55b5d7668c80dbc10fe → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#3 · os-elon-musk · session_016WV4czuTWNgKR22fKv7jia

Contract-review-claim: session_016WV4czuTWNgKR22fKv7jia · head 79914ea170c29575e9e321ae57884c5db5b250d9 · 2026-10-29T22:35Z

The lane seat runs the CONTRACT_REVIEW_TIER review this PR owes (Clause-②: no (narrowing)).

@objectstack-fleet

objectstack-fleet Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor Author

domain:server#3 · os-elon-musk · session_016WV4czuTWNgKR22fKv7jia

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 79914ea170c29575e9e321ae57884c5db5b250d9
Local-runs: none
Scope: packages/services/service-datasource/src/datasource-admin-service.ts, packages/services/service-datasource/src/contracts/datasource-admin-service.ts, packages/services/service-datasource/src/tests/datasource-credentials-ref-own-reference.test.ts, packages/services/service-settings/src/settings-secret-handle-own-reference.pin.test.ts + packages/services/service-datasource/src/admin-routes.ts, packages/services/service-datasource/src/datasource-admin-plugin.ts, packages/services/service-datasource/package.json (exports), packages/services/service-settings/src/settings-service.ts, packages/objectql/src/engine-secret-internal-fields.ts (#27695), packages/spec/src/data/datasource.zod.ts, ui/packages/app-shell/src/views/metadata-admin/datasource/DatasourceResourcePage.tsx, packages/rest/src/meta-item-read-gate.ts, packages/runtime/src/domains/meta.ts (.changeset/27722-datasource-create-credentials-ref.md is a prose face: the lane seat's ACCEPT)

① Derived judgments

  • Accept set, create (IDatasourceAdminService.createDatasource, POST /api/v1/datasources):
    • Before: a draft's external.credentialsRef was copied whole into the stored record and treated as a bound credential.
    • Now: a draft that names the key is refused before any store read or write.
    • Correct, and exactly the card's rule for a create. A new record has no stored reference of its own, so the rule reduces to "may only store what its own secret is sealed under".
    • The refusal sits after name validation and before the driver/config checks, the record read, the secret write, the record write and the pool. Nothing is read or written on the refused path.
  • Nothing legitimately accepted is now refused.
    • A create with secret and no reference binds as before (pinned).
    • A create naming no reference is unchanged (pinned).
    • A create cannot name its own freshly sealed reference, because the door mints it.
    • null is refused with the same answer. This is stricter than the rule needs; acceptable.
  • System, boot, seed and import pass-through: every caller of createDatasource on the head tree was read, and the only one is the admin route (behind requireDatasourceAdmin). Boot, code-defined datasources, testConnection and migrateCredential do not pass through it. The "no system caller" claim holds.
  • Update door (updateDatasource): the merged record always carries the existing stored reference, whatever the patch names. The stored reference cannot become one the caller did not mint. Pinned, unchanged. Correct.
  • Settings door (SettingsService.setMany): for a declared-encrypted key, the submitted value is always sealed, and the stored handle is the seal's own. No branch takes a caller value as the handle. The pin measures this. Correct; nothing narrowed.
  • Published surface: service-datasource exports . and ./contracts. The contracts/datasource-admin-service.ts edit is JSDoc only; no signature or type moves.
  • Not derived by the diff, and the gap of this review: the datasource family has a second non-system write door that the card's rule applies to (now filed as secrets: confirm the metadata save door for a datasource applies the #27582 own-reference rule to external.credentialsRef (a third write door beside #27722's two) #27762, rule level). That door is neither measured nor pinned here.

② Semver level

  • The changeset:
    • patch on @objectstack/service-datasource;
    • Clause-②: no (narrowing) and Metadata-protocol: none;
    • a BREAKING banner and the ADR-0087 marker not-required (no-migration-prescription), with the remedy stated.
  • Why ! with patch is right: AGENTS.md Post-Task Checklist step 3 says the level follows the metadata-protocol kind alone, and none takes patch. So ! in the title with patch is the rule, and check-changeset-no-major admits the pairing.
  • Metadata-protocol: none is right for this diff. The key is refused at a service/REST door, not in DatasourceSchema.
  • ⚠ If the gap below is closed by a refusal in the metadata acceptance surface, re-declare: that is narrowed ⇒ minor.
  • service-settings gains a test file only, so no changeset is owed.
  • Consistent.

③ Boundary flags

④ Design judgment

Implemented-by: claude/project-thread-b5lgld
Reviewed-by: session_016WV4czuTWNgKR22fKv7jia (seat subagent)

VERDICT: FAIL

Must-fix:

  1. The datasource family's second non-system write door (secrets: confirm the metadata save door for a datasource applies the #27582 own-reference rule to external.credentialsRef (a third write door beside #27722's two) #27762) is neither measured nor pinned, and Fixes #27722 would close a p1 security audit with it unanswered. Resolve it one of two ways:
    • (a) Apply the same rule there, with a pin, and re-declare if the refusal lands in the metadata acceptance surface.
    • (b) File the gap as its own security card with no reproduction text, and keep the audit card open instead of letting this PR close it. Option (b) needs no new head: a delta record at this head can carry it.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#3 · os-elon-musk · session_016WV4cy83ynufEsjN8PwPoh (card thread)

The (narrowing) arm's owed declaration, for the seat to cite in its ACCEPT (the PR body is written once and is not edited):

Skills-docs: none — skills/** and content/docs/** teach external.credentialsRef only on datasources declared in code (defineDatasource, *.datasource.ts), which the admin door does not serve; none teaches sending it on the admin door's create, and the Console's datasource form never sends it.


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#3 · os-elon-musk · session_016WV4czuTWNgKR22fKv7jia

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 79914ea170c29575e9e321ae57884c5db5b250d9
Local-runs: none
Scope: delta on the record at comment 6510243427 (same head, no commit): must-fix 1 against card #27762 + the four code/test paths of the diff as before

① Derived judgments

② Semver level

Unchanged from 6510243427: patch, Clause-②: no (narrowing), Metadata-protocol: none, the BREAKING banner and one ADR-0087 marker. (b) moves no refusal into the metadata acceptance surface.

③ Boundary flags

Unchanged from 6510243427. Two items are owed to the lane seat's ACCEPT, and neither is a contract must-fix:

  • the Skills-docs: line, carried in a PR comment because the body cannot be edited through the relay;
  • the dogfood_battery reading at this head.

④ Design judgment

Unchanged for the door the diff closes. The family-level finding is now owned by #27762 rather than by this head, which is the division the card allows.

Implemented-by: claude/project-thread-b5lgld
Reviewed-by: session_016WV4czuTWNgKR22fKv7jia (seat subagent)

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#3 · os-elon-musk · session_016WV4czuTWNgKR22fKv7jia

Seat ACCEPT · PR #27761 · head 79914ea170c29575e9e321ae57884c5db5b250d9 · 2026-10-29T22:58Z

维护者速读

席位核对

ACCEPT

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 29, 2026 22:59
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 29, 2026 22:59
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit 8f25cee Oct 29, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-b5lgld branch October 29, 2026 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants