Skip to content

refactor(core,plugin-audit,service-storage,plugin-reports): give the __ operation-private-key convention a single owner (#7284) - #7352

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-7284-private-keys-shared-home
Aug 10, 2026
Merged

refactor(core,plugin-audit,service-storage,plugin-reports): give the __ operation-private-key convention a single owner (#7284)#7352
os-zhuang merged 1 commit into
mainfrom
claude/issue-7284-private-keys-shared-home

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Closes#7284.

withoutOperationPrivateKeys — the rule that a consumer forwarding a caller's execution envelope to a question about a different object must first drop the __-prefixed keys plugin-security stamped for the operation in flight — existed in three unexported, hand-copied copies. This gives it one owner.

The fact, re-verified on this base

Confirmed at f3f855a (triage measured at 62b6a2f), located by content rather than by line number:

copyfilelanded
originalpackages/plugins/plugin-audit/src/comment-access-hooks.ts#7141
copy 2packages/services/service-storage/src/attachment-access-hooks.ts#7145
copy 3packages/plugins/plugin-reports/src/report-service.ts#7204

Sweep for a fourth: none.withoutOperationPrivateKeys / OPERATION_PRIVATE_KEY_PREFIX across all .ts/.tsx/.md outside node_modules returns exactly those three files. A second sweep for hand-rolled near-misses under other names (startsWith('__'), __readScope / __writeScope handling) found only the producer side — plugin-security, plugin-sharing — which stamps and reads these keys rather than stripping them, and is unaffected.

Where the single owner lives, and why

Measured from the three consumers' package.json dependency blocks:

candidatenew dependency edgesverdict
@objectstack/core0 — all three already depend on itchosen
@objectstack/spec0 — all three already depend on itrejected: Prime Directive #2 fences business logic out of spec, and a key-filtering function with a security argument attached is not a schema
plugin-security (the producer)3 — none of the three depends on it todayrejected: the most honest owner, but a string-prefix filter does not justify three new dependency edges onto a plugin, and it would give the producer reverse edges from its own consumers
a new packagen/arejected: ten lines

Inside core the home is src/security/operation-private-keys.ts, deliberately beside assemble-execution-context.ts. Those two own opposite ends of one lifecycle: #6216 made that file the single place an ExecutionContext is built at a transport entry point; this is the single place one is stripped back down before being forwarded. Exported from src/security/index.ts, which src/index.ts already re-exports wholesale.

⛔ Nothing lands in packages/objectql/src/engine.ts (#7095), packages/core/src/qa/runner.ts (#7256), or packages/spec/** / packages/metadata test files (#7223). packages/spec is untouched — no cross-seat declaration needed.

The reasoning moved with the code

Not three thinner copies. The shared home carries the full argument, generalised out of the three divergent prose versions: what a __ key is and which ones exist today (the ADR-0057 D1 depths, the ADR-0090 D10 delegator halves, the engine's __expandRead / __referentialFieldClear privilege markers); why a consumer must drop them (one object's widening applied to another object's question — including the concrete survival path, that plugin-security only overwrites__readScope when it actually resolves permission sets for the new object); why dropping is safe in the one direction that matters; why by prefix and never by a name list, plus the ⛔ corollary for whoever next changes the middleware; and why the fresh copy is load-bearing in both directions.

Each consumer keeps only its genuinely local half — which object its gates ask about (sys_comment, sys_attachment, report.object_name) — and points at the shared home. The [#7141] / [#7145] / [#7204] provenance markers and the surrounding #6206/#6523 envelope-forwarding arguments at the call sites are untouched.

Pins

  • operation-private-keys.test.ts — the rule's own behaviour, which no package-level test had ever asserted directly (each package tested only its own gates' effect): prefix-dropping, that it strips rather than projects (every principal field survives — the defect half of the five-field projections these call sites replaced), the fresh copy even on a clean envelope, the deliberate shallow-copy boundary, and that a key the middleware has not stamped yet is dropped by prefix alone.

  • operation-private-keys.pin.test.ts — the anti-fourth-copy pin. Scans every .ts/.tsx under packages/ for a declaration (not a mention) of either symbol outside the home, and fails with a message naming the import to use. Two anti-vacuity guards travel with it (check:react-declaration-parity 是唯一没接进任何 workflow 的源码审计门禁,且无 MANIFEST 时静默 skip 退出 0 —— 它现在永远不可能红 #4690's "a check that can't see its input has verified nothing"): the home must really declare both symbols, and the walker must demonstrably reach all three former-copy files.

    Scope is deliberately narrow — it detects redeclaration, not "a consumer that should have used the helper and didn't". The latter is the more interesting question and is not decidable by scanning; a regex ambitious enough would be a false-red generator, and an inert or noisy gate reads as a gate that is watching. Redeclaration is exactly how all three copies got here.

    Reverse-verified: planting a fourth copy in plugin-audit turns it red naming that file; removing it turns it green.

Changeset

Real patch bumps for the four packages, not an empty changeset and notskip-changeset, read off check-empty-changeset.mjs's own ledger. That gate's text is one-directional: an empty changeset produces no CHANGELOG entry (measured, #5471), buys nothing the label doesn't, and uniquely is a real input to changesets/action that can trigger #4898 — the failure mode that silently stalled 17.0.0-rc.2. This PR changes published source in four packages, so a real bump is both honest and the route that produces an actual CHANGELOG entry.

Nothing breaking, so no ADR-0087 conversion.

Gates

Enumerated fresh off origin/main — 64 check:* in lint.yml, 70 across all workflows.

The export-surface family (check:export-origins, check:api-surface, check:exported-any, check:dual-source-exports) is --filter @objectstack/spec in every workflow invocation — it reads spec's surface, not core's. Adding an export to @objectstack/core therefore required nothing of them. Run results in a follow-up comment.

Verification

  • @objectstack/core — 738 tests green, including the 11 new ones
  • plugin-audit 160 · service-storage 352 · plugin-reports 68 — all green, unchanged

Generated by Claude Code

…`__` operation-private-key convention a single owner (#7284)
`withoutOperationPrivateKeys` and its `OPERATION_PRIVATE_KEY_PREFIX` had been
hand-copied into three packages — plugin-audit's comment access hooks (#7141),
service-storage's attachment access hooks (#7145) and plugin-reports' report
service (#7204). All three were byte-equivalent in behaviour; their doc blocks
had already diverged in prose.
The rule now lives once in `@objectstack/core`
(`security/operation-private-keys.ts`), beside `assemble-execution-context.ts`
— that file owns where an ExecutionContext is BUILT at a transport entry point,
this one owns where it is stripped before being forwarded to a question it was
not resolved for.
Home chosen by dependency measurement: all three consumers already depend on
`@objectstack/core`, none depends on `plugin-security` (the producer, and the
most honest owner, but a string-prefix filter does not justify three new
dependency edges onto a plugin), and `@objectstack/spec` is fenced by Prime
Directive #2. Core is the only zero-new-edge candidate.
The reasoning moved with the code rather than being thinned; each consumer
keeps only its own local half and points at the shared home.
Pins: `operation-private-keys.test.ts` asserts the rule's own behaviour, which
no package-level test had asserted directly, and `operation-private-keys.pin.test.ts`
turns red if a fourth file declares its own copy.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017AD2nx7MRuje3kLqoLBHPM
@vercel

vercelBot commented Aug 10, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 10, 2026 7:50am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/core, @objectstack/plugin-audit, @objectstack/plugin-reports, @objectstack/service-storage.

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

  • content/docs/ai/actions-as-tools.mdx(via @objectstack/core)
  • content/docs/ai/knowledge-rag.mdx(via @objectstack/core)
  • content/docs/ai/natural-language-queries.mdx(via @objectstack/core)
  • content/docs/api/client-sdk.mdx(via @objectstack/plugin-reports)
  • content/docs/api/plugin-endpoints.mdx(via @objectstack/service-storage)
  • content/docs/automation/webhooks.mdx(via @objectstack/core)
  • content/docs/concepts/north-star.mdx(via packages/core)
  • content/docs/deployment/cli.mdx(via @objectstack/plugin-audit)
  • content/docs/deployment/migration-from-objectql.mdx(via @objectstack/core)
  • content/docs/deployment/production-readiness.mdx(via @objectstack/plugin-audit)
  • content/docs/kernel/contracts/index.mdx(via @objectstack/core)
  • content/docs/kernel/runtime-services/examples.mdx(via @objectstack/core)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/core, @objectstack/service-storage)
  • content/docs/kernel/services.mdx(via @objectstack/core)
  • content/docs/permissions/authentication.mdx(via @objectstack/core)
  • content/docs/permissions/authorization.mdx(via packages/core)
  • content/docs/plugins/anatomy.mdx(via @objectstack/core)
  • content/docs/plugins/development.mdx(via @objectstack/core)
  • content/docs/plugins/index.mdx(via @objectstack/core)
  • content/docs/plugins/packages.mdx(via @objectstack/core, @objectstack/plugin-audit, @objectstack/plugin-reports, @objectstack/service-storage)
  • content/docs/protocol/kernel/index.mdx(via @objectstack/core)
  • content/docs/protocol/kernel/lifecycle.mdx(via @objectstack/core)
  • content/docs/protocol/kernel/plugin-spec.mdx(via @objectstack/core)

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

  • content/docs/releases/implementation-status.mdx(via @objectstack/core, @objectstack/plugin-audit, @objectstack/service-storage)
  • content/docs/releases/v12.mdx(via @objectstack/core)
  • content/docs/releases/v15.mdx(via @objectstack/core)
  • content/docs/releases/v17.mdx(via @objectstack/core)

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.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 10, 2026
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Gate runs (local, on the pushed head)

Gates enumerated fresh off origin/main — 64 check:* in lint.yml, 70 across all workflows. CI is the authority; these are the local runs, all exit 0.

Changeset family — the decision in the PR body, checked against the gates that judge it:

gateresult
check:empty-changeset (+ --self-test, 118 assertions)✓ "No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added)"
check-changeset-no-major✓ "introduces no major bump"
check-adr-0087-registration✓ "adds no declared-breaking changeset (1 non-breaking changeset(s) seen)"

Export-surface family — the §4 question, answered: they required nothing of this PR. Every workflow invocation of these four is pnpm --filter @objectstack/spec, so they read @objectstack/spec's surface, not @objectstack/core's. Adding an export to core does not enter their corpus. Run anyway to confirm no collateral movement:

gateresult
check:export-origins✓ 4979 exports across 16 entry points resolve exactly as recorded
check:api-surface✓ spec public API surface + factory signatures unchanged
check:exported-any✓ 2403 types + 1506 schemas, none resolves to any
check:dual-source-exports✓ 4809 names, 0 new dual-source
check:spec-changesspec-changes.json up to date
check:generated✓ all 11 generated artifacts up to date
check:strictness-ledger✓ ledger current

Security- and packaging-adjacent gates that could plausibly read a new file under packages/core/src/security/:

gateresult
check:authz-resolver✓ single shared authorization resolver intact; both entry points delegate
check:tenant-chokepoint✓ 19 getBuilder() bindings, every read builder routes through applyTenantScope()
check:published-files✓ 69 publishable packages declare a files whitelist admitting no test file — the two new *.test.ts at the home are correctly excluded from the published tarball
check:type-check-coverage✓ 63/77 packages type-checked; this PR adds no debt row
check:nul-bytes✓ 6741 files scanned

Build / lint / tests

  • eslint --no-inline-config over all 7 changed source files — clean
  • @objectstack/core tsup build incl. DTS — success. plugin-audit / plugin-reportstsc --noEmit — clean. core and service-storage declare no typecheck script; their type safety is the DTS build, green for both.
  • @objectstack/core738 tests green (11 new) · plugin-audit160 · service-storage352 · plugin-reports68 — all green and unchanged in count apart from core's additions.

Anti-copy pin, reverse-verified. Planting a fourth copy in packages/plugins/plugin-audit/src/ turns operation-private-keys.pin.test.ts red naming that exact file and printing the import to use instead; removing it turns it green. The pin is watching, not inert.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 10, 2026 08:22
@os-zhuang
os-zhuang added this pull request to the merge queueAug 10, 2026
Merged via the queue into main with commit d0d5205Aug 10, 2026
26 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-7284-private-keys-shared-home branch August 10, 2026 08:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

withoutOperationPrivateKeys is now hand-copied into three packages — the __ convention has no single owner

2 participants

@os-zhuang@claude