Preserve identifier identity, pin the conformance catalog, and harden the release scripts - #27
Merged
Merged
Conversation
… branch
CI cloned github.com/AuthPlane/conformance at its default branch, so any change
to the catalog reached this repo immediately — a case added there could turn an
unrelated PR red here, with nothing in this repo having changed. Pinning
decouples them: a catalog change arrives only when this repo deliberately bumps
the pin together with the coverage for it.
The revision is single-sourced in a tracked .conformance-catalog-ref, guarded by
a ^[0-9a-f]{40}$ shape check so a branch or tag name cannot silently un-pin CI,
and read by both ci.yml and release.yml through one script rather than a
copy-pasted block — the pin was single-sourced before, the logic reading it was
not.
A weekly conformance-catalog-drift workflow clones the unpinned tip and runs the
alignment assertion, so new cases surface as an early warning instead of a
surprise. Its report step distinguishes real drift from a harness failure by
matching the two messages that mean drift rather than the marker prefix, which is
also emitted when the catalog cannot be read at all. The alignment step declares
shell: bash so the pipeline gets pipefail — without it the step takes tee's exit
status and reports success unconditionally.
The catalog now lands in $RUNNER_TEMP rather than the workspace, so it stays out
of `go list ./...`, coverage globs, and `git add -A` in the release commit.
No SDK source changes. Verified locally: the fetch script runs end-to-end against
the pinned revision, and the conformance suite passes against the catalog it
checks out — the repo already aligns with this revision, so the pin adopts no new
cases.
The pin is only useful if a contributor can reproduce CI. CONTRIBUTING and the conformance-tests README now say where the ref lives, how to check the catalog out at it, and that adopting new cases means bumping the ref in the same change. Also drops a clause the pin change made false: the release bot token is no longer used to check out the conformance catalog — the fetch script clones it directly, since the repo is public and needs no credential.
…e-only slash handling, escaped-path PRM Identifiers are identity, not something to normalise. The SDK stripped and reconciled them in several places, and every one of those was a silent rewrite of a value the operator configured. Identity is now preserved verbatim (RFC 8414/9728 §3.3) and slash removal happens only at derivation (§3.1) — the two concerns the old code conflated: - The token verifier stores the configured issuer byte-for-byte and compares a token's `iss` exactly (§4 spells the comparison out as code-point-for-code-point with no normalisation), so an AS whose issuer legitimately ends in `/` stops having every token rejected. - The RFC 8414 §3.3 metadata check compares both sides raw. Derivation is many-to-one, and the strict comparison is what turns an unavoidable collision into a clean discovery failure instead of a silent bind to another issuer's metadata. - PRM derivation reads the escaped path, so a percent-encoded octet survives instead of decoding to a delimiter and changing which resource is named. One rule, one place. `verifier.ValidateIssuer` is exported and every construction boundary calls it — `NewTokenVerifier`, `resource.New`, and `authplane.NewClient`, which needs its own call because a client used only for token, introspection and revocation never builds a verifier and would otherwise have no gate at all. The rejected identifier is redacted to scheme and host: that branch fires precisely for a credential-shaped query, and a construction error lands in a startup log. The `net/http` adapter's PRM discovery bypass compares escaped paths on both sides. Comparing the decoded path let `%2F` collapse to `/`, the two sides disagreed, and the discovery endpoint returned 401 — which RFC 9728 §3.2 requires to be publicly reachable. Breaking changes are enumerated in the changelog with migration notes. All are construction-time rejections of configurations that used to start, or output changes on two exported methods. Conformance rows added for the trailing-slash and escaped-path cases the catalog already carries.
… release scripts release.yml deletes release/vX.Y.Z once the tag is pushed, so the tag is the only ref that names those commits afterwards — which is what the release summary tells the operator to pass. --from only resolved branches, so the documented invocation failed at the point it was needed. The resolver now asks the remote what --from and --to name before fetching just those two refs, and validates both as ref names first: they are interpolated into fetch refspecs, and `git ls-remote` matches its arguments as globs, so an unvalidated `--from 'release/*'` fetched a wildcard expansion and left a ref name that is not a commit. `--flag=value` spellings and empty values are now usage errors instead of an exit 1 with no message or a silent fallback to a derived branch name. workflows-lint.yml gains scripts/*.sh: shellcheck plus the new backport-fixes.test.sh suite. A break in these scripts otherwise surfaces only when someone reaches for them right after a release, which is the worst moment to discover it.
muralx
approved these changes
Aug 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Everything queued for the next release, consolidated into a single branch so it reviews and lands as one unit. Supersedes the two narrower branches that carried parts of it.
Identifier identity (RFC 8414 §2/§3.3, RFC 9728 §3.1, RFC 8707 §2)
The configured issuer is an identity and is now preserved byte-for-byte. The trailing slash is stripped only where a document URL is derived; the stored issuer, the expected
iss, and the metadataissuercomparison all keep what the operator configured.ValidateIssuer(issuer string) erroris exported fromcore/resource/verifier, soNewTokenVerifier,resource.Newandauthplane.NewClientapply one implementation of the RFC 8414 §2 shape rule instead of a copy each. It rejects a query or fragment component and requires an absolute URL with a scheme and host.ErrInvalidIssueris a matchable sentinel — useerrors.Is.https://as.example.com?access_token=…), andnet/url.Errorprints its URL field unredacted, so the raw identifier reached whatever log the construction error landed in. Messages now carry scheme and host only; parse failures are still wrapped, soerrors.As(err, new(*url.Error))keeps working.net/httpadapter comparesr.URL.EscapedPath()against the escaped well-known path. Comparing the decoded path let a percent-encoded%2Fin the resource identifier collapse to/, the two sides disagreed, and the PRM discovery endpoint returned 401 even though RFC 9728 §3.2 requires it publicly reachable.Migration: pass the authorization server's issuer identifier exactly as published — absolute,
https, no query, no fragment. Construction that succeeded in 0.2.0 with a relative reference (/tenant) or a query component now fails withErrInvalidIssuer.Conformance catalog
The catalog is fetched at a pinned SHA instead of cloning its default branch, so a change there cannot silently alter what CI asserts here. A drift job reports when the pin falls behind, and the pinned ref is single-sourced in
.conformance-catalog-refand guarded as a 40-hex SHA.Release scripts
release.ymldeletesrelease/vX.Y.Zonce the tag is pushed, so afterwards the tag is the only ref naming those commits — which is what the release summary tells the operator to pass tobackport-fixes.sh --from. It only resolved branches, so the documented invocation failed at the point it was needed. The resolver now asks the remote what--fromand--toname before fetching just those two refs, and validates both as ref names first: they are interpolated into fetch refspecs andgit ls-remotematches its arguments as globs, so an unvalidated--from 'release/*'fetched a wildcard expansion and left a ref name that is not a commit.--flag=valuespellings and empty values are usage errors instead of an exit 1 with no message.workflows-lint.ymlnow coversscripts/*.shwith shellcheck and the newbackport-fixes.test.shsuite — a break in these surfaces only when someone reaches for them right after a release, which is the worst moment to discover it.