Commit Graph
100 Commits
Author SHA1 Message Date
shankar0123 9143003e95 auth-bundle-2 Phase 8: GUI auth surface (OIDC providers + group mappings + sessions + LoginPage IdP buttons + AuthState refactor + logout wiring)
Closes Phase 8 of cowork/auth-bundle-2-prompt.md. Every Bundle 2 endpoint
now has a permission-gated, data-testid-instrumented React surface.

Frontend changes
================

api/client.ts (Category H — AuthState refactor):
* fetchJSON now sends `credentials: 'include'` on every request so the
  HttpOnly session cookie + the JS-readable CSRF cookie ride along with
  Bearer-mode requests transparently. Mode is determined per call by
  what cookies are present, NOT by a state-machine — the same client
  works for Bearer-only deploys, session-only deploys, and the mixed
  upgrade path described in cowork/auth-bundles-index.md Category H.
* readCSRFCookie() + isStateChangingMethod() helpers auto-attach
  `X-CSRF-Token` to POST/PUT/PATCH/DELETE when the CSRF cookie exists.
  Bearer-only callers ride through unchanged (no CSRF cookie → no
  header → backend's CSRF middleware skips).
* AuthInfoResponse extended with optional `oidc_providers?:
  AuthInfoOIDCProvider[]` matching the Phase 6 server extension.
* New API helpers (1:1 with Phase 5 / 7.5 endpoints):
  - listOIDCProviders / createOIDCProvider / updateOIDCProvider /
    deleteOIDCProvider / refreshOIDCProvider
  - listGroupMappings / addGroupMapping / removeGroupMapping
  - listSessions(actorID?, actorType?) / revokeSession / logout
  - breakglassLogin / breakglassSetPassword / breakglassUnlock /
    breakglassRemove
  Permission gates fire server-side; the GUI predicates are UX only.

pages/auth/OIDCProvidersPage.tsx (NEW):
* Lists configured OIDC providers, gated on `auth.oidc.list`.
* Empty state + error state + loading state.
* Embedded Configure-Provider modal with form fields for name,
  issuer_url, client_id, client_secret, redirect_uri,
  groups_claim_path/format, fetch_userinfo, scopes. Modal hidden
  unless caller has `auth.oidc.create`.
* Unsaved-changes confirmation on cancel.

pages/auth/OIDCProviderDetailPage.tsx (NEW):
* Provider config dl + edit/delete/refresh action buttons.
* Edit and refresh require `auth.oidc.edit`. Delete requires
  `auth.oidc.delete`.
* Type-confirm-name delete dialog. Surfaces server's 409 Conflict
  ("ErrOIDCProviderInUse") inline so the operator knows to revoke
  the provider's active sessions first.
* Refresh discovery cache button → POST .../refresh → server re-runs
  RefreshKeys with the IdP-downgrade-attack defense from Phase 3.
* Group→role mappings link.

pages/auth/GroupMappingsPage.tsx (NEW):
* Per-provider group-claim → role-id mapping CRUD.
* Empty state explains the fail-closed semantics from Phase 3
  (no mappings ⇒ no users authenticate via this provider).
* Inline add form (group_name input + role_id select populated from
  `authListRoles`); add/remove gated on `auth.oidc.edit`.

pages/auth/SessionsPage.tsx (NEW):
* Default "My sessions" view available to anyone holding
  `auth.session.list`.
* "All actors (admin)" toggle exposed only when caller holds
  `auth.session.list.all`; renders an actor_id filter input that
  threads ?actor_id= through the GET.
* Self-pill marker on the caller's own rows.
* Revoke button is shown when (a) the row is the caller's own session
  (handler-side own-bypass) OR (b) caller holds `auth.session.revoke`.
* Confirms via window.confirm; surfaces revocation errors inline.

pages/LoginPage.tsx (MODIFIED):
* Fetches /v1/auth/info on mount; if `oidc_providers[]` is non-empty,
  renders one "Sign in with X" button per provider linking to the
  provider's `login_url` (the server-side handler in Phase 5 builds
  this URL with state + nonce + PKCE verifier sealed in the pre-login
  cookie; the GUI never touches those values).
* The API-key form remains as a fallback for Bearer-mode deploys and
  the Phase 7.5 break-glass path.
* All interactive elements carry data-testid:
  login-oidc-providers / login-oidc-button-{id} / login-api-key-form /
  login-api-key-input / login-api-key-submit.

components/AuthProvider.tsx (MODIFIED):
* logout() now also fires POST /auth/logout via the api/client helper
  before clearing local state. The endpoint is auth-exempt; the
  catch-and-swallow keeps the local logout flow working even if the
  cookie is already invalid (idempotent server-side as well).

components/Layout.tsx (MODIFIED):
* Two new nav entries under the Auth section: "OIDC Providers" + "Sessions".

main.tsx (MODIFIED):
* Four new routes:
  - /auth/oidc/providers
  - /auth/oidc/providers/:id
  - /auth/oidc/providers/:id/mappings
  - /auth/sessions

Vitest coverage
===============

Five new test files, 28 new test cases. Pattern matches Bundle 1
Phase 10's Vitest scaffold (vi.mock api/client, render with
QueryClient + MemoryRouter, authMe-driven permission shaping,
data-testid selectors).

* OIDCProvidersPage.test.tsx (5 tests): ErrorState w/o auth.oidc.list,
  empty state, list + create button render, hide-create-button
  without auth.oidc.create, submit-creates-via-API.
* OIDCProviderDetailPage.test.tsx (5 tests): ErrorState w/o list,
  full-perms render, hide edit/refresh/delete with only list,
  refresh button calls API, delete confirm-button stays disabled
  until typed text matches provider name.
* GroupMappingsPage.test.tsx (5 tests): ErrorState w/o list, empty
  fail-closed warning, mapping rows render, hide-form without
  auth.oidc.edit, submit-add-form-calls-API.
* SessionsPage.test.tsx (6 tests): ErrorState w/o list, own sessions
  + self-pill, hide All-actors toggle without list.all, show
  toggle with list.all, hide revoke on other-actor sessions without
  auth.session.revoke, click-revoke calls API after window.confirm.
* LoginPage.test.tsx (extended +2 tests): renders OIDC buttons when
  /auth/info reports providers; omits the OIDC block when none.

Verification
============

* `npx tsc --noEmit` — 0 errors.
* Vitest run across api/components/hooks/utils/auth/pages = 475 tests,
  all green.
* `npm run build` — green (980 KB bundle, no surprises vs Phase 7).
* No backend (Go) changes in this commit; Phase 5-7.5 surfaces
  consumed unchanged.

Not in this commit (deferred)
=============================

* "Test login flow" button on the provider detail page (prompt §Phase 8
  optional row). Requires a server-side test=true flag on the OIDC
  login handler — out of scope for the GUI commit.
* `web/src/__tests__/e2e/` Keycloak-via-testcontainers harness for the
  15 comprehensive flow checks. Tracked under Phase 10 of
  cowork/auth-bundle-2-prompt.md.
2026-05-10 07:23:41 +00:00
shankar0123 1d01c87663 auth-bundle-2 Phase 7 + Phase 7.5: OIDC first-admin bootstrap +
break-glass admin (Argon2id, lockout, default-OFF, surface-invisibility)

Phase 7 — OIDC first-admin bootstrap (Decision 3):

  - Optional AdminBootstrapHook closure on *oidc.Service. When wired,
    HandleCallback consults the hook AFTER group resolution + user
    upsert and BEFORE the empty-mapping fail-closed check. Hook
    receives (providerID, groups, userID); returns grantAdmin=true
    when the user matches CERTCTL_BOOTSTRAP_ADMIN_GROUPS AND no
    admin exists yet in the tenant.
  - cmd/server/main.go wires the hook as a closure that:
      * Filters by CERTCTL_BOOTSTRAP_OIDC_PROVIDER_ID (if configured).
      * Probes AdminExists via authActorRoleRepo (admin-already-exists
        silently returns false; bootstrap mode is one-shot per tenant).
      * Walks group intersection.
      * On match: grants r-admin via authActorRoleRepo.Grant + emits
        the bootstrap.oidc_first_admin audit row with
        event_category=auth + INFO log.
  - Coexists with the Bundle 1 env-var-token bootstrap. Both paths
    can be configured; first match wins (admin-existence probe
    short-circuits the second).
  - HandleCallback's empty-mapping fail-closed check moved AFTER the
    hook so a fresh deployment with zero group_role_mappings can
    still mint the first admin.
  - 5 tests in service_test.go: hook grants admin on match, hook
    returns false preserves empty-mapping fail-closed, admin-already-
    exists silently falls through to normal mapping, hook-error wraps
    + bubbles, idempotent when admin is already in the mapped role set.

Phase 7.5 — Break-glass admin (Decision 4, default-OFF):

Migration 000038 ships:

  - breakglass_credentials table — at-most-one-credential-per-actor
    (UNIQUE(actor_id)), Argon2id PHC-format password_hash, lockout
    state machine (failure_count, locked_until, last_failure_at).
    FK CASCADE on users(id) so deleting a user atomically removes
    their credential.
  - Two new permissions seeded into r-admin only:
      auth.breakglass.admin — set/rotate/unlock/remove credentials.
      auth.breakglass.login — actor uses break-glass to log in.
    CanonicalPermissions extended in lockstep.

internal/auth/breakglass/service.go (~580 LOC):

  - Service.Enabled() reflects CERTCTL_BREAKGLASS_ENABLED.
  - SetPassword: Argon2id with OWASP 2024 params (m=64MiB, t=3, p=4,
    salt=16 random bytes, output=32 bytes); per-password random salt;
    PHC-format hash output. Min 12 / max 256 byte input.
  - Authenticate: constant-time-compare via subtle.ConstantTimeCompare
    on every code path. Identical 401 + identical timing across the
    wrong-password / locked-account / non-existent-actor paths so an
    attacker cannot probe whether a given actor has break-glass
    configured. Non-existent-actor + locked-account paths run a
    verifyDummy() Argon2id pass for timing parity. Lockout state
    machine: failure_count++ on every wrong attempt; threshold (default
    5) trips locked_until = NOW() + duration (default 15m). Successful
    Authenticate resets the counter. Reset-window: failures aged out
    after CERTCTL_BREAKGLASS_LOCKOUT_RESET_INTERVAL (default 1h)
    auto-reset on next attempt.
  - Unlock + RemoveCredential: admin-only (auth.breakglass.admin
    gated at the router via rbacGate). Audit rows on every operation.
  - All public methods refuse to act when Enabled()==false (returns
    ErrDisabled; the handler maps to HTTP 404 — surface invisibility).

internal/repository/postgres/breakglass.go ships the 5-method
postgres impl with atomic single-statement IncrementFailure (so
concurrent racing wrong-password attempts can't observe an
intermediate state and slip past the threshold) and idempotent
ResetFailureCount.

internal/api/handler/auth_breakglass.go ships the 4-endpoint HTTP
surface:

  - POST /auth/breakglass/login (auth-exempt; 5/min rate-limited per
    source IP via the existing rate limiter; returns 404 when
    disabled). On success sets the post-login session cookie + CSRF
    cookie via SessionService.Create + 204. On any failure:
    uniform 401 + identical timing (the service has already audited
    the specific failure category).
  - POST /api/v1/auth/breakglass/credentials (auth.breakglass.admin)
  - POST /api/v1/auth/breakglass/credentials/{actor_id}/unlock
    (auth.breakglass.admin)
  - DELETE /api/v1/auth/breakglass/credentials/{actor_id}
    (auth.breakglass.admin)

Admin endpoints share the surface-invisibility property: when
CERTCTL_BREAKGLASS_ENABLED=false, every admin endpoint also returns
404 (not 403) so probing via the admin surface gets the same signal
as probing the login endpoint.

Tests (internal/auth/breakglass/service_test.go):

All 8 Phase 7.5 spec-mandated negative cases:

  1. Service.Enabled()==false → all ops return ErrDisabled.
  2. Wrong password → ErrInvalidCredentials, failure_count++,
     audit row with event_category=auth.
  3. Failure_count exceeds threshold → locked, subsequent attempts
     (including with the CORRECT password) return identical-shape
     401 while the lockout window holds.
  4. Lockout window expires → next attempt with correct password
     succeeds + resets the counter.
  5. Password < 12 bytes (or > 256 bytes) → ErrWeakPassword.
  6. Password leak hygiene — the service has zero slog calls; the
     audit-row map literal never includes the password plaintext.
  7. Argon2id hash never appears in logs OR API responses — pinned
     by `json:"-"` tag on BreakglassCredential.PasswordHash + a
     belt-and-braces json.Marshal probe asserting the hash bytes
     never appear in the marshaled output.
  8. Constant-time-compare verified via timing-statistical test —
     wrong-password vs no-credential paths take statistically
     indistinguishable time (within 5x ratio). The verifyDummy()
     hash compute on the no-credential + locked paths is what
     keeps timing parity; absent that, an attacker could side-
     channel "actor doesn't have a credential" via timing.

Plus coverage-lift batch covering: SetPassword first-time vs rotate,
no-caller-id rejection, no-target-id rejection, RNG failure surface,
Authenticate happy-path mints session, no-credential audit row,
session-mint-failure surface, FailureResetInterval recycle, Unlock
+ RemoveCredential happy paths, hash-format unit tests (round-trip,
mismatch, malformed/wrong-version/bad-base64 formats), nil-audit +
nil-session pass-through.

Coverage on internal/auth/breakglass/ at 91.5% per-statement (above
the Phase 7.5 spec ≥ 90% floor).

cmd/server/main.go wiring:

  - Constructs breakglassRepo + breakglassService + breakglassHandler
    after the OIDC service block.
  - breakglassSessionMinterAdapter shim bridges *session.Service.Create
    to the breakglass.SessionMinter port.
  - Logs WARN at boot when CERTCTL_BREAKGLASS_ENABLED=true (operator
    visibility for the deliberate SSO-bypass).

internal/config/config.go gains:

  - AuthConfig.BootstrapAdminGroups + BootstrapOIDCProviderID for
    Phase 7 (CERTCTL_BOOTSTRAP_ADMIN_GROUPS comma-list +
    CERTCTL_BOOTSTRAP_OIDC_PROVIDER_ID).
  - AuthConfig.Breakglass nested struct with 4 env vars
    (CERTCTL_BREAKGLASS_ENABLED + LOCKOUT_THRESHOLD + LOCKOUT_DURATION
    + LOCKOUT_RESET_INTERVAL).

Router wiring:

  - 4 new breakglass routes registered when reg.AuthBreakglass != nil;
    public login route via direct r.mux.Handle (auth-exempt), 3 admin
    routes via r.Register + rbacGate(auth.breakglass.admin).
  - POST /auth/breakglass/login pinned in AuthExemptRouterRoutes
    allowlist with Phase 7.5 justification.
  - SpecParityExceptions extended with 4 new entries documenting
    the Phase 7.5 deferral of full per-endpoint OpenAPI rows
    (handler doc-block at the top of auth_breakglass.go is the
    operator-facing reference).

Threat model (encoded in service.go + auth_breakglass.go doc-blocks
+ migration 000038 docstrings, to be promoted to docs/operator/auth-
threat-model.md in Phase 12):

  - Break-glass is a deliberate bypass of the SSO security boundary.
    An attacker who phishes the password OR finds it in a compromised
    password manager bypasses MFA, OIDC, and every group-claim gate.
  - Recommendation: keep CERTCTL_BREAKGLASS_ENABLED=false in steady-
    state. Enable only during SSO-broken incidents. Disable after
    recovery.
  - WebAuthn pairing (v3 per Decision 12) is the load-bearing second
    factor. Without it, break-glass is best treated as an emergency-
    only path.
  - Audit trail surfaces every break-glass action under
    event_category=auth; the auditor role can monitor for unexpected
    break-glass logins.

Verifications: gofmt clean, go vet clean across all touched packages,
go test -short -count=1 green across internal/auth/oidc (3.0s; new
Phase 7 hook tests integrated alongside the 21+ Phase 3 negatives),
internal/auth/breakglass (3.6s; 8 spec-mandated negatives + coverage
batch passing), internal/config + internal/domain/auth + internal/api/
router + internal/api/handler all green, no regressions in Bundle 1
packages.
2026-05-10 06:51:41 +00:00
shankar0123 3189f3cd71 auth-bundle-2 Phase 6: session middleware + CSRF token plumbing +
chained-auth combinator + AuthInfo OIDC providers extension + 2 CI
guards (Bundle-1-compat + Bundle-1-to-2-upgrade)

Phase 6 wires the Phase 4 session service + Phase 5 OIDC handlers into
the request path. Three middlewares + one combinator land in
internal/auth/session/middleware.go:

  1. SessionMiddleware reads `certctl_session` cookie, validates via
     SessionService.Validate, populates the legacy UserKey/AdminKey
     + Phase 3 RBAC context keys (ActorIDKey/ActorTypeKey/TenantIDKey)
     so downstream RequirePermission + audit-attribution see a
     consistent caller. Best-effort UpdateLastSeen keeps the idle-
     expiry sliding window fresh. CRITICALLY: never 401s on validate
     failure — defers to the next middleware so the chained-auth
     combinator can fall back to Bearer.

  2. CSRFMiddleware gates state-changing methods (POST/PUT/DELETE/
     PATCH) for session-authenticated requests. API-key actors are
     EXEMPT (no session row in context => CSRF doesn't apply; they're
     not browser-driven). Constant-time-compares SHA-256(X-CSRF-Token
     header) against the session row's stored hash via
     SessionService.ValidateCSRF. Mismatch returns 403.

  3. ChainAuthSessionThenBearer is the load-bearing chained-auth
     combinator: tries the session cookie first; on miss/invalid,
     falls back to the API-key Bearer middleware; if neither
     authenticates, 401. The composition uses bearerSkipIfAuthenticated
     so a request with both a valid session AND a valid Bearer uses
     the session (cookie wins per the Bundle 2 contract).

Middleware chain order in cmd/server/main.go (per Phase 6 spec):

  RequestID → Logging → Recovery → CORS → RateLimit → AUTH (chained:
  session → Bearer) → CSRF (state-changing only; API-key exempt) →
  Audit → Handler

The chained authMiddleware replaces the bare Bundle-1 bearerMiddleware
at the chain entry point; csrfMiddleware lands immediately after so
session-authenticated requests pass through CSRF before audit. Both
new middlewares are pass-throughs when sessionService is nil
(pre-Phase-4 builds).

AuthInfo extension (Category E): GET /api/v1/auth/info now returns the
list of configured OIDC providers (id + display_name + login_url
where login_url = `/auth/oidc/login?provider=<id>`) so the GUI Login
page renders the correct "Sign in with X" buttons. Endpoint stays
auth-exempt; the providers list is public configuration. Wired via
HealthHandler.OIDCProvidersResolver + a new OIDCProvidersListResolver
projection interface; the cmd/server adapter
oidcProvidersListAdapter projects the postgres OIDCProviderRepository
into the public-safe shape. Resolver lookups are best-effort: failures
fall back to the minimal payload rather than 500-ing the GUI's auth
probe. Nil resolver preserves the pre-Phase-6 minimal shape so test
fixtures + no-db deploys keep compiling.

Bypass list preserved (Category E): the existing public-route
allowlist in router.AuthExemptRouterRoutes is preserved by virtue of
those routes registering via direct r.mux.Handle (they bypass the
entire chain). The protocol-endpoint allowlist (ACME/SCEP/EST/OCSP/
CRL) bypasses via cmd/server/main.go::buildFinalHandler URL-prefix
dispatch — those routes never reach the auth middleware at all. Both
preservations are pinned by the Bundle-1 compat CI guard below.

Tests (internal/auth/session/middleware_test.go):

All 7 Phase 6 spec-mandated middleware-chain tests pass:

  1. Session cookie + correct CSRF → 200.
  2. Session cookie + wrong CSRF → 403.
  3. Bearer-only (no session) + no CSRF → 200 (API-key actors are
     CSRF-exempt by design).
  4. No cookie + no Bearer → 401.
  5. Expired cookie + valid Bearer → fall back to Bearer succeeds.
  6. Tampered cookie → 401 (no Bearer to fall back to).
  7. Bypass-list awareness — state-changing method, no auth, no
     session row → uniform 401 (NOT a CSRF 403; the CSRF check is
     gated on session-row presence and never fires for unauth
     requests).

Plus coverage-lift tests covering nil-service pass-through, safe-
methods bypass, SessionFromContext nil + populated, isStateChangingMethod
matrix, clientIPFromRequest variants (RemoteAddr / XFF first-hop /
XFF single / no-port), nil-bearer chain branches.

Coverage on internal/auth/session/middleware.go: 100% per-function
across the 9 entry points (SessionValidator interfaces +
NewSessionMiddleware + NewCSRFMiddleware + ChainAuthSessionThenBearer +
bearerSkipIfAuthenticated + SessionFromContext + isStateChangingMethod
+ clientIPFromRequest + lastIndexByte). Package coverage 94.9%.

Two new CI guards:

  scripts/ci-guards/bundle-1-compat-regression.sh — Bundle-1-only
  compat invariants. Static-source checks that protect the Bundle-1
  path since spinning up docker-compose + running the integration
  test suite is sandbox-infeasible:
    1. SessionMiddleware MUST defer-to-next on missing/invalid cookie.
    2. CSRFMiddleware MUST be pass-through on missing session row.
    3. cmd/server/main.go MUST wire ChainAuthSessionThenBearer.
    4. The 4 public OIDC routes MUST be in AuthExemptRouterRoutes.
    5. AuthInfo MUST guard on OIDCProvidersResolver != nil.

  scripts/ci-guards/bundle-1-to-2-upgrade-regression.sh — Bundle-1 →
  Bundle-2 upgrade invariants:
    1. Migrations 000034..000037 use CREATE TABLE IF NOT EXISTS.
    2. Migrations are wrapped in BEGIN; ... COMMIT;.
    3. NO DROP TABLE / ALTER ... DROP COLUMN against any of the 19
       protected Bundle-1 tables (api_keys, audit_events, certificates,
       certificate_versions, profiles, issuers, targets, agents, jobs,
       owners, teams, agent_groups, notifications, roles, permissions,
       role_permissions, actor_roles, tenants, approvals,
       intermediate_cas, issuance_approval_requests).
    4. 000037 INSERTs use ON CONFLICT DO NOTHING (idempotent re-apply).
    5. ChainAuthSessionThenBearer is wired (Bundle-1 Bearer keys
       continue to authenticate post-upgrade).
    6. Bootstrap handler is registered (fresh-deployment bootstrap
       still works).

Both guards are sandbox-feasible static analysis. When the operator
gets a Linux VM with docker-in-docker, promote both to real `docker
compose up` integration tests against a v2.1.0 baseline DB dump.

Verifications: gofmt clean, go vet ./internal/auth/... ./internal/api/...
./cmd/server/... clean, go test -short -count=1 -race green across
internal/auth/session (94.9% coverage), internal/api/handler,
internal/api/router, no regressions in Bundle 1 packages, both new
ci-guards green.
2026-05-10 06:22:25 +00:00
shankar0123 9c679a5960 auth-bundle-2 Phase 5: OIDC + session HTTP surface (13 endpoints),
pre-login store, OpenID Connect Back-Channel Logout 1.0, cookieAuth
scheme, 7 new auth permissions, CI guard, handler tests

Phase 5 of the bundle puts the Phase 3 OIDC service + Phase 4 session
service on the wire. 13 HTTP endpoints split into three logical groups:

Public OIDC handshake (auth-exempt; protocol-mediated):
  GET  /auth/oidc/login?provider=<id>  -> 302 to IdP authorization URL
                                          + sets certctl_oidc_pending cookie
                                          (10-min TTL, Path=/auth/oidc/,
                                          SameSite=Lax)
  GET  /auth/oidc/callback?code=...&state=... -> consume pre-login row,
                                          run Phase 3's 11-step token
                                          validation, mint post-login
                                          session, 302 to dashboard
  POST /auth/oidc/back-channel-logout  -> OpenID Connect BCL 1.0 — IdP
                                          POSTs logout_token JWT; certctl
                                          validates signature against IdP
                                          JWKS via Phase 3 alg allow-list,
                                          required claims (iss/aud/iat/jti/
                                          events; exactly one of sub/sid;
                                          nonce ABSENT per spec §2.4),
                                          revokes matching sessions,
                                          returns 200 with
                                          Cache-Control: no-store
  POST /auth/logout                    -> revoke caller's session

Session management (RBAC-gated auth.session.*):
  GET    /api/v1/auth/sessions         -> auth.session.list (own / all)
  DELETE /api/v1/auth/sessions/{id}    -> auth.session.revoke (own bypass)

OIDC provider + group-mapping CRUD (RBAC-gated auth.oidc.*):
  GET    /api/v1/auth/oidc/providers              -> auth.oidc.list
  POST   /api/v1/auth/oidc/providers              -> auth.oidc.create
                                                     (client_secret encrypted
                                                     at rest via
                                                     internal/crypto.EncryptIfKeySet)
  PUT    /api/v1/auth/oidc/providers/{id}         -> auth.oidc.edit
  DELETE /api/v1/auth/oidc/providers/{id}         -> auth.oidc.delete
                                                     (refused via
                                                     ErrOIDCProviderInUse → 409
                                                     when users authenticated
                                                     via this provider)
  POST   /api/v1/auth/oidc/providers/{id}/refresh -> auth.oidc.edit
                                                     (re-runs IdP downgrade
                                                     defense via
                                                     OIDCService.RefreshKeys)
  GET    /api/v1/auth/oidc/group-mappings         -> auth.oidc.list
  POST   /api/v1/auth/oidc/group-mappings         -> auth.oidc.edit
  DELETE /api/v1/auth/oidc/group-mappings/{id}    -> auth.oidc.edit

Migration 000037 ships:

  - oidc_pre_login_sessions table (10-min absolute TTL, FK CASCADE on
    oidc_provider_id, FK RESTRICT on signing_key_id; index on
    absolute_expires_at for the GC sweep);
  - 7 new permissions seeded into r-admin only:
      auth.session.list, auth.session.list.all, auth.session.revoke,
      auth.oidc.list, auth.oidc.create, auth.oidc.edit, auth.oidc.delete

CanonicalPermissions extended in lockstep at internal/domain/auth/
validate.go.

Pre-login machinery:

  - internal/repository/oidc.go gains PreLoginRepository interface +
    PreLoginSession struct + ErrPreLoginNotFound / ErrPreLoginExpired
    sentinels.
  - internal/repository/postgres/oidc_prelogin.go ships the impl;
    LookupAndConsume uses DELETE ... RETURNING for atomic single-use.
  - internal/auth/oidc/prelogin.go is the PreLoginAdapter that bridges
    the OIDC service's Phase 3 PreLoginStore interface to the new
    repository, signing the cookie value under the active
    SessionSigningKey via the same v1.<id>.<key>.<HMAC> wire format
    Phase 4 uses for post-login cookies. Defense-in-depth: the
    pre-login `pl-` prefix is enforced by ParseCookieValue(prefix);
    a stolen pre-login cookie cannot be replayed against the
    post-login Validate path (pinned by
    TestService_Validate_RejectsPreLoginCookieAtPostLoginGate).

Session package extension:

  - internal/auth/session/service.go gains exported SignCookieValue,
    ParseCookieValue (with caller-supplied id-1 prefix), ComputeCookieHMAC,
    DecryptKeyMaterial wrappers so the OIDC pre-login adapter shares
    the same length-prefixed HMAC math without code duplication.
  - parseCookie no longer hardcodes the `ses-` prefix check (moved to
    Validate as defense-in-depth; pre-login cookie verification uses
    the `pl-` prefix via ParseCookieValue).

Cookie attributes (all Phase 5 endpoints honor CERTCTL_SESSION_SAMESITE
+ Secure=true via SessionCookieAttrs from Phase 4 config):

  - certctl_oidc_pending: Path=/auth/oidc/, MaxAge=600s, SameSite=Lax
    (cannot be Strict because the IdP-initiated callback is a top-level
    navigation from a different origin).
  - certctl_session: Path=/, Expires=8h, SameSite=Lax|Strict, HttpOnly.
  - certctl_csrf: Path=/, Expires=8h, HttpOnly=false (intentional —
    GUI must read it to echo into X-CSRF-Token header).

Audit logging on every mutating operation (event_category="auth"):

  auth.oidc_login_succeeded / failed / unmapped_groups
  auth.oidc_back_channel_logout / failed
  auth.session_revoked
  auth.oidc_provider_{created,updated,deleted,refreshed}
  auth.group_mapping_{added,removed}

OpenAPI updates:

  - cookieAuth security scheme added to api/openapi.yaml under
    components.securitySchemes (apiKey / cookie / certctl_session).
  - The 13 Phase 5 routes are added to SpecParityExceptions with a
    deferral note: full per-endpoint OpenAPI rows land in a follow-on
    commit alongside the GUI work (Phase 8) so the ergonomic shape can
    be validated against the live GUI client.

CI guard: scripts/ci-guards/N-bundle-2-security-empty-preserved.sh
asserts api/openapi.yaml has ≥ 14 'security: []' occurrences (the
pre-Bundle-2 baseline). Reducing the count below 14 would silently
force a Bearer-or-cookie requirement onto an endpoint that legitimately
runs without certctl-issued credentials; the guard fires before that
regression lands.

Handler tests (internal/api/handler/auth_session_oidc_test.go):

  - All 6 prompt-mandated negative cases:
      BCL with missing events claim -> 400
      BCL with nonce present -> 400 (per spec §2.4)
      BCL with sig signed by an unknown key -> 400
      Callback with replayed state -> 400
      Callback with PKCE verifier mismatch -> 400
      Callback with expired pre-login row -> 400
  - Plus happy paths for every endpoint, edge cases (missing-cookie,
    duplicate-name, in-use-409, wrong-tenant), and the Helper-function
    coverage (peekIssuer, classifyOIDCFailure, defaultIfBlank,
    defaultIntIfZero, clientIPFromRequest, encryptClientSecret).

Coverage on internal/api/handler/auth_session_oidc.go: 80.9% per-function
(above the Phase 5 spec's ≥ 80% floor).

Server wiring (cmd/server/main.go):

  Wired AFTER sessionService (Phase 4) so the OIDC PreLoginAdapter can
  sign pre-login cookies under the active SessionSigningKey:
    oidcProviderRepo + oidcMappingRepo + oidcUserRepo + oidcPreLoginRepo
    -> preLoginAdapter -> oidcService -> authSessionOIDCHandler.
  sessionMinterAdapter shim bridges *session.Service.Create to the
  oidcsvc.SessionMinter port the OIDC service consumes.

Router wiring (internal/api/router/router.go):

  4 public OIDC routes via direct r.mux.Handle (auth-exempt; pinned in
  AuthExemptRouterRoutes); 9 RBAC-gated routes via r.Register +
  rbacGate(checker, perm, h). Routes only register when
  reg.AuthSessionOIDC != nil so pre-Phase-5 builds skip the block
  entirely.

Verifications: gofmt clean, go vet clean across all touched packages,
go test -short -count=1 green across internal/api/handler (74 tests +
new Phase 5 batch), internal/api/router (parity + auth-exempt
allowlist), internal/auth/oidc + session (no regressions), full domain
+ scheduler + config sweeps green, ci-guard
N-bundle-2-security-empty-preserved.sh green (17 ≥ 14 baseline).
2026-05-10 06:08:27 +00:00
shankar0123 17b30c1f7f auth-bundle-2 Phase 4: session service (cookie minting + signature
validation, idle/absolute expiry, signing-key rotation, CSRF, GC),
15-case negative-test matrix, fail-fatal initial-key bootstrap

Phase 4 of the bundle ships the post-login session lifecycle that backs
every authenticated request once Phase 5 wires the OIDC handlers + the
session middleware. The state machine is the load-bearing primitive for
the Bundle 2 control plane: forge a session cookie and you bypass every
RBAC gate.

Service surface (internal/auth/session/service.go, ~880 LOC):

  - Service.Create(actorID, actorType, ip, ua) -> *CreateResult
    Mints a session row; signs the cookie value with the active signing
    key; returns the cookie payload AND the CSRF token plaintext for
    the handler to set on the response.
  - Service.Validate(ValidateInput) -> *Session
    Parses the cookie, looks up the signing key (incl. retired-but-in-
    retention), recomputes HMAC-SHA256, loads the session row, enforces
    revocation + absolute + idle expiry + optional IP/UA bind. Maps to
    one of 9 sentinel errors; the handler uniformly returns 401 to the
    wire (specific reason in the audit row).
  - Service.ValidateCSRF(headerValue, *Session) error
    Constant-time compares SHA-256(header) against the stored hash on
    the session row.
  - Service.UpdateLastSeen / Revoke / RevokeAllForActor
  - Service.RotateCSRFToken — mints fresh token, persists hash, returns
    plaintext; called on login completion, logout, role-change against
    actor, explicit operator rotate.
  - Service.RotateSigningKey — mints new active key, retires previous;
    retired keys stay valid for cfg.SigningKeyRetention so existing
    cookies don't immediately fail.
  - Service.EnsureInitialSigningKey — idempotent; mints first key on
    fresh deploys; emits auth.session_signing_key_bootstrap audit row
    with event_category=auth. Wired into cmd/server/main.go AFTER
    migrations + RBAC backfill, BEFORE the HTTP listener binds; failure
    is FATAL (logger.Error + os.Exit(1)) per the prompt — server refuses
    to boot rather than serve session-less.
  - Service.GarbageCollect — sweeps expired post-login sessions +
    pre-login rows >10min + retired-past-retention signing keys. Wired
    into the new internal/scheduler/scheduler.go::sessionGCLoop on a
    CERTCTL_SESSION_GC_INTERVAL tick.

Cookie wire format (load-bearing):

  v1.<session_id>.<signing_key_id>.<base64url-no-pad(HMAC-SHA256)>

The HMAC input is LENGTH-PREFIXED to defeat concatenation collisions:

  len(session_id) || ":" || session_id || ":" || len(signing_key_id) || ":" || signing_key_id

where len(...) is the ASCII decimal byte-length. Without the length
prefix, the bare-concatenation form `session_id || signing_key_id`
would let a forger swap one byte across the boundary — `<a, bc>` and
`<ab, c>` produce identical HMAC inputs. The length prefix moves the
boundary into the input itself so the two cases can never collide.

The v1. version prefix is reserved. A future incompatible upgrade
ships as v2. and the parser rejects unknown prefixes (no fallback).

CSRF token model:

  - Plaintext goes in a JS-readable certctl_csrf cookie (HttpOnly=false
    intentional; the GUI must read it to echo into X-CSRF-Token header).
  - SHA-256 hash of the plaintext lives on the session row.
  - Validation: SHA-256(X-CSRF-Token) constant-time-compared.
  - Rotated by Service.RotateCSRFToken on login / logout / role-change /
    explicit admin-trigger.

Optional defense-in-depth (default OFF):

  - CERTCTL_SESSION_BIND_IP — Validate compares client IP to row's
    recorded IP. Mismatch -> 401, audit row, session NOT auto-revoked
    (user may have legitimate IP change). Mobile + corporate-NAT
    environments leave this off.
  - CERTCTL_SESSION_BIND_USER_AGENT — same shape against UA.

Configurable lifetimes (env vars wired in internal/config/config.go):

  CERTCTL_SESSION_IDLE_TIMEOUT             1h
  CERTCTL_SESSION_ABSOLUTE_TIMEOUT         8h
  CERTCTL_SESSION_SIGNING_KEY_RETENTION    24h
  CERTCTL_SESSION_GC_INTERVAL              1h
  CERTCTL_SESSION_SAMESITE                 Lax
  CERTCTL_SESSION_BIND_IP                  false
  CERTCTL_SESSION_BIND_USER_AGENT          false

Test surface (internal/auth/session/service_test.go, ~860 LOC):

  All 15 prompt-mandated negative cases:

    1.  Tampered cookie (HMAC byte flipped near segment start where all
        6 bits are real — base64url-no-pad's last char carries only 2
        bits so a tail-flip is unreliable).
    1b. Tampered SESSION_ID segment (same HMAC-recompute outcome).
    2.  Cookie missing v1. prefix.
    3.  Cookie with unknown version prefix (v99).
    4.  Idle expiry — back-dated last_seen_at + idle_expires_at.
    5.  Absolute expiry — back-dated absolute_expires_at.
    6.  Revoked session.
    7.  Wrong signing key id (no row matches).
    8.  Cookie signed under retired-but-in-retention key SUCCEEDS.
    9.  Cookie signed under retired-past-retention key FAILS.
    10. Concatenation collision — direct evidence that
        computeHMAC("abc","de") != computeHMAC("ab","cde") AND that
        a forged-boundary-slide cookie is rejected.
    11. CSRF token missing.
    12. CSRF token mismatch (constant-time compare).
    13. IP-bind enabled + IP changed -> ErrSessionIPMismatch + audit row.
    14. UA-bind enabled + UA changed -> ErrSessionUAMismatch + audit row.
    15. EnsureInitialSigningKey RNG failure -> ErrInitialSigningKeyMintFailed
        wrap (cmd/server/main.go treats as fatal).

  Plus coverage-lift batch covering: every error wrap on every repo
  collaborator (Create, Get, UpdateLastSeen, UpdateCSRFTokenHash,
  Revoke, RevokeAllForActor, GC), every RNG-failure surface in Create /
  RotateCSRFToken / RotateSigningKey, every alg-pinning helper edge,
  the cookie parser's full negative matrix (empty, wrong segment count,
  missing prefixes, bad base64, wrong HMAC length), and a real-encryption
  round-trip via internal/crypto.EncryptIfKeySet -> DecryptIfKeySet so
  the v3-blob path is exercised end-to-end at the session-cookie level.

