Skip to content

ci: build and publish the image to ghcr on v* tags - #40

Open
sbp-bvanb wants to merge 6 commits into
mainfrom
build/ghcr-publish
Open

ci: build and publish the image to ghcr on v* tags#40
sbp-bvanb wants to merge 6 commits into
mainfrom
build/ghcr-publish

Conversation

@sbp-bvanb

@sbp-bvanb sbp-bvanb commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Adds .github/workflows/docker.yml, which builds the image and pushes it to
ghcr.io/schubergphilis/claude-docker using the house
schubergphilis/mcvs-docker-action.

This is the narrow version of #17. That PR set out to do the same thing but grew
a multi-stage Dockerfile split, a baked-in claude user at UID/GID 999, an
entrypoint privilege-drop rework and matching smoke/CI updates alongside it, and
main has since moved 20+ commits past its base. Only the workflow and the
scanner config it needs are here — Dockerfile, entrypoint.sh, ci.yml and
the README are untouched.

Two constraints of the action worth knowing before review

It only pushes on tag pushes. The push step is gated on
github.event_name == 'push' && contains(github.ref, 'refs/tags/'), so the
tags: ["v*"] filter is load-bearing. #17's workflow triggered on push: (all
branches) and pull_request_review, which means it would have built and scanned
but never actually published. This repo has no tags and no releases yet, so
nothing publishes until the first v* tag is pushed.

Tagging is not what you might expect. docker/metadata-action defaults to
type=ref only and the action forces flavor: latest=false, with no input to
change either. A v0.1.0 tag therefore publishes exactly
ghcr.io/schubergphilis/claude-docker:v0.1.0 — no latest, no 0.1, no semver
expansion. Consumers must pin the full v-prefixed tag.

amd64 only, because the action exposes no platforms input and pushes
whatever the runner built natively — schubergphilis/mcvs-docker-action#31, open
since 2024-02-16.

Why the scanner config looks the way it does

.dockleignore carries exactly the two checkpoints that can fail the build at
dockle's exit-level: warn:

  • CIS-DI-0001 (WARN) — no USER, by design. The entrypoint needs root to
    chown /root to the host UID before dropping via runuser.
  • DKL-DI-0005 (FATAL) — fires on the glab layer, which apt-get installs a
    local .deb and so never populates /var/lib/apt/lists. Note hadolint's
    DL3009 keys on apt-get update alone, which is why this same layer passes
    hadolint and fails dockle.

CIS-DI-0008 (setuid/setgid) and DKL-LI-0003 (/opt/tfenv/Dockerfile,
root/.npm) are INFO — they cannot gate at exit-level: warn, so they are
deliberately left visible rather than suppressed. #17 changed the Dockerfile for
both; that turns out not to have been necessary for the gate.

.grype.yaml is what makes publishing possible at all. The scan runs before
the push, and the action hardcodes severity-cutoff: high, fail-build: true
and only-fixed: false with no input to relax any of them — so without
suppression the job fails and the image never publishes. #17's only failing
check was exactly this.

The measured result on this branch: grype finds 531 matches across 1962
packages, 202 are ignored by the rules below, and 0 High/Critical remain.
Before the rules, the image scan reported 105 High/Critical across ~63 advisory
IDs. Two rules are scoped by package type:

type what it is why it can't be fixed here
go-module Go stdlib + vendored deps inside the prebuilt gh, glab, task binaries nothing here compiles Go; only upstream can rebuild
binary / python AWS CLI v2's bundled interpreter moves with pins/awscli.env, not the Dockerfile

The third rule is scoped by location, and is worth review attention because
it corrects an assumption I started with. After the type rules, 8 High/Critical
npm findings remained — brace-expansion@5.0.6, ip-address@10.2.0,
tar@7.5.16, undici@6.26.0 — which I expected to be fixable from pins/.
They are not:

  • @anthropic-ai/claude-code@2.1.233 and pnpm@11.22.0 both declare zero
    dependencies
    and ship as bundled artifacts, so nothing under pins/
    influences resolution. The claude-code tarball carries no node_modules.
  • All four, at exactly the flagged versions, are bundled inside npm 11.17.0
    — the npm that node 24.19.0 ships via the NodeSource deb.
  • No released Node clears the gate: 24.20.0 (npm 11.19.0) fixes
    brace-expansion and undici but still carries ip-address@10.2.0 (needs
    10.3.1) and tar@7.5.19 (one High needs 7.5.21).

