fix: verify automation re-export path stays inside the automations root

electron/automationbuilder/builder.test.ts

@@ -0,0 +1,136 @@
+import assert from "node:assert/strict";
+import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import path from "node:path";
+import test from "node:test";
+
+import type { AutomationPlan, BuiltAutomation } from "../../common/automation";
+import { sessionDir } from "../recorder/session-store";
+import { AutomationBuilder } from "./builder";
+
+function samplePlan(overrides: Partial<AutomationPlan> = {}): AutomationPlan {
+  return {
+    architecture: "scout",
+    name: "daily-digest",
+    title: "Daily digest",
+    description: "Sends a daily digest.",
+    summary: "",
+    generalization: "",
+    trigger: {
+      type: "schedule",
+      schedule: { kind: "single", naturalLanguage: "", days: [], time: { hour: 9, minute: 0 } },
+      condition: "",
+    },
+    values: [],
+    steps: [{ label: "Send digest", prompt: "Send the daily digest email." }],
+    model: "",
+    skillNames: [],
+    ...overrides,
+  };
+}
+
+test("exportAutomation refuses to reuse a re-exported path outside the automations root", async () => {
+  const automationsRoot = await mkdtemp(path.join(tmpdir(), "skill-recorder-automations-"));
+  const sessionsRoot = await mkdtemp(path.join(tmpdir(), "skill-recorder-sessions-"));
+  const outsideRoot = await mkdtemp(path.join(tmpdir(), "skill-recorder-outside-"));
+
+  const previousAutomationsDir = process.env.SKILL_RECORDER_AUTOMATIONS_DIR;
+  const previousSessionsDir = process.env.SKILL_RECORDER_SESSIONS_DIR;
+  process.env.SKILL_RECORDER_AUTOMATIONS_DIR = automationsRoot;
+  process.env.SKILL_RECORDER_SESSIONS_DIR = sessionsRoot;
+
+  try {
+    const sessionId = "attack-session";
+
+    // Seed a persisted automation whose exportedPath was relocated outside the
+    // automations root (e.g. a tampered/relocated built-automation.json).
+    const maliciousExportedPath = path.join(outsideRoot, "automation.json");
+    const priorAutomation: BuiltAutomation = {
+      version: 1,
+      sessionId,
+      kind: "automation",
+      architecture: "scout",
+      name: "daily-digest",
+      description: "",
+      triggerType: "schedule",
+      schedule: { kind: "single", naturalLanguage: "", days: [], time: { hour: 9, minute: 0 } },
+      condition: "",
+      model: "",
+      steps: [{ label: "Send digest", prompt: "Send the daily digest email." }],
+      values: [],
+      plan: null,
+      createdAt: Date.now(),
+      exportedPath: maliciousExportedPath,
+      exportedAt: Date.now(),
+    };
+    await mkdir(sessionDir(sessionId), { recursive: true });
+    await writeFile(path.join(sessionDir(sessionId), "built-automation.json"), JSON.stringify(priorAutomation));
+
+    const builder = new AutomationBuilder(() => undefined);
+    const { path: exportedPath } = await builder.create(sessionId, samplePlan());
+
+    // The export must land inside the automations root, never inside the
+    // attacker-controlled outside directory the prior exportedPath pointed to.
+    const relative = path.relative(automationsRoot, exportedPath);
+    assert.ok(
+      relative !== "" && !relative.startsWith("..") && !path.isAbsolute(relative),
+      `expected export path inside ${automationsRoot}, got ${exportedPath}`,
+    );
+    assert.notEqual(path.dirname(exportedPath), outsideRoot);
+
+    // Nothing should have been written into the attacker-controlled directory.
+    await assert.rejects(readFile(maliciousExportedPath));
+  } finally {
+    if (previousAutomationsDir === undefined) delete process.env.SKILL_RECORDER_AUTOMATIONS_DIR;
+    else process.env.SKILL_RECORDER_AUTOMATIONS_DIR = previousAutomationsDir;
+    if (previousSessionsDir === undefined) delete process.env.SKILL_RECORDER_SESSIONS_DIR;
+    else process.env.SKILL_RECORDER_SESSIONS_DIR = previousSessionsDir;
+  }
+});
+
+test("exportAutomation reuses a prior export path when it is inside the automations root", async () => {
+  const automationsRoot = await mkdtemp(path.join(tmpdir(), "skill-recorder-automations-"));
+  const sessionsRoot = await mkdtemp(path.join(tmpdir(), "skill-recorder-sessions-"));
+
+  const previousAutomationsDir = process.env.SKILL_RECORDER_AUTOMATIONS_DIR;
+  const previousSessionsDir = process.env.SKILL_RECORDER_SESSIONS_DIR;
+  process.env.SKILL_RECORDER_AUTOMATIONS_DIR = automationsRoot;
+  process.env.SKILL_RECORDER_SESSIONS_DIR = sessionsRoot;
+
+  try {
+    const sessionId = "reuse-session";
+    const priorDir = path.join(automationsRoot, "daily-digest");
+    const priorExportedPath = path.join(priorDir, "automation.json");
+
+    const priorAutomation: BuiltAutomation = {
+      version: 1,
+      sessionId,
+      kind: "automation",
+      architecture: "scout",
+      name: "daily-digest",
+      description: "",
+      triggerType: "schedule",
+      schedule: { kind: "single", naturalLanguage: "", days: [], time: { hour: 9, minute: 0 } },
+      condition: "",
+      model: "",
+      steps: [{ label: "Send digest", prompt: "Send the daily digest email." }],
+      values: [],
+      plan: null,
+      createdAt: Date.now(),
+      exportedPath: priorExportedPath,
+      exportedAt: Date.now(),
+    };
+    await mkdir(sessionDir(sessionId), { recursive: true });
+    await writeFile(path.join(sessionDir(sessionId), "built-automation.json"), JSON.stringify(priorAutomation));
+
+    const builder = new AutomationBuilder(() => undefined);
+    const { path: exportedPath } = await builder.create(sessionId, samplePlan());
+
+    assert.equal(exportedPath, priorExportedPath);
+  } finally {
+    if (previousAutomationsDir === undefined) delete process.env.SKILL_RECORDER_AUTOMATIONS_DIR;
+    else process.env.SKILL_RECORDER_AUTOMATIONS_DIR = previousAutomationsDir;
+    if (previousSessionsDir === undefined) delete process.env.SKILL_RECORDER_SESSIONS_DIR;
+    else process.env.SKILL_RECORDER_SESSIONS_DIR = previousSessionsDir;
+  }
+});

