Skip to content

exec-server: require explicit filesystem sandbox cwd#19046

Merged
bolinfest merged 1 commit intomainfrom
pr19046
Apr 22, 2026
Merged

exec-server: require explicit filesystem sandbox cwd#19046
bolinfest merged 1 commit intomainfrom
pr19046

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 22, 2026

Why

This is a cleanup PR for the PermissionProfile migration stack. #19016 fixed remote exec-server sandbox contexts so Docker-backed filesystem requests use a request/container cwd instead of leaking the local test runner cwd. That exposed the broader API problem: FileSystemSandboxContext::new(SandboxPolicy) could still reconstruct filesystem permissions by reading the exec-server process cwd with AbsolutePathBuf::current_dir().

That made cwd-dependent legacy entries, such as :cwd, :project_roots, and relative deny globs, depend on ambient process state instead of the request sandbox cwd. As later PRs make PermissionProfile the primary permissions abstraction, sandbox contexts should be explicit about whether they carry a request cwd or are profile-only. Removing the implicit constructor prevents new call sites from accidentally rebuilding permissions against the wrong cwd.

What changed

  • Removed FileSystemSandboxContext::new(SandboxPolicy).
  • Kept production callers on explicit constructors: from_legacy_sandbox_policy(..., cwd), from_permission_profile(...), and from_permission_profile_with_cwd(...).
  • Updated exec-server test helpers to construct PermissionProfile values directly instead of routing through legacy SandboxPolicy projections.
  • Updated the environment regression test to use an explicit restricted profile with no synthetic cwd.

Verification

  • cargo test -p codex-exec-server
  • just fix -p codex-exec-server

Stack created with Sapling. Best reviewed with ReviewStack.

Copy link
Copy Markdown
Contributor

@starr-openai starr-openai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @viyatb-oai since you were also working on this recently

@bolinfest bolinfest enabled auto-merge (squash) April 22, 2026 22:35
@bolinfest bolinfest merged commit 44dbd9e into main Apr 22, 2026
48 of 50 checks passed
@bolinfest bolinfest deleted the pr19046 branch April 22, 2026 23:05
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 22, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants