diff --git a/cli/release-core/http.js b/cli/release-core/http.js index 14d411f746..3f4d45ffa3 100644 --- a/cli/release-core/http.js +++ b/cli/release-core/http.js @@ -60,7 +60,18 @@ function evaluateRedirect(currentUrl, location, options = {}) { } } - const originalHostname = new URL(options.originalUrl || currentUrl).hostname + const originalUrl = new URL(options.originalUrl || currentUrl) + const originalHostname = originalUrl.hostname + + if (target.username || target.password) { + return { ok: false, reason: 'Refusing redirect containing URL credentials' } + } + if (target.port !== originalUrl.port) { + return { + ok: false, + reason: `Refusing redirect to unexpected port ${target.port || '(default)'}`, + } + } if (!isRedirectHostAllowed(target.hostname, originalHostname)) { return { ok: false, diff --git a/cli/release-core/launcher.js b/cli/release-core/launcher.js index d830cf074e..f82b79efcc 100644 --- a/cli/release-core/launcher.js +++ b/cli/release-core/launcher.js @@ -101,13 +101,18 @@ function resolveDownloadOrigin(configuredOrigin, { warn = () => {} } = {}) { const isHttp = parsed.protocol === 'http:' || parsed.protocol === 'https:' // Only an origin: credentials or a query would ride along on every request. const isBareOrigin = - !parsed.username && !parsed.password && !parsed.search && !parsed.hash + parsed.pathname === '/' && + !parsed.username && + !parsed.password && + !parsed.search && + !parsed.hash if (isHttp && isLocal && isBareOrigin) { return trimmed } if ( parsed.protocol === 'https:' && isReleaseOriginHost(parsed.hostname) && + !parsed.port && isBareOrigin ) { return trimmed diff --git a/cli/src/__tests__/release/proxy-http-get.test.ts b/cli/src/__tests__/release/proxy-http-get.test.ts index 13858cef41..be515ac560 100644 --- a/cli/src/__tests__/release/proxy-http-get.test.ts +++ b/cli/src/__tests__/release/proxy-http-get.test.ts @@ -334,6 +334,43 @@ for (const helperModule of helperModules) { expect(httpsGetCalls.every((call) => call.agent != null)).toBe(true) }) + test('refuses redirect credentials and unexpected ports', async () => { + for (const location of [ + 'https://user:pass@github.com/releases/freebuff.tar.gz', + 'https://github.com:444/releases/freebuff.tar.gz', + ]) { + let callCount = 0 + const { createReleaseHttpClient } = require(helperModule.path) + const client = createReleaseHttpClient({ + env: {}, + userAgent: 'release-test-agent', + requestTimeout: 2500, + httpsModule: { + get(_options: unknown, callback: (response: Readable) => void) { + callCount += 1 + queueMicrotask(() => { + callback(createResponse(302, { location })) + }) + return { + on() { + return this + }, + setTimeout() { + return this + }, + destroy() {}, + } + }, + }, + }) + + await expect( + client.httpGet('https://codebuff.com/api/releases/download/file'), + ).rejects.toMatchObject({ code: 'EREDIRECT_REFUSED' }) + expect(callCount).toBe(1) + } + }) + test('limits redirect chains', async () => { const { createReleaseHttpClient } = require(helperModule.path) let callCount = 0 diff --git a/cli/src/__tests__/release/wrapper-safety.test.ts b/cli/src/__tests__/release/wrapper-safety.test.ts index 26ac6f9062..168480b2d2 100644 --- a/cli/src/__tests__/release/wrapper-safety.test.ts +++ b/cli/src/__tests__/release/wrapper-safety.test.ts @@ -166,7 +166,30 @@ for (const wrapper of wrappers) { describe('shared release launcher safety', () => { const launcherPath = join(repoRoot, 'cli/release-core/launcher.js') - const { createLauncher } = require(launcherPath) + const { createLauncher, resolveDownloadOrigin, DEFAULT_DOWNLOAD_ORIGIN } = + require(launcherPath) + + test('rejects release URL overrides with a path or nonstandard public port', () => { + const warnings: string[] = [] + + expect( + resolveDownloadOrigin('https://freebuff.com/api', { + warn: (message: string) => warnings.push(message), + }), + ).toBe(DEFAULT_DOWNLOAD_ORIGIN) + expect( + resolveDownloadOrigin('https://codebuff.com:8443', { + warn: (message: string) => warnings.push(message), + }), + ).toBe(DEFAULT_DOWNLOAD_ORIGIN) + expect(warnings).toHaveLength(2) + }) + + test('accepts a bare loopback origin with a custom port for local development', () => { + expect(resolveDownloadOrigin('http://127.0.0.1:43127')).toBe( + 'http://127.0.0.1:43127', + ) + }) test('stages an update before stopping the running process', () => { const source = readFileSync(launcherPath, 'utf8')