diff --git a/platform/internal/router/canvas_proxy.go b/platform/internal/router/canvas_proxy.go index 36b6ecff..2b1657f9 100644 --- a/platform/internal/router/canvas_proxy.go +++ b/platform/internal/router/canvas_proxy.go @@ -19,6 +19,11 @@ import ( // (Next.js checks Host in dev mode). Response headers from canvas flow back // to the client unchanged. // +// Security: Authorization and Cookie headers are stripped before forwarding. +// Workspace bearer tokens must not reach the Next.js process — canvas has no +// token-validation logic and an unpatched Next.js route could echo them back +// to an attacker via an error page or debug endpoint (issue #451). +// // Why NoRoute + proxy instead of nginx: one fewer process, one fewer config // file, and the Go router already knows which routes are API routes. Any // path not registered as an API route is a canvas page by elimination. @@ -33,6 +38,10 @@ func newCanvasProxy(targetURL string) gin.HandlerFunc { req.URL.Scheme = target.Scheme req.URL.Host = target.Host req.Host = target.Host + // N2 (issue #451): strip credential headers — workspace bearer + // tokens and session cookies must not reach the canvas process. + req.Header.Del("Authorization") + req.Header.Del("Cookie") }, ErrorHandler: func(w http.ResponseWriter, _ *http.Request, err error) { log.Printf("canvas_proxy: %v", err) diff --git a/platform/internal/router/canvas_proxy_test.go b/platform/internal/router/canvas_proxy_test.go new file mode 100644 index 00000000..d85152b5 --- /dev/null +++ b/platform/internal/router/canvas_proxy_test.go @@ -0,0 +1,121 @@ +package router + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" +) + +// TestCanvasProxy_StripsAuthorizationHeader verifies that workspace bearer +// tokens are NOT forwarded to the canvas Next.js server (issue #451 / N2). +// A compromised or unpatched Next.js route could echo the token back to an +// attacker; stripping it at the proxy layer is the safe default. +func TestCanvasProxy_StripsAuthorizationHeader(t *testing.T) { + var capturedAuth string + + // Stand up a tiny upstream that records what headers it received. + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedAuth = r.Header.Get("Authorization") + w.WriteHeader(http.StatusOK) + })) + defer upstream.Close() + + gin.SetMode(gin.TestMode) + engine := gin.New() + engine.NoRoute(newCanvasProxy(upstream.URL)) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/some-canvas-page", nil) + req.Header.Set("Authorization", "Bearer ws-secret-token") + engine.ServeHTTP(w, req) + + if capturedAuth != "" { + t.Errorf("Authorization header must not reach canvas upstream, got %q", capturedAuth) + } +} + +// TestCanvasProxy_StripsCookieHeader verifies that session cookies are not +// forwarded to the canvas Next.js server (same rationale as Authorization). +func TestCanvasProxy_StripsCookieHeader(t *testing.T) { + var capturedCookie string + + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedCookie = r.Header.Get("Cookie") + w.WriteHeader(http.StatusOK) + })) + defer upstream.Close() + + gin.SetMode(gin.TestMode) + engine := gin.New() + engine.NoRoute(newCanvasProxy(upstream.URL)) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/canvas-route", nil) + req.Header.Set("Cookie", "session=abc123; auth=secret") + engine.ServeHTTP(w, req) + + if capturedCookie != "" { + t.Errorf("Cookie header must not reach canvas upstream, got %q", capturedCookie) + } +} + +// TestCanvasProxy_ForwardsOtherHeaders verifies that non-credential headers +// (e.g. Accept, X-Request-ID) still reach the upstream — stripping is +// surgical, not a blanket header wipe. +func TestCanvasProxy_ForwardsOtherHeaders(t *testing.T) { + var capturedAccept, capturedRequestID string + + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedAccept = r.Header.Get("Accept") + capturedRequestID = r.Header.Get("X-Request-Id") + w.WriteHeader(http.StatusOK) + })) + defer upstream.Close() + + gin.SetMode(gin.TestMode) + engine := gin.New() + engine.NoRoute(newCanvasProxy(upstream.URL)) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/page", nil) + req.Header.Set("Accept", "text/html") + req.Header.Set("X-Request-Id", "trace-abc") + req.Header.Set("Authorization", "Bearer should-be-stripped") + engine.ServeHTTP(w, req) + + if capturedAccept != "text/html" { + t.Errorf("Accept header should be forwarded, got %q", capturedAccept) + } + if capturedRequestID != "trace-abc" { + t.Errorf("X-Request-Id should be forwarded, got %q", capturedRequestID) + } +} + +// TestCanvasProxy_NoBothCredentialHeaders is the combined regression: a request +// carrying both Authorization AND Cookie must have both stripped. +func TestCanvasProxy_NoBothCredentialHeaders(t *testing.T) { + var gotAuth, gotCookie string + + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + gotCookie = r.Header.Get("Cookie") + w.WriteHeader(http.StatusOK) + })) + defer upstream.Close() + + gin.SetMode(gin.TestMode) + engine := gin.New() + engine.NoRoute(newCanvasProxy(upstream.URL)) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/dashboard", nil) + req.Header.Set("Authorization", "Bearer token123") + req.Header.Set("Cookie", "sid=xyz") + engine.ServeHTTP(w, req) + + if gotAuth != "" || gotCookie != "" { + t.Errorf("both credential headers must be stripped: Authorization=%q Cookie=%q", gotAuth, gotCookie) + } +}