Skip to content

feat(proxies): cache GitHub release artifacts and Packagist - #202

Merged
luthermonson merged 2 commits into
mainfrom
feat/ghrel-proxy
Sep 21, 2026
Merged

luthermonson merged 2 commits into
mainfrom
feat/ghrel-proxy

Conversation

@luthermonson

Copy link
Copy Markdown
Contributor

Two new CacheProxy implementations, plus a fix that makes ServeArtifact honour TTL.

Why these two

An inventory of the org’s workflows found the largest uncached ecosystems were not the ones with proxies:

ecosystem files had a cache?
cargo build 39 proxy existed, switched off — now enabled on the fleet
composer install 22 no
releases/download 19 no
dtolnay/rust-toolchain 11 via cargo proxy
go mod download 4 yes — the only one

Measured cost on a real build (php-sdk, x86_64 gnu, 2026-09-20): 40 source artifacts, 87s of a 591s build.

ghrel

GitHub releases are a distribution channel with no registry and no GOPROXY-style redirect — a release download is a plain github.com URL baked into whatever tool fetches it. Intercepting transparently would mean terminating TLS for job containers (a CA in every image, and a real change to what untrusted job code is exposed to).

So it advertises GHREL_PROXY and lets tools opt in. Unset, everything goes upstream exactly as today — which also keeps builds correct on GitHub-hosted runners and laptops.

Metadata is cached with a TTL and its asset URLs rewritten to the proxy’s /download/ route — otherwise a caller takes kilobytes of JSON from the LAN and every megabyte from the CDN.

composer

Deliberately rewritten on pkgcache rather than lifting the implementation from the stale feat/cargo-composer-cache branch. That version predates pkgcache entirely and would have reinvented the bounded LRU store, traversal-safe key mapping and fail-open fetch — and disk-full is a failure mode these nodes have hit twice.

It also fixes what that version documented about itself:

"distribution zips come from mirrors and GitHub, which this proxy does not front"

Metadata is the cheap part. Packagist dist URLs name an exact commit, so they are genuinely immutable — now fronted and cached permanently. The archive route re-checks the host allowlist, so a job cannot hand it an arbitrary URL and use ephemerd as an open proxy.

ServeArtifact now honours TTL

This is the part most worth reviewing.

ServeArtifact served any cached entry unconditionally — it never consulted req.TTL. That is correct for every existing caller: npm, PyPI and pub all refuse to re-publish a file, so their proxies pass Immutable and the question never arises.

GitHub does not. An asset can be deleted and re-uploaded under the same URL. Without this, ghrel’s non-immutable default was a no-op and the safe default was silently "cache forever" — the exact hazard it exists to avoid.

A stale, non-immutable entry is now revalidated with a conditional GET, reusing decide() — the same primitive Document already uses. Gated so Immutable callers are untouched; npm/pip/pub/cargo suites are unchanged.

Testing

go build, go vet, and go test ./pkg/proxies/... ./pkg/config/... ./cmd/... all pass.

Coverage: URL rewriting for both proxies (single-object and array release shapes), the composer open-proxy guard, cache-key safety, and both TTL branches.

Revert-verified — each of these was confirmed to fail when the behaviour is neutered:

  • revalidation off → conditional requests = 0, want 2
  • either rewrite made a no-op → its suite fails
  • composer allowlist check removed → a metadata-service URL returns 200 instead of 403

Notes for reviewers

  • Windows pools get none of this by design. Gateway proxies bind the bridge gateway, which Hyper-V job containers cannot route to; injecting the env var there hangs the build (mayfly already suppresses the others for this reason).
  • Four silent-failure bugs were caught during review and fixed before this landed: cache keys containing ? (rejected by SafeSegments, so paginated listings went upstream forever), nested API paths colliding file-vs-directory, a composer dist key embedding a host with a port, and advertiseBase() breaking on a port-0 listener. All failed silentlyDocument() logs a warning and serves the bytes — so the cache would have appeared to work while caching nothing.

Adds two CacheProxy implementations and makes ServeArtifact honour TTL.

WHY THESE TWO

An inventory of the fleet's workflows found the largest uncached ecosystems
were not the ones with proxies. Files per ecosystem across the org:

  cargo build            39   (proxy existed, was switched off -- now enabled)
  composer install       22   (no proxy on main)
  releases/download      19   (no proxy anywhere)
  dtolnay/rust-toolchain 11   (served by the cargo proxy)
  go mod download         4   (the one ecosystem that HAD a cache)

Measured cost on a real build (php-sdk, x86_64 gnu, ducks, 2026-09-20):
40 source artifacts took 87s of a 591s build.

GHREL

GitHub releases are a distribution channel with no registry, and no redirect
of the GOPROXY kind: a release download is a plain github.com URL baked into
whatever tool fetches it. Intercepting it transparently would mean terminating
TLS for job containers -- a CA in every image and a real change to what
untrusted job code is exposed to. So this advertises GHREL_PROXY instead and
lets a tool opt in; unset, everything goes upstream exactly as before.

Metadata is cached with a TTL; asset URLs inside it are rewritten to the
proxy's /download/ route, because otherwise a caller takes kilobytes of JSON
from the LAN and every megabyte from the CDN.

COMPOSER

Rewritten on pkgcache rather than lifting the implementation from the stale
feat/cargo-composer-cache branch: that version predates pkgcache entirely and
would have reinvented the bounded LRU store, traversal-safe key mapping and
fail-open fetch -- and disk-full is a failure mode these nodes have hit twice.

It also fixes what that version documented about itself: "distribution zips
come from mirrors and GitHub, which this proxy does not front". Metadata is
the cheap part. Packagist dist URLs name an exact commit, so they are
genuinely immutable and are now fronted and cached permanently. The archive
route re-checks the host allowlist so a job cannot hand it an arbitrary URL
and use ephemerd as an open proxy.

SERVEARTIFACT NOW HONOURS TTL

ServeArtifact served any cached entry unconditionally -- it never consulted
req.TTL. That is correct for every existing caller: npm, PyPI and pub all
refuse to re-publish a file, so their proxies pass Immutable and the question
never arises. GitHub does not: an asset can be deleted and re-uploaded under
the same URL. Without this, ghrel's non-immutable default was a no-op and the
safe default was silently "cache forever" -- the exact hazard it exists to
avoid.

A stale, non-immutable entry is now revalidated with a conditional GET, reusing
decide(), the same primitive Document already uses. Gated so Immutable callers
are untouched; npm/pip/pub/cargo suites unchanged.

Tests cover URL rewriting for both proxies (single-object AND array release
shapes), the composer open-proxy guard, cache-key safety, and both TTL
branches. Revert-verified: neutering revalidation gives "conditional requests
= 0, want 2"; neutering either rewrite fails its suite; removing the composer
allowlist check lets a metadata-service URL through with 200 instead of 403.
@ephpm

ephpm Bot commented Sep 21, 2026

Copy link
Copy Markdown

ePHPm Preview — removed

Preview deployment has been torn down.

@ephpm
ephpm Bot temporarily deployed to preview-pr-202 September 21, 2026 04:40 Inactive
@ephpm
ephpm Bot temporarily deployed to preview-pr-202 September 21, 2026 04:40 Inactive
@ephpm
ephpm Bot temporarily deployed to preview-pr-202 September 21, 2026 04:40 Inactive
CI failed with 23 bodyclose violations that go vet and go test do not catch --
golangci-lint runs that linter, mage lint does not.

The first attempt was to have get() close the body itself via t.Cleanup. That
does not work: bodyclose flags the CALL SITE of anything returning a response
it cannot see closed, so a helper that hands one back can never satisfy it
however careful the caller is.

So get() no longer returns *http.Response. It reads and closes the body and
returns a small result{code, data}, which is all these tests ever used. No
response escapes the helper, and body() became redundant and is deleted.

Verified with the linter CI actually runs (mage lint), not just go vet:
bodyclose 21 -> 0. The single remaining staticcheck hit is in
pkg/networking/network_windows_test.go, which this branch does not touch and
which Linux CI never builds -- it is why the original failure listed only
bodyclose.
@ephpm
ephpm Bot temporarily deployed to preview-pr-202 September 21, 2026 05:32 Inactive
@ephpm
ephpm Bot temporarily deployed to preview-pr-202 September 21, 2026 05:32 Inactive
@luthermonson
luthermonson merged commit d0d29e4 into main Sep 21, 2026
@luthermonson
luthermonson deleted the feat/ghrel-proxy branch September 21, 2026 05:34

This branch was successfully deployed

1 active deployment
preview-pr-202 46d2dd46 Deployed Sep 21, 2026 by ephpm[bot]
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