improve patch

This commit is contained in:
curious-rabbit committed 2026-04-20 14:28:18 +02:00
1 parent 92c54d37c0
commit 63a7d85d26
7 files changed
+73 -159

No files matched your search

+39 -4
View File
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::cell::OnceCell;
use std::ffi::OsString;
use std::fs::{FileType, Metadata};
@@ -54,18 +55,26 @@ impl DirEntry {
}
/// Returns the path as it should be presented to the user.
pub fn stripped_path(&self, config: &Config) -> &Path {
/// When stripping `./` would leave the path starting with `-`, keep the `./` so
/// downstream tools don't interpret the filename as an option.
pub fn stripped_path(&self, config: &Config) -> Cow<'_, Path> {
let path = self.path();
if config.strip_cwd_prefix {
strip_current_dir(self.path())
let stripped = strip_current_dir(path);
if starts_with_dash(stripped) {
Cow::Owned(Path::new(".").join(stripped))
} else {
Cow::Borrowed(stripped)
}
} else {
self.path()
Cow::Borrowed(path)
}
}
/// Returns the path as it should be presented to the user.
pub fn into_stripped_path(self, config: &Config) -> PathBuf {
if config.strip_cwd_prefix {
self.stripped_path(config).to_path_buf()
self.stripped_path(config).into_owned()
} else {
self.into_path()
}
@@ -101,6 +110,10 @@ impl DirEntry {
}
}
fn starts_with_dash(path: &Path) -> bool {
path.as_os_str().as_encoded_bytes().first() == Some(&b'-')
}
impl PartialEq for DirEntry {
#[inline]
fn eq(&self, other: &Self) -> bool {
@@ -153,3 +166,25 @@ impl Colorable for DirEntry {
self.metadata().cloned()
}
}
#[cfg(test)]
mod tests {
use super::starts_with_dash;
use std::path::Path;
#[test]
fn dash_prefixed_paths_detected() {
assert!(starts_with_dash(Path::new("-rf")));
assert!(starts_with_dash(Path::new("--delete")));
assert!(starts_with_dash(Path::new("-")));
}
#[test]
fn safe_paths_not_flagged() {
assert!(!starts_with_dash(Path::new("foo")));
assert!(!starts_with_dash(Path::new("./foo")));
assert!(!starts_with_dash(Path::new("sub/-rf")));
assert!(!starts_with_dash(Path::new("")));
assert!(!starts_with_dash(Path::new(" -rf")));
}
}
+1 -1
View File
@@ -32,7 +32,7 @@ pub fn job(
// Generate a command, execute it and store its exit code.
let code = cmd.execute(
dir_entry.stripped_path(config),
&dir_entry.stripped_path(config),
config.path_separator.as_deref(),
config.null_separator,
buffer_output,
+3 -5
View File
@@ -175,7 +175,7 @@ impl CommandBuilder {
self.finish()?;
}
let arg = self.path_arg.generate_for_exec(path, separator);
let arg = self.path_arg.generate(path, separator);
if !self
.cmd
.args_would_fit(iter::once(&arg).chain(&self.post_args))
@@ -242,8 +242,6 @@ impl CommandTemplate {
bail!("No executable provided for --exec or --exec-batch");
}
// The first argument is the command to execute and must be a fixed string;
// allowing a placeholder here would turn every matched file into an execve target.
if args[0].has_tokens() {
bail!("First argument of --exec/--exec-batch must be a fixed executable, not a placeholder");
}
@@ -265,9 +263,9 @@ impl CommandTemplate {
/// Using the internal `args` field, and a supplied `input` variable, a `Command` will be
/// build.
fn generate(&self, input: &Path, path_separator: Option<&str>) -> io::Result<Command> {
let mut cmd = Command::new(self.args[0].generate_for_exec(input, path_separator));
let mut cmd = Command::new(self.args[0].generate(input, path_separator));
for arg in &self.args[1..] {
cmd.try_arg(arg.generate_for_exec(input, path_separator))?;
cmd.try_arg(arg.generate(input, path_separator))?;
}
Ok(cmd)
}
+1 -124
View File
@@ -24,19 +24,6 @@ pub enum Token {
Text(String),
}
impl Token {
fn is_path_derived(&self) -> bool {
matches!(
self,
Token::Placeholder
| Token::Basename
| Token::Parent
| Token::NoExt
| Token::BasenameNoExt
)
}
}
impl Display for Token {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
match *self {
@@ -123,26 +110,8 @@ impl FormatTemplate {
/// the path separator in all placeholder tokens. Fixed text and tokens are not affected by
/// path separator substitution.
pub fn generate(&self, path: impl AsRef<Path>, path_separator: Option<&str>) -> OsString {
self.generate_impl(path.as_ref(), path_separator, false)
}
/// Like `generate`, but prepends `./` when a path-derived token produces a leading `-`,
/// so the target of `--exec` does not see it as an option.
pub fn generate_for_exec(
&self,
path: impl AsRef<Path>,
path_separator: Option<&str>,
) -> OsString {
self.generate_impl(path.as_ref(), path_separator, true)
}
fn generate_impl(
&self,
path: &Path,
path_separator: Option<&str>,
protect_leading_dash: bool,
) -> OsString {
use Token::*;
let path = path.as_ref();
match *self {
Self::Tokens(ref tokens) => {
@@ -165,16 +134,6 @@ impl FormatTemplate {
Text(string) => s.push(string),
}
}
if protect_leading_dash
&& tokens.first().is_some_and(|t| t.is_path_derived())
&& s.as_encoded_bytes().first() == Some(&b'-')
{
let mut prefixed = OsString::from("./");
prefixed.push(s);
return prefixed;
}
s
}
Self::Text(ref text) => OsString::from(text),
@@ -319,86 +278,4 @@ mod fmt_tests {
basenameNoExt=file }"
);
}
fn tmpl(s: &str) -> FormatTemplate {
FormatTemplate::parse(s)
}
#[test]
fn exec_basename_with_leading_dash_is_guarded() {
let out = tmpl("{/}")
.generate_for_exec(Path::new("./some/dir/-rf"), None)
.into_string()
.unwrap();
assert_eq!(out, "./-rf");
}
#[test]
fn exec_basename_no_ext_with_leading_dash_is_guarded() {
let out = tmpl("{/.}")
.generate_for_exec(Path::new("./some/dir/-evil.txt"), None)
.into_string()
.unwrap();
assert_eq!(out, "./-evil");
}
#[test]
fn exec_parent_with_leading_dash_is_guarded() {
let out = tmpl("{//}")
.generate_for_exec(Path::new("-startdir/inner/file.txt"), None)
.into_string()
.unwrap();
assert_eq!(out, "./-startdir/inner");
}
#[test]
fn exec_plain_placeholder_already_safe() {
let out = tmpl("{}")
.generate_for_exec(Path::new("./some/dir/-rf"), None)
.into_string()
.unwrap();
assert_eq!(out, "./some/dir/-rf");
}
#[test]
fn exec_user_literal_dash_prefix_not_rewritten() {
let out = tmpl("-{/}")
.generate_for_exec(Path::new("./some/dir/normal.txt"), None)
.into_string()
.unwrap();
assert_eq!(out, "-normal.txt");
}
#[test]
fn exec_user_literal_dash_before_dash_basename() {
let out = tmpl("-{/}")
.generate_for_exec(Path::new("./some/dir/-name"), None)
.into_string()
.unwrap();
assert_eq!(out, "--name");
}
#[test]
fn exec_suffix_after_placeholder_preserves_guard() {
let out = tmpl("{/}.bak")
.generate_for_exec(Path::new("./some/dir/-foo"), None)
.into_string()
.unwrap();
assert_eq!(out, "./-foo.bak");
}
#[test]
fn format_mode_does_not_protect_leading_dash() {
let out = tmpl("{/}")
.generate(Path::new("./some/dir/-rf"), None)
.into_string()
.unwrap();
assert_eq!(out, "-rf");
}
#[test]
fn exec_empty_result_not_prefixed() {
let out = tmpl("{/}").generate_for_exec(Path::new(""), None);
assert!(out.is_empty());
}
}
+1 -7
View File
@@ -66,13 +66,7 @@ fn main() {
exit_code.exit();
}
Err(err) => {
// Gap fix: anyhow errors bubbled from run() may embed user paths/args.
let formatted = format!("{err:#}");
if std::io::stderr().is_terminal() {
eprintln!("[fd error]: {}", sanitize::sanitize_for_terminal(&formatted));
} else {
eprintln!("[fd error]: {formatted}");
}
crate::error::print_error(format!("{err:#}"));
ExitCode::GeneralError.exit();
}
}
+27 -17
View File
@@ -1,24 +1,29 @@
//! TTY-output sanitization to prevent terminal escape injection via filenames.
use std::borrow::Cow;
use std::fmt::Write;
#[inline]
fn is_dangerous_control(c: char) -> bool {
// C0 (except HT), DEL, and C1 controls (gap fix: U+0080..=U+009F can act
// as single-byte CSI/OSC initiators on 8-bit-control terminals).
// C0 (except HT), DEL, and C1 controls (U+0080..=U+009F can act as
// single-byte CSI/OSC initiators on 8-bit-control terminals).
matches!(c, '\x00'..='\x08' | '\x0A'..='\x1F' | '\x7F' | '\u{80}'..='\u{9F}')
}
/// Replace control characters with `?` (matches `ls -q`). Borrows on the fast path.
/// Replace control characters with `\xNN` so the original filename remains recoverable.
pub fn sanitize_for_terminal(s: &str) -> Cow<'_, str> {
if !s.chars().any(is_dangerous_control) {
return Cow::Borrowed(s);
}
Cow::Owned(
s.chars()
.map(|c| if is_dangerous_control(c) { '?' } else { c })
.collect(),
)
let mut out = String::with_capacity(s.len());
for c in s.chars() {
if is_dangerous_control(c) {
let _ = write!(out, "\\x{:02X}", c as u32);
} else {
out.push(c);
}
}
Cow::Owned(out)
}
#[cfg(test)]
@@ -27,7 +32,6 @@ mod tests {
#[test]
fn preserves_safe_content() {
// Plain text, unicode, tab, and existing U+FFFD all pass through without allocation.
for s in ["hello.txt", "résumé.pdf", "文档.txt", "🦀.rs", "a\tb", "a\u{FFFD}b"] {
assert!(matches!(sanitize_for_terminal(s), Cow::Borrowed(_)), "{s:?}");
assert_eq!(sanitize_for_terminal(s), s);
@@ -39,12 +43,12 @@ mod tests {
let attack = "innocent\x1b]52;c;cHduZWQ=\x1b\\.txt";
let safe = sanitize_for_terminal(attack);
assert!(!safe.contains('\x1b'));
assert_eq!(safe, "innocent?]52;c;cHduZWQ=?\\.txt");
assert_eq!(safe, "innocent\\x1B]52;c;cHduZWQ=\\x1B\\.txt");
}
#[test]
fn strips_cr_output_forgery() {
assert_eq!(sanitize_for_terminal("A\rFAKE OUTPUT"), "A?FAKE OUTPUT");
assert_eq!(sanitize_for_terminal("A\rFAKE OUTPUT"), "A\\x0DFAKE OUTPUT");
}
#[test]
@@ -55,24 +59,30 @@ mod tests {
#[test]
fn strips_del() {
assert_eq!(sanitize_for_terminal("a\x7fb"), "a?b");
assert_eq!(sanitize_for_terminal("a\x7fb"), "a\\x7Fb");
}
#[test]
fn strips_bel_and_null() {
assert_eq!(sanitize_for_terminal("a\x07b"), "a?b");
assert_eq!(sanitize_for_terminal("a\0b"), "a?b");
assert_eq!(sanitize_for_terminal("a\x07b"), "a\\x07b");
assert_eq!(sanitize_for_terminal("a\0b"), "a\\x00b");
}
#[test]
fn strips_newline() {
assert_eq!(sanitize_for_terminal("a\nb"), "a?b");
assert_eq!(sanitize_for_terminal("a\nb"), "a\\x0Ab");
}
#[test]
fn escape_preserves_information() {
let s = "name\x1bX\x07Y.txt";
assert_eq!(sanitize_for_terminal(s), "name\\x1BX\\x07Y.txt");
}
#[test]
fn strips_c1_csi_and_osc_initiators() {
// U+009B is CSI, U+009D is OSC; dangerous on 8-bit-control terminals.
assert_eq!(sanitize_for_terminal("\u{9b}31m"), "?31m");
assert_eq!(sanitize_for_terminal("\u{9d}0;pwned\u{9c}"), "?0;pwned?");
assert_eq!(sanitize_for_terminal("\u{9b}31m"), "\\x9B31m");
assert_eq!(sanitize_for_terminal("\u{9d}0;pwned\u{9c}"), "\\x9D0;pwned\\x9C");
}
}
+1 -1
View File
@@ -1906,7 +1906,7 @@ fn test_exec_batch() {
te.assert_failure_with_error(
&["foo", "--exec-batch", "echo {}"],
"error: First argument of exec-batch is expected to be a fixed executable\n\
"error: First argument of --exec/--exec-batch must be a fixed executable, not a placeholder\n\
\n\
Usage: fd [OPTIONS] [pattern] [path]...\n\
\n\