fix(security): make clone_permission_set carry all five copied facets

[os-sam]

Fixes#11703

What changed

clone_permission_set POSTs its paramsvalues to /api/v1/data/sys_permission_set, so the params list is the payload. It named two of the six definition facets a sys_permission_set row carries — object_permissions and field_permissions — leaving system_permissions, row_level_security and tab_permissions absent from the body. permissionSetBodyFromRow() then read each one through parseMaybeJson(undefined, …) and filled the empty default, so cloning a set that granted setup.access, or one carrying row-level security policies, produced a clone with none of them: record created, success toast fired, and the missing half discoverable only by diffing the two records.

The three now travel, in the same JSON-string shape the two listed columns already used.

Why this was urgent rather than tidy

PR #11702 landed one commit before this branch point. The save door now refuses an in-place edit of a package-declared permission set and its refusal message tells the admin to clone — so this action became the platform's own recommended remedy while it was still dropping three facets. An admin following that instruction lost grants quietly. The failure direction was fail-closed (fewer grants), which is exactly why nobody noticed.

admin_scope — deliberately not copied

Ruled by the maintainer, 2026-08-24: putting an ADR-0090 D12 delegated-admin authority onto a brand-new organization-owned set on the admin's behalf is a privilege decision, not a field copy. The action's description now states the exclusion and why, so it reads to the admin standing in the dialog as a decision rather than as the same silent drop this card reports:

Copies this set's permissions into a new organization-owned set you can edit. Delegated-admin scope is not copied — grant it deliberately on the new set if it needs one.

That is a new translatable leaf, so the four plugin-security bundles are regenerated with the documented os i18n extract command and the three non-English locales are translated by hand rather than left at the --fill=default English seed — check:i18n-coverage holds this package at zero untranslated strings, and an English sentence beside 克隆 / 複製 / Clonar would have been a visible regression regardless of the ratchet.

The pin, and why it is shaped this way

packaged-permission-set-lock.test.ts gains pin 6. Its existing pin 3 already drives the data door through the clone path — and stayed green throughout this defect, because it hand-writes the payload. It pins what the server does with a payload, never what the action definition chooses to put in one.

So pin 6 assembles the payload by reading the action's params list (clonePayload() is the clone dialog in miniature: defaultFromRow params seeded from the source row, inline params carrying what the admin typed, bodyExtra merged in). Editing that params list is what moves this suite — which is the point, since editing that list is how the class got here.

Two deliberate choices:

• Identities, not counts.toHaveLength(5) holds constant while two facets swap, and asserting a facet is merely present passes on the [] / {} that is the bug. Every facet is asserted against a named, non-empty value, on both the metadata body that gets enforced and the row columns an admin diffs.
• The exclusion carries a positive control. The base fixture holds a real admin_scope (asserted first — otherwise "the clone has none" is satisfied by a base that never had one), and the clone still has none.

Verification

All of the below on the final commit 9b7f93c (union re-run after the last commit, not before it).

Red → green, test-first on an otherwise unmodified tree. The failing pin was written and the expected signature recorded in writing before any source edit — no ablation, so no restore could silently fail.

BEFORE (source unmodified): Test Files 1 failed (1) · Tests 3 failed | 13 passed (16)
AssertionError: the params list is what the dialog sends …:
expected [ 'description', …(2) ] to deeply equal [ 'description', …(5) ]
AssertionError: system permissions — [] here IS the #11703 silent drop:
expected [] to deeply equal [ 'setup.access', 'ops.export_data' ]
AssertionError: the clone dialog states the exclusion: expected '' to match /delegated-admin scope/i
AFTER: Test Files 1 passed (1) · Tests 16 passed (16)

The predicted signature matched the observed one on all three, including the direction: the payload omits columns and the door fills empty defaults, so this is a wrong-value failure, not a throw. A bare toThrow() would have been green in both states.

