Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/bunfig/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,16 @@ use crate::bunfig::Bunfig;
fn get_home_config_path(buf: &mut PathBuffer) -> Option<&ZStr> {
let paths: [&[u8]; 1] = [b".bunfig.toml"];

if let Some(data_dir) = env_var::XDG_CONFIG_HOME.get() {
if let Some(data_dir) = env_var::XDG_CONFIG_HOME
.get()
.filter(|p| bun_paths::is_absolute(p))
{
return Some(resolve_path::join_abs_string_buf_z::<platform::Auto>(
data_dir, &mut **buf, &paths,
));
}

if let Some(home_dir) = env_var::HOME.get() {
if let Some(home_dir) = env_var::HOME.get().filter(|p| bun_paths::is_absolute(p)) {
return Some(resolve_path::join_abs_string_buf_z::<platform::Auto>(
home_dir, &mut **buf, &paths,
));
Expand Down
12 changes: 10 additions & 2 deletions src/install/PackageManager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1927,16 +1927,24 @@ pub fn init(

// npm reads `$HOME/.npmrc` and ignores XDG_CONFIG_HOME; keep
// `$XDG_CONFIG_HOME/.npmrc` only when that file actually exists.
// Non-absolute values are skipped: `join_abs_string_buf_z` requires
// an absolute base (asserts on Windows, mangles on POSIX).
Comment thread
robobun marked this conversation as resolved.
Outdated
let mut global_len: usize = 0;
if let Some(xdg_dir) = bun_core::env_var::XDG_CONFIG_HOME.get_not_empty() {
if let Some(xdg_dir) = bun_core::env_var::XDG_CONFIG_HOME
.get_not_empty()
.filter(|p| bun_paths::is_absolute(p))
{
let p =
resolve_path::join_abs_string_buf_z::<platform::Auto>(xdg_dir, &mut buf, &parts);
if bun_sys::exists_z(p) {
global_len = p.len();
}
}
if global_len == 0 {
if let Some(home_dir) = bun_core::env_var::HOME.get_not_empty() {
if let Some(home_dir) = bun_core::env_var::HOME
.get_not_empty()
.filter(|p| bun_paths::is_absolute(p))
{
global_len = resolve_path::join_abs_string_buf_z::<platform::Auto>(
home_dir, &mut buf, &parts,
)
Expand Down
30 changes: 20 additions & 10 deletions src/install/PackageManager/PackageManagerOptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,12 +298,24 @@
pub use bun_install_types::DependencyGroup;
pub use bun_install_types::NodeLinker::NodeLinker;

// `join_abs_string_buf` below requires an absolute base: on Windows it
// asserts, and on POSIX a relative base yields a rooted path with the first
// byte dropped. These env vars are user input and can be empty or relative
// (e.g. `BUN_INSTALL=~/.bun` copied to a Windows shell where `~` is not
// expanded), so skip values that are not absolute and fall through to the
// next candidate. Resolving against the process cwd is not an option because
// cwd changes between `open_global_dir` and `open_global_bin_dir`.
Comment thread
robobun marked this conversation as resolved.
Outdated
#[inline]
fn get_abs(v: Option<&'static [u8]>) -> Option<&'static [u8]> {
v.filter(|p| bun_paths::is_absolute(p))
}

Check warning on line 311 in src/install/PackageManager/PackageManagerOptions.rs

View check run for this annotation

Claude / Claude Code Review

Multi-line explanatory comments flagged by comment-cop

The 7-line comment above `get_abs` (PackageManagerOptions.rs:301-307) and the two added comment lines at PackageManager.rs:1930-1931 both trip the repo's comment-cop check (unresolved github-actions flags at both lines) and violate REVIEW.md's "Only comment what the code cannot say. One line." rule. The rationale (join_abs_string_buf's absolute-base precondition, `~/.bun`-on-Windows example, cwd-change caveat) is already captured in the PR description and the Sentry-link comment in the test file

Check notice on line 311 in src/install/PackageManager/PackageManagerOptions.rs

View check run for this annotation

Claude / Claude Code Review

Missed sibling: bun pm diff ~/... passes unfiltered $HOME to join_abs_string

(pre-existing, missed sibling) `src/runtime/cli/pm_diff_command.rs:92-94` passes raw `env_var::HOME.get()` as the base to `join_abs_string::<platform::Auto>(home, &[rest])` when expanding a `~/`-prefixed `bun pm diff` argument — the same env-var-as-`join_abs_string_buf`-base pattern this PR fixes at four other sites (and the criterion the author used to include `get_home_config_path` and exclude `fetch_cache_directory_path`). Adding `.filter(|p| bun_paths::is_absolute(p))` makes a non-absolute
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.

// mkdir -p + open the dir. Callers store the raw `Fd` (`options.global_bin_dir: Fd`).
pub fn open_global_dir(explicit_global_dir: &[u8]) -> crate::Result<bun_sys::Fd> {
use bun_paths::{platform, resolve_path::join_abs_string_buf};
use bun_sys::{Dir, OpenDirOptions};

if let Some(home_dir) = env_var::BUN_INSTALL_GLOBAL_DIR.get() {
if let Some(home_dir) = env_var::BUN_INSTALL_GLOBAL_DIR.get_not_empty() {
return Dir::cwd()
.make_open_path(home_dir, OpenDirOptions::default())
.map(|d| d.into_raw())
Expand All @@ -317,7 +329,7 @@
.map_err(Into::into);
}

if let Some(home_dir) = env_var::BUN_INSTALL.get() {
if let Some(home_dir) = get_abs(env_var::BUN_INSTALL.get()) {
let mut buf = PathBuffer::uninit();
let parts: [&[u8]; 2] = [b"install", b"global"];
let path = join_abs_string_buf::<platform::Auto>(home_dir, &mut buf.0, &parts);
Expand All @@ -327,9 +339,8 @@
.map_err(Into::into);
}

if let Some(home_dir) = env_var::XDG_CACHE_HOME
.get()
.or_else(|| env_var::HOME.get())
if let Some(home_dir) =
get_abs(env_var::XDG_CACHE_HOME.get()).or_else(|| get_abs(env_var::HOME.get()))
{
let mut buf = PathBuffer::uninit();
let parts: [&[u8]; 3] = [b".bun", b"install", b"global"];
Expand All @@ -347,7 +358,7 @@
use bun_paths::{platform, resolve_path::join_abs_string_buf};
use bun_sys::{Dir, OpenDirOptions};

if let Some(home_dir) = env_var::BUN_INSTALL_BIN.get() {
if let Some(home_dir) = env_var::BUN_INSTALL_BIN.get_not_empty() {
return Dir::cwd()
.make_open_path(home_dir, OpenDirOptions::default())
.map(|d| d.into_raw())
Expand All @@ -365,7 +376,7 @@
}
}

if let Some(home_dir) = env_var::BUN_INSTALL.get() {
if let Some(home_dir) = get_abs(env_var::BUN_INSTALL.get()) {
let mut buf = PathBuffer::uninit();
let parts: [&[u8]; 1] = [b"bin"];
let path = join_abs_string_buf::<platform::Auto>(home_dir, &mut buf.0, &parts);
Expand All @@ -375,9 +386,8 @@
.map_err(Into::into);
}

if let Some(home_dir) = env_var::XDG_CACHE_HOME
.get()
.or_else(|| env_var::HOME.get())
if let Some(home_dir) =
get_abs(env_var::XDG_CACHE_HOME.get()).or_else(|| get_abs(env_var::HOME.get()))
{
let mut buf = PathBuffer::uninit();
let parts: [&[u8]; 2] = [b".bun", b"bin"];
Expand Down
5 changes: 4 additions & 1 deletion src/install/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,10 @@ impl SloppyGlobalGitConfig {
}

fn load_and_parse() -> SloppyGlobalGitConfig {
let Some(home_dir) = bun_core::env_var::HOME.get() else {
let Some(home_dir) = bun_core::env_var::HOME
.get()
.filter(|p| Path::is_absolute(p))
else {
return SloppyGlobalGitConfig::default();
};

Expand Down
50 changes: 49 additions & 1 deletion test/cli/install/bun-pm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { spawn } from "bun";
import { afterAll, afterEach, beforeAll, beforeEach, expect, it, test } from "bun:test";
import { exists, mkdir, writeFile } from "fs/promises";
import { bunEnv, bunExe, bunEnv as env, readdirSorted, tempDir, tmpdirSync } from "harness";
import { cpSync } from "node:fs";
import { cpSync, realpathSync } from "node:fs";
import { join } from "path";
import {
dummyAfterAll,
Expand Down Expand Up @@ -936,3 +936,51 @@ test("bun pm cache rm does not create the directory named by a project-local .en
expect(stderr).not.toContain("error");
expect(exitCode).toBe(0);
});

// https://bun-p9.sentry.io/issues/7403306202/
// Windows panicked in _joinAbsStringBufWindows when $BUN_INSTALL was not an
// absolute path; POSIX silently opened a mangled path rooted at "/". Now
// empty and relative values are skipped so the next candidate ($HOME) wins.
for (const [title, bunInstallValue, base] of [
["empty $BUN_INSTALL falls through to $HOME", "", ["fake-home", ".bun"]],
["relative $BUN_INSTALL falls through to $HOME", "relative-dir", ["fake-home", ".bun"]],
["absolute $BUN_INSTALL", null, ["abs-bun"]],
] as const) {
test(`global dir: ${title}`, async () => {
using dir = tempDir("pm-global-dir-env", {
"package.json": JSON.stringify({ name: "pm-global-dir-env", version: "1.0.0" }),
});
const cwd = String(dir);
const globalDir = join(cwd, ...base, "install", "global");
const binDir = join(cwd, ...base, "bin");
await mkdir(globalDir, { recursive: true });
await writeFile(join(globalDir, "package.json"), JSON.stringify({ name: "global", version: "1.0.0" }));

const spawnEnv: NodeJS.Dict<string> = {
...env,
BUN_INSTALL: bunInstallValue ?? join(cwd, "abs-bun"),
HOME: join(cwd, "fake-home"),
USERPROFILE: join(cwd, "fake-home"),
};
delete spawnEnv.BUN_INSTALL_GLOBAL_DIR;
delete spawnEnv.BUN_INSTALL_BIN;
delete spawnEnv.XDG_CACHE_HOME;
Comment thread
robobun marked this conversation as resolved.
delete spawnEnv.XDG_CONFIG_HOME;

await using proc = Bun.spawn({
cmd: [bunExe(), "pm", "bin", "-g"],
cwd,
stdout: "pipe",
stderr: "pipe",
env: spawnEnv,
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect({ stderr, exitCode }).toEqual({
stderr: expect.not.stringContaining("error:"),
exitCode: 0,
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
expect(await exists(binDir)).toBeTrue();
expect(realpathSync(stdout.trim())).toBe(realpathSync(binDir));
});
}
Loading