fix(core): correct four rule-specificity bugs in the Dockerfile engine#15
Merged
Conversation
Establishes a verification baseline for @docker-doctor/cli by spawning the built dist/cli.mjs against small Dockerfile fixtures and asserting on the three documented exit-code paths and the --json report contract.
The no-root-user rule tracked the last USER instruction across the entire Dockerfile without resetting at each FROM, so a USER set in an earlier build stage (e.g. the builder) silently satisfied the check for a later stage that never sets one. Reset the tracked user to "root" on every FROM so only the final stage's effective user is evaluated.
no-secrets-in-env matched keywords like /auth/ anywhere in the env key, so ENV AUTHOR=grumpy tripped the check even though "author" has no secret in it. Anchor each keyword to a SCREAMING_SNAKE_CASE segment boundary (_, -, or string start/end) so substring matches like AUTHOR and OAUTH_ISSUER_URL no longer false-positive, while genuine segments like DB_PASSWORD still match.
no-add-remote, prefer-copy-over-add, and use-dockerignore all read
parts[0] as the ADD/COPY source, so a leading flag like --chown or
--from silently disabled them (e.g. ADD --chown=node:node
https://... never tripped no-add-remote). Use the same
parts.find((p) => !p.startsWith("--")) idiom already used by
pin-image-version and order-layers.
As a side effect, use-dockerignore would now fire on stage-to-stage
COPY --from=<stage> . ./, which never reads the build context, so
that instruction shape is explicitly skipped.
order-layers never reset copyAllLine on FROM, so a copy-everything in an earlier build stage falsely implicated an install command in a later, correctly-ordered stage. It also matched any path containing the substring "src" (e.g. /usr/src/lib, mysrcdir), not just an actual src directory. Reset the tracker per stage and replace the substring check with an exact match against "." / "./" / "*" / "src" / "src/*".
Bundled packages/core ships inside @docker-doctor/cli, and these four diagnostic changes are user-visible, so they require a changeset per this repo's convention.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Jul 23, 2026
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
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.
What
Fixes four confirmed rule-specificity bugs in the bundled
@docker-doctor/corerule engine — cases where a rule read the wrong thing and so fired when it shouldn't, or stayed silent when it should. Each is its own atomic, independently-revertable commit.f51920eno-root-usernever reset per stage (false negative)rootat eachFROM; only the final stage's effective user counts. A multi-stage file withUSER nodein the builder but none in the runtime stage now correctly flags.2db2b6bno-secrets-in-envunanchored substring (false positive)_/-/string-boundary segments.ENV AUTHOR=…/OAUTH_ISSUER_URLno longer trip the only error-severity rule (which was failing CI);DB_PASSWORDstill matches.0b1cd1ano-add-remote,prefer-copy-over-add,use-dockerignorenow take the first non---operand (the idiom already used bypinImageVersion/orderLayers).ADD --chown=… https://…now flags.use-dockerignoreadditionally skipsCOPY --from=<stage>(never reads the build context).aa6706dorder-layersnever reset per stage + substring-matched"src"(false positive)FROM; replacesrc.includes("src")with an exact normalized predicate./usr/src/lib,mysrcdir, and correctly-ordered multi-stage builds no longer trip a −4 warning.3d4d06b@docker-doctor/clipatch).Scope
packages/core/src/rules/{security,best-practices,performance}.ts— the four fixes.packages/core/test/rules.test.ts— 7 new standalone regressiontest()blocks, purely additive (no existing assertion changed). Written test-first: all confirmed failing before the fixes, passing after..changeset/four-quiet-rules.md— patch, with a per-bug note that scores will move on existing Dockerfiles.pinImageVersionandimage-size.tsdeliberately untouched (plan 004 owns them); scoring untouched (plan 008).Verification (re-run independently during review)
packages/coretests → 35 pass, 0 fail, 0 skip/todo.bun run test→ both packages green; plan 002's CLI suite (12/12) still passes,fixtures/cleanstill 0 error-severity.bun run typecheck→ 0;bun x ultracite check→ 0.grep "const [src] = parts"→ 0 matches;grep 'includes("src")'→ 0 matches.no-root-user;ADD --chown=node:node https://…now emitsno-add-remote;fixtures/cleanemits nono-secrets-in-envand exits 0.Expect docker-doctor scores to move on existing Dockerfiles: A and C surface new diagnostics, B and D remove false positives.
🤖 Generated with Claude Code