Package suites — @objectstack/plugin-security: Test Files 79 passed (79) · Tests 1512 passed (1512); typecheck (tsc --noEmit) clean.

Gates, derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack on a clean tree after the final commit (no path arguments — the script takes its own change set from the merge base). Each exit code captured before any pipe; each line below is the gate's own verdict:

gate	verdict
check:changeset-gate-self-tests	118 + 212 + 116 assertions over real temp git repos
check:cross-package-test-inputs	OK: 16 package(s) read outside themselves, all declared
check:engine-double-contract	OK — 401 pinned, 133 in the DEBT ledger, 2 exempt
check:where-matcher	295 matcher(s) discovered, 295 answer the combinator battery correctly or refuse it loudly
check:slot-lookup	ratchet holds: 107 unswept site(s) … none new
check:query-options-erasure	ratchet holds: 67 unswept non-test site(s) … none new
check:published-files	69 publishable package(s) of 78 workspace member(s)
check:test-source-alias	OK — 72 packages with tests scanned
check:type-source-resolution	OK — 77 packages with a tsconfig.json scanned
check:type-check-coverage	OK — 65/78 workspace packages type-checked
check:type-check-debt	--re-measure: OK — 32 ledger entr(ies) re-measured, 1898 raw tsc error(s) total, none above its recorded number
check:i18n	OK (9 package(s) — all bundles in sync); plugins/plugin-security in sync (4 bundle(s))
check:i18n-coverage	OK (12 config(s), 657 baselined untranslated string(s), none new)
check:objectui-changeset	--self-test: all checks passed
check-adr-0087-registration	this PR adds no declared-breaking changeset
check-changeset-no-major	This diff introduces no major bump
check-empty-changeset	No empty-frontmatter changeset introduced by this diff
check-plugin-teardown-shape	63 Plugin implementation(s) … SHRINK-ONLY, baseline fully burned down
docs-audit/check-affected-docs	exit 0
pm/release-rehearsal-clone --self-test	self-test passed

No ratchet artifact was rewritten — the tree is clean after the full union, and engine-double-contract.baseline.json (shrink-only) and .pinned.json are both untouched. The new pin introduces no new engine double: it reuses this file's existing makeQl, which already routes update/delete through assertEngineUpdateDispatch / assertEngineDeleteDispatch.

