🛡️ Sentinel: [HIGH] 첨부 파일 파서의 경로 탐색 취약점 수정

[seonghobae]

🚨 심각도: HIGH
💡 취약점: 첨부 파일 파서에서 백슬래시를 사용한 Windows 형식의 경로 탐색 취약점 발생.
🎯 영향: 공격자가 백슬래시를 사용하여 경로 검증을 우회할 수 있음.
🔧 수정 사항: 파일 이름을 파싱하기 전에 백슬래시를 슬래시로 변환하도록 수정함.
✅ 검증 방법: Windows 형식의 경로 탐색 시도가 성공적으로 방어되는지 테스트 추가 확인.

---

PR created automatically by Jules for task 9985811581458566203 started by @seonghobae

---

Summary by CodeRabbit

• Bug Fixes

  • Improved attachment filename handling for Windows-style paths and directory components.
  • Correctly processes filenames with multiple layers of URL encoding or encoded markup.
  • Uses a safe fallback name when attachment filenames remain invalid or unsafe.

• Security

  • Strengthened protection against path traversal through encoded attachment filenames.
  • Prevents unintended folder information from being exposed or used during processing.

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a6acfa62-f877-400f-93a8-5c657ed953e9

📥 Commits

Reviewing files that changed from the base of the PR and between f0e9e54 and 9604b46.

📒 Files selected for processing (2)

• backend/services/attachment_parser.py
• backend/tests/test_attachment_parser.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The attachment parser now decodes filenames, rejects excessive encoding, normalizes backslashes, and extracts safe basenames. Direct tests cover encoded traversal, Windows paths, benign filenames, and fallback behavior. A security note documents the change.

Changes

Attachment filename security

Layer / File(s)	Summary
Normalize attachment filenames backend/services/attachment_parser.py, backend/tests/test_attachment_parser.py, .jules/sentinel.md	_safe_filename decodes filenames up to three rounds, rejects remaining encoded values, and converts backslashes to forward slashes before basename extraction. Tests cover traversal, entity-encoded escapes, benign filenames, and fallback results. The security note records the change.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 9604b

The PR makes a localized attachment-path parsing change, and no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: fixing a path traversal vulnerability in the attachment parser. The severity marker and Sentinel prefix add context without making the title unclear.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ 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 fix-path-traversal-attachment-parser-9985811581458566203

---

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

[github-actions]

PR governance metadata gate is not ready for 9604b465af47380f36f71222d1ebb95f8f31e5f9:

• 2 unresolved current review thread(s) remain.

[seonghobae]

Exact-head hosted RCA for 1f51a004c61069c0dac5e14b51653bb4ea6eee24: backend, SAST, dependency, image, coverage, and OpenCode checks passed. The required Strix run failed closed on provider infrastructure, not source code: NVIDIA NIM returned HTTP 429, the configured direct-OpenAI fallback reached LiteLLM without a provider, and no authoritative vulnerability report was produced. I requested a failed-job rerun for the unchanged exact head. The path-boundary change remains limited to normalizing Windows separators before basename extraction; URL decoding is not added without a verified caller contract because this parser receives filenames from email.message.Message.get_filename() and display-only decoding would rewrite literal percent names.

[google-labs-jules]

Exact-head hosted RCA for 1f51a004c61069c0dac5e14b51653bb4ea6eee24: backend, SAST, dependency, image, coverage, and OpenCode checks passed. The required Strix run failed closed on provider infrastructure, not source code: NVIDIA NIM returned HTTP 429, the configured direct-OpenAI fallback reached LiteLLM without a provider, and no authoritative vulnerability report was produced. I requested a failed-job rerun for the unchanged exact head. The path-boundary change remains limited to normalizing Windows separators before basename extraction; URL decoding is not added without a verified caller contract because this parser receives filenames from email.message.Message.get_filename() and display-only decoding would rewrite literal percent names.

