Tighten private Auto Memory patch allowlist (#26535)

packages/core/src/commands/memory.test.ts

@@ -325,6 +325,8 @@ describe('memory commands', () => {
     let projectRoot: string;
     let globalMemoryDir: string;
     let patchConfig: Config;
+    const isCaseInsensitivePathPlatform =
+      process.platform === 'win32' || process.platform === 'darwin';
 
     function buildUpdatePatch(
       absoluteTargetPath: string,
@@ -372,6 +374,12 @@ describe('memory commands', () => {
       ].join('\n');
     }
 
+    function swapAsciiPathCase(filePath: string): string {
+      return filePath.replace(/[a-z]/gi, (char) =>
+        char === char.toLowerCase() ? char.toUpperCase() : char.toLowerCase(),
+      );
+    }
+
     beforeEach(async () => {
       tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'memory-patch-test-'));
       // Canonicalize so test-side paths match production's
@@ -466,6 +474,49 @@ describe('memory commands', () => {
       expect(result.message).toMatch(/outside the private memory root/i);
     });
 
+    it('rejects private patches that target in-root non-memory documents', async () => {
+      const patchDir = path.join(memoryTempDir, '.inbox', 'private');
+      await fs.mkdir(patchDir, { recursive: true });
+
+      const rejectedTargets = [
+        ['state.patch', path.join(memoryTempDir, '.extraction-state.json')],
+        ['lock.patch', path.join(memoryTempDir, '.extraction.lock')],
+        [
+          'inbox.patch',
+          path.join(memoryTempDir, '.inbox', 'private', 'review.md'),
+        ],
+        [
+          'skills.patch',
+          path.join(memoryTempDir, 'skills', 'generated', 'SKILL.md'),
+        ],
+        ['text.patch', path.join(memoryTempDir, 'notes.txt')],
+        ['nested.patch', path.join(memoryTempDir, 'nested', 'topic.md')],
+      ] as const;
+
+      for (const [fileName, targetPath] of rejectedTargets) {
+        await fs.writeFile(
+          path.join(patchDir, fileName),
+          buildCreationPatch(targetPath, 'rejected\n'),
+        );
+      }
+
+      const patches = await listInboxMemoryPatches(patchConfig);
+      expect(patches).toHaveLength(0);
+
+      for (const [fileName, targetPath] of rejectedTargets) {
+        const result = await applyInboxMemoryPatch(
+          patchConfig,
+          'private',
+          fileName,
+        );
+        expect(result.success).toBe(false);
+        expect(result.message).toMatch(
+          /outside the private memory root or target allowlist/i,
+        );
+        await expect(fs.access(targetPath)).rejects.toThrow();
+      }
+    });
+
     it('omits global patches with disallowed targets from the listing', async () => {
       // Same defense for the global tier: only ~/.gemini/GEMINI.md is allowed.
       // memory.md (legacy lowercase), sibling .md files, and settings.json all
@@ -490,6 +541,13 @@ describe('memory commands', () => {
         path.join(patchDir, 'settings.patch'),
         buildCreationPatch(path.join(globalMemoryDir, 'settings.json'), '{}\n'),
       );
+      await fs.writeFile(
+        path.join(patchDir, 'nested.patch'),
+        buildCreationPatch(
+          path.join(globalMemoryDir, 'GEMINI.md', 'nested.md'),
+          'rejected\n',
+        ),
+      );
 
       const patches = await listInboxMemoryPatches(patchConfig);
       expect(patches).toHaveLength(0);
@@ -519,6 +577,39 @@ describe('memory commands', () => {
       ).rejects.toThrow();
     });
 
+    it.runIf(isCaseInsensitivePathPlatform)(
+      'accepts private memory patch targets with different path casing',
+      async () => {
+        const target = path.join(memoryTempDir, 'MEMORY.md');
+        await fs.writeFile(target, '- old\n');
+
+        const patchDir = path.join(memoryTempDir, '.inbox', 'private');
+        await fs.mkdir(patchDir, { recursive: true });
+        await fs.writeFile(
+          path.join(patchDir, 'MEMORY.patch'),
+          buildUpdatePatch(
+            swapAsciiPathCase(target),
+            '- old\n',
+            '- accepted\n',
+          ),
+        );
+
+        const patches = await listInboxMemoryPatches(patchConfig);
+        expect(patches).toHaveLength(1);
+
+        const result = await applyInboxMemoryPatch(
+          patchConfig,
+          'private',
+          'MEMORY.patch',
+        );
+
+        expect(result.success).toBe(true);
+        await expect(fs.readFile(target, 'utf-8')).resolves.toBe(
+          '- accepted\n',
+        );
+      },
+    );
+
     it('applies a private creation patch with a paired MEMORY.md pointer', async () => {
       // The auto-memory contract: creating a sibling .md file requires a
       // hunk that adds a pointer to MEMORY.md (so the sibling becomes
@@ -713,6 +804,39 @@ describe('memory commands', () => {
       ).rejects.toThrow();
     });
 
+    it.runIf(isCaseInsensitivePathPlatform)(
+      'accepts global memory patch targets with different path casing',
+      async () => {
+        const target = path.join(globalMemoryDir, 'GEMINI.md');
+        await fs.writeFile(target, '- prefer X\n');
+
+        const patchDir = path.join(memoryTempDir, '.inbox', 'global');
+        await fs.mkdir(patchDir, { recursive: true });
+        await fs.writeFile(
+          path.join(patchDir, 'GEMINI.patch'),
+          buildUpdatePatch(
+            swapAsciiPathCase(target),
+            '- prefer X\n',
+            '- prefer Y\n',
+          ),
+        );
+
+        const patches = await listInboxMemoryPatches(patchConfig);
+        expect(patches).toHaveLength(1);
+
+        const result = await applyInboxMemoryPatch(
+          patchConfig,
+          'global',
+          'GEMINI.patch',
+        );
+
+        expect(result.success).toBe(true);
+        await expect(fs.readFile(target, 'utf-8')).resolves.toBe(
+          '- prefer Y\n',
+        );
+      },
+    );
+
     it('dismisses a single memory patch from the inbox (legacy single-file mode)', async () => {
       const patchDir = path.join(memoryTempDir, '.inbox', 'global');
       await fs.mkdir(patchDir, { recursive: true });
@@ -871,10 +995,20 @@ describe('memory commands', () => {
         ),
       );
 
+      // Child paths under the single allowed file path are not allowed either.
+      await fs.writeFile(
+        path.join(patchDir, 'nested.patch'),
+        buildCreationPatch(
+          path.join(globalMemoryDir, 'GEMINI.md', 'nested.md'),
+          'Should be rejected.\n',
+        ),
+      );
+
       for (const fileName of [
         'wrong-name.patch',
         'sibling.patch',
         'settings.patch',
+        'nested.patch',
       ]) {
         const result = await applyInboxMemoryPatch(
           patchConfig,
@@ -891,6 +1025,9 @@ describe('memory commands', () => {
           fs.access(path.join(globalMemoryDir, orphan)),
         ).rejects.toThrow();
       }
+      await expect(
+        fs.access(path.join(globalMemoryDir, 'GEMINI.md', 'nested.md')),
+      ).rejects.toThrow();
     });
 
     it('rejects invalid memory patch paths', async () => {

packages/core/src/commands/memory.ts

@@ -13,7 +13,11 @@ import type { Config } from '../config/config.js';
 import { Storage } from '../config/storage.js';
 import { flattenMemory } from '../config/memory.js';
 import { loadSkillFromFile, loadSkillsFromDir } from '../skills/skillLoader.js';
-import { getGlobalMemoryFilePath } from '../tools/memoryTool.js';
+import {
+  getGlobalMemoryFilePath,
+  PROJECT_MEMORY_INDEX_FILENAME,
+} from '../tools/memoryTool.js';
+import { isSubpath } from '../utils/paths.js';
 import {
   type AppliedSkillPatchTarget,
   applyParsedPatchesWithAllowedRoots,
@@ -424,11 +428,7 @@ function getMemoryPatchRoot(
 }
 
 function isSubpathOrSame(childPath: string, parentPath: string): boolean {
-  const relativePath = path.relative(parentPath, childPath);
-  return (
-    relativePath === '' ||
-    (!relativePath.startsWith('..') && !path.isAbsolute(relativePath))
-  );
+  return isSubpath(parentPath, childPath);
 }
 
 function normalizeInboxMemoryPatchPath(
@@ -455,11 +455,11 @@ function normalizeInboxMemoryPatchPath(
 }
 
 /**
- * Returns the directory roots (or single-file allowlists) that a memory patch
- * of the given kind is allowed to modify. Memory patch headers must reference
- * paths inside / equal to one of these entries after canonical resolution.
+ * Returns coarse directory roots (or single-file roots) used for canonical
+ * containment checks before the kind-specific target validator runs.
  *
- * - `private` allows any markdown file inside the project memory directory.
+ * - `private` is rooted at the project memory directory, then narrowed to
+ *   direct memory markdown documents by `isAllowedPrivateMemoryDocumentPath`.
  * - `global` is intentionally a single-file allowlist: the only writeable
  *   global file is the personal `~/.gemini/GEMINI.md`. Other files under
  *   `~/.gemini/` (settings, credentials, oauth, keybindings, etc.) are off-limits.
@@ -478,6 +478,178 @@ export function getAllowedMemoryPatchRoots(
   }
 }
 
+interface MemoryPatchTargetValidationContext {
+  kind: InboxMemoryPatchKind;
+  allowedRoots: string[];
+  privateMemoryDirs: string[];
+  globalMemoryFiles: string[];
+}
+
+function hasMarkdownExtension(fileName: string): boolean {
+  return fileName.toLowerCase().endsWith('.md');
+}
+
+function isAllowedPrivateMemoryFileName(fileName: string): boolean {
+  if (fileName === PROJECT_MEMORY_INDEX_FILENAME) {
+    return true;
+  }
+  return !fileName.startsWith('.') && hasMarkdownExtension(fileName);
+}
+
+function uniqueResolvedPaths(paths: readonly string[]): string[] {
+  return Array.from(new Set(paths.map((filePath) => path.resolve(filePath))));
+}
+
+function isSamePath(leftPath: string, rightPath: string): boolean {
+  return isSubpath(leftPath, rightPath) && isSubpath(rightPath, leftPath);
+}
+
+function includesSamePath(
+  paths: readonly string[],
+  targetPath: string,
+): boolean {
+  return paths.some((candidate) => isSamePath(candidate, targetPath));
+}
+
+function isAllowedPrivateMemoryDocumentPath(
+  targetPath: string,
+  memoryDirs: readonly string[],
+): boolean {
+  const resolvedTargetPath = path.resolve(targetPath);
+  const targetDir = path.dirname(resolvedTargetPath);
+  if (!includesSamePath(memoryDirs, targetDir)) {
+    return false;
+  }
+  return isAllowedPrivateMemoryFileName(path.basename(resolvedTargetPath));
+}
+
+function isAllowedGlobalMemoryDocumentPath(
+  targetPath: string,
+  globalMemoryFiles: readonly string[],
+): boolean {
+  const resolvedTargetPath = path.resolve(targetPath);
+  return includesSamePath(globalMemoryFiles, resolvedTargetPath);
+}
+
+async function getMemoryPatchTargetValidationContext(
+  config: Config,
+  kind: InboxMemoryPatchKind,
+): Promise<MemoryPatchTargetValidationContext> {
+  const allowedRoots = await canonicalizeAllowedPatchRoots(
+    getAllowedMemoryPatchRoots(config, kind),
+  );
+
+  if (kind === 'global') {
+    const rawGlobalMemoryFile = path.resolve(getGlobalMemoryFilePath());
+    const canonicalGlobalMemoryFiles = await canonicalizeAllowedPatchRoots([
+      rawGlobalMemoryFile,
+    ]);
+    return {
+      kind,
+      allowedRoots,
+      privateMemoryDirs: [],
+      globalMemoryFiles: uniqueResolvedPaths([
+        rawGlobalMemoryFile,
+        ...canonicalGlobalMemoryFiles,
+      ]),
+    };
+  }
+
+  const rawPrivateMemoryDir = path.resolve(
+    config.storage.getProjectMemoryTempDir(),
+  );
+  const canonicalPrivateMemoryDirs = await canonicalizeAllowedPatchRoots([
+    rawPrivateMemoryDir,
+  ]);
+  const privateMemoryDirs = uniqueResolvedPaths([
+    rawPrivateMemoryDir,
+    ...canonicalPrivateMemoryDirs,
+  ]);
+
+  return { kind, allowedRoots, privateMemoryDirs, globalMemoryFiles: [] };
+}
+
+function isResolvedMemoryPatchTargetAllowed(
+  resolvedTargetPath: string,
+  context: MemoryPatchTargetValidationContext,
+): boolean {
+  if (context.kind === 'global') {
+    return isAllowedGlobalMemoryDocumentPath(
+      resolvedTargetPath,
+      context.globalMemoryFiles,
+    );
+  }
+  if (context.kind === 'private') {
+    return isAllowedPrivateMemoryDocumentPath(
+      resolvedTargetPath,
+      context.privateMemoryDirs,
+    );
+  }
+  return true;
+}
+
+async function resolveMemoryPatchTargetWithinAllowedSet(
+  targetPath: string,
+  context: MemoryPatchTargetValidationContext,
+): Promise<string | undefined> {
+  const resolvedTargetPath = await resolveTargetWithinAllowedRoots(
+    targetPath,
+    context.allowedRoots,
+  );
+  if (!resolvedTargetPath) {
+    return undefined;
+  }
+  if (
+    context.kind === 'private' &&
+    (!isAllowedPrivateMemoryDocumentPath(
+      targetPath,
+      context.privateMemoryDirs,
+    ) ||
+      !isAllowedPrivateMemoryDocumentPath(
+        resolvedTargetPath,
+        context.privateMemoryDirs,
+      ))
+  ) {
+    return undefined;
+  }
+  if (
+    context.kind === 'global' &&
+    (!isAllowedGlobalMemoryDocumentPath(
+      targetPath,
+      context.globalMemoryFiles,
+    ) ||
+      !isAllowedGlobalMemoryDocumentPath(
+        resolvedTargetPath,
+        context.globalMemoryFiles,
+      ))
+  ) {
+    return undefined;
+  }
+  return resolvedTargetPath;
+}
+
+async function findDisallowedMemoryPatchTarget(
+  parsedPatches: Diff.StructuredPatch[],
+  context: MemoryPatchTargetValidationContext,
+): Promise<string | undefined> {
+  const validated = validateParsedSkillPatchHeaders(parsedPatches);
+  if (!validated.success) {
+    return undefined;
+  }
+
+  for (const header of validated.patches) {
+    if (
+      !(await resolveMemoryPatchTargetWithinAllowedSet(
+        header.targetPath,
+        context,
+      ))
+    ) {
+      return header.targetPath;
+    }
+  }
+  return undefined;
+}
+
 async function getFileMtimeIso(filePath: string): Promise<string | undefined> {
   try {
     const stats = await fs.stat(filePath);
@@ -585,8 +757,8 @@ async function listInboxPatchFiles(
 
 /**
  * Returns only the inbox patch files that pass the same validation as the
- * inbox listing (parseable, has hunks, valid headers, targets in the
- * kind's allowed root). Used by aggregate apply so the user only ever sees
+ * inbox listing (parseable, has hunks, valid headers, targets in the kind's
+ * allowed target set). Used by aggregate apply so the user only ever sees
  * results for patches the inbox actually surfaced.
  */
 async function listValidInboxPatchFiles(
@@ -598,8 +770,9 @@ async function listValidInboxPatchFiles(
     return [];
   }
 
-  const allowedRoots = await canonicalizeAllowedPatchRoots(
-    getAllowedMemoryPatchRoots(config, kind),
+  const validationContext = await getMemoryPatchTargetValidationContext(
+    config,
+    kind,
   );
 
   const valid: string[] = [];
@@ -629,9 +802,9 @@ async function listValidInboxPatchFiles(
     const targetsAllAllowed = await Promise.all(
       validated.patches.map(
         async (header) =>
-          (await resolveTargetWithinAllowedRoots(
+          (await resolveMemoryPatchTargetWithinAllowedSet(
             header.targetPath,
-            allowedRoots,
+            validationContext,
           )) !== undefined,
       ),
     );
@@ -648,8 +821,8 @@ async function listValidInboxPatchFiles(
  * Scans `<memoryDir>/.inbox/{private,global}/` and returns ONE consolidated
  * inbox entry per kind. Each entry aggregates all hunks from every valid
  * underlying `.patch` file. Patches that fail validation (unparseable, no
- * hunks, target outside allowed root) are silently skipped so they don't
- * pollute the inbox UI.
+ * hunks, target outside the allowed target set) are silently skipped so they
+ * don't pollute the inbox UI.
  */
 export async function listInboxMemoryPatches(
   config: Config,
@@ -658,8 +831,9 @@ export async function listInboxMemoryPatches(
   const aggregated: InboxMemoryPatch[] = [];
 
   for (const kind of kinds) {
-    const allowedRoots = await canonicalizeAllowedPatchRoots(
-      getAllowedMemoryPatchRoots(config, kind),
+    const validationContext = await getMemoryPatchTargetValidationContext(
+      config,
+      kind,
     );
     const patchFiles = await listInboxPatchFiles(config, kind);
 
@@ -691,13 +865,13 @@ export async function listInboxMemoryPatches(
       }
 
       // Skip the entire source file if ANY of its targets escapes the kind's
-      // allowed root.
+      // allowed target set.
       const targetsAllAllowed = await Promise.all(
         validated.patches.map(
           async (header) =>
-            (await resolveTargetWithinAllowedRoots(
+            (await resolveMemoryPatchTargetWithinAllowedSet(
               header.targetPath,
-              allowedRoots,
+              validationContext,
             )) !== undefined,
         ),
       );
@@ -1015,12 +1189,31 @@ async function applyMemoryPatchFile(
     };
   }
 
-  const allowedRoots = await canonicalizeAllowedPatchRoots(
-    getAllowedMemoryPatchRoots(config, kind),
+  const validationContext = await getMemoryPatchTargetValidationContext(
+    config,
+    kind,
   );
+  const disallowedTargetPath = await findDisallowedMemoryPatchTarget(
+    parsed,
+    validationContext,
+  );
+  if (disallowedTargetPath) {
+    return {
+      success: false,
+      message: `Memory patch "${displayName}" targets a file outside the ${kind} memory root or target allowlist: ${disallowedTargetPath}`,
+    };
+  }
+
   const applied = await applyParsedPatchesWithAllowedRoots(
     parsed,
-    allowedRoots,
+    validationContext.allowedRoots,
+    {
+      isResolvedTargetAllowed: (resolvedTargetPath) =>
+        isResolvedMemoryPatchTargetAllowed(
+          resolvedTargetPath,
+          validationContext,
+        ),
+    },
   );
   if (!applied.success) {
     switch (applied.reason) {
@@ -1037,7 +1230,7 @@ async function applyMemoryPatchFile(
       case 'outsideAllowedRoots':
         return {
           success: false,
-          message: `Memory patch "${displayName}" targets a file outside the ${kind} memory root: ${applied.targetPath}`,
+          message: `Memory patch "${displayName}" targets a file outside the ${kind} memory root or target allowlist: ${applied.targetPath}`,
         };
       case 'newFileAlreadyExists':
         return {

packages/core/src/services/memoryPatchUtils.ts

@@ -252,15 +252,24 @@ export async function applyParsedSkillPatches(
   return applyParsedPatchesWithAllowedRoots(parsedPatches, allowedRoots);
 }
 
+export interface ApplyParsedPatchesWithAllowedRootsOptions {
+  /**
+   * Optional fine-grained allowlist for callers whose allowed root is broader
+   * than their actual target surface. Receives the canonical target path after
+   * root containment has already passed.
+   */
+  isResolvedTargetAllowed?: (resolvedTargetPath: string) => boolean;
+}
+
 /**
  * Applies parsed unified diff patches against any caller-supplied set of
  * allowed root directories. This is the kind-agnostic core used by both the
  * skill patch flow and the memory patch flow.
  *
  * The patch headers must reference absolute paths inside one of the allowed
- * roots (after canonical resolution). Update patches must reference an
- * existing target; creation patches (`/dev/null` source) must reference a path
- * that does not yet exist.
+ * roots (after canonical resolution) and pass any caller-supplied fine-grained
+ * target predicate. Update patches must reference an existing target; creation
+ * patches (`/dev/null` source) must reference a path that does not yet exist.
  *
  * Returns the per-target before/after content so callers can stage commits
  * and roll back on failure.
@@ -268,6 +277,7 @@ export async function applyParsedSkillPatches(
 export async function applyParsedPatchesWithAllowedRoots(
   parsedPatches: StructuredPatch[],
   allowedRoots: string[],
+  options: ApplyParsedPatchesWithAllowedRootsOptions = {},
 ): Promise<ApplyParsedSkillPatchesResult> {
   const results = new Map<string, AppliedSkillPatchTarget>();
   const patchedContentByTarget = new Map<string, string>();
@@ -285,7 +295,11 @@ export async function applyParsedPatchesWithAllowedRoots(
       targetPath,
       allowedRoots,
     );
-    if (!resolvedTargetPath) {
+    if (
+      !resolvedTargetPath ||
+      (options.isResolvedTargetAllowed &&
+        !options.isResolvedTargetAllowed(resolvedTargetPath))
+    ) {
       return {
         success: false,
         reason: 'outsideAllowedRoots',