fix(rp1): identify the Docker image operand after options

[ilaigold]

Closes #781. It's the Docker image case from #672 that #683 left for later.

I hit this while scanning my skills with --no-llm: on OpenShell's debug-openshell-cluster skill, RP1 said the unpinned image was docker run --rm. RP1 takes the first word after docker run|create|pull as the image and looks for a tag anywhere in it. So options get reported as images, and a registry port (localhost:5000/team/tool) or an option value (--publish=8080:80) passes as a tag.

The fix reads the command as shell words, skips options, and only looks for a tag in the image's last path component. Two things you might ask about:

• The option table is copied from docker/cli (container/opts.go, run.go, create.go, image/pull.go), hidden and deprecated flags included, so nothing is guessed about which options take a value. A value option takes the next word even if it starts with -, same as pflag.
• When the image can't be identified (unknown option, missing value, unterminated quote, $((...)), or the 1024-character read bound), the command is still reported, like on main.

Detections: a finding from main only goes away when every word before the image is a known option (or a self-contained --name=value) and the image passes the same _VERSION_PIN_RE check main already uses. On 20,000 generated docker run|create commands, main missed 1,026 unpinned images and this branch misses none. Pinned images that still get reported drop from 7,852 to 237, and every one of those has an unknown option.

Tests: paired cases in tests/test_mcp_rug_pull.py, 12 benign (pinned images after flags, value options, -p8080:80, --, line continuations) and 14 malicious with exact matched_text (--user 1000:1000, a registry port, --publish=, the OpenShell ${VAR:-...:latest} form). Another 9 check that commands with an unresolvable image are still reported.

ReDoS: the alternatives in the shell-word regex each start with a different character class, and each scan reads at most 1024 characters past the subcommand. test_rp1_docker_operand_scan_is_linear runs 7 adversarial ~50k-character lines with a 1 s limit each. Outside the suite, _check_rp1 stayed under 0.1 s on 11 such lines, and the single-finding ones grew linearly up to 200k characters.

One fixture change needs your call. test_privileged_payload_in_referenced_reference_file_stays_install_unsafe (#682) only reached DO_NOT_INSTALL through the false positive (--privileged reported as the image). Once vendor/collector:1.4 reads as pinned, PE5 and TM4 alone score 48 and the test fails. I made the fixture image unpinned so RP1 fires for a real reason, and left PE5/TM4 scoring alone since that's the #644 B7 question. Tell me if you'd rather handle it another way.

matched_text now runs from docker through the image, so fingerprints change for findings that used to name a flag (and where main kept a trailing backtick or ; in the match).

Checked on main @ 5485cda:

• uv run pytest -m "not integration and not provider" tests/ -q: 10309 passed, 0 failed
• the three new behavior tests fail with only mcp_rug_pull.py reverted to main
• uv run make lint and uv run make format-check pass
• OpenShell debug-openshell-cluster @ 360c5a0: same two RP1 findings, now naming the real images, score still 60
• merges cleanly with the other open RP1 PRs, fix(rp1): prevent npx matches across lines #646 and fix(mcp-rug-pull): inspect npx commands in JSON configs #776

🤖 Generated with Claude Code

[yashrajp22]

Two introduced RP1 missed-warning cases are confirmed below. The selected HEAD tests passed (173 passed, 4 expected failures) in source and fresh-wheel runs; separate focused checks reproduced both issues through the full CLI.

[yashrajp22]

Could we keep RP1 for image expressions whose selected value is unknown? For example, with IMAGE=alpine, docker run --rm "${IMAGE:-alpine:3.20}" selects unpinned alpine, but this check accepts the fallback’s :3.20 and suppresses the warning. "${IMAGE:1}" has the same problem when IMAGE=_alpine: :1 is a substring offset, not an image tag.

Both cases produced one RP1 finding on the baseline and none on this commit in direct analyzer checks and full CLI scans against source and freshly installed wheels. Please retain the conservative warning for unresolved expansions unless the resulting image is provably pinned.

[yashrajp22]

Could we handle shell redirections before accepting this word as the image, or conservatively leave the operand unresolved? In docker run --rm 2>log:1 alpine, the shell consumes 2>log:1 as stderr redirection and Docker runs the unpinned alpine image. This branch instead returns 2>log:1, and the pin check treats the filename’s :1 as an image tag.

The baseline emits RP1 for this input, while this commit emits no warning in both source and installed-wheel CLI scans. A redirection filename should not be able to suppress the real image’s warning.

[ilaigold]

Thanks for the careful review and the repros. Both are fixed in b25acf8.

Expansions: an image word with an expansion now only counts as pinned when every expansion is double-quoted and the tag or digest is literal text after the last one, either past a / ("${REGISTRY}/tool:1.4") or right after the expansion ("${IMAGE}:1.4", "$IMAGE"@sha256:...). So "${IMAGE:-alpine:3.20}" and "${IMAGE:1}" are reported again. Unquoted $REGISTRY/tool:1.4 and brace expansion like {alpine,alpine:3.20} are reported too. While I was in there I found two more ways to fake the literal part: "$@/tool:1.4" still splits into several words inside quotes, and in a command substitution a ) from a case pattern can look like the closing paren. Both are always reported now. Covered by test_rp1_docker_image_with_unknown_expansion_is_reported, with test_rp1_docker_image_pinned_after_an_expansion_no_finding for the benign side.

