Fix/filename

[Ahmath-Gadji]

This PR adds a sanitize_filename utility function to clean and normalize uploaded filenames, preventing issues from special characters and ensuring consistent file naming.

Changes:

• New sanitize_filename function removes special characters, normalizes separators to underscores, and handles edge cases
• unittests added

Summary by CodeRabbit

• New Features

  • Implemented automatic filename sanitization for uploaded files, normalizing and removing disallowed characters to improve file handling robustness.

• Tests

  • Added comprehensive test coverage for filename sanitization, including edge cases with special characters and multiple spaces.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This change introduces a new sanitize_filename utility function that normalizes filenames by replacing disallowed characters with underscores and collapsing consecutive underscores. The function is integrated into the file upload validation flow in the router and covered with comprehensive parameterized tests.

Changes

Cohort / File(s)	Summary
Core sanitization utility openrag/components/files.py	Added sanitize_filename(filename: str) -> str function that normalizes filenames by splitting name/extension, replacing disallowed characters, collapsing underscores, and trimming. Added imports for asyncio, re, secrets, time, and typing.
Test coverage expansion openrag/components/test_files.py	Added import for sanitize_filename and new parameterized test suite test_sanitize_filename covering edge cases (spaces, special characters, multiple underscores, boundary inputs). Reformatted existing test invocation.
Router integration openrag/routers/utils.py	Added import for sanitize_filename and integrated sanitization into the file upload flow to normalize uploaded filenames post-validation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

• Review the regex pattern and character replacement logic in sanitize_filename for correctness and edge case handling
• Verify parameterized test cases cover the intended normalization behavior
• Confirm the router integration preserves validation logic and applies sanitization at the correct point in the flow

Poem

🐰 A filename walks in, so messy and wild,
With spaces and slashes, not safe for a file!
We sanitize, normalize, make it pristine,
Underscores collapse, the cleanest you've seen,
No more broken uploads—the filesystem smiles! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The title 'Fix/filename' is vague and uses a generic format that doesn't clearly convey what the specific change accomplishes; it lacks meaningful description of the actual improvement or fix.	Revise the title to be more descriptive, such as 'Add filename sanitization utility for uploaded files' to clearly communicate the main change.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

---

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.}

[paultranvan]

Is is a problem to have multiple underscore? I know I have files with them, this case seems legit to me

[Ahmath-Gadji]

openrag/openrag/components/files.py

Lines 21 to 22 in 8bd8f1c

	# Remove special characters (keep only word characters and hyphens temporarily)
	name = re.sub(r"[^\w\-]", "_", name)

With this code, if there are consecutive special characters, each one is replaced with “_”, which can result in filenames containing many underscores. I added that line for further cleaning, but having multiple underscores doesn’t cause any issues during indexing.

[paultranvan]

Is hyphens really problematic ? Lot of files have it

[Ahmath-Gadji]

This isn’t an issue during indexation, but it becomes one with Chainlit.

For example, the filename allocution_de_Mr_trump,__au_COP21_-_Hello_20251119065407.txt indexes correctly, but when displayed in the Chainlit UI, some underscores are interpreted as Markdown italics markers, breaking the file links.

• No issue with filenames like:
  allocution_de_Mr_trump,__au_COP21__Hello_20251119065407.txt (only underscores)
  allocution_de_Mr_trump,-au-COP21-Hello-20251119065407.txt (only hyphens)

• The issue happens specifically with:
  allocution_de_Mr_trump,__au_COP21_-_Hello_20251119065407.txt
  as shown in the screenshot.

This suggests that Chainlit doesn’t fail on _ or - individually, but on certain combinations of both.

To keep things simple, we can sanitize the filename for safe display and during indexation. In another file metadata we can store the original name, such as "original_filename".

[paultranvan]

Ok, let's keep it like this then and add the original_filename

[paultranvan]

Could you add a test with . in file name? 🙏

[Ahmath-Gadji]

Added

[paultranvan]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)

openrag/components/files.py (1)

73-94: Avoid mutable default {} for metadata in serialize_file

Using {} as a default argument is a Python anti-pattern that can cause subtle bugs. Since the function parameter is typed as Optional[Dict], use None as the default and initialize inside the function:

-async def serialize_file(task_id: str, path: str, metadata: Optional[Dict] = {}):
+async def serialize_file(task_id: str, path: str, metadata: Optional[Dict] = None):

@@
-    serializer = ray.get_actor("DocSerializer", namespace="openrag")
-    # Kick off the remote task
-    future = serializer.serialize_document.remote(task_id, path, metadata=metadata)
+    serializer = ray.get_actor("DocSerializer", namespace="openrag")
+    # Kick off the remote task
+    if metadata is None:
+        metadata = {}
+    future = serializer.serialize_document.remote(task_id, path, metadata=metadata)

Note: serialize_document in openrag/components/indexer/loaders/serializer.py has the same issue and should be fixed.

♻️ Duplicate comments (2)

openrag/components/files.py (1)

13-34: sanitize_filename is reasonable; consider Path-based splitting and revisiting hyphen/underscore normalization

The sanitizer correctly strips problematic characters, normalizes separators, and is idempotent. Two optional refinements to consider:

• Use Path(filename) (stem / suffix) for name/extension splitting to handle edge cases more robustly, as previously suggested.
• Re-evaluate whether converting all hyphens to underscores and collapsing multiple underscores is desired, since both are valid and common in filenames; if not strictly needed, you could keep those characters as-is while still replacing truly problematic ones.

openrag/components/test_files.py (1)

60-82: Good sanitizer test coverage; add cases with dots and path separators

The parametrized tests nicely cover spaces, special characters, multiple underscores, and an empty string. To fully exercise sanitize_filename and address the earlier request about dots, consider adding cases like:

• ("file.v1.txt", "file_v1.txt") # dot in basename
• (".env", "env") # leading dot
• ("folder/file.txt", "folder_file.txt") # path separator

These will lock in behavior for common edge cases and protect against regressions.

🧹 Nitpick comments (1)

openrag/routers/utils.py (1)

213-236: In‑place filename sanitization is good; consider preserving the original name if needed

Sanitizing file.filename in place after format checks is a good point to normalize names and ensures downstream code (e.g. disk writes) sees the sanitized value. If any part of the system needs to display or log the raw user‑provided filename, consider storing it separately (e.g. in metadata["original_filename"]) before overwriting file.filename.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 61e1db1 and 8bd8f1c.

📒 Files selected for processing (3)

• openrag/components/files.py (1 hunks)
• openrag/components/test_files.py (3 hunks)
• openrag/routers/utils.py (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

openrag/routers/utils.py (1)

openrag/components/files.py (1)

• sanitize_filename (13-34)

openrag/components/test_files.py (1)

openrag/components/files.py (2)

• sanitize_filename (13-34)
• save_file_to_disk (44-70)

🔇 Additional comments (2)

openrag/routers/utils.py (1)

7-7: Import of sanitize_filename is consistent and scoped correctly

Bringing sanitize_filename in via from components.files import sanitize_filename matches existing import style and keeps routers using the shared utility; no issues here.

openrag/components/test_files.py (1)

5-7: Test imports and save_file_to_disk usage look correct

Updating imports to pull sanitize_filename and save_file_to_disk from components.files and simplifying the save_file_to_disk call keeps the tests aligned with the production module; behavior is unchanged and clear.

Also applies to: 19-23