Commit message · TimServer-side translation manager: install, update, and remove language packs from the BTCPay UI (#7347)
* refactor : change dictionary/ies to translation/s
* migrate LanguagePackUpdateService to manifest-based fetching
* refactor : translations UI
* test : add tests
* fix : coderabbit comments
* fix(translations): verify downloaded language pack SHA-256 against manifest
LanguagePackUpdateService.FetchLanguagePackFromRepository previously
returned the manifest's Sha field as the version without ever hashing
the downloaded translation content. The manifest's Sha is SHA-256 of
the source file content, but the value was never compared against the
download, allowing a tampered or corrupted file that still parses as
valid JSON to be saved as a valid language pack.
This change downloads the file as bytes, computes SHA-256, and compares
case-insensitive against the expected Sha from the manifest. On
mismatch, throws InvalidOperationException so the controller's existing
error path returns a user-visible failure instead of silently writing
bad data.
Add LanguagePackUpdateService_RejectsLanguagePackOnShaMismatch test
covering the rejection path. Update the existing
LanguagePackUpdateService_FetchesLanguagePackFromManifest test to use
the actual SHA-256 of the test fixture so the happy path also exercises
the new verification step.
* fix(translations): preserve all old /server/dictionaries paths via catchall
The previous backward-compat shim only handled GET /server/dictionaries
with a single redirect; the seven other old routes (create, edit by id,
select, delete, download) returned 404 under the new controller. The PR
description's promise to "preserve existing integrations and prevent
breaking changes for legacy clients" therefore did not match what was
shipped.
Replace the single-route redirect with a catchall route that captures
any /server/dictionaries/* subpath and 308-permanent-redirects to the
equivalent /server/translations/* URL, preserving the HTTP method (so
POST clients re-POST to the new endpoint instead of degrading to GET).
The existing GET /server/dictionaries handler stays for the bare list
URL.
Also remove the redundant DeleteTranslation action: both it and
UninstallLanguagePack POST routes called localizer.DeleteTranslation
with identical TempData; only UninstallLanguagePack is referenced from
the view. Keep the named route the view uses, drop the dead one.
* fix(translations): restore typed-confirmation modal for Uninstall
ListDictionaries.cshtml had a _Confirm modal requiring the user to
type "Delete" to enable the destructive action; the new
ListTranslations.cshtml replaced it with a single-click submit button
that fired immediately on click. Custom translation packs are
irreversible without re-import and may represent hours of user-edited
entries; one accidental click on the wrong row was enough to lose them.
Restore the typed-confirmation modal pattern used elsewhere in
UIServer (LndSeedBackup, ListUsers, SSHService): replace the inline
form with a link that opens #ConfirmModal, supplying the per-row
description and the "Delete" confirm-input. Add the _Confirm partial
at the end of the view.
Restore the corresponding two test steps in the Playwright integration
test: ConfirmInput.FillAsync("Delete") + ConfirmContinue.ClickAsync()
between the row-Delete click and the alert-message assertion.
* fix(translations): manifest fetch back-off, narrow exception, more tests
Three small fixes on the LanguagePackUpdateService surface:
1. Back-off on manifest fetch failure. Under sustained upstream
outage, every translations-page hit was retrying the manifest
URL because FetchManifest only cached on success. Add a 60-second
back-off after any fetch failure: subsequent calls within the
window throw immediately, which the existing GetManifestLanguages
degraded path translates to a graceful empty response. Stops
the retry hammer without changing the steady-state contract.
2. Narrow the catch in CheckForLanguagePackUpdate. The broad
catch (Exception) loses information about why an update check
fails. Narrow to the four exception types we actually expect to
silence: HttpRequestException, TaskCanceledException,
InvalidOperationException (manifest-back-off + missing-Languages),
and Newtonsoft.Json.JsonException. Programming errors and
unexpected runtime faults bubble up instead of silently
producing "no update available."
3. Three more unit tests filling test coverage gaps:
- LanguagePackUpdateService_ReturnsDegradedModeOnMalformedManifest
covers the JsonReaderException path through the degraded mode.
- LanguagePackUpdateService_ReturnsDegradedModeOnMissingLanguagesKey
covers the InvalidOperationException for a manifest payload
without the expected Languages array.
- LanguagePackUpdateService_ThrowsArgumentExceptionForUnknownLanguage
covers the FetchLanguagePackFromRepository unknown-language
path.
All 8 unit tests pass on .NET 10 RC.2; build clean.
* fix(translations): server-side uninstall guards + duplicate-name tolerance + casing
Address CodeRabbit re-review findings on the previous iteration:
1. UninstallLanguagePack endpoint now validates server-side instead of
relying solely on the UI hiding the Uninstall button. A crafted POST
to /server/translations/{translation}/uninstall could otherwise
delete any translation, including built-in defaults or the currently
selected one. Now: NotFound if the translation does not exist; an
error TempData message if Source != "Custom" or if the translation
is currently selected as the server's display language. The localizer
call only fires when all three guards pass.
2. ListTranslations manifest indexing uses TryAdd instead of
ToDictionary. The previous .ToDictionary call would throw
ArgumentException if upstream manifest contained duplicate or
case-variant Name values, breaking the translations page hard
instead of degrading gracefully. First-wins semantics keep the page
functional even on a malformed upstream payload.
3. CheckForLanguagePackUpdate compares the remote and local SHA hex
strings case-insensitively. The corresponding compare in
FetchLanguagePackFromRepository was already case-insensitive; this
aligns the update-check path with the verification path.
4. SetTranslation test helper asserts the translation is non-null
before saving. Failure mode is now local to the helper rather than
surfacing as a confusing NullReferenceException downstream when
fixture setup changes.
Build clean, 8/8 unit tests pass on .NET 10 RC.2.
* fix(translations): NicolasDorier review pass
- Drop the Translator/ URL prefix from RawBaseUrl. The upstream
fix for issue #7341 in btcpayserver-translator removed the
Translator/translations symlink redirect; raw.githubusercontent.com
does not follow symlinks for file requests, so the existing
RawBaseUrl pointed at a 404. Now resolves against
main/translations/<file>.json directly.
- GetManifestLanguages no longer swallows exceptions. Returns the
entry array directly; caller (UITranslationController) wraps in
try/catch and sets degradedMode locally.
- Drop unused server-side GetAvailableLanguages.
- Cache the parsed LanguageManifestEntry[] for 1 hour so repeat
Translations page hits skip both the http call and the JSON
projection.
- Tests updated to match the new signature + URL.
* fix : coderabbit comment
* little UI fix
* fix(translations): NicolasDorier round-2 review - IMemoryCache + Newtonsoft deserialization
Address Nicolas's 5 inline comments on LanguagePackUpdateService.cs
(PR #7347, head d1f8d3c):
- Replace manual JObject field-picking in ToManifestEntry with
Newtonsoft deserialization to a typed DTO (ManifestLanguageDto +
ManifestRootDto). Maintainer "handle|url" split + Updated parse
live in a single static factory LanguageManifestEntry.FromDto.
Public API of LanguageManifestEntry unchanged (Name, Native,
MaintainerHandle, MaintainerUrl, Updated as DateTimeOffset?,
File, Sha) so callers (UITranslationController) need no edit.
- Collapse the two hand-rolled tuple caches (_manifestCache,
_entriesCache), the SemaphoreSlim _manifestLock, and the
_manifestNextFetchAllowedAt failure-backoff field into a single
IMemoryCache entry keyed "translations.manifest" with a 1h
absolute expiration. The per-language _updateCheckCache
ConcurrentDictionary moves to the same IMemoryCache for
consistency, keyed "translations.update.<language>".
- Drop the 60s failure backoff. If GitHub raw is down, surface
the error per-request rather than holding a sticky-error
window. Faster recovery, no fake-error responses when the
remote has already recovered.
Service constructor now takes (IHttpClientFactory, IMemoryCache);
IMemoryCache is already registered globally via
Startup.AddMemoryCache(), so DI wiring needs no change.
File drops from ~200 to ~140 lines. 8/8 LanguagePackUpdateService
unit tests pass; tests updated to pass a MemoryCache instance.
* fix : differentiation between languagepack and custom
* chore : update default translations
* update translation text
---------
Co-authored-by: r1ckstardev <r1ckstardev@users.noreply.github.com>
Co-authored-by: rockstardev <rockstar@btcpayserver.org>
100/100 · StrongMessage clarity
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference✓ Names security-relevant behavior explicitly
Why it was queuedsigning boundarydefensive validationboot or update path
AI analysis · Moderate 60/100This commit adds a server-side translation manager to BTCPay Server and, in the process, fixes several security-relevant bugs. The most important fix is that downloaded language packs are now verified against a SHA-256 hash from a manifest, preventing a corrupted or tampered translation file from being accepted. The commit also adds server-side guards so built-in or currently-selected translations cannot be uninstalled by a crafted request, restores a typed 'Delete' confirmation before removing custom translations, and redirects old URL paths to new ones so existing integrations keep working. Most of the change is a feature refactor (renaming 'dictionaries' to 'translations' and moving to a manifest-based system), but the security hardening is explicitly described in the commit message and diff.