diff --git a/src/tools/bash.rs b/src/tools/bash.rs index ef628a73..e7dc35cc 100644 --- a/src/tools/bash.rs +++ b/src/tools/bash.rs @@ -6,7 +6,7 @@ use anyhow::{Context, bail}; use crate::bounded_process::{self, BoundedProcessOutcomeKind}; -mod path_tokens; +pub(crate) mod path_tokens; mod read_guard; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(180); @@ -815,6 +815,17 @@ pub fn path_confinement_rejection( } } } + // Issue #613 design 2: a `/`-less static word can be an outward symlink that + // no directory separator exposes. It is proven when joined onto the + // workspace root and each working-directory candidate, but only when the + // joined path actually is a symlink (the read guard holds that body). An + // undetermined working directory is left to the relative-candidate branch + // below, which already refuses the words a `cd` could redirect. + if let Some(word) = + read_guard::relative_word_rejection(&read.relative_words, &root, &inspection.bases) + { + return Some(path_reference_rejection(&word, &root, None)); + } if inspection.has_working_directory() { if inspection.undecidable && !inspection.relative_words.is_empty() { return Some(path_reference_rejection( diff --git a/src/tools/bash/path_tokens.rs b/src/tools/bash/path_tokens.rs index f2173428..15b7a1eb 100644 --- a/src/tools/bash/path_tokens.rs +++ b/src/tools/bash/path_tokens.rs @@ -9,6 +9,21 @@ //! the existing write-side proof, and a word whose value cannot be determined is //! refused. This module never returns a *result*; it only classifies. +/// Whether a carriage return separates words or is an ordinary word character +/// (Issue #613). Bash reads a space, a tab, and a newline as word separators and +/// keeps a carriage return inside the word, so the same text has two readings: +/// the historical one that treats `\r` as a separator and might miss an outward +/// symlink whose name holds a `\r`, and the shell-faithful one that keeps it. +/// The guard judges a command with both readings and refuses or recognizes when +/// either does. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum CrMode { + /// A carriage return separates words (the historical reading). + Separator, + /// A carriage return is an ordinary word character, as Bash reads it. + WordChar, +} + /// One shell word with the metadata the read-path judge needs. #[derive(Debug, Clone, PartialEq, Eq, Default)] pub(super) struct Word { @@ -49,7 +64,7 @@ impl Word { /// Split a command into segments of words, tracking shell expansions without /// running them. Returns `None` when the quoting cannot be read (an unterminated /// quote), so the caller fails closed. -pub(super) fn read_words(command: &str) -> Option>> { +pub(super) fn read_words(command: &str, cr_mode: CrMode) -> Option>> { let chars: Vec = command.chars().collect(); let mut segments: Vec> = Vec::new(); let mut words: Vec = Vec::new(); @@ -153,7 +168,9 @@ pub(super) fn read_words(command: &str) -> Option>> { has_word = true; index += 1; } - ';' | '|' | '&' | '<' | '>' | '(' | ')' | '\n' | '\r' => { + ';' | '|' | '&' | '<' | '>' | '(' | ')' | '\n' | '\r' + if cr_mode == CrMode::Separator || ch != '\r' => + { flush(&mut words, &mut word, &mut has_word); if matches!(ch, ';' | '|' | '&') && chars.get(index + 1) == Some(&ch) { index += 1; @@ -162,10 +179,11 @@ pub(super) fn read_words(command: &str) -> Option>> { index += 1; } // Only the shell's default word separators end a word: a space and a - // tab (the newline and carriage return are handled above). Every - // other Unicode whitespace — an ideographic space, a no-break space — - // is an ordinary character of the word, which is what the shell - // reads (Issue #603 design 2). + // tab (the newline is handled above; a carriage return is a separator + // only in [`CrMode::Separator`] and reaches the `other` arm in + // [`CrMode::WordChar`]). Every other Unicode whitespace — an + // ideographic space, a no-break space — is an ordinary character of + // the word, which is what the shell reads (Issue #603 design 2). ' ' | '\t' => { flush(&mut words, &mut word, &mut has_word); index += 1; @@ -396,6 +414,11 @@ pub(super) fn contains_glob_or_brace(value: &str) -> bool { mod tests { use super::*; + /// The historical reading, where a carriage return separates words. + fn read_words(command: &str) -> Option>> { + super::read_words(command, CrMode::Separator) + } + fn words(command: &str) -> Vec { read_words(command) .unwrap_or_else(|| panic!("expected readable words: {command}")) @@ -522,6 +545,25 @@ mod tests { assert_eq!(words("cat a\nb"), ["cat", "a", "b"]); } + /// Issue #613: Bash keeps a carriage return inside the word. The + /// `WordChar` reading reproduces that, while `Separator` keeps the historical + /// split. A newline still ends the word in both readings. + #[test] + fn carriage_return_word_reading_table() { + let word_char = |command: &str| { + super::read_words(command, CrMode::WordChar) + .unwrap_or_else(|| panic!("expected readable words: {command}")) + .into_iter() + .flatten() + .map(|word| word.text) + .collect::>() + }; + assert_eq!(word_char("cat lfcr\r"), ["cat", "lfcr\r"]); + assert_eq!(word_char("cat a\rb"), ["cat", "a\rb"]); + assert_eq!(word_char("cat a\r\nb"), ["cat", "a\r", "b"]); + assert_eq!(words("cat a\rb"), ["cat", "a", "b"]); + } + /// The whitespace-allowing form accepts a quoted or escaped single word and a /// `=` right-hand side, but still refuses a multiword program, an option, and /// an assignment (Issue #603 designs 1 and 3). diff --git a/src/tools/bash/read_guard.rs b/src/tools/bash/read_guard.rs index ac15c618..672a2f14 100644 --- a/src/tools/bash/read_guard.rs +++ b/src/tools/bash/read_guard.rs @@ -21,7 +21,9 @@ //! or runs a substitution to resolve a word, and it is never used to prove a //! later variable reference safe. -use super::path_tokens::{self, Word}; +use std::path::{Path, PathBuf}; + +use super::path_tokens::{self, CrMode, Word}; /// The reason recorded for a read word whose expanded value cannot be proven to /// remain in the workspace. @@ -59,6 +61,9 @@ pub(super) struct ReadVerdict { /// Path spellings to confine with the write-target proof and the /// working-directory candidate union. pub candidates: Vec, + /// `/`-less static words the caller re-checks against the working-directory + /// candidates when no directory separator names them (Issue #613 design 2). + pub relative_words: Vec, } /// Reads the read-path judgement for a command the write guard allowed. @@ -67,10 +72,24 @@ pub(super) fn inspect(command: &str) -> ReadVerdict { // has readable text, so the raw fallback is only a belt-and-braces guard. let elided = super::super::shell_lexical::strip_comments_and_heredocs(command); let text = elided.as_deref().unwrap_or(command); - let Some(segments) = path_tokens::read_words(text) else { + let mut verdict = inspect_text(text, CrMode::Separator); + // Issue #613: Bash keeps a carriage return inside the word. Read the text + // that way too and union the verdicts, so an outward path whose spelling + // holds a `\r` is refused when either reading sees it. The two readings + // coincide when the text has no `\r`. + if text.contains('\r') { + merge(&mut verdict, inspect_text(text, CrMode::WordChar)); + } + verdict +} + +/// The read-path judgement of one carriage-return reading of the elided text. +fn inspect_text(text: &str, cr_mode: CrMode) -> ReadVerdict { + let Some(segments) = path_tokens::read_words(text, cr_mode) else { return ReadVerdict { unverifiable: Some("".to_string()), candidates: Vec::new(), + relative_words: Vec::new(), }; }; let glob_shell_option = uses_glob_shell_option(text); @@ -114,11 +133,113 @@ pub(super) fn inspect(command: &str) -> ReadVerdict { continue; } push_static(&mut verdict.candidates, word, index >= leading); + collect_relative_targets(&mut verdict.relative_words, word, index >= leading); } } verdict } +/// Unions another carriage-return reading into `into`. An unverifiable word in +/// either reading is kept, and the candidates and `/`-less words are combined, +/// so the union never admits more than one reading alone. +fn merge(into: &mut ReadVerdict, other: ReadVerdict) { + if into.unverifiable.is_none() { + into.unverifiable = other.unverifiable; + } + into.candidates.extend(other.candidates); + into.relative_words.extend(other.relative_words); +} + +/// Records the `/`-less static words the caller re-checks against the +/// working-directory candidates (Issue #613 design 2). A leading assignment +/// (`NAME=value`) is a variable, not a path; `.`, `..`, an empty word, and a +/// `~`-prefixed word are excluded. The command name and options are not +/// excluded, matching the existing `cd`-relative word check; a word that is not +/// an actual outward symlink costs only one `lstat` and stays allowed. +fn collect_relative_targets(targets: &mut Vec, word: &Word, is_value_word: bool) { + if !is_value_word { + return; + } + if is_relative_target(&word.text) { + targets.push(word.text.clone()); + } + // The right-hand side of a non-assignment `NAME=value` word names a read + // path (`dd if=lf`, `grep --file=lf`, `make IN=lf`) even without a `/`. + if let Some(value) = relative_equals_right_hand_side(&word.text) { + targets.push(value.to_string()); + } +} + +/// Whether a static word can name a `/`-less relative path the caller re-checks. +fn is_relative_target(word: &str) -> bool { + !word.is_empty() && word != "." && word != ".." && !word.starts_with('~') && !word.contains('/') +} + +/// The right-hand side of a `NAME=value` word when it is a non-empty, `/`-less +/// spelling. An empty or `/`-containing right side is left to the existing +/// handling. +fn relative_equals_right_hand_side(word: &str) -> Option<&str> { + let (_, value) = word.split_once('=')?; + (!value.is_empty() && !value.contains('/')).then_some(value) +} + +/// The first `/`-less static read word that escapes the workspace when joined +/// onto a working-directory candidate (Issue #613 design 2). The literal read +/// proof runs only when the joined path is an existing symlink (or cannot be +/// `lstat`ed for a reason other than a missing name or a non-directory parent), +/// so a word that is not a symlink — an option, a display string, or an +/// `awk -F= '{print $1}'`-style quoted fragment — keeps its existing handling. +/// +/// A working directory that cannot be determined is left to the existing +/// relative-candidate logic in `bash.rs`: that path already refuses the words a +/// `cd` could redirect, while this check deliberately walks only the workspace +/// root and the destinations that *can* be determined. Rejecting every +/// `/`-less word of an undecidable walk here would refuse a bare `cd` chain that +/// names no path (Issue #568 `working_directory_caps_candidate_growth`). +pub(super) fn relative_word_rejection( + words: &[String], + root: &Path, + bases: &[String], +) -> Option { + for word in words { + for base in bases { + let joined = join_base(base, word); + if !joined_is_symlink(root, &joined) { + continue; + } + if super::super::path_guard::ensure_bash_read_target(root, &joined).is_err() { + return Some(word.clone()); + } + } + } + None +} + +/// Whether the joined path is an existing symlink, or failed to `lstat` for a +/// reason other than a missing name or a non-directory ancestor. +fn joined_is_symlink(root: &Path, joined: &str) -> bool { + let candidate = if Path::new(joined).is_absolute() { + PathBuf::from(joined) + } else { + root.join(joined) + }; + match std::fs::symlink_metadata(&candidate) { + Ok(metadata) => metadata.file_type().is_symlink(), + Err(error) => { + error.kind() != std::io::ErrorKind::NotFound + && error.raw_os_error() != Some(libc::ENOTDIR) + } + } +} + +fn join_base(base: &str, path: &str) -> String { + if base.is_empty() { + path.to_string() + } else { + format!("{base}/{path}") + } +} + fn push_static(candidates: &mut Vec, word: &Word, equals_rhs_is_candidate: bool) { let mut produced = Vec::new(); if word.text.starts_with('/') { @@ -256,6 +377,10 @@ mod tests { "frontend", "dir1", "dir2", + // Issue #613: an inside directory whose name holds a carriage return. + // Only the WordChar reading reaches it, and a nested outward symlink + // below it is invisible to every other candidate. + "cr\rsub", ] { std::fs::create_dir_all(root.join(directory)).unwrap(); } @@ -283,6 +408,20 @@ mod tests { std::os::unix::fs::symlink(dir.path().join("missing-target"), root.join("dangling")) .unwrap(); std::os::unix::fs::symlink(root.join("sub"), root.join("sub/loop")).unwrap(); + std::os::unix::fs::symlink(root.join("sub"), root.join("inlink")).unwrap(); + // Issue #613 problem 1: outward symlinks whose name holds a carriage + // return. Bash keeps the `\r` inside the word; the historical reading + // splits on it and misses the symlink. + std::os::unix::fs::symlink(&outside, root.join("sub/cr\rlink")).unwrap(); + std::os::unix::fs::symlink(&outside, root.join("cr\rdir")).unwrap(); + std::os::unix::fs::symlink(outside.join("secret"), root.join("lfcr\r")).unwrap(); + // Issue #613 problem 2: a `/`-less name can itself be an outward symlink. + std::os::unix::fs::symlink(&outside, root.join("lf")).unwrap(); + std::os::unix::fs::symlink(&outside, root.join("space lf")).unwrap(); + std::os::unix::fs::symlink(&outside, root.join("wide\u{3000}lf")).unwrap(); + // Issue #613: an outward symlink inside the CR-named directory above, so + // only the merged WordChar candidate reaches it (`cd cr␍sub && cat esc`). + std::os::unix::fs::symlink(&outside, root.join("cr\rsub/esc")).unwrap(); let root = root.canonicalize().unwrap(); Fixture { _dir: dir, root } } @@ -335,9 +474,41 @@ mod tests { assert!(seen >= 20, "the decision table is too small: {seen}"); } + /// Issue #613 design 2 "存在しない・ディレクトリでない以外の理由で lstat に + /// 失敗したら、確かめる": an `lstat` that fails for any reason other than a + /// missing name or a non-directory ancestor still needs the literal proof. A + /// component longer than `NAME_MAX` fails with `ENAMETOOLONG`, and narrowing + /// the check to `NotFound` would drop that path. + #[test] + fn joined_is_symlink_treats_other_lstat_failures_as_needing_proof() { + let fixture = fixture(); + let root = &fixture.root; + let over_long = "a".repeat(300); + assert!( + super::joined_is_symlink(root, &over_long), + "an over-long name (ENAMETOOLONG) must reach the literal proof" + ); + assert!( + !super::joined_is_symlink(root, "no-such-name-f613"), + "a missing name is not a symlink" + ); + assert!( + !super::joined_is_symlink(root, "a.txt"), + "a regular file is not a symlink" + ); + assert!( + super::joined_is_symlink(root, "lf"), + "an existing symlink needs the proof" + ); + } + #[test] fn read_candidates_keeps_static_word_metadata() { - let parsed = super::path_tokens::read_words("echo $HOME && head sub/link/secret").unwrap(); + let parsed = super::path_tokens::read_words( + "echo $HOME && head sub/link/secret", + super::CrMode::Separator, + ) + .unwrap(); // The dynamic word and the static relative word are both present: the // static word is not dropped because a sibling word is dynamic. let words = parsed.into_iter().flatten().collect::>(); @@ -387,7 +558,8 @@ mod tests { /// `/abs/x$?`) and can change the decision. This is a read-only check. #[test] fn read_candidates_keep_exit_status_in_the_word_text() { - let parsed = super::path_tokens::read_words("echo /abs/x$?").unwrap(); + let parsed = + super::path_tokens::read_words("echo /abs/x$?", super::CrMode::Separator).unwrap(); let word = &parsed[0][1]; assert_eq!(word.text, "/abs/x$?"); assert!(word.dynamic && word.simple_variable); diff --git a/src/tools/bash_write_guard.rs b/src/tools/bash_write_guard.rs index 8a54ee3c..33ed451a 100644 --- a/src/tools/bash_write_guard.rs +++ b/src/tools/bash_write_guard.rs @@ -1,5 +1,6 @@ use std::path::Path; +use super::bash::path_tokens::CrMode; use super::shell_lexical; mod ansi_c_quoting; @@ -324,7 +325,22 @@ fn write_targets(command: &str) -> Vec { let Some(view) = view else { return vec![unreadable_target()]; }; - let Some(lexed) = lex_shell(&view.text) else { + let mut targets = targets_for_view(&view, CrMode::Separator); + // #613: a carriage return is an ordinary word character in Bash. Read the + // text that way too and add whatever it names, so an outward intermediary + // symlink whose name holds a `\r` is still proven. When the text has no `\r` + // the two readings coincide and the second pass is skipped. + if view.text.contains('\r') { + targets.extend(targets_for_view(&view, CrMode::WordChar)); + } + targets +} + +/// The write targets of one reading of the inspector's view. The caller unions +/// the historical reading with the carriage-return-as-word-character reading +/// (Issue #613). +fn targets_for_view(view: &shell_lexical::WriteGuardView, cr_mode: CrMode) -> Vec { + let Some(lexed) = lex_shell(&view.text, cr_mode) else { return vec![unreadable_target()]; }; // #581: keep the targets the text already names, then add the refusal marker @@ -351,7 +367,16 @@ fn raw_write_targets(command: &str) -> Vec { if ansi_c_quoting::outside_quotes(command).is_some() { return Vec::new(); } - let Some(lexed) = lex_shell(command) else { + let mut targets = raw_targets_for(command, CrMode::Separator); + if command.contains('\r') { + targets.extend(raw_targets_for(command, CrMode::WordChar)); + } + targets +} + +/// The raw write targets of one reading of the text (Issue #613). +fn raw_targets_for(command: &str, cr_mode: CrMode) -> Vec { + let Some(lexed) = lex_shell(command, cr_mode) else { return Vec::new(); }; let mut targets = targets_from_tokens(&lexed.tokens); @@ -625,14 +650,16 @@ fn is_environment_assignment(word: &str) -> bool { /// Splits a command into shell tokens. A thin wrapper over [`lex_shell`] for the /// callers that only need the token stream (the working-directory walk). -fn shell_tokens(command: &str) -> Option> { - lex_shell(command).map(|lexed| lexed.tokens) +fn shell_tokens(command: &str, cr_mode: CrMode) -> Option> { + lex_shell(command, cr_mode).map(|lexed| lexed.tokens) } /// Splits a command into shell tokens and, alongside them, the de-quoted texts /// of the word tokens that mix quoted and executed glob/brace characters -/// (Issue #602). -fn lex_shell(command: &str) -> Option { +/// (Issue #602). `cr_mode` selects whether a carriage return separates words +/// (the historical reading) or is an ordinary word character, as Bash reads it +/// (Issue #613). +fn lex_shell(command: &str, cr_mode: CrMode) -> Option { let mut tokens = Vec::new(); let mut ambiguous_words = Vec::new(); let mut current = ShellWord::default(); @@ -726,7 +753,9 @@ fn lex_shell(command: &str) -> Option { } tokens.push(ShellToken::InputRedirect); } - ';' | '|' | '&' | '(' | ')' | '\n' | '\r' => { + ';' | '|' | '&' | '(' | ')' | '\n' | '\r' + if cr_mode == CrMode::Separator || ch != '\r' => + { push_word( &mut tokens, &mut ambiguous_words, @@ -809,6 +838,16 @@ fn push_word( mod tests { use super::*; + /// The historical carriage-return reading, for the lexer-shape tests. + fn shell_tokens(command: &str) -> Option> { + super::shell_tokens(command, CrMode::Separator) + } + + /// The historical carriage-return reading, for the lexer-shape tests. + fn lex_shell(command: &str) -> Option { + super::lex_shell(command, CrMode::Separator) + } + /// A tempdir under the repository's `target` directory. `tempfile::tempdir()` /// lives under `/tmp` on Linux, which `is_system_prefix_allowed` accepts, so /// a fixture path outside the workspace could be admitted as a system path diff --git a/src/tools/bash_write_guard/working_directory.rs b/src/tools/bash_write_guard/working_directory.rs index 82091844..e1b73a04 100644 --- a/src/tools/bash_write_guard/working_directory.rs +++ b/src/tools/bash_write_guard/working_directory.rs @@ -23,6 +23,7 @@ use std::path::Path; use super::command_prefix::{self, Resolution}; use super::{ShellToken, command_words, positional_operands}; +use crate::tools::bash::path_tokens::CrMode; /// Upper bound on the working-directory candidates. Consecutive distinct `cd` /// destinations can double the set at each step, so past this bound the @@ -51,6 +52,32 @@ pub(crate) struct Inspection { } impl Inspection { + /// An inspection that has seen nothing yet: the workspace root candidate and + /// no `cd` destination. + fn empty() -> Self { + Inspection { + bases: vec![String::new()], + targets: Vec::new(), + undecidable: false, + relative_words: Vec::new(), + } + } + + /// Unions another reading of the same command (Issue #613). The candidates, + /// `cd` targets, and relative words are combined, and the undecidable mark + /// holds if either reading raised it, so the union is never looser than one + /// reading alone. + fn merge(&mut self, other: Inspection) { + for base in other.bases { + if !self.bases.contains(&base) { + self.bases.push(base); + } + } + self.targets.extend(other.targets); + self.relative_words.extend(other.relative_words); + self.undecidable |= other.undecidable; + } + /// Whether a `cd`/`pushd` added a candidate beyond the workspace root. pub(crate) fn has_working_directory(&self) -> bool { self.bases.iter().any(|base| !base.is_empty()) @@ -74,15 +101,10 @@ impl Inspection { /// Reads the working-directory candidates out of a command. pub(crate) fn inspect(command: &str) -> Inspection { - let mut inspection = Inspection { - bases: vec![String::new()], - targets: Vec::new(), - undecidable: false, - relative_words: Vec::new(), - }; let Some(view) = super::super::shell_lexical::write_guard_view(command) else { // The shell text cannot be read, so no working-directory change can be // modelled; relative writes after it must be refused (Issue #576). + let mut inspection = Inspection::empty(); inspection.undecidable = true; return inspection; }; @@ -91,8 +113,21 @@ pub(crate) fn inspect(command: &str) -> Inspection { // so the protected-path variants stay visible but relative writes are still // refused (Issue #585). let ambiguous = view.ambiguous_expansion; - let text = view.text; - let Some(tokens) = super::shell_tokens(&text) else { + // Issue #613: a carriage return is an ordinary word character in Bash. Read + // the text that way too and union the candidates, so a `cd` destination + // whose name holds a `\r` still adds the directory the shell changes into. + // The two readings coincide when the text has no `\r`. + let mut inspection = walk(&view.text, ambiguous, CrMode::Separator); + if view.text.contains('\r') { + inspection.merge(walk(&view.text, ambiguous, CrMode::WordChar)); + } + inspection +} + +/// The working-directory walk of one reading of the elided text (Issue #613). +fn walk(text: &str, ambiguous: bool, cr_mode: CrMode) -> Inspection { + let mut inspection = Inspection::empty(); + let Some(tokens) = super::shell_tokens(text, cr_mode) else { // The elided text of an ambiguous expansion can leave an unterminated // quote, so the walk cannot run. Keep the ambiguity signal anyway: the // design promise "an ambiguous expansion is undecidable" must hold on @@ -425,7 +460,7 @@ mod tests { .unwrap_or_else(|| panic!("expected a view: {command:?}")); assert!(view.ambiguous_expansion, "{command:?}"); assert!( - super::super::shell_tokens(&view.text).is_none(), + super::super::shell_tokens(&view.text, CrMode::Separator).is_none(), "the fixture must take the early-return path: {command:?}" ); assert!( @@ -434,4 +469,58 @@ mod tests { ); } } + + /// Issue #613: `inspect` unions the historical carriage-return reading with + /// the WordChar reading. `merge` must combine the candidates, the `cd` + /// targets, the relative words, and OR the undecidable mark — emptying it, + /// dropping the `!` in the base-dedup check, or turning the `|=` into `&=` + /// each lose a value the WordChar reading alone found. + #[test] + fn working_directory_merge_unions_both_readings() { + let mut merged = Inspection::empty(); + let mut word_char = Inspection::empty(); + word_char.bases.push("cr\rdir".to_string()); + word_char.targets.push("cr\rdir".to_string()); + word_char.relative_words.push("esc".to_string()); + word_char.undecidable = true; + merged.merge(word_char); + + assert_eq!(merged.bases, vec![String::new(), "cr\rdir".to_string()]); + assert_eq!(merged.targets, vec!["cr\rdir".to_string()]); + assert_eq!(merged.relative_words, vec!["esc".to_string()]); + assert!( + merged.undecidable, + "the undecidable mark of either reading must survive the union" + ); + + // A candidate already present is not added twice. + let mut duplicate = Inspection::empty(); + duplicate.bases.push("cr\rdir".to_string()); + merged.merge(duplicate); + assert_eq!(merged.bases, vec![String::new(), "cr\rdir".to_string()]); + } + + /// Issue #613: only the WordChar reading sees the `\r` in a `cd` + /// destination, so the merged candidate is what keeps it. A `/`-less operand + /// that is an outward symlink *inside* the CR-named directory is reached by + /// no other route, so dropping the union would allow it. + #[test] + fn working_directory_keeps_carriage_return_cd_candidates() { + let inspection = inspect("cd cr\rdir && cat esc"); + assert!( + inspection.bases.contains(&"cr\rdir".to_string()), + "the CR destination must be a candidate: {:?}", + inspection.bases + ); + assert!( + inspection.targets.contains(&"cr\rdir".to_string()), + "the CR destination must be confined: {:?}", + inspection.targets + ); + assert!( + inspection.relative_words.contains(&"esc".to_string()), + "the relative operand must stay visible: {:?}", + inspection.relative_words + ); + } } diff --git a/tests/corpus/apps/issue582-bash-read-candidates/fixtures/decision-cases.jsonl b/tests/corpus/apps/issue582-bash-read-candidates/fixtures/decision-cases.jsonl index 9cd6513f..6b124f68 100644 --- a/tests/corpus/apps/issue582-bash-read-candidates/fixtures/decision-cases.jsonl +++ b/tests/corpus/apps/issue582-bash-read-candidates/fixtures/decision-cases.jsonl @@ -49,3 +49,55 @@ {"id": "allow-awk-equals", "command": "awk -F= '{print $1}' a.txt", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} {"id": "allow-leading-assignment", "command": "RUST_LOG=debug cargo test", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} {"id": "allow-leading-assignment-path", "command": "OUT=out/x cargo test", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} +{"id": "cr-redirect-write", "command": "echo x > sub/cr\rlink/f", "R": true, "A": true, "W": true, "P": false, "S": false, "V": false} +{"id": "cr-static-read", "command": "cat sub/cr\rlink/secret", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "cr-cd-write", "command": "cd cr\rdir && echo x > f", "R": true, "A": true, "W": true, "P": false, "S": false, "V": false} +{"id": "cr-tee-write", "command": "tee sub/cr\rlink/f", "R": true, "A": true, "W": true, "P": false, "S": false, "V": false} +{"id": "cr-cp-write", "command": "cp a.txt sub/cr\rlink/f", "R": true, "A": true, "W": true, "P": false, "S": false, "V": false} +{"id": "cr-cd-touch", "command": "cd cr\rdir; touch f", "R": true, "A": true, "W": true, "P": false, "S": false, "V": false} +{"id": "cr-pushd-touch", "command": "pushd cr\rdir && touch f", "R": true, "A": true, "W": true, "P": false, "S": false, "V": false} +{"id": "cr-cd-read", "command": "cd cr\rdir && cat secret", "R": true, "A": false, "W": true, "P": false, "S": false, "V": false} +{"id": "cr-cd-relative-nested", "command": "cd cr\rsub && cat esc", "R": true, "A": false, "W": false, "P": false, "S": false, "V": false, "operation": "path reference"} +{"id": "cr-crlf-read", "command": "cat sub/cr\rlink/secret\r\n", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "cr-crlf-tail", "command": "cat lfcr\r\n", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-cat", "command": "cat lf", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-head", "command": "head -n1 lf", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-grep", "command": "grep x lf", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-diff", "command": "diff a.txt lf", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-quoted-space", "command": "cat \"space lf\"", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-escaped-space", "command": "cat space\\ lf", "R": true, "A": false, "W": false, "P": false, "S": false, "V": false, "operation": "path reference"} +{"id": "rel-fullwidth", "command": "cat wide\u3000lf", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-dd-if", "command": "dd if=lf", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-grep-file", "command": "grep --file=lf a.txt", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-make", "command": "make IN=lf", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-input", "command": "cat < lf", "R": true, "A": false, "W": false, "P": false, "S": false, "V": false, "operation": "path reference"} +{"id": "rel-pipe", "command": "cat lf | head", "R": true, "A": false, "W": false, "P": false, "S": false, "V": false, "operation": "path reference"} +{"id": "rel-subshell", "command": "(cat lf)", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-and", "command": "true && cat lf", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-cp", "command": "cp lf a2.txt", "R": true, "A": true, "W": false, "P": false, "S": false, "V": false, "operation": "path reference"} +{"id": "rel-source", "command": "source lf", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-dot", "command": ". lf", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-python", "command": "python3 lf", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-ls-linked", "command": "ls linked-outside", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "rel-dangling", "command": "cat dangling", "R": true, "A": false, "W": false, "P": false, "S": false, "V": true, "operation": "path reference"} +{"id": "cr-true-rm-stays", "command": "true\rrm tests/spec.rs", "R": false, "A": true, "W": false, "P": true, "S": false, "V": false} +{"id": "cr-true-tee-stays", "command": "true\rtee /nonexistent-f613-outside/f", "R": true, "A": true, "W": true, "P": false, "S": false, "V": false} +{"id": "allow-cat-readme", "command": "cat README.md", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} +{"id": "allow-ls-src", "command": "ls src", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} +{"id": "allow-ls-la", "command": "ls -la", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} +{"id": "allow-git-status", "command": "git status", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} +{"id": "allow-echo-out", "command": "echo x > out.txt", "R": false, "A": true, "W": false, "P": false, "S": false, "V": false} +{"id": "allow-touch-newfile", "command": "touch newfile", "R": false, "A": true, "W": false, "P": false, "S": false, "V": false} +{"id": "allow-cp-rename", "command": "cp a.txt c.txt", "R": false, "A": true, "W": false, "P": false, "S": false, "V": false} +{"id": "allow-cd-cat-f", "command": "cd sub && cat f", "R": false, "A": false, "W": false, "P": false, "S": false, "V": false} +{"id": "allow-cd-ls", "command": "cd sub && ls", "R": false, "A": false, "W": false, "P": false, "S": false, "V": false} +{"id": "allow-make-cc", "command": "make CC=gcc", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} +{"id": "allow-cargo-features", "command": "cargo build --features=gui", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} +{"id": "allow-cat-inlink", "command": "cat inlink", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} +{"id": "allow-echo-node", "command": "echo node", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} +{"id": "allow-foo-assign", "command": "FOO=lf cargo test", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} +{"id": "allow-crlf-cat", "command": "cat a.txt\r\n", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} +{"id": "allow-crlf-cargo", "command": "cargo test\r\n", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} +{"id": "allow-crlf-git", "command": "git status\r\ngit diff\r\n", "R": false, "A": false, "W": false, "P": false, "S": false, "V": true} +{"id": "allow-crlf-cd-cat", "command": "cd sub\r\ncat f\r\n", "R": false, "A": false, "W": false, "P": false, "S": false, "V": false} +{"id": "allow-crlf-echo-out", "command": "echo x > out.txt\r\n", "R": false, "A": true, "W": false, "P": false, "S": false, "V": false} diff --git a/tests/issue567_bash_glob_write_targets.rs b/tests/issue567_bash_glob_write_targets.rs index 285e996a..eeb9ed91 100644 --- a/tests/issue567_bash_glob_write_targets.rs +++ b/tests/issue567_bash_glob_write_targets.rs @@ -87,6 +87,10 @@ fn ambiguous_fixture() -> Fixture { std::fs::write(outside.join("secret"), "outside-secret").unwrap(); std::os::unix::fs::symlink(&outside, root.join("sub/link")).unwrap(); std::os::unix::fs::symlink(&outside, root.join("sub/wide\u{3000}link")).unwrap(); + // Issue #613 problem 1: outward symlinks whose name holds a carriage return. + // Bash keeps the `\r` inside the word; the write guard reads both ways. + std::os::unix::fs::symlink(&outside, root.join("sub/cr\rlink")).unwrap(); + std::os::unix::fs::symlink(&outside, root.join("cr\rdir")).unwrap(); let root = root.canonicalize().unwrap(); Fixture { _dir: dir, root } } @@ -461,6 +465,36 @@ fn rejects_mixed_quoted_and_executed_glob_write_targets() { } } +#[test] +fn rejects_carriage_return_write_targets() { + // Issue #613 problem 1 on the write side: a carriage return is an ordinary + // word character in Bash, so a write target or `cd` destination whose name + // holds a `\r` still resolves through the outward symlink and is refused. + // The historical reading split on the `\r` and saw only the harmless halves. + let fixture = ambiguous_fixture(); + let root = &fixture.root; + for command in [ + "echo x > sub/cr\rlink/f", + "tee sub/cr\rlink/f", + "cp a.txt sub/cr\rlink/f", + "cd cr\rdir && echo x > f", + "cd cr\rdir; touch f", + "pushd cr\rdir && touch f", + ] { + assert!( + path_confinement_rejection(command, root).is_some(), + "expected rejection: {command:?}" + ); + } + // A `\r` that just ends the line points at no symlink and stays allowed. + for command in ["echo x > out.txt\r\n", "cat a.txt\r\n"] { + assert!( + path_confinement_rejection(command, root).is_none(), + "expected allow: {command:?}" + ); + } +} + #[test] fn rejects_write_targets_broken_by_unicode_whitespace() { // Issue #602 problem 2: bash splits words only on space and tab, so a diff --git a/tests/issue582_bash_read_candidates.rs b/tests/issue582_bash_read_candidates.rs index c0238698..1fa3fae8 100644 --- a/tests/issue582_bash_read_candidates.rs +++ b/tests/issue582_bash_read_candidates.rs @@ -64,6 +64,10 @@ fn fixture() -> Fixture { "frontend", "dir1", "dir2", + // Issue #613: an inside directory whose name holds a carriage return. + // Only the WordChar reading reaches it, and a nested outward symlink + // below it is invisible to every other candidate. + "cr\rsub", ] { std::fs::create_dir_all(root.join(directory)).unwrap(); } @@ -91,6 +95,20 @@ fn fixture() -> Fixture { std::os::unix::fs::symlink(&outside, root.join(".hidden-out")).unwrap(); std::os::unix::fs::symlink(dir.path().join("missing-target"), root.join("dangling")).unwrap(); std::os::unix::fs::symlink(root.join("sub"), root.join("sub/loop")).unwrap(); + std::os::unix::fs::symlink(root.join("sub"), root.join("inlink")).unwrap(); + // Issue #613 problem 1: outward symlinks whose name holds a carriage return. + // Bash keeps the `\r` inside the word; the historical reading splits on it + // and misses the symlink. + std::os::unix::fs::symlink(&outside, root.join("sub/cr\rlink")).unwrap(); + std::os::unix::fs::symlink(&outside, root.join("cr\rdir")).unwrap(); + std::os::unix::fs::symlink(outside.join("secret"), root.join("lfcr\r")).unwrap(); + // Issue #613 problem 2: a `/`-less name can itself be an outward symlink. + std::os::unix::fs::symlink(&outside, root.join("lf")).unwrap(); + std::os::unix::fs::symlink(&outside, root.join("space lf")).unwrap(); + std::os::unix::fs::symlink(&outside, root.join("wide\u{3000}lf")).unwrap(); + // Issue #613: an outward symlink inside the CR-named directory above, so + // only the merged WordChar candidate reaches it (`cd cr␍sub && cat esc`). + std::os::unix::fs::symlink(&outside, root.join("cr\rsub/esc")).unwrap(); let root = root.canonicalize().unwrap(); Fixture { _dir: dir, root } } @@ -699,3 +717,117 @@ fn read_candidates_reject_embedded_absolute_path_in_an_allowed_dynamic_word() { ], ); } + +/// Issue #613 problem 1: Bash keeps a carriage return inside the word. The +/// historical reading split on it and missed an outward symlink whose name holds +/// a `\r`; the guard now reads the text both ways. The last row pins acceptance +/// criterion 3 (`true⏎rm 保護 path` keeps its value). +#[test] +fn read_candidates_reject_carriage_return_symlinks() { + let fixture = fixture(); + let root = &fixture.root; + assert_rejected( + root, + &[ + "echo x > sub/cr\rlink/f", + "cat sub/cr\rlink/secret", + "cd cr\rdir && echo x > f", + "tee sub/cr\rlink/f", + "cp a.txt sub/cr\rlink/f", + "cd cr\rdir; touch f", + "pushd cr\rdir && touch f", + "cd cr\rdir && cat secret", + "cat sub/cr\rlink/secret\r\n", + "cat lfcr\r\n", + "true\rtee /nonexistent-f613-outside/f", + // Only the WordChar reading reaches `cr\rsub`; the nested outward + // symlink `cr\rsub/esc` is invisible to every other candidate, so + // dropping the `working_directory` union would allow it. + "cd cr\rsub && cat esc", + ], + ); +} + +/// Issue #613 design 2 ("存在しない・ディレクトリでない以外の理由で lstat に +/// 失敗したら、確かめる"): a `/`-less word whose `lstat` fails for a reason other +/// than a missing name still reaches the literal proof. A component longer than +/// `NAME_MAX` fails with `ENAMETOOLONG` and is refused, not admitted. +#[test] +fn read_candidates_reject_unstattable_relative_words() { + let fixture = fixture(); + let root = &fixture.root; + let over_long = "a".repeat(300); + assert_rejected(root, &[&format!("cat {over_long}")]); +} + +/// Issue #613 problem 2: a `/`-less relative word that is itself an outward +/// symlink is proven against the workspace root and every working-directory +/// candidate, whatever position it holds. +#[test] +fn read_candidates_reject_relative_symlink_words() { + let fixture = fixture(); + let root = &fixture.root; + assert_rejected( + root, + &[ + "cat lf", + "head -n1 lf", + "grep x lf", + "diff a.txt lf", + "cat \"space lf\"", + "cat space\\ lf", + "cat wide\u{3000}lf", + "dd if=lf", + "grep --file=lf a.txt", + "make IN=lf", + "cat < lf", + "cat lf | head", + "(cat lf)", + "true && cat lf", + "cp lf a2.txt", + "source lf", + ". lf", + "python3 lf", + "ls linked-outside", + "cat dangling", + ], + ); +} + +/// The `許可のまま` shapes must not be falsely refused: a word that is not an +/// outward symlink costs one `lstat` and stays allowed, a leading assignment's +/// value is a variable, an inside symlink resolves inside, and the CRLF line end +/// keeps its value. The last two rows pin the two values of acceptance criterion +/// 3. +#[test] +fn read_candidates_allow_the_carriage_return_and_relative_normal_forms() { + let fixture = fixture(); + let root = &fixture.root; + assert_allowed( + root, + &[ + "cat README.md", + "ls src", + "ls -la", + "git status", + "echo x > out.txt", + "touch newfile", + "cp a.txt c.txt", + "cd sub && cat f", + "cd sub && ls", + "make CC=gcc", + "cargo build --features=gui", + "dd if=input.txt of=out.bin", + "cat inlink", + "echo node", + "awk -F= '{print $1}' a.txt", + "FOO=lf cargo test", + "cat a.txt\r\n", + "cargo test\r\n", + "git status\r\ngit diff\r\n", + "cd sub\r\ncat f\r\n", + "echo x > out.txt\r\n", + "true\rrm tests/spec.rs", + ], + ); +}