diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7766a8f..257f3e6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -80,6 +80,7 @@ jobs: # Deploy band server using limactl copy (avoids pipe issues with --plain) limactl shell bands-executor -- mkdir -p $VM_HOME/bands-server limactl copy packages/runtime/src/band-server.ts bands-executor:$VM_HOME/bands-server/server.ts + limactl copy packages/runtime/src/cli-wrapper.ts bands-executor:$VM_HOME/bands-server/cli-wrapper.ts limactl shell bands-executor -- bash -c "cd $VM_HOME/bands-server && $VM_HOME/.bun/bin/bun add hono" # Create systemd user service for the band server limactl shell bands-executor -- mkdir -p $VM_HOME/.config/systemd/user @@ -198,6 +199,7 @@ jobs: # Deploy band server using limactl copy (avoids pipe issues with --plain) limactl shell bands-executor -- mkdir -p $VM_HOME/bands-server limactl copy packages/runtime/src/band-server.ts bands-executor:$VM_HOME/bands-server/server.ts + limactl copy packages/runtime/src/cli-wrapper.ts bands-executor:$VM_HOME/bands-server/cli-wrapper.ts limactl shell bands-executor -- bash -c "cd $VM_HOME/bands-server && $VM_HOME/.bun/bin/bun add hono" # Create systemd user service for the band server limactl shell bands-executor -- mkdir -p $VM_HOME/.config/systemd/user diff --git a/packages/runtime/src/band-server.ts b/packages/runtime/src/band-server.ts index 3a8d5b0..585f4d3 100644 --- a/packages/runtime/src/band-server.ts +++ b/packages/runtime/src/band-server.ts @@ -21,6 +21,7 @@ import { Hono } from "hono"; import { createHash } from "crypto"; +import { buildCliWrapperScript, buildDenyPatternsFile, SAFE_CMD_NAME } from "./cli-wrapper"; const BAND_RUNNER_USER = "band-runner"; let executing = false; @@ -344,7 +345,7 @@ function setupCliWrappers( const allowedCommands = new Set(ESSENTIAL_COMMANDS); for (const pattern of allowPatterns) { const cmd = pattern.split(/\s+/)[0]; - if (cmd && !cmd.includes("*") && !cmd.includes("/")) { + if (cmd && SAFE_CMD_NAME.test(cmd)) { allowedCommands.add(cmd); } } @@ -352,13 +353,14 @@ function setupCliWrappers( const denyByCmd = new Map(); for (const pattern of denyPatterns) { const cmd = pattern.split(/\s+/)[0]; - if (!cmd) continue; + if (!cmd || !SAFE_CMD_NAME.test(cmd)) continue; const existing = denyByCmd.get(cmd) || []; existing.push(pattern); denyByCmd.set(cmd, existing); } for (const cmd of allowedCommands) { + if (!SAFE_CMD_NAME.test(cmd)) continue; let realPath: string; try { realPath = shell(`readlink -f $(which ${cmd}) 2>/dev/null`).trim(); @@ -366,34 +368,11 @@ function setupCliWrappers( if (!realPath) continue; const denyPats = denyByCmd.get(cmd) || []; - const logLine = `[ -n "\$BAND_OPS_FILE" ] && echo "${cmd} $*" >> "\$BAND_OPS_FILE"`; - const trackLine = trackOps ? buildInsistTracker(cmd) : ""; - - let wrapper: string; if (denyPats.length > 0) { - const patternArray = denyPats.map(p => `"${p.replace(/"/g, '\\"')}"`).join(" "); - wrapper = `#!/bin/bash -FULL_CMD="${cmd} $*" -DENY_PATTERNS=(${patternArray}) -for P in "\${DENY_PATTERNS[@]}"; do - if eval "[[ \\"\\$FULL_CMD\\" == \\$P ]]" 2>/dev/null; then - echo "DENIED: $FULL_CMD" >&2 - exit 126 - fi -done -${logLine} -${trackLine} -exec ${realPath} "$@" -`; - } else { - wrapper = `#!/bin/bash -${logLine} -${trackLine} -exec ${realPath} "$@" -`; + writeFile(`${wrapperDir}/.deny-${cmd}`, buildDenyPatternsFile(denyPats)); } - - writeFile(`${wrapperDir}/${cmd}`, wrapper); + const trackLine = trackOps ? buildInsistTracker(cmd) : ""; + writeFile(`${wrapperDir}/${cmd}`, buildCliWrapperScript(cmd, realPath, denyPats.length > 0, trackLine)); shell(`chmod +x ${wrapperDir}/${cmd}`); } } diff --git a/packages/runtime/src/cli-wrapper.ts b/packages/runtime/src/cli-wrapper.ts new file mode 100644 index 0000000..5a4eaaf --- /dev/null +++ b/packages/runtime/src/cli-wrapper.ts @@ -0,0 +1,54 @@ +/** + * CLI wrapper-script generation. Pure functions, no side effects. + * + * The band server creates a per-execution directory of small bash wrapper + * scripts — one per allowed command. PATH is set to that directory inside the + * sandbox, so only declared commands resolve. Each wrapper logs its + * invocation, checks deny patterns, and execs the real binary. + * + * Security note: deny patterns originate from user-authored BAND.md files. + * They must never be embedded into the wrapper script source. Bash array + * literals and unquoted heredocs perform command substitution at parse time, + * so a pattern like `foo$(id)*` would execute when the wrapper is generated. + * Instead, patterns are written to a side file and read at run time with + * `read -r`, which does no expansion. The match expression `[[ "$x" == $P ]]` + * does glob matching on the value of P without re-parsing. + */ + +/** Conventional command names: letters, digits, underscore, dash, dot. */ +export const SAFE_CMD_NAME = /^[a-zA-Z0-9_.-]+$/; + +export function buildCliWrapperScript( + cmd: string, + realPath: string, + hasDeny: boolean, + trackLine = "" +): string { + if (!SAFE_CMD_NAME.test(cmd)) { + throw new Error(`unsafe cmd name: ${cmd}`); + } + const logLine = `[ -n "\$BAND_OPS_FILE" ] && echo "${cmd} $*" >> "\$BAND_OPS_FILE"`; + const track = trackLine ? `${trackLine}\n` : ""; + if (!hasDeny) { + return `#!/bin/bash +${logLine} +${track}exec ${realPath} "$@" +`; + } + return `#!/bin/bash +FULL_CMD="${cmd} $*" +while IFS= read -r P; do + [ -z "\$P" ] && continue + if [[ "\$FULL_CMD" == \$P ]]; then + echo "DENIED: \$FULL_CMD" >&2 + exit 126 + fi +done < "\$(dirname "\$0")/.deny-${cmd}" +${logLine} +${track}exec ${realPath} "$@" +`; +} + +export function buildDenyPatternsFile(patterns: string[]): string { + return patterns.join("\n") + "\n"; +} diff --git a/packages/runtime/test/unit/band-server-cli-wrappers.test.ts b/packages/runtime/test/unit/band-server-cli-wrappers.test.ts new file mode 100644 index 0000000..410a673 --- /dev/null +++ b/packages/runtime/test/unit/band-server-cli-wrappers.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, test } from "bun:test"; +import { buildCliWrapperScript, buildDenyPatternsFile, SAFE_CMD_NAME } from "../../src/cli-wrapper"; + +describe("buildCliWrapperScript", () => { + test("no-deny wrapper just logs and execs realPath", () => { + const script = buildCliWrapperScript("ls", "/usr/bin/ls", false); + expect(script).toContain("exec /usr/bin/ls"); + expect(script).not.toContain("DENY_PATTERNS"); + expect(script).not.toContain("eval"); + expect(script).not.toContain(".deny-"); + }); + + test("deny wrapper reads patterns from side file, never embeds them", () => { + const script = buildCliWrapperScript("rm", "/usr/bin/rm", true); + expect(script).toContain("read -r P"); + expect(script).toContain('"$(dirname "$0")/.deny-rm"'); + expect(script).toContain("[[ \"$FULL_CMD\" == $P ]]"); + expect(script).not.toContain("eval"); + expect(script).not.toContain("DENY_PATTERNS=("); + }); + + test("rejects cmd names containing shell metacharacters", () => { + expect(() => buildCliWrapperScript("rm$(id)", "/usr/bin/rm", false)).toThrow(); + expect(() => buildCliWrapperScript("rm;ls", "/usr/bin/rm", false)).toThrow(); + expect(() => buildCliWrapperScript("rm`id`", "/usr/bin/rm", false)).toThrow(); + expect(() => buildCliWrapperScript("rm ls", "/usr/bin/rm", false)).toThrow(); + expect(() => buildCliWrapperScript("../bin/rm", "/usr/bin/rm", false)).toThrow(); + }); + + test("accepts conventional cmd names", () => { + expect(() => buildCliWrapperScript("ls", "/usr/bin/ls", false)).not.toThrow(); + expect(() => buildCliWrapperScript("aws-cli", "/usr/bin/aws-cli", false)).not.toThrow(); + expect(() => buildCliWrapperScript("git_lfs", "/usr/bin/git_lfs", false)).not.toThrow(); + expect(() => buildCliWrapperScript("node18", "/usr/bin/node18", false)).not.toThrow(); + expect(() => buildCliWrapperScript("a.out", "/tmp/a.out", false)).not.toThrow(); + }); +}); + +describe("buildDenyPatternsFile", () => { + test("emits patterns one per line with trailing newline, no escaping", () => { + const file = buildDenyPatternsFile(["rm -rf *", 'foo$(id)*', "echo `whoami`"]); + expect(file).toBe("rm -rf *\nfoo$(id)*\necho `whoami`\n"); + }); +}); + +describe("SAFE_CMD_NAME", () => { + test("rejects shell metacharacters", () => { + for (const bad of ["$(id)", "`id`", "rm;ls", "rm|ls", "rm&", "rm>x", "a b", "*", "../rm", "rm$X"]) { + expect(SAFE_CMD_NAME.test(bad)).toBe(false); + } + }); + + test("accepts conventional identifiers", () => { + for (const ok of ["ls", "git", "gh", "git-lfs", "git_lfs", "a.out", "node18"]) { + expect(SAFE_CMD_NAME.test(ok)).toBe(true); + } + }); +});