Skip to content

fix(core): correct four rule-specificity bugs in the Dockerfile engine#15

Merged
PunGrumpy merged 6 commits into
mainfrom
advisor/003-fix-rule-specificity-bugs
Jul 23, 2026
Merged

fix(core): correct four rule-specificity bugs in the Dockerfile engine#15
PunGrumpy merged 6 commits into
mainfrom
advisor/003-fix-rule-specificity-bugs

Conversation

@PunGrumpy

Copy link
Copy Markdown
Owner

Stacked on #14 (plan 002). Base is advisor/002-cli-verification-baseline so this PR's diff shows only plan 003's changes. Once #14 merges, GitHub auto-retargets this to main. Implements plan 003.

What

Fixes four confirmed rule-specificity bugs in the bundled @docker-doctor/core rule 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.

Commit Bug Fix
f51920e A no-root-user never reset per stage (false negative) Reset tracked user to root at each FROM; only the final stage's effective user counts. A multi-stage file with USER node in the builder but none in the runtime stage now correctly flags.
2db2b6b B no-secrets-in-env unanchored substring (false positive) Anchor keywords to _/-/string-boundary segments. ENV AUTHOR=… / OAUTH_ISSUER_URL no longer trip the only error-severity rule (which was failing CI); DB_PASSWORD still matches.
0b1cd1a C three rules read a flag as the source path (false negative) no-add-remote, prefer-copy-over-add, use-dockerignore now take the first non--- operand (the idiom already used by pinImageVersion/orderLayers). ADD --chown=… https://… now flags. use-dockerignore additionally skips COPY --from=<stage> (never reads the build context).
aa6706d D order-layers never reset per stage + substring-matched "src" (false positive) Reset per FROM; replace src.includes("src") with an exact normalized predicate. /usr/src/lib, mysrcdir, and correctly-ordered multi-stage builds no longer trip a −4 warning.
3d4d06b Changeset (@docker-doctor/cli patch).

Scope

  • packages/core/src/rules/{security,best-practices,performance}.ts — the four fixes.
  • packages/core/test/rules.test.ts7 new standalone regression test() 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.

pinImageVersion and image-size.ts deliberately untouched (plan 004 owns them); scoring untouched (plan 008).

Verification (re-run independently during review)

  • packages/core tests → 35 pass, 0 fail, 0 skip/todo.
  • Root bun run test → both packages green; plan 002's CLI suite (12/12) still passes, fixtures/clean still 0 error-severity.
  • bun run typecheck → 0; bun x ultracite check → 0.
  • grep "const [src] = parts" → 0 matches; grep 'includes("src")' → 0 matches.
  • Live end-to-end on the built source: a multi-stage-builder-only Dockerfile now emits no-root-user; ADD --chown=node:node https://… now emits no-add-remote; fixtures/clean emits no no-secrets-in-env and exits 0.

Note: bug B's regex uses non-capturing groups (?:…) rather than the plan's capturing-group example — required to satisfy the repo's prefer-named-capture-group lint; behavior is identical (the groups were never captured).

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

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.
@vercel

vercel Bot commented Jul 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docker-doctor Ready Ready Preview, Comment Jul 22, 2026 7:47pm

Base automatically changed from advisor/002-cli-verification-baseline to main July 23, 2026 07:39
@PunGrumpy
PunGrumpy merged commit 859f48c into main Jul 23, 2026
9 checks passed
@PunGrumpy
PunGrumpy deleted the advisor/003-fix-rule-specificity-bugs branch July 23, 2026 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant