Ensure that copied extensions are writable in the user's local directory (#23016)

packages/cli/src/config/extension-manager-permissions.test.ts

@@ -0,0 +1,133 @@
+/**
+ * @license
+ * Copyright 2026 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import * as path from 'node:path';
+import { copyExtension } from './extension-manager.js';
+
+describe('copyExtension permissions', () => {
+  let tempDir: string;
+  let sourceDir: string;
+  let destDir: string;
+
+  beforeEach(() => {
+    tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gemini-permission-test-'));
+    sourceDir = path.join(tempDir, 'source');
+    destDir = path.join(tempDir, 'dest');
+    fs.mkdirSync(sourceDir);
+  });
+
+  afterEach(() => {
+    // Ensure we can delete the temp directory by making everything writable again
+    const makeWritableSync = (p: string) => {
+      try {
+        const stats = fs.lstatSync(p);
+        fs.chmodSync(p, stats.mode | 0o700);
+        if (stats.isDirectory()) {
+          fs.readdirSync(p).forEach((child) =>
+            makeWritableSync(path.join(p, child)),
+          );
+        }
+      } catch (_e) {
+        // Ignore errors during cleanup
+      }
+    };
+
+    if (fs.existsSync(tempDir)) {
+      makeWritableSync(tempDir);
+      fs.rmSync(tempDir, { recursive: true, force: true });
+    }
+  });
+
+  it('should make destination writable even if source is read-only', async () => {
+    const fileName = 'test.txt';
+    const filePath = path.join(sourceDir, fileName);
+    fs.writeFileSync(filePath, 'hello');
+
+    // Make source read-only: 0o555 for directory, 0o444 for file
+    fs.chmodSync(filePath, 0o444);
+    fs.chmodSync(sourceDir, 0o555);
+
+    // Verify source is read-only
+    expect(() => fs.writeFileSync(filePath, 'fail')).toThrow();
+
+    // Perform copy
+    await copyExtension(sourceDir, destDir);
+
+    // Verify destination is writable
+    const destFilePath = path.join(destDir, fileName);
+    const destFileStats = fs.statSync(destFilePath);
+    const destDirStats = fs.statSync(destDir);
+
+    // Check that owner write bits are set (0o200)
+    expect(destFileStats.mode & 0o200).toBe(0o200);
+    expect(destDirStats.mode & 0o200).toBe(0o200);
+
+    // Verify we can actually write to the destination file
+    fs.writeFileSync(destFilePath, 'writable');
+    expect(fs.readFileSync(destFilePath, 'utf-8')).toBe('writable');
+
+    // Verify we can delete the destination (which requires write bit on destDir)
+    fs.rmSync(destFilePath);
+    expect(fs.existsSync(destFilePath)).toBe(false);
+  });
+
+  it('should handle nested directories with restrictive permissions', async () => {
+    const subDir = path.join(sourceDir, 'subdir');
+    fs.mkdirSync(subDir);
+    const fileName = 'nested.txt';
+    const filePath = path.join(subDir, fileName);
+    fs.writeFileSync(filePath, 'nested content');
+
+    // Make nested structure read-only
+    fs.chmodSync(filePath, 0o444);
+    fs.chmodSync(subDir, 0o555);
+    fs.chmodSync(sourceDir, 0o555);
+
+    // Perform copy
+    await copyExtension(sourceDir, destDir);
+
+    // Verify nested destination is writable
+    const destSubDir = path.join(destDir, 'subdir');
+    const destFilePath = path.join(destSubDir, fileName);
+
+    expect(fs.statSync(destSubDir).mode & 0o200).toBe(0o200);
+    expect(fs.statSync(destFilePath).mode & 0o200).toBe(0o200);
+
+    // Verify we can delete the whole destination tree
+    await fs.promises.rm(destDir, { recursive: true, force: true });
+    expect(fs.existsSync(destDir)).toBe(false);
+  });
+
+  it('should not follow symlinks or modify symlink targets', async () => {
+    const symlinkTarget = path.join(tempDir, 'external-target');
+    fs.writeFileSync(symlinkTarget, 'external content');
+    // Target is read-only
+    fs.chmodSync(symlinkTarget, 0o444);
+
+    const symlinkPath = path.join(sourceDir, 'symlink-file');
+    fs.symlinkSync(symlinkTarget, symlinkPath);
+
+    // Perform copy
+    await copyExtension(sourceDir, destDir);
+
+    const destSymlinkPath = path.join(destDir, 'symlink-file');
+    const destSymlinkStats = fs.lstatSync(destSymlinkPath);
+
+    // Verify it is still a symlink in the destination
+    expect(destSymlinkStats.isSymbolicLink()).toBe(true);
+
+    // Verify the target (external to the extension) was NOT modified
+    const targetStats = fs.statSync(symlinkTarget);
+    // Owner write bit should still NOT be set (0o200)
+    expect(targetStats.mode & 0o200).toBe(0o000);
+
+    // Clean up
+    fs.chmodSync(symlinkTarget, 0o644);
+  });
+});

packages/cli/src/config/extension-manager-skills.test.ts

@@ -15,6 +15,10 @@ import { createExtension } from '../test-utils/createExtension.js';
 import { EXTENSIONS_DIRECTORY_NAME } from './extensions/variables.js';
 
 const mockHomedir = vi.hoisted(() => vi.fn(() => '/tmp/mock-home'));
+const mockIntegrityManager = vi.hoisted(() => ({
+  verify: vi.fn().mockResolvedValue('verified'),
+  store: vi.fn().mockResolvedValue(undefined),
+}));
 
 vi.mock('node:os', async (importOriginal) => {
   const actual = await importOriginal<typeof import('node:os')>();
@@ -31,6 +35,9 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
   return {
     ...actual,
     homedir: mockHomedir,
+    ExtensionIntegrityManager: vi
+      .fn()
+      .mockImplementation(() => mockIntegrityManager),
     loadAgentsFromDirectory: vi
       .fn()
       .mockImplementation(async () => ({ agents: [], errors: [] })),
@@ -64,6 +71,7 @@ describe('ExtensionManager skills validation', () => {
       requestConsent: vi.fn().mockResolvedValue(true),
       requestSetting: vi.fn(),
       workspaceDir: tempDir,
+      integrityManager: mockIntegrityManager,
     });
   });
 
@@ -139,6 +147,7 @@ describe('ExtensionManager skills validation', () => {
       requestConsent: vi.fn().mockResolvedValue(true),
       requestSetting: vi.fn(),
       workspaceDir: tempDir,
+      integrityManager: mockIntegrityManager,
     });
 
     // 4. Load extensions

packages/cli/src/config/extension-manager.ts

@@ -1248,11 +1248,32 @@ function filterMcpConfig(original: MCPServerConfig): MCPServerConfig {
   return Object.freeze(rest);
 }
 
+/**
+ * Recursively ensures that the owner has write permissions for all files
+ * and directories within the target path.
+ */
+async function makeWritableRecursive(targetPath: string): Promise<void> {
+  const stats = await fs.promises.lstat(targetPath);
+
+  if (stats.isDirectory()) {
+    // Ensure directory is rwx for the owner (0o700)
+    await fs.promises.chmod(targetPath, stats.mode | 0o700);
+    const children = await fs.promises.readdir(targetPath);
+    for (const child of children) {
+      await makeWritableRecursive(path.join(targetPath, child));
+    }
+  } else if (stats.isFile()) {
+    // Ensure file is rw for the owner (0o600)
+    await fs.promises.chmod(targetPath, stats.mode | 0o600);
+  }
+}
+
 export async function copyExtension(
   source: string,
   destination: string,
 ): Promise<void> {
   await fs.promises.cp(source, destination, { recursive: true });
+  await makeWritableRecursive(destination);
 }
 
 function getContextFileNames(config: ExtensionConfig): string[] {

packages/cli/src/config/extensions/extensionUpdates.test.ts

@@ -36,6 +36,8 @@ vi.mock('node:fs', async (importOriginal) => {
       rm: vi.fn(),
       cp: vi.fn(),
       readFile: vi.fn(),
+      lstat: vi.fn(),
+      chmod: vi.fn(),
     },
   };
 });
@@ -143,6 +145,11 @@ describe('extensionUpdates', () => {
     vi.mocked(fs.promises.rm).mockResolvedValue(undefined);
     vi.mocked(fs.promises.cp).mockResolvedValue(undefined);
     vi.mocked(fs.promises.readdir).mockResolvedValue([]);
+    vi.mocked(fs.promises.lstat).mockResolvedValue({
+      isDirectory: () => true,
+      mode: 0o755,
+    } as unknown as fs.Stats);
+    vi.mocked(fs.promises.chmod).mockResolvedValue(undefined);
     vi.mocked(isWorkspaceTrusted).mockReturnValue({
       isTrusted: true,
       source: 'file',