Skip to content

shell(rm): refuse '.' and '..' operands instead of emptying the directory #34906

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 5 commits into from
Closed

shell(rm): refuse '.' and '..' operands instead of emptying the directory #34906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9ce2e56
shell(rm): refuse '.' and '..' operands before recursing; resolve pre…
robobun Jul 21, 2026
55b2880
[autofix.ci] apply automated fixes
autofix-ci[bot] Jul 21, 2026
0e26ef3
test(shell/rm): split the '...' case out and skip it on Windows
robobun Jul 21, 2026
4902e1c
shell(rm): trim review-flagged comments
robobun Jul 29, 2026
a9438ff
test(shell/rm): assert the child process launches from / in the prese…
robobun Jul 29, 2026
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
201 changes: 83 additions & 118 deletions src/runtime/shell/builtin/rm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@ pub enum RmState {
Done {
exit_code: ExitCode,
},
WaitingWriteErr,
Err(ExitCode),
}

pub struct ExecState {
/// Index into argv where filepath args start.
pub args_start: usize,
pub total_tasks: usize,
/// An operand was refused up front; folded into [`exit_code`].
pub had_refusal: bool,
pub err: Option<bun_sys::Error>,
pub error_signal: AtomicBool,
pub output_done: AtomicUsize,
Expand All @@ -51,6 +52,15 @@ impl ExecState {
fn output_drained(&self) -> bool {
self.output_done.load(Ordering::SeqCst) >= self.output_count.load(Ordering::SeqCst)
}

#[inline]
fn exit_code(&self) -> ExitCode {
if self.err.is_some() || self.had_refusal {
1
} else {
0
}
}
}

#[derive(Clone, Copy)]
Expand Down Expand Up @@ -111,7 +121,6 @@ impl Rm {
Exec,
Done(ExitCode),
Err(ExitCode),
WaitErr,
}
let tag = match &Self::state_mut(interp, cmd).state {
RmState::Idle => Tag::Idle,
Expand All @@ -122,10 +131,8 @@ impl Rm {
RmState::Exec(_) => Tag::Exec,
RmState::Done { exit_code } => Tag::Done(*exit_code),
RmState::Err(c) => Tag::Err(*c),
RmState::WaitingWriteErr => Tag::WaitErr,
};
match tag {
Tag::WaitErr => return Yield::suspended(),
Tag::Idle => {
Self::state_mut(interp, cmd).state = RmState::ParseOpts {
idx: 0,
Expand Down Expand Up @@ -171,87 +178,10 @@ impl Rm {
}

let args_start = idx as usize;

// Check that none of the paths will delete the root.
{
let mut buf = bun_paths::PathBuffer::uninit();
let cwd = match bun_sys::getcwd_z(&mut buf) {
Ok(c) => c.as_bytes().to_vec(),
Err(err) => {
let msg =
err.msg().map(bstr::BStr::new).unwrap_or_else(|| {
bstr::BStr::new(b"failed to get cwd")
});
let buf = Builtin::fmt_error_arena(
interp,
cmd,
Some(Kind::Rm),
format_args!("getcwd: {}", msg),
)
.to_vec();
return Self::write_failing_error(interp, cmd, &buf, 1);
}
};

for i in args_start..argc {
let path = Builtin::of(interp, cmd).arg_bytes(i);
let resolved: &[u8] = if Platform::AUTO.is_absolute(path) {
path
} else {
resolve_path::join::<platform::Auto>(&[&cwd, path])
};
let normalized = resolve_path::normalize_string::<
false,
platform::Auto,
>(resolved);
let dirname =
resolve_path::dirname::<platform::Auto>(normalized);
if dirname.is_empty() {
// Copy resolved before
// re-borrowing `interp` mutably.
let resolved_owned = resolved.to_vec();
if let Some(safeguard) =
Builtin::of(interp, cmd).stderr.needs_io()
{
Self::state_mut(interp, cmd).state =
RmState::ParseOpts {
idx,
wait_write_err: true,
};
let child = ChildPtr::new(cmd, WriterTag::Builtin);
return Builtin::of_mut(interp, cmd)
.stderr
.enqueue_fmt(
child,
Some(Kind::Rm),
format_args!(
"\"{}\" may not be removed\n",
bstr::BStr::new(&resolved_owned)
),
safeguard,
);
}
let buf = Builtin::fmt_error_arena(
interp,
cmd,
Some(Kind::Rm),
format_args!(
"\"{}\" may not be removed\n",
bstr::BStr::new(&resolved_owned)
),
)
.to_vec();
let _ =
Builtin::write_no_io(interp, cmd, IoKind::Stderr, &buf);
return Builtin::done(interp, cmd, 1);
}
}
}

let total_tasks = argc - args_start;
Self::state_mut(interp, cmd).state = RmState::Exec(ExecState {
args_start,
total_tasks,
total_tasks: argc - args_start,
had_refusal: false,
err: None,
error_signal: AtomicBool::new(false),
output_done: AtomicUsize::new(0),
Expand Down Expand Up @@ -304,7 +234,8 @@ impl Rm {
_ => unreachable!(),
};
if !started {
let cwd = Builtin::cwd(interp, cmd);
let cwd_fd = Builtin::cwd(interp, cmd);
let cwd_path = Builtin::shell(interp, cmd).cwd().to_vec();
let evtloop = Builtin::event_loop(interp, cmd);
let opts = Self::state_mut(interp, cmd).opts;
let interp_ptr: *mut Interpreter = interp.as_ctx_ptr();
Expand All @@ -316,21 +247,84 @@ impl Rm {
e.started = true;
(e.args_start, e.args_start + e.total_tasks)
};

let mut refused_msg = Vec::<u8>::new();
let mut args_to_run = Vec::<usize>::with_capacity(argc - args_start);
for i in args_start..argc {
use std::io::Write as _;
let path = Builtin::of(interp, cmd).arg_bytes(i);
let base = resolve_path::basename(path);
if base == b"." || base == b".." {
let _ = writeln!(
&mut refused_msg,
"rm: refusing to remove '.' or '..' directory: skipping '{}'",
bstr::BStr::new(path),
);
continue;
}
Comment thread
robobun marked this conversation as resolved.
let resolved: &[u8] = if Platform::AUTO.is_absolute(path) {
path
} else {
resolve_path::join::<platform::Auto>(&[&cwd_path, path])
};
let normalized =
resolve_path::normalize_string::<false, platform::Auto>(resolved);
let dirname = resolve_path::dirname::<platform::Auto>(normalized);
if dirname.is_empty() {
let _ = writeln!(
&mut refused_msg,
"rm: \"{}\" may not be removed",
bstr::BStr::new(resolved),
);
continue;
}
args_to_run.push(i);
Comment thread
robobun marked this conversation as resolved.
}

let total_tasks = args_to_run.len();
let had_refusal = !refused_msg.is_empty();
{
let RmState::Exec(e) = &mut Self::state_mut(interp, cmd).state else {
unreachable!()
};
e.total_tasks = total_tasks;
e.had_refusal = had_refusal;
}
let (sig, out_count) = match &Self::state_mut(interp, cmd).state {
RmState::Exec(e) => (
bun_ptr::BackRef::new(&e.error_signal),
bun_ptr::BackRef::new(&e.output_count),
),
_ => unreachable!(),
};
for i in args_start..argc {
for i in args_to_run {
let root = Builtin::of(interp, cmd).arg_bytes(i);
let task = ShellRmTask::create(
cmd, opts, root, cwd, sig, out_count, evtloop, interp_ptr,
cmd, opts, root, cwd_fd, sig, out_count, evtloop, interp_ptr,
);
// SAFETY: freshly heap-allocated.
unsafe { ShellRmTask::schedule(task) };
}
if !refused_msg.is_empty() {
if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() {
if let RmState::Exec(e) = &mut Self::state_mut(interp, cmd).state {
e.output_count.fetch_add(1, Ordering::SeqCst);
}
let child = ChildPtr::new(cmd, WriterTag::Builtin);
return Builtin::of_mut(interp, cmd).stderr.enqueue(
child,
&refused_msg,
safeguard,
);
}
let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, &refused_msg);
}
if total_tasks == 0 {
Self::state_mut(interp, cmd).state = RmState::Done {
exit_code: if had_refusal { 1 } else { 0 },
};
continue;
}
}
return Yield::suspended();
}
Expand All @@ -355,23 +349,6 @@ impl Rm {
Builtin::done(interp, cmd, 1)
}

fn write_failing_error(
interp: &Interpreter,
cmd: NodeId,
buf: &[u8],
exit_code: ExitCode,
) -> Yield {
if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() {
Self::state_mut(interp, cmd).state = RmState::WaitingWriteErr;
let child = ChildPtr::new(cmd, WriterTag::Builtin);
return Builtin::of_mut(interp, cmd)
.stderr
.enqueue(child, buf, safeguard);
}
let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, buf);
Builtin::done(interp, cmd, exit_code)
}

pub(crate) fn on_io_writer_chunk(
interp: &Interpreter,
cmd: NodeId,
Expand All @@ -382,7 +359,7 @@ impl Rm {
RmState::Exec(exec) => {
exec.output_done.fetch_add(1, Ordering::SeqCst);
if exec.tasks_done >= exec.total_tasks && exec.output_drained() {
Some(if exec.err.is_some() { 1 } else { 0 })
Some(exec.exit_code())
} else {
None
}
Expand Down Expand Up @@ -456,13 +433,7 @@ impl Rm {
};
if tasks_done >= total && all_out {
let code = match &Self::state_mut(interp, cmd).state {
RmState::Exec(exec) => {
if exec.err.is_some() {
1
} else {
0
}
}
RmState::Exec(exec) => exec.exit_code(),
_ => 0,
};
Self::state_mut(interp, cmd).state = RmState::Done { exit_code: code };
Expand Down Expand Up @@ -511,13 +482,7 @@ impl Rm {
};
if done {
let code = match &Self::state_mut(interp, cmd).state {
RmState::Exec(exec) => {
if exec.err.is_some() {
1
} else {
0
}
}
RmState::Exec(exec) => exec.exit_code(),
_ => 0,
};
return Builtin::done(interp, cmd, code);
Expand Down
Loading
Loading