Skip to content

fix(plugin-security): a cloned permission set is not reported as an unowned declaration on boot (#21669) - #21692

Merged
objectstack-fleet[bot] merged 2 commits into
mainfrom
claude/issue-21669-cloned-set-not-unowned
Oct 4, 2026
Merged

objectstack-fleet[bot] merged 2 commits into
mainfrom
claude/issue-21669-cloned-set-not-unowned

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #21669
Clause-②: no

What was wrong

bootstrapDeclaredPermissions (plugin-security) walks every permission item in the engine SchemaRegistry. That registry also holds the permission sets an environment authored itself: loadMetaFromDb hydrates every env-wide sys_metadata permission row into the same collection. A set made with Setup's Clone action is one of them, and it has no package id there, because it has none. The loop judged "no owning package" before it looked at the set's row, so every boot, and every metadata:reloaded, logged [permission_set_declaration_unowned] declared permission set "…" has no owning package — not materialized … the Setup admin surface reads sys_permission_set and cannot see this set for a set whose managed_by: admin row Setup lists and edits.

Reproduced first, on the real showcase composition

Boot 1 of bootStack(showcaseStack, { databaseFile }): the admin signs in and clones showcase_contributor through POST /api/v1/data/sys_permission_set, with the shipped clone_permission_set action's payload. Then a cold boot on the same file. Measured on a4fd82a8e6, with the plugin-security dist built from that tree:

reading value
clone POST 201
clone row managed_by: admin, package_id: null, organization_id: null
clone sys_metadata row one row, state: active, organization_id: null, package_id: null
boot 2, registry item for the clone present; no _packageId, no packageId; _provenance: 'org' (stamped at hydration)
boot 2, the other 17 registry items every one carries _packageId (com.example.showcase or com.objectstack.plugin-security) and _provenance: 'package'
boot 2, unowned lines 1, naming the clone
boot 2, seeding summary skippedUnowned: 1, skippedEnvAuthored: 8, total: 18 (boot 1: skippedUnowned: 0, total: 17)

The line that reported it is upsertPackagePermissionSet's if (!packageId) branch (reportPermissionSetDeclarationUnowned), reached from the boot loop with packageId = ps._packageId ?? ps.packageId, which is undefined for the clone. The report became visible with cf39b83c09; git log -S on the call finds that commit alone.

The same steps with this branch's dist: 0 unowned lines, and the seeding summary reads skippedUnowned: 0, skippedEnvAuthored: 9.

The fix

Before the loop judges ownership, it checks whether the environment already owns a row under the name. A registry item with no package id, whose sys_permission_set row is environment-owned, is the environment's own set. It lands in skippedEnvAuthored, where a package declaration over an environment row already lands, and nothing is logged.

Which signal, and why. The signal is the row's managed_by, read through the existing env-authored verdict. That is the one definition of environment ownership this seeder already had: the last branch of upsertPackagePermissionSet, where a row that is not managed_by: 'package' is environment-authored and never clobbered (ADR-0086 two-doors). It is now named isEnvironmentOwnedRow, and both that branch and the new check call it, so there is still exactly one spelling. Two alternatives were measured and not taken:

The check reads from the batched existence read the loop already makes, so no query is added. On a per-organization pass, it also counts the organization-less row the environment door writes. permission-set-projection.ts is deliberately not per-organization, so a clone's row has no organization_id there. If the read cannot answer, that is not proof the environment owns the name, so the unowned refusal stands, exactly as before. The publish-time materializer (ADR-0086 P2) is unchanged.

The warning stays honest. A declaration with no owning package and no environment row still logs permission_set_declaration_unowned, with the same text, and still counts as skippedUnowned. seed-refusal-diagnostics.ts is untouched.

#14491 (the third registry copy) was read first

It shares the registry walk, and the fix does not depend on which copy the loop reads.

Pins

  • bootstrap-declared-permissions.test.ts, under [#21669], 7 cases:
    • a cloned set (the measured item shape over a managed_by: admin row) logs nothing on any of the five console channels, counts skippedEnvAuthored: 1, and its row is untouched;
    • the same on a per-organization pass;
    • a truly unowned declaration (no package id, no row) still warns, with the token and the first sentence declared permission set "crm_orphan" has no owning package — not materialized.; the same on a per-organization pass;
    • controls: an unowned item over a managed_by: package row still warns; an unreadable read still warns;
    • conservation with a clone in the walk.
  • A boot-level pin, packages/qa/dogfood/test/permission-set-clone-boot-unowned-warning.dogfood.test.ts. The unit seam cannot show that the real boot puts the clone in the walk, so this pin boots the showcase twice:
    • It clones through the shipped action's own declaration: its target, its bodyExtra, its typed label/name, and every defaultFromRow facet.
    • It asserts the precondition that boot 2's walk holds the clone with no package id.
    • Control: the capture saw the seeding summary line.
    • It asserts no permission_set_declaration_unowned line names the clone.

Ablations (one-time, nothing left in the tree)

Each leg ran through scripts/ablation-replace.mjs with an anchor that must hit, on top of the committed fix (067aa4f58f). Each leg rebuilt @objectstack/plugin-security, and scripts/ablation-dist-preflight.mjs confirmed the marker had reached dist/ before any result was read. The direction was predicted before each run.

leg mutation predicted observed
A: guard removed environmentOwnsName returns false only the clone rows go red unit: 3 red, 27 green: the clone (single), the clone (per-organization), and conservation-with-clone. Boot pin: 1 red, 2 green; the absence case failed with one unowned line.
B: guard admits all environmentOwnsName returns true every input with no package id and no environment row goes red, i.e. the new truly-unowned rows, the package-row and unreadable controls, and the pre-existing #18091 / #18571 unowned pins of the same shape; the clone rows stay green unit: 12 red (5 new, 7 pre-existing, all of that shape), 18 green, both clone rows among them. Boot pin: 3 of 3 green.

Restore, both legs: ablation-replace reported that the file's blob equals HEAD (56a5addec78e) and that git diff HEAD is empty. A rebuild then followed, and ablation-dist-preflight --absent reported both markers absent from all 6 built files and the tree clean.

Verification, on HEAD 067aa4f58f

  • pnpm --filter @objectstack/plugin-security test: Test Files 164 passed (164), Tests 3534 passed | 45 skipped (3579).
  • pnpm --filter @objectstack/plugin-security typecheck: exit 0. check:test-typecheck OK, and tsc -p tsconfig.test.json --listFiles includes the edited test file.
  • pnpm --filter @objectstack/dogfood typecheck: exit 0, and the new pin is in the program.
  • The boot pin: Tests 3 passed (3).
  • Gates: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 69 commands; all 69 exited 0. Reconciled with --ran: 69 run, 0 NOT-MEASURED (a DERIVED zero — all 69 recorded an exit code and none of them is 3).
    • check:dual-build-cjs-loads first exited 3 (PREREQUISITE NOT MET: 8 unrelated packages had no dist/). After those were built it exited 0.
    • Two families take values only CI supplies and were not run locally: check-shard-attestation (dogfood shard matrix) and check-issue-citations --census.
  • Lint: narrowed to the 3 touched TS files, eslint --no-inline-config --format json: 3 file results, 0 errors, 0 warnings. eslint.config.mjs uses no parserOptions.project (no type-aware rules), so this diff cannot move the verdict on any untouched file. The repo-wide lint is CI's.
  • Docs: content/docs/** (outside releases/) and skills/** have no mention of permission_set_declaration_unowned or "no owning package", so no sentence was made false.

Acceptance notes


Generated by Claude Code

claude added 2 commits October 4, 2026 05:46
…ration (WIP)

The boot loop tells an environment-owned set apart from a package
declaration before it judges ownership.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…ning, and a truly unowned one still warns

Unit pins at the seeder seam (single and per-organization passes, the
package-owned-row and unreadable controls, conservation) and a boot-level
pin over the real showcase composition: clone through the shipped action's
own payload on boot 1, assert the walk holds the clone and no unowned line
names it on boot 2. Plus the patch changeset.

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

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security, touching 4 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/permissions/authorization.mdx (via bootstrapDeclaredPermissions (symbol, a top-level function), upsertPackagePermissionSet (symbol, a top-level function))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v12.mdx (via bootstrapDeclaredPermissions (symbol, a top-level function))

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.

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

Which tree this was computed on

This run read content/docs from 46f9a46a943cc2738784dd26479f45d0eff4134d — the merge of head 067aa4f58f1434dcd0d1675c5aee76c57b1636f1 into base 251a7dd4b491d1f216e8a470d0efd0dc7e8ac5e8, 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 46f9a46a943cc2738784dd26479f45d0eff4134d && git checkout 46f9a46a943cc2738784dd26479f45d0eff4134d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 251a7dd4b491d1f216e8a470d0efd0dc7e8ac5e8 067aa4f58f1434dcd0d1675c5aee76c57b1636f1 && git checkout -B drift-repro 251a7dd4b491d1f216e8a470d0efd0dc7e8ac5e8 && git merge --no-ff 067aa4f58f1434dcd0d1675c5aee76c57b1636f1

node scripts/docs-audit/affected-docs.mjs --json 251a7dd4b491d1f216e8a470d0efd0dc7e8ac5e8

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

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

2 participants