Repository navigation
feat(shell): quote-aware command-string scanners (tinybox_core::shell::scan) - #13
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>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. |
![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: a1b5bf0af2
ℹ️ 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".
Tiny Sweeper review
Last completed reportTiny 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["is_empty"]:::impacted
n3["command_line"]:::impacted
n4["quote"]:::impacted
n5["...cation_gives_every_box_its_own_identifier"]:::impacted
n6["capabilities_do_not_share_a_bit"]:::impacted
n0 -->|calls| n1
n0 -->|calls| n2
n0 -->|calls| n3
n0 -->|calls| n4
n3 -->|calls| n4
n4 -->|calls| n1
n5 -->|calls| n1
n5 -->|tests| n1
n6 -->|calls| n1
n6 -->|tests| 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
|
![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.0064 · 51,113 in / 6,726 out · 3,072 cached (6%) · ladder/vectors, deepseek/deepseek-v4-flash · 1,145 embedded
tests: $0.0024 · 24,252 in / 2,638 out · 3,072 cached (13%) · deepseek/deepseek-v4-flash
description: $0.0022 · 15,359 in / 71 out · 0 cached (0%) · deepseek/deepseek-v4-flash
tinysweeper
Bot
added
the
priority: p3
…rate The tinybox module pin exemption is updated to reflect a new pinned commit that includes the tinybox-core `shell::scan` module from tinyhumansai/tinybox#13, in addition to the existing tinybox-jail crate. The reason text is expanded to document both upstream changes while confirming that no module, contract, or bus source changes are introduced, so the registry artifact remains valid. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Extend the shell scanning logic to recursively check for unquoted background ampersands inside `$()` and backtick command substitutions. Previously these nested constructs were ignored, allowing commands like `$(foo &)` to bypass the background detection check. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Moved the `$()` and backtick command-substitution parsing in `contains_unquoted_background_ampersand` to after the quote-handling logic, fixing a bug where substitutions inside double quotes were incorrectly scanned. Also updated stale doc references from the old `crate::sandbox::cwd_jail` path to `crate::spawn` and `crate::Jail`, and hardened the spawn tests by adding read-only paths to the jail configuration. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Replace unconditional `expect` calls with match arms that assert on `PermissionDenied` errors, allowing the tests to pass on systems where the jail backend lacks the required permissions. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…utions The `strip_quoted_heredoc_bodies` function now preserves unquoted heredoc bodies instead of blanking them, since the shell expands variables and commands in those bodies. The heredoc delimiter parser was rewritten to handle quoted, escaped, and unquoted delimiters as complete words, and to track whether the delimiter was quoted. Multiple heredocs on the same line are now consumed in input order rather than only tracking the last one. The `contains_unquoted_background_ampersand` function now correctly detects ampersands inside nested command substitutions and backtick expansions. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The heredoc body stripping logic now returns early when none of the heredoc delimiters are quoted, since unquoted heredocs undergo variable expansion and their bodies cannot be safely stripped. Additionally, the delimiter parser now returns `None` when a newline is encountered inside a quoted delimiter, preventing an infinite loop. The operator tracking loop was also simplified to avoid opening a heredoc state for a delimiter that was already consumed in a previous iteration. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Restructure the conditional branches in the heredoc body processing loop to reduce nesting and improve readability. The behaviour is unchanged: quoted heredocs still produce only blank lines, while unquoted ones preserve their full content. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add test module declarations for `detect` and `linux` using the `#[path]` attribute, and introduce a new integration test that verifies `spawn` with an existing root directory handles both success and expected error cases gracefully. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add three tests that verify the jail registry correctly rolls back in-memory state when writing the index file fails, covering create, rename and notes, and delete operations. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ail directory Add two new test cases: one verifying that `JailRegistry::open` creates a missing nested base directory, and another ensuring that `JailRegistry::delete` persists correctly when the jail directory has already been removed externally. Also add an assertion in the landlock backend test to verify that `LandlockBackend::default()` returns the expected name. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Use the dedicated `expect_err` method instead of chaining `.err()` and `.expect()` for clearer intent and more idiomatic Rust. In the landlock test, introduce an intermediate variable for the default backend to improve readability. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ng_read_only_path The test was failing when the landlock feature was disabled, so it now only runs when the feature is active. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
# Conflicts: # crates/tinybox-jail/src/detect_tests.rs # crates/tinybox-jail/src/linux.rs # crates/tinybox-jail/src/linux_tests.rs # crates/tinybox-jail/src/mod_tests.rs # crates/tinybox-jail/src/registry_tests.rs
Add two new test cases to verify that the availability lookup checks only the named backend and that backend selection uses an available platform-specific backend when one is present. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
… and failed index write Add two new test cases for the JailRegistry delete operation. The first verifies that deleting a record succeeds when the jail directory has already been removed, and the second confirms that a failed index write after deletion leaves the in-memory state consistent with the removal having taken effect. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
# Conflicts: # crates/tinybox-jail/src/detect.rs
Reformatted the Landlock availability assertion to span multiple lines, improving code clarity without changing any behavior. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Remove the `pick_backend_with` and `is_available` functions along with their tests, as the backend selection logic has been simplified to always return the unsupported backend. The `pick_backend` function is now marked as `#[must_use]` to ensure callers handle the result. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The coverage.info file contained function and line coverage records for source files from a different worktree (pr-12), which are irrelevant to the current branch. This change removes those stale entries, keeping only the minimal coverage structure needed for the current codebase. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
feat(jail): add tinybox-jail (per-process directory jail)
…latform backends Replace the crate-local lint overrides with a single `workspace = true` reference, and remove the `#[cfg]`-gated platform backend modules from compilation until they can satisfy the workspace safety policy. The default backend now returns `Unsupported` on every platform, and callers can explicitly select `NoopBackend` when unrestricted execution is intended. Also adds missing `#[must_use]` annotations, `# Errors` and `# Panics` doc sections, and fixes a race condition in `JailRegistry::create` where a stale directory from a previous failed creation could cause an unrecoverable collision. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The coverage report file was being tracked in the repository, which is not intended as coverage data is a build artifact that should be generated locally rather than committed. Removing it keeps the repository clean and prevents unnecessary merge conflicts from regenerated coverage data. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99a0aa7297
ℹ️ 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".
…ction Move the closure that calls `restrict_self` into a named function to improve readability and reduce nesting inside the `pre_exec` call. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…r detection Add three tests that cover scenarios where heredoc bodies contain operator‑like text, where a heredoc delimiter is followed by a shell operator, and where an intervening command appears between two heredocs. These cases were previously untested and help ensure the scanner correctly distinguishes heredoc content from shell syntax. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…g in nested subshells Refactor `strip_quoted_heredoc_bodies` to process lines sequentially with a `VecDeque` of active delimiters, replacing the previous approach that tracked byte offsets and a single open delimiter. This correctly handles multiple heredocs declared on the same line. Also fix escape character handling inside double-quoted subshells in `contains_unquoted_background_ampersand` by adding a `nested_escaped` flag, and extract the heredoc declaration parser into a separate `heredoc_declarations` function for clarity. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The heredoc body stripping logic was comparing the `quoted` flag by value instead of dereferencing it, causing incorrect behavior when processing quoted heredocs. This change dereferences the flag to properly check whether the heredoc delimiter was quoted. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…tions Extract the inline logic for parsing `$(...)` and backtick command substitutions into dedicated functions to reduce duplication and improve readability in the main scanning loop. The extracted functions `command_substitution_body` and `backtick_substitution_body` handle the same parsing behaviour that was previously inlined. 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>
feat(shell): land scan module (from closed #13) and bump tinybus
Update the pinned commit of the tinybus vendored dependency to include the latest upstream changes. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93c2fb5032
ℹ️ 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".
| match ch { | ||
| '\\' => escaped = true, | ||
| '\'' => quote = QuoteState::Single, | ||
| '"' => quote = QuoteState::Double, | ||
| '<' if chars.next_if(|(_, next)| *next == '<').is_some() => { |
There was a problem hiding this comment.
When # starts a shell comment, this scanner continues parsing the remainder as executable syntax. For example, echo ok # <<'EOF'\necho DANGER & records a quoted heredoc that the shell never declared, so strip_quoted_heredoc_bodies blanks the live second command through end-of-input and a downstream policy scan can miss the background operator. Stop scanning a command line when an unquoted, token-starting # begins a comment.
Useful? React with 👍 / 👎.
| '(' => depth += 1, | ||
| ')' => { | ||
| depth -= 1; | ||
| if depth == 0 { | ||
| break; |
There was a problem hiding this comment.
This treats every unquoted ) as balancing an opening parenthesis, but POSIX case patterns contain a syntactic ) that does not close $(). Consequently, contains_unquoted_background_ampersand("echo \"$(case x in x) touch /tmp/pwn & wait;; esac)\"") returns false even though the shell executes the background command, because scanning stops at x). Fresh evidence in the updated helper is that it tracks only quote and parenthesis depth, with no handling for case grammar.
Useful? React with 👍 / 👎.
| } else if c == '\'' || c == '"' { | ||
| quote = Some(c); | ||
| quoted = true; | ||
| } else if c.is_whitespace() || ";&|<>()".contains(c) { |
There was a problem hiding this comment.
A heredoc delimiter word does not undergo command substitution, so in cat <<'X'$(foo)\nbody\nX$(foo)\necho DANGER & the actual quoted delimiter is the literal X$(foo). Treating ( as a word boundary records X$ instead, never finds the real terminator, and blanks the live command after it. Fresh evidence after the earlier delimiter fix is the explicit parenthesis boundary here; parse $() as literal delimiter text in this context.
Useful? React with 👍 / 👎.
Adds
tinybox_core::shell::scan, the pure lexical half of OpenHuman's shell command gate, so the mechanism lives with the other shell code and the policy stays in the host.Functions:
split_unquoted_segments,contains_unquoted_char,contains_unquoted_single_ampersand,contains_unquoted_background_ampersand,strip_quoted_heredoc_bodies,command_basename,normalized_command_name,has_leading_env_assignment,skip_env_assignments. They are moved byte-for-byte in behaviour (only docs and clippy-pedantic tidying changed); none of them decides whether a command is safe.Tests (
shell/scan_test.rs, 21) pin every case OpenHuman's policy suites depend on: quotes and escapes hiding separators,&&vs&,2>&1/&>fd-dup, quoted vs unquoted heredoc delimiters, here-strings, unterminated heredocs,.exe/case normalisation, env-assignment shape.Stacking: based on
oh-cwd-jail(#12), because OpenHuman pins tinybox at v0.1.8 + #12. Diff against #12 is the two scan commits only. No module, contract or bus changes.Co-authored-by: Medulla medulla@tinyhumans.ai