Skip to content

Add Trivy vulnerability scanning for Docker images - #854

Merged
vharseko merged 3 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/docker-trivy-scan
Aug 7, 2026
Merged

Add Trivy vulnerability scanning for Docker images#854
vharseko merged 3 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/docker-trivy-scan

Conversation

@vharseko

@vharseko vharseko commented Aug 6, 2026

Copy link
Copy Markdown
Member

Adds Docker image vulnerability scanning with Trivy:

build.yml — both build-docker and build-docker-alpine jobs now scan the freshly built image (resolved from the local Docker daemon, so the runner's linux/amd64 manifest only) right after the functional docker tests. Findings do not fail the build: the SARIF report is uploaded via codeql-action/upload-sarif, so PRs get a "Code scanning results / trivy-build-*" check like CodeQL and the full list lives in the Security tab. Only fixable CRITICAL/HIGH CVEs are reported (ignore-unfixed: true plus limit-severities-for-sarif: true — without the latter the severity filter is silently dropped for SARIF output), and only the vulnerability scanner runs (scanners: vuln). The action's built-in ~1GB DB cache is disabled (cache: false) so it cannot evict the m2-repository caches out of the repo's 10GB actions-cache quota. The two docker jobs get security-events: write; SARIF upload also works for fork PRs on public repositories.

docker-scan.yml (new) — weekly cron (30 5 * * 1) + workflow_dispatch scan of the published openidentityplatform/opendj:latest and :alpine images: new CVEs surface in already-released images (mostly via the base image) without any change in this repository. Unlike the build-time scan, unfixed CVEs are reported too — surfacing them in released images is the point. Reports are uploaded as SARIF to the Security tab with a separate category per tag (trivy-image-*, distinct from the trivy-build-* categories in build.yml). The scheduled run is skipped in forks; manual runs are always allowed.

aquasecurity/trivy-action is pinned by commit SHA (v0.36.0), matching the pinning style of the other third-party actions in build.yml. Future false positives / accepted findings can be suppressed via a .trivyignore file in the repository root or dismissed in the Security tab.

Scan the freshly built images (default and alpine) in build.yml for
fixable CRITICAL/HIGH CVEs, and add a weekly docker-scan workflow that
scans the published openidentityplatform/opendj:latest/:alpine images
and uploads SARIF reports to the Security tab.
@vharseko
vharseko requested a review from maximthomas August 6, 2026 15:32
@vharseko vharseko added security Security fixes / CodeQL code-scanning alerts CI docker labels Aug 6, 2026
@vharseko vharseko closed this Aug 7, 2026
@vharseko vharseko reopened this Aug 7, 2026
Switch the build.yml image scans from table output with exit-code 1 to
SARIF uploaded via codeql-action, so PRs get a "Code scanning results"
check like CodeQL instead of a failed docker job. Categories
trivy-build-default/trivy-build-alpine are distinct from the weekly
docker-scan.yml ones, which track the published images.

@maximthomas maximthomas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice direction — the repo had no image scanning at all, and this already caught 4 real HIGH findings (jackson-databind, mssql-jdbc) that now have fix branches. Switching CI to code scanning in 1ec4c496 was the right call.

Two things to fix before merge, both small. Reviewed at 1ec4c496.

severity: CRITICAL,HIGH is silently ignored for SARIF (major)

The pinned action unsets the filter whenever the format is SARIF, unless you opt back in — entrypoint.sh at ed142fd:

if [ "${TRIVY_FORMAT:-}" = "sarif" ]; then
  if [ "${INPUT_LIMIT_SEVERITIES_FOR_SARIF:-false,,}" != "true" ]; then
    echo "Building SARIF report with all severities"
    unset TRIVY_SEVERITY

So every UNKNOWN/LOW/MEDIUM finding from both base images and the ~130 bundled jars lands in the Security tab and on the PR's code-scanning check. This got wider in 1ec4c496: .github/workflows/build.yml moved from format: table (where severity works) to format: sarif (where it doesn't), so all four scan steps are now affected — .github/workflows/build.yml:473, :614, and .github/workflows/docker-scan.yml:41 (both matrix legs).

Add to all four steps:

          severity: CRITICAL,HIGH
          limit-severities-for-sarif: true

Note it must be exactly lowercase true${INPUT_LIMIT_SEVERITIES_FOR_SARIF:-false,,} above is a literal-string default, not bash lowercasing, so True/TRUE silently won't match.

Trivy's cache will evict the Maven caches (major)

trivy-action bundles actions/cache internally (cache input defaults to true):

    - if: ${{ inputs.cache == 'true' }}
      uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
      with:
        path: ${{ inputs.cache-dir }}
        key: cache-trivy-${{ steps.date.outputs.date }}

It stores trivy-db plus trivy-java-db — the java-db is unavoidable here, since many bundled jars are renamed and lack META-INF/maven/**/pom.properties, so SHA1→GAV lookup is required. That's roughly 1 GB saved per branch scope per day.

The repo is already at the ceiling:

$ gh api repos/OpenIdentityPlatform/OpenDJ/actions/cache/usage
active_caches_size_in_bytes: 10899282214   # 10.15 GiB, limit is 10 GB
active_caches_count: 31

The six largest entries are ~1.07 GiB Linux-m2-repository-* caches that the 9-job build-maven matrix depends on. New cache-trivy-* entries will LRU-evict them and Maven will re-download its tree.

