Skip to content

fix(metadata-protocol): redact stored credentials on the metadata read path, with the write-path carry-forward (#8154) - #8673

Merged
qq9340100 merged 3 commits into
mainfrom
claude/issue-8154-meta-read-path-redaction
Aug 14, 2026
Merged

fix(metadata-protocol): redact stored credentials on the metadata read path, with the write-path carry-forward (#8154)#8673
qq9340100 merged 3 commits into
mainfrom
claude/issue-8154-meta-read-path-redaction

Conversation

@qq9340100

Copy link
Copy Markdown
Collaborator

Fixes #8154

decorateMetadataItem returned the whole stored body, so a datasource row written before #8078 closed the write door came back with config.password in cleartext — and the password embedded in config.url beside it — from GET /api/v1/meta/datasources, from the single-item read, and from the layered read in both its overlay and effective layers. PR #8126 closed the datasource-admin door (GET /api/v1/datasources/:name); this closes the platform door one over.

This carries all five things the PM review settled in comment 5275779695, in one PR: the hook consumption, the datasource redactor as its first consumer, the read-exit application, the getMetaItemLayered exit, and the saveMetaItem write-path carry-forward.

Why the read scrub could not ship alone

Measured on origin/main before this change, and the reason this is one PR rather than two: saveMetaItem accepts a redacted datasource body and persists the credential away. So a read-path redaction with no write-path inverse converts today's loud 422 into silent credential deletion on an ordinary /meta GET → edit → PUT round trip. config.url makes the inverse unavoidable rather than a masking choice — a URL-embedded password is schema-accepted (#8078 pinned that as fact), so dropping it round-trips to deletion and masking it round-trips to storing the mask as the literal password.

The carry-forward is the generic form of what PR #8126 added to updateDatasource, and sits where that one sits: after every gate, immediately before the put. The gates judge what the author wrote; this restores material the author never saw and is not asking to change. Stored material is re-applied only where the incoming body is indistinguishable from what the read served — anything the author actually wrote wins and is still judged by #8078's write gate on its own merits, so a typed-in password is refused exactly as before.

It also restores the #4326 byte-identical round-trip invariant that read-redaction alone would have broken: an untouched GET → PUT of a legacy datasource now persists a body identical to the one at rest.

The generic hook, not a datasource patch

The redactor is resolved through getMetadataTypeRedactor from @objectstack/spec/kernel — the seam #8300 landed. datasource is that registry's first consumer and is never named in this diff's logic; a plugin whose metadata type stores secrets (the SSO seat next) gets the same protection by calling registerMetadataTypeRedactor, with no change here. packages/spec is consumed read-only — nothing in it is touched.

Two fail-open traps closed deliberately:

_diagnostics ordering — load-bearing, and made structural

Diagnostics are still computed on the raw stored body, before redaction. The redacted body is precisely the shape the post-#8078 schema accepts, so computing them afterwards flips valid:false to valid:true on exactly the rows holding a stored credential — deleting the operator's only inventory of what still needs migrating (#8081 item 3).

Rather than leave that as an ordering four call sites must remember, the two steps are composed inside decorateMetadataItem, so no call site can invert an ordering it cannot see. Reverse-verified in the predicted direction: inverting the two lines turns the badge assertions red with expected true to be false, while the redaction assertions stay green.

⚠️ The layered exit redacts code and overlay too — please contest this if it is wrong

getMetaItemLayered never calls decorateMetadataItem; it computes _diagnostics itself and serves three raw layers. All three are redacted here, and that is a reading of another lane's ruling, stated rather than inherited, for domain:engine-core / domain:spec reviewers to check:

#7556 keeps the code / overlay layers deliberately raw so a Studio diff shows what the tenant actually customised. That reason does not extend to credentials, and redacting the key on both sides leaves the diff itself unchanged — the key is absent from both layers, so "what was customised" reads exactly as before.

⛔ It is not a licence to fold, govern or inject on those layers. Redaction subtracts, and only a credential.

Scope boundaries held

The two measurements #8154 owed (A3)

  • SysMetadataRepository.getByHash — an ADR-0009 contract method implemented by four repositories with no production caller anywhere: every reference outside the implementations is the contract suite, unit tests, or a docblock. It serves nothing today.
  • SysMetadataRepository.restoreVersion — a write. Both callers (rollbackMetaItem, revertCommit) return receipts with no body; the restored body reaches only applyRegistryWriteThrough, which must stay raw. No leak, and redacting there would corrupt the restored row.

A third exit does leak and is filed rather than folded in: GET /api/v1/meta/:type/:name/diff returns stored body values from sys_metadata_history, a plane no item-level redactor reaches — #8671. Redacting it is a genuinely different decision (redacting both bodies makes a credential rotation an invisible no-diff), so it needs its own evidence.

Verification — all readings taken at f0221b7c9, the head this PR pushes

Per-package, not an aggregate:

package result
@objectstack/metadata-protocol 89 files / 1317 passed
@objectstack/rest 115 files / 1903 passed
@objectstack/runtime 156 files / 2373 passed
@objectstack/service-datasource 16 files / 376 passed

typecheck green on all four. Gate union at the same head — check:nul-bytes, check:cross-package-test-inputs, check:durability-log-level, check:error-code-casing, check:filter-alias-parity, check:changeset-gate-self-tests, check:objectui-changeset, check:engine-double-contract, check:query-options-erasure, check:type-check-coverage — all pass, plus pnpm --filter @objectstack/spec check:generated (13/13 up to date) after merging main.

The last four gates were derived from the actual changed paths with scripts/pm/dispatch-gates.mjs; the changeset family and the test-file convention family were not on the dispatch list and are named here for that reason.

17 new assertions in protocol.metadata-redaction.test.ts, with both anti-vacuity arms wired:

  1. an ablation block re-registers datasource with an identity redactor through the public registerMetadataTypeRedactor overlay and asserts the cleartext comes back on all three exits — so the green above is a statement about the redactor running, not about a credential-free fixture;
  2. the badge assertions go red when the ordering is inverted, pinned directly by asserting that the redacted body parses clean against the same schema the raw body fails.

Every fixture row is seeded directly into the stub engine rather than through saveMetaItem — that is the population under test, since #8078 means such a row can no longer be authored through any door.


Generated by Claude Code

claude added 3 commits August 14, 2026 11:28
…d path (#8154)

The /meta read exits served the whole stored body, so a legacy datasource
row came back with config.password in cleartext on getMetaItems,
getMetaItem and getMetaItemLayered (both overlay and effective layers).

Consume the per-type redactor registry landed by #8300
(@objectstack/spec/kernel) rather than patching the datasource shape:
datasource is the registry's first consumer, not this code's subject.

_diagnostics stay computed on the RAW stored body BEFORE redaction — the
redacted body is exactly what the post-#8078 schema accepts, so the
inverse ordering flips valid:false to valid:true and destroys the #8081
item-3 migration inventory. Composed inside decorateMetadataItem so no
call site can invert an ordering it cannot see.

Ships with the write-path inverse, which is not optional: saveMetaItem
accepts a redacted body and persists the credential away, so the read
scrub alone converts today's loud 422 into silent credential deletion on
an ordinary GET-edit-PUT round trip.

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

vercel Bot commented Aug 14, 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 14, 2026 12:44pm

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.

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.

[security] GET /api/v1/meta/datasources still serves stored cleartext credentials — the metadata read path has no per-type redaction hook

2 participants