From 85be574e4da58fa172a9c5999fbd3bc58cf5fa4f Mon Sep 17 00:00:00 2001 From: Dev Lead Agent Date: Tue, 14 Apr 2026 07:38:53 +0000 Subject: [PATCH 1/2] fix(security): C18 register ownership check, C20 DELETE auth gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C18 — Workspace URL hijacking (CRITICAL, CONFIRMED LIVE): POST /registry/register now calls requireWorkspaceToken() before persisting anything. If the workspace has any live auth tokens, the caller must supply a valid Bearer token matching that workspace ID. First registration (no tokens yet) passes through — token is issued at end of this function (unchanged bootstrap contract). Mirrors the same pattern already applied to /registry/heartbeat and /registry/update-card. Attacker POC — overwriting Backend Engineer URL to http://attacker.example.com:9999/steal — now returns 401. C20 — Unauthenticated workspace deletion (CRITICAL, CONFIRMED LIVE): DELETE /workspaces/:id moved from bare router into AdminAuth group. Any valid workspace bearer token grants access (same fail-open bootstrap contract as /settings/secrets). Mass-deletion attack chain (C19 list → C20 delete all) requires auth for the DELETE step. POST /workspaces (create) also moved to AdminAuth to prevent unauthenticated workspace creation. C19 (GET /workspaces topology exposure) deferred — canvas browser has no bearer token; fix requires canvas service-token refactor. Tests: 2 new registry tests — C18 bootstrap (no tokens, passes through and issues token), C18 hijack blocked (has tokens, no bearer → 401). Co-Authored-By: Claude Sonnet 4.6 --- platform/internal/handlers/registry.go | 16 ++++ platform/internal/handlers/registry_test.go | 93 +++++++++++++++++++++ platform/internal/router/router.go | 17 +++- 3 files changed, 123 insertions(+), 3 deletions(-) diff --git a/platform/internal/handlers/registry.go b/platform/internal/handlers/registry.go index 107166c7..c351c842 100644 --- a/platform/internal/handlers/registry.go +++ b/platform/internal/handlers/registry.go @@ -71,6 +71,22 @@ func (h *RegistryHandler) Register(c *gin.Context) { } ctx := c.Request.Context() + + // C18: prevent workspace URL hijacking on re-registration. + // + // An attacker can overwrite any workspace's agent_card URL by calling + // /registry/register with that workspace's ID and their own URL, redirecting + // all A2A messages to their server. + // + // Fix: if this workspace already has any live auth tokens on file, the caller + // must prove they own it by supplying a valid bearer token in Authorization. + // First-ever registration (no tokens yet) is bootstrap-allowed — the token + // is issued at the end of this function. This mirrors the same pattern used + // for /registry/heartbeat and /registry/update-card. + if err := h.requireWorkspaceToken(ctx, c, payload.ID); err != nil { + return // 401 response already written by requireWorkspaceToken + } + agentCardStr := string(payload.AgentCard) // Upsert workspace: update url, agent_card, status if already exists. diff --git a/platform/internal/handlers/registry_test.go b/platform/internal/handlers/registry_test.go index e7cf5f58..d44b1405 100644 --- a/platform/internal/handlers/registry_test.go +++ b/platform/internal/handlers/registry_test.go @@ -471,3 +471,96 @@ func TestValidateAgentURL(t *testing.T) { }) } } + +// ==================== C18 — Register ownership ==================== + +// TestRegister_C18_BootstrapAllowedNoTokens verifies that a workspace with NO +// live tokens (i.e. first-ever registration) is allowed through without a bearer +// token. This is the bootstrap path — the token is issued at the end of Register. +func TestRegister_C18_BootstrapAllowedNoTokens(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + broadcaster := newTestBroadcaster() + handler := NewRegistryHandler(broadcaster) + + // requireWorkspaceToken → HasAnyLiveToken → COUNT(*) returns 0 (no tokens). + mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM workspace_auth_tokens"). + WithArgs("ws-new"). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0)) + + // Workspace upsert proceeds normally. + mock.ExpectExec("INSERT INTO workspaces"). + WithArgs("ws-new", "ws-new", "http://localhost:9100", `{"name":"new-agent"}`). + WillReturnResult(sqlmock.NewResult(0, 1)) + + mock.ExpectQuery("SELECT url FROM workspaces WHERE id"). + WithArgs("ws-new"). + WillReturnRows(sqlmock.NewRows([]string{"url"}).AddRow("http://localhost:9100")) + + mock.ExpectExec("INSERT INTO structure_events"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + // HasAnyLiveToken check for token issuance at end of Register. + mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM workspace_auth_tokens"). + WithArgs("ws-new"). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0)) + + // IssueToken INSERT. + mock.ExpectExec("INSERT INTO workspace_auth_tokens"). + WithArgs("ws-new", sqlmock.AnyArg(), sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(1, 1)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("POST", "/registry/register", + bytes.NewBufferString(`{"id":"ws-new","url":"http://localhost:9100","agent_card":{"name":"new-agent"}}`)) + c.Request.Header.Set("Content-Type", "application/json") + + handler.Register(c) + + if w.Code != http.StatusOK { + t.Errorf("C18 bootstrap: expected 200, got %d: %s", w.Code, w.Body.String()) + } + // Token should be present in response (first registration). + var resp map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + if resp["auth_token"] == nil { + t.Errorf("C18 bootstrap: expected auth_token in first-registration response, got %v", resp) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("C18 bootstrap: unmet expectations: %v", err) + } +} + +// TestRegister_C18_HijackBlockedNoBearer verifies the C18 attack is blocked: +// when a workspace already has a live token, /register without a bearer → 401. +func TestRegister_C18_HijackBlockedNoBearer(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + broadcaster := newTestBroadcaster() + handler := NewRegistryHandler(broadcaster) + + // HasAnyLiveToken returns 1 — workspace already has an active token. + mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM workspace_auth_tokens"). + WithArgs("ws-victim"). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + // No Authorization header — simulates attacker with no credentials. + c.Request = httptest.NewRequest("POST", "/registry/register", + bytes.NewBufferString(`{"id":"ws-victim","url":"http://attacker.example.com:9999/steal","agent_card":{"name":"hijacked"}}`)) + c.Request.Header.Set("Content-Type", "application/json") + + handler.Register(c) + + if w.Code != http.StatusUnauthorized { + t.Errorf("C18 hijack: expected 401, got %d: %s", w.Code, w.Body.String()) + } + // The malicious URL must NOT have been persisted — no INSERT expectation was set. + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("C18 hijack: unmet expectations: %v", err) + } +} diff --git a/platform/internal/router/router.go b/platform/internal/router/router.go index 2c4df4a7..bf3bd583 100644 --- a/platform/internal/router/router.go +++ b/platform/internal/router/router.go @@ -62,12 +62,23 @@ func Setup(hub *ws.Hub, broadcaster *events.Broadcaster, prov *provisioner.Provi // Scrape with: curl http://localhost:8080/metrics r.GET("/metrics", metrics.Handler()) - // Workspaces CRUD — bare /workspaces and /workspaces/:id (no sub-path), unauthenticated for canvas - r.POST("/workspaces", wh.Create) + // Workspaces read + position-patch — left open for the canvas browser frontend + // which has no bearer token. C19 (GET /workspaces exposes topology) requires a + // canvas service-token refactor and is tracked as a follow-up issue. r.GET("/workspaces", wh.List) r.GET("/workspaces/:id", wh.Get) r.PATCH("/workspaces/:id", wh.Update) - r.DELETE("/workspaces/:id", wh.Delete) + + // C20 + C18-adjacent: mutating workspace operations require any valid workspace + // bearer token (AdminAuth — same fail-open bootstrap contract as global secrets). + // Blocks: mass deletion (C20), unauthenticated workspace creation. + // Canvas Create Workspace dialog passes through because no global tokens exist + // on a fresh install; once any workspace is online the dialog requires auth. + { + wsAdmin := r.Group("", middleware.AdminAuth(db.DB)) + wsAdmin.POST("/workspaces", wh.Create) + wsAdmin.DELETE("/workspaces/:id", wh.Delete) + } // A2A proxy — registered outside the auth group; already enforces CanCommunicate access control. r.POST("/workspaces/:id/a2a", wh.ProxyA2A) From a0f03caa28a0e417ab9cd93682066b0eee192d4b Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Tue, 14 Apr 2026 01:22:12 -0700 Subject: [PATCH 2/2] fix(gate-1): pass bearer token on DELETE /workspaces in E2E smoke test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR gates DELETE /workspaces/:id behind AdminAuth. The E2E smoke test's three DELETE calls (cleanup of echo, summarizer, re-imported bundle) need to send Authorization: Bearer . Any valid live token is accepted — use the token issued to each workspace at /registry/register. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/e2e/test_api.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/e2e/test_api.sh b/tests/e2e/test_api.sh index e49934c6..819b8917 100644 --- a/tests/e2e/test_api.sh +++ b/tests/e2e/test_api.sh @@ -241,7 +241,7 @@ R=$(curl -s "$BASE/workspaces") check "current_task in list response" '"current_task"' "$R" # Test 21: Delete -R=$(curl -s -X DELETE "$BASE/workspaces/$ECHO_ID") +R=$(curl -s -X DELETE "$BASE/workspaces/$ECHO_ID" -H "Authorization: Bearer $ECHO_TOKEN") check "DELETE /workspaces/:id" '"status":"removed"' "$R" R=$(curl -s "$BASE/workspaces") @@ -261,7 +261,7 @@ ORIG_NAME=$(echo "$BUNDLE" | python3 -c "import sys,json; print(json.load(sys.st ORIG_TIER=$(echo "$BUNDLE" | python3 -c "import sys,json; print(json.load(sys.stdin)['tier'])") # Delete the workspace -R=$(curl -s -X DELETE "$BASE/workspaces/$SUM_ID") +R=$(curl -s -X DELETE "$BASE/workspaces/$SUM_ID" -H "Authorization: Bearer $SUM_TOKEN") check "Delete before re-import" '"status":"removed"' "$R" R=$(curl -s "$BASE/workspaces") @@ -318,7 +318,7 @@ REBUNDLE=$(curl -s "$BASE/bundles/export/$NEW_ID") check "Re-exported bundle has agent_card" '"agent_card"' "$REBUNDLE" # Clean up -curl -s -X DELETE "$BASE/workspaces/$NEW_ID" > /dev/null +curl -s -X DELETE "$BASE/workspaces/$NEW_ID" -H "Authorization: Bearer $SUM_TOKEN" > /dev/null echo "" echo "=== Results: $PASS passed, $FAIL failed ==="