Redirections: < and > now end a word, and a redirection is skipped together with its target wherever it shows up (2>log:1, >, >>, <, &>, 2>&1, >|, <<<, heredoc markers), including between an option and its value. So docker run --rm 2>log:1 alpine reports alpine, and alpine>log:1 is read as alpine. When the target is missing or I can't read it (2> at the end of the line, <(env)), the command stays reported. Covered by test_rp1_docker_redirection_target_is_not_taken_as_image, test_rp1_docker_redirections_before_pinned_image_no_finding and test_rp1_docker_unclear_redirection_is_still_reported. The linear-time test has a few new inputs for both paths and still passes.

Two things I left on purpose. A placeholder like docker run <image>:1.0 is now reported, since the shell reads it as two redirections and no image. And I didn't treat unquoted expansions in option values before the image as unresolved (-v $(pwd):/app node:20). Word splitting there could in theory shift which word is the image, but the pattern is everywhere in docs and flagging it would bring back most of the noise this PR removes. Happy to make that strict if you'd rather.

[yashrajp22]

Rechecked b25acf8. The original expansion and redirection examples now behave as expected. Two distinct operand-selection cases still suppress RP1, detailed below.

Validation: 236 offline source/installed-wheel scans, with all 118 paired reports matching after removing timestamps and generated finding IDs. The selected HEAD tests passed 178 cases with 4 existing xfails in each mode. Forty scan reports retain partial coverage; this is scoped offline verification, not a claim of complete scanner accuracy.

[yashrajp22]

Could we keep the operand unresolved when an option value can expand into multiple arguments? With DOCKER_ENV='MODE=dev ubuntu', docker run --rm -e $DOCKER_ENV alpine:3.20 uses unpinned ubuntu as the image; alpine:3.20 becomes the container command. This branch skips one raw word and accepts that later tag instead. The baseline reports RP1, while this commit produces a complete scan with no findings in both source and wheel runs. Attached option values and quoted array expansions have the same problem. Please preserve the warning unless the option value is provably one argument.

[yashrajp22]

Could we exclude &> and &>> from the optional descriptor prefix? Bash leaves 2 as an argument in docker run --rm 2&>log alpine:3.20: the image is untagged 2, and the later tagged word is the container command. Numeric image names are valid, including a local 2:latest image. This regex instead consumes 2&>log and accepts the later tag. The baseline reports RP1, while both source and wheel scans at this commit report no findings with complete coverage. The ordinary &>log alpine:3.20 control correctly stays clear.