fix: address Gemini review round 2

- Change --attachment from comma-separated to ArgAction::Append
  (multiple --attachment flags, no more fragile split on comma)
- Add path canonicalization and regular-file check in read_attachments
- Remove unwrap() in base64 folding, use direct byte-index slicing

Author: pae-hexa

src/helpers/gmail/forward.rs

@@ -36,8 +36,8 @@ pub(super) async fn handle_forward(
         (orig, Some(t))
     };
 
-    let attachments = match parse_optional_trimmed(matches, "attachment") {
-        Some(paths) => read_attachments(&paths)?,
+    let attachments = match matches.get_many::<String>("attachment") {
+        Some(paths) => read_attachments(&paths.cloned().collect::<Vec<_>>())?,
         None => Vec::new(),
     };
 

src/helpers/gmail/mod.rs

@@ -313,23 +313,39 @@ pub(super) fn guess_content_type(filename: &str) -> &'static str {
     }
 }
 
-/// Read attachment files from a comma-separated list of paths.
-pub(super) fn read_attachments(paths_csv: &str) -> Result<Vec<Attachment>, GwsError> {
+/// Read attachment files from a list of paths.
+///
+/// Each path is canonicalized to resolve symlinks and `..` components; the
+/// function rejects paths that do not point to a regular file.
+pub(super) fn read_attachments(paths: &[String]) -> Result<Vec<Attachment>, GwsError> {
     let mut attachments = Vec::new();
-    for path_str in paths_csv.split(',') {
+    for path_str in paths {
         let path_str = path_str.trim();
         if path_str.is_empty() {
             continue;
         }
         let path = std::path::Path::new(path_str);
-        let data = std::fs::read(path).map_err(|e| {
+        let canonical = path.canonicalize().map_err(|e| {
+            GwsError::Other(anyhow::anyhow!(
+                "Failed to resolve attachment path '{}': {}",
+                path_str,
+                e
+            ))
+        })?;
+        if !canonical.is_file() {
+            return Err(GwsError::Other(anyhow::anyhow!(
+                "Attachment path '{}' is not a regular file",
+                path_str,
+            )));
+        }
+        let data = std::fs::read(&canonical).map_err(|e| {
             GwsError::Other(anyhow::anyhow!(
                 "Failed to read attachment '{}': {}",
                 path_str,
                 e
             ))
         })?;
-        let filename = path
+        let filename = canonical
             .file_name()
             .and_then(|n| n.to_str())
             .unwrap_or(path_str)
@@ -456,12 +472,17 @@ impl MessageBuilder<'_> {
         for att in attachments {
             let encoded = STANDARD.encode(&att.data);
             // Fold base64 into 76-character lines per RFC 2045.
-            let folded: String = encoded
-                .as_bytes()
-                .chunks(76)
-                .map(|chunk| std::str::from_utf8(chunk).unwrap())
-                .collect::<Vec<_>>()
-                .join("\r\n");
+            // Base64 output is pure ASCII, so byte-index slicing is safe.
+            let mut folded = String::with_capacity(encoded.len() + (encoded.len() / 76) * 2);
+            let mut offset = 0;
+            while offset < encoded.len() {
+                if offset > 0 {
+                    folded.push_str("\r\n");
+                }
+                let end = (offset + 76).min(encoded.len());
+                folded.push_str(&encoded[offset..end]);
+                offset = end;
+            }
 
             let safe_filename = escape_quoted_string(&att.filename);
             message.push_str(&format!(
@@ -630,8 +651,9 @@ impl Helper for GmailHelper {
                 .arg(
                     Arg::new("attachment")
                         .long("attachment")
-                        .help("File path(s) to attach, comma-separated")
-                        .value_name("PATHS"),
+                        .help("File path to attach (may be repeated)")
+                        .value_name("PATH")
+                        .action(ArgAction::Append),
                 )
                 .arg(
                     Arg::new("dry-run")
@@ -646,7 +668,7 @@ EXAMPLES:
   gws gmail +send --to alice@example.com --subject 'Hello' --body 'Hi!' --cc bob@example.com
   gws gmail +send --to alice@example.com --subject 'Hello' --body 'Hi!' --bcc secret@example.com
   gws gmail +send --to alice@example.com --subject 'Report' --body 'See attached' --attachment report.pdf
-  gws gmail +send --to alice@example.com --subject 'Files' --body 'Multiple' --attachment 'a.pdf,b.zip'
+  gws gmail +send --to alice@example.com --subject 'Files' --body 'Multiple' --attachment a.pdf --attachment b.zip
 
 TIPS:
   Handles RFC 2822 formatting and base64 encoding automatically.
@@ -734,8 +756,9 @@ TIPS:
                 .arg(
                     Arg::new("attachment")
                         .long("attachment")
-                        .help("File path(s) to attach, comma-separated")
-                        .value_name("PATHS"),
+                        .help("File path to attach (may be repeated)")
+                        .value_name("PATH")
+                        .action(ArgAction::Append),
                 )
                 .arg(
                     Arg::new("dry-run")
@@ -803,8 +826,9 @@ TIPS:
                 .arg(
                     Arg::new("attachment")
                         .long("attachment")
-                        .help("File path(s) to attach, comma-separated")
-                        .value_name("PATHS"),
+                        .help("File path to attach (may be repeated)")
+                        .value_name("PATH")
+                        .action(ArgAction::Append),
                 )
                 .arg(
                     Arg::new("remove")
@@ -881,8 +905,9 @@ TIPS:
                 .arg(
                     Arg::new("attachment")
                         .long("attachment")
-                        .help("File path(s) to attach, comma-separated")
-                        .value_name("PATHS"),
+                        .help("File path to attach (may be repeated)")
+                        .value_name("PATH")
+                        .action(ArgAction::Append),
                 )
                 .arg(
                     Arg::new("dry-run")

src/helpers/gmail/reply.rs

@@ -94,8 +94,8 @@ pub(super) async fn handle_reply(
         body: &config.body_text,
     };
 
-    let attachments = match parse_optional_trimmed(matches, "attachment") {
-        Some(paths) => read_attachments(&paths)?,
+    let attachments = match matches.get_many::<String>("attachment") {
+        Some(paths) => read_attachments(&paths.cloned().collect::<Vec<_>>())?,
         None => Vec::new(),
     };
 

src/helpers/gmail/send.rs

@@ -6,8 +6,8 @@ pub(super) async fn handle_send(
 ) -> Result<(), GwsError> {
     let config = parse_send_args(matches);
 
-    let attachments = match parse_optional_trimmed(matches, "attachment") {
-        Some(paths) => read_attachments(&paths)?,
+    let attachments = match matches.get_many::<String>("attachment") {
+        Some(paths) => read_attachments(&paths.cloned().collect::<Vec<_>>())?,
         None => Vec::new(),
     };