AI-generated analysisPublished automatically and not human-verified. Validated context appears in community notes below.
← Watch feed
Moderate 60 Bitcoin

Server-side translation manager: install, update, and remove language packs from the BTCPay UI (#7347)

Public commit record

What the developer wrote

Authored by Tim

100/100 · Strong
Server-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>
✓ 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
The short version

What changed, and why it matters

This 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.

Recommended action

Reviewers should verify that the SHA-256 comparison is truly case-insensitive and that the manifest URL and raw file base URL are pinned to the expected upstream repository. Confirm that the UninstallLanguagePack guards cannot be bypassed by case variation or alternate Source values, and that the IMemoryCache dependency is correctly registered in production DI. Consider whether manifest fetch failures should degrade silently or alert administrators, and ensure the new route redirects do not create open-redirect opportunities.

Security signals we found

01

SHA-256 verification of downloaded language packs against manifest

02

Server-side authorization guards on uninstall (source check + selected-language check)

03

Typed confirmation modal restored for destructive uninstall action

04

Catchall backward-compatible redirect for old /server/dictionaries routes

05

TryAdd used for manifest name indexing to avoid DoS/crash from duplicate names

06

Case-insensitive SHA comparison in update check and download paths

07

Broad exception handler narrowed to expected network/JSON/manifest exception types

Risk score

Why this scored 60/100

Our methodology →
Potential impact 18/30
Exploitability 12/25
Stealth signal 8/15
Affected reach 10/15
Confidence 8/10
Evidence quality 4/5
Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.