core-lead/molecule-core
1
0
Fork 0
forked from molecule-ai/molecule-core
Code Pull Requests Activity

fix(middleware): split CSP by route type — strict for API, permissive for canvas (#450)

Browse Source 
API routes return JSON and never need 'unsafe-inline' or 'unsafe-eval'.
Serving those directives globally defeated the purpose of CSP and gave
false security assurance. Canvas-proxied routes (NoRoute → Next.js) keep
'unsafe-inline' because React hydration requires it; 'unsafe-eval' was
already absent and is confirmed unnecessary in production builds.

Implementation:
- Add isAPIPath() helper with an explicit prefix allowlist that mirrors
  the routes registered in router/router.go
- Strict "default-src 'self'" on all /workspaces, /registry, /health,
  /admin, /metrics, /settings, /bundles, /org, /templates, /plugins,
  /webhooks, /channels, /ws, /events, /approvals paths
- Permissive CSP (unsafe-inline, no unsafe-eval) on canvas/NoRoute paths
- 4 new test functions: TestCSPAPIRoutesGetStrictPolicy (covers every
  prefix + sub-path), TestCSPCanvasRoutesGetPermissivePolicy, and
  TestIsAPIPath unit test including substring-non-match guard

Resolves #450

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Molecule AI Backend Engineer 2026-04-16 20:26:17 +00:00
parent 57d9e23211
commit a84a33523c
2 changed files with 210 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

110
platform/internal/middleware/securityheaders.go
View File

@ -1,52 +1,102 @@
package middleware
import "github.com/gin-gonic/gin"
import (
"strings"
"github.com/gin-gonic/gin"
)
// apiPrefixes lists the URL path prefixes that are served by Go platform
// handlers (JSON/binary responses). Canvas-proxied routes (Next.js HTML) are
// everything not in this list — they require 'unsafe-inline' for hydration.
//
// Keep this in sync with the routes registered in router/router.go. A path
// not on this list gets the permissive (canvas-compatible) CSP, which is
// acceptable: adding a new API prefix here is an opt-in tightening, never a
// silent breakage.
var apiPrefixes = []string{
"/workspaces",
"/registry",
"/health",
"/admin",
"/metrics",
"/settings",
"/bundles",
"/org",
"/templates",
"/plugins",
"/webhooks",
"/channels",
"/ws",
"/events",
"/approvals",
}
// isAPIPath reports whether a URL path belongs to a Go platform handler.
// Such paths return JSON and do not need 'unsafe-inline' in their CSP.
// Canvas-proxied paths (NoRoute → Next.js) are anything not matched here.
func isAPIPath(path string) bool {
for _, prefix := range apiPrefixes {
if path == prefix || strings.HasPrefix(path, prefix+"/") {
return true
}
}
return false
}
// SecurityHeaders returns a Gin middleware that sets standard HTTP security
// headers on every response to mitigate common web-application attacks:
//
// - X-Content-Type-Options: nosniff — prevents MIME-type sniffing
// - X-Frame-Options: DENY — blocks iframe embedding (clickjacking)
// - Content-Security-Policy: default-src 'self' — restricts resource loading to same origin
// - Content-Security-Policy — two tiers (see below)
// - Strict-Transport-Security: max-age=31536000; includeSubDomains — enforces HTTPS for 1 year
// - Referrer-Policy: strict-origin-when-cross-origin — avoids leaking full paths/queries in Referer
// - Permissions-Policy: camera=(), microphone=(), geolocation=() — denies sensor access for embedded content
//
// CSP tiers (fix for #450):
//
// 1. API routes (/workspaces, /registry, /health, …) — return JSON, not HTML.
// Strict "default-src 'self'" with no unsafe directives. XSS in a JSON
// response body is not executable without being reflected into an HTML
// page, so the permissive directives would only provide false assurance.
//
// 2. Canvas-proxied routes (NoRoute → Next.js) — serve HTML with inline
// scripts required for Next.js hydration. 'unsafe-inline' is kept here
// because removing it breaks the canvas. 'unsafe-eval' was dropped after
// confirming the production build renders without it.
func SecurityHeaders() gin.HandlerFunc {
return func(c *gin.Context) {
c.Header("X-Content-Type-Options", "nosniff")
c.Header("X-Frame-Options", "DENY")
c.Header("Strict-Transport-Security", "max-age=31536000; includeSubDomains")
// #282: these two were documented in CLAUDE.md but missing from
// the middleware. Referrer-Policy prevents browsers from leaking
// the full Referer URL to cross-origin resources (which can
// expose internal paths/queries). Permissions-Policy denies
// sensor access by default — especially relevant because the
// canvas embeds iframes for Langfuse traces.
// #282: Referrer-Policy prevents browsers from leaking the full Referer
// URL to cross-origin resources (which can expose internal paths/queries).
// Permissions-Policy denies sensor access by default — especially relevant
// because the canvas embeds iframes for Langfuse traces.
c.Header("Referrer-Policy", "strict-origin-when-cross-origin")
c.Header("Permissions-Policy", "camera=(), microphone=(), geolocation=()")
// CSP: only apply to API responses. Canvas-proxied routes
// (NoRoute → reverse-proxy to Next.js) serve HTML with inline
// scripts + styles that `default-src 'self'` blocks. Next.js
// sets its own CSP via <meta> tags. The Go middleware should
// not override it for proxied HTML responses.
//
// Detection: API routes are registered explicitly in the router;
// canvas-proxied routes hit NoRoute. We can't detect NoRoute
// before c.Next() fires, so instead we check the response
// Content-Type after Next() — but that's too late for headers.
//
// Simpler: apply a permissive CSP that allows Next.js to work.
// 'unsafe-inline' is needed for Next.js standalone builds that
// inject inline scripts for hydration. 'unsafe-eval' was dropped
// after confirming production canvas renders without it.
c.Header("Content-Security-Policy",
"default-src 'self'; "+
"script-src 'self' 'unsafe-inline'; "+
"style-src 'self' 'unsafe-inline'; "+
"img-src 'self' data: blob:; "+
"connect-src 'self' ws: wss:; "+
"font-src 'self' data:")
// #450: differentiate CSP by route type.
if isAPIPath(c.Request.URL.Path) {
// API routes return JSON — no inline scripts are ever needed.
// A strict CSP here is meaningful: it prevents a hypothetical
// reflected-XSS payload in an error message from executing if
// the JSON is ever mistakenly served with a text/html content-type.
c.Header("Content-Security-Policy", "default-src 'self'")
} else {
// Canvas routes (NoRoute → reverse-proxy to Next.js) serve HTML
// that requires inline script injection for React hydration.
// 'unsafe-eval' was deliberately removed — Next.js production
// builds do not require it; only the dev server does.
c.Header("Content-Security-Policy",
"default-src 'self'; "+
"script-src 'self' 'unsafe-inline'; "+
"style-src 'self' 'unsafe-inline'; "+
"img-src 'self' data: blob:; "+
"connect-src 'self' ws: wss:; "+
"font-src 'self' data:")
}
c.Next()
}
}

139
platform/internal/middleware/securityheaders_test.go
View File

@ -48,11 +48,9 @@ func TestSecurityHeaders(t *testing.T) {
}
}
// CSP: widened to allow Next.js inline scripts/styles + data:/blob:
// images because the canvas is reverse-proxied through the same
// gin middleware stack. Assert the policy starts with the tight
// default-src and contains each expected directive — exact-match
// would brittle-break every time we tune a subsource list.
// /test is not a registered API prefix → canvas-style permissive CSP.
// Fragment-match rather than exact — CSP subsource lists may be tuned
// without changing the security posture.
csp := w.Header().Get("Content-Security-Policy")
for _, fragment := range []string{
"default-src 'self'",
@ -94,10 +92,7 @@ func TestSecurityHeadersPresenceOnMultipleRoutes(t *testing.T) {
if v := w2.Header().Get("Strict-Transport-Security"); v != "max-age=31536000; includeSubDomains" {
t.Errorf("POST /b: Strict-Transport-Security = %q, want max-age=31536000; includeSubDomains", v)
}
// Fragment-match rather than exact — CSP subsource lists get tuned
// without changing the security posture. Test intent is "CSP is
// present + starts with the tight default-src", not "CSP matches
// this exact byte-for-byte string".
// /a and /b are not API prefixes → canvas-style permissive CSP.
csp := w2.Header().Get("Content-Security-Policy")
if !strings.Contains(csp, "default-src 'self'") {
t.Errorf("POST /b: CSP missing default-src 'self' (full: %q)", csp)
@ -124,3 +119,129 @@ func TestSecurityHeadersDoNotOverrideExisting(t *testing.T) {
t.Errorf("expected handler override SAMEORIGIN, got %q", got)
}
}
// TestCSPAPIRoutesGetStrictPolicy verifies that all registered Go platform
// API prefixes receive a strict "default-src 'self'" CSP with no unsafe
// directives. This is the core fix for issue #450.
func TestCSPAPIRoutesGetStrictPolicy(t *testing.T) {
r := gin.New()
r.Use(SecurityHeaders())
// Register representative routes for each API prefix.
for _, prefix := range apiPrefixes {
prefix := prefix // capture
r.GET(prefix, func(c *gin.Context) { c.JSON(http.StatusOK, nil) })
r.GET(prefix+"/sub", func(c *gin.Context) { c.JSON(http.StatusOK, nil) })
}
strictCSP := "default-src 'self'"
paths := make([]string, 0, len(apiPrefixes)*2)
for _, p := range apiPrefixes {
paths = append(paths, p, p+"/sub")
}
for _, path := range paths {
t.Run(path, func(t *testing.T) {
w := httptest.NewRecorder()
req, _ := http.NewRequest(http.MethodGet, path, nil)
r.ServeHTTP(w, req)
csp := w.Header().Get("Content-Security-Policy")
if csp != strictCSP {
t.Errorf("API path %q: want strict CSP %q, got %q", path, strictCSP, csp)
}
// Belt-and-suspenders: confirm no unsafe directives leak through.
for _, bad := range []string{"unsafe-inline", "unsafe-eval"} {
if strings.Contains(csp, bad) {
t.Errorf("API path %q: CSP must not contain %q, got %q", path, bad, csp)
}
}
})
}
}
// TestCSPCanvasRoutesGetPermissivePolicy verifies that paths not in the API
// prefix list receive the permissive CSP needed for Next.js hydration.
func TestCSPCanvasRoutesGetPermissivePolicy(t *testing.T) {
r := gin.New()
r.Use(SecurityHeaders())
// Simulate canvas/NoRoute paths — register them explicitly so Gin
// doesn't 404 before reaching our middleware.
r.GET("/", func(c *gin.Context) { c.String(http.StatusOK, "<html/>") })
r.GET("/canvas", func(c *gin.Context) { c.String(http.StatusOK, "<html/>") })
r.GET("/canvas/some-page", func(c *gin.Context) { c.String(http.StatusOK, "<html/>") })
r.GET("/some-unknown-path", func(c *gin.Context) { c.String(http.StatusOK, "<html/>") })
canvasPaths := []string{
"/",
"/canvas",
"/canvas/some-page",
"/some-unknown-path",
}
for _, path := range canvasPaths {
t.Run(path, func(t *testing.T) {
w := httptest.NewRecorder()
req, _ := http.NewRequest(http.MethodGet, path, nil)
r.ServeHTTP(w, req)
csp := w.Header().Get("Content-Security-Policy")
// Canvas CSP must contain unsafe-inline for Next.js hydration.
if !strings.Contains(csp, "'unsafe-inline'") {
t.Errorf("canvas path %q: CSP should contain 'unsafe-inline' for Next.js, got %q", path, csp)
}
// 'unsafe-eval' must NOT be present — it was removed after
// confirming production canvas renders without it.
if strings.Contains(csp, "'unsafe-eval'") {
t.Errorf("canvas path %q: CSP must not contain 'unsafe-eval', got %q", path, csp)
}
})
}
}
// TestIsAPIPath unit-tests the path classifier directly.
func TestIsAPIPath(t *testing.T) {
cases := []struct {
path string
want bool
}{
// Exact prefix matches
{"/workspaces", true},
{"/health", true},
{"/admin", true},
{"/metrics", true},
{"/registry", true},
{"/settings", true},
{"/bundles", true},
{"/org", true},
{"/templates", true},
{"/plugins", true},
{"/webhooks", true},
{"/channels", true},
{"/ws", true},
{"/events", true},
{"/approvals", true},
// Sub-paths
{"/workspaces/abc-123", true},
{"/workspaces/abc-123/state", true},
{"/registry/discover/xyz", true},
{"/admin/liveness", true},
// Canvas / non-API paths
{"/", false},
{"/canvas", false},
{"/canvas/viewport", false}, // returned by Next.js canvas page, not the Go API
{"/some-page", false},
{"/_next/static/chunks/main.js", false},
// Ensure prefix is not a substring match (e.g. "/workspaces" should
// not match "/workspacesXXX").
{"/workspacesX", false},
{"/healthcheck", false},
}
for _, tc := range cases {
got := isAPIPath(tc.path)
if got != tc.want {
t.Errorf("isAPIPath(%q) = %v, want %v", tc.path, got, tc.want)
}
}
}