Skip to content

fix(metadata-protocol): stop the meta diff endpoint serving credential values (#8671) - #8797

Merged
hotlong merged 4 commits into
mainfrom
claude/issue-8671-diff-credential-redaction
Aug 15, 2026
Merged

fix(metadata-protocol): stop the meta diff endpoint serving credential values (#8671)#8797
hotlong merged 4 commits into
mainfrom
claude/issue-8671-diff-credential-redaction

Conversation

@hotlong

@hotlong hotlong commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Fixes #8671

GET /api/v1/meta/:type/:name/diff is a routed, live GA endpoint that answers res.json(result) unmodified. diffMetaItem read two stored bodies verbatim — history rows out of sys_metadata_history, and the active row via repo.get() — and emitted the raw values it found. A pre-#8078 datasource row whose credential rotated between versions therefore returned both the old and the new password in cleartext, in the inline config.password spelling and in the password component of config.url alike.

The ruled shape

Implements the maintainer ruling in issue comment 5299845282 (delegated adjudication) — Option B:

Ruled: Option B — diff raw, then redact emitted values at redactor-named paths, keeping the path and the "changed" signal.

The diff is still computed on the raw bodies, so a credential rotation continues to report its path as changed; only the values leaving the function are taken from the type's redacted projection of those same bodies. Both sides, all three emission buckets (added.value, removed.value, changed.from / changed.to).

Option A (redact before diffing) is ruled out and the pins are written to catch a drift back to it: two redacted bodies are equal at a redacted path, so the rotation would vanish into a no-diff.

The one design point worth review

The obvious reading of "redact at redactor-named paths" is to match the diff entry's path against the redactor's redactedKeys. That matches nothing on the real leak, and it is worth stating plainly because it looks correct:

  • diffShallow is top-level — a nested change collapses to one entry whose path is config and whose value is the whole sub-object;
  • redactedKeys is nested and dottedconfig.password.

So redactedKeys.includes(entry.path) compares two different planes and never fires. This PR instead reads each emitted value out of the already-redacted body at the same top-level path. That gets the nested case right for free, and mints no second redaction rule set — the registered per-type redactor stays the single source of what a credential is, which was the ruling's stated rationale.

Types with no registered redactor are skipped through hasMetadataRedactor and keep serving their values by reference, exactly as before.

Verification

New pin: packages/metadata-protocol/src/protocol.diff-credential-redaction.test.ts (7 cases), fixture seeded directly into a table-aware stub engine — #8078 closed the write door on inline credentials, so only a direct seed reproduces the rows that leak.

  • The ruling's named pin — a credential value changes across versions: the path is reported as changed and neither value is served, asserted as two separate facts so a drift to Option A fails on the first.
  • The discriminating control — an ordinary label field still diffs with both values visible, proving the redaction is path-selective rather than a blanket value scrub (a diff that emitted no values at all would pass a sweep-only test).
  • Non-credential keys (host, username) survive inside the redacted value.
  • added and removed buckets pinned separately.
  • Anti-vacuity: an ablation arm re-registers datasource with an identity redactor through the public registry overlay and asserts the cleartext comes back — so the green above is a statement about the redaction running, not about a credential-free fixture.

Reverse verification, run from the committed state with the direction predicted first: reverting the emission substitution turns exactly 3 of 7 red (AssertionError: expected 'hunter2-old' to be undefined) and leaves 4 green — the control, the nested-keys case, the ablation arm and the no-redactor case, none of which depends on the fix.

Second commit — the stub's WHERE matcher refuses combinators

CI's ESLint job flagged the new test double under check:where-matcher: its matches walked the filter treating every key as a field name, so a $or / $and key would be compared as a column (r.$or is undefined, the row is silently excluded, and a suite can go green asserting on a query nobody wrote — the gate's shape (b)).

Fixed by refusing rather than implementing, the path 142 of 236 matchers in this repo take and the one the gate's own header recommends for a double that only ever sees scalar equality. This double serves two readers that both issue flat { organization_id, type, name } filters, so no combinator can legitimately arrive; the throw makes it loud if one ever does. The baseline was not touched and no inline disable was added. The seven pins are unaffected by the change.

Gate union, re-run after the final commit, at HEAD 971b25753

  • pnpm --filter @objectstack/metadata-protocol test — 92 files / 1376 tests passed
  • check:where-matcher — 236 discovered, 236 conform (142 refuse), 0 silently-wrong, 0 unjudged, baseline not grown
  • check:engine-double-contract — pass
  • check:nul-bytes, check:changeset-gate-self-tests, check:objectui-changeset, check:filter-alias-parity, check:durability-log-level, check:cross-package-test-inputs, check:query-options-erasure, check:type-check-coverage — all pass
  • check-adr-0087-registration, check-changeset-no-major, check-empty-changeset — pass
  • check:type-check-debt (the ratchet, on a fully rebuilt 70-package closure) — none above its recorded number

Path-derived gate names came from node scripts/pm/dispatch-gates.mjs. check:where-matcher and check:engine-double-contract are convention-triggered — they fire because the diff adds a test double, not because of which paths it touches — so no path derivation can name them; both are included above.

Scope

packages/metadata-protocol/src/protocol.ts is under region-level declaration this round; both hunks land inside diffMetaItem and nothing else in the file is touched. origin/main merged at caaae2cca.

Two things recorded on the card were re-confirmed as not in scope and deliberately untouched: SysMetadataRepository.getByHash (contract method, no production caller — redaction belongs at a future consumer's exit) and restoreVersion (a write returning receipts only, no leak; redacting there would corrupt the restored row).


Generated by Claude Code

claude added 2 commits August 15, 2026 02:17
…l values (#8671)

`diffMetaItem` read two stored bodies verbatim and emitted the raw values, so
a `datasource` row whose credential rotated between versions returned both the
old and the new password in cleartext through the live GA endpoint
`GET /api/v1/meta/:type/:name/diff`.

Implements the maintainer ruling (issue comment 5299845282, Option B): diff the
RAW bodies so a rotation still reports its path as changed, then take the
emitted value/from/to from the type's redacted projection of those same bodies
— both sides, all three buckets.

Substitution rather than path-matching against `redactedKeys` is deliberate:
`diffShallow` is top-level while `redactedKeys` is nested and dotted, so
matching `redactedKeys` against an entry path would match nothing on the real
leak. Reading each emitted value out of the already-redacted body keeps the
redactor as the single source of what a credential is.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8
@vercel

vercel Bot commented Aug 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 15, 2026 3:45am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol.

3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/metadata-protocol)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/metadata-protocol)
  • content/docs/protocol/kernel/http-protocol.mdx (via @objectstack/metadata-protocol)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/v9.mdx (via @objectstack/metadata-protocol)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

claude added 2 commits August 15, 2026 03:17
…mbinators (#8671)

`check:where-matcher` caught the new test double's `matches` as silently wrong:
it walked the filter treating every key as a field name, so a `$or` / `$and`
key was compared as if it were a column — `r.$or` is undefined, the row is
silently excluded, and a suite can go green while asserting on a query nobody
wrote.

Fixed by REFUSING rather than implementing, the path 142 of 236 matchers in
this repo take and the one the gate's header recommends for a double that only
ever sees scalar equality. This double serves two readers that both issue flat
`{ organization_id, type, name }` filters, so no combinator can legitimately
arrive; if one ever does, the throw makes it loud instead of silent.

Gate now reports 236/236 conforming, 0 silently-wrong, 0 unjudged, baseline
not grown. The seven pins are unaffected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8
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

Development

Successfully merging this pull request may close these issues.

[security] GET /api/v1/meta/:type/:name/diff serves stored credential VALUES — a plane the per-type item redactor cannot reach

2 participants