Skip to content

Commit 6561d7a

Browse files
committed
fix: address Gemini review round 3
- Validate attachment paths stay under CWD (path traversal protection) consistent with validate.rs patterns - Encode non-ASCII filenames using RFC 2231 (filename*=UTF-8''...) for correct display in all email clients - Add #[derive(Debug)] to Attachment struct - Add tests for path traversal rejection, RFC 2231 encoding, and filename quoting
1 parent 875ae35 commit 6561d7a

1 file changed

Lines changed: 91 additions & 7 deletions

File tree

src/helpers/gmail/mod.rs

Lines changed: 91 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ pub(super) fn encode_header_value(value: &str) -> String {
273273
}
274274

275275
/// A file attachment to include in an outgoing email.
276+
#[derive(Debug)]
276277
pub(super) struct Attachment {
277278
pub filename: String,
278279
pub content_type: String,
@@ -315,23 +316,39 @@ pub(super) fn guess_content_type(filename: &str) -> &'static str {
315316

316317
/// Read attachment files from a list of paths.
317318
///
318-
/// Each path is canonicalized to resolve symlinks and `..` components; the
319-
/// function rejects paths that do not point to a regular file.
319+
/// Each path is resolved relative to the current working directory and
320+
/// canonicalized to resolve symlinks and `..` components. The function
321+
/// rejects paths that resolve outside CWD (path traversal protection,
322+
/// consistent with `validate.rs`) or that do not point to a regular file.
320323
pub(super) fn read_attachments(paths: &[String]) -> Result<Vec<Attachment>, GwsError> {
324+
let cwd = std::env::current_dir().map_err(|e| {
325+
GwsError::Other(anyhow::anyhow!("Failed to determine current directory: {e}"))
326+
})?;
327+
let canonical_cwd = cwd.canonicalize().map_err(|e| {
328+
GwsError::Other(anyhow::anyhow!("Failed to canonicalize current directory: {e}"))
329+
})?;
330+
321331
let mut attachments = Vec::new();
322332
for path_str in paths {
323333
let path_str = path_str.trim();
324334
if path_str.is_empty() {
325335
continue;
326336
}
327-
let path = std::path::Path::new(path_str);
337+
let path = cwd.join(path_str);
328338
let canonical = path.canonicalize().map_err(|e| {
329339
GwsError::Other(anyhow::anyhow!(
330340
"Failed to resolve attachment path '{}': {}",
331341
path_str,
332342
e
333343
))
334344
})?;
345+
if !canonical.starts_with(&canonical_cwd) {
346+
return Err(GwsError::Other(anyhow::anyhow!(
347+
"Attachment '{}' resolves to '{}' which is outside the current directory",
348+
path_str,
349+
canonical.display(),
350+
)));
351+
}
335352
if !canonical.is_file() {
336353
return Err(GwsError::Other(anyhow::anyhow!(
337354
"Attachment path '{}' is not a regular file",
@@ -388,6 +405,29 @@ fn escape_quoted_string(s: &str) -> String {
388405
.replace('"', "\\\"")
389406
}
390407

408+
/// Encode a filename for MIME Content-Type/Content-Disposition headers.
409+
///
410+
/// For ASCII-only filenames, returns a simple `name="filename"` pair.
411+
/// For non-ASCII filenames, uses RFC 2231 encoding (`name*=UTF-8''...`)
412+
/// which is supported by all modern email clients.
413+
fn encode_mime_filename(param: &str, filename: &str) -> String {
414+
if filename.is_ascii() {
415+
format!("{}=\"{}\"", param, escape_quoted_string(filename))
416+
} else {
417+
// RFC 2231: parameter*=charset'language'value
418+
// Percent-encode non-ASCII bytes and RFC 5987 special chars.
419+
let mut encoded = String::new();
420+
for &byte in filename.as_bytes() {
421+
if byte.is_ascii_alphanumeric() || b"!#$&+-.^_`|~".contains(&byte) {
422+
encoded.push(byte as char);
423+
} else {
424+
encoded.push_str(&format!("%{:02X}", byte));
425+
}
426+
}
427+
format!("{}*=UTF-8''{}", param, encoded)
428+
}
429+
}
430+
391431
impl MessageBuilder<'_> {
392432
/// Build the common RFC 2822 headers shared by both plain and multipart
393433
/// messages. The `content_type_line` parameter supplies the Content-Type
@@ -484,15 +524,16 @@ impl MessageBuilder<'_> {
484524
offset = end;
485525
}
486526

487-
let safe_filename = escape_quoted_string(&att.filename);
527+
let ct_name = encode_mime_filename("name", &att.filename);
528+
let cd_filename = encode_mime_filename("filename", &att.filename);
488529
message.push_str(&format!(
489530
"--{}\r\n\
490-
Content-Type: {}; name=\"{}\"\r\n\
491-
Content-Disposition: attachment; filename=\"{}\"\r\n\
531+
Content-Type: {}; {}\r\n\
532+
Content-Disposition: attachment; {}\r\n\
492533
Content-Transfer-Encoding: base64\r\n\
493534
\r\n\
494535
{}\r\n",
495-
boundary, att.content_type, safe_filename, safe_filename, folded,
536+
boundary, att.content_type, ct_name, cd_filename, folded,
496537
));
497538
}
498539

@@ -1423,4 +1464,47 @@ mod tests {
14231464
assert_eq!(resolved.http_method, "POST");
14241465
assert_eq!(resolved.path, "gmail/v1/users/{userId}/messages/send");
14251466
}
1467+
1468+
#[test]
1469+
fn test_encode_mime_filename_ascii() {
1470+
assert_eq!(
1471+
encode_mime_filename("name", "report.pdf"),
1472+
"name=\"report.pdf\""
1473+
);
1474+
}
1475+
1476+
#[test]
1477+
fn test_encode_mime_filename_with_quotes() {
1478+
assert_eq!(
1479+
encode_mime_filename("name", "my \"file\".pdf"),
1480+
"name=\"my \\\"file\\\".pdf\""
1481+
);
1482+
}
1483+
1484+
#[test]
1485+
fn test_encode_mime_filename_non_ascii() {
1486+
let result = encode_mime_filename("filename", "résumé.pdf");
1487+
assert!(result.starts_with("filename*=UTF-8''"));
1488+
assert!(result.contains("r%C3%A9sum%C3%A9.pdf"));
1489+
}
1490+
1491+
#[test]
1492+
fn test_read_attachments_rejects_path_traversal() {
1493+
let paths = vec!["../../../etc/passwd".to_string()];
1494+
let result = read_attachments(&paths);
1495+
assert!(result.is_err());
1496+
let err = result.unwrap_err().to_string();
1497+
assert!(
1498+
err.contains("outside the current directory") || err.contains("Failed to resolve"),
1499+
"Expected path traversal rejection, got: {err}"
1500+
);
1501+
}
1502+
1503+
#[test]
1504+
fn test_read_attachments_rejects_empty_paths() {
1505+
let paths = vec![" ".to_string(), "".to_string()];
1506+
let result = read_attachments(&paths);
1507+
assert!(result.is_ok());
1508+
assert!(result.unwrap().is_empty());
1509+
}
14261510
}

0 commit comments

Comments
 (0)