--- status: complete supersedes: permissions-metadata-refactor-drift.md related: - docs/design/permission-system/ - backend/internal/bootstrap/standard_permissions.json - backend/internal/controlplane/packs/permissions.go - backend/internal/features/permissions/service.go - backend/internal/infrastructure/persistence/perm_system.go --- # Permission Metadata Refactor — Outcome Companion to the P0 drift audit (`permissions-metadata-refactor-drift.md`). This document records what the refactor actually changed across phases P1–P16. ## Motivation The pre-refactor permission system had **three authoring sites** for the same 70-permission catalog: 1. Go enum (`PermActivateOrder Permission = "ActivateOrder"` × 70) 2. Go catalog function (`AllSystemPermissions() []PermissionDefinition`) 3. Physical bitmap columns on `PermissionSet` (`PermissionsViewSetup TINYINT(1)` × 26) Adding a permission required editing at least two of these sites and possibly a third (bitmap column + reconcile pass). ProfileId-style behavior ("system_admin can do X") and field-writability maps were creeping into platform code because the enum was treated as canonical instead of derivative. The refactor collapses everything to one authoring site — `_System_Permission` rows, seeded from a JSON pack — and treats the engine code as a pure row reader. ## Final Architecture ### Sole authoring site — `backend/internal/bootstrap/standard_permissions.json` 70 rows. Adding a permission is a pack edit, not a code edit: ```json { "apiName": "ApiEnabled", "label": "API Enabled", "category": "Administrative", "description": "Allows API access", "defaultGrantedTo": ["SystemAdmin"] } ``` Admins can add org-specific permissions at runtime via `_System_CustomPermission` rows; those flow through the same runtime evaluator. ### Seeder — `backend/internal/controlplane/packs/permissions.go` `SeedSystemPermissions` reads the pack via `bootstrap.LoadStandardPermissions()` and writes two batches: - Batch 1: one `_System_Permission` row per pack entry, including `RequiresPermission` (prereq chain is data, not Go). - Batch 2: default grants derived from each row's `defaultGrantedTo` audience list, written to `_System_PermissionSetPermission` (the junction SSoT). Truth-of-pack: after UPSERT, a scoped `DELETE ... NOT IN (...)` removes grants whose `(permSetID, apiName)` is no longer declared for the two backing default perm sets (P13). Admin-created custom perm sets are untouched. The three-direction reconcile pass (catalog ↔ columns, columns ↔ grants, grants ↔ catalog) was deleted in P5. Correctness now comes from having one authoring site rather than reconciling three. ### Runtime — row reads, no Go catalog - `PermissionRepository.ListSystemPermissions(ctx)` — single query against `_System_Permission`, projects all 8 columns including `RequiresPermission`. - `PermissionRepository.GetPermissionSetGrantsMany(ctx, ids)` (P11) — batched junction lookup keyed by PermissionSet ID. Used by the read-path projection to avoid N+1 on list/query. - `PermissionService.ApplyPermissionChanges(ctx, permSetID, current, changes, user)` — the sole write gate (P12). Validates catalog membership of every apiName (rejects orphans with `"unknown permission: X"`), validates the prereq chain against the merged final state, writes diff-only junction rows, records one Setup Audit Trail entry per flip. - `PermissionService.ValidatePermissionPrerequisites(ctx, current, changes)` — loads the row catalog, walks `RequiresPermission` pointers against the merged final state. Called from `ApplyPermissionChanges` *only* when the diff is non-empty (P11 ordering change). - `PermissionRepository.SetSystemPermission(ctx, permSetID, apiName, enabled)` — the low-level junction upsert. Unexported at the service layer (P9 of the response series) so `ApplyPermissionChanges` is the only public write gate. No column write-through, no bitmap maintenance. ### SObject projection — Permissions as virtual schema fields The SF wire protocol uses `PermissionsViewSetup: true` on the `PermissionSet` SObject. The 26 physical bitmap columns were dropped in P6 (original series); the post-review response series turned Permissions into **schema-declared virtual fields** so every engine — describe, query, write, validation — treats them identically to physical boolean fields and diverges only at the persistence boundary. The `SchemaDecorator` (P2 of the response series) registers itself at cache-build time and, for `PermissionSet` only, appends one synthesized `FieldMetadata` per active `_System_Permission` row with: - `Type: Boolean`, `IsVirtual: true`, `VirtualKind: "SystemPermission"`, `VirtualSourceAPIName: ` - Capability bits gated by the first-version contract: `Filterable=false`, `Sortable=false`, `Groupable=false`, `Aggregatable=false` (no SOQL rewrite to junction logic yet — describe must not advertise capabilities the query engine cannot honor). `Createable/Updateable/Readable` inherit from the PermissionSet object-level CRUD for the caller. - **Write path** (`persistence_insert.go`, `persistence_bulk_insert.go`, `persistence_service.go` Update): Normalize → Extract → Validate. Virtual boolean values coerce through `NormalizeSObject` with the same accept-set as `ValidateSchemaCompliance.validateType` (P13 parity lock); unrecognized types are left as-is so the downstream guard produces a clear error. `ExtractPermissionGrants(record, schema)` returns `(map, error)` and **fails loud** on non-bool (P12) — the silent-revoke regression flagged in review #1831 P1 can no longer recur. Matched keys are deleted from the payload before validation / FLS; the grants map routes to `ApplyPermissionChanges` inside the same transaction as the physical write. - **Update true no-op**: `currentGrants` are loaded under the SObject lock before the change gate. The gate returns early when both the physical diff AND the permission diff (vs. currentGrants) are empty, so a PATCH that only repeats the current grant state produces zero writes, zero audit entries, and no `LastModifiedDate` bump (P11). The loaded snapshot is reused by `ApplyPermissionChanges` downstream to avoid a duplicate read. - **Read path — describe** (`sobject_describe.go`): no PermissionSet- specific branch; the handler walks `schema.Fields` like any other object. Virtual fields appear naturally because the decorator placed them there. Capability overrides for `IsVirtual` entries are applied in the same DTO loop (Filterable/Sortable/Groupable = false). - **Read path — retrieve / query** (`persistence_bulk_ops.go`, `query_service.go`): post-fetch, batches `GetPermissionSetGrantsMany` and fans each active catalog entry into a boolean key on the record. Projection gate fires when either `SelectFields` is empty (SELECT *) or any requested name resolves to a virtual `SystemPermission` field via `models.FindField` — case-insensitive via `FieldsMap`. Non- permission-shaped queries pay zero cost. - **Catalog-active consistency** (P12 of the response series): both `getAllPermissionSetPermissions` and `getGroupSystemPermissions` `INNER JOIN _System_Permission ... IsActive = 1`. Deactivated catalog rows (tombstoned by Batch 1b when removed from the pack) cannot bleed their historical grants into `/permissions/effective` or `CheckSystemPermission`. `_System_Permission` is the runtime SSoT for what a permission even *is*. ### Permission Category — fully row-driven `_System_Permission.Category` (string column) is the only category source. The Go `PermissionCategory` enum and 13 `PermCat*` constants were zero-use deadweight — deleted in P8. ### CustomPermission parallel infrastructure `_System_CustomPermission` + `ProfileCustomPermission` + `PermissionSetCustomPermission` form a parallel, fully admin-authorable lane for org-specific permissions (e.g. action-visibility gates like `SkipOpportunityOnConvert`). CRUD flows through the generic SObject REST surface — no dedicated endpoints — matching Salesforce's treatment of CustomPermission as an ordinary metadata object. Runtime evaluation (`GetEffectiveCustomPermissions`) joins Profile + PermissionSet grant paths with `IsActive`/`IsDeleted` guards and merges with system perms in `service_bulk_perms.go`. ## What was deleted | Removed | LoC | Rationale | |---|---|---| | `AllSystemPermissions() []PermissionDefinition` | ~88 | Row catalog replaces it | | `PermissionDefinition` struct | (included above) | Same | | `permissionSetSystemPermissionColumns` map (26 entries) | ~30 | Bitmap columns gone | | `permColumnToApiName` / `permApiNameToColumn` inverse maps | ~60 | Initially kept as prefix-strip helpers; **response-series P6** deleted them. Callers now read `FieldMetadata.VirtualSourceAPIName` directly — the prefix convention is data (lives on the synthesized field), not code. | | `PermissionVirtualFieldPrefix` / `IsPermissionVirtualField` / `BuildPermissionVirtualFields` | ~50 | Initially added to P11 of the original series for describe/query integration; **response-series P6** deleted them. Virtual fields now live in `schema.Fields` via the decorator, so existence checks use `FindField + IsVirtual + VirtualKind` and describe iterates `schema.Fields` uniformly. | | `init()` populating `PermissionColumnMappings` | ~10 | No consumer | | Three-direction reconcile pass | ~70 | One authoring site, no reconcile needed | | `GrantProfilePermission`/`RevokeProfilePermission` (service + repo) | ~30 | Zero callers; bypassed `ApplyPermissionChanges` gate (original P16) | | `BulkGrantPermissions`/`BulkRevokePermissions` + bulk repo impl (`perm_bulk.go`) | ~60 | Same (original P16) | | `PermissionService.SetSystemPermission` exported wrapper | ~10 | Response-series P9 unexported the service-layer bypass route. `ApplyPermissionChanges` is the only public write gate. | | 26 `Permissions TINYINT(1)` column declarations in `system_tables.json` | 156 | Junction is SSoT | | `PermissionCategory` type + 13 `PermCat*` constants | 19 | Zero callers | | Three `strings.EqualFold(objectName, "PermissionSet")` branches in generic describe/query/write engines | ~20 | Response-series P4/P6 replaced with schema-driven checks. The decorator owns the object-name knowledge; downstream engines treat every object uniformly. | Net: **~410 lines removed, ~200 lines added** across the refactor. ## What survives - `Permission` newtype (string-typed API names for type safety at call sites). - Engine-intrinsic permission constants (e.g. `PermRead`, `PermEdit`, `PermCreate`, `PermDelete`, `PermModifyAll`, `PermViewAll`) — these are engine operation labels used in generic CRUD gates, not catalog entries. - `SuperUserProfile` / `ProfileSystemAdmin` string identifiers (these are **standard system profile identifiers**, permitted under the "Standard Permission keys" carve-out in CLAUDE.md). ## How to add a permission 1. Append a row to `backend/internal/bootstrap/standard_permissions.json`. 2. Run `go run ./cmd/codegen` if you referenced it by constant in tests (optional — most call sites use the API-name string directly from a row). 3. Run `cmd/setup apply` against the target org (reseeds the catalog; upsert semantics mean re-running is safe). No Go edit. No migration. No column to add. ## How to grant a permission Admins edit rows: - **Default grants**: edit `defaultGrantedTo` in the pack row + reseed (changes the out-of-box grant — apply with care). - **Per-org grants**: create/edit `_System_PermissionSetPermission` rows via the PermissionSet admin UI (writes through the SObject Update path's `Permissions` virtual-field intercept). - **Runtime custom permissions**: insert `_System_CustomPermission` rows + junction rows to `Profile` or `PermissionSet` — both flow through the generic SObject CRUD surface. No code change is required for any of these. ## Regression guards In `backend/internal/features/records/persistence_bulk_insert_permissions_test.go`: - `TestBulkInsert_PermissionSet*` (3 tests) — confirm BulkInsert routes `Permissions` to the junction and invalidates the right cache scope. - `TestUpdate_PermissionSetVirtualFieldRoutesToJunction` — guards the P6 write-intercept placement with a post-P6-shaped schema (only `Id`+`Name` declared). Fails with `"Field 'PermissionsViewSetup' does not exist"` if the intercept moves back to after `ValidateSchemaCompliance`. - `TestRetrieve_PermissionSetProjectsJunctionGrantsIntoVirtualFields` — guards the P11 read projection. Fetches a PermissionSet whose schema has no `PermissionsX` physical fields and asserts the retrieved record has `PermissionsViewSetup=true` (from junction) and `PermissionsApiEnabled=false` (ungranted). - `TestUpdate_PermissionSetEmitsAuditAndRejectsUnknownPermission` — guards P12/B2 (audit emission) + W1 (orphan rejection). Confirms granting `ViewSetup` produces exactly one Setup Audit Trail entry keyed to the user; rejects `PermissionsFoobar` with `"unknown permission"` and zero audit entries. In `backend/internal/features/permissions/permissions_prereq_test.go`: - `TestValidatePermissionPrerequisites_RejectsMissingPrereq` — single- link prereq check against merged final state. - `TestValidatePermissionPrerequisites_TransitiveChain` — A→B→C chain evaluated in a single sweep. - `TestApplyPermissionChanges_RejectsUnknownAPIName` — uses a panic-on-write repo to prove the catalog-membership check runs before any `SetSystemPermission` call. In `backend/internal/bootstrap/standard_permissions_test.go`: - 7 contract tests (pack parses, rows coherent, APINames unique, requires-chain resolves, audiences known, SystemAdmin gets all, StandardUser gets only `CreateCustomizeReports`). ## Upgrade data-spec Operational cleanup on upgraded clusters is now a **three-stage legacy remediation chain** with a blocking audit between the non-destructive and destructive work (enforces R9 of `docs/lessons/2026-04-23-permission-ssot-cutover.md` — "never combine backfill and destructive operations in one migration"). The platform runtime does not contain legacy bitmap special cases; all cleanup stays in the runbooks below. All three stages discover the same per-cluster legacy column manifest via `INFORMATION_SCHEMA.COLUMNS`: ```sql WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = 'PermissionSet' AND COLUMN_NAME LIKE 'Permissions%' AND DATA_TYPE = 'tinyint' ``` Greenfield clusters do not need the remediation chain. If an operator runs the preflight anyway and it returns zero rows, the stage records a no-op; no SQL statement references a missing `Permissions%` column, so the data-spec applier's strict missing-column behavior remains intact. There is no 1054 exemption. 1. `data-specs/2026-04-23a-backfill-permissionset-bitmap-to-junction.md` — non-destructive `data-spec/v1`. For each manifest row, `INSERT IGNORE` adds every `(permSetID, PermissionApiName)` where the legacy bitmap column is `1`. Existing junction rows are preserved; admin revocations made through the junction SSoT are never overwritten. 2. `data-specs/2026-04-23b-audit-permissionset-bitmap-backfill.md` — read-only `data-spec/v1`. The only blocking gate is `bitmap=1 AND junction row missing`; `bitmap=1 AND IsEnabled=0` is an informational report because the junction is current truth. A second informational report lists catalog drift when a live legacy bitmap suffix is not an active `_System_Permission.ApiName`. 3. `data-specs/2026-04-23c-drop-permissionset-bitmap-columns.md` — manual `destructive-runbook/v1`, intentionally not executable by `data-spec-apply`. The operator verifies stage 1 and stage 2 status, checks that the stage 3 preflight manifest matches the recorded stage 2 manifest, obtains the documented CODEOWNER approval, then runs per-manifest `ALTER TABLE PermissionSet DROP COLUMN IF EXISTS ...` and post-verifies that no legacy `Permissions%` columns remain. ## Post-initial-refactor cutover fixes (P11–P13) After the P1–P17 series above landed, reviewer feedback surfaced four correctness gaps. Each fix tightens a specific side-channel in the write pipeline so virtuals travel through the same normalize → validate → extract path as physical boolean fields. ### P11 — side-channel gaps flagged in review 4162129569 - `ApplyPermissionChanges` reordered to `validate-all → diff → prereq-on-final`. A no-op PATCH (every key matches current grant) short-circuits cleanly; PS-004's unknown-permission rejection still fires for direct callers even on no-op values. - Update-path no-op: `currentGrants` is loaded under the SObject lock before the physical-vs-virtual change gate, and the gate now returns early when both the physical diff and the permission diff are empty (no `LastModifiedDate` bump, no `BeforeUpdate` events). The loaded grants are reused by `ApplyPermissionChanges` downstream — avoids a duplicate junction read inside the tx. - Catalog truth-of-pack: new Batch 1b sets `IsActive=0` on `_System_Permission` rows whose `ApiName` is not in the pack. Tombstone rather than delete so junction rows retain FK integrity; a permission returning to the pack flips back via Batch 1's `ON DUPLICATE KEY UPDATE ... IsActive=VALUES(IsActive)` (see P12). - Grants are additive-only seed: Batch 2 switched from UPSERT to `INSERT IGNORE` and the truth-of-pack DELETE on junction rows was removed entirely. `defaultGrantedTo` is now a true INITIAL seed — admin edits survive reseed. ### P12 — catalog-active consistency + bool pipeline tightening - `_System_Permission` becomes the runtime SSoT: both read paths (`getAllPermissionSetPermissions` and `getGroupSystemPermissions`) `INNER JOIN _System_Permission ON ApiName = PermissionApiName AND IsActive = 1`. Deactivated catalog rows no longer bleed their historical grants into `/permissions/effective`, `CheckSystemPermission`, folder gates, or flow gates. - `NormalizeSObject.default` case for booleans is strict: only the types `ValidateSchemaCompliance.validateType` accepts get coerced; everything else is left uncoerced so the downstream guard produces a clear error instead of a silent revoke. - `ExtractPermissionGrants` returns `(map, error)` and fails loud on non-bool values. Three write-path call sites wrap it and surface the error as a validation failure. Removes the "silent default to false" hazard entirely. ### P13 — boolean-coercion parity with physical fields - `NormalizeSObject`'s inline bool branch had only recognized literal `"true"` / `"false"`, so `PermissionsViewSetup: "1"` was rejected while an equivalent physical boolean field would pass via `strconv.ParseBool`. P13 aligns the coercion set with `ValidateSchemaCompliance.validateType` exactly: `bool`, `float64 ∈ {0, 1}`, any `ParseBool`-accepted string. Types validator does not accept (`int`, `int64`, `[]uint8`, maps, slices, unparseable strings) are left as-is for the downstream guard. - `TestExtractPermissionGrants_BooleanParityWithPhysical` locks the accept-set equivalence: 14 input forms that pass for physicals must pass for virtuals. ### Virtual-field contract (P1–P10 of the review-response series) Recorded here for completeness; the decorator infrastructure is what made P11–P13 possible: - `FieldMetadata.IsVirtual` / `VirtualKind` / `VirtualSourceAPIName` — the typed contract every engine honors. - `MetadataService.SchemaDecorator` interface + `RegisterSchemaDecorator` — invalidate-on-register; decorators run inside `buildCoreSnapshot` so the shared cached pointer stays immutable. - `models.CloneWithAdditionalFields` — collision dedupe via lowercase APIName match (`later-wins` in both slice and `FieldsMap`), fixing the upgrade-path describe duplication flagged in the post-P2 smoke test. - `models.LogicalFields` / `PersistableFields` / `FindField` / `VirtualFieldsByKind` — intent-named helpers so every consumer declares whether it wants the physical-SQL view or the logical view. Physical-SQL sites (`getStorageColumns`, the `visibleFields` builder in QueryService, `filterToLogicalFields`) now iterate `PersistableFields` so virtuals cannot reach an INSERT/SELECT column list. ## Commits (chronological) Original series (P1–P17 of the authoring-site collapse): ``` 50984e89 P0 + P1 substep 1: drift audit + 70-row permission pack 540b1716 P1: seeder reads from standard_permissions.json pack d96bc29f P2: ListSystemPermissions reads rows, 3 callers migrated off Go catalog 320d0143 P3: delete AllSystemPermissions() and PermissionDefinition struct bc01c870 P4/P5: drop bitmap write path + hardcoded 26-entry column map da194888 P6: drop 26 Permissions columns from PermissionSet + SObject write intercept 71adf6f8 P8: delete PermissionCategory enum and 13 unused PermCat* constants 395660cb P10: design-doc handoff (this file) 3b622b3a simplify: extract ExtractPermissionGrants helper; trim noise 69709cdd P11 (B1): read-path projection for Permissions on describe/retrieve/query 705ab402 P12 (B2+W1): ApplyPermissionChanges consolidates catalog + prereq + audit ba9994bb P13 (B3): seeder truth-of-pack — revoke stale default grants on re-seed 188b869c P14 (W3): prereq-chain + orphan-apiName regression tests 368e8b07 P15 (B4): data-spec to backfill junction + drop bitmap columns on upgrade c25c716d P16 (cleanup): delete dead grant helpers that bypass ApplyPermissionChanges ``` Post-review cutover tightening (P1–P13 of the response series, new numbering): ``` 2dfeb5b3 P1a: virtual-field contract — types, constants, helpers cf8c52d3 P2: schema decorator at cache-build time cb413697 P3: metadata-driven write-path extraction 5e385ee8 P4: read path consumes schema.Fields directly 3a882e25 P5: SQL-generating paths use PersistableFields 3c55539a P6: delete ad-hoc virtual-field machinery e6be6049 P7: ApplyPermissionChanges ordering — validate-all → diff → prereq b8a03087 P8: split bitmap cleanup into 3-stage data-spec ef01ba51 P9: close the single-write-gate invariant + fix stale docs 9fbf77f8 P10: dedupe colliding fields in CloneWithAdditionalFields (upgrade-path fix) 8ac116f1 P11: close virtual-field side-channel gaps flagged in review 4162129569 4ec041cb P12: catalog-active consistency + bool pipeline tightening 11d75922 P13: virtual-field boolean coercion parity with physical ``` P7 and P9 of the original series were verification-only (prereq reader already on `RequiresPermission` column after P2; CustomPermission parallel infra confirmed intact).