Commit Graph

4 Commits

Author SHA1 Message Date
shankar0123 4f2d865b51 feat(middleware): SEC-008 — Permissions-Policy deny-all-features header
Acquisition-audit SEC-008 closure (Sprint 2 ACQ, 2026-05-16).

Add Permissions-Policy as a sixth security header alongside HSTS,
X-Frame-Options, X-Content-Type-Options, Referrer-Policy, and CSP.
Default value is a deny-all-features baseline:

  accelerometer=(), camera=(), geolocation=(), microphone=(),
  payment=(), usb=(), interest-cohort=()

certctl is a control-plane API + dashboard; no part of the surface
needs camera / microphone / geolocation / accelerometer / payment /
USB access, and `interest-cohort=()` opts out of the deprecated
FLoC browser feature. The deny-all default removes those
attack/fingerprint surfaces if certctl is ever embedded in a
malicious page or if a dashboard route is XSS-compromised
post-CSP-bypass.

Per-field empty-string suppression is preserved: operators who want
to allow a feature (e.g. hardware-attestation flows wanting
WebAuthn's USB transport) can either set Cfg.PermissionsPolicy to
their own narrowed allowlist or set it to "" to suppress the
header entirely.

Tests:
  - TestSecurityHeaders_PermissionsPolicyDefault — pins the literal
    default value byte-for-byte so any widening (e.g. someone adding
    camera=*) breaks the test.
  - TestSecurityHeaders_PermissionsPolicyOverrideToEmptySuppresses —
    pins the operator escape hatch and that the per-field
    suppression contract still holds field-by-field.
  - TestSecurityHeaders_DefaultsAllPresent gains Permissions-Policy
    in its loop, so the existing on-error and on-2xx paths now
    cover the new header too.

The middleware pre-trim slice capacity bumps from 5 → 6 entries.
2026-05-16 17:13:17 +00:00
shankar0123 7268d12a17 feat(web): close FE-M6 — migrate static inline-style attrs to Tailwind + correct CSP rationale comment
Closes frontend-design-audit finding FE-M6 (Med):

  CSP allows 'unsafe-inline' for `style-src` — necessary today
  because of inline SVG `style=` attrs (related to FE-H2)

═══════════════════════════ GROUND-TRUTH FINDINGS ═══════════════════

Ground-truth recon found 4 audit-framing errors:

(1) The "17 inline-style tsx files" count was stale — actual is 9
    (8 after excluding a Layout.tsx comment match the audit's grep
    counted).

(2) The CSP rationale comment at securityheaders.go:35 LIED about
    WHY 'unsafe-inline' is needed. It claimed "Tailwind (via Vite)
    injects per-component <style> blocks at build time." Verified
    against the post-build artifact: `grep -c '<style' dist/index.html`
    = 0; Vite's CSS output is a single .css file linked via
    `<link rel="stylesheet">`. The 'unsafe-inline' grant exists for
    React's `style={...}` attribute model, NOT for Vite or Tailwind.

(3) The 9 sites split cleanly into:
    LOAD-BEARING DYNAMIC (5 sites; can't be Tailwind utilities
    because values are computed at runtime):
      - Tooltip.tsx Floating-UI position (left/top px per-tick)
      - AgentFleetPage.tsx dynamic color+width chart bars
      - dashboard/charts.tsx Recharts color props
      - CertificatesPage.tsx progress-bar percent width
      - IssuerHierarchyPage.tsx depth-based marginLeft
    STATIC PIXEL VALUES (3 files, ~12 sites; clean Tailwind
    migration targets):
      - UsersPage.tsx — filter UI + table styling
      - DigestPage.tsx — iframe min-height
      - AuthProvider.tsx — demo-mode banner

(4) Fully eliminating 'unsafe-inline' would require either banning
    dynamic `style={...}` (CSS-in-JS rewrite of the 5 load-bearing
    sites) or adopting CSP nonces with React 18+'s style runtime.
    Neither fits the original FE-M6 phase budget.

═══════════════════════════ CHANGES ═══════════════════════════════

web/src/pages/auth/UsersPage.tsx:
  9 inline-style attrs → Tailwind utility classes. The filter UI
  (mb-4, mr-2, w-[280px] p-1), the table (w-full border-collapse),
  the thead row (border-b-2 border-gray-300 text-left), per-row
  borders (border-b border-gray-200 + opacity-50/100 conditional),
  buttons (px-3 py-1), the empty-state cell (p-3 text-center).
  Behavior-preserving.

web/src/pages/DigestPage.tsx:
  iframe `style={{ minHeight: '600px' }}` → className "min-h-[600px]"
  (composed into the existing className).

web/src/components/AuthProvider.tsx:
  Demo-mode banner: 6-prop `style={{ background, color, padding,
  fontSize, fontWeight, textAlign }}` → className "bg-red-700
  text-white px-4 py-2 text-[13px] font-semibold text-center".
  Same visual.

internal/api/middleware/securityheaders.go:
  CSP rationale comment rewritten to accurately describe WHY
  'unsafe-inline' is required. New comment:
    - Names the 5 load-bearing dynamic-style sites explicitly
    - Lists the 3 static sites that were migrated to Tailwind today
    - Documents that the OLD comment's "Tailwind/Vite injects
      <style> blocks" claim was factually wrong (verified against
      built dist/index.html — zero <style> tags emitted)
    - Records the future-tightening path (React style-runtime
      nonces OR CSS-in-JS rewrite of the 5 sites) and notes it
      doesn't fit the original FE-M6 phase budget

