diff --git a/CHANGELOG.md b/CHANGELOG.md index de48ddf..32fe13e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +### Added +- **Responder-lock enforcement in the MCP room tools (#42)**: `room_post` and `alert_recipient` now refuse when another live session holds the machine's room-responder lock — a PASSIVE session that ignores its injected instructions still cannot post as the agent through IAK tooling. Lock semantics mirror the SessionStart hook exactly (pid + process-start-time identity, stale locks ignored, fail-open on mechanics); the owning session is recognized by finding the lock's pid in the caller's process-ancestor chain. Read-only room tools (`room_recent`) are unaffected. Raw HTTP posting against GroupMind is out of scope client-side and tracked in issue #42's server-side follow-up. + ### Fixed - **Duplicate room-responder voices**: `session-bootstrap.sh` now enforces a single room responder per notification file via a lock file (`.responder.lock`). The first session claims the lock and gets the full bootstrap; any later session on the same machine gets PASSIVE instructions (serve the user directly, never answer the room) instead of booting a second copy of the same handle. The lock stores the owner's pid + session id + process start time, so the owner reclaims it across resume/compact, a lock whose owner process died is stolen automatically, and a recycled pid never masquerades as the owner (adversarial review by @codexmb: initial claim is `O_EXCL`-atomic, aliveness uses `ps` so a cross-user EPERM never reads as dead, and every winner re-verifies ownership after a settle window so concurrent starts elect exactly one responder). Disable with `IAK_RESPONDER_LOCK=off`, relocate with `IAK_RESPONDER_LOCK=`. Motivation: on 2026-07-16 a stray `claude rc` session on the Mac mini booted the same bootstrap and posted to the room as a second @claudemm, producing conflicting answers. diff --git a/src/mcp-server.mjs b/src/mcp-server.mjs index 8ee1b45..7e8fef6 100644 --- a/src/mcp-server.mjs +++ b/src/mcp-server.mjs @@ -33,6 +33,7 @@ import { import { nudgeTmux } from './common/notify.mjs'; import { tmuxRun } from './ide/tmux-runner.mjs'; import { loadConfig } from './config.mjs'; +import { assertRoomVoice } from './responder-lock.mjs'; import { createIntent, decideIntent, @@ -710,6 +711,11 @@ export async function runMcpServer({ configPath } = {}) { } case 'room_post': { if (!roomToolsEnabled) return err('room_post: room API is not configured.'); + // #42: a PASSIVE session (another live session holds the machine's + // room-responder lock) is refused here even if it ignores its + // injected instructions. Read-only room tools stay available. + const voice = assertRoomVoice({ config }); + if (!voice.allowed) return err(`room_post refused: ${voice.reason}`); const posted = await postRoomMessage({ config, room: args.room, @@ -727,6 +733,8 @@ export async function runMcpServer({ configPath } = {}) { if (!roomToolsEnabled) return err('alert_recipient: room API is not configured.'); if (!args.handle) return err('alert_recipient: handle is required'); if (!args.body) return err('alert_recipient: body is required'); + const voice = assertRoomVoice({ config }); + if (!voice.allowed) return err(`alert_recipient refused: ${voice.reason}`); const handle = String(args.handle).startsWith('@') ? String(args.handle) : `@${args.handle}`; const posted = await postRoomMessage({ config, diff --git a/src/responder-lock.mjs b/src/responder-lock.mjs new file mode 100644 index 0000000..7a7e453 --- /dev/null +++ b/src/responder-lock.mjs @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: AGPL-3.0-only +// +// Enforcement half of the single room-responder lock (#40 wrote it, #42 +// enforces it): scripts/session-bootstrap.sh gives PASSIVE sessions +// instructions not to answer the room, but instructions are model-followed. +// This module lets the MCP room-post path REFUSE the post when another live +// session holds the machine's responder lock, so a session that ignores its +// PASSIVE context still cannot speak as the agent through IAK tooling. +// (Raw curl against the GroupMind API is out of scope here — that needs +// server-side per-session keys; see issue #42.) +// +// Semantics mirror the bash hook exactly: +// - lock file: $IAK_RESPONDER_LOCK, else .responder.lock; +// the literal value "off" disables enforcement. +// - owner identity: pid + process start time (a recycled pid is stale). +// - fail OPEN on mechanics (missing/corrupt lock, ps failures): a machine +// with a broken lock keeps its room voice; the hook is the same way. +// - "we hold the lock": the lock's pid is this process or one of its +// ancestors — the MCP server is spawned by the session process, so the +// owning session appears in our parent chain. + +import { readFileSync } from 'node:fs'; +import { execFileSync } from 'node:child_process'; + +export function lockPathFor(config, env = process.env) { + const explicit = env.IAK_RESPONDER_LOCK; + if (explicit) return explicit; // may be the literal 'off' + const newFile = + env.IAK_NEW_FILE || + config?.poller?.notification_file || + '/tmp/iak-new-messages.txt'; + return `${newFile}.responder.lock`; +} + +export function readResponderLock(lockPath) { + try { + const raw = readFileSync(lockPath, 'utf8'); + const get = (k) => { + const m = raw.match(new RegExp(`^${k}=(.*)$`, 'm')); + return m ? m[1].trim() : ''; + }; + const pid = parseInt(get('pid'), 10); + return { + pid: Number.isInteger(pid) && pid > 0 ? pid : null, + sid: get('sid'), + pstart: get('pstart'), + }; + } catch { + return null; + } +} + +function procStart(pid, exec) { + try { + return exec('ps', ['-p', String(pid), '-o', 'lstart='], { encoding: 'utf8' }).trim(); + } catch { + return ''; + } +} + +// Returns the parent pid, 0 when the chain legitimately ended (ps ran and +// the pid is gone), or null when the LOOKUP ITSELF failed (spawn-level +// error) — the caller must treat null as unknown, never as "not an +// ancestor", or a transient ps failure would refuse a legitimate owner +// (codex review on #45). +function parentOf(pid, exec) { + try { + const out = exec('ps', ['-p', String(pid), '-o', 'ppid='], { encoding: 'utf8' }).trim(); + const parent = parseInt(out, 10); + return Number.isInteger(parent) && parent > 0 ? parent : 0; + } catch (e) { + // execFileSync sets .status when the command RAN and exited nonzero — + // for ps that means "no such pid": the chain ended. No .status means + // ps never ran (ENOENT, EAGAIN, …): mechanics failure -> unknown. + if (e && typeof e.status === 'number') return 0; + return null; + } +} + +// true / false / null(unknown — caller fails open). +function isSelfOrAncestor(targetPid, fromPid, exec) { + let pid = fromPid; + // The owning session process is normally our direct parent; walk a few + // levels to cover wrapper processes between the session and this server. + for (let hops = 0; hops < 10 && pid && pid > 1; hops++) { + if (pid === targetPid) return true; + pid = parentOf(pid, exec); + if (pid === null) return null; + } + return false; +} + +/** + * Decide whether THIS process may post to the room as the agent. + * Returns {allowed, reason}. Never throws. + * + * Injectables (tests): env, selfPid (start of the ancestor walk), exec. + */ +export function assertRoomVoice({ + config, + env = process.env, + selfPid = process.pid, + exec = execFileSync, +} = {}) { + try { + const lockPath = lockPathFor(config, env); + if (lockPath === 'off') return { allowed: true, reason: 'responder lock disabled' }; + + const lock = readResponderLock(lockPath); + if (!lock || !lock.pid) return { allowed: true, reason: 'no responder lock' }; + + const start = procStart(lock.pid, exec); + if (!start) return { allowed: true, reason: 'lock owner process is gone (stale lock)' }; + if (lock.pstart && start !== lock.pstart) { + return { allowed: true, reason: 'lock owner pid was recycled (stale lock)' }; + } + + const ancestry = isSelfOrAncestor(lock.pid, selfPid, exec); + if (ancestry === null) { + return { allowed: true, reason: 'ancestor lookup failed mid-walk (fail open)' }; + } + if (ancestry) { + return { allowed: true, reason: 'this session holds the responder lock' }; + } + + return { + allowed: false, + reason: + `another live session (pid ${lock.pid}${lock.sid ? `, session ${lock.sid}` : ''}) ` + + `holds the room-responder lock (${lockPath}). This session is PASSIVE and must not ` + + `post to the room — duplicate agent voices confuse the team. Takeover only on the ` + + `user's explicit request: delete the lock file, then re-run the standard bootstrap.`, + }; + } catch { + // Mechanics failure: fail open, same philosophy as the bash hook. + return { allowed: true, reason: 'lock check failed (fail open)' }; + } +} diff --git a/test/responder-lock.test.mjs b/test/responder-lock.test.mjs new file mode 100644 index 0000000..72a9a80 --- /dev/null +++ b/test/responder-lock.test.mjs @@ -0,0 +1,182 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +import { describe, it, afterEach } from 'node:test'; +import { strict as assert } from 'node:assert'; +import { mkdtempSync, writeFileSync, rmSync, realpathSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { assertRoomVoice, lockPathFor, readResponderLock } from '../src/responder-lock.mjs'; + +const tempPaths = []; +function tempDir() { + const dir = realpathSync(mkdtempSync(join(tmpdir(), 'iak-voice-'))); + tempPaths.push(dir); + return dir; +} +afterEach(() => { + while (tempPaths.length > 0) rmSync(tempPaths.pop(), { recursive: true, force: true }); +}); + +/** + * Fake process table for the injectable exec: {pid: {lstart, ppid}}. + * Throws (like execFileSync) for unknown pids. + */ +function fakeExec(table) { + return (cmd, args) => { + const pid = parseInt(args[1], 10); + const col = args[3]; + const row = table[pid]; + if (!row) { + // Mimic execFileSync when ps RAN and found no such pid: nonzero exit. + const e = new Error(`no such process ${pid}`); + e.status = 1; + throw e; + } + if (row === 'SPAWN_FAILURE') { + // Mimic a spawn-level failure: ps never ran, no .status on the error. + throw new Error('spawn ps EAGAIN'); + } + if (col === 'lstart=') return `${row.lstart}\n`; + if (col === 'ppid=') return `${row.ppid}\n`; + throw new Error(`unexpected ps column ${col}`); + }; +} + +function writeLock(dir, { pid, sid = 'sess-owner', pstart = '' }) { + const path = join(dir, 'new.txt.responder.lock'); + writeFileSync(path, `pid=${pid}\nsid=${sid}\n${pstart ? `pstart=${pstart}\n` : ''}`); + return path; +} + +const START = 'Thu Jul 16 11:08:28 2026'; + +describe('lockPathFor', () => { + it('derives from the notification file and honors env overrides', () => { + assert.equal(lockPathFor({}, {}), '/tmp/iak-new-messages.txt.responder.lock'); + assert.equal(lockPathFor({ poller: { notification_file: '/x/n.txt' } }, {}), '/x/n.txt.responder.lock'); + assert.equal(lockPathFor({}, { IAK_NEW_FILE: '/y/n.txt' }), '/y/n.txt.responder.lock'); + assert.equal(lockPathFor({}, { IAK_RESPONDER_LOCK: 'off' }), 'off'); + assert.equal(lockPathFor({}, { IAK_RESPONDER_LOCK: '/z/lock' }), '/z/lock'); + }); +}); + +describe('readResponderLock', () => { + it('parses the hook lock format and tolerates junk', () => { + const dir = tempDir(); + const path = writeLock(dir, { pid: 42, sid: 's1', pstart: START }); + assert.deepEqual(readResponderLock(path), { pid: 42, sid: 's1', pstart: START }); + assert.equal(readResponderLock(join(dir, 'missing')), null); + const junk = join(dir, 'junk'); + writeFileSync(junk, 'pid=not-a-number\n%%%\n'); + assert.equal(readResponderLock(junk).pid, null); + }); +}); + +describe('assertRoomVoice', () => { + it('allows when the lock is disabled, absent, or unreadable', () => { + const dir = tempDir(); + assert.equal(assertRoomVoice({ env: { IAK_RESPONDER_LOCK: 'off' } }).allowed, true); + assert.equal(assertRoomVoice({ + env: { IAK_RESPONDER_LOCK: join(dir, 'nope') }, exec: fakeExec({}), + }).allowed, true); + }); + + it('allows when the owner process is dead (stale lock)', () => { + const dir = tempDir(); + const lock = writeLock(dir, { pid: 999, pstart: START }); + const verdict = assertRoomVoice({ + env: { IAK_RESPONDER_LOCK: lock }, selfPid: 10, exec: fakeExec({ 10: { lstart: 'x', ppid: 1 } }), + }); + assert.equal(verdict.allowed, true); + assert.match(verdict.reason, /stale/); + }); + + it('allows when the owner pid was recycled (pstart mismatch)', () => { + const dir = tempDir(); + const lock = writeLock(dir, { pid: 999, pstart: 'Thu Jan 1 00:00:00 1970' }); + const verdict = assertRoomVoice({ + env: { IAK_RESPONDER_LOCK: lock }, + selfPid: 10, + exec: fakeExec({ 999: { lstart: START, ppid: 1 }, 10: { lstart: 'x', ppid: 1 } }), + }); + assert.equal(verdict.allowed, true); + assert.match(verdict.reason, /recycled/); + }); + + it('allows the session that holds the lock (owner in the ancestor chain)', () => { + const dir = tempDir(); + const lock = writeLock(dir, { pid: 500, pstart: START }); + // self 10 -> parent 20 -> parent 500 (the owning session process) + const table = { + 10: { lstart: 'a', ppid: 20 }, + 20: { lstart: 'b', ppid: 500 }, + 500: { lstart: START, ppid: 1 }, + }; + const verdict = assertRoomVoice({ env: { IAK_RESPONDER_LOCK: lock }, selfPid: 10, exec: fakeExec(table) }); + assert.equal(verdict.allowed, true); + assert.match(verdict.reason, /holds the responder lock/); + }); + + it('REFUSES a passive session while the owner is alive elsewhere', () => { + const dir = tempDir(); + const lock = writeLock(dir, { pid: 500, sid: 'sess-owner', pstart: START }); + // self 10 -> 20 -> 1: owner 500 alive but NOT in our chain. + const table = { + 10: { lstart: 'a', ppid: 20 }, + 20: { lstart: 'b', ppid: 1 }, + 500: { lstart: START, ppid: 1 }, + }; + const verdict = assertRoomVoice({ env: { IAK_RESPONDER_LOCK: lock }, selfPid: 10, exec: fakeExec(table) }); + assert.equal(verdict.allowed, false); + assert.match(verdict.reason, /pid 500/); + assert.match(verdict.reason, /sess-owner/); + assert.match(verdict.reason, /PASSIVE/); + }); + + it('legacy lock without pstart still refuses on a live owner pid', () => { + const dir = tempDir(); + const lock = writeLock(dir, { pid: 500 }); + const table = { 10: { lstart: 'a', ppid: 1 }, 500: { lstart: START, ppid: 1 } }; + const verdict = assertRoomVoice({ env: { IAK_RESPONDER_LOCK: lock }, selfPid: 10, exec: fakeExec(table) }); + assert.equal(verdict.allowed, false); + }); + + it('fails OPEN when an intermediate ancestor lookup fails at spawn level (codex regression)', () => { + const dir = tempDir(); + const lock = writeLock(dir, { pid: 500, pstart: START }); + // Owner 500 is alive and IS our grandparent, but the walk's lookup of + // pid 20's parent fails at spawn level — must fail open, not refuse. + const table = { + 10: { lstart: 'a', ppid: 20 }, + 20: 'SPAWN_FAILURE', + 500: { lstart: START, ppid: 1 }, + }; + const verdict = assertRoomVoice({ env: { IAK_RESPONDER_LOCK: lock }, selfPid: 10, exec: fakeExec(table) }); + assert.equal(verdict.allowed, true); + assert.match(verdict.reason, /fail open/); + }); + + it('a walk ancestor that exited (ps ran, pid gone) ends the chain and refuses', () => { + const dir = tempDir(); + const lock = writeLock(dir, { pid: 500, pstart: START }); + // self 10 -> parent 77 which is GONE (ps exits 1): chain ends without + // reaching the live owner 500 -> refused, not crashed. + const table = { + 10: { lstart: 'a', ppid: 77 }, + 500: { lstart: START, ppid: 1 }, + }; + const verdict = assertRoomVoice({ env: { IAK_RESPONDER_LOCK: lock }, selfPid: 10, exec: fakeExec(table) }); + assert.equal(verdict.allowed, false); + }); + + it('fails open when ps itself explodes', () => { + const dir = tempDir(); + const lock = writeLock(dir, { pid: 500, pstart: START }); + const verdict = assertRoomVoice({ + env: { IAK_RESPONDER_LOCK: lock }, + selfPid: 10, + exec: () => { throw new Error('ps unavailable'); }, + }); + assert.equal(verdict.allowed, true); + }); +});