diff --git a/agent_tool/shell/README.mbt.md b/agent_tool/shell/README.mbt.md index 569d7379a..bf5903e07 100644 --- a/agent_tool/shell/README.mbt.md +++ b/agent_tool/shell/README.mbt.md @@ -57,7 +57,7 @@ workspace root, respecting `moon -C ` and explicit cwd changes such as `cd ./dir && moon ...`. Bare relative `cd dir` is skipped because `CDPATH` can change which directory the shell enters. This covers commands such as `moon add`, `moon remove`, `moon update`, `moon fmt`, `moon info`, -`moon test --update`, and `moon ide rename ... --apply`. Read-only variants +`moon test -u`/`moon test --update`, and `moon ide rename ... --apply`. Read-only variants such as dry-run commands and `moon fmt --check` are left alone, as are Moon invocations with per-command environment overrides such as `FOO=bar moon` or `env FOO=bar moon`. Follow-up checks are skipped when `timeout_ms` is set and @@ -96,12 +96,15 @@ source paths are indirect, as in `while read f; do sed -i ... "$f"; done`. Too-complex commands with visible MoonBit source creation or tree transfer markers are also rejected before execution. -Narrowly recognized Moon commands that are expected to write source or package -metadata run outside the source-write sandbox: `moon fmt`, `moon info`, -`moon add`, `moon remove`, `moon update`, `moon test --update`, and -`moon ide rename ... --apply`. Compounds are conservative; `moon fmt && -moon check` is trusted, while a broad script or source rewrite through shell is -not. +Recognized Moon commands that can run pre-build hooks or intentionally update +source and package metadata run outside the source-write sandbox. This includes +`moon check`, `moon build`, `moon test`, package-based `moon run`, `moon fmt`, +`moon info`, `moon add`, `moon remove`, `moon update`, `moon test -u`/`moon test +--update`, and `moon ide rename ... --apply`. Compounds are conservative; `moon +fmt && moon check` is trusted. A trusted writer may pipe output through the +read-only `head` limiter, so commands such as `moon test -u 2>&1 | head -40` can +update snapshots while bounding output. Other pipeline consumers, broad scripts, +and source rewrites through shell remain sandboxed. Recognized first-party Git porcelain composes outside the source-write sandbox. This lets ordinary workflows such as `git fetch && git rebase`, `git pull diff --git a/agent_tool/shell/moon_auto_check.mbt b/agent_tool/shell/moon_auto_check.mbt index 6850f34df..6748e9f69 100644 --- a/agent_tool/shell/moon_auto_check.mbt +++ b/agent_tool/shell/moon_auto_check.mbt @@ -517,6 +517,7 @@ test "detect moon commands that should trigger follow-up check" { ), ) assert_true(should_auto_check_moon_command("moon test --update")) + assert_true(should_auto_check_moon_command("moon test -u")) assert_true(should_auto_check_moon_command("command moon fmt")) assert_true(should_auto_check_moon_command("command -- moon fmt")) assert_true(should_auto_check_moon_command("command -p moon fmt")) diff --git a/agent_tool/shell/review_guard.mbt b/agent_tool/shell/review_guard.mbt index 41a788984..16da99c8f 100644 --- a/agent_tool/shell/review_guard.mbt +++ b/agent_tool/shell/review_guard.mbt @@ -181,6 +181,13 @@ test "read-only guard blocks mutating moon anywhere; allows read-only commands" scratch_dir=None, ), ) + assert_true( + review_read_only_blocks( + @shell_parse.parse_for_policy("moon test -u; true"), + base_cwd=".", + scratch_dir=None, + ), + ) assert_true( review_read_only_blocks( @shell_parse.parse_for_policy("FOO=bar moon fmt"), diff --git a/agent_tool/shell/sandbox.mbt b/agent_tool/shell/sandbox.mbt index 3b2ad8502..7e693b587 100644 --- a/agent_tool/shell/sandbox.mbt +++ b/agent_tool/shell/sandbox.mbt @@ -181,10 +181,17 @@ fn commands_are_trusted_source_writes( ) -> Bool { let mut has_trusted_write = false for index, command in commands { - if command.separator_before is Some("|" | "||") { - return false + let command_class = trusted_command_class(command, commands, index) + match command.separator_before { + Some("||") => return false + // A trusted source-writing command may feed a read-only output consumer + // such as `head`. Never trust a writer after a pipe: that would make the + // whole shell run unsandboxed for a command shape that is not needed for + // validation output limiting. + Some("|") if !(command_class is TrustedCommandRead) => return false + _ => () } - match trusted_command_class(command, commands, index) { + match command_class { TrustedCommandWrite => has_trusted_write = true TrustedCommandNeutral | TrustedCommandRead => () UntrustedCommand => return false @@ -220,12 +227,29 @@ fn trusted_command_class( } "moon" => trusted_moon_command_class(command, command_index) "git" => trusted_git_command_class(command, command_index) + "head" => trusted_head_command_class(command, command_index) _ => UntrustedCommand } _ => UntrustedCommand } } +///| +/// `head` is a read-only output limiter used to keep compiler and test output +/// bounded (`moon test ... 2>&1 | head -30`). Trust only the direct command +/// without environment wrappers; source-path redirects are rejected before +/// this classifier runs. +fn trusted_head_command_class( + command : @shell_parse.SimpleCommand, + head_index : Int, +) -> TrustedCommandClass { + if head_index == 0 && !command_has_custom_environment(command) { + TrustedCommandRead + } else { + UntrustedCommand + } +} + ///| /// Classify a `git` command for the trusted-source-write path. Recognized /// first-party porcelain composes by default, while aliases, plumbing, external diff --git a/agent_tool/shell/sandbox_wbtest.mbt b/agent_tool/shell/sandbox_wbtest.mbt index 163a9e6a2..84c25afb4 100644 --- a/agent_tool/shell/sandbox_wbtest.mbt +++ b/agent_tool/shell/sandbox_wbtest.mbt @@ -16,10 +16,18 @@ test "sandbox trusts only narrow moon source-writing commands" { assert_true(mode_for("moon fmt >/dev/null") is TrustedSourceWrite) assert_true(mode_for("moon info > /tmp/log") is TrustedSourceWrite) assert_true(mode_for("moon test --update 2>/tmp/err") is TrustedSourceWrite) + assert_true(mode_for("moon test -u 2>/tmp/err") is TrustedSourceWrite) assert_true(mode_for("moon fmt > out.log") is TrustedSourceWrite) assert_true(mode_for("moon check") is TrustedSourceWrite) assert_true(mode_for("moon build") is TrustedSourceWrite) assert_true(mode_for("moon test") is TrustedSourceWrite) + assert_true( + mode_for("moon test cfront/build -u 2>&1 | head -30") is TrustedSourceWrite, + ) + assert_true( + mode_for("moon test cfront/build --update 2>&1 | head -n 40") + is TrustedSourceWrite, + ) assert_true(mode_for("moon run cmd/main") is TrustedSourceWrite) assert_true(mode_for("moon run cmd/main -- --help") is TrustedSourceWrite) assert_true(mode_for("moon run cmd/main -- -u") is TrustedSourceWrite) @@ -48,6 +56,21 @@ test "sandbox trusts only narrow moon source-writing commands" { assert_true(mode_for("sed -i '' 's/a/b/' *.mbt") is SourceReadOnly) } +///| +test "sandbox trusts only read-only consumers after moon output pipes" { + assert_true(mode_for("moon test 2>&1 | head -30") is TrustedSourceWrite) + assert_true( + mode_for("moon test -u 2>&1 | head -40 | head -20") is TrustedSourceWrite, + ) + for + cmd in [ + "moon test -u | tee snapshots.txt", "moon test -u | sh", "moon test -u | moon fmt", + "env LIMIT=30 head -30 | moon test -u", + ] { + assert_true(mode_for(cmd) is SourceReadOnly) + } +} + ///| async test "sandbox preblocks source-writing script snippets" { let root = "/workspace"