mirror of
https://github.com/affaan-m/ECC.git
synced 2026-10-23 03:27:50 +09:00
fix(install): support invocation through POSIX sh (#3014)
* fix(install): make install.sh robust to sh/dash invocation Two related portability fixes so `sh install.sh` behaves correctly even though the script is written for bash: - The cygpath detection used the bash-only `&>/dev/null` redirection. dash misparses `&>`, so `command -v cygpath &>/dev/null` always took the true branch and tried to run the nonexistent `cygpath` binary, failing with "cygpath: not found". Switched to the POSIX-portable `>/dev/null 2>&1` form. - Some dash builds don't support `set -o pipefail`, so `sh install.sh` can fail immediately at that line before even reaching the cygpath check (or the `[[ ... ]]` symlink-resolution logic further down). Added a guard that re-execs the script under bash when the current shell lacks bash capabilities, so the rest of the bash-only syntax always runs under a real bash regardless of the invoking shell. The guard probes for the `[[` compound command directly (via `eval '[[ 1 == 1 ]]'`) rather than trusting the $BASH_VERSION environment variable, since that variable could be inherited or spoofed under a non-bash shell and cause the guard to be skipped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XBaBRjaZgwhrZtYoqcQfp * test(install): cover the sh install.sh re-exec path Greptile flagged that the bash re-exec guard added in install.sh has no automated coverage, since the existing test helper only invoked the wrapper via `bash`. Adds two regression tests: - "delegates to the Node installer when invoked via a POSIX sh wrapper" — runs the script via `sh` and asserts the args/cwd still reach the Node installer correctly. - "re-execs into bash under sh even when BASH_VERSION is spoofed in the environment" — exercises the eval '[[ 1 == 1 ]]' capability probe directly, guarding against a regression back to trusting the (spoofable) $BASH_VERSION variable. CodeRabbit then pointed out that the second test used generic `sh`, which could trivially pass without exercising the re-exec branch at all if `sh` ever resolves to bash on some system. Added a findPosixOnlyShell() helper that prefers `dash` (falling back to checking `sh`, and skipping with an explicit message if neither genuinely lacks bash's `[[`), so the test reliably exercises the branch it claims to cover instead of passing vacuously. CodeRabbit then flagged that the skip path itself was miscounted as a pass (the callback returned normally, so `test()` reported success and `passed` was incremented even though nothing executed). Moved the findPosixOnlyShell() check outside the test() registration, so the test is only registered — and only counted — when a genuinely POSIX-only shell is actually available; otherwise it's excluded from both the passed and failed counts with an explicit skip line. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XBaBRjaZgwhrZtYoqcQfp --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
eaeef53a15
commit
042924e1f4
2 files changed
+98
-2
No files matched your search
+9
-1
@@ -4,6 +4,14 @@
|
|||||||
# This wrapper resolves the real repo/package root when invoked through a
|
# This wrapper resolves the real repo/package root when invoked through a
|
||||||
# symlinked npm bin, then delegates to the Node-based installer runtime.
|
# symlinked npm bin, then delegates to the Node-based installer runtime.
|
||||||
|
|
||||||
|
# Re-exec under bash if invoked via a POSIX sh (e.g. `sh install.sh`), since
|
||||||
|
# the rest of this script relies on bash features (`set -o pipefail`, `[[`).
|
||||||
|
# Probe actual shell capability instead of trusting $BASH_VERSION, which is
|
||||||
|
# just an environment variable and could be inherited/spoofed under sh.
|
||||||
|
if ! (eval '[[ 1 == 1 ]]' >/dev/null 2>&1); then
|
||||||
|
exec bash "$0" "$@"
|
||||||
|
fi
|
||||||
|
|
||||||
set -euo pipefail
|
set -euo pipefail
|
||||||
|
|
||||||
SCRIPT_PATH="$0"
|
SCRIPT_PATH="$0"
|
||||||
@@ -25,7 +33,7 @@ fi
|
|||||||
# On MSYS2/Git Bash, convert the POSIX path to a Windows path so Node.js
|
# On MSYS2/Git Bash, convert the POSIX path to a Windows path so Node.js
|
||||||
# (a native Windows binary) receives a valid path instead of a doubled one
|
# (a native Windows binary) receives a valid path instead of a doubled one
|
||||||
# like G:\g\projects\... that results from Git Bash's auto path conversion.
|
# like G:\g\projects\... that results from Git Bash's auto path conversion.
|
||||||
if command -v cygpath &>/dev/null; then
|
if command -v cygpath >/dev/null 2>&1; then
|
||||||
NODE_SCRIPT="$(cygpath -w "$SCRIPT_DIR/scripts/install-apply.js")"
|
NODE_SCRIPT="$(cygpath -w "$SCRIPT_DIR/scripts/install-apply.js")"
|
||||||
else
|
else
|
||||||
NODE_SCRIPT="$SCRIPT_DIR/scripts/install-apply.js"
|
NODE_SCRIPT="$SCRIPT_DIR/scripts/install-apply.js"
|
||||||
|
|||||||
@@ -18,6 +18,26 @@ function cleanup(dirPath) {
|
|||||||
fs.rmSync(dirPath, { recursive: true, force: true });
|
fs.rmSync(dirPath, { recursive: true, force: true });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Finds a shell that genuinely lacks bash's `[[` compound command, so tests
|
||||||
|
// that exercise install.sh's capability-probe re-exec guard actually take
|
||||||
|
// the "not bash" branch instead of trivially passing on a system where
|
||||||
|
// `sh` happens to resolve to bash.
|
||||||
|
function findPosixOnlyShell() {
|
||||||
|
for (const candidate of ['dash', 'sh']) {
|
||||||
|
try {
|
||||||
|
execFileSync(candidate, ['-c', "eval '[[ 1 == 1 ]]'"], { stdio: 'ignore' });
|
||||||
|
// Probe succeeded: this shell supports `[[`, so it can't stand in for
|
||||||
|
// a POSIX-only shell.
|
||||||
|
} catch (error) {
|
||||||
|
if (error.code === 'ENOENT') {
|
||||||
|
continue; // candidate not installed, try the next one
|
||||||
|
}
|
||||||
|
return candidate; // probe failed: genuinely lacks `[[` support
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
function run(args = [], options = {}) {
|
function run(args = [], options = {}) {
|
||||||
const env = {
|
const env = {
|
||||||
...process.env,
|
...process.env,
|
||||||
@@ -26,7 +46,7 @@ function run(args = [], options = {}) {
|
|||||||
};
|
};
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const stdout = execFileSync('bash', [options.scriptPath || SCRIPT, ...args], {
|
const stdout = execFileSync(options.shell || 'bash', [options.scriptPath || SCRIPT, ...args], {
|
||||||
cwd: options.cwd,
|
cwd: options.cwd,
|
||||||
env,
|
env,
|
||||||
encoding: 'utf8',
|
encoding: 'utf8',
|
||||||
@@ -130,6 +150,74 @@ function runTests() {
|
|||||||
}
|
}
|
||||||
})) passed++; else failed++;
|
})) passed++; else failed++;
|
||||||
|
|
||||||
|
if (test('delegates to the Node installer when invoked via a POSIX sh wrapper', () => {
|
||||||
|
const sourceDir = createTempDir('install-sh-posix-source-');
|
||||||
|
const projectDir = createTempDir('install-sh-posix-target-');
|
||||||
|
const scriptsDir = path.join(sourceDir, 'scripts');
|
||||||
|
const fixtureScript = path.join(sourceDir, 'install.sh');
|
||||||
|
|
||||||
|
try {
|
||||||
|
fs.mkdirSync(scriptsDir, { recursive: true });
|
||||||
|
fs.mkdirSync(path.join(sourceDir, 'node_modules'), { recursive: true });
|
||||||
|
fs.copyFileSync(SCRIPT, fixtureScript);
|
||||||
|
fs.writeFileSync(
|
||||||
|
path.join(scriptsDir, 'install-apply.js'),
|
||||||
|
'console.log(JSON.stringify({ cwd: process.cwd(), args: process.argv.slice(2) }));\n'
|
||||||
|
);
|
||||||
|
|
||||||
|
const result = run(['--target', 'antigravity', '--dry-run', 'typescript'], {
|
||||||
|
cwd: projectDir,
|
||||||
|
scriptPath: fixtureScript,
|
||||||
|
shell: 'sh',
|
||||||
|
});
|
||||||
|
|
||||||
|
assert.strictEqual(result.code, 0, result.stderr);
|
||||||
|
const payload = JSON.parse(result.stdout.trim().split('\n').at(-1));
|
||||||
|
assert.strictEqual(payload.cwd, fs.realpathSync(projectDir));
|
||||||
|
assert.deepStrictEqual(payload.args, ['--target', 'antigravity', '--dry-run', 'typescript']);
|
||||||
|
} finally {
|
||||||
|
cleanup(sourceDir);
|
||||||
|
cleanup(projectDir);
|
||||||
|
}
|
||||||
|
})) passed++; else failed++;
|
||||||
|
|
||||||
|
const posixOnlyShell = findPosixOnlyShell();
|
||||||
|
if (!posixOnlyShell) {
|
||||||
|
console.log(
|
||||||
|
' - skipped: re-execs into bash under sh even when BASH_VERSION is spoofed in the environment ' +
|
||||||
|
'(no shell without `[[` support was found on this system)'
|
||||||
|
);
|
||||||
|
} else if (test('re-execs into bash under sh even when BASH_VERSION is spoofed in the environment', () => {
|
||||||
|
const sourceDir = createTempDir('install-sh-spoof-source-');
|
||||||
|
const projectDir = createTempDir('install-sh-spoof-target-');
|
||||||
|
const scriptsDir = path.join(sourceDir, 'scripts');
|
||||||
|
const fixtureScript = path.join(sourceDir, 'install.sh');
|
||||||
|
|
||||||
|
try {
|
||||||
|
fs.mkdirSync(scriptsDir, { recursive: true });
|
||||||
|
fs.mkdirSync(path.join(sourceDir, 'node_modules'), { recursive: true });
|
||||||
|
fs.copyFileSync(SCRIPT, fixtureScript);
|
||||||
|
fs.writeFileSync(
|
||||||
|
path.join(scriptsDir, 'install-apply.js'),
|
||||||
|
'console.log(JSON.stringify({ cwd: process.cwd(), args: process.argv.slice(2) }));\n'
|
||||||
|
);
|
||||||
|
|
||||||
|
const result = run(['--target', 'antigravity', '--dry-run', 'typescript'], {
|
||||||
|
cwd: projectDir,
|
||||||
|
scriptPath: fixtureScript,
|
||||||
|
shell: posixOnlyShell,
|
||||||
|
env: { BASH_VERSION: '9.9.9(1)-spoofed' },
|
||||||
|
});
|
||||||
|
|
||||||
|
assert.strictEqual(result.code, 0, result.stderr);
|
||||||
|
const payload = JSON.parse(result.stdout.trim().split('\n').at(-1));
|
||||||
|
assert.deepStrictEqual(payload.args, ['--target', 'antigravity', '--dry-run', 'typescript']);
|
||||||
|
} finally {
|
||||||
|
cleanup(sourceDir);
|
||||||
|
cleanup(projectDir);
|
||||||
|
}
|
||||||
|
})) passed++; else failed++;
|
||||||
|
|
||||||
if (test('exposes the corrected Claude target help text', () => {
|
if (test('exposes the corrected Claude target help text', () => {
|
||||||
const result = run(['--help']);
|
const result = run(['--help']);
|
||||||
assert.strictEqual(result.code, 0, result.stderr);
|
assert.strictEqual(result.code, 0, result.stderr);
|
||||||
|
|||||||
Reference in new issue
Block a user