fix(work): name the remedy when verify rejects a shell interpreter - #416
Conversation
verify run rejects shell interpreters (bash, sh, ...) as high-risk executables, but unlike the sibling shell-metacharacter branch the message gave no path forward, dead-ending the caller. Add a shared _high_risk_command_message helper used by both the --command and --argv-json rejection sites. It names the supported pattern (a resolvable executable, e.g. a chmod +x script invoked by its path) and deliberately does not point at --argv-json, which applies the same high-risk block. Message-only change; the set of accepted and rejected commands is unchanged. Adds coverage for both rejection paths. Closes #412
|
Caution Review failedThe pull request is closed. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: escoffier-labs/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughHigh-risk verification executable rejections now use a shared message with remediation guidance. A parametrized CLI test covers both ChangesVerification rejection guidance
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Closes #412.
What
verify runrejects shell interpreters (bash,sh,zsh, ...) as verifyexecutables, which is correct: verify runs argv directly with
shell=False, so ashell interpreter is never a valid executable. But the rejection message
dead-ended at
high-risk verification command: bash, while the siblingshell-metacharacter branch already names its remedy (
use --argv-json). A callerwhose check was
bash ./script.shhad no pointer to the supported pattern.Change
_high_risk_command_messagehelper, used by both the--command(
_verify_parse_command) and--argv-json(_verify_parse_argv) rejectionsites so the two stay in sync.
chmod +xscript invoked by its path) and deliberately does not point at--argv-json, which applies the same high-risk block and so is not a shellescape hatch.
Message-only: the set of accepted and rejected commands is unchanged.
Tests
Adds
test_work_verify_run_rejects_shell_interpreter_with_remedy, parametrizedover
--commandand--argv-json, asserting the rejection carries the remedytext and does not mention
--argv-json../scripts/verifygreen: ruff, ruff-format, mypy, version-sync, managed-snapshot,and
3502 passed, 3 skipped, coverage 82.42%.Summary by CodeRabbit
Bug Fixes
Tests
work verify runrejects these high-risk commands in both--commandand--argv-jsonmodes, and that the error details include the recommended remedy without referencing the--argv-jsonflag.