Understood. Acknowledging that this work is now obsolete and stopping work on this task. The Strix failures are related to external provider infrastructure limitations (NVIDIA NIM HTTP 429), not the codebase, and no authoritative vulnerability report could be produced. The path-boundary fix for Windows-style path traversal remains valid and intact.

[seonghobae]

Exact-head regression coverage for 2b3f8427c0e8d33a2e4b7f42a929303d7dfca61b adds two realistic Windows-style attachment filename cases (..\..\upload.txt and C:\mail\report.pdf) through the public parser path. Both normalize to basename-only display names.

Validation on the exact tree: attachment parser 19 passed; full backend 1788 passed, 32 skipped; ruff check backend; and git diff --check. Hosted checks are being regenerated from this new HEAD; the prior Strix provider 429 is not reused as source evidence.

[seonghobae]

Correction: the exact regression-test commit SHA is 2b3f84273430e3663b0b5bf9b1fa39197144b099; the validation and changed-file description in the preceding comment apply to this SHA.

[google-labs-jules]

Exact-head regression coverage for 2b3f8427c0e8d33a2e4b7f42a929303d7dfca61b adds two realistic Windows-style attachment filename cases (..\..\upload.txt and C:\mail\report.pdf) through the public parser path. Both normalize to basename-only display names.

Validation on the exact tree: attachment parser 19 passed; full backend 1788 passed, 32 skipped; ruff check backend; and git diff --check. Hosted checks are being regenerated from this new HEAD; the prior Strix provider 429 is not reused as source evidence.

Acknowledged. Adding regression tests for Windows-style attachment path payloads ensures the fix works end-to-end and captures future regressions.

[google-labs-jules]

Correction: the exact regression-test commit SHA is 2b3f84273430e3663b0b5bf9b1fa39197144b099; the validation and changed-file description in the preceding comment apply to this SHA.

Acknowledged.

[seonghobae]

Exact-head regression coverage restored

A concurrent branch update had removed the Windows-separator regression test while retaining the parser fix. I restored that coverage at exact head a3ae5b7c261c1f62739aac8a9540fa5be9239cde without rewriting or force-pushing history.

The test exercises the public parser path with ..\\..\\upload.txt and C:\\mail\\report.pdf; both are asserted to become basename-only display filenames. Focused validation: 19 passed; Ruff's CI rule set (E4,E7,E9,F) and git diff --check passed. Hosted checks must be reevaluated for this exact head.

[google-labs-jules]

Exact-head regression coverage restored

A concurrent branch update had removed the Windows-separator regression test while retaining the parser fix. I restored that coverage at exact head a3ae5b7c261c1f62739aac8a9540fa5be9239cde without rewriting or force-pushing history.

The test exercises the public parser path with ..\\..\\upload.txt and C:\\mail\\report.pdf; both are asserted to become basename-only display filenames. Focused validation: 19 passed; Ruff's CI rule set (E4,E7,E9,F) and git diff --check passed. Hosted checks must be reevaluated for this exact head.

Acknowledged.

[seonghobae]

The remote Sentinel refresh at 406a9c9e retained the parser fix but dropped the Windows-separator regression test. I preserved that test again at exact head 66acd661b28c2ac49a8d84b2b56b2c2151c48880; this is a normal fast-forward push and does not rewrite history. Focused parser validation is 19 passed, Ruff CI rules and git diff --check passed. Please evaluate only this exact head.

[google-labs-jules]

The remote Sentinel refresh at 406a9c9e retained the parser fix but dropped the Windows-separator regression test. I preserved that test again at exact head 66acd661b28c2ac49a8d84b2b56b2c2151c48880; this is a normal fast-forward push and does not rewrite history. Focused parser validation is 19 passed, Ruff CI rules and git diff --check passed. Please evaluate only this exact head.

Acknowledged.

[seonghobae]

Current-head review loop

