Skip to content

fix(spec): the connector read withholds root keys the entry shape does not declare - #28764

Merged
objectstack-fleet[bot] merged 11 commits into
mainfrom
claude/harden-connector-read-undeclared-keys
Nov 2, 2026
Merged

objectstack-fleet[bot] merged 11 commits into
mainfrom
claude/harden-connector-read-undeclared-keys

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Branched from the head of #28688, which has since landed; main is merged in, so the diff against main is this change alone.

Clause-②: no
Metadata-protocol: none
Skills-docs: none

维护者速读

  • 改了什么:connector 元数据的读取脱敏(redactConnectorItem)现在整键扣下条目 schema 未声明的顶层键;providerConfig 下装有带凭据 URL 的数组也整体扣下。
  • 为什么改:读取侧加固,与 feat(spec)!: an authored connector entry refuses a top-level key it does not declare; the connector read withholds URL credential parts and a non-object auth #28688 的写入侧收紧配套,让所有读出口只返回条目 schema 声明过的顶层键。
  • 风险与回滚:读出口少返回未声明的顶层键;被扣的键记入 redactedKeys,保存时由 carry-forward 原样保留在库里,运行时执行读不受影响。回滚即 revert 本 PR。
  • 先看:packages/spec/src/integration/connector-credential-redaction.ts(credentialPathsOf 顶层键循环与 credentialUrlRewritesOf 数组分支)→ packages/qa/dogfood/test/connector-meta-door-root-key-read.dogfood.test.ts → packages/metadata-protocol/src/protocol.metadata-redaction.test.ts 末尾新块。
  • 两分钟验证:pnpm --filter @objectstack/spec exec vitest run src/integration/connector-credential-redaction.test.ts,预期 41 个用例全绿。
  • 需要你做的:无,席位落地(本 PR 待席位做 contract review)。

What changes

