fix(core): parse exec form the way Docker does - #96
Merged
Conversation
Docker only treats bracket-wrapped instruction args as exec form when they parse as a JSON array of strings; `CMD [node, index.js]` and `ENTRYPOINT ['x']` fall back to shell form under /bin/sh -c. Two rules were each deciding that for themselves with a different, looser test. parseExecForm returns the argv or null, so callers get the parsed command back instead of re-parsing after a boolean check. An array holding a non-string element is null as well: Docker rejects it outright, so the line is broken either way and a shell-form diagnostic still points at it.
use-exec-form accepted anything starting with [ and ending with ], so `CMD [node, index.js]`, `ENTRYPOINT ['docker-entrypoint.sh']` and a trailing comma all passed as exec form. Docker parses none of them as JSON, runs them under /bin/sh -c, and drops signals on the way to the process -- the exact failure the rule exists to catch. Route the check through parseExecForm so the rule agrees with Docker. Dockerfiles that relied on the looser check will now see a warning; quoting the elements resolves it. Closes #89
use-pipefail carried its own isExecForm and jsonArrayEnablesPipefail, both looser than use-exec-form's check: an array of non-strings counted as exec form here but not there, so the two rules could disagree about what a line even is. parseExecForm returns the argv, which removes the parse-then-reparse both helpers did -- isExecForm parsed the args only to throw the result away, then jsonArrayEnablesPipefail parsed the same string again. Behavior is unchanged; SHELL and exec-form RUN keep their existing diagnostics.
🦋 Changeset detectedLatest commit: 0b36d74 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
The latest Docker Doctor scan for this pull request. Learn more about Docker Doctor.
Score: 84 / 100 · 3 issues
Scanned by Docker Doctor for commit |
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.
Description
use-exec-formdecided exec form withargs.startsWith("[") && args.endsWith("]"), so anything wearing brackets passed:None of those are JSON, so Docker runs them under
/bin/sh -cand signals never reach the process — the exact failure the rule exists to catch. All three reported 0 diagnostics; they now report 1.What changes
parseExecForm(args): string[] | nulllands inparsers/, backed byJSON.parseplus an array-of-strings check, and returns the argv rather than a boolean so callers don't parse twice.use-exec-formroutes its check through it.use-pipefail(from #86) carried two helpers of its own —isExecFormandjsonArrayEnablesPipefail— that were looser thanuse-exec-form's check, so the two rules could disagree about what a line even is. Both now go through the shared parser. That commit is a pure refactor:SHELLand exec-formRUNkeep their existing diagnostics.An array holding a non-string element (
CMD [1, 2]) returns null as well. Docker rejects it outright instead of falling back to shell form, so the Dockerfile is broken either way and a shell-form diagnostic still points at the offending line.This surfaces new warnings on Dockerfiles that previously passed. Quoting the elements (
CMD ["node", "index.js"]) resolves them, which is what the rule was always asking for.Related Issues
Closes #89
Checklist
use-exec-formpage already documents JSON-array syntax; only detection changedScreenshots (if applicable)
N/A — no UI surface.
Additional Notes
use-exec-formfix, theuse-pipefailrefactor, then the changeset. Each is independently checked out and green.parseExecFormhas direct unit coverage for every spelling above plus whitespace, empty arrays, non-array JSON, and unterminated input;rules.test.tscovers the three reproductions end to end through the rule.bun testfrom the repo root failsworkflow-scaffold > creates the workflow under the scanned root, not the cwd. Its assertion checks that.github/workflows/docker-doctor.ymlis absent fromprocess.cwd(), which the repo root legitimately owns.bun run test(what CI runs) passes because it runs per package. Reproduces on8105153, before this branch.