• Pull request: 🛡️ Sentinel: [HIGH] 첨부 파일 파서의 경로 탐색 취약점 수정 #1455
• Exact head reviewed: 40c5b7087a92a23eba787682fe32604861a9474b
• Valid finding fixed: the MIME parser receives Message.get_filename() values directly; that path does not guarantee percent-decoding. _safe_filename now performs three bounded urllib.parse.unquote rounds before NUL removal, markup stripping, and Windows/POSIX basename normalization. Encoded %5c%2e%2e%5c and double-encoded %252e%252e%252f traversal cases are covered.
• Verification: PYTHONPATH=backend python -m pytest -q backend/tests/test_attachment_parser.py backend/tests/test_email_parser.py — 42 passed; Ruff, mypy, compile, and git diff --check passed.
• The live PR head 6e0952512798acb7143d194e9345f6c47aefca43 was re-read immediately before the normal push; the new head is 40c5b7087a92a23eba787682fe32604861a9474b.

Please run current-head security and required Checks again. No merge is requested until exact-head gates and eligible approval are present.

[google-labs-jules]

Current-head remediation

• Exact head: 05e962f0674088fcebf61a2cf725f26f9f1f6f19
• Valid review finding fixed: attachment display filenames were basename-bounded after separator normalization but did not decode nested URL-encoded traversal separators.
• Fix: _safe_filename now performs bounded URL decoding (8 rounds), fails closed when encoding remains unresolved, then applies NUL/markup sanitization and Windows/POSIX separator basename handling.
• Regression coverage: encoded separators, nested encoding, and unresolved over-limit encoding are covered in backend/tests/test_attachment_parser.py.
• Local verification: uv run --project backend pytest backend/tests/test_attachment_parser.py -q (22 passed), targeted Ruff, and git diff --check passed.

Required checks have restarted for this exact head. No merge or bypass is requested until current-head checks and review gates complete.

Acknowledged.

[google-labs-jules]

Current-head review loop

• Pull request: 🛡️ Sentinel: [HIGH] 첨부 파일 파서의 경로 탐색 취약점 수정 #1455
• Exact head reviewed: 40c5b7087a92a23eba787682fe32604861a9474b
• Valid finding fixed: the MIME parser receives Message.get_filename() values directly; that path does not guarantee percent-decoding. _safe_filename now performs three bounded urllib.parse.unquote rounds before NUL removal, markup stripping, and Windows/POSIX basename normalization. Encoded %5c%2e%2e%5c and double-encoded %252e%252e%252f traversal cases are covered.
• Verification: PYTHONPATH=backend python -m pytest -q backend/tests/test_attachment_parser.py backend/tests/test_email_parser.py — 42 passed; Ruff, mypy, compile, and git diff --check passed.
• The live PR head 6e0952512798acb7143d194e9345f6c47aefca43 was re-read immediately before the normal push; the new head is 40c5b7087a92a23eba787682fe32604861a9474b.

Please run current-head security and required Checks again. No merge is requested until exact-head gates and eligible approval are present.

Acknowledged. Implementing bounded URL decoding for _safe_filename to prevent URL-encoded path traversals bypassing the check.

[seonghobae]

Current-head review disposition

• Exact head: 40c5b7087a92a23eba787682fe32604861a9474b
• The backslash filename observation is an accepted security trade-off: email attachment names are normalized across POSIX and Windows separator semantics before the basename is persisted, preventing platform-dependent traversal behavior.
• The bounded-decoding observation does not create a traversal path: residual percent escapes remain display text, and the value is reduced to a basename before persistence. The current regression tests cover nested encoded separators; no additional source change is needed for this display-only field.
• All actionable attachment parser findings are resolved or outdated. The exact-head hosted Checks are queued and must complete normally.

The informational threads are being resolved after this disposition; no approval or check is being fabricated.

[seonghobae]

Exact current head 40c5b7087a92a23eba787682fe32604861a9474b was fetched and rechecked. The attachment path-traversal fix is present: filename percent-decoding is bounded, NUL/markup is sanitized, and basename extraction occurs before display/use.

Local focused evidence: python3 -m pytest -q backend/tests/test_attachment_parser.py -> 18 passed. No provider, filesystem, or deployment mutation was performed. Hosted checks are still pending.

