From d8c61177c3dcecb48caeb204bf0f40aeb31b8da5 Mon Sep 17 00:00:00 2001 From: Pierrick Gourlain Date: Tue, 22 Sep 2026 19:26:44 +0200 Subject: [PATCH] add useShell config in order to avoid '/bin/sh ENOENT' in sandbox --- CHANGELOG.md | 9 +++++++- lib/ErlangConfigurationProvider.ts | 13 ++++++++++++ lib/ErlangShellDebugger.ts | 19 ++++++++++------- lib/GenericShell.ts | 21 ++++++++++++++++++- lib/RebarShell.ts | 8 +++---- lib/erlangDebugSession.ts | 2 ++ lib/erlangSettings.ts | 2 ++ lib/lsp/ErlangShellLSP.ts | 7 ++++--- package.json | 15 +++++++++++++ test/test-suite/erlangShellLSP.test.ts | 14 ++++++++----- test/test-suite/useShell.test.ts | 29 ++++++++++++++++++++++++++ 11 files changed, 117 insertions(+), 22 deletions(-) create mode 100644 test/test-suite/useShell.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 378347a0..1cc195e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Change log +## Unreleased + +* [351](https://github.com/pgourlain/vscode_erlang/issues/351) : `erl`/`escript`/`erlc` no longer always spawn through a shell + - A sandboxed or minimal environment (e.g. Kiro) can deny or lack a shell entirely while still allowing the binary itself to run; spawning everything through `/bin/sh -c ...` there failed with `spawn /bin/sh ENOENT` regardless of whether `erl` or the working directory were actually fine + - New setting `erlang.useShell` (`auto`/`always`/`never`, default `auto`): `auto` uses a shell on Windows only (`cmd.exe` is still needed there for `.bat`/`.cmd` dispatch and quoting) and spawns the binary directly elsewhere; `always` restores the previous behavior everywhere, for setups whose `erlangArgs`/`erlangPath`/`rebarPath` rely on shell expansion + +--- + ## Version 1.2.2 (September 22, 2026) * [350](https://github.com/pgourlain/vscode_erlang/issues/350) : Semantic tokens in files that include a header @@ -10,7 +18,6 @@ Thanks to * [Alex Sasnouski](https://github.com/alesasnouski) - --- ## Version 1.2.1 (September 21, 2026) diff --git a/lib/ErlangConfigurationProvider.ts b/lib/ErlangConfigurationProvider.ts index 549a54a1..00ffce72 100644 --- a/lib/ErlangConfigurationProvider.ts +++ b/lib/ErlangConfigurationProvider.ts @@ -20,12 +20,24 @@ export class ErlangDebugConfigurationProvider implements DebugConfigurationProvi } debugConfiguration.verbose = cfg.verbose; debugConfiguration.erlangPath = cfg.erlangPath; + debugConfiguration.useShell = cfg.useShell; return debugConfiguration; } }; let currentSettings: ErlangSettings = null; +// "auto" matches prior (unconfigurable) behavior: a shell is only needed on +// Windows to dispatch .bat/.cmd and re-parse quoted args. "always"/"never" +// are the escape hatch - e.g. for a sandboxed POSIX environment that blocks +// or lacks a shell (#351), or for erlangArgs/erlangPath/rebarPath values that +// rely on shell expansion and would break under "auto" on non-Windows. +export function resolveUseShell(setting: string): boolean { + if (setting === 'always') return true; + if (setting === 'never') return false; + return process.platform === 'win32'; +} + export function configurationChanged(): void { let erlangConf = workspace.getConfiguration("erlang"); let settings: ErlangSettings = { @@ -42,6 +54,7 @@ export function configurationChanged(): void { linting: erlangConf.get('linting', false), rebarBuildArgs: erlangConf.get("rebarBuildArgs", ['compile']), rootPath: extractRootPath(), + useShell: resolveUseShell(erlangConf.get("useShell", "auto")), verbose: erlangConf.get("verbose", false) }; currentSettings = settings; diff --git a/lib/ErlangShellDebugger.ts b/lib/ErlangShellDebugger.ts index 595ad962..4d8ead4d 100644 --- a/lib/ErlangShellDebugger.ts +++ b/lib/ErlangShellDebugger.ts @@ -16,6 +16,7 @@ export interface LaunchRequestArguments extends DebugProtocol.LaunchRequestArgum verbose: boolean; addEbinsToCodepath: boolean; erlangPath : string; // path of erlang if specified in configuration + useShell: boolean; // resolved from erlang.useShell (see ErlangConfigurationProvider) } /** `attach` request: debug a node that is already running on this machine. */ @@ -26,9 +27,11 @@ export interface AttachRequestArguments extends DebugProtocol.AttachRequestArgum erlpath: string; verbose: boolean; erlangPath : string; + useShell: boolean; } -// Values end up on a shell command line (GenericShell spawns with shell:true). +// Values end up on a shell command line when this shell is spawned through +// one (see erlang.useShell); direct-spawn otherwise has no shell to inject into. const NODE_NAME = /^[A-Za-z0-9_\-.]+@[A-Za-z0-9_\-.]+$/; const COOKIE = /^[A-Za-z0-9_\-.@]+$/; @@ -88,10 +91,10 @@ export class ErlangShellForDebugging extends GenericShell { this.argsFileName = path.join(os.tmpdir(), path.basename(startDir) + '_' + randomSuffix); this.argsPrecompiledFileName = path.join(os.tmpdir(), 'bp_' + randomSuffix + ".erl"); this.argsPrecompiledFileName = this.formatPath(this.argsPrecompiledFileName); - var debugStartArgs = ["-noshell", "-pa", `"${bridgePath}"`, "-s", "int", + var debugStartArgs = ["-noshell", "-pa", this.shellQuote(bridgePath), "-s", "int", "-vscode_port", listen_port.toString()]; if (!launchArguments.noDebug) { - debugStartArgs.push("-compiled_args_file", `"${this.argsPrecompiledFileName}"`); + debugStartArgs.push("-compiled_args_file", this.shellQuote(this.argsPrecompiledFileName)); } debugStartArgs.push("-s", "vscode_connection", "start"); @@ -120,11 +123,11 @@ export class ErlangShellForDebugging extends GenericShell { } processArgs.push( // loopback-only distribution listener, as for the LSP node - "-kernel", "inet_dist_use_interface", '"{127,0,0,1}"', - "-pa", `"${bridgePath}"`, + "-kernel", "inet_dist_use_interface", this.shellQuote("{127,0,0,1}"), + "-pa", this.shellQuote(bridgePath), "-vscode_port", listen_port.toString(), "-vscode_attach_node", args.node, - "-compiled_args_file", `"${this.argsPrecompiledFileName}"`, + "-compiled_args_file", this.shellQuote(this.argsPrecompiledFileName), "-s", "vscode_connection", "attach"); this.started = true; return this.LaunchProcess(erlPath, startDir, processArgs, !args.verbose); @@ -230,7 +233,7 @@ export class ErlangShellForDebugging extends GenericShell { fs.writeFileSync(this.argsFileName, argsFileContents); result.push("-args_file"); - result.push("\"" + this.argsFileName + "\""); + result.push(this.shellQuote(this.argsFileName)); } return result; } @@ -313,7 +316,7 @@ export class ErlangShellForDebugging extends GenericShell { fs.writeFileSync(this.argsFileName, argsFileContents); result.push("-args_file"); - result.push("\"" + this.argsFileName + "\""); + result.push(this.shellQuote(this.argsFileName)); } return result; } diff --git a/lib/GenericShell.ts b/lib/GenericShell.ts index 628caa4a..68632fd8 100644 --- a/lib/GenericShell.ts +++ b/lib/GenericShell.ts @@ -30,6 +30,14 @@ export interface IShellOutput { append(value: string): void; } +// Wraps a value that must survive cmd.exe re-parsing (a space, or a +// metacharacter like the `{a,b,c}` in an inet address tuple). A no-op when +// useShell is false: spawn then passes argv elements straight to the OS, so +// a literal quote character would otherwise end up inside the argument itself. +export function quoteForShell(value: string, useShell: boolean): string { + return useShell ? `"${value}"` : value; +} + export class GenericShell extends EventEmitter { protected childProcess: ChildProcess; protected logOutput: ILogOutput; @@ -40,6 +48,12 @@ export class GenericShell extends EventEmitter { public erlangArgs : string[] = []; public erlangDistributedNode: boolean = false; public cacheManagement: string = "memory"; + // cmd.exe is still needed on Windows to dispatch .bat/.cmd and to + // re-parse quoted args; this default ("auto", see erlang.useShell) spawns + // the binary directly everywhere else, since a sandboxed or minimal + // environment (e.g. Kiro, #351) can deny/lack a shell entirely while + // still allowing the binary itself to run. Overridable per erlang.useShell. + public useShell: boolean = process.platform === 'win32'; //provide IGenericShellConfiguration, in order to avoid dependencies on vscode module (it doesn't works with debugger-adpater) constructor(logOutput?: ILogOutput, shellOutput?: IShellOutput, erlangConfiguration?: ErlangSettings) { @@ -69,9 +83,14 @@ export class GenericShell extends EventEmitter { this.erlangArgs = erlangConfiguration.erlangArgs; this.erlangDistributedNode = erlangConfiguration.erlangDistributedNode; this.cacheManagement = erlangConfiguration.cacheManagement; + this.useShell = erlangConfiguration.useShell; } } + protected shellQuote(value: string): string { + return quoteForShell(value, this.useShell); + } + protected RunProcess(processName, startDir: string, args: string[]): Promise { return new Promise((resolve, reject) => { @@ -111,7 +130,7 @@ export class GenericShell extends EventEmitter { // until an unrelated layer surfaced an opaque failure later // (#326, #239) instead of the real spawn error. let launchFailed = false; - this.childProcess = spawn(processName, args, { cwd: startDir, shell: true, stdio: 'pipe', env : childEnv }); + this.childProcess = spawn(processName, args, { cwd: startDir, shell: this.useShell, stdio: 'pipe', env : childEnv }); this.childProcess.on('error', error => { launchFailed = true; this.log("stderr", error.message); diff --git a/lib/RebarShell.ts b/lib/RebarShell.ts index 24f50c80..1cd04217 100644 --- a/lib/RebarShell.ts +++ b/lib/RebarShell.ts @@ -40,10 +40,10 @@ export default class RebarShell extends GenericShell { escript = path.join(erlPath, escript); } let rebarFileName = await this.getRebarFullPath(); - if (rebarFileName.search(' ') > -1) { - // There is at least one space in rebarPath. Use double quotes - // instead of single quotes for cross-operability between - // Unix shells (e.g. bash) and the Windows shell. + if (this.useShell && rebarFileName.search(' ') > -1) { + // There is at least one space in rebarPath. cmd.exe (the only + // shell this still goes through - see erlang.useShell) needs it + // quoted to keep it one argument. rebarFileName = ('"' + rebarFileName + '"'); } let args = [rebarFileName].concat(commands); diff --git a/lib/erlangDebugSession.ts b/lib/erlangDebugSession.ts index a27297b5..fcedf01f 100644 --- a/lib/erlangDebugSession.ts +++ b/lib/erlangDebugSession.ts @@ -206,6 +206,7 @@ export class ErlangDebugSession extends DebugSession implements ILogOutput { this.debug(` arguments : ${args.arguments}`); } this.erlDebugger.erlangPath = args.erlangPath; + this.erlDebugger.useShell = args.useShell ?? (process.platform === 'win32'); //path of erl_connection.beam not compiled with lsp, because we don't that the target access to lsp_xx.beam var bridgeBinPath = path.normalize(path.join(erlangBridgePath, "..", "ebin")) this.erlDebugger.Start(args.erlpath, args.cwd, this._port, bridgeBinPath, args).then(r => { @@ -220,6 +221,7 @@ export class ErlangDebugSession extends DebugSession implements ILogOutput { const args = this._AttachArguments; this.log(`Attaching to node ${args.node}`); this.erlDebugger.erlangPath = args.erlangPath; + this.erlDebugger.useShell = args.useShell ?? (process.platform === 'win32'); var bridgeBinPath = path.normalize(path.join(erlangBridgePath, "..", "ebin")) this.erlDebugger.Attach(args.erlpath, args.cwd, this._port, bridgeBinPath, args).then(r => { this.sendResponse(response); diff --git a/lib/erlangSettings.ts b/lib/erlangSettings.ts index 1c8b944f..fc93df7c 100644 --- a/lib/erlangSettings.ts +++ b/lib/erlangSettings.ts @@ -12,6 +12,8 @@ export interface ErlangSettings { semanticTokensEnabled: boolean; verbose: boolean; debuggerRunMode : string; + // resolved from erlang.useShell ("auto"/"always"/"never") - see ErlangConfigurationProvider.resolveUseShell + useShell: boolean; /// workspace.rootPath, since VSCode 1.78 workspace.workspaceFolders[0].Uri.path rootPath: string; } diff --git a/lib/lsp/ErlangShellLSP.ts b/lib/lsp/ErlangShellLSP.ts index b81b3e4f..8eca73e7 100644 --- a/lib/lsp/ErlangShellLSP.ts +++ b/lib/lsp/ErlangShellLSP.ts @@ -14,13 +14,14 @@ export class ErlangShellLSP extends GenericShell { // -name @127.0.0.1 so local remsh still connects over loopback (a short name // resolves to the LAN address, which the loopback-bound listener refuses), and // confine both the distribution listener (inet_dist_use_interface) and epmd - // (ERL_EPMD_ADDRESS) to 127.0.0.1. The {127,0,0,1} tuple is double-quoted so the - // shell (GenericShell spawns with shell:true) passes it as a single argument. + // (ERL_EPMD_ADDRESS) to 127.0.0.1. this.shellQuote double-quotes the + // {127,0,0,1} tuple when this shell is spawned through one (see + // erlang.useShell) so it re-parses as one argument. if (this.erlangDistributedNode) { debugStartArgs.push( "-name", "vscode_" + listen_port.toString() + "@127.0.0.1", "-setcookie", "vscode_" + listen_port.toString(), - "-kernel", "inet_dist_use_interface", '"{127,0,0,1}"', + "-kernel", "inet_dist_use_interface", this.shellQuote("{127,0,0,1}"), "-env", "ERL_EPMD_ADDRESS", "127.0.0.1"); } // Set management mode for large caches diff --git a/package.json b/package.json index 5584eb94..99081574 100644 --- a/package.json +++ b/package.json @@ -498,6 +498,21 @@ "type": "number", "default": 100, "description": "Maximum line length for document formatting." + }, + "erlang.useShell": { + "type": "string", + "default": "auto", + "description": "Whether erl/escript/erlc are spawned through a shell. 'auto' uses one on Windows only (cmd.exe is needed there for .bat/.cmd and quoting) and spawns the binary directly elsewhere. Set to 'always' if you rely on shell expansion in erlangArgs/erlangPath/rebarPath, or your environment needs it; set to 'never' to force direct spawning (e.g. if a sandboxed environment blocks or lacks a shell).", + "enum": [ + "auto", + "always", + "never" + ], + "enumDescriptions": [ + "Shell on Windows only (default)", + "Always spawn through a shell", + "Never spawn through a shell" + ] } } }, diff --git a/test/test-suite/erlangShellLSP.test.ts b/test/test-suite/erlangShellLSP.test.ts index 83152e6b..5cd69df2 100644 --- a/test/test-suite/erlangShellLSP.test.ts +++ b/test/test-suite/erlangShellLSP.test.ts @@ -1,9 +1,11 @@ import * as assert from 'assert'; import { ErlangShellLSP } from '../../lib/lsp/ErlangShellLSP'; +import { quoteForShell } from '../../lib/GenericShell'; // Capture the argument list ErlangShellLSP.Start would pass to `erl`, without // actually spawning a process, by overriding the protected LaunchProcess. -function captureStartArgs(distributed: boolean): Promise { +// Also returns the shell instance so tests can check its resolved useShell. +function captureStartArgs(distributed: boolean): Promise<{ args: string[], shell: ErlangShellLSP }> { const log = { appendLine: (_: string) => {}, debug: (_: string) => {} }; const shell = new ErlangShellLSP(log); shell.erlangDistributedNode = distributed; @@ -12,12 +14,12 @@ function captureStartArgs(distributed: boolean): Promise { captured = args; return Promise.resolve(true); }; - return shell.Start('', '.', 12345, 'src', '').then(() => captured); + return shell.Start('', '.', 12345, 'src', '').then(() => ({ args: captured, shell })); } suite('ErlangShellLSP distribution hardening', () => { test('distributed node binds distribution and epmd to loopback', async () => { - const args = await captureStartArgs(true); + const { args, shell } = await captureStartArgs(true); // -name @127.0.0.1 (not -sname) so local remsh still reaches the // loopback-bound listener. assert.ok(args.includes('-name'), '-name flag present'); @@ -25,13 +27,15 @@ suite('ErlangShellLSP distribution hardening', () => { assert.ok(!args.includes('-sname'), '-sname must not be used (resolves to LAN host)'); // Distribution listener and epmd confined to loopback. assert.ok(args.includes('inet_dist_use_interface'), 'inet_dist_use_interface present'); - assert.ok(args.includes('"{127,0,0,1}"'), 'loopback tuple quoted for the shell'); + // Quoted only when this shell is spawned through one (erlang.useShell, + // "auto" by default: Windows only) - POSIX under "auto" spawns `erl` directly. + assert.ok(args.includes(quoteForShell('{127,0,0,1}', shell.useShell)), 'loopback tuple quoted for the shell where one is used'); assert.ok(args.includes('ERL_EPMD_ADDRESS'), 'ERL_EPMD_ADDRESS present'); assert.ok(args.includes('127.0.0.1'), 'epmd bound to loopback'); }); test('non-distributed node adds no distribution flags', async () => { - const args = await captureStartArgs(false); + const { args } = await captureStartArgs(false); assert.ok(!args.includes('-name'), 'no -name when not distributed'); assert.ok(!args.includes('-sname'), 'no -sname when not distributed'); assert.ok(!args.includes('inet_dist_use_interface'), 'no distribution binding when not distributed'); diff --git a/test/test-suite/useShell.test.ts b/test/test-suite/useShell.test.ts new file mode 100644 index 00000000..37cce061 --- /dev/null +++ b/test/test-suite/useShell.test.ts @@ -0,0 +1,29 @@ +import * as assert from 'assert'; +import { quoteForShell } from '../../lib/GenericShell'; +import { resolveUseShell } from '../../lib/ErlangConfigurationProvider'; + +suite('erlang.useShell resolution', () => { + test('"always" forces a shell regardless of platform', () => { + assert.strictEqual(resolveUseShell('always'), true); + }); + + test('"never" forces no shell regardless of platform', () => { + assert.strictEqual(resolveUseShell('never'), false); + }); + + test('"auto" (and any unrecognized value) matches the platform default', () => { + const expected = process.platform === 'win32'; + assert.strictEqual(resolveUseShell('auto'), expected); + assert.strictEqual(resolveUseShell('anything-else'), expected); + }); +}); + +suite('quoteForShell', () => { + test('wraps the value in double quotes when useShell is true', () => { + assert.strictEqual(quoteForShell('{127,0,0,1}', true), '"{127,0,0,1}"'); + }); + + test('leaves the value untouched when useShell is false', () => { + assert.strictEqual(quoteForShell('{127,0,0,1}', false), '{127,0,0,1}'); + }); +});