From 670b785239d0defd46ef905bba8044412f0fe2ab Mon Sep 17 00:00:00 2001 From: Cistern Agent Date: Mon, 13 Apr 2026 22:15:12 -0600 Subject: [PATCH 1/4] sc-d9v2m: fix IDOR and worker callback auth gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Invitation List/Revoke: add claims.TeamID != teamID check before role check, returning 403 for cross-team access - Invitation Create: same team membership check added - ReportProgress/ReportTestResult/ReportWorkerStatus: add auth check (claims nil → 401), DB nil check (→ 503), and ownsExecution verification (→ 404) - Add ownsExecution helper method using SELECT EXISTS - 9 new tests covering both IDOR and auth gap scenarios - Fix existing tests that had mismatched team IDs (claims vs URL) --- internal/handler/executions.go | 74 ++++++++++++++++++++--- internal/handler/executions_test.go | 88 +++++++++++++++++++++++++++- internal/handler/invitations.go | 15 +++++ internal/handler/invitations_test.go | 57 +++++++++++++++--- 4 files changed, 219 insertions(+), 15 deletions(-) diff --git a/internal/handler/executions.go b/internal/handler/executions.go index 9b8ade39..4aa454bf 100644 --- a/internal/handler/executions.go +++ b/internal/handler/executions.go @@ -26,13 +26,13 @@ import ( // ExecutionsHandler handles test execution endpoints. type ExecutionsHandler struct { DB *db.Pool - Hub *ws.Hub // WebSocket hub for real-time broadcasting (optional) - AuditStore *store.AuditStore // optional; nil means no audit logging - K8s *k8s.Client // optional; nil means K8s job launch is disabled - WorkerImage string // default container image for test workers - WorkerToken string // auth token workers use to report back - APIBaseURL string // base URL workers use to call the API - Webhooks *webhook.Notifier // optional; nil means no webhook dispatch + Hub *ws.Hub // WebSocket hub for real-time broadcasting (optional) + AuditStore *store.AuditStore // optional; nil means no audit logging + K8s *k8s.Client // optional; nil means K8s job launch is disabled + WorkerImage string // default container image for test workers + WorkerToken string // auth token workers use to report back + APIBaseURL string // base URL workers use to call the API + Webhooks *webhook.Notifier // optional; nil means no webhook dispatch } // CreateExecutionRequest is the request body for creating a test execution. @@ -446,6 +446,18 @@ func getExecution(ctx context.Context, pool *db.Pool, id, teamID string) (*model return &e, nil } +// ownsExecution checks whether the given execution belongs to the specified team. +func (h *ExecutionsHandler) ownsExecution(ctx context.Context, executionID, teamID string) bool { + var exists bool + err := h.DB.QueryRow(ctx, + `SELECT EXISTS(SELECT 1 FROM test_executions WHERE id = $1 AND team_id = $2)`, + executionID, teamID).Scan(&exists) + if err != nil { + return false + } + return exists +} + // ProgressRequest is the request body for reporting test progress. type ProgressRequest struct { Passed int `json:"passed"` @@ -459,6 +471,12 @@ type ProgressRequest struct { // ReportProgress handles POST /api/v1/executions/{executionID}/progress. // Called by workers to stream live test counters. func (h *ExecutionsHandler) ReportProgress(w http.ResponseWriter, r *http.Request) { + claims := auth.GetClaims(r.Context()) + if claims == nil { + Error(w, http.StatusUnauthorized, "unauthorized") + return + } + executionID := chi.URLParam(r, "executionID") if executionID == "" { Error(w, http.StatusBadRequest, "missing execution ID") @@ -471,6 +489,16 @@ func (h *ExecutionsHandler) ReportProgress(w http.ResponseWriter, r *http.Reques return } + if h.DB == nil { + Error(w, http.StatusServiceUnavailable, "database not configured") + return + } + + if !h.ownsExecution(r.Context(), executionID, claims.TeamID) { + Error(w, http.StatusNotFound, "execution not found") + return + } + // Broadcast progress via WebSocket if h.Hub != nil { h.Hub.BroadcastProgress(executionID, map[string]interface{}{ @@ -503,6 +531,12 @@ type TestResultEvent struct { // ReportTestResult handles POST /api/v1/executions/{executionID}/test-result. // Called by workers to stream individual test results as they complete. func (h *ExecutionsHandler) ReportTestResult(w http.ResponseWriter, r *http.Request) { + claims := auth.GetClaims(r.Context()) + if claims == nil { + Error(w, http.StatusUnauthorized, "unauthorized") + return + } + executionID := chi.URLParam(r, "executionID") if executionID == "" { Error(w, http.StatusBadRequest, "missing execution ID") @@ -515,6 +549,16 @@ func (h *ExecutionsHandler) ReportTestResult(w http.ResponseWriter, r *http.Requ return } + if h.DB == nil { + Error(w, http.StatusServiceUnavailable, "database not configured") + return + } + + if !h.ownsExecution(r.Context(), executionID, claims.TeamID) { + Error(w, http.StatusNotFound, "execution not found") + return + } + // Broadcast individual test result via WebSocket if h.Hub != nil { h.Hub.BroadcastTestResult(executionID, map[string]interface{}{ @@ -545,6 +589,12 @@ type WorkerStatusEvent struct { // ReportWorkerStatus handles POST /api/v1/executions/{executionID}/worker-status. // Called by workers to report their health and progress. func (h *ExecutionsHandler) ReportWorkerStatus(w http.ResponseWriter, r *http.Request) { + claims := auth.GetClaims(r.Context()) + if claims == nil { + Error(w, http.StatusUnauthorized, "unauthorized") + return + } + executionID := chi.URLParam(r, "executionID") if executionID == "" { Error(w, http.StatusBadRequest, "missing execution ID") @@ -557,6 +607,16 @@ func (h *ExecutionsHandler) ReportWorkerStatus(w http.ResponseWriter, r *http.Re return } + if h.DB == nil { + Error(w, http.StatusServiceUnavailable, "database not configured") + return + } + + if !h.ownsExecution(r.Context(), executionID, claims.TeamID) { + Error(w, http.StatusNotFound, "execution not found") + return + } + // Broadcast worker status via WebSocket if h.Hub != nil { h.Hub.BroadcastWorkerStatus(executionID, map[string]interface{}{ diff --git a/internal/handler/executions_test.go b/internal/handler/executions_test.go index 540ede8c..fdb6eb0c 100644 --- a/internal/handler/executions_test.go +++ b/internal/handler/executions_test.go @@ -8,7 +8,6 @@ import ( "testing" ) - func TestListExecutions_Unauthorized(t *testing.T) { h := &ExecutionsHandler{} w := httptest.NewRecorder() @@ -223,6 +222,93 @@ func TestUpdateStatus_NoDB(t *testing.T) { } } +func TestReportProgress_Unauthorized(t *testing.T) { + h := &ExecutionsHandler{} + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/api/v1/executions/abc/progress", strings.NewReader(`{"total":1,"passed":1}`)) + r.Header.Set("Content-Type", "application/json") + r = testWithChiParam(r, "executionID", "abc") + + h.ReportProgress(w, r) + + if w.Code != http.StatusUnauthorized { + t.Errorf("ReportProgress without claims: got %d, want %d", w.Code, http.StatusUnauthorized) + } +} + +func TestReportProgress_NoDB(t *testing.T) { + h := &ExecutionsHandler{DB: nil} + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/api/v1/executions/abc/progress", strings.NewReader(`{"total":1,"passed":1}`)) + r.Header.Set("Content-Type", "application/json") + r = testWithClaimsSimple(r, "user-1", "team-1", "owner") + r = testWithChiParam(r, "executionID", "abc") + + h.ReportProgress(w, r) + + if w.Code != http.StatusServiceUnavailable { + t.Errorf("ReportProgress without DB: got %d, want %d", w.Code, http.StatusServiceUnavailable) + } +} + +func TestReportTestResult_Unauthorized(t *testing.T) { + h := &ExecutionsHandler{} + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/api/v1/executions/abc/test-result", strings.NewReader(`{"name":"test","status":"passed"}`)) + r.Header.Set("Content-Type", "application/json") + r = testWithChiParam(r, "executionID", "abc") + + h.ReportTestResult(w, r) + + if w.Code != http.StatusUnauthorized { + t.Errorf("ReportTestResult without claims: got %d, want %d", w.Code, http.StatusUnauthorized) + } +} + +func TestReportTestResult_NoDB(t *testing.T) { + h := &ExecutionsHandler{DB: nil} + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/api/v1/executions/abc/test-result", strings.NewReader(`{"name":"test","status":"passed"}`)) + r.Header.Set("Content-Type", "application/json") + r = testWithClaimsSimple(r, "user-1", "team-1", "owner") + r = testWithChiParam(r, "executionID", "abc") + + h.ReportTestResult(w, r) + + if w.Code != http.StatusServiceUnavailable { + t.Errorf("ReportTestResult without DB: got %d, want %d", w.Code, http.StatusServiceUnavailable) + } +} + +func TestReportWorkerStatus_Unauthorized(t *testing.T) { + h := &ExecutionsHandler{} + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/api/v1/executions/abc/worker-status", strings.NewReader(`{"worker_id":"w1","status":"running"}`)) + r.Header.Set("Content-Type", "application/json") + r = testWithChiParam(r, "executionID", "abc") + + h.ReportWorkerStatus(w, r) + + if w.Code != http.StatusUnauthorized { + t.Errorf("ReportWorkerStatus without claims: got %d, want %d", w.Code, http.StatusUnauthorized) + } +} + +func TestReportWorkerStatus_NoDB(t *testing.T) { + h := &ExecutionsHandler{DB: nil} + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/api/v1/executions/abc/worker-status", strings.NewReader(`{"worker_id":"w1","status":"running"}`)) + r.Header.Set("Content-Type", "application/json") + r = testWithClaimsSimple(r, "user-1", "team-1", "owner") + r = testWithChiParam(r, "executionID", "abc") + + h.ReportWorkerStatus(w, r) + + if w.Code != http.StatusServiceUnavailable { + t.Errorf("ReportWorkerStatus without DB: got %d, want %d", w.Code, http.StatusServiceUnavailable) + } +} + func TestErrorResponse_Format(t *testing.T) { w := httptest.NewRecorder() Error(w, http.StatusBadRequest, "test error") diff --git a/internal/handler/invitations.go b/internal/handler/invitations.go index eb7c3a09..b5c96877 100644 --- a/internal/handler/invitations.go +++ b/internal/handler/invitations.go @@ -67,6 +67,11 @@ func (h *InvitationsHandler) Create(w http.ResponseWriter, r *http.Request) { return } + if claims.TeamID != teamID { + Error(w, http.StatusForbidden, "not a member of this team") + return + } + if claims.Role != "maintainer" && claims.Role != "owner" { Error(w, http.StatusForbidden, "maintainer or owner role required") return @@ -148,6 +153,11 @@ func (h *InvitationsHandler) List(w http.ResponseWriter, r *http.Request) { return } + if claims.TeamID != teamID { + Error(w, http.StatusForbidden, "not a member of this team") + return + } + if h.Store == nil { JSON(w, http.StatusOK, map[string]interface{}{"invitations": []interface{}{}}) return @@ -299,6 +309,11 @@ func (h *InvitationsHandler) Revoke(w http.ResponseWriter, r *http.Request) { return } + if claims.TeamID != teamID { + Error(w, http.StatusForbidden, "not a member of this team") + return + } + if claims.Role != "maintainer" && claims.Role != "owner" { Error(w, http.StatusForbidden, "maintainer or owner role required") return diff --git a/internal/handler/invitations_test.go b/internal/handler/invitations_test.go index 7b9610b8..ca68d04a 100644 --- a/internal/handler/invitations_test.go +++ b/internal/handler/invitations_test.go @@ -49,10 +49,10 @@ func (m *mockInvitationStore) AcceptInvitation(_ context.Context, _, _, _, _, _, // mockMailer is a test double for mailer.Mailer. type mockMailer struct { - called bool - sentTo string + called bool + sentTo string sentURL string - err error + err error } func (m *mockMailer) SendInvitation(_ context.Context, to, url string) error { @@ -248,9 +248,9 @@ func TestRevokeInvitation_Unauthorized(t *testing.T) { func TestRevokeInvitation_ReadonlyForbidden(t *testing.T) { h := &InvitationsHandler{} w := httptest.NewRecorder() - r := httptest.NewRequest("DELETE", "/api/v1/teams/t1/invitations/inv1", nil) + r := httptest.NewRequest("DELETE", "/api/v1/teams/team-1/invitations/inv1", nil) r = testWithClaimsAndParams(r, invClaims("user-1", "team-1", "readonly"), map[string]string{ - "teamID": "t1", + "teamID": "team-1", "invitationID": "inv1", }) @@ -264,9 +264,9 @@ func TestRevokeInvitation_ReadonlyForbidden(t *testing.T) { func TestRevokeInvitation_NoDB(t *testing.T) { h := &InvitationsHandler{Store: nil} w := httptest.NewRecorder() - r := httptest.NewRequest("DELETE", "/api/v1/teams/t1/invitations/inv1", nil) + r := httptest.NewRequest("DELETE", "/api/v1/teams/team-1/invitations/inv1", nil) r = testWithClaimsAndParams(r, invClaims("user-1", "team-1", "owner"), map[string]string{ - "teamID": "t1", + "teamID": "team-1", "invitationID": "inv1", }) @@ -524,6 +524,49 @@ func TestCreateInvitation_NilAuditStore_NoPanic(t *testing.T) { } } +func TestListInvitations_WrongTeam_Forbidden(t *testing.T) { + h := &InvitationsHandler{Store: &mockInvitationStore{}} + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/api/v1/teams/team-other/invitations", nil) + r = testWithClaimsAndParam(r, invClaims("user-1", "team-1", "owner"), "teamID", "team-other") + + h.List(w, r) + + if w.Code != http.StatusForbidden { + t.Errorf("List with wrong team: got %d, want %d", w.Code, http.StatusForbidden) + } +} + +func TestCreateInvitation_WrongTeam_Forbidden(t *testing.T) { + h := &InvitationsHandler{Store: &mockInvitationStore{}} + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/api/v1/teams/team-other/invitations", strings.NewReader(`{"email":"x@example.com","role":"readonly"}`)) + r.Header.Set("Content-Type", "application/json") + r = testWithClaimsAndParam(r, invClaims("user-1", "team-1", "owner"), "teamID", "team-other") + + h.Create(w, r) + + if w.Code != http.StatusForbidden { + t.Errorf("Create with wrong team: got %d, want %d", w.Code, http.StatusForbidden) + } +} + +func TestRevokeInvitation_WrongTeam_Forbidden(t *testing.T) { + h := &InvitationsHandler{Store: &mockInvitationStore{}} + w := httptest.NewRecorder() + r := httptest.NewRequest("DELETE", "/api/v1/teams/team-other/invitations/inv1", nil) + r = testWithClaimsAndParams(r, invClaims("user-1", "team-1", "owner"), map[string]string{ + "teamID": "team-other", + "invitationID": "inv1", + }) + + h.Revoke(w, r) + + if w.Code != http.StatusForbidden { + t.Errorf("Revoke with wrong team: got %d, want %d", w.Code, http.StatusForbidden) + } +} + func TestRevokeInvitation_LogsAuditEvent(t *testing.T) { ms := &mockInvitationStore{} al := &capAuditLogger{} From 127501265f4eaa2bc2f5984de31b0200edb97b2f Mon Sep 17 00:00:00 2001 From: Cistern Agent Date: Mon, 13 Apr 2026 22:17:31 -0600 Subject: [PATCH 2/4] sc-d9v2m: simplify: extract requireWorkerCallback to deduplicate ReportProgress/ReportTestResult/ReportWorkerStatus preamble --- AGENTS.md | 311 ++++++++++++++++++++++++++------- internal/handler/executions.go | 89 ++++------ 2 files changed, 284 insertions(+), 116 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 60862174..72a43723 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,96 +1,289 @@ -# Agent Instructions +You are a Cataracta operating within the Cistern agentic pipeline. -## Project Mission +Cistern is an automated software delivery system. The Castellarius (a pure state +machine) watches the cistern and routes droplets (units of work) into named +aqueducts. You are one cataractae — one gate — in that aqueduct. You receive a +droplet, complete your assigned role, and signal your outcome so the droplet +continues flowing. -ScaledTest is an end-to-end testing platform for running tests at massive scale with unprecedented reporting capabilities. It serves both humans (visual dashboards, analytics) and machines (CI integration, quality gates, webhooks). All test reporting uses the CTRF (Common Test Results Format) standard. +THE CASTELLARIUS WATCHES THE CISTERN, ROUTES DROPLETS INTO AVAILABLE AQUEDUCTS. +EACH AQUEDUCT FLOWS THE DROPLET THROUGH ITS CATARACTAE. ---- +## Your contract — non-negotiable + +1. Read CONTEXT.md before doing anything else. It contains your droplet ID, + requirements, and all revision notes from prior cycles. +2. Adopt the persona described in your role instructions below. +3. Complete your work according to that persona. +4. Every 60 seconds while working, call: ct droplet heartbeat + This signals the scheduler that you are alive and making progress. Without + heartbeats, the stall detector may flag your session as stuck. +5. Signal your outcome before exiting. You MUST call one of: + ct droplet pass --notes "..." + ct droplet recirculate --notes "..." + ct droplet pool --notes "..." + A cataractae that exits without signaling leaves the droplet stranded. + +Your role persona and skill instructions follow. + +## System safety invariants — never break these + +The Castellarius is a state machine. Its correctness depends on these invariants holding. If your work could affect any of them, you must verify they still hold before signaling pass. + +**1. Signaling is the only valid way to advance state.** +Never manipulate droplet state by any means other than ct droplet pass/recirculate/pool. +Never exit without signaling — a stranded droplet burns resources indefinitely. + +**2. Session spawning must expose the agent process directly to tmux.** +Do not wrap the agent command in a shell (bash -c, sh -c, pipes, tee) unless you have explicitly verified that pane_current_command and /proc//cmdline still correctly identify the agent. Wrappers that change what the process monitor sees will cause every healthy session to be misclassified as exited without outcome and respawned in a loop. + +**3. CONTEXT.md is pipeline state — never commit it.** +CONTEXT.md is injected at dispatch time and listed in .gitignore. If you see it in a git add or git commit, stop. Committing it causes merge conflicts across concurrent deliveries and corrupts origin/main. + +**4. The circuit breaker pools after 5 dead sessions in 15 minutes.** +If your droplet keeps dying without signaling an outcome, the scheduler will pool it automatically. This stops token burn — a human needs to investigate before the droplet is re-opened. + +**5. Do not call git add -f or git add --force on any ignored file.** +The .gitignore exists for a reason. Overriding it for pipeline state files (CONTEXT.md, .current-stage, session logs) corrupts the state machine. + +## Your Role + + + +# Role: Code Simplifier + +You are a code simplification specialist in a Cistern Aqueduct. You refine code on this +branch for clarity and maintainability while preserving exact behaviour. + +## Context + +You have **full codebase access** — you can read the full repository to understand +patterns and conventions. However, you are **diff-scoped by design**: you may only +modify files that were changed on this branch. This restriction exists to prevent +whole-codebase refactoring and to keep simplification focused on the work under review. + +## Step 1 — Identify changed code +Run: git log $(git merge-base HEAD origin/main)..HEAD --oneline +If empty: signal pass immediately — nothing to simplify. + +Run: git diff $(git merge-base HEAD origin/main)..HEAD --name-only +These are the only files you may touch. -This project uses **Cistern** (`ct`) for work item tracking. Your work item is described in `CONTEXT.md`. +Run: git diff $(git merge-base HEAD origin/main)..HEAD +Read the actual changes to understand what was implemented. +(See cistern-git skill for git conventions.) -## Quick Reference +## Step 2 — Look for simplification opportunities +For each changed file, check for: +- Unnecessary complexity and nesting +- Redundant code, dead variables, and unused imports +- Unclear names that obscure intent +- Comments that describe obvious code +- Logic that can be consolidated without sacrificing clarity +- Repeated patterns that could be a shared helper +Do NOT touch: +- Code that was not changed on this branch +- Tests (unless they are also unnecessarily complex) +- Anything that changes what the code does + +## Step 3 — Apply changes (or skip) +If no simplifications are warranted: + ct droplet pass --notes "No simplifications required — code is already clear and idiomatic." +and stop. + +Rules when making changes: +- NEVER change behaviour — only how it is expressed +- Prefer explicit over compact +- Run go test ./... -count=1 after each file — revert immediately if anything fails + +## Step 4 — Commit +Use cistern-git skill conventions (exclude CONTEXT.md, verify HEAD advances). ```bash -ct droplet pass --notes "..." # Signal work complete -ct droplet block --notes "..." # Signal blocked (external dependency) -ct droplet recirculate --notes "..." # Send back for rework -ct droplet note "..." # Add a progress note +git add -A -- ':!CONTEXT.md' +git commit -m ": simplify: " ``` -## Architecture +## Step 5 — Signal +ct droplet pass --notes "Simplified: . Tests: all N packages pass." +ct droplet recirculate --notes "Blocked: " + +## Skills + +## Skill: cistern-droplet-state + +# Cistern Droplet State + +Manage droplet state in the Cistern agentic pipeline using the `ct` CLI. + +## When to use this skill + +Use at the end of every cataractae session to signal your outcome. +You MUST signal before exiting. A cataractae that exits without signaling leaves +the droplet stranded in the cistern. -ScaledTest is a v2 Go backend + React SPA: +## Your Droplet ID + +Your droplet ID is in CONTEXT.md under `## Item: `. Use it in every command. + +## Signaling Commands + +### Pass — work complete, ready to flow forward +```bash +ct droplet pass --notes "Summary of what was done and verified." +``` + +### Recirculate — needs revision, send back upstream +```bash +ct droplet recirculate --notes "Specific issues: 1. 2. " +``` -- **Go backend**: chi router, pgxpool, JWT auth, RBAC, CTRF ingestion -- **React frontend**: React 19, TanStack Router/Query, Zustand, Recharts — served as embedded SPA via `go:embed` -- **K8s Job management** for distributed test execution -- **Quality gate rule DSL**, WebSocket hub for real-time updates +### Recirculate to a specific cataractae +```bash +ct droplet recirculate --to implement --notes "Reason for routing to implement." +``` -### Key Directories +### Pool — cannot proceed +```bash +ct droplet pool --notes "Cannot proceed because: " +``` +### Add a note (without signaling) +```bash +ct droplet note "Intermediate finding or progress update." ``` -cmd/ # Go binaries (server, worker, ci-notify) -internal/ # Go backend packages (auth, handler, store, ctrf, quality, ws, …) -frontend/ # React 19 SPA (Vite + TypeScript) -sdk/ # TypeScript SDK -migrations/ # Database migrations -e2e/ # End-to-end tests + +## Rules + +1. Always include `--notes` when signaling — describe what you did or found +2. **Implementer**: signal `pass` when your work is committed and tests pass. Never signal `recirculate` — that is the reviewer's signal. Open issues are for the reviewer to verify and resolve; you cannot resolve your own issues. +3. **Reviewer/QA/Security**: signal `recirculate` when you have findings that require implementation changes. Signal `pass` when all prior issues are resolved and no new issues found. +4. Implementer: never push to origin — local commits only +5. Be specific in notes — "Fixed 3 issues in client.go" not "fixed it" +6. If CONTEXT.md has revision notes from prior cycles, address every single one + +## Skill: cistern-git + +--- +name: cistern-git +description: Git conventions for Cistern aqueduct cataractae. Use for all git operations in sandboxes: staging, committing, diffing, branching, and pushing. Covers rules specific to per-droplet worktrees, CONTEXT.md exclusion, merge-base diff, and no-stash policy. +--- + +# Cistern Git Conventions + +## Worktree model + +Each droplet has an isolated worktree at `~/.cistern/sandboxes///`. +The Castellarius creates and cleans up worktrees. Agents never run `git worktree add/remove`. + +## Staging — always exclude CONTEXT.md + +CONTEXT.md is written by the Castellarius on every dispatch. Never commit it. + +```bash +git add -A -- ':!CONTEXT.md' +git status --short # verify no CONTEXT.md, no binaries ``` -### Test Commands +## Committing — verify HEAD advances ```bash -make test # All Go tests (with race detector) -make test-short # Fast Go tests (no race) -make test-integration # Integration tests (requires TEST_DATABASE_URL) -cd frontend && npm test # Frontend tests (Vitest) +BEFORE=$(git rev-parse HEAD) +git add -A -- ':!CONTEXT.md' +git commit -m ": " +AFTER=$(git rev-parse HEAD) +if [ "$BEFORE" = "$AFTER" ]; then + echo "ERROR: nothing staged. Commit failed." + ct droplet recirculate --notes "Commit failed: HEAD did not advance." + exit 1 +fi ``` -## Work Submission +## Diffing — always use merge-base -Create PRs using the standard GitHub CLI: +```bash +# Correct — shows only this branch's own changes, regardless of rebase state +git diff $(git merge-base HEAD origin/main)..HEAD +git diff $(git merge-base HEAD origin/main)..HEAD --name-only +``` + +Two-dot (`git diff origin/main..HEAD`) is wrong for unrebased branches: it includes all commits since the branch diverged, meaning other PRs that merged to main after branching appear in the diff. Merge-base is always correct. +To list commits on the branch: ```bash -gh pr create --title "sc-XXXXX: short description" --body "..." +git log $(git merge-base HEAD origin/main)..HEAD --oneline ``` -## Non-Interactive Shell Commands +## No stash -**ALWAYS use non-interactive flags** with file operations to avoid hanging on confirmation prompts. +Per-droplet worktrees start clean. **Never run `git stash`** — it silently hides uncommitted work and has caused phantom empty deliveries. If the worktree is dirty before your work, the Castellarius will detect it and recirculate. -Shell commands like `cp`, `mv`, and `rm` may be aliased to include `-i` (interactive) mode on some systems, causing the agent to hang indefinitely waiting for y/n input. +## Pushing -**Use these forms instead:** +```bash +git push origin +# If rebased: +git push --force-with-lease origin +``` +## Branch name + +Your branch is `feat/`. It is created by the Castellarius. Check with: ```bash -# Force overwrite without prompting -cp -f source dest # NOT: cp source dest -mv -f source dest # NOT: mv source dest -rm -f file # NOT: rm file - -# For recursive operations -rm -rf directory # NOT: rm -r directory -cp -rf source dest # NOT: cp -r source dest +git branch --show-current ``` -**Other commands that may prompt:** +## Skill: code-simplifier + +--- +name: code-simplifier +description: Simplifies and refines code for clarity, consistency, and maintainability while preserving all functionality. Focuses on recently modified code unless instructed otherwise. +model: opus +--- + +You are an expert code simplification specialist focused on enhancing code clarity, consistency, and maintainability while preserving exact functionality. Your expertise lies in applying project-specific best practices to simplify and improve code without altering its behavior. You prioritize readable, explicit code over overly compact solutions. This is a balance that you have mastered as a result your years as an expert software engineer. + +You will analyze recently modified code and apply refinements that: + +1. **Preserve Functionality**: Never change what the code does - only how it does it. All original features, outputs, and behaviors must remain intact. + +2. **Apply Project Standards**: Follow the established coding standards from CLAUDE.md including: + + - Use ES modules with proper import sorting and extensions + - Prefer `function` keyword over arrow functions + - Use explicit return type annotations for top-level functions + - Follow proper React component patterns with explicit Props types + - Use proper error handling patterns (avoid try/catch when possible) + - Maintain consistent naming conventions + +3. **Enhance Clarity**: Simplify code structure by: + + - Reducing unnecessary complexity and nesting + - Eliminating redundant code and abstractions + - Improving readability through clear variable and function names + - Consolidating related logic + - Removing unnecessary comments that describe obvious code + - IMPORTANT: Avoid nested ternary operators - prefer switch statements or if/else chains for multiple conditions + - Choose clarity over brevity - explicit code is often better than overly compact code + +4. **Maintain Balance**: Avoid over-simplification that could: -- `scp` - use `-o BatchMode=yes` for non-interactive -- `ssh` - use `-o BatchMode=yes` to fail instead of prompting -- `apt-get` - use `-y` flag -- `brew` - use `HOMEBREW_NO_AUTO_UPDATE=1` env var + - Reduce code clarity or maintainability + - Create overly clever solutions that are hard to understand + - Combine too many concerns into single functions or components + - Remove helpful abstractions that improve code organization + - Prioritize "fewer lines" over readability (e.g., nested ternaries, dense one-liners) + - Make the code harder to debug or extend -## Cistern Workflow for AI Agents +5. **Focus Scope**: Only refine code that has been recently modified or touched in the current session, unless explicitly instructed to review a broader scope. -1. **Read CONTEXT.md** — your work item, requirements, and any revision notes are there -2. **Implement** — write tests first (TDD), then implementation -3. **Run tests** — `make test` for Go; `cd frontend && npm test` for frontend -4. **Commit** — `git commit -m ": "` (do NOT push) -5. **Signal outcome** — use the appropriate `ct droplet` command from Quick Reference above +Your refinement process: -### Signaling Rules +1. Identify the recently modified code sections +2. Analyze for opportunities to improve elegance and consistency +3. Apply project-specific best practices and coding standards +4. Ensure all functionality remains unchanged +5. Verify the refined code is simpler and more maintainable +6. Document only significant changes that affect understanding -- Always include `--notes` describing what you did or found -- Never signal `pass` if required issues remain unresolved -- Use `block` only for genuine external blockers, not for ordinary revision work -- Local commits only — do NOT push to origin +You operate autonomously and proactively, refining code immediately after it's written or modified without requiring explicit requests. Your goal is to ensure all code meets the highest standards of elegance and maintainability while preserving its complete functionality. diff --git a/internal/handler/executions.go b/internal/handler/executions.go index 4aa454bf..93b02824 100644 --- a/internal/handler/executions.go +++ b/internal/handler/executions.go @@ -458,6 +458,32 @@ func (h *ExecutionsHandler) ownsExecution(ctx context.Context, executionID, team return exists } +func (h *ExecutionsHandler) requireWorkerCallback(w http.ResponseWriter, r *http.Request) (*auth.Claims, string, bool) { + claims := auth.GetClaims(r.Context()) + if claims == nil { + Error(w, http.StatusUnauthorized, "unauthorized") + return nil, "", false + } + + executionID := chi.URLParam(r, "executionID") + if executionID == "" { + Error(w, http.StatusBadRequest, "missing execution ID") + return nil, "", false + } + + if h.DB == nil { + Error(w, http.StatusServiceUnavailable, "database not configured") + return nil, "", false + } + + if !h.ownsExecution(r.Context(), executionID, claims.TeamID) { + Error(w, http.StatusNotFound, "execution not found") + return nil, "", false + } + + return claims, executionID, true +} + // ProgressRequest is the request body for reporting test progress. type ProgressRequest struct { Passed int `json:"passed"` @@ -471,15 +497,8 @@ type ProgressRequest struct { // ReportProgress handles POST /api/v1/executions/{executionID}/progress. // Called by workers to stream live test counters. func (h *ExecutionsHandler) ReportProgress(w http.ResponseWriter, r *http.Request) { - claims := auth.GetClaims(r.Context()) - if claims == nil { - Error(w, http.StatusUnauthorized, "unauthorized") - return - } - - executionID := chi.URLParam(r, "executionID") - if executionID == "" { - Error(w, http.StatusBadRequest, "missing execution ID") + _, executionID, ok := h.requireWorkerCallback(w, r) + if !ok { return } @@ -489,16 +508,6 @@ func (h *ExecutionsHandler) ReportProgress(w http.ResponseWriter, r *http.Reques return } - if h.DB == nil { - Error(w, http.StatusServiceUnavailable, "database not configured") - return - } - - if !h.ownsExecution(r.Context(), executionID, claims.TeamID) { - Error(w, http.StatusNotFound, "execution not found") - return - } - // Broadcast progress via WebSocket if h.Hub != nil { h.Hub.BroadcastProgress(executionID, map[string]interface{}{ @@ -531,15 +540,8 @@ type TestResultEvent struct { // ReportTestResult handles POST /api/v1/executions/{executionID}/test-result. // Called by workers to stream individual test results as they complete. func (h *ExecutionsHandler) ReportTestResult(w http.ResponseWriter, r *http.Request) { - claims := auth.GetClaims(r.Context()) - if claims == nil { - Error(w, http.StatusUnauthorized, "unauthorized") - return - } - - executionID := chi.URLParam(r, "executionID") - if executionID == "" { - Error(w, http.StatusBadRequest, "missing execution ID") + _, executionID, ok := h.requireWorkerCallback(w, r) + if !ok { return } @@ -549,16 +551,6 @@ func (h *ExecutionsHandler) ReportTestResult(w http.ResponseWriter, r *http.Requ return } - if h.DB == nil { - Error(w, http.StatusServiceUnavailable, "database not configured") - return - } - - if !h.ownsExecution(r.Context(), executionID, claims.TeamID) { - Error(w, http.StatusNotFound, "execution not found") - return - } - // Broadcast individual test result via WebSocket if h.Hub != nil { h.Hub.BroadcastTestResult(executionID, map[string]interface{}{ @@ -589,15 +581,8 @@ type WorkerStatusEvent struct { // ReportWorkerStatus handles POST /api/v1/executions/{executionID}/worker-status. // Called by workers to report their health and progress. func (h *ExecutionsHandler) ReportWorkerStatus(w http.ResponseWriter, r *http.Request) { - claims := auth.GetClaims(r.Context()) - if claims == nil { - Error(w, http.StatusUnauthorized, "unauthorized") - return - } - - executionID := chi.URLParam(r, "executionID") - if executionID == "" { - Error(w, http.StatusBadRequest, "missing execution ID") + _, executionID, ok := h.requireWorkerCallback(w, r) + if !ok { return } @@ -607,16 +592,6 @@ func (h *ExecutionsHandler) ReportWorkerStatus(w http.ResponseWriter, r *http.Re return } - if h.DB == nil { - Error(w, http.StatusServiceUnavailable, "database not configured") - return - } - - if !h.ownsExecution(r.Context(), executionID, claims.TeamID) { - Error(w, http.StatusNotFound, "execution not found") - return - } - // Broadcast worker status via WebSocket if h.Hub != nil { h.Hub.BroadcastWorkerStatus(executionID, map[string]interface{}{ From bc997843f5501b90fb89d110f4b03150159546f0 Mon Sep 17 00:00:00 2001 From: Cistern Agent Date: Mon, 13 Apr 2026 22:22:37 -0600 Subject: [PATCH 3/4] sc-d9v2m: fix ownsExecution error handling and add cross-team auth tests --- AGENTS.md | 311 ++++++---------------------- internal/handler/executions.go | 23 +- internal/handler/executions_test.go | 84 ++++++++ 3 files changed, 162 insertions(+), 256 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 72a43723..60862174 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,289 +1,96 @@ -You are a Cataracta operating within the Cistern agentic pipeline. +# Agent Instructions -Cistern is an automated software delivery system. The Castellarius (a pure state -machine) watches the cistern and routes droplets (units of work) into named -aqueducts. You are one cataractae — one gate — in that aqueduct. You receive a -droplet, complete your assigned role, and signal your outcome so the droplet -continues flowing. +## Project Mission -THE CASTELLARIUS WATCHES THE CISTERN, ROUTES DROPLETS INTO AVAILABLE AQUEDUCTS. -EACH AQUEDUCT FLOWS THE DROPLET THROUGH ITS CATARACTAE. +ScaledTest is an end-to-end testing platform for running tests at massive scale with unprecedented reporting capabilities. It serves both humans (visual dashboards, analytics) and machines (CI integration, quality gates, webhooks). All test reporting uses the CTRF (Common Test Results Format) standard. -## Your contract — non-negotiable - -1. Read CONTEXT.md before doing anything else. It contains your droplet ID, - requirements, and all revision notes from prior cycles. -2. Adopt the persona described in your role instructions below. -3. Complete your work according to that persona. -4. Every 60 seconds while working, call: ct droplet heartbeat - This signals the scheduler that you are alive and making progress. Without - heartbeats, the stall detector may flag your session as stuck. -5. Signal your outcome before exiting. You MUST call one of: - ct droplet pass --notes "..." - ct droplet recirculate --notes "..." - ct droplet pool --notes "..." - A cataractae that exits without signaling leaves the droplet stranded. - -Your role persona and skill instructions follow. - -## System safety invariants — never break these - -The Castellarius is a state machine. Its correctness depends on these invariants holding. If your work could affect any of them, you must verify they still hold before signaling pass. - -**1. Signaling is the only valid way to advance state.** -Never manipulate droplet state by any means other than ct droplet pass/recirculate/pool. -Never exit without signaling — a stranded droplet burns resources indefinitely. - -**2. Session spawning must expose the agent process directly to tmux.** -Do not wrap the agent command in a shell (bash -c, sh -c, pipes, tee) unless you have explicitly verified that pane_current_command and /proc//cmdline still correctly identify the agent. Wrappers that change what the process monitor sees will cause every healthy session to be misclassified as exited without outcome and respawned in a loop. - -**3. CONTEXT.md is pipeline state — never commit it.** -CONTEXT.md is injected at dispatch time and listed in .gitignore. If you see it in a git add or git commit, stop. Committing it causes merge conflicts across concurrent deliveries and corrupts origin/main. - -**4. The circuit breaker pools after 5 dead sessions in 15 minutes.** -If your droplet keeps dying without signaling an outcome, the scheduler will pool it automatically. This stops token burn — a human needs to investigate before the droplet is re-opened. - -**5. Do not call git add -f or git add --force on any ignored file.** -The .gitignore exists for a reason. Overriding it for pipeline state files (CONTEXT.md, .current-stage, session logs) corrupts the state machine. - -## Your Role - - - -# Role: Code Simplifier - -You are a code simplification specialist in a Cistern Aqueduct. You refine code on this -branch for clarity and maintainability while preserving exact behaviour. - -## Context - -You have **full codebase access** — you can read the full repository to understand -patterns and conventions. However, you are **diff-scoped by design**: you may only -modify files that were changed on this branch. This restriction exists to prevent -whole-codebase refactoring and to keep simplification focused on the work under review. - -## Step 1 — Identify changed code -Run: git log $(git merge-base HEAD origin/main)..HEAD --oneline -If empty: signal pass immediately — nothing to simplify. - -Run: git diff $(git merge-base HEAD origin/main)..HEAD --name-only -These are the only files you may touch. - -Run: git diff $(git merge-base HEAD origin/main)..HEAD -Read the actual changes to understand what was implemented. -(See cistern-git skill for git conventions.) - -## Step 2 — Look for simplification opportunities -For each changed file, check for: -- Unnecessary complexity and nesting -- Redundant code, dead variables, and unused imports -- Unclear names that obscure intent -- Comments that describe obvious code -- Logic that can be consolidated without sacrificing clarity -- Repeated patterns that could be a shared helper - -Do NOT touch: -- Code that was not changed on this branch -- Tests (unless they are also unnecessarily complex) -- Anything that changes what the code does +--- -## Step 3 — Apply changes (or skip) -If no simplifications are warranted: - ct droplet pass --notes "No simplifications required — code is already clear and idiomatic." -and stop. +This project uses **Cistern** (`ct`) for work item tracking. Your work item is described in `CONTEXT.md`. -Rules when making changes: -- NEVER change behaviour — only how it is expressed -- Prefer explicit over compact -- Run go test ./... -count=1 after each file — revert immediately if anything fails +## Quick Reference -## Step 4 — Commit -Use cistern-git skill conventions (exclude CONTEXT.md, verify HEAD advances). ```bash -git add -A -- ':!CONTEXT.md' -git commit -m ": simplify: " +ct droplet pass --notes "..." # Signal work complete +ct droplet block --notes "..." # Signal blocked (external dependency) +ct droplet recirculate --notes "..." # Send back for rework +ct droplet note "..." # Add a progress note ``` -## Step 5 — Signal -ct droplet pass --notes "Simplified: . Tests: all N packages pass." -ct droplet recirculate --notes "Blocked: " - -## Skills - -## Skill: cistern-droplet-state - -# Cistern Droplet State - -Manage droplet state in the Cistern agentic pipeline using the `ct` CLI. - -## When to use this skill - -Use at the end of every cataractae session to signal your outcome. -You MUST signal before exiting. A cataractae that exits without signaling leaves -the droplet stranded in the cistern. +## Architecture -## Your Droplet ID - -Your droplet ID is in CONTEXT.md under `## Item: `. Use it in every command. - -## Signaling Commands - -### Pass — work complete, ready to flow forward -```bash -ct droplet pass --notes "Summary of what was done and verified." -``` - -### Recirculate — needs revision, send back upstream -```bash -ct droplet recirculate --notes "Specific issues: 1. 2. " -``` +ScaledTest is a v2 Go backend + React SPA: -### Recirculate to a specific cataractae -```bash -ct droplet recirculate --to implement --notes "Reason for routing to implement." -``` +- **Go backend**: chi router, pgxpool, JWT auth, RBAC, CTRF ingestion +- **React frontend**: React 19, TanStack Router/Query, Zustand, Recharts — served as embedded SPA via `go:embed` +- **K8s Job management** for distributed test execution +- **Quality gate rule DSL**, WebSocket hub for real-time updates -### Pool — cannot proceed -```bash -ct droplet pool --notes "Cannot proceed because: " -``` +### Key Directories -### Add a note (without signaling) -```bash -ct droplet note "Intermediate finding or progress update." ``` - -## Rules - -1. Always include `--notes` when signaling — describe what you did or found -2. **Implementer**: signal `pass` when your work is committed and tests pass. Never signal `recirculate` — that is the reviewer's signal. Open issues are for the reviewer to verify and resolve; you cannot resolve your own issues. -3. **Reviewer/QA/Security**: signal `recirculate` when you have findings that require implementation changes. Signal `pass` when all prior issues are resolved and no new issues found. -4. Implementer: never push to origin — local commits only -5. Be specific in notes — "Fixed 3 issues in client.go" not "fixed it" -6. If CONTEXT.md has revision notes from prior cycles, address every single one - -## Skill: cistern-git - ---- -name: cistern-git -description: Git conventions for Cistern aqueduct cataractae. Use for all git operations in sandboxes: staging, committing, diffing, branching, and pushing. Covers rules specific to per-droplet worktrees, CONTEXT.md exclusion, merge-base diff, and no-stash policy. ---- - -# Cistern Git Conventions - -## Worktree model - -Each droplet has an isolated worktree at `~/.cistern/sandboxes///`. -The Castellarius creates and cleans up worktrees. Agents never run `git worktree add/remove`. - -## Staging — always exclude CONTEXT.md - -CONTEXT.md is written by the Castellarius on every dispatch. Never commit it. - -```bash -git add -A -- ':!CONTEXT.md' -git status --short # verify no CONTEXT.md, no binaries +cmd/ # Go binaries (server, worker, ci-notify) +internal/ # Go backend packages (auth, handler, store, ctrf, quality, ws, …) +frontend/ # React 19 SPA (Vite + TypeScript) +sdk/ # TypeScript SDK +migrations/ # Database migrations +e2e/ # End-to-end tests ``` -## Committing — verify HEAD advances +### Test Commands ```bash -BEFORE=$(git rev-parse HEAD) -git add -A -- ':!CONTEXT.md' -git commit -m ": " -AFTER=$(git rev-parse HEAD) -if [ "$BEFORE" = "$AFTER" ]; then - echo "ERROR: nothing staged. Commit failed." - ct droplet recirculate --notes "Commit failed: HEAD did not advance." - exit 1 -fi +make test # All Go tests (with race detector) +make test-short # Fast Go tests (no race) +make test-integration # Integration tests (requires TEST_DATABASE_URL) +cd frontend && npm test # Frontend tests (Vitest) ``` -## Diffing — always use merge-base +## Work Submission -```bash -# Correct — shows only this branch's own changes, regardless of rebase state -git diff $(git merge-base HEAD origin/main)..HEAD -git diff $(git merge-base HEAD origin/main)..HEAD --name-only -``` - -Two-dot (`git diff origin/main..HEAD`) is wrong for unrebased branches: it includes all commits since the branch diverged, meaning other PRs that merged to main after branching appear in the diff. Merge-base is always correct. +Create PRs using the standard GitHub CLI: -To list commits on the branch: ```bash -git log $(git merge-base HEAD origin/main)..HEAD --oneline +gh pr create --title "sc-XXXXX: short description" --body "..." ``` -## No stash +## Non-Interactive Shell Commands -Per-droplet worktrees start clean. **Never run `git stash`** — it silently hides uncommitted work and has caused phantom empty deliveries. If the worktree is dirty before your work, the Castellarius will detect it and recirculate. +**ALWAYS use non-interactive flags** with file operations to avoid hanging on confirmation prompts. -## Pushing +Shell commands like `cp`, `mv`, and `rm` may be aliased to include `-i` (interactive) mode on some systems, causing the agent to hang indefinitely waiting for y/n input. -```bash -git push origin -# If rebased: -git push --force-with-lease origin -``` +**Use these forms instead:** -## Branch name - -Your branch is `feat/`. It is created by the Castellarius. Check with: ```bash -git branch --show-current +# Force overwrite without prompting +cp -f source dest # NOT: cp source dest +mv -f source dest # NOT: mv source dest +rm -f file # NOT: rm file + +# For recursive operations +rm -rf directory # NOT: rm -r directory +cp -rf source dest # NOT: cp -r source dest ``` -## Skill: code-simplifier - ---- -name: code-simplifier -description: Simplifies and refines code for clarity, consistency, and maintainability while preserving all functionality. Focuses on recently modified code unless instructed otherwise. -model: opus ---- - -You are an expert code simplification specialist focused on enhancing code clarity, consistency, and maintainability while preserving exact functionality. Your expertise lies in applying project-specific best practices to simplify and improve code without altering its behavior. You prioritize readable, explicit code over overly compact solutions. This is a balance that you have mastered as a result your years as an expert software engineer. - -You will analyze recently modified code and apply refinements that: - -1. **Preserve Functionality**: Never change what the code does - only how it does it. All original features, outputs, and behaviors must remain intact. - -2. **Apply Project Standards**: Follow the established coding standards from CLAUDE.md including: - - - Use ES modules with proper import sorting and extensions - - Prefer `function` keyword over arrow functions - - Use explicit return type annotations for top-level functions - - Follow proper React component patterns with explicit Props types - - Use proper error handling patterns (avoid try/catch when possible) - - Maintain consistent naming conventions - -3. **Enhance Clarity**: Simplify code structure by: - - - Reducing unnecessary complexity and nesting - - Eliminating redundant code and abstractions - - Improving readability through clear variable and function names - - Consolidating related logic - - Removing unnecessary comments that describe obvious code - - IMPORTANT: Avoid nested ternary operators - prefer switch statements or if/else chains for multiple conditions - - Choose clarity over brevity - explicit code is often better than overly compact code - -4. **Maintain Balance**: Avoid over-simplification that could: +**Other commands that may prompt:** - - Reduce code clarity or maintainability - - Create overly clever solutions that are hard to understand - - Combine too many concerns into single functions or components - - Remove helpful abstractions that improve code organization - - Prioritize "fewer lines" over readability (e.g., nested ternaries, dense one-liners) - - Make the code harder to debug or extend +- `scp` - use `-o BatchMode=yes` for non-interactive +- `ssh` - use `-o BatchMode=yes` to fail instead of prompting +- `apt-get` - use `-y` flag +- `brew` - use `HOMEBREW_NO_AUTO_UPDATE=1` env var -5. **Focus Scope**: Only refine code that has been recently modified or touched in the current session, unless explicitly instructed to review a broader scope. +## Cistern Workflow for AI Agents -Your refinement process: +1. **Read CONTEXT.md** — your work item, requirements, and any revision notes are there +2. **Implement** — write tests first (TDD), then implementation +3. **Run tests** — `make test` for Go; `cd frontend && npm test` for frontend +4. **Commit** — `git commit -m ": "` (do NOT push) +5. **Signal outcome** — use the appropriate `ct droplet` command from Quick Reference above -1. Identify the recently modified code sections -2. Analyze for opportunities to improve elegance and consistency -3. Apply project-specific best practices and coding standards -4. Ensure all functionality remains unchanged -5. Verify the refined code is simpler and more maintainable -6. Document only significant changes that affect understanding +### Signaling Rules -You operate autonomously and proactively, refining code immediately after it's written or modified without requiring explicit requests. Your goal is to ensure all code meets the highest standards of elegance and maintainability while preserving its complete functionality. +- Always include `--notes` describing what you did or found +- Never signal `pass` if required issues remain unresolved +- Use `block` only for genuine external blockers, not for ordinary revision work +- Local commits only — do NOT push to origin diff --git a/internal/handler/executions.go b/internal/handler/executions.go index 93b02824..b0048cc8 100644 --- a/internal/handler/executions.go +++ b/internal/handler/executions.go @@ -33,6 +33,10 @@ type ExecutionsHandler struct { WorkerToken string // auth token workers use to report back APIBaseURL string // base URL workers use to call the API Webhooks *webhook.Notifier // optional; nil means no webhook dispatch + + // ownsExecFunc overrides ownsExecution for testing. If nil, the DB-based + // implementation is used. Callers must not set this outside of tests. + ownsExecFunc func(ctx context.Context, executionID, teamID string) (bool, error) } // CreateExecutionRequest is the request body for creating a test execution. @@ -447,15 +451,21 @@ func getExecution(ctx context.Context, pool *db.Pool, id, teamID string) (*model } // ownsExecution checks whether the given execution belongs to the specified team. -func (h *ExecutionsHandler) ownsExecution(ctx context.Context, executionID, teamID string) bool { +// Returns (false, nil) if the execution does not belong to the team, +// (false, err) on database errors (distinguishing 404 from 500), +// and (true, nil) if the execution belongs to the team. +func (h *ExecutionsHandler) ownsExecution(ctx context.Context, executionID, teamID string) (bool, error) { + if h.ownsExecFunc != nil { + return h.ownsExecFunc(ctx, executionID, teamID) + } var exists bool err := h.DB.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM test_executions WHERE id = $1 AND team_id = $2)`, executionID, teamID).Scan(&exists) if err != nil { - return false + return false, err } - return exists + return exists, nil } func (h *ExecutionsHandler) requireWorkerCallback(w http.ResponseWriter, r *http.Request) (*auth.Claims, string, bool) { @@ -476,7 +486,12 @@ func (h *ExecutionsHandler) requireWorkerCallback(w http.ResponseWriter, r *http return nil, "", false } - if !h.ownsExecution(r.Context(), executionID, claims.TeamID) { + owned, err := h.ownsExecution(r.Context(), executionID, claims.TeamID) + if err != nil { + Error(w, http.StatusInternalServerError, "database error") + return nil, "", false + } + if !owned { Error(w, http.StatusNotFound, "execution not found") return nil, "", false } diff --git a/internal/handler/executions_test.go b/internal/handler/executions_test.go index fdb6eb0c..d97d6c77 100644 --- a/internal/handler/executions_test.go +++ b/internal/handler/executions_test.go @@ -1,11 +1,15 @@ package handler import ( + "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "strings" "testing" + + "github.com/jackc/pgx/v5/pgxpool" ) func TestListExecutions_Unauthorized(t *testing.T) { @@ -309,6 +313,86 @@ func TestReportWorkerStatus_NoDB(t *testing.T) { } } +func TestReportProgress_CrossTeam_Forbidden(t *testing.T) { + h := &ExecutionsHandler{ + DB: new(pgxpool.Pool), + ownsExecFunc: func(_ context.Context, executionID, teamID string) (bool, error) { + return false, nil + }, + } + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/api/v1/executions/exec-1/progress", strings.NewReader(`{"total":1,"passed":1}`)) + r.Header.Set("Content-Type", "application/json") + r = testWithClaimsSimple(r, "user-1", "team-other", "owner") + r = testWithChiParam(r, "executionID", "exec-1") + + h.ReportProgress(w, r) + + if w.Code != http.StatusNotFound { + t.Errorf("ReportProgress cross-team: got %d, want %d", w.Code, http.StatusNotFound) + } +} + +func TestReportTestResult_CrossTeam_Forbidden(t *testing.T) { + h := &ExecutionsHandler{ + DB: new(pgxpool.Pool), + ownsExecFunc: func(_ context.Context, executionID, teamID string) (bool, error) { + return false, nil + }, + } + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/api/v1/executions/exec-1/test-result", strings.NewReader(`{"name":"test","status":"passed"}`)) + r.Header.Set("Content-Type", "application/json") + r = testWithClaimsSimple(r, "user-1", "team-other", "owner") + r = testWithChiParam(r, "executionID", "exec-1") + + h.ReportTestResult(w, r) + + if w.Code != http.StatusNotFound { + t.Errorf("ReportTestResult cross-team: got %d, want %d", w.Code, http.StatusNotFound) + } +} + +func TestReportWorkerStatus_CrossTeam_Forbidden(t *testing.T) { + h := &ExecutionsHandler{ + DB: new(pgxpool.Pool), + ownsExecFunc: func(_ context.Context, executionID, teamID string) (bool, error) { + return false, nil + }, + } + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/api/v1/executions/exec-1/worker-status", strings.NewReader(`{"worker_id":"w1","status":"running"}`)) + r.Header.Set("Content-Type", "application/json") + r = testWithClaimsSimple(r, "user-1", "team-other", "owner") + r = testWithChiParam(r, "executionID", "exec-1") + + h.ReportWorkerStatus(w, r) + + if w.Code != http.StatusNotFound { + t.Errorf("ReportWorkerStatus cross-team: got %d, want %d", w.Code, http.StatusNotFound) + } +} + +func TestReportProgress_DBError_Returns500(t *testing.T) { + h := &ExecutionsHandler{ + DB: new(pgxpool.Pool), + ownsExecFunc: func(_ context.Context, executionID, teamID string) (bool, error) { + return false, fmt.Errorf("connection refused") + }, + } + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/api/v1/executions/exec-1/progress", strings.NewReader(`{"total":1,"passed":1}`)) + r.Header.Set("Content-Type", "application/json") + r = testWithClaimsSimple(r, "user-1", "team-1", "owner") + r = testWithChiParam(r, "executionID", "exec-1") + + h.ReportProgress(w, r) + + if w.Code != http.StatusInternalServerError { + t.Errorf("ReportProgress DB error: got %d, want %d", w.Code, http.StatusInternalServerError) + } +} + func TestErrorResponse_Format(t *testing.T) { w := httptest.NewRecorder() Error(w, http.StatusBadRequest, "test error") From 61ccc9a90daea1cdd9d61c1005fd040ed56f3385 Mon Sep 17 00:00:00 2001 From: Cistern Agent Date: Mon, 13 Apr 2026 22:33:16 -0600 Subject: [PATCH 4/4] sc-d9v2m: docs: update documentation for IDOR and worker callback auth fixes --- AGENTS.md | 234 ++++++++++++++++++++++++++++++++++++++------------- CHANGELOG.md | 4 + README.md | 11 ++- 3 files changed, 185 insertions(+), 64 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 60862174..b9f02159 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,96 +1,210 @@ -# Agent Instructions +You are a Cataracta operating within the Cistern agentic pipeline. -## Project Mission +Cistern is an automated software delivery system. The Castellarius (a pure state +machine) watches the cistern and routes droplets (units of work) into named +aqueducts. You are one cataractae — one gate — in that aqueduct. You receive a +droplet, complete your assigned role, and signal your outcome so the droplet +continues flowing. -ScaledTest is an end-to-end testing platform for running tests at massive scale with unprecedented reporting capabilities. It serves both humans (visual dashboards, analytics) and machines (CI integration, quality gates, webhooks). All test reporting uses the CTRF (Common Test Results Format) standard. +THE CASTELLARIUS WATCHES THE CISTERN, ROUTES DROPLETS INTO AVAILABLE AQUEDUCTS. +EACH AQUEDUCT FLOWS THE DROPLET THROUGH ITS CATARACTAE. ---- +## Your contract — non-negotiable -This project uses **Cistern** (`ct`) for work item tracking. Your work item is described in `CONTEXT.md`. +1. Read CONTEXT.md before doing anything else. It contains your droplet ID, + requirements, and all revision notes from prior cycles. +2. Adopt the persona described in your role instructions below. +3. Complete your work according to that persona. +4. Every 60 seconds while working, call: ct droplet heartbeat + This signals the scheduler that you are alive and making progress. Without + heartbeats, the stall detector may flag your session as stuck. +5. Signal your outcome before exiting. You MUST call one of: + ct droplet pass --notes "..." + ct droplet recirculate --notes "..." + ct droplet pool --notes "..." + A cataractae that exits without signaling leaves the droplet stranded. -## Quick Reference +Your role persona and skill instructions follow. + +## System safety invariants — never break these + +The Castellarius is a state machine. Its correctness depends on these invariants holding. If your work could affect any of them, you must verify they still hold before signaling pass. + +**1. Signaling is the only valid way to advance state.** +Never manipulate droplet state by any means other than ct droplet pass/recirculate/pool. +Never exit without signaling — a stranded droplet burns resources indefinitely. + +**2. Session spawning must expose the agent process directly to tmux.** +Do not wrap the agent command in a shell (bash -c, sh -c, pipes, tee) unless you have explicitly verified that pane_current_command and /proc//cmdline still correctly identify the agent. Wrappers that change what the process monitor sees will cause every healthy session to be misclassified as exited without outcome and respawned in a loop. + +**3. CONTEXT.md is pipeline state — never commit it.** +CONTEXT.md is injected at dispatch time and listed in .gitignore. If you see it in a git add or git commit, stop. Committing it causes merge conflicts across concurrent deliveries and corrupts origin/main. + +**4. The circuit breaker pools after 5 dead sessions in 15 minutes.** +If your droplet keeps dying without signaling an outcome, the scheduler will pool it automatically. This stops token burn — a human needs to investigate before the droplet is re-opened. + +**5. Do not call git add -f or git add --force on any ignored file.** +The .gitignore exists for a reason. Overriding it for pipeline state files (CONTEXT.md, .current-stage, session logs) corrupts the state machine. + +## Your Role + + + +# Role: Docs Writer + +You are a documentation writer in a Cistern Aqueduct. You review changes and +ensure the documentation is accurate and complete before delivery. + +## Context + +You have **full codebase access**. Your environment contains: + +- The full repository with the implementation committed +- `CONTEXT.md` describing the work item and requirements + +Read `CONTEXT.md` first to understand your droplet ID and what was built. + +## Protocol + +1. **Read CONTEXT.md** — note your droplet ID and what changed +2. **Run git diff main...HEAD** — understand all user-visible changes +3. **Find all .md files** — `find . -name "*.md" -not -path "./.git/*"` +4. **Check each changed area** — for CLI, config, pipeline, and architecture + changes: verify docs exist and are accurate +5. **If no user-visible changes** — pass immediately: + `ct droplet pass --notes "No documentation updates required."` +6. **Otherwise** — update outdated sections, add missing docs +7. **Commit** — `git add -A && git commit -m ": docs: update documentation for changes"` +8. **Signal outcome** + +## Signaling -```bash -ct droplet pass --notes "..." # Signal work complete -ct droplet block --notes "..." # Signal blocked (external dependency) -ct droplet recirculate --notes "..." # Send back for rework -ct droplet note "..." # Add a progress note ``` +ct droplet pass --notes "Updated docs: ." +ct droplet recirculate --notes "Ambiguous: " +``` + +## Skills + +## Skill: cistern-droplet-state + +# Cistern Droplet State + +Manage droplet state in the Cistern agentic pipeline using the `ct` CLI. + +## When to use this skill -## Architecture +Use at the end of every cataractae session to signal your outcome. +You MUST signal before exiting. A cataractae that exits without signaling leaves +the droplet stranded in the cistern. -ScaledTest is a v2 Go backend + React SPA: +## Your Droplet ID -- **Go backend**: chi router, pgxpool, JWT auth, RBAC, CTRF ingestion -- **React frontend**: React 19, TanStack Router/Query, Zustand, Recharts — served as embedded SPA via `go:embed` -- **K8s Job management** for distributed test execution -- **Quality gate rule DSL**, WebSocket hub for real-time updates +Your droplet ID is in CONTEXT.md under `## Item: `. Use it in every command. -### Key Directories +## Signaling Commands +### Pass — work complete, ready to flow forward +```bash +ct droplet pass --notes "Summary of what was done and verified." ``` -cmd/ # Go binaries (server, worker, ci-notify) -internal/ # Go backend packages (auth, handler, store, ctrf, quality, ws, …) -frontend/ # React 19 SPA (Vite + TypeScript) -sdk/ # TypeScript SDK -migrations/ # Database migrations -e2e/ # End-to-end tests + +### Recirculate — needs revision, send back upstream +```bash +ct droplet recirculate --notes "Specific issues: 1. 2. " ``` -### Test Commands +### Recirculate to a specific cataractae +```bash +ct droplet recirculate --to implement --notes "Reason for routing to implement." +``` +### Pool — cannot proceed ```bash -make test # All Go tests (with race detector) -make test-short # Fast Go tests (no race) -make test-integration # Integration tests (requires TEST_DATABASE_URL) -cd frontend && npm test # Frontend tests (Vitest) +ct droplet pool --notes "Cannot proceed because: " ``` -## Work Submission +### Add a note (without signaling) +```bash +ct droplet note "Intermediate finding or progress update." +``` + +## Rules + +1. Always include `--notes` when signaling — describe what you did or found +2. **Implementer**: signal `pass` when your work is committed and tests pass. Never signal `recirculate` — that is the reviewer's signal. Open issues are for the reviewer to verify and resolve; you cannot resolve your own issues. +3. **Reviewer/QA/Security**: signal `recirculate` when you have findings that require implementation changes. Signal `pass` when all prior issues are resolved and no new issues found. +4. Implementer: never push to origin — local commits only +5. Be specific in notes — "Fixed 3 issues in client.go" not "fixed it" +6. If CONTEXT.md has revision notes from prior cycles, address every single one + +## Skill: cistern-git + +--- +name: cistern-git +description: Git conventions for Cistern aqueduct cataractae. Use for all git operations in sandboxes: staging, committing, diffing, branching, and pushing. Covers rules specific to per-droplet worktrees, CONTEXT.md exclusion, merge-base diff, and no-stash policy. +--- + +# Cistern Git Conventions + +## Worktree model + +Each droplet has an isolated worktree at `~/.cistern/sandboxes///`. +The Castellarius creates and cleans up worktrees. Agents never run `git worktree add/remove`. + +## Staging — always exclude CONTEXT.md -Create PRs using the standard GitHub CLI: +CONTEXT.md is written by the Castellarius on every dispatch. Never commit it. ```bash -gh pr create --title "sc-XXXXX: short description" --body "..." +git add -A -- ':!CONTEXT.md' +git status --short # verify no CONTEXT.md, no binaries ``` -## Non-Interactive Shell Commands +## Committing — verify HEAD advances -**ALWAYS use non-interactive flags** with file operations to avoid hanging on confirmation prompts. +```bash +BEFORE=$(git rev-parse HEAD) +git add -A -- ':!CONTEXT.md' +git commit -m ": " +AFTER=$(git rev-parse HEAD) +if [ "$BEFORE" = "$AFTER" ]; then + echo "ERROR: nothing staged. Commit failed." + ct droplet recirculate --notes "Commit failed: HEAD did not advance." + exit 1 +fi +``` -Shell commands like `cp`, `mv`, and `rm` may be aliased to include `-i` (interactive) mode on some systems, causing the agent to hang indefinitely waiting for y/n input. +## Diffing — always use merge-base -**Use these forms instead:** +```bash +# Correct — shows only this branch's own changes, regardless of rebase state +git diff $(git merge-base HEAD origin/main)..HEAD +git diff $(git merge-base HEAD origin/main)..HEAD --name-only +``` + +Two-dot (`git diff origin/main..HEAD`) is wrong for unrebased branches: it includes all commits since the branch diverged, meaning other PRs that merged to main after branching appear in the diff. Merge-base is always correct. +To list commits on the branch: ```bash -# Force overwrite without prompting -cp -f source dest # NOT: cp source dest -mv -f source dest # NOT: mv source dest -rm -f file # NOT: rm file - -# For recursive operations -rm -rf directory # NOT: rm -r directory -cp -rf source dest # NOT: cp -r source dest +git log $(git merge-base HEAD origin/main)..HEAD --oneline ``` -**Other commands that may prompt:** +## No stash -- `scp` - use `-o BatchMode=yes` for non-interactive -- `ssh` - use `-o BatchMode=yes` to fail instead of prompting -- `apt-get` - use `-y` flag -- `brew` - use `HOMEBREW_NO_AUTO_UPDATE=1` env var +Per-droplet worktrees start clean. **Never run `git stash`** — it silently hides uncommitted work and has caused phantom empty deliveries. If the worktree is dirty before your work, the Castellarius will detect it and recirculate. -## Cistern Workflow for AI Agents +## Pushing -1. **Read CONTEXT.md** — your work item, requirements, and any revision notes are there -2. **Implement** — write tests first (TDD), then implementation -3. **Run tests** — `make test` for Go; `cd frontend && npm test` for frontend -4. **Commit** — `git commit -m ": "` (do NOT push) -5. **Signal outcome** — use the appropriate `ct droplet` command from Quick Reference above +```bash +git push origin +# If rebased: +git push --force-with-lease origin +``` -### Signaling Rules +## Branch name -- Always include `--notes` describing what you did or found -- Never signal `pass` if required issues remain unresolved -- Use `block` only for genuine external blockers, not for ordinary revision work -- Local commits only — do NOT push to origin +Your branch is `feat/`. It is created by the Castellarius. Check with: +```bash +git branch --show-current +``` diff --git a/CHANGELOG.md b/CHANGELOG.md index 5382bfc7..25139794 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ All notable changes to this project will be documented here. ### Fixed +- **IDOR vulnerability in invitation handlers**: `Create`, `List`, and `Revoke` invitation endpoints (`POST/GET/DELETE /api/v1/teams/{teamID}/invitations`) now verify that the authenticated user's team matches the URL `teamID` before checking role permissions. Previously, any maintainer or owner could list, create, or revoke invitations for any team regardless of membership. + +- **Worker callback authorization gap**: `ReportProgress`, `ReportTestResult`, and `ReportWorkerStatus` endpoints (`POST /api/v1/executions/{executionID}/progress|test-result|worker-status`) now verify that the execution belongs to the caller's team before proceeding. Previously, any authenticated user could broadcast WebSocket messages for any execution by guessing IDs. Unauthorized or cross-team requests return 404 (to avoid information leakage); database errors return 500 (fail closed). + - **`GET /api/v1/reports/compare` endpoint returning 500**: Fixed a database query issue where NULL values in optional text columns (`message`, `trace`, `file_path`, `suite`) could not be scanned into string destinations in pgx v5, causing the compare endpoint to return HTTP 500 for reports with missing optional fields. The fix wraps these columns with `COALESCE(..., '')` to convert NULL to empty string, ensuring the endpoint returns HTTP 200 with a valid diff payload. The fix maintains team isolation — reports from different teams return HTTP 404. - **`GET /api/v1/reports` (ListReports) query parameter validation**: The `since` and `until` query parameters now return HTTP 400 with a clear error message when provided in a malformed format (not RFC3339). Previously, malformed dates were silently ignored, causing the endpoint to return all records instead of signaling a bad request. Empty string values for these parameters continue to be accepted and ignored as before. diff --git a/README.md b/README.md index ee0ecce9..efbda937 100644 --- a/README.md +++ b/README.md @@ -257,7 +257,7 @@ curl -X GET "https://your-instance/api/v1/reports/compare?base=& ### Invitations -Team owners and maintainers can invite users by email. The invitee receives a token link that opens a sign-up page. +Team owners and maintainers can invite users by email. The invitee receives a token link that opens a sign-up page. All authenticated endpoints below require the caller to be a member of the specified team (the `teamID` in the URL must match the caller's team). ```bash # Create an invitation (maintainer or owner; returns token shown once) @@ -296,6 +296,9 @@ Tokens are prefixed `inv_`, valid for **7 days**, and stored as SHA-256 hashes. | `GET` | `/api/v1/executions` | List executions | | `DELETE` | `/api/v1/executions/{id}` | Cancel/delete execution | | `PUT` | `/api/v1/executions/{id}/status` | Update execution status | +| `POST` | `/api/v1/executions/{id}/progress` | Report test progress (worker callback; team-scoped) | +| `POST` | `/api/v1/executions/{id}/test-result` | Report individual test result (worker callback; team-scoped) | +| `POST` | `/api/v1/executions/{id}/worker-status` | Report worker health (worker callback; team-scoped) | | `GET` | `/api/v1/analytics/trends` | Pass-rate trends | | `GET` | `/api/v1/analytics/flaky-tests` | Flaky test detection | | `GET` | `/api/v1/teams/{id}/quality-gates` | List quality gates | @@ -313,9 +316,9 @@ Tokens are prefixed `inv_`, valid for **7 days**, and stored as SHA-256 hashes. | `GET` | `/api/v1/teams/{id}/webhooks/{wid}/deliveries` | List recent delivery attempts | | `POST` | `/api/v1/teams/{id}/webhooks/{wid}/deliveries/{did}/retry` | Re-dispatch a stored delivery (maintainer+) | | `GET` | `/api/v1/teams` | List teams | -| `POST` | `/api/v1/teams/{id}/invitations` | Invite user to team | -| `GET` | `/api/v1/teams/{id}/invitations` | List team invitations | -| `DELETE` | `/api/v1/teams/{id}/invitations/{iid}` | Revoke invitation | +| `POST` | `/api/v1/teams/{id}/invitations` | Invite user to team (team member; maintainer+) | +| `GET` | `/api/v1/teams/{id}/invitations` | List team invitations (team member; maintainer+) | +| `DELETE` | `/api/v1/teams/{id}/invitations/{iid}` | Revoke invitation (team member; maintainer+) | | `GET` | `/api/v1/invitations/{token}` | Preview invitation (public) | | `POST` | `/api/v1/invitations/{token}/accept` | Accept invitation (public) | | `GET` | `/api/v1/teams/{id}/tokens` | List API tokens |