Env-injection deny-list and allow-list SSH safe-allow fast path#37
Merged
Conversation
Block dangerous injected environment variable names (LD_PRELOAD, BASH_ENV, GIT_SSH_COMMAND, PYTHONPATH, NODE_OPTIONS, the LD_AUDIT and GIT_CONFIG_KEY/ VALUE families, etc.) from --env/--secret injection, since a value under any of them is code the child runs before its own logic. Add a deterministic pre-LLM allow for a fixed, narrow set of trivially safe read-only commands: local id/whoami/hostname/uptime and, over ssh, a fixed read-only diagnostic remote command. Disabled in paranoid mode, never applied when env/secrets are injected, and forfeited by any shell metacharacter or risky ssh transport option (-L/-D/-J/-W/ProxyCommand/ LocalCommand/forwarding). Routes as a bypass allow like a trusted verb. Salvaged and re-implemented fresh against current main from the wsl-broker-wip snapshot.
The deterministic safe-allow fast path skipped the LLM evaluator for a fixed set of read-only diagnostics, including one run over ssh. It forfeited the fast path by matching a deny-list of risky ssh option names, which is inherently incomplete: any unlisted option (an external -F config, an -I PKCS#11 module, a newline-smuggled second -o directive, RemoteCommand, or a future addition) slipped through. Replace the deny-list with an allow-list: the ssh fast path is taken only when every option in the invocation is on a small vetted set (address family, compression, verbosity, no-tty and the restrictive agent/X11/GSSAPI toggles; -p, -l, and an -o keyword whitelist covering timeouts, batch mode, keepalive, and host-key handling). Anything else forfeits to the evaluator. The -o value is rejected outright if it contains a newline. The scan covers the whole option zone, not just options before the destination: ssh honors options placed between the destination and the remote command, so the scan stops only at the command token (the second positional). This behavior was verified against ssh's own -G dry run. Adds table-driven tests for the vetted/forfeited option sets, the between-host-and-command position, and newline-smuggled -o directives.
Review follow-up. The -o StrictHostKeyChecking directive was allowed on the fast path regardless of value, so StrictHostKeyChecking=no would take the deterministic path over an unauthenticated channel. Permit the keyword only for its security-preserving values (yes, accept-new, or an empty value that falls back to ssh's strict default); no/off/ask now forfeit to the evaluator. accept-new, which the --hostkey mode injects, stays on the fast path. Also add tests for a value option (-p/-l) given without its value.
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
1 participant
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.
Adds a deny-list for environment variable names that can inject code into a child process (LD_PRELOAD, BASH_ENV, GIT_SSH_COMMAND, and similar) and a deterministic safe-allow fast path that lets a fixed set of read-only diagnostics skip the LLM evaluator. The ssh side of that fast path uses allow-list (deny-by-default) semantics: it is taken only when every ssh option is on a small vetted set and the remote command is a fixed read-only diagnostic, so any unrecognized option, external config, forwarding/proxy directive, or newline-smuggled -o forfeits to the evaluator. The option scan matches ssh's real getopt behavior (options between the destination and the remote command are honored, verified against ssh -G), StrictHostKeyChecking is permitted only in its security-preserving values, and the change is covered by table-driven tests.