Add content-key seam for CDN content-key enforcement - #346
Merged
Conversation
`ContentController` now exposes two opt-in hooks so an app can attach its own headers to the requests Storm makes for content, without ThunderCloud needing to know anything about where those headers come from: - `contentRequestHeaderProvider` is consulted before every content request (update check, delta download and full bundle download) and its headers are merged onto the outgoing request. It is passed the url the request will be sent to so an app can scope what it returns to a particular host. - `contentAuthFailureHandler` is called when a content request comes back 401 or 403, giving the app a chance to refresh whatever the provider returns before the request is re-sent. The request is only ever re-sent once. Both default to nil, in which case content requests are made exactly as they were before. The response handling for the update check and the bundle download moves into `finishUpdateCheck(...)` and `finishDownload(...)` so it can be reached either directly or after a retry. The logic is unchanged. Tests cover the headers reaching the outgoing request on all three request paths, 401 and 403 each causing exactly one refresh and one retry, and nil hooks leaving requests untouched. They intercept the requests with a `URLProtocol` stub, which needs `URLSessionConfiguration.default` swizzling because `RequestController` builds its sessions internally. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`URLSession` copies a request's headers onto the request it sends to a redirect's target, including when that target is on a different host. A content request can be redirected off the Storm API onto whichever host serves the bundle, so a header an app only meant for one host was being sent on to whichever host the redirect named. `ContentRequestSession` performs content requests and asks `contentRequestHeaderProvider` what should be sent each time it is about to follow a redirect, removing any header the provider doesn't return for the new url before the request is sent. An app can therefore scope a header to the hosts it trusts and rely on it not leaving them. This session is only used when a provider has been set. With no provider `ContentController` still makes its requests through `RequestController` exactly as it always has, so apps which don't use the hook are unaffected. Downloads made in the background use a session with a fixed identifier, which `handleEventsForBackgroundURLSession(session:completionHandler:)` now recognises so the events for it are still delivered after a relaunch. Tests cover a redirect to a host the provider does not vouch for arriving without the header, and a redirect to one it does vouch for still carrying it, for both the update check and a bundle download. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
juwoncube
requested review from
3scsamdavis,
JonValijonov and
Szuyun-3SC
and removed request for
a team
August 11, 2026 08:53
3scsamdavis
approved these changes
Aug 11, 2026
…ound relaunch handling
…s; fix status mapping
…d bundle downloads
Records the sharp edges this branch's work turned up: PRs target the active release branch rather than master, and any per-redirect logic has to be applied by pre-resolving the chain on a default session, because background URLSession tasks follow redirects without a delegate and content downloads default to the background session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This reverts commit eacc2d3.
…key-seam-pinned # Conflicts: # ThunderCloud.xcodeproj/project.pbxproj
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.
What
ContentControllercontent requests, scoped to the Storm content host(s), with headers re-scoped when a request is redirected.No behaviour change for any app that does not inject a provider — ThunderCloud stays App-Check-agnostic.
Why
The ARC CDN now enforces a content key on Storm bundle downloads (ARCBLOOD-4351 / SEC-578; dev is already returning 403 to unauthenticated pulls, prod follows).
ContentController's request controllers areinternal, so a consuming app cannot attach thex-content-keyheader itself. This seam lets each app own its key lifecycle app-side and feed the framework.Related
Notes for reviewers
release/v4.3.4(cut fromrelease/v4.3.3) per the package release convention — no master merge.e389b82, the revision Blood currently pins, so the diff is exactly the two seam commits.Redirect hardening (added to this PR)
A real-device verification found that nothing stripped provider-injected headers on redirect:
URLSessioncopies request headers onto the redirect target's request, so a provider header was carried across the 303 from the Storm API onto the CDN host. Both hosts are ours today and the download depends on that carry, but a CMS-authored redirect to a third-party host would have carried the key off-site.The provider's host-scoping contract now extends across redirects. The allowed-host decision stays with the injected provider — the seam asks it about the new target url, never a hardcoded host list — so an app that grants both the API and CDN hosts keeps its working download, while a chain ending anywhere it does not vouch for arrives with the provider headers stripped.
Because background
URLSessiontasks follow redirects automatically and never callwillPerformHTTPRedirection, anddownloadPackagedefaults toinBackground: true, live-delegate scoping alone would not have covered the bundle download path. Background downloads now pre-resolve the redirect chain on the default session with scoping applied per hop, then hand the final resolved url to the background transfer.Verification: device e2e re-proof at branch tip passed (
data/blood-e2e-seamfix-verify/report.md).Known follow-ups (accepted, not fixed here)
inBackground: falsetests already cover. They would still pass if the resolved url were ignored, so the pre-resolution behaviour is implemented but not test-verified.RequestControllerpath.contentRequestError(for:)returns.invalidResponsefor every error status, where theRequestControllerpath returnedHTTP.Errorcarrying the status code. Apps on the provider path can no longer tell a 404 from a 500.handleEventsForBackgroundURLSessioncan only rebuild the session ifcontentRequestHeaderProvideris already set; an app that configures it asynchronously will miss queued background events.Notes for future work on this seam
release/*branch, nevermaster. Automation that defaults to the repository's default branch will open amaster-targeted PR — check the base before relying on a tool-created one.URLSessiontasks follow redirects automatically and never callwillPerformHTTPRedirection, anddownloadPackagedefaults toinBackground: true. Any per-redirect logic therefore has to be applied by pre-resolving the chain on a default session before the final url is handed to the background transfer; a test that forcesinBackground: falsewill not catch the gap.