fix(github): stop forwarding the GitHub token across a redirect origin

secureGitHubRequest passed no redirectPolicy, and the transport only strips
credential headers when one is present, so an api.github.com redirect to another
origin carried the workspace's Authorization: Bearer header to the new host.

Adopts the standard policy already used by the internal Google Drive client.
stripAuthOnRedirect stays off: GitHub redirects same-origin for legitimate
reasons (a renamed repository answers 301), and dropping auth there would turn
a working call into a 401.

Author: waleedlatif1

apps/sim/tools/github/utils.server.test.ts

@@ -0,0 +1,128 @@
+/**
+ * Pins the redirect contract of the GitHub direct-execution transport: the workspace
+ * token must never cross an origin boundary, while a legitimate same-origin GitHub
+ * redirect (a renamed repository) must stay authenticated.
+ *
+ * @vitest-environment node
+ */
+import http from 'node:http'
+import type { AddressInfo } from 'node:net'
+import { afterEach, describe, expect, it, vi } from 'vitest'
+
+vi.mock('@sim/security/dns', () => ({
+  resolveHostAddresses: vi.fn(async () => ({ addresses: ['127.0.0.1'] })),
+  preferIpv4: (addresses: string[]) => addresses[0],
+}))
+
+vi.mock('@/lib/core/config/env-flags', () => ({
+  isHosted: false,
+  isPrivateDatabaseHostsAllowed: false,
+  getProxyUrl: () => undefined,
+}))
+
+import { secureGitHubRequest } from '@/tools/github/utils.server'
+
+interface RecordedHop {
+  url: string
+  method: string
+  body: string
+  headers: http.IncomingHttpHeaders
+}
+
+const servers: http.Server[] = []
+
+afterEach(() => {
+  for (const server of servers.splice(0)) server.close()
+})
+
+async function startServer(handler: http.RequestListener): Promise<string> {
+  const server = http.createServer(handler)
+  servers.push(server)
+  await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve))
+  return `http://127.0.0.1:${(server.address() as AddressInfo).port}`
+}
+
+function record(hops: RecordedHop[], req: http.IncomingMessage, res: http.ServerResponse): void {
+  let body = ''
+  req.on('data', (chunk) => {
+    body += chunk
+  })
+  req.on('end', () => {
+    hops.push({ url: req.url ?? '', method: req.method ?? '', body, headers: req.headers })
+    res.writeHead(200, { 'Content-Type': 'application/json' })
+    res.end('{"ok":true}')
+  })
+}
+
+/** Records every request it receives, then answers 200. */
+async function startRecordingServer(hops: RecordedHop[]): Promise<string> {
+  return startServer((req, res) => record(hops, req, res))
+}
+
+const GITHUB_HEADERS = {
+  Accept: 'application/vnd.github.v3+json',
+  Authorization: 'Bearer ghp_workspace_token',
+  'X-GitHub-Api-Version': '2022-11-28',
+}
+
+describe('secureGitHubRequest redirects', () => {
+  it('does not forward the GitHub token across an origin boundary', async () => {
+    const hops: RecordedHop[] = []
+    const attacker = await startRecordingServer(hops)
+    const origin = await startServer((req, res) => {
+      req.resume()
+      res.writeHead(302, { location: `${attacker}/stolen` })
+      res.end()
+    })
+
+    const response = await secureGitHubRequest(origin, { headers: GITHUB_HEADERS })
+
+    expect(response.status).toBe(200)
+    expect(hops).toHaveLength(1)
+    expect(hops[0].url).toBe('/stolen')
+    expect(hops[0].headers.authorization).toBeUndefined()
+  })
+
+  it('does not replay a comment POST body across an origin boundary', async () => {
+    const hops: RecordedHop[] = []
+    const attacker = await startRecordingServer(hops)
+    const origin = await startServer((req, res) => {
+      req.resume()
+      res.writeHead(302, { location: `${attacker}/stolen` })
+      res.end()
+    })
+
+    await secureGitHubRequest(origin, {
+      method: 'POST',
+      headers: { ...GITHUB_HEADERS, 'Content-Type': 'application/json' },
+      body: '{"body":"Looks good"}',
+    })
+
+    expect(hops).toHaveLength(1)
+    expect(hops[0].headers.authorization).toBeUndefined()
+    expect(hops[0].method).toBe('GET')
+    expect(hops[0].body).toBe('')
+  })
+
+  it('keeps the token on a same-origin redirect, as a renamed repository needs', async () => {
+    const hops: RecordedHop[] = []
+    const origin = await startServer((req, res) => {
+      if (req.url === '/repos/octo/old/pulls/7') {
+        req.resume()
+        res.writeHead(301, { location: '/repos/octo/new/pulls/7' })
+        res.end()
+        return
+      }
+      record(hops, req, res)
+    })
+
+    const response = await secureGitHubRequest(`${origin}/repos/octo/old/pulls/7`, {
+      headers: GITHUB_HEADERS,
+    })
+
+    expect(response.status).toBe(200)
+    expect(hops).toHaveLength(1)
+    expect(hops[0].url).toBe('/repos/octo/new/pulls/7')
+    expect(hops[0].headers.authorization).toBe('Bearer ghp_workspace_token')
+  })
+})

apps/sim/tools/github/utils.server.ts

@@ -25,6 +25,15 @@ export interface SecureGitHubRequestOptions {
  * This deliberately carries no retry loop: the tools on this path declare no
  * `request.retry`, so the transport retries them zero times today, and the second
  * phase of a comment flow is a non-idempotent POST that must not be replayed.
+ *
+ * The redirect policy is explicit because omitting it leaves the workspace's GitHub
+ * token on the request across a cross-origin hop — the transport only strips
+ * credentials when a policy is present. `standard` also refuses to replay the POST
+ * a 301/302 downgrades, which is the same non-idempotency reasoning as the missing
+ * retry loop above. `stripAuthOnRedirect` is deliberately NOT set: it drops the
+ * token on every hop, and GitHub redirects same-origin for legitimate reasons (a
+ * renamed repository answers 301 within api.github.com), so an unauthenticated
+ * replay there would turn a working call into a 401.
  */
 export async function secureGitHubRequest(
   url: string,
@@ -40,6 +49,7 @@ export async function secureGitHubRequest(
     headers: options.headers,
     body: options.body,
     maxResponseBytes: options.maxResponseBytes ?? GITHUB_MAX_RESPONSE_BYTES,
+    redirectPolicy: { mode: 'standard', sendCredentialsOnCrossOriginRedirect: false },
     signal: options.signal,
   })