fix: address Codex security audit findings (v2)#64
Merged
Conversation
…s in findings - Add caret escape handling in QState::Double for ShellType::Cmd, matching cmd.exe behavior where ^ is processed even inside double quotes. - Redact env assignment values in pipe-to-interpreter, dotfile overwrite, and archive extract evidence strings to avoid leaking secrets in output. - Change redact_env_value to always return [REDACTED] instead of a prefix, preventing partial secret leakage in sensitive env export findings. - Skip pipe-to-interpreter when source is plain `tirith` (trusted) but still flag when source is `tirith run` (user-fetched content). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…view Security fixes for shell hooks (bash, zsh, fish, PowerShell): - Remove inline TIRITH=0 prefix check on pasted content that allowed attacker-controlled paste to skip scanning. The env var check (TIRITH=0 set in environment) remains as an intentional user opt-out. - Escape blocked command/paste previews using shell-appropriate quoting (printf %q for bash, string escape for fish, ConvertTo-Json for PS) to prevent terminal escape injection when echoing blocked content. - Add _TIRITH_BASH_INTERNAL guard to prevent recursive tirith invocations when the hook calls tirith check/paste. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ndly Previously, a tools/call message without a JSON-RPC id was classified as GuardedNotification and forwarded to the upstream tool without running security analysis. This allowed an attacker-controlled MCP client to bypass tirith's command inspection by omitting the id field. Now notifications matching guarded tools are analyzed through the full engine pipeline. Blocked notifications are silently dropped (no response possible per JSON-RPC spec). Allow/warn notifications are forwarded with audit logging. Also adds InvalidRequest handling for guarded requests with non-standard id types and NotificationExtractionFailed for notifications where the command cannot be extracted. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add env_long_flag_takes_value() whitelist so only --unset, --chdir, --split-string consume the next token. Flags like --ignore-environment no longer incorrectly skip the command argument. - Stop split_raw_words at '#' for PowerShell to prevent comment text from being parsed as words (which could false-trigger $env:TIRITH=0). - Reject inline bypass when input has multiple segments (pipes, &&, ;) or unquoted &, preventing TIRITH=0 from suppressing analysis of chained malicious commands. - Remove self-invocation guard entirely. User-typed tirith commands now go through the full analysis pipeline like any other command, closing the PATH-spoofing and command-substitution bypass vectors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add validate_fetch_url() that validates URLs before cloaking requests, blocking private IPs, loopback, link-local, metadata endpoints, and embedded credentials. - Add DNS resolution check: hostnames are resolved and each IP is validated against the private-network blocklist, preventing DNS rebinding and hostname-based SSRF bypasses. - Replace cloaking's redirect policy with a custom policy that re-validates each redirect target, preventing SSRF via open redirects. - Expand url_validate with comprehensive test coverage for both server and fetch URL validation modes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Extend extract_urls() to scan leading env-assignment values (e.g., URL=https://evil.com curl ...) in addition to command + args tokens. Adds ignores_env_assignment_url() to skip known false-positive vars like HTTPS_PROXY, HTTP_PROXY, etc. - Add leading_env_assignments() and leading_env_assignment_values() to tokenize.rs for structured env-prefix parsing. - Set MCP_SCAN_MAX_FILES=5000 default cap for MCP scan operations to prevent unbounded directory enumeration DoS. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add redact_shell_assignments() and redacted_findings() to redact.rs, scrubbing VAR=value patterns and $env:VAR assignments from evidence strings before they reach JSON output or last_trigger.json. - Pass DLP custom patterns through write_json() and last_trigger to ensure user-defined secrets are redacted in all output paths. - Use posix_single_quote() and powershell_single_quote() in init.rs to properly escape hook source paths, preventing shell metacharacter injection via TIRITH_SHELL_DIR or unusual install paths. - Quote paths in shell_profile.rs setup for consistency. - Add cli_integration tests for redaction and quoting behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Set LOCALAPPDATA in addition to XDG_DATA_HOME in the last_trigger redaction test so it finds the data dir on Windows. - Update rustls-webpki 0.103.9 → 0.103.10 to fix RUSTSEC-2026-0049 (CRL distribution point matching advisory).
…0.101.x - Windows test: set APPDATA (not LOCALAPPDATA) since etcetera's Windows strategy uses APPDATA for data_dir(). - Ignore RUSTSEC-2026-0049 for rustls-webpki 0.101.7 which is pinned by rust-s3 → rustls 0.21. The 0.103.x copy is already updated to 0.103.10. Tirith does not use CRL revocation checking.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses findings from the Codex automated security audit. Of 47 reported findings (many duplicates), 7 distinct real bugs were identified and fixed:
Fixes
^inside double quotes forShellType::Cmdso"c^md"normalizes tocmdfor interpreter detection--unset,--chdir,--split-string) instead of blindly skipping next token for all--flagssplit_raw_wordsat#for PowerShell so comment text can't trigger$env:TIRITH=0bypassvalidate_fetch_url()with DNS resolution checks; re-validate each redirect targetAdditional hardening
tirithcommands now go through full analysis)&&,;,&)tirith inithook pathsHostResolvertype alias for clippy complianceNot fixed (by design)
--stops scan — correct CLI semanticsprintf '%s'(safe)Test plan
cargo fmt --all -- --checkpassescargo clippy --workspace -- -D warningspassescargo test --workspacepasses (all tests)