From 413098e03a9f2141dba6f912284db11f3d9d2e53 Mon Sep 17 00:00:00 2001
From: jacob314 <jacob314@gmail.com>
Date: Tue, 21 Apr 2026 15:46:16 -0700
Subject: [PATCH] fix(shell): address rcFile security issues

- Only source rcFile if the workspace is a trusted folder

- Properly escape the rcFile path to prevent command injection
---
 packages/core/src/tools/shell.test.ts | 50 +++++++++++++++++++++++++--
 packages/core/src/tools/shell.ts      | 26 ++++++++++----
 2 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts
index 17e78336ba..a54ad98993 100644
--- a/packages/core/src/tools/shell.test.ts
+++ b/packages/core/src/tools/shell.test.ts
@@ -165,6 +165,7 @@ describe('ShellTool', () => {
         addSessionApproval: vi.fn(),
       },
       getShellToolRcFile: vi.fn().mockReturnValue(undefined),
+      isTrustedFolder: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     const bus = createMockMessageBus();
@@ -487,9 +488,10 @@ EOF`;
       expect(mockShellExecutionService.mock.calls[0][0]).toMatch(/\nEOF\n\)\n/);
     });
 
-    it('should source rcfile when shellToolRcFile setting is present', async () => {
+    it('should source rcfile when shellToolRcFile setting is present and folder is trusted', async () => {
       const rcFilePath = '~/.geminirc';
       (mockConfig.getShellToolRcFile as Mock).mockReturnValue(rcFilePath);
+      (mockConfig.isTrustedFolder as Mock).mockReturnValue(true);
 
       const invocation = shellTool.build({ command: 'my-command' });
       const promise = invocation.execute({ abortSignal: mockAbortSignal });
@@ -497,7 +499,51 @@ EOF`;
       await promise;
 
       expect(mockShellExecutionService).toHaveBeenCalledWith(
-        expect.stringContaining(`source ${rcFilePath} && my-command`),
+        expect.stringContaining(
+          `source '${rcFilePath.replace('~', '/home/user')}'; export PAGER=cat`,
+        ),
+        expect.any(String),
+        expect.any(Function),
+        expect.any(AbortSignal),
+        false,
+        expect.any(Object),
+      );
+    });
+
+    it('should NOT source rcfile when shellToolRcFile setting is present but folder is untrusted', async () => {
+      const rcFilePath = '~/.geminirc';
+      (mockConfig.getShellToolRcFile as Mock).mockReturnValue(rcFilePath);
+      (mockConfig.isTrustedFolder as Mock).mockReturnValue(false);
+
+      const invocation = shellTool.build({ command: 'my-command' });
+      const promise = invocation.execute({ abortSignal: mockAbortSignal });
+      resolveShellExecution();
+      await promise;
+
+      expect(mockShellExecutionService).not.toHaveBeenCalledWith(
+        expect.stringContaining(`source '${rcFilePath}'`),
+        expect.any(String),
+        expect.any(Function),
+        expect.any(AbortSignal),
+        false,
+        expect.any(Object),
+      );
+    });
+
+    it('should properly escape quotes in rcFilePath', async () => {
+      const rcFilePath = "/path/with/'quotes'/rc";
+      (mockConfig.getShellToolRcFile as Mock).mockReturnValue(rcFilePath);
+      (mockConfig.isTrustedFolder as Mock).mockReturnValue(true);
+
+      const invocation = shellTool.build({ command: 'my-command' });
+      const promise = invocation.execute({ abortSignal: mockAbortSignal });
+      resolveShellExecution();
+      await promise;
+
+      expect(mockShellExecutionService).toHaveBeenCalledWith(
+        expect.stringContaining(
+          `source '/path/with/'\\''quotes'\\''/rc'; export PAGER=cat`,
+        ),
         expect.any(String),
         expect.any(Function),
         expect.any(AbortSignal),
diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts
index 049a03e369..a6837f8eea 100644
--- a/packages/core/src/tools/shell.ts
+++ b/packages/core/src/tools/shell.ts
@@ -50,7 +50,12 @@ import type { MessageBus } from '../confirmation-bus/message-bus.js';
 import { getShellDefinition } from './definitions/coreTools.js';
 import { resolveToolDeclaration } from './definitions/resolver.js';
 import type { AgentLoopContext } from '../config/agent-loop-context.js';
-import { toPathKey, isSubpath, resolveToRealPath } from '../utils/paths.js';
+import {
+  homedir,
+  toPathKey,
+  isSubpath,
+  resolveToRealPath,
+} from '../utils/paths.js';
 import {
   getProactiveToolSuggestions,
   isNetworkReliantCommand,
@@ -463,11 +468,20 @@ export class ShellToolInvocation extends BaseToolInvocation<
 
     const onAbort = () => combinedController.abort();
     let strippedCommandWithRc = strippedCommand;
-    const rcFilePath = this.context.config.getShellToolRcFile();
-    if (rcFilePath) {
-      strippedCommandWithRc = `source ${rcFilePath} && ${strippedCommand}`;
-    } else if (!isWindows) {
-      strippedCommandWithRc = `export PAGER=cat GIT_PAGER=cat; more() { cat "$@"; }; less() { cat "$@"; }; ${strippedCommand}`;
+    if (!isWindows) {
+      strippedCommandWithRc = `export PAGER=cat GIT_PAGER=cat; ${strippedCommand}`;
+      const rcFilePath = this.context.config.getShellToolRcFile();
+      if (rcFilePath && this.context.config.isTrustedFolder()) {
+        let resolvedRcFilePath = rcFilePath;
+        if (rcFilePath === '~' || rcFilePath.startsWith('~/')) {
+          resolvedRcFilePath = homedir() + rcFilePath.substring(1);
+        }
+        const escapedRcFilePath = resolvedRcFilePath.replace(
+          /'/g,
+          () => "'\\''",
+        );
+        strippedCommandWithRc = `source '${escapedRcFilePath}'; ${strippedCommandWithRc}`;
+      }
     }
 
     try {