feat: load ExecPolicyManager from ConfigLayerStack by bolinfest · Pull Request #8453 · openai/codex

bolinfest · 2025-12-22T23:07:17Z

#8354 added support for in-repo .config/ files, so this PR updates the logic for loading *.rules files to load *.rules files from all relevant layers. The main change to the business logic is load_exec_policy() in codex-rs/core/src/exec_policy.rs.

Note this adds a config_folder() method to ConfigLayerSource that returns Option<AbsolutePathBuf> so that it is straightforward to iterate over the sources and get the associated config folder, if any.

…empty (#8456) This is necessary so that `$CODEX_HOME/skills` and `$CODEX_HOME/rules` still get loaded even if `$CODEX_HOME/config.toml` does not exist. See #8453. For now, it is possible to omit this layer when creating a dummy `ConfigLayerStack` in a test. We can revisit that later, if it turns out to be the right thing to do.

joshka-oai

Nothing blocking.

codex-rs/core/src/config_loader/state.rs

joshka-oai · 2025-12-23T00:56:08Z

codex-rs/core/src/exec_policy.rs

+    for layer in config_stack.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst) {
+        if let Some(config_folder) = layer.config_folder() {
+            #[expect(clippy::expect_used)]
+            let policy_dir = config_folder.join(RULES_DIR_NAME).expect("safe join");


(not a blocker - more interested in how this might be avoided in case the assumption is wrong)

So it seems like this would fail if the path can't be resolved against the base path (I'm assuming you've thought through why this happen a bit more than I have here...)

codex/codex-rs/utils/absolute-path/src/lib.rs

Lines 42 to 44 in 14dbd06

pub fn join<P: AsRef<Path>>(&self, path: P) -> std::io::Result<Self> {

Self::resolve_path_against_base(path, &self.0)

}

Are we missing a small ResolvedAbsolutePath concept here perhaps that would codify this more definitely?

Something like ResolvedAbsolutePath would avoid the expect(), but given that RULES_DIR_NAME is "rules" and is very, very unlikely to change (since it's now API), I don't see how the join() could fail given that AbsolutePathBuf is already guaranteeing its internal path is a valid absolute path?

seems reasonable.

joshka-oai · 2025-12-23T01:08:55Z

codex-rs/exec-server/src/posix.rs

+    // TODO(mbolin): At a minimum, `cwd` should be configurable via
+    // `codex/sandbox-state/update` or some other custom MCP call.


Probably worth expanding on this a little (I found myself digging for more context about the call here being unfamiliar with the code).
Something like: We currently don't include cwd in the layers to load exec policy rule from because ... In the future we should ... because ...

This is still an experimental feature that's on me to get over the finish line. It requires a bit of background to have the full context, I admit:

codex/codex-rs/exec-server/src/posix.rs

Lines 1 to 57 in 14dbd06

//! This is an MCP that implements an alternative `shell` tool with fine-grained privilege

//! escalation based on a per-exec() policy.

//!

//! We spawn Bash process inside a sandbox. The Bash we spawn is patched to allow us to intercept

//! every exec() call it makes by invoking a wrapper program and passing in the arguments it would

//! have passed to exec(). The Bash process (and its descendants) inherit a communication socket

//! from us, and we give its fd number in the CODEX_ESCALATE_SOCKET environment variable.

//!

//! When we intercept an exec() call, we send a message over the socket back to the main

//! MCP process. The MCP process can then decide whether to allow the exec() call to proceed

//! or to escalate privileges and run the requested command with elevated permissions. In the

//! latter case, we send a message back to the child requesting that it forward its open FDs to us.

//! We then execute the requested command on its behalf, patching in the forwarded FDs.

//!

//!

//! ### The privilege escalation flow

//!

//! Child MCP Bash Escalate Helper

//! |

//! o----->o

//! | |

//! | o--(exec)-->o

//! | | |

//! |o<-(EscalateReq)--o

//! || | |

//! |o--(Escalate)---->o

//! || | |

//! |o<---------(fds)--o

//! || | |

//! o<-----o | |

//! | || | |

//! x----->o | |

//! || | |

//! |x--(exit code)--->o

//! | | |

//! | o<--(exit)--x

//! | |

//! o<-----x

//!

//! ### The non-escalation flow

//!

//! MCP Bash Escalate Helper Child

//! |

//! o----->o

//! | |

//! | o--(exec)-->o

//! | | |

//! |o<-(EscalateReq)--o

//! || | |

//! |o-(Run)---------->o

//! | | |

//! | | x--(exec)-->o

//! | | |

//! | o<--------------(exit)--x

//! | |

//! o<-----x

//!

bolinfest mentioned this pull request Dec 23, 2025

chore: include User layer in ConfigLayerStack even if config.toml is empty #8456

Merged

bolinfest force-pushed the pr8453 branch 3 times, most recently from d78ca23 to 870cf22 Compare December 23, 2025 00:45

feat: load ExecPolicyManager from ConfigLayerStack

85497f7

bolinfest force-pushed the pr8453 branch from 870cf22 to 85497f7 Compare December 23, 2025 00:47

bolinfest requested review from joshka-oai and xl-openai December 23, 2025 00:49

joshka-oai approved these changes Dec 23, 2025

View reviewed changes

bolinfest merged commit 277babb into main Dec 23, 2025
52 checks passed

bolinfest deleted the pr8453 branch December 23, 2025 01:24

github-actions bot locked and limited conversation to collaborators Dec 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: load ExecPolicyManager from ConfigLayerStack#8453

feat: load ExecPolicyManager from ConfigLayerStack#8453
bolinfest merged 1 commit intomainfrom
pr8453

bolinfest commented Dec 22, 2025 •

edited

Loading

Uh oh!

joshka-oai left a comment

Uh oh!

Uh oh!

joshka-oai Dec 23, 2025

Uh oh!

bolinfest Dec 23, 2025

Uh oh!

joshka-oai Dec 23, 2025

Uh oh!

joshka-oai Dec 23, 2025

Uh oh!

bolinfest Dec 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	pub fn join<P: AsRef<Path>>(&self, path: P) -> std::io::Result<Self> {
	Self::resolve_path_against_base(path, &self.0)
	}

		// TODO(mbolin): At a minimum, `cwd` should be configurable via
		// `codex/sandbox-state/update` or some other custom MCP call.

	//! This is an MCP that implements an alternative `shell` tool with fine-grained privilege
	//! escalation based on a per-exec() policy.
	//!
	//! We spawn Bash process inside a sandbox. The Bash we spawn is patched to allow us to intercept
	//! every exec() call it makes by invoking a wrapper program and passing in the arguments it would
	//! have passed to exec(). The Bash process (and its descendants) inherit a communication socket
	//! from us, and we give its fd number in the CODEX_ESCALATE_SOCKET environment variable.
	//!
	//! When we intercept an exec() call, we send a message over the socket back to the main
	//! MCP process. The MCP process can then decide whether to allow the exec() call to proceed
	//! or to escalate privileges and run the requested command with elevated permissions. In the
	//! latter case, we send a message back to the child requesting that it forward its open FDs to us.
	//! We then execute the requested command on its behalf, patching in the forwarded FDs.
	//!
	//!
	//! ### The privilege escalation flow
	//!
	//! Child MCP Bash Escalate Helper
	//! \|
	//! o----->o
	//! \| \|
	//! \| o--(exec)-->o
	//! \| \| \|
	//! \|o<-(EscalateReq)--o
	//! \|\| \| \|
	//! \|o--(Escalate)---->o
	//! \|\| \| \|
	//! \|o<---------(fds)--o
	//! \|\| \| \|
	//! o<-----o \| \|
	//! \| \|\| \| \|
	//! x----->o \| \|
	//! \|\| \| \|
	//! \|x--(exit code)--->o
	//! \| \| \|
	//! \| o<--(exit)--x
	//! \| \|
	//! o<-----x
	//!
	//! ### The non-escalation flow
	//!
	//! MCP Bash Escalate Helper Child
	//! \|
	//! o----->o
	//! \| \|
	//! \| o--(exec)-->o
	//! \| \| \|
	//! \|o<-(EscalateReq)--o
	//! \|\| \| \|
	//! \|o-(Run)---------->o
	//! \| \| \|
	//! \| \| x--(exec)-->o
	//! \| \| \|
	//! \| o<--------------(exit)--x
	//! \| \|
	//! o<-----x
	//!

Conversation

bolinfest commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshka-oai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joshka-oai Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

bolinfest Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

joshka-oai Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

joshka-oai Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

bolinfest Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bolinfest commented Dec 22, 2025 •

edited

Loading