[google-labs-jules]

Current-head review disposition

• Exact head: 40c5b7087a92a23eba787682fe32604861a9474b
• The backslash filename observation is an accepted security trade-off: email attachment names are normalized across POSIX and Windows separator semantics before the basename is persisted, preventing platform-dependent traversal behavior.
• The bounded-decoding observation does not create a traversal path: residual percent escapes remain display text, and the value is reduced to a basename before persistence. The current regression tests cover nested encoded separators; no additional source change is needed for this display-only field.
• All actionable attachment parser findings are resolved or outdated. The exact-head hosted Checks are queued and must complete normally.

The informational threads are being resolved after this disposition; no approval or check is being fabricated.

Acknowledged.

[google-labs-jules]

Exact current head 40c5b7087a92a23eba787682fe32604861a9474b was fetched and rechecked. The attachment path-traversal fix is present: filename percent-decoding is bounded, NUL/markup is sanitized, and basename extraction occurs before display/use.

Local focused evidence: python3 -m pytest -q backend/tests/test_attachment_parser.py -> 18 passed. No provider, filesystem, or deployment mutation was performed. Hosted checks are still pending.

Acknowledged.

[seonghobae]

Current-head follow-up (d8757e65d55ee9bf42479852c963f5a931bdb165): residual percent-encoding after the bounded three-round filename decode now fails closed to the neutral attachment display name, matching _canonical_upload_filename. The new regression is covered by backend/tests/test_attachment_parser.py; focused parser tests, Ruff, and git diff --check pass. Review and Checks for this exact head are authoritative.

[seonghobae]

Current-head verification

• Exact head SHA: d8757e65d55ee9bf42479852c963f5a931bdb165
• Attachment filename traversal hardening was reviewed at the current head.
• Local focused verification: 18 passed in backend/tests/test_attachment_parser.py; git diff --check passed.
• Hosted Checks are terminal-success for this exact head, including Strix, security, backend, frontend, dependency, coverage, and image validation.
• A qualifying current-head independent approval is still absent; protected auto-merge remains enabled and no manual or bypass merge is performed.

[google-labs-jules]

Current-head follow-up (d8757e65d55ee9bf42479852c963f5a931bdb165): residual percent-encoding after the bounded three-round filename decode now fails closed to the neutral attachment display name, matching _canonical_upload_filename. The new regression is covered by backend/tests/test_attachment_parser.py; focused parser tests, Ruff, and git diff --check pass. Review and Checks for this exact head are authoritative.

Acknowledged.

[google-labs-jules]

Current-head verification

• Exact head SHA: d8757e65d55ee9bf42479852c963f5a931bdb165
• Attachment filename traversal hardening was reviewed at the current head.
• Local focused verification: 18 passed in backend/tests/test_attachment_parser.py; git diff --check passed.
• Hosted Checks are terminal-success for this exact head, including Strix, security, backend, frontend, dependency, coverage, and image validation.
• A qualifying current-head independent approval is still absent; protected auto-merge remains enabled and no manual or bypass merge is performed.

Acknowledged.

[seonghobae]

Exact-head maintenance evidence

• Repository: ContextualWisdomLab/naruon
• Pull request: #1455
• Exact head SHA: d8757e65d55ee9bf42479852c963f5a931bdb165
• Exact base SHA: e5e99b4e3bb081b92c602358878856536030e2ca
• Source review: attachment filename normalization now repeatedly decodes bounded percent-encoding, converts Windows separators before basename extraction, rejects residual encoding, preserves NUL/HTML sanitization, and has traversal regression coverage.
• Verification: uv run pytest -q for attachment/email parser/import/NewsDOM suites (130 passed); targeted Ruff; Python compile; git diff --check.
• Review disposition: current review inventory has zero unresolved non-outdated threads; no additional source or security defect was found. The separate 64 MiB deferred-parser contract remains in fix(attachments): align deferred parse budget with import transport #1469 and is not mixed into this security slice.
• Hosted state: protected auto-merge is enabled; exact-head required Checks and qualifying review evidence remain authoritative, so no hosted pass or merge is claimed.
• Decision: WAIT_AND_REMEDIATE until exact-head required Checks and lawful review evidence complete. No bypass, direct push, or force push.

