diff --git a/apps/backend/internal/agent/docker/handlers.go b/apps/backend/internal/agent/docker/handlers.go index a169334333..c55944080e 100644 --- a/apps/backend/internal/agent/docker/handlers.go +++ b/apps/backend/internal/agent/docker/handlers.go @@ -4,10 +4,13 @@ package docker import ( "bufio" "context" + "errors" + "io" "net/http" "time" "github.com/gin-gonic/gin" + "github.com/kandev/kandev/internal/auth/authn" "github.com/kandev/kandev/internal/common/logger" "go.uber.org/zap" ) @@ -38,22 +41,85 @@ type stopContainerRequest struct { // ClientProvider lazily resolves the Docker client. Returns nil if Docker is unavailable. type ClientProvider func() *Client +// containerAPI is the Docker surface these handlers use. *Client satisfies it; +// tests inject a fake so handler behavior can be exercised without a daemon. +type containerAPI interface { + BuildImage(ctx context.Context, dockerfile string, tag string, buildArgs map[string]*string) (io.ReadCloser, error) + ListContainers(ctx context.Context, labels map[string]string) ([]ContainerInfo, error) + GetContainerLabels(ctx context.Context, containerID string) (map[string]string, error) + StopContainer(ctx context.Context, containerID string, timeout time.Duration) error + RemoveContainer(ctx context.Context, containerID string, force bool) error +} + +var _ containerAPI = (*Client)(nil) + +// clientResolver resolves the Docker surface per request, returning nil when +// Docker is unavailable. +type clientResolver func() containerAPI + // TaskTitleProvider resolves a task ID to its display title for container listings. type TaskTitleProvider func(ctx context.Context, taskID string) (string, bool) +// SessionAuthorizer scopes container operations to the task session that owns +// the container. Kandev stamps every managed container with its owning task +// and session (internal/agent/runtime/lifecycle/container.go), so the labels +// are the ownership record these checks key off. +// +// Both methods follow task/service.Service's contract: no identity in ctx +// (internal callers) or a synthetic identity (authentication disabled) is +// unscoped, and denials come back as not-found sentinels so a foreign resource +// is indistinguishable from a missing one. +type SessionAuthorizer interface { + AuthorizeSessionAccess(ctx context.Context, sessionID string) error + AuthorizeTaskAccess(ctx context.Context, taskID string) error +} + +// Labels stamped on every Kandev-managed container. +const ( + sessionIDLabel = "kandev.session_id" + taskIDLabel = "kandev.task_id" + taskTitleLabel = "kandev.task_title" +) + +// errContainerUnowned marks a container whose owning session or task cannot be +// determined from its labels. Scoped callers never see such a container. +var errContainerUnowned = errors.New("container has no resolvable owner") + // RegisterDockerRoutes registers Docker management HTTP routes on the given router. -// The clientProvider lazily resolves the Docker client on each request. -func RegisterDockerRoutes(router *gin.Engine, clientProvider ClientProvider, taskTitleProvider TaskTitleProvider, log *logger.Logger) { +// The clientProvider lazily resolves the Docker client on each request, and the +// authorizer scopes container access to the caller's own task sessions. +func RegisterDockerRoutes( + router *gin.Engine, clientProvider ClientProvider, + taskTitleProvider TaskTitleProvider, authorizer SessionAuthorizer, log *logger.Logger, +) { + resolve := func() containerAPI { + client := clientProvider() + if client == nil { + return nil + } + return client + } + registerRoutes(router, resolve, taskTitleProvider, authorizer, log) +} + +// registerRoutes is the testable form of RegisterDockerRoutes, taking the +// Docker surface as an interface instead of the concrete client. +func registerRoutes( + router *gin.Engine, resolve clientResolver, + taskTitleProvider TaskTitleProvider, authorizer SessionAuthorizer, log *logger.Logger, +) { api := router.Group("/api/v1/docker") - api.POST("/build", handleBuildImage(clientProvider, log)) - api.GET("/containers", handleListContainers(clientProvider, taskTitleProvider, log)) - api.POST("/containers/:id/stop", handleStopContainer(clientProvider, log)) - api.DELETE("/containers/:id", handleRemoveContainer(clientProvider, log)) + // Building an image is a host-level operation with no per-user resource, + // so it is admin-only like the other install-wide mutations. + api.POST("/build", authn.RequireAdmin(), handleBuildImage(resolve, log)) + api.GET("/containers", handleListContainers(resolve, taskTitleProvider, authorizer, log)) + api.POST("/containers/:id/stop", handleStopContainer(resolve, authorizer, log)) + api.DELETE("/containers/:id", handleRemoveContainer(resolve, authorizer, log)) } // requireDocker resolves the Docker client and returns 503 if unavailable. -func requireDocker(c *gin.Context, clientProvider ClientProvider) *Client { - client := clientProvider() +func requireDocker(c *gin.Context, resolve clientResolver) containerAPI { + client := resolve() if client == nil { c.JSON(http.StatusServiceUnavailable, gin.H{"error": "Docker is not available"}) return nil @@ -61,11 +127,85 @@ func requireDocker(c *gin.Context, clientProvider ClientProvider) *Client { return client } +// callerScoped mirrors task/service's callerScope: a missing identity +// (internal callers) or the synthetic identity injected while authentication +// is disabled means no scoping at all, preserving single-user behavior. +func callerScoped(ctx context.Context) bool { + identity, ok := authn.IdentityFromContext(ctx) + return ok && !identity.Synthetic +} + +// authorizeContainerLabels reports whether the caller may see the container +// described by labels. A container with no owning session or task, or with no +// authorizer wired, is denied rather than exposed. +// +// The session label goes stale when a session is rolled back or removed while +// its task and container live on, so a failed session check falls back to the +// task label — the container GC treats kandev.task_id as the durable owner for +// the same reason. The fallback cannot widen access: both labels are stamped +// from one launch config, and AuthorizeSessionAccess resolves through the +// task's workspace anyway, so it admits nobody AuthorizeTaskAccess would deny. +func authorizeContainerLabels(ctx context.Context, labels map[string]string, authorizer SessionAuthorizer) error { + if authorizer == nil { + return errContainerUnowned + } + sessionID, taskID := labels[sessionIDLabel], labels[taskIDLabel] + if sessionID != "" { + err := authorizer.AuthorizeSessionAccess(ctx, sessionID) + if err == nil || taskID == "" { + return err + } + } + if taskID != "" { + return authorizer.AuthorizeTaskAccess(ctx, taskID) + } + return errContainerUnowned +} + +// visibleContainers drops every container the caller may not see. It runs +// before any task title is resolved, so a foreign task's title is never read. +func visibleContainers(ctx context.Context, containers []ContainerInfo, authorizer SessionAuthorizer) []ContainerInfo { + if !callerScoped(ctx) { + return containers + } + visible := make([]ContainerInfo, 0, len(containers)) + for _, ctr := range containers { + if authorizeContainerLabels(ctx, ctr.Labels, authorizer) == nil { + visible = append(visible, ctr) + } + } + return visible +} + +// authorizeContainerID resolves a container's owner from its labels and +// authorizes the caller. It writes 404 and returns false for a denied +// container, an unknown container, and an unresolvable owner alike, so none of +// the three can be told apart. +func authorizeContainerID( + c *gin.Context, client containerAPI, authorizer SessionAuthorizer, + containerID string, log *logger.Logger, +) bool { + ctx := c.Request.Context() + if !callerScoped(ctx) { + return true + } + labels, err := client.GetContainerLabels(ctx, containerID) + if err == nil { + err = authorizeContainerLabels(ctx, labels, authorizer) + } + if err != nil { + log.Debug("Denied container access", zap.String("id", containerID), zap.Error(err)) + c.JSON(http.StatusNotFound, gin.H{"error": "container not found"}) + return false + } + return true +} + // handleBuildImage handles POST /api/v1/docker/build. // It streams the Docker build output as JSON lines to the client. -func handleBuildImage(clientProvider ClientProvider, log *logger.Logger) gin.HandlerFunc { +func handleBuildImage(resolve clientResolver, log *logger.Logger) gin.HandlerFunc { return func(c *gin.Context) { - dockerClient := requireDocker(c, clientProvider) + dockerClient := requireDocker(c, resolve) if dockerClient == nil { return } @@ -118,9 +258,12 @@ func streamBuildOutput(c *gin.Context, reader interface{ Read([]byte) (int, erro // handleListContainers handles GET /api/v1/docker/containers. // Supports optional query params: image, labels (comma-separated key=value pairs). -func handleListContainers(clientProvider ClientProvider, taskTitleProvider TaskTitleProvider, log *logger.Logger) gin.HandlerFunc { +func handleListContainers( + resolve clientResolver, taskTitleProvider TaskTitleProvider, + authorizer SessionAuthorizer, log *logger.Logger, +) gin.HandlerFunc { return func(c *gin.Context) { - dockerClient := requireDocker(c, clientProvider) + dockerClient := requireDocker(c, resolve) if dockerClient == nil { return } @@ -135,7 +278,8 @@ func handleListContainers(clientProvider ClientProvider, taskTitleProvider TaskT return } - resp := newContainerResponsesWithTaskTitles(c.Request.Context(), containers, taskTitleProvider) + visible := visibleContainers(c.Request.Context(), containers, authorizer) + resp := newContainerResponsesWithTaskTitles(c.Request.Context(), visible, taskTitleProvider) c.JSON(http.StatusOK, gin.H{"containers": resp}) } @@ -169,12 +313,12 @@ func labelsWithTaskTitle(ctx context.Context, labels map[string]string, taskTitl for key, value := range labels { result[key] = value } - if result["kandev.task_title"] != "" || taskTitleProvider == nil { + if result[taskTitleLabel] != "" || taskTitleProvider == nil { return result } - title, ok := taskTitleProvider(ctx, result["kandev.task_id"]) + title, ok := taskTitleProvider(ctx, result[taskIDLabel]) if ok && title != "" { - result["kandev.task_title"] = title + result[taskTitleLabel] = title } return result } @@ -226,9 +370,9 @@ func addImageFilter(c *gin.Context, labels map[string]string) { } // handleStopContainer handles POST /api/v1/docker/containers/:id/stop. -func handleStopContainer(clientProvider ClientProvider, log *logger.Logger) gin.HandlerFunc { +func handleStopContainer(resolve clientResolver, authorizer SessionAuthorizer, log *logger.Logger) gin.HandlerFunc { return func(c *gin.Context) { - dockerClient := requireDocker(c, clientProvider) + dockerClient := requireDocker(c, resolve) if dockerClient == nil { return } @@ -239,6 +383,10 @@ func handleStopContainer(clientProvider ClientProvider, log *logger.Logger) gin. return } + if !authorizeContainerID(c, dockerClient, authorizer, containerID, log) { + return + } + var req stopContainerRequest // Bind is optional; ignore errors for empty body _ = c.ShouldBindJSON(&req) @@ -259,9 +407,9 @@ func handleStopContainer(clientProvider ClientProvider, log *logger.Logger) gin. } // handleRemoveContainer handles DELETE /api/v1/docker/containers/:id. -func handleRemoveContainer(clientProvider ClientProvider, log *logger.Logger) gin.HandlerFunc { +func handleRemoveContainer(resolve clientResolver, authorizer SessionAuthorizer, log *logger.Logger) gin.HandlerFunc { return func(c *gin.Context) { - dockerClient := requireDocker(c, clientProvider) + dockerClient := requireDocker(c, resolve) if dockerClient == nil { return } @@ -272,6 +420,10 @@ func handleRemoveContainer(clientProvider ClientProvider, log *logger.Logger) gi return } + if !authorizeContainerID(c, dockerClient, authorizer, containerID, log) { + return + } + if err := dockerClient.RemoveContainer(c.Request.Context(), containerID, true); err != nil { log.Error("Failed to remove container", zap.String("id", containerID), zap.Error(err)) c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) diff --git a/apps/backend/internal/agent/docker/handlers_auth_test.go b/apps/backend/internal/agent/docker/handlers_auth_test.go new file mode 100644 index 0000000000..6a843f1cde --- /dev/null +++ b/apps/backend/internal/agent/docker/handlers_auth_test.go @@ -0,0 +1,488 @@ +package docker + +import ( + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/kandev/kandev/internal/auth/authn" + "github.com/kandev/kandev/internal/common/logger" + "go.uber.org/zap" +) + +// fakeDockerAPI stands in for *Client so handler tests need no daemon. +type fakeDockerAPI struct { + containers []ContainerInfo + listErr error + + listCalls int + inspected []string + stopped []string + removed []string + builtTags []string +} + +func (f *fakeDockerAPI) find(containerID string) (ContainerInfo, bool) { + for _, ctr := range f.containers { + if ctr.ID == containerID { + return ctr, true + } + } + return ContainerInfo{}, false +} + +func (f *fakeDockerAPI) BuildImage(_ context.Context, _ string, tag string, _ map[string]*string) (io.ReadCloser, error) { + f.builtTags = append(f.builtTags, tag) + return io.NopCloser(strings.NewReader(`{"stream":"built"}`)), nil +} + +func (f *fakeDockerAPI) ListContainers(_ context.Context, _ map[string]string) ([]ContainerInfo, error) { + f.listCalls++ + if f.listErr != nil { + return nil, f.listErr + } + return f.containers, nil +} + +func (f *fakeDockerAPI) GetContainerLabels(_ context.Context, containerID string) (map[string]string, error) { + f.inspected = append(f.inspected, containerID) + ctr, ok := f.find(containerID) + if !ok { + return nil, errors.New("no such container: " + containerID) + } + return ctr.Labels, nil +} + +func (f *fakeDockerAPI) StopContainer(_ context.Context, containerID string, _ time.Duration) error { + if _, ok := f.find(containerID); !ok { + return errors.New("no such container: " + containerID) + } + f.stopped = append(f.stopped, containerID) + return nil +} + +func (f *fakeDockerAPI) RemoveContainer(_ context.Context, containerID string, _ bool) error { + if _, ok := f.find(containerID); !ok { + return errors.New("no such container: " + containerID) + } + f.removed = append(f.removed, containerID) + return nil +} + +// fakeAuthorizer mirrors task/service's scoping contract: no identity or a +// synthetic identity (auth disabled) is unscoped, a real identity sees only +// the sessions and tasks it owns, and denials are indistinguishable from +// missing rows. +type fakeAuthorizer struct { + sessionOwners map[string]string + taskOwners map[string]string +} + +var errFakeNotFound = errors.New("not found") + +func (f *fakeAuthorizer) scope(ctx context.Context) (string, bool) { + identity, ok := authn.IdentityFromContext(ctx) + if !ok || identity.Synthetic { + return "", false + } + return identity.UserID, true +} + +func (f *fakeAuthorizer) AuthorizeSessionAccess(ctx context.Context, sessionID string) error { + userID, scoped := f.scope(ctx) + if !scoped { + return nil + } + owner, ok := f.sessionOwners[sessionID] + if !ok || owner != userID { + return errFakeNotFound + } + return nil +} + +func (f *fakeAuthorizer) AuthorizeTaskAccess(ctx context.Context, taskID string) error { + userID, scoped := f.scope(ctx) + if !scoped { + return nil + } + owner, ok := f.taskOwners[taskID] + if !ok || owner != userID { + return errFakeNotFound + } + return nil +} + +// titleRecorder records every task ID whose title was resolved, so tests can +// prove a foreign container's title is never looked up (filtering after +// resolution would still leak titles into logs and metrics). +type titleRecorder struct { + titles map[string]string + queried []string +} + +func (r *titleRecorder) provider() TaskTitleProvider { + return func(_ context.Context, taskID string) (string, bool) { + r.queried = append(r.queried, taskID) + title, ok := r.titles[taskID] + return title, ok + } +} + +func (r *titleRecorder) queriedTask(taskID string) bool { + for _, seen := range r.queried { + if seen == taskID { + return true + } + } + return false +} + +func kandevContainer(id, taskID, sessionID string) ContainerInfo { + return ContainerInfo{ + ID: id, + Name: "kandev-agent-" + id, + Image: "kandev/agent:test", + State: "running", + Status: "Up 2 seconds", + Labels: map[string]string{ + "kandev.managed": "true", + "kandev.task_id": taskID, + "kandev.session_id": sessionID, + }, + } +} + +func testLogger(t *testing.T) *logger.Logger { + t.Helper() + log, err := logger.NewFromZap(zap.NewNop()) + if err != nil { + t.Fatalf("logger: %v", err) + } + return log +} + +// dockerTestRouter registers the Docker routes against fakes. A nil identity +// means no auth middleware ran at all. +func dockerTestRouter( + t *testing.T, client containerAPI, titles TaskTitleProvider, + authorizer SessionAuthorizer, identity *authn.Identity, +) *gin.Engine { + t.Helper() + gin.SetMode(gin.TestMode) + router := gin.New() + if identity != nil { + router.Use(func(c *gin.Context) { + authn.SetOnGin(c, *identity) + c.Next() + }) + } + registerRoutes(router, func() containerAPI { return client }, titles, authorizer, testLogger(t)) + return router +} + +func member(userID string) *authn.Identity { + return &authn.Identity{UserID: userID, Role: authn.RoleMember} +} + +func do(router *gin.Engine, method, path string, body string) *httptest.ResponseRecorder { + var reader io.Reader + if body != "" { + reader = strings.NewReader(body) + } + request := httptest.NewRequest(method, path, reader) + if body != "" { + request.Header.Set("Content-Type", "application/json") + } + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + return response +} + +func twoUserFixture() (*fakeDockerAPI, *fakeAuthorizer, *titleRecorder) { + client := &fakeDockerAPI{containers: []ContainerInfo{ + kandevContainer("ctr-a", "task-a", "session-a"), + kandevContainer("ctr-b", "task-b", "session-b"), + }} + authorizer := &fakeAuthorizer{ + sessionOwners: map[string]string{"session-a": "user-a", "session-b": "user-b"}, + taskOwners: map[string]string{"task-a": "user-a", "task-b": "user-b"}, + } + titles := &titleRecorder{titles: map[string]string{"task-a": "Alice Task", "task-b": "Bob Task"}} + return client, authorizer, titles +} + +func TestListContainersHidesForeignContainersAndTitles(t *testing.T) { + client, authorizer, titles := twoUserFixture() + router := dockerTestRouter(t, client, titles.provider(), authorizer, member("user-b")) + + response := do(router, http.MethodGet, "/api/v1/docker/containers", "") + + if response.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", response.Code, response.Body.String()) + } + body := response.Body.String() + if strings.Contains(body, "ctr-a") || strings.Contains(body, "task-a") || strings.Contains(body, "Alice Task") { + t.Fatalf("foreign container leaked into listing: %s", body) + } + if !strings.Contains(body, "ctr-b") { + t.Fatalf("own container missing from listing: %s", body) + } + if titles.queriedTask("task-a") { + t.Fatalf("task title resolved for foreign container: %v", titles.queried) + } +} + +func TestListContainersDropsContainersWithoutResolvableOwner(t *testing.T) { + client, authorizer, titles := twoUserFixture() + client.containers = append(client.containers, ContainerInfo{ + ID: "ctr-unlabeled", Name: "stray", State: "running", + }) + router := dockerTestRouter(t, client, titles.provider(), authorizer, member("user-b")) + + response := do(router, http.MethodGet, "/api/v1/docker/containers", "") + + if strings.Contains(response.Body.String(), "ctr-unlabeled") { + t.Fatalf("unowned container leaked into listing: %s", response.Body.String()) + } +} + +func TestStopForeignContainerIsNotFoundAndDoesNotStop(t *testing.T) { + client, authorizer, titles := twoUserFixture() + router := dockerTestRouter(t, client, titles.provider(), authorizer, member("user-b")) + + response := do(router, http.MethodPost, "/api/v1/docker/containers/ctr-a/stop", "") + + if response.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404; body = %s", response.Code, response.Body.String()) + } + if len(client.stopped) != 0 { + t.Fatalf("docker StopContainer was invoked: %v", client.stopped) + } +} + +func TestRemoveForeignContainerIsNotFoundAndDoesNotRemove(t *testing.T) { + client, authorizer, titles := twoUserFixture() + router := dockerTestRouter(t, client, titles.provider(), authorizer, member("user-b")) + + response := do(router, http.MethodDelete, "/api/v1/docker/containers/ctr-a", "") + + if response.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404; body = %s", response.Code, response.Body.String()) + } + if len(client.removed) != 0 { + t.Fatalf("docker RemoveContainer was invoked: %v", client.removed) + } +} + +func TestForeignContainerIsIndistinguishableFromMissingContainer(t *testing.T) { + client, authorizer, titles := twoUserFixture() + router := dockerTestRouter(t, client, titles.provider(), authorizer, member("user-b")) + + cases := []struct{ method, foreign, missing string }{ + {http.MethodPost, "/api/v1/docker/containers/ctr-a/stop", "/api/v1/docker/containers/ctr-nope/stop"}, + {http.MethodDelete, "/api/v1/docker/containers/ctr-a", "/api/v1/docker/containers/ctr-nope"}, + } + for _, tc := range cases { + foreign := do(router, tc.method, tc.foreign, "") + missing := do(router, tc.method, tc.missing, "") + if foreign.Code != missing.Code { + t.Fatalf("%s: foreign status %d != missing status %d", tc.method, foreign.Code, missing.Code) + } + if foreign.Body.String() != missing.Body.String() { + t.Fatalf("%s: foreign body %q != missing body %q", tc.method, foreign.Body.String(), missing.Body.String()) + } + } +} + +func TestOwnerCanListStopAndRemoveOwnContainer(t *testing.T) { + client, authorizer, titles := twoUserFixture() + router := dockerTestRouter(t, client, titles.provider(), authorizer, member("user-a")) + + list := do(router, http.MethodGet, "/api/v1/docker/containers", "") + if list.Code != http.StatusOK || !strings.Contains(list.Body.String(), "Alice Task") { + t.Fatalf("owner listing = %d %s", list.Code, list.Body.String()) + } + + stop := do(router, http.MethodPost, "/api/v1/docker/containers/ctr-a/stop", `{"timeout_seconds":5}`) + if stop.Code != http.StatusOK { + t.Fatalf("owner stop status = %d, body = %s", stop.Code, stop.Body.String()) + } + if len(client.stopped) != 1 || client.stopped[0] != "ctr-a" { + t.Fatalf("stopped = %v, want [ctr-a]", client.stopped) + } + + remove := do(router, http.MethodDelete, "/api/v1/docker/containers/ctr-a", "") + if remove.Code != http.StatusOK { + t.Fatalf("owner remove status = %d, body = %s", remove.Code, remove.Body.String()) + } + if len(client.removed) != 1 || client.removed[0] != "ctr-a" { + t.Fatalf("removed = %v, want [ctr-a]", client.removed) + } +} + +func TestBuildImageIsAdminOnly(t *testing.T) { + client, authorizer, titles := twoUserFixture() + const body = `{"dockerfile":"FROM scratch","tag":"evil:latest"}` + + memberRouter := dockerTestRouter(t, client, titles.provider(), authorizer, member("user-b")) + memberResponse := do(memberRouter, http.MethodPost, "/api/v1/docker/build", body) + if memberResponse.Code != http.StatusForbidden { + t.Fatalf("member build status = %d, want 403; body = %s", memberResponse.Code, memberResponse.Body.String()) + } + if len(client.builtTags) != 0 { + t.Fatalf("member build reached Docker: %v", client.builtTags) + } + + adminIdentity := &authn.Identity{UserID: "admin-1", Role: authn.RoleAdmin} + adminRouter := dockerTestRouter(t, client, titles.provider(), authorizer, adminIdentity) + adminResponse := do(adminRouter, http.MethodPost, "/api/v1/docker/build", body) + if adminResponse.Code != http.StatusOK { + t.Fatalf("admin build status = %d, want 200; body = %s", adminResponse.Code, adminResponse.Body.String()) + } + if len(client.builtTags) != 1 || client.builtTags[0] != "evil:latest" { + t.Fatalf("builtTags = %v, want [evil:latest]", client.builtTags) + } +} + +// TestAuthDisabledLeavesEveryRouteUnchanged pins the single-user contract: the +// synthetic identity injected while authentication is disabled must behave +// exactly like the pre-fix handlers. +func TestAuthDisabledLeavesEveryRouteUnchanged(t *testing.T) { + client, authorizer, titles := twoUserFixture() + client.containers = append(client.containers, ContainerInfo{ID: "ctr-unlabeled", Name: "stray", State: "running"}) + synthetic := &authn.Identity{UserID: "single-user", Role: authn.RoleAdmin, Synthetic: true} + router := dockerTestRouter(t, client, titles.provider(), authorizer, synthetic) + + list := do(router, http.MethodGet, "/api/v1/docker/containers", "") + if list.Code != http.StatusOK { + t.Fatalf("list status = %d, body = %s", list.Code, list.Body.String()) + } + for _, want := range []string{"ctr-a", "ctr-b", "ctr-unlabeled", "Alice Task", "Bob Task"} { + if !strings.Contains(list.Body.String(), want) { + t.Fatalf("single-user listing missing %q: %s", want, list.Body.String()) + } + } + + stop := do(router, http.MethodPost, "/api/v1/docker/containers/ctr-a/stop", "") + if stop.Code != http.StatusOK || len(client.stopped) != 1 { + t.Fatalf("single-user stop = %d %s, stopped = %v", stop.Code, stop.Body.String(), client.stopped) + } + remove := do(router, http.MethodDelete, "/api/v1/docker/containers/ctr-b", "") + if remove.Code != http.StatusOK || len(client.removed) != 1 { + t.Fatalf("single-user remove = %d %s, removed = %v", remove.Code, remove.Body.String(), client.removed) + } + build := do(router, http.MethodPost, "/api/v1/docker/build", `{"dockerfile":"FROM scratch","tag":"local:dev"}`) + if build.Code != http.StatusOK { + t.Fatalf("single-user build status = %d, body = %s", build.Code, build.Body.String()) + } + + // A missing container still surfaces the daemon's failure, not a 404, and + // no ownership inspect happens on the unscoped path. + missing := do(router, http.MethodPost, "/api/v1/docker/containers/ctr-nope/stop", "") + if missing.Code != http.StatusInternalServerError { + t.Fatalf("single-user stop of missing container = %d, want 500; body = %s", missing.Code, missing.Body.String()) + } + if len(client.inspected) != 0 { + t.Fatalf("unscoped path inspected containers: %v", client.inspected) + } +} + +// TestNoIdentityIsUnscoped covers internal callers (no auth middleware in the +// chain), which must keep the pre-auth behavior. +func TestNoIdentityIsUnscoped(t *testing.T) { + client, authorizer, titles := twoUserFixture() + router := dockerTestRouter(t, client, titles.provider(), authorizer, nil) + + list := do(router, http.MethodGet, "/api/v1/docker/containers", "") + if !strings.Contains(list.Body.String(), "ctr-a") || !strings.Contains(list.Body.String(), "ctr-b") { + t.Fatalf("unauthenticated listing = %s", list.Body.String()) + } +} + +// TestStaleSessionLabelFallsBackToTaskOwnership covers a container whose +// session row is gone (rolled back or removed) while its task and container +// live on: the owner must still see and control it. +func TestStaleSessionLabelFallsBackToTaskOwnership(t *testing.T) { + client, authorizer, titles := twoUserFixture() + delete(authorizer.sessionOwners, "session-a") + router := dockerTestRouter(t, client, titles.provider(), authorizer, member("user-a")) + + list := do(router, http.MethodGet, "/api/v1/docker/containers", "") + if !strings.Contains(list.Body.String(), "ctr-a") { + t.Fatalf("owner lost their container to a stale session label: %s", list.Body.String()) + } + + stop := do(router, http.MethodPost, "/api/v1/docker/containers/ctr-a/stop", "") + if stop.Code != http.StatusOK { + t.Fatalf("stop with stale session label = %d, want 200; body = %s", stop.Code, stop.Body.String()) + } + remove := do(router, http.MethodDelete, "/api/v1/docker/containers/ctr-a", "") + if remove.Code != http.StatusOK { + t.Fatalf("remove with stale session label = %d, want 200; body = %s", remove.Code, remove.Body.String()) + } +} + +// TestStaleSessionLabelStillDeniesForeignTask pins that the task label is a +// fallback, not a bypass: a stale session on someone else's task stays hidden. +func TestStaleSessionLabelStillDeniesForeignTask(t *testing.T) { + client, authorizer, titles := twoUserFixture() + delete(authorizer.sessionOwners, "session-a") + router := dockerTestRouter(t, client, titles.provider(), authorizer, member("user-b")) + + list := do(router, http.MethodGet, "/api/v1/docker/containers", "") + if strings.Contains(list.Body.String(), "ctr-a") { + t.Fatalf("foreign container leaked via task fallback: %s", list.Body.String()) + } + stop := do(router, http.MethodPost, "/api/v1/docker/containers/ctr-a/stop", "") + if stop.Code != http.StatusNotFound || len(client.stopped) != 0 { + t.Fatalf("foreign stop = %d, stopped = %v", stop.Code, client.stopped) + } +} + +// TestNilAuthorizerDeniesScopedCallers pins the fail-closed half of the wiring +// contract: with no authorizer wired (partial builds), a scoped caller sees +// nothing and can act on nothing. +func TestNilAuthorizerDeniesScopedCallers(t *testing.T) { + client, _, titles := twoUserFixture() + router := dockerTestRouter(t, client, titles.provider(), nil, member("user-a")) + + list := do(router, http.MethodGet, "/api/v1/docker/containers", "") + if list.Code != http.StatusOK || strings.Contains(list.Body.String(), "ctr-") { + t.Fatalf("nil-authorizer listing = %d %s, want an empty listing", list.Code, list.Body.String()) + } + stop := do(router, http.MethodPost, "/api/v1/docker/containers/ctr-a/stop", "") + if stop.Code != http.StatusNotFound { + t.Fatalf("nil-authorizer stop = %d, want 404", stop.Code) + } + remove := do(router, http.MethodDelete, "/api/v1/docker/containers/ctr-a", "") + if remove.Code != http.StatusNotFound { + t.Fatalf("nil-authorizer remove = %d, want 404", remove.Code) + } + if len(client.stopped) != 0 || len(client.removed) != 0 { + t.Fatalf("nil-authorizer reached Docker: stopped=%v removed=%v", client.stopped, client.removed) + } +} + +// TestBuildWithoutAnyIdentityIsRejected documents the deliberate asymmetry +// between /build and the container routes. Every HTTP request goes through the +// global identity middleware, so an identity-free request is unreachable in +// production; where the container routes stay unscoped for internal callers, +// the host-level build fails closed with RequireAdmin's 401. +func TestBuildWithoutAnyIdentityIsRejected(t *testing.T) { + client, authorizer, titles := twoUserFixture() + router := dockerTestRouter(t, client, titles.provider(), authorizer, nil) + + build := do(router, http.MethodPost, "/api/v1/docker/build", `{"dockerfile":"FROM scratch","tag":"x:1"}`) + if build.Code != http.StatusUnauthorized { + t.Fatalf("identity-free build = %d, want 401; body = %s", build.Code, build.Body.String()) + } + if len(client.builtTags) != 0 { + t.Fatalf("identity-free build reached Docker: %v", client.builtTags) + } +} diff --git a/apps/backend/internal/backendapp/docker_routes_test.go b/apps/backend/internal/backendapp/docker_routes_test.go new file mode 100644 index 0000000000..b4e435f181 --- /dev/null +++ b/apps/backend/internal/backendapp/docker_routes_test.go @@ -0,0 +1,23 @@ +package backendapp + +import ( + "testing" + + taskservice "github.com/kandev/kandev/internal/task/service" +) + +// TestDockerSessionAuthorizerNilTaskServiceIsUntypedNil pins the fail-closed +// half of the Docker route wiring: a partial build with no task service must +// hand the handlers an untyped nil, which they treat as "deny scoped callers", +// rather than a non-nil interface wrapping a nil *Service that panics on call. +func TestDockerSessionAuthorizerNilTaskServiceIsUntypedNil(t *testing.T) { + if authorizer := dockerSessionAuthorizer(nil); authorizer != nil { + t.Fatalf("dockerSessionAuthorizer(nil) = %#v, want untyped nil", authorizer) + } +} + +func TestDockerSessionAuthorizerReturnsTaskService(t *testing.T) { + if authorizer := dockerSessionAuthorizer(&taskservice.Service{}); authorizer == nil { + t.Fatal("dockerSessionAuthorizer(svc) = nil, want the task service") + } +} diff --git a/apps/backend/internal/backendapp/helpers.go b/apps/backend/internal/backendapp/helpers.go index 7fd2141beb..9b27910b36 100644 --- a/apps/backend/internal/backendapp/helpers.go +++ b/apps/backend/internal/backendapp/helpers.go @@ -1306,7 +1306,10 @@ func registerSecondaryRoutes( p.log.Debug("Registered Plugins handlers (HTTP)") } - docker.RegisterDockerRoutes(p.router, p.lifecycleMgr.DockerClientProvider(), dockerTaskTitleProvider(p.taskRepo, p.log), p.log) + docker.RegisterDockerRoutes( + p.router, p.lifecycleMgr.DockerClientProvider(), + dockerTaskTitleProvider(p.taskRepo, p.log), dockerSessionAuthorizer(p.taskSvc), p.log, + ) p.log.Debug("Registered Docker management handlers (HTTP)") registerHealthRoutes(p) @@ -1473,6 +1476,17 @@ func workspaceIDFromPath(path string) string { return rest } +// dockerSessionAuthorizer scopes the Docker management endpoints to the +// caller's own task sessions. Returning an untyped nil when the task service +// is absent (partial builds) keeps the handlers failing closed instead of +// calling through a typed-nil interface. +func dockerSessionAuthorizer(taskSvc *taskservice.Service) docker.SessionAuthorizer { + if taskSvc == nil { + return nil + } + return taskSvc +} + func dockerTaskTitleProvider(taskRepo *sqliterepo.Repository, log *logger.Logger) docker.TaskTitleProvider { return func(ctx context.Context, taskID string) (string, bool) { if taskRepo == nil || taskID == "" {