Merge pull request #35 from Molecule-AI/fix/c18-c20-workspace-auth

fix(security): C18 URL hijacking + C20 unauthenticated workspace deletion

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.

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

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)

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 ==="