electron/automationbuilder/builder.ts

@@ -21,6 +21,7 @@ import { createReadTools } from "../builders/read-tools";
 import { loadPersistedAnalysis } from "../describer/describer";
 import { createLogger } from "../logger";
 import { isValidSessionId, sessionDir } from "../recorder/session-store";
+import { isInside } from "../skillbuilder/builder";
 import { AUTOMATION_BUILDER_INSTRUCTIONS } from "./instructions";
 import { automationCatalogueFor } from "./scout-automation-catalog";
 import { createAutomationBuilderTools } from "./tools";
@@ -219,10 +220,13 @@ export class AutomationBuilder extends AgentBuilder<LiveBuild> {
     const root = automationsRoot();
     const name = slugifySkillName(automation.name);
     const prior = loadPersistedAutomation(automation.sessionId);
-    // Re-export to the same folder if this session already exported one; otherwise pick
-    // a fresh, non-colliding directory so we never clobber an unrelated automation.
-    let dir = prior?.exportedPath ? path.dirname(prior.exportedPath) : path.join(root, name);
-    if (!prior?.exportedPath && existsSync(dir)) {
+    const priorDir = prior?.exportedPath ? path.dirname(prior.exportedPath) : null;
+    // Re-export to the same folder only when it already lives under the automations root;
+    // a relocated/tampered `exportedPath` must not be reused here. Otherwise pick a fresh,
+    // non-colliding directory so we never clobber an unrelated automation.
+    const reuse = priorDir !== null && isInside(root, priorDir);
+    let dir = reuse ? priorDir : path.join(root, name);
+    if (!reuse && existsSync(dir)) {
       let n = 2;
       while (existsSync(path.join(root, `${name}-${n}`))) n++;
       dir = path.join(root, `${name}-${n}`);

electron/skillbuilder/builder.ts

@@ -53,7 +53,7 @@ function skillsRoot(): string {
 }
 
 /** True when `dir` is `root` or nested inside it (so we can safely re-use it). */
-function isInside(root: string, dir: string): boolean {
+export function isInside(root: string, dir: string): boolean {
   const rel = path.relative(root, dir);
   return rel === "" || (!rel.startsWith("..") && !path.isAbsolute(rel));
 }

package.json

@@ -14,7 +14,7 @@
     "build": "tsc --noEmit && vite build",
     "typecheck": "tsc --noEmit",
     "typecheck:evals": "tsc --noEmit -p evals/tsconfig.json",
-    "test": "node --experimental-transform-types --no-warnings --import ./evals/register.mjs --test common/audio.test.ts common/microphone.test.ts common/narration.test.ts electron/recording-controls-bounds.test.ts electron/recording-privacy.test.ts electron/recorder/controller.test.ts electron/frames/extractor.test.ts electron/narration/audio-analysis.test.ts electron/narration/analyze-gate.test.ts electron/narration/transcribe.test.ts electron/narration/whisper.test.ts electron/sessions.test.ts electron/debug-bundle.test.ts scripts/compliance.test.mjs",
+    "test": "node --experimental-transform-types --no-warnings --import ./evals/register.mjs --test common/audio.test.ts common/microphone.test.ts common/narration.test.ts electron/recording-controls-bounds.test.ts electron/recording-privacy.test.ts electron/recorder/controller.test.ts electron/frames/extractor.test.ts electron/narration/audio-analysis.test.ts electron/narration/analyze-gate.test.ts electron/narration/transcribe.test.ts electron/narration/whisper.test.ts electron/sessions.test.ts electron/debug-bundle.test.ts electron/automationbuilder/builder.test.ts scripts/compliance.test.mjs",
     "eval": "node --experimental-transform-types --no-warnings --import ./evals/register.mjs evals/run.ts",
     "eval:builder": "node --experimental-transform-types --no-warnings --import ./evals/register.mjs evals/builder/run.ts",
     "eval:skill": "node --experimental-transform-types --no-warnings --import ./evals/register.mjs evals/skillbuilder/run.ts",