mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
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.
This commit is contained in:
committed by
GitHub
Unverified
parent
9cbd4c0371
commit
ae15343243
+12
-1
@@ -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"
|
||||
}
|
||||
}
|
||||
+16
-1
@@ -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"
|
||||
}
|
||||
}
|
||||
+16
-1
@@ -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"
|
||||
}
|
||||
}
|
||||
+11
@@ -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"
|
||||
],
|
||||
|
||||
+12
@@ -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",
|
||||
|
||||
+5
@@ -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";
|
||||
+2
-1
@@ -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<PluginShareTarget>, };
|
||||
export type PluginShareUpdateTargetsParams = { remotePluginId: string, discoverability: PluginShareUpdateDiscoverability, shareTargets: Array<PluginShareTarget>, };
|
||||
|
||||
+2
-1
@@ -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<PluginSharePrincipal>, };
|
||||
export type PluginShareUpdateTargetsResponse = { principals: Array<PluginSharePrincipal>, discoverability: PluginShareDiscoverability, };
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -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<PluginShareTarget>,
|
||||
}
|
||||
|
||||
@@ -226,6 +227,7 @@ pub struct PluginShareUpdateTargetsParams {
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct PluginShareUpdateTargetsResponse {
|
||||
pub principals: Vec<PluginSharePrincipal>,
|
||||
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 {
|
||||
|
||||
@@ -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",
|
||||
}),
|
||||
);
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<PluginShareTarget>,
|
||||
) -> Vec<codex_core_plugins::remote::RemotePluginShareTarget> {
|
||||
@@ -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<AppSummary>,
|
||||
|
||||
@@ -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(())
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -32,7 +32,7 @@ pub struct RemotePluginShareAccessPolicy {
|
||||
pub share_targets: Option<Vec<RemotePluginShareTarget>>,
|
||||
}
|
||||
|
||||
#[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<RemotePluginSharePrincipal>,
|
||||
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<RemotePluginShareTarget>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
|
||||
struct RemotePluginShareUpdateTargetsResponse {
|
||||
principals: Vec<RemotePluginSharePrincipal>,
|
||||
discoverability: Option<RemotePluginShareDiscoverability>,
|
||||
}
|
||||
|
||||
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<RemotePluginShareTarget>,
|
||||
discoverability: RemotePluginShareUpdateDiscoverability,
|
||||
) -> Result<RemotePluginShareUpdateTargetsResult, RemotePluginCatalogError> {
|
||||
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<RemotePluginShareDiscoverability>,
|
||||
targets: Option<Vec<RemotePluginShareTarget>>,
|
||||
) -> Result<Option<Vec<RemotePluginShareTarget>>, 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,
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user