From 543b895d3f51022e3988ecf59fd54fe380a883a4 Mon Sep 17 00:00:00 2001 From: DevOps Engineer Date: Wed, 15 Apr 2026 07:27:19 +0000 Subject: [PATCH 1/2] fix(security): revoke workspace tokens on delete (root-cause fix for C1 E2E) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Delete handler marked workspaces 'removed' but never touched workspace_auth_tokens. That left stale live tokens in the table, so HasAnyLiveTokenGlobal stayed true after the last workspace was deleted. AdminAuth then blocked the unauthenticated GET /workspaces in the E2E count-zero assertion with 401, and the previous commit worked around it by commenting out the assertion. This commit fixes the root cause: - workspace.go Delete: batch-revoke auth tokens for all deleted workspace IDs (including descendants) immediately after the canvas_layouts clean-up, using the same pq.Array pattern as the status update. - workspace_test.go TestWorkspaceDelete_CascadeWithChildren: add the expected UPDATE workspace_auth_tokens SET revoked_at sqlmock expectation. - tests/e2e/test_api.sh: restore the count=0 post-delete assertion (now passes because tokens are revoked → fail-open), capture NEW_TOKEN from the re-imported workspace registration for the final cleanup call (SUM_TOKEN is revoked after SUM_ID is deleted). Co-Authored-By: Claude Sonnet 4.6 --- platform/internal/handlers/workspace.go | 11 +++++++++++ platform/internal/handlers/workspace_test.go | 3 +++ tests/e2e/test_api.sh | 19 ++++++++++++------- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/platform/internal/handlers/workspace.go b/platform/internal/handlers/workspace.go index 2d4a016a..b75f0e17 100644 --- a/platform/internal/handlers/workspace.go +++ b/platform/internal/handlers/workspace.go @@ -575,6 +575,17 @@ func (h *WorkspaceHandler) Delete(c *gin.Context) { pq.Array(allIDs)); err != nil { log.Printf("Delete canvas_layouts error for %s: %v", id, err) } + // Revoke all auth tokens for the deleted workspaces. Once the workspace is + // gone its tokens are meaningless; leaving them alive would keep + // HasAnyLiveTokenGlobal = true even after the platform is otherwise empty, + // which prevents AdminAuth from returning to fail-open and breaks the E2E + // test's count-zero assertion (and local re-run cleanup). + if _, err := db.DB.ExecContext(ctx, + `UPDATE workspace_auth_tokens SET revoked_at = now() + WHERE workspace_id = ANY($1::uuid[]) AND revoked_at IS NULL`, + pq.Array(allIDs)); err != nil { + log.Printf("Delete token revocation error for %s: %v", id, err) + } // Now stop containers + remove volumes for all descendants (any depth). // Any concurrent heartbeat / registration / liveness-triggered restart diff --git a/platform/internal/handlers/workspace_test.go b/platform/internal/handlers/workspace_test.go index fb7f5579..7127e208 100644 --- a/platform/internal/handlers/workspace_test.go +++ b/platform/internal/handlers/workspace_test.go @@ -442,6 +442,9 @@ func TestWorkspaceDelete_CascadeWithChildren(t *testing.T) { // Batch canvas_layouts DELETE for the same id set. mock.ExpectExec("DELETE FROM canvas_layouts WHERE workspace_id = ANY"). WillReturnResult(sqlmock.NewResult(2, 2)) + // Token revocation: once a workspace is gone its auth tokens are meaningless. + mock.ExpectExec("UPDATE workspace_auth_tokens SET revoked_at"). + WillReturnResult(sqlmock.NewResult(0, 2)) // Broadcast for child WORKSPACE_REMOVED (fires during the descendant loop). mock.ExpectExec("INSERT INTO structure_events"). WillReturnResult(sqlmock.NewResult(0, 1)) diff --git a/tests/e2e/test_api.sh b/tests/e2e/test_api.sh index c8455d74..cdefa74f 100644 --- a/tests/e2e/test_api.sh +++ b/tests/e2e/test_api.sh @@ -265,11 +265,13 @@ ORIG_TIER=$(echo "$BUNDLE" | python3 -c "import sys,json; print(json.load(sys.st R=$(curl -s -X DELETE "$BASE/workspaces/$SUM_ID" -H "Authorization: Bearer $SUM_TOKEN") check "Delete before re-import" '"status":"removed"' "$R" -# Skipping the "count=0 after delete" assertion: soft-delete leaves the -# workspace_auth_tokens row live, so HasAnyLiveTokenGlobal stays >0 and -# an unauthenticated GET /workspaces returns 401 — exactly #99's C1 contract. -# The bundle round-trip below re-creates a workspace and exercises the -# full import path, so deletion correctness is still covered end-to-end. +# Deletion revokes the workspace's auth tokens (PR #99 C1 fix: workspace.go +# now runs UPDATE workspace_auth_tokens SET revoked_at on delete). With no +# live tokens remaining, HasAnyLiveTokenGlobal = false → AdminAuth fail-open +# → GET /workspaces is reachable without a bearer token again. +R=$(curl -s "$BASE/workspaces") +COUNT=$(echo "$R" | python3 -c "import sys,json; print(len(json.load(sys.stdin)))") +check "All workspaces deleted (count=0)" "0" "$COUNT" # Re-import from the exported bundle R=$(curl -s -X POST "$BASE/bundles/import" -H "Content-Type: application/json" -d "$BUNDLE") @@ -315,13 +317,16 @@ fi R=$(curl -s -X POST "$BASE/registry/register" -H "Content-Type: application/json" \ -d "{\"id\":\"$NEW_ID\",\"url\":\"http://localhost:8002\",\"agent_card\":{\"name\":\"Summarizer\",\"skills\":[{\"id\":\"summarize\",\"name\":\"Summarize\"}]}}") check "Register re-imported workspace" '"status":"registered"' "$R" +# Capture the fresh token issued to the re-imported workspace. SUM_TOKEN was +# revoked when SUM_ID was deleted above — use this one for cleanup instead. +NEW_TOKEN=$(echo "$R" | e2e_extract_token) # Re-export and verify agent_card survives the round-trip 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" -H "Authorization: Bearer $SUM_TOKEN" > /dev/null +# Clean up — use the token just issued to the re-imported workspace +curl -s -X DELETE "$BASE/workspaces/$NEW_ID" -H "Authorization: Bearer $NEW_TOKEN" > /dev/null echo "" echo "=== Results: $PASS passed, $FAIL failed ===" From b6df58886e036137ab7e6964df2ac825d1f67fd9 Mon Sep 17 00:00:00 2001 From: DevOps Engineer Date: Wed, 15 Apr 2026 07:34:40 +0000 Subject: [PATCH 2/2] =?UTF-8?q?ci:=20retry=20=E2=80=94=20trigger=20fresh?= =?UTF-8?q?=20runner=20allocation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit