Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion cli/release-core/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion cli/release-core/launcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 37 additions & 0 deletions cli/src/__tests__/release/proxy-http-get.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 24 additions & 1 deletion cli/src/__tests__/release/wrapper-safety.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Loading