fix(interp): gate $(<file) shortcut on cat being in the allowlist#192
Merged
fix(interp): gate $(<file) shortcut on cat being in the allowlist#192
Conversation
The $(<file) POSIX shortcut read files directly from cmdSubst, bypassing AllowedCommands. Reject any < redirect that is not paired with a command so the only way to read a file is through an allowed builtin (e.g. $(cat file)).
The previous commit rejected bare < redirects but accepted any command paired with <. That is broader than needed: `echo < file` would open a file purely to hand its stdin to a command that ignores it. Narrow the rule to the 11 file-reading builtins (cat, cut, grep, head, sed, sort, strings, tail, tr, uniq, wc) so < only applies where reading is the command's actual purpose. Also reject dynamic command names since they cannot be validated statically.
matt-dz
changed the title
Revert the overly-broad validation that blocked $(<file) entirely and restricted < to a fixed set of commands. The report's remediation keeps the shortcut functional but checks AllowedCommands at runtime: $(<file) is treated as an implicit $(cat file), so the read only proceeds when cat is allowed. This matches bash semantics and keeps legitimate uses working while closing the original allowlist bypass.
matt-dz
changed the title
Add three regression tests to the existing pentest suite: - SandboxEnforcedWhenCatAllowed: the gate and AllowedPaths are independent defences. Allowing cat does not let a caller read files outside the sandbox. - SymlinkEscapeBlocked: a symlink inside AllowedPaths pointing outside must be refused by os.Root, not followed. - VariableExpandedPath: the shortcut runs word expansion on its path, both for legitimate use (F=data.txt; $(<$F)) and to guarantee the gate cannot be dodged by hiding the path behind a variable.
Collaborator
Author
|
@codex conduct a comprehensive security and code review |
![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: 4f45c641f3
ℹ️ 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".
Per AGENTS.md, scenarios should prefer expect.stderr over stderr_contains when the output is deterministic. The blocked $(<file) gate emits a single fixed diagnostic and no variable sandbox warnings, so the contains-only form would miss extra stderr noise from a future regression.
Collaborator
Author
|
@codex conduct a comprehensive security and code review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
matt-dz
requested review from
AlexandreYang,
astuyve,
julesmcrt,
thieman and
val06
as code owners
thieman
reviewed
Apr 22, 2026
thieman
reviewed
Apr 22, 2026
thieman
approved these changes
Apr 22, 2026
…:cat is allowed Pairs with blocked_redirects/cat_shortcut_bypass.yaml to prove the gate decision hinges purely on cat's membership in allowed_commands and has no other side effects on stdout/stderr.
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
2 participants
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
$(<file)POSIX shortcut incmdSubstread files directly, skippingAllowedCommands. A caller withrshell:echo(norshell:cat) could still read any file insideAllowedPathsviax=$(<file); echo "$x".catbeing inAllowedCommands. The shortcut is conceptually$(cat file), so the allowlist check usescatas its key.catis missing from the allowlist, the shortcut prints$(<file): file read not permitted (cat not in allowed commands)to stderr, sets$?=1, and yields empty output — no file is opened.Behaviour
Test plan
make fmtcleango test ./...greenTestCmdSubstCatShortcutCommandAllowlistBypass— the original exploit.TestCmdSubstCatShortcutAllowedWhenCatAllowed— shortcut works when onlycat(not all commands) is allowed.redir_input_bypass_pentest_test.go:BlockedWhenCatNotAllowed— the exploit is refusedAllowedWhenCatAllowed— legitimate use worksExitStatusPropagates—$?=1after a blocked shortcutNoFileAccessWhenBlocked— gate short-circuits before any filesystem accessVariousContexts— shortcut in assignment / echo arg / backticks / for-iterator, each tested for both allowed and blocked casescat_shortcut*.yamlscenarios restored to their pre-exploit behaviour (shortcut works under default test harness which allows all commands), andcat_shortcut_bypass.yamlasserts the runtime gate refuses whencatis missing fromallowed_commands.rshellbuild.RSHELL_BASH_TEST=1 go test ./tests/ -run TestShellScenariosAgainstBash) —cat_shortcut_bypass.yamldiverges from bash and is markedskip_assert_against_bash: true; the rest match bash.