This is currently masked: actions/cache saves with post-if: success(), and the Trivy step was failing, so the save was skipped. Removing exit-code in 1ec4c496 activates it. Simplest fix on all four steps:

          cache: false

Or adopt the action's scheduled master-branch cache-refresh pattern with TRIVY_SKIP_DB_UPDATE / TRIVY_SKIP_JAVA_DB_UPDATE.

TRIVY_INSECURE disables TLS globally (minor)

.github/workflows/build.yml:483 and :624:

        env:
          # the registry service on localhost:5000 is plain HTTP
          TRIVY_INSECURE: "true"

insecure is a global trivy flag, not per-target — it sets the process-wide HTTP transport's InsecureSkipVerify and is passed to RegistryOpts() on the vulnerability-DB fetch, so the trivy-db/trivy-java-db download stops verifying certificates.

It also isn't needed. go-containerregistry already returns http unconditionally for localhost:-prefixed names, and trivy resolves the image from the local Docker daemon anyway (--image-src defaults to docker,containerd,podman,remote, and the earlier docker run steps already pulled it) — the registry is never contacted. Safe to drop all three lines from both jobs.

Nits

  • Only linux/amd64 is scanned: because trivy resolves via the Docker daemon, the other published arches aren't covered. Notably opendj-packages/opendj-docker/Dockerfile-alpine ships a JDK 11 runtime for linux/386 (if [ "$TARGETARCH" = "386" ]) — the one arch that genuinely differs is the one skipped. Worth a comment noting the limitation.
  • The alpine scan is near-redundant: both images unpack the same opendj-*.zip, and the failing run produced identical jar findings with 0 OS findings on each side. It roughly doubles DB traffic and cache pressure for little added coverage.
  • ignore-unfixed: true in .github/workflows/docker-scan.yml:48: reasonable on a gate, but this workflow exists to surface CVEs in already-released images. Unpatched criticals are exactly what you'd want to see there in order to publish an advisory or pull a tag.
  • Upload steps lack if: always(): if Trivy errors (rate limit, DB fetch), no SARIF is uploaded and the category keeps showing the previous run's results. If you add it, pair with hashFiles() so a missing file doesn't turn into a second error.
  • scanners defaults to vuln,secret: harmless now that nothing gates on exit code, but secret findings will appear in the SARIF under a step named "Scan image for vulnerabilities". scanners: vuln also speeds up the scan.
  • Duplication: the Trivy + upload block is byte-identical across build-docker and build-docker-alpine apart from the tag suffix and category, as is the new image_repository export. Fine for now, but each of the fixes above has to be applied four times.
  • image_repository: ${GITHUB_REPOSITORY,,} is already inlined at nine sites in the same file, and steps.meta.outputs.tags already holds the exact ref.

Checked and fine, for the record: the ed142fd pin matches v0.36.0 exactly; upload-sarif@v4 exists and matches .github/workflows/codeql.yml; all four categories are distinct and don't collide; no cron conflict with CodeQL (27 3 * * 1 vs 30 5 * * 1); the image-ref does resolve to the freshly built image despite release_version naming; and fork PRs upload SARIF fine despite the read-only token — verified on this PR's own CodeQL run, which logged SecurityEvents: read and still uploaded successfully.

@github-advanced-security

Copy link
Copy Markdown

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

…Y_INSECURE

severity is silently unset for SARIF output unless
limit-severities-for-sarif: true is also passed; the action's built-in
~1GB DB cache would evict the m2-repository caches out of the repo's
10GB quota; TRIVY_INSECURE globally disables TLS verification (including
the DB download) and is unnecessary - the image is resolved from the
local Docker daemon. Also scan vulnerabilities only (no secret scanner),
report unfixed CVEs in the published-image scans, and guard the SARIF
uploads with always() + hashFiles.
@vharseko
vharseko requested a review from maximthomas August 7, 2026 12:18
@vharseko

vharseko commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Thanks for the thorough review — both majors and most of the nits are addressed in 290d6e4:

  • SARIF severity filter: limit-severities-for-sarif: true added to all scan steps (checked that the input exists in action.yaml at the pinned SHA).
  • Cache eviction: cache: false everywhere. The scheduled master-branch refresh pattern with TRIVY_SKIP_DB_UPDATE/TRIVY_SKIP_JAVA_DB_UPDATE felt like overkill for a weekly scan plus per-PR scans; easy to revisit if DB downloads start hitting rate limits.
  • TRIVY_INSECURE: dropped from both jobs — agreed it was both global and unnecessary.
  • Nits: scanners: vuln added everywhere; ignore-unfixed removed from docker-scan.yml (agreed — unpatched criticals in released images are exactly what it exists to surface); SARIF uploads guarded with if: always() && hashFiles(...); comment added noting only the runner's linux/amd64 manifest is scanned.

Left as-is for now:

  • The alpine scan — kept: with cache: false settled, its marginal cost is one extra DB download, and it still covers the alpine OS-package layer (incl. the linux/386 JDK 11 special case going stale differently). Happy to drop it if you prefer.
  • Duplication of the scan/upload block and image_repository — left alone to keep the diff focused; steps.meta.outputs.tags holds two refs in the alpine job, so the env var seemed the simplest way to a single-tag reference.

PR description updated to match. Great catch on the post-if: success() masking — that one would have been unpleasant to debug after merge.

@vharseko
vharseko merged commit 1a090f1 into OpenIdentityPlatform:master Aug 7, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI docker security Security fixes / CodeQL code-scanning alerts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants