Sanitize attachment download filenames

[yyswhsccc]

Claim

• Claim Token comment: Claim MRG Tokens for Bug Bounty Reports - Comment New Bugs Here Before Opening a PR #1 (comment)
• Bounty amount: 25 MRG requested for a small backend/header bug fix; final amount is maintainer decision.
• Bounty type: bug bounty
• Bounty size: small

Description

Attachment download filenames were written into Content-Disposition without enough header-safe cleanup for characters such as quotes, CR/LF, tabs, slashes, and backslashes.

This PR sanitizes download filenames before the header is written and falls back to a safe name when cleanup leaves an empty filename.

Evidence

Before:

• Header-unsafe filename characters could be reflected into the attachment download Content-Disposition value.

After:

• Header-unsafe filename characters are replaced before the response header is written, and empty cleaned names use a safe fallback.

Additional logs or test output:

• go test ./internal/core -run 'TestSafeAttachmentDownloadName' -> ok mergeos/backend/internal/core 0.392s
• Current GitHub Backend build is blocked by the shared Go 1.25.10 govulncheck baseline; Bump backend Go patch version #218 updates the backend Go patch version and is green. Secret scan, web checks, and MergeIDE are passing on this PR.

Safety

• Download header formatting only.
• Does not change upload storage paths, attachment authorization, payout logic, wallets, or secrets.

Tests

• I ran the relevant tests.
• I included the command output or explained why tests were not run.

Bounty Checklist

• I starred this repository before claiming or starting bounty work.
• This PR links to a confirmed Claim Token comment.
• This PR includes image evidence for UI bugs or logs/test output for non-UI bugs.
• This PR explains the behavior before and after the fix.
• This PR does not expose secrets, private keys, customer data, or sensitive production details.

[TUPM96]

Findings
No blocking code issues. The safeAttachmentDownloadName function correctly sanitizes filenames for the Content-Disposition header, effectively mitigating a potential header injection vulnerability and ensuring robust behavior. The logic handles various unsafe characters (quotes, slashes, control characters) and provides a safe fallback name. The added unit tests adequately cover the new functionality.

Bounty Readiness

• Repository Star: Verified.
• Evidence: The system reports Evidence detected: false. However, the PR description provides clear "Before" and "After" explanations of the behavior and includes relevant test output (go test ./internal/core -run 'TestSafeAttachmentDownloadName') which serves as sufficient evidence for this non-UI backend bug.
• Bounty Issue/Claim Context: Linked to a confirmed Claim Token comment.
• Test Commands: Provided and output included.
• Unrelated Rewrites: No unrelated rewrites detected; the changes are focused and scoped.

Tests/Evidence Needed
The provided textual evidence and test output in the PR description are sufficient for this type of bug. No further evidence is strictly required from the author.

Suggested Labels
bug, security, backend, go, bounty-small, ready-for-merge

---

MergeOS automated readiness signals:

• Evidence signal: evidence: missing
• Repository star: star: verified

[yyswhsccc]

Maintenance note: I checked the failing Backend build and test job on this PR. The failure is the shared base/toolchain govulncheck issue from backend/go.mod using Go 1.25.10, not this PR's local test suite. govulncheck reports standard-library issues GO-2026-5039 and GO-2026-5037 fixed in Go 1.25.11 before project tests run.

I opened #218 to bump backend/go.mod to Go 1.25.11. #218 is now green across Backend build and test, Secret scan, frontend/admin/scan web checks, and MergeIDE. After that toolchain patch lands and this PR is updated against it, the backend check should no longer fail on the Go 1.25.10 standard-library findings.

[yZangEren]

MergeOS PR #214 Verification Report

PR: #214
Verifier: independent QA reviewer
Checked at: 2026-06-06 11:17 +08:00
Head SHA: aeae16afdb187f4d23202abde1347e99c4c0b3e3

Verdict

Approve from code review and available evidence. I found no blocking issue in the attachment filename sanitization change.

Scope Reviewed

Changed files:

• backend/internal/core/attachments.go
• backend/internal/core/attachments_test.go
• backend/internal/core/server.go

The previous download header construction only removed double quotes:

strings.ReplaceAll(attachment.OriginalName, "\"", "")

PR #214 routes the filename through safeAttachmentDownloadName() before writing it into:

Content-Disposition: inline; filename="..."

The new helper:

• trims leading/trailing whitespace;
• replaces double quotes, backslashes, and forward slashes with _;
• replaces CR, LF, tab, ASCII control characters, and DEL with _;
• returns attachment if cleanup leaves an empty name.

That covers the important header-safety issue: a submitted filename can no longer inject a second header line through CR/LF, break out of the quoted filename with ", or carry path separators into the visible download name.

Local Checks

Diff scope:

git diff --name-only origin/master...HEAD
backend/internal/core/attachments.go
backend/internal/core/attachments_test.go
backend/internal/core/server.go

Whitespace check:

git diff --check origin/master...HEAD
no output, exit 0

Go version used for local test:

go version go1.25.11 windows/amd64

Targeted backend test:

go test ./internal/core -run 'TestSafeAttachmentDownloadName'

Result:

ok  	mergeos/backend/internal/core	2.474s

Test Coverage Review

The added tests cover the two most important cases:

• a malicious/readable mixed filename such as report"\r\nX-Bad: yes\final/.pdf;
• an all-unsafe filename that should fall back to attachment.

The assertions verify that unsafe characters are removed and expected readable fragments are preserved. This matches the implementation and the intended security behavior.

CI Review

Current PR checks:

• Secret scan: success
• Web build and test (frontend): success
• Web build and test (admin): success
• Web build and test (scan): success
• MergeIDE task runner: success
• Backend build and test: failure

The backend failure is the same repository/toolchain baseline issue seen on nearby PRs. The log shows go1.25.10, then govulncheck, then standard-library findings:

• GO-2026-5039, found in net/textproto@go1.25.10, fixed in go1.25.11
• GO-2026-5037, found in crypto/x509@go1.25.10, fixed in go1.25.11

Because PR #214 only changes attachment filename handling and tests, I do not treat this backend CI failure as caused by the patch.

Recommendation

Approve/accept after maintainer review. No required code changes from my pass.

Optional future hardening, not a blocker for this PR: if the project later needs full internationalized Content-Disposition behavior, add an RFC 5987 filename* value alongside the quoted filename; the current patch still materially fixes the immediate quote/control-character injection issue.