═══════════════════════════ AUDIT FRAMING ════════════════════════

The audit said FE-M6 was about "inline SVG style= attrs (related
to FE-H2)." Ground-truth: FE-H2 (Phase 3 Layout SVG → Lucide
icons) ALREADY happened; the remaining inline-style sites have
nothing to do with SVGs. The audit's bridge from FE-H2 → FE-M6
was a red herring.

The OPERATOR-VISIBLE win from this closure:
  • 3 production tsx files now use Tailwind utility classes for
    static styling — consistent with the rest of the codebase.
  • The CSP comment now tells the truth about why 'unsafe-inline'
    is needed, so the next operator who reads it doesn't waste
    time hunting for non-existent <style> blocks.
  • The inline-style attribute surface is reduced to ONLY
    load-bearing dynamic styling — making any future tightening
    work (nonces, CSS-in-JS migration) easier to scope.

The CSP header itself is UNCHANGED ("style-src 'self'
'unsafe-inline'"). True elimination of 'unsafe-inline' is a
separate workstream tracked in the corrected comment.

═══════════════════════════ VERIFICATION ═══════════════════════════

  • gofmt -l internal/api/middleware/securityheaders.go — clean
  • go vet ./internal/api/middleware/... — exit 0
  • go test -short -count=1 ./internal/api/middleware/... —
    ok 0.247s (existing securityheaders_test.go pins the
    Content-Security-Policy header value byte-string; unchanged
    by this commit so test stays green)
  • npx tsc --noEmit — exit 0
  • npx vitest run AuthProvider DigestPage UsersPage — 16/16 pass
  • npx vite build — built in 3.42s

Ground-truth: origin/master tip 9ba5ee4 (P-M2 just pushed)
verified via GitHub API BEFORE commit.

Falsifiable proof: a future engineer reading securityheaders.go:35
sees an accurate explanation of why 'unsafe-inline' is needed,
NOT the previous false "Tailwind/Vite" claim.
2026-05-14 20:40:55 +00:00
shankar0123 21aeed4f4e legal: addlicense headers + normalize legacy variants (Phase 0 RED-4)
Phase 0 closure (Path B2, post-rewrite):

addlicense sweep — adds the canonical certctl LLC copyright + BUSL-1.1
SPDX header to every production Go file. Template:

  // Copyright 2026 certctl LLC. All rights reserved.
  // SPDX-License-Identifier: BUSL-1.1

Coverage: 338 / 338 production Go files (cmd/ + internal/, excluding
*_test.go and **/testdata/**). Pre-sweep coverage was 22 / 338 (6.5%);
post-sweep is 338 / 338 (100%).

Normalized 22 pre-existing legacy headers (`// Copyright (c) certctl`
+ `// SPDX-License-Identifier: BSL-1.1`) and 1 file using a
`Certctl Contributors` attribution. The legacy SPDX ID `BSL-1.1`
is non-standard; the official SPDX identifier for Business Source
License 1.1 is `BUSL-1.1` (capital U). All 338 files now share the
canonical form.

Generated via:
  addlicense -c "certctl LLC" -y 2026 \
    -f cowork/legal/copyright-header.tpl \
    -ignore '**/testdata/**' -ignore '**/*_test.go' \
    cmd/ internal/

Verification:
  find cmd internal -name '*.go' -not -name '*_test.go' \
    -not -path '*/testdata/*' \
    -exec grep -L '^// Copyright 2026 certctl LLC' {} \; | wc -l

  Returns: 0

gofmt clean. Header additions are comments only, no compile impact.

Closes: cowork/certctl-architecture-diligence-audit.html#fix-RED-4
2026-05-13 21:23:35 +00:00
shankar0123 3e78ecb799 feat(security): bodyLimit on noAuth + security headers + encryption-key validation (H-1 master)
Closes three 2026-04-24 audit findings (all P2):
  - cat-s5-4936a1cf0118: noAuthHandler chain accepted arbitrary-size
    bodies (EST simpleenroll, SCEP, PKI CRL/OCSP, /health, /ready).
    Memory exhaustion vector without HTTP-layer auth gatekeeping.
  - cat-s11-missing_security_headers: zero security headers on any
    response. Clickjacking, MIME-sniffing, untrusted-origin resource
    loads against the dashboard and API.
  - cat-r-encryption_key_no_length_validation: CERTCTL_CONFIG_ENCRYPTION_KEY
    accepted with any non-empty value including a single character.
    PBKDF2-SHA256 (100k rounds) does not compensate for low-entropy
    passphrases at scale (CWE-916, CWE-329).

Changes:
- cmd/server/main.go::noAuthHandler chain — added bodyLimitMiddleware
  + securityHeadersMiddleware. Same default cap as authed surface
  (1MB via CERTCTL_MAX_BODY_SIZE), same 413 on overflow.
- cmd/server/main.go::middlewareStack (authed) — added
  securityHeadersMiddleware before corsMiddleware.
- internal/api/middleware/securityheaders.go (new) — SecurityHeaders
  middleware + SecurityHeadersDefaults() with conservative defaults:
  HSTS 1y+includeSubDomains, X-Frame-Options DENY, X-Content-Type-
  Options nosniff, Referrer-Policy no-referrer-when-downgrade, CSP
  default-src 'self' + img/data + style 'unsafe-inline' (Tailwind/Vite
  needs it; scripts still 'self' only) + connect 'self' + frame-
  ancestors 'none'. Operators behind a customising reverse proxy can
  disable any header by setting its config field to empty.
- internal/config/config.go::Validate() — enforce minEncryptionKeyLength
  = 32 bytes when CERTCTL_CONFIG_ENCRYPTION_KEY is set. Empty stays
  accepted (downstream fail-closed sentinel handles it). Structured
  error names the env var, the actual length, the required minimum,
  and the canonical generation command (`openssl rand -base64 32`).

Tests:
- internal/api/middleware/securityheaders_test.go (new) — 4 cases
  (defaults present, empty value disables single header, override
  applied, headers on 4xx/5xx).
- internal/config/config_test.go — 5 new cases for the encryption-key
  length check (empty accepted, 1-byte rejected, 31-byte rejected at
  boundary, 32-byte accepted, 44-byte realistic operator key accepted).

Documentation:
- CHANGELOG.md — H-1 section above D-2 under [unreleased] with
  Breaking-change callout (operators with low-entropy keys must rotate
  before upgrade).
- coverage-gap-audit-2026-04-24-v5/unified-audit.md — Live Tracker
  25/47 → 33/47, P1 14/14 (zero remaining), P2 11/27 → 16/27. Three
  H-1 findings flipped + closed-bundle row added.

Verification:
- go build ./... — clean
- go vet ./... — clean
- golangci-lint v2.11.4 run ./... — 0 issues
- go test ./internal/api/middleware/... — pass (incl. 4 new
  SecurityHeaders cases)
- go test ./internal/config/... — pass (incl. 5 new EncryptionKey
  cases)
- tsc --noEmit (frontend) — clean
- All sibling guardrails (S-1 / G-3 / D-1 / D-2 / B-1 / L-1) still pass

Audit findings closed:
- cat-s5-4936a1cf0118 (P2)
- cat-s11-missing_security_headers (P2)
- cat-r-encryption_key_no_length_validation (P2)

Breaking change:
- Operators with CERTCTL_CONFIG_ENCRYPTION_KEY shorter than 32 bytes
  must rotate before upgrade. Generate via `openssl rand -base64 32`.

Deferred follow-ups:
- Weak-key dictionary check (reject password123, common ASCII patterns)
  — adds operational friction with low marginal entropy gain at the
  32-byte minimum.
- CSP 'unsafe-inline' for styles — required for Tailwind/Vite
  per-component <style> blocks; removing requires HTML report or
  component refactor outside H-1 scope.
- Permissions-Policy header — dashboard uses no advanced browser APIs
  (camera, mic, geolocation); deferred until a real consumer needs it.
2026-04-25 16:40:21 +00:00