diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 75daeda8b..c84c34773 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `glob` rejecting safe `memory://root//**` 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 diff --git a/packages/coding-agent/src/internal-urls/memory-protocol.ts b/packages/coding-agent/src/internal-urls/memory-protocol.ts index 789ae82b4..4653b7514 100644 --- a/packages/coding-agent/src/internal-urls/memory-protocol.ts +++ b/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 { ); } 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, diff --git a/packages/coding-agent/test/internal-urls/memory-protocol.test.ts b/packages/coding-agent/test/internal-urls/memory-protocol.test.ts index b25412340..d3a3821fb 100644 --- a/packages/coding-agent/test/internal-urls/memory-protocol.test.ts +++ b/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): } } +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();