From dc71dcdfb7c4b9be4c8c736b394a266867ca0608 Mon Sep 17 00:00:00 2001
From: Test User <test@example.com>
Date: Sat, 27 Jun 2026 15:04:35 +0800
Subject: [PATCH] fix(tgit): shell-quote every gf CLI arg to prevent injection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

gfExec built `bash -c "<gfPath> <args.join(' ')>"`. Only the MR title and
description were shell-quoted (in gfMrCreate); repo, source/target branch,
reviewers, and the clone local-path were interpolated raw. A repo or
branch value containing shell metacharacters (`;`, `|`, `$()`, …) was
therefore executed by `bash -c`.

Example — gfMrCreate({ repo: "team/repo;echo PWNED", ... }) produced the
command:
  gf mr create -R team/repo;echo PWNED -s feat ...
which runs `echo PWNED` when gfExec spawns it. Verified with a real bash
invocation + fake gf: the old cmd executes the injected `echo` and gf
receives split args; the fixed cmd passes `team/repo;echo PWNED` as a
single literal argument and nothing extra runs.

Fix: shell-quote every token (including the binary path) centrally in
gfExec, and drop the now-redundant per-call shellQuote() in gfMrCreate.
The two-segment `gf repo clone` path through gfExec is covered by the
same change; the multi-segment git-clone path already used a spawnSync
argv array.

Added a regression test asserting repo/source/target metacharacters are
single-quoted as single tokens with no unquoted `;`/`|` left for bash.
Existing gf-cli tests (newline preservation, single-quote escaping, URL
extraction) still pass. `npm test` 1440 passed; `tsc --noEmit` clean.

Co-Authored-By: Claude <noreply@anthropic.com>
---
 src/__tests__/gf-cli.test.ts | 30 ++++++++++++++++++++++++++++++
 src/providers/tgit/gf-cli.ts | 10 +++++++---
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/src/__tests__/gf-cli.test.ts b/src/__tests__/gf-cli.test.ts
index 5757e2d..fad57cd 100644
--- a/src/__tests__/gf-cli.test.ts
+++ b/src/__tests__/gf-cli.test.ts
@@ -155,6 +155,36 @@ describe('gfMrCreate', () => {
     expect(cmd).toContain("'it'\\''s a\ndescription'");
   });
 
+  it('should shell-quote repo/branch args to prevent argument injection into bash -c', () => {
+    mockSpawnSync.mockReturnValue({
+      stdout: 'https://git.woa.com/team/repo/-/merge_requests/3',
+      stderr: '',
+      status: 0,
+    } as any);
+
+    gfMrCreate({
+      // Values with shell metacharacters that would break out of the command
+      // if interpolated raw into `bash -c "<gfPath> <args>"`.
+      repo: 'team/repo;echo PWNED',
+      source: 'feat;rm -rf / #',
+      target: 'master|cat /etc/passwd',
+      title: 't',
+    });
+
+    const cmd = mockSpawnSync.mock.calls[0][1]![1] as string;
+    // Each dangerous value must be a single single-quoted token so bash -c
+    // treats its metacharacters (`;`, `|`, `#`, spaces) as literal argument
+    // content rather than command separators.
+    expect(cmd).toContain("'team/repo;echo PWNED'");
+    expect(cmd).toContain("'feat;rm -rf / #'");
+    expect(cmd).toContain("'master|cat /etc/passwd'");
+    // No bare (unquoted) `;` that bash could interpret as a command separator:
+    // every `;` must sit inside a single-quoted token.
+    const outsideQuotes = cmd.replace(/'[^']*'/g, '');
+    expect(outsideQuotes).not.toContain(';');
+    expect(outsideQuotes).not.toContain('|');
+  });
+
   it('should return MR URL from gf output', () => {
     mockSpawnSync.mockReturnValue({
       stdout: 'Created: https://git.woa.com/team/repo/-/merge_requests/42',
diff --git a/src/providers/tgit/gf-cli.ts b/src/providers/tgit/gf-cli.ts
index 84b32de..8c3cc0c 100644
--- a/src/providers/tgit/gf-cli.ts
+++ b/src/providers/tgit/gf-cli.ts
@@ -35,7 +35,11 @@ export function gfExec(
   options?: { inheritStdio?: boolean; cwd?: string },
 ): { stdout: string; stderr: string; status: number } {
   const gfPath = getGfPath();
-  const cmd = `${gfPath} ${args.join(' ')}`;
+  // Shell-quote every token (including the binary path) so values such as repo
+  // paths, branch names, titles, and descriptions cannot inject shell
+  // metacharacters (`;`, `|`, `$()`, …) into the `bash -c` string. Callers pass
+  // raw values; quoting centrally here keeps every entry point safe.
+  const cmd = [shellQuote(gfPath), ...args.map(shellQuote)].join(' ');
   log.debug(`gf exec: ${cmd}`);
 
   if (options?.inheritStdio) {
@@ -398,11 +402,11 @@ export function gfMrCreate(opts: GfMrCreateOptions): string {
     '-R', opts.repo,
     '-s', opts.source,
     '-T', opts.target,
-    '-t', shellQuote(opts.title),
+    '-t', opts.title,
   ];
 
   if (opts.description) {
-    args.push('-d', shellQuote(opts.description));
+    args.push('-d', opts.description);
   }
 
   if (opts.reviewers && opts.reviewers.length > 0) {