mirror of
https://github.com/0xrsydn/idx-cli.git
synced 2026-11-07 09:45:36 +00:00
fix(npm): absolute cache root, per-arch cache dir, SIGINT during install
From a second Codex review (gpt-6.1-sol, xhigh): - A relative XDG_CACHE_HOME (invalid per the XDG spec) made the launcher look for, and run, a binary under the current directory, which an untrusted checkout could supply. Use it only when absolute. - The cache dir now includes <platform>-<arch>, so a home directory shared across machines cannot hand an arm64 host an x64 binary. - SIGINT sent to the launcher's PID alone during the first-run install is forwarded to the installer (it was ignored); the launcher exits 130. - npm smoke runs the first-run install with an empty user cache and checks the package binary was created, so a cached binary cannot pass for it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh
This commit is contained in:
parent
907223a838
commit
3041606916
4 changed files with 30 additions and 10 deletions
|
|
@ -54,8 +54,9 @@ its own npm 12, which is how 0.3.1 broke there (issue #41).
|
||||||
|
|
||||||
The binary goes in the package's `bin/`, or, when that is not writable (a
|
The binary goes in the package's `bin/`, or, when that is not writable (a
|
||||||
root-owned `sudo npm install -g` whose postinstall npm 12 skipped), in a
|
root-owned `sudo npm install -g` whose postinstall npm 12 skipped), in a
|
||||||
per-user cache: `$XDG_CACHE_HOME`, else `~/.cache` (`~/Library/Caches` on
|
per-user cache: `$XDG_CACHE_HOME` (if absolute), else `~/.cache`
|
||||||
macOS), under `<package>/<version>/`. The launcher looks in both. A
|
(`~/Library/Caches` on macOS), under `<package>/<version>/<platform>-<arch>/`.
|
||||||
|
The launcher looks in both. A
|
||||||
termination signal during that first-run download stops the installer too,
|
termination signal during that first-run download stops the installer too,
|
||||||
and it removes its partial file.
|
and it removes its partial file.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -20,6 +20,7 @@ let child = null;
|
||||||
// A termination signal received while installing; the installer cleans up
|
// A termination signal received while installing; the installer cleans up
|
||||||
// and exits, and the launcher then stops the same way.
|
// and exits, and the launcher then stops the same way.
|
||||||
let stopping = null;
|
let stopping = null;
|
||||||
|
let installing = false;
|
||||||
|
|
||||||
// Pass termination signals from a supervisor (kill, docker stop) to the child.
|
// Pass termination signals from a supervisor (kill, docker stop) to the child.
|
||||||
for (const signal of ["SIGTERM", "SIGHUP"]) {
|
for (const signal of ["SIGTERM", "SIGHUP"]) {
|
||||||
|
|
@ -32,8 +33,15 @@ for (const signal of ["SIGTERM", "SIGHUP"]) {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
// Ctrl+C already reaches the child through the terminal's process group.
|
// Ctrl+C already reaches the binary through the terminal's process group, so
|
||||||
process.on("SIGINT", () => {});
|
// the launcher ignores SIGINT then. During the first-run install, forward it
|
||||||
|
// too: a supervisor may signal only the launcher's PID.
|
||||||
|
process.on("SIGINT", () => {
|
||||||
|
if (installing) {
|
||||||
|
stopping = "SIGINT";
|
||||||
|
child.kill("SIGINT");
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
function exitLike(code, signal) {
|
function exitLike(code, signal) {
|
||||||
if (signal) {
|
if (signal) {
|
||||||
|
|
@ -61,6 +69,7 @@ if (installed) {
|
||||||
// or its download failed: install now. Its log goes to stderr, since stdout
|
// or its download failed: install now. Its log goes to stderr, since stdout
|
||||||
// is the MCP channel for idx-mcp.
|
// is the MCP channel for idx-mcp.
|
||||||
console.error(`${packageName}: binary not installed; running the installer`);
|
console.error(`${packageName}: binary not installed; running the installer`);
|
||||||
|
installing = true;
|
||||||
child = spawn(process.execPath, [installer], { stdio: ["ignore", 2, 2] });
|
child = spawn(process.execPath, [installer], { stdio: ["ignore", 2, 2] });
|
||||||
child.once("error", (error) => {
|
child.once("error", (error) => {
|
||||||
console.error(`${packageName}: failed to run the installer: ${error.message}`);
|
console.error(`${packageName}: failed to run the installer: ${error.message}`);
|
||||||
|
|
@ -68,6 +77,7 @@ if (installed) {
|
||||||
});
|
});
|
||||||
child.once("exit", (code, signal) => {
|
child.once("exit", (code, signal) => {
|
||||||
child = null;
|
child = null;
|
||||||
|
installing = false;
|
||||||
if (signal || stopping) {
|
if (signal || stopping) {
|
||||||
exitLike(code, signal || stopping);
|
exitLike(code, signal || stopping);
|
||||||
return;
|
return;
|
||||||
|
|
|
||||||
|
|
@ -23,13 +23,19 @@ const binaryName = process.platform === "win32" ? `${nativeBinary}.exe` : native
|
||||||
// (a root-owned global install whose postinstall npm 12 skipped), in a
|
// (a root-owned global install whose postinstall npm 12 skipped), in a
|
||||||
// per-user cache. scripts/idx.js looks in both, in this order.
|
// per-user cache. scripts/idx.js looks in both, in this order.
|
||||||
const packageBinaryPath = path.join(packageRoot, "bin", binaryName);
|
const packageBinaryPath = path.join(packageRoot, "bin", binaryName);
|
||||||
|
// A relative XDG_CACHE_HOME is invalid per the XDG spec, and would make the
|
||||||
|
// launcher run a binary from the current directory: ignore it.
|
||||||
|
const xdgCacheHome = process.env.XDG_CACHE_HOME;
|
||||||
const cacheBinaryPath = path.join(
|
const cacheBinaryPath = path.join(
|
||||||
process.env.XDG_CACHE_HOME ||
|
xdgCacheHome && path.isAbsolute(xdgCacheHome)
|
||||||
(process.platform === "darwin"
|
? xdgCacheHome
|
||||||
|
: process.platform === "darwin"
|
||||||
? path.join(os.homedir(), "Library", "Caches")
|
? path.join(os.homedir(), "Library", "Caches")
|
||||||
: path.join(os.homedir(), ".cache")),
|
: path.join(os.homedir(), ".cache"),
|
||||||
packageName,
|
packageName,
|
||||||
packageManifest.version,
|
packageManifest.version,
|
||||||
|
// A home directory shared across machines must not mix architectures.
|
||||||
|
`${process.platform}-${process.arch}`,
|
||||||
binaryName,
|
binaryName,
|
||||||
);
|
);
|
||||||
const binaryLocations = [packageBinaryPath, cacheBinaryPath];
|
const binaryLocations = [packageBinaryPath, cacheBinaryPath];
|
||||||
|
|
|
||||||
|
|
@ -88,9 +88,12 @@ echo "npm smoke: installing idx-mcp with --ignore-scripts"
|
||||||
no_scripts_dir="${smoke_dir}/idx-mcp-no-scripts"
|
no_scripts_dir="${smoke_dir}/idx-mcp-no-scripts"
|
||||||
npm_config_cache="$npm_cache" npm install --prefix "$no_scripts_dir" "${smoke_dir}"/idx-mcp/idx-mcp-*.tgz \
|
npm_config_cache="$npm_cache" npm install --prefix "$no_scripts_dir" "${smoke_dir}"/idx-mcp/idx-mcp-*.tgz \
|
||||||
--no-save --no-package-lock --no-audit --no-fund --ignore-scripts
|
--no-save --no-package-lock --no-audit --no-fund --ignore-scripts
|
||||||
IDX_BINARY_URL="${repo_root}/target/release/idx-mcp" \
|
# An empty user cache, so a binary cached by an earlier run cannot stand in
|
||||||
|
# for the first-run install.
|
||||||
|
XDG_CACHE_HOME="${smoke_dir}/no-scripts-cache" IDX_BINARY_URL="${repo_root}/target/release/idx-mcp" \
|
||||||
"${no_scripts_dir}/node_modules/.bin/idx-mcp" --help > "${smoke_dir}/no-scripts-help.txt"
|
"${no_scripts_dir}/node_modules/.bin/idx-mcp" --help > "${smoke_dir}/no-scripts-help.txt"
|
||||||
if ! head -n 1 "${smoke_dir}/no-scripts-help.txt" | grep -q "MCP server"; then
|
if ! head -n 1 "${smoke_dir}/no-scripts-help.txt" | grep -q "MCP server" \
|
||||||
|
|| [[ ! -x "${no_scripts_dir}/node_modules/idx-mcp/bin/idx-mcp" ]]; then
|
||||||
echo "npm smoke: idx-mcp without postinstall did not start cleanly:" >&2
|
echo "npm smoke: idx-mcp without postinstall did not start cleanly:" >&2
|
||||||
cat "${smoke_dir}/no-scripts-help.txt" >&2
|
cat "${smoke_dir}/no-scripts-help.txt" >&2
|
||||||
exit 1
|
exit 1
|
||||||
|
|
@ -104,7 +107,7 @@ chmod -R a-w "${read_only_dir}/node_modules/idx-mcp"
|
||||||
XDG_CACHE_HOME="${smoke_dir}/user-cache" IDX_BINARY_URL="${repo_root}/target/release/idx-mcp" \
|
XDG_CACHE_HOME="${smoke_dir}/user-cache" IDX_BINARY_URL="${repo_root}/target/release/idx-mcp" \
|
||||||
"${read_only_dir}/node_modules/.bin/idx-mcp" --help > "${smoke_dir}/read-only-help.txt"
|
"${read_only_dir}/node_modules/.bin/idx-mcp" --help > "${smoke_dir}/read-only-help.txt"
|
||||||
chmod -R u+w "${read_only_dir}/node_modules/idx-mcp"
|
chmod -R u+w "${read_only_dir}/node_modules/idx-mcp"
|
||||||
cached=("${smoke_dir}"/user-cache/idx-mcp/*/idx-mcp)
|
cached=("${smoke_dir}"/user-cache/idx-mcp/*/*/idx-mcp)
|
||||||
if ! head -n 1 "${smoke_dir}/read-only-help.txt" | grep -q "MCP server" || [[ ! -x "${cached[0]}" ]]; then
|
if ! head -n 1 "${smoke_dir}/read-only-help.txt" | grep -q "MCP server" || [[ ! -x "${cached[0]}" ]]; then
|
||||||
echo "npm smoke: idx-mcp with a read-only package did not install to the user cache:" >&2
|
echo "npm smoke: idx-mcp with a read-only package did not install to the user cache:" >&2
|
||||||
cat "${smoke_dir}/read-only-help.txt" >&2
|
cat "${smoke_dir}/read-only-help.txt" >&2
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue