fix: sandbox run is only interactive

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 "-- <command>" 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 -- <command> 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

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

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(/exit/)
-  expect(TheCommand.description).toMatch(/-- <command>/)
+  expect(TheCommand.description).toMatch(/interactively/)
   expect(TheCommand.description).toMatch(/\.detached <command>/)
 })
 
@@ -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 () => {