Skip to content

fix: defects found by end-to-end testing of 2026.9.20 - #187

Merged
advaitpatel merged 4 commits into
mainfrom
fix/e2e-findings-2026-09-20
Sep 20, 2026
Merged

advaitpatel merged 4 commits into
mainfrom
fix/e2e-findings-2026-09-20

Conversation

@advaitpatel

Copy link
Copy Markdown
Collaborator

Six defects, each found by running the released artifact rather than reading it. The first shipped because nothing in CI or the test suite exercised entrypoint.sh with command-line arguments.

  • The container image ignored every command-line argument. entrypoint.sh built its args only from the Action's INPUT_* variables and never forwarded "$@", so docker run ghcr.io/owasp/docksec:latest --version and --help both failed with "Dockerfile path is required". Both forms now work and can be combined.

  • Fix commands ignored the EPSS priority just computed above them. The output printed a Fix Now / Fix Soon breakdown and then sorted commands by severity and package name, burying the exploited CVEs among hundreds of lower-priority entries. Ordering now leads with the priority tier, each row is labelled, and the CVE driving a package's tier is kept in the displayed IDs.

  • SARIF set security-severity to an empty string whenever a finding had no CVSS score - 14 of 25 rules on an nginx scan. GitHub Code Scanning ranks by that property, so those findings did not surface at the severity DockSec assigned. It now falls back to the severity band.

  • SARIF carried no EPSS data at all. Results now include epss, epssPercentile and priority.

  • Compose scans never pulled images, so on a clean machine both services of the insecure example reported as unscanned. Missing images are now pulled on demand, never under --offline, and switchable off with DOCKSEC_PULL_MISSING_IMAGES=false.

  • Coverage notes now state the Trivy and Hadolint versions used, so the image's pinned versions differing from a local install is visible rather than looking like a bug.

test_stage0_correctness's missing-image test relied on the runner not having nginx and postgres cached, so it passed on CI and failed on a developer machine. It now names images that cannot resolve.

Pull Request

Description

Summary of the changes and the related issue. Include motivation and context.

Closes #

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Code style update (formatting, renaming)
  • Code refactoring (no functional changes)
  • Performance improvement
  • Test update
  • Build / CI configuration
  • Security fix

How Has This Been Tested?

Describe the tests run to verify the changes:

  • Unit tests
  • Manual testing

Test Configuration:

  • Python version:
  • Operating System:
  • DockSec version:

Checklist

  • Code follows the style guidelines of this project
  • Self-review completed
  • Hard-to-understand areas are commented
  • Documentation updated where needed
  • No new warnings or errors introduced
  • Tests added that prove the fix or feature works
  • All existing tests pass
  • Dependent changes have been merged and published
  • Spelling checked

Screenshots (if applicable)

Related Issues / PRs

  • Relates to #
  • Depends on #

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

Six defects, each found by running the released artifact rather than
reading it. The first shipped because nothing in CI or the test suite
exercised entrypoint.sh with command-line arguments.

- The container image ignored every command-line argument. entrypoint.sh
  built its args only from the Action's INPUT_* variables and never
  forwarded "$@", so `docker run ghcr.io/owasp/docksec:latest --version`
  and --help both failed with "Dockerfile path is required". Both forms
  now work and can be combined.

- Fix commands ignored the EPSS priority just computed above them. The
  output printed a Fix Now / Fix Soon breakdown and then sorted commands
  by severity and package name, burying the exploited CVEs among
  hundreds of lower-priority entries. Ordering now leads with the
  priority tier, each row is labelled, and the CVE driving a package's
  tier is kept in the displayed IDs.

- SARIF set security-severity to an empty string whenever a finding had
  no CVSS score - 14 of 25 rules on an nginx scan. GitHub Code Scanning
  ranks by that property, so those findings did not surface at the
  severity DockSec assigned. It now falls back to the severity band.

- SARIF carried no EPSS data at all. Results now include epss,
  epssPercentile and priority.

- Compose scans never pulled images, so on a clean machine both services
  of the insecure example reported as unscanned. Missing images are now
  pulled on demand, never under --offline, and switchable off with
  DOCKSEC_PULL_MISSING_IMAGES=false.

- Coverage notes now state the Trivy and Hadolint versions used, so the
  image's pinned versions differing from a local install is visible
  rather than looking like a bug.

test_stage0_correctness's missing-image test relied on the runner not
having nginx and postgres cached, so it passed on CI and failed on a
developer machine. It now names images that cannot resolve.
@github-actions github-actions Bot added documentation Improvements or additions to documentation core Changes to core scanning logic reports Changes to report generation docker Changes to Docker/container assets tests Changes to the test suite labels Sep 20, 2026
@github-actions

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Comment thread tests/test_e2e_regressions.py Fixed
Comment thread tests/test_e2e_regressions.py Fixed
Comment thread tests/test_e2e_regressions.py Fixed
Advait Patel added 2 commits September 20, 2026 13:27
…the image

