Conversation
…dule pins
zeroid could not be consumed. In a fresh module,
go get github.com/highflame-ai/zeroid@v1.9.3
fails with `unknown revision pkg/authjwt/v0.0.0`. In-repo builds never noticed,
because the local `replace` hides it — and every consumer we have (highflame-authn)
already pins pkg/authjwt explicitly, so nobody hit it.
TWO PINS, ONE ROOT CAUSE. Consumers ignore `replace` directives in dependency
modules, so the `require` lines are exactly what the proxy serves.
pkg/authjwt v0.0.0 -> v1.9.2 placeholder; unresolvable for consumers
pkg/dpop v1.6.2 -> v1.6.3 real tag, but stale: v1.6.3 carries the
replay-store namespacing fix from #347
Neither submodule's source has drifted from the tag now referenced, so this is a
pure pin correction with no release dance required.
WHY THE authjwt PLACEHOLDER WAS THERE, AND WHY IT STOPPED BEING SAFE. RELEASING.md
documented an asymmetry: pkg/dpop is imported by non-test code so it needs a real
tag, while pkg/authjwt was imported "only from tests/integration/" and Go does not
follow test imports across module boundaries — so its pin was "invisible to
downstream consumers and never needs to be a real version".
That was true when written. It stopped being true when private_key_jwt (#347)
added internal/service/client_jwks.go, which imports pkg/authjwt from non-test
code; server.go and internal/service/external_issuer_registry.go do too. Nothing
fails at the moment such an import is added. It surfaces only when an outside
consumer tries to resolve the module.
THE DRIFT GUARD WORKED AND STILL COULD NOT SAVE US, which is worth recording
because it changes what the guard is for. The v1.9.3 release run failed at exactly
`Verify pkg/dpop/ source matches the version in go.mod` — correctly, since dpop
source had drifted from the pinned v1.6.2. But for a Go module, PUSHING THE TAG IS
PUBLICATION; the workflow only runs afterwards. So v1.9.3 went out as a
half-release: the module tag is live and serving a broken go.mod, while
tag-submodules, goreleaser and the Docker build were all skipped. v1.9.3 has zero
release assets against v1.9.2's five, which is why pkg/authjwt/v1.9.3 does not
exist.
Changes:
- go.mod: both pins point at real published tags, with the history recorded
where the next person will be editing.
- release.yml: the guard now covers BOTH submodules instead of pkg/dpop alone,
and rejects a v0.0.0 placeholder outright with the consumer-facing symptom in
the error text. Its comment no longer claims pkg/authjwt is test-only.
Verified by running the guard logic against three states: the fixed tree
passes; reverting authjwt to v0.0.0 fails; reverting dpop to the stale v1.6.2
fails on drift (i.e. it reproduces the original bug).
- RELEASING.md: the asymmetry section is corrected rather than deleted, so the
reasoning that went stale is visible. The "test-only nested module" recipe no
longer recommends a v0.0.0 placeholder — a module's test-only status is not a
property anyone maintains.
FOLLOW-UP THIS DOES NOT DO: v1.9.3 remains half-published. A v1.9.4 cut after this
merges gets a clean release with both submodule tags and artifacts.
go build, go vet, gofmt clean; full unit + domain suites green under -race.
I mis-attributed the regression in the previous commit. `git log -S` over each importing file shows pkg/authjwt became a NON-test dependency on 2026-06-19 in #211 (direct OIDC IdP federation), which added internal/service/external_issuer_registry.go and the server.go option plumbing. internal/service/client_jwks.go joined later with private_key_jwt (#347), so #347 added an importer to an already-broken state rather than causing it. The consequence is materially larger than the previous commit claimed. #211 first shipped in v1.7.1, so EVERY release from v1.7.1 onward is unresolvable from the proxy, not just v1.9.3. Verified directly rather than inferred: a fresh module running `go get github.com/highflame-ai/zeroid@v1.7.1` fails with the same "unknown revision pkg/authjwt/v0.0.0". That also means roughly three months of releases were affected before anyone noticed — which is itself the argument for the guard this PR adds, since nothing in the repo could surface it and our only consumer pins pkg/authjwt explicitly. No code change: go.mod pins, the guard, and its behaviour are unchanged. This corrects the comments in go.mod and release.yml and the note in RELEASING.md.
Supersedes the pin-correction approach in this PR's earlier commits. Those made the pins real; this makes them incapable of going stale. WHY THE PREVIOUS SCHEME COULD NOT WORK, not just did not. pkg/dpop had a decoupled cadence, go.mod pinned whatever version it last got, and a drift guard in release.yml checked the pinned tag still matched pkg/dpop/ source. A release guard cannot gate a Go release. Pushing the tag IS publication; the workflow runs afterwards. v1.9.3 failed that guard CORRECTLY and shipped anyway: the module tag went live serving a go.mod that referenced a stale pkg/dpop, while tag-submodules, goreleaser and the Docker build were all skipped. v1.9.3 has zero release assets and no pkg/authjwt/v1.9.3 tag, and it cannot be un-published. The cadence also bought nothing. Consumers pinned mismatched versions and lagged by minors — cerberus authjwt v1.7.5, shield v1.8.1, authn v1.8.8 with dpop v1.6.1 — not deliberately, but because nothing coordinated them. What the independence did cost was the coordination step that left the #347 replay-store fix unpublished, and it is the same shape of assumption that left pkg/authjwt pinned v0.0.0 for three months of unresolvable releases. WHAT LOCKSTEP CHANGES. All three modules carry one version, tagged at one commit. go.mod names the version being released NEXT, and the nested tags are created at the commit whose go.mod names them — so "go.mod references a stale submodule" has no representable state. Drift is not detected; it is absent. go.mod both pins -> v1.9.4 (pkg/dpop makes a one-time forward jump from v1.6.3 to join the shared line) release.yml drift guard -> lockstep check: pins must EQUAL the release tag, and it runs BEFORE any nested tag is created. tag-submodules now tags pkg/dpop as well as pkg/authjwt. Makefile release-dpop -> release-prep VERSION=vX.Y.Z, which bumps the pins; release.yml refuses to release if it was skipped. release-dpop.yml deleted. Under lockstep an out-of-band nested release breaks the invariant, so leaving it would be a foot-gun. RELEASING.md rewritten: the flow, why lockstep replaced the drift guard, and the "adding a nested module" recipe no longer branches on whether a module is test-only — that property is not one anyone maintains, which is exactly how v0.0.0 survived #211. The lockstep check was verified by running it against three states: pins matching the release version pass; a release version ahead of the pins fails; one pin reverted to the old dpop numbering fails. Costs, stated plainly: version numbers advance without changes, and pkg/dpop jumps v1.6.3 -> v1.9.4. Both are cheap next to a release that cannot be un-published. go build, go vet, gofmt clean; unit + domain suites green under -race.
…tion library zeroid's non-test source imported pkg/authjwt — the token-VERIFICATION library resource servers use — purely to get its JWKS client. That inverts the layering: a consumer-facing API, with the compatibility obligations that implies, sitting in the authorization server's dependency graph. It had also leaked into zeroid's own exported surface as WithExternalIssuerJWKSOption(opt authjwt.JWKSOption). The dependency was not intentional. It arrived with #211 (direct OIDC IdP federation) because a remote-JWKS fetcher was needed and one already existed next door; #347 then added a second importer. RELEASING.md meanwhile still described pkg/authjwt as test-only and pinned it v0.0.0 on that basis, which is how every release from v1.7.1 became unresolvable from the proxy. WHAT MOVED, AND WHAT DID NOT. pkg/jwks is a new module holding the JWKS client — MOVED, not copied. Duplicating it was considered and rejected: it fetches the keys that decide who may authenticate, and two copies of that drifting apart is the same failure this PR already exists to fix. The file is byte-identical to the original after normalising the de-stuttering renames (JWKSClient -> Client, NewJWKSClient -> New); the only other delta is gofmt re-aligning two struct fields whose names got shorter. pkg/authjwt now re-exports the moved types as ALIASES (type JWKSClient = jwks.Client), so cerberus, shield, authn and observatory compile unchanged — its own suite passes untouched. Aliases rather than wrappers means the names are interchangeable at the type level and nothing needs keeping in sync. zeroid imports pkg/jwks directly and no longer imports pkg/authjwt from non-test code at all. tests/integration still does, correctly: it verifies tokens the way a consumer would, which is what that package is for. ENFORCED IN CODE, because prose already failed here. RELEASING.md stated this boundary correctly and it rotted anyway — nothing fails at the moment a source file adds the import. TestNonTestSourceDoesNotImportAuthjwt walks the tree and fails on any non-test import, with a floor on files scanned so a broken walk cannot read as success. Verified by mutation, and the mutation is the point: with the aliases in place, reintroducing the authjwt import COMPILES AND PASSES everything else. The ratchet is the only thing that catches it, which is exactly how it would happen for real. pkg/jwks joins the lockstep line — release.yml's check and tag loops, and make release-prep, all cover it. Verification: all four modules build, vet and pass golangci-lint v2.13.2 (0 issues); pkg/authjwt's suite green under -race through the aliases; zeroid's root, domain and internal suites green under -race. The integration suite could not be run to completion on this machine — it timed out at 600s and again at 1200s. That is environmental, not this change: on clean origin/main WITHOUT the extraction, the TestDelegation subset alone takes 320.9s, versus 322.9s with it (0.6%, noise), on a box whose load average is 3.25 with 13 unrelated containers. The full suite took 444s earlier today on the same tree. CI, which is not so encumbered, is the check that matters here.
The jwks extraction broke highflame-docker-check and, downstream of the same image, highflame-notebook-check. zeroid's go.mod replaces every nested module with ./pkg/<name>, and the Dockerfile copies each one's go.mod/go.sum before `go mod download` so the download resolves from disk. pkg/jwks was added to the replaces and not to the COPY list, so the replace target was absent from the build context and the download failed outright. There is no network fallback by design: lockstep pins name the version being released NEXT, so the proxy has not indexed those tags yet. Every nested module must be copied; this is not an optimisation. The same commit fixes the checklist that should have caught it. "Adding a new nested module" in RELEASING.md listed four places to update and the Dockerfile was not among them — I followed my own list and it was incomplete. It is now step 5, with the symptom spelled out, and the Dockerfile comment points back at the list. Verified by building the image locally rather than pushing and watching CI, which is what let this through: `docker build .` completes.
![highflame-oracle[bot]](https://avatars.githubusercontent.com/u/141064602?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔮 Oracle Review
🎯 Start Here
pkg/authjwt/go.mod — Security-sensitive file changed
Why this first: Pay close attention to security implications
📋 PR Summary
What this PR does: Corrects the root go.mod submodule pins so zeroid resolves from the Go proxy for external consumers — pkg/authjwt moves from an unresolvable v0.0.0 placeholder to v1.9.2, and pkg/dpop from a stale v1.6.2 to v1.6.3 — while hardening the release guard and fixing stale release documentation. The diff also goes beyond the described 'pure pin correction': pkg/jwks is added as a new workspace module, internal/service JWKS client imports are migrated from pkg/authjwt to pkg/jwks, and pkg/authjwt gains a backwards-compatible alias shim.
Key changes:
- Pin pkg/authjwt at real tag v1.9.2 (was unresolvable v0.0.0 placeholder) and pkg/dpop at v1.6.3 (was stale, missing the #347 replay-store namespacing fix)
- Extend the release.yml drift guard to cover both submodules and reject v0.0.0 placeholders outright, with the consumer-facing symptom in the error text
- Correct the RELEASING.md asymmetry section rather than deleting it, and stop recommending v0.0.0 placeholders for 'test-only' nested modules
- Add pkg/jwks as a third workspace module in go.work and migrate internal/service/client_jwks.go and external_issuer_registry.go to import it
- Add pkg/authjwt/jwks_alias.go, a declarations-only shim re-exporting the pkg/jwks client under pkg/authjwt's historical names
Areas affected: Module dependency management (go.mod, go.work), Release CI guard (release.yml), Release documentation (RELEASING.md), JWKS client import graph (pkg/authjwt, pkg/jwks, internal/service), Build tooling (Makefile)
Testing notes: Author reports go build/vet/gofmt clean and full unit + domain suites green under -race, with the guard verified by mutation across three trees (fixed passes; placeholder-reverted and stale-pin-reverted both fail as intended). Open verification items flagged in review: whether pkg/jwks has a resolvable tag/pin for external consumers (it is absent from the pin table, the release guard, and the v1.9.4 follow-up), whether the pinned pkg/authjwt/v1.9.2 tag actually contains the new jwks_alias.go, and whether pkg/authjwt's own go.mod carries a resolvable require pin for its new pkg/jwks dependency.
🔍 Code Review
The core fix is exemplary: the root-cause analysis traces the bug to the specific commit that broke the test-only invariant, the blast radius is verified empirically, and the hardened guard is proven by mutation testing. The main concern is the gap between the description ('pure pin correction, no source drift') and the actual diff, which introduces a new pkg/jwks nested module and rewires the JWKS import graph without extending the pin table, release guard, or v1.9.4 follow-up to cover it — leaving pkg/jwks exposed to the exact class of unresolvable-pin bug this PR exists to fix. Nothing here is blocking, but the pkg/jwks story should be resolved before or alongside the v1.9.4 release.
What's good:
- ✨ Outstanding root-cause forensics: identifying that #211 silently broke the documented test-only invariant for pkg/authjwt, and verifying the three-month blast radius with an actual 'go get' reproduction rather than inference
- ✨ Verifying the guard by mutation across three trees — proving both placeholder rejection and drift detection actually fire — is exactly how CI hardening should be validated
- ✨ Correcting the stale RELEASING.md reasoning instead of deleting it preserves the institutional memory of why the asymmetry existed and why it stopped being safe
- ✨ Embedding the consumer-facing symptom in the guard's error text makes the next occurrence self-diagnosing
- ✨ The declarations-only alias shim in pkg/authjwt is the right shape for a compatibility layer — no runtime code paths and exceptionally well documented
- ✨ Honest scoping: explicitly recording the v1.9.3 half-release and deferring a clean v1.9.4 rather than pretending the fix retroactively repairs it
Review Stats: 
Generated by Oracle - Highflame's AI Code Reviewer
| . | ||
| ./pkg/authjwt | ||
| ./pkg/dpop | ||
| ./pkg/jwks |
There was a problem hiding this comment.
pkg/jwks added to the workspace but absent from the guard/release scope described in this PR
This is the only functional change in the file, and it's the one part of the PR the description never mentions. The description scopes the fix to two submodules: the pin table lists only pkg/authjwt and pkg/dpop, the release.yml guard 'now covers both submodules instead of pkg/dpop alone', and the v1.9.4 follow-up promises 'both submodule tags and artifacts'. But this line promotes pkg/jwks to a third nested workspace module, and the comment at line 10 asserts each nested module has a require pin + replace in zeroid/go.mod — implying go.mod gained a pkg/jwks require/replace alongside this change.
That pin is now consumer-visible: consumers ignore replaces, so the require line for pkg/jwks is exactly what the proxy serves. Two things must hold before merge:
-
The pkg/jwks require pin points at a real published tag — not v0.0.0. If it's a placeholder or an unpublished tag, this PR reintroduces the exact bug it fixes, and none of the stated verification (go build / go vet / test suites green under -race) can catch it, because the workspace masks pins. That masking is the whole reason the original breakage went unnoticed for three months.
-
The release.yml guard includes pkg/jwks. A hardcoded two-module guard recreates the structural blind spot this PR documents: a submodule outside guard coverage can carry a placeholder or drift undetected until an outside consumer breaks.
If either doesn't hold, this should be a blocker. Please confirm both in the PR reply (or paste the nested-module require block from go.mod).
Suggested fix:
| ./pkg/jwks | |
| In release.yml, derive the module list from the filesystem so the next nested module is covered by default instead of hardcoding names: | |
| ```bash | |
| for mod in pkg/*/; do | |
| name="${mod%/}" # pkg/jwks | |
| path="github.com/highflame-ai/zeroid/${name#pkg/}" | |
| pin=$(go mod edit -json | jq -r --arg p "$path" '.Require[] | select(.Path==$p) | .Version') | |
| if [ -z "$pin" ] || [ "$pin" = "v0.0.0" ]; then | |
| echo "::error::$name pinned to '$pin' — downstream 'go get' fails with 'unknown revision'" | |
| exit 1 | |
| fi | |
| # ...existing source-vs-tag drift check for $name | |
| done |
*Related: `go.mod`, `.github/workflows/release.yml`, `pkg/jwks/go.mod`, `RELEASING.md`*
| // Publish in UI" — no version-bumping ceremony required because | ||
| // zeroid/go.mod pins pkg/dpop to a specific released version that | ||
| // stays static across most zeroid releases. | ||
| // Releases: see RELEASING.md. All modules release in LOCKSTEP at one |
There was a problem hiding this comment.
Lockstep claim is ahead of the state this PR lands in — mark the transition
'All modules release in LOCKSTEP at one version' is the right target process, but the go.mod landing in this same PR pins pkg/authjwt@v1.9.2 and pkg/dpop@v1.6.3 — deliberately not lockstep, per the 'pure pin correction, no release dance' rationale. That's fine, but this PR is itself a case study in how a true-when-written comment quietly goes stale (the RELEASING.md 'pkg/authjwt is test-only' note). One clause marking the transition would save the next reader from a 'which artifact is wrong, the comment or go.mod?' investigation.
Related: please confirm make release-prep actually tags all three nested modules now. If it was built for two, this comment is making the same class of promise the old 'just click Publish in UI' text did.
Suggested fix:
| // Releases: see RELEASING.md. All modules release in LOCKSTEP at one | |
| Append a transitional note after line 18, e.g.: | |
| // (Until the v1.9.4 release, the authjwt/dpop pins are one-off corrections, | |
| // not yet lockstep.) |
Related: go.mod, RELEASING.md, Makefile
|
|
||
| "github.com/highflame-ai/zeroid/domain" | ||
| "github.com/highflame-ai/zeroid/pkg/authjwt" | ||
| "github.com/highflame-ai/zeroid/pkg/jwks" |
There was a problem hiding this comment.
Migration to pkg/jwks is unaddressed by the PR description — confirm it resolves for external consumers
The PR description cites this file as a pkg/authjwt importer (added by #347) and lists exactly two pin corrections — pkg/authjwt → v1.9.2 and pkg/dpop → v1.6.3. This diff instead migrates the file to pkg/jwks, which the description never mentions. One question needs an explicit answer before merge: does pkg/jwks have its own go.mod?
If it's a nested module, it needs its own real tagged pin in the root go.mod. Consumers ignore replace directives in dependency modules — the central observation of this PR — so an unpinned pkg/jwks reproduces the exact unknown revision failure being fixed. The hardened release guard (extended to cover "both submodules") would not catch a third, and highflame-authn, which pins submodules explicitly, would need to resolve it on upgrade.
If it's a plain package in the root module, it resolves for free and this migration actually removes this file's dependency on the nested module — but the description should say so, and note that external_issuer_registry.go and server.go still import pkg/authjwt, which keeps that pin load-bearing.
Either way, note that the stated verification (go build/go vet/tests green) cannot distinguish the two cases: the local replace hides nested-module resolution, which is this PR's own finding. The only meaningful check is a fresh-module go get github.com/highflame-ai/zeroid@<tag>.
One security-relevant check while confirming: the SSRF guard for jwks_uri depends on server.go injecting the guarded HTTP client through these opts (see the preserved comment at lines 43–46). If pkg/jwks is a fresh implementation rather than a verbatim move of the authjwt client, confirm the HTTP-client option is actually honored — a silently dropped option would reopen registered jwks_uris as internal-network probes (link-local metadata endpoints, internal services), the exact vector that comment describes.
Suggested fix:
| "github.com/highflame-ai/zeroid/pkg/jwks" | |
| If pkg/jwks is a nested module: add `require github.com/highflame-ai/zeroid/pkg/jwks vX.Y.Z` (a real published tag, not v0.0.0) to go.mod and add it to the release.yml guard's submodule list alongside pkg/authjwt and pkg/dpop. If it's a root-module package: add a line to the PR description recording the migration and noting the authjwt pin remains required for external_issuer_registry.go and server.go. In both cases, verify with a fresh-module `go get github.com/highflame-ai/zeroid@<candidate-tag>` rather than in-repo builds. |
Related: go.mod, release.yml, RELEASING.md, internal/service/external_issuer_registry.go, server.go
| "github.com/highflame-ai/zeroid/domain" | ||
| "github.com/highflame-ai/zeroid/internal/attestation" | ||
| "github.com/highflame-ai/zeroid/pkg/authjwt" | ||
| "github.com/highflame-ai/zeroid/pkg/jwks" |
There was a problem hiding this comment.
Confirm pkg/jwks module status — it decides whether this PR actually fixes the bug
The PR description presents this as a pure pin correction (go.mod, release.yml, RELEASING.md) with the new guard covering 'both submodules' — but this hunk rewires the root module's import graph from the nested pkg/authjwt module to pkg/jwks, and the description never mentions it. That matters because one question determines whether the fix is complete: does pkg/jwks have its own go.mod?
- If yes, the root module now depends on a third nested module. Unless its
requirepin points at a real published tag and the release guard covers it, external consumers will hitunknown revisionall over again — while in-repo builds keep passing behind the localreplace, which is precisely the three-month blindness this PR documents. The guard as described covers only authjwt and dpop. - If no (plain package in the root module), this is actually the better long-term shape — one fewer non-test importer of the nested module — but then where did the client implementation come from? If it was copied out of
pkg/authjwt, we now maintain two copies of security-sensitive JWKS fetch/cache/refresh code that will drift: an SSRF-handling or refresh fix in one will not reach the other. If it is a thin alias over authjwt, the pin stays load-bearing and the indirection deserves a note.
Either way, please answer this in the PR description — the next reader of 'pure pin correction, no release dance needed' should not discover a moved import by accident.
Suggested fix:
| "github.com/highflame-ai/zeroid/pkg/jwks" | |
| If pkg/jwks is a nested module: pin it to a real published tag and add it to the release.yml drift guard alongside authjwt/dpop. If it is a root-module package: add a provenance note (moved verbatim from pkg/authjwt@<tag>, or new implementation) to the PR description and the package doc comment. |
Related: go.mod, release.yml, pkg/jwks/go.mod
| jwks.WithRefreshInterval(cfg.JWKSCacheTTL), | ||
| }, opts...) | ||
| client, err := authjwt.NewJWKSClient(cfg.JWKSURI, issuerOpts...) | ||
| client, err := jwks.New(cfg.JWKSURI, issuerOpts...) |
There was a problem hiding this comment.
Verify jwks.New preserves the authjwt.JWKSClient contract this file documents
The doc comment above (lines 34-43) encodes guarantees that were properties of the authjwt implementation, and the registry's security posture depends on them:
- Best-effort warm-up —
jwks.Newmust not fail on an unreachable JWKS (boot resilience), with failure deferred to a synchronousEnsureLoadedat first verification so an unreachable issuer fails closed rather than open. - Last-wins option application — the SSRF-guard ordering below is only correct if a later
WithHTTPClientoverrides an earlier one. A first-wins implementation in the new package would silently invert the documented override semantics. Closeidempotency and background-refresh teardown — the registry relies on both (the closers list, plusr.Close()on the partial-failure path at line 64).
If pkg/jwks is the authjwt code moved verbatim, a one-line provenance note in the PR makes this trivially reviewable. If it is new or reworked code, these three properties need explicit tests in the new package — (1) and (2) in particular are easy to regress silently and neither would necessarily fail an in-repo test suite.
Suggested fix:
| client, err := jwks.New(cfg.JWKSURI, issuerOpts...) | |
| Add tests in pkg/jwks covering: New() succeeding with an unreachable JWKS URI (assert warning + deferred failure via EnsureLoaded); a later WithHTTPClient overriding an earlier one; Close() being idempotent and stopping background refresh. |
Related: pkg/jwks
| package authjwt | ||
|
|
||
| import ( | ||
| "github.com/highflame-ai/zeroid/pkg/jwks" |
There was a problem hiding this comment.
New source file in pkg/authjwt vs. the 'pure pin correction' claim
The PR description enumerates three changes (go.mod pins, release.yml guard, RELEASING.md) and asserts 'neither submodule's source has drifted from the tag now referenced.' But this diff adds a brand-new source file to pkg/authjwt. If the tag being pinned — pkg/authjwt/v1.9.2 — does not contain this exact file, that claim does not hold: in-repo builds (masked by the local replace directive) would expose authjwt.JWKSClient / NewJWKSClient / the With* options, while proxy-resolved consumers of v1.9.2 get a package without them. That is precisely the in-repo-vs-proxy divergence this PR exists to eliminate, and the newly extended drift guard would fail the next release run. The mutation table in the description ('fixed → passes') suggests the author already verified no drift — if so, please state that explicitly and add this file to the PR's change list, because an undocumented public-API addition inside a release-mechanics PR is exactly what reviewers should not have to infer.
Suggested fix:
| "github.com/highflame-ai/zeroid/pkg/jwks" | |
| Verify with: git fetch --tags && git diff pkg/authjwt/v1.9.2 -- pkg/authjwt/ — this must be empty for the pin correction to be pure. If it is not empty, this file must ship in a tagged pkg/authjwt release (e.g., cut with v1.9.4) before go.mod can pin a version containing it. Either way, mention this file in the PR description's change list. |
Related: go.mod, release.yml
| "github.com/highflame-ai/zeroid/pkg/jwks" | ||
| ) | ||
|
|
||
| // The JWKS client moved to github.com/highflame-ai/zeroid/pkg/jwks so that both |
There was a problem hiding this comment.
This import creates a module-level cycle — consider a leaf submodule for pkg/jwks
The package-level reasoning in this comment is right: moving the client to pkg/jwks breaks the unwanted package dependency (the authorization server no longer drags a resource-server token-verification library into its graph). But at the module level the published graph is now cyclic: the root zeroid module requires pkg/authjwt (the very pin this PR corrects), and pkg/authjwt must require zeroid to resolve this import of pkg/jwks. Go tolerates module-level cycles as long as package imports stay acyclic — which the move achieved — but it turns every release into a sequencing exercise: an authjwt tag can only reference published root tags and vice versa. This PR's incident (a half-published v1.9.3 serving an unresolvable go.mod for three months) is exactly the failure mode this structure invites. A cleaner endgame: give pkg/jwks its own go.mod as a leaf submodule that depends on nothing else in zeroid. Root and pkg/authjwt would both require it, the cycle disappears, and the release guard would cover three independent tags instead of a mutually-referencing pair. Not a blocker for this PR, but worth a tracking issue.
Suggested fix:
| // The JWKS client moved to github.com/highflame-ai/zeroid/pkg/jwks so that both | |
| Tracking issue: extract pkg/jwks into a leaf submodule (own go.mod, no imports from the zeroid root), yielding root -> pkg/jwks <- pkg/authjwt with no cycle; the drift guard then validates three independent pins. |
Related: go.mod, RELEASING.md
| // New code should prefer the pkg/jwks names directly. These remain for | ||
| // compatibility and carry no deprecation date — removing them would break | ||
| // consumers for no benefit. | ||
| type ( |
There was a problem hiding this comment.
Verify the alias set covers the full pre-move authjwt JWKS API
The alias set here is {Client, Option, New, and four options}. If the pre-move pkg/authjwt exported anything else JWKS-related that external callers could reference — error sentinels used with errors.Is (e.g., a no-matching-key error), an exported interface, or additional option functions — those names silently vanish from the authjwt namespace, and cerberus/shield/authn break the next time they bump this pin. The stated goal is 'existing callers — cerberus, shield, authn — compile unchanged'; the compiler only catches the import sites that get rebuilt, so it is worth mechanically diffing the exported surface of the pre-move package against this file rather than relying on recall.
Suggested fix:
| type ( | |
| Mechanically diff the surface: git show <pre-move-tag>:pkg/authjwt/jwks.go (plus any sibling files) and confirm every exported JWKS-related name — including error values and interfaces — has an alias here. |
| WithRequestTimeout = jwks.WithRequestTimeout | ||
| // WithHTTPClient supplies the HTTP client used for JWKS fetches — the hook | ||
| // zeroid uses to force every fetch through its SSRF-guarded transport. | ||
| WithHTTPClient = jwks.WithHTTPClient |
There was a problem hiding this comment.
Surface the SSRF guidance at the compatibility surface
This alias file is the API that cerberus, shield, authn — and now any proxy-resolved consumer — will actually read, and JWKS URL fetching is a classic SSRF vector when the URL is issuer-derived rather than fixed, trusted configuration. The comment already identifies WithHTTPClient as the SSRF-guarded-transport hook; consider one extra line making the consumer-facing guidance explicit: callers resolving issuer-derived JWKS URLs should inject a guarded transport here, since the default http.Client performs no SSRF checks. Cheap insurance on a security-sensitive path, and consistent with our posture that JWT verification always goes through JWKS with no shortcuts.
Suggested fix:
| WithHTTPClient = jwks.WithHTTPClient | |
| // WithHTTPClient supplies the HTTP client used for JWKS fetches — the hook | |
| // zeroid uses to force every fetch through its SSRF-guarded transport. | |
| // Callers resolving issuer-derived JWKS URLs should inject a guarded | |
| // transport here; the default http.Client performs no SSRF checks. | |
| WithHTTPClient = jwks.WithHTTPClient |
3 participants
zeroid cannot currently be consumed. In a fresh module:
In-repo builds never notice, because the local
replacehides it — and our only consumer (highflame-authn) already pinspkg/authjwtexplicitly, so nobody hit it.Two pins, one root cause
Consumers ignore
replacedirectives in dependency modules, so therequirelines are exactly what the proxy serves.pkg/authjwtv0.0.0v1.9.2pkg/dpopv1.6.2v1.6.3Neither submodule's source has drifted from the tag now referenced, so this is a pure pin correction — no release dance needed.
Why the
authjwtplaceholder was there, and why it stopped being safeRELEASING.md documented an asymmetry:
pkg/dpopis imported by non-test code so it needs a real tag, whilepkg/authjwtwas imported "only fromtests/integration/" and Go doesn't follow test imports across module boundaries — so its pin was "invisible to downstream consumers and never needs to be a real version."That was true when written. It stopped being true on 2026-06-19, when #211 (direct OIDC IdP federation) added
internal/service/external_issuer_registry.goand theserver.gooption plumbing — both importing pkg/authjwt from non-test code.internal/service/client_jwks.gojoined later withprivate_key_jwt(#347), adding an importer to an already-broken state.Nothing fails at the moment such an import is added; it surfaces only when an outside consumer tries to resolve the module.
So the blast radius is every release from v1.7.1 onward — roughly three months — not just v1.9.3. Verified directly rather than inferred:
That nobody noticed for three months is itself the argument for the guard: nothing in the repo could surface it, and our only consumer (highflame-authn) pins
pkg/authjwtexplicitly.The drift guard worked, and still couldn't save us
Worth recording, because it changes what the guard is for.
The v1.9.3 release run failed at exactly
Verify pkg/dpop/ source matches the version in go.mod— correctly, since dpop source had drifted from the pinned v1.6.2. But for a Go module, pushing the tag is publication; the workflow only runs afterwards.So v1.9.3 went out as a half-release: the module tag is live and serving a broken
go.mod, whiletag-submodules,goreleaserand the Docker build were all skipped. v1.9.3 has 0 release assets against v1.9.2's 5 — which is whypkg/authjwt/v1.9.3doesn't exist.Changes
go.mod— both pins point at real published tags, with the history recorded where the next person will be editing.release.yml— the guard now covers both submodules instead ofpkg/dpopalone, and rejects av0.0.0placeholder outright with the consumer-facing symptom in the error text. Its comment no longer claimspkg/authjwtis test-only.RELEASING.md— the asymmetry section is corrected rather than deleted, so the reasoning that went stale stays visible. The "test-only nested module" recipe no longer recommends av0.0.0placeholder, since a module's test-only status is not a property anyone maintains.Guard verified by mutation
Ran the guard logic against three trees:
authjwtreverted tov0.0.0dpopreverted to stalev1.6.2Follow-up this does not do
v1.9.3 remains half-published. A v1.9.4 cut after this merges gets a clean release with both submodule tags and artifacts.
Verification
go build,go vet,gofmtclean; full unit + domain suites green under-race.🤖 Generated with Claude Code