Skip to content

feat(cli): build the cross-producer sys_secret reference union (#12663) - #12754

Merged
os-litant merged 7 commits into
mainfrom
claude/issue-12663-secret-reference-union
Aug 27, 2026
Merged

feat(cli): build the cross-producer sys_secret reference union (#12663)#12754
os-litant merged 7 commits into
mainfrom
claude/issue-12663-secret-reference-union

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes#12663

Builds the cross-producer sys_secret reference union — the precondition the deletion half of #8103 is blocked on. Read-only across all three producer surfaces; no producer needed a change, and this PR contains no deletion command, no dry-run, no sweep and no classifier.

  • packages/cli/src/utils/secret-reference-union.ts — the union
  • packages/cli/src/utils/secret-reference-union.test.ts — 20 pins
  • .changeset/sys-secret-cross-producer-reference-union.md — patch, @objectstack/cli

The premise, reproduced against real code

Ruling 5 asked for the #8103 measurement to be driven rather than repeated. It reproduces. the premise: the shipped settings-scoped classifier calls a LIVE credential 'orphaned' boots a real ObjectQL engine, the real LocalCryptoProvider, the real datasource secret binder and the real shipped classifySysSecretRows, and shows a live, engine-owned credential classified orphaned:

  • an object named smtp with a secret-typed field password mints its sys_secret row with namespace = smtp, key = password (that is encryptSecretFields, verbatim);
  • a settings manifest declaring namespace smtp with an encrypted specifier password produces the identical attribution pair;
  • the business row still holds the ref, so the credential is in force — and the classifier answers orphaned.

Attribution by (namespace, key) is a name match, not ownership, exactly as the card says. In the same fixture the datasource handle answers unattributable only because datasource/main happens not to collide — the escape is luck, not a guard. The union names all three handles, which is what makes the deletion predicate decidable.

Completeness is structural, not asserted

An incomplete union is strictly worse than no union, so two devices carry it rather than a claim in a comment:

  1. The family set is closed and the assembler demands every member.buildSecretReferenceUnion takes a Record keyed on the family union, so omitting a producer is a type error rather than a smaller union.
  2. A read that did not happen is a declared gap, never an empty answer. A missing driver, a throwing read, an unparseable artefact, or a host that did not declare its code-defined datasources each produce a gap; the union then reports complete: false and assertSecretReferenceUnionComplete refuses it with the ADR-0112 pair PRECONDITION_REQUIRED / 428. undefined and [] are deliberately different answers for the host's code-defined datasources: "nobody answered" versus "there are none".

Two design decisions worth reviewing explicitly:

  • Reads go to the driver, through the engine's public getDriverForObject(). The secret: ref only exists at that layer — the read path masks it unconditionally, which is why the engine's own privileged verbs read there too. A positive control pins this: the driver-level read is load-bearing shows the generic engine read returning SECRET_MASK while the union still finds the handle.
  • The reads are unscoped and carry no bound. Tenant scoping, sharing, FLS and a page size all subtract rows, and every row subtracted here is a live handle sys_secret: rotations that happened BEFORE #8030's fix left decryptable orphans on deployed instances, and nothing reaps them #8103 would read as unreferenced. Under-reporting is the direction that deletes live credentials. Every driver in this tree bounds a result only when limit is present, so an unbounded read returns the whole holder set.

The acceptance condition: each family separately ablated, each reds a named pin

Ruling 4's criterion is falsifiable, so it was falsified rather than asserted. Each leg injects a silent early return into exactly one collector — the dangerous shape, since the union still reports complete: true — proves the mutation on disk by marker count and blob hash, runs the suite, then restores and proves the restore by blob hash plus an empty git diff HEAD. No build is involved: packages/cli/dist does not exist in this worktree, so the test's relative import resolves to the TypeScript source.

ablated familyresultthe named pin that reds
settings4 failed / 15 passedfamily 1 — settings ... names the handle held ONLY by sys_setting.value_enc
object-field4 failed / 15 passedfamily 2 ... names the handle held ONLY by a business row
datasource5 failed / 14 passedfamily 3 ... names the handle held ONLY by a datasource artefact

Each family pin asserts a handle only that family holds, and also that exactly one reference names it — so the coverage cannot be borrowed from a sibling family.

Family 2 carries the extra pin the card asks for, because it is the one family that cannot be precomputed: registers a new secret field at runtime registers a tenant-shaped object after the first union is built, writes a row through the same producer path, and shows the new handle arriving with no code change — with the before-state asserted absent so the pin cannot pass vacuously.

Placement — measured today, not cited

The card's numbers are a 2026-08-26 reading and it says so. Re-measured on this checkout:

  • 80 tracked package.json files, not 73. The count moved; the answer did not.
  • Exactly 2 packages declare all three producer families as direct runtime dependencies: @objectstack/cli and @objectstack/verify. Unchanged from the card.
  • Under transitive closure the set is 3: @objectstack/dogfood joins, but only by depending on those two, so it is downstream of both rather than an independent candidate.

Chose @objectstack/cli. Both candidates are in this lane, so no cross-lane coordination either way. @objectstack/verify is a published boot-and-verify harness whose charter is CRUD round-trip fidelity and the RLS invariant; an operator-safety primitive does not belong inside it. The named reader — #8103's deletion command — is in packages/cli, and src/utils/ is where this package already keeps read-only diagnostics over a booted runtime. Landing here also needed no package.json edit, which the fence forbids.

Not exported from the package entry barrel. The named reader is in this same package and imports it relatively; publishing an external API with no external consumer is the surface expansion the startup-scope rule declines. That, and not the file count, is why the changeset is patch and not minor — an external export is a separate decision with its own changeset.

Producer read-only

Nothing on the three producer surfaces changed. The union consumes each producer through its own published predicate — isSecretHandle (service-settings), collectSecretFields and parseSecretRef (objectql), parseCredentialsRef (service-datasource) — so none of the three ref spellings is restated here. A restated prefix would be a second de-facto contract, and the failure it produces is a handle missing from the union.

One measured limit, stated rather than papered over: registerDatasourceDef keeps only schemaMode and external.allowWrites, so the engine cannot answer family 3 at all. Persisted artefacts are read from sys_metadata; code-defined ones must be supplied by the host, and not supplying them is a declared gap rather than a silent zero. Widening the engine to keep credentialsRef would be a producer change, so it is not made here.

Verification

Union of gates run locally at 929d12c69 (the tree this PR pushes), after pnpm install --frozen-lockfile and a rebuild of the @objectstack/cli closure:

  • pnpm --filter @objectstack/cli exec vitest run src/utils/secret-reference-union.test.ts — 20 passed
  • pnpm --filter @objectstack/cli typecheck — clean. Both new files are in the tsc program (checked with --listFiles: 1 hit each), so the green is about them.
  • green: check:nul-bytes, check:objectql-double-limit, check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:slot-lookup, check:cross-package-test-inputs (both spellings), check:test-source-alias, check:type-source-resolution, check:published-files, check:page-declaration-shape, check:type-check-coverage, check:changeset-gate-self-tests, check:objectui-changeset, check:durability-log-level, check:startup-registry-verdict, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-ci-filter-parity, check-comment-mask-adoption, check-plugin-teardown-shape, both docs-audit checks, release-rehearsal-clone --self-test
  • pnpm lint — the full repo-wide eslint . --no-inline-config, exit 0. Not a narrowed run, so no narrowing claim is being made.
  • check:i18n, check:i18n-coverage, check:type-check-debt needed built prerequisites; each was rebuilt and re-run to a real green (check:i18n: 9 packages in sync; check:i18n-coverage: 12 configs, 602 baselined, none new; check:type-check-debt: exit 0 with the closure built)
  • scripts/pm/check-half-states.mjs exits 3 = PREREQUISITE NOT MET (this container has no valid GitHub credential). Reported as NOT MEASURED, not as red.

check:objectql-double-limit was red on the first pass and is the reason the test's find double now holds the caller's bound by presence, after the filter and before the row copy.


Generated by Claude Code

@github-actionsgithub-actionsBot added size/xl documentation Improvements or additions to documentation tests tooling labels Aug 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 30 documentable anchor(s).

19 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 168941cea851ca77a42ce5db01b77f1c91273bee.

3 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 11 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 23 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 168941cea851ca77a42ce5db01b77f1c91273beepackageMentionDocs.

Which tree this was computed on

This run read content/docs from 72cc0ed6360062e0def30a2eab358a85037800dc — the merge of head 929d12c698b030d53869aaacdd4ffc89d2f4de1d into base 168941cea851ca77a42ce5db01b77f1c91273bee, 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 72cc0ed6360062e0def30a2eab358a85037800dc && git checkout 72cc0ed6360062e0def30a2eab358a85037800dc
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 168941cea851ca77a42ce5db01b77f1c91273bee 929d12c698b030d53869aaacdd4ffc89d2f4de1d && git checkout -B drift-repro 168941cea851ca77a42ce5db01b77f1c91273bee && git merge --no-ff 929d12c698b030d53869aaacdd4ffc89d2f4de1d
node scripts/docs-audit/affected-docs.mjs --json 168941cea851ca77a42ce5db01b77f1c91273bee

⚠️ 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 168941cea851ca77a42ce5db01b77f1c91273bee → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-litant
os-litant marked this pull request as ready for review August 27, 2026 17:01
@os-litant
os-litant added this pull request to the merge queueAug 27, 2026
Merged via the queue into main with commit 5091da3Aug 27, 2026
34 checks passed
@os-litant
os-litant deleted the claude/issue-12663-secret-reference-union branch August 27, 2026 17:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/xlteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sys_secret: build the cross-producer reference union — the primitive any safe orphan sweep needs (unblocks #8103's deletion half)

2 participants

@os-litant@claude