From 8d1a4f0f67c61dc845941693919b31a9877068b6 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 23 Jun 2026 12:02:55 +0100 Subject: [PATCH 1/8] path-uri: add lexical containment --- codex-rs/utils/path-uri/src/lib.rs | 40 +++++++++++++++++++ codex-rs/utils/path-uri/src/tests.rs | 59 ++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/codex-rs/utils/path-uri/src/lib.rs b/codex-rs/utils/path-uri/src/lib.rs index 6c92c5e1df2f..9794e7c70940 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -247,6 +247,33 @@ impl PathUri { std::iter::successors(Some(self.clone()), Self::parent) } + /// Returns true when this URI is lexically equal to or below `base`. + /// + /// Containment is computed using URI authority and path-segment boundaries, + /// without consulting the host filesystem. Percent-encoded path separators + /// fail closed because native path conversion may interpret them as segment + /// boundaries. Opaque fallback URIs created by [`Self::from_abs_path`] only + /// contain themselves. + pub fn starts_with(&self, base: &Self) -> bool { + if self == base { + return true; + } + if decode_bad_path_uri(&self.0).is_some() || decode_bad_path_uri(&base.0).is_some() { + return false; + } + if self.0.host_str() != base.0.host_str() { + return false; + } + + let Some(path_segments) = containment_path_segments(&self.0) else { + return false; + }; + let Some(base_segments) = containment_path_segments(&base.0) else { + return false; + }; + path_segments.starts_with(&base_segments) + } + /// Lexically resolves native absolute or relative path text against this URI. /// /// Path text is interpreted using the POSIX or Windows convention inferred @@ -542,6 +569,19 @@ fn is_windows_drive_uri_segment(segment: &str) -> bool { matches!(segment.as_bytes(), [drive, b':'] if drive.is_ascii_alphabetic()) } +fn containment_path_segments(url: &Url) -> Option> { + let segments = url + .path_segments()? + .filter(|segment| !segment.is_empty()) + .collect::>(); + (!segments.iter().any(|segment| { + urlencoding::decode_binary(segment.as_bytes()) + .iter() + .any(|byte| matches!(*byte, b'/' | b'\\')) + })) + .then_some(segments) +} + fn infer_opaque_path_convention(path_bytes: &[u8]) -> Option { if path_bytes.starts_with(b"/") { return Some(PathConvention::Posix); diff --git a/codex-rs/utils/path-uri/src/tests.rs b/codex-rs/utils/path-uri/src/tests.rs index 0e45baaa122e..c52c3f9e6a16 100644 --- a/codex-rs/utils/path-uri/src/tests.rs +++ b/codex-rs/utils/path-uri/src/tests.rs @@ -283,9 +283,16 @@ fn structurally_valid_bad_path_uri_with_invalid_native_payload_fails_conversion( fn bad_path_uris_are_opaque_to_lexical_operations() { let uri = PathUri::parse("file:///%00/bad/path/YQ") .expect("canonical base64 fallback URI should parse"); + let other = PathUri::parse("file:///%00/bad/path/Yg") + .expect("canonical base64 fallback URI should parse"); + let root = PathUri::parse("file:///").expect("valid root URI"); assert_eq!(uri.basename(), None); assert_eq!(uri.parent(), None); + assert!(uri.starts_with(&uri)); + assert!(!uri.starts_with(&root)); + assert!(!uri.starts_with(&other)); + assert!(!other.starts_with(&uri)); assert_eq!(uri.join(""), Ok(uri.clone())); assert_eq!( uri.join("child"), @@ -698,6 +705,58 @@ fn join_uses_the_base_uri_path_convention() { } } +#[test] +fn starts_with_uses_uri_segment_boundaries() { + for (path, base, expected) in [ + ("file:///workspace/plugin", "file:///", true), + ("file:///workspace/plugin", "file:///workspace/plugin", true), + ( + "file:///workspace/plugin/assets/icon.svg", + "file:///workspace/plugin", + true, + ), + ( + "file:///workspace/plugin-other/icon.svg", + "file:///workspace/plugin", + false, + ), + ( + "file:///C:/plugins/foo/assets/icon.svg", + "file:///C:/plugins/foo", + true, + ), + ( + "file:///C:/plugins/foo2/assets/icon.svg", + "file:///C:/plugins/foo", + false, + ), + ( + "file://server/share/plugins/foo/icon.svg", + "file://server/share/plugins/foo", + true, + ), + ( + "file://other/share/plugins/foo/icon.svg", + "file://server/share/plugins/foo", + false, + ), + ( + "file:///workspace/plugin/%2F..%2Foutside", + "file:///workspace/plugin", + false, + ), + ( + "file:///workspace/plugin/%5C..%5Coutside", + "file:///workspace/plugin", + false, + ), + ] { + let path = PathUri::parse(path).expect("valid path URI"); + let base = PathUri::parse(base).expect("valid base URI"); + assert_eq!(path.starts_with(&base), expected); + } +} + #[test] fn to_url_returns_the_validated_url() { let uri = PathUri::parse("file://localhost/workspace/a%20file.rs").expect("valid file URI"); From 01b4840b95f46f810c900b6303c28b8cf52aa241 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 23 Jun 2026 12:58:07 +0100 Subject: [PATCH 2/8] Decouple plugin manifest path resolution --- codex-rs/core-plugins/src/manifest.rs | 173 +++++++++++++++++--------- 1 file changed, 116 insertions(+), 57 deletions(-) diff --git a/codex-rs/core-plugins/src/manifest.rs b/codex-rs/core-plugins/src/manifest.rs index 2352fdbefb03..f2f75fe88f16 100644 --- a/codex-rs/core-plugins/src/manifest.rs +++ b/codex-rs/core-plugins/src/manifest.rs @@ -141,6 +141,52 @@ pub(crate) fn parse_plugin_manifest( manifest_path: &Path, contents: &str, ) -> Result { + let resolver = HostManifestResourceResolver { + plugin_root, + manifest_path, + }; + parse_plugin_manifest_with_resolver(&resolver, contents) +} + +trait ManifestResourceResolver { + type Resource; + + fn plugin_name_fallback(&self) -> Option; + fn manifest_path_for_warning(&self) -> String; + fn resolve_path(&self, field: &'static str, path: Option<&str>) -> Option; +} + +struct HostManifestResourceResolver<'a> { + plugin_root: &'a Path, + manifest_path: &'a Path, +} + +impl ManifestResourceResolver for HostManifestResourceResolver<'_> { + type Resource = AbsolutePathBuf; + + fn plugin_name_fallback(&self) -> Option { + self.plugin_root + .file_name() + .and_then(|entry| entry.to_str()) + .map(str::to_string) + } + + fn manifest_path_for_warning(&self) -> String { + self.manifest_path.display().to_string() + } + + fn resolve_path(&self, field: &'static str, path: Option<&str>) -> Option { + resolve_manifest_path(self.plugin_root, field, path) + } +} + +fn parse_plugin_manifest_with_resolver( + resolver: &R, + contents: &str, +) -> Result, serde_json::Error> +where + R: ManifestResourceResolver, +{ let RawPluginManifest { name: raw_name, version, @@ -152,12 +198,11 @@ pub(crate) fn parse_plugin_manifest( hooks, interface, } = serde_json::from_str::(contents)?; - let name = plugin_root - .file_name() - .and_then(|entry| entry.to_str()) + let name = resolver + .plugin_name_fallback() .filter(|_| raw_name.trim().is_empty()) - .unwrap_or(&raw_name) - .to_string(); + .unwrap_or(raw_name); + let manifest_path_for_warning = resolver.manifest_path_for_warning(); let version = version.and_then(|version| { let version = version.trim(); (!version.is_empty()).then(|| version.to_string()) @@ -181,7 +226,7 @@ pub(crate) fn parse_plugin_manifest( screenshots, } = interface; - let interface = PluginManifestInterface { + let interface = codex_plugin::manifest::PluginManifestInterface { display_name, short_description, long_description, @@ -191,16 +236,19 @@ pub(crate) fn parse_plugin_manifest( website_url, privacy_policy_url, terms_of_service_url, - default_prompt: resolve_default_prompts(manifest_path, default_prompt.as_ref()), + default_prompt: resolve_default_prompts( + &manifest_path_for_warning, + default_prompt.as_ref(), + ), brand_color, composer_icon: resolve_interface_asset_path( - plugin_root, + resolver, "interface.composerIcon", composer_icon.as_deref(), ), - logo: resolve_interface_asset_path(plugin_root, "interface.logo", logo.as_deref()), + logo: resolve_interface_asset_path(resolver, "interface.logo", logo.as_deref()), logo_dark: resolve_interface_asset_path( - plugin_root, + resolver, "interface.logoDark", logo_dark.as_deref(), ), @@ -208,7 +256,7 @@ pub(crate) fn parse_plugin_manifest( .iter() .filter_map(|screenshot| { resolve_interface_asset_path( - plugin_root, + resolver, "interface.screenshots", Some(screenshot), ) @@ -234,41 +282,46 @@ pub(crate) fn parse_plugin_manifest( has_fields.then_some(interface) }); - Ok(PluginManifest { + Ok(codex_plugin::manifest::PluginManifest { name, version, description, keywords, - paths: PluginManifestPaths { - skills: resolve_manifest_paths(plugin_root, "skills", skills.as_ref()), - mcp_servers: resolve_manifest_mcp_servers(plugin_root, mcp_servers), - apps: resolve_manifest_path(plugin_root, "apps", apps.as_deref()), - hooks: resolve_manifest_hooks(plugin_root, hooks), + paths: codex_plugin::manifest::PluginManifestPaths { + skills: resolve_manifest_paths(resolver, "skills", skills.as_ref()), + mcp_servers: resolve_manifest_mcp_servers(resolver, mcp_servers), + apps: resolver.resolve_path("apps", apps.as_deref()), + hooks: resolve_manifest_hooks(resolver, hooks), }, interface, }) } -fn resolve_manifest_hooks( - plugin_root: &Path, +fn resolve_manifest_hooks( + resolver: &R, hooks: Option, -) -> Option { +) -> Option> +where + R: ManifestResourceResolver, +{ match hooks? { - RawPluginManifestHooks::Path(path) => { - resolve_manifest_path(plugin_root, "hooks", Some(&path)) - .map(|path| PluginManifestHooks::Paths(vec![path])) - } + RawPluginManifestHooks::Path(path) => resolver + .resolve_path("hooks", Some(&path)) + .map(|path| codex_plugin::manifest::PluginManifestHooks::Paths(vec![path])), RawPluginManifestHooks::Paths(paths) => { let hooks = paths .iter() - .filter_map(|path| resolve_manifest_path(plugin_root, "hooks", Some(path))) + .filter_map(|path| resolver.resolve_path("hooks", Some(path))) .collect::>(); - (!hooks.is_empty()).then_some(PluginManifestHooks::Paths(hooks)) + (!hooks.is_empty()).then_some(codex_plugin::manifest::PluginManifestHooks::Paths(hooks)) } - RawPluginManifestHooks::Inline(hooks) => Some(PluginManifestHooks::Inline(vec![hooks])), - RawPluginManifestHooks::InlineList(hooks) => { - (!hooks.is_empty()).then_some(PluginManifestHooks::Inline(hooks)) + RawPluginManifestHooks::Inline(hooks) => { + Some(codex_plugin::manifest::PluginManifestHooks::Inline(vec![ + hooks, + ])) } + RawPluginManifestHooks::InlineList(hooks) => (!hooks.is_empty()) + .then_some(codex_plugin::manifest::PluginManifestHooks::Inline(hooks)), RawPluginManifestHooks::Invalid(value) => { tracing::warn!( "ignoring hooks: expected a string, string array, object, or object array; found {}", @@ -279,17 +332,21 @@ fn resolve_manifest_hooks( } } -fn resolve_manifest_mcp_servers( - plugin_root: &Path, +fn resolve_manifest_mcp_servers( + resolver: &R, mcp_servers: Option, -) -> Option { +) -> Option> +where + R: ManifestResourceResolver, +{ match mcp_servers? { - RawPluginManifestMcpServers::Path(path) => { - resolve_manifest_path(plugin_root, "mcpServers", Some(&path)) - .map(PluginManifestMcpServers::Path) - } + RawPluginManifestMcpServers::Path(path) => resolver + .resolve_path("mcpServers", Some(&path)) + .map(codex_plugin::manifest::PluginManifestMcpServers::Path), RawPluginManifestMcpServers::Object(servers) => match serde_json::to_string(&servers) { - Ok(servers) => Some(PluginManifestMcpServers::Object(servers)), + Ok(servers) => Some(codex_plugin::manifest::PluginManifestMcpServers::Object( + servers, + )), Err(err) => { tracing::warn!("ignoring mcpServers: failed to serialize object: {err}"); None @@ -305,16 +362,19 @@ fn resolve_manifest_mcp_servers( } } -fn resolve_interface_asset_path( - plugin_root: &Path, +fn resolve_interface_asset_path( + resolver: &R, field: &'static str, path: Option<&str>, -) -> Option { - resolve_manifest_path(plugin_root, field, path) +) -> Option +where + R: ManifestResourceResolver, +{ + resolver.resolve_path(field, path) } fn resolve_default_prompts( - manifest_path: &Path, + manifest_path: &str, value: Option<&RawPluginManifestDefaultPrompt>, ) -> Option> { match value? { @@ -370,7 +430,7 @@ fn resolve_default_prompts( } } -fn resolve_default_prompt_str(manifest_path: &Path, field: &str, prompt: &str) -> Option { +fn resolve_default_prompt_str(manifest_path: &str, field: &str, prompt: &str) -> Option { let prompt = prompt.split_whitespace().collect::>().join(" "); if prompt.is_empty() { warn_invalid_default_prompt(manifest_path, field, "prompt must not be empty"); @@ -387,11 +447,8 @@ fn resolve_default_prompt_str(manifest_path: &Path, field: &str, prompt: &str) - Some(prompt) } -fn warn_invalid_default_prompt(manifest_path: &Path, field: &str, message: &str) { - tracing::warn!( - path = %manifest_path.display(), - "ignoring {field}: {message}" - ); +fn warn_invalid_default_prompt(manifest_path: &str, field: &str, message: &str) { + tracing::warn!(path = manifest_path, "ignoring {field}: {message}"); } fn json_value_type(value: &JsonValue) -> &'static str { @@ -405,20 +462,22 @@ fn json_value_type(value: &JsonValue) -> &'static str { } } -fn resolve_manifest_paths( - plugin_root: &Path, +fn resolve_manifest_paths( + resolver: &R, field: &'static str, paths: Option<&RawPluginManifestPaths>, -) -> Vec { +) -> Vec +where + R: ManifestResourceResolver, +{ match paths { - Some(RawPluginManifestPaths::Path(path)) => { - resolve_manifest_path(plugin_root, field, Some(path)) - .map(|path| vec![path]) - .unwrap_or_default() - } + Some(RawPluginManifestPaths::Path(path)) => resolver + .resolve_path(field, Some(path)) + .map(|path| vec![path]) + .unwrap_or_default(), Some(RawPluginManifestPaths::Paths(paths)) => paths .iter() - .filter_map(|path| resolve_manifest_path(plugin_root, field, Some(path))) + .filter_map(|path| resolver.resolve_path(field, Some(path))) .collect(), Some(RawPluginManifestPaths::Invalid(value)) => { tracing::warn!( From 25a3f8dea2978f7118a91520f5edc377b2b159bc Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 23 Jun 2026 13:17:39 +0100 Subject: [PATCH 3/8] Make selected plugin roots URI-native --- codex-rs/Cargo.lock | 1 + .../schema/json/ClientRequest.json | 11 +- .../codex_app_server_protocol.schemas.json | 11 +- .../codex_app_server_protocol.v2.schemas.json | 11 +- .../schema/json/v2/ThreadStartParams.json | 11 +- .../typescript/v2/CapabilityRootLocation.ts | 6 +- codex-rs/app-server/README.md | 5 +- .../app-server/tests/suite/v2/executor_mcp.rs | 3 +- .../tests/suite/v2/executor_skills.rs | 3 +- codex-rs/core-plugins/src/manifest.rs | 226 +++++++++++++++++- codex-rs/core-plugins/src/provider.rs | 82 +++---- codex-rs/core-plugins/src/provider_tests.rs | 120 ++++++---- codex-rs/core/src/thread_manager_tests.rs | 2 +- .../ext/mcp/src/executor_plugin/provider.rs | 25 +- .../mcp/src/executor_plugin/provider_tests.rs | 30 ++- codex-rs/ext/mcp/tests/executor_plugin_mcp.rs | 11 +- codex-rs/ext/skills/src/provider/executor.rs | 12 +- .../tests/executor_file_system_authority.rs | 3 +- codex-rs/ext/skills/tests/skills_extension.rs | 5 +- codex-rs/plugin/Cargo.toml | 1 + codex-rs/plugin/src/provider.rs | 27 +-- codex-rs/plugin/src/provider_tests.rs | 54 +++-- codex-rs/protocol/src/capabilities.rs | 19 +- codex-rs/protocol/src/capabilities_tests.rs | 50 ++++ codex-rs/utils/path-uri/src/lib.rs | 24 +- codex-rs/utils/path-uri/src/tests.rs | 5 + 26 files changed, 562 insertions(+), 196 deletions(-) create mode 100644 codex-rs/protocol/src/capabilities_tests.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 65d581db6f7a..d7b1002ab7a1 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3605,6 +3605,7 @@ dependencies = [ "codex-config", "codex-protocol", "codex-utils-absolute-path", + "codex-utils-path-uri", "codex-utils-plugins", "pretty_assertions", "thiserror 2.0.18", diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 28597eff6254..e74b544e56c7 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -210,7 +210,13 @@ "type": "string" }, "path": { - "type": "string" + "allOf": [ + { + "$ref": "#/definitions/PathUri" + } + ], + "description": "Canonical `file:` URI for the root in the selected environment.", + "pattern": "^file:" }, "type": { "enum": [ @@ -1788,6 +1794,9 @@ ], "type": "string" }, + "PathUri": { + "type": "string" + }, "PermissionProfileListParams": { "properties": { "cursor": { 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 597fd5a1b820..678a8af88957 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 @@ -7044,7 +7044,13 @@ "type": "string" }, "path": { - "type": "string" + "allOf": [ + { + "$ref": "#/definitions/v2/PathUri" + } + ], + "description": "Canonical `file:` URI for the root in the selected environment.", + "pattern": "^file:" }, "type": { "enum": [ @@ -13000,6 +13006,9 @@ } ] }, + "PathUri": { + "type": "string" + }, "PermissionProfileListParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 3af48cc6c075..e6c066772e95 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -1170,7 +1170,13 @@ "type": "string" }, "path": { - "type": "string" + "allOf": [ + { + "$ref": "#/definitions/PathUri" + } + ], + "description": "Canonical `file:` URI for the root in the selected environment.", + "pattern": "^file:" }, "type": { "enum": [ @@ -9404,6 +9410,9 @@ } ] }, + "PathUri": { + "type": "string" + }, "PermissionProfileListParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json index 9932456f1b55..f3a467d4d7ed 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json @@ -74,7 +74,13 @@ "type": "string" }, "path": { - "type": "string" + "allOf": [ + { + "$ref": "#/definitions/PathUri" + } + ], + "description": "Canonical `file:` URI for the root in the selected environment.", + "pattern": "^file:" }, "type": { "enum": [ @@ -203,6 +209,9 @@ ], "type": "string" }, + "PathUri": { + "type": "string" + }, "Personality": { "enum": [ "none", diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CapabilityRootLocation.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CapabilityRootLocation.ts index 6266101eb846..f2bccf3a37d3 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CapabilityRootLocation.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CapabilityRootLocation.ts @@ -5,4 +5,8 @@ /** * Location used to resolve a selected capability root. */ -export type CapabilityRootLocation = { "type": "environment", environmentId: string, path: string, }; +export type CapabilityRootLocation = { "type": "environment", environmentId: string, +/** + * Canonical `file:` URI for the root in the selected environment. + */ +path: string, }; diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 50f00520949f..53428aa57d7b 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -137,7 +137,7 @@ Example with notification opt-out: ## API Overview -- `thread/start` — create a new thread; emits `thread/started` (including the current `thread.status`) and auto-subscribes you to turn/item events for that thread. When the request includes a `cwd` and the resolved sandbox is `workspace-write` or full access, app-server also marks that project as trusted in the user `config.toml`. Pass `sessionStartSource: "clear"` when starting a replacement thread after clearing the current session so `SessionStart` hooks receive `source: "clear"` instead of the default `"startup"`. Experimental `runtimeWorkspaceRoots` replaces the thread-scoped runtime workspace roots used to materialize `:workspace_roots`; paths must be absolute. For permissions, prefer experimental `permissions` profile selection by id; the legacy `sandbox` shorthand is still accepted but cannot be combined with `permissions`. Experimental `multiAgentMode` selects the initial thread mode and defaults to `explicitRequestOnly` when omitted; use `none` to keep multi-agent tools available without injecting mode instructions. Experimental `environments` selects the sticky execution environments for turns on the thread; omit it to use the server default, pass `[]` to disable environments, or pass explicit environment ids with per-environment `cwd`. Experimental `selectedCapabilityRoots` selects environment-owned plugin or standalone-skill roots. Skills found below those roots are listed and read through the owning environment. Stdio MCP servers declared by selected plugins are also started in that environment; HTTP MCP declarations remain inactive. +- `thread/start` — create a new thread; emits `thread/started` (including the current `thread.status`) and auto-subscribes you to turn/item events for that thread. When the request includes a `cwd` and the resolved sandbox is `workspace-write` or full access, app-server also marks that project as trusted in the user `config.toml`. Pass `sessionStartSource: "clear"` when starting a replacement thread after clearing the current session so `SessionStart` hooks receive `source: "clear"` instead of the default `"startup"`. Experimental `runtimeWorkspaceRoots` replaces the thread-scoped runtime workspace roots used to materialize `:workspace_roots`; paths must be absolute. For permissions, prefer experimental `permissions` profile selection by id; the legacy `sandbox` shorthand is still accepted but cannot be combined with `permissions`. Experimental `multiAgentMode` selects the initial thread mode and defaults to `explicitRequestOnly` when omitted; use `none` to keep multi-agent tools available without injecting mode instructions. Experimental `environments` selects the sticky execution environments for turns on the thread; omit it to use the server default, pass `[]` to disable environments, or pass explicit environment ids with per-environment `cwd`. Experimental `selectedCapabilityRoots` selects environment-owned plugin or standalone-skill roots using `file:` URIs. Skills found below those roots are listed and read through the owning environment. Stdio MCP servers declared by selected plugins are also started in that environment; HTTP MCP declarations remain inactive. - `thread/resume` — reopen an existing thread by id so subsequent `turn/start` calls append to it. Accepts the same permission override rules as `thread/start`. Multi-agent mode restores the last effective mode from rollout history when available; clients can select another mode on the first `turn/start`. - `thread/fork` — fork an existing thread into a new thread id by copying the stored history; if the source thread is currently mid-turn, the fork records the same interruption marker as `turn/interrupt` instead of inheriting an unmarked partial turn suffix. The returned `thread.forkedFromId` points at the source thread when known. Accepts `ephemeral: true` for an in-memory temporary fork, emits `thread/started` (including the current `thread.status`), and auto-subscribes you to turn/item events for the new thread. Experimental clients can pass `excludeTurns: true` when they plan to page fork history via `thread/turns/list` instead of receiving the full turn array immediately. Accepts the same permission override rules as `thread/start`. - `thread/start`, `thread/resume`, and `thread/fork` responses include the legacy `sandbox` compatibility projection. `instructionSources` lists loaded instruction files using each source environment's native absolute path syntax, including files loaded from remote environments. Experimental clients can read `runtimeWorkspaceRoots` for the thread-scoped runtime roots and `activePermissionProfile` for the named or implicit built-in profile identity/provenance when known. Their experimental `multiAgentMode` field, and the corresponding thread setting, report the thread's current mode. Turn construction separately determines whether that mode is applicable to the selected model and runtime configuration. @@ -267,8 +267,7 @@ Start a fresh thread when you need a new Codex conversation. "location": { "type": "environment", "environmentId": "workspace", - // Opaque to app-server; interpreted in the selected environment. - "path": "/opt/cca/plugins/github" + "path": "file:///opt/cca/plugins/github" } } ], diff --git a/codex-rs/app-server/tests/suite/v2/executor_mcp.rs b/codex-rs/app-server/tests/suite/v2/executor_mcp.rs index 0eb22a395c4a..299ca81aa5ee 100644 --- a/codex-rs/app-server/tests/suite/v2/executor_mcp.rs +++ b/codex-rs/app-server/tests/suite/v2/executor_mcp.rs @@ -13,6 +13,7 @@ use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::UserInput; +use codex_utils_path_uri::PathUri; use core_test_support::responses; use core_test_support::stdio_server_bin; use pretty_assertions::assert_eq; @@ -92,7 +93,7 @@ args = ["exec-server", "--listen", "stdio"] id: "executor-demo@1".to_string(), location: CapabilityRootLocation::Environment { environment_id: EXECUTOR_ID.to_string(), - path: plugin.path().to_string_lossy().into_owned(), + path: PathUri::from_host_native_path(plugin.path())?, }, }]), ) diff --git a/codex-rs/app-server/tests/suite/v2/executor_skills.rs b/codex-rs/app-server/tests/suite/v2/executor_skills.rs index ede5b20fb8e1..a15e285d31ad 100644 --- a/codex-rs/app-server/tests/suite/v2/executor_skills.rs +++ b/codex-rs/app-server/tests/suite/v2/executor_skills.rs @@ -11,6 +11,7 @@ use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::UserInput; +use codex_utils_path_uri::PathUri; use core_test_support::responses; use tempfile::TempDir; use tokio::time::timeout; @@ -90,7 +91,7 @@ stream_max_retries = 0 id: "demo-plugin@1".to_string(), location: CapabilityRootLocation::Environment { environment_id: "local".to_string(), - path: plugin_dir.path().to_string_lossy().into_owned(), + path: PathUri::from_host_native_path(plugin_dir.path())?, }, }]), ..Default::default() diff --git a/codex-rs/core-plugins/src/manifest.rs b/codex-rs/core-plugins/src/manifest.rs index f2f75fe88f16..2a7c51c3e6de 100644 --- a/codex-rs/core-plugins/src/manifest.rs +++ b/codex-rs/core-plugins/src/manifest.rs @@ -1,5 +1,7 @@ use codex_config::HooksFile; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathConvention; +use codex_utils_path_uri::PathUri; use codex_utils_plugins::find_plugin_manifest_path; use serde::Deserialize; use serde_json::Value as JsonValue; @@ -16,6 +18,8 @@ pub type PluginManifestMcpServers = codex_plugin::manifest::PluginManifestMcpServers; pub type PluginManifestPaths = codex_plugin::manifest::PluginManifestPaths; +pub(crate) type UriPluginManifest = codex_plugin::manifest::PluginManifest; + #[derive(Debug, Default, Deserialize)] #[serde(rename_all = "camelCase")] struct RawPluginManifest { @@ -148,6 +152,18 @@ pub(crate) fn parse_plugin_manifest( parse_plugin_manifest_with_resolver(&resolver, contents) } +pub(crate) fn parse_plugin_manifest_uri( + plugin_root: &PathUri, + manifest_path: &PathUri, + contents: &str, +) -> Result { + let resolver = UriManifestResourceResolver { + plugin_root, + manifest_path, + }; + parse_plugin_manifest_with_resolver(&resolver, contents) +} + trait ManifestResourceResolver { type Resource; @@ -180,6 +196,27 @@ impl ManifestResourceResolver for HostManifestResourceResolver<'_> { } } +struct UriManifestResourceResolver<'a> { + plugin_root: &'a PathUri, + manifest_path: &'a PathUri, +} + +impl ManifestResourceResolver for UriManifestResourceResolver<'_> { + type Resource = PathUri; + + fn plugin_name_fallback(&self) -> Option { + self.plugin_root.basename() + } + + fn manifest_path_for_warning(&self) -> String { + self.manifest_path.to_string() + } + + fn resolve_path(&self, field: &'static str, path: Option<&str>) -> Option { + resolve_manifest_path_uri(self.plugin_root, field, path) + } +} + fn parse_plugin_manifest_with_resolver( resolver: &R, contents: &str, @@ -533,6 +570,60 @@ fn resolve_manifest_path( .ok() } +fn resolve_manifest_path_uri( + plugin_root: &PathUri, + field: &'static str, + path: Option<&str>, +) -> Option { + let path = path?; + if path.is_empty() { + return None; + } + let Some(relative_path) = path.strip_prefix("./") else { + tracing::warn!("ignoring {field}: path must start with `./` relative to plugin root"); + return None; + }; + if relative_path.is_empty() { + tracing::warn!("ignoring {field}: path must not be `./`"); + return None; + } + + let convention = plugin_root.infer_path_convention(); + let has_parent_component = match convention { + Some(PathConvention::Windows) => relative_path + .split(['/', '\\']) + .any(|component| component == ".."), + Some(PathConvention::Posix) | None => { + relative_path.split('/').any(|component| component == "..") + } + }; + if has_parent_component { + tracing::warn!("ignoring {field}: path must not contain '..'"); + return None; + } + + let has_windows_root = convention == Some(PathConvention::Windows) + && (relative_path.starts_with('\\') + || matches!(relative_path.as_bytes(), [drive, b':', ..] if drive.is_ascii_alphabetic())); + if relative_path.starts_with('/') || has_windows_root { + tracing::warn!("ignoring {field}: path must stay within the plugin root"); + return None; + } + + let resolved = match plugin_root.join(relative_path) { + Ok(resolved) => resolved, + Err(err) => { + tracing::warn!("ignoring {field}: path must resolve under plugin root: {err}"); + return None; + } + }; + if !resolved.starts_with(plugin_root) { + tracing::warn!("ignoring {field}: path must stay within the plugin root"); + return None; + } + Some(resolved) +} + #[cfg(test)] mod tests { use super::MAX_DEFAULT_PROMPT_LEN; @@ -542,9 +633,15 @@ mod tests { use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_plugin::PluginProvider; use codex_plugin::ResolvedPlugin; + use codex_plugin::manifest::PluginManifest as GenericPluginManifest; + use codex_plugin::manifest::PluginManifestHooks; + use codex_plugin::manifest::PluginManifestInterface; + use codex_plugin::manifest::PluginManifestMcpServers; + use codex_plugin::manifest::PluginManifestPaths; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_utils_absolute_path::AbsolutePathBuf; + use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use std::fs; use std::path::Path; @@ -766,14 +863,16 @@ mod tests { "composerIcon": "./assets/icon.svg" }"#, ); - let host_manifest = load_plugin_manifest(&plugin_root).expect("host manifest"); + let plugin_root = + AbsolutePathBuf::from_absolute_path_checked(plugin_root).expect("absolute plugin root"); + let plugin_root_uri = PathUri::from_abs_path(&plugin_root); let provider = ExecutorPluginProvider::new(Arc::new(EnvironmentManager::default_for_tests())); let selected_root = SelectedCapabilityRoot { id: "selected-demo".to_string(), location: CapabilityRootLocation::Environment { environment_id: LOCAL_ENVIRONMENT_ID.to_string(), - path: plugin_root.to_string_lossy().into_owned(), + path: plugin_root_uri.clone(), }, }; @@ -782,17 +881,130 @@ mod tests { .await .expect("resolve executor plugin") .expect("plugin descriptor"); - let plugin_root = - AbsolutePathBuf::from_absolute_path_checked(plugin_root).expect("absolute plugin root"); + let manifest_path = plugin_root_uri + .join(".codex-plugin/plugin.json") + .expect("manifest URI"); + let manifest_contents = + fs::read_to_string(plugin_root.join(".codex-plugin/plugin.json")).expect("manifest"); + let expected_manifest = + super::parse_plugin_manifest_uri(&plugin_root_uri, &manifest_path, &manifest_contents) + .expect("URI manifest"); let expected_plugin = ResolvedPlugin::from_environment( "selected-demo".to_string(), LOCAL_ENVIRONMENT_ID.to_string(), - plugin_root.clone(), - plugin_root.join(".codex-plugin/plugin.json"), - host_manifest, + plugin_root_uri, + manifest_path, + expected_manifest, ) .expect("valid expected descriptor"); assert_eq!(executor_plugin, expected_plugin); } + + #[test] + fn uri_manifest_resolves_resources_below_foreign_root() { + let plugin_root = + PathUri::parse("file:///C:/plugins/demo-plugin").expect("plugin root URI"); + let manifest_path = plugin_root + .join(".codex-plugin/plugin.json") + .expect("manifest URI"); + let manifest = super::parse_plugin_manifest_uri( + &plugin_root, + &manifest_path, + r#"{ + "name": "demo-plugin", + "skills": "./skills", + "mcpServers": "./.mcp.json", + "apps": "./apps", + "hooks": "./hooks.json", + "interface": { + "displayName": "Demo Plugin", + "composerIcon": "./assets/icon.svg" + } +}"#, + ) + .expect("URI manifest"); + + assert_eq!( + manifest, + GenericPluginManifest { + name: "demo-plugin".to_string(), + version: None, + description: None, + keywords: Vec::new(), + paths: PluginManifestPaths { + skills: vec![plugin_root.join("skills").expect("skills URI")], + mcp_servers: Some(PluginManifestMcpServers::Path( + plugin_root.join(".mcp.json").expect("MCP URI"), + )), + apps: Some(plugin_root.join("apps").expect("apps URI")), + hooks: Some(PluginManifestHooks::Paths(vec![ + plugin_root.join("hooks.json").expect("hooks URI"), + ])), + }, + interface: Some(PluginManifestInterface { + display_name: Some("Demo Plugin".to_string()), + composer_icon: Some( + plugin_root + .join("assets/icon.svg") + .expect("composer icon URI"), + ), + ..PluginManifestInterface::default() + }), + } + ); + } + + #[test] + fn uri_manifest_validates_paths_using_the_root_convention() { + let windows_root = + PathUri::parse("file:///C:/plugins/demo-plugin").expect("Windows plugin root URI"); + let posix_root = + PathUri::parse("file:///plugins/demo-plugin").expect("POSIX plugin root URI"); + + for composer_icon in [ + "./assets/../icon.svg", + r"./assets\..\icon.svg", + ".//outside.svg", + "./C:/outside.svg", + ] { + assert_eq!( + parse_uri_composer_icon(&windows_root, composer_icon), + None, + "parsing {composer_icon} under a Windows root" + ); + } + for (composer_icon, relative_path) in [ + (r"./assets\..\icon.svg", r"assets\..\icon.svg"), + (r"./\icon.svg", r"\icon.svg"), + ("./C:/icon.svg", "C:/icon.svg"), + ] { + assert_eq!( + parse_uri_composer_icon(&posix_root, composer_icon), + Some(posix_root.join(relative_path).expect("composer icon URI")), + "parsing {composer_icon} under a POSIX root" + ); + } + } + + fn parse_uri_composer_icon(plugin_root: &PathUri, composer_icon: &str) -> Option { + let manifest_path = plugin_root + .join(".codex-plugin/plugin.json") + .expect("manifest URI"); + let composer_icon_json = + serde_json::to_string(composer_icon).expect("serialize composer icon"); + let contents = format!( + r#"{{ + "name": "demo-plugin", + "interface": {{ + "displayName": "Demo Plugin", + "composerIcon": {composer_icon_json} + }} +}}"# + ); + super::parse_plugin_manifest_uri(plugin_root, &manifest_path, &contents) + .expect("URI manifest") + .interface + .and_then(|interface| interface.composer_icon) + } } diff --git a/codex-rs/core-plugins/src/provider.rs b/codex-rs/core-plugins/src/provider.rs index 1a42ad42b6ed..5a371b3d2eae 100644 --- a/codex-rs/core-plugins/src/provider.rs +++ b/codex-rs/core-plugins/src/provider.rs @@ -1,4 +1,4 @@ -use crate::manifest::parse_plugin_manifest; +use crate::manifest::parse_plugin_manifest_uri; use codex_exec_server::EnvironmentManager; use codex_exec_server::ExecutorFileSystem; use codex_plugin::PluginProvider; @@ -6,23 +6,16 @@ use codex_plugin::ResolvedPlugin; use codex_plugin::ResolvedPluginError; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; -use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; +use codex_utils_path_uri::PathUriParseError; use codex_utils_plugins::DISCOVERABLE_PLUGIN_MANIFEST_PATHS; use std::io; -use std::path::PathBuf; use std::sync::Arc; use thiserror::Error; /// Failure to resolve an environment-owned capability root as a plugin package. #[derive(Debug, Error)] pub enum ExecutorPluginProviderError { - #[error("selected capability root `{root_id}` has invalid path `{path}`: {message}")] - InvalidRootPath { - root_id: String, - path: String, - message: String, - }, #[error( "selected capability root `{root_id}` references unavailable environment `{environment_id}`" )] @@ -33,33 +26,40 @@ pub enum ExecutorPluginProviderError { #[error("failed to inspect selected capability root `{root_id}` at {path}: {source}")] InspectRoot { root_id: String, - path: AbsolutePathBuf, + path: PathUri, #[source] source: io::Error, }, #[error("selected capability root `{root_id}` path {path} is not a directory")] - RootNotDirectory { + RootNotDirectory { root_id: String, path: PathUri }, + #[error( + "failed to resolve plugin manifest path `{relative_path}` below selected capability root `{root_id}` at {root}: {source}" + )] + InvalidManifestPath { root_id: String, - path: AbsolutePathBuf, + root: PathUri, + relative_path: &'static str, + #[source] + source: PathUriParseError, }, #[error("failed to inspect plugin manifest for `{root_id}` at {path}: {source}")] InspectManifest { root_id: String, - path: AbsolutePathBuf, + path: PathUri, #[source] source: io::Error, }, #[error("failed to read plugin manifest for `{root_id}` at {path}: {source}")] ReadManifest { root_id: String, - path: AbsolutePathBuf, + path: PathUri, #[source] source: io::Error, }, #[error("failed to parse plugin manifest for `{root_id}` at {path}: {source}")] ParseManifest { root_id: String, - path: AbsolutePathBuf, + path: PathUri, #[source] source: serde_json::Error, }, @@ -110,7 +110,7 @@ impl ExecutorPluginProvider { selected_root: &SelectedCapabilityRoot, ) -> Result, ExecutorPluginProviderError> { let root_id = &selected_root.id; - let plugin_root = selected_plugin_root(selected_root)?; + let plugin_root = selected_plugin_root(selected_root); let CapabilityRootLocation::Environment { environment_id, .. } = &selected_root.location; let environment = self .environment_manager @@ -142,38 +142,20 @@ impl PluginProvider for ExecutorPluginProvider { } } -fn selected_plugin_root( - selected_root: &SelectedCapabilityRoot, -) -> Result { - let root_id = &selected_root.id; +fn selected_plugin_root(selected_root: &SelectedCapabilityRoot) -> PathUri { let CapabilityRootLocation::Environment { path, .. } = &selected_root.location; - let plugin_root = PathBuf::from(path); - if !plugin_root.is_absolute() { - return Err(ExecutorPluginProviderError::InvalidRootPath { - root_id: root_id.clone(), - path: path.clone(), - message: "executor path must be absolute".to_string(), - }); - } - AbsolutePathBuf::from_absolute_path_checked(plugin_root).map_err(|err| { - ExecutorPluginProviderError::InvalidRootPath { - root_id: root_id.clone(), - path: path.clone(), - message: err.to_string(), - } - }) + path.clone() } async fn resolve_plugin_root( selected_root: &SelectedCapabilityRoot, - plugin_root: AbsolutePathBuf, + plugin_root: PathUri, file_system: &dyn ExecutorFileSystem, ) -> Result, ExecutorPluginProviderError> { let root_id = &selected_root.id; let CapabilityRootLocation::Environment { environment_id, .. } = &selected_root.location; - let root_uri = PathUri::from_abs_path(&plugin_root); let root_metadata = file_system - .get_metadata(&root_uri, /*sandbox*/ None) + .get_metadata(&plugin_root, /*sandbox*/ None) .await .map_err(|source| ExecutorPluginProviderError::InspectRoot { root_id: root_id.clone(), @@ -189,14 +171,20 @@ async fn resolve_plugin_root( let mut manifest_path = None; for relative_path in DISCOVERABLE_PLUGIN_MANIFEST_PATHS { - let candidate = plugin_root.join(relative_path); - let candidate_uri = PathUri::from_abs_path(&candidate); + let candidate_uri = plugin_root.join(relative_path).map_err(|source| { + ExecutorPluginProviderError::InvalidManifestPath { + root_id: root_id.clone(), + root: plugin_root.clone(), + relative_path, + source, + } + })?; match file_system .get_metadata(&candidate_uri, /*sandbox*/ None) .await { Ok(metadata) if metadata.is_file => { - manifest_path = Some((candidate, candidate_uri)); + manifest_path = Some(candidate_uri); break; } Ok(_) => {} @@ -204,13 +192,13 @@ async fn resolve_plugin_root( Err(source) => { return Err(ExecutorPluginProviderError::InspectManifest { root_id: root_id.clone(), - path: candidate, + path: candidate_uri, source, }); } } } - let Some((manifest_path, manifest_uri)) = manifest_path else { + let Some(manifest_uri) = manifest_path else { return Ok(None); }; let contents = file_system @@ -218,14 +206,14 @@ async fn resolve_plugin_root( .await .map_err(|source| ExecutorPluginProviderError::ReadManifest { root_id: root_id.clone(), - path: manifest_path.clone(), + path: manifest_uri.clone(), source, })?; let manifest = - parse_plugin_manifest(&plugin_root, &manifest_path, &contents).map_err(|source| { + parse_plugin_manifest_uri(&plugin_root, &manifest_uri, &contents).map_err(|source| { ExecutorPluginProviderError::ParseManifest { root_id: root_id.clone(), - path: manifest_path.clone(), + path: manifest_uri.clone(), source, } })?; @@ -234,7 +222,7 @@ async fn resolve_plugin_root( root_id.clone(), environment_id.clone(), plugin_root, - manifest_path, + manifest_uri, manifest, ) .map_err(|source| ExecutorPluginProviderError::ConstructDescriptor { diff --git a/codex-rs/core-plugins/src/provider_tests.rs b/codex-rs/core-plugins/src/provider_tests.rs index 3f92a47f0b9f..e35a094940b9 100644 --- a/codex-rs/core-plugins/src/provider_tests.rs +++ b/codex-rs/core-plugins/src/provider_tests.rs @@ -1,7 +1,7 @@ use super::ExecutorPluginProvider; use super::ExecutorPluginProviderError; use super::resolve_plugin_root; -use crate::manifest::parse_plugin_manifest; +use crate::manifest::parse_plugin_manifest_uri; use codex_exec_server::CopyOptions; use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::EnvironmentManager; @@ -18,7 +18,6 @@ use codex_plugin::PluginProvider; use codex_plugin::ResolvedPlugin; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; -use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use std::fs; @@ -43,13 +42,13 @@ const MANIFEST_CONTENTS: &str = r#"{ #[derive(Debug, PartialEq, Eq)] enum FileSystemCall { - Metadata(AbsolutePathBuf), - Read(AbsolutePathBuf), + Metadata(PathUri), + Read(PathUri), } struct SyntheticPluginFileSystem { - plugin_root: AbsolutePathBuf, - manifest_path: AbsolutePathBuf, + plugin_root: PathUri, + manifest_path: PathUri, calls: Mutex>, } @@ -77,12 +76,11 @@ impl ExecutorFileSystem for SyntheticPluginFileSystem { _sandbox: Option<&'a FileSystemSandboxContext>, ) -> ExecutorFileSystemFuture<'a, Vec> { Box::pin(async move { - let path = path.to_abs_path()?; self.calls .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) .push(FileSystemCall::Read(path.clone())); - if path == self.manifest_path { + if path == &self.manifest_path { Ok(MANIFEST_CONTENTS.as_bytes().to_vec()) } else { Err(io::Error::new(io::ErrorKind::NotFound, "not found")) @@ -122,14 +120,13 @@ impl ExecutorFileSystem for SyntheticPluginFileSystem { _sandbox: Option<&'a FileSystemSandboxContext>, ) -> ExecutorFileSystemFuture<'a, FileMetadata> { Box::pin(async move { - let path = path.to_abs_path()?; self.calls .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) .push(FileSystemCall::Metadata(path.clone())); - let (is_directory, is_file) = if path == self.plugin_root { + let (is_directory, is_file) = if path == &self.plugin_root { (true, false) - } else if path == self.manifest_path { + } else if path == &self.manifest_path { (false, true) } else { return Err(io::Error::new(io::ErrorKind::NotFound, "not found")); @@ -185,7 +182,17 @@ fn selected_root(id: &str, environment_id: &str, path: &Path) -> SelectedCapabil id: id.to_string(), location: CapabilityRootLocation::Environment { environment_id: environment_id.to_string(), - path: path.to_string_lossy().into_owned(), + path: PathUri::from_host_native_path(path).expect("path URI"), + }, + } +} + +fn selected_root_uri(id: &str, environment_id: &str, path: PathUri) -> SelectedCapabilityRoot { + SelectedCapabilityRoot { + id: id.to_string(), + location: CapabilityRootLocation::Environment { + environment_id: environment_id.to_string(), + path, }, } } @@ -195,22 +202,20 @@ async fn plugin_root_resolution_uses_supplied_executor_file_system() { let temp_dir = tempdir().expect("tempdir"); let plugin_root = temp_dir.path().join("executor-only-plugin"); assert!(!plugin_root.exists()); - let plugin_root = - AbsolutePathBuf::from_absolute_path_checked(plugin_root).expect("absolute plugin root"); - let manifest_path = plugin_root.join(".codex-plugin/plugin.json"); - let parsed_manifest = parse_plugin_manifest( - plugin_root.as_path(), - manifest_path.as_path(), - MANIFEST_CONTENTS, - ) - .expect("parse manifest"); + let plugin_root = PathUri::from_host_native_path(&plugin_root).expect("plugin root URI"); + let manifest_path = plugin_root + .join(".codex-plugin/plugin.json") + .expect("manifest URI"); + let parsed_manifest = + parse_plugin_manifest_uri(&plugin_root, &manifest_path, MANIFEST_CONTENTS) + .expect("parse manifest"); let file_system = SyntheticPluginFileSystem { plugin_root: plugin_root.clone(), manifest_path: manifest_path.clone(), calls: Mutex::new(Vec::new()), }; let resolved = resolve_plugin_root( - &selected_root("selected-demo", "executor-test", plugin_root.as_path()), + &selected_root_uri("selected-demo", "executor-test", plugin_root.clone()), plugin_root.clone(), &file_system, ) @@ -243,6 +248,51 @@ async fn plugin_root_resolution_uses_supplied_executor_file_system() { ); } +#[tokio::test] +async fn plugin_root_resolution_accepts_foreign_executor_file_uri() { + let plugin_root = PathUri::parse("file:///C:/plugins/foo").expect("Windows plugin root URI"); + let manifest_path = plugin_root + .join(".codex-plugin/plugin.json") + .expect("manifest URI"); + let parsed_manifest = + parse_plugin_manifest_uri(&plugin_root, &manifest_path, MANIFEST_CONTENTS) + .expect("parse manifest"); + let file_system = SyntheticPluginFileSystem { + plugin_root: plugin_root.clone(), + manifest_path: manifest_path.clone(), + calls: Mutex::new(Vec::new()), + }; + let selected_root = selected_root_uri("selected-demo", "executor-test", plugin_root.clone()); + let resolved = resolve_plugin_root(&selected_root, plugin_root.clone(), &file_system) + .await + .expect("resolve executor plugin"); + + assert_eq!( + resolved, + Some( + ResolvedPlugin::from_environment( + "selected-demo".to_string(), + "executor-test".to_string(), + plugin_root.clone(), + manifest_path.clone(), + parsed_manifest, + ) + .expect("valid expected descriptor") + ) + ); + assert_eq!( + *file_system + .calls + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner), + vec![ + FileSystemCall::Metadata(plugin_root), + FileSystemCall::Metadata(manifest_path.clone()), + FileSystemCall::Read(manifest_path), + ] + ); +} + #[tokio::test] async fn standalone_capability_root_is_not_a_plugin() { let temp_dir = tempdir().expect("tempdir"); @@ -292,8 +342,8 @@ async fn malformed_preferred_manifest_does_not_fall_through_to_alternate() { MANIFEST_CONTENTS, ); let expected_path = - AbsolutePathBuf::from_absolute_path_checked(plugin_root.join(".codex-plugin/plugin.json")) - .expect("absolute manifest path"); + PathUri::from_host_native_path(plugin_root.join(".codex-plugin/plugin.json")) + .expect("manifest URI"); let provider = ExecutorPluginProvider::new(Arc::new(EnvironmentManager::default_for_tests())); let err = provider @@ -318,25 +368,3 @@ async fn malformed_preferred_manifest_does_not_fall_through_to_alternate() { ("selected-demo".to_string(), expected_path) ); } - -#[tokio::test] -async fn executor_root_must_be_an_explicit_absolute_path() { - let provider = ExecutorPluginProvider::new(Arc::new(EnvironmentManager::default_for_tests())); - let selected_root = SelectedCapabilityRoot { - id: "selected-demo".to_string(), - location: CapabilityRootLocation::Environment { - environment_id: LOCAL_ENVIRONMENT_ID.to_string(), - path: "~/plugins/demo".to_string(), - }, - }; - - let err = provider - .resolve(&selected_root) - .await - .expect_err("home-relative executor path should fail"); - - assert_eq!( - err.to_string(), - "selected capability root `selected-demo` has invalid path `~/plugins/demo`: executor path must be absolute" - ); -} diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index afb103b883b2..7460922616db 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -449,7 +449,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() id: id.to_string(), location: CapabilityRootLocation::Environment { environment_id: environment_id.to_string(), - path: format!("/plugins/{id}"), + path: PathUri::parse(&format!("file:///plugins/{id}")).expect("plugin root URI"), }, }]); init diff --git a/codex-rs/ext/mcp/src/executor_plugin/provider.rs b/codex-rs/ext/mcp/src/executor_plugin/provider.rs index e9d5f803db58..eb30e9bb52dc 100644 --- a/codex-rs/ext/mcp/src/executor_plugin/provider.rs +++ b/codex-rs/ext/mcp/src/executor_plugin/provider.rs @@ -8,9 +8,9 @@ use codex_plugin::PluginResourceLocator; use codex_plugin::ResolvedPlugin; use codex_plugin::ResolvedPluginLocation; use codex_plugin::manifest::PluginManifestMcpServers; -use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use std::io; +use std::path::PathBuf; use thiserror::Error; const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; @@ -25,14 +25,14 @@ pub(super) enum ExecutorPluginMcpProviderError { #[error("failed to read MCP config for selected plugin `{plugin_id}` at `{path}`: {source}")] ReadConfig { plugin_id: String, - path: AbsolutePathBuf, + path: PathUri, #[source] source: io::Error, }, #[error("failed to parse MCP config for selected plugin `{plugin_id}` at `{path}`: {source}")] ParseConfig { plugin_id: String, - path: AbsolutePathBuf, + path: PathUri, #[source] source: serde_json::Error, }, @@ -52,7 +52,7 @@ impl ExecutorPluginMcpProvider { async fn load_from_file_system( plugin: &ResolvedPlugin, - plugin_root: &AbsolutePathBuf, + plugin_root: &PathUri, file_system: &dyn ExecutorFileSystem, ) -> Result, ExecutorPluginMcpProviderError> { let ResolvedPluginLocation::Environment { environment_id, .. } = plugin.location(); @@ -61,10 +61,9 @@ async fn load_from_file_system( Some(PluginManifestMcpServers::Path(PluginResourceLocator::Environment { path, .. })) => { - let config_uri = PathUri::from_abs_path(path); ( file_system - .read_file_text(&config_uri, /*sandbox*/ None) + .read_file_text(path, /*sandbox*/ None) .await .map_err(|source| ExecutorPluginMcpProviderError::ReadConfig { plugin_id: plugin_id.to_string(), @@ -76,13 +75,16 @@ async fn load_from_file_system( } Some(PluginManifestMcpServers::Object(object_config)) => ( object_config.clone(), - plugin_root.join(".codex-plugin/plugin.json"), + plugin_root + .join(".codex-plugin/plugin.json") + .expect("static plugin manifest path must be valid"), ), None => { - let config_path = plugin_root.join(DEFAULT_MCP_CONFIG_FILE); - let config_uri = PathUri::from_abs_path(&config_path); + let config_path = plugin_root + .join(DEFAULT_MCP_CONFIG_FILE) + .expect("static MCP config path must be valid"); let contents = match file_system - .read_file_text(&config_uri, /*sandbox*/ None) + .read_file_text(&config_path, /*sandbox*/ None) .await { Ok(contents) => contents, @@ -100,8 +102,9 @@ async fn load_from_file_system( (contents, config_path) } }; + let plugin_root_path = PathBuf::from(plugin_root.inferred_native_path_string()); let parsed = parse_plugin_mcp_config( - plugin_root.as_path(), + plugin_root_path.as_path(), &contents, PluginMcpServerPlacement::Environment { environment_id }, ) diff --git a/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs b/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs index 07d6621856ab..791d562623ca 100644 --- a/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs +++ b/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs @@ -156,7 +156,8 @@ async fn reads_declared_config_only_through_executor_file_system() { reads: Mutex::new(Vec::new()), }; - let servers = load_from_file_system(&plugin, &plugin_root, &file_system) + let plugin_root_uri = PathUri::from_abs_path(&plugin_root); + let servers = load_from_file_system(&plugin, &plugin_root_uri, &file_system) .await .expect("load executor MCP config"); @@ -210,7 +211,8 @@ async fn reads_manifest_object_config_without_executor_file_system_access() { reads: Mutex::new(Vec::new()), }; - let servers = load_from_file_system(&plugin, &plugin_root, &file_system) + let plugin_root_uri = PathUri::from_abs_path(&plugin_root); + let servers = load_from_file_system(&plugin, &plugin_root_uri, &file_system) .await .expect("load manifest object executor MCP config"); @@ -259,7 +261,8 @@ async fn missing_default_config_is_empty() { reads: Mutex::new(Vec::new()), }; - let servers = load_from_file_system(&plugin, &plugin_root, &file_system) + let plugin_root_uri = PathUri::from_abs_path(&plugin_root); + let servers = load_from_file_system(&plugin, &plugin_root_uri, &file_system) .await .expect("missing default config should be ignored"); @@ -283,7 +286,8 @@ async fn malformed_declared_config_is_an_error() { reads: Mutex::new(Vec::new()), }; - let err = load_from_file_system(&plugin, &plugin_root, &file_system) + let plugin_root_uri = PathUri::from_abs_path(&plugin_root); + let err = load_from_file_system(&plugin, &plugin_root_uri, &file_system) .await .expect_err("malformed declared config should fail"); @@ -297,7 +301,10 @@ async fn malformed_declared_config_is_an_error() { }; assert_eq!( (plugin_id, path), - ("selected-root".to_string(), config_path.clone()) + ( + "selected-root".to_string(), + PathUri::from_abs_path(&config_path) + ) ); assert_eq!(reads(&file_system), vec![config_path]); } @@ -306,11 +313,20 @@ fn resolved_plugin( plugin_root: &AbsolutePathBuf, mcp_servers: Option>, ) -> ResolvedPlugin { + let plugin_root_uri = PathUri::from_abs_path(plugin_root); + let mcp_servers = mcp_servers.map(|mcp_servers| match mcp_servers { + PluginManifestMcpServers::Path(path) => { + PluginManifestMcpServers::Path(PathUri::from_abs_path(&path)) + } + PluginManifestMcpServers::Object(config) => PluginManifestMcpServers::Object(config), + }); ResolvedPlugin::from_environment( "selected-root".to_string(), "executor-test".to_string(), - plugin_root.clone(), - plugin_root.join(".codex-plugin/plugin.json"), + plugin_root_uri.clone(), + plugin_root_uri + .join(".codex-plugin/plugin.json") + .expect("manifest URI"), PluginManifest { name: "demo-plugin".to_string(), version: None, diff --git a/codex-rs/ext/mcp/tests/executor_plugin_mcp.rs b/codex-rs/ext/mcp/tests/executor_plugin_mcp.rs index 8820b35102bf..7bdbc83d62e4 100644 --- a/codex-rs/ext/mcp/tests/executor_plugin_mcp.rs +++ b/codex-rs/ext/mcp/tests/executor_plugin_mcp.rs @@ -9,6 +9,7 @@ use codex_extension_api::McpServerContribution; use codex_extension_api::McpServerContributionContext; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use std::sync::Arc; @@ -59,7 +60,7 @@ command = "expected-command" .build() .await?; - let contributions = selected_plugin_contributions(&config, plugin_root.path()).await; + let contributions = selected_plugin_contributions(&config, plugin_root.path()).await?; assert_eq!( contributions, @@ -93,7 +94,7 @@ command = "expected-command" async fn selected_plugin_contributions( config: &Config, plugin_root: &std::path::Path, -) -> Vec { +) -> Result, Box> { let mut builder = ExtensionRegistryBuilder::new(); codex_mcp_extension::install_executor_plugins( &mut builder, @@ -105,12 +106,12 @@ async fn selected_plugin_contributions( id: "selected-root".to_string(), location: CapabilityRootLocation::Environment { environment_id: LOCAL_ENVIRONMENT_ID.to_string(), - path: plugin_root.to_string_lossy().into_owned(), + path: PathUri::from_host_native_path(plugin_root)?, }, }]); codex_mcp_extension::initialize_executor_plugin_thread_data(&mut thread_init); - registry.mcp_server_contributors()[0] + Ok(registry.mcp_server_contributors()[0] .contribute(McpServerContributionContext::for_thread( config, &thread_init, @@ -135,5 +136,5 @@ async fn selected_plugin_contributions( panic!("expected selected plugin contribution") } }) - .collect() + .collect()) } diff --git a/codex-rs/ext/skills/src/provider/executor.rs b/codex-rs/ext/skills/src/provider/executor.rs index 75234dfbee61..273a1614bafa 100644 --- a/codex-rs/ext/skills/src/provider/executor.rs +++ b/codex-rs/ext/skills/src/provider/executor.rs @@ -1,4 +1,3 @@ -use std::path::PathBuf; use std::sync::Arc; use codex_core_skills::SkillMetadata; @@ -197,13 +196,6 @@ fn catalog_entry_from_skill( entry } -fn executor_absolute_path(path: &str) -> std::io::Result { - let path = PathBuf::from(path); - if !path.is_absolute() { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "executor path must be absolute", - )); - } - AbsolutePathBuf::from_absolute_path_checked(path) +fn executor_absolute_path(path: &PathUri) -> std::io::Result { + path.to_abs_path() } diff --git a/codex-rs/ext/skills/tests/executor_file_system_authority.rs b/codex-rs/ext/skills/tests/executor_file_system_authority.rs index e4b09e96a054..8c5160e2c115 100644 --- a/codex-rs/ext/skills/tests/executor_file_system_authority.rs +++ b/codex-rs/ext/skills/tests/executor_file_system_authority.rs @@ -222,7 +222,6 @@ async fn skill_loading_and_reads_use_the_supplied_executor_file_system() { #[tokio::test] async fn selected_root_id_distinguishes_identical_executor_paths() { let test_root = create_local_skill_root("root-identity").expect("create local skill root"); - let root_path = test_root.to_string_lossy().into_owned(); let canonical_root = AbsolutePathBuf::from_absolute_path_checked(&test_root) .expect("absolute skill root") .canonicalize() @@ -242,7 +241,7 @@ async fn selected_root_id_distinguishes_identical_executor_paths() { id: id.to_string(), location: CapabilityRootLocation::Environment { environment_id: "local".to_string(), - path: root_path.clone(), + path: PathUri::from_host_native_path(&test_root).expect("skill root URI"), }, }) .collect(), diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index 8b02558347df..761dc34d71b3 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -48,6 +48,7 @@ use codex_skills_extension::provider::SkillProviderFuture; use codex_skills_extension::provider::SkillReadRequest; use codex_skills_extension::provider::SkillSearchRequest; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; type TestResult = Result<(), Box>; @@ -171,7 +172,7 @@ async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_in id: "lint-fix".to_string(), location: CapabilityRootLocation::Environment { environment_id: "env-1".to_string(), - path: "/skills/lint-fix".to_string(), + path: PathUri::parse("file:///skills/lint-fix").expect("skill root URI"), }, }]); let session_source = SessionSource::Cli; @@ -494,7 +495,7 @@ async fn root_qualified_locator_selects_only_the_matching_executor_skill() -> Te id: id.to_string(), location: CapabilityRootLocation::Environment { environment_id: "env-1".to_string(), - path: path.to_string(), + path: PathUri::parse(&format!("file://{path}")).expect("skill root URI"), }, }) .collect::>(), diff --git a/codex-rs/plugin/Cargo.toml b/codex-rs/plugin/Cargo.toml index 77116dfa3ef4..3e55f55c332a 100644 --- a/codex-rs/plugin/Cargo.toml +++ b/codex-rs/plugin/Cargo.toml @@ -16,6 +16,7 @@ workspace = true codex-config = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } codex-utils-plugins = { workspace = true } thiserror = { workspace = true } diff --git a/codex-rs/plugin/src/provider.rs b/codex-rs/plugin/src/provider.rs index d2d8cc7c81c0..6dd81db8a77c 100644 --- a/codex-rs/plugin/src/provider.rs +++ b/codex-rs/plugin/src/provider.rs @@ -1,6 +1,6 @@ use crate::manifest::PluginManifest; use codex_protocol::capabilities::SelectedCapabilityRoot; -use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use std::error::Error as StdError; use std::future::Future; use thiserror::Error; @@ -11,8 +11,8 @@ pub enum PluginResourceLocator { Environment { /// Environment whose filesystem owns the resource. environment_id: String, - /// Absolute resource path within that filesystem. - path: AbsolutePathBuf, + /// Resource URI within that filesystem. + path: PathUri, }, } @@ -22,8 +22,8 @@ pub enum ResolvedPluginLocation { Environment { /// Environment whose filesystem owns the package. environment_id: String, - /// Absolute package root within that filesystem. - root: AbsolutePathBuf, + /// Package root URI within that filesystem. + root: PathUri, }, } @@ -40,10 +40,7 @@ pub struct ResolvedPlugin { #[derive(Clone, Debug, Error, PartialEq, Eq)] pub enum ResolvedPluginError { #[error("plugin resource path `{path}` is outside package root `{root}`")] - ResourceOutsideRoot { - root: AbsolutePathBuf, - path: AbsolutePathBuf, - }, + ResourceOutsideRoot { root: PathUri, path: PathUri }, } impl ResolvedPlugin { @@ -51,9 +48,9 @@ impl ResolvedPlugin { pub fn from_environment( selected_root_id: String, environment_id: String, - root: AbsolutePathBuf, - manifest_path: AbsolutePathBuf, - manifest: PluginManifest, + root: PathUri, + manifest_path: PathUri, + manifest: PluginManifest, ) -> Result { let manifest_path = environment_resource(&environment_id, &root, manifest_path)?; let manifest = manifest @@ -92,10 +89,10 @@ impl ResolvedPlugin { fn environment_resource( environment_id: &str, - root: &AbsolutePathBuf, - path: AbsolutePathBuf, + root: &PathUri, + path: PathUri, ) -> Result { - if !path.as_path().starts_with(root.as_path()) { + if !path.starts_with(root) { return Err(ResolvedPluginError::ResourceOutsideRoot { root: root.clone(), path, diff --git a/codex-rs/plugin/src/provider_tests.rs b/codex-rs/plugin/src/provider_tests.rs index 99bc3c20891f..7f9e70d3300a 100644 --- a/codex-rs/plugin/src/provider_tests.rs +++ b/codex-rs/plugin/src/provider_tests.rs @@ -7,22 +7,28 @@ use crate::manifest::PluginManifestInterface; use crate::manifest::PluginManifestMcpServers; use crate::manifest::PluginManifestPaths; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; fn absolute(path: impl AsRef) -> AbsolutePathBuf { AbsolutePathBuf::from_absolute_path_checked(path.as_ref()).expect("absolute test path") } -fn resource(environment_id: &str, path: AbsolutePathBuf) -> PluginResourceLocator { +fn path_uri(path: &AbsolutePathBuf) -> PathUri { + PathUri::from_abs_path(path) +} + +fn resource(environment_id: &str, path: &AbsolutePathBuf) -> PluginResourceLocator { PluginResourceLocator::Environment { environment_id: environment_id.to_string(), - path, + path: path_uri(path), } } #[test] fn environment_descriptor_binds_every_manifest_resource() { let root = absolute(std::env::current_dir().expect("cwd").join("plugin-root")); + let root_uri = path_uri(&root); let manifest_path = root.join(".codex-plugin/plugin.json"); let skills = root.join("skills"); let mcp_servers = root.join(".mcp.json"); @@ -37,15 +43,15 @@ fn environment_descriptor_binds_every_manifest_resource() { description: None, keywords: Vec::new(), paths: PluginManifestPaths { - skills: vec![skills.clone()], - mcp_servers: Some(PluginManifestMcpServers::Path(mcp_servers.clone())), - apps: Some(apps.clone()), - hooks: Some(PluginManifestHooks::Paths(vec![hooks.clone()])), + skills: vec![path_uri(&skills)], + mcp_servers: Some(PluginManifestMcpServers::Path(path_uri(&mcp_servers))), + apps: Some(path_uri(&apps)), + hooks: Some(PluginManifestHooks::Paths(vec![path_uri(&hooks)])), }, interface: Some(PluginManifestInterface { - composer_icon: Some(composer_icon.clone()), - logo: Some(logo.clone()), - screenshots: vec![screenshot.clone()], + composer_icon: Some(path_uri(&composer_icon)), + logo: Some(path_uri(&logo)), + screenshots: vec![path_uri(&screenshot)], ..PluginManifestInterface::default() }), }; @@ -53,15 +59,15 @@ fn environment_descriptor_binds_every_manifest_resource() { let plugin = ResolvedPlugin::from_environment( "selected-demo".to_string(), "executor-1".to_string(), - root, - manifest_path.clone(), + root_uri, + path_uri(&manifest_path), manifest, ) .expect("valid descriptor"); assert_eq!( plugin.manifest_path(), - &resource("executor-1", manifest_path) + &resource("executor-1", &manifest_path) ); assert_eq!( plugin.manifest(), @@ -71,21 +77,21 @@ fn environment_descriptor_binds_every_manifest_resource() { description: None, keywords: Vec::new(), paths: PluginManifestPaths { - skills: vec![resource("executor-1", skills)], + skills: vec![resource("executor-1", &skills)], mcp_servers: Some(PluginManifestMcpServers::Path(resource( "executor-1", - mcp_servers, + &mcp_servers, ))), - apps: Some(resource("executor-1", apps)), + apps: Some(resource("executor-1", &apps)), hooks: Some(PluginManifestHooks::Paths(vec![resource( "executor-1", - hooks, + &hooks )])), }, interface: Some(PluginManifestInterface { - composer_icon: Some(resource("executor-1", composer_icon)), - logo: Some(resource("executor-1", logo)), - screenshots: vec![resource("executor-1", screenshot)], + composer_icon: Some(resource("executor-1", &composer_icon)), + logo: Some(resource("executor-1", &logo)), + screenshots: vec![resource("executor-1", &screenshot)], ..PluginManifestInterface::default() }), } @@ -104,7 +110,7 @@ fn environment_descriptor_rejects_resources_outside_package_root() { keywords: Vec::new(), paths: PluginManifestPaths { skills: Vec::new(), - mcp_servers: Some(PluginManifestMcpServers::Path(outside.clone())), + mcp_servers: Some(PluginManifestMcpServers::Path(path_uri(&outside))), apps: None, hooks: None, }, @@ -114,8 +120,8 @@ fn environment_descriptor_rejects_resources_outside_package_root() { let err = ResolvedPlugin::from_environment( "selected-demo".to_string(), "executor-1".to_string(), - root.clone(), - root.join(".codex-plugin/plugin.json"), + path_uri(&root), + path_uri(&root.join(".codex-plugin/plugin.json")), manifest, ) .expect_err("outside resource should fail"); @@ -123,8 +129,8 @@ fn environment_descriptor_rejects_resources_outside_package_root() { assert_eq!( err, ResolvedPluginError::ResourceOutsideRoot { - root, - path: outside, + root: path_uri(&root), + path: path_uri(&outside), } ); } diff --git a/codex-rs/protocol/src/capabilities.rs b/codex-rs/protocol/src/capabilities.rs index cfdead7b2841..8430244fc926 100644 --- a/codex-rs/protocol/src/capabilities.rs +++ b/codex-rs/protocol/src/capabilities.rs @@ -1,3 +1,4 @@ +use codex_utils_path_uri::PathUri; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -25,6 +26,22 @@ pub enum CapabilityRootLocation { #[serde(rename = "environmentId")] #[ts(rename = "environmentId")] environment_id: String, - path: String, + /// Canonical `file:` URI for the root in the selected environment. + #[serde(deserialize_with = "deserialize_strict_path_uri")] + #[schemars(regex(pattern = r"^file:"))] + #[ts(type = "string")] + path: PathUri, }, } + +fn deserialize_strict_path_uri<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + let path = String::deserialize(deserializer)?; + PathUri::parse(&path).map_err(serde::de::Error::custom) +} + +#[cfg(test)] +#[path = "capabilities_tests.rs"] +mod tests; diff --git a/codex-rs/protocol/src/capabilities_tests.rs b/codex-rs/protocol/src/capabilities_tests.rs new file mode 100644 index 000000000000..c31b022c2275 --- /dev/null +++ b/codex-rs/protocol/src/capabilities_tests.rs @@ -0,0 +1,50 @@ +use super::CapabilityRootLocation; +use super::SelectedCapabilityRoot; +use codex_utils_path_uri::PathUri; +use pretty_assertions::assert_eq; + +#[test] +fn environment_capability_root_requires_path_uri_on_the_wire() { + let err = serde_json::from_str::( + r#"{ + "id": "selected-demo", + "location": { + "type": "environment", + "environmentId": "executor-test", + "path": "/plugins/demo" + } + }"#, + ) + .expect_err("native paths should be rejected"); + + assert!( + err.to_string().contains("relative URL without a base"), + "unexpected error: {err}" + ); +} + +#[test] +fn environment_capability_root_accepts_foreign_file_uri() { + let selected_root = serde_json::from_str::( + r#"{ + "id": "selected-demo", + "location": { + "type": "environment", + "environmentId": "executor-test", + "path": "file:///C:/plugins/demo" + } + }"#, + ) + .expect("file URI should deserialize"); + + assert_eq!( + selected_root, + SelectedCapabilityRoot { + id: "selected-demo".to_string(), + location: CapabilityRootLocation::Environment { + environment_id: "executor-test".to_string(), + path: PathUri::parse("file:///C:/plugins/demo").expect("path URI"), + }, + } + ); +} diff --git a/codex-rs/utils/path-uri/src/lib.rs b/codex-rs/utils/path-uri/src/lib.rs index 9794e7c70940..8e2e85b30c5a 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -250,10 +250,10 @@ impl PathUri { /// Returns true when this URI is lexically equal to or below `base`. /// /// Containment is computed using URI authority and path-segment boundaries, - /// without consulting the host filesystem. Percent-encoded path separators - /// fail closed because native path conversion may interpret them as segment - /// boundaries. Opaque fallback URIs created by [`Self::from_abs_path`] only - /// contain themselves. + /// without consulting the host filesystem. Percent-encoded native path + /// separators fail closed because native path conversion may interpret them + /// as segment boundaries. Opaque fallback URIs created by + /// [`Self::from_abs_path`] only contain themselves. pub fn starts_with(&self, base: &Self) -> bool { if self == base { return true; @@ -265,10 +265,18 @@ impl PathUri { return false; } - let Some(path_segments) = containment_path_segments(&self.0) else { + let Some(path_segments) = containment_path_segments( + &self.0, + self.infer_path_convention() + .unwrap_or(PathConvention::Posix), + ) else { return false; }; - let Some(base_segments) = containment_path_segments(&base.0) else { + let Some(base_segments) = containment_path_segments( + &base.0, + base.infer_path_convention() + .unwrap_or(PathConvention::Posix), + ) else { return false; }; path_segments.starts_with(&base_segments) @@ -569,7 +577,7 @@ fn is_windows_drive_uri_segment(segment: &str) -> bool { matches!(segment.as_bytes(), [drive, b':'] if drive.is_ascii_alphabetic()) } -fn containment_path_segments(url: &Url) -> Option> { +fn containment_path_segments(url: &Url, convention: PathConvention) -> Option> { let segments = url .path_segments()? .filter(|segment| !segment.is_empty()) @@ -577,7 +585,7 @@ fn containment_path_segments(url: &Url) -> Option> { (!segments.iter().any(|segment| { urlencoding::decode_binary(segment.as_bytes()) .iter() - .any(|byte| matches!(*byte, b'/' | b'\\')) + .any(|byte| *byte == b'/' || (convention == PathConvention::Windows && *byte == b'\\')) })) .then_some(segments) } diff --git a/codex-rs/utils/path-uri/src/tests.rs b/codex-rs/utils/path-uri/src/tests.rs index c52c3f9e6a16..1eead137f62a 100644 --- a/codex-rs/utils/path-uri/src/tests.rs +++ b/codex-rs/utils/path-uri/src/tests.rs @@ -748,6 +748,11 @@ fn starts_with_uses_uri_segment_boundaries() { ( "file:///workspace/plugin/%5C..%5Coutside", "file:///workspace/plugin", + true, + ), + ( + "file:///C:/plugins/foo/%5C..%5Coutside", + "file:///C:/plugins/foo", false, ), ] { From de54d62aeabcae124f3beb3310dc3a410b525d65 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 23 Jun 2026 15:08:59 +0100 Subject: [PATCH 4/8] Document manifest resource resolver contract --- codex-rs/core-plugins/src/manifest.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/codex-rs/core-plugins/src/manifest.rs b/codex-rs/core-plugins/src/manifest.rs index f2f75fe88f16..888069a827cc 100644 --- a/codex-rs/core-plugins/src/manifest.rs +++ b/codex-rs/core-plugins/src/manifest.rs @@ -148,6 +148,12 @@ pub(crate) fn parse_plugin_manifest( parse_plugin_manifest_with_resolver(&resolver, contents) } +/// Separates manifest decoding from filesystem-specific resource resolution. +/// +/// Implementations interpret manifest-relative paths using the convention of +/// the filesystem that owns the plugin root. They must reject paths that are +/// invalid for that convention or lexically escape the plugin root, and supply +/// source-appropriate values for fallback naming and diagnostics. trait ManifestResourceResolver { type Resource; From 1c837bc3004c595ac7a3dad8df9438964b05101e Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 23 Jun 2026 15:17:52 +0100 Subject: [PATCH 5/8] Preserve manifest warning path formatting --- codex-rs/core-plugins/src/manifest.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/core-plugins/src/manifest.rs b/codex-rs/core-plugins/src/manifest.rs index 888069a827cc..22cd3bc5f661 100644 --- a/codex-rs/core-plugins/src/manifest.rs +++ b/codex-rs/core-plugins/src/manifest.rs @@ -454,7 +454,7 @@ fn resolve_default_prompt_str(manifest_path: &str, field: &str, prompt: &str) -> } fn warn_invalid_default_prompt(manifest_path: &str, field: &str, message: &str) { - tracing::warn!(path = manifest_path, "ignoring {field}: {message}"); + tracing::warn!(path = %manifest_path, "ignoring {field}: {message}"); } fn json_value_type(value: &JsonValue) -> &'static str { From c9dee0badccb62649c67a9236489215751619a5a Mon Sep 17 00:00:00 2001 From: jif Date: Tue, 23 Jun 2026 16:13:00 +0100 Subject: [PATCH 6/8] Avoid expect in executor MCP provider --- .../ext/mcp/src/executor_plugin/provider.rs | 28 +++++++++---- .../mcp/src/executor_plugin/provider_tests.rs | 40 ++++++++++++++++++- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/codex-rs/ext/mcp/src/executor_plugin/provider.rs b/codex-rs/ext/mcp/src/executor_plugin/provider.rs index eb30e9bb52dc..d3f264fcc2cc 100644 --- a/codex-rs/ext/mcp/src/executor_plugin/provider.rs +++ b/codex-rs/ext/mcp/src/executor_plugin/provider.rs @@ -9,6 +9,7 @@ use codex_plugin::ResolvedPlugin; use codex_plugin::ResolvedPluginLocation; use codex_plugin::manifest::PluginManifestMcpServers; use codex_utils_path_uri::PathUri; +use codex_utils_path_uri::PathUriParseError; use std::io; use std::path::PathBuf; use thiserror::Error; @@ -29,6 +30,16 @@ pub(super) enum ExecutorPluginMcpProviderError { #[source] source: io::Error, }, + #[error( + "failed to resolve MCP config path `{relative_path}` below selected plugin `{plugin_id}` at `{root}`: {source}" + )] + InvalidConfigPath { + plugin_id: String, + root: PathUri, + relative_path: &'static str, + #[source] + source: PathUriParseError, + }, #[error("failed to parse MCP config for selected plugin `{plugin_id}` at `{path}`: {source}")] ParseConfig { plugin_id: String, @@ -73,16 +84,19 @@ async fn load_from_file_system( path.clone(), ) } - Some(PluginManifestMcpServers::Object(object_config)) => ( - object_config.clone(), - plugin_root - .join(".codex-plugin/plugin.json") - .expect("static plugin manifest path must be valid"), - ), + Some(PluginManifestMcpServers::Object(object_config)) => { + let PluginResourceLocator::Environment { path, .. } = plugin.manifest_path(); + (object_config.clone(), path.clone()) + } None => { let config_path = plugin_root .join(DEFAULT_MCP_CONFIG_FILE) - .expect("static MCP config path must be valid"); + .map_err(|source| ExecutorPluginMcpProviderError::InvalidConfigPath { + plugin_id: plugin_id.to_string(), + root: plugin_root.clone(), + relative_path: DEFAULT_MCP_CONFIG_FILE, + source, + })?; let contents = match file_system .read_file_text(&config_path, /*sandbox*/ None) .await diff --git a/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs b/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs index 791d562623ca..a007acaac73d 100644 --- a/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs +++ b/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs @@ -309,6 +309,44 @@ async fn malformed_declared_config_is_an_error() { assert_eq!(reads(&file_system), vec![config_path]); } +#[tokio::test] +async fn malformed_manifest_object_config_reports_actual_manifest_path() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let plugin_root = AbsolutePathBuf::from_absolute_path_checked(temp_dir.path().join("plugin")) + .expect("absolute plugin root"); + let plugin = resolved_plugin( + &plugin_root, + Some(PluginManifestMcpServers::Object("{not-json".to_string())), + ); + let file_system = SyntheticExecutorFileSystem { + config_path: plugin_root.join(DEFAULT_MCP_CONFIG_FILE), + config_contents: None, + reads: Mutex::new(Vec::new()), + }; + + let plugin_root_uri = PathUri::from_abs_path(&plugin_root); + let err = load_from_file_system(&plugin, &plugin_root_uri, &file_system) + .await + .expect_err("malformed manifest object config should fail"); + + let ExecutorPluginMcpProviderError::ParseConfig { + plugin_id, + path, + source: _, + } = err + else { + panic!("expected parse error"); + }; + assert_eq!( + (plugin_id, path), + ( + "selected-root".to_string(), + PathUri::from_abs_path(&plugin_root.join(".claude-plugin/plugin.json")) + ) + ); + assert_eq!(reads(&file_system), Vec::new()); +} + fn resolved_plugin( plugin_root: &AbsolutePathBuf, mcp_servers: Option>, @@ -325,7 +363,7 @@ fn resolved_plugin( "executor-test".to_string(), plugin_root_uri.clone(), plugin_root_uri - .join(".codex-plugin/plugin.json") + .join(".claude-plugin/plugin.json") .expect("manifest URI"), PluginManifest { name: "demo-plugin".to_string(), From 709bce24ab7f31af13afd23dc06b8467935bdfe5 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 23 Jun 2026 20:35:10 +0100 Subject: [PATCH 7/8] Share URI-native manifest path resolution --- codex-rs/core-plugins/src/manifest.rs | 247 +++++++++++++------------- codex-rs/plugin/src/manifest.rs | 3 +- codex-rs/utils/path-uri/src/lib.rs | 16 +- codex-rs/utils/path-uri/src/tests.rs | 5 + 4 files changed, 146 insertions(+), 125 deletions(-) diff --git a/codex-rs/core-plugins/src/manifest.rs b/codex-rs/core-plugins/src/manifest.rs index 22cd3bc5f661..14d7a4fe419d 100644 --- a/codex-rs/core-plugins/src/manifest.rs +++ b/codex-rs/core-plugins/src/manifest.rs @@ -1,10 +1,11 @@ use codex_config::HooksFile; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathConvention; +use codex_utils_path_uri::PathUri; use codex_utils_plugins::find_plugin_manifest_path; use serde::Deserialize; use serde_json::Value as JsonValue; use std::fs; -use std::path::Component; use std::path::Path; const MAX_DEFAULT_PROMPT_COUNT: usize = 3; const MAX_DEFAULT_PROMPT_LEN: usize = 128; @@ -16,6 +17,8 @@ pub type PluginManifestMcpServers = codex_plugin::manifest::PluginManifestMcpServers; pub type PluginManifestPaths = codex_plugin::manifest::PluginManifestPaths; +pub(crate) type UriPluginManifest = codex_plugin::manifest::PluginManifest; + #[derive(Debug, Default, Deserialize)] #[serde(rename_all = "camelCase")] struct RawPluginManifest { @@ -141,58 +144,19 @@ pub(crate) fn parse_plugin_manifest( manifest_path: &Path, contents: &str, ) -> Result { - let resolver = HostManifestResourceResolver { - plugin_root, - manifest_path, - }; - parse_plugin_manifest_with_resolver(&resolver, contents) -} - -/// Separates manifest decoding from filesystem-specific resource resolution. -/// -/// Implementations interpret manifest-relative paths using the convention of -/// the filesystem that owns the plugin root. They must reject paths that are -/// invalid for that convention or lexically escape the plugin root, and supply -/// source-appropriate values for fallback naming and diagnostics. -trait ManifestResourceResolver { - type Resource; - - fn plugin_name_fallback(&self) -> Option; - fn manifest_path_for_warning(&self) -> String; - fn resolve_path(&self, field: &'static str, path: Option<&str>) -> Option; -} - -struct HostManifestResourceResolver<'a> { - plugin_root: &'a Path, - manifest_path: &'a Path, -} - -impl ManifestResourceResolver for HostManifestResourceResolver<'_> { - type Resource = AbsolutePathBuf; - - fn plugin_name_fallback(&self) -> Option { - self.plugin_root - .file_name() - .and_then(|entry| entry.to_str()) - .map(str::to_string) - } - - fn manifest_path_for_warning(&self) -> String { - self.manifest_path.display().to_string() - } - - fn resolve_path(&self, field: &'static str, path: Option<&str>) -> Option { - resolve_manifest_path(self.plugin_root, field, path) - } + let plugin_root_uri = + PathUri::from_host_native_path(plugin_root).map_err(serde_json::Error::io)?; + let manifest_path_uri = + PathUri::from_host_native_path(manifest_path).map_err(serde_json::Error::io)?; + parse_plugin_manifest_uri(&plugin_root_uri, &manifest_path_uri, contents)? + .try_map_resources(|path| path.to_abs_path().map_err(serde_json::Error::io)) } -fn parse_plugin_manifest_with_resolver( - resolver: &R, +pub(crate) fn parse_plugin_manifest_uri( + plugin_root: &PathUri, + manifest_path: &PathUri, contents: &str, -) -> Result, serde_json::Error> -where - R: ManifestResourceResolver, -{ +) -> Result { let RawPluginManifest { name: raw_name, version, @@ -204,11 +168,11 @@ where hooks, interface, } = serde_json::from_str::(contents)?; - let name = resolver - .plugin_name_fallback() + let name = plugin_root + .basename() .filter(|_| raw_name.trim().is_empty()) .unwrap_or(raw_name); - let manifest_path_for_warning = resolver.manifest_path_for_warning(); + let manifest_path_for_warning = manifest_path.to_string(); let version = version.and_then(|version| { let version = version.trim(); (!version.is_empty()).then(|| version.to_string()) @@ -248,13 +212,13 @@ where ), brand_color, composer_icon: resolve_interface_asset_path( - resolver, + plugin_root, "interface.composerIcon", composer_icon.as_deref(), ), - logo: resolve_interface_asset_path(resolver, "interface.logo", logo.as_deref()), + logo: resolve_interface_asset_path(plugin_root, "interface.logo", logo.as_deref()), logo_dark: resolve_interface_asset_path( - resolver, + plugin_root, "interface.logoDark", logo_dark.as_deref(), ), @@ -262,7 +226,7 @@ where .iter() .filter_map(|screenshot| { resolve_interface_asset_path( - resolver, + plugin_root, "interface.screenshots", Some(screenshot), ) @@ -294,30 +258,28 @@ where description, keywords, paths: codex_plugin::manifest::PluginManifestPaths { - skills: resolve_manifest_paths(resolver, "skills", skills.as_ref()), - mcp_servers: resolve_manifest_mcp_servers(resolver, mcp_servers), - apps: resolver.resolve_path("apps", apps.as_deref()), - hooks: resolve_manifest_hooks(resolver, hooks), + skills: resolve_manifest_paths(plugin_root, "skills", skills.as_ref()), + mcp_servers: resolve_manifest_mcp_servers(plugin_root, mcp_servers), + apps: resolve_manifest_path(plugin_root, "apps", apps.as_deref()), + hooks: resolve_manifest_hooks(plugin_root, hooks), }, interface, }) } -fn resolve_manifest_hooks( - resolver: &R, +fn resolve_manifest_hooks( + plugin_root: &PathUri, hooks: Option, -) -> Option> -where - R: ManifestResourceResolver, -{ +) -> Option> { match hooks? { - RawPluginManifestHooks::Path(path) => resolver - .resolve_path("hooks", Some(&path)) - .map(|path| codex_plugin::manifest::PluginManifestHooks::Paths(vec![path])), + RawPluginManifestHooks::Path(path) => { + resolve_manifest_path(plugin_root, "hooks", Some(&path)) + .map(|path| codex_plugin::manifest::PluginManifestHooks::Paths(vec![path])) + } RawPluginManifestHooks::Paths(paths) => { let hooks = paths .iter() - .filter_map(|path| resolver.resolve_path("hooks", Some(path))) + .filter_map(|path| resolve_manifest_path(plugin_root, "hooks", Some(path))) .collect::>(); (!hooks.is_empty()).then_some(codex_plugin::manifest::PluginManifestHooks::Paths(hooks)) } @@ -338,17 +300,15 @@ where } } -fn resolve_manifest_mcp_servers( - resolver: &R, +fn resolve_manifest_mcp_servers( + plugin_root: &PathUri, mcp_servers: Option, -) -> Option> -where - R: ManifestResourceResolver, -{ +) -> Option> { match mcp_servers? { - RawPluginManifestMcpServers::Path(path) => resolver - .resolve_path("mcpServers", Some(&path)) - .map(codex_plugin::manifest::PluginManifestMcpServers::Path), + RawPluginManifestMcpServers::Path(path) => { + resolve_manifest_path(plugin_root, "mcpServers", Some(&path)) + .map(codex_plugin::manifest::PluginManifestMcpServers::Path) + } RawPluginManifestMcpServers::Object(servers) => match serde_json::to_string(&servers) { Ok(servers) => Some(codex_plugin::manifest::PluginManifestMcpServers::Object( servers, @@ -368,15 +328,12 @@ where } } -fn resolve_interface_asset_path( - resolver: &R, +fn resolve_interface_asset_path( + plugin_root: &PathUri, field: &'static str, path: Option<&str>, -) -> Option -where - R: ManifestResourceResolver, -{ - resolver.resolve_path(field, path) +) -> Option { + resolve_manifest_path(plugin_root, field, path) } fn resolve_default_prompts( @@ -468,22 +425,20 @@ fn json_value_type(value: &JsonValue) -> &'static str { } } -fn resolve_manifest_paths( - resolver: &R, +fn resolve_manifest_paths( + plugin_root: &PathUri, field: &'static str, paths: Option<&RawPluginManifestPaths>, -) -> Vec -where - R: ManifestResourceResolver, -{ +) -> Vec { match paths { - Some(RawPluginManifestPaths::Path(path)) => resolver - .resolve_path(field, Some(path)) - .map(|path| vec![path]) - .unwrap_or_default(), + Some(RawPluginManifestPaths::Path(path)) => { + resolve_manifest_path(plugin_root, field, Some(path)) + .map(|path| vec![path]) + .unwrap_or_default() + } Some(RawPluginManifestPaths::Paths(paths)) => paths .iter() - .filter_map(|path| resolver.resolve_path(field, Some(path))) + .filter_map(|path| resolve_manifest_path(plugin_root, field, Some(path))) .collect(), Some(RawPluginManifestPaths::Invalid(value)) => { tracing::warn!( @@ -497,12 +452,10 @@ where } fn resolve_manifest_path( - plugin_root: &Path, + plugin_root: &PathUri, field: &'static str, path: Option<&str>, -) -> Option { - // `plugin.json` paths are required to be relative to the plugin root and we return the - // normalized absolute path to the rest of the system. +) -> Option { let path = path?; if path.is_empty() { return None; @@ -516,27 +469,40 @@ fn resolve_manifest_path( return None; } - let mut normalized = std::path::PathBuf::new(); - for component in Path::new(relative_path).components() { - match component { - Component::Normal(component) => normalized.push(component), - Component::ParentDir => { - tracing::warn!("ignoring {field}: path must not contain '..'"); - return None; - } - _ => { - tracing::warn!("ignoring {field}: path must stay within the plugin root"); - return None; - } + let convention = plugin_root.infer_path_convention(); + let has_parent_component = match convention { + Some(PathConvention::Windows) => relative_path + .split(['/', '\\']) + .any(|component| component == ".."), + Some(PathConvention::Posix) | None => { + relative_path.split('/').any(|component| component == "..") } + }; + if has_parent_component { + tracing::warn!("ignoring {field}: path must not contain '..'"); + return None; } - AbsolutePathBuf::try_from(plugin_root.join(normalized)) - .map_err(|err| { - tracing::warn!("ignoring {field}: path must resolve to an absolute path: {err}"); - err - }) - .ok() + let has_windows_root = convention == Some(PathConvention::Windows) + && (relative_path.starts_with('\\') + || matches!(relative_path.as_bytes(), [drive, b':', ..] if drive.is_ascii_alphabetic())); + if relative_path.starts_with('/') || has_windows_root { + tracing::warn!("ignoring {field}: path must stay within the plugin root"); + return None; + } + + let resolved = match plugin_root.join(relative_path) { + Ok(resolved) => resolved, + Err(err) => { + tracing::warn!("ignoring {field}: path must resolve under plugin root: {err}"); + return None; + } + }; + if !resolved.starts_with(plugin_root) { + tracing::warn!("ignoring {field}: path must stay within the plugin root"); + return None; + } + Some(resolved) } #[cfg(test)] @@ -551,6 +517,7 @@ mod tests { use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_utils_absolute_path::AbsolutePathBuf; + use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use std::fs; use std::path::Path; @@ -760,6 +727,46 @@ mod tests { ); } + #[test] + fn uri_manifest_uses_the_root_path_convention() { + let windows_root = + PathUri::parse("file:///C:/plugins/demo-plugin").expect("Windows plugin root URI"); + let posix_root = + PathUri::parse("file:///plugins/demo-plugin").expect("POSIX plugin root URI"); + let composer_icon = r"./assets\..\icon.svg"; + + assert_eq!(parse_uri_composer_icon(&windows_root, composer_icon), None); + assert_eq!( + parse_uri_composer_icon(&posix_root, composer_icon), + Some( + posix_root + .join(r"assets\..\icon.svg") + .expect("composer icon URI") + ) + ); + } + + fn parse_uri_composer_icon(plugin_root: &PathUri, composer_icon: &str) -> Option { + let manifest_path = plugin_root + .join(".codex-plugin/plugin.json") + .expect("manifest URI"); + let composer_icon_json = + serde_json::to_string(composer_icon).expect("serialize composer icon"); + let contents = format!( + r#"{{ + "name": "demo-plugin", + "interface": {{ + "displayName": "Demo Plugin", + "composerIcon": {composer_icon_json} + }} +}}"# + ); + super::parse_plugin_manifest_uri(plugin_root, &manifest_path, &contents) + .expect("URI manifest") + .interface + .and_then(|interface| interface.composer_icon) + } + #[tokio::test] async fn host_and_executor_sources_parse_the_same_manifest() { let temp_dir = tempdir().expect("tempdir"); diff --git a/codex-rs/plugin/src/manifest.rs b/codex-rs/plugin/src/manifest.rs index eb8e1f7d9393..89cfd8d0b68c 100644 --- a/codex-rs/plugin/src/manifest.rs +++ b/codex-rs/plugin/src/manifest.rs @@ -90,7 +90,8 @@ impl PluginManifest { .unwrap_or(&self.name) } - pub(crate) fn try_map_resources( + /// Maps every path-bearing resource in the manifest. + pub fn try_map_resources( self, mut map: impl FnMut(Resource) -> Result, ) -> Result, Error> { diff --git a/codex-rs/utils/path-uri/src/lib.rs b/codex-rs/utils/path-uri/src/lib.rs index 9794e7c70940..650343209845 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -265,10 +265,18 @@ impl PathUri { return false; } - let Some(path_segments) = containment_path_segments(&self.0) else { + let Some(path_segments) = containment_path_segments( + &self.0, + self.infer_path_convention() + .unwrap_or(PathConvention::Posix), + ) else { return false; }; - let Some(base_segments) = containment_path_segments(&base.0) else { + let Some(base_segments) = containment_path_segments( + &base.0, + base.infer_path_convention() + .unwrap_or(PathConvention::Posix), + ) else { return false; }; path_segments.starts_with(&base_segments) @@ -569,7 +577,7 @@ fn is_windows_drive_uri_segment(segment: &str) -> bool { matches!(segment.as_bytes(), [drive, b':'] if drive.is_ascii_alphabetic()) } -fn containment_path_segments(url: &Url) -> Option> { +fn containment_path_segments(url: &Url, convention: PathConvention) -> Option> { let segments = url .path_segments()? .filter(|segment| !segment.is_empty()) @@ -577,7 +585,7 @@ fn containment_path_segments(url: &Url) -> Option> { (!segments.iter().any(|segment| { urlencoding::decode_binary(segment.as_bytes()) .iter() - .any(|byte| matches!(*byte, b'/' | b'\\')) + .any(|byte| *byte == b'/' || (convention == PathConvention::Windows && *byte == b'\\')) })) .then_some(segments) } diff --git a/codex-rs/utils/path-uri/src/tests.rs b/codex-rs/utils/path-uri/src/tests.rs index c52c3f9e6a16..1eead137f62a 100644 --- a/codex-rs/utils/path-uri/src/tests.rs +++ b/codex-rs/utils/path-uri/src/tests.rs @@ -748,6 +748,11 @@ fn starts_with_uses_uri_segment_boundaries() { ( "file:///workspace/plugin/%5C..%5Coutside", "file:///workspace/plugin", + true, + ), + ( + "file:///C:/plugins/foo/%5C..%5Coutside", + "file:///C:/plugins/foo", false, ), ] { From 63b9f7a9c52e3500bd061202dc42fe645f8f460f Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 23 Jun 2026 22:36:16 +0100 Subject: [PATCH 8/8] Accept native paths for selected capability roots --- .../schema/json/ClientRequest.json | 14 +++---------- .../codex_app_server_protocol.schemas.json | 18 +++++------------ .../codex_app_server_protocol.v2.schemas.json | 14 +++---------- .../schema/json/v2/ThreadStartParams.json | 14 +++---------- .../typescript/v2/CapabilityRootLocation.ts | 2 +- codex-rs/app-server/README.md | 4 ++-- codex-rs/protocol/src/capabilities.rs | 16 +++++++++------ codex-rs/protocol/src/capabilities_tests.rs | 20 ------------------- 8 files changed, 27 insertions(+), 75 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 06a8b7156d05..dcdbfa53f1bb 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -209,13 +209,8 @@ "type": "string" }, "path": { - "allOf": [ - { - "$ref": "#/definitions/PathUri" - } - ], - "description": "Canonical `file:` URI for the root in the selected environment.", - "pattern": "^file:" + "description": "Absolute path for the root in the selected environment.", + "type": "string" }, "type": { "enum": [ @@ -1793,9 +1788,6 @@ ], "type": "string" }, - "PathUri": { - "type": "string" - }, "PermissionProfileListParams": { "properties": { "cursor": { @@ -6842,4 +6834,4 @@ } ], "title": "ClientRequest" -} \ No newline at end of file +} 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 51bd7bcaefc5..3cecc0e11600 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 @@ -7042,15 +7042,10 @@ "environmentId": { "type": "string" }, - "path": { - "allOf": [ - { - "$ref": "#/definitions/v2/PathUri" - } - ], - "description": "Canonical `file:` URI for the root in the selected environment.", - "pattern": "^file:" - }, + "path": { + "description": "Absolute path for the root in the selected environment.", + "type": "string" + }, "type": { "enum": [ "environment" @@ -13005,9 +13000,6 @@ } ] }, - "PathUri": { - "type": "string" - }, "PermissionProfileListParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { @@ -20698,4 +20690,4 @@ }, "title": "CodexAppServerProtocol", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 58dff20db239..b94593a009de 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -1169,13 +1169,8 @@ "type": "string" }, "path": { - "allOf": [ - { - "$ref": "#/definitions/PathUri" - } - ], - "description": "Canonical `file:` URI for the root in the selected environment.", - "pattern": "^file:" + "description": "Absolute path for the root in the selected environment.", + "type": "string" }, "type": { "enum": [ @@ -9409,9 +9404,6 @@ } ] }, - "PathUri": { - "type": "string" - }, "PermissionProfileListParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { @@ -18476,4 +18468,4 @@ }, "title": "CodexAppServerProtocolV2", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json index 79c5bfbc6530..e447c644d741 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json @@ -73,13 +73,8 @@ "type": "string" }, "path": { - "allOf": [ - { - "$ref": "#/definitions/PathUri" - } - ], - "description": "Canonical `file:` URI for the root in the selected environment.", - "pattern": "^file:" + "description": "Absolute path for the root in the selected environment.", + "type": "string" }, "type": { "enum": [ @@ -208,9 +203,6 @@ ], "type": "string" }, - "PathUri": { - "type": "string" - }, "Personality": { "enum": [ "none", @@ -396,4 +388,4 @@ }, "title": "ThreadStartParams", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CapabilityRootLocation.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CapabilityRootLocation.ts index f2bccf3a37d3..6c2ac9082efd 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CapabilityRootLocation.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CapabilityRootLocation.ts @@ -7,6 +7,6 @@ */ export type CapabilityRootLocation = { "type": "environment", environmentId: string, /** - * Canonical `file:` URI for the root in the selected environment. + * Absolute path for the root in the selected environment. */ path: string, }; diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index e786fc3542a5..d9357313b24f 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -137,7 +137,7 @@ Example with notification opt-out: ## API Overview -- `thread/start` — create a new thread; emits `thread/started` (including the current `thread.status`) and auto-subscribes you to turn/item events for that thread. When the request includes a `cwd` and the resolved sandbox is `workspace-write` or full access, app-server also marks that project as trusted in the user `config.toml`. Pass `sessionStartSource: "clear"` when starting a replacement thread after clearing the current session so `SessionStart` hooks receive `source: "clear"` instead of the default `"startup"`. Experimental `runtimeWorkspaceRoots` replaces the thread-scoped runtime workspace roots used to materialize `:workspace_roots`; paths must be absolute. For permissions, prefer experimental `permissions` profile selection by id; the legacy `sandbox` shorthand is still accepted but cannot be combined with `permissions`. Experimental `multiAgentMode` selects the initial thread mode and defaults to `explicitRequestOnly` when omitted; use `none` to keep multi-agent tools available without injecting mode instructions. Experimental `environments` selects the sticky execution environments for turns on the thread; omit it to use the server default, pass `[]` to disable environments, or pass explicit environment ids with per-environment `cwd`. Experimental `selectedCapabilityRoots` selects environment-owned plugin or standalone-skill roots using `file:` URIs. Skills found below those roots are listed and read through the owning environment. Stdio MCP servers declared by selected plugins are also started in that environment; HTTP MCP declarations remain inactive. +- `thread/start` — create a new thread; emits `thread/started` (including the current `thread.status`) and auto-subscribes you to turn/item events for that thread. When the request includes a `cwd` and the resolved sandbox is `workspace-write` or full access, app-server also marks that project as trusted in the user `config.toml`. Pass `sessionStartSource: "clear"` when starting a replacement thread after clearing the current session so `SessionStart` hooks receive `source: "clear"` instead of the default `"startup"`. Experimental `runtimeWorkspaceRoots` replaces the thread-scoped runtime workspace roots used to materialize `:workspace_roots`; paths must be absolute. For permissions, prefer experimental `permissions` profile selection by id; the legacy `sandbox` shorthand is still accepted but cannot be combined with `permissions`. Experimental `multiAgentMode` selects the initial thread mode and defaults to `explicitRequestOnly` when omitted; use `none` to keep multi-agent tools available without injecting mode instructions. Experimental `environments` selects the sticky execution environments for turns on the thread; omit it to use the server default, pass `[]` to disable environments, or pass explicit environment ids with per-environment `cwd`. Experimental `selectedCapabilityRoots` selects environment-owned plugin or standalone-skill roots using environment-native absolute paths. Skills found below those roots are listed and read through the owning environment. Stdio MCP servers declared by selected plugins are also started in that environment; HTTP MCP declarations remain inactive. - `thread/resume` — reopen an existing thread by id so subsequent `turn/start` calls append to it. Accepts the same permission override rules as `thread/start`. Multi-agent mode restores the last effective mode from rollout history when available; clients can select another mode on the first `turn/start`. - `thread/fork` — fork an existing thread into a new thread id by copying the stored history; if the source thread is currently mid-turn, the fork records the same interruption marker as `turn/interrupt` instead of inheriting an unmarked partial turn suffix. The returned `thread.forkedFromId` points at the source thread when known. Accepts `ephemeral: true` for an in-memory temporary fork, emits `thread/started` (including the current `thread.status`), and auto-subscribes you to turn/item events for the new thread. Experimental clients can pass `excludeTurns: true` when they plan to page fork history via `thread/turns/list` instead of receiving the full turn array immediately. Accepts the same permission override rules as `thread/start`. - `thread/start`, `thread/resume`, and `thread/fork` responses include the legacy `sandbox` compatibility projection. `instructionSources` lists loaded instruction files using each source environment's native absolute path syntax, including files loaded from remote environments. Experimental clients can read `runtimeWorkspaceRoots` for the thread-scoped runtime roots and `activePermissionProfile` for the named or implicit built-in profile identity/provenance when known. Their experimental `multiAgentMode` field, and the corresponding thread setting, report the thread's current mode. Turn construction separately determines whether that mode is applicable to the selected model and runtime configuration. @@ -267,7 +267,7 @@ Start a fresh thread when you need a new Codex conversation. "location": { "type": "environment", "environmentId": "workspace", - "path": "file:///opt/cca/plugins/github" + "path": "/opt/cca/plugins/github" } } ], diff --git a/codex-rs/protocol/src/capabilities.rs b/codex-rs/protocol/src/capabilities.rs index 8430244fc926..f9fe8dcc63a4 100644 --- a/codex-rs/protocol/src/capabilities.rs +++ b/codex-rs/protocol/src/capabilities.rs @@ -1,3 +1,4 @@ +use codex_utils_path_uri::LegacyAppPathString; use codex_utils_path_uri::PathUri; use schemars::JsonSchema; use serde::Deserialize; @@ -26,20 +27,23 @@ pub enum CapabilityRootLocation { #[serde(rename = "environmentId")] #[ts(rename = "environmentId")] environment_id: String, - /// Canonical `file:` URI for the root in the selected environment. - #[serde(deserialize_with = "deserialize_strict_path_uri")] - #[schemars(regex(pattern = r"^file:"))] + /// Absolute path for the root in the selected environment. + #[serde(deserialize_with = "deserialize_path_uri_from_api_path")] + #[schemars(with = "String")] #[ts(type = "string")] path: PathUri, }, } -fn deserialize_strict_path_uri<'de, D>(deserializer: D) -> Result +fn deserialize_path_uri_from_api_path<'de, D>(deserializer: D) -> Result where D: serde::Deserializer<'de>, { - let path = String::deserialize(deserializer)?; - PathUri::parse(&path).map_err(serde::de::Error::custom) + let path = LegacyAppPathString::deserialize(deserializer)?; + if let Ok(path_uri) = PathUri::parse(path.as_str()) { + return Ok(path_uri); + } + path.try_into().map_err(serde::de::Error::custom) } #[cfg(test)] diff --git a/codex-rs/protocol/src/capabilities_tests.rs b/codex-rs/protocol/src/capabilities_tests.rs index c31b022c2275..05f713ee85c3 100644 --- a/codex-rs/protocol/src/capabilities_tests.rs +++ b/codex-rs/protocol/src/capabilities_tests.rs @@ -3,26 +3,6 @@ use super::SelectedCapabilityRoot; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; -#[test] -fn environment_capability_root_requires_path_uri_on_the_wire() { - let err = serde_json::from_str::( - r#"{ - "id": "selected-demo", - "location": { - "type": "environment", - "environmentId": "executor-test", - "path": "/plugins/demo" - } - }"#, - ) - .expect_err("native paths should be rejected"); - - assert!( - err.to_string().contains("relative URL without a base"), - "unexpected error: {err}" - ); -} - #[test] fn environment_capability_root_accepts_foreign_file_uri() { let selected_root = serde_json::from_str::(