Skip to content

Commit f0141ae

Browse files
authored
fix(tui): constrain file injection paths
Merge the file-injection path confinement fix. - keep @file references constrained to the canonical workspace root - reject symlink escapes through canonical path checks - clarify that unresolved paths are skipped when they cannot be resolved safely
1 parent 13ee395 commit f0141ae

1 file changed

Lines changed: 69 additions & 15 deletions

File tree

src-rust/crates/tui/src/file_injection.rs

Lines changed: 69 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ pub enum AtFileIssue {
77
TooLarge(usize), // size in KB that exceeds limit
88
Binary,
99
Unreadable(String), // error message
10-
IsDirectory, // Path points to a directory, not a file
10+
IsDirectory, // Path points to a directory, not a file
1111
}
1212

1313
/// A parsed file reference from the user's input (e.g., "@src/main.rs").
@@ -28,10 +28,19 @@ pub struct AtFileRef {
2828
/// Parse word-boundary @ tokens from text.
2929
/// Returns (within_limit, oversized).
3030
/// If max_size_kb == 0, oversized is always empty (accept all).
31-
pub fn parse_at_refs(text: &str, cwd: &Path, max_size_kb: usize) -> (Vec<AtFileRef>, Vec<AtFileRef>) {
31+
pub fn parse_at_refs(
32+
text: &str,
33+
cwd: &Path,
34+
max_size_kb: usize,
35+
) -> (Vec<AtFileRef>, Vec<AtFileRef>) {
3236
let mut within_limit = Vec::new();
3337
let mut oversized = Vec::new();
3438

39+
let canonical_cwd = match cwd.canonicalize() {
40+
Ok(path) => path,
41+
Err(_) => return (within_limit, oversized),
42+
};
43+
3544
let words: Vec<&str> = text.split_whitespace().collect();
3645

3746
for word in words {
@@ -45,7 +54,10 @@ pub fn parse_at_refs(text: &str, cwd: &Path, max_size_kb: usize) -> (Vec<AtFileR
4554
let mut token = word.to_string();
4655

4756
// Remove trailing punctuation, but never strip the leading '@' itself.
48-
while token.len() > 1 && token.ends_with(|c: char| c.is_ascii_punctuation()) && !token.ends_with('/') {
57+
while token.len() > 1
58+
&& token.ends_with(|c: char| c.is_ascii_punctuation())
59+
&& !token.ends_with('/')
60+
{
4961
token.pop();
5062
}
5163

@@ -67,28 +79,32 @@ pub fn parse_at_refs(text: &str, cwd: &Path, max_size_kb: usize) -> (Vec<AtFileR
6779
cwd.join(path_part)
6880
};
6981

70-
// Check if path exists and is a file (not a directory)
71-
if !expanded_path.exists() {
72-
continue; // Skip non-existent paths
82+
let canonical_path = match expanded_path.canonicalize() {
83+
Ok(path) => path,
84+
Err(_) => continue, // Skip paths that cannot be resolved safely
85+
};
86+
87+
if !canonical_path.starts_with(&canonical_cwd) {
88+
continue;
7389
}
7490

75-
if expanded_path.is_dir() {
91+
if canonical_path.is_dir() {
7692
oversized.push(AtFileRef {
7793
token: token.clone(),
78-
path: expanded_path,
94+
path: canonical_path,
7995
size_kb: 0,
8096
contents: None,
8197
issue: Some(AtFileIssue::IsDirectory),
8298
});
8399
continue;
84100
}
85101

86-
let size_kb = match fs::metadata(&expanded_path) {
102+
let size_kb = match fs::metadata(&canonical_path) {
87103
Ok(meta) => (meta.len() as usize + 1023) / 1024, // Round up to KB
88104
Err(e) => {
89105
oversized.push(AtFileRef {
90106
token: token.clone(),
91-
path: expanded_path,
107+
path: canonical_path,
92108
size_kb: 0,
93109
contents: None,
94110
issue: Some(AtFileIssue::Unreadable(e.to_string())),
@@ -98,12 +114,12 @@ pub fn parse_at_refs(text: &str, cwd: &Path, max_size_kb: usize) -> (Vec<AtFileR
98114
};
99115

100116
// Check if file is binary
101-
let contents = match fs::read_to_string(&expanded_path) {
117+
let contents = match fs::read_to_string(&canonical_path) {
102118
Ok(contents) => contents,
103119
Err(_) => {
104120
oversized.push(AtFileRef {
105121
token: token.clone(),
106-
path: expanded_path,
122+
path: canonical_path,
107123
size_kb,
108124
contents: None,
109125
issue: Some(AtFileIssue::Binary),
@@ -116,15 +132,15 @@ pub fn parse_at_refs(text: &str, cwd: &Path, max_size_kb: usize) -> (Vec<AtFileR
116132
if max_size_kb > 0 && size_kb > max_size_kb {
117133
oversized.push(AtFileRef {
118134
token,
119-
path: expanded_path,
135+
path: canonical_path,
120136
size_kb,
121137
contents: None,
122138
issue: Some(AtFileIssue::TooLarge(size_kb)),
123139
});
124140
} else {
125141
within_limit.push(AtFileRef {
126142
token,
127-
path: expanded_path,
143+
path: canonical_path,
128144
size_kb,
129145
contents: Some(contents),
130146
issue: None,
@@ -185,13 +201,51 @@ mod tests {
185201
#[test]
186202
fn test_parse_at_refs_nonexistent() {
187203
let temp = TempDir::new().unwrap();
188-
let input = format!("Please check @{}", temp.path().join("nonexistent.txt").display());
204+
let input = format!(
205+
"Please check @{}",
206+
temp.path().join("nonexistent.txt").display()
207+
);
189208

190209
let (within, oversized) = parse_at_refs(&input, temp.path(), 1024);
191210
assert_eq!(within.len(), 0);
192211
assert_eq!(oversized.len(), 0);
193212
}
194213

214+
#[test]
215+
fn test_parse_at_refs_rejects_absolute_path_outside_cwd() {
216+
let cwd = TempDir::new().unwrap();
217+
let outside = TempDir::new().unwrap();
218+
let secret_path = outside.path().join("secret.txt");
219+
fs::write(&secret_path, "secret content").unwrap();
220+
221+
let input = format!("Please check @{}", secret_path.display());
222+
let (within, oversized) = parse_at_refs(&input, cwd.path(), 1024);
223+
224+
assert_eq!(within.len(), 0);
225+
assert_eq!(oversized.len(), 0);
226+
}
227+
228+
#[cfg(unix)]
229+
#[test]
230+
fn test_parse_at_refs_rejects_symlink_to_file_outside_cwd() {
231+
use std::os::unix::fs::symlink;
232+
233+
let cwd = TempDir::new().unwrap();
234+
let outside = TempDir::new().unwrap();
235+
let secret_path = outside.path().join("secret.txt");
236+
fs::write(&secret_path, "secret content").unwrap();
237+
238+
let docs_dir = cwd.path().join("docs");
239+
fs::create_dir(&docs_dir).unwrap();
240+
let symlink_path = docs_dir.join("review.md");
241+
symlink(&secret_path, &symlink_path).unwrap();
242+
243+
let (within, oversized) = parse_at_refs("Please check @docs/review.md", cwd.path(), 1024);
244+
245+
assert_eq!(within.len(), 0);
246+
assert_eq!(oversized.len(), 0);
247+
}
248+
195249
#[test]
196250
fn test_parse_at_refs_size_limit() {
197251
let temp = TempDir::new().unwrap();

0 commit comments

Comments
 (0)