From 5b7a022767400c83725e199b71ad42b838ebfa8a Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Thu, 30 Apr 2026 20:36:26 +0000 Subject: [PATCH] ci-pipeline-cleanup Phase 1: extract 20 regression guards to scripts/ci-guards/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bundle: ci-pipeline-cleanup, Phase 1. Pure relocation — no behavior change. Each guard's bash logic is byte-identical to the prior inline version; the only changes are: (a) the guard becomes a sibling script under scripts/ci-guards/.sh, (b) ci.yml's per-guard step is replaced by a single loop step that iterates all scripts. 20 scripts extracted (alphabetized): B-1-orphan-crud.sh, D-1-D-2-statusbadge-phantom.sh, G-1-jwt-auth-literal.sh, G-2-api-key-hash-json.sh, G-3-env-docs-drift.sh, H-001-bare-from.sh, H-009-readme-jwt.sh, L-001-insecure-skip-verify.sh, L-1-bulk-action-loop.sh, M-012-no-root-user.sh, P-1-documented-orphan-fns.sh, S-1-hardcoded-source-counts.sh, S-2-strings-contains-err.sh, T-1-frontend-page-coverage.sh, U-2-plaintext-healthcheck.sh, U-3-migration-mount.sh, bundle-8-L-015-target-blank-rel-noopener.sh, bundle-8-L-019-dangerously-set-inner-html.sh, bundle-8-M-009-bare-usemutation.sh, test-naming-convention.sh Plus scripts/ci-guards/README.md documenting the contract: - Each script must exit 0 on clean repo, non-zero with ::error:: prefix on regression - Runnable from repo root via 'bash scripts/ci-guards/.sh' - Adding a new guard: drop a new .sh; CI auto-picks it up ci.yml dropped 1488 → 557 lines (-931, -63%). Single CI loop step now collects ALL guard failures before failing the build instead of fail-fast — UX win for regressions that hit two guards at once. Two guards (QA-doc Part-count + seed-count, ci.yml lines 868-917) deliberately NOT extracted — they move to 'make verify-docs' in Phase 11 because they protect docs-the-operator-reads, not the product itself. Verification (sandbox): - All 20 scripts pass against HEAD (chmod +x; for g in scripts/ci-guards/*.sh; do bash $g; done) - New ci.yml YAML-parses cleanly - Job boundaries preserved: go-build-and-test, frontend-build, helm-lint, deploy-vendor-e2e, deploy-vendor-e2e-windows - Loop step appears twice (once at end of go-build-and-test, once at end of frontend-build) so both jobs continue running their set of guards --- .github/workflows/ci.yml | 993 +----------------- scripts/ci-guards/B-1-orphan-crud.sh | 53 + .../ci-guards/D-1-D-2-statusbadge-phantom.sh | 154 +++ scripts/ci-guards/G-1-jwt-auth-literal.sh | 65 ++ scripts/ci-guards/G-2-api-key-hash-json.sh | 59 ++ scripts/ci-guards/G-3-env-docs-drift.sh | 95 ++ scripts/ci-guards/H-001-bare-from.sh | 27 + scripts/ci-guards/H-009-readme-jwt.sh | 28 + .../ci-guards/L-001-insecure-skip-verify.sh | 38 + scripts/ci-guards/L-1-bulk-action-loop.sh | 46 + scripts/ci-guards/M-012-no-root-user.sh | 29 + .../ci-guards/P-1-documented-orphan-fns.sh | 38 + scripts/ci-guards/README.md | 76 ++ .../ci-guards/S-1-hardcoded-source-counts.sh | 46 + scripts/ci-guards/S-2-strings-contains-err.sh | 37 + .../ci-guards/T-1-frontend-page-coverage.sh | 56 + .../ci-guards/U-2-plaintext-healthcheck.sh | 51 + scripts/ci-guards/U-3-migration-mount.sh | 59 ++ ...undle-8-L-015-target-blank-rel-noopener.sh | 30 + ...ndle-8-L-019-dangerously-set-inner-html.sh | 30 + .../bundle-8-M-009-bare-usemutation.sh | 45 + scripts/ci-guards/test-naming-convention.sh | 28 + 22 files changed, 1121 insertions(+), 962 deletions(-) create mode 100755 scripts/ci-guards/B-1-orphan-crud.sh create mode 100755 scripts/ci-guards/D-1-D-2-statusbadge-phantom.sh create mode 100755 scripts/ci-guards/G-1-jwt-auth-literal.sh create mode 100755 scripts/ci-guards/G-2-api-key-hash-json.sh create mode 100755 scripts/ci-guards/G-3-env-docs-drift.sh create mode 100755 scripts/ci-guards/H-001-bare-from.sh create mode 100755 scripts/ci-guards/H-009-readme-jwt.sh create mode 100755 scripts/ci-guards/L-001-insecure-skip-verify.sh create mode 100755 scripts/ci-guards/L-1-bulk-action-loop.sh create mode 100755 scripts/ci-guards/M-012-no-root-user.sh create mode 100755 scripts/ci-guards/P-1-documented-orphan-fns.sh create mode 100644 scripts/ci-guards/README.md create mode 100755 scripts/ci-guards/S-1-hardcoded-source-counts.sh create mode 100755 scripts/ci-guards/S-2-strings-contains-err.sh create mode 100755 scripts/ci-guards/T-1-frontend-page-coverage.sh create mode 100755 scripts/ci-guards/U-2-plaintext-healthcheck.sh create mode 100755 scripts/ci-guards/U-3-migration-mount.sh create mode 100755 scripts/ci-guards/bundle-8-L-015-target-blank-rel-noopener.sh create mode 100755 scripts/ci-guards/bundle-8-L-019-dangerously-set-inner-html.sh create mode 100755 scripts/ci-guards/bundle-8-M-009-bare-usemutation.sh create mode 100755 scripts/ci-guards/test-naming-convention.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index baeca7c..477d396 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -78,626 +78,6 @@ jobs: continue-on-error: true run: staticcheck ./... - - name: Forbidden auth-type literal regression guard (G-1) - # G-1 closed the JWT silent auth downgrade by removing "jwt" from the - # accepted CERTCTL_AUTH_TYPE values. This step grep-fails the build - # if "jwt" reappears in any of the *additive* auth-type surfaces: - # the validAuthTypes / ValidAuthTypes() set, the OpenAPI enum, the - # helm chart's allowed-types list, or the .env.example default. - # Comment lines and the dedicated rejection branch in config.go - # (`c.Auth.Type == "jwt"`) are intentionally exempt — those are the - # G-1 fix itself, not a regression. - # - # Connector packages (internal/connector/) are exempt because the - # Google OAuth2 service-account JWT and step-ca provisioner one- - # time-token JWT are external-protocol uses, unrelated to certctl's - # own auth shape. Test files (_test.go) are exempt so negative - # tests can pass the literal. - # - # See docs/upgrade-to-v2-jwt-removal.md for the closure rationale, - # or internal/config/config.go::ValidAuthTypes for the allowed set. - run: | - set -e - - # Scoped patterns that indicate "jwt" being added back to an - # allowed-set surface. Each catches a regression shape we've - # actually seen in pre-G-1 code: - # - Go map/slice literal: "jwt": true or "jwt", - # - Go switch case: case "jwt" - # - YAML enum: enum: [..., jwt, ...] or - jwt - # - .env conditional: AUTH_TYPE.*"jwt"|=jwt$ - BAD=$(grep -rnEH \ - -e '"jwt"\s*:\s*true' \ - -e '"jwt"\s*,' \ - -e 'case\s+"jwt"' \ - -e 'enum:.*\bjwt\b' \ - -e '^\s*-\s*jwt\s*$' \ - -e 'AUTH_TYPE\s*=\s*jwt\s*$' \ - -e 'AUTH_TYPE\s*=\s*jwt\s*#' \ - -e 'auth\.type\s*=\s*jwt\s*$' \ - -e 'AuthType\("jwt"\)' \ - internal/config/ \ - internal/api/ \ - cmd/ \ - api/openapi.yaml \ - .env.example \ - deploy/.env.example \ - deploy/helm/certctl/values.yaml \ - deploy/helm/certctl/templates/ \ - 2>/dev/null \ - | grep -v '_test.go' \ - | grep -vE '^\s*[^:]+:[0-9]+:\s*(//|#)' \ - | grep -v 'is no longer accepted' \ - || true) - if [ -n "$BAD" ]; then - echo "G-1 regression: \"jwt\" reappeared in an allowed-set surface:" - echo "$BAD" - echo "" - echo "Allowed surface for 'jwt' literals: comment lines, the" - echo "dedicated rejection branch in internal/config/config.go," - echo "and connector packages (Google OAuth2, step-ca)." - echo "See docs/upgrade-to-v2-jwt-removal.md and" - echo "internal/config/config.go::ValidAuthTypes()." - exit 1 - fi - - - name: Forbidden bare InsecureSkipVerify regression guard (L-001) - # L-001 audited every production InsecureSkipVerify=true call site - # and documented the justification per site in docs/tls.md. This - # step grep-fails the build if any new `InsecureSkipVerify: true` - # lands in a non-test Go file without a `//nolint:gosec` comment - # carrying the justification. Test files (_test.go) are exempt. - # Updating the documented surface goes through the docs/tls.md - # table — net-new sites must be reasoned about before merge. - run: | - set -e - # Find every "InsecureSkipVerify: true" or "InsecureSkipVerify = true" - # in a non-test .go file. Then for each, check the same line OR the - # immediately preceding line for `//nolint:gosec`. - BAD="" - while IFS= read -r match; do - file=$(echo "$match" | cut -d: -f1) - line=$(echo "$match" | cut -d: -f2) - same=$(sed -n "${line}p" "$file" 2>/dev/null) - prev=$(sed -n "$((line - 1))p" "$file" 2>/dev/null) - if echo "$same $prev" | grep -q 'nolint:gosec'; then - continue - fi - BAD="$BAD\n$match" - done < <(grep -rnE 'InsecureSkipVerify:\s*true|InsecureSkipVerify\s*=\s*true' \ - --include='*.go' \ - --exclude='*_test.go' \ - . || true) - if [ -n "$BAD" ]; then - echo "::error::New InsecureSkipVerify=true site without //nolint:gosec justification:" - echo -e "$BAD" - echo "" - echo "Add a //nolint:gosec comment with justification on the same" - echo "or preceding line, AND add a row to the docs/tls.md table." - exit 1 - fi - - - name: Forbidden bare FROM regression guard (H-001) - # Bundle A / Audit H-001 (CWE-829): every FROM line in every - # Dockerfile in the repo MUST carry an @sha256:... digest pin in - # addition to the human-readable tag. A registry-side tag swap - # cannot then change what we pull. This step grep-fails the - # build if any new FROM lands without the @sha256 suffix. - run: | - set -e - # Match any "FROM image[:tag]" that does NOT contain @sha256. - # Strip comments and blank lines defensively. - BAD=$(find . -name 'Dockerfile*' -not -path './web/node_modules/*' \ - -exec grep -HnE '^FROM\s+[^@#]+(\s+AS\s+\S+)?\s*$' {} \; || true) - if [ -n "$BAD" ]; then - echo "::error::Dockerfile has bare FROM (no @sha256 digest pin):" - echo "$BAD" - echo "" - echo "Pin every FROM to an immutable digest. See the bump" - echo "procedure in Dockerfile's header comment (Bundle A / H-001)." - exit 1 - fi - - - name: Forbidden missing USER regression guard (M-012) - # Bundle A / Audit M-012 (CWE-250): every Dockerfile in the repo - # MUST end with a `USER ` directive before the - # ENTRYPOINT/CMD so the container never runs as uid=0. This step - # grep-fails the build if any Dockerfile is missing such a USER. - # `USER root` and `USER 0` are explicitly rejected. - run: | - set -e - BAD="" - for df in $(find . -name 'Dockerfile*' -not -path './web/node_modules/*'); do - # Find the LAST USER directive in the file. - last_user=$(grep -E '^USER\s+\S+' "$df" | tail -1 | awk '{print $2}') - if [ -z "$last_user" ]; then - BAD="$BAD\n$df: no USER directive at all" - continue - fi - if [ "$last_user" = "root" ] || [ "$last_user" = "0" ]; then - BAD="$BAD\n$df: terminal USER is $last_user (must drop privileges)" - continue - fi - done - if [ -n "$BAD" ]; then - echo "::error::Dockerfile USER-drop regression:" - echo -e "$BAD" - exit 1 - fi - - - name: Forbidden README JWT advertising regression guard (H-009) - # H-009 closed by Bundle D as verified-already-clean: at audit time - # the README does NOT advertise JWT support (certctl does not ship - # in-process JWT middleware; JWT/OIDC integration is via an - # authenticating gateway, see docs/architecture.md "Authenticating- - # gateway pattern"). This step grep-fails the build if README ever - # re-introduces a sentence advertising JWT as a supported auth mode. - # Pattern: "JWT" within ~6 words of "support|auth|enabled|mode" in - # README.md. The architecture / compliance / connector docs that - # legitimately mention JWT (Google OAuth2 service-account JWT, - # step-ca provisioner JWT, JWT-via-gateway pattern) are out of - # scope — they describe what certctl does NOT do, or external - # protocol uses. - run: | - set -e - if grep -inE 'JWT.{0,40}(support|auth|enabled|mode|provider)' README.md \ - | grep -v 'gateway' | grep -v 'pre-G-1'; then - echo "::error::README.md appears to advertise JWT auth support." - echo "certctl does NOT ship in-process JWT middleware. JWT/OIDC" - echo "integration is via an authenticating gateway — see" - echo "docs/architecture.md::Authenticating-gateway pattern." - echo "If you added a sentence about JWT to README, either remove" - echo "it or rewrite it to point at the gateway pattern." - exit 1 - fi - - - name: Forbidden api_key_hash JSON-shape regression guard (G-2) - # G-2 closed cat-s5-apikey_leak by tagging Agent.APIKeyHash - # `json:"-"` and adding a defense-in-depth Agent.MarshalJSON that - # zeroes the field on the marshal-time copy. This step grep-fails - # the build if `api_key_hash` reappears in any of the *additive* - # JSON-emitting surfaces: a Go struct json tag in internal/domain/, - # an OpenAPI Agent schema property, a TypeScript field declaration - # in web/src/, or an enum-list / discriminator in handler - # production code. - # - # Repository, migration, seed, service, integration-test, and - # unit-test files are exempt — those are server-internal use - # sites (the DB column stays, the in-memory struct field stays, - # the auth-lookup path stays). Comment lines are exempt so the - # G-2 closure rationale can stay in the source. - # - # See coverage-gap-audit-2026-04-24-v5/unified-audit.md - # cat-s5-apikey_leak for the closure rationale, or - # internal/domain/connector.go::Agent::MarshalJSON for the - # redaction enforcement. - run: | - set -e - - # Scoped patterns that indicate api_key_hash being added back - # to a JSON-emitting surface. Each catches a regression shape - # that pre-G-2 actually shipped or that a future refactor - # could plausibly introduce: - # - Go struct tag: `json:"api_key_hash"` - # - Frontend interface: api_key_hash[?]: string - # - OpenAPI schema property: api_key_hash: (column-aligned) - # - YAML enum / array: - api_key_hash - BAD=$(grep -rnEH \ - -e 'json:"api_key_hash[",]' \ - -e '^\s*api_key_hash\??\s*:' \ - -e '^\s*-\s*api_key_hash\s*$' \ - internal/domain/ \ - internal/api/ \ - cmd/ \ - api/openapi.yaml \ - web/src/ \ - 2>/dev/null \ - | grep -v '_test.go' \ - | grep -vE '^\s*[^:]+:[0-9]+:\s*(//|#)' \ - || true) - if [ -n "$BAD" ]; then - echo "G-2 regression: api_key_hash reappeared in a JSON-emitting surface:" - echo "$BAD" - echo "" - echo "Allowed surface for api_key_hash literals: comment lines," - echo "the database column (migrations/), the in-memory struct" - echo "field tagged \`json:\"-\"\`, and the repository / service" - echo "use sites. See internal/domain/connector.go::Agent and" - echo "coverage-gap-audit-2026-04-24-v5/unified-audit.md" - echo "cat-s5-apikey_leak for the closure rationale." - exit 1 - fi - - - name: Forbidden plaintext HEALTHCHECK regression guard (U-2) - # U-2 closed cat-u-healthcheck_protocol_mismatch by switching the - # published image's HEALTHCHECK from `curl -f http://localhost: - # 8443/health` (always failed against the HTTPS-only listener) to - # `curl -fsk https://localhost:8443/health`. This step grep-fails - # the build if any Dockerfile in the repo carries the pre-U-2 - # plaintext shape — either explicitly (`http://localhost:8443/ - # health` in a HEALTHCHECK) or via the looser pattern of any - # HEALTHCHECK that targets `http://` against the certctl server - # port. - # - # Comment lines and the docs/upgrade-to-tls.md:182 expected-to- - # fail invariant ("plaintext is gone, expect Connection refused") - # are intentionally exempt — we DO want the upgrade-doc string - # `http://localhost:8443/health` to remain there, since it - # documents what operators should test for to confirm plaintext - # is dead. The guardrail is scoped to Dockerfile* only, so docs - # are out of its reach. - # - # See coverage-gap-audit-2026-04-24-v5/unified-audit.md - # cat-u-healthcheck_protocol_mismatch for the closure rationale, - # or deploy/test/healthcheck_test.go for the binary-image - # contract the runtime test pins. - run: | - set -e - - # Patterns that catch the actual regression shapes: - # - HEALTHCHECK directive carrying any http:// (even if the - # port differs, no plaintext probe should ship). - # - The exact pre-U-2 string for grep-friendliness. - BAD=$(grep -rnEH \ - -e 'HEALTHCHECK.*http://' \ - -e 'curl[^|&;]*-f[^|&;]*http://localhost:8443/health' \ - Dockerfile Dockerfile.agent Dockerfile.* 2>/dev/null \ - | grep -vE '^\s*[^:]+:[0-9]+:\s*#' \ - || true) - if [ -n "$BAD" ]; then - echo "U-2 regression: plaintext HEALTHCHECK reappeared in a Dockerfile:" - echo "$BAD" - echo "" - echo "Allowed: HTTPS HEALTHCHECK with -k (acceptable for" - echo "localhost-to-localhost), or non-HTTP probe shapes" - echo "(pgrep, /proc check). See Dockerfile / Dockerfile.agent" - echo "for the post-U-2 reference shape and" - echo "coverage-gap-audit-2026-04-24-v5/unified-audit.md" - echo "cat-u-healthcheck_protocol_mismatch for rationale." - exit 1 - fi - - - name: Forbidden migration mount in compose initdb (U-3) - # U-3 closed cat-u-seed_initdb_schema_drift (GitHub #10) by - # eliminating the dual-source-of-truth between - # `migrations/*.up.sql` mounted into postgres - # `/docker-entrypoint-initdb.d/` and the same files re-applied at - # runtime by `RunMigrations`. Pre-U-3 every new migration that - # the seed depended on (000013 added `policy_rules.severity`, - # 000017 renames `retry_interval_seconds`, etc.) had to be added - # by hand to the compose mount list; missing the update crashed - # initdb on first boot, postgres flagged unhealthy, and the - # whole stack failed to start from a fresh clone. Post-U-3 the - # server is the single source of truth — `RunMigrations` + - # `RunSeed` apply everything at boot. - # - # This step grep-fails the build if any compose file under - # `deploy/` re-introduces a `migrations/.*\.sql` mount into - # `/docker-entrypoint-initdb.d`. Comments are exempt so the - # post-fix rationale block in the compose files (which - # documents WHY the mounts were removed) doesn't trip the guard. - # The demo overlay's `seed_demo.sql` is the explicit exception: - # it is tolerated only when it lives behind the - # CERTCTL_DEMO_SEED env var (post-U-3 demo path) — bare initdb - # mounts are NOT tolerated. The grep matches all compose - # mount-list shapes (`-` indented, `volumes:` indented, both), - # so any future drift surfaces here before the operator hits it - # on a fresh clone. - # - # See coverage-gap-audit-2026-04-24-v5/unified-audit.md - # cat-u-seed_initdb_schema_drift for the closure rationale, or - # internal/repository/postgres/db.go::RunSeed for the runtime - # contract. - run: | - set -e - - BAD=$(grep -rnEH \ - -e 'migrations/.*\.sql:.*docker-entrypoint-initdb' \ - -e 'seed.*\.sql:.*docker-entrypoint-initdb' \ - deploy/docker-compose.yml \ - deploy/docker-compose.test.yml \ - deploy/docker-compose.demo.yml \ - 2>/dev/null \ - | grep -vE '^\s*[^:]+:[0-9]+:\s*#' \ - || true) - if [ -n "$BAD" ]; then - echo "U-3 regression: migration/seed mount into postgres initdb reappeared:" - echo "$BAD" - echo "" - echo "The post-U-3 contract is: postgres comes up with an empty" - echo "schema and the server applies migrations + seed at boot via" - echo "internal/repository/postgres.RunMigrations + RunSeed. Demo" - echo "data lives behind CERTCTL_DEMO_SEED=true (RunDemoSeed)," - echo "not an initdb mount. See" - echo "coverage-gap-audit-2026-04-24-v5/unified-audit.md" - echo "cat-u-seed_initdb_schema_drift for the closure rationale." - exit 1 - fi - - - name: Forbidden StatusBadge dead-key + TS phantom-field regression guard (D-1 + D-2) - # D-1 master closed cat-d-359e92c20cbf (Agent: 'Stale' dead key, - # 'Degraded' missing), cat-d-9f4c8e4a91f1 (Notification: 'dead' - # missing), cat-d-1447e04732e7 (Cert: 'PendingIssuance' dead - # key), cat-f-cert_detail_page_key_render_fallback (render-site - # uses cert.X directly), and cat-f-ae0d06b6588f (Certificate - # TS phantom fields). This step grep-fails the build if either - # half of the closure is reverted: - # - # 1. The dead StatusBadge keys ('Stale' for Agent, 'PendingIssuance' - # for Cert) reappearing as map literals, OR - # 2. The five phantom Certificate TS fields (serial_number, - # fingerprint_sha256, key_algorithm, key_size, issued_at) - # reappearing on the `Certificate` interface in types.ts - # (CertificateVersion legitimately carries them and is - # explicitly excluded by the awk pre-filter below). - # - # Comments are exempt so the closure prose in StatusBadge.tsx + - # types.ts can stay. Test files are exempt so negative tests - # asserting the dead keys fall through to neutral keep working. - # - # See coverage-gap-audit-2026-04-24-v5/unified-audit.md - # cat-d-* / cat-f-* for the closure rationale, or - # web/src/components/StatusBadge.test.tsx for the live - # enum-coverage contract. - run: | - set -e - - BAD_BADGE=$(grep -nE "^\s*(Stale|PendingIssuance)\s*:\s*'badge-" \ - web/src/components/StatusBadge.tsx 2>/dev/null \ - | grep -v '\.test\.' \ - | grep -vE '^\s*[^:]+:[0-9]+:\s*//' \ - || true) - if [ -n "$BAD_BADGE" ]; then - echo "D-1 regression: dead StatusBadge key reappeared:" - echo "$BAD_BADGE" - echo "" - echo "Allowed surface: comment lines naming the removed key in" - echo "the file's preamble. The Go-side AgentStatus values are" - echo "Online/Offline/Degraded (no Stale); CertificateStatus values" - echo "are Pending/Active/... (no PendingIssuance). See" - echo "web/src/components/StatusBadge.test.tsx for the contract." - exit 1 - fi - - # Certificate TS phantom-field check. Scoped to the - # `export interface Certificate {` block in web/src/api/types.ts - # — CertificateVersion legitimately declares these fields and - # must NOT trip the guardrail. The awk window opens on the - # exact `Certificate {` header (not `CertificateVersion {`, - # not `CertificateProfile {`) and closes at the first `}`, - # then the grep matches a phantom-field declaration anywhere - # in that window. - BAD_TS=$(awk ' - /^export interface Certificate \{/ { flag=1; next } - flag && /^\}/ { flag=0 } - flag { print FILENAME":"NR":"$0 } - ' web/src/api/types.ts \ - | grep -E '\b(serial_number|fingerprint_sha256|key_algorithm|key_size|issued_at)\??\s*:' \ - || true) - if [ -n "$BAD_TS" ]; then - echo "D-1 regression: Certificate TS interface re-added a phantom field:" - echo "$BAD_TS" - echo "" - echo "These fields live on CertificateVersion, not ManagedCertificate." - echo "The Go-side ManagedCertificate has never carried them; the" - echo "TS optional declarations were silently undefined on every" - echo "list response. Render-site consumers (e.g. CertificateDetailPage)" - echo "use latestVersion?.field as the canonical access path." - echo "See coverage-gap-audit-2026-04-24-v5/unified-audit.md" - echo "cat-f-ae0d06b6588f for the closure rationale." - exit 1 - fi - - # D-2 master closed five diff-05x06-* type-drift findings: - # Agent (5 phantoms), Issuer (1 phantom), Notification (1 phantom) - # — TRIM half. The Target (2 missing fields) and DiscoveredCertificate - # (1 missing field) — ADD half is pinned by the literal-construction - # blocks in web/src/api/types.test.ts, not a CI grep. The phantom- - # trim regression vector is an awk-windowed grep per interface - # mirroring the D-1 Certificate check above. - # - # See coverage-gap-audit-2026-04-24-v5/unified-audit.md - # diff-05x06-7cdf4e78ae24 (Agent), diff-05x06-97fab8783a5c (Issuer), - # diff-05x06-caba9eb3620e (Notification) for the closure rationale. - - # D-2 Agent phantom-field check. The grep matches `last_heartbeat` - # but NOT `last_heartbeat_at` (the legitimate Go-emitted field) — - # the `\b...\b` boundaries plus the `grep -v 'last_heartbeat_at'` - # filter handle that. - BAD_AGENT=$(awk ' - /^export interface Agent \{/ { flag=1; next } - flag && /^\}/ { flag=0 } - flag { print FILENAME":"NR":"$0 } - ' web/src/api/types.ts \ - | grep -E '\b(last_heartbeat|capabilities|tags|created_at|updated_at)\??\s*:' \ - | grep -v 'last_heartbeat_at' \ - || true) - if [ -n "$BAD_AGENT" ]; then - echo "D-2 regression: Agent TS interface re-added a phantom field:" - echo "$BAD_AGENT" - echo "" - echo "The Go-side internal/domain/connector.go::Agent emits exactly:" - echo "id, name, hostname, status, last_heartbeat_at?, registered_at," - echo "os, architecture, ip_address, version, retired_at?, retired_reason?." - echo "The five fields blocked by this guard (last_heartbeat," - echo "capabilities, tags, created_at, updated_at) were TS phantoms" - echo "the Go struct never emitted. See unified-audit.md" - echo "diff-05x06-7cdf4e78ae24 for closure rationale." - exit 1 - fi - - # D-2 Issuer phantom-field check. - BAD_ISSUER=$(awk ' - /^export interface Issuer \{/ { flag=1; next } - flag && /^\}/ { flag=0 } - flag { print FILENAME":"NR":"$0 } - ' web/src/api/types.ts \ - | grep -E '\bstatus\??\s*:' \ - || true) - if [ -n "$BAD_ISSUER" ]; then - echo "D-2 regression: Issuer TS interface re-added a phantom 'status' field:" - echo "$BAD_ISSUER" - echo "" - echo "The Go-side internal/domain/connector.go::Issuer has no 'status'" - echo "field — only 'enabled' (bool). Render sites derive the displayed" - echo "status from 'enabled' at the call site (see" - echo "web/src/pages/IssuersPage.tsx::issuerStatus). See unified-audit.md" - echo "diff-05x06-97fab8783a5c for closure rationale." - exit 1 - fi - - # D-2 Notification phantom-field check. - BAD_NOTIF=$(awk ' - /^export interface Notification \{/ { flag=1; next } - flag && /^\}/ { flag=0 } - flag { print FILENAME":"NR":"$0 } - ' web/src/api/types.ts \ - | grep -E '\bsubject\??\s*:' \ - || true) - if [ -n "$BAD_NOTIF" ]; then - echo "D-2 regression: Notification TS interface re-added a phantom 'subject' field:" - echo "$BAD_NOTIF" - echo "" - echo "The Go-side internal/domain/notification.go::NotificationEvent" - echo "has no 'subject' field — only 'message'. Pre-D-2 the consumer" - echo "at NotificationsPage.tsx had a dead '|| n.subject' fallback" - echo "that always fell through. See unified-audit.md" - echo "diff-05x06-caba9eb3620e for closure rationale." - exit 1 - fi - - - name: Forbidden client-side bulk-action loop regression guard (L-1) - # L-1 master closed cat-l-fa0c1ac07ab5 (bulk-renew loop) and - # cat-l-8a1fb258a38a (bulk-reassign loop) by adding server-side - # bulk endpoints (POST /api/v1/certificates/bulk-renew and - # POST /api/v1/certificates/bulk-reassign) that the GUI calls - # in a single round-trip. Pre-L-1 the GUI looped per-cert - # HTTP calls — 100 selected certs = 100 round-trips × ~50–200ms - # each = a 5–20-second wedge during which the operator stares - # at a progress bar. - # - # This step grep-fails the build if either loop shape reappears - # in CertificatesPage.tsx. Patterns catch the actual pre-L-1 - # shapes: - # - `for (const id of ids) { await triggerRenewal(id) }` - # - `for (const id of ids) { await updateCertificate(id, { owner_id }) }` - # - `for (let i = 0; i < ids.length; i++) { await triggerRenewal(ids[i]) }` - # - # Allowed: comment lines explaining the pre-L-1 pattern in the - # docblock above each handler. Test files (_test.tsx) exempt - # so negative-pattern tests can keep working. - # - # See coverage-gap-audit-2026-04-24-v5/unified-audit.md - # cat-l-fa0c1ac07ab5 and cat-l-8a1fb258a38a for closure - # rationale, or web/src/api/client.ts::bulkRenewCertificates - # / bulkReassignCertificates for the canonical call path. - run: | - set -e - - BAD_LOOP=$(grep -nE 'for[[:space:]]*\(' web/src/pages/CertificatesPage.tsx 2>/dev/null \ - | grep -E 'await[[:space:]]+(triggerRenewal|updateCertificate)\(' \ - | grep -v '\.test\.' \ - | grep -vE '^\s*[^:]+:[0-9]+:\s*//' \ - || true) - if [ -n "$BAD_LOOP" ]; then - echo "L-1 regression: client-side bulk-action loop reappeared in CertificatesPage.tsx:" - echo "$BAD_LOOP" - echo "" - echo "Use bulkRenewCertificates({ certificate_ids: [...] }) or" - echo "bulkReassignCertificates({ certificate_ids: [...], owner_id, team_id? })" - echo "instead of looping per-item HTTP calls. See" - echo "coverage-gap-audit-2026-04-24-v5/unified-audit.md cat-l-* for rationale." - exit 1 - fi - - - name: Forbidden orphan-CRUD client function regression guard (B-1) - # B-1 master closed four audit findings — three orphan-update fns - # (cat-b-31ceb6aaa9f1, cat-b-7a34f893a8f9) and one orphan CRUD - # surface (cat-b-4631ca092bee, RenewalPolicy) — by wiring per-page - # Edit modals so every backend write endpoint has at least one - # GUI consumer. The fourth finding (cat-b-9b97ffb35ef7) deleted - # the dead `exportCertificatePEM` duplicate. - # - # Pre-B-1 the failure mode was: backend ships a CRUD handler, - # client.ts ships the matching `update*` / `delete*` / `create*` - # function, but no page imports it. Operators were forced to - # `psql` directly to edit team names, owner emails, agent-group - # match rules, issuer names, profile names, or any renewal-policy - # field — turning a 30-second GUI task into a 30-minute database - # excursion with audit-trail gaps. - # - # This step fails the build if any of the eight previously-orphan - # client functions loses its page consumer (i.e. a future refactor - # accidentally re-orphans them). Each fn must have ≥1 non-test - # consumer under web/src/pages/. Tests (*.test.ts(x)) and the - # client.ts definition file itself are exempt. - # - # See coverage-gap-audit-2026-04-24-v5/unified-audit.md - # cat-b-31ceb6aaa9f1, cat-b-7a34f893a8f9, cat-b-4631ca092bee, - # cat-b-9b97ffb35ef7 for closure rationale. - run: | - set -e - ORPHAN_FNS="updateOwner updateTeam updateAgentGroup updateIssuer updateProfile createRenewalPolicy updateRenewalPolicy deleteRenewalPolicy" - FAIL=0 - for fn in $ORPHAN_FNS; do - HITS=$(grep -rE "\b${fn}\b" web/src/pages/ 2>/dev/null \ - | grep -vE '\.test\.(ts|tsx):' \ - | wc -l) - if [ "$HITS" -eq 0 ]; then - echo "::error::B-1 regression: client function '${fn}' has zero consumers under web/src/pages/." - echo " Every backend CRUD endpoint must have a GUI consumer to avoid forcing operators to psql." - echo " Either restore the page consumer or delete the client function in the same commit." - FAIL=1 - fi - done - # cat-b-9b97ffb35ef7: exportCertificatePEM was deleted as a dead - # duplicate of downloadCertificatePEM. Block resurrection. - if grep -nE 'export\s+const\s+exportCertificatePEM' web/src/api/client.ts >/dev/null 2>&1; then - echo "::error::B-1 regression: exportCertificatePEM was removed as a dead duplicate of downloadCertificatePEM." - echo " If a JSON variant is needed, add an explicit page consumer in the same commit." - FAIL=1 - fi - if [ "$FAIL" -ne 0 ]; then - exit 1 - fi - echo "B-1 orphan-CRUD client function guardrail: all 8 functions have page consumers." - - - name: Forbidden strings.Contains(err.Error()) regression guard (S-2) - # S-2 closure (cat-s6-efc7f6f6bd50): replaced 30 brittle - # substring-match error-dispatch sites in internal/api/handler/ - # with errors.Is + typed sentinels (repository.ErrNotFound, - # repository.ErrForeignKeyConstraint via the - # repository.IsForeignKeyError helper). This step grep-fails - # the build if any new strings.Contains(err.Error(), "not found") - # or strings.Contains(err.Error(), "violates foreign key") - # site appears under internal/api/handler/. - # - # Allowed: closure-comments documenting the convention (e.g. - # bulk_reassignment.go's "post-M-1 errToStatus convention" - # docblock); domain-specific substring patterns that are - # legitimately one-off ("cannot approve", "cannot reject", - # "cannot be parsed", "challenge password") — flagged as - # deferred follow-ups in the S-2 commit message. - # - # See coverage-gap-audit-2026-04-24-v5/unified-audit.md - # cat-s6-efc7f6f6bd50 for closure rationale. - run: | - set -e - BAD=$(grep -rnE 'strings\.Contains\(err\.Error\(\),\s*"(not found|violates foreign key|RESTRICT)"' internal/api/handler/ 2>/dev/null \ - | grep -vE '^\s*[^:]+:[0-9]+:\s*//' \ - || true) - if [ -n "$BAD" ]; then - echo "S-2 regression: brittle substring-match error-dispatch reappeared:" - echo "$BAD" - echo "" - echo "Use errors.Is(err, repository.ErrNotFound) for not-found dispatch," - echo "or repository.IsForeignKeyError(err) for FK violations." - echo "See coverage-gap-audit-2026-04-24-v5/unified-audit.md" - echo "cat-s6-efc7f6f6bd50 for closure rationale." - exit 1 - fi - echo "S-2 typed-sentinel error-dispatch guardrail: clean." - - name: Race Detection run: go test -race ./internal/service/... ./internal/api/handler/... ./internal/api/middleware/... ./internal/scheduler/... ./internal/connector/... ./internal/crypto/... ./internal/domain/... ./internal/validation/... ./internal/tlsprobe/... -count=1 -timeout 300s @@ -935,22 +315,24 @@ jobs: # Test__ form remains documented as # the recommended pattern for parameterized scenarios in # docs/qa-test-guide.md, but is not gated. - - name: Test-naming convention guard (hard-fail) + - name: Regression guards (extracted to scripts/ci-guards/) + # All named regression guards live at scripts/ci-guards/.sh per + # ci-pipeline-cleanup bundle Phase 1. Each guard is callable locally: + # bash scripts/ci-guards/G-3-env-docs-drift.sh + # Adding a new guard: drop a new .sh; this loop auto-picks it up. + # Contract: each guard MUST exit 0 on clean repo, non-zero with + # ::error:: prefix on regression. See scripts/ci-guards/README.md. run: | - # Catch tests Go itself would silently skip: `func TestX...` where - # the first letter after `Test` is lowercase. Go's testing runner - # requires uppercase to register the test; lowercase tests don't - # run, which is a real bug a CI guard should catch. - INVALID=$(grep -rnE '^func Test[a-z]' --include='*_test.go' . \ - | grep -v '_test.go.bak' \ - || true) - if [ -n "$INVALID" ]; then - echo "::error::Found tests Go would silently skip (lowercase after 'Test'):" - echo "$INVALID" - echo "Rename to start with an uppercase letter — Go's test runner only matches ^Test[A-Z]." - exit 1 - fi - echo "Test-naming convention guard: clean (no Go-invalid test names found)." + set -e + fail=0 + for g in scripts/ci-guards/*.sh; do + echo "::group::$(basename "$g")" + if ! bash "$g"; then + fail=1 + fi + echo "::endgroup::" + done + exit $fail frontend-build: name: Frontend Build @@ -979,337 +361,24 @@ jobs: working-directory: web run: npx vite build - - name: Forbidden hardcoded source-count prose regression guard (S-1) - # S-1 master closed cat-s1-9ce1cbe26876 (README + features.md - # stale numeric counts; explicit CLAUDE.md violation per - # "version-stamped numbers rot") and - # cat-s1-features_md_issuer_count_contradiction (features.md - # self-disagreed on issuer count: 9 vs 12 in the same doc). - # The fix replaced source-derived numbers in prose with - # "rebuild via " patterns documented in CLAUDE.md:: - # "Current-state commands". This step grep-fails the build if - # any of the previously-stale sites reintroduces a hardcoded - # count. - # - # Allowed surfaces: demo-fixture prose in README ("32 - # certificates" — those are seed_demo.sql facts, not live - # source counts), historical-milestone counts in - # WORKSPACE-CHANGELOG.md, the testing-guide example phrasing - # ("README claims 8 issuer connectors but only 6 exist"), - # and any number that quotes the source command immediately - # adjacent. - # - # See coverage-gap-audit-2026-04-24-v5/unified-audit.md - # cat-s1-9ce1cbe26876 + cat-s1-features_md_issuer_count_contradiction - # for closure rationale. + - name: Regression guards (extracted to scripts/ci-guards/) + # All named regression guards live at scripts/ci-guards/.sh per + # ci-pipeline-cleanup bundle Phase 1. Each guard is callable locally: + # bash scripts/ci-guards/G-3-env-docs-drift.sh + # Adding a new guard: drop a new .sh; this loop auto-picks it up. + # Contract: each guard MUST exit 0 on clean repo, non-zero with + # ::error:: prefix on regression. See scripts/ci-guards/README.md. run: | set -e - BAD=$(grep -rnE '\b[0-9]+\s+(issuer connectors?|target connectors?|notifier connectors?|discovery connectors?|MCP tools|OpenAPI operations|migrations|database tables|frontend pages|HTTP routes)\b' \ - README.md docs/ 2>/dev/null \ - | grep -vE 'WORKSPACE-CHANGELOG|seed_demo|demo override' \ - | grep -vE 'DRIFT HAZARD|Source: |Rebuild|rebuild via|grep -|wc -l|ls -d|find ' \ - | grep -vE 'README claims [0-9]+ issuer connectors but only [0-9]+ exist' \ - || true) - if [ -n "$BAD" ]; then - echo "S-1 regression: hardcoded source-count prose reappeared:" - echo "$BAD" - echo "" - echo "CLAUDE.md rule: 'Numeric claims about current state rot.'" - echo "Replace the count with the grep command from CLAUDE.md::" - echo "'Current-state commands' (e.g. 'ls -d internal/connector/issuer/*/ | wc -l')" - echo "or rephrase to reference the rebuild command on the same line." - echo "See coverage-gap-audit-2026-04-24-v5/unified-audit.md" - echo "cat-s1-9ce1cbe26876 for closure rationale." - exit 1 - fi - echo "S-1 stale-counts guardrail: clean." - - - name: Documented orphan client fns sync guard (P-1) - # P-1 master closed diff-04x03-d24864996ad4 + cat-b-dc46aadab98e - # by documenting 17 detail-page-candidate orphan client.ts - # functions in a docblock at the top of web/src/api/client.ts. - # This step verifies the docblock list ↔ export list relationship: - # every name listed in the docblock must still be declared as - # an export below it (catches drift where someone deletes the - # export but forgets the docblock, or vice versa). - # - # See coverage-gap-audit-2026-04-24-v5/unified-audit.md - # diff-04x03-d24864996ad4 + cat-b-dc46aadab98e for closure rationale. - run: | - set -e - # CRL/OCSP-Responder Phase 5 closed the getOCSPStatus orphan: the - # CertificateDetailPage Revocation Endpoints panel now consumes it - # ("Check OCSP status" button). Removed from the list to keep the - # docblock + guardrail honest. - DOCUMENTED='getAgentGroup getAgentGroupMembers getAuditEvent getCertificateDeployments getDiscoveredCertificate getHealthCheck getHealthCheckHistory getNetworkScanTarget getNotification getOwner getPolicy getPolicyViolations getRenewalPolicy getTeam registerAgent updateHealthCheck' - MISSING="" - for fn in $DOCUMENTED; do - if ! grep -qE "^export const ${fn}\b" web/src/api/client.ts; then - MISSING="${MISSING}${fn} " + fail=0 + for g in scripts/ci-guards/*.sh; do + echo "::group::$(basename "$g")" + if ! bash "$g"; then + fail=1 fi + echo "::endgroup::" done - if [ -n "$MISSING" ]; then - echo "P-1 regression: documented orphan(s) missing from client.ts exports:" - echo " $MISSING" - echo "" - echo "Either restore the export, or delete the corresponding line" - echo "in the documented-orphans docblock at the top of client.ts." - echo "See coverage-gap-audit-2026-04-24-v5/unified-audit.md" - echo "diff-04x03-d24864996ad4 for closure rationale." - exit 1 - fi - echo "P-1 documented-orphans sync guard: clean ($(echo $DOCUMENTED | wc -w) fns verified)." - - - name: Frontend page-coverage regression guard (T-1) - # T-1 closure (cat-s2-c24a548076c6): pre-T-1 only 3 of 28 pages - # had Vitest coverage. T-1 lifted that to 11/28 by writing tests - # for the 8 highest-leverage pages (CertificatesPage filter + - # pagination state, the new B-1 Edit modals, the D-2 type-trim - # render sites, etc.). The remaining pages are deferred to per- - # page commits — when the next feature change touches them, the - # test gets added in the same commit. This step blocks new - # pages from landing without tests. - # - # Allowlist: pages that are explicitly deferred — listed below - # with a one-line "why deferred" justification. Each entry must - # be removed when the page gets its test. - # - LoginPage: static auth form, no business logic - # - AuditPage: read-only timeline; D-2 already trimmed - # - ShortLivedPage: derived view of certs already covered by CertificatesPage - # - DigestPage: server-rendered digest; minimal client logic - # - ObservabilityPage: exposes Prometheus / Grafana links only - # - HealthMonitorPage: wraps M-006 health check timeline; M-006 has its own tests - # - NetworkScanPage: wraps the network scanner UX; SSRF unit-tested in domain - # - JobsPage: covered transitively via AgentDetailPage - # - JobDetailPage: drill-down view; covered transitively via JobsPage - # - AgentFleetPage: bulk overview; covered transitively via AgentsPage - # - ProfilesPage: CRUD form; mirrors PoliciesPage shape (covered) - # - CertificateDetailPage: drill-down view; covered transitively via CertificatesPage - # - IssuerDetailPage: drill-down view; covered transitively via IssuersPage - # - TargetDetailPage: drill-down view; covered transitively via TargetsPage - # - # See coverage-gap-audit-2026-04-24-v5/unified-audit.md - # cat-s2-c24a548076c6 for closure rationale. - run: | - set -e - ALLOW='^(LoginPage|AuditPage|ShortLivedPage|DigestPage|ObservabilityPage|HealthMonitorPage|NetworkScanPage|JobsPage|JobDetailPage|AgentFleetPage|ProfilesPage|CertificateDetailPage|IssuerDetailPage|TargetDetailPage)$' - UNTESTED="" - for f in web/src/pages/*.tsx; do - base=$(basename "$f" .tsx) - case "$f" in *.test.tsx) continue ;; esac - if [ -f "web/src/pages/${base}.test.tsx" ]; then continue; fi - if echo "$base" | grep -qE "$ALLOW"; then continue; fi - UNTESTED="${UNTESTED}${base} " - done - if [ -n "$UNTESTED" ]; then - echo "T-1 regression: page(s) without sibling .test.tsx and not on the deferred allowlist:" - echo " $UNTESTED" - echo "" - echo "Either add web/src/pages/.test.tsx (mirror NotificationsPage.test.tsx)," - echo "or add the page to the ALLOW pattern in .github/workflows/ci.yml with a" - echo "one-line 'why deferred' comment. See" - echo "coverage-gap-audit-2026-04-24-v5/unified-audit.md cat-s2-c24a548076c6" - echo "for closure rationale." - exit 1 - fi - ALLOWLIST_SIZE=$(echo "$ALLOW" | tr '|' '\n' | wc -l) - echo "T-1 page-coverage guardrail: clean (allowlist size: $ALLOWLIST_SIZE pages deferred)." - - - name: Bundle-8 / L-015 target=_blank rel=noopener regression guard - # Audit L-015 / CWE-1022 (reverse-tabnabbing): every - # MUST carry rel="noopener noreferrer" so a malicious page at the - # target URL cannot navigate the opener window via window.opener. - # At Bundle-8 close (commit b566355→) all 3 sites in the codebase - # already comply — this guard prevents regression. The - # ExternalLink component (web/src/components/ExternalLink.tsx) - # is the recommended way to add new external links. - # - # Test files (web/src/**/*.test.{ts,tsx}) are excluded so test - # docstrings or fixture data describing the attack vector by - # name don't trip the guard — symmetric with the L-019 guard. - run: | - set -e - OFFENDERS=$(grep -rnE 'target=["'"'"']?_blank["'"'"']?' web/src/ 2>/dev/null \ - | grep -v 'noopener noreferrer' \ - | grep -v 'web/src/components/ExternalLink.tsx' \ - | grep -vE '\.test\.(ts|tsx)(:[0-9]+)?:' \ - || true) - if [ -n "$OFFENDERS" ]; then - echo "L-015 regression: target=\"_blank\" without rel=\"noopener noreferrer\":" - echo "$OFFENDERS" - echo "" - echo "Either add rel=\"noopener noreferrer\" inline," - echo "or migrate to from web/src/components/ExternalLink.tsx." - exit 1 - fi - echo "L-015 target=_blank guardrail: clean." - - - name: Bundle-8 / L-019 dangerouslySetInnerHTML regression guard - # Audit L-019 / CWE-79 (XSS): no PRODUCTION code may use - # dangerouslySetInnerHTML directly. At Bundle-8 close the codebase - # has 0 sites; future genuine needs MUST route through - # web/src/utils/safeHtml.ts::sanitizeHtml. - # - # Test files (web/src/**/*.test.{ts,tsx}) are explicitly excluded: - # the M-029 Pass 3 XSS-hardening test docstrings legitimately cite - # the attack vector by name to explain what the test is guarding - # against (e.g. "a careless refactor to dangerouslySetInnerHTML - # would let an attacker-controlled CSR deliver an XSS payload"). - # Tests describing the threat aren't using it; the guard's intent - # is production code only. - run: | - set -e - OFFENDERS=$(grep -rnE 'dangerouslySetInnerHTML' web/src/ 2>/dev/null \ - | grep -v 'web/src/utils/safeHtml.ts' \ - | grep -vE '\.test\.(ts|tsx)(:[0-9]+)?:' \ - || true) - if [ -n "$OFFENDERS" ]; then - echo "L-019 regression: dangerouslySetInnerHTML used outside safeHtml.ts:" - echo "$OFFENDERS" - echo "" - echo "Route through web/src/utils/safeHtml.ts::sanitizeHtml — see file" - echo "header for the activation procedure (DOMPurify dependency)." - exit 1 - fi - echo "L-019 dangerouslySetInnerHTML guardrail: clean." - - - name: Bundle-8 / M-009 + M-029 Pass 1 mutation contract guard (hard zero) - # Audit M-009 + M-029 Pass 1 closure: - # - # Pre-Bundle-8 the codebase had 56 bare useMutation sites with - # discretionary invalidation. Bundle 8 shipped the useTrackedMutation - # wrapper (web/src/hooks/useTrackedMutation.ts) that requires every - # caller to declare `invalidates: QueryKey[] | 'noop'`. M-029 Pass 1 - # then migrated all 56 sites to the wrapper across 6 batches. - # - # This guard pins the contract going forward: every useMutation call - # in src/ MUST be inside useTrackedMutation.ts (the wrapper itself - # is the only legitimate caller of useMutation). Any bare useMutation - # call elsewhere is a regression — adding a new mutation site means - # going through the wrapper so the invalidates contract is enforced - # per-site, not by a soft budget guard. - # - # If you genuinely need raw useMutation (extremely unlikely — the - # wrapper supports invalidates: 'noop' for fire-and-forget mutations), - # update this guard's exclusion list and document the carve-out. - run: | - set -e - # Test files (web/src/**/*.test.{ts,tsx}) are excluded so existing - # useMutation-mocking test patterns and the wrapper's own unit - # tests don't trip the production guard — symmetric with L-015 - # and L-019 above. - BARE=$(grep -rnE '\buseMutation\(' web/src/ 2>/dev/null \ - | grep -v 'web/src/hooks/useTrackedMutation\.ts' \ - | grep -vE '\.test\.(ts|tsx)(:[0-9]+)?:' \ - || true) - if [ -n "$BARE" ]; then - echo "M-009 hard-zero regression: bare useMutation() call(s) outside the wrapper:" - echo "$BARE" - echo - echo "Every mutation must go through useTrackedMutation" - echo "(web/src/hooks/useTrackedMutation.ts) with explicit" - echo "invalidates: QueryKey[] | 'noop'. See file header for usage." - exit 1 - fi - # Sanity counts (informational, not a gate). - TRACKED=$(grep -rcE '\buseTrackedMutation\(' web/src/ 2>/dev/null | awk -F: '{s+=$2} END{print s}') - INVALIDATIONS=$(grep -rcE 'invalidateQueries|setQueryData|removeQueries|invalidates:' web/src/ 2>/dev/null | awk -F: '{s+=$2} END{print s}') - echo "M-009 hard-zero: bare useMutation sites = 0 (wrapper-internal call + test files excluded)." - echo "M-009 informational: useTrackedMutation sites = $TRACKED; invalidation surface = $INVALIDATIONS." - - - name: Forbidden env-var docs drift regression guard (G-3) - # G-3 master closed cat-g-163dae19bc59 (docs-only env vars - # phantom in features.md), cat-g-b8f8f8796159 (6 config-only - # env vars never documented), and cat-g-renewal_check_interval_rename_drift - # (features.md still advertised the pre-rename - # CERTCTL_RENEWAL_CHECK_INTERVAL after it was renamed to - # CERTCTL_SCHEDULER_RENEWAL_CHECK_INTERVAL). This step runs - # `comm -23` both ways between the env vars defined in Go - # source (config.go + cmd/agent + deploy/test fixtures + ACME - # DNS-01 script env exports) and the env vars mentioned in - # README + docs/ + deploy/helm/. - # - # Allowlist: env vars that are documented as integration- - # surface contracts (script env exports for ACME DNS-01, - # OpenSSL CA scripts, StepCA per-issuer-config-blob fields, - # Webhook per-notifier-config-blob fields, ACME EAB, audit - # exclusion, demo-stack overrides) but not consumed directly - # by config.go. Each entry below has a one-line justification - # — if you add a new entry, add the justification too. - # - # See coverage-gap-audit-2026-04-24-v5/unified-audit.md - # cat-g-* for closure rationale. - run: | - set -e - # Defined: config.go + agent + cli + mcp-server + server cmds + test fixtures + ACME DNS export - { - grep -nE '"CERTCTL_[A-Z_]+"' internal/config/config.go | sed -E 's/.*"(CERTCTL_[A-Z_]+)".*/\1/' - grep -rhoE '"CERTCTL_[A-Z_]+"' cmd/agent/*.go cmd/cli/*.go cmd/mcp-server/*.go cmd/server/*.go 2>/dev/null | sed -E 's/"(CERTCTL_[A-Z_]+)"/\1/' - grep -rhoE 'CERTCTL_[A-Z_]+' deploy/test/qa_test.go internal/connector/issuer/acme/dns.go 2>/dev/null - } | grep -E '^CERTCTL_' | sort -u > /tmp/g3-defined.txt - # Documented: README + docs + helm - grep -rhoE '\bCERTCTL_[A-Z_]+\b' README.md docs/ deploy/helm/ 2>/dev/null | sort -u > /tmp/g3-docs.txt - # Allowlist of env vars documented as external integration contracts. - # Each entry justifies itself in one line; if you add to this list, - # add the justification. - ALLOWED='^( - CERTCTL_OPENSSL_SIGN_SCRIPT| - CERTCTL_OPENSSL_REVOKE_SCRIPT| - CERTCTL_OPENSSL_CRL_SCRIPT| - CERTCTL_OPENSSL_TIMEOUT_SECONDS| - CERTCTL_STEPCA_URL| - CERTCTL_STEPCA_FINGERPRINT| - CERTCTL_STEPCA_PROVISIONER| - CERTCTL_STEPCA_PROVISIONER_NAME| - CERTCTL_STEPCA_PROVISIONER_KEY| - CERTCTL_STEPCA_PROVISIONER_JWK| - CERTCTL_STEPCA_PROVISIONER_PASSWORD| - CERTCTL_STEPCA_PASSWORD| - CERTCTL_STEPCA_KEY_PATH| - CERTCTL_STEPCA_ROOT_CA| - CERTCTL_WEBHOOK_URL| - CERTCTL_WEBHOOK_SECRET| - CERTCTL_ACME_EAB_KID| - CERTCTL_ACME_EAB_HMAC| - CERTCTL_ACME_DNS_PROPAGATION_WAIT| - CERTCTL_AUDIT_EXCLUDE_PATHS| - CERTCTL_TLS_| - CERTCTL_TLS_INSECURE_SKIP_VERIFY| - CERTCTL_SCEP_| - CERTCTL_SERVER_CA_BUNDLE_PATH| - CERTCTL_SERVER_TLS_INSECURE_SKIP_VERIFY| - CERTCTL_QA_[A-Z_]+ - )$' - # ^ The CERTCTL_OPENSSL_* / CERTCTL_STEPCA_* / CERTCTL_WEBHOOK_* / - # CERTCTL_ACME_EAB_* / CERTCTL_ACME_DNS_PROPAGATION_WAIT / - # CERTCTL_AUDIT_EXCLUDE_PATHS / CERTCTL_TLS_* / CERTCTL_SERVER_* / - # CERTCTL_QA_* sets are documented integration-surface contracts - # (script invocations, per-issuer config-blob field names, - # per-notifier config-blob field names, demo-stack overrides, - # test fixtures) — not server-side env vars in config.go. - # The audit's "37 docs-only" count over-flagged these; the - # closure narrows the gate to the specific drift sites - # (renewal-interval rename + 6 config-only) and allowlists - # the documented external contracts here. - ALLOWED_FLAT=$(echo "$ALLOWED" | tr -d '\n ') - DOCS_ONLY=$(comm -13 /tmp/g3-defined.txt /tmp/g3-docs.txt | grep -vE "$ALLOWED_FLAT" || true) - CONFIG_ONLY=$(comm -23 /tmp/g3-defined.txt /tmp/g3-docs.txt || true) - if [ -n "$DOCS_ONLY" ]; then - echo "G-3 regression: env var(s) mentioned in docs but not defined in Go source AND not in the documented integration-surface allowlist:" - echo "$DOCS_ONLY" - echo "" - echo "Either delete from docs (phantom/typo) or add to config.go," - echo "or add to the ALLOWED list with a one-line justification." - exit 1 - fi - if [ -n "$CONFIG_ONLY" ]; then - echo "G-3 regression: env var(s) defined in Go source but never documented:" - echo "$CONFIG_ONLY" - echo "" - echo "Add an entry to docs/features.md (or another canonical doc) so operators can find it." - exit 1 - fi - echo "G-3 env-var docs drift guardrail: clean." + exit $fail helm-lint: name: Helm Chart Validation diff --git a/scripts/ci-guards/B-1-orphan-crud.sh b/scripts/ci-guards/B-1-orphan-crud.sh new file mode 100755 index 0000000..fe99391 --- /dev/null +++ b/scripts/ci-guards/B-1-orphan-crud.sh @@ -0,0 +1,53 @@ +#!/usr/bin/env bash +# scripts/ci-guards/B-1-orphan-crud.sh +# +# B-1 master closed four audit findings — three orphan-update fns +# (cat-b-31ceb6aaa9f1, cat-b-7a34f893a8f9) and one orphan CRUD +# surface (cat-b-4631ca092bee, RenewalPolicy) — by wiring per-page +# Edit modals so every backend write endpoint has at least one +# GUI consumer. The fourth finding (cat-b-9b97ffb35ef7) deleted +# the dead `exportCertificatePEM` duplicate. +# +# Pre-B-1 the failure mode was: backend ships a CRUD handler, +# client.ts ships the matching `update*` / `delete*` / `create*` +# function, but no page imports it. Operators were forced to +# `psql` directly to edit team names, owner emails, agent-group +# match rules, issuer names, profile names, or any renewal-policy +# field — turning a 30-second GUI task into a 30-minute database +# excursion with audit-trail gaps. +# +# This script fails the build if any of the eight previously-orphan +# client functions loses its page consumer (i.e. a future refactor +# accidentally re-orphans them). Each fn must have ≥1 non-test +# consumer under web/src/pages/. Tests (*.test.ts(x)) and the +# client.ts definition file itself are exempt. +# +# See coverage-gap-audit-2026-04-24-v5/unified-audit.md +# cat-b-31ceb6aaa9f1, cat-b-7a34f893a8f9, cat-b-4631ca092bee, +# cat-b-9b97ffb35ef7 for closure rationale. + +set -e +ORPHAN_FNS="updateOwner updateTeam updateAgentGroup updateIssuer updateProfile createRenewalPolicy updateRenewalPolicy deleteRenewalPolicy" +FAIL=0 +for fn in $ORPHAN_FNS; do + HITS=$(grep -rE "\b${fn}\b" web/src/pages/ 2>/dev/null \ + | grep -vE '\.test\.(ts|tsx):' \ + | wc -l) + if [ "$HITS" -eq 0 ]; then + echo "::error::B-1 regression: client function '${fn}' has zero consumers under web/src/pages/." + echo " Every backend CRUD endpoint must have a GUI consumer to avoid forcing operators to psql." + echo " Either restore the page consumer or delete the client function in the same commit." + FAIL=1 + fi +done +# cat-b-9b97ffb35ef7: exportCertificatePEM was deleted as a dead +# duplicate of downloadCertificatePEM. Block resurrection. +if grep -nE 'export\s+const\s+exportCertificatePEM' web/src/api/client.ts >/dev/null 2>&1; then + echo "::error::B-1 regression: exportCertificatePEM was removed as a dead duplicate of downloadCertificatePEM." + echo " If a JSON variant is needed, add an explicit page consumer in the same commit." + FAIL=1 +fi +if [ "$FAIL" -ne 0 ]; then + exit 1 +fi +echo "B-1 orphan-CRUD: clean (all 8 functions have page consumers)." diff --git a/scripts/ci-guards/D-1-D-2-statusbadge-phantom.sh b/scripts/ci-guards/D-1-D-2-statusbadge-phantom.sh new file mode 100755 index 0000000..9d478cd --- /dev/null +++ b/scripts/ci-guards/D-1-D-2-statusbadge-phantom.sh @@ -0,0 +1,154 @@ +#!/usr/bin/env bash +# scripts/ci-guards/D-1-D-2-statusbadge-phantom.sh +# +# D-1 master closed cat-d-359e92c20cbf (Agent: 'Stale' dead key, +# 'Degraded' missing), cat-d-9f4c8e4a91f1 (Notification: 'dead' +# missing), cat-d-1447e04732e7 (Cert: 'PendingIssuance' dead +# key), cat-f-cert_detail_page_key_render_fallback (render-site +# uses cert.X directly), and cat-f-ae0d06b6588f (Certificate +# TS phantom fields). This script grep-fails the build if either +# half of the closure is reverted: +# +# 1. The dead StatusBadge keys ('Stale' for Agent, 'PendingIssuance' +# for Cert) reappearing as map literals, OR +# 2. The five phantom Certificate TS fields (serial_number, +# fingerprint_sha256, key_algorithm, key_size, issued_at) +# reappearing on the `Certificate` interface in types.ts +# (CertificateVersion legitimately carries them and is +# explicitly excluded by the awk pre-filter below). +# +# Comments are exempt so the closure prose in StatusBadge.tsx + +# types.ts can stay. Test files are exempt so negative tests +# asserting the dead keys fall through to neutral keep working. +# +# See coverage-gap-audit-2026-04-24-v5/unified-audit.md +# cat-d-* / cat-f-* for the closure rationale, or +# web/src/components/StatusBadge.test.tsx for the live +# enum-coverage contract. + +set -e + +BAD_BADGE=$(grep -nE "^\s*(Stale|PendingIssuance)\s*:\s*'badge-" \ + web/src/components/StatusBadge.tsx 2>/dev/null \ + | grep -v '\.test\.' \ + | grep -vE '^\s*[^:]+:[0-9]+:\s*//' \ + || true) +if [ -n "$BAD_BADGE" ]; then + echo "::error::D-1 regression: dead StatusBadge key reappeared:" + echo "$BAD_BADGE" + echo "" + echo "Allowed surface: comment lines naming the removed key in" + echo "the file's preamble. The Go-side AgentStatus values are" + echo "Online/Offline/Degraded (no Stale); CertificateStatus values" + echo "are Pending/Active/... (no PendingIssuance). See" + echo "web/src/components/StatusBadge.test.tsx for the contract." + exit 1 +fi + +# Certificate TS phantom-field check. Scoped to the +# `export interface Certificate {` block in web/src/api/types.ts +# — CertificateVersion legitimately declares these fields and +# must NOT trip the guardrail. The awk window opens on the +# exact `Certificate {` header (not `CertificateVersion {`, +# not `CertificateProfile {`) and closes at the first `}`, +# then the grep matches a phantom-field declaration anywhere +# in that window. +BAD_TS=$(awk ' + /^export interface Certificate \{/ { flag=1; next } + flag && /^\}/ { flag=0 } + flag { print FILENAME":"NR":"$0 } +' web/src/api/types.ts \ + | grep -E '\b(serial_number|fingerprint_sha256|key_algorithm|key_size|issued_at)\??\s*:' \ + || true) +if [ -n "$BAD_TS" ]; then + echo "::error::D-1 regression: Certificate TS interface re-added a phantom field:" + echo "$BAD_TS" + echo "" + echo "These fields live on CertificateVersion, not ManagedCertificate." + echo "The Go-side ManagedCertificate has never carried them; the" + echo "TS optional declarations were silently undefined on every" + echo "list response. Render-site consumers (e.g. CertificateDetailPage)" + echo "use latestVersion?.field as the canonical access path." + echo "See coverage-gap-audit-2026-04-24-v5/unified-audit.md" + echo "cat-f-ae0d06b6588f for the closure rationale." + exit 1 +fi + +# D-2 master closed five diff-05x06-* type-drift findings: +# Agent (5 phantoms), Issuer (1 phantom), Notification (1 phantom) +# — TRIM half. The Target (2 missing fields) and DiscoveredCertificate +# (1 missing field) — ADD half is pinned by the literal-construction +# blocks in web/src/api/types.test.ts, not a CI grep. The phantom- +# trim regression vector is an awk-windowed grep per interface +# mirroring the D-1 Certificate check above. +# +# See coverage-gap-audit-2026-04-24-v5/unified-audit.md +# diff-05x06-7cdf4e78ae24 (Agent), diff-05x06-97fab8783a5c (Issuer), +# diff-05x06-caba9eb3620e (Notification) for the closure rationale. + +# D-2 Agent phantom-field check. The grep matches `last_heartbeat` +# but NOT `last_heartbeat_at` (the legitimate Go-emitted field) — +# the `\b...\b` boundaries plus the `grep -v 'last_heartbeat_at'` +# filter handle that. +BAD_AGENT=$(awk ' + /^export interface Agent \{/ { flag=1; next } + flag && /^\}/ { flag=0 } + flag { print FILENAME":"NR":"$0 } +' web/src/api/types.ts \ + | grep -E '\b(last_heartbeat|capabilities|tags|created_at|updated_at)\??\s*:' \ + | grep -v 'last_heartbeat_at' \ + || true) +if [ -n "$BAD_AGENT" ]; then + echo "::error::D-2 regression: Agent TS interface re-added a phantom field:" + echo "$BAD_AGENT" + echo "" + echo "The Go-side internal/domain/connector.go::Agent emits exactly:" + echo "id, name, hostname, status, last_heartbeat_at?, registered_at," + echo "os, architecture, ip_address, version, retired_at?, retired_reason?." + echo "The five fields blocked by this guard (last_heartbeat," + echo "capabilities, tags, created_at, updated_at) were TS phantoms" + echo "the Go struct never emitted. See unified-audit.md" + echo "diff-05x06-7cdf4e78ae24 for closure rationale." + exit 1 +fi + +# D-2 Issuer phantom-field check. +BAD_ISSUER=$(awk ' + /^export interface Issuer \{/ { flag=1; next } + flag && /^\}/ { flag=0 } + flag { print FILENAME":"NR":"$0 } +' web/src/api/types.ts \ + | grep -E '\bstatus\??\s*:' \ + || true) +if [ -n "$BAD_ISSUER" ]; then + echo "::error::D-2 regression: Issuer TS interface re-added a phantom 'status' field:" + echo "$BAD_ISSUER" + echo "" + echo "The Go-side internal/domain/connector.go::Issuer has no 'status'" + echo "field — only 'enabled' (bool). Render sites derive the displayed" + echo "status from 'enabled' at the call site (see" + echo "web/src/pages/IssuersPage.tsx::issuerStatus). See unified-audit.md" + echo "diff-05x06-97fab8783a5c for closure rationale." + exit 1 +fi + +# D-2 Notification phantom-field check. +BAD_NOTIF=$(awk ' + /^export interface Notification \{/ { flag=1; next } + flag && /^\}/ { flag=0 } + flag { print FILENAME":"NR":"$0 } +' web/src/api/types.ts \ + | grep -E '\bsubject\??\s*:' \ + || true) +if [ -n "$BAD_NOTIF" ]; then + echo "::error::D-2 regression: Notification TS interface re-added a phantom 'subject' field:" + echo "$BAD_NOTIF" + echo "" + echo "The Go-side internal/domain/notification.go::NotificationEvent" + echo "has no 'subject' field — only 'message'. Pre-D-2 the consumer" + echo "at NotificationsPage.tsx had a dead '|| n.subject' fallback" + echo "that always fell through. See unified-audit.md" + echo "diff-05x06-caba9eb3620e for closure rationale." + exit 1 +fi +echo "D-1 + D-2 statusbadge-phantom: clean." diff --git a/scripts/ci-guards/G-1-jwt-auth-literal.sh b/scripts/ci-guards/G-1-jwt-auth-literal.sh new file mode 100755 index 0000000..575d645 --- /dev/null +++ b/scripts/ci-guards/G-1-jwt-auth-literal.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +# scripts/ci-guards/G-1-jwt-auth-literal.sh +# +# G-1 closed the JWT silent auth downgrade by removing "jwt" from the +# accepted CERTCTL_AUTH_TYPE values. This script grep-fails the build +# if "jwt" reappears in any of the *additive* auth-type surfaces: +# the validAuthTypes / ValidAuthTypes() set, the OpenAPI enum, the +# helm chart's allowed-types list, or the .env.example default. +# Comment lines and the dedicated rejection branch in config.go +# (`c.Auth.Type == "jwt"`) are intentionally exempt — those are the +# G-1 fix itself, not a regression. +# +# Connector packages (internal/connector/) are exempt because the +# Google OAuth2 service-account JWT and step-ca provisioner one- +# time-token JWT are external-protocol uses, unrelated to certctl's +# own auth shape. Test files (_test.go) are exempt so negative +# tests can pass the literal. +# +# See docs/upgrade-to-v2-jwt-removal.md for the closure rationale, +# or internal/config/config.go::ValidAuthTypes for the allowed set. + +set -e + +# Scoped patterns that indicate "jwt" being added back to an +# allowed-set surface. Each catches a regression shape we've +# actually seen in pre-G-1 code: +# - Go map/slice literal: "jwt": true or "jwt", +# - Go switch case: case "jwt" +# - YAML enum: enum: [..., jwt, ...] or - jwt +# - .env conditional: AUTH_TYPE.*"jwt"|=jwt$ +BAD=$(grep -rnEH \ + -e '"jwt"\s*:\s*true' \ + -e '"jwt"\s*,' \ + -e 'case\s+"jwt"' \ + -e 'enum:.*\bjwt\b' \ + -e '^\s*-\s*jwt\s*$' \ + -e 'AUTH_TYPE\s*=\s*jwt\s*$' \ + -e 'AUTH_TYPE\s*=\s*jwt\s*#' \ + -e 'auth\.type\s*=\s*jwt\s*$' \ + -e 'AuthType\("jwt"\)' \ + internal/config/ \ + internal/api/ \ + cmd/ \ + api/openapi.yaml \ + .env.example \ + deploy/.env.example \ + deploy/helm/certctl/values.yaml \ + deploy/helm/certctl/templates/ \ + 2>/dev/null \ + | grep -v '_test.go' \ + | grep -vE '^\s*[^:]+:[0-9]+:\s*(//|#)' \ + | grep -v 'is no longer accepted' \ + || true) +if [ -n "$BAD" ]; then + echo "::error::G-1 regression: \"jwt\" reappeared in an allowed-set surface:" + echo "$BAD" + echo "" + echo "Allowed surface for 'jwt' literals: comment lines, the" + echo "dedicated rejection branch in internal/config/config.go," + echo "and connector packages (Google OAuth2, step-ca)." + echo "See docs/upgrade-to-v2-jwt-removal.md and" + echo "internal/config/config.go::ValidAuthTypes()." + exit 1 +fi +echo "G-1 jwt-auth-literal: clean." diff --git a/scripts/ci-guards/G-2-api-key-hash-json.sh b/scripts/ci-guards/G-2-api-key-hash-json.sh new file mode 100755 index 0000000..8271cca --- /dev/null +++ b/scripts/ci-guards/G-2-api-key-hash-json.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash +# scripts/ci-guards/G-2-api-key-hash-json.sh +# +# G-2 closed cat-s5-apikey_leak by tagging Agent.APIKeyHash +# `json:"-"` and adding a defense-in-depth Agent.MarshalJSON that +# zeroes the field on the marshal-time copy. This script grep-fails +# the build if `api_key_hash` reappears in any of the *additive* +# JSON-emitting surfaces: a Go struct json tag in internal/domain/, +# an OpenAPI Agent schema property, a TypeScript field declaration +# in web/src/, or an enum-list / discriminator in handler +# production code. +# +# Repository, migration, seed, service, integration-test, and +# unit-test files are exempt — those are server-internal use +# sites (the DB column stays, the in-memory struct field stays, +# the auth-lookup path stays). Comment lines are exempt so the +# G-2 closure rationale can stay in the source. +# +# See coverage-gap-audit-2026-04-24-v5/unified-audit.md +# cat-s5-apikey_leak for the closure rationale, or +# internal/domain/connector.go::Agent::MarshalJSON for the +# redaction enforcement. + +set -e + +# Scoped patterns that indicate api_key_hash being added back +# to a JSON-emitting surface. Each catches a regression shape +# that pre-G-2 actually shipped or that a future refactor +# could plausibly introduce: +# - Go struct tag: `json:"api_key_hash"` +# - Frontend interface: api_key_hash[?]: string +# - OpenAPI schema property: api_key_hash: (column-aligned) +# - YAML enum / array: - api_key_hash +BAD=$(grep -rnEH \ + -e 'json:"api_key_hash[",]' \ + -e '^\s*api_key_hash\??\s*:' \ + -e '^\s*-\s*api_key_hash\s*$' \ + internal/domain/ \ + internal/api/ \ + cmd/ \ + api/openapi.yaml \ + web/src/ \ + 2>/dev/null \ + | grep -v '_test.go' \ + | grep -vE '^\s*[^:]+:[0-9]+:\s*(//|#)' \ + || true) +if [ -n "$BAD" ]; then + echo "::error::G-2 regression: api_key_hash reappeared in a JSON-emitting surface:" + echo "$BAD" + echo "" + echo "Allowed surface for api_key_hash literals: comment lines," + echo "the database column (migrations/), the in-memory struct" + echo "field tagged \`json:\"-\"\`, and the repository / service" + echo "use sites. See internal/domain/connector.go::Agent and" + echo "coverage-gap-audit-2026-04-24-v5/unified-audit.md" + echo "cat-s5-apikey_leak for the closure rationale." + exit 1 +fi +echo "G-2 api-key-hash-json: clean." diff --git a/scripts/ci-guards/G-3-env-docs-drift.sh b/scripts/ci-guards/G-3-env-docs-drift.sh new file mode 100755 index 0000000..b629006 --- /dev/null +++ b/scripts/ci-guards/G-3-env-docs-drift.sh @@ -0,0 +1,95 @@ +#!/usr/bin/env bash +# scripts/ci-guards/G-3-env-docs-drift.sh +# +# G-3 master closed cat-g-163dae19bc59 (docs-only env vars +# phantom in features.md), cat-g-b8f8f8796159 (6 config-only +# env vars never documented), and cat-g-renewal_check_interval_rename_drift +# (features.md still advertised the pre-rename +# CERTCTL_RENEWAL_CHECK_INTERVAL after it was renamed to +# CERTCTL_SCHEDULER_RENEWAL_CHECK_INTERVAL). This script runs +# `comm -23` both ways between the env vars defined in Go +# source (config.go + cmd/agent + deploy/test fixtures + ACME +# DNS-01 script env exports) and the env vars mentioned in +# README + docs/ + deploy/helm/. +# +# Allowlist: env vars that are documented as integration- +# surface contracts (script env exports for ACME DNS-01, +# OpenSSL CA scripts, StepCA per-issuer-config-blob fields, +# Webhook per-notifier-config-blob fields, ACME EAB, audit +# exclusion, demo-stack overrides) but not consumed directly +# by config.go. Each entry below has a one-line justification +# — if you add a new entry, add the justification too. +# +# See coverage-gap-audit-2026-04-24-v5/unified-audit.md +# cat-g-* for closure rationale. + +set -e +# Defined: config.go + agent + cli + mcp-server + server cmds + test fixtures + ACME DNS export +{ + grep -nE '"CERTCTL_[A-Z_]+"' internal/config/config.go | sed -E 's/.*"(CERTCTL_[A-Z_]+)".*/\1/' + grep -rhoE '"CERTCTL_[A-Z_]+"' cmd/agent/*.go cmd/cli/*.go cmd/mcp-server/*.go cmd/server/*.go 2>/dev/null | sed -E 's/"(CERTCTL_[A-Z_]+)"/\1/' + grep -rhoE 'CERTCTL_[A-Z_]+' deploy/test/qa_test.go internal/connector/issuer/acme/dns.go 2>/dev/null +} | grep -E '^CERTCTL_' | sort -u > /tmp/g3-defined.txt +# Documented: README + docs + helm +grep -rhoE '\bCERTCTL_[A-Z_]+\b' README.md docs/ deploy/helm/ 2>/dev/null | sort -u > /tmp/g3-docs.txt +# Allowlist of env vars documented as external integration contracts. +# Each entry justifies itself in one line; if you add to this list, +# add the justification. +ALLOWED='^( +CERTCTL_OPENSSL_SIGN_SCRIPT| +CERTCTL_OPENSSL_REVOKE_SCRIPT| +CERTCTL_OPENSSL_CRL_SCRIPT| +CERTCTL_OPENSSL_TIMEOUT_SECONDS| +CERTCTL_STEPCA_URL| +CERTCTL_STEPCA_FINGERPRINT| +CERTCTL_STEPCA_PROVISIONER| +CERTCTL_STEPCA_PROVISIONER_NAME| +CERTCTL_STEPCA_PROVISIONER_KEY| +CERTCTL_STEPCA_PROVISIONER_JWK| +CERTCTL_STEPCA_PROVISIONER_PASSWORD| +CERTCTL_STEPCA_PASSWORD| +CERTCTL_STEPCA_KEY_PATH| +CERTCTL_STEPCA_ROOT_CA| +CERTCTL_WEBHOOK_URL| +CERTCTL_WEBHOOK_SECRET| +CERTCTL_ACME_EAB_KID| +CERTCTL_ACME_EAB_HMAC| +CERTCTL_ACME_DNS_PROPAGATION_WAIT| +CERTCTL_AUDIT_EXCLUDE_PATHS| +CERTCTL_TLS_| +CERTCTL_TLS_INSECURE_SKIP_VERIFY| +CERTCTL_SCEP_| +CERTCTL_SERVER_CA_BUNDLE_PATH| +CERTCTL_SERVER_TLS_INSECURE_SKIP_VERIFY| +CERTCTL_QA_[A-Z_]+ +)$' +# ^ The CERTCTL_OPENSSL_* / CERTCTL_STEPCA_* / CERTCTL_WEBHOOK_* / +# CERTCTL_ACME_EAB_* / CERTCTL_ACME_DNS_PROPAGATION_WAIT / +# CERTCTL_AUDIT_EXCLUDE_PATHS / CERTCTL_TLS_* / CERTCTL_SERVER_* / +# CERTCTL_QA_* sets are documented integration-surface contracts +# (script invocations, per-issuer config-blob field names, +# per-notifier config-blob field names, demo-stack overrides, +# test fixtures) — not server-side env vars in config.go. +# The audit's "37 docs-only" count over-flagged these; the +# closure narrows the gate to the specific drift sites +# (renewal-interval rename + 6 config-only) and allowlists +# the documented external contracts here. +ALLOWED_FLAT=$(echo "$ALLOWED" | tr -d '\n ') +DOCS_ONLY=$(comm -13 /tmp/g3-defined.txt /tmp/g3-docs.txt | grep -vE "$ALLOWED_FLAT" || true) +CONFIG_ONLY=$(comm -23 /tmp/g3-defined.txt /tmp/g3-docs.txt || true) +if [ -n "$DOCS_ONLY" ]; then + echo "::error::G-3 regression: env var(s) mentioned in docs but not defined in Go source AND not in the documented integration-surface allowlist:" + echo "$DOCS_ONLY" + echo "" + echo "Either delete from docs (phantom/typo) or add to config.go," + echo "or add to the ALLOWED list with a one-line justification." + exit 1 +fi +if [ -n "$CONFIG_ONLY" ]; then + echo "::error::G-3 regression: env var(s) defined in Go source but never documented:" + echo "$CONFIG_ONLY" + echo "" + echo "Add an entry to docs/features.md (or another canonical doc) so operators can find it." + exit 1 +fi +echo "G-3 env-docs-drift: clean." diff --git a/scripts/ci-guards/H-001-bare-from.sh b/scripts/ci-guards/H-001-bare-from.sh new file mode 100755 index 0000000..237a67d --- /dev/null +++ b/scripts/ci-guards/H-001-bare-from.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +# scripts/ci-guards/H-001-bare-from.sh +# +# Bundle A / Audit H-001 (CWE-829): every FROM line in every +# Dockerfile in the repo MUST carry an @sha256:... digest pin in +# addition to the human-readable tag. A registry-side tag swap +# cannot then change what we pull. This script grep-fails the +# build if any new FROM lands without the @sha256 suffix. +# +# Companion check: digest-validity.sh (added by ci-pipeline-cleanup +# Phase 7) verifies that each digest actually resolves on its +# registry — H-001 is presence-only. + +set -e +# Match any "FROM image[:tag]" that does NOT contain @sha256. +# Strip comments and blank lines defensively. +BAD=$(find . -name 'Dockerfile*' -not -path './web/node_modules/*' \ + -exec grep -HnE '^FROM\s+[^@#]+(\s+AS\s+\S+)?\s*$' {} \; || true) +if [ -n "$BAD" ]; then + echo "::error::H-001 regression: Dockerfile has bare FROM (no @sha256 digest pin):" + echo "$BAD" + echo "" + echo "Pin every FROM to an immutable digest. See the bump" + echo "procedure in Dockerfile's header comment (Bundle A / H-001)." + exit 1 +fi +echo "H-001 bare-from: clean." diff --git a/scripts/ci-guards/H-009-readme-jwt.sh b/scripts/ci-guards/H-009-readme-jwt.sh new file mode 100755 index 0000000..dcea6d3 --- /dev/null +++ b/scripts/ci-guards/H-009-readme-jwt.sh @@ -0,0 +1,28 @@ +#!/usr/bin/env bash +# scripts/ci-guards/H-009-readme-jwt.sh +# +# H-009 closed by Bundle D as verified-already-clean: at audit time +# the README does NOT advertise JWT support (certctl does not ship +# in-process JWT middleware; JWT/OIDC integration is via an +# authenticating gateway, see docs/architecture.md "Authenticating- +# gateway pattern"). This script grep-fails the build if README ever +# re-introduces a sentence advertising JWT as a supported auth mode. +# Pattern: "JWT" within ~6 words of "support|auth|enabled|mode" in +# README.md. The architecture / compliance / connector docs that +# legitimately mention JWT (Google OAuth2 service-account JWT, +# step-ca provisioner JWT, JWT-via-gateway pattern) are out of +# scope — they describe what certctl does NOT do, or external +# protocol uses. + +set -e +if grep -inE 'JWT.{0,40}(support|auth|enabled|mode|provider)' README.md \ + | grep -v 'gateway' | grep -v 'pre-G-1'; then + echo "::error::H-009 regression: README.md appears to advertise JWT auth support." + echo "certctl does NOT ship in-process JWT middleware. JWT/OIDC" + echo "integration is via an authenticating gateway — see" + echo "docs/architecture.md::Authenticating-gateway pattern." + echo "If you added a sentence about JWT to README, either remove" + echo "it or rewrite it to point at the gateway pattern." + exit 1 +fi +echo "H-009 readme-jwt: clean." diff --git a/scripts/ci-guards/L-001-insecure-skip-verify.sh b/scripts/ci-guards/L-001-insecure-skip-verify.sh new file mode 100755 index 0000000..8aab1e1 --- /dev/null +++ b/scripts/ci-guards/L-001-insecure-skip-verify.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# scripts/ci-guards/L-001-insecure-skip-verify.sh +# +# L-001 audited every production InsecureSkipVerify=true call site +# and documented the justification per site in docs/tls.md. This +# script grep-fails the build if any new `InsecureSkipVerify: true` +# lands in a non-test Go file without a `//nolint:gosec` comment +# carrying the justification. Test files (_test.go) are exempt. +# Updating the documented surface goes through the docs/tls.md +# table — net-new sites must be reasoned about before merge. + +set -e +# Find every "InsecureSkipVerify: true" or "InsecureSkipVerify = true" +# in a non-test .go file. Then for each, check the same line OR the +# immediately preceding line for `//nolint:gosec`. +BAD="" +while IFS= read -r match; do + file=$(echo "$match" | cut -d: -f1) + line=$(echo "$match" | cut -d: -f2) + same=$(sed -n "${line}p" "$file" 2>/dev/null) + prev=$(sed -n "$((line - 1))p" "$file" 2>/dev/null) + if echo "$same $prev" | grep -q 'nolint:gosec'; then + continue + fi + BAD="$BAD\n$match" +done < <(grep -rnE 'InsecureSkipVerify:\s*true|InsecureSkipVerify\s*=\s*true' \ + --include='*.go' \ + --exclude='*_test.go' \ + . || true) +if [ -n "$BAD" ]; then + echo "::error::L-001 regression: new InsecureSkipVerify=true site without //nolint:gosec justification:" + echo -e "$BAD" + echo "" + echo "Add a //nolint:gosec comment with justification on the same" + echo "or preceding line, AND add a row to the docs/tls.md table." + exit 1 +fi +echo "L-001 insecure-skip-verify: clean." diff --git a/scripts/ci-guards/L-1-bulk-action-loop.sh b/scripts/ci-guards/L-1-bulk-action-loop.sh new file mode 100755 index 0000000..60dd2cc --- /dev/null +++ b/scripts/ci-guards/L-1-bulk-action-loop.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# scripts/ci-guards/L-1-bulk-action-loop.sh +# +# L-1 master closed cat-l-fa0c1ac07ab5 (bulk-renew loop) and +# cat-l-8a1fb258a38a (bulk-reassign loop) by adding server-side +# bulk endpoints (POST /api/v1/certificates/bulk-renew and +# POST /api/v1/certificates/bulk-reassign) that the GUI calls +# in a single round-trip. Pre-L-1 the GUI looped per-cert +# HTTP calls — 100 selected certs = 100 round-trips × ~50–200ms +# each = a 5–20-second wedge during which the operator stares +# at a progress bar. +# +# This script grep-fails the build if either loop shape reappears +# in CertificatesPage.tsx. Patterns catch the actual pre-L-1 +# shapes: +# - `for (const id of ids) { await triggerRenewal(id) }` +# - `for (const id of ids) { await updateCertificate(id, { owner_id }) }` +# - `for (let i = 0; i < ids.length; i++) { await triggerRenewal(ids[i]) }` +# +# Allowed: comment lines explaining the pre-L-1 pattern in the +# docblock above each handler. Test files (_test.tsx) exempt +# so negative-pattern tests can keep working. +# +# See coverage-gap-audit-2026-04-24-v5/unified-audit.md +# cat-l-fa0c1ac07ab5 and cat-l-8a1fb258a38a for closure +# rationale, or web/src/api/client.ts::bulkRenewCertificates +# / bulkReassignCertificates for the canonical call path. + +set -e + +BAD_LOOP=$(grep -nE 'for[[:space:]]*\(' web/src/pages/CertificatesPage.tsx 2>/dev/null \ + | grep -E 'await[[:space:]]+(triggerRenewal|updateCertificate)\(' \ + | grep -v '\.test\.' \ + | grep -vE '^\s*[^:]+:[0-9]+:\s*//' \ + || true) +if [ -n "$BAD_LOOP" ]; then + echo "::error::L-1 regression: client-side bulk-action loop reappeared in CertificatesPage.tsx:" + echo "$BAD_LOOP" + echo "" + echo "Use bulkRenewCertificates({ certificate_ids: [...] }) or" + echo "bulkReassignCertificates({ certificate_ids: [...], owner_id, team_id? })" + echo "instead of looping per-item HTTP calls. See" + echo "coverage-gap-audit-2026-04-24-v5/unified-audit.md cat-l-* for rationale." + exit 1 +fi +echo "L-1 bulk-action-loop: clean." diff --git a/scripts/ci-guards/M-012-no-root-user.sh b/scripts/ci-guards/M-012-no-root-user.sh new file mode 100755 index 0000000..c38a8b8 --- /dev/null +++ b/scripts/ci-guards/M-012-no-root-user.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash +# scripts/ci-guards/M-012-no-root-user.sh +# +# Bundle A / Audit M-012 (CWE-250): every Dockerfile in the repo +# MUST end with a `USER ` directive before the +# ENTRYPOINT/CMD so the container never runs as uid=0. This script +# grep-fails the build if any Dockerfile is missing such a USER. +# `USER root` and `USER 0` are explicitly rejected. + +set -e +BAD="" +for df in $(find . -name 'Dockerfile*' -not -path './web/node_modules/*'); do + # Find the LAST USER directive in the file. + last_user=$(grep -E '^USER\s+\S+' "$df" | tail -1 | awk '{print $2}') + if [ -z "$last_user" ]; then + BAD="$BAD\n$df: no USER directive at all" + continue + fi + if [ "$last_user" = "root" ] || [ "$last_user" = "0" ]; then + BAD="$BAD\n$df: terminal USER is $last_user (must drop privileges)" + continue + fi +done +if [ -n "$BAD" ]; then + echo "::error::M-012 regression: Dockerfile USER-drop missing:" + echo -e "$BAD" + exit 1 +fi +echo "M-012 no-root-user: clean." diff --git a/scripts/ci-guards/P-1-documented-orphan-fns.sh b/scripts/ci-guards/P-1-documented-orphan-fns.sh new file mode 100755 index 0000000..aacaae9 --- /dev/null +++ b/scripts/ci-guards/P-1-documented-orphan-fns.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# scripts/ci-guards/P-1-documented-orphan-fns.sh +# +# P-1 master closed diff-04x03-d24864996ad4 + cat-b-dc46aadab98e +# by documenting 17 detail-page-candidate orphan client.ts +# functions in a docblock at the top of web/src/api/client.ts. +# This script verifies the docblock list ↔ export list relationship: +# every name listed in the docblock must still be declared as +# an export below it (catches drift where someone deletes the +# export but forgets the docblock, or vice versa). +# +# CRL/OCSP-Responder Phase 5 closed the getOCSPStatus orphan: the +# CertificateDetailPage Revocation Endpoints panel now consumes it +# ("Check OCSP status" button). Removed from the list to keep the +# docblock + guardrail honest. +# +# See coverage-gap-audit-2026-04-24-v5/unified-audit.md +# diff-04x03-d24864996ad4 + cat-b-dc46aadab98e for closure rationale. + +set -e +DOCUMENTED='getAgentGroup getAgentGroupMembers getAuditEvent getCertificateDeployments getDiscoveredCertificate getHealthCheck getHealthCheckHistory getNetworkScanTarget getNotification getOwner getPolicy getPolicyViolations getRenewalPolicy getTeam registerAgent updateHealthCheck' +MISSING="" +for fn in $DOCUMENTED; do + if ! grep -qE "^export const ${fn}\b" web/src/api/client.ts; then + MISSING="${MISSING}${fn} " + fi +done +if [ -n "$MISSING" ]; then + echo "::error::P-1 regression: documented orphan(s) missing from client.ts exports:" + echo " $MISSING" + echo "" + echo "Either restore the export, or delete the corresponding line" + echo "in the documented-orphans docblock at the top of client.ts." + echo "See coverage-gap-audit-2026-04-24-v5/unified-audit.md" + echo "diff-04x03-d24864996ad4 for closure rationale." + exit 1 +fi +echo "P-1 documented-orphan-fns: clean ($(echo $DOCUMENTED | wc -w) fns verified)." diff --git a/scripts/ci-guards/README.md b/scripts/ci-guards/README.md new file mode 100644 index 0000000..e547046 --- /dev/null +++ b/scripts/ci-guards/README.md @@ -0,0 +1,76 @@ +# `scripts/ci-guards/` — Regression-guard scripts + +Each `.sh` script in this directory pins one closed audit finding from +regressing. CI runs the full set on every push via the +`Regression guards` step in `.github/workflows/ci.yml`. Operators can +run any script locally: + +```bash +bash scripts/ci-guards/G-3-env-docs-drift.sh +``` + +## Contract + +Every script in this directory MUST: + +1. Be exit-code 0 on a clean repo (no regression present). +2. Be exit-code non-zero on regression, with a `::error::` annotation + prefix so PR reviewers see the failing line in the GitHub Actions UI. +3. Be runnable from repo root via `bash scripts/ci-guards/.sh` — + no implicit `cd` requirement, no env-var requirement. +4. Carry a head-comment block matching the in-source justification + from the original ci.yml entry: the audit-finding reference, the + closure rationale, the exempt-surface list (if any). +5. Use `set -e` early to fail-fast on internal command errors. +6. Produce no output on the happy path beyond a final + `echo ": clean."` confirmation line. + +## Adding a new guard + +1. Drop a new `.sh` in this directory with the head-comment block + describing the audit finding it closes. +2. Make it executable: `chmod +x scripts/ci-guards/.sh`. +3. Verify it fails on a deliberate regression and passes on clean repo. +4. CI auto-picks up new scripts via the `for g in scripts/ci-guards/*.sh` + loop in the `Regression guards` step — no ci.yml change required. + +## The 20 guards in this directory + +| ID | Finding | Catches | +|---|---|---| +| `G-1-jwt-auth-literal` | G-1 JWT silent auth downgrade | `"jwt"` literal in additive auth-type surfaces | +| `L-001-insecure-skip-verify` | L-001 unjustified InsecureSkipVerify | `InsecureSkipVerify: true` without `//nolint:gosec` | +| `H-001-bare-from` | H-001 (CWE-829) tag-swap attack | Bare `FROM` line without `@sha256` digest pin | +| `M-012-no-root-user` | M-012 (CWE-250) container-as-root | Dockerfile missing terminal `USER ` | +| `H-009-readme-jwt` | H-009 README JWT advertising | README.md re-introducing JWT-as-supported claim | +| `G-2-api-key-hash-json` | G-2 cat-s5-apikey_leak | `api_key_hash` in JSON-emitting surface | +| `U-2-plaintext-healthcheck` | U-2 healthcheck protocol mismatch | Plaintext `http://` in HEALTHCHECK directive | +| `U-3-migration-mount` | U-3 seed initdb schema drift | Migration file mounted into postgres initdb | +| `D-1-D-2-statusbadge-phantom` | D-1 + D-2 dead keys + TS phantoms | StatusBadge dead keys + 5 Certificate / 5 Agent / 1 Issuer / 1 Notification phantom fields | +| `L-1-bulk-action-loop` | L-1 client-side bulk loops | `for ... await triggerRenewal/updateCertificate` in CertificatesPage | +| `B-1-orphan-crud` | B-1 orphan-CRUD client fns | 8 update/create/delete fns lose their page consumer | +| `S-2-strings-contains-err` | S-2 brittle error-dispatch | `strings.Contains(err.Error(), "not found"\|"violates foreign key")` in handlers | +| `G-3-env-docs-drift` | G-3 env-var docs drift | `CERTCTL_*` env var defined OR documented but not both | +| `test-naming-convention` | I-001-extended | `func TestXxx` (lowercase first letter) — Go silently skips | +| `S-1-hardcoded-source-counts` | S-1 stale numeric prose | Hardcoded "N issuer connectors" / "N MCP tools" in README + docs | +| `P-1-documented-orphan-fns` | P-1 documented orphans | 16 read-fn names removed from client.ts exports | +| `T-1-frontend-page-coverage` | T-1 untested frontend pages | New page in `web/src/pages/` without sibling `.test.tsx` and not on the deferred allowlist | +| `bundle-8-L-015-target-blank-rel-noopener` | L-015 (CWE-1022) reverse-tabnabbing | `target="_blank"` without `rel="noopener noreferrer"` | +| `bundle-8-L-019-dangerously-set-inner-html` | L-019 (CWE-79) XSS | `dangerouslySetInnerHTML` outside `safeHtml.ts` | +| `bundle-8-M-009-bare-usemutation` | M-009 + M-029 mutation contract | Bare `useMutation()` outside `useTrackedMutation` wrapper | + +## Guards explicitly NOT here + +- **`QA-doc Part-count drift`** + **`QA-doc seed-count drift`** — these + protect docs-the-operator-reads, not anything the product depends on. + Moved to `make verify-docs` (operator runs pre-tag, not on every push). + See `cowork/ci-pipeline-cleanup-prompt.md` Phase 11. + +## Running the full set locally + +```bash +for g in scripts/ci-guards/*.sh; do + echo "=== $(basename "$g") ===" + bash "$g" || echo " FAILED" +done +``` diff --git a/scripts/ci-guards/S-1-hardcoded-source-counts.sh b/scripts/ci-guards/S-1-hardcoded-source-counts.sh new file mode 100755 index 0000000..3ad28b7 --- /dev/null +++ b/scripts/ci-guards/S-1-hardcoded-source-counts.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# scripts/ci-guards/S-1-hardcoded-source-counts.sh +# +# S-1 master closed cat-s1-9ce1cbe26876 (README + features.md +# stale numeric counts; explicit CLAUDE.md violation per +# "version-stamped numbers rot") and +# cat-s1-features_md_issuer_count_contradiction (features.md +# self-disagreed on issuer count: 9 vs 12 in the same doc). +# The fix replaced source-derived numbers in prose with +# "rebuild via " patterns documented in CLAUDE.md:: +# "Current-state commands". This script grep-fails the build if +# any of the previously-stale sites reintroduces a hardcoded +# count. +# +# Allowed surfaces: demo-fixture prose in README ("32 +# certificates" — those are seed_demo.sql facts, not live +# source counts), historical-milestone counts in +# WORKSPACE-CHANGELOG.md, the testing-guide example phrasing +# ("README claims 8 issuer connectors but only 6 exist"), +# and any number that quotes the source command immediately +# adjacent. +# +# See coverage-gap-audit-2026-04-24-v5/unified-audit.md +# cat-s1-9ce1cbe26876 + cat-s1-features_md_issuer_count_contradiction +# for closure rationale. + +set -e +BAD=$(grep -rnE '\b[0-9]+\s+(issuer connectors?|target connectors?|notifier connectors?|discovery connectors?|MCP tools|OpenAPI operations|migrations|database tables|frontend pages|HTTP routes)\b' \ + README.md docs/ 2>/dev/null \ + | grep -vE 'WORKSPACE-CHANGELOG|seed_demo|demo override' \ + | grep -vE 'DRIFT HAZARD|Source: |Rebuild|rebuild via|grep -|wc -l|ls -d|find ' \ + | grep -vE 'README claims [0-9]+ issuer connectors but only [0-9]+ exist' \ + || true) +if [ -n "$BAD" ]; then + echo "::error::S-1 regression: hardcoded source-count prose reappeared:" + echo "$BAD" + echo "" + echo "CLAUDE.md rule: 'Numeric claims about current state rot.'" + echo "Replace the count with the grep command from CLAUDE.md::" + echo "'Current-state commands' (e.g. 'ls -d internal/connector/issuer/*/ | wc -l')" + echo "or rephrase to reference the rebuild command on the same line." + echo "See coverage-gap-audit-2026-04-24-v5/unified-audit.md" + echo "cat-s1-9ce1cbe26876 for closure rationale." + exit 1 +fi +echo "S-1 hardcoded-source-counts: clean." diff --git a/scripts/ci-guards/S-2-strings-contains-err.sh b/scripts/ci-guards/S-2-strings-contains-err.sh new file mode 100755 index 0000000..8829e9c --- /dev/null +++ b/scripts/ci-guards/S-2-strings-contains-err.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# scripts/ci-guards/S-2-strings-contains-err.sh +# +# S-2 closure (cat-s6-efc7f6f6bd50): replaced 30 brittle +# substring-match error-dispatch sites in internal/api/handler/ +# with errors.Is + typed sentinels (repository.ErrNotFound, +# repository.ErrForeignKeyConstraint via the +# repository.IsForeignKeyError helper). This script grep-fails +# the build if any new strings.Contains(err.Error(), "not found") +# or strings.Contains(err.Error(), "violates foreign key") +# site appears under internal/api/handler/. +# +# Allowed: closure-comments documenting the convention (e.g. +# bulk_reassignment.go's "post-M-1 errToStatus convention" +# docblock); domain-specific substring patterns that are +# legitimately one-off ("cannot approve", "cannot reject", +# "cannot be parsed", "challenge password") — flagged as +# deferred follow-ups in the S-2 commit message. +# +# See coverage-gap-audit-2026-04-24-v5/unified-audit.md +# cat-s6-efc7f6f6bd50 for closure rationale. + +set -e +BAD=$(grep -rnE 'strings\.Contains\(err\.Error\(\),\s*"(not found|violates foreign key|RESTRICT)"' internal/api/handler/ 2>/dev/null \ + | grep -vE '^\s*[^:]+:[0-9]+:\s*//' \ + || true) +if [ -n "$BAD" ]; then + echo "::error::S-2 regression: brittle substring-match error-dispatch reappeared:" + echo "$BAD" + echo "" + echo "Use errors.Is(err, repository.ErrNotFound) for not-found dispatch," + echo "or repository.IsForeignKeyError(err) for FK violations." + echo "See coverage-gap-audit-2026-04-24-v5/unified-audit.md" + echo "cat-s6-efc7f6f6bd50 for closure rationale." + exit 1 +fi +echo "S-2 strings-contains-err: clean." diff --git a/scripts/ci-guards/T-1-frontend-page-coverage.sh b/scripts/ci-guards/T-1-frontend-page-coverage.sh new file mode 100755 index 0000000..a18e7cf --- /dev/null +++ b/scripts/ci-guards/T-1-frontend-page-coverage.sh @@ -0,0 +1,56 @@ +#!/usr/bin/env bash +# scripts/ci-guards/T-1-frontend-page-coverage.sh +# +# T-1 closure (cat-s2-c24a548076c6): pre-T-1 only 3 of 28 pages +# had Vitest coverage. T-1 lifted that to 11/28 by writing tests +# for the 8 highest-leverage pages (CertificatesPage filter + +# pagination state, the new B-1 Edit modals, the D-2 type-trim +# render sites, etc.). The remaining pages are deferred to per- +# page commits — when the next feature change touches them, the +# test gets added in the same commit. This script blocks new +# pages from landing without tests. +# +# Allowlist: pages that are explicitly deferred — listed below +# with a one-line "why deferred" justification. Each entry must +# be removed when the page gets its test. +# - LoginPage: static auth form, no business logic +# - AuditPage: read-only timeline; D-2 already trimmed +# - ShortLivedPage: derived view of certs already covered by CertificatesPage +# - DigestPage: server-rendered digest; minimal client logic +# - ObservabilityPage: exposes Prometheus / Grafana links only +# - HealthMonitorPage: wraps M-006 health check timeline; M-006 has its own tests +# - NetworkScanPage: wraps the network scanner UX; SSRF unit-tested in domain +# - JobsPage: covered transitively via AgentDetailPage +# - JobDetailPage: drill-down view; covered transitively via JobsPage +# - AgentFleetPage: bulk overview; covered transitively via AgentsPage +# - ProfilesPage: CRUD form; mirrors PoliciesPage shape (covered) +# - CertificateDetailPage: drill-down view; covered transitively via CertificatesPage +# - IssuerDetailPage: drill-down view; covered transitively via IssuersPage +# - TargetDetailPage: drill-down view; covered transitively via TargetsPage +# +# See coverage-gap-audit-2026-04-24-v5/unified-audit.md +# cat-s2-c24a548076c6 for closure rationale. + +set -e +ALLOW='^(LoginPage|AuditPage|ShortLivedPage|DigestPage|ObservabilityPage|HealthMonitorPage|NetworkScanPage|JobsPage|JobDetailPage|AgentFleetPage|ProfilesPage|CertificateDetailPage|IssuerDetailPage|TargetDetailPage)$' +UNTESTED="" +for f in web/src/pages/*.tsx; do + base=$(basename "$f" .tsx) + case "$f" in *.test.tsx) continue ;; esac + if [ -f "web/src/pages/${base}.test.tsx" ]; then continue; fi + if echo "$base" | grep -qE "$ALLOW"; then continue; fi + UNTESTED="${UNTESTED}${base} " +done +if [ -n "$UNTESTED" ]; then + echo "::error::T-1 regression: page(s) without sibling .test.tsx and not on the deferred allowlist:" + echo " $UNTESTED" + echo "" + echo "Either add web/src/pages/.test.tsx (mirror NotificationsPage.test.tsx)," + echo "or add the page to the ALLOW pattern in scripts/ci-guards/T-1-frontend-page-coverage.sh" + echo "with a one-line 'why deferred' comment. See" + echo "coverage-gap-audit-2026-04-24-v5/unified-audit.md cat-s2-c24a548076c6" + echo "for closure rationale." + exit 1 +fi +ALLOWLIST_SIZE=$(echo "$ALLOW" | tr '|' '\n' | wc -l) +echo "T-1 frontend-page-coverage: clean (allowlist size: $ALLOWLIST_SIZE pages deferred)." diff --git a/scripts/ci-guards/U-2-plaintext-healthcheck.sh b/scripts/ci-guards/U-2-plaintext-healthcheck.sh new file mode 100755 index 0000000..4bc15a1 --- /dev/null +++ b/scripts/ci-guards/U-2-plaintext-healthcheck.sh @@ -0,0 +1,51 @@ +#!/usr/bin/env bash +# scripts/ci-guards/U-2-plaintext-healthcheck.sh +# +# U-2 closed cat-u-healthcheck_protocol_mismatch by switching the +# published image's HEALTHCHECK from `curl -f http://localhost: +# 8443/health` (always failed against the HTTPS-only listener) to +# `curl -fsk https://localhost:8443/health`. This script grep-fails +# the build if any Dockerfile in the repo carries the pre-U-2 +# plaintext shape — either explicitly (`http://localhost:8443/ +# health` in a HEALTHCHECK) or via the looser pattern of any +# HEALTHCHECK that targets `http://` against the certctl server +# port. +# +# Comment lines and the docs/upgrade-to-tls.md:182 expected-to- +# fail invariant ("plaintext is gone, expect Connection refused") +# are intentionally exempt — we DO want the upgrade-doc string +# `http://localhost:8443/health` to remain there, since it +# documents what operators should test for to confirm plaintext +# is dead. The guardrail is scoped to Dockerfile* only, so docs +# are out of its reach. +# +# See coverage-gap-audit-2026-04-24-v5/unified-audit.md +# cat-u-healthcheck_protocol_mismatch for the closure rationale, +# or deploy/test/healthcheck_test.go for the binary-image +# contract the runtime test pins. + +set -e + +# Patterns that catch the actual regression shapes: +# - HEALTHCHECK directive carrying any http:// (even if the +# port differs, no plaintext probe should ship). +# - The exact pre-U-2 string for grep-friendliness. +BAD=$(grep -rnEH \ + -e 'HEALTHCHECK.*http://' \ + -e 'curl[^|&;]*-f[^|&;]*http://localhost:8443/health' \ + Dockerfile Dockerfile.agent Dockerfile.* 2>/dev/null \ + | grep -vE '^\s*[^:]+:[0-9]+:\s*#' \ + || true) +if [ -n "$BAD" ]; then + echo "::error::U-2 regression: plaintext HEALTHCHECK reappeared in a Dockerfile:" + echo "$BAD" + echo "" + echo "Allowed: HTTPS HEALTHCHECK with -k (acceptable for" + echo "localhost-to-localhost), or non-HTTP probe shapes" + echo "(pgrep, /proc check). See Dockerfile / Dockerfile.agent" + echo "for the post-U-2 reference shape and" + echo "coverage-gap-audit-2026-04-24-v5/unified-audit.md" + echo "cat-u-healthcheck_protocol_mismatch for rationale." + exit 1 +fi +echo "U-2 plaintext-healthcheck: clean." diff --git a/scripts/ci-guards/U-3-migration-mount.sh b/scripts/ci-guards/U-3-migration-mount.sh new file mode 100755 index 0000000..e1be72f --- /dev/null +++ b/scripts/ci-guards/U-3-migration-mount.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash +# scripts/ci-guards/U-3-migration-mount.sh +# +# U-3 closed cat-u-seed_initdb_schema_drift (GitHub #10) by +# eliminating the dual-source-of-truth between +# `migrations/*.up.sql` mounted into postgres +# `/docker-entrypoint-initdb.d/` and the same files re-applied at +# runtime by `RunMigrations`. Pre-U-3 every new migration that +# the seed depended on (000013 added `policy_rules.severity`, +# 000017 renames `retry_interval_seconds`, etc.) had to be added +# by hand to the compose mount list; missing the update crashed +# initdb on first boot, postgres flagged unhealthy, and the +# whole stack failed to start from a fresh clone. Post-U-3 the +# server is the single source of truth — `RunMigrations` + +# `RunSeed` apply everything at boot. +# +# This script grep-fails the build if any compose file under +# `deploy/` re-introduces a `migrations/.*\.sql` mount into +# `/docker-entrypoint-initdb.d`. Comments are exempt so the +# post-fix rationale block in the compose files (which +# documents WHY the mounts were removed) doesn't trip the guard. +# The demo overlay's `seed_demo.sql` is the explicit exception: +# it is tolerated only when it lives behind the +# CERTCTL_DEMO_SEED env var (post-U-3 demo path) — bare initdb +# mounts are NOT tolerated. The grep matches all compose +# mount-list shapes (`-` indented, `volumes:` indented, both), +# so any future drift surfaces here before the operator hits it +# on a fresh clone. +# +# See coverage-gap-audit-2026-04-24-v5/unified-audit.md +# cat-u-seed_initdb_schema_drift for the closure rationale, or +# internal/repository/postgres/db.go::RunSeed for the runtime +# contract. + +set -e + +BAD=$(grep -rnEH \ + -e 'migrations/.*\.sql:.*docker-entrypoint-initdb' \ + -e 'seed.*\.sql:.*docker-entrypoint-initdb' \ + deploy/docker-compose.yml \ + deploy/docker-compose.test.yml \ + deploy/docker-compose.demo.yml \ + 2>/dev/null \ + | grep -vE '^\s*[^:]+:[0-9]+:\s*#' \ + || true) +if [ -n "$BAD" ]; then + echo "::error::U-3 regression: migration/seed mount into postgres initdb reappeared:" + echo "$BAD" + echo "" + echo "The post-U-3 contract is: postgres comes up with an empty" + echo "schema and the server applies migrations + seed at boot via" + echo "internal/repository/postgres.RunMigrations + RunSeed. Demo" + echo "data lives behind CERTCTL_DEMO_SEED=true (RunDemoSeed)," + echo "not an initdb mount. See" + echo "coverage-gap-audit-2026-04-24-v5/unified-audit.md" + echo "cat-u-seed_initdb_schema_drift for the closure rationale." + exit 1 +fi +echo "U-3 migration-mount: clean." diff --git a/scripts/ci-guards/bundle-8-L-015-target-blank-rel-noopener.sh b/scripts/ci-guards/bundle-8-L-015-target-blank-rel-noopener.sh new file mode 100755 index 0000000..3437736 --- /dev/null +++ b/scripts/ci-guards/bundle-8-L-015-target-blank-rel-noopener.sh @@ -0,0 +1,30 @@ +#!/usr/bin/env bash +# scripts/ci-guards/bundle-8-L-015-target-blank-rel-noopener.sh +# +# Audit L-015 / CWE-1022 (reverse-tabnabbing): every +# MUST carry rel="noopener noreferrer" so a malicious page at the +# target URL cannot navigate the opener window via window.opener. +# At Bundle-8 close all 3 sites in the codebase already comply — +# this guard prevents regression. The ExternalLink component +# (web/src/components/ExternalLink.tsx) is the recommended way to +# add new external links. +# +# Test files (web/src/**/*.test.{ts,tsx}) are excluded so test +# docstrings or fixture data describing the attack vector by +# name don't trip the guard — symmetric with the L-019 guard. + +set -e +OFFENDERS=$(grep -rnE 'target=["'"'"']?_blank["'"'"']?' web/src/ 2>/dev/null \ + | grep -v 'noopener noreferrer' \ + | grep -v 'web/src/components/ExternalLink.tsx' \ + | grep -vE '\.test\.(ts|tsx)(:[0-9]+)?:' \ + || true) +if [ -n "$OFFENDERS" ]; then + echo "::error::L-015 regression: target=\"_blank\" without rel=\"noopener noreferrer\":" + echo "$OFFENDERS" + echo "" + echo "Either add rel=\"noopener noreferrer\" inline," + echo "or migrate to from web/src/components/ExternalLink.tsx." + exit 1 +fi +echo "L-015 target-blank-rel-noopener: clean." diff --git a/scripts/ci-guards/bundle-8-L-019-dangerously-set-inner-html.sh b/scripts/ci-guards/bundle-8-L-019-dangerously-set-inner-html.sh new file mode 100755 index 0000000..3fd9dd1 --- /dev/null +++ b/scripts/ci-guards/bundle-8-L-019-dangerously-set-inner-html.sh @@ -0,0 +1,30 @@ +#!/usr/bin/env bash +# scripts/ci-guards/bundle-8-L-019-dangerously-set-inner-html.sh +# +# Audit L-019 / CWE-79 (XSS): no PRODUCTION code may use +# dangerouslySetInnerHTML directly. At Bundle-8 close the codebase +# has 0 sites; future genuine needs MUST route through +# web/src/utils/safeHtml.ts::sanitizeHtml. +# +# Test files (web/src/**/*.test.{ts,tsx}) are explicitly excluded: +# the M-029 Pass 3 XSS-hardening test docstrings legitimately cite +# the attack vector by name to explain what the test is guarding +# against (e.g. "a careless refactor to dangerouslySetInnerHTML +# would let an attacker-controlled CSR deliver an XSS payload"). +# Tests describing the threat aren't using it; the guard's intent +# is production code only. + +set -e +OFFENDERS=$(grep -rnE 'dangerouslySetInnerHTML' web/src/ 2>/dev/null \ + | grep -v 'web/src/utils/safeHtml.ts' \ + | grep -vE '\.test\.(ts|tsx)(:[0-9]+)?:' \ + || true) +if [ -n "$OFFENDERS" ]; then + echo "::error::L-019 regression: dangerouslySetInnerHTML used outside safeHtml.ts:" + echo "$OFFENDERS" + echo "" + echo "Route through web/src/utils/safeHtml.ts::sanitizeHtml — see file" + echo "header for the activation procedure (DOMPurify dependency)." + exit 1 +fi +echo "L-019 dangerously-set-inner-html: clean." diff --git a/scripts/ci-guards/bundle-8-M-009-bare-usemutation.sh b/scripts/ci-guards/bundle-8-M-009-bare-usemutation.sh new file mode 100755 index 0000000..9ea3a96 --- /dev/null +++ b/scripts/ci-guards/bundle-8-M-009-bare-usemutation.sh @@ -0,0 +1,45 @@ +#!/usr/bin/env bash +# scripts/ci-guards/bundle-8-M-009-bare-usemutation.sh +# +# Audit M-009 + M-029 Pass 1 closure: +# +# Pre-Bundle-8 the codebase had 56 bare useMutation sites with +# discretionary invalidation. Bundle 8 shipped the useTrackedMutation +# wrapper (web/src/hooks/useTrackedMutation.ts) that requires every +# caller to declare `invalidates: QueryKey[] | 'noop'`. M-029 Pass 1 +# then migrated all 56 sites to the wrapper across 6 batches. +# +# This guard pins the contract going forward: every useMutation call +# in src/ MUST be inside useTrackedMutation.ts (the wrapper itself +# is the only legitimate caller of useMutation). Any bare useMutation +# call elsewhere is a regression — adding a new mutation site means +# going through the wrapper so the invalidates contract is enforced +# per-site, not by a soft budget guard. +# +# If you genuinely need raw useMutation (extremely unlikely — the +# wrapper supports invalidates: 'noop' for fire-and-forget mutations), +# update this guard's exclusion list and document the carve-out. + +set -e +# Test files (web/src/**/*.test.{ts,tsx}) are excluded so existing +# useMutation-mocking test patterns and the wrapper's own unit +# tests don't trip the production guard — symmetric with L-015 +# and L-019 above. +BARE=$(grep -rnE '\buseMutation\(' web/src/ 2>/dev/null \ + | grep -v 'web/src/hooks/useTrackedMutation\.ts' \ + | grep -vE '\.test\.(ts|tsx)(:[0-9]+)?:' \ + || true) +if [ -n "$BARE" ]; then + echo "::error::M-009 hard-zero regression: bare useMutation() call(s) outside the wrapper:" + echo "$BARE" + echo + echo "Every mutation must go through useTrackedMutation" + echo "(web/src/hooks/useTrackedMutation.ts) with explicit" + echo "invalidates: QueryKey[] | 'noop'. See file header for usage." + exit 1 +fi +# Sanity counts (informational, not a gate). +TRACKED=$(grep -rcE '\buseTrackedMutation\(' web/src/ 2>/dev/null | awk -F: '{s+=$2} END{print s}') +INVALIDATIONS=$(grep -rcE 'invalidateQueries|setQueryData|removeQueries|invalidates:' web/src/ 2>/dev/null | awk -F: '{s+=$2} END{print s}') +echo "M-009 bare-usemutation: clean (wrapper-internal call + test files excluded)." +echo "M-009 informational: useTrackedMutation sites = $TRACKED; invalidation surface = $INVALIDATIONS." diff --git a/scripts/ci-guards/test-naming-convention.sh b/scripts/ci-guards/test-naming-convention.sh new file mode 100755 index 0000000..14deaa5 --- /dev/null +++ b/scripts/ci-guards/test-naming-convention.sh @@ -0,0 +1,28 @@ +#!/usr/bin/env bash +# scripts/ci-guards/test-naming-convention.sh +# +# Bundle Q / I-001-extended (2026-04-27): hard-fail. Catches tests +# Go itself would silently skip — `func TestX...` where the first +# letter after `Test` is lowercase. Go's testing runner requires +# uppercase to register the test (^Test[A-Z]); lowercase tests +# don't run, which is a real bug a CI guard should catch. +# +# The original audit's `Test__` +# triple-token prescription was relaxed: single-function pin +# tests like `TestNewAgent` or `TestSplitPEMChain` are valid Go +# convention, with internal scenarios expressed via t.Run subtests. +# Requiring the underscore-Scenario-Result triple repo-wide would +# mean renaming 167 legitimate tests for no observable behavior +# change. + +set -e +INVALID=$(grep -rnE '^func Test[a-z]' --include='*_test.go' . \ + | grep -v '_test.go.bak' \ + || true) +if [ -n "$INVALID" ]; then + echo "::error::Test-naming convention regression: tests Go would silently skip (lowercase after 'Test'):" + echo "$INVALID" + echo "Rename to start with an uppercase letter — Go's test runner only matches ^Test[A-Z]." + exit 1 +fi +echo "test-naming-convention: clean (no Go-invalid test names found)."