fix(github): send an explicit User-Agent and stop downgrading a redirected comment POST

`secureGitHubRequest` powers the GitHub comment tool's `directExecution`
path, which bypasses the declarative transport. Two behaviors the transport
provided did not survive the move.

User-Agent: the transport sets `User-Agent: Sim` on every request it formats
(`request-transport.ts`), and `secureFetchWithPinnedIP` adds none of its own —
it builds the request with raw `node:https` and passes headers through
verbatim. Production runs on Bun, whose `node:http` shim injects
`user-agent: Bun/x.y.z`, so GitHub does not reject these calls today; the
defect is that Sim's deliberate attribution is silently replaced by a runtime
version string, and that the tool depends on an undocumented runtime behavior
that does not hold under Node, where GitHub answers 403 "Request forbidden by
administrative rules". Set in the helper rather than in the tool's header map
so every future caller inherits it; a caller-supplied value still wins.

Redirect method: the policy was `mode: 'standard'`, under which
`resolveRedirectHop` rewrites a 301/302'd POST to a bodyless GET regardless of
origin. GitHub answers 301 within api.github.com for a renamed repository, so
commenting on a PR there would GET `/pulls/{n}/comments`, receive a JSON array,
fail the payload shape check, and report success with no comment created.
`legacy` keeps the method and body across that hop. Cross-origin credential
stripping is unaffected — the guarded follower strips Authorization,
Proxy-Authorization and Cookie whenever `sendCredentialsOnCrossOriginRedirect`
is false, in either mode.

Author: waleedlatif1

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

@@ -83,7 +83,7 @@ describe('secureGitHubRequest redirects', () => {
     expect(hops[0].headers.authorization).toBeUndefined()
   })
 
