fix: windows filename chars

[taylorwilsdon]

Closes #772

Summary by CodeRabbit

Release Notes

Bug Fixes

• Attachment filenames are now sanitized to remove invalid characters and Windows reserved device names, ensuring reliable file storage across all platforms.
• When saving attachments, the response now displays the actual filename used on disk, providing better confirmation of the saved file.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a27fab17-6ae7-425b-be6c-f7ae32965b77

📥 Commits

Reviewing files that changed from the base of the PR and between cb679b3 and b96d2c0.

⛔ Files ignored due to path filters (1)

• uv.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

• core/attachment_storage.py
• tests/gmail/test_attachment_fix.py

---

📝 Walkthrough

Walkthrough

Adds filename sanitization for attachments (Windows-invalid characters and reserved device names), lazily creates the storage directory with restrictive permissions, derives on-disk names from sanitized input and records original filenames in metadata, updates Gmail tooling to report the saved filename, and adds tests verifying sanitization and metadata.

Changes

Attachment Filename Sanitization

Layer / File(s)	Summary
Sanitization Constants & Imports core/attachment_storage.py	Adds re import and defines Windows-invalid-character regex and reserved device-name constants (COM1–COM9, LPT1–LPT9, CON, PRN, AUX, NUL, etc.).
Sanitization Function core/attachment_storage.py	sanitize_attachment_filename() normalizes filenames by handling None/empty inputs, replacing invalid characters, trimming trailing spaces/dots, and avoiding reserved device names.
Storage Directory Management core/attachment_storage.py	Introduces lazy directory creation with _ensure_storage_dir() that defers initialization to first use and enforces restrictive permissions (0o700).
Attachment Save Implementation core/attachment_storage.py	save_attachment() derives extension/stem from the sanitized filename when provided, constructs a UUID-suffixed stored name (fallback attachment), writes bytes, sets metadata "filename" to the stored name and "original_filename" to the raw input.
Gmail Integration gmail/gmail_tools.py	get_gmail_attachment_content() extracts the actual saved filename from result.path and includes "Saved filename: …" in the success response while retaining the original "Filename: …" line.
Tests & Validation tests/gmail/test_attachment_fix.py, tests/gmail/test_get_gmail_attachment_content.py	Adds parametrized unit tests validating sanitization of Windows reserved names and invalid characters (binary integrity checks) and an async integration test verifying Gmail tooling reports sanitized saved filenames and stores valid files.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• taylorwilsdon/google_workspace_mcp#495: Both PRs modify core/attachment_storage.py's save_attachment implementation; this PR adds filename sanitization while #495 adjusted file-open flags for Windows binary handling.

Poem

🐰 I hop through filenames, tidy and spry,

I nibble colons, question marks goodbye,
Reserved names softened with a gentle tuck,
Saved bytes snug — no zero-byte muck,
Hooray — attachments land safe nearby!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is minimal ('Closes #772') and does not follow the required template. It lacks description of changes, type of change, testing information, and the required 'Allow edits from maintainers' checklist.	Complete the PR description using the template: add a summary of changes, select the type of change (bug fix), confirm tests were added/passed, and check the 'Allow edits from maintainers' checkbox.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: windows filename chars' clearly and concisely summarizes the main change: fixing handling of Windows-illegal filename characters in attachment saving.
Linked Issues check	✅ Passed	The code changes fully address issue #772 requirements: sanitizes Windows-illegal filename characters, preserves original filename in metadata, returns sanitized filename in response, and includes comprehensive tests validating the fix.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing issue #772: filename sanitization logic, updated metadata handling, and related tests. No unrelated or out-of-scope modifications were introduced.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issues/772

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

core/attachment_storage.py (1)

106-127: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Persist sanitized filename in metadata for consistency

save_attachment() now sanitizes the on-disk filename, but metadata still stores the original filename (Line 166). This creates a mismatch between what is saved and what later consumers read from metadata.

Suggested fix

-        self._metadata[file_id] = {
+        display_filename = safe_filename or f"attachment{extension}"
+        self._metadata[file_id] = {
             "file_path": str(file_path),
-            "filename": filename or f"attachment{extension}",
+            "filename": display_filename,
             "mime_type": mime_type or "application/octet-stream",
             "size": len(file_bytes),
             "created_at": datetime.now(),
             "expires_at": expires_at,
         }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/attachment_storage.py` around lines 106 - 127, The metadata currently
records the original filename while the stored file uses the
sanitized/save_name, causing a mismatch; in save_attachment (and where metadata
is built), replace the metadata 'filename' value with the actual on-disk name
(use save_name when you generate stem + file_id or safe_filename if you don't
append the UUID) so metadata reflects the sanitized/stored filename; optionally
keep the original user filename in a separate metadata field like
'original_filename' if you need to preserve it.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@core/attachment_storage.py`:
- Around line 106-127: The metadata currently records the original filename
while the stored file uses the sanitized/save_name, causing a mismatch; in
save_attachment (and where metadata is built), replace the metadata 'filename'
value with the actual on-disk name (use save_name when you generate stem +
file_id or safe_filename if you don't append the UUID) so metadata reflects the
sanitized/stored filename; optionally keep the original user filename in a
separate metadata field like 'original_filename' if you need to preserve it.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9aeb522d-2520-4c0f-b1b2-ceed6b43b461

📥 Commits

Reviewing files that changed from the base of the PR and between 9d69115 and cb679b3.

📒 Files selected for processing (4)

• core/attachment_storage.py
• gmail/gmail_tools.py
• tests/gmail/test_attachment_fix.py
• tests/gmail/test_get_gmail_attachment_content.py