Commit Graph

5 Commits

  • windows-sandbox: pass workspace roots to runner (#24108)
    ## Why
    
    #23813 switches the Windows sandbox runner path to `PermissionProfile`,
    but it still left one runtime anchor for resolving symbolic
    `:workspace_roots` entries. That is not enough once a turn has multiple
    effective workspace roots: exact entries and deny globs under
    `:workspace_roots` need to be materialized for every runtime root before
    the command runner chooses token mode or builds ACL plans.
    
    ## What Changed
    
    - Replaces the Windows runner/setup `permission_profile_cwd` plumbing
    with `workspace_roots: Vec<AbsolutePathBuf>`.
    - Resolves Windows-local `PermissionProfile` data with
    `materialize_project_roots_with_workspace_roots(...)` instead of the
    single-cwd helper.
    - Threads `Config::effective_workspace_roots()` through core execution,
    unified exec, TUI setup/read-grant flows, app-server setup, app-server
    `command/exec`, and `debug sandbox` on Windows.
    - Preserves those workspace roots through the zsh-fork escalation
    executor instead of rebuilding them from `sandbox_policy_cwd`.
    - Makes `ExecRequest::new(...)` and the remaining
    `build_exec_request(...)` helper path take
    `windows_sandbox_workspace_roots` explicitly so new call sites cannot
    silently fall back to `vec![cwd]`.
    - Clarifies the `debug sandbox` non-Windows comment: remaining
    cwd-dependent resolution still uses `sandbox_policy_cwd`, while
    `:workspace_roots` entries are already materialized from config roots.
    - Updates elevated runner IPC `SpawnRequest` to send `workspace_roots`
    and bumps the framed IPC protocol version to `3` for the payload shape
    change.
    - Adds Windows-local resolver coverage for expanding exact and glob
    `:workspace_roots` entries across multiple roots, plus core helper
    coverage proving explicit roots are preserved.
    
    ## Verification
    
    - `cargo check -p codex-windows-sandbox -p codex-core -p codex-tui -p
    codex-cli -p codex-app-server`
    - `cargo test -p codex-windows-sandbox`
    - `cargo test -p codex-core windows_sandbox`
    - `cargo test -p codex-core unix_escalation`
    - `cargo test -p codex-app-server windows_sandbox`
    - `cargo test -p codex-tui windows_sandbox`
    - `cargo test -p codex-cli debug_sandbox`
    - `just test -p codex-core unified_exec`
    - `just test -p codex-core
    build_exec_request_preserves_windows_workspace_roots`
    - `env -u CODEX_NETWORK_PROXY_ACTIVE -u
    CODEX_NETWORK_ALLOW_LOCAL_BINDING just test -p codex-app-server --lib
    command_exec`
    - `just test -p codex-windows-sandbox`
    - `just test -p codex-exec sandbox`
    - `just fix -p codex-core -p codex-app-server -p codex-windows-sandbox`
    
    A local macOS cross-check with `cargo check --target
    x86_64-pc-windows-msvc ...` did not reach crate Rust code because native
    dependencies require Windows SDK headers (`windows.h` / `assert.h`) in
    this environment; Windows CI remains the real target validation.
    
    Two local targeted filters compile but do not run assertions on macOS:
    `env -u CODEX_NETWORK_PROXY_ACTIVE -u CODEX_NETWORK_ALLOW_LOCAL_BINDING
    just test -p codex-app-server --lib command_exec_processor` matched zero
    tests, and `just test -p codex-linux-sandbox landlock` matched zero
    tests because the landlock suite is Linux-only.
  • windows-sandbox: feed setup from resolved permissions (#23167)
    ## Why
    
    This is the next step in the Windows sandbox migration away from the
    legacy `SandboxPolicy` abstraction. #22923 moved write-root and token
    decisions onto `ResolvedWindowsSandboxPermissions`, but setup and
    identity still accepted `SandboxPolicy` and converted internally. This
    PR pushes that conversion outward so the setup path consumes the
    resolved Windows permission view directly.
    
    ## What Changed
    
    - Changed `SandboxSetupRequest` to carry
    `ResolvedWindowsSandboxPermissions` instead of `SandboxPolicy` plus
    policy cwd.
    - Updated setup refresh/elevation and identity credential preparation to
    use resolved permissions for read roots, write roots, network identity,
    and deny-write payload planning.
    - Removed the production `allow.rs` legacy wrapper; allow-path
    computation now takes resolved permissions directly.
    - Added a permissions-based world-writable audit entry point while
    keeping the existing legacy wrapper for compatibility.
    - Updated legacy ACL setup and the core Windows setup bridge to
    construct resolved permissions at the boundary.
    - Hardened the Windows sandbox integration test helper staging so Bazel
    retries can reuse an already-staged helper if a prior sandbox helper
    process still has the executable open.
    
    ## Verification
    
    - `cargo test -p codex-windows-sandbox`
    - `cargo test -p codex-core --test all --no-run`
    - `just fix -p codex-windows-sandbox`
    - `just fix -p codex-core`
    - Attempted `cargo check -p codex-windows-sandbox --target
    x86_64-pc-windows-gnullvm`, but the local machine is missing
    `x86_64-w64-mingw32-clang`; Windows CI should cover that target.
    
    
    
    
    
    
    
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/23167).
    * #23715
    * #23714
    * __->__ #23167
  • windows-sandbox: drive write roots from resolved permissions (#22923)
    ## Why
    
    This is the third PR in the Windows sandbox `SandboxPolicy` ->
    `PermissionProfile` migration stack.
    
    #22896 introduced `ResolvedWindowsSandboxPermissions`, and #22918 moved
    elevated runner IPC to carry `PermissionProfile`. This PR starts moving
    the remaining setup/spawn helpers away from asking legacy enum questions
    like “is this `WorkspaceWrite`?” and toward resolved runtime permission
    questions like “does this profile require write capability roots?”
    
    ## What changed
    
    - Added resolved-permissions helpers for network identity and
    write-capability detection.
    - Moved setup write-root gathering to operate on
    `ResolvedWindowsSandboxPermissions`, with the legacy `SandboxPolicy`
    wrapper left in place for existing call sites.
    - Updated identity setup, elevated capture setup, and world-writable
    audit denies to use resolved write roots.
    - Updated spawn preparation to carry resolved permissions in
    `SpawnContext` and use them for network blocking, setup write roots,
    elevated capability SID selection, and legacy capability roots.
    - Removed a now-unused legacy write-root helper.
    
    ## Verification
    
    - `cargo test -p codex-windows-sandbox`
    - `just fix -p codex-windows-sandbox`
    - Existing stack checks are green on #22896 and #22918; CI has started
    for this PR.
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/22923).
    * #23715
    * #23714
    * #23167
    * __->__ #22923
  • Make deny canonical for filesystem permission entries (#23493)
    ## Why
    Filesystem permission profiles used `none` for deny-read entries, which
    is less direct than the action the entry actually represents. This
    change makes `deny` the canonical filesystem permission spelling while
    preserving compatibility for older configs that still send `none`.
    
    ## What changed
    - rename `FileSystemAccessMode::None` to `Deny`
    - serialize and generate schemas with `deny` as the canonical value
    - retain `none` only as a legacy input alias for temporary config
    compatibility
    - update filesystem glob diagnostics and regression coverage to use the
    canonical spelling
    - refresh config and app-server schema fixtures to match the new wire
    shape
    
    ## Validation
    - `cargo test -p codex-protocol`
    - `cargo test -p codex-app-server-protocol`
    - `cargo test -p codex-core config_toml_deserializes_permission_profiles
    --lib`
    - `cargo test -p codex-core
    read_write_glob_patterns_still_reject_non_subpath_globs --lib`
    
    Earlier in the session, a broad `cargo test -p codex-core` run reached
    unrelated pre-existing failures in timing/snapshot/git-info tests under
    this environment; the targeted surfaces touched by this PR passed
    cleanly.
  • feat(sandbox): add Windows deny-read parity (#18202)
    ## Why
    
    The split filesystem policy stack already supports exact and glob
    `access = none` read restrictions on macOS and Linux. Windows still
    needed subprocess handling for those deny-read policies without claiming
    enforcement from a backend that cannot provide it.
    
    ## Key finding
    
    The unelevated restricted-token backend cannot safely enforce deny-read
    overlays. Its `WRITE_RESTRICTED` token model is authoritative for write
    checks, not read denials, so this PR intentionally fails that backend
    closed when deny-read overrides are present instead of claiming
    unsupported enforcement.
    
    ## What changed
    
    This PR adds the Windows deny-read enforcement layer and makes the
    backend split explicit:
    
    - Resolves Windows deny-read filesystem policy entries into concrete ACL
    targets.
    - Preserves exact missing paths so they can be materialized and denied
    before an enforceable sandboxed process starts.
    - Snapshot-expands existing glob matches into ACL targets for Windows
    subprocess enforcement.
    - Honors `glob_scan_max_depth` when expanding Windows deny-read globs.
    - Plans both the configured lexical path and the canonical target for
    existing paths so reparse-point aliases are covered.
    - Threads deny-read overrides through the elevated/logon-user Windows
    sandbox backend and unified exec.
    - Applies elevated deny-read ACLs synchronously before command launch
    rather than delegating them to the background read-grant helper.
    - Reconciles persistent deny-read ACEs per sandbox principal so policy
    changes do not leave stale deny-read ACLs behind.
    - Fails closed on the unelevated restricted-token backend when deny-read
    overrides are present, because its `WRITE_RESTRICTED` token model is not
    authoritative for read denials.
    
    ## Landed prerequisites
    
    These prerequisite PRs are already on `main`:
    
    1. #15979 `feat(permissions): add glob deny-read policy support`
    2. #18096 `feat(sandbox): add glob deny-read platform enforcement`
    3. #17740 `feat(config): support managed deny-read requirements`
    
    This PR targets `main` directly and contains only the Windows deny-read
    enforcement layer.
    
    ## Implementation notes
    
    - Exact deny-read paths remain enforceable on the elevated path even
    when they do not exist yet: Windows materializes the missing path before
    applying the deny ACE, so the sandboxed command cannot create and read
    it during the same run.
    - Existing exact deny paths are preserved lexically until the ACL
    planner, which then adds the canonical target as a second ACL target
    when needed. That keeps both the configured alias and the resolved
    object covered.
    - Windows ACLs do not consume Codex glob syntax directly, so glob
    deny-read entries are expanded to the concrete matches that exist before
    process launch.
    - Glob traversal deduplicates directory visits within each pattern walk
    to avoid cycles, without collapsing distinct lexical roots that happen
    to resolve to the same target.
    - Persistent deny-read ACL state is keyed by sandbox principal SID, so
    cleanup only removes ACEs owned by the same backend principal.
    - Deny-read ACEs are fail-closed on the elevated path: setup aborts if
    mandatory deny-read ACL application fails.
    - Unelevated restricted-token sessions reject deny-read overrides early
    instead of running with a silently unenforceable read policy.
    
    ## Verification
    
    - `cargo test -p codex-core
    windows_restricted_token_rejects_unreadable_split_carveouts`
    - `just fmt`
    - `just fix -p codex-core`
    - `just fix -p codex-windows-sandbox`
    - GitHub Actions rerun is in progress on the pushed head.
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>