-  it('does not replay a comment POST body across an origin boundary', async () => {
+  it('does not forward the GitHub token when a comment POST crosses an origin boundary', async () => {
     const hops: RecordedHop[] = []
     const attacker = await startRecordingServer(hops)
     const origin = await startServer((req, res) => {
@@ -100,8 +100,32 @@ describe('secureGitHubRequest redirects', () => {
 
     expect(hops).toHaveLength(1)
     expect(hops[0].headers.authorization).toBeUndefined()
-    expect(hops[0].method).toBe('GET')
-    expect(hops[0].body).toBe('')
+    expect(hops[0].headers.cookie).toBeUndefined()
+  })
+
+  it('replays a comment POST as a POST across a same-origin renamed-repository 301', async () => {
+    const hops: RecordedHop[] = []
+    const origin = await startServer((req, res) => {
+      if (req.url === '/repos/octo/old/pulls/7/comments') {
+        req.resume()
+        res.writeHead(301, { location: '/repos/octo/new/pulls/7/comments' })
+        res.end()
+        return
+      }
+      record(hops, req, res)
+    })
+
+    await secureGitHubRequest(`${origin}/repos/octo/old/pulls/7/comments`, {
+      method: 'POST',
+      headers: { ...GITHUB_HEADERS, 'Content-Type': 'application/json' },
+      body: '{"body":"Looks good"}',
+    })
+
+    expect(hops).toHaveLength(1)
+    expect(hops[0].url).toBe('/repos/octo/new/pulls/7/comments')
+    expect(hops[0].method).toBe('POST')
+    expect(hops[0].body).toBe('{"body":"Looks good"}')
+    expect(hops[0].headers.authorization).toBe('Bearer ghp_workspace_token')
   })
 
   it('keeps the token on a same-origin redirect, as a renamed repository needs', async () => {
@@ -126,3 +150,32 @@ describe('secureGitHubRequest redirects', () => {
     expect(hops[0].headers.authorization).toBe('Bearer ghp_workspace_token')
   })
 })
+
+describe('secureGitHubRequest User-Agent', () => {
+  it('sends an explicit Sim User-Agent on the commit lookup and the comment POST', async () => {
+    const hops: RecordedHop[] = []
+    const origin = await startRecordingServer(hops)
+
+    await secureGitHubRequest(`${origin}/repos/octo/repo/pulls/7`, { headers: GITHUB_HEADERS })
+    await secureGitHubRequest(`${origin}/repos/octo/repo/pulls/7/comments`, {
+      method: 'POST',
+      headers: { ...GITHUB_HEADERS, 'Content-Type': 'application/json' },
+      body: '{"body":"Looks good"}',
+    })
+
+    expect(hops).toHaveLength(2)
+    expect(hops[0].headers['user-agent']).toBe('Sim')
+    expect(hops[1].headers['user-agent']).toBe('Sim')
+  })
+
+  it('leaves a caller-supplied User-Agent untouched', async () => {
+    const hops: RecordedHop[] = []
+    const origin = await startRecordingServer(hops)
+
+    await secureGitHubRequest(origin, {
+      headers: { ...GITHUB_HEADERS, 'user-agent': 'Sim-Custom' },
+    })
+
+    expect(hops[0].headers['user-agent']).toBe('Sim-Custom')
+  })
+})

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

@@ -18,6 +18,22 @@ export interface SecureGitHubRequestOptions {
   signal?: AbortSignal
 }
 
+/**
+ * GitHub's API rejects a request without a User-Agent with 403 "Request forbidden by
+ * administrative rules". The declarative transport sets `User-Agent: Sim` for every
+ * tool it formats; a tool on this helper bypasses that, and the guarded fetch builds
+ * its request with raw `node:https`, which adds no default. Bun's `node:http` shim
+ * does inject its own `Bun/x.y.z`, so the call happens to work in production today —
+ * but that silently replaces Sim's attribution and does not hold under Node.
+ *
+ * Set here rather than in each caller's header map so every future tool on this
+ * helper inherits it. A caller that supplies its own User-Agent, in any casing, wins.
+ */
+function withUserAgent(headers: Record<string, string>): Record<string, string> {
+  const hasUserAgent = Object.keys(headers).some((name) => name.toLowerCase() === 'user-agent')
+  return hasUserAgent ? headers : { ...headers, 'User-Agent': 'Sim' }
+}
+
 /**
  * Executes one DNS-validated, IP-pinned GitHub request for a tool that cannot use
  * the declarative transport — a multi-phase tool running under `directExecution`.
@@ -28,11 +44,19 @@ export interface SecureGitHubRequestOptions {
  *
  * 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
+ * credentials when a policy is present. `legacy` is chosen over `standard` for the
+ * method rules: `standard` rewrites a redirected POST to a bodyless GET on 301/302
+ * regardless of origin, and GitHub answers 301 within api.github.com for a renamed
+ * repository, so a comment POST there would be replayed as a GET of the comment
+ * list — a JSON array that fails the tool's payload shape check and reports success
+ * with no comment created. `legacy` keeps the method and body across that hop.
+ *
+ * Credential stripping is unaffected by the mode: the guarded follower strips
+ * Authorization, Proxy-Authorization and Cookie on a cross-origin hop whenever
+ * `sendCredentialsOnCrossOriginRedirect` is false, in either mode.
+ *
+ * `stripAuthOnRedirect` is deliberately NOT set: it drops the token on every hop,
+ * including the legitimate same-origin renamed-repository 301, so an unauthenticated
  * replay there would turn a working call into a 401.
  */
 export async function secureGitHubRequest(
@@ -46,10 +70,10 @@ export async function secureGitHubRequest(
 
   const response = await secureFetchWithPinnedIP(url, validation.resolvedIP, {
     method: options.method ?? 'GET',
-    headers: options.headers,
+    headers: withUserAgent(options.headers),
     body: options.body,
     maxResponseBytes: options.maxResponseBytes ?? GITHUB_MAX_RESPONSE_BYTES,
-    redirectPolicy: { mode: 'standard', sendCredentialsOnCrossOriginRedirect: false },
+    redirectPolicy: { mode: 'legacy', sendCredentialsOnCrossOriginRedirect: false },
     signal: options.signal,
   })