5 Commits

Author	SHA1	Message	Date
shankar0123	ddad647ee7	fix(auth/rbac): scope-aware ActorRole revoke (A-4) ... HIGH-10's UNIQUE (actor, role, scope_type, scope_id, tenant) uniqueness extension lets an operator grant the same role to the same actor at multiple scopes (e.g. r-operator on profile=p-acme AND profile=p-globex). But ActorRoleRepository.Revoke's WHERE clause omitted (scope_type, scope_id) — a single call deleted every variant. Selective revoke was unrepresentable; operators had to drop all and re-grant N-1, opening a race window where the actor's access was briefly different. Closure across all layers (handler → service → repo → MCP → GUI client), preserving the legacy "revoke all variants" contract for unmodified callers: internal/repository/auth.go - New ActorRoleRevokeOptions struct. Zero value = legacy semantic; non-empty ScopeType narrows to one variant. - New ErrActorRoleNotFound sentinel for scoped no-match (HTTP 404). internal/repository/postgres/auth.go - Revoke signature extended with opts. Empty opts.ScopeType uses the legacy SQL (no scope WHERE), zero-row delete = no error. - Non-empty narrows with `scope_type = $5 AND scope_id IS NOT DISTINCT FROM $6` — the IS-NOT-DISTINCT-FROM is load-bearing, vanilla `=` would silently miss the (global, NULL) case because NULL ≠ NULL in standard SQL. - Selective revoke with zero matching rows returns ErrActorRoleNotFound; operators get feedback on typos. internal/service/auth/actor_role_service.go - Revoke takes opts. Audit row's details map records the scope so SIEMs can distinguish wide-vs-selective revokes: `scope: "all_variants"` for the legacy path, or `scope_type` + `scope_id` for selective. Privilege check (auth.role.assign) and reserved-actor guard unchanged. internal/api/handler/auth.go - RevokeRoleFromKey parses optional `?scope_type=` / `?scope_id=` query params via new parseRevokeScope helper. - Validation mirrors AssignRoleToKey: scope_id forbidden with scope_type=global, required with profile/issuer, invalid scope_type → 400. scope_id without scope_type also → 400. - writeAuthError maps ErrActorRoleNotFound to 404. internal/mcp/tools_auth.go + types.go - AuthRevokeKeyRoleInput gains optional ScopeType + ScopeID with jsonschema descriptions explaining the dual-mode contract. - Tool call site appends URL-encoded query params when ScopeType is set; legacy callers (no scope_type) emit the bare DELETE path unchanged. web/src/api/client.ts - authRevokeKeyRole signature: optional 3rd argument `{ scope_type?, scope_id? }`. Pre-A-4 call sites (no opts arg) keep firing the bare DELETE — fully backward compatible. The GUI KeysPage's per-row revoke button (still one row per role, pre-Fix-12) continues to use the legacy shape; future GUI work can pass scope params for per-variant rows. docs/operator/rbac.md - New "Revoke: legacy 'all variants' vs scope-selective" subsection under "From the HTTP API" with curl examples for both modes plus the audit-row payload shape that lets SOC/SIEM tell them apart. Regression coverage: Repository (testcontainers, skipped under -short — 6 tests in internal/repository/postgres/auth_revoke_scope_test.go): TestRevokeActorRole_NoOpts_RemovesAllVariants TestRevokeActorRole_WithScope_RemovesOnlyMatching TestRevokeActorRole_WithGlobalScope_RemovesOnlyGlobal — pins the IS-NOT-DISTINCT-FROM branch (global, NULL) TestRevokeActorRole_NoMatch_ReturnsNotFound — pins the new sentinel TestRevokeActorRole_NoOpts_NoMatch_IsNoOp — pins the legacy idempotence contract TestRevokeActorRole_IssuerScope_RemovesOnlyMatching — pin the issuer-scope half (profile + issuer are symmetric scope types) Handler (7 new tests in auth_test.go): TestAuthHandler_RevokeRoleFromKey — extended to assert no scope filter is forwarded when query string is empty (legacy behaviour) TestAuthHandler_RevokeRoleFromKey_A4_ScopedProfile TestAuthHandler_RevokeRoleFromKey_A4_ScopedGlobal TestAuthHandler_RevokeRoleFromKey_A4_RejectsScopeIDWithGlobal TestAuthHandler_RevokeRoleFromKey_A4_RejectsMissingScopeID TestAuthHandler_RevokeRoleFromKey_A4_RejectsScopeIDWithoutScopeType TestAuthHandler_RevokeRoleFromKey_A4_RejectsInvalidScopeType TestAuthHandler_RevokeRoleFromKey_A4_ScopedNotFoundReturns404 MCP (2 new table rows in tools_per_tool_test.go): Scoped revoke with scope_type=profile + scope_id=p-acme → `?scope_type=profile&scope_id=p-acme` Scoped revoke with scope_type=global (no scope_id) → `?scope_type=global` Service-layer test plumbing (service_test.go) updated for new opts arg: 4 existing call sites pass repository.ActorRoleRevokeOptions{} to keep their pre-A-4 semantics; the fakeActorRoleRepo.Revoke implementation now mirrors the postgres scope-aware behaviour (legacy zero-value vs scoped narrowing + ErrActorRoleNotFound on no-match). Verify gate green: gofmt clean, go vet clean, go test -short across repository/postgres, service/auth, api/handler, and mcp. The pre-existing KeysPage.test.tsx failure observed on the baseline commit (reproduced via `git stash` earlier in Fix 03) is unrelated; my client.ts change adds an optional third argument and is fully backward-compatible. Spec at cowork/auth-bundles-fixes-2026-05-11/04-high-actor-role-revoke-scope.md. Audit doc updated: new row A-4 (2026-05-11) CLOSED appended to the status table at the bottom of cowork/auth-bundles-audit-2026-05-10.md. Operator-visible advisory in CHANGELOG.md v2.1.0 release notes under Security (non-BREAKING — legacy callers are unchanged). Depends on Fix 01 (the scope-aware EffectivePermissions read path on branch fix/audit-2026-05-11/crit-actor-role-scope-reads). This fix makes the inverse op selectively reversible; without Fix 01 the read side would mis-evaluate scoped grants anyway, making selective revoke moot at runtime.	2026-05-11 10:50:34 +00:00
shankar0123	77860fbcc3	harden(auth): LOW + Nit batch — bootstrap audit, crypto/rand, XFF trust, CSRF check, protocol-prefix unify (Batch 1) ... Audit 2026-05-10 — close 8 LOWs + 2 Nits in-bundle. Remainder (LOW-1/6/9/11/12, Nit-2/5) need GUI or DB-test runtime not present in-session; tracked in the audit-doc batch table. LOW-2: bootstrap.ValidateAndMint now emits 'bootstrap.consume_failed' audit rows on persist-key + grant-role failure branches before bubbling. Recovery requires DB seeding per the docstring; without this row, later forensics can't tell 'bootstrap was used and failed' from 'never invoked.' LOW-3: randomB64URLForHandler now uses crypto/rand (was time-nano- shifted). Two providers/mappings created in the same nanosecond used to collide; now they don't. Time-nano fallback retained for the unlikely crypto/rand-broken path. LOW-4: breakglass.verifyDummy uses s.readRand(salt) for the dummy Argon2id verify. Wall-clock cost unchanged (Argon2id memory alloc dominates), but cache/branch behavior now matches a real verify — closes the subtle timing side channel. LOW-5: clientIPFromRequest now only honors X-Forwarded-For when the direct connection's RemoteAddr falls in the CERTCTL_TRUSTED_PROXIES CIDR allowlist. Default-deny: empty list means XFF is ignored. SetTrustedProxies wired in cmd/server/main.go from cfg.Auth.TrustedProxies. LOW-7: internal/auth/protocol_endpoints.go::ProtocolEndpointPrefixes now carries /scep-mtls + /.well-known/est-mtls (previously only in router.AuthExemptDispatchPrefixes; the two lists had drifted). The canonical-prefix coverage test in Phase 12 still pins the set. LOW-8: docs/operator/rbac.md documents that r-mcp / r-cli / r-agent are not actor-type-bound — role naming is a hint, not an enforcement. Operators wanting hard binding must apply periodic audit queries. Native binding is on the v2 roadmap. LOW-10: Session.Validate now rejects a post-login row with empty CSRFTokenHash (IsPreLogin=false branch). validSession test fixture updated with a valid 64-hex CSRF hash. Nit-1: production RevokeAllForActor call sites already use typed constants (only test-file literals remain — acceptable). Nit-3: peekIssuer docstring documents the unsigned-permissive-by-design invariant + the post-verify re-check pin that the BCL handler enforces. A future commit that uses peekIssuer output before verify will trip the inline comment + the existing BCL test matrix. Status table updated in cowork/auth-bundles-audit-2026-05-10.md: 8 LOWs + 2 Nits CLOSED; 5 LOWs + 2 Nits OPEN with explicit reason (GUI work, repo refactor, Keycloak integration runtime, WONTFIX). Refs: cowork/auth-bundles-audit-2026-05-10.md LOW-2/3/4/5/7/8/10 cowork/auth-bundles-audit-2026-05-10.md Nit-1/3	2026-05-10 22:26:12 +00:00
shankar0123	457962f21a	fix(auth): apply rbacGate to every state-changing + read handler (CRIT-1 closure) ... Closes the wire-layer authorization gap surfaced by the 2026-05-10 audit (CRIT-1). Before this commit only ~24 of ~140 routes carried rbacGate enforcement — all of them admin-only fine-grained perms (auth.session.*, auth.oidc.*, auth.breakglass.admin, cert.bulk_revoke, crl.admin, scep.admin, est.admin, ca.hierarchy.manage). Every catalogued legacy-CRUD perm (cert.read/issue/revoke/delete, profile.edit/delete, issuer.edit/delete, target.*, agent.*, plus role-mgmt verbs) was declared in internal/domain/auth/validate.go but never wired at the router. A r-viewer Bearer was essentially r-admin minus five verbs at the wire layer (CWE-862). This commit: - Adds rbacGateScoped(checker, perm, scopeType, scopeFn, h) helper to internal/api/router/router.go for path-bound scope resolution. Per-profile and per-issuer grants (Decision 2) now reach the wire layer. - Wraps every state-changing route AND every read endpoint in router.go with rbacGate (global) or rbacGateScoped (path-bound). The auth-management routes (POST /api/v1/auth/roles, etc.) gain router-level enforcement in addition to the existing service-layer Authorizer check — defense in depth (HIGH-9 of the same audit collapses into this closure). - Auth-exempt surfaces stay un-gated by design: login, callback, BCL, logout, breakglass-login, bootstrap, health, auth-info, version. Allowlist is documented in TestRouterRBACGateCoverage. - Extends internal/domain/auth/validate.go CanonicalPermissions with 30 new perms across 12 namespaces: cert.edit; job.read, job.cancel; approval.read, approval.approve, approval.reject; policy.read/edit/delete; team.read/edit/delete; owner.read/edit/delete; notification.read/edit; discovery.read/run/claim; network_scan.read/edit/run; healthcheck.read/edit/delete/acknowledge; digest.read, digest.send; verification.read, verification.run; stats.read; metrics.read. - Updates DefaultRoles for r-admin / r-operator / r-viewer / r-mcp / r-cli / r-agent. r-auditor gets NOTHING new — the auditor pin (TestAuditorRoleHoldsExactlyAuditReadAndExport) stays invariant. - Migration 000039_audit_crit1_perms seeds the new perm rows + role grants per the updated DefaultRoles map. Idempotent ON CONFLICT DO NOTHING. Reverse migration removes role_permissions before permissions (ON DELETE RESTRICT on the FK). - AST-level CI guard TestRouterRBACGateCoverage in internal/api/router/router_rbac_coverage_test.go walks router.go and asserts every state-changing + read route is wrapped (or in the documented allowlist). Adding a new ungated route fails CI. - Updates docs/operator/rbac.md permission-catalogue table with the new namespaces + footer link to the AST CI guard. - Updates certctl/CHANGELOG.md v2.1.0 section with the closure narrative. Audit doc cowork/auth-bundles-audit-2026-05-10.md CRIT-1 row annotated CLOSED 2026-05-10. Bundle's exit-gate spec lives at cowork/auth-bundles-fixes-2026-05-10/01-crit-1-rbac-gates.md. CRIT-2 / CRIT-3 / CRIT-4 / CRIT-5 of the same audit remain open and continue to block the v2.1.0 tag. Verification gate green: - gofmt -d (no diff after gofmt -w on the touched files) - go vet ./... - go test -short -count=1 ./... (all packages pass including auditor pin) - go build ./... HIGH-9 of the audit closes via this commit's router-layer rbacGate on POST /api/v1/auth/keys/{id}/roles + DELETE /api/v1/auth/keys/{id}/roles/{role_id} (defense-in-depth on top of the existing service-layer privilege check). Refs: cowork/auth-bundles-audit-2026-05-10.md CRIT-1 HIGH-9	2026-05-10 19:58:26 +00:00
shankar0123	f4cdce764c	auth-bundle-1 Phase 13 follow-up: em-dash sweep + broken-link fix ... Self-audit on ba68f9a flagged the prompt's 'zero em dashes' discipline rule. The four new Phase 13 docs and the v2.1.0 CHANGELOG section had 97 em-dash hits between them; this commit sweeps them all to ASCII hyphens. Counts before -> after: docs/operator/rbac.md 28 -> 0 docs/operator/auth-threat-model.md 36 -> 0 docs/migration/api-keys-to-rbac.md 16 -> 0 docs/operator/security.md 8 -> 0 docs/reference/profiles.md 3 -> 0 CHANGELOG.md 6 -> 0 Mechanical: ' - ' (spaced em dash) and bare em-dash both replaced with spaced ASCII hyphen, then double-spaces collapsed. Markdown list bullets ('^- ', '^ - ', '^ - ') verified intact across all six files. Internal-link sweep also re-run. Also fixes a pre-existing broken link the audit caught: docs/operator/security.md:70 referenced '../internal/crypto/encryption.go' which is a 1-level-up jump from docs/operator/, not the 2-level-up jump it actually needs ('../../internal/crypto/encryption.go'). Pre-Bundle-1 link rot; fixed in lockstep so the merge gate's docs validation passes cleanly. Final state across the Phase-13 docs + CHANGELOG: - 0 em dashes - 0 broken internal links - Last-reviewed: 2026-05-09 header on every new doc Bundle 1 documentation is now ready for the operator-side merge gate review.	2026-05-10 00:15:30 +00:00
shankar0123	ba68f9a994	auth-bundle-1 Phase 13: docs (rbac.md + threat model + migration guide + security.md update) ... Closes the last Phase before the Bundle 1 Exit gate. Operators now have authoritative reference + threat model + migration guide covering every behavior change Bundles 0-12 introduced. # New docs * docs/operator/rbac.md (340 lines) — operator how-to: - Mental model (actors / roles / permissions / scopes) - 7 default roles seeded by migration 000029 + the 5 admin-only fine-grained perms seeded by 000030 - Permission catalogue table by namespace - Scope semantics (global beats specific) + the Bundle-2 deferral on scope_id FK enforcement - Granting / revoking access from GUI + CLI + HTTP API + MCP - The auditor pattern (audit-only, no resource read) - Day-0 bootstrap flow (CERTCTL_BOOTSTRAP_TOKEN → curl → HTTP 410 thereafter) - Demo-mode (CERTCTL_AUTH_TYPE=none) caveat for production * docs/operator/auth-threat-model.md (180 lines) — what the controls defend against: - 5 threat actors (external, wrong-role, compromised key, insider operator, compromised auditor) - Per-defense walk-through (API-key auth, RBAC, bootstrap, approval workflow + Phase 9 closure, audit trail, protocol-endpoint allowlist) - 9 explicit deferrals (OIDC, sessions, local accounts, JIT elevation, MFA, etc.) — Bundle 2 / future scope - Compliance mapping (SOC 2 CC6.1/CC6.3, HIPAA §164.312(b), NIST SSDF PO.5.2, FedRAMP AU-9, PCI-DSS §10) - 5 operator-runnable sanity checks (e.g., 'SELECT FROM audit_events WHERE actor=system-bypass' MUST return 0 in production) * docs/migration/api-keys-to-rbac.md (200 lines) — v2.0.x → v2.1.0 upgrade flow: - The SECURITY: AUDIT YOUR API KEYS callout - Migration list (000029-000033) + what each does - 4-mode scope-down flow (interactive / non-interactive JSON / --suggest / --suggest --apply) - What changes for code that called auth.IsAdmin - Helm-specific upgrade flow with example post-upgrade Job - Docker Compose upgrade flow + the 5 examples folders that ride demo mode unchanged - Verification queries + rollback flow # Updated docs * docs/operator/security.md — Last-reviewed bumped to 2026-05-09; existing Authentication-surface section extended to call out the Bundle 1 RBAC primitive, day-0 bootstrap path, and approval-bypass closure with cross-references to the new docs. * docs/reference/profiles.md — Last-reviewed header formatting fixed (added the > blockquote prefix used consistently across the docs tree). # docs/README.md navigation * Operator section gains 2 new rows (RBAC + auth-threat-model) and Approval-workflow row updated to mention Phase 9 closure. * Reference section gains the Profiles row. * Migration section gains the api-keys-to-rbac row with the AUDIT YOUR API KEYS callout in the link description. # CHANGELOG.md v2.1.0 section refreshed The Phase 7 commit landed the SECURITY: AUDIT YOUR API KEYS callout. This commit appends the missing Phase 9-12 highlights: - Approval-bypass closure (profile-edit gate + flip-flop loophole + ErrApproveBySameActor invariant) - GUI: Roles / API Keys / Auth Settings / Approvals queue - 12 new MCP RBAC tools - Coverage gates on internal/auth + internal/service/auth - Protocol-endpoint allowlist pinned at 3 layers Trailing cross-reference block now points at all 4 new docs. # Verifications * Every internal link in the 4 new/modified docs validated by shell sweep (find broken links → 0 hits). * Every new doc carries 'Last reviewed: 2026-05-09' header with the > blockquote prefix matching the docs-tree convention. * go vet ./... clean. * staticcheck across every Bundle-1-touched Go package clean. * gofmt -l clean repo-wide. * go test -short -count=1 green across internal/auth (incl. bootstrap), internal/api/handler, internal/api/router, internal/cli, internal/service (incl. auth), internal/domain/auth, internal/mcp, cmd/cli (cmd/server has 1 environmental failure on the sandbox virtiofs-tmp: TestPreflightSCEPRACertKey_KeyWorldReadable_Refuses depends on tmpfs file-mode semantics that virtiofs propagates differently — pre-existing, unrelated to Bundle 1). * Frontend: 19 Vitest tests across src/pages/auth/ + AuditPage all pass; tsc --noEmit clean.	2026-05-10 00:10:15 +00:00