mirror of
https://github.com/openai/codex.git
synced 2026-11-13 06:45:07 +00:00
Isolate automatic plugin Git operations (#39520)
## Why Background marketplace and plugin refreshes can otherwise inherit repository-local or command-scoped Git configuration from the project that launched Codex. That configuration can redirect remotes or invoke Git helpers during an automatic operation. ## What changed - Run automatic marketplace upgrades, plugin cache refreshes, and curated marketplace lookups with repository-scoped Git environment variables removed and a temporary trusted repository under the Codex home. - Carry the automatic/manual trust mode through marketplace upgrades and plugin materialization. - Preserve the caller's Git configuration for explicit marketplace upgrades, plugin installs, and refreshes. ## Testing - Add regression coverage for repository URL rewrites, protocol helpers, command-scoped configuration, and manual Git filters. GitOrigin-RevId: 957f45fa3bd7ce7e1b83355f276cf65dc5e99c8e
This commit is contained in:
1 parent
18937b2265
commit
ffad922340
13 files changed
+690
-89
No files matched your search
@@ -38,6 +38,7 @@ const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10);
|
||||
struct CuratedMcpSyncFixture {
|
||||
mcp: TestAppServer,
|
||||
sync_barrier: PathBuf,
|
||||
malicious_git_helper_marker: PathBuf,
|
||||
mcp_server_handle: JoinHandle<()>,
|
||||
_fixture_root: TempDir,
|
||||
_responses_server: MockServer,
|
||||
@@ -49,10 +50,11 @@ impl CuratedMcpSyncFixture {
|
||||
let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?;
|
||||
let fixture_root = TempDir::new()?;
|
||||
let codex_home = fixture_root.path().join("codex-home");
|
||||
let curated_repo = fixture_root.path().join("curated-repo");
|
||||
let curated_repo = fixture_root.path().join("plugins.git");
|
||||
let git_wrapper_dir = fixture_root.path().join("bin");
|
||||
let git_config = fixture_root.path().join("gitconfig");
|
||||
let sync_barrier = fixture_root.path().join("allow-curated-sync");
|
||||
let malicious_git_helper_marker = fixture_root.path().join("malicious-git-helper-ran");
|
||||
std::fs::create_dir_all(&codex_home)?;
|
||||
std::fs::create_dir_all(&curated_repo)?;
|
||||
std::fs::create_dir_all(&git_wrapper_dir)?;
|
||||
@@ -91,7 +93,7 @@ impl CuratedMcpSyncFixture {
|
||||
&["commit", "-m", "test curated plugins"],
|
||||
)?;
|
||||
|
||||
let curated_repo_url = format!("file://{}", curated_repo.display());
|
||||
let curated_repo_url = format!("file://{}/", fixture_root.path().display());
|
||||
let rewrite_key = format!("url.{curated_repo_url}.insteadOf");
|
||||
run_git(
|
||||
&real_git,
|
||||
@@ -103,10 +105,34 @@ impl CuratedMcpSyncFixture {
|
||||
.to_str()
|
||||
.context("git config path should be UTF-8")?,
|
||||
&rewrite_key,
|
||||
GITHUB_PLUGINS_GIT_URL,
|
||||
"https://github.com/openai/",
|
||||
],
|
||||
)?;
|
||||
|
||||
let malicious_git_helper = fixture_root.path().join("malicious-git-helper.sh");
|
||||
std::fs::write(
|
||||
&malicious_git_helper,
|
||||
format!(
|
||||
"#!/bin/sh\nprintf ran > '{}'\nexit 73\n",
|
||||
malicious_git_helper_marker.display()
|
||||
),
|
||||
)?;
|
||||
let mut helper_permissions = std::fs::metadata(&malicious_git_helper)?.permissions();
|
||||
helper_permissions.set_mode(0o755);
|
||||
std::fs::set_permissions(&malicious_git_helper, helper_permissions)?;
|
||||
run_git(&real_git, &codex_home, &["init", "-b", "main"])?;
|
||||
run_git(
|
||||
&real_git,
|
||||
&codex_home,
|
||||
&["config", "protocol.ext.allow", "always"],
|
||||
)?;
|
||||
let malicious_rewrite_key =
|
||||
format!("url.ext::{}.insteadOf", malicious_git_helper.display());
|
||||
run_git(
|
||||
&real_git,
|
||||
&codex_home,
|
||||
&["config", &malicious_rewrite_key, GITHUB_PLUGINS_GIT_URL],
|
||||
)?;
|
||||
let git_wrapper = git_wrapper_dir.join("git");
|
||||
std::fs::write(
|
||||
&git_wrapper,
|
||||
@@ -168,6 +194,7 @@ url = "{mcp_server_url}/mcp""#
|
||||
Ok(Self {
|
||||
mcp,
|
||||
sync_barrier,
|
||||
malicious_git_helper_marker,
|
||||
mcp_server_handle,
|
||||
_fixture_root: fixture_root,
|
||||
_responses_server: responses_server,
|
||||
@@ -208,6 +235,10 @@ async fn existing_thread_loads_api_curated_mcp_after_auth_switch_sync() -> Resul
|
||||
// Let sync materialize the bundle; its completion callback must refresh this same thread.
|
||||
std::fs::write(&fixture.sync_barrier, "continue")?;
|
||||
wait_for_mcp_ready(&mut fixture.mcp, TEST_SERVER_NAME).await?;
|
||||
ensure!(
|
||||
!fixture.malicious_git_helper_marker.exists(),
|
||||
"curated startup synchronization executed repository-local Git configuration"
|
||||
);
|
||||
|
||||
let response: McpServerToolCallResponse = fixture
|
||||
.mcp
|
||||
|
||||
@@ -8,6 +8,11 @@ use app_test_support::TestAppServer;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::MarketplaceUpgradeParams;
|
||||
use codex_app_server_protocol::MarketplaceUpgradeResponse;
|
||||
use codex_app_server_protocol::PluginInstallParams;
|
||||
use codex_app_server_protocol::PluginInstallResponse;
|
||||
use codex_app_server_protocol::PluginListMarketplaceKind;
|
||||
use codex_app_server_protocol::PluginListParams;
|
||||
use codex_app_server_protocol::PluginListResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_config::MarketplaceConfigUpdate;
|
||||
use codex_config::record_user_marketplace;
|
||||
@@ -195,6 +200,141 @@ async fn marketplace_upgrade_all_configured_git_marketplaces() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn automatic_upgrade_isolates_git_while_explicit_install_preserves_configuration()
|
||||
-> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let source = TempDir::new()?;
|
||||
let plugin_source = TempDir::new()?;
|
||||
let old_revision = init_marketplace_repo(source.path(), "trusted", "old")?;
|
||||
|
||||
init_marketplace_repo(plugin_source.path(), "plugin", "plugin")?;
|
||||
let plugin_root = plugin_source.path().join(".codex-plugin");
|
||||
std::fs::create_dir_all(&plugin_root)?;
|
||||
std::fs::write(
|
||||
plugin_root.join("plugin.json"),
|
||||
r#"{"name":"toolkit","version":"1.0.0","interface":{"displayName":"Toolkit"}}"#,
|
||||
)?;
|
||||
run_git(plugin_source.path(), &["add", "."])?;
|
||||
run_git(plugin_source.path(), &["commit", "-qm", "add plugin"])?;
|
||||
let plugin_url = url::Url::from_directory_path(plugin_source.path())
|
||||
.map_err(|()| anyhow::anyhow!("invalid plugin source path"))?
|
||||
.to_string();
|
||||
std::fs::write(
|
||||
source.path().join(".agents/plugins/marketplace.json"),
|
||||
serde_json::json!({
|
||||
"name": "trusted",
|
||||
"plugins": [{
|
||||
"name": "toolkit",
|
||||
"source": {"source": "url", "url": plugin_url},
|
||||
}],
|
||||
})
|
||||
.to_string(),
|
||||
)?;
|
||||
run_git(source.path(), &["add", ".agents/plugins/marketplace.json"])?;
|
||||
commit_marketplace_marker(source.path(), "new")?;
|
||||
record_git_marketplace(
|
||||
codex_home.path(),
|
||||
"trusted",
|
||||
source.path(),
|
||||
&old_revision,
|
||||
/*ref_name*/ None,
|
||||
)?;
|
||||
let config_path = codex_home.path().join("config.toml");
|
||||
std::fs::write(
|
||||
&config_path,
|
||||
format!(
|
||||
"{}\n[plugins.\"toolkit@trusted\"]\nenabled = true\n",
|
||||
std::fs::read_to_string(&config_path)?
|
||||
),
|
||||
)?;
|
||||
|
||||
run_git(codex_home.path(), &["init", "--quiet"])?;
|
||||
let source_path = source.path().display().to_string();
|
||||
let missing_path = codex_home.path().join("missing").display().to_string();
|
||||
let rewrite_key = format!("url.{missing_path}.insteadOf");
|
||||
for untrusted_source in [&source_path, &plugin_url] {
|
||||
run_git(
|
||||
codex_home.path(),
|
||||
&["config", "--add", &rewrite_key, untrusted_source],
|
||||
)?;
|
||||
assert!(run_git(codex_home.path(), &["ls-remote", untrusted_source, "HEAD"]).is_err());
|
||||
}
|
||||
|
||||
let manual_plugin_alias = "https://manual.example/plugin.git";
|
||||
let manual_plugin_rewrite = format!("url.{}.insteadOf", plugin_source.path().display());
|
||||
let mut server = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.with_plugin_startup_tasks()
|
||||
.with_env_overrides(&[
|
||||
("GIT_CONFIG_COUNT", Some("1")),
|
||||
("GIT_CONFIG_KEY_0", Some(&manual_plugin_rewrite)),
|
||||
("GIT_CONFIG_VALUE_0", Some(manual_plugin_alias)),
|
||||
])
|
||||
.build_initialized_with_timeout(DEFAULT_TIMEOUT)
|
||||
.await?;
|
||||
let plugin_root = codex_home
|
||||
.path()
|
||||
.join("plugins/cache/trusted/toolkit/1.0.0/.codex-plugin/plugin.json");
|
||||
timeout(DEFAULT_TIMEOUT, async {
|
||||
while !plugin_root.exists() {
|
||||
tokio::time::sleep(Duration::from_millis(10)).await;
|
||||
}
|
||||
})
|
||||
.await?;
|
||||
|
||||
let marketplace_path = marketplace_install_root(codex_home.path())
|
||||
.join("trusted/.agents/plugins/marketplace.json");
|
||||
std::fs::write(
|
||||
&marketplace_path,
|
||||
serde_json::json!({
|
||||
"name": "trusted",
|
||||
"plugins": [{
|
||||
"name": "toolkit",
|
||||
"source": {"source": "url", "url": manual_plugin_alias},
|
||||
}],
|
||||
})
|
||||
.to_string(),
|
||||
)?;
|
||||
let marketplace_path = AbsolutePathBuf::try_from(marketplace_path)?;
|
||||
let _: PluginInstallResponse = server
|
||||
.request(|request_id| ClientRequest::PluginInstall {
|
||||
request_id,
|
||||
params: PluginInstallParams {
|
||||
marketplace_path: Some(marketplace_path),
|
||||
remote_marketplace_name: None,
|
||||
install_attempt_id: None,
|
||||
plugin_name: "toolkit".to_string(),
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
|
||||
std::fs::write(
|
||||
plugin_source.path().join(".codex-plugin/plugin.json"),
|
||||
r#"{"name":"toolkit","version":"1.1.0","interface":{"displayName":"Toolkit"}}"#,
|
||||
)?;
|
||||
run_git(plugin_source.path(), &["add", "."])?;
|
||||
run_git(plugin_source.path(), &["commit", "-qm", "upgrade plugin"])?;
|
||||
let _: PluginListResponse = server
|
||||
.request(|request_id| ClientRequest::PluginList {
|
||||
request_id,
|
||||
params: PluginListParams {
|
||||
cwds: None,
|
||||
marketplace_kinds: Some(vec![PluginListMarketplaceKind::Local]),
|
||||
force_refetch: true,
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
assert!(
|
||||
codex_home
|
||||
.path()
|
||||
.join("plugins/cache/trusted/toolkit/1.1.0/.codex-plugin/plugin.json")
|
||||
.is_file(),
|
||||
"explicit plugin/list refresh must preserve command-scoped Git configuration"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn marketplace_upgrade_named_marketplace_only() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
@@ -211,17 +351,26 @@ async fn marketplace_upgrade_named_marketplace_only() -> Result<()> {
|
||||
&debug_old_revision,
|
||||
/*ref_name*/ None,
|
||||
)?;
|
||||
record_git_marketplace(
|
||||
let tools_source_alias = "manual:tools";
|
||||
record_user_marketplace(
|
||||
codex_home.path(),
|
||||
"tools",
|
||||
tools_source.path(),
|
||||
&tools_old_revision,
|
||||
/*ref_name*/ None,
|
||||
&configured_git_marketplace_update(
|
||||
tools_source_alias,
|
||||
Some(&tools_old_revision),
|
||||
/*ref_name*/ None,
|
||||
),
|
||||
)?;
|
||||
disable_plugin_startup_tasks(codex_home.path())?;
|
||||
let tools_rewrite = format!("url.{}.insteadOf", tools_source.path().display());
|
||||
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.with_env_overrides(&[
|
||||
("GIT_CONFIG_COUNT", Some("1")),
|
||||
("GIT_CONFIG_KEY_0", Some(&tools_rewrite)),
|
||||
("GIT_CONFIG_VALUE_0", Some(tools_source_alias)),
|
||||
])
|
||||
.without_auto_env()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::path::Path;
|
||||
use std::process::Command;
|
||||
use tempfile::TempDir;
|
||||
|
||||
// These variables can redirect Git to an untrusted repository or inject command-scoped settings.
|
||||
pub(crate) const REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES: &[&str] = &[
|
||||
"GIT_ALTERNATE_OBJECT_DIRECTORIES",
|
||||
"GIT_CEILING_DIRECTORIES",
|
||||
"GIT_COMMON_DIR",
|
||||
"GIT_CONFIG",
|
||||
"GIT_CONFIG_COUNT",
|
||||
"GIT_CONFIG_PARAMETERS",
|
||||
"GIT_DIR",
|
||||
"GIT_DISCOVERY_ACROSS_FILESYSTEM",
|
||||
"GIT_GRAFT_FILE",
|
||||
"GIT_IMPLICIT_WORK_TREE",
|
||||
"GIT_INDEX_FILE",
|
||||
"GIT_NAMESPACE",
|
||||
"GIT_OBJECT_DIRECTORY",
|
||||
"GIT_PREFIX",
|
||||
"GIT_REPLACE_REF_BASE",
|
||||
"GIT_SHALLOW_FILE",
|
||||
"GIT_WORK_TREE",
|
||||
];
|
||||
|
||||
/// Distinguishes isolated background Git operations from explicitly requested user operations.
|
||||
#[derive(Clone, Copy, PartialEq, Eq)]
|
||||
pub(crate) enum PluginGitMode {
|
||||
/// Ignores repository-scoped Git configuration inherited from the launch project.
|
||||
Automatic,
|
||||
/// Preserves the configuration explicitly available to user-requested Git commands.
|
||||
Manual,
|
||||
}
|
||||
|
||||
impl PluginGitMode {
|
||||
/// Creates a Git command with the repository-environment policy for this operation.
|
||||
/// Automatic remote lookups and initial clones must also select a trusted repository.
|
||||
pub(crate) fn command(self, git_binary: &Path) -> Command {
|
||||
let mut command = Command::new(git_binary);
|
||||
command.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG]);
|
||||
if matches!(self, Self::Automatic) {
|
||||
command.env("GIT_OPTIONAL_LOCKS", "0");
|
||||
for name in REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES {
|
||||
command.env_remove(name);
|
||||
}
|
||||
}
|
||||
command
|
||||
}
|
||||
}
|
||||
|
||||
/// Isolates an automatic Git command from repository discovery in the launch project.
|
||||
///
|
||||
/// The command must already have repository-scoped environment variables removed. Keep the
|
||||
/// returned guard alive until the Git process exits because it owns the selected repository.
|
||||
pub(crate) fn configure_trusted_git_repository(
|
||||
command: &mut Command,
|
||||
codex_home: &Path,
|
||||
) -> Result<TempDir, String> {
|
||||
let canonical_home = AbsolutePathBuf::from_absolute_path(codex_home)
|
||||
.and_then(|path| path.canonicalize())
|
||||
.map_err(|err| format!("failed to resolve trusted Codex home: {err}"))?;
|
||||
let staging_root = canonical_home.join(".tmp");
|
||||
std::fs::create_dir_all(staging_root.as_path())
|
||||
.map_err(|err| format!("failed to create trusted Git repository root: {err}"))?;
|
||||
let staging_root = staging_root
|
||||
.canonicalize()
|
||||
.map_err(|err| format!("failed to resolve trusted Git repository root: {err}"))?;
|
||||
if !staging_root.as_path().starts_with(canonical_home.as_path()) {
|
||||
return Err("trusted Git repository root escapes Codex home".to_string());
|
||||
}
|
||||
|
||||
let repository = tempfile::Builder::new()
|
||||
.prefix("git-")
|
||||
.tempdir_in(staging_root.as_path())
|
||||
.map_err(|err| format!("failed to create trusted Git repository: {err}"))?;
|
||||
for directory in ["objects", "refs"] {
|
||||
std::fs::create_dir(repository.path().join(directory))
|
||||
.map_err(|err| format!("failed to initialize trusted Git repository: {err}"))?;
|
||||
}
|
||||
std::fs::write(repository.path().join("HEAD"), "ref: refs/heads/main\n")
|
||||
.map_err(|err| format!("failed to initialize trusted Git repository HEAD: {err}"))?;
|
||||
command.env("GIT_DIR", repository.path());
|
||||
Ok(repository)
|
||||
}
|
||||
@@ -3,6 +3,7 @@ mod artifact_operation;
|
||||
mod command_migration;
|
||||
mod discoverable;
|
||||
mod error_subtype;
|
||||
mod git_policy;
|
||||
mod http_client_selector;
|
||||
pub mod installed_marketplaces;
|
||||
pub mod loader;
|
||||
@@ -32,6 +33,9 @@ mod test_support;
|
||||
pub mod toggles;
|
||||
mod tool_suggest_metadata;
|
||||
|
||||
pub(crate) use git_policy::PluginGitMode;
|
||||
pub(crate) use git_policy::configure_trusted_git_repository;
|
||||
|
||||
pub const OPENAI_CURATED_MARKETPLACE_NAME: &str = "openai-curated";
|
||||
pub const OPENAI_API_CURATED_MARKETPLACE_NAME: &str = "openai-api-curated";
|
||||
pub const OPENAI_BUNDLED_MARKETPLACE_NAME: &str = "openai-bundled";
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use crate::PluginGitMode;
|
||||
use crate::app_mcp_routing::apply_app_mcp_routing_policy;
|
||||
use crate::app_mcp_routing::apps_route_available;
|
||||
use crate::is_openai_curated_marketplace_name;
|
||||
@@ -56,7 +57,6 @@ use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
use std::process::Command;
|
||||
use tempfile::TempDir;
|
||||
use tracing::instrument;
|
||||
use tracing::warn;
|
||||
@@ -496,6 +496,7 @@ pub(crate) fn refresh_non_curated_plugin_cache(
|
||||
codex_home,
|
||||
additional_roots,
|
||||
configured_plugin_keys,
|
||||
PluginGitMode::Automatic,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -503,12 +504,14 @@ pub(crate) fn refresh_non_curated_plugin_cache_detailed(
|
||||
codex_home: &Path,
|
||||
additional_roots: &[AbsolutePathBuf],
|
||||
configured_plugin_keys: &[String],
|
||||
git_mode: PluginGitMode,
|
||||
) -> Result<NonCuratedCacheRefreshOutcome, String> {
|
||||
refresh_non_curated_plugin_cache_with_mode(
|
||||
codex_home,
|
||||
additional_roots,
|
||||
configured_plugin_keys,
|
||||
NonCuratedCacheRefreshMode::IfVersionChanged,
|
||||
git_mode,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -522,6 +525,7 @@ pub(crate) fn refresh_non_curated_plugin_cache_force_reinstall(
|
||||
codex_home,
|
||||
additional_roots,
|
||||
configured_plugin_keys,
|
||||
PluginGitMode::Automatic,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -529,12 +533,14 @@ pub(crate) fn refresh_non_curated_plugin_cache_force_reinstall_detailed(
|
||||
codex_home: &Path,
|
||||
additional_roots: &[AbsolutePathBuf],
|
||||
configured_plugin_keys: &[String],
|
||||
git_mode: PluginGitMode,
|
||||
) -> Result<NonCuratedCacheRefreshOutcome, String> {
|
||||
refresh_non_curated_plugin_cache_with_mode(
|
||||
codex_home,
|
||||
additional_roots,
|
||||
configured_plugin_keys,
|
||||
NonCuratedCacheRefreshMode::ForceReinstall,
|
||||
git_mode,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -543,6 +549,7 @@ fn refresh_non_curated_plugin_cache_with_mode(
|
||||
additional_roots: &[AbsolutePathBuf],
|
||||
configured_plugin_keys: &[String],
|
||||
mode: NonCuratedCacheRefreshMode,
|
||||
git_mode: PluginGitMode,
|
||||
) -> Result<NonCuratedCacheRefreshOutcome, String> {
|
||||
let mut configured_non_curated_plugin_ids = configured_plugin_keys
|
||||
.iter()
|
||||
@@ -644,9 +651,10 @@ fn refresh_non_curated_plugin_cache_with_mode(
|
||||
};
|
||||
let refresh_result = (|| -> Result<bool, String> {
|
||||
let materialized =
|
||||
materialize_marketplace_plugin_source(codex_home, &source).map_err(|err| {
|
||||
format!("failed to materialize plugin source for {plugin_key}: {err}")
|
||||
})?;
|
||||
materialize_marketplace_plugin_source_with_mode(codex_home, &source, git_mode)
|
||||
.map_err(|err| {
|
||||
format!("failed to materialize plugin source for {plugin_key}: {err}")
|
||||
})?;
|
||||
let source_path = materialized.path;
|
||||
let plugin_version = match manifest_fallback_contents.as_deref() {
|
||||
Some(manifest_contents) => plugin_version_for_source_with_fallback_manifest(
|
||||
@@ -1706,6 +1714,15 @@ pub struct MaterializedMarketplacePluginSource {
|
||||
pub fn materialize_marketplace_plugin_source(
|
||||
codex_home: &Path,
|
||||
source: &MarketplacePluginSource,
|
||||
) -> Result<MaterializedMarketplacePluginSource, String> {
|
||||
materialize_marketplace_plugin_source_with_mode(codex_home, source, PluginGitMode::Manual)
|
||||
}
|
||||
|
||||
/// Applies the initiating operation's Git trust policy throughout plugin materialization.
|
||||
pub(crate) fn materialize_marketplace_plugin_source_with_mode(
|
||||
codex_home: &Path,
|
||||
source: &MarketplacePluginSource,
|
||||
mode: PluginGitMode,
|
||||
) -> Result<MaterializedMarketplacePluginSource, String> {
|
||||
match source {
|
||||
MarketplacePluginSource::Local { path } => Ok(MaterializedMarketplacePluginSource {
|
||||
@@ -1735,11 +1752,13 @@ pub fn materialize_marketplace_plugin_source(
|
||||
)
|
||||
})?;
|
||||
clone_git_plugin_source(
|
||||
codex_home,
|
||||
url,
|
||||
ref_name.as_deref(),
|
||||
sha.as_deref(),
|
||||
path.as_deref(),
|
||||
tempdir.path(),
|
||||
mode,
|
||||
)?;
|
||||
let path = if let Some(path) = path {
|
||||
AbsolutePathBuf::try_from(tempdir.path().join(path)).map_err(|err| {
|
||||
@@ -1775,12 +1794,18 @@ pub fn materialize_marketplace_plugin_source(
|
||||
}
|
||||
|
||||
fn clone_git_plugin_source(
|
||||
codex_home: &Path,
|
||||
url: &str,
|
||||
ref_name: Option<&str>,
|
||||
sha: Option<&str>,
|
||||
sparse_checkout_path: Option<&str>,
|
||||
destination: &Path,
|
||||
mode: PluginGitMode,
|
||||
) -> Result<(), String> {
|
||||
let clone_cwd = match mode {
|
||||
PluginGitMode::Automatic => Some(codex_home),
|
||||
PluginGitMode::Manual => None,
|
||||
};
|
||||
if let Some(sparse_checkout_path) = sparse_checkout_path {
|
||||
run_git(
|
||||
&[
|
||||
@@ -1791,7 +1816,8 @@ fn clone_git_plugin_source(
|
||||
url,
|
||||
destination.to_string_lossy().as_ref(),
|
||||
],
|
||||
/*cwd*/ None,
|
||||
clone_cwd,
|
||||
mode,
|
||||
)?;
|
||||
run_git(
|
||||
&[
|
||||
@@ -1802,42 +1828,56 @@ fn clone_git_plugin_source(
|
||||
sparse_checkout_path,
|
||||
],
|
||||
Some(destination),
|
||||
mode,
|
||||
)?;
|
||||
} else {
|
||||
run_git(
|
||||
&["clone", url, destination.to_string_lossy().as_ref()],
|
||||
/*cwd*/ None,
|
||||
clone_cwd,
|
||||
mode,
|
||||
)?;
|
||||
}
|
||||
if let Some(sha) = sha {
|
||||
run_git(&["checkout", sha], Some(destination))?;
|
||||
let checked_out_sha = run_git_output(&["rev-parse", "HEAD"], Some(destination))?;
|
||||
run_git(&["checkout", sha], Some(destination), mode)?;
|
||||
let checked_out_sha = run_git_output(&["rev-parse", "HEAD"], Some(destination), mode)?;
|
||||
if !checked_out_sha.eq_ignore_ascii_case(sha) {
|
||||
return Err(format!(
|
||||
"checked out Git SHA {checked_out_sha} does not match requested SHA {sha}"
|
||||
));
|
||||
}
|
||||
} else if let Some(ref_name) = ref_name {
|
||||
run_git(&["checkout", ref_name], Some(destination))?;
|
||||
run_git(&["checkout", ref_name], Some(destination), mode)?;
|
||||
} else if sparse_checkout_path.is_some() {
|
||||
run_git(&["checkout"], Some(destination))?;
|
||||
run_git(&["checkout"], Some(destination), mode)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn run_git(args: &[&str], cwd: Option<&Path>) -> Result<(), String> {
|
||||
run_git_output(args, cwd).map(drop)
|
||||
fn run_git(args: &[&str], cwd: Option<&Path>, mode: PluginGitMode) -> Result<(), String> {
|
||||
run_git_output(args, cwd, mode).map(drop)
|
||||
}
|
||||
|
||||
fn run_git_output(args: &[&str], cwd: Option<&Path>) -> Result<String, String> {
|
||||
let mut command = Command::new("git");
|
||||
command
|
||||
.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG])
|
||||
.args(args);
|
||||
fn run_git_output(
|
||||
args: &[&str],
|
||||
cwd: Option<&Path>,
|
||||
mode: PluginGitMode,
|
||||
) -> Result<String, String> {
|
||||
let mut command = mode.command(Path::new("git"));
|
||||
command.args(args);
|
||||
command.env("GIT_TERMINAL_PROMPT", "0");
|
||||
if let Some(cwd) = cwd {
|
||||
command.current_dir(cwd);
|
||||
}
|
||||
let _trusted_repository = if let Some(cwd) = cwd
|
||||
&& args.first() == Some(&"clone")
|
||||
{
|
||||
Some(crate::configure_trusted_git_repository(&mut command, cwd)?)
|
||||
} else {
|
||||
if let Some(cwd) = cwd {
|
||||
command.current_dir(cwd);
|
||||
if matches!(mode, PluginGitMode::Manual) {
|
||||
command.env_remove("GIT_DIR");
|
||||
}
|
||||
}
|
||||
None
|
||||
};
|
||||
|
||||
let output = command
|
||||
.output()
|
||||
|
||||
@@ -630,6 +630,9 @@ fn load_plugin_hooks_supports_inline_manifest_hook_list() {
|
||||
|
||||
#[test]
|
||||
fn materialize_git_subdir_uses_sparse_checkout() {
|
||||
let run_git = |args: &[&str], cwd| super::run_git(args, cwd, PluginGitMode::Manual);
|
||||
let run_git_output =
|
||||
|args: &[&str], cwd| super::run_git_output(args, cwd, PluginGitMode::Manual);
|
||||
let codex_home = tempfile::tempdir().expect("create codex home");
|
||||
let repo = tempfile::tempdir().expect("create git repo");
|
||||
let plugin_dir = repo.path().join("plugins/toolkit");
|
||||
@@ -678,6 +681,9 @@ fn materialize_git_subdir_uses_sparse_checkout() {
|
||||
|
||||
#[test]
|
||||
fn materialize_git_source_rejects_sha_that_resolves_to_hostile_default_branch() {
|
||||
let run_git = |args: &[&str], cwd| super::run_git(args, cwd, PluginGitMode::Manual);
|
||||
let run_git_output =
|
||||
|args: &[&str], cwd| super::run_git_output(args, cwd, PluginGitMode::Manual);
|
||||
let codex_home = tempfile::tempdir().expect("create codex home");
|
||||
let repo = tempfile::tempdir().expect("create git repo");
|
||||
run_git(&["init"], Some(repo.path())).expect("init git repo");
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::LoadedPlugin;
|
||||
use super::PluginLoadOutcome;
|
||||
use crate::PluginGitMode;
|
||||
use crate::app_mcp_routing::apply_app_mcp_routing_policy;
|
||||
use crate::installed_marketplaces::installed_marketplace_roots_from_layer_stack;
|
||||
use crate::is_openai_curated_marketplace_name;
|
||||
@@ -43,7 +44,7 @@ use crate::marketplace_policy::MarketplacePolicy;
|
||||
use crate::marketplace_policy::configured_plugins_from_stack;
|
||||
use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeError;
|
||||
use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome;
|
||||
use crate::marketplace_upgrade::upgrade_configured_git_marketplaces;
|
||||
use crate::marketplace_upgrade::upgrade_configured_git_marketplaces_with_mode;
|
||||
use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME;
|
||||
use crate::remote::RecommendedPluginsMode;
|
||||
use crate::remote::RemoteInstalledPlugin;
|
||||
@@ -263,6 +264,7 @@ struct NonCuratedCacheRefreshRequest {
|
||||
configured_plugin_keys: Vec<String>,
|
||||
configured_plugin_sources: Vec<NonCuratedPluginSource>,
|
||||
mode: NonCuratedCacheRefreshMode,
|
||||
git_mode: PluginGitMode,
|
||||
}
|
||||
|
||||
#[derive(Clone, PartialEq, Eq)]
|
||||
@@ -2157,8 +2159,10 @@ impl PluginsManager {
|
||||
if let Err(err) = std::thread::Builder::new()
|
||||
.name("plugins-marketplace-auto-upgrade".to_string())
|
||||
.spawn(move || {
|
||||
let outcome = manager.upgrade_configured_marketplaces_for_config(
|
||||
&config, /*marketplace_name*/ None,
|
||||
let outcome = manager.upgrade_configured_marketplaces_for_config_with_mode(
|
||||
&config,
|
||||
/*marketplace_name*/ None,
|
||||
PluginGitMode::Automatic,
|
||||
);
|
||||
match outcome {
|
||||
Ok(outcome) => {
|
||||
@@ -2255,10 +2259,25 @@ impl PluginsManager {
|
||||
config: &PluginsConfigInput,
|
||||
marketplace_name: Option<&str>,
|
||||
) -> Result<ConfiguredMarketplaceUpgradeOutcome, String> {
|
||||
let mut outcome = upgrade_configured_git_marketplaces(
|
||||
self.upgrade_configured_marketplaces_for_config_with_mode(
|
||||
config,
|
||||
marketplace_name,
|
||||
PluginGitMode::Manual,
|
||||
)
|
||||
}
|
||||
|
||||
/// Carries the initiating Git trust policy through marketplace and plugin refreshes.
|
||||
fn upgrade_configured_marketplaces_for_config_with_mode(
|
||||
&self,
|
||||
config: &PluginsConfigInput,
|
||||
marketplace_name: Option<&str>,
|
||||
mode: PluginGitMode,
|
||||
) -> Result<ConfiguredMarketplaceUpgradeOutcome, String> {
|
||||
let mut outcome = upgrade_configured_git_marketplaces_with_mode(
|
||||
self.codex_home.as_path(),
|
||||
&config.config_layer_stack,
|
||||
marketplace_name,
|
||||
mode,
|
||||
);
|
||||
if let Some(marketplace_name) = marketplace_name
|
||||
&& outcome.selected_marketplaces.is_empty()
|
||||
@@ -2279,6 +2298,7 @@ impl PluginsManager {
|
||||
self.codex_home.as_path(),
|
||||
&outcome.upgraded_roots,
|
||||
&configured_plugin_keys,
|
||||
mode,
|
||||
) {
|
||||
Ok(refresh_outcome) => {
|
||||
self.clear_caches_after_marketplace_source_refresh(
|
||||
@@ -2319,9 +2339,11 @@ impl PluginsManager {
|
||||
config,
|
||||
roots,
|
||||
NonCuratedCacheRefreshMode::IfVersionChanged,
|
||||
PluginGitMode::Automatic,
|
||||
);
|
||||
}
|
||||
|
||||
/// Runs an explicitly requested refresh using the caller's normal Git configuration.
|
||||
pub async fn refresh_non_curated_plugin_cache_for_config(
|
||||
self: &Arc<Self>,
|
||||
config: &PluginsConfigInput,
|
||||
@@ -2332,7 +2354,12 @@ impl PluginsManager {
|
||||
};
|
||||
let mut completion = self.non_curated_cache_refresh_completion.subscribe();
|
||||
let changed_sequence = completion.borrow_and_update().changed_sequence;
|
||||
self.maybe_start_non_curated_plugin_cache_refresh(config, roots);
|
||||
self.schedule_non_curated_plugin_cache_refresh(
|
||||
config,
|
||||
roots,
|
||||
NonCuratedCacheRefreshMode::IfVersionChanged,
|
||||
PluginGitMode::Manual,
|
||||
);
|
||||
|
||||
loop {
|
||||
let in_flight = match self.non_curated_cache_refresh_state.read() {
|
||||
@@ -2466,6 +2493,7 @@ impl PluginsManager {
|
||||
config: &PluginsConfigInput,
|
||||
roots: &[AbsolutePathBuf],
|
||||
mode: NonCuratedCacheRefreshMode,
|
||||
git_mode: PluginGitMode,
|
||||
) {
|
||||
let marketplace_roots =
|
||||
self.marketplace_roots(config, roots, /*include_openai_curated*/ false);
|
||||
@@ -2535,6 +2563,7 @@ impl PluginsManager {
|
||||
configured_plugin_keys,
|
||||
configured_plugin_sources,
|
||||
mode,
|
||||
git_mode,
|
||||
};
|
||||
|
||||
let should_spawn = {
|
||||
@@ -2568,16 +2597,24 @@ impl PluginsManager {
|
||||
request.mode = NonCuratedCacheRefreshMode::ForceReinstall;
|
||||
}
|
||||
// Reconcile each canonical plugin generation once before publishing its resource.
|
||||
if state.requested.as_ref() == Some(&request)
|
||||
|| (request.mode == NonCuratedCacheRefreshMode::IfVersionChanged
|
||||
&& !state.in_flight
|
||||
&& state.last_refreshed.as_ref().is_some_and(|last_refreshed| {
|
||||
last_refreshed.roots == request.roots
|
||||
&& last_refreshed.configured_plugin_keys
|
||||
== request.configured_plugin_keys
|
||||
&& last_refreshed.configured_plugin_sources
|
||||
== request.configured_plugin_sources
|
||||
}))
|
||||
if state.requested.as_ref().is_some_and(|requested| {
|
||||
requested == &request
|
||||
|| (requested.git_mode == PluginGitMode::Manual
|
||||
&& request.git_mode == PluginGitMode::Automatic
|
||||
&& requested.roots == request.roots
|
||||
&& requested.configured_plugin_keys == request.configured_plugin_keys
|
||||
&& requested.configured_plugin_sources == request.configured_plugin_sources
|
||||
&& requested.mode == request.mode)
|
||||
}) || (request.mode == NonCuratedCacheRefreshMode::IfVersionChanged
|
||||
&& !state.in_flight
|
||||
&& state.last_refreshed.as_ref().is_some_and(|last_refreshed| {
|
||||
last_refreshed.roots == request.roots
|
||||
&& last_refreshed.configured_plugin_keys == request.configured_plugin_keys
|
||||
&& last_refreshed.configured_plugin_sources
|
||||
== request.configured_plugin_sources
|
||||
&& (last_refreshed.git_mode == request.git_mode
|
||||
|| last_refreshed.git_mode == PluginGitMode::Manual)
|
||||
}))
|
||||
{
|
||||
return;
|
||||
}
|
||||
@@ -2819,6 +2856,7 @@ impl PluginsManager {
|
||||
self.codex_home.as_path(),
|
||||
&request.roots,
|
||||
&request.configured_plugin_keys,
|
||||
request.git_mode,
|
||||
)
|
||||
}
|
||||
NonCuratedCacheRefreshMode::ForceReinstall => {
|
||||
@@ -2826,6 +2864,7 @@ impl PluginsManager {
|
||||
self.codex_home.as_path(),
|
||||
&request.roots,
|
||||
&request.configured_plugin_keys,
|
||||
request.git_mode,
|
||||
)
|
||||
}
|
||||
};
|
||||
|
||||
@@ -6197,6 +6197,106 @@ enabled = true
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn non_curated_plugin_cache_refresh_preserves_manual_priority() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let marketplace_root = codex_home.path().join("marketplace");
|
||||
fs::create_dir_all(marketplace_root.join(".git")).unwrap();
|
||||
write_plugin_with_version(
|
||||
&marketplace_root,
|
||||
"sample-plugin",
|
||||
"sample-plugin",
|
||||
Some("1.0.0"),
|
||||
);
|
||||
write_file(
|
||||
&marketplace_root.join(".agents/plugins/marketplace.json"),
|
||||
r#"{
|
||||
"name": "debug",
|
||||
"plugins": [{
|
||||
"name": "sample-plugin",
|
||||
"source": { "source": "local", "path": "./sample-plugin" }
|
||||
}]
|
||||
}"#,
|
||||
);
|
||||
write_file(
|
||||
&codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"[features]
|
||||
plugins = true
|
||||
|
||||
[plugins."sample-plugin@debug"]
|
||||
enabled = true
|
||||
"#,
|
||||
);
|
||||
let config = load_plugins_config_input(codex_home.path(), &marketplace_root).await;
|
||||
let roots = [AbsolutePathBuf::try_from(marketplace_root.clone()).unwrap()];
|
||||
let manager = Arc::new(test_plugins_manager(codex_home.path().to_path_buf()));
|
||||
manager
|
||||
.non_curated_cache_refresh_state
|
||||
.write()
|
||||
.expect("refresh state lock")
|
||||
.in_flight = true;
|
||||
|
||||
for git_mode in [
|
||||
PluginGitMode::Automatic,
|
||||
PluginGitMode::Manual,
|
||||
PluginGitMode::Automatic,
|
||||
] {
|
||||
manager.schedule_non_curated_plugin_cache_refresh(
|
||||
&config,
|
||||
&roots,
|
||||
NonCuratedCacheRefreshMode::IfVersionChanged,
|
||||
git_mode,
|
||||
);
|
||||
}
|
||||
|
||||
{
|
||||
let mut state = manager
|
||||
.non_curated_cache_refresh_state
|
||||
.write()
|
||||
.expect("refresh state lock");
|
||||
assert!(matches!(
|
||||
state.requested.as_ref().map(|request| request.git_mode),
|
||||
Some(PluginGitMode::Manual)
|
||||
));
|
||||
state.last_refreshed = state.requested.take();
|
||||
state.in_flight = false;
|
||||
}
|
||||
|
||||
manager.maybe_start_non_curated_plugin_cache_refresh(&config, &roots);
|
||||
|
||||
{
|
||||
let mut state = manager
|
||||
.non_curated_cache_refresh_state
|
||||
.write()
|
||||
.expect("refresh state lock");
|
||||
assert!(!state.in_flight);
|
||||
assert!(state.requested.is_none());
|
||||
state.requested = state.last_refreshed.clone();
|
||||
state.in_flight = true;
|
||||
}
|
||||
|
||||
write_plugin_with_version(
|
||||
&marketplace_root,
|
||||
"sample-plugin",
|
||||
"sample-plugin",
|
||||
Some("2.0.0"),
|
||||
);
|
||||
manager.maybe_start_non_curated_plugin_cache_refresh(&config, &roots);
|
||||
|
||||
let state = manager
|
||||
.non_curated_cache_refresh_state
|
||||
.read()
|
||||
.expect("refresh state lock");
|
||||
let request = state.requested.as_ref().expect("pending refresh");
|
||||
assert!(matches!(request.git_mode, PluginGitMode::Automatic));
|
||||
assert_eq!(
|
||||
request.configured_plugin_sources[0]
|
||||
.local_version
|
||||
.as_deref(),
|
||||
Some("2.0.0")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn refresh_non_curated_plugin_cache_reinstalls_missing_configured_plugin_with_manifest_version() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
|
||||
@@ -6,6 +6,7 @@ use self::activation::installed_marketplace_metadata_matches;
|
||||
use self::activation::write_installed_marketplace_metadata;
|
||||
use self::git::clone_git_source;
|
||||
use self::git::git_remote_revision;
|
||||
use crate::PluginGitMode;
|
||||
use crate::installed_marketplaces::marketplace_install_root;
|
||||
use crate::marketplace::validate_marketplace_root;
|
||||
use crate::marketplace_add::MarketplaceSource;
|
||||
@@ -72,6 +73,21 @@ pub fn upgrade_configured_git_marketplaces(
|
||||
codex_home: &Path,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
marketplace_name: Option<&str>,
|
||||
) -> ConfiguredMarketplaceUpgradeOutcome {
|
||||
upgrade_configured_git_marketplaces_with_mode(
|
||||
codex_home,
|
||||
config_layer_stack,
|
||||
marketplace_name,
|
||||
PluginGitMode::Manual,
|
||||
)
|
||||
}
|
||||
|
||||
/// Applies the initiating operation's Git trust policy to every selected marketplace.
|
||||
pub(crate) fn upgrade_configured_git_marketplaces_with_mode(
|
||||
codex_home: &Path,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
marketplace_name: Option<&str>,
|
||||
mode: PluginGitMode,
|
||||
) -> ConfiguredMarketplaceUpgradeOutcome {
|
||||
let loaded = load_configured_git_marketplaces(config_layer_stack);
|
||||
let marketplaces = loaded
|
||||
@@ -118,6 +134,7 @@ pub fn upgrade_configured_git_marketplaces(
|
||||
&install_root,
|
||||
&marketplace,
|
||||
normalized_source.as_ref(),
|
||||
mode,
|
||||
) {
|
||||
Ok(Some(upgraded_root)) => upgraded_roots.push(upgraded_root),
|
||||
Ok(None) => {}
|
||||
@@ -208,6 +225,7 @@ fn upgrade_configured_git_marketplace(
|
||||
install_root: &Path,
|
||||
marketplace: &ConfiguredGitMarketplace,
|
||||
normalized_source: Option<&MarketplaceSource>,
|
||||
mode: PluginGitMode,
|
||||
) -> Result<Option<AbsolutePathBuf>, String> {
|
||||
validate_plugin_segment(&marketplace.name, "marketplace name")?;
|
||||
let (source, ref_name) = match normalized_source {
|
||||
@@ -217,7 +235,13 @@ fn upgrade_configured_git_marketplace(
|
||||
}
|
||||
None => (marketplace.source.as_str(), marketplace.ref_name.as_deref()),
|
||||
};
|
||||
let remote_revision = git_remote_revision(source, ref_name, MARKETPLACE_UPGRADE_GIT_TIMEOUT)?;
|
||||
let remote_revision = git_remote_revision(
|
||||
codex_home,
|
||||
source,
|
||||
ref_name,
|
||||
MARKETPLACE_UPGRADE_GIT_TIMEOUT,
|
||||
mode,
|
||||
)?;
|
||||
let destination = install_root.join(&marketplace.name);
|
||||
if validate_marketplace_root(&destination)
|
||||
.is_ok_and(|marketplace_name| marketplace_name == marketplace.name)
|
||||
@@ -245,11 +269,13 @@ fn upgrade_configured_git_marketplace(
|
||||
})?;
|
||||
|
||||
let activated_revision = clone_git_source(
|
||||
codex_home,
|
||||
source,
|
||||
ref_name,
|
||||
&marketplace.sparse_paths,
|
||||
staged_dir.path(),
|
||||
MARKETPLACE_UPGRADE_GIT_TIMEOUT,
|
||||
mode,
|
||||
)?;
|
||||
let marketplace_name = validate_marketplace_root(staged_dir.path())
|
||||
.map_err(|err| format!("failed to validate upgraded marketplace root: {err}"))?;
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use crate::PluginGitMode;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
@@ -6,9 +7,11 @@ use std::process::Stdio;
|
||||
use std::time::Duration;
|
||||
|
||||
pub(super) fn git_remote_revision(
|
||||
codex_home: &Path,
|
||||
source: &str,
|
||||
ref_name: Option<&str>,
|
||||
timeout: Duration,
|
||||
mode: PluginGitMode,
|
||||
) -> Result<String, String> {
|
||||
if let Some(ref_name) = ref_name
|
||||
&& is_full_git_sha(ref_name)
|
||||
@@ -17,8 +20,12 @@ pub(super) fn git_remote_revision(
|
||||
}
|
||||
|
||||
let ref_name = ref_name.unwrap_or("HEAD");
|
||||
let mut command = git_command(mode);
|
||||
let _trusted_repository = matches!(mode, PluginGitMode::Automatic)
|
||||
.then(|| crate::configure_trusted_git_repository(&mut command, codex_home))
|
||||
.transpose()?;
|
||||
let output = run_git_command_with_timeout(
|
||||
git_command().arg("ls-remote").arg(source).arg(ref_name),
|
||||
command.arg("ls-remote").arg(source).arg(ref_name),
|
||||
"git ls-remote marketplace source",
|
||||
timeout,
|
||||
)?;
|
||||
@@ -41,23 +48,29 @@ pub(super) fn git_remote_revision(
|
||||
}
|
||||
|
||||
pub(super) fn clone_git_source(
|
||||
codex_home: &Path,
|
||||
source: &str,
|
||||
ref_name: Option<&str>,
|
||||
sparse_paths: &[String],
|
||||
destination: &Path,
|
||||
timeout: Duration,
|
||||
mode: PluginGitMode,
|
||||
) -> Result<String, String> {
|
||||
let git_destination = git_path_arg(destination);
|
||||
let mut command = git_command(mode);
|
||||
let _trusted_repository = matches!(mode, PluginGitMode::Automatic)
|
||||
.then(|| crate::configure_trusted_git_repository(&mut command, codex_home))
|
||||
.transpose()?;
|
||||
if sparse_paths.is_empty() {
|
||||
let output = run_git_command_with_timeout(
|
||||
git_command().arg("clone").arg(source).arg(&git_destination),
|
||||
command.arg("clone").arg(source).arg(&git_destination),
|
||||
"git clone marketplace source",
|
||||
timeout,
|
||||
)?;
|
||||
ensure_git_success(&output, "git clone marketplace source")?;
|
||||
if let Some(ref_name) = ref_name {
|
||||
let output = run_git_command_with_timeout(
|
||||
git_command()
|
||||
git_command(mode)
|
||||
.arg("-C")
|
||||
.arg(&git_destination)
|
||||
.arg("checkout")
|
||||
@@ -67,11 +80,11 @@ pub(super) fn clone_git_source(
|
||||
)?;
|
||||
ensure_git_success(&output, "git checkout marketplace ref")?;
|
||||
}
|
||||
return git_worktree_revision(&git_destination, timeout);
|
||||
return git_worktree_revision(&git_destination, timeout, mode);
|
||||
}
|
||||
|
||||
let output = run_git_command_with_timeout(
|
||||
git_command()
|
||||
command
|
||||
.arg("clone")
|
||||
.arg("--filter=blob:none")
|
||||
.arg("--no-checkout")
|
||||
@@ -82,7 +95,7 @@ pub(super) fn clone_git_source(
|
||||
)?;
|
||||
ensure_git_success(&output, "git clone marketplace source")?;
|
||||
|
||||
let mut sparse_checkout = git_command();
|
||||
let mut sparse_checkout = git_command(mode);
|
||||
sparse_checkout
|
||||
.arg("-C")
|
||||
.arg(&git_destination)
|
||||
@@ -97,7 +110,7 @@ pub(super) fn clone_git_source(
|
||||
ensure_git_success(&output, "git sparse-checkout marketplace source")?;
|
||||
|
||||
let output = run_git_command_with_timeout(
|
||||
git_command()
|
||||
git_command(mode)
|
||||
.arg("-C")
|
||||
.arg(&git_destination)
|
||||
.arg("checkout")
|
||||
@@ -106,12 +119,16 @@ pub(super) fn clone_git_source(
|
||||
timeout,
|
||||
)?;
|
||||
ensure_git_success(&output, "git checkout marketplace ref")?;
|
||||
git_worktree_revision(&git_destination, timeout)
|
||||
git_worktree_revision(&git_destination, timeout, mode)
|
||||
}
|
||||
|
||||
fn git_worktree_revision(destination: &Path, timeout: Duration) -> Result<String, String> {
|
||||
fn git_worktree_revision(
|
||||
destination: &Path,
|
||||
timeout: Duration,
|
||||
mode: PluginGitMode,
|
||||
) -> Result<String, String> {
|
||||
let output = run_git_command_with_timeout(
|
||||
git_command()
|
||||
git_command(mode)
|
||||
.arg("-C")
|
||||
.arg(destination)
|
||||
.arg("rev-parse")
|
||||
@@ -133,10 +150,9 @@ fn is_full_git_sha(value: &str) -> bool {
|
||||
value.len() == 40 && value.chars().all(|ch| ch.is_ascii_hexdigit())
|
||||
}
|
||||
|
||||
fn git_command() -> Command {
|
||||
let mut command = Command::new("git");
|
||||
fn git_command(mode: PluginGitMode) -> Command {
|
||||
let mut command = mode.command(Path::new("git"));
|
||||
command
|
||||
.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG])
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.env("GIT_TERMINAL_PROMPT", "0");
|
||||
command
|
||||
@@ -239,7 +255,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn git_command_uses_path_lookup_with_stable_noninteractive_env() {
|
||||
let command = git_command();
|
||||
let command = git_command(crate::PluginGitMode::Automatic);
|
||||
|
||||
assert_eq!(command.get_program(), OsStr::new("git"));
|
||||
assert_eq!(
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
use super::*;
|
||||
use crate::PluginGitMode;
|
||||
use codex_config::ConfigLayerEntry;
|
||||
use codex_config::ConfigLayerSource;
|
||||
use codex_config::ConfigRequirements;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
use tempfile::TempDir;
|
||||
|
||||
@@ -87,6 +89,134 @@ source = {good_url:?}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn automatic_marketplace_git_ignores_inherited_repository_configuration() {
|
||||
const CHILD_HOME: &str = "CODEX_MARKETPLACE_GIT_ISOLATION_CHILD_HOME";
|
||||
const CHILD_SOURCE: &str = "CODEX_MARKETPLACE_GIT_ISOLATION_CHILD_SOURCE";
|
||||
|
||||
if let Some(codex_home) = std::env::var_os(CHILD_HOME) {
|
||||
let codex_home = PathBuf::from(codex_home);
|
||||
let source = std::env::var(CHILD_SOURCE).expect("read configured marketplace source");
|
||||
let config =
|
||||
format!("[marketplaces.trusted]\nsource_type = \"git\"\nsource = {source:?}\n");
|
||||
std::fs::write(codex_home.join(CONFIG_TOML_FILE), &config).expect("write config");
|
||||
let stack = config_layer_stack(&codex_home, &config);
|
||||
let outcome = upgrade_configured_git_marketplaces_with_mode(
|
||||
&codex_home,
|
||||
&stack,
|
||||
/*marketplace_name*/ None,
|
||||
PluginGitMode::Automatic,
|
||||
);
|
||||
assert_eq!(
|
||||
outcome,
|
||||
ConfiguredMarketplaceUpgradeOutcome {
|
||||
selected_marketplaces: vec!["trusted".to_string()],
|
||||
upgraded_roots: vec![
|
||||
AbsolutePathBuf::try_from(
|
||||
marketplace_install_root(&codex_home).join("trusted")
|
||||
)
|
||||
.expect("installed marketplace root"),
|
||||
],
|
||||
errors: Vec::new(),
|
||||
}
|
||||
);
|
||||
for (url, mode) in [
|
||||
("global:marketplace", PluginGitMode::Automatic),
|
||||
("manual:marketplace", PluginGitMode::Manual),
|
||||
] {
|
||||
let materialized = crate::loader::materialize_marketplace_plugin_source_with_mode(
|
||||
&codex_home,
|
||||
&crate::marketplace::MarketplacePluginSource::Git {
|
||||
url: url.to_string(),
|
||||
path: None,
|
||||
ref_name: matches!(mode, PluginGitMode::Manual)
|
||||
.then(|| "manual-filter".to_string()),
|
||||
sha: None,
|
||||
},
|
||||
mode,
|
||||
)
|
||||
.expect("materialize automatic or manually installed Git plugin");
|
||||
if matches!(mode, PluginGitMode::Manual) {
|
||||
assert!(
|
||||
std::fs::read_to_string(materialized.path.as_path().join("manual.txt"))
|
||||
.expect("read manually filtered checkout")
|
||||
.starts_with("git version ")
|
||||
);
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
let root = TempDir::new().expect("create temporary directory");
|
||||
let project = root.path().join("project");
|
||||
let codex_home = project.join("codex-home");
|
||||
let remote = root.path().join("remote");
|
||||
std::fs::create_dir_all(&codex_home).expect("create Codex home");
|
||||
std::fs::create_dir_all(&remote).expect("create remote marketplace");
|
||||
init_marketplace_repo(&remote, "trusted");
|
||||
run_git(&remote, &["switch", "--create", "manual-filter"]);
|
||||
std::fs::write(
|
||||
remote.join(".gitattributes"),
|
||||
"manual.txt filter=codex-required\n",
|
||||
)
|
||||
.expect("write required manual checkout filter");
|
||||
std::fs::write(remote.join("manual.txt"), "manual checkout")
|
||||
.expect("write manual checkout fixture");
|
||||
run_git(&remote, &["add", "."]);
|
||||
run_git(
|
||||
&remote,
|
||||
&["commit", "-m", "add required manual checkout filter"],
|
||||
);
|
||||
run_git(&remote, &["switch", "-"]);
|
||||
run_git(&project, &["init", "--quiet"]);
|
||||
|
||||
let source = url::Url::from_directory_path(&remote)
|
||||
.expect("remote marketplace URL")
|
||||
.to_string();
|
||||
let untrusted = url::Url::from_directory_path(root.path().join("missing"))
|
||||
.expect("malicious replacement URL")
|
||||
.to_string();
|
||||
let rewrite_key = format!("url.{untrusted}.insteadOf");
|
||||
run_git(&project, &["config", &rewrite_key, &source]);
|
||||
std::fs::write(
|
||||
root.path().join("global.conf"),
|
||||
"[url \"../remote\"]\n\tinsteadOf = global:marketplace\n[protocol \"file\"]\n\tallow = always\n",
|
||||
)
|
||||
.expect("write global Git configuration");
|
||||
std::fs::write(
|
||||
root.path().join("system.conf"),
|
||||
"[protocol \"file\"]\n\tallow = never\n",
|
||||
)
|
||||
.expect("write system Git configuration");
|
||||
let output = Command::new(std::env::current_exe().expect("locate test binary"))
|
||||
.args([
|
||||
"--exact",
|
||||
"marketplace_upgrade::tests::automatic_marketplace_git_ignores_inherited_repository_configuration",
|
||||
"--nocapture",
|
||||
])
|
||||
.current_dir(&project)
|
||||
.env(CHILD_HOME, &codex_home)
|
||||
.env(CHILD_SOURCE, &source)
|
||||
.env("GIT_CONFIG_GLOBAL", "../global.conf")
|
||||
.env("GIT_CONFIG_SYSTEM", "../system.conf")
|
||||
.env("GIT_DIR", project.join(".git"))
|
||||
.env("GIT_CONFIG_COUNT", "3")
|
||||
.env("GIT_CONFIG_KEY_0", &rewrite_key)
|
||||
.env("GIT_CONFIG_VALUE_0", &source)
|
||||
.env("GIT_CONFIG_KEY_1", "url.../remote.insteadOf")
|
||||
.env("GIT_CONFIG_VALUE_1", "manual:marketplace")
|
||||
.env("GIT_CONFIG_KEY_2", "filter.codex-required.smudge")
|
||||
.env("GIT_CONFIG_VALUE_2", "git version")
|
||||
.output()
|
||||
.expect("run marketplace Git isolation regression");
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"marketplace Git isolation failed: {}{}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn upgrade_uses_validated_source_for_git_operations() {
|
||||
let codex_home = TempDir::new().expect("create Codex home");
|
||||
@@ -124,6 +254,7 @@ ref = "missing-ref"
|
||||
&install_root,
|
||||
&marketplace,
|
||||
Some(&normalized_source),
|
||||
PluginGitMode::Manual,
|
||||
)
|
||||
.expect("upgrade should use the validated source")
|
||||
.expect("marketplace should be upgraded");
|
||||
@@ -168,6 +299,7 @@ fn up_to_date_fast_path_validates_marketplace_name() {
|
||||
&install_root,
|
||||
&marketplace,
|
||||
Some(&normalized_source),
|
||||
PluginGitMode::Manual,
|
||||
)
|
||||
.expect_err("mismatched marketplace name must not use the up-to-date fast path");
|
||||
|
||||
|
||||
@@ -38,28 +38,6 @@ const CURATED_PLUGINS_HTTP_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
const CURATED_PLUGINS_BACKUP_ARCHIVE_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
// Keep this comfortably above a normal sync attempt so we do not race another Codex process.
|
||||
const CURATED_PLUGINS_STALE_TEMP_DIR_MAX_AGE: Duration = Duration::from_secs(10 * 60);
|
||||
// These variables can redirect Git away from the repository selected by `-C`,
|
||||
// or inject command-scoped configuration into the sync commands.
|
||||
const REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES: &[&str] = &[
|
||||
"GIT_ALTERNATE_OBJECT_DIRECTORIES",
|
||||
"GIT_CEILING_DIRECTORIES",
|
||||
"GIT_COMMON_DIR",
|
||||
"GIT_CONFIG",
|
||||
"GIT_CONFIG_COUNT",
|
||||
"GIT_CONFIG_PARAMETERS",
|
||||
"GIT_DIR",
|
||||
"GIT_DISCOVERY_ACROSS_FILESYSTEM",
|
||||
"GIT_GRAFT_FILE",
|
||||
"GIT_IMPLICIT_WORK_TREE",
|
||||
"GIT_INDEX_FILE",
|
||||
"GIT_NAMESPACE",
|
||||
"GIT_OBJECT_DIRECTORY",
|
||||
"GIT_PREFIX",
|
||||
"GIT_REPLACE_REF_BASE",
|
||||
"GIT_SHALLOW_FILE",
|
||||
"GIT_WORK_TREE",
|
||||
];
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct GitHubRepositorySummary {
|
||||
default_branch: String,
|
||||
@@ -210,7 +188,7 @@ fn sync_openai_plugins_repo_via_git(
|
||||
) -> Result<String, String> {
|
||||
let repo_path = curated_plugins_repo_path(codex_home);
|
||||
let sha_path = codex_home.join(CURATED_PLUGINS_SHA_FILE);
|
||||
let remote_sha = git_ls_remote_head_sha(git_binary)?;
|
||||
let remote_sha = git_ls_remote_head_sha(codex_home, git_binary)?;
|
||||
let local_sha = read_local_git_or_sha_file(&repo_path, &sha_path, git_binary);
|
||||
|
||||
if local_sha.as_deref() == Some(remote_sha.as_str()) && repo_path.join(".git").is_dir() {
|
||||
@@ -629,8 +607,9 @@ fn read_local_git_or_sha_file(
|
||||
read_sha_file(sha_path)
|
||||
}
|
||||
|
||||
fn git_ls_remote_head_sha(git_binary: &Path) -> Result<String, String> {
|
||||
fn git_ls_remote_head_sha(codex_home: &Path, git_binary: &Path) -> Result<String, String> {
|
||||
let mut command = git_command(git_binary);
|
||||
let _trusted_repository = crate::configure_trusted_git_repository(&mut command, codex_home)?;
|
||||
command
|
||||
.arg("ls-remote")
|
||||
.arg("https://github.com/openai/plugins.git")
|
||||
@@ -683,14 +662,7 @@ fn git_head_sha(repo_path: &Path, git_binary: &Path) -> Result<String, String> {
|
||||
}
|
||||
|
||||
fn git_command(git_binary: &Path) -> Command {
|
||||
let mut command = Command::new(git_binary);
|
||||
command
|
||||
.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG])
|
||||
.env("GIT_OPTIONAL_LOCKS", "0");
|
||||
for name in REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES {
|
||||
command.env_remove(name);
|
||||
}
|
||||
command
|
||||
crate::PluginGitMode::Automatic.command(git_binary)
|
||||
}
|
||||
|
||||
#[cfg(any(target_os = "macos", test))]
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use super::*;
|
||||
use crate::git_policy::REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES;
|
||||
use crate::test_support::RecordingHttpClientSelector;
|
||||
use crate::test_support::recorded_http_client_urls;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
Reference in new issue
Block a user