fix(security): project only spec-declared keys into permission-set metadata, and make a failed backfill loud (#4669)

`sys_permission_set` carries an `active` STORAGE COLUMN — the on/off switch the
Setup list views filter on and the two lifecycle actions toggle. ADR-0094 D4's
boot backfill turned a whole row into a metadata body, so `active` went with it;
#4001 then sealed `PermissionSetSchema` `.strict()`, and every backfill began
failing with `[invalid_metadata] … Unrecognized key(s) on this permission set:
'active'`. The failure was caught into one `warn` and no counter moved, so a
100%-failing projection path stayed green for a release.

`active` is row state, not a declaration — its entire consumer surface (column,
highlightFields, list-view filters, the actions' `bodyExtra`) is the record's
runtime switch, never a capability boundary an author declared. So the fix is on
the PROJECTION side; `packages/spec` is untouched.

- `permissionSetBodyFromRow()` / `mergeRowPatchIntoBody()` now pass through a
  whitelist DERIVED from `PermissionSetSchema.shape` — not a transcribed string
  list, which would silently drop the next key the spec grows (this defect, one
  layer over). Storage columns never enter a body, and a body STORED before
  #4001 (data at rest can still carry `active`) is filtered at the same choke
  point, so its data-door edits stop 422-ing.
- The activate/deactivate actions keep working: a patch that touches only row
  state is not a definition write and passes through to the driver, so the
  column write keeps its ordinary engine semantics and no bogus "customization"
  overlay is minted on a packaged set. The projector no longer reads `active`
  from a body either — a stale body can no longer re-activate a set an admin
  just switched off.
- A real backfill failure is now loud per AGENTS.md "Degradation log levels"
  (#4632): `error` level with the consequence and the fix, said once at the
  first failure, plus a new `ProjectionReconcileOutcome.backfillFailed` counter
  so the degradation lands in the RESULT and not only in a log line.

Tests: the mock protocol now validates with the REAL `PermissionSetSchema`,
exactly as `saveMetaItem` does — re-introducing the defect fails 6 of them with
the issue's verbatim error.

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

Author: claude

.changeset/permission-backfill-row-state-columns.md

@@ -0,0 +1,31 @@
+---
+"@objectstack/plugin-security": patch
+---
+
+fix(security): 让 permission-set 投影只写 spec 认的键，并把静默失败的 backfill 变响亮 (#4669)
+
+ADR-0094 D4 的 permission-set backfill 在 #4001 之后 **100% 失败**：`sys_permission_set`
+每一行都有 `active` 存储列，`permissionSetBodyFromRow()` 把整行转成 metadata body 时把它
+一起带上，而 #4001 已经把 `PermissionSetSchema` 封成 `.strict()` —— 于是每一次
+`saveMetaItem` 都抛 `[invalid_metadata] … Unrecognized key(s) on this permission set:
+'active'`。失败被 `catch` 成一条 `warn`、计数器不加一，所以测试全绿、没有任何自动信号：
+一个整条停摆的投影路径就这样过了一个发布周期。
+
+**归属判定：`active` 是行状态，不是声明。** 它的全部消费面 —— 表列、`highlightFields`、
+Setup 列表视图的过滤器、两个启停动作的 `bodyExtra: { active: … }` —— 都是记录的运行时开关，
+不是作者声明的能力边界。所以修法是在**投影侧挑键**，而不是把状态提升进 spec
+（`packages/spec/**` 零改动）。
+
+- `permissionSetBodyFromRow()` / `mergeRowPatchIntoBody()` 现在都经过一个**从
+  `PermissionSetSchema.shape` 派生**的键白名单（不是手抄的字符串数组 —— 手抄的话 spec 加键
+  时这里又会静默漏，正是本 bug 的翻版）。存储列（`active`、时间戳、`managed_by` /
+  `package_id` / `customized`）一律不进 metadata body；`#4001` 之前**已经落库**、body 里
+  仍带着 `active` 的历史 overlay 行，也在同一个闸口被滤掉，因此它们的数据门编辑不再报 422。
+- 两个启停动作行为不变：只含行状态的 PATCH 不再被改写成 metadata 写入，而是原样交给驱动
+  执行列写入（保留 history / `updated_at` / FLS 等正常语义），并且不会再给一个包自带的
+  permission set 平白造出一条“customization” overlay。投影通道则不再从 body 读 `active` ——
+  一次投影不会再用陈旧 body 把管理员刚停用的 set 重新打开。
+- backfill 真失败时按 AGENTS.md「Degradation log levels」(#4632) 变响亮：`error` 级、
+  文案写明后果（记录照常列出、看起来一切正常，但定义不在 metadata 里，重新 provision 不会
+  重建它）与修复动作，并新增 `ProjectionReconcileOutcome.backfillFailed` 计数，让降级出现在
+  结果里而不只在日志里。

packages/plugins/plugin-security/src/permission-set-projection.test.ts

@@ -9,9 +9,12 @@
  */
 
 import { describe, it, expect } from 'vitest';
+import { PermissionSetSchema } from '@objectstack/spec/security';
 import {
   permissionSetRowFields,
   permissionSetBodyFromRow,
+  permissionSpecBodyKeys,
+  pickRowStateColumns,
   mergeRowPatchIntoBody,
   recordDiffersFromBody,
   upsertEnvPermissionSet,
@@ -81,6 +84,21 @@ function makeProtocol(ql: any, declared: Record<string, any> = {}) {
       projector = fn;
     },
     async saveMetaItem(req: { type: string; name: string; item: any; actor?: string }) {
+      // [#4669] The REAL `PermissionSetSchema`, exactly as `saveMetaItem` runs
+      // it (metadata-protocol/src/protocol.ts → `resolveOverlaySchema`), same
+      // `[invalid_metadata]` 422 envelope. Without this the mock accepts any
+      // object and the suite stays green while every real backfill fails —
+      // which is how a 100%-failing projection shipped.
+      const parsed = PermissionSetSchema.safeParse(req.item);
+      if (!parsed.success) {
+        const summary = parsed.error.issues
+          .map((i: any) => `${i.path.join('.') || '<root>'}: ${i.message}`)
+          .join('; ');
+        const err: any = new Error(`[invalid_metadata] permission/${req.name} failed spec validation: ${summary}`);
+        err.code = 'INVALID_METADATA';
+        err.status = 422;
+        throw err;
+      }
       const existing = overlayFor(req.name);
       if (existing) existing.metadata = JSON.stringify(req.item);
       else {
@@ -157,12 +175,83 @@ describe('permissionSetBodyFromRow / permissionSetRowFields (round-trip)', () =>
     expect(body.rowLevelSecurity[0].using).toBe('org == current_user.org');
     expect(body.tabPermissions).toEqual({ crm_leads: 'visible' });
     expect(body.adminScope.businessUnit).toBe('Sales');
-    expect(body.active).toBe(true);
     // and projecting the rebuilt body changes nothing
     expect(recordDiffersFromBody(row, body)).toBe(false);
   });
 });
 
+// ── The definition ⊆ spec contract (#4669) ─────────────────────────────────
+//
+// `sys_permission_set` carries columns the DEFINITION does not (`active`, the
+// timestamps, the provenance trio). Feeding them to `saveMetaItem` is what
+// #4001's `.strict()` schema rejects, and what took the ADR-0094 D4 backfill
+// to a 100% failure rate.
+
+describe('row→body projection keeps ONLY spec-declared keys (#4669)', () => {
+  const legacyRow = () => ({
+    id: 'ps_1',
+    name: 'organization_admin',
+    ...permissionSetRowFields(envBody()),
+    // every storage column a real row carries…
+    active: true,
+    managed_by: 'admin',
+    package_id: null,
+    customized: false,
+    created_at: '2026-01-01T00:00:00Z',
+    updated_at: '2026-02-02T00:00:00Z',
+    // …plus a column this code has never heard of
+    some_future_column: 'whatever',
+  });
+
+  it('the whitelist is DERIVED from the spec schema, not transcribed', () => {
+    const keys = permissionSpecBodyKeys();
+    // identical to PermissionSetSchema's own shape — the single source
+    expect([...keys].sort()).toEqual(Object.keys((PermissionSetSchema as any).shape).sort());
+    expect(keys.has('objects')).toBe(true);
+    expect(keys.has('systemPermissions')).toBe(true);
+    expect(keys.has('adminScope')).toBe(true);
+    // `active` is a TABLE column, never a spec key — that is the whole bug
+    expect(keys.has('active')).toBe(false);
+  });
+
+  it('drops `active` and every other storage column from the projected body', () => {
+    const body = permissionSetBodyFromRow(legacyRow());
+    for (const col of ['active', 'managed_by', 'package_id', 'customized', 'created_at', 'updated_at', 'id', 'some_future_column']) {
+      expect(body, `storage column '${col}' must not enter the metadata body`).not.toHaveProperty(col);
+    }
+    // the definition itself survives intact
+    expect(body.objects).toEqual(envBody().objects);
+    expect(body.systemPermissions).toEqual(envBody().systemPermissions);
+  });
+
+  it('every key the projection emits is one the spec ACCEPTS (parsed by the real schema)', () => {
+    const parsed = PermissionSetSchema.safeParse(permissionSetBodyFromRow(legacyRow()));
+    expect(parsed.success, parsed.success ? '' : JSON.stringify(parsed.error.issues)).toBe(true);
+    // …and the reverse guard: a spec RENAME must fail here rather than silently
+    // dropping the value at runtime.
+    const keys = permissionSpecBodyKeys();
+    for (const key of Object.keys(permissionSetBodyFromRow(legacyRow()))) {
+      expect(keys.has(key), `body key '${key}' is not declared by PermissionSetSchema`).toBe(true);
+    }
+  });
+
+  it('filters a body STORED before #4001 (data at rest can still carry `active`)', () => {
+    // a legacy sys_metadata overlay written while the schema still stripped it
+    const legacyStored = { ...envBody(), active: false, _packageId: 'com.x' };
+    const merged = mergeRowPatchIntoBody(legacyStored, { label: 'Renamed' });
+    expect(merged).not.toHaveProperty('active');
+    expect(merged).not.toHaveProperty('_packageId');
+    expect(PermissionSetSchema.safeParse(merged).success).toBe(true);
+  });
+
+  it('pickRowStateColumns isolates the record-state columns (normalized)', () => {
+    expect(pickRowStateColumns({ active: 'false', label: 'x' })).toEqual({ active: false });
+    expect(pickRowStateColumns({ active: true })).toEqual({ active: true });
+    expect(pickRowStateColumns({ label: 'x' })).toBeNull();
+    expect(pickRowStateColumns(null)).toBeNull();
+  });
+});
+
 describe('upsertEnvPermissionSet (ADR-0094 — record is a pure projection)', () => {
   it('CREATES a missing record (managed_by admin) — Studio-authored sets appear in Setup', async () => {
     const ql = makeQl();
@@ -176,16 +265,28 @@ describe('upsertEnvPermissionSet (ADR-0094 — record is a pure projection)', ()
     expect(JSON.parse(row.object_permissions)).toEqual(envBody().objects);
   });
 
-  it('projects all facets (and active) onto an existing env-authored row', async () => {
+  it('projects all facets onto an existing env-authored row', async () => {
     const ql = makeQl();
     ql.permRows.push({ id: 'ps_env', name: 'organization_admin', managed_by: 'user', system_permissions: '[]', active: true });
-    const r = await upsertEnvPermissionSet(ql, envBody({ active: false }));
+    const r = await upsertEnvPermissionSet(ql, envBody());
     expect(r.updated).toBe(1);
     const row = ql.permRows[0];
     expect(row.id).toBe('ps_env'); // id stable — junction FKs stay valid
     expect(JSON.parse(row.system_permissions)).toEqual(['setup.access', 'manage_org_users']);
     expect(JSON.parse(row.admin_scope).businessUnit).toBe('Sales');
-    expect(row.active).toBe(false);
+  });
+
+  it('[#4669] NEVER re-flips `active` from a body — it is row state, not definition', async () => {
+    // A body carrying `active` can only be legacy data at rest (pre-#4001) or a
+    // caller mistake. Projecting it would silently undo an admin's
+    // deactivate — the record's switch is the record's own.
+    const ql = makeQl();
+    ql.permRows.push({ id: 'ps_env', name: 'organization_admin', managed_by: 'user', system_permissions: '[]', active: false });
+    await upsertEnvPermissionSet(ql, { ...envBody(), active: true } as any);
+    expect(ql.permRows[0].active, 'a stale body must not re-activate a deactivated set').toBe(false);
+    // …and a record the projector CREATES starts active (column default).
+    await upsertEnvPermissionSet(ql, envBody({ name: 'fresh_set' }));
+    expect(ql.permRows.find((r: any) => r.name === 'fresh_set')?.active).toBe(true);
   });
 
   it('projects onto a legacy row with ABSENT provenance (platform default)', async () => {
@@ -426,15 +527,76 @@ describe('createPermissionSetWriteThrough (data door → metadata store)', () =>
     // metadata is the store that changed…
     const overlay = JSON.parse(ql.metaRows[0].metadata);
     expect(overlay.systemPermissions).toEqual(['setup.access']);
-    expect(overlay.active).toBe(false);
     expect(overlay.objects).toEqual(envBody().objects); // unmentioned facets preserved
+    // [#4669] …but `active` rode along as a COLUMN, never as a body key: the
+    // definition stays spec-clean while the record's switch still flips.
+    expect(overlay).not.toHaveProperty('active');
     // …and the record followed via projection
     expect(JSON.parse(ql.permRows[0].system_permissions)).toEqual(['setup.access']);
     expect(ql.permRows[0].active).toBe(false);
     expect(ql.permRows[0].id).toBe(rowId);
     expect(opCtx.result?.id).toBe(rowId);
   });
 
+  it('[#4669] the activate/deactivate ACTIONS write the column and nothing else', async () => {
+    // `sys-permission-set.object.ts` ships two `type:'api'` actions that PATCH
+    // /data/sys_permission_set/{id} with `bodyExtra: { active: true|false }`.
+    // A row-state-only patch is not a definition write: it passes through to
+    // the driver, mints no overlay, and touches the metadata store not at all.
+    const ql = makeQl();
+    const protocol = makeProtocol(ql);
+    registerPermissionSetProjection(protocol, { ql });
+    await protocol.saveMetaItem({ type: 'permission', name: 'organization_admin', item: envBody() });
+    const rowId = ql.permRows[0].id;
+    const savesBefore = protocol.saves.length;
+    const metaRowsBefore = JSON.stringify(ql.metaRows);
+    const mw = makeMiddleware(ql, protocol);
+
+    for (const active of [false, true]) {
+      const nextCalled = await run(mw, {
+        object: 'sys_permission_set', operation: 'update', context: userCtx,
+        data: { id: rowId, active },
+      });
+      expect(nextCalled, 'the driver performs the column write, with its ordinary semantics').toBe(true);
+    }
+    expect(protocol.saves.length, 'no metadata write for a pure row-state patch').toBe(savesBefore);
+    expect(JSON.stringify(ql.metaRows)).toBe(metaRowsBefore);
+  });
+
+  it('[#4669] deactivating a PACKAGE-owned set mints no customization overlay', async () => {
+    const ql = makeQl();
+    const declaredBody = envBody({ name: 'crm_rep', systemPermissions: ['pkg.baseline'] });
+    (ql as any).registry = { listItems: (t: string) => (t === 'permission' ? [declaredBody] : []) };
+    const protocol = makeProtocol(ql, { crm_rep: declaredBody });
+    registerPermissionSetProjection(protocol, { ql });
+    ql.permRows.push({ id: 'ps_pkg', name: 'crm_rep', managed_by: 'package', package_id: 'com.example.crm', system_permissions: '["pkg.baseline"]', active: true });
+    const mw = makeMiddleware(ql, protocol);
+    const nextCalled = await run(mw, {
+      object: 'sys_permission_set', operation: 'update', context: userCtx, data: { id: 'ps_pkg', active: false },
+    });
+    expect(nextCalled).toBe(true);
+    expect(ql.metaRows.length, 'switching a packaged set off is not a customization of it').toBe(0);
+    expect(ql.permRows[0].customized).toBeUndefined();
+  });
+
+  it('[#4669] INSERT honours an explicit `active` on the record (Clone action sends one)', async () => {
+    const ql = makeQl();
+    const protocol = makeProtocol(ql);
+    registerPermissionSetProjection(protocol, { ql });
+    const mw = makeMiddleware(ql, protocol);
+    const opCtx: any = {
+      object: 'sys_permission_set', operation: 'insert', context: userCtx,
+      data: {
+        name: 'support_agent', label: 'Support Agent', active: false,
+        object_permissions: JSON.stringify({ ticket: { allowRead: true } }),
+      },
+    };
+    await run(mw, opCtx);
+    expect(protocol.saves[0].item, 'the definition never carries row state').not.toHaveProperty('active');
+    expect(ql.permRows[0].active).toBe(false);
+    expect(opCtx.result?.active).toBe(false);
+  });
+
   it('UPDATE that renames is rejected (the name is the metadata identity)', async () => {
     const ql = makeQl();
     const protocol = makeProtocol(ql);
@@ -570,6 +732,7 @@ describe('reconcilePermissionSetProjection', () => {
     });
     const out = await reconcilePermissionSetProjection(protocol, { ql });
     expect(out.backfilledIntoMetadata).toBe(1);
+    expect(out.backfillFailed).toBe(0);
     expect(ql.metaRows.length).toBe(1);
     const body = JSON.parse(ql.metaRows[0].metadata);
     expect(body.objects).toEqual({ ticket: { allowRead: true } });
@@ -578,6 +741,83 @@ describe('reconcilePermissionSetProjection', () => {
     expect(out2.backfilledIntoMetadata).toBe(0);
   });
 
+  it('[#4669] a row carrying the `active` STORAGE COLUMN backfills instead of failing spec validation', async () => {
+    // The reported symptom: every `sys_permission_set` row has an `active`
+    // column, `permissionSetBodyFromRow` handed it to `saveMetaItem`, and
+    // #4001's `.strict()` schema rejected all of them — a 100%-failing
+    // backfill behind one `warn`, with `backfilledIntoMetadata` stuck at 0.
+    const ql = makeQl();
+    const protocol = makeProtocol(ql); // validates with the real PermissionSetSchema
+    ql.permRows.push({
+      id: 'ps_d8', name: 'd8_qc_user', managed_by: 'admin',
+      active: true, customized: false, package_id: null,
+      created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-02T00:00:00Z',
+      label: 'D8 QC User', ...permissionSetRowFields(envBody({ name: 'd8_qc_user' })),
+    });
+    const logs: Array<{ level: string; msg: string }> = [];
+    const logger = {
+      info: (m: string) => logs.push({ level: 'info', msg: m }),
+      warn: (m: string) => logs.push({ level: 'warn', msg: m }),
+      error: (m: string) => logs.push({ level: 'error', msg: m }),
+    };
+    const out = await reconcilePermissionSetProjection(protocol, { ql, logger });
+    expect(out.backfilledIntoMetadata).toBe(1);
+    expect(out.backfillFailed).toBe(0);
+    expect(logs.some((l) => /backfill into metadata failed|FAILED/i.test(l.msg))).toBe(false);
+    const stored = JSON.parse(ql.metaRows[0].metadata);
+    expect(stored).not.toHaveProperty('active');
+    expect(stored.name).toBe('d8_qc_user');
+  });
+
+  it('[#4669/#4632] a REAL backfill failure is loud: error level, counted, consequence + fix', async () => {
+    const ql = makeQl();
+    const protocol = makeProtocol(ql);
+    // Not a key problem — the stored facet JSON itself is off-contract, so no
+    // amount of key-filtering saves it. This is the case that MUST shout.
+    ql.permRows.push({
+      id: 'ps_bad', name: 'broken_set', managed_by: 'admin', active: true,
+      label: 'Broken Set', object_permissions: JSON.stringify({ ticket: { allowRead: 'yes-please' } }),
+    });
+    ql.permRows.push({
+      id: 'ps_bad2', name: 'broken_set_2', managed_by: 'admin', active: true,
+      label: 'Broken Set 2', object_permissions: JSON.stringify({ ticket: { nonsense: true } }),
+    });
+    // `error` follows the platform Logger contract: (message, error?, meta?).
+    const logs: Array<{ level: string; msg: string; meta?: any; cause?: Error }> = [];
+    const logger = {
+      info: (m: string, meta?: any) => logs.push({ level: 'info', msg: m, meta }),
+      warn: (m: string, meta?: any) => logs.push({ level: 'warn', msg: m, meta }),
+      error: (m: string, cause?: Error, meta?: any) => logs.push({ level: 'error', msg: m, cause, meta }),
+    };
+    const out = await reconcilePermissionSetProjection(protocol, { ql, logger });
+
+    // counted in the RESULT — not only in a log line nobody reads
+    expect(out.backfillFailed).toBe(2);
+    expect(out.backfilledIntoMetadata).toBe(0);
+    expect(ql.metaRows.length).toBe(0);
+
+    // level: error, never warn/info for a durability degradation
+    const errors = logs.filter((l) => l.level === 'error');
+    expect(errors.length).toBeGreaterThan(0);
+    expect(logs.some((l) => l.level === 'warn' && /backfill/i.test(l.msg))).toBe(false);
+    // said ONCE, at the first failure — not once per failed write
+    const firstFailure = errors[0]!;
+    expect(errors.filter((l) => /backfill into metadata FAILED/.test(l.msg)).length).toBe(1);
+    // the consequence…
+    expect(firstFailure.msg).toMatch(/Nothing will look broken/);
+    expect(firstFailure.msg).toMatch(/re-provision/);
+    // …and the fix
+    expect(firstFailure.msg).toMatch(/Fix:/);
+    expect(firstFailure.meta?.name).toBe('broken_set');
+
+    // the summary carries the failure too — an `info` "reconciled" line over a
+    // failed backfill is the reassuring half-truth the rule exists to remove
+    const summary = errors.at(-1)!;
+    expect(summary.msg).toMatch(/2 FAILED backfill/);
+    expect(summary.meta?.failedNames).toEqual(['broken_set', 'broken_set_2']);
+    expect(logs.some((l) => l.level === 'info' && /reconciled/.test(l.msg))).toBe(false);
+  });
+
   it('heals a record that drifted from an EXISTING metadata definition (metadata wins)', async () => {
     const ql = makeQl();
     const declared = { member_default: envBody({ name: 'member_default', systemPermissions: ['declared.baseline'] }) };