From ae15343243eecf363757ba9d8117e824ab718927 Mon Sep 17 00:00:00 2001 From: xl-openai Date: Thu, 7 May 2026 21:28:18 -0700 Subject: [PATCH] feat: Update plugin share settings with discoverability (#21637) Requires discoverability on plugin/share/updateTargets so the server can manage workspace link access consistently, including auto-adding the workspace principal for UNLISTED. Also rejects LISTED on share creation and blocks client-supplied workspace principals while preserving response parsing for LISTED. --- .../schema/json/ClientRequest.json | 13 +- .../codex_app_server_protocol.schemas.json | 17 +- .../codex_app_server_protocol.v2.schemas.json | 17 +- .../v2/PluginShareUpdateTargetsParams.json | 11 ++ .../v2/PluginShareUpdateTargetsResponse.json | 12 ++ .../v2/PluginShareUpdateDiscoverability.ts | 5 + .../v2/PluginShareUpdateTargetsParams.ts | 3 +- .../v2/PluginShareUpdateTargetsResponse.ts | 3 +- .../schema/typescript/v2/index.ts | 1 + .../src/protocol/v2/plugin.rs | 13 ++ .../src/protocol/v2/tests.rs | 4 + codex-rs/app-server/src/request_processors.rs | 1 + .../src/request_processors/plugins.rs | 64 +++++++- .../app-server/tests/suite/v2/plugin_share.rs | 149 ++++++++++++++++-- codex-rs/core-plugins/src/remote.rs | 1 + codex-rs/core-plugins/src/remote/share.rs | 68 +++++++- .../core-plugins/src/remote/share/tests.rs | 88 +++++++++-- 17 files changed, 436 insertions(+), 34 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/PluginShareUpdateDiscoverability.ts diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index d3f4d44b7..fe3738c88 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -2091,8 +2091,18 @@ ], "type": "object" }, + "PluginShareUpdateDiscoverability": { + "enum": [ + "UNLISTED", + "PRIVATE" + ], + "type": "string" + }, "PluginShareUpdateTargetsParams": { "properties": { + "discoverability": { + "$ref": "#/definitions/PluginShareUpdateDiscoverability" + }, "remotePluginId": { "type": "string" }, @@ -2104,6 +2114,7 @@ } }, "required": [ + "discoverability", "remotePluginId", "shareTargets" ], @@ -6177,4 +6188,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 e81ba3f05..156f6ddc4 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 @@ -12414,9 +12414,19 @@ ], "type": "object" }, + "PluginShareUpdateDiscoverability": { + "enum": [ + "UNLISTED", + "PRIVATE" + ], + "type": "string" + }, "PluginShareUpdateTargetsParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { + "discoverability": { + "$ref": "#/definitions/v2/PluginShareUpdateDiscoverability" + }, "remotePluginId": { "type": "string" }, @@ -12428,6 +12438,7 @@ } }, "required": [ + "discoverability", "remotePluginId", "shareTargets" ], @@ -12437,6 +12448,9 @@ "PluginShareUpdateTargetsResponse": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { + "discoverability": { + "$ref": "#/definitions/v2/PluginShareDiscoverability" + }, "principals": { "items": { "$ref": "#/definitions/v2/PluginSharePrincipal" @@ -12445,6 +12459,7 @@ } }, "required": [ + "discoverability", "principals" ], "title": "PluginShareUpdateTargetsResponse", @@ -18396,4 +18411,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 4c9ae5010..3c5eb030c 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 @@ -9007,9 +9007,19 @@ ], "type": "object" }, + "PluginShareUpdateDiscoverability": { + "enum": [ + "UNLISTED", + "PRIVATE" + ], + "type": "string" + }, "PluginShareUpdateTargetsParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { + "discoverability": { + "$ref": "#/definitions/PluginShareUpdateDiscoverability" + }, "remotePluginId": { "type": "string" }, @@ -9021,6 +9031,7 @@ } }, "required": [ + "discoverability", "remotePluginId", "shareTargets" ], @@ -9030,6 +9041,9 @@ "PluginShareUpdateTargetsResponse": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { + "discoverability": { + "$ref": "#/definitions/PluginShareDiscoverability" + }, "principals": { "items": { "$ref": "#/definitions/PluginSharePrincipal" @@ -9038,6 +9052,7 @@ } }, "required": [ + "discoverability", "principals" ], "title": "PluginShareUpdateTargetsResponse", @@ -16263,4 +16278,4 @@ }, "title": "CodexAppServerProtocolV2", "type": "object" -} +} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/json/v2/PluginShareUpdateTargetsParams.json b/codex-rs/app-server-protocol/schema/json/v2/PluginShareUpdateTargetsParams.json index 080ec980a..f6b44c92e 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/PluginShareUpdateTargetsParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/PluginShareUpdateTargetsParams.json @@ -23,9 +23,19 @@ "principalType" ], "type": "object" + }, + "PluginShareUpdateDiscoverability": { + "enum": [ + "UNLISTED", + "PRIVATE" + ], + "type": "string" } }, "properties": { + "discoverability": { + "$ref": "#/definitions/PluginShareUpdateDiscoverability" + }, "remotePluginId": { "type": "string" }, @@ -37,6 +47,7 @@ } }, "required": [ + "discoverability", "remotePluginId", "shareTargets" ], diff --git a/codex-rs/app-server-protocol/schema/json/v2/PluginShareUpdateTargetsResponse.json b/codex-rs/app-server-protocol/schema/json/v2/PluginShareUpdateTargetsResponse.json index 28fcb665b..fe47f1f4a 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/PluginShareUpdateTargetsResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/PluginShareUpdateTargetsResponse.json @@ -1,6 +1,14 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "PluginShareDiscoverability": { + "enum": [ + "LISTED", + "UNLISTED", + "PRIVATE" + ], + "type": "string" + }, "PluginSharePrincipal": { "properties": { "name": { @@ -30,6 +38,9 @@ } }, "properties": { + "discoverability": { + "$ref": "#/definitions/PluginShareDiscoverability" + }, "principals": { "items": { "$ref": "#/definitions/PluginSharePrincipal" @@ -38,6 +49,7 @@ } }, "required": [ + "discoverability", "principals" ], "title": "PluginShareUpdateTargetsResponse", diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/PluginShareUpdateDiscoverability.ts b/codex-rs/app-server-protocol/schema/typescript/v2/PluginShareUpdateDiscoverability.ts new file mode 100644 index 000000000..fd601987a --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/PluginShareUpdateDiscoverability.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type PluginShareUpdateDiscoverability = "UNLISTED" | "PRIVATE"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/PluginShareUpdateTargetsParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/PluginShareUpdateTargetsParams.ts index 53ef2b302..eecd4be82 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/PluginShareUpdateTargetsParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/PluginShareUpdateTargetsParams.ts @@ -2,5 +2,6 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { PluginShareTarget } from "./PluginShareTarget"; +import type { PluginShareUpdateDiscoverability } from "./PluginShareUpdateDiscoverability"; -export type PluginShareUpdateTargetsParams = { remotePluginId: string, shareTargets: Array, }; +export type PluginShareUpdateTargetsParams = { remotePluginId: string, discoverability: PluginShareUpdateDiscoverability, shareTargets: Array, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/PluginShareUpdateTargetsResponse.ts b/codex-rs/app-server-protocol/schema/typescript/v2/PluginShareUpdateTargetsResponse.ts index 3d6f6e9ee..0ce722460 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/PluginShareUpdateTargetsResponse.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/PluginShareUpdateTargetsResponse.ts @@ -1,6 +1,7 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { PluginShareDiscoverability } from "./PluginShareDiscoverability"; import type { PluginSharePrincipal } from "./PluginSharePrincipal"; -export type PluginShareUpdateTargetsResponse = { principals: Array, }; +export type PluginShareUpdateTargetsResponse = { principals: Array, discoverability: PluginShareDiscoverability, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index 0918008cd..3cd919cb9 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -287,6 +287,7 @@ export type { PluginSharePrincipalType } from "./PluginSharePrincipalType"; export type { PluginShareSaveParams } from "./PluginShareSaveParams"; export type { PluginShareSaveResponse } from "./PluginShareSaveResponse"; export type { PluginShareTarget } from "./PluginShareTarget"; +export type { PluginShareUpdateDiscoverability } from "./PluginShareUpdateDiscoverability"; export type { PluginShareUpdateTargetsParams } from "./PluginShareUpdateTargetsParams"; export type { PluginShareUpdateTargetsResponse } from "./PluginShareUpdateTargetsResponse"; export type { PluginSkillReadParams } from "./PluginSkillReadParams"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs b/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs index 0d31e463b..6f425b4a6 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs @@ -218,6 +218,7 @@ pub struct PluginShareSaveResponse { #[ts(export_to = "v2/")] pub struct PluginShareUpdateTargetsParams { pub remote_plugin_id: String, + pub discoverability: PluginShareUpdateDiscoverability, pub share_targets: Vec, } @@ -226,6 +227,7 @@ pub struct PluginShareUpdateTargetsParams { #[ts(export_to = "v2/")] pub struct PluginShareUpdateTargetsResponse { pub principals: Vec, + pub discoverability: PluginShareDiscoverability, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -275,6 +277,17 @@ pub enum PluginShareDiscoverability { Private, } +#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] +#[ts(export_to = "v2/")] +pub enum PluginShareUpdateDiscoverability { + #[serde(rename = "UNLISTED")] + #[ts(rename = "UNLISTED")] + Unlisted, + #[serde(rename = "PRIVATE")] + #[ts(rename = "PRIVATE")] + Private, +} + #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] #[ts(export_to = "v2/")] pub enum PluginSharePrincipalType { diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index cbbae5161..da0ad2c10 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -2936,6 +2936,7 @@ fn plugin_share_params_and_response_serialization_use_camel_case_fields() { assert_eq!( serde_json::to_value(PluginShareUpdateTargetsParams { remote_plugin_id: "plugins~Plugin_00000000000000000000000000000000".to_string(), + discoverability: PluginShareUpdateDiscoverability::Unlisted, share_targets: vec![PluginShareTarget { principal_type: PluginSharePrincipalType::Group, principal_id: "group-1".to_string(), @@ -2944,6 +2945,7 @@ fn plugin_share_params_and_response_serialization_use_camel_case_fields() { .unwrap(), json!({ "remotePluginId": "plugins~Plugin_00000000000000000000000000000000", + "discoverability": "UNLISTED", "shareTargets": [{ "principalType": "group", "principalId": "group-1", @@ -2958,6 +2960,7 @@ fn plugin_share_params_and_response_serialization_use_camel_case_fields() { principal_id: "user-1".to_string(), name: "Gavin".to_string(), }], + discoverability: PluginShareDiscoverability::Unlisted, }) .unwrap(), json!({ @@ -2966,6 +2969,7 @@ fn plugin_share_params_and_response_serialization_use_camel_case_fields() { "principalId": "user-1", "name": "Gavin", }], + "discoverability": "UNLISTED", }), ); diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index 19201b8ae..cfd2589df 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -124,6 +124,7 @@ use codex_app_server_protocol::PluginSharePrincipalType; use codex_app_server_protocol::PluginShareSaveParams; use codex_app_server_protocol::PluginShareSaveResponse; use codex_app_server_protocol::PluginShareTarget; +use codex_app_server_protocol::PluginShareUpdateDiscoverability; use codex_app_server_protocol::PluginShareUpdateTargetsParams; use codex_app_server_protocol::PluginShareUpdateTargetsResponse; use codex_app_server_protocol::PluginSkillReadParams; diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index cc5de4058..65bb39085 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -133,6 +133,33 @@ fn remote_plugin_share_discoverability( } } +fn remote_plugin_share_update_discoverability( + discoverability: PluginShareUpdateDiscoverability, +) -> codex_core_plugins::remote::RemotePluginShareUpdateDiscoverability { + match discoverability { + PluginShareUpdateDiscoverability::Unlisted => { + codex_core_plugins::remote::RemotePluginShareUpdateDiscoverability::Unlisted + } + PluginShareUpdateDiscoverability::Private => { + codex_core_plugins::remote::RemotePluginShareUpdateDiscoverability::Private + } + } +} + +fn validate_client_plugin_share_targets( + targets: &[PluginShareTarget], +) -> Result<(), JSONRPCErrorError> { + if targets + .iter() + .any(|target| target.principal_type == PluginSharePrincipalType::Workspace) + { + return Err(invalid_request( + "shareTargets cannot include workspace principals; use discoverability UNLISTED for workspace link access", + )); + } + Ok(()) +} + fn remote_plugin_share_targets( targets: Vec, ) -> Vec { @@ -729,9 +756,17 @@ impl PluginRequestProcessor { } if remote_plugin_id.is_some() && (discoverability.is_some() || share_targets.is_some()) { return Err(invalid_request( - "discoverability and shareTargets are only supported when creating a plugin share; use plugin/share/updateTargets to update share targets", + "discoverability and shareTargets are only supported when creating a plugin share; use plugin/share/updateTargets to update share settings", )); } + if discoverability == Some(PluginShareDiscoverability::Listed) { + return Err(invalid_request( + "discoverability LISTED is not supported for plugin/share/save; use UNLISTED or PRIVATE", + )); + } + if let Some(share_targets) = share_targets.as_ref() { + validate_client_plugin_share_targets(share_targets)?; + } let remote_plugin_service_config = RemotePluginServiceConfig { chatgpt_base_url: config.chatgpt_base_url.clone(), @@ -765,11 +800,14 @@ impl PluginRequestProcessor { let (config, auth) = self.load_plugin_share_config_and_auth().await?; let PluginShareUpdateTargetsParams { remote_plugin_id, + discoverability, share_targets, } = params; if remote_plugin_id.is_empty() || !is_valid_remote_plugin_id(&remote_plugin_id) { return Err(invalid_request("invalid remote plugin id")); } + validate_client_plugin_share_targets(&share_targets)?; + let requested_share_targets = share_targets.clone(); let remote_plugin_service_config = RemotePluginServiceConfig { chatgpt_base_url: config.chatgpt_base_url.clone(), @@ -779,6 +817,7 @@ impl PluginRequestProcessor { auth.as_ref(), &remote_plugin_id, remote_plugin_share_targets(share_targets), + remote_plugin_share_update_discoverability(discoverability), ) .await .map_err(|err| { @@ -790,7 +829,14 @@ impl PluginRequestProcessor { .principals .into_iter() .map(plugin_share_principal_from_remote) + .filter(|principal| { + requested_share_targets.iter().any(|target| { + target.principal_type == principal.principal_type + && target.principal_id == principal.principal_id + }) + }) .collect(), + discoverability: remote_plugin_share_discoverability_to_info(result.discoverability), }) } @@ -1487,6 +1533,22 @@ fn remote_plugin_share_context_to_info( } } +fn remote_plugin_share_discoverability_to_info( + discoverability: codex_core_plugins::remote::RemotePluginShareDiscoverability, +) -> PluginShareDiscoverability { + match discoverability { + codex_core_plugins::remote::RemotePluginShareDiscoverability::Listed => { + PluginShareDiscoverability::Listed + } + codex_core_plugins::remote::RemotePluginShareDiscoverability::Unlisted => { + PluginShareDiscoverability::Unlisted + } + codex_core_plugins::remote::RemotePluginShareDiscoverability::Private => { + PluginShareDiscoverability::Private + } + } +} + fn remote_plugin_detail_to_info( detail: RemoteCatalogPluginDetail, apps: Vec, diff --git a/codex-rs/app-server/tests/suite/v2/plugin_share.rs b/codex-rs/app-server/tests/suite/v2/plugin_share.rs index 09b39ca24..dc1f56d48 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_share.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_share.rs @@ -219,7 +219,7 @@ async fn plugin_share_save_forwards_access_policy() -> Result<()> { .and(body_json(json!({ "file_id": "file_123", "etag": "\"upload_etag_123\"", - "discoverability": "PRIVATE", + "discoverability": "UNLISTED", "share_targets": [ { "principal_type": "user", @@ -227,7 +227,7 @@ async fn plugin_share_save_forwards_access_policy() -> Result<()> { }, { "principal_type": "workspace", - "principal_id": "workspace-1", + "principal_id": "account-123", }, ], }))) @@ -247,16 +247,12 @@ async fn plugin_share_save_forwards_access_policy() -> Result<()> { "plugin/share/save", Some(json!({ "pluginPath": expected_plugin_path, - "discoverability": "PRIVATE", + "discoverability": "UNLISTED", "shareTargets": [ { "principalType": "user", "principalId": "user-1", }, - { - "principalType": "workspace", - "principalId": "workspace-1", - }, ], })), ) @@ -279,6 +275,124 @@ async fn plugin_share_save_forwards_access_policy() -> Result<()> { Ok(()) } +#[tokio::test] +async fn plugin_share_save_rejects_listed_discoverability() -> Result<()> { + let codex_home = TempDir::new()?; + let plugin_root = TempDir::new()?; + let plugin_path = write_test_plugin(plugin_root.path(), "demo-plugin")?; + let server = MockServer::start().await; + write_remote_plugin_config(codex_home.path(), &format!("{}/backend-api", server.uri()))?; + write_chatgpt_auth( + codex_home.path(), + ChatGptAuthFixture::new("chatgpt-token") + .account_id("account-123") + .chatgpt_user_id("user-123") + .chatgpt_account_id("account-123"), + AuthCredentialsStoreMode::File, + )?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + let request_id = mcp + .send_raw_request( + "plugin/share/save", + Some(json!({ + "pluginPath": AbsolutePathBuf::try_from(plugin_path)?, + "discoverability": "LISTED", + })), + ) + .await?; + + let error: JSONRPCError = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_error_message(RequestId::Integer(request_id)), + ) + .await??; + + assert_eq!(error.error.code, -32600); + assert_eq!( + error.error.message, + "discoverability LISTED is not supported for plugin/share/save; use UNLISTED or PRIVATE" + ); + Ok(()) +} + +#[tokio::test] +async fn plugin_share_rejects_workspace_targets_from_client() -> Result<()> { + let codex_home = TempDir::new()?; + let plugin_root = TempDir::new()?; + let plugin_path = write_test_plugin(plugin_root.path(), "demo-plugin")?; + let server = MockServer::start().await; + write_remote_plugin_config(codex_home.path(), &format!("{}/backend-api", server.uri()))?; + write_chatgpt_auth( + codex_home.path(), + ChatGptAuthFixture::new("chatgpt-token") + .account_id("account-123") + .chatgpt_user_id("user-123") + .chatgpt_account_id("account-123"), + AuthCredentialsStoreMode::File, + )?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + let request_id = mcp + .send_raw_request( + "plugin/share/save", + Some(json!({ + "pluginPath": AbsolutePathBuf::try_from(plugin_path)?, + "discoverability": "UNLISTED", + "shareTargets": [ + { + "principalType": "workspace", + "principalId": "account-123", + }, + ], + })), + ) + .await?; + + let error: JSONRPCError = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_error_message(RequestId::Integer(request_id)), + ) + .await??; + + assert_eq!(error.error.code, -32600); + assert_eq!( + error.error.message, + "shareTargets cannot include workspace principals; use discoverability UNLISTED for workspace link access" + ); + + let request_id = mcp + .send_raw_request( + "plugin/share/updateTargets", + Some(json!({ + "remotePluginId": "plugins_123", + "discoverability": "UNLISTED", + "shareTargets": [ + { + "principalType": "workspace", + "principalId": "account-123", + }, + ], + })), + ) + .await?; + + let error: JSONRPCError = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_error_message(RequestId::Integer(request_id)), + ) + .await??; + + assert_eq!(error.error.code, -32600); + assert_eq!( + error.error.message, + "shareTargets cannot include workspace principals; use discoverability UNLISTED for workspace link access" + ); + Ok(()) +} + #[tokio::test] async fn plugin_share_save_rejects_access_policy_for_existing_plugin() -> Result<()> { let codex_home = TempDir::new()?; @@ -323,7 +437,7 @@ async fn plugin_share_save_rejects_access_policy_for_existing_plugin() -> Result assert_eq!(error.error.code, -32600); assert_eq!( error.error.message, - "discoverability and shareTargets are only supported when creating a plugin share; use plugin/share/updateTargets to update share targets" + "discoverability and shareTargets are only supported when creating a plugin share; use plugin/share/updateTargets to update share settings" ); Ok(()) } @@ -420,24 +534,39 @@ async fn plugin_share_update_targets_updates_share_targets() -> Result<()> { )?; Mock::given(method("PUT")) - .and(path("/backend-api/public/plugins/plugins_123/shares")) + .and(path("/backend-api/ps/plugins/plugins_123/shares")) .and(header("authorization", "Bearer chatgpt-token")) .and(header("chatgpt-account-id", "account-123")) .and(body_json(json!({ + "discoverability": "UNLISTED", "targets": [ { "principal_type": "user", "principal_id": "user-1", }, + { + "principal_type": "workspace", + "principal_id": "account-123", + }, ], }))) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "principals": [ + { + "principal_type": "user", + "principal_id": "owner-1", + "name": "Owner", + }, { "principal_type": "user", "principal_id": "user-1", "name": "Gavin", }, + { + "principal_type": "workspace", + "principal_id": "account-123", + "name": "Workspace", + }, ], }))) .expect(1) @@ -451,6 +580,7 @@ async fn plugin_share_update_targets_updates_share_targets() -> Result<()> { "plugin/share/updateTargets", Some(json!({ "remotePluginId": "plugins_123", + "discoverability": "UNLISTED", "shareTargets": [ { "principalType": "user", @@ -476,6 +606,7 @@ async fn plugin_share_update_targets_updates_share_targets() -> Result<()> { principal_id: "user-1".to_string(), name: "Gavin".to_string(), }], + discoverability: codex_app_server_protocol::PluginShareDiscoverability::Unlisted, } ); Ok(()) diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index b2da554fd..431697628 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -35,6 +35,7 @@ pub use share::RemotePluginSharePrincipal; pub use share::RemotePluginSharePrincipalType; pub use share::RemotePluginShareSaveResult; pub use share::RemotePluginShareTarget; +pub use share::RemotePluginShareUpdateDiscoverability; pub use share::RemotePluginShareUpdateTargetsResult; pub use share::delete_remote_plugin_share; pub use share::list_remote_plugin_shares; diff --git a/codex-rs/core-plugins/src/remote/share.rs b/codex-rs/core-plugins/src/remote/share.rs index 22751a7ad..d69d22ea5 100644 --- a/codex-rs/core-plugins/src/remote/share.rs +++ b/codex-rs/core-plugins/src/remote/share.rs @@ -32,7 +32,7 @@ pub struct RemotePluginShareAccessPolicy { pub share_targets: Option>, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "SCREAMING_SNAKE_CASE")] pub enum RemotePluginShareDiscoverability { Listed, @@ -40,6 +40,13 @@ pub enum RemotePluginShareDiscoverability { Private, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "SCREAMING_SNAKE_CASE")] +pub enum RemotePluginShareUpdateDiscoverability { + Unlisted, + Private, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] pub enum RemotePluginSharePrincipalType { @@ -64,6 +71,7 @@ pub struct RemotePluginSharePrincipal { #[derive(Debug, Clone, PartialEq, Eq)] pub struct RemotePluginShareUpdateTargetsResult { pub principals: Vec, + pub discoverability: RemotePluginShareDiscoverability, } #[derive(Debug, Clone, PartialEq, Eq, Serialize)] @@ -100,12 +108,14 @@ struct RemoteWorkspacePluginCreateResponse { #[derive(Debug, Clone, PartialEq, Eq, Serialize)] struct RemotePluginShareUpdateTargetsRequest { + discoverability: RemotePluginShareUpdateDiscoverability, targets: Vec, } #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] struct RemotePluginShareUpdateTargetsResponse { principals: Vec, + discoverability: Option, } pub async fn save_remote_plugin_share( @@ -137,6 +147,9 @@ pub async fn save_remote_plugin_share( .etag .ok_or(RemotePluginCatalogError::MissingUploadEtag)?; put_workspace_plugin_upload(&upload.upload_url, archive_bytes).await?; + let share_targets = access_policy.share_targets; + let share_targets = + ensure_unlisted_workspace_target(auth, access_policy.discoverability, share_targets)?; let response = finalize_workspace_plugin_upload( config, auth, @@ -145,7 +158,7 @@ pub async fn save_remote_plugin_share( file_id: upload.file_id, etag, discoverability: access_policy.discoverability, - share_targets: access_policy.share_targets, + share_targets, }, ) .await?; @@ -245,19 +258,64 @@ pub async fn update_remote_plugin_share_targets( auth: Option<&CodexAuth>, remote_plugin_id: &str, targets: Vec, + discoverability: RemotePluginShareUpdateDiscoverability, ) -> Result { let auth = ensure_chatgpt_auth(auth)?; + let target_discoverability = match discoverability { + RemotePluginShareUpdateDiscoverability::Unlisted => { + RemotePluginShareDiscoverability::Unlisted + } + RemotePluginShareUpdateDiscoverability::Private => { + RemotePluginShareDiscoverability::Private + } + }; + let targets = + ensure_unlisted_workspace_target(auth, Some(target_discoverability), Some(targets))? + .unwrap_or_default(); let base_url = config.chatgpt_base_url.trim_end_matches('/'); - let url = format!("{base_url}/public/plugins/{remote_plugin_id}/shares"); + let url = format!("{base_url}/ps/plugins/{remote_plugin_id}/shares"); let client = build_reqwest_client(); - let request = authenticated_request(client.put(&url), auth)? - .json(&RemotePluginShareUpdateTargetsRequest { targets }); + let request = authenticated_request(client.put(&url), auth)?.json( + &RemotePluginShareUpdateTargetsRequest { + discoverability, + targets, + }, + ); let response: RemotePluginShareUpdateTargetsResponse = send_and_decode(request, &url).await?; Ok(RemotePluginShareUpdateTargetsResult { principals: response.principals, + // TODO: Remove this fallback once deployed plugin-service responses always include + // discoverability per the API schema. + discoverability: response.discoverability.unwrap_or(target_discoverability), }) } +fn ensure_unlisted_workspace_target( + auth: &CodexAuth, + discoverability: Option, + targets: Option>, +) -> Result>, RemotePluginCatalogError> { + if discoverability != Some(RemotePluginShareDiscoverability::Unlisted) { + return Ok(targets); + } + let account_id = auth.get_account_id().ok_or_else(|| { + RemotePluginCatalogError::UnexpectedResponse( + "workspace plugin share requires an account id".to_string(), + ) + })?; + let mut targets = targets.unwrap_or_default(); + if !targets.iter().any(|target| { + target.principal_type == RemotePluginSharePrincipalType::Workspace + && target.principal_id == account_id + }) { + targets.push(RemotePluginShareTarget { + principal_type: RemotePluginSharePrincipalType::Workspace, + principal_id: account_id, + }); + } + Ok(Some(targets)) +} + async fn fetch_created_workspace_plugins( config: &RemotePluginServiceConfig, auth: &CodexAuth, diff --git a/codex-rs/core-plugins/src/remote/share/tests.rs b/codex-rs/core-plugins/src/remote/share/tests.rs index 84e698f9d..35909a8b1 100644 --- a/codex-rs/core-plugins/src/remote/share/tests.rs +++ b/codex-rs/core-plugins/src/remote/share/tests.rs @@ -204,7 +204,7 @@ async fn save_remote_plugin_share_creates_workspace_plugin() { .and(body_json(json!({ "file_id": "file_123", "etag": "\"upload_etag_123\"", - "discoverability": "PRIVATE", + "discoverability": "UNLISTED", "share_targets": [ { "principal_type": "user", @@ -212,7 +212,7 @@ async fn save_remote_plugin_share_creates_workspace_plugin() { }, { "principal_type": "workspace", - "principal_id": "workspace-1", + "principal_id": "account_id", }, ], }))) @@ -231,17 +231,11 @@ async fn save_remote_plugin_share_creates_workspace_plugin() { &plugin_path, /*remote_plugin_id*/ None, RemotePluginShareAccessPolicy { - discoverability: Some(RemotePluginShareDiscoverability::Private), - share_targets: Some(vec![ - RemotePluginShareTarget { - principal_type: RemotePluginSharePrincipalType::User, - principal_id: "user-1".to_string(), - }, - RemotePluginShareTarget { - principal_type: RemotePluginSharePrincipalType::Workspace, - principal_id: "workspace-1".to_string(), - }, - ]), + discoverability: Some(RemotePluginShareDiscoverability::Unlisted), + share_targets: Some(vec![RemotePluginShareTarget { + principal_type: RemotePluginSharePrincipalType::User, + principal_id: "user-1".to_string(), + }]), }, ) .await @@ -401,10 +395,11 @@ async fn update_remote_plugin_share_targets_updates_targets() { let auth = test_auth(); Mock::given(method("PUT")) - .and(path("/backend-api/public/plugins/plugins_123/shares")) + .and(path("/backend-api/ps/plugins/plugins_123/shares")) .and(header("authorization", "Bearer Access Token")) .and(header("chatgpt-account-id", "account_id")) .and(body_json(json!({ + "discoverability": "UNLISTED", "targets": [ { "principal_type": "user", @@ -414,6 +409,10 @@ async fn update_remote_plugin_share_targets_updates_targets() { "principal_type": "group", "principal_id": "group-1", }, + { + "principal_type": "workspace", + "principal_id": "account_id", + }, ], }))) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ @@ -429,6 +428,7 @@ async fn update_remote_plugin_share_targets_updates_targets() { "name": "Engineering", }, ], + "discoverability": "UNLISTED", }))) .expect(1) .mount(&server) @@ -448,6 +448,7 @@ async fn update_remote_plugin_share_targets_updates_targets() { principal_id: "group-1".to_string(), }, ], + RemotePluginShareUpdateDiscoverability::Unlisted, ) .await .unwrap(); @@ -467,6 +468,65 @@ async fn update_remote_plugin_share_targets_updates_targets() { name: "Engineering".to_string(), }, ], + discoverability: RemotePluginShareDiscoverability::Unlisted, + } + ); +} + +#[tokio::test] +async fn update_remote_plugin_share_targets_falls_back_to_requested_discoverability() { + let server = MockServer::start().await; + let config = test_config(&server); + let auth = test_auth(); + + Mock::given(method("PUT")) + .and(path("/backend-api/ps/plugins/plugins_123/shares")) + .and(header("authorization", "Bearer Access Token")) + .and(header("chatgpt-account-id", "account_id")) + .and(body_json(json!({ + "discoverability": "PRIVATE", + "targets": [ + { + "principal_type": "user", + "principal_id": "user-1", + }, + ], + }))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "principals": [ + { + "principal_type": "user", + "principal_id": "user-1", + "name": "Gavin", + }, + ], + }))) + .expect(1) + .mount(&server) + .await; + + let result = update_remote_plugin_share_targets( + &config, + Some(&auth), + "plugins_123", + vec![RemotePluginShareTarget { + principal_type: RemotePluginSharePrincipalType::User, + principal_id: "user-1".to_string(), + }], + RemotePluginShareUpdateDiscoverability::Private, + ) + .await + .unwrap(); + + assert_eq!( + result, + RemotePluginShareUpdateTargetsResult { + principals: vec![RemotePluginSharePrincipal { + principal_type: RemotePluginSharePrincipalType::User, + principal_id: "user-1".to_string(), + name: "Gavin".to_string(), + }], + discoverability: RemotePluginShareDiscoverability::Private, } ); }