From eb87143eb9223aa72216a4d017c54824630a910f Mon Sep 17 00:00:00 2001 From: LUOSENGWA Date: Mon, 31 Aug 2026 16:33:08 +0000 Subject: [PATCH 1/4] feat(controller): read-only worker knowledge base file endpoints for L2 humans L2 humans have no read path to their team's knowledge base: the /docker/ proxy is admin/Manager-only, and the checkpoint endpoints expose file indexes, not the knowledge text. Proxy three read-only QwenPaw workspace file endpoints (QwenPaw >= 2.1) through the Controller using the fixed- subpath pattern from the worker checkpoint proxy: GET /api/v1/workers/{name}/workspace-files/tree GET /api/v1/workers/{name}/workspace-files/file-metadata GET /api/v1/workers/{name}/workspace-files/file-content - Path allowlist (not a denylist): only MEMORY.md, memory/** and digest/** are addressable. Every other workspace location - SOUL.md, PROFILE.md, TODO.md, checkpoints/, skills/ and all dot directories (.copaw/agent.json carries the worker's credentials) - is rejected with 400 before the request reaches the worker. The upstream path resolver serves directly requested dot paths, which is why the allowlist lives in the Controller; covered by an explicit test. - root=workspace pinned server-side (the QwenPaw default root=project is the primary bound project directory, not the knowledge base); the client query surface has no root parameter and the upstream query is rebuilt from validated values only. - Strict per-subpath query whitelist (unknown/duplicate parameters are 400); limit bounds mirror the upstream caps (tree 1..500, file-content 1..1048576); cursor is an opaque passthrough; offset >= 0. - Same address resolution as the checkpoint proxy: effective container prefix + system-wins console port (a conflicting spec.env port is discarded, so the proxy always dials the port the container listens on). Embedded mode only; kube mode returns a uniform 503 before any worker lookup. - Authorization reuses the existing chain: RequireAuthz(ActionGet, "worker") (RoleHuman already granted by the L2 authentication work) plus the handler-level team-scope check, so unknown, out-of-scope and standalone workers are all hidden as 404. No authorizer changes. - Version gate: a worker on QwenPaw < 2.1 has no workspace file router; the upstream 404 is passed through verbatim and the documented file-metadata?path=MEMORY.md probe distinguishes "worker upgrade required" from "file missing". - Write endpoints (file-content PUT, file-upload) and binary streaming (file-download) are not in the subpath whitelist. Tests: 24 new (forwarding with root-pinning assertion, allowlist positive/negative tables, sensitive-path and traversal rejection, write-subpath rejection, scope 200/404 matrix, kube 503 before lookup, unreachable 502, 404 passthrough, bounded 500, prefix+port matrix). go test ./... green against the main baseline (only pre-existing failure is the sandbox-missing unzip binary in internal/executor); gofmt/vet clean; rename gate passes. --- agentteams-controller/internal/server/http.go | 4 + .../internal/server/worker_workspace_files.go | 357 +++++++++++ .../server/worker_workspace_files_test.go | 587 ++++++++++++++++++ docs/usage/project-workflow-api.md | 53 ++ docs/zh-cn/usage/project-workflow-api.md | 43 ++ 5 files changed, 1044 insertions(+) create mode 100644 agentteams-controller/internal/server/worker_workspace_files.go create mode 100644 agentteams-controller/internal/server/worker_workspace_files_test.go diff --git a/agentteams-controller/internal/server/http.go b/agentteams-controller/internal/server/http.go index 9d2f20f80..11ac49bd0 100644 --- a/agentteams-controller/internal/server/http.go +++ b/agentteams-controller/internal/server/http.go @@ -127,6 +127,10 @@ func NewHTTPServer(addr string, deps ServerDeps) *HTTPServer { ckh := NewCheckpointHandler(deps.Client, deps.Namespace, deps.KubeMode, deps.ContainerPrefix) mux.Handle("GET /api/v1/workers/{name}/checkpoints/{sub}", mw.RequireAuthz(authpkg.ActionGet, "worker", nameFn)(http.HandlerFunc(ckh.proxyCheckpoint))) + // --- Worker knowledge base files (read-only MEMORY.md / memory/** / digest/** inspection; proxy to the worker's qwenpaw app) --- + wfh := NewWorkspaceFilesHandler(deps.Client, deps.Namespace, deps.KubeMode, deps.ContainerPrefix) + mux.Handle("GET /api/v1/workers/{name}/workspace-files/{sub}", mw.RequireAuthz(authpkg.ActionGet, "worker", nameFn)(http.HandlerFunc(wfh.proxyWorkspaceFiles))) + // W-PR-2: human intervention + lifecycle (write endpoints). All writes go // through RequireAuthz ActionUpdate + "project" so the authorizer's // requireSameTeam (TeamLeader / L2) rejects cross-team writes at the code diff --git a/agentteams-controller/internal/server/worker_workspace_files.go b/agentteams-controller/internal/server/worker_workspace_files.go new file mode 100644 index 000000000..b95eb4ebb --- /dev/null +++ b/agentteams-controller/internal/server/worker_workspace_files.go @@ -0,0 +1,357 @@ +package server + +// Worker knowledge base file inspection +// (GET /api/v1/workers/{name}/workspace-files/...). +// +// Each worker's qwenpaw app (QwenPaw >= 2.1) exposes read-only workspace +// file endpoints on :8088 (0.0.0.0 listen; no auth in worker context +// because no console user is registered): /workspace/tree (paginated +// directory listing), /workspace/file-metadata and /workspace/file-content +// (bounded UTF-8 chunk reads). The Controller proxies those three +// read-only subpaths so L2 humans and the workbench plugin can inspect a +// worker's knowledge base (MEMORY.md, memory/**, digest/**) without +// reaching into the docker network directly. +// +// Embedded mode only: the worker app is reachable by container name inside +// the shared docker network. The effective container name prefix comes from +// configuration (AGENTTEAMS_PROXY_CONTAINER_PREFIX, or derived from +// AGENTTEAMS_RESOURCE_PREFIX when auto-prefixing is enabled; empty when +// auto-prefixing is disabled), and the port is the effective console port +// resolved through the same system-wins env chain used at container +// creation (service.EffectiveWorkerConsolePort — a conflicting spec.env +// value is discarded, so the container always listens on 8088). In kube +// mode there is no stable in-cluster DNS name for the worker pod, so the +// endpoints return 503. +// +// Two independent path boundaries: +// +// - The upstream app hardens path resolution (no absolute paths, no +// ".." segments, no NUL bytes, no symlink escape outside the workspace +// root) and hides dot entries in directory listings — but a directly +// requested path still resolves, so dot directories (.copaw/agent.json +// carries the worker's Matrix token and MinIO credentials) remain +// reachable upstream. +// +// - This handler therefore enforces its own allowlist on top: only +// MEMORY.md, memory/** and digest/** are addressable. The allowlist is +// a prefix allowlist on exact root names (memory, digest) plus the +// single top-level file MEMORY.md — never a denylist — so any other +// workspace content (SOUL.md, PROFILE.md, TODO.md, .copaw/, .qwenpaw/, +// checkpoints/, skills/, ...) is rejected before the request reaches +// the worker. +// +// The upstream root=workspace parameter (the agent's own storage root, as +// opposed to root=project, the primary bound project directory) is pinned +// server-side and is never part of the client-facing query surface. +// +// Fixed-path forwarding only (tree / file-metadata / file-content, plus +// their whitelisted queries) — never a generic reverse proxy, and never a +// write endpoint, so the attack surface is limited to three read-only +// QwenPaw endpoints. + +import ( + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strconv" + "strings" + "time" + + v1beta1 "github.com/agentscope-ai/AgentTeams/agentteams-controller/api/v1beta1" + authpkg "github.com/agentscope-ai/AgentTeams/agentteams-controller/internal/auth" + "github.com/agentscope-ai/AgentTeams/agentteams-controller/internal/httputil" + "github.com/agentscope-ai/AgentTeams/agentteams-controller/internal/service" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + // workspaceFilesProxyTimeout bounds each upstream call. + workspaceFilesProxyTimeout = 5 * time.Second + + // kbPathMaxSegments bounds the depth of addressable knowledge paths + // (memory/2026-08-31/topic.md is three segments; four leaves margin + // for one more nesting level without opening the full workspace tree). + kbPathMaxSegments = 4 + + // kbSegmentMaxBytes matches the upstream per-segment limit + // (QwenPaw workspace_files._validate_segment). + kbSegmentMaxBytes = 255 + + // kbMaxPageLimit mirrors the upstream tree pagination cap (QwenPaw + // workspace_files.MAX_PAGE_SIZE). + kbMaxPageLimit = 500 + + // kbMaxFileLimit mirrors the upstream file-content chunk cap (QwenPaw + // workspace_files.MAX_CHUNK_SIZE). + kbMaxFileLimit = 1024 * 1024 +) + +// workspaceFileSubpaths is the fixed whitelist of forwardable QwenPaw +// endpoints. Write endpoints (file-content PUT, file-upload) and binary +// streaming (file-download) are deliberately absent. +var workspaceFileSubpaths = map[string]bool{ + "tree": true, + "file-metadata": true, + "file-content": true, +} + +// kbFileRoots are the top-level single files addressable by the +// file-metadata / file-content subpaths. +var kbFileRoots = []string{"MEMORY.md"} + +// kbDirRoots are the knowledge base directories addressable by all three +// subpaths (tree on the directory or any of its subpaths; the file +// subpaths on files below it). Matching is on the exact first segment, so +// memories/ and memoryX/ are not prefixes of memory/. +var kbDirRoots = []string{"memory", "digest"} + +// validateKbPath enforces the knowledge base allowlist (see the package +// comment). forFile selects the file subpaths (file-metadata / +// file-content); the tree subpath takes directory paths. +func validateKbPath(path string, forFile bool) error { + if path == "" { + return errors.New("path is required (memory/ or digest/)") + } + if strings.ContainsRune(path, '\\') || strings.ContainsRune(path, 0) || strings.HasPrefix(path, "/") { + return errors.New("path must be a relative POSIX path") + } + segments := strings.Split(path, "/") + if len(segments) > kbPathMaxSegments { + return errors.New("path is too deep") + } + for _, seg := range segments { + if seg == "" || seg == "." || seg == ".." { + return errors.New("path contains an invalid segment") + } + if strings.HasPrefix(seg, ".") { + return errors.New("hidden paths are not accessible") + } + if len(seg) > kbSegmentMaxBytes { + return errors.New("path segment is too long") + } + } + first := segments[0] + if forFile { + for _, root := range kbFileRoots { + if first == root { + return nil + } + } + } + for _, root := range kbDirRoots { + if first == root { + return nil + } + } + return errors.New("path is not in the knowledge base allowlist") +} + +// validateWorkspaceFilesQuery enforces the strict per-subpath query +// whitelist and returns the upstream query string with root=workspace +// pinned. Unknown or duplicate parameters are rejected rather than +// silently dropped so client mistakes surface immediately (the same +// semantics the checkpoint proxy enforces). +func validateWorkspaceFilesQuery(sub string, q url.Values) (string, error) { + var allowed map[string]bool + switch sub { + case "tree": + allowed = map[string]bool{"path": true, "cursor": true, "limit": true} + case "file-metadata": + allowed = map[string]bool{"path": true} + case "file-content": + allowed = map[string]bool{"path": true, "offset": true, "limit": true} + } + for key, vals := range q { + if !allowed[key] { + return "", fmt.Errorf("unsupported query parameter: %s", key) + } + if len(vals) > 1 { + return "", fmt.Errorf("duplicate query parameter: %s", key) + } + } + if err := validateKbPath(q.Get("path"), sub != "tree"); err != nil { + return "", err + } + if sub == "tree" || sub == "file-content" { + if raw := q.Get("limit"); raw != "" { + maxLimit := kbMaxFileLimit + if sub == "tree" { + maxLimit = kbMaxPageLimit + } + limit, err := strconv.Atoi(raw) + if err != nil || limit < 1 || limit > maxLimit { + return "", fmt.Errorf("limit must be an integer between 1 and %d", maxLimit) + } + } + } + if sub == "file-content" { + if raw := q.Get("offset"); raw != "" { + offset, err := strconv.Atoi(raw) + if err != nil || offset < 0 { + return "", errors.New("offset must be a non-negative integer") + } + } + } + // Rebuild the upstream query from the validated values only, in a + // fixed order, and pin the root. The client's raw query string is + // never forwarded verbatim. + up := url.Values{} + up.Set("path", q.Get("path")) + if raw := q.Get("cursor"); raw != "" { + up.Set("cursor", raw) + } + if raw := q.Get("limit"); raw != "" { + up.Set("limit", raw) + } + if raw := q.Get("offset"); raw != "" { + up.Set("offset", raw) + } + up.Set("root", "workspace") + return up.Encode(), nil +} + +// WorkspaceFilesHandler proxies worker knowledge base read endpoints. +type WorkspaceFilesHandler struct { + client client.Client + namespace string + kubeMode string + http *http.Client + // containerPrefix is the effective worker container name prefix — the + // same value the docker backend uses for container naming (derived from + // AGENTTEAMS_PROXY_CONTAINER_PREFIX / AGENTTEAMS_RESOURCE_PREFIX / + // auto-prefix; empty when auto-prefixing is disabled). + containerPrefix string + // workerBaseURL resolves a worker name to its qwenpaw app base URL from + // the effective prefix and the worker's env. Injectable for tests. + workerBaseURL func(name string, env map[string]string) string +} + +// NewWorkspaceFilesHandler creates the handler with the default +// embedded-mode worker address resolution. containerPrefix must be the +// effective prefix from controller configuration (see +// config.ContainerPrefix). +func NewWorkspaceFilesHandler(c client.Client, namespace, kubeMode, containerPrefix string) *WorkspaceFilesHandler { + h := &WorkspaceFilesHandler{ + client: c, + namespace: namespace, + kubeMode: kubeMode, + http: &http.Client{Timeout: workspaceFilesProxyTimeout}, + containerPrefix: containerPrefix, + } + h.workerBaseURL = h.defaultWorkerBaseURL + return h +} + +// defaultWorkerBaseURL resolves a worker's qwenpaw app base URL from the +// effective container prefix and the effective console port. The port goes +// through service.EffectiveWorkerConsolePort — the same system-wins env +// chain used at container creation — so the proxy can never target a port +// the container does not listen on (a conflicting spec.env value is +// discarded before the container is created, so the raw spec.env must not +// be read here). +func (h *WorkspaceFilesHandler) defaultWorkerBaseURL(name string, env map[string]string) string { + port := service.EffectiveWorkerConsolePort(env) + return fmt.Sprintf("http://%s%s:%s", h.containerPrefix, name, port) +} + +// proxyWorkspaceFiles handles GET /api/v1/workers/{name}/workspace-files/{sub}. +// Scoped callers (team leaders / L2 humans) may only inspect workers in the +// teams they control — mirrors GET /api/v1/workers/{name} and the +// checkpoint proxy (W8: 404, not 403, so worker existence cannot be +// probed). +func (h *WorkspaceFilesHandler) proxyWorkspaceFiles(w http.ResponseWriter, r *http.Request) { + name := r.PathValue("name") + sub := r.PathValue("sub") + if name == "" || !workerNamePattern.MatchString(name) { + httputil.WriteError(w, http.StatusBadRequest, "worker name is required and must be a valid DNS label") + return + } + if !workspaceFileSubpaths[sub] { + httputil.WriteError(w, http.StatusBadRequest, "unsupported workspace file subpath") + return + } + // Kube-mode check runs before any worker lookup: the endpoints are + // entirely unavailable in kube mode, and a uniform 503 (rather than a + // per-worker 404 vs 503 split) avoids leaking worker existence. + if h.kubeMode != "embedded" { + httputil.WriteError(w, http.StatusServiceUnavailable, "worker workspace file inspection requires embedded mode") + return + } + + var worker v1beta1.Worker + if err := h.client.Get(r.Context(), client.ObjectKey{Name: name, Namespace: h.namespace}, &worker); err != nil { + if apierrors.IsNotFound(err) { + httputil.WriteError(w, http.StatusNotFound, "worker not found") + return + } + writeK8sError(w, "get worker workspace files", err) + return + } + // Resolve the owning team for the scoped-caller check (same chain as + // ResourceHandler.GetWorker and the checkpoint proxy: standalone + // workers hide as 404 for scoped callers). + teamObj, _, _, err := findTeamMember(r.Context(), h.client, h.namespace, name) + if err != nil { + writeK8sError(w, "get worker workspace files", err) + return + } + // Note: findTeamMember's second return value is the member (worker) + // name, not the team name — the scoped check must compare against the + // Team CR name (see ResourceHandler.GetWorker). + teamName := "" + if teamObj != nil { + teamName = teamObj.Name + } + if caller := authpkg.CallerFromContext(r.Context()); caller != nil && + (caller.Role == authpkg.RoleTeamLeader || caller.Role == authpkg.RoleHuman) && + !caller.TeamMatches(teamName) { + httputil.WriteError(w, http.StatusNotFound, "worker not found") + return + } + + query, err := validateWorkspaceFilesQuery(sub, r.URL.Query()) + if err != nil { + httputil.WriteError(w, http.StatusBadRequest, err.Error()) + return + } + + target := h.workerBaseURL(name, worker.Spec.Env) + "/workspace/" + sub + if query != "" { + target += "?" + query + } + + req, err := http.NewRequestWithContext(r.Context(), http.MethodGet, target, nil) + if err != nil { + httputil.WriteError(w, http.StatusInternalServerError, "build workspace files request: "+err.Error()) + return + } + resp, err := h.http.Do(req) + if err != nil { + // Connection refused (worker stopped), DNS failure, timeout. + httputil.WriteError(w, http.StatusBadGateway, "worker workspace API unreachable") + return + } + defer resp.Body.Close() + + switch resp.StatusCode { + case http.StatusOK: + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = io.Copy(w, resp.Body) + case http.StatusBadRequest, http.StatusNotFound, http.StatusConflict, http.StatusRequestedRangeNotSatisfiable: + // Pass through verbatim: invalid cursor/offset (400), file not + // found — which is also the pre-2.1 router-missing signal, see the + // documentation's MEMORY.md probe heuristic (404), file changed + // while being read (409), or offset beyond end of file (416). + body, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(resp.StatusCode) + _, _ = w.Write(body) + default: + body, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) + httputil.WriteError(w, http.StatusBadGateway, fmt.Sprintf("workspace files API error (status %d): %s", resp.StatusCode, string(body))) + } +} diff --git a/agentteams-controller/internal/server/worker_workspace_files_test.go b/agentteams-controller/internal/server/worker_workspace_files_test.go new file mode 100644 index 000000000..7ddd8ec47 --- /dev/null +++ b/agentteams-controller/internal/server/worker_workspace_files_test.go @@ -0,0 +1,587 @@ +package server + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + v1beta1 "github.com/agentscope-ai/AgentTeams/agentteams-controller/api/v1beta1" + authpkg "github.com/agentscope-ai/AgentTeams/agentteams-controller/internal/auth" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +// newTestWorkspaceFilesHandler builds a handler with a fake K8s client +// (given teams and workers) whose worker URLs point at the given httptest +// server (mimicking one worker's qwenpaw app). +func newTestWorkspaceFilesHandler(t *testing.T, kubeMode string, ts *httptest.Server, objs ...runtime.Object) *WorkspaceFilesHandler { + t.Helper() + k8s := fake.NewClientBuilder().WithScheme(newProjectTestScheme(t)).WithRuntimeObjects(objs...).Build() + h := NewWorkspaceFilesHandler(k8s, "default", kubeMode, "agentteams-worker-") + if ts != nil { + h.workerBaseURL = func(string, map[string]string) string { return ts.URL } + } + return h +} + +// kbRequest builds a request against the workspace-files route. The URL +// always uses a safe placeholder; the actual (possibly invalid) name is +// set via PathValue so handler validation is what's tested. query is an +// already-encoded raw query string (no leading "?"). +func kbRequest(name, sub, query string) *http.Request { + target := "/api/v1/workers/placeholder/workspace-files/" + sub + if query != "" { + target += "?" + query + } + req := httptest.NewRequest(http.MethodGet, target, nil) + req.SetPathValue("name", name) + req.SetPathValue("sub", sub) + return req +} + +// kbWorkerFixture builds a Team plus its Worker CRs. +func kbWorkerFixture(team string, workers ...string) []runtime.Object { + return checkpointTeamWithWorkers(team, workers...) +} + +func TestWorkspaceFilesTree_ForwardsWithRootPinned(t *testing.T) { + const payload = `{"directory":"memory","entries":[{"kind":"directory","name":"2026-08-31","path":"memory/2026-08-31"}],"has_more":false,"next_cursor":null}` + var gotPath, gotQuery string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotQuery = r.URL.RawQuery + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(payload)) + })) + defer upstream.Close() + + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("market-writer", "tree", "path=memory"))) + + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + if gotPath != "/workspace/tree" { + t.Fatalf("upstream path=%q, want /workspace/tree", gotPath) + } + want := url.Values{"path": {"memory"}, "root": {"workspace"}} + if gotQuery != want.Encode() { + t.Fatalf("upstream query=%q, want %q (root must be pinned server-side)", gotQuery, want.Encode()) + } + if strings.Contains(gotQuery, "project") { + t.Fatalf("upstream query=%q must never expose a root=project default", gotQuery) + } + if rec.Body.String() != payload { + t.Fatalf("body not verbatim:\n got %s\nwant %s", rec.Body.String(), payload) + } +} + +func TestWorkspaceFilesFileContent_ForwardsOffsetLimit(t *testing.T) { + const payload = `{"content":"# Memory","encoding":"utf-8","eof":true,"etag":"W/\"1-2\"","limit":2,"next_offset":2,"offset":0,"path":"MEMORY.md","truncated":false}` + var gotPath, gotQuery string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotQuery = r.URL.RawQuery + _, _ = w.Write([]byte(payload)) + })) + defer upstream.Close() + + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("market-writer", "file-content", "path=MEMORY.md&offset=0&limit=4096"))) + + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + if gotPath != "/workspace/file-content" { + t.Fatalf("upstream path=%q", gotPath) + } + want := url.Values{"path": {"MEMORY.md"}, "offset": {"0"}, "limit": {"4096"}, "root": {"workspace"}} + if gotQuery != want.Encode() { + t.Fatalf("upstream query=%q, want %q", gotQuery, want.Encode()) + } +} + +func TestWorkspaceFilesFileMetadata_Forwards(t *testing.T) { + const payload = `{"etag":"W/\"1-2\"","modified_at":"2026-08-31T00:00:00Z","path":"MEMORY.md","preview_kind":"text","size":2}` + var gotPath, gotQuery string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotQuery = r.URL.RawQuery + _, _ = w.Write([]byte(payload)) + })) + defer upstream.Close() + + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("market-writer", "file-metadata", "path=MEMORY.md"))) + + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + if gotPath != "/workspace/file-metadata" { + t.Fatalf("upstream path=%q", gotPath) + } + want := url.Values{"path": {"MEMORY.md"}, "root": {"workspace"}} + if gotQuery != want.Encode() { + t.Fatalf("upstream query=%q, want %q", gotQuery, want.Encode()) + } +} + +func TestWorkspaceFilesTree_CursorPassthrough(t *testing.T) { + var gotQuery string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotQuery = r.URL.RawQuery + _, _ = w.Write([]byte(`{"directory":"memory","entries":[],"has_more":true,"next_cursor":"abc123"}`)) + })) + defer upstream.Close() + + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("market-writer", "tree", "path=memory&cursor=abc123&limit=50"))) + + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + want := url.Values{"path": {"memory"}, "cursor": {"abc123"}, "limit": {"50"}, "root": {"workspace"}} + if gotQuery != want.Encode() { + t.Fatalf("upstream query=%q, want %q (cursor is an opaque passthrough)", gotQuery, want.Encode()) + } +} + +// TestWorkspaceFiles_AllowlistOK locks the positive boundary of the +// knowledge base allowlist: MEMORY.md at top level, memory/** and +// digest/** at realistic depths (real worker layout: +// memory/YYYY-MM-DD/{topic}.md and digest/{personal,procedure,wiki}/*.md). +func TestWorkspaceFiles_AllowlistOK(t *testing.T) { + filePaths := []string{ + "MEMORY.md", + "memory/2026-08-31/topic-note.md", + "digest/personal/user-preferences.md", + } + for _, p := range filePaths { + if err := validateKbPath(p, true); err != nil { + t.Errorf("validateKbPath(%q, file)=%v, want ok", p, err) + } + } + dirPaths := []string{ + "memory", + "memory/2026-08-31", + "digest", + "digest/personal", + "digest/procedure", + "digest/wiki", + } + for _, p := range dirPaths { + if err := validateKbPath(p, false); err != nil { + t.Errorf("validateKbPath(%q, dir)=%v, want ok", p, err) + } + } +} + +// TestWorkspaceFiles_RejectsSensitivePaths is the security boundary of +// this PR: the worker's runtime configuration (Matrix token, MinIO +// credentials) and every other dot directory must be unreachable through +// the proxy, even though the upstream path resolver would happily serve a +// directly requested dot path. +func TestWorkspaceFiles_RejectsSensitivePaths(t *testing.T) { + for _, p := range []string{ + ".copaw/agent.json", + ".git/config", + ".qwenpaw/agent.json", + ".reme_store_v1/index.json", + ".skill.json.lock", + "memory/.hidden-note.md", + "digest/.keep", + } { + if err := validateKbPath(p, true); err == nil { + t.Errorf("validateKbPath(%q, file)=nil, want rejection", p) + } + if err := validateKbPath(p, false); err == nil { + t.Errorf("validateKbPath(%q, dir)=nil, want rejection", p) + } + } +} + +func TestWorkspaceFiles_RejectsTraversal(t *testing.T) { + for _, p := range []string{ + "../MEMORY.md", + "memory/../../SOUL.md", + "/MEMORY.md", + "MEMORY.md\\", + "memory/./x.md", + "memory/x/.hidden", + "memory//x.md", + } { + if err := validateKbPath(p, true); err == nil { + t.Errorf("validateKbPath(%q, file)=nil, want rejection", p) + } + } +} + +// TestWorkspaceFiles_RejectsNonKB locks the negative boundary: workspace +// content outside MEMORY.md / memory/** / digest/** (identity files, +// TODO, checkpoint store, skills, prefix-confusion names) must not be +// addressable. +func TestWorkspaceFiles_RejectsNonKB(t *testing.T) { + for _, p := range []string{ + "SOUL.md", + "PROFILE.md", + "AGENTS.md", + "TODO.md", + "checkpoints/shadow.git", + "skills/some-skill.md", + "memories/x.md", + "memoryX/x.md", + "digestX/x.md", + "memory/a/b/c/d", + "", + } { + if err := validateKbPath(p, true); err == nil { + t.Errorf("validateKbPath(%q, file)=nil, want rejection", p) + } + } + for _, p := range []string{ + "SOUL.md", + "checkpoints", + "skills", + "memories", + "memoryX", + "", + } { + if err := validateKbPath(p, false); err == nil { + t.Errorf("validateKbPath(%q, dir)=nil, want rejection", p) + } + } +} + +func TestWorkspaceFiles_UnknownQueryRejected(t *testing.T) { + h := newTestWorkspaceFilesHandler(t, "embedded", nil, kbWorkerFixture("market-team", "market-writer")...) + cases := []struct { + sub, query string + }{ + {"tree", "path=memory&offset=0"}, + {"tree", "path=memory&root=project"}, + {"file-metadata", "path=MEMORY.md&cursor=x"}, + {"file-metadata", "path=MEMORY.md&offset=0"}, + {"file-content", "path=MEMORY.md&cursor=x"}, + {"tree", "path=memory&path=memory"}, + } + for _, tc := range cases { + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("market-writer", tc.sub, tc.query))) + if rec.Code != http.StatusBadRequest { + t.Fatalf("sub=%s query=%q status=%d, want 400", tc.sub, tc.query, rec.Code) + } + } +} + +func TestWorkspaceFiles_InvalidBounds(t *testing.T) { + h := newTestWorkspaceFilesHandler(t, "embedded", nil, kbWorkerFixture("market-team", "market-writer")...) + for _, q := range []string{ + "path=memory&limit=0", + "path=memory&limit=1001", + "path=memory&limit=abc", + "path=MEMORY.md&offset=-1", + "path=MEMORY.md&offset=1.5", + "path=MEMORY.md&offset=abc", + "path=MEMORY.md&limit=0", + "path=MEMORY.md&limit=1048577", // above the upstream MAX_CHUNK_SIZE mirror + "path=memory&limit=501", // above the upstream MAX_PAGE_SIZE mirror + } { + sub := "tree" + if strings.Contains(q, "offset") || (strings.Contains(q, "limit") && strings.Contains(q, "MEMORY.md")) { + sub = "file-content" + } + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("market-writer", sub, q))) + if rec.Code != http.StatusBadRequest { + t.Fatalf("query=%q status=%d, want 400", q, rec.Code) + } + } +} + +func TestWorkspaceFiles_FileSubpathsRequirePath(t *testing.T) { + h := newTestWorkspaceFilesHandler(t, "embedded", nil, kbWorkerFixture("market-team", "market-writer")...) + for _, sub := range []string{"file-metadata", "file-content"} { + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("market-writer", sub, ""))) + if rec.Code != http.StatusBadRequest { + t.Fatalf("sub=%s status=%d, want 400 for missing path", sub, rec.Code) + } + } +} + +func TestWorkspaceFiles_RejectsWriteAndUnknownSubpaths(t *testing.T) { + h := newTestWorkspaceFilesHandler(t, "embedded", nil, kbWorkerFixture("market-team", "market-writer")...) + for _, sub := range []string{"file-upload", "download", "restore", "graph", "status", "agent.json"} { + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("market-writer", sub, "path=MEMORY.md"))) + if rec.Code != http.StatusBadRequest { + t.Fatalf("sub=%s status=%d, want 400 (write endpoints must never be reachable)", sub, rec.Code) + } + } +} + +func TestWorkspaceFiles_RejectsInvalidWorkerName(t *testing.T) { + h := newTestWorkspaceFilesHandler(t, "embedded", nil) + for _, name := range []string{"", "Market-Writer", "../etc", "a b"} { + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest(name, "tree", "path=memory"))) + if rec.Code != http.StatusBadRequest { + t.Fatalf("name=%q status=%d, want 400", name, rec.Code) + } + } +} + +func TestWorkspaceFiles_UnknownWorker(t *testing.T) { + h := newTestWorkspaceFilesHandler(t, "embedded", nil) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("ghost-worker", "tree", "path=memory"))) + if rec.Code != http.StatusNotFound { + t.Fatalf("status=%d, want 404 for unknown worker", rec.Code) + } +} + +// TestWorkspaceFiles_L2HumanInScopeAllowed is the purpose case of this +// PR: an L2 human reads a worker in one of their accessibleTeams with +// their own Matrix-token identity. +func TestWorkspaceFiles_L2HumanInScopeAllowed(t *testing.T) { + const payload = `{"content":"# Market team memory","encoding":"utf-8","eof":true,"limit":23,"next_offset":23,"offset":0,"path":"MEMORY.md","truncated":false}` + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(payload)) + })) + defer upstream.Close() + + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + req := withCaller(kbRequest("market-writer", "file-content", "path=MEMORY.md"), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "maizong", Teams: []string{"market-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s, want 200 for in-scope L2 human", rec.Code, rec.Body.String()) + } + if rec.Body.String() != payload { + t.Fatalf("body=%s", rec.Body.String()) + } +} + +func TestWorkspaceFiles_L2HumanCrossTeamHidden(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{}`)) + })) + defer upstream.Close() + + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + req := withCaller(kbRequest("market-writer", "tree", "path=memory"), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "sunzong", Teams: []string{"biz-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status=%d, want 404 for cross-team access (W8: hidden, not 403)", rec.Code) + } +} + +func TestWorkspaceFiles_TeamLeaderCrossTeamDenied(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{}`)) + })) + defer upstream.Close() + + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + req := withCaller(kbRequest("market-writer", "tree", "path=memory"), + &authpkg.CallerIdentity{Role: authpkg.RoleTeamLeader, Username: "biz-lead", Team: "biz-team"}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status=%d, want 404 for cross-team team leader (W8)", rec.Code) + } +} + +// TestWorkspaceFiles_StandaloneWorkerScopedHidden: a worker without a +// Team is hidden from scoped callers (they cannot enumerate standalone +// workers) but visible to admin. +func TestWorkspaceFiles_StandaloneWorkerScopedHidden(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"directory":"memory","entries":[],"has_more":false,"next_cursor":null}`)) + })) + defer upstream.Close() + + // A Worker CR that no Team references (standalone worker). + k8s := fake.NewClientBuilder().WithScheme(newProjectTestScheme(t)).WithRuntimeObjects(checkpointWorker("standalone-worker")).Build() + h := NewWorkspaceFilesHandler(k8s, "default", "embedded", "agentteams-worker-") + h.workerBaseURL = func(string, map[string]string) string { return upstream.URL } + + scoped := withCaller(kbRequest("standalone-worker", "tree", "path=memory"), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "maizong", Teams: []string{"market-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, scoped) + if rec.Code != http.StatusNotFound { + t.Fatalf("scoped status=%d, want 404 for standalone worker", rec.Code) + } + + rec = httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("standalone-worker", "tree", "path=memory"))) + if rec.Code != http.StatusOK { + t.Fatalf("admin status=%d, want 200 for standalone worker", rec.Code) + } +} + +// TestWorkspaceFiles_KubeModeUnsupported uses a ghost worker: the 503 +// must come before any worker lookup, otherwise a 404-vs-503 split would +// leak worker existence. +func TestWorkspaceFiles_KubeModeUnsupported(t *testing.T) { + h := newTestWorkspaceFilesHandler(t, "k8s", nil) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("ghost-worker", "tree", "path=memory"))) + if rec.Code != http.StatusServiceUnavailable { + t.Fatalf("status=%d, want 503 in kube mode (before worker lookup)", rec.Code) + } +} + +func TestWorkspaceFiles_UnreachableWorker(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + urlStr := upstream.URL + upstream.Close() + + h := newTestWorkspaceFilesHandler(t, "embedded", nil, kbWorkerFixture("market-team", "market-writer")...) + h.workerBaseURL = func(string, map[string]string) string { return urlStr } + + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("market-writer", "tree", "path=memory"))) + if rec.Code != http.StatusBadGateway { + t.Fatalf("status=%d, want 502 for unreachable worker", rec.Code) + } + if !strings.Contains(rec.Body.String(), "unreachable") { + t.Fatalf("body=%s, want unreachable message", rec.Body.String()) + } +} + +// TestWorkspaceFiles_Upstream404PassThrough: an upstream 404 means either +// "file not found" or "worker runs QwenPaw < 2.1" (no workspace router). +// It is passed through verbatim; clients use the MEMORY.md file-metadata +// probe to tell the two apart (see the API documentation). +func TestWorkspaceFiles_Upstream404PassThrough(t *testing.T) { + const detail = `{"detail":"File not found"}` + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(detail)) + })) + defer upstream.Close() + + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("market-writer", "file-content", "path=MEMORY.md"))) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status=%d, want 404 passthrough", rec.Code) + } + if rec.Body.String() != detail { + t.Fatalf("body=%s, want verbatim upstream detail", rec.Body.String()) + } +} + +func TestWorkspaceFiles_Upstream500Bounded(t *testing.T) { + big := strings.Repeat("x", 8192) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(big)) + })) + defer upstream.Close() + + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("market-writer", "tree", "path=memory"))) + + if rec.Code != http.StatusBadGateway { + t.Fatalf("status=%d, want 502", rec.Code) + } + var body map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("body not json: %v", err) + } +} + +// TestWorkspaceFilesWorkerBaseURL_PrefixAndPortResolution covers the +// address resolution matrix: default / non-default / empty container +// prefixes. The console port is ALWAYS the effective system port: the +// worker system env defines AGENTTEAMS_CONSOLE_PORT and the system-wins +// user-env merge discards conflicting spec.env values before the container +// is created, so any user-declared port must resolve to the same port the +// container actually listens on. +func TestWorkspaceFilesWorkerBaseURL_PrefixAndPortResolution(t *testing.T) { + cases := []struct { + name string + prefix string + env map[string]string + want string + }{ + {"default prefix and port", "agentteams-worker-", nil, "http://agentteams-worker-alice:8088"}, + {"non-default prefix", "acme-worker-", nil, "http://acme-worker-alice:8088"}, + {"empty prefix (auto-prefix disabled)", "", nil, "http://alice:8088"}, + {"user port discarded (system wins)", "agentteams-worker-", map[string]string{"AGENTTEAMS_CONSOLE_PORT": "9090"}, "http://agentteams-worker-alice:8088"}, + {"custom prefix, user port discarded", "acme-worker-", map[string]string{"AGENTTEAMS_CONSOLE_PORT": " 7000 "}, "http://acme-worker-alice:8088"}, + {"invalid user port discarded", "agentteams-worker-", map[string]string{"AGENTTEAMS_CONSOLE_PORT": "not-a-port"}, "http://agentteams-worker-alice:8088"}, + {"out-of-range user port discarded", "agentteams-worker-", map[string]string{"AGENTTEAMS_CONSOLE_PORT": "99999"}, "http://agentteams-worker-alice:8088"}, + {"zero user port discarded", "agentteams-worker-", map[string]string{"AGENTTEAMS_CONSOLE_PORT": "0"}, "http://agentteams-worker-alice:8088"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h := &WorkspaceFilesHandler{containerPrefix: tc.prefix} + if got := h.defaultWorkerBaseURL("alice", tc.env); got != tc.want { + t.Errorf("got %s, want %s", got, tc.want) + } + }) + } +} + +// captureWorkspaceFilesRoundTripper records the upstream URL the handler +// dials and returns a canned 200 JSON response. +type captureWorkspaceFilesRoundTripper struct { + gotURL *url.URL +} + +func (c *captureWorkspaceFilesRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + c.gotURL = req.URL + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(bytes.NewReader([]byte(`{"ok":true}`))), + }, nil +} + +// TestWorkspaceFiles_EffectivePrefixAndPortReachUpstream is the +// end-to-end regression: a controller configured with a non-default +// container prefix and a worker whose spec.env conflicts with the system +// console port must dial exactly http://{prefix}{name}:{effective-port} +// with root=workspace pinned. +func TestWorkspaceFiles_EffectivePrefixAndPortReachUpstream(t *testing.T) { + objs := kbWorkerFixture("market-team", "market-writer") + objs[1].(*v1beta1.Worker).Spec.Env = map[string]string{"AGENTTEAMS_CONSOLE_PORT": "9090"} + k8s := fake.NewClientBuilder().WithScheme(newProjectTestScheme(t)).WithRuntimeObjects(objs...).Build() + + h := NewWorkspaceFilesHandler(k8s, "default", "embedded", "acme-worker-") + rt := &captureWorkspaceFilesRoundTripper{} + h.http = &http.Client{Transport: rt} + + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, adminCaller(kbRequest("market-writer", "file-metadata", "path=MEMORY.md"))) + + if rec.Code != http.StatusOK { + t.Fatalf("status=%d, want 200; body=%s", rec.Code, rec.Body.String()) + } + want := "http://acme-worker-market-writer:8088/workspace/file-metadata?path=MEMORY.md&root=workspace" + if rt.gotURL.String() != want { + t.Fatalf("upstream URL=%s, want %s", rt.gotURL.String(), want) + } +} diff --git a/docs/usage/project-workflow-api.md b/docs/usage/project-workflow-api.md index 392cceafe..39ef08e0d 100644 --- a/docs/usage/project-workflow-api.md +++ b/docs/usage/project-workflow-api.md @@ -753,3 +753,56 @@ Error responses: | `404` | Worker not found / caller does not own it (existence hidden). | | `502` | Worker app unreachable, pre-2.1 checkpoint API, or upstream error. | | `503` | Kube mode (no stable worker pod DNS to proxy). | + +## Worker knowledge base (workspace files) endpoints + +The Controller proxies three read-only endpoints of each worker's QwenPaw +app (QwenPaw ≥ 2.1) so L2 humans and frontends can inspect a worker's +knowledge base — the long-term memory file `MEMORY.md`, the daily note +tree `memory/`, and the distilled knowledge tree `digest/`. + +| Endpoint | Meaning | +|:--|:--| +| `GET /api/v1/workers/{name}/workspace-files/tree` | Paginated listing of one knowledge directory: `?path=` (required, `memory` / `digest` or any subpath of them), optional `?cursor=` (opaque) and `?limit=` (1..500). Returns `{directory, entries[], has_more, next_cursor}`. | +| `GET /api/v1/workers/{name}/workspace-files/file-metadata` | `?path=` (required, an allowed knowledge file): `{etag, modified_at, path, preview_kind, size}`. | +| `GET /api/v1/workers/{name}/workspace-files/file-content` | `?path=` (required) plus optional `?offset=` (≥0) and `?limit=` (1..1048576): a bounded UTF-8 chunk `{content, eof, next_offset, truncated, etag, ...}` — continue with `offset=next_offset` while `truncated` is true. | + +- **Scope**: same worker read authorization as `GET /api/v1/workers/{name}` + and the checkpoint endpoints — team leaders / L2 humans only see workers + in their accessible teams; unknown or out-of-scope workers are hidden as + `404`. +- **Path allowlist (read-only knowledge boundary)**: only `MEMORY.md`, + `memory/**` and `digest/**` are addressable. Every other workspace + location — `SOUL.md`, `PROFILE.md`, `TODO.md`, `checkpoints/`, `skills/`, + and all dot directories (`.copaw/agent.json` carries the worker's + credentials) — is rejected with `400` before the request reaches the + worker. Roots match on the exact first segment, so `memories/` and + `memoryX/` are not prefixes of `memory/`. +- **Root pinned**: the QwenPaw `root=workspace` parameter (the agent's own + storage root, as opposed to `root=project`, the primary bound project + directory) is fixed server-side and is not part of the client query + surface. +- **Embedded mode only**: the endpoints proxy the worker's qwenpaw app + inside the shared docker network, using the effective container prefix + and the system-wins console port — the same address resolution as the + checkpoint endpoints. In kube mode they return `503`. +- **Version gate (passthrough 404)**: a worker running QwenPaw < 2.1 has no + workspace file router, so every request is an upstream `404` passed + through verbatim. To distinguish "worker too old" from "file missing", + probe `file-metadata?path=MEMORY.md` — that file exists in every + initialized QwenPaw workspace, so a `404` there means the worker is + pre-2.1 (or its workspace is not initialized) while other `404`s are + plain missing files. +- Forwarding is fixed-path (tree / file-metadata / file-content) with a + strict query whitelist — not a generic reverse proxy, and no write + endpoint of the workspace API is reachable. + +Error responses: + +| Code | Meaning | +|:--|:--| +| `400` | Invalid worker name / unsupported subpath or query parameter / path outside the knowledge allowlist / out-of-bounds `limit` or `offset`. | +| `404` | Worker not found or not in the caller's teams (existence hidden); or (passthrough) the file does not exist — see the version-gate probe above. | +| `409` / `416` | (passthrough) the file changed while being read / offset beyond end of file. | +| `502` | Worker app unreachable, or an upstream error (status echoed in the body). | +| `503` | Kube mode (no stable worker pod DNS to proxy). | diff --git a/docs/zh-cn/usage/project-workflow-api.md b/docs/zh-cn/usage/project-workflow-api.md index e17ba9a08..7f0f683c6 100644 --- a/docs/zh-cn/usage/project-workflow-api.md +++ b/docs/zh-cn/usage/project-workflow-api.md @@ -371,3 +371,46 @@ agt project complete demo-project-001 ``` 同样的 bearer 令牌转发适用（L2 人类用 Matrix 令牌）。 + +## Worker 知识库（工作区文件）端点 + +Controller 代理每个 worker 的 QwenPaw app（QwenPaw ≥ 2.1）的三个只读端点， +让 L2 人类与前端可以查看 worker 的知识库——长期记忆文件 `MEMORY.md`、 +日记目录树 `memory/` 与沉淀知识目录树 `digest/`。 + +| 端点 | 含义 | +|:--|:--| +| `GET /api/v1/workers/{name}/workspace-files/tree` | 分页列出某个知识目录：`?path=`（必填，`memory` / `digest` 或其子路径），可选 `?cursor=`（不透明串）与 `?limit=`（1..500）。返回 `{directory, entries[], has_more, next_cursor}`。 | +| `GET /api/v1/workers/{name}/workspace-files/file-metadata` | `?path=`（必填，允许的知识库文件）：`{etag, modified_at, path, preview_kind, size}`。 | +| `GET /api/v1/workers/{name}/workspace-files/file-content` | `?path=`（必填）加可选 `?offset=`（≥0）与 `?limit=`（1..1048576）：有界 UTF-8 分块 `{content, eof, next_offset, truncated, etag, ...}`——`truncated` 为真时用 `offset=next_offset` 续读。 | + +- **范围**：与 `GET /api/v1/workers/{name}` 相同的 worker 读授权——团队 + leader / L2 人类只能看自己可访问团队内的 worker；未知或越权 worker 一律 + 隐藏为 `404`。 +- **路径白名单（只读知识边界）**：只放行 `MEMORY.md`、`memory/**` 与 + `digest/**`。工作区内其他一切位置——`SOUL.md`、`PROFILE.md`、`TODO.md`、 + `checkpoints/`、`skills/`，以及所有 dot 目录（`.copaw/agent.json` 承载 + worker 凭据）——在请求到达 worker 之前即被 `400` 拒绝。根目录按完整首段 + 精确匹配，`memories/` 与 `memoryX/` 不构成 `memory/` 的前缀。 +- **root 固定**：QwenPaw 的 `root=workspace` 参数（agent 自身存储根，相对 + 于 `root=project` 即主绑定项目目录）由服务端固定，不属于客户端查询面。 +- **仅 embedded 模式**：端点经共享 docker 网络代理 worker 的 qwenpaw app， + 地址解析与 checkpoint 端点相同（生效容器前缀 + system-wins 控制台端口）。 + kube 模式返回 `503`。 +- **版本门（404 透传）**：worker 运行 QwenPaw < 2.1 时没有工作区文件 + 路由，所有请求均为上游 `404` 原样透传。区分"worker 版本过旧"与"文件不 + 存在"的方法：探测 `file-metadata?path=MEMORY.md`——该文件在每个已初始 + 化的 QwenPaw 工作区中都存在，因此这里的 `404` 表示 worker 为 2.1 以下 + （或工作区未初始化），其余 `404` 即普通文件缺失。 +- 转发为固定子路径（tree / file-metadata / file-content）+ 严格查询白名单 + ——不是通用反向代理，工作区 API 的任何写端点均不可达。 + +错误响应： + +| 码 | 含义 | +|:--|:--| +| `400` | worker 名非法 / 不支持的子路径或查询参数 / 路径不在知识白名单内 / `limit` 或 `offset` 越界。 | +| `404` | worker 不存在或不在调用方团队内（存在性隐藏）；或（透传）文件不存在——见上方版本门探测。 | +| `409` / `416` | （透传）读取期间文件被修改 / offset 超出文件末尾。 | +| `502` | worker app 不可达，或上游错误（状态码回显在 body 中）。 | +| `503` | kube 模式（无稳定的 worker pod DNS 可代理）。 | From 40978e5cf80be7e83df5157186c99c0e6f752029 Mon Sep 17 00:00:00 2001 From: LUOSENGWA Date: Tue, 1 Sep 2026 01:38:33 +0000 Subject: [PATCH 2/4] docs(usage): scope workspace file endpoints to QwenPaw-runtime workers --- docs/usage/project-workflow-api.md | 6 ++++++ docs/zh-cn/usage/project-workflow-api.md | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/docs/usage/project-workflow-api.md b/docs/usage/project-workflow-api.md index 39ef08e0d..0fae97341 100644 --- a/docs/usage/project-workflow-api.md +++ b/docs/usage/project-workflow-api.md @@ -793,6 +793,12 @@ tree `memory/`, and the distilled knowledge tree `digest/`. initialized QwenPaw workspace, so a `404` there means the worker is pre-2.1 (or its workspace is not initialized) while other `404`s are plain missing files. +- **Runtime scope**: the endpoints target workers running the QwenPaw app + (`qwenpaw` runtime). Workers on other runtimes have no QwenPaw workspace + API: when that runtime's app serves the console port the proxy passes its + response through verbatim (typically `404`); when nothing listens it + returns `502`. The MEMORY.md probe is therefore only meaningful for + QwenPaw workers. - Forwarding is fixed-path (tree / file-metadata / file-content) with a strict query whitelist — not a generic reverse proxy, and no write endpoint of the workspace API is reachable. diff --git a/docs/zh-cn/usage/project-workflow-api.md b/docs/zh-cn/usage/project-workflow-api.md index 7f0f683c6..4d997e5c8 100644 --- a/docs/zh-cn/usage/project-workflow-api.md +++ b/docs/zh-cn/usage/project-workflow-api.md @@ -402,6 +402,10 @@ Controller 代理每个 worker 的 QwenPaw app（QwenPaw ≥ 2.1）的三个只 存在"的方法：探测 `file-metadata?path=MEMORY.md`——该文件在每个已初始 化的 QwenPaw 工作区中都存在，因此这里的 `404` 表示 worker 为 2.1 以下 （或工作区未初始化），其余 `404` 即普通文件缺失。 +- **Runtime 范围**：端点面向运行 QwenPaw app 的 worker（`qwenpaw` + runtime）。其他 runtime 的 worker 没有 QwenPaw 工作区 API：该 runtime 的 + 应用若在服务 console 端口，代理原样透传其响应（通常 `404`）；无人监听 + 时返回 `502`。MEMORY.md 探测因此只对 QwenPaw worker 有意义。 - 转发为固定子路径（tree / file-metadata / file-content）+ 严格查询白名单 ——不是通用反向代理，工作区 API 的任何写端点均不可达。 From 0dbcb291881032050f6aeaab2fe7acf38b889ecb Mon Sep 17 00:00:00 2001 From: LUOSENGWA Date: Tue, 1 Sep 2026 10:28:31 +0000 Subject: [PATCH 3/4] feat(controller): add team-scoped knowledge base write access and file download MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - PUT /api/v1/workers/{name}/workspace-files/file-content: L1 writes any team; L2 humans write only own teams while Human CR workspaceFileAccess != "read" (default readwrite, L1-locked to read); team leaders stay read-only; cross-team hides as 404 (W8) - authorizer: new workspace-files-write action, deliberately authorizer-allowed cross-team for RoleHuman — the middleware resolves the worker's team, and a 403 there would leak worker existence (the read path hides cross-team as 404; the handler is the real boundary, pinned by authorizer tests); separate action so it does not collide with the L2 worker-update PR - If-Match mandatory for existing files (lost-update guard vs worker auto-append), forbidden for new files; 1 MiB write cap; empty content rejected; every write audit-logged - GET /api/v1/workers/{name}/workspace-files/file-download: bounded stream with attachment headers forwarded, same KB allowlist and scope - Human CRD gains workspaceFileAccess (read|readwrite); CRD synced to Helm - 41 tests green (24 read + 16 write/download + 1 checkpoint regression + 2 authorizer W8 cases); full controller suite green (1 pre-existing env-only failure: executor ZIP test needs unzip, present in CI) --- agentteams-controller/api/v1beta1/types.go | 10 + .../config/crd/humans.agentteams.io.yaml | 6 + .../internal/auth/authorizer.go | 38 +- .../internal/auth/authorizer_test.go | 6 + agentteams-controller/internal/server/http.go | 1 + .../internal/server/worker_workspace_files.go | 290 +++++++++++- .../server/worker_workspace_files_test.go | 437 ++++++++++++++++++ docs/usage/project-workflow-api.md | 56 ++- docs/zh-cn/usage/project-workflow-api.md | 43 +- .../agentteams/crds/humans.agentteams.io.yaml | 6 + 10 files changed, 841 insertions(+), 52 deletions(-) diff --git a/agentteams-controller/api/v1beta1/types.go b/agentteams-controller/api/v1beta1/types.go index edf522781..e67572e7d 100644 --- a/agentteams-controller/api/v1beta1/types.go +++ b/agentteams-controller/api/v1beta1/types.go @@ -585,6 +585,16 @@ type HumanSpec struct { AccessibleWorkers []string `json:"accessibleWorkers,omitempty"` IdentitySource *IdentitySourceSpec `json:"identitySource,omitempty"` Note string `json:"note,omitempty"` + // WorkspaceFileAccess controls what this human may do to the knowledge + // base files (workspace-files endpoints) of workers in their own teams: + // "read" (the default, including when empty) allows the read endpoints + // (tree / file-metadata / file-content / file-download); "readwrite" + // additionally allows PUT file-content. Write is an explicit opt-in so + // that a controller upgrade cannot silently grant pre-existing humans + // the new ability to modify worker knowledge files. Admin (L1) callers + // are never restricted, and team leaders always stay read-only on this + // API. + WorkspaceFileAccess string `json:"workspaceFileAccess,omitempty"` } type IdentitySourceSpec struct { diff --git a/agentteams-controller/config/crd/humans.agentteams.io.yaml b/agentteams-controller/config/crd/humans.agentteams.io.yaml index 63f64f809..2597cbbcc 100644 --- a/agentteams-controller/config/crd/humans.agentteams.io.yaml +++ b/agentteams-controller/config/crd/humans.agentteams.io.yaml @@ -50,6 +50,12 @@ spec: required: [issuer, subject] note: type: string + workspaceFileAccess: + type: string + enum: + - read + - readwrite + description: "Knowledge base file access for this human's own teams: read (default when empty) or readwrite (explicit write opt-in); L1 admin is never restricted, team leaders stay read-only" status: type: object properties: diff --git a/agentteams-controller/internal/auth/authorizer.go b/agentteams-controller/internal/auth/authorizer.go index 441ead89a..a124ff3bc 100644 --- a/agentteams-controller/internal/auth/authorizer.go +++ b/agentteams-controller/internal/auth/authorizer.go @@ -6,19 +6,20 @@ import "fmt" type Action string const ( - ActionCreate Action = "create" - ActionUpdate Action = "update" - ActionDelete Action = "delete" - ActionGet Action = "get" - ActionList Action = "list" - ActionWake Action = "wake" - ActionSleep Action = "sleep" - ActionEnsureReady Action = "ensure-ready" - ActionReady Action = "ready" - ActionSTS Action = "sts" - ActionStatus Action = "status" - ActionRefreshMatrixToken Action = "refresh-matrix-token" - ActionGateway Action = "gateway" + ActionCreate Action = "create" + ActionUpdate Action = "update" + ActionDelete Action = "delete" + ActionGet Action = "get" + ActionList Action = "list" + ActionWake Action = "wake" + ActionSleep Action = "sleep" + ActionEnsureReady Action = "ensure-ready" + ActionReady Action = "ready" + ActionSTS Action = "sts" + ActionStatus Action = "status" + ActionRefreshMatrixToken Action = "refresh-matrix-token" + ActionGateway Action = "gateway" + ActionWorkspaceFilesWrite Action = "workspace-files-write" ) // AuthzRequest describes the resource being accessed. @@ -114,6 +115,17 @@ func (a *Authorizer) authorizeHuman(caller *CallerIdentity, req AuthzRequest) er if req.Action == ActionUpdate { return a.requireSameTeam(caller, req) } + if req.Action == ActionWorkspaceFilesWrite { + // W3②-rw: L2 humans may write knowledge base files of workers + // in their own teams. Like ActionGet/ActionList this action is + // NOT rejected cross-team at the authorizer level: the + // middleware resolves the worker's team, and a 403 here would + // let a scoped caller probe which workers exist in other teams + // (W8 anti-probing). The handler is the real boundary — it + // hides cross-team workers as 404 and enforces the per-user + // workspaceFileAccess flag and the knowledge base allowlist. + return nil + } return deny(caller, req) default: diff --git a/agentteams-controller/internal/auth/authorizer_test.go b/agentteams-controller/internal/auth/authorizer_test.go index fb4b3f0c6..ca552defb 100644 --- a/agentteams-controller/internal/auth/authorizer_test.go +++ b/agentteams-controller/internal/auth/authorizer_test.go @@ -45,6 +45,12 @@ func TestAuthorizer_HumanScoped(t *testing.T) { {Action: ActionUpdate, ResourceKind: "worker", ResourceTeam: "market-team"}, {Action: ActionUpdate, ResourceKind: "worker"}, {Action: ActionGet, ResourceKind: "status"}, + // W3②-rw: the knowledge base write action is allowed at the + // authorizer level even cross-team (ResourceTeam filled by the + // middleware) — a denial here would leak worker existence via 403 + // (W8 anti-probing). The handler is the real boundary (404). + {Action: ActionWorkspaceFilesWrite, ResourceKind: "worker"}, + {Action: ActionWorkspaceFilesWrite, ResourceKind: "worker", ResourceTeam: "another-team"}, } for _, req := range allowed { if err := az.Authorize(caller, req); err != nil { diff --git a/agentteams-controller/internal/server/http.go b/agentteams-controller/internal/server/http.go index 11ac49bd0..6b5539b86 100644 --- a/agentteams-controller/internal/server/http.go +++ b/agentteams-controller/internal/server/http.go @@ -130,6 +130,7 @@ func NewHTTPServer(addr string, deps ServerDeps) *HTTPServer { // --- Worker knowledge base files (read-only MEMORY.md / memory/** / digest/** inspection; proxy to the worker's qwenpaw app) --- wfh := NewWorkspaceFilesHandler(deps.Client, deps.Namespace, deps.KubeMode, deps.ContainerPrefix) mux.Handle("GET /api/v1/workers/{name}/workspace-files/{sub}", mw.RequireAuthz(authpkg.ActionGet, "worker", nameFn)(http.HandlerFunc(wfh.proxyWorkspaceFiles))) + mux.Handle("PUT /api/v1/workers/{name}/workspace-files/file-content", mw.RequireAuthz(authpkg.ActionWorkspaceFilesWrite, "worker", nameFn)(http.HandlerFunc(wfh.proxyWorkspaceFileWrite))) // W-PR-2: human intervention + lifecycle (write endpoints). All writes go // through RequireAuthz ActionUpdate + "project" so the authorizer's diff --git a/agentteams-controller/internal/server/worker_workspace_files.go b/agentteams-controller/internal/server/worker_workspace_files.go index b95eb4ebb..b66ecf329 100644 --- a/agentteams-controller/internal/server/worker_workspace_files.go +++ b/agentteams-controller/internal/server/worker_workspace_files.go @@ -1,16 +1,17 @@ package server -// Worker knowledge base file inspection -// (GET /api/v1/workers/{name}/workspace-files/...). +// Worker knowledge base file inspection and management +// (/api/v1/workers/{name}/workspace-files/...). // -// Each worker's qwenpaw app (QwenPaw >= 2.1) exposes read-only workspace -// file endpoints on :8088 (0.0.0.0 listen; no auth in worker context -// because no console user is registered): /workspace/tree (paginated -// directory listing), /workspace/file-metadata and /workspace/file-content -// (bounded UTF-8 chunk reads). The Controller proxies those three -// read-only subpaths so L2 humans and the workbench plugin can inspect a -// worker's knowledge base (MEMORY.md, memory/**, digest/**) without -// reaching into the docker network directly. +// Each worker's qwenpaw app (QwenPaw >= 2.1) exposes workspace file +// endpoints on :8088 (0.0.0.0 listen; no auth in worker context because no +// console user is registered): /workspace/tree (paginated directory +// listing), /workspace/file-metadata, /workspace/file-content (bounded +// UTF-8 chunk reads and ETag-guarded writes), and /workspace/file-download +// (bounded stream). The Controller proxies those four subpaths so L2 humans +// and the workbench plugin can inspect — and, where the Human CR grants it, +// update — a worker's knowledge base (MEMORY.md, memory/**, digest/**) +// without reaching into the docker network directly. // // Embedded mode only: the worker app is reachable by container name inside // the shared docker network. The effective container name prefix comes from @@ -44,12 +45,27 @@ package server // opposed to root=project, the primary bound project directory) is pinned // server-side and is never part of the client-facing query surface. // -// Fixed-path forwarding only (tree / file-metadata / file-content, plus -// their whitelisted queries) — never a generic reverse proxy, and never a -// write endpoint, so the attack surface is limited to three read-only -// QwenPaw endpoints. +// Fixed-path forwarding only (tree / file-metadata / file-content GET+PUT / +// file-download, plus their whitelisted queries) — never a generic reverse +// proxy — so the attack surface is limited to four QwenPaw endpoints. +// +// Write scope (PUT file-content, introduced with the team-scoped write +// access): admin/manager (L1) may write any worker's knowledge base; an L2 +// human may write only workers in their own teams, and only when their +// Human CR carries workspaceFileAccess="readwrite" — write is an explicit +// opt-in. An empty/missing value means "read" (never "readwrite"), so a +// controller upgrade cannot silently grant pre-existing L2 humans the new +// ability to modify worker knowledge files; L1 opts a user into writing by +// setting the field to "readwrite". Team leaders stay read-only on this +// API. The proxy enforces optimistic concurrency for +// existing files (If-Match is mandatory — the worker auto-appends to its +// memory files, so a skipped ETag check is a lost update) and caps the +// write body at 1 MiB, mirroring the read chunk cap. Every write is +// audit-logged. import ( + "context" + "encoding/json" "errors" "fmt" "io" @@ -65,6 +81,7 @@ import ( "github.com/agentscope-ai/AgentTeams/agentteams-controller/internal/service" apierrors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" ) const ( @@ -90,12 +107,19 @@ const ( ) // workspaceFileSubpaths is the fixed whitelist of forwardable QwenPaw -// endpoints. Write endpoints (file-content PUT, file-upload) and binary -// streaming (file-download) are deliberately absent. +// endpoints. file-content is served for both GET (chunked read) and PUT +// (ETag-guarded write); everything else the upstream app exposes +// (file-upload multipart, running-config, ...) stays deliberately absent. var workspaceFileSubpaths = map[string]bool{ "tree": true, "file-metadata": true, "file-content": true, + "file-download": true, +} + +// workspaceFileWriteSubpaths are the subpaths served by the PUT handler. +var workspaceFileWriteSubpaths = map[string]bool{ + "file-content": true, } // kbFileRoots are the top-level single files addressable by the @@ -137,6 +161,14 @@ func validateKbPath(path string, forFile bool) error { if forFile { for _, root := range kbFileRoots { if first == root { + // File roots are single top-level files (MEMORY.md) — + // exactly one segment. Rejecting deeper paths keeps the + // allowlist in line with the documented contract (MEMORY.md + // is a file, not a directory); nested files must live under + // the memory/ or digest/ directory roots. + if len(segments) != 1 { + return errors.New("file roots are single top-level files (e.g. MEMORY.md); use memory/ or digest/ for nested paths") + } return nil } } @@ -163,6 +195,8 @@ func validateWorkspaceFilesQuery(sub string, q url.Values) (string, error) { allowed = map[string]bool{"path": true} case "file-content": allowed = map[string]bool{"path": true, "offset": true, "limit": true} + case "file-download": + allowed = map[string]bool{"path": true} } for key, vals := range q { if !allowed[key] { @@ -338,7 +372,17 @@ func (h *WorkspaceFilesHandler) proxyWorkspaceFiles(w http.ResponseWriter, r *ht switch resp.StatusCode { case http.StatusOK: - w.Header().Set("Content-Type", "application/json") + if sub == "file-download" { + // Stream the file verbatim with its attachment headers — + // the client saves it under the worker's file name. + for _, h := range []string{"Content-Disposition", "Content-Length", "ETag", "Accept-Ranges", "Content-Type"} { + if v := resp.Header.Get(h); v != "" { + w.Header().Set(h, v) + } + } + } else { + w.Header().Set("Content-Type", "application/json") + } w.WriteHeader(http.StatusOK) _, _ = io.Copy(w, resp.Body) case http.StatusBadRequest, http.StatusNotFound, http.StatusConflict, http.StatusRequestedRangeNotSatisfiable: @@ -355,3 +399,215 @@ func (h *WorkspaceFilesHandler) proxyWorkspaceFiles(w http.ResponseWriter, r *ht httputil.WriteError(w, http.StatusBadGateway, fmt.Sprintf("workspace files API error (status %d): %s", resp.StatusCode, string(body))) } } + +// upstreamKBFileExists probes the worker's file-metadata endpoint to decide +// the If-Match policy before a write. exists=true means the file is present +// (a write must carry a matching If-Match); exists=false means the file is +// absent (a write must NOT carry If-Match — upstream treats an ETag on a +// missing file as a conflict). +func (h *WorkspaceFilesHandler) upstreamKBFileExists(ctx context.Context, baseURL, path string) (bool, error) { + target := baseURL + "/workspace/file-metadata?path=" + url.QueryEscape(path) + "&root=workspace" + req, err := http.NewRequestWithContext(ctx, http.MethodGet, target, nil) + if err != nil { + return false, err + } + resp, err := h.http.Do(req) + if err != nil { + return false, err + } + defer resp.Body.Close() + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 4096)) + switch { + case resp.StatusCode == http.StatusOK: + return true, nil + case resp.StatusCode == http.StatusNotFound: + return false, nil + default: + return false, fmt.Errorf("file-metadata probe returned status %d", resp.StatusCode) + } +} + +// proxyWorkspaceFileWrite handles PUT +// /api/v1/workers/{name}/workspace-files/file-content. +// +// Write scope: admin/manager (L1) may write any worker's knowledge base; an +// L2 human may write only workers in their own teams while their Human CR +// carries workspaceFileAccess="readwrite" (explicit opt-in — an +// empty/missing value means "read"). Team leaders stay read-only on this +// API. Out-of-scope workers hide as 404 (same W8 anti-probing rule as the +// read path). +// +// Concurrency: the proxy first probes file-metadata; for an existing file +// the If-Match header is mandatory (the worker auto-appends to its memory +// files, so a write without an ETag check is a lost update), and for a new +// file it must be absent (upstream rejects an ETag on a missing file). The +// write body is capped at kbMaxFileLimit (1 MiB, the read chunk cap). +func (h *WorkspaceFilesHandler) proxyWorkspaceFileWrite(w http.ResponseWriter, r *http.Request) { + name := r.PathValue("name") + sub := r.PathValue("sub") + if name == "" || !workerNamePattern.MatchString(name) { + httputil.WriteError(w, http.StatusBadRequest, "worker name is required and must be a valid DNS label") + return + } + if !workspaceFileWriteSubpaths[sub] { + httputil.WriteError(w, http.StatusBadRequest, "unsupported workspace file write subpath") + return + } + if h.kubeMode != "embedded" { + httputil.WriteError(w, http.StatusServiceUnavailable, "worker workspace file inspection requires embedded mode") + return + } + + var worker v1beta1.Worker + if err := h.client.Get(r.Context(), client.ObjectKey{Name: name, Namespace: h.namespace}, &worker); err != nil { + if apierrors.IsNotFound(err) { + httputil.WriteError(w, http.StatusNotFound, "worker not found") + return + } + writeK8sError(w, "write worker workspace file", err) + return + } + // Same team-scope chain as the read path: findTeamMember's second return + // value is the member name, not the team name — the scope check must + // compare against the Team CR name. + teamObj, _, _, err := findTeamMember(r.Context(), h.client, h.namespace, name) + if err != nil { + writeK8sError(w, "write worker workspace file", err) + return + } + teamName := "" + if teamObj != nil { + teamName = teamObj.Name + } + caller := authpkg.CallerFromContext(r.Context()) + if caller == nil { + httputil.WriteError(w, http.StatusForbidden, "caller identity required") + return + } + if caller.Role == authpkg.RoleTeamLeader || caller.Role == authpkg.RoleHuman { + if !caller.TeamMatches(teamName) { + httputil.WriteError(w, http.StatusNotFound, "worker not found") + return + } + } + // Write-role boundary (see the function comment). + switch caller.Role { + case authpkg.RoleAdmin, authpkg.RoleManager: + // full access + case authpkg.RoleTeamLeader: + httputil.WriteError(w, http.StatusForbidden, "team leaders can read workspace files but not write them") + return + case authpkg.RoleHuman: + var human v1beta1.Human + if err := h.client.Get(r.Context(), client.ObjectKey{Name: caller.Username, Namespace: h.namespace}, &human); err != nil { + httputil.WriteError(w, http.StatusForbidden, "workspace file access cannot be verified for this user") + return + } + // Write is an explicit opt-in: an empty or missing + // workspaceFileAccess means "read". Defaulting to readwrite would + // silently grant every pre-existing L2 human a new ability to + // modify worker knowledge files on controller upgrade. + if !strings.EqualFold(human.Spec.WorkspaceFileAccess, "readwrite") { + httputil.WriteError(w, http.StatusForbidden, "workspace file write access requires workspaceFileAccess=readwrite (explicit opt-in)") + return + } + default: + httputil.WriteError(w, http.StatusForbidden, "workspace file write denied for this caller role") + return + } + + q := r.URL.Query() + if len(q) != 1 || len(q["path"]) != 1 || q.Get("path") == "" { + httputil.WriteError(w, http.StatusBadRequest, "the only supported query parameter is path (exactly once)") + return + } + path := q.Get("path") + if err := validateKbPath(path, true); err != nil { + httputil.WriteError(w, http.StatusBadRequest, err.Error()) + return + } + + // Body: {"content": string}, capped at the 1 MiB write limit. + raw, err := io.ReadAll(io.LimitReader(r.Body, kbMaxFileLimit+1)) + if err != nil { + httputil.WriteError(w, http.StatusBadRequest, "read request body: "+err.Error()) + return + } + if len(raw) > kbMaxFileLimit { + httputil.WriteError(w, http.StatusBadRequest, "content exceeds the 1 MiB write limit") + return + } + var payload struct { + Content string `json:"content"` + } + if err := json.Unmarshal(raw, &payload); err != nil { + httputil.WriteError(w, http.StatusBadRequest, "request body must be a JSON object with a content string field") + return + } + if payload.Content == "" { + // An empty write would truncate the worker's knowledge file — + // the write API is for updating knowledge, not blanking it. + httputil.WriteError(w, http.StatusBadRequest, "content must be a non-empty string") + return + } + + // If-Match policy: probe existence first (the worker app is the source + // of truth for what is on disk). + baseURL := h.workerBaseURL(name, worker.Spec.Env) + ifMatch := r.Header.Get("If-Match") + exists, err := h.upstreamKBFileExists(r.Context(), baseURL, path) + if err != nil { + httputil.WriteError(w, http.StatusBadGateway, "probe worker workspace file: "+err.Error()) + return + } + if exists && ifMatch == "" { + httputil.WriteError(w, http.StatusBadRequest, "If-Match header is required to update an existing file") + return + } + if !exists && ifMatch != "" { + httputil.WriteError(w, http.StatusBadRequest, "If-Match must not be sent when creating a new file") + return + } + + target := baseURL + "/workspace/file-content?path=" + url.QueryEscape(path) + "&root=workspace" + body, _ := json.Marshal(map[string]string{"content": payload.Content}) + req, err := http.NewRequestWithContext(r.Context(), http.MethodPut, target, strings.NewReader(string(body))) + if err != nil { + httputil.WriteError(w, http.StatusInternalServerError, "build workspace file write request: "+err.Error()) + return + } + req.Header.Set("Content-Type", "application/json") + if ifMatch != "" { + req.Header.Set("If-Match", ifMatch) + } + resp, err := h.http.Do(req) + if err != nil { + httputil.WriteError(w, http.StatusBadGateway, "worker workspace API unreachable") + return + } + defer resp.Body.Close() + + switch resp.StatusCode { + case http.StatusOK: + bodyOut, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write(bodyOut) + if logger := log.FromContext(r.Context()).WithName("workspace-files"); logger.Enabled() { + logger.Info("knowledge base file written", + "worker", name, "path", path, "caller", caller.Username, + "role", caller.Role, "bytes", len(payload.Content), "create", !exists) + } + case http.StatusBadRequest, http.StatusNotFound, http.StatusConflict, http.StatusUnprocessableEntity: + // Pass through verbatim: invalid path (400), file vanished between + // probe and write (404), ETag mismatch — file changed on disk (409), + // or content rejected upstream (422). + bodyOut, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(resp.StatusCode) + _, _ = w.Write(bodyOut) + default: + bodyOut, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) + httputil.WriteError(w, http.StatusBadGateway, fmt.Sprintf("workspace files API error (status %d): %s", resp.StatusCode, string(bodyOut))) + } +} diff --git a/agentteams-controller/internal/server/worker_workspace_files_test.go b/agentteams-controller/internal/server/worker_workspace_files_test.go index 7ddd8ec47..46217791a 100644 --- a/agentteams-controller/internal/server/worker_workspace_files_test.go +++ b/agentteams-controller/internal/server/worker_workspace_files_test.go @@ -12,6 +12,7 @@ import ( v1beta1 "github.com/agentscope-ai/AgentTeams/agentteams-controller/api/v1beta1" authpkg "github.com/agentscope-ai/AgentTeams/agentteams-controller/internal/auth" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -240,6 +241,7 @@ func TestWorkspaceFiles_RejectsNonKB(t *testing.T) { "memories/x.md", "memoryX/x.md", "digestX/x.md", + "MEMORY.md/foo", "memory/a/b/c/d", "", } { @@ -585,3 +587,438 @@ func TestWorkspaceFiles_EffectivePrefixAndPortReachUpstream(t *testing.T) { t.Fatalf("upstream URL=%s, want %s", rt.gotURL.String(), want) } } + +// ── Knowledge base write (PUT file-content) + file-download tests ───────── + +// kbWriteRequest builds a PUT request against the workspace-files route with +// a JSON body and an optional If-Match header. +func kbWriteRequest(name, sub, query, body, ifMatch string) *http.Request { + target := "/api/v1/workers/placeholder/workspace-files/" + sub + if query != "" { + target += "?" + query + } + req := httptest.NewRequest(http.MethodPut, target, strings.NewReader(body)) + req.SetPathValue("name", name) + req.SetPathValue("sub", sub) + if ifMatch != "" { + req.Header.Set("If-Match", ifMatch) + } + return req +} + +// kbHumanFixture builds a Human CR (L2 by default). +func kbHumanFixture(name string, teams []string, access string) *v1beta1.Human { + return &v1beta1.Human{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, + Spec: v1beta1.HumanSpec{ + DisplayName: name, + PermissionLevel: 2, + AccessibleTeams: teams, + WorkspaceFileAccess: access, + }, + } +} + +// kbWriteUpstream mimics the worker qwenpaw app for write tests: a +// configurable file-metadata probe answer and a PUT capture that verifies +// the forwarded body / If-Match. +func kbWriteUpstream(t *testing.T, exists bool, wantIfMatch string) (*httptest.Server, *int) { + t.Helper() + putCalls := 0 + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/workspace/file-metadata"): + if exists { + _, _ = w.Write([]byte(`{"path":"memory/t.md","size":6,"modified":1,"etag":"et-1"}`)) + } else { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"detail":"File not found"}`)) + } + case r.Method == http.MethodPut && strings.HasPrefix(r.URL.Path, "/workspace/file-content"): + putCalls++ + body, _ := io.ReadAll(r.Body) + if r.Header.Get("If-Match") != wantIfMatch { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"detail":"unexpected If-Match"}`)) + return + } + if !strings.Contains(string(body), `"content":"hello-kb"`) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"detail":"unexpected content"}`)) + return + } + _, _ = w.Write([]byte(`{"etag":"et-2","path":"memory/t.md","size":9}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + return upstream, &putCalls +} + +// TestWorkspaceFilesWrite_CreateNewFile: L2 human in scope creates a new +// file (no If-Match — upstream forbids an ETag on a missing file). +func TestWorkspaceFilesWrite_CreateNewFile(t *testing.T) { + upstream, putCalls := kbWriteUpstream(t, false, "") + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, + append(kbWorkerFixture("market-team", "market-writer"), + kbHumanFixture("maizong", []string{"market-team"}, "readwrite"))...) + req := withCaller(kbWriteRequest("market-writer", "file-content", "path=memory/t.md", `{"content":"hello-kb"}`, ""), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "maizong", Teams: []string{"market-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s, want 200 for in-scope L2 human create", rec.Code, rec.Body.String()) + } + if *putCalls != 1 { + t.Fatalf("upstream PUT calls=%d, want 1", *putCalls) + } + if !strings.Contains(rec.Body.String(), `"etag":"et-2"`) { + t.Fatalf("body=%s, want upstream etag passthrough", rec.Body.String()) + } +} + +// TestWorkspaceFilesWrite_UpdateWithETag: existing file + matching If-Match +// forwards the ETag to the worker app. +func TestWorkspaceFilesWrite_UpdateWithETag(t *testing.T) { + upstream, _ := kbWriteUpstream(t, true, "et-1") + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, + append(kbWorkerFixture("market-team", "market-writer"), + kbHumanFixture("maizong", []string{"market-team"}, "readwrite"))...) + req := withCaller(kbWriteRequest("market-writer", "file-content", "path=memory/t.md", `{"content":"hello-kb"}`, "et-1"), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "maizong", Teams: []string{"market-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s, want 200 for ETag-matched update", rec.Code, rec.Body.String()) + } +} + +// TestWorkspaceFilesWrite_ExistingWithoutIfMatchRejected: the proxy must not +// let a client skip the optimistic-concurrency check on an existing file +// (the worker auto-appends to its memory files — a bare overwrite is a lost +// update). +func TestWorkspaceFilesWrite_ExistingWithoutIfMatchRejected(t *testing.T) { + upstream, putCalls := kbWriteUpstream(t, true, "") + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, + append(kbWorkerFixture("market-team", "market-writer"), + kbHumanFixture("maizong", []string{"market-team"}, "readwrite"))...) + req := withCaller(kbWriteRequest("market-writer", "file-content", "path=memory/t.md", `{"content":"hello-kb"}`, ""), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "maizong", Teams: []string{"market-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status=%d, want 400 (If-Match required for existing file)", rec.Code) + } + if *putCalls != 0 { + t.Fatalf("upstream PUT calls=%d, want 0 (no write without ETag check)", *putCalls) + } +} + +// TestWorkspaceFilesWrite_NewFileWithIfMatchRejected: an ETag on a missing +// file is a client error (upstream would 409; the proxy rejects earlier). +func TestWorkspaceFilesWrite_NewFileWithIfMatchRejected(t *testing.T) { + upstream, putCalls := kbWriteUpstream(t, false, "") + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, + append(kbWorkerFixture("market-team", "market-writer"), + kbHumanFixture("maizong", []string{"market-team"}, "readwrite"))...) + req := withCaller(kbWriteRequest("market-writer", "file-content", "path=memory/t.md", `{"content":"hello-kb"}`, "et-1"), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "maizong", Teams: []string{"market-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status=%d, want 400 (no If-Match on new file)", rec.Code) + } + if *putCalls != 0 { + t.Fatalf("upstream PUT calls=%d, want 0", *putCalls) + } +} + +// TestWorkspaceFilesWrite_ETagConflictPassthrough: the worker changed the +// file between the client's read and write — upstream 409 passes through. +func TestWorkspaceFilesWrite_ETagConflictPassthrough(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case strings.HasPrefix(r.URL.Path, "/workspace/file-metadata"): + _, _ = w.Write([]byte(`{"path":"memory/t.md","size":6,"modified":1,"etag":"et-1"}`)) + default: // the PUT + w.WriteHeader(http.StatusConflict) + _, _ = w.Write([]byte(`{"detail":"File changed on disk"}`)) + } + })) + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, + append(kbWorkerFixture("market-team", "market-writer"), + kbHumanFixture("maizong", []string{"market-team"}, "readwrite"))...) + req := withCaller(kbWriteRequest("market-writer", "file-content", "path=memory/t.md", `{"content":"hello-kb"}`, "stale-et"), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "maizong", Teams: []string{"market-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, req) + + if rec.Code != http.StatusConflict { + t.Fatalf("status=%d, want 409 passthrough", rec.Code) + } + if !strings.Contains(rec.Body.String(), "File changed on disk") { + t.Fatalf("body=%s, want upstream conflict detail", rec.Body.String()) + } +} + +// TestWorkspaceFilesWrite_CrossTeamHidden: W8 — cross-team writes hide the +// worker as 404 (existence must not be probeable). +func TestWorkspaceFilesWrite_CrossTeamHidden(t *testing.T) { + upstream, putCalls := kbWriteUpstream(t, false, "") + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, + append(kbWorkerFixture("market-team", "market-writer"), + kbHumanFixture("sunzong", []string{"biz-team"}, ""))...) + req := withCaller(kbWriteRequest("market-writer", "file-content", "path=memory/t.md", `{"content":"hello-kb"}`, ""), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "sunzong", Teams: []string{"biz-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status=%d, want 404 for cross-team write (W8)", rec.Code) + } + if *putCalls != 0 { + t.Fatalf("upstream PUT calls=%d, want 0", *putCalls) + } +} + +// TestWorkspaceFilesWrite_ReadOnlyHumanDenied: L1 locked the user to +// read-only via workspaceFileAccess="read" — in-scope writes get 403. +func TestWorkspaceFilesWrite_ReadOnlyHumanDenied(t *testing.T) { + upstream, putCalls := kbWriteUpstream(t, false, "") + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, + append(kbWorkerFixture("market-team", "market-writer"), + kbHumanFixture("maizong", []string{"market-team"}, "read"))...) + req := withCaller(kbWriteRequest("market-writer", "file-content", "path=memory/t.md", `{"content":"hello-kb"}`, ""), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "maizong", Teams: []string{"market-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("status=%d, want 403 for read-only human", rec.Code) + } + if *putCalls != 0 { + t.Fatalf("upstream PUT calls=%d, want 0", *putCalls) + } +} + +// TestWorkspaceFilesWrite_DefaultAccessDenied is the upgrade regression the +// review required: a Human CR with no workspaceFileAccess field at all (the +// shape of every pre-existing L2 human right after a controller upgrade) +// must NOT be able to write — empty means "read", write is an explicit +// opt-in. +func TestWorkspaceFilesWrite_DefaultAccessDenied(t *testing.T) { + upstream, putCalls := kbWriteUpstream(t, false, "") + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, + append(kbWorkerFixture("market-team", "market-writer"), + kbHumanFixture("maizong", []string{"market-team"}, ""))...) + req := withCaller(kbWriteRequest("market-writer", "file-content", "path=memory/t.md", `{"content":"hello-kb"}`, ""), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "maizong", Teams: []string{"market-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("status=%d body=%s, want 403 for empty workspaceFileAccess (upgrade default = read)", rec.Code, rec.Body.String()) + } + if *putCalls != 0 { + t.Fatalf("upstream PUT calls=%d, want 0", *putCalls) + } +} + +// TestWorkspaceFilesWrite_LeaderDenied: team leaders stay read-only on the +// write API (their KB management surface is the chat/tools path, not REST). +func TestWorkspaceFilesWrite_LeaderDenied(t *testing.T) { + upstream, putCalls := kbWriteUpstream(t, false, "") + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + req := withCaller(kbWriteRequest("market-writer", "file-content", "path=memory/t.md", `{"content":"hello-kb"}`, ""), + &authpkg.CallerIdentity{Role: authpkg.RoleTeamLeader, Username: "market-writer", Team: "market-team"}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("status=%d, want 403 for in-scope team leader", rec.Code) + } + if *putCalls != 0 { + t.Fatalf("upstream PUT calls=%d, want 0", *putCalls) + } +} + +// TestWorkspaceFilesWrite_AdminAllowed: L1 (admin) may write any team's +// knowledge base — no Human CR needed, no team scope. +func TestWorkspaceFilesWrite_AdminAllowed(t *testing.T) { + upstream, _ := kbWriteUpstream(t, false, "") + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, adminCaller(kbWriteRequest("market-writer", "file-content", "path=memory/t.md", `{"content":"hello-kb"}`, ""))) + + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s, want 200 for admin", rec.Code, rec.Body.String()) + } +} + +// TestWorkspaceFilesWrite_OverSizeRejected: the 1 MiB write cap is enforced +// before the worker app is touched. +func TestWorkspaceFilesWrite_OverSizeRejected(t *testing.T) { + upstream, putCalls := kbWriteUpstream(t, false, "") + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, + append(kbWorkerFixture("market-team", "market-writer"), + kbHumanFixture("maizong", []string{"market-team"}, "readwrite"))...) + big := strings.Repeat("x", 1024*1024+1) + req := withCaller(kbWriteRequest("market-writer", "file-content", "path=memory/t.md", `{"content":"`+big+`"}`, ""), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "maizong", Teams: []string{"market-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status=%d, want 400 for over-size body", rec.Code) + } + if *putCalls != 0 { + t.Fatalf("upstream PUT calls=%d, want 0", *putCalls) + } +} + +// TestWorkspaceFilesWrite_SensitivePathRejected: the knowledge base +// allowlist applies to writes exactly as to reads (SOUL.md is the team +// owner's domain — never writable through this API). +func TestWorkspaceFilesWrite_SensitivePathRejected(t *testing.T) { + upstream, putCalls := kbWriteUpstream(t, true, "") + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, + append(kbWorkerFixture("market-team", "market-writer"), + kbHumanFixture("maizong", []string{"market-team"}, "readwrite"))...) + for _, path := range []string{"SOUL.md", "PROFILE.md", "skills/x/SKILL.md", ".copaw/agent.json", "memory/../../SOUL.md", "MEMORY.md/foo"} { + req := withCaller(kbWriteRequest("market-writer", "file-content", "path="+url.QueryEscape(path), `{"content":"hello-kb"}`, ""), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "maizong", Teams: []string{"market-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, req) + if rec.Code != http.StatusBadRequest { + t.Fatalf("path=%s status=%d, want 400 (allowlist)", path, rec.Code) + } + } + if *putCalls != 0 { + t.Fatalf("upstream PUT calls=%d, want 0", *putCalls) + } +} + +// TestWorkspaceFilesWrite_KubeModeUnavailable: uniform 503 in kube mode. +func TestWorkspaceFilesWrite_KubeModeUnavailable(t *testing.T) { + h := newTestWorkspaceFilesHandler(t, "kube", nil, kbWorkerFixture("market-team", "market-writer")...) + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, adminCaller(kbWriteRequest("market-writer", "file-content", "path=memory/t.md", `{"content":"hello-kb"}`, ""))) + if rec.Code != http.StatusServiceUnavailable { + t.Fatalf("status=%d, want 503 in kube mode", rec.Code) + } +} + +// TestWorkspaceFilesWrite_UnknownSubpathAndBody: only file-content is a +// write subpath; the body must be {"content": string}. +func TestWorkspaceFilesWrite_UnknownSubpathAndBody(t *testing.T) { + h := newTestWorkspaceFilesHandler(t, "embedded", nil, kbWorkerFixture("market-team", "market-writer")...) + for sub, body := range map[string]string{"file-metadata": `{}`, "file-download": `{}`, "tree": `{}`} { + rec := httptest.NewRecorder() + h.proxyWorkspaceFileWrite(rec, adminCaller(kbWriteRequest("market-writer", sub, "path=memory", body, ""))) + if rec.Code != http.StatusBadRequest { + t.Fatalf("sub=%s status=%d, want 400 (not a write subpath)", sub, rec.Code) + } + } + upstream, _ := kbWriteUpstream(t, false, "") + defer upstream.Close() + h2 := newTestWorkspaceFilesHandler(t, "embedded", upstream, + append(kbWorkerFixture("market-team", "market-writer"), + kbHumanFixture("maizong", []string{"market-team"}, "readwrite"))...) + for _, body := range []string{`[]`, `{"noContent":true}`, `not-json`} { + req := withCaller(kbWriteRequest("market-writer", "file-content", "path=memory/t.md", body, ""), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "maizong", Teams: []string{"market-team"}}) + rec := httptest.NewRecorder() + h2.proxyWorkspaceFileWrite(rec, req) + if rec.Code != http.StatusBadRequest { + t.Fatalf("body=%q status=%d, want 400 (invalid body)", body, rec.Code) + } + } +} + +// TestWorkspaceFilesDownload_InScopeAllowed: file-download streams with the +// attachment headers forwarded verbatim. +func TestWorkspaceFilesDownload_InScopeAllowed(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.HasPrefix(r.URL.Path, "/workspace/file-download") { + w.WriteHeader(http.StatusNotFound) + return + } + w.Header().Set("Content-Type", "text/markdown") + w.Header().Set("Content-Disposition", `attachment; filename="MEMORY.md"`) + w.Header().Set("Content-Length", "11") + w.Header().Set("ETag", `"dl-1"`) + _, _ = w.Write([]byte("memory-body")) + })) + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + req := withCaller(kbRequest("market-writer", "file-download", "path=MEMORY.md"), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "maizong", Teams: []string{"market-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status=%d, want 200 for in-scope download", rec.Code) + } + if got := rec.Header().Get("Content-Disposition"); got != `attachment; filename="MEMORY.md"` { + t.Fatalf("Content-Disposition=%q, want attachment header forwarded", got) + } + if got := rec.Header().Get("Content-Type"); got != "text/markdown" { + t.Fatalf("Content-Type=%q, want upstream media type (not forced JSON)", got) + } + if rec.Body.String() != "memory-body" { + t.Fatalf("body=%q", rec.Body.String()) + } +} + +// TestWorkspaceFilesDownload_CrossTeamHidden: W8 applies to downloads. +func TestWorkspaceFilesDownload_CrossTeamHidden(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("secret")) + })) + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + req := withCaller(kbRequest("market-writer", "file-download", "path=MEMORY.md"), + &authpkg.CallerIdentity{Role: authpkg.RoleHuman, Username: "sunzong", Teams: []string{"biz-team"}}) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status=%d, want 404 for cross-team download (W8)", rec.Code) + } +} + +// TestWorkspaceFilesDownload_SensitivePathRejected: the allowlist covers +// downloads — agent.json (Matrix token) must never stream out. +func TestWorkspaceFilesDownload_SensitivePathRejected(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"token":"leak"}`)) + })) + defer upstream.Close() + h := newTestWorkspaceFilesHandler(t, "embedded", upstream, kbWorkerFixture("market-team", "market-writer")...) + for _, path := range []string{".copaw/agent.json", "MEMORY.md/foo"} { + req := adminCaller(kbRequest("market-writer", "file-download", "path="+url.QueryEscape(path))) + rec := httptest.NewRecorder() + h.proxyWorkspaceFiles(rec, req) + if rec.Code != http.StatusBadRequest { + t.Fatalf("path=%s status=%d, want 400 (allowlist) even for admin", path, rec.Code) + } + } +} diff --git a/docs/usage/project-workflow-api.md b/docs/usage/project-workflow-api.md index 0fae97341..454cb45d8 100644 --- a/docs/usage/project-workflow-api.md +++ b/docs/usage/project-workflow-api.md @@ -756,28 +756,58 @@ Error responses: ## Worker knowledge base (workspace files) endpoints -The Controller proxies three read-only endpoints of each worker's QwenPaw -app (QwenPaw ≥ 2.1) so L2 humans and frontends can inspect a worker's -knowledge base — the long-term memory file `MEMORY.md`, the daily note -tree `memory/`, and the distilled knowledge tree `digest/`. +The Controller proxies four endpoints of each worker's QwenPaw app +(QwenPaw ≥ 2.1) so L2 humans and frontends can inspect — and, where the +caller's Human CR permits, update — a worker's knowledge base: the +long-term memory file `MEMORY.md`, the daily note tree `memory/`, and the +distilled knowledge tree `digest/`. | Endpoint | Meaning | |:--|:--| | `GET /api/v1/workers/{name}/workspace-files/tree` | Paginated listing of one knowledge directory: `?path=` (required, `memory` / `digest` or any subpath of them), optional `?cursor=` (opaque) and `?limit=` (1..500). Returns `{directory, entries[], has_more, next_cursor}`. | | `GET /api/v1/workers/{name}/workspace-files/file-metadata` | `?path=` (required, an allowed knowledge file): `{etag, modified_at, path, preview_kind, size}`. | | `GET /api/v1/workers/{name}/workspace-files/file-content` | `?path=` (required) plus optional `?offset=` (≥0) and `?limit=` (1..1048576): a bounded UTF-8 chunk `{content, eof, next_offset, truncated, etag, ...}` — continue with `offset=next_offset` while `truncated` is true. | +| `PUT /api/v1/workers/{name}/workspace-files/file-content` | Save one knowledge file: `?path=` (required), body `{"content": ""}` (1 MiB cap, non-empty) and the `If-Match` header (see the concurrency rule below). Returns the new `{etag, path, size}`. | +| `GET /api/v1/workers/{name}/workspace-files/file-download` | Stream one knowledge file as an attachment: `?path=` (required). Forwards the upstream `Content-Disposition` / `Content-Length` / `ETag` headers. | - **Scope**: same worker read authorization as `GET /api/v1/workers/{name}` and the checkpoint endpoints — team leaders / L2 humans only see workers in their accessible teams; unknown or out-of-scope workers are hidden as `404`. -- **Path allowlist (read-only knowledge boundary)**: only `MEMORY.md`, - `memory/**` and `digest/**` are addressable. Every other workspace +- **Write scope**: `PUT file-content` is allowed for admin/manager (any + team); an L2 human may write only workers in their own teams while + `Human.spec.workspaceFileAccess` is explicitly `"readwrite"` — the + default is `read` (an empty or unset value means read-only), so a + controller upgrade cannot silently grant pre-existing humans write + access; L1 grants a user write access by setting the field to + `readwrite`. Team leaders stay read-only on this API. Cross-team writes + hide the worker as `404` (existence must not be probeable); an in-scope + caller without write permission gets an explicit `403`. +- **Concurrency (ETag)**: before a write the proxy probes `file-metadata`. + For an existing file the `If-Match` header is mandatory (a worker + auto-appends to its memory files, so an unconditional overwrite would be + a lost update) and for a new file it must be absent. An upstream ETag + mismatch passes through as `409` — reload the file and retry. +- **Write limits**: the body is capped at 1 MiB (the read chunk cap), + `content` must be a non-empty string, and every successful write is + audit-logged by the controller (worker, path, caller, byte count). +- **`workspaceFileAccess` (Human CRD field)**: `read` | `readwrite` + (default is `read` when empty — write is an explicit opt-in) — the + per-user read/write permission L1 can set on a human's access to team + knowledge base files. Settable at human creation (`agt apply`) and + through `PUT /api/v1/humans/{name}` (that update API carries the field + in its updatable set, shipped with the humans-update PR). +- **Path allowlist (knowledge boundary)**: only `MEMORY.md`, + `memory/**` and `digest/**` are addressable — for reads and writes + alike. Every other workspace location — `SOUL.md`, `PROFILE.md`, `TODO.md`, `checkpoints/`, `skills/`, and all dot directories (`.copaw/agent.json` carries the worker's credentials) — is rejected with `400` before the request reaches the worker. Roots match on the exact first segment, so `memories/` and - `memoryX/` are not prefixes of `memory/`. + `memoryX/` are not prefixes of `memory/`. File roots are single + top-level files: `MEMORY.md` is addressable, but `MEMORY.md/foo` is + rejected (`MEMORY.md` is one file, not a directory — nested files must + live under `memory/` or `digest/`). - **Root pinned**: the QwenPaw `root=workspace` parameter (the agent's own storage root, as opposed to `root=project`, the primary bound project directory) is fixed server-side and is not part of the client query @@ -799,16 +829,18 @@ tree `memory/`, and the distilled knowledge tree `digest/`. response through verbatim (typically `404`); when nothing listens it returns `502`. The MEMORY.md probe is therefore only meaningful for QwenPaw workers. -- Forwarding is fixed-path (tree / file-metadata / file-content) with a - strict query whitelist — not a generic reverse proxy, and no write - endpoint of the workspace API is reachable. +- Forwarding is fixed-path (tree / file-metadata / file-content GET+PUT / + file-download) with a strict query whitelist — not a generic reverse + proxy. The multipart `file-upload` endpoint and the rest of the + workspace API surface remain unreachable. Error responses: | Code | Meaning | |:--|:--| -| `400` | Invalid worker name / unsupported subpath or query parameter / path outside the knowledge allowlist / out-of-bounds `limit` or `offset`. | +| `400` | Invalid worker name / unsupported subpath or query parameter / path outside the knowledge allowlist / out-of-bounds `limit` or `offset` / (write) missing or misplaced `If-Match`, over-size or empty body. | +| `403` | (write only) in-scope caller without write permission — read-only human or team leader. | | `404` | Worker not found or not in the caller's teams (existence hidden); or (passthrough) the file does not exist — see the version-gate probe above. | -| `409` / `416` | (passthrough) the file changed while being read / offset beyond end of file. | +| `409` / `416` | (passthrough) the file changed while being read / while waiting for the write (ETag mismatch — reload and retry) / offset beyond end of file. | | `502` | Worker app unreachable, or an upstream error (status echoed in the body). | | `503` | Kube mode (no stable worker pod DNS to proxy). | diff --git a/docs/zh-cn/usage/project-workflow-api.md b/docs/zh-cn/usage/project-workflow-api.md index 4d997e5c8..596a2c888 100644 --- a/docs/zh-cn/usage/project-workflow-api.md +++ b/docs/zh-cn/usage/project-workflow-api.md @@ -374,24 +374,45 @@ agt project complete demo-project-001 ## Worker 知识库（工作区文件）端点 -Controller 代理每个 worker 的 QwenPaw app（QwenPaw ≥ 2.1）的三个只读端点， -让 L2 人类与前端可以查看 worker 的知识库——长期记忆文件 `MEMORY.md`、 -日记目录树 `memory/` 与沉淀知识目录树 `digest/`。 +Controller 代理每个 worker 的 QwenPaw app（QwenPaw ≥ 2.1）的四个端点， +让 L2 人类与前端可以查看——并在 Human CR 允许时更新——worker 的知识库： +长期记忆文件 `MEMORY.md`、日记目录树 `memory/` 与沉淀知识目录树 `digest/`。 | 端点 | 含义 | |:--|:--| | `GET /api/v1/workers/{name}/workspace-files/tree` | 分页列出某个知识目录：`?path=`（必填，`memory` / `digest` 或其子路径），可选 `?cursor=`（不透明串）与 `?limit=`（1..500）。返回 `{directory, entries[], has_more, next_cursor}`。 | | `GET /api/v1/workers/{name}/workspace-files/file-metadata` | `?path=`（必填，允许的知识库文件）：`{etag, modified_at, path, preview_kind, size}`。 | | `GET /api/v1/workers/{name}/workspace-files/file-content` | `?path=`（必填）加可选 `?offset=`（≥0）与 `?limit=`（1..1048576）：有界 UTF-8 分块 `{content, eof, next_offset, truncated, etag, ...}`——`truncated` 为真时用 `offset=next_offset` 续读。 | +| `PUT /api/v1/workers/{name}/workspace-files/file-content` | 保存一个知识库文件：`?path=`（必填），body `{"content": "<文本>"}`（≤1 MiB，非空），`If-Match` 请求头（并发规则见下）。返回新的 `{etag, path, size}`。 | +| `GET /api/v1/workers/{name}/workspace-files/file-download` | 以附件形式流式下载一个知识库文件：`?path=`（必填）。透传上游 `Content-Disposition` / `Content-Length` / `ETag` 头。 | - **范围**：与 `GET /api/v1/workers/{name}` 相同的 worker 读授权——团队 leader / L2 人类只能看自己可访问团队内的 worker；未知或越权 worker 一律 隐藏为 `404`。 -- **路径白名单（只读知识边界）**：只放行 `MEMORY.md`、`memory/**` 与 - `digest/**`。工作区内其他一切位置——`SOUL.md`、`PROFILE.md`、`TODO.md`、 +- **写范围**：`PUT file-content` 对 admin/manager 全团队开放；L2 人类仅可写 + 自己团队内的 worker，且 `Human.spec.workspaceFileAccess` 显式为 + `"readwrite"`（缺省/未设置即 `read` 只读——Controller 升级不会静默授予 + 既有用户写权限；L1 把字段设为 `readwrite` 即授予）。团队 leader 在本 API + 上保持只读。跨团队写隐藏 worker 为 `404`（存在性不可探测）；范围内但无 + 写权限的调用得到明确的 `403`。 +- **并发（ETag）**：写之前代理先探测 `file-metadata`。文件已存在时 + `If-Match` 头必填（worker 会向自己的记忆文件自动追加，无条件覆盖即丢 + 更新）；新建文件时不得携带。上游 ETag 不匹配原样透传 `409`——重新加载 + 后重试。 +- **写限制**：body 上限 1 MiB（与读分块上限一致），`content` 必须为非空 + 字符串，每次成功写均由 controller 审计记录（worker、路径、调用者、字节 + 数）。 +- **`workspaceFileAccess`（Human CRD 字段）**：`read` | `readwrite` + （缺省为 `read`，写权限为显式 opt-in）——L1 可逐用户授予/收回的团队知识 + 库文件写权限。建 Human 时（`agt apply`）与 `PUT /api/v1/humans/{name}` + 均可设置（后者随 humans-update PR 把该字段纳入可更新集）。 +- **路径白名单（知识边界）**：只放行 `MEMORY.md`、`memory/**` 与 + `digest/**`（读写同界）。工作区内其他一切位置——`SOUL.md`、`PROFILE.md`、`TODO.md`、 `checkpoints/`、`skills/`，以及所有 dot 目录（`.copaw/agent.json` 承载 worker 凭据）——在请求到达 worker 之前即被 `400` 拒绝。根目录按完整首段 - 精确匹配，`memories/` 与 `memoryX/` 不构成 `memory/` 的前缀。 + 精确匹配，`memories/` 与 `memoryX/` 不构成 `memory/` 的前缀。文件根只能是 + 单个顶层文件：`MEMORY.md` 可寻址，但 `MEMORY.md/foo` 被拒（它是文件而非 + 目录——嵌套文件必须在 `memory/` 或 `digest/` 下）。 - **root 固定**：QwenPaw 的 `root=workspace` 参数（agent 自身存储根，相对 于 `root=project` 即主绑定项目目录）由服务端固定，不属于客户端查询面。 - **仅 embedded 模式**：端点经共享 docker 网络代理 worker 的 qwenpaw app， @@ -406,15 +427,17 @@ Controller 代理每个 worker 的 QwenPaw app（QwenPaw ≥ 2.1）的三个只 runtime）。其他 runtime 的 worker 没有 QwenPaw 工作区 API：该 runtime 的 应用若在服务 console 端口，代理原样透传其响应（通常 `404`）；无人监听 时返回 `502`。MEMORY.md 探测因此只对 QwenPaw worker 有意义。 -- 转发为固定子路径（tree / file-metadata / file-content）+ 严格查询白名单 - ——不是通用反向代理，工作区 API 的任何写端点均不可达。 +- 转发为固定子路径（tree / file-metadata / file-content GET+PUT / + file-download）+ 严格查询白名单——不是通用反向代理。multipart 的 + `file-upload` 端点与工作区 API 的其余面均不可达。 错误响应： | 码 | 含义 | |:--|:--| -| `400` | worker 名非法 / 不支持的子路径或查询参数 / 路径不在知识白名单内 / `limit` 或 `offset` 越界。 | +| `400` | worker 名非法 / 不支持的子路径或查询参数 / 路径不在知识白名单内 / `limit` 或 `offset` 越界 /（写）`If-Match` 缺失或误用、body 超限或为空。 | +| `403` | （仅写）范围内但无写权限的调用者——只读人类或团队 leader。 | | `404` | worker 不存在或不在调用方团队内（存在性隐藏）；或（透传）文件不存在——见上方版本门探测。 | -| `409` / `416` | （透传）读取期间文件被修改 / offset 超出文件末尾。 | +| `409` / `416` | （透传）读取期间或写入等待期间文件被修改（ETag 不匹配——重载重试）/ offset 超出文件末尾。 | | `502` | worker app 不可达，或上游错误（状态码回显在 body 中）。 | | `503` | kube 模式（无稳定的 worker pod DNS 可代理）。 | diff --git a/helm/agentteams/crds/humans.agentteams.io.yaml b/helm/agentteams/crds/humans.agentteams.io.yaml index 63f64f809..2597cbbcc 100644 --- a/helm/agentteams/crds/humans.agentteams.io.yaml +++ b/helm/agentteams/crds/humans.agentteams.io.yaml @@ -50,6 +50,12 @@ spec: required: [issuer, subject] note: type: string + workspaceFileAccess: + type: string + enum: + - read + - readwrite + description: "Knowledge base file access for this human's own teams: read (default when empty) or readwrite (explicit write opt-in); L1 admin is never restricted, team leaders stay read-only" status: type: object properties: From 53e26014106cc8f6191f9fa2405bacab41004d02 Mon Sep 17 00:00:00 2001 From: LUOSENGWA Date: Fri, 11 Sep 2026 05:50:55 +0000 Subject: [PATCH 4/4] fix(controller): stop reading phantom sub path value on the fixed PUT route The registered write route is the fixed literal /api/v1/workers/{name}/workspace-files/file-content (http.go) with no {sub} capture, so r.PathValue("sub") in proxyWorkspaceFileWrite was always "" and every authorized write (admin / human with workspaceFileAccess=readwrite) was rejected with 400 "unsupported workspace file write subpath" before any worker lookup. - keep the fixed PUT route as the single source of truth for the write subpath and drop the handler's dependency on the nonexistent sub path value (the now-unreferenced workspaceFileWriteSubpaths map is removed) - add TestWorkspaceFilesWrite_RouteAcceptsAuthorizedWrite, which drives the request through the actual NewHTTPServer(...).Mux registration (no SetPathValue shims): an admin PUT reaches the upstream probe stage (502 against the dead pod URL in tests) instead of the phantom 400, and an unknown worker yields the 404 from the worker lookup --- .../internal/server/worker_workspace_files.go | 21 ++++---- .../server/worker_workspace_files_test.go | 52 +++++++++++++++++++ 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/agentteams-controller/internal/server/worker_workspace_files.go b/agentteams-controller/internal/server/worker_workspace_files.go index b66ecf329..a2eef816a 100644 --- a/agentteams-controller/internal/server/worker_workspace_files.go +++ b/agentteams-controller/internal/server/worker_workspace_files.go @@ -117,11 +117,11 @@ var workspaceFileSubpaths = map[string]bool{ "file-download": true, } -// workspaceFileWriteSubpaths are the subpaths served by the PUT handler. -var workspaceFileWriteSubpaths = map[string]bool{ - "file-content": true, -} - +// The write subpath is not a map-checked path value: the PUT route is the +// fixed literal /api/v1/workers/{name}/workspace-files/file-content +// registered in http.go (no {sub} capture), so the route table is the single +// source of truth and proxyWorkspaceFileWrite reads no sub path value. +// // kbFileRoots are the top-level single files addressable by the // file-metadata / file-content subpaths. var kbFileRoots = []string{"MEMORY.md"} @@ -444,15 +444,16 @@ func (h *WorkspaceFilesHandler) upstreamKBFileExists(ctx context.Context, baseUR // write body is capped at kbMaxFileLimit (1 MiB, the read chunk cap). func (h *WorkspaceFilesHandler) proxyWorkspaceFileWrite(w http.ResponseWriter, r *http.Request) { name := r.PathValue("name") - sub := r.PathValue("sub") if name == "" || !workerNamePattern.MatchString(name) { httputil.WriteError(w, http.StatusBadRequest, "worker name is required and must be a valid DNS label") return } - if !workspaceFileWriteSubpaths[sub] { - httputil.WriteError(w, http.StatusBadRequest, "unsupported workspace file write subpath") - return - } + // The registered route is the fixed literal + // /api/v1/workers/{name}/workspace-files/file-content (see http.go) — the + // PUT route has no {sub} capture, so no sub path value is read here. + // (Review P1: the previous code read r.PathValue("sub"), which was always + // "" on the registered route, so every authorized write was rejected with + // 400 "unsupported workspace file write subpath" before the worker lookup.) if h.kubeMode != "embedded" { httputil.WriteError(w, http.StatusServiceUnavailable, "worker workspace file inspection requires embedded mode") return diff --git a/agentteams-controller/internal/server/worker_workspace_files_test.go b/agentteams-controller/internal/server/worker_workspace_files_test.go index 46217791a..57ff08d1e 100644 --- a/agentteams-controller/internal/server/worker_workspace_files_test.go +++ b/agentteams-controller/internal/server/worker_workspace_files_test.go @@ -1022,3 +1022,55 @@ func TestWorkspaceFilesDownload_SensitivePathRejected(t *testing.T) { } } } + +// TestWorkspaceFilesWrite_RouteAcceptsAuthorizedWrite is the regression test +// for the P1 review finding on this PR: the registered PUT route is the fixed +// literal /api/v1/workers/{name}/workspace-files/file-content (http.go) with +// no {sub} capture, but the handler used to read r.PathValue("sub") — always +// "" on the registered route — and rejected every authorized write with 400 +// "unsupported workspace file write subpath" before any worker lookup. The +// requests below go through the actual NewHTTPServer(...).Mux registration, +// with no SetPathValue shims. +func TestWorkspaceFilesWrite_RouteAcceptsAuthorizedWrite(t *testing.T) { + scheme := newProjectTestScheme(t) + k8s := fake.NewClientBuilder().WithScheme(scheme). + WithRuntimeObjects(kbWorkerFixture("market-team", "market-writer")...).Build() + enricher := authpkg.NewCREnricher(k8s, "default") + mw := authpkg.NewMiddleware(&alwaysAdminAuth{}, enricher, authpkg.NewAuthorizer(), k8s, "default") + srv := NewHTTPServer(":0", ServerDeps{ + Client: k8s, + Namespace: "default", + KubeMode: "embedded", + AuthMw: mw, + }) + + const body = `{"content":"# market memory"}` + + // 1) Existing worker (admin): must pass routing + authz + validation and + // reach the upstream probe — 502 against the dead worker pod URL in + // the test environment, never the phantom-sub 400. + req := httptest.NewRequest(http.MethodPut, + "/api/v1/workers/market-writer/workspace-files/file-content?path=MEMORY.md", + strings.NewReader(body)) + req.Header.Set("Authorization", "Bearer sa-token") + rec := httptest.NewRecorder() + srv.Mux.ServeHTTP(rec, req) + if rec.Code == http.StatusBadRequest && strings.Contains(rec.Body.String(), "unsupported workspace file write subpath") { + t.Fatalf("P1 regression: authorized write rejected by the phantom sub check: %s", rec.Body.String()) + } + if rec.Code != http.StatusBadGateway { + t.Fatalf("status=%d body=%s, want 502 (upstream probe against the dead pod URL) — the request must reach the worker/upstream stage", rec.Code, rec.Body.String()) + } + + // 2) Unknown worker: routing + authz pass and the worker lookup runs — + // 404 from the lookup proves the request went past the route level. + req2 := httptest.NewRequest(http.MethodPut, + "/api/v1/workers/ghost-writer/workspace-files/file-content?path=MEMORY.md", + strings.NewReader(body)) + req2.Header.Set("Authorization", "Bearer sa-token") + rec2 := httptest.NewRecorder() + srv.Mux.ServeHTTP(rec2, req2) + if rec2.Code != http.StatusNotFound { + t.Fatalf("unknown worker status=%d body=%s, want 404 from the worker lookup", rec2.Code, rec2.Body.String()) + } +}