Created this PR by:
- adding `redundant_clone` to `[workspace.lints.clippy]` in
`cargo-rs/Cargol.toml`
- running `cargo clippy --tests --fix`
- running `just fmt`
Though I had to clean up one instance of the following that resulted:
```rust
let codex = codex;
```
This PR:
* Added the clippy.toml to configure allowable expect / unwrap usage in
tests
* Removed as many expect/allow lines as possible from tests
* moved a bunch of allows to expects where possible
Note: in integration tests, non `#[test]` helper functions are not
covered by this so we had to leave a few lingering `expect(expect_used`
checks around
Previous to this PR, `codex-rs/core/tests/sandbox.rs` contained
integration tests that were specific to Seatbelt. This PR moves those
tests to `codex-rs/core/src/seatbelt.rs` and designates
`codex-rs/core/tests/sandbox.rs` to be used as the home for
cross-platform (well, Mac and Linux...) sandbox tests.
To start, this migrates
`python_multiprocessing_lock_works_under_seatbelt()` from #1823 to the
new `sandbox.rs` because this is the type of thing that should work on
both Mac _and_ Linux, though I still need to do some work to clean up
the test so it works on both platforms.
## Summary
- add a unit test to ensure the macOS seatbelt policy allows POSIX
semaphores
- add a macOS-only test that runs a Python multiprocessing Lock under
Seatbelt
## Testing
- `cargo test -p codex_core seatbelt_base_policy_allows_ipc_posix_sem
--no-fail-fast` (failed: failed to download from
`https://static.crates.io/crates/tokio-stream/0.1.17/download`)
- `cargo test -p codex_core seatbelt_base_policy_allows_ipc_posix_sem
--no-fail-fast --offline` (failed: attempting to make an HTTP request,
but --offline was specified)
- `cargo test --all-features --no-fail-fast --offline` (failed:
attempting to make an HTTP request, but --offline was specified)
- `just fmt` (failed: command not found: just)
- `just fix` (failed: command not found: just)
Ran tests locally to confirm it passes on master and failed before my
previous change
------
https://chatgpt.com/codex/tasks/task_i_6890f221e0a4833381cfb53e11499bcc
Replaces the `include_default_writable_roots` option on
`sandbox_workspace_write` (that defaulted to `true`, which was slightly
weird/annoying) with `exclude_tmpdir_env_var`, which defaults to
`false`.
Though perhaps more importantly `/tmp` is now enabled by default as part
of `sandbox_mode = "workspace-write"`, though `exclude_slash_tmp =
false` can be used to disable this.
To make `--full-auto` safer, this PR updates the Seatbelt policy so that
a `SandboxPolicy` with a `writable_root` that contains a `.git/`
_directory_ will make `.git/` _read-only_ (though as a follow-up, we
should also consider the case where `.git` is a _file_ with a `gitdir:
/path/to/actual/repo/.git` entry that should also be protected).
The two major changes in this PR:
- Updating `SandboxPolicy::get_writable_roots_with_cwd()` to return a
`Vec<WritableRoot>` instead of a `Vec<PathBuf>` where a `WritableRoot`
can specify a list of read-only subpaths.
- Updating `create_seatbelt_command_args()` to honor the read-only
subpaths in `WritableRoot`.
The logic to update the policy is a fairly straightforward update to
`create_seatbelt_command_args()`, but perhaps the more interesting part
of this PR is the introduction of an integration test in
`tests/sandbox.rs`. Leveraging the new API in #1785, we test
`SandboxPolicy` under various conditions, including ones where `$TMPDIR`
is not readable, which is critical for verifying the new behavior.
To ensure that Codex can run its own tests, e.g.:
```
just codex debug seatbelt --full-auto -- cargo test if_git_repo_is_writable_root_then_dot_git_folder_is_read_only
```
I had to introduce the use of `CODEX_SANDBOX=sandbox`, which is
comparable to how `CODEX_SANDBOX_NETWORK_DISABLED=1` was already being
used.
Adding a comparable change for Landlock will be done in a subsequent PR.
At 550 lines, `exec.rs` was a bit large. In particular, I found it hard
to locate the Seatbelt-related code quickly without a file with
`seatbelt` in the name, so this refactors things so:
- `spawn_command_under_seatbelt()` and dependent code moves to a new
`seatbelt.rs` file
- `spawn_child_async()` and dependent code moves to a new `spawn.rs`
file
