feat: add hadolint for dockerfiles #4002
Conversation
Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com>
|
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:
WalkthroughAdds Hadolint config and a pre-commit Hadolint hook; updates Dependabot; switches many Dockerfiles to use corepack/pnpm and to install Python deps via a build requirements file; adjusts frontend packageManager version and cspell metadata; introduces fuzz schemathesis dependency. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.pre-commit-config.yaml (1)
3-3: Bumphadolint-pytov2.14.0The latest version is
v2.14.0(released Sep 22, 2025). The pinnedv2.12.0.2(released Apr 24, 2024) is ~17 months outdated, missing new rules like DL3062 and improvements to existing rules.♻️ Proposed update
- rev: v2.12.0.2 + rev: v2.14.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pre-commit-config.yaml at line 3, Update the pinned hadolint-py pre-commit hook version: replace the existing rev value "v2.12.0.2" with "v2.14.0" in the .pre-commit-config.yaml entry for hadolint-py so the project picks up DL3062 and other rule improvements; ensure the change only modifies the rev field for the hadolint-py hook and run pre-commit autoupdate or a quick local hook run to verify compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.hadolint.yaml:
- Around line 2-4: Remove the global ignores for DL3013, DL3016, and DL3018 from
.hadolint.yaml (leave DL3042 if desired) and instead add targeted inline
suppressions in the specific Dockerfiles: use comments like “# hadolint
ignore=DL3013” on the RUN line that executes pip (e.g., the RUN python -m pip
install ...), “# hadolint ignore=DL3018” on the RUN lines that use apk add, and
only add “# hadolint ignore=DL3016” where npm installs intentionally omit
pinning (the npm install -g pnpm@10.30.0 already pins so needs no suppression);
this keeps hadolint active globally while documenting and scoping any
intentional exceptions.
---
Nitpick comments:
In @.pre-commit-config.yaml:
- Line 3: Update the pinned hadolint-py pre-commit hook version: replace the
existing rev value "v2.12.0.2" with "v2.14.0" in the .pre-commit-config.yaml
entry for hadolint-py so the project picks up DL3062 and other rule
improvements; ensure the change only modifies the rev field for the hadolint-py
hook and run pre-commit autoupdate or a quick local hook run to verify
compatibility.
arkid15r
left a comment
There was a problem hiding this comment.
This might work, is there a way to make it more verbose for the error output? It fails but doesn't indicate why when I test it locally.
Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.pre-commit-config.yaml (1)
2-9:--format=ttyis redundant and produces ANSI codes in CI logs
ttyis hadolint's default output format, so the--format=ttyarg is a no-op. More importantly, the TTY format emits ANSI escape codes that clutter CI log output.hadolintsupports several formats via--format ARG— consider using--format=jsonor--format=checkstylefor CI-friendly output that can be parsed by tooling, or simply drop the arg to accept the default.The
--verboseflag similarly adds per-file scanning noise to CI runs; unless the extra output is actively useful, it can be omitted.🔧 Suggested cleanup
- id: hadolint args: - - --verbose - - --format=tty - --failure-threshold=error # fail only on errors, not warnings🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pre-commit-config.yaml around lines 2 - 9, Remove the CI-unfriendly hadolint arguments in the pre-commit hook: delete the --format=tty arg (it is redundant and emits ANSI codes) and remove --verbose to reduce noisy per-file output; either replace --format=tty with a CI-friendly format such as --format=json or --format=checkstyle if structured output is needed, or omit the --format arg entirely to use the default. Target the hadolint hook block (repo: https://github.com/AleksaC/hadolint-py and hook id: hadolint) and update the args list accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.pre-commit-config.yaml:
- Line 72: Revert the change that sets language: system for the
igorshubovych/markdownlint-cli pre-commit hook: remove the language: system line
so pre-commit can create and manage an isolated environment for the pinned rev:
v0.47.0, restoring hermetic behavior (locate the markdownlint hook entry
referencing rev: v0.47.0 / igorshubovych/markdownlint-cli and delete the
language: system key).
---
Nitpick comments:
In @.pre-commit-config.yaml:
- Around line 2-9: Remove the CI-unfriendly hadolint arguments in the pre-commit
hook: delete the --format=tty arg (it is redundant and emits ANSI codes) and
remove --verbose to reduce noisy per-file output; either replace --format=tty
with a CI-friendly format such as --format=json or --format=checkstyle if
structured output is needed, or omit the --format arg entirely to use the
default. Target the hadolint hook block (repo:
https://github.com/AleksaC/hadolint-py and hook id: hadolint) and update the
args list accordingly.
|
I'm not really sure if inline ignore is preferred for those warnings or global. Let me know and I will update accordingly. |
Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com>
There was a problem hiding this comment.
2 issues found across 20 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docker/backend/Dockerfile">
<violation number="1" location="docker/backend/Dockerfile:27">
P1: Dockerfile now copies requirements.build.txt, but the file does not exist in the repo. This will make the image build fail at the COPY step (before poetry install).</violation>
</file>
<file name="docker/backend/Dockerfile.fuzz">
<violation number="1" location="docker/backend/Dockerfile.fuzz:22">
P2: Poetry venv path is assumed to be /home/owasp/.venv, but this Dockerfile never sets POETRY_VIRTUALENVS_IN_PROJECT. If Poetry creates the venv in its cache (default), /home/owasp/.venv won’t exist and the COPY/ PATH will break the image.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
docker/frontend/Dockerfile.a11y.test (1)
14-17: Same corepack binary cache opportunity asDockerfile.unit.test.Consider adding
--mount=type=cache,id=corepack,target=/root/.cache/node/corepackto avoid re-downloading the pnpm binary on cold builds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/frontend/Dockerfile.a11y.test` around lines 14 - 17, The RUN layer that installs corepack/pnpm (the RUN --mount=type=cache,id=pnpm,target=/pnpm/store ... corepack enable && corepack install && pnpm install ...) should reuse a cache for the corepack binary; add an additional buildkit mount --mount=type=cache,id=corepack,target=/root/.cache/node/corepack to the same RUN instruction so the pnpm/corepack binary is cached across cold builds (use the same cache id convention as Dockerfile.unit.test to share behavior)..pre-commit-config.yaml (1)
2-9: Redundantname: hadolintoverride.The
name:key on line 8 re-declares the same name already defined in hadolint-py's.pre-commit-hooks.yaml, adding noise without effect.♻️ Remove redundant name override
- repo: https://github.com/AleksaC/hadolint-py rev: v2.14.0 hooks: - id: hadolint args: - --failure-threshold=warning - name: hadolint🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pre-commit-config.yaml around lines 2 - 9, Remove the redundant name override by deleting the "name: hadolint" entry under the hadolint hook; keep the repo: https://github.com/AleksaC/hadolint-py, rev: v2.14.0, hooks: and the hook with id: hadolint and args: --failure-threshold=warning intact so the hook relies on the canonical name from the hadolint-py .pre-commit-hooks.yaml.cspell/Dockerfile (1)
12-14: Same corepack binary cache opportunity applies here.As noted in
docker/frontend/Dockerfile.unit.test, adding--mount=type=cache,id=corepack,target=/root/.cache/node/corepackto this RUN block avoids re-downloading the pnpm binary on each cold build.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cspell/Dockerfile` around lines 12 - 14, The RUN step that enables corepack and installs pnpm should reuse a corepack cache to avoid re-downloading the pnpm binary; update the RUN command that currently calls "corepack enable && corepack install && pnpm install --frozen-lockfile --ignore-scripts" to include the corepack cache mount option (use --mount=type=cache,id=corepack,target=/root/.cache/node/corepack) so corepack enable/install reuses the cached binary across builds; keep the existing pnpm store cache mount (id=pnpm,target=/pnpm/store) and ensure both mounts are specified on the same RUN invocation.docker/frontend/Dockerfile.unit.test (1)
14-17: Consider caching the corepack binary store to avoid re-downloading pnpm on cold builds.The
--mount=type=cache,id=pnpm,target=/pnpm/storecaches pnpm's package store, butcorepack installdownloads the pnpm binary into its own cache (~/.cache/node/corepackby default). Without a mount for it, every cache-cold build triggers a fresh binary download.♻️ Proposed corepack cache mount (same pattern applies to all affected Dockerfiles)
RUN --mount=type=cache,id=pnpm,target=/pnpm/store \ + --mount=type=cache,id=corepack,target=/root/.cache/node/corepack \ corepack enable && corepack install && \ pnpm install --frozen-lockfile --ignore-scripts && \ chown node:node /app🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/frontend/Dockerfile.unit.test` around lines 14 - 17, The RUN step currently mounts pnpm's package store but not Corepack's binary cache, causing pnpm to be re-downloaded on cold builds; update the RUN that calls "corepack enable && corepack install" to add a cache mount for Corepack's binary cache (e.g., --mount=type=cache,id=corepack,target=/root/.cache/node/corepack or the appropriate user cache path) alongside the existing --mount=type=cache,id=pnpm,target=/pnpm/store so Corepack's download is persisted across builds.docker/frontend/Dockerfile.local (1)
50-51: Consider adding a corepack cache mount to avoid re-downloading pnpm on every cache miss.
RUN corepack enable && corepack installin the final stage has no--mount=type=cache, whereas the builder stage mounts the pnpm store. Whenpackage.jsonchanges (invalidating this layer), corepack will re-download the pnpm binary from the registry unconditionally.⚡ Proposed cache mount for corepack in the final stage
-RUN corepack enable && corepack install +RUN --mount=type=cache,id=corepack,target=/root/.cache/node/corepack \ + corepack enable && corepack install🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/frontend/Dockerfile.local` around lines 50 - 51, The RUN step in the final stage using "corepack enable && corepack install" re-downloads pnpm whenever the package.json layer is invalidated; update that RUN invocation in the final Dockerfile stage to use a buildkit cache mount for corepack (e.g., add a --mount=type=cache pointing to a pnpm/corepack cache) so the pnpm binary/store is persisted across builds, matching the builder stage’s pnpm store mount and preventing unconditional redownloads when package.json changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/backend/Dockerfile.fuzz`:
- Around line 5-9: The builder stage ENV block is missing
POETRY_VIRTUALENVS_IN_PROJECT=true causing poetry to create virtualenvs in the
cache instead of /home/owasp/.venv; add POETRY_VIRTUALENVS_IN_PROJECT=true to
the same ENV list that defines APK_CACHE_DIR, PIP_CACHE_DIR, POETRY_CACHE_DIR
and PYTHONUNBUFFERED so that the subsequent poetry install (referenced in the
Dockerfile builder stage) will create /home/owasp/.venv and the later COPY
--from=builder /home/owasp/.venv and PATH setup will reference the correct
directory.
---
Nitpick comments:
In @.pre-commit-config.yaml:
- Around line 2-9: Remove the redundant name override by deleting the "name:
hadolint" entry under the hadolint hook; keep the repo:
https://github.com/AleksaC/hadolint-py, rev: v2.14.0, hooks: and the hook with
id: hadolint and args: --failure-threshold=warning intact so the hook relies on
the canonical name from the hadolint-py .pre-commit-hooks.yaml.
In `@cspell/Dockerfile`:
- Around line 12-14: The RUN step that enables corepack and installs pnpm should
reuse a corepack cache to avoid re-downloading the pnpm binary; update the RUN
command that currently calls "corepack enable && corepack install && pnpm
install --frozen-lockfile --ignore-scripts" to include the corepack cache mount
option (use --mount=type=cache,id=corepack,target=/root/.cache/node/corepack) so
corepack enable/install reuses the cached binary across builds; keep the
existing pnpm store cache mount (id=pnpm,target=/pnpm/store) and ensure both
mounts are specified on the same RUN invocation.
In `@docker/frontend/Dockerfile.a11y.test`:
- Around line 14-17: The RUN layer that installs corepack/pnpm (the RUN
--mount=type=cache,id=pnpm,target=/pnpm/store ... corepack enable && corepack
install && pnpm install ...) should reuse a cache for the corepack binary; add
an additional buildkit mount
--mount=type=cache,id=corepack,target=/root/.cache/node/corepack to the same RUN
instruction so the pnpm/corepack binary is cached across cold builds (use the
same cache id convention as Dockerfile.unit.test to share behavior).
In `@docker/frontend/Dockerfile.local`:
- Around line 50-51: The RUN step in the final stage using "corepack enable &&
corepack install" re-downloads pnpm whenever the package.json layer is
invalidated; update that RUN invocation in the final Dockerfile stage to use a
buildkit cache mount for corepack (e.g., add a --mount=type=cache pointing to a
pnpm/corepack cache) so the pnpm binary/store is persisted across builds,
matching the builder stage’s pnpm store mount and preventing unconditional
redownloads when package.json changes.
In `@docker/frontend/Dockerfile.unit.test`:
- Around line 14-17: The RUN step currently mounts pnpm's package store but not
Corepack's binary cache, causing pnpm to be re-downloaded on cold builds; update
the RUN that calls "corepack enable && corepack install" to add a cache mount
for Corepack's binary cache (e.g.,
--mount=type=cache,id=corepack,target=/root/.cache/node/corepack or the
appropriate user cache path) alongside the existing
--mount=type=cache,id=pnpm,target=/pnpm/store so Corepack's download is
persisted across builds.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4002 +/- ##
=======================================
Coverage 99.79% 99.79%
=======================================
Files 518 518
Lines 15939 15939
Branches 2171 2129 -42
=======================================
Hits 15907 15907
Misses 3 3
Partials 29 29
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|



Proposed change
Resolves #3983
This pr adds hadolint to lint all Docker files; the linter is python wrapper, so we don't need to install any other dependencies in CI. Some other warnings are ignored by placing that in the hadolint YAML file. Locally, we can simply run pre-commit run --all-files to test it, and pre-commit already runs in CI.
Checklist
make check-testlocally: all warnings addressed, tests passed