Skip to content
Open
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
73 changes: 72 additions & 1 deletion apps/pi-extension/server.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { afterEach, describe, expect, test } from "bun:test";
import { spawnSync } from "node:child_process";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { createServer as createNetServer } from "node:net";
import { tmpdir } from "node:os";
import { join } from "node:path";
Expand All @@ -10,6 +10,7 @@ const tempDirs: string[] = [];
const originalCwd = process.cwd();
const originalHome = process.env.HOME;
const originalPort = process.env.PLANNOTATOR_PORT;
const originalPath = process.env.PATH;

function makeTempDir(prefix: string): string {
const dir = mkdtempSync(join(tmpdir(), prefix));
Expand Down Expand Up @@ -39,6 +40,14 @@ function initRepo(): string {
return repoDir;
}

function installFakeCli(name: string): void {
const dir = makeTempDir(`plannotator-${name}-cli-`);
const path = join(dir, name);
writeFileSync(path, "#!/bin/sh\nexit 0\n", "utf-8");
chmodSync(path, 0o755);
process.env.PATH = [dir, originalPath].filter(Boolean).join(":");
}

function reservePort(): Promise<number> {
return new Promise((resolve, reject) => {
const server = createNetServer();
Expand Down Expand Up @@ -75,6 +84,11 @@ afterEach(() => {
} else {
process.env.PLANNOTATOR_PORT = originalPort;
}
if (originalPath === undefined) {
delete process.env.PATH;
} else {
process.env.PATH = originalPath;
}

for (const dir of tempDirs.splice(0)) {
rmSync(dir, { recursive: true, force: true });
Expand Down Expand Up @@ -251,6 +265,63 @@ describe("pi review server", () => {
}
});

test("requires the review session token to launch agent jobs", async () => {
const homeDir = makeTempDir("plannotator-pi-home-");
const repoDir = initRepo();
installFakeCli("codex");
process.env.HOME = homeDir;
process.chdir(repoDir);
process.env.PLANNOTATOR_PORT = String(await reservePort());

writeFileSync(join(repoDir, "tracked.txt"), "after\n", "utf-8");

const gitContext = await getGitContext();
const diff = await runGitDiff("uncommitted", gitContext.defaultBranch);

const server = await startReviewServer({
rawPatch: diff.patch,
gitRef: diff.label,
error: diff.error,
diffType: "uncommitted",
gitContext,
origin: "pi",
htmlContent: "<!doctype html><html><body>review</body></html>",
});

try {
const diffResponse = await fetch(`${server.url}/api/diff`);
const diffPayload = await diffResponse.json() as { agentJobToken?: string };
expect(diffPayload.agentJobToken).toBeTruthy();

const unauthorized = await fetch(`${server.url}/api/agents/jobs`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({
provider: "codex",
command: ["/usr/bin/true"],
label: "Injected",
}),
});
expect(unauthorized.status).toBe(403);

const authorized = await fetch(`${server.url}/api/agents/jobs`, {
method: "POST",
headers: {
"Content-Type": "application/json",
"X-Plannotator-Agent-Token": diffPayload.agentJobToken!,
},
body: JSON.stringify({
provider: "codex",
command: ["/usr/bin/true"],
label: "Injected",
}),
});
expect(authorized.status).toBe(201);
} finally {
server.stop();
}
});

test("git-add endpoint stages and unstages files in review mode", async () => {
const homeDir = makeTempDir("plannotator-pi-home-");
const repoDir = initRepo();
Expand Down
21 changes: 21 additions & 0 deletions apps/pi-extension/server/agent-jobs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const BASE = "/api/agents";
const JOBS = `${BASE}/jobs`;
const JOBS_STREAM = `${JOBS}/stream`;
const CAPABILITIES = `${BASE}/capabilities`;
const AGENT_JOB_TOKEN_HEADER = "x-plannotator-agent-token";

// ---------------------------------------------------------------------------
// which() helper for Node.js
Expand Down Expand Up @@ -66,6 +67,7 @@ export interface AgentJobHandlerOptions {
} | null>;
/** Called when a job completes successfully — parse results and push annotations. */
onJobComplete?: (job: AgentJobInfo, meta: { outputPath?: string; stdout?: string; cwd?: string }) => void | Promise<void>;
authToken?: string;
}

export function createAgentJobHandler(options: AgentJobHandlerOptions) {
Expand Down Expand Up @@ -307,6 +309,13 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) {
return Array.from(jobs.values()).map((e) => ({ ...e.info }));
}

function isAuthorizedRequest(req: IncomingMessage): boolean {
if (!options.authToken) return true;
const tokenHeader = req.headers[AGENT_JOB_TOKEN_HEADER];
if (Array.isArray(tokenHeader)) return tokenHeader.includes(options.authToken);
return tokenHeader === options.authToken;
}

// --- HTTP handler ---
return {
killAll,
Expand Down Expand Up @@ -377,6 +386,10 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) {

// --- POST /api/agents/jobs (launch) ---
if (url.pathname === JOBS && req.method === "POST") {
if (!isAuthorizedRequest(req)) {
json(res, { error: "Unauthorized agent job request" }, 403);
return true;
}
try {
const body = await parseBody(req);
const provider = typeof body.provider === "string" ? body.provider : "";
Expand Down Expand Up @@ -430,6 +443,10 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) {

// --- DELETE /api/agents/jobs/:id (kill one) ---
if (url.pathname.startsWith(JOBS + "/") && url.pathname !== JOBS_STREAM && req.method === "DELETE") {
if (!isAuthorizedRequest(req)) {
json(res, { error: "Unauthorized agent job request" }, 403);
return true;
}
const id = url.pathname.slice(JOBS.length + 1);
if (!id) {
json(res, { error: "Missing job ID" }, 400);
Expand All @@ -446,6 +463,10 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) {

// --- DELETE /api/agents/jobs (kill all) ---
if (url.pathname === JOBS && req.method === "DELETE") {
if (!isAuthorizedRequest(req)) {
json(res, { error: "Unauthorized agent job request" }, 403);
return true;
}
const count = killAll();
json(res, { ok: true, killed: count });
return true;
Expand Down
15 changes: 14 additions & 1 deletion apps/pi-extension/server/network.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { afterEach, describe, expect, test } from "bun:test";
import { getServerPort, isRemoteSession } from "./network";
import { getServerHostname, getServerPort, isRemoteSession } from "./network";

const savedEnv: Record<string, string | undefined> = {};
const envKeys = ["PLANNOTATOR_REMOTE", "PLANNOTATOR_PORT", "SSH_TTY", "SSH_CONNECTION"];
Expand Down Expand Up @@ -94,3 +94,16 @@ describe("pi port selection", () => {
expect(getServerPort()).toEqual({ port: 9999, portSource: "env" });
});
});

describe("pi server hostname", () => {
test("binds local sessions to loopback", () => {
clearEnv();
expect(getServerHostname()).toBe("127.0.0.1");
});

test("binds remote sessions to loopback", () => {
clearEnv();
process.env.PLANNOTATOR_REMOTE = "1";
expect(getServerHostname()).toBe("127.0.0.1");
});
});
7 changes: 6 additions & 1 deletion apps/pi-extension/server/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type { Server } from "node:http";
import { release } from "node:os";

const DEFAULT_REMOTE_PORT = 19432;
const LOOPBACK_HOST = "127.0.0.1";

/**
* Check if running in a remote session (SSH, devcontainer, etc.)
Expand Down Expand Up @@ -67,6 +68,10 @@ export function getServerPort(): {
return { port: 0, portSource: "random" };
}

export function getServerHostname(): string {
return LOOPBACK_HOST;
}

const MAX_RETRIES = 5;
const RETRY_DELAY_MS = 500;

Expand All @@ -81,7 +86,7 @@ export async function listenOnPort(
server.once("error", reject);
server.listen(
result.port,
isRemoteSession() ? "0.0.0.0" : "127.0.0.1",
getServerHostname(),
() => {
server.removeListener("error", reject);
resolve();
Expand Down
3 changes: 3 additions & 0 deletions apps/pi-extension/server/serverReview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ export async function startReviewServer(options: {
let currentGitRef = options.gitRef;
let currentDiffType: DiffType = options.diffType || "uncommitted";
let currentError = options.error;
const agentJobToken = crypto.randomUUID();

// Agent jobs — background process manager (late-binds serverUrl via getter)
let serverUrl = "";
Expand All @@ -205,6 +206,7 @@ export async function startReviewServer(options: {
const agentJobs = createAgentJobHandler({
mode: "review",
getServerUrl: () => serverUrl,
authToken: agentJobToken,
getCwd: resolveAgentCwd,

async buildCommand(provider) {
Expand Down Expand Up @@ -423,6 +425,7 @@ export async function startReviewServer(options: {
shareBaseUrl,
repoInfo,
isWSL: wslFlag,
agentJobToken,
...(options.agentCwd && { agentCwd: options.agentCwd }),
...(isPRMode && { prMetadata: prMeta, platformUser }),
...(isPRMode && initialViewedFiles.length > 0 && { viewedFiles: initialViewedFiles }),
Expand Down
7 changes: 6 additions & 1 deletion packages/review-editor/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ interface DiffData {
diffType?: string;
gitContext?: GitContext;
sharingEnabled?: boolean;
agentJobToken?: string;
}

// Simple diff parser to extract files from unified diff
Expand Down Expand Up @@ -155,6 +156,7 @@ const ReviewApp: React.FC = () => {
const [diffType, setDiffType] = useState<string>('uncommitted');
const [gitContext, setGitContext] = useState<GitContext | null>(null);
const [agentCwd, setAgentCwd] = useState<string | null>(null);
const [agentJobToken, setAgentJobToken] = useState<string | null>(null);
const [isLoadingDiff, setIsLoadingDiff] = useState(false);
const [diffError, setDiffError] = useState<string | null>(null);
const [isSendingFeedback, setIsSendingFeedback] = useState(false);
Expand Down Expand Up @@ -220,7 +222,7 @@ const ReviewApp: React.FC = () => {
// The same !!origin proxy is used elsewhere in this file (draft hook, feedback guard, conditional UI)
// so this should be addressed as a broader refactor.
const { externalAnnotations, updateExternalAnnotation, deleteExternalAnnotation } = useExternalAnnotations<CodeAnnotation>({ enabled: !!origin });
const agentJobs = useAgentJobs({ enabled: !!origin });
const agentJobs = useAgentJobs({ enabled: !!origin, authToken: agentJobToken ?? undefined });

// Dockview center panel API for the review workspace.
const [dockApi, setDockApi] = useState<DockviewApi | null>(null);
Expand Down Expand Up @@ -624,6 +626,7 @@ const ReviewApp: React.FC = () => {
diffType?: string;
gitContext?: GitContext;
agentCwd?: string;
agentJobToken?: string;
sharingEnabled?: boolean;
repoInfo?: { display: string; branch?: string };
prMetadata?: PRMetadata;
Expand All @@ -646,12 +649,14 @@ const ReviewApp: React.FC = () => {
diffType: data.diffType,
gitContext: data.gitContext,
sharingEnabled: data.sharingEnabled,
agentJobToken: data.agentJobToken,
});
setFiles(apiFiles);
if (data.origin) setOrigin(data.origin);
if (data.diffType) setDiffType(data.diffType);
if (data.gitContext) setGitContext(data.gitContext);
if (data.agentCwd) setAgentCwd(data.agentCwd);
if (data.agentJobToken) setAgentJobToken(data.agentJobToken);
if (data.sharingEnabled !== undefined) setSharingEnabled(data.sharingEnabled);
if (data.repoInfo) setRepoInfo(data.repoInfo);
if (data.prMetadata) setPrMetadata(data.prMetadata);
Expand Down
84 changes: 84 additions & 0 deletions packages/server/agent-jobs.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { afterEach, describe, expect, test } from "bun:test";
import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { createAgentJobHandler } from "./agent-jobs";

const originalPath = process.env.PATH;
const tempDirs: string[] = [];

function makeFakeCli(name: string): void {
const dir = mkdtempSync(join(tmpdir(), "plannotator-agent-jobs-"));
const path = join(dir, name);
writeFileSync(path, "#!/bin/sh\nexit 0\n", "utf-8");
chmodSync(path, 0o755);
tempDirs.push(dir);
process.env.PATH = [dir, originalPath].filter(Boolean).join(":");
}

afterEach(() => {
process.env.PATH = originalPath;
for (const dir of tempDirs.splice(0)) {
rmSync(dir, { recursive: true, force: true });
}
});

describe("agent job auth", () => {
test("rejects launch requests without the session token", async () => {
makeFakeCli("codex");

const handler = createAgentJobHandler({
mode: "review",
getServerUrl: () => "http://localhost:19432",
getCwd: () => process.cwd(),
authToken: "session-token",
});

const req = new Request("http://localhost/api/agents/jobs", {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({
provider: "codex",
command: ["/usr/bin/true"],
label: "Injected",
}),
});

const res = await handler.handle(req, new URL(req.url));
expect(res?.status).toBe(403);
expect(await res?.json()).toEqual({ error: "Unauthorized agent job request" });
});

test("accepts authenticated launch requests and preserves raw commands when no builder is configured", async () => {
makeFakeCli("codex");

const handler = createAgentJobHandler({
mode: "review",
getServerUrl: () => "http://localhost:19432",
getCwd: () => process.cwd(),
authToken: "session-token",
});

const req = new Request("http://localhost/api/agents/jobs", {
method: "POST",
headers: {
"Content-Type": "application/json",
"X-Plannotator-Agent-Token": "session-token",
},
body: JSON.stringify({
provider: "codex",
command: ["/usr/bin/true"],
label: "Shell-compatible codex wrapper",
}),
});

const res = await handler.handle(req, new URL(req.url));
expect(res?.status).toBe(201);

const payload = await res?.json() as { job: { command: string[]; label: string } };
expect(payload.job.command).toEqual(["/usr/bin/true"]);
expect(payload.job.label).toBe("Shell-compatible codex wrapper");

handler.killAll();
});
});
Loading