Skip to content

feat(sts): cache AssumeRoleWithWebIdentity responses across isolates - #175

Open
alukach wants to merge 6 commits into
mainfrom
fix/sts-credential-cache
Open

feat(sts): cache AssumeRoleWithWebIdentity responses across isolates#175
alukach wants to merge 6 commits into
mainfrom
fix/sts-credential-cache

Conversation

@alukach

@alukach alukach commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Draft / stacked on fix/sts-request-timeout (#172). The post_form change
builds on that PR's timeout, and the two are complementary (cache removes STS
from the hot path; timeout bounds the rare cold miss). Retarget to main once
#172 merges.

Problem

Private products federate to AWS STS (AssumeRoleWithWebIdentity) on the cold path. multistore's credential cache lives in per-isolate memory (OIDC_PROVIDER is a OnceLock), and Cloudflare spins up many short-lived isolates — so a large fraction of requests re-run the STS exchange on the request hot path. When that exchange stalls, the worker hangs until the edge kills it, surfacing to the app as an unparseable 503. This is the root cause behind the intermittent "product won't load, self-heals on reload" reports.

Approach

Add an L2 cache for the STS response, shared across isolates within a colo via the Cloudflare Cache API — the same pattern already used for Source API responses in source_api/cache.rs. It sits under multistore's in-isolate L1 cache:

  • L1 (multistore, per-isolate): caches typed BackendCredentials, single-flights within an isolate.
  • L2 (this PR, per-colo): caches the raw STS response body, keyed by RoleArn (L1's own cache key). On a hit, the proxy skips the STS round-trip entirely.

The only seam data.source.coop controls in the mint path is FetchHttpExchange::post_form (the outbound STS call) — get_credentials and the L1 cache live inside multistore. So the L2 cache wraps post_form.

Effect: STS goes from ~once per isolate per credential lifetime → ~once per colo. The slow exchange leaves the user hot path almost entirely.

What's cached (and not)

  • Only AssumeRoleWithWebIdentity forms (role_arn_from_form returns None for other actions / Azure-GCP flows → bypass).
  • TTL = time to the response's <Expiration> minus a 300s lead (≥ multistore's 60s refresh lead, so an L2 entry always expires before L1 would call the derived credential stale).
  • STS error documents are never cachedttl_secs returns None when there's no parseable <Expiration>.
  • On an L2 hit we still mint the (cheap, local) JWT; only the slow STS network call is skipped. Skipping the mint too would need an L1-level hook in multistore (noted below).

Security

