Skip to content

Commit 7c7fd6c

Browse files
authored
fix: strip auth headers for GitHub release assets redirects (#9211)
1 parent 7977708 commit 7c7fd6c

File tree

4 files changed

+666
-3
lines changed

4 files changed

+666
-3
lines changed

.changeset/lovely-moose-worry.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"builder-util-runtime": minor
3+
---
4+
5+
fix: implement industry-standard cross-origin redirect auth handling
6+
7+
Replace hardcoded service-specific hostname checks with sophisticated cross-origin redirect detection that matches industry standards from Python requests library and Apache HttpClient.
8+
9+
**Key improvements:**
10+
- **Case-insensitive hostname comparison** for robust origin detection
11+
- **HTTP→HTTPS upgrade allowance** on standard ports (80→443) for backward compatibility
12+
- **Proper default port handling** that treats implicit and explicit default ports as equivalent
13+
- **Standards-compliant cross-origin detection** following RFC specifications
14+
15+
**Fixes GitHub issue #9207:** GitHub release asset downloads failing with 403 Forbidden when redirected from `api.github.com` to `release-assets.githubusercontent.com` (Azure backend) or other cloud storage services that don't accept GitHub tokens.
16+
17+
The implementation now handles all cross-origin redirect scenarios while maintaining compatibility with legitimate same-origin redirects and industry-standard HTTP→HTTPS upgrades.

.github/workflows/test.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ jobs:
6767
fail-fast: false
6868
matrix:
6969
testFiles:
70-
- ArtifactPublisherTest,BuildTest,ExtraBuildTest,RepoSlugTest,binDownloadTest,configurationValidationTest,filenameUtilTest,filesTest,globTest,ignoreTest,macroExpanderTest,mainEntryTest,urlUtilTest,extraMetadataTest,linuxArchiveTest,linuxPackagerTest,HoistedNodeModuleTest,MemoLazyTest,HoistTest,ExtraBuildResourcesTest
70+
- ArtifactPublisherTest,BuildTest,ExtraBuildTest,RepoSlugTest,binDownloadTest,configurationValidationTest,filenameUtilTest,filesTest,globTest,httpExecutorTest,ignoreTest,macroExpanderTest,mainEntryTest,urlUtilTest,extraMetadataTest,linuxArchiveTest,linuxPackagerTest,HoistedNodeModuleTest,MemoLazyTest,HoistTest,ExtraBuildResourcesTest
7171
- snapTest,debTest,fpmTest,protonTest
7272
- winPackagerTest,winCodeSignTest,webInstallerTest
7373
- oneClickInstallerTest,assistedInstallerTest

packages/builder-util-runtime/src/httpExecutor.ts

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,14 +319,69 @@ Please double check that your authentication token is correct. Due to security r
319319
const newOptions = configureRequestOptionsFromUrl(redirectUrl, { ...options })
320320
const headers = newOptions.headers
321321
if (headers?.authorization) {
322-
const parsedNewUrl = parseUrl(redirectUrl, options)
323-
if (parsedNewUrl.hostname.endsWith(".amazonaws.com") || parsedNewUrl.searchParams.has("X-Amz-Credential")) {
322+
// Parse original and redirect URLs to compare origins
323+
const originalUrl = HttpExecutor.reconstructOriginalUrl(options)
324+
const parsedRedirectUrl = parseUrl(redirectUrl, options)
325+
326+
// Strip authorization header on cross-origin redirects (different protocol, hostname, or port)
327+
if (HttpExecutor.isCrossOriginRedirect(originalUrl, parsedRedirectUrl)) {
328+
if (debug.enabled) {
329+
debug(`Given the cross-origin redirect (from ${originalUrl.host} to ${parsedRedirectUrl.host}), the Authorization header will be stripped out.`)
330+
}
324331
delete headers.authorization
325332
}
326333
}
327334
return newOptions
328335
}
329336

337+
private static reconstructOriginalUrl(options: RequestOptions): URL {
338+
const protocol = options.protocol || "https:"
339+
if (!options.hostname) {
340+
throw new Error("Missing hostname in request options")
341+
}
342+
const hostname = options.hostname
343+
const port = options.port ? `:${options.port}` : ""
344+
const path = options.path || "/"
345+
return new URL(`${protocol}//${hostname}${port}${path}`)
346+
}
347+
348+
private static isCrossOriginRedirect(originalUrl: URL, redirectUrl: URL): boolean {
349+
// Case-insensitive hostname comparison
350+
if (originalUrl.hostname.toLowerCase() !== redirectUrl.hostname.toLowerCase()) {
351+
return true
352+
}
353+
354+
// Special case: allow http -> https redirect on same host with standard ports
355+
// This matches the behavior of Python requests library for backward compatibility
356+
// url.port returns an empty string if the port is omitted
357+
// or explicitly set to the default port for a given protocol.
358+
if (
359+
originalUrl.protocol === "http:" &&
360+
// This can be replaced with `!originalUrl.port`, but for the sake of clarity.
361+
["80", ""].includes(originalUrl.port) &&
362+
redirectUrl.protocol === "https:" &&
363+
// This can be replaced with `!redirectUrl.port`, but for the sake of clarity.
364+
["443", ""].includes(redirectUrl.port)
365+
) {
366+
return false
367+
}
368+
369+
// According to RFC 7235, a change in protocol or port constitutes a cross-origin request.
370+
// Forwarding authentication headers to a different origin can be a security risk.
371+
// For example, https://example.com:443 and http://example.com:80 are different origins.
372+
// We only make an exception for HTTP -> HTTPS upgrades on standard ports for backward compatibility.
373+
374+
// Strip auth on any other protocol change
375+
if (originalUrl.protocol !== redirectUrl.protocol) {
376+
return true
377+
}
378+
379+
// Strip auth on port change (accounting for default ports)
380+
const originalPort = originalUrl.port
381+
const redirectPort = redirectUrl.port
382+
return originalPort !== redirectPort
383+
}
384+
330385
static retryOnServerError(task: () => Promise<any>, maxRetries = 3) {
331386
for (let attemptNumber = 0; ; attemptNumber++) {
332387
try {

0 commit comments

Comments
 (0)