Read the credential expiry from the token shape the minter emits - #289
Merged
Conversation
`storage.credential_expires_at` reported "no expiry" for every temporary credential, which is the most dangerous answer a monitoring field can give: staging and lab expire on 2026-08-24 and 2026-08-26, and the surface built to warn about exactly that reported them as permanent. `credential_expiry` decoded the session token as a bare JWT. The minter emits standard base64 of `jwt/<header>.<claims>.<signature>` (`.github/scripts/r2_temporary_credential.mjs:108`), so splitting the outer encoding on `.` finds no claims and yields `None` — indistinguishable from a permanent credential, which production legitimately uses. Unwrap the outer encoding first, falling back to a bare JWT so either shape decodes. The existing test passed the literal `"temporary-session"` as a session token and so exercised no decoding at all. The new tests build a token in the shape the minter actually produces, and pin that an unreadable token yields no expiry rather than a wrong one. Found by running `bin/hosted-diagnostics staging` against a live provider.
`check:architecture` caps `blob_store.rs` at 1000 lines and the new tests took it to 1005. Five lines over is exactly where trimming comments to fit is most tempting and least honest, so this extracts instead, as `lifecycle.rs` did for `diagnostics.rs` in beta.78. `r2_session_token` holds the decoding and the tests that establish the token's shape, which belong together: the shape is the entire defect. `blob_store` keeps a two-line delegation and one test covering the seam, including the case where no expiry is the truth rather than a failure to read. blob_store.rs is now 940 lines.
Merged
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.
What
storage.credential_expires_atin/internal/v1/diagnosticsreported no expiry for every temporary credential. Staging and lab reportedcredential does not expire; their credentials expire on 2026-08-24 and 2026-08-26.That is the most dangerous way this field could fail.
Nonealso means "permanent credential", which production legitimately uses, so the broken case is indistinguishable from the healthy one — and the field exists specifically to turn a known future outage into a warning.Why it failed
credential_expirytreated the session token as a bare JWT. The minter emits something else, at.github/scripts/r2_temporary_credential.mjs:108:Standard base64 of
jwt/<header>.<claims>.<signature>. Splitting that on.finds no second field, so the function returnedNoneevery time.The fix unwraps the outer encoding first and falls back to a bare JWT, so either shape decodes.
Why the test did not catch it
The existing test passed the literal string
"temporary-session"as a session token. It asserted the field round-tripped through config; it exercised no decoding whatsoever. The new tests build a token in the shape the minter actually produces, and pin that an unreadable token yields no expiry rather than a wrong one.Cross-checked independently by replaying the decode steps in Node against a token built from the minter's own encoding lines:
exprecovered exactly; the old path foundnull.Verification
cargo test -p mdbase-connect-hosted-provider --lib blob_store— 7 passed. fmt and clippy clean.Found by running
bin/hosted-diagnostics staging(cloud-ops) against a live provider — the first real use of the surface shipped in beta.78.Note
This needs a release and deploy to reach production. Until then
credential does not expirefrom the diagnostics surface is unverified for staging and lab.