Fix 13 bugs found during the test build-out - #8
Merged
Conversation
api: - /api/register admin branch was unreachable: exempt paths now get best-effort identity resolution (API key/session) without ever rejecting, so admins can register users without an invite - Oversized bodies return 413 (Content-Length short-circuit + MaxBytesError mapping in a shared decodeJSONBody helper) - Six handlers no longer ignore JSON decode errors (400 on garbage) db: - JobStore.Get/Items/Values return copies instead of live references to maps concurrently mutated by workers (data race) download: - Deluge re-auths and retries once on JSON-RPC "Not authenticated" (previously only transport errors triggered re-login) - DDL staging failures (MkdirAll/Create) are logged and surfaced in the job error instead of a bare "Download failed" - Watcher uses the same fuzzy title match as job tracking so tracker-renamed torrents aren't double-imported - Removed dead err var and unreachable dead_letter guard metadata: - RAWG platform mapping is deterministic longest-match-first (was random map iteration: Xbox 360/Wii U/SNES/Genesis/PS2 flapped) - Sega Master System uses RAWG ID 74 (was 26 = Game Boy) - Description truncation no longer splits UTF-8 runes organize: - Versioned PC directory names (Game.v1.0-FITGIRL) are cleaned as extensionless names instead of misparsing .0-FITGIRL as an extension scheduler: - Inter-item rate-limit wait is interruptible by Stop and applies on continue paths too monitor: - Self-heal container name configurable via QB_CONTAINER_NAME web: - Ship favicon.svg (kills the 404 on every page load) Every fix carries a regression test that fails on the old code.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return sourcePath, err | ||
| } | ||
|
|
||
| fi, err := os.Stat(sourcePath) |
- DDL downloads: filenames from Content-Disposition/URL are reduced to a single safe path component and joined via a containment check (safeChild), closing a path traversal from a malicious server - Backup restore: zip entry names are Base()'d before joining into the data dir (zip-slip); backup names Base()'d after sanitizing - platform slugs from download requests are sanitized before building ROM library paths; organizePC keeps cleaned names single-component - copyDir/extractArchives verify children stay inside their roots - externally supplied values (URLs, filenames, usernames) are stripped of newlines before logging (log forging) Adds sanitizeFilename/safeChild/sanitizeLog helpers with unit tests and an end-to-end regression test proving a filename="../../pwned.zip" response cannot escape the staging dir.
sanitizeFilename/safeChild now gate on filepath.IsLocal (the sanitizer CodeQL's go/path-injection query recognizes), and torrent content basenames + backup names are routed through them. Closes the alerts tied to sanitizable filename components.
Clears the two remaining CodeQL path-injection alerts on the backup create path (name was already restricted to [A-Za-z0-9_-] but CodeQL didn't track the guard through Sprintf; safeJoin gates the final component on filepath.IsLocal, the recognized barrier).
waitFor/waitJobStatus polled 5-10s deadlines that a worker finishes in milliseconds locally, but under the CI -race build on a throttled shared runner the deadline could occasionally lose (one timeout on TestDownloadDDL). Floor the poll timeout at 45s; a passing test still returns the instant its condition holds, so green runs are unaffected.
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.
Fixes every bug surfaced while writing the unit/e2e test suites. Each fix carries a regression test that fails on the old code.
Correctness
ctxUserRolewas neveradmin. Exempt paths now get best-effort identity resolution (API key / session) that never rejects; admins can register users without an invite, anonymous behavior unchanged.Get/Items/Valuesreturned live references to maps mutated by worker goroutines; they now return copies (race-test proven under-race).Not authenticated, not a transport error; now re-logins and retries once.hasMatchingJobused exact title equality while job tracking matched fuzzily; both now sharetitlesMatch.Error handling
MaxBytesErrormapping.MkdirAll/Create) are logged and surfaced in the job error with the actual cause instead of a bare "Download failed".Polish
Game.v1.0-FITGIRL) clean correctly (extension parsing skipped for directories).Stop()and applies oncontinuepaths.QB_CONTAINER_NAME(defaultqbittorrent).favicon.svg— no more 404 on every page load.errvar and an unreachabledead_letterguard.Verification
gofmt/go vet/staticcheckcleango test ./... -race -count=1— all packages pass