Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/agent/pi-coding-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,21 @@ 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,
purpose: "path",
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 {
Expand Down Expand Up @@ -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,
});
Expand Down
26 changes: 20 additions & 6 deletions src/agent/tool-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}).` };
}

Expand Down
7 changes: 5 additions & 2 deletions src/gateway/run-loop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@ export async function runGatewayLoop(params: GatewayRunLoopParams): Promise<void

const request = (action: "stop" | "restart", signal: NodeJS.Signals): void => {
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(() => {
Expand Down
27 changes: 27 additions & 0 deletions test/tool-policy-hooks.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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();
Expand Down
Loading