Repository navigation
feat(shell): land scan module (from closed #13) and bump tinybus - #14
Conversation
Make the scan module publicly accessible and add a dedicated test module for it, enabling external use of the scanning functionality and keeping its tests separate from the existing shell tests. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Removed an unnecessary `continue` statement in the quote state machine that was unreachable after the closing quote handling. Also refactored two `match` expressions into more idiomatic `let Some` patterns with guard conditions, making the control flow clearer without changing behaviour. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
Warning Review limit reached
This review includes 4 billable files and costs up to $1.00. Or wait 15 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 0 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Incomplete Review snapshot
Completeness: Incomplete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Could not review: crates/tinybox-core/src/shell/mod.rs, crates/tinybox-core/src/shell/scan.rs, crates/tinybox-core/src/shell/scan_test.rs Before merge
How this fits togetherflowchart LR
n0["script<br/>changed"]:::changed
n1["push"]:::impacted
n2["strip_quoted_heredoc_bodies"]:::impacted
n3["is_empty"]:::impacted
n4["command_line"]:::impacted
n5["split_unquoted_segments"]:::impacted
n6["quote"]:::impacted
n0 -->|calls| n1
n0 -->|calls| n3
n0 -->|calls| n4
n0 -->|calls| n6
n2 -->|calls| n1
n2 -->|calls| n3
n4 -->|calls| n6
n5 -->|calls| n1
n5 -->|calls| n3
n6 -->|calls| n1
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a56dca329
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| '\\' => escaped = true, | ||
| '\'' => quote = QuoteState::Single, | ||
| '"' => quote = QuoteState::Double, | ||
| '<' if chars.next_if(|(_, c)| *c == '<').is_some() => { |
There was a problem hiding this comment.
Because this pass records every << before it knows which lines are heredoc bodies—and does not recognize shell comments—inert text can create fake heredocs. For example, cat <<'EOF'\nliteral <<'X'\nEOF\necho ok\nrm -rf / queues X from the body and then blanks the live rm line; true # <<'X'\nrm -rf / has the same effect. A policy guard scanning the returned string therefore misses commands the shell will execute, so operators must be discovered only in actual command text while processing lines sequentially.
Useful? React with 👍 / 👎.
| while next_delim < operators.len() && operators[next_delim] < line_end { | ||
| // The last operator on a line wins: `cat << 'A' << 'B'` reads A's | ||
| // body first, but only tracking one at a time is enough for the | ||
| // guard, and a nested case simply keeps scanning as today. | ||
| open = Some(delimiters[next_delim].clone()); |
There was a problem hiding this comment.
When an unquoted heredoc precedes a quoted one, such as cat <<EOF <<'Q'\n$(danger)\nEOF\nliteral\nQ, operators contains only Q, so this loop opens it immediately after the command line and blanks the earlier unquoted body. The shell reads heredocs in redirection order and expands $(danger) in the EOF body, meaning policy scanning the stripped result misses live command substitution; track all heredocs in order and blank only the bodies whose own delimiter was quoted.
Useful? React with 👍 / 👎.
| let Some(quote @ ('\'' | '"')) = chars.next() else { | ||
| return None; | ||
| }; |
There was a problem hiding this comment.
Shell quote removal disables heredoc-body expansion when any part of the delimiter word is quoted, including forms such as <<\EOF and <<E"OF", but this parser accepts only tokens beginning with a single or double quote. Consequently these valid quoted heredocs are returned unchanged and inert body text containing $(), backticks, or & can be rejected by downstream guards; parse quoting throughout the delimiter word and remember whether any character was quoted.
Useful? React with 👍 / 👎.
| if let Some(delim) = open.clone() { | ||
| if line.trim() == delim { | ||
| open = None; |
There was a problem hiding this comment.
For ordinary <<, the delimiter must appear exactly by itself; only <<- permits leading tabs, and neither form permits arbitrary surrounding whitespace. Using trim() therefore ends stripping early on a body line such as EOF, leaving the remainder of the still-inert quoted body visible to policy scanners and causing false rejections. Compare exactly for <<, and for <<- remove only leading tab characters.
Useful? React with 👍 / 👎.
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinybox-core/src/shell/mod.rs, crates/tinybox-core/src/shell/scan.rs, crates/tinybox-core/src/shell/scan_test.rs.
$0.0069 · 50,559 in / 15,525 out · 5,888 cached (12%) · ladder/vectors, deepseek/deepseek-v4-flash · 1,144 embedded
tests: $0.0029 · 24,270 in / 5,105 out · 3,072 cached (13%) · deepseek/deepseek-v4-flash
description: $0.0024 · 14,847 in / 7,032 out · 2,304 cached (16%) · deepseek/deepseek-v4-flash
tinysweeper
Bot
added
the
priority: p3
Re-lands the two commits from closed PR #13 (branch oh-w2-tinybox-on-v0.1.8, a1b5bf0) that add the public
tinybox_core::shell::scanmodule and tests. The OpenHuman host (crates/openhuman-core security/policy) importstinybox_core::shell::scan, so main needs it. Also bumps nested tinybus to df6f990. fmt, clippy -D warnings, tests and docs pass locally.