mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 14:11:31 +00:00
fix(auth/rbac): close HIGH-10 lying field — EffectivePermissions reads actor-role scope (A-1)
Audit 2026-05-11 A-1 closure. Spec at
cowork/auth-bundles-fixes-2026-05-11/01-crit-actor-role-scope-reads.md.
WHAT.
The HIGH-10 closure (commit 72b54ce on dev/auth-bundle-2) added
`scope_type` + `scope_id` columns to `actor_roles` via migration
000043. The handler accepted them on POST /api/v1/auth/keys/{id}/roles.
The repo Grant INSERTed them. The uniqueness tuple was extended to
include them. The GUI exposed them as form inputs.
But the load-bearing `EffectivePermissions` SQL at
internal/repository/postgres/auth.go:470 never read them. The query
only JOINed against rp.scope_type/rp.scope_id (role-permission
scope) and ignored ar.scope_type/ar.scope_id (actor-role scope).
Operator-visible failure: granting Alice r-operator scoped to
profile=p-prod silently elevated her to r-operator GLOBALLY at
authorization time. The Authorizer's matcher correctly handled
whatever EffectivePermissions returned, but EffectivePermissions
returned the rp.scope (typically global), not the ar.scope
narrowing.
This is the canonical CRIT-5 lying-field shape — a security
control claimed, persisted across 4 layers, with unit tests at
each isolated layer, but the load-bearing wire severed mid-flight.
CLAUDE.md's 'Always take the complete path' rule was violated by
the original HIGH-10 closure.
Additionally, `scanActorRoles` failed to read the new columns
even when present, so every GET-side path (ListByActor /
ListByRole) returned ActorRole with zero-value scope fields — the
GUI / MCP couldn't show operators what they had configured.
HOW.
internal/repository/postgres/auth.go:
- EffectivePermissions SQL extended to intersect ar.scope with
rp.scope via a CASE-in-subquery. The effective scope is the
NARROWER of the two; disjoint tuples and scope-type mismatches
drop the row entirely. WHERE filter on effective_scope_type
IS NOT NULL excludes dropped rows.
Match matrix (encoded by the CASE):
ar.scope rp.scope effective_scope
───────── ───────── ──────────────────
global global global / NULL
global profile=X profile=X (rp narrows)
profile=X global profile=X (ar narrows)
profile=X profile=X profile=X (both agree)
profile=X profile=Y ROW DROPPED (disjoint)
profile=X issuer=* ROW DROPPED (type mismatch)
- ListByActor + ListByRole SELECTs extended with scope_type +
scope_id columns so the read-side surfaces what was persisted.
- scanActorRoles reads the new columns into ActorRole.ScopeType
+ ScopeID via the existing sql.NullString + ScopeType cast
pattern (mirrors RolePermission scan).
internal/repository/postgres/auth_scope_test.go (NEW):
Testcontainer-backed regression matrix. 8 cases:
1. ActorRoleGlobal_RolePermGlobal — trivial happy path.
2. ActorRoleGlobal_RolePermProfile — rp narrows.
3. ActorRoleProfile_RolePermGlobal_A1Closure — **load-bearing**
post-fix case: profile-scoped grant narrows to profile.
4. BothScopedSameTuple_Matches — exact-match collapse.
5. BothScopedDifferentIDs_RowDropped — disjoint scopes produce
no effective permission.
6. ScopeTypeMismatch_RowDropped — profile vs issuer mismatch.
7. ExpiredGrant_Excluded — pre-fix behavior preserved.
8. ListByActor_ReturnsScopeColumns — read-side surface check.
Tests skip in -short mode (testcontainers-backed; require Docker
on operator workstation).
internal/service/auth/service_test.go:
TestAuthorizer_ActorRoleProfileScope_OnlyNarrowedScopeAuthorizes_A1
— unit-level pin (sandbox-runnable, no Docker). Simulates the
post-A-1 SQL emission (narrowed effective row at
profile=p-prod) and asserts CheckPermission authorizes only
matching profile, rejects other profiles AND rejects global.
Existing matcher code is unchanged; this proves the integration
point.
CHANGELOG.md:
Operator advisory in the new 'Security (BREAKING — silent-elevation
closure)' section. Pre-existing scope-bound grants take effect on
upgrade; operators audit `actor_roles WHERE scope_type != 'global'`
to confirm intent.
cowork/auth-bundles-audit-2026-05-10.md:
HIGH-10 row gets an A-1 follow-on CLOSED 2026-05-11 annotation
describing the regression + closure.
VERIFY.
- gofmt -l <changed files> (no diff)
- go vet ./internal/repository/postgres/... ./internal/service/auth/...
./internal/api/handler/... ./internal/auth/... ./cmd/server/... PASS
- go test -short -count=1 ./internal/service/auth/...
./internal/repository/postgres/... ./internal/api/handler/... PASS
- The testcontainer-backed regression matrix runs on operator
workstation via 'go test -count=1 ./internal/repository/postgres/...'
(skip in -short).
Refs: cowork/auth-bundles-audit-2026-05-10.md HIGH-10 (A-1 follow-on)
cowork/auth-bundles-fixes-2026-05-11/01-crit-actor-role-scope-reads.md
CLAUDE.md 'Always take the complete path' rule
This commit is contained in:
@@ -2,6 +2,26 @@
|
||||
|
||||
## Unreleased
|
||||
|
||||
### Security (BREAKING — silent-elevation closure)
|
||||
|
||||
- **HIGH-10 actor-role scope is now enforced (Audit 2026-05-11 A-1).**
|
||||
Pre-fix, `actor_roles.scope_type` / `scope_id` (added in migration 000043
|
||||
by the HIGH-10 closure) were persisted by Grant + accepted on the handler
|
||||
body + surfaced through the GUI/MCP — but the load-bearing
|
||||
`EffectivePermissions` SQL never read them. A profile-scoped grant
|
||||
silently elevated to global at authorization time. Canonical CRIT-5
|
||||
lying-field shape, replicated. **The post-fix authorization narrows
|
||||
correctly**: every existing `actor_roles` row with `scope_type != 'global'`
|
||||
now takes effect.
|
||||
|
||||
> **Operator advisory:** if you used the HIGH-10 scope-bound role-grant
|
||||
> API between commit `551812b` and the v2.1.0 tag (the column was
|
||||
> populated but ignored), the grants were silently global. After
|
||||
> upgrading, audit `SELECT actor_id, role_id, scope_type, scope_id FROM
|
||||
> actor_roles WHERE scope_type != 'global'` and confirm the narrowing
|
||||
> reflects intent. If an actor was granted a scoped role but expected
|
||||
> global behavior, re-grant with `scope_type=global`.
|
||||
|
||||
### Security (BREAKING)
|
||||
|
||||
- **`__Host-` cookie prefix on all three auth cookies (Audit 2026-05-10 MED-14).**
|
||||
|
||||
Reference in New Issue
Block a user