mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 23:42:00 +00:00
7268d12a17
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.
126 lines
5.3 KiB
Go
126 lines
5.3 KiB
Go
// Copyright 2026 certctl LLC. All rights reserved.
|
|
// SPDX-License-Identifier: BUSL-1.1
|
|
|
|
package middleware
|
|
|
|
import (
|
|
"net/http"
|
|
"strings"
|
|
)
|
|
|
|
// SecurityHeadersConfig configures the SecurityHeaders middleware.
|
|
//
|
|
// Each field is the literal value to send. An empty string means
|
|
// "do not send this header" — operators behind a customising reverse
|
|
// proxy can disable any header per-deployment without touching code.
|
|
// Defaults are applied via SecurityHeadersDefaults() which encodes
|
|
// the H-1 closure's recommended baseline for an HTTPS-only API+UI
|
|
// host: HSTS, deny-frame, no-MIME-sniff, conservative CSP, and a
|
|
// no-referrer-when-downgrade fallback.
|
|
//
|
|
// H-1 closure (cat-s11-missing_security_headers).
|
|
type SecurityHeadersConfig struct {
|
|
HSTS string // Strict-Transport-Security
|
|
FrameOptions string // X-Frame-Options
|
|
ContentTypeOptions string // X-Content-Type-Options
|
|
ReferrerPolicy string // Referrer-Policy
|
|
ContentSecurityPolicy string // Content-Security-Policy
|
|
}
|
|
|
|
// SecurityHeadersDefaults returns a recommended baseline.
|
|
//
|
|
// CSP: default-src 'self' confines fetches to the same origin.
|
|
// img-src 'self' data: allows inline base64 images (used by the
|
|
// dashboard's certctl-logo and a few status icons).
|
|
// style-src 'self' 'unsafe-inline' — the 'unsafe-inline' grant
|
|
// is required by React's inline `style={...}` attribute model,
|
|
// which emits HTML `style="..."` attributes that the browser
|
|
// treats as inline styles for CSP purposes. The dashboard has 5
|
|
// load-bearing dynamic-style sites: Tooltip's Floating-UI
|
|
// position (left/top px values computed per-tick),
|
|
// AgentFleetPage's dynamic color+width chart bars,
|
|
// dashboard/charts.tsx Recharts color props, CertificatesPage's
|
|
// progress-bar percent width, IssuerHierarchyPage's depth-based
|
|
// marginLeft. The static-pixel uses (UsersPage filter + table UI,
|
|
// DigestPage iframe min-height, AuthProvider demo-mode banner)
|
|
// were migrated to Tailwind utility classes via FE-M6 closure
|
|
// 2026-05-14.
|
|
//
|
|
// FE-M6 audit-framing correction: this comment USED TO say
|
|
// "Tailwind (via Vite) injects per-component <style> blocks at
|
|
// build time." That was factually wrong. Vite's CSS output is a
|
|
// single .css file linked via <link rel="stylesheet"> — verified
|
|
// against dist/index.html post-build: zero <style> tags emitted.
|
|
// The 'unsafe-inline' grant exists for React's style-attribute
|
|
// output path, not for Vite or Tailwind.
|
|
//
|
|
// Fully eliminating 'unsafe-inline' would require either banning
|
|
// dynamic `style={...}` (rewriting the 5 load-bearing sites with
|
|
// a CSS-in-JS library that emits hashed/nonce'd <style> blocks)
|
|
// or adopting CSP nonces with React 18+'s style runtime. Neither
|
|
// fits the original FE-M6 phase budget; tracked as a future
|
|
// security-hardening item.
|
|
//
|
|
// 'unsafe-inline' is intentionally NOT in script-src — the
|
|
// front-end ships as a bundled JS file, no inline scripts.
|
|
//
|
|
// HSTS: 1-year max-age + includeSubDomains. No `preload` directive
|
|
// because preload submission requires explicit operator action and
|
|
// the deployment topology may not span all subdomains.
|
|
//
|
|
// X-Frame-Options: DENY — the dashboard does not need to be embedded
|
|
// anywhere, and DENY is more conservative than SAMEORIGIN against
|
|
// clickjacking via subdomain takeover.
|
|
//
|
|
// X-Content-Type-Options: nosniff — prevent MIME sniffing on
|
|
// JSON/PEM responses that browsers might otherwise interpret as HTML.
|
|
//
|
|
// Referrer-Policy: no-referrer-when-downgrade — preserves Referer
|
|
// for same-origin navigation (useful for support/diagnostics) but
|
|
// strips it on HTTPS→HTTP transitions.
|
|
func SecurityHeadersDefaults() SecurityHeadersConfig {
|
|
return SecurityHeadersConfig{
|
|
HSTS: "max-age=31536000; includeSubDomains",
|
|
FrameOptions: "DENY",
|
|
ContentTypeOptions: "nosniff",
|
|
ReferrerPolicy: "no-referrer-when-downgrade",
|
|
ContentSecurityPolicy: "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; script-src 'self'; connect-src 'self'; frame-ancestors 'none'",
|
|
}
|
|
}
|
|
|
|
// SecurityHeaders returns a middleware that applies the configured
|
|
// HTTP response headers on every response. Headers configured to the
|
|
// empty string are omitted (operator opted out for that deployment).
|
|
//
|
|
// Apply BEFORE the audit middleware so headers reach 4xx/5xx responses
|
|
// — which is where header omissions matter most for the security
|
|
// posture (an attacker probing for misconfiguration sees the same
|
|
// headers on a 401 as on a 200).
|
|
func SecurityHeaders(cfg SecurityHeadersConfig) func(http.Handler) http.Handler {
|
|
// Pre-trim each value once; the per-request hot path stays a
|
|
// straight set of map writes.
|
|
type headerEntry struct{ name, value string }
|
|
entries := make([]headerEntry, 0, 5)
|
|
add := func(name, value string) {
|
|
v := strings.TrimSpace(value)
|
|
if v != "" {
|
|
entries = append(entries, headerEntry{name, v})
|
|
}
|
|
}
|
|
add("Strict-Transport-Security", cfg.HSTS)
|
|
add("X-Frame-Options", cfg.FrameOptions)
|
|
add("X-Content-Type-Options", cfg.ContentTypeOptions)
|
|
add("Referrer-Policy", cfg.ReferrerPolicy)
|
|
add("Content-Security-Policy", cfg.ContentSecurityPolicy)
|
|
|
|
return func(next http.Handler) http.Handler {
|
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
h := w.Header()
|
|
for _, e := range entries {
|
|
h.Set(e.name, e.value)
|
|
}
|
|
next.ServeHTTP(w, r)
|
|
})
|
|
}
|
|
}
|