From de37af4b8cba4f3662d7d9b0444eff3d6452b383 Mon Sep 17 00:00:00 2001 From: Agent Date: Mon, 12 Oct 2026 22:33:11 +0000 Subject: [PATCH] feat: lift shell read restrictions and page bash/read tool outputs to 200 lines --- README.md | 24 ++++----- client.js | 68 +++++++++++++++++++++--- index.js | 108 +++++++++++++++++++++++++++++++++++--- package.json | 2 +- test/client.test.mjs | 2 +- test/guard.test.mjs | 35 +++++++++++- test/integration.test.mjs | 6 ++- 7 files changed, 213 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index b4a005f..0e5c9f8 100644 --- a/README.md +++ b/README.md @@ -62,37 +62,33 @@ Every tagged `read`/`grep`/`edit` records a snapshot. When the file changed sinc A tag that was never issued for that file (e.g. copied from another file) is rejected with a hint. -## Paging: read, grep and glob +## Paging: read, grep, glob and bash -Like oh-my-pi, outputs are cut into pages so the model never pulls a whole file or huge search into context at once: -- **`read`**: at most **200 lines** per call (`limit` above 200 is capped); the footer gives the `offset` to continue. +Outputs are cut into pages so the model never pulls a whole file, huge search, or giant shell stream into context at once: +- **`read`**: at most **200 lines** per call (`limit` above 200 is capped); the footer gives the `offset` to continue. The UI row indicates the line range being read (`Read lines A–B`). - **`grep`**: at most **200 matches** per call, grouped under `[PATH#TAG]` headers. Pass `offset` for the next page (`… call grep again with offset=200`). - **`glob`**: at most **200 paths** per call, with the same `offset` cursor. +- **`bash`**: output is capped at **200 lines** per stream (`stdout` / `stderr`). The complete, uncut output is saved to a temporary file (`/tmp/omp-edit-output/...txt`), and the footer instructs the model how to continue reading with `read` (`read file_path="..." offset=201`). Change the page size with `pageLines`. ## Guard rule -A `tools/pre-execute` listener makes the agent use these tools instead of working around them through the shell. Denied calls get a short reason that names the right tool. +A `tools/pre-execute` listener ensures file modifications go through appropriate tools while lifting restrictions on reading files via shell. Writing through shell is blocked. | Denied | Use instead | | --- | --- | | `sed -i`, `perl -i`, `ed`/`ex`, `tee`, `>`/`>>` into files, heredocs, `truncate`/`dd` | `edit` (hashline) | -| Inline scripts that can touch files, processes or the network (`node -e "require('fs')…"`, `node --input-type=module --eval "import …"`, `python3 -c "open(…)"`, `ruby -e "File.write…"`, `deno eval`, `bun -e`, `php -r`…), or scripts piped in through stdin (`… \| node`, `node - < f`) | `read`/`grep`/`edit` | -| `cat`/`head`/`tail`/`less`/`bat`/`nl FILE`, `sed -n`, `awk` | `read` (paged) | -| `grep`/`rg`/`ag`/`ack` over files | `grep` (paged, tagged) | -| `find`/`fd`/`locate`, `ls -R` | `glob` | +| Inline scripts that write files (`node -e "require('fs').writeFileSync…"`, `python3 -c "open(…,'w')"`, `ruby -e "File.write…"`, `awk` writes) | `edit` / `write` | +| `find -delete`, `find -fprint` | `edit` / file tools | | `str_replace_editor`, `apply_patch` tools; `write` on an **existing** file | `edit` (`write` stays allowed for brand-new files) | What stays allowed: +- **Reading files and searching via shell:** `cat`, `head`, `tail`, `less`, `more`, `bat`, `nl`, `sed -n`, `grep`, `rg`, `ag`, `find`, `ls -R`, read-only `awk` are all permitted. Their output is automatically paged to 200 lines by the `bash` wrapper. - Text inside quotes is ignored (`git commit -m "fix > bug"`). -- Redirection to `/dev/null` is fine. +- Redirection to `/dev/null`, `/dev/stderr`, `/dev/stdout` is fine. - Filters on piped output keep working (`npm test 2>&1 | tail -5`, `ps aux | grep node`). -- Computation-only one-liners: `node -e "console.log(1+1)"`, `node -p "2**10"`, `python3 -c "print(sum(range(9)))"`, `ruby -e 'puts 1+1'`. Running a file is a normal command (`node --test test/`, `node build.mjs`). - -How inline scripts are judged: the guard works out which interpreter runs (also through `env`, `sudo` and absolute paths) and pulls out its program from `-e`/`-p`/`--eval`/`--print`/`--eval=`/`-c`/`-r`/`eval`. The program is allowed only if it contains **none** of the identifiers that start any I/O path, for example: -- JavaScript: `require`, `import`, `process`, `globalThis`, `eval`, `Function`, `constructor`, `fetch`, `Deno`, `Bun`, or bracket access with a string (`x['y']`); -- Python: `import`, `__import__`, `open`, `os`, `subprocess`, dunder attributes; +- Computation-only one-liners: `node -e "console.log(1+1)"`, `node -p "2**10"`, `python3 -c "print(sum(range(9)))"`. - Ruby/Perl: `File`, `IO`, `system`, `open`, backticks. Banning the identifiers that start an I/O path covers obfuscation too: `fs['write'+'File']` still needs `require` or `import` to get `fs`. A program the guard can't read (stdin, `-`) is denied. diff --git a/client.js b/client.js index 7db23f3..92ec2c3 100644 --- a/client.js +++ b/client.js @@ -340,15 +340,71 @@ window.__ModuleLoader__.load({ return preparing ? h(PreparingEditRow, props) : h(StartedEditRow, props); } + /** `[first, last]` file lines a read call covers: from the result window once settled, else from the arguments. */ + function readRange(block) { + if (!block) return null; + const done = 'kind' in block; + if (done && block.isError) return null; + const lines = done && block.meta && Array.isArray(block.meta.lines) ? block.meta.lines : null; + if (lines) { + if (!lines.length) return null; + const a = lines[0]?.number; const b = lines[lines.length - 1]?.number; + return Number.isInteger(a) && Number.isInteger(b) ? [a, b] : null; + } + let args = {}; + try { args = JSON.parse((done ? block.call?.argsRaw : block.argsRaw) || '{}') || {}; } catch { return null; } + const pos = (v) => (Number.isInteger(v) && v >= 1 ? v : undefined); + const start = pos(args.offset) ?? 1; + const limit = Math.min(pos(args.limit) ?? PAGE_LINES, PAGE_LINES); + return [start, start + limit - 1]; + } + + const PAGE_LINES = 200; + + /** The stock read row (priority 0 entry of the same slot); looked up at render time so load order doesn't matter. */ + let stockRead; + function findStockRead(ctx, self) { + if (stockRead) return stockRead; + try { + const entries = typeof ctx.slots.entries === 'function' ? ctx.slots.entries('tool.call.toolview') : []; + const hit = entries.find((e) => e.options?.key === 'read' && e.component !== self); + if (hit) stockRead = hit.component; + } catch { /* slot service without entry access */ } + return stockRead; + } + + /** Stock read row with the title changed to "Read lines A–B". */ + function makeReadRow(ctx) { + function ReadRow(props) { + const stock = findStockRead(ctx, ReadRow); + if (!stock) return null; + const range = readRange(props.block); + if (!range) return h(stock, props); + const base = tr(props.t, 'tool.title.read', 'Read'); + const title = `${base} lines ${range[0]}–${range[1]}`; + const t = (key, params) => (key === 'tool.title.read' ? title : props.t(key, params)); + return h(stock, { ...props, t }); + } + return ReadRow; + } + return { inject: ['slots'], apply(ctx) { - ctx.slots.inject('tool.call.toolview', () => ctx.slots.register({ - name: 'tool.call.toolview', - key: 'edit', - locale: 'conversation', - priority: -1, - }, EditRow)); + ctx.slots.inject('tool.call.toolview', function* () { + yield ctx.slots.register({ + name: 'tool.call.toolview', + key: 'edit', + locale: 'conversation', + priority: -1, + }, EditRow); + yield ctx.slots.register({ + name: 'tool.call.toolview', + key: 'read', + locale: 'conversation', + priority: -1, + }, makeReadRow(ctx)); + }); }, }; }, diff --git a/index.js b/index.js index 99282ee..4dbc38e 100644 --- a/index.js +++ b/index.js @@ -18,6 +18,9 @@ * delegate to them; edit is replaced outright). */ import path from 'node:path' +import fs from 'node:fs' +import os from 'node:os' +import { randomUUID } from 'node:crypto' import { SnapshotStore, rebaseOps } from './lib/store.js' import { parseHashline, applySection } from './lib/hashline.js' import { applyReplace } from './lib/replace.js' @@ -41,6 +44,9 @@ const isObj = (v) => v !== null && typeof v === 'object' && !Array.isArray(v) // Shell patterns the guard rejects. Each entry: [regex, message]. Matching runs per command // segment (split on ; && || |) after quoted strings are blanked, so text inside quotes such as a // commit message or an echo argument cannot trigger it. +// +// The guard only blocks WRITING through the shell. Reading (cat/head/tail/sed -n/grep/find/ls -R …) +// is allowed; the bash wrapper pages its output instead (see pageShellResult). const SHELL_RULES = [ [/(^|\s)sed\s+(-[a-zA-Z]*i|--in-place)/, 'in-place sed edits are not allowed — use the edit tool (hashline patch).'], [/(^|\s)perl\s+-[a-zA-Z]*i/, 'perl -i edits are not allowed — use the edit tool.'], @@ -52,15 +58,13 @@ const SHELL_RULES = [ // are checked separately by inlineScriptViolation(): harmless computation passes, anything that // can reach the filesystem or spawn a process is denied. [/(^|\s)(truncate|dd)\s/, 'truncate/dd are not allowed — use the edit tool.'], - // A pipe consumer (`… | head`) only trims command output and stays allowed: segments start after the pipe. - [/^(cat|head|tail|less|more|bat|nl)\s+(?:-\S+\s+)*[^-\s]/, 'reading files through the shell is not allowed — use the read tool (paged, 200 lines per call).', /^(head|tail)\s+(?:-[a-zA-Z]*\s*\d*\s*|-\d+\s*|--?\w+(=\S+)?\s*)*$/], - [/(^|\s)(grep|egrep|fgrep|rg|ag|ack)\s/, 'searching through the shell is not allowed — use the grep tool (paged, hashline-tagged).', /^(grep|egrep|fgrep|rg)\s+(?:-\S+\s+)*(?:''|""|\S+)\s*$/], - [/(^|\s)(find|fd|locate)\s/, 'finding files through the shell is not allowed — use the glob tool.'], - [/(^|\s)ls\s+(-[a-zA-Z]*R|--recursive)/, 'recursive listings are not allowed — use the glob tool.'], - [/(^|\s)(awk|gawk)\s/, 'awk is not allowed for reading or editing files — use read/grep/edit.'], - [/(^|\s)sed\s+(-n|--quiet)/, 'sed -n is not allowed for reading files — use the read tool with offset.'], + [/(^|\s)find\s.*\s-(delete|fprint\w*|fls|fprintf)\b/, 'find -delete/-fprint write or remove files — use the edit tool (read-only find is fine).'], + [/(^|\s)(awk|gawk)\s+.*(-i\s*inplace|--in-place)/, 'awk in-place edits are not allowed — use the edit tool.'], ] +/** awk programs live in quotes (blanked above), so writes from inside them are checked on the raw command. */ +const AWK_WRITE = /(^|[\s;&|(])g?awk\b[\s\S]*(\bsystem\s*\(|\bprintf?\b[^;}\n]*>|\|\s*["'][^"']*["']\s*[;}]|\bclose\s*\()/ + // ---------------------------------------------------------------- inline scripts // // Allowlist, not denylist: an inline script is allowed only when its body contains NO identifier @@ -171,6 +175,7 @@ export function inlineScriptViolation(cmd) { export function shellViolation(cmd) { const inline = inlineScriptViolation(cmd) if (inline) return inline + if (AWK_WRITE.test(String(cmd))) return `Blocked by omp-edit guard: this awk program writes files or runs commands — use the edit tool. (command: ${String(cmd).slice(0, 120)})` const blanked = String(cmd) .replace(/'[^']*'/g, "''") .replace(/"(?:\\.|[^"\\])*"/g, '""') @@ -192,6 +197,77 @@ export function shellViolation(cmd) { return null } +// ---------------------------------------------------------------- shell output paging +// +// bash may read any file, but the model sees at most `pageLines` lines of a stream per call. +// The complete output (the executor's spill file when it truncated, else the text itself) is kept +// in a file under the OS temp dir and the footer tells the model how to read the next lines with +// the paged `read` tool. + +const OUTPUT_DIR = path.join(os.tmpdir(), 'omp-edit-output') +const OUTPUT_TTL_MS = 24 * 3600 * 1000 +const MAX_FULL_BYTES = 64 * 1024 * 1024 + +/** Write the full output of a stream to a private temp file, sweeping files older than a day. */ +function saveFullOutput(text) { + fs.mkdirSync(OUTPUT_DIR, { recursive: true, mode: 0o700 }) + try { + const now = Date.now() + for (const f of fs.readdirSync(OUTPUT_DIR)) { + const p = path.join(OUTPUT_DIR, f) + try { if (now - fs.statSync(p).mtimeMs > OUTPUT_TTL_MS) fs.unlinkSync(p) } catch {} + } + } catch {} + const file = path.join(OUTPUT_DIR, `${randomUUID()}.txt`) + fs.writeFileSync(file, text, { mode: 0o600 }) + return file +} + +/** + * Cut `text` to its first `pageLines` lines plus a footer that names the lines still unseen and + * the read call that fetches them. Returns null when everything fits in one page. + * @param {string} text full stream text + * @param {number} pageLines page size + * @param {(text: string) => string} save persists the full text, returning its path + * @param {string} [label] stream name shown in the footer + */ +export function pageText(text, pageLines, save, label = 'output') { + const body = text.endsWith('\n') ? text.slice(0, -1) : text + const lines = body.split('\n') + if (lines.length <= pageLines) return null + const file = save(text) + const next = pageLines + 1 + const nextEnd = Math.min(pageLines * 2, lines.length) + const head = lines.slice(0, pageLines).join('\n') + return { + file, + total: lines.length, + text: `${head}\n[${label} lines 1-${pageLines} of ${lines.length}. Read lines ${next}-${nextEnd}: read file_path="${file}" offset=${next} (up to ${pageLines} lines per call)]`, + } +} + +/** Page the stdout/stderr of a finished foreground bash result; other result kinds pass through. */ +export function pageShellResult(value, pageLines, save = saveFullOutput) { + if (!isObj(value) || value.kind !== 'foreground') return value + const out = { ...value } + for (const stream of ['stdout', 'stderr']) { + const s = value[stream] + if (!isObj(s) || typeof s.text !== 'string') continue + let full = s.text + if (s.truncated) { + // The executor kept only the tail; the complete stream is in its spill file. + try { + if (!s.spillPath || fs.statSync(s.spillPath).size > MAX_FULL_BYTES) continue + full = fs.readFileSync(s.spillPath, 'utf8') + } catch { continue } + } + const paged = pageText(full, pageLines, save, stream) + if (!paged) continue + out[stream] = { ...s, text: paged.text, truncated: false, spillPath: paged.file } + } + return out +} + /** Clone a parameters schema, documenting the page cap on `limit`. */ function capLimit(params, max) { const p = structuredClone(params ?? { type: 'object', properties: {} }) @@ -229,6 +305,8 @@ export function apply(ctx, config = {}) { replaceGrep: true, replaceEdit: true, replaceGlob: true, + // bash may read anything; its output is paged to pageLines per call (writes stay guarded). + replaceBash: true, fuzzy: true, // read/grep/glob return at most this many lines/matches/paths per call. pageLines: 200, @@ -722,6 +800,17 @@ export function apply(ctx, config = {}) { } } + /** Wrap the built-in bash: the model sees at most `pageLines` lines per stream, the rest is readable with `read`. */ + function bashTool(builtin) { + return { + ...builtin, + description: `${builtin.description} Reading files and searching through the shell is fine; writing files through the shell is blocked (use edit/write). Output is capped at ${cfg.pageLines} lines per stream: the full output is saved to a file named in the footer, so read the next lines with read (file_path + offset).`, + async execute(args, exec) { + return pageShellResult(await builtin.execute(args, exec), cfg.pageLines) + }, + } + } + // ---------------------------------------------------------------- per-agent installation const installed = new Map() @@ -775,6 +864,7 @@ export function apply(ctx, config = {}) { let wrappedRead = false let wrappedGrep = false let wrappedGlob = false + let wrappedBash = false const ensureWrappers = () => { if (cfg.replaceRead && !wrappedRead) { const builtin = ctx.tools.get('read', agent) @@ -788,6 +878,10 @@ export function apply(ctx, config = {}) { const builtin = ctx.tools.get('grep', agent) if (builtin && !builtin.__ompWrapped) { const t = grepTool(builtin); t.__ompWrapped = true; reg(scoped.tools.register(t)); wrappedGrep = true } } + if (cfg.replaceBash && !wrappedBash) { + const builtin = ctx.tools.get('bash', agent) + if (builtin && !builtin.__ompWrapped) { const t = bashTool(builtin); t.__ompWrapped = true; reg(scoped.tools.register(t)); wrappedBash = true } + } } ensureWrappers() ensure = ensureWrappers diff --git a/package.json b/package.json index 1dc73ac..83dfae7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@local/omp-edit", - "version": "1.2.1", + "version": "1.3.0", "description": "oh-my-pi style editing for DSH: replaces the edit tool with hashline patches (content-hash tagged line anchors, block ops, cut/paste registers, multi-file, stale-tag recovery), plus replace/sloppy/apply_patch modes; tags read and grep output.", "private": true, "type": "module", diff --git a/test/client.test.mjs b/test/client.test.mjs index e868220..c5ecf9a 100644 --- a/test/client.test.mjs +++ b/test/client.test.mjs @@ -38,7 +38,7 @@ function load() { globalThis.window = { __ModuleLoader__: { load: ({ factory }) => { mod = factory((id) => (id === 'react' ? React : P)) } }, setTimeout } new Function(SRC)() let view - mod.apply({ slots: { inject: (_n, f) => f(), register: (_m, C) => { view = C } } }) + mod.apply({ slots: { inject: (_n, f) => { const r = f(); if (r && typeof r.next === 'function') [...r] }, register: (m, C) => { if (m.key === 'edit') view = C } } }) return { React, view } } diff --git a/test/guard.test.mjs b/test/guard.test.mjs index 7c10b4f..6e44088 100644 --- a/test/guard.test.mjs +++ b/test/guard.test.mjs @@ -1,7 +1,7 @@ // Guard rules for shell commands, focused on inline interpreter scripts. import test from 'node:test' import assert from 'node:assert/strict' -import { shellViolation, inlineScriptViolation } from '../index.js' +import { shellViolation, inlineScriptViolation, pageText, pageShellResult } from '../index.js' const ALLOW = [ // harmless computation one-liners @@ -25,6 +25,14 @@ const ALLOW = [ // unrelated commands 'git status', 'npm test 2>&1 | tail -5', + // reading / searching through the shell is allowed (output is paged by the bash wrapper) + 'cat README.md', + 'head -50 index.js', + 'sed -n 10,20p index.js', + 'grep -rn foo .', + 'find . -name "*.js"', + 'ls -R lib', + `awk '{print $1}' f.txt`, ] const BLOCK = [ @@ -61,6 +69,12 @@ const BLOCK = [ `sudo node -e "require('fs')"`, `/usr/bin/node -e "require('fs')"`, `ls && node -e "require('fs')"`, + // shell writes stay blocked + `awk '{print > "out.txt"}' f.txt`, + `awk 'BEGIN{system("rm x")}'`, + 'find . -name "*.tmp" -delete', + 'sed -i s/a/b/ f.txt', + 'echo x > f.txt', ] test('harmless inline one-liners and normal commands are allowed', () => { @@ -76,3 +90,22 @@ test('denial message points at the right tools', () => { assert.match(m, /read\/grep\/edit/) assert.match(m, /computation-only one-liners are fine/) }) + +test('pageText caps output at pageLines and names the next read', () => { + const text = Array.from({ length: 450 }, (_, i) => `l${i + 1}`).join('\n') + '\n' + let saved + const p = pageText(text, 200, (t) => { saved = t; return '/tmp/x.txt' }) + assert.equal(saved, text) + assert.equal(p.text.split('\n').length, 201) + assert.match(p.text, /\[output lines 1-200 of 450\. Read lines 201-400: read file_path="\/tmp\/x\.txt" offset=201/) + assert.equal(pageText('a\nb\n', 200, () => { throw new Error('no save') }), null) +}) + +test('pageShellResult pages stdout and leaves short streams alone', () => { + const long = Array.from({ length: 250 }, (_, i) => String(i)).join('\n') + const v = pageShellResult({ kind: 'foreground', stdout: { text: long, truncated: false }, stderr: { text: 'e', truncated: false } }, 200, () => '/tmp/full.txt') + assert.equal(v.stdout.spillPath, '/tmp/full.txt') + assert.match(v.stdout.text, /stdout lines 1-200 of 250/) + assert.equal(v.stderr.text, 'e') + assert.deepEqual(pageShellResult({ kind: 'background', jobId: 'j' }, 200), { kind: 'background', jobId: 'j' }) +}) diff --git a/test/integration.test.mjs b/test/integration.test.mjs index fdce9a2..6e80dc4 100644 --- a/test/integration.test.mjs +++ b/test/integration.test.mjs @@ -162,8 +162,10 @@ test('guard denies shell edits/reads/searches and non-hashline write tools via t return next() } assert.equal((await gate('bash', { command: 'sed -i s/a/b/ f.txt' })).kind, 'deny') - assert.equal((await gate('bash', { command: 'grep -rn foo .' })).kind, 'deny') - assert.equal((await gate('bash', { command: 'cat README.md' })).kind, 'deny') + assert.equal((await gate('bash', { command: 'grep -rn foo .' })).kind, 'allow') + assert.equal((await gate('bash', { command: 'cat README.md' })).kind, 'allow') + assert.equal((await gate('bash', { command: 'sed -n 1,5p README.md' })).kind, 'allow') + assert.equal((await gate('bash', { command: 'cat a > b' })).kind, 'deny') h.write('exists.txt', 'old\n') const d = await gate('write', { file_path: 'exists.txt', content: 'y' }) assert.equal(d.kind, 'deny')