Skip to content

Conversation

@cpjet64
Copy link

@cpjet64 cpjet64 commented Oct 24, 2025

Persist approval keys across Windows shells and volatile args

Problem & intent

“Allow every time / for this session” approvals didn’t persist across Windows shells because approval keys included wrappers (pwsh/cmd/wsl/bash -lc) or volatile args (e.g., sed ranges).
Users were re‑prompted for equivalent actions that should have been considered identical approvals.


Before / After

Before
Approval keys were based on full argv; wrapper or volatile‑arg differences caused repeated prompts for logically identical commands.

After

  • Approvals now persist across PowerShell 5/7, Git Bash/MSYS2/MINGW, and WSL.
  • Keys are normalized by stripping wrappers and, for sed, de‑noising numeric ranges.
  • Canonicalization preserves arguments while ignoring shell wrappers or volatile digits.

Scope of change

codex-rs/core/src/tools/runtimes/shell.rs

  • strip_wrapper() handles wsl/bash/cmd/pwsh/powershell launchers.
  • canonicalize() preserves full argv; only sed script tokens normalized (digits → N). rg untouched.
  • Multi‑key lookup order: RAW → STRIPPED → CANONICAL.
  • Added negative test verifying sed -n 10,20p safe.txt ≠ sed -n 10,20p secrets.txt.

codex-rs/core/src/tools/sandboxing.rs

  • ApprovalStore::get_any() and put_all() implement ordered multi‑key lookup/store.

Security effects

None. No new capabilities; deduplicates approvals already granted.
Approval caching remains scoped and sandbox‑checked.


Testing evidence

Static

  • cargo fmt (workspace): clean
  • cargo clippy -p codex-core -- -D warningsclean

Unit tests (cargo test -p codex-core)

  • approvals_match_across_windows_wrappers_and_sed_ranges
  • approvals_match_rg_across_pwsh_bash_and_preserve_pattern
  • shell_requires_initial_approval_for_dangerous_command
  • approvals_cache_allows_dangerous_command_across_wrappers
  • sed_canonicalization_does_not_cross_files

All targeted tests pass. Approval persistence verified across environments.


Notes

  • Runtime delta ≈ 36 LOC (excluding tests).
  • No CRLF, TUI, PATH/PATHEXT, or env‑related changes.
  • Fully validated against clippy and tests; safe command gating unchanged.

Checklist

… argv; sed-only canonicalization + neg test

Implements cross-wrapper approval key normalization for PowerShell 5/7 (pwsh),
cmd, Git Bash/MSYS2/MINGW, and WSL while keeping the sandbox policy intact.

- Added `strip_wrapper()` to peel launchers:
  - wsl[.exe] [--|-e|--exec] (optional) bash -lc <real…> → <real…>
  - bash -lc <real…> → <real…>
  - cmd[.exe] /c <real…> → <real…>
  - powershell[.exe]/pwsh[.exe] [-NoProfile]* (-Command|-c) <real…> → <real…>
- Added `canonicalize()`:
  - Preserves the full argv after wrapper stripping.
  - For `sed`, only normalizes the numeric range token (e.g., 10,20p → N,Np).
  - For `rg`, returns full argv unchanged (wrapper stripping alone unifies).
- Matching order: RAW → STRIPPED → CANONICAL.
- Added `ApprovalStore::get_any()` and `put_all()` for multi-key lookups/writes.
- Added negative test `sed_canonicalization_does_not_cross_files` to verify
  `sed -n 10,20p safe.txt` does not match `sed -n 10,20p secrets.txt`.

Security: No policy or sandbox expansion. Only de-duplicates approvals the user
already granted.

Tests: clippy clean, targeted tests passing:
- approvals_match_across_windows_wrappers_and_sed_ranges
- approvals_match_rg_across_pwsh_bash_and_preserve_pattern
- shell_requires_initial_approval_for_dangerous_command
- approvals_cache_allows_dangerous_command_across_wrappers
- sed_canonicalization_does_not_cross_files

Refs: openai#4212 openai#4603 openai#3691; addresses openai#5199 (MINGW persistence)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant