fix(attachments): preserve literal MIME filename identity

[seonghobae]

Defect

Protected develop@042b0c70531b229af3acbd0421a2f23098d848b3 allowed MIME filename text to change parser selection through representation re-decoding. The predecessor _safe_filename() percent-decoded URL escapes and HTML-decoded character references even though the email parser supplies filename identity, not URL or HTML source. Under generic application/octet-stream, literals such as quarterly%2Ejson or quarterly.json could therefore become .json parser authority.

Production ingress also proved that raw angle-bracket filename text, NUL, RFC 2231-decoded line controls, and Unicode bidirectional formatting controls can reach the attachment filename boundary. Display/storage identity and generic-MIME parser authority therefore cannot share a normalization pipeline, and unsafe control semantics must fail closed rather than being deleted or retained.

The latest production probe uses RFC 2231 filename*=utf-8''quarterly%E2%80%AEfdp.json. Python policy.default exposes that as the logical filename quarterly\u202efdp.json; U+202E RIGHT-TO-LEFT OVERRIDE changes visual ordering while Path(...).suffix still reports .json. The fix rejects Unicode Bidi_Control characters at the filename trust boundary without rejecting ordinary Arabic/Hebrew or other RTL-script text.

TDD lineage

• protected base: develop@042b0c70531b229af3acbd0421a2f23098d848b3;
• RED 3d401b327aeb017972d6f33251f7f2a3d9ec60d4: literal quarterly%2Ejson must remain unsupported_binary under generic MIME;
• causal fix e6565aeae8049b77994b49abca0256ca72d326fc: remove URL percent-decoding from the MIME filename boundary;
• alignment 0b01f6ba305d49a2deb662480e2358f16e0bf089: align pre-existing percent-escape tests to the corrected representation contract;
• RED 63fa5d737afc1c4a22faf2f92628aaa3c3c51416: character-reference text cannot synthesize parser authority;
• causal fix c4140c125b7956d33593ddeddbf1c11df0171d89: stop HTML/entity re-decoding from deciding generic-MIME parser selection;
• alignment f2626bcf26bda30575abee170321edc548a83296: align earlier filename tests with literal entity handling;
• RED dd6976d97f55e7145f4ba9b8dd053974fb4e4404: unknown <Q4>.json must not retain generic-MIME extension authority;
• causal fix 1261589e95be1ea9bf77360243c1aff7ed287bf7: reject raw angle-bracket parser authority;
• discarded experiment 026a8e8111bec393032d02196bca7ac4e1b76439 / c32786ac078fcf4c4681e2afa72b5386376efdae: the trailing-space hypothesis was rejected after checking real stdlib ingress; source/test delta was non-force reverted in ab263a954709c8e726dbd5478f33097776a8eb2b / 232f6448447e968df089cee21a68febfb903816a;
• production RED e68c017e40e705b4cca17c305acc547602b10850: raw multipart EML with filename="quarterly.json\x00" proves NUL deletion can fabricate .json authority;
• causal fix f17846ed37d7ad15a97acadd1d74ee21659ddee7: NUL-bearing filename identity fails closed;
• RED 40eb882e49eac1b7102794b45074a6d389dcea83 plus alignment 0e7df51292fab0efd4ad5e286f11991cdd7dbe11: safe display sanitization must not become parser authority while preserving the established display contract;
• causal fix fea71c7fc2b49d7d47b0c91862786bccddd29d07: split _safe_filename() display/storage projection from _parser_authority_filename() extension authority;
• production RED 288136ef8b1a6ffd4c1d910fcd9654d25671f9d1: RFC 2231 %0A decoding plus C0/C1 probes show control-bearing filenames retain .json authority on the predecessor;
• causal fix 92ec6151d11d9b125880a0405dab9ef59bc9293a: C0, DEL, and C1 controls fail closed before both projections;
• standards traceability 4a7b6ec4216bc74d5b944aef1c0e53fe5b504f9d: RFC 2183/RFC 2231 ownership and production ingress evidence;
• production RED 51f45492309b40799d7a026c52688a6a999a043e: add real RFC 2231 U+202E ingress plus all twelve Unicode Bidi_Control cases and prove ordinary RTL-script filenames remain allowed;
• causal fix ddc11593ac5af1528c9068f3275be728e28f3dc8: reject the UAX chore(deps): bump pytest from 8.0.0 to 9.0.3 in /backend in the pip group across 1 directory #9 Bidi_Control code-point set through the existing control boundary;
• current standards/decision traceability 2eaf6134434a2ad29ad8fe0365aa1b34b848dd5f: document the Unicode display-order threat, exact RED/fix, tests, and APA 7th UAX chore(deps): bump pytest from 8.0.0 to 9.0.3 in /backend in the pip group across 1 directory #9 reference.

Boundary and invariant

This PR changes only MIME attachment filename handling. It does not change CardDAV/DAV/HTTP URL canonicalization, authorization, declared non-generic attachment content types, PDF byte validation, parser implementations, attachment-body HTML normalization, or shared body-text sanitization.

