Commit Graph

17 Commits

Author SHA1 Message Date
iceweasel-oai
f32c496144 [codex] Handle git pagination flags by position (#21381)
## Why

This is a follow-up to the Windows Git safe-command bypass fix for
BUGB-15601. Git's global `--paginate` / `-p` flags can route output
through a configured pager, so they should not be auto-approved as safe
before the subcommand. At the same time, `-p` after read-only
subcommands like `log`, `diff`, and `show` is the common patch-output
flag, so treating every `-p` as unsafe would make ordinary read-only
inspection commands prompt unnecessarily.

## What Changed

- Split Git option safety matching into explicit global-option and
subcommand-option lists.
- Treat global `git --paginate ...` and `git -p ...` as unsafe.
- Keep post-subcommand patch usage such as `git log -p`, `git diff -p`,
and `git show -p HEAD` safe.
- Keep the pagination coverage with the shared Git safe-command
implementation rather than the Windows wrapper tests.
- Remove the stale `git_global_option_requires_prompt` helper now that
safe-command Git option matching owns the prompt-required lists.

## Testing

- `cargo test -p codex-shell-command`
2026-05-06 11:53:26 -07:00
iceweasel-oai
db22c91e61 Share Git safe-command logic on Windows (#21275)
## Why

BUGB-15601 showed that the Windows safe-command path had drifted from
the generic Git classifier. The Windows-specific Git parser could
classify a PowerShell-wrapped `git` command as safe as soon as it found
a safelisted subcommand, without applying the generic checks for unsafe
subcommand options such as `--output`, `--ext-diff`, `--textconv`,
`--paginate`, or `cat-file --filters`.

The generic classifier already models the Git command boundary and the
read-only argument checks more carefully, so Windows should reuse that
logic instead of maintaining a smaller parallel parser.

## What Changed

- Extracted the existing generic Git classification logic into
`is_safe_git_command`.
- Updated `windows_safe_commands.rs` to call that shared helper for
parsed PowerShell `git` commands.
- Removed the Windows-only Git subcommand safelist, including the
`cat-file` allowance that was part of the reported bypass.
- Added a Windows regression test that keeps PowerShell-wrapped Git
commands with side-effecting options classified unsafe.
- Made the full-path PowerShell test discover the installed PowerShell
executable instead of depending on one hard-coded `pwsh.exe` path.

## Verification

- `cargo test -p codex-shell-command
rejects_git_subcommand_options_with_side_effects`
- `cargo test -p codex-shell-command
git_global_override_flags_are_not_safe`
- `cargo test -p codex-shell-command
windows_powershell_full_path_is_safe -- --nocapture`

Co-authored-by: Codex <codex@openai.com>
2026-05-05 17:49:42 -07:00
Dylan Hurd
af089fb21d fix(exec_policy) heredoc parsing file_redirect (#20113)
## Summary
Fixes a regression introduced in #10941 so that heredocs do not permit
file redirects to be approved by rules, and adds scenario tests to cover
this behavior.


Previously, heredoc command parsing would allow redirects and
environment variables:
```bash
# commands_for_exec_policy() would parse this via parse_shell_lc_single_command_prefix
PATH=/tmp/bad:$PATH cat <<'EOF' > /tmp/bad/hello.txt
hello
EOF
```
This conflicts with the Codex Rules documentation; heredoc parsing logic
should abide by the same strictness of parsing.


## Tests
- [x] Updated unit tests accordingly
- [x] Added scenario tests for these cases

---------

Co-authored-by: Codex <noreply@openai.com>
2026-05-01 01:05:02 +00:00
iceweasel-oai
4f96001fa7 execpolicy: unwrap PowerShell -Command wrappers on Windows (#20336)
## Why
On Windows, Codex runs shell commands through a top-level
`powershell.exe -NoProfile -Command ...` wrapper. `execpolicy` was
matching that wrapper instead of the inner command, so prefix rules like
`["git", "push"]` did not fire for PowerShell-wrapped commands even
though the same normalization already happens for `bash -lc` on Unix.

This change makes the Windows shell wrapper transparent to rule matching
while preserving the existing Windows unmatched-command safelist and
dangerous-command heuristics.

## What changed
- add `parse_powershell_command_plain_commands()` in
`shell-command/src/powershell.rs` to unwrap the top-level PowerShell
`-Command` body with `extract_powershell_command()` and parse it with
the existing PowerShell AST parser
- update `core/src/exec_policy.rs` so `commands_for_exec_policy()`
treats top-level PowerShell wrappers like `bash -lc` and evaluates rules
against the parsed inner commands
- carry a small `ExecPolicyCommandOrigin` through unmatched-command
evaluation and expose `is_safe_powershell_words()` /
`is_dangerous_powershell_words()` so Windows safelist and
dangerous-command checks still work after unwrap
- add Windows-focused tests for wrapped PowerShell prompt/allow matches,
wrapper parsing, and unmatched safe/dangerous inner commands, and
re-enable the end-to-end `execpolicy_blocks_shell_invocation` test on
Windows

## Testing
- `cargo test -p codex-shell-command`
2026-05-01 00:56:20 +00:00
Owen Lin
2e598df6fc fix: don't auto approve git -C ... (#20085)
It's safer to make sure these commands go through approval flows.
2026-04-28 22:06:55 +00:00
pakrym-oai
0c8f3173e4 [codex] Remove unused Rust helpers (#17146)
## Summary

Removes high-confidence unused Rust helper functions and exports across
`codex-tui`, `codex-shell-command`, and utility crates.

The cleanup includes dead TUI helper methods, unused
path/string/elapsed/fuzzy-match utilities, an unused Windows PowerShell
lookup helper, and the unused terminal palette version counter. This
keeps the remaining public surface smaller without changing behavior.

## Validation

- `just fmt`
- `cargo test -p codex-tui -p codex-shell-command -p codex-utils-elapsed
-p codex-utils-fuzzy-match -p codex-utils-string -p codex-utils-path`
- `just fix -p codex-tui -p codex-shell-command -p codex-utils-elapsed
-p codex-utils-fuzzy-match -p codex-utils-string -p codex-utils-path`
- `git diff --check`
2026-04-13 18:27:00 -07:00
pakrym-oai
f1a2b920f9 [codex] Make AbsolutePathBuf joins infallible (#16981)
Having to check for errors every time join is called is painful and
unnecessary.
2026-04-07 10:52:08 -07:00
pakrym-oai
413c1e1fdf [codex] reduce module visibility (#16978)
## Summary
- reduce public module visibility across Rust crates, preferring private
or crate-private modules with explicit crate-root public exports
- update external call sites and tests to use the intended public crate
APIs instead of reaching through module trees
- add the module visibility guideline to AGENTS.md

## Validation
- `cargo check --workspace --all-targets --message-format=short` passed
before the final fix/format pass
- `just fix` completed successfully
- `just fmt` completed successfully
- `git diff --check` passed
2026-04-07 08:03:35 -07:00
Michael Bolin
142681ef93 shell-command: reuse a PowerShell parser process on Windows (#16057)
## Why

`//codex-rs/shell-command:shell-command-unit-tests` became a real
bottleneck in the Windows Bazel lane because repeated calls to
`is_safe_command_windows()` were starting a fresh PowerShell parser
process for every `powershell.exe -Command ...` assertion.

PR #16056 was motivated by that same bottleneck, but its test-only
shortcut was the wrong layer to optimize because it weakened the
end-to-end guarantee that our runtime path really asks PowerShell to
parse the command the way we expect.

This PR attacks the actual cost center instead: it keeps the real
PowerShell parser in the loop, but turns that parser into a long-lived
helper process so both tests and the runtime safe-command path can reuse
it across many requests.

## What Changed

- add `shell-command/src/command_safety/powershell_parser.rs`, which
keeps one mutex-protected parser process per PowerShell executable path
and speaks a simple JSON-over-stdio request/response protocol
- turn `shell-command/src/command_safety/powershell_parser.ps1` into a
long-running parser server with comments explaining the protocol, the
AST-shape restrictions, and why unsupported constructs are rejected
conservatively
- keep request ids and a one-time respawn path so a dead or
desynchronized cached child fails closed instead of silently returning
mixed parser output
- preserve separate parser processes for `powershell.exe` and
`pwsh.exe`, since they do not accept the same language surface
- avoid a direct `PipelineChainAst` type reference in the PowerShell
script so the parser service still runs under Windows PowerShell 5.1 as
well as newer `pwsh`
- make `shell-command/src/command_safety/windows_safe_commands.rs`
delegate to the new parser utility instead of spawning a fresh
PowerShell process for every parse
- add a Windows-only unit test that exercises multiple sequential
requests against the same parser process

## Testing

- adds a Windows-only parser-reuse unit test in `powershell_parser.rs`
- the main end-to-end verification for this change is the Windows CI
lane, because the new service depends on real `powershell.exe` /
`pwsh.exe` behavior
2026-03-27 19:33:41 -07:00
Adrian
af04273778 [codex] Block unsafe git global options from safe allowlist (#15796)
## Summary
- block git global options that can redirect config, repository, or
helper lookup from being auto-approved as safe
- share the unsafe global-option predicate across the Unix and Windows
git safety checks
- add regression coverage for inline and split forms, including `bash
-lc` and PowerShell wrappers

## Root cause
The Unix safe-command gate only rejected `-c` and `--config-env`, even
though the shared git parser already knew how to skip additional
pre-subcommand globals such as `--git-dir`, `--work-tree`,
`--exec-path`, `--namespace`, and `--super-prefix`. That let those
arguments slip through safe-command classification on otherwise
read-only git invocations and bypass approval. The Windows-specific
safe-command path had the same trust-boundary gap for git global
options.
2026-03-26 10:46:04 -07:00
Jeremy Rose
c2d008aca5 Collapse parsed command summaries when any stage is unknown (#13043)
## Summary
- collapse parsed command output to a single `Unknown` whenever the
normal parse includes any unknown entry
- preserve the existing parsing flow and existing `cd` handling,
including the current `cd && ...` collapse behavior
- trim redundant tests and add focused coverage for collapse-on-unknown
cases

## Testing
- `cargo test -p codex-shell-command`
2026-03-03 19:45:34 +00:00
Michael Bolin
6a673e7339 core: resolve host_executable() rules during preflight (#13065)
## Why

[#12964](https://github.com/openai/codex/pull/12964) added
`host_executable()` support to `codex-execpolicy`, and
[#13046](https://github.com/openai/codex/pull/13046) adopted it in the
zsh-fork interception path.

The remaining gap was the preflight execpolicy check in
`core/src/exec_policy.rs`. That path derives approval requirements
before execution for `shell`, `shell_command`, and `unified_exec`, but
it was still using the default exact-token matcher.

As a result, a command that already included an absolute executable
path, such as `/usr/bin/git status`, could still miss a basename rule
like `prefix_rule(pattern = ["git"], ...)` during preflight even when
the policy also defined a matching `host_executable(name = "git", ...)`
entry.

This PR brings the same opt-in `host_executable()` resolution to the
preflight approval path when an absolute program path is already present
in the parsed command.

## What Changed

- updated
`ExecPolicyManager::create_exec_approval_requirement_for_command()` in
`core/src/exec_policy.rs` to use `check_multiple_with_options(...)` with
`MatchOptions { resolve_host_executables: true }`
- kept the existing shell parsing flow for approval derivation, but now
allow basename rules to match absolute executable paths during preflight
when `host_executable()` permits it
- updated requested-prefix amendment evaluation to use the same
host-executable-aware matching mode, so suggested `prefix_rule()`
amendments are checked consistently for absolute-path commands
- added preflight coverage for:
- absolute-path commands that should match basename rules through
`host_executable()`
- absolute-path commands whose paths are not in the allowed
`host_executable()` mapping
  - requested prefix-rule amendments for absolute-path commands

## Verification

- `just fix -p codex-core`
- `cargo test -p codex-core --lib exec_policy::tests::`
2026-02-28 17:25:30 +00:00
Dylan Hurd
85b00ae8de fix(core) exec policy parsing 3 (#12485)
## Summary
Quick fix
2026-02-22 06:26:13 +00:00
Dylan Hurd
0fbe10a807 fix(core) exec_policy parsing fixes (#11951)
## Summary
Fixes a few things in our exec_policy handling of prefix_rules:
1. Correctly match redirects specifically for exec_policy parsing. i.e.
if you have `prefix_rule(["echo"], decision="allow")` then `echo hello >
output.txt` should match - this should fix #10321
2. If there already exists any rule that would match our prefix rule
(not just a prompt), then drop it, since it won't do anything.


## Testing
- [x] Updated unit tests, added approvals ScenarioSpecs
2026-02-16 23:11:59 -08:00
Josh McKinney
fc073c9c5b Remove git commands from dangerous command checks (#11510)
### Motivation

- Git subcommand matching was being classified as "dangerous" and caused
benign developer workflows (for example `git push --force-with-lease`)
to be blocked by the preflight policy.
- The change aligns behavior with the intent to reserve the dangerous
checklist for truly destructive shell ops (e.g. `rm -rf`) and avoid
surprising developer-facing blocks.

### Description

- Remove git-specific subcommand checks from
`is_dangerous_to_call_with_exec` in
`codex-rs/shell-command/src/command_safety/is_dangerous_command.rs`,
leaving only explicit `rm` and `sudo` passthrough checks.
- Deleted the git-specific helper logic that classified `reset`,
`branch`-delete, `push` (force/delete/refspec) and `clean --force` as
dangerous.
- Updated unit tests in the same file to assert that various `git
reset`/`git branch`/`git push`/`git clean` variants are no longer
classified as dangerous.
- Kept `find_git_subcommand` (used by safe-command classification)
intact so safe/unsafe parsing elsewhere remains functional.

### Testing

- Ran formatter with `just fmt` successfully.  
- Ran unit tests with `cargo test -p codex-shell-command` and all tests
passed (`144 passed; 0 failed`).

------
[Codex
Task](https://chatgpt.com/codex/tasks/task_i_698d19dedb4883299c3ceb5bbc6a0dcf)
2026-02-13 01:33:02 +00:00
Dylan Hurd
cc8c293378 fix(exec-policy) No empty command lists (#11397)
## Summary
This should rarely, if ever, happen in practice. But regardless, we
should never provide an empty list of `commands` to ExecPolicy. This PR
is almost entirely adding test around these cases.

## Testing
- [x] Adds a bunch of unit tests for this
2026-02-10 19:22:23 -08:00
Michael Bolin
d44f4205fb chore: rename codex-command to codex-shell-command (#11378)
This addresses some post-merge feedback on
https://github.com/openai/codex/pull/11361:

- crate rename
- reuse `detect_shell_type()` utility
2026-02-10 17:03:46 -08:00