🏗️🔧:stop a filename being read as an option - #903
Conversation
Quoting settles what the shell does with a name and nothing about what the tool then makes of it. `'--write.md'` reaches prettier as `--write.md`, and prettier reads it as an option: it printed `Cannot read properties of null` and exited 0. A check that passed having checked nothing, which is worse than one that failed. A leading `./` says the argument is a path. It costs a relative name two characters and only the names that need it get them. Demonstrated on a file called `--write.md` holding deliberately bad Markdown. Before: prettier errored and the task exited 0. After: prettier reports the file, and markdownlint reports MD019 inside it. This is the residue of the same defect as the quoting itself -- a filename is attacker-chosen input that reaches a command line, and there were two ways for it to matter rather than one. Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is> Assisted-by: Claude-Code:claude-opus-5
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe ChangesShell Path Quoting
Estimated code review effort: 2 (Simple) | ~5 minutes Merge Risk: ⚪ Minimal · up to Dash-prefixed relative filenames are now passed as paths rather than options, while normal and absolute paths retain their existing behavior. The change is covered and ready to merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Follow-on from #888, found while sweeping the portal for the same class
of bug after a review there. Quoting a filename settles what the shell
does with it and nothing about what the tool then makes of it.
quote('--write.md')produces'--write.md'. The shell strips thequotes and prettier receives
--write.md, which it reads as an option:Exit 0. A check that passed having checked nothing, which is worse
than one that failed —
verify.mdwould go green while quietlyprocessing neither file.
The fix
A leading
./says the argument is a path. Two characters, and only onthe names that need them; an ordinary or absolute path is untouched.
Demonstrated end to end
A file called
--write.mdholding deliberately bad Markdown:[error] Cannot read properties of null, task exits 0[warn] --write.mdfrom prettier, andMD019/no-multiple-space-atxfrom markdownlintSo the file is genuinely checked rather than silently skipped.
Why it is worth doing at all
verify.filenameswould reject such a name — but as #900 explains, it isone task among the rest and a failing one does not stop the others being
handed what it just objected to. A filename is input chosen by whoever
opens the pull request, and it turned out to have two ways of mattering
rather than one.
The portal needs the same change; I will fold it into
openinf.github.io#1881,
which is where its copy of
quote()lives.Summary by CodeRabbit
Bug Fixes
-, ensuring command-line tools interpret them as paths rather than options.Tests
--write.mdand-.