fix(pm-dispatch): constrain session directories for every caller

pm-stack/dispatch/README.md

@@ -31,13 +31,12 @@ Body:
 {
   "repo": "developing-today/code", // required, allowlisted fleet repo
   "prompt": "Fix the flaky widget test", // required
-  "title": "fix(widget): flaky test", // optional; default: prompt's first line
+  "title": "fix(widget): flaky test", // optional; blank/omitted defaults to prompt's first line (issue #2664)
   "model": "opencode-go/gpt-5.6-luna", // optional "provider/model" or {"providerID","id"}
   "directory": "/home/user/git/code", // optional; default: the repo's laptop primary.
-  // OIDC callers may only omit it or repeat the repo's primary — anything
-  // else is rejected (400) before any side effect (issue #2903).
-  // Bearer callers keep the free-form directory (deliberate: the bearer
-  // secret lives on the laptop, so whoever holds it already has the run).
+  // Every caller may use only the target repo's primary or a directory
+  // inside it; relative/outside paths are rejected before any side effect.
+  // Empty and whitespace-only values use the default (issue #2664).
   "priority": "high", // optional: integer 10-90 (number or decimal string), or critical|high|normal|low → 10/20/40/70
   "issue": true, // optional; default true
   "wait": true, // optional; default true
@@ -46,9 +45,11 @@ Body:
 
 Flow:
 
-0. Scope the directory: OIDC callers must omit `directory` or repeat
-   the repo's laptop primary clone — anything else is a 400 with no
-   issue, task, or session created (issue #2903).
+0. Scope the directory uniformly for every caller: absent, empty, or
+   whitespace-only values use the target repo's laptop primary clone.
+   Otherwise require an absolute path equal to or inside that clone;
+   reject relative and outside paths with 400 before any issue, task, or
+   session is created (issue #2664).
 1. Create the tracking GitHub issue (`source/dispatch` label; body = the
    prompt + a dispatch footer). Skipped with a `warnings` entry for
    dezren39/nix, where the pm-stack GitHub App is not installed.
@@ -141,12 +142,10 @@ Every endpoint except `GET /health` requires `Authorization: Bearer …`:
   the claim need not equal the request body's `repo`. Repos outside
   the allowlist can neither call nor be targeted. Bearer callers are
   bound only by the allowlist on the target.
-  **Directory scoping (issue #2903):** OIDC callers may only omit
-  `directory` or repeat the target repo's laptop primary clone — any
-  other value is rejected with 400 before the issue, task, or session
-  is created. Bearer callers keep the free-form directory as a
-  deliberate choice: the static secret lives on the laptop, so its
-  holder already has the run the directory would grant.
+  **Directory scoping (issue #2664):** all callers may omit `directory`
+  or pass an absolute path equal to or inside the target repo's primary
+  clone. Empty/whitespace values default to the clone; relative, foreign,
+  and outside paths are rejected with HTTP 400 before any side effect.
   See `examples/dispatch-workflow.yml`.
 - **Static bearer secret**: `DISPATCH_API_TOKEN` (in the laptop's
   `~/secrets/dispatch.env`), compared in constant time — the shape a

pm-stack/dispatch/src/server.ts

@@ -175,7 +175,7 @@ async function handleSessions(req: Request, caller: CallerIdentity): Promise<Res
   const body = parseSessionRequest(await readJsonBody(req));
   assertCallerRepo(caller, body.repo, cfg.repoAllowlist);
   const warnings: string[] = [];
-  // Directory scoping (issue #2903) runs before ANY side effect: the
+  // Directory scoping (issue #2664) runs before ANY side effect: the
   // tracking issue, queue task and session below are all created after
   // this line, so a rejected directory leaves nothing behind.
   const directory = resolveSessionDirectory(caller.type, body.repo, body.directory, REPO_DIRS);

pm-stack/dispatch/src/validate.ts

@@ -1,6 +1,7 @@
 // Request validation helpers: priority mapping, model parsing, and the
 // body validators for POST /v1/sessions and POST /v1/tasks. Pure functions —
 // unit-tested without a server.
+import { isAbsolute, relative, resolve, sep } from "node:path";
 
 export class ValidationError extends Error {
   status = 400;
@@ -107,6 +108,11 @@ function optString(body: Record<string, unknown>, field: string): string | undef
   return v;
 }
 
+function optNonBlankString(body: Record<string, unknown>, field: string): string | undefined {
+  const value = optString(body, field);
+  return value?.trim() ? value : undefined;
+}
+
 function optBool(body: Record<string, unknown>, field: string, dflt: boolean): boolean {
   const v = body[field];
   if (v === undefined || v === null) return dflt;
@@ -131,7 +137,7 @@ export function parseSessionRequest(body: unknown): SessionRequest {
   return {
     repo: reqString(b, "repo"),
     prompt: reqString(b, "prompt"),
-    title: optString(b, "title"),
+    title: optNonBlankString(b, "title"),
     model: parseModel(b.model),
     directory: optString(b, "directory"),
     priority: resolvePriority(b.priority),
@@ -140,29 +146,36 @@ export function parseSessionRequest(body: unknown): SessionRequest {
   };
 }
 
-/** Session-directory scoping for POST /v1/sessions (issue #2903).
- * OIDC callers (workflows in allowlisted repos) may only run inside the
- * target repo's laptop primary clone: directory omitted (default) or
- * exactly the repo's scoped directory. Anything else is rejected BEFORE
- * any issue/task/session is created. Bearer callers hold the laptop
- * secret, so they keep the free-form directory — a deliberate choice,
- * documented in the README. */
+/** Session-directory scoping for POST /v1/sessions (issue #2664).
+ * Every caller may use only the target repo's configured laptop clone or a
+ * path inside it. Blank input selects the clone; relative and out-of-tree
+ * paths are rejected before any issue/task/session is created. Caller type
+ * is used only in diagnostics; the path policy is identical for both. */
 export function resolveSessionDirectory(
   callerType: "oidc" | "bearer",
   repo: string,
   directory: string | undefined,
   repoDirs: Record<string, string>,
 ): string {
   const scoped = repoDirs[repo];
-  if (directory === undefined) return scoped!;
-  if (callerType === "bearer") return directory;
-  const norm = (d: string) => (d.length > 1 ? d.replace(/\/+$/, "") : d);
-  if (scoped === undefined || norm(directory) !== norm(scoped)) {
+  if (scoped === undefined || !isAbsolute(scoped)) {
+    throw new ValidationError(
+      `directory: no absolute repo directory is configured for ${repo}`,
+    );
+  }
+  const root = resolve(scoped);
+  if (directory === undefined || !directory.trim()) return root;
+  if (!isAbsolute(directory)) {
     throw new ValidationError(
-      `directory: OIDC callers may only omit directory or use the repo clone (${scoped ?? "unknown"})`,
+      `directory: ${callerType} callers must use an absolute path inside the repo directory (${root})`,
     );
   }
-  return scoped;
+  const target = resolve(directory);
+  const fromRoot = relative(root, target);
+  if (fromRoot === ".." || fromRoot.startsWith(`..${sep}`) || isAbsolute(fromRoot)) {
+    throw new ValidationError(`directory: ${callerType} callers must stay inside the repo directory (${root})`);
+  }
+  return target;
 }
 
 export interface TaskRequest {

pm-stack/dispatch/test/server.test.ts

@@ -1,7 +1,7 @@
-// HTTP-level tests for POST /v1/sessions directory scoping (issue #2903).
+// HTTP-level tests for POST /v1/sessions directory scoping (issue #2664).
 // Drives the exported handler with a stubbed-JWKS OIDC token: no network,
-// no queue, no GitHub — any non-JWKS fetch throws, so a test that tried
-// to create an issue/task/session would fail loudly instead of 400ing.
+// queue, or GitHub — valid requests reach the unconfigured queue (502), while
+// rejected directories must return 400 first.
 import { describe, expect, test } from "bun:test";
 import { createSign, generateKeyPairSync } from "node:crypto";
 
@@ -56,29 +56,25 @@ function sessionsRequest(token: string, body: Record<string, unknown>): Request
   });
 }
 
-const base = { repo: "developing-today/code", prompt: "do the thing" };
+const base = { repo: "developing-today/code", prompt: "do the thing", issue: false };
 
-describe("POST /v1/sessions directory scoping (issue #2903)", () => {
-  test("OIDC + foreign directory is rejected with 400 before any side effect", async () => {
-    const token = signOidc("developing-today/code");
-    const res = await handler(sessionsRequest(token, { ...base, directory: "/tmp/evil" }));
-    expect(res.status).toBe(400);
-    const body = (await res.json()) as { error: string };
-    expect(body.error).toMatch(/directory/);
+describe("POST /v1/sessions directory scoping (issue #2664)", () => {
+  test("relative, foreign, and outside paths are rejected with 400 before any side effect", async () => {
+    for (const directory of ["src", "/home/user/git/opencode", "/home/user/git/code-other", "/tmp/evil"]) {
+      const res = await handler(sessionsRequest(signOidc("developing-today/code"), { ...base, directory }));
+      expect(res.status).toBe(400);
+      const body = (await res.json()) as { error: string };
+      expect(body.error).toMatch(/directory/);
+    }
   });
 
-  test("OIDC + omitted directory passes validation (fails later at the unconfigured queue)", async () => {
-    // No SUPABASE_SERVICE_ROLE_KEY in test env, so a request that clears
-    // validation reaches requireQueue() and maps to 502 — proving the
-    // directory gate let it through. Any network beyond JWKS throws.
-    const token = signOidc("developing-today/code");
-    const res = await handler(sessionsRequest(token, base));
-    expect(res.status).toBe(502);
-  });
-
-  test("OIDC + own clone directory passes validation the same way", async () => {
-    const token = signOidc("developing-today/code");
-    const res = await handler(sessionsRequest(token, { ...base, directory: "/home/user/git/code" }));
-    expect(res.status).toBe(502);
+  test("blank directory defaults and in-repo subdirectory passes validation", async () => {
+    for (const directory of [undefined, "", " \n ", "/home/user/git/code/src"]) {
+      const body = { ...base, ...(directory === undefined ? {} : { directory }) };
+      const res = await handler(sessionsRequest(signOidc("developing-today/code"), body));
+      // These requests pass validation; without a test queue they fail only
+      // when trying to enqueue, after the directory gate.
+      expect(res.status).toBe(502);
+    }
   });
 });

pm-stack/dispatch/test/validate.test.ts

@@ -131,6 +131,20 @@ describe("request validation", () => {
     expect(() => parseSessionRequest("nope")).toThrow(ValidationError);
   });
 
+  test("blank session titles are omitted so the prompt-derived title is used", () => {
+    for (const title of ["", " ", "\n\t"]) {
+      const request = parseSessionRequest({ repo: "developing-today/code", prompt: "  fix the thing\nmore", title });
+      expect(request.title).toBeUndefined();
+      expect(titleFromPrompt(request.prompt)).toBe("fix the thing");
+    }
+  });
+
+  test("nonblank session titles are preserved", () => {
+    expect(
+      parseSessionRequest({ repo: "developing-today/code", prompt: "p", title: "  custom title  " }).title,
+    ).toBe("  custom title  ");
+  });
+
   test("a prototype-spelling priority 400s at parse time, so no non-number reaches the packet (#2757)", () => {
     expect(() =>
       parseSessionRequest({ repo: "developing-today/code", prompt: "x", priority: "constructor" }),
@@ -189,39 +203,48 @@ describe("task issue_number parsing", () => {
   });
 });
 
-describe("session directory scoping (issue #2903)", () => {
+describe("session directory scoping (issue #2664)", () => {
   const repoDirs = {
     "developing-today/code": "/home/user/git/code",
     "developing-today/opencode": "/home/user/git/opencode",
   };
   const code = "developing-today/code";
 
-  test("OIDC + omitted directory resolves to the repo clone", () => {
-    expect(resolveSessionDirectory("oidc", code, undefined, repoDirs)).toBe("/home/user/git/code");
-  });
-
-  test("OIDC + own clone directory is accepted", () => {
-    expect(resolveSessionDirectory("oidc", code, "/home/user/git/code", repoDirs)).toBe("/home/user/git/code");
-  });
-
-  test("OIDC + own clone with trailing slashes is accepted and normalized", () => {
-    expect(resolveSessionDirectory("oidc", code, "/home/user/git/code///", repoDirs)).toBe("/home/user/git/code");
-  });
-
-  test("OIDC + another allowlisted clone is rejected", () => {
-    expect(() => resolveSessionDirectory("oidc", code, "/home/user/git/opencode", repoDirs)).toThrow(ValidationError);
-  });
+  for (const callerType of ["oidc", "bearer"] as const) {
+    test(`${callerType}: omitted, empty, and whitespace-only directory resolve to the repo clone`, () => {
+      for (const directory of [undefined, "", "  ", "\n\t"]) {
+        expect(resolveSessionDirectory(callerType, code, directory, repoDirs)).toBe("/home/user/git/code");
+      }
+    });
 
-  test("OIDC + arbitrary directory is rejected", () => {
-    expect(() => resolveSessionDirectory("oidc", code, "/tmp/evil", repoDirs)).toThrow(ValidationError);
-    expect(() => resolveSessionDirectory("oidc", code, "/home/user/git", repoDirs)).toThrow(ValidationError);
-  });
+    test(`${callerType}: root and normalized in-repo subdirectories are accepted`, () => {
+      expect(resolveSessionDirectory(callerType, code, "/home/user/git/code", repoDirs)).toBe("/home/user/git/code");
+      expect(resolveSessionDirectory(callerType, code, "/home/user/git/code///", repoDirs)).toBe("/home/user/git/code");
+      expect(resolveSessionDirectory(callerType, code, "/home/user/git/code/src", repoDirs)).toBe(
+        "/home/user/git/code/src",
+      );
+      expect(resolveSessionDirectory(callerType, code, "/home/user/git/code/src/../test/", repoDirs)).toBe(
+        "/home/user/git/code/test",
+      );
+    });
 
-  test("bearer + omitted directory resolves to the repo clone", () => {
-    expect(resolveSessionDirectory("bearer", code, undefined, repoDirs)).toBe("/home/user/git/code");
-  });
+    test(`${callerType}: relative, foreign, sibling-prefix, and outside paths are rejected`, () => {
+      for (const directory of [
+        "src",
+        "../opencode",
+        "/home/user/git/opencode",
+        "/home/user/git/code-other",
+        "/home/user/git",
+        "/tmp/evil",
+        "/home/user/git/code/../../etc",
+      ]) {
+        expect(() => resolveSessionDirectory(callerType, code, directory, repoDirs)).toThrow(ValidationError);
+      }
+    });
+  }
 
-  test("bearer + foreign directory is the documented deliberate choice", () => {
-    expect(resolveSessionDirectory("bearer", code, "/tmp/ops-scratch", repoDirs)).toBe("/tmp/ops-scratch");
+  test("unknown repo or missing root configuration fails closed", () => {
+    expect(() => resolveSessionDirectory("oidc", "unknown/repo", undefined, repoDirs)).toThrow(ValidationError);
+    expect(() => resolveSessionDirectory("bearer", "unknown/repo", "/tmp/evil", repoDirs)).toThrow(ValidationError);
   });
 });