fix(helpers): resolve fuzz crashes in DecodeURIIfNeeded and FilenameFromPath

[wizzomafizzo]

Summary

• Fix DecodeURIIfNeeded producing invalid UTF-8 when url.PathUnescape decodes bytes like %80 to non-UTF-8 sequences
• Fix DecodeURIIfNeeded idempotence failures for custom schemes by decoding path segments individually, preserving unencoded slash structure while re-encoding decoded %2F
• Fix FilenameFromPath returning "/" for inputs like "//" where path.Base returns a separator
• Fix ParseVirtualPathStr inconsistent trailing slash handling (TrimSuffix → TrimRight)
• Add crash corpus files from nightly fuzz runs as regression tests

Fixes #675, fixes #667

Summary by CodeRabbit

• Tests

  • Added comprehensive fuzz test cases for URI decoding and filename extraction functions.

• Bug Fixes

  • Improved URI decoding for custom schemes with enhanced UTF-8 validation and character normalization.
  • Strengthened filename extraction from URLs with stricter UTF-8 validation.
  • Fixed path normalization to correctly handle multiple trailing slashes.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds regression test cases from nightly fuzz crashes to test corpus and fixes URI decoding, filename extraction, and virtual path parsing logic to properly validate UTF-8, handle percent-encoded slashes, and trim trailing slashes correctly.

Changes

Cohort / File(s)	Summary
Fuzz test corpus entries pkg/helpers/testdata/fuzz/FuzzDecodeURIIfNeeded/{2f33ed11876b7a01,4ec218b9e58eabec,5e5f04a1d77ed26a}, pkg/helpers/testdata/fuzz/FuzzFilenameFromPath/c8a94dd46f34b25f	Added regression test cases from nightly fuzz crashes to the test corpus for URI decoding and filename extraction functions.
URI decoding logic pkg/helpers/uris.go	Modified DecodeURIIfNeeded to replace ParseVirtualPathStr-based parsing with manual reconstruction; now validates UTF-8 after decoding per-segment, replaces decoded / with %2F, and trims trailing slashes. Tightened FilenameFromPath to require valid UTF-8 after url.PathUnescape, and added early return for filesystem paths when basename is / or ..
Virtual path parsing pkg/helpers/virtualpath/virtualpath.go	Changed ParseVirtualPathStr to trim all trailing / characters using TrimRight instead of removing only a single trailing / via TrimSuffix.
Test expectations pkg/helpers/uris_test.go	Updated TestDecodeURIIfNeeded case steam_with_url_encoding to expect %2F to remain encoded in the output instead of being decoded to /.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 When fuzz tests crash in the night so deep,
A curious hare finds bugs to keep,
With UTF-8 checks and slashes tamed,
Percent-encoded paths are now reclaimed,
The corpus grows with each test case bright! 🌙

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Out of Scope Changes check	❓ Inconclusive	The PR includes a change to ParseVirtualPathStr replacing TrimSuffix with TrimRight; while this improves trailing slash handling, it appears tangential to the primary fuzz crash fixes.	Clarify whether the ParseVirtualPathStr change is necessary to fix the reported fuzz crashes or if it should be addressed in a separate PR.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: resolving fuzz crashes in DecodeURIIfNeeded and FilenameFromPath functions.
Linked Issues check	✅ Passed	The PR successfully addresses both linked issues #675 and #667 by adding fuzz corpus files as regression tests and implementing fixes to prevent crashes in DecodeURIIfNeeded and FilenameFromPath.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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/fuzz-crashes-675-667

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

pkg/helpers/uris.go (1)

191-196: Consider adding debug log for UTF-8 validation failures.

When url.PathUnescape succeeds but the result is invalid UTF-8, the code silently falls back to the original path. For consistency with the error logging on line 195, consider logging this case as well.

Optional: Add debug log for UTF-8 validation failure

 		if pathPart != "" {
 			decoded, err := url.PathUnescape(pathPart)
 			if err == nil && utf8.ValidString(decoded) {
 				decodedPath = decoded
 			} else if err != nil {
 				log.Debug().Err(err).Str("uri", uri).Msg("failed to decode web URI path, using as-is")
+			} else {
+				log.Debug().Str("uri", uri).Msg("decoded path is invalid UTF-8, using as-is")
 			}
 		}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/helpers/uris.go` around lines 191 - 196, The code path in the
url.PathUnescape handling in pkg/helpers/uris.go silently ignores cases where
unescape succeeds but the decoded string is not valid UTF-8; update the block
around decoded, err := url.PathUnescape(pathPart) in the function handling URI
path decoding to add a debug log when utf8.ValidString(decoded) is false (e.g.,
log.Debug().Str("uri", uri).Str("decoded", decoded).Msg("decoded web URI path is
not valid UTF-8, using original path")), keeping the existing error log for err
!= nil intact.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/helpers/uris.go`:
- Around line 191-196: The code path in the url.PathUnescape handling in
pkg/helpers/uris.go silently ignores cases where unescape succeeds but the
decoded string is not valid UTF-8; update the block around decoded, err :=
url.PathUnescape(pathPart) in the function handling URI path decoding to add a
debug log when utf8.ValidString(decoded) is false (e.g., log.Debug().Str("uri",
uri).Str("decoded", decoded).Msg("decoded web URI path is not valid UTF-8, using
original path")), keeping the existing error log for err != nil intact.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 414620fe-b901-4166-9eaf-1193097d8f94

📥 Commits

Reviewing files that changed from the base of the PR and between 08b21df and a76d7fa.

📒 Files selected for processing (7)

• pkg/helpers/testdata/fuzz/FuzzDecodeURIIfNeeded/2f33ed11876b7a01
• pkg/helpers/testdata/fuzz/FuzzDecodeURIIfNeeded/4ec218b9e58eabec
• pkg/helpers/testdata/fuzz/FuzzDecodeURIIfNeeded/5e5f04a1d77ed26a
• pkg/helpers/testdata/fuzz/FuzzFilenameFromPath/c8a94dd46f34b25f
• pkg/helpers/uris.go
• pkg/helpers/uris_test.go
• pkg/helpers/virtualpath/virtualpath.go

[sentry]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!