Fix detection edge cases, shell hooks, and audit reliability#38
Merged
Fix detection edge cases, shell hooks, and audit reliability#38
Conversation
Report errors via eprintln instead of silently swallowing them in audit log open/write/sync/lock/unlock and CLI output write paths.
push_segment() incorrectly treated VAR=VALUE as the command token. Now skips leading environment variable assignments to find the real command. Adds pub is_env_assignment() helper for use by engine bypass detection. Fixes: TIRITH=0 curl evil.com now correctly identifies curl as command. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add command-aware output-flag skipping for curl (-o/--output) and wget (-O/-OFILE/--output-document). Extract URLs from command+args instead of raw segment text to avoid matching URLs in env-prefix values. Add conservative non-TLD file extensions (.png, .jpg, .mp4, etc.) to schemeless host exclusion list. Fixes issue #33. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…detection - Treat single & as segment boundary in split_raw_words (was only handling &&) - Add Windows backslash path check in is_tirith_command - Use exact equality for TIRITH=0 bypass check (was starts_with, matching TIRITH=00 etc) - Collapse else-if block in check.rs (clippy collapsible_else_if) - cargo fmt: fix indentation in extract.rs file_exts array
| // Case 1: Leading VAR=VALUE assignments before the command | ||
| let mut idx = 0; | ||
| while idx < words.len() && tokenize::is_env_assignment(&words[idx]) { | ||
| if words[idx] == "TIRITH=0" { |
There was a problem hiding this comment.
🟢 Low src/engine.rs:46
The comparison words[idx] == "TIRITH=0" won't match quoted variants like 'TIRITH=0' since split_raw_words preserves quotes. Consider stripping quotes before the comparison, similar to how is_env_assignment does.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file crates/tirith-core/src/engine.rs around line 46:
The comparison `words[idx] == "TIRITH=0"` won't match quoted variants like `'TIRITH=0'` since `split_raw_words` preserves quotes. Consider stripping quotes before the comparison, similar to how `is_env_assignment` does.
Evidence trail:
crates/tirith-core/src/engine.rs lines 46, 65, 85: direct comparison `words[idx] == "TIRITH=0"`
crates/tirith-core/src/engine.rs lines 122-133 (single quotes), 134-151 (double quotes): split_raw_words preserves quotes by pushing them into current
crates/tirith-core/src/tokenize.rs lines 320-328: is_env_assignment strips quotes before checking (lines 322-328)
Without this, tirith.exe on Windows would not be recognized as a self-invocation, causing false positives on every tirith command.
612325a to
819fdeb
Compare
- Merge origin/main (already up to date) - Skip flags in resolve_command_wrapper - Remove dead code in is_tirith_command - Remove quote-stripping in is_env_assignment (callers handle this) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The _shell parameter was unused — PowerShell users couldn't use inline bypass ($env:TIRITH="0"; cmd) because only POSIX VAR=value syntax was detected. Now handles $env:TIRITH=0, $env:TIRITH="0", $Env:TIRITH='0', and spaced form ($env:TIRITH = "0") when shell is PowerShell. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
crates/tirith-core/src/engine.rs
Outdated
Comment on lines
+144
to
+147
| if after_dollar.len() < 4 || !after_dollar[..4].eq_ignore_ascii_case("env:") { | ||
| return false; | ||
| } | ||
| &after_dollar[4..] == var_name |
There was a problem hiding this comment.
🟠 High src/engine.rs:144
PowerShell $env: parsing: avoid byte slicing and case‑sensitive matches. Suggest checking the $env: prefix and TIRITH name using ASCII‑insensitive prefix checks (e.g., starts_with/strip_prefix) or by iterating chars, and parse the = value without indexing. This prevents UTF‑8 boundary panics and aligns with PowerShell’s case‑insensitivity.
- if after_dollar.len() < 4 || !after_dollar[..4].eq_ignore_ascii_case("env:") {
+ let prefix = "env:";
+ if !after_dollar.get(..prefix.len()).map_or(false, |s| s.eq_ignore_ascii_case(prefix)) {
return false;
}
- &after_dollar[4..] == var_name
+ after_dollar.get(prefix.len()..).map_or(false, |s| s == var_name)🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file crates/tirith-core/src/engine.rs around lines 144-147:
PowerShell `$env:` parsing: avoid byte slicing and case‑sensitive matches. Suggest checking the `$env:` prefix and `TIRITH` name using ASCII‑insensitive prefix checks (e.g., `starts_with`/`strip_prefix`) or by iterating chars, and parse the `=` value without indexing. This prevents UTF‑8 boundary panics and aligns with PowerShell’s case‑insensitivity.
Evidence trail:
crates/tirith-core/src/engine.rs lines 139-147 (is_powershell_env_ref function with byte slicing at lines 143-144, 147), lines 122-135 (is_powershell_tirith_bypass with similar byte slicing at lines 126-127, 130, 134 and case-sensitive TIRITH= check at line 131). Rust documentation confirms that &str slicing panics on non-UTF-8-boundary indices.
…nics Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
command -v and command -V are lookup operations (print path/version), not execution. Return None from resolve_command_wrapper for these flags to avoid false positive self-invocation detection. Also fix clippy::unnecessary_map_or lints in is_powershell_env_ref. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
Changes
VAR=valassignments when resolving command nameconf.ddirectory and all profile candidates--interactiveflag across zsh, bash, fish, PowerShellTest plan
cargo fmt --allpassescargo clippy --workspace -- -D warningspassescargo test --workspacepasses (all 175+ tests)tirith checkon schemeless URLs with pathstirith doctordetects fish conf.d hooks🤖 Generated with Claude Code
Note
Detect inline TIRITH=0 bypasses and self-invocations in
tirith-core::engine::analyze, and add stderr diagnostics for audit and CLI output failures to improve audit reliabilityAdds inline bypass parsing for env wrappers and PowerShell, introduces self-invocation fast-allow in Exec scans, tokenizes segments to skip leading env assignments for accurate command resolution, adjusts URL extraction to avoid env and output-file false positives, and prints stderr errors on audit and CLI write failures.
📍Where to Start
Start with
analyzein crates/tirith-core/src/engine.rs, then reviewfind_inline_bypass,is_self_invocation, and tokenization changes in crates/tirith-core/src/tokenize.rs.Macroscope summarized 92ba3f5.