Expire a batch action's proxy token no later than upstream's href [patch]

[matt-edmondson]

Fixes #66

What was wrong

BatchRewriter computed a single now + TokenLifetime (1h by default) for the whole response. For every action it removed upstream's expires_at and overwrote expires_in with that lifetime. The token still carries upstream's href and header, and for GitHub LFS and pre-signed S3/Azure URLs those often last only 5 to 15 minutes. git-lfs had been told the action was good for an hour, so it never re-batched. A fetch of an uncached object after upstream's expiry was then relayed to it as a 401 or 403.

Change

• Each action's expiry is now the earliest of:
  • the proxy lifetime;
  • upstream's expires_at;
  • now + expires_in.
• Upstream's expiry is brought forward by a 30-second margin (BatchRewriter.UpstreamExpiryMargin), so a transfer that starts just inside the advertised window still reaches upstream before the credential lapses. The triage note asked for this margin.
• That one value is both the token's ExpiresAt and the advertised expires_in, so the two can't disagree. expires_at is still removed, and expires_in never goes below 0.
• An action with no upstream expiry, or with one later than the proxy lifetime, keeps TokenLifetime exactly as before.
• An expires_at/expires_in of the wrong JSON type, or one that can't be parsed, is ignored rather than throwing. A huge expires_in is clamped so the date arithmetic can't overflow.
• The design spec line "Set expires_in from the token lifetime" and the README's TokenLifetime row now say "never later than upstream", as the issue asked.

Tests

In BatchRewriterTests:

• Rewrite_UpstreamExpiresSooner_TokenAndExpiresInFollowUpstream covers expires_in: 600, expires_at 10 minutes out, and both together in either order. Each case checks that expires_in is 570, that the token's ExpiresAt is now + 570s, and that the token is refused once the clock reaches 600 s.
• Rewrite_UpstreamExpiresLater_KeepsTheTokenLifetime and Rewrite_ActionWithNoUpstreamExpiry_KeepsTheTokenLifetime check that 3600 is kept.
• Rewrite_UnusableUpstreamExpiry_IsIgnored covers a string expires_in, a numeric or unparseable expires_at, and long.MaxValue.
• Rewrite_UpstreamAlreadyExpired_AdvertisesZero.
• Two existing tests are updated. The ADO fixture's expires_at is exactly one hour out, so with the margin its token now expires 30 s sooner. Rewrite_SetsExpiresInFromTokenLifetimeAndDropsExpiresAt is split so the lifetime case uses an action with no upstream expiry.

With the fix reverted, 6 cases fail (the four upstream-sooner rows, the already-expired case and the updated ADO assertion). The full suite passes locally: 365 tests, 0 failed.

BatchRewriter.cs is also touched by #104. This change is placed so the two don't overlap, and the branch merges cleanly with #101, #102, #103, #104 and #107.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WY7QGzbH6kE2oceF1hP8fh

---

Generated by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud