fix(auth): scope Docker management endpoints to the caller (#3025)

Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
João Salavisa
2026-08-25 11:38:55 +01:00
committed by GitHub
parent 16ebbdb012
commit 9bfd94bcde
4 changed files with 698 additions and 21 deletions
+172 -20
View File
@@ -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()})
@@ -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)
}
}
@@ -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")
}
}
+15 -1
View File
@@ -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 == "" {