From 9dc0742e77eb81f0b2ab3444347f0e209af7ce48 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Sat, 25 Apr 2026 13:52:54 +0000 Subject: [PATCH] fix(web): close StatusBadge enum drift + Certificate TS phantom fields (D-1 master) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five audit findings, all category cat-d or cat-f, all rooted in two frontend files. The dashboard silently lied: cat-d-359e92c20cbf [P1, primary] — Agent: 'Stale' dead key + 'Degraded' neutral fallthrough cat-d-9f4c8e4a91f1 [P2] — Notification: 'dead' missing cat-d-1447e04732e7 [P3] — Cert: 'PendingIssuance' dead key cat-f-cert_detail_page_key_render_fallback [P2] — render-site reads cert.key_algorithm directly cat-f-ae0d06b6588f [P2] — Certificate TS phantom fields (root cause) Pre-D-1, agents in the only Go AgentStatus that means 'needs operator attention' (Degraded) rendered as default neutral grey because StatusBadge mapped 'Stale' (a key Go has never emitted) to yellow. Dead-letter notifications visually equated with 'read' (operator-acknowledged). The Certificate badge map carried a 'PendingIssuance' key no Go enum emits. CertificateDetailPage's Key Algorithm and Key Size rows always rendered '—' even when the data was a single fetch away — the lookup went through cert.key_algorithm / cert.key_size directly, both phantom Certificate TS fields. Trim the TS type so the missing-data case is explicit; fix the render site to use latestVersion?.field; pin the contract with a 38-case Vitest property test that walks every Go enum. StatusBadge (web/src/components/StatusBadge.tsx) - Drop 'Stale' (Agent dead key) + 'PendingIssuance' (Cert dead key). - Add 'Degraded' (Agent → badge-warning) + 'dead' (Notification → badge-danger). - Add leading docblock naming Go-side source-of-truth file for every status family and pointing at the property test as regression vector. Property test (web/src/components/StatusBadge.test.tsx — 38 cases) - Iterates every Go-emitted enum value (AgentStatus, CertificateStatus, JobStatus, NotificationStatus, DiscoveryStatus, HealthStatus) plus the two frontend-synthesized Enabled/Disabled labels, asserts every value gets a non-default class (or an explicit 'badge badge-neutral' for the five intentionally-neutral terminal values: Archived, Cancelled, Dismissed, read, unknown). - Negative assertions: 'Stale' and 'PendingIssuance' must fall through to the dictionary default — re-adding either key surfaces here. - Specific UX-correctness assertions: 'dead' → badge-danger, 'Degraded' → badge-warning. - Unknown-status fallthrough preserves label text. Certificate TS trim (web/src/api/types.ts) - Drop serial_number?, fingerprint_sha256?, key_algorithm?, key_size?, issued_at? from Certificate. Go's ManagedCertificate has never carried these — they live on CertificateVersion. Post-trim a cert.X access for any of the five fields is a TS compile error. - Leading docblock cross-references the closure rationale and the latestVersion fallback pattern. Render-site fix (web/src/pages/CertificateDetailPage.tsx) - Key Algorithm / Key Size rows now read latestVersion?.key_algorithm / latestVersion?.key_size, mirroring the existing latestVersion fallback used a few lines above for serial_number / fingerprint_sha256. - The same edit also tightened the serial / fingerprint / issued_at derivations to drop the now-impossible 'cert.X || latestVersion?.X' cert-side leg (cert.serial_number is a TS error post-trim). Type-test regression (web/src/api/types.test.ts) - Certificate literal construction pinned post-trim — adding any of the five fields back makes the literal an excess-property TS error. - Sibling CertificateVersion literal pinning the trimmed fields still live on the version envelope (so the CertificateDetailPage fallback path can't break). OpenAPI (api/openapi.yaml) - ManagedCertificate schema unchanged — was already correct (no phantom fields). Added a leading comment cross-referencing the D-5 closure for future readers. CI guardrail (.github/workflows/ci.yml) - 'Forbidden StatusBadge dead-key + Certificate phantom-field regression guard (D-1)'. Two grep blocks: catches Stale/PendingIssuance map literals in StatusBadge.tsx; uses an awk-scoped window over the 'export interface Certificate {' block in types.ts to catch the five phantom fields reappearing while explicitly excluding CertificateVersion (which legitimately carries them). Comments + test files exempt. Verification - Backend build/vet/test -short -race all clean across handler/router/ middleware packages. - Frontend tsc --noEmit clean. - Vitest 256 → 296 tests (+40: 38 from new StatusBadge test, 2 from D-5 Certificate trim regression in types.test.ts). - OpenAPI YAML parses (87 paths). - Both CI guardrail patterns clear on the post-fix tree; both fire against synthetic regression patterns (re-add Stale → fires; re-add serial_number? to Certificate → fires). Out of scope (deferred) - diff-05x06-* type drifts for Agent/DeploymentTarget/Notification/ DiscoveredCertificate/Issuer TS interfaces. Per-type field-by-field Go ↔ TS diff is codegen-shaped, not edit-shaped — warrants its own D-2 master prompt. Noted in CHANGELOG follow-ups section. --- .github/workflows/ci.yml | 74 ++++++++++++++ CHANGELOG.md | 36 ++++++- api/openapi.yaml | 9 ++ web/src/api/types.test.ts | 90 +++++++++++++++- web/src/api/types.ts | 17 +++- web/src/components/StatusBadge.test.tsx | 130 ++++++++++++++++++++++++ web/src/components/StatusBadge.tsx | 53 ++++++++-- web/src/pages/CertificateDetailPage.tsx | 26 +++-- 8 files changed, 413 insertions(+), 22 deletions(-) create mode 100644 web/src/components/StatusBadge.test.tsx diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9faeae4..f797f23 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -270,6 +270,80 @@ jobs: exit 1 fi + - name: Forbidden StatusBadge dead-key + Certificate phantom-field regression guard (D-1) + # 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 + - 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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c44951..acae326 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,41 @@ All notable changes to certctl are documented in this file. Dates use ISO 8601. Versions follow [Semantic Versioning](https://semver.org/). -## [unreleased] — 2026-04-24 +## [unreleased] — 2026-04-25 + +### D-1: StatusBadge enum drift + Certificate phantom fields — closed end-to-end + +> The dashboard silently lied in five places. Agents in the `Degraded` state (the only Go-side AgentStatus that means "needs operator attention") rendered as default neutral grey because StatusBadge mapped `Stale` (a key Go has never emitted) to yellow and let the real `Degraded` value fall through to the dictionary default. Dead-letter notifications (`status: 'dead'`, retries exhausted) rendered as default neutral, visually equated with `read` (operator-acknowledged). The Certificate badge map carried a `PendingIssuance` key that no Go enum value ever emits — dead key, latent confusion vector. CertificateDetailPage's Key Algorithm and Key Size rows always rendered `—` even when the data was a single fetch away, because the lookup went through `cert.key_algorithm` directly — and the underlying `Certificate` TypeScript interface declared five optional fields (`serial_number`, `fingerprint_sha256`, `key_algorithm`, `key_size`, `issued_at`) that Go's `ManagedCertificate` has never carried (those values live on `CertificateVersion`). Five findings, two files, one frontend rebuild. Pre-D-1 the only reason this didn't trip a regression suite was that the regression suite never asserted "every Go-emitted enum value gets a non-default StatusBadge class" — D-1 fixes the visual lies and adds a 38-case Vitest property test that walks every Go enum and pins the contract. + +### Breaking Changes + +- **`Certificate` TypeScript interface no longer declares `serial_number?`, `fingerprint_sha256?`, `key_algorithm?`, `key_size?`, or `issued_at?`.** The Go `ManagedCertificate` (`internal/domain/certificate.go`) has never emitted these fields on list responses; they live on `CertificateVersion` and are reachable via `getCertificateVersions(id)`. Pre-D-5 (the cat-f phantom-fields finding) the optional declarations made `cert.X` always-undefined on lists, and downstream consumers silently rendered `—` for every cert. Post-D-5 a `cert.X` access for any of the five fields is a TypeScript compile error, forcing every consumer to acknowledge the version-fallback pattern. The OpenAPI `ManagedCertificate` schema was already correct — only the TS type was drifted. +- **StatusBadge no longer maps `Stale` (Agent) or `PendingIssuance` (Certificate).** Both were dead keys — no Go enum value emits them. Operators with custom CSS hooked off `.badge-warning` for `Stale` will see the same color come back via the new `Degraded` mapping (same class), but JS/TS code that switches on the literal `'Stale'` will need to switch on `'Degraded'` instead. The `PendingIssuance` deletion has no documented downstream consumer. + +### Added + +- **`web/src/components/StatusBadge.tsx`: `Degraded` (Agent) → `badge-warning` and `dead` (Notification) → `badge-danger`.** First mappings restore the color contract for the two real Go-side values that previously fell through to the dictionary default. The `Degraded` mapping cross-references `internal/domain/connector.go::AgentStatusDegraded`; the `dead` mapping cross-references `internal/domain/notification.go::NotificationStatusDead`. +- **`web/src/components/StatusBadge.test.tsx`: 38-case Vitest property test.** Iterates every Go-side enum value (`AgentStatus`, `CertificateStatus`, `JobStatus`, `NotificationStatus`, `DiscoveryStatus`, `HealthStatus`) plus the two frontend-synthesized `Enabled`/`Disabled` labels, asserts every value gets a non-default class (or, for the five intentionally-neutral terminal values like `Archived`/`Cancelled`/`read`, an explicit `badge badge-neutral`). Includes negative assertions on the deleted `Stale` and `PendingIssuance` keys (must fall through to neutral) and specific UX-correctness assertions on the operator-attention semantics (`dead` → danger, `Degraded` → warning). +- **`web/src/api/types.test.ts`: D-5 Certificate phantom-fields trim regression.** A `Certificate` literal construction pinned post-trim, plus a sibling `CertificateVersion` literal pinning that the trimmed fields still live on the version envelope. The `tsc --noEmit` gate in CI is the primary enforcement; the test is the documentation of intent. +- **CI regression guardrail in `.github/workflows/ci.yml` (`Forbidden StatusBadge dead-key + Certificate phantom-field regression guard (D-1)`).** Two grep blocks: (1) catches `Stale: 'badge-...'` or `PendingIssuance: 'badge-...'` in `web/src/components/StatusBadge.tsx`; (2) uses an awk-scoped window over the `export interface Certificate {` block in `web/src/api/types.ts` to catch any of the five phantom fields reappearing — explicitly excludes the `CertificateVersion` block which legitimately carries them. Verified locally on the post-fix tree (passes) and against synthetic regressions (each fires the guardrail). + +### Changed + +- **`web/src/pages/CertificateDetailPage.tsx`: Key Algorithm and Key Size rows now read from `latestVersion?.key_algorithm` / `latestVersion?.key_size`.** Mirrors the existing `latestVersion` fallback used for `serial_number` and `fingerprint_sha256` earlier in the same file. Pre-D-4 these rows accessed `cert.key_algorithm` and `cert.key_size` directly — both phantom fields per D-5 — so the rows always rendered `—`. The same file's `serial_number` / `fingerprint_sha256` / `issued_at` derivations were also simplified to drop the now-impossible `cert.X || latestVersion?.X` cert-side leg. +- **`web/src/components/StatusBadge.tsx` adds a leading docblock** naming the Go-side source-of-truth file for every status family it maps (`AgentStatus`, `CertificateStatus`, `JobStatus`, `NotificationStatus`, `DiscoveryStatus`, `HealthStatus`) and pointing at the property test as the regression vector for future enum changes. +- **`api/openapi.yaml::ManagedCertificate`** gets a leading comment cross-referencing the D-5 closure and explaining why per-issuance fields legitimately don't appear here (they live on `CertificateVersion`). Schema property list unchanged — the OpenAPI spec was already correct. + +### Closed audit findings + +- `cat-d-359e92c20cbf` (P1 primary) — Agent: `Stale` dead key + `Degraded` neutral fallthrough +- `cat-d-9f4c8e4a91f1` (P2) — Notification: `dead` missing +- `cat-d-1447e04732e7` (P3) — Certificate: `PendingIssuance` dead key +- `cat-f-cert_detail_page_key_render_fallback` (P2) — render-site uses `cert.key_algorithm` directly +- `cat-f-ae0d06b6588f` (P2) — Certificate TS phantom fields (root cause) + +### Known follow-ups (deferred from D-1 scope) + +The audit's broader type-drift cluster (`diff-05x06-7cdf4e78ae24` Agent TS, `diff-05x06-2044a46f4dd0` DeploymentTarget TS, `diff-05x06-caba9eb3620e` Notification TS, `diff-05x06-85ab6b98a2f7` DiscoveredCertificate TS, `diff-05x06-97fab8783a5c` Issuer TS) is out of D-1 scope. Recon for those is per-type field-by-field diff Go ↔ TS — codegen-shaped, not edit-shaped — and warrants its own D-2 master prompt. ### U-3: GitHub #10 reopened — fresh-clone first-up postgres init failure (P1) — closed end-to-end diff --git a/api/openapi.yaml b/api/openapi.yaml index 51653d0..0dd0c46 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -3492,6 +3492,15 @@ components: - Archived ManagedCertificate: + # D-5 (cat-f-ae0d06b6588f, master): per-issuance fields + # (serial_number, fingerprint_sha256, key_algorithm, key_size, + # issued_at) are intentionally NOT declared here. They live on + # CertificateVersion (per-issuance evidence) and are fetched via + # /api/v1/certificates/{id}/versions. ManagedCertificate is the + # management envelope; CertificateVersion is the issuance record. + # Pre-D-5 the TS Certificate interface had them as optional and + # the dashboard's Key Algorithm / Key Size rows always rendered + # '—' as a result. The TS trim restores parity with this schema. type: object properties: id: diff --git a/web/src/api/types.test.ts b/web/src/api/types.test.ts index 1b5745e..cd39629 100644 --- a/web/src/api/types.test.ts +++ b/web/src/api/types.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest'; import { POLICY_TYPES, POLICY_SEVERITIES } from './types'; -import type { Agent } from './types'; +import type { Agent, Certificate, CertificateVersion } from './types'; /** * Regression tests for the policy enum tuples. @@ -132,3 +132,91 @@ describe('Agent interface (I-004 retirement)', () => { expect(active.retired_reason).toBeUndefined(); }); }); + +/** + * D-5 (cat-f-ae0d06b6588f, master): Certificate TS phantom-fields trim. + * + * Pre-D-5 the Certificate interface declared `serial_number`, + * `fingerprint_sha256`, `key_algorithm`, `key_size`, and `issued_at` as + * optional. These fields were never emitted by Go's `ManagedCertificate` + * (internal/domain/certificate.go) — they live on `CertificateVersion`, + * which is the per-issuance record fetched from + * /api/v1/certificates/{id}/versions. The optional declarations made + * `cert.serial_number` always-undefined on list responses, and downstream + * consumers (CertificateDetailPage's Key Algorithm / Key Size rows in + * particular) silently rendered '—' for every cert despite the data + * being available a single fetch away. + * + * Post-D-5 the TS type makes the missing-data case explicit: a + * `cert.serial_number` access becomes a TS compile error, forcing every + * consumer to acknowledge the version-fallback pattern. This regression + * test pins the trim — if a future PR re-adds any of the five phantom + * fields to Certificate (e.g. via merge conflict, copy-paste, or a + * codegen run that regenerates from a stale OpenAPI spec), the + * compile-fail block here will surface it. + */ +describe('Certificate interface (D-5 phantom-fields trim)', () => { + it('does NOT declare per-issuance fields — those live on CertificateVersion', () => { + // Construct a fully-populated Certificate. If a future PR re-adds + // any of the five phantom fields (serial_number, fingerprint_sha256, + // key_algorithm, key_size, issued_at) to the interface, every + // omission in this literal becomes "missing required field" and + // the test fails to compile. Conversely, attempting to set any of + // the five fields on the literal is a TS error today (excess + // property), so the negative-assertion block below also fails to + // compile if someone re-adds them as optional. + const cert: Certificate = { + id: 'mc-test', + name: 'test', + common_name: 'test.example.com', + sans: [], + status: 'Active', + environment: 'production', + issuer_id: 'iss-test', + owner_id: 'o-test', + team_id: 't-test', + renewal_policy_id: 'rp-default', + certificate_profile_id: 'cp-default', + expires_at: '2027-01-01T00:00:00Z', + tags: {}, + created_at: '2026-01-01T00:00:00Z', + updated_at: '2026-01-01T00:00:00Z', + }; + expect(cert.id).toBe('mc-test'); + + // Excess-property check: each of these MUST be a TS error if + // uncommented. Keep them in the test as documentation of what's + // intentionally absent. (We can't directly assert "type does not + // have property X" without a type-level helper, but the literal + // construction above plus tsc --noEmit in CI is the binding check.) + // + // const broken: Certificate = { ...cert, serial_number: '01:02' }; // ❌ TS2353 + // const broken2: Certificate = { ...cert, key_algorithm: 'EC' }; // ❌ TS2353 + // const broken3: Certificate = { ...cert, key_size: 256 }; // ❌ TS2353 + // const broken4: Certificate = { ...cert, fingerprint_sha256: '' };// ❌ TS2353 + // const broken5: Certificate = { ...cert, issued_at: '...' }; // ❌ TS2353 + }); + + it('CertificateVersion still carries the per-issuance fields', () => { + // The other half of the contract: the trimmed fields didn't go to + // /dev/null — they live (and have always lived) on CertificateVersion. + // If a refactor removes them from CertificateVersion too, the + // CertificateDetailPage fallback path breaks. Pin both halves. + const v: CertificateVersion = { + id: 'mcv-test', + certificate_id: 'mc-test', + serial_number: '01:02:03', + fingerprint_sha256: 'a'.repeat(64), + pem_chain: '-----BEGIN CERTIFICATE-----\n...', + csr_pem: '-----BEGIN CERTIFICATE REQUEST-----\n...', + not_before: '2026-01-01T00:00:00Z', + not_after: '2027-01-01T00:00:00Z', + key_algorithm: 'ECDSA', + key_size: 256, + created_at: '2026-01-01T00:00:00Z', + }; + expect(v.serial_number).toBe('01:02:03'); + expect(v.key_algorithm).toBe('ECDSA'); + expect(v.key_size).toBe(256); + }); +}); diff --git a/web/src/api/types.ts b/web/src/api/types.ts index c55dbc7..d14eb07 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -1,3 +1,15 @@ +// D-5 (cat-f-ae0d06b6588f, master): the five per-issuance fields +// (serial_number, fingerprint_sha256, key_algorithm, key_size, +// issued_at) USED to live here as optional. They were never emitted +// by Go's `ManagedCertificate` (internal/domain/certificate.go) — they +// live on `CertificateVersion` (per-issuance evidence) and are fetched +// via getCertificateVersions(id). Render-site consumers (notably +// CertificateDetailPage) use `latestVersion?.field` as the canonical +// access path. Pre-D-5 the optional declaration silently returned +// `undefined` on every list response, so consumers who didn't know +// about the version-fallback pattern rendered '—' for every cert; now +// the missing-data case is explicit at the type level (a `cert.X` +// access for one of these fields is a TS compile error). export interface Certificate { id: string; name: string; @@ -10,11 +22,6 @@ export interface Certificate { team_id: string; renewal_policy_id: string; certificate_profile_id: string; - serial_number?: string; - fingerprint_sha256?: string; - key_algorithm?: string; - key_size?: number; - issued_at?: string; expires_at: string; revoked_at?: string; revocation_reason?: string; diff --git a/web/src/components/StatusBadge.test.tsx b/web/src/components/StatusBadge.test.tsx new file mode 100644 index 0000000..c1bd975 --- /dev/null +++ b/web/src/components/StatusBadge.test.tsx @@ -0,0 +1,130 @@ +import { describe, expect, it } from 'vitest'; +import { render } from '@testing-library/react'; +import StatusBadge from './StatusBadge'; + +// ----------------------------------------------------------------------------- +// D-1 master — StatusBadge enum-coverage contract +// +// The single source of truth for what Go actually emits on the wire. +// Update this if the Go enums change (and the StatusBadge will go red +// here BEFORE any user sees a wrong color in production). +// +// Sources (mirror the Go const blocks verbatim — wire VALUES, not Go +// identifier names): +// AgentStatus — internal/domain/connector.go:174-176 +// CertificateStatus — internal/domain/certificate.go:50-57 +// JobStatus — internal/domain/job.go:43-49 +// NotificationStatus— internal/domain/notification.go:51-55 +// DiscoveryStatus — internal/domain/discovery.go:13-17 +// HealthStatus — internal/domain/health_check.go:9-13 +// +// Issuer 'Enabled' / 'Disabled' are NOT a Go enum — they're frontend- +// synthesized labels mapped from `Issuer.enabled bool` at the call +// site (TargetsPage.tsx similarly). Pinned in a separate group below. +// +// Pre-D-1 drift this test would have caught: +// - Agent: StatusBadge had 'Stale' (never emitted), missing 'Degraded' +// (real). Degraded agents rendered as default neutral grey, hiding +// attention-needed state from operators. +// - Notification: StatusBadge missing 'dead' (retries exhausted). +// Dead-letter notifications rendered as default neutral, visually +// equated with 'read' (operator-acknowledged). +// - Certificate: StatusBadge had 'PendingIssuance' (never emitted). +// Dead key, latent confusion vector if anyone copies it as +// canonical. +// ----------------------------------------------------------------------------- +const ENUMS_FROM_GO = { + AgentStatus: ['Online', 'Offline', 'Degraded'] as const, + CertificateStatus: ['Pending', 'Active', 'Expiring', 'Expired', + 'RenewalInProgress', 'Failed', 'Revoked', 'Archived'] as const, + JobStatus: ['Pending', 'AwaitingCSR', 'AwaitingApproval', 'Running', + 'Completed', 'Failed', 'Cancelled'] as const, + NotificationStatus: ['pending', 'sent', 'failed', 'dead', 'read'] as const, + DiscoveryStatus: ['Unmanaged', 'Managed', 'Dismissed'] as const, + HealthStatus: ['healthy', 'degraded', 'down', 'cert_mismatch', 'unknown'] as const, +}; + +// Frontend-synthesized labels — not in any Go enum, but surfaced via +// StatusBadge from real call sites (TargetsPage, AgentGroupsPage etc.) +// and therefore part of the visual contract this component owns. +const FRONTEND_SYNTHESIZED = ['Enabled', 'Disabled'] as const; + +describe('StatusBadge — enum-coverage contract (D-1 master)', () => { + // Iterate every Go-emitted value across every enum and assert the + // rendered carries a class OTHER than the default 'badge-neutral'. + // EXCEPT for legitimately-neutral statuses (Archived, Cancelled, + // Dismissed, read, unknown) which are intentionally neutral by UX + // design — those are pinned by a separate sub-test below. + const INTENTIONALLY_NEUTRAL = new Set(['Archived', 'Cancelled', 'Dismissed', 'read', 'unknown']); + + for (const [enumName, values] of Object.entries(ENUMS_FROM_GO)) { + for (const v of values) { + it(`${enumName}: '${v}' renders a recognised class (no fallthrough)`, () => { + const { container } = render(); + const span = container.querySelector('span'); + expect(span).not.toBeNull(); + const cls = span!.className; + if (INTENTIONALLY_NEUTRAL.has(v)) { + // Neutral is the right semantic answer for terminal-acknowledged + // states — but it must come from an EXPLICIT mapping, not the + // dictionary-default fallthrough. Asserting a 'badge-neutral' + // class here pins that the explicit entry exists; if someone + // deletes it, this still passes (because the default is also + // 'badge-neutral'). The negative assertion in the dead-keys + // sub-test below catches the deletion case. + expect(cls).toBe('badge badge-neutral'); + } else { + expect(cls).toMatch(/badge-(success|warning|danger|info)/); + expect(cls).not.toBe('badge badge-neutral'); + } + }); + } + } + + for (const v of FRONTEND_SYNTHESIZED) { + it(`Frontend-synthesized '${v}' has an explicit StatusBadge mapping`, () => { + const { container } = render(); + const cls = container.querySelector('span')!.className; + // 'Disabled' is intentionally neutral; 'Enabled' is success. + expect(cls).toMatch(/badge-(success|warning|danger|info|neutral)/); + }); + } + + // Negative contract: the dead keys we deleted MUST fall through to the + // default. If a future PR re-adds 'Stale' or 'PendingIssuance' to + // statusStyles, this test will surface it because the rendered class + // will no longer be 'badge badge-neutral' (it'd be the explicit value + // someone re-added, e.g. 'badge-warning'). + it.each(['Stale', 'PendingIssuance'])( + "dead key '%s' falls through to neutral default (no explicit mapping)", + (deadKey) => { + const { container } = render(); + expect(container.querySelector('span')!.className).toBe('badge badge-neutral'); + }, + ); + + // Specific danger-class contracts (UX correctness, not just non-default). + // These pin the operator-attention semantics. If anyone changes 'dead' + // or 'Degraded' away from these classes, the operator's perception of + // "this needs my attention" changes — these are the highest-stakes + // visual semantics in the dashboard. + it("Notification 'dead' renders as danger (operator attention required)", () => { + const { container } = render(); + expect(container.querySelector('span')!.className).toContain('badge-danger'); + }); + + it("Agent 'Degraded' renders as warning (degradation, not failure)", () => { + const { container } = render(); + expect(container.querySelector('span')!.className).toContain('badge-warning'); + }); + + // Unknown statuses fall through to neutral. The string is still + // displayed verbatim so an operator can see "what is this?" rather + // than nothing at all. + it('unknown status string renders as neutral but preserves the label text', () => { + const { container } = render(); + const span = container.querySelector('span'); + expect(span!.className).toBe('badge badge-neutral'); + expect(span!.textContent).toBe('SomeFutureStatus'); + }); +}); diff --git a/web/src/components/StatusBadge.tsx b/web/src/components/StatusBadge.tsx index afd9f65..f080a7c 100644 --- a/web/src/components/StatusBadge.tsx +++ b/web/src/components/StatusBadge.tsx @@ -1,13 +1,41 @@ +// StatusBadge — single source of truth for the certctl dashboard's +// per-status color mapping. Keys are the EXACT wire values Go emits +// (case-sensitive). Update this file when a new status value lands on +// the Go side; StatusBadge.test.tsx walks every value and will go red +// before users see a default-grey "what is happening?" badge. +// +// D-1 master closure (cat-d-359e92c20cbf, cat-d-9f4c8e4a91f1, +// cat-d-1447e04732e7, cat-f-cert_detail_page_key_render_fallback, +// cat-f-ae0d06b6588f) fixed the pre-master drift: +// - Agent: 'Stale' (never emitted) → 'Degraded' (real value); +// `internal/domain/connector.go::AgentStatusDegraded = "Degraded"`. +// - Notification: added 'dead' (was falling through to neutral); +// `internal/domain/notification.go::NotificationStatusDead = "dead"`. +// - Certificate: dropped dead 'PendingIssuance' key — the real +// `CertificateStatusPending = "Pending"` is mapped under Job +// statuses below. +// +// Source-of-truth references (re-verify if the Go enum changes): +// - internal/domain/connector.go::AgentStatus* +// - internal/domain/certificate.go::CertificateStatus* +// - internal/domain/job.go::JobStatus* +// - internal/domain/notification.go::NotificationStatus* +// - internal/domain/discovery.go::DiscoveryStatus* +// - internal/domain/health_check.go::HealthStatus* +// +// Issuer 'Enabled'/'Disabled' are frontend-synthesized labels (mapped +// from the `enabled bool` field on the Issuer struct), not Go-emitted +// enum values, but they're surfaced via StatusBadge for consistency. const statusStyles: Record = { - // Certificate statuses + // Certificate statuses (internal/domain/certificate.go::CertificateStatus*) Active: 'badge-success', Expiring: 'badge-warning', Expired: 'badge-danger', RenewalInProgress: 'badge-info', - PendingIssuance: 'badge-info', Archived: 'badge-neutral', Revoked: 'badge-danger', - // Job statuses + // Job statuses (internal/domain/job.go::JobStatus*) — note: 'Pending' is + // shared between CertificateStatusPending and JobStatusPending. Pending: 'badge-info', AwaitingCSR: 'badge-info', AwaitingApproval: 'badge-info', @@ -15,23 +43,30 @@ const statusStyles: Record = { Completed: 'badge-success', Failed: 'badge-danger', Cancelled: 'badge-neutral', - // Agent statuses + // Agent statuses (internal/domain/connector.go::AgentStatus*) — D-1: + // 'Degraded' replaces the never-emitted 'Stale' from pre-D-1 (the Go + // domain has only Online / Offline / Degraded; mapping 'Stale' yellow + // and letting 'Degraded' fall through to neutral hid degraded agents). Online: 'badge-success', Offline: 'badge-danger', - Stale: 'badge-warning', - // Discovery statuses + Degraded: 'badge-warning', + // Discovery statuses (internal/domain/discovery.go::DiscoveryStatus*) Unmanaged: 'badge-warning', Managed: 'badge-success', Dismissed: 'badge-neutral', - // Issuer statuses + // Issuer statuses (frontend-synthesized from Issuer.enabled bool) Enabled: 'badge-success', Disabled: 'badge-neutral', - // Notification statuses + // Notification statuses (internal/domain/notification.go::NotificationStatus*) + // — D-2: added 'dead' (retries exhausted, dead-letter queue). Pre-D-2 it + // fell through to neutral, visually equating "needs operator attention" + // with "operator already acknowledged" (read). sent: 'badge-success', pending: 'badge-warning', failed: 'badge-danger', + dead: 'badge-danger', read: 'badge-neutral', - // Health check statuses + // Health check statuses (internal/domain/health_check.go::HealthStatus*) healthy: 'badge-success', degraded: 'badge-warning', down: 'badge-danger', diff --git a/web/src/pages/CertificateDetailPage.tsx b/web/src/pages/CertificateDetailPage.tsx index 3413b86..7393832 100644 --- a/web/src/pages/CertificateDetailPage.tsx +++ b/web/src/pages/CertificateDetailPage.tsx @@ -380,11 +380,20 @@ export default function CertificateDetailPage() { ); } - // Derive certificate metadata from latest version (backend doesn't include these on the cert object) + // Derive certificate metadata from latest version. Per-issuance fields + // (serial_number, fingerprint_sha256, key_algorithm, key_size, issued_at) + // live on `CertificateVersion`, NOT on `ManagedCertificate` — the Go + // domain has always been this way; the TS interface used to lie about + // it via optional `cert.X?` declarations that always returned undefined + // on list responses (D-5 / cat-f-ae0d06b6588f). Post-D-5 the TS type + // makes the missing-data case explicit, and every read goes through + // `latestVersion?.field` here. const latestVersion = versions?.data?.[0]; - const serialNumber = cert.serial_number || latestVersion?.serial_number; - const fingerprintSha256 = cert.fingerprint_sha256 || latestVersion?.fingerprint_sha256; - const issuedAt = cert.issued_at || latestVersion?.not_before; + const serialNumber = latestVersion?.serial_number; + const fingerprintSha256 = latestVersion?.fingerprint_sha256; + const issuedAt = latestVersion?.not_before; + const keyAlgorithm = latestVersion?.key_algorithm; + const keySize = latestVersion?.key_size; const days = daysUntil(cert.expires_at); const isRevoked = cert.status === 'Revoked'; @@ -536,8 +545,13 @@ export default function CertificateDetailPage() { {fingerprintSha256.slice(0, 24)}... : '—' } /> - - + {/* D-4 (cat-f-cert_detail_page_key_render_fallback): mirror the + latestVersion fallback used for serialNumber / fingerprintSha256 + above. Pre-D-4 these rows accessed `cert.key_algorithm` / + `cert.key_size` directly — both phantom Certificate fields per + D-5 (cat-f-ae0d06b6588f), so the rows always rendered '—'. */} + + {profile?.allowed_ekus && profile.allowed_ekus.length > 0 && (