Skip to content

CI: surface risky Dockerfile content to reviewers, add Copilot review, fix S3 upload#625

Merged
ypriverol merged 2 commits into
masterfrom
ci/dockerfile-risk-scan
Jul 7, 2026
Merged

CI: surface risky Dockerfile content to reviewers, add Copilot review, fix S3 upload#625
ypriverol merged 2 commits into
masterfrom
ci/dockerfile-risk-scan

Conversation

@ypriverol

Copy link
Copy Markdown
Member

Why

Container PRs (e.g. #621) can add arbitrary RUN content — the classic case is a
remote script piped into a shell (curl … | install.julialang.org | sh). A reviewer
skimming 20 lines can miss it. This adds defense-in-depth detection so nothing
"strange" merges unreviewed, plus a bioconda-style local test path, and fixes the
Singularity → S3 upload that was silently failing.

What

1. Deterministic Dockerfile risk scan (validate.py) — zero new deps.
scan_dockerfile_risks() lifts the lines a human must consciously approve into the PR
comment as an advisory 🔍 Reviewer checklist:

  • curl … | sh / wget … | bash, ADD <url>, insecure http://, paste/URL-shortener/
    bare-IP hosts, chmod 777, --insecure/--no-check-certificate, non-biocontainers/*
    base image.
  • Blocking: high-confidence embedded credentials (AWS/GitHub/Slack tokens, private keys).
  • Backslash line-continuations are folded (a split curl \| sh can't evade); download
    rules are scoped to RUN/ADD/COPY so an http:// homepage in a LABEL is not a
    false positive. 12 new unit tests (30 total, all green).

2. Reviewer aids (pr-report.yml) — renders the checklist and a bioconda-style
"🧪 Build & test this container locally" block, pointing at the existing test-cmds.txt
convention. Untrusted report fields stay sanitized (same path as errors/warnings).

3. Copilot review layer.github/copilot-instructions.md +
.github/instructions/dockerfile.instructions.md (applyTo: **/Dockerfile) so automatic
Copilot code review hunts the same risks on every container PR. (Requires enabling
automatic Copilot review + a Copilot subscription — repo owner action.)

4. S3 upload fix (publish.yml) — the Ceph/RGW store rejects aws-cli v2.23+ default
CRC32 checksums on multipart uploads (InvalidRequest on CreateMultipartUpload), so
every .sif over ~8 MB failed to upload. Set AWS_REQUEST/RESPONSE_CHECKSUM_CALCULATION= when_required. Confirmed the endpoint supports multipart via boto3; end-to-end re-test
after merge.

On #621

The scanner flags line 19 (curl … | sh) in the reviewer checklist; Copilot would comment
on it independently. Nothing blocks — it spotlights, so the human signs off explicitly.

…S3 fix

Detection layers so a container PR never merges with unreviewed 'strange'
content (e.g. #621's `curl | sh`):

- validate.py: scan_dockerfile_risks() lifts the lines a human must sign off on
  into the PR comment as an advisory reviewer checklist (curl|sh, http://, ADD
  <url>, paste/shortener/bare-IP hosts, chmod 777, --insecure, odd base image).
  High-confidence embedded credentials (AWS/GitHub/Slack tokens, private keys)
  block the PR. Line-continuations are folded so a split `curl \<nl>| sh`
  cannot evade; download rules are scoped to RUN/ADD/COPY so an http:// homepage
  in a LABEL is not a false positive. 12 new unit tests.
- pr-report.yml: render the checklist + a bioconda-style 'build & test locally'
  block (and point at the existing test-cmds.txt convention).
- Copilot: .github/copilot-instructions.md + instructions/dockerfile.instructions.md
  so automatic Copilot review hunts the same risks on every container PR.
- publish.yml: fix Singularity S3 upload on Ceph/RGW (aws-cli v2.23+ default
  checksums broke multipart; set *_CHECKSUM to when_required).
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

BioContainers CI failed for container.

Errors (must fix):

  • Cannot modify GitHub CI files (.github/copilot-instructions.md).
  • Cannot modify GitHub CI files (.github/instructions/dockerfile.instructions.md).
  • Cannot modify GitHub CI files (.github/scripts/tests/test_validate.py).
  • Cannot modify GitHub CI files (.github/scripts/validate.py).
  • Cannot modify GitHub CI files (.github/workflows/pr-report.yml).
  • Cannot modify GitHub CI files (.github/workflows/publish.yml).

- Never echo a snippet from a line that also tripped a secret rule (no token
  leak into the public PR comment).
- Detect credentials in `ENV KEY value` (whitespace) form, not just `KEY=value`.
- Fold heredoc bodies (RUN <<EOF … EOF) into their instruction so a heredoc'd
  `curl | sh` is scanned under RUN scope.
- Base-image check: parse all FROM stages (not just the first), skip `--platform=`
  flags, ignore references to prior build stages, dedupe.
- pr-report: markdown-escape untrusted report strings (backticks + angle brackets)
  before rendering, so a fork-controlled report cannot inject formatting/HTML.
- 8 new regression tests (37 total, all green).
@ypriverol ypriverol requested a review from mboudet July 7, 2026 05:06
@ypriverol ypriverol merged commit ff56b43 into master Jul 7, 2026
0 of 2 checks passed
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