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 | diff --git a/internal/handler/executions.go b/internal/handler/executions.go index 9b8ade39..b0048cc8 100644 --- a/internal/handler/executions.go +++ b/internal/handler/executions.go @@ -26,13 +26,17 @@ 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 + + // 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. @@ -446,6 +450,55 @@ 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. +// 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, err + } + return exists, nil +} + +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 + } + + 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 + } + + return claims, executionID, true +} + // ProgressRequest is the request body for reporting test progress. type ProgressRequest struct { Passed int `json:"passed"` @@ -459,9 +512,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) { - executionID := chi.URLParam(r, "executionID") - if executionID == "" { - Error(w, http.StatusBadRequest, "missing execution ID") + _, executionID, ok := h.requireWorkerCallback(w, r) + if !ok { return } @@ -503,9 +555,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) { - executionID := chi.URLParam(r, "executionID") - if executionID == "" { - Error(w, http.StatusBadRequest, "missing execution ID") + _, executionID, ok := h.requireWorkerCallback(w, r) + if !ok { return } @@ -545,9 +596,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) { - executionID := chi.URLParam(r, "executionID") - if executionID == "" { - Error(w, http.StatusBadRequest, "missing execution ID") + _, executionID, ok := h.requireWorkerCallback(w, r) + if !ok { return } diff --git a/internal/handler/executions_test.go b/internal/handler/executions_test.go index 540ede8c..d97d6c77 100644 --- a/internal/handler/executions_test.go +++ b/internal/handler/executions_test.go @@ -1,13 +1,16 @@ 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) { h := &ExecutionsHandler{} @@ -223,6 +226,173 @@ 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 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") 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{}