From c5c3ab2d3b5d6d1ccd8e6acf1b3353e5cf1b0916 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20Brandst=C3=A4tter?= Date: Tue, 13 Oct 2026 09:23:52 +0200 Subject: [PATCH] fix(storage): allow currency and unicode symbols in file path sanitization --- backend/app/files/storage.py | 10 ++--- backend/tests/test_storage.py | 34 +++++++++++++++++ .../.openspec.yaml | 2 + .../design.md | 37 +++++++++++++++++++ .../proposal.md | 21 +++++++++++ .../specs/file-storage/spec.md | 12 ++++++ .../tasks.md | 5 +++ 7 files changed, 116 insertions(+), 5 deletions(-) create mode 100644 openspec/changes/fix-storage-symbol-path-sanitization/.openspec.yaml create mode 100644 openspec/changes/fix-storage-symbol-path-sanitization/design.md create mode 100644 openspec/changes/fix-storage-symbol-path-sanitization/proposal.md create mode 100644 openspec/changes/fix-storage-symbol-path-sanitization/specs/file-storage/spec.md create mode 100644 openspec/changes/fix-storage-symbol-path-sanitization/tasks.md diff --git a/backend/app/files/storage.py b/backend/app/files/storage.py index 49a2ec4..a9c621c 100644 --- a/backend/app/files/storage.py +++ b/backend/app/files/storage.py @@ -12,8 +12,8 @@ log = logging.getLogger(__name__) # Safe path segment: letters, numbers, common punctuation. No / \ (traversal). -# Allow: . _ - space ( ) + ~ # ! & ' , ; = [ ] @ for "File (1).txt", "user@host.txt", etc. -_SAFE_SEGMENT_ASCII = re.compile(r"^[a-zA-Z0-9_. \-()+~#!&',;=\[\]@]+$") +# Allow: . _ - space ( ) + ~ # ! & ' , ; = [ ] @ $ for "File (1).txt", "user@host.txt", "Invoice_$100.pdf", etc. +_SAFE_SEGMENT_ASCII = re.compile(r"^[a-zA-Z0-9_. \-()+~#!&',;=\[\]@$]+$") # Email used as folder name: allow @, dots, and + (plus aliasing) _SAFE_EMAIL = re.compile(r"^[a-zA-Z0-9_.@+-]+$") @@ -29,11 +29,11 @@ def _is_safe_path_char(c: str) -> bool: return False # % can be used in encoding/URLs; keep path segments safe if ord(c) < 32: return False - if ("a" <= c <= "z") or ("A" <= c <= "Z") or ("0" <= c <= "9") or c in "_. -()+~#!&',;=[]@": + if ("a" <= c <= "z") or ("A" <= c <= "Z") or ("0" <= c <= "9") or c in "_. -()+~#!&',;=[]@$": return True cat = unicodedata.category(c) - # Letter, Number, or Punctuation (e.g. fullwidth parentheses （） in "Manual（CN）.pdf") - return cat.startswith("L") or cat.startswith("N") or cat.startswith("P") + # Letter, Number, Punctuation (e.g. fullwidth parentheses （） in "Manual（CN）.pdf"), or Symbol (e.g. €, £, ¥, ©, ™) + return cat.startswith("L") or cat.startswith("N") or cat.startswith("P") or cat.startswith("S") def _sanitize_segment(segment: str) -> Optional[str]: diff --git a/backend/tests/test_storage.py b/backend/tests/test_storage.py index 7d09a7b..beb51ad 100644 --- a/backend/tests/test_storage.py +++ b/backend/tests/test_storage.py @@ -167,6 +167,40 @@ def test_resolve_user_path_accepts_fullwidth_parens(monkeypatch) -> None: assert got == Path("/data/u@x.co/Documents/3D print/1. 3D Printer User Manual/Ender-3 V2-SM-002_User Manual（CN）.pdf") +def test_resolve_user_path_accepts_dollar_and_currency_symbols(monkeypatch) -> None: + """Path segments with $ and Unicode currency symbols (€, £, ¥) are accepted.""" + from app.files import storage + mock_settings = MagicMock() + mock_settings.storage_base_path = Path("/data") + monkeypatch.setattr(storage, "get_settings", lambda: mock_settings) + + for path in [ + "Finances/Invoice_$100.pdf", + "Finances/Price_€50.pdf", + "Finances/Statement_£25.xlsx", + "Finances/Budget_¥1000.txt", + ]: + got = resolve_user_path("u@x.co", path) + assert got == Path(f"/data/u@x.co/{path}") + + +def test_resolve_user_path_accepts_office_lockfile_and_symbols(monkeypatch) -> None: + """Path segments with office lockfiles (~$) and symbols (©, ™, °) are accepted.""" + from app.files import storage + mock_settings = MagicMock() + mock_settings.storage_base_path = Path("/data") + monkeypatch.setattr(storage, "get_settings", lambda: mock_settings) + + for path in [ + "Docs/~$ProjectPlan.docx", + "Docs/Copyright_©2024.pdf", + "Docs/Trademark_Brand™_v1.txt", + "Videos/VR_360°.mp4", + ]: + got = resolve_user_path("u@x.co", path) + assert got == Path(f"/data/u@x.co/{path}") + + def test_user_base_path_rejects_invalid_email(monkeypatch) -> None: """Email with slash or empty raises ValueError.""" from app.files import storage diff --git a/openspec/changes/fix-storage-symbol-path-sanitization/.openspec.yaml b/openspec/changes/fix-storage-symbol-path-sanitization/.openspec.yaml new file mode 100644 index 0000000..5fcc9ec --- /dev/null +++ b/openspec/changes/fix-storage-symbol-path-sanitization/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-13 diff --git a/openspec/changes/fix-storage-symbol-path-sanitization/design.md b/openspec/changes/fix-storage-symbol-path-sanitization/design.md new file mode 100644 index 0000000..943159b --- /dev/null +++ b/openspec/changes/fix-storage-symbol-path-sanitization/design.md @@ -0,0 +1,37 @@ +## Context + +`backend/app/files/storage.py` sanitizes path segments via `_sanitize_segment` and validates paths with `resolve_user_path`. +Currently, `_SAFE_SEGMENT_ASCII` regex is: +`r"^[a-zA-Z0-9_. \-()+~#!&',;=\[\]@]+$"` +and Unicode character checking in `_is_safe_path_char` is: +```python +cat = unicodedata.category(ch) +return cat.startswith("L") or cat.startswith("N") or cat.startswith("P") +``` +This excludes the ASCII character `$` and any Unicode character in the Symbol (`S`) category (which includes `Sc` Currency, `Sm` Math, `Sk` Modifier, `So` Other). +Filenames with dollar signs (e.g. `Invoice_$100.pdf` or `~$Document.docx`), euro signs (`Budget_€50.xlsx`), etc., fail sanitization and raise `ValueError("Unsafe path segment")`, returning HTTP 400 Bad Request to clients and breaking desktop sync loops. + +## Goals / Non-Goals + +**Goals:** +- Allow `$` in ASCII segment matching. +- Allow `$` and Unicode category `S` (symbols, including currency symbols) in `_is_safe_path_char`. +- Ensure directory traversal (`..`, `.`, `/`, `\`), null bytes, control characters, and reserved device names remain strictly disallowed. + +**Non-Goals:** +- Changing path separators or directory nesting structure. +- Modifying client sync upload logic. + +## Decisions + +- **Decision 1: Include `$` in `_SAFE_SEGMENT_ASCII` and `_is_safe_path_char`** + - Rationale: `$` is a common and standard filename character in modern operating systems (Linux, macOS, Windows) and commonly used in Office temporary files (`~$...`) and financial documents. It poses no filesystem traversal hazard. + - Alternative considered: Escaping or stripping `$` - rejected because filenames must match on both client and server for hash-based sync. + +- **Decision 2: Include Unicode category `S` in `_is_safe_path_char`** + - Rationale: Unicode category `S` covers Currency (`Sc` like `€`, `£`, `¥`), Math (`Sm`), Modifier (`Sk`), and Other symbols (`So` like `©`, `®`, `°`). Allowing `cat.startswith("S")` in addition to Letters (`L`), Numbers (`N`), and Punctuation (`P`) correctly allows legitimate international symbols while continuing to reject control characters (`C`), formatting characters (`Cf`), marks (`M`), or whitespace separators (`Z`, handled explicitly as spaces). + +## Risks / Trade-offs + +- [Risk] Permitting additional characters could unintentionally allow shell or injection characters → + - Mitigation: BrandyBox uses Python's standard `pathlib.Path` and direct filesystem system calls (no shell execution / `subprocess` calls on file paths), and directory separators (`/`, `\`) and traversal (`..`, `.`) remain strictly blocked by earlier checks. diff --git a/openspec/changes/fix-storage-symbol-path-sanitization/proposal.md b/openspec/changes/fix-storage-symbol-path-sanitization/proposal.md new file mode 100644 index 0000000..f3b5865 --- /dev/null +++ b/openspec/changes/fix-storage-symbol-path-sanitization/proposal.md @@ -0,0 +1,21 @@ +## Why + +Users who sync or upload files containing currency symbols (such as `$`, `€`, `£`, `¥`) or Unicode symbols (such as `©`, `™`, `°`), or temporary Office lockfiles (such as `~$Document.docx`), currently experience HTTP 400 Bad Request errors (`Unsafe path segment`) and perpetual desktop client sync failures. The path segment sanitization regex and Unicode validation reject these safe characters unnecessarily. + +## What Changes + +- Permit `$` and Unicode Symbol categories (`S`, including `Sc`, `Sm`, `Sk`, `So`) in backend storage path segment sanitization and validation. +- Allow filenames such as `Invoice_$100.pdf`, `Budget_€50.xlsx`, and `~$Doc.docx` to be safely resolved and stored without compromising path traversal safeguards. + +## Capabilities + +### New Capabilities + +### Modified Capabilities +- `file-storage`: Update path sanitization requirements to allow currency and Unicode symbol characters in path segments. + +## Impact + +- `backend/app/files/storage.py`: Update `_SAFE_SEGMENT_ASCII` regex and `_is_safe_path_char` Unicode category filter. +- `backend/tests/test_storage.py`: Add unit test coverage for filenames with currency symbols and other symbols. +- Desktop client sync will no longer reject uploads of documents containing currency symbols or Office lockfile prefixes. diff --git a/openspec/changes/fix-storage-symbol-path-sanitization/specs/file-storage/spec.md b/openspec/changes/fix-storage-symbol-path-sanitization/specs/file-storage/spec.md new file mode 100644 index 0000000..ad694b7 --- /dev/null +++ b/openspec/changes/fix-storage-symbol-path-sanitization/specs/file-storage/spec.md @@ -0,0 +1,12 @@ +## ADDED Requirements + +### Requirement: Path Sanitization Allows Currency and Unicode Symbols +The storage layer SHALL allow file and directory path segments containing ASCII currency symbol `$` and Unicode symbols (such as currency symbols `€`, `£`, `¥` and common symbols `©`, `™`, `°`) as well as Office temporary lockfiles (such as `~$Document.docx`) to be safely resolved and stored without raising validation errors. + +#### Scenario: User uploads file with currency symbol +- **WHEN** an authenticated user uploads a file with a name containing `$` or Unicode currency symbols like `Invoice_$100.pdf` or `Receipt_€50.pdf` +- **THEN** the system resolves the storage path successfully and accepts the upload instead of returning HTTP 400 Bad Request + +#### Scenario: User syncs Office temporary lockfile +- **WHEN** a client uploads or syncs a temporary file starting with `~$` such as `~$Project.docx` +- **THEN** the system resolves the storage path successfully and allows the operation without raising an unsafe path segment error diff --git a/openspec/changes/fix-storage-symbol-path-sanitization/tasks.md b/openspec/changes/fix-storage-symbol-path-sanitization/tasks.md new file mode 100644 index 0000000..0c182e6 --- /dev/null +++ b/openspec/changes/fix-storage-symbol-path-sanitization/tasks.md @@ -0,0 +1,5 @@ +## 1. Storage Path Sanitization Updates + +- [x] 1.1 Update `_SAFE_SEGMENT_ASCII` regex and `_is_safe_path_char` in `backend/app/files/storage.py` to allow `$` and Unicode category `S` symbols +- [x] 1.2 Add unit tests in `backend/tests/test_storage.py` covering currency symbols (`$`, `€`, `£`), lockfiles (`~$Doc.docx`), and Unicode symbols, and verify with pytest +- [x] 1.3 Run full QA test suite via `./scripts/run-qa.sh` to ensure all tests pass