avatarTag was a login-time snapshot, so uploading an avatar left the menu on the
old icon and removing one left it pointing at getAvatar, which now falls through
to the Navidrome logo placeholder. The edit page now updates the stored tag when
the record is the logged-in user, and the menu re-reads the identity on refresh.
The gate ran inside saveFn, so a request destined for 403 still spooled up to
10MB of multipart body to temp first. Artist, playlist and radio already gate
before parsing; the shared handler now takes the gate as a parameter.
Parsing username before the EnableGravatar short-circuit turned a request with
no username into Subsonic error 10, where it used to return a 200 placeholder
without touching the DB.
The uploader re-encodes the image (PNG only when the source was PNG, WebP when
EnableWebPEncoding is on, JPEG otherwise) but stores it under the caller's
extension, so a .gif file can hold JPEG bytes. Deriving Content-Type from the
extension made getAvatar and /UserImage announce a type they were not sending.
postUserImage capped the read for base64 inflation but never checked the
decoded body against MaxImageUploadSize, so a raw upload up to ~33% over
the limit reached SetAvatar, whose io.LimitReader silently truncated it.
Add the same explicit size check postItemImage already uses.
Adds POST/DELETE /userimage (authenticated-only), enforcing the
self-or-admin write rule and the EnableUserAvatarUpload flag. Also
strengthens the anonymous-401 GET /userimage test to seed the private
user with a real avatar file, so it fails for the right reason if the
isPublicUser gate is ever removed.
Registered unauthenticated (matching real Jellyfin's login-picker use case),
but narrowed to conf.Server.Jellyfin.ExposedPublicUsers for anonymous callers.
Also fills PrimaryImageTag on userToDto and getPublicUsers.
The old getAvatar never looked up the user when Gravatar was disabled
(the default), so an unknown username served the placeholder. Restore
that for the unresolvable-user case now that lookup always happens.
Adds POST/DELETE /api/user/{id}/image, gated by EnableUserAvatarUpload
(never EnableArtworkUpload) and restricted to the target user or an
admin. Reuses the artist/playlist/radio image-upload handlers via a new
gate opt-out so their EnableArtworkUpload behavior is unchanged.
ServeUserAvatar lives in server/imghttp so both server/subsonic and
server/jellyfin can call it without importing each other. It delegates
ETag/If-None-Match handling and Content-Type detection to
http.ServeContent instead of hand-parsing the header, avoiding a
false-304 on multi-valued or wildcard If-None-Match headers.
The user object was fetched after UpdateImage, so its in-memory
UploadedImage already matched and Put's structs:"-" tag was never
exercised. Fetch the stale object first and assert the DB value it
would have clobbered survives.
Adds the interface method, real repository implementation, and mock
together so the build stays green. UpdateImage sets updated_at
explicitly in the same UPDATE statement, since AvatarTag's ETag
derives from uploaded_image + updated_at.
The theme rounded the cover image directly and set a border radius on
albumContainer, which has no background or clipping, so it rounded
nothing. The hover overlay is a sibling of the image inside the same
link, so it kept square corners that poked out over the rounded cover.
Move the radius to that link and clip it, so both the image and the
overlay follow the same rounded box. This also covers the mobile bar,
which is always visible.
Fixes#6110
The comment justified anonymous access with "item ids are unguessable".
That is not true: an artist id is a deterministic, unsalted hash of the
artist name, id.NewHash(id.NewHash(str.Clear(lower(name)))), so it is
computable offline by anyone who knows the name.
The real reason the route is public is that upstream Jellyfin's is too.
ImageController.GetItemImage carries no [Authorize] attribute (verified on
v12.0, master/13.0.0, v10.11.9 and v10.10.7), and an anonymous request
reaches LibraryManager.ItemIsVisible with a null user, which returns true
unconditionally. Clients build cover URLs with no credentials at all, so
requiring auth here would break them.
No behavior change.
* added catppuccin mocha theme
* added catppuccin mocha theme to index.js
* fix syntax
* add catppuccin frappé theme
* made frappe and mocha themes more consistant with official color palette and added comments for easy verification
* same for macchiato, seperate commit in case original is preferred
* added comments to .js files
---------
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
* docs: point jftui client link to canonical repository
* docs: fix relative path to webhook-rs example in nd-pdk-host README
* docs: fix capability schema paths in plugin examples README
---------
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
The spec asserted on artistID("Kraftwerk") and intermittently found zero artists
on Windows. The scan did import the artist; it was then made invisible.
RefreshStats selects touched artists with a strict artist.updated_at >
library.last_scan_at (persistence/artist_repository.go:466). Windows' wall clock
has ~15ms granularity, so a new artist written by a quick scan can land in the
same tick as the previous scan's last_scan_at and be excluded. Its
library_artist.stats then stays at the '{}' default and the unscoped cleanup
DELETE removes the row, after which selectArtist's INNER JOIN on library_artist
hides the artist from GetAll.
Backdate last_scan_at before the scan so the comparison is unambiguous, matching
the fix already applied to the search_normalized spec below it.
* fix(i18n): Update Japanese translation
* fix(i18n): fix Japanese translation
* fix(i18n): fix Japanese translation
Update Japanese translations for `recentlyAdded`, `recentlyPlayed`, and `mostPlayed` in album lists
---------
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
Jellify's Discover tab calls GET /Items/Latest and got a 404: we only routed the
/Users/{userId}/Items/Latest form, which real Jellyfin marks [Obsolete] and hides
from its OpenAPI spec, so SDK-generated clients never see it. Add the current
route alongside the legacy one, both served by the same handler.
getLatest also ignored ParentId, so browsing a library or an artist returned the
newest albums across everything the user can see. It now scopes to the library
when ParentId names one, and filters to that artist's albums otherwise, which
also makes a stale id return nothing instead of silently widening back to the
full library set. A malformed ParentId 404s, matching /Items and the contract
decodeFilterParam documents.
PlaybackInfo returned a TranscodingUrl prefixed with the /jellyfin mount path.
Clients concatenate that value onto a server base URL that already carries the
prefix, producing /jellyfin/jellyfin/Audio/{id}/universal and a 404, so playback
never started. Jellify, jellyfin-web, jellyfin-vue and Streamyfin all consume the
field this way; real Jellyfin emits it server-relative (StreamInfo.ToUrl is called
with a nil baseUrl).
Emit the path server-relative to match. Finamp is unaffected: it builds its own
stream URLs and never reads the field.
`buildPlaylist` reported `evaluated_at` as `changed` for smart playlists, so a
rename or comment edit was invisible to clients until the next evaluation. A
never-evaluated smart playlist also reported the current time on every call,
which never settled.
Report `updated_at` for every playlist. `refreshCounters` now syncs the stamp it
writes back onto the model, and `refreshSmartPlaylist` reuses it for
`evaluated_at`. `changed` therefore still equals the evaluation time for a
just-evaluated playlist, and `validUntil` stays anchored to it.
Original Subsonic always bumps `changed` on any playlist update, so this also
aligns the behavior with upstream.
The Refresh Metadata action was only reachable from the Album and Artist context menus, so it could not be triggered from AlbumShow or ArtistShow. This adds an icon-only button, with a tooltip, to the action toolbar on both detail pages. Like the menu entry, it is only rendered for admins.
The button is built on react-admin's Button rather than a plain IconButton: the surrounding toolbars use the former, so the theme colour and the icon-only swap at the xs breakpoint are inherited instead of restated. The dataProvider call and its two notifications move into a new useRefreshMetadata hook, which ContextMenus now shares, keeping a single copy of that logic.
A playlistTrack id is a position in the playlist, not a stable key, so refetching
a row by id after the annotation is saved can return a different song: in a smart
playlist filtered on that annotation the track is gone and every later row has
shifted up. The stale-keyed record then renders as a duplicate of its neighbour.
Signed-off-by: Bjørn A. Andersen <polybjorn@users.noreply.github.com>
Co-authored-by: Bjørn A. Andersen <polybjorn@users.noreply.github.com>
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
The exclusion only worked on master. Coverage profiles name files by import
path; octocov shortens those to repo-relative paths using the checked-out
source, but coverage-on-pr.yml sparse-checks-out only .octocov.yml, so the
paths stay as github.com/navidrome/navidrome/tests/mock_*.go and 'tests/**'
never matched. '**/*_gen.go' matched either way, which is why only the 30
tests/ files leaked.
Every pull request since b77fb45 therefore reported ~-3.7% against master:
447 files on the base side, 477 on the pull request side (#6002, #6069).
* chore: ensure that all durations are nonnegative
* make sure you actually include the test file
* test(conf): use non-zero durations in the valid_duration fixture
Zero is the boundary between the accepted and rejected ranges, so it
passes even if the guard is off by one. 1s exercises an ordinary value.
---------
Co-authored-by: Deluan <deluan@navidrome.org>
An absent artwork state was revisited by an hourly job, by viewing the entity, and
by the startup backfill on any artwork config change. On a large library the last
one queued tens of thousands of external lookups at once and got the provider to
rate-limit us for hours.
Nothing revisits an absent state now. Retrying is explicit: `artwork reprocess` on
the CLI, or the refresh button in the UI. The config fingerprint survives only as an
advisory, warning at startup and naming the command that clears it.
Since absent is terminal, `artwork status` splits it into two disjoint columns, and
`--source failed` targets only the ones that gave up rather than being answered.
Both read through the filter CountBySource and EnqueueBySource already share, so the
reported number is the set the command acts on.
Also fixes the last_failure default left by 20260819204637, which marked every
pre-existing absent row as failed, and removes the code the deleted retry paths
orphaned.
The coverage profile counted the tests/ package and the *_gen.go files, none of
which are code under test: tests/ is the mock and helper package, and generated
code is never hand-tested. Together they added 2926 uncounted statements at 0%,
pulling the reported number down by almost 6 points (70.13% -> 75.98% on the
current master profile).
octocov's coverage.exclude takes doublestar globs matched against git-root-relative
paths. All 26 mock_*.go files live under tests/, so the single 'tests/**' pattern
covers them.
* fix(deezer): treat an exhausted quota as a throttle, not as a missing artist
Deezer reports quota exhaustion in the response body, with HTTP 200 and no
rate-limit headers. The client only looked for errors when the status was
not 200, so a throttled reply was decoded into an empty result type, and an
empty search became ErrNotFound. The agent then compounded it: it tested
`errors.Is(err, ErrNotFound) || len(artists) == 0` before testing err, so
any failed search — which also returns no artists — reported not-found too.
The artwork worker settles an entity as "no image" on agents.ErrNotFound.
So being throttled did not make Navidrome back off; it made it record the
artist as having no artwork, and move on to do the same to the next one.
Errors are now parsed out of the body regardless of status, and the quota
code is joined with agents.RetryLaterError so the circuit breaker and the
artwork retry budget see a throttle for what it is. The agent checks err
before the empty-result case.
Last.fm already handles this exact shape (client.go errCodeRateLimit, with
a comment noting the 200-with-body-error pattern); this brings Deezer in
line with it, including the zero-delay RetryLaterError so both providers
share the default cooldown rather than a per-provider number.
Measured against the live API to pin the shape: a 120-request burst
returned 54 results and 66 quota replies, every one of them HTTP 200 with
{"error":{"type":"Exception","message":"Quota limit exceeded","code":4}}
and no Retry-After or rate-limit headers. A single request 5s later
succeeded, so the window is short and a cooldown fully clears it.
* refactor(deezer): fold the error envelope into one type
The envelope declared the code and message inline, parseBodyError copied
them field by field into a second struct with the same shape, and a zero
Code stood in for "no error reported". Making the envelope hold a pointer
to the error type removes all three: absent is nil, present is the error
itself, and the value returned needs no conversion.
searchArtist loses its empty-result branch. searchArtists converts an
empty result to errNotFound and returns early on any error, so it never
answers with no artists and no error, and the branch could not run. What
it left behind was a comment explaining an ordering that only mattered
while the branch existed.
ErrNotFound is unexported: nothing outside this package referenced it,
and it sat three lines from agents.ErrNotFound, which is a different
error with the opposite meaning for callers.
Throttling now joins agents.ErrRetryLater, the sentinel documented as the
zero-delay RetryLaterError, rather than allocating an equivalent value.
* refactor(deezer): return agents.ErrNotFound from the client
The client raised a package-local sentinel that the agent then translated
into agents.ErrNotFound, one call site each. Deezer was the only adapter
carrying its own: last.fm and listenbrainz have none.
The client already reports throttling with agents.ErrRetryLater, so it
already speaks the agent vocabulary; saying "not found" in the same words
costs nothing and lets searchArtist drop to plain error propagation.
* test(scrobbler): remove a race in the longest-server-delay test
newBufferedScrobbler starts its drain goroutine, and run() drains once
before it ever waits on the wake signal. The test enqueued user2, then
enqueued user1 via Scrobble, so that startup drain could land between
the two: it saw only user2, took its 45s delay, and set backingOff. The
wake from the second enqueue is then deliberately ignored — a wake
during a backoff window must not drain, which is the hammering the
window exists to prevent — so user1 was never attempted and the first
assertion read 1 instead of 2.
Buffering both users before the goroutine exists removes the window.
The test no longer goes through Scrobble, which the sibling tests
already cover; what this one is about is which delay wins.
Reproduced deterministically by forcing the interleaving with a
synctest.Wait between the two enqueues, which fails with the same
"expected both users drained, got 1 attempts" seen in CI. With both
enqueued first, that same forced drain passes.
An explicit RetryLaterError now opens the agent's breaker immediately for the
provider's own delay, instead of counting it as one generic failure that needs
five to open and then always probes after a fixed minute.
* ci: comment coverage on pull requests from forks
A pull_request run from a fork gets a read-only GITHUB_TOKEN, so octocov could not post its comment: it logged a 403 and exited 0, leaving the job green and the PR silent. The 'permissions:' block cannot grant what the token does not have.
The comment now comes from a workflow_run workflow, which runs on the base repository and does get a write token. The pipeline job keeps the job summary and the default-branch baseline, and hands the merged profile and the PR number to it as an artifact.
A workflow_run job otherwise looks like a push to the default branch, so octocov is pointed back at the pull request and at the run that produced the profile via its OCTOCOV_ environment overrides. Without the run id override the test execution time would be read from the wrong run; without the ref override a fork's coverage would be stored as the master baseline.
The job holds a write token, so it reads .octocov.yml from the base branch rather than from the fork.
* ci: stop checking out the fork in the coverage comment workflow
CodeQL flagged the pull request checkout as untrusted code in a privileged context (actions/untrusted-checkout/high): the job holds a write token. The checkout existed only so the code-to-test ratio would reflect the pull request, which does not justify the alert.
The workflow now checks out just .octocov.yml from the base branch, and the ratio is skipped when reporting from there. Coverage and its delta against master, the metrics that motivated the report, are unaffected: they come from the profile the pipeline uploads.
* ci: treat the coverage artifact as untrusted input
A pull_request run executes the fork's own copy of pipeline.yml, so every file in the octocov-pr artifact is attacker-controlled. The artifact was extracted into the workspace root, on top of the base-branch checkout, and download-artifact truncates existing files. A fork could therefore replace .octocov.yml before octocov loaded it.
That is not only a config swap. config.Load expands ${VAR} from the job environment and the action sets OCTOCOV_GITHUB_TOKEN, so a crafted comment.message posts the privileged job's token into a public comment; a body: section rewrites a pull request description, which pull-requests: write allows.
The artifact now lands in a subdirectory and only coverage.out is copied out, after pr_number is checked to be digits and the named pull request's head is confirmed to be the sha that triggered this run. Without that check the artifact could aim the comment at any open pull request, and unvalidated content reached GITHUB_OUTPUT.
* ci: report Go test coverage on pull requests
Adds octocov to the existing 'Test Go code' job. It reads the coverage
profile, posts a PR comment with the coverage percentage and the delta
against master, and writes the same report to the job summary.
The master-branch report is stored as a GitHub Actions artifact, so no
external service or secret is needed.
* ci: merge the plugins job coverage into the same report
The plugins suite runs in its own job, so its coverage was missing from
the report. Both jobs now upload their profile as an artifact and a new
'Report coverage' job merges them into a single PR comment.
* ci: update the coverage comment in place instead of reposting
octocov's default is to collapse the previous comment and create a new
one. updatePrevious edits the existing comment instead, so a PR keeps a
single coverage comment across pushes.
* ci: fix octocov timeout and step-time lookup
Storing the report hit the 30s default timeout: scanning this repo's
artifacts for the baseline consumed it first. Raise it to 5m.
The step-time lookup also matched the Windows job's 'Test' step and
waited for a job that was still running, so execution time was dropped
from the report. Rename the step to make it unique.
* ci: only store the coverage baseline from the default branch
* ci: report statement coverage instead of line coverage
octocov reports statement coverage for a single profile but switches to
line counting when it merges several itself, which made the number
disagree with 'go tool cover -func'. Merge the two job profiles into one
file first, so the reported number matches what developers see locally.
* ci: stop the download-link comment from clobbering the coverage report
Both comments are posted by github-actions[bot], and the download-link
job updated the first bot comment it found. On a new PR the coverage
comment is created first, so it would be overwritten. Match on the body
as well, and keep the coverage profiles out of the download list.
* test(plugins): build test plugins in Go instead of shelling out to make
The plugins suite built its .ndp test packages by running `make -C
plugins/testdata`, which needs make and zip on the PATH. That is the reason
the 26 WASM-dependent spec files are tagged //go:build !windows.
buildTestPlugins now does the same work in Go: the same mtime check make
performed, `GOOS=wasip1 GOARCH=wasm go build` per plugin, and archive/zip
for the package. TinyGo was already optional and unused in CI, so nothing is
lost there. The first plugin builds on its own so the shared wasip1 stdlib
and PDK objects land in the build cache before the rest fan out: on a cold
cache that is 2.2s against 3.4s for the sequential make and 7.3s for an
unrestrained fan-out.
Packaging moved into a writeNdp helper shared with createTestPackage, which
was already writing the same two-entry archive. Entries are written in a
fixed order, so the .ndp bytes are now reproducible; the loader hashes those
bytes, and `zip` also stored file mtimes, so the previous packages differed
on every rebuild.
The Makefile is unchanged and still works for building the plugins by hand.
Removing the !windows tags is a separate step, once CI is green here.
* test(plugins): run the WASM plugin specs on Windows
With the test plugins now built in Go, nothing in the suite needs a Unix
toolchain, so the //go:build !windows tags come off all 25 spec files. The
Windows CI job runs `go test ./...`, so it picks the suite up with no
workflow change.
plugins_suite_windows_test.go existed only to bootstrap the handful of specs
that compiled on Windows; plugins_suite_test.go now serves both.
* test(plugins): skip the planted-symlink spec where symlinks need privileges
os.Symlink needs an elevated token or Developer Mode on Windows, so the
unconditional Expect(...).To(Succeed()) would fail for contributors running
the suite on an ordinary Windows box. The elevated GitHub runner hides this.
The equivalent spec in sandbox_fs_internal_test.go already attempts the
symlink and skips on error; this does the same, keeping the pin live
everywhere it can run, including Windows CI.
* fix(ci): stop the Windows ndpgen test failing silently
The ndpgen suite builds its helper binary to %TEMP%\ndpgen-test, and Windows
will not exec a file without an executable extension, so the "supports
verbose mode" spec has been failing there. Nobody noticed because the
Test ndpgen step ran under pwsh, which carries on after a non-zero exit and
takes the step's status from the last command, so the job stayed green with
a FAIL line in its log.
Add the .exe suffix, and run the step under bash like the Linux job does, so
a failure in any of its three commands fails the job.
* fix(plugins): build public URLs on the caller's address instead of localhost
The artwork host service had no `*http.Request`, so it passed `nil` to
`publicurl.ImageURL`. With neither `ShareURL` nor `BaseURL` configured, that
produced `http://localhost/share/img/...`, which is useless to anything outside
the server. The Discord Rich Presence plugin explicitly drops localhost URLs, so
it fell back to the Navidrome logo instead of the real cover art.
`serverAddressMiddleware` already works out the client-facing scheme and host
from the `X-Forwarded-*` headers. It now also records them in the request
context, and `publicurl` takes a `context.Context` instead of an `*http.Request`
so any caller can reach them. Extism passes the caller's context through to host
functions, so plugins invoked during a request now get a reachable URL with no
configuration.
Switching the parameter also removes the need for a second, parallel entry
point: the package previously wanted only a scheme, a host, and a context, and
took a whole request to get them. `AbsoluteURL` no longer dereferences a
possibly-nil request on its parse-error path.
Plugin calls that start from `context.Background()` (scheduler and websocket
callbacks, the buffered scrobble drain) still fall back to localhost, since they
have no request to learn from. A debug log now points at `ShareURL` when that
happens.
* fix(publicurl): include the configured port in the localhost fallback
The last-resort fallback built `http://localhost/...`, which points at port 80
and so is unreachable for a server listening anywhere else — the default 4533
included. Use `conf.Server.Port` so a consumer on the same machine can actually
fetch the URL.
* fix(publicurl): use https in the localhost fallback when TLS is configured
The fallback hardcoded the http scheme, so a TLS-only server with no BaseURL
advertised a URL it does not answer on. Mirror the server's own switch, which
requires both a certificate and a key.
* refactor(publicurl): tidy the localhost fallback and its tests
Use gg.If for the fallback scheme so it reads as an expression, like the
BaseScheme branch above it, instead of assigning http and overwriting it.
Drop two tests the ctx refactor left redundant: one asserted PublicURL "works
without a request" but became a byte-identical copy of the ShareURL spec once
the *http.Request parameter went away, and the two port specs differed only in
the integer, where the non-default port is the stronger assertion.
* refactor(conf): add TLSEnabled and use it instead of repeating the predicate
Whether the server speaks HTTPS was decided inline in three unconnected
places. This PR added the third, in a URL-building package that has no
business inferring the transport config.
Move the rule to conf, next to the fields it derives from, and call it from
publicurl and the insights collector. server.Run keeps its own expression: it
takes the certificate and key as parameters, and its test passes values that
do not come from the config.
* fix(nativeapi): stop partial PUTs from clearing untouched columns
The REST layer parses the request body's top-level JSON keys and passes them
to Repository.Update as colsToUpdate. The radio and library repositories
discarded that list and issued a full-row UPDATE, so any field absent from
the body was written as its zero value.
For radio this wiped uploaded_image, deleting the station's cover on every
partial update (the Web UI is unaffected because its form submits the whole
record). For library it silently cleared remote_path and default_new_users.
Thread the column list through to Put in both repositories, and extract the
column-selection half of filterUpdateValues into selectUpdateColumns so
library, which hand-builds its update map, shares the same rule instead of
copying it.
Fixes#6057
* refactor(persistence): drop pluginRepository's dead rest.Persistable methods
Save and Update had no callers: PUT /api/plugin/{id} is served by the
hand-written updatePlugin handler over a typed request struct, and the
route only wires rest.GetAll and rest.Get. Both methods delegated to Put,
which upserts all twelve columns, so wiring rest.Put to this repository
would have reintroduced the partial-update clobbering fixed in the previous
commit. Removing them, along with the rest.Persistable assertion, makes
that a compile error instead of a silent data loss.
Put itself is unchanged and still backs plugin discovery.
* perf(persistence): avoid a full media_file scan when resolving playlist paths
FindByPaths built one OR-ed equality term per path. On the real media_file
schema SQLite abandons the path index at just two OR-ed terms and falls back to
SCAN media_file, re-testing every term against every row, so the cost grows with
(rows x terms).
Group the candidates by library and emit one IN list per library instead, which
plans as SEARCH media_file USING INDEX media_file_path_nocase. The NOCASE
collation is kept so ASCII case-insensitive matching still works.
This is the dominant cost of M3U playlist import, which resolves every track on
every scan. Measured with a 1000-track playlist against a migrated DB:
100k media_file rows: 397 -> 51,414 tracks/sec
500k media_file rows: 78.5 -> 47,174 tracks/sec
The rate no longer degrades as the table grows, which is the expected shape for
an index lookup. Reported in #6043, where an 8 hour scan of a 2M-song library
spent 7h52m in the playlist phase.
* docs(playlists): correct the stale reason for the M3U lookup chunk size
The expression-tree depth ceiling applied to the old OR-per-path query, which
capped a batch at roughly 500 terms. The IN form is bound by SQLite's 32766
variable limit instead, which the 400 candidates per chunk sit far below.
