356 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
shankar0123	aa6fafdee9	Merge branch 'fix/u2-dockerfile-healthcheck-https' v2.0.50	2026-04-25 12:02:28 +00:00
shankar0123	86fffa305a	fix(deploy,helm,docs): published-image HEALTHCHECK speaks HTTPS + Helm /ready path + docs HTTPS sweep (U-2) ... Pre-U-2 the published `ghcr.io/shankar0123/certctl-server` image shipped with `HEALTHCHECK CMD curl -f http://localhost:8443/health`. The server has been HTTPS-only since the v2.2 HTTPS-Everywhere milestone (`cmd/server/main.go::ListenAndServeTLS`, no plaintext fallback, TLS 1.3 pinned), so the probe failed on every interval and Docker marked the container `unhealthy` indefinitely. Operators inside docker- compose / Helm / the example stacks were unaffected — compose overrides the HEALTHCHECK with `--cacert + https://`, Helm uses explicit `httpGet` probes that ignore Docker's HEALTHCHECK, and every example compose file overrides with `curl -sfk https://localhost:8443/health`. But anyone running bare `docker run` / Docker Swarm / Nomad / ECS — exactly the "I just pulled the published image" path — saw permanent `unhealthy` status and (depending on orchestrator policy) a restart- loop. (Audit: cat-u-healthcheck_protocol_mismatch in coverage-gap-audit-2026-04-24-v5/unified-audit.md.) Recon for U-2 surfaced two adjacent bugs from the same v2.2 milestone gap, both bundled into this commit because they share the same root cause and the same operator surface: 1. Helm chart `server.readinessProbe.httpGet.path` pointed at `/readyz`, the kube-flavored convention. The certctl server doesn't register `/readyz` (only `/health` and `/ready` are wired and bypass the auth middleware — see internal/api/router/router.go:81 and cmd/server/main.go:920). K8s readiness probes therefore got 401 (api-key auth rejection) or 404 (when auth was disabled), pods stayed `NotReady` indefinitely, and Helm rollouts stalled. 2. The agent image (`Dockerfile.agent`) had no HEALTHCHECK at all, so bare-`docker run` agents got zero health signal. The compose override at `deploy/docker-compose.yml:173` called `pgrep -f certctl-agent` against the agent image, but the agent image didn't ship `procps` — pgrep was missing too. The compose probe was a latent always-fail. We fixed all three with the audit-recommended shape (option (a) — `-k`) plus three structural backstops: Files changed: Phase 1 — Dockerfile fix: - Dockerfile: HEALTHCHECK switched from `curl -f http://localhost:8443/ health` to `curl -fsk https://localhost:8443/health`. `-k` (insecure) is acceptable because the probe is localhost-to-localhost: the same process serving the cert is being probed, no network hop. Pinning `--cacert` is not viable for the published image because the bootstrap cert is per-deploy (generated into the `certs` named volume on first up; operator-supplied via Helm's `existingSecret` or cert-manager). Long-form docblock cross-references the audit closure, the compose vs Helm vs examples coverage matrix, and the CI guardrail. - Dockerfile.agent: added HEALTHCHECK using `pgrep -f certctl-agent` matching the compose pattern. Added `procps` to the runtime apk install — fixes both the new image-level HEALTHCHECK AND the pre-existing compose probe that was silently failing. Phase 2 — Helm readiness probe path: - deploy/helm/certctl/values.yaml: server.readinessProbe.httpGet.path changed from `/readyz` to `/ready`. Liveness probe path (`/health`) was correct and is unchanged. Probes block now carries an explanatory comment naming the registered no-auth probe routes and the U-2 closure rationale. Phase 3 — Image-level integration tests: - deploy/test/healthcheck_test.go (new, //go:build integration): TestPublishedServerImage_HealthcheckSpecUsesHTTPS builds the server image, inspects `Config.Healthcheck.Test` via `docker inspect`, and asserts the array contains `https://localhost:8443/health` and `-k`, and does NOT contain `http://localhost:8443/health` (positive + negative regression contracts). TestPublishedAgentImage_HealthcheckSpecExists builds the agent image and asserts the HEALTHCHECK uses `pgrep` against `certctl-agent`. Both tests `t.Skip` cleanly when docker isn't available (sandbox / CI without docker-in-docker) — verified locally: tests skip with the diagnostic and the suite returns PASS. TestPublishedServerImage_HealthcheckTransitionsToHealthy is a documented `t.Skip` placeholder until the harness wires a sidecar postgres for image-level smoke; the spec-level tests above cover the audit-flagged regression. Phase 4 — CI guardrail: - .github/workflows/ci.yml: new "Forbidden plaintext HEALTHCHECK regression guard (U-2)" step. Scoped patterns catch `HEALTHCHECK.*http://` and `curl -f http://localhost:8443/health` in any `Dockerfile*`. Comment lines exempt; docs/upgrade-to-tls.md out of scope (the post-cutover invariant string at line 182 is intentionally a documented expected-failure assertion). Verified locally on the real tree (passes) and against synthetic regressions (each fires the guard). Phase 5 — Docs sweep: - docs/connectors.md: 15 stale curl examples updated from `http://localhost:8443/...` to `https://localhost:8443/...` with `--cacert "$CA"` injected on every site. Added a one-time introductory note documenting the `$CA` extraction with `docker compose ... exec ... cat /etc/certctl/tls/ca.crt`, matching the pattern in docs/quickstart.md. Pre-U-2 these examples silently failed against the HTTPS listener. Phase 6 — Release surface: - CHANGELOG.md: appended U-2 section to the existing [unreleased] block (immediately below the G-1 entry). Sections: explanatory blockquote covering all three bugs (primary + 2 adjacent), Fixed, Added, Changed. Verification (all gates pass): - go build ./... — clean - go vet ./... — clean - go vet -tags integration ./deploy/test/ — clean - go test -short ./... — every package green - go test -tags integration -v -run TestPublishedServerImage|TestPublishedAgentImage ./deploy/test/ — three tests SKIP cleanly with "docker not available" diagnostic - helm lint deploy/helm/certctl/ — clean - helm template smoke render — succeeds; rendered Deployment carries `path: /ready` and zero `/readyz` matches - python3 yaml.safe_load on api/openapi.yaml — parses - govulncheck ./... — no vulnerabilities in our code - CI guardrail mirror: clean on real tree, fires on synthetic regression patterns Out of scope (intentionally untouched): - cmd/server/main.go::ListenAndServeTLS — HTTPS-only is correct, this finding does NOT propose adding back a plaintext listener. - deploy/docker-compose.yml:126 HEALTHCHECK — already correct. - deploy/docker-compose.test.yml HEALTHCHECK blocks — already correct. - All 5 examples/*/docker-compose.yml HEALTHCHECK overrides — already correct (they ALSO use `-fsk https://localhost:8443/health`). - Helm server.livenessProbe.httpGet — already uses `scheme: HTTPS` + `path: /health`, correct. - docs/upgrade-to-tls.md:182 `curl ... http://localhost:8443/health` invariant line — that's the expected-failure assertion for the post-cutover state ("plaintext is gone, expect Connection refused"); intentionally left intact. - Go production code — this is purely a deploy-image / probe / docs / Helm-chart fix. Refs: coverage-gap-audit-2026-04-24-v5/unified-audit.md §2 P1 cluster, cat-u-healthcheck_protocol_mismatch Audit recommendation followed verbatim: 'change Dockerfile:80 to CMD curl -kf https://localhost:8443/health'.	2026-04-25 12:02:18 +00:00
shankar0123	e17788355b	Merge branch 'fix/g2-apikey-hash-redaction'	2026-04-25 01:56:34 +00:00
shankar0123	87213128cc	fix(security,domain): redact Agent.APIKeyHash from JSON wire shape (G-2) ... Pre-G-2 internal/domain/connector.go::Agent::APIKeyHash was tagged `json:"api_key_hash"` and shipped on every wire surface that returned domain.Agent — GET /api/v1/agents (PagedResponse{Data: agents}), GET /api/v1/agents/{id}, GET /api/v1/agents/retired, and the POST /api/v1/agents registration response. Every authenticated client (browser, CLI --json, MCP tool calls) received the SHA-256-of-the-API-key string. The browser silently dropped it because web/src/api/types.ts omits the field, but CLI and MCP consumers print full JSON so the hash was visible there. Even though the value is a hash and not the plaintext key, shipping it gives an attacker an offline brute-force target if the API-key entropy is low (certctl doesn't enforce a minimum on operator- supplied keys), and there's no business reason for any client to ever receive it — the value is server-internal, used only for the lookup at internal/repository/postgres/agent.go::GetByAPIKey. (Audit: cat-s5-apikey_leak in coverage-gap-audit-2026-04-24-v5/unified-audit.md.) We chose the audit's recommended fix (json:"-") plus a defense-in-depth MarshalJSON plus a CI guardrail. Three layers because struct-tag redaction alone is one rebase away from being silently reverted, the custom MarshalJSON catches the case where a parent struct embeds Agent under a different tag, and the CI grep blocks reintroduction at the spec or frontend boundary even without a code review catching it. Files changed: Phase 1 — Domain redaction: - internal/domain/connector.go: APIKeyHash tag flipped from `json:"api_key_hash"` to `json:"-"`. New Agent.MarshalJSON with value receiver + type-alias-recursion-break that explicitly zeroes APIKeyHash on the marshal-time copy. Long-form docblock explaining the G-2 closure rationale + cross-references to service.RegisterAgent (populator), repository.AgentRepository:: GetByAPIKey (consumer), docs/architecture.md (DB-shape vs API-shape distinction), and the audit finding. Phase 2 — Domain tests (5 test functions): - internal/domain/connector_test.go: TestAgent_MarshalJSON_RedactsAPIKeyHash pins the marshal-boundary contract on a value receiver. ...RedactsViaPointer pins the *Agent path. ...RedactsInSlice pins the []Agent path that the ListAgents handler actually emits via PagedResponse. ...DoesNotMutateReceiver pins the by-value-receiver contract so a future refactor that switches to pointer-receiver gets caught. ...RoundTrip pins the wire-shape guarantee that APIKeyHash is dropped on encode and cannot reappear on decode. Single sentinel value ("sha256:LEAKED-CREDENTIAL-DERIVATIVE- SENTINEL") flows through every fixture for grep-ability on regression. Phase 3 — Handler tests (4 test functions): - internal/api/handler/agent_handler_test.go: TestListAgents_DoesNotLeakAPIKeyHash, TestGetAgent_DoesNotLeakAPIKeyHash, TestRegisterAgent_DoesNotLeakAPIKeyHash, TestListRetiredAgents_DoesNotLeakAPIKeyHash. Each asserts (a) the literal substring "api_key_hash" is absent from the httptest-captured body, (b) the leak sentinel value is absent, (c) the non-leaked fields ARE present (sanity that the handler is serving real data, not just empty payloads). Shared sentinel "sha256:LEAKED-CREDENTIAL-DERIVATIVE- HANDLER-SENTINEL" so a single grep over a failing test's output identifies the leak surface immediately. Phase 4 — Spec / docs: - api/openapi.yaml: api_key_hash property REMOVED from Agent schema (was at line 3690). Inline G-2 comment naming the closure + the database-vs-API-shape distinction so a future spec edit doesn't silently re-introduce the field. - docs/architecture.md: ER-diagram block already documents the agents table including api_key_hash (DB shape — correct). Added a sibling note paragraph immediately below the diagram explaining that several columns are intentionally server-internal (api_key_hash redaction + issuers.config / deployment_targets.config encrypted shadow), with cross-references to the redaction enforcement site, the OpenAPI schema, the frontend interface, and the CI guardrail. - web/src/api/types.ts: Agent interface unchanged in shape (already omitted the field) but added a leading comment block explaining WHY the omission is intentional — stops a future frontend dev from "completing" the interface from the OpenAPI spec or the Go struct. Phase 5 — CI guardrail: - .github/workflows/ci.yml: new "Forbidden api_key_hash JSON-shape regression guard (G-2)" step. Scoped patterns catch the actual regression shapes — Go struct tag (json:"api_key_hash"), frontend interface declaration, OpenAPI schema property, YAML enum/array membership. Repository / migration / seed / service / integration / unit-test / comment lines exempt. Verified locally on the real tree (passes) and against 4 synthetic regression patterns (each fires the guardrail). Mirrors the G-1 pattern from .github/workflows/ ci.yml lines 47-108. Phase 5b — Sweep verification (no changes, results documented for the next reader): - internal/api/middleware/audit.go: doesn't serialize Agent struct; records request body only. No leak. - service.RegisterAgent audit-event payload: `map[string]interface{}{ "name": name, "hostname": hostname}` — name + hostname only, no APIKeyHash. No leak. - All 9 slog sites that mention agent: scalar attrs only ("agent_id", "error", "agent_hostname"), never the full struct. No leak. - internal/mcp, internal/cli, cmd/cli, cmd/mcp-server: zero matches for APIKeyHash / api_key_hash. Both pass server JSON verbatim, so the wire-side fix transitively closes them. Verification (all gates pass): - go build ./... - go vet ./... - go test -short ./... — every package green - go test -short -race ./internal/domain/... ./internal/api/handler/... — clean - govulncheck ./... — no vulnerabilities in our code - helm lint deploy/helm/certctl/ — clean - helm template smoke render — succeeds - python3 yaml.safe_load on api/openapi.yaml — parses - OpenAPI Agent schema scan: no api_key_hash property - CI guardrail mirror: clean on real tree, fires on all 4 synthetic regression patterns - Domain pkg coverage: Agent.MarshalJSON 100%, connector.go total 87.5% - Handler pkg coverage: 79.2% Sample response body (httptest captured during verification, GET /api/v1/agents/{id} via the new handler test): {"id":"agent-demo","name":"demo-agent","hostname":"demo.host", "status":"Online","last_heartbeat_at":"2026-04-24T11:59:30Z", "registered_at":"2026-04-24T12:00:00Z","os":"linux", "architecture":"amd64","ip_address":"10.0.0.42", "version":"v2.0.49"} Note the absence of any api_key_hash key, even though the in-memory struct passed to the handler had APIKeyHash set to a sentinel. Out of scope (intentionally untouched): - internal/repository/postgres/agent.go SELECT/INSERT/UPDATE/scan paths and GetByAPIKey lookup — DB column stays, repo still populates the struct, auth lookup still works. The redaction is a marshal-boundary concern. - migrations/000001_initial_schema.up.sql + migrations/seed_*.sql — DB schema and seed data unchanged. - internal/service/agent.go::RegisterAgent — service-side hashing and persistence unchanged. - Other domain types with potential credential-derivative fields (Issuer.Config, DeploymentTarget.Config, notifier configs). Not flagged by the audit; some are already protected (e.g., DeploymentTarget.EncryptedConfig []byte `json:"-"`). File a separate audit pass if recon surfaces additional leaks. - Per-resource DTO layer across every handler. Single audit finding, single domain type. - A separate possible follow-up: the v2 RegisterAgent endpoint doesn't return the plaintext API key to the agent, which may mean self-bootstrap via POST /api/v1/agents is broken. Verified during recon; out of scope for G-2; should be its own ticket. Refs: coverage-gap-audit-2026-04-24-v5/unified-audit.md §2 P1 cluster, cat-s5-apikey_leak Audit recommendation: 'json:"-" or API-response DTO excluding APIKeyHash' — went with the json:"-" + MarshalJSON defense-in-depth pair plus CI guardrail and structural docs.	2026-04-25 01:56:26 +00:00
shankar0123	697fa792ea	Merge branch 'fix/g1-jwt-silent-auth-downgrade-removal'	2026-04-25 00:22:33 +00:00
shankar0123	9c1d446e40	fix(security,config): remove unimplemented JWT auth-type, close silent downgrade (G-1) ... The pre-G-1 config validator accepted CERTCTL_AUTH_TYPE=jwt and the startup log faithfully echoed 'authentication enabled type=jwt'. Reasonable people read that and concluded JWT auth was on. It wasn't. The auth-middleware wiring at cmd/server/main.go unconditionally routed every request through the api-key bearer middleware regardless of cfg.Auth.Type. So CERTCTL_AUTH_TYPE=jwt quietly compared the incoming 'Authorization: Bearer <token>' against whatever string the operator put in CERTCTL_AUTH_SECRET — real JWT clients got 401, and operators who treated CERTCTL_AUTH_SECRET as a *signing* secret (because they thought they were configuring JWT) had effectively handed an attacker an api-key. A security finding masquerading as a config option. We chose the audit-recommended structural fix: remove the option, fail fast at startup, and add the gateway-fronting pattern as the documented forward path. Implementing JWT middleware would have meant jwks vs static-secret rotation, claim mapping, expiry enforcement, audience and issuer validation, key rollover semantics, and regression coverage at the same depth as the existing api-key path — a feature, not a fix. Operators who genuinely need JWT/OIDC front certctl with an authenticating gateway (oauth2-proxy / Envoy ext_authz / Traefik ForwardAuth / Pomerium / Authelia) and run the upstream certctl with CERTCTL_AUTH_TYPE=none. Same shape works on docker-compose and Helm. The change is comprehensive across 7 phases — every surface that mentioned 'jwt' as a certctl-auth-type is updated, plus structural backstops (typed enum, runtime guard, helm template validation, CI grep guard) so the lie can't reappear. Files changed: Phase 1 — production code (typed enum + jwt removal): - internal/config/config.go: AuthType typed alias + AuthTypeAPIKey / AuthTypeNone constants + ValidAuthTypes() helper. Validate() routes literal 'jwt' through a dedicated multi-line diagnostic naming the authenticating-gateway pattern, then cross-checks against ValidAuthTypes(). Secret-required branch simplified to api-key-only. Field comment on AuthConfig.Type rewritten to drop jwt and point at the gateway pattern. - internal/api/middleware/middleware.go: AuthConfig.Type field comment references the typed config.AuthType constants. - internal/api/handler/health.go: same treatment for HealthHandler.AuthType. - cmd/server/main.go: defense-in-depth runtime switch immediately after config.Load() — exits 1 on any unsupported auth-type that bypassed the validator. Auth-disabled startup log explicitly names the authenticating-gateway pattern. Phase 2 — tests (Red→Green, contract pinning): - internal/config/config_test.go: TestValidate_JWTAuth_RejectedDedicated (two table rows pinning the dedicated G-1 error fires regardless of whether Secret is set), TestValidAuthTypesDoesNotContainJWT (property guard against future re-introduction), TestValidAuthTypesIsExactly_APIKey_None (allowed-set contract), TestValidate_GenericInvalidAuthType (pins non-jwt invalid values still hit the generic invalid-auth-type error). Removed the prior TestValidate_JWTAuth_MissingSecret happy-path since its premise is inverted post-G-1. - internal/api/handler/health_test.go: removed TestAuthInfo_ReturnsAuthType_JWT (which baked the silent-downgrade lie into the regression suite). Pre-existing _APIKey test continues to cover the api-key happy path. Phase 3 — spec, docs, env templates: - api/openapi.yaml: auth_type enum dropped to [api-key, none] with inline comment naming the G-1 closure. - .env.example (root): CERTCTL_AUTH_TYPE comment block rewritten to drop jwt and point at the gateway pattern; secret-required conditional simplified to api-key-only. - docs/architecture.md: middleware-stack bullet rewritten to drop the JWT mention; new H3 'Authenticating-gateway pattern (JWT, OIDC, mTLS)' section explaining the design rationale and listing oauth2-proxy / Envoy ext_authz / Traefik ForwardAuth / Pomerium / Authelia / Caddy forward_auth / Apache mod_auth_openidc / nginx auth_request as the standard fronting options. - docs/upgrade-to-v2-jwt-removal.md (new ~125 lines): migration guide with preconditions, what-changes, both recovery paths, complete docker-compose oauth2-proxy walkthrough, Traefik ForwardAuth and Envoy ext_authz patterns, rollback posture. Phase 4 — Helm chart (template validation + docs): - deploy/helm/certctl/templates/_helpers.tpl: new certctl.validateAuthType helper mirroring the existing certctl.tls.required pattern. Fails template render on any server.auth.type outside {api-key, none} with a multi-line diagnostic. - deploy/helm/certctl/templates/server-deployment.yaml, server-configmap.yaml, server-secret.yaml: invoke the helper at the top of each template that depends on .Values.server.auth.type. - deploy/helm/certctl/values.yaml: auth: block comment expanded with the G-1 rationale and gateway-pattern cross-reference. - deploy/helm/CHART_SUMMARY.md: server.auth.type table row now surfaces the allowed set and points at the upgrade doc. - deploy/helm/certctl/README.md: new 'JWT / OIDC via authenticating gateway' section with a Kubernetes-flavored oauth2-proxy + certctl walkthrough. Phase 5 — release surface: - CHANGELOG.md: new [unreleased] top entry with Breaking / Removed / Added / Changed sections; explicit pointer at docs/upgrade-to-v2-jwt-removal.md from the Breaking subsection. Phase 6 — CI guardrail: - .github/workflows/ci.yml: new 'Forbidden auth-type literal regression guard (G-1)' step. Scoped patterns catch the actual regression shapes (map literal, slice literal, switch case, OpenAPI enum, env-file default, AuthType('jwt') cast). Comments and the dedicated rejection branch are intentionally exempt; connector-package JWT references (Google OAuth2 / step-ca) are exempt as out-of-scope external protocols. Verified locally: the guard passes on the actual tree and fires on all 4 synthetic regression patterns. Out of scope (explicitly untouched): - internal/connector/discovery/gcpsm/gcpsm.go — Google OAuth2 service- account JWT (external protocol). - internal/connector/issuer/googlecas/googlecas.go — same. - internal/connector/issuer/stepca/stepca.go — step-ca's provisioner one-time-token JWT for /sign API. - docs/test-env.md, docs/connectors.md, docs/features.md — describe external CAs' use of JWT, not certctl's auth shape. - Implementing actual JWT middleware. Feature, not a fix. Verification (all gates pass): - go build ./... — clean - go vet ./... — clean - go test -short ./... — every package green - go test -short -race ./internal/config/... ./internal/api/... — clean - govulncheck ./... — no vulnerabilities in our code - helm lint deploy/helm/certctl/ — clean - helm template with auth.type=api-key — renders OK - helm template with auth.type=none — renders OK - helm template with auth.type=jwt — fails with validateAuthType diagnostic (exit 1) - python3 yaml.safe_load on api/openapi.yaml — parses - CI guardrail mirror — clean on real tree, fires on all 4 synthetic regression patterns - Smoke test: 'CERTCTL_AUTH_TYPE=jwt ./certctl-server' exits non-zero with: 'Failed to load configuration: CERTCTL_AUTH_TYPE=jwt is no longer accepted (G-1 silent auth downgrade): no JWT middleware ships with certctl. To use JWT/OIDC, run an authenticating gateway (oauth2-proxy / Envoy ext_authz / Traefik ForwardAuth / Pomerium) in front of certctl and set CERTCTL_AUTH_TYPE=none on the upstream. See docs/architecture.md "Authenticating-gateway pattern" and docs/upgrade-to-v2-jwt-removal.md for the migration walkthrough' config pkg coverage: ValidAuthTypes 100%, Validate 94.7%, total 75.5%. Refs: coverage-gap-audit-2026-04-24-v5/unified-audit.md §2 P1 cluster, cat-g-jwt_silent_auth_downgrade Audit recommendation followed verbatim: 'Remove jwt from validAuthTypes until middleware ships'.	2026-04-25 00:22:23 +00:00
shankar0123	3192cd15c5	Merge branch 'fix/u1-followups-helm-rootenv-examples'	2026-04-24 23:51:18 +00:00
shankar0123	af47d19ae2	fix(deploy,examples,env): close U-1 trap end-to-end across Helm, examples, and root env ... Follow-up to cfc234e (U-1 docker-compose fix) — closes the remaining adjacent code paths that share the postgres-first-boot-password-binding root cause but were scoped out of the original commit. The runtime diagnostic in internal/repository/postgres/db.go::wrapPingError (landed in a911970) already covers every NewDB call site, so Helm operators and example users hit the SQLSTATE 28P01 guidance for free at startup. What was missing: deployment-shape-specific remediation guidance (kubectl vs docker-compose), the hardcoded password in the *root* .env.example, and shared ops notes for the 5 examples/ compose files. This commit closes all three. Files changed: - .env.example (root) — line 16 had `postgres://certctl:certctl@...` with the password hardcoded literally instead of interpolating POSTGRES_PASSWORD. Edit if a user copied this file as their .env (binary-direct deployment, not docker-compose) and rotated POSTGRES_PASSWORD on line 10, the URL on line 16 still carried 'certctl' — silent two-line drift. Replaced 'certctl' with the same default that line 10 carries ('change-me-in-production') and added an explanatory comment block describing the docker-compose override semantics, when this URL matters (binary-direct), and the cross-reference to the U-1 wrapPingError diagnostic. Also fixed an adjacent bug: line 31 CERTCTL_SERVER_URL was `http://localhost:8443`, which agents reject at startup since v2.2 (HTTPS-everywhere milestone made the control plane HTTPS-only with TLS 1.3 pinned). Updated to https:// with a comment pointing operators at the bootstrap CA bundle. - deploy/helm/certctl/values.yaml — postgresql.auth.password field had a one-line 'REQUIRED' comment. Expanded into a full WARNING block (~25 lines) explaining the PVC retention semantics, the failure symptom, and both kubectl-flavored remediation paths: non-destructive (`kubectl exec ... ALTER ROLE`) preferred for environments with data, and destructive (`helm uninstall + kubectl delete pvc`) for dev/demo. Cross-references the wrapPingError runtime diagnostic. - deploy/helm/certctl/README.md (new, ~115 lines) — chart-level operational guide. Covers quick install, both remediation paths with concrete kubectl commands, why-we-don't-fix-this-in-the-chart explanation, cross-references to the docker-compose docs, server API key rotation (the easy case — comma-separated key list), TLS provisioning shapes, embedded-vs-external postgres, and uninstall semantics with the PVC retention gotcha called out. - examples/README.md (new, ~55 lines) — shared operational notes for the 5 example deployments. Covers the postgres password rotation trap with example-flavored remediation paths (`docker compose -f examples/<x>/...`), the TLS warning, and teardown semantics. Replaces what would otherwise be 5x duplication across per-example READMEs. - examples/{acme-nginx,acme-wildcard-dns01,multi-issuer,private-ca-traefik, step-ca-haproxy}/*.md — one-line cross-reference at the top of each example's primary doc, pointing at examples/README.md for the shared ops notes. Avoids 5x duplication of the same warning text while still surfacing the link in every operator's first-touch surface. Verification: - go build ./... — clean - go vet ./... — clean - go test -short ./internal/repository/postgres/ — 4/4 wrapPingError tests still passing (no production-code touch in this commit) - helm lint deploy/helm/certctl/ — clean (1 INFO about chart icon, pre-existing) - helm template smoke test — renders without error - python3 yaml.safe_load on values.yaml — parses Refs: coverage-gap-audit-2026-04-24-v5/unified-audit.md §2 P1 cluster, cat-u-quickstart_postgres_password_volume_trap Closes the three deliberate scope-outs from cfc234e (Helm, root .env.example, examples/) end-to-end. Adjacent bugs caught while in scope: - root .env.example:16 hardcoded password not matching line 10 - root .env.example:31 http:// URL incompatible with HTTPS-only v2.2	2026-04-24 23:51:13 +00:00
shankar0123	cfc234ec42	Merge branch 'fix/u1-postgres-password-volume-trap-diagnostic'	2026-04-24 23:21:33 +00:00
shankar0123	a91197014f	fix(db): emit volume-state guidance on postgres auth failure (U-1, #10) ... The shipped quickstart instructs operators to copy deploy/.env.example to deploy/.env, edit POSTGRES_PASSWORD, and run docker compose up. On the *first* boot of a fresh checkout this works. On the *second* boot — i.e., when an operator first booted with the default POSTGRES_PASSWORD=certctl, then edited .env and re-ran up — the certctl-server container picks up the new password (env interpolated at every container start) but postgres does not. The postgres docker-entrypoint runs initdb only when the data dir is empty; on subsequent boots the persistent named volume postgres_data is non-empty so pg_authid retains the password baked in on first boot. The server connects with the new credentials, postgres rejects them, and the operator sees an opaque `pq: password authentication failed for user "certctl"` in the server log with no pointer to the actual cause. New- operator onboarding gets blocked on the documented production path. Why a doc fix alone is not sufficient. Operators don't reread the docs after a successful first boot — the trap fires on the *second* up, when they think they've already learned the system. The opaque pq error is indistinguishable in the log from a typo'd password or a misconfigured secret store. The diagnostic has to fire at the moment the failure is observed. Why we don't try to fix the bootstrap. The env-vs-pg_authid divergence is intrinsic to how the official postgres image bootstraps (see docker-entrypoint.sh: initdb runs only if PGDATA is empty). Switching to a bind mount or ephemeral volume breaks the production path; switching to POSTGRES_PASSWORD_FILE + ALTER ROLE adds operator surface without eliminating the divergence. The ergonomic fix is to surface the failure mode loudly, with both remediation paths, at the exact log line where it becomes visible. Two remediation paths, surfaced together. Destructive: `docker compose -f deploy/docker-compose.yml down -v && up -d --build` — wipes the postgres volume so initdb re-runs with the new env value. Use this on demos / first-time setup where data loss is acceptable. Non-destructive: `docker compose exec postgres psql -U certctl -c "ALTER ROLE certctl PASSWORD '<new>';"` followed by a server restart with the matching POSTGRES_PASSWORD. Use this on any environment that holds data you want to keep. Surfacing both means the operator can pick based on their environment without us assuming. Files changed: - internal/repository/postgres/db.go — extract wrapPingError(err) helper. errors.As against *pq.Error; on SQLSTATE 28P01 (invalid_password) emit the multi-line guidance preserving the %w wrap chain. Non-28P01 errors retain the original `failed to ping database: %w` shape so transient connection-refused / timeout paths don't get noisy. Add pgErrInvalidPassword = "28P01" constant. Convert blank `_ "github.com/lib/pq"` import to direct import (driver registration still works via init()) so we can name the *pq.Error type at compile time. NewDB now calls wrapPingError(err) instead of inlining the wrap. - internal/repository/postgres/db_test.go (new) — 4 internal-package unit tests covering wrapPingError. AuthFailureGuidance pins the contract substrings ("SQLSTATE 28P01", "POSTGRES_PASSWORD", "first boot", "down -v", "ALTER ROLE"). NonAuthErrorPreservesOriginalWrap pins the no-leak contract for SQLSTATE 08006 (connection_failure). NonPqErrorPreservesOriginalWrap pins the network-level path. NilReturnsNil pins defensive contract. All run in -short without testcontainers — package postgres (internal) so the unexported helper is callable directly. - docs/quickstart.md — `> **Warning:**` callout immediately after the `cp deploy/.env.example deploy/.env` block at lines 56-61. Names the trap, names the SQLSTATE, gives both remediation paths. Uses the in-file `> **Note:**` blockquote convention. - deploy/ENVIRONMENTS.md — `**Stateful volume — first-boot password binding (U-1)**` paragraph appended to the Postgres expert-note block. Explains the env-vs-pg_authid divergence, points at wrapPingError as the runtime diagnostic, lists both remediation paths. Uses the in-file `**Expert note:**` convention. Out of scope (separate follow-ups): - deploy/helm/certctl/templates/postgres-statefulset.yaml has the same root cause via PVC retention. The wrapPingError diagnostic covers the Helm path because the same NewDB code runs at server startup; the Helm-specific doc warning lands separately. - /.env.example at repo root (line 16 hardcodes the password literally inside CERTCTL_DATABASE_URL rather than interpolating) — adjacent trap, separate fix. - examples/{acme-nginx,private-ca-traefik,step-ca-haproxy,multi-issuer, acme-wildcard-dns01}/docker-compose.yml all carry the pattern. The diagnostic covers them; targeted doc warnings are scoped to the canonical quickstart + ENVIRONMENTS docs. Out of consideration: - Switch to bind mount / ephemeral volume — breaks the production path. - POSTGRES_PASSWORD_FILE + Docker secret + ALTER ROLE rotation — adds operator surface without fixing the env-vs-pg_authid divergence. Verification (all passing): - go build ./... - go vet ./... - go test -short -race ./internal/repository/postgres/ — 4/4 new tests pass plus existing tests - go test -short ./... — every package green - govulncheck ./... — no vulnerabilities in our code - wrapPingError coverage 100%; postgres pkg total unchanged in shape (NewDB/RunMigrations were 0% pre-fix, still 0% post-fix; new helper adds 100%-covered statements) Refs: coverage-gap-audit-2026-04-24-v5/unified-audit.md §2 P1 cluster, cat-u-quickstart_postgres_password_volume_trap GitHub Issue #10 (mikeakasully)	2026-04-24 23:21:26 +00:00
shankar0123	d6959a75c1	Merge branch 'test/l1-repo-integration-coverage' v2.0.49	2026-04-20 20:39:10 +00:00
shankar0123	97b23e98d9	test(repository): close L-1 integration-coverage gap for HealthCheck + RenewalPolicy ... The coverage-gap audit flagged L-1 (P2): `HealthCheckRepository` (453 LOC, 11 methods) and `RenewalPolicyRepository` (289 LOC, 5 methods post-G-1 — the audit's "92 lines, 2 methods" figure was stale) ship to production with zero live-DB integration coverage. The existing `repo_test.go` header self-documents the gap: "15 of 17 PostgreSQL repository files". Operationally load-bearing piece: M48's scheduler calls `HealthCheckRepository.ListDueForCheck` every tick to drive continuous TLS health monitoring. A silent SQL regression there — wrong INTERVAL math, NULL-handling slip, lost ORDER BY — would fail open: operator adds endpoint → scheduler never picks it up → endpoint degrades in production → no alert. The loop continues ticking and logs "processed 0 endpoints" normally, so the failure mode is operationally invisible. Closure shape (test-only; no production code touched): - internal/repository/postgres/health_check_test.go (new file, 7 tests) · TestHealthCheckRepository_CRUD · TestHealthCheckRepository_GetByEndpoint · TestHealthCheckRepository_List_Filters · TestHealthCheckRepository_ListDueForCheck (the load-bearing one — seeds four rows with differing last_checked_at+interval relationships to NOW() plus one NULL-last_checked_at row, asserts the correct subset returns and ORDER BY last_checked_at ASC NULLS FIRST holds) · TestHealthCheckRepository_RecordHistory_GetHistory · TestHealthCheckRepository_PurgeHistory · TestHealthCheckRepository_GetSummary - internal/repository/postgres/renewal_policy_test.go (new file, 3 tests) · TestRenewalPolicyRepository_CRUD (exercises auto-generated rp-<slug(name)> PK, JSONB round-trip of [30,14,7,0] thresholds, UpdatedAt monotonic advance, ORDER BY name for List) · TestRenewalPolicyRepository_DuplicateName (asserts errors.Is(err, repository.ErrRenewalPolicyDuplicateName) on both Create-name-unique and Update-name-unique collision paths, the pg 23505 sentinel mapping) · TestRenewalPolicyRepository_DeleteInUse (raw-INSERTs a managed_certificates row FK'ing the policy, asserts errors.Is(err, repository.ErrRenewalPolicyInUse) from pg 23503 ON DELETE RESTRICT, cleans up, then asserts not-found surfaces distinctly) - internal/repository/postgres/repo_test.go (one-line header flip) "covering 15 of 17 ... repository files" → "17 of 17"; added cross-reference pointing readers at the two sibling files. Both new files use the existing getTestDB(t) + schema-per-test-isolation convention and skip via testing.Short() in CI, matching M26 TICKET-003 scaffolding byte-for-byte. Repository/postgres is not in the CI coverage-gate path (grep -nE "internal/repository/postgres" .github/workflows/ci.yml → no hits), so adding test-only files cannot regress gated coverage elsewhere. Verification gates run locally (sandbox without Docker, so the -short skip gate itself is what's exercised; operator runs the testcontainer path locally): 1. go vet ./... — clean 2. go build ./... — clean 3. go test -short -count=1 ./... — clean 4. go test -race -short ./internal/repository/postgres/... — clean 5. staticcheck — absent; CI checkset holds 6. govulncheck — skipped; test-only, no deps 7. per-layer coverage no-regression — N/A; repo/pg not gated 8. tsc --noEmit — N/A; no frontend change 9. vitest run — N/A; no frontend change 10. vite build — N/A; no frontend change 11. OpenAPI lint — N/A; no spec change No migration, no interface change, no production code diff. The RenewalPolicyRepository drift between audit ("92 lines, 2 methods") and HEAD (289 lines, 5 methods post-G-1) is documented honestly in the audit report's Resolution Log, not papered over. Closes: coverage-gap-audit L-1 (P2)	2026-04-20 20:39:06 +00:00
shankar0123	4cf5fcdb4f	Merge branch 'fix/d1-cli-status-endpoint'	2026-04-20 19:41:03 +00:00
shankar0123	1ee67b7792	D-1: correct certctl-cli status endpoint path (/api/v1/health -> /health) ... The CLI's GetStatus() was issuing GET /api/v1/health, but the real liveness route is GET /health at internal/api/router/router.go:76 (mounted at root, not under /api/v1/). Every 'certctl-cli status' invocation 404'd since M16b. The regression was masked because TestClient_GetStatus encoded the same wrong path on both sides of the contract -- the mock server also dispatched on /api/v1/health -- so the production request matched the test's buggy dispatch and the green bar hid the bug. Two-line fix: - internal/cli/client.go:615: "/api/v1/health" -> "/health" - internal/cli/client_test.go:296: mock dispatch to match Red receipt captured before the green fix: with the test fixture corrected but production still wrong, TestClient_GetStatus fails 'parsing response: unexpected end of JSON input' (the client falls through the mock's if/else to the default 200 OK empty body and the JSON decoder chokes). After the production edit the test passes. GetStatus()'s response decoder is already compatible with the real /health shape (graceful 'ok' check on health["status"], optional health["timestamp"]). No interface change. No migration. No frontend change. No OpenAPI delta -- /health is a root-level liveness probe, not part of the /api/v1/ surface.	2026-04-20 19:40:58 +00:00
shankar0123	128d0eeaa8	Merge branch 'fix/g1-renewal-policies-api' ... G-1: renewal-policies API + frontend FK-drift fix. Adds /api/v1/renewal-policies CRUD backing the dropdown that managed_certificates.renewal_policy_id FKs into. Three frontend call sites swapped from getPolicies() (pol-*, compliance rules) to getRenewalPolicies() (rp-*, lifecycle policies). Validation bounds, pg 23503/23505 error mapping to HTTP 409, OpenAPI coverage, test suite. No migration — renewal_policies table already exists from schema 000001.	2026-04-20 18:53:09 +00:00
shankar0123	9834b4e4a4	G-1: renewal-policies API + frontend FK-drift fix ... Three frontend call sites (OnboardingWizard.tsx:603, CertificatesPage.tsx:52, CertificateDetailPage.tsx:169) populated the renewal_policy_id dropdown from getPolicies() — the compliance-rule endpoint returning pol-* IDs — which violated the FK managed_certificates.renewal_policy_id REFERENCES renewal_policies(id) ON DELETE RESTRICT. Create would fail pg 23503 at insert. Backend (new): - RenewalPolicyRepository CRUD + ListAll/ExistsByID (pg 23503 → ErrRenewalPolicyInUse → HTTP 409; pg 23505 → ErrRenewalPolicyDuplicateName → HTTP 409) - RenewalPolicyService with repo-only constructor. Service sentinels var-alias the repo sentinels so errors.Is walks across layers. - RenewalPolicyHandler with validation bounds: name 1–255; renewal_window_days [1,365] default 30; max_retries [0,10] not defaulted; retry_interval_seconds [60,86400] default 3600; alert_thresholds_days [0,365] default [30,14,7,0]. Auto-generated IDs rp-<slug(name)>. - Router registers 5 routes under /api/v1/renewal-policies[/{id}]. Frontend: - CertificatesPage/CertificateDetailPage/OnboardingWizard now call getRenewalPolicies() and render rp-* IDs. - client.ts adds getRenewalPolicies/createRenewalPolicy/updateRenewalPolicy/ deleteRenewalPolicy. types.ts adds the RenewalPolicy shape. OpenAPI: RenewalPolicies tag + 5 operations + 3 schemas (RenewalPolicy, RenewalPolicyCreateRequest, RenewalPolicyUpdateRequest). 409 responses on create/update duplicate-name and delete FK-in-use. No migration — renewal_policies table already exists from the initial schema (000001). Tests: - internal/service/renewal_policy_test.go: CRUD + validation + sentinel error wrapping. - internal/api/handler/renewal_policy_handler_test.go: handler endpoint contracts including 400/404/409. - web/src/api/client.test.ts: 4 subtests covering the 4 new API functions. Phase 3 gates all green: go vet, build, short tests, race tests (service/ handler/router/scheduler), staticcheck (G-1 packages), govulncheck (0 reachable), coverage (service 69.7%, handler 79.0%, domain 86.9%, middleware 80.6% — all above thresholds), tsc, vitest (256 passed), vite build, OpenAPI structural validation.	2026-04-20 18:53:01 +00:00
shankar0123	cab579368b	Merge branch 'fix/audit-f001-f002-f003' ... Closes F-001 (CRL scoped query via composite index), F-002 (digest error body sanitization), and F-003 (ctx-aware sleep at three sites). Verification: build, vet, race-short test sweep across all packages green. govulncheck clean. golangci-lint run deferred — local environment's golangci-lint is v1.64.8 built with go1.24 and rejects the go1.25.9 project; fresh install blocked by disk constraints. CI lane will cover it.	2026-04-20 16:52:00 +00:00
shankar0123	4e5522a999	F-001/F-002/F-003: CRL prefix-scan, digest error sanitization, ctx-aware sleeps ... F-001 (P3): GenerateDERCRL scoped to issuer via composite index - Add RevocationRepository.ListByIssuer leveraging migration 000012's idx_certificate_revocations_issuer_serial composite index as a prefix-scan target. Previously CAOperationsSvc.GenerateDERCRL called ListAll() and filtered by IssuerID in Go — O(total revocations) regardless of how many revocations belonged to the target issuer. - Rewrite GenerateDERCRL to call ListByIssuer(ctx, issuerID) so PostgreSQL drives a prefix scan of the composite index. Drops the in-memory filter. - New regression test in ca_operations_test.go asserts the CRL hot path invokes ListByIssuer exactly once and never ListAll, and that the issuerID is threaded through correctly. F-002 (P3): digest.go admin-auth endpoints no longer leak internal errors - PreviewDigest (GET /api/v1/digest/preview) and SendDigest (POST /api/v1/digest/send) previously wrote err.Error() into the HTTP response body on 500s. Replace with slog.Error server-side logging plus a generic "internal error" response body, matching the house pattern in certificates.go and export.go. F-003 (P4): three blocking time.Sleep sites now honor ctx cancellation - internal/connector/issuer/acme/acme.go:672 (DNS-01 propagation wait) now runs under a select{case <-ctx.Done(): CleanUp + return ctx.Err(); case <-time.After(d):} so graceful shutdown doesn't get stuck behind the propagation delay. - internal/connector/issuer/acme/acme.go:786 (dns-persist-01 propagation wait) same pattern, returns ctx.Err() on cancel. - cmd/agent/main.go:272 (polling backoff inside the heartbeat loop) now wraps the sleep in select{case <-ctx.Done(): continue; case <-time.After(backoff):} so the outer <-ctx.Done() case on the parent loop fires cleanly. Verification: build, vet, and race-enabled short tests green across all 55+ packages. govulncheck reports zero vulnerabilities in the code path. No migration needed — F-001 reuses the existing 000012 composite index. No frontend changes.	2026-04-20 16:51:52 +00:00
shankar0123	55ce86b132	v2.0.48: swap self-signed TLS bootstrap algorithm ed25519 → ECDSA-P256 ... Follow-up to v2.0.47 (HTTPS-Everywhere). The Phase-3 self-signed bootstrap sidecar shipped an ed25519 server cert. Apple's TLS stack — Safari Network Framework and the macOS-bundled LibreSSL 3.3.6 /usr/bin/curl — does not advertise ed25519 in the ClientHello signature_algorithms extension for server certs, so the handshake fails with the server-side log line: tls: peer doesn't support any of the certificate's signature algorithms Homebrew OpenSSL 3.x, Chrome, Firefox, and Linux curl all accept ed25519 server certs fine. Apple is the outlier. Rather than gate the demo stack behind "install Homebrew OpenSSL first," swap the bootstrap algorithm to ECDSA-P256 with SHA-256 — universally supported, including on the Apple stack. Changes - deploy/docker-compose.yml: certctl-tls-init openssl invocation swapped to `-newkey ec -pkeyopt ec_paramgen_curve:P-256 -nodes`; header comment + echo line updated; multi-line rationale paragraph added. - deploy/docker-compose.test.yml: same openssl swap + echo update for the test harness sidecar that writes to the bind-mounted ./test/certs directory the Go integration_test.go pins via CERTCTL_TEST_CA_BUNDLE. - docs/tls.md: Pattern 1 description + code block updated; "Why ECDSA-P256 and not ed25519" rationale paragraph added covering pre-v2.0.48 history, the Apple diagnosis, accepting clients, and the operator migration command. Patterns 2 (existing Secret) and 3 (cert-manager) explicitly called out as unaffected. - docs/upgrade-to-tls.md: docker-compose procedure sentence updated with cross-reference to tls.md Pattern 1. - docs/test-env.md: "Get the CA bundle for curl" sentence updated. Migration Existing demo installs must tear the `certs` named volume down to pick up the new algorithm: docker compose -f deploy/docker-compose.yml down -v docker compose -f deploy/docker-compose.yml up -d --build Not touched - cmd/server/tls.go: algorithm-agnostic. TLS 1.3 min version with [X25519, P-256] curve preferences for key exchange is orthogonal to the server cert's signature algorithm. No Go code change needed. - Helm chart: Patterns 2 and 3 operators supply their own cert; this patch does not affect them. - Unrelated ed25519 uses (agent key algorithm detection, profile algorithm options, SSH key path examples, tlsprobe key metadata, cloud discovery key-algo display): all orthogonal to the server TLS bootstrap cert. Incidental cleanup - .gitignore: dropped dangling `strategy.md` entry (file doesn't exist in repo; entry was cruft). v2.0.48	2026-04-20 04:17:05 +00:00
shankar0123	52248be717	v2.0.47: HTTPS Everywhere — TLS-only control plane, agents/CLI/MCP ... Breaking change release. Plaintext HTTP listener removed. The certctl control plane now terminates TLS 1.3 on :8443 via http.Server.ListenAndServeTLS. No CERTCTL_TLS_ENABLED=false escape hatch. No dual-listener mode. One-step cutover per docs/upgrade-to-tls.md. Server - cmd/server/tls.go: certHolder with SIGHUP hot-reload + atomic cert swap, buildServerTLSConfig (TLS 1.3 min, GetCertificate callback), preflightServerTLS validation - cmd/server/main.go: ListenAndServeTLS in place of ListenAndServe, watchSIGHUP wiring, cert/key path config threading - tls_test.go: 418-line regression coverage of reload, preflight, callback behavior, SAN validation Config - CERTCTL_TLS_CERT_PATH / CERTCTL_TLS_KEY_PATH (required) - Plaintext rejection: agents/CLI/MCP pre-flight-fail on http:// URLs with a pointer to docs/upgrade-to-tls.md Agents, CLI, MCP - All three pre-flight-reject http:// URLs with fail-loud diagnostic - CERTCTL_SERVER_CA_BUNDLE_PATH for private-CA trust - CERTCTL_SERVER_TLS_INSECURE_SKIP_VERIFY for dev-only bypass (loud warning on startup) - install-agent.sh emits both vars as commented template lines docker-compose - certctl-tls-init sidecar generates SAN-valid self-signed cert into deploy/test/certs/ on first boot - All demo-stack curls pin against ca.crt with --cacert Helm chart - Three TLS provisioning modes, exactly one required: - server.tls.existingSecret (operator-supplied) - server.tls.certManager.enabled (cert-manager integration) - server.tls.selfSigned.enabled (eval only — not for production) - server-certificate.yaml template for cert-manager mode - helm install without a TLS source fails at template render with a pointer to docs/tls.md CI - .github/workflows/ci.yml Helm Chart Validation step renders the chart in both existingSecret and cert-manager modes, plus an inverse guard-regression test that asserts helm template MUST refuse to render when no TLS source is configured. Previously the single `helm template` invocation hit the certctl.tls.required fail-loud guard and exit-1'd CI. Four invocations now: lint (existingSecret), template (existingSecret), template (cert-manager), template (no args — must fail). Integration tests - deploy/test/integration_test.go stands up the Compose stack over HTTPS, extracts the CA bundle, and exercises every certctl API over https://localhost:8443 - All 34 integration subtests green (per Phase 8 local CI-parity) Documentation - New: docs/tls.md (provisioning patterns, rotation, SIGHUP reload) - New: docs/upgrade-to-tls.md (one-step cutover, no-downgrade warnings, fleet-roll sequencing) - CHANGELOG.md: v2.2.0 "HTTPS Everywhere — The Irony" entry (file heading unchanged; release tag is v2.0.47) - All curls in docs/, examples/, deploy/helm/ guides use https://localhost:8443 --cacert Verification - grep -rn "ListenAndServe[^T]" cmd/ internal/ → 0 hits - grep -rn "\"http://" cmd/ internal/ → 2 benign hits (Caddy admin API default, SSRF doc comment) — zero certctl endpoints - Tasks #197–#206 (Phases 0–8) all closed in the tracker Files: 65 changed, 3489 insertions, 372 deletions (pre-CI-fix). v2.0.47	2026-04-20 03:43:10 +00:00
shankar0123	04c7eca615	docs: reconcile scheduler topology across sibling docs (7 → 12 loops) ... Authoritative 12-loop table lives at docs/architecture.md:522-534 (committed via the I-001/I-003/I-005 + M48/M50 milestone commits). This change brings six sibling docs into parity with that table so every surface — user-facing features reference, SOC 2 compliance mapping, connectors guide, advanced demo architecture diagram, testing guide, and in-line architecture prose — reflects the same 8 always-on + 4 opt-in topology. Touches: - docs/architecture.md: 2 inline ordinal references (9th / 8th loop) replaced with descriptive names (opt-in cloud discovery / opt-in endpoint health), cross-linked to the authoritative table to prevent future ordinal rot. - docs/features.md: metric row (7 → 12), inline reference to 9th loop, and full scheduler table expanded to include Always-on column + env vars + I-001/I-003/I-005 refs. - docs/compliance-soc2.md: background scheduler monitoring bullets expanded to list all 12 loops with env vars + I-series refs; table row updated with 8 always-on + 4 opt-in summary. - docs/connectors.md: three inline ordinals (7th/6th/9th loop) replaced with descriptive names, cross-linked to architecture.md. - docs/demo-advanced.md: Mermaid SCHED node label updated from '7 background loops' to '12 background loops (8 always-on + 4 opt-in)'. - docs/testing-guide.md: Test 20.1.1 header + grep pattern expanded to include job-retry / job-timeout / notification-retry / digest / endpoint-health / cloud-discovery loops; sign-off chart row label updated. Pure documentation reconciliation. No code changes. Master HEAD pre-commit: 6e646e0.	2026-04-20 02:51:34 +00:00
shankar0123	6e646e0fe8	M-001/M-006: strip HTTP auth from EST/SCEP + fail-loud SCEP preflight ... Closes CWE-306 (missing authentication for critical function) for SCEP via a fail-loud startup gate, and aligns EST/SCEP HTTP dispatch with their respective RFCs. CRL/OCSP remain unauthenticated under .well-known/pki/* per RFC 5280 §5 / RFC 6960 / RFC 8615. Option (D): no mTLS in this milestone. - RFC 7030 §3.2.3 (EST auth is deployment-specific) and §4.1.1 (/cacerts explicitly anonymous): EST paths served unauthenticated; CSR-signature + profile policy enforce identity inside ESTService. - RFC 8894 §3.2: SCEP authenticates via the challengePassword PKCS#10 attribute (OID 1.2.840.113549.1.9.7), not an HTTP credential. HTTP dispatch is unauthenticated; preflightSCEPChallengePassword refuses to start when CERTCTL_SCEP_ENABLED=true without CERTCTL_SCEP_CHALLENGE_PASSWORD. SCEPService.PKCSReq enforces the same invariant defense-in-depth and compares with crypto/subtle.ConstantTimeCompare. cmd/server/main.go: - Extract buildFinalHandler(apiHandler, noAuthHandler, webDir, dashboardEnabled); route /.well-known/est/*, /scep, /scep/*, /.well-known/pki/crl/{id}, /.well-known/pki/ocsp/{id}/{serial}, and health probes through noAuthHandler (RequestID + structuredLogger + Recovery only). - Add preflightSCEPChallengePassword fail-loud gate; startup log emits challenge_password_set boolean for operator visibility. cmd/server/finalhandler_test.go (new, 314 lines, 27 subtests): - TestBuildFinalHandler_Dispatch (20) + TestBuildFinalHandler_NoDashboard (7) pin the dispatch surface: EST 4-endpoint, SCEP exact + trailing-slash + query-string, PKI CRL+OCSP, health, /api/v1/* authenticated, /assets/* file server, SPA fallback. internal/api/router/router.go, internal/config/config.go: - Router-level comments explain why EST/SCEP/PKI dispatchers sit outside the authenticated mux; SCEP challenge password config plumbed through. docs/architecture.md: - New EST Authentication subsection (RFC 7030 §3.2.3 + §4.1.1, buildFinalHandler + noAuthHandler references). - Rewrite SCEP Authentication subsection; replaces pre-existing factually-incorrect "any value accepted" claim with CWE-306 preflight, service-layer defense-in-depth, and crypto/subtle.ConstantTimeCompare. - Top-level Authentication section: qualify /api/v1/* scope on API clients bullet; add standards-based-endpoints bullet referencing the 27-subtest regression harness. docs/compliance-soc2.md: - CC6.1: scope API Key Authentication to /api/v1/*; add standards-based endpoints bullet citing RFCs and CWE-306 closure. - CC6.3: scope API Key Policy to /api/v1/* with cross-reference to CC6.1. - Evidence Locations augmented with buildFinalHandler, preflightSCEPChallengePassword, scep.go defense path, regression harness, and OpenAPI security:[] overrides. api/openapi.yaml: verified already correct (global bearerAuth default overridden with security:[] on /cacerts, /simpleenroll, /simplereenroll, /csrattrs, /scep GET+POST, /crl/{issuer_id}, /ocsp/{issuer_id}/{serial}); no edits needed.	2026-04-19 17:20:05 +00:00
shankar0123	675b87ba63	I-005: notification retry loop + dead-letter queue ... Critical alerts can no longer be silently dropped by a transient notifier failure. Failed notification attempts now ride an exponential backoff retry loop, with a 5-attempt budget before promotion to the dead-letter queue for operator intervention. Schema (migration 000016, idempotent): - retry_count INTEGER NOT NULL DEFAULT 0 - next_retry_at TIMESTAMPTZ - last_error TEXT - idx_notification_events_retry_sweep partial index (next_retry_at) WHERE status='failed' AND next_retry_at IS NOT NULL Dead rows clear next_retry_at so the index stops matching them. Service contract: - NotificationService.RetryFailedNotifications drives 2^n-minute exponential backoff capped at 1h (notifRetryBackoffCap) with 5-attempt budget (notifRetryMaxAttempts). - Exhaustion (RetryCount >= notifRetryMaxAttempts-1) promotes to status='dead' via MarkAsDead. - Non-terminal failures record via RecordFailedAttempt. - Success path promotes to 'sent' without touching retry_count (audit preserves "delivered on attempt N"). - Missing-notifier branch defensively promotes to 'sent' to avoid wedging a row on a deleted channel. - RequeueNotification operator escape hatch atomically resets retry_count -> 0, next_retry_at -> NULL, last_error -> NULL, status -> pending via notifRepo.Requeue. Scheduler: - New always-on notificationRetryLoop wired into the base loop set at CERTCTL_NOTIFICATION_RETRY_INTERVAL (default 2m). - sync/atomic.Bool idempotency guard. - sync.WaitGroup shutdown drain via WaitForCompletion. StatsService: - SetNotifRepo setter pattern preserves 9 pre-existing NewStatsService call sites (main.go + stats_test.go + 8 digest tests) without touching the constructor signature. - DashboardSummary.NotificationsDead populated via notifRepo.CountByStatus(ctx, "dead") — nil-safe when unwired (reports zero on systems without a notification repository). - CountByStatus error is non-fatal (dashboard summary is best-effort for this field). - Prometheus certctl_notification_dead_total counter emitted from the same snapshot. Handler: - New POST /api/v1/notifications/{id}/requeue endpoint. - dead status surfaces to MCP + CLI. Frontend: - NotificationsPage gains two-tab toolbar ("All" / "Dead letter") with queryKey: ['notifications', activeTab] so switching tabs doesn't serve stale data until the 30s refetch. - Dead rows surface "Retry {n}/5" + truncated last_error with full-text title tooltip. - Requeue mutation wrapped as mutationFn: (id: string) => requeueNotification(id) to prevent react-query v5's positional context argument from leaking into the API client — pinned against future refactors by strict-match toHaveBeenCalledWith('notif-dead-001') in NotificationsPage.test.tsx:181. Closes I-005.	2026-04-19 15:17:27 +00:00
shankar0123	707d8de4fb	UX-001: sidebar re-entry + inline team/owner creation in wizard ... Closes UX-001 (OnboardingWizard CertificateStep dead-end): users no longer have to navigate away from the wizard and lose their in-flight state when the required Owner/Team dropdowns are empty. Layout.tsx - Adds persistent 'Setup guide' button in the left sidebar. - Clears localStorage 'certctl:onboarding-dismissed' then navigates to /?onboarding=1 as a re-entry signal that overrides dismissal. - localStorage.removeItem wrapped in try/catch to tolerate storage access errors (private browsing, quota, etc.). DashboardPage.tsx - Reads ?onboarding=1 via useSearchParams as a forceOnboarding flag. - forceOnboarding bypasses the latched first-run gate so the wizard reopens even after dismissal or with certs/issuers already present. - onDismiss now also strips ?onboarding=1 via setSearchParams(next, { replace: true }) so a page refresh does not relaunch the wizard. OnboardingWizard.tsx - Adds CreateTeamModalInline and CreateOwnerModalInline inside CertificateStep. Both wire through React Query: createTeam / createOwner mutation on success invalidates ['teams'] / ['owners'] and calls onCreated(id) so the parent select auto-selects the new row as soon as the refetch lands. - '+ New team' and '+ New owner' buttons placed next to the select labels; empty-state copy replaced with inline 'create one now' buttons (no more Link back to /owners /teams). - CreateOwner coerces empty teamId to undefined before mutation so the server contract matches OwnersPage. Tests (12 new, all green; total suite 252 passed / 0 failed): - Layout.test.tsx (4): Setup guide button renders, clicking it clears the dismissal key and navigates to /?onboarding=1, tolerates localStorage.removeItem throwing. - DashboardPage.test.tsx (4): first-run auto-open, ?onboarding=1 re-entry after dismissal, onDismiss writes localStorage + strips the query param, dismissed-with-no-param stays closed. - OnboardingWizard.test.tsx (4): Skip-Skip reaches CertificateStep with '+ New team' / '+ New owner' buttons visible; '+ New team' happy path with React Query invalidation + parent-select auto-select via option-parent traversal (label is a sibling, not htmlFor-linked); '+ New owner' happy path pins team_id: undefined coercion; Cancel abort never mutates. Test infrastructure notes: - Closure-driven vi.fn().mockImplementation pattern drives the post-invalidation refetch: the mutation mock mutates a closure variable that the getTeams/getOwners mock reads, so the parent select's new <option> exists by the time the refetch lands. - Anchored regex (/^Create Team$/, /^Create Owner$/) disambiguates the modal submit from the '+ New team' / '+ New owner' triggers. Verification gates (all green): - vitest run: 252 passed / 0 failed (8 files, 13.98s) - tsc --noEmit: 0 errors - vite build: clean production bundle (851.77 kB js / 226.81 kB gzip) No new runtime dependencies. Frontend-only change. v2.0.46	2026-04-19 14:49:04 +00:00
shankar0123	0725713e19	Close I-004 (agent hard-delete cascades targets) coverage-gap finding ... Operator decision answered as full soft-delete with optional forced cascade — hard-delete is not reachable from any public surface. Prior to this commit, DELETE /agents/{id} ran a plain `DELETE FROM agents` whose schema-level `ON DELETE CASCADE` on deployment_targets.agent_id silently wiped every target, orphaning certs and aborting in-flight jobs. The finding closure reshapes the agent-removal contract around soft retirement with explicit preflight counts, an opt-in cascade gated by a mandatory reason, and unconditional protection for the four reserved sentinel agents used by discovery sources. Schema — migration 000015: migrations/000015_agent_retire.up.sql flips deployment_targets_agent_id_fkey from ON DELETE CASCADE to ON DELETE RESTRICT, so a stray `DELETE FROM agents` now errors at the DB boundary instead of quietly destroying targets. Both `agents` and `deployment_targets` grow a retired_at TIMESTAMPTZ + retired_reason TEXT pair (TEXT not VARCHAR so operator comments are never truncated), indexed via partial indexes WHERE retired_at IS NOT NULL. The migration is self-healing (ADD COLUMN IF NOT EXISTS, DROP CONSTRAINT IF EXISTS then ADD CONSTRAINT, CREATE INDEX IF NOT EXISTS) so repeated runs against partially-migrated databases converge. migrations/000015_agent_retire.down.sql restores CASCADE and drops the new columns for clean rollback. A dedicated repository-layer testcontainers test (internal/repository/postgres/migration_000015_test.go) asserts the before/after FK action, column presence, index presence, and round-trip idempotency under up→down→up. Domain — sentinel guard + dependency counts: internal/domain/connector.go gains IsRetired() on Agent, the exported SentinelAgentIDs slice listing server-scanner, cloud-aws-sm, cloud-azure-kv, cloud-gcp-sm verbatim (matching the four reserved IDs documented in CLAUDE.md and created at startup in cmd/server/main.go), IsSentinelAgent(id string) predicate, AgentDependencyCounts{ActiveTargets, ActiveCertificates, PendingJobs} with a HasDependencies() method, and ActorTypeAgent / ActorTypeSystem enum values used by audit emission downstream. Coverage locked down by internal/domain/connector_test.go. Service — 8-step ordered contract: internal/service/agent_retire.go:RetireAgent(ctx, id, actor, opts{Force, Reason}) enforces a fixed execution order: (1) sentinel guard — IsSentinelAgent(id) returns ErrAgentIsSentinel unconditionally; force=true does NOT bypass it. (2) fetch — ErrAgentNotFound on miss. (3) idempotency — if IsRetired() already, return AgentRetirementResult{AlreadyRetired: true} with no new audit event and no state change (safe to replay from flaky clients). (4) preflight counts — collectAgentDependencyCounts runs ActiveTargets, ActiveCertificates, PendingJobs sequentially (not in parallel; keeps the per-query timeout predictable and matches the repo's existing call-chain shape). (5) force-reason guard — opts.Force=true with empty Reason returns ErrForceReasonRequired (wired into the 400 status surface). (6) dependency guard — HasDependencies() with opts.Force=false returns BlockedByDependenciesError{Counts} (wired into the 409 body with per-bucket counts). (7) mutation — single pinned retiredAt := time.Now(); agent retirement first, then cascade target retirement if opts.Force, all under the repo's single transaction so the two retired_at stamps match to the second. (8) best-effort audit — agent_retired always; agent_retirement_ cascaded additionally on the force path. Actor is whatever the handler resolves from the request; actor type is mapped by resolveActorType (system/agent-prefix→Agent/else→User). Audit emission failures are logged via slog.Error but do not abort the retirement (matches the house convention used by every other scheduler-emitted event). BlockedByDependenciesError implements Error() as "active_targets=%d, active_certificates=%d, pending_jobs=%d" and Unwrap() → ErrBlockedByDependencies. The single struct satisfies errors.Is via Unwrap (used by scheduler-level tests) and errors.As via the concrete type (used by the handler to fish out Counts for the 409 body). ListRetiredAgents(page, perPage) adds a separate paginated accessor with page<1→1 and perPage<1→50 normalization so retired rows are queryable without polluting the default agent listing. Sentinel guard coverage is asymmetric by design: all four reserved IDs are protected, and force=true cannot override. Regression tests in internal/service/agent_retire_test.go assert each of the eight steps in order, plus sentinel bypass attempts and idempotency replay. Handler + router — status-code surface: internal/api/handler/agents.go:RetireAgent exposes seven status codes on DELETE /agents/{id}: 200 on a fresh retirement (body echoes AgentRetirementResult). 204 on idempotent replay (AlreadyRetired=true; no new audit). 400 on ErrForceReasonRequired. 403 on ErrAgentIsSentinel. 404 on ErrAgentNotFound. 409 on BlockedByDependenciesError, with a custom body shape {error, counts{active_targets, active_certificates, pending_jobs}} that bypasses the default ErrorWithRequestID envelope so callers get the per-bucket numbers directly. 500 on any other error. Heartbeat HandleHeartbeat returns 410 Gone when the agent is retired (ErrAgentRetired), signalling the agent to shut down. Query params `force=true` and `reason=<text>` drive the cascade path; both are forwarded as url.Values through the new MCP transport. internal/api/router/router.go registers GET /api/v1/agents/retired literal-path BEFORE /api/v1/agents/{id} — Go 1.22 ServeMux's literal-beats-pattern-var precedence routes "retired" to the paginated retired-agents listing instead of fetching a hypothetical agent named "retired". Agent binary — clean shutdown on 410: cmd/agent/main.go gains the ErrAgentRetired sentinel, a retiredOnce sync.Once, and a retiredSignal chan struct{}. A markRetired(source, statusCode, body) helper closes the channel exactly once; the Run() select loop observes the close and returns ErrAgentRetired; main() matches via errors.Is(err, ErrAgentRetired) and exits cleanly instead of spinning in the heartbeat retry loop. The 410 Gone surface is therefore terminal for the agent process. MCP transport: internal/mcp/client.go adds Client.DeleteWithQuery(path, query), a new additive transport method. Client.Delete is path-only; without this method the retire tool would silently drop `force` and `reason`, turning every cascade retire into a default soft-retire. The new method shares do()'s 204 normalization and 4xx/5xx error propagation so tool authors get one contract. internal/mcp/tools.go + internal/mcp/types.go expose the retire_agent tool with Force+Reason inputs wired through DeleteWithQuery. CLI: cmd/cli/main.go + internal/cli/client.go add two CLI surfaces: `agents list --retired` (client-side strip of --retired then delegation to ListRetiredAgents, sharing --page/--per-page parsing with the default listing) and `agents retire <id> [--force --reason "…"]` (mirrors ErrForceReasonRequired — force without reason is rejected client-side before the request is sent). JSON + table output modes both honor the new columns. Frontend: web/src/pages/AgentsPage.tsx surfaces retired/retire affordances. web/src/api/client.ts + web/src/api/types.ts expose the retire endpoint and the retired-listing. 4 new Vitest regression cases. OpenAPI: api/openapi.yaml documents DELETE /agents/{id} with all seven status codes, 410 on heartbeat, and the 409 per-bucket body shape. Regression coverage (six new test files, all green): internal/service/agent_retire_test.go — 8-step contract + sentinel guards internal/api/handler/agent_retire_handler_test.go — 7-status-code surface + 410 heartbeat internal/mcp/retire_agent_test.go — DeleteWithQuery wire-through internal/cli/agent_retire_test.go — --retired listing + --force/--reason pairing internal/repository/postgres/migration_000015_test.go — FK flip + columns + indexes + up↔down internal/domain/connector_test.go — IsRetired, IsSentinelAgent, SentinelAgentIDs, HasDependencies Files: api/openapi.yaml — DELETE + 410 + 409 body shape cmd/agent/main.go — ErrAgentRetired, markRetired, retiredSignal cmd/cli/main.go — handleAgents list/get/retire dispatch docs/architecture.md, docs/concepts.md, docs/testing-guide.md — retirement contract narrative internal/api/handler/agents.go — RetireAgent, status surface, 410 on heartbeat internal/api/handler/agent_handler_test.go — extended coverage internal/api/handler/agent_retire_handler_test.go — new internal/api/router/router.go — /agents/retired before /agents/{id} internal/cli/agent_retire_test.go — new internal/cli/client.go — ListRetiredAgents + RetireAgent internal/domain/connector.go — IsRetired, SentinelAgentIDs, IsSentinelAgent, AgentDependencyCounts, ActorTypeAgent/System internal/domain/connector_test.go — new internal/integration/lifecycle_test.go — retirement fixture internal/mcp/client.go — DeleteWithQuery additive transport internal/mcp/retire_agent_test.go — new internal/mcp/tools.go, internal/mcp/types.go — retire_agent tool + Force/Reason inputs internal/repository/interfaces.go — AgentRepository retirement methods internal/repository/postgres/agent.go — retire + cascade target retire + counts internal/repository/postgres/migration_000015_test.go — new internal/service/agent.go — wire into AgentService surface internal/service/agent_retire.go — new 8-step contract internal/service/agent_retire_test.go — new internal/service/deployment.go — skip retired agents internal/service/target.go — skip retired agents internal/service/testutil_test.go — shared mocks extended migrations/000015_agent_retire.up.sql — new migrations/000015_agent_retire.down.sql — new web/src/api/client.ts, types.ts + tests — retire endpoint wiring web/src/pages/AgentsPage.tsx — retire UI	2026-04-19 05:24:00 +00:00
shankar0123	1ee77c89f8	I-003: job timeout reaper closes AwaitingCSR/AwaitingApproval gap ... Add 11th always-on scheduler loop that transitions jobs stuck in AwaitingCSR (default 24h TTL) or AwaitingApproval (default 168h TTL) to Failed. I-001's retry loop then auto-promotes eligible Failed jobs back to Pending. No new status enum, no schema migration. - JobRepository.ListTimedOutAwaitingJobs with per-status cutoff WHERE - JobService.ReapTimedOutJobs mirrors RetryFailedJobs structure - Scheduler jobTimeoutLoop with atomic.Bool idempotency guard, 2m per-tick context, WaitGroup shutdown drain - Config: CERTCTL_JOB_TIMEOUT_INTERVAL (10m), CERTCTL_JOB_AWAITING_CSR_TIMEOUT (24h), CERTCTL_JOB_AWAITING_APPROVAL_TIMEOUT (168h) - Audit event per transition: actor=system, actorType=System, action=job_timeout, details={old_status, new_status, timeout_reason, age_hours} - 14 new tests: 3 config, 7 service, 4 scheduler	2026-04-19 01:37:18 +00:00
shankar0123	4bc8b3e723	fix(config): add RetryInterval to TestValidate_ValidConfig + TestValidate_AuthTypeNone fixtures (I-001 follow-up) ... Problem: TestValidate_ValidConfig and TestValidate_AuthTypeNone construct a SchedulerConfig without RetryInterval, so Validate() fails the 'retry interval must be at least 1 second' check at config.go:1086 with 'retry interval must be at least 1 second'. Both tests expect success, so they fail whenever run. Root cause (re-derived from source, not inherited from memory): git log -S 'retry interval must be at least' --source --all shows the validation was introduced in 0200c7f (I-001, RetryFailedJobs scheduler wiring). git log -- internal/config/config_test.go shows the test file was last touched in 7382e5f, which predates 0200c7f. I-001 added a new Validate() rule without updating the two positive test fixtures — a gap in I-001's verification pass. This is NOT C-001 fallout. The config_test.go file was untouched by the C-001 closure commits 91642e2 and 4696116. The failure surfaced during the full test suite run after C-001 landed because no one had run 'go test ./internal/config/...' since I-001. Scope: - internal/config/config_test.go (2 fixtures: TestValidate_ValidConfig, TestValidate_AuthTypeNone). Implementation: Added 'RetryInterval: 5 * time.Minute' to both SchedulerConfig literals. 5 minutes matches the I-001 default at config.go:818: RetryInterval: getEnvDuration("CERTCTL_SCHEDULER_RETRY_INTERVAL", 5*time.Minute) The other two TestValidate_* tests (InvalidAuthType, APIKeyAuth_ MissingSecret) are unaffected because they expect Validate() to error at the auth-type check (line 1052) or auth-secret check (line 1057), both of which fire before the RetryInterval check at line 1086. Verification: - go test -count=1 -run 'TestValidate_' ./internal/config/...: PASS - go test -short -count=1 ./...: all packages PASS - go vet ./...: exit 0 Residual: None. This is a pure test-fixture fix — production code is unchanged. Commit: 0200c7f (I-001) should have included this edit. Attributed here for traceability. v2.0.45	2026-04-19 00:33:22 +00:00
shankar0123	469611650c	fix(cli): add missing os + path/filepath imports to client_test.go ... Follow-up to 91642e2. TestClient_ImportCertificates_SixFieldPayload uses filepath.Join(t.TempDir(), ...) and os.WriteFile to stage a test PEM, but the import block only listed encoding/json, encoding/pem, net/http, etc. — neither os nor path/filepath was imported. go vet rejected the package with 'undefined: filepath' (and would have caught 'undefined: os' next). Add both imports. No behavioral change — the referenced symbols are the standard library's usual names for their respective packages, so the test compiles and runs exactly as intended. CI should now pass go build + go vet on the cli package.	2026-04-19 00:27:11 +00:00
shankar0123	91642e2860	C-001 scope expansion: tighten parallel POST /api/v1/certificates call sites to six-field contract ... Problem: a53a4b8 closed C-001 at the handler boundary by tightening the ValidateRequired contract on POST /api/v1/certificates to require six fields: name, common_name, renewal_policy_id, issuer_id, owner_id, team_id. (Correction re-derived from source: the handler ValidateRequired calls on owner_id/team_id/renewal_policy_id were actually installed in 3287e17 under M-002/M-003/M-006 auth unification — a53a4b8's commit message overstates scope.) Post-audit on 2026-04-18 found three parallel call sites still shipping three-to-four-field payloads that the newly strict handler would reject with HTTP 400: - GUI: OnboardingWizard CertificateStep (common_name + sans + issuer_id + environment only) - CLI: certctl-cli import (common_name + issuer_id + status only; no required-flag gating) - Tests: deploy/test/qa_test.go Part03 positive paths Scope: Bring every POST /api/v1/certificates caller to six-field parity. No handler changes — the contract is authoritative; the callers must conform. Implementation: GUI — OnboardingWizard CertificateStep expansion: web/src/pages/OnboardingWizard.tsx adds name/owner_id/team_id/ renewal_policy_id state. React Query hooks for getOwners/ getTeams/getPolicies use per_page: '500' to populate dropdowns without pagination-driven truncation. Payload ships all six required fields plus sans/certificate_profile_id/environment. nextDisabled gate enforces all six before the Continue button activates. CLI — ImportCertificates rewrite: internal/cli/client.go rewrites ImportCertificates with flag.NewFlagSet("import", flag.ContinueOnError). Required flags: --owner-id, --team-id, --renewal-policy-id, --issuer-id. Optional: --name-template (default {cn}, templated via strings.ReplaceAll against cert.Subject.CommonName), --environment (default imported). Missing required flags fail pre-HTTP with a clear error. Request map ships all six required fields plus sans/ environment/status/optional serial_number. cmd/cli/main.go — usage string updated to document the new required/optional flags. Tests — qa_test.go Part03 positive paths: deploy/test/qa_test.go Part03 Create_Minimal and Create_Full updated to include all six fields. Uses seed_demo.sql-supplied IDs (o-alice, t-platform, rp-standard) — docker-compose.demo.yml is the run context. C-001 explanatory comment added above Create_Minimal so future readers understand why the minimal payload is no longer minimal. MCP parity: Verified no-op. internal/mcp/types.go:28 CreateCertificateInput already declares all six fields; internal/mcp/tools.go:102 forwards the typed struct unchanged. Verification: Go CLI regression tests (internal/cli/client_test.go): * TestClient_ImportCertificates_MissingRequiredFlags — 5 subtests, one per missing required flag, confirms flag.ContinueOnError rejects with non-nil error before any HTTP call is attempted. * TestClient_ImportCertificates_MissingPositionalArgs — confirms the "usage: import <file>" error path when no PEM file is supplied after the flags. * TestClient_ImportCertificates_SixFieldPayload — uses httptest to decode the POST body and assert all six required fields plus sans/environment are present on the wire. Frontend regression test (web/src/api/client.test.ts): 'createCertificate accepts and transmits all six required fields' pins the wire shape for both GUI call sites (OnboardingWizard CertificateStep + CertificatesPage CreateCertificateModal). If either UI surface accidentally drops a field, this assertion fails in CI rather than surfacing as a 400 at runtime. Grep-based call-site sweep: Enumerated every POST /api/v1/certificates create caller. Four total: OnboardingWizard, CertificatesPage, MCP tools, CLI import. All four now ship six-field payloads. Claim path (internal/service/discovery.go) updates existing rows and does not POST. EST/SCEP handlers invoke internal certService.CreateVersion, not the public API. Negative-path tests (qa_test.go:1085/1267/1274/1288/1298) remain valid: they assert 400/non-500 on oversized/malformed/missing-CN/UTF-8/empty bodies, and these properties still hold under the stricter handler. Static gates: go build ./..., go vet ./..., go test ./internal/cli/..., and cd web && npm run test deferred to operator pre-push — the Go toolchain is not available in the session sandbox. Grep-based verification confirms the syntactic shape of every changed file. Residual: None. Every POST /api/v1/certificates call site now conforms to the six-field contract; the wire shape is pinned by both Go and TypeScript regression tests. Commit: TBD-SHA (audit doc + CLAUDE.md carry TBD-SHA placeholders to be amended after commit)	2026-04-19 00:25:10 +00:00
shankar0123	0200c7f4a4	Close I-001 (RetryFailedJobs never invoked) coverage-gap finding ... Operator decision answered as Option A: JobService.RetryFailedJobs is now wired into the scheduler as an always-on 10th loop. Prior to this commit the method was implemented, unit-tested, and exported but had zero runtime callers — any job that transitioned to status=Failed stayed Failed forever regardless of how many attempts it had remaining. Scheduler — 10th loop: internal/scheduler/scheduler.go grows a jobRetryLoop alongside the existing nine loops (renewal, jobs, health, notifications, short-lived, network scan, digest, health check, cloud discovery). The loop follows the established run-immediately-then-tick pattern (same shape as jobProcessorLoop), gated by a sync/atomic.Bool idempotency guard and joined into the scheduler's sync.WaitGroup so WaitForCompletion drains it on graceful shutdown. Each tick runs under a 2-minute context timeout mirroring jobProcessorLoop's opCtx budget. The runJobRetry helper invokes jobService.RetryFailedJobs(ctx, 3) — the advisory maxRetries cap is belt-and-suspenders; per-job eligibility is still enforced inside the service via Attempts < MaxAttempts. The JobServicer scheduler-interface gains RetryFailedJobs so the scheduler's dependency surface stays explicit and mockable. Service — audit trail per retry: internal/service/job.go:RetryFailedJobs now emits an audit event for every Failed→Pending transition. Following the house convention used by all scheduler-emitted events, actor='system' and actorType= domain.ActorTypeSystem; action='job_retry'; details capture old_status, new_status, attempts, max_attempts. JobService carries an optional *AuditService (SetAuditService) that nil-guards to preserve test-wiring ergonomics — existing tests that construct JobService without an audit service continue to pass unchanged. Config — env var with sane default: internal/config/config.go:SchedulerConfig grows RetryInterval, wired to CERTCTL_SCHEDULER_RETRY_INTERVAL with a 5-minute default. Validate rejects intervals below 1 second (matches other scheduler interval validators). Server wiring: cmd/server/main.go calls jobService.SetAuditService(auditService) after JobService construction and sched.SetJobRetryInterval( cfg.Scheduler.RetryInterval) alongside the other SetXxxInterval calls. Regression coverage: internal/service/job_test.go (3 new) - TestJobService_RetryFailedJobs_EligibleJobTransitionsAndAudits - TestJobService_RetryFailedJobs_SkipsJobsAtMaxAttempts - TestJobService_RetryFailedJobs_NoAuditServiceOK internal/scheduler/scheduler_test.go (3 new) - TestScheduler_JobRetryLoop_CallsService - TestScheduler_JobRetryLoop_IdempotencyGuard - TestScheduler_JobRetryLoop_WaitForCompletion The service tests assert status transitions, attempt-cap short- circuiting, and audit event shape (actor='system', action='job_retry', details keys). The scheduler tests assert the loop invokes the service, the atomic.Bool guard skips overlapping ticks with the expected 'still running, skipping tick' log, and WaitForCompletion drains the in-flight tick on Stop. Residual follow-up (not in scope for this commit): internal/service/renewal.go:RetryFailedJobs is a parallel dead-code duplicate of the same logic on RenewalService — untested and has no runtime caller. The audit finding called this out as 'implemented twice'. Removing it is a separate cleanup and does not block the Option-A wiring this commit delivers. Files: cmd/server/main.go — SetAuditService + SetJobRetryInterval internal/config/config.go — RetryInterval field + env + validate internal/scheduler/scheduler.go — 10th loop, interface, field, setter internal/scheduler/scheduler_test.go — 3 new scheduler-loop tests internal/service/job.go — RetryFailedJobs audit emission + SetAuditService internal/service/job_test.go — 3 new service-layer tests	2026-04-18 23:24:54 +00:00
shankar0123	fe7e766510	Close M-004 (OCSP issuer binding) and M-005 (discovery actor propagation) coverage-gap findings ... M-004 — OCSP issuer binding (composite key): The OCSP lookup path now binds (issuer_id, serial) as a composite key rather than resolving by serial alone. CertificateRepository and RevocationRepository gain GetByIssuerAndSerial methods; ca_operations.go scopes both lookups by the issuer_id path param. When no managed cert binds to that (issuer, serial) tuple, GetOCSPResponse constructs an RFC 6960 §2.2 'unknown' response (CertStatus=2) instead of the prior default 'good'. Short-lived cert exemption (profile TTL < 1h) is preserved. Real repo errors (non-sql.ErrNoRows) fail closed with a log. Regression coverage: internal/service/ca_operations_test.go - TestCAOperationsSvc_GetOCSPResponse_Unknown_CrossIssuer - TestCAOperationsSvc_GetOCSPResponse_Unknown_UnknownSerial M-005 — Discovery Claim/Dismiss actor propagation: DiscoveryService.ClaimDiscovered and DismissDiscovered now accept an explicit 'actor string' parameter (propagation pattern mirrors bulk_revocation.go / revocation_svc.go). The handler layer passes resolveActor(r.Context()) — the named-key identity established by the M-002 auth unification — and the service falls back to 'api' (the same safe sentinel resolveActor uses when no auth context is present) only when the caller passes an empty string. Never falls back to 'operator'. Regression coverage: internal/service/discovery_test.go - TestDiscoveryService_ClaimDiscovered_AuditActor - TestDiscoveryService_DismissDiscovered_AuditActor - TestDiscoveryService_ClaimDiscovered_EmptyActorFallsBackToAPI - TestDiscoveryService_DismissDiscovered_EmptyActorFallsBackToAPI Each new test asserts event.Actor matches the caller-supplied string (or 'api' on empty input) and explicitly asserts event.Actor != 'operator' to lock in the historical fix intent. Files: internal/api/handler/discovery.go — pass resolveActor(ctx) internal/api/handler/discovery_handler_test.go — updated call sites internal/integration/lifecycle_test.go — updated mock wiring internal/repository/interfaces.go — GetByIssuerAndSerial on CertificateRepository + RevocationRepository internal/repository/postgres/certificate.go — composite key lookup internal/service/ca_operations.go — (issuer_id, serial) scoping internal/service/ca_operations_test.go — 2 new M-004 tests internal/service/discovery.go — actor parameter + 'api' fallback internal/service/discovery_test.go — 4 new M-005 tests internal/service/shortlived_test.go — mock signature update internal/service/testutil_test.go — mock GetByIssuerAndSerial	2026-04-18 22:20:25 +00:00
shankar0123	ff7357f889	fix(lint): godoc comment on NewAuthWithNamedKeys must lead with function name (ST1020) ... CI failure on master (commit 3287e17) — staticcheck ST1020: internal/api/middleware/middleware.go:125:1: ST1020: comment on exported function NewAuthWithNamedKeys should be of the form "NewAuthWithNamedKeys ..." (staticcheck) When NewAuth was renamed to NewAuthWithNamedKeys during the M-002 auth unification, the leading godoc sentence was left pointing at the old name. Rewrite the comment so its first sentence starts with the new function name, and expand the body to describe the named-key + admin-flag contract introduced in 3287e17. Also gitignore /.gopath/ — session-scoped tool install cache, same category as /.gocache/ and /.gomodcache/. Verification: go vet ./internal/api/middleware/... — clean go build ./internal/api/middleware/... — clean go test ./internal/api/middleware/... — PASS (0.245s) staticcheck -checks=all,<project exclusions> — clean across middleware, handler, service, domain, cmd/server, scheduler Closes: CI failure on 3287e17.	2026-04-18 21:38:46 +00:00
shankar0123	3287e174dc	Unify API auth + RFC-compliant CRL/OCSP (M-002 + M-003 + M-006, auto-closes M-001) ... Closes the remaining P1 gaps from coverage-gap-audit.md (M-001/M-002/M-003/M-006) on top of the C-001/C-002 ownership + agent-FK contract fixes landed in a53a4b8. The work lands as a single commit spanning server, docs, tests, and the React client. M-002 — Named API keys with per-key actor propagation * Migration 000014 adds the 'api_keys' table (id, name, hash, principal, role, created_at, last_used_at, disabled_at) so every credential carries an identifiable principal instead of the opaque 'anonymous'/'api-key' sentinel. * Auth middleware now rotates through configured keys, performs constant-time hash comparison, stamps 'last_used_at', and emits an actor struct via contextWithActor(). The audit middleware, bulk-revocation handler, approval handlers, and MCP tool layer now read the principal off the context and persist it on every audit_events row. * Regression coverage: - internal/api/middleware/audit_test.go — actor propagation, principal redaction for disabled keys, anonymous fallback for unauthenticated endpoints. - internal/api/handler/bulk_revocation_handler_test.go, job_handler_test.go — principal-on-audit assertions. M-003 — Authorization gates (Phase B) * Approval handler rejects self-approval / self-rejection with 403 when the actor principal equals the job's requested_by field. * Bulk revocation is gated behind the 'admin' role; operators and viewers receive 403. * Regression coverage: - internal/service/job_test.go — TestApproveJob_NotSelf, TestRejectJob_NotSelf. - internal/api/handler/bulk_revocation_handler_test.go — TestBulkRevoke_RequiresAdmin, TestBulkRevoke_AdminSucceeds. M-006 — RFC-compliant CRL/OCSP on the unauthenticated .well-known mux * Per RFC 8615, relying parties cannot reasonably be asked to authenticate against the issuing certctl instance to retrieve revocation material. CRL and OCSP move off the authenticated '/api/v1/crl*' and '/api/v1/ocsp/*' paths onto: GET /.well-known/pki/crl/{issuer_id} Content-Type: application/pkix-crl (RFC 5280 §5) GET /.well-known/pki/ocsp/{issuer_id}/{serial} Content-Type: application/ocsp-response (RFC 6960) * Non-standard JSON CRL shape is removed; only DER is served. * Short-lived certificate exemption (profile TTL < 1h → skip CRL/OCSP) is preserved; the response simply omits the serial. * Routes are registered on the unauthenticated 'finalHandler' mux in cmd/server/main.go alongside EST ('/.well-known/est/*') and SCEP ('/scep'). Legacy authenticated paths return 404. * Regression coverage: - internal/api/handler/certificate_handler_test.go — content type, DER parseability, 404 for unknown issuer. - internal/api/handler/adversarial_path_test.go — unauthenticated access asserted for CRL, OCSP, EST, SCEP. - internal/api/router/router_test.go — route-table assertion that '.well-known/pki/*', '.well-known/est/*', and '/scep' are mounted on the unauthenticated branch. M-001 — Auto-closed by M-002 EST and SCEP were already registered on the unauthenticated 'finalHandler' mux; the router comment at internal/api/router/router.go:247 now matches reality. The adversarial-path tests above lock the behavior in. Verification (all gates green): * go vet ./... — clean * go build ./... — ok * go test -short ./... (55+ packages) — all pass * web/ : npm test (225 Vitest tests) — all pass * web/ : npx tsc --noEmit — clean * grep sweep for '/api/v1/(crl|ocsp)' — 13 surviving hits, all intentional M-006 tombstone/relocation comments. Documentation: * coverage-gap-audit.md — status flips M-001/M-002/M-003/M-006 → Fixed, with per-finding resolution paragraphs citing regression test IDs. (Audit file lives outside this repo; see cowork root.) * CLAUDE.md Project Status line updated with the auth-unification closure note. * docs/features.md, docs/architecture.md, docs/quickstart.md, docs/concepts.md, docs/connectors.md, docs/test-env.md, docs/testing-guide.md, docs/compliance-*.md, docs/demo-advanced.md — refreshed for the new '.well-known/pki/*' namespace and named API keys. * api/openapi.yaml — documents the new unauthenticated endpoints and removes the legacy '/api/v1/crl*' + '/api/v1/ocsp/*' paths. .gitignore: adds '/.gocache/' and '/.gomodcache/' for the session- scoped Go caches so they never enter the tree.	2026-04-18 18:17:41 +00:00
shankar0123	a53a4b845b	fix(gui,api): close C-001 + C-002 — ownership + agent FK contract ... C-001 — CreateCertificate was server-accepted with null owner_id, team_id, renewal_policy_id because the GUI neither collected the fields nor enforced them, even though the backend's ManagedCertificate schema and handler contract treat them as required. Fix the contract at all four layers: - web/src/pages/CertificatesPage.tsx: replace owner_id/team_id free- text inputs with <select> elements fed by getOwners/getTeams/ getPolicies queries; mark all three required; gate the Create button on owner_id + team_id + renewal_policy_id being set. - internal/api/handler/certificates.go: ValidateRequired for owner_id, team_id, renewal_policy_id on CreateCertificate so the handler returns HTTP 400 with the offending field name before the service layer is reached. - internal/mcp/types.go: drop ',omitempty' from CreateCertificateInput.RenewalPolicyID so the MCP schema reflects the required contract; Update inputs keep partial-update semantics. - api/openapi.yaml: 'required: [name, common_name, renewal_policy_id, issuer_id, owner_id, team_id]' was already present on the Create schema; clarified DeploymentTarget.agent_id description to note the FK contract. C-002 — CreateTargetWizard accepted an empty or bogus agent_id and the service inserted directly, producing a Postgres 23503 FK-violation that bubbled out as a generic HTTP 500. The FK itself (migration 000001 line 104: agent_id TEXT NOT NULL REFERENCES agents(id)) is correct; we keep the schema strict and add validation at three layers: - internal/service/target.go: introduce ErrAgentNotFound sentinel and pre-validate agent_id in TargetService.CreateTarget — empty string returns 'agent_id is required'; a nonexistent id returns the full 'referenced agent does not exist: <id>' error. Both wrap ErrAgentNotFound via fmt.Errorf %w so callers can use errors.Is. - internal/api/handler/targets.go: ValidateRequired on agent_id; map errors.Is(err, service.ErrAgentNotFound) to HTTP 400 instead of letting it fall through to the generic 500 branch. - internal/mcp/types.go: drop ',omitempty' from CreateTargetInput.AgentID to match the required contract. - web/src/pages/TargetsPage.tsx: replace the free-text Agent ID input with a <select> populated from getAgents(); include agent in the canProceedToReview gate so Next is disabled until an agent is chosen. Regression coverage (21 new subtests total): - TestCreateCertificate_MissingRequiredField_Returns400 — 6 subtests, one per required field, each proves the handler guard fires before the mock service is called. - TestCreateTarget_MissingAgentID_Returns400 — handler guard. - TestCreateTarget_NonexistentAgent_Returns400 — pins the ErrAgentNotFound -> 400 translation. - TestTargetService_CreateTarget_MissingAgentID — errors.Is sentinel. - TestTargetService_CreateTarget_NonexistentAgentID — errors.Is. - The existing TestTargetService_CreateTarget_Success, along with TestCreateTarget_{MissingName,MissingType,NameTooLong}_* handler tests, were updated to seed a real agent or include agent_id in the request body so the happy paths still run cleanly. Gates (Phase 4): - go build/vet/test/race: green - go test -cover: internal/service 68.7% (gate 55%), internal/api/handler 78.9% (gate 60%) - golangci-lint on service+handler+mcp: 0 issues - govulncheck: no reachable vulns - tsc --noEmit: clean - vitest: 223/223 passing See cowork/certctl-coverage-gap-audit.md entries C-001 and C-002. v2.0.44	2026-04-18 16:01:40 +00:00
shankar0123	9143da5fa8	Merge branch 'fix/d-008-policy-engine-drift'	2026-04-18 14:56:06 +00:00
shankar0123	b3cc7cbdb2	fix(policies): close the D-006 loop — TitleCase seed canonicals + severity-aware, config-consuming rule engine (D-008) ... D-008 was a three-part drift in the policy engine that made the D-005/D-006 remediation cosmetic below the DB layer: (a) migrations/seed.sql INSERTed rules with pre-D-005 lowercase types ('ownership', 'environment', 'lifetime', 'renewal_window') that the handler validator rejects on Create/Update but that raw SQL INSERTs bypassed entirely. At runtime evaluateRule's switch fell through to the default "unknown policy rule type" error branch on every demo rule × every cert × every cycle, flooding logs while emitting zero violations. (b) migrations/seed_demo.sql persisted lowercase severity values ('critical', 'error', 'warning') on policy_violations rows. INSERT succeeded because that column had no CHECK, but any frontend comparing against the canonical PolicySeverity enum mis-categorized every seeded violation. (c) evaluateRule hardcoded Severity: PolicySeverityWarning on every emitted violation and ignored rule.Config entirely — so the D-006 per-rule severity column (000013) and every per-arm Config JSON ({allowed_issuer_ids, allowed_domains, required_keys, allowed, lead_time_days, max_days}) was dead data below the evaluation layer. This commit lands (a)+(b)+(c) atomically. Shipping any subset leaves the feature half-working. ## Changes Domain (internal/domain/policy.go): * Add PolicyTypeCertificateLifetime as the 6th TitleCase canonical. Pre-D-008 the seeded "max-certificate-lifetime" rule had no engine arm — routing it through RenewalLeadTime would conflate "how close to expiry before we renew" with "how long can the cert possibly be", two distinct semantics. The new type accepts config {"max_days": int} and flags certs whose NotAfter - NotBefore exceeds the cap. Handler validator (internal/api/handler/validation.go): * ValidatePolicyType allowlist grown to 6 canonicals (AllowedIssuers, AllowedDomains, RequiredMetadata, AllowedEnvironments, RenewalLeadTime, CertificateLifetime). OpenAPI (api/openapi.yaml): * PolicyType enum grown to match domain. Frontend (web/src/api/types.ts, types.test.ts): * POLICY_TYPES tuple gains CertificateLifetime; pin test asserts all 6 canonicals and rejects casing drift. Migration 000014 (policy_violations severity CHECK): * Named CHECK constraint (policy_violations_severity_check) mirroring 000013's allowlist, defense-in-depth at the DB layer against future drift from bypassed writes (migrations, psql sessions, future callers). Symmetric down migration drops by name. Seed data: * migrations/seed.sql rewritten to emit TitleCase canonicals with per-arm config JSON that actually exercises the config-consuming paths (not the missing-field backstops): - pr-require-owner → RequiredMetadata {"required_keys":["owner"]} Warning - pr-allowed-environments → AllowedEnvironments {"allowed":["production","staging","development"]} Error - pr-max-certificate-lifetime → CertificateLifetime {"max_days":90} Critical - pr-min-renewal-window → RenewalLeadTime {"lead_time_days":14} Warning Severities are now differentiated per rule (D-006 intent). * migrations/seed_demo.sql violation rows flipped to TitleCase severity ('Critical', 'Error', 'Warning') so migration 000014 applies cleanly on upgrade paths. Engine rewrite (internal/service/policy.go): * evaluateRule rewritten. All six arms now: 1. Parse rule.Config into the per-arm typed struct. 2. Bad JSON → log at ValidateCertificate boundary and skip this rule (no co-located poisoning of other rules in the same batch). 3. Empty/null Config → emit the pre-D-008 missing-field violation (backwards compat invariant — operators who haven't reconfigured still see the same output). 4. Violations emitted carry rule.Severity (no more hardcoded Warning); D-006 column is now load-bearing. * CertificateLifetime arm reads NotBefore/NotAfter from the certificate's latest version via CertRepo. Injected via PolicyService.SetCertRepo() setter — avoids churning ~36 NewPolicyService call sites while keeping the lifetime arm optional (degrades to a log+skip if the setter is not wired). Server wiring (cmd/server/main.go): * policyService.SetCertRepo(certRepo) wired after construction. Tests (internal/service/policy_test.go): * 25 new subtests across 5 groups: - TestEvaluateRule_SeverityPassThrough (6): every rule type emits violations carrying rule.Severity, not hardcoded. - TestEvaluateRule_ConfigConsumed (12): every per-arm Config path exercised positive + negative. - TestEvaluateRule_EmptyConfig_BackCompat (3): empty/null Config still emits pre-D-008 missing-field violations. - TestEvaluateRule_BadConfig_SkipsRule: malformed JSON logs and skips cleanly without poisoning neighbors. - TestEvaluateRule_CertificateLifetime_RepoScenarios (3): ok when repo wired, log+skip when not, handles missing NotBefore/NotAfter edges. Provenance: D-008 surfaced during D-005/D-006 remediation review in eef1db0. That commit added persistence and CI pins for the severity field but did not re-verify the evaluation layer consumed it; this finding and fix close the audit-process gap.	2026-04-18 14:55:56 +00:00
shankar0123	eef1db0f0a	fix(policies): stop 400ing the "+ New Policy" button + add per-rule severity (D-005, D-006) ... Coverage Gap Audit findings D-005 (P0) + D-006 (P1) fixed together in a single commit because they share the same root cause — policy CRUD sending values the backend silently rejects — and splitting them would leave a half-working UI between commits. ## D-005 (P0): PoliciesPage dropdown 400s every Create Policy Root cause ---------- `web/src/pages/PoliciesPage.tsx` populated the Type `<select>` from a hardcoded `['key_algorithm', 'ownership', 'allowed_issuers', ...]` array. The backend's `internal/api/handler/validators.go::ValidatePolicyType` enforces the TitleCase allowlist `AllowedIssuers`, `AllowedDomains`, `RequiredMetadata`, `AllowedEnvironments`, `RenewalLeadTime` — defined in `internal/domain/policy.go`. Every Create Policy request was rejected with `400 invalid policy type`. The error surfaced only as a transient toast; the modal closed anyway. Silent user-visible failure. Fix --- - `web/src/api/types.ts`: added `POLICY_TYPES` and `POLICY_SEVERITIES` tuples with `as const` and narrowed `PolicyRule.type`, `.severity`, and `PolicyViolation.severity` to the literal-union types. Dropdown is now sourced from the tuple; casing drift becomes a compile error. - `web/src/pages/PoliciesPage.tsx`: rekeyed `severityStyles` / `severityDots` to the TitleCase values, added `humanize()` for display (AllowedIssuers → "Allowed Issuers"), removed the `badge-neutral` fallback that was papering over the mismatch. - `web/src/api/types.test.ts` (new): pins both tuples exactly. If anyone edits one side of the frontend/backend contract without the other, CI fails with a clear assertion. Pure-TS vitest, no RTL dependency. ## D-006 (P1): `severity` field silently dropped on create/update Root cause ---------- `PolicyRule` had no `Severity` field in `internal/domain/policy.go`. The frontend has always sent `severity` on create/update, but Go's `json.Decoder` (default settings, no `DisallowUnknownFields`) silently dropped it. The value never reached PostgreSQL. Every rule rendered with the same severity because there was no severity — just a display computation downstream. Fix: option (b), full-stack schema add (not delete-the-field) ------------------------------------------------------------- - Migration `000013_policy_rule_severity` (up + down): adds `severity VARCHAR(50) NOT NULL DEFAULT 'Warning'` to `policy_rules` with CHECK constraint `severity IN ('Warning', 'Error', 'Critical')`. No index — three-value column on a low-thousands-rows table, planner will seq-scan regardless. PG 11+ metadata-only ADD COLUMN, safe on live data. - `internal/domain/policy.go`: added `Severity PolicySeverity` field. - `internal/repository/postgres/policy.go`: plumbed `severity` through ListRules SELECT + Scan, GetRule SELECT + Scan, CreateRule INSERT, UpdateRule UPDATE (4 queries). - `internal/service/policy.go::UpdatePolicy`: if the client omits severity on a PUT (zero-value empty string), fetch the existing rule and preserve its severity. Without this, partial updates would trip the NOT NULL CHECK and 500. Preserves pre-existing behavior for Name/Type (out of scope). - `internal/api/handler/policies.go::CreatePolicy`: default empty severity to `'Warning'`, then validate via `ValidatePolicySeverity`. 400 with clear message instead of 500 on CHECK violation. `UpdatePolicy`: validates severity only when provided. - `internal/mcp/types.go` + `internal/mcp/tools.go`: added optional `severity` on the MCP `create_policy` / `update_policy` tool inputs so LLM callers stay in sync with the wire contract. - `api/openapi.yaml`: added `severity` to the `PolicyRule` schema with the enum and default. Acceptance criterion (user-defined) ----------------------------------- "Create a rule with severity=Critical, reload the page, and still see Critical — no silent drops." Verified end-to-end: frontend sends `severity: "Critical"`, handler validates, service persists, DB stores, GET returns, React renders the correct badge. Seed data --------- `migrations/seed.sql`: four demo rules now have differentiated severities — `pr-require-owner` → Warning, `pr-allowed-environments` → Error, `pr-max-certificate-lifetime` → Critical, `pr-min-renewal-window` → Warning. The user called out that seeding all four at the same severity makes the feature look decorative; differentiation demonstrates the column carries real signal. ## Integration test fix (side effect of D-006) `internal/integration/e2e_test.go::TestCrossResourceWorkflow/CreatePolicy` was sending `"severity": "High"` — a value from the pre-audit severity vocabulary that the new `ValidatePolicySeverity` correctly rejects with 400. Changed to `"Error"` (closest semantic match in the new TitleCase allowlist). Only severity reference in the integration/ directory; verified via grep. ## Out of scope, logged for follow-up (d/D-008) Three policy-engine drift issues orthogonal to D-005 + D-006, explicitly deferred per direction: 1. `migrations/seed.sql` policy_rules INSERTs use lowercase TYPE values (`'ownership'`, `'environment'`, `'lifetime'`, `'renewal_window'`). These are load-bearing on `internal/service/policy.go::evaluateRule`'s `switch rule.Type` (which also uses the lowercase strings). Migrating requires coordinated changes across seed + evaluation engine. 2. `migrations/seed_demo.sql:482-483` contains lowercase `'critical'` severity — will now fail the new CHECK constraint. Separate fix. 3. `evaluateRule` hardcodes `Severity: domain.PolicySeverityWarning` on emitted violations and ignores the configured `rule.Config`. The new severity column is read correctly on the CRUD path but not yet consulted during evaluation. ## Verification Backend: - `go build ./...` — clean - `go vet ./...` — clean - `go test -short ./...` — all packages green, including `internal/service` (policy service), `internal/api/handler` (policy + MCP handler tests), `internal/integration` (e2e_test.go after fix), `internal/domain`, `internal/repository/postgres`. Frontend: - `tsc --noEmit` — clean - `vitest run` — 223/223 passing (4 new assertions in types.test.ts) - `vite build` — clean (only the pre-existing chunk-size warning)	2026-04-18 13:02:04 +00:00
shankar0123	72f5246ce3	Merge branch 'fix/m11-cosign-v3-sign-blob-bundle': M-11 cosign v3 sign-blob migration v2.0.43	2026-04-18 09:29:25 +00:00
shankar0123	cb308bb4c7	ci(release): migrate cosign sign-blob to --bundle (cosign v3.0) ... Cosign v3.0 (shipped by default with sigstore/cosign-installer@cad07c2e, release v3.0.5) removed --output-signature and --output-certificate from the sign-blob subcommand. The replacement is a single --bundle flag that emits a unified Sigstore bundle (.sigstore.json) containing the signature, certificate chain, and Rekor inclusion proof in one file. This change migrates both sign-blob invocations in .github/workflows/ release.yml (per-binary matrix signing and aggregate checksums.txt signing), updates the artefact upload paths, the artefact aggregation case filter, the GitHub Release asset list, and the release-notes body verify-blob example. The README cosign verification snippet and sidecar description are also updated to the --bundle / .sigstore.json shape. No cosign version pinning. No legacy fallback. OCI image signing (cosign sign on image digest) is unchanged — only sign-blob flags changed in v3.0. See M-11 in certctl-audit-report.md. Verification gates: - YAML parse: OK - go vet ./...: exit 0 - go build ./...: exit 0 - grep 'cosign sign-blob' release.yml: 2 (expected: 2) - grep '.sigstore.json' release.yml: 9 (expected: >=5) - grep '.sig/.pem' release.yml non-comment: 0 (expected: 0) - README legacy cosign refs: 0 (expected: 0) - docs/ legacy cosign refs: 0 (expected: 0) Coverage: unchanged (CI workflow edit + README — zero Go code touched).	2026-04-18 09:29:20 +00:00
shankar0123	ad93e99158	Merge branch 'fix/m10-openapi-spec-drift': M-10 OpenAPI spec drift reconciliation v2.0.42	2026-04-18 03:21:45 +00:00
shankar0123	9d0c3dfa15	docs(openapi): reconcile api/openapi.yaml with router routes (M-10) ... Add 9 missing operations to api/openapi.yaml that exist in router.go but were absent from the spec. Spec-only change with no runtime Go code changes; all 106 pre-existing operationIds preserved byte-identical. New operationIds: - testTargetConnection (POST /api/v1/targets/{id}/test) - verifyDeployment (POST /api/v1/jobs/{id}/verify) - getJobVerification (GET /api/v1/jobs/{id}/verification) - estCACerts (GET /.well-known/est/cacerts) - estSimpleEnroll (POST /.well-known/est/simpleenroll) - estSimpleReEnroll (POST /.well-known/est/simplereenroll) - estCSRAttrs (GET /.well-known/est/csrattrs) - scepGet (GET /scep) - scepPost (POST /scep) Spec operations: 106 → 115 (matches 115 router routes exactly). Verification: - openapi-spec-validator: OK - go build ./...: clean - go vet ./...: clean - go test -race -count=1 -short ./...: 54 packages ok, 0 FAIL - golangci-lint run ./...: 0 issues - govulncheck ./...: 0 vulnerabilities in our code - tsc --noEmit: 0 errors - vitest run: 3 files, 218 tests passed sha256 before: 7c14f77107a86f8de82fe91b7f5e16cca11206d1e1fab7b7bd77ff396620fdf3 sha256 after: 87bd92d0407d63643bec612d27261bf489563beb90d0791ea71cde26346f83d3	2026-04-18 03:21:40 +00:00
shankar0123	2c9602db71	Merge branch 'fix/m9-sentinel-discovery-log-levels': M-9 sentinel discovery log-level fix	2026-04-18 02:53:50 +00:00
shankar0123	ef670fa6da	fix(m-9): aggregate per-endpoint scan errors in NetworkScanService ... Before this fix, RunScan declared `scanErrors []string` but never appended to it. As a result: - the summary Info log ("network target scan completed") always reported `"errors": 0`, regardless of how many endpoints failed - the DiscoveryReport's `Errors` field — stored on the scan record and surfaced in the GUI scan history — was always nil Operators who needed to understand scan failures had to enable Debug logging and grep through the noise of expected sweep-scan connection refusals. The per-endpoint log level (Debug) is deliberate and correct — scanning a /24 typically produces 200+ connection-refused results, and logging each at Warn would create massive log spam at default verbosity. The bug was the silent loss of the aggregate count. This commit: - extracts the partitioning logic into `collectScanResults`, a pure method that splits per-endpoint results into discovered certificate entries and a list of endpoint error strings - populates the errors list with "<address>: <error>" so the scan record correlates failures back to specific endpoints - preserves the existing Debug-level per-endpoint log (sweep noise discipline) — no change to default-verbosity log output The summary Info log's "errors" field and the DiscoveryReport's Errors field now reflect the true failure count. Debug detail remains available for operators diagnosing specific endpoints. Audit scope note: the M-9 finding narrative implied broad Debug-level hiding of real errors across AWS SM, Azure KV, GCP SM, and network scan sentinel agents. On investigation, the three cloud-discovery connectors (awssm, azurekv, gcpsm) already use appropriate Warn/Error discipline for per-item and root-level failures. Only the network scanner had a silent observability gap, and it was a missed append rather than a misapplied log level. See audit resolution log for full details. CWE: CWE-778 (Insufficient Logging) — aggregate failure count lost. Tests: 4 new unit tests on collectScanResults covering the aggregation path (success + failure mix), all-success, all-failed, and empty-input degenerate cases. All tests pass with -race. Verification: - go build ./cmd/server/... ./cmd/agent/... ./cmd/mcp-server/... ./cmd/cli/... exit 0 - go vet ./... exit 0 - go test -race -count=1 -timeout 300s [full CI race path] exit 0 - golangci-lint run ./... --timeout 5m (v2.11.4) 0 issues - govulncheck ./... (@latest) 0 in-code vulnerabilities - go test -count=1 -cover ./internal/service/... 68.0% (> 55% threshold) Invariants preserved: - collectScanResults signature: method on *NetworkScanService, input []domain.NetworkScanResult, return ([]DiscoveredCertEntry, []string) - Debug log key names unchanged ("address", "error") - DiscoveryReport schema unchanged (Errors field already existed) - Sentinel agent ID "server-scanner" unchanged - No migration, no API, no wire-format change Refs: M-9 Medium finding; audit resolution log appended in follow-up commit on workspace-level audit report.	2026-04-18 02:34:14 +00:00
shankar0123	5a6ec39cfd	Merge branch 'fix/m2-pr-f-scheduler-contextcheck-audit-closeout'	2026-04-18 01:43:56 +00:00
shankar0123	e3196e7b50	M-2 PR-F: Middleware/ACME ctx-propagation + contextcheck linter + audit closeout ... Final PR in the six-commit M-2 sequence (PR-A: CertificateService cluster cdc9d03, PR-B: IssuerService+TargetService eb14236, PR-C: Policy/Profile/ Owner/Team 2497be4, PR-D: Job/Notification/Audit ccd89c3, PR-E: AgentService 283ec27, PR-F: this commit). PR-A through PR-E collapsed the service-layer shim methods and deleted every in-production context.Background() / context.TODO() call from internal/service/; this PR completes the sweep across the non-service tiers (HTTP middleware + ACME connector) and wires the contextcheck linter so regressions fail CI. Three narrow edits land the D-3 pattern (context.WithoutCancel for subsidiary async writes and deferred shutdown contexts): - internal/api/middleware/audit.go -- async audit goroutine now runs on auditCtx := context.WithoutCancel(r.Context()) instead of context.Background(). Preserves request-scoped values (trace ID, auth) while detaching from the request's cancellation so the audit write does not get killed when the response completes. Goroutine is still tracked via a.wg (M-1 shutdown drain) so Flush(ctx) behaviour is unchanged. CWE-770 Missing Release (goroutine leak potential) + CWE-400 Resource Exhaustion (missed cancellation propagation). - internal/api/middleware/middleware.go -- Recovery panic path now logs via slog.ErrorContext(ctx, ...) instead of log.Printf. Request- scoped trace/auth metadata now carries through the panic log, matching every other request log. D-3 non-bypass: the context is r.Context() captured before the defer, so even a panic mid-handler propagates the ctx's trace ID into the ERROR log line. - internal/connector/issuer/acme/acme.go (HTTP-01 challenge server shutdown) -- defer shutdown context derived from context.WithTimeout(context.WithoutCancel(ctx), 5s) instead of context.Background(). Preserves parent ctx values, detaches from parent cancellation so Shutdown always gets its full 5-second budget even when the parent was cancelled. Matches the same pattern applied in ACME's solveAuthorizationsDNS01 and solveAuthorizationsDNSPersist01. Linter wiring: .golangci.yml adds `contextcheck` to the enabled set. golangci-lint v2.11.4 now fails CI on any function that takes a context.Context parameter but calls into context.Background() or context.TODO() instead of propagating -- regression guard for all five prior PRs. Verification (CI parity, GOCACHE=/tmp/gocache GOMODCACHE=/tmp/gomodcache GOLANGCI_LINT_CACHE=/tmp/lintcache): - go build ./... -> 0 - go vet ./... -> 0 - golangci-lint run (contextcheck enabled) -> 0 issues - go test -race -short ./internal/api/middleware/... -> PASS - go test -race -short ./internal/scheduler/... -> PASS - go test -race -short ./internal/connector/issuer/acme/... -> PASS - go test -race -short ./internal/service/... -> PASS - rg "context\.(Background|TODO)\(\)" internal/service/ internal/scheduler/ internal/connector/ internal/api/middleware/ -> 0 non-test hits (one pedagogical godoc reference in audit.go documenting why context.Background() would be wrong remains intentional) Wire-format invariants preserved: 0 API routes, 0 SQL migrations, 0 frontend bytes, 0 OpenAPI bytes, 0 connector interface signature changes, 0 new env vars, 0 new external dependencies (pure context stdlib). The AuditRecorder interface signature, the body-hash algorithm (SHA-256 16 hex chars), the excluded-path short-circuit, the actor-extraction path, the responseWriter status-capture wrapper, the AuditServiceAdapter, and all 116 API routes under /api/v1/, /.well-known/est/, /scep, /health, /auth are byte-identical. M-2 aggregate across PR-A through PR-F: 57 files, +635 / -613 (PR-A 12f +227/-237, PR-B 9f +150/-146, PR-C 17f +156/-148, PR-D 11f +67/-63, PR-E 4f +9/-15, PR-F 4f +26/-4). With M-2 closed, 8 of 10 Medium findings resolved; M-9, M-10, L-1..L-4, I-1..I-8 remain post-v2.1.0 hardening batch. Audit complete. Commit: 1f6cf0eafa. Sections: 12. Findings: 2/7/10/4/6.	2026-04-18 01:43:47 +00:00
shankar0123	bea69efd12	Merge branch 'fix/m2-pr-e-agent-service' ... PR-E of 6: AgentService ctx-first collapse. Collapses the HeartbeatWithContext wrapper into a single Heartbeat method. Handler-facing method name is preserved (D-4); the handler service interface and mock already expected ctx-first, so this PR touches only the service layer and its tests (4 files, 9+/15-). Verification on the feature branch: build, vet, test (-short), test -race, full-module test -short, and golangci-lint all clean. Audit complete. Commit: 1f6cf0eafa. Sections: 12. Findings: 2/7/10/4/6.	2026-04-18 01:25:30 +00:00
shankar0123	283ec27ca4	fix(m2-pr-e): collapse AgentService.HeartbeatWithContext into Heartbeat ... PR-E of 6 in the M-2 end-to-end remediation sequence. Collapses the HeartbeatWithContext wrapper into a single ctx-first Heartbeat method, matching D-1 (ctx-only signatures, no dual forms). The handler-facing method name is preserved (D-4) — internal/api/handler/agents.go already declares `Heartbeat(ctx, ...)` on its local service interface, and the handler mock at internal/api/handler/agent_handler_test.go already takes `_ context.Context` as its first param, so no handler churn. Changes ------- internal/service/agent.go - Delete the zero-body Heartbeat wrapper that forwarded to HeartbeatWithContext with context.Background(). - Rename HeartbeatWithContext → Heartbeat (ctx-bearing body folded directly into the canonical method). internal/service/agent_test.go - TestHeartbeat (L95) and TestHeartbeat_NotFound (L128): agentService.HeartbeatWithContext(ctx, ...) → .Heartbeat(ctx, ...). internal/service/concurrent_test.go - L162: agentSvc.HeartbeatWithContext(ctx, agentID, metadata) → .Heartbeat(ctx, agentID, metadata). internal/service/context_test.go - L179 + L232: agentSvc.HeartbeatWithContext(ctx, ...) → .Heartbeat(...) - L185 + L238 t.Logf strings: "HeartbeatWithContext with ..." → "Heartbeat with ..." to match the collapsed method name. Verification (Go 1.25.9 linux/arm64, CI-parity caches) ------------------------------------------------------ go build ./... clean go vet ./... clean go test -short ./internal/service/... ./internal/api/handler/... \ ./internal/integration/... all ok go test -race -short same set all ok go test -short ./... all packages ok golangci-lint run ./... 0 issues Locked decisions from the M-2 plan: D-1 ctx-only signatures (no dual forms) D-4 preserve handler method names facing the router D-5 domain types stay ctx-free Audit complete. Commit: 1f6cf0eafa. Sections: 12. Findings: 2/7/10/4/6.	2026-04-18 01:25:20 +00:00
shankar0123	a67a6b6c30	Merge branch 'fix/m2-pr-d-job-notification-audit' ... PR-D: Thread ctx through Job + Notification + Audit service cluster. Collapse CancelJobWithContext into CancelJob; eliminate 10 context.Background() hits. Audit complete. Commit: 1f6cf0eafa. Sections: 12. Findings: 2/7/10/4/6.	2026-04-18 01:20:58 +00:00
shankar0123	ccd89c348f	fix(m2-pr-d): thread ctx through Job/Notification/Audit services ... Collapse CancelJobWithContext into CancelJob; eliminate 10 context.Background() hits across the Job+Notification+Audit service cluster by threading ctx through their handler-facing service interfaces. Services (ctx-first): - service/job.go: ListJobs, GetJob, CancelJob, ApproveJob, RejectJob now accept ctx; the CancelJobWithContext wrapper is removed (handler callers continue to invoke CancelJob, now ctx-aware). - service/notification.go: ListNotifications, GetNotification, MarkAsRead accept ctx. - service/audit.go: ListAuditEvents, GetAuditEvent accept ctx. Handlers (interface + callsites): - handler/jobs.go, handler/notifications.go, handler/audit.go: local service interfaces updated, r.Context() threaded at every callsite. Tests: - Mock services updated to match the new interfaces (ctx accepted and ignored via '_ context.Context' first parameter; Fn closure fields unchanged). - job_test.go / notification_test.go callsites thread context.Background() to match production shape. Verification: go build ./... ok go vet ./... ok go test -short ./... ok go test -race -short ./... ok golangci-lint run ./... 0 issues Locked decisions from the M-2 plan: D-1 ctx-only signatures (no dual forms) D-4 preserve handler method names facing the router D-5 domain types stay ctx-free Audit complete. Commit: 1f6cf0eafa. Sections: 12. Findings: 2/7/10/4/6.	2026-04-18 01:20:46 +00:00
shankar0123	478a141498	Merge branch 'fix/m2-pr-c-crud-cluster'	2026-04-18 01:10:10 +00:00