Add Windows AppContainer sandbox helper #4905
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| configure_writable_roots(sandbox_policy, sandbox_policy_cwd, sid.sid())?; | ||
| configure_writable_roots_for_command_cwd(&command_cwd, sid.sid())?; |
There was a problem hiding this comment.
The helper unconditionally invokes configure_writable_roots_for_command_cwd(&command_cwd, …) immediately after applying the policy. That function calls grant_path_with_flags(..., true) and adds a FILE_GENERIC_WRITE ACE for the AppContainer SID on the working directory regardless of whether the policy is ReadOnly. As a result, a policy that is supposed to provide only read access still allows the sandboxed process to modify files in its CWD, which violates the semantics of SandboxPolicy::ReadOnly and weakens the sandbox (a read‑only tool call can still write into the workspace).
Useful? React with 👍 / 👎.
| let permissions = if write { | ||
| (FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE).0 | ||
| } else { | ||
| (FILE_GENERIC_READ | FILE_GENERIC_EXECUTE).0 | ||
| }; | ||
| let explicit = EXPLICIT_ACCESS_W { | ||
| grfAccessPermissions: permissions, | ||
| grfAccessMode: SET_ACCESS, | ||
| grfInheritance: SUB_CONTAINERS_AND_OBJECTS_INHERIT | OBJECT_INHERIT_ACE, |
There was a problem hiding this comment.
When a writable root contains subpaths marked as read‑only, the code first grants the root FILE_GENERIC_READ|WRITE|EXECUTE with inheritance and then calls grant_path_with_flags for each subpath with write=false. However grant_path_with_flags always uses SET_ACCESS to add another allow ACE (lines 442‑450) and never removes or denies the previously inherited write ACE. Consequently the AppContainer SID still inherits write permissions from the root and directories meant to stay read‑only (e.g. .git) remain writable. Enforcing read‑only subpaths requires adding a deny ACE or preventing inheritance from the writable root.
Useful? React with 👍 / 👎.
Summary
Testing
https://chatgpt.com/codex/tasks/task_i_68e594cbdb9c8327828648e82576299a