Skip to content

fix(gha): pull jsii-terraform CI image from GHCR instead of Docker Hub - #261

Merged
jsteinich merged 2 commits into
mainfrom
fix/ghcr-ci-image
Jun 11, 2026
Merged

jsteinich merged 2 commits into
mainfrom
fix/ghcr-ci-image

Conversation

@so0k

@so0k so0k commented Jun 10, 2026 •

Copy link
Copy Markdown
Contributor

Refs #206 (GHCR-for-CI half; OCF Public ECR for the devcontainer is a follow-up).

Why

Docker Hub's anonymous pull throttling on GitHub's shared runner IP pool is causing intermittent container-init failures across CI (see the audit in #206). All ~27 container: references across 12 workflows pull terraconstructs/jsii-terraform anonymously; GHCR public-package pulls from Actions runners are free, unmetered, and unauthenticated.

What

  • docker.yml: also pushes to ghcr.io/open-constructs/jsii-terraform (packages: write + docker/login-action against ghcr.io with GITHUB_TOKEN). Docker Hub publishing stays until consumers have moved.
  • Dockerfile: adds org.opencontainers.image.source label so future pushes auto-link the GHCR package to this repo (plus description/licenses labels).
  • 12 consumer workflows: container.image repointed to ghcr.io/open-constructs/jsii-terraform@sha256:c46ac91… — the same digest as today, because the image is mirrored registry-to-registry (digest-preserving). No rebuild needed for this PR's CI to pass.

Prerequisites before marking ready (one-time, maintainer)

  • Mirror the current image to GHCR (digest-preserving copy):
    echo $GHCR_TOKEN | docker login ghcr.io -u <user> --password-stdin   # PAT with write:packages
    docker buildx imagetools create \
      --tag ghcr.io/open-constructs/jsii-terraform:latest \
      docker.io/terraconstructs/jsii-terraform@sha256:c46ac91c8be20947a2b0e27a517eb4a5bfc015c16a27341876ed0b04d734ddf5
  • Set the new package public (org → Packages → jsii-terraform → settings → Change visibility). New GHCR packages default to private; anonymous CI pulls get 403 until this is done.
  • Manually link the package to this repo in package settings (the mirrored manifest predates the source label; the label takes over on the next docker.yml push to main).
  • Re-run CI on this PR to confirm GHCR pulls resolve, then mark ready.

Follow-ups (tracked in #206)

  • Mirror to OCF Public ECR for devcontainer/contributor distribution; update .devcontainer.json + CONTRIBUTING.md.
  • Retire Docker Hub publishing and the DOCKERHUB_* secrets once consumers have moved.

🤖 Generated with Claude Code

Docker Hub anonymous pull throttling on GitHub's shared runner IP pool
causes intermittent container-init failures across CI jobs. Publish the
image to ghcr.io/open-constructs/jsii-terraform (public-package pulls
from Actions runners are free and unauthenticated) and repoint all
workflow container references to it.

The pinned digest is unchanged: the existing image is mirrored
registry-to-registry, which preserves the manifest digest. docker.yml
keeps pushing to Docker Hub until consumers have moved, and adds the
OCI source label so the GHCR package auto-links to this repository.

Refs #206

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@so0k

so0k commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author
image migrated the docker hub image to GHCR for this repo

@so0k
so0k marked this pull request as ready for review June 10, 2026 10:25
@so0k
so0k requested a review from a team as a code owner June 10, 2026 10:25
@sakul-learning

Copy link
Copy Markdown
Contributor

🔒 Cache poisoning risks in workflow changes

This PR is clean and correct in intent, but the docker.yml cache configuration (pre-existing) has two issues worth flagging:

1. Bare restore-keys prefix — cache poisoning vector

https://github.com/open-constructs/cdk-terrain/blob/fix/ghcr-ci-image/.github/workflows/docker.yml#L26-L28

key: ${{ runner.os }}-buildx-${{ hashFiles('/Dockerfile', '.terraform.versions.json') }}
restore-keys: |
  ${{ runner.os }}-buildx-

The restore-keys has no hash suffix — it matches ANY previous buildx cache regardless of Dockerfile or TF version changes. A stale/poisoned cache from a different Dockerfile version can be silently restored, injecting different layers into the build.

Fix: add a hash fallback:

restore-keys: |
  ${{ runner.os }}-buildx-${{ hashFiles('/Dockerfile', '.terraform.versions.json') }}
  ${{ runner.os }}-buildx-

This prefers the exact-hash match but falls back to any buildx cache as a last resort.

2. hashFiles('/Dockerfile') — absolute path may not resolve

The leading / in hashFiles('/Dockerfile', '.terraform.versions.json') is an absolute path. In GHA, hashFiles patterns are typically workspace-relative. If /Dockerfile resolves to nothing, the hash is constant (empty), making all builds share the same cache key — defeating cache isolation.

Fix: use hashFiles('Dockerfile', '.terraform.versions.json') or hashFiles('**/Dockerfile', '.terraform.versions.json').


These are pre-existing, not introduced by this PR, but since this PR touches docker.yml it's a good moment to address them.

…sh workflow

zizmor flags the docker publish workflow with a high-severity
cache-poisoning finding: it restored a GitHub Actions layer cache
(actions/cache + buildx type=local) and then published the resulting
image, so a cache entry poisoned from a less-privileged context could
be baked into the published image. The pre-existing cache key also used
an absolute hashFiles('/Dockerfile') path that never resolves, plus a
bare restore-keys prefix that matched any prior buildx cache — so the
cache provided little isolation while carrying the risk.

Switch to a registry-backed buildx cache at
ghcr.io/open-constructs/jsii-terraform:buildcache (mode=max). Writing
to it requires packages:write, which only this workflow's token has
(push to main / dispatch), so less-privileged contexts cannot poison
it — unlike the Actions cache. Layer caching is preserved without the
attack vector.

Also set persist-credentials: false on checkout (zizmor artipacked).

zizmor now reports no findings on this workflow.

Refs #206

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@so0k
so0k force-pushed the fix/ghcr-ci-image branch from 882a6d4 to 676fcbd Compare June 10, 2026 12:28
@so0k

so0k commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for flagging this — addressed in 676fcbd, though with a different fix than the suggested key tightening.

I ran zizmor to pin down the exact rule: it reports the caching as a high-severity cache-poisoning finding, and the trigger is the combination of a GitHub Actions cache feeding a workflow that publishes an image (docker/build-push-action with push: true). Tightening restore-keys or fixing the hashFiles('/Dockerfile') path wouldn't clear it: the Actions cache itself is writable from less-privileged contexts (any branch workflow in the repo can populate entries under the same scope), so however precise the key, a poisoned entry could be baked into the published image. That's why I moved off the GHA cache mechanism entirely rather than patching the key.

Resolution — registry-backed buildx cache:

cache-from: type=registry,ref=ghcr.io/open-constructs/jsii-terraform:buildcache
cache-to: type=registry,ref=ghcr.io/open-constructs/jsii-terraform:buildcache,mode=max
  • Writing to :buildcache requires packages: write, which only this workflow's GITHUB_TOKEN has (it runs on push-to-main / workflow_dispatch only; fork PR tokens are read-only and same-repo PRs never run docker.yml) — so no less-privileged context can poison it.
  • mode=max exports all intermediate layers, so caching is as good as before. Better, actually: your two observations were spot-on — the absolute hashFiles('/Dockerfile') never resolved and the bare restore-keys prefix matched any prior cache, so the old setup had the poisoning exposure without meaningful cache isolation.
  • First run after merge is a cold build that seeds :buildcache; the cache ref shows up alongside the image tags in the GHCR package, which is the standard pattern.

Also set persist-credentials: false on the checkout while in there (zizmor artipacked). zizmor now reports no findings on docker.yml.

@sakul-learning

Copy link
Copy Markdown
Contributor

Retracting the cache-poisoning concerns from my earlier comment — 676fcbd removes actions/cache entirely and replaces it with registry-backed Buildx cache (cache-from/cache-to: type=registry).

Re-verified at 676fcbd — this is the right approach:

  • ✅ actions/cache removed — no GHA-cache poisoning surface at all
  • ✅ Registry cache requires packages:write scope, same as the push step — no privilege escalation possible
  • ✅ persist-credentials: false on checkout prevents token leakage
  • ✅ cache-from: type=registry pulls cache layers from GHCR, cache-to: type=registry,mode=max pushes all layers (not just final image layers) for maximal reuse

First-run cold cache: the :buildcache tag won't exist in GHCR until the first push-to-main after merge. That first build does a full rebuild. Every subsequent run hits the registry cache — which is also available to any branch that has packages:read access to the GHCR package.

@jsteinich
jsteinich merged commit f19d34b into main Jun 11, 2026
258 checks passed
@jsteinich
jsteinich deleted the fix/ghcr-ci-image branch June 11, 2026 02:41
so0k pushed a commit that referenced this pull request Jul 3, 2026
🤖 Release PR — merge to cut a new release. Kept open and rebased
as commits land on `main`.
---


<details><summary>0.23.4</summary>

##
[0.23.4](v0.23.3...v0.23.4)
(2026-07-02)


### Features

* **cli:** validate Terraform CLI feature versions
([#237](#237))
([32d434c](32d434c))
* declared Terraform/OpenTofu target versions (validation foundation)
([#269](#269))
([0aa2343](0aa2343))
* **lib:** S3-backend - Add `useLockfile` field and update
`dynamodbTable` deprecated JSDoc
([#234](#234))
([32a408f](32a408f))
* **lib:** validate Fn usage against Terraform/OpenTofu versions
([#268](#268))
([cdf8115](cdf8115))


### Bug Fixes

* **cli:** don't mistake terraform error output for a missing-variable
prompt
([#267](#267))
([0ea946f](0ea946f))
* **cli:** release terraform state lock when interrupting diff/deploy
([#284](#284))
([de617db](de617db))
* **cli:** resolve prebuilt Java provider versions via repo1.maven.org
([#285](#285))
([d5af07d](d5af07d))
* **gha:** Adding CHANGELOG.md to prettier ignore list
([#258](#258))
([5160469](5160469))
* **gha:** pull jsii-terraform CI image from GHCR instead of Docker Hub
([#261](#261))
([f19d34b](f19d34b))
* **provider-generator:** copy only publishable files into the jsii
compile bundle
([#292](#292))
([562d4c7](562d4c7))
* **provider-generator:** match shorthand provider sources against
OpenTofu schema keys
([#293](#293))
([fda0103](fda0103))
* **release:** add Changelog title for correct release-please insertion
([#255](#255))
([004818a](004818a))
* **release:** drop devEngines.packageManager that broke npm publish
([ef508a6](ef508a6))
* **tools:** repair generate-function-bindings tool
([#273](#273))
([821e1cf](821e1cf))


### Miscellaneous Chores

* Add Gradle to Mise config
([#271](#271))
([1578270](1578270))
* add pnpm to mise
([#266](#266))
([48ab91f](48ab91f))
* add uv to mise
([#260](#260))
([163cf3d](163cf3d))
* **deps:** bump nrwl/nx-set-shas from
38457b511e60ee4e1da09255b658e480748a1af4 to
afb73a62d26e41464e9254689e1fd6122ee683c1
([#236](#236))
([b5ac018](b5ac018))
* **deps:** bump the github-actions-backward-compatible group across 1
directory with 7 updates
([#235](#235))
([de67d9f](de67d9f))
* **deps:** replace uuid dependency with node:crypto.randomUUID
([#291](#291))
([6d3657a](6d3657a))
* ESLint ignore test edge-provider-bindings
([#270](#270))
([ba407bb](ba407bb))
* Fix Verdaccio logging config
([#272](#272))
([aded64b](aded64b))
* **gha:** parallelize provider binding generation for documentation
examples
([#244](#244))
([0a9c955](0a9c955))
* Migrate package manager from yarn to pnpm
([#170](#170))
([d23da4e](d23da4e))
* migrate planning artifacts to cdktn-planning [skip ci]
([968c358](968c358))
* **release:** add release-please automation for versioning
([#243](#243))
([c759eec](c759eec))
* Remove dependency-pinner tool
([#250](#250))
([42c27d5](42c27d5))
* Remove unused dependencies
([#281](#281))
([c7de65e](c7de65e))
* **tests:** run verdaccio in-process
([#248](#248))
([c17677d](c17677d))
* Update JSII version constraint
([#245](#245))
([d89558f](d89558f))
* Update scheduled workflows to only run on open-constructs/cdk-terrain
([#256](#256))
([ba408cf](ba408cf))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: open-constructs-cdktn[bot] <291052431+open-constructs-cdktn[bot]@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants