mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 22:11:38 +00:00
ec72c69d20f7ed271f278901cd9e60a2d4495cb5
900 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 shankar0123
|
ec72c69d20 |
fix(compose): wire CERTCTL_BOOTSTRAP_TOKEN interpolation (cold-DB smoke fix #3)
Third latent bug surfaced by the Auditable Codebase Bundle's cold-DB compose smoke. Server cold-boot and migration re-runs are now clean after the prior two fixes ( |
||
|
shankar0123
|
f1aecc6c77 |
fix(migrations): 000043 idempotency — wrap CHECK + UNIQUE adds in DO blocks
Cold-DB compose smoke ran the migration ladder twice (first cold-boot,
then smoke step 4 force-recreate certctl-server with the bootstrap
token env var). On the second run, 000043 fails with:
pq: constraint "actor_roles_scope_type_enum" for relation
"actor_roles" already exists
Server then crashloops trying the same migration every ~10s until the
healthcheck times out and the smoke gives up (5 min wall clock).
Root cause: internal/repository/postgres/db.go::RunMigrations has
no schema_migrations tracker — every *.up.sql runs on every boot.
That makes idempotency mandatory; the CLAUDE.md architecture
decision 'Idempotent migrations. IF NOT EXISTS + ON CONFLICT for
safe repeated execution' is the contract every migration must
honor. Most do; 000043 didn't.
PostgreSQL CHECK constraints don't support IF NOT EXISTS directly,
so each non-idempotent statement gets wrapped in a DO block that
guards against duplication via pg_constraint lookup. The canonical
pattern lives in migrations/000033_approval_kinds.up.sql — mirrored
here exactly. ADD COLUMN already used IF NOT EXISTS; DROP
CONSTRAINT already used IF EXISTS; CREATE INDEX already used IF
NOT EXISTS. Only the two ADD CONSTRAINT CHECK and one ADD
CONSTRAINT UNIQUE needed the DO-block wrap.
Wrapped in BEGIN/COMMIT to match 000033 — keeps all schema
changes inside a single transaction.
Behavior:
- Fresh DB: every DO block runs the ADD CONSTRAINT (no row in
pg_constraint yet). Schema lands identically to the
non-idempotent original.
- Warm DB (constraints already present): every DO block
short-circuits via the NOT EXISTS guard. Migration is a no-op.
Same bug class as 2026-05-09 migration 000045 broken INSERT
(commit
|
||
|
shankar0123
|
4737fff71d |
fix(compose): set CERTCTL_DEMO_MODE_ACK=true in demo compose (cold-DB smoke fix)
The cold-db-compose-smoke job (Auditable Codebase Bundle item 6) fired
on first run and surfaced a real bug: certctl-server fail-fasts at
startup with:
Failed to load configuration: CERTCTL_AUTH_TYPE=none with non-loopback
CERTCTL_SERVER_HOST="0.0.0.0" requires CERTCTL_DEMO_MODE_ACK=true to
acknowledge that every request will be served as the synthetic admin
actor `actor-demo-anon`.
Root cause: the 2026-05-10 HIGH-12 closure (Fix 11) added the
fail-fast guard in internal/config/config.go::Validate() but did NOT
update deploy/docker-compose.yml to provide the explicit ACK. The
clean default compose IS the bundled demo path
(CERTCTL_AUTH_TYPE=none + KEYGEN_MODE=server + DEMO_SEED=true per the
inline comments on lines 137-143), so the ACK is correct here by
design.
Latent in master since the HIGH-12 fix landed. Nobody hit it because
warm containers + warm DBs masked the boot-time validation. The
cold-DB compose smoke caught it on the first true cold-boot run —
exactly the bug class it was built for.
Fix:
- Add CERTCTL_DEMO_MODE_ACK: "true" to the certctl-server env block
in deploy/docker-compose.yml.
- Add a head-comment explaining why the ACK is correct in this
compose (it IS the demo path) and that production deploys override
AUTH_TYPE + KEYGEN_MODE + DEMO_SEED + DEMO_MODE_ACK via their own
compose.
Verified:
- YAML parse clean.
- scripts/ci-guards/complete-path-config-coverage.sh green (194
env vars; new CERTCTL_DEMO_MODE_ACK reference in deploy/ counts
as a consumer).
Audit-Closes: post-v2.1.0-anti-rot/item-6
Audit-Closes: audit-2026-05-10/HIGH-12-followon
|
||
|
shankar0123
|
5332de1e36 |
fix(ciparity): drop unused methodPathRe regex (golangci-lint cleanup)
golangci-lint v2.11.4 surfaced one finding against the bundle's new code: 'var methodPathRe is unused' in internal/ciparity/surface_parity_test.go:46. The regex was leftover scaffolding from when I drafted the file as a package-router test before moving it into the stdlib-only ciparity package. The router-route scanner in this package uses its own inline regex (registerRe + muxHandleRe via scanRouterRoutes) and never reads methodPathRe. Verified clean against the two bundle packages: - golangci-lint run --timeout 5m ./internal/ciparity/... ./internal/config/... → 0 issues - gofmt -l → no output - go vet → clean - go test -short -count=1 → ciparity 0.017s, config 0.727s Audit-Closes: post-v2.1.0-anti-rot/item-2 |
||
|
shankar0123
|
31e4855721 |
ci(cold-db-smoke): inline into workflow; remove the script (operator: not a per-commit gate)
Operator pushback: 'I don't want a smoke test I have to manually run every time I commit.' Correct read — the script existed for local debugging but its presence in scripts/ci-guards/ implied 'operator runs this regularly,' which is the opposite of the design intent. Changes: - Removed scripts/ci-guards/cold-db-compose-smoke.sh. - Inlined the smoke logic directly into the cold-db-compose-smoke job in .github/workflows/ci.yml. Same semantics: docker compose down -v -> up -d -> wait-healthy -> bootstrap admin -> issue/renew/revoke -> assert audit rows -> teardown. 15-min wall-clock cap. Logs dump on failure. - Removed the cold-db-compose-smoke.sh skip case from the generic regression-guards loop (no longer needed). - Updated scripts/ci-guards/README.md and docs/contributor/ci-guards.md to reflect the new shape: 'lives in the workflow, not as a script.' Workspace docs updated (cowork/WORKSPACE-CHANGELOG.md, cowork/CLAUDE.md, cowork/auditable-codebase-bundle/RESULTS.md). The gate is unchanged: CI runs the smoke on every push, master branch-protection enforces it as a required check. Operator's manual action is once — adding the check to branch-protection. Audit-Closes: post-v2.1.0-anti-rot/item-6 |
||
|
shankar0123
|
5948163f21 |
Merge dev/auditable-codebase-bundle into master: Auditable Codebase Bundle (post-v2.1.0 anti-rot items 1+2+5+6)
7 commits across Phases 0-7: |
||
|
shankar0123
|
1ec920e75e |
docs(contributor): document the Auditable Codebase Bundle guards
Three doc changes for the bundle's discoverability: 1. New docs/contributor/ci-guards.md (185 lines) Entry-point doc for new contributors. Explains the four categories of guards (code-shape, contract-parity, build/dep, operational), the discipline that keeps them honest (allowlist + expiration), and how to add a new one. Cross-references scripts/ci-guards/README.md for the exhaustive list. 2. scripts/ci-guards/README.md — added a 'Forward-looking guards' subsection naming complete-path-config-coverage, doc-rot-detector, and cold-db-compose-smoke with their item references + a one-sentence description of what each catches. Replaced the stale '22 guards' header with 'Count: re-derive via ls' per the no-version-stamped-numbers convention from CLAUDE.md. 3. docs/README.md — wired ci-guards.md into the Contributor section navigation table. Bumped 'Last reviewed:' to 2026-05-12 on the two docs touched (docs/README.md, docs/contributor/ci-pipeline.md). Verified: doc-rot-detector.sh green at 91 docs scanned, 89 dated, 0 warns, 0 fails. Audit-Closes: post-v2.1.0-anti-rot/item-1 Audit-Closes: post-v2.1.0-anti-rot/item-2 Audit-Closes: post-v2.1.0-anti-rot/item-5 Audit-Closes: post-v2.1.0-anti-rot/item-6 |
||
|
shankar0123
|
ae9e55e860 |
ci(workflows): wire Auditable Codebase Bundle guards into ci.yml
Three changes to .github/workflows/ci.yml:
1. Add internal/ciparity/... to the Go Test with Coverage package
list. The four surface-parity tests run alongside everything else
and contribute to the coverage report.
2. Skip cold-db-compose-smoke.sh in the existing generic
regression-guards loop (under go-build-and-test). The script needs
Docker + a fresh postgres volume; including it here would always
fail because that job doesn't bring up compose.
The other two new Bundle guards
(complete-path-config-coverage.sh, doc-rot-detector.sh) are
plain-shell + Python and need no Docker — the existing
'for g in scripts/ci-guards/*.sh' loop auto-picks them up.
3. New top-level job: 'cold-db-compose-smoke'
- needs: go-build-and-test (don't waste compute if the basics are red)
- 15-min wall-clock cap (image pull + compose-up + probe + teardown)
- Dumps compose logs on failure for postgres + certctl-server +
certctl-agent + certctl-tls-init so the failure is actionable
without a re-run.
Validated:
- python3 -c 'import yaml; yaml.safe_load(...)' → yaml ok
Operator follow-up:
- Add 'cold-db-compose-smoke' to the master branch-protection
required-checks list once the first successful run lands.
Audit-Closes: post-v2.1.0-anti-rot/item-6
|
||
|
shankar0123
|
bef1feb54f |
feat(ci): item-6 cold-DB compose smoke script (CI wiring in Phase 5)
scripts/ci-guards/cold-db-compose-smoke.sh — wipes the postgres
volume (docker compose down -v), brings the stack up cold, mints a
day-0 admin via /api/v1/auth/bootstrap, issues + renews + revokes a
test certificate, asserts the three audit rows exist, tears down.
Catches the bug class fixed by commit
|
||
|
shankar0123
|
79b02d9245 |
feat(ci): item-5 doc rot detector (90d warn / 120d fail)
scripts/ci-guards/doc-rot-detector.sh — walks every *.md under docs/,
parses the '> Last reviewed: YYYY-MM-DD' blockquote convention
established by the 2026-05-04 docs overhaul, emits:
- ::warning:: GitHub annotation when a doc is >= 90 days old
(heads-up; non-blocking).
- ::error:: + exit 1 when >= 120 days (build-blocking).
Uses HEAD commit timestamp (git log -1 --format=%cs) as 'now' rather
than wall clock — keeps the guard reproducible on a release that's
been on a shelf.
Verified in sandbox:
- Clean run: 90 docs scanned, 88 dated (2 in docs/archive/
allowlisted in bulk), 0 missing field, 0 warns, 0 fails.
- Negative test (backdated docs/README.md to 2025-12-01, 162d):
fires with '::error::Docs older than 120 days (build-blocking)'
+ three remediation paths listed.
Allowlist at scripts/ci-guards/doc-rot-detector-exceptions.yaml:
- 'docs/archive/' bulk-allowlisted (intentionally frozen content)
- Per-doc entries require name + justification + expiration date;
expired entries fail the guard.
Bootstrap sweep NOT required — baseline survey at branch creation
shows oldest doc is 7 days old (2026-05-05); zero docs over either
threshold today. Forward-looking insurance only.
Audit-Closes: post-v2.1.0-anti-rot/item-5
|
||
|
shankar0123
|
370b772fbd |
feat(ci): item-2 cross-surface contract parity (stdlib-only package)
internal/ciparity/ — new stdlib-only package with four tests: 1. TestSurfaceParity_MCPToolCatalogue (HARD GATE): - Every MCP tool name conforms to certctl_<word>(_<word>)* - No duplicate names across the five tools*.go files - Total tools ≥ mcpBaselineFloor (150; current count 155) Catches accidental tool deletions + naming-convention drift. 2. TestSurfaceParity_CLICommandCatalogue (INFORMATIONAL): Walks cmd/cli/main.go's switch-case dispatcher. Logs the 31 distinct verbs. Per frozen decision 0.9, warn-only until the CLI surface stabilizes. 3. TestSurfaceParity_OpenAPI_MCPHeuristicCoverage (INFORMATIONAL): Reports the fraction of OpenAPI ops whose path tokens overlap with MCP tool name tokens. Trend metric; current coverage 92%. 4. TestSurfaceParity_Summary (INFORMATIONAL): One-glance count of router routes / OpenAPI ops / MCP tools / CLI verbs. Easy eyeball for a PR reviewer. Verified in sandbox: - gofmt clean - go vet clean - go test -short -count=1: all four PASS in 0.017s Stdlib-only by design — the tests read source files with os.ReadFile + regexp + go/ast. Keeps the test runnable without pulling in the rest of the codebase's transitive deps; fast self-contained signal. Router ↔ OpenAPI parity (TestRouter_OpenAPIParity) stays in internal/api/router/openapi_parity_test.go where it already lives. This bundle does not duplicate it. Allowlist scaffold at scripts/ci-guards/surface-parity-mcp-exemptions.yaml for the day TestSurfaceParity_OpenAPI_MCP* is promoted from informational to hard gate. Audit-Closes: post-v2.1.0-anti-rot/item-2 |
||
|
shankar0123
|
7a2ba11391 |
feat(ci): item-1 complete-path config-coverage guard (PARTIAL — sandbox could not verify Go test)
Shell guard verified working in sandbox:
- Green on clean repo: 'OK — every CERTCTL_* env var (194) has at least
one non-config-package consumer.'
- Red on injected orphan: '::error::Orphan env vars — defined in
config.go but no consumer found outside internal/config/' with three
remediation paths listed.
Go test internal/config/coverage_test.go written but NOT verified —
sandbox Go 1.25.9 < go.mod's 1.25.10 requirement; toolchain
auto-download fails (disk full). Operator must run `make verify` from
workstation before merge.
Allowlist scaffold at scripts/ci-guards/complete-path-config-coverage-exceptions.yaml.
Every entry requires name + justification + expires fields; expired
entries fail the guard.
Catches the lying-field bug class — env var defined in config.go that no
business-logic code reads. The 2026-04-29 SCEP MustStaple Phase 5.6 gap
(domain field shipped, service layer never read profile.MustStaple) is
the canonical case this guard would have caught at commit time.
Audit-Closes: post-v2.1.0-anti-rot/item-1
|
||
|
shankar0123
|
5ea4f85b4a |
chore(ci): start Auditable Codebase Bundle — record baseline counts
Branch: dev/auditable-codebase-bundle off master @
|
||
|
shankar0123
|
594cf3508a | chore: routine maintenance | ||
|
shankar0123
|
7a18bd7c65 | docs(readme): revert Status block to 4-paragraph form (over-split was too choppy) | ||
|
shankar0123
|
c49eaf9855 | docs(readme): one sentence per blockquote in Status block — full breathing room | ||
|
shankar0123
|
1f4a297dae |
docs(readme): Status block rewrite — design-partner CTA, paragraph cadence
Earlier versions were either link-soup or so tight they read as
boilerplate. This pass aims for CMO-grade copy:
- Paragraph 1: lede that combines the early-access label with the
design-partner ask — sets the tone in one line.
- Paragraph 2: what's production-quality today, with the RBAC + OIDC
doc links inline (no bold, no link-soup). Names the v2.1.0 layer
on top.
- Paragraph 3: the ask — production deployments wanted, framed
explicitly as 'we can't manufacture this exposure in CI'. Honest
about the federated-identity surface being where the new exposure
lives. Mutual-value framing.
- Paragraph 4: the actionable bit — file issues liberally, with the
why ('how the platform earns the right to drop early-access').
Three inline doc links (RBAC, OIDC runbook index, file-issues).
Same factual content, warmer voice, paragraph cadence with
breathing room between.
|
||
|
shankar0123
|
77a1bfc200 | docs(readme): space out the Status block — three separate blockquotes | ||
|
shankar0123
|
638efbe122 |
docs(readme): tighten Status block; add RBAC + OIDC runbook links
Quieter version of the Status block — single blockquote, three short sentences, three inline links (RBAC, OIDC, file-issues). Drops: - The Local-CA / ACME / agent-deployment / CRUD / audit feature pile (those live in the doc table immediately below) - The 6-IdP enumeration (Keycloak / Authentik / Okta / Auth0 / Entra ID / Google Workspace) — operators find that in the OIDC runbook index, now linked inline - The double 'in early-access' phrasing - 'HMAC-signed server-side sessions with __Host- cookies and CSRF rotation; OIDC Back-Channel Logout; Argon2id break-glass admin' — the spec details belong in the auth-threat-model + security docs, not the front-page status Same early-access framing, same issue-link CTA, far more readable. |
||
|
shankar0123
|
c2611c462b |
docs(readme): audit pass — fix 7 stale/inaccurate claims
Each claim ground-truthed against the live repo, not memory. Numeric drift (claims rotted since they were written): - Screenshot caption 'Catalog with 10 CA types' → 12 (matches internal/connector/issuerfactory/factory.go enumeration). - '33-permission canonical catalogue' → dropped the number. 33 was the base in migration 000029; across all 45 migrations 82 unique perms are seeded (+5 admin / +7 OIDC / +2 break-glass / +33 audit-CRIT-1 / +2 user). 'Fine-grained permission catalogue' is monotonic prose. - 'PostgreSQL 16 backend (35+ tables, idempotent migrations)' → '…backend with idempotent migrations'. Actual table count is 49 across 45 migrations; bare 'idempotent migrations' is drift-proof. - Demo overlay seeds '32 certificates across 10 issuers, 8 agents, 180 days' → '180 days of realistic history across 13 issuers, 8 agents, managed + discovered certs, jobs, deploys, audit, and notification events'. seed_demo.sql actually seeds 14 managed certs + 16 cert versions + 12 discovered, 13 issuers (not 10), 8 agents ✓, 23 INTERVAL '180 days' refs ✓. - 'golangci-lint (11 linters)' → '(govet + staticcheck + contextcheck + unused)'. .golangci.yml lists exactly 4 active linters; 6 others are commented-out 'temporarily disabled' so neither 4 nor 10 explains 11. Broken Helm one-liner (silently no-ops because --set against a nonexistent path doesn't error): - '--set server.apiKey=…' → 'server.auth.apiKey' (deploy/helm/certctl/values.yaml:147 + templates/server- secret.yaml:16). - '--set postgres.password=…' → 'postgresql.password' (top-level key is 'postgresql', not 'postgres'; password sits at postgresql.password per values.yaml:315). Verified accurate (no change): - 12 issuers / 15 targets / 6 notifiers (factory + dir listings). - 7 default roles seeded in migration 000029. - Coverage thresholds (service 70 / handler 75 / crypto 88 / auth packages 85-95) against .github/coverage-thresholds.yml. - All 6 OIDC runbooks present (auth0 / authentik / azure-ad / google-workspace / keycloak / okta). - 4 referenced screenshots all exist on disk. - 8 agents in demo seed, 180 days of history. - RFC 9700 §4.7.1 / 9207 / 8555 / 9773 / 8894 / 9266 / 5280 / 6960 citations match source. - ChromeOS in SCEP description matches source. - install-agent.sh uses uname for OS / arch detection + systemd (Linux) / launchd (macOS). |
||
|
shankar0123
|
5102dec306 | docs(readme): split Status block into two blockquotes for breathing room | ||
|
shankar0123
|
261203fea9 |
docs(readme): quiet the Status block
The previous version crammed 5 bold-emphasized inline links plus inline code into a single paragraph — visually loud and hard to scan. Rewrite as two short paragraphs: - First paragraph: what's production-quality + what's still maturing. No links, em-dash cadence for breathing room. - Second paragraph: v2.1.0 OIDC + sessions + break-glass slice with a single issue-link tail. Drops the bold-link sandwich in favor of plain prose; the doc-nav table directly below handles per-doc routing. Same content, same early-access framing, far less visual noise. |
||
|
shankar0123
|
30034085e6 |
docs: v2.1.0 release polish — strip internal bundle/phase tags, update status for OIDC ship
README:
- Rewrite Status block: drop the stale 'federated identity not yet
shipped' line; flag v2.1.0 OIDC + sessions + back-channel logout
+ break-glass as early-access; encourage GitHub issues for IdP
rough edges. (A1 framing — keep early-access umbrella, no
SAML/WebAuthn/JIT roadmap teaser.)
- Add OIDC SSO bullet to 'What it does' covering per-IdP runbooks,
group-claim → role mapping, AES-256-GCM client_secret encryption,
JWKS auto-refresh, PKCE-S256, RFC 9700 §4.7.1 pre-login binding,
RFC 9207 iss check, __Host- cookies, CSRF rotation, idle+absolute
expiry, BCL, break-glass admin.
- Update Security paragraph: three auth paths (API keys / OIDC /
break-glass), HMAC-signed sessions, CSRF rotation, RFC OIDC BCL.
- Correct CI coverage thresholds against
.github/coverage-thresholds.yml (service 70%, handler 75%,
crypto 88%, auth packages 85-95%); 'static analysis' replaces
the inflated '11 linters' claim (actual count is 4 active).
Docs B3 sweep — strip operator-facing 'Bundle N' / 'Phase N' tags:
- docs/operator/auth-threat-model.md — rewrite intro; rename 5 H2
sections (API-key + RBAC defenses / OIDC + sessions + break-glass
defenses / OIDC + sessions threat catalogue / Closed federated-
identity threats / Future-work threats); clean ~12 H3/prose hits.
- docs/operator/rbac.md — strip Bundle 1 framing from intro,
scope_id deferral note, MCP tools section, day-0 bootstrap, and
'Where to look next'.
- docs/operator/auth-benchmarks.md — drop 'Phase 14' framing from
title intro, hardware floor caption, result table caption,
methodology, and pre-merge audit section.
- docs/operator/security.md — already cleaned earlier this session
(RBAC / day-0 / approval-bypass / OIDC federation / sessions /
OIDC first-admin / break-glass H3s).
- docs/operator/oidc-runbooks/{index,keycloak,authentik,okta,
azure-ad}.md — strip Auth Bundle 2 framing + Phase 10/3/4
references; replace with feature-name prose.
- docs/operator/legacy-clients-tls-1.2.md — drop Bundle F / M-023
audit-reference framing; keep CWE-326.
- docs/operator/database-tls.md — drop Bundle B / M-018 framing
from intro + Helm section.
- docs/operator/runbooks/disaster-recovery.md — drop 'Production
hardening II Phase 10' status callout.
- docs/migration/oidc-enable.md — retitle 'Enable OIDC SSO';
strip Bundle 1/2 framing from prereqs, troubleshooting, related
docs; update __Host- cookie callout from 'audit MED-14' to
v2.1.0-BREAKING.
- docs/migration/api-keys-to-rbac.md — strip Bundle 1 framing from
intro, migration table, IsAdmin section, and cross-references.
- docs/migration/acme-from-cert-manager.md — strip residual
'Phase 5' tags from cert-manager integration test references.
- docs/reference/configuration.md — retitle Auth section.
- docs/reference/profiles.md — strip Bundle 1 Phase 9 framing
from RequiresApproval section + Related list.
- docs/reference/auth-standards-implemented.md — rewrite intro
(API-key + RBAC + OIDC + sessions + back-channel logout +
break-glass); rename 'Bundle 1 (RBAC) standards covered
separately' H2; clean per-row Phase references.
- docs/README.md — rewrite nav-table entries to drop Bundle 1/2
parentheticals; retitle 'Enable OIDC SSO' migration entry.
No code or test changes; pure operator-facing prose polish for
the v2.1.0 tag.
|
||
|
shankar0123
|
e772ff0d4d |
fix(repo/job): split UNION ALL + FOR UPDATE into two queries (Postgres-correctness)
Phase-9 docker compose smoke surfaced a latent production-breaking bug introduced by commit |
||
|
shankar0123
|
6444e13b8f |
fix(migrations): two cold-DB regressions surfaced by Phase-9 docker compose smoke
The v2.1.0 release-gate Phase-9 docker compose smoke run against a
fresh Postgres surfaced two real defects in the migration files that
testcontainers schema-per-test never exercised. Both reproduce by
running 'docker compose down -v && docker compose up --build'
against the current master tree.
Bug A — migration 000045_users_deactivated_at.up.sql is malformed.
The 000029 schema defines:
permissions (id TEXT PRIMARY KEY, name TEXT NOT NULL UNIQUE,
namespace TEXT NOT NULL)
role_permissions (..., permission_id TEXT NOT NULL REFERENCES ..., ...)
But 000045 was written as:
INSERT INTO permissions (name) VALUES ... -- missing id + namespace
INSERT INTO role_permissions (role_id, permission, ...) VALUES ...
^^ wrong column name
On a cold-DB run this fails immediately with:
pq: null value in column "id" of relation "permissions"
violates not-null constraint
Fix: provide id + namespace columns, use permission_id (the actual
column name), ON CONFLICT (id) DO NOTHING. The new permission ids
follow the existing 'p-auth-*' prefix convention (p-auth-user-read +
p-auth-user-deactivate) used by 000029.
Bug B — migration 000029_rbac.up.sql is not idempotent post-000043.
000029 originally created actor_roles with:
UNIQUE (actor_id, actor_type, role_id, tenant_id)
Audit 2026-05-10 HIGH-10 closure / migration 000043 drops that
constraint and re-creates it WITH scope columns:
UNIQUE (actor_id, actor_type, role_id, scope_type, scope_id, tenant_id)
The migration runner (internal/repository/postgres/db.go::RunMigrations)
is naive — no tracker table — and re-runs every *.up.sql file on
every server boot. On the second-and-later boots, 000029's seed
INSERT for actor-demo-anon-admin still references the
pre-000043 constraint name in its ON CONFLICT clause:
ON CONFLICT (actor_id, actor_type, role_id, tenant_id) DO NOTHING
Postgres errors out with:
pq: there is no unique or exclusion constraint matching the
ON CONFLICT specification
Fix: pin the conflict target to the row's primary key 'id' column
(always present, never altered). The seed row's deterministic id
'ar-demo-anon-admin' makes ON CONFLICT (id) work under both pre-
and post-000043 schemas.
Why testcontainers schema-per-test missed these:
Each test in internal/repository/postgres/*_test.go spins up a
fresh schema and applies every .up.sql in order ONCE. The full
'000029 -> 000043 -> retry 000029' cascade never happens because
migrations don't re-run within a test. Phase-9 docker compose
smoke is the only test path that exercises the server-restart-
on-error retry, which is exactly the missing coverage.
Verify (sandbox): go test ./internal/repository/postgres/ PASS.
Workstation re-runs 'docker compose down -v && docker compose up'
to confirm both bugs are closed.
|
||
|
shankar0123
|
752e036dac |
fix(oidc/testfixtures): set legacy KEYCLOAK_ADMIN* env vars for start-dev master-admin bootstrap
Phase-10 live-IdP smoke (post-iss-param fix landing in
|
||
|
shankar0123
|
8f8339393c |
fix(oidc/integration): pass fx.IssuerURL as callbackIss arg in 7 HandleCallback call sites
Phase-10 live-IdP smoke (post-Enabled-true fix landing in
|
||
|
shankar0123
|
2d29175b52 |
fix(oidc/testfixtures): set Enabled=true on Keycloak integration-test provider
Phase-10 live-IdP smoke re-run (after the alg-downgrade relax landed in
|
||
|
shankar0123
|
92c50d9e19 |
harden(oidc): relax alg-downgrade IdP-bind check to intersection-empty (Keycloak compat)
Phase-10 live-IdP smoke (Keycloak 26.x via testcontainers-go) revealed
the IdP-bind alg-downgrade check was too strict for real-world IdPs.
6 of the integration tests in internal/auth/oidc/integration_keycloak*_test.go
were failing with:
oidc: IdP advertises weak signing algorithms (HS*/none);
refusing to use as defense against downgrade attacks: HS256
Keycloak 26.x (and several other real-world IdPs — Auth0 when HS-mode is
enabled, some Authentik configs) advertise EVERY alg they're capable of
in the discovery doc's id_token_signing_alg_values_supported field, even
when the realm only signs with RS256 in practice. Pre-fix the IdP-bind
check refused on ANY HS* or 'none' advertisement → no real Keycloak deploy
could ever bind a provider row, hence the integration-test failures.
The strict-deny check was defense-in-depth on top of the load-bearing
per-token alg-pin at sig-verify time (isDisallowedAlg, service.go L1177):
that check rejects every ID token whose JWS header carries an alg outside
DefaultAllowedAlgs, regardless of what the discovery doc advertises.
A forged HS256 token signed with the IdP's RS256 pubkey as HMAC secret
is rejected at sig-verify time → the actual algorithm-confusion attack
is closed by the per-token pin, NOT by the discovery-doc check.
Fix: relax the IdP-bind check to refuse only when the intersection of
advertised vs DefaultAllowedAlgs is EMPTY (the pathological all-weak-alg
IdP case). Keycloak (RS256 + HS256 advertised) now binds successfully;
an HS-only IdP still fails closed.
Changes:
- internal/auth/oidc/service.go: rewrite the alg-check loop at L1067 in
getOrLoad / RefreshKeys to compute the intersection set; refuse only
when no acceptable alg is advertised. ErrIdPDowngradeAdvertised
docstring updated to reflect new contract. DefaultAllowedAlgs
docstring + the package-level design-comment block at L40-72 updated
with v2.1.0-relaxed semantics callouts.
- internal/auth/oidc/test_discovery.go: TestDiscovery dry-run validator
rewritten to surface HS*/none alongside RS* as an informational note
('note: IdP advertises weak algorithms %v alongside acceptable ones')
rather than a hard-fail error. HS-only / none-only still hard-fails.
- internal/auth/oidc/service_test.go: TestService_IdPDowngradeDefense_*
tests updated. Renamed:
- RejectsHSAdvertised → RS256PlusHS256_BindsSuccessfully (positive)
- RejectsNoneAdvertised → RejectsHSOnlyAdvertised (intersection-empty)
- RefreshKeys_CatchesPostLoadDowngrade rotated to HS-only post-load
- internal/auth/oidc/coverage_fill_test.go: TestTestDiscovery_AlgDowngradeDetected
split into _HS256AlongsideRS256_BindsWithNote (positive, asserts note
but no hard-fail) + _HSOnly_StillTrips_HardFail (intersection-empty).
- docs/operator/auth-threat-model.md: OIDC token-validation alg-allow-list
section rewritten to call out the load-bearing-defense hierarchy
(per-token pin first, IdP-bind check defense-in-depth) and document
the v2.1.0 relaxation rationale.
- CHANGELOG.md: ### Security entry under Unreleased.
Verify: go test ./internal/auth/oidc/ -short PASS; gofmt clean; go vet
clean. The Keycloak integration tests should now pass when the operator
re-runs 'make keycloak-integration-test'.
|
||
|
shankar0123
|
e7e60df8f5 | Merge dev/auth-bundle-2 → master (v2.1.0): Auth Bundle 2 + 2026-05-11 audit fixes | ||
|
shankar0123
|
125e59fb79 |
test(client): mock headers.get() so 401 tests survive HIGH-8 WWW-Authenticate read
Audit 2026-05-10 HIGH-8 closure landed a parseWWWAuthenticateCause()
call in api/client.ts (line 144) that reads res.headers.get(...) on the
401 path. The two test files in web/src/api/ both provide a Response
mock with no headers property, so every 401 test threw 'Cannot read
properties of undefined (reading get)' instead of the expected
'Authentication required'.
13 tests fail without this fix: 12 in client.error.test.ts (one per
401-mapped endpoint helper) + 1 in client.test.ts (the auth-required
event-dispatch test).
Fix: add headers: { get: () => null } to both mockErrorResponse helpers.
The null return short-circuits parseWWWAuthenticateCause to the default
'Authentication required' message, so every existing 401 assertion
keeps passing.
|
||
|
shankar0123
|
e347a9a908 |
chore(ci-guards): close 4 CI-guard regressions surfaced by v2.1.0 release-gate Phase 5
Four scripts/ci-guards/*.sh trips on dev/auth-bundle-2 vs master:
1. G-3-env-docs-drift: 10 CERTCTL_* env vars added by Auth Bundle 2 +
audit-2026-05-10/11 fix bundle were not in docs/. Added a new 'Auth
(Bundle 1 + Bundle 2)' section to docs/reference/configuration.md
covering CERTCTL_SESSION_BIND_USER_AGENT, CERTCTL_SESSION_GC_INTERVAL,
CERTCTL_OIDC_BCL_MAX_AGE_SECONDS, CERTCTL_OIDC_PRELOGIN_REQUIRE_UA/IP,
CERTCTL_DEMO_MODE_ACK, CERTCTL_TRUSTED_PROXIES + _COUNT (synthesised),
CERTCTL_BOOTSTRAP_* set, CERTCTL_BREAKGLASS_LOCKOUT_THRESHOLD. Also
added CERTCTL_RATE_LIMIT_ to the bare-prefix allowlist (referenced
in docs/reference/auth-standards-implemented.md prose).
2. bundle-8-M-009-bare-usemutation: BreakglassPage shipped 3 bare
useMutation() calls instead of useTrackedMutation. Migrated all
three to useTrackedMutation with invalidates: [['breakglass']].
3. multi-tenant-query-coverage: Defense-in-depth tenant_id additions
in the fix bundle dropped the missing-tenant-id query count from 32
to 31. Ratcheted baseline 32 -> 31 (forward-only invariant).
4. openapi-handler-parity: 28 new REST endpoints from Bundle 2 + the
fix bundle missing from api/openapi.yaml. Added them to
api/openapi-handler-exceptions.yaml with per-route 'why:'
justifications. OpenAPI schema generation deferred to pre-v2.2.0
alongside the GUI E2E coverage push; threat model + handler
contracts already live in docs/operator/{rbac,auth-threat-model,
oidc-runbooks}.md.
After this commit every script in scripts/ci-guards/*.sh exits 0.
|
||
|
shankar0123
|
7227844f29 |
test(coverage): backfill 5 packages to clear v2.1.0 release-gate Phase 3 floors
Phase 3 of /Users/shankar/Desktop/cowork/v2.1.0-release-gate.md surfaced
four packages below their coverage floors. All four are regressions from
new code shipped in the audit-2026-05-10/11 fix bundles that didn't get
per-function tests:
internal/auth/breakglass 87.5% -> 93.3% (floor: 90%)
+ List (was 0%) — 3 tests (disabled, empty+populated, repo err)
+ RemoveCredential, Unlock disabled-branch tests
internal/auth/oidc 89.4% -> 95.4% (floor: 90%)
+ JWKSStatus (was 0%) — 2 tests (unknown provider, after AuthRequest)
+ TestDiscovery (was 0%) — 5 tests (discovery failure, happy path,
HS256 alg-downgrade detected, missing jwks_uri, JWKS 500 fetch)
internal/auth/session 89.9% -> 94.4% (floor: 90%)
+ SetTrustedProxies (was 0%) — round-trip + clear
+ ComputeCookieHMAC (was 0%) — determinism + key/inputs differ
+ DecryptKeyMaterial (was 0%) — round-trip + wrong-passphrase
internal/api/handler 73.2% -> 75.5% (floor: 75%)
+ 6 auth_breakglass handler funcs (were all 0%) — 14 tests
(disabled/404, invalid JSON, empty fields, service err, happy
path with cookies, admin endpoints, ListCredentials no
password_hash on the wire)
+ WithPermissionChecker setter test (was 0%, Bundle 2 MED-2)
+ NewAdminCRLCacheServiceImpl + CacheRows (were 0%) — 3 tests
+ itoaForRetryAfter + challengeURLBuilder ACME helpers (were 0%) —
4 tests
All five coverage gates green:
internal/service 72.7% (floor: 70%)
internal/api/handler 75.5% (floor: 75%)
internal/api/middleware 67.9% (floor: 30%)
internal/auth 93.3% (floor: 85%)
internal/service/auth 91.8% (floor: 85%)
internal/auth/oidc 95.4% (floor: 90%)
internal/auth/oidc/groupclaim 100.0% (floor: 95%)
internal/auth/oidc/domain 97.6% (floor: 90%)
internal/auth/session 94.4% (floor: 90%)
internal/auth/session/domain 98.3% (floor: 90%)
internal/auth/breakglass 93.3% (floor: 90%)
internal/auth/breakglass/domain 100.0% (floor: 90%)
internal/auth/user/domain 96.2% (floor: 90%)
(and 6 more — all green)
Per CLAUDE.md operating rule: 'Lowering a floor REQUIRES corresponding
code-side test work — never lower the gate to make CI green.' The
floors stay at their committed values; the new tests close the gap.
|
||
|
shankar0123
|
4d859468ab |
chore(lint): close 5 golangci-lint v2 findings surfaced by v2.1.0 release-gate Phase 1.3
Five golangci-lint v2 findings surfaced when running the v2.1.0 release gate (auth-bundle-2 → master pre-flight). Each is mechanical: 1. govet/printf-style misuse — internal/auth/oidc/service_test.go used integer literal 501 in http.Error; switched to http.StatusNotImplemented. 2. staticcheck SA1019 — internal/auth/breakglass/reflect_helper_test.go referenced reflect.Ptr; the canonical name since Go 1.18 is reflect.Pointer. 3. staticcheck ST1020 — internal/repository/postgres/auth.go ActorRoleRepository.Revoke had a doc comment that did not begin with the method name. Prepended 'Revoke drops actor_roles rows.' to the comment so it now starts with the method name. 4. staticcheck ST1022 — internal/api/handler/auth_session_oidc.go DefaultBCLVerifierMaxAge docstring was attached to the DefaultBCLVerifier type docstring. Moved the const docstring directly above the const declaration, separated by a blank line. 5. unused — internal/auth/session/bench_test.go declared benchSessionMinSamples and never referenced it; the bench loop relies on Go's default b.N scaling. Replaced the const block with a comment describing the rationale. Lint clean (golangci-lint v2.12.2 with the .golangci.yml config) on the five edited packages. |
||
|
shankar0123
|
b8c1bf3617 |
chore(fmt): gofmt cleanup on three pre-bundle drift files surfaced by v2.1.0 release-gate Phase 1
Phase 1 (make verify) of cowork/v2.1.0-release-gate.md surfaced three
files with pre-existing gofmt drift that pre-dated the 2026-05-11 fix
bundle work:
internal/auth/oidc/domain/types.go
internal/auth/oidc/integration_keycloak_rotate_test.go
internal/auth/oidc/test_discovery.go
The 2026-05-11 Fix 08 fmt-cleanup commit (
|
||
|
shankar0123
|
4f7cf63ae5 |
Merge Fix 13 (HIGH-2 fourth call site): CSRF rotation on Logout
# Conflicts: # CHANGELOG.md |
||
|
shankar0123
|
46529bbf5c | Merge Fix 12: Vitest coverage for the 2026-05-10/11 GUI batch | ||
|
shankar0123
|
af32695f1f |
Merge Fix 11 (MED-11 discoverability): UsersPage sidebar nav entry
# Conflicts: # CHANGELOG.md |
||
|
shankar0123
|
4bcd650b98 |
Merge Fix 10 (MED-7 GUI half): JWKS health panel + Refresh-now button
# Conflicts: # CHANGELOG.md # web/src/pages/auth/OIDCProviderDetailPage.tsx |
||
|
shankar0123
|
8ba69e8cb7 |
Merge Fix 09 (MED-5 GUI half): Test Connection panel on OIDC create + edit forms
# Conflicts: # CHANGELOG.md |
||
|
shankar0123
|
e8233cf6e3 | Merge Fix 08 (HIGH A-8): demo-mode residual-grants detector + cleanup endpoint + CI guard | ||
|
shankar0123
|
f50c68e199 |
harden(auth/sessions): CSRF rotation on logout closes HIGH-2 fourth call site
Audit 2026-05-11 Fix 13 closure. The HIGH-2 closure on dev/auth-bundle-2 documented four RotateCSRFTokenForActor call sites — login completion (fresh by construction), Assign/Revoke RoleToKey (wired at internal/api/handler/auth.go:498 + 546), Logout, and an explicit operator endpoint. The 2026-05-11 adversarial review observed only 3 of the 4: Logout did NOT rotate the actor's sibling sessions post-revoke. Threat closed: a token captured pre-logout (browser DevTools, malicious extension, session-storage leak) could be replayed against the user's other-device/other-browser sessions until those sessions hit their own idle/absolute expiry. Rotation on logout defeats this — the captured token is dead the moment the user clicks 'Sign out' anywhere. What this changes: * internal/api/handler/auth_session_oidc.go::SessionMinter interface gains RotateCSRFTokenForActor(ctx, actorID, actorType string) int. Nil-safe semantics by convention — the production wiring is *session.Service which already implements the method; rotation NEVER errors (returns int count, swallows per-row failures via the underlying Service.RotateCSRFToken) so it can't block the surrounding Revoke that triggered it. * internal/api/handler/auth_session_oidc.go::Logout calls RotateCSRFTokenForActor after Revoke(sess.ID) succeeds. The auth.session_revoked audit row gains a csrf_rotated detail key carrying the count so SOC/SIEM can correlate logout events with CSRF churn on sibling sessions. * The no-cookie + invalid-cookie 204 short-circuit paths skip rotation. No session row exists to rotate against; the caller is already unauthenticated. Rotation on those paths would do nothing useful and pollute the audit log. Test coverage in internal/api/handler/auth_session_oidc_test.go: * TestLogout_RotatesCSRFForActor — happy path. Mocks rotateCSRFReturnCount=2; asserts Revoke fires before rotation, rotation fires exactly once with caller's (actor_id, actor_type), audit details carry csrf_rotated=2. * TestLogout_NoCookie_SkipsCSRFRotation — pins the 204 short-circuit branch when there's no cookie. Rotation count stays at 0. * TestLogout_InvalidCookie_SkipsCSRFRotation — pins the 204 short-circuit branch when Validate rejects the cookie. Same rationale: no session row, no rotation. The stubSession test fake gains RotateCSRFTokenForActor with call-recording fields; the phase5StubAudit gains a details slice append-aligned 1:1 with events so the happy-path test can index into the latest entry and assert the count. Spec Phase 3 (explicit operator endpoint) — intentionally NOT shipped. The three automatic triggers (login + role- mutation + logout) cover the HIGH-2 threat model; operators who want a nuclear option can use the existing RevokeAllForActor flow which forces re-login → fresh session → fresh CSRF. Adding a dedicated POST /api/v1/auth/sessions/ rotate-csrf admin endpoint would be defense-in-depth without new attack-surface coverage. Documented in the audit-doc annotation. Verify gate: * gofmt -l — clean * go vet ./internal/api/handler/... — clean * go build ./cmd/server/... ./internal/... — clean (production *session.Service satisfies the extended interface out of the box) * go test -short -count=1 ./internal/api/handler/... ./internal/auth/session/... — all green; 3 new Logout cases + the 2 pre-existing Logout cases all pass. Audit doc annotation at cowork/auth-bundles-audit-2026-05-10.md flips the HIGH-2 row from 'CLOSED 2026-05-10 (3/4 call sites wired)' to 'A-B-3 verified 2026-05-11: HIGH-2 fully closed across all four documented call sites.' Refs cowork/auth-bundles-fixes-2026-05-11/13-verify-logout-csrf-rotation.md. |
||
|
shankar0123
|
8564e2fcd6 |
test(gui): Vitest coverage for the 2026-05-10/11 GUI batch (Fix 12)
Audit 2026-05-11 Fix 12 closure. The original GUI-batch commit
|
||
|
shankar0123
|
51fdc8cf62 |
feat(gui/nav): UsersPage sidebar nav entry under Auth section (MED-11)
Audit 2026-05-11 Fix 11 closure. The MED-11 closure shipped
web/src/pages/auth/UsersPage.tsx and wired the /auth/users route
in web/src/main.tsx, but the sidebar nav never gained a
corresponding entry. Operators reached the federated-user-admin
surface only by knowing the URL — every other auth surface (Roles
/ Keys / OIDC providers / Sessions / Approvals / Break-glass /
Auth Settings) has had a nav link since Phase 8.
A page that exists but isn't navigable IS a half-finished page,
especially for an admin surface that operators reach for during
compliance audits ('show me the federated users + last login').
30 minutes closes the inconsistency.
What this changes:
* web/src/components/Layout.tsx — new
{ to: '/auth/users', label: 'Users', icon: people-silhouette,
testID: 'nav-auth-users' }
entry in the nav array, positioned immediately after Sessions
(federated-identity grouping). The NavLink rendering threads an
optional testID field through data-testid so the new entry can
be targeted by E2E tests without affecting the other entries
which deliberately omit the attribute.
* Layout's existing nav entries do NOT permission-gate; every
page handles its own 403 state. UsersPage already returns an
ErrorState directing the user to auth.user.read for callers
without the perm. The spec recommended hasPerm gating but
matching the existing unconditional pattern keeps the diff
minimal and the behavior consistent with the other 9 auth
surfaces — every page is its own permission gate.
Tests added in web/src/components/Layout.test.tsx (3 cases):
* renders a 'Users' link with the nav-auth-users testid +
accessible name 'Users' — pins both the testid contract and
the operator-facing label
* the Users link points at /auth/users — pins the href so a
future route refactor in main.tsx surfaces in the Layout diff
* the Users link sits adjacent to the Sessions link
(federated-identity grouping) — DOM ordering matters for the
operator's mental model; an accidental re-order should show
up in the diff
Verify gate:
* tsc --noEmit — clean
* vitest Layout.test.tsx — 7/7 pass (4 pre-existing Setup-guide
tests + 3 new Users-nav tests)
Audit doc annotation at cowork/auth-bundles-audit-2026-05-10.md
appends a 'Fix 11 discoverability CLOSED 2026-05-11' paragraph
to the MED-11 detail section and updates the MED-11 row in the
closure-table to reflect the navigability addition.
Refs cowork/auth-bundles-fixes-2026-05-11/11-med-users-sidebar-nav.md.
|
||
|
shankar0123
|
55661db777 |
feat(gui/oidc): JWKS health panel + Refresh-now button on OIDCProviderDetailPage (MED-7 GUI half)
Audit 2026-05-11 Fix 10 closure. MED-7's backend endpoint
GET /api/v1/auth/oidc/providers/{id}/jwks-status (commit
|
||
|
shankar0123
|
17b400a684 |
feat(gui/oidc): Test Connection panel on create + edit forms (MED-5 GUI half)
Audit 2026-05-11 Fix 09 closure. MED-5's backend dry-run endpoint
(POST /api/v1/auth/oidc/test, gated auth.oidc.create) shipped on
dev/auth-bundle-2 (commit
|
||
|
shankar0123
|
ff3f1cd864 |
harden(auth): demo-mode residual-grants detector + cleanup endpoint + CI guard (A-8)
Audit 2026-05-11 A-8 closure. Closes the deferred Phase 2 leg of the
2026-05-10 HIGH-12 closure (
|
||
|
shankar0123
|
b3e3a8dbb1 |
chore(fmt): gofmt cleanup on files touched by audit-2026-05-11 fix bundle
Whitespace alignment drift surfaced by gofmt -l after merging 7 fix branches.
Pure formatting, no semantic change. Pre-existing master drift in
internal/auth/oidc/{domain/types.go, integration_keycloak_rotate_test.go,
test_discovery.go} left untouched — that's separate tech debt.
|
||
|
shankar0123
|
f6e114b8bd |
Merge Fix 07 (HIGH A-7): editable Advanced form on OIDCProviderDetailPage (MED-4)
# Conflicts: # CHANGELOG.md # web/src/pages/auth/OIDCProviderDetailPage.test.tsx # web/src/pages/auth/OIDCProviderDetailPage.tsx |
||
|
shankar0123
|
e6ae81f478 |
Merge Fix 06 (HIGH A-6): strict UA/IP binding — close request-empty bypass in MED-16
# Conflicts: # CHANGELOG.md # internal/api/handler/auth_session_oidc.go # internal/api/handler/auth_session_oidc_test.go |