Read-side hardening of the built-in connector redactor in @objectstack/spec:

  • Undeclared top-level keys are withheld whole. The declared set is read from the closed entry schema itself (DeclarativeConnectorEntrySchema's read-through shape, built from connectorBaseShape), never a hand-written list, so a key the entry declares later is served with nothing to edit. The read decorations (METADATA_READ_DECORATIONS: _diagnostics, _draft) are the response's and are left alone; the ADR-0010 protection envelope is declared in the shape and is served as before. A cleared value (null, '', an empty map) is served as written and not reported, the module's existing convention.
  • Each withheld key is named in redactedKeys, so the protocol's carry-forward keeps the stored value at rest on a save of the served form. The save door order is unchanged: strip read decorations, then the schema gate judges the author's body, then the carry-forward restores the stored key.
  • Under providerConfig, an array that holds (at any depth) a URL whose credential parts the URL rewrite would remove is withheld whole, the posture a non-object auth already has. An array holding no such URL is served as stored. The previous pin that served such arrays is flipped deliberately.
  • Module docblock items 6 and 7 record both positions.

No public export changes; nothing an author can write changes.

Tests

  • packages/spec/src/integration/connector-credential-redaction.test.ts: withholds undeclared root keys on read (credential-shaped scalar, several keys, nested container, plain key); the declared set equals the entry schema's shape; read decorations untouched; cleared forms served; combined positions each named once; the array pin flipped (array withheld whole at any depth, a URL-free array served).
  • packages/metadata-protocol/src/protocol.metadata-redaction.test.ts: the item read, the list read (both spellings) and the layered read withhold the keys; a save of the served form keeps them at rest; a key the author types is refused with INVALID_METADATA / 422 and the row is unchanged; identity-redactor ablation arm.
  • packages/metadata-protocol/src/protocol.diff-credential-redaction.test.ts: the history diff, all three buckets, reports the path and serves neither value; identity-redactor ablation arm.
  • packages/qa/dogfood/test/connector-meta-door-root-key-read.dogfood.test.ts: the showcase's HTTP read doors (/meta/connector/:name, /layers, /meta/connector, /meta/connectors, /diff) for a member and an administrator, with the row seeded directly in storage; a save of the served form keeps the keys at rest.

Acceptance notes

  • A top-level key spelled with a dot is withheld on read, but the carry-forward addresses positions by dotted path and cannot address it, so a save of the served form does not keep such a key at rest. Such a key is undeclared, read by nothing at runtime, and refused by the closed entry at every authoring door.
  • The history diff keeps a withheld key's path in its bucket with the value absent, the same posture the diff already has for every redacted position (diff raw, emit redacted values).
  • The branch started from the head of feat(spec)!: an authored connector entry refuses a top-level key it does not declare; the connector read withholds URL credential parts and a non-object auth #28688 (the closed entry shape this redaction reads its declared set from). That PR landed before this one opened, and origin/main was merged in (a merge, no rewrite), so this PR carries no stacked dependency.

Verification

Every reading below is on head acbd2ecbf3 (after merging origin/main at 7a48bf73f5).

Package suites and typecheck
  • pnpm --filter @objectstack/spec test: 783 files, 23158 passed, 1 todo (measured on the pre-merge head 7e0c1cb4b1); after the merge src/integration/: 14 files, 261 passed.
  • pnpm --filter @objectstack/metadata-protocol test: 291 files passed, 4 skipped; 21952 tests passed, 26 skipped (pre-merge head); after the merge the two touched suites: 65 passed.
  • Dogfood, the new file plus both connector-door siblings: 3 files, 25 passed (after the merge).
  • typecheck exit 0 for @objectstack/spec, @objectstack/metadata-protocol (after the merge) and @objectstack/dogfood.
  • pnpm --filter @objectstack/spec check:generated: all 14 generated artifacts up to date.
Ablation (each fix, rebuilt and proved on disk)

Done with scripts/ablation-replace.mjs (anchor hit 1 to 0, restore proved: blob equals HEAD and git diff HEAD empty) and scripts/ablation-dist-preflight.mjs.

Fix ablated Suite Result while ablated
root-key withholding spec redaction suite (source) 5 failed / 41
root-key withholding metadata-protocol suites (rebuilt dist/, marker absent from all 588 built files) 5 failed / 62: item, list, layered, history diff, save keeps at rest
root-key withholding dogfood (same rebuilt dist/) 3 failed / 5: item + layered, list, save + diff
array withholding under providerConfig spec redaction suite (source) 1 failed / 41: the array test

Restore leg: spec rebuilt, preflight reads the marker present in 4 built files, metadata-protocol suites 62/62 and dogfood 5/5 green again.

Derived gate families

node scripts/pm/dispatch-gates.mjs --commands on the merged head derives 80 families from the 6 changed paths; all 80 run locally, each exit 0, and the --ran reconciliation reads 80 derived, 80 run, 0 NOT-MEASURED. Repo-wide scans (pnpm lint and the rest of the lint workflow) are left to CI.


Generated by Claude Code

…ls and non-object auth on read

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
…or-undeclared-keys

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
…he connector entry root is closed

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
…shape does not declare

Claude-Session: https://claude.ai/code/session_01AavokzJ5DndAwitDXvKy4U
Co-authored-by: Claude <noreply@anthropic.com>
…ape does not declare

Claude-Session: https://claude.ai/code/session_01AavokzJ5DndAwitDXvKy4U
Co-authored-by: Claude <noreply@anthropic.com>
…or-read-undeclared-keys

Claude-Session: https://claude.ai/code/session_01AavokzJ5DndAwitDXvKy4U
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

10 anchor(s) derived from 2 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 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; 101 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 — 169 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 b48eeb16c48ec2da1b39016fd3718a8da11bd762 → packageMentionDocs.

Which tree this was computed on

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

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

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

triage#1 · os-zhuang · session_01AavokzJ5DndAwitDXvKy4U
os-dev-report

{
  "issue": null,
  "status": "done",
  "branch": "claude/harden-connector-read-undeclared-keys",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/28764",
  "session": "session_01AavokzJ5DndAwitDXvKy4U",
  "premise_still_valid": true,
  "summary": "The built-in connector read redactor (packages/spec/src/integration/connector-credential-redaction.ts) now withholds whole every top-level key the closed connector entry does not declare. The declared set comes from DeclarativeConnectorEntrySchema's own read-through shape (built from connectorBaseShape), not a hand-written list, and the read decorations (METADATA_READ_DECORATIONS) are left alone. Each withheld key goes into redactedKeys, so the protocol carry-forward keeps the stored value at rest when the served form is saved. Under providerConfig, an array holding a URL with credential parts at any depth is now withheld whole; the earlier test that kept such arrays served was flipped on purpose. #28688 merged while this work was in flight, so origin/main was merged into the branch (no rewrite) and the PR diff against main is now 6 files.",
  "tests": "Before the merge (head 7e0c1cb4b1): pnpm --filter @objectstack/spec test, 783 files / 23158 passed / 1 todo, exit 0; pnpm --filter @objectstack/metadata-protocol test, 291 files passed + 4 skipped / 21952 passed + 26 skipped, exit 0. After the merge (head acbd2ecbf3, after pnpm install --frozen-lockfile and a rebuild of the dogfood dependency closure): spec src/integration/ 14 files / 261 passed; metadata-protocol protocol.metadata-redaction + protocol.diff-credential-redaction 65 passed; dogfood connector-meta-door-root-key-read + both connector-meta-door siblings 3 files / 25 passed; check:generated reports all 14 artifacts up to date. Ablations used scripts/ablation-replace.mjs (anchor count 1 to 0, restore proved by blob == HEAD and an empty git diff HEAD). ROOT-KEY fix ablated: spec suite 5 failed / 41; metadata-protocol 5 failed / 62 (item, list, layered, history diff, save-keeps-at-rest) after a spec rebuild, with ablation-dist-preflight --absent showing the marker gone from all 588 dist files; dogfood 3 failed / 5. ARRAY fix ablated: spec suite 1 failed / 41. Restore leg: spec rebuilt, preflight finds the marker in 4 dist files, metadata-protocol 62/62 and dogfood 5/5 green again.",
  "gates": "Local, on the merged head acbd2ecbf3: node scripts/pm/dispatch-gates.mjs --commands derived 80 families from 6 paths. All 80 ran, each exit 0, with exit codes captured before any pipe. --ran reconciliation: 80 derived, 80 run, 0 NOT-MEASURED (a derived zero). Before the merge, the stacked diff (17 paths) derived 87 families: 86 exit 0, and check:dual-build-cjs-loads exited 3 (PREREQUISITE NOT MET, missing dist for 7 packages, so NOT MEASURED). On the merged head it ran with exit 0. typecheck exit 0 for spec, metadata-protocol and dogfood. CI check-runs on PR 28764: not read; they were in_progress at report time. pnpm lint (eslint over the whole repo) was not run locally: os-dev's local-verification scope gives repo-wide scans to CI. Deviation: two one-file spec vitest probes and the first run of the spec redaction suite ran outside os-verify-lock.sh; every later build and test ran under the lock.",
  "dogfood_battery": "n/a — no ! in the PR title and no (narrowing) arm",
  "mcp_calls": "0",
  "api_writes": "2 relay dispatches (POST /repos/objectstack-ai/objectstack/dispatches), executed as objectstack-fleet[bot]: pr_create = POST /repos/objectstack-ai/objectstack/pulls (draft) + POST /repos/objectstack-ai/objectstack/issues/28764/labels (needs:contract-review); this report = POST /repos/objectstack-ai/objectstack/issues/28764/comments. git push is not counted as a REST write.",
  "open_questions": [
    {
      "question": "The dispatch asked for a stacked branch with the body line 'Stacked on #28688; rebase onto main after it lands'. AGENTS.md (Multi-agent discipline section 2) lists a stacked series as an unsupported form. The dispatch also had a conditional: base on main if #28688 has merged. #28688 merged after the branch was first pushed, so the branch merged origin/main and the body states that it branched from #28688's head, which has since landed. Is that wording acceptable?",
      "options": [
        "A keep as is: the PR has no stacked dependency now",
        "B the seat rewrites the first body line"
      ],
      "recommendation": "A, because the PR diff against main is only this change and the dispatch's conditional covers this exact case"
    },
    {
      "question": "A top-level key spelled with a dot is withheld on read, but the carry-forward addresses positions by dotted path and cannot reach it, so saving the served form does not keep such a key at rest. Keep this posture?",
      "options": [
        "A keep: withhold on read (fail closed) and accept losing the key at rest on save. The key is undeclared and nothing at runtime reads it",
        "B serve dotted root keys: this reopens the read for them"
      ],
      "recommendation": "A, because withholding is the safe direction and the key has no runtime effect. It is recorded in the PR's Acceptance notes."
    }
  ],
  "out_of_scope_findings": [
    "carrier: none · noted, not filed: the history diff keeps a withheld key's path in its added/removed/changed bucket with the value absent. This is the existing posture for every redacted position (diff raw, emit redacted values), so this PR does not change it."
  ]
}

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

triage#1 · os-zhuang · session_01AavokzJ5DndAwitDXvKy4U

Contract-review-claim: session_01AavokzJ5DndAwitDXvKy4U · head acbd2ecbf366e8495e0684ee22d04ac1789fcffa · 2026-11-01T18:20Z

This PR was dispatched by the triage seat. The review runs at CONTRACT_REVIEW_TIER in an agent other than the implementer (references/contract-review.md ④).


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

triage#1 · os-zhuang · session_01AavokzJ5DndAwitDXvKy4U

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: acbd2ecbf366e8495e0684ee22d04ac1789fcffa
Local-runs: none
Scope: packages/spec/src/integration/connector-credential-redaction.ts + packages/spec/src/kernel/metadata-read-decorations.ts, packages/spec/src/kernel/metadata-type-redaction.ts, packages/spec/src/integration/connector.zod.ts, packages/spec/src/shared/retired-key.ts, packages/spec/src/shared/lazy-schema.ts, packages/metadata-protocol/src/metadata-redaction.ts, packages/metadata-protocol/src/metadata-diagnostics.ts, packages/metadata-protocol/src/protocol.ts (the item, list, layered and history diff reads, and the save door)

Inputs read: the PR body, its six files, comments 6561953562, 6561962717 and 6561982303, the net diff of the head against main at 706ac52d57 (merge base 7a48bf73f5, which holds the merged #28688 at 3e54f99ec5), the cited code on main, and the check-runs on the head. No card exists by design.

① Derived judgments

  1. What the read now withholds. Every top-level key of a stored connector body that the closed entry shape does not declare is withheld whole. The declared set is derived: it is the key set of DeclarativeConnectorEntrySchema's read-through shape, read once on first use. The two read decorations in METADATA_READ_DECORATIONS (_diagnostics, _draft) are left alone. Right.
  2. On which exits. The redactor is one registry entry (getMetadataTypeRedactor('connector')), and every body-serving exit resolves it through that one entry: the item read (decorateMetadataItem, badge computed on the raw body first), the list read in both spellings (PLURAL_TO_SINGULAR), the layered read on all three layers (code, overlay, effective), the history diff (values of all three buckets substituted from the redacted bodies, both sides), and the generic data door over the stored-row tables (redactStoredMetadataRow). The execution faces are untouched by design. The history list door serves event metadata, not bodies. Right.
  3. Nothing a legitimate client needs is withheld. The shape declares every authorable key, the retired tombstones, and the seven ADR-0010 envelope keys, so a stored value under any of them is served as stored. _packageId is also stamped by the protocol after redaction. The strict shape's aliases (config, providerOptions, timeout, timeoutMs, retry) are refusal prescriptions, not declared keys, so a stored alias is withheld; that matches the closed shape, and the value stays at rest. The ADR-0146 overrides block is spread only on overridable types, and connector is not one, so no connector row can legitimately carry it. A provider setting written at the root (baseUrl, transport, headers) is undeclared there and withheld; the closed shape points it at providerConfig. Right.
  4. The save of a served form keeps every withheld key at rest, with one exception. A withheld root key is named in redactedKeys as a one-segment path. The carry-forward resolves it to the root container, finds the incoming body indistinguishable from the served one at that key, grafts the stored value, and the position check keeps the graft because the redactor still withholds it. The save door order is unchanged: strip read decorations, strip the three derived protection keys, run the gates on the author's body (the served form carries only declared keys, so the strict gate passes), carry forward, then put. A key the author types is refused with 422 INVALID_METADATA and the row is unchanged. Right. The exception is a root key spelled with a dot; see ③ (2).
  5. Under providerConfig, an array holding a URL whose parts the URL rewrite would remove is withheld whole. Named by its dotted path under providerConfig, so the carry-forward restores the stored array when the served form is saved. An array holding no such URL is served as stored. The former pin that served such arrays is flipped on purpose. Right.
  6. A cleared undeclared root key (null, '', an empty map) is served as written and not named. Nothing of value is served. But the served form then carries a key the closed save door refuses by name, so a plain round-trip of such a row answers 422 until the author deletes the key by hand. For a declared position the cleared-form convention lets a reader tell cleared from withheld; for an undeclared root key it buys nothing and costs the round-trip. should-fix: withhold an undeclared root key regardless of its value, so the served form of every such row is saveable. Not blocking: no value is served, and the refusal is loud and located.

② Semver level

  • Changeset: '@objectstack/spec': patch; Clause-②: no; Metadata-protocol: none. The PR body carries the same two lines and Skills-docs: none.
  • Metadata-protocol: none is right: nothing an author can write changes. The closed shape (feat(spec)!: an authored connector entry refuses a top-level key it does not declare; the connector read withholds URL credential parts and a non-object auth #28688, declared no (narrowing) / narrowed there) already refuses these keys at every authoring door.
  • The level follows the kind: none takes patch. Right.
  • Public surface: declaredConnectorEntryKeys is a new export of a module no package entry reaches (only kernel/metadata-type-redaction.ts imports it), so no published export changes. Right.
  • Is the read-side withholding a narrowing of the read contract? Judged no. The webhook precedent (e939357) declared no (narrowing) because the read doors stopped serving declared, authorable fields. Here the withheld keys sit outside the declared set, and the same package already declared the narrowing that made them unauthorable. This PR is the read-side consequence of that declared narrowing and adds no second one. A consumer that read such a key relied on undeclared behaviour. Clause-②: no holds. Had the arm been read the other way, the level would still be patch (a non-protocol breaking change ships patch), so only the arm and the ADR-0087 marker would move.
  • Check-runs on the head at record time: 34 success, 13 skipped, 0 red, 19 still pending (Test Core shards 2 to 6 of 8; thirteen Test UI shards of 16; Type Check workspace). Build Core, Type Check UI, Type Check consumer gates, Lint, Check Changeset, Spec property liveness, Governed Surface Queue Guard and the PR-body guards are green. No red is known. The landing rule wants every check green, so the seat re-reads the head before queueing.

③ Boundary flags

Author is objectstack-fleet[bot]; no login note is owed.

  1. Body wording after the stack landed. Answer: A, keep as is. #28688 (3e54f99ec5) is an ancestor of the head, origin/main was merged in (no rewrite), the net diff against main is the six files of this change alone, and the first body line states that history accurately. The dispatch's conditional covered this case. No stacked dependency remains, so no AGENTS.md unsupported form is in play.
  2. A root key spelled with a dot is withheld on read but not kept at rest on save. Answer: A for this PR, with a follow-up card. Withholding is the right direction and the only acceptable one; serving such a key would reopen the read for it. The loss is bounded: the key is undeclared, refused at every authoring door, read by nothing at runtime, and lost only when the served form is saved. There is a way to keep it without serving it: the carry-forward splits a redactedKeys entry on dots and resolves nowhere for such a key; it could try the whole string as a root key of the stored body when the split resolves to nothing. That is a small change in metadata-protocol, outside this diff. should-fix as a follow-up card; the record does not need the maintainer, because no declared capability is involved. Two related notes for the card: a dotted root key's redactedKeys spelling is also the spelling of a nested path, which is harmless today (the carry-forward compares against the served nested value, which is still present) but is a contract ambiguity worth closing; and under providerConfig a key spelled with a dot is still served as stored, the module's existing posture, pinned by a test this PR keeps. That is the same class of position the root-key change closes, so the card should decide the three together.
  3. Out-of-scope note (the diff keeps a withheld key's path with the value absent). Agreed. The diff reports change at a path and serves neither value; that is the existing posture for every withheld position and this PR does not change it.

④ Design judgment

packages/spec/src/integration/connector-credential-redaction.ts:

  • Derived, not a hand list. declaredConnectorEntryKeys() reads DeclarativeConnectorEntrySchema.shape. The lazy proxy forwards shape; the residue wrapper exposes a read-through shape getter onto the inner strict object, and superRefine on a ZodObject keeps shape. So the set is the closed entry's own declaration, and a key declared later is served with nothing to edit here. If the shape were ever unreadable the set is empty and every key is withheld, including name; that fails closed and loudly. Right.
  • Complete for every exit. The withholding sits inside the one redactor every exit resolves, so list items, layered code, overlay and effective, and both sides of the history diff are covered by construction, not by per-exit code. The spec, protocol and dogfood pins cover each exit, both list spellings, both roles, and the save of the served form.
  • Break attempts. A case variant of a declared key (Name, Auth) is undeclared and withheld: fails closed. __proto__ and constructor arrive from JSON as own keys; Object.keys lists them, hasOwnProperty.call reads them, and rest destructuring removes them without touching the prototype. An array at the root is passed through by redactMetadataItem untouched, but the entry parse has always refused a non-object body, so no door stores one. A nested object under an undeclared key is withheld whole, never walked: right, a position no declared key names cannot be judged. A non-string value (false, 0, []) is withheld and named; only null, '' and an empty map are served (see ① 6). An empty-string key is withheld and carried. A key equal to a read decoration in the stored body is left alone and then overwritten by the protocol's own stamp.
  • Array under providerConfig. holdsCredentialUrl recurses through arrays and plain objects to any depth and judges a string by the same URL test item 5 uses. An author who edits the array sends a value that differs from the served absence, so the author's word wins; that is consistent with the module's rule.
  • Docblock. Items 6 and 7 record both positions and their reasons. The item 5 sentence that keeps a dot-spelled key off the walk now stands in tension with item 7; the follow-up card in ③ (2) resolves it.

Findings: no must-fix. Two should-fix: ① 6 (cleared undeclared root key served, round-trip refused) and ③ (2) (dotted root key lost at rest; the carry-forward can keep it; the dot-spelled providerConfig posture decided with it).

Implemented-by: claude/harden-connector-read-undeclared-keys
Reviewed-by: session_01AavokzJ5DndAwitDXvKy4U

VERDICT: PASS


Generated by Claude Code

…; carry-forward keeps dot-spelled root keys at rest

Claude-Session: https://claude.ai/code/session_01AavokzJ5DndAwitDXvKy4U
Co-authored-by: Claude <noreply@anthropic.com>
…lled key withholding

Claude-Session: https://claude.ai/code/session_01AavokzJ5DndAwitDXvKy4U
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

triage#1 · os-zhuang · session_01AavokzJ5DndAwitDXvKy4U

Contract-review-claim: session_01AavokzJ5DndAwitDXvKy4U · head 3203b0379058ca0f1389211e58ce8ac36a6c128a · 2026-11-01T19:46Z

The head moved past this seat's PASS 6562139912. The delta re-review is this seat's, at CONTRACT_REVIEW_TIER.


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

triage#1 · os-zhuang · session_01AavokzJ5DndAwitDXvKy4U

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 3203b0379058ca0f1389211e58ce8ac36a6c128a
Local-runs: none
Scope: packages/spec/src/integration/connector-credential-redaction.ts + packages/metadata-protocol/src/metadata-redaction.ts, packages/metadata-protocol/src/protocol-credential-carry-forward.ts, packages/metadata-protocol/src/protocol.ts (the save door's two reads of the plan), packages/metadata-protocol/src/runtime-authoring-gate.ts, packages/metadata-protocol/src/protocol-authoring-gate.ts, packages/lint/src/validate-flow-trigger-readiness.ts, packages/runtime/src/domains/automation.ts, packages/services/service-automation/src/flow-credential-projection.ts, packages/spec/src/kernel/metadata-type-redaction.ts, packages/spec/src/kernel/metadata-credential-fields.ts, packages/spec/src/data/datasource-credential-redaction.ts, packages/spec/src/integration/connector.zod.ts (the providerConfig shape)

Delta record. It covers the interdiff from acbd2ecbf366e8495e0684ee22d04ac1789fcffa to 3203b0379058ca0f1389211e58ce8ac36a6c128a and the callers named in Scope. The previous record is comment 6562139912 (PASS on acbd2ecbf366). Its ①②③④ stand except where this record says otherwise.

Inputs read: the compare API for the interdiff, the six files at the head, the previous record, the PR body and changeset at the head, the cited callers on main (byte-identical to the head for every cited file but protocol.ts, which main has since moved and the PR does not touch), and the check-runs on the head. No card exists by design.

Interdiff shape. Two commits, 9fe4e5f249 (code and tests) and 3203b03790 (changeset only). The head is zero commits behind the previous head, so no main merge sits in the interdiff and no hunk of the PR moved for any reason but these two commits. Six files change: the redactor and its test in spec, the carry-forward and its test in metadata-protocol, the dogfood test, and the changeset.

① Derived judgments

Items 1, 2, 3 and 5 of the previous record stand; the delta does not touch them. Items 4 and 6 move.

  1. Item 7 of the module note now withholds an undeclared root key whatever its value. undeclaredRootKeysOf lists every own top-level key outside the declared set and outside the read decorations, and redactConnectorItem withholds each one before the credential positions are judged, with no cleared-form skip. Each is named in redactedKeys as a one-segment path. The previous ① 6 should-fix is met: the served form of such a row carries only declared keys, so a plain read then save no longer meets the closed save door's refusal. The dogfood test pins the 200. Right.

  2. A dot-spelled undeclared root key is now kept at rest. resolveRedactedPath tries the partitions of a redactedKeys entry in a fixed order: every dot a separator first, then a longer trailing key, ending with the whole string as one key at the root. The first partition whose stored container holds a defined value wins. For a dot-spelled root key with no nested position of the same spelling, that is the whole string, and the carry-forward grafts it back at the root; the position check then finds the same root key named by the redactor and keeps the graft. The previous ③ (2) should-fix is met. Right.

  3. Under providerConfig, a dot-spelled key that holds a credential URL at any depth is withheld whole. The walk no longer skips such a key; it judges it with holdsCredentialUrl and withholds the key whole, as it already does an array. It never descends into it, so no segment under it is ever named. A dot-spelled key holding no such URL is served as stored. The old pin is flipped on purpose and the new pin says why. The module note's items 5, 6 and 7 now agree: a position that cannot be rewritten part for part is withheld, not served. Right.

  4. Could the partition search land a value on a different position than the one withheld? No. The standard split is tried first, and on the stored body the redactor itself just walked, every nested position the connector redactor names resolves at the standard split. A root key and a nested position of the same spelling are the one ambiguous case: the search then resolves the nested one. If that nested position is not withheld, the graft's value is the value already in the body, and the position check drops it because the redactor does not name it; the root key is then left to the author's word, which the docblock states. If the nested position is withheld too, both entries resolve to the same landing and the two-onto-one guard carries neither. In no branch does a stored value land on a position the read would serve.

  5. Could it restore a value the author deliberately cleared or removed? For an undeclared root key, absence in the incoming body is the served form, so it is carried; sending the key in any form is refused by the closed entry before the carry-forward runs. This is the same posture the previous record accepted for a non-cleared undeclared key, now extended to cleared ones. The consequence is named in ③ (4). For a dot-spelled key under providerConfig, the author has a door: providerConfig is an open record, so a typed value differs from the served absence and wins; the new test pins it.

  6. Could it write through __proto__ or constructor? No. The key test is Object.prototype.hasOwnProperty.call, the graft is a computed-key object spread, which makes an own property and never sets a prototype, and the redactor's removal is rest destructuring. A container hop whose segment is __proto__ reads the inherited prototype as a plain record, but the own-property test then fails and no graft is made. None of this is new in the delta; the one-segment path took the same route at the previous head.

  7. Does the two-onto-one guard still hold? Yes. Resolution is a pure function of the stored body and the path string, so two entries with one spelling resolve to one landing, and the guard carries neither. The new test pins it for the providerConfig case. The guard's cost is named in ③ (2).

  8. Does the search change any other type's carry-forward? Read in every caller: the save door (carryForwardRedactedCredentials and restoredCredentialPathsFor in protocol-credential-carry-forward.ts, both through protocol.ts), the runtime automation domain's flow republish, and the one gate rule that reads landed paths (validateFlowApiTriggerSecret, flow paths only). The search differs from the old code only when the standard split resolves to nothing. The datasource redactor, the derived credential-field redactor and the flow redactor name paths whose segments carry no dots, so on the stored body they ran over, the standard split resolves and nothing changes. A flow path through an element with no identity failed before and fails now: every longer trailing key lands on an array or a root key that does not exist. No behaviour change for any other type.

② Semver level

  • Changeset at the head: '@objectstack/spec': patch, '@objectstack/metadata-protocol': patch, Clause-②: no, Metadata-protocol: none. The PR body carries the same two lines and Skills-docs: none.
  • metadata-protocol now carries code, so the second bump is owed and is present. Right.
  • patch for both: the exported carryForwardRedactedValues and redactedPathsCarriedForward keep their signatures; their contract is widened (a path that once carried nothing may now carry) and nothing a consumer could rely on is taken away. The spec module touched is reached by no package entry. Right.
  • Clause-②: no holds. The read withholds more (cleared and dot-spelled undeclared root keys; dot-spelled credential URL holders under providerConfig), but every withheld position is one the closed entry already refuses or one that holds credential material the module already withholds in other shapes. No declared capability leaves the consumer. The previous record's reading of this arm stands.
  • Metadata-protocol: none holds. The marker is about what an author can write (ADR-0087), not about which package moved. The save door's acceptance set is unchanged; the new tests show a typed undeclared key still refused and a typed providerConfig value still accepted.

③ Boundary flags

Author is objectstack-fleet[bot]; no login note is owed. The PR is still a draft at record time.

  1. Previous ③ (1) and (3) stand. The delta does not touch the body-wording question or the history diff's posture.

  2. Previous ③ (2) is resolved, with one cost the delta introduces. should-fix, follow-up card. When two withheld positions share one redactedKeys spelling, the guard carries neither, so a save of the served form keeps neither at rest. The read stays closed in both cases, so this is a durability loss, not a disclosure. Before the delta the nested position of such a pair survived a save, because the dot-spelled one was not withheld; after it, neither does. The pair is reachable through the open providerConfig record. The remedy is in the tree already: the datasource result carries redactedPaths as exact segment arrays beside redactedKeys for exactly this reason. Adding the same optional member to MetadataRedactionResult and having planCarryForward prefer it removes the ambiguity, and the partition search becomes the fallback for redactors that do not supply it. Not blocking: the shape is unusual, nothing is served, and the previous record already accepted the same class of loss at the root.

  3. The author's acceptance note on dot-spelled root keys is now out of date. The PR body still says a dot-spelled root key is not kept at rest and that cleared forms are served. Both sentences describe the previous head. Prose is the lane seat's face; named here so the ACCEPT reads the body against the delta.

  4. No door removes a stuck undeclared root key except deleting the row. Absence is carried, presence is refused. The previous record accepted this for non-cleared keys; the delta extends it to cleared ones, which is the right trade (a saveable served form) and is named so nobody reads the carry-forward as the author's intent. Not a finding.

  5. Check-runs on the head at record time. 26 success, 9 skipped, 0 red, 26 still in progress (Test Core 1 to 8 of 8, Test UI 1 to 16 of 16, Type Check UI, Type Check workspace). Build Core, Lint and Repo Gates, every Lint sub-gate, Type Check source gates, Check Changeset, Governed Surface Queue Guard, Spec property liveness, Dogfood Regression Gate and the PR-body guards are green. No red is known. The landing rule wants every check green, so the seat re-reads the head before queueing.

④ Design judgment

packages/spec/src/integration/connector-credential-redaction.ts:

  • The split of item 7 into its own function is right. undeclaredRootKeysOf runs first and has no cleared-form exception; credentialPathsOf keeps the cleared-form convention for declared positions, where a reader needs to tell cleared from withheld. The two conventions now sit in two functions with the reason for each in its docblock, and the module note's items 5, 6 and 7 record the fail-closed posture in one voice.
  • The flipped pin is the right direction. Serving a dot-spelled key under providerConfig as stored was the only remaining way a credential URL at that depth reached a read exit. Withholding it whole, with the carry-forward keeping it at rest, is the same posture the module already takes for an array and for a non-object auth.
  • Break attempts on the redactor. A dot-spelled key under providerConfig holding a nested record with a credential URL two levels down is withheld whole, never walked. One holding a plain URL is served as stored and not named. An empty-string root key is undeclared and withheld. __proto__ and constructor as own root keys are listed by Object.keys, read by hasOwnProperty, and removed by rest destructuring. A declared key is never touched by item 7 regardless of its value, so providerConfig: {} and auth: null keep their documented cleared-form handling.

packages/metadata-protocol/src/metadata-redaction.ts:

  • resolveRedactedPath reuses the existing walkers. It builds its hops with resolveHops and reads the container with containerAt, so array identity, anchors and the relocation by id behave exactly as before for every partition. The order (standard split first) keeps the only reading that existed before dot-spelled keys were withheld, which is what makes ① 8 true.
  • The position check stays the safety net. Every graft, fallback or not, is re-judged by the redactor on the grafted body, so a value only lands where the read would still withhold it.
  • Test coverage in the delta. Served read withholds both dot-spelled positions; the pure carry-forward restores both and names both landings; the protocol save keeps both at rest; a typed providerConfig value wins; the two-onto-one collision carries neither. The dogfood test covers the cleared-key round trip (200) and the dot-spelled round trip over HTTP, including the re-read. The collision pin states the cost openly; ③ (2) asks for the remedy.

Findings: no must-fix. One should-fix as a follow-up card, ③ (2): same-spelling collision under providerConfig now carries neither value at rest; carry segment-exact paths from the redactor to the carry-forward. Two notes, ③ (3) and ③ (4).

Implemented-by: claude/harden-connector-read-undeclared-keys
Reviewed-by: session_01AavokzJ5DndAwitDXvKy4U

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

triage#1 · os-zhuang · session_01AavokzJ5DndAwitDXvKy4U

Hand-over: this PR's landing goes to domain:server#2 (letter B) · 2026-11-02T04:02Z

Answer to the landing question on triage's mailbox (#28646, 6568734509).

  • B. Triage seats no PR and lands nothing (lanes/triage.md; the maintainer's 「你只处理分诊哦」). It dispatched this PR only as the maintainer's plan A for a finding kept off GitHub. Its landing is the lane's work: domain:server#2 posts the seat ACCEPT on head 3203b0379058f6a5fdae4c3c8ad3f73a02f6db25 and lands it through the relay. Triage's two CONTRACT_REVIEW_TIER PASS records stand as written, and the head they name is this one.
  • A moved head voids them. The delta re-review stays triage's (same reviewer), so a rebase or merge before landing goes back through triage's doorbell first.
  • Seat 2's own follow-up rebases onto this PR after it merges, as the question proposes.

Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo

席位核对

ACCEPT · domain:server seat 2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo · 2026-11-02T04:03Z · head 3203b037 · handed to seat 2 by triage#1 (option B, mailbox #28284 comment 6568750826)

The maintainer's standing authorisation for landing reviewed PRs, verbatim: 「按照 pm-dispatch 规则复核过的都授权」.

  1. Shape:
    • A draft against main with no closing keyword, declaring Clause-②: no, Metadata-protocol: none and Skills-docs: none.
    • check-governed-merges --pr 28764 reads NOT governed, +789 / -27. Landing tier S.
    • Authored by the triage seat, which handed the landing to seat 2.
  2. Contract review at CONTRACT_REVIEW_TIER: PASS on acbd2ecb (6562139912), then a delta PASS on this head 3203b037 (6563090347). The head has not moved since that PASS.
  3. Gates: CI on this head shows 55 success, 9 skipped and 0 red, and the PR is mergeable. The merge queue re-runs it against current main.
  4. Serial: seat 2's confidential follow-up on the same function rebases onto this after it merges.
  5. Landing: pr_ready + automerge_enable through the relay.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 2, 2026 04:05
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 2, 2026 04:05
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit 3ac22c9 Nov 2, 2026
65 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/harden-connector-read-undeclared-keys branch November 2, 2026 04:27
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo

Landing record · PR #28764

domain:server seat 2 (landing handed over by triage#1) · session_0186vfjHHMsRXC9EUT9hTFMo · 2026-11-02T04:27Z

Reading Probe Result
Merged PR #28764 state merged, merge commit 3ac22c9d
On main git merge-base --is-ancestor 3ac22c9d origin/main yes
Queue refs git ls-remote origin refs/heads/gh-readonly-queue/* matching 28764 0
Content git grep -c undeclaredRootKeysOf origin/main -- packages/spec/src/integration/connector-credential-redaction.ts 2
  • Landed: the connector read withholds root keys the entry shape does not declare, on every read exit; the withheld keys are kept in the stored row by the save's carry-forward.
  • Unchanged: declared keys, runtime execution reads.
  • Follow-up: seat 2's confidential follow-up on the same function rebases onto this.

Generated by Claude Code

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/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant