Approval-link follow-ups: expired-link message, pending release on approval, canonical link host#18
Merged
Merged
Conversation
…proval, canonical link host Three related fixes around proposal tokens, prompted by the 2026-07-22 production incident where every overnight approval link 403'd with "The proposal token is invalid" (the tokens were merely expired — see trace db0322e6a6b1861fb22321941bf771e6): 1. accept() now distinguishes an expired token from a genuinely invalid one and says so, including the configured validity window and what to do next. The expiry probe parses the payload without signature verification and only ever selects the error message. 2. Approving a proposal now releases the SECOP-1101 duplicate-submit reservation (both key variants, since the variant isn't encoded in the token). Without this, raising APPROVAL_TIMEOUT would block re-requests of the same group for the full token lifetime after an approval. 3. New optional PUBLIC_BASE_URL setting + CanonicalLinkBuilder: approval links generated from requests on the app's *default* appspot host are rewritten to the canonical URL (pam.wavemm.net in prod). Versioned staging hosts (rev-…-dot-…appspot.com), custom domains, and localhost are never rewritten, so promote_traffic=false smoke tests keep versioned links. Full unit suite: 1070 tests, 0 failures (ITest* skipped, need GCP env). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Context
PAM approval proposal tokens expire after
APPROVAL_TIMEOUT(currently the 1h default). This means most cross-timezone approvals die on the 1h window, and the error message sends people bug-hunting instead of re-requesting.The wavemm-iam tf will raise
APPROVAL_TIMEOUTto 12h (companion PR there). This PR contains the app-side changes that make that safe and fix the two UX problems it exposed.Changes
Expired links say so —
AbstractProposalHandler.accept()now distinguishes an expired token from an invalid one: "This approval link has expired: links can only be approved within 12 hours of the request being made. Ask the requester to submit a new request." The expiry probe parses the JWT payload without signature verification and is only ever used to pick the error message — both paths still deny access, so a forgedexpgains nothing.Approval releases the SECOP-1101 pending reservation — previously the duplicate-submit marker was only released when propose failed, and otherwise lived until token expiry. With a 12h token TTL and 2h elevations, an approved request would have blocked same-day re-requests of the same group for the remaining ~10h.
Proposal.onCompleted()now releases both key variants (auto-selected + hand-picked; the variant isn't recoverable from the token — releases are idempotent and best-effort, Firestore TTL reaps stragglers).Canonical link host, staging-safe — new optional
PUBLIC_BASE_URLsetting +CanonicalLinkBuilder. Approval links inherit the host the requester was browsing, so a request made onwavemm-workspace-groups-pam.nw.r.appspot.comfans out appspot links to every reviewer even though prod ispam.wavemm.net. WithPUBLIC_BASE_URLset, links are rewritten to the canonical URL only when the request arrived on the default appspot host. Versioned hosts (rev-…-dot-…appspot.com) never match — they are the documented staging surface (promote_traffic=falsesmoke tests), and staging-generated links keep pointing at the staged version. Custom domains and localhost pass through unchanged. Unset = exact upstream behaviour.Also: restored the executable bit on
sources/mvnw(it was committed non-executable).Testing
sh mvnw test -DskipITs=true: 1070 tests, 0 failures (ITest* need a GCP test environment).onCompletedreleases both reservation variants then notifies;CanonicalLinkBuilderhost matrix (default/legacy appspot rewritten; versioned-dot-, custom domain, localhost passthrough; scheme forcing preserved);PUBLIC_BASE_URLparsing/validation.TokenObfuscatorcleanly and its signature verifies OK — expiry was the only failure, which this message change would have surfaced immediately.Rollout
Deploy this before or together with the wavemm-iam
APPROVAL_TIMEOUT=720bump (usualtargetsubmodule re-point). The env vars are inert until set: withoutPUBLIC_BASE_URLlinks behave exactly as today, and item 2 only ever releases markers.🤖 Generated with Claude Code