From 575978ba0f06462ca84fa88e2a895e33fbda4008 Mon Sep 17 00:00:00 2001 From: Michael Goberling Date: Thu, 4 Jun 2026 11:39:42 -0400 Subject: [PATCH] fix: sandbox run is only interactive --- src/commands/runtime/sandbox/run.js | 48 ++++++------ src/sandbox-helpers.js | 28 ------- test/commands/runtime/sandbox/run.test.js | 89 ++++++++++++++--------- 3 files changed, 74 insertions(+), 91 deletions(-) diff --git a/src/commands/runtime/sandbox/run.js b/src/commands/runtime/sandbox/run.js index b9815212..1e5984d4 100644 --- a/src/commands/runtime/sandbox/run.js +++ b/src/commands/runtime/sandbox/run.js @@ -16,7 +16,6 @@ const { Flags } = require('@oclif/core') const RuntimeBaseCommand = require('../../../RuntimeBaseCommand') const { buildNetworkPolicy, - buildSandboxCommand, parsePortFlags, parseEgressFlags, splitArgvAtDoubleDash @@ -72,13 +71,22 @@ class SandboxRun extends RuntimeBaseCommand { const { cliArgs, commandArgs } = splitArgvAtDoubleDash(this.argv) const { flags } = await this.parse(SandboxRun, cliArgs) + if (commandArgs.length > 0) { + this._failUsage('This command only supports interactive use. Omit "-- " and type commands when prompted.') + return + } + + if (process.stdin.isTTY !== true) { + this._failUsage('This command requires an interactive terminal. Piped stdin is not supported.') + return + } + let sandbox let rl try { const policy = buildNetworkPolicy(flags.egress) const ports = parsePortFlags(flags.port) const options = await this.getOptions() - const command = buildSandboxCommand(commandArgs) this.log('\nCreating sandbox...') sandbox = await Sandbox.create({ @@ -96,17 +104,11 @@ class SandboxRun extends RuntimeBaseCommand { this._logPolicy(policy) await this._logPreviewUrls(sandbox, ports) - if (command) { - await this._runOnce(sandbox, command) - } - - if (!command) { - this.log('\nSandbox ready. Type "exit" to destroy and quit.\n') + this.log('\nSandbox ready. Type "exit" to destroy and quit.\n') - rl = readline.createInterface({ input: process.stdin, output: process.stdout }) - rl.setPrompt(REPL_PROMPT) - await this._repl(rl, sandbox) - } + rl = readline.createInterface({ input: process.stdin, output: process.stdout }) + rl.setPrompt(REPL_PROMPT) + await this._repl(rl, sandbox) } catch (err) { await this.handleError('failed to run sandbox', err) } finally { @@ -124,6 +126,11 @@ class SandboxRun extends RuntimeBaseCommand { } } + _failUsage (message) { + process.stderr.write(`${message}\n`) + process.exitCode = 2 + } + _logPolicy (policy) { if (!policy) { this.log('Network policy: default-deny (DNS + NATS only)') @@ -199,15 +206,6 @@ class SandboxRun extends RuntimeBaseCommand { this.log(`[detached: ${command.execId} pid: ${command.pid || 'unknown'}]`) } - async _runOnce (sandbox, cmd) { - const result = await sandbox.exec(cmd, { timeout: EXEC_TIMEOUT_MS }) - if (result.stdout) process.stdout.write(result.stdout) - if (result.stderr) process.stderr.write(result.stderr) - if (result.exitCode) { - process.exitCode = result.exitCode - } - } - async _handleHereString (sandbox, input) { const idx = input.indexOf(' <<< ') const command = input.slice(0, idx).trim() @@ -235,9 +233,7 @@ SandboxRun.description = ` sandboxes enabled before you can use this command; contact Adobe to request access. -Create a sandbox and run commands against it. - -Pass -- to run one command and destroy the sandbox. +Create a sandbox and run commands against it interactively. Each command you enter runs in a fresh process. Shell state (working directory, env exports) does not persist between prompts. Chain commands to work @@ -275,8 +271,7 @@ SandboxRun.flags = { SandboxRun.examples = [ '<%= config.bin %> <%= command.id %>', - '<%= config.bin %> <%= command.id %> -- node --version', - '<%= config.bin %> <%= command.id %> -n my-sandbox -- node --version', + '<%= config.bin %> <%= command.id %> -n my-sandbox', '<%= config.bin %> <%= command.id %> -p 3000 -p 8080', '<%= config.bin %> <%= command.id %> -e allow-all', '<%= config.bin %> <%= command.id %> -e "pypi.org:443" -e "api.github.com:443|GET:/repos/**"' @@ -289,6 +284,5 @@ SandboxRun.parseEgressFlags = parseEgressFlags SandboxRun.parsePortFlags = parsePortFlags SandboxRun.buildNetworkPolicy = buildNetworkPolicy SandboxRun.splitArgvAtDoubleDash = splitArgvAtDoubleDash -SandboxRun.buildSandboxCommand = buildSandboxCommand module.exports = SandboxRun diff --git a/src/sandbox-helpers.js b/src/sandbox-helpers.js index 8323db2b..2ddeeef9 100644 --- a/src/sandbox-helpers.js +++ b/src/sandbox-helpers.js @@ -50,33 +50,6 @@ function splitArgvAtDoubleDash (argv) { } } -/** - * Quote argv tokens so the remote shell receives the same argument boundaries - * the local shell passed after `--`. - * - * @param {string} arg command argument - * @returns {string} shell-safe argument - */ -function shellQuote (arg) { - if (/^[A-Za-z0-9_./:=@%+,-]+$/.test(arg)) { - return arg - } - return `'${arg.replace(/'/g, "'\\''")}'` -} - -/** - * Convert args after `--` to a command string for the sandbox executor. - * - * @param {string[]} commandArgs raw command args - * @returns {string} command string - */ -function buildSandboxCommand (commandArgs) { - if (commandArgs.length === 1) { - return commandArgs[0] - } - return commandArgs.map(shellQuote).join(' ') -} - /** * Parse repeatable `--port` flag values for sandbox preview URLs. * @@ -163,7 +136,6 @@ function buildNetworkPolicy (egressArgs) { module.exports = { buildNetworkPolicy, - buildSandboxCommand, parsePortFlags, parseEgressFlags, splitArgvAtDoubleDash diff --git a/test/commands/runtime/sandbox/run.test.js b/test/commands/runtime/sandbox/run.test.js index ddc33323..f606f37d 100644 --- a/test/commands/runtime/sandbox/run.test.js +++ b/test/commands/runtime/sandbox/run.test.js @@ -79,14 +79,14 @@ test('examples', async () => { expect(TheCommand.examples).toBeDefined() expect(TheCommand.examples).toBeInstanceOf(Array) expect(TheCommand.examples.length).toBeGreaterThan(0) - expect(TheCommand.examples).toContain('<%= config.bin %> <%= command.id %> -n my-sandbox -- node --version') + expect(TheCommand.examples).toContain('<%= config.bin %> <%= command.id %> -n my-sandbox') }) test('description includes REPL usage notes', async () => { expect(TheCommand.description).toMatch(/fresh process/) expect(TheCommand.description).toMatch(/<</) + expect(TheCommand.description).toMatch(/interactively/) expect(TheCommand.description).toMatch(/\.detached /) }) @@ -125,7 +125,7 @@ describe('splitArgvAtDoubleDash', () => { }) }) - test('splits CLI args from one-shot command args', () => { + test('splits CLI args from rejected command args', () => { expect(TheCommand.splitArgvAtDoubleDash(['--name', 'box', '--', 'node', '--version'])).toEqual({ cliArgs: ['--name', 'box'], commandArgs: ['node', '--version'], @@ -134,20 +134,6 @@ describe('splitArgvAtDoubleDash', () => { }) }) -describe('buildSandboxCommand', () => { - test('joins safe command args', () => { - expect(TheCommand.buildSandboxCommand(['node', '--version'])).toBe('node --version') - }) - - test('preserves a single command string as-is', () => { - expect(TheCommand.buildSandboxCommand(['npm test -- --watch'])).toBe('npm test -- --watch') - }) - - test('quotes command args that contain shell-sensitive characters', () => { - expect(TheCommand.buildSandboxCommand(['node', '-e', 'console.log("hello world")'])).toBe('node -e \'console.log("hello world")\'') - }) -}) - describe('parseEgressFlags', () => { test('parses single L4 rule', () => { expect(TheCommand.parseEgressFlags(['pypi.org:443'])).toEqual([ @@ -267,6 +253,8 @@ describe('run', () => { let command let handleError let sandbox + const originalStdinIsTTY = process.stdin.isTTY + const originalExitCode = process.exitCode beforeEach(async () => { command = new TheCommand([]) @@ -279,6 +267,18 @@ describe('run', () => { readline.clearLine = jest.fn() readline.cursorTo = jest.fn() readline.createInterface.mockReturnValue(makeRl(['exit'])) + Object.defineProperty(process.stdin, 'isTTY', { + value: true, + configurable: true + }) + }) + + afterEach(() => { + process.exitCode = originalExitCode + Object.defineProperty(process.stdin, 'isTTY', { + value: originalStdinIsTTY, + configurable: true + }) }) test('creates a sandbox with default flags and destroys on exit', async () => { @@ -299,33 +299,50 @@ describe('run', () => { expect(sandbox.destroy).toHaveBeenCalled() }) - test('runs command after -- once, prints output, and destroys sandbox', async () => { + test('rejects command after -- before creating a sandbox', async () => { command.argv = ['--', 'node', '--version'] await command.run() + expect(stderr.output).toMatch('only supports interactive use') + expect(stderr.output).not.toMatch('CLIError') + expect(process.exitCode).toBe(2) + expect(handleError).not.toHaveBeenCalled() + expect(Sandbox.create).not.toHaveBeenCalled() expect(readline.createInterface).not.toHaveBeenCalled() - expect(sandbox.exec).toHaveBeenCalledWith('node --version', { timeout: 30000 }) - expect(stdout.output).toMatch('v20.0.0') - expect(sandbox.destroy).toHaveBeenCalled() + expect(sandbox.exec).not.toHaveBeenCalled() + expect(sandbox.destroy).not.toHaveBeenCalled() }) - test('one-shot command preserves argument boundaries', async () => { - command.argv = ['--', 'node', '-e', 'console.log("hello world")'] + test('rejects piped stdin before creating a sandbox', async () => { + Object.defineProperty(process.stdin, 'isTTY', { + value: false, + configurable: true + }) + command.argv = [] await command.run() - expect(sandbox.exec).toHaveBeenCalledWith('node -e \'console.log("hello world")\'', { timeout: 30000 }) + expect(stderr.output).toMatch('Piped stdin is not supported') + expect(stderr.output).not.toMatch('CLIError') + expect(process.exitCode).toBe(2) + expect(handleError).not.toHaveBeenCalled() + expect(Sandbox.create).not.toHaveBeenCalled() + expect(readline.createInterface).not.toHaveBeenCalled() }) - test('one-shot command writes stderr and sets process exitCode', async () => { - const previousExitCode = process.exitCode - sandbox.exec.mockResolvedValue({ stdout: '', stderr: 'boom\n', exitCode: 7 }) - - command.argv = ['--', 'false'] + test('rejects file-redirected stdin before creating a sandbox', async () => { + Object.defineProperty(process.stdin, 'isTTY', { + value: undefined, + configurable: true + }) + command.argv = [] await command.run() - expect(stderr.output).toMatch('boom') - expect(process.exitCode).toBe(7) - process.exitCode = previousExitCode + expect(stderr.output).toMatch('Piped stdin is not supported') + expect(stderr.output).not.toMatch('CLIError') + expect(process.exitCode).toBe(2) + expect(handleError).not.toHaveBeenCalled() + expect(Sandbox.create).not.toHaveBeenCalled() + expect(readline.createInterface).not.toHaveBeenCalled() }) test('omitting a command enters the REPL', async () => { @@ -362,15 +379,15 @@ describe('run', () => { })) }) - test('forwards -n when running a one-shot command', async () => { - command.argv = ['-n', 'my-sandbox', '--', 'node', '--version'] + test('forwards -n when entering the REPL', async () => { + command.argv = ['-n', 'my-sandbox'] await command.run() expect(Sandbox.create).toHaveBeenCalledWith(expect.objectContaining({ name: 'my-sandbox' })) - expect(sandbox.exec).toHaveBeenCalledWith('node --version', { timeout: 30000 }) - expect(readline.createInterface).not.toHaveBeenCalled() + expect(sandbox.exec).not.toHaveBeenCalled() + expect(readline.createInterface).toHaveBeenCalled() }) test('quit also destroys the sandbox', async () => {