diff --git a/.github/workflows/eval-smoke.yaml b/.github/workflows/eval-smoke.yaml index 7f2a0922..42818808 100644 --- a/.github/workflows/eval-smoke.yaml +++ b/.github/workflows/eval-smoke.yaml @@ -203,6 +203,10 @@ jobs: port: 2480 base_url: "http://127.0.0.1:2480" sandbox_url: "http://127.0.0.1:2480" + # This server runs unauthenticated; concurrent eval workers each tear down + # only their own sandbox sessions between tests, scoped by the + # X-Panda-On-Behalf-Of header the agent sets per worker. + attribution_session_scoping: true sandbox: backend: docker diff --git a/config.example.yaml b/config.example.yaml index 25c8f9b7..e5fc7cfa 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -8,6 +8,10 @@ server: base_url: "http://localhost:2480" # ep clients should point at this URL sandbox_url: "http://ethpandaops-panda-server:2480" # URL sandbox containers use to call the local server # uploads: false # opt out of `panda upload` (upload/publish API + /u/ preview pages) + # attribution_session_scoping: true # scope unauthenticated sandbox sessions by the + # caller-supplied X-Panda-On-Behalf-Of header. That header is untrusted and + # spoofable by any caller that can reach this server, so leave this off unless + # the deployment already accepts that tradeoff (e.g. an automated test harness). # Sandbox configuration sandbox: diff --git a/pkg/config/config.go b/pkg/config/config.go index 53738af5..3f2af637 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -79,6 +79,17 @@ type ServerConfig struct { // Nothing leaves the machine without an explicit publish either way. Uploads *bool `yaml:"uploads,omitempty"` + // AttributionSessionScoping opts into using the caller-supplied + // X-Panda-On-Behalf-Of attribution value as a sandbox session ownership + // key when the server has no authenticated user for a request. That value + // is untrusted and audit-only by design (see pkg/attribution): any caller + // that can reach the server can set it to any value, including someone + // else's. Leave this off (the default) unless the deployment already + // accepts that tradeoff, such as an automated eval harness isolating its + // own worker sessions. Off, unauthenticated requests get no ownership + // scoping at all. + AttributionSessionScoping bool `yaml:"attribution_session_scoping,omitempty"` + // Deprecated: Transport is accepted for backwards compatibility but ignored. // The server always runs HTTP with both SSE and streamable-http transports. Transport string `yaml:"transport,omitempty"` diff --git a/pkg/server/api.go b/pkg/server/api.go index 7e9b138b..530d7721 100644 --- a/pkg/server/api.go +++ b/pkg/server/api.go @@ -296,7 +296,7 @@ func (s *service) handleAPIExecute(w http.ResponseWriter, r *http.Request) { return } - ownerID := authOwnerID(r) + ownerID := s.authOwnerID(r) result, err := s.execService.Execute(r.Context(), execsvc.ExecuteRequest{ Code: req.Code, Timeout: req.Timeout, @@ -337,7 +337,7 @@ func (s *service) handleAPIListSessions(w http.ResponseWriter, r *http.Request) return } - sessions, maxSessions, err := s.execService.ListSessions(r.Context(), authOwnerID(r)) + sessions, maxSessions, err := s.execService.ListSessions(r.Context(), s.authOwnerID(r)) if err != nil { writeAPIError(w, http.StatusBadRequest, err.Error()) return @@ -372,7 +372,7 @@ func (s *service) handleAPICreateSession(w http.ResponseWriter, r *http.Request) return } - ownerID := authOwnerID(r) + ownerID := s.authOwnerID(r) sessionID, err := s.execService.CreateSession(r.Context(), ownerID) if err != nil { writeAPIError(w, http.StatusBadRequest, err.Error()) @@ -404,7 +404,7 @@ func (s *service) handleAPIDestroySession(w http.ResponseWriter, r *http.Request return } - if err := s.execService.DestroySession(r.Context(), sessionID, authOwnerID(r)); err != nil { + if err := s.execService.DestroySession(r.Context(), sessionID, s.authOwnerID(r)); err != nil { writeAPIError(w, http.StatusBadRequest, err.Error()) return } @@ -940,14 +940,20 @@ func runtimeStorageScope(r *http.Request, executionID string) storage.Scope { } } -func authOwnerID(r *http.Request) string { +func (s *service) authOwnerID(r *http.Request) string { if user := auth.GetAuthUser(r.Context()); user != nil { return fmt.Sprintf("%d", user.GitHubID) } - // Fall back to caller attribution so sessions are owner-scoped even when - // the server runs unauthenticated (e.g. local mode, the eval harness). - // Empty when no attribution is present, preserving prior behavior. + // Caller attribution is untrusted and audit-only (pkg/attribution): any + // caller that can reach this server can set it to any value, including + // someone else's. Only use it as a session-ownership key when the + // operator has explicitly opted in; otherwise an unauthenticated request + // gets no ownership scoping at all. + if !s.cfg.AttributionSessionScoping { + return "" + } + return attribution.FromContext(r.Context()) } diff --git a/pkg/server/runtime_attribution_test.go b/pkg/server/runtime_attribution_test.go index 18fd3e33..f08dfe37 100644 --- a/pkg/server/runtime_attribution_test.go +++ b/pkg/server/runtime_attribution_test.go @@ -94,14 +94,31 @@ func TestRuntimeCallbacksInheritExecutionAttribution(t *testing.T) { "runtime callbacks must inherit the spawning execution's attribution") } -func TestAuthOwnerIDAttributionFallback(t *testing.T) { - // No auth user: fall back to the caller attribution so sessions are owner-scoped - // even on an unauthenticated server (local mode, the eval harness). +func TestAuthOwnerIDIgnoresAttributionByDefault(t *testing.T) { + // Attribution is untrusted and audit-only. By default, an unauthenticated + // request with no auth user gets no ownership scoping at all, even if a + // caller-supplied attribution value is present, since any other caller + // could set the same value. + svc := &service{} + + withAttr := httptest.NewRequest(http.MethodGet, "/api/v1/sessions", nil) + withAttr = withAttr.WithContext(attribution.WithValue(withAttr.Context(), "alice")) + require.Equal(t, "", svc.authOwnerID(withAttr)) + + bare := httptest.NewRequest(http.MethodGet, "/api/v1/sessions", nil) + require.Equal(t, "", svc.authOwnerID(bare)) +} + +func TestAuthOwnerIDUsesAttributionWhenExplicitlyEnabled(t *testing.T) { + // An operator that explicitly opts into attribution-based session scoping + // (e.g. an automated eval harness isolating its own worker sessions) gets + // the caller attribution value as the owner id. + svc := &service{cfg: config.ServerConfig{AttributionSessionScoping: true}} + withAttr := httptest.NewRequest(http.MethodGet, "/api/v1/sessions", nil) withAttr = withAttr.WithContext(attribution.WithValue(withAttr.Context(), "eval-worker-abc123")) - require.Equal(t, "eval-worker-abc123", authOwnerID(withAttr)) + require.Equal(t, "eval-worker-abc123", svc.authOwnerID(withAttr)) - // Neither auth user nor attribution: empty owner, preserving prior behavior. bare := httptest.NewRequest(http.MethodGet, "/api/v1/sessions", nil) - require.Equal(t, "", authOwnerID(bare)) + require.Equal(t, "", svc.authOwnerID(bare)) } diff --git a/tests/eval/scripts/_panda_env.py b/tests/eval/scripts/_panda_env.py index 5a7727e3..7daad3cd 100644 --- a/tests/eval/scripts/_panda_env.py +++ b/tests/eval/scripts/_panda_env.py @@ -50,6 +50,9 @@ def write_scratch_config(port: int, *, source: Path | None = None) -> Path: cfg["server"]["port"] = port cfg["server"]["base_url"] = f"http://localhost:{port}" cfg["server"]["sandbox_url"] = f"http://host.docker.internal:{port}" + # This scratch server runs unauthenticated; the per-test owner-scoped cleanup + # below relies on the X-Panda-On-Behalf-Of header actually scoping sessions. + cfg["server"]["attribution_session_scoping"] = True sb = cfg.setdefault("sandbox", {}) sb["image"] = "ethpandaops-panda-sandbox:latest" sb["network"] = "ethpandaops-panda-harden"