security: restrict unattended Layered Intelligence cmd sources to read-only binaries (#5669)

server/lib/README.md

@@ -269,7 +269,7 @@ The barrel `server/lib/index.js` is a machine-checkable enumeration of every pub
 | `setupScriptRunner.js` | `spawnSetupScript(envVars)` / `stopSetupScript(child)` / `SETUP_IMAGE_VIDEO_SCRIPT` — the one way to run `scripts/setup-image-video.sh` (shared by the Video Gen BYOV runtimes, the music engines and the MuScriptor venv). Runs it under `resolveBashBinary()` with a `toBashPath` script path, and on Windows presets the `PYTHON_BIN` the script would otherwise default to `python3` for. Cancel via `stopSetupScript`, which tree-kills so uv / pip / git die with bash. |
 | `commandExists.js` | `commandExists(cmd, args = ['--version'], { timeoutMs = 5_000, env, cwd })` — does running `cmd args` succeed? A capability probe (`execFile`-based), not a PATH lookup like `processEnv.js`'s `whichFirst`; `env`/`cwd` let a caller check the exact child process configuration. Consolidates the two previously-private copies in `localLlm.js`/`ollamaManager.js`; callers probing a heavier CLI (e.g. `codeReview.js`'s reviewer-binary probe) pass a longer `timeoutMs`. |
 | `spawnCwd.js` | `resolveSpawnCwd(workspacePath, fallbackRoot, label)` — resolves and **logs** the working directory a run/agent spawns into (expanding `~`), and throws when a workspace was requested but is missing / not a directory. Behind `services/runner.js#resolveRunCwd`, which turns that throw into a normal failed-run record for the two spawning runners. Stops a bad app `repoPath` from silently spawning in the PortOS checkout (#3180). `usesCreativeDirectorScratchCwd(task)` / `creativeDirectorScratchCwd(agentId)` / `removeCreativeDirectorScratchCwd(agentId)` / `resolveAgentCliCwd({ workspacePath, fallbackRoot, task, agentId })` — Creative Director no-worktree tasks get a per-agent scratch cwd under `os.tmpdir()/portos-cd-cwd/<id>` (outside the PortOS git tree) instead of the PortOS root, so native CLI AGENTS.md / CLAUDE.md discovery cannot walk up into the repo (#4650). `removeCreativeDirectorScratchCwd` is the matching finalize cleanup. `withSpawnCwdEnv(env, cwd)` — returns a copy of `env` with `PWD` pinned to `cwd` (dropping stale case-variant keys), because `spawn({ cwd })` doesn't rewrite the inherited `PWD` and OpenCode resolves its project root as `process.env.PWD ?? process.cwd()` (#3193). Apply it at every spawn that names its own cwd — the shared wrappers (`bufferedSpawn`, `spawnDetached`) already do, so their callers inherit it. `spawnCwd.test.js` discovers cwd-passing spawns across `server/` and fails on any that neither pins nor is listed exempt.
-| `commandSecurity.js` | Allowlist of safe shell commands + `validatePm2Command(args)` (rejects daemon-wide `pm2 kill`/`startup`/`unstartup` and `<verb> all`). `validateCommand` runs the pm2 check for `pm2` base commands. Mirrored by the `agentGuard/` PATH shim for agentic paths. |
+| `commandSecurity.js` | Two allowlists, one parser. `validateCommand(cmd)` gates the OPERATOR-driven runner against `ALLOWED_COMMANDS` (+ `validatePm2Command(args)`, which rejects daemon-wide `pm2 kill`/`startup`/`unstartup` and `<verb> all`). `validateUnattendedCommand(cmd)` gates the UNATTENDED lane (Layered Intelligence `cmd` sources) against the far narrower `UNATTENDED_READONLY_COMMANDS` — read-only inspection binaries only, no `npx`/`node`/`python`/`pip`/`curl`/`wget`/`brew`, since those execute network code with no shell metacharacter. Both share one parse + `DANGEROUS_SHELL_CHARS` body. Mirrored by the `agentGuard/` PATH shim for agentic paths. |
 | `detachedSpawn.js` | `spawnDetached(bin, args, {controlDir,env,cwd,killProcessGroup?})` → ChildProcess-like handle for a long media job that SURVIVES `pm2 restart portos-server`. A pure-`sh` double-fork reparents the job to init (escaping pm2's PPID-based TreeKill — `detached:true` alone doesn't, since it only changes the process group); the server tails on-disk log files for `stdout`/`stderr`/`close`. Group-kill mode persists a marker so cancel, reattach, and orphan reaping terminate a group-leader wrapper plus every runtime child together. Windows has no double-fork (plain-spawn fallback), so its handle's `kill` delegates to `killProcessTree` (`taskkill /T /F`) — a cancel there takes the runner's children with it. Used by loraTraining + videoGen. Also exports `reattachDetached(controlDir)` / `isReattachable(controlDir)` to RE-ATTACH a survivor after a restart, `isDetachedRunning(controlDir, expectedProcess?)` with optional executable/argument validation for fixed-command control dirs, and `reapDetached` to checkpoint-kill one when re-attach isn't possible. |
 | `hostShutdown.js` | Tells "PortOS was restarted out from under a running agent" apart from "the agent failed" (#3202). `markHostShuttingDown()` / `isHostShuttingDown()` are the in-process latch the SIGTERM/SIGINT handler sets first thing; `shouldAbandonForHostShutdown({sentinelPresent,terminatedByUser,paused})` keeps every spawn path on the same preserve-vs-finalize policy. `writeHostShutdownMarker({agentIds,signal})` / `readHostShutdownMarker()` / `clearHostShutdownMarker()` persist that verdict to `data/cos/host-shutdown.json` so the NEXT boot's orphan sweep can requeue those agents as *interrupted* — no orphan-retry charge, no 30-minute cooldown. All non-throwing: a missing marker degrades to the ordinary orphan path. |
 | `execGit.js` | `execGit(args, cwd, options)` utility imported by `git.js` + worktree manager. `cwd` is REQUIRED — it rejects on a missing/blank one rather than letting `spawn` fall back to `process.cwd()` and run git against PortOS's own checkout (#4554). |

server/lib/commandSecurity.js

@@ -15,6 +15,29 @@ export const ALLOWED_COMMANDS = new Set([
 // Pre-sorted list for API responses
 export const ALLOWED_COMMANDS_SORTED = Array.from(ALLOWED_COMMANDS).sort();
 
+// Narrower allowlist for the UNATTENDED lane (Layered Intelligence `cmd` custom
+// sources), which runs on a schedule with nobody watching. Read-only repository
+// and tracker inspection is the entire documented purpose of a `cmd` source, so
+// this list deliberately excludes every network-fetch and code-execution verb
+// that ALLOWED_COMMANDS admits for the operator-driven runner (`npx`, `node`,
+// `python`, `pip`, `curl`, `wget`, `go`, `cargo`, `make`, `brew`, `pm2`, …).
+// `npx <pkg>` / `pip install <pkg>` / `curl -o <path> <url>` contain no shell
+// metacharacter, so the metacharacter filter alone does NOT stop them.
+//
+// Scope: this gate is binary-level, not subcommand-level. Every admitted binary
+// is one whose *documented* use here is inspection, but a few are multi-purpose
+// (`git commit`, `find -delete`, `gh api -X POST`) and are NOT rejected. That is
+// a deliberately smaller step than subcommand gating: it removes remote-code
+// fetch/exec from the unattended lane, which is the class that turns hostile
+// config into arbitrary RCE. Subcommand gating is tracked separately.
+export const UNATTENDED_READONLY_COMMANDS = new Set([
+  'git', 'gh', 'glab',
+  'ls', 'cat', 'head', 'tail', 'grep', 'find', 'wc',
+  'pwd', 'echo'
+]);
+
+const UNATTENDED_READONLY_COMMANDS_SORTED = Array.from(UNATTENDED_READONLY_COMMANDS).sort();
+
 // Shell metacharacters that could be used for command injection
 // Security: Reject any command containing these to prevent injection via pipes, chaining, etc.
 export const DANGEROUS_SHELL_CHARS = /[;|&`$(){}[\]<>\\!#*?~]/;
@@ -78,10 +101,12 @@ export function validatePm2Command(args) {
 }
 
 /**
- * Validate a command against the allowlist.
+ * Shared shape/metacharacter/allowlist gate. Both public validators route
+ * through this so the two lanes can never disagree about parsing or about
+ * which shell metacharacters are rejected — only the allowlist differs.
  * Returns { valid, error?, baseCommand?, args? }
  */
-export function validateCommand(command) {
+function validateAgainst(command, allowlist, allowlistSorted) {
   if (!command || typeof command !== 'string') {
     return { valid: false, error: 'Command is required' };
   }
@@ -92,15 +117,38 @@ export function validateCommand(command) {
   }
   const parts = parseCommandArgs(trimmed);
   const baseCommand = parts[0];
-  if (!ALLOWED_COMMANDS.has(baseCommand)) {
-    return { valid: false, error: `Command '${baseCommand}' is not in the allowlist. Allowed: ${ALLOWED_COMMANDS_SORTED.join(', ')}` };
+  if (!allowlist.has(baseCommand)) {
+    return { valid: false, error: `Command '${baseCommand}' is not in the allowlist. Allowed: ${allowlistSorted.join(', ')}` };
   }
-  const args = parts.slice(1);
-  if (baseCommand === 'pm2') {
-    const pm2Check = validatePm2Command(args);
+  return { valid: true, baseCommand, args: parts.slice(1) };
+}
+
+/**
+ * Validate a command against the operator-driven allowlist (the manual command
+ * runner, POST /api/commands/execute — a human triggers and watches each run).
+ * Returns { valid, error?, baseCommand?, args? }
+ */
+export function validateCommand(command) {
+  const check = validateAgainst(command, ALLOWED_COMMANDS, ALLOWED_COMMANDS_SORTED);
+  if (!check.valid) return check;
+  if (check.baseCommand === 'pm2') {
+    const pm2Check = validatePm2Command(check.args);
     if (!pm2Check.valid) return pm2Check;
   }
-  return { valid: true, baseCommand, args };
+  return check;
+}
+
+/**
+ * Validate a command for the UNATTENDED lane — a scheduled job executing a
+ * string that lives in persistent, attacker-reachable config with no human in
+ * the loop. Same parsing and metacharacter rules as `validateCommand`, but only
+ * `UNATTENDED_READONLY_COMMANDS` are admitted, so a config that lands `npx`,
+ * `curl` or `pip install` cannot reach a spawn. No pm2 sub-check is needed —
+ * pm2 is not on the list at all.
+ * Returns { valid, error?, baseCommand?, args? }
+ */
+export function validateUnattendedCommand(command) {
+  return validateAgainst(command, UNATTENDED_READONLY_COMMANDS, UNATTENDED_READONLY_COMMANDS_SORTED);
 }
 
 // Patterns matching sensitive env var values in command output

server/lib/commandSecurity.test.js

@@ -4,6 +4,7 @@ import {
   ALLOWED_COMMANDS_SORTED,
   DANGEROUS_SHELL_CHARS,
   validateCommand,
+  validateUnattendedCommand,
   validatePm2Command,
   redactOutput,
   parseCommandArgs
@@ -287,4 +288,72 @@ describe('commandSecurity', () => {
       expect(validatePm2Command(['delete', 'all']).valid).toBe(false)
     })
   })
+
+  describe('validateUnattendedCommand (#5669)', () => {
+    // The unattended lane (Layered Intelligence `cmd` sources) runs persistent,
+    // attacker-reachable config on a schedule with nobody watching. Binaries that
+    // fetch and execute network code carry NO shell metacharacter, so the
+    // metacharacter filter alone does not stop them — the narrower allowlist must.
+    it.each([
+      ['npx runs an arbitrary package straight off the network', 'npx some-package'],
+      ['curl writes an arbitrary file', 'curl https://example.com -o /tmp/x'],
+      ['pip install runs a setup script', 'pip install evil'],
+      ['pip3 install runs a setup script', 'pip3 install evil'],
+      ['node executes a script', 'node evil.js'],
+      ['python executes a script', 'python evil.py'],
+      ['wget downloads to disk', 'wget https://example.com/x'],
+      ['brew installs software', 'brew install evil'],
+      ['make runs a Makefile target', 'make install'],
+      ['npm runs lifecycle scripts', 'npm install'],
+      ['pm2 is not on the unattended list at all', 'pm2 list'],
+    ])('rejects %s', (_label, cmd) => {
+      const result = validateUnattendedCommand(cmd)
+      expect(result.valid).toBe(false)
+      expect(result.error).toMatch(/not in the allowlist/)
+    })
+
+    it.each([
+      'git log --oneline -20',
+      'gh pr list',
+      'glab mr list',
+      'cat README.md',
+      'head -n 20 README.md',
+      'grep -rn TODO src',
+      'wc -l README.md',
+      'pwd',
+    ])('accepts read-only inspection command %s', (cmd) => {
+      expect(validateUnattendedCommand(cmd).valid).toBe(true)
+    })
+
+    it('parses args the same way as validateCommand', () => {
+      const result = validateUnattendedCommand('git log --grep "two words"')
+      expect(result.valid).toBe(true)
+      expect(result.baseCommand).toBe('git')
+      expect(result.args).toEqual(['log', '--grep', 'two words'])
+    })
+
+    it.each([
+      'git log | sh',
+      'git log; rm -rf ~',
+      'echo $(curl evil.example)',
+    ])('rejects shell metacharacters in %s', (cmd) => {
+      const result = validateUnattendedCommand(cmd)
+      expect(result.valid).toBe(false)
+      expect(result.error).toMatch(/disallowed shell characters/)
+    })
+
+    it.each([
+      ['', 'Command is required'],
+      ['   ', 'Command cannot be empty'],
+    ])('rejects blank input %j', (cmd, error) => {
+      expect(validateUnattendedCommand(cmd).error).toBe(error)
+    })
+
+    it('leaves the operator-facing runner untouched', () => {
+      // POST /api/commands/execute must behave identically — a human triggers it.
+      expect(validateCommand('npx vitest').valid).toBe(true)
+      expect(validateCommand('curl https://example.com').valid).toBe(true)
+      expect(validateCommand('pip install requests').valid).toBe(true)
+    })
+  })
 })

server/services/layeredIntelligence.test.js

@@ -2659,6 +2659,30 @@ describe('runShellCommand (restricted by default — #2515)', () => {
     // Full string handed to the shell verbatim (opt-in only).
     expect(exec).toHaveBeenCalledWith('git log --oneline | head', [], expect.objectContaining({ cwd: '/x', shell: true }));
   });
+
+  // #5669 — the unattended lane uses the narrow read-only allowlist, NOT the
+  // operator-facing one. These binaries pass `validateCommand` and carry no shell
+  // metacharacter, yet each fetches and/or runs arbitrary code on a schedule.
+  it.each([
+    'npx some-package',
+    'node evil.js',
+    'curl https://example.com -o /tmp/x',
+    'pip install evil',
+    'brew install evil',
+  ])('drops the operator-allowlisted-but-code-executing cmd source %j without spawning', async (cmd) => {
+    const exec = vi.fn();
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    expect(await runShellCommand(cmd, { cwd: '/x', exec })).toBeNull();
+    expect(exec).not.toHaveBeenCalled();
+    expect(warn).toHaveBeenCalledWith(expect.stringContaining('read-only inspection commands'));
+    warn.mockRestore();
+  });
+
+  it('still runs an npx cmd source when trustShellSources is true', async () => {
+    const exec = vi.fn().mockResolvedValue({ code: 0, stdout: 'ran\n', stderr: '' });
+    expect(await runShellCommand('npx some-package', { cwd: '/x', exec, trustShellSources: true })).toBe('ran');
+    expect(exec).toHaveBeenCalledWith('npx some-package', [], expect.objectContaining({ cwd: '/x', shell: true }));
+  });
 });
 
 describe('getTrustShellSources (install-level opt-in — #2515)', () => {

server/services/layeredIntelligence/sources.js

@@ -12,7 +12,7 @@ import { realpath } from 'fs/promises';
 import { tryReadFile, readJSONFile, PATHS } from '../../lib/fileUtils.js';
 import { bufferedSpawn } from '../../lib/bufferedSpawn.js';
 import { fetchPublicText } from '../../lib/safeUrlFetch.js';
-import { validateCommand } from '../../lib/commandSecurity.js';
+import { validateUnattendedCommand } from '../../lib/commandSecurity.js';
 import { getSettings } from '../settings.js';
 import { computeWindowedStats } from '../taskLearning/store.js';
 import { computeChurn, summarizeRecentRuns, SHORT_LIVED_MS } from '../agentChurn.js';
@@ -505,19 +505,29 @@ export async function fetchHttpSource(url, { timeoutMs = 10_000, fetchText = fet
  * by length + a 15s timeout. That is arbitrary RCE: `; rm -rf ~`, `$(curl … | sh)`,
  * pipes to `sh`, etc. all execute. Issue #2515.
  *
- * Defense: by default we DENY the shell. The command is parsed and checked
- * against the shared binary allowlist (`validateCommand` in commandSecurity.js —
- * same gate the manual command runner uses), which rejects shell metacharacters
- * (`;|&$(){}` …) and any binary not on the allowlist, then we spawn the base
+ * Defense: by default we DENY the shell. The command is parsed and checked by
+ * `validateUnattendedCommand` (commandSecurity.js), which rejects shell
+ * metacharacters (`;|&$(){}` …) and admits only the read-only inspection
+ * binaries in `UNATTENDED_READONLY_COMMANDS` (`git`, `gh`, `glab`, `ls`, `cat`,
+ * `head`, `tail`, `grep`, `find`, `wc`, `pwd`, `echo`); then we spawn the base
  * binary with parsed args and `shell: false` — so no shell ever interprets the
- * string. A non-allowlisted / metacharacter command is dropped (key omitted) with
- * a warning, exactly like any other failed source read.
+ * string. A rejected command is dropped (key omitted) with a warning, exactly
+ * like any other failed source read.
+ *
+ * This lane deliberately does NOT use the operator-facing `validateCommand`
+ * allowlist. That one admits `npx`, `node`, `python`, `pip`, `curl`, `wget`,
+ * `go`, `cargo`, `make` and `brew` — every one of which fetches and/or executes
+ * arbitrary code without a single shell metacharacter (`npx <pkg>`,
+ * `pip install <pkg>`, `curl -o <path> <url>`), so the metacharacter filter
+ * would be doing all the work here. Fine for a run a human triggered and is
+ * watching; not fine for persistent config on an autonomous schedule. Issue #5669.
  *
  * Escape hatch: an operator who genuinely needs a pipeline (`git log … | head`)
- * can set the install-level `settings.layeredIntelligence.trustShellSources`
- * flag, which restores the full `shell: true` behavior for THIS install only.
- * It is an explicit, install-wide opt-in — off by default — so a fresh install
- * (or a synced-in app config) can never execute an un-allowlisted command.
+ * or a binary outside the read-only list can set the install-level
+ * `settings.layeredIntelligence.trustShellSources` flag, which restores the full
+ * `shell: true` behavior for THIS install only. It is an explicit, install-wide
+ * opt-in — off by default — so a fresh install (or a synced-in app config) can
+ * never execute anything but a read-only inspection command.
  *
  * `exec` is injectable for tests; `trustShellSources` is resolved by the caller
  * (`gatherSources`) from install settings and threaded in.
@@ -533,9 +543,9 @@ export async function runShellCommand(cmd, { cwd, timeoutMs = 15_000, exec = buf
     if (code !== 0) return null;
     return (stdout || '').trim() || null;
   }
-  const check = validateCommand(cmd);
+  const check = validateUnattendedCommand(cmd);
   if (!check.valid) {
-    console.warn(`⚠️ Layered Intelligence: custom cmd source "${cmd}" rejected — ${check.error} (enable settings.layeredIntelligence.trustShellSources to allow arbitrary shell commands)`);
+    console.warn(`⚠️ Layered Intelligence: custom cmd source "${cmd}" rejected — ${check.error} (unattended sources are limited to read-only inspection commands; enable settings.layeredIntelligence.trustShellSources to allow arbitrary shell commands)`);
     return null;
   }
   const { code, stdout } = await exec(check.baseCommand, check.args, { cwd, timeoutMs, shell: false });