fix(security): eliminate shell injection in pr_merge GraphQL fallback

lib/adapters/pr-merge-github.test.ts

@@ -461,15 +461,77 @@ describe('prMergeGithub — subprocess boundary', () => {
     expect(result.data.queue_position).toBe(3);
     // Verify both --auto and GraphQL calls were made
     const autoCall = execCalls.find(
-      (c) => c.includes('gh pr merge 284') && c.includes('--auto'),
+      (c) => unquote(c).includes('gh pr merge 284') && unquote(c).includes('--auto'),
     );
     expect(autoCall).toBeDefined();
-    const nodeIdCall = execCalls.find((c) => c.includes('gh api repos/org/repo/pulls/284'));
+    const nodeIdCall = execCalls.find((c) =>
+      unquote(c).includes('gh api repos/org/repo/pulls/284'),
+    );
     expect(nodeIdCall).toBeDefined();
     const graphqlCall = execCalls.find(
-      (c) => c.includes('gh api graphql') && c.includes('enqueuePullRequest'),
+      (c) =>
+        unquote(c).includes('gh api graphql') && unquote(c).includes('enqueuePullRequest'),
+    );
+    expect(graphqlCall).toBeDefined();
+  });
+
+  test('pr-merge-github — GraphQL fallback shell-escapes repo and prId (security)', async () => {
+    // Regression: enqueuePullRequestViaGraphQL must use argv-array form
+    // (runArgv) so that special characters in `repo` or `prId` cannot break
+    // out of the shell command. We verify by stubbing a node_id with chars
+    // that would terminate a string-template'd command.
+    on(
+      'enqueuePullRequest',
+      JSON.stringify({
+        data: { enqueuePullRequest: { mergeQueueEntry: { position: 7 } } },
+      }),
+    );
+    stubNoQueue();
+    on('gh pr merge 286 --squash --delete-branch', () => {
+      throw mergeQueueError();
+    });
+    on('gh pr merge 286 --squash --delete-branch --auto', () => {
+      const err = new Error('Auto merge is not allowed for this repository') as ThrowableError;
+      err.stderr = 'Auto merge is not allowed for this repository\n';
+      throw err;
+    });
+    // node_id contains characters that WOULD break out of a quoted shell value
+    on(
+      'gh api repos/org/repo/pulls/286',
+      JSON.stringify({ node_id: `PR_kw"; echo PWNED; #` }),
+    );
+    on(
+      'gh pr view 286 --json state,url,mergeCommit',
+      JSON.stringify({
+        state: 'OPEN',
+        url: 'https://github.com/org/repo/pull/286',
+        mergeCommit: null,
+      }),
+    );
+
+    const result = await prMergeGithub({ number: 286, repo: 'org/repo' });
+    expectOk(result);
+    // The dangerous prId value should appear inside single-quoted argv
+    // elements, never as bare shell text. With shell-escape, every `'` in
+    // the value is replaced by `'\''` and the value is wrapped in `'...'`.
+    const graphqlCall = execCalls.find(
+      (c) => c.includes('gh') && c.includes('graphql') && c.includes('PWNED'),
     );
     expect(graphqlCall).toBeDefined();
+    // Shell-escape contract: every argv element is wrapped in `'...'` and
+    // any embedded single quote is rewritten as `'\''`. The dangerous value
+    // (including spaces and `; echo PWNED; #`) must be contained inside one
+    // single-quoted region. We verify via a regex that captures the entire
+    // single-quoted prId argv element.
+    const prIdMatch = graphqlCall!.match(/'(prId=[^']*)'/);
+    expect(prIdMatch).not.toBeNull();
+    expect(prIdMatch![1]).toContain('PWNED');
+    // Confirm there are no UNQUOTED `;` chars (which would let the shell run
+    // the rest as a separate command). Strip all single-quoted regions and
+    // the residual must not contain `;`.
+    const residual = graphqlCall!.replace(/'[^']*'/g, '');
+    expect(residual).not.toContain(';');
+    expect(residual).not.toContain('echo');
   });
 
   test('pr-merge-github — direct merge success has graphql_fallback:false', async () => {

lib/adapters/pr-merge-github.ts

@@ -21,6 +21,7 @@ import { execSync } from 'child_process';
 import { writeFileSync } from 'fs';
 import { detectMergeQueue, type MergeQueueInfo } from '../merge_queue_detect.js';
 import { parseRepoSlug } from '../shared/parse-repo-slug.js';
+import { runArgv } from '../shared/error-norm.js';
 import { getAdapter } from './index.js';
 import type {
   AdapterResult,
@@ -81,35 +82,47 @@ function enqueuePullRequestViaGraphQL(
   number: number,
   repo: string | undefined,
 ): number | null {
-  // Step 1: fetch the PR's node_id. When repo is undefined, gh uses the
-  // current directory's remote. When repo is provided, we must include it in
-  // both the API path and --repo flag.
-  let nodeIdCmd: string;
+  // Use runArgv (argv-array form, each element shell-escaped) instead of
+  // string-template interpolation to eliminate shell-injection surface on
+  // `repo` and `prId` values. Both flow from external sources (caller-supplied
+  // repo arg, GitHub API response) and must not be trusted as shell-safe.
+  const cwd = process.env.CLAUDE_PROJECT_DIR ?? process.cwd();
+
+  // Step 1: fetch the PR's node_id.
+  let nodeIdArgv: string[];
   if (repo !== undefined) {
-    nodeIdCmd = `gh api repos/${repo}/pulls/${number} --repo ${repo} --jq '.node_id'`;
+    nodeIdArgv = ['gh', 'api', `repos/${repo}/pulls/${number}`, '--repo', repo, '--jq', '.node_id'];
   } else {
     // When repo is undefined, resolve via the slug parser (same logic as
-    // detectMergeQueue). If that fails, let gh api infer from cwd.
+    // detectMergeQueue). If that fails, fall back to `gh pr view` which infers
+    // the repo from cwd's remote.
     const slug = parseRepoSlug();
     if (slug !== null) {
-      nodeIdCmd = `gh api repos/${slug}/pulls/${number} --jq '.node_id'`;
+      nodeIdArgv = ['gh', 'api', `repos/${slug}/pulls/${number}`, '--jq', '.node_id'];
     } else {
-      // Last resort: gh api without explicit repo path — will fail if cwd isn't
-      // a repo, but that's a caller error (same as other gh invocations).
-      nodeIdCmd = `gh pr view ${number} --json id --jq '.id'`;
+      nodeIdArgv = ['gh', 'pr', 'view', String(number), '--json', 'id', '--jq', '.id'];
     }
   }
-  const prId = exec(nodeIdCmd).trim();
+  const nodeIdResult = runArgv(nodeIdArgv, cwd);
+  if (nodeIdResult.exitCode !== 0) {
+    throw new Error(`gh node_id lookup failed: ${nodeIdResult.stderr || nodeIdResult.stdout}`);
+  }
+  const prId = nodeIdResult.stdout.trim();
 
   // Step 2: invoke the enqueuePullRequest mutation
   const mutation = `mutation($prId:ID!){enqueuePullRequest(input:{pullRequestId:$prId}){mergeQueueEntry{position}}}`;
-  const repoFlag = repo !== undefined ? `--repo ${repo}` : '';
-  const graphqlCmd = `gh api graphql -f query='${mutation}' -f prId="${prId}" ${repoFlag}`.trim();
-  const response = exec(graphqlCmd);
+  const graphqlArgv = ['gh', 'api', 'graphql', '-f', `query=${mutation}`, '-f', `prId=${prId}`];
+  if (repo !== undefined) {
+    graphqlArgv.push('--repo', repo);
+  }
+  const graphqlResult = runArgv(graphqlArgv, cwd);
+  if (graphqlResult.exitCode !== 0) {
+    throw new Error(`gh graphql enqueue failed: ${graphqlResult.stderr || graphqlResult.stdout}`);
+  }
 
   // Parse the response to extract the queue position
   try {
-    const data = JSON.parse(response);
+    const data = JSON.parse(graphqlResult.stdout);
     return data?.data?.enqueuePullRequest?.mergeQueueEntry?.position ?? null;
   } catch {
     return null;