From 5da806671e41abebff0002d56564265a31387f60 Mon Sep 17 00:00:00 2001 From: djdembeck Date: Tue, 14 Apr 2026 17:13:54 -0500 Subject: [PATCH] fix: prevent local:// URI from creating local: directory on Linux On Linux, Node's path.normalize() collapses the double slash in local://PLAN.md to local:/PLAN.md, creating a directory called local: in the project root instead of routing through the local:// protocol handler. Defense-in-depth fixes across 5 layers: 1. resolveToCwd() now throws if a path starts with any internal URL scheme prefix (local:, agent:, skill:, etc.), preventing all 59 call sites from treating URIs as relative filesystem paths. 2. resolvePlanPath() now matches on local: prefix (not just local://) and normalizes local:/ to local:// before resolution, catching all slash variants. 3. Bash URL expansion regex and early-exit checks now also match local:/ (single slash), and normalize before resolution. 4. Edit preview/diff functions now gracefully skip internal URL paths instead of crashing via the resolveToCwd guard. 5. All startsWith('local://') checks updated to startsWith('local:') with normalization in agent-session, interactive-mode, and approved-plan modules. Also adds local: to .gitignore to prevent accidental commits of the leaked directory. --- packages/coding-agent/.gitignore | 2 ++ packages/coding-agent/src/edit/diff.ts | 5 +++- .../coding-agent/src/edit/modes/hashline.ts | 5 +++- packages/coding-agent/src/edit/modes/patch.ts | 5 +++- .../src/modes/interactive-mode.ts | 5 ++-- .../src/plan-mode/approved-plan.ts | 4 ++-- .../coding-agent/src/session/agent-session.ts | 6 ++--- .../coding-agent/src/tools/bash-skill-urls.ts | 15 ++++++------ packages/coding-agent/src/tools/bash.ts | 2 +- packages/coding-agent/src/tools/path-utils.ts | 23 ++++++++++++++++++- .../coding-agent/src/tools/plan-mode-guard.ts | 13 +++++++---- 11 files changed, 62 insertions(+), 23 deletions(-) diff --git a/packages/coding-agent/.gitignore b/packages/coding-agent/.gitignore index a4a1c26c9..d4ade25dd 100644 --- a/packages/coding-agent/.gitignore +++ b/packages/coding-agent/.gitignore @@ -1 +1,3 @@ src/core/export-html/template.generated.ts + +local: diff --git a/packages/coding-agent/src/edit/diff.ts b/packages/coding-agent/src/edit/diff.ts index e82316f4e..feddb0411 100644 --- a/packages/coding-agent/src/edit/diff.ts +++ b/packages/coding-agent/src/edit/diff.ts @@ -6,7 +6,7 @@ */ import { isEnoent } from "@oh-my-pi/pi-utils"; import * as Diff from "diff"; -import { resolveToCwd } from "../tools/path-utils"; +import { isInternalUrlPath, resolveToCwd } from "../tools/path-utils"; import { DEFAULT_FUZZY_THRESHOLD, EditMatchError, findMatch } from "./modes/replace"; import { adjustIndentation, normalizeToLF, stripBom } from "./normalize"; @@ -761,6 +761,9 @@ export async function computeEditDiff( if (oldText.length === 0) { return { error: "oldText must not be empty." }; } + if (isInternalUrlPath(path)) { + return { error: `Preview not available for internal URL: ${path}` }; + } const absolutePath = resolveToCwd(path, cwd); try { diff --git a/packages/coding-agent/src/edit/modes/hashline.ts b/packages/coding-agent/src/edit/modes/hashline.ts index f0e435918..b49c6f2b0 100644 --- a/packages/coding-agent/src/edit/modes/hashline.ts +++ b/packages/coding-agent/src/edit/modes/hashline.ts @@ -27,7 +27,7 @@ import { invalidateFsScanAfterWrite, } from "../../tools/fs-cache-invalidation"; import { outputMeta } from "../../tools/output-meta"; -import { resolveToCwd } from "../../tools/path-utils"; +import { isInternalUrlPath, resolveToCwd } from "../../tools/path-utils"; import { enforcePlanModeWrite, resolvePlanPath } from "../../tools/plan-mode-guard"; import { generateDiffString } from "../diff"; import { computeLineHash, formatLineHash } from "../line-hash"; @@ -1157,6 +1157,9 @@ export async function computeHashlineDiff( } > { const { path, edits, move } = input; + if (isInternalUrlPath(path) || (move && isInternalUrlPath(move))) { + return { error: `Preview not available for internal URL: ${path}` }; + } const absolutePath = resolveToCwd(path, cwd); const movePath = move ? resolveToCwd(move, cwd) : undefined; const isMoveOnly = Boolean(movePath) && movePath !== absolutePath && edits.length === 0; diff --git a/packages/coding-agent/src/edit/modes/patch.ts b/packages/coding-agent/src/edit/modes/patch.ts index 7f6e9f9f7..414dc3637 100644 --- a/packages/coding-agent/src/edit/modes/patch.ts +++ b/packages/coding-agent/src/edit/modes/patch.ts @@ -25,7 +25,7 @@ import { invalidateFsScanAfterWrite, } from "../../tools/fs-cache-invalidation"; import { outputMeta } from "../../tools/output-meta"; -import { resolveToCwd } from "../../tools/path-utils"; +import { isInternalUrlPath, resolveToCwd } from "../../tools/path-utils"; import { enforcePlanModeWrite, resolvePlanPath } from "../../tools/plan-mode-guard"; import { ApplyPatchError, @@ -1557,6 +1557,9 @@ export async function computePatchDiff( error: string; } > { + if (isInternalUrlPath(input.path) || (input.rename && isInternalUrlPath(input.rename))) { + return { error: `Preview not available for internal URL: ${input.path}` }; + } try { const result = await previewPatch(input, { cwd, diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index f7a086c33..e3685171b 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -637,8 +637,9 @@ export class InteractiveMode implements InteractiveModeContext { } #resolvePlanFilePath(planFilePath: string): string { - if (planFilePath.startsWith("local://")) { - return resolveLocalUrlToPath(planFilePath, { + if (planFilePath.startsWith("local:")) { + const normalized = planFilePath.replace(/^(local:)\/(?!\/)/, "$1//"); + return resolveLocalUrlToPath(normalized, { getArtifactsDir: () => this.sessionManager.getArtifactsDir(), getSessionId: () => this.sessionManager.getSessionId(), }); diff --git a/packages/coding-agent/src/plan-mode/approved-plan.ts b/packages/coding-agent/src/plan-mode/approved-plan.ts index fa2284dd2..4bc6cd4f1 100644 --- a/packages/coding-agent/src/plan-mode/approved-plan.ts +++ b/packages/coding-agent/src/plan-mode/approved-plan.ts @@ -10,8 +10,8 @@ interface RenameApprovedPlanFileOptions { } function assertLocalUrl(path: string, label: "source" | "destination"): void { - if (!path.startsWith("local://")) { - throw new Error(`Approved plan ${label} path must use local:// (received ${path}).`); + if (!path.startsWith("local:")) { + throw new Error(`Approved plan ${label} path must use local:// scheme (received ${path}).`); } } diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 9aa420d09..222059ccd 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -2445,8 +2445,8 @@ export class AgentSession { const state = this.#planModeState; if (!state?.enabled) return null; const sessionPlanUrl = "local://PLAN.md"; - const resolvedPlanPath = state.planFilePath.startsWith("local://") - ? resolveLocalUrlToPath(state.planFilePath, { + const resolvedPlanPath = state.planFilePath.startsWith("local:") + ? resolveLocalUrlToPath(state.planFilePath.replace(/^(local:)\/(?!\/)/, "$1//"), { getArtifactsDir: () => this.sessionManager.getArtifactsDir(), getSessionId: () => this.sessionManager.getSessionId(), }) @@ -2456,7 +2456,7 @@ export class AgentSession { getSessionId: () => this.sessionManager.getSessionId(), }); const displayPlanPath = - state.planFilePath.startsWith("local://") || resolvedPlanPath !== resolvedSessionPlan + state.planFilePath.startsWith("local:") || resolvedPlanPath !== resolvedSessionPlan ? state.planFilePath : sessionPlanUrl; diff --git a/packages/coding-agent/src/tools/bash-skill-urls.ts b/packages/coding-agent/src/tools/bash-skill-urls.ts index bf98babea..7d7181816 100644 --- a/packages/coding-agent/src/tools/bash-skill-urls.ts +++ b/packages/coding-agent/src/tools/bash-skill-urls.ts @@ -9,9 +9,8 @@ import { ToolError } from "./tool-errors"; /** Regex to find skill:// tokens in command text. */ const SKILL_URL_PATTERN = /'skill:\/\/[^'\s")`\\]+'|"skill:\/\/[^"\s')`\\]+"|skill:\/\/[^\s'")`\\]+/g; -/** Regex to find supported internal URL tokens in command text. */ -const INTERNAL_URL_PATTERN = - /'(?:skill|agent|artifact|plan|memory|rule|local):\/\/[^'\s")`\\]+'|"(?:skill|agent|artifact|plan|memory|rule|local):\/\/[^"\s')`\\]+"|(?:skill|agent|artifact|plan|memory|rule|local):\/\/[^\s'")`\\]+/g; +const INTERNAL_URL_PATTERN_INCLUDING_NORMALIZED_LOCAL = + /'(?:skill|agent|artifact|plan|memory|rule|local):\/\/[^'\s")`\\]+'|"(?:skill|agent|artifact|plan|memory|rule|local):\/\/[^"\s')`\\]+"|(?:skill|agent|artifact|plan|memory|rule|local):\/\/[^\s'")`\\]+|local:\/[^\s'")`\\]+/g; const SUPPORTED_INTERNAL_SCHEMES = ["skill", "agent", "artifact", "plan", "memory", "rule", "local"] as const; @@ -146,12 +145,13 @@ function shellEscape(p: string): string { } async function resolveInternalUrlToPath( - url: string, + rawUrl: string, skills: readonly Skill[], internalRouter?: InternalUrlResolver, localOptions?: LocalProtocolOptions, ensureLocalParentDirs?: boolean, ): Promise { + const url = rawUrl.replace(/^(local:)\/(?!\/)/, "$1//"); const scheme = extractScheme(url); if (!scheme) { throw new ToolError(`Unsupported internal URL in bash command: ${url}`); @@ -218,9 +218,9 @@ export function expandSkillUrls(command: string, skills: readonly Skill[]): stri * Supported schemes: skill://, agent://, artifact://, memory://, rule://, local:// */ export async function expandInternalUrls(command: string, options: InternalUrlExpansionOptions): Promise { - if (!command.includes("://")) return command; + if (!command.includes("://") && !command.includes("local:/")) return command; - const matches = Array.from(command.matchAll(INTERNAL_URL_PATTERN)); + const matches = Array.from(command.matchAll(INTERNAL_URL_PATTERN_INCLUDING_NORMALIZED_LOCAL)); if (matches.length === 0) return command; let expanded = command; @@ -230,7 +230,8 @@ export async function expandInternalUrls(command: string, options: InternalUrlEx const index = match.index; if (index === undefined) continue; - const url = unquoteToken(token); + const rawUrl = unquoteToken(token); + const url = rawUrl.replace(/^(local:)\/(?!\/)/, "$1//"); const resolvedPath = await resolveInternalUrlToPath( url, options.skills, diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index 54fee3794..ffd933048 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -512,7 +512,7 @@ export class BashTool implements AgentTool { : undefined; // Resolve protocol URLs (skill://, agent://, etc.) in extracted cwd. - if (cwd?.includes("://")) { + if (cwd?.includes("://") || cwd?.includes("local:/")) { cwd = await expandInternalUrls(cwd, { ...internalUrlOptions, noEscape: true }); } diff --git a/packages/coding-agent/src/tools/path-utils.ts b/packages/coding-agent/src/tools/path-utils.ts index ce97f32da..7175c54db 100644 --- a/packages/coding-agent/src/tools/path-utils.ts +++ b/packages/coding-agent/src/tools/path-utils.ts @@ -74,7 +74,7 @@ function normalizeAtPrefix(filePath: string): string { withoutAt.startsWith("artifact://") || withoutAt.startsWith("skill://") || withoutAt.startsWith("rule://") || - withoutAt.startsWith("local://") || + withoutAt.startsWith("local:") || withoutAt.startsWith("mcp://") ) { return withoutAt; @@ -110,6 +110,24 @@ export function expandPath(filePath: string): string { return expandTilde(normalized); } +function assertNotInternalUrl(expanded: string, original: string): void { + for (const prefix of TOP_LEVEL_INTERNAL_URL_PREFIXES) { + if (expanded.startsWith(prefix)) { + throw new Error( + `Path "${original}" uses internal scheme "${prefix}" and must be resolved through the proper protocol handler, not as a filesystem path.`, + ); + } + } +} + +export function isInternalUrlPath(filePath: string): boolean { + const expanded = expandPath(filePath); + for (const prefix of TOP_LEVEL_INTERNAL_URL_PREFIXES) { + if (expanded.startsWith(prefix)) return true; + } + return false; +} + /** * Resolve a path relative to the given cwd. * Handles ~ expansion and absolute paths. @@ -120,6 +138,9 @@ export function expandPath(filePath: string): string { */ export function resolveToCwd(filePath: string, cwd: string): string { const expanded = expandPath(filePath); + + assertNotInternalUrl(expanded, filePath); + if (/^\/+$/.test(expanded)) { return cwd; } diff --git a/packages/coding-agent/src/tools/plan-mode-guard.ts b/packages/coding-agent/src/tools/plan-mode-guard.ts index eeeca435f..538bbb2d2 100644 --- a/packages/coding-agent/src/tools/plan-mode-guard.ts +++ b/packages/coding-agent/src/tools/plan-mode-guard.ts @@ -3,17 +3,22 @@ import type { ToolSession } from "."; import { resolveToCwd } from "./path-utils"; import { ToolError } from "./tool-errors"; -const LOCAL_URL_PREFIX = "local://"; +const LOCAL_SCHEME_PREFIX = "local:"; + +function normalizeLocalScheme(path: string): string { + return path.replace(/^(local:)\/(?!\/)/, "$1//"); +} export function resolvePlanPath(session: ToolSession, targetPath: string): string { - if (targetPath.startsWith(LOCAL_URL_PREFIX)) { - return resolveLocalUrlToPath(targetPath, { + const normalized = normalizeLocalScheme(targetPath); + if (normalized.startsWith(LOCAL_SCHEME_PREFIX)) { + return resolveLocalUrlToPath(normalized, { getArtifactsDir: session.getArtifactsDir, getSessionId: session.getSessionId, }); } - return resolveToCwd(targetPath, session.cwd); + return resolveToCwd(normalized, session.cwd); } export function enforcePlanModeWrite(