The cached values are short-lived, role-scoped temporary credentials, stored under a synthetic non-routable cache key (https://sts-creds.cache.internal/…, never a real edge request URL, so not externally addressable), with TTL ≤ credential lifetime, per-colo. If a deployment needs global reach, encryption-at-rest, or true cross-isolate single-flight (a cold colo can still see a small STS herd), the same cache_key/ttl_secs helpers drop into KV (global, encrypted) or a Durable Object (global, single-flight).

Follow-up (not here)

Cleaner long-term: give multistore's CredentialCache::get_or_fetch an optional runtime L2 hook (the crate doc already anticipates "a runtime can layer an additional cache tier inside the closure"). That caches typed creds at L1 and skips the JWT mint on hits too — but it's a cross-repo API change + release, vs. this which ships from data.source.coop today.

Verification

  • cargo test --test sts_cache8/8 (role/key/ttl helpers, incl. error-doc and near-expiry → not cached).
  • cargo check --target wasm32-unknown-unknown — clean.
  • cargo clippy --target wasm32-unknown-unknown -- -D warnings — clean.

🤖 Generated with Claude Code

alukach and others added 2 commits June 29, 2026 15:09
…eout

Private products federate to AWS STS on the cold path: the OIDC backend-auth
middleware POSTs AssumeRoleWithWebIdentity over the shared reqwest client, which
was built with `Client::new()` — no timeout. If that exchange stalls, the whole
Worker request hangs until the Cloudflare edge kills it and returns a non-XML
`error code: NNNN` plaintext body. The caller's AWS S3 SDK then fails to parse
it ("char 'e' is not expected.:1:1"), surfacing as an opaque contents-load
failure that self-heals after a few reloads.

It self-heals because the OIDC provider caches credentials across requests in a
warm isolate, so STS only runs on a cold isolate / cache miss — exactly when
it's slow. Public products list anonymously and never hit this path, which is
why only private products are affected.

Bound the STS POST with a 10s per-request timeout (reqwest's wasm backend honors
it via AbortController). On a stall the call now returns OidcProviderError::
HttpError → ProxyError::BackendError → a proper 503 ServiceUnavailable XML error
the client can parse and retry, instead of an unparseable edge timeout body.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
multistore's credential cache is per-isolate (OIDC_PROVIDER is a OnceLock),
and Cloudflare spins up many short-lived isolates — so a large fraction of
requests re-run the STS exchange on the request hot path. When that stalls,
the worker hangs until the edge kills it, surfacing as an unparseable 503.

Layer an L2 (cross-isolate, per-colo) Cloudflare Cache API tier UNDER
multistore's in-isolate cache, keyed by RoleArn, wrapping the only seam this
repo controls in the mint path: FetchHttpExchange::post_form. On a hit the
proxy skips the slow STS round-trip entirely.

Pure helpers (role/key/ttl) live in sts_cache.rs (host-tested); the Cache API
I/O lives in lib.rs. TTL derives from the response <Expiration> minus a 300s
lead (>= multistore's 60s refresh lead, so L2 never outlives L1's freshness).
STS error documents are never cached.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Claude finished @alukach's task in 3m 51s —— View job


Review

  • Gather context (diff, CLAUDE.md, changed files)
  • Review src/sts_cache.rs and src/lib.rs changes
  • Review test coverage in tests/sts_cache.rs
  • Post final review

✅ No blocking issues — safe to merge.

The implementation is careful and consistent with the existing source_api/cache.rs Cache-API pattern: fail-closed key derivation (cache_inputs_from_form requires both RoleArn and RoleSessionName), a synthetic non-routable cache key, and a TTL helper that only returns Some for a parseable, non-near-expiry <Expiration> — so STS error documents are structurally excluded from the cache. Both the security rationale (keying on session name, not just role) and its known limitation (lossy truncation in sts_session_name()) are already called out in-line with an upgrade path, which is exactly the right way to carry that tradeoff forward.

A couple of non-blocking observations, not required for merge:

  • src/sts_cache.rs:20REFRESH_LEAD_SECS = 300 is documented as "≥ multistore's own 60s in-isolate refresh lead," but that's a cross-crate invariant this repo can't enforce at compile time. If multistore ever widens its refresh lead past 300s, the safety margin silently erodes. Worth a one-line note-to-self (or a comment in the multistore changelog check) if that dependency is bumped in the future.
  • src/sts_cache.rs:47cache_inputs_from_form takes the first match for RoleArn/RoleSessionName via .find(...). This is harmless given form is built internally by trusted code with no duplicate keys today, but if that ever changes, the cache key could silently diverge from what's actually sent to STS (which encodes all entries via .form(form)). Not actionable now, just worth keeping in mind if form construction ever becomes more dynamic.
  • src/lib.rs:140-143sts_cache_get treats any Cache API error as a plain miss (vs. source_api/cache.rs's cached_fetch, which propagates cache-get errors as ProxyError::Internal). That's the right call here specifically because this cache sits in the hot path purely as a latency optimization — degrading to a live STS call on any hiccup is strictly safer than failing the request, and it's called out explicitly in the doc comment.

Test coverage (tests/sts_cache.rs) is solid for the pure helpers: assume-role gating, missing-field fail-closed behavior, key encoding/collision-avoidance, and TTL edge cases (near-expiry, expired, error document, unparseable <Expiration>).

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown

🚀 Latest commit deployed to https://source-data-proxy-pr-175.source-coop.workers.dev

  • Date: 2026-07-14T14:50:41Z
  • Commit: b387a10

Base automatically changed from fix/sts-request-timeout to main July 1, 2026 17:45
alukach and others added 2 commits July 1, 2026 11:10
Resolve Cargo.toml conflict: keep both [[test]] entries (sts_cache from
this branch, object_path from main) and take main's multistore 0.6.3 bump.
Also shrink the chrono dependency comment to one line (ponytail review).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `.ok()?` branch in `ttl_secs` (STS response has an <Expiration> tag
but it isn't valid RFC3339) was untested. Asserts the credential-safety
invariant: never cache a credential whose real expiry we can't determine.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The role's trust policy conditions on the token subject (the per-connection
`scv1:conn:{id}`, surfaced as RoleSessionName) and is evaluated at mint time,
inside the STS call. An L2 cache hit skips that mint, so keying on RoleArn alone
could serve a credential minted for one connection to a different connection
that shares the role but that the trust policy would reject — bypassing that
authorization gate. Key on the session name too so credentials are never shared
across connections/account-products that merely share a role.

Fail closed: if RoleArn or RoleSessionName is absent, bypass the cache and make
a live STS call rather than key on the role alone.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
alukach added a commit that referenced this pull request Jul 28, 2026
Keying on the role ARN alone — inherited from multistore's `AwsBackendAuth`,
whose cache key is the ARN — is unsound when two connections point at the same
role. The role's trust policy conditions on the assertion's `sub`
(`scv1:conn:{id}`), so a connection whose subject that policy would reject could
be served another connection's cached credentials, succeeding where STS would
have refused. It also silently misattributes CloudTrail, which is the whole
point of `sts_session_name`.

Spotted in the review of #175, which keys its L2 cache on
(RoleArn, RoleSessionName) for the same reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@alukach
alukach marked this pull request as ready for review July 28, 2026 05:20
alukach added a commit that referenced this pull request Jul 28, 2026
… overtake

Mirrors developmentseed/multistore#133 (bcf3024), from review of that PR.

"Bounded to one burst per isolate cold start" undersold the case that matters.
One cold isolate's burst is small; a deploy or mass eviction cools every isolate
at once, so the bursts coincide fleet-wide and reach STS together — where
throttling becomes a 502 per request. Name that, and point at an L2 tier (#175)
as the mitigation rather than a bigger in-memory cache, which is exactly what a
deploy discards.

Also state, in the module docs and at the call site, that inside MIN_SERVE_SECS
a live claim is deliberately overtaken so several exchanges can run at once at
the edge of expiry. Implied by the code but not written down; the existing test
now asserts it rather than leaving it looking accidental.

No behaviour change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
alukach added a commit that referenced this pull request Jul 28, 2026
…197)

## What this does

Bumps multistore `0.7.1` → `0.7.2`, which fixes production requests
being cancelled with `The Workers runtime canceled this request because
it detected that your Worker's code had hung and would never generate a
response`.

That is the whole diff. The fix itself is
[developmentseed/multistore#133][up]; this PR started as a local
reimplementation of it and was reduced to a dependency bump once that
merged and released.

[up]: developmentseed/multistore#133

## The bug

`multistore-oidc-provider`'s `CredentialCache` single-flighted
concurrent misses on a role ARN behind a `futures::lock::Mutex`. On
Cloudflare Workers, awaiting that lock is fatal rather than slow: per
[Cloudflare's docs][err], the runtime cancels a request once "all the
code associated with the request has executed and no events are left in
the event loop", and a future parked on an in-memory waker has no
pending I/O of its own. It is killed immediately, not when the lock
holder finishes.

The fast path took the same lock, so requests whose credentials were
already cached died too. Nearly all of `data.source.coop`'s open data
sits behind one connection — one role ARN, therefore one lock — so a
single isolate renewing credentials took out every concurrent read on
it.

A five-minute `wrangler tail --status error` on production caught 167
cancellations, **all 167** on that one connection (`tge-labs/aef` 163,
`protomaps/openstreetmap` 3, `ftw/global-data` 1). The timing split as
the mechanism predicts: 149 at 5–8ms wall / 1–2ms CPU (warm requests,
killed instantly with nothing pending) and 19 at 30–66ms / 21–26ms CPU
(cold isolates — the lock holders' siblings).

[err]: https://developers.cloudflare.com/workers/observability/errors/

## Why it looked random

The failures need a credential fetch in flight, so they cluster at
isolate cold start and at renewal, and vanish once an isolate is warm.
That is the "product won't load, works on reload" shape.

## Verification

Two live Cloudflare deployments, same staging API, same product, same
federated connection (`aws-opendata-eu-central-1`) — so the dependency
version is the only variable. Control is `data.staging.source.coop`,
running `main` on multistore 0.7.1. Four rounds of 80 concurrent GETs of
`/alukach/euro-test/README.md` each, cache-busted:

| round | `main` (0.7.1) | this PR (0.7.2) |
|---|---|---|
| 1 (cold) | 59× 200, **21× 500** | 80× 200 |
| 2 | 80× 200 | 80× 200 |
| 3 | 79× 200, **1× 500** | 80× 200 |
| 4 | 80× 200 | 80× 200 |

`wrangler tail` over that window: **staging logged 22 `hung and would
never generate a response` cancellations, all on this product, at the
same 5–8ms / 1–2ms signature. The preview logged zero events with
exceptions across all 320 requests.**

Also verified locally against the stub-API harness (50 concurrent
federated reads: 49/50 cancelled on `main`, 0/50 on this branch), and 73
native tests, `cargo clippy --target wasm32-unknown-unknown -D
warnings`, and the integration suite all pass.

## What changed upstream

For anyone reading this later, 0.7.2 replaces the async lock with a
cache that never blocks a caller:

- Fresh credentials are served from a plain map under a sync lock that
is never held across an `.await`.
- **Renewals are single-flighted without anyone waiting**: inside the
refresh lead the credential is still valid, so one caller claims the
exchange and the rest keep serving what they have. That works precisely
because latecomers never need the claimant's result — the thing a lock
cannot give you here.
- Two guards: a 5s floor so a credential that could expire in transit is
never served (an expired one comes back from S3 as `ExpiredToken` and
reaches the client as a misleading `AccessDenied`), and a 30s claim
timeout so a claimant killed mid-exchange cannot pin the key as renewing
until expiry.
- Cache entries are now keyed by role ARN **and** subject. Keying on the
ARN alone was unsound when two connections share a role: the trust
policy conditions on the assertion's `sub`, so a connection that policy
would reject could be served another's cached credentials.

A cold key is deliberately *not* single-flighted — there is nothing
valid to serve, so collapsing that burst would mean waiting. Worth
knowing that a deploy or mass eviction cools every isolate at once, so
those bursts coincide fleet-wide; the mitigation is an L2 tier (#175),
not a larger in-memory cache, since that tier is exactly what a deploy
discards.

## Relationship to #175

#175 adds an L2 Cloudflare-Cache-API tier for the STS response. It does
**not** fix this on its own: it wraps `FetchHttpExchange::post_form`,
which ran *inside* the old lock's critical section, so concurrent
arrivals were still cancelled. It shortens the held window a lot, but a
cold colo — L2 miss, highest concurrency — is exactly when it doesn't
help.

The two compose: 0.7.2 is L1, #175 is L2, and #175 keeps independent
value by making the cold-key duplication mostly network-free. Its
central constraint ("the only seam we control is `post_form`") is worth
revisiting now that upstream owns the cache.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant