docs(19.6.1): record gap-closure code review

.planning/phases/19.6.1-backend-administrators-screen/19.6.1-REVIEW-DISPOSITION.md

@@ -5,7 +5,7 @@ titles: json
 findings:
   - id: CR-01
     severity: critical
-    disposition: open
+    disposition: fixed
     title: "Roles screen lets a non-superuser rename or create a role onto a code with code-declared grants (D-07 ceiling bypass, self-escalation)"
   - id: WR-01
     severity: warning
@@ -17,7 +17,7 @@ findings:
     title: "Stored override values other than 1/-1 are dropped on every save, re-granting what they removed"
   - id: WR-03
     severity: warning
-    disposition: open
+    disposition: fixed
     title: "Record actions contributed to the Administrators screen bypass D-19 (outranking) and D-09 (last superuser)"
   - id: IN-01
     severity: info
@@ -43,19 +43,19 @@ findings:
     severity: info
     disposition: open
     title: "Role picker on your own record is editable in the SPA but always refused"
-open: 10
+open: 8
 total: 10
-recorded: 2026-10-22T10:36:13.024Z
+recorded: 2026-10-22T11:50:00.000Z
 ---
 
 # Phase 19.6.1: Code Review Disposition
 
 | Finding | Severity | Disposition | Source |
 |---------|----------|-------------|--------|
-| CR-01 | critical | open | - |
+| CR-01 | critical | fixed | 19.6.1-04 refuseCodeDeclaredCode |
 | WR-01 | warning | open | - |
 | WR-02 | warning | open | - |
-| WR-03 | warning | open | - |
+| WR-03 | warning | fixed | 19.6.1-04 wrapAdministratorsContributedRun (D-19). D-09 on contributed Run stays out of the gap plan |
 | IN-01 | info | open | - |
 | IN-02 | info | open | - |
 | IN-03 | info | open | - |

.planning/phases/19.6.1-backend-administrators-screen/19.6.1-REVIEW.md

@@ -80,6 +80,26 @@ findings:
   info: 6
   total: 10
 status: issues_found
+gap_closure:
+  reviewed: 2026-10-22T11:50:00Z
+  depth: standard
+  plan: "04"
+  files_reviewed: 7
+  files_reviewed_list:
+    - modules/cabana/backend_roles_admin.go
+    - modules/cabana/action_extensions.go
+    - modules/cabana/action_extensions_test.go
+    - modules/cabana/roles_postgres_test.go
+    - modules/cabana/action_extensions_postgres_test.go
+    - modules/pact/capabilities.go
+    - scripts/check-phase19.6.1.sh
+  findings:
+    critical: 0
+    warning: 0
+    info: 0
+    nice_to_have: 0
+    total: 0
+  status: clean
 ---
 
 # Phase 19.6.1: Code Review Report
@@ -145,6 +165,8 @@ func (c rolesAdmin) checkCodeGrants(ctx context.Context, newCode, storedCode str
 ```
 A stricter option that matches D-16 is to refuse `len(grants) > 0 && newCode != storedCode` for everyone, so a role can never become a code-declared system role through the screen. Add a Postgres test that renames a held role onto a code-declared code as a non-superuser and expects 403.
 
+**Gap-closure (2026-10-22):** addressed by plan 04 (`refuseCodeDeclaredCode`). See the gap-closure pass below. Finding kept for history.
+
 ## Warnings
 
 ### WR-01: D-07/D-19 ceilings ignore a scoped personal API token's scope
@@ -179,6 +201,8 @@ Winter strips 0 on write, so the precondition needs rows written by SQL or other
 The first planned consumer is 19.7's frontend-link action, which binds a frontend account to a backend admin that MCP then acts as. Through the seam, any `backend.manage_users` holder could link their own frontend account to an administrator who outranks them, which is the account-takeover path D-19 was written to close. Every contributor would have to re-implement unexported guards it cannot reach.
 **Fix:** When compiling extensions whose target is `AdministratorsControllerID`, wrap each contributed `Run` (and `Applies`) so that `guardOutranked(ctx, reg, principal, record.(*BackendUser))` runs inside the action transaction before the contributor's `Run`. Alternatively, export a `cabana.GuardAdministratorWrite(ctx, record)` helper and document in `pact.AdminActionExtension` that it is required. Add a test with a contributed action on an outranking target that expects 403.
 
+**Gap-closure (2026-10-22):** D-19 half addressed by plan 04 (`wrapAdministratorsContributedRun`). D-09 (last superuser on a contributed Run) and wrapping `Applies` were out of that plan's contract and are not re-filed. Finding kept for history.
+
 ## Info
 
 ### IN-01: Last-superuser guard silently depends on READ COMMITTED
@@ -219,6 +243,56 @@ The first planned consumer is 19.7's frontend-link action, which binds a fronten
 
 ---
 
-_Reviewed: 2026-10-22T10:34:44Z_
+## Gap-closure pass (2026-10-22)
+
+**Reviewed:** 2026-10-22T11:50:00Z
+**Depth:** standard
+**Plan:** 04 (commits `068adaa`, `cb460b6`, `e620860`, `6bd2506`)
+**Files Reviewed:** 7
+**Status:** clean
+
+This pass reviewed only the plan-04 gap-closure sources at HEAD. It does not re-review the original 70-file phase.
+
+### Gap-closure files
+
+- `modules/cabana/backend_roles_admin.go` (`refuseCodeDeclaredCode`, `FormBeforeCreate`, `FormBeforeUpdate`)
+- `modules/cabana/action_extensions.go` (`wrapAdministratorsContributedRun`, `compileActionExtensions` wrap)
+- `modules/cabana/action_extensions_test.go`
+- `modules/cabana/roles_postgres_test.go` (`TestRolesCodeDeclaredCodeRefused`)
+- `modules/cabana/action_extensions_postgres_test.go` (`TestActionExtensionOutranked`)
+- `modules/pact/capabilities.go` (`AdminActionExtension` comment)
+- `scripts/check-phase19.6.1.sh` (`DOCKER_TESTS` names)
+
+### Gap-closure summary
+
+No new Critical, Warning, Info, or Nice-to-have findings.
+
+`refuseCodeDeclaredCode` is called from create (stored code `""`) and update (stored code from the save transaction). It refuses when `submitted != stored` and `len(registry.rolePermissions(submitted)) > 0`, including for a superuser. Same-code saves of a system role still pass. `rolePermissions` is nil-safe. The 422 shape reuses `backend::lang.user.role.code_locked` on `code`. `guardOutranked` stays unexported.
+
+`compileActionExtensions` wraps a contributed `Run` only when `ext.Controller == AdministratorsControllerID`, after `checkContributedAction` has required a non-nil `Run`. The wrapper type-asserts `*BackendUser`, fails closed without calling `next` on a bad record, then calls `guardOutranked` inside `CRUDService.RecordAction`'s transaction. `lifecycleFailure` / `actionFailure` pass `*ForbiddenError` through as 403. Other controller IDs are stored unchanged. `Applies` is not wrapped, as the plan required.
+
+The boot smoke now supplies a superuser principal so the wrap's `actsAsSuperuser` short-circuit avoids needing `TxFromContext` in a unit test. The Postgres tests cover the HTTP paths: create/rename onto a unique code-declared code with no row (editor and superuser), a unique non-declared create, a same-code system-role save, an outranking contributed `acme_ping` (403, spy empty), built-in deactivate still 403, and a covered target still 200 inside the transaction. Both names are in `DOCKER_TESTS` and not in `SHORT_TESTS`. The `AdminActionExtension` comment states that cabana, not the contributor, applies `user.outranked`. No consuming-application names, no new dependency, YAML parser unchanged.
+
+Prior findings WR-01, WR-02, and IN-01..IN-06 were out of this pass. WR-03's D-09 clause (contributed Run still does not call `guardLastSuperuser`) remains as originally written; a non-superuser still cannot load a superuser row (`scopeAdministrators` → 404), so that leftover is the superuser-on-superuser path the plan did not take.
+
+### Gap-closure Critical Issues
+
+None.
+
+### Gap-closure Warnings
+
+None.
+
+### Gap-closure Info
+
+None.
+
+### Gap-closure Nice-to-have
+
+None.
+
+---
+
+_Reviewed: 2026-10-22T10:34:44Z (original); 2026-10-22T11:50:00Z (gap-closure plan 04)_
 _Reviewer: Claude (msd-code-reviewer)_
 _Depth: standard_