Since the scan runs before the push, leaving that unresolved would mean the
workflow can never publish at all. So the exemption is scoped by location
(**/node_modules/npm/**) rather than by package name: it covers npm's
vendored tree at any prefix and does not match the globals installed beside
it, whose own dependencies stay enforced. Ignoring by name would instead have
masked those four packages everywhere in the image, including openspec's real
tree. The glob was verified against bmatcuk/doublestar v2, the matcher grype
actually uses.

deb findings and npm findings outside that bundle remain enforced.

Please do not add this to the ruleset

main currently requires Validate and Docker build (validate, no push).
This job should stay advisory: with only-fixed: false at a high cutoff, a
CVE disclosed against ubuntu/node/Go turns it red on a PR that never touched the
image, and the author can't fix it. The flip side is worth stating explicitly —
because the scan gates the push, a red grype run on a v* tag means the release
silently does not publish.

Verification status

All nine checks are green, including both required contexts. Inside the
Docker job every stage passed: hadolint (at the action's stricter style
threshold) -> build -> dockle -> dive -> grype source scan -> grype image scan.

  • dockle reports CIS-DI-0008 and DKL-LI-0003 as INFO. They do not gate, so
    they are left visible rather than suppressed.
  • dive passes on the single-stage image (lowestEfficiency,
    highestUserWastedPercent), so no .dive-ci is needed.
  • The login step runs and the push step reports skipped — correct, since
    this is not a tag push.

That last point is the limit of what this PR proves: publishing is not
exercised here.
After merge it needs a real v* tag, and the GHCR package
will likely be created private and need a one-time flip to public.

Follow-ups, deliberately not here

🤖 Generated with Claude Code

sbp-bvanb and others added 3 commits August 29, 2026 17:07
Adds a Docker workflow built on schubergphilis/mcvs-docker-action, which
wraps hadolint, dockle, dive and two grype scans around the build and
pushes to ghcr.io/schubergphilis/claude-docker.

Scope is the workflow and the scanner config it needs. The Dockerfile,
entrypoint.sh, ci.yml and README are untouched.

Two constraints of the action shape the result:

- It pushes only when `github.event_name == 'push'` and the ref is a tag,
  so the `tags: ["v*"]` filter is load-bearing. Branch and PR runs build
  and scan with the push step skipped. The repo has no tags yet, so
  nothing publishes until the first one.
- It exposes no `platforms` input and pushes what the runner built
  natively, so this is amd64 only — schubergphilis/mcvs-docker-action#31.

The scanner config carries the reasoning that took an earlier attempt at
this a dozen commits to find:

- .dockleignore suppresses exactly the two checkpoints that gate at
  dockle's `exit-level: warn`. CIS-DI-0001 (no USER) is by design — the
  entrypoint needs root to chown /root before dropping via runuser.
  DKL-DI-0005 is a false positive on the glab layer, which installs a
  local .deb and so never populates /var/lib/apt/lists. CIS-DI-0008 and
  DKL-LI-0003 are INFO, do not gate, and are deliberately left visible.
- dockle-accept-key covers CIS-DI-0010, which flags benign NAME=value
  tokens out of layer history one per line at a time.
- .grype.yaml is what makes publishing possible at all: the scan runs
  before the push, and the action hardcodes severity-cutoff=high,
  fail-build=true and only-fixed=false with no way to relax them. The
  last full image scan produced 105 High/Critical matches over ~63
  advisory IDs, 93 of them go-module — Go stdlib and vendored code inside
  the prebuilt gh, glab and task binaries, which only upstream can
  rebuild. The rules are scoped by package type rather than by CVE so
  they do not go stale weekly; npm and deb stay enforced because those
  are actionable from pins/.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two fixes from the first run of the new workflow on PR #40.

yamllint: mcvs-general-action lints with its own config, not yamllint's
defaults, and it enables quoted-strings with required=only-when-needed.
Drop the redundant double quotes on `tags: ["v*"]` and on the two
.grype.yaml reason values. `tags: [v*]` still parses to ["v*"], so the
trigger is unchanged, and `"on":` keeps its quotes because unquoted `on`
is a YAML truthy value.

dockle: CIS-DI-0010 failed on `--clear`, which comes from the base
image's own history — Canonical builds ubuntu with rockcraft/umoci — and
not from this Dockerfile at all. Rather than add one key per CI round
(the check reports only the first match per history line), the base
image config was read from the registry and every NAME=value token it
contributes enumerated: `--clear`, the four org.opencontainers.image.*
labels, and PATH, which was already listed. All five are now accepted.

The same enumeration over this Dockerfile found no gaps. `signed-by=`
and `arch=` look like gaps but are not: they occur only inside the
double-quoted `echo "deb [...]"` strings, which shlex emits as one word
whose name half contains a space, and dockle skips those.

Also confirmed by that run, and reflected in .dockleignore already:
hadolint passes at the action's stricter `style` threshold, and
CIS-DI-0008 and DKL-LI-0003 report as INFO so they never gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The image scan was the last failing stage. The type-scoped rules worked —
all 93 go-module and 5 binary/python findings cleared — leaving 8
High/Critical, all npm: brace-expansion@5.0.6, ip-address@10.2.0,
tar@7.5.16 and undici@6.26.0.

Those are not ours to fix, which the previous commit's premise ("npm
stays enforced because it is actionable from pins/") got wrong. Traced
to source against the registry:

- @anthropic-ai/claude-code@2.1.233 and pnpm@11.22.0 both declare zero
  dependencies and ship as bundled artifacts, so nothing under pins/
  influences resolution. The claude-code tarball carries no node_modules
  at all.
- All four packages, at exactly the flagged versions, are bundled inside
  npm 11.17.0 — the npm that node 24.19.0 ships via the NodeSource deb.

No released Node clears the gate either: 24.20.0 (npm 11.19.0) fixes
brace-expansion and undici but still carries ip-address@10.2.0 (needs
10.3.1) and tar@7.5.19 (one High needs 7.5.21). Since the scan runs
before the push, leaving this unresolved means the workflow can never
publish at all.

The exemption is scoped by location rather than by package name so it
stays honest: `**/node_modules/npm/**` covers npm's vendored tree at any
prefix and does not match the globals installed beside it, whose own
dependencies stay enforced. Verified against bmatcuk/doublestar v2, the
matcher grype uses. Ignoring by name would instead have masked these
four packages everywhere in the image, including openspec's real tree.

deb findings and npm findings outside the bundle remain enforced.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sbp-bvanb
sbp-bvanb marked this pull request as ready for review August 29, 2026 17:34
sbp-bvanb and others added 3 commits August 29, 2026 17:38
Comments-to-code was 66/88/70 percent across the three files. Keeps the
rationale a maintainer needs — why the tags filter is load-bearing, why
setup-buildx must not be added, why each suppression exists — and drops
the derivation behind it, which lives in the PR and the git history.

No functional change: docker.yml parses identically, .dockleignore's
codes are unchanged, and only grype's free-text reason strings differ.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The blanket type: go-module rule also matched stdlib from the Go
toolchain this repo installs at /usr/local/go via ARG GO_VERSION — a
stdlib CVE there is fixable here by bumping one line, so silencing it
contradicted the rule's own reason. Scoped to /usr/bin, which covers the
prebuilt gh, glab and task binaries and nothing else. go1.26.6 currently
has no reported vulnerabilities, so nothing new is expected to surface.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ten of the 36 entries can never match: CLAUDE_CODE_VERSION,
OPENSPEC_VERSION, PNPM_VERSION, TFENV_URL and TFENV_SHA256 are sourced
from pins/*.env inside RUN and so never appear literally in layer
history; HOME, curl and gh have no NAME=value occurrence; libcrypto3>
and libssl3> are Alpine artifacts carried over from the action's README
example and reference packages this Ubuntu image does not have.

Remaining 26 are the exhaustive union of the base image's history tokens
and this Dockerfile's. The comment now records why the list is
load-bearing: the action passes an empty sensitive-word, which leaves
dockle's CIS-DI-0010 regex ending in an .* alternative, so every
NAME=value token in history is FATAL unless accepted here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sbp-bvanb
sbp-bvanb requested a review from dtump August 29, 2026 18:43

@dtump dtump 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.

Reviewed the CI/CD publishing workflow, scanner config, and the PR description's claims against the action's actual source (schubergphilis/mcvs-docker-action@v0.11.6). All 10 checks are green and the grype/dockle reasoning holds up against source — nice forensic work tracing the npm-bundle exemption and the dockle accept-key enumeration back to actual layer history. Three things should land before merge.

1. amd64-only publish, without exploring the arm64 path

The Dockerfile itself is fully multi-arch — every download (glab, AWS CLI, uv, Go) already branches on dpkg --print-architecture/uname -m with per-arch pins. But the published GHCR image will be amd64-only, because mcvs-docker-action builds natively on whatever runner it's given and exposes no platforms input (schubergphilis/mcvs-docker-action#31).

The PR only considers docker/setup-buildx-action as a fix and rejects it (correctly) because it'd break the action's assumption that the default docker driver auto-loads the image for the dockle/dive/grype steps. What it doesn't discuss is the more common workaround for this exact class of problem: a matrix over [ubuntu-24.04, ubuntu-24.04-arm] (GitHub's free hosted arm64 runners, GA since 2025), each leg running the action natively to build/scan/push its own arch-suffixed tag, followed by one docker buildx imagetools create step to weld the two into a single multi-arch manifest. That gets real arm64 support without needing the action itself to grow a platforms input.

Shipping amd64-only isn't a correctness bug today (there are no tags yet), but it means the feature is unusable for arm64 Docker Desktop / Colima users the moment someone cuts a v* tag — without any warning that the image will run emulated.

Ask: either implement multi-arch (matrix + manifest-list), or make the "amd64-only for v1" scope call explicit and reviewed rather than something that fell out of reaching for a single action.

2. Missing OpenSpec change

CONTRIBUTING.md requires drafting the OpenSpec change before the code for anything that's a new feature or behaviour change, and this repo's own history backs that up — even 2026-08-29-export-go-env, a two-ENV-line addition, went through proposal.md/design.md/tasks.md. Publishing a new distribution channel (GHCR) is a bigger feature than that, and skipping the proposal here is exactly why finding 1 can't be answered: there's no artifact recording whether amd64-only was a deliberate, reviewed tradeoff or just what fell out of the first action tried.

README is also untouched, so once this publishes there is nowhere documenting docker pull ghcr.io/schubergphilis/claude-docker:vX.Y.Z as a supported install path.

Ask: draft openspec/changes/* for this capability — recording the amd64/arm64 decision explicitly — and add the README section before merge.

3. Duplicate, uncached Docker build on every PR

docker.yml triggers on pull_request: branches: [main] (lines 11-12), running mcvs-docker-action's full build+hadolint+dockle+dive+grype pipeline on every PR. I checked the action's source: its docker/build-push-action call sets no cache-from/cache-to, so this is a from-scratch rebuild every time — unlike ci.yml's own Docker build (validate, no push) job, which already builds+smoke-tests every PR using cache-from/cache-to: type=gha.

The two jobs check different things (lint/scan vs. smoke test) so this isn't pure duplication, but the cost is real and growing: on this PR's own checks, the cached build took 1m10s while the uncached one took 3m0s/4m6s — roughly 3-4x slower, and that gap only widens as the Dockerfile grows.

Ask: drop the pull_request trigger (ci.yml already covers "does the image build" on every PR) or scope this workflow to push/workflow_dispatch, and rely on the tag-triggered run for the scan.

Strengths

  • The .grype.yaml/.dockleignore reasoning is unusually well-sourced — the npm-bundle exemption is scoped by location rather than package name specifically so it can't mask unrelated vulnerable packages elsewhere in the image, and that scoping was verified against the actual doublestar matcher grype uses.
  • The go-module ignore rule is correctly scoped to /usr/bin/** so it doesn't accidentally swallow a future CVE in the Go toolchain this repo pins itself and controls.
  • Commit history shows real iteration against live CI feedback (yamllint config, dockle base-image tokens, the go-module scope correction) rather than a single guess-and-check.

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.

2 participants