From 4d76bacdd58790372f1487b2bf4c6d6ecae80ae7 Mon Sep 17 00:00:00 2001 From: raiseCatError <315733358+raiseCatError@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:41:32 +0530 Subject: [PATCH 1/5] /track: a stricter gate for repeated commands; every redirect judged on its own target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #416/#420 found commands that /track (and /watch) started at once and repeated every interval without asking: - `echo x > ~/.zshrc 2>/dev/null`: the shared classifier excused every redirect in a command once `>/dev/null` appeared anywhere; `>|` was split as a pipe. Paste review labelled the same text read-only. - `env rm -rf …`: `env` is read-only on its own but runs its operand. - Anything classified network: `npm publish`, `npm login`, `npm audit fix`, `npm x`/`pnpm dlx`/`bun x`, `ssh host rm …`, `scp`, `rsync`, `curl -X POST`, `curl -o file`, plain `wget`. - `git -c core.fsmonitor=… status`, `GIT_EXTERNAL_DIFF=… git diff`, `--ext-diff` (run arbitrary programs) and `git log --output=…` (writes files). pasteGuard now judges each redirection's target (descriptor duplication and the null/standard-stream devices are not files) and treats `>|` as a redirection. repeatSafety.ts adds the repetition gate on top of the classifier: network commands start at once only as read-only probes; env prefixes, wrappers and git program options ask first; file-writing options, --fix/--write/snapshot updates, --watch modes, scp/rsync and power control are refused. `2>&1` is no longer mistaken for a background job. Network targets are remote unless they are this computer. Refs #393, #416, #420 --- src/input/pasteGuard.ts | 28 +++++- src/tasks/WatchTasks.ts | 23 +++-- src/tasks/repeatSafety.ts | 189 ++++++++++++++++++++++++++++++++++++++ tests/trackSafety.test.ts | 66 +++++++++++++ 4 files changed, 296 insertions(+), 10 deletions(-) create mode 100644 src/tasks/repeatSafety.ts create mode 100644 tests/trackSafety.test.ts diff --git a/src/input/pasteGuard.ts b/src/input/pasteGuard.ts index 7242b3c2..fe23cd4a 100644 --- a/src/input/pasteGuard.ts +++ b/src/input/pasteGuard.ts @@ -53,6 +53,8 @@ export function splitCommands(text: string): Array<{text: string; pipedFrom: boo if (char === '\n' || char === ';') { push(false); continue; } if (char === '&' && next === '&') { push(false); index += 1; continue; } if (char === '|' && next === '|') { push(false); index += 1; continue; } + // `>|` is a redirection (overwrite despite noclobber), not a pipe. + if (char === '|' && current.endsWith('>')) { current += char; continue; } if (char === '|') { push(true); continue; } current += char; } @@ -62,7 +64,31 @@ export function splitCommands(text: string): Array<{text: string; pipedFrom: boo const INTERPRETERS = /^(?:sh|bash|zsh|fish|dash|ksh|python3?|node|perl|ruby|eval|source|\.)$/u; +/** Shell text with quoted strings and escaped characters masked, so operators inside them are not read as operators. */ +export function maskQuoted(text: string): string { + return text.replace(/\\[\s\S]|"(?:\\.|[^"\\])*"|'[^']*'/gu, match => match.startsWith('\\') ? '__' : '""'); +} + +/** + * Whether shell text redirects output into a file. Every redirection is judged on its own target, so `>/dev/null` + * elsewhere in the line never excuses `> ~/.zshrc`. Descriptor duplication (`2>&1`), closing (`>&-`), the null device + * and the standard streams are not files; `>|`, `>&file`, `&>file` and `<>` (opens for writing) are. + */ +export function writesFiles(text: string): boolean { + for (const match of maskQuoted(text).matchAll(/(&>>?|\d*>>|\d*>\||\d*<>|\d*>&|\d*>)\s*([^\s;&|<>()]*)/gu)) { + const operator = match[1] ?? ''; + const target = match[2] ?? ''; + if (operator.endsWith('<>')) return true; + if (operator.endsWith('>&') && /^(?:\d+|-)$/u.test(target)) continue; + if (/^\/dev\/(?:null|stdout|stderr|fd\/[12])$/u.test(target)) continue; + return true; + } + return false; +} + /** Words of one command (quotes removed for classification only). */ +export function commandWords(text: string): string[] { return words(text); } + function words(text: string): string[] { return text.match(/"(?:\\.|[^"])*"|'[^']*'|\S+/gu)?.map(word => word.replace(/^["']|["']$/gu, '')) ?? []; } @@ -196,7 +222,7 @@ function shellKinds(text: string, pipedFrom: boolean): PasteKind[] { if (/^(?:mv|cp|mkdir|touch|ln|tee|sed|perl|install|chmod|chown|patch|unzip|tar)$/u.test(command) && !kinds.has('destructive')) { if (command !== 'sed' || /-i\b|--in-place/u.test(flags)) { kinds.delete('read'); kinds.add('modifies'); } } - if (/(?:^|[^>&])>>?\s*[^\s&|]/u.test(text.replace(/"(?:\\.|[^"])*"|'[^']*'/gu, '""')) && !/>\s*\/dev\/null/u.test(text)) kinds.add('modifies'); + if (writesFiles(text)) kinds.add('modifies'); if (!kinds.size) kinds.add('unknown'); return [...kinds]; } diff --git a/src/tasks/WatchTasks.ts b/src/tasks/WatchTasks.ts index f0568a70..0f5617f7 100644 --- a/src/tasks/WatchTasks.ts +++ b/src/tasks/WatchTasks.ts @@ -1,6 +1,7 @@ import {spawn, type ChildProcess} from 'node:child_process'; import {StringDecoder} from 'node:string_decoder'; -import {classifyCommand, splitCommands, type PasteKind} from '../input/pasteGuard.js'; +import {classifyCommand, maskQuoted, splitCommands, writesFiles, type PasteKind} from '../input/pasteGuard.js'; +import {reachesRemote, repeatRisk} from './repeatSafety.js'; import {stripTerminalControls} from '../util/terminalControls.js'; /** @@ -36,32 +37,36 @@ export function watchSafety(command: string): WatchSafety { if (/\$\(|`|[<>]\(/u.test(command)) { return {kind: 'refused', reason: 'Shell and process substitutions cannot be tracked repeatedly.'}; } - const outsideQuotes = command.replace(/"(?:\\.|[^"\\])*"|'[^']*'/gu, '""'); - // Detached shell jobs would outlive the observed sample and multiply on every - // tick; heredocs also need a different stdin/input-ownership contract. - if (/(?:^|[^&])&(?!&)|<&1`) and `&>` are not background operators. + if (/(?:^|[^&<>])&(?![&>])|< REFUSED.has(kind)); - if (bad) return {kind: 'refused', reason: `/watch won't repeatedly run a command that ${bad === 'navigation' ? 'changes directory' : bad === 'install' ? 'installs packages' : bad === 'privilege' ? 'runs as root' : bad === 'pipeline' ? 'runs downloaded code' : bad === 'destructive' ? 'deletes or destroys things' : 'modifies files'} (${part.text}).`}; + if (bad) return {kind: 'refused', reason: `NMSh won't repeatedly run a command that ${bad === 'navigation' ? 'changes directory' : bad === 'install' ? 'installs packages' : bad === 'privilege' ? 'runs as root' : bad === 'pipeline' ? 'runs downloaded code' : bad === 'destructive' ? 'deletes or destroys things' : 'modifies files'} (${part.text}).`}; + // Repetition has its own, stricter gate (repeatSafety.ts): a refusal there ends it; a doubt asks first. + const risk = repeatRisk(part.text, kinds); + if (risk?.kind === 'refused') return {kind: 'refused', reason: `NMSh won't repeatedly run a command that ${risk.reason}.`}; + if (risk) { unknown = true; reason ??= risk.reason; } if (kinds.includes('network')) { network = true; - const target = /https?:\/\/([^/:\s]+)/u.exec(part.text)?.[1] ?? /\s([\w.-]+\.[a-z]{2,})(?:\s|$)/iu.exec(part.text)?.[1]; - if (target && !/^(?:localhost|127\.0\.0\.1|0\.0\.0\.0|\[?::1\]?)$/u.test(target)) remote = true; + if (reachesRemote(part.text, kinds)) remote = true; } // Allow only Docker's explicit read-only disk report. Other Docker verbs (including // 'system prune') still require confirmation, or are refused by the classifier. const dockerDiskReport = /^docker\s+system\s+df(?:\s+(?:-v|--verbose))?\s*$/u.test(part.text); if ((kinds.includes('unknown') || kinds.includes('project') || kinds.includes('text')) && !dockerDiskReport && !TEST_RUNNERS.test(part.text) && !/^git\s+(?:status|log|diff|branch|remote|show)\b/u.test(part.text)) unknown = true; } - return unknown ? {kind: 'confirm', reason: 'NMSh can\'t tell whether this command changes anything; it will run every interval.', network, remote} : {kind: 'allowed', network, remote}; + return unknown ? {kind: 'confirm', reason: `${reason ?? 'NMSh can\'t tell whether this command changes anything.'} It will run every interval.`, network, remote} : {kind: 'allowed', network, remote}; } /** Parse a finite interval strictly. Invalid cadence never falls through as shell text. */ diff --git a/src/tasks/repeatSafety.ts b/src/tasks/repeatSafety.ts new file mode 100644 index 00000000..865818c6 --- /dev/null +++ b/src/tasks/repeatSafety.ts @@ -0,0 +1,189 @@ +import {commandWords, type PasteKind} from '../input/pasteGuard.js'; + +/** + * /track and /watch run a command again and again with nobody reading each run. The shared classifier says what a + * command is; this is the stricter gate for repeating it: whether it may start at once, needs an explicit Yes, or is + * never repeated. When it cannot tell, it asks rather than allows. Network commands start at once only when they are + * read-only probes (ping, dig, `nc -z`, a curl GET or a wget to standard output); everything that sends data, saves + * files, signs in, publishes or runs another command asks first. + */ +export type RepeatRisk = {kind: 'refused' | 'confirm'; reason: string}; + +const LOOPBACK = /^(?:localhost|localhost\.localdomain|ip6-localhost|127(?:\.\d{1,3}){3}|::1|0\.0\.0\.0)$/u; +const PROBES = new Set(['ping', 'ping6', 'dig', 'nslookup', 'host', 'traceroute', 'traceroute6', 'mtr']); +const NETCAT = new Set(['nc', 'ncat', 'netcat']); +const REMOTE_SESSIONS = new Set(['ssh', 'sftp', 'telnet', 'ftp', 'mosh']); +const HTTP_CLIENTS = new Set(['http', 'https', 'xh', 'httpie']); +const PACKAGE_MANAGERS = new Set(['npm', 'pnpm', 'yarn', 'bun']); +const PACKAGE_QUERIES = new Set(['view', 'info', 'show', 'search', 'outdated', 'fund', 'audit']); +const POWER = /^(?:shutdown|reboot|halt|poweroff)$/u; +const POWER_VERBS = /^(?:reboot|poweroff|halt|suspend|hibernate|hybrid-sleep|kexec|soft-reboot)$/u; +/** curl short options that take a value (the rest of the cluster, or the next word). */ +const CURL_VALUE = new Set([...'oDcKXdFTHwAmueEbrxYyzCPUQt']); +/** wget short options that take a value. */ +const WGET_VALUE = new Set([...'OoaPtTweiBUQlARDIX']); +const ASSIGNMENT = /^[A-Za-z_]\w*=/u; + +const refused = (reason: string): RepeatRisk => ({kind: 'refused', reason}); +const confirm = (reason: string): RepeatRisk => ({kind: 'confirm', reason}); +const standardStream = (value: string | undefined) => value === '-' || value === '/dev/null' || value === '/dev/stdout'; +const cluster = (arg: string) => /^-[A-Za-z0-9]+$/u.test(arg) ? arg.slice(1) : ''; + +/** The command's name and arguments, after any leading `NAME=value` assignments. */ +function parts(text: string): {assignments: string[]; name: string; args: string[]} { + const all = commandWords(text); + let index = 0; + while (ASSIGNMENT.test(all[index] ?? '')) index += 1; + const [command = '', ...args] = all.slice(index); + return {assignments: all.slice(0, index), name: command.split('/').pop() ?? command, args}; +} + +/** What repeating one command (one part of a command line) would risk beyond what the classifier already refuses. */ +export function repeatRisk(text: string, kinds: readonly PasteKind[]): RepeatRisk | undefined { + const {assignments, name, args} = parts(text); + if (POWER.test(name) || (/^(?:systemctl|loginctl|launchctl)$/u.test(name) && args.some(arg => POWER_VERBS.test(arg))) || (name === 'init' && /^[06]$/u.test(args[0] ?? ''))) { + return refused(`shuts down or restarts the computer (${text})`); + } + const writes = writesThroughOptions(name, args, text); + if (writes) return refused(writes); + if (args.some(arg => /^--watch(?:All)?(?:=true)?$/u.test(arg))) return refused(`runs until it is stopped (${text}); /track repeats a command that finishes`); + if (assignments.length) return confirm(`It sets ${assignments.map(word => `${word.split('=')[0]}=…`).join(' ')} for the command, which can change what it runs.`); + if (name === 'env') { + const inner = envCommand(args); + if (inner) return confirm(`It runs another command (env … ${inner}).`); + } + if (name === 'git') return gitRisk(args, kinds); + if (PROBES.has(name)) return undefined; + if (NETCAT.has(name)) return args.some(arg => cluster(arg).includes('z')) ? undefined : confirm('It opens a network connection every interval.'); + if (name === 'curl') return curlRisk(args); + if (name === 'wget') return wgetRisk(args); + if (REMOTE_SESSIONS.has(name)) return confirm('It connects to another computer every interval.'); + if (HTTP_CLIENTS.has(name)) return confirm('It sends HTTP requests that may change data.'); + if (PACKAGE_MANAGERS.has(name) && kinds.includes('network')) { + const sub = args.find(arg => !arg.startsWith('-')) ?? ''; + if (PACKAGE_QUERIES.has(sub) && !(sub === 'audit' && args.some(arg => arg === 'fix' || arg === '--fix'))) return undefined; + return confirm(`It publishes, signs in, changes packages or downloads and runs one (${name} ${sub}).`); + } + if (kinds.includes('network') && !/^(?:npx|pnpx|bunx)$/u.test(name)) return confirm('It contacts the network every interval and NMSh can\'t tell what it sends.'); + return undefined; +} + +/** Options that make an otherwise read-only command write files, delete them or update what it checks. */ +function writesThroughOptions(name: string, args: readonly string[], text: string): string | undefined { + const has = (pattern: RegExp) => args.some(arg => pattern.test(arg)); + if (name === 'git' && has(/^--output(?:=|$)/u)) return `writes files (${text})`; + if (name === 'sort' && (has(/^--output(?:=|$)/u) || args.some(arg => cluster(arg).includes('o')))) return `writes files (${text})`; + if (name === 'find' && has(/^-(?:fprint0?|fprintf|fls|delete|exec|execdir|ok|okdir)$/u)) return `writes, deletes or runs things (${text})`; + if (name === 'tree' && has(/^-o$/u)) return `writes files (${text})`; + if (name === 'yq' && has(/^(?:-i|--inplace)$/u)) return `modifies files (${text})`; + if (name === 'scp') return `copies files (${text})`; + if (name === 'rsync' && !(has(/^--(?:dry-run|list-only)$/u) || args.some(arg => cluster(arg).includes('n')))) return `copies files (${text})`; + if (has(/^(?:--fix|--write|--update|--update-?snapshots?|--updateSnapshot|--snapshot-update)(?:=.*)?$/u)) return `rewrites files (${text})`; + if (args.includes('-u') && /\b(?:jest|vitest)\b|^(?:npm|pnpm|yarn|bun)\s+(?:run\s+)?test\b/u.test(text)) return `updates snapshots (${text})`; + return undefined; +} + +/** The command `env` would run, if any (options and assignments alone only print the environment). */ +function envCommand(args: readonly string[]): string | undefined { + for (let at = 0; at < args.length; at += 1) { + const arg = args[at]!; + if (arg === '--') return args[at + 1]; + if (/^-[uC]$/u.test(arg)) { at += 1; continue; } + if (/^-\w*S|^--split-string/u.test(arg)) return arg; + if (arg.startsWith('-') || ASSIGNMENT.test(arg)) continue; + return arg; + } + return undefined; +} + +function gitRisk(args: readonly string[], kinds: readonly PasteKind[]): RepeatRisk | undefined { + // Global options come before the subcommand; `-c` there sets configuration (which can name programs to run). + for (let at = 0; at < args.length; at += 1) { + const arg = args[at]!; + if (!arg.startsWith('-')) break; + if (arg === '-c' || /^--(?:config-env|exec-path)(?:=|$)/u.test(arg)) return confirm('It sets git options that can run other programs.'); + if (arg === '-C') at += 1; + } + if (args.includes('--ext-diff')) return confirm('It runs an external diff program.'); + if (kinds.includes('network') && args.find(arg => !arg.startsWith('-')) === 'fetch') return confirm('It fetches from a remote and updates its branches every interval.'); + return undefined; +} + +function curlRisk(args: readonly string[]): RepeatRisk | undefined { + for (let at = 0; at < args.length; at += 1) { + const arg = args[at]!; + const scheme = /^([a-z][a-z0-9+.-]*):\/\//iu.exec(arg)?.[1]?.toLowerCase(); + if (scheme && scheme !== 'http' && scheme !== 'https') return confirm(`It uses ${scheme}://, which can send commands, not only read.`); + if (arg.startsWith('--')) { + const equals = arg.indexOf('='); + const option = equals === -1 ? arg : arg.slice(0, equals); + const value = equals === -1 ? args[at + 1] : arg.slice(equals + 1); + if (/^--(?:data|form|upload-file|json|remote-name|remote-header-name|output-dir|create-dirs|cookie-jar|config|trace|libcurl|etag-save|hsts|alt-svc|mail-|quote|post30)/u.test(option)) { + return confirm(`It sends data or saves files (${option}).`); + } + if (option === '--request' && !/^(?:GET|HEAD)$/iu.test(value ?? '')) return confirm(`It sends ${value ?? 'other'} requests.`); + if ((option === '--output' || option === '--dump-header') && !standardStream(value)) return confirm(`It saves files (${option}).`); + continue; + } + if (!/^-[^-]/u.test(arg)) continue; + for (let index = 1; index < arg.length; index += 1) { + const letter = arg[index]!; + if ('dFTJOKQ'.includes(letter)) return confirm(`It sends data or saves files (-${letter}).`); + if (!CURL_VALUE.has(letter)) continue; + const value = index + 1 < arg.length ? arg.slice(index + 1) : args[at + 1]; + if (letter === 'X' && !/^(?:GET|HEAD)$/iu.test(value ?? '')) return confirm(`It sends ${value ?? 'other'} requests.`); + if ('oDc'.includes(letter) && !standardStream(value)) return confirm(`It saves files (-${letter}).`); + if (index + 1 >= arg.length) at += 1; + break; + } + } + return undefined; +} + +function wgetRisk(args: readonly string[]): RepeatRisk | undefined { + let toOutput = false; + for (let at = 0; at < args.length; at += 1) { + const arg = args[at]!; + if (arg.startsWith('--')) { + const equals = arg.indexOf('='); + const option = equals === -1 ? arg : arg.slice(0, equals); + const value = equals === -1 ? args[at + 1] : arg.slice(equals + 1); + if (option === '--spider') toOutput = true; + else if (option === '--output-document') toOutput = standardStream(value); + else if (/^--(?:post-|body-|method|recursive|mirror|page-requisites|convert-links|timestamping|continue|force-directories|save-cookies|warc-|backups)/u.test(option)) return confirm(`It sends data or saves files (${option}).`); + else if (/^--(?:output-file|append-output)$/u.test(option) && !standardStream(value)) return confirm(`It writes a log file (${option}).`); + continue; + } + if (!/^-[^-]/u.test(arg)) continue; + for (let index = 1; index < arg.length; index += 1) { + const letter = arg[index]!; + if ('rmpkNxc'.includes(letter)) return confirm(`It saves files (-${letter}).`); + if (!WGET_VALUE.has(letter)) continue; + const value = index + 1 < arg.length ? arg.slice(index + 1) : args[at + 1]; + if (letter === 'O') toOutput = standardStream(value); + if ('oa'.includes(letter) && !standardStream(value)) return confirm(`It writes a log file (-${letter}).`); + if (index + 1 >= arg.length) at += 1; + break; + } + } + return toOutput ? undefined : confirm('It saves what it downloads to a file every interval.'); +} + +/** + * Whether a network command reaches beyond this computer: only loopback targets are local. Registries and git + * remotes are remote; a command whose target NMSh cannot find is treated as remote. + */ +export function reachesRemote(text: string, kinds: readonly PasteKind[]): boolean { + const {name, args} = parts(text); + if (/^(?:npx|pnpx|bunx)$/u.test(name)) return false; + if (PACKAGE_MANAGERS.has(name) || name === 'git' || name === 'gh') return kinds.includes('network'); + const hosts: string[] = []; + for (const arg of args) { + if (arg.startsWith('-')) continue; + const url = /^[a-z][a-z0-9+.-]*:\/\/(?:[^@/]*@)?(\[[^\]]*\]|[^:/?#]*)/iu.exec(arg); + if (url) { hosts.push(url[1] ?? ''); continue; } + const host = /^@?(?:[^@\s]+@)?(\[[0-9a-fA-F:]+\]|[A-Za-z0-9-]+(?:\.[A-Za-z0-9-]+)*)(?::\S*)?$/u.exec(arg)?.[1]; + if (host && !/^\d+(?:\.\d+)?$/u.test(host)) hosts.push(host); + } + return !hosts.length || hosts.some(host => !LOOPBACK.test(host.replace(/^\[|\]$/gu, '').toLowerCase())); +} diff --git a/tests/trackSafety.test.ts b/tests/trackSafety.test.ts new file mode 100644 index 00000000..4eb97d26 --- /dev/null +++ b/tests/trackSafety.test.ts @@ -0,0 +1,66 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import {classifyCommand, splitCommands, writesFiles} from '../src/input/pasteGuard.js'; +import {watchSafety} from '../src/tasks/WatchTasks.js'; + +const kind = (command: string) => watchSafety(command).kind; +const remote = (command: string) => { const safety = watchSafety(command); return safety.kind === 'refused' ? undefined : safety.remote; }; + +test('a redirect into a file is a write even when /dev/null appears elsewhere (paste review and /track)', () => { + for (const command of ['echo pwned > ~/.zshrc 2>/dev/null', 'ls 2>/dev/null >out', 'echo x >/dev/null >out', 'ls >| out', 'ls >&out', 'ls <>out', 'ls &>out', 'ls 1>>log']) { + assert.ok(writesFiles(command), command); + assert.ok(splitCommands(command).some(part => classifyCommand(part.text, part.pipedFrom).includes('modifies')), `paste review: ${command}`); + assert.equal(kind(command), 'refused', command); + } + for (const command of ['ls >/dev/null', 'ls 2>/dev/null', 'npm test 2>&1', 'ls >&2', 'ls 2>&1 | tail -5', 'ls &>/dev/null', 'echo "a > b"', "echo 'x >y'", 'echo a\\>b', 'ls >&-']) { + assert.ok(!writesFiles(command), command); + assert.notEqual(kind(command), 'refused', command); + } + assert.deepEqual(splitCommands('ls >| out').map(part => part.text), ['ls >| out'], '>| is a redirection, not a pipe'); +}); + +test('/track asks before a command that runs another command or changes its environment', () => { + for (const command of ['env rm -rf /tmp/x', 'env -i rm -rf x', 'env -u HOME rm x', 'env -- rm x', 'env FOO=1 df', 'env -S "rm x"', 'FOO=1 df', 'LD_PRELOAD=/tmp/x.so df', + 'GIT_EXTERNAL_DIFF=/tmp/x git diff', 'git -c core.fsmonitor=/tmp/x status', 'git --config-env=core.pager=X log', 'git --exec-path=/tmp status', 'git diff --ext-diff']) { + assert.equal(kind(command), 'confirm', command); + } + for (const command of ['env', 'env | grep PATH', 'env -0', 'printenv HOME', 'git status', 'git -C sub status', 'git log --oneline -5', 'git log -c -1', 'git diff --stat', 'df -h']) { + assert.equal(kind(command), 'allowed', command); + } +}); + +test('/track refuses options that write files, update what they check, never finish, or power off', () => { + for (const command of ['git log --output=x', 'git diff --output x.patch', 'git show --output=f HEAD', 'sort -o out in', 'sort -no out in', 'sort --output=out in', + 'find . -fprint out', 'find . -fls out', 'find . -okdir rm {} ;', 'tree -o out', 'yq -i .a=1 f.yaml', 'npx jest -u', 'npx vitest run -u', 'npm test -- -u', + 'npm run lint -- --fix', 'npx prettier --write .', 'pytest --snapshot-update', 'npx jest --watch', 'scp host:a b', 'rsync -a a b', + 'shutdown now', 'reboot', '/sbin/halt', 'poweroff', 'systemctl reboot', 'launchctl reboot', 'init 0']) { + assert.equal(kind(command), 'refused', command); + } + for (const command of ['sort file', 'sort -n -k2 file', 'find . -name x', 'tree -L 2', 'npx jest', 'npm test', 'npm run lint', 'npx eslint --fix-dry-run .', 'rsync -an a b']) { + assert.notEqual(kind(command), 'refused', command); + } +}); + +test('/track starts only read-only network probes at once; sending, saving, signing in or running packages asks first', () => { + for (const command of ['ping -c1 localhost', 'curl -s http://localhost:3000/health', 'curl -fsSL http://127.0.0.1:8080/', 'curl -s -o /dev/null -w "%{http_code}" http://localhost/', + 'curl -X HEAD http://localhost/', 'wget -qO- http://localhost/x', 'wget -O - http://localhost/x', 'wget --spider http://localhost/', 'nc -z localhost 5432', 'nc -zv localhost 80', + 'dig example.com', 'npm view react version', 'npm outdated', 'npm audit', 'git ls-remote origin']) { + assert.equal(kind(command), 'allowed', command); + } + for (const command of ['curl -X POST http://localhost:3000/x', 'curl -d a=1 http://localhost/x', 'curl --data-binary @f http://localhost/x', 'curl --json {} http://localhost/', + 'curl -o out http://localhost/x', 'curl -sSLo out http://localhost/x', 'curl -O http://localhost/x', 'curl -T f http://localhost/', 'curl -K cfg http://localhost/', 'curl -c jar http://localhost/', + 'curl --request=DELETE http://localhost/x', 'curl gopher://localhost:6379/_FLUSHALL', 'wget http://localhost/x', 'wget -O page http://localhost/x', 'wget -r http://localhost/', + 'http POST localhost:3000', 'ssh host uptime', 'ssh host rm -rf x', 'sftp host', 'telnet host', 'nc localhost 80', 'git fetch', + 'npm publish', 'pnpm publish', 'npm login', 'npm logout', 'npm audit fix', 'npm x cowsay', 'pnpm dlx cowsay', 'bun x cowsay']) { + assert.equal(kind(command), 'confirm', command); + } +}); + +test('/track treats a network target as remote unless it is this computer', () => { + for (const command of ['ping -c 3 -i 0.2 localhost', 'curl -s http://localhost:3000/health', 'curl localhost:3000/health', 'nc -z 127.0.0.1 5432', 'curl -s http://[::1]:8080/']) { + assert.equal(remote(command), false, command); + } + for (const command of ['ping -c1 8.8.8.8', 'ping nas', 'dig @1.1.1.1 example.com', 'curl -s https://example.com/health', 'ssh host uptime', 'npm outdated', 'git fetch', 'curl -s']) { + assert.equal(remote(command), true, command); + } +}); From b4eb409758ae8af74c013e00f83c16d6a6b0a170 Mon Sep 17 00:00:00 2001 From: raiseCatError <315733358+raiseCatError@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:41:53 +0530 Subject: [PATCH 2/5] /track: a sample's leftover processes never outlive it; refresh event after wiring - When a sample's shell exits, the close handler cancelled the KILL fallback even if other members of its process group were still running. A child that ignored TERM and closed its output survived `/track stop` (verified with a pid probe; the new test fails without the fix), and a stray process left by a finished sample piled up on every refresh. On close, anything still in the group now gets TERM, then KILL after the grace period; that timer keeps a closing window alive for at most the grace period so nothing is left behind. - #420's refresh-start event now fires after the sample's handlers are attached, so a listener that throws cannot leave a child without them. - A tracker whose directory disappeared said `spawn /bin/sh ENOENT`; it now names the directory and keeps trying at its interval. Refs #393, #416, #420 --- src/tasks/WatchTasks.ts | 33 +++++++++++++++++++++++++++++---- tests/track.test.ts | 34 ++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/tasks/WatchTasks.ts b/src/tasks/WatchTasks.ts index 0f5617f7..9503efd9 100644 --- a/src/tasks/WatchTasks.ts +++ b/src/tasks/WatchTasks.ts @@ -1,4 +1,5 @@ import {spawn, type ChildProcess} from 'node:child_process'; +import {statSync} from 'node:fs'; import {StringDecoder} from 'node:string_decoder'; import {classifyCommand, maskQuoted, splitCommands, writesFiles, type PasteKind} from '../input/pasteGuard.js'; import {reachesRemote, repeatRisk} from './repeatSafety.js'; @@ -27,6 +28,7 @@ export const MAX_ACTIVE_WATCHES = 8; const MAX_COMMAND = 4096; const MAX_OUTPUT = 200_000; const STOP_GRACE_MS = 1500; +const isDirectory = (path: string) => { try { return statSync(path).isDirectory(); } catch { return false; } }; export function watchSafety(command: string): WatchSafety { if (!command.trim() || command.length > MAX_COMMAND || /[\u0000-\u001f\u007f]/u.test(command)) { @@ -147,6 +149,8 @@ export class WatchTasks { private readonly timers = new Map(); private readonly children = new Map(); private readonly killTimers = new Map(); + /** KILL escalations for leftovers of finished samples (see endStragglers); never cleared early. */ + private readonly stragglers = new Set(); private readonly listeners = new Set<(watch: WatchTask, transition?: 'pass' | 'fail') => void>(); /** @@ -210,9 +214,6 @@ export class WatchTasks { return true; } this.children.set(id, child); - // Notify the presenter when a refresh begins, not only when it completes: - // a slow sample must be shown as refreshing while its previous result stays visible. - this.emit(watch); const take = (decoder: StringDecoder, chunk: Buffer) => { const kept = Math.min(chunk.length, Math.max(0, MAX_OUTPUT - receivedBytes)); if (kept) { receivedBytes += kept; output += decoder.write(chunk.subarray(0, kept)); } @@ -230,13 +231,17 @@ export class WatchTasks { child.stderr?.destroy(); }, Math.min(60_000, watch.intervalMs * 3)); limit.unref(); - child.on('error', error => { spawnError = String(error); }); + child.on('error', error => { + // Node names the shell when the working directory is what is missing; say which one it is. + spawnError = (error as NodeJS.ErrnoException).code === 'ENOENT' && !isDirectory(watch.cwd) ? `the directory ${watch.cwd} is no longer available` : String(error); + }); // Only 'close' guarantees the captured stdio has finished, unlike 'exit'. child.on('close', (code, signal) => { clearTimeout(limit); const killTimer = this.killTimers.get(id); if (killTimer) clearTimeout(killTimer); this.killTimers.delete(id); + this.endStragglers(child); if (this.children.get(id) !== child) return; this.children.delete(id); output += stdoutDecoder.end() + stderrDecoder.end(); @@ -246,9 +251,29 @@ export class WatchTasks { ...(spawnError ? {error: stripTerminalControls(spawnError, 300)} : {}), ...(signal ? {signal} : {})}); }); + // The presenter shows `refreshing…` over the previous result from now on; told only once the sample is wired, so + // a listener that throws cannot leave a child without its handlers. + this.emit(watch); return true; } + /** + * A finished sample's process group must be empty: anything left (a child that ignored TERM, or one that closed its + * output and kept running) outlived its sample and would pile up on every refresh. End it with TERM, then KILL after + * the grace period. The KILL timer keeps the window alive that long when it is closing, so nothing is left behind. + */ + private endStragglers(child: ChildProcess): void { + const group = child.pid; + if (!group) return; + try { process.kill(-group, 0); } catch { return; } + try { process.kill(-group, 'SIGTERM'); } catch { return; } + const timer = setTimeout(() => { + this.stragglers.delete(timer); + try { process.kill(-group, 'SIGKILL'); } catch { /* already gone */ } + }, STOP_GRACE_MS); + this.stragglers.add(timer); + } + private signalGroup(child: ChildProcess, signal: NodeJS.Signals): void { if (!child.pid) return; try { process.kill(-child.pid, signal); } diff --git a/tests/track.test.ts b/tests/track.test.ts index 6402cadf..20f77c7e 100644 --- a/tests/track.test.ts +++ b/tests/track.test.ts @@ -1,6 +1,6 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import {mkdtempSync, rmSync} from 'node:fs'; +import {mkdtempSync, readFileSync, rmSync} from 'node:fs'; import {tmpdir} from 'node:os'; import {join} from 'node:path'; import {parseSlashCommand} from '../src/commands/slashCommands.js'; @@ -188,7 +188,7 @@ test('/track fails visibly when the command or working directory is unavailable' {kind: 'allowed', network: false, remote: false}); await waitFor(() => missingCwd.runs === 1); assert.notEqual(missingCwd.current?.exitCode, 0, 'invalid cwd cannot be a successful refresh'); - assert.match(missingCwd.current?.error ?? '', /ENOENT|spawn/u); + assert.equal(missingCwd.current?.error, `the directory ${join(cwd, 'not-here')} is no longer available`, 'names the directory, not the shell'); assert.equal(engine.active().length, 2); } finally { engine.dispose(); @@ -196,6 +196,36 @@ test('/track fails visibly when the command or working directory is unavailable' } }); +test('/track ends what a sample leaves in its process group: after stop, and after a sample that finished', async () => { + const cwd = mkdtempSync(join(tmpdir(), 'nmsh-track-orphans-')); + const engine = new WatchTasks('/bin/sh'); + const alive = (pid: number) => { try { process.kill(pid, 0); return true; } catch { return false; } }; + // A child that ignores TERM and closes its output: the shell that started it can end while it keeps running. + const stubborn = (name: string) => `sh -c 'trap "" TERM; echo $$ > ${name}; exec >/dev/null 2>&1; exec sleep 30'`; + const pidOf = async (name: string) => { + let pid = 0; + await waitFor(() => { try { pid = Number(readFileSync(join(cwd, name), 'utf8').trim()); } catch { /* not yet */ } return pid > 0; }); + return pid; + }; + try { + const stopped = engine.start(`${stubborn('stopped.pid')}; echo done`, cwd, 5000, {kind: 'allowed', network: false, remote: false}); + const stoppedPid = await pidOf('stopped.pid'); + assert.ok(alive(stoppedPid)); + assert.equal(engine.stop(stopped.id), true); + await waitFor(() => !alive(stoppedPid), 6000); + + const finished = engine.start(`${stubborn('finished.pid')} & echo started`, cwd, 5000, {kind: 'allowed', network: false, remote: false}); + await waitFor(() => finished.runs === 1, 6000); + assert.match(finished.current?.output ?? '', /started/u); + assert.equal(finished.current?.exitCode, 0, 'the sample itself finished normally'); + const finishedPid = await pidOf('finished.pid'); + await waitFor(() => !alive(finishedPid), 6000); + } finally { + engine.dispose(); + rmSync(cwd, {recursive: true, force: true}); + } +}); + test('/track confirmation refuses clipped small-terminal review', () => { const confirm = {command: 'unknown status-command', cwd: '/tmp', intervalMs: 5000, reason: 'Unknown command', choice: 'no' as const}; const rows = renderWatchPanel({selected: 0, output: false, confirm}, [], 30, Date.now(), 6); From 4cdf9712ba31f2b1d8c4e32e3343891e4ee6970c Mon Sep 17 00:00:00 2001 From: raiseCatError <315733358+raiseCatError@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:42:38 +0530 Subject: [PATCH 3/5] /track: one summary row for many trackers, Safe-glyph marks, cached result rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Above the composer each active tracker had its own row, repeating its block's status; eight trackers took eight rows. More than two now share one summary row (`4 trackers · 1 failing · /track opens the manager`). - The state marks (◉ ■ ‖ ◌) ignored Safe glyphs; they now use ASCII there, in the block and the manager. - Live rows split and sanitized the whole 200 kB result on every frame (1–3 ms per frame with eight large trackers); the first rows and line count are now computed once per result (~0.5 ms). - The confirmation for an unknown /track command says "Track …?", not "Watch …?". - Live PTY tests: four trackers at once (slow, failing, missing command, large output) with `refreshing…`, a resize and `/track stop all` ending every process; Bash and Fish trackers finding a PATH change made after startup. Refs #393, #416, #420 --- src/app/TerminalApp.ts | 4 +- src/tasks/WatchPanel.ts | 14 ++++++- src/tasks/trackBlock.ts | 37 ++++++++++++++----- tests/track.test.ts | 22 ++++++++++- tests/trackLive.test.ts | 81 ++++++++++++++++++++++++++++++++++++++++- 5 files changed, 142 insertions(+), 16 deletions(-) diff --git a/src/app/TerminalApp.ts b/src/app/TerminalApp.ts index 2564a507..9dd9eec2 100644 --- a/src/app/TerminalApp.ts +++ b/src/app/TerminalApp.ts @@ -269,7 +269,7 @@ import {ManagedTasks} from '../tasks/ManagedTasks.js'; import {CursorPresenter} from '../cursor/CursorPresenter.js'; import {runDoctor} from '../doctor/doctor.js'; import {parseWatch, watchSafety, WatchTasks, type WatchSafety} from '../tasks/WatchTasks.js'; -import {renderWatchPanel, watchPanelKey, watchRow, type WatchPanelState} from '../tasks/WatchPanel.js'; +import {renderWatchPanel, watchNoticeRows, watchPanelKey, type WatchPanelState} from '../tasks/WatchPanel.js'; import {finalResultRows, trackBlockRows} from '../tasks/trackBlock.js'; import {explainModules, moduleTimings} from '../prompt/moduleReport.js'; import {analyzePaste, KIND_LABELS, needsPreview, PASTE_EXACT_NOTE, pasteHeader, primaryKind, type PasteAnalysis, nmshCommandNote} from '../input/pasteGuard.js'; @@ -8100,7 +8100,7 @@ export class TerminalApp { const now = Date.now(); const suggestion = this.clipboardChangeSuggested && !this.running && this.clipboardChangeWatch?.enabled ? [truncateAnsi(`${SUBTLE}Clipboard changed · /ps reads it for review${RESET}`, columns)] : []; - const rows = [...this.clipboardRows(columns), ...suggestion, ...this.sessionNoticeRows(columns), ...this.taskRows(columns), ...this.watches.active().map(watch => truncateAnsi(watchRow(watch, now), columns)), ...this.pastePreviewRows(columns)]; + const rows = [...this.clipboardRows(columns), ...suggestion, ...this.sessionNoticeRows(columns), ...this.taskRows(columns), ...watchNoticeRows(this.watches.active(), now).map(row => truncateAnsi(row, columns)), ...this.pastePreviewRows(columns)]; const background = this.backgroundRows(columns); return shelf ? [...rows, ...background, shelf] : [...rows, ...background]; } diff --git a/src/tasks/WatchPanel.ts b/src/tasks/WatchPanel.ts index fae7ff3d..fa7b0d15 100644 --- a/src/tasks/WatchPanel.ts +++ b/src/tasks/WatchPanel.ts @@ -3,6 +3,7 @@ import {renderControls} from '../ui/controls.js'; import {GLYPHS} from '../ui/glyphs.js'; import {foreground, UI_COLORS} from '../ui/palette.js'; import {truncateAnsi} from '../util/text.js'; +import {trackMark} from './trackBlock.js'; import type {WatchTask} from './WatchTasks.js'; /** /track and /watch share one live snapshot view: no duplicated transcript output. */ @@ -50,12 +51,21 @@ export function watchRow(watch: WatchTask, now: number): string { const secondary = foreground(UI_COLORS.secondary); const reset = '\u001b[0m'; const state = watch.current ? (watch.current.exitCode === 0 && !watch.current.timedOut && !watch.current.error ? foreground(UI_COLORS.success) : foreground(UI_COLORS.failure)) : subtle; - const mark = watch.status === 'paused' ? '‖' : '◉'; + const mark = trackMark(watch.status === 'paused' ? 'paused' : 'live'); const last = watch.current ? ` · updated ${ago(Math.max(0, now - watch.current.at - watch.current.durationMs))} ago · ${runStatus(watch.current)}` : ' · first run pending'; return `${state}${mark}${reset} ${secondary}Track · ${watch.command.slice(0, 40)}${reset}${subtle} · every ${formatEvery(watch.intervalMs)}${watch.summary ? ` · ${watch.summary}` : ''}` + `${watch.changes.length ? ' · changed' : ''} · runs ${watch.runs}${watch.status === 'paused' ? ' · paused' : last}${reset}`; } +/** The rows above the composer for active watches: one each for up to two, else one summary (each /track block and the manager show the rest). */ +export function watchNoticeRows(watches: readonly WatchTask[], now: number): string[] { + if (watches.length <= 2) return watches.map(watch => watchRow(watch, now)); + const failing = watches.filter(watch => watch.current && (watch.current.exitCode !== 0 || watch.current.timedOut || watch.current.error)).length; + const paused = watches.filter(watch => watch.status === 'paused').length; + const parts = [`${watches.length} trackers`, ...(failing ? [`${failing} failing`] : []), ...(paused ? [`${paused} paused`] : []), '/track opens the manager']; + return [`${foreground(failing ? UI_COLORS.failure : UI_COLORS.success)}${trackMark('live')}\u001b[0m ${foreground(UI_COLORS.secondary)}${parts.join(' · ')}\u001b[0m`]; +} + export function renderWatchPanel(state: WatchPanelState, watches: readonly WatchTask[], columns: number, now: number, rowsAvailable: number): string[] { const primary = foreground(UI_COLORS.primary); const secondary = foreground(UI_COLORS.secondary); @@ -69,7 +79,7 @@ export function renderWatchPanel(state: WatchPanelState, watches: readonly Watch `${secondary} Confirmation is disabled at this size. Esc cancels.${reset}`].map(row => truncateAnsi(row, columns)); } const yes = state.confirm.choice === 'yes'; - rows.push('', ` ${primary}Watch ${state.confirm.command}?${reset}`, ` ${secondary}${state.confirm.reason}${reset}`, + rows.push('', ` ${primary}${state.confirm.track ? 'Track' : 'Watch'} ${state.confirm.command}?${reset}`, ` ${secondary}${state.confirm.reason}${reset}`, ` ${subtle}It runs in ${state.confirm.cwd}, every ${formatEvery(state.confirm.intervalMs ?? 5000)}, until you stop it.${reset}`, '', ` ${!yes ? `${accent}${GLYPHS.selection} No${reset}` : `${subtle} No${reset}`} ${yes ? `${accent}${GLYPHS.selection} Yes${reset}` : `${subtle} Yes${reset}`}`, '', renderControls([['←→', 'choose'], ['Enter', 'confirm'], ['Esc', 'cancel']])); diff --git a/src/tasks/trackBlock.ts b/src/tasks/trackBlock.ts index 07df7446..74bdf048 100644 --- a/src/tasks/trackBlock.ts +++ b/src/tasks/trackBlock.ts @@ -1,5 +1,6 @@ import {displaySafe} from '../input/PasteReview.js'; import {colorLevel} from '../presentation/capabilities.js'; +import {getCurrentGlyphMode} from '../ui/glyphs.js'; import {foreground, UI_COLORS} from '../ui/palette.js'; import {truncateAnsi, truncateText} from '../util/text.js'; import type {WatchRun, WatchTask} from './WatchTasks.js'; @@ -14,6 +15,10 @@ import type {WatchRun, WatchTask} from './WatchTasks.js'; export const MAX_TRACK_ROWS = 12; const RESET = '\u001b[0m'; +/** A tracker's state mark; Safe glyphs use ASCII. */ +const MARKS = {live: ['◉', '*'], stopped: ['■', '#'], paused: ['‖', '='], pending: ['◌', 'o']} as const; +export const trackMark = (state: keyof typeof MARKS): string => MARKS[state][getCurrentGlyphMode() === 'nerd' ? 0 : 1]; + const clock = (at: number) => new Date(at).toLocaleTimeString('en-GB', {hour12: false}); const every = (ms: number) => ms % 60_000 === 0 ? `${ms / 60_000}m` : `${Math.round(ms / 100) / 10}s`; const span = (ms: number) => ms < 60_000 ? `${Math.round(ms / 1000)}s` : `${Math.floor(ms / 60_000)}m ${Math.round((ms % 60_000) / 1000)}s`; @@ -35,10 +40,10 @@ export function trackStatus(watch: WatchTask, now: number, sampling: boolean): s const parts: string[] = []; let mark: string; let role: keyof typeof UI_COLORS = 'subtle'; - if (watch.status === 'stopped') { mark = '■'; parts.push('stopped'); } - else if (watch.status === 'paused') { mark = '‖'; parts.push('paused'); } - else if (!run) { mark = '◌'; parts.push(sampling ? 'first run…' : 'first run pending'); } - else { mark = '◉'; role = outcome!.ok ? 'success' : 'failure'; parts.push('live'); } + if (watch.status === 'stopped') { mark = trackMark('stopped'); parts.push('stopped'); } + else if (watch.status === 'paused') { mark = trackMark('paused'); parts.push('paused'); } + else if (!run) { mark = trackMark('pending'); parts.push(sampling ? 'first run…' : 'first run pending'); } + else { mark = trackMark('live'); role = outcome!.ok ? 'success' : 'failure'; parts.push('live'); } parts.push(`every ${every(watch.intervalMs)}`); if (run && outcome) parts.push(`updated ${clock(finished!)}`, outcome.text); if (run?.truncated) parts.push('output cut at 200 kB'); @@ -48,22 +53,34 @@ export function trackStatus(watch: WatchTask, now: number, sampling: boolean): s return `${paint(role)}${mark}${RESET} ${paint('secondary')}${parts.join(' · ')}${RESET}`; } +/** A result's first rows (inert, cut far past any terminal width) and its line count: once per result, not per frame. */ +const resultCache = new WeakMap(); +function resultLines(run: WatchRun): {head: string[]; total: number} { + let cached = resultCache.get(run); + if (!cached) { + const lines = run.output.replace(/\n$/u, '').split('\n'); + cached = {head: lines.slice(0, MAX_TRACK_ROWS).map(line => displaySafe(line.slice(0, 4096))), total: lines.length}; + resultCache.set(run, cached); + } + return cached; +} + export function trackBlockRows(watch: WatchTask | undefined, width: number, now: number, reserved: number, sampling = false): {rows: string[]; reserved: number} { const color = colorLevel() !== 'none'; const subtle = color ? foreground(UI_COLORS.subtle) : ''; const fit = (text: string) => truncateText(text, Math.max(1, width - 4)); - if (!watch) return {rows: [` ${subtle}■ not running: trackers stop with the window that started them${RESET}`], reserved: 0}; + if (!watch) return {rows: [` ${subtle}${trackMark('stopped')} not running: trackers stop with the window that started them${RESET}`], reserved: 0}; const status = truncateAnsi(` ${trackStatus(watch, now, sampling)}`, width); // Stopped by the person: the final result was written into the transcript; the live rows end here. if (watch.status === 'stopped') return {rows: [status], reserved: 0}; - const lines = watch.current ? watch.current.output.replace(/\n$/u, '').split('\n') : []; - const height = Math.max(reserved, Math.min(MAX_TRACK_ROWS, Math.max(1, lines.length))); + const {head, total} = watch.current ? resultLines(watch.current) : {head: [], total: 0}; + const height = Math.max(reserved, Math.min(MAX_TRACK_ROWS, Math.max(1, total))); const body: string[] = []; if (!watch.current) body.push(` ${subtle}waiting for the first result…${RESET}`); else { - const take = lines.length > height ? height - 1 : lines.length; - for (const line of lines.slice(0, take)) body.push(` ${fit(displaySafe(line))}`); - if (take < lines.length) body.push(` ${subtle}${fit(`… ${lines.length - take} more lines · /track copy copies them all`)}${RESET}`); + const take = total > height ? height - 1 : total; + for (const line of head.slice(0, take)) body.push(` ${fit(line)}`); + if (take < total) body.push(` ${subtle}${fit(`… ${total - take} more lines · /track copy copies them all`)}${RESET}`); } while (body.length < height) body.push(''); return {rows: [status, ...body], reserved: height}; diff --git a/tests/track.test.ts b/tests/track.test.ts index 20f77c7e..83abf4fe 100644 --- a/tests/track.test.ts +++ b/tests/track.test.ts @@ -5,7 +5,8 @@ import {tmpdir} from 'node:os'; import {join} from 'node:path'; import {parseSlashCommand} from '../src/commands/slashCommands.js'; import {parseWatch, watchSafety, WatchTasks, MIN_LOCAL_MS} from '../src/tasks/WatchTasks.js'; -import {renderWatchPanel, watchPanelKey, watchRow} from '../src/tasks/WatchPanel.js'; +import {renderWatchPanel, watchNoticeRows, watchPanelKey, watchRow} from '../src/tasks/WatchPanel.js'; +import {setIconStyle} from '../src/ui/glyphs.js'; import {stripAnsi} from '../src/util/text.js'; function waitFor(predicate: () => boolean, timeoutMs = 5000): Promise { @@ -257,6 +258,14 @@ test('track status says only what is known: pending, live, failed, timed out, pa assert.match(plain(task({current: run('ok'), runs: 1}), 2_000, true), /refreshing…/u); assert.match(plain(task({current: run('ok', {truncated: true}), runs: 1})), /output cut at 200 kB/u); assert.match(plain(task({current: run('ok'), status: 'stopped', runs: 4})), /^■ stopped/u); + setIconStyle('safe'); + try { + assert.match(plain(task()), /^o first run pending/u, 'Safe glyphs: ASCII marks'); + assert.match(plain(task({current: run('ok'), runs: 1})), /^\* live/u); + assert.match(plain(task({current: run('ok'), status: 'paused', runs: 1})), /^= paused/u); + assert.match(plain(task({current: run('ok'), status: 'stopped', runs: 4})), /^# stopped/u); + assert.match(stripAnsi(watchRow(task({current: run('ok'), status: 'paused', runs: 1}), 2_000)), /^= Track/u); + } finally { setIconStyle('nerd'); } }); test('the live result keeps a stable height: shorter results pad, longer ones say how much is not shown; text is inert', () => { @@ -295,3 +304,14 @@ test('the block: stored lines once, live rows replaced in place under them, neve buffer.clearPresentation(); assert.equal(buffer.wrapped(80).some(row => row.derived), false); }); + +test('above the composer: one row per tracker for up to two, then one summary row', () => { + const one = task({current: run('ok'), runs: 1}); + const failing = task({id: 'w2', current: run('no', {exitCode: 2}), runs: 1}); + const paused = task({id: 'w3', current: run('ok'), status: 'paused', runs: 1}); + assert.equal(watchNoticeRows([one, failing], 2_000).length, 2); + const summary = watchNoticeRows([one, failing, paused, task({id: 'w4'})], 2_000); + assert.equal(summary.length, 1); + assert.equal(stripAnsi(summary[0]!), '◉ 4 trackers · 1 failing · 1 paused · /track opens the manager'); + assert.deepEqual(watchNoticeRows([], 2_000), []); +}); diff --git a/tests/trackLive.test.ts b/tests/trackLive.test.ts index dcf5c600..5fc630ba 100644 --- a/tests/trackLive.test.ts +++ b/tests/trackLive.test.ts @@ -2,7 +2,8 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import {chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync} from 'node:fs'; import {join} from 'node:path'; -import {LiveSandbox, until} from './helpers/liveFrontend.js'; +import {LiveSandbox, processAlive, until} from './helpers/liveFrontend.js'; +import {shellAdapter} from '../src/shell/adapters/registry.js'; /** /track through the real composer and PTY: the live result updates on screen, copy and stop work, history is not spammed. */ const supported = process.platform === 'darwin' || process.platform === 'linux'; @@ -45,3 +46,81 @@ test('live: /track refreshes one result in place, /track copy copies it, /track assert.equal(text.includes('alpha-1'), false, 'earlier refreshes are never stored'); } finally { await sandbox.dispose(); } }); + +test('live: several trackers at once (slow, failing, missing, large), resized, then /track stop all ends them and their processes', {skip: supported ? false : 'unsupported platform', timeout: 180_000}, async () => { + const sandbox = new LiveSandbox({provider: 'none', shellBackend: 'zsh'}); + const home = sandbox.home; + const script = (name: string, body: string) => { writeFileSync(join(home, name), `#!/bin/sh\n${body}\n`); chmodSync(join(home, name), 0o755); }; + script('slow.sh', 'echo $$ >> slow.pids\nsleep 3\necho SLOW-RESULT'); + script('fail.sh', 'echo FAIL-RESULT\nexit 3'); + writeFileSync(join(home, 'big.txt'), `${Array.from({length: 5_000}, (_, index) => `big-line-${index}`).join('\n')}\n`); + const frontend = sandbox.launch([], {cols: 100, rows: 48}); + // Commands NMSh can't vouch for ask first: Yes starts them. + const confirmed = async (command: string) => { + const mark = frontend.mark; + frontend.pty.write(`/track --every 2s -- ${command}\r`); + await frontend.waitFor(new RegExp(`Track ${command.replace(/[.*+?^${}()|[\]\\/]/gu, '\\$&')}\\?`, 'u'), mark); + frontend.pty.write('y'); + frontend.pty.write('\r'); + await frontend.waitFor(/Tracking /u, mark); + }; + try { + await frontend.waitFor(/Vespyr|notMyShell|zsh/u); + await frontend.run('echo READY', /READY/u); + let mark = frontend.mark; + await confirmed('./slow.sh'); + await frontend.waitFor(/SLOW-RESULT/u, mark); + // A refresh that takes a while shows as refreshing over the previous result (#420), then completes in place. + await frontend.waitFor(/live · every 2s[^\n]*refreshing…/u, mark); + mark = frontend.mark; + await confirmed('./fail.sh'); + await frontend.waitFor(/live · every 2s · updated \d\d:\d\d:\d\d · exit 3/u, mark); + mark = frontend.mark; + await confirmed('nmsh-no-such-command-393'); + await frontend.waitFor(/exit 127/u, mark); + mark = frontend.mark; + frontend.pty.write('/track --every 2s -- cat big.txt\r'); + await frontend.waitFor(/big-line-0[\s\S]*… 4,?989 more lines · \/track copy copies them all/u, mark); + // Four trackers: one summary row above the composer, not one row each. + await frontend.waitFor(/4 trackers · 2 failing · \/track opens the manager/u, mark); + // Narrower and back: the trackers keep drawing, the shell stays usable. + frontend.pty.resize(56, 48); + await frontend.run('echo NARROW-OK', /NARROW-OK/u); + frontend.pty.resize(100, 48); + await frontend.run('echo SHELL-STILL-HERE', /SHELL-STILL-HERE/u); + mark = frontend.mark; + frontend.pty.write('/track stop all\r'); + await frontend.waitFor(/Stopped every tracker/u, mark); + await frontend.waitFor(/(?:Stopped · last result[\s\S]*){4}/u, mark); + const pids = readFileSync(join(home, 'slow.pids'), 'utf8').trim().split('\n').map(Number); + await until(() => pids.every(pid => !processAlive(pid)), 10_000, 'every tracked process to end'); + await frontend.run('echo AFTER-ALL', /AFTER-ALL/u); + } finally { await sandbox.dispose(); } +}); + +for (const shell of ['bash', 'fish'] as const) { + const installed = Boolean(shellAdapter(shell).resolveExecutable(process.env)); + test(`live: /track from ${shell} finds what the live shell's PATH finds, including a change made after startup`, {skip: !supported ? 'unsupported platform' : !installed && `${shell} is not installed`, timeout: 120_000}, async () => { + const sandbox = new LiveSandbox({provider: 'none', shellBackend: shell}); + const tools = join(sandbox.home, 'tools'); + mkdirSync(tools); + writeFileSync(join(tools, 'nmsh-track-tool'), '#!/bin/sh\necho TOOL-FROM-SHELL-PATH\n'); + chmodSync(join(tools, 'nmsh-track-tool'), 0o755); + const frontend = sandbox.launch([], {cols: 100, rows: 40}); + try { + await frontend.waitFor(/Vespyr|notMyShell|bash|fish/u); + await frontend.run('echo READY', /READY/u); + await frontend.run(shell === 'fish' ? `set -gx PATH ${tools} $PATH; echo PATH-SET` : `export PATH="${tools}:$PATH"; echo PATH-SET`, /PATH-SET/u); + let mark = frontend.mark; + frontend.pty.write('/track --every 2s -- nmsh-track-tool\r'); + await frontend.waitFor(/Track nmsh-track-tool\?/u, mark); + frontend.pty.write('y'); + frontend.pty.write('\r'); + await frontend.waitFor(/live · every 2s[\s\S]*TOOL-FROM-SHELL-PATH/u, mark); + mark = frontend.mark; + frontend.pty.write('/track stop\r'); + await frontend.waitFor(/Stopped · last result[\s\S]*TOOL-FROM-SHELL-PATH/u, mark); + await frontend.run('echo AFTER-TRACK', /AFTER-TRACK/u); + } finally { await sandbox.dispose(); } + }); +} From 24af7d487e981bd128175e5a1cad769874a3d723 Mon Sep 17 00:00:00 2001 From: raiseCatError <315733358+raiseCatError@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:42:50 +0530 Subject: [PATCH 4/5] /track docs: the repetition gate, process cleanup, the summary row and new physical checks Refs #393, #416, #420 --- CHANGELOG.md | 3 ++ .../development/track-live-command-refresh.md | 30 +++++++++++++++---- docs/testing/v021-physical-qa.md | 12 +++++++- 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 862aad11..6162170d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - Multi-line progress displays that redraw their own rows (cursor up and erase line, as concurrent downloads and container pulls do) now update in place instead of leaving a copy of every frame in the transcript ([#388](https://github.com/raiseCatError/notMyShell/issues/388)). A program can redraw only its own recent rows, at most a screen height up, never NMSh's lines or earlier commands; `/copy` copies the final rows once. Absolute cursor positioning keeps the previous behavior. - A queued command whose command word was still being classified when the next prompt reported new shell knowledge no longer keeps the unclassified color ([#408](https://github.com/raiseCatError/notMyShell/issues/408)). +- Paste review and `/track`: a redirect into a file is recognized wherever `>/dev/null` also appears (`echo x > ~/.zshrc 2>/dev/null` was labelled read-only), and `>|` is a redirection, not a pipe. +- `/track` and `/watch` no longer start, without asking, commands that change things through the network or another command: `env `, `NAME=value` prefixes, git's `-c`/`--ext-diff`, `npm publish`/`login`/`x`, `pnpm dlx`, `ssh`, `curl -X POST`/`-d`/`-o file`, plain `wget`; options that write files (`git … --output`, `sort -o`, `jest -u`, `--fix`) are refused. Read-only probes (`ping`, `dig`, `nc -z`, a curl GET) still start at once, and `npm test 2>&1` is no longer refused as a background job ([#393](https://github.com/raiseCatError/notMyShell/issues/393)). +- `/track stop` and a closing window end every process a tracked command left in its process group, including one that ignored TERM; a tracker whose directory disappeared names it instead of reporting `spawn /bin/sh ENOENT`. ## [0.20.0] - 2026-10-18 diff --git a/docs/development/track-live-command-refresh.md b/docs/development/track-live-command-refresh.md index 01366024..7e5c89e8 100644 --- a/docs/development/track-live-command-refresh.md +++ b/docs/development/track-live-command-refresh.md @@ -31,6 +31,10 @@ keeps a command's own flags from reading as NMSh's. - Live rows are presentation: find, selection and `/copy` never include them, and nothing accumulates per refresh. - Stopping a tracker (`/track stop`, or S in the manager) writes its **last result once** into the transcript, so history keeps it. A tracker whose window closes leaves only its stored lines. +- Above the composer, each active tracker has a compact row; with more than two, one summary row (`4 trackers · 1 + failing · /track opens the manager`) replaces them. `clear` removes the block from the transcript; the tracker keeps + running and stays in the manager. +- Safe glyphs use ASCII state marks (`*` live, `=` paused, `o` pending, `#` stopped). ## Execution and safety @@ -40,13 +44,29 @@ keeps a command's own flags from reading as NMSh's. - Default interval 5 s; at least 2 s locally and 15 s for remote network targets; at most 60 minutes. Invalid intervals fail before anything runs. At most 8 active trackers. A sample never overlaps another of the same tracker; a refresh while one runs is refused with the reason. -- Repeated commands can repeat side effects. Read-only commands (and the exact `docker system df`) start at once; - unknown ones need an explicit Yes; installing, modifying, destroying, privileged, downloaded-code, `cd`, substitution, - background (`&`) and heredoc commands are refused. +- Repeated commands can repeat side effects, so repetition has its own gate (`src/tasks/repeatSafety.ts`) on top of + the shared command classifier, and it asks when unsure: + - **Start at once:** read-only commands, test and check runners, the exact `docker system df`, and read-only network + probes: `ping`, `dig`, `nslookup`, `host`, `traceroute`, `nc -z`, a curl GET or HEAD to standard output (or + `-o /dev/null`), `wget -O -` or `--spider`, and package queries (`npm view`, `outdated`, `audit`). + - **Ask first (an explicit Yes):** unknown commands; `NAME=value` prefixes and `env ` (they change or run + another command); git's `-c`, `--config-env`, `--exec-path` and `--ext-diff`; `git fetch`; anything that sends + data, saves files, signs in, publishes or runs a downloaded package (`curl -X POST`, `-d`, `-o file`, non-HTTP + URLs, plain `wget`, `ssh`, `sftp`, HTTPie, `npm publish`, `npm login`, `npm x`, `pnpm dlx`, `npm audit fix`). + Network targets other than this computer also ask, and their minimum interval is 15 s. + - **Refused:** installing, modifying, destroying, privileged, downloaded-code and `cd` commands; substitutions, + background `&` and heredocs; any output redirection into a file (judged per redirection, so `2>/dev/null` never + excuses `> file`; `2>&1` is fine); options that write or update files (`git … --output`, `sort -o`, `find + -fprint`/`-fls`, `tree -o`, `yq -i`, `--fix`, `--write`, `jest -u`, snapshot updates); `scp` and `rsync` (except + a dry run); `--watch` modes that never finish; shutdown, reboot, halt and power-off. - Each sample keeps at most 200 kB of stdout and stderr (decoded as UTF-8 across chunk boundaries), says when it was cut, and has its terminal control sequences removed. A sample is stopped after the lesser of 60 s or three - intervals and shown as timed out. Stopping signals the owned process group (TERM, then KILL after a grace period); - processes that deliberately leave the group are not followed. + intervals and shown as timed out. Stopping signals the owned process group (TERM, then KILL after a grace period). + When a sample ends, anything still in its process group (a child that ignored TERM, or one that closed its output and + kept running) gets TERM and then KILL, so trackers never accumulate processes; a closing window waits for that + bounded grace period. Processes that deliberately leave the group (`setsid`, daemons) are not followed. +- A tracker whose directory disappears says so (`the directory … is no longer available`) and keeps trying at its + interval, so a remounted volume resumes on its own. ## Ownership and limits diff --git a/docs/testing/v021-physical-qa.md b/docs/testing/v021-physical-qa.md index b4fe9763..7dfc545c 100644 --- a/docs/testing/v021-physical-qa.md +++ b/docs/testing/v021-physical-qa.md @@ -103,9 +103,19 @@ - [ ] 5.5 `/track copy` copies only the latest result; find (Ctrl+F) never matches the live rows. - [ ] 5.6 `/track stop`: the last result appears once in the transcript; `/resume` of the saved session shows it without any per-refresh copies. -- [ ] 5.7 An unknown command asks for Yes first; `rm`, `sudo`, `cd`, `$(...)`, `&` and heredocs are refused. +- [ ] 5.7 An unknown command asks for Yes first ("Track …?"); `rm`, `sudo`, `cd`, `$(...)`, `&` and heredocs are + refused. Also refused: `/track echo x > /tmp/t 2>/dev/null`, `/track git log --output=/tmp/t`, `/track npx jest -u`. + Asks first: `/track env ls`, `/track curl -X POST http://localhost:1/`, `/track npm publish`, `/track ssh localhost + true`. Starts at once: `/track npm test 2>&1`, `/track curl -s http://localhost:1/` (fails, shows the exit code). - [ ] 5.8 Close the window with a tracker running: no `sh` or command processes are left (`ps`). `/watch git status` still opens its manager. +- [ ] 5.9 Four trackers at once (`ls`, `df -h`, `cat f.txt`, a failing one): each block refreshes on its own, the + composer area shows one `4 trackers · 1 failing` row, narrowing the window to 60 columns and back keeps every block + readable, and `/track stop all` leaves each last result once. +- [ ] 5.10 `/track --every 2s -- sh -c 'trap "" TERM; exec >/dev/null 2>&1; exec sleep 300'` (Yes), then `/track stop`: + within 3 seconds `pgrep -f "sleep 300"` finds nothing. +- [ ] 5.11 Safe glyphs (Settings → Appearance) and `NO_COLOR=1`: the status marks are ASCII (`*`, `=`, `#`) and the + status still says live, paused or stopped in words. ## 6. Native modules and diagnostics From 9780d4d75d3e345c750e86287d0fcd1ed9bda7a7 Mon Sep 17 00:00:00 2001 From: raiseCatError <315733358+raiseCatError@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:43:20 +0530 Subject: [PATCH 5/5] /track: keep refusing `&>`, a background job in the POSIX sh that runs samples dash reads `cmd &>/dev/null` as `cmd &` followed by an empty redirection, so the command would run detached. Descriptor duplication (`2>&1`, `>&2`) stays allowed. Refs #393, #420 --- src/tasks/WatchTasks.ts | 5 +++-- tests/trackSafety.test.ts | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/tasks/WatchTasks.ts b/src/tasks/WatchTasks.ts index 9503efd9..ca021c43 100644 --- a/src/tasks/WatchTasks.ts +++ b/src/tasks/WatchTasks.ts @@ -40,8 +40,9 @@ export function watchSafety(command: string): WatchSafety { return {kind: 'refused', reason: 'Shell and process substitutions cannot be tracked repeatedly.'}; } // Detached shell jobs would outlive the observed sample and multiply on every tick; heredocs also need a different - // stdin/input-ownership contract. `&&`, descriptor duplication (`2>&1`) and `&>` are not background operators. - if (/(?:^|[^&<>])&(?![&>])|<&1`, `>&2`) are not background operators; `&>` + // is one for the POSIX sh that runs samples (dash reads `cmd &>file` as `cmd &` then `>file`), so it stays refused. + if (/(?:^|[^&<>])&(?!&)|< classifyCommand(part.text, part.pipedFrom).includes('modifies')), `paste review: ${command}`); assert.equal(kind(command), 'refused', command); } - for (const command of ['ls >/dev/null', 'ls 2>/dev/null', 'npm test 2>&1', 'ls >&2', 'ls 2>&1 | tail -5', 'ls &>/dev/null', 'echo "a > b"', "echo 'x >y'", 'echo a\\>b', 'ls >&-']) { + for (const command of ['ls >/dev/null', 'ls 2>/dev/null', 'npm test 2>&1', 'ls >&2', 'ls 2>&1 | tail -5', 'echo "a > b"', "echo 'x >y'", 'echo a\\>b', 'ls >&-']) { assert.ok(!writesFiles(command), command); assert.notEqual(kind(command), 'refused', command); } assert.deepEqual(splitCommands('ls >| out').map(part => part.text), ['ls >| out'], '>| is a redirection, not a pipe'); + // POSIX sh (dash) reads `cmd &>/dev/null` as a background job: never repeated, even to /dev/null. + assert.equal(watchSafety('ls &>/dev/null').kind, 'refused'); + assert.equal(watchSafety('ls &>/dev/null').kind === 'refused' && watchSafety('ls &>/dev/null').reason, 'Background operators and heredocs cannot be tracked safely.'); }); test('/track asks before a command that runs another command or changes its environment', () => {