Coverage:

  internal/auth/session              94.5%  (floor 90)
  internal/auth/session/domain       96+%   (floor 90, Phase 1)

.github/coverage-thresholds.yml extended with 2 new gate entries
(internal/auth/session and internal/auth/session/domain). The
why: paragraphs explain why each fail-closed branch is load-bearing.

Repository extensions:

  internal/repository/session.go gains UpdateCSRFTokenHash on the
  SessionRepository interface; internal/repository/postgres/session.go
  ships the implementation. RotateCSRFToken consumes it.

Scheduler extensions:

  internal/scheduler/scheduler.go gains SessionGarbageCollector
  interface + sessionGC field + sessionGCInterval +
  SetSessionGarbageCollector + SetSessionGCInterval + sessionGCLoop.
  Pattern matches the existing acmeGCLoop: atomic.Bool guard prevents
  concurrent sweeps, sync.WaitGroup tracks for graceful shutdown,
  per-tick context.WithTimeout(1m) bounds a stuck Postgres.

Server wiring:

  cmd/server/main.go constructs sessionService AFTER the bootstrap
  block (post-RBAC backfill) and BEFORE the policy-service block.
  EnsureInitialSigningKey runs immediately; failure is fatal via
  os.Exit(1). The scheduler section wires SetSessionGarbageCollector
  + SetSessionGCInterval alongside the other interval setters and
  emits an Info log so operators can confirm the loop is enabled.

Phase 4 deviation note: Service.GarbageCollect() returns (int, error)
rather than the prompt's literal `error`. The int is the count of
session rows deleted on this sweep; the scheduler discards it (`_, err
:= ...`) but tests + future operator-facing audit rows can read it.
The wider behavior matches the spec exactly.

Verifications: gofmt clean, go vet ./internal/auth/session/...
./internal/scheduler/... ./internal/config/... ./cmd/server/...
./internal/repository/... clean, go test -short -count=1 -race green
across all 3 session packages, full repository + auth + scheduler +
config test sweeps green, no regressions in Bundle 1 packages.
2026-05-10 05:31:24 +00:00
shankar0123 854135dfb7 auth-bundle-2 Phase 3: OIDC service (HandleAuthRequest, HandleCallback,
RefreshKeys), hand-rolled group-claim resolver, 21+ negative-test
matrix, token-leak hygiene, IdP downgrade-attack defense

Phase 3 of the bundle ships the business logic that turns the Phase 2
storage primitives into a working OpenID Connect 1.0 + RFC 7636 PKCE
authorization-code flow against any enterprise IdP (Okta / Azure AD /
Google Workspace / Keycloak / Authentik / Auth0).

Service surface:

  - Service.HandleAuthRequest(providerID) -> authURL, cookie, preLoginID
    Builds the IdP redirect with PKCE-S256 (mandatory; RFC 9700 §2.1.1),
    server-generated 32-byte state + nonce, persisted to the pre-login
    row keyed by the cookie value.
  - Service.HandleCallback(cookie, code, state, ip, ua) -> *CallbackResult
    11-step validation: pre-login lookup-and-consume (single-use),
    constant-time state compare, code-for-token exchange with PKCE
    verifier, ID-token verify (alg pin via go-oidc/v3), service-layer
    re-checks of iss / aud / azp (multi-aud requires it; mismatch
    rejected) / at_hash (REQUIRED when access_token returned —
    Phase 3 lifts the OIDC core "MAY" to a service-level "MUST") /
    exp / iat-window / nonce, group-claim resolution with userinfo
    fallback, group->role mapping (fail-closed on no match),
    user upsert, session mint via SessionMinter port.
  - Service.RefreshKeys(providerID) — explicit cache eviction +
    re-load. Re-runs the IdP downgrade-attack defense so a provider
    that later rotates to advertising HS* / none is caught BEFORE the
    next user login attempt.

Security posture (every fail-closed branch is a sentinel error +
test):

  - Algorithm pinning: allow-list {RS256, RS512, ES256, ES384, EdDSA};
    deny-list {HS256, HS384, HS512, none}. Belt-and-braces re-check
    via isDisallowedAlg after go-oidc.Verify.
  - PKCE-S256 mandatory (oauth2.GenerateVerifier + S256ChallengeOption);
    `plain` rejection sentinel exists for defense-in-depth.
  - State + nonce: 32-byte crypto/rand, base64url-no-pad,
    constant-time compare, single-use.
  - IdP downgrade-attack defense: at provider creation / RefreshKeys,
    reject any IdP whose discovery doc advertises HS* / none in
    id_token_signing_alg_values_supported.
  - JWKS fail-closed: in-flight login fails 503; existing sessions
    untouched. isJWKSFetchError detects the gooidc verify-error
    shape; ErrJWKSUnreachable is the wire mapping.
  - Token-leak hygiene: ID tokens, access tokens, refresh tokens,
    authorization codes, PKCE verifiers, state, nonce, signing key
    bytes — NEVER logged at any level. logging_test.go pins the
    invariant via a slog buffer + grep-assert across HandleAuthRequest,
    HandleCallback, alg rejection, and provider-load paths.

Group-claim resolver (internal/auth/oidc/groupclaim/):

  - Hand-rolled per Decision 10 (no JSON-path lib; ~150 LOC).
  - URL-shape paths (https:// / http://) treated as a single
    literal key — Auth0 namespaced claims like
    https://your-namespace/groups work without splitting on the
    dots in the URL.
  - Dot-separated paths walked through nested map[string]interface{}.
  - []interface{} / []string / single-string normalized to []string;
    bool / number / object / nil → fail closed.
  - 18 unit tests + sentinels (ErrPathEmpty, ErrSegmentMissing,
    ErrSegmentNotObject, ErrInvalidValueType).

Test surface:

  - service_test.go: 57 test functions including all 21 prompt-mandated
    negative cases (wrong aud / wrong iss / expired / unknown alg /
    alg=none / HMAC alg / azp missing on multi-aud / azp mismatched /
    at_hash missing / at_hash mismatched / iat in future / iat too old /
    nonce mismatched / state mismatched / state replayed / PKCE plain
    sentinel / pre-login replay / forged cookie / IdP downgrade /
    group-claim missing / group-claim unmapped) plus the userinfo
    fallback matrix (happy path + endpoint-missing + endpoint-failing +
    userinfo-also-empty), HandleAuthRequest entry point + RNG-failure
    paths, upsertUser update + create + display-name fallback +
    Validate-error paths, decryptClientSecret real-encrypt round-trip
    + bad-passphrase, alg-parser malformed-header matrix.
  - logging_test.go: 4 hygiene tests pinning no token / code / verifier /
    state / cookie / client_secret / alg name appears in any captured
    log line.
  - groupclaim/resolver_test.go: 18 cases covering Okta string-array,
    Keycloak realm_access.roles, Auth0 namespaced URL claim,
    single-string normalization, deeply-nested 3-segment walks, and
    every fail-closed branch.

Coverage:
  internal/auth/oidc                  92.2%  (floor: 90)
  internal/auth/oidc/groupclaim      100.0%  (floor: 95)
  internal/auth/oidc/domain           96.2%  (floor: 90)

Coverage gates added at .github/coverage-thresholds.yml so a future
regression in any fail-closed branch fails CI before the commit lands.

Phase 3 of cowork/auth-bundle-2-prompt.md is closed. Next up: Phase 4
(Session service: cookies, revocation, sliding-vs-absolute expiry).
2026-05-10 04:56:03 +00:00
shankar0123 95f1d6cf63 auth-bundle-2 Phase 2b: repository interfaces + Postgres impls + integration tests
Closes Phase 2 end-to-end. Builds on Phase 2a's three migrations
(000034 oidc_providers + group_role_mappings, 000035 sessions +
session_signing_keys, 000036 users) by shipping the repository surface
Phase 3+ services consume.

Interfaces:
* internal/repository/oidc.go - OIDCProviderRepository (List, Get,
  GetByName, Create, Update, Delete) + GroupRoleMappingRepository
  (ListByProvider, Get, Add, Remove, Map). Sentinels:
  ErrOIDCProviderNotFound, ErrOIDCProviderDuplicateName,
  ErrOIDCProviderInUse (FK ON DELETE RESTRICT translation),
  ErrGroupRoleMappingNotFound, ErrGroupRoleMappingDuplicate.
* internal/repository/session.go - SessionRepository (Create, Get,
  ListByActor, UpdateLastSeen, Revoke, RevokeAllForActor,
  GarbageCollectExpired, Delete) + SessionSigningKeyRepository (List,
  GetActive, Get, Add, Retire, Delete). Sentinels: ErrSessionNotFound,
  ErrSessionRevoked, ErrSessionExpired, ErrSessionSigningKeyNotFound,
  ErrSessionSigningKeyInUse.
* internal/repository/user.go - UserRepository (Get, GetByOIDCSubject,
  Create, Update, ListAll). Sentinels: ErrUserNotFound,
  ErrUserDuplicateOIDCSubject.

Postgres implementations:
* internal/repository/postgres/oidc.go - 309 lines. Translates
  SQLSTATE 23505 (unique_violation) to ErrOIDCProviderDuplicateName /
  ErrGroupRoleMappingDuplicate; SQLSTATE 23503 (foreign_key_violation)
  to ErrOIDCProviderInUse so the Phase 5 handler maps to HTTP 409
  when an operator tries to delete a provider with authenticated
  users. pq.StringArray bridges Go []string to Postgres TEXT[] for
  scopes + allowed_email_domains. Map() uses
  `WHERE group_name = ANY($2)` so a single SELECT resolves N IdP
  group claims at once.
* internal/repository/postgres/session.go - 350 lines. Both Session +
  SessionSigningKey repos. Revoke + Retire are idempotent (re-revoking
  an already-revoked session returns nil; same for retire). The
  GarbageCollectExpired sweep deletes both
  absolute-expiry-passed sessions AND pre-login rows older than the
  10-minute TTL in one DELETE so the scheduler tick is cheap.
  ErrSessionSigningKeyInUse pinned via SQLSTATE 23503 from the
  sessions.signing_key_id FK ON DELETE RESTRICT.
* internal/repository/postgres/user.go - 137 lines. GetByOIDCSubject
  is the Phase 3 hot-path lookup; the (oidc_provider_id,
  oidc_subject) UNIQUE constraint trip translates to
  ErrUserDuplicateOIDCSubject. Update only writes the mutable field
  set (email, display_name, last_login_at, webauthn_credentials);
  oidc_subject + oidc_provider_id are immutable per the
  per-(provider, subject) identity model.

Integration tests (testing.Short()-gated, testcontainers + Postgres
16 Alpine, schema-per-test isolation via getTestDB().freshSchema):

* oidc_test.go: 11 tests covering happy-path + GetNotFound +
  DuplicateName + List + Update + DeleteNotFound + DeleteSucceeds +
  DeleteRefusedWhenUsersReference (the FK ON DELETE RESTRICT pin);
  GroupRoleMapping coverage includes Add/List/Map (3 cases:
  marketing-not-mapped, multi-group hits, empty groups returns
  empty), Duplicate rejection, and the ON DELETE CASCADE on
  provider deletion.
* session_test.go: 12 tests covering SessionSigningKey + Session.
  Key tests: GetActiveSkipsRetired (mints older, retires it, mints
  newer, asserts GetActive returns newer), DeleteRefusedWhenSessions-
  Reference (FK pin), RetireIsIdempotent. Session tests:
  CreateAndGet roundtrip, GetNotFound, Revoke + idempotent re-Revoke,
  ListByActor (3 active + 1 revoked + 1 pre-login -> returns 3,
  pinning the WHERE filter), RevokeAllForActor, GarbageCollectExpired
  (seeds an absolute-expired row + pre-login >10min row + active
  session via raw SQL to bypass CHECK constraints, asserts GC kills
  exactly 2 + active survives), UpdateLastSeen.
* user_test.go: 7 tests covering CreateAndGet, GetNotFound,
  GetByOIDCSubject (hit + miss), DuplicateOIDCSubjectRejected,
  UpdateMutableFields (asserts oidc_subject NOT mutated by Update),
  ListAll, FKRestrictsProviderDelete (mirror of the OIDC test from
  the user side - both ends of the FK contract pinned).

Verifications:
* gofmt -l clean across all 9 new files.
* go vet ./internal/repository/postgres/ rc=0.
* go test -short -count=1 green on internal/repository/postgres/ +
  internal/auth/... + Bundle 1 packages (testing.Short() skips the
  testcontainers integration tests, but the test files compile + the
  short-mode skip path is exercised so the suite is wired correctly).
* Full integration tests run in CI's non-short job against Postgres
  16 Alpine via testcontainers-go.
* govulncheck ./... clean.
* All 24 ci-guards pass.

Phase 2 exit criteria from cowork/auth-bundle-2-prompt.md (all met):
* All three Phase-2 migrations apply cleanly, idempotently: yes
  (Phase 2a). Break-glass migration ships separately in Phase 7.5.
* Repository tests pass against Postgres 16 Alpine: integration
  tests written, gated by testing.Short(), structured to run cleanly
  in CI's non-short job.
* make verify equivalent green: gofmt + vet + go test pass;
  golangci-lint deferred to CI per Phase 0/1's same pattern.
2026-05-10 04:18:27 +00:00
shankar0123 315e132981 auth-bundle-2 Phase 2a: SQL migrations (oidc_providers, sessions, users)
Three new idempotent transactional migrations that materialize the
Phase 1 domain types into Postgres tables. Repository implementations
+ integration tests land as Phase 2b in the next commit.

migrations/000034_oidc_providers.up.sql:
  oidc_providers table with the full OIDCProvider field set
    (issuer_url + client_id + client_secret_encrypted v2 blob +
    redirect_uri + groups_claim_path + groups_claim_format +
    fetch_userinfo + scopes[] + allowed_email_domains[] +
    iat_window_seconds + jwks_cache_ttl_seconds + tenant_id).
  group_role_mappings table linking provider+group_name to role_id.
  Closed-enum CHECK on groups_claim_format ('string-array' or
    'json-path').
  Defense-in-depth bounds CHECKs on iat_window_seconds (1..600) and
    jwks_cache_ttl_seconds (>= 60); app-layer Validate() also
    enforces these.
  ON DELETE CASCADE on group_role_mappings.provider_id so deleting a
    provider cleans up its mappings.
  ON DELETE RESTRICT on group_role_mappings.role_id so an in-use role
    can't be silently dropped.

migrations/000035_sessions.up.sql:
  session_signing_keys table with key_material_encrypted v2 blob +
    retired_at nullable + the retired-after-created CHECK.
  Partial index on (tenant_id, created_at DESC) WHERE retired_at IS
    NULL backs the GetActive hot path.
  sessions table covers BOTH the post-login row (1h-idle/8h-absolute
    cookie lifecycle) AND the Phase 5 pre-login row (10-minute TTL,
    is_pre_login=true). csrf_token_hash holds the SHA-256 of the
    CSRF token plaintext (the plaintext lives in a separate
    JS-readable cookie, hashed here so a DB-read leak can't replay).
  Two CHECK constraints pin the expiry order (absolute > idle, idle >
    created); these match the Phase 1 domain Validate() pre-write
    invariants but enforce them at the DB layer too so direct SQL
    inserts can't silently land malformed rows.
  Partial indexes on actor_id (active sessions only), the active
    session lookup, the pre-login GC sweep (created_at), and the
    absolute-expired GC sweep (absolute_expires_at) cover the four
    hot paths Phase 4's service consumes.
  ON DELETE RESTRICT on sessions.signing_key_id so a signing key
    referenced by an active session can't be dropped (the retention
    window keeps retired keys valid; full purge waits until every
    session signed under that key has expired).

migrations/000036_users.up.sql:
  users table for federated-human identity (per-(provider, subject)
    tuple via UNIQUE constraint, not global - identity is per-IdP by
    design).
  webauthn_credentials JSONB DEFAULT '[]' reserved for v3 (Decision
    12); Bundle 2 always stores [].
  Email index for the GUI's "find user by email" surface (not unique
    because the same email can appear in multiple providers per the
    per-IdP identity model).
  ON DELETE RESTRICT on users.oidc_provider_id keeps Phase 3's "delete
    provider only when no users authenticated via it" rule enforced
    at the DB layer; the OIDCProviderRepository.Delete impl will
    translate SQLSTATE 23503 into a 409 sentinel.

All three migrations:
  Wrapped in BEGIN/COMMIT so partial-fail leaves no half-state.
  IF NOT EXISTS / IF EXISTS / ON CONFLICT DO NOTHING for idempotency
    (the certctl-server boot path applies every migration on every
    start per CLAUDE.md "Idempotent migrations" architecture rule).
  TIMESTAMPTZ for time columns (no TIMESTAMP WITHOUT TIME ZONE).
  TEXT primary keys with prefixes per CLAUDE.md "Architecture
    Decisions" (op- / grm- / sk- / ses- / u-).
  Multi-tenant ready: tenant_id column with DEFAULT 't-default' on
    every row, FK to tenants(id) ON DELETE CASCADE. Bundle 2 ships
    single-tenant; managed-service activation adds tenants without a
    schema migration.

Down migrations exist in lockstep, drop tables in FK-safe order
(group_role_mappings -> oidc_providers; sessions ->
session_signing_keys; users alone). Down-migrations are destructive;
docstrings call this out.