Both of these were exposed by compose scans now pulling images that are
not present locally. The hardened example's services previously failed to
scan, so the assertion over them was running against an empty result and
had never tested what it claimed.

Gate the hardened example on configuration findings rather than on every
CRITICAL/HIGH finding. "Hardened" is a statement about configuration, and
zero compose rules fire on that example - the 35 findings are real CVEs
in the pinned nginx:1.25.3-alpine and postgres:15.5-alpine base images.
Those are true positives, and failing on them would break this job every
time upstream publishes an advisory against a deliberately held pin. The
CVE count is printed so it stays visible. The score check goes with it:
the score is driven by those same CVEs.

The guard still discriminates - the hardened example reports 0 config
findings and the insecure one reports 6, so a rule that starts misfiring
on hardened input still fails CI.

Separately, assert that command-line arguments reach the CLI through the
image entrypoint. Every other step in the smoke workflow reaches DockSec
via `--entrypoint sh` or INPUT_*, which is why an entrypoint that dropped
"$@" shipped green. Verified locally against both entrypoints: the
assertion passes with the fix and fails without it.
@github-actions github-actions Bot added the ci Changes to CI/CD workflows label Sep 20, 2026
CodeQL flagged py/overly-permissive-file: the stub was created 0o755,
world-readable and world-executable. Only the test process ever runs it,
so 0o700 is sufficient.
@advaitpatel
advaitpatel merged commit 9fc0c82 into main Sep 20, 2026
13 checks passed
advaitpatel pushed a commit to dhruvatr/DockSec that referenced this pull request Sep 23, 2026
Both follow-ups from the OWASP#187 review.

The hardened compose example had been pinned to nginx:1.25.3-alpine and
postgres:15.5-alpine since June, carrying 35 CRITICAL/HIGH CVEs. Nothing
surfaced that because the example's images were silently failing to scan,
so the assertion over them was running against an empty result.

nginx:1.31.6-alpine is clean. The 22 findings that remain are all in the
Go stdlib compiled into the official postgres image and have no fix at
any tag - 18.6-alpine reports exactly the same 22 - so the 15 line stays,
which keeps the example about configuration rather than turning it into a
major version upgrade. Verified by scanning all three candidates.

CodeQL gains `language: actions`, so the workflow files are analyzed. It
was already expecting a /language:actions configuration and warned on
every pull request that it could not find one. Autobuild is skipped for
that language, which analyses YAML in place and has nothing to build.
advaitpatel pushed a commit to dhruvatr/DockSec that referenced this pull request Sep 23, 2026
The three strategy items that were still outstanding (1.5, 3.6, 3.7).

Golden-file tests cover the terminal summary, the --json payload and the
SARIF report. Each renders from a fixed findings set - no scanner, no
network, no clock - and compares against a stored file, so an unintended
change fails CI instead of reaching a user. Verified by reintroducing the
fix-command ordering regression from OWASP#187: two tests fail, and pass again
when it is reverted. Re-record with DOCKSEC_UPDATE_GOLDEN=1 and read the
diff.

Ten examples with documented expected findings, every count produced by
running the scan. Eight Dockerfiles across Node, Python, Java, Go and
BuildKit secret mounts, plus the existing two compose stacks, each
insecure file paired with a hardened one. Two carry a lesson beyond the
delta: the distroless Go image reports a HEALTHCHECK finding that is
correct to keep, and the BuildKit example passes a build secret without
tripping the secret rule that the ENV-based examples do.

Three case studies against official images - node:18 (2,200 findings, 9
worth acting on today, one at the 100th EPSS percentile), python:3.12-slim
(44 findings, none fixable) and nginx:1.31.6-alpine (clean, and what that
does not prove). Official images keep the numbers reproducible and name
no third party unfavourably.

PR comment mode ships as two workflows. Stage one runs in the untrusted
pull-request context with contents: read and no secrets, and emits an
artifact. Stage two runs on workflow_run in the base repository, reads
only that artifact, never checks out the PR's code, and posts the
comment. Everything the renderer prints is escaped, because a fork
controls the text of its own findings; the PR number is validated as an
integer before it reaches an API path. Injection cases are covered by
tests: pipes, HTML, Markdown links, newline-injected rows and backticks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Changes to CI/CD workflows core Changes to core scanning logic docker Changes to Docker/container assets documentation Improvements or additions to documentation reports Changes to report generation tests Changes to the test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants