Fix Vimm path traversal, truncated-download corruption, and Prowlarr query escaping - #9
Merged
JeremiahM37 merged 2 commits intoJul 14, 2026
Conversation
downloadVimmGame joined the server-supplied Content-Disposition filename straight onto the staging dir, so a malicious or MITM'd Vimm response with filename="../../.." could write outside it. The sibling DDL path already guards this with sanitizeFilename + safeChild (PR #8); apply the same to Vimm. Both copy loops also discarded the f.Write error and returned success without checking downloaded == Content-Length, so a dropped connection or full disk passed a truncated archive to the scan/organize pipeline as a complete game. Check the write error, treat a non-EOF read error as failure, verify the byte count, and delete the partial file on failure. Adds TestDownloadDDLTruncatedDownloadIsError.
The hand-rolled queryEscape only replaced spaces and '&', so any game title containing '?', '#', '%' or '+' corrupted the query string and returned wrong or empty results (e.g. "...Millionaire?", "R.B.I. Baseball", "C++"). Replace it with url.QueryEscape and drop the helper. Replaces the old escaper's unit test with a round-trip regression test over the previously-broken characters.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Three real bugs found while auditing the download and search paths — all verified against current
main, self-contained, and covered by tests. The download-path issues are the sharp ones: PR #8's bug-sweep fixed them for the DDL path but two sibling paths were missed.1. Path traversal in the Vimm download (security)
downloadVimmGamejoined the server-suppliedContent-Dispositionfilename straight onto the staging dir (filepath.Join(destPath, filename)). A malicious or MITM'd Vimm response withfilename="../../.."could write outside the staging directory. The DDL path already defends against this withsanitizeFilename+safeChild(added in #8) — this applies the same guard to Vimm.2. Truncated downloads reported as complete (reliability / corruption)
Both the Vimm and DDL copy loops discarded the
f.Writeerror and returned success without checkingdownloaded == Content-Length. A dropped connection or full disk left a partial.7zthat was then handed to the scan/organize pipeline and landed in the library as a "complete" game. Now: check the write error, treat a non-EOF read error as failure, verify the byte count againstContent-Length, and delete the partial file on failure.3. Broken Prowlarr query escaping (correctness)
queryEscapeonly replaced spaces and&, so any title containing?,#,%, or+corrupted the query string and returned wrong/empty results — e.g. "Who Wants to Be a Millionaire?", "R.B.I. Baseball", "C++". Replaced withurl.QueryEscape.Validation
go build ./...,go vet ./...,gofmtcleango test ./...— all 17 packages passTestDownloadDDLTruncatedDownloadIsError(partial-download guard) andTestQueryEscapingRoundTrips(special-character round-trip regression)