Two representations are explicit:

1. Display/storage filename — may strip known active markup and literal path segments; rejects C0/DEL/C1, Unicode Bidi_Control, and unsafe raw angle-bracket names; otherwise preserves literal percent/entity text and ordinary RTL scripts.
2. Parser-authority filename — is never derived from the display projection; rejects the same unsafe controls/markup, removes only literal path segments, and otherwise preserves the pre-display representation without URL/entity decoding or whitespace trimming.

Generic MIME extension fallback uses only parser authority. Display sanitization, semantic decoding, control deletion, or Unicode display-order formatting cannot manufacture or disguise trusted parser identity at this boundary.

Standards and decision evidence are code-current in docs/doctoring/mime-attachment-filename-identity.md, with APA 7th references to RFC 2183, RFC 2231, and Unicode Standard Annex #9 (Unicode 17.0.0, Revision 51).

Current verification state

Exact current head: 2eaf6134434a2ad29ad8fe0365aa1b34b848dd5f. No hosted exact-head GREEN is claimed.

Current exact-head workflow evidence:

• Application CI 33729810951: queued;
• Scorecard 33729810960: queued;
• Bandit 33729810968: queued;
• Dependency Review 33729810959: queued;
• Security Scan 33729811001: queued;
• SAST Semgrep 33729811065: queued;
• Docker image validation 33729811139: queued;
• OSV 33729811338: pending;
• CodeQL/other required organization-workflow evidence is not yet present for this exact head and is therefore non-passing, not implicitly successful.

Formal reviews and inline threads must be refetched after the latest push; predecessor evidence does not transfer. Keep Draft until this exact head or a direct non-force successor has executed focused/full Python 3.14 evidence, owned production coverage/docstrings, all live security/SAST/dependency/package/SBOM/provenance gates terminal-success, zero valid unresolved findings, and a qualifying independent approval after the last push as required by live governance.

Pending, queued, cancelled, absent, neutral, failed, stale, predecessor, model-only, status-only, or author-only evidence is non-passing. No self-approval, bypass/admin merge, force-push, destructive rebase, dummy/no-op requeue commit, or gate weakening.

[coderabbitai]

Warning

Review limit reached

Next included review available in 40 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 40aa4f18-a50c-4219-9f71-b2bfd5730c88

📥 Commits

Reviewing files that changed from the base of the PR and between 042b0c7 and 2eaf613.

📒 Files selected for processing (4)

• backend/services/attachment_parser.py
• backend/tests/test_attachment_filename_identity.py
• backend/tests/test_attachment_parser.py
• docs/doctoring/mime-attachment-filename-identity.md

---

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

[seonghobae]

@coderabbitai review

Please review the exact current Draft head 0b01f6ba305d49a2deb662480e2358f16e0bf089. Focus on the MIME filename representation boundary, parser-dispatch integrity, traversal/sanitization regressions, and whether preserving literal percent text can reach any later filesystem or URL-decoding sink. This is review-only; do not apply source changes automatically.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Checked-out commit =='
git rev-parse HEAD
git status --short
printf '%s\n' '== Diff from stated protected base =='
git diff --stat 042b0c70531b229af3acbd0421a2f23098d848b3..HEAD
git diff --name-only 042b0c70531b229af3acbd0421a2f23098d848b3..HEAD

printf '%s\n' '== Candidate files =='
fd -t f 'attachment_parser\.py|test.*attachment.*\.py|.*attachment.*test.*\.py' . || true

printf '%s\n' '== Attachment parser outline =='
ast-grep outline backend/services/attachment_parser.py --items all --view expanded 2>/dev/null || true

printf '%s\n' '== Filename and decoding references in backend =='
rg -n -i -C 4 \
  '(_safe_filename|parse_email_attachment|filename|unquote|unquote_plus|percent[-_ ]decode|urllib\.parse|Path\(|open\(|write_bytes|write_text|save\()' \
  backend -g '*.py' || true

printf '%s\n' '== Exact pull-request diff =='
git diff --find-renames --find-copies --unified=80 \
  042b0c70531b229af3acbd0421a2f23098d848b3..HEAD -- \
  backend/services/attachment_parser.py \
  || true

printf '%s\n' '== Changed tests diff =='
git diff --find-renames --find-copies --unified=80 \
  042b0c70531b229af3acbd0421a2f23098d848b3..HEAD -- \
  '*attachment*' '*email*' \
  || true

Length of output: 240673

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Attachment filename consumers, with enclosing code =='
rg -n -U -P -C 5 \
  '(?:(?:attachment|attachment_obj)\.filename|Attachment\([^)]*filename=|filename=attachment\.filename)' \
  backend -g '*.py' || true