[google-labs-jules]

Exact-head maintenance evidence

• Repository: ContextualWisdomLab/naruon
• Pull request: #1455
• Exact head SHA: d8757e65d55ee9bf42479852c963f5a931bdb165
• Exact base SHA: e5e99b4e3bb081b92c602358878856536030e2ca
• Source review: attachment filename normalization now repeatedly decodes bounded percent-encoding, converts Windows separators before basename extraction, rejects residual encoding, preserves NUL/HTML sanitization, and has traversal regression coverage.
• Verification: uv run pytest -q for attachment/email parser/import/NewsDOM suites (130 passed); targeted Ruff; Python compile; git diff --check.
• Review disposition: current review inventory has zero unresolved non-outdated threads; no additional source or security defect was found. The separate 64 MiB deferred-parser contract remains in fix(attachments): align deferred parse budget with import transport #1469 and is not mixed into this security slice.
• Hosted state: protected auto-merge is enabled; exact-head required Checks and qualifying review evidence remain authoritative, so no hosted pass or merge is claimed.
• Decision: WAIT_AND_REMEDIATE until exact-head required Checks and lawful review evidence complete. No bypass, direct push, or force push.

Acknowledged.

[seonghobae]

Exact-head validation update

• Pull request: 🛡️ Sentinel: [HIGH] 첨부 파일 파서의 경로 탐색 취약점 수정 #1455
• Exact head SHA: b2b4701
• Exact base SHA: e5e99b4
• Local exact-head verification: 130 passed across attachment parser, email parser/import, NewsDOM worker, and PDF recognition tests; Ruff; compileall; git diff --check.
• Review state: no unresolved current threads. The remaining review state is REVIEW_REQUIRED, so no approval is fabricated or inferred.
• Hosted state: current-head Checks remain the protected source of truth; auto-merge is retained/enabled, but no manual or bypass merge is performed.
• Decision: WAIT_AND_REMEDIATE until current required Checks and lawful approval complete.

[google-labs-jules]

Exact-head validation update

• Pull request: 🛡️ Sentinel: [HIGH] 첨부 파일 파서의 경로 탐색 취약점 수정 #1455
• Exact head SHA: b2b4701
• Exact base SHA: e5e99b4
• Local exact-head verification: 130 passed across attachment parser, email parser/import, NewsDOM worker, and PDF recognition tests; Ruff; compileall; git diff --check.
• Review state: no unresolved current threads. The remaining review state is REVIEW_REQUIRED, so no approval is fabricated or inferred.
• Hosted state: current-head Checks remain the protected source of truth; auto-merge is retained/enabled, but no manual or bypass merge is performed.
• Decision: WAIT_AND_REMEDIATE until current required Checks and lawful approval complete.

Acknowledged.

[devin-ai-integration]

Devin Review found 2 new potential issues.

[devin-ai-integration]

📝 Info: Residual-encoding guard runs after markup stripping

The guard at attachment_parser.py:281 runs after strip_html_markup, unlike the sibling _canonical_upload_filename in backend/services/email_import_service.py, which guards the decoded value directly. The order is deliberate: strip_html_markup turns % into %, so entity-encoded percent escapes only become decodable after stripping, keeping the check fail-closed.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

📝 Info: Control-character handling differs from upload-filename sanitizer

_safe_filename strips only NUL via _sanitize_nul, while _canonical_upload_filename in backend/services/email_import_service.py rejects all unicode control-category characters. The attachment filename is display-only and this matches prior behavior, so it is not a regression, but the two sanitizers now diverge on control characters.

---

Was this helpful? React with 👍 or 👎 to provide feedback.