Merge PR #6733: fix(coding-agent): safely glob memory directories (@usr-bin-roygbiv)

packages/coding-agent/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- Fixed `glob` rejecting safe `memory://root/<directory>/**` patterns. Memory globs now resolve their directory prefix inside the project memory root while rejecting traversal and percent-encoded path separators across the complete glob path.
+
 ## [17.1.4] - 2026-07-26
 
 ### Added

packages/coding-agent/src/internal-urls/memory-protocol.ts

@@ -6,6 +6,7 @@ import { getMnemopiSessionState, type MnemopiScopedMemoryHit, type MnemopiSessio
 import { AgentRegistry } from "../registry/agent-registry";
 import { isMarkdownPath } from "../utils/lang-from-path";
 import { buildDirectoryResource } from "./filesystem-resource";
+import { parseInternalUrl } from "./parse";
 import { validateRelativePath } from "./skill-protocol";
 import type { InternalResource, InternalUrl, ProtocolHandler, ResolveContext, UrlCompletion } from "./types";
 
@@ -45,6 +46,62 @@ function toMemoryValidationError(error: unknown): Error {
 	return new Error(message.replace("skill://", "memory://"));
 }
 
+export interface MemoryGlobPattern {
+	baseUrl: string;
+	globPattern: string;
+}
+
+/**
+ * Split a memory:// glob at its first wildcard after validating the complete
+ * decoded path. The suffix is validated before filesystem globbing so `..`
+ * cannot escape a safely resolved base directory.
+ */
+export function splitMemoryGlobPattern(input: string): MemoryGlobPattern {
+	const urlMatch = input.match(/^([a-z][a-z0-9+.-]*:\/\/[^/?#]*)(\/.*)?$/i);
+	if (!urlMatch) {
+		throw new Error(`Invalid memory glob URL: ${input}`);
+	}
+
+	// Parse only the scheme and authority. A literal `?` in the path is glob
+	// syntax, not a query delimiter, and must survive unchanged.
+	const url = parseInternalUrl(urlMatch[1]);
+	const namespace = url.rawHost || url.hostname;
+	if (url.protocol !== "memory:" || namespace !== MEMORY_NAMESPACE) {
+		throw new Error(`Memory glob patterns require the ${MEMORY_NAMESPACE} namespace: ${input}`);
+	}
+
+	const rawPathname = urlMatch[2] ?? "";
+	if (/%(?:2f|5c)/i.test(rawPathname)) {
+		throw new Error(`Encoded path separators are not allowed in memory:// glob patterns: ${input}`);
+	}
+
+	let relativePath: string;
+	try {
+		relativePath = decodeURIComponent(rawPathname.replace(/^\//, ""));
+	} catch {
+		throw new Error(`Invalid URL encoding in memory:// path: ${input}`);
+	}
+
+	try {
+		validateRelativePath(relativePath);
+	} catch (error) {
+		throw toMemoryValidationError(error);
+	}
+
+	const rawSegments = rawPathname.replace(/^\//, "").split("/");
+	const firstGlobIndex = rawSegments.findIndex(segment => ["*", "?", "[", "{"].some(char => segment.includes(char)));
+	if (firstGlobIndex === -1) {
+		throw new Error(`memory:// URL does not contain a glob pattern: ${input}`);
+	}
+
+	const decodedSegments = relativePath.split("/");
+	const rawBasePath = rawSegments.slice(0, firstGlobIndex).join("/") || ".";
+	return {
+		baseUrl: `memory://${namespace}/${rawBasePath}`,
+		globPattern: decodedSegments.slice(firstGlobIndex).join("/"),
+	};
+}
+
 /**
  * Resolve a memory:// URL to an absolute filesystem path under memory root.
  */
@@ -91,12 +148,14 @@ async function tryResolveInRoot(url: InternalUrl, memoryRoot: string): Promise<I
 	const targetPath = resolveMemoryUrlToPath(url, resolvedRoot);
 	ensureWithinRoot(targetPath, resolvedRoot);
 
-	const parentDir = path.dirname(targetPath);
-	try {
-		const realParent = await fs.realpath(parentDir);
-		ensureWithinRoot(realParent, resolvedRoot);
-	} catch (error) {
-		if (!isEnoent(error)) throw error;
+	if (targetPath !== resolvedRoot) {
+		const parentDir = path.dirname(targetPath);
+		try {
+			const realParent = await fs.realpath(parentDir);
+			ensureWithinRoot(realParent, resolvedRoot);
+		} catch (error) {
+			if (!isEnoent(error)) throw error;
+		}
 	}
 
 	let realTargetPath: string;

packages/coding-agent/src/internal-urls/skill-protocol.ts

@@ -30,7 +30,12 @@ export function validateRelativePath(relativePath: string): void {
 	}
 
 	const normalized = path.normalize(relativePath);
-	if (normalized.startsWith("..") || normalized.includes("/../") || normalized.includes("/..")) {
+	if (
+		relativePath.split(/[\\/]/).includes("..") ||
+		normalized.startsWith("..") ||
+		normalized.includes("/../") ||
+		normalized.includes("/..")
+	) {
 		throw new Error("Path traversal (..) is not allowed in skill:// URLs");
 	}
 }

packages/coding-agent/src/tools/glob.ts

@@ -9,6 +9,7 @@ import { formatGroupedPaths, isEnoent, prompt, untilAborted } from "@oh-my-pi/pi
 import { type } from "arktype";
 import type { RenderResultOptions } from "../extensibility/custom-tools/types";
 import { InternalUrlRouter } from "../internal-urls";
+import { splitMemoryGlobPattern } from "../internal-urls/memory-protocol";
 import type { Theme } from "../modes/theme/theme";
 import globDescription from "../prompts/tools/glob.md" with { type: "text" };
 import { type TruncationResult, truncateHead } from "../session/streaming-output";
@@ -176,7 +177,25 @@ export class GlobTool implements AgentTool<typeof findSchema, GlobToolDetails> {
 					);
 				}
 				if (hasGlobPathChars(rawPattern)) {
-					throw new ToolError(`Glob patterns are not supported for internal URLs: ${rawPattern}`);
+					if (!/^memory:\/\//i.test(rawPattern)) {
+						throw new ToolError(`Glob patterns are not supported for internal URLs: ${rawPattern}`);
+					}
+					const memoryGlob = splitMemoryGlobPattern(rawPattern);
+					const resource = await internalRouter.resolve(memoryGlob.baseUrl, {
+						cwd: this.session.cwd,
+						settings: this.session.settings,
+						signal,
+						localProtocolOptions: this.session.localProtocolOptions,
+						skills: this.session.skills,
+						pathOnly: true,
+					});
+					if (!resource.sourcePath) {
+						throw new ToolError(`Cannot find internal URL without a backing file: ${memoryGlob.baseUrl}`);
+					}
+					normalizedPatterns.push(
+						path.join(resource.sourcePath.replace(/[*?[{]/g, "[$&]"), memoryGlob.globPattern),
+					);
+					continue;
 				}
 				const resource = await internalRouter.resolve(rawPattern, {
 					cwd: this.session.cwd,

packages/coding-agent/test/internal-urls/memory-protocol.test.ts

@@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test";
 import * as fs from "node:fs/promises";
 import * as os from "node:os";
 import * as path from "node:path";
+import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
 import { InternalUrlRouter } from "@oh-my-pi/pi-coding-agent/internal-urls";
 import { getMemoryRoot } from "@oh-my-pi/pi-coding-agent/memories";
 import {
@@ -12,6 +13,8 @@ import {
 } from "@oh-my-pi/pi-coding-agent/mnemopi/state";
 import { AgentRegistry } from "@oh-my-pi/pi-coding-agent/registry/agent-registry";
 import type { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session";
+import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
+import { GlobTool } from "@oh-my-pi/pi-coding-agent/tools/glob";
 import { getAgentDir, removeWithRetries, setAgentDir, TempDir } from "@oh-my-pi/pi-utils";
 
 // Mnemopi state is loaded lazily; preload so `new MnemopiSessionState(...)` can
@@ -55,6 +58,17 @@ async function withMemoryFixture(fn: (fixture: MemoryFixture) => Promise<void>):
 	}
 }
 
+function createGlobTool(cwd: string): GlobTool {
+	const session: ToolSession = {
+		cwd,
+		hasUI: false,
+		settings: Settings.isolated({}),
+		getSessionFile: () => null,
+		getSessionSpawns: () => null,
+	};
+	return new GlobTool(session);
+}
+
 describe("MemoryProtocolHandler", () => {
 	beforeEach(() => {
 		AgentRegistry.resetGlobalForTests();
@@ -234,6 +248,79 @@ describe("MemoryProtocolHandler", () => {
 		});
 	});
 
+	it("globs nested directories within the memory root", async () => {
+		await withMemoryFixture(async ({ cwd, memoryRoot }) => {
+			const nestedSkill = path.join(memoryRoot, "skills", "demo", "nested", "SKILL.md");
+			await fs.mkdir(path.dirname(nestedSkill), { recursive: true });
+			await Bun.write(nestedSkill, "nested skill");
+			await Bun.write(path.join(memoryRoot, "skills", "demo", "notes.txt"), "not markdown");
+
+			const tool = createGlobTool(cwd);
+			const result = await tool.execute("memory-glob", { path: "memory://root/skills/**/*.md" });
+
+			expect(result.details?.files).toHaveLength(1);
+			expect(result.details?.files?.[0]).toEndWith("/skills/demo/nested/SKILL.md");
+
+			const rootResult = await tool.execute("memory-root-glob", { path: "memory://root/**/*.md" });
+
+			expect(rootResult.details?.files).toHaveLength(1);
+			expect(rootResult.details?.files?.[0]).toEndWith("/skills/demo/nested/SKILL.md");
+		});
+	});
+
+	it("preserves literal question-mark wildcards in memory globs", async () => {
+		await withMemoryFixture(async ({ cwd, memoryRoot }) => {
+			const skillsDir = path.join(memoryRoot, "skills");
+			await fs.mkdir(skillsDir, { recursive: true });
+			await Bun.write(path.join(skillsDir, "a.md"), "single character");
+			await Bun.write(path.join(skillsDir, "ab.md"), "two characters");
+
+			const result = await createGlobTool(cwd).execute("memory-question-glob", {
+				path: "memory://root/skills/?.md",
+			});
+
+			expect(result.details?.files).toHaveLength(1);
+			expect(result.details?.files?.[0]).toEndWith("/skills/a.md");
+		});
+	});
+
+	it("resolves encoded literal glob characters before the wildcard boundary", async () => {
+		await withMemoryFixture(async ({ cwd, memoryRoot }) => {
+			const encodedLiteralDir = path.join(memoryRoot, "skills", "[demo]");
+			await fs.mkdir(encodedLiteralDir, { recursive: true });
+			await Bun.write(path.join(encodedLiteralDir, "SKILL.md"), "encoded literal directory");
+
+			const result = await createGlobTool(cwd).execute("memory-encoded-literal-glob", {
+				path: "memory://root/skills/%5Bdemo%5D/*.md",
+			});
+
+			expect(result.details?.files).toHaveLength(1);
+			expect(result.details?.files?.[0]).toEndWith("/skills/[demo]/SKILL.md");
+		});
+	});
+
+	it.each(["memory://root/skills/**/../*.md", "memory://root/skills/**/%2e%2e/*.md"])(
+		"rejects traversal in a memory glob suffix: %s",
+		async pattern => {
+			await withMemoryFixture(async ({ cwd }) => {
+				await expect(createGlobTool(cwd).execute("memory-glob-traversal", { path: pattern })).rejects.toThrow(
+					/traversal/i,
+				);
+			});
+		},
+	);
+
+	it.each(["memory://root/skills/**/demo%2fnested/*.md", "memory://root/skills/**/demo%5cnested/*.md"])(
+		"rejects encoded separators in a memory glob suffix: %s",
+		async pattern => {
+			await withMemoryFixture(async ({ cwd }) => {
+				await expect(createGlobTool(cwd).execute("memory-glob-separator", { path: pattern })).rejects.toThrow(
+					/encoded path separator/i,
+				);
+			});
+		},
+	);
+
 	it("throws clear error for missing files", async () => {
 		await withMemoryFixture(async () => {
 			const router = InternalUrlRouter.instance();