Commit Graph

19 Commits

  • [codex] treat PowerShell stop-parsing forms as unsupported (#22643)
    ## Summary
    - Treat PowerShell stop-parsing token forms as unsupported in the
    AST-backed command flattener.
    - Add focused regressions at the parser layer and Windows command-safety
    layer.
    
    ## Why
    The command-safety parser lowers PowerShell AST elements into argv-like
    words. Stop-parsing syntax preserves a native-command argument shape
    that this lowering does not model, so these forms should stay on the
    conservative unsupported path.
    
    ## Validation
    - `cargo fmt --manifest-path codex-rs/Cargo.toml --all --check`
    - `cargo test --manifest-path codex-rs/Cargo.toml -p
    codex-shell-command`
  • Disable empty Cargo test targets (#21584)
    ## Summary
    
    `cargo test` has entails both running standard Rust tests and doctests.
    It turns out that the doctest discovery is fairly slow, and it's a cost
    you pay even for crates that don't include any doctests.
    
    This PR disables doctests with `doctest = false` for crates that lack
    any doctests.
    
    For the collection of crates below, this speeds up test execution by
    >4x.
    
    E.g., before this PR:
    
    ```
    Benchmark 1: cargo test     -p codex-utils-absolute-path     -p codex-utils-cache     -p codex-utils-cli     -p codex-utils-home-dir     -p codex-utils-output-truncation     -p codex-utils-path     -p codex-utils-string     -p codex-utils-template     -p codex-utils-elapsed     -p codex-utils-json-to-toml
      Time (mean ± σ):      1.849 s ±  4.455 s    [User: 0.752 s, System: 1.367 s]
      Range (min … max):    0.418 s … 14.529 s    10 runs
    ```
    
    And after:
    
    ```
    Benchmark 1: cargo test     -p codex-utils-absolute-path     -p codex-utils-cache     -p codex-utils-cli     -p codex-utils-home-dir     -p codex-utils-output-truncation     -p codex-utils-path     -p codex-utils-string     -p codex-utils-template     -p codex-utils-elapsed     -p codex-utils-json-to-toml
      Time (mean ± σ):     428.6 ms ±   6.9 ms    [User: 187.7 ms, System: 219.7 ms]
      Range (min … max):   418.0 ms … 436.8 ms    10 runs
    ```
    
    For a single crate, with >2x speedup, before:
    
    ```
    Benchmark 1: cargo test -p codex-utils-string
      Time (mean ± σ):     491.1 ms ±   9.0 ms    [User: 229.8 ms, System: 234.9 ms]
      Range (min … max):   480.9 ms … 512.0 ms    10 runs
    ```
    
    And after:
    
    ```
    Benchmark 1: cargo test -p codex-utils-string
      Time (mean ± σ):     213.9 ms ±   4.3 ms    [User: 112.8 ms, System: 84.0 ms]
      Range (min … max):   206.8 ms … 221.0 ms    13 runs
    ```
    
    Co-authored-by: Codex <noreply@openai.com>
  • [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`
  • 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>
  • 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>
  • 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`
  • fix: don't auto approve git -C ... (#20085)
    It's safer to make sure these commands go through approval flows.
  • [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`
  • [codex] Make AbsolutePathBuf joins infallible (#16981)
    Having to check for errors every time join is called is painful and
    unnecessary.
  • [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
  • 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
  • [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.
  • 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`
  • 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::`
  • 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
  • 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)
  • 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
  • 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