diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index 617fa1f3cb..4ccc4397c4 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/AbsolutePathBuf" @@ -253,6 +262,199 @@ } ] }, + "FileSystemAccessMode": { + "description": "Access mode for a filesystem entry.\n\nWhen two equally specific entries target the same path, we compare these by conflict precedence rather than by capability breadth: `none` beats `write`, and `write` beats `read`.", + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "description": "WARNING: `:special_path` tokens are part of config compatibility. Do not make older runtimes reject newly introduced tokens. New parser support should be additive, while unknown values must stay representable so config from a newer Codex degrades to warn-and-ignore instead of failing to load. Codex 0.112.0 rejected unknown values here, which broke forward compatibility for newer config. Preserves future special-path tokens so older runtimes can ignore them without rejecting config authored by a newer release.", + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "NetworkApprovalContext": { "properties": { "host": { diff --git a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json index ac8d5c4010..d05a6d332f 100644 --- a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/AbsolutePathBuf" @@ -39,6 +48,199 @@ }, "type": "object" }, + "FileSystemAccessMode": { + "description": "Access mode for a filesystem entry.\n\nWhen two equally specific entries target the same path, we compare these by conflict precedence rather than by capability breadth: `none` beats `write`, and `write` beats `read`.", + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "description": "WARNING: `:special_path` tokens are part of config compatibility. Do not make older runtimes reject newly introduced tokens. New parser support should be additive, while unknown values must stay representable so config from a newer Codex degrades to warn-and-ignore instead of failing to load. Codex 0.112.0 rejected unknown values here, which broke forward compatibility for newer config. Preserves future special-path tokens so older runtimes can ignore them without rejecting config authored by a newer release.", + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "RequestPermissionProfile": { "additionalProperties": false, "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json index 7b0c2b1a3b..eb3c65283a 100644 --- a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json +++ b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/AbsolutePathBuf" @@ -39,6 +48,199 @@ }, "type": "object" }, + "FileSystemAccessMode": { + "description": "Access mode for a filesystem entry.\n\nWhen two equally specific entries target the same path, we compare these by conflict precedence rather than by capability breadth: `none` beats `write`, and `write` beats `read`.", + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "description": "WARNING: `:special_path` tokens are part of config compatibility. Do not make older runtimes reject newly introduced tokens. New parser support should be additive, while unknown values must stay representable so config from a newer Codex degrades to warn-and-ignore instead of failing to load. Codex 0.112.0 rejected unknown values here, which broke forward compatibility for newer config. Preserves future special-path tokens so older runtimes can ignore them without rejecting config authored by a newer release.", + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "GrantedPermissionProfile": { "properties": { "fileSystem": { diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index 7c11a4c02b..a168badebb 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/AbsolutePathBuf" @@ -582,6 +591,199 @@ ], "type": "object" }, + "FileSystemAccessMode": { + "description": "Access mode for a filesystem entry.\n\nWhen two equally specific entries target the same path, we compare these by conflict precedence rather than by capability breadth: `none` beats `write`, and `write` beats `read`.", + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "description": "WARNING: `:special_path` tokens are part of config compatibility. Do not make older runtimes reject newly introduced tokens. New parser support should be additive, while unknown values must stay representable so config from a newer Codex degrades to warn-and-ignore instead of failing to load. Codex 0.112.0 rejected unknown values here, which broke forward compatibility for newer config. Preserves future special-path tokens so older runtimes can ignore them without rejecting config authored by a newer release.", + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "McpElicitationArrayType": { "enum": [ "array" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 084da013e4..3d0c60ec4d 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/v2/AbsolutePathBuf" @@ -2078,6 +2087,199 @@ "title": "FileChangeRequestApprovalResponse", "type": "object" }, + "FileSystemAccessMode": { + "description": "Access mode for a filesystem entry.\n\nWhen two equally specific entries target the same path, we compare these by conflict precedence rather than by capability breadth: `none` beats `write`, and `write` beats `read`.", + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/v2/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "description": "WARNING: `:special_path` tokens are part of config compatibility. Do not make older runtimes reject newly introduced tokens. New parser support should be additive, while unknown values must stay representable so config from a newer Codex degrades to warn-and-ignore instead of failing to load. Codex 0.112.0 rejected unknown values here, which broke forward compatibility for newer config. Preserves future special-path tokens so older runtimes can ignore them without rejecting config authored by a newer release.", + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "FuzzyFileSearchMatchType": { "enum": [ "file", diff --git a/codex-rs/app-server-protocol/schema/typescript/FileSystemAccessMode.ts b/codex-rs/app-server-protocol/schema/typescript/FileSystemAccessMode.ts new file mode 100644 index 0000000000..ccaa07a8bf --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/FileSystemAccessMode.ts @@ -0,0 +1,12 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +/** + * Access mode for a filesystem entry. + * + * When two equally specific entries target the same path, we compare these by + * conflict precedence rather than by capability breadth: `none` beats + * `write`, and `write` beats `read`. + */ +export type FileSystemAccessMode = "read" | "write" | "none"; diff --git a/codex-rs/app-server-protocol/schema/typescript/FileSystemPath.ts b/codex-rs/app-server-protocol/schema/typescript/FileSystemPath.ts new file mode 100644 index 0000000000..3b022f1744 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/FileSystemPath.ts @@ -0,0 +1,7 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { AbsolutePathBuf } from "./AbsolutePathBuf"; +import type { FileSystemSpecialPath } from "./FileSystemSpecialPath"; + +export type FileSystemPath = { "type": "path", path: AbsolutePathBuf, } | { "type": "special", value: FileSystemSpecialPath, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/FileSystemSandboxEntry.ts b/codex-rs/app-server-protocol/schema/typescript/FileSystemSandboxEntry.ts new file mode 100644 index 0000000000..f37cd0d63e --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/FileSystemSandboxEntry.ts @@ -0,0 +1,7 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { FileSystemAccessMode } from "./FileSystemAccessMode"; +import type { FileSystemPath } from "./FileSystemPath"; + +export type FileSystemSandboxEntry = { path: FileSystemPath, access: FileSystemAccessMode, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/FileSystemSpecialPath.ts b/codex-rs/app-server-protocol/schema/typescript/FileSystemSpecialPath.ts new file mode 100644 index 0000000000..8ba7b433a6 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/FileSystemSpecialPath.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type FileSystemSpecialPath = { "kind": "root" } | { "kind": "minimal" } | { "kind": "current_working_directory" } | { "kind": "project_roots", subpath?: string, } | { "kind": "tmpdir" } | { "kind": "slash_tmp" } | { "kind": "unknown", path: string, subpath?: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index 09c388337f..643a34b671 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -16,6 +16,10 @@ export type { ExecCommandApprovalParams } from "./ExecCommandApprovalParams"; export type { ExecCommandApprovalResponse } from "./ExecCommandApprovalResponse"; export type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; export type { FileChange } from "./FileChange"; +export type { FileSystemAccessMode } from "./FileSystemAccessMode"; +export type { FileSystemPath } from "./FileSystemPath"; +export type { FileSystemSandboxEntry } from "./FileSystemSandboxEntry"; +export type { FileSystemSpecialPath } from "./FileSystemSpecialPath"; export type { ForcedLoginMethod } from "./ForcedLoginMethod"; export type { FunctionCallOutputBody } from "./FunctionCallOutputBody"; export type { FunctionCallOutputContentItem } from "./FunctionCallOutputContentItem"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts index 0ed8a1b12f..be84d3a093 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts @@ -2,5 +2,6 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AbsolutePathBuf } from "../AbsolutePathBuf"; +import type { FileSystemSandboxEntry } from "../FileSystemSandboxEntry"; -export type AdditionalFileSystemPermissions = { read: Array | null, write: Array | null, }; +export type AdditionalFileSystemPermissions = { read: Array | null, write: Array | null, entries: Array | null, }; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 30061c716e..a54ba03121 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1834,6 +1834,7 @@ mod tests { file_system: Some(v2::AdditionalFileSystemPermissions { read: Some(vec![absolute_path("/tmp/allowed")]), write: None, + entries: None, }), }), proposed_execpolicy_amendment: None, diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 9373c852d8..cf76c47c1e 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -41,6 +41,9 @@ use codex_protocol::openai_models::ModelAvailabilityNux as CoreModelAvailability use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::default_input_modalities; use codex_protocol::parse_command::ParsedCommand as CoreParsedCommand; +use codex_protocol::permissions::FileSystemAccessMode as CoreFileSystemAccessMode; +use codex_protocol::permissions::FileSystemPath as CoreFileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry as CoreFileSystemSandboxEntry; use codex_protocol::plan_tool::PlanItemArg as CorePlanItemArg; use codex_protocol::plan_tool::StepStatus as CorePlanStepStatus; use codex_protocol::protocol::AgentStatus as CoreAgentStatus; @@ -1087,22 +1090,46 @@ impl From for NetworkApprovalContext { pub struct AdditionalFileSystemPermissions { pub read: Option>, pub write: Option>, + pub entries: Option>, } impl From for AdditionalFileSystemPermissions { fn from(value: CoreFileSystemPermissions) -> Self { + let entries = value + .entries + .iter() + .any(|entry| { + entry.access == CoreFileSystemAccessMode::None + || !matches!(entry.path, CoreFileSystemPath::Path { .. }) + }) + .then_some(value.entries.clone()); + let read = value + .explicit_path_entries() + .filter_map(|(path, access)| { + (access == CoreFileSystemAccessMode::Read).then_some(path.clone()) + }) + .collect::>(); + let write = value + .explicit_path_entries() + .filter_map(|(path, access)| { + (access == CoreFileSystemAccessMode::Write).then_some(path.clone()) + }) + .collect::>(); Self { - read: value.read, - write: value.write, + read: (!read.is_empty()).then_some(read), + write: (!write.is_empty()).then_some(write), + entries, } } } impl From for CoreFileSystemPermissions { fn from(value: AdditionalFileSystemPermissions) -> Self { - Self { - read: value.read, - write: value.write, + match value.entries { + Some(entries) if !entries.is_empty() => CoreFileSystemPermissions { entries }, + Some(_) | None => { + CoreFileSystemPermissions::from_read_write_roots(value.read, value.write) + } } } } @@ -6124,6 +6151,7 @@ mod tests { AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) .expect("path must be absolute"), ]), + entries: None, }), } ); @@ -6134,16 +6162,16 @@ mod tests { network: Some(CoreNetworkPermissions { enabled: Some(true), }), - file_system: Some(CoreFileSystemPermissions { - read: Some(vec![ + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + Some(vec![ AbsolutePathBuf::try_from(PathBuf::from(read_only_path)) .expect("path must be absolute"), ]), - write: Some(vec![ + Some(vec![ AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) .expect("path must be absolute"), ]), - }), + )), } ); } @@ -6217,6 +6245,7 @@ mod tests { AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) .expect("path must be absolute"), ]), + entries: None, }), } ); @@ -6227,20 +6256,102 @@ mod tests { network: Some(CoreNetworkPermissions { enabled: Some(true), }), - file_system: Some(CoreFileSystemPermissions { - read: Some(vec![ + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + Some(vec![ AbsolutePathBuf::try_from(PathBuf::from(read_only_path)) .expect("path must be absolute"), ]), - write: Some(vec![ + Some(vec![ AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) .expect("path must be absolute"), ]), - }), + )), } ); } + #[test] + fn additional_file_system_permissions_empty_entries_fall_back_to_read_write_roots() { + let read_only_path = if cfg!(windows) { + r"C:\tmp\read-only" + } else { + "/tmp/read-only" + }; + let read_write_path = if cfg!(windows) { + r"C:\tmp\read-write" + } else { + "/tmp/read-write" + }; + + assert_eq!( + CoreFileSystemPermissions::from(AdditionalFileSystemPermissions { + read: Some(vec![ + AbsolutePathBuf::try_from(PathBuf::from(read_only_path)) + .expect("path must be absolute"), + ]), + write: Some(vec![ + AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) + .expect("path must be absolute"), + ]), + entries: Some(Vec::new()), + }), + CoreFileSystemPermissions::from_read_write_roots( + Some(vec![ + AbsolutePathBuf::try_from(PathBuf::from(read_only_path)) + .expect("path must be absolute"), + ]), + Some(vec![ + AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) + .expect("path must be absolute"), + ]), + ) + ); + } + + #[test] + fn additional_permission_profile_preserves_canonical_file_system_entries() { + let deny_path = if cfg!(windows) { + r"C:\tmp\secret.txt" + } else { + "/tmp/secret.txt" + }; + let file_system = CoreFileSystemPermissions { + entries: vec![ + CoreFileSystemSandboxEntry { + path: CoreFileSystemPath::Special { + value: codex_protocol::permissions::FileSystemSpecialPath::Root, + }, + access: CoreFileSystemAccessMode::Write, + }, + CoreFileSystemSandboxEntry { + path: CoreFileSystemPath::Path { + path: AbsolutePathBuf::try_from(PathBuf::from(deny_path)) + .expect("path must be absolute"), + }, + access: CoreFileSystemAccessMode::None, + }, + ], + }; + let permissions = CorePermissionProfile { + file_system: Some(file_system.clone()), + ..Default::default() + }; + + let additional_permissions = AdditionalPermissionProfile::from(permissions.clone()); + assert_eq!( + additional_permissions.file_system, + Some(AdditionalFileSystemPermissions { + read: None, + write: None, + entries: Some(file_system.entries), + }) + ); + assert_eq!( + CorePermissionProfile::from(additional_permissions), + permissions + ); + } + #[test] fn permissions_request_approval_response_defaults_scope_to_turn() { let response = serde_json::from_value::(json!({ diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 9a850c0b1e..5207db806b 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -3083,10 +3083,10 @@ mod tests { network: Some(CoreNetworkPermissions { enabled: Some(true), }), - file_system: Some(CoreFileSystemPermissions { - read: Some(vec![absolute_path(input_path)]), - write: Some(vec![absolute_path(output_path)]), - }), + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path(input_path)]), + Some(vec![absolute_path(output_path)]), + )), }; let cases = vec![ ( @@ -3113,10 +3113,10 @@ mod tests { }, }), CoreRequestPermissionProfile { - file_system: Some(CoreFileSystemPermissions { - read: None, - write: Some(vec![absolute_path(output_path)]), - }), + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![absolute_path(output_path)]), + )), ..CoreRequestPermissionProfile::default() }, ), @@ -3131,10 +3131,10 @@ mod tests { }, }), CoreRequestPermissionProfile { - file_system: Some(CoreFileSystemPermissions { - read: Some(vec![absolute_path(input_path)]), - write: Some(vec![absolute_path(output_path)]), - }), + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path(input_path)]), + Some(vec![absolute_path(output_path)]), + )), ..CoreRequestPermissionProfile::default() }, ), diff --git a/codex-rs/app-server/src/transport/mod.rs b/codex-rs/app-server/src/transport/mod.rs index fa744a1af5..6b22aca22a 100644 --- a/codex-rs/app-server/src/transport/mod.rs +++ b/codex-rs/app-server/src/transport/mod.rs @@ -799,6 +799,7 @@ mod tests { codex_app_server_protocol::AdditionalFileSystemPermissions { read: Some(vec![absolute_path("/tmp/allowed")]), write: None, + entries: None, }, ), }, @@ -861,6 +862,7 @@ mod tests { codex_app_server_protocol::AdditionalFileSystemPermissions { read: Some(vec![absolute_path("/tmp/allowed")]), write: None, + entries: None, }, ), }, @@ -887,7 +889,8 @@ mod tests { "network": null, "fileSystem": { "read": [allowed_path], - "write": null, + "write": null, + "entries": null, }, }) ); diff --git a/codex-rs/app-server/tests/suite/v2/request_permissions.rs b/codex-rs/app-server/tests/suite/v2/request_permissions.rs index 5a0679415d..05a45011de 100644 --- a/codex-rs/app-server/tests/suite/v2/request_permissions.rs +++ b/codex-rs/app-server/tests/suite/v2/request_permissions.rs @@ -93,6 +93,7 @@ async fn request_permissions_round_trip() -> Result<()> { file_system: Some(codex_app_server_protocol::AdditionalFileSystemPermissions { read: None, write: Some(vec![requested_writes[0].clone()]), + entries: None, }), }, scope: PermissionGrantScope::Turn, diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index 908dec56c6..2c727d2766 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -157,8 +157,6 @@ fn strip_total_output_header(output: &str) -> Option<(&str, u32)> { } pub(crate) mod tools { - pub(crate) use codex_tools::FreeformTool; - pub(crate) use codex_tools::FreeformToolFormat; pub(crate) use codex_tools::ResponsesApiTool; pub(crate) use codex_tools::ToolSearchOutputTool; pub(crate) use codex_tools::ToolSpec; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index f1f008530c..bd8bc65e0f 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1301,6 +1301,12 @@ impl Session { per_turn_config.service_tier = session_configuration.service_tier; per_turn_config.personality = session_configuration.personality; per_turn_config.approvals_reviewer = session_configuration.approvals_reviewer; + per_turn_config.permissions.approval_policy = session_configuration.approval_policy.clone(); + per_turn_config.permissions.sandbox_policy = session_configuration.sandbox_policy.clone(); + per_turn_config.permissions.file_system_sandbox_policy = + session_configuration.file_system_sandbox_policy.clone(); + per_turn_config.permissions.network_sandbox_policy = + session_configuration.network_sandbox_policy; let resolved_web_search_mode = resolve_web_search_mode_for_turn( &per_turn_config.web_search_mode, session_configuration.sandbox_policy.get(), @@ -3323,6 +3329,11 @@ impl Session { ts.granted_permissions() } + pub(crate) async fn cwd(&self) -> AbsolutePathBuf { + let state = self.state.lock().await; + state.session_configuration.cwd.clone() + } + pub(crate) async fn granted_session_permissions(&self) -> Option { let state = self.state.lock().await; state.granted_permissions() diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index da0936f27e..4ea9c049f1 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -19,6 +19,7 @@ use assert_matches::assert_matches; use codex_config::CONFIG_TOML_FILE; use codex_features::Feature; use codex_features::FeaturesToml; +use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; @@ -545,6 +546,13 @@ fn default_permissions_profile_populates_runtime_sandbox_policy() -> std::io::Re }, ]), ); + assert_eq!( + config.permissions.runtime_permission_profile(), + PermissionProfile::from_runtime_permissions( + &config.permissions.file_system_sandbox_policy, + config.permissions.network_sandbox_policy, + ) + ); assert_eq!( config.permissions.sandbox_policy.get(), &SandboxPolicy::WorkspaceWrite { @@ -1164,6 +1172,14 @@ exclude_slash_tmp = true NetworkSandboxPolicy::from(sandbox_policy), "case `{name}` should preserve network semantics from legacy config" ); + assert_eq!( + config.permissions.runtime_permission_profile(), + PermissionProfile::from_runtime_permissions( + &config.permissions.file_system_sandbox_policy, + config.permissions.network_sandbox_policy, + ), + "case `{name}` should populate canonical permission profile from runtime policies" + ); assert_eq!( config .permissions diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 58cffc2659..d979a5c2f8 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -78,6 +78,8 @@ use codex_protocol::config_types::WebSearchConfig; use codex_protocol::config_types::WebSearchMode; use codex_protocol::config_types::WebSearchToolConfig; use codex_protocol::config_types::WindowsSandboxLevel; +#[cfg(test)] +use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ModelsResponse; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::permissions::FileSystemSandboxPolicy; @@ -212,6 +214,16 @@ pub struct Permissions { pub windows_sandbox_private_desktop: bool, } +impl Permissions { + #[cfg(test)] + pub(crate) fn runtime_permission_profile(&self) -> PermissionProfile { + PermissionProfile::from_runtime_permissions( + &self.file_system_sandbox_policy, + self.network_sandbox_policy, + ) + } +} + /// Application configuration loaded from disk and merged with overrides. #[derive(Debug, Clone, PartialEq)] pub struct Config { diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 8bdaea059d..9568327821 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -1,13 +1,8 @@ -use std::collections::BTreeMap; use std::path::Path; use crate::apply_patch; use crate::apply_patch::InternalApplyPatchInvocation; use crate::apply_patch::convert_apply_patch_to_protocol; -use crate::client_common::tools::FreeformTool; -use crate::client_common::tools::FreeformToolFormat; -use crate::client_common::tools::ResponsesApiTool; -use crate::client_common::tools::ToolSpec; use crate::codex::Session; use crate::codex::TurnContext; use crate::function_tool::FunctionCallError; @@ -26,8 +21,6 @@ use crate::tools::registry::ToolKind; use crate::tools::runtimes::apply_patch::ApplyPatchRequest; use crate::tools::runtimes::apply_patch::ApplyPatchRuntime; use crate::tools::sandboxing::ToolCtx; -use crate::tools::spec::ApplyPatchToolArgs; -use crate::tools::spec::JsonSchema; use async_trait::async_trait; use codex_apply_patch::ApplyPatchAction; use codex_apply_patch::ApplyPatchFileChange; @@ -36,14 +29,13 @@ use codex_protocol::models::PermissionProfile; use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy; use codex_sandboxing::policy_transforms::merge_permission_profiles; use codex_sandboxing::policy_transforms::normalize_additional_permissions; +use codex_tools::ApplyPatchToolArgs; use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::BTreeSet; use std::sync::Arc; pub struct ApplyPatchHandler; -const APPLY_PATCH_LARK_GRAMMAR: &str = include_str!("tool_apply_patch.lark"); - fn file_paths_for_action(action: &ApplyPatchAction) -> Vec { let mut keys = Vec::new(); let cwd = action.cwd.as_path(); @@ -88,10 +80,10 @@ fn write_permissions_for_paths( .ok()?; let permissions = (!write_paths.is_empty()).then_some(PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(write_paths), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(write_paths), + )), ..Default::default() })?; @@ -356,112 +348,6 @@ pub(crate) async fn intercept_apply_patch( } } -/// Returns a custom tool that can be used to edit files. Well-suited for GPT-5 models -/// https://platform.openai.com/docs/guides/function-calling#custom-tools -pub(crate) fn create_apply_patch_freeform_tool() -> ToolSpec { - ToolSpec::Freeform(FreeformTool { - name: "apply_patch".to_string(), - description: "Use the `apply_patch` tool to edit files. This is a FREEFORM tool, so do not wrap the patch in JSON.".to_string(), - format: FreeformToolFormat { - r#type: "grammar".to_string(), - syntax: "lark".to_string(), - definition: APPLY_PATCH_LARK_GRAMMAR.to_string(), - }, - }) -} - -/// Returns a json tool that can be used to edit files. Should only be used with gpt-oss models -pub(crate) fn create_apply_patch_json_tool() -> ToolSpec { - let mut properties = BTreeMap::new(); - properties.insert( - "input".to_string(), - JsonSchema::String { - description: Some(r#"The entire contents of the apply_patch command"#.to_string()), - }, - ); - - ToolSpec::Function(ResponsesApiTool { - name: "apply_patch".to_string(), - description: r#"Use the `apply_patch` tool to edit files. -Your patch language is a stripped‑down, file‑oriented diff format designed to be easy to parse and safe to apply. You can think of it as a high‑level envelope: - -*** Begin Patch -[ one or more file sections ] -*** End Patch - -Within that envelope, you get a sequence of file operations. -You MUST include a header to specify the action you are taking. -Each operation starts with one of three headers: - -*** Add File: - create a new file. Every following line is a + line (the initial contents). -*** Delete File: - remove an existing file. Nothing follows. -*** Update File: - patch an existing file in place (optionally with a rename). - -May be immediately followed by *** Move to: if you want to rename the file. -Then one or more “hunks”, each introduced by @@ (optionally followed by a hunk header). -Within a hunk each line starts with: - -For instructions on [context_before] and [context_after]: -- By default, show 3 lines of code immediately above and 3 lines immediately below each change. If a change is within 3 lines of a previous change, do NOT duplicate the first change’s [context_after] lines in the second change’s [context_before] lines. -- If 3 lines of context is insufficient to uniquely identify the snippet of code within the file, use the @@ operator to indicate the class or function to which the snippet belongs. For instance, we might have: -@@ class BaseClass -[3 lines of pre-context] -- [old_code] -+ [new_code] -[3 lines of post-context] - -- If a code block is repeated so many times in a class or function such that even a single `@@` statement and 3 lines of context cannot uniquely identify the snippet of code, you can use multiple `@@` statements to jump to the right context. For instance: - -@@ class BaseClass -@@ def method(): -[3 lines of pre-context] -- [old_code] -+ [new_code] -[3 lines of post-context] - -The full grammar definition is below: -Patch := Begin { FileOp } End -Begin := "*** Begin Patch" NEWLINE -End := "*** End Patch" NEWLINE -FileOp := AddFile | DeleteFile | UpdateFile -AddFile := "*** Add File: " path NEWLINE { "+" line NEWLINE } -DeleteFile := "*** Delete File: " path NEWLINE -UpdateFile := "*** Update File: " path NEWLINE [ MoveTo ] { Hunk } -MoveTo := "*** Move to: " newPath NEWLINE -Hunk := "@@" [ header ] NEWLINE { HunkLine } [ "*** End of File" NEWLINE ] -HunkLine := (" " | "-" | "+") text NEWLINE - -A full patch can combine several operations: - -*** Begin Patch -*** Add File: hello.txt -+Hello world -*** Update File: src/app.py -*** Move to: src/main.py -@@ def greet(): --print("Hi") -+print("Hello, world!") -*** Delete File: obsolete.txt -*** End Patch - -It is important to remember: - -- You must include a header with your intended action (Add/Delete/Update) -- You must prefix new lines with `+` even when creating a new file -- File references can only be relative, NEVER ABSOLUTE. -"# - .to_string(), - strict: false, - defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: Some(vec!["input".to_string()]), - additional_properties: Some(false.into()), - }, - output_schema: None, - }) -} - #[cfg(test)] #[path = "apply_patch_tests.rs"] mod tests; diff --git a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs index 86ae586929..b6a3252523 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs @@ -74,7 +74,13 @@ fn write_permissions_for_paths_keep_dirs_outside_workspace_root() { .expect("outside dir should be absolute"); assert_eq!( - permissions.and_then(|profile| profile.file_system.and_then(|fs| fs.write)), - Some(vec![expected_outside]) + permissions, + Some(PermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![expected_outside]), + )), + ..Default::default() + }) ); } diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 86f2175be6..d7d8184538 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -18,9 +18,9 @@ mod tool_suggest; pub(crate) mod unified_exec; mod view_image; -use codex_sandboxing::policy_transforms::intersect_permission_profiles; use codex_sandboxing::policy_transforms::merge_permission_profiles; use codex_sandboxing::policy_transforms::normalize_additional_permissions; +use codex_sandboxing::policy_transforms::permission_profile_is_preapproved; use codex_utils_absolute_path::AbsolutePathBufGuard; pub use plan::PLAN_TOOL; use serde::Deserialize; @@ -44,9 +44,7 @@ pub use mcp::McpHandler; pub use mcp_resource::McpResourceHandler; pub use plan::PlanHandler; pub use request_permissions::RequestPermissionsHandler; -pub(crate) use request_permissions::request_permissions_tool_description; pub use request_user_input::RequestUserInputHandler; -pub(crate) use request_user_input::request_user_input_tool_description; pub use shell::ShellCommandHandler; pub use shell::ShellHandler; pub use test_sync::TestSyncHandler; @@ -197,13 +195,18 @@ pub(super) async fn apply_granted_turn_permissions( additional_permissions.as_ref(), granted_permissions.as_ref(), ); - let permissions_preapproved = match (effective_permissions.as_ref(), granted_permissions) { - (Some(effective_permissions), Some(granted_permissions)) => { - intersect_permission_profiles(effective_permissions.clone(), granted_permissions) - == *effective_permissions - } - _ => false, - }; + let session_cwd = session.cwd().await; + let permissions_preapproved = + match (effective_permissions.as_ref(), granted_permissions.as_ref()) { + (Some(effective_permissions), Some(granted_permissions)) => { + permission_profile_is_preapproved( + effective_permissions, + granted_permissions, + session_cwd.as_path(), + ) + } + _ => false, + }; let sandbox_permissions = if effective_permissions.is_some() && !sandbox_permissions.uses_additional_permissions() { @@ -245,12 +248,12 @@ mod tests { fn file_system_permissions(path: &std::path::Path) -> PermissionProfile { PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![ AbsolutePathBuf::from_absolute_path(path).expect("absolute path"), ]), - }), + )), ..Default::default() } } diff --git a/codex-rs/core/src/tools/handlers/request_permissions.rs b/codex-rs/core/src/tools/handlers/request_permissions.rs index 4df8328e70..e0755deac7 100644 --- a/codex-rs/core/src/tools/handlers/request_permissions.rs +++ b/codex-rs/core/src/tools/handlers/request_permissions.rs @@ -10,11 +10,6 @@ use crate::tools::handlers::parse_arguments_with_base_path; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; -pub(crate) fn request_permissions_tool_description() -> String { - "Request additional filesystem or network permissions from the user and wait for the client to grant a subset of the requested permission profile. Granted permissions apply automatically to later shell-like commands in the current turn, or for the rest of the session if the client approves them at session scope." - .to_string() -} - pub struct RequestPermissionsHandler; #[async_trait] diff --git a/codex-rs/core/src/tools/handlers/request_user_input.rs b/codex-rs/core/src/tools/handlers/request_user_input.rs index 4d95a2c20d..a5d43d5748 100644 --- a/codex-rs/core/src/tools/handlers/request_user_input.rs +++ b/codex-rs/core/src/tools/handlers/request_user_input.rs @@ -6,50 +6,8 @@ use crate::tools::handlers::parse_arguments; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use async_trait::async_trait; -use codex_protocol::config_types::ModeKind; -use codex_protocol::config_types::TUI_VISIBLE_COLLABORATION_MODES; use codex_protocol::request_user_input::RequestUserInputArgs; - -fn request_user_input_is_available(mode: ModeKind, default_mode_request_user_input: bool) -> bool { - mode.allows_request_user_input() - || (default_mode_request_user_input && mode == ModeKind::Default) -} - -fn format_allowed_modes(default_mode_request_user_input: bool) -> String { - let mode_names: Vec<&str> = TUI_VISIBLE_COLLABORATION_MODES - .into_iter() - .filter(|mode| request_user_input_is_available(*mode, default_mode_request_user_input)) - .map(ModeKind::display_name) - .collect(); - - match mode_names.as_slice() { - [] => "no modes".to_string(), - [mode] => format!("{mode} mode"), - [first, second] => format!("{first} or {second} mode"), - [..] => format!("modes: {}", mode_names.join(",")), - } -} - -pub(crate) fn request_user_input_unavailable_message( - mode: ModeKind, - default_mode_request_user_input: bool, -) -> Option { - if request_user_input_is_available(mode, default_mode_request_user_input) { - None - } else { - let mode_name = mode.display_name(); - Some(format!( - "request_user_input is unavailable in {mode_name} mode" - )) - } -} - -pub(crate) fn request_user_input_tool_description(default_mode_request_user_input: bool) -> String { - let allowed_modes = format_allowed_modes(default_mode_request_user_input); - format!( - "Request user input for one to three short questions and wait for the response. This tool is only available in {allowed_modes}." - ) -} +use codex_tools::request_user_input_unavailable_message; pub struct RequestUserInputHandler { pub default_mode_request_user_input: bool, @@ -119,7 +77,3 @@ impl ToolHandler for RequestUserInputHandler { Ok(FunctionToolOutput::from_text(content, Some(true))) } } - -#[cfg(test)] -#[path = "request_user_input_tests.rs"] -mod tests; diff --git a/codex-rs/core/src/tools/handlers/request_user_input_tests.rs b/codex-rs/core/src/tools/handlers/request_user_input_tests.rs deleted file mode 100644 index bf44233e72..0000000000 --- a/codex-rs/core/src/tools/handlers/request_user_input_tests.rs +++ /dev/null @@ -1,61 +0,0 @@ -use super::*; -use pretty_assertions::assert_eq; - -#[test] -fn request_user_input_mode_availability_defaults_to_plan_only() { - assert!(ModeKind::Plan.allows_request_user_input()); - assert!(!ModeKind::Default.allows_request_user_input()); - assert!(!ModeKind::Execute.allows_request_user_input()); - assert!(!ModeKind::PairProgramming.allows_request_user_input()); -} - -#[test] -fn request_user_input_unavailable_messages_respect_default_mode_feature_flag() { - assert_eq!( - request_user_input_unavailable_message( - ModeKind::Plan, - /*default_mode_request_user_input*/ false - ), - None - ); - assert_eq!( - request_user_input_unavailable_message( - ModeKind::Default, - /*default_mode_request_user_input*/ false - ), - Some("request_user_input is unavailable in Default mode".to_string()) - ); - assert_eq!( - request_user_input_unavailable_message( - ModeKind::Default, - /*default_mode_request_user_input*/ true - ), - None - ); - assert_eq!( - request_user_input_unavailable_message( - ModeKind::Execute, - /*default_mode_request_user_input*/ false - ), - Some("request_user_input is unavailable in Execute mode".to_string()) - ); - assert_eq!( - request_user_input_unavailable_message( - ModeKind::PairProgramming, - /*default_mode_request_user_input*/ false - ), - Some("request_user_input is unavailable in Pair Programming mode".to_string()) - ); -} - -#[test] -fn request_user_input_tool_description_mentions_available_modes() { - assert_eq!( - request_user_input_tool_description(/*default_mode_request_user_input*/ false), - "Request user input for one to three short questions and wait for the response. This tool is only available in Plan mode.".to_string() - ); - assert_eq!( - request_user_input_tool_description(/*default_mode_request_user_input*/ true), - "Request user input for one to three short questions and wait for the response. This tool is only available in Default or Plan mode.".to_string() - ); -} diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index 11b75b28d4..09f71d804c 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -186,10 +186,10 @@ fn exec_command_args_resolve_relative_additional_permissions_against_workdir() - assert_eq!( args.additional_permissions, Some(PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![AbsolutePathBuf::try_from(expected_write)?]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![AbsolutePathBuf::try_from(expected_write)?]), + )), ..Default::default() }) ); diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index 226f7f0540..856c2d1a62 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -250,12 +250,12 @@ fn map_exec_result_preserves_stdout_and_stderr() { #[test] fn shell_request_escalation_execution_is_explicit() { let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![ AbsolutePathBuf::from_absolute_path("/tmp/output").unwrap(), ]), - }), + )), ..Default::default() }; let sandbox_policy = SandboxPolicy::WorkspaceWrite { diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 3e9f2f6fa3..74f28f46a7 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -10,13 +10,9 @@ use crate::tools::handlers::TOOL_SEARCH_DEFAULT_LIMIT; use crate::tools::handlers::TOOL_SEARCH_TOOL_NAME; use crate::tools::handlers::TOOL_SUGGEST_TOOL_NAME; use crate::tools::handlers::agent_jobs::BatchJobHandler; -use crate::tools::handlers::apply_patch::create_apply_patch_freeform_tool; -use crate::tools::handlers::apply_patch::create_apply_patch_json_tool; use crate::tools::handlers::multi_agents_common::DEFAULT_WAIT_TIMEOUT_MS; use crate::tools::handlers::multi_agents_common::MAX_WAIT_TIMEOUT_MS; use crate::tools::handlers::multi_agents_common::MIN_WAIT_TIMEOUT_MS; -use crate::tools::handlers::request_permissions_tool_description; -use crate::tools::handlers::request_user_input_tool_description; use crate::tools::registry::ToolRegistryBuilder; use crate::tools::registry::tool_handler_key; use codex_protocol::config_types::WebSearchMode; @@ -35,6 +31,8 @@ use codex_tools::ToolUserShellType; use codex_tools::ViewImageToolOptions; use codex_tools::WaitAgentTimeoutOptions; use codex_tools::augment_tool_spec_for_code_mode; +use codex_tools::create_apply_patch_freeform_tool; +use codex_tools::create_apply_patch_json_tool; use codex_tools::create_assign_task_tool; use codex_tools::create_close_agent_tool_v1; use codex_tools::create_close_agent_tool_v2; @@ -68,9 +66,9 @@ use codex_tools::create_wait_tool; use codex_tools::create_write_stdin_tool; use codex_tools::dynamic_tool_to_responses_api_tool; use codex_tools::mcp_tool_to_responses_api_tool; +use codex_tools::request_permissions_tool_description; +use codex_tools::request_user_input_tool_description; use codex_tools::tool_spec_to_code_mode_tool_definition; -use serde::Deserialize; -use serde::Serialize; use std::collections::HashMap; pub type JsonSchema = codex_tools::JsonSchema; @@ -103,12 +101,6 @@ fn agent_type_description(config: &ToolsConfig) -> String { } } -/// TODO(dylan): deprecate once we get rid of json tool -#[derive(Serialize, Deserialize)] -pub(crate) struct ApplyPatchToolArgs { - pub(crate) input: String, -} - fn push_tool_spec( builder: &mut ToolRegistryBuilder, spec: ToolSpec, diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 157d962b4b..989b4fd165 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -30,6 +30,7 @@ use codex_tools::ResponsesApiWebSearchUserLocation; use codex_tools::SpawnAgentToolOptions; use codex_tools::ViewImageToolOptions; use codex_tools::WaitAgentTimeoutOptions; +use codex_tools::create_apply_patch_freeform_tool; use codex_tools::create_close_agent_tool_v1; use codex_tools::create_close_agent_tool_v2; use codex_tools::create_exec_command_tool; @@ -45,6 +46,8 @@ use codex_tools::create_wait_agent_tool_v1; use codex_tools::create_wait_agent_tool_v2; use codex_tools::create_write_stdin_tool; use codex_tools::mcp_tool_to_deferred_responses_api_tool; +use codex_tools::request_permissions_tool_description; +use codex_tools::request_user_input_tool_description; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use serde_json::json; diff --git a/codex-rs/core/tests/suite/request_permissions.rs b/codex-rs/core/tests/suite/request_permissions.rs index 2cfd1cf6f7..1ab44593d8 100644 --- a/codex-rs/core/tests/suite/request_permissions.rs +++ b/codex-rs/core/tests/suite/request_permissions.rs @@ -7,6 +7,7 @@ use codex_features::Feature; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecApprovalRequestEvent; @@ -293,20 +294,20 @@ fn workspace_write_excluding_tmp() -> SandboxPolicy { fn requested_directory_write_permissions(path: &Path) -> RequestPermissionProfile { RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(path)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(path)]), + )), ..RequestPermissionProfile::default() } } fn normalized_directory_write_permissions(path: &Path) -> Result { Ok(RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), + )), ..RequestPermissionProfile::default() }) } @@ -343,10 +344,10 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res let call_id = "request_permissions_skip_approval"; let command = "touch requested-dir/requested-but-unused.txt"; let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(&requested_dir_canonical)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(&requested_dir_canonical)]), + )), ..Default::default() }; let event = shell_event_with_request_permissions(call_id, command, &requested_permissions)?; @@ -521,10 +522,10 @@ async fn relative_additional_permissions_resolve_against_tool_workdir() -> Resul let call_id = "request_permissions_relative_workdir"; let command = "touch relative-write.txt"; let expected_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![absolute_path(&nested_dir_canonical)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![absolute_path(&nested_dir_canonical)]), + )), ..Default::default() }; let event = shell_event_with_raw_request_permissions( @@ -624,10 +625,10 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_cwd "cwd-widened", unrequested_write, unrequested_write ); let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(&requested_write)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(&requested_write)]), + )), ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -725,10 +726,10 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_tmp "tmp-widened", tmp_write, tmp_write ); let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(&requested_write)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(&requested_write)]), + )), ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -824,19 +825,19 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() -> "outside-cwd-ok", outside_write, outside_write ); let requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(outside_dir.path())]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(outside_dir.path())]), + )), ..RequestPermissionProfile::default() }; let normalized_requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from( + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from( outside_dir.path().canonicalize()?, )?]), - }), + )), ..RequestPermissionProfile::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -926,19 +927,19 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul "should-not-write", outside_write, outside_write ); let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(outside_dir.path())]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(outside_dir.path())]), + )), ..Default::default() }; let normalized_requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from( + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from( outside_dir.path().canonicalize()?, )?]), - }), + )), ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -1028,19 +1029,19 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul "sticky-grant-ok", outside_write, outside_write ); let requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(outside_dir.path())]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(outside_dir.path())]), + )), ..Default::default() }; let normalized_requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from( + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from( outside_dir.path().canonicalize()?, )?]), - }), + )), ..Default::default() }; let responses = mount_sse_sequence( @@ -1492,35 +1493,35 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() ); let requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![ absolute_path(first_dir.path()), absolute_path(second_dir.path()), ]), - }), + )), ..RequestPermissionProfile::default() }; let normalized_requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![ AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?, AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?, ]), - }), + )), ..RequestPermissionProfile::default() }; let granted_permissions = normalized_directory_write_permissions(first_dir.path())?; let second_dir_permissions = requested_directory_write_permissions(second_dir.path()); let merged_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![ AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?, AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?, ]), - }), + )), ..Default::default() }; @@ -1584,16 +1585,31 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() let approval_file_system = approval_permissions .file_system .unwrap_or_else(|| panic!("expected filesystem permissions")); - assert!(approval_file_system.read.as_ref().is_none_or(Vec::is_empty)); - let mut approval_writes = approval_file_system.write.unwrap_or_default(); + assert_eq!( + approval_file_system + .explicit_path_entries() + .filter(|(_, access)| *access == FileSystemAccessMode::Read) + .count(), + 0 + ); + + let mut approval_writes = approval_file_system + .explicit_path_entries() + .filter_map(|(path, access)| { + (access == FileSystemAccessMode::Write).then_some(path.clone()) + }) + .collect::>(); approval_writes.sort_by_key(|path| path.display().to_string()); let mut expected_writes = merged_permissions .file_system .unwrap_or_else(|| panic!("expected merged filesystem permissions")) - .write - .unwrap_or_default(); + .explicit_path_entries() + .filter_map(|(path, access)| { + (access == FileSystemAccessMode::Write).then_some(path.clone()) + }) + .collect::>(); expected_writes.sort_by_key(|path| path.display().to_string()); assert_eq!(approval_writes, expected_writes); diff --git a/codex-rs/core/tests/suite/request_permissions_tool.rs b/codex-rs/core/tests/suite/request_permissions_tool.rs index 14506f4a41..0578441e99 100644 --- a/codex-rs/core/tests/suite/request_permissions_tool.rs +++ b/codex-rs/core/tests/suite/request_permissions_tool.rs @@ -81,20 +81,20 @@ fn workspace_write_excluding_tmp() -> SandboxPolicy { fn requested_directory_write_permissions(path: &Path) -> RequestPermissionProfile { RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(path)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(path)]), + )), ..RequestPermissionProfile::default() } } fn normalized_directory_write_permissions(path: &Path) -> Result { Ok(RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), + )), ..RequestPermissionProfile::default() }) } diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index d4e568fea9..5317ef14bc 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::io; use std::path::Path; use std::sync::LazyLock; @@ -14,6 +15,12 @@ use ts_rs::TS; use crate::config_types::ApprovalsReviewer; use crate::config_types::CollaborationMode; use crate::config_types::SandboxMode; +use crate::permissions::FileSystemAccessMode; +use crate::permissions::FileSystemPath; +use crate::permissions::FileSystemSandboxEntry; +use crate::permissions::FileSystemSandboxPolicy; +use crate::permissions::FileSystemSpecialPath; +use crate::permissions::NetworkSandboxPolicy; use crate::protocol::AskForApproval; use crate::protocol::COLLABORATION_MODE_CLOSE_TAG; use crate::protocol::COLLABORATION_MODE_OPEN_TAG; @@ -80,15 +87,119 @@ impl SandboxPermissions { } } -#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, JsonSchema, TS)] pub struct FileSystemPermissions { - pub read: Option>, - pub write: Option>, + pub entries: Vec, } impl FileSystemPermissions { pub fn is_empty(&self) -> bool { - self.read.is_none() && self.write.is_none() + self.entries.is_empty() + } + + pub fn from_read_write_roots( + read: Option>, + write: Option>, + ) -> Self { + let mut entries = Vec::new(); + if let Some(read) = read { + entries.extend(read.into_iter().map(|path| FileSystemSandboxEntry { + path: FileSystemPath::Path { path }, + access: FileSystemAccessMode::Read, + })); + } + if let Some(write) = write { + entries.extend(write.into_iter().map(|path| FileSystemSandboxEntry { + path: FileSystemPath::Path { path }, + access: FileSystemAccessMode::Write, + })); + } + Self { entries } + } + + pub fn explicit_path_entries( + &self, + ) -> impl Iterator { + self.entries.iter().filter_map(|entry| match &entry.path { + FileSystemPath::Path { path } => Some((path, entry.access)), + FileSystemPath::Special { .. } => None, + }) + } + + fn as_legacy_permissions(&self) -> Option { + let mut read = Vec::new(); + let mut write = Vec::new(); + + for entry in &self.entries { + let FileSystemPath::Path { path } = &entry.path else { + return None; + }; + match entry.access { + FileSystemAccessMode::Read => read.push(path.clone()), + FileSystemAccessMode::Write => write.push(path.clone()), + FileSystemAccessMode::None => return None, + } + } + + Some(LegacyFileSystemPermissions { + read: (!read.is_empty()).then_some(read), + write: (!write.is_empty()).then_some(write), + }) + } +} + +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +struct LegacyFileSystemPermissions { + #[serde(default, skip_serializing_if = "Option::is_none")] + read: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + write: Option>, +} + +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +struct CanonicalFileSystemPermissions { + #[serde(default, skip_serializing_if = "Vec::is_empty")] + entries: Vec, +} + +#[derive(Debug, Clone, Deserialize)] +#[serde(untagged)] +enum FileSystemPermissionsDe { + Canonical(CanonicalFileSystemPermissions), + Legacy(LegacyFileSystemPermissions), +} + +impl Serialize for FileSystemPermissions { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + if let Some(legacy) = self.as_legacy_permissions() { + legacy.serialize(serializer) + } else { + CanonicalFileSystemPermissions { + entries: self.entries.clone(), + } + .serialize(serializer) + } + } +} + +impl<'de> Deserialize<'de> for FileSystemPermissions { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + match FileSystemPermissionsDe::deserialize(deserializer)? { + FileSystemPermissionsDe::Canonical(CanonicalFileSystemPermissions { entries }) => { + Ok(Self { entries }) + } + FileSystemPermissionsDe::Legacy(LegacyFileSystemPermissions { read, write }) => { + Ok(Self::from_read_write_roots(read, write)) + } + } } } @@ -113,6 +224,80 @@ impl PermissionProfile { pub fn is_empty(&self) -> bool { self.network.is_none() && self.file_system.is_none() } + + pub fn from_runtime_permissions( + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + ) -> Self { + Self { + network: Some(network_sandbox_policy.into()), + file_system: Some(file_system_sandbox_policy.into()), + } + } + + pub fn from_legacy_sandbox_policy(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Self { + Self::from_runtime_permissions( + &FileSystemSandboxPolicy::from_legacy_sandbox_policy(sandbox_policy, cwd), + NetworkSandboxPolicy::from(sandbox_policy), + ) + } + + pub fn file_system_sandbox_policy(&self) -> FileSystemSandboxPolicy { + self.file_system.as_ref().map_or_else( + || FileSystemSandboxPolicy::restricted(Vec::new()), + FileSystemSandboxPolicy::from, + ) + } + + pub fn network_sandbox_policy(&self) -> NetworkSandboxPolicy { + if self + .network + .as_ref() + .and_then(|network| network.enabled) + .unwrap_or(false) + { + NetworkSandboxPolicy::Enabled + } else { + NetworkSandboxPolicy::Restricted + } + } + + pub fn to_legacy_sandbox_policy(&self, cwd: &Path) -> io::Result { + self.file_system_sandbox_policy() + .to_legacy_sandbox_policy(self.network_sandbox_policy(), cwd) + } +} + +impl From for NetworkPermissions { + fn from(value: NetworkSandboxPolicy) -> Self { + Self { + enabled: Some(value.is_enabled()), + } + } +} + +impl From<&FileSystemSandboxPolicy> for FileSystemPermissions { + fn from(value: &FileSystemSandboxPolicy) -> Self { + let entries = match value.kind { + crate::permissions::FileSystemSandboxKind::Restricted => value.entries.clone(), + crate::permissions::FileSystemSandboxKind::Unrestricted + | crate::permissions::FileSystemSandboxKind::ExternalSandbox => { + vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }] + } + }; + Self { entries } + } +} + +impl From<&FileSystemPermissions> for FileSystemSandboxPolicy { + fn from(value: &FileSystemPermissions) -> Self { + FileSystemSandboxPolicy::restricted(value.entries.clone()) + } } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)] @@ -1436,6 +1621,10 @@ mod tests { use std::path::PathBuf; use tempfile::tempdir; + fn absolute_path(path: &str) -> AbsolutePathBuf { + AbsolutePathBuf::from_absolute_path(path).expect("path must be absolute") + } + #[test] fn sandbox_permissions_helpers_match_documented_semantics() { let cases = [ @@ -1546,6 +1735,58 @@ mod tests { assert_eq!(permission_profile.is_empty(), false); } + #[test] + fn file_system_permissions_deserialize_legacy_read_write_shape() { + let file_system = serde_json::from_value::(serde_json::json!({ + "read": [absolute_path("/tmp/read-only")], + "write": [absolute_path("/tmp/read-write")], + })) + .expect("deserialize legacy filesystem permissions"); + + assert_eq!( + file_system, + FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/read-only")]), + Some(vec![absolute_path("/tmp/read-write")]), + ) + ); + } + + #[test] + fn file_system_permissions_serialize_explicit_paths_as_legacy_read_write_shape() { + let file_system = FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/read-only")]), + Some(vec![absolute_path("/tmp/read-write")]), + ); + + let value = serde_json::to_value(file_system).expect("serialize filesystem permissions"); + + assert_eq!( + value, + serde_json::json!({ + "read": [absolute_path("/tmp/read-only")], + "write": [absolute_path("/tmp/read-write")], + }) + ); + } + + #[test] + fn file_system_permissions_round_trip_special_entries_through_canonical_shape() { + let file_system = FileSystemPermissions { + entries: vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }], + }; + + let value = serde_json::to_value(&file_system).expect("serialize filesystem permissions"); + let reparsed = serde_json::from_value::(value) + .expect("deserialize filesystem permissions"); + + assert_eq!(reparsed, file_system); + } #[test] fn convert_mcp_content_to_items_builds_data_urls_when_missing_prefix() { let contents = vec![serde_json::json!({ diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 4aa1c7b36f..de3dade0d7 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -45,6 +45,7 @@ impl NetworkSandboxPolicy { Copy, PartialEq, Eq, + Hash, PartialOrd, Ord, Serialize, @@ -71,7 +72,7 @@ impl FileSystemAccessMode { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] #[serde(tag = "kind", rename_all = "snake_case")] #[ts(tag = "kind")] pub enum FileSystemSpecialPath { @@ -114,7 +115,7 @@ impl FileSystemSpecialPath { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] pub struct FileSystemSandboxEntry { pub path: FileSystemPath, pub access: FileSystemAccessMode, @@ -155,7 +156,7 @@ struct FileSystemSemanticSignature { unreadable_roots: Vec, } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] #[serde(tag = "type", rename_all = "snake_case")] #[ts(tag = "type")] pub enum FileSystemPath { @@ -369,6 +370,50 @@ impl FileSystemSandboxPolicy { self.resolve_access_with_cwd(path, cwd).can_write() } + pub fn covers_entry_with_cwd(&self, entry: &FileSystemSandboxEntry, cwd: &Path) -> bool { + match &entry.path { + FileSystemPath::Path { path } => match entry.access { + FileSystemAccessMode::Read => self.can_read_path_with_cwd(path.as_path(), cwd), + FileSystemAccessMode::Write => self.can_write_path_with_cwd(path.as_path(), cwd), + FileSystemAccessMode::None => { + self.resolve_access_with_cwd(path.as_path(), cwd) == FileSystemAccessMode::None + } + }, + FileSystemPath::Special { value } => match value { + FileSystemSpecialPath::Root => match entry.access { + FileSystemAccessMode::Read => self.has_full_disk_read_access(), + FileSystemAccessMode::Write => self.has_full_disk_write_access(), + FileSystemAccessMode::None => { + !self.has_full_disk_read_access() && !self.has_full_disk_write_access() + } + }, + FileSystemSpecialPath::Minimal => match entry.access { + FileSystemAccessMode::Read => { + self.has_full_disk_read_access() || self.include_platform_defaults() + } + FileSystemAccessMode::Write => self.has_full_disk_write_access(), + FileSystemAccessMode::None => { + !self.has_full_disk_read_access() && !self.include_platform_defaults() + } + }, + _ => resolve_file_system_special_path( + value, + AbsolutePathBuf::from_absolute_path(cwd).ok().as_ref(), + ) + .is_some_and(|path| match entry.access { + FileSystemAccessMode::Read => self.can_read_path_with_cwd(path.as_path(), cwd), + FileSystemAccessMode::Write => { + self.can_write_path_with_cwd(path.as_path(), cwd) + } + FileSystemAccessMode::None => { + self.resolve_access_with_cwd(path.as_path(), cwd) + == FileSystemAccessMode::None + } + }), + }, + } + } + pub fn with_additional_readable_roots( mut self, cwd: &Path, diff --git a/codex-rs/sandboxing/src/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index 6fc34c9637..7ee57ddd6a 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -130,10 +130,10 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![path]), - write: Some(Vec::new()), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path]), + Some(Vec::new()), + )), }), }, policy: &SandboxPolicy::ExternalSandbox { @@ -183,10 +183,10 @@ fn transform_additional_permissions_preserves_denied_entries() { cwd: cwd.clone(), env: HashMap::new(), additional_permissions: Some(PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![allowed_path.clone()]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![allowed_path.clone()]), + )), ..Default::default() }), }, diff --git a/codex-rs/sandboxing/src/policy_transforms.rs b/codex-rs/sandboxing/src/policy_transforms.rs index 25c79fcc9e..ffac9e8813 100644 --- a/codex-rs/sandboxing/src/policy_transforms.rs +++ b/codex-rs/sandboxing/src/policy_transforms.rs @@ -1,7 +1,6 @@ use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::NetworkPermissions; use codex_protocol::models::PermissionProfile; -use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxKind; @@ -13,6 +12,7 @@ use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use dunce::canonicalize; use std::collections::HashSet; +use std::path::Path; #[derive(Debug, Clone, PartialEq, Eq)] pub struct EffectiveSandboxPermissions { @@ -45,13 +45,26 @@ pub fn normalize_additional_permissions( let file_system = additional_permissions .file_system .map(|file_system| { - let read = file_system - .read - .map(|paths| normalize_permission_paths(paths, "file_system.read")); - let write = file_system - .write - .map(|paths| normalize_permission_paths(paths, "file_system.write")); - FileSystemPermissions { read, write } + let mut entries = Vec::with_capacity(file_system.entries.len()); + for entry in file_system.entries { + let path = match entry.path { + FileSystemPath::Path { path } => FileSystemPath::Path { + path: canonicalize(path.as_path()) + .ok() + .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) + .unwrap_or(path), + }, + FileSystemPath::Special { value } => FileSystemPath::Special { value }, + }; + let normalized_entry = FileSystemSandboxEntry { + path, + access: entry.access, + }; + if !entries.contains(&normalized_entry) { + entries.push(normalized_entry); + } + } + FileSystemPermissions { entries } }) .filter(|file_system| !file_system.is_empty()); Ok(PermissionProfile { @@ -89,8 +102,7 @@ pub fn merge_permission_profiles( }; let file_system = match (base.file_system.as_ref(), permissions.file_system.as_ref()) { (Some(base), Some(permissions)) => Some(FileSystemPermissions { - read: merge_permission_paths(base.read.as_ref(), permissions.read.as_ref()), - write: merge_permission_paths(base.write.as_ref(), permissions.write.as_ref()), + entries: merge_permission_entries(&base.entries, &permissions.entries), }) .filter(|file_system| !file_system.is_empty()), (Some(base), None) => Some(base.clone()), @@ -115,12 +127,13 @@ pub fn intersect_permission_profiles( let file_system = requested .file_system .map(|requested_file_system| { - let granted_file_system = granted.file_system.unwrap_or_default(); - let read = - intersect_permission_paths(requested_file_system.read, granted_file_system.read); - let write = - intersect_permission_paths(requested_file_system.write, granted_file_system.write); - FileSystemPermissions { read, write } + let granted_entries = granted.file_system.unwrap_or_default().entries; + let entries = requested_file_system + .entries + .into_iter() + .filter(|entry| granted_entries.contains(entry)) + .collect(); + FileSystemPermissions { entries } }) .filter(|file_system| !file_system.is_empty()); let network = match (requested.network, granted.network) { @@ -143,67 +156,46 @@ pub fn intersect_permission_profiles( } } -fn intersect_permission_paths( - requested: Option>, - granted: Option>, -) -> Option> { - requested.and_then(|requested_paths| { - if requested_paths.is_empty() { - return granted.map(|_| Vec::new()); +pub fn permission_profile_is_preapproved( + requested: &PermissionProfile, + granted: &PermissionProfile, + cwd: &Path, +) -> bool { + let network_preapproved = !requested + .network + .as_ref() + .and_then(|network| network.enabled) + .unwrap_or(false) + || granted + .network + .as_ref() + .and_then(|network| network.enabled) + .unwrap_or(false); + let file_system_preapproved = match requested.file_system.as_ref() { + Some(requested_file_system) => { + let granted_policy = granted.file_system_sandbox_policy(); + requested_file_system + .entries + .iter() + .all(|entry| granted_policy.covers_entry_with_cwd(entry, cwd)) } + None => true, + }; - let granted_paths = granted.unwrap_or_default(); - Some( - requested_paths - .into_iter() - .filter(|path| granted_paths.contains(path)) - .collect::>(), - ) - .filter(|paths| !paths.is_empty()) - }) + network_preapproved && file_system_preapproved } -fn normalize_permission_paths( - paths: Vec, - _permission_kind: &str, -) -> Vec { - let mut out = Vec::with_capacity(paths.len()); - let mut seen = HashSet::new(); - - for path in paths { - let canonicalized = canonicalize(path.as_path()) - .ok() - .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) - .unwrap_or(path); - if seen.insert(canonicalized.clone()) { - out.push(canonicalized); +fn merge_permission_entries( + base: &[FileSystemSandboxEntry], + permissions: &[FileSystemSandboxEntry], +) -> Vec { + let mut merged = Vec::with_capacity(base.len() + permissions.len()); + for entry in base.iter().chain(permissions.iter()) { + if !merged.contains(entry) { + merged.push(entry.clone()); } } - - out -} - -fn merge_permission_paths( - base: Option<&Vec>, - permissions: Option<&Vec>, -) -> Option> { - match (base, permissions) { - (Some(base), Some(permissions)) => { - let mut merged = Vec::with_capacity(base.len() + permissions.len()); - let mut seen = HashSet::with_capacity(base.len() + permissions.len()); - - for path in base.iter().chain(permissions.iter()) { - if seen.insert(path.clone()) { - merged.push(path.clone()); - } - } - - Some(merged).filter(|paths| !paths.is_empty()) - } - (Some(base), None) => Some(base.clone()), - (None, Some(permissions)) => Some(permissions.clone()), - (None, None) => None, - } + merged } fn dedup_absolute_paths(paths: Vec) -> Vec { @@ -217,7 +209,7 @@ fn dedup_absolute_paths(paths: Vec) -> Vec { out } -fn additional_permission_roots( +fn additional_permission_explicit_path_roots( additional_permissions: &PermissionProfile, ) -> (Vec, Vec) { ( @@ -225,14 +217,24 @@ fn additional_permission_roots( additional_permissions .file_system .as_ref() - .and_then(|file_system| file_system.read.clone()) + .map(|file_system| { + file_system + .explicit_path_entries() + .filter_map(|(path, access)| access.can_read().then_some(path.clone())) + .collect() + }) .unwrap_or_default(), ), dedup_absolute_paths( additional_permissions .file_system .as_ref() - .and_then(|file_system| file_system.write.clone()) + .map(|file_system| { + file_system + .explicit_path_entries() + .filter_map(|(path, access)| access.can_write().then_some(path.clone())) + .collect() + }) .unwrap_or_default(), ), ) @@ -240,28 +242,14 @@ fn additional_permission_roots( fn merge_file_system_policy_with_additional_permissions( file_system_policy: &FileSystemSandboxPolicy, - extra_reads: Vec, - extra_writes: Vec, + additional_permissions: &FileSystemPermissions, ) -> FileSystemSandboxPolicy { match file_system_policy.kind { FileSystemSandboxKind::Restricted => { let mut merged_policy = file_system_policy.clone(); - for path in extra_reads { - let entry = FileSystemSandboxEntry { - path: FileSystemPath::Path { path }, - access: FileSystemAccessMode::Read, - }; - if !merged_policy.entries.contains(&entry) { - merged_policy.entries.push(entry); - } - } - for path in extra_writes { - let entry = FileSystemSandboxEntry { - path: FileSystemPath::Path { path }, - access: FileSystemAccessMode::Write, - }; - if !merged_policy.entries.contains(&entry) { - merged_policy.entries.push(entry); + for entry in &additional_permissions.entries { + if !merged_policy.entries.contains(entry) { + merged_policy.entries.push(entry.clone()); } } merged_policy @@ -280,14 +268,15 @@ pub fn effective_file_system_sandbox_policy( return file_system_policy.clone(); }; - let (extra_reads, extra_writes) = additional_permission_roots(additional_permissions); - if extra_reads.is_empty() && extra_writes.is_empty() { + let Some(file_system_permissions) = additional_permissions.file_system.as_ref() else { + return file_system_policy.clone(); + }; + if file_system_permissions.is_empty() { file_system_policy.clone() } else { merge_file_system_policy_with_additional_permissions( file_system_policy, - extra_reads, - extra_writes, + file_system_permissions, ) } } @@ -347,7 +336,11 @@ fn sandbox_policy_with_additional_permissions( return sandbox_policy.clone(); } - let (extra_reads, extra_writes) = additional_permission_roots(additional_permissions); + // Legacy SandboxPolicy remains a best-effort projection during the + // migration to PermissionProfile-backed thread permissions. The direct + // filesystem sandbox policy carries the full permission shape. + let (extra_reads, extra_writes) = + additional_permission_explicit_path_roots(additional_permissions); match sandbox_policy { SandboxPolicy::DangerFullAccess => SandboxPolicy::DangerFullAccess, diff --git a/codex-rs/sandboxing/src/policy_transforms_tests.rs b/codex-rs/sandboxing/src/policy_transforms_tests.rs index d287bd243c..e5f9e5a60e 100644 --- a/codex-rs/sandboxing/src/policy_transforms_tests.rs +++ b/codex-rs/sandboxing/src/policy_transforms_tests.rs @@ -2,6 +2,7 @@ use super::effective_file_system_sandbox_policy; use super::intersect_permission_profiles; use super::merge_file_system_policy_with_additional_permissions; use super::normalize_additional_permissions; +use super::permission_profile_is_preapproved; use super::sandbox_policy_with_additional_permissions; use super::should_require_platform_sandbox; use codex_protocol::models::FileSystemPermissions; @@ -107,10 +108,10 @@ fn normalize_additional_permissions_preserves_network() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![path.clone()]), - write: Some(vec![path.clone()]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path.clone()]), + Some(vec![path.clone()]), + )), }) .expect("permissions"); @@ -122,10 +123,10 @@ fn normalize_additional_permissions_preserves_network() { ); assert_eq!( permissions.file_system, - Some(FileSystemPermissions { - read: Some(vec![path.clone()]), - write: Some(vec![path]), - }) + Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path.clone()]), + Some(vec![path]), + )) ); } @@ -147,20 +148,20 @@ fn normalize_additional_permissions_canonicalizes_symlinked_write_paths() { .expect("absolute canonical write dir"); let permissions = normalize_additional_permissions(PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![link_write_dir]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![link_write_dir]), + )), ..Default::default() }) .expect("permissions"); assert_eq!( permissions.file_system, - Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![expected_write_dir]), - }) + Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![expected_write_dir]), + )) ); } @@ -168,50 +169,22 @@ fn normalize_additional_permissions_canonicalizes_symlinked_write_paths() { fn normalize_additional_permissions_drops_empty_nested_profiles() { let permissions = normalize_additional_permissions(PermissionProfile { network: Some(NetworkPermissions { enabled: None }), - file_system: Some(FileSystemPermissions { - read: None, - write: None, - }), + file_system: Some(FileSystemPermissions::default()), }) .expect("permissions"); assert_eq!(permissions, PermissionProfile::default()); } -#[test] -fn intersect_permission_profiles_preserves_explicit_empty_requested_reads() { - let temp_dir = TempDir::new().expect("create temp dir"); - let path = AbsolutePathBuf::from_absolute_path( - canonicalize(temp_dir.path()).expect("canonicalize temp dir"), - ) - .expect("absolute temp dir"); - let requested = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![path]), - }), - ..Default::default() - }; - let granted = requested.clone(); - - assert_eq!( - intersect_permission_profiles(requested.clone(), granted), - requested - ); -} - #[test] fn intersect_permission_profiles_drops_ungranted_nonempty_path_requests() { - let temp_dir = TempDir::new().expect("create temp dir"); - let path = AbsolutePathBuf::from_absolute_path( - canonicalize(temp_dir.path()).expect("canonicalize temp dir"), - ) - .expect("absolute temp dir"); let requested = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![path]), - write: None, - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(Vec::from(["/tmp/requested" + .try_into() + .expect("absolute path")])), + /*write*/ None, + )), ..Default::default() }; @@ -222,24 +195,65 @@ fn intersect_permission_profiles_drops_ungranted_nonempty_path_requests() { } #[test] -fn intersect_permission_profiles_drops_explicit_empty_reads_without_grant() { +fn permission_profile_is_preapproved_when_granted_parent_directory_covers_requested_child() { let temp_dir = TempDir::new().expect("create temp dir"); - let path = AbsolutePathBuf::from_absolute_path( + let parent = AbsolutePathBuf::from_absolute_path( canonicalize(temp_dir.path()).expect("canonicalize temp dir"), ) .expect("absolute temp dir"); + let child = AbsolutePathBuf::from_absolute_path(parent.as_path().join("child")) + .expect("absolute child path"); + let granted = PermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![parent]), + )), + ..Default::default() + }; let requested = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![path]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![child]), + )), ..Default::default() }; - assert_eq!( - intersect_permission_profiles(requested, PermissionProfile::default()), - PermissionProfile::default() - ); + assert!(permission_profile_is_preapproved( + &requested, + &granted, + temp_dir.path(), + )); +} + +#[test] +fn permission_profile_is_preapproved_when_granted_cwd_covers_requested_child() { + let temp_dir = TempDir::new().expect("create temp dir"); + let child = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("child")) + .expect("absolute child path"); + let granted = PermissionProfile { + file_system: Some(FileSystemPermissions { + entries: vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }], + }), + ..Default::default() + }; + let requested = PermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![child]), + )), + ..Default::default() + }; + + assert!(permission_profile_is_preapproved( + &requested, + &granted, + temp_dir.path(), + )); } #[test] @@ -261,10 +275,10 @@ fn read_only_additional_permissions_can_enable_network_without_writes() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![path.clone()]), - write: Some(Vec::new()), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path.clone()]), + Some(Vec::new()), + )), }, ); @@ -295,10 +309,10 @@ fn external_sandbox_additional_permissions_can_enable_network() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![path]), - write: Some(Vec::new()), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path]), + Some(Vec::new()), + )), }, ); @@ -334,8 +348,10 @@ fn merge_file_system_policy_with_additional_permissions_preserves_unreadable_roo access: FileSystemAccessMode::None, }, ]), - vec![allowed_path.clone()], - Vec::new(), + &FileSystemPermissions::from_read_write_roots( + Some(vec![allowed_path.clone()]), + Some(Vec::new()), + ), ); assert_eq!( @@ -405,10 +421,10 @@ fn effective_file_system_sandbox_policy_merges_additional_write_roots() { }, ]); let additional_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![allowed_path.clone()]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![allowed_path.clone()]), + )), ..Default::default() }; diff --git a/codex-rs/tools/BUILD.bazel b/codex-rs/tools/BUILD.bazel index d2e730cfa9..7b1541e4e8 100644 --- a/codex-rs/tools/BUILD.bazel +++ b/codex-rs/tools/BUILD.bazel @@ -3,4 +3,7 @@ load("//:defs.bzl", "codex_rust_crate") codex_rust_crate( name = "tools", crate_name = "codex_tools", + compile_data = [ + "src/tool_apply_patch.lark", + ], ) diff --git a/codex-rs/tools/src/apply_patch_tool.rs b/codex-rs/tools/src/apply_patch_tool.rs new file mode 100644 index 0000000000..6360eef5bd --- /dev/null +++ b/codex-rs/tools/src/apply_patch_tool.rs @@ -0,0 +1,126 @@ +use crate::FreeformTool; +use crate::FreeformToolFormat; +use crate::JsonSchema; +use crate::ResponsesApiTool; +use crate::ToolSpec; +use serde::Deserialize; +use serde::Serialize; +use std::collections::BTreeMap; + +const APPLY_PATCH_LARK_GRAMMAR: &str = include_str!("tool_apply_patch.lark"); + +const APPLY_PATCH_JSON_TOOL_DESCRIPTION: &str = r#"Use the `apply_patch` tool to edit files. +Your patch language is a stripped‑down, file‑oriented diff format designed to be easy to parse and safe to apply. You can think of it as a high‑level envelope: + +*** Begin Patch +[ one or more file sections ] +*** End Patch + +Within that envelope, you get a sequence of file operations. +You MUST include a header to specify the action you are taking. +Each operation starts with one of three headers: + +*** Add File: - create a new file. Every following line is a + line (the initial contents). +*** Delete File: - remove an existing file. Nothing follows. +*** Update File: - patch an existing file in place (optionally with a rename). + +May be immediately followed by *** Move to: if you want to rename the file. +Then one or more “hunks”, each introduced by @@ (optionally followed by a hunk header). +Within a hunk each line starts with: + +For instructions on [context_before] and [context_after]: +- By default, show 3 lines of code immediately above and 3 lines immediately below each change. If a change is within 3 lines of a previous change, do NOT duplicate the first change’s [context_after] lines in the second change’s [context_before] lines. +- If 3 lines of context is insufficient to uniquely identify the snippet of code within the file, use the @@ operator to indicate the class or function to which the snippet belongs. For instance, we might have: +@@ class BaseClass +[3 lines of pre-context] +- [old_code] ++ [new_code] +[3 lines of post-context] + +- If a code block is repeated so many times in a class or function such that even a single `@@` statement and 3 lines of context cannot uniquely identify the snippet of code, you can use multiple `@@` statements to jump to the right context. For instance: + +@@ class BaseClass +@@ def method(): +[3 lines of pre-context] +- [old_code] ++ [new_code] +[3 lines of post-context] + +The full grammar definition is below: +Patch := Begin { FileOp } End +Begin := "*** Begin Patch" NEWLINE +End := "*** End Patch" NEWLINE +FileOp := AddFile | DeleteFile | UpdateFile +AddFile := "*** Add File: " path NEWLINE { "+" line NEWLINE } +DeleteFile := "*** Delete File: " path NEWLINE +UpdateFile := "*** Update File: " path NEWLINE [ MoveTo ] { Hunk } +MoveTo := "*** Move to: " newPath NEWLINE +Hunk := "@@" [ header ] NEWLINE { HunkLine } [ "*** End of File" NEWLINE ] +HunkLine := (" " | "-" | "+") text NEWLINE + +A full patch can combine several operations: + +*** Begin Patch +*** Add File: hello.txt ++Hello world +*** Update File: src/app.py +*** Move to: src/main.py +@@ def greet(): +-print("Hi") ++print("Hello, world!") +*** Delete File: obsolete.txt +*** End Patch + +It is important to remember: + +- You must include a header with your intended action (Add/Delete/Update) +- You must prefix new lines with `+` even when creating a new file +- File references can only be relative, NEVER ABSOLUTE. +"#; + +/// TODO(dylan): deprecate once we get rid of json tool +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ApplyPatchToolArgs { + pub input: String, +} + +/// Returns a custom tool that can be used to edit files. Well-suited for GPT-5 models +/// https://platform.openai.com/docs/guides/function-calling#custom-tools +pub fn create_apply_patch_freeform_tool() -> ToolSpec { + ToolSpec::Freeform(FreeformTool { + name: "apply_patch".to_string(), + description: "Use the `apply_patch` tool to edit files. This is a FREEFORM tool, so do not wrap the patch in JSON.".to_string(), + format: FreeformToolFormat { + r#type: "grammar".to_string(), + syntax: "lark".to_string(), + definition: APPLY_PATCH_LARK_GRAMMAR.to_string(), + }, + }) +} + +/// Returns a json tool that can be used to edit files. Should only be used with gpt-oss models +pub fn create_apply_patch_json_tool() -> ToolSpec { + let properties = BTreeMap::from([( + "input".to_string(), + JsonSchema::String { + description: Some("The entire contents of the apply_patch command".to_string()), + }, + )]); + + ToolSpec::Function(ResponsesApiTool { + name: "apply_patch".to_string(), + description: APPLY_PATCH_JSON_TOOL_DESCRIPTION.to_string(), + strict: false, + defer_loading: None, + parameters: JsonSchema::Object { + properties, + required: Some(vec!["input".to_string()]), + additional_properties: Some(false.into()), + }, + output_schema: None, + }) +} + +#[cfg(test)] +#[path = "apply_patch_tool_tests.rs"] +mod tests; diff --git a/codex-rs/tools/src/apply_patch_tool_tests.rs b/codex-rs/tools/src/apply_patch_tool_tests.rs new file mode 100644 index 0000000000..5b3f1117c6 --- /dev/null +++ b/codex-rs/tools/src/apply_patch_tool_tests.rs @@ -0,0 +1,47 @@ +use super::*; +use pretty_assertions::assert_eq; +use std::collections::BTreeMap; + +#[test] +fn create_apply_patch_freeform_tool_matches_expected_spec() { + assert_eq!( + create_apply_patch_freeform_tool(), + ToolSpec::Freeform(FreeformTool { + name: "apply_patch".to_string(), + description: + "Use the `apply_patch` tool to edit files. This is a FREEFORM tool, so do not wrap the patch in JSON." + .to_string(), + format: FreeformToolFormat { + r#type: "grammar".to_string(), + syntax: "lark".to_string(), + definition: APPLY_PATCH_LARK_GRAMMAR.to_string(), + }, + }) + ); +} + +#[test] +fn create_apply_patch_json_tool_matches_expected_spec() { + assert_eq!( + create_apply_patch_json_tool(), + ToolSpec::Function(ResponsesApiTool { + name: "apply_patch".to_string(), + description: APPLY_PATCH_JSON_TOOL_DESCRIPTION.to_string(), + strict: false, + defer_loading: None, + parameters: JsonSchema::Object { + properties: BTreeMap::from([( + "input".to_string(), + JsonSchema::String { + description: Some( + "The entire contents of the apply_patch command".to_string(), + ), + }, + )]), + required: Some(vec!["input".to_string()]), + additional_properties: Some(false.into()), + }, + output_schema: None, + }) + ); +} diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 77622eed35..1a65e1dda9 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -3,6 +3,7 @@ mod agent_job_tool; mod agent_tool; +mod apply_patch_tool; mod code_mode; mod dynamic_tool; mod image_detail; @@ -35,6 +36,9 @@ pub use agent_tool::create_spawn_agent_tool_v1; pub use agent_tool::create_spawn_agent_tool_v2; pub use agent_tool::create_wait_agent_tool_v1; pub use agent_tool::create_wait_agent_tool_v2; +pub use apply_patch_tool::ApplyPatchToolArgs; +pub use apply_patch_tool::create_apply_patch_freeform_tool; +pub use apply_patch_tool::create_apply_patch_json_tool; pub use code_mode::augment_tool_spec_for_code_mode; pub use code_mode::create_code_mode_tool; pub use code_mode::create_wait_tool; @@ -54,12 +58,15 @@ pub use local_tool::create_request_permissions_tool; pub use local_tool::create_shell_command_tool; pub use local_tool::create_shell_tool; pub use local_tool::create_write_stdin_tool; +pub use local_tool::request_permissions_tool_description; pub use mcp_resource_tool::create_list_mcp_resource_templates_tool; pub use mcp_resource_tool::create_list_mcp_resources_tool; pub use mcp_resource_tool::create_read_mcp_resource_tool; pub use mcp_tool::mcp_call_tool_result_output_schema; pub use mcp_tool::parse_mcp_tool; pub use request_user_input_tool::create_request_user_input_tool; +pub use request_user_input_tool::request_user_input_tool_description; +pub use request_user_input_tool::request_user_input_unavailable_message; pub use responses_api::FreeformTool; pub use responses_api::FreeformToolFormat; pub use responses_api::ResponsesApiNamespace; diff --git a/codex-rs/tools/src/local_tool.rs b/codex-rs/tools/src/local_tool.rs index 4b29264ecf..8779ac2631 100644 --- a/codex-rs/tools/src/local_tool.rs +++ b/codex-rs/tools/src/local_tool.rs @@ -318,6 +318,11 @@ pub fn create_request_permissions_tool(description: String) -> ToolSpec { }) } +pub fn request_permissions_tool_description() -> String { + "Request additional filesystem or network permissions from the user and wait for the client to grant a subset of the requested permission profile. Granted permissions apply automatically to later shell-like commands in the current turn, or for the rest of the session if the client approves them at session scope." + .to_string() +} + fn unified_exec_output_schema() -> Value { json!({ "type": "object", diff --git a/codex-rs/tools/src/request_user_input_tool.rs b/codex-rs/tools/src/request_user_input_tool.rs index 8d34c68f97..dd4f4f9954 100644 --- a/codex-rs/tools/src/request_user_input_tool.rs +++ b/codex-rs/tools/src/request_user_input_tool.rs @@ -1,6 +1,8 @@ use crate::JsonSchema; use crate::ResponsesApiTool; use crate::ToolSpec; +use codex_protocol::config_types::ModeKind; +use codex_protocol::config_types::TUI_VISIBLE_COLLABORATION_MODES; use std::collections::BTreeMap; pub fn create_request_user_input_tool(description: String) -> ToolSpec { @@ -89,6 +91,47 @@ pub fn create_request_user_input_tool(description: String) -> ToolSpec { }) } +pub fn request_user_input_unavailable_message( + mode: ModeKind, + default_mode_request_user_input: bool, +) -> Option { + if request_user_input_is_available(mode, default_mode_request_user_input) { + None + } else { + let mode_name = mode.display_name(); + Some(format!( + "request_user_input is unavailable in {mode_name} mode" + )) + } +} + +pub fn request_user_input_tool_description(default_mode_request_user_input: bool) -> String { + let allowed_modes = format_allowed_modes(default_mode_request_user_input); + format!( + "Request user input for one to three short questions and wait for the response. This tool is only available in {allowed_modes}." + ) +} + +fn request_user_input_is_available(mode: ModeKind, default_mode_request_user_input: bool) -> bool { + mode.allows_request_user_input() + || (default_mode_request_user_input && mode == ModeKind::Default) +} + +fn format_allowed_modes(default_mode_request_user_input: bool) -> String { + let mode_names: Vec<&str> = TUI_VISIBLE_COLLABORATION_MODES + .into_iter() + .filter(|mode| request_user_input_is_available(*mode, default_mode_request_user_input)) + .map(ModeKind::display_name) + .collect(); + + match mode_names.as_slice() { + [] => "no modes".to_string(), + [mode] => format!("{mode} mode"), + [first, second] => format!("{first} or {second} mode"), + [..] => format!("modes: {}", mode_names.join(",")), + } +} + #[cfg(test)] #[path = "request_user_input_tool_tests.rs"] mod tests; diff --git a/codex-rs/tools/src/request_user_input_tool_tests.rs b/codex-rs/tools/src/request_user_input_tool_tests.rs index 132dc14d1a..e7a305f86d 100644 --- a/codex-rs/tools/src/request_user_input_tool_tests.rs +++ b/codex-rs/tools/src/request_user_input_tool_tests.rs @@ -1,4 +1,5 @@ use super::*; +use codex_protocol::config_types::ModeKind; use pretty_assertions::assert_eq; use std::collections::BTreeMap; @@ -100,3 +101,54 @@ fn request_user_input_tool_includes_questions_schema() { }) ); } + +#[test] +fn request_user_input_unavailable_messages_respect_default_mode_feature_flag() { + assert_eq!( + request_user_input_unavailable_message( + ModeKind::Plan, + /*default_mode_request_user_input*/ false + ), + None + ); + assert_eq!( + request_user_input_unavailable_message( + ModeKind::Default, + /*default_mode_request_user_input*/ false + ), + Some("request_user_input is unavailable in Default mode".to_string()) + ); + assert_eq!( + request_user_input_unavailable_message( + ModeKind::Default, + /*default_mode_request_user_input*/ true + ), + None + ); + assert_eq!( + request_user_input_unavailable_message( + ModeKind::Execute, + /*default_mode_request_user_input*/ false + ), + Some("request_user_input is unavailable in Execute mode".to_string()) + ); + assert_eq!( + request_user_input_unavailable_message( + ModeKind::PairProgramming, + /*default_mode_request_user_input*/ false + ), + Some("request_user_input is unavailable in Pair Programming mode".to_string()) + ); +} + +#[test] +fn request_user_input_tool_description_mentions_available_modes() { + assert_eq!( + request_user_input_tool_description(/*default_mode_request_user_input*/ false), + "Request user input for one to three short questions and wait for the response. This tool is only available in Plan mode.".to_string() + ); + assert_eq!( + request_user_input_tool_description(/*default_mode_request_user_input*/ true), + "Request user input for one to three short questions and wait for the response. This tool is only available in Default or Plan mode.".to_string() + ); +} diff --git a/codex-rs/core/src/tools/handlers/tool_apply_patch.lark b/codex-rs/tools/src/tool_apply_patch.lark similarity index 100% rename from codex-rs/core/src/tools/handlers/tool_apply_patch.lark rename to codex-rs/tools/src/tool_apply_patch.lark diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 82e74888f9..0e91f80256 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -20,6 +20,9 @@ use codex_features::Features; use codex_protocol::ThreadId; use codex_protocol::mcp::RequestId; use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::ElicitationAction; use codex_protocol::protocol::FileChange; use codex_protocol::protocol::NetworkApprovalContext; @@ -743,22 +746,20 @@ pub(crate) fn format_additional_permissions_rule( parts.push("network".to_string()); } if let Some(file_system) = additional_permissions.file_system.as_ref() { - if let Some(read) = file_system.read.as_ref() { - let reads = read - .iter() - .map(|path| format!("`{}`", path.display())) - .collect::>() - .join(", "); + if let Some(reads) = format_file_system_permissions(file_system, FileSystemAccessMode::Read) + { parts.push(format!("read {reads}")); } - if let Some(write) = file_system.write.as_ref() { - let writes = write - .iter() - .map(|path| format!("`{}`", path.display())) - .collect::>() - .join(", "); + if let Some(writes) = + format_file_system_permissions(file_system, FileSystemAccessMode::Write) + { parts.push(format!("write {writes}")); } + if let Some(denies) = + format_file_system_permissions(file_system, FileSystemAccessMode::None) + { + parts.push(format!("deny {denies}")); + } } if parts.is_empty() { None @@ -767,6 +768,40 @@ pub(crate) fn format_additional_permissions_rule( } } +fn format_file_system_permissions( + file_system: &codex_protocol::models::FileSystemPermissions, + access: FileSystemAccessMode, +) -> Option { + let values = file_system + .entries + .iter() + .filter(|entry| entry.access == access) + .map(|entry| format!("`{}`", format_file_system_path(&entry.path))) + .collect::>(); + (!values.is_empty()).then(|| values.join(", ")) +} + +fn format_file_system_path(path: &FileSystemPath) -> String { + match path { + FileSystemPath::Path { path } => path.display().to_string(), + FileSystemPath::Special { value } => match value { + FileSystemSpecialPath::Root => ":root".to_string(), + FileSystemSpecialPath::Minimal => ":minimal".to_string(), + FileSystemSpecialPath::CurrentWorkingDirectory => ":cwd".to_string(), + FileSystemSpecialPath::ProjectRoots { subpath } => subpath.as_ref().map_or_else( + || ":project_roots".to_string(), + |subpath| format!(":project_roots/{}", subpath.display()), + ), + FileSystemSpecialPath::Tmpdir => ":tmpdir".to_string(), + FileSystemSpecialPath::SlashTmp => "/tmp".to_string(), + FileSystemSpecialPath::Unknown { path, subpath } => subpath.as_ref().map_or_else( + || path.clone(), + |subpath| format!("{path}/{}", subpath.display()), + ), + }, + } +} + pub(crate) fn format_requested_permissions_rule( permissions: &RequestPermissionProfile, ) -> Option { @@ -848,6 +883,10 @@ mod tests { use crate::app_event::AppEvent; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::NetworkPermissions; + use codex_protocol::permissions::FileSystemAccessMode; + use codex_protocol::permissions::FileSystemPath; + use codex_protocol::permissions::FileSystemSandboxEntry; + use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::ExecPolicyAmendment; use codex_protocol::protocol::NetworkApprovalProtocol; use codex_protocol::protocol::NetworkPolicyAmendment; @@ -880,6 +919,7 @@ mod tests { [ (absolute_path("/tmp/readme.txt"), "/tmp/readme.txt"), (absolute_path("/tmp/out.txt"), "/tmp/out.txt"), + (absolute_path("/tmp/secret.txt"), "/tmp/secret.txt"), ] .into_iter() .fold(rendered, |rendered, (path, normalized)| { @@ -910,10 +950,10 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/readme.txt")]), - write: Some(vec![absolute_path("/tmp/out.txt")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/readme.txt")]), + Some(vec![absolute_path("/tmp/out.txt")]), + )), }, } } @@ -1190,10 +1230,10 @@ mod tests { #[test] fn additional_permissions_exec_options_hide_execpolicy_amendment() { let additional_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/readme.txt")]), - write: Some(vec![absolute_path("/tmp/out.txt")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/readme.txt")]), + Some(vec![absolute_path("/tmp/out.txt")]), + )), ..Default::default() }; let options = exec_options( @@ -1271,10 +1311,10 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/readme.txt")]), - write: Some(vec![absolute_path("/tmp/out.txt")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/readme.txt")]), + Some(vec![absolute_path("/tmp/out.txt")]), + )), }), }; @@ -1321,10 +1361,10 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/readme.txt")]), - write: Some(vec![absolute_path("/tmp/out.txt")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/readme.txt")]), + Some(vec![absolute_path("/tmp/out.txt")]), + )), }), }; @@ -1335,6 +1375,46 @@ mod tests { ); } + #[test] + fn additional_permissions_special_entries_prompt_snapshot() { + let (tx, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx); + let exec_request = ApprovalRequest::Exec { + thread_id: ThreadId::new(), + thread_label: None, + id: "test".into(), + command: vec!["cat".into(), "/tmp/readme.txt".into()], + reason: Some("need broader filesystem access".into()), + available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort], + network_approval_context: None, + additional_permissions: Some(PermissionProfile { + file_system: Some(FileSystemPermissions { + entries: vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: absolute_path("/tmp/secret.txt"), + }, + access: FileSystemAccessMode::None, + }, + ], + }), + ..Default::default() + }), + }; + + let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults()); + assert_snapshot!( + "approval_overlay_additional_permissions_special_entries_prompt", + normalize_snapshot_paths(render_overlay_lines(&view, /*width*/ 120)) + ); + } + #[test] fn permissions_prompt_snapshot() { let (tx, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_additional_permissions_special_entries_prompt.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_additional_permissions_special_entries_prompt.snap new file mode 100644 index 0000000000..0e868386cc --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_additional_permissions_special_entries_prompt.snap @@ -0,0 +1,16 @@ +--- +source: tui/src/bottom_pane/approval_overlay.rs +expression: "normalize_snapshot_paths(render_overlay_lines(&view, 120))" +--- + Would you like to run the following command? + + Reason: need broader filesystem access + + Permission rule: write `:root`; deny `/tmp/secret.txt` + + $ cat /tmp/readme.txt + +› 1. Yes, proceed (y) + 2. No, and tell Codex what to do differently (esc) + + Press enter to confirm or esc to cancel