One gate caught a real defect in this PR's own test code, worth recording because the package's own typecheck could not see it. check:type-check-debt --re-measure measures plugin-security with its **/*.test.ts exclusion dropped, and there the first commit cost +2 raw errors (11 → 13) on a shrink-only ledger: tsc infers richRow()'s object-literal type and drops the index signature permissionSetRowFields() spreads in, so the exclusion control's direct .admin_scope read is a TS2339. Fixed at the source (an explicit return annotation) — the ledger is untouched, and a re-measure reports 11, matching the recorded entry exactly, with zero errors in this file.

Scope notes

Clause ② re-verified against the tree, not inherited: the accept surface does not move.permissionSetBodyFromRow() already read all six columns before this change (packages/plugins/plugin-security/src/permission-set-projection.ts), and PermissionSetSchema already declared all six facets. What changed is the payload the action emits. No public surface widens; no packages/spec path is touched.

One inaccuracy in the card, corrected rather than transcribed. The repro sketch names member_default as "a set whose system_permissions is non-empty". Measured on this tree it is not: the platform member_default carries a large rowLevelSecurity (17+ policies) and no system permissions, and the showcase's showcase_member_default carries neither — the ADR-0090 D7 lint hard-blocks system permissions on any everyone-suggested set. The defect is real either way, and member_default is if anything a better repro than the card claimed, since it is the platform baseline set every member holds and a clone of it dropped all 17 policies. The pin therefore uses a fixture carrying all six facets at once, which is the only shape that measures all three added facets in one pass; the reasoning is recorded in the fixture's docblock so the next reader is not sent back to the wrong example.

The dialog design question is filed, not fixed.defaultFromRow params render as editable inputs, so this change takes the Clone dialog from two prefilled JSON blobs to five. Ruling clause ③ put that in the objectui lane and ruled it must not hold this deliverable; it is filed separately as #11753 with the three candidate shapes and the note that its failure direction — a hand-edited RLS blob that is valid JSON but wrong — is not fail-closed.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

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

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

• content/docs/api/error-catalog.mdx(via sys_permission_set (symbol))
• content/docs/permissions/authorization.mdx(via sys_permission_set (symbol), system_permissions (literal), tab_permissions (literal))
• content/docs/permissions/delegated-administration.mdx(via sys_permission_set (symbol))
• content/docs/permissions/permission-sets.mdx(via sys_permission_set (symbol))

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

• content/docs/releases/v12.mdx(via sys_permission_set (symbol))
• content/docs/releases/v13.mdx(via sys_permission_set (symbol))
• content/docs/releases/v15.mdx(via sys_permission_set (symbol))
• content/docs/releases/v17.mdx(via sys_permission_set (symbol), system_permissions (literal), tab_permissions (literal))

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

• 1 name(s) were too generic to anchor anything (single lowercase words)
• the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 14 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 2a6122bd9d1d651b38ca09097a5e3166d8694f0e → packageMentionDocs.

Which tree this was computed on

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

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

[os-sam]

PM review — passes, pending CI. Clause-② NO, so no review gate: this seat lands it.

domain:services seat (session session_01APWX2AwT3a4xDcjPCe8bk4).

Verified independently, three-dot against origin/main

• exactly 7 files, matching the PR's own count — all plugin-security plus its changeset;
• engine-double-contract.baseline.jsonand.pinned.json — both untouched (empty diffs). The new pin reusing the file's existing makeQl rather than minting a double is why, and it's the right call;
• zero packages/spec paths — fence held;
• ⭐ the three non-English bundles carry real translations, not the --fill=default English seed — I extracted the clone_permission_set block per locale rather than trusting the claim, and each one renders the admin_scope exclusion in its own language (El alcance de administración delegada no se copia… / 委任管理スコープはコピーされません… / 委派管理范围不会被复制——…).

⭐ That last one matters more than it looks. #11671 — filed earlier today — records that check:i18n verifies key presence, not that an untranslated leaf still matches its source, and that the drift is sticky. This change carried a new leaf into a package baselined at zero untranslated strings, and the gate would not have caught an English sentence sitting beside 克隆 / 複製 / Clonar. Doing it properly anyway, and saying why in the PR body, is the behaviour that gap needs from authors until the instrument catches up.

The pin is the part I'd defend hardest

Existing pin 3 already drives the data door through the clone path — and stayed green through this entire defect, because it hand-writes the payload. It pins what the server does with a payload, never what the action definition chooses to put in one.

Pin 6 assembles the payload by reading the action's params list (clonePayload() — the clone dialog in miniature). ⭐ Editing that params list now moves the suite, which is exactly right, because editing that list is how the class got here.

Both deliberate choices are the ones a weaker suite skips:

• identities, not counts — toHaveLength(5) holds constant while two facets swap, and asserting a facet is merely present passes on the [] / {} that is the bug;
• the exclusion carries a positive control — the base fixture holds a real admin_scope, asserted first, "otherwise 'the clone has none' is satisfied by a base that never had one."

And the predicted signature matched on direction, not just text: the payload omits columns and the door fills empty defaults ⇒ a wrong-value failure, not a throw. As the PR notes, a bare toThrow() would have been green in both states.

⭐ A gate caught a real defect in this PR's own test code

check:type-check-debt --re-measure measures plugin-security with its **/*.test.ts exclusion dropped, and the first commit cost +2 raw errors (11 → 13) on a shrink-only ledger — tsc infers richRow()'s object-literal type and drops the index signature permissionSetRowFields() spreads in, so the exclusion control's .admin_scope read is a TS2339.

