Repository navigation
fix(O170): block shell-exec env assignments (BASH_ENV etc.) and tmux/screen spawn forms - #158
Conversation
BASH_ENV / ENV / PROMPT_COMMAND / ZDOTDIR assignments (every shape: prefix, env, export, set, bare, pwsh $env:) and exported BASH_FUNC_* bodies make a POSIX shell run code; tmux new-session/new-window/ split-window/run-shell/send-keys and screen with a command spawn outside Moss's tree. None tripped the exec-capable gate on main — a BASH_ENV probe ran. The env forms fire only when a shell head runs on the same line (ENV=production npm start stays green); command-less tmux/screen forms are unaffected.
…c-bypass # Conflicts: # CHANGELOG.md # README.en.md # README.md
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: eec8b9628e
ℹ️ 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".
| for (const w of words) { | ||
| const m = ASSIGN.exec(w) ?? w.match(/^\$env:([A-Za-z_][A-Za-z0-9_]*)=/i); | ||
| if (m && isShellExecEnvName(m[1]!)) shellExecEnv.add(m[1]!.toUpperCase()); |
There was a problem hiding this comment.
For a normal app-config invocation such as bash -c 'ENV=production node -p process.env.ENV', this loop treats the entire quoted command-string argument as an environment assignment because it starts with ENV=. The outer bash then satisfies sawShellHead, even though the assignment happens inside its payload and only applies to Node, not to a shell startup. I verified that this command returns no forms before this commit but now produces an exec-capable protected-path hit, causing legitimate headless commands to be denied. Let the existing nested scan interpret shell payloads instead of treating arbitrary argument words as assignments.
Useful? React with 👍 / 👎.
| while (i < words.length && words[i]!.startsWith('-')) i += 1; | ||
| // `screen -dmS name cmd …`: an -S option consumed the session name. | ||
| if (i < words.length && /^-[a-zA-Z]*S$/.test(words[i - 1] ?? '')) i += 1; | ||
| if (i < words.length) return 'screen command'; |
There was a problem hiding this comment.
For existing-session operations such as screen -r build, this loop stops at the session selector build and incorrectly reports screen command. The GNU Screen manual documents -r [pid.sessionname] as “Resume a detached screen session,” not spawning a command. Query operations such as screen -S build -Q windows are also misclassified because parsing does not continue past the -S operand. I verified that both previously returned no forms and now produce exec-capable protected-path hits, introducing unnecessary interactive approvals and headless denials. Consume option operands and distinguish resume/query modes from actual command-spawning forms.
Useful? React with 👍 / 👎.
…c-bypass # Conflicts: # CHANGELOG.md # README.en.md # README.md
O170 (P2): indirect execution bypasses that already existed on main
BASH_ENV=<file> bash -c ':'makes bash source the file;ENV=does thesame for POSIX sh/dash;
PROMPT_COMMAND=runs before every prompt of aninteractive bash;
ZDOTDIRmoves zsh's.zshenv; an exportedBASH_FUNC_name%%=() { … }body is code itself.tmux new-session/new-window/split-window/run-shell/send-keys, andscreencarrying a command, run it outside Moss'sprocess tree.
None of these tripped the exec-capable gate on main — a probe planted in
BASH_ENVran while the command looked like an inert assignment.Fix (
src/safety/exec-capable-read.ts)isShellExecEnvName+isBashFuncWord(line-level: tokenizers split onthe parens) collect the assignments from every shape — prefix
assignments,
env,export/declare,set, bare statements, pwsh$env:/[Environment]::SetEnvironmentVariable— and the forms arereported only when a POSIX shell head runs on the same line: the common
app-config name
ENVmust not false-positive (ENV=production npm startstays green; pwsh heads don't read these variables).
shellSpawnFormreports tmux/screen only when they carry a command:tmux ls,tmux attach,screen -lsare unaffected.Tests —
test/o170-indirect-exec-bypass.spec.mjs(39)split-window/run-shell/send-keys; screen
-dmSand-X stuff.ENV=production npm start,BASH_ENV=… node x.js(no shell head),plain
bash -c/zsh -c,FOO=1 bash -c, command-less tmux/screen.dist/cli.js -p, marker probe):BASH_ENV=<probe> bash -c ':'— probe never runs, exit 9 「被拦」; controlbash -c ':'exit 0.Revert-red (local, macOS)
Neutralizing
isShellExecEnvName+shellSpawnForm→ 43 red; with thefix 39/39. Related: o151 204/204, o101 24/24, o117 51/51,
hooks-fail-closed 25/25,
npm run checkgreen.Head
eec8b962includes main at9d455d94(#155).