printf '%s\n' '== Exact filename data-flow references in import and worker modules =='
ast-grep outline backend/services/email_import_service.py --items all --view expanded 2>/dev/null || true
ast-grep outline backend/services/newsdom_worker.py --items all --view expanded 2>/dev/null || true
ast-grep outline backend/services/newsdom_pdf_recognition.py --items all --view expanded 2>/dev/null || true
ast-grep outline backend/services/newsdom_client.py --items all --view expanded 2>/dev/null || true
ast-grep outline backend/api/data.py --items all --match 'attachment|asset|materialized' --view expanded 2>/dev/null || true

printf '%s\n' '== Focused source ranges: import persistence and PDF worker/client =='
sed -n '360,415p' backend/services/email_import_service.py
sed -n '130,235p' backend/services/newsdom_worker.py
sed -n '185,235p' backend/services/newsdom_pdf_recognition.py
sed -n '370,425p' backend/services/newsdom_client.py

printf '%s\n' '== Search for Attachment filename sent to paths, URL functions, or file I/O =='
rg -n -U -P -C 8 \
  '(?s)(attachment(?:_obj)?\.filename|Attachment\.filename).{0,800}(?:unquote|urljoin|urlsplit|urlunsplit|quote|Path\(|os\.path|open\(|write_bytes|write_text|mkdir|http(?:x)?\.)' \
  backend -g '*.py' || true

printf '%s\n' '== Email parser attachment boundary and relevant end-to-end tests =='
sed -n '76,112p' backend/services/email_parser.py
rg -n -C 8 \
  '(quarterly%2Ejson|literal_percent|percent_encoded|Content-Disposition: attachment)' \
  backend/tests/test_email_parser.py backend/tests/test_attachment_parser.py || true

printf '%s\n' '== Current focused test status from GitHub =='
gh pr checks 1541 --repo ContextualWisdomLab/naruon --json name,state,workflow,link 2>&1 || true

Length of output: 55033

---

@seonghobae Reviewed commit 0b01f6ba305d49a2deb662480e2358f16e0bf089.

I found no source-level issues in the requested scope.

• _safe_filename() no longer applies URL decoding.
• Literal percent text remains part of the MIME filename identity.
• Generic MIME dispatch now keeps quarterly%2Ejson as application/octet-stream and unsupported_binary.
• Literal / and \ path traversal input still reduces to the basename.
• Encoded traversal text remains display metadata. It does not reach a traced filesystem path, URL constructor, or percent-decoding sink.
• The only downstream transport use is the NewsDOM multipart filename for deferred PDFs. This code does not construct a URL from the attachment filename.

Current hosted verification is not passing yet. The Python 3.14 backend check and security checks are queued. The Strix check is cancelled. Therefore, this review does not provide GREEN verification.

⚠️ Action not completed

Review rate limited.

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.

[seonghobae]

Exact-head source trace for 0b01f6ba305d49a2deb662480e2358f16e0bf089 (author investigation only; not approval):

• email_parser._process_multipart_body() takes Message.get_filename() output and passes it directly to parse_email_attachment; the returned filename is persisted as attachment metadata. This is a MIME/header-decoded filename boundary, not a URL-path boundary.
• Repository search found no Naruon unquote(attachment.filename) sink. Data-quality surfaces use the stored filename for opaque hashing/display, not filesystem materialization.
• The deferred PDF path passes attachment.filename to NewsDOM only as the multipart file filename metadata; newsdom_client.request_pdf_dom() sends (filename, pdf_bytes, "application/pdf") and does not URL-decode or join the filename into a path.
• Therefore the reviewed Naruon paths do not currently turn preserved %2f/%2e text into path separators after this sanitizer. The existing literal / and \\ basename isolation still applies before persistence.
• CodeRabbit manual review was requested for this exact head but is currently rate-limited, so there is still no qualifying independent review evidence. Hosted Python 3.14 and other queued/failed-required contexts remain non-passing.

This trace supports the representation-boundary fix but does not authorize Ready/merge; independent review and exact-head gates are still required.

[github-actions]

PR governance metadata gate is not ready for 2eaf6134434a2ad29ad8fe0365aa1b34b848dd5f:

• Draft PR: merge automation is paused.
• Required check coverage-source-tree is CANCELLED on the current head.
• Required check strix is CANCELLED on the current head.

[seonghobae]

Fresh exact-head verification correction (2026-09-03): required strix is no longer merely pending on 4a7b6ec4216bc74d5b944aef1c0e53fe5b504f9d.

Strix run 33696650519, job/check 100466849042 is terminal cancelled after never acquiring a runner (runner_id=0, empty runner/group, steps=[]). Its sibling cancel-superseded-pr-runs job 100466848802 remains queued with the same no-runner evidence, so GitHub still considers the workflow run active; a targeted job rerun was therefore rejected with 403: workflow run containing this job is already running. This is not a product/security verdict and does not justify source churn.

The fresh no-runner/cancelled evidence has been handed to canonical control-plane owner .github#712. Keep this PR Draft and fail closed. Retry the unchanged exact head only when the active workflow/runner lane can actually schedule; do not create a no-op commit to manufacture a new evidence identity.