From 19b4dffd65b164b45fc6556df72ea2d63353dc60 Mon Sep 17 00:00:00 2001 From: "molecule-ai[bot]" <276602405+molecule-ai[bot]@users.noreply.github.com> Date: Fri, 17 Apr 2026 16:15:47 +0000 Subject: [PATCH] fix(security): reject X-Workspace-ID system-caller prefix forgery (#761) Added an early guard in ProxyA2A() that rejects HTTP requests whose X-Workspace-ID header passes isSystemCaller() with 403 Forbidden. Legitimate system callers (webhooks, scheduler, restart_context) call proxyA2ARequest() directly via ProxyA2ARequest() and never send HTTP headers with system-caller prefixes. Co-Authored-By: Claude Sonnet 4.6 --- platform/internal/handlers/a2a_proxy.go | 20 +++++- platform/internal/handlers/a2a_proxy_test.go | 75 ++++++++++++++++---- 2 files changed, 77 insertions(+), 18 deletions(-) diff --git a/platform/internal/handlers/a2a_proxy.go b/platform/internal/handlers/a2a_proxy.go index f7664b22..96233c67 100644 --- a/platform/internal/handlers/a2a_proxy.go +++ b/platform/internal/handlers/a2a_proxy.go @@ -175,17 +175,31 @@ func (h *WorkspaceHandler) ProxyA2A(c *gin.Context) { callerID := c.GetHeader("X-Workspace-ID") + // #761 SECURITY: reject requests where the client-supplied X-Workspace-ID + // contains a system-caller prefix. isSystemCaller() bypasses both token + // validation and CanCommunicate. On the public /a2a endpoint, system-caller + // semantics only apply to callerIDs set by trusted server-side code + // (ProxyA2ARequest), never to HTTP header values. Legitimate system callers + // (webhooks, scheduler, restart_context) call proxyA2ARequest directly and + // never go through this HTTP handler. + if isSystemCaller(callerID) { + log.Printf("security: system-caller prefix forge attempt — remote=%q header=%q", + c.ClientIP(), callerID) + c.JSON(http.StatusForbidden, gin.H{"error": "invalid caller ID"}) + return + } + // Phase 30.5 — validate the caller's auth token when the caller IS // a workspace (not canvas or a system caller). Canvas requests have // no X-Workspace-ID so they bypass this check (the existing // access-control layer already trusts them). System callers - // (webhook:* / system:* / test:*) also bypass — they never hold a - // workspace token. + // (webhook:* / system:* / test:*) only reach proxyA2ARequest via + // the server-side ProxyA2ARequest wrapper, never via this HTTP path. // // The bind is strict: the token must match `callerID`, not // `workspaceID` (the target). A compromised token from workspace A // must never authenticate calls from A pretending to be B. - if callerID != "" && !isSystemCaller(callerID) && callerID != workspaceID { + if callerID != "" && callerID != workspaceID { if err := validateCallerToken(ctx, c, callerID); err != nil { return // response already written with 401 } diff --git a/platform/internal/handlers/a2a_proxy_test.go b/platform/internal/handlers/a2a_proxy_test.go index 7de89c31..99c32d61 100644 --- a/platform/internal/handlers/a2a_proxy_test.go +++ b/platform/internal/handlers/a2a_proxy_test.go @@ -406,21 +406,16 @@ func TestProxyA2A_AllowedSelf_SkipsAccessCheck(t *testing.T) { } } -func TestProxyA2A_SystemCaller_BypassesAccessCheck(t *testing.T) { - mock := setupTestDB(t) - mr := setupTestRedis(t) +// TestProxyA2A_SystemCaller_HTTPHeaderRejected verifies the #761 fix: +// system-caller prefixes in X-Workspace-ID MUST be rejected on the HTTP path. +// Legitimate system callers (webhooks, scheduler, restart_context) call +// proxyA2ARequest directly and never send HTTP headers with these prefixes. +func TestProxyA2A_SystemCaller_HTTPHeaderRejected(t *testing.T) { + setupTestDB(t) + setupTestRedis(t) broadcaster := newTestBroadcaster() handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir()) - agentServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"jsonrpc":"2.0","id":"1","result":{}}`) - })) - defer agentServer.Close() - mr.Set(fmt.Sprintf("ws:%s:url", "ws-target"), agentServer.URL) - - mock.ExpectExec("INSERT INTO activity_logs").WillReturnResult(sqlmock.NewResult(0, 1)) - w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) c.Params = gin.Params{{Key: "id", Value: "ws-target"}} @@ -428,13 +423,63 @@ func TestProxyA2A_SystemCaller_BypassesAccessCheck(t *testing.T) { body := `{"method":"message/send","params":{"message":{"role":"user","parts":[{"text":"hi"}]}}}` c.Request = httptest.NewRequest("POST", "/workspaces/ws-target/a2a", bytes.NewBufferString(body)) c.Request.Header.Set("Content-Type", "application/json") + // Supply a real system-caller prefix — must be blocked at the HTTP layer. c.Request.Header.Set("X-Workspace-ID", "webhook:github") handler.ProxyA2A(c) - time.Sleep(50 * time.Millisecond) - if w.Code != http.StatusOK { - t.Errorf("expected 200 for system caller, got %d: %s", w.Code, w.Body.String()) + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 for system-caller prefix in HTTP header, got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("body not JSON: %v", err) + } + if resp["error"] != "invalid caller ID" { + t.Errorf("expected error 'invalid caller ID', got %v", resp["error"]) + } +} + +// TestA2AProxy_SystemCallerForge_IsRejected verifies that an attacker who +// sets X-Workspace-ID to a system-caller prefix (to bypass token validation +// and CanCommunicate) receives 403 Forbidden — not 200 OK. +// This is the core fix for issue #761. +func TestA2AProxy_SystemCallerForge_IsRejected(t *testing.T) { + forgePrefixes := []string{ + "system:forge", + "system:admin", + "webhook:evil", + "test:attacker", + "channel:hijack", + } + for _, forgedID := range forgePrefixes { + t.Run(forgedID, func(t *testing.T) { + setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-victim"}} + + body := `{"method":"message/send","params":{"message":{"role":"user","parts":[{"text":"exploit"}]}}}` + c.Request = httptest.NewRequest("POST", "/workspaces/ws-victim/a2a", bytes.NewBufferString(body)) + c.Request.Header.Set("Content-Type", "application/json") + c.Request.Header.Set("X-Workspace-ID", forgedID) + + handler.ProxyA2A(c) + + if w.Code != http.StatusForbidden { + t.Errorf("forged caller %q: expected 403, got %d: %s", forgedID, w.Code, w.Body.String()) + } + var resp map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("body not JSON: %v", err) + } + if resp["error"] != "invalid caller ID" { + t.Errorf("forged caller %q: expected error 'invalid caller ID', got %v", forgedID, resp["error"]) + } + }) } }