From ddca1d3926972c2fcd95820337d70db5925d1322 Mon Sep 17 00:00:00 2001 From: clagentic <10177887+akuehner@users.noreply.github.com> Date: Wed, 10 Jun 2026 21:44:31 -0400 Subject: [PATCH 1/2] fix(lr-eb1a): add missing permission gates on schedule_move, set_session_visibility, kill_process Three WS handlers in project-sessions.js skipped access-control checks that all sibling handlers enforce: - schedule_move: add scheduledTasks permission check matching the gate already present for schedule_create in project-user-message.js. Without this fix, the sessions.js handler short-circuits before the gate in project-user-message.js is reached. - set_session_visibility: add canAccessSession guard before calling sm.setSessionVisibility, matching the pattern used by set_session_agent, set_session_bookmark, and reorder_session_bookmarks. - kill_process: add to the admin-only gate block so non-admin users cannot SIGTERM other users' claude subprocesses in multi-user mode. Single-user mode (isMultiUser() === false) behavior is unchanged for all three fixes. Co-Authored-By: Claude Sonnet 4.6 --- lib/project-sessions.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/project-sessions.js b/lib/project-sessions.js index a68da8a..886f061 100644 --- a/lib/project-sessions.js +++ b/lib/project-sessions.js @@ -231,6 +231,11 @@ function attachSessions(ctx) { if (msg.type === "set_session_visibility") { if (typeof msg.sessionId === "number" && (msg.visibility === "shared" || msg.visibility === "private")) { + var visTarget = sm.sessions.get(msg.sessionId); + if (!visTarget) return true; + if (usersModule.isMultiUser() && ws._clayUser) { + if (!usersModule.canAccessSession(ws._clayUser.id, visTarget, { visibility: "public" })) return true; + } sm.setSessionVisibility(msg.sessionId, msg.visibility); } return true; @@ -1405,6 +1410,13 @@ function attachSessions(ctx) { // --- Move a single schedule to another project --- if (msg.type === "schedule_move") { + if (ws._clayUser) { + var schMovePerms = usersModule.getEffectivePermissions(ws._clayUser, osUsers); + if (!schMovePerms.scheduledTasks) { + sendTo(ws, { type: "error", text: "Scheduled tasks access is not permitted" }); + return true; + } + } var moveResult = moveScheduleToProject(msg.recordId, msg.fromSlug, msg.toSlug); if (moveResult.ok) { // Re-broadcast updated records to this project's clients @@ -1525,7 +1537,7 @@ function attachSessions(ctx) { // --- Daemon config / server management (admin-only in multi-user mode) --- if (msg.type === "get_daemon_config" || msg.type === "set_pin" || msg.type === "set_keep_awake" || msg.type === "set_auto_continue" || msg.type === "set_image_retention" || msg.type === "set_mem_available_threshold" || - msg.type === "shutdown_server" || msg.type === "restart_server") { + msg.type === "shutdown_server" || msg.type === "restart_server" || msg.type === "kill_process") { if (usersModule.isMultiUser()) { var _wsUser = ws._clayUser; if (!_wsUser || _wsUser.role !== "admin") { From 6ebd932d1b90444944cb3b9e9e26d6d8b4353cb9 Mon Sep 17 00:00:00 2001 From: clagentic <10177887+akuehner@users.noreply.github.com> Date: Wed, 10 Jun 2026 21:50:38 -0400 Subject: [PATCH 2/2] fix(lr-eb1a): move kill_process admin gate inline; add regression tests for all three WS permission gates - kill_process gate was dead (handler return true before line-1540 gate); moved inline at handler entry matching the update_now pattern (line 533). - Removed kill_process from the line-1540 gate block (it never reached there). - test/ws-permission-gates-lr-eb1a.test.js: 7 tests driving real attachSessions; covers 3a (schedule_move denied/allowed by scheduledTasks perm), 3b (set_session_visibility denied/allowed by canAccessSession), 3c (kill_process denied for non-admin, allowed for admin, allowed single-user). Co-Authored-By: Claude Sonnet 4.5 --- lib/project-sessions.js | 8 +- test/ws-permission-gates-lr-eb1a.test.js | 259 +++++++++++++++++++++++ 2 files changed, 266 insertions(+), 1 deletion(-) create mode 100644 test/ws-permission-gates-lr-eb1a.test.js diff --git a/lib/project-sessions.js b/lib/project-sessions.js index 886f061..fe6cd64 100644 --- a/lib/project-sessions.js +++ b/lib/project-sessions.js @@ -592,6 +592,12 @@ function attachSessions(ctx) { } if (msg.type === "kill_process") { + // Restrict to admins in multi-user mode — any user could otherwise SIGTERM + // another user's running claude subprocess (lr-eb1a). + if (usersModule.isMultiUser() && (!ws._clayUser || ws._clayUser.role !== "admin")) { + sendTo(ws, { type: "error", message: "Admin access required" }); + return true; + } var pid = msg.pid; if (!pid || typeof pid !== "number") return true; // Verify target is actually a claude process before killing @@ -1537,7 +1543,7 @@ function attachSessions(ctx) { // --- Daemon config / server management (admin-only in multi-user mode) --- if (msg.type === "get_daemon_config" || msg.type === "set_pin" || msg.type === "set_keep_awake" || msg.type === "set_auto_continue" || msg.type === "set_image_retention" || msg.type === "set_mem_available_threshold" || - msg.type === "shutdown_server" || msg.type === "restart_server" || msg.type === "kill_process") { + msg.type === "shutdown_server" || msg.type === "restart_server") { if (usersModule.isMultiUser()) { var _wsUser = ws._clayUser; if (!_wsUser || _wsUser.role !== "admin") { diff --git a/test/ws-permission-gates-lr-eb1a.test.js b/test/ws-permission-gates-lr-eb1a.test.js new file mode 100644 index 0000000..8022520 --- /dev/null +++ b/test/ws-permission-gates-lr-eb1a.test.js @@ -0,0 +1,259 @@ +"use strict"; +// Regression tests for lr-eb1a: three WS handlers lacked access-control gates. +// +// 3a. schedule_move: scheduledTasks permission required (was bypassed before +// project-user-message.js gate due to short-circuit in project-sessions.js) +// 3b. set_session_visibility: canAccessSession required (sibling handlers +// already had this gate; set_session_visibility did not) +// 3c. kill_process: admin-only in multi-user mode (isClaudeProcess check was +// sufficient in single-user but allowed cross-user SIGTERM in multi-user) +// +// All tests drive the real attachSessions handler from project-sessions.js. +// Tests fail if the gates are removed. + +var test = require("node:test"); +var assert = require("node:assert/strict"); + +var { attachSessions } = require("../lib/project-sessions"); + +// --------------------------------------------------------------------------- +// Shared ctx builder +// --------------------------------------------------------------------------- + +function makeCtx(overrides) { + overrides = overrides || {}; + + var regularUser = { id: "user-1", role: "user", permissions: {} }; + var adminUser = { id: "user-2", role: "admin", permissions: { scheduledTasks: true } }; + var otherSession = { localId: 99, ownerId: "user-9", sessionVisibility: "private" }; + var wsSession = { localId: 1, ownerId: "user-1", sessionVisibility: "private" }; + + var killCalls = []; + var moveCalls = []; + var visCalls = []; + + var ctx = Object.assign({ + cwd: "/tmp/test-eb1a", slug: "test", osUsers: false, currentVersion: "0.0.0", + sm: { + sessions: new Map([[1, wsSession], [99, otherSession]]), + getActiveSession: function() { return wsSession; }, + broadcastSessionList: function() {}, + setSessionVisibility: function(id, vis) { visCalls.push({ id: id, vis: vis }); return { ok: true }; }, + }, + sdk: { + stopTask: function() { return Promise.resolve(); }, + isClaudeProcess: function() { return true; }, // always claims valid claude proc + }, + tm: null, clients: [], opts: {}, + send: function() {}, + sendTo: function() {}, + sendToAdmins: function() {}, + sendToSession: function() {}, + sendToSessionOthers: function() {}, + usersModule: { + isMultiUser: function() { return true; }, // default: multi-user mode + canAccessSession: function(userId, session) { + // user can access their own session only + return session.ownerId === userId; + }, + getUserPermission: function(user, perm) { + return user && user.permissions && !!user.permissions[perm]; + }, + // schedule_move uses getEffectivePermissions (returns permissions object) + getEffectivePermissions: function(user) { + return user ? (user.permissions || {}) : {}; + }, + }, + userPresence: null, pushModule: null, + getSessionForWs: function(ws) { return ws._session || null; }, + getLinuxUserForSession: function() { return null; }, + ensureProjectAccessForSession: function() { return true; }, + getOsUserInfoForWs: function() { return null; }, + hydrateImageRefs: function(o) { return o; }, + onProcessingChanged: function() {}, + broadcastPresence: function() {}, + adapter: null, + getProjectList: function() { return []; }, + getProjectCount: function() { return 0; }, + getScheduleCount: function() { return 0; }, + moveScheduleToProject: function(args) { moveCalls.push(args); }, + }, overrides); + + return { + ctx: ctx, + regularUser: regularUser, + adminUser: adminUser, + wsSession: wsSession, + otherSession: otherSession, + killCalls: killCalls, + moveCalls: moveCalls, + visCalls: visCalls, + }; +} + +// --------------------------------------------------------------------------- +// 3a — schedule_move: scheduledTasks permission gate +// --------------------------------------------------------------------------- + +test("3a: schedule_move denied for user without scheduledTasks permission", function() { + var stub = makeCtx(); + // regularUser has no scheduledTasks permission + stub.ctx.usersModule.getEffectivePermissions = function(user) { + return user && user.permissions ? user.permissions : {}; + }; + var errors = []; + stub.ctx.sendTo = function(ws, msg) { if (msg.type === "error") errors.push(msg); }; + + var h = attachSessions(stub.ctx); + var ws = { _session: stub.wsSession, _clayUser: stub.regularUser }; // regularUser: { permissions: {} } + h.handleSessionsMessage(ws, { + type: "schedule_move", + recordId: "sched-1", + fromSlug: "proj-1", + toSlug: "proj-2", + }); + + assert.equal(stub.moveCalls.length, 0, + "moveScheduleToProject must not be called without scheduledTasks permission"); + assert.ok(errors.length > 0, "non-permitted user should receive an error"); +}); + +test("3a: schedule_move allowed for user with scheduledTasks permission", function() { + var stub = makeCtx(); + stub.ctx.usersModule.getEffectivePermissions = function(user) { + return user && user.permissions ? user.permissions : {}; + }; + var moveCalls = []; + stub.ctx.moveScheduleToProject = function() { moveCalls.push(true); return { ok: true }; }; + + // Stub getHubSchedules (used in the ok branch) + stub.ctx.getHubSchedules = function() { return []; }; + + var h = attachSessions(stub.ctx); + var ws = { _session: stub.wsSession, _clayUser: stub.adminUser }; // adminUser: { permissions: { scheduledTasks: true } } + h.handleSessionsMessage(ws, { + type: "schedule_move", + recordId: "sched-1", + fromSlug: "proj-1", + toSlug: "proj-2", + }); + + assert.equal(moveCalls.length, 1, + "moveScheduleToProject should be called when scheduledTasks permission is granted"); +}); + +// --------------------------------------------------------------------------- +// 3b — set_session_visibility: canAccessSession gate +// --------------------------------------------------------------------------- + +test("3b: set_session_visibility denied for user who cannot access target session", function() { + var stub = makeCtx(); + var errors = []; + stub.ctx.sendTo = function(ws, msg) { if (msg.type === "error") errors.push(msg); }; + // regular user (user-1) trying to change otherSession (owned by user-9) + stub.ctx.usersModule.canAccessSession = function(userId, session) { + return session.ownerId === userId; + }; + + var h = attachSessions(stub.ctx); + var ws = { _session: stub.wsSession, _clayUser: stub.regularUser }; + h.handleSessionsMessage(ws, { + type: "set_session_visibility", + sessionId: 99, // otherSession.localId + visibility: "shared", + }); + + assert.equal(stub.visCalls.length, 0, + "setSessionVisibility must not be called for a session the user cannot access"); +}); + +test("3b: set_session_visibility allowed for user who owns the session", function() { + var stub = makeCtx(); + stub.ctx.usersModule.canAccessSession = function(userId, session) { + return session.ownerId === userId; + }; + + var h = attachSessions(stub.ctx); + var ws = { _session: stub.wsSession, _clayUser: stub.regularUser }; + h.handleSessionsMessage(ws, { + type: "set_session_visibility", + sessionId: 1, // wsSession.localId — owned by regularUser + visibility: "shared", + }); + + assert.equal(stub.visCalls.length, 1, + "setSessionVisibility should proceed when user owns the session"); + assert.equal(stub.visCalls[0].vis, "shared"); +}); + +// --------------------------------------------------------------------------- +// 3c — kill_process: admin-only in multi-user mode +// --------------------------------------------------------------------------- + +test("3c: kill_process denied for non-admin in multi-user mode", function() { + var killed = []; + var errors = []; + // Patch process.kill to spy without actually killing anything + var origKill = process.kill.bind(process); + process.kill = function(pid, sig) { killed.push({ pid: pid, sig: sig }); }; + + var stub = makeCtx(); + stub.ctx.sendTo = function(ws, msg) { if (msg.type === "error") errors.push(msg); }; + + try { + var h = attachSessions(stub.ctx); + var ws = { _session: stub.wsSession, _clayUser: stub.regularUser }; + h.handleSessionsMessage(ws, { type: "kill_process", pid: 99999 }); + + assert.equal(killed.length, 0, + "process.kill must not be called for non-admin in multi-user mode"); + assert.ok(errors.length > 0, + "non-admin should receive an error response"); + } finally { + process.kill = origKill; + } +}); + +test("3c: kill_process allowed for admin in multi-user mode", function() { + var killed = []; + var origKill = process.kill.bind(process); + process.kill = function(pid, sig) { killed.push({ pid: pid, sig: sig }); }; + + var stub = makeCtx(); + stub.ctx.sdk.isClaudeProcess = function() { return true; }; + + try { + var h = attachSessions(stub.ctx); + var ws = { _session: stub.wsSession, _clayUser: stub.adminUser }; + h.handleSessionsMessage(ws, { type: "kill_process", pid: 99999 }); + + assert.equal(killed.length, 1, + "process.kill should be called when admin issues kill_process"); + assert.equal(killed[0].pid, 99999); + assert.equal(killed[0].sig, "SIGTERM"); + } finally { + process.kill = origKill; + } +}); + +test("3c: kill_process allowed in single-user mode (no auth required)", function() { + var killed = []; + var origKill = process.kill.bind(process); + process.kill = function(pid, sig) { killed.push({ pid: pid, sig: sig }); }; + + var stub = makeCtx(); + // Single-user mode + stub.ctx.usersModule.isMultiUser = function() { return false; }; + stub.ctx.sdk.isClaudeProcess = function() { return true; }; + + try { + var h = attachSessions(stub.ctx); + var ws = { _session: stub.wsSession, _clayUser: stub.regularUser }; + h.handleSessionsMessage(ws, { type: "kill_process", pid: 99999 }); + + assert.equal(killed.length, 1, + "kill_process should work without auth in single-user mode"); + } finally { + process.kill = origKill; + } +});