diff --git a/apps/backend/AGENTS.md b/apps/backend/AGENTS.md index bfefa7015f..2330c8e157 100644 --- a/apps/backend/AGENTS.md +++ b/apps/backend/AGENTS.md @@ -177,7 +177,7 @@ Standalone agentctl is launched in its own process group so terminal Ctrl+C is h - **New user-facing service entry points must apply scoping** — call the `authorize*` helpers in `task/service/service_access.go` (or the same pattern) when adding routes that read or mutate workspace-scoped data. This includes **session-keyed** entry points: any service method taking a caller-supplied `sessionID` (or a `taskID` used to reach sessions) authorizes via `authorizeTaskID(session.TaskID)`. Authorize off the row you already read rather than calling `AuthorizeSessionAccess`, which re-reads the session. Batch helpers that take ID *lists* (`BatchGetSessionsForTasks`, `GetPrimarySessionInfoForTasks`, …) are exempt by convention: their callers derive the IDs from an already-authorized task list, so per-ID checks would add N queries to list views for no gain — keep it that way, and don't hand them caller-supplied IDs. - **Two session paths bypass the task service and must guard themselves.** (1) Handlers that resolve an execution by a bare in-memory lookup (`GetExecutionBySessionID`, `*BySessionID`) skip the `GetOrEnsure*` chokepoint where the lifecycle check runs — call `service.AuthorizeSessionAccess` (see `ProcessHandlers.denySessionAccess`) or `lifecycleMgr.CheckSessionAccess` first. (2) The orchestrator reads sessions through its own repo handle, so it inherits nothing from the task service; session-keyed entry points there call `authorizeSession` / `authorizeTask`, wired by `SetSessionAccessChecker` / `SetTaskAccessChecker`. An entry point that accepts **both** a task and a session ID must use `authorizeTaskSessionPair`, never `authorizeSession` alone: the caller can satisfy a session-only check with one of their own sessions while pointing `taskID` at someone else's task, which the method then uses for its task-scoped work. Put the guard **first** in the method, before any repo/executor/agent-manager use — `TestSessionKeyedEntryPointsGuardBeforeDependencies` runs each entry point with nil dependencies and fails on a panic if a guard is placed too late. - **The workflow-step surface scopes itself in `internal/workflow`.** Steps, workspace step lists, export and import all reach a workflow or workspace by caller-supplied ID, and none of those IDs is a name the WS dispatch backstop parses, so the guard has to be in this package. `internal/workflow/service/access.go` holds `AuthorizeWorkflow` / `AuthorizeWorkspace` / `AuthorizeStep`, backed by `SetWorkflowAccessChecker` / `SetWorkspaceAccessChecker` (wired to `taskSvc.AuthorizeWorkflowAccess` / `AuthorizeWorkspaceAccess` in `backendapp/services.go`, next to the older `SetSessionAccessChecker`). Step CRUD authorizes in the **controller**, because REST, WS and the MCP tools all share it; `ListStepsByWorkspaceID` and the export/import trio authorize in the **service**, because the boot-payload builder and the MCP config tools call those directly. A denial and a genuine miss both surface as `service.ErrNotVisible` → one 404, so a foreign step and a nonexistent one are indistinguishable. Templates are deliberately unscoped (install-global, ownerless). Note that an empty ID **fails closed** here rather than being passed to `AuthorizeWorkspaceAccess`, which reads `""` as "no scoping applies". A step also *contains* IDs — `move_to_step` names a step, `queue_run` names a task — and the engine dereferences them later on the event bus, with no identity to check against, so they are validated at write time: `models.CollectStepEventReferences` enumerates them (keep it in sync with `RemapStepEvents` when adding a trigger), a transition target must resolve to a step in the same workflow, and a task target is authorized like any other. A reorder's step IDs must belong to the workflow named in the URL, which is also what keeps a read-only workflow's steps from being reordered through a mutable workflow's route. -- **Dispatched WS actions have a gateway backstop.** `Client.authorizeAction` (`internal/gateway/websocket/dispatch_scope.go`) checks any payload carrying `task_id` or `session_id` against the client's identity *before* dispatch, so a newly added action is scoped by default rather than only when its author remembers. Handler-level scoping is still required (defense in depth, and it covers non-WS callers) — do not delete a service-layer check because the backstop exists. The backstop reads `task_id`, `session_id` and `task_environment_id`, plus `id` on **top-level `task.` actions only** (`task.state`, `task.move`, `task.get`, … — `task.` and exactly one more segment). Keep using those names for task-scoped actions, because an action that invents a new name for the same kind of resource silently opts out. (`user_shell.stop` was exactly that: it keys off `task_environment_id` with `task_id` optional, so a `task_id`-only backstop missed it. `task.state` and `task.move` were the second instance: they name the task `id`, so the backstop parsed no refs and let one user mutate any other user's task — hence the `id` rule.) Deeper `task.*` namespaces are deliberately excluded: in `task.plan.revision.get` and `task.review.finding.update` the `id` names a revision or a finding, so those actions must carry `task_id` when they are task-scoped. Also avoid permissive types (`any`, `json.RawMessage`) for those fields: a payload that fails to unmarshal into the ref struct is treated as naming nothing. +- **Dispatched WS actions have a gateway backstop.** `Client.authorizeAction` (`internal/gateway/websocket/dispatch_scope.go`) checks any payload carrying `task_id` or `session_id` against the client's identity *before* dispatch, so a newly added action is scoped by default rather than only when its author remembers. Handler-level scoping is still required (defense in depth, and it covers non-WS callers) — do not delete a service-layer check because the backstop exists. The backstop reads `task_id`, `session_id` and `task_environment_id`, plus `id` on **top-level `task.` actions only** (`task.state`, `task.move`, `task.get`, … — `task.` and exactly one more segment). Keep using those names for task-scoped actions, because an action that invents a new name for the same kind of resource silently opts out. (`user_shell.stop` was exactly that: it keys off `task_environment_id` with `task_id` optional, so a `task_id`-only backstop missed it. `task.state` and `task.move` were the second instance: they name the task `id`, so the backstop parsed no refs and let one user mutate any other user's task — hence the `id` rule.) Deeper `task.*` namespaces are deliberately excluded: in `task.plan.revision.get` and `task.review.finding.update` the `id` names a revision or a finding, so those actions must carry `task_id` when they are task-scoped. Also avoid permissive types (`any`, `json.RawMessage`) for those fields: a payload that fails to unmarshal into the ref struct is treated as naming nothing. **The backstop does not cover HTTP.** The dispatch backstop only sees WS payloads, so an HTTP route that names a task or environment has to guard itself, and only the third-party integration prefixes have a global middleware. The two SSR terminal lists (`GET /api/v1/environments/:id/terminals`, `GET /api/v1/tasks/:id/terminals`) were the gap: they read straight from the interactive runner and the terminal service (which has no authorization of its own), leaking another user's terminal display names and initial command lines. They now mount `authorizeEnvironmentRoute` / `authorizeTaskRoute` (`internal/agent/handlers/shell_handlers.go`) as gin route guards calling `lifecycleMgr.CheckEnvironmentAccess` / `CheckTaskAccess`, so the check runs before any terminal state is read. Guard **every** ID the handler consumes, not just the path param: `/tasks/:id/terminals` also takes `?task_environment_id=`, which is a second way into a foreign environment. When a handler merges state keyed by two IDs, authorize them as a **pair** (`AuthorizeTaskEnvironmentAccess`, the task/environment sibling of `AuthorizeTaskSessionAccess`) rather than independently: both single-ID checks pass for a caller who holds each ID separately, and the handler then merges two unrelated lists. Pair on the binding, not on row ownership: `inherit_parent` binds a subtask's session to the parent task's environment and `shared_group` binds every group member to one canonical environment, so `env.TaskID == taskID` is the wrong predicate and would empty the terminal panel for both modes. - **WS**: clients carry their identity; dispatched actions and subscriptions are scoped; workspace-carrying events route via `Hub.BroadcastToWorkspace`. A new `hub.Broadcast` (global) call site needs a `//ws:global` justification comment. Subscription actions (`*.subscribe`) `return` before reaching the dispatch backstop (`Client.authorizeAction`), so scoping a new subscribe topic means adding a hook to `SubscriptionAccessPolicy` (`internal/gateway/websocket/access.go`) *and* calling it explicitly inside that action's own handler — the backstop's `scopedActionRefs` cannot reach it. `run.subscribe`'s `Subscriptions.Run` hook (added for WO-02, resolving the run's workspace via `runs.GetRunWorkspaceID`) is the worked example. - **Task overview vs. session detail:** Keep cross-task state in the bounded, persisted `TaskStatusSummary` projection and `task.status_summary.updated`; keep unbounded transcript and execution data session-scoped. The [bounded task-status spec](../../docs/specs/platform/requirements/bounded-task-status-delivery.md) and [accepted ADR](../../docs/decisions/2026-08-01-separate-task-summary-session-stream-traffic.md) define the shared contract. - **Port-proxy subtree capability:** with auth enforced, `/port-proxy/:sessionId/:port/*` needs a credential on *every* request, but browser subresource fetches cannot always carry one (the `` fetch never sends cookies; sandboxed iframes and `?token=`-opened previews drop them elsewhere). `PortProxyHandler` mints a short-lived (15 min, sliding), path-scoped, HMAC-signed capability after the document authenticates and propagates it as a cookie scoped to the proxy subtree plus a `kandev_cap` query parameter appended to rewritten asset URLs. `requireConnectionAuth` (`internal/gateway/websocket/access.go`) accepts either form only for the exact session:port it was minted for and restores the issuing identity, so `CheckSessionAccess` still gates on the real owner. Synthetic identities (auth disabled) skip minting — proxied bodies stay byte-identical to the pre-auth behavior. diff --git a/apps/backend/internal/agent/handlers/shell_handlers.go b/apps/backend/internal/agent/handlers/shell_handlers.go index c04bb3fe8c..3e96d3d99c 100644 --- a/apps/backend/internal/agent/handlers/shell_handlers.go +++ b/apps/backend/internal/agent/handlers/shell_handlers.go @@ -693,8 +693,81 @@ func RegisterShellRoutes(router *gin.Engine, lifecycleMgr *lifecycle.Manager, lo // and the terminal service has been injected. func RegisterShellRoutesOn(router *gin.Engine, handlers *ShellHandlers) { api := router.Group("/api/v1") - api.GET("/environments/:id/terminals", handlers.httpListTerminals) - api.GET("/tasks/:id/terminals", handlers.httpListTaskTerminals) + api.GET("/environments/:id/terminals", handlers.authorizeEnvironmentRoute, handlers.httpListTerminals) + api.GET("/tasks/:id/terminals", handlers.authorizeTaskRoute, handlers.httpListTaskTerminals) +} + +// Per-user scoping for the two SSR terminal lists (opt-in auth). +// +// Both handlers read terminal state directly - one from the interactive +// runner, one from the terminal service - so neither passes through the +// lifecycle manager's GetOrEnsure* chokepoint where the check normally runs, +// and the terminal service has no authorization of its own. Unguarded, any +// authenticated user could list another user's terminals by naming their task +// or environment, including the display names and initial command lines of +// their work. +// +// The WS equivalents (user_shell.list|create|stop|rename|park|resume) are +// covered structurally by the dispatch backstop in +// internal/gateway/websocket/dispatch_scope.go, which parses the same IDs out +// of the payload. These routes get the same treatment: the guard is mounted on +// the route rather than written into the handler body, so it runs before any +// terminal state is read and a future handler edit cannot quietly drop it. +// +// Denials are 404 with no distinction between "not yours" and "no such thing", +// matching the no-existence-leak convention documented at the top of +// internal/task/service/service_access.go. + +// authorizeEnvironmentRoute gates the env-keyed SSR list on :id. +func (h *ShellHandlers) authorizeEnvironmentRoute(c *gin.Context) { + if !h.allowEnvironment(c, c.Param("id")) { + return + } + c.Next() +} + +// authorizeTaskRoute gates the task-keyed SSR list on :id, plus the optional +// ?task_environment_id= it forwards to appendUnmanagedShells - without that +// second check the query param is a second way into a foreign environment. +// +// The two IDs are checked as a pair, not independently. This handler merges +// ordinary terminals belonging to the task with unmanaged shells belonging to +// the environment, and authorizing each ID on its own passes for a caller who +// holds both but for whom they are unrelated, which merges two unrelated +// terminal lists. +func (h *ShellHandlers) authorizeTaskRoute(c *gin.Context) { + taskID := c.Param("id") + if taskID != "" { + if err := h.lifecycleMgr.CheckTaskAccess(c.Request.Context(), taskID); err != nil { + c.AbortWithStatusJSON(http.StatusNotFound, gin.H{"error": "task not found"}) + return + } + } + envID := c.Query("task_environment_id") + if envID == "" { + c.Next() + return + } + if err := h.lifecycleMgr.CheckTaskEnvironmentAccess(c.Request.Context(), taskID, envID); err != nil { + c.AbortWithStatusJSON(http.StatusNotFound, gin.H{"error": "task environment not found"}) + return + } + c.Next() +} + +// allowEnvironment reports whether the caller may see environmentID, aborting +// the request with 404 when not. An empty ID is allowed through so the +// handlers keep returning their own 400 for a missing identifier; empty reads +// no state either way. +func (h *ShellHandlers) allowEnvironment(c *gin.Context, environmentID string) bool { + if environmentID == "" { + return true + } + if err := h.lifecycleMgr.CheckEnvironmentAccess(c.Request.Context(), environmentID); err != nil { + c.AbortWithStatusJSON(http.StatusNotFound, gin.H{"error": "task environment not found"}) + return false + } + return true } // httpListTerminals (env-keyed legacy SSR) — kept for backwards-compat with diff --git a/apps/backend/internal/agent/handlers/shell_scope_test.go b/apps/backend/internal/agent/handlers/shell_scope_test.go new file mode 100644 index 0000000000..f0eae72434 --- /dev/null +++ b/apps/backend/internal/agent/handlers/shell_scope_test.go @@ -0,0 +1,367 @@ +package handlers + +import ( + "context" + "database/sql" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-gonic/gin" + "github.com/jmoiron/sqlx" + _ "github.com/mattn/go-sqlite3" + + "github.com/kandev/kandev/internal/agent/runtime/lifecycle" + terminalrepo "github.com/kandev/kandev/internal/terminal/repository" + terminalservice "github.com/kandev/kandev/internal/terminal/service" +) + +// The two SSR terminal-listing routes read terminal state straight from the +// terminal service and the interactive runner, bypassing the lifecycle +// manager's checked accessors. With auth enabled that let any authenticated +// user list another user's terminals: display names and the initial command +// lines of their work. The WS siblings are covered by the dispatch backstop in +// internal/gateway/websocket/dispatch_scope.go; these HTTP routes had no +// equivalent, so the guard is mounted on the routes themselves. + +const ( + ownTaskID = "task-a" + ownEnvID = "env-a" + foreignTaskID = "task-b" + foreignEnvID = "env-b" + // unknown* stand in for IDs that exist for nobody. A foreign ID and an + // unknown one must be indistinguishable (no existence leak). + unknownTaskID = "task-zzz" + unknownEnvID = "env-zzz" + // unrelatedEnvID is an environment the caller DOES own, but which is not + // bound to ownTaskID. Authorizing the two IDs independently admits this + // pair; only a pairing check refuses it. + unrelatedEnvID = "env-a-other" + + // leakMarker is the initial command stored on the foreign task's + // terminal. It must never appear in a response to another user. + leakMarker = "psql -h prod-db -U admin" +) + +// scopedShellHandlers builds handlers whose lifecycle manager admits only +// ownTaskID/ownEnvID, backed by a real terminal service holding one terminal +// for the caller's task and one for a foreign task. +func scopedShellHandlers(t *testing.T) *ShellHandlers { + t.Helper() + mgr := &lifecycle.Manager{} + mgr.SetTaskAccessChecker(func(_ context.Context, taskID string) error { + if taskID == ownTaskID { + return nil + } + return errors.New("task not found") + }) + // The caller owns two environments; only ownEnvID is bound to ownTaskID. + mgr.SetEnvironmentAccessChecker(func(_ context.Context, envID string) error { + if envID == ownEnvID || envID == unrelatedEnvID { + return nil + } + return errors.New("task environment not found") + }) + mgr.SetTaskEnvironmentAccessChecker(func(_ context.Context, taskID, envID string) error { + if taskID == ownTaskID && envID == ownEnvID { + return nil + } + return errors.New("task not found") + }) + h := NewShellHandlers(mgr, nil, newTestLogger()) + h.SetTerminalService(seededTerminalService(t)) + return h +} + +// seededTerminalService returns a real terminal service over an in-memory +// SQLite DB, holding one terminal per task. Using the real service (rather +// than a stub returning nothing) means a leak would actually show up in the +// response body. +func seededTerminalService(t *testing.T) *terminalservice.Service { + t.Helper() + // Unique DSN per test: `cache=shared` in-memory DBs are keyed by name + // process-wide, so a fixed name would let parallel tests share rows. + rawDB, err := sql.Open("sqlite3", "file:"+strings.ReplaceAll(t.Name(), "/", "_")+"?mode=memory&cache=shared") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + rawDB.SetMaxOpenConns(1) + db := sqlx.NewDb(rawDB, "sqlite3") + t.Cleanup(func() { _ = db.Close() }) + repo, err := terminalrepo.NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("terminal repo: %v", err) + } + svc := terminalservice.New(repo, noopPTYBackend{}, nil) + if _, err := svc.Create(context.Background(), ownTaskID, ownEnvID, "echo mine"); err != nil { + t.Fatalf("seed own terminal: %v", err) + } + if _, err := svc.Create(context.Background(), foreignTaskID, foreignEnvID, leakMarker); err != nil { + t.Fatalf("seed foreign terminal: %v", err) + } + return svc +} + +// noopPTYBackend satisfies terminalservice.PTYBackend without a real agentctl. +type noopPTYBackend struct{} + +func (noopPTYBackend) Register(string, string) {} +func (noopPTYBackend) Stop(context.Context, string, string) error { return nil } +func (noopPTYBackend) StopScope(context.Context, string) (int, error) { return 0, nil } +func (noopPTYBackend) IsAlive(string, string) bool { return false } + +func scopedShellRouter(t *testing.T) *gin.Engine { + t.Helper() + gin.SetMode(gin.TestMode) + router := gin.New() + RegisterShellRoutesOn(router, scopedShellHandlers(t)) + return router +} + +func getPath(t *testing.T, router *gin.Engine, path string) *httptest.ResponseRecorder { + t.Helper() + recorder := httptest.NewRecorder() + router.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, path, nil)) + return recorder +} + +// TestSSRTerminalRoutesDenyForeignScope is the core regression: both routes +// must refuse a scope the caller cannot see, with 404 (not 403) and without +// echoing any terminal state. +func TestSSRTerminalRoutesDenyForeignScope(t *testing.T) { + cases := map[string]struct { + path string + want string + }{ + "environment route": { + path: "/api/v1/environments/" + foreignEnvID + "/terminals", + want: `{"error":"task environment not found"}`, + }, + "task route": { + path: "/api/v1/tasks/" + foreignTaskID + "/terminals", + want: `{"error":"task not found"}`, + }, + "task route with environment query": { + path: "/api/v1/tasks/" + foreignTaskID + "/terminals?task_environment_id=" + foreignEnvID, + want: `{"error":"task not found"}`, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + router := scopedShellRouter(t) + + recorder := getPath(t, router, tc.path) + + if recorder.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := strings.TrimSpace(recorder.Body.String()); got != tc.want { + t.Errorf("body = %s, want %s", got, tc.want) + } + if strings.Contains(recorder.Body.String(), leakMarker) { + t.Errorf("response leaked the foreign initial command: %s", recorder.Body.String()) + } + }) + } +} + +// TestSSRTaskTerminalsGuardsEnvironmentQuery covers the second way in: +// task_environment_id is passed to appendUnmanagedShells, so naming your own +// task while pointing the query param at someone else's environment must fail +// too. +func TestSSRTaskTerminalsGuardsEnvironmentQuery(t *testing.T) { + router := scopedShellRouter(t) + + recorder := getPath(t, router, "/api/v1/tasks/"+ownTaskID+"/terminals?task_environment_id="+foreignEnvID) + + if recorder.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := strings.TrimSpace(recorder.Body.String()); got != `{"error":"task environment not found"}` { + t.Errorf("body = %s", got) + } +} + +// TestSSRTerminalRoutesHideExistence keeps the no-existence-leak convention +// documented at the top of internal/task/service/service_access.go: a foreign +// ID and a nonexistent one must be byte-identical. +func TestSSRTerminalRoutesHideExistence(t *testing.T) { + cases := map[string][2]string{ + "environment route": { + "/api/v1/environments/" + foreignEnvID + "/terminals", + "/api/v1/environments/" + unknownEnvID + "/terminals", + }, + "task route": { + "/api/v1/tasks/" + foreignTaskID + "/terminals", + "/api/v1/tasks/" + unknownTaskID + "/terminals", + }, + } + for name, paths := range cases { + t.Run(name, func(t *testing.T) { + router := scopedShellRouter(t) + + foreign := getPath(t, router, paths[0]) + unknown := getPath(t, router, paths[1]) + + if foreign.Code != unknown.Code { + t.Errorf("status foreign = %d, unknown = %d", foreign.Code, unknown.Code) + } + if foreign.Body.String() != unknown.Body.String() { + t.Errorf("body foreign = %s, unknown = %s", foreign.Body.String(), unknown.Body.String()) + } + }) + } +} + +// TestSSRTerminalGuardsAbortBeforeReadingState proves the guard is a route +// filter, not a post-hoc response filter: the listing handler is the only code +// that touches the terminal service and the interactive runner, so a sentinel +// that never runs in its place means neither was queried. A "list then drop +// the rows" implementation cannot pass this. +func TestSSRTerminalGuardsAbortBeforeReadingState(t *testing.T) { + h := scopedShellHandlers(t) + cases := map[string]struct { + guard gin.HandlerFunc + route string + path string + }{ + "environment route": { + guard: h.authorizeEnvironmentRoute, + route: "/api/v1/environments/:id/terminals", + path: "/api/v1/environments/" + foreignEnvID + "/terminals", + }, + "task route": { + guard: h.authorizeTaskRoute, + route: "/api/v1/tasks/:id/terminals", + path: "/api/v1/tasks/" + foreignTaskID + "/terminals", + }, + "task route via environment query": { + guard: h.authorizeTaskRoute, + route: "/api/v1/tasks/:id/terminals", + path: "/api/v1/tasks/" + ownTaskID + "/terminals?task_environment_id=" + foreignEnvID, + }, + "task route via unrelated owned environment": { + guard: h.authorizeTaskRoute, + route: "/api/v1/tasks/:id/terminals", + path: "/api/v1/tasks/" + ownTaskID + "/terminals?task_environment_id=" + unrelatedEnvID, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + gin.SetMode(gin.TestMode) + router := gin.New() + reached := false + router.GET(tc.route, tc.guard, func(c *gin.Context) { + reached = true + c.JSON(http.StatusOK, gin.H{"terminals": []any{}}) + }) + + if code := getPath(t, router, tc.path).Code; code != http.StatusNotFound { + t.Fatalf("status = %d, want 404", code) + } + if reached { + t.Error("listing handler ran for a denied scope; terminal state was read before the guard") + } + }) + } +} + +// TestSSRTerminalRoutesServeOwner is the other half of the guard: the person +// who owns the task still gets their terminals. +// +// The manager here holds no executions and no interactive runner, which is the +// state after the agent behind the environment has been torn down. That is the +// case worth pinning: these routes back a server-rendered panel, so an +// over-eager guard does not surface as an error, it silently empties the +// owner's terminal list. Authorization must key off task and environment +// ownership only, never off a live execution. +func TestSSRTerminalRoutesServeOwner(t *testing.T) { + router := scopedShellRouter(t) + + recorder := getPath(t, router, "/api/v1/tasks/"+ownTaskID+"/terminals?task_environment_id="+ownEnvID) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", recorder.Code, recorder.Body.String()) + } + if !strings.Contains(recorder.Body.String(), "echo mine") { + t.Errorf("owner did not get their own terminal: %s", recorder.Body.String()) + } + if strings.Contains(recorder.Body.String(), leakMarker) { + t.Errorf("owner response included another task's terminal: %s", recorder.Body.String()) + } + + if code := getPath(t, router, "/api/v1/environments/"+ownEnvID+"/terminals").Code; code != http.StatusOK { + t.Errorf("environment route status = %d, want 200", code) + } +} + +// TestSSRTerminalGuardsAreNoOpWithAuthDisabled pins single-user behavior. +// With auth disabled no checker denies anything (callerScope reports unscoped +// for the synthetic identity, and nothing is wired at all in the legacy boot +// path), so both routes must answer exactly as they did before this guard. +func TestSSRTerminalGuardsAreNoOpWithAuthDisabled(t *testing.T) { + gin.SetMode(gin.TestMode) + router := gin.New() + h := NewShellHandlers(newTestManager(), nil, newTestLogger()) + h.SetTerminalService(seededTerminalService(t)) + RegisterShellRoutesOn(router, h) + + // Any ID at all — including one belonging to "another user" in the + // scoped tests — is served, because there is no scoping to apply. + for _, path := range []string{ + "/api/v1/environments/" + foreignEnvID + "/terminals", + "/api/v1/tasks/" + foreignTaskID + "/terminals?task_environment_id=" + foreignEnvID, + "/api/v1/tasks/" + unknownTaskID + "/terminals", + } { + recorder := getPath(t, router, path) + if recorder.Code != http.StatusOK { + t.Errorf("%s status = %d, want 200 (body %s)", path, recorder.Code, recorder.Body.String()) + } + } + + // The foreign task's rows are still returned verbatim in single-user mode. + if body := getPath(t, router, "/api/v1/tasks/"+foreignTaskID+"/terminals").Body.String(); !strings.Contains(body, leakMarker) { + t.Errorf("single-user mode lost a terminal: %s", body) + } + + // And the manager's own predicate is permissive when unwired. + bare := &lifecycle.Manager{} + if err := bare.CheckTaskAccess(context.Background(), ownTaskID); err != nil { + t.Errorf("unwired task checker denied the call (%v); pre-auth behavior broken", err) + } +} + +// TestSSRTaskTerminalsRefusesUnpairedEnvironment is the pairing regression. +// Authorizing the task ID and the environment ID independently both succeed +// for a caller who owns each of them separately; the handler would then merge +// the path task's ordinary terminals with the unrelated environment's +// unmanaged shells, labels and initial commands included. The guard must +// refuse the pair, and must do so before appendUnmanagedShells can run. +func TestSSRTaskTerminalsRefusesUnpairedEnvironment(t *testing.T) { + router := scopedShellRouter(t) + + recorder := getPath(t, router, "/api/v1/tasks/"+ownTaskID+"/terminals?task_environment_id="+unrelatedEnvID) + + if recorder.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (body %s)", recorder.Code, recorder.Body.String()) + } + if got := strings.TrimSpace(recorder.Body.String()); got != `{"error":"task environment not found"}` { + t.Errorf("body = %s", got) + } + + // The environment is legitimately the caller's own, so the env-keyed route + // still serves it. Only the *pair* is refused, not the environment. + if code := getPath(t, router, "/api/v1/environments/"+unrelatedEnvID+"/terminals").Code; code != http.StatusOK { + t.Errorf("env-keyed route status = %d, want 200; the pairing check must not gate the environment itself", code) + } + + // A refused pair is indistinguishable from a foreign one. + unrelated := getPath(t, router, "/api/v1/tasks/"+ownTaskID+"/terminals?task_environment_id="+unrelatedEnvID) + foreign := getPath(t, router, "/api/v1/tasks/"+ownTaskID+"/terminals?task_environment_id="+foreignEnvID) + if unrelated.Code != foreign.Code || unrelated.Body.String() != foreign.Body.String() { + t.Errorf("unrelated = %d %s, foreign = %d %s", unrelated.Code, unrelated.Body.String(), + foreign.Code, foreign.Body.String()) + } +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager.go b/apps/backend/internal/agent/runtime/lifecycle/manager.go index decf628298..90920ef93d 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager.go @@ -92,6 +92,16 @@ type Manager struct { // resolves executions by environment ID. Nil = no scoping. environmentAccessCheck func(ctx context.Context, environmentID string) error + // taskAccessCheck is the task-keyed sibling of sessionAccessCheck, used by + // the task-keyed SSR terminal list which reads terminal rows by task ID + // without resolving an execution at all. Nil = no scoping. + taskAccessCheck func(ctx context.Context, taskID string) error + + // taskEnvironmentAccessCheck authorizes a (task, environment) pair for + // surfaces that merge state keyed by both, where authorizing each ID on + // its own would not establish that they belong together. Nil = no scoping. + taskEnvironmentAccessCheck func(ctx context.Context, taskID, environmentID string) error + // singleflight deduplicates concurrent GetOrEnsureExecution calls for the same session ensureExecutionGroup singleflight.Group @@ -441,6 +451,20 @@ func (m *Manager) SetEnvironmentAccessChecker(check func(ctx context.Context, en m.environmentAccessCheck = check } +// SetTaskAccessChecker installs the per-user task visibility check used by +// the task-keyed SSR terminal route. The checker must return nil for contexts +// without a request identity (internal callers). +func (m *Manager) SetTaskAccessChecker(check func(ctx context.Context, taskID string) error) { + m.taskAccessCheck = check +} + +// SetTaskEnvironmentAccessChecker installs the per-user check for a +// (task, environment) pair, used by the task-keyed SSR terminal route which +// merges terminals from the task with unmanaged shells from the environment. +func (m *Manager) SetTaskEnvironmentAccessChecker(check func(ctx context.Context, taskID, environmentID string) error) { + m.taskEnvironmentAccessCheck = check +} + // CheckSessionAccess authorizes a session-scoped operation for the ctx // identity. Handlers that resolve an execution by a bare in-memory lookup // (vscode/port reverse proxies) must call this before serving, since only the @@ -464,6 +488,29 @@ func (m *Manager) CheckEnvironmentAccess(ctx context.Context, taskEnvironmentID return m.environmentAccessCheck(ctx, taskEnvironmentID) } +// CheckTaskAccess authorizes a task-scoped operation for the ctx identity. +// The task-keyed sibling of CheckSessionAccess, for handlers that read +// task-owned state (the SSR terminal list) without going through an +// execution. No-op when no checker is set. +func (m *Manager) CheckTaskAccess(ctx context.Context, taskID string) error { + if m.taskAccessCheck == nil { + return nil + } + return m.taskAccessCheck(ctx, taskID) +} + +// CheckTaskEnvironmentAccess authorizes a (task, environment) pair for the ctx +// identity: both IDs visible, and the environment actually bound to the task. +// Handlers that merge state keyed by both must use this rather than the two +// single-ID checks, which pass independently for an unrelated pair. No-op when +// no checker is set. +func (m *Manager) CheckTaskEnvironmentAccess(ctx context.Context, taskID, taskEnvironmentID string) error { + if m.taskEnvironmentAccessCheck == nil { + return nil + } + return m.taskEnvironmentAccessCheck(ctx, taskID, taskEnvironmentID) +} + // SetWorkspaceInfoProvider sets the provider for workspace information. // // The WorkspaceInfoProvider interface allows the lifecycle manager to dynamically create diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_access_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_access_test.go index 41f5410c90..11020a717a 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_access_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_access_test.go @@ -52,4 +52,23 @@ func TestExecutionAccessChecksGateBeforeCache(t *testing.T) { t.Fatalf("nil checker must pass: %v", err) } }) + + t.Run("CheckTaskAccess delegates to the installed checker", func(t *testing.T) { + mgr := &Manager{logger: newTestLogger(), executionStore: NewExecutionStore()} + if err := mgr.CheckTaskAccess(context.Background(), "task-x"); err != nil { + t.Fatalf("nil checker must pass: %v", err) + } + mgr.SetTaskAccessChecker(func(_ context.Context, taskID string) error { + if taskID == "task-1" { + return denied + } + return nil + }) + if err := mgr.CheckTaskAccess(context.Background(), "task-1"); !errors.Is(err, denied) { + t.Fatalf("expected denial, got %v", err) + } + if err := mgr.CheckTaskAccess(context.Background(), "task-2"); err != nil { + t.Fatalf("unexpected denial: %v", err) + } + }) } diff --git a/apps/backend/internal/backendapp/helpers.go b/apps/backend/internal/backendapp/helpers.go index bef031eadd..feb23e9996 100644 --- a/apps/backend/internal/backendapp/helpers.go +++ b/apps/backend/internal/backendapp/helpers.go @@ -1451,6 +1451,29 @@ func dockerSessionAuthorizer(taskSvc *taskservice.Service) docker.SessionAuthori return taskSvc } +// lifecycleAccessAuthorizer is the task-service surface the lifecycle manager's +// per-user visibility checks are wired to. Narrowed to an interface so the +// wiring itself is testable: the three single-ID checkers take the same +// signature, so a crossed wire (session visibility installed in the task slot, +// say) compiles and silently authorizes the wrong resource. +type lifecycleAccessAuthorizer interface { + AuthorizeSessionAccess(ctx context.Context, sessionID string) error + AuthorizeEnvironmentAccess(ctx context.Context, taskEnvironmentID string) error + AuthorizeTaskAccess(ctx context.Context, taskID string) error + AuthorizeTaskEnvironmentAccess(ctx context.Context, taskID, taskEnvironmentID string) error +} + +// wireLifecycleAccessCheckers installs every per-user visibility check on the +// lifecycle manager. Kept together so a surface that needs a new kind of check +// has one place to add it, rather than another setter call somewhere else in +// startup that nothing asserts on. +func wireLifecycleAccessCheckers(lifecycleMgr *lifecycle.Manager, authz lifecycleAccessAuthorizer) { + lifecycleMgr.SetSessionAccessChecker(authz.AuthorizeSessionAccess) + lifecycleMgr.SetEnvironmentAccessChecker(authz.AuthorizeEnvironmentAccess) + lifecycleMgr.SetTaskAccessChecker(authz.AuthorizeTaskAccess) + lifecycleMgr.SetTaskEnvironmentAccessChecker(authz.AuthorizeTaskEnvironmentAccess) +} + func dockerTaskTitleProvider(taskRepo *sqliterepo.Repository, log *logger.Logger) docker.TaskTitleProvider { return func(ctx context.Context, taskID string) (string, bool) { if taskRepo == nil || taskID == "" { diff --git a/apps/backend/internal/backendapp/lifecycle_access_wiring_test.go b/apps/backend/internal/backendapp/lifecycle_access_wiring_test.go new file mode 100644 index 0000000000..c7ac91c646 --- /dev/null +++ b/apps/backend/internal/backendapp/lifecycle_access_wiring_test.go @@ -0,0 +1,70 @@ +package backendapp + +import ( + "context" + "testing" + + "github.com/kandev/kandev/internal/agent/runtime/lifecycle" +) + +// recordingAuthorizer records which authorizer the lifecycle manager reached +// for each kind of check. +type recordingAuthorizer struct{ calls []string } + +func (r *recordingAuthorizer) AuthorizeSessionAccess(_ context.Context, sessionID string) error { + r.calls = append(r.calls, "session:"+sessionID) + return nil +} + +func (r *recordingAuthorizer) AuthorizeEnvironmentAccess(_ context.Context, envID string) error { + r.calls = append(r.calls, "environment:"+envID) + return nil +} + +func (r *recordingAuthorizer) AuthorizeTaskAccess(_ context.Context, taskID string) error { + r.calls = append(r.calls, "task:"+taskID) + return nil +} + +func (r *recordingAuthorizer) AuthorizeTaskEnvironmentAccess(_ context.Context, taskID, envID string) error { + r.calls = append(r.calls, "pair:"+taskID+"/"+envID) + return nil +} + +// TestWireLifecycleAccessCheckers pins the hop between the task service and the +// lifecycle manager. The three single-ID checkers share the signature +// `func(context.Context, string) error`, so wiring session visibility into the +// task slot compiles cleanly and authorizes the wrong resource for every +// caller downstream (the SSR terminal routes, the vscode and port proxies). +// Checking each check reaches its own authorizer is the only thing that catches +// a crossed or dropped wire. +func TestWireLifecycleAccessCheckers(t *testing.T) { + authz := &recordingAuthorizer{} + mgr := &lifecycle.Manager{} + + wireLifecycleAccessCheckers(mgr, authz) + + ctx := context.Background() + if err := mgr.CheckSessionAccess(ctx, "sess-1"); err != nil { + t.Fatalf("CheckSessionAccess: %v", err) + } + if err := mgr.CheckEnvironmentAccess(ctx, "env-1"); err != nil { + t.Fatalf("CheckEnvironmentAccess: %v", err) + } + if err := mgr.CheckTaskAccess(ctx, "task-1"); err != nil { + t.Fatalf("CheckTaskAccess: %v", err) + } + if err := mgr.CheckTaskEnvironmentAccess(ctx, "task-1", "env-1"); err != nil { + t.Fatalf("CheckTaskEnvironmentAccess: %v", err) + } + + want := []string{"session:sess-1", "environment:env-1", "task:task-1", "pair:task-1/env-1"} + if len(authz.calls) != len(want) { + t.Fatalf("calls = %v, want %v", authz.calls, want) + } + for i, call := range want { + if authz.calls[i] != call { + t.Errorf("call %d = %q, want %q (crossed wire)", i, authz.calls[i], call) + } + } +} diff --git a/apps/backend/internal/backendapp/main.go b/apps/backend/internal/backendapp/main.go index 1cb08c7825..30c93db6a6 100644 --- a/apps/backend/internal/backendapp/main.go +++ b/apps/backend/internal/backendapp/main.go @@ -550,9 +550,9 @@ func startAgentInfrastructure( // LSP, terminals) enforce per-user workspace scoping (opt-in auth). The // GetOrEnsure* execution paths run these checks internally; the vscode and // port reverse proxies (bare lookup + cache) call CheckSessionAccess at - // the handler. - lifecycleMgr.SetSessionAccessChecker(services.Task.AuthorizeSessionAccess) - lifecycleMgr.SetEnvironmentAccessChecker(services.Task.AuthorizeEnvironmentAccess) + // the handler, and the SSR terminal-list routes call CheckTaskAccess / + // CheckEnvironmentAccess / CheckTaskEnvironmentAccess in a route guard. + wireLifecycleAccessCheckers(lifecycleMgr, services.Task) log.Info("Workspace info provider configured for session recovery") // TODO(task-model-unification Phase 2, ADR 0004): wire agentruntime.New(lifecycleMgr) diff --git a/apps/backend/internal/task/service/service_access.go b/apps/backend/internal/task/service/service_access.go index ea8a407201..f0a22d9380 100644 --- a/apps/backend/internal/task/service/service_access.go +++ b/apps/backend/internal/task/service/service_access.go @@ -185,6 +185,50 @@ func (s *Service) AuthorizeEnvironmentAccess(ctx context.Context, taskEnvironmen return s.authorizeTaskID(ctx, env.TaskID) } +// AuthorizeTaskEnvironmentAccess checks that both identifiers are visible to +// the caller and that the environment is one the task is actually bound to. +// Mismatches use the task not-found sentinel, mirroring +// AuthorizeTaskSessionAccess, so callers cannot enumerate environments by +// pairing them against a task they do own. +// +// Authorizing the two IDs separately is not enough: both checks pass for a +// caller who owns the task and, independently, owns some unrelated +// environment, and the terminal route then merges state from the two. +// +// The relationship is deliberately not `env.TaskID == taskID`. inherit_parent +// binds a subtask's session to the parent task's environment, and shared_group +// binds every member of a workspace group to one canonical environment, so the +// row's owning task is legitimately a different task in both. What establishes +// the pair is a session of this task pointing at this environment. +func (s *Service) AuthorizeTaskEnvironmentAccess(ctx context.Context, taskID, taskEnvironmentID string) error { + if _, scoped := callerScope(ctx); !scoped { + return nil + } + if err := s.AuthorizeTaskAccess(ctx, taskID); err != nil { + return err + } + if err := s.AuthorizeEnvironmentAccess(ctx, taskEnvironmentID); err != nil { + return err + } + env, err := s.taskEnvironments.GetTaskEnvironment(ctx, taskEnvironmentID) + if err != nil { + return err + } + if env != nil && env.TaskID == taskID { + return nil + } + sessions, err := s.sessions.ListTaskSessions(ctx, taskID) + if err != nil { + return err + } + for _, session := range sessions { + if session != nil && session.TaskEnvironmentID == taskEnvironmentID { + return nil + } + } + return repoerrors.ErrTaskNotFound +} + // authorizeRepositoryID checks visibility of a repository via its workspace. // Denials use ErrRepositoryNotFound (no existence leak). func (s *Service) authorizeRepositoryID(ctx context.Context, repositoryID string) error { diff --git a/apps/backend/internal/task/service/service_access_terminal_test.go b/apps/backend/internal/task/service/service_access_terminal_test.go new file mode 100644 index 0000000000..6cca96e4f7 --- /dev/null +++ b/apps/backend/internal/task/service/service_access_terminal_test.go @@ -0,0 +1,157 @@ +package service + +import ( + "context" + "errors" + "testing" + + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/repository/repoerrors" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// The SSR terminal-list routes (GET /api/v1/environments/:id/terminals and +// GET /api/v1/tasks/:id/terminals) call these two authorizers through the +// lifecycle manager's CheckTaskAccess / CheckEnvironmentAccess. Their guard +// only holds if the authorizers themselves deny a foreign scope and, just as +// importantly, no-op when auth is disabled: the route must behave exactly as +// it did pre-auth for the single-user install. +func TestTerminalRouteAuthorizersScopeByOwner(t *testing.T) { + ctx := context.Background() + svc, _, repo := createTestService(t) + seedScopedWorkspaces(t, repo) + // Deliberately a torn-down environment: stopped, with a materialization + // session that no longer exists. Authorization reads task and workspace + // rows, never the in-memory execution, so a dead agent must not turn into + // a denial for the owner (the SSR panel renders a denial as empty). + if err := repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{ + ID: "env-b", TaskID: "task-b", ExecutorType: "worktree", + WorkspacePath: "/tmp/b", Status: models.TaskEnvironmentStatusStopped, + MaterializationSessionID: "sess-deleted", + }); err != nil { + t.Fatalf("create task environment: %v", err) + } + + // Foreign caller: denied, with the sentinel that reads as "no such thing". + if err := svc.AuthorizeTaskAccess(ctxAs("user-a"), "task-b"); !errors.Is(err, repoerrors.ErrTaskNotFound) { + t.Errorf("foreign task access = %v, want ErrTaskNotFound", err) + } + if err := svc.AuthorizeEnvironmentAccess(ctxAs("user-a"), "env-b"); !errors.Is(err, repoerrors.ErrTaskNotFound) { + t.Errorf("foreign environment access = %v, want ErrTaskNotFound", err) + } + + // Owner: allowed. + if err := svc.AuthorizeTaskAccess(ctxAs("user-b"), "task-b"); err != nil { + t.Errorf("owner task access = %v, want nil", err) + } + if err := svc.AuthorizeEnvironmentAccess(ctxAs("user-b"), "env-b"); err != nil { + t.Errorf("owner environment access = %v, want nil", err) + } + + // Auth disabled (synthetic identity) and internal callers stay unscoped, + // so the guard is invisible on a single-user install. + for name, callerCtx := range map[string]context.Context{ + "internal": context.Background(), + "synthetic": ctxSynthetic(), + } { + t.Run(name, func(t *testing.T) { + if err := svc.AuthorizeTaskAccess(callerCtx, "task-b"); err != nil { + t.Errorf("task access = %v, want nil", err) + } + if err := svc.AuthorizeEnvironmentAccess(callerCtx, "env-b"); err != nil { + t.Errorf("environment access = %v, want nil", err) + } + }) + } +} + +// TestAuthorizeTaskEnvironmentAccessPairsBinding covers the pairing the two +// independent checks miss: authorizing a task and authorizing an environment +// separately does not establish that they belong together, so a caller could +// combine the ordinary terminals of one of their tasks with the unmanaged +// shells of an unrelated environment. +// +// The relationship checked is deliberately NOT `env.TaskID == taskID`. Two +// shipping workspace modes bind a task's session to an environment row owned +// by a different task: inherit_parent gives a subtask the parent's environment +// (orchestrator.resolveInheritedEnvironment) and shared_group gives every +// member of the group one canonical environment +// (orchestrator.inheritFromSharedGroup). The frontend sends the session's +// environment alongside the task ID, so an owner-of-the-row check would 404 +// the terminal panel for both. What matters is whether a session of this task +// actually points at this environment. +func TestAuthorizeTaskEnvironmentAccessPairsBinding(t *testing.T) { + ctx := context.Background() + svc, _, repo := createTestService(t) + seedScopedWorkspaces(t, repo) + + // task-b owns env-b; sess-b is bound to it. + if err := repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{ + ID: "env-b", TaskID: "task-b", ExecutorType: "worktree", + WorkspacePath: "/tmp/b", Status: models.TaskEnvironmentStatusReady, + }); err != nil { + t.Fatalf("create env-b: %v", err) + } + + // task-c is a second task of the SAME owner, with its own environment. + // Nothing links it to task-b: this is the pair that must be refused. + if err := repo.CreateTask(ctx, &models.Task{ + ID: "task-c", WorkspaceID: "ws-b", WorkflowID: "wf-b", WorkflowStepID: "step-1", + Title: "B's other task", State: v1.TaskStateCreated, Priority: "medium", + }); err != nil { + t.Fatalf("create task-c: %v", err) + } + if err := repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{ + ID: "env-c", TaskID: "task-c", ExecutorType: "worktree", + WorkspacePath: "/tmp/c", Status: models.TaskEnvironmentStatusReady, + }); err != nil { + t.Fatalf("create env-c: %v", err) + } + + // task-d owns no environment; its session is bound to task-b's, the shape + // inherit_parent and shared_group produce. + if err := repo.CreateTask(ctx, &models.Task{ + ID: "task-d", WorkspaceID: "ws-b", WorkflowID: "wf-b", WorkflowStepID: "step-1", + Title: "B's subtask", State: v1.TaskStateCreated, Priority: "medium", + }); err != nil { + t.Fatalf("create task-d: %v", err) + } + if err := repo.CreateTaskSession(ctx, &models.TaskSession{ + ID: "sess-d", TaskID: "task-d", State: models.TaskSessionStateCreated, + TaskEnvironmentID: "env-b", + }); err != nil { + t.Fatalf("create sess-d: %v", err) + } + + owner := ctxAs("user-b") + if err := svc.AuthorizeTaskEnvironmentAccess(owner, "task-b", "env-b"); err != nil { + t.Errorf("task with its own environment = %v, want nil", err) + } + if err := svc.AuthorizeTaskEnvironmentAccess(owner, "task-d", "env-b"); err != nil { + t.Errorf("inherited environment = %v, want nil", err) + } + if err := svc.AuthorizeTaskEnvironmentAccess(owner, "task-b", "env-c"); !errors.Is(err, repoerrors.ErrTaskNotFound) { + t.Errorf("unrelated environment of the same owner = %v, want ErrTaskNotFound", err) + } + if err := svc.AuthorizeTaskEnvironmentAccess(owner, "task-d", "env-c"); !errors.Is(err, repoerrors.ErrTaskNotFound) { + t.Errorf("unrelated environment for a subtask = %v, want ErrTaskNotFound", err) + } + + // A foreign caller is refused on the pair as well, with the same sentinel. + if err := svc.AuthorizeTaskEnvironmentAccess(ctxAs("user-a"), "task-b", "env-b"); !errors.Is(err, repoerrors.ErrTaskNotFound) { + t.Errorf("foreign pair = %v, want ErrTaskNotFound", err) + } + + // Auth disabled: the pairing is an authorization check, so it must not + // start rejecting combinations the single-user install served before. + for name, callerCtx := range map[string]context.Context{ + "internal": context.Background(), + "synthetic": ctxSynthetic(), + } { + t.Run(name, func(t *testing.T) { + if err := svc.AuthorizeTaskEnvironmentAccess(callerCtx, "task-b", "env-c"); err != nil { + t.Errorf("unscoped pair = %v, want nil", err) + } + }) + } +}