diff --git a/.github/dotslash-bash-config.json b/.github/dotslash-bash-config.json new file mode 100644 index 0000000000..59c46831f0 --- /dev/null +++ b/.github/dotslash-bash-config.json @@ -0,0 +1,32 @@ +{ + "outputs": { + "codex-bash": { + "platforms": { + "macos-aarch64": { + "name": "codex-bash-aarch64-apple-darwin.tar.gz", + "format": "tar.gz", + "hash": "sha256", + "path": "codex-bash/bin/bash" + }, + "macos-x86_64": { + "name": "codex-bash-x86_64-apple-darwin.tar.gz", + "format": "tar.gz", + "hash": "sha256", + "path": "codex-bash/bin/bash" + }, + "linux-x86_64": { + "name": "codex-bash-x86_64-unknown-linux-musl.tar.gz", + "format": "tar.gz", + "hash": "sha256", + "path": "codex-bash/bin/bash" + }, + "linux-aarch64": { + "name": "codex-bash-aarch64-unknown-linux-musl.tar.gz", + "format": "tar.gz", + "hash": "sha256", + "path": "codex-bash/bin/bash" + } + } + } + } +} diff --git a/.github/scripts/build-bash-release-artifact.sh b/.github/scripts/build-bash-release-artifact.sh new file mode 100755 index 0000000000..8dfe7ceedf --- /dev/null +++ b/.github/scripts/build-bash-release-artifact.sh @@ -0,0 +1,70 @@ +#!/usr/bin/env bash + +set -euo pipefail + +if [[ "$#" -ne 1 ]]; then + echo "usage: $0 " >&2 + exit 1 +fi + +archive_path="$1" +workspace="${GITHUB_WORKSPACE:?missing GITHUB_WORKSPACE}" +bash_commit="${BASH_COMMIT:?missing BASH_COMMIT}" +bash_patch="${BASH_PATCH:?missing BASH_PATCH}" +temp_root="${RUNNER_TEMP:-/tmp}" +work_root="$(mktemp -d "${temp_root%/}/codex-bash-release.XXXXXX")" +trap 'rm -rf "$work_root"' EXIT + +source_root="${work_root}/bash" +package_root="${work_root}/codex-bash" +wrapper_path="${work_root}/exec-wrapper" +stdout_path="${work_root}/stdout.txt" +wrapper_log_path="${work_root}/wrapper.log" +socket_probe_path="${work_root}/socket-probe.txt" + +git clone https://git.savannah.gnu.org/git/bash "$source_root" +cd "$source_root" +git checkout "$bash_commit" +git apply "${workspace}/${bash_patch}" +./configure --without-bash-malloc + +cores="$(command -v nproc >/dev/null 2>&1 && nproc || getconf _NPROCESSORS_ONLN)" +make -j"${cores}" + +cat > "$wrapper_path" <<'EOF' +#!/usr/bin/env bash +set -euo pipefail +: "${CODEX_WRAPPER_LOG:?missing CODEX_WRAPPER_LOG}" +: "${EXEC_WRAPPER:?missing EXEC_WRAPPER}" +: "${CODEX_ESCALATE_SOCKET:?missing CODEX_ESCALATE_SOCKET}" +printf 'socket-open\n' >&"${CODEX_ESCALATE_SOCKET}" +printf '%s\n' "$@" >> "$CODEX_WRAPPER_LOG" +file="$1" +shift +if [[ "$#" -eq 0 ]]; then + exec "$file" +fi +arg0="$1" +shift +exec -a "$arg0" "$file" "$@" +EOF +chmod +x "$wrapper_path" + +CODEX_WRAPPER_LOG="$wrapper_log_path" \ +CODEX_ESCALATE_SOCKET=9 \ +EXEC_WRAPPER="$wrapper_path" \ +"${source_root}/bash" \ + -c "\"${source_root}/bash\" -c '/bin/echo smoke-bash'" \ + > "$stdout_path" \ + 9> "$socket_probe_path" + +grep -Fx "smoke-bash" "$stdout_path" +grep -Fx "${source_root}/bash" "$wrapper_log_path" +grep -Fx "/bin/echo" "$wrapper_log_path" +[[ "$(grep -Fxc "socket-open" "$socket_probe_path")" -eq 2 ]] + +mkdir -p "$package_root/bin" "$(dirname "${workspace}/${archive_path}")" +cp "${source_root}/bash" "$package_root/bin/bash" +chmod +x "$package_root/bin/bash" + +(cd "$work_root" && tar -czf "${workspace}/${archive_path}" codex-bash) diff --git a/.github/scripts/build-zsh-release-artifact.sh b/.github/scripts/build-zsh-release-artifact.sh index 4fc3db3903..8ef20d54f3 100755 --- a/.github/scripts/build-zsh-release-artifact.sh +++ b/.github/scripts/build-zsh-release-artifact.sh @@ -20,6 +20,7 @@ package_root="${work_root}/codex-zsh" wrapper_path="${work_root}/exec-wrapper" stdout_path="${work_root}/stdout.txt" wrapper_log_path="${work_root}/wrapper.log" +socket_probe_path="${work_root}/socket-probe.txt" git clone https://git.code.sf.net/p/zsh/code "$source_root" cd "$source_root" @@ -35,7 +36,10 @@ cat > "$wrapper_path" <<'EOF' #!/usr/bin/env bash set -euo pipefail : "${CODEX_WRAPPER_LOG:?missing CODEX_WRAPPER_LOG}" -printf '%s\n' "$@" > "$CODEX_WRAPPER_LOG" +: "${EXEC_WRAPPER:?missing EXEC_WRAPPER}" +: "${CODEX_ESCALATE_SOCKET:?missing CODEX_ESCALATE_SOCKET}" +printf 'socket-open\n' >&"${CODEX_ESCALATE_SOCKET}" +printf '%s\n' "$@" >> "$CODEX_WRAPPER_LOG" file="$1" shift if [[ "$#" -eq 0 ]]; then @@ -48,11 +52,17 @@ EOF chmod +x "$wrapper_path" CODEX_WRAPPER_LOG="$wrapper_log_path" \ +CODEX_ESCALATE_SOCKET=9 \ EXEC_WRAPPER="$wrapper_path" \ -"${source_root}/Src/zsh" -fc '/bin/echo smoke-zsh' > "$stdout_path" +"${source_root}/Src/zsh" \ + -fc "\"${source_root}/Src/zsh\" -fc '/bin/echo smoke-zsh'" \ + > "$stdout_path" \ + 9> "$socket_probe_path" grep -Fx "smoke-zsh" "$stdout_path" +grep -Fx "${source_root}/Src/zsh" "$wrapper_log_path" grep -Fx "/bin/echo" "$wrapper_log_path" +[[ "$(grep -Fxc "socket-open" "$socket_probe_path")" -eq 2 ]] mkdir -p "$package_root/bin" "$(dirname "${workspace}/${archive_path}")" cp "${source_root}/Src/zsh" "$package_root/bin/zsh" diff --git a/.github/scripts/test-shell-exec-wrapper-chain.sh b/.github/scripts/test-shell-exec-wrapper-chain.sh new file mode 100755 index 0000000000..58f725bfc9 --- /dev/null +++ b/.github/scripts/test-shell-exec-wrapper-chain.sh @@ -0,0 +1,61 @@ +#!/usr/bin/env bash + +set -euo pipefail + +if [[ "$#" -ne 2 ]]; then + echo "usage: $0 " >&2 + exit 1 +fi + +bash_path="$(cd "$(dirname "$1")" && pwd)/$(basename "$1")" +zsh_path="$(cd "$(dirname "$2")" && pwd)/$(basename "$2")" +temp_root="${RUNNER_TEMP:-/tmp}" +work_root="$(mktemp -d "${temp_root%/}/codex-shell-chain.XXXXXX")" +trap 'rm -rf "$work_root"' EXIT + +wrapper_path="${work_root}/exec-wrapper" + +cat > "$wrapper_path" <<'EOF' +#!/usr/bin/env bash +set -euo pipefail +: "${CODEX_WRAPPER_LOG:?missing CODEX_WRAPPER_LOG}" +: "${EXEC_WRAPPER:?missing EXEC_WRAPPER}" +: "${CODEX_ESCALATE_SOCKET:?missing CODEX_ESCALATE_SOCKET}" +printf 'socket-open\n' >&"${CODEX_ESCALATE_SOCKET}" +printf '%s\n' "$@" >> "$CODEX_WRAPPER_LOG" +file="$1" +shift +if [[ "$#" -eq 0 ]]; then + exec "$file" +fi +arg0="$1" +shift +exec -a "$arg0" "$file" "$@" +EOF +chmod +x "$wrapper_path" + +run_chain() { + local outer_shell="$1" + local outer_flag="$2" + local inner_shell="$3" + local inner_flag="$4" + local marker="$5" + local wrapper_log="${work_root}/${marker}-wrapper.log" + local socket_probe="${work_root}/${marker}-socket.log" + local stdout="${work_root}/${marker}-stdout.txt" + local command + command="\"${inner_shell}\" ${inner_flag} '/bin/echo ${marker}'" + + CODEX_WRAPPER_LOG="$wrapper_log" \ + CODEX_ESCALATE_SOCKET=9 \ + EXEC_WRAPPER="$wrapper_path" \ + "$outer_shell" "$outer_flag" "$command" > "$stdout" 9> "$socket_probe" + + grep -Fx "$marker" "$stdout" + grep -Fx "$inner_shell" "$wrapper_log" + grep -Fx "/bin/echo" "$wrapper_log" + [[ "$(grep -Fxc "socket-open" "$socket_probe")" -eq 2 ]] +} + +run_chain "$bash_path" -c "$zsh_path" -fc bash-zsh-chain +run_chain "$zsh_path" -fc "$bash_path" -c zsh-bash-chain diff --git a/.github/workflows/rust-release-bash.yml b/.github/workflows/rust-release-bash.yml new file mode 100644 index 0000000000..1fb06ce960 --- /dev/null +++ b/.github/workflows/rust-release-bash.yml @@ -0,0 +1,223 @@ +name: rust-release-bash + +on: + push: + tags: + - "codex-bash-v*.*.*" + +env: + BASH_COMMIT: a8a1c2fac029404d3f42cd39f5a20f24b6e4fe4b + BASH_PATCH: codex-rs/shell-escalation/patches/bash-exec-wrapper.patch + ZSH_FORK_RELEASE_TAG: codex-zsh-v0.1.0 + +concurrency: + group: ${{ github.workflow }}::${{ github.ref_name }} + cancel-in-progress: false + +jobs: + metadata: + runs-on: ubuntu-latest + outputs: + release_tag: ${{ steps.release_tag.outputs.release_tag }} + + steps: + - name: Validate release tag + id: release_tag + env: + RELEASE_TAG: ${{ github.ref_name }} + shell: bash + run: | + set -euo pipefail + + if [[ ! "${RELEASE_TAG}" =~ ^codex-bash-v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "Tag ${RELEASE_TAG} does not match codex-bash-vX.Y.Z." >&2 + exit 1 + fi + + echo "release_tag=${RELEASE_TAG}" >> "${GITHUB_OUTPUT}" + + - name: Ensure release does not exist + env: + GH_TOKEN: ${{ github.token }} + RELEASE_TAG: ${{ steps.release_tag.outputs.release_tag }} + shell: bash + run: | + set -euo pipefail + + if gh release view "${RELEASE_TAG}" --repo "${GITHUB_REPOSITORY}" > /dev/null 2>&1; then + echo "Release ${RELEASE_TAG} already exists; publish changed artifacts under a new tag." >&2 + exit 1 + fi + + linux: + name: Build bash (Linux) - ${{ matrix.variant }} - ${{ matrix.target }} + needs: metadata + runs-on: ${{ matrix.runner }} + timeout-minutes: 30 + container: + image: ${{ matrix.image }} + + strategy: + fail-fast: false + matrix: + include: + - runner: ubuntu-24.04 + target: x86_64-unknown-linux-musl + variant: ubuntu-24.04 + image: ubuntu:24.04 + archive_name: codex-bash-x86_64-unknown-linux-musl.tar.gz + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + variant: ubuntu-24.04 + image: arm64v8/ubuntu:24.04 + archive_name: codex-bash-aarch64-unknown-linux-musl.tar.gz + + steps: + - name: Install build prerequisites + shell: bash + run: | + set -euo pipefail + apt-get update + DEBIAN_FRONTEND=noninteractive apt-get install -y \ + autoconf \ + bison \ + build-essential \ + ca-certificates \ + gettext \ + git \ + libncursesw5-dev + + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + + - name: Build, smoke-test, and stage bash artifact + shell: bash + run: | + "${GITHUB_WORKSPACE}/.github/scripts/build-bash-release-artifact.sh" \ + "dist/bash/${{ matrix.target }}/${{ matrix.archive_name }}" + + - uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + with: + name: codex-bash-${{ matrix.target }} + path: dist/bash/${{ matrix.target }}/* + if-no-files-found: error + + darwin: + name: Build bash (macOS) - ${{ matrix.variant }} - ${{ matrix.target }} + needs: metadata + runs-on: ${{ matrix.runner }} + timeout-minutes: 30 + + strategy: + fail-fast: false + matrix: + include: + - runner: macos-15-large + target: x86_64-apple-darwin + variant: macos-15 + archive_name: codex-bash-x86_64-apple-darwin.tar.gz + - runner: macos-15-xlarge + target: aarch64-apple-darwin + variant: macos-15 + archive_name: codex-bash-aarch64-apple-darwin.tar.gz + + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + + - name: Build, smoke-test, and stage bash artifact + shell: bash + run: | + "${GITHUB_WORKSPACE}/.github/scripts/build-bash-release-artifact.sh" \ + "dist/bash/${{ matrix.target }}/${{ matrix.archive_name }}" + + - uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + with: + name: codex-bash-${{ matrix.target }} + path: dist/bash/${{ matrix.target }}/* + if-no-files-found: error + + cross-shell-smoke: + name: Smoke test bash and zsh fork chain + needs: + - linux + runs-on: ubuntu-24.04 + permissions: + contents: read + actions: read + + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + + - name: Download bash artifact + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: codex-bash-x86_64-unknown-linux-musl + path: dist/bash + + - name: Download zsh fork artifact + env: + GH_TOKEN: ${{ github.token }} + shell: bash + run: | + set -euo pipefail + mkdir -p dist/zsh + gh release download "${ZSH_FORK_RELEASE_TAG}" \ + --repo "${GITHUB_REPOSITORY}" \ + --pattern codex-zsh-x86_64-unknown-linux-musl.tar.gz \ + --dir dist/zsh + + - name: Test nested shell forks + shell: bash + run: | + set -euo pipefail + mkdir -p dist/extracted/bash dist/extracted/zsh + tar -xzf dist/bash/codex-bash-x86_64-unknown-linux-musl.tar.gz \ + -C dist/extracted/bash + tar -xzf dist/zsh/codex-zsh-x86_64-unknown-linux-musl.tar.gz \ + -C dist/extracted/zsh + .github/scripts/test-shell-exec-wrapper-chain.sh \ + dist/extracted/bash/codex-bash/bin/bash \ + dist/extracted/zsh/codex-zsh/bin/zsh + + publish-release: + needs: + - metadata + - linux + - darwin + - cross-shell-smoke + runs-on: ubuntu-latest + permissions: + contents: write + actions: read + + steps: + - name: Checkout repository + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + path: dist + + - name: Create GitHub Release + uses: softprops/action-gh-release@153bb8e04406b158c6c84fc1615b65b24149a1fe # v2.6.1 + with: + tag_name: ${{ needs.metadata.outputs.release_tag }} + name: ${{ needs.metadata.outputs.release_tag }} + files: dist/** + # Keep bash artifact releases out of Codex's normal "latest release" channel. + prerelease: true + + - name: Publish DotSlash manifest + uses: facebook/dotslash-publish-release@9c9ec027515c34db9282a09a25a9cab5880b2c52 # v2 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + tag: ${{ needs.metadata.outputs.release_tag }} + config: .github/dotslash-bash-config.json diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index bc5953803e..fbae5fd049 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2557,6 +2557,7 @@ dependencies = [ "multimap", "pretty_assertions", "prost 0.14.3", + "regex-lite", "schemars 0.8.22", "serde", "serde_ignored", diff --git a/codex-rs/config/Cargo.toml b/codex-rs/config/Cargo.toml index 509ffd189a..4fe529d743 100644 --- a/codex-rs/config/Cargo.toml +++ b/codex-rs/config/Cargo.toml @@ -30,6 +30,7 @@ gethostname = { workspace = true } indexmap = { workspace = true, features = ["serde"] } multimap = { workspace = true } prost = "0.14.3" +regex-lite = { workspace = true } schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_ignored = { workspace = true } diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index e757c2149c..199ca4eafa 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -19,6 +19,7 @@ use super::requirements_exec_policy::RequirementsExecPolicyToml; use crate::Constrained; use crate::ConstraintError; use crate::ManagedHooksRequirementsToml; +use crate::mcp_requirements::McpServerRequirement; use crate::mcp_types::AppToolApproval; use crate::permissions_toml::PermissionProfileToml; use crate::types::WindowsSandboxModeToml; @@ -217,18 +218,6 @@ impl ConfigRequirements { } } -#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] -#[serde(untagged)] -pub enum McpServerIdentity { - Command { command: String }, - Url { url: String }, -} - -#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] -pub struct McpServerRequirement { - pub identity: McpServerIdentity, -} - #[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)] pub struct PluginRequirementsToml { pub mcp_servers: Option>, @@ -1201,6 +1190,25 @@ impl ConfigRequirementsToml { } } +fn validate_mcp_server_requirements( + requirements: &BTreeMap, + source: &RequirementSource, + plugin_name: Option<&str>, +) -> Result<(), ConstraintError> { + for (server_name, requirement) in requirements { + requirement + .validate() + .map_err(|reason| ConstraintError::McpServerRequirementParse { + server_name: plugin_name + .map(|plugin_name| format!("{plugin_name}/{server_name}")) + .unwrap_or_else(|| server_name.clone()), + requirement_source: source.clone(), + reason, + })?; + } + Ok(()) +} + impl TryFrom for ConfigRequirements { type Error = ConstraintError; @@ -1233,6 +1241,25 @@ impl TryFrom for ConfigRequirements { guardian_policy_config, } = toml; + if let Some(requirements) = &mcp_servers { + validate_mcp_server_requirements( + &requirements.value, + &requirements.source, + /*plugin_name*/ None, + )?; + } + if let Some(plugin_requirements) = &plugins { + for (plugin_name, plugin) in &plugin_requirements.value { + if let Some(requirements) = &plugin.mcp_servers { + validate_mcp_server_requirements( + requirements, + &plugin_requirements.source, + Some(plugin_name), + )?; + } + } + } + let approval_policy = match allowed_approval_policies { Some(Sourced { value: policies, @@ -1542,6 +1569,9 @@ pub fn sandbox_mode_requirement_for_permission_profile( mod tests { use super::*; use crate::HookEventsToml; + use crate::McpServerCommandMatcher; + use crate::McpServerIdentity; + use crate::McpServerValueMatcher; use anyhow::Result; use codex_execpolicy::Decision; use codex_execpolicy::Evaluation; @@ -3454,6 +3484,9 @@ command = "python3 /enterprise/hooks/pre.py" #[test] fn deserialize_mcp_server_requirements() -> Result<()> { let toml_str = r#" + [mcp_servers.docs] + description = "ignored legacy field" + [mcp_servers.docs.identity] command = "codex-mcp" @@ -3469,7 +3502,7 @@ command = "python3 /enterprise/hooks/pre.py" BTreeMap::from([ ( "docs".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "codex-mcp".to_string(), }, @@ -3477,7 +3510,7 @@ command = "python3 /enterprise/hooks/pre.py" ), ( "remote".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Url { url: "https://example.com/mcp".to_string(), }, @@ -3490,6 +3523,74 @@ command = "python3 /enterprise/hooks/pre.py" Ok(()) } + #[test] + fn deserialize_mcp_server_matcher_requirements() -> Result<()> { + let toml_str = r#" + [mcp_servers.internal_mcp_proxy.identity] + command = { executable = "company-cli", args = [ + { match = "exact", value = "mcp" }, + { match = "exact", value = "proxy" }, + { match = "exact", value = "--server" }, + { match = "regex", expression = '^https://[A-Za-z0-9-]+\.mcp\.internal\.example\.com(?::443)?(?:/.*)?$' }, + ] } + "#; + let requirements: ConfigRequirements = + with_unknown_source(from_str(toml_str)?).try_into()?; + + assert_eq!( + requirements.mcp_servers, + Some(Sourced::new( + BTreeMap::from([( + "internal_mcp_proxy".to_string(), + McpServerRequirement::Command(McpServerCommandMatcher { + executable: "company-cli".to_string(), + args: vec![ + McpServerValueMatcher::Exact { + value: "mcp".to_string(), + }, + McpServerValueMatcher::Exact { + value: "proxy".to_string(), + }, + McpServerValueMatcher::Exact { + value: "--server".to_string(), + }, + McpServerValueMatcher::Regex { + expression: r"^https://[A-Za-z0-9-]+\.mcp\.internal\.example\.com(?::443)?(?:/.*)?$" + .to_string(), + }, + ], + }), + )]), + RequirementSource::Unknown, + )) + ); + Ok(()) + } + + #[test] + fn invalid_mcp_server_requirement_regex_reports_the_server_name_and_source() -> Result<()> { + let toml_str = r#" + [mcp_servers.broken_rule.identity] + url = { match = "regex", expression = "[" } + "#; + + let err = ConfigRequirements::try_from(with_unknown_source(from_str(toml_str)?)) + .expect_err("invalid matcher regex should fail requirements normalization"); + let ConstraintError::McpServerRequirementParse { + server_name, + requirement_source, + reason, + } = err + else { + panic!("unexpected error: {err:?}"); + }; + + assert_eq!(server_name, "broken_rule"); + assert_eq!(requirement_source, RequirementSource::Unknown); + assert!(reason.contains("invalid regex `[`"), "{reason}"); + Ok(()) + } + #[test] fn deserialize_plugin_mcp_server_requirements() -> Result<()> { let toml_str = r#" @@ -3511,7 +3612,7 @@ command = "python3 /enterprise/hooks/pre.py" PluginRequirementsToml { mcp_servers: Some(BTreeMap::from([( "remote".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Url { url: "https://example.com/mcp".to_string(), }, @@ -3524,7 +3625,7 @@ command = "python3 /enterprise/hooks/pre.py" PluginRequirementsToml { mcp_servers: Some(BTreeMap::from([( "sample".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "sample-mcp".to_string(), }, @@ -3539,6 +3640,70 @@ command = "python3 /enterprise/hooks/pre.py" Ok(()) } + #[test] + fn deserialize_plugin_mcp_server_matcher_requirement() -> Result<()> { + let toml_str = r#" + [plugins."sample@test".mcp_servers.internal_proxy.identity] + command = { executable = "company-cli", args = [ + { match = "exact", value = "mcp" }, + { match = "regex", expression = '^https://[a-z]+\.example\.com$' }, + ] } + "#; + let requirements: ConfigRequirements = + with_unknown_source(from_str(toml_str)?).try_into()?; + + assert_eq!( + requirements.plugins, + Some(Sourced::new( + BTreeMap::from([( + "sample@test".to_string(), + PluginRequirementsToml { + mcp_servers: Some(BTreeMap::from([( + "internal_proxy".to_string(), + McpServerRequirement::Command(McpServerCommandMatcher { + executable: "company-cli".to_string(), + args: vec![ + McpServerValueMatcher::Exact { + value: "mcp".to_string(), + }, + McpServerValueMatcher::Regex { + expression: r"^https://[a-z]+\.example\.com$".to_string(), + }, + ], + }), + )])), + }, + )]), + RequirementSource::Unknown, + )) + ); + Ok(()) + } + + #[test] + fn invalid_plugin_mcp_server_regex_reports_plugin_and_server_name() -> Result<()> { + let toml_str = r#" + [plugins."sample@test".mcp_servers.broken_rule.identity] + url = { match = "regex", expression = "[" } + "#; + + let err = ConfigRequirements::try_from(with_unknown_source(from_str(toml_str)?)) + .expect_err("invalid plugin MCP regex should fail requirements normalization"); + let ConstraintError::McpServerRequirementParse { + server_name, + requirement_source, + reason, + } = err + else { + panic!("unexpected error: {err:?}"); + }; + + assert_eq!(server_name, "sample@test/broken_rule"); + assert_eq!(requirement_source, RequirementSource::Unknown); + assert!(reason.contains("invalid regex `[`"), "{reason}"); + Ok(()) + } + #[test] fn deserialize_exec_policy_requirements() -> Result<()> { let toml_str = r#" diff --git a/codex-rs/config/src/constraint.rs b/codex-rs/config/src/constraint.rs index 64b604cbc2..8628f3909c 100644 --- a/codex-rs/config/src/constraint.rs +++ b/codex-rs/config/src/constraint.rs @@ -24,6 +24,15 @@ pub enum ConstraintError { requirement_source: RequirementSource, reason: String, }, + + #[error( + "invalid requirement for MCP server `{server_name}` (set by {requirement_source}): {reason}" + )] + McpServerRequirementParse { + server_name: String, + requirement_source: RequirementSource, + reason: String, + }, } impl ConstraintError { diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index afda2cf9f3..61e6b6966a 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -12,6 +12,7 @@ mod key_aliases; pub mod loader; mod marketplace_edit; mod mcp_edit; +mod mcp_requirements; mod mcp_types; mod merge; mod overrides; @@ -66,8 +67,6 @@ pub use config_requirements::FilesystemDenyReadPattern; pub use config_requirements::MarketplaceAllowedSourceKind; pub use config_requirements::MarketplaceAllowedSourceToml; pub use config_requirements::MarketplaceRequirementsToml; -pub use config_requirements::McpServerIdentity; -pub use config_requirements::McpServerRequirement; pub use config_requirements::NetworkConstraints; pub use config_requirements::NetworkDomainPermissionToml; pub use config_requirements::NetworkDomainPermissionsToml; @@ -113,6 +112,10 @@ pub use marketplace_edit::remove_user_marketplace; pub use marketplace_edit::remove_user_marketplace_config; pub use mcp_edit::ConfigEditsBuilder; pub use mcp_edit::load_global_mcp_servers; +pub use mcp_requirements::McpServerCommandMatcher; +pub use mcp_requirements::McpServerIdentity; +pub use mcp_requirements::McpServerRequirement; +pub use mcp_requirements::McpServerValueMatcher; pub use mcp_types::AppToolApproval; pub use mcp_types::DEFAULT_MCP_SERVER_ENVIRONMENT_ID; pub use mcp_types::McpServerAuth; diff --git a/codex-rs/config/src/mcp_requirements.rs b/codex-rs/config/src/mcp_requirements.rs new file mode 100644 index 0000000000..b8f9a12d69 --- /dev/null +++ b/codex-rs/config/src/mcp_requirements.rs @@ -0,0 +1,163 @@ +use crate::mcp_types::McpServerConfig; +use crate::mcp_types::McpServerTransportConfig; +use regex_lite::Regex; +use serde::Deserialize; + +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(untagged)] +pub enum McpServerIdentity { + Command { command: String }, + Url { url: String }, +} + +/// String matching operations available to managed MCP server matchers. +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(tag = "match", rename_all = "snake_case", deny_unknown_fields)] +pub enum McpServerValueMatcher { + Exact { value: String }, + Prefix { value: String }, + Regex { expression: String }, +} + +impl McpServerValueMatcher { + fn compile_full_regex(expression: &str) -> Result { + Regex::new(&format!(r"\A(?:{expression})\z")).map_err(|err| { + format!("regex `{expression}` cannot be used for full-value matching: {err}") + }) + } + + fn validate(&self) -> Result<(), String> { + let Self::Regex { expression } = self else { + return Ok(()); + }; + + Regex::new(expression).map_err(|err| format!("invalid regex `{expression}`: {err}"))?; + Self::compile_full_regex(expression).map(|_| ()) + } + + fn matches(&self, candidate: &str) -> bool { + match self { + Self::Exact { value } => candidate == value, + Self::Prefix { value } => candidate.starts_with(value), + Self::Regex { expression } => Self::compile_full_regex(expression) + .ok() + .is_some_and(|regex| regex.is_match(candidate)), + } + } +} + +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct McpServerCommandMatcher { + pub executable: String, + pub args: Vec, +} + +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +struct RawMcpServerCommandIdentity { + command: McpServerCommandMatcher, +} + +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +struct RawMcpServerUrlIdentity { + url: McpServerValueMatcher, +} + +/// A requirement for one named MCP server. +/// +/// The `Identity` variant preserves the released exact-match contract. The +/// command and URL variants are the normalized matcher-based forms accepted +/// under the `identity` key. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum McpServerRequirement { + Identity { identity: McpServerIdentity }, + Command(McpServerCommandMatcher), + Url(McpServerValueMatcher), +} + +#[derive(Deserialize)] +struct RawMcpServerRequirement { + identity: RawMcpServerIdentity, +} + +#[derive(Deserialize)] +#[serde(untagged)] +enum RawMcpServerIdentity { + Exact(McpServerIdentity), + Command(RawMcpServerCommandIdentity), + Url(RawMcpServerUrlIdentity), +} + +impl<'de> Deserialize<'de> for McpServerRequirement { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let RawMcpServerRequirement { identity } = + RawMcpServerRequirement::deserialize(deserializer)?; + match identity { + RawMcpServerIdentity::Exact(identity) => Ok(Self::Identity { identity }), + RawMcpServerIdentity::Command(matcher) => Ok(Self::Command(matcher.command)), + RawMcpServerIdentity::Url(matcher) => Ok(Self::Url(matcher.url)), + } + } +} + +impl McpServerRequirement { + pub(crate) fn validate(&self) -> Result<(), String> { + match self { + Self::Identity { .. } => Ok(()), + Self::Command(matcher) => { + for (index, arg) in matcher.args.iter().enumerate() { + arg.validate().map_err(|err| { + format!("invalid argument matcher at index {index}: {err}") + })?; + } + Ok(()) + } + Self::Url(matcher) => matcher.validate(), + } + } + + pub fn matches(&self, server: &McpServerConfig) -> bool { + match (self, &server.transport) { + ( + Self::Identity { + identity: + McpServerIdentity::Command { + command: want_command, + }, + }, + McpServerTransportConfig::Stdio { + command: got_command, + .. + }, + ) => got_command == want_command, + ( + Self::Identity { + identity: McpServerIdentity::Url { url: want_url }, + }, + McpServerTransportConfig::StreamableHttp { url: got_url, .. }, + ) => got_url == want_url, + (Self::Command(matcher), McpServerTransportConfig::Stdio { command, args, .. }) => { + matcher.executable == *command + && matcher.args.len() == args.len() + && matcher + .args + .iter() + .zip(args) + .all(|(matcher, arg)| matcher.matches(arg)) + } + (Self::Url(matcher), McpServerTransportConfig::StreamableHttp { url, .. }) => { + matcher.matches(url) + } + _ => false, + } + } +} + +#[cfg(test)] +#[path = "mcp_requirements_tests.rs"] +mod tests; diff --git a/codex-rs/config/src/mcp_requirements_tests.rs b/codex-rs/config/src/mcp_requirements_tests.rs new file mode 100644 index 0000000000..b854873568 --- /dev/null +++ b/codex-rs/config/src/mcp_requirements_tests.rs @@ -0,0 +1,217 @@ +use super::*; +use crate::mcp_types::McpServerConfig; +use pretty_assertions::assert_eq; +use std::collections::HashMap; + +fn stdio_server(command: &str, args: &[&str]) -> McpServerConfig { + McpServerConfig { + transport: McpServerTransportConfig::Stdio { + command: command.to_string(), + args: args.iter().map(ToString::to_string).collect(), + env: None, + env_vars: Vec::new(), + cwd: None, + }, + environment_id: crate::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), + enabled: true, + required: false, + supports_parallel_tool_calls: false, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + default_tools_approval_mode: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + oauth: None, + oauth_resource: None, + tools: HashMap::new(), + } +} + +#[test] +fn command_matcher_matches_exact_positional_arguments() { + let requirement = McpServerRequirement::Command(McpServerCommandMatcher { + executable: "company-cli".to_string(), + args: vec![ + McpServerValueMatcher::Exact { + value: "mcp".to_string(), + }, + McpServerValueMatcher::Regex { + expression: r"https://[a-z]+\.example\.com".to_string(), + }, + ], + }); + + assert!(requirement.matches(&stdio_server( + "company-cli", + &["mcp", "https://pricing.example.com"] + ))); + assert!(!requirement.matches(&stdio_server( + "company-cli", + &["https://pricing.example.com", "mcp"] + ))); + assert!(!requirement.matches(&stdio_server( + "company-cli", + &["mcp", "https://pricing.example.com", "--verbose"] + ))); + assert!(!requirement.matches(&stdio_server( + "/usr/local/bin/company-cli", + &["mcp", "https://pricing.example.com"] + ))); +} + +#[test] +fn regex_matcher_requires_a_full_value_match() { + let matcher = McpServerValueMatcher::Regex { + expression: "mcp".to_string(), + }; + + assert!(matcher.matches("mcp")); + assert!(!matcher.matches("mcp-proxy")); + assert!(!matcher.matches("prefix-mcp")); +} + +#[test] +fn regex_matcher_allows_a_later_alternative_to_match_the_full_value() { + let matcher = McpServerValueMatcher::Regex { + expression: r"https://api\.example\.com|https://api\.example\.com/mcp".to_string(), + }; + + assert!(matcher.matches("https://api.example.com/mcp")); +} + +#[test] +fn regex_matcher_validation_rejects_expression_that_cannot_be_wrapped() { + let matcher = McpServerValueMatcher::Regex { + expression: "(?x)mcp # trailing comment".to_string(), + }; + + let err = matcher + .validate() + .expect_err("expression should not be valid for full-value matching"); + assert!( + err.contains("cannot be used for full-value matching"), + "{err}" + ); +} + +#[test] +fn legacy_command_identity_keeps_ignoring_arguments() { + let requirement: McpServerRequirement = toml::from_str( + r#" +[identity] +command = "company-cli" +"#, + ) + .expect("legacy command identity"); + + assert!(requirement.matches(&stdio_server( + "company-cli", + &["any", "arguments", "remain", "allowed"] + ))); + assert!(!requirement.matches(&stdio_server("different-cli", &[]))); +} + +#[test] +fn requirement_deserializes_command_and_url_matcher_shapes() { + let command: McpServerRequirement = toml::from_str( + r#" +[identity] +command = { executable = "company-cli", args = [ + { match = "exact", value = "mcp" }, + { match = "regex", expression = '^https://[a-z]+\.example\.com$' }, +] } +"#, + ) + .expect("command matcher"); + let url: McpServerRequirement = toml::from_str( + r#" +[identity] +url = { match = "prefix", value = "https://mcp.example.com/" } +"#, + ) + .expect("URL matcher"); + + assert_eq!( + command, + McpServerRequirement::Command(McpServerCommandMatcher { + executable: "company-cli".to_string(), + args: vec![ + McpServerValueMatcher::Exact { + value: "mcp".to_string(), + }, + McpServerValueMatcher::Regex { + expression: r"^https://[a-z]+\.example\.com$".to_string(), + }, + ], + }) + ); + assert_eq!( + url, + McpServerRequirement::Url(McpServerValueMatcher::Prefix { + value: "https://mcp.example.com/".to_string(), + }) + ); +} + +#[test] +fn requirement_rejects_matchers_outside_identity() { + for contents in [ + r#" +command = "company-cli" +"#, + r#" +command = { executable = "company-cli", args = [] } +"#, + r#" +url = { match = "prefix", value = "https://mcp.example.com/" } +"#, + ] { + let err = toml::from_str::(contents) + .expect_err("MCP server requirements should use the identity key"); + assert!( + err.to_string().contains("missing field `identity`"), + "{err}" + ); + } +} + +#[test] +fn matcher_identity_rejects_unknown_fields() { + for contents in [ + r#" +[identity] +unknown = "value" +command = { executable = "company-cli", args = [] } +"#, + r#" +[identity] +command = { executable = "company-cli", args = [], unknown = "value" } +"#, + ] { + toml::from_str::(contents) + .expect_err("matcher identities should reject unknown fields"); + } +} + +#[test] +fn identity_requirement_keeps_ignoring_unrelated_sibling_fields() { + let requirement: McpServerRequirement = toml::from_str( + r#" +unrelated = "ignored" +[identity] +command = "company-cli" +"#, + ) + .expect("legacy identity with unrelated sibling field"); + + assert_eq!( + requirement, + McpServerRequirement::Identity { + identity: McpServerIdentity::Command { + command: "company-cli".to_string(), + }, + } + ); +} diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 237e67d6c0..6f7114928e 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -6,6 +6,10 @@ use codex_config::CONFIG_TOML_FILE; use codex_config::ConfigLayerEntry; use codex_config::ConfigLayerSource; use codex_config::ConfigLayerStack; +use codex_config::McpServerCommandMatcher; +use codex_config::McpServerIdentity; +use codex_config::McpServerRequirement; +use codex_config::McpServerValueMatcher; use codex_config::ProfileV2Name; use codex_config::RequirementSource; use codex_config::Sourced; @@ -113,11 +117,15 @@ use std::time::Duration; use tempfile::TempDir; fn stdio_mcp(command: &str) -> McpServerConfig { + stdio_mcp_with_args(command, &[]) +} + +fn stdio_mcp_with_args(command: &str, args: &[&str]) -> McpServerConfig { McpServerConfig { auth: Default::default(), transport: McpServerTransportConfig::Stdio { command: command.to_string(), - args: Vec::new(), + args: args.iter().map(ToString::to_string).collect(), env: None, env_vars: Vec::new(), cwd: None, @@ -4128,7 +4136,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() { BTreeMap::from([ ( MISMATCHED_URL_SERVER.to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Url { url: "https://example.com/other".to_string(), }, @@ -4136,7 +4144,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() { ), ( MISMATCHED_COMMAND_SERVER.to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "other-cmd".to_string(), }, @@ -4144,7 +4152,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() { ), ( MATCHED_URL_SERVER.to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Url { url: GOOD_URL.to_string(), }, @@ -4152,7 +4160,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() { ), ( MATCHED_COMMAND_SERVER.to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: GOOD_CMD.to_string(), }, @@ -4209,6 +4217,117 @@ fn filter_mcp_servers_by_allowlist_allows_all_when_unset() { ); } +#[test] +fn filter_mcp_servers_by_matchers_enforces_command_and_positional_args() { + let mut servers = HashMap::from([ + ( + "internal_mcp_proxy".to_string(), + stdio_mcp_with_args( + "company-cli", + &[ + "mcp", + "proxy", + "--server", + "https://pricing.mcp.internal.example.com", + ], + ), + ), + ( + "unlisted".to_string(), + stdio_mcp_with_args( + "company-cli", + &[ + "mcp", + "proxy", + "--server", + "https://pricing.mcp.internal.example.com", + ], + ), + ), + ( + "wrong-order".to_string(), + stdio_mcp_with_args( + "company-cli", + &[ + "proxy", + "mcp", + "--server", + "https://pricing.mcp.internal.example.com", + ], + ), + ), + ( + "trailing-arg".to_string(), + stdio_mcp_with_args( + "company-cli", + &[ + "mcp", + "proxy", + "--server", + "https://pricing.mcp.internal.example.com", + "--verbose", + ], + ), + ), + ( + "wrong-host".to_string(), + stdio_mcp_with_args( + "company-cli", + &["mcp", "proxy", "--server", "https://mcp.example.com"], + ), + ), + ]); + let source = RequirementSource::LegacyManagedConfigTomlFromMdm; + let requirement = McpServerRequirement::Command(McpServerCommandMatcher { + executable: "company-cli".to_string(), + args: vec![ + McpServerValueMatcher::Exact { + value: "mcp".to_string(), + }, + McpServerValueMatcher::Exact { + value: "proxy".to_string(), + }, + McpServerValueMatcher::Exact { + value: "--server".to_string(), + }, + McpServerValueMatcher::Regex { + expression: + r"^https://[A-Za-z0-9-]+\.mcp\.internal\.example\.com(?::443)?(?:/.*)?$" + .to_string(), + }, + ], + }); + let requirements = Sourced::new( + BTreeMap::from([ + ("internal_mcp_proxy".to_string(), requirement.clone()), + ("wrong-order".to_string(), requirement.clone()), + ("trailing-arg".to_string(), requirement.clone()), + ("wrong-host".to_string(), requirement), + ]), + source.clone(), + ); + + filter_mcp_servers_by_requirements(&mut servers, Some(&requirements)); + + let reason = Some(McpServerDisabledReason::Requirements { source }); + assert_eq!( + servers + .iter() + .map(|(name, server)| ( + name.clone(), + (server.enabled, server.disabled_reason.clone()) + )) + .collect::)>>(), + HashMap::from([ + ("internal_mcp_proxy".to_string(), (true, None)), + ("unlisted".to_string(), (false, reason.clone())), + ("wrong-order".to_string(), (false, reason.clone())), + ("trailing-arg".to_string(), (false, reason.clone())), + ("wrong-host".to_string(), (false, reason)), + ]) + ); +} + #[test] fn filter_mcp_servers_by_allowlist_blocks_all_when_empty() { let mut servers = HashMap::from([ @@ -4259,7 +4378,7 @@ fn filter_plugin_mcp_servers_by_allowlist_enforces_plugin_and_identity_rules() { mcp_servers: Some(BTreeMap::from([ ( MATCHED_SERVER.to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: GOOD_CMD.to_string(), }, @@ -4267,7 +4386,7 @@ fn filter_plugin_mcp_servers_by_allowlist_enforces_plugin_and_identity_rules() { ), ( MISMATCHED_SERVER.to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: GOOD_CMD.to_string(), }, @@ -4308,7 +4427,7 @@ fn filter_plugin_mcp_servers_by_allowlist_blocks_unlisted_plugin() { codex_config::PluginRequirementsToml { mcp_servers: Some(BTreeMap::from([( "server-a".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "cmd-a".to_string(), }, @@ -4339,6 +4458,65 @@ fn filter_plugin_mcp_servers_by_allowlist_blocks_unlisted_plugin() { ); } +#[test] +fn filter_plugin_mcp_servers_by_matchers_enforces_name_and_invocation() { + const MATCHED_SERVER: &str = "matched"; + const MISMATCHED_SERVER: &str = "mismatched"; + const UNLISTED_SERVER: &str = "unlisted"; + + let mut servers = HashMap::from([ + ( + MATCHED_SERVER.to_string(), + stdio_mcp_with_args("company-cli", &["approved"]), + ), + ( + MISMATCHED_SERVER.to_string(), + stdio_mcp_with_args("company-cli", &["rejected"]), + ), + ( + UNLISTED_SERVER.to_string(), + stdio_mcp_with_args("company-cli", &["approved"]), + ), + ]); + let source = RequirementSource::LegacyManagedConfigTomlFromMdm; + let requirement = McpServerRequirement::Command(McpServerCommandMatcher { + executable: "company-cli".to_string(), + args: vec![McpServerValueMatcher::Exact { + value: "approved".to_string(), + }], + }); + let requirements = Sourced::new( + BTreeMap::from([( + "sample@test".to_string(), + codex_config::PluginRequirementsToml { + mcp_servers: Some(BTreeMap::from([ + (MATCHED_SERVER.to_string(), requirement.clone()), + (MISMATCHED_SERVER.to_string(), requirement), + ])), + }, + )]), + source.clone(), + ); + + filter_plugin_mcp_servers_by_requirements("sample@test", &mut servers, Some(&requirements)); + + let reason = Some(McpServerDisabledReason::Requirements { source }); + assert_eq!( + servers + .iter() + .map(|(name, server)| ( + name.clone(), + (server.enabled, server.disabled_reason.clone()) + )) + .collect::)>>(), + HashMap::from([ + (MATCHED_SERVER.to_string(), (true, None)), + (MISMATCHED_SERVER.to_string(), (false, reason.clone())), + (UNLISTED_SERVER.to_string(), (false, reason)), + ]) + ); +} + #[tokio::test] async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io::Result<()> { let codex_home = TempDir::new()?; @@ -4348,7 +4526,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io:: let mcp_requirements = BTreeMap::from([ ( "session_overrides_user".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "session-command".to_string(), }, @@ -4356,7 +4534,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io:: ), ( "managed_overrides_session".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "managed-command".to_string(), }, @@ -4364,7 +4542,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io:: ), ( "fresh_global".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "fresh-global-command".to_string(), }, @@ -4372,7 +4550,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io:: ), ( "fresh_project".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "fresh-project-command".to_string(), }, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 897a31c8db..d90d010102 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -14,7 +14,6 @@ use codex_config::ConfigRequirements; use codex_config::ConfigRequirementsToml; use codex_config::ConstrainedWithSource; use codex_config::FeatureRequirementsToml; -use codex_config::McpServerIdentity; use codex_config::McpServerRequirement; use codex_config::PluginRequirementsToml; use codex_config::ProfileV2Name; @@ -40,7 +39,6 @@ use codex_config::types::AuthKeyringBackendKind; use codex_config::types::History; use codex_config::types::McpServerConfig; use codex_config::types::McpServerDisabledReason; -use codex_config::types::McpServerTransportConfig; use codex_config::types::MemoriesConfig; use codex_config::types::ModelAvailabilityNuxConfig; use codex_config::types::Notice; @@ -1890,7 +1888,7 @@ fn filter_mcp_servers_by_requirements( let allowed = allowlist .value .get(name) - .is_some_and(|requirement| mcp_server_matches_requirement(requirement, server)); + .is_some_and(|requirement| requirement.matches(server)); if allowed { server.disabled_reason = None; } else { @@ -1919,7 +1917,7 @@ fn filter_plugin_mcp_servers_by_requirements( for (name, server) in mcp_servers.iter_mut() { let allowed = plugin_mcp_requirements .and_then(|mcp_requirements| mcp_requirements.get(name)) - .is_some_and(|requirement| mcp_server_matches_requirement(requirement, server)); + .is_some_and(|requirement| requirement.matches(server)); if allowed { server.disabled_reason = None; } else { @@ -1982,26 +1980,6 @@ where Ok(false) } -fn mcp_server_matches_requirement( - requirement: &McpServerRequirement, - server: &McpServerConfig, -) -> bool { - match &requirement.identity { - McpServerIdentity::Command { - command: want_command, - } => matches!( - &server.transport, - McpServerTransportConfig::Stdio { command: got_command, .. } - if got_command == want_command - ), - McpServerIdentity::Url { url: want_url } => matches!( - &server.transport, - McpServerTransportConfig::StreamableHttp { url: got_url, .. } - if got_url == want_url - ), - } -} - pub async fn load_global_mcp_servers( codex_home: &Path, ) -> std::io::Result> { diff --git a/codex-rs/shell-escalation/README.md b/codex-rs/shell-escalation/README.md index 69cd038220..c6836cfebb 100644 --- a/codex-rs/shell-escalation/README.md +++ b/codex-rs/shell-escalation/README.md @@ -15,6 +15,30 @@ decision to the shell-escalation protocol over a shared file descriptor (specifi - `Deny`: the server has declared the proposed command to be forbidden, so `codex-execve-wrapper` prints an error to `stderr` and exits with `1`. +Both patched shells use `EXEC_WRAPPER` to locate `codex-execve-wrapper` and +preserve `CODEX_ESCALATE_SOCKET` as the inherited escalation socket file +descriptor. This shared environment contract lets intercepted commands pass +through trees containing either shell without losing the escalation session. + +## Patched bash + +We carry a small patch to `execute_cmd.c` (see +`patches/bash-exec-wrapper.patch`) that adds support for `EXEC_WRAPPER`. The +patch applies to `a8a1c2fac029404d3f42cd39f5a20f24b6e4fe4b` from +https://git.savannah.gnu.org/git/bash. To rebuild manually: + +```bash +git clone https://git.savannah.gnu.org/git/bash +git checkout a8a1c2fac029404d3f42cd39f5a20f24b6e4fe4b +git apply /path/to/patches/bash-exec-wrapper.patch +./configure --without-bash-malloc +make -j"$(nproc)" +``` + +Release artifacts are built by `.github/workflows/rust-release-bash.yml` when +a `codex-bash-vX.Y.Z` tag is pushed. When the bash commit or patch changes, +publish the next version tag. + ## Patched zsh We carry a small patch to `Src/exec.c` (see `patches/zsh-exec-wrapper.patch`) that adds support for `EXEC_WRAPPER`. The patch applies to `77045ef899e53b9598bebc5a41db93a548a40ca6` from https://git.code.sf.net/p/zsh/code. To rebuild manually: diff --git a/codex-rs/shell-escalation/patches/bash-exec-wrapper.patch b/codex-rs/shell-escalation/patches/bash-exec-wrapper.patch new file mode 100644 index 0000000000..63e480229d --- /dev/null +++ b/codex-rs/shell-escalation/patches/bash-exec-wrapper.patch @@ -0,0 +1,24 @@ +diff --git a/execute_cmd.c b/execute_cmd.c +index 070f5119..d20ad2b9 100644 +--- a/execute_cmd.c ++++ b/execute_cmd.c +@@ -6129,6 +6129,19 @@ shell_execve (char *command, char **args, char **env) + char sample[HASH_BANG_BUFSIZ]; + size_t larray; + ++ char *exec_wrapper = getenv("EXEC_WRAPPER"); ++ if (exec_wrapper && *exec_wrapper && !whitespace (*exec_wrapper)) ++ { ++ char *orig_command = command; ++ ++ larray = strvec_len (args); ++ ++ memmove (args + 2, args, (++larray) * sizeof (char *)); ++ args[0] = exec_wrapper; ++ args[1] = orig_command; ++ command = exec_wrapper; ++ } ++ + SETOSTYPE (0); /* Some systems use for USG/POSIX semantics */ + execve (command, args, env); + i = errno; /* error from execve() */ diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index ed9aeffddb..7a35cc4506 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -699,7 +699,7 @@ mod tests { mcp_servers: Some(Sourced::new( BTreeMap::from([( "docs".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "codex-mcp".to_string(), }, @@ -778,7 +778,7 @@ mod tests { hooks: None, mcp_servers: Some(BTreeMap::from([( "docs".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "codex-mcp".to_string(), },