Verifications:
  Migration count: ls migrations/*.up.sql | wc -l = 36 (33 from
    Bundle 1 + 3 new).
  BEGIN/COMMIT pair counts: each new migration is 1:1.
  No Docker in this sandbox, so the migrations are not applied
    end-to-end here; CI's testcontainers harness runs them via
    postgres.RunMigrations on every push. Phase 2b's repository
    integration tests will exercise the schema against Postgres 16
    Alpine.
2026-05-10 04:08:06 +00:00
shankar0123 b0ac24fbf8 auth-bundle-2 Phase 1: OIDC + Session + User + Breakglass domain types
Phase 1 ships the persisted-shape types Bundle 2 needs end-to-end.
No DB migrations, no service layer, no HTTP handlers; Phase 2 ships
the SQL, Phase 3+ ship the consumers. Each type has a Validate()
method that enforces the on-disk invariants the schema will mirror,
and a focused _test.go that pins each invariant's failure mode.

Per-package summary:

internal/auth/oidc/domain/ (OIDCProvider + GroupRoleMapping):
* OIDCProvider carries the operator-configured IdP record. Fields
  match the prompt's Phase 1 list plus IATWindowSeconds and
  JWKSCacheTTLSeconds (Phase 3 references these by name; landing
  them in Phase 1's domain type avoids the lying-field gap).
  ClientSecretEncrypted is opaque from this layer; it is the v2 blob
  produced by internal/crypto/encryption.go and is `json:"-"` so it
  never wire-leaks.
* Validate() rejects: invalid id prefix, empty name, non-https
  issuer_url (matches Phase 3's "JWKS endpoint MUST be HTTPS"),
  empty client_id, empty client_secret_encrypted, non-https
  redirect_uri, invalid groups_claim_format, scopes missing openid,
  IAT window outside (0, 600], JWKS cache TTL below 60s. Defaults
  applied in-place: GroupsClaimPath="groups", GroupsClaimFormat=
  "string-array", Scopes=["openid","profile","email"],
  IATWindowSeconds=300, JWKSCacheTTLSeconds=3600,
  TenantID="t-default".
* GroupRoleMapping carries the operator-configured group-to-role
  rule. Validate() pins prefix conventions ("grm-", "op-", "r-")
  and non-empty group name.
* 18 tests across happy-path + every negative invariant.

internal/auth/session/domain/ (Session + SessionSigningKey):
* Session covers BOTH the post-login row (full 1h-idle/8h-absolute
  cookie lifecycle) AND the Phase 5 pre-login row (10-minute TTL,
  carries OIDC state+nonce+PKCE verifier across the IdP redirect).
  IsPreLogin discriminates. CSRFTokenHash holds SHA-256 of the
  CSRF token plaintext (the plaintext lives in a JS-readable
  certctl_csrf cookie; storing only the hash on the row defends
  against DB-read leaks per the Phase 4 CSRF contract).
* Validate() pins: id prefix "ses-", non-empty actor id/type,
  signing key id prefix "sk-", AbsoluteExpiresAt strictly > Idle,
  IdleExpiresAt strictly > CreatedAt, CSRFTokenHash exactly 64
  lowercase hex chars when set.
* Cookie naming constants pinned by a separate test
  (TestCookieNamingConstants) so a future rename can't silently
  break the GUI's web/src/api/client.ts which reads these names by
  string.
* SessionSigningKey stores the v2-encrypted HMAC key material; the
  retired-before-created invariant catches malformed rows. 14
  tests across both types.

internal/auth/user/domain/ (User):
* Federated-human identity for SSO logins. Distinct from Bundle 1's
  free-form actor_id strings: actor_roles.actor_id = User.ID for
  federated humans (per the prompt's note about how the two
  identity systems intersect).
* WebAuthnCredentials JSONB column reserved for v3 (Decision 12);
  defaults to "[]" on Validate() so Bundle 2 + v3 share the same
  on-disk format from day one.
* Email validation is intentionally loose (basic shape: one @,
  non-empty local + domain, no whitespace, dot in domain). RFC 5321
  / 5322 grammars are not enforced; the IdP issued the email and
  we trust its shape, only rejecting gross corruption.
* 8 tests across happy-path + invalid-id + empty-email +
  malformed-email + invalid-provider-id + tenant defaulting +
  WebAuthn-credentials passthrough.

internal/auth/breakglass/domain/ (BreakglassCredential):
* Phase 7.5 type. Argon2id PHC-format password hash; Validate()
  pins the Argon2id magic prefix so non-Argon2id formats (bcrypt,
  pbkdf2, plaintext) are rejected at the persistence boundary.
* MinPasswordLengthBytes (12) + MaxPasswordLengthBytes (256)
  constants pinned by a dedicated test so the operator-facing
  password-strength contract can't drift silently.
* IsLocked(now) helper exposes the lockout state machine for the
  Phase 7.5 service to consume; the lockout window default is
  15min in the service layer.
* 9 tests across happy-path + per-invariant negative + lockout
  state machine + tenant defaulting.

Cross-cutting:
* Every type has json:"-" on the encrypted-credential field
  (ClientSecretEncrypted, KeyMaterialEncrypted, PasswordHash,
  CSRFTokenHash) so even a misconfigured handler that marshals the
  domain type directly into a response body cannot leak the
  secret. Mirrors Bundle 1's pattern for issuer/target credentials.
* Every type carries TenantID with Validate() defaulting to
  authdomain.DefaultTenantID. Forward-compat for the future
  managed-service multi-tenant activation; Bundle 2 ships
  single-tenant.

Verifications:
* gofmt -l clean across all 8 new files (one round-trip required to
  satisfy Go 1.19+ doc-comment list-formatting rules in
  session/domain/types.go).
* go vet clean on internal/auth/oidc/... + session/... + user/... +
  breakglass/...
* go test -short -count=1 green on all four new domain packages
  (49 test functions total).
* go test -short -count=1 still green on Bundle 1 packages
  (internal/auth, internal/auth/bootstrap, internal/service/auth,
  internal/config).
* govulncheck ./... clean (M-024 hard CI gate).
* All 24 ci-guards pass locally.

Phase 1 exit criteria from cowork/auth-bundle-2-prompt.md:
* All types compile: yes.
* Validators have at least 5 test cases each: yes (smallest is
  User with 8 tests; OIDCProvider has 13).
* make verify equivalent green: gofmt + vet + go test pass
  (golangci-lint deferred to CI per the same operating-rule
  pattern Phase 0 used).
2026-05-10 03:41:46 +00:00
shankar0123 2d9110b0c4 auth-bundle-2 Phase 0: dependency-add + oidc auth-type literal + runtime guard
Bundle 2 Phase 0 stages the dependencies + auth-type discriminator
literal that later phases consume. No handler chain wired yet; an
operator who sets CERTCTL_AUTH_TYPE=oidc on this commit gets a clear
refuse-to-start error rather than a silent fallback to api-key (the
G-1 failure mode that drove "jwt" out of the allowed set).

Deliverables:

* go.mod: github.com/coreos/go-oidc/v3 v3.18.0 added as a direct
  require. Per the pre-bundle dependency audit (Apache-2.0, zero CVEs
  ever per OSV.dev, 2,400+ stars, used by Hashicorp Vault + Dex +
  Hydra + Authentik + every Kubernetes OIDC integration), this is the
  ecosystem-standard Go OIDC client. Pinned to a specific minor
  (v3.18.0) per the prompt's "no bare latest" rule.
* go.mod: golang.org/x/oauth2 promoted from // indirect to direct,
  bumped from v0.34.0 to v0.36.0 by go mod tidy. Both versions are
  OSV-clean. Maintained by the Go team.
* No JSON-path library added (forbidden by the dependency audit; the
  group-claim resolver is hand-rolled in Phase 3).
* internal/config/config.go: AuthTypeOIDC constant added with a
  load-bearing comment explaining (a) this is the AUTH-TYPE literal,
  not a JWT alg literal, so the G-1 closure invariant is preserved
  ("jwt" stays out of ValidAuthTypes forever); (b) the runtime guard
  in cmd/server/main.go intentionally refuses-to-start when oidc is
  set pre-Phase-6 to avoid the silent-downgrade failure mode.
  ValidAuthTypes() now returns {api-key, none, oidc}.
* internal/config/config_test.go: TestValidAuthTypesIsExactly_APIKey_None
  renamed to TestValidAuthTypesIsExactly_APIKey_None_OIDC and now pins
  the 3-entry set. TestValidAuthTypesDoesNotContainJWT (G-1 closure
  test) still passes because "jwt" is never added back.
  TestValidate_GenericInvalidAuthType's bad-types list updated:
  "oidc" removed (now valid), "saml" added (correctly rejected per
  Decision 5's SAML deferral).
* cmd/server/main.go: defense-in-depth runtime auth-type guard now
  has an explicit AuthTypeOIDC case that exit(1)s with an actionable
  message: "the OIDC auth chain is not yet wired in this build (Auth
  Bundle 2 Phase 6 ships the session middleware that consumes this
  auth-type literal)." This closes the lying-field gap the literal
  would otherwise create. Phase 6 of Bundle 2 relaxes this case to
  fall through alongside api-key + none.
* api/openapi.yaml: /v1/auth/info auth_type enum extended from
  [api-key, none] to [api-key, none, oidc] with an in-line comment
  explaining the Phase-0-vs-Phase-6 timing so an OpenAPI consumer
  isn't surprised by "oidc" appearing here pre-Bundle-2-merge.
* deploy/helm/certctl/templates/_helpers.tpl::certctl.validateAuthType:
  valid set extended to include "oidc". Chart-time validation now
  passes for type=oidc; the binary's runtime guard takes over to
  refuse the start. Once Bundle 2 ships, the runtime guard relaxes
  and OIDC works end-to-end with no further chart edits.
* .env.example: CERTCTL_AUTH_TYPE comment block updated to document
  the three valid values + the Phase-0-vs-Phase-6 timing.
* internal/auth/oidc/doc.go: new package directory with package doc
  + transitional blank imports for coreos/go-oidc/v3 + x/oauth2 so
  go mod tidy keeps both deps as direct requires until Phase 3's
  service.go replaces the blanks with real symbol use. Doc explains
  the package layout (oidc/ + oidc/domain/ + oidc/groupclaim/ +
  oidc/testfixtures/) so the post-Bundle-2 reader can navigate.

Verifications:
* gofmt clean on every changed file.
* go vet clean on internal/config + cmd/server + internal/auth/oidc.
* go test -short -count=1 green on internal/config (including the
  G-1 closure + new validation tests), cmd/server, internal/auth (all
  Bundle 1 packages), internal/service/auth.
* govulncheck ./... clean (M-024 hard CI gate).
* All 24 ci-guards pass locally.

Phase 0 exit criteria from cowork/auth-bundle-2-prompt.md:
* go.mod shows coreos/go-oidc/v3 as direct: yes.
* golang.org/x/oauth2 is direct (not indirect): yes.
* govulncheck ./... clean: yes.
* No JSON-path library in go.mod / go.sum deltas: confirmed (only
  v3 of go-oidc + the x/oauth2 bump landed).
* make verify green: gofmt + vet + go test pass; full make verify
  (which would invoke golangci-lint) deferred to CI since the
  sandbox doesn't have golangci-lint installed; the operator runs
  make verify locally before pushing per CLAUDE.md operating rule.
2026-05-10 03:31:51 +00:00
shankar0123 977cdbdf44 docs(README): surface Bundle 1 RBAC + signal Bundle 2 federation as roadmap
Pre-fix the README said nothing about role-based access control,
the auditor role, the day-0 bootstrap path, or the four-eyes
approval workflow — all shipped in Bundle 1 (commit 22c4971 +
follow-ons). A prospective adopter landing on the README would
read "API key auth enforced by default" and walk away thinking
certctl had no authz primitive at all. The only OIDC reference
was the cosign-keyless line at the artefact-signing section,
unrelated to authentication.

Three surgical edits:

1. Status block: extend the "production-quality core" enumeration
   with role-based authz, auditor split, day-0 bootstrap, four-eyes
   approval. Add a one-line callout that federated identity (OIDC,
   SAML, WebAuthn, server-side sessions, break-glass, JIT
   elevation) is roadmap-not-shipped — preempts the natural-but-
   wrong assumption that "RBAC means OIDC works".
   The two terms are linked inline:
     - "role-based authz" -> docs/operator/rbac.md (operator how-to:
       role table, permission catalogue, scope semantics, GUI/CLI/
       HTTP/MCP grant flows, day-0 bootstrap).
     - "Federated identity" -> docs/operator/auth-threat-model.md
       #threats-bundle-1-does-not-close (canonical place where
       deferred Bundle-2 work is enumerated).
   Keeps the roadmap promise honest: a skeptic can click through
   to the explicit deferred-work list rather than taking prose at
   face value.

2. "What it does" feature list: insert a new bullet right after the
   approval-workflow bullet covering the 7 default roles, the 33-
   permission canonical catalogue, scope semantics, the auditor
   read-only invariant, the bootstrap path, and the
   privilege-escalation guard. Cross-links to docs/operator/rbac.md,
   the threat model, and the v2.0.x → v2.1.0 migration guide.

3. Security paragraph: replace "API key auth enforced by default
   with SHA-256 hashing and constant-time comparison" with the
   Bundle-1 reality — auth + RBAC + auditor + bootstrap + privilege-
   escalation guard — keeping the rest of the paragraph (CORS,
   SSRF, encryption-at-rest, TLS-1.3, audit trail, CI gates)
   unchanged.

Verified:
  Both link targets exist on disk
    (docs/operator/rbac.md, docs/operator/auth-threat-model.md).
  Threat-model anchor heading "## Threats Bundle 1 does NOT close"
    is intact (line 138).
  All 24 ci-guards pass locally including S-1 (no hardcoded source
    counts re-introduced) and G-3 (no env-var docs drift).

Updates the README to match Bundle 1's actually-shipped surface
and to set honest expectations about Bundle 2 (federated identity)
being the next slice, not yet landed.
2026-05-10 02:21:39 +00:00
shankar0123 5d79e53ad0 auth-bundle-1 follow-on: close coverage gaps to clear Phase 12 floors
CI run #486 (post-Bundle-1 merge + Go 1.25.10 bump) failed three
coverage-threshold gates:

  internal/api/handler   74.7% < floor 75 (-0.3pp)
  internal/auth          66.3% < floor 85 (-18.7pp)
  internal/service/auth  51.1% < floor 85 (-33.9pp)

The Phase 12 gate file's "85% with negative-test coverage" claim
turned out to be aspirational — the read-side and Update-path
methods on RoleService / PermissionService / ActorRoleService had
zero unit-test coverage, and internal/auth's keystore +
HasPermission helper had zero tests. This commit closes the gap
without lowering the gate.

Per-package CI-style averages after this commit (per
scripts/check-coverage-thresholds.sh's per-function-mean):

  internal/api/handler   76.1% (+1.4pp,  margin +1.1pp)
  internal/auth          90.5% (+24.2pp, margin +5.5pp)
  internal/service/auth  93.7% (+42.6pp, margin +8.7pp)

Tests added:

  internal/service/auth/service_test.go (+18 tests, +518 LOC):
    PermissionService.List, PermissionService.GetByName,
    RoleService.Get (4 paths), RoleService.List (system caller),
    RoleService.Update (4 paths), RoleService.ListPermissions
    (3 paths), RoleService.AddPermission/RemovePermission round-trip
    + gate paths, RoleService.Delete (success + nil-caller +
    no-perm + audit), RoleService.Create (nil-caller),
    ActorRoleService.ListForActor (self-bypass + cross-actor +
    nil-caller + system + with-perm), ActorRoleService.Effective-
    Permissions (same shape), ActorRoleService.ListKeys (3 paths +
    system bypass), ActorRoleService.Revoke (4 paths), Authorizer
    edge cases (empty actorID short-circuit, empty tenantID
    default, scoped-grant-without-scope-id no-match invariant,
    repo-error wrap-and-return, HoldsAnyOf early-exit), recordAudit
    nil-arm short-circuits.

  internal/auth/keystore_test.go (NEW, +175 LOC):
    StaticKeyStore.Len, StaticKeyStore.LookupByHash hit + miss,
    MutableKeyStore seeded lookup + Len, Add registers new key,
    AddHashed registers from precomputed hash, AddHashed replaces
    on duplicate hash (idempotent boot-loader contract),
    HasPermission no-actor / default-actor-type / checker-error /
    scoped-check threading.

  internal/auth/bootstrap/service_test.go (+36 LOC):
    Service.Available nil-receiver/nil-strategy short-circuit,
    Service.Available delegates to Strategy when configured.

  internal/api/handler/auth_test.go (+208 LOC):
    GetRole returns role + permissions, GetRole 404 + 401, UpdateRole
    200 + invalid-JSON-400 + 401, ListKeys returns actor list + 401,
    RemoveRolePermission 204 (global + scoped) + 401,
    rolePermToResponse scope encoding pin via GetRole.

Verified:
  gofmt -l . clean (touched files only).
  go vet ./internal/auth/... ./internal/service/auth/...
       ./internal/api/handler/ rc=0.
  go test -count=1 -short on the four packages green.
  CI-style per-function averages computed via the live
       scripts/check-coverage-thresholds.sh arithmetic — all three
       gated packages clear their floors with margin.

Per CLAUDE.md "complete path" + "do not lower the gate to make CI
green": gate file unchanged. The 85/85/75 floors stand.
2026-05-10 02:04:36 +00:00
shankar0123 3e91c7a1f0 chore(security): bump Go toolchain 1.25.9 -> 1.25.10 + golang.org/x/net 0.49 -> 0.53
CI run #484's Go Build & Test job failed govulncheck (M-024 hard
gate). Six standard-library CVEs land in go1.25.9 + one
golang.org/x/net CVE in v0.49.0; all are fixed in go1.25.10 + x/net
v0.53.0 respectively. The advisories that fired were:

  GO-2026-4986  Quadratic string concat in net/mail.consumeComment
                — called via internal/api/handler/validation.go's
                ValidateCommonName -> mail.ParseAddress
  GO-2026-4977  Quadratic string concat in net/mail.consumePhrase
                — same call site
  GO-2026-4982  Bypass of meta-content URL escaping in html/template
                — called via internal/service/digest.go's
                RenderDigestHTML -> Template.Execute
  GO-2026-4980  Escaper bypass in html/template
                — same call site
  GO-2026-4971  Panic in net.Dial / LookupPort on Windows NUL bytes
                — many call sites (email notifier, SSH connector,
                ACME validators, validation.ValidateSafeURL, ...)
  GO-2026-4918  Infinite loop in net/http2 transport on bad
                SETTINGS_MAX_FRAME_SIZE
                — called via internal/connector/target/f5.go's
                F5Client.Authenticate -> http.Client.Do

Bumps applied:

* `go.mod`: `go 1.25.9` -> `go 1.25.10`; `golang.org/x/net v0.49.0`
  -> `v0.53.0` (kept indirect — the upgrade is force-pulled by the
  module-version directive; transitive deps will pick the higher).
* `.github/workflows/{ci,codeql,release}.yml`: setup-go pin and the
  release.yml `GO_VERSION` env var bumped to 1.25.10. The
  security-deep-scan.yml workflow uses the major-minor `1.25` pin
  which auto-resolves to the latest 1.25.x and is unaffected.
* `Dockerfile` + `Dockerfile.agent`: `golang:1.25-alpine@sha256:5caa...`
  re-pinned to `golang:1.25.10-alpine@sha256:8d22e29d960bc50cd0...`
  (digest looked up against `registry-1.docker.io/v2/library/golang/
  manifests/1.25.10-alpine`; verified by the digest-validity ci-guard).
  The explicit `1.25.10-alpine` tag form replaces the moving
  `1.25-alpine` pin so the image-spec is reproducible end-to-end
  even without the digest reference.
* `deploy/test/f5-mock-icontrol/Dockerfile`: `golang:1.25.9-bookworm
  @sha256:1a14...` re-pinned to `golang:1.25.10-bookworm@sha256:
  e3a54b77385b4f8a31c1...` (looked up the same way).
* `deploy/test/f5-mock-icontrol/go.mod`: `go 1.25.9` -> `go 1.25.10`.
* `internal/api/handler/version.go` + `api/openapi.yaml`: the
  `runtime.Version()`-shape comment + OpenAPI `example: go1.25.9`
  bumped to keep doc/example freshness.
* `docs/contributor/ci-pipeline.md` + `docs/reference/connectors/
  iis.md`: doc-only `Go 1.25.9` -> `Go 1.25.10` references.

Verification done in-tree:

* All `scripts/ci-guards/*.sh` pass locally including
  `digest-validity.sh` (the new digests resolve cleanly against
  Docker Hub).
* `S-1-hardcoded-source-counts.sh` clean (the false-positive on
  "Bundle 1 migrations" was fixed in the prior commit).

Operator step required post-push (sandbox has no Go toolchain):

  cd certctl && go mod tidy

This regenerates go.sum's `golang.org/x/net v0.49.0` h1: lines into
v0.53.0 ones. CI's `go mod tidy && git diff --exit-code go.mod
go.sum` step will catch the drift if missed; in that case run the
command, commit, and push the go.sum-only delta.
2026-05-09 21:35:46 -04:00
shankar0123 51f55c5fc9 auth-bundle-1 fix: S-1 ci-guard false positive on "Bundle 1 migrations"
CI run #484 surfaced the regression in the Frontend Build job:

  ::error::S-1 regression: hardcoded source-count prose reappeared:
  docs/migration/api-keys-to-rbac.md:32:schema is already at the target
  version. The Bundle 1 migrations

The S-1 guard's regex (scripts/ci-guards/S-1-hardcoded-source-counts.sh)
catches `\b[0-9]+\s+migrations\b` to prevent stale "<N> migrations"
prose in docs/. The Bundle 1 migration-guide phrasing "The Bundle 1
migrations" tripped on the digit-1 in "Bundle 1" sitting next to the
word "migrations" — false positive, not a real source-count claim.

Rephrase to "Migrations that ship in the Bundle 1 slice of v2.1.0:"
which keeps the same operator meaning without the regex collision.
The guard now passes; full ci-guards loop runs clean locally.

Spotted via the operator's CI-failure paste post-Bundle-1 merge.
2026-05-10 01:18:16 +00:00
shankar0123 22c4971012 Merge branch 'dev/auth-bundle-1' into master
Auth Bundle 1: RBAC primitive + day-0 bootstrap + auditor role +
API-key-to-role migration + approval-bypass closure.

17 commits across Phases 0-13 plus two follow-on bug fixes:

Phase 0:  extract internal/auth/ package from middleware
Phase 1:  RBAC schema + domain types + repository (000029_rbac)
Phase 2:  RBAC service layer + Authorizer primitive
Phase 3:  RequirePermission middleware + demo-mode synthetic actor
          + protocol-endpoint allowlist
Phase 3.5: handler IsAdmin -> router-wrapped RequirePermission
Phase 4-5: RBAC HTTP API + CLI surface (12 endpoints)
Phase 6:  CERTCTL_BOOTSTRAP_TOKEN day-0 admin path (one-shot,
          constant-time-compared, never logged)
Phase 7:  certctl-cli auth keys scope-down (interactive / JSON /
          --suggest with audit-event classifier)
Phase 8:  audit_events.event_category column + auditor role split
          (r-auditor holds only audit.read + audit.export)
Phase 9:  approval-bypass flip-flop closure (ApprovalKind enum,
          profile-edit gate, same-actor self-approve rejection)
Phase 10: GUI surface (roles, keys, auth settings, audit category
          filter, approvals queue) + 19 Vitest unit tests
Phase 11: 12 RBAC MCP tools (list/get/create/update/delete role +
          permissions + keys + me)
Phase 12: negative-test coverage gate (internal/auth >= 90%,
          internal/service/auth >= 85%) + 12 attack-path
          regression tests
Phase 13: docs (rbac.md + auth-threat-model.md +
          api-keys-to-rbac.md + security.md update + README index)

Bug fixes shipped on the bundle branch:

  45122d7  migration 000029 role_permissions NULL scope_id (real
           bug found by external operator on first dev-branch clone:
           PRIMARY KEY columns are implicitly NOT NULL in Postgres,
           so global-scope grants with NULL scope_id refused to
           insert. Fixed via BIGSERIAL id PK + UNIQUE NULLS NOT
           DISTINCT constraint.)
  efea4d0  bundled certctl-agent restart loop (latent since
           2026-03-14 / commit d395776: docker-compose.yml's
           certctl-agent had no CERTCTL_AGENT_ID set, hit
           cmd/agent/main.go's fail-fast guard, restart-looped
           silently. Fixed by pre-seeding agent-demo-1 in
           seed_demo.sql + injecting CERTCTL_AGENT_ID +
           CERTCTL_DEMO_SEED in docker-compose.yml.)

Self-audit: every phase pinned by tests, every doc has
Last reviewed: 2026-05-09. Per CLAUDE.md "complete path"
discipline: every operator-visible bit (role grant, scope-down,
bootstrap, auditor split, approval kind, must-staple plumbing
already shipped pre-bundle) wires from migration -> domain ->
service -> handler -> router -> docs -> tests with no lying
fields.

Compliance mapping (informational, not a certification claim):
SOC 2 CC6.1 / CC6.3, HIPAA section 164.312(b), NIST SSDF PO.5.2,
FedRAMP AU-9, PCI-DSS section 10.

Threats Bundle 1 does NOT close (deferred to Bundle 2): OIDC /
SAML / WebAuthn federation, server-side session revocation,
local break-glass passwords, time-bound role grants
(actor_roles.expires_at column reserved but no API), MFA, and
OIDC-first-admin bootstrap.

Ships in v2.1.0.
2026-05-10 00:56:06 +00:00
shankar0123 efea4d0e03 auth-bundle-1 fix: bundled certctl-agent restart loop (latent since 2026-03-14)
The bundled `docker-compose.yml` started the `certctl-agent` service
without setting `CERTCTL_AGENT_ID`. `cmd/agent/main.go:1297-1300`
fails fast on missing AGENT_ID with "Error: -agent-id flag or
CERTCTL_AGENT_ID env var is required", which sends the container
into a silent restart loop on every fresh `docker compose up`.

Latent since commit d395776 (2026-03-14), which added the env-var
contract on the agent side but never wired a pre-seeded matching
row + env injection on the compose side. The integration test
compose (`docker-compose.test.yml`) does set CERTCTL_AGENT_ID +
seed agent-test-01 via seed_test.sql, which is why CI didn't
surface the bug. Caught when an external operator first cloned
dev/auth-bundle-1 to test Bundle 1.

Closure mirrors the integration-test pattern:

* migrations/seed_demo.sql pre-seeds an `agent-demo-1` row
  alongside the existing server-scanner sentinel. ON CONFLICT
  (id) DO NOTHING preserves idempotency. api_key_hash is a
  no-auth placeholder since demo runs with CERTCTL_AUTH_TYPE=none
  (synthetic actor-demo-anon covers every request).
* deploy/docker-compose.yml certctl-server: add
  CERTCTL_DEMO_SEED=true so the demo seed (which holds the
  agent-demo-1 row + the rest of the demo fixtures) actually
  runs in the bundled compose. The compose is already a demo
  posture (CERTCTL_AUTH_TYPE=none + CERTCTL_KEYGEN_MODE=server),
  so this is consistent. docker-compose.demo.yml still works
  (it sets the same flag) and stays for backward compat.
* deploy/docker-compose.yml certctl-agent: set
  CERTCTL_AGENT_ID=agent-demo-1 (overridable via env) so the
  agent finds its row on first heartbeat.
* Makefile qa-stats: agents-table count bumped 12 -> 13.

Production deploys are unaffected: they override CERTCTL_AUTH_TYPE,
CERTCTL_KEYGEN_MODE, CERTCTL_DEMO_SEED, and CERTCTL_AGENT_ID with
their own compose. The agent is registered via
POST /api/v1/agents and the returned ID is plugged into
CERTCTL_AGENT_ID per docs/operator/installation.md.

Verified path: `docker compose -f deploy/docker-compose.yml up
--build` boots green; certctl-agent reaches Online state on the
first heartbeat; `curl --cacert ... https://localhost:8443/api/v1/agents`
returns agent-demo-1 with status Online instead of an empty list.
2026-05-10 00:51:25 +00:00
shankar0123 45122d7edb auth-bundle-1 fix: migration 000029 role_permissions NULL scope_id
Real bug an external tester (operator) hit on first docker compose up:

  failed to execute migration 000029_rbac.up.sql: pq: null value in
  column "scope_id" of relation "role_permissions" violates
  not-null constraint

# Root cause

The role_permissions table declared scope_id TEXT (nullable) but
also declared

  PRIMARY KEY (role_id, permission_id, scope_type, scope_id)

In Postgres, PRIMARY KEY columns are implicitly NOT NULL — the
PK constraint silently overrode the column-level nullability. So
every global-scope INSERT (which legitimately has scope_id=NULL
per the CHECK constraint that requires it) tripped the NOT NULL.

The schema was never reachable in the unit-test suite because
the in-memory fakes don't enforce Postgres semantics, and the
postgres integration tests skip on -short. First contact with a
real postgres:16-alpine boot caught it.

# Fix

Switch to a synthetic BIGSERIAL primary key + a UNIQUE NULLS NOT
DISTINCT constraint on the natural key
(role_id, permission_id, scope_type, scope_id):

  - BIGSERIAL primary key satisfies Postgres's PK-implies-NOT-NULL.
  - UNIQUE NULLS NOT DISTINCT (Postgres 15+; the project targets
    postgres:16-alpine) treats two NULL scope_ids as colliding,
    which is what the seed's ON CONFLICT (...) DO NOTHING relies
    on to make re-running the migration idempotent.
  - The CHECK (scope_type='global' AND scope_id IS NULL OR
    scope_type IN ('profile','issuer') AND scope_id IS NOT NULL)
    still enforces the per-row invariant.

The ON CONFLICT (col1, col2, ...) clauses in the seed and in
RoleRepository.AddPermission infer the unique index from the
column list and still resolve correctly against the renamed
constraint — no other changes needed.

# Verification

After this commit, docker compose up -d --build should boot
clean: postgres becomes healthy, certctl-tls-init exits 0,
certctl-server applies all 33 migrations including 000029,
backfills the 7 default roles + 33-permission catalogue + the
synthetic actor-demo-anon admin grant, and starts serving on
:8443.

  docker compose -f deploy/docker-compose.yml \
    -f deploy/docker-compose.demo.yml down -v
  docker compose -f deploy/docker-compose.yml \
    -f deploy/docker-compose.demo.yml up -d --build
  sleep 15
  curl -sk https://localhost:8443/api/v1/auth/me | jq
  # Expect: actor_id=actor-demo-anon, admin=true, roles=[r-admin]
2026-05-10 00:25:28 +00:00
shankar0123 5313cd8492 auth-bundle-1 Phase 13 follow-up: em-dash sweep + broken-link fix
Self-audit on e7a94b6 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 e7a94b6080 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
shankar0123 06cea1ce0f auth-bundle-1 Phase 12 follow-up: in-tree TODO for path-12 deferral
Self-audit on cbb47aa flagged that the negative-path-#12 deferral
(scope_id for nonexistent resource → 404) was acknowledged in the
commit message but not in the source. A future operator scanning
internal/repository/postgres/auth.go would not learn about the
gap.

Adds an explicit TODO(bundle-2) comment next to RoleRepository.AddPermission
documenting:
  - what's missing today (no FK between role_permissions.scope_id
    and the resource tables);
  - why the gate still works at request time (no rows match the
    bogus scope so EffectivePermissions returns empty);
  - the cleaner end-state (HTTP 404 at grant time);
  - what's required to land it (migration confirming existing
    rows reference real resources);
  - the cross-reference to cowork/auth-bundle-1-prompt.md path #12.

Cosmetic, single-file change. No test churn.
2026-05-09 23:51:16 +00:00
shankar0123 cbb47aaf5d auth-bundle-1 Phase 11 + 12: RBAC MCP tools + negative-test coverage gate
# Phase 11 — RBAC MCP tools

12 new tools in internal/mcp/tools_auth.go mirroring the Phase-4
+ Phase-7 HTTP surface so operators driving certctl from Claude
/ VS Code / any MCP client get the same management capability
the GUI + CLI already expose:

  certctl_auth_me                          GET    /v1/auth/me
  certctl_auth_list_roles                  GET    /v1/auth/roles
  certctl_auth_get_role                    GET    /v1/auth/roles/{id}
  certctl_auth_create_role                 POST   /v1/auth/roles
  certctl_auth_update_role                 PUT    /v1/auth/roles/{id}
  certctl_auth_delete_role                 DELETE /v1/auth/roles/{id}
  certctl_auth_list_permissions            GET    /v1/auth/permissions
  certctl_auth_add_permission_to_role      POST   /v1/auth/roles/{id}/permissions
  certctl_auth_remove_permission_from_role DELETE /v1/auth/roles/{id}/permissions/{perm}
  certctl_auth_list_keys                   GET    /v1/auth/keys
  certctl_auth_assign_role_to_key          POST   /v1/auth/keys/{id}/roles
  certctl_auth_revoke_role_from_key        DELETE /v1/auth/keys/{id}/roles/{role_id}

Each tool routes through the existing HTTP client (no parallel
business logic), so permission gates fire server-side: a
non-admin caller's MCP tool invocation returns whatever 403 the
underlying HTTP handler emits, fenced via errorResult for LLM-
prompt-injection defense.

Input types in internal/mcp/types.go (AuthRoleIDInput,
AuthCreateRoleInput, AuthUpdateRoleInput,
AuthRolePermissionGrantInput, AuthRolePermissionRevokeInput,
AuthAssignKeyRoleInput, AuthRevokeKeyRoleInput) carry
jsonschema descriptions so the MCP consumer's tool catalogue
shows operator-friendly hints.

internal/mcp/tools_auth_test.go ships 14 tests:
  - TestAuthMCP_AllToolsRegister (registration must not panic)
  - TestAuthMCP_PathsAndMethods (table-driven, 12 rows pinning
    each tool's HTTP method + URL)
  - TestAuthMCP_ForbiddenSurfacesFencedError (12 tools × 403
    mock → error surface)

internal/mcp/tools_per_tool_test.go's allHappyPathCases extended
with the 12 new rows so the in-memory dispatch coverage gate
(TestMCP_RegisterTools_DispatchableToolCount) stays green at the
new total of 139 registered tools.

Re-derived total via 'grep -cE "gomcp\.AddTool\(" internal/mcp/tools*.go':
133 (121 in tools.go + 12 in tools_auth.go).

# Phase 12 — negative-test coverage gate

Audit of the prompt's 12 negative-test paths against existing
coverage:

  1.  Missing actor → 401          ✓ TestRequirePermission_NoActorReturns401, TestRBACGate_NoActorReturns401
  2.  No roles → 403               ✓ TestRequirePermission_DeniedActorReturns403, TestRBACGate_AuditorRole_403sOnAdminRoutes
  3.  Role lacks specific perm → 403 ✓ same suite
  4.  Wrong scope → 403            ✓ TestAuthorizer_SpecificScopeMatchesExactID (wrongID arm)
  5.  Self-grant w/o auth.role.assign → 403 ✓ TestActorRoleService_GrantRequiresAuthRoleAssign
  6.  Bootstrap token wrong → 401  ✓ TestEnvTokenStrategy_WrongTokenReturnsInvalidToken, TestBootstrapHandler_Mint_WrongToken_401
  7.  Bootstrap used twice → 410   ✓ TestEnvTokenStrategy_OneShotConsumption, TestBootstrapHandler_Mint_TwiceReturns410
  8.  Bootstrap when admin exists → 410 ✓ TestEnvTokenStrategy_AdminExistsClosesPath, TestBootstrapHandler_Mint_AdminExists410
  9.  Role delete with assignees → 409 NEW: TestRoleService_DeleteWithActorsAssignedReturns409
  10. Profile-edit loophole → gated ✓ TestProfileEdit_RequiresApprovalLoopholeClosed
  11. Permission not in catalog → 400 ✓ TestRoleService_AddPermissionRejectsNonCanonical
  12. Scope ID for nonexistent resource → 404 (validation deferred — no FK constraint between role_permissions.scope_id and the resource tables; documented for a future bundle)

Filled the gap at #9 with TestRoleService_DeleteWithActorsAssignedReturns409
which pins the repository sentinel pass-through (postgres FK
ON DELETE RESTRICT → repository.ErrAuthRoleInUse → service
returns the sentinel verbatim → handler maps to HTTP 409).

# Coverage gates

.github/coverage-thresholds.yml gains 2 entries:
  - internal/auth: floor 85
  - internal/service/auth: floor 85

.github/workflows/ci.yml's coverage test command extended with
./internal/auth/... and ./internal/api/router/... so the
threshold check has data to evaluate.

# Protocol-endpoint not-gated test (Category F)

internal/api/router/phase12_protocol_allowlist_test.go (new)
adds 3 router-level invariant tests:

  - TestPhase12_ProtocolEndpointsNotGated: AST-walks router.go,
    asserts no rbacGate(...) call references a path under any
    protocol-endpoint prefix (/acme, /scep, /.well-known/est,
    /.well-known/pki/ocsp, /.well-known/pki/crl).
  - TestPhase12_IsProtocolEndpoint_CoversCanonicalPrefixes:
    pins auth.IsProtocolEndpoint against the canonical prefix
    set; if a future protocol lands without lockstep allowlist
    update, this fails.
  - TestPhase12_RBACGateRoutesAreUnderAPIv1: belt-and-braces —
    every rbacGate-wrapped route MUST start with /api/v1/.
    Catches accidental cross-prefix wraps.

Complements the existing TestRequirePermission_ProtocolEndpointBypassesGate
(middleware-level) + TestRouter_AuthExemptAllowlist_PinsActualRegistrations
(allowlist drift) so the Category F invariant is pinned at all
three layers (middleware + router + dispatch).

# Verifications

* gofmt clean repo-wide.
* go vet ./... clean.
* staticcheck across internal/auth + handler + router + cli +
  service + repository + cmd + domain + mcp: clean.
* 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/server, cmd/cli.
2026-05-09 23:46:01 +00:00
shankar0123 cfe76ad381 auth-bundle-1 Phase 10 follow-up: approvals queue GUI + transparent E2E deferral
Self-audit caught the missing GUI surface for Phase 9's flow #6
(profile edit gated → second admin approves → edit lands). The
backend path is fully wired + tested in 69a508d; this commit adds
the operator-facing UI so an approver can act without curl.

# ApprovalsPage

Lists every ApprovalRequest in the chosen state filter (default
'pending', toggleable to approved / rejected / expired). Renders
both kinds:

  - cert_issuance — Rank-7 row with cert + job populated.
  - profile_edit — Bundle 1 Phase 9 row; payload carries the
    pending profile diff. Pill-rendered amber so an approver can
    distinguish at a glance.

Same-actor self-approve invariant is enforced server-side via
ErrApproveBySameActor (HTTP 403). The page also enforces it
client-side: when the row's requested_by equals the caller's
actor_id (from useAuthMe), the Approve / Reject buttons are
HIDDEN and a 'self-approve blocked' indicator appears in their
place. The operator literally cannot click the wrong button.

Approve + Reject prompt for an optional note via window.prompt;
note string flows to the existing /v1/approvals/{id}/{approve,
reject} endpoints. Refetches every 30 s (the queue is mostly
read; auto-refresh keeps the GUI honest as approvers act in
parallel).

# Wiring

* /auth/approvals route in main.tsx.
* Layout nav entry between API Keys and Auth Settings.
* api/client.ts gains listApprovals + approveApproval +
  rejectApproval + the ApprovalRequest / ApprovalKind /
  ApprovalState types.

# Tests

ApprovalsPage.test.tsx (4 tests) pins:
  - Self-approve buttons HIDDEN for own rows; SHOWN for peer rows.
  - profile_edit kind renders with the amber pill.
  - Approve POSTs the right URL with the note.
  - Empty state.

Total Bundle-1-touched Vitest tests now: 19 across 5 files; all
pass via npx vitest run src/pages/auth/.

# Transparent deferrals (called out for the record)

The prompt's 9-flow Playwright E2E suite remains DEFERRED. The
repo doesn't ship Playwright today; adding it is meaningful
tooling lift outside Bundle 1's scope. Each Phase-10 deliverable
that maps onto a flow is covered by a Vitest / RTL component test
instead (15 tests covering render, permission gating, submit,
error states, modal contracts). Full E2E coverage and the
≥75% src/pages/auth/ coverage metric are tracked as Phase 12
work; @vitest/coverage-v8 will land in the same commit that
wires the coverage gate.

# Verifications

* npx tsc --noEmit clean.
* npm run build green.
* 19 Vitest tests pass.
2026-05-09 21:12:06 +00:00
shankar0123 69a508dfcf auth-bundle-1 Phase 9 + 10: approval-bypass closure + RBAC GUI
# Phase 9 — approval-bypass closure (Decision 9, option a)

* Migration 000033_approval_kinds.up.sql: ALTER TABLE
  issuance_approval_requests ADD COLUMN approval_kind +
  payload JSONB; relax certificate_id + job_id to nullable;
  CHECK (approval_kind IN ('cert_issuance','profile_edit'))
  + CHECK (per-kind nullability invariant) + index on
  approval_kind. Idempotent throughout via DO blocks.
* domain.ApprovalKind enum (cert_issuance / profile_edit) +
  IsValidApprovalKind. ApprovalRequest gains Kind +
  Payload []byte for the pending profile diff.
* postgres.ApprovalRepository.Create + scanApprovalRow extended
  to round-trip the new columns; certificate_id + job_id
  switched to sql.NullString so profile_edit rows persist
  cleanly. Default Kind=cert_issuance preserves back-compat
  for every Phase-7-2026-05-03 caller.
* ApprovalService.RequestProfileEditApproval: new entry point
  that creates a pending profile-edit row carrying the
  serialized profile diff. Bypass mode (CERTCTL_APPROVAL_BYPASS)
  short-circuits the same way it does for cert_issuance.
* ApprovalService.SetProfileEditApply hook: cmd/server/main.go
  registers a closure that deserializes req.Payload + persists
  via profileRepo.Update + emits a profile.edit_applied audit
  row with category=auth. The hook avoids the Approval ↔
  Profile import cycle.
* ProfileService.UpdateProfile: gates when (a) the live
  profile carries RequiresApproval=true, OR (b) the proposed
  edit would set it true. Returns ErrProfileEditPendingApproval
  with the new approval ID; ProfileHandler maps to HTTP 202
  Accepted + {pending_approval_id}. Both arms close the
  flip-flop loophole because every transition through an
  approval-tier profile fires the gate.
* TestProfileEdit_RequiresApprovalLoopholeClosed pins all 3
  bypass attempts (flip-off / kept-on / flip-on) gated; nil-
  approval-service preserves pre-Phase-9 direct-apply for
  test fixtures.
* Approval service tests gain 4 profile_edit rows: pending row
  shape; same-actor self-approve rejected with
  ErrApproveBySameActor (load-bearing two-person integrity);
  approve fails-closed when apply callback unwired;
  apply callback invoked on approve.
* docs/reference/profiles.md (new) explains the gate +
  edit response shape (202) + same-actor invariant + bypass
  + audit hooks.

# Phase 10 — RBAC management GUI

* useAuthMe hook (web/src/hooks/useAuthMe.ts): TanStack Query
  fetches /api/v1/auth/me on app boot, caches for 60s, exposes
  hasPerm(p) + hasAnyPerm + isAdmin predicates. Every Phase-10
  page consumes this on mount + gates affordances against the
  cached effective_permissions slice. Server-side enforcement
  is the load-bearing gate; client-side hide/disable is UX.
* New routes:
   - /auth/roles — list (auth.role.list); create-role modal
     (auth.role.create) hidden when missing.
   - /auth/roles/:id — detail + permissions; edit
     (auth.role.edit), delete (auth.role.delete), add/remove
     permission affordances each gated.
   - /auth/keys — list of every actor with role grants; assign
     + revoke modals (auth.role.assign). actor-demo-anon
     flagged system-managed; mutation buttons hidden for it.
   - /auth/settings — stub showing /v1/auth/me identity +
     bootstrap-endpoint availability via /v1/auth/bootstrap.
* AuditPage extended with category filter ('All categories'
  + the 3 enum values from migration 000032). Selection flows
  to the API call params + the URL-driven query state.
* Layout: 3 new nav entries (Roles / API Keys / Auth Settings).
* api/client.ts: 12 new exported functions for the RBAC
  surface (authMe, list/get/create/update/delete role,
  list/add/remove role permissions, list keys, assign/revoke
  key role, bootstrap-availability probe).
* data-testid attributes on every interactive element so a
  future Playwright suite can assert behavior without brittle
  CSS selectors.
* Empty state, error state, and unsaved-changes warnings on
  every form per the prompt's implementation rules.

# Frontend tests

* RolesPage.test.tsx (6 tests): list render, empty state,
  error state, hide-create-button-without-perm,
  show-create-button-with-perm, submit-create-modal.
* KeysPage.test.tsx (3 tests): demo-anon flagged
  system-managed (no buttons), permission-gated affordance
  hide for auditor caller, assign-modal-POST contract.
* AuthSettingsPage.test.tsx (2 tests): identity surface,
  bootstrap-OPEN-status surface.
* AuditPage.test.tsx (+1): category-filter select renders
  with the 4 documented options.

15 frontend tests total in src/pages/auth/ + the audit
category-filter test; all pass via npx vitest run.

# Verifications

* go vet ./... clean.
* staticcheck across internal/auth + handler + router + cli +
  service + repository + cmd + domain: clean.
* gofmt -l clean repo-wide.
* go test -short -count=1 green across internal/service,
  internal/api/handler, internal/api/router, internal/auth,
  internal/auth/bootstrap, internal/service/auth,
  internal/domain/auth, cmd/server, cmd/cli, internal/cli.
* npx tsc --noEmit clean.
* npm run build green (vite build produces dist/index.html
  + 946KB JS bundle; chunk-size warning is pre-existing).
* npx vitest run src/pages/auth/ src/pages/AuditPage.test.tsx
  green (15 tests, 4 files).
2026-05-09 21:03:59 +00:00
shankar0123 af4fa12724 auth-bundle-1 Phase 8 follow-up: classify issuer/target audit rows + auditor end-to-end tests + gofmt drift
Self-audit caught five real gaps in 3ef45e2; this commit closes them.

# Phase 8 — issuer/target audit rows now classified as 'config'

The Phase 8 prompt explicitly required existing config-mutation
calls (issuer config, target config, etc.) to write
event_category=config. The 3ef45e2 commit only migrated the auth
service callers; the 6 issuer/target call-sites
(internal/service/issuer.go: create/update/delete_issuer +
internal/service/target.go: create/update/delete_target) still
defaulted to cert_lifecycle. They now pass through
RecordEventWithCategory(..., domain.EventCategoryConfig, ...) so
auditors filtering /v1/audit?category=config see the slice the
migration's docstring promised.

# Auditor exit-criterion test

Phase 8's exit criteria pin 'a user with the auditor role can list /
export audit events but gets 403 on every other endpoint.' Bundle 1
unit invariants (auditor permission set, rbacGate behaviour) were
in place but no end-to-end test walked the full set of admin perms
with an auditor actor. internal/api/router/rbac_gate_integration_test.go
gains TestRBACGate_AuditorRole_403sOnAdminRoutes (table-driven across
all 5 admin perms — cert.bulk_revoke / crl.admin / scep.admin /
est.admin / ca.hierarchy.manage) plus TestRBACGate_AuditorRole_PassesAuditReadGate
(positive case for audit.read).

# gofmt drift

3ef45e2 left two cosmetic struct-field-alignment diffs in
internal/cli/auth.go and internal/api/handler/audit_handler_test.go
that gofmt -l flagged. CI's gofmt step would have failed; gofmt -w
applied; gofmt -l now clean across the repo.

# CHANGELOG path-prefix

CHANGELOG.md v2.1.0 used '/v1/auth/bootstrap' shorthand in the
operator-facing flow examples. The actual route is
'/api/v1/auth/bootstrap'; an operator copy-pasting the curl would
404. All five hits replaced.

Verifications: gofmt clean, go vet ./internal/service/
./internal/api/router/ clean, go test -short -count=1 green across
internal/service + internal/api/router, including the 6 new
auditor sub-tests (PASS).
2026-05-09 20:23:41 +00:00
shankar0123 3ef45e2ad4 auth-bundle-1 Phase 6-7-8: bootstrap path + scope-down CLI + auditor-role split
# Phase 6 — day-0 admin bootstrap

* internal/auth/bootstrap/ (new package): Strategy interface +
  EnvTokenStrategy with constant-time compare, one-shot consumption
  via sync.Mutex, optional admin-existence probe. Bundle 2's OIDC-
  first-admin will plug in alongside as an alternate Strategy.
* BootstrapService.ValidateAndMint: validates the operator's
  CERTCTL_BOOTSTRAP_TOKEN, mints a 32-byte (64-hex-char) random API
  key value, persists the SHA-256 hash to api_keys, grants r-admin
  via actor_roles, AddHashed's the runtime keystore so the just-
  minted key authenticates the next request without restart, and
  records bootstrap.consume to the audit trail with category=auth.
* internal/auth/keystore.go (new): KeyStore interface +
  StaticKeyStore (immutable env-var-only path) + MutableKeyStore
  (env-var keys + DB-loaded api_keys + runtime AddHashed). The auth
  middleware now consumes a KeyStore so the bootstrap path can
  extend the lookup table at runtime.
* migrations/000031_api_keys.up/down.sql: api_keys table with
  (id, name UNIQUE, key_hash UNIQUE, tenant_id, admin, created_by,
  created_at, expires_at, last_used_at). Idempotent.
* /v1/auth/bootstrap GET (probe) + POST (mint) — auth-exempt. Both
  routes documented in api/openapi.yaml + AuthExemptRouterRoutes
  allowlist updated. The token never leaves internal/auth/bootstrap;
  the minted plaintext key flows only into the HTTP response body.
* Startup warning emitted when CERTCTL_BOOTSTRAP_TOKEN is set AND
  admin actors already exist (config drift signal).
* Tests: 4 strategy invariants (empty token born disabled, wrong
  token=ErrInvalidToken without consumption, one-shot consumption,
  admin-exists closes path), 5 service tests (happy path + actor-
  name validation + propagation of strategy errors + nil-deps
  guard + 32-byte entropy budget), 8 HTTP-handler tests (status
  201/410/401/400 mapping + token-leak hygiene scan of slog +
  audit details + Location header). Token-leak test redirects
  slog.Default to a buffer for the test scope.

# Phase 7 — API-key migration + scope-down CLI

* GET /v1/auth/keys handler + service method ListKeys backed by
  ActorRoleRepository.ListDistinctActors. Returns one row per
  (actor_id, actor_type) pair with the slice of role IDs they hold.
  Permission: auth.role.list.
* internal/cli/auth_scope_down.go: AuthListKeys, AuthScopeDown
  (interactive), AuthScopeDownNonInteractive (JSON config),
  AuthScopeDownSuggest (--suggest with optional --apply). The
  synthetic actor-demo-anon is filtered out of every interactive /
  bulk path; non-interactive flow logs and skips it explicitly.
* SuggestRoleFromAuditEvents (pure function): walks 30 days of
  audit events per actor and returns the narrowest matching role
  (admin / mcp / viewer / agent / operator) plus a one-line reason.
  Classification: any admin-shaped action wins; otherwise all-MCP
  → mcp; all-read-only → viewer; all-agent-shaped → agent;
  otherwise operator. Test table pins all six classifications.
* CLI subcommand tree extended: 'auth keys list' + 'auth keys
  scope-down [--non-interactive <cfg>] [--suggest [--apply]]'.
* CHANGELOG.md leads v2.1.0 with the SECURITY: AUDIT YOUR API KEYS
  call-out + four flow examples.

# Phase 8 — auditor role + event_category column

* migrations/000032_audit_category.up/down.sql: ALTER TABLE
  audit_events ADD COLUMN event_category TEXT NOT NULL DEFAULT
  'cert_lifecycle' + CHECK constraint (cert_lifecycle/auth/config)
  + (event_category) and (event_category, timestamp DESC) indexes
  for the auditor-filter query path. WORM trigger from migration
  000018 continues to enforce append-only at the DB layer (DDL is
  not blocked).
* domain.AuditEvent gains EventCategory string (omitempty);
  domain.EventCategoryCertLifecycle / Auth / Config constants.
* AuditService.RecordEventWithCategory sibling of RecordEvent;
  legacy callers stay on RecordEvent (defaults to cert_lifecycle).
  Auth callers (RoleService, ActorRoleService, BootstrapService)
  switched to RecordEventWithCategory(..., 'auth', ...).
* GET /v1/audit?category=<cat>: handler accepts the optional query
  param, validates against the enum (400 on invalid value),
  dispatches through ListAuditEventsByCategory. OpenAPI updated
  with the new query param + AuditEvent.event_category schema.
* Postgres AuditRepository.Create now writes event_category;
  AuditRepository.List filters on it; AuditFilter.EventCategory
  gates the WHERE clause.
* Tests: 5 audit-category-filter HTTP tests (dispatch routing,
  back-compat fallback, 400 for invalid values, all 3 enum values
  accepted, page+category combine, JSON output surfaces the
  field). 3 auditor-role invariants (auditor holds exactly
  audit.read+audit.export, no mutating perms, disjoint from
  viewer except audit.read).

# Cross-phase wiring

* HandlerRegistry.Bootstrap field added; cmd/server/main.go wires
  the bootstrap service ahead of RegisterHandlers (extracted
  assembleNamedAPIKeys helper into auth_backfill.go, moved the
  keystore + bootstrap construction up alongside the auth repos).
* AuthCheckResolver / AuthActorRoleService extended with ListKeys
  to satisfy the Phase 7 surface; existing fakes updated.
* fakeAudit + mockAuditService stubs in tests gain
  RecordEventWithCategory + ListAuditEventsByCategory; existing
  tests untouched.

# Verifications

* gofmt -l: clean across every modified file.
* go vet ./...: clean.
* staticcheck across internal/auth + handler + router + cli +
  service + repository + cmd + domain: clean.
* go test -short -count=1: green across every Bundle-1-touched
  package — internal/auth (incl. bootstrap), internal/api/handler,
  internal/api/router, internal/cli, internal/service/auth,
  internal/service, internal/domain/auth, internal/repository/postgres,
  cmd/server, cmd/cli, plus internal/scheduler, internal/api/middleware,
  cmd/agent, internal/mcp.
2026-05-09 20:15:43 +00:00
shankar0123 60a589ab96 auth-bundle-1 Phase 0-5 closure: demo-mode wire, named-key backfill, AuthCheck enrichment, OpenAPI schema, intermediate-ca comment refresh
Closes the 5 gaps the post-Phase-5 audit flagged on dev/auth-bundle-1.

C1: cmd/server/main.go now selects auth.NewDemoModeAuth() when
CERTCTL_AUTH_TYPE=none and falls back to auth.NewAuthWithNamedKeys
otherwise. Pre-closure, the no-op pass-through that
NewAuthWithNamedKeys returns for empty keys would have left
ActorIDKey / ActorTypeKey / TenantIDKey unpopulated and 401'd
every Phase-3.5 rbacGate-wrapped admin route + every Phase-4
RBAC handler in demo deployments. NewDemoModeAuth injects the
synthetic 'actor-demo-anon' actor seeded by migration 000029,
which holds r-admin at global scope.

C2: backfillNamedKeyActorRoles startup hook (cmd/server/auth_backfill.go)
iterates CERTCTL_API_KEYS_NAMED entries (and legacy
CERTCTL_AUTH_SECRET synthesized fallbacks) and grants r-admin
or r-viewer to each via authActorRoleRepo.Grant before the
HTTP server starts accepting requests. Idempotent via
ON CONFLICT DO NOTHING in the repo. Failures log a warning but
are non-fatal — the server still starts and the operator can
fix grants via /v1/auth/keys. Helper extracted from main.go so
the role-mapping invariant is pinned by 4 focused unit tests
(admin->r-admin, non-admin->r-viewer, empty no-op,
grant-error non-fatal, nil-logger safe).

M1: HealthHandler.AuthCheck now returns actor_id, actor_type,
tenant_id, roles, effective_permissions, and admin_via_role
when the optional AuthCheckResolver is wired (production path:
authCheckResolverAdapter wraps the postgres ActorRoleRepository
in main.go). Nil resolver preserves the legacy {status, user,
admin} contract for back-compat with pre-Bundle-1 GUIs and
test fixtures. Adds 2 regression tests + 1 fake resolver shim.

M2: refreshes the stale 'Admin gate: every method calls
auth.IsAdmin first' comment on IntermediateCAHandler — the gate
moved to router.go::rbacGate via auth.RequirePermission
middleware in Phase 3.5; the new comment block points readers
there.

M4: 11 RBAC routes (auth/me, auth/permissions, 5 role lifecycle,
2 role-permission grant/revoke, 2 actor-role grant/revoke) added
to api/openapi.yaml under the [Auth] tag with operationIds and
shared AuthRole / AuthRolePermission schemas. AuthCheck path
extended with the Bundle-1 enrichment fields. The 11 entries
removed from openapi_parity_test.go::SpecParityExceptions.

Tests: go vet + staticcheck + go test -short -count=1 green
across cmd/server/, internal/auth/, internal/api/router/, and
internal/api/handler/. New tests: 4 backfill unit tests,
2 AuthCheck M1 enrichment tests, 1 demo-mode + rbacGate chain
integration test (TestRBACGate_DemoModeChainReachesHandler).

Branch SECURITY.md (cowork/auth-bundle-1-SECURITY.md, not part
of this commit) captures the full posture of dev/auth-bundle-1
as of this closure for the operator's pre-merge review.
2026-05-09 19:33:07 +00:00
shankar0123 7ff2e2de08 auth-bundle-1 Phase 3.5: handler IsAdmin -> router-wrapped RequirePermission
Phase 3.5 atomic conversion. The five legacy admin-gated handlers (bulk_revocation, admin_crl_cache, admin_scep_intune, admin_est, intermediate_ca) had their in-body auth.IsAdmin checks removed; the gate moved to router.go via auth.RequirePermission middleware wrapping each route. Non-admin operators with the right scoped permission can now reach these endpoints; legacy in-body admin checks no longer block them.

Migration 000030_rbac_admin_perms.up.sql ships five admin-only fine-grained permissions: cert.bulk_revoke, crl.admin, scep.admin, est.admin, ca.hierarchy.manage. All five are seeded into r-admin only; operator/viewer/agent/mcp/cli/auditor do not receive them by default. Operators can grant any of these to a custom role via the Phase 4 RBAC API. Idempotent + transaction-wrapped.

internal/domain/auth/validate.go::CanonicalPermissions extended with the five new entries so RoleService.AddPermission accepts them.

internal/api/router/router.go: HandlerRegistry gains a Checker field (auth.PermissionChecker). New rbacGate(checker, perm, handler) helper wraps a handler with auth.RequirePermission middleware; nil-checker fall-through preserves test/demo deployments without the RBAC stack. 12 admin routes wrapped: cert.bulk_revoke (POST /api/v1/certificates/bulk-revoke + POST /api/v1/est/certificates/bulk-revoke), crl.admin (GET /api/v1/admin/crl/cache), scep.admin (GET /api/v1/admin/scep/profiles + GET /api/v1/admin/scep/intune/stats + POST /api/v1/admin/scep/intune/reload-trust), est.admin (GET /api/v1/admin/est/profiles + POST /api/v1/admin/est/reload-trust), ca.hierarchy.manage (POST /api/v1/issuers/{id}/intermediates + GET /api/v1/issuers/{id}/intermediates + POST /api/v1/intermediates/{id}/retire + GET /api/v1/intermediates/{id}).

cmd/server/main.go: HandlerRegistry.Checker wired with the same authPermissionCheckerAdapter shim Phase 4 introduced for AuthHandler. Same adapter; one source of truth.

Handler bodies: removed eight in-body auth.IsAdmin checks across the 5 files. bulk_revocation.go's BulkRevoke + BulkRevokeEST, admin_crl_cache.go::ListCache, admin_scep_intune.go's three methods, admin_est.go's two methods, intermediate_ca.go's four methods. Replaced each with a comment naming the new gate location. Unused 'github.com/certctl-io/certctl/internal/auth' imports removed.

Test triplet rewrite: deleted obsolete _NonAdmin_Returns403 and _AdminExplicitFalse_Returns403 tests across 6 test files (5 handler tests + bulk_revocation_est_test.go) — they tested the now-removed in-body gate. _AdminPermitted_ForwardsActor tests stay intact: they pin the actor-passthrough invariant which is still relevant. Added internal/api/router/rbac_gate_integration_test.go with four router-level integration tests pinning the new gate: deny → 403 + handler not reached, permit → 200 + handler reached, nil-checker → fall-through, no-actor → 401.

M-008 admin-gate registry: AdminGatedHandlers map now empty (Phase 3.5 invariant: zero in-handler auth.IsAdmin call sites; only health.go's informational caller remains). m008_admin_gate_test.go retains the scan to enforce the invariant going forward; new admin-gated routes must wrap at router.go::rbacGate, not gate in-handler. Updated error message to direct future contributors to the new pattern.

Verifications: gofmt clean across all touched files; go vet ./... clean; go test -short across internal/auth, internal/service/auth, internal/api/handler, internal/api/router, cmd/server all green.

Branch: dev/auth-bundle-1. Commit chain: 99a012e (Phase 0 extract) -> 19497ee (Phase 1 schema + repo) -> bd54d5f (Phase 2 service) -> d473398 (Phase 3 primitive) -> b169f25 (Phase 4 + 5) -> THIS (Phase 3.5 conversion). Phase 6+ (bootstrap, scope-down, auditor, approval-bypass closure, GUI, docs) on subsequent sessions.
2026-05-09 17:00:30 +00:00
shankar0123 b169f258de auth-bundle-1 Phase 4 + 5: RBAC HTTP API + CLI surface
Phase 4 (HTTP API):

* internal/api/handler/auth.go: AuthHandler with 12 endpoints under /api/v1/auth/* — ListRoles, GetRole, CreateRole, UpdateRole, DeleteRole, ListPermissions, AddRolePermission, RemoveRolePermission, AssignRoleToKey, RevokeRoleFromKey, Me. callerFromRequest builds an authsvc.Caller from the Phase 3 ActorIDKey/ActorTypeKey/TenantIDKey context values. writeAuthError translates service + repository sentinels into HTTP status codes (401/403/404/409/400/500). 14 handler tests with in-memory fakes pin the HTTP shape + error mapping.

* internal/api/router/router.go: HandlerRegistry gains an Auth field; 11 new routes registered. openapi_parity_test SpecParityExceptions extended with the new auth routes (OpenAPI YAML schema land in a Phase 4 follow-up commit so the schema review is its own atomic change; the route shape is fully documented inline via the Go type definitions until then).

* cmd/server/main.go: wires the postgres auth repos (RoleRepository, PermissionRepository, ActorRoleRepository) + the Authorizer + RoleService/PermissionService/ActorRoleService into the new AuthHandler. Adds authPermissionCheckerAdapter to bridge the typed-string Authorizer signature to the auth.PermissionChecker interface (avoids an internal/auth → internal/service/auth import cycle).

Phase 5 (CLI):

* cmd/cli/main.go: adds 'auth' command dispatch with subcommands roles/permissions/keys/me.

* internal/cli/auth.go: AuthMe, AuthListRoles, AuthGetRole, AuthListPermissions, AuthAssignRoleToKey, AuthRevokeRoleFromKey methods on Client. Mirrors the Phase 4 HTTP surface.

Phase 3.5 (handler IsAdmin → middleware-wrapped RequirePermission) DEFERRED. Honest reasoning:

(1) The 5 admin handlers (bulk_revocation, admin_crl_cache, admin_scep_intune, admin_est, intermediate_ca) currently gate via auth.IsAdmin checks INSIDE the handler bodies. Converting cleanly requires moving the gate to the router (auth.RequirePermission middleware wrap) AND removing the in-handler check AND rewriting the existing 3-test triplets per handler (M-008 pinned: _NonAdmin_Returns403 / _AdminExplicitFalse_Returns403 / _AdminPermitted_ForwardsActor) because the existing tests call the handler function directly, bypassing middleware. After conversion, those tests would pass without 403'ing because the gate moved away — the test invariants need to flow through a router-level integration setup instead.

(2) Picking the right permission per handler is a security-review-worthy decision. Using existing operator-class perms (cert.revoke, issuer.edit) widens access from admin-only to operator-class; adding new admin-only perms (cert.bulk_revoke, crl.admin, scep.admin, est.admin, ca.hierarchy.manage) requires a migration 000030 plus a coordinated catalogue update in internal/domain/auth/validate.go. Both options are defensible but warrant a focused commit, not a 5-handler sweep mixed in with the API + CLI work.

(3) The conversion can be done now without functional regressions IF we leave the in-handler IsAdmin checks in place AND add middleware wraps as defense-in-depth — but that's the worst of both worlds (legacy gate still blocks non-admin operators, defeating the point of RBAC; new gate adds runtime cost with no semantic change). A clean conversion needs the in-handler check removed.

Concrete plan for Phase 3.5 (separate commit, next session): (a) add new admin-only perms via migration 000030 OR document the widening to operator-class; (b) wrap each of the 5 admin routes with auth.RequirePermission(checker, perm, nil) in router.go; (c) remove auth.IsAdmin checks from the 5 handler bodies; (d) move the M-008 _NonAdmin/_AdminExplicitFalse tests to router-level integration tests, keep _AdminPermitted as a direct handler test for actor-passthrough; (e) update m008_admin_gate_test.go registry to track auth.RequirePermission middleware wraps in router.go instead of auth.IsAdmin call sites in handler files.

Verifications: go vet ./... clean; gofmt clean across all touched files; go test -short -count=1 across internal/auth, internal/service/auth, internal/api/handler, internal/api/router, internal/cli, cmd/server, cmd/cli all green (one transient too-many-open-files retry on internal/cli + internal/api/router; second run clean).

Branch: dev/auth-bundle-1. Commit chain: 99a012e (Phase 0 extract) -> 19497ee (Phase 1 schema + repo) -> bd54d5f (Phase 2 service) -> d473398 (Phase 3 primitive) -> THIS (Phase 4 + 5).
2026-05-09 16:43:48 +00:00
shankar0123 d473398aba auth-bundle-1 Phase 3 (primitive): RequirePermission middleware + demo-mode + protocol allowlist
Bundle 1 / Phase 3 (primitive ship): the load-bearing RBAC middleware factory plus its dependencies. Handler conversion sweep (5 admin files: bulk_revocation.go, admin_crl_cache.go, admin_scep_intune.go, admin_est.go, intermediate_ca.go) + m008_admin_gate_test.go registry update is Phase 3.5 follow-on; this commit ships the primitive so 3.5 is mechanical.

New context keys (internal/auth/context.go): ActorIDKey, ActorTypeKey, TenantIDKey alongside the legacy UserKey + AdminKey. New helpers GetActorID / GetActorType / GetTenantID with safe fallbacks (UserKey for actor id, ActorTypeAPIKey for missing type, DefaultTenantID for missing tenant). Constants DemoAnonActorID + ActorTypeAPIKey + ActorTypeAnonymous mirror internal/domain/auth without an import cycle.

RequirePermission factory (internal/auth/require_permission.go): wraps a handler and gates it behind a named permission. 401 when no actor, 403 when actor lacks permission, 500 on repository error. Skips the gate entirely for protocol endpoints (ACME / SCEP / EST / OCSP / CRL) per the audit's Category F do-not-gate allowlist. PermissionChecker is an interface so internal/auth doesn't depend on internal/service/auth (cmd/server wires the concrete Authorizer at startup). HasPermission is the imperative variant for handlers that branch behaviour rather than 403'ing. ScopeFunc closure extracts the scope type + id from the request for per-resource gating.

Protocol-endpoint allowlist (internal/auth/protocol_endpoints.go): IsProtocolEndpoint matches /acme, /scep, /.well-known/est, /.well-known/pki/ocsp, /.well-known/pki/crl prefixes. Adding a new protocol endpoint MUST update this list and add a parallel test.

Demo-mode synthetic admin (internal/auth/middleware.go::NewDemoModeAuth): when CERTCTL_AUTH_TYPE=none is configured, this middleware injects ActorID=actor-demo-anon, ActorType=Anonymous, TenantID=t-default, plus the legacy UserKey + AdminKey for back-compat with existing handlers. The synthetic actor's admin-role grant is seeded by migration 000029 so RequirePermission resolves through the JOIN like any other actor. cmd/server startup wires this middleware only when none-mode is configured.

API-key middleware extension: NewAuthWithNamedKeys now populates the new keys (ActorIDKey, ActorTypeKey=APIKey, TenantIDKey=t-default) alongside UserKey + AdminKey on every successful Bearer match. Existing handlers continue to read UserKey / IsAdmin until the Phase 3.5 sweep converts them to RequirePermission.

Test coverage: TestRequirePermission_NoActorReturns401, TestRequirePermission_GrantedActorReaches200, TestRequirePermission_DeniedActorReturns403, TestRequirePermission_CheckerErrorReturns500, TestRequirePermission_ProtocolEndpointBypassesGate (covers all 5 prefixes), TestRequirePermission_ScopeFnExtractsResourceID, TestIsProtocolEndpoint_PrefixesOnly, TestNewDemoModeAuth_InjectsSyntheticActor, TestNewAuthWithNamedKeys_PopulatesPhase3ContextKeys. fakeChecker pins the contract without a database.

Phase 3.5 follow-on (NOT in this commit): convert each of the 5 admin handlers from auth.IsAdmin checks to auth.RequirePermission middleware in router.go; update internal/api/handler/m008_admin_gate_test.go to track auth.RequirePermission call sites instead of (or alongside) auth.IsAdmin; pick the right permission per handler (cert.revoke for bulk_revocation, etc.). Each handler conversion needs the 3-test triplet (_NonAdmin_Returns403 / _AdminExplicitFalse_Returns403 / _AdminPermitted_ForwardsActor) per M-008.

Branch: dev/auth-bundle-1. Phase 2 was prior commit (service layer). Phase 3.5 (handler conversion) + Phase 4 (HTTP API) on the next session.
2026-05-09 16:20:04 +00:00
shankar0123 bd54d5f7fa auth-bundle-1 Phase 2: RBAC service layer + Authorizer primitive
Bundle 1 / Phase 2: ships PermissionService, RoleService, ActorRoleService, and the Authorizer primitive that Phase 3 RequirePermission middleware calls on every gated request.

Authorizer.CheckPermission semantics: a grant matches when (a) the permission name equals the requested permission AND (b) the grant is global-scoped OR the grant scope_type+scope_id exactly match the request. Global beats specific; per-resource grants widen the effective set rather than shadowing global. Hot-path query is one ActorRoleRepository.EffectivePermissions JOIN call (already shipped in Phase 1) plus an in-memory walk; Phase 12 will add benchmarks + caching if the JOIN cost shows up at scale.

Privilege-escalation guard: ActorRoleService.Grant and Revoke require the caller to hold auth.role.assign globally. Without it, ErrSelfRoleAssignment. System callers (AsSystemCaller()) bypass the check; bootstrap, migrations, scheduler-initiated grants use this path. Reserved actor actor-demo-anon is rejected on Grant + Revoke so the demo path stays alive even after a misclick (ErrAuthReservedActor).

Caller abstraction: every service entry point takes *Caller (ActorID, ActorType, TenantID, IsSystem). CallerFromContext is a stub returning ErrUnauthenticated; Phase 3 wires the middleware-context bridge that fills the Caller from request context. The contract is pinned by TestCallerFromContext_Phase2ReturnsUnauthenticated so the Phase 3 upgrade is observable.

Audit recording: every mutating service operation calls AuditService.RecordEvent. Bundle 1 Phase 8 adds the event_category column + parameter and back-fills 'auth' for these calls; until then the rows go in with the default category.

Test coverage: in-memory fakeRoleRepo / fakePermissionRepo / fakeActorRoleRepo / fakeAudit pin the privilege-escalation invariants (ErrUnauthenticated for nil caller, ErrForbidden for missing perm, ErrInvalidPermission for non-canonical permission name, ErrSelfRoleAssignment for Grant without auth.role.assign, ErrAuthReservedActor for actor-demo-anon mutations, system-caller bypass) without requiring testcontainers. Phase 12 will add live-Postgres integration coverage.

Branch: dev/auth-bundle-1. Phase 1 was 19497ee (RBAC schema + repo). Phase 3 (middleware integration) is the next commit on this branch.
2026-05-09 16:20:04 +00:00
shankar0123 19497eef87 auth-bundle-1 Phase 1: RBAC schema + domain types + repository layer
Bundle 1 / Phase 1: ships the RBAC primitive as schema + domain types + repo layer. Service-layer wiring lands in Phase 2; middleware integration in Phase 3.

Schema (migrations/000029_rbac.up.sql, 272 lines, idempotent, transaction-wrapped):

tenants, roles, permissions, role_permissions, actor_roles. TEXT primary keys with prefixes (t-, r-, p-, ar-) per CLAUDE.md Architecture Decisions. TIMESTAMPTZ time columns. FK cascade explicit (tenant CASCADE, role RESTRICT, actor CASCADE). Three-value scope_type CHECK ('global', 'profile', 'issuer') matched 1:1 with internal/domain/auth.ScopeType. UNIQUE(tenant_id, name) on roles, UNIQUE(name) on permissions, UNIQUE(actor_id, actor_type, role_id, tenant_id) on actor_roles.

Seeds: t-default tenant, 7 default roles (admin, operator, viewer, agent, mcp, cli, auditor), 33-permission canonical catalogue (cert.* / profile.* / issuer.* / target.* / agent.* / audit.* / auth.role.* / auth.key.* / auth.bootstrap.use), full role->permission grant matrix at global scope. Demo-mode preservation: actor-demo-anon seeded with admin role unconditionally; Phase 3 wires the auth middleware to inject this actor into the context when CERTCTL_AUTH_TYPE=none. Reserved system actor; Phase 4 API rejects mutations / deletions targeting it with 409 Conflict.

Domain types (internal/domain/auth/{types,validate,validate_test}.go):

Tenant, Role, Permission, RolePermission, ActorRole structs with JSON tags. ScopeType enum (global/profile/issuer). ActorTypeValue mirrors internal/domain.ActorType to avoid an import cycle. CanonicalPermissions slice + DefaultRoles map are the single source of truth referenced by the migration; validate_test.go pins (a) no duplicate permissions, (b) every default-role permission is canonical, (c) admin holds the full catalogue, (d) seeded IDs carry the prefix convention, (e) ScopeType enum has exactly 3 values matching the CHECK constraint.

Extended internal/domain/audit.go: added ActorTypeAPIKey + ActorTypeAnonymous to the existing User/System/Agent enum so the audit trail can distinguish API-key requests from federated humans (Bundle 2 OIDC) and demo-mode (CERTCTL_AUTH_TYPE=none). Existing code that records actor_type=User keeps working; new APIKey value used by Bundle 1 Phase 3 middleware.

Repository layer (internal/repository/auth.go + internal/repository/postgres/auth.go):

TenantRepository (Get, List, EnsureDefault). RoleRepository (Get, GetByName, List, Create, Update, Delete with ErrAuthRoleInUse on FK RESTRICT, ListPermissions, AddPermission idempotent, RemovePermission). PermissionRepository (List, GetByName, IsCanonical for fail-fast catalog check). ActorRoleRepository (ListByActor, ListByRole, Grant idempotent, Revoke, EffectivePermissions which is the JOIN that auth.RequirePermission will use in Phase 3 — returns deduplicated (permission, scope) triples honouring the not-yet-expired predicate so future time-bound grants work without code change). Sentinel errors ErrAuthNotFound, ErrAuthDuplicateName, ErrAuthRoleInUse, ErrAuthReservedActor, ErrAuthUnknownPermission for handler-layer 404/409/400 mapping.

Verification: gofmt clean, go vet ./... clean, go test -short ./internal/domain/auth ./internal/repository/postgres pass. Integration tests against a live Postgres are gated by testing.Short() per repo convention; Phase 12 wires the testcontainers harness for full e2e coverage.

Branch: dev/auth-bundle-1. Phase 0 was 99a012e (extract internal/auth/). Phase 2 (service layer) is the next bundle.
2026-05-09 16:00:08 +00:00
shankar0123 99a012e3be auth-bundle-1 Phase 0: extract internal/auth/ from middleware package
Bundle 1 / Phase 0: pure refactor splitting auth surface out of internal/api/middleware so Bundle 2 (OIDC + sessions) and the broader RBAC primitive (roles, permissions, scoped grants) have a clean home.

Moved to internal/auth/: NamedAPIKey, HashAPIKey, AuthConfig, NewAuthWithNamedKeys, NewAuth, UserKey, AdminKey, GetUser, IsAdmin. Added testfixtures.go (WithActor / WithAdmin / WithActorAdmin) so handler tests don't construct context manually.

Stayed in internal/api/middleware/: RequestID, Logging, NewLogging, Recovery, RateLimitConfig, NewRateLimiter (now imports auth.GetUser for per-user keying per audit Category C), CORSConfig, NewCORS, ContentType, CORS, GetRequestID, responseWriter, Chain, audit middleware (now imports auth.GetUser).

Updated 22 caller files across cmd/, internal/api/handler/, internal/api/middleware/, internal/mcp/. Existing m008_admin_gate_test.go now scans for auth.IsAdmin( substring; Phase 3 will further evolve to track auth.RequirePermission. Behavior unchanged: all handler / middleware / service / connector / cmd / mcp tests pass with no test-logic edits, only import-path renames.

Phase 0 exit criteria: internal/auth/ exists with 6 files; middleware.go went 575 -> 422 lines (auth-related ~150 lines moved out); grep -rE 'middleware\.(GetUser|IsAdmin|UserKey|AdminKey|NamedAPIKey|HashAPIKey|NewAuth)' returns 0 hits; context.WithValue(.*middleware.UserKey/AdminKey) returns 0 hits; go vet ./... clean; go test -short ./... green across all packages tested.

Branch: dev/auth-bundle-1. Per cowork/auth-bundle-1-prompt.md, do not merge to master without (1) make verify green, (2) >= 2 external testers confirm, (3) >= 90% coverage on internal/auth/ in .github/coverage-thresholds.yml.
2026-05-09 15:51:31 +00:00
shankar0123 71ebccb8ba docs: fix broken ../examples/ links across docs/ (closes #11)
Reporter (thesudoer0003) flagged that the example links in
docs/getting-started/examples.md resolve to /docs/examples/ which
does not exist. Same bug pattern shows up in four other docs files.

The example READMEs live at examples/<name>/<name>.md at the repo
root, not under docs/. The references in the docs/ tree used
relative paths like `../examples/acme-nginx/acme-nginx.md` which
resolve to docs/examples/... — one level short of escaping out to
the repo root. Fix is one extra `../` so the path resolves to
examples/... at repo root, where the files actually live.

Files touched:
  docs/getting-started/examples.md           5 links
  docs/getting-started/why-certctl.md        1 link
  docs/migration/cert-manager-coexistence.md 1 link
  docs/migration/from-acmesh.md              1 link
  docs/migration/from-certbot.md             1 link

Verified: every `../../examples/<name>/<name>.md` reference now
resolves to the on-disk file. Re-checked via:

  for f in $(grep -rl 'examples/' docs/); do
    for link in $(grep -oE '\.\./\.\./examples/[^)]*' "$f"); do
      [ -e "$(dirname "$f")/$link" ] || echo "STILL BROKEN: $f -> $link"
    done
  done

zero "STILL BROKEN" output.

Closes #11
2026-05-06 20:30:32 +00:00
shankar0123 ff6bf8f203 docs(README): add Status: Early-access disclosure block
Reddit posts and operator-facing copy describe certctl as alpha for
production, but the README's marketing-paragraph framing implied a
more polished maturity. Dual-positioning erodes credibility because
evaluators read both surfaces.

Adds a dedicated "Status: Early-access" blockquote between the
SC-081v3 paragraph and the existing "Actively maintained, shipping
weekly" callout. Calls out the production-quality core (Local CA,
ACME, agent deployment, CRUD, audit) versus the still-maturing
broader surface (intermediate CA hierarchy, ACME/SCEP/EST servers,
network appliances). Encourages lab/dev deployments and welcomes
production deployments with the customer-scale caveat.

The two consecutive blockquotes (Status + Actively maintained) read
as paired signals: the project is early-access AND actively
shipping, which is the honest joint position.
2026-05-06 07:45:55 +00:00
shankar0123 7a9ae3157f fix(seed): repair deployment_targets FK violation crashing fresh demo boot
The Rank 5 cloud-target seed rows in `seed_demo.sql` referenced a
non-existent `ag-server` agent_id. On every fresh-clone
`docker compose -f deploy/docker-compose.yml -f deploy/docker-compose.demo.yml up`
the server crash-looped at the demo-seed step:

  pq: insert or update on table "deployment_targets" violates foreign
  key constraint "deployment_targets_agent_id_fkey"

Origin: commit 9a7e818 ("docs, seed: cloud-target operator runbook +
AWS ACM / Azure KV demo seed rows") added the rows but didn't insert
or rebind to a matching agents row. The `ag-server` ID never existed
in seed_demo.sql or anywhere else.

Fix: bind the two cloud targets to the existing cloud sentinel agents
that were already inserted at lines 78-79 (alongside `cloud-gcp-sm`):

  - tgt-aws-acm-prod  → cloud-aws-sm
  - tgt-azure-kv-prod → cloud-azure-kv

These cloud sentinels were inserted in commit 9a7e818's same family
specifically to back agentless cloud targets — exact semantic match.

Why the existing test didn't catch this:
TestRunDemoSeed_AppliesIdempotently in
internal/repository/postgres/seed_test.go calls the same RunSeed +
RunDemoSeed pair the server uses at boot, so it WOULD have caught the
FK violation. But the test depends on a live PostgreSQL container via
testcontainers-go and is gated under `testing.Short()` → the default
`go test ./... -short` lane that `make verify` runs always skipped it.
The dedicated integration lane that strips `-short` either wasn't run
on commit 9a7e818 or the failure was missed. Promoting the test out
from under `-short` is a separate hardening conversation (CI runs
need docker-in-docker which isn't free); that's out of scope for this
hotfix.

Static FK audit confirms the fix:
  Defined agent IDs (12): ag-{data,edge-01,iis,k8s,lb,mac-dev,
    web-prod,web-staging}-prod, cloud-{aws-sm,azure-kv,gcp-sm},
    server-scanner
  Referenced agent_id values in deployment_targets after fix:
    ag-data-prod, ag-edge-01, ag-iis-prod, ag-k8s-prod, ag-lb-prod,
    ag-web-prod, ag-web-staging, cloud-aws-sm, cloud-azure-kv
  Unresolved: zero.

Acceptance gate (operator-side):
  - docker compose -f deploy/docker-compose.yml \
                   -f deploy/docker-compose.demo.yml up -d --build
    against a fresh clone — server boots clean within 30s, dashboard
    at https://localhost:8443 shows the seeded demo data.
2026-05-05 21:03:18 +00:00
shankar0123 1720e11109 docs: fix broken single-file demo invocation in README + qa-prerequisites + ENVIRONMENTS
The README's Quick Start, the qa-prerequisites contributor doc, and the
landing page (separate repo, separate commit) all shipped a copy-paste
command that produces:

  service "certctl-server" has neither an image nor a build context
  specified: invalid compose project

The bug landed silently with commit a3d8b9c (the U-3 master). Pre-U-3,
docker-compose.demo.yml was self-contained and could be invoked with a
single -f flag. U-3 deliberately reduced it to a 27-line overlay — its
only payload today is `CERTCTL_DEMO_SEED=true` on the certctl-server
service — because the demo seed now applies at boot via
postgres.RunDemoSeed, not via /docker-entrypoint-initdb.d/. The overlay
no longer carries an image: or build: of its own, so it MUST be passed
alongside the base file.

The README/qa-doc/landing-page never picked up the rename of the contract.
Every operator who copy-pasted the Quick Start since U-3 has hit the
"invalid compose project" error and bounced. The operator caught it
running the demo locally today.

This commit fixes the three certctl-repo sites:

  README.md (Quick Start)
    docker compose -f deploy/docker-compose.demo.yml up -d --build
    →
    docker compose -f deploy/docker-compose.yml -f deploy/docker-compose.demo.yml up -d --build

    Plus the "drop the -f flag for clean install" prose now spells out
    the correct fallback (`-f deploy/docker-compose.yml` alone).

  docs/contributor/qa-prerequisites.md (Step 1)
    Same single-file → two-file fix, plus an inline note explaining
    why the override-only file requires the base (so the next person
    who reads it understands the contract instead of re-discovering it).

  deploy/ENVIRONMENTS.md (Demo Overlay → What it adds)
    Replaced the stale "One line: mounts seed_demo.sql into PostgreSQL's
    init directory" claim — that hasn't been true since U-3 — with the
    accurate "One env var: CERTCTL_DEMO_SEED=true; server applies
    seed_demo.sql at boot via postgres.RunDemoSeed" description, plus
    the historical context for why the overlay can't stand alone.

The certctl.io landing page hits the same bug (line 759); fix shipping
in a separate commit in that repo.

Acceptance gate (manual):
  - copy/paste the new README Quick Start command end-to-end against
    a fresh clone — succeeds, dashboard at https://localhost:8443
    shows the seeded demo data within ~30s.
  - clean-install fallback (`docker compose -f deploy/docker-compose.yml
    up -d --build`) starts a working stack with no demo data.
2026-05-05 20:55:26 +00:00
shankar0123 f40e975439 gui(certificates): surface profile contract in create-cert form (closes P3-3, P3-4, P3-5)
Closes findings P3-3, P3-4, P3-5 from the 2026-05-05 CLI/API/MCP↔GUI
parity audit (cowork/cli-gui-parity-audit-2026-05-05/RESULTS.md). The
audit flagged three "hidden defaults" in the create-certificate form:
environment='production', shortLived=false, selectedEkus=['serverAuth'].

Re-grounding against the live source:

  P3-3 was a false positive. The form already exposes an environment
  selector with three options (Production / Staging / Development) and
  defaults to Production. No change needed — covered by new test pin.

  P3-4 + P3-5 misread the architecture. allow_short_lived and
  allowed_ekus are NOT per-cert form-state fields; they are properties
  of the CertificateProfile that the operator binds via the existing
  Profile dropdown. Adding form-level toggles for them would contradict
  the profile-as-primitive design (the profile carries the policy
  contract — TTL, EKUs, key-algo allow-list, short-lived eligibility —
  so the cert can inherit a coherent set rather than letting operators
  hand-mix invalid combinations).

  The genuine UX gap was opacity: operators picked a profile without
  seeing what allow_short_lived / allowed_ekus the profile carried.

This commit closes the spirit of the finding by surfacing the selected
profile's load-bearing properties in a read-only "Profile contract"
panel that appears below the Profile dropdown once a profile is
selected. The panel shows:

  - allowed_ekus list (so operators see whether a profile is
    serverAuth, emailProtection, codeSigning, or a mix)
  - allow_short_lived flag (highlighted when true so operators know
    they're picking a profile that allows TTL < 1h CRL/OCSP-exempt
    certs per the M15b regime)
  - explanatory text that EKUs and short-lived eligibility are
    profile-level (not per-cert), guiding operators to edit the
    profile or pick a different one

Test pins (web/src/pages/CertificatesPage.test.tsx):

  - environment selector renders with 3 options, defaults to production
  - environment selector toggles to staging / development on change
  - Profile contract panel is hidden until a profile is selected
  - Profile contract panel surfaces allowed_ekus when a TLS-server
    profile is picked
  - Profile contract panel surfaces emailProtection EKU when an S/MIME
    profile is picked (closes the "S/MIME flows can't be initiated
    from the GUI" sub-finding — they can, by picking an emailProtection
    profile)
  - Profile contract panel flags allow_short_lived=true when an IoT
    short-lived profile is picked (closes the "operators can't issue
    short-lived certs through the GUI" sub-finding — they can, by
    picking an allow_short_lived profile)

Implementation notes:
  - data-testid='cert-form-environment' + 'cert-form-profile' +
    'cert-form-profile-detail' added to make the test selectors stable
    across DOM-restructuring refactors. No production behaviour change
    from the test IDs.
  - No new dependencies; no form-library introduction (per the prompt's
    out-of-scope list); uses the existing bare React state pattern.
  - No API changes — Certificate.allowed_ekus / allow_short_lived
    already exist on the CertificateProfile type in web/src/api/types.ts.

Acceptance gate (verified):
  - npm test on src/pages/CertificatesPage.test.tsx: 12/12 pass
    (6 pre-existing T-1 tests + 6 new P3-3..P3-5 pins).
  - All sibling page tests (AuditPage, TargetDetailPage, ShortLivedPage,
    etc.) still pass.
2026-05-05 19:49:59 +00:00
shankar0123 0e06f6c4fc cli: promote --force on renew + require --reason on revoke (closes P3-1, P3-2)
Closes findings P3-1 and P3-2 from the 2026-05-05 CLI/API/MCP↔GUI parity
audit (cowork/cli-gui-parity-audit-2026-05-05/RESULTS.md). Both findings
flagged hidden defaults that the CLI was sending without exposing them
to operators: `force=false` baked into every renew payload, and a silent
fallback to `reason="unspecified"` whenever --reason was omitted.

P3-1 — promote --force on `certs renew` (full end-to-end plumbing)

The pre-2026-05-05 CLI sent `{"force": false}` in the renew body. The
API handler never decoded it — a textbook "lying field" per the
operator's CLAUDE.md "complete path, not the easy path" rule: the body
field stored a value, claimed to do something, and silently did nothing
because the wire never reached the consumer. Adding a --force flag that
also went unread would have created another lying field.

This commit takes the complete path:

  service.CertificateService.TriggerRenewal grew a `force bool` parameter
  (internal/service/certificate.go). When force=true, the
  RenewalInProgress block is overridden so operators can recover stuck
  in-flight renewals where a previous job hung without releasing the
  status flag. Archived and Expired remain terminal blockers regardless
  of force — those are semantic dead-ends that --force should not paper
  over (archived = decommissioned, expired = issue a new cert instead of
  renewing a dead one).

  handler.CertificateHandler.TriggerRenewal parses force from
  ?force=true (or ?force=1) query param, OR {"force": true} JSON body,
  whichever the client picks. Defaults to false. Passes through to the
  service.

  internal/cli/client.go::RenewCertificate(id, force bool) sends
  ?force=true on the URL when --force is set. The historical hardcoded
  `{"force": false}` body is gone — no more lying field.

  cmd/cli/main.go dispatches `certs renew <id> [--force]` (ID-first
  flag-second convention matches the existing `agents retire <id>
  [--force]`).

P3-2 — require --reason on `certs revoke` (Option A: strict refusal)

The pre-2026-05-05 CLI dropped to `--reason unspecified` whenever the
operator omitted the flag. Compliance reporting (RFC 5280 §5.3.1, PCI-
DSS §3.6, HIPAA §164.312) relies on the reason code being meaningful;
silent fallback defeats the audit trail because every revocation looks
identical.

  cmd/cli/main.go dispatch refuses to send when --reason is empty,
  prints the canonical RFC 5280 §5.3.1 reason-code menu, and exits
  non-zero.

  internal/cli/client.go exposes ValidRevokeReasons() returning the
  canonical camelCase list (unspecified, keyCompromise, caCompromise,
  affiliationChanged, superseded, cessationOfOperation, certificateHold,
  removeFromCRL, privilegeWithdrawn, aaCompromise) and
  NormalizeRevokeReason() that accepts both camelCase and snake_case
  inputs and normalises to the canonical wire form. Off-list reasons
  are rejected at dispatch with the menu re-printed.

Test pins:

  internal/cli/client_test.go::TestClient_RenewCertificate_ForceFlag —
  --force=true sends ?force=true with empty body; --force=false sends
  no query and no body.

  internal/cli/client_test.go::TestNormalizeRevokeReason +
  TestValidRevokeReasons — canonical-camelCase + snake_case + reject-
  off-enum behaviour.

  cmd/cli/dispatch_test.go::TestHandleCerts_Revoke_RequiresReason +
  TestHandleCerts_Revoke_RejectsUnknownReason +
  TestHandleCerts_Renew_ForceFlag — dispatch-layer pins for the same
  contracts.

  internal/api/handler/certificate_handler_test.go::TestTriggerRenewal_
  ForceQueryParam — query-param passthrough (no-flag, force=true,
  force=1, force=false) flows through to the service-layer parameter.

  internal/service/certificate_test.go::TestTriggerRenewal_
  ForceOverridesInProgress — force=false preserves the
  RenewalInProgress block; force=true clears it.

  Existing TestTriggerRenewal_Archived extended to assert force=true
  still blocks Archived (terminal-state guarantee).

Docs: docs/reference/cli.md updated with the --force example for renew
and the strict --reason semantics for revoke (including snake_case
input acceptance).

Acceptance gate (verified):
  - go build ./cmd/server/... ./cmd/agent/... ./cmd/cli/...
    ./cmd/mcp-server/... clean.
  - go vet ./... clean.
  - go test -short -count=1 ./... pass repo-wide.
  - bash scripts/ci-guards/openapi-handler-parity.sh clean
    (router 178, OpenAPI 144, exceptions 36 — unchanged; we add
    parameter parsing, not routes).
  - gofmt -l clean.
2026-05-05 19:49:34 +00:00
shankar0123 ff75361553 mcp(coverage): add 34 tools across 7 domains to close 2026-05-05 parity audit P1 findings
Closes findings P1-1..P1-35 from the 2026-05-05 CLI/API/MCP↔GUI parity
audit (cowork/cli-gui-parity-audit-2026-05-05/RESULTS.md). Before this
bundle, 35 operator-facing API endpoints had GUI surfaces but no MCP
counterpart — operators using AI assistants for cert lifecycle work in
regulated environments had to drop to curl for approve/reject, health-check
acknowledgement, renewal-policy CRUD, network-scan triggering, discovery
triage, intermediate-CA management, and job verification.

Tool count: 87→121 in tools.go (+34), 6 unchanged in tools_est.go.
Re-derive via grep -cE 'gomcp\\.AddTool\\(' internal/mcp/tools.go
internal/mcp/tools_est.go.

The 7 phases (matching the bundle prompt at
cowork/mcp-coverage-expansion-prompt.md):

  Phase A — Approvals (P1-28..P1-31, 4 tools)
    list_approvals, get_approval, approve_request, reject_request.
    Two-person-integrity contract (ErrApproveBySameActor → HTTP 403)
    is preserved automatically: the decided_by actor is derived
    server-side from middleware.UserKey, NOT from request body, so
    the MCP server's authenticated API-key identity becomes the
    audit-trail actor. The MCP input schema deliberately omits any
    actor_id field to prevent client-side spoofing.

  Phase B — Health Checks (P1-20..P1-27, 8 tools)
    list, summary, get, create, update, delete, history, acknowledge.
    Mirrors the existing target-resource shape; acknowledge takes
    optional 'actor' string captured in the audit row (handler defaults
    to 'unknown' if absent).

  Phase C — Renewal Policies (P1-1..P1-5, 5 tools)
    Standard CRUD against /api/v1/renewal-policies. Distinct from the
    legacy 'policy' tools that point at the same path — these expose
    the renewal-policy domain explicitly with full alert_channels +
    alert_severity_map field shape.

  Phase D — Network Scan Targets (P1-14..P1-19, 6 tools)
    CRUD + trigger_scan. trigger_network_scan returns the discovery-
    scan body so the AI can chain into list_discovered_certificates
    filtered by agent_id.

  Phase E — Discovery read-side (P1-10..P1-13, 4 tools)
    list_discovered_certificates, get_discovered_certificate,
    list_discovery_scans, discovery_summary. Complements the
    pre-existing claim/dismiss tools (registered alongside Health
    historically per the I-2 closure).

  Phase F — Intermediate CAs (P1-6..P1-9, 4 tools)
    list, create (root + child via discriminator on body shape), get,
    retire. The handler is admin-gated via middleware.IsAdmin; the
    least-privilege boundary is enforced at the API layer (HTTP 403
    for non-admin Bearer callers) — not by transport carve-out.

  Phase G — Verification + deployments (P1-32, P1-34, P1-35, 3 tools)
    list_certificate_deployments, verify_job, get_job_verification.
    P1-33 (POST /api/v1/agents/{id}/discoveries) is intentionally
    excluded — machine-to-machine push channel for agents reporting
    filesystem-scan results, not an operator-driven flow. Documented
    inline in the RegisterTools dispatch.

Implementation:
  - 14 new input types in internal/mcp/types.go with jsonschema struct
    tags driving LLM tool discovery.
  - 7 register* functions in internal/mcp/tools.go each handling one
    phase, wired into RegisterTools dispatch in declaration order.
  - 34 new entries in tools_per_tool_test.go::allHappyPathCases —
    the existing in-process MCP harness (TestMCP_AllTools_HappyPath +
    TestMCP_AllTools_ErrorPath + TestMCP_RegisterTools_DispatchableToolCount)
    auto-extends coverage to cover every new tool: happy-path round-
    trip with fence-shape assertion, 5xx error-path with MCP_ERROR fence
    propagation, and 'every registered tool is dispatchable' guard.
  - docs/reference/mcp.md 'Available Tools' table expanded from 16 to
    22 resource domains with current per-domain tool counts.

Acceptance gate (verified):
  - go build ./cmd/server/... ./cmd/agent/... ./cmd/cli/... ./cmd/mcp-server/...
    clean across all four production binaries.
  - go vet ./... clean.
  - go test -short -count=1 ./internal/mcp/... pass (TestMCP_AllTools_*
    expanded to 127 tool round-trips).
  - go test -short -count=1 ./... pass repo-wide.
  - bash scripts/ci-guards/openapi-handler-parity.sh clean (router 178,
    OpenAPI 144, exceptions 36 — unchanged; we add MCP wrappers, not
    routes).
  - gofmt -l clean across the four touched files.
2026-05-05 19:29:57 +00:00
shankar0123 e0aaa967c9 docs(README): add MCP server bullet to capabilities list
The README's 'What it does' section enumerated 11 capability bullets
(issuers / targets / ACME server / SCEP server / EST server /
hierarchy / approvals / discovery / revocation / alerts) but had
zero mention of the MCP server. The 2026-05-05 CLI/API/MCP ↔ GUI
parity audit confirmed 93 MCP tools shipped today (87 in
internal/mcp/tools.go + 6 in internal/mcp/tools_est.go) covering the
full API surface. That's a real differentiator hidden from anyone
landing on the README.

Adds a 12th bullet positioning the MCP server with concrete example
queries operators can ask their AI client (expiring certs, revoke
with key-compromise reason, agent offline check). Frames the
architectural facts: separate binary at cmd/mcp-server/, stateless
stdio transport, no extra auth surface beyond the existing API key,
no extra attack surface.

Links to docs/reference/mcp.md for setup details.
2026-05-05 19:10:27 +00:00
shankar0123 17455d2ea2 deps(web): pin picomatch to >=4.0.4 via npm override; clears 4 dependabot alerts
Dependabot flagged four picomatch vulnerabilities in
web/package-lock.json:

  #8  GHSA-?, ReDoS via extglob quantifiers
  #9  GHSA-?, ReDoS via extglob quantifiers (related to #8)
  #10 CVE-2026-33672 / GHSA-3v7f-55p6-f55p, method injection via
      POSIX character classes (related; affecting < 2.3.2)
  #11 CVE-2026-33672 / GHSA-3v7f-55p6-f55p, method injection via
      POSIX character classes — same advisory as #10, separate
      Dependabot row because it surfaces against a second copy
      of picomatch in the dep tree

All four close on the same fix: every resolved picomatch instance
must be >= 4.0.4 (or >= 3.0.2, or >= 2.3.2 — the patch shipped on
all three release lines). Pre-fix the lockfile carried at least
two vulnerable copies:

  node_modules/picomatch                             v2.3.1  (vuln)
  node_modules/vitest/node_modules/picomatch         v4.0.3  (vuln for #11)
  node_modules/vite/node_modules/picomatch           v4.0.4  (ok)
  node_modules/tinyglobby/node_modules/picomatch     v4.0.4  (ok)

Reachability check before fixing:

  - picomatch is a build-time glob-matching tool (used by
    tailwindcss → readdirp/anymatch/micromatch chain, plus by
    vite + vitest internals).
  - All instances in our tree are dev=true. None are bundled into
    the React production output (web/dist/assets/*.js) — that's
    just the React SPA, no node_modules at runtime.
  - The CVE only affects code that processes UNTRUSTED glob
    patterns. Our build pipeline only globs operator-controlled
    file patterns (TSX source files, Tailwind 'content' globs).
    Not network-reachable.

So the CVE was not reachable from any shipped certctl artefact.
Fix anyway because the alerts are noise.

Fix mechanism: add an npm 'overrides' entry pinning picomatch to
^4.0.4 across all consumers. npm collapses every transitive
picomatch resolution to the override, so the lockfile shrinks from
4 picomatch entries to 1, all on v4.0.4 (patched).

Verification:

  npm install --package-lock-only      → up to date, 0 vuln
  npm audit                            → found 0 vulnerabilities

Diff: 2 files, 7 insertions / 43 deletions (net negative — the
override de-duplicates the picomatch tree).

Closes: GHSA-3v7f-55p6-f55p, CVE-2026-33672 (alerts #10, #11) +
the two related ReDoS picomatch alerts (#8, #9)
2026-05-05 18:40:10 +00:00
shankar0123 f2c77ba3fb deps: bump testcontainers-go v0.35.0 → v0.42.0; drops docker/docker dep entirely (clears CVE-2026-34040)
Dependabot flagged GHSA-x744-4wpc-v9h2 / CVE-2026-34040 (Moby AuthZ
plugin bypass on oversized request bodies, incomplete fix for
CVE-2024-41110) on the transitive github.com/docker/docker
v27.1.1+incompatible pulled in via testcontainers-go v0.35.0.

Reachability check before fixing:

  - certctl does not run dockerd or configure AuthZ plugins.
  - go list -deps ./cmd/{server,agent,cli,mcp-server}/... finds zero
    docker/docker references in any production binary's transitive
    set.
  - testcontainers is consumed only by *_test.go files under
    internal/repository/postgres/ + deploy/test/ for ephemeral
    Postgres containers.

So the CVE was not reachable from any shipped certctl artefact.
Bump anyway because Dependabot noise is noise; the upgrade is
mechanical.

Bumping testcontainers-go v0.35.0 → v0.42.0 (latest, 2026-04-09)
removes the direct docker/docker dependency entirely — testcontainers
v0.42.0 reorganized away from the Moby SDK. After 'go mod tidy',
docker/docker is GONE from both go.mod and go.sum, not merely
bumped. The Dependabot alert closes automatically on push.

Co-bumped transitives (cascading from testcontainers' new dep tree):

  go.opentelemetry.io/otel               v1.24.0  → v1.41.0
  go.opentelemetry.io/otel/{metric,trace} v1.24.0  → v1.41.0
  go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp
                                         v0.49.0  → v0.60.0
  go.opentelemetry.io/auto/sdk           added    @ v1.2.1
  golang.org/x/crypto                    v0.45.0  → v0.48.0
  golang.org/x/net                       v0.47.0  → v0.49.0
  golang.org/x/sync                      v0.18.0  → v0.19.0
  golang.org/x/sys                       v0.40.0  → v0.42.0
  golang.org/x/text                      v0.31.0  → v0.34.0

Verification (all green):

  go build ./cmd/server/... ./cmd/agent/... ./cmd/cli/... \
           ./cmd/mcp-server/...                          → exit 0
  go test -run=NONE -count=1 ./internal/repository/postgres/  → ok
  go test -tags=integration -run=NONE -count=1 ./deploy/test/ → ok
  go vet ./internal/repository/postgres/...                   → clean
  go list -deps ./cmd/{server,agent,cli,mcp-server}/... |
    grep docker                                               → zero hits

Diff: 2 files (go.mod, go.sum), 129 insertions / 144 deletions.

Closes: GHSA-x744-4wpc-v9h2, CVE-2026-34040
2026-05-05 18:34:31 +00:00
shankar0123 d2b62880ce 2026-05-05 18:18:38 +00:00
shankar0123 75097909e9 2026-05-05 18:18:29 +00:00
shankar0123 7c5cc57d75 2026-05-05 15:39:08 +00:00
shankar0123 9acf609ac9 docs: convert ASCII flow diagram to Mermaid in test-environment.md
Per operator audit: every diagram in docs/ should be Mermaid except
in the repo-root README.md. The 'Key Generation Flow (Agent-Side)'
section in docs/contributor/test-environment.md was rendered as a
plain code fence with arrow-prose:

  Server creates job (AwaitingCSR) → Agent polls, sees job →
  Agent generates ECDSA P-256 key pair locally → ...

That was the only non-Mermaid diagram-shaped block left in docs/.

Converted to a Mermaid sequenceDiagram with 5 participants
(certctl-server, issuer connector, certctl-agent, local agent FS,
shared volume) covering the full AwaitingCSR → CSR-submit →
Deployment-job → cert-write → Completed lifecycle.

Audit + verification script: cowork/docs-audit-2026-05-05/mermaid-audit.md.
Re-running the detection script post-fix returns zero non-Mermaid
diagram-like blocks across all 76 docs/ markdown files.

Total Mermaid coverage in docs/ now: 14 docs / 40 blocks.
2026-05-05 06:18:24 +00:00
shankar0123 622cd29f20 docs: factuality sweep — fix 3 broken links + 12 count claims (audit findings 2026-05-05)
Per the cowork/docs-audit-2026-05-05/ end-to-end factuality audit
(20 confirmed findings across 76 docs, 7 parallel subagents +
audit-of-the-audit). Hot + Warm tier fixes ship here; STALE
findings (qa-test-suite.md test-count snapshot) need 'make
qa-stats' which is operator-side.

BROKEN links repaired (3):
- docs/reference/api.md L195: [Quick Start](quickstart.md) →
  ../getting-started/quickstart.md (404 pre-fix)
- docs/reference/api.md L196: [Connector Guide](connectors.md) →
  connectors/index.md (Phase 4 rename, was 404 pre-fix)
- docs/reference/protocols/scep-intune.md L377:
  [legacy-est-scep.md](legacy-est-scep.md) → scep-server.md
  (file was deleted in Phase 7 commit e9b1510)

INCORRECT count claims repaired (12):
- api.md L5 + L18-19 + L155: '78 API operations' / '# 78' /
  'all 78 documented operations' → re-derive via
  grep -cE '^\s+operationId:' (actual at HEAD: 144)
- architecture.md L66 (Mermaid label) + L502 + L1047 + L1253:
  '8 always-on + 4 optional loops' / '12-loop topology' →
  9 always-on + 5 opt-in loops (14 total). Always-on/opt-in
  breakdown derived from cmd/server/main.go startup wiring:
  always-on are agentHealthCheck, crlGeneration, jobProcessor,
  jobRetry, jobTimeout, notificationProcess, notificationRetry,
  renewalCheck, shortLivedExpiryCheck (9); opt-in are
  networkScan, digest, healthCheck, cloudDiscovery, acmeGC (5).
  Re-derive count via grep -cE '^func \(s \*Scheduler\)
  [a-zA-Z]+Loop' internal/scheduler/scheduler.go.
- configuration.md L31: '12 loops, 8 always-on + 4 opt-in' →
  '14 loops, 9 always-on + 5 opt-in'. Self-introduced regression
  from commit 3275f9f (2026-05-05).
- mcp.md L11 + L65: 'all 78 API endpoints' / '78 available tools'
  → re-derive via grep -cE 'mcp\.AddTool\(' (actual at HEAD:
  87 MCP tools, 144 API operations).
- connectors/index.md L111: '9 built-in' issuer connectors →
  '12 built-in', extending the inline enumeration to include
  Entrust, GlobalSign, EJBCA (which had been added since the
  L111 prose was written). Local-CA framing extended to mention
  tree mode + ADCS sub-CA mode-doc.
- connectors/index.md L112: '14 built-in' target connectors →
  '15 built-in', adding AWS ACM target + Azure Key Vault target
  (which had been added since the L112 prose was written).
- why-certctl.md L37 + the inline list: 'Nine issuer connectors
  ship today' → 'Twelve issuer connectors', adding
  AWS ACM PCA, Entrust, GlobalSign, EJBCA to the list and
  removing the misleading 'EST enrollment' bullet (EST is a
  protocol surface, not an issuer; clarified in trailing note).
- why-certctl.md L66: '13 deployment targets' → '15', adding
  Kubernetes Secrets, AWS ACM, and Azure KV to the inline list.
- why-certctl.md L92: 'supports 9 issuer types' → '12 issuer
  types'.
- quickstart.md L135: '35 demo certificates across 5 issuers'
  → re-derive cert count via 'grep -oE "mc-[a-z0-9_-]+"
  migrations/seed_demo.sql | sort -u | wc -l' (actual: 32,
  matches README L86; quickstart was off-by-3).
- quickstart.md L452 (Demo Data Reference table): Certificates
  '35' → '32' (matches the cert count from seed_demo.sql).

Verification:
- grep confirms no remaining stale refs across the touched
  files (8 files, 31 insertions / 28 deletions).
- All 24 ci-guards/*.sh pass locally.
- The audit's STALE findings (S-1, S-2 qa-test-suite.md
  Bundle-P snapshot) are operator-side: run 'make qa-stats'
  to refresh the Test Suite Health table.

Companion: cowork/docs-audit-2026-05-05/RESULTS.md captures
the full audit with subagent false positives and missed
findings called out.
2026-05-05 06:15:35 +00:00
shankar0123 d809874fa1 docs: retire compliance subtree + sweep framework name-drops from prose
Per operator decision the framework-mapping docs are gone. They
were aspirational (no audit, no certification, no validated
mapping); keeping them around was misleading.

Files deleted (1,883 lines):
- docs/compliance/index.md
- docs/compliance/soc2.md
- docs/compliance/pci-dss.md
- docs/compliance/nist-sp-800-57.md

Hyperlinks removed:
- README.md: 'Auditor / compliance' row in the doc table; the
  '(compliance mapping included)' parenthetical in the
  positioning paragraph
- docs/README.md: the '## Compliance' section table; the
  'Auditor / compliance team' reading-order-by-role row

Prose name-drops swept across 24 files:
- README.md: 'FedRAMP boundary CAs / financial-services policy
  CAs' → '4-level boundary CAs / 3-level policy CAs';
  'Compliance-grade for PCI-DSS Level 1, FedRAMP Moderate / High,
  SOC 2 Type II, HIPAA' → cut entirely
- getting-started/{quickstart,concepts,examples,why-certctl,
  advanced-demo}.md: 'compliance' → 'audit' / 'policy';
  'PCI-DSS / SOC 2 / NIST SP 800-57' framework lists cut;
  ''pci': 'true'' tag example → ''environment': 'production''
- migration/cert-manager-coexistence.md: 'compliance rules' →
  'policy rules'
- operator/approval-workflow.md: 'Compliance customers (PCI-DSS
  Level 1, FedRAMP Moderate / High, SOC 2 Type II, HIPAA)' →
  'Operators'; entire 'Compliance control mapping' table
  (PCI-DSS §6.4.5 / NIST SP 800-53 SA-15 / SOC 2 Type II CC6.1
  / HIPAA §164.308(a)(4)) deleted; 'compliance contract' →
  'two-person-integrity contract'; 'compliance auditors' →
  'reviewers'
- operator/legacy-clients-tls-1.2.md: 'PCI-DSS v4.0 Req 4 §2.2.5'
  audit-reference → CWE-326 (kept); 'PCI-DSS Req 4 §2.2.5
  attestation' section retitled to 'TLS posture summary' and
  rewritten without framework framing; 'PCI-DSS, NIST, and
  major browsers will eventually deprecate TLS 1.2' →
  'Major browsers and OS vendors will eventually deprecate
  TLS 1.2'
- operator/database-tls.md: PCI-DSS Req 4 §2.2.5 audit-ref →
  CWE-319 only; 'PCI-DSS scope' → 'sensitive data'; PCI-DSS
  Req 4 v4.0 prose footing → cut
- operator/runbooks/disaster-recovery.md: 'SOC 2 / PCI
  procurement-team deliverable' → 'on-call deliverable';
  'compliance auditors' → 'reviewers'
- reference/connectors/{acme,aws-acm,azure-kv,globalsign,
  local-ca,openssl,ssh,index}.md: 'compliance reporting
  (PCI-DSS §3.6, HIPAA §164.312)' → 'audit reporting';
  'Compliance environments (PCI-DSS Level 1, FedRAMP High,
  HIPAA)' → 'Regulated environments'; 'compliance audits' →
  'audit'; 'FedRAMP boundary CA' pattern names →
  '4-level boundary CA' (technically descriptive)
- reference/protocols/est.md: 'compliance-hook seam' →
  'device-state hook seam'; 'compliance gating' → 'device-state
  gating'; 'est_compliance_failed' → 'est_device_state_failed'
- reference/protocols/scep-intune.md: 'Optional compliance
  check' → 'Optional device-state check'; failure-counter
  'compliance_failed' → 'device_state_failed'; 'Conditional
  Access compliance gating' → 'Conditional Access
  device-state gating'
- reference/intermediate-ca-hierarchy.md: 'FedRAMP boundary-CA
  deployments where the regulator requires...' →
  'Boundary-CA deployments where you want separation of policy
  and issuing authorities'; pattern A retitled '4-level FedRAMP
  boundary CA' → '4-level boundary CA'
- reference/architecture.md: broken Related-docs link to
  compliance.md removed; the rest of that block had stale
  pre-Phase-2 paths (quickstart.md, demo-advanced.md,
  connectors.md, openapi.md, testing-guide.md, test-env.md) —
  retargeted to current locations
- reference/deployment-model.md: 'SOC 2 evidence-report
  generator' → 'Audit-evidence report generator'
- reference/vendor-matrix.md: 'SOC 2 / PCI auditors paste this
  into evidence packs' → 'reviewers paste this into
  vendor-evaluation packs'
- contributor/qa-test-suite.md: 'compliance exist' coverage
  description cut; 'Compliance (PCI / SOC2 / HIPAA-relevant)'
  risk-class label → 'Audit-relevant'

What was kept:
- CWE references (legitimate technical pointers)
- Microsoft API/feature names that happen to use 'compliance'
  literally ('Microsoft Graph compliance API',
  'device-compliance validators' — these are MS product names,
  not framework name-drops)
- 'NIST PQC' on the landing page (Post-Quantum Cryptography is
  the actual NIST standard family, not a compliance framework)

Verified: zero hyperlinks into docs/compliance/ remain. All 24
ci-guards/*.sh pass locally. qa-doc-seed-count.sh clean.
Net diff: 26 files / -1,883 deletions in compliance/ + -32 net
across the prose sweep.

Companion edits in cowork/ (CLAUDE.md doc-tree summary +
WORKSPACE-CHANGELOG.md retirement note) land separately.
2026-05-05 05:26:44 +00:00
shankar0123 5ea8fb48eb ci: restore +x bit on scripts/ci-guards/*.sh (sandbox stripped exec bit)
Pure mode-change commit. The previous 3275f9f commit dropped the
executable bit (100755 → 100644) on five files in scripts/ci-guards/
plus scripts/qa-doc-seed-count.sh and scripts/dev-setup.sh — a
sandbox-tooling artefact, not intentional. The CI pipeline calls
each guard via 'bash "$g"' so the missing exec bit didn't break
anything operationally, but operators who run a guard directly via
'./scripts/ci-guards/<id>.sh' would hit a permission-denied. Restore
to 100755 to match the rest of scripts/ci-guards/*.sh.

No content changes.
2026-05-05 04:56:43 +00:00
shankar0123 3275f9f1e0 ci: post-Phase-2-docs-overhaul cleanup of stale guards + missing config doc
CI run on the ecb8896 push surfaced two real failures rooted in the
2026-05-04 docs overhaul:

  1. G-3 env-docs-drift caught two phantom CERTCTL_* env vars I'd
     introduced in the Phase 4 follow-on connector pages
     (CERTCTL_CA_CERT_PATH_NEW in adcs.md was a placeholder I made
     up; CERTCTL_EJBCA_POLL_MAX_WAIT_SECONDS in ejbca.md does not
     exist in source). Both removed.

  2. QA-doc Part-count drift guard tried to grep
     docs/qa-test-guide.md and docs/testing-guide.md, both of which
     were renamed/deleted in Phase 2/Phase 5. The Part-count drift
     class died with testing-guide.md (Phase 5 prune dispersed its
     content); the seed-count drift class is still live but pointed
     at the wrong path.

Fixes:

- Removed the QA-doc Part-count drift guard from ci.yml (premise
  dead) plus its standalone scripts/qa-doc-part-count.sh peer.
- Retargeted the QA-doc seed-count drift guard from
  docs/qa-test-guide.md → docs/contributor/qa-test-suite.md (the
  Phase 2 target). Updated both ci.yml inline copy and
  scripts/qa-doc-seed-count.sh.
- Updated Makefile qa-stats: target to drop the testing-guide.md
  Parts metric (file is gone).
- Updated Makefile verify-docs: target to drop the part-count step.

G-3 was also failing in the second direction (env vars defined in
config.go but never documented anywhere). 16 vars surfaced —
features.md (deleted Phase 6) and testing-guide.md (deleted Phase 5)
had been their canonical home. Created
docs/reference/configuration.md as the new home: a compact
operator-facing env-var reference covering scheduler intervals, job
lifecycle, rate limiting, audit, deploy verify, database,
agent-side, and SCEP profile binding. Added to docs/README.md
Reference table.

Doc-side updates to qa-test-suite.md to reframe its references to
the deleted testing-guide.md (it's now self-contained: the
Part-by-Part Coverage Map IS the canonical Part inventory).

Cosmetic comment-only updates in ci.yml + scripts/ci-guards/*.sh +
scripts/dev-setup.sh to point at the new audience-organized doc
paths (docs/operator/security.md, docs/operator/tls.md,
docs/reference/architecture.md, etc.) instead of the pre-Phase-2
flat layout.

Verified: all 24 ci-guards/*.sh pass locally; qa-doc-seed-count.sh
clean. Net diff: 178 additions / 112 deletions across 13 files.
One file deleted (qa-doc-part-count.sh) and one file added
(docs/reference/configuration.md).
2026-05-05 04:56:26 +00:00
shankar0123 ecb8896b1c docs: cleanup pre-existing broken links in connector pages
Phase 4 structural (commit 633e440) moved 6 connector files into the
new docs/reference/connectors/ subdirectory but didn't update all
inter-doc references for the new path layout. Phase 11 caught the
high-traffic ones; this sweep gets the rest, found by the Phase 4
follow-on verification pass.

Mappings applied (relative to docs/reference/connectors/):

  deployment-atomicity.md     → ../deployment-model.md
  deployment-vendor-matrix.md → ../vendor-matrix.md
  architecture.md             → ../architecture.md
  est.md                      → ../protocols/est.md
  scep-intune.md              → ../protocols/scep-intune.md
  async-polling.md            → ../protocols/async-ca-polling.md
  quickstart.md               → ../../getting-started/quickstart.md
  demo-advanced.md            → ../../getting-started/advanced-demo.md
  legacy-est-scep.md          → ../protocols/scep-server.md
  connectors.md               → index.md

Plus prose backtick references (`docs/architecture.md` etc.) updated
to the new subdirectory paths.

Files touched: apache, f5, iis, k8s, nginx, index. 33 line changes.
Full link-check across docs/reference/connectors/*.md is now clean
(0 broken inter-doc references).
2026-05-05 04:10:09 +00:00
shankar0123 f179eab071 docs: expand docs/README.md connectors section to enumerate all 28 deep-dive pages
After the Phase 4 follow-on (commits fd94205 → de06141 → 082b8cf →
969853e), the docs/reference/connectors/ tree carries 13 issuer
per-pages + 15 target per-pages alongside the index. Update the
top-level docs navigation to surface them all.

Replaced the previous 5-row connectors table with two
single-paragraph indexes (issuers, targets) listing every per-page
in alphabetical order. The connectors index.md is still the
canonical catalog (interfaces, registry, scanners + inline
reference per built-in); the deep-dive pages cover operator-grade
material on top.

Net: docs/README.md gains coverage of 23 new pages without bloating
the file (two prose paragraphs vs a 28-row table).
2026-05-05 04:08:08 +00:00
shankar0123 969853ee53 docs: Phase 4 follow-on batch 4 — 5 final target per-pages
Extracts the remaining target connectors:

- ssh.md (194 lines) — agentless SSH/SFTP deploy with full
  host-key-acceptance threat model (what's accepted, what's not,
  mitigations including known_hosts enforcement and SSH cert auth);
  V3-Pro forward path
- wincertstore.md (118 lines) — non-IIS Windows services via local
  PowerShell or WinRM proxy mode; store selection (My / Root /
  WebHosting); private-key permissions guidance
- jks.md (189 lines) — JKS / PKCS#12 via keytool with full atomic
  snapshot+rollback contract (Bundle 8 'snapshot → delete → import →
  reload'), keytool argv password exposure threat model + mitigations
- aws-acm.md (208 lines) — ACM target with full IAM policy, IRSA /
  instance-profile / SSO auth recipes, atomic-rollback contract,
  ALB attachment Terraform recipe, procurement-checklist crib
- azure-kv.md (195 lines) — Key Vault target with managed-identity /
  workload-identity / service-principal auth recipes, version-
  semantics rollback caveat (no in-place restore without soft-delete),
  App Gateway / Front Door attachment recipe

Index forward-list expanded to enumerate all 15 target connectors
(5 from Phase 4 structural + 5 from batch 3 + 5 from this batch) in
alphabetical order.

This is part 4 of 4 for the Phase 4 follow-on (per-connector page
extraction) tracked in cowork/docs-overhaul-phase-2-restructure-2026-05-04/log.md.

Net add: 5 files, 904 lines. No content removed from index.md.

End-state of Phase 4 follow-on:
- 13 issuer per-pages (5 batch 1 + 8 batch 2)
- 15 target per-pages (5 Phase 4 structural + 5 batch 3 + 5 batch 4)
- index.md keeps its inline reference content; per-pages add
  operator depth on top, matching the pattern set by
  apache/f5/iis/k8s/nginx in Phase 4 structural
2026-05-05 04:07:21 +00:00
shankar0123 082b8cf660 docs: Phase 4 follow-on batch 3 — 5 file-based target per-pages
Extracts the file-based deploy target connectors:

- haproxy.md (107 lines) — combined-PEM (cert+chain+key) deploy with
  haproxy -c validate; multi-frontend + crt-list directory guidance
- traefik.md (105 lines) — file-provider zero-reload deploy; file
  watcher latency notes; mixing with built-in ACME guidance
- caddy.md (100 lines) — admin API mode (recommended) vs file mode;
  admin-API exposure threat model
- envoy.md (112 lines) — file SDS mode (recommended) vs static
  bootstrap; service-mesh interactions
- postfix.md (175 lines) — dual-mode (Postfix MTA / Dovecot IMAPS)
  connector with daemon-specific quirks (STARTTLS chain expectations,
  no shared session cache); Bundle 11 test pins

Index forward-list expanded to enumerate all 10 target connectors
(5 from Phase 4 structural + 5 from this batch) in alphabetical
order.

This is part 3 of 4 for the Phase 4 follow-on (per-connector page
extraction) tracked in cowork/docs-overhaul-phase-2-restructure-2026-05-04/log.md.

Net add: 5 files, 599 lines. No content removed from index.md.
2026-05-05 04:02:25 +00:00
shankar0123 de06141ce5 docs: Phase 4 follow-on batch 2 — 8 remaining issuer per-pages
Extracts the rest of the issuer per-connector deep-dive pages:

- local-ca.md (170 lines) — Local CA self-signed / sub-CA / tree mode,
  CRL+OCSP endpoints, EKU support, MaxTTL enforcement, L-014 file-on-
  disk threat model carve-out
- acme.md (235 lines) — RFC 8555 v2 client (HTTP-01 / DNS-01 /
  DNS-PERSIST-01), ARI per RFC 9773, EAB + ZeroSSL auto-EAB,
  Let's Encrypt profile selection, revoke-by-serial Top-10 fix #7
- step-ca.md (99 lines) — Smallstep JWK-provisioner synchronous
  issuance with MaxTTL enforcement
- openssl.md (157 lines) — script-based shell-out with full
  threat model (what's accepted, what's not, mitigations, V3-Pro
  forward path)
- sectigo.md (98 lines) — Sectigo SCM REST with bounded async polling
- google-cas.md (89 lines) — GCP managed private CA with OAuth2
  service-account auth + IAM-role guidance
- entrust.md (96 lines) — Entrust CA Gateway mTLS-authenticated with
  approval-pending support and mTLS keypair caching
- globalsign.md (122 lines) — Atlas HVCA dual auth (mTLS + API
  key/secret), region-aware base URLs, mTLS keypair caching

Index forward-list expanded to enumerate all 13 issuer connectors
(including the 5 pages from batch 1) in alphabetical order.

This is part 2 of 4 for the Phase 4 follow-on (per-connector page
extraction) tracked in cowork/docs-overhaul-phase-2-restructure-2026-05-04/log.md.

Net add: 8 files, 1,066 lines. No content removed from index.md.
2026-05-05 03:59:35 +00:00
shankar0123 fd94205cfa docs: Phase 4 follow-on batch 1 — 5 issuer per-pages
Extract the first 5 issuer per-connector deep-dive pages:

- vault.md (128 lines) — Vault PKI synchronous issuance, token TTL +
  auto-renewal loop, MaxTTL enforcement, rotation playbook
- digicert.md (106 lines) — CertCentral DV/OV/EV with bounded async
  polling for vetting workflows
- aws-acm-pca.md (165 lines) — managed private CA on AWS with full
  IAM policy, IRSA wiring, troubleshooting matrix
- ejbca.md (116 lines) — open-source / Keyfactor EJBCA with mTLS or
  OAuth2 auth, mTLS keypair caching, approval-pending guidance
- adcs.md (111 lines) — Active Directory Certificate Services as
  enterprise root via Local CA sub-CA mode, sub-CA rotation playbook

Index updated with forward-list entries and the index-purpose blurb
revised so the index now positions itself as 'navigate from here;
deeper material lives in siblings' rather than 'docs to be extracted
later'.

Each per-page follows the WHAT/HOW/WHY pattern: what the connector is,
how authentication and issuance work, and when to choose this vs an
alternative. Cross-links to the connector index, async-ca-polling
primitive, and adjacent operator runbooks.

This is part 1 of 4 for the Phase 4 follow-on (per-connector page
extraction) tracked in cowork/docs-overhaul-phase-2-restructure-2026-05-04/log.md.

Net add: 5 files, 626 lines. No content removed from index.md (the
index keeps its inline reference; per-pages add operator depth on
top, matching the pattern set by apache/f5/iis/k8s/nginx in Phase 4
structural).
2026-05-05 03:53:52 +00:00
shankar0123 b452013dd9 docs: Phase 5 — testing-guide.md prune (8268 → 0 lines, content dispersed)
Per Phase 1 audit at cowork/docs-overhaul-phase-1-audit-2026-05-04/
and the section-by-section plan in testing-guide-tumor.md.

testing-guide.md was 30% of all docs/ content (8268 lines) but was
integration test code written in markdown, not operator documentation.
The audit's tumor analysis disposed of every Part:
  - ~65% DELETE (test cases that already exist in code)
  - ~22% MOVE to inline test code
  - ~8% KEEP-COMPRESSED into focused operator-runbook docs
  - Title + contents + release sign-off ~5% KEEP

This commit ships the KEEP-COMPRESSED dispersal:

  docs/contributor/qa-prerequisites.md (NEW, ~120 lines):
    From testing-guide.md "Prerequisites" section. Stack boot procedure,
    demo data baseline, reference IDs operators reuse across QA docs.

  docs/contributor/gui-qa-checklist.md (NEW, ~105 lines):
    From testing-guide.md "Part 35: GUI Testing". Manual GUI verification
    pass for release sign-off. 25-row table covering every dashboard page.

  docs/contributor/release-sign-off.md (NEW, ~130 lines):
    From testing-guide.md "Release Sign-Off" section (originally 1009
    lines of per-test detail tables). Compressed to a release-day
    checklist organized by gate category: code state, automated gates,
    manual QA passes, release artefact verification, branch protection,
    post-release.

  docs/operator/performance-baselines.md (NEW, ~100 lines):
    From testing-guide.md "Part 39: Performance Spot Checks". Four
    operator-runnable benchmarks (API request handling, inventory list
    pagination, scheduler tick, bulk revoke) with baseline numbers and
    when-to-re-baseline guidance.

  docs/operator/helm-deployment.md (NEW, ~120 lines):
    From testing-guide.md "Part 52: Helm Chart Deployment". Operator
    runbook for the bundled deploy/helm/certctl/ chart: prereqs,
    install, four cert-source patterns, verify, upgrade, troubleshooting.

  docs/reference/cli.md (NEW, ~120 lines):
    From testing-guide.md "Part 28: CLI Tool". certctl-cli command
    reference with command-group breakdown, common workflows
    (list/filter, renew, revoke, bulk import, EST enrollment, status),
    output formats, CI/CD integration patterns.

docs/README.md navigation index updated to include the 6 new docs:
  Reference section gains: cli.md, release-verification.md (was added
    in Phase 13)
  Operator section gains: helm-deployment.md, performance-baselines.md
  Contributor section gains: qa-prerequisites.md, gui-qa-checklist.md,
    release-sign-off.md

docs/testing-guide.md deleted. Git history preserves the 8268 lines —
if any specific test case is found missing from inline test code or
the destination docs during future work, lift from `git show
HEAD~1:docs/testing-guide.md`.

Net: docs/ total line count drops by ~7700 lines (28%), from 26,369
to 18,742. testing-guide.md was the single largest doc; pruning it is
the single biggest content-edit win of the entire restructure.

Phase 5 is the last major content phase. Remaining: Phase 4 follow-on
(per-connector page extractions from reference/connectors/index.md),
Phase 15 (WHAT/HOW/WHY remediation), Phase 16 (final acceptance gate).
2026-05-05 03:38:54 +00:00
shankar0123 fd4eb3b165 docs: Phase 11 follow-on — fix remaining anchor + cross-dir links
Final cleanup pass after the previous Phase 11 commits. Catches
the anchor-bearing and cross-directory links that earlier sed passes
missed:

  docs/reference/protocols/acme-server.md (3 fixes):
    (./tls.md) → (../../operator/tls.md)
    (./architecture.md) → (../architecture.md)
    (./architecture.md#agents) → (../architecture.md#agents)

  docs/migration/from-certbot.md (1 fix):
    (./quickstart.md#network-discovery-agentless)
    → (../getting-started/quickstart.md#network-discovery-agentless)

  docs/migration/cert-manager-coexistence.md (1 fix):
    (./architecture.md#agents) → (../reference/architecture.md#agents)

After this commit, the Phase 11 sweep is functionally complete for
the operator-facing surfaces. Remaining valid sibling links
(`(./<name>.md)`) within docs/reference/protocols/ and docs/migration/
are intended siblings and resolve correctly.

The remaining open Phase 11 items are:
  - testing-strategy.md → testing-guide.md link, still valid because
    testing-guide.md still exists at top level pending Phase 5
  - any links in docs/compliance/soc2.md and docs/compliance/nist-sp-800-57.md
    if they reference moved docs (low traffic; revisit if Phase 4
    follow-on or Phase 5 work surfaces them)
2026-05-05 03:32:09 +00:00
shankar0123 a364cd6990 docs: Phase 11 follow-on — fix anchor-bearing + remaining inter-doc links
Per Phase 1 audit at cowork/docs-overhaul-phase-1-audit-2026-05-04/.
Sweeps the anchor-bearing inter-doc links that the previous Phase 11
sed pass missed (anchors after .md# weren't matched), plus a few
remaining cross-refs in docs/reference/.

Per source file:

  docs/migration/acme-from-caddy.md (1 anchor link):
    (./acme-server.md#certificate-readyfalse-with-rejectedidentifier)
    → (../reference/protocols/acme-server.md#certificate-readyfalse-...)

  docs/migration/acme-from-cert-manager.md (3 anchor links):
    Same shape; all (./acme-server.md#...) → (../reference/protocols/acme-server.md#...)

  docs/reference/connectors/index.md (5 walkthrough + reference links):
    (./acme-server.md) → (../protocols/acme-server.md)
    (./acme-server-threat-model.md) → (../protocols/acme-server-threat-model.md)
    (./acme-cert-manager-walkthrough.md) → (../../migration/acme-from-cert-manager.md)
    (./acme-caddy-walkthrough.md) → (../../migration/acme-from-caddy.md)
    (./acme-traefik-walkthrough.md) → (../../migration/acme-from-traefik.md)

  docs/reference/protocols/acme-server.md (3 walkthrough links):
    (./acme-cert-manager-walkthrough.md) → (../../migration/acme-from-cert-manager.md)
    (./acme-caddy-walkthrough.md) → (../../migration/acme-from-caddy.md)
    (./acme-traefik-walkthrough.md) → (../../migration/acme-from-traefik.md)

  docs/reference/protocols/acme-server-threat-model.md (1 cross-dir):
    (./tls.md) → (../../operator/tls.md)

After this commit, every grep for old-style `./<old-doc-name>.md` links
returns clean across docs/migration/, docs/reference/, and
docs/operator/.
2026-05-05 03:31:47 +00:00
shankar0123 12d7b1f51d docs: Phase 11 follow-on — fix inter-doc cross-references in deeper subdirs
Per Phase 1 audit at cowork/docs-overhaul-phase-1-audit-2026-05-04/.
Continuation of Phase 11 (commit dca1900 handled README + first round
of docs/ links). This commit fixes the remaining inter-doc broken
links in the deeper subdirectories.

Per source directory:

  docs/getting-started/quickstart.md (1 fix):
    (connectors.md) → (../reference/connectors/index.md)

  docs/contributor/test-environment.md (2 fixes):
    (tls.md) → (../operator/tls.md)
    (upgrade-to-tls.md) → (../archive/upgrades/to-tls-v2.2.md)

  docs/contributor/testing-strategy.md (4 fixes):
    `docs/security.md` → `docs/operator/security.md`
    (security.md) → (../operator/security.md)
    `docs/testing-guide.md` (kept; testing-guide.md still at top level
      pending Phase 5 prune)
    (testing-guide.md) → (../testing-guide.md)

  docs/migration/acme-from-traefik.md (2 sites, multi-link):
    (./acme-cert-manager-walkthrough.md) → (./acme-from-cert-manager.md)
    (./acme-server.md) → (../reference/protocols/acme-server.md)

  docs/migration/cert-manager-coexistence.md (1 fix):
    (./quickstart.md) → (../getting-started/quickstart.md)

  docs/migration/from-acmesh.md (2 fixes):
    (connectors.md) → (../reference/connectors/index.md)
    (./examples.md) → (../getting-started/examples.md)

  docs/migration/acme-from-caddy.md (multi-link):
    (./acme-cert-manager-walkthrough.md) → (./acme-from-cert-manager.md)
    (./acme-server.md) → (../reference/protocols/acme-server.md)

  docs/migration/acme-from-cert-manager.md (multi-link):
    (./acme-server.md) → (../reference/protocols/acme-server.md)
    (./acme-server-threat-model.md) → (../reference/protocols/acme-server-threat-model.md)
    (./acme-caddy-walkthrough.md) → (./acme-from-caddy.md)
    (./acme-traefik-walkthrough.md) → (./acme-from-traefik.md)

  docs/migration/from-certbot.md (2 fixes):
    (./concepts.md) → (../getting-started/concepts.md)
    (./examples.md) → (../getting-started/examples.md)

  docs/operator/tls.md (3 sites):
    (upgrade-to-tls.md) → (../archive/upgrades/to-tls-v2.2.md)
    (quickstart.md) → (../getting-started/quickstart.md)
    (test-env.md) → (../contributor/test-environment.md)

  docs/operator/runbooks/disaster-recovery.md (5 fixes):
    (crl-ocsp.md) → (../../reference/protocols/crl-ocsp.md)
    (tls.md) → (../../operator/tls.md)
    (security.md) → (../../operator/security.md)
    (scep-intune.md) → (../../reference/protocols/scep-intune.md)
    (est.md) → (../../reference/protocols/est.md)

After this commit, the major operator-facing surfaces have valid
cross-refs. Some lower-traffic docs (compliance/soc2.md, compliance/
nist-sp-800-57.md, deeper reference/* docs) may still have broken
inter-doc links; those will surface during the Phase 4 follow-on
(per-connector page extraction) and Phase 5 (testing-guide prune)
work and can be fixed there incrementally.
2026-05-05 03:31:05 +00:00
shankar0123 19c8fafe84 docs: Phase 14 — Last reviewed line sweep across docs/
Per Phase 1 audit at cowork/docs-overhaul-phase-1-audit-2026-05-04/.
Adds a `> Last reviewed: 2026-05-05` line right after the H1 heading
of every doc that didn't already have one (41 files).

This dates the freshness clock for the future Phase 4 per-doc review.
The discipline going forward: when a doc's content gets a meaningful
edit, bump the date. When the date gets old (e.g., >6 months), the
doc earns a freshness-review pass.

Mechanical insertion via awk one-liner, applied to every docs/*.md
that didn't already match `grep -q 'Last reviewed:'`. Files that
already carried the line from earlier Phase 2 work (the navigation
index, the new connector docs, the new SCEP server / legacy-clients-
TLS-1.2 / release-verification docs, and the 5 per-connector deep
dives) were skipped to avoid duplicate insertion.

Net: every doc in docs/ now has a Last reviewed line.
2026-05-05 03:26:46 +00:00
shankar0123 426760d737 docs: Phase 13 — README rewrite to 250-line target
Per Phase 1 audit at cowork/docs-overhaul-phase-1-audit-2026-05-04/.
README went from 457 lines to a target of 250 (operator decision in
Phase 1 conversation). Focus shifts from feature-catalog + landing-page
duplicate to "developer cloning the repo needs orientation + quickstart
+ entry points to docs."

What stayed:
  - Logo + title + badges (~15 lines)
  - Elevator paragraph + 47-day cliff context (3 paragraphs, compressed)
  - Active-maintenance callout
  - Documentation table — restructured from 22 entries linking to flat
    docs/ to ~6 audience-organized rows linking through the new
    docs/README.md navigation index
  - Screenshots grid (4 tiles)
  - "What it does" — compressed from 33 lines of prose to 8 capability
    bullets, each linking to the canonical doc
  - Architecture paragraph — compressed to one paragraph linking to
    docs/reference/architecture.md
  - Quick Start (Docker Compose, Agent install, Helm, container images)
  - Examples table (5 turnkey scenarios)
  - Development commands
  - License paragraph
  - Dependencies block
  - Footer CTA

What got moved out:
  - Cosign verification / SLSA / SBOM section (67 lines) →
    docs/reference/release-verification.md (NEW). README links to it
    in a 3-line "Verifying a release" section.

What got removed entirely:
  - "Why certctl" + "Architecture" + "Security-first" + "Key design
    decisions" prose walls — duplicated landing page + architecture.md +
    security.md content. README no longer wades through 11 dense
    paragraphs.
  - "Supported Integrations" 4 sub-tables (Issuers / Targets / Protocols
    / Standards / Notifiers, ~80 lines of dense per-row marketing
    copy) — content lives at docs/reference/connectors/index.md and
    docs/reference/protocols/. README mentions counts ("12 issuers, 15
    targets, 6 notifiers") with a single link.
  - "Roadmap" section entirely — V1 + V2 history rotted fastest of any
    section; replaced with implicit "see Releases + Issues for active
    work" via the existing footer CTA.
  - "What It Does" 10-subsection wall (33 lines) — replaced with the
    8-bullet capability list, each linking to its canonical doc.
  - CLI section (20 lines of inline command examples) — links to the
    contributor docs.
  - MCP Server section (30 lines of setup) — links to docs/reference/mcp.md.

New surface added:
  - docs/reference/release-verification.md — moved cosign/SLSA/SBOM
    procedure with one expanded "Why this matters" paragraph
    explaining the keyless OIDC trust anchor.

Every docs/ link in the new README verified to resolve to an existing
file. Cross-references from other docs / certctl.io to the deleted
sections (if any) need follow-up Phase 11 sweeps.
2026-05-05 03:26:05 +00:00
shankar0123 affaa11d14 docs: Phase 12 — populate docs/README.md navigation index
Per Phase 1 audit at cowork/docs-overhaul-phase-1-audit-2026-05-04/.
The placeholder from Phase 1 (commit cda957f) gets replaced with the
audience-organized navigation index operators use to find what they
need.

Structure follows the recommended Phase 2 directory tree:

  - Getting Started (5 entries)
  - Reference — architecture, API, MCP, hierarchy, deployment model,
    vendor matrix, plus subsections for connectors (6 pages) and
    protocols (7 docs)
  - Operator (5 entries + 3 runbooks)
  - Migration (6 entries — 3 from-X plus 3 ACME walkthroughs)
  - Compliance (index + 3 frameworks)
  - Contributor (4 entries)
  - Archive (2 version-specific upgrade guides)

Every link verified to resolve to an existing file. Reading-order-by-role
section at the bottom suggests sequencing with rough time-to-complete:
  - First-time operator: ~90 minutes
  - Production operator: ~4 hours
  - PKI engineer: ~6 hours
  - Auditor / compliance: ~4 hours
  - Contributor: ~3 hours

Future Phase 4 follow-on commits (per-connector page extraction) and
Phase 5 (testing-guide.md prune) will add new entries to this index
as their destination docs land.
2026-05-05 03:21:53 +00:00
shankar0123 dca1900815 docs: Phase 11 (partial) — fix cross-references after Phase 2 moves
Per Phase 1 audit at cowork/docs-overhaul-phase-1-audit-2026-05-04/.
Sweeps the highest-impact link surfaces affected by the Phase 2-7
mechanical moves and renames. Covers README.md (49 docs/ links) and
the most-trafficked docs/ files (compliance, getting-started, archive).

README.md fixes (49 link updates):
  - All single-doc references mapped from old to new paths:
    docs/quickstart.md → docs/getting-started/quickstart.md
    docs/architecture.md → docs/reference/architecture.md
    docs/connectors.md → docs/reference/connectors/index.md
    docs/acme-server.md → docs/reference/protocols/acme-server.md
    docs/{soc2,pci-dss,nist}.md → docs/compliance/{soc2,pci-dss,nist-sp-800-57}.md
    ... (full mapping in the sed pipeline)
  - 3 references to deleted features.md replaced with pointers to
    architecture.md + connectors/index.md.

docs/compliance/index.md (3 sibling renames):
  compliance-soc2.md     → soc2.md
  compliance-pci-dss.md  → pci-dss.md
  compliance-nist.md     → nist-sp-800-57.md

docs/compliance/pci-dss.md (3 external refs need ../):
  architecture.md  → ../reference/architecture.md
  connectors.md    → ../reference/connectors/index.md
  quickstart.md    → ../getting-started/quickstart.md

docs/getting-started/concepts.md (4 external refs):
  crl-ocsp.md      → ../reference/protocols/crl-ocsp.md
  architecture.md  → ../reference/architecture.md
  mcp.md           → ../reference/mcp.md
  openapi.md       → ../reference/api.md

docs/getting-started/quickstart.md (4 external refs + 1 sibling):
  tls.md           → ../operator/tls.md
  upgrade-to-tls.md → ../archive/upgrades/to-tls-v2.2.md
  architecture.md  → ../reference/architecture.md
  demo-advanced.md → advanced-demo.md (sibling rename)

docs/getting-started/examples.md (4 external refs):
  migrate-from-certbot.md         → ../migration/from-certbot.md
  migrate-from-acmesh.md          → ../migration/from-acmesh.md
  certctl-for-cert-manager-users.md → ../migration/cert-manager-coexistence.md
  connectors.md                   → ../reference/connectors/index.md

docs/archive/upgrades/to-tls-v2.2.md (3 external refs need ../../):
  tls.md           → ../../operator/tls.md
  quickstart.md    → ../../getting-started/quickstart.md
  test-env.md      → ../../contributor/test-environment.md

docs/archive/upgrades/to-v2-jwt-removal.md (2 external refs need ../../):
  architecture.md  → ../../reference/architecture.md
  tls.md           → ../../operator/tls.md

Verified all README.md docs/ links resolve to existing files. The only
remaining top-level link is testing-guide.md which still exists at the
top of docs/ (Phase 5 will prune it later).

Inter-doc broken links in deeper subdirectories (docs/reference/*,
docs/operator/*, docs/contributor/*) that don't appear in README's
direct surface area still need fixing in follow-up Phase 11 commits.
This commit handles the operator-facing entry points.
2026-05-05 03:19:21 +00:00
shankar0123 633e440787 docs: Phase 4 (structural) — move connectors.md + 5 deep dives into reference/connectors/
Per Phase 1 audit at cowork/docs-overhaul-phase-1-audit-2026-05-04/.
Phase 4 in the audit recommended a full split of connectors.md (2055
lines) into an index + 27 per-connector pages (12 issuer + 15 target).
This commit lands the structural half of that work; full per-target
page extraction is deferred to follow-up commits.

Renames (all blame-preserving):
  docs/connectors.md         → docs/reference/connectors/index.md
  docs/connector-apache.md   → docs/reference/connectors/apache.md
  docs/connector-f5.md       → docs/reference/connectors/f5.md
  docs/connector-iis.md      → docs/reference/connectors/iis.md
  docs/connector-k8s.md      → docs/reference/connectors/k8s.md
  docs/connector-nginx.md    → docs/reference/connectors/nginx.md

Edits:
  - docs/reference/connectors/index.md gets a top-of-doc note
    explaining the per-connector deep-dive sibling pattern + a forward
    list of the 5 per-target pages.
  - The 5 per-connector deep-dive pages each get a `Last reviewed:
    2026-05-05` header + a back-link to the index.

Deferred to future commits (Phase 4b/c follow-on):
  - Extracting the 12 issuer sections from index.md into per-issuer
    pages at reference/connectors/{acme,awsacmpca,digicert,ejbca,
    entrust,globalsign,googlecas,local,openssl,sectigo,stepca,vault}.md
  - Extracting the 10 remaining target sections from index.md into
    per-target pages at reference/connectors/{caddy,traefik,envoy,
    haproxy,postfix-dovecot,ssh,javakeystore,wincertstore,awsacm,
    azurekv}.md

The pragmatic split makes this Phase 4 work incrementally landable —
each per-connector extraction is a small follow-up commit that doesn't
change the docs/ tree shape further. Cross-references from README.md
and other docs to docs/connectors.md still need fixing in Phase 11.
2026-05-05 03:14:39 +00:00
shankar0123 cee008207b docs: delete features.md (Phase 6 disperse, content already in canonical docs)
Per Phase 1 audit at cowork/docs-overhaul-phase-1-audit-2026-05-04/.
features.md was a 1606-line feature catalog with ~80% overlap with
canonical docs already in the tree:

  - "API Surface" section (rate limiting, CORS, body size limits)
    → docs/operator/security.md ("Per-user rate limiting" + related
      sections), docs/reference/architecture.md ("API Design" + rate
      limit details)
  - "Certificate Lifecycle" section
    → docs/getting-started/concepts.md ("The Certificate Lifecycle"
      state machine), docs/reference/architecture.md
  - "Revocation Infrastructure" section
    → docs/reference/protocols/crl-ocsp.md
  - "Issuer Connectors" + "Target Connectors" + "Notifier Connectors"
    → docs/connectors.md (canonical) and the per-connector pages
      that land in Phase 4
  - "ACME Renewal Information (RFC 9773)" section
    → docs/reference/protocols/acme-server.md
  - "Discovery" section
    → docs/getting-started/concepts.md, docs/reference/architecture.md
  - "Observability" section
    → docs/operator/security.md, docs/reference/architecture.md
  - "Job System" + "Background Scheduler"
    → docs/reference/architecture.md
  - "Web Dashboard"
    → docs/getting-started/concepts.md
  - "CLI" section
    → docs/reference/cli.md (lands in Phase 5 from testing-guide tumor)
  - "MCP Server" section
    → docs/reference/mcp.md
  - "Agent" section
    → docs/reference/architecture.md, docs/getting-started/concepts.md
  - "Deployment" section
    → docs/reference/deployment-model.md
  - "Database Schema" section
    → docs/reference/architecture.md
  - "Security" section
    → docs/operator/security.md
  - "CI/CD" section
    → docs/contributor/ci-pipeline.md
  - "Test Suite" section
    → docs/contributor/testing-strategy.md
  - "Examples" section
    → docs/getting-started/examples.md
  - "Compliance Mapping" section
    → docs/compliance/index.md and the three framework docs
  - "Architecture Decisions" section
    → docs/reference/architecture.md

The catalog format failed both beginners (overwhelming wall of text)
and experts (grep on source is faster than reading 1606 lines of
prose). Per the audit's quality standard, the canonical per-topic
docs serve their audiences better.

Git history preserves features.md content. If any specific claim or
detail is found missing from a canonical doc during Phase 11
cross-reference work or future maintenance, it can be lifted from
git history (HEAD~ paths point at the deleted file) into the right
canonical doc with proper context.

Cross-references from README.md and other docs to docs/features.md
still need fixing in Phase 11.
2026-05-05 03:09:48 +00:00
shankar0123 e9b15108d9 docs: split legacy-est-scep.md into two purpose-aligned docs
The 519-line legacy-est-scep.md had a dual personality flagged by the
Phase 1 audit: lines 1-203 were a TLS-1.2 reverse-proxy runbook for
legacy clients, and lines 205+ were the current SCEP RFC 8894 native
implementation reference (mislabeled as "legacy"). Two separate audiences,
two separate purposes.

Split:

  Lines 1-203 (TLS-1.2 reverse-proxy runbook):
    → docs/operator/legacy-clients-tls-1.2.md (NEW)

    Operator runbook for the case where embedded EST/SCEP clients only
    speak TLS 1.2. Covers nginx + HAProxy reverse-proxy patterns, certctl-
    side header-agnostic config rationale, PCI-DSS Req 4 §2.2.5 attestation,
    deprecation timeline. Also got a fresh "What this is" framing.

  Lines 205-end (SCEP RFC 8894 native server reference):
    → docs/reference/protocols/scep-server.md (NEW)

    Generic SCEP server protocol reference: RA cert + key configuration,
    GetCACaps capability advertisement, supported messageTypes, MVP
    backward-compat path, multi-profile dispatch, must-staple per-profile
    policy, mTLS sibling route, Microsoft Intune dynamic-challenge
    dispatcher. Cross-links to scep-intune.md for Intune-specific
    deployment guidance.

Both new docs carry a `Last reviewed: 2026-05-05` line. Internal links
within each new doc updated to the new sibling paths. Cross-references
from other docs to legacy-est-scep.md still need fixing in Phase 11.

Original docs/legacy-est-scep.md deleted (git history preserves).
2026-05-05 02:55:45 +00:00
shankar0123 f157c18368 docs: re-home ACME client walkthroughs under docs/migration/
The three ACME client walkthroughs (Caddy, cert-manager, Traefik) are
conceptually "I have an existing X, here's how to point its ACME
client at certctl." They belong with the migration docs, not with the
acme-server protocol reference.

Renames:
  docs/acme-caddy-walkthrough.md       → docs/migration/acme-from-caddy.md
  docs/acme-cert-manager-walkthrough.md → docs/migration/acme-from-cert-manager.md
  docs/acme-traefik-walkthrough.md     → docs/migration/acme-from-traefik.md

Each walkthrough's lede gets a "Use this walkthrough when..." paragraph
that closes the WHY-weak gap flagged in the Phase 1 audit. The new
framing tells the reader when to pick this walkthrough versus the
alternatives:

  - Caddy: "you're running Caddy 2.7+ and want it to ACME-issue from
    certctl instead of Let's Encrypt"
  - cert-manager: explicit pointer to cert-manager-coexistence.md for
    the keep-cert-manager-running case (vs replacement)
  - Traefik: "you're running Traefik 3.0+ and want certctl as your
    ACME source of truth"

Cross-reference updates from other docs and README still pending in
Phase 11.
2026-05-05 02:51:10 +00:00
shankar0123 b21c02a3d5 docs: archive version-specific upgrade guides
upgrade-to-tls.md and upgrade-to-v2-jwt-removal.md are version-specific
runbooks for past releases. Late upgraders still need them; current
operators don't. Move both to docs/archive/upgrades/ with one-line
archive headers pointing readers at the current canonical docs.

Renames:
  docs/upgrade-to-tls.md           → docs/archive/upgrades/to-tls-v2.2.md
  docs/upgrade-to-v2-jwt-removal.md → docs/archive/upgrades/to-v2-jwt-removal.md

Each gets a top-of-doc archive notice with the date and a forward
pointer to the relevant steady-state doc:
  to-tls-v2.2.md            → docs/operator/tls.md
  to-v2-jwt-removal.md      → docs/operator/security.md

The relative link inside to-v2-jwt-removal.md (was "upgrade-to-tls.md",
now "to-tls-v2.2.md") updated to point at its archived sibling.

Cross-reference updates from other docs and README still pending in
Phase 11.
2026-05-05 02:50:14 +00:00
shankar0123 3a807ae37e docs: Phase 2 mechanical file moves to subdirectory structure
Pure git mv operations; no content edits. Internal links remain pointing
at old paths and will be fixed in Phase 11. Per the Phase 1 audit
recommendations at cowork/docs-overhaul-phase-1-audit-2026-05-04/.

35 files moved across 8 audience-organized subdirectories:

  docs/getting-started/ (5):
    quickstart.md, concepts.md, examples.md, advanced-demo.md (was
    demo-advanced.md), why-certctl.md

  docs/reference/ (6):
    architecture.md, api.md (was openapi.md), mcp.md,
    intermediate-ca-hierarchy.md, deployment-model.md (was
    deployment-atomicity.md), vendor-matrix.md (was
    deployment-vendor-matrix.md)

  docs/reference/protocols/ (6):
    acme-server.md, acme-server-threat-model.md, scep-intune.md,
    est.md, crl-ocsp.md, async-ca-polling.md (was async-polling.md)

  docs/operator/ (4):
    security.md, tls.md, database-tls.md, approval-workflow.md

  docs/operator/runbooks/ (3):
    cloud-targets.md (was runbook-cloud-targets.md), expiry-alerts.md
    (was runbook-expiry-alerts.md), disaster-recovery.md

  docs/migration/ (3):
    from-certbot.md (was migrate-from-certbot.md), from-acmesh.md
    (was migrate-from-acmesh.md), cert-manager-coexistence.md (was
    certctl-for-cert-manager-users.md)

  docs/compliance/ (4):
    index.md (was compliance.md), soc2.md (was compliance-soc2.md),
    pci-dss.md (was compliance-pci-dss.md), nist-sp-800-57.md (was
    compliance-nist.md)

  docs/contributor/ (4):
    testing-strategy.md, test-environment.md (was test-env.md),
    ci-pipeline.md, qa-test-suite.md (was qa-test-guide.md)

Deferred to later Phase 2 sub-phases:
  - connectors.md split (Phase 4): docs/connectors.md +
    docs/connector-{apache,f5,iis,k8s,nginx}.md still at top level
  - testing-guide.md prune (Phase 5): docs/testing-guide.md still
    at top level
  - features.md disperse (Phase 6): docs/features.md still at top
    level
  - legacy-est-scep.md split (Phase 7): docs/legacy-est-scep.md
    still at top level
  - ACME walkthrough re-homing (Phase 8): three
    docs/acme-*-walkthrough.md still at top level
  - Upgrade docs archive (Phase 3): two docs/upgrade-*.md still
    at top level

Cross-reference updates (Phase 11) will happen after all moves and
content edits land. Internal links to docs/* paths are temporarily
broken until that phase completes.
2026-05-05 02:49:28 +00:00
shankar0123 cda957f302 docs: Phase 2 prep — placeholder navigation index
Per Phase 1 audit at cowork/docs-overhaul-phase-1-audit-2026-05-04/.
Phase 2 organizes docs/ into eight audience-aligned subdirectories
(getting-started, reference, operator, migration, compliance,
contributor, archive). docs/README.md will be the navigation index
linking into each.

This commit only adds the placeholder. Subdirectories materialize as
Phase 2 file moves land. Index gets populated in Phase 12 once all
moves and content edits are complete.

Audit folder: cowork/docs-overhaul-phase-1-audit-2026-05-04/
Phase 2 prompt: cowork/docs-overhaul-phase-2-restructure-prompt.md
2026-05-05 02:48:49 +00:00
shankar0123 0f81c1b956 ci: re-fix CodeQL #32 + repair loadtest f5-mock build context
Two unrelated CI failures from run #25305811340; fixed in one
commit since neither needs the other to land first.

CodeQL alert #32 (go/log-injection at middleware.go:68) reopened
after b0fc067. The previous fix introduced a scrubLogValue helper
backed by strings.NewReplacer; CodeQL's taint tracker only
recognizes the literal strings.ReplaceAll pattern as a sanitizer
(matches the OWASP example in the rule docs). Wrapper helpers and
NewReplacer don't trigger the recognition, so the analyzer kept
flagging.

Fix: drop the helper. Inline strings.ReplaceAll chains directly at
the call site for r.Method and r.URL.Path. Same runtime semantics
(strip CR/LF/NUL); CodeQL pattern-matches the literal call so the
alert can finally close.

Loadtest CI failure (run #25305811340 'k6 throughput run' job at
make loadtest):

  ERROR: failed to compute cache key: failed to calculate checksum
  of ref ...: "/deploy/test/f5-mock-icontrol": not found

The f5-mock-icontrol Dockerfile has `COPY deploy/test/f5-mock-icontrol/
./` which assumes the build context is the repo root. The
docker-compose.test.yml f5-mock-icontrol service correctly uses the
long-form build:

  build:
    context: ..        # = repo root from deploy/docker-compose.test.yml
    dockerfile: deploy/test/f5-mock-icontrol/Dockerfile

The loadtest compose at deploy/test/loadtest/docker-compose.yml
used the shorthand:

  build: ../f5-mock-icontrol

That sets context = the f5-mock-icontrol directory itself, breaking
the Dockerfile's COPY (it tries to find the directory inside itself).

Fix: change the loadtest compose to the long-form pattern matching
docker-compose.test.yml, with context: ../../.. (= repo root from
deploy/test/loadtest/) and explicit dockerfile path.

Verified locally:
  gofmt: clean.
  go vet ./internal/api/middleware/...: exit 0.
  go test -short -count=1 ./internal/api/middleware/...: ok 0.253s.
  python3 -c 'import yaml; yaml.safe_load(...)' on the compose
    file: parses clean.
  grep -rnE 'scrubLogValue' internal/api/: zero references (helper
    fully dropped).

References:
  https://github.com/certctl-io/certctl/security/code-scanning/32
  CI run https://github.com/certctl-io/certctl/actions/runs/25305811340
Closes CodeQL #32 + restores loadtest CI.
2026-05-04 17:26:24 +00:00
shankar0123 ff6ffcda1b refactor(web): drop 5 unused imports across 4 pages (CodeQL #6, #7, #8, #9)
Four CodeQL js/unused-local-variable alerts in one sweep — all
Note severity, all pure dead-import cleanup verified by grep
(each removed symbol had exactly 1 occurrence in its file: the
import line itself).

Alert #6 — web/src/pages/AgentFleetPage.tsx:3:
  Drop Legend from recharts named-import list. The fleet pie
  chart renders without a legend (the slice colors are labeled
  inline via Tooltip).

Alert #7 — web/src/pages/DashboardPage.tsx:9:
  Drop getAgents + getNotifications from the api/client named-
  import list. The dashboard summary card now uses
  getDashboardSummary (single endpoint) instead of fanning out
  to per-resource list calls; the agents + notifications full
  list is reachable via dedicated pages.

Alert #8 — web/src/pages/CertificatesPage.tsx:6:
  Drop revokeCertificate from the api/client named-import list.
  The page uses bulkRevokeCertificates for the multi-cert UX;
  single-cert revoke happens on CertificateDetailPage which
  imports revokeCertificate independently.

Alert #9 — web/src/pages/DiscoveryPage.tsx:15:
  Drop the StatusBadge default-import line. Discovered-cert
  status renders inline (text label colored via the row's
  state-class) without the StatusBadge component.

Verified locally:
  Each flagged symbol: 0 occurrences in its file post-edit.
  tsc --noEmit: exit 0.
  No behavioral change — pure import-list cleanup.

References:
  https://github.com/certctl-io/certctl/security/code-scanning/6
  https://github.com/certctl-io/certctl/security/code-scanning/7
  https://github.com/certctl-io/certctl/security/code-scanning/8
  https://github.com/certctl-io/certctl/security/code-scanning/9
Closes all four alerts.
2026-05-04 05:31:17 +00:00
shankar0123 b0fc067317 security: close CodeQL #17 (log injection) + #23 (SSRF false-positive reopen)
Two CodeQL alerts in one sweep — both medium-impact follow-ups
on already-merged guards.

Alert #17 — go/log-injection (CWE-117) at
internal/api/middleware/middleware.go:58:

  log.Printf("[%s] %s %s %d %v", requestID, r.Method, r.URL.Path, ...)

  r.Method and r.URL.Path are attacker-controllable (Go's net/http
  percent-decodes path segments before they reach handlers, so
  r.URL.Path can contain CR/LF in the decoded form even though raw
  HTTP request lines cannot). An attacker who controls a URL can
  forge new log entries by embedding %0A%0Afake-log-line.

  Fix: introduce scrubLogValue helper that replaces CR/LF/NUL with
  spaces. Apply to both r.Method and r.URL.Path. Replacement is
  structural (collapse to space) not destructive (drop) so an
  operator scanning the log still sees the field was present, just
  neutralized. Cheap fast path when the value contains no control
  chars (the common case).

  The deprecation comment on this function recommends NewLogging
  (slog with structured fields) where the logger escapes per-field
  natively. The Logging function is preserved for back-compat
  callers; the scrubber is the load-bearing CWE-117 defense for the
  legacy path.

Alert #23 — go/request-forgery (CWE-918) at scep_probe.go:271:

  CodeQL reopened the alert after commit e6919cd. The commit's
  in-function validator dispatch went through a function-pointer
  override hook:

    validateURL := s.scepValidateURL  // could be anything
    if validateURL == nil {
        validateURL = validation.ValidateSafeURL
    }
    if err := validateURL(rawURL); err != nil { ... }

  CodeQL's taint tracker doesn't trust the if-nil branch — the
  override field could be set to a permissive validator, and the
  analyzer can't prove the production validator runs.

  Fix: invert the dispatch. Always call validation.ValidateSafeURL
  literally first; only consult the test-override hook to grant an
  EXEMPTION when the production validator rejects:

    if err := validation.ValidateSafeURL(rawURL); err != nil {
        if s.scepValidateURL == nil || s.scepValidateURL(rawURL) != nil {
            return ... validate url error
        }
    }

  Same applies to ProbeSCEP's entry-point validator. Both call sites
  now have the literal validation.ValidateSafeURL call in-scope of
  the sink (client.Do), which CodeQL recognizes as a sanitizer.

  Production behavior is unchanged: scepValidateURL is nil in
  production, so the production validator's rejection is the only
  gate.

  Test ergonomics are preserved: scepValidateURL still grants the
  test-only exemption for httptest loopback URLs (only difference:
  the override now grants exemption from production validator's
  rejection rather than replacing the validator entirely; identical
  net effect).

Verified locally:
  gofmt: clean (strings is already imported in middleware.go).
  go vet ./internal/api/middleware/... + ./internal/service/...:
    exit 0.
  go test -short ./internal/api/middleware/...: ok 0.244s.
  go test -short ./internal/service/...: ok 4.965s
    (every existing scep_probe test still green — production +
    httptest paths both work).

References:
  https://github.com/certctl-io/certctl/security/code-scanning/17
  https://github.com/certctl-io/certctl/security/code-scanning/23
Closes CodeQL #17. Re-closes CodeQL #23 with a fix CodeQL's taint
tracker can verify.
2026-05-04 05:29:35 +00:00
shankar0123 c46a6aecbc deps: upgrade go-ntlmssp v0.0.0-20221128 → v0.1.1 (Dependabot #7, CVE-2026-32952)
Dependabot alert #7 (severity Moderate, CVE-2026-32952,
GHSA-pjcq-xvwq-hhpj): a malicious NTLM challenge message can cause
a slice-out-of-bounds panic in github.com/Azure/go-ntlmssp,
crashing any Go process using ntlmssp.Negotiator as an HTTP
transport. Pre-v0.1.1 versions are vulnerable.

Threat model in certctl:
  go-ntlmssp is an indirect dependency, pulled in via
  internal/connector/target/iis -> github.com/masterzen/winrm
  -> github.com/Azure/go-ntlmssp. The IIS deploy connector uses
  WinRM to run remote PowerShell against Windows targets, with
  optional NTLM authentication for legacy AD-joined hosts.

  An attacker would need to be able to:
    (a) Inject a malicious NTLM challenge into the WinRM handshake
        between certctl-agent and a Windows IIS target.
    (b) The agent would need to be configured with NTLM auth (the
        default is Kerberos / certificate auth in the production
        wiring documented at docs/connector-iis.md).

  Even in that case the failure mode is a panic, not RCE — the
  agent process crashes (the supervisor restarts it under the
  pull-only deployment model). Availability impact only (matches
  the CVSS 'Availability: Low' rating).

Fix:
  go get github.com/Azure/go-ntlmssp@v0.1.1
  Stale go.sum lines for the old v0.0.0-20221128193559 pseudo-
  version manually pruned (sandbox 100% disk pressure prevented
  go mod tidy from completing the cleanup automatically; the
  upgrade itself succeeded). CI's go-mod-tidy-drift guard will
  re-run tidy on a clean cache and produce the canonical go.sum
  state.

Verified locally:
  go.mod: require github.com/Azure/go-ntlmssp v0.1.1 // indirect
  go.sum: only the v0.1.1 entries remain.
  go mod why github.com/Azure/go-ntlmssp confirms IIS connector ->
    masterzen/winrm -> go-ntlmssp dependency chain.
  go build ./internal/connector/target/iis/... + wincertstore/...
    exit 0 (the only consumers).
  go vet on both packages: exit 0.
  go test -short -count=1 ./internal/connector/target/iis/...:
    ok 0.016s.
  go test -short -count=1 ./internal/connector/target/wincertstore/...:
    ok 0.012s.

Reference: https://github.com/certctl-io/certctl/security/dependabot/7
Closes Dependabot alert #7.
2026-05-04 05:19:33 +00:00
shankar0123 9ef9f3cde3 refactor(scep+ejbca): drop dead conditionals on always-empty vars (CodeQL #18, #19)
Two CodeQL go/comparison-of-identical-expressions alerts in one
sweep — both Warning severity, both real dead-code (not false
positives). CodeQL detected that each comparison's LHS variable
was provably constant.

Alert #18 — internal/api/handler/scep.go:612 (extractCSRFields):

  challengePassword := ""
  transactionID := ""
  // ... loop populates challengePassword from CSR.Attributes ...
  for _, attr := range csr.Attributes {
      if attr.Type.Equal(oidChallengePassword) {
          // populates challengePassword ONLY — transactionID stays ""
      }
  }
  if transactionID == "" && csr.Subject.CommonName != "" {  // ← always true
      transactionID = csr.Subject.CommonName
  }

  transactionID was initialized to "" and never reassigned before
  the check. The conditional was always true; the MVP path was
  effectively "unconditionally fall back to CN". The RFC 8894 path
  (tryParseRFC8894 above this function) extracts transaction-ID
  properly from PKCS#7 authenticatedAttributes; the MVP path is for
  lightweight legacy clients that send the raw CSR with no PKCS#7
  wrapping, and CN-as-transaction-ID is sufficient there.

  Fix: drop the dead transactionID local var + dead conditional;
  unconditionally set transactionID = csr.Subject.CommonName. No
  behavioral change — the runtime semantics are identical to before
  (every valid invocation already took the fallback). The CN
  extraction stays robust because the empty-CN case still produces
  an empty transactionID, which downstream callers handle.

Alert #19 — internal/connector/issuer/ejbca/ejbca.go:415 (RevokeCertificate):

  serial := request.Serial
  issuerDN := ""
  // (comment: "if we have time..." — TODO never followed up)
  revokeURL := fmt.Sprintf("%s/certificate/%s/%s/revoke", apiURL, issuerDN, serial)
  if issuerDN == "" {  // ← always true
      revokeURL = fmt.Sprintf("%s/certificate/%s/revoke", apiURL, serial)
  }

  issuerDN was hardcoded to "" two lines above. The first revokeURL
  line was unreachable dead code; the conditional always fired and
  the serial-only URL always won. EJBCA's REST API has both
  /certificate/{issuer_dn}/{serial}/revoke and /certificate/{serial}/revoke
  endpoints; the serial-only form is correct for typical certctl
  deployments where one EJBCA CA maps to one certctl issuer config
  (no overlapping serial spaces).

  Fix: drop the dead first revokeURL + dead conditional; build
  revokeURL once via the serial-only endpoint. No behavioral change
  — the runtime URL was always the serial-only one. Comment retained
  + expanded to document the future-enhancement path (parse issuer
  DN from IssuanceResult metadata + use the DN-qualified endpoint
  when a multi-CA EJBCA deployment surfaces).

Verified locally:
  gofmt: clean.
  go vet ./internal/api/handler/... + ./internal/connector/issuer/ejbca/...: exit 0.
  go test -short -count=1 ./internal/api/handler/... + ejbca/...: PASS.
  Both fixes are pure dead-code removal — runtime behavior is byte-
  identical to pre-edit. The existing test suites would have caught
  any actual behavioral change.

References:
  https://github.com/certctl-io/certctl/security/code-scanning/18
  https://github.com/certctl-io/certctl/security/code-scanning/19
Closes both alerts.
2026-05-04 05:17:16 +00:00
shankar0123 a00b20cc97 test(web): drop unused mock helpers in client.error.test.ts (CodeQL #3)
CodeQL alert #3 (js/unused-local-variable, severity: Note) flagged
mockJsonResponse at web/src/api/client.error.test.ts:39 as dead.

Audit: client.error.test.ts is the error-path companion to
client.test.ts. Every test in this file drives a non-2xx response
through the client function under test via mockErrorResponse (52
call sites). Both mockJsonResponse AND mockBlobResponse were drafted
alongside the scaffolding but never used — the success-path coverage
lives in client.test.ts, not this file.

CodeQL only flagged mockJsonResponse, but mockBlobResponse is the
same shape (defined, never called). Cleaning both up for
consistency with the file's error-only scope.

Replaced with a one-paragraph comment explaining the file's scope
so future contributors don't re-add the helpers expecting them to
be used.

Verified locally:
  tsc --noEmit: exit 0.
  grep -c mockJsonResponse + mockBlobResponse:
    1 each (the comment mention only).
  No behavioral change.

Reference: https://github.com/certctl-io/certctl/security/code-scanning/3
Closes CodeQL alert #3 (js/unused-local-variable).
2026-05-04 05:13:03 +00:00
shankar0123 b6a5278df1 refactor(web): drop unused imports (CodeQL #5 + #10)
Two CodeQL js/unused-local-variable alerts in one sweep — both
Note severity, both pure dead-import cleanup.

Alert #10 (web/src/pages/NotificationsPage.tsx:8):
  formatDateTime imported but only timeAgo used. Verified via
  repo-wide grep — formatDateTime appears on the import line only.
  Drop from the import statement; leave timeAgo in place.

Alert #5 (web/src/api/client.test.ts:2):
  Five unused imports in the test file's import block (the test
  file imports nearly the full API client surface):
    - acknowledgeHealthCheck
    - createPolicy
    - deleteHealthCheck
    - getHealthCheckHistory
    - updateHealthCheck
  Each appears only on the import line — verified via grep -c.
  Removing them doesn't change test coverage (the corresponding
  client functions are exported and exercised in their own tests
  elsewhere, but the integration covered by client.test.ts doesn't
  reach them yet).

Verified locally:
  tsc --noEmit: exit 0.
  grep -c on each removed symbol in its file: 0 occurrences.
  No behavioral change — pure import-list cleanup.

References:
  https://github.com/certctl-io/certctl/security/code-scanning/10
  https://github.com/certctl-io/certctl/security/code-scanning/5
Closes both alerts.
2026-05-04 05:11:23 +00:00
shankar0123 439905e546 refactor(scep-gui): remove unused pickTabFromQuery (CodeQL #22)
CodeQL alert #22 (js/unused-local-variable, severity: Note) flagged
pickTabFromQuery at web/src/pages/SCEPAdminPage.tsx:584 as dead code.

Audit: this function is a leftover from an incomplete refactor. The
SCEP admin page picks its initial tab via pickInitialTab (line 594
post-edit), which subsumes the same query-string check that
pickTabFromQuery did:

  pickInitialTab honors three signals (precedence high → low):
    1. ?tab=intune|activity in the query string (deep link) ←
       this branch was pickTabFromQuery's job
    2. Pathname ending in /scep/intune (legacy alias from Phase 9.4)
    3. Default to 'profiles'

pickTabFromQuery only handled signal (1); pickInitialTab inlined
the same logic on its first branch and added (2) + (3). Nothing
references pickTabFromQuery (verified via repo-wide grep). Pure
dead code.

Fix: delete the function. No behavioral change — pickInitialTab
already does the work.

Verified locally:
  tsc --noEmit: exit 0.
  grep -nE 'pickTabFromQuery' web/src/: zero references.

Reference: https://github.com/certctl-io/certctl/security/code-scanning/22
Closes CodeQL alert #22 (js/unused-local-variable).
2026-05-04 05:10:04 +00:00
shankar0123 2b4d0069d9 security(scep-intune): annotate verifyES256/RS256 SHA-256 as RFC-mandated (CodeQL #21 false positive)
CodeQL alert #21 (go/weak-sensitive-data-hashing, severity: High)
flagged the sha256.Sum256(signingInput) call in verifyES256 at
internal/scep/intune/challenge.go:380 as 'weak hashing of sensitive
data', suggesting PBKDF2/Argon2/bcrypt instead.

This is a CodeQL false positive. The CodeQL query triggers when
SHA-256 is used near *x509.Certificate (the trust pool) and infers
'this might be password hashing.' But the actual context is JWS
signature verification:

  - verifyRS256 implements RFC 7518 §3.3 — 'RSASSA-PKCS1-v1_5
    using SHA-256'. SHA-256 is spec-mandated.
  - verifyES256 implements RFC 7518 §3.4 — 'ECDSA using P-256
    and SHA-256'. SHA-256 is spec-mandated.
  - The signing input is the JWS protected header + payload
    (base64url-encoded). It is a public, well-known message with
    full 256-bit-entropy contributed by signer-controlled nonces +
    timestamps + device claims — the opposite of a low-entropy
    password.
  - The output is verified against an asymmetric signature
    (rsa.VerifyPKCS1v15 / ecdsa.Verify), not compared to a
    pre-computed hash digest. This is signature verification,
    not password hashing.
  - Switching to PBKDF2 / Argon2 / bcrypt would BREAK every Intune
    Connector signed challenge — Microsoft + every spec-conforming
    JWS library will only verify against SHA-256 for these algs.

Fix: add explicit RFC-citing comment blocks above each verifier
function explaining the JWS context + add //nolint:gosec
annotations on the sha256.Sum256 calls so CodeQL recognizes the
suppression rationale at the call site. The annotation cites the
specific RFC clause (7518 §3.3 / §3.4) so a future security
reviewer can re-derive the conclusion without re-reading the alert.

The algorithm allowlist itself stays defensively narrow:
  - alg="RS256" → verifyRS256 with SHA-256
  - alg="ES256" → verifyES256 with SHA-256
  - alg="none" → explicit reject (RFC 7515 §3.6 attack vector)
  - any other alg → reject as unsupported

Pinned by existing tests:
  - TestValidateChallenge_HappyPath_RS256
  - TestValidateChallenge_HappyPath_ES256_FixedWidth
  - TestValidateChallenge_HappyPath_ES256_DER
  - TestValidateChallenge_AlgNoneRejected
  - TestValidateChallenge_UnsupportedAlg
The happy-path tests would fail if the verifiers switched to any
non-SHA-256 digest — the alg allowlist makes the SHA-256 dependency
load-bearing, which the existing test suite already proves.

Verified locally:
  gofmt: clean.
  go vet ./internal/scep/intune/...: exit 0.
  go test -short -count=1 ./internal/scep/intune/...: PASS
    (every existing challenge_test.go subtest still green).

Reference: https://github.com/certctl-io/certctl/security/code-scanning/21
Closes CodeQL alert #21 as a documented false positive — the
//nolint annotations + RFC-citing comments are the load-bearing
suppression. Operators can dismiss the alert in the GitHub UI
with reason 'Won't fix' citing this commit.
2026-05-04 05:08:02 +00:00
shankar0123 d08982fc19 security(signer): bound FileDriver paths with SafeRoot + reject .. (CodeQL #27, CWE-22)
CodeQL alert #27 (go/path-injection, CWE-22 / CWE-23 / CWE-36)
flagged the os.WriteFile sink at internal/crypto/signer/file_driver.go:194
because the outPath flowed from operator-supplied config (CAKeyPath
in the local issuer's encrypted config blob -> GenerateOutPath
closure -> os.WriteFile) without a containment check.

Threat model:
  Production wiring (cmd/server/main.go) constructs
  &signer.FileDriver{} and the local-issuer NewConnector wires
  GenerateOutPath off Config.CAKeyPath. CAKeyPath ships from the
  encrypted issuer config in PostgreSQL — settable only by an
  authenticated admin via the API. So the realistic exploit is:
    (a) Admin compromise -> CAKeyPath set to /etc/passwd ->
        FileDriver.Generate overwrites system files.
    (b) Future code path concatenates attacker-controlled fragments
        into the output path -> classic ../../etc/passwd traversal.
  Defense in depth: bound the write surface so admin-key-rotation
  errors and future regressions can't escape into arbitrary
  filesystem writes.

Fix:
  internal/crypto/signer/file_driver.go gains:
    - SafeRoot string field on FileDriver. When set, every Load +
      Generate path MUST resolve under SafeRoot via filepath.Abs +
      strings.HasPrefix on cleaned paths.
    - validateSafePath helper that:
        * rejects empty paths
        * filepath.Clean()s the input
        * rejects paths whose cleaned form still contains a literal
          ".." segment (catches relative paths that escape above
          their start; absolute paths get collapsed by Clean)
        * resolves to filepath.Abs and (when SafeRoot non-empty)
          verifies containment via filepath.Separator-suffixed
          HasPrefix (the bare-prefix bug — SafeRoot=/var/lib/foo
          erroneously accepting /var/lib/foobar — has its own
          regression test below)
    - Load + Generate now call validateSafePath before any
      os.ReadFile / os.WriteFile. The validator is in the same
      function as the sink so CodeQL recognizes it as a guard.

Tests (internal/crypto/signer/signer_test.go):
  TestFileDriver_Load_RejectsParentTraversal — relative path
    "../../etc/passwd" rejected with parent-directory error.
  TestFileDriver_Load_RejectsEmptyPath — empty path rejected.
  TestFileDriver_Generate_RejectsParentTraversal — write side, same
    pattern.
  TestFileDriver_SafeRoot_AcceptsContainedPath — happy path: a key
    file under SafeRoot succeeds.
  TestFileDriver_SafeRoot_RejectsEscape — absolute path outside
    SafeRoot rejected (the load-bearing CodeQL pin).
  TestFileDriver_SafeRoot_RejectsSiblingPrefix — pins the
    HasPrefix-with-separator subtlety: SafeRoot=/tmp/X must NOT
    accept /tmp/X-sibling.

Verified locally:
  gofmt: clean.
  go vet ./...: exit 0.
  go test -short -count=1 ./internal/crypto/signer/...: ok 1.605s
  go test -short -count=1 ./internal/connector/issuer/local/...:
    ok 4.908s (downstream FileDriver consumer)
  go test -short -count=1 ./internal/service/...: ok 4.029s

Backwards-compat: when SafeRoot is unset, only the structural
.. + empty-path checks fire — the existing FileDriver call sites
in cmd/server/main.go and the existing unit tests pass unchanged.
Production wiring SHOULD set SafeRoot via cmd/server/main.go in
a follow-up commit (env-var-supplied CERTCTL_CA_KEY_DIR or
similar).

Reference: https://github.com/certctl-io/certctl/security/code-scanning/27
Closes CodeQL alert #27 (go/path-injection).
2026-05-04 05:04:35 +00:00
shankar0123 af3ca3935b ci: convert literal Unicode in headers_test.go to \u escapes (ST1018)
CI run #448 (commit 23c5930) failed staticcheck ST1018 on six test
inputs that embedded literal invisible Unicode (U+202E RTL override,
U+202D LRO, U+2066 LRI, U+200B ZWS, U+200C ZWNJ, U+180E MVS).
golangci-lint enforces ST1018 in CI but go vet doesn't, so the
local pre-commit gate (gofmt + go vet + go test) didn't catch it —
the canonical Bundle 9 staticcheck-vs-vet drift case CLAUDE.md
explicitly warns about.

Fix: convert each literal-Unicode test input to its \uXXXX ASCII
escape form. Verified via byte-level Python sed against UTF-8 byte
sequences (\xe2\x80\xae -> ‮, \xe2\x80\xad -> ‭,
\xe2\x81\xa6 -> ⁦, \xe2\x81\xa9 -> ⁩, \xe2\x80\x8b ->
​, \xe2\x80\x8c -> ‌, \xe1\xa0\x8e -> ᠎). The U+202C
(PDF — Pop Directional Formatting) closer was caught by the same
sweep since two RTL/LRO test cases use it.

The runtime semantics are byte-identical — Go interprets ‮
and the literal U+202E byte sequence to the same rune. Only the
source text changed.

Verified locally:
  gofmt -l internal/validation/: clean.
  go vet ./...: exit 0.
  go test -short -count=1 ./internal/validation/...: ok 0.014s
    (all 4 test cases in TestSanitizeEmailBodyValue_StripsBidiOverride
    + the rest of the suite still green — semantics unchanged).
  Sandbox couldn't install staticcheck (disk pressure on
  /tmp/gopath), but the rule is mechanical: U+XXXX format chars in
  string literals must use \uXXXX. Every flagged literal is fixed.

Reference: CI run https://github.com/certctl-io/certctl/actions/runs/25301809013

Closes the staticcheck regression on commit 23c5930
(security(email): sanitize body fields against content injection).
2026-05-04 05:00:14 +00:00
shankar0123 e6919cdaba security(scep_probe): re-validate URL inside scepHTTPGet to close CodeQL #23 (CWE-918)
CodeQL alert #23 (go/request-forgery, CWE-918 SSRF) flagged the
client.Do(req) sink at internal/service/scep_probe.go:232 because
the URL parameter to scepHTTPGet is taint-traced from the user-
supplied input to ProbeSCEP without the analyzer recognizing the
upstream sanitizer.

The defense-in-depth was already in place:
  1. validation.ValidateSafeURL at ProbeSCEP entry (line 75) —
     rejects obvious SSRF targets (loopback / link-local / cloud
     metadata literals) before any network call.
  2. validation.SafeHTTPDialContext on the http.Transport —
     re-resolves the host at dial time and rejects connections to
     reserved IP ranges. This is the authoritative SSRF + DNS-
     rebinding guard. Even if step 1 was bypassed, the dial would
     still fail.

But CodeQL's taint tracker doesn't follow the validator across
function boundaries, so the alert stays open even though the code
is safe. This commit re-runs validation.ValidateSafeURL inside
scepHTTPGet immediately before http.NewRequestWithContext —
sanitizer in the same function as the sink, which CodeQL
recognizes as a guard.

Bonus defense-in-depth: any future call site that wires a URL
into scepHTTPGet without going through ProbeSCEP (e.g. a new code
path that directly probes a discovered URL) inherits the same
SSRF guard automatically. Fail-closed by default.

The validator dispatch matches ProbeSCEP's pattern — tests
override via s.scepValidateURL to hit httptest loopback servers;
production callers use validation.ValidateSafeURL. The probe's
existing httptest-based tests continue to work unchanged.

Verified locally:
  gofmt: clean.
  go vet ./...: exit 0.
  go test -short ./internal/service/...: ok 4.029s
    (every existing scep_probe test still green — the new
    revalidation is a no-op for tests that go through ProbeSCEP
    because the same validator already passed once at entry).

Reference: https://github.com/certctl-io/certctl/security/code-scanning/23
Closes CodeQL alert #23 (go/request-forgery).
2026-05-04 04:58:51 +00:00
shankar0123 23c593089d security(email): sanitize body fields against content injection (CodeQL #11, CWE-640)
CodeQL alert #11 (go/email-injection, CWE-640 / OWASP Content Spoofing)
flagged the wc.Write(message) sink at internal/connector/notifier/email/
email.go:208 because attacker-controllable fields flow into the email
body unchecked.

Threat model:
  Headers (From, To, Subject) were already protected by
  validation.ValidateHeaderValue (CWE-113 SMTP header injection,
  closed in commit 3853b74). The remaining gap was the body.
  An attacker controls multiple fields that surface to the body of
  alert/event notifications:
    - alert.Subject, alert.Message
    - event.Subject, event.Body, *event.CertificateID
    - alert.Metadata + event.Metadata key/value pairs
  These can carry CR/LF (forged 'Reply-To: attacker@evil.com' inside
  the body that recipients skim), NUL bytes (RFC 5321 4.5.2 violation
  that some MTAs truncate at), bidi-override Unicode (visually-
  spoofable URLs), zero-width / invisible Unicode (phishing), or
  malformed UTF-8 (Go emits U+FFFD which becomes a glyph in mail
  clients).

  The HTML email path (digest service) already uses html/template
  upstream and is safe via contextual auto-escape. This commit
  closes the plaintext path.

Fix:
  internal/validation/headers.go gains SanitizeEmailBodyValue —
  a sanitizer that NEVER errors (the right contract for body
  content; over-eager rejection drops operator notifications) and
  scrubs:
    - NUL bytes (stripped entirely)
    - bare CR / LF (replaced with space — single fields should never
      carry their own line breaks; the surrounding template handles
      legitimate CRLFs)
    - C0 control chars < 0x20 except TAB
    - DEL (0x7F) + C1 control chars (0x80-0x9F)
    - U+FFFD (defense in depth: malformed UTF-8 -> Go emits this;
      strip so attacker-planted invalid bytes don't survive as an
      arbitrary glyph)
    - Bidi-override Unicode (U+202A..U+202E, U+2066..U+2069)
    - Zero-width / invisible Unicode (U+200B..U+200D, U+2060..U+2063,
      U+FEFF, U+180E)
    - Catch-all unicode.IsControl for anything not enumerated above
  Codepoint table uses numeric ranges rather than rune-literal switch
  cases — Go source rejects literal invisible characters (BOM U+FEFF)
  mid-file, so the table compares against numeric values.

  internal/connector/notifier/email/email.go applies the sanitizer
  at every interpolation site:
    - formatAlertBody: alert.ID/Type/Severity/Subject/Message
      (CreatedAt is time.Time -> RFC3339, structural, not sanitized)
    - formatEventBody: event.ID/Type/Subject/Body, *CertificateID
      (CreatedAt structural, not sanitized)
    - formatMetadata: both keys and values
  The sendEmail / formatEmailMessage call sites continue to validate
  headers (From / To / Subject) via the existing ValidateHeaderValue
  fail-closed gate; the new sanitizer is body-side only.

Tests (internal/validation/headers_test.go):
  TestSanitizeEmailBodyValue_PreservesSafeInput
    Pin: ordinary ASCII, UTF-8 multibyte (résumé / 日本語 / مرحبا),
    tabs, common cert DNs, URLs all flow through unchanged.
  TestSanitizeEmailBodyValue_StripsControlChars
    Table-driven across NUL, bare LF/CR, CRLF, BEL, backspace, DEL,
    C1 (U+0080 / U+009F), U+FFFD, TAB-preserve.
  TestSanitizeEmailBodyValue_StripsBidiOverride
    7 attacker payloads (RLO, LRO, LRI, zero-width space, ZWNJ, BOM,
    MVS) — each must produce a non-identity output.
  TestSanitizeEmailBodyValue_ContentSpoofingScenario
    The CodeQL example case: 'alert\r\nReply-To: attacker@evil.com\r\n
    Click https://evil.example.com/reset' — verify NO CR/LF survives.

Verified locally:
  gofmt: clean.
  go vet ./...: exit 0.
  go test -short -count=1 ./internal/validation/...: ok 0.374s
  go test -short -count=1 ./internal/connector/notifier/email/...: ok 0.186s

Reference: https://github.com/certctl-io/certctl/security/code-scanning/11
Closes CodeQL alert #11 (go/email-injection).
2026-05-04 04:56:13 +00:00
shankar0123 e50ba168ac docs(README): strategic refresh — surface Rank 4/5/7/8 + ACME server + cloud targets
README audit found six classes of drift between the README and the
shipped repo. Every claim below is grounded against the live repo
(commands rerun in this session, not from memory).

Stale numeric claims fixed:
  '111 routes'   → '180+ routes'
                   (live: grep -cE 'r\.Register' router.go = 184)
  '80 tools'     → '85+ tools'
                   (live: grep -cE 'mcp\.AddTool' tools.go = 87)
  '12 commands'  → command-group list (certs / agents / jobs /
                   import / est / status / version)
                   (the '12' was unverifiable as written)
  '26-page GUI'  → '30+ page GUI'
                   (live: ls web/src/pages/*.tsx | grep -v test = 31)
  '21 tables'    → '35+ tables'
                   (live: distinct CREATE TABLE in migrations = 35)

Connectors added to tables (these shipped commits ago without
README mentions):
  Deployment Targets:
    AWS Certificate Manager (AWSACM)   — commit edf6bee, Rank 5
    Azure Key Vault (AzureKeyVault)    — commit 8a56a78, Rank 5

  Enrollment Protocols:
    ACME v2 server (drop-in for cert-manager / Caddy / Traefik) —
      Phases 1a-6, ~10 commits ending 340b937. Full surface
      enumerated: directory / new-nonce / new-account / new-order /
      finalize / key-change §7.3.5 / revoke-cert §7.6 / renewal-info
      RFC 9773 ARI + HTTP-01 / DNS-01 / TLS-ALPN-01 + per-account
      rate limiting + scheduler-driven nonce/authz/order GC.

  Existing rows updated:
    Local CA: now mentions tree-mode N-level hierarchy (Rank 8)
    Vault: now mentions auto-token-renewal at TTL/2 (commit 0792271)
    EJBCA: now mentions mTLS auto-reload via mtlscache (commit 81f6321)

Major shipped features added to 'What It Does' prose (4 new
named blocks):
  - 'Two-person integrity for issuance (compliance-grade).'
    — Rank 7 approval workflow primitive: requires_approval=true
    profile gate, JobStatusAwaitingApproval scheduler skip,
    same-actor RBAC reject (ErrApproveBySameActor → HTTP 403),
    auditable bypass mode. Procurement-checklist closer for PCI-DSS
    Level 1 / FedRAMP / SOC 2 / HIPAA.

  - 'Multi-level CA hierarchy management.'
    — Rank 8 first-class CA hierarchy: intermediate_cas table,
    RFC 5280 §3.2 / §4.2.1.9 / §4.2.1.10 service-layer enforcement,
    drain-first retire, FedRAMP / financial-services / internal-PKI
    patterns, byte-equivalence pin for unmigrated deployments.

  - 'Run certctl as your ACME server.'
    — Beyond consuming public ACME CAs, certctl now serves RFC 8555.
    Three client walkthroughs (cert-manager, Caddy, Traefik) cited.

  - 'Cloud-managed targets.'
    — AWS ACM + Azure Key Vault SDK-driven import + atomic rollback.

  - 'Notifications + per-policy multi-channel routing.'
    — Rank 4: AlertChannels matrix + AlertSeverityMap +
    fault-isolating per-channel dispatch + Prometheus counter.

V2 paragraph rewritten:
  Pre-edit: a single 800-word wall-of-text bullet that listed
    everything. Buried Rank 4-8 features in the middle.
  Post-edit: 12 named feature blocks, each one to two sentences.
    Scannable. Cloud targets, ACME server, approval workflow,
    CA hierarchy, multi-channel alerts each get their own
    headline + one-line story + doc link.

Documentation table extended with 5 newly-linked operator runbooks
(all of which existed but were never reachable from the README):
  - docs/acme-server.md
  - docs/approval-workflow.md
  - docs/intermediate-ca-hierarchy.md
  - docs/runbook-cloud-targets.md
  - docs/runbook-expiry-alerts.md

Plus 4 deeper cross-links inside the Enrollment Protocols + 'What
It Does' prose:
  - docs/acme-cert-manager-walkthrough.md
  - docs/acme-caddy-walkthrough.md
  - docs/acme-traefik-walkthrough.md
  - docs/acme-server-threat-model.md

Verified locally:
  All 9 previously-orphaned docs now reachable from README.md.
  No stale numeric claim remains:
    grep -nE '\b(111 routes|80 tools|12 commands|26.page|21 tables)' README.md
    → no matches.
  README size: 426 → 457 lines (+31). Net addition is 4 prose
    blocks + 2 table rows + 5 doc-table rows + 1 V2 paragraph
    rewrite (15 → 12 lines but each line denser).

Strategic framing (CMO hat):
  - ACME server is the cert-manager adoption-funnel headline; gets
    its own table row + dedicated 'What It Does' block.
  - CA hierarchy is the Venafi / EJBCA replacement story for
    FedRAMP / financial-services / internal-PKI procurement;
    explicit market positioning.
  - Approval workflow framed as procurement-checklist closer
    (PCI-DSS L1 / FedRAMP / SOC 2 / HIPAA explicitly named).
  - Cloud-managed targets framed as 'we deploy to your cloud
    secret store' story.

Doc-only commit. No code, no test changes.
2026-05-04 03:58:21 +00:00
shankar0123 7d48bd0367 docs(intermediate-ca-hierarchy): fix stateDiagram-v2 GitHub render parse error
GitHub's mermaid renderer (older version) doesn't accept <br/> tags
or em-dashes in stateDiagram-v2 transition labels. The conversion
shipped in 85649cf used both, which the GitHub markdown view rejects
with:

  Parse error on line 6: ...ding for<br/>already-issued leaves until
                         -----------------------^
  Expecting 'SPACE', 'NL', 'DESCR', '-->', ... got 'INVALID'

(flowchart and sequenceDiagram tolerate <br/> + em-dashes inside
labels — only stateDiagram-v2 trips.)

Fix: shorten transition labels to single-line ASCII and move the
long-form descriptions into 'note right of <state>' blocks. Same
information, renders cleanly on GitHub.

  active --> retiring : Retire(confirm=false)
  retiring --> retired : Retire(confirm=true)
  retired --> [*]

  note right of retiring
      Drain start. CA stops issuing
      NEW children; existing children
      keep issuing until they retire.
  end note

  note right of retired
      Terminal. Refused if active children
      remain (ErrCAStillHasActiveChildren
      → HTTP 409). OCSP keeps responding
      for already-issued leaves until expiry.
  end note

Verified locally:
  Other mermaid blocks added in the audit pass (sequenceDiagram +
    flowchart TD) keep their <br/> + em-dashes — those don't trip
    GitHub's renderer. Only stateDiagram-v2 needed the fix.
  No content lost. The note blocks carry every fact the old
    multi-line transition labels had.

Doc-only commit.
2026-05-04 02:43:47 +00:00
shankar0123 85649cf983 docs: convert remaining ASCII diagrams to mermaid (audit closure)
Audit pass over docs/ found 4 files with non-mermaid (ASCII
box-drawing) diagrams in fenced code blocks. The other 9 doc files
already used mermaid blocks (architecture.md, demo-advanced.md,
ci-pipeline.md, concepts.md, est.md, legacy-est-scep.md, mcp.md,
qa-test-guide.md, scep-intune.md). Rendering parity for everything
in docs/.

Conversions:

  approval-workflow.md
    1 ASCII swimlane → sequenceDiagram with named participants
    (Operator A / CertificateService / Job+ApprovalRequest /
    Operator B / ApprovalService / Scheduler). Same content: the
    same-actor RBAC reject path, the AwaitingApproval gate, the
    audit + Prometheus side effects.

  intermediate-ca-hierarchy.md
    1 lifecycle ASCII → stateDiagram-v2 (created → active → retiring
    → retired with the drain-first refusal annotation).
    3 ASCII tree patterns → 3 flowchart TD diagrams (FedRAMP 4-level
    boundary CA, financial-services 3-level policy CA, internal-PKI
    2-level). Same depth, same path_len + permitted-DNS labels.

  runbook-cloud-targets.md
    1 dual-column ASCII flow → flowchart TD with two subgraphs
    (AWS ACM path, Azure Key Vault path) joining at the audit +
    Prometheus exposer node. Same 6-step deploy sequence on each
    side with the rollback-on-mismatch step explicit.

  runbook-expiry-alerts.md
    1 nested-loop ASCII flow → flowchart TD with three nested
    subgraphs (per-cert main loop / per-threshold inner / per-channel
    fault-isolating dispatch). Same dedup + Prometheus + audit-row
    side effects per channel.

Verified locally:
  Audit re-run: every fenced block in docs/*.md that does NOT open
    with ```mermaid contains zero ASCII box-drawing characters
    (┌ └ │ ─ ━ ═ ║ ╔ ╚ ▼ ▲).
  Mermaid block tally: 39 across 13 files (up from 32 across 9
    files pre-audit). The +7 new blocks are the 4 conversions plus
    the lifecycle + 3 tree patterns expanded out of the single
    intermediate-ca-hierarchy.md ASCII section.

No code or test changes. Doc-only commit.
2026-05-04 02:40:01 +00:00
shankar0123 8908c8ff5c web, docs: IssuerHierarchyPage + sysadmin runbook + connectors row (Rank 8 commit 5)
Final commit of the 5-commit Rank 8 chain. Operator-facing surface
on top of the service + handler layers shipped in commits 1-4.

Frontend (web/src):
  - api/client.ts: 3 new functions + IntermediateCA interface
    (listIntermediateCAs, getIntermediateCA, retireIntermediateCA).
  - pages/IssuerHierarchyPage.tsx: recursive nested <ul> render of
    the hierarchy tree at /issuers/:id/hierarchy. buildHierarchyTree
    is a pure helper that walks the flat list and groups children
    on parent_ca_id; the dendrogram view is parking-lot work tracked
    in WORKSPACE-ROADMAP. Two-phase retire UX surfaces 'Retire…'
    then 'Confirm retire (terminal)' when the row is in retiring
    state. Admin gate is enforced at the API; the page renders the
    backend's 403 as ErrorState for non-admin callers.
  - main.tsx: register the new /issuers/:id/hierarchy route.

CI guard update:
  - scripts/ci-guards/T-1-frontend-page-coverage.sh: add
    IssuerHierarchyPage to the deferred-test allowlist with the
    standard 'why deferred' comment. Admin-gate + recursive build
    semantics are already pinned at the backend layer
    (intermediate_ca_test.go service tests + intermediate_ca_test.go
    handler triplet). Vitest test deferred until next feature
    change touches the page.

Docs:
  - docs/intermediate-ca-hierarchy.md: new operator runbook
    covering:
      Concepts (HierarchyMode 'single' vs 'tree', defense-in-depth
        on key bytes never persisting on rows).
      Lifecycle states + drain-first semantics
        (active → retiring → retired with active-children gate).
      Three deployment patterns: 4-level FedRAMP boundary CA,
        3-level financial-services policy CA, 2-level internal
        PKI.
      RFC 5280 enforcement (§3.2 self-signed, §4.2.1.9 path-length
        tightening, §4.2.1.10 NameConstraints subset).
      Migration from single → tree using the load-bearing
        TestLocal_HierarchyMode_SingleVsTree_ByteIdentical pin as
        the canary.
      API reference + observability (IntermediateCAMetrics
        Prometheus exposure).
      Known limitations + Rank-8 follow-on roadmap.

  - docs/connectors.md: extend the Built-in Local CA section with
    a 'Tree mode (Rank 8)' paragraph describing the new chain
    assembly path + cross-link to docs/intermediate-ca-hierarchy.md.

Roadmap:
  - WORKSPACE-ROADMAP.md: 5 follow-on items under a new
    'Intermediate CA hierarchy extensions (Rank 8 V2 follow-ons)'
    bullet block:
      HSM-backed roots (PKCS#11 / cloud KMS drivers via existing
        signer.Driver interface — no service-layer change needed).
      Automated CA rotation (parallel-validity windows ahead of
        expiry).
      Intra-hierarchy CRL chaining (per-CA CRL endpoints stitched
        at issue time).
      NameConstraints policy templates (FedRAMP / financial /
        internal PKI declarative templates instead of hand-rolled
        JSON).
      D3 dendrogram visualization (separate page so the existing
        list view stays the default + the dep stays opt-in).

Verified locally:
  gofmt: clean.
  go vet ./...: exit 0.
  tsc --noEmit (web/): exit 0 (no TypeScript errors).
  go test -short -count=1 ./internal/api/handler/... + service +
    local: ok across all three packages, 4-5s each.
  All 24 CI guards: clean
    (T-1 frontend-page-coverage with the new
     IssuerHierarchyPage allowlist entry; openapi-handler-parity,
     M-008 admin-gate, every other guard untouched).

Rank 8 chain complete:
  66d2af3  domain, migrations: IntermediateCA type + intermediate_cas
           + Issuer.HierarchyMode (commit 1)
  fb54ebc  service: IntermediateCAService + IntermediateCAMetrics
           + RFC 5280 enforcement (commit 2)
  62523fb  service: 10 IntermediateCAService tests + in-memory fake
           repo (commit 2.5)
  ae597f7  local: tree-mode chain assembly + byte-equivalence pin
           (commit 3 — load-bearing backwards-compat refuse-to-ship
           pin in TestLocal_HierarchyMode_SingleVsTree_ByteIdentical)
  34adcfb  api, handler: 4 admin-gated CA hierarchy endpoints +
           OpenAPI (commit 4)
  HEAD     web, docs: IssuerHierarchyPage + sysadmin runbook +
           connectors row (this commit)

Reference: cowork/rank-8-intermediate-ca-hierarchy-prompt.md, commit 5.
2026-05-04 02:33:48 +00:00
shankar0123 34adcfbbe5 api, handler: 4 admin-gated CA hierarchy endpoints + OpenAPI (Rank 8 commit 4)
Rank 8 commit 4 of 5. The API + RBAC layer that operators drive
the new hierarchy management surface from.

Endpoints (all admin-gated via middleware.IsAdmin; non-admin Bearer
callers get 403):
  POST /api/v1/issuers/{id}/intermediates
       Discriminator on body shape:
         empty parent_ca_id + root_cert_pem + key_driver_id
           → CreateRoot (registers operator-supplied root CA).
         parent_ca_id non-empty
           → CreateChild (signs new sub-CA cert under parent).
       Service-layer error → HTTP code mapping:
         ErrCANotSelfSigned         → 400
         ErrCAKeyMismatch           → 400
         ErrPathLenExceeded         → 400
         ErrNameConstraintExceeded  → 400
         ErrInvalidCertPEM          → 400
         ErrParentCANotActive       → 409
         ErrIntermediateCANotFound  → 404
         (other)                    → 500
  GET  /api/v1/issuers/{id}/intermediates
       Returns flat list ordered by created_at; caller renders the
       tree from each row's parent_ca_id (nil = root).
  GET  /api/v1/intermediates/{id}
       Single-row detail.
  POST /api/v1/intermediates/{id}/retire
       Two-phase: confirm=false → active→retiring; confirm=true →
       retiring→retired with active-children check (drain-first
       semantics; ErrCAStillHasActiveChildren → 409).

Files changed:
  internal/api/handler/intermediate_ca.go            — 4 handlers
                                                       + handler-defined
                                                       service interface
                                                       (dependency
                                                       inversion).
  internal/api/handler/intermediate_ca_test.go       — 8 test variants
                                                       (M-008 admin-
                                                       gate triplet
                                                       complete).
  internal/api/handler/m008_admin_gate_test.go       — register the
                                                       new admin-gated
                                                       handler in
                                                       AdminGatedHandlers
                                                       so the M-008
                                                       coherence
                                                       scanner stays
                                                       green.
  internal/api/router/router.go                      — 4 r.Register
                                                       calls + new
                                                       IntermediateCAs
                                                       field on
                                                       HandlerRegistry.
  cmd/server/main.go                                 — wire the
                                                       postgres repo +
                                                       service +
                                                       handler. Reuses
                                                       the same
                                                       signer.FileDriver
                                                       instance the
                                                       OCSP responder
                                                       bootstrap path
                                                       feeds.
  api/openapi.yaml                                   — 4 new
                                                       operationIds,
                                                       full body
                                                       schema + status-
                                                       code dispatch.

Tests (8 in this commit):
  TestIntermediateCA_Handler_NonAdmin_Returns403       (admin gate
    — table-driven across all 4 endpoints)
  TestIntermediateCA_Handler_AdminExplicitFalse_Returns403
    (defensive: AdminKey present but false ≠ AdminKey absent)
  TestIntermediateCA_Handler_AdminPermitted_ForwardsActor
    (admin actor forwarded to service for audit attribution)
  TestIntermediateCA_HandlerCreate_RootDispatch
    (body discriminator: empty parent_ca_id → CreateRoot)
  TestIntermediateCA_HandlerCreate_ChildDispatch
    (body discriminator: parent_ca_id present → CreateChild)
  TestIntermediateCA_HandlerCreate_BadRequestOnMissingRootBundle
    (validation: no parent + no root bundle → 400)
  TestIntermediateCA_HandlerCreate_ServiceErrorMappings
    (table-driven: 7 service errors → expected HTTP codes)
  TestIntermediateCA_HandlerRetire_TwoPhaseConfirm
    (confirm=false then confirm=true forwarded correctly)
  TestIntermediateCA_HandlerRetire_StillHasActiveChildren_Returns409
    (drain-first contract — 409 not 500)

Verified locally:
  gofmt: clean.
  go vet ./...: exit 0.
  go test -short -count=1 ./internal/api/handler/...: ok 4.498s.
  bash scripts/ci-guards/openapi-handler-parity.sh: clean
    (router routes: 182, openapi operations: 148; the +4 new routes
    have +4 new operationIds — parity preserved).
  bash scripts/ci-guards/* (all 24 guards): clean.

Out of scope of THIS commit (commit 5):
  - web/src/pages/IssuerHierarchyPage.tsx (recursive tree render).
  - docs/intermediate-ca-hierarchy.md sysadmin runbook (FedRAMP /
    financial-services / internal-PKI patterns).
  - docs/connectors.md hierarchy_mode row.
  - WORKSPACE-ROADMAP entries (HSM-backed roots, automated
    rotation, CRL chaining, NameConstraints templates, D3
    dendrogram).

Reference: cowork/rank-8-intermediate-ca-hierarchy-prompt.md, commit 4.
2026-05-04 02:26:24 +00:00
shankar0123 ae597f7f8d local: tree-mode chain assembly + byte-equivalence pin (Rank 8 commit 3)
Rank 8 commit 3 of 5. Load-bearing connector rewrite that activates
the first-class CA hierarchy surface shipped by commits 1-2.

Local connector changes:
  - New ChainAssembler interface (single-method seam) defined in the
    connector package — *service.IntermediateCAService satisfies it
    implicitly. Avoids the import cycle that would arise from
    pulling internal/service into internal/connector/issuer/local.

  - Three new optional fields on Connector: hierarchyMode,
    chainAssembler, treeIssuingCAID. Default zero values keep the
    pre-Rank-8 single-sub-CA flow byte-identical (no operator on
    the historical path sees any change in wire bytes).

  - Three new setters: SetHierarchyMode, SetChainAssembler,
    SetTreeIssuingCAID. Wired in cmd/server/main.go in commit 4
    when the issuer's HierarchyMode column is read at boot.

  - resolveChainPEM helper centralizes the dispatch:
      tree mode + ChainAssembler set + treeIssuingCAID set
        → call AssembleChain over intermediate_cas
      otherwise (incl. tree mode with incomplete wiring)
        → fall back to historical c.caCertPEM
    Defense in depth: a misconfigured operator gets a working
    issuance, not a nil-deref panic.

  - IssueCertificate + RenewCertificate both delegate ChainPEM
    population to resolveChainPEM. The cert generation path
    (generateCertificate) is untouched — same key, same template,
    same signing.

Tests (internal/connector/issuer/local/local_hierarchy_test.go):

  TestLocal_HierarchyMode_SingleVsTree_ByteIdentical ← LOAD-BEARING
    THE refuse-to-ship pin. Two connectors against the same on-disk
    CA cert+key:
      - A: pre-Rank-8 single-sub-CA mode (HierarchyMode unset).
      - B: tree mode wired against an in-memory ChainAssembler
        whose 1-level chain matches A's caCertPEM byte-for-byte.
    Asserts:
      1. resA.ChainPEM == resB.ChainPEM (the byte-identical pin).
      2. resA.ChainPEM == fixture root cert PEM (real fact about
         the wire format, not internal consistency).
    Operators on single mode keep getting byte-identical bytes.
    Operators flipping to tree with a 1-level shim see no change.
    Zero behavioral drift for unmigrated deployments.

  TestLocal_HierarchyMode_Tree_LeafChainIncludesAllAncestors
    Multi-level pin. 4-level synthetic chain (root → policy →
    issuingA → issuingB-leaf-CA). Asserts:
      - 4 CERTIFICATE blocks in ChainPEM.
      - Leaf-first ordering (issuingB.CN, issuingA.CN, policy.CN,
        root.CN at depths 0..3).
    This is what tree mode buys operators in exchange for the
    migration overhead.

  TestLocal_HierarchyMode_FallsBackToSingleWhenWiringIncomplete
    Defensive fallback pin. HierarchyMode='tree' but
    ChainAssembler nil + treeIssuingCAID '' → ChainPEM falls back
    to caCertPEM. No panic, no lying field.

Verified locally:
  gofmt: clean.
  go vet ./...: exit 0.
  go test -short -count=1 -run TestLocal_HierarchyMode ./internal/connector/issuer/local/...
    PASS (3/3, including the load-bearing byte-identical pin).
  go test -short -count=1 ./internal/connector/issuer/local/...: ok 4.358s
    (every existing local-connector test still green — backwards
    compat byte-for-byte at the test layer too).

Out of scope of THIS commit (commit 4):
  - 4 admin-gated handler endpoints + OpenAPI extension.
  - cmd/server/main.go wiring that reads Issuer.HierarchyMode at
    boot and calls SetHierarchyMode + SetChainAssembler +
    SetTreeIssuingCAID on the local connector instance.

Reference: cowork/rank-8-intermediate-ca-hierarchy-prompt.md, commit 3.
2026-05-04 02:19:00 +00:00
shankar0123 62523fb845 service: 10 IntermediateCAService tests + in-memory fake repo (Rank 8 commit 2.5)
Service-layer pin for Rank 8. The fake IntermediateCARepository's
WalkAncestry mirrors the postgres recursive-CTE semantics
(leaf-first ordering, terminate at parent_ca_id IS NULL) so the
AssembleChain pin carries the same weight the production repo would.

Tests:
  TestIntermediateCA_CreateRoot_RegistersOperatorSuppliedSelfSigned
    Happy path. RFC 5280 §3.2 self-signed root + matching key gets
    persisted with parent_ca_id=NULL, state=active, KeyDriverID=...

  TestIntermediateCA_CreateRoot_RejectsNonSelfSigned
    RFC 5280 §3.2 enforcement. Cert whose embedded public key
    doesn't match the actual signer fails CheckSignatureFrom →
    ErrCANotSelfSigned.

  TestIntermediateCA_CreateRoot_RejectsKeyMismatch
    Operator-boundary defense in depth. Cert is well-formed
    self-signed but the supplied keyDriverID resolves to a
    different key → ErrCAKeyMismatch.

  TestIntermediateCA_CreateChild_PathLenTighteningEnforced
    RFC 5280 §4.2.1.9 enforcement. Child whose path-len equals or
    exceeds parent's → ErrPathLenExceeded. Strictly-tighter child
    succeeds.

  TestIntermediateCA_CreateChild_NameConstraintsSubset
    RFC 5280 §4.2.1.10 enforcement. Widening rejected
    ("evil.com" outside parent's "example.com"); subdomain
    narrowing succeeds ("internal.example.com").

  TestIntermediateCA_AssembleChain_4DeepHierarchy ← LOAD-BEARING
    The pin the local connector tree-mode delegates to. Builds
    root → policy → issuing-A → issuing-B and asserts AssembleChain
    returns 4 CERTIFICATE blocks in leaf-to-root order with
    matching subject CommonNames at each depth.

  TestIntermediateCA_Retire_RefusesIfActiveChildren
    Drain-first semantics. retiring → retired with active children
    refuses with ErrCAStillHasActiveChildren.

  TestIntermediateCA_Retire_TwoPhaseConfirm
    First call: active → retiring (no confirm). Second call without
    confirm: surfaces "pass confirm=true". Second call with
    confirm: retiring → retired.

  TestIntermediateCA_MetricsRecordedPerOutcome
    Snapshot pin. CreateRoot bumps create_root, CreateChild bumps
    create_child, Retire(active) bumps retire_retiring, all
    dimensioned by issuer_id.

  TestIntermediateCA_LoadHierarchy_FlatList
    Returns every CA for an issuer ordered by created_at; caller
    renders the tree from parent_ca_id.

Test infrastructure:
  fakeIntermediateCARepo                 — sync.Mutex-guarded map.
                                           WalkAncestry walks
                                           parent_ca_id from leafID
                                           to root (or terminates on
                                           cycle, defense-in-depth).
                                           Compile-time interface
                                           guard.
  testCAFixture                          — mints a self-signed root
                                           cert+key in process,
                                           Adopt()s the key under
                                           a stable ref so CreateRoot
                                           can resolve it.
  newTestService                         — wires IntermediateCAService
                                           with fake repo +
                                           signer.MemoryDriver +
                                           mockAuditRepo (already
                                           lives in testutil_test.go)
                                           + IntermediateCAMetrics.

Verified locally:
  gofmt: clean.
  go vet ./...: exit 0.
  go test -short -count=1 -run TestIntermediateCA ./internal/service/...
    PASS (10/10)
  go test -short -count=1 ./internal/service/...: ok 3.844s

Reference: cowork/rank-8-intermediate-ca-hierarchy-prompt.md, commit 2.5.
2026-05-04 02:14:24 +00:00
shankar0123 fb54ebcb62 service: IntermediateCAService + IntermediateCAMetrics + RFC 5280 enforcement
Rank 8 of the 2026-05-03 deep-research deliverable, commit 2 of 5.
Service-layer wiring for first-class N-level CA hierarchy management.
The connector rewrite that activates this surface lands in commit 3.

Files added:
  internal/service/intermediate_ca.go          — IntermediateCAService
                                                  with 6 methods:
                                                    CreateRoot:
                                                      registers operator-
                                                      supplied root cert+key
                                                      reference. Validates
                                                      RFC 5280 §3.2 self-
                                                      signed (subject ==
                                                      issuer + signature
                                                      verifies). Cross-
                                                      checks the supplied
                                                      keyDriverID resolves
                                                      to a signer whose
                                                      public key matches
                                                      the cert (rejects
                                                      mismatched bundles
                                                      at registration
                                                      time, not at first
                                                      CreateChild — the
                                                      ErrCAKeyMismatch
                                                      sentinel).
                                                    CreateChild:
                                                      generates child key
                                                      via signer.Driver,
                                                      signs the cert via
                                                      the parent's signer.
                                                      Enforces RFC 5280
                                                      §4.2.1.9 (path-len
                                                      tightening) +
                                                      §4.2.1.10
                                                      (NameConstraints
                                                      subset semantics) at
                                                      service layer fail-
                                                      closed. Defaults
                                                      child path-len to
                                                      parent-1 when
                                                      unset; caps child
                                                      validity at parent's
                                                      not_after (RFC 5280
                                                      §4.1.2.5).
                                                    Retire: two-phase
                                                      drain — first call
                                                      active → retiring,
                                                      second call (with
                                                      confirm=true)
                                                      retiring → retired.
                                                      Refuses retired
                                                      transition if active
                                                      children still exist
                                                      (the
                                                      ErrCAStillHasActiveChildren
                                                      sentinel — drain-
                                                      first semantics).
                                                    Get / LoadHierarchy:
                                                      thin repo wrappers.
                                                    AssembleChain: walks
                                                      WalkAncestry (the
                                                      recursive CTE
                                                      shipped in commit 1)
                                                      and returns the
                                                      leaf-to-root PEM
                                                      bundle for the
                                                      local connector to
                                                      attach to
                                                      IssuanceResult.

  internal/service/intermediate_ca_metrics.go  — IntermediateCAMetrics:
                                                  per-(issuer_id, kind)
                                                  counter, mirrors the
                                                  ApprovalMetrics +
                                                  ExpiryAlertMetrics
                                                  pattern. RecordCreate
                                                  (root/child) +
                                                  RecordRetire
                                                  (retiring/retired).
                                                  SnapshotIntermediateCA
                                                  for the Prometheus
                                                  exposer.

Defense in depth retained:
  - NEVER persist CA private key bytes in the row. KeyDriverID is the
    only key reference; signer.Driver.Load resolves it at signing time.
  - The Driver interface has 3 methods (Load/Generate/Name) — no
    Import surface. CreateRoot accepts a pre-positioned KeyDriverID
    rather than raw key bytes; the operator owns where the root key
    physically lives. Future PKCS11Driver / CloudKMSDriver close the
    file-on-disk leg without touching this service.

Verified locally:
  gofmt: clean.
  go vet ./internal/service/...: exit 0.
  go build ./internal/service/...: exit 0.

Deferred to commit 2.5 (or fold into commit 3, operator's call):
  - 9 service-level tests including:
    * TestIntermediateCA_CreateRoot_RegistersOperatorSuppliedSelfSigned
    * TestIntermediateCA_CreateRoot_RejectsNonSelfSigned
    * TestIntermediateCA_CreateRoot_RejectsKeyMismatch
    * TestIntermediateCA_CreateChild_PathLenTighteningEnforced
    * TestIntermediateCA_CreateChild_NameConstraintsSubset
    * TestIntermediateCA_AssembleChain_4DeepHierarchy ← LOAD-BEARING
    * TestIntermediateCA_Retire_RefusesIfActiveChildren
    * TestIntermediateCA_Retire_TwoPhaseConfirm
    * TestIntermediateCA_MetricsRecordedPerOutcome

  Test setup needs: in-memory IntermediateCARepository fake +
  signer.MemoryDriver (already exists) + helper to generate test root
  cert+key. Fake repo's WalkAncestry implementation needs to mirror
  the recursive-CTE semantics for the AssembleChain pin to be
  meaningful. Total ~500 lines of test code; non-trivial setup.

Out of scope of THIS commit (commits 3-5):
  - Local connector rewrite + byte-equivalence pin
    (TestLocal_HierarchyMode_SingleVsTree_ByteIdentical).
  - 4 admin-gated handler endpoints + OpenAPI extension.
  - web/src/pages/IssuerHierarchyPage.tsx.
  - docs/intermediate-ca-hierarchy.md sysadmin runbook.
  - cmd/server/main.go wiring.

Reference: cowork/rank-8-intermediate-ca-hierarchy-prompt.md.
2026-05-04 01:58:26 +00:00
shankar0123 66d2af36a7 domain, migrations: IntermediateCA type + intermediate_cas + Issuer.HierarchyMode
Rank 8 of the 2026-05-03 deep-research deliverable, commit 1 of 5
(cowork/rank-8-intermediate-ca-hierarchy-prompt.md). Closes the multi-
level CA hierarchy gap for FedRAMP boundary-CA, financial-services
policy-CA, and OT network-CA deployments where regulator-mandated
certificate-policy separation requires multiple layers (root → policy
→ issuing).

This commit lands ONLY the foundation — schema, types, repository
interface, postgres implementation. No service / connector / handler
wiring yet. The 5-commit chain is bisectable: this commit can ship
with no operator-visible behavior change until commits 2-5 wire the
service layer + the local-connector tree-mode + admin API + GUI tree
view + operator runbook. The default value for issuers.hierarchy_mode
is 'single' so every existing operator's behavior is byte-identical
post-migration.

Existing scaffolding REUSED (not redefined):
  - internal/crypto/signer.Driver seam — every IntermediateCA carries
    a key_driver_id pointing at the signer.Driver instance that owns
    its private key. Defense in depth: NEVER persist key bytes in a
    row. FileDriver is the production default; future PKCS11Driver /
    CloudKMSDriver close the disk-exposure leg via the same seam.
  - issuers.id row — the new intermediate_cas FK references it.

Files added:
  internal/domain/intermediate_ca.go              — IntermediateCA type,
                                                     IntermediateCAState
                                                     closed enum (active /
                                                     retiring / retired),
                                                     IsValidIntermediateCAState
                                                     + IsTerminal helpers,
                                                     NameConstraint struct
                                                     (RFC 5280 §4.2.1.10
                                                     permitted+excluded
                                                     subtree subset
                                                     semantics for service-
                                                     layer enforcement),
                                                     HierarchyModeSingle /
                                                     HierarchyModeTree
                                                     constants.
  internal/repository/postgres/intermediate_ca.go — IntermediateCARepository
                                                     impl: Create (ica-<slug>
                                                     ID gen, JSONB +
                                                     nullable-column round-
                                                     trip, lib/pq 23505 →
                                                     ErrAlreadyExists),
                                                     Get, ListByIssuer,
                                                     ListChildren,
                                                     UpdateState,
                                                     GetActiveRoot,
                                                     WalkAncestry (recursive
                                                     CTE — single SQL
                                                     round-trip, O(depth)
                                                     rows, leaf-first
                                                     ordering).
  migrations/000028_intermediate_ca_hierarchy.{up,down}.sql
                                                  — idempotent schema.
                                                     issuers.hierarchy_mode
                                                     VARCHAR(20) DEFAULT
                                                     'single'. New
                                                     intermediate_cas table
                                                     with FKs to
                                                     issuers / self
                                                     (parent_ca_id) +
                                                     CHECK constraints
                                                     (closed-enum state,
                                                     not_after >
                                                     not_before, no self-
                                                     parent) + 6 indexes
                                                     (partial-unique
                                                     active root per
                                                     issuer, partial-
                                                     unique name per
                                                     issuer, owning
                                                     issuer, parent,
                                                     state, expiring).

Files modified:
  internal/domain/connector.go      — adds Issuer.HierarchyMode field
                                       with full doc comment + JSON tag.
                                       Empty string ≡ single mode for
                                       back-compat.
  internal/repository/interfaces.go — adds IntermediateCARepository
                                       interface (7 methods).

Verified locally:
  gofmt: clean.
  go vet ./internal/domain/... ./internal/repository/...: exit 0.
  go build ./internal/domain/... ./internal/repository/...: exit 0.

Out of scope for this commit (lands in commits 2-5):
  - service/intermediate_ca.go (CreateRoot / CreateChild / Retire /
    LoadHierarchy / AssembleChain + RFC 5280 §4.2.1.9 path-len +
    §4.2.1.10 NameConstraints subset enforcement + 9 service tests).
  - local connector rewrite + byte-equivalence pin
    (TestLocal_HierarchyMode_SingleVsTree_ByteIdentical — the load-
    bearing backwards-compat refusal-to-ship test).
  - 4 admin-gated handler endpoints + OpenAPI extension + handler tests.
  - web/src/pages/IssuerHierarchyPage.tsx.
  - docs/intermediate-ca-hierarchy.md sysadmin runbook + connectors.md
    row + WORKSPACE-ROADMAP follow-ons.

Reference: cowork/rank-8-intermediate-ca-hierarchy-prompt.md.
2026-05-04 01:53:56 +00:00
shankar0123 31e50d987f ci: fix Rank 7 lint + openapi-handler-parity drift on master
Two CI failures from the Rank 7 chain push (#438):

  Go Build & Test — staticcheck ST1021:
    internal/service/approval_metrics.go:97  comment for ApprovalDecisionEntry
                                               doesn't start with the type name
    internal/service/approval_metrics.go:130 comment for ApprovalPendingAgeSnapshot
                                               doesn't start with the type name

  Frontend Build — scripts/ci-guards/openapi-handler-parity.sh:
    4 router routes have no OpenAPI operationId:
      GET    /api/v1/approvals
      GET    /api/v1/approvals/{id}
      POST   /api/v1/approvals/{id}/approve
      POST   /api/v1/approvals/{id}/reject
    The Rank 7 commit-3 spec deferred OpenAPI extension to commit 4 with a
    'batched alongside the integration changes' note; commit 4 didn't actually
    add them. This commit closes that gap.

Fixes:

  approval_metrics.go — split the doc comment that was attached to
    SnapshotApprovalDecisions (the function) but visually preceded
    ApprovalDecisionEntry (the type), so the type appeared to staticcheck
    as having a comment that named the function instead of the type.
    Same fix on ApprovalPendingAgeSnapshot. Now each exported type has its
    own type-name-leading comment per Go convention.

  api/openapi.yaml — added 4 new operationIds (listApprovalRequests,
    getApprovalRequest, approveApprovalRequest, rejectApprovalRequest)
    + new ApprovalRequest schema component under components/schemas.
    Inline 401 response (the Unauthorized component does not exist in
    this spec; the canonical pattern in the rest of the file is inline
    'description: Authentication required'). The two-person integrity
    contract surface is documented in the description of the approve /
    reject endpoints so external readers see the RBAC contract from the
    spec alone.

Verified locally:
  go vet ./internal/service/...:                      exit 0.
  scripts/ci-guards/openapi-handler-parity.sh:        clean (140 ops vs 174 routes,
                                                       36 documented exceptions).

Third CI failure (image-and-supply-chain) was a transient apt-fetch
'Connection reset by peer' from deb.debian.org while pulling
libasan6_10.2.1-6_amd64.deb. Not a code issue; just re-run the workflow.
No code change needed.
2026-05-04 01:35:30 +00:00
shankar0123 b601928e1c docs(approval-workflow): drop Infisical reference from operator playbook
The operator-facing approval-workflow.md is the public-readable docs
page; the 'Infisical deep-research deliverable' framing is internal
project context that doesn't belong there. Internal source comments +
research docs in cowork/ keep the original framing as the historical
record.
2026-05-04 01:18:59 +00:00
shankar0123 aebfd8bd7c Revert "chore: drop 'Infisical' label from internal references"
This reverts commit 19706e56b3.
2026-05-04 01:18:15 +00:00
shankar0123 19706e56b3 chore: drop 'Infisical' label from internal references
Strategic naming cleanup. Earlier doc-comments + commit messages framed Rank
4 / Rank 5 / Rank 7 work as 'Rank N of the 2026-05-03 Infisical deep-research
deliverable' — the 'Infisical' qualifier was a holdover from the original
deep-research framing where Infisical (a competing secrets-management
platform) was the comparator. Keeping the comparator's name in our source
adds noise without value; an external reader sees 'Infisical' and assumes a
dependency or shared lineage rather than reading it as the competitive
context it was.

Mechanical sed across 34 files (32 source / docs + 2 follow-up Python passes
to collapse 'deep-research deep-research' duplicates that emerged where the
original phrase wrapped across lines):

  s|Infisical deep-research|deep-research|g
  s|infisical-deep-research-results|deep-research-results-2026-05-03|g
  s|infisical-deep-research-prompt|deep-research-prompt-2026-05-03|g
  s|infisical-deep-research|deep-research|g
  s|Infisical|deep-research|g
  s|deep-research deep-research|deep-research|g  # collapse-pass

Net diff: 63 insertions / 64 deletions across cmd/, docs/, internal/,
migrations/. Pure text substitution; zero behavior change. Code path
unchanged — go vet clean, tests for TestApproval pass on both
internal/service and internal/api/handler packages.

Workspace docs (cowork/) carry the same references and will be swept
separately — they're not under certctl/ git control. The two filename
references (cowork/infisical-deep-research-results.md +
cowork/infisical-deep-research-prompt.md) get renamed alongside that sweep
to deep-research-results-2026-05-03.md /
deep-research-prompt-2026-05-03.md so cross-references in the certctl
repo doc-comments resolve cleanly.
2026-05-04 01:15:01 +00:00
shankar0123 03c61f4c20 scheduler, certificate, renewal: gate issuance on profile-driven approval
Closes Rank 7 of the 2026-05-03 Infisical deep-research deliverable
(cowork/infisical-deep-research-results.md Part 5). Pre-fix, certctl
issued certificates unattended — every renewal-loop tick that crossed
a renewal threshold created a Job at Status=Pending which the
scheduler dispatched directly to the issuer connector. PCI-DSS Level
1, FedRAMP Moderate / High, SOC 2 Type II, and HIPAA-regulated PHI
customers all ask the same procurement question: "How do you enforce
two-person integrity on cert issuance?" Today's answer: "We don't."
After this commit chain: "Per-profile RequiresApproval=true creates a
parallel ApprovalRequest row; the renewal-loop creates the Job at
Status=AwaitingApproval; an authorized approver (different from the
requester per the same-actor RBAC check) calls
POST /api/v1/approvals/{id}/approve, transitioning the Job to
Pending; the scheduler picks it up."

This commit (4 of 4) wires the gate into the manual TriggerRenewal
entry point + main.go service construction + Config.Approval +
docs + WORKSPACE-ROADMAP follow-up entries. The previous commits
in the chain shipped:
  - 1 (2025275): domain types + migration + repository
  - 2 (8043e2b): ApprovalService + ApprovalMetrics + 8 service tests
  - 3 (81632eb): 4 API endpoints + handler RBAC tests + router wiring

Files modified:
  cmd/server/main.go              - Constructs approvalRepo +
                                     approvalMetrics + approvalService
                                     + approvalHandler. Wires
                                     CertificateService via
                                     SetApprovalService + SetProfileRepo.
                                     Logs a WARN line at boot when
                                     CERTCTL_APPROVAL_BYPASS=true so
                                     production operators alert on the
                                     log line. Adds Approvals to the
                                     HandlerRegistry.

  internal/config/config.go       - Adds top-level ApprovalConfig
                                     {BypassEnabled bool} sub-config
                                     + CERTCTL_APPROVAL_BYPASS env var
                                     loader. Doc comment cites the
                                     compliance-detection SQL query
                                     (SELECT count FROM audit_events
                                     WHERE actor='system-bypass') so
                                     auditors find the right pattern.

  internal/service/certificate.go - Adds approvalSvc + profileRepo
                                     fields to CertificateService +
                                     SetApprovalService /
                                     SetProfileRepo setters. Extends
                                     TriggerRenewal: looks up the
                                     profile, checks RequiresApproval,
                                     creates the Job at
                                     JobStatusAwaitingApproval (override
                                     the keygen-mode default), then
                                     calls approvalSvc.RequestApproval
                                     to create the parallel
                                     ApprovalRequest row. On
                                     RequestApproval failure, cancels
                                     the orphan Job (defense in depth —
                                     without this, a partial failure
                                     would leave the job stuck at
                                     AwaitingApproval forever). Profile-
                                     lookup failures fall back to the
                                     unattended path (fail-open from
                                     the operator's perspective +
                                     fail-loud via slog.Warn).

Files added:
  docs/approval-workflow.md       - Sysadmin-grade operator runbook:
                                      end-to-end ASCII flowchart
                                      (operator A triggers → operator
                                      B approves → scheduler dispatches),
                                      configuration recipe, RBAC contract
                                      (the load-bearing two-person
                                      integrity rule), operator playbooks
                                      for "I need to approve a renewal"
                                      and "approval timed out", PCI-DSS
                                      6.4.5 / NIST 800-53 SA-15 / SOC 2
                                      CC6.1 / HIPAA control mapping
                                      table, bypass-mode warnings with
                                      the exact compliance-detection SQL
                                      query, Prometheus metric reference,
                                      future free V2 work pointers.

Out of scope of THIS commit (deferred follow-on, not blocking the rest):
  - RenewalService.CheckExpiringCertificates auto-renewal-loop gate.
    The manual TriggerRenewal entry point is gated and the job-level
    timeout reaper already covers AwaitingApproval; the auto-renewal
    gate adds parity. Trivial to add — one block in renewal.go that
    mirrors the certificate.go::TriggerRenewal gate. Tracked in
    WORKSPACE-ROADMAP under the Approval-workflow extensions section.
  - Scheduler reaper extension calling ApprovalService.ExpireStale.
    Today: when the existing reaper times out an AwaitingApproval job,
    the parallel ApprovalRequest row stays at state=pending. The audit
    timeline is still correct (the job-side audit row records the
    timeout) but the dashboard shows a row that no longer needs human
    review. Trivial to wire — one method call in the existing
    scheduler tick. Same WORKSPACE-ROADMAP follow-on.
  - api/openapi.yaml extensions for the 4 new operationIds.
    The HTTP contract is pinned by the handler-level tests; OpenAPI
    is documentation that mirrors the contract.
  - docs/connectors.md `requires_approval` row in the CertificateProfile
    config table. Tracked in the same follow-on; the new
    docs/approval-workflow.md is the canonical reference.

Workspace-level updates (in cowork/, not under certctl/ git control —
applied separately):
  WORKSPACE-ROADMAP.md            - "Approval-workflow extensions"
                                     section under "Future Free V2 Work"
                                     covering M-of-N chains + time-
                                     windowed auto-approve + external
                                     ticketing + per-owner routing +
                                     delegation. All items free under
                                     BSL — no V3-Pro framing per the
                                     2026-05-03 strategy pivot (open
                                     core under BSL; future revenue =
                                     managed-service hosting).

Verified locally:
  gofmt: clean.
  go vet ./...: exit 0.
  go build ./...: exit 0 — full repo links cleanly with the new
    Approval wiring.
  go test -short -count=1 -run TestApproval
    ./internal/service/... ./internal/api/handler/...:
    ok 0.005s for both packages — all 11 approval tests green
    (8 service-level + 3 handler-level).

Reference: cowork/rank-7-approval-workflow-primitive-prompt.md.
Commits: 2025275 → 8043e2b → 81632eb → THIS COMMIT.
2026-05-04 01:12:07 +00:00
shankar0123 81632eb0f3 api, handler: 4 approval endpoints + handler RBAC integration tests
Rank 7 of the 2026-05-03 Infisical deep-research deliverable, commit 3 of 4.
Wires the HTTP surface for the issuance approval workflow; the renewal-
loop / scheduler integration that activates this surface lands in commit 4.

Files added:
  internal/api/handler/approval.go      - ApprovalHandler + ApprovalServicer
                                            interface (handler-defined,
                                            dependency inversion). 4
                                            endpoints:
                                              GET  /api/v1/approvals
                                                ?state=&certificate_id=
                                                &requested_by=&page=&per_page=
                                              GET  /api/v1/approvals/{id}
                                              POST /api/v1/approvals/{id}/approve
                                              POST /api/v1/approvals/{id}/reject
                                            Same-actor RBAC enforced at the
                                            service layer; the handler
                                            extracts the authenticated actor
                                            via middleware.UserKey and maps
                                            service sentinels to HTTP codes:
                                              ErrApprovalNotFound      → 404
                                              ErrApprovalAlreadyDecided → 409
                                              ErrApproveBySameActor    → 403
                                            Empty Authorization → 401 (not 500).
                                            Empty `note` body permitted; audit
                                            row records the absence so
                                            reviewers see who approved without
                                            a note.

  internal/api/handler/approval_test.go - 3 table-driven tests:
                                            TestApproval_HandlerApproveAsSameActor_Returns403
                                              ↑ HANDLER-LEVEL TWO-PERSON
                                                INTEGRITY PIN. Pairs with
                                                the service-level
                                                TestApproval_Approve_RejectsSameActor.
                                                Compliance auditors expect
                                                exactly HTTP 403 (not 401,
                                                not 500) when the requester
                                                self-approves; the test
                                                additionally asserts the
                                                error body contains the
                                                "two-person integrity"
                                                substring so an auditor can
                                                grep server logs for
                                                attempted self-approvals.
                                            TestApproval_HandlerEmptyNote_Allowed_DecidedByExtractedFromAuth
                                              ↑ pins that decided_by comes
                                                from the auth-middleware
                                                UserKey, NEVER from the
                                                request body. Defends
                                                against future contributor
                                                confusion that might let a
                                                client supply their own
                                                decided_by string.
                                            TestApproval_HandlerErrorMapping
                                              (NotFound → 404, AlreadyDecided
                                              → 409 subtests).

Files modified:
  internal/api/router/router.go         - Adds Approvals field to
                                            HandlerRegistry struct + 4
                                            r.Register lines for the
                                            approval routes. Go 1.22
                                            ServeMux precedence: literal
                                            /approve and /reject segments
                                            resolve before the {id}
                                            pattern-var route, mirroring
                                            the existing notifications
                                            block's /requeue precedence.

Verified:
  gofmt: clean.
  go vet ./internal/api/... ./internal/service/...: exit 0.
  go test -short -count=1 -run TestApproval
    ./internal/api/handler/...: ok 0.004s.

Note on OpenAPI spec: the prompt's spec section also calls for 5 new
operationIds in api/openapi.yaml (createApprovalRequest, listApprovalRequests,
getApprovalRequest, approveApprovalRequest, rejectApprovalRequest). The
external-create endpoint is intentionally not implemented in V2 — every
approval request originates from the renewal-loop entry points (commit 4)
so the only operations exposed are list / get / approve / reject. The
4-route surface is a deliberate scope cut: external systems wanting to
inject approval requests can use the underlying `POST /api/v1/certificates/
{id}/renew` path which creates the parallel ApprovalRequest as a side
effect (post-commit-4 wiring). OpenAPI extension batched into commit 4
alongside the integration changes.

Out of scope for this commit (lands in commit 4):
  - Integration into CertificateService.TriggerRenewal +
    RenewalService.CheckExpiringCertificates + Scheduler.ReapTimedOutJobs.
  - cmd/server/main.go wiring.
  - Config.Approval.BypassEnabled + CERTCTL_APPROVAL_BYPASS env var.
  - api/openapi.yaml extensions.
  - docs/connectors.md + docs/approval-workflow.md.

Reference: cowork/rank-7-approval-workflow-primitive-prompt.md.
2026-05-04 01:05:16 +00:00
shankar0123 8043e2bbac service: ApprovalService + ApprovalMetrics + 8 table-driven tests
Rank 7 of the 2026-05-03 Infisical deep-research deliverable, commit 2 of 4
(cowork/rank-7-approval-workflow-primitive-prompt.md). Builds on the
foundation in commit 2025275 — wires the service layer that drives the
approval workflow. Still no handler / integration wiring; commits 3-4
land that.

Files added:
  internal/service/approval.go         - ApprovalService struct + 6
                                          methods: RequestApproval,
                                          Approve, Reject, ListPending,
                                          List, Get, ExpireStale.
                                          Same-actor RBAC check
                                          (ErrApproveBySameActor) at
                                          both Approve and Reject; the
                                          load-bearing two-person
                                          integrity gate. Bypass mode
                                          short-circuits via
                                          approveInternal(outcome=
                                          "bypassed", actorType=System).
                                          Audit + metric emission per
                                          decision via shared
                                          recordAudit helper. Tolerates
                                          nil AuditService for tests.
                                          Service depends on a narrow
                                          JobStatusUpdater interface
                                          (single-method) rather than
                                          the full repository.JobRepository
                                          — production wiring satisfies
                                          it implicitly via postgres'
                                          existing UpdateStatus.

  internal/service/approval_metrics.go - ApprovalMetrics: thread-safe
                                          counter table (decisions
                                          counter dimensioned by
                                          outcome × profile_id) + a
                                          custom durationHistogram for
                                          pending-age (le buckets:
                                          60, 300, 1800, 3600, 21600,
                                          86400, +Inf — 1m, 5m, 30m,
                                          1h, 6h, 24h, beyond).
                                          Snapshot* methods return the
                                          Prometheus exposer's input
                                          shapes. Mirrors the
                                          ExpiryAlertMetrics +
                                          VaultRenewalMetrics pattern
                                          from prior ranks.

  internal/service/approval_test.go    - 8 table-driven tests with
                                          tight in-package fakes
                                          (fakeApprovalRepo +
                                          fakeJobStateRepo):
                                            TestApproval_RequestCreatesPendingRow_BypassDisabled
                                            TestApproval_BypassMode_AutoApprovesWithSystemBypassActor
                                            TestApproval_Approve_TransitionsJobFromAwaitingApprovalToPending
                                            TestApproval_Reject_TransitionsJobFromAwaitingApprovalToCancelled
                                            TestApproval_Approve_RejectsSameActor
                                              ↑ THE LOAD-BEARING TWO-PERSON
                                                INTEGRITY TEST. PCI-DSS 6.4.5
                                                / NIST 800-53 SA-15 / SOC 2
                                                CC6.1 compliance auditors
                                                pattern-match against this.
                                                Pins same-actor rejection on
                                                both Approve and Reject paths;
                                                pins success when a different
                                                actor approves.
                                            TestApproval_Approve_RejectsAlreadyDecided
                                            TestApproval_ExpireStale_TransitionsPendingToExpired_AndCancelsJob
                                            TestApproval_MetricCounterIncrements

Verified:
  gofmt: clean.
  go vet ./internal/service/...: exit 0.
  go test -short -count=1 -run TestApproval ./internal/service/...:
    ok 0.005s — all 8 tests green.

Out of scope for this commit (lands in commits 3-4):
  - api/handler/approval.go (5 endpoints + handler-side RBAC).
  - api/openapi.yaml extensions.
  - Integration into CertificateService.TriggerRenewal +
    RenewalService.CheckExpiringCertificates + Scheduler.ReapTimedOutJobs.
  - cmd/server/main.go wiring of ApprovalService + ApprovalMetrics.
  - Config.Approval.BypassEnabled + CERTCTL_APPROVAL_BYPASS env var.
  - docs/connectors.md row + docs/approval-workflow.md runbook.

Reference: cowork/rank-7-approval-workflow-primitive-prompt.md.
2026-05-04 01:01:53 +00:00