From 9e5a99db12981aadccfdcaeb9cf3ceb713497fa0 Mon Sep 17 00:00:00 2001 From: wangzhengkui Date: Thu, 9 Apr 2026 21:55:28 +0800 Subject: [PATCH 1/7] fix(mail): restrict --output-dir to current working directory Previously, mail +watch --output-dir accepted absolute paths (e.g. /etc, /tmp) and home directory paths (~/), allowing writes to arbitrary locations. Since mail content is sender-controlled, this posed a risk of writing attacker-influenced data to sensitive system directories. Now all --output-dir values go through validate.SafeOutputPath which: - Rejects absolute paths and ~ expansion - Resolves .. and symlinks - Enforces the result stays under CWD --- shortcuts/mail/mail_watch.go | 33 +++++++-------------------------- 1 file changed, 7 insertions(+), 26 deletions(-) diff --git a/shortcuts/mail/mail_watch.go b/shortcuts/mail/mail_watch.go index 8de4d87d9..47f00b25f 100644 --- a/shortcuts/mail/mail_watch.go +++ b/shortcuts/mail/mail_watch.go @@ -192,36 +192,17 @@ var MailWatch = common.Shortcut{ msgFormat := runtime.Str("msg-format") outputDir := runtime.Str("output-dir") if outputDir != "" { - if outputDir == "~" || strings.HasPrefix(outputDir, "~/") { - home, err := vfs.UserHomeDir() - if err != nil { - return fmt.Errorf("cannot expand ~: %w", err) - } - if outputDir == "~" { - outputDir = home - } else { - outputDir = filepath.Join(home, outputDir[2:]) - } - } else if filepath.IsAbs(outputDir) { - outputDir = filepath.Clean(outputDir) - } else { - safePath, err := validate.SafeOutputPath(outputDir) - if err != nil { - return err - } - outputDir = safePath + // Enforce CWD containment: reject absolute paths, ~, path traversal, + // and symlink escapes. SafeOutputPath returns a resolved absolute path + // under CWD, preventing writes to arbitrary system directories. + safePath, err := validate.SafeOutputPath(outputDir) + if err != nil { + return err } - // Resolve symlinks on the output directory so all writes use the real - // filesystem path. This prevents a symlink from redirecting writes to - // an unintended location (TOCTOU mitigation). + outputDir = safePath if err := vfs.MkdirAll(outputDir, 0700); err != nil { return fmt.Errorf("cannot create output directory %q: %w", outputDir, err) } - resolved, err := filepath.EvalSymlinks(outputDir) - if err != nil { - return fmt.Errorf("cannot resolve output directory: %w", err) - } - outputDir = resolved } labelIDsInput := runtime.Str("label-ids") folderIDsInput := runtime.Str("folder-ids") From 0b96b2de1b78d43860897e96d03df16400f70828 Mon Sep 17 00:00:00 2001 From: wangzhengkui Date: Thu, 9 Apr 2026 22:09:15 +0800 Subject: [PATCH 2/7] fix(mail): reject tilde paths in --output-dir explicitly SafeOutputPath treats ~/x as a literal relative path, silently creating a directory named "~" under CWD. Reject ~ prefixed paths with a clear error message instead. --- shortcuts/mail/mail_watch.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/shortcuts/mail/mail_watch.go b/shortcuts/mail/mail_watch.go index 47f00b25f..a49d118f1 100644 --- a/shortcuts/mail/mail_watch.go +++ b/shortcuts/mail/mail_watch.go @@ -192,7 +192,13 @@ var MailWatch = common.Shortcut{ msgFormat := runtime.Str("msg-format") outputDir := runtime.Str("output-dir") if outputDir != "" { - // Enforce CWD containment: reject absolute paths, ~, path traversal, + // Reject tilde paths explicitly — SafeOutputPath treats "~/x" as a + // literal relative path (creating a directory named "~"), which is + // confusing. Fail fast with a clear hint instead. + if outputDir == "~" || strings.HasPrefix(outputDir, "~/") { + return fmt.Errorf("--output-dir does not support ~ expansion; use a relative path like ./output instead") + } + // Enforce CWD containment: reject absolute paths, path traversal, // and symlink escapes. SafeOutputPath returns a resolved absolute path // under CWD, preventing writes to arbitrary system directories. safePath, err := validate.SafeOutputPath(outputDir) From 2a5ec249a86f6588f0bc8486cdffc089d0353a51 Mon Sep 17 00:00:00 2001 From: wangzhengkui Date: Thu, 9 Apr 2026 22:22:50 +0800 Subject: [PATCH 3/7] fix(mail): reject all tilde-prefixed paths and use ErrValidation - Broaden ~ check from "~ || ~/" to "~" prefix, covering ~user/path forms - Use output.ErrValidation for consistent error type (exit code 2) --- shortcuts/mail/mail_watch.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shortcuts/mail/mail_watch.go b/shortcuts/mail/mail_watch.go index a49d118f1..db97a221b 100644 --- a/shortcuts/mail/mail_watch.go +++ b/shortcuts/mail/mail_watch.go @@ -192,11 +192,11 @@ var MailWatch = common.Shortcut{ msgFormat := runtime.Str("msg-format") outputDir := runtime.Str("output-dir") if outputDir != "" { - // Reject tilde paths explicitly — SafeOutputPath treats "~/x" as a + // Reject all tilde-prefixed paths — SafeOutputPath treats "~/x" as a // literal relative path (creating a directory named "~"), which is - // confusing. Fail fast with a clear hint instead. - if outputDir == "~" || strings.HasPrefix(outputDir, "~/") { - return fmt.Errorf("--output-dir does not support ~ expansion; use a relative path like ./output instead") + // confusing. This also covers ~user/path forms. + if strings.HasPrefix(outputDir, "~") { + return output.ErrValidation("--output-dir does not support ~ expansion; use a relative path like ./output instead") } // Enforce CWD containment: reject absolute paths, path traversal, // and symlink escapes. SafeOutputPath returns a resolved absolute path From 1264987234903c7e46ec9ed2b03b3f0417c0ef3d Mon Sep 17 00:00:00 2001 From: wangzhengkui Date: Thu, 9 Apr 2026 22:26:51 +0800 Subject: [PATCH 4/7] fix(mail): add post-mkdir EvalSymlinks + CWD re-verification (TOCTOU) SafeOutputPath validates before MkdirAll, but an attacker could replace the newly created directory with a symlink between mkdir and the first write. Add EvalSymlinks after MkdirAll and re-verify the resolved path is still under CWD. Also broaden ~ rejection to all tilde-prefixed paths (~user/path) and use output.ErrValidation for consistent error types. --- shortcuts/mail/mail_watch.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/shortcuts/mail/mail_watch.go b/shortcuts/mail/mail_watch.go index db97a221b..efe0fc6e9 100644 --- a/shortcuts/mail/mail_watch.go +++ b/shortcuts/mail/mail_watch.go @@ -209,6 +209,24 @@ var MailWatch = common.Shortcut{ if err := vfs.MkdirAll(outputDir, 0700); err != nil { return fmt.Errorf("cannot create output directory %q: %w", outputDir, err) } + // TOCTOU mitigation: after MkdirAll, resolve symlinks on the now-existing + // directory and re-verify it is still under CWD. This prevents an attacker + // from replacing the newly created directory with a symlink between mkdir + // and the first write. + resolved, err := filepath.EvalSymlinks(outputDir) + if err != nil { + return fmt.Errorf("cannot resolve output directory: %w", err) + } + cwd, err := os.Getwd() + if err != nil { + return fmt.Errorf("cannot determine working directory: %w", err) + } + canonicalCwd, _ := filepath.EvalSymlinks(cwd) + rel, err := filepath.Rel(canonicalCwd, resolved) + if err != nil || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || rel == ".." { + return output.ErrValidation("--output-dir %q resolves outside the current working directory after mkdir (possible symlink attack)", outputDir) + } + outputDir = resolved } labelIDsInput := runtime.Str("label-ids") folderIDsInput := runtime.Str("folder-ids") From 37c3fa6d7bc9da2e1294c03040282580a53fb134 Mon Sep 17 00:00:00 2001 From: wangzhengkui Date: Fri, 10 Apr 2026 14:43:26 +0800 Subject: [PATCH 5/7] fix(mail): use validate.SafeOutputPath for post-mkdir TOCTOU check Replace direct os.Getwd and filepath.EvalSymlinks calls with a second SafeOutputPath call after MkdirAll. This satisfies the forbidigo lint rule (no direct os/filepath calls in shortcuts/) while maintaining the same TOCTOU protection. --- shortcuts/mail/mail_watch.go | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/shortcuts/mail/mail_watch.go b/shortcuts/mail/mail_watch.go index efe0fc6e9..2c61b5eca 100644 --- a/shortcuts/mail/mail_watch.go +++ b/shortcuts/mail/mail_watch.go @@ -209,24 +209,15 @@ var MailWatch = common.Shortcut{ if err := vfs.MkdirAll(outputDir, 0700); err != nil { return fmt.Errorf("cannot create output directory %q: %w", outputDir, err) } - // TOCTOU mitigation: after MkdirAll, resolve symlinks on the now-existing - // directory and re-verify it is still under CWD. This prevents an attacker - // from replacing the newly created directory with a symlink between mkdir - // and the first write. - resolved, err := filepath.EvalSymlinks(outputDir) + // TOCTOU mitigation: after MkdirAll the directory now exists, so + // re-validate with SafeOutputPath which will EvalSymlinks on the + // real path and re-verify CWD containment. This prevents an attacker + // from replacing the newly created directory with a symlink between + // mkdir and the first write. + outputDir, err = validate.SafeOutputPath(outputDir) if err != nil { - return fmt.Errorf("cannot resolve output directory: %w", err) - } - cwd, err := os.Getwd() - if err != nil { - return fmt.Errorf("cannot determine working directory: %w", err) - } - canonicalCwd, _ := filepath.EvalSymlinks(cwd) - rel, err := filepath.Rel(canonicalCwd, resolved) - if err != nil || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || rel == ".." { - return output.ErrValidation("--output-dir %q resolves outside the current working directory after mkdir (possible symlink attack)", outputDir) + return err } - outputDir = resolved } labelIDsInput := runtime.Str("label-ids") folderIDsInput := runtime.Str("folder-ids") From 44583712e2ab51418f92e2af7aab48c5a38674ef Mon Sep 17 00:00:00 2001 From: wangzhengkui Date: Fri, 10 Apr 2026 14:48:18 +0800 Subject: [PATCH 6/7] fix(mail): use original relative path for post-mkdir re-validation SafeOutputPath rejects absolute paths, but after the first call outputDir was already resolved to an absolute path. Pass the original relative path to the second SafeOutputPath call so it can properly re-validate after MkdirAll. --- shortcuts/mail/mail_watch.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/shortcuts/mail/mail_watch.go b/shortcuts/mail/mail_watch.go index 2c61b5eca..6ea80d889 100644 --- a/shortcuts/mail/mail_watch.go +++ b/shortcuts/mail/mail_watch.go @@ -210,11 +210,11 @@ var MailWatch = common.Shortcut{ return fmt.Errorf("cannot create output directory %q: %w", outputDir, err) } // TOCTOU mitigation: after MkdirAll the directory now exists, so - // re-validate with SafeOutputPath which will EvalSymlinks on the - // real path and re-verify CWD containment. This prevents an attacker - // from replacing the newly created directory with a symlink between - // mkdir and the first write. - outputDir, err = validate.SafeOutputPath(outputDir) + // re-validate with the original relative path. SafeOutputPath will + // EvalSymlinks on the now-existing directory and re-verify CWD + // containment. This catches symlink replacement between mkdir and + // the first write. + outputDir, err = validate.SafeOutputPath(runtime.Str("output-dir")) if err != nil { return err } From 716dade4fe0ce5a74ff860fa6798cc1c52d1cbda Mon Sep 17 00:00:00 2001 From: wangzhengkui Date: Fri, 10 Apr 2026 14:55:55 +0800 Subject: [PATCH 7/7] fix(mail): remove redundant post-mkdir SafeOutputPath call The second SafeOutputPath call after MkdirAll provided no real TOCTOU protection: mail +watch is long-running, so the directory could be replaced at any point during the session, not just between mkdir and the check. The first SafeOutputPath already validates and resolves the path; one call is sufficient. --- shortcuts/mail/mail_watch.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/shortcuts/mail/mail_watch.go b/shortcuts/mail/mail_watch.go index 6ea80d889..db97a221b 100644 --- a/shortcuts/mail/mail_watch.go +++ b/shortcuts/mail/mail_watch.go @@ -209,15 +209,6 @@ var MailWatch = common.Shortcut{ if err := vfs.MkdirAll(outputDir, 0700); err != nil { return fmt.Errorf("cannot create output directory %q: %w", outputDir, err) } - // TOCTOU mitigation: after MkdirAll the directory now exists, so - // re-validate with the original relative path. SafeOutputPath will - // EvalSymlinks on the now-existing directory and re-verify CWD - // containment. This catches symlink replacement between mkdir and - // the first write. - outputDir, err = validate.SafeOutputPath(runtime.Str("output-dir")) - if err != nil { - return err - } } labelIDsInput := runtime.Str("label-ids") folderIDsInput := runtime.Str("folder-ids")