Image performance optimizations & API hardening#553
Open
therobbiedavis wants to merge 72 commits into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a152f5b016
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
T4g1
requested changes
Apr 30, 2026
Contributor
T4g1
left a comment
T4g1
left a comment
There was a problem hiding this comment.
Like i said, i did not process the frontend part as this is not my area of expertise, but i might look into it later this week if i get the time
therobbiedavis
force-pushed
the
bugfix/audiobook-images
branch
from
9592070 to
4c26deb
Compare
therobbiedavis
changed the title
therobbiedavis
force-pushed
the
bugfix/audiobook-images
branch
from
8614967 to
23e16cc
Compare
kevinheneveld
pushed a commit
to kevinheneveld/Listenarr
that referenced
this pull request
May 14, 2026
The committed script assumed (a) docker on Clyde, (b) kevin@media.local SSH, and (c) the docker compose plugin — none of which match this environment. Three real divergences captured by Clyde's deploy procedure write-up: - Build host. Clyde has no docker binary; image is built on media. Script now rsyncs the working tree to /root/listenarr-build/listenarr-src/ on media (override via BUILD_DIR_REMOTE) and runs `docker build` there. The old docker save | ssh ... docker load transfer step is gone. - SSH target. Defaults to the `media` ssh alias from ~/.ssh/config (root@192.168.1.35), override via MEDIA_SSH. Health-check curls use MEDIA_IP (default 192.168.1.35) since `media` isn't a public DNS name. - Compose binary. Restart step now uses the standalone `docker-compose` v2.x binary that media actually has, not the `docker compose` plugin. The script's printed rollback hint uses the same. Also: the backup check now matches the timestamped pattern (config.bak.*) that's actually used, not the bare config.bak it looked for before. smoke-test.sh's "check container logs" hint now says `ssh media` instead of `ssh kevin@media.local`. Added a closing note about browser-loading SPA changes — HTTP smoke is necessary but not sufficient for JS module-init failures; this is the trap that bit us when we accidentally deployed kevin/live with PR Listenarrs#553 merged in.
therobbiedavis
force-pushed
the
bugfix/audiobook-images
branch
from
23e16cc to
7faf429
Compare
Introduce short-lived image-only access tokens and client/server plumbing to allow direct, signed backend image URLs without requiring cookie/bearer auth. Changes include: - New backend ImageAccessTokenService (IDataProtector-based tokens) and tests (ImageAccessTokenServiceTests). - AccountController: return imageToken on login and add GET /account/image-token endpoint for issuing per-user tokens. - SessionAuthenticationMiddleware: accept image token via ?t= on /api/images/* and authenticate that request principal when valid. - Image serving: set Cache-Control header for cached images and use resolved content-root paths (IAppPathService injection used where applicable). - LibraryController, FfmpegController, DiscordController: add optional IAppPathService usage and resolve paths via content root instead of Directory.GetCurrentDirectory(). - ImageCacheService: use content root path for static asset lookups. - SystemController: add /system/ready readiness endpoint (AllowAnonymous). Frontend changes (fe/src): - ApiService: manage image access token lifecycle (fetch, cache, expiry), attach token as ?t= to backend image URLs, avoid blob-fetch path for signed backend images, add blob URL LRU cache and limited concurrency for image blob fetching, expose clear/get helpers used in tests. - main.ts: prefetch image access token at startup alongside antiforgery token. - Add frontend Vitest tests for image token flows (api.imageAccessToken.spec.ts). Tests: - Update SessionCookieAuthTests to cover image-token endpoint, image access with token, non-image endpoint protection, and readiness probe. Overall this enables safer direct image URL usage (query-string signed tokens), reduces unnecessary blob-ification for same-origin signed images, and improves client image caching/concurrency behavior.
CI: Add a step that checks prior run attempts via the GitHub CLI and sets an already_notified output; update the Discord notification condition to run only on the first attempt or when no prior attempt succeeded to avoid duplicate messages. Dockerfiles: install libcap2 (addresses CVE-2026-4878) and extend the npm-based hotpatching to include brace-expansion v5 in addition to v2, while keeping the picomatch patch. Tarballs are cleaned up after extraction. Changes applied to both the repository root Dockerfile and listenarr.api/Dockerfile.runtime to mitigate CVE-2026-33671 and CVE-2026-33750 and pull the patched OS package.
Introduce IAppPathService / AppPathService to centralize app paths (content, config, logs, ffmpeg, tools, wwwroot) and resolve paths relative to the host content root. Update FfmpegInstallerService and SystemService to use the new service instead of AppContext/CurrentDirectory, and register IAppPathService (and ImageAccessTokenService) in DI. Adjust Listenarr.Api.csproj to stop copying config\appsettings files to output/publish so local dev config remains in listenarr.api/config. Update package.json dev script to wait for the /api/v1/system/ready endpoint instead of /api/v1/system/health.
Migrate frontend and API client to rely on HttpOnly session cookies and a bootstrap config endpoint instead of embedding bearer/image tokens. ApiService: add getBootstrapConfig, simplify getImageUrl/fetchImageObjectUrl to return backend URLs directly, remove image access token caching/attachment logic, and streamline auth/antiforgery handling (session marker used for cross-tab sync). UI/composables: remove protected image blob-fetch flow and startup-config-dependent auth checks; update SystemLogs SignalR options and include credentials for log fetches. Tests and components: update mocks to include getBootstrapConfig, adjust AuthenticationSection to emit apiKey, add auth store tests, and update session-token/storage tests to assert sanitized cross-tab markers. Also include server-side support for bootstrap config and authorization middleware/filter changes to enforce admin or API key when authentication is enabled.
Remove custom image blob/object-url handling and lazy-loading utilities, and simplify image URL resolution to use backend image endpoints directly. useProtectedImages now delegates to apiService.getImageUrl and no longer tracks per-image cache keys, object URLs, or retry state. ApiService removes blob fetch, concurrency and metadataUrlCache logic; ensureImageCached now attempts the provided /images/{id}?url=... endpoint first and falls back to the base /images/{id} endpoint. UI changes: components no longer pass cache keys to getProtectedImageSrc and rely on the simplified API; lazyLoad util deleted and related lazy-loading hooks removed. Search UX: debounce/autosearch removed from useSearch, unified/advanced search got improved keyboard accessibility and explicit submit handling; AddNewView updated to use the composable APIs and handle results synchronously. Tests updated to match the new behavior (including renamed test file) and new unit tests added for search submit/accessibility and ensureImageCached behavior.
Enforce stricter access for API key operations: add a RequireAdministratorSessionWhenAuthenticationEnabled filter and a RequireApiKeyManagementAccess helper in ConfigurationController to require an administrator session when authentication is enabled, while allowing local/private-network callers when auth is disabled. Update SecurityRequestUtils.ShouldRedactSecretsForCaller to trust local/private requests for non-redaction. Update Swagger docs to reflect the new access rules. Add tests covering remote vs private access when authentication is disabled, rejection of API-key auth for these endpoints when auth is enabled, admin-session regeneration flow, and an antiforgery helper used by tests.
T4g1
reviewed
May 22, 2026
T4g1
reviewed
May 22, 2026
Remove FileUtils.IsPathMissing and replace its usages with string.IsNullOrEmpty to simplify path emptiness checks. Update LibraryController to validate request.SourcePath/DestinationPath using string.IsNullOrEmpty, adjust IsPathInvalidForOs to avoid the null-forgiving operator, and simplify NormalizeStoredPath with null-coalescing assignment. Also remove the unit test that targeted IsPathMissing.
Refactor the MoveAudiobook test to use the BaseTests fixture and test DI instead of manually creating an in-memory DbContext and numerous mocks. Tests now use FileService for temp paths, save application settings with ApplicationSettingsBuilder, add an audiobook via _audiobookRepository, and obtain the controller from the test provider. Simplifies setup, preserves destination-path-whitespace assertions, and removes redundant mock move-queue verification.
Update CONTRIBUTING.md to clarify DbContext and test-host registration (recommend AddListenarrInfrastructure, ListenarrWebApplicationFactory and Program.Testing.cs/ApplyTestHostPatches), centralize HttpClient registration in listenarr.infrastructure, suggest IDownloadClientAdapterFactory for adapter resolution, and strengthen testing guidance (refer to tests/README.md and use builders). Also replace a manual Audiobook instantiation in LibraryController_MoveTests with AudiobookBuilder to improve test consistency and readability.
Move ImageCacheService from the Services feature to a new Cache feature (file and namespace rename). Update CONTRIBUTING.md to document the Cache folder and clarify where infrastructure implementations should live. Adjust InfrastructureServiceRegistrationExtensions to import the new Cache namespace. Relocate ImageCacheService tests from tests/Features/Api/Services to tests/Features/Infrastructure/Cache and update test imports accordingly (old test deleted, new test added). These changes reorganize code by feature/technology and update references to match the new structure.
Decouple DTO from domain model and centralize API version normalization. StartupConfigDto.FromStartupConfig now accepts primitive auth flag and api version instead of a StartupConfig instance. Controller GetBootstrapConfig is made synchronous and uses the startup config service's IsAuthenticationRequired/GetEffectiveApiVersion. StartupConfigService.GetEffectiveApiVersion now delegates to NormalizeApiVersion, and StartupConfig.IsAuthenticationEnabled was simplified (GetEffectiveApiVersion and related static helper removed). Tests and mocks updated to use ApiVersionNormalizer from Listenarr.Domain.Common.
Add AudiobookBuilder.WithSeriesNumber and refactor LibraryController_BasePathTests to use the builder. Tests now declare root/path and pattern constants, retrieve the private ComputeAudiobookBaseDirectoryFromPattern via reflection, and use a CreateController helper to simplify controller construction. Mocking of IFileNamingService is simplified and expectations condensed; assertions use Path.Join with the shared RootPath. Overall this cleans up test setup and removes duplicated controller/mock instantiation.
Annotate infrastructure test classes and methods with xUnit [Trait] metadata for improved test organization and filtering. Updated files: - tests/Features/Infrastructure/Platform/ApplicationVersionServiceTests.cs - tests/Features/Infrastructure/Platform/SystemServiceVersionTests.cs - tests/Features/Infrastructure/Services/ApplicationPathServiceTests.cs Each class now includes Area/Name/Category traits and several tests include Method and Scenario traits. These changes are metadata-only and do not alter test logic.
Switch tests to use builders and shared test utilities for clarity and reuse. Added AudiobookBuilder.WithFilePath and replaced manual model construction with AudiobookBuilder/AudiobookFileBuilder usages. Tests now use FileService for temp directories, include Traits, follow Arrange/Act/Assert comments, and remove the fragile TryDeleteDirectory cleanup. CreateController was made an instance method and updated to use LibraryControllerMockFactory.CreateApplicationPathService(FileService.GetTempPath()).
Enhance test builders and refactor LibraryController tests for consistency and reuse. AudiobookBuilder now initializes Authors/Genres/AuthorAsins and adds fluent With... helpers (ImageUrl, Monitored, Description, Subtitle, FileSize, OpenLibraryId, WithGenre, WithAuthorAsin). AudiobookFileBuilder adds WithSize and WithFormat. Tests were updated to use these builders, added Trait attributes, replaced Arrange/Act/Assert comments with Given/When/Then, and switched to shared test utilities (LibraryControllerMockFactory/FileService) for application path and temp directories. Some fragile cleanup and ad-hoc object construction were removed in favor of builder usage and stable temp-path helpers.
Replace ad-hoc test setup with shared BaseTests, builders and mock factories across many tests. Added fluent methods to DownloadBuilder (Title, Artist, Series, AudiobookId) and updated tests to use DownloadBuilder, AudiobookBuilder, AudiobookFileBuilder and ApplicationSettingsBuilder. Consolidated controller construction into CreateController helpers, added Trait attributes to tests, removed direct in-memory DbContext/service provider setups, and streamlined assertions and arrange/act/then comments for clarity.
Replace verbose inline Download construction and manual IAudiobookFileRepository setup with reusable helpers. The test now uses DownloadBuilder to add a download and LibraryControllerMockFactory.CreateAudiobookFileRepository(allFiles) to create the mocked file repo, improving readability and reducing duplication while preserving existing behavior (e.g. DownloadStatus.Downloading).
Replace FileService.GetTempDirectory("ffmpeg") with Path.Combine(FileService.GetTempPath(), "ffmpeg") and remove the explicit Directory.Delete cleanup and its try/catch. The test now asserts the ffmpeg directory does not exist initially and verifies ffprobe/ffmpeg paths after installation without deleting the temp directory.
Introduce NormalizeOrDefault in ApiVersionNormalizer to return a default version when normalization yields null, and update ApiVersionUtils to call this new helper. Removed the redundant NormalizeApiVersionString wrapper in the application layer and replaced its usages with ApiVersionNormalizer.NormalizeOrDefault to centralize defaulting behavior. Changes touch listenarr.domain/Common/ApiVersionNormalizer.cs and listenarr.application/Common/ApiVersionUtils.cs.
Add an optional ILogger parameter to ResolveApiVersion and import Microsoft.Extensions.Logging. Replace System.Diagnostics.Debug.WriteLine calls with logger?.LogWarning to emit structured warnings on route/path parse failures. The logger is optional (defaults to null) so the method remains backwards-compatible while improving diagnostics.
Register ImagesController in the test service collection and refactor ImagesController_ContentRootResolutionTests to use DI-provided controller and shared mocks. The test now creates a temporary content root, injects mocked IImageCacheService and IApplicationPathService (returning the temp root) during InitializeAsync, and obtains the controller from the test service provider. This simplifies setup by removing ad-hoc mock/instance creation in the test and ensures the controller resolves its effective content root from IApplicationPathService.
Rename file to Models/Configurations/FfmpegConfig.cs and update the class namespace from Listenarr.Domain.Models to Listenarr.Domain.Models.Configurations to match the new folder structure. No other changes to the class implementation.
T4g1
reviewed
May 23, 2026
Switch ImageCacheService to accept a HttpClient directly and register it as a typed HttpClient. Removed the ImageCacheHttpClientNames constant and IHttpClientFactory usage, and updated the DI registration to services.AddHttpClient<ImageCacheService>() with the same handler configuration. Tests were adjusted to pass a real HttpClient instance instead of mocking IHttpClientFactory and to look up HttpClientFactoryOptions by the typed client name. This simplifies DI and enables typed client configuration for the image cache service.
Return early before calling UpdateAsync when the download does not exist, preventing silent recreation of deleted or never-persisted downloads. Drop the previousStatus intermediate variable since the direct null check on previous is sufficient.
Rename AudiobookFileFormatSummary to AudiobookFormatSummary and update all usages. Updated file name and type references across the application and infrastructure layers (LibraryListService, IAudiobookFileRepository, EfAudiobookFileRepository, AudiobookStatusEvaluator) and adjusted tests/mocks to use the new type to keep naming consistent.
Use Path.IsPathRooted exclusively when validating relative path segments and remove the redundant IsWindowsDriveRootedPath helper. This simplifies the combine logic and eliminates an extra, unnecessary root-detection implementation while preserving the existing behavior of throwing for rooted segments.
Update test service setup to use DI-provided application/path and library services. ServiceCollectionBuilder.Build now accepts an optional contentRootPath and forwards it to AddListenarrInfrastructure. BaseTests exposes IApplicationPathService and initializes the service collection with FileService.GetTempPath(). Multiple controller tests now resolve IApplicationPathService and ILibraryListService from the test provider instead of using LibraryControllerMockFactory; tests were adapted to use real repositories/services where appropriate (notably LibraryListSlimPayload). Removed the now-unused tests/Mocks/Api/LibraryControllerMockFactory.cs.
Call services.AddLogging() in the tests' ServiceCollectionBuilder so logging services (ILogger, etc.) are available during tests. This prevents null logger issues and supports components that depend on logging when building the test DI container.
T4g1
approved these changes
May 25, 2026
Contributor
T4g1
left a comment
T4g1
left a comment
There was a problem hiding this comment.
Two remarks left for the tests, but they are non blocking
| } | ||
|
|
||
| public ServiceCollection Build() | ||
| public ServiceCollection Build(string? contentRootPath = null) |
Contributor
There was a problem hiding this comment.
This should be another method WithContentRootPath to define the content root path to use when calling the build method instead if we want to respect the builder pattern
| public void Init() | ||
| { | ||
| _services ??= new ServiceCollectionBuilder().Build(); | ||
| _services ??= new ServiceCollectionBuilder().Build(FileService.GetTempPath()); |
Contributor
There was a problem hiding this comment.
You could add an optional parameter to the method that takes the content root path and calls ServiceCollectionBuilder() with a WithContentRootPath if a path is given, that way we can keep an empty Init() method and you can properly handle the content root path for the tests here
Contributor
There was a problem hiding this comment.
Or we dont even need to change the method Init() and can just give it WithContentRootPath(FileService.GetTempPath()) every time
To be able to chain them, WithContentRootPath should return the ServiceCollectionBuilder with return this (see how it's done in the others ...Builder.cs)
| typeof(LibraryController).GetMethod("ComputeAudiobookBaseDirectoryFromPattern", | ||
| BindingFlags.NonPublic | BindingFlags.Instance)!; | ||
|
|
||
| private LibraryController CreateController(Mock<IFileNamingService> fileNamingService) => |
Contributor
There was a problem hiding this comment.
This is optional but here, i believe you could replace this entire method with a _provider.GetRequiredService() (provided you add it in the ServiceCollectionBuilder)
This applies for every CreateController method you defined. The main advantage is that you no longer need to update the tests setup if you change the constructor of LibraryController, add a service, rename one, and so on
Centralize DI registration by moving IAudiobookMetadataService from listenarr.api/Program.cs into the infrastructure extension. Removed the AddScoped call from Program.cs and added services.AddScoped<IAudiobookMetadataService, AudiobookMetadataService>() in listenarr.infrastructure/Extensions/AppServiceRegistrationExtensions.cs to keep service registrations consolidated.
Introduce a private _contentRootPath and a fluent WithContentRootPath(string?) setter on ServiceCollectionBuilder, and change Build() to use the stored value instead of accepting a parameter. Update BaseTests to use the new fluent API (WithContentRootPath(...).Build()). This refactors how the content root path is provided to the test service builder for clearer, chainable construction.
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
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
This PR cleans up audiobook image loading, moves browser auth to HttpOnly cookie sessions, tightens configuration/API key access rules, slims the library API payload, and improves UI navigation responsiveness.
The frontend now uses direct backend image URLs instead of image tokens or blob/object URL plumbing. Audiobook, author, series, collection, wanted, and modal image paths all go through the simpler protected image helper, with lazy-loaded audiobook covers using shimmer placeholders.
The library list endpoint is now significantly leaner: the backend loads audiobooks without file includes, fetches compact format summaries and per-audiobook file counts in parallel, and returns a slim payload. This reduces memory and I/O on large libraries.
UI navigation is snappier: clicking a sidebar link immediately marks it as active (optimistic
pendingNavPath) without waiting for async route guards to resolve, route transitions use a short page-fade to avoid blank flashes, and the startup-config cache window is extended to 30 s to reduce per-navigation waits.It also updates the Add New search flow so searches only run when submitted, improves keyboard behavior, adds a public bootstrap config endpoint for pre-auth SPA startup, and fixes secret redaction/API key management rules so public remote callers cannot read unredacted secrets when auth is disabled.
This branch also includes cross-platform git hooks for local quality enforcement (pre-commit: lint-staged + layering + async void; pre-push: version sync + backend format + frontend type check + frontend tests) and runtime/CI maintenance for Docker image scanning and build notification behavior.
Changes
Added
StartupBootstrapConfigmodelAppPathServicefor shared app/config/cache path resolutionRequireAdminOrApiKeyWhenAuthenticationEnabledfilterRequireAdministratorSessionWhenAuthenticationEnabledfilter/system/readyendpoint for local tooling and dev server readiness checksIAudiobookRepository.GetAllNoFilesAsync— loads audiobooks without eagerly loading filesIAudiobookFileRepository.GetFormatSummariesAsync— compact per-audiobook format summariesIAudiobookFileRepository.GetCountsByAudiobookIdAsync— file counts by audiobook idpendingNavPath) set on click, cleared after navigation resolvesAddNewViewtests: no auto-search on typing, Enter submission, keyboard accessibility, advanced-field Enter submissionasync voidenforcementdotnet formatverify,vue-tsctype check, vitest run (nonpm run, no.binwrappers)check-vue-template-handlers.mjsChanged
LibraryControllernow parallelizes file-summary/count/download lookups and uses counts forFileCountAudiobooksViewvirtual scroller: responsive items-per-row calculation, measured row height sync, resets visible range on grouping changeWantedViewrelies onlibraryStore.loadinginstead of issuing a redundant library fetchAuthenticationSectionemitsupdate:apiKeydirectly instead of wrapping inStartupConfigformat:checkandformatscripts now also run the vue-handler template checkerlibcap2npm run devreadiness check to use/system/readyFixed
npm runinvokes bash-only.binwrapper scriptsRemoved
lazyLoad.tscustom image observerX-Session-Tokenbrowser session auth pathsaccess_tokensession-token query plumbingmetadataUrlCachein-memory candidate URL cache (image URLs now carry source hint as a query param)'audiobooks'→'books'group query mapping (only valid group values are accepted)Testing
npm run buildnpm run test:unit -- --runnpm run test:unit -- --run src/__tests__/api.ensureImageCached.spec.ts src/__tests__/api.imageUrls.spec.ts src/__tests__/AddNewView.spec.tsdotnet test tests/Listenarr.Api.Tests/Listenarr.Api.Tests.csproj --filter "ConfigurationControllerDownloadClientTests|SessionCookieAuthTests" --no-restore -v minimaldotnet test tests/Listenarr.Api.Tests/Listenarr.Api.Tests.csproj --filter "LibraryController" --no-restore -v minimal