fix: NZBGet protocol selector, imageproxy race, CI hardening#396
Merged
fix: NZBGet protocol selector, imageproxy race, CI hardening#396
Conversation
…ardening
NZBGet/Deluge download client bugs (internal/db/download_clients.go):
- GetFirstEnabledByProtocol and GetEnabledByProtocol only queried
"sabnzbd" for the usenet protocol — NZBGet rows were never returned,
causing "no enabled download clients" for any user with only NZBGet
configured. Added "nzbget" to both usenet IN-lists.
- Torrent protocol queries were also missing "deluge". Fixed both
GetFirstEnabledByProtocol and GetEnabledByProtocol to include it.
- hydrateClientCredentials blanked username/password for all non-qBit/
Transmission clients, silently zeroing NZBGet and Deluge credentials
on every read. Switched to a switch statement so nzbget and deluge
pass through as-is while sabnzbd (API-key auth) still gets zeroed.
Imageproxy concurrent-write race (internal/api/imageproxy.go):
- All concurrent goroutines fetching the same image URL wrote to the
shared path imgFile+".tmp". One goroutine's O_TRUNC open could zero
the file mid-write while another goroutine renamed it into place,
resulting in an empty cache file served to subsequent cache-hit
requests. Replaced both fixed .tmp paths with os.CreateTemp() so each
goroutine gets a unique temp file; TestImageProxy_ConcurrentSameURL
now passes under -race.
CI/security hardening:
- auth_test.go: add gitleaks:allow to session-secret fixture string to
stop the Security workflow failing on every push.
- .gitleaks.toml: add "test-secret-" to global stopwords as a fallback.
- promote.yml: move contents+PR permissions to job scope; set top-level
to {} to satisfy OpenSSF TokenPermissions check.
- ci.yml: pin govulncheck install to commit hash (v1.1.4 =
d1f380186385b4f64e00313f31743df8e4b89a77) to resolve downloadThenRun.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if _, werr := f.Write(body); werr == nil { | ||
| _ = f.Chmod(imageCacheMode) | ||
| f.Close() | ||
| _ = os.Rename(f.Name(), imgFile) // #nosec G304 |
| _ = os.Rename(f.Name(), imgFile) // #nosec G304 | ||
| } else { | ||
| f.Close() | ||
| _ = os.Remove(f.Name()) |
| if _, werr := f.Write([]byte(ct)); werr == nil { | ||
| _ = f.Chmod(imageCacheMode) | ||
| f.Close() | ||
| _ = os.Rename(f.Name(), ctFile) // #nosec G304 |
| _ = os.Rename(f.Name(), ctFile) // #nosec G304 | ||
| } else { | ||
| f.Close() | ||
| _ = os.Remove(f.Name()) |
| if f, ferr := os.CreateTemp(filepath.Dir(imgFile), ".img-*"); ferr == nil { // #nosec G304 | ||
| if _, werr := f.Write(body); werr == nil { | ||
| _ = f.Chmod(imageCacheMode) | ||
| f.Close() |
| f.Close() | ||
| _ = os.Rename(f.Name(), imgFile) // #nosec G304 | ||
| } else { | ||
| f.Close() |
| if f, ferr := os.CreateTemp(filepath.Dir(imgFile), ".ct-*"); ferr == nil { // #nosec G304 | ||
| if _, werr := f.Write([]byte(ct)); werr == nil { | ||
| _ = f.Chmod(imageCacheMode) | ||
| f.Close() |
| f.Close() | ||
| _ = os.Rename(f.Name(), ctFile) // #nosec G304 | ||
| } else { | ||
| f.Close() |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Summary
GetFirstEnabledByProtocolandGetEnabledByProtocolonly queriedsabnzbdfor theusenetprotocol; NZBGet rows were never returned. Any user with only NZBGet configured got "no enabled download clients" on every grab. Addednzbgetto both usenet IN-lists.hydrateClientCredentialsblankedusername/passwordfor all non-qBit/Transmission clients, silently wiping NZBGet auth before it reached the adapter. Fixed with aswitchso nzbget/deluge pass credentials through as-is.delugefrom the torrentINlist. Fixed in the same pass.TestImageProxy_ConcurrentSameURLfailing on v1.2.5 tag CI) — all goroutines serving the same URL wrote to the sharedimgFile+".tmp"path; one goroutine'sO_TRUNCopen could zero the file while another renamed it into the cache. Replaced fixed.tmppaths withos.CreateTemp()so each goroutine gets a unique temp file. Test now passes under-race.internal/api/auth_test.gosession-secret fixture string was flagging the Security workflow on every push. Added# gitleaks:allowinline andtest-secret-to.gitleaks.tomlstopwords.promote.ymltoken permissions to job level (was top-levelcontents: write); pinnedgovulncheckinstall to commit hash instead of semver tag to resolvedownloadThenRun.Test plan
go build ./...— cleango test ./internal/db/... ./internal/api/...— passgo test -race -run TestImageProxy ./internal/api/...— pass (was failing before fix)🤖 Generated with Claude Code