Merge pull request #457 from Molecule-AI/fix/issue-451-strip-auth-header-canvas-proxy

[Backend Engineer] fix(security): strip Authorization + Cookie in canvas reverse proxy

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)

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)
+	}
+}