diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 42e54bf24..6884a915e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -65,16 +65,17 @@ to recover a lost creation response. Session metadata updates require a supplied metadata field, with null/empty clearing it. Validate an empty update before any resource lookup, after authentication. -List order parsing distinguishes omission from an explicit empty value. Lists read by -the shared list parser and single-resource routes ignore unknown query keys; a -repeated supported list key still rejects. The Environment Files list keeps its own -strict key parser and still rejects unknown keys; that difference is deferred. Reuse the shared parser and error serializer, preserving the observed -Beta, Files and Skills error fields and per-family limit bounds rather than applying -one policy to every resource. Change page bounds, cursor ownership or parent lookup -order only with owned evidence for that family. Record uncertain range/lookup -behavior separately; do not reproduce observed upstream server failures as -compatibility behavior. See `contracts/agents-api/list-query-semantics.md` for the -bounded evidence. +List order parsing distinguishes omission from an explicit empty value. Lists and +single-resource routes ignore unknown query keys; a repeated supported list key +still rejects. The Environment Files list keeps its own path and cursor parsing but +uses the same unknown-key and duplicate-key rules, except that it still rejects +malformed query encoding (such as `%GG` or `;` separators) that the shared lists +drop. Reuse the shared parser and error serializer, preserving the observed Beta, Files and Skills error fields and +per-family limit bounds rather than applying one policy to every resource. Change +page bounds, cursor ownership or parent lookup order only with owned evidence for +that family. Record uncertain range/lookup behavior separately; do not reproduce +observed upstream server failures as compatibility behavior. See +`contracts/agents-api/list-query-semantics.md` for the bounded evidence. Report validation failures with official evidence through the typed field error, which emits `invalid_request_error` with the observed param and message; keep @@ -824,8 +825,11 @@ Revoke the scoped read transport credential on completion or failure. Runtime retains uncertain cleanup ownership and capacity; this does not require a second durable Core owner registry or establish remote write retirement. Public Files.list delegates workspace access to this reader; -the API owns tenant authorization, path validation and protocol pagination. Keep -partial directory coverage and unverified defaults explicit in the Files contract. +the API owns tenant authorization, path validation and protocol pagination. Only +the reader's distinct `not_directory` result (a missing path, a regular file or an +unfollowed symlink) becomes an empty page; root, permission, transport and +uncertain failures keep their errors. Keep partial directory coverage and +unverified defaults explicit in the Files contract. Source Files belong to the execution project and have an independent lifecycle from copied workspace files. Store immutable source metadata and PostgreSQL large diff --git a/apps/parsar-daemon/internal/agent/workspace_read.go b/apps/parsar-daemon/internal/agent/workspace_read.go index be85bc9a2..403a9b070 100644 --- a/apps/parsar-daemon/internal/agent/workspace_read.go +++ b/apps/parsar-daemon/internal/agent/workspace_read.go @@ -21,4 +21,7 @@ var ( ErrWorkspaceReadBusy = errors.New("workspace read busy") ErrWorkspaceReadInvalid = errors.New("workspace read invalid") ErrWorkspaceReadUncertain = errors.New("workspace read outcome uncertain") + // ErrWorkspaceNotDirectory reports that a directory request's own path is + // missing, a regular file or a symbolic link; the link was not followed. + ErrWorkspaceNotDirectory = errors.New("workspace path is not a directory") ) diff --git a/apps/parsar-daemon/internal/dispatch/local_directory_test.go b/apps/parsar-daemon/internal/dispatch/local_directory_test.go index dbe992d78..0515a59d6 100644 --- a/apps/parsar-daemon/internal/dispatch/local_directory_test.go +++ b/apps/parsar-daemon/internal/dispatch/local_directory_test.go @@ -90,3 +90,55 @@ func waitWorkspaceRead(t *testing.T, sender *recSender, id string) proto.Workspa t.Fatal("read result missing", id) return proto.WorkspaceReadResultPayload{} } + +func TestLocalDirectoryKeepsNotDirectorySeparateFromFailures(t *testing.T) { + workspace, helper := t.TempDir(), filepath.Join(t.TempDir(), "directory") + script := `#!/bin/sh +case "$2" in + missing) printf '%s' '{"version":1,"error":"not_directory"}' ;; + invalid) printf '%s' '{"version":1,"error":"invalid_path"}' ;; + gone) printf '%s' '{"version":1,"error":"not_found"}' ;; + denied) printf '%s' '{"version":1,"error":"permission_denied"}' ;; + broken) printf '%s' '{"version":1,"error":"native_error"}' ;; +esac +` + if err := os.WriteFile(helper, []byte(script), 0o700); err != nil { + t.Fatal(err) + } + environment, session := uuid.NewString(), uuid.NewString() + binding, err := localworkspace.New(environment, session, workspace, helper) + if err != nil { + t.Fatal(err) + } + reg := agent.NewRegistry() + reg.RegisterKind(proto.SupportedAgentKind{Kind: "native", Available: true, Capabilities: proto.AgentKindCapabilities{LocalEnvironment: true}}, func(context.Context, proto.PromptRequestPayload, chan<- proto.Envelope) (agent.Session, error) { + return nil, errors.New("must not start a model") + }) + reg.RegisterPreparation("native", true, func(context.Context, proto.PromptRequestPayload) (agent.Prepared, error) { + return nil, errors.New("must not prepare a harness") + }) + sender := &recSender{} + r, err := dispatch.New(dispatch.Config{Registry: reg, Sender: sender, LocalWorkspace: binding}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = r.Shutdown(context.Background()) }) + request := proto.PromptRequestPayload{AgentKind: "native", LocalEnvironment: &proto.LocalEnvironment{ID: environment}, AgentStateKey: "agents-api-" + session, StrictResume: true, ReleaseOnCompletion: true, WorkspaceReadOnly: true} + if err := r.Handle(t.Context(), mustEnv(t, proto.TypeExecutionPrepare, "idle", proto.ExecutionPreparePayload{Configuration: request})); err != nil { + t.Fatal(err) + } + ready := waitPreparationStatus(t, sender, "idle", "ready", "") + for path, want := range map[string]proto.WorkspaceReadResultPayload{ + "missing": {Outcome: "rejected", ErrorCode: proto.WorkspaceReadNotDirectory}, + "invalid": {Outcome: "rejected", ErrorCode: "invalid_request"}, + "gone": {Outcome: "rejected", ErrorCode: "not_found"}, + "denied": {Outcome: "rejected", ErrorCode: "permission_denied"}, + "broken": {Outcome: "unknown", ErrorCode: "read_unconfirmed"}, + } { + read := proto.WorkspaceReadPayload{EnvironmentID: environment, Handle: ready.Handle, Operation: "directory", Path: path, MaxEntries: 10} + _ = r.Handle(t.Context(), mustEnv(t, proto.TypeWorkspaceRead, path, read)) + if got := waitWorkspaceRead(t, sender, path); got.Outcome != want.Outcome || got.ErrorCode != want.ErrorCode || got.Directory != nil || got.CloseAcknowledged { + t.Fatal("native directory result changed", path, got) + } + } +} diff --git a/apps/parsar-daemon/internal/dispatch/workspace_read.go b/apps/parsar-daemon/internal/dispatch/workspace_read.go index 1ad78dc20..77aeb8b67 100644 --- a/apps/parsar-daemon/internal/dispatch/workspace_read.go +++ b/apps/parsar-daemon/internal/dispatch/workspace_read.go @@ -128,6 +128,7 @@ func workspaceReadResult(read agent.WorkspaceReadResult, err error, limit int) p {agent.ErrWorkspaceReadUnavailable, "resource_unavailable"}, {agent.ErrWorkspaceReadBusy, "read_capacity"}, {agent.ErrWorkspaceReadInvalid, "invalid_request"}, + {agent.ErrWorkspaceNotDirectory, proto.WorkspaceReadNotDirectory}, {fs.ErrNotExist, "not_found"}, {fs.ErrPermission, "permission_denied"}, } { diff --git a/apps/parsar-daemon/internal/localworkspace/binding_test.go b/apps/parsar-daemon/internal/localworkspace/binding_test.go index 062d45a95..cd245ba7e 100644 --- a/apps/parsar-daemon/internal/localworkspace/binding_test.go +++ b/apps/parsar-daemon/internal/localworkspace/binding_test.go @@ -1,10 +1,13 @@ package localworkspace import ( + "errors" + "io/fs" "os" "path/filepath" "testing" + "github.com/MiniMax-AI-Dev/parsar/apps/parsar-daemon/internal/agent" "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/proto" "github.com/google/uuid" ) @@ -76,6 +79,20 @@ func TestLocalHelperCannotInheritCredentials(t *testing.T) { } } +func TestDirectoryClassifiesNativeErrors(t *testing.T) { + for code, want := range map[string]error{ + "not_directory": agent.ErrWorkspaceNotDirectory, + "not_found": fs.ErrNotExist, + "permission_denied": fs.ErrPermission, + "invalid_path": agent.ErrWorkspaceReadInvalid, + "native_error": agent.ErrWorkspaceReadUncertain, + } { + if _, err := decodeDirectory([]byte(`{"version":1,"error":"`+code+`"}`), 2); !errors.Is(err, want) { + t.Fatal("native error classification changed", code, err) + } + } +} + func TestDirectoryRejectsMalformedOrIncompleteResponses(t *testing.T) { for _, frame := range []string{ `{"version":2,"directory":{"entries":[],"truncated":false}}`, diff --git a/apps/parsar-daemon/internal/localworkspace/directory.go b/apps/parsar-daemon/internal/localworkspace/directory.go index 40980ffa8..b747c3821 100644 --- a/apps/parsar-daemon/internal/localworkspace/directory.go +++ b/apps/parsar-daemon/internal/localworkspace/directory.go @@ -64,6 +64,8 @@ func decodeDirectory(data []byte, limit int) (agent.WorkspaceDirectoryResult, er err = fs.ErrPermission case "invalid_path": err = agent.ErrWorkspaceReadInvalid + case proto.WorkspaceReadNotDirectory: + err = agent.ErrWorkspaceNotDirectory } return agent.WorkspaceDirectoryResult{}, err } diff --git a/apps/parsar-daemon/internal/localworkspace/directory_native_test.go b/apps/parsar-daemon/internal/localworkspace/directory_native_test.go index 7ba84f1f4..b9800bd91 100644 --- a/apps/parsar-daemon/internal/localworkspace/directory_native_test.go +++ b/apps/parsar-daemon/internal/localworkspace/directory_native_test.go @@ -1,10 +1,12 @@ package localworkspace import ( + "errors" "os" "path/filepath" "testing" + "github.com/MiniMax-AI-Dev/parsar/apps/parsar-daemon/internal/agent" "github.com/google/uuid" ) @@ -33,8 +35,15 @@ func TestNativeLocalDirectoryConfinement(t *testing.T) { if err != nil || got.Truncated || len(got.Entries) != 1 || got.Entries[0].Name != "data.bin" || got.Entries[0].SizeBytes == nil || *got.Entries[0].SizeBytes != 4 { t.Fatalf("native file metadata: %+v %v", got, err) } - if _, err := b.ListWorkspaceDirectory(t.Context(), "escape", 10); err == nil { - t.Fatal("native helper followed an external symlink") + if err := os.WriteFile(filepath.Join(root, "file.txt"), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + // A link, a regular file and a missing path are not listable directories; + // the external link target is never listed. + for _, path := range []string{"escape", "escape/secret", "file.txt", "missing", "missing/deeper"} { + if _, err := b.ListWorkspaceDirectory(t.Context(), path, 10); !errors.Is(err, agent.ErrWorkspaceNotDirectory) { + t.Fatal("native helper classified a non-directory path differently", path, err) + } } got, err = b.ListWorkspaceDirectory(t.Context(), "", 1) if err != nil || !got.Truncated || len(got.Entries) != 1 { @@ -47,7 +56,7 @@ func TestNativeLocalDirectoryConfinement(t *testing.T) { if err := os.Symlink(outside, root); err != nil { t.Fatal(err) } - if _, err := b.ListWorkspaceDirectory(t.Context(), "", 10); err == nil { - t.Fatal("native helper followed a replaced root") + if _, err := b.ListWorkspaceDirectory(t.Context(), "", 10); !errors.Is(err, agent.ErrWorkspaceReadInvalid) { + t.Fatal("native helper followed a replaced root or hid it as an empty directory", err) } } diff --git a/apps/web/e2e/fixture-core.mjs b/apps/web/e2e/fixture-core.mjs index 127487337..4f5b1a67f 100644 --- a/apps/web/e2e/fixture-core.mjs +++ b/apps/web/e2e/fixture-core.mjs @@ -1494,7 +1494,7 @@ const server = http.createServer(async (request, response) => { response.destroy(); return; } - return sendJson(response, result); + return sendJson(response, result, 201); } if (request.method === "GET" && environmentFilesMatch) { trackAbort(response, "environmentFileReads"); @@ -1523,7 +1523,7 @@ const server = http.createServer(async (request, response) => { .filter((file) => file.path.slice(0, file.path.lastIndexOf("/")) === directory) .sort((left, right) => left.path.localeCompare(right.path)); if (order === "desc") files.reverse(); - return sendJson(response, { data: files.slice(0, limit), next: null }); + return sendJson(response, { object: "page", data: files.slice(0, limit), next: null, has_more: false }); } const sessionEnvironment = state.sessions[0]?.environment; const expectedId = sessionEnvironment?.type === "self_hosted" ? sessionEnvironment.id : null; @@ -1547,9 +1547,12 @@ const server = http.createServer(async (request, response) => { })); if (order === "desc") allFiles.reverse(); const start = cursor === "fixture-page-2" ? 20 : 0; + const next = start + limit < allFiles.length ? "fixture-page-2" : null; return sendJson(response, { + object: "page", data: allFiles.slice(start, start + limit), - next: start + limit < allFiles.length ? "fixture-page-2" : null, + next, + has_more: next !== null, }); } diff --git a/apps/web/src/features/sessions/environment/EnvironmentFilesPanel.test.tsx b/apps/web/src/features/sessions/environment/EnvironmentFilesPanel.test.tsx index 73399edd5..a3476ebca 100644 --- a/apps/web/src/features/sessions/environment/EnvironmentFilesPanel.test.tsx +++ b/apps/web/src/features/sessions/environment/EnvironmentFilesPanel.test.tsx @@ -6,6 +6,7 @@ import { AgentCoreError } from "@agents-core-web/agents-client"; import { EnvironmentFilesPanel, environmentFilesFailureMessage, + environmentFilesRequestDirectory, formatFileSize, validEnvironmentFilesDirectory, } from "./EnvironmentFilesPanel"; @@ -27,6 +28,12 @@ describe("EnvironmentFilesPanel", () => { expect(validEnvironmentFilesDirectory("/test/../secret", "/test")).toBe(false); }); + it("sends the cleaned directory form that Core requires", () => { + expect(environmentFilesRequestDirectory("/workspace/project/src/")).toBe("/workspace/project/src"); + expect(environmentFilesRequestDirectory("/workspace//")).toBe("/workspace"); + expect(environmentFilesRequestDirectory("/workspace/project")).toBe("/workspace/project"); + }); + it("reports an unsupported Core instead of a generic directory failure", () => { expect(environmentFilesFailureMessage( new AgentCoreError("This API operation is not supported.", 404, "unsupported_operation"), @@ -50,7 +57,7 @@ describe("EnvironmentFilesPanel", () => { ({ data: [], next: null })} + onListFiles={async () => ({ object: "page", data: [], next: null, has_more: false })} />, ); diff --git a/apps/web/src/features/sessions/environment/EnvironmentFilesPanel.tsx b/apps/web/src/features/sessions/environment/EnvironmentFilesPanel.tsx index 02530ab91..2ea76c2ac 100644 --- a/apps/web/src/features/sessions/environment/EnvironmentFilesPanel.tsx +++ b/apps/web/src/features/sessions/environment/EnvironmentFilesPanel.tsx @@ -44,6 +44,11 @@ export function validEnvironmentFilesDirectory( return root === "/" || directory === root || directory.startsWith(`${root}/`); } +/** Core accepts only the cleaned form, so a valid trailing separator is dropped before sending. */ +export function environmentFilesRequestDirectory(value: string): string { + return canonicalDirectory(value) ?? value; +} + export function formatFileSize(bytes: number): string { if (bytes < 1024) return `${bytes} B`; if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(bytes < 10 * 1024 ? 1 : 0)} KB`; @@ -119,7 +124,7 @@ export function EnvironmentFilesPanel({ abortRef.current = controller; const request = requestRef.current + 1; requestRef.current = request; - const requestedDirectory = append ? appliedDirectory : directory; + const requestedDirectory = append ? appliedDirectory : environmentFilesRequestDirectory(directory); const requestedOrder = append ? appliedOrder : order; setState(append ? "loading-more" : "loading"); setError(null); diff --git a/apps/web/src/features/sessions/environment/EnvironmentPanel.test.tsx b/apps/web/src/features/sessions/environment/EnvironmentPanel.test.tsx index a73ed3e9d..2b5b00fdb 100644 --- a/apps/web/src/features/sessions/environment/EnvironmentPanel.test.tsx +++ b/apps/web/src/features/sessions/environment/EnvironmentPanel.test.tsx @@ -111,7 +111,7 @@ describe("EnvironmentPanel", () => { }} connectionActions={[{ type: "environment_connection", environment_id: hostedEnvironmentUuid }]} environmentFilesEnabled - onListFiles={async () => ({ data: [], next: null })} + onListFiles={async () => ({ object: "page", data: [], next: null, has_more: false })} onCreateFile={async (_environmentId, input) => ({ environment_id: hostedEnvironmentUuid, object: "agent.environment.file", @@ -205,7 +205,7 @@ describe("EnvironmentPanel", () => { }} connectionActions={[]} environmentFilesEnabled - onListFiles={async () => ({ data: [], next: null })} + onListFiles={async () => ({ object: "page", data: [], next: null, has_more: false })} onCreateFile={async () => ({ environment_id: hostedEnvironmentUuid, object: "agent.environment.file", @@ -270,7 +270,7 @@ describe("EnvironmentPanel", () => { observation={null} connectionActions={[]} environmentFilesEnabled - onListFiles={async () => ({ data: [], next: null })} + onListFiles={async () => ({ object: "page", data: [], next: null, has_more: false })} />, ); expect(complete).toContain("Workspace files"); @@ -290,7 +290,7 @@ describe("EnvironmentPanel", () => { observation={null} connectionActions={[]} environmentFilesEnabled={false} - onListFiles={async () => ({ data: [], next: null })} + onListFiles={async () => ({ object: "page", data: [], next: null, has_more: false })} />, ); expect(buildDisabled).not.toContain("Workspace files"); @@ -310,7 +310,7 @@ describe("EnvironmentPanel", () => { observation={null} connectionActions={[]} environmentFilesEnabled - onListFiles={async () => ({ data: [], next: null })} + onListFiles={async () => ({ object: "page", data: [], next: null, has_more: false })} />, ); expect(incomplete).not.toContain("Workspace files"); @@ -327,7 +327,7 @@ describe("EnvironmentPanel", () => { observation={null} connectionActions={[]} environmentFilesEnabled - onListFiles={async () => ({ data: [], next: null })} + onListFiles={async () => ({ object: "page", data: [], next: null, has_more: false })} />, ); expect(unsupportedProfile).not.toContain("Workspace files"); diff --git a/contracts/agents-api/README.md b/contracts/agents-api/README.md index 96fd68809..68a02be7e 100644 --- a/contracts/agents-api/README.md +++ b/contracts/agents-api/README.md @@ -104,7 +104,7 @@ paths start at `/vaults`, not `/agents/vaults`. | sessions.subagents.turns | retrieve, list | Implemented; shared Session/child IDs | | sessions.subagents.turns.items | list | Implemented; scoped persisted reads | | environments | retrieve | Three-harness colocated self-hosted implementation and qualified Docker hosted profiles: durable status and safe initial-file metadata; other installation inventory and full lifecycle parity remain gaps | -| environments.files | create, list | [Bounded live listing and inline/source-file creation](environment-files.md) on qualified Docker workspaces; [user-managed enrollment](user-managed-runtime-v1.md) reuses the local implementation with separate real public acceptance. Full listing, overwrite and error semantics remain partial | +| environments.files | create, list | [Bounded live listing and inline/source-file creation](environment-files.md) on qualified Docker workspaces; [user-managed enrollment](user-managed-runtime-v1.md) reuses the local implementation with separate real public acceptance. [Aligned](environment-files.md#wire-alignment--september-23-2026) the 201 status, page envelope, query keys, empty pages for non-directory paths on local workspace readers, sampled path/token errors and pending hosted rejection; recursion, parent creation, overwrite and other errors remain partial | | environments.templates | create, retrieve, update, list, delete | [Reusable network, files, env/setup/packages, inline/referenced Skills and Session snapshots](environment-templates.md); other initialization and full semantics remain gaps | | vaults | create, retrieve, list, delete | Create/retrieve/list/delete with independent tenant persistence, stored status filtering, atomic Credential cascade and frozen Session attachments; archive semantics and full hosted lifecycle parity remain missing | | vaults.credentials | create, retrieve, update, list, delete | Static-bearer and OAuth create/retrieve/list/replacement/deletion with scoped encrypted storage and dispatch-time refresh; Session attachment and exact-URL HTTPS MCP binding; archive semantics and full hosted lifecycle parity remain missing | diff --git a/contracts/agents-api/environment-files.md b/contracts/agents-api/environment-files.md index 8f256ba5b..dad9a53d5 100644 --- a/contracts/agents-api/environment-files.md +++ b/contracts/agents-api/environment-files.md @@ -25,7 +25,8 @@ Each [EnvironmentFile](https://github.com/openai/openai-python/blob/d7c41efee1b0 has `environment_id`, `object: agent.environment.file`, absolute `path`, and integer `size_bytes`. The pinned [TokenPage](https://github.com/openai/openai-python/blob/d7c41efee1b0802b79f3f88a678ef2052b06e9ce/src/openai/pagination.py) requires `data`; `has_more` and `next` are optional and nullable. This implementation -returns `data` and `next` (null on the final page), with no additional page fields. +returns the observed official envelope `object: page`, `data`, `next` (null on the +final page) and `has_more`, which is true exactly when `next` is set. ## Current scope and local policies @@ -33,18 +34,22 @@ returns `data` and `next` (null on the final page), with no additional page fiel directory. Local public paths are rooted at `/workspace`, independently of the physical path frozen into its dedicated Runtime. Omitted path selects the workspace root. Do not recurse or follow symlinks; - directory, symlink and other non-regular entries are omitted. -- Omitted limit uses 20. Query keys may occur once; empty values, unknown keys and - malformed query encoding are rejected. This list keeps its own key parser; unlike - the shared list parser it still rejects unknown keys, a deferred difference. Its - limit range errors use the Beta `invalid_request_error` code. The pinned SDK's + directory, symlink and other non-regular entries are omitted. A path that is + missing, a regular file or a symlink lists an empty page; the link is not followed. + Daemons without a local workspace binding use the Claude SDK adapter reader, + which keeps 404 for a missing path and 503 for a regular file or symlink. +- Omitted limit uses 20. Well-formed unknown query keys are ignored; a repeated + `path`, `limit`, `order` or `page` returns the Beta duplicate-field error. Unlike + the shared lists, which drop malformed pairs, malformed query encoding (such as + `?foo=%GG` or a `;` separator) still rejects the request locally. Empty values are + rejected by their own rule. Limit errors use the Beta `invalid_request_error` code. The pinned SDK's [query serializer](https://github.com/openai/openai-python/blob/d7c41efee1b0802b79f3f88a678ef2052b06e9ce/src/openai/_qs.py) omits scalar `None` values, so nullable limit/path follow omission behavior. Literal `null` and empty scalar query values are not accepted. - Require an absolute UTF-8 POSIX directory of at most 4096 bytes within the - workspace. Reject `..` components before normalization, backslash, NUL, CR and LF. - Normalize redundant separators, `.` and trailing separators before binding a - cursor or passing the workspace-relative directory to execution. + workspace, in cleaned form. Backslash, NUL, CR and LF are rejected. Redundant or + trailing separators, `.` and `..` components are rejected rather than normalized; + the exact directory binds the cursor and is passed workspace-relative to execution. - Authorize through the existing tenant-scoped Environment lookup before inspecting directory paths, cursors or runtime availability. Existing project-shared reads remain permitted. The reader receives that exact Environment and rechecks its @@ -58,16 +63,20 @@ returns `data` and `next` (null on the final page), with no additional page fiel result. Every page rereads the directory. Changed files or parameters invalidate continuation with safe 400. No cache, durable cursor registry or snapshot is promised; unchanged path/size metadata does not prove unchanged contents. -- Reader errors reuse the existing safe error mapping: not found 404, invalid input - 400 and unavailable execution 503. Native error text never enters the response. +- Reader errors reuse the existing safe error mapping: invalid input 400 and + unavailable execution 503. Only the native reader's distinct `not_directory` + result becomes an empty page; a missing workspace root or other native `not_found` + keeps 404, and permission, transport and uncertain failures keep 503. Native error + text never enters the response. Listing does not create a Turn or admit model input. Idle reads use temporary read-only preparation; actual transport disconnect/reconnect events remain visible. -Default limit, omitted-path scope, recursion, non-regular entries, exact invalid or -missing-path errors and cursor invalidation behavior are -local policies or remaining gaps, not verified hosted semantics. The pinned source -does not establish them. Do not interpret the bounded direct-file implementation -as complete Files.list compatibility. +Default limit, omitted-path scope, recursion, non-regular entries and cursor +invalidation behavior are local policies or remaining gaps, not verified hosted +semantics. The pinned source does not establish them. The sampled envelope, query, +path and empty-page rows are recorded in [wire alignment](#wire-alignment--september-23-2026). +Do not interpret the bounded direct-file implementation as complete Files.list +compatibility. ## Inline and source-file creation @@ -76,7 +85,9 @@ absolute destination `path` under `/workspace`, or `type: file_id`, `file_id` an that path. Source IDs resolve only within the authenticated execution project; filenames, URLs and filesystem paths cannot substitute for an ID. Both members use the same destination writer. Required null/omitted fields, extra fields and -invalid Base64 are rejected; unknown query keys are ignored. Empty bytes are valid. Inline +invalid Base64 are rejected; an unknown top-level field names itself as `param` +when its name is short and printable. +Unknown query keys are ignored. Empty bytes are valid. Inline paths must be canonical and cannot name the workspace root; the parent must exist. The current destination limit is 50 MiB for either source, with bounded JSON and 64 KiB daemon frames. These are local limits and policies, not verified upstream restrictions. @@ -108,9 +119,9 @@ and placement replacement are outside this batch. Controlled failures preserve the destination only when the installer proves rejection, and only against this operation, not independent workspace writers. Temporary-file cleanup is best effort. -Successful creation returns only the four EnvironmentFile fields. Reuse the common -safe error mapper; current 400/409/413/503 policies and error timing are not evidence -of exact upstream parity. This referenced-source milestone cannot close the complete Files +Successful creation returns 201 with only the four EnvironmentFile fields. Reuse the +common safe error mapper; apart from the sampled rows below, current 400/409/413/503 +policies and error timing are not evidence of exact upstream parity. This referenced-source milestone cannot close the complete Files resource or Environment lifecycle requirements. Source resolution reads an immutable snapshot before destination admission. A @@ -139,3 +150,99 @@ denial, and verifies that it cannot bypass the smaller destination bound. Intern large-object streaming integrity has separate PostgreSQL tests. This distinction keeps private setup separate from public hosted creation acceptance. Mechanism tests exercise detached/unknown outcomes and durable gates independently of model output. + +## Wire alignment — September 23, 2026 + +The pin is unchanged: SDK 3.13.0, commit `d7c41ef`, `agents=v1`. This batch starts +from main `e4b124c` and aligns Files.create and Files.list wire behavior with the +hosted-environment campaign scan, recorded privately in +`~/.parsar/remediation/20260923/campaign-scan-2/hosted-env/` (`findings.json` +HE-10, 16, 18, 32, 34–39; raw records under `official/` and `run1/`). The probe +used three owned hosted Sessions, all deleted. The batch plan is +`~/.parsar/remediation/20260923/environment-files-wire/PLAN.md`. + +| Row | Case | Core behavior | Evidence (finding: request ID) | +| --- | --- | --- | --- | +| F1 | Successful Files.create | 201 with the same four fields | HE-10: `req_cce5244cdd76471d85575b7bbcc3fdac`, `req_69a44a96866a4b52b637a5a5530dcd3c` | +| F2 | List envelope | `object: page`, `data`, `next`, `has_more`; `has_more` is true exactly when `next` is set. Paging and ordering are unchanged | HE-32: `req_c8c247cfd13145a2b92be5cad412508f`, `req_a311bf80ff0643db945aa5db0a78e95b` | +| F3 | Unknown list query key | Ignored; the page equals the request without it. Malformed query encoding (such as `?foo=%GG` or `;` separators) is still rejected locally, while the shared lists drop those pairs | HE-34: `req_c4cd4618f7264840ad7454e80ac7f83c` | +| F4 | Repeated `path`, `limit`, `order` or `page` | 400 `invalid_request_error`, param null, ``Failed to deserialize query string: duplicate field `` `` | HE-35: `req_30b3c8bfced64383bf13c9d5bbc44a74` | +| F5 | `path` names a missing directory | 200 `{object: page, data: [], next: null, has_more: false}` with a local workspace reader (Claude SDK adapter exception below) | HE-36: `req_a311bf80ff0643db945aa5db0a78e95b` | +| F6 | `path` names a regular file or a symlink to a directory | The same empty page with a local workspace reader; the link is never followed | HE-37: `req_25fb0220fa7d4157a783711622b5d580`, `req_58b6784dc49640c194910c719d5c8102` | +| F7 | `path` not in cleaned form (trailing or repeated separator, `.` or `..`) | 400 `invalid_request_error`, param null, `path must identify a non-reserved directory inside /workspace` | HE-38: `req_a7ee5a49d1d24529b9de877c7ec5bc58` | +| F8 | Other validation errors | Code `invalid_request_error`, param null: list relative or outside path `path must be an absolute directory inside /workspace`; malformed, foreign or stale page token `Invalid file page token for this request`; create relative, root, outside or NUL path `environment.files[0].path must be an absolute POSIX path inside /workspace`; create empty, `.` or `..` components `environment.files[0].path cannot contain empty, . or .. path components`; unknown create body field `Unknown parameter: ''.` with param `` | HE-16: `req_3fb9feef630c442ba1c136506458362d`, `req_a41eb84c48594daf8fb55b95d8b118bd`, `req_f592639cfe28416395187ae76ad18228`; HE-39: `req_cbfed6a0336e47a395f42b82b45b20d7`, `req_5c2494e082714500bccbadbf60c6195d` | +| F9 | Files.create or list on an `openai_hosted` Environment still `pending` | 400 `invalid_request_error`, param null, `the hosted environment is still provisioning; wait until it is connected before accessing files` | HE-18: `req_938b99e48d3c4f4ab685dcf9b29baa6f`, `req_ff81d9155bb841618d5d1bbc1c987fdf` | + +Decisions: + +- F5/F6 are result mapping only. The native directory helper walks the requested + path below the anchored root with `O_PATH | O_DIRECTORY | O_NOFOLLOW`. When that + walk fails with `ENOENT` or `ENOTDIR`, it reports the distinct result + `not_directory`: a missing component, a regular file, a symlink to a directory, + a dangling or external symlink, and a path through any of them. A symlink fails + at its own component, so no target is followed, opened or listed. Every + component is validated before any is opened, so an invalid request cannot become + an empty page. The daemon and gateway carry `not_directory` only for directory + reads. Core lists nothing for it only after the same confirmed release that a + listing needs. +- Real failures keep their errors. The existing `invalid_path` code also covers + unsafe entry names inside a directory, and `not_found` also covers a missing + workspace root and an entry removed during observation; mapping either would hide + a failure as an empty listing. A missing or replaced root keeps 404 or 503. A + root removed or replaced after it was opened fails lookups or reads as empty. + So before reporting `not_directory` or an empty listing, the helper reopens the + root path without following links and compares device and inode with the held + root; a removed or replaced root keeps 404. Link counts are not used, because + overlayfs can keep a nonzero count for a removed lower-layer directory. The + check relies on local filesystem inode pinning; network filesystems such as NFS + are not qualified. + Permission denial, observation errors, transport loss and uncertain output keep + 503, and tenant and Environment authorization run first. The shared workspace + path code and the write installer are unchanged. +- The list path is rejected instead of normalized. `..` and other non-clean forms + use the F7 message. Outside paths, backslash, control characters, invalid UTF-8 + and paths over 4096 bytes use the F8 absolute-directory message; only the + relative form was sampled. +- The create messages keep the observed `environment.files[0].path` field name, + which comes from the official service's shared file validation. Backslash, CR, + LF, invalid UTF-8 and overlong paths are unsampled and use the absolute-path + message. The accepted path set is unchanged. +- The first unknown create field in document order is reported. Its name is + repeated in the message and param only when it is at most 256 bytes of + printable UTF-8; otherwise the error keeps code `invalid_request_error` with + param null and the message `Unknown parameter.`, so the response stays bounded. + A field of the other union member (such as `file_id` on `inline`), missing or null fields, an + unknown `type` and invalid Base64 keep the local `invalid_request` code; none was + sampled. +- Every rejected page token uses the sampled token message, including a valid token + for other parameters or a changed directory. +- F9 runs after the tenant-scoped lookup and request validation, and before + source-file resolution or execution. It applies only when the stored type is + `openai_hosted` and its status is `pending`; a disconnected hosted Environment + and every `self_hosted` status keep the existing execution path. The official + order between validation and this check was not sampled. +- Core Web sends the cleaned form of a directory entered with a trailing slash. + The client and Web accept only the new envelope and 201. + +Deferred and unchanged: recursive listing (HE-30); creating parent directories, +overwrite and the 50 MiB inline limit (HE-11/12/15, which change the shared +installer); the Environment retrieve `files[]` projection (HE-03); Environment +events (HE-02); the `self_hosted` status value (HE-04); `self_hosted` Files +support, which the official service refuses and Core's daemon keeps; limit bounds, +the default path and ordering. Malformed query encoding (such as `?foo=%GG` or `;` +separators) stays a local rejection on this list, while the shared lists drop those +pairs; there is no official sample. +The official conflict and size-limit messages and the deleted-Session 404 message +are not adopted. The Claude SDK adapter directory reader, used only when a daemon has +no local workspace binding, is unchanged: F5 and F6 there keep 404 for a missing +path and 503 for a regular file or symlink. A Runtime image built before this batch +keeps the same 404/503 results until it is rebuilt. + +Go handler tests cover F1–F9 with foreign-equals-missing checks; real-PostgreSQL +Worker tests cover the `not_directory` mapping, its release confirmation and the +unchanged rejections; gateway, daemon and Rust helper tests cover the native +classification, including links to a directory and outside the workspace, dangling +links and a replaced root. The TS client and Web unit tests cover the envelope and +201. The opt-in official scripts replay the list and create rows through raw HTTP +and the pinned SDK. Live acceptance, the server gate and independent review are +recorded separately by the coordinator. diff --git a/contracts/agents-api/list-query-semantics.md b/contracts/agents-api/list-query-semantics.md index 1ab5c81ee..9ecde20ce 100644 --- a/contracts/agents-api/list-query-semantics.md +++ b/contracts/agents-api/list-query-semantics.md @@ -166,14 +166,16 @@ unsampled inputs: ### Deferred These remain registered differences and are not changed here: repeated Files -`purpose` values (SFT-18); unsampled overflowing limits; unknown and repeated keys -on the Environment Files list, which keeps its own strict parser (its limit range -errors now use the Beta code through the shared limit reader); Skill sole-version +`purpose` values (SFT-18); unsampled overflowing limits; Skill sole-version deletion and number reuse (SFT-01/02); Session deletion lifecycle (SES-29/30); whitespace input (SES-01..04); Template network forms (SFT-21/22); and response defaults (VA-11, SES-23/25). Malformed path IDs (SES-28), metadata and name error fields (VA-07/08/09), U+0000 (VA-10) and Template network codes (SFT-20) are addressed by the [validation error batch](official-semantics-alignment.md#validation-error-fields--september-23). +Unknown and repeated keys on the Environment Files list (HE-34/35) follow rows A1 +and B1 through the [Environment Files wire batch](environment-files.md#wire-alignment--september-23-2026); +that list still rejects malformed query encoding (such as `?foo=%GG` or `;` +separators), which the shared lists drop. ### Acceptance boundary diff --git a/contracts/agents-api/official-semantics-alignment.md b/contracts/agents-api/official-semantics-alignment.md index 1cd4cbeea..9e477bbbe 100644 --- a/contracts/agents-api/official-semantics-alignment.md +++ b/contracts/agents-api/official-semantics-alignment.md @@ -190,7 +190,9 @@ officially (SFT-21) and `disabled` with domains, which the official service accepts (SFT-22); non-canonical UUID spellings such as uppercase, braces or `urn:uuid:` still resolve to the same resource; Skill sole-version deletion and number reuse; Session deletion lifecycle; whitespace input; response defaults; -the Environment Files list query parser; and the Files `limit=abc` code. +and the Files `limit=abc` code. The Environment Files list query parser is aligned +for unknown and repeated keys by the [Environment Files wire batch](environment-files.md#wire-alignment--september-23-2026); +it still rejects malformed query encoding locally. Go handler tests cover every row. Real-PostgreSQL tests replay every path-ID route for malformed, missing and foreign identifiers (tenant B), with valid and diff --git a/contracts/agents-api/openapi.yaml b/contracts/agents-api/openapi.yaml index 8da7d07ce..3291782b6 100644 --- a/contracts/agents-api/openapi.yaml +++ b/contracts/agents-api/openapi.yaml @@ -540,11 +540,19 @@ definitions: items: $ref: '#/definitions/v1.EnvironmentFile' type: array + has_more: + type: boolean next: type: string x-nullable: true + object: + enum: + - page + type: string required: - data + - has_more + - object type: object v1.EnvironmentInfo: properties: @@ -2501,16 +2509,21 @@ paths: /agents/environments/{environment_id}/files: get: description: Lists direct regular files in one authorized self_hosted or qualified - local workspace directory. Local paths use the public /workspace root. This - partial implementation defaults to the workspace root and limit 20; recursive - scope, directory/symlink treatment and these defaults are not verified upstream - semantics. Sorts by case-sensitive path components, descending by default. - Keep the same path, order and limit when using page. Each page rereads the - complete bounded directory; changed file paths/sizes invalidate continuation - locally with 400. There is no snapshot guarantee. Truncated or uncertain native - results fail with 503 without returning a partial page. This read never starts - a Turn or admits model input. Actual transport disconnect/reconnect events - remain observable. + local workspace directory. Local paths use the public /workspace root and + must be in cleaned form. This partial implementation defaults to the workspace + root and limit 20; recursive scope and these defaults are not verified upstream + semantics. A missing path, a regular file or a symbolic link returns an empty + page; links are never followed. Daemons without a local workspace binding + use the Claude SDK adapter reader, which keeps 404 for a missing path and + 503 for a regular file or symbolic link. Well-formed unknown query keys are + ignored; malformed query encoding and a repeated supported key are rejected. + Sorts by case-sensitive path components, descending by default. Keep the same + path, order and limit when using page. Each page rereads the complete bounded + directory; changed file paths/sizes invalidate continuation locally with 400. + There is no snapshot guarantee. An openai_hosted Environment that has not + connected yet returns 400. Truncated or uncertain native results fail with + 503 without returning a partial page. This read never starts a Turn or admits + model input. Actual transport disconnect/reconnect events remain observable. parameters: - description: agents=v1 in: header @@ -2522,7 +2535,7 @@ paths: name: environment_id required: true type: string - - description: Absolute directory inside the Environment workspace + - description: Absolute directory in cleaned form inside /workspace in: query name: path type: string @@ -2581,14 +2594,15 @@ paths: consumes: - application/json description: Uploads standard Base64 bytes to a file beneath /workspace in a - qualified local Environment. Accepts inline bytes or a project-owned source - file_id through the same write path. Basic public hosted creation requires - explicit managed Runtime configuration. A private 50 MiB decoded-content limit - applies. The parent directory must exist. Replacement installs a new mode-0600 - inode; upstream overwrite metadata semantics remain unverified. Idle writes - exclude execution. Missing receipts return unavailable and retain a durable - mutation gate without automatic replay. Error/timing parity with upstream - remains unverified. + qualified local Environment and returns 201. Accepts inline bytes or a project-owned + source file_id through the same write path. Unknown body fields are rejected + with their name as param. Basic public hosted creation requires explicit managed + Runtime configuration; an openai_hosted Environment that has not connected + yet returns 400. A private 50 MiB decoded-content limit applies. The parent + directory must exist. Replacement installs a new mode-0600 inode; upstream + overwrite metadata semantics remain unverified. Idle writes exclude execution. + Missing receipts return unavailable and retain a durable mutation gate without + automatic replay. Error/timing parity with upstream remains unverified. parameters: - description: agents=v1 in: header @@ -2609,8 +2623,8 @@ paths: produces: - application/json responses: - "200": - description: OK + "201": + description: Created schema: $ref: '#/definitions/v1.EnvironmentFile' "400": diff --git a/contracts/agents-api/operation-evidence.md b/contracts/agents-api/operation-evidence.md index 40f9d0579..a8e104e55 100644 --- a/contracts/agents-api/operation-evidence.md +++ b/contracts/agents-api/operation-evidence.md @@ -1,6 +1,6 @@ # Pinned operation evidence inventory — 2026-09-23 -Baseline inventory of main `b5715912f09333e2b4449ec6f0eecaabce44c9b7`. The Session admission batch below updates creation and metadata validation, the list query tolerance batch (L) updates list and resource query handling, and the validation error batch (X) updates field error codes/params, malformed path IDs and U+0000 handling; historical evidence retains its original revision and scope. This inventory guides repeated qualification and does not assert complete compatibility. +Baseline inventory of main `b5715912f09333e2b4449ec6f0eecaabce44c9b7`. The Session admission batch below updates creation and metadata validation, the list query tolerance batch (L) updates list and resource query handling, the validation error batch (X) updates field error codes/params, malformed path IDs and U+0000 handling, and the Environment Files wire batch (G) updates Files.create/list status, envelope, query, path and empty-page behavior; historical evidence retains its original revision and scope. This inventory guides repeated qualification and does not assert complete compatibility. Baseline: `contracts/agents-api/upstream.json`, SDK **3.13.0**, upstream commit **d7c41efee1b0802b79f3f88a678ef2052b06e9ce**, `OpenAI-Beta: agents=v1`. AGENTS.md and relevant CONTRIBUTING.md compatibility, ownership and evidence rules govern this inventory. @@ -38,6 +38,7 @@ Repository paths below are relative to the inspected worktree; private evidence | W | `~/.parsar/remediation/20260923/file-resource-semantics/official-files/` and `official-skills/`: 56 owned resource requests, no Sessions/models; [qualified observations and limitations](file-resource-semantics.md). | | X | [Validation error fields](official-semantics-alignment.md#validation-error-fields--september-23); private `~/.parsar/remediation/20260923/campaign-scan-1/{vaults-agents,sessions,skills-files-templates}/findings.json` VA-07/08/09/10, SES-28, SFT-20 and the September 22 Session `metadata.a` null observation. Metadata/name field errors, U+0000 local limit, malformed path IDs and Template network codes; Go handler and real-PostgreSQL route/no-write tests, no model execution. | | L | [List query tolerance](list-query-semantics.md#list-query-tolerance--september-23-2026); private `~/.parsar/remediation/20260923/campaign-scan-1/{vaults-agents,sessions,skills-files-templates}/findings.json`: owned-collection unknown/repeated keys, limit bounds, Vault status union and Files empty purpose, plus unknown keys on a deleted Vault read and Agent delete. Rows A1–D2 of that section; no model execution. | +| G | [Environment Files wire alignment](environment-files.md#wire-alignment--september-23-2026); private `~/.parsar/remediation/20260923/campaign-scan-2/hosted-env/findings.json` HE-10, 16, 18, 32, 34–39 with raw records under `official/` (labels `fc01`–`fc17`, `fl01`–`fl16`, `files-*-pending`) and `run1/`: three owned hosted Sessions, all deleted; first official Environment Files observations. Rows F1–F9 of that section. Go handler, real-PostgreSQL Worker, gateway/daemon and Rust helper tests without a model; live acceptance is recorded with the batch. | | Y | [Artifact capture and listing](official-semantics-alignment.md#artifact-capture-and-listing--september-23); private `~/.parsar/remediation/20260923/campaign-scan-2/hosted-env/findings.json` HE-50..62 with raw records under `official/` (labels `al01`–`al09`, `ar01`–`ar04`, `ac01`–`ac05`, `ad01`/`ad02`): three owned Sessions and two tiny Turns, all deleted; first official Artifact observations. Rows A1–A4 of that section: symlink skip, republication, list envelope and malformed filter. Rust link tests, real-PostgreSQL store/HTTP and pinned-SDK tests without a model; live acceptance is recorded with the batch. | ## Per-operation evidence matrix @@ -72,8 +73,8 @@ Paths in the appendix include `/v1`. SDK names here omit `client.`. `P` means pa | 24 | beta.agents.sessions.subagents.turns.list | P: persisted child history | None located | B recorded Live paging/cold continuation | Full concurrent/cancel ordering and accounting | | 25 | beta.agents.sessions.subagents.turns.items.list | P: exact child-and-Turn Items | None located | B recorded Live six-read matrix | Complete union and live ordering, overlapping mutation/cursors | | 26 | beta.agents.environments.retrieve | P: durable status, safe configured installation metadata | None located | D/I/K recorded native readiness and metadata | All lifecycle timing and installation inventory; configured metadata is not arbitrary workspace discovery | -| 27 | beta.agents.environments.files.create | P: inline/source-file copy to qualified workspace | None located | F/D/K recorded real copy, hashes/consumption/retention | 50 MiB local bound, parent/path/overwrite/error/unknown-write semantics | -| 28 | beta.agents.environments.files.list | P: direct regular-file directory, opaque cursor | None located | F/D/K recorded live workspace listing | 1,024-entry prefilter bound; no recursion/symlinks; exact defaults/path/errors/mutation invalidation unknown | +| 27 | beta.agents.environments.files.create | P: inline/source-file copy to qualified workspace; 201; observed path, unknown-field and provisioning errors | G HE-10 success/nested/empty, HE-16 path forms and unknown field, HE-18 pending | F/D/K recorded real copy, hashes/consumption/retention; G handler tests | 50 MiB local bound and no parent creation (official 5 MiB, creates parents); overwrite/conflict messages, other body errors and unknown-write semantics | +| 28 | beta.agents.environments.files.list | P: direct regular-file directory, opaque cursor; `page` envelope; unknown keys ignored (malformed query encoding still rejected locally), repeated keys rejected; missing, file and symlink paths list empty without following on local workspace readers (the Claude SDK adapter reader keeps 404/503); cleaned-path, token and provisioning errors | G HE-32 envelope, HE-34/35 query keys, HE-36/37 empty pages, HE-38/39 path and token errors, HE-18 pending | F/D/K recorded live workspace listing; G handler, Worker and native helper tests | 1,024-entry prefilter bound; no recursion (HE-30); exact defaults, unsampled path forms and mutation invalidation unknown | | 29 | beta.agents.environments.templates.create | P: reusable network/files/env/setup/packages/Skills/Plugins/capability config, 201; network rejections use `invalid_request_error` | R `template-create`; V nullable Skill selector projection; X SFT-20 | C DB; I/K recorded real frozen-reference initialization | Restricted forms and unqualified combinations; complete hosted initialization semantics | | 30 | beta.agents.environments.templates.retrieve | P: safe resource read | R `template-read`, deleted owned read; V nullable Skill selector projection | C DB; I/K recorded reference workflow | Full field/default/redaction parity; no live-secret projection inference | | 31 | beta.agents.environments.templates.update | P: field replacement/null clearing, empty timestamp touch; network rejections use `invalid_request_error` | R `template-patch`, `template-null`, `template-noop`; V nullable Skill selector projection; X SFT-20 | C DB no-op; I/K recorded frozen Session behavior | Template update and referencing Session selection are distinct; composition and null inheritance are qualified separately in environment-templates.md/template-null-selection.md; uncommon fields/errors remain unverified | @@ -107,11 +108,11 @@ Paths in the appendix include `/v1`. SDK names here omit `client.`. `P` means pa ## Remaining gaps without task ordering -1. **Public generic semantics:** sampled create/event/envelope/error/no-op corrections are merged. L aligns unknown/repeated list keys, sampled limit bounds and single-resource unknown keys. X gives malformed path IDs on every Beta, Files and Skills route the exact missing-resource response, reports metadata/name field errors with official code and param, maps Template network rejections to `invalid_request_error`, and rejects U+0000 in stored strings as a documented local limit (the official service stores it). Other resource-by-resource omissions/null/default/error params, overflowing limits, Environment Files list query tolerance, list caps, concurrent mutation and deletion require separate evidence. +1. **Public generic semantics:** sampled create/event/envelope/error/no-op corrections are merged. L aligns unknown/repeated list keys, sampled limit bounds and single-resource unknown keys. X gives malformed path IDs on every Beta, Files and Skills route the exact missing-resource response, reports metadata/name field errors with official code and param, maps Template network rejections to `invalid_request_error`, and rejects U+0000 in stored strings as a documented local limit (the official service stores it). G aligns Environment Files list query tolerance and the sampled Files.create/list errors. Other resource-by-resource omissions/null/default/error params, overflowing limits, list caps, concurrent mutation and deletion require separate evidence. 2. **Session differences:** The Session admission batch removes idle `none` creation and empty metadata update. Local durable creation idempotency remains an explicit difference. Whitespace-only input succeeds officially but is rejected by the existing Core message validator; this newly observed difference is queued separately. Session agent updates, newer Environment shapes and root Item turn_id are baseline-upgrade questions. 3. **Template/Skill composition:** shared env/files/setup/packages selection is covered by the composition batch; template-reference null network/capability lists are covered by the null-selection batch. Official derived capability-directory projection remains different. Skill content/default metadata are covered by file-resource-semantics.md; sole-version deletion, visibility and broader numbering/error behavior remain unverified. 4. **Execution coverage:** use T's qualified matrix, not a blanket missing-image/structured-output claim. MiniMax functions/service MCP, optional tool combinations, unsupported images/placements and broader native lifecycle are explicit restrictions. PTC omission retains approved native behavior; Claude/MiniMax public Usage remains null; child settlement cadence/native close limits remain visible. No second executor/model loop or guessed counters are justified. -5. **Workspace and resources:** live Files bounds, symlink/path/cursor choices, artifact capture edges for hard links/special files and cancellation (Y aligns output symlinks, republication and the list envelope), full Environment metadata/lifecycle and Vault archive/in-flight-token semantics remain partial or unknown. Retired Core-managed E2B acceptance cannot qualify current user enrollment. +5. **Workspace and resources:** live Files bounds, recursion and parent creation (G aligns the sampled envelope, empty pages and path errors), artifact capture edges for hard links/special files and cancellation (Y aligns output symlinks, republication and the list envelope), full Environment metadata/lifecycle and Vault archive/in-flight-token semantics remain partial or unknown. Retired Core-managed E2B acceptance cannot qualify current user enrollment. ## Enumeration and wording mismatches diff --git a/contracts/agents-api/v1/environment_files.go b/contracts/agents-api/v1/environment_files.go index 8272328c9..4427a565d 100644 --- a/contracts/agents-api/v1/environment_files.go +++ b/contracts/agents-api/v1/environment_files.go @@ -15,7 +15,11 @@ type EnvironmentFile struct { SizeBytes int64 `json:"size_bytes" binding:"required" minimum:"0"` } +// EnvironmentFileList is the official token page: has_more is true exactly +// when next carries a continuation token. type EnvironmentFileList struct { - Data []EnvironmentFile `json:"data" binding:"required"` - Next *string `json:"next" extensions:"x-nullable"` + Object string `json:"object" binding:"required" enums:"page"` + Data []EnvironmentFile `json:"data" binding:"required"` + Next *string `json:"next" extensions:"x-nullable"` + HasMore bool `json:"has_more" binding:"required"` } diff --git a/docs/web/protocol-coverage.md b/docs/web/protocol-coverage.md index e1e9012a5..4ec7156fa 100644 --- a/docs/web/protocol-coverage.md +++ b/docs/web/protocol-coverage.md @@ -288,8 +288,10 @@ the Core key binding. Agents Core Web's local proxy owns the bearer server-side. Workspace, `limit=20`, the selected `asc` or `desc` order, and Core's opaque `page` token for continuation while preserving the applied directory, order, and limit. -- The client accepts only HTTP 200 and the recorded Core page projection - `{data,next}`. Every entry must have the matching Environment ID, +- The client accepts only HTTP 200 and the official page envelope + `{object:"page",data,next,has_more}`, where `has_more` is true exactly when + `next` is set. A directory typed with a trailing separator is sent in cleaned + form, which Core requires. Every entry must have the matching Environment ID, `object:"agent.environment.file"`, a safe absolute path, and a non-negative safe-integer `size_bytes`. A malformed success rejects the complete page; no partial result is accepted or retried automatically. @@ -316,8 +318,8 @@ the Core key binding. Agents Core Web's local proxy owns the bearer server-side. GET to distinguish currently present from absent, never a second DELETE. - Environment Files.create is a single-attempt exact union: strict standard Base64 `inline` bytes or a project-owned Source `file_id`, plus one canonical file path - beneath `/workspace/`. Source upload is bounded at 512 MiB while destination copy - is bounded at 50 MiB. The Web's primary flow is upload → returned Source ID → + beneath `/workspace/`. The client accepts only HTTP 201 Created. Source upload is + bounded at 512 MiB while destination copy is bounded at 50 MiB. The Web's primary flow is upload → returned Source ID → `file_id` copy; it never substitutes a local filename or path for that ID. A missing response can outlive caller cancellation. Web performs at most one read-only directory list as a clue; matching path and size cannot prove byte diff --git a/internal/agentdaemon/gateway/workspace_read.go b/internal/agentdaemon/gateway/workspace_read.go index f50c817c2..6724b16ce 100644 --- a/internal/agentdaemon/gateway/workspace_read.go +++ b/internal/agentdaemon/gateway/workspace_read.go @@ -73,6 +73,10 @@ func validWorkspaceOperationResult(result proto.WorkspaceReadResultPayload, requ if request.Operation == "directory" && result.Outcome == "completed" { return result.CloseAcknowledged && result.ErrorCode == "" && len(result.Data) == 0 && !result.Truncated && proto.ValidWorkspaceDirectory(result.Directory, request.MaxEntries) } + // Only a directory request can report that its path names no directory. + if request.Operation == "directory" && result.Outcome == "rejected" && result.ErrorCode == proto.WorkspaceReadNotDirectory { + return result.Directory == nil && len(result.Data) == 0 && !result.Truncated && !result.CloseAcknowledged + } return validWorkspaceReadResult(result, request.MaxBytes) } diff --git a/internal/agentdaemon/gateway/workspace_read_test.go b/internal/agentdaemon/gateway/workspace_read_test.go index 21a9b53ab..4c9b912f7 100644 --- a/internal/agentdaemon/gateway/workspace_read_test.go +++ b/internal/agentdaemon/gateway/workspace_read_test.go @@ -67,6 +67,42 @@ func TestWorkspaceReadRejectsIncompleteOrContradictoryReplies(t *testing.T) { } } +func TestWorkspaceDirectoryAcceptsNotDirectoryOnlyForDirectoryReads(t *testing.T) { + directory := proto.WorkspaceReadPayload{Handle: "prepared", EnvironmentID: "environment", Path: "missing", MaxEntries: 2} + for _, test := range []struct { + directory bool + result proto.WorkspaceReadResultPayload + accepted bool + }{ + {true, proto.WorkspaceReadResultPayload{Outcome: "rejected", ErrorCode: proto.WorkspaceReadNotDirectory}, true}, + {true, proto.WorkspaceReadResultPayload{Outcome: "rejected", ErrorCode: "not_found"}, true}, + {true, proto.WorkspaceReadResultPayload{Outcome: "rejected", ErrorCode: proto.WorkspaceReadNotDirectory, CloseAcknowledged: true}, false}, + {true, proto.WorkspaceReadResultPayload{Outcome: "rejected", ErrorCode: proto.WorkspaceReadNotDirectory, Directory: &proto.WorkspaceDirectoryResult{Entries: []proto.WorkspaceDirectoryEntry{}}}, false}, + {true, proto.WorkspaceReadResultPayload{Outcome: "unknown", ErrorCode: proto.WorkspaceReadNotDirectory}, false}, + {true, proto.WorkspaceReadResultPayload{Outcome: "completed", CloseAcknowledged: true, ErrorCode: proto.WorkspaceReadNotDirectory, Directory: &proto.WorkspaceDirectoryResult{Entries: []proto.WorkspaceDirectoryEntry{}}}, false}, + {false, proto.WorkspaceReadResultPayload{Outcome: "rejected", ErrorCode: proto.WorkspaceReadNotDirectory}, false}, + } { + s := NewSession(newFakeConn(), "device", "tenant", "test", nil, nil) + done := make(chan error, 1) + go func() { + var err error + if test.directory { + _, err = s.ListWorkspaceDirectory(t.Context(), directory) + } else { + _, err = s.ReadWorkspaceFile(t.Context(), workspaceReadRequest()) + } + done <- err + }() + request := <-s.sendCh + reply, _ := proto.NewEnvelope(proto.TypeWorkspaceReadResult, request.ID, test.result) + s.dispatch(reply) + if err := <-done; (err == nil) != test.accepted { + t.Fatal("directory result validation changed", test.directory, test.result, err) + } + s.Close("test") + } +} + func TestWorkspaceReadObserverCancellationDoesNotSendCancelOrRetry(t *testing.T) { s := NewSession(newFakeConn(), "device", "tenant", "test", nil, nil) defer s.Close("test") diff --git a/internal/agentdaemon/proto/workspace_read.go b/internal/agentdaemon/proto/workspace_read.go index 5e6f7dcb6..428224554 100644 --- a/internal/agentdaemon/proto/workspace_read.go +++ b/internal/agentdaemon/proto/workspace_read.go @@ -6,6 +6,9 @@ const ( WorkspaceReadMaxBytes = 1 << 20 WorkspaceReadMaxRequestBytes = 8 << 10 WorkspaceReadMaxIDBytes = 128 + // WorkspaceReadNotDirectory rejects a directory read whose own path is + // missing, a regular file or a symbolic link. The link is never followed. + WorkspaceReadNotDirectory = "not_directory" ) // WorkspaceReadPayload targets one existing resource on the current daemon connection. diff --git a/packages/agents-client/src/client.test.ts b/packages/agents-client/src/client.test.ts index 74538b0a5..48d0e2746 100644 --- a/packages/agents-client/src/client.test.ts +++ b/packages/agents-client/src/client.test.ts @@ -1172,6 +1172,7 @@ describe("OpenAIAgentsClient", () => { const calls: FetchCall[] = []; const controller = new AbortController(); const page = { + object: "page", data: [ { environment_id: "environment/one", @@ -1181,6 +1182,7 @@ describe("OpenAIAgentsClient", () => { }, ], next: null, + has_more: false, } as const; const client = new OpenAIAgentsClient({ baseUrl: "https://core.example/v1/", @@ -1205,12 +1207,12 @@ describe("OpenAIAgentsClient", () => { it("uses the fixed public /workspace root when path is omitted and validates direct children", async () => { const calls: FetchCall[] = []; - const page = { data: [{ + const page = { object: "page", data: [{ environment_id: "environment", object: "agent.environment.file", path: "/workspace/report.json", size_bytes: 3, - }], next: null }; + }], next: null, has_more: false }; const client = new OpenAIAgentsClient({ fetch: recordingFetch(jsonResponse(page), calls) }); await expect(client.listEnvironmentFiles("environment", { order: "asc" })) @@ -1218,7 +1220,7 @@ describe("OpenAIAgentsClient", () => { expect(new URL(String(calls[0]?.input), "https://web.example").searchParams.has("path")).toBe(false); const nested = new OpenAIAgentsClient({ fetch: recordingFetch(jsonResponse({ - data: [{ ...page.data[0], path: "/workspace/nested/report.json" }], next: null, + ...page, data: [{ ...page.data[0], path: "/workspace/nested/report.json" }], }), []) }); await expect(nested.listEnvironmentFiles("environment", { order: "asc" })) .rejects.toMatchObject({ status: 502, code: "invalid_environment_files" }); @@ -1226,12 +1228,12 @@ describe("OpenAIAgentsClient", () => { it("accepts an explicit self-hosted Workspace root and validates direct children there", async () => { const calls: FetchCall[] = []; - const page = { data: [{ + const page = { object: "page", data: [{ environment_id: "environment", object: "agent.environment.file", path: "/test/report.json", size_bytes: 3, - }], next: null }; + }], next: null, has_more: false }; const client = new OpenAIAgentsClient({ fetch: recordingFetch(jsonResponse(page), calls) }); await expect(client.listEnvironmentFiles("environment", { path: "/test", order: "asc" })) @@ -1260,18 +1262,24 @@ describe("OpenAIAgentsClient", () => { it.each([ null, {}, - { data: [], next: null, extra: true }, - { data: [], next: "" }, - { data: [], next: "opaque-next" }, - { data: [{ environment_id: "environment", object: "agent.environment.file", path: "/workspace/file", size_bytes: 1 }], next: "opaque-next" }, - { data: [{ environment_id: "environment", object: "agent.environment.file", path: "/workspace/file", size_bytes: 1 }], next: "x".repeat(1025) }, - { data: null, next: null }, - { data: [{ environment_id: "other", object: "agent.environment.file", path: "/workspace/file", size_bytes: 1 }], next: null }, - { data: [{ environment_id: "environment", object: "file", path: "/workspace/file", size_bytes: 1 }], next: null }, - { data: [{ environment_id: "environment", object: "agent.environment.file", path: "relative", size_bytes: 1 }], next: null }, - { data: [{ environment_id: "environment", object: "agent.environment.file", path: "/workspace/../secret", size_bytes: 1 }], next: null }, - { data: [{ environment_id: "environment", object: "agent.environment.file", path: "/workspace/file", size_bytes: -1 }], next: null }, - { data: [{ environment_id: "environment", object: "agent.environment.file", path: "/workspace/file", size_bytes: 1, extra: true }], next: null }, + { data: [], next: null }, + { object: "list", data: [], next: null, has_more: false }, + { data: [], next: null, has_more: false }, + { object: "page", data: [], next: null }, + { object: "page", data: [], next: null, has_more: true }, + { object: "page", data: [], next: null, has_more: "false" }, + { object: "page", data: [], next: null, has_more: false, extra: true }, + { object: "page", data: [], next: "", has_more: true }, + { object: "page", data: [], next: "opaque-next", has_more: true }, + { object: "page", data: [{ environment_id: "environment", object: "agent.environment.file", path: "/workspace/file", size_bytes: 1 }], next: "opaque-next", has_more: true }, + { object: "page", data: [{ environment_id: "environment", object: "agent.environment.file", path: "/workspace/file", size_bytes: 1 }], next: "x".repeat(1025), has_more: true }, + { object: "page", data: null, next: null, has_more: false }, + { object: "page", data: [{ environment_id: "other", object: "agent.environment.file", path: "/workspace/file", size_bytes: 1 }], next: null, has_more: false }, + { object: "page", data: [{ environment_id: "environment", object: "file", path: "/workspace/file", size_bytes: 1 }], next: null, has_more: false }, + { object: "page", data: [{ environment_id: "environment", object: "agent.environment.file", path: "relative", size_bytes: 1 }], next: null, has_more: false }, + { object: "page", data: [{ environment_id: "environment", object: "agent.environment.file", path: "/workspace/../secret", size_bytes: 1 }], next: null, has_more: false }, + { object: "page", data: [{ environment_id: "environment", object: "agent.environment.file", path: "/workspace/file", size_bytes: -1 }], next: null, has_more: false }, + { object: "page", data: [{ environment_id: "environment", object: "agent.environment.file", path: "/workspace/file", size_bytes: 1, extra: true }], next: null, has_more: false }, ])("rejects a malformed Environment files page without retrying", async (page) => { const calls: FetchCall[] = []; const client = new OpenAIAgentsClient({ fetch: recordingFetch(jsonResponse(page), calls) }); @@ -1291,13 +1299,14 @@ describe("OpenAIAgentsClient", () => { path, size_bytes: 1, }); + const final = (data: unknown[]) => ({ object: "page", data, next: null, has_more: false }); const cases: Array<{ page: unknown; options: EnvironmentFileListOptions }> = [ - { page: { data: [file("/other/file")], next: null }, options: { path: "/workspace", order: "asc" } }, - { page: { data: [file("/workspace/nested/file")], next: null }, options: { path: "/workspace", order: "asc" } }, - { page: { data: [file("/workspace//file")], next: null }, options: { path: "/workspace", order: "asc" } }, - { page: { data: [file("/workspace/a"), file("/workspace/a")], next: null }, options: { path: "/workspace", order: "asc" } }, - { page: { data: [file("/workspace/b"), file("/workspace/a")], next: null }, options: { path: "/workspace", order: "asc" } }, - { page: { data: [file("/workspace/a"), file("/workspace/b")], next: null }, options: { path: "/workspace", order: "asc", limit: 1 } }, + { page: final([file("/other/file")]), options: { path: "/workspace", order: "asc" } }, + { page: final([file("/workspace/nested/file")]), options: { path: "/workspace", order: "asc" } }, + { page: final([file("/workspace//file")]), options: { path: "/workspace", order: "asc" } }, + { page: final([file("/workspace/a"), file("/workspace/a")]), options: { path: "/workspace", order: "asc" } }, + { page: final([file("/workspace/b"), file("/workspace/a")]), options: { path: "/workspace", order: "asc" } }, + { page: final([file("/workspace/a"), file("/workspace/b")]), options: { path: "/workspace", order: "asc", limit: 1 } }, ]; for (const entry of cases) { @@ -1562,7 +1571,7 @@ describe("OpenAIAgentsClient", () => { } as const; const client = new OpenAIAgentsClient({ baseUrl: "https://core.example/v1/", - fetch: recordingFetch(jsonResponse(result), calls), + fetch: recordingFetch(jsonResponse(result, 201), calls), }); await expect(client.createEnvironmentFile("environment/one", { @@ -1584,6 +1593,37 @@ describe("OpenAIAgentsClient", () => { expect(headers.get("Content-Type")).toBe("application/json"); }); + it("lists a continued Environment files page and requires the official has_more flag", async () => { + const file = (name: string) => ({ + environment_id: "environment", + object: "agent.environment.file", + path: `/workspace/${name}`, + size_bytes: 1, + }); + const page = { object: "page", data: [file("a"), file("b")], next: "opaque-next", has_more: true } as const; + const client = new OpenAIAgentsClient({ fetch: recordingFetch(jsonResponse(page), []) }); + await expect(client.listEnvironmentFiles("environment", { order: "asc", limit: 2 })).resolves.toEqual(page); + + const empty = { object: "page", data: [], next: null, has_more: false } as const; + const missing = new OpenAIAgentsClient({ fetch: recordingFetch(jsonResponse(empty), []) }); + await expect(missing.listEnvironmentFiles("environment", { path: "/workspace/missing" })).resolves.toEqual(empty); + }); + + it("requires 201 Created for an Environment file and never retries another success status", async () => { + const calls: FetchCall[] = []; + const result = { + environment_id: "environment", + object: "agent.environment.file", + path: "/workspace/notes.txt", + size_bytes: 1, + }; + const client = new OpenAIAgentsClient({ fetch: recordingFetch(jsonResponse(result, 200), calls) }); + + await expect(client.createEnvironmentFile("environment", { type: "inline", data: "YQ==", path: result.path })) + .rejects.toMatchObject({ status: 200 }); + expect(calls).toHaveLength(1); + }); + it("rejects invalid Source and Environment file inputs before making a request", async () => { const calls: FetchCall[] = []; const client = new OpenAIAgentsClient({ fetch: recordingFetch(jsonResponse({}), calls) }); diff --git a/packages/agents-client/src/client.ts b/packages/agents-client/src/client.ts index 095ab164c..c41af13bf 100644 --- a/packages/agents-client/src/client.ts +++ b/packages/agents-client/src/client.ts @@ -229,7 +229,7 @@ function withQuery(path: string, params: URLSearchParams): string { const environmentResourceFields = new Set(["id", "object", "type", "status", "files", "plugins", "skills"]); const environmentFileFields = new Set(["environment_id", "object", "path", "size_bytes"]); -const environmentFileListFields = new Set(["data", "next"]); +const environmentFileListFields = new Set(["object", "data", "next", "has_more"]); const sourceFileFields = new Set([ "id", "object", "bytes", "created_at", "filename", "purpose", "status", "expires_at", "status_details", ]); @@ -1568,6 +1568,8 @@ function projectEnvironmentFileList( if ( fields.length !== environmentFileListFields.size || fields.some((field) => !environmentFileListFields.has(field)) || + page.object !== "page" || + page.has_more !== (page.next !== null) || !Array.isArray(page.data) || !Number.isSafeInteger(limit) || limit < 1 || @@ -1624,7 +1626,7 @@ function projectEnvironmentFileList( }; }); - return { data: files, next: page.next as string | null }; + return { object: "page", data: files, next: page.next as string | null, has_more: page.has_more as boolean }; } export class OpenAIAgentsClient implements AgentCore { @@ -2100,7 +2102,7 @@ export class OpenAIAgentsClient implements AgentCore { const value = await this.request( `/agents/environments/${encodeURIComponent(environmentId)}/files`, { method: "POST", body: JSON.stringify(input), signal: options?.signal }, - 200, + 201, ); return projectEnvironmentFile(value, environmentId, input.path, expectedSize); } diff --git a/packages/agents-client/src/types.ts b/packages/agents-client/src/types.ts index 89eba3ab9..689bd0753 100644 --- a/packages/agents-client/src/types.ts +++ b/packages/agents-client/src/types.ts @@ -381,9 +381,12 @@ export interface EnvironmentFile { size_bytes: number; } +/** Official token page: has_more is true exactly when next carries a token. */ export interface EnvironmentFileList { + object: "page"; data: EnvironmentFile[]; next: string | null; + has_more: boolean; } export interface EnvironmentFileListOptions extends ReadOptions { diff --git a/packages/codex-executor/src/bin/directory.rs b/packages/codex-executor/src/bin/directory.rs index 983df1942..7ac7a56d2 100644 --- a/packages/codex-executor/src/bin/directory.rs +++ b/packages/codex-executor/src/bin/directory.rs @@ -1,11 +1,11 @@ -use rustix::fs::FileType; +use rustix::fs::{FileType, fstat}; #[path = "../directory.rs"] mod directory; use directory::observe; #[path = "../workspace_path.rs"] mod workspace_path; use serde_json::json; -use std::{io, path::Path}; +use std::{io, os::fd::OwnedFd, path::Path}; use workspace_path::{anchor, directory}; fn run() -> io::Result { @@ -13,15 +13,49 @@ fn run() -> io::Result { if args.len() != 3 { return Err(io::ErrorKind::InvalidInput.into()); } - let limit = args[2] + list(&args[0], &args[1], &args[2]) +} + +fn list(root: &str, relative: &str, limit: &str) -> io::Result { + let limit = limit .parse() .map_err(|_| io::Error::from(io::ErrorKind::InvalidInput))?; if !(1..=4096).contains(&limit) { return Err(io::ErrorKind::InvalidInput.into()); } - let root = anchor(Path::new(&args[0]))?; - let selected = directory(&root, &args[1])?; + // Validate every component first, so that a missing earlier component can + // never hide an invalid request as a listable-directory result. + if !relative.is_empty() + && relative.split('/').any(|part| { + part.is_empty() + || part == "." + || part == ".." + || part.contains(['\\', '\0', '\r', '\n']) + }) + { + return Err(io::ErrorKind::InvalidInput.into()); + } + let root_path = Path::new(root); + let root = anchor(root_path)?; + list_anchored(root_path, &root, relative, limit) +} + +fn list_anchored( + root_path: &Path, + root: &OwnedFd, + relative: &str, + limit: usize, +) -> io::Result { + let Some(selected) = select(root, relative)? else { + root_unchanged(root_path, root)?; + return Ok(json!({"version": 1, "error": "not_directory"})); + }; let result = observe(&selected, limit)?; + // Reading a removed directory ends like an empty one, so an empty listing + // also confirms that the workspace root is still in place. + if result.entries.is_empty() { + root_unchanged(root_path, root)?; + } let entries: Vec<_> = result .entries .into_iter() @@ -38,6 +72,50 @@ fn run() -> io::Result { Ok(json!({"version": 1, "directory": {"entries": entries, "truncated": result.truncated}})) } +/// Resolves the requested path below the anchored root. `None` means that +/// path does not name a listable directory: a missing component, a regular file +/// or a symbolic link (never followed). Root, permission and observation +/// failures keep their existing codes. +fn select(root: &OwnedFd, relative: &str) -> io::Result> { + match directory(root, relative) { + Ok(selected) => Ok(Some(selected)), + Err(error) + if matches!( + error.kind(), + io::ErrorKind::NotFound | io::ErrorKind::NotADirectory + ) => + { + Ok(None) + } + Err(error) => Err(error), + } +} + +/// Confirms that the root path still names the held root directory. The path +/// is reopened without following links and compared by device and inode, which +/// does not depend on link counts that some filesystems (such as overlayfs) +/// keep for removed directories. A removed or replaced root is a missing +/// workspace, not a missing requested directory. +fn root_unchanged(root_path: &Path, root: &OwnedFd) -> io::Result<()> { + let current = match anchor(root_path) { + Ok(current) => current, + Err(error) + if matches!( + error.kind(), + io::ErrorKind::NotFound | io::ErrorKind::NotADirectory + ) => + { + return Err(io::ErrorKind::NotFound.into()); + } + Err(error) => return Err(error), + }; + let (held, now) = (fstat(root)?, fstat(¤t)?); + if held.st_dev != now.st_dev || held.st_ino != now.st_ino { + return Err(io::ErrorKind::NotFound.into()); + } + Ok(()) +} + fn main() -> std::process::ExitCode { let response = match run() { Ok(value) => value, @@ -58,3 +136,120 @@ fn main() -> std::process::ExitCode { Err(_) => std::process::ExitCode::FAILURE, } } + +#[cfg(test)] +mod tests { + use super::{list, list_anchored}; + use crate::directory::observe; + use crate::workspace_path::{anchor, directory}; + use std::{fs, io, os::unix::fs::symlink}; + + fn code(result: io::Result) -> String { + match result { + Ok(value) => value["error"].as_str().unwrap_or("listed").to_string(), + Err(error) => format!("{:?}", error.kind()), + } + } + + #[test] + fn only_the_requested_path_resolution_is_not_a_directory() { + let workspace = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + let root = workspace.path().join("workspace"); + fs::create_dir_all(root.join("d")).unwrap(); + fs::write(root.join("d/f.txt"), b"hidden").unwrap(); + fs::write(root.join("file.txt"), b"x").unwrap(); + fs::write(outside.path().join("secret"), b"secret").unwrap(); + symlink(root.join("d"), root.join("linkdir")).unwrap(); + symlink("d", root.join("relative-link")).unwrap(); + symlink(outside.path(), root.join("outside-link")).unwrap(); + symlink("missing", root.join("dangling")).unwrap(); + let root = root.to_str().unwrap(); + for path in [ + "missing", + "missing/deeper", + "file.txt", + "file.txt/deeper", + "linkdir", + "linkdir/f.txt", + "relative-link", + "outside-link", + "dangling", + ] { + let value = list(root, path, "10").unwrap(); + assert_eq!( + value, + serde_json::json!({"version": 1, "error": "not_directory"}), + "{path}" + ); + } + let listed = list(root, "d", "10").unwrap(); + assert_eq!(listed["directory"]["entries"][0]["name"], "f.txt"); + // Invalid requests are rejected before any component is opened. + for path in ["missing/..", "missing//x", "./d", "d/", "missing/a\\b"] { + assert_eq!(code(list(root, path, "10")), "InvalidInput", "{path}"); + } + // Root failures keep their existing classification. + let missing_root = format!("{root}/missing"); + assert_eq!(code(list(&missing_root, "", "10")), "NotFound"); + let linked_root = format!("{root}/linkdir"); + assert_eq!(code(list(&linked_root, "", "10")), "NotADirectory"); + let file_root = format!("{root}/file.txt"); + assert_eq!(code(list(&file_root, "", "10")), "NotADirectory"); + } + + fn not_found(result: io::Result) -> bool { + matches!(result, Err(error) if error.kind() == io::ErrorKind::NotFound) + } + + #[test] + fn a_root_removed_after_opening_is_a_missing_workspace() { + let workspace = tempfile::tempdir().unwrap(); + let root = workspace.path().join("workspace"); + fs::create_dir_all(root.join("d")).unwrap(); + let anchored = anchor(&root).unwrap(); + let listed = list_anchored(&root, &anchored, "missing", 10).unwrap(); + assert_eq!(listed["error"], "not_directory"); + let selected = directory(&anchored, "").unwrap(); + fs::remove_dir_all(&root).unwrap(); + // Reading the removed root ends like an empty directory. + assert!(observe(&selected, 10).unwrap().entries.is_empty()); + for path in ["", "d", "missing", "missing/deeper"] { + assert!( + not_found(list_anchored(&root, &anchored, path, 10)), + "{path}" + ); + } + } + + #[test] + fn a_root_replaced_after_opening_is_a_missing_workspace() { + let workspace = tempfile::tempdir().unwrap(); + let root = workspace.path().join("workspace"); + let moved = workspace.path().join("moved"); + fs::create_dir_all(root.join("d")).unwrap(); + fs::create_dir_all(root.join("empty")).unwrap(); + fs::write(root.join("d/f.txt"), b"x").unwrap(); + let anchored = anchor(&root).unwrap(); + fs::rename(&root, &moved).unwrap(); + fs::create_dir_all(root.join("d")).unwrap(); + // The held root stays authoritative for entries it still lists. + let listed = list_anchored(&root, &anchored, "d", 10).unwrap(); + assert_eq!(listed["directory"]["entries"][0]["name"], "f.txt"); + for path in ["missing", "empty"] { + assert!( + not_found(list_anchored(&root, &anchored, path, 10)), + "{path}" + ); + } + fs::remove_dir_all(&root).unwrap(); + symlink(&moved, &root).unwrap(); + assert!(not_found(list_anchored(&root, &anchored, "missing", 10))); + // An unchanged root still lists missing paths and empty directories. + let anchored = anchor(&moved).unwrap(); + let listed = list_anchored(&moved, &anchored, "missing", 10).unwrap(); + assert_eq!(listed["error"], "not_directory"); + let listed = list_anchored(&moved, &anchored, "empty", 10).unwrap(); + assert_eq!(listed["directory"]["entries"], serde_json::json!([])); + } +} diff --git a/services/agents-api/internal/api/environment_files.go b/services/agents-api/internal/api/environment_files.go index 13f5b559a..2646eb8fc 100644 --- a/services/agents-api/internal/api/environment_files.go +++ b/services/agents-api/internal/api/environment_files.go @@ -2,6 +2,7 @@ package api import ( "context" + "encoding/json" "net/http" "path" "slices" @@ -24,13 +25,13 @@ func WithEnvironmentDirectoryReader(reader EnvironmentDirectoryReader) Option { } // @Summary List live Environment files -// @Description Lists direct regular files in one authorized self_hosted or qualified local workspace directory. Local paths use the public /workspace root. This partial implementation defaults to the workspace root and limit 20; recursive scope, directory/symlink treatment and these defaults are not verified upstream semantics. Sorts by case-sensitive path components, descending by default. Keep the same path, order and limit when using page. Each page rereads the complete bounded directory; changed file paths/sizes invalidate continuation locally with 400. There is no snapshot guarantee. Truncated or uncertain native results fail with 503 without returning a partial page. This read never starts a Turn or admits model input. Actual transport disconnect/reconnect events remain observable. +// @Description Lists direct regular files in one authorized self_hosted or qualified local workspace directory. Local paths use the public /workspace root and must be in cleaned form. This partial implementation defaults to the workspace root and limit 20; recursive scope and these defaults are not verified upstream semantics. A missing path, a regular file or a symbolic link returns an empty page; links are never followed. Daemons without a local workspace binding use the Claude SDK adapter reader, which keeps 404 for a missing path and 503 for a regular file or symbolic link. Well-formed unknown query keys are ignored; malformed query encoding and a repeated supported key are rejected. Sorts by case-sensitive path components, descending by default. Keep the same path, order and limit when using page. Each page rereads the complete bounded directory; changed file paths/sizes invalidate continuation locally with 400. There is no snapshot guarantee. An openai_hosted Environment that has not connected yet returns 400. Truncated or uncertain native results fail with 503 without returning a partial page. This read never starts a Turn or admits model input. Actual transport disconnect/reconnect events remain observable. // @Tags Environments // @Produce json // @Security BearerAuth // @Param OpenAI-Beta header string true "agents=v1" // @Param environment_id path string true "Environment ID" -// @Param path query string false "Absolute directory inside the Environment workspace" +// @Param path query string false "Absolute directory in cleaned form inside /workspace" // @Param limit query int false "Maximum file count; local default 20" minimum(1) maximum(100) // @Param order query string false "Case-sensitive path-component order; omit for descending, explicit empty values are invalid" Enums(asc,desc) default(desc) // @Param page query string false "Opaque continuation token; keep path, order and limit unchanged" @@ -44,10 +45,10 @@ func (h *Handler) listEnvironmentFiles(w http.ResponseWriter, r *http.Request) { return } options, ok := readEnvironmentFileQuery(w, r, environment) - if !ok { + if !ok || !environmentFilesAccessible(w, environment) { return } - if h.directoryReader == nil { + if h.directoryReader == nil || !execution.LocalWorkspaceConfiguration(environment.Configuration) { writeStoreError(w, r, execution.ErrExecutionUnavailable) return } @@ -84,8 +85,26 @@ func (h *Handler) listEnvironmentFiles(w http.ResponseWriter, r *http.Request) { }) response, err := environmentFilePage(files, options) if err != nil { - writeStoreError(w, r, err) + if !writeFieldError(w, err) { + writeStoreError(w, r, err) + } return } writeJSON(w, http.StatusOK, response) } + +var errHostedEnvironmentProvisioning = &fieldError{message: "the hosted environment is still provisioning; wait until it is connected before accessing files"} + +// environmentFilesAccessible rejects Files operations on an openai_hosted +// Environment whose first connection has not been observed (HE-18). Callers +// run it after the tenant-scoped lookup, so foreign Environments stay missing. +func environmentFilesAccessible(w http.ResponseWriter, environment store.Environment) bool { + var configuration struct { + Type string `json:"type"` + } + if environment.Status == "pending" && json.Unmarshal(environment.Configuration, &configuration) == nil && configuration.Type == "openai_hosted" { + writeFieldError(w, errHostedEnvironmentProvisioning) + return false + } + return true +} diff --git a/services/agents-api/internal/api/environment_files_create.go b/services/agents-api/internal/api/environment_files_create.go index ca00e6743..42f202cae 100644 --- a/services/agents-api/internal/api/environment_files_create.go +++ b/services/agents-api/internal/api/environment_files_create.go @@ -1,13 +1,17 @@ package api import ( + "bytes" "context" "encoding/base64" + "encoding/json" "io" "net/http" - "path" + "slices" "strings" "time" + "unicode" + "unicode/utf8" v1 "github.com/MiniMax-AI-Dev/parsar/contracts/agents-api/v1" "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/proto" @@ -25,7 +29,7 @@ func WithEnvironmentFileWriter(writer EnvironmentFileWriter) Option { } // @Summary Create an Environment file from inline bytes or a source file -// @Description Uploads standard Base64 bytes to a file beneath /workspace in a qualified local Environment. Accepts inline bytes or a project-owned source file_id through the same write path. Basic public hosted creation requires explicit managed Runtime configuration. A private 50 MiB decoded-content limit applies. The parent directory must exist. Replacement installs a new mode-0600 inode; upstream overwrite metadata semantics remain unverified. Idle writes exclude execution. Missing receipts return unavailable and retain a durable mutation gate without automatic replay. Error/timing parity with upstream remains unverified. +// @Description Uploads standard Base64 bytes to a file beneath /workspace in a qualified local Environment and returns 201. Accepts inline bytes or a project-owned source file_id through the same write path. Unknown body fields are rejected with their name as param. Basic public hosted creation requires explicit managed Runtime configuration; an openai_hosted Environment that has not connected yet returns 400. A private 50 MiB decoded-content limit applies. The parent directory must exist. Replacement installs a new mode-0600 inode; upstream overwrite metadata semantics remain unverified. Idle writes exclude execution. Missing receipts return unavailable and retain a durable mutation gate without automatic replay. Error/timing parity with upstream remains unverified. // @Tags Environments // @Accept json // @Produce json @@ -33,7 +37,7 @@ func WithEnvironmentFileWriter(writer EnvironmentFileWriter) Option { // @Param OpenAI-Beta header string true "agents=v1" // @Param environment_id path string true "Environment ID" // @Param request body v1.EnvironmentFileCreateRequest true "Inline bytes or source file ID and absolute workspace path" -// @Success 200 {object} v1.EnvironmentFile +// @Success 201 {object} v1.EnvironmentFile // @Failure 400,401,404,409,413,500,503 {object} v1.ErrorResponse // @Router /agents/environments/{environment_id}/files [post] func (h *Handler) createEnvironmentFile(w http.ResponseWriter, r *http.Request) { @@ -49,6 +53,14 @@ func (h *Handler) createEnvironmentFile(w http.ResponseWriter, r *http.Request) } var request v1.EnvironmentFileCreateRequest fields := []string{"type", "path", "data", "file_id"} + if field, found := unknownBodyField(raw, fields...); found { + if !echoableField(field) { + writeFieldError(w, errUnknownEnvironmentFileField) + return + } + writeFieldError(w, &fieldError{param: field, message: "Unknown parameter: '" + field + "'."}) + return + } if err := decodeInputObject(raw, &request, fields...); err != nil || request.Path == nil { writeStoreError(w, r, store.ErrInvalidInput) return @@ -74,8 +86,8 @@ func (h *Handler) createEnvironmentFile(w http.ResponseWriter, r *http.Request) writeStoreError(w, r, store.ErrInvalidInput) return } - if !validEnvironmentFilePath(*request.Path) || path.Clean(*request.Path) != *request.Path || !strings.HasPrefix(*request.Path, "/workspace/") { - writeStoreError(w, r, store.ErrInvalidInput) + if err := environmentFileCreatePathError(*request.Path); err != nil { + writeFieldError(w, err) return } var data []byte @@ -89,7 +101,11 @@ func (h *Handler) createEnvironmentFile(w http.ResponseWriter, r *http.Request) writeStoreError(w, r, store.ErrSourceFileTooLarge) return } - } else { + } + if !environmentFilesAccessible(w, environment) { + return + } + if request.Type == "file_id" { if !h.sourceFilesAvailable(w) { return } @@ -127,5 +143,75 @@ func (h *Handler) createEnvironmentFile(w http.ResponseWriter, r *http.Request) writeStoreError(w, r, execution.ErrExecutionUnavailable) return } - writeJSON(w, http.StatusOK, v1.EnvironmentFile{EnvironmentID: environment.ID, Object: "agent.environment.file", Path: *request.Path, SizeBytes: size}) + writeJSON(w, http.StatusCreated, v1.EnvironmentFile{EnvironmentID: environment.ID, Object: "agent.environment.file", Path: *request.Path, SizeBytes: size}) +} + +// Official Files.create path errors (HE-16); the messages keep the observed field name. +var ( + errEnvironmentFileCreatePath = &fieldError{message: "environment.files[0].path must be an absolute POSIX path inside /workspace"} + errEnvironmentFileCreateComponents = &fieldError{message: "environment.files[0].path cannot contain empty, . or .. path components"} +) + +// environmentFileCreatePathError accepts exactly the canonical absolute paths +// below /workspace; the accepted set is unchanged, only the errors are specific. +func environmentFileCreatePathError(value string) error { + if !strings.HasPrefix(value, "/") { + return errEnvironmentFileCreatePath + } + for _, component := range strings.Split(value[1:], "/") { + if component == "" || component == "." || component == ".." { + return errEnvironmentFileCreateComponents + } + } + if len(value) > 4096 || !utf8.ValidString(value) || strings.ContainsAny(value, "\\\x00\r\n") || !strings.HasPrefix(value, "/workspace/") { + return errEnvironmentFileCreatePath + } + return nil +} + +// errUnknownEnvironmentFileField keeps the official code for an unknown field +// whose name is not echoed. +var errUnknownEnvironmentFileField = &fieldError{message: "Unknown parameter."} + +// echoableField bounds the caller-supplied name that an unknown-field error +// repeats in both message and param; JSON escaping can grow each byte sixfold. +// encoding/json has already replaced invalid bytes and lone surrogates with +// U+FFFD, so a name containing it is not repeated either. +func echoableField(field string) bool { + if len(field) > 256 || !utf8.ValidString(field) || strings.ContainsRune(field, utf8.RuneError) { + return false + } + for _, r := range field { + if !unicode.IsPrint(r) { + return false + } + } + return true +} + +// unknownBodyField returns the first top-level member outside allowed, in +// document order. Malformed and non-object bodies are left to the caller's +// decoder, which reports them with the existing malformed-body error. +func unknownBodyField(raw []byte, allowed ...string) (string, bool) { + if !json.Valid(raw) { + return "", false + } + decoder := json.NewDecoder(bytes.NewReader(raw)) + if token, err := decoder.Token(); err != nil || token != json.Delim('{') { + return "", false + } + for decoder.More() { + token, err := decoder.Token() + if err != nil { + return "", false + } + if key, _ := token.(string); !slices.Contains(allowed, key) { + return key, true + } + var value json.RawMessage + if decoder.Decode(&value) != nil { + return "", false + } + } + return "", false } diff --git a/services/agents-api/internal/api/environment_files_create_test.go b/services/agents-api/internal/api/environment_files_create_test.go index 56bbcb61d..32d4f9749 100644 --- a/services/agents-api/internal/api/environment_files_create_test.go +++ b/services/agents-api/internal/api/environment_files_create_test.go @@ -70,7 +70,7 @@ func TestEnvironmentFileCreateInlineAndLocalListing(t *testing.T) { body, _ := json.Marshal(map[string]any{"type": "inline", "data": base64.StdEncoding.EncodeToString(data), "path": "/workspace/input.bin"}) w := requestCreateEnvironmentFile(h, f.environment.ID, string(body), "files-key") var file v1.EnvironmentFile - if w.Code != 200 || json.Unmarshal(w.Body.Bytes(), &file) != nil { + if w.Code != 201 || json.Unmarshal(w.Body.Bytes(), &file) != nil { t.Fatalf("create: %d %s", w.Code, w.Body) } if f.writes != 1 || f.path != "input.bin" || !bytes.Equal(f.data, data) || f.readEnvironment.ID != f.environment.ID || file.EnvironmentID != f.environment.ID || file.Path != "/workspace/input.bin" || file.Object != "agent.environment.file" || file.SizeBytes != int64(len(data)) { @@ -129,7 +129,7 @@ func TestEnvironmentFileCreateAuthorityAndUncertainResults(t *testing.T) { } f.environment.Configuration = json.RawMessage(`{"type":"self_hosted","workspace_directory":"/workspace"}`) f.writes, f.wrongSize = 0, false - if w := requestCreateEnvironmentFile(h, f.environment.ID, body, "files-key"); w.Code != 200 || f.writes != 1 { + if w := requestCreateEnvironmentFile(h, f.environment.ID, body, "files-key"); w.Code != 201 || f.writes != 1 { t.Fatal("enrolled local workspace write rejected", w.Code) } } diff --git a/services/agents-api/internal/api/environment_files_cursor.go b/services/agents-api/internal/api/environment_files_cursor.go index 7fabf2dfd..ec25dc535 100644 --- a/services/agents-api/internal/api/environment_files_cursor.go +++ b/services/agents-api/internal/api/environment_files_cursor.go @@ -9,9 +9,12 @@ import ( "io" v1 "github.com/MiniMax-AI-Dev/parsar/contracts/agents-api/v1" - "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" ) +// errEnvironmentFilePage reports every malformed, foreign or stale continuation +// with the official Files.list token error (HE-39). +var errEnvironmentFilePage = &fieldError{message: "Invalid file page token for this request"} + type environmentFileCursor struct { Version int `json:"v"` Binding string `json:"b"` @@ -28,16 +31,16 @@ func environmentFilesDigest(value any) string { func decodeEnvironmentFileCursor(token, binding string) (environmentFileCursor, error) { var cursor environmentFileCursor if len(token) > 1024 { - return cursor, store.ErrInvalidInput + return cursor, errEnvironmentFilePage } data, err := base64.RawURLEncoding.Strict().DecodeString(token) if err != nil { - return cursor, store.ErrInvalidInput + return cursor, errEnvironmentFilePage } decoder := json.NewDecoder(bytes.NewReader(data)) decoder.DisallowUnknownFields() if decoder.Decode(&cursor) != nil || decoder.Decode(new(any)) != io.EOF || cursor.Version != 1 || cursor.Binding != binding || len(cursor.Fingerprint) != 64 || cursor.Offset <= 0 { - return environmentFileCursor{}, store.ErrInvalidInput + return environmentFileCursor{}, errEnvironmentFilePage } return cursor, nil } @@ -47,16 +50,16 @@ func environmentFilePage(files []v1.EnvironmentFile, options environmentFileOpti start := 0 if cursor := options.cursor; cursor != nil { if cursor.Fingerprint != fingerprint || cursor.Offset >= len(files) || cursor.Offset%options.limit != 0 { - return v1.EnvironmentFileList{}, store.ErrInvalidInput + return v1.EnvironmentFileList{}, errEnvironmentFilePage } start = cursor.Offset } end := min(start+options.limit, len(files)) - response := v1.EnvironmentFileList{Data: files[start:end]} + response := v1.EnvironmentFileList{Object: "page", Data: files[start:end]} if end < len(files) { data, _ := json.Marshal(environmentFileCursor{Version: 1, Binding: options.binding, Fingerprint: fingerprint, Offset: end}) token := base64.RawURLEncoding.EncodeToString(data) - response.Next = &token + response.Next, response.HasMore = &token, true } return response, nil } diff --git a/services/agents-api/internal/api/environment_files_deadline_test.go b/services/agents-api/internal/api/environment_files_deadline_test.go index ea87ace2f..e27d3a5eb 100644 --- a/services/agents-api/internal/api/environment_files_deadline_test.go +++ b/services/agents-api/internal/api/environment_files_deadline_test.go @@ -41,7 +41,7 @@ func TestEnvironmentFilesReadOutlivesDefaultHTTPWriteDeadline(t *testing.T) { t.Fatal("invalid delayed response", response.StatusCode, err) } if status == http.StatusOK { - if string(body["data"]) != "[]" || string(body["next"]) != "null" { + if string(body["object"]) != `"page"` || string(body["data"]) != "[]" || string(body["next"]) != "null" || string(body["has_more"]) != "false" { t.Fatal("delayed page changed shape") } } else if body["error"] == nil || body["data"] != nil || body["next"] != nil { diff --git a/services/agents-api/internal/api/environment_files_query.go b/services/agents-api/internal/api/environment_files_query.go index 3bf7e51d1..68a288364 100644 --- a/services/agents-api/internal/api/environment_files_query.go +++ b/services/agents-api/internal/api/environment_files_query.go @@ -4,11 +4,9 @@ import ( "net/http" "net/url" "path" - "slices" "strings" "unicode/utf8" - "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/execution" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" ) @@ -21,16 +19,29 @@ type environmentFileOptions struct { cursor *environmentFileCursor } +var environmentFileQueryKeys = []string{"path", "limit", "order", "page"} + +// Official Files.list path errors (HE-38, HE-39). +var ( + errEnvironmentFileDirectory = &fieldError{message: "path must be an absolute directory inside /workspace"} + errEnvironmentFileCanonical = &fieldError{message: "path must identify a non-reserved directory inside /workspace"} +) + +// readEnvironmentFileQuery follows the shared list rules: unknown keys are +// ignored and a repeated supported key uses the Beta duplicate-field error. +// Unlike the shared lists, which drop malformed pairs, it rejects malformed +// query encoding. func readEnvironmentFileQuery(w http.ResponseWriter, r *http.Request, environment store.Environment) (environmentFileOptions, bool) { var options environmentFileOptions + // Malformed query encoding remains a local rejection; no official sample exists. q, err := url.ParseQuery(r.URL.RawQuery) if err != nil { writeStoreError(w, r, store.ErrInvalidInput) return options, false } - for key, values := range q { - if !slices.Contains([]string{"path", "limit", "order", "page"}, key) || len(values) != 1 || (key != "order" && values[0] == "") { - writeError(w, http.StatusBadRequest, "invalid_request", "Supported list parameters are path, limit, order and page, each supplied once with a nonempty value.") + for _, key := range environmentFileQueryKeys { + if len(q[key]) > 1 { + writeListDuplicateError(w, r, key, environmentFileQueryKeys) return options, false } } @@ -44,34 +55,24 @@ func readEnvironmentFileQuery(w http.ResponseWriter, r *http.Request, environmen if !ok { return options, false } - root := "/workspace" - if !execution.LocalWorkspaceConfiguration(environment.Configuration) { - writeStoreError(w, r, execution.ErrExecutionUnavailable) - return options, false - } - + const root = "/workspace" directory := root if requested, exists := q["path"]; exists { - if !validEnvironmentFilePath(requested[0]) { - writeStoreError(w, r, store.ErrInvalidInput) + if err := environmentFileDirectoryError(requested[0]); err != nil { + writeFieldError(w, err) return options, false } - directory = path.Clean(requested[0]) - } - rootPrefix := strings.TrimSuffix(root, "/") + "/" - if directory != root && !strings.HasPrefix(directory, rootPrefix) { - writeStoreError(w, r, store.ErrInvalidInput) - return options, false + directory = requested[0] } options = environmentFileOptions{directory: directory, limit: page.limit, ascending: page.ascending} if directory != root { - options.relativeDirectory = strings.TrimPrefix(directory, rootPrefix) + options.relativeDirectory = strings.TrimPrefix(directory, root+"/") } options.binding = environmentFilesDigest([]any{tenantID(r), environment.ID, directory, options.limit, options.ascending}) if token, exists := q["page"]; exists { cursor, err := decodeEnvironmentFileCursor(token[0], options.binding) if err != nil { - writeStoreError(w, r, err) + writeFieldError(w, err) return environmentFileOptions{}, false } options.cursor = &cursor @@ -79,7 +80,16 @@ func readEnvironmentFileQuery(w http.ResponseWriter, r *http.Request, environmen return options, true } -func validEnvironmentFilePath(value string) bool { - return path.IsAbs(value) && len(value) <= 4096 && utf8.ValidString(value) && - !strings.ContainsAny(value, "\\\x00\r\n") && !slices.Contains(strings.Split(value, "/"), "..") +// environmentFileDirectoryError accepts only the cleaned form of the workspace +// root or a directory below it. Nothing is normalized before execution. +func environmentFileDirectoryError(value string) error { + switch { + case !path.IsAbs(value) || len(value) > 4096 || !utf8.ValidString(value) || strings.ContainsAny(value, "\\\x00\r\n"): + return errEnvironmentFileDirectory + case path.Clean(value) != value: + return errEnvironmentFileCanonical + case value != "/workspace" && !strings.HasPrefix(value, "/workspace/"): + return errEnvironmentFileDirectory + } + return nil } diff --git a/services/agents-api/internal/api/environment_files_test.go b/services/agents-api/internal/api/environment_files_test.go index e80683aad..662ed3001 100644 --- a/services/agents-api/internal/api/environment_files_test.go +++ b/services/agents-api/internal/api/environment_files_test.go @@ -112,7 +112,8 @@ func decodeEnvironmentFiles(t *testing.T, w *httptest.ResponseRecorder) v1.Envir } var fields map[string]any _ = json.Unmarshal(w.Body.Bytes(), &fields) - if len(fields) != 2 || fields["data"] == nil || !strings.HasPrefix(w.Header().Get("Content-Type"), "application/json") || w.Header().Get("Cache-Control") != "no-store" { + if len(fields) != 4 || fields["object"] != "page" || fields["data"] == nil || fields["has_more"] != (page.Next != nil) || page.HasMore != (page.Next != nil) || + !strings.HasPrefix(w.Header().Get("Content-Type"), "application/json") || w.Header().Get("Cache-Control") != "no-store" { t.Fatal("unexpected wire page", w.Header(), fields) } return page @@ -126,7 +127,7 @@ func TestEnvironmentFilesOrderingPaginationAndProjection(t *testing.T) { environmentFileEntry("a.txt", 14), environmentFileEntry("z.txt", 5), environmentFileEntry("A.txt", 0), environmentFileEntry("a-b.txt", 6), {Name: "directory", Kind: "directory"}, {Name: "symlink", Kind: "symlink"}, {Name: "socket", Kind: "other"}, } - q := url.Values{"path": {"/workspace/sub/.//"}, "limit": {"2"}} + q := url.Values{"path": {"/workspace/sub"}, "limit": {"2"}} if order != "" { q.Set("order", order) } @@ -201,7 +202,7 @@ func TestEnvironmentFilesAuthorizationPrecedesInspection(t *testing.T) { func TestEnvironmentFilesRejectsInvalidRequestsBeforeRead(t *testing.T) { for _, query := range []string{ - "limit=0", "limit=101", "limit=no", "limit=", "limit=1&limit=2", "order=ASC", "order=", "after=x", "unknown=x", "path=", "path=relative", "path=/workspace-sibling", "path=/workspace/../workspace", "path=/workspace/a/../../workspace", "path=/workspace/%00", "path=/workspace/%5C", "path=/workspace/%0A", "path=/workspace/%FF", "path=x&path=y", "path=" + strings.Repeat("a", 4097), "page=", "page=not-json", "page=" + strings.Repeat("a", 1025), "bad=%GG", + "limit=0", "limit=101", "limit=no", "limit=", "limit=1&limit=2", "order=ASC", "order=", "path=", "path=relative", "path=/workspace-sibling", "path=/workspace/../workspace", "path=/workspace/a/../../workspace", "path=/workspace/%00", "path=/workspace/%5C", "path=/workspace/%0A", "path=/workspace/%FF", "path=x&path=y", "path=" + strings.Repeat("a", 4097), "page=", "page=not-json", "page=" + strings.Repeat("a", 1025), "bad=%GG", "foo=1;bar=2", } { t.Run(query[:min(len(query), 70)], func(t *testing.T) { h, f := environmentFilesHandler(t, true) diff --git a/services/agents-api/internal/api/environment_files_wire_test.go b/services/agents-api/internal/api/environment_files_wire_test.go new file mode 100644 index 000000000..925627013 --- /dev/null +++ b/services/agents-api/internal/api/environment_files_wire_test.go @@ -0,0 +1,262 @@ +package api + +import ( + "encoding/json" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/proto" + "github.com/google/uuid" +) + +// Official Environment Files wire rows F1–F9 of the environment-files-wire batch. + +func TestEnvironmentFilesPageEnvelope(t *testing.T) { + h, f := environmentFilesHandler(t, true) + w := requestEnvironmentFiles(h, f.environment.ID, "", "files-key") + if w.Code != 200 || strings.TrimSpace(w.Body.String()) != `{"object":"page","data":[],"next":null,"has_more":false}` { + t.Fatalf("empty page: %d %s", w.Code, w.Body) + } + f.result.Entries = []proto.WorkspaceDirectoryEntry{environmentFileEntry("a", 1), environmentFileEntry("b", 2), environmentFileEntry("c", 3)} + q := url.Values{"limit": {"2"}, "order": {"asc"}} + first := decodeEnvironmentFiles(t, requestEnvironmentFiles(h, f.environment.ID, "?"+q.Encode(), "files-key")) + if len(first.Data) != 2 || first.Next == nil || !first.HasMore || first.Object != "page" { + t.Fatal("continued page", first) + } + q.Set("page", *first.Next) + last := decodeEnvironmentFiles(t, requestEnvironmentFiles(h, f.environment.ID, "?"+q.Encode(), "files-key")) + if len(last.Data) != 1 || last.Next != nil || last.HasMore || last.Data[0].Path != "/workspace/c" { + t.Fatal("final page", last) + } +} + +func TestEnvironmentFilesIgnoresUnknownQueryKeys(t *testing.T) { + h, f := environmentFilesHandler(t, true) + f.result.Entries = []proto.WorkspaceDirectoryEntry{environmentFileEntry("a", 1)} + want := requestEnvironmentFiles(h, f.environment.ID, "", "files-key").Body.String() + for _, query := range []string{"?foo=bar", "?after=x&after=y", "?tenant_id=" + uuid.NewString(), "?include=all&foo", "?Path=/workspace/secret"} { + f.directory = "unset" + w := requestEnvironmentFiles(h, f.environment.ID, query, "files-key") + if w.Code != 200 || w.Body.String() != want || f.directory != "" { + t.Fatalf("%s: %d %s directory=%q", query, w.Code, w.Body, f.directory) + } + } + // Unknown keys never bypass authorization: a foreign caller still sees 404. + if w := requestEnvironmentFiles(h, f.environment.ID, "?foo=bar", "other-key"); w.Code != 404 { + t.Fatal("foreign read with unknown key", w.Code) + } +} + +func TestEnvironmentFilesRejectsRepeatedQueryKeys(t *testing.T) { + for _, key := range []string{"path", "limit", "order", "page"} { + t.Run(key, func(t *testing.T) { + h, f := environmentFilesHandler(t, true) + query := "?" + key + "=1&" + key + "=2" + w := requestEnvironmentFiles(h, f.environment.ID, query, "files-key") + assertListQueryError(t, w, "invalid_request_error", nil, "Failed to deserialize query string: duplicate field `"+key+"`") + if f.reads != 0 { + t.Fatal("repeated key reached the reader") + } + foreign := requestEnvironmentFiles(h, f.environment.ID, query, "other-key") + missing := requestEnvironmentFiles(h, uuid.NewString(), query, "files-key") + if foreign.Code != 404 || foreign.Body.String() != missing.Body.String() { + t.Fatal("foreign duplicate differs from missing", foreign.Code, foreign.Body, missing.Body) + } + }) + } +} + +func TestEnvironmentFilesPathErrors(t *testing.T) { + const directory = "path must be an absolute directory inside /workspace" + const canonical = "path must identify a non-reserved directory inside /workspace" + for path, message := range map[string]string{ + "/workspace/outputs/": canonical, + "/workspace/": canonical, + "/workspace//a": canonical, + "/workspace/./a": canonical, + "/workspace/a/..": canonical, + "/workspace/../workspace": canonical, + "/workspace/../etc": canonical, + "outputs": directory, + "": directory, + "workspace/a/": directory, + "/workspace/a\x00": directory, + "/workspace/a\\b": directory, + "/workspace/a\nb": directory, + "/workspace/\xff": directory, + "/" + strings.Repeat("a", 4096): directory, + "/workspace-sibling": directory, + "/etc": directory, + "/": directory, + } { + t.Run(path[:min(len(path), 40)], func(t *testing.T) { + h, f := environmentFilesHandler(t, true) + w := requestEnvironmentFiles(h, f.environment.ID, "?"+url.Values{"path": {path}}.Encode(), "files-key") + assertListQueryError(t, w, "invalid_request_error", nil, message) + if f.reads != 0 { + t.Fatal("invalid path reached the reader") + } + }) + } + for path, relative := range map[string]string{"/workspace": "", "/workspace/a": "a", "/workspace/a/b c": "a/b c"} { + h, f := environmentFilesHandler(t, true) + decodeEnvironmentFiles(t, requestEnvironmentFiles(h, f.environment.ID, "?"+url.Values{"path": {path}}.Encode(), "files-key")) + if f.directory != relative { + t.Fatal("canonical path changed", path, f.directory) + } + } +} + +func TestEnvironmentFilesPageTokenErrors(t *testing.T) { + h, f := environmentFilesHandler(t, true) + f.result.Entries = []proto.WorkspaceDirectoryEntry{environmentFileEntry("a", 1), environmentFileEntry("b", 2)} + page := decodeEnvironmentFiles(t, requestEnvironmentFiles(h, f.environment.ID, "?limit=1", "files-key")) + for name, query := range map[string]url.Values{ + "garbage": {"page": {"garbage"}}, + "empty": {"page": {""}}, + "oversized": {"page": {strings.Repeat("a", 1025)}}, + "binding": {"page": {*page.Next}, "limit": {"2"}}, + "stale": {"page": {*page.Next}, "limit": {"1"}}, + } { + t.Run(name, func(t *testing.T) { + if name == "stale" { + f.result.Entries = []proto.WorkspaceDirectoryEntry{environmentFileEntry("a", 1), environmentFileEntry("b", 3)} + } + w := requestEnvironmentFiles(h, f.environment.ID, "?"+query.Encode(), "files-key") + assertListQueryError(t, w, "invalid_request_error", nil, "Invalid file page token for this request") + }) + } +} + +func TestEnvironmentFileCreateFieldErrors(t *testing.T) { + const absolute = "environment.files[0].path must be an absolute POSIX path inside /workspace" + const components = "environment.files[0].path cannot contain empty, . or .. path components" + type expected struct{ param, message string } + for body, want := range map[string]expected{ + `{"type":"inline","path":"rel.txt","data":"cg=="}`: {"", absolute}, + `{"type":"inline","path":"/workspace","data":"cg=="}`: {"", absolute}, + `{"type":"inline","path":"/workspace/slash/","data":"cg=="}`: {"", components}, + `{"type":"inline","path":"/workspace/../escape.txt","data":"cg=="}`: {"", components}, + `{"type":"inline","path":"/workspace/n1/../dotdot.txt","data":"cg=="}`: {"", components}, + `{"type":"inline","path":"/workspace/./dot.txt","data":"cg=="}`: {"", components}, + `{"type":"inline","path":"/workspace/nul\u0000.txt","data":"cg=="}`: {"", absolute}, + `{"type":"inline","path":"/tmp/outside.txt","data":"cg=="}`: {"", absolute}, + `{"type":"inline","path":"/workspace//dbl.txt","data":"cg=="}`: {"", components}, + `{"type":"inline","path":"/workspacex/a","data":"cg=="}`: {"", absolute}, + `{"type":"file_id","path":"relative","file_id":"file-x"}`: {"", absolute}, + `{"type":"inline","path":"/workspace/extra.txt","data":"eA==","extra_field":1}`: {"extra_field", "Unknown parameter: 'extra_field'."}, + `{"second":1,"type":"inline","path":"/workspace/a","data":"","first":2}`: {"second", "Unknown parameter: 'second'."}, + `{"type":"inline","path":"rel.txt","data":"?","extra_field":null}`: {"extra_field", "Unknown parameter: 'extra_field'."}, + } { + h, f := environmentFileCreateHandler(t) + w := requestCreateEnvironmentFile(h, f.environment.ID, body, "files-key") + var param any + if want.param != "" { + param = want.param + } + assertListQueryError(t, w, "invalid_request_error", param, want.message) + if f.writes != 0 { + t.Fatal("rejected body was written", body) + } + } + // Validation without an official sample keeps the local code, including a + // malformed body whose first key is unknown. + for _, body := range []string{`{"foo":1,`, `{"type":"inline",`, `{"foo":1} {}`, `{"type":"inline","path":"/workspace/a"}`, `{"type":"inline","path":"/workspace/a","data":"?"}`, `{"type":"inline","path":"/workspace/a","data":"","file_id":"x"}`, `[]`} { + h, f := environmentFileCreateHandler(t) + w := requestCreateEnvironmentFile(h, f.environment.ID, body, "files-key") + assertListQueryError(t, w, "invalid_request", nil, "Invalid resource identifier or request limits.") + if f.writes != 0 { + t.Fatal("rejected body was written", body) + } + } + // Only a short, printable name is echoed; every response stays small. + for key, echoed := range map[string]bool{ + strings.Repeat("<>", 64): true, + strings.Repeat("<", 257): false, + strings.Repeat("a", 4<<20): false, + `tab\tkey`: false, + `line\u2028separator`: false, + `bell\u0007`: false, + `caf\u00e9 \u5b57`: true, + "\xff\xfe": false, + `\ud800`: false, + `\ufffd`: false, + } { + h, f := environmentFileCreateHandler(t) + body := `{"type":"inline","path":"/workspace/a","data":"","` + key + `":1}` + w := requestCreateEnvironmentFile(h, f.environment.ID, body, "files-key") + if w.Body.Len() > 4096 || f.writes != 0 { + t.Fatal("unknown field response is unbounded", len(key), w.Body.Len()) + } + if echoed { + var decoded string + _ = json.Unmarshal([]byte(`"`+key+`"`), &decoded) + assertListQueryError(t, w, "invalid_request_error", decoded, "Unknown parameter: '"+decoded+"'.") + } else { + assertListQueryError(t, w, "invalid_request_error", nil, "Unknown parameter.") + } + } + // Foreign Environments stay missing before any body inspection. + h, f := environmentFileCreateHandler(t) + body := `{"type":"inline","path":"/workspace/a","data":"","extra_field":1}` + foreign := requestCreateEnvironmentFile(h, f.environment.ID, body, "other-key") + missing := requestCreateEnvironmentFile(h, uuid.NewString(), body, "files-key") + if foreign.Code != 404 || foreign.Body.String() != missing.Body.String() || f.writes != 0 { + t.Fatal("foreign create inspected", foreign.Code, foreign.Body) + } +} + +func TestEnvironmentFilesHostedProvisioning(t *testing.T) { + const message = "the hosted environment is still provisioning; wait until it is connected before accessing files" + const hosted = `{"type":"openai_hosted","network":{"access":"disabled"}}` + const createBody = `{"type":"inline","data":"YWJj","path":"/workspace/a"}` + sources := &sourceFilesFixture{} + h, f := environmentFileCreateHandler(t, WithSourceFiles(sources)) + f.environment.Configuration, f.environment.Status = json.RawMessage(hosted), "pending" + + w := requestEnvironmentFiles(h, f.environment.ID, "?path=/workspace/a", "files-key") + assertListQueryError(t, w, "invalid_request_error", nil, message) + w = requestCreateEnvironmentFile(h, f.environment.ID, createBody, "files-key") + assertListQueryError(t, w, "invalid_request_error", nil, message) + // The check precedes the source lookup: a missing file_id is not reported. + w = requestCreateEnvironmentFile(h, f.environment.ID, `{"type":"file_id","file_id":"file-missing","path":"/workspace/a"}`, "files-key") + assertListQueryError(t, w, "invalid_request_error", nil, message) + if f.reads != 0 || f.writes != 0 || sources.reads != 0 { + t.Fatal("provisioning Environment reached execution", f.reads, f.writes, sources.reads) + } + + // Request validation still reports its own error first. + assertListQueryError(t, requestEnvironmentFiles(h, f.environment.ID, "?path=/workspace/a/", "files-key"), + "invalid_request_error", nil, "path must identify a non-reserved directory inside /workspace") + assertListQueryError(t, requestCreateEnvironmentFile(h, f.environment.ID, `{"type":"inline","data":"","path":"rel"}`, "files-key"), + "invalid_request_error", nil, "environment.files[0].path must be an absolute POSIX path inside /workspace") + + // Foreign and missing Environments stay identical 404s. + for _, method := range []string{"GET", "POST"} { + var foreign, missing *httptest.ResponseRecorder + if method == "GET" { + foreign, missing = requestEnvironmentFiles(h, f.environment.ID, "", "other-key"), requestEnvironmentFiles(h, uuid.NewString(), "", "files-key") + } else { + foreign, missing = requestCreateEnvironmentFile(h, f.environment.ID, createBody, "other-key"), requestCreateEnvironmentFile(h, uuid.NewString(), createBody, "files-key") + } + if foreign.Code != 404 || foreign.Body.String() != missing.Body.String() { + t.Fatal("foreign provisioning Environment disclosed", method, foreign.Code, foreign.Body) + } + } + + // Other states and placements keep the existing execution path. + for _, state := range []struct{ configuration, status string }{ + {hosted, "connected"}, {hosted, "disconnected"}, {`{"type":"self_hosted","workspace_directory":"/workspace"}`, "pending"}, + } { + f.environment.Configuration, f.environment.Status = json.RawMessage(state.configuration), state.status + reads, writes := f.reads, f.writes + if w := requestEnvironmentFiles(h, f.environment.ID, "", "files-key"); w.Code != 200 || f.reads != reads+1 { + t.Fatal("list rejected", state, w.Code, w.Body) + } + if w := requestCreateEnvironmentFile(h, f.environment.ID, createBody, "files-key"); w.Code != 201 || f.writes != writes+1 { + t.Fatal("create rejected", state, w.Code, w.Body) + } + } +} diff --git a/services/agents-api/internal/api/resource_query_test.go b/services/agents-api/internal/api/resource_query_test.go index ba3274734..75817eac1 100644 --- a/services/agents-api/internal/api/resource_query_test.go +++ b/services/agents-api/internal/api/resource_query_test.go @@ -227,7 +227,7 @@ func TestEnvironmentFileCreateIgnoresUnknownQueryKeys(t *testing.T) { if w := request(f.environment.ID, "files-key", `{"type":"inline","path":"/workspace/a"}`); w.Code != http.StatusBadRequest || f.writes != 0 { t.Fatalf("invalid body admitted: %d", w.Code) } - if w := request(f.environment.ID, "files-key", body); w.Code != http.StatusOK || f.writes != 1 || f.path != "a" || string(f.data) != "abc" { + if w := request(f.environment.ID, "files-key", body); w.Code != http.StatusCreated || f.writes != 1 || f.path != "a" || string(f.data) != "abc" { t.Fatalf("create: %d %s path=%q", w.Code, w.Body, f.path) } } diff --git a/services/agents-api/internal/api/source_files_test.go b/services/agents-api/internal/api/source_files_test.go index d1230b92e..aaa176ba9 100644 --- a/services/agents-api/internal/api/source_files_test.go +++ b/services/agents-api/internal/api/source_files_test.go @@ -165,7 +165,7 @@ func TestSourceFilesPublicLifecycleAndEnvironmentCopy(t *testing.T) { if status, _ := sourceRequest(t, server, "POST", "/v1/agents/environments/"+env.environment.ID+"/files", "files-key", "application/json", []byte(copyBody)); status != 400 { t.Fatal("Agents Beta requirement changed", status) } - if got := requestCreateEnvironmentFile(h, env.environment.ID, copyBody, "files-key"); got.Code != 200 || !bytes.Equal(env.data, data) || env.writes != 1 { + if got := requestCreateEnvironmentFile(h, env.environment.ID, copyBody, "files-key"); got.Code != 201 || !bytes.Equal(env.data, data) || env.writes != 1 { t.Fatalf("copy: %d %s", got.Code, got.Body) } if status, _ := sourceRequest(t, server, "DELETE", "/v1/files/"+file.ID, "other-key", "", nil); status != 404 { diff --git a/services/agents-api/internal/execution/environment_directory.go b/services/agents-api/internal/execution/environment_directory.go index eb00489bc..ac4883d35 100644 --- a/services/agents-api/internal/execution/environment_directory.go +++ b/services/agents-api/internal/execution/environment_directory.go @@ -141,6 +141,12 @@ func readEnvironmentDirectory(ctx context.Context, peer *gateway.Session, reques if err == nil && result.Outcome == "completed" && result.Directory != nil && !result.Directory.Truncated && proto.ValidWorkspaceDirectory(result.Directory, request.MaxEntries) { return directoryReadResult{directory: *result.Directory} } + // The requested path is missing, a regular file or a symbolic link, which the + // native reader never follows: like the official service, list nothing. + // Root, permission, transport and uncertain failures keep their errors. + if err == nil && result.Outcome == "rejected" && result.ErrorCode == proto.WorkspaceReadNotDirectory { + return directoryReadResult{directory: proto.WorkspaceDirectoryResult{Entries: []proto.WorkspaceDirectoryEntry{}}} + } if err == nil && result.Outcome == "rejected" && result.ErrorCode == "not_found" { return directoryReadResult{err: store.ErrNotFound} } diff --git a/services/agents-api/internal/store/environment_directory_test.go b/services/agents-api/internal/store/environment_directory_test.go index 34d8389fd..903084d26 100644 --- a/services/agents-api/internal/store/environment_directory_test.go +++ b/services/agents-api/internal/store/environment_directory_test.go @@ -163,6 +163,60 @@ func TestEnvironmentDirectoryWorkerRejectsIncompleteOrUnreleasedResults(t *testi } } +func rejectDirectoryRead(t *testing.T, h *dispatchHarness, request, read, code string, cleanupFailed bool) { + t.Helper() + h.write(read, proto.TypeWorkspaceReadResult, proto.WorkspaceReadResultPayload{Outcome: "rejected", ErrorCode: code}) + release := h.read(proto.TypeExecutionRelease) + var input proto.ExecutionReleasePayload + if release.ID != request || release.DecodePayload(&input) != nil || input.Handle == "" { + t.Fatal("reader did not release its preparation") + } + status := proto.PreparationStatusPayload{Handle: input.Handle, Revision: 3, State: "released"} + if cleanupFailed { + status.State, status.ErrorCode = "failed", "cleanup_unconfirmed" + } + h.write(request, proto.TypePreparationStatus, status) +} + +// A path that names no directory lists nothing only after confirmed release; +// every other native rejection keeps its existing error. +func TestEnvironmentDirectoryNotDirectoryIsAnEmptyListing(t *testing.T) { + for _, test := range []struct { + code string + cleanupFailed bool + err error + }{ + {proto.WorkspaceReadNotDirectory, false, nil}, + {proto.WorkspaceReadNotDirectory, true, execution.ErrExecutionUnavailable}, + {"not_found", false, store.ErrNotFound}, + {"invalid_request", false, execution.ErrExecutionUnavailable}, + {"permission_denied", false, execution.ErrExecutionUnavailable}, + {"resource_unavailable", false, execution.ErrExecutionUnavailable}, + } { + t.Run(test.code, func(t *testing.T) { + h, w, environment := directoryWorker(t) + foreign := environment + foreign.TenantID = uuid.NewString() + if _, err := w.ReadEnvironmentDirectory(t.Context(), foreign, "reports"); !errors.Is(err, store.ErrNotFound) { + t.Fatal("foreign reader admitted", err) + } + result := startDirectoryRead(t.Context(), w, environment) + request, read := prepareDirectoryRead(t, h, environment) + rejectDirectoryRead(t, h, request, read, test.code, test.cleanupFailed) + got := awaitDirectoryResult(t, result) + if test.err == nil { + if got.err != nil || got.value.Entries == nil || len(got.value.Entries) != 0 || got.value.Truncated { + t.Fatal("not-directory result was not an empty listing", got.err, got.value) + } + return + } + if !errors.Is(got.err, test.err) || len(got.value.Entries) != 0 { + t.Fatal("native rejection changed its error", got.err) + } + }) + } +} + func TestEnvironmentDirectorySequentialReadsReleaseSchedulingOwnership(t *testing.T) { h, w, environment := directoryWorker(t) const pages = 32 diff --git a/services/agents-api/tests/official_environment_files.py b/services/agents-api/tests/official_environment_files.py index dfeaf2f30..ad52a6f88 100644 --- a/services/agents-api/tests/official_environment_files.py +++ b/services/agents-api/tests/official_environment_files.py @@ -1,8 +1,27 @@ """Pinned Files.list checks for a known, unchanged directory of regular files.""" from pathlib import PurePosixPath +from urllib.parse import urlencode -from openai import NotFoundError +from openai import BadRequestError, NotFoundError + + +EMPTY_PAGE = {"object": "page", "data": [], "next": None, "has_more": False} +PROVISIONING = "the hosted environment is still provisioning; wait until it is connected before accessing files" + + +def assert_invalid_request(response, message, param=None): + """Official Environment Files validation fields (HE-16, 18, 35, 38, 39).""" + assert response.status_code == 400, "Expected a 400 validation error" + assert response.json() == {"error": {"type": "invalid_request_error", "code": "invalid_request_error", + "message": message, "param": param}}, "Wrong validation error fields" + + +def verify_wire_page(value): + """The official token page envelope (HE-32): has_more is true exactly when next is set.""" + assert isinstance(value, dict) and set(value) == {"object", "data", "next", "has_more"}, "Wrong page fields" + assert value["object"] == "page" and type(value["has_more"]) is bool, "Wrong page envelope" + assert value["has_more"] is (value["next"] is not None), "has_more disagrees with next" def verify_file_page(value, environment_id, limit): @@ -40,6 +59,7 @@ def verify_environment_files(client, http, environment_id, directory, expected): assert response.status_code == 200, "Raw Files.list failed" assert response.headers.get("content-type", "").startswith("application/json"), "Wrong file page content type" value = response.json() + verify_wire_page(value) more = verify_file_page(value, environment_id, limit) found.extend(value["data"]) page_sizes.append(len(value["data"])) @@ -79,10 +99,16 @@ def verify_environment_files(client, http, environment_id, directory, expected): def verify_file_tenant_isolation(client, other, http, environment_id, directory, page, private_paths): endpoint = str(client.base_url).rstrip("/") + "/agents/environments/" + environment_id + "/files" + foreign = {"Authorization": "Bearer " + other.api_key, "OpenAI-Beta": "agents=v1"} + # Missing paths, repeated keys and invalid bodies never reveal a foreign Environment. + response = http.post(endpoint, headers=foreign, json={"type": "inline", "data": "", "path": directory + "/x", "extra": 1}) + assert response.status_code == 404, "Foreign tenant can reach Files.create validation" + response = http.get(endpoint + "?" + urlencode([("limit", "1"), ("limit", "2")]), headers=foreign) + assert response.status_code == 404, "Foreign tenant can reach Files.list query validation" for params in ({"path": directory, "limit": 1, "order": "asc"}, - {"path": directory, "limit": 1, "order": "asc", "page": page}): - response = http.get(endpoint, params=params, headers={ - "Authorization": "Bearer " + other.api_key, "OpenAI-Beta": "agents=v1"}) + {"path": directory, "limit": 1, "order": "asc", "page": page}, + {"path": directory + "/missing-directory"}): + response = http.get(endpoint, params=params, headers=foreign) assert response.status_code == 404, "Foreign tenant can access Files.list" assert isinstance(response.json().get("error"), dict), "Missing safe error envelope" assert all(secret not in response.text for secret in ( @@ -93,3 +119,99 @@ def verify_file_tenant_isolation(client, other, http, environment_id, directory, pass else: raise AssertionError("Foreign tenant can access SDK Files.list") + + +def verify_file_list_rows(client, http, environment_id, rows, empty_pages=True): + """Replays Files.list rows F3-F8. rows: absolute directory, missing, file and optional symlink paths. + + empty_pages=False selects the documented exception for daemons without a local + workspace binding: the Claude SDK adapter reader keeps 404 for a missing path + and 503 for a regular file or symlink. + """ + endpoint = str(client.base_url).rstrip("/") + "/agents/environments/" + environment_id + "/files" + headers = {"Authorization": "Bearer " + client.api_key, "OpenAI-Beta": "agents=v1"} + resource = client.beta.agents.environments.files + directory = rows["directory"] + baseline = http.get(endpoint, headers=headers, params={"path": directory}) + assert baseline.status_code == 200, "Baseline Files.list failed" + verify_wire_page(baseline.json()) + checked = [] + # F3: unknown keys are ignored. + response = http.get(endpoint, headers=headers, params={"path": directory, "foo": "bar"}) + assert response.status_code == 200 and response.json() == baseline.json(), "Unknown query key changed the page" + sdk = resource.list(environment_id, path=directory, extra_query={"foo": "bar"}) + assert [item.to_dict() for item in sdk.data] == baseline.json()["data"], "SDK unknown key changed the page" + checked.append("unknown_key_ignored") + # F4: a repeated supported key is rejected. + for key, value in (("path", directory), ("limit", "1"), ("order", "asc"), ("page", "token")): + response = http.get(endpoint + "?" + urlencode([(key, value), (key, value)]), headers=headers) + assert_invalid_request(response, "Failed to deserialize query string: duplicate field `" + key + "`") + checked.append("repeated_key_rejected") + # F5/F6: paths that name no listable directory list nothing; links are not followed. + for name in ("missing", "file", "symlink"): + if name not in rows: + continue + response = http.get(endpoint, headers=headers, params={"path": rows[name]}) + if not empty_pages: + expected = 404 if name == "missing" else 503 + assert response.status_code == expected and set(response.json()) == {"error"}, "Adapter reader result changed" + checked.append(name + "_adapter_" + str(expected)) + continue + assert response.status_code == 200 and response.json() == EMPTY_PAGE, "Non-directory path did not list empty" + page = resource.list(environment_id, path=rows[name]) + assert page.data == [] and page.has_more is False and page.next is None and not page.has_next_page(), "SDK empty page" + checked.append(name + "_empty_page") + # F7 and F8: path and token validation errors. + for params, message in ( + ({"path": directory + "/"}, "path must identify a non-reserved directory inside /workspace"), + ({"path": directory + "/./x"}, "path must identify a non-reserved directory inside /workspace"), + ({"path": directory.lstrip("/")}, "path must be an absolute directory inside /workspace"), + ({"path": "/etc"}, "path must be an absolute directory inside /workspace"), + ({"page": "garbage"}, "Invalid file page token for this request"), + ): + assert_invalid_request(http.get(endpoint, headers=headers, params=params), message) + try: + resource.list(environment_id, path=directory + "/") + except BadRequestError as error: + assert error.status_code == 400, "SDK trailing-slash status" + else: + raise AssertionError("SDK accepted a trailing-slash path") + checked.append("path_and_token_errors") + return checked + + +def verify_file_create_rows(client, http, environment_id, directory): + """Replays Files.create rows F1 and F8 below an existing workspace directory.""" + endpoint = str(client.base_url).rstrip("/") + "/agents/environments/" + environment_id + "/files" + headers = {"Authorization": "Bearer " + client.api_key, "OpenAI-Beta": "agents=v1"} + absolute = "environment.files[0].path must be an absolute POSIX path inside /workspace" + components = "environment.files[0].path cannot contain empty, . or .. path components" + for path, message in ( + ("relative.txt", absolute), ("/workspace", absolute), ("/tmp/outside.txt", absolute), + ("/workspace/nul\u0000.txt", absolute), (directory + "/slash/", components), + ("/workspace/../escape.txt", components), (directory + "//double.txt", components), + ): + response = http.post(endpoint, headers=headers, json={"type": "inline", "data": "cg==", "path": path}) + assert_invalid_request(response, message) + response = http.post(endpoint, headers=headers, json={"type": "inline", "data": "eA==", "path": directory + "/extra.txt", "extra_field": 1}) + assert_invalid_request(response, "Unknown parameter: 'extra_field'.", "extra_field") + path = directory + "/created-201.txt" + response = http.post(endpoint, headers=headers, json={"type": "inline", "data": "MjAx", "path": path}) + assert response.status_code == 201, "Files.create did not return 201" + assert response.json() == {"environment_id": environment_id, "object": "agent.environment.file", "path": path, "size_bytes": 3} + return path + + +def verify_files_provisioning(client, http, environment_id): + """Files.list/create on a pending hosted Environment (F9). Returns None when it connected first.""" + endpoint = str(client.base_url).rstrip("/") + "/agents/environments/" + environment_id + "/files" + headers = {"Authorization": "Bearer " + client.api_key, "OpenAI-Beta": "agents=v1"} + before = client.beta.agents.environments.retrieve(environment_id).status + listed = http.get(endpoint, headers=headers) + created = http.post(endpoint, headers=headers, json={"type": "inline", "data": "cA==", "path": "/workspace/pending.txt"}) + after = client.beta.agents.environments.retrieve(environment_id).status + if before != "pending" or after != "pending": + return None + assert_invalid_request(listed, PROVISIONING) + assert_invalid_request(created, PROVISIONING) + return {"list": listed.status_code, "create": created.status_code} diff --git a/services/agents-api/tests/official_environment_files_create.py b/services/agents-api/tests/official_environment_files_create.py index 0ebc82af3..93b20f6b8 100644 --- a/services/agents-api/tests/official_environment_files_create.py +++ b/services/agents-api/tests/official_environment_files_create.py @@ -15,7 +15,8 @@ import httpx2 from openai import OpenAI -from official_environment_files import verify_environment_files, verify_file_tenant_isolation +from official_environment_files import (verify_environment_files, verify_file_create_rows, verify_file_list_rows, + verify_file_tenant_isolation) from official_environment_files_native import caller_token from official_source_files import verify_source_files @@ -49,7 +50,7 @@ def main(): body = {"type": "inline", "data": base64.b64encode(content).decode(), "path": path} if index % 2: response = http.post(endpoint, headers=headers, json=body) - assert response.status_code == 200, "Raw inline upload failed" + assert response.status_code == 201, "Raw inline upload failed" receipt = response.json() else: receipt = client.beta.agents.environments.files.create(environment, **body).to_dict() @@ -62,6 +63,12 @@ def main(): pages, continuation = verify_environment_files(client, http, environment, directory, expected) verify_file_tenant_isolation(client, foreign, http, environment, directory, continuation, list(expected)) target = directory + "/model-input.txt" + # The fixture's staging link must list as empty without exposing staging entries. + wire_rows = verify_file_list_rows(client, http, environment, { + "directory": directory, "missing": directory + "/missing-directory", "file": target, + "symlink": "/workspace/stage-link"}) + created = verify_file_create_rows(client, http, environment, directory) + receipts.append({"path": created, "size": 3, "sha256": hashlib.sha256(b"201").hexdigest()}) for body in ( {"type": "inline", "data": "?", "path": target}, {"type": "inline", "data": "", "path": "/workspace/../escape"}, @@ -77,6 +84,7 @@ def main(): assert response.status_code == 404, "Foreign tenant upload accepted" assert token not in response.text and foreign_token not in response.text, "Credential leaked in error" print(json.dumps({"sdk": pin["sdk_version"], "commit": pin["commit"], "uploads": receipts, "listing": pages, "sources": source_proof, + "wire_rows": wire_rows + ["create_201_and_field_errors"], "limits": ["Private Environment setup", "Other source purposes/expiration/listing unimplemented", "Overwrite metadata/error parity unverified", "Real-model consumption verified by invoking fixture"]})) diff --git a/services/agents-api/tests/official_environment_files_native.py b/services/agents-api/tests/official_environment_files_native.py index 2f3356e3c..dae7a1b19 100644 --- a/services/agents-api/tests/official_environment_files_native.py +++ b/services/agents-api/tests/official_environment_files_native.py @@ -1,4 +1,8 @@ -"""Opt-in live check; stdin supplies engine, base and two tenants with session_id and token_file/token_env.""" +"""Opt-in live check; stdin supplies engine, base and two tenants with session_id and token_file/token_env. + +Optional directory_reader is "local" (default, a local workspace binding) or, for +claude_sdk only, "claude_sdk_adapter" for a daemon without that binding. +""" import importlib.metadata import json @@ -14,13 +18,13 @@ import httpx2 from openai import OpenAI -from official_environment_files import verify_environment_files, verify_file_tenant_isolation +from official_environment_files import verify_environment_files, verify_file_list_rows, verify_file_tenant_isolation UNVERIFIED = [ - "Recursive traversal, directory entries, symlinks and missing paths are unspecified by the pinned SDK.", + "Recursive traversal and directory entries are unspecified by the pinned SDK; symlinked directories are not generated here.", "The scope when path is omitted is not asserted.", - "Default limit, changed-filter cursors and exact invalid-parameter errors are not asserted.", + "Default limit and changed-filter cursors are not asserted.", "The operator must qualify the real provider, native engine and isolated placement separately.", "This fixture uses existing Sessions; it does not qualify Session creation or executor installation.", "Generated fixture directories remain in the caller-owned workspaces for independent inspection.", @@ -79,6 +83,8 @@ def generate_files(client, session_id, label): def main(): settings = json.load(sys.stdin) assert settings["engine"] in ("codex", "claude_sdk"), "Select one qualified native engine" + reader = settings.get("directory_reader", "local") + assert reader == "local" or (reader == "claude_sdk_adapter" and settings["engine"] == "claude_sdk"), "Unsupported directory reader" assert len(settings["tenants"]) == 2, "Two independent tenant Sessions are required" pin = json.loads((Path(__file__).resolve().parents[3] / "contracts/agents-api/upstream.json").read_text()) distribution = importlib.metadata.distribution("openai") @@ -99,11 +105,14 @@ def main(): client, http, fixture["environment_id"], fixture["directory"], fixture["expected"]) fixture["sibling_checks"], _ = verify_environment_files( client, http, fixture["environment_id"], fixture["sibling_directory"], fixture["sibling_expected"]) + fixture["wire_rows"] = verify_file_list_rows(client, http, fixture["environment_id"], { + "directory": fixture["directory"], "missing": fixture["directory"] + "-missing", + "file": next(iter(fixture["expected"]))}, empty_pages=reader == "local") verify_file_tenant_isolation(client, clients[1 - index], http, fixture["environment_id"], fixture["directory"], page, fixture["expected"] | fixture["sibling_expected"]) assert [turn.to_dict() for turn in client.beta.agents.sessions.turns.list(fixture["session_id"])] == before, "Files.list changed Turns" fixture["cross_tenant_denied"] = True - proof = {"engine": settings["engine"], "sdk_version": distribution.version, "sdk_commit": pin["commit"], + proof = {"engine": settings["engine"], "directory_reader": reader, "sdk_version": distribution.version, "sdk_commit": pin["commit"], "scope": "Public input-generated flat files, SDK/raw listing, sorting, pagination and two-tenant isolation", "fixtures": generated, "unverified": UNVERIFIED} serialized = json.dumps(proof, indent=2)