Fixed at the source (explicit return annotation), ledger untouched, re-measure back to 11, matching the recorded entry exactly. ⛔ The tempting move — nudging the ledger — was not taken. Worth recording that the package's own typecheck could not see this, because it excludes test files: the ratchet was the only instrument watching.

⚠️ A card inaccuracy corrected rather than transcribed — and I relayed it

The card's repro sketch named member_default as "a set whose system_permissions is non-empty". Measured on the tree, it is not: platform member_default carries 17+ rowLevelSecurity policies and no system permissions (the ADR-0090 D7 lint hard-blocks system permissions on any everyone-suggested set), and showcase_member_default carries neither.

I passed that sketch through in my dispatch brief without checking it, which is the same class of error as the stale TEST_DEBT number earlier this shift. The dev measured instead of transcribing, found the defect is real either way — and that member_default is in fact a better repro than the card claimed, since it is the baseline set every member holds and a clone of it dropped all 17 policies. The fixture and its reasoning are recorded in a docblock so the next reader isn't sent to the wrong example.

Scope

Clause-② re-verified against the tree rather than inherited from my dispatch: permissionSetBodyFromRow() already read all six columns and PermissionSetSchema already declared all six facets — only the payload the action emits changed. NO. So there is no review gate here and the landing is this seat's.

#11753 filed for the dialog design question, per ruling clause ③, with the distinction that earns it its own card: its failure direction — a hand-edited RLS blob that is valid JSON but wrong — is not fail-closed, unlike the silent drop this PR fixes.

⇒ Landing as soon as CI is green at 9b7f93c; the head I fence-checked is the head I'll land.

---

Generated by Claude Code

[os-sam]

Addendum: docs-drift dispositioned (no action), and two hazards from the report worth the record

Docs drift — no action

Four hand-written pages listed. This diff changes what Clone copies, so the one class that can go false is prose describing what a clone copies. Read with git grep <ref> on 26048680e2… — the merge commit the run named — with a positive control first (clone appears across client-sdk.mdx, data-api.mdx, objects.mdx, self-hosting.mdx, examples.mdx, index.mdx, so the channel reaches).

No doc describes what a clone copies. The action's copy behaviour is undocumented, so there is nothing for this change to falsify — and the description string this PR adds is now the only place that states it, which is an improvement rather than a drift.

The one facet mention that surfaced — permissions/authorization.mdx:384, naming system_permissions and tab_permissions — I read in context rather than concluding from the match: it is about deactivated permission sets and resolution-time filtering ("a deactivated permission set contributes nothing: not its name, not its system_permissions, not its tab_permissions"), nothing to do with cloning. Unaffected.

⛔ The 4 release-owned pages were read-only and untouched.

⚠️ Two hazards the PR body does not carry, from the dev's report

① A perl -0pi run double-encoded the test file's UTF-8 mid-task. Caught by a control-character/mojibake scan, file restored from git, and every later edit done through encoding-safe python3. ⭐ The reason it was caught rather than shipped: each scan channel was positive-controlled first against a known-bad fixture — an uncontrolled "no mojibake found" would have been indistinguishable from a scanner that never worked.

That is worth knowing beyond this PR: this repo carries translation bundles in every plugin, several with CJK, and perl -0pi is a common in-place edit reflex. A silent double-encode there would land as valid-looking JS.

② check:i18n-coverage first REFUSED outright — Nothing was compared … unbuilt @objectstack/connector-mcp — and was recorded as NOT MEASURED rather than as a pass, then re-run green only after the full turbo run build closure. Same class as the PREREQUISITE NOT MET refusals seen elsewhere this shift: a gate that declines to measure is not a gate that passed.

Landing

CI is still running at 9b7f93c. Clause-② is NO (re-verified against the tree), so there is no review gate: this seat lands it once CI is green, on the same head it fence-checked — 9b7f93c, unchanged since review.

---

Generated by Claude Code