fix(lr-d049): validate loop registry ID to block path traversal

lib/project-loop.js

@@ -6,6 +6,20 @@ var { createLoopRegistry } = require("./scheduler");
 var store = require("./store");
 var usersModule = require("./users");
 
+var LOOP_ID_RE = /^loop_[A-Za-z0-9_-]+$/;
+
+/**
+ * Validate a loop ID before using it as a path component.
+ *
+ * Returns true if the id passes the allowlist regex.
+ * Returns false for any value that could enable path traversal or injection.
+ * All handlers that build a loop directory path from a user-supplied value
+ * must route through this function before any path.join / path.resolve call.
+ */
+function validateLoopId(id) {
+  return LOOP_ID_RE.test(id || "");
+}
+
 /**
  * Attach loop engine to a project context.
  *
@@ -284,6 +298,13 @@ function attachLoop(ctx) {
       console.log("[loop-registry] Schedule triggered: " + record.name + " -> linked task " + loopFilesId);
     }
 
+    // Validate the loop ID before using it as a path component (path traversal guard).
+    // linkedTaskId flows from client-supplied sData.taskId via schedule_create.
+    if (!validateLoopId(loopFilesId)) {
+      console.error("[loop-registry] Invalid loop ID rejected: " + loopFilesId);
+      return;
+    }
+
     // Verify the loop directory and PROMPT.md exist
     var recDir = path.join(cwd, ".claude", "loops", loopFilesId);
     try {
@@ -1010,7 +1031,17 @@ function attachLoop(ctx) {
 
     if (msg.type === "loop_registry_files") {
       var recId = msg.id;
-      var lDir = path.join(cwd, ".claude", "loops", recId);
+      // Validate ID before using it as a path component (path traversal guard).
+      if (!validateLoopId(recId)) {
+        send({ type: "loop_registry_error", text: "invalid_loop_id" });
+        return true;
+      }
+      var loopBase = path.resolve(cwd, ".claude", "loops");
+      var lDir = path.resolve(loopBase, recId);
+      if (!lDir.startsWith(loopBase + path.sep)) {
+        send({ type: "loop_registry_error", text: "invalid_loop_id" });
+        return true;
+      }
       var promptContent = "";
       var judgeContent = "";
       var loopSettings = null;
@@ -1032,7 +1063,17 @@ function attachLoop(ctx) {
 
     if (msg.type === "loop_registry_save_files") {
       var recId2 = msg.id;
-      var lDir2 = path.join(cwd, ".claude", "loops", recId2);
+      // Validate ID before using it as a path component (path traversal guard).
+      if (!validateLoopId(recId2)) {
+        send({ type: "loop_registry_error", text: "invalid_loop_id" });
+        return true;
+      }
+      var loopBase2 = path.resolve(cwd, ".claude", "loops");
+      var lDir2 = path.resolve(loopBase2, recId2);
+      if (!lDir2.startsWith(loopBase2 + path.sep)) {
+        send({ type: "loop_registry_error", text: "invalid_loop_id" });
+        return true;
+      }
       try {
         fs.mkdirSync(lDir2, { recursive: true });
         if (msg.prompt !== undefined) {
@@ -1348,4 +1389,4 @@ function attachLoop(ctx) {
   };
 }
 
-module.exports = { attachLoop: attachLoop };
+module.exports = { attachLoop: attachLoop, validateLoopId: validateLoopId };

lib/scheduler.js

@@ -12,6 +12,11 @@ var path = require("path");
 var { CONFIG_DIR } = require("./config");
 var { encodeCwd } = require("./utils");
 
+// NOTE: this regex mirrors validateLoopId() in project-loop.js. A circular
+// import prevents sharing the function directly (project-loop requires this
+// module). If the allowlist changes, update both. Tracked: lr-d049 nit.
+var LOOP_ID_RE = /^loop_[A-Za-z0-9_-]+$/;
+
 // --- Cron parser (5-field: minute hour day-of-month month day-of-week) ---
 
 function parseCronField(field, min, max) {
@@ -297,8 +302,15 @@ function createLoopRegistry(opts) {
   // --- CRUD ---
 
   function register(data) {
+    // Validate caller-supplied id before storing it.
+    // Auto-generated IDs (the default) always pass the regex; this guard
+    // blocks client-supplied IDs that could later be used in path construction.
+    var suppliedId = data.id || null;
+    if (suppliedId !== null && !LOOP_ID_RE.test(suppliedId)) {
+      throw new Error("invalid_loop_id: " + suppliedId);
+    }
     var rec = {
-      id: data.id || ("loop_" + Date.now() + "_" + require("crypto").randomBytes(3).toString("hex")),
+      id: suppliedId || ("loop_" + Date.now() + "_" + require("crypto").randomBytes(3).toString("hex")),
       name: data.name || "Untitled",
       task: data.task || "",
       cron: data.cron || null,

test/security.test.js

@@ -1324,6 +1324,158 @@ test("server-dm.js dm_typing handler: silently drops traversal dmKey", function
   assert.strictEqual(forEachClientCalled, false, "forEachClient must not be called for traversal key");
 });
 
+// ============================================================
+// 18. Loop registry path traversal prevention (lr-d049)
+//     Tests the REAL production validateLoopId exported from project-loop.js
+//     and drives the real handleLoopMessage handler through a minimal ctx stub
+//     to confirm the guard fires before any filesystem access.
+// ============================================================
+
+var { validateLoopId: prodValidateLoopId, attachLoop } = require("../lib/project-loop");
+
+// --- helpers ---
+
+// Build a minimal ctx stub sufficient for attachLoop.
+// The invalid-ID guard fires before any sm/sdk/fs access, so most
+// ctx fields can be no-ops for the invalid-ID tests.
+function makeLoopCtx(cwd, sendSpy) {
+  var sessions = new Map();
+  return {
+    cwd: cwd,
+    slug: "test",
+    sm: {
+      sessions: sessions,
+      createSession: function () { return { localId: "s1", history: [], loop: null }; },
+      saveSessionFile: function () {},
+      appendToSessionFile: function () {},
+      switchSession: function () {},
+      broadcastSessionList: function () {},
+      deleteSessionQuiet: function () {},
+      setResolveLoopInfo: function () {},
+    },
+    sdk: { startQuery: function () {} },
+    send: sendSpy,
+    sendTo: function (_ws, msg) { sendSpy(msg); },
+    sendToSession: function () {},
+    pushModule: null,
+    notificationsModule: null,
+    getHubSchedules: function () { return []; },
+    getAllProjectSessions: function () { return []; },
+    getStatus: function () { return null; },
+    getLinuxUserForSession: function () { return null; },
+    onProcessingChanged: function () {},
+    hydrateImageRefs: function () {},
+  };
+}
+
+// --- validateLoopId unit tests (call the REAL production export) ---
+
+test("validateLoopId: rejects path traversal payload", function () {
+  assert.strictEqual(prodValidateLoopId("../../../etc/passwd"), false,
+    "../../../etc/passwd must be rejected");
+});
+
+test("validateLoopId: rejects null/undefined/empty id", function () {
+  assert.strictEqual(prodValidateLoopId(null), false, "null must be rejected");
+  assert.strictEqual(prodValidateLoopId(undefined), false, "undefined must be rejected");
+  assert.strictEqual(prodValidateLoopId(""), false, "empty string must be rejected");
+});
+
+test("validateLoopId: rejects id without loop_ prefix", function () {
+  assert.strictEqual(prodValidateLoopId("abc123"), false, "no prefix must be rejected");
+  assert.strictEqual(prodValidateLoopId("loop"), false, "bare 'loop' must be rejected");
+});
+
+test("validateLoopId: rejects id with dots or slashes", function () {
+  assert.strictEqual(prodValidateLoopId("loop_abc/../../../etc/shadow"), false,
+    "id with ../ must be rejected");
+  assert.strictEqual(prodValidateLoopId("loop_abc/subdir"), false,
+    "id with embedded slash must be rejected");
+  assert.strictEqual(prodValidateLoopId("loop_abc.evil"), false,
+    "id with dot must be rejected");
+});
+
+test("validateLoopId: rejects null byte and shell metacharacters", function () {
+  assert.strictEqual(prodValidateLoopId("loop_abc\x00def"), false, "null byte must be rejected");
+  assert.strictEqual(prodValidateLoopId("loop_abc;whoami"), false, "semicolon must be rejected");
+  assert.strictEqual(prodValidateLoopId("loop_abc def"), false, "space must be rejected");
+});
+
+test("validateLoopId: accepts well-formed loop IDs", function () {
+  var valid = [
+    "loop_abc",
+    "loop_ABC123",
+    "loop_my-task",
+    "loop_task_2025",
+    "loop_A-b_C-d",
+  ];
+  for (var i = 0; i < valid.length; i++) {
+    assert.strictEqual(prodValidateLoopId(valid[i]), true,
+      valid[i] + " should be accepted");
+  }
+});
+
+// --- handleLoopMessage integration tests ---
+// Drive the REAL handler dispatch path through a minimal ctx stub.
+// For invalid IDs the guard fires before any fs operation; we assert the
+// correct error is emitted and no exception propagates.
+
+test("loop_registry_files handler: sends loop_registry_error for invalid id via real handleLoopMessage", function () {
+  var tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "loop-h-test-"));
+  try {
+    var sent = [];
+    var ctx = makeLoopCtx(tmpDir, function (msg) { sent.push(msg); });
+    var loop = attachLoop(ctx);
+
+    var attackIds = [
+      "../../../etc/passwd",
+      "loop_ok/../../../tmp/evil",
+      "loop_ok%2F..%2F..%2Fetc%2Fpasswd",
+      null,
+      "",
+      "abc123",
+    ];
+    for (var i = 0; i < attackIds.length; i++) {
+      sent = [];
+      loop.handleLoopMessage({}, { type: "loop_registry_files", id: attackIds[i] });
+      assert.ok(sent.length >= 1, "handler must emit a message for id=" + attackIds[i]);
+      assert.strictEqual(sent[0].type, "loop_registry_error",
+        "must emit loop_registry_error for id=" + attackIds[i]);
+      assert.strictEqual(sent[0].text, "invalid_loop_id",
+        "error text must be invalid_loop_id for id=" + attackIds[i]);
+    }
+  } finally {
+    fs.rmSync(tmpDir, { recursive: true });
+  }
+});
+
+test("loop_registry_save_files handler: sends loop_registry_error for invalid id via real handleLoopMessage", function () {
+  var tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "loop-h-test-"));
+  try {
+    var sent = [];
+    var ctx = makeLoopCtx(tmpDir, function (msg) { sent.push(msg); });
+    var loop = attachLoop(ctx);
+
+    var attackIds = [
+      "../../../etc/passwd",
+      "loop_ok/../../../tmp/evil",
+      null,
+      "",
+    ];
+    for (var i = 0; i < attackIds.length; i++) {
+      sent = [];
+      loop.handleLoopMessage({}, { type: "loop_registry_save_files", id: attackIds[i] });
+      assert.ok(sent.length >= 1, "handler must emit a message for id=" + attackIds[i]);
+      assert.strictEqual(sent[0].type, "loop_registry_error",
+        "must emit loop_registry_error for id=" + attackIds[i]);
+      assert.strictEqual(sent[0].text, "invalid_loop_id",
+        "error text must be invalid_loop_id for id=" + attackIds[i]);
+    }
+  } finally {
+    fs.rmSync(tmpDir, { recursive: true });
+  }
+});
+
 test("server-dm.js dm_send handler: allows valid key and calls dm.sendMessage", function () {
   var { attachDm } = require("../lib/server-dm");