diff --git a/src/agent/pi-coding-tools.ts b/src/agent/pi-coding-tools.ts index c346321..332fe83 100644 --- a/src/agent/pi-coding-tools.ts +++ b/src/agent/pi-coding-tools.ts @@ -55,6 +55,13 @@ export class PiCodingToolsExecutor { } private canUsePath(rawPath: string, options?: { allowSkillRootsForRead?: boolean }): boolean { + return this.resolvePath(rawPath, options) !== null; + } + + private resolvePath( + rawPath: string, + options?: { allowSkillRootsForRead?: boolean }, + ): string | null { const resolved = resolveWorkspacePathPolicy({ workspaceDir: this.config.workspaceDir, inputPath: rawPath, @@ -62,7 +69,7 @@ export class PiCodingToolsExecutor { workspaceOnly: this.config.codingTools.workspaceOnly, allowSkillRootsForRead: options?.allowSkillRootsForRead, }); - return resolved.ok; + return resolved.ok && resolved.resolved ? resolved.resolved : null; } private resolveWorkdir(inputPath?: string): string | null { @@ -94,11 +101,12 @@ export class PiCodingToolsExecutor { } if (action.type === "read") { - if (!this.canUsePath(action.path, { allowSkillRootsForRead: true })) { + const resolvedReadPath = this.resolvePath(action.path, { allowSkillRootsForRead: true }); + if (!resolvedReadPath) { return null; } const result = await this.readTool.execute("pi-read", { - path: action.path, + path: resolvedReadPath, offset: action.from, limit: action.lines, }); diff --git a/src/agent/tool-policy.ts b/src/agent/tool-policy.ts index 2cd068b..40b349d 100644 --- a/src/agent/tool-policy.ts +++ b/src/agent/tool-policy.ts @@ -99,24 +99,38 @@ export function resolveWorkspacePathPolicy(params: ResolveWorkspacePathPolicyPar } const resolved = path.resolve(params.workspaceDir, expandTilde(raw)); - if (!params.workspaceOnly) { - return { ok: true, resolved }; - } - if (pathWithin(params.workspaceDir, resolved)) { - return { ok: true, resolved }; - } + // Skill root resolution must run BEFORE the workspace check. + // Paths like ".openpocket/skills/..." resolve inside the workspace dir + // (e.g. /workspace/.openpocket/skills/...) and would pass pathWithin, + // returning the wrong path. We need to redirect them to the real + // skill root under the user's home directory first. if (params.allowSkillRootsForRead) { const skillRoots = [ path.join(params.workspaceDir, "skills"), path.join(openpocketHome(), "skills"), BUNDLED_SKILLS_DIR, ]; + + // The model may output home-relative paths without the ~ prefix + // (e.g. ".openpocket/skills/..."). Try resolving against $HOME. + const homeResolved = path.resolve(os.homedir(), raw); + if (homeResolved !== resolved && skillRoots.some((root) => pathWithin(root, homeResolved))) { + return { ok: true, resolved: homeResolved }; + } + if (skillRoots.some((root) => pathWithin(root, resolved))) { return { ok: true, resolved }; } } + if (!params.workspaceOnly) { + return { ok: true, resolved }; + } + if (pathWithin(params.workspaceDir, resolved)) { + return { ok: true, resolved }; + } + return { ok: false, error: `${params.purpose}: path escapes workspace (${raw}).` }; } diff --git a/src/gateway/run-loop.ts b/src/gateway/run-loop.ts index 0cccc84..3911bb3 100644 --- a/src/gateway/run-loop.ts +++ b/src/gateway/run-loop.ts @@ -22,12 +22,15 @@ export async function runGatewayLoop(params: GatewayRunLoopParams): Promise { if (shuttingDown) { - return; + log( + `[OpenPocket][gateway-loop][warn] ${new Date().toISOString()} signal=${signal} received again — forcing exit`, + ); + process.exit(1); } shuttingDown = true; restarting = action === "restart"; log( - `[OpenPocket][gateway-loop][warn] ${new Date().toISOString()} signal=${signal} action=${restarting ? "restart" : "stop"}`, + `[OpenPocket][gateway-loop][warn] ${new Date().toISOString()} signal=${signal} action=${restarting ? "restart" : "stop"} (press Ctrl+C again to force quit)`, ); void Promise.resolve(current?.stop(`signal:${signal}`)) .catch(() => { diff --git a/test/tool-policy-hooks.test.mjs b/test/tool-policy-hooks.test.mjs index 5dedc38..918e8ef 100644 --- a/test/tool-policy-hooks.test.mjs +++ b/test/tool-policy-hooks.test.mjs @@ -8,6 +8,7 @@ const { loadConfig } = await import("../dist/config/index.js"); const { PiCodingToolsExecutor } = await import("../dist/agent/pi-coding-tools.js"); const { CodingExecutor } = await import("../dist/tools/coding-executor.js"); const { ScriptExecutor } = await import("../dist/tools/script-executor.js"); +const { resolveWorkspacePathPolicy } = await import("../dist/agent/tool-policy.js"); async function withTempHome(prefix, fn) { const home = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); @@ -117,6 +118,32 @@ test("tool policy strips sensitive env vars for coding and script command execut }); }); +test("resolveWorkspacePathPolicy redirects home-relative skill paths to real skill root", async () => { + await withTempHome("openpocket-skill-resolve-", async (opHome) => { + const skillDir = path.join(opHome, "skills", "my-skill"); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync(path.join(skillDir, "SKILL.md"), "# test"); + + const workspaceDir = path.join(opHome, "workspace"); + fs.mkdirSync(workspaceDir, { recursive: true }); + + // Model outputs ".openpocket/skills/my-skill/SKILL.md" without ~ prefix. + // openpocketHome() returns opHome (set via OPENPOCKET_HOME env). + // The path ".openpocket/skills/..." resolved against workspace would + // land inside workspace, but the REAL file lives under opHome/skills/. + // With the home-relative fallback this must resolve to the real location. + const result = resolveWorkspacePathPolicy({ + workspaceDir, + inputPath: path.relative(os.homedir(), path.join(opHome, "skills", "my-skill", "SKILL.md")), + purpose: "read", + workspaceOnly: true, + allowSkillRootsForRead: true, + }); + assert.equal(result.ok, true, `expected ok but got: ${result.error}`); + assert.equal(result.resolved, path.join(skillDir, "SKILL.md")); + }); +}); + test("workspace boundary checks remain enforced across coding paths", async () => { await withTempHome("openpocket-policy-workspace-", async () => { const cfg = loadConfig();