merge: restrictive CSP and narrowed asset scope (C1, C2)

Set a CSP with script-src 'self' (Tauri nonces the one inline bootstrap script),
object-src/frame-src 'none', and necessarily-permissive img/media/connect for the
user-supplied Jellyfin origin. Narrow assetProtocol $APPDATA/** -> thumbnails/**,
which is convertFileSrc's only remaining caller.

Conflict resolution: scripts/extract-traces.test.ts pinned counts summed rather
than side-picked — DR-189 and DR-198 were added independently on two branches,
so DR 187 -> 189 and total 330 -> 332. docs/traceability.md regenerated.
This commit is contained in:
2026-08-16 23:01:50 +02:00
10 changed files with 4799 additions and 4085 deletions
+61
View File
@@ -51,6 +51,66 @@ pub struct EncryptedFileStorage; // AES-256-GCM fallback
| Token Transmission | Bearer token in `Authorization` header only | | Token Transmission | Bearer token in `Authorization` header only |
| Token Refresh | Handled by Jellyfin server (long-lived tokens) | | Token Refresh | Handled by Jellyfin server (long-lived tokens) |
## Webview Content Security Policy
`app.security.csp` in `tauri.conf.json` (TRACES: UR-012, UR-071 | DR-198). It was
`null` — CSP disabled — which meant any script that reached the web layer
inherited the full IPC surface. Tauri computes the header from this value when it
serves the embedded HTML, injecting a nonce for SvelteKit's inline bootstrap
script, so `script-src` needs no `'unsafe-inline'`.
```
default-src 'self';
script-src 'self';
style-src 'self' 'unsafe-inline';
font-src 'self' data:;
img-src 'self' data: blob: asset: http://asset.localhost http: https:;
media-src 'self' blob: asset: http://asset.localhost http://127.0.0.1:* http: https:;
connect-src 'self' ipc: http://ipc.localhost http: https:;
worker-src 'self' blob:;
object-src 'none'; frame-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'
```
| Directive | Why |
|-----------|-----|
| `default-src 'self'` | Everything not named below is same-origin only. |
| `script-src 'self'` | The genuinely restrictive half. Bundled JS only; Tauri's build-time nonce covers the one inline `<script>` in `index.html`. Adding `'unsafe-inline'` here would silently do nothing anyway — a nonce in a directive voids it. |
| `style-src 'self' 'unsafe-inline'` | Svelte compiles `style="…"` attributes into markup, including `app.html`'s `display: contents` wrapper, and CSP treats a style *attribute* as inline. Safe only while no `<style>` **element** survives into `index.html`: Tauri would nonce it, and the nonce would then void `'unsafe-inline'`. The production build extracts all CSS to files, so it currently has none. |
| `img-src` | Thumbnails come from two places: the asset protocol (`asset://localhost/…` on Linux/macOS, `http://asset.localhost/…` on Windows/Android — the same protocol, named differently by `convertFileSrc`) and, on a cache miss, straight from the Jellyfin server. `data:`/`blob:` cover inline and generated images. |
| `media-src` | `<video>`/`<audio>` sources: HLS transcodes and progressive streams from the server, the token-guarded loopback media server on `http://127.0.0.1:<random port>` (DR-137), and `blob:` for the MSE object URL hls.js attaches. |
| `connect-src` | `ipc:` / `http://ipc.localhost` is Tauri's `invoke` transport (custom scheme on Linux/macOS, `http` host on Windows/Android) — without it every command is blocked. `http:`/`https:` is hls.js fetching manifests and segments; ordinary API traffic goes through Rust and is not subject to CSP. |
| `worker-src 'self' blob:` | hls.js runs its demuxer in a worker built from a blob (`enableWorker: true`). Without `blob:` it falls back to main-thread demuxing — playback survives but costs more CPU. |
| `object-src`, `frame-src` = `'none'` | No plugins, no iframes; both are classic injection sinks. |
| `base-uri 'self'`, `form-action 'self'`, `frame-ancestors 'none'` | Block `<base>` hijacking, form exfiltration and framing. `frame-ancestors` is only honoured when the policy is delivered as a header, which is platform-dependent; it is harmless where it is not. |
**`img-src`/`media-src`/`connect-src` are deliberately permissive.** The Jellyfin
origin is typed in by the user at run time and is routinely plain `http` on a
LAN, so it cannot be enumerated at build time. `http: https:` is a wide grant for
*data* — but it still bars `file:`, `filesystem:` and scripting schemes, and it
does not touch `script-src`, which is where an injected origin would actually
hurt. A run-time policy naming the server exactly was considered and rejected:
Tauri derives the header from immutable config at the moment it serves the HTML,
so it would mean rebuilding the config and reloading the webview whenever the
user adds or switches a server, to constrain a destination the user chooses
anyway.
`devCsp` mirrors the policy with `'unsafe-inline' 'unsafe-eval'` on `script-src`
and `ws:`/`wss:` on `connect-src`, because the Vite dev server injects styles and
code and drives HMR over a websocket. It applies only to `tauri dev`.
### Asset protocol scope
`app.security.assetProtocol.scope` is `$APPDATA/thumbnails/**` — not the storage
root. `imageCache.ts` is the only `convertFileSrc` caller left in the frontend:
downloaded media moved to the loopback media server in DR-137, and downloaded
audio is opened by MPV/ExoPlayer directly from its path. The old `$APPDATA/**`
grant let the webview read the SQLite database and the encrypted-token fallback
file alongside the thumbnails it actually needs.
If a new feature hands the webview a local file, widen this scope to that
subdirectory specifically; a path outside it resolves to nothing and the webview
reports `NETWORK_NO_SOURCE` (which is exactly how DR-134's failure presented).
## Local Data Protection ## Local Data Protection
| Data Type | Protection | | Data Type | Protection |
@@ -67,3 +127,4 @@ pub struct EncryptedFileStorage; // AES-256-GCM fallback
3. **Logout Cleanup**: Token deletion from secure storage on logout 3. **Logout Cleanup**: Token deletion from secure storage on logout
4. **No Token Logging**: Tokens are never written to logs or debug output 4. **No Token Logging**: Tokens are never written to logs or debug output
5. **IPC Security**: Tauri's IPC uses structured commands, not arbitrary code execution 5. **IPC Security**: Tauri's IPC uses structured commands, not arbitrary code execution
6. **Webview Containment**: A restrictive `script-src` keeps injected script off the IPC surface; the asset protocol is scoped to the thumbnail cache only (see above)
+469
View File
@@ -0,0 +1,469 @@
# JellyTau Codebase Audit
**Date:** 2026-08-16 · **Version:** v0.6.0 · **Commit:** `be907b49` (master)
A review of the Rust/Svelte/Android codebase against its own requirements matrix
and against current Android and Tauri v2 platform practice. Every finding was
verified by running the project's own tooling or reading the code it points at —
nothing here is inferred from documentation alone.
**Scale:** 55,835 LOC Rust · 50,490 LOC TS/Svelte · 530 requirements · 824 traces
| Severity | Count |
|----------|-------|
| High | 5 |
| Medium | 10 |
| Low | 5 |
| Tests passing | 1,719 |
| Untraced requirements | 86 |
| Traceability coverage | 86% (285/330) |
> **Revisions, 2026-08-16.** Three rankings changed after device testing and
> platform research, all documented in place:
> - **B1 High → Low.** The predicted impact was refuted on a physical Android 16
> device. The residual risk turned out to be a different, narrower one.
> - **B7 Low → Medium, re-framed.** The original reading of predictive back was
> backwards: at targetSdk 36 it is already enabled, not merely un-opted-into.
> - **B8 added (Medium).** Android 16 Local Network Protections versus a
> LAN-hosted Jellyfin server.
>
> Original ranking was 6 High / 8 Medium / 5 Low.
**Verified by running:** `bun run check` · `bun run test` · `cargo test` ·
`cargo clippy --all-targets` · `bun run check:boundary` · `bun run traces:json`
**Device-verified (2026-08-16):** B1 and B2 were checked against a physical HONOR
ROD2-W09 running Android 16 (SDK 36) with the shipped app installed. B2 was
confirmed; B1 was refuted and downgraded.
**Not covered:** the e2e suite (`test:e2e` is not wired into CI and was not run),
Windows and Arch packaging paths, and the docs-site build. B3, C1 and C2 still
need a device/desktop playback pass.
---
## A. Requirements versus code
The traceability matrix is the project's own claim about what is built. Of 530
defined requirement IDs, 86 carry no `TRACES:` tag anywhere in the tree. Most of
those gaps are documentation debt rather than missing features — which is
precisely the problem, because it makes the matrix unreliable as evidence.
### A1 · High · Twelve requirements are marked "Done" but have zero traces
`UR-006` (lockscreen/BLE control), `UR-037` (video library presentation),
`IR-006` (Android MediaSession), `IR-008` (audio focus), `IR-022` (person/cast
API), `IR-024` (home-screen API) and six Jellyfin API requirements (`JA-006`,
`JA-009`, `JA-013`, `JA-014`, `JA-015`, `JA-018`) all claim completion with
nothing pointing at an implementation.
These features demonstrably work — lockscreen control, Next Up, favourites are
all shipped. The code is there; the tags are not. That means the matrix currently
over-reports on exactly the requirements a reviewer would most want to verify,
and a regression in any of them would leave no trace to follow.
**Fix:** Tag the existing implementations. Highest value per keystroke in the
whole audit: six of the twelve are single Jellyfin API call sites.
### A2 · Medium · Requirement statuses contradict each other across layers
`UR-020` (subtitle selection) and `UR-021` (audio track selection) are marked
*Done*, while the integration requirements they decompose into — `IR-018` and
`IR-019`, both libmpv-specific — are still *Planned*. Similarly `IR-005` (MPRIS)
sits at *Planned* under a *Done* `UR-006`.
The likely truth is that these user requirements were satisfied through a
different path than the one originally specified (HTML5 `<video>` and ExoPlayer
rather than libmpv), and the IRs were never re-scoped. Left as-is, the matrix
reads as though shipped features depend on unbuilt integrations.
**Fix:** Re-scope or retire the stale IRs so each Done UR rests on Done IRs.
### A3 · Medium · The traceability gate is set far below actual coverage
`traceability-check.yml` fails only below 50%. Real coverage is well above that,
so the gate cannot catch a coverage regression until roughly half the matrix has
rotted. A gate that can only fire after a catastrophe is not protecting anything.
**Measured coverage: 86% (285/330)** — UR 71/75, IR 19/32, DR 166/187, JA 29/36.
IR is by far the weakest dimension, which corroborates A1.
**Fix applied:** `MIN_THRESHOLD` ratcheted 50 → 82, with the ratchet policy
written into the workflow (only goes up; never lowered to make a red build pass).
The same figure is mirrored as `MIN_COVERAGE_PERCENT` in
`scripts/extract-traces.ts` so local `traces:coverage` gates on the same bar, and
a test parses the workflow YAML and fails if the two drift apart.
### A4 · Low · Two traced IDs do not exist in the requirements document
`DR-189` and `UT-188` are referenced by `TRACES:` comments but are defined
nowhere in `docs/requirements.md`. The extraction tool accepts them silently, so
typos and renames pass unnoticed.
**Fix:** Add a dangling-ID check to the extractor and fail CI on it — cheap, and
it keeps the matrix honest in both directions.
### A5 · Not a gap · The remaining untraced requirements are legitimately unbuilt
`UR-016`, `UR-022` and `UR-070` are Planned or Proposed, and `UR-031`
(crossfade) is explicitly blocked by `DR-034`. Their absence from the trace graph
is correct and needs no action — noted so it does not get swept into the fix list.
---
## B. Android platform practice
The app targets SDK 36 with a minSdk of 24. Several manifest and WebView settings
still reflect an earlier target level.
### B1 · Low · `POST_NOTIFICATIONS` is declared but never requested at runtime
*Downgraded from High. The original ranking was refuted by device testing — the
evidence is below, and it is the reason this finding is now near-trivial.*
The permission appears in the manifest, but there is no `requestPermissions` call
anywhere in the Kotlin, Rust or TypeScript sources, and
`JellyTauPlaybackService.startForeground()` runs with no `checkSelfPermission`
guard. On Android 13+ notification permission defaults to denied.
This was ranked High on the theory that it would suppress the media notification
and with it the lockscreen transport controls (`UR-006`). Testing on an HONOR
ROD2-W09 running **Android 16 (SDK 36)**, with the shipped app installed and
playing, shows otherwise. The permission is genuinely denied:
```
POST_NOTIFICATIONS: granted=false, flags=[USER_SENSITIVE_WHEN_GRANTED|USER_SENSITIVE_WHEN_DENIED]
appops POST_NOTIFICATION: ignore
```
and the notification is nonetheless live and complete:
```
ServiceRecord{... com.dtourolle.jellytau/.player.JellyTauPlaybackService}
isForeground=true foregroundId=1 types=0x00000002
foregroundNoti=Notification(flags=NO_CLEAR|FOREGROUND_SERVICE
category=transport actions=3 vis=PUBLIC)
```
Foreground-service notifications are exempt from `POST_NOTIFICATIONS` — a
foreground service cannot run without one. All three transport actions are
present. **`UR-006` is not at risk.**
What remains is minor. The FGS notification is the only one the app ever posts:
the single `notificationManager.notify(NOTIFICATION_ID, …)` call updates that same
foreground notification, so it inherits the exemption while the service is
foreground. The declared permission therefore currently buys nothing.
**The real risk here is not the permission — it is how narrowly the exemption is
earned.** AOSP's `Notification.isMediaNotification()` grants it only when the
style is `MediaStyle`/`DecoratedMediaCustomViewStyle` **and**
`Notification.EXTRA_MEDIA_SESSION` holds a non-null *platform* session token. If
either is missing while the permission is denied, the notification is **silently
suppressed** — no exception, no log.
JellyTau earns it at two sites, both of which hang it on a null-safe call:
```kotlin
androidx.media.app.NotificationCompat.MediaStyle()
.setMediaSession(mediaSessionCompat?.sessionToken) // :273 and :466
```
Ordering currently saves it — `mediaSessionCompat` is assigned in `onCreate`
(:195) and `createBasicNotification()` is only reached from `onStartCommand`
(:251) — and the device test confirms it works. But it is one reordering away
from breaking invisibly, and only for users who denied the permission, which is
a population most developers never test as.
**Fix:** Keep the permission declared — download-service FGS notifications are
*not* covered by the media exemption, and this app has a downloads feature that
may want them. Comment both `setMediaSession` sites to record what earns the
exemption, and log loudly if the token is ever null at build time, converting a
silent failure into a diagnosable one.
**Location:** `src-tauri/android/src/main/java/com/dtourolle/jellytau/player/JellyTauPlaybackService.kt:251`, `:273`, `:466`
### B2 · High · Cloud backup is on by default, and it will break credential restore
The manifest sets neither `android:allowBackup="false"` nor a
`dataExtractionRules`/`fullBackupContent` file, so Android's default applies: the
app's data directory is backed up to the user's Google account. That ships the
SQLite catalogue — library metadata and watch history — off the device.
The credential path makes it worse rather than better. `SecureStorage.kt`
encrypts with AES/GCM under an Android Keystore key, and Keystore keys are never
backed up. A user restoring onto a new phone therefore gets the ciphertext
without the key: undecryptable credentials and a silent authentication failure,
with no code path that recognises the situation.
**Fix applied.** `allowBackup="false"`. Extraction rules that merely excluded the
DB and credential prefs would have left nothing worth backing up: the SQLite
catalogue is a rebuildable mirror of the server and watch state lives server-side,
so there is no user-authored data to preserve.
**A gap this audit missed:** on API 31+, `allowBackup="false"` disables *cloud*
backup but **not device-to-device transfer**, which reproduces the identical
failure — the prefs travel, the Keystore key does not. A
`data_extraction_rules.xml` excluding all five domains from both `<cloud-backup>`
and `<device-transfer>` was added to close it.
**A real bug found while fixing this:** the Rust encrypted-file fallback in
`credentials.rs` propagated a decrypt failure as `CredentialError::Encryption`,
which `storage_get_access_token` turned into a hard `Err` — so an undecryptable
blob was an error state, not a logout. It now logs and returns an empty map, so
the caller sees `NotFound``Ok(None)` → login screen, and the next sign-in
self-heals the file. `SecureStorage.getCredential` on the Kotlin side already
returned null, but could not distinguish "nothing stored" from "unreadable" and
left the dead blob in prefs forever; it now separates the cases and discards it.
Three tests written and watched fail first, per the red→green rule.
### B3 · High · `MIXED_CONTENT_ALWAYS_ALLOW` undoes the network security config
`network_security_config.xml` is careful and well-argued: cleartext blocked
everywhere, exempted only for `127.0.0.1` so the local media server can serve
downloads. Its own comment warns "this must not become a blanket cleartext
opt-in."
But `MainActivity.kt` sets `mixedContentMode = MIXED_CONTENT_ALWAYS_ALLOW`, which
permits the WebView to load http subresources into an https page from any origin.
Alongside it, `allowFileAccess = true` and `allowContentAccess = true` are both
broader than anything the app needs, since Tauri serves the UI from its own scheme
and media comes from the token-guarded loopback server. These read as leftovers
from before the media server existed.
**Fix:** Drop to `MIXED_CONTENT_COMPATIBILITY_MODE` and set both file and content
access to false, then verify offline video still plays.
**Location:** `src-tauri/android/src/main/java/com/dtourolle/jellytau/MainActivity.kt:504-507`
### B4 · Medium · Android TV is half-declared
The manifest advertises `LEANBACK_LAUNCHER` and a non-required leanback feature,
but omits `<uses-feature android:name="android.hardware.touchscreen"
android:required="false"/>` and an `android:banner`. That combination fails Play's
TV validation, and on a real TV the app would launch into a UI with no D-pad focus
model behind it.
**Fix:** Either commit to TV — add the feature declaration, a banner, and a focus
pass — or remove the leanback category until you do.
### B5 · Medium · `jvmTarget` is pinned to 1.8 under compileSdk 36
The Kotlin target has not moved with the SDK. AGP 8 warns on it, and it locks the
Kotlin sources out of APIs and desugaring behaviour that everything else in the
toolchain assumes.
**Fix:** Move `jvmTarget` and the Java source/target compatibility to 17.
### B6 · Low · Media3 is several minor versions behind
`androidx.media3` is pinned at 1.5.0 across exoplayer, hls, session and common.
Given how much of this app's hard-won behaviour lives in ExoPlayer edge cases —
truncated progressive streams, background audio handoff, HLS resume — staying
current on its bug-fix releases has unusually high value here.
**Fix:** Schedule a Media3 bump with a device pass over the playback regression list.
### B7 · Medium · Predictive back is already on, not merely un-opted-into
*Upgraded from Low, and re-framed — the original framing was backwards.*
The audit first read the absent `enableOnBackInvokedCallback` as the app
*forgoing* the Android 13+ back-gesture preview. That is not what the flag means
at this target level. Predictive back is enabled by default for apps targeting
recent SDKs, and Android 16's own behaviour-change list carries "Migration or
opt-out required for predictive back" — with the opt-out being removed. Targeting
36, JellyTau is already getting predictive back; it simply hasn't been checked
against it.
That matters more than a missing opt-in would, because the app does not use
ordinary Android back. It runs a WebView with its own history model —
`src/lib/utils/navigation.ts` tracks a depth counter, applies a popstate delta,
and falls back to a path when `history.back()` would trap the user, with
`scrollRestore.ts` keying off the same popstate events. That is exactly the kind
of custom back handling predictive back is most likely to disagree with.
**Fix:** This is a device test, not a code change — exercise the back gesture
(including the drag-and-release preview and the cancel) from a library page, a
detail page, the player, and the settings screen, and watch for the depth counter
desynchronising. Only change code if it misbehaves.
Separately and unrelatedly: `JellyTauPlaybackService` is `exported="true"` with a
`MediaSessionService` intent filter — conventional for Media3, but it means any
app on the device can attempt to bind and drive playback. Confirm the session's
`onConnect` callback rejects unknown packages.
### B8 · Medium (forward-looking) · Android 16 Local Network Protections vs a LAN Jellyfin server
*New finding, surfaced while researching B1.*
Android 16's behaviour-change list includes **Local Network Permission**. JellyTau's
entire purpose is reaching a Jellyfin server that, for most users, sits on the
local network — so a permission gate on local-network access is a direct threat to
the app's core function, not a peripheral concern.
Stated carefully, because the timing matters: in Android 16 this is **opt-in for
testing**, not enforced by default, with enforcement signalled for a future
release. Nothing is broken today, and the device test will not surface it. But
this is the rare platform change that could stop the app working at all, and it
is much cheaper to handle before it is mandatory.
**Fix:** Investigate what the permission will require, then test the app against
it with the opt-in flag enabled on the Android 16 device already to hand. Track it
as a release-blocking item for whichever Android version enforces it.
---
## C. Tauri v2 configuration
The capability model here is genuinely well done — see section E. The gaps are in
the two settings that govern what a compromised web layer could reach.
### C1 · High · `"csp": null` contradicts the project's own security convention
`CLAUDE.md` lists "keep the CSP restrictive in `tauri.conf.json`" as a standing
rule; the config disables CSP entirely. With it off, any script that reaches the
web layer inherits the full IPC surface.
The realistic exposure today is low, and worth stating plainly rather than
inflating: the frontend has a single `{@html}` — an app-owned icon in
`GenericGenreBrowser.svelte`, not server data — and no `innerHTML`, `eval` or
`new Function` outside tests. So this is a missing defence rather than an open
hole. But it is the defence that stops the next careless interpolation of a
Jellyfin-supplied string from becoming a full compromise.
**Fix:** Set a CSP permitting `'self'`, `asset.localhost`, `http://127.0.0.1:*`
for media, and the configured Jellyfin origin for images. Expect one or two
iterations against HLS playback.
### C2 · Medium · The asset protocol scope is wider than what it serves
`assetProtocol.scope` is `$APPDATA/**`, which covers the whole app data directory
— the SQLite database and the credential store included — while the protocol only
needs to reach cached thumbnails and downloaded media.
Since `DR-137` introduced the token-guarded loopback media server, the asset
protocol's remaining job may be thumbnails alone, which would make the narrowing
nearly free.
**Fix:** Scope it to the thumbnail and download subdirectories, and confirm
nothing else still resolves through `convertFileSrc`.
### C3 · Low · Shipped desktop bundles have no update path
The bundle targets deb, rpm and nsis, but `tauri-plugin-updater` is not among the
dependencies. Every desktop user upgrades by manually fetching a new package,
which in practice means a long tail of installs pinned to whatever version they
first downloaded.
**Fix:** Add the updater plugin with a signed release manifest, or document the
manual upgrade path in the README so the omission is at least deliberate.
---
## D. CI and code health
Local discipline in this project is strong and well documented. CI enforces only
part of it, which means the discipline holds exactly as long as every contributor
remembers it.
### D1 · High · CI runs neither `cargo clippy` nor `cargo fmt --check`
`CLAUDE.md` requires both before committing. Neither appears anywhere in
`.gitea/workflows/`. The build-and-test job runs the boundary check, the frontend
tests, the Rust tests and an Android `cargo check` — a good set, with the two lint
gates missing.
Clippy currently reports 51 warnings across the lib and its tests, including
unused imports and a redundant import that a gate would have stopped at the door.
**Fix:** Add both to the test job. Start with `-D warnings` on new code only if
clearing the existing 51 is too large a first step.
### D2 · Medium · A flaky test will intermittently redden CI
`offlineCatalog.test.ts` — "pushes include=true while the server is reachable"
(`UT-068`) — timed out at the 5 s limit during a full-suite run, then passed twice
in isolation taking 1.13 s and 0.61 s.
**Root cause (corrected):** this audit originally attributed it to a real
wall-clock timer. It isn't. The cost is the **first dynamic
`import("./offlineCatalog")`**, which pays to transform the service and its whole
dependency graph (~1072 ms cold) inside a test body, charged against vitest's 5 s
default. Later re-imports after `vi.resetModules()` cost ~30 ms. Under full-suite
contention the cold transform alone crosses the limit.
**Fix applied:** warm the import once at collection time with a top-level
`await import(...)`, so no test is timing the compiler. Slowest test 1072 ms →
129 ms; file total 1170 ms → 238 ms. Timeout deliberately left at the default.
A latent cross-test leak was also fixed alongside it — the store shim's
subscribers were never cleared, so every module instance discarded by
`resetModules()` kept pushing its own visibility value.
**Location:** `src/lib/services/offlineCatalog.test.ts:58`
### D3 · Medium · 820 `unwrap()`/`expect()` calls sit outside test code
They cluster in exactly the files that have historically produced the worst bugs:
`player/mod.rs` (145), `repository/offline.rs` (125), `storage/mod.rs` (70),
`commands/download/mod.rs` (51). A panic inside a Tauri command kills the task and
can leave shared player state inconsistent.
Related: 33 raw `.lock().unwrap()` / `.read().unwrap()` / `.write().unwrap()`
calls remain despite the project's own `MutexSafe`/`RwLockSafe` convention, so
poison recovery is not uniform.
**Fix:** Sweep the command-handler paths first, since those are the ones with a
`Result<T, String>` to return into. Convert the 33 raw locks to the safe helpers
as a mechanical pass.
### D4 · Low · Five files carry a disproportionate share of the complexity
`player/mod.rs` (4,726 lines), `repository/offline.rs` (4,696),
`repository/online.rs` (3,702), `commands/player/mod.rs` (3,299) and
`commands/download/mod.rs` (3,226), plus `VideoPlayer.svelte` (2,778) on the
frontend.
These are the same files the changelog keeps returning to for deadlocks and
playback regressions. Not a defect in itself, and not worth a speculative
refactor — but the next time one of them needs substantial work, splitting it is
likely cheaper than continuing to grow it.
---
## E. Verified sound
Things this audit specifically went looking for and found in good order —
including one that looked alarming from the warning output and turned out to be
fine.
| Area | Finding |
|------|---------|
| **The 9 "MutexGuard across await" warnings are test-only** | All nine sit in `#[tokio::test]` functions holding a serialization lock, not in the production async paths that `CLAUDE.md`'s deadlock gotcha warns about. |
| **The local media server is exemplary** | Loopback-only bind, a 32-hex-char per-session token, lexical `..` folding rather than `canonicalize`, and a test asserting reads stay inside the data directory. |
| **Tauri capabilities are minimal** | Three permissions total — `core:default`, `opener:default`, `core:path:default`. No blanket grants, no `withGlobalTauri`. |
| **SQL is parameterised** | Two `format!`-built statements in the whole Rust tree, neither interpolating caller-controlled input into a query. |
| **R8 keep rules are correct and explained** | JNI-loaded player and security classes, the JavascriptInterface bridges and Media3 are all kept, each with a comment naming the crash it prevents. |
| **Type and boundary gates are green** | `svelte-check`: 0 errors, 0 warnings. `check:boundary` passes with three reviewed allowlist entries. 698 Rust tests and 1,021 frontend tests pass. |
---
## F. Suggested order
Sequenced so the cheap gates land before the work they would have caught. B2
leads because it is the finding a user is most likely to actually feel.
*(B1 originally led this list. It was demoted to row 10 after device testing —
see B1. This is a good advertisement for testing a finding before scheduling
work against it.)*
| # | Finding | What it buys | Effort |
|---|---------|--------------|--------|
| 1 | B2 | Catalogue and credentials stop leaving the device; restore stops failing silently | S — **confirmed on device**: `ALLOW_BACKUP` set, Google transport active |
| 3 | D1 | Lint discipline becomes enforced rather than remembered | S |
| 4 | B3 | The network security config actually holds | S — needs an offline-playback check |
| 5 | A1 | The matrix stops over-reporting on twelve shipped requirements | M — mostly mechanical |
| 6 | D2 | CI stops flaking | S |
| 7 | C1 · C2 | The web layer stops being one interpolation away from full IPC | M — iterate against HLS |
| 8 | A2 · A3 · A4 | The matrix becomes self-consistent and defended by a real gate | M |
| 9 | B4 · B5 · B6 · B7 | Platform hygiene brought level with the SDK target | M |
| 10 | B1 · D3 · C3 · D4 | Long-tail robustness; opportunistic rather than scheduled | L |
+5 -3
View File
@@ -328,7 +328,7 @@ Internal architecture, components, and application logic.
| DR-131 | The offline mutation queue is drained. `sync_queue` had producers and no consumer: `PlaybackReporter::queue_for_sync` writes a row for every start/stop/mark-played that cannot reach the server, `sync_mark_processing`/`_completed`/`_failed` were registered commands with no callers, and no Rust task processed the table — so queued watch positions never reached Jellyfin and the offline banner's count only ever grew. A drain hangs off the same `connectivity:reconnected` transition as DR-120 (in Rust, because a drain started by a component dies with it) and replays rows oldest-first, so a stale start cannot move the server's resume position backwards after a later stop. `update_progress` replays as *stopped at N* rather than as progress — replaying a mid-playback report hours later would claim the item is still playing — and payloads are read in both dialects that exist in users' databases (`position_ticks` from Rust, camelCase `positionMs` from the frontend helper). A failed row stays queued for the next reconnect; after `MAX_SYNC_ATTEMPTS` it is `abandoned` and stops counting, because a row nothing can ever push is what turns the queue into a counter that only grows. An *unreachable* server is not counted as an attempt at all — the row goes back to `pending` untouched — so opening the app offline a few times cannot abandon good rows; only a server that answers and refuses spends the budget. The drain also runs once at startup, because a queue built in a previous session would otherwise sit untouched for a whole run whenever the server was reachable the entire time and no offline→online transition ever fired. Requires `MediaRepository::mark_played` (JA-035) — the previous stand-in reported a stop at `i64::MAX` | Backend | UR-025, UR-002 | Done | | DR-131 | The offline mutation queue is drained. `sync_queue` had producers and no consumer: `PlaybackReporter::queue_for_sync` writes a row for every start/stop/mark-played that cannot reach the server, `sync_mark_processing`/`_completed`/`_failed` were registered commands with no callers, and no Rust task processed the table — so queued watch positions never reached Jellyfin and the offline banner's count only ever grew. A drain hangs off the same `connectivity:reconnected` transition as DR-120 (in Rust, because a drain started by a component dies with it) and replays rows oldest-first, so a stale start cannot move the server's resume position backwards after a later stop. `update_progress` replays as *stopped at N* rather than as progress — replaying a mid-playback report hours later would claim the item is still playing — and payloads are read in both dialects that exist in users' databases (`position_ticks` from Rust, camelCase `positionMs` from the frontend helper). A failed row stays queued for the next reconnect; after `MAX_SYNC_ATTEMPTS` it is `abandoned` and stops counting, because a row nothing can ever push is what turns the queue into a counter that only grows. An *unreachable* server is not counted as an attempt at all — the row goes back to `pending` untouched — so opening the app offline a few times cannot abandon good rows; only a server that answers and refuses spends the budget. The drain also runs once at startup, because a queue built in a previous session would otherwise sit untouched for a whole run whenever the server was reachable the entire time and no offline→online transition ever fired. Requires `MediaRepository::mark_played` (JA-035) — the previous stand-in reported a stop at `i64::MAX` | Backend | UR-025, UR-002 | Done |
| DR-132 | The pending-sync count is answerable. The offline banner's badge read "N pending sync(s)" and led nowhere, so it was taken for pending *transfers* and looked for on the Downloads page — which lists the `downloads` table and structurally cannot show `sync_queue` rows. The badge becomes a button opening the queue it counts: each row's operation, the item's title (resolved by a `LEFT JOIN items` in `sync_get_pending`, not a per-row frontend fetch), when it was queued, and the error of anything failing, plus a "Sync now" that runs the DR-131 drain on demand. The same list is a Settings section, because a row that keeps failing is still queued when the server is reachable and no banner is on screen. The drain emits `sync-queue-changed` so the badge updates on reconnect instead of lagging by up to one 10s poll | UI | UR-025 | Done | | DR-132 | The pending-sync count is answerable. The offline banner's badge read "N pending sync(s)" and led nowhere, so it was taken for pending *transfers* and looked for on the Downloads page — which lists the `downloads` table and structurally cannot show `sync_queue` rows. The badge becomes a button opening the queue it counts: each row's operation, the item's title (resolved by a `LEFT JOIN items` in `sync_get_pending`, not a per-row frontend fetch), when it was queued, and the error of anything failing, plus a "Sync now" that runs the DR-131 drain on demand. The same list is a Settings section, because a row that keeps failing is still queued when the server is reachable and no banner is on screen. The drain emits `sync-queue-changed` so the badge updates on reconnect instead of lagging by up to one 10s poll | UI | UR-025 | Done |
| DR-133 | A downloaded file has exactly one on-disk path, and the row that names it is authoritative. `downloads.file_path` starts relative to the storage root, but the worker rewrites it to the absolute path it actually wrote when the transfer completes — so a *completed* row is already rooted. The video player's offline branch rooted it a second time, handing the asset protocol `/data/user/0/app//data/user/0/app/videos/x.mp4`; the webview reported `MEDIA_ERR_SRC_NOT_SUPPORTED` with `NETWORK_NO_SOURCE`, so every downloaded video failed to play while audio — which resolves the same column through Rust's `resolve_local_media_path`, without re-rooting — played fine. The join is absolute-aware (POSIX, Windows drive letters and UNC) so rows written before completion still resolve | Playback | UR-071 | Done | | DR-133 | A downloaded file has exactly one on-disk path, and the row that names it is authoritative. `downloads.file_path` starts relative to the storage root, but the worker rewrites it to the absolute path it actually wrote when the transfer completes — so a *completed* row is already rooted. The video player's offline branch rooted it a second time, handing the asset protocol `/data/user/0/app//data/user/0/app/videos/x.mp4`; the webview reported `MEDIA_ERR_SRC_NOT_SUPPORTED` with `NETWORK_NO_SOURCE`, so every downloaded video failed to play while audio — which resolves the same column through Rust's `resolve_local_media_path`, without re-rooting — played fine. The join is absolute-aware (POSIX, Windows drive letters and UNC) so rows written before completion still resolve | Playback | UR-071 | Done |
| DR-134 | The webview can actually fetch the local files it is handed. `convertFileSrc` rewrites a path to `http://asset.localhost/…` unconditionally, but Tauri only answers that origin when the `protocol-asset` cargo feature is compiled in *and* `app.security.assetProtocol.enable` is set — neither was, so every such URL reached a protocol with no handler and the webview reported `NETWORK_NO_SOURCE`. This silently defeated both offline video (`<video src>`) and the cached-thumbnail path in `imageCache`, which fails soft to the server copy and so hid the breakage whenever the server was reachable. The scope is `$APPDATA/**` — the storage root under which the database, `downloads/` and the thumbnail cache all live — rather than an unrestricted grant, so the webview can read the app's own media and nothing else | Security | UR-071 | Done | | DR-134 | The webview can actually fetch the local files it is handed. `convertFileSrc` rewrites a path to `http://asset.localhost/…` unconditionally, but Tauri only answers that origin when the `protocol-asset` cargo feature is compiled in *and* `app.security.assetProtocol.enable` is set — neither was, so every such URL reached a protocol with no handler and the webview reported `NETWORK_NO_SOURCE`. This silently defeated both offline video (`<video src>`) and the cached-thumbnail path in `imageCache`, which fails soft to the server copy and so hid the breakage whenever the server was reachable. The scope was `$APPDATA/**` — the storage root under which the database, `downloads/` and the thumbnail cache all live — rather than an unrestricted grant; DR-198 narrows it further to `$APPDATA/thumbnails/**`, since DR-137 moved downloaded media off this protocol and thumbnails are all it still serves | Security | UR-071 | Done |
| DR-140 | An audio track is pinned only when the user picked one. Jellyfin's `MediaStream.Index` is global across every stream in a media source, so index 0 is the *video* stream on virtually all files — yet `AudioStreamIndex=0` was sent as "the first audio track" on the HLS transcode URL, the background audio-only handoff URL, the direct-play fallback URL, and the `PlaybackInfo` negotiation body. A server that honours the request literally then transcodes the video stream into the audio slot and the result plays as a picture with no sound; only servers that silently correct the index hid the bug, which is why it presented as "some videos have no audio". The parameter is now omitted whenever no track has been chosen, so the server resolves the source's `DefaultAudioStreamIndex`; an explicit selection from `player_switch_audio_track` is still carried through unchanged. On the `static=true` direct-play URL it is dropped outright — the original file is served untouched, so the parameter could only mislead | Playback | UR-004, UR-040 | Done | | DR-140 | An audio track is pinned only when the user picked one. Jellyfin's `MediaStream.Index` is global across every stream in a media source, so index 0 is the *video* stream on virtually all files — yet `AudioStreamIndex=0` was sent as "the first audio track" on the HLS transcode URL, the background audio-only handoff URL, the direct-play fallback URL, and the `PlaybackInfo` negotiation body. A server that honours the request literally then transcodes the video stream into the audio slot and the result plays as a picture with no sound; only servers that silently correct the index hid the bug, which is why it presented as "some videos have no audio". The parameter is now omitted whenever no track has been chosen, so the server resolves the source's `DefaultAudioStreamIndex`; an explicit selection from `player_switch_audio_track` is still carried through unchanged. On the `static=true` direct-play URL it is dropped outright — the original file is served untouched, so the parameter could only mislead | Playback | UR-004, UR-040 | Done |
| DR-147 | One search input per screen, and the URL is the search's single source of truth. The header bar rendered only under `/library/**` and merely *navigated* to `/search` (DR-063), so a desktop search handed the user to a screen whose input was a different element — the header box cleared itself and vanished, and the page's own box took over mid-word. That page then re-derived its input from `?q=` against `library.searchQuery` on every store write, so the next keystroke re-ran the effect and snapped the text back to the query the header had sent (and a scope chip back to the URL's scope); entering from the bottom-nav Search tab skipped it only because the effect early-returned on an empty query. The bar now renders on `/search` too (`showHeaderSearch`) and is the sole md+ input — the page's own input is `md:hidden` — and on that route it republishes the query into the URL with `replaceState`, so a whole session of typing costs one history entry. The page *consumes* that URL once per distinct value (`seedFromSearchUrl` against a non-reactive `applied` marker) instead of continuously reconciling it, and the scope chips publish through the same URL so the bar and the chips cannot disagree. Landing on `/search` with a seeded query focuses the bar and puts the caret at the end, because the box the user was typing in belonged to the unmounted route | UI | UR-049, UR-054 | Done | | DR-147 | One search input per screen, and the URL is the search's single source of truth. The header bar rendered only under `/library/**` and merely *navigated* to `/search` (DR-063), so a desktop search handed the user to a screen whose input was a different element — the header box cleared itself and vanished, and the page's own box took over mid-word. That page then re-derived its input from `?q=` against `library.searchQuery` on every store write, so the next keystroke re-ran the effect and snapped the text back to the query the header had sent (and a scope chip back to the URL's scope); entering from the bottom-nav Search tab skipped it only because the effect early-returned on an empty query. The bar now renders on `/search` too (`showHeaderSearch`) and is the sole md+ input — the page's own input is `md:hidden` — and on that route it republishes the query into the URL with `replaceState`, so a whole session of typing costs one history entry. The page *consumes* that URL once per distinct value (`seedFromSearchUrl` against a non-reactive `applied` marker) instead of continuously reconciling it, and the scope chips publish through the same URL so the bar and the chips cannot disagree. Landing on `/search` with a seeded query focuses the bar and puts the caret at the end, because the box the user was typing in belonged to the unmounted route | UI | UR-049, UR-054 | Done |
| DR-142 | An episode has exactly **one** surface, and it is complete. Two divergent renderings existed: `EpisodeFocusView` (reached from Continue Watching, the series episode list, the TV landing page and Downloads — i.e. every real entry point) offered only Play and Favourite, while the bare `/library/<episodeId>` page nobody routed to carried the download button, the series/season breadcrumbs and the cast section. Opening an episode the normal way therefore silently lost the ability to download it. The Focus View is now the single surface and carries the full §5B.2 composition — hero action row `Play / Download / Favourite`, series name and `SxEy` badge as links back to the series and to that season's anchor, then genres → cast → similar shows *below* the episode strip, never above it (DR-062). `/library/<episodeId>` redirects into it (`episodeRedirectTarget`, the same rule seasons follow under DR-103), and an episode with no `seriesId` renders the same component series-less rather than falling back to a second, lesser page. The focused episode is fetched in full rather than reused from the season fan-out, because that is a *list* query and carries neither cast nor genres — the sections would have rendered empty. The strip hides itself when the episode has no siblings, a card that only shows the episode you are already on being noise | UI | UR-048, UR-058 | Done | | DR-142 | An episode has exactly **one** surface, and it is complete. Two divergent renderings existed: `EpisodeFocusView` (reached from Continue Watching, the series episode list, the TV landing page and Downloads — i.e. every real entry point) offered only Play and Favourite, while the bare `/library/<episodeId>` page nobody routed to carried the download button, the series/season breadcrumbs and the cast section. Opening an episode the normal way therefore silently lost the ability to download it. The Focus View is now the single surface and carries the full §5B.2 composition — hero action row `Play / Download / Favourite`, series name and `SxEy` badge as links back to the series and to that season's anchor, then genres → cast → similar shows *below* the episode strip, never above it (DR-062). `/library/<episodeId>` redirects into it (`episodeRedirectTarget`, the same rule seasons follow under DR-103), and an episode with no `seriesId` renders the same component series-less rather than falling back to a second, lesser page. The focused episode is fetched in full rather than reused from the season fan-out, because that is a *list* query and carries neither cast nor genres — the sections would have rendered empty. The strip hides itself when the episode has no siblings, a card that only shows the episode you are already on being noise | UI | UR-048, UR-058 | Done |
@@ -387,6 +387,7 @@ Internal architecture, components, and application logic.
| DR-137 | Local media is served to the player over a loopback HTTP server, not the asset protocol. Tauri's `asset` protocol answers a request carrying no `Range` header by reading the whole file into memory, and only advertises `Accept-Ranges: bytes` from *inside* its range branch — so the first request never learns ranges exist and a multi-gigabyte body is attempted instead. Chromium abandoned it with `PIPELINE_ERROR_READ` after ~31s, which reached the user as "downloaded video does not play offline". Real HTTP on `127.0.0.1` is chosen over a custom URI scheme deliberately: range support becomes a property of the transport rather than depending on whether a platform's webview forwards `Range` to a custom scheme. No response ever exceeds a 4 MiB chunk and bodies stream from the file handle, so memory is bounded regardless of file size. Because **loopback is shared between apps on Android**, the server binds `127.0.0.1` only and every URL carries a random per-session token; paths are additionally confined to the app data directory, so a leaked URL cannot read outside it. This is stage 1 of making the server the single media origin — remote passthrough and download-while-watching are deliberately out of scope here | Playback | UR-071 | Done | | DR-137 | Local media is served to the player over a loopback HTTP server, not the asset protocol. Tauri's `asset` protocol answers a request carrying no `Range` header by reading the whole file into memory, and only advertises `Accept-Ranges: bytes` from *inside* its range branch — so the first request never learns ranges exist and a multi-gigabyte body is attempted instead. Chromium abandoned it with `PIPELINE_ERROR_READ` after ~31s, which reached the user as "downloaded video does not play offline". Real HTTP on `127.0.0.1` is chosen over a custom URI scheme deliberately: range support becomes a property of the transport rather than depending on whether a platform's webview forwards `Range` to a custom scheme. No response ever exceeds a 4 MiB chunk and bodies stream from the file handle, so memory is bounded regardless of file size. Because **loopback is shared between apps on Android**, the server binds `127.0.0.1` only and every URL carries a random per-session token; paths are additionally confined to the app data directory, so a leaked URL cannot read outside it. This is stage 1 of making the server the single media origin — remote passthrough and download-while-watching are deliberately out of scope here | Playback | UR-071 | Done |
| DR-138 | Loopback is exempted from Android's cleartext ban, and nothing else is. Release builds set `usesCleartextTraffic="false"`, so the webview's request to the local media server (DR-137) was rejected by network security policy before any I/O — `<video>` failed in the same millisecond as `loadstart`, with `NETWORK_NO_SOURCE` and no server-side log at all, which is why it looked identical to a missing file. A `network-security-config` resource permits cleartext for `127.0.0.1` only and keeps `base-config cleartextTrafficPermitted="false"`, so a remote server must still be HTTPS; this is deliberately not a blanket opt-in. The manifest attribute is ignored once the config is present, so the config is the single authority. `sync-android-sources.sh` also had to learn to copy `res/xml`, which it skipped — the manifest references the resource, so a missed copy fails the resource link rather than degrading quietly | Security | UR-071 | Done | | DR-138 | Loopback is exempted from Android's cleartext ban, and nothing else is. Release builds set `usesCleartextTraffic="false"`, so the webview's request to the local media server (DR-137) was rejected by network security policy before any I/O — `<video>` failed in the same millisecond as `loadstart`, with `NETWORK_NO_SOURCE` and no server-side log at all, which is why it looked identical to a missing file. A `network-security-config` resource permits cleartext for `127.0.0.1` only and keeps `base-config cleartextTrafficPermitted="false"`, so a remote server must still be HTTPS; this is deliberately not a blanket opt-in. The manifest attribute is ignored once the config is present, so the config is the single authority. `sync-android-sources.sh` also had to learn to copy `res/xml`, which it skipped — the manifest references the resource, so a missed copy fails the resource link rather than degrading quietly | Security | UR-071 | Done |
| DR-093 | Traceability coverage gate derives its requirement denominators from `requirements.md` at run time rather than hardcoded literals: `countDefinedRequirements` counts an ID only where it leads a markdown table row (ignoring the "Traces To" column and prose) and deduplicates IDs listed both in the definition tables and in the §3 traceability matrix; `computeCoverage` reports the *intersection* of traced and defined IDs so an ID traced in code but absent from `requirements.md` is surfaced as `orphaned` instead of inflating the ratio past 100%. UT/IT test identifiers are excluded as a separate taxonomy. CI and `bun run traces:coverage` share this computation and fail on both a sub-threshold and an impossible >100% result | Tooling | - | Done | | DR-093 | Traceability coverage gate derives its requirement denominators from `requirements.md` at run time rather than hardcoded literals: `countDefinedRequirements` counts an ID only where it leads a markdown table row (ignoring the "Traces To" column and prose) and deduplicates IDs listed both in the definition tables and in the §3 traceability matrix; `computeCoverage` reports the *intersection* of traced and defined IDs so an ID traced in code but absent from `requirements.md` is surfaced as `orphaned` instead of inflating the ratio past 100%. UT/IT test identifiers are excluded as a separate taxonomy. CI and `bun run traces:coverage` share this computation and fail on both a sub-threshold and an impossible >100% result | Tooling | - | Done |
| DR-198 | The webview runs under a real Content-Security-Policy, and the asset protocol is scoped to the one directory it still serves. `csp` was `null`, which disables CSP entirely: any script that reached the web layer — through a future `{@html}`, a dependency, or a devtools paste — would have inherited the whole IPC surface, and with it the user's session. `script-src 'self'` (Tauri injects a nonce for SvelteKit's inline bootstrap script at build time, so no `'unsafe-inline'` is needed) plus `object-src`/`frame-src 'none'` and `base-uri 'self'` is the part that is genuinely restrictive. `img-src`/`media-src`/`connect-src` cannot be: the Jellyfin origin is typed in by the user at run time and is commonly plain `http` on a LAN, so they allow `http:`/`https:` — a wide grant for *data*, but one that still bars `file:`, `filesystem:` and scripting schemes, and leaves `script-src` untouched. `style-src` keeps `'unsafe-inline'` because Svelte compiles `style="…"` attributes (including `app.html`'s `display: contents` wrapper) into markup; this is safe only while no `<style>` element survives into `index.html`, since a nonce there would make Tauri's injection outrank — and therefore void — `'unsafe-inline'`. `worker-src blob:` and `media-src blob:` are hls.js: it demuxes in a worker built from a blob and attaches MSE through `URL.createObjectURL`. `asset:` and `http://asset.localhost` are the same protocol under the two naming schemes `convertFileSrc` emits (custom scheme on Linux/macOS, `http` host on Windows/Android); `ipc:`/`http://ipc.localhost` is the invoke transport, which would otherwise be blocked by `connect-src`. A run-time CSP naming the server origin exactly was rejected: Tauri computes the header from immutable config when it serves the HTML, so it would mean rebuilding config and reloading the webview on every server change, for a policy the user can already point anywhere. The asset-protocol scope narrows from `$APPDATA/**` to `$APPDATA/thumbnails/**` — since DR-137 moved downloaded media to the loopback server, `imageCache` is the only `convertFileSrc` caller left, so the database and the encrypted-token fallback file no longer sit inside the grant | Security | UR-012, UR-071 | Done |
--- ---
@@ -407,7 +408,7 @@ Internal architecture, components, and application logic.
| UR-009 | IR-009, IR-010, IR-011 | - | | UR-009 | IR-009, IR-010, IR-011 | - |
| UR-010 | IR-012, IR-021 | DR-037, DR-059 | | UR-010 | IR-012, IR-021 | DR-037, DR-059 |
| UR-011 | IR-013 | DR-003, DR-015, DR-018 | | UR-011 | IR-013 | DR-003, DR-015, DR-018 |
| UR-012 | IR-009, IR-014 | - | | UR-012 | IR-009, IR-014 | DR-198 |
| UR-013 | IR-013 | DR-017 | | UR-013 | IR-013 | DR-017 |
| UR-014 | IR-010 | DR-014, DR-019 | | UR-014 | IR-010 | DR-014, DR-019 |
| UR-015 | - | DR-005, DR-020 | | UR-015 | - | DR-005, DR-020 |
@@ -465,7 +466,7 @@ Internal architecture, components, and application logic.
| UR-068 | - | DR-119 | | UR-068 | - | DR-119 |
| UR-069 | - | DR-113, DR-114, DR-120 | | UR-069 | - | DR-113, DR-114, DR-120 |
| UR-070 | - | DR-121, DR-122 | | UR-070 | - | DR-121, DR-122 |
| UR-071 | IR-032 | DR-123, DR-124, DR-125, DR-126, DR-127, DR-128, DR-133, DR-134, DR-135, DR-136, DR-137, DR-138, DR-170, DR-171, DR-180 | | UR-071 | IR-032 | DR-123, DR-124, DR-125, DR-126, DR-127, DR-128, DR-133, DR-134, DR-135, DR-136, DR-137, DR-138, DR-170, DR-171, DR-180, DR-198 |
| UR-072 | - | DR-156 | | UR-072 | - | DR-156 |
| UR-073 | - | DR-158 | | UR-073 | - | DR-158 |
| UR-074 | - | DR-162, DR-177, DR-181 | | UR-074 | - | DR-162, DR-177, DR-181 |
@@ -665,6 +666,7 @@ Internal architecture, components, and application logic.
| UT-190 | `build_next_up_endpoint` sends `EnableResumable=false` with the user and limit, and no `SeriesId` filter when none was requested | DR-197, JA-036 | Done | | UT-190 | `build_next_up_endpoint` sends `EnableResumable=false` with the user and limit, and no `SeriesId` filter when none was requested | DR-197, JA-036 | Done |
| UT-191 | A per-series next-up query keeps `SeriesId` and the resumable exclusion, and defaults the limit | DR-197 | Done | | UT-191 | A per-series next-up query keeps `SeriesId` and the resumable exclusion, and defaults the limit | DR-197 | Done |
| UT-192 | `filterInProgressNextUpItems` drops an episode present in the resume list, keeps the genuinely unstarted next episode, leaves the rest of the row intact, and is a no-op when nothing is in progress | DR-197 | Done | | UT-192 | `filterInProgressNextUpItems` drops an episode present in the resume list, keeps the genuinely unstarted next episode, leaves the rest of the row intact, and is a no-op when nothing is in progress | DR-197 | Done |
| UT-193 | The shipped Tauri security config stays restrictive: `csp` is set, `script-src` carries no `'unsafe-inline'`/`'unsafe-eval'`/wildcard, `object-src`/`frame-src` are `'none'`, the directives playback needs (asset scheme, loopback, `blob:`, `ipc:`) are present, and the asset-protocol scope covers only the thumbnail cache — never the storage root that holds the database | DR-198 | Done |
### Integration Tests ### Integration Tests
+4135 -4062
View File
File diff suppressed because it is too large Load Diff
+6 -4
View File
@@ -264,10 +264,12 @@ describe("live requirements.md", () => {
expect(defined.UR).toBe(75); expect(defined.UR).toBe(75);
expect(defined.IR).toBe(32); expect(defined.IR).toBe(32);
// 188 since DR-189 was given the definition its TRACES comments in // 189 = 187 + two independently-added requirements that landed together:
// VideoPlayer.svelte / controlsVisibility.ts had always referenced. // DR-189 (the definition its TRACES comments in VideoPlayer.svelte /
expect(defined.DR).toBe(188); // controlsVisibility.ts had always referenced) and DR-198 (asset-protocol
// scope). Each branch bumped 187 -> 188 for its own; merged, they sum.
expect(defined.DR).toBe(189);
expect(defined.JA).toBe(36); expect(defined.JA).toBe(36);
expect(defined.total).toBe(331); expect(defined.total).toBe(332);
}); });
}); });
+92
View File
@@ -0,0 +1,92 @@
/**
* Guards the shipped webview security configuration.
*
* `csp` was `null` and the asset protocol was scoped to the whole storage root,
* which is the directory holding the SQLite database and the encrypted-token
* fallback file. Both are one-character regressions away and neither is visible
* in any behavioural test, so they are asserted here instead: the restrictive
* half of the policy must stay restrictive, and the permissive half must keep
* the schemes playback actually needs.
*
* TRACES: UR-012, UR-071 | DR-198 | UT-193
*/
import { describe, it, expect } from "vitest";
import { readFileSync } from "fs";
import { resolve } from "path";
const config = JSON.parse(
readFileSync(resolve(__dirname, "../src-tauri/tauri.conf.json"), "utf-8")
);
const security = config.app.security;
/** Split a CSP string into `directive -> sources`. */
function directives(csp: string): Record<string, string[]> {
const map: Record<string, string[]> = {};
for (const part of csp.split(";")) {
const [name, ...sources] = part.trim().split(/\s+/);
if (name) map[name] = sources;
}
return map;
}
describe("tauri.conf.json CSP", () => {
it("is set at all — a null CSP hands any injected script the full IPC surface", () => {
expect(typeof security.csp).toBe("string");
expect(security.csp.length).toBeGreaterThan(0);
});
const csp = directives(security.csp as string);
it("locks down script execution", () => {
// Tauri injects a nonce for SvelteKit's inline bootstrap script at build
// time, so 'self' alone is enough and inline/eval must never be re-added.
expect(csp["script-src"]).toEqual(["'self'"]);
expect(csp["object-src"]).toEqual(["'none'"]);
expect(csp["frame-src"]).toEqual(["'none'"]);
expect(csp["base-uri"]).toEqual(["'self'"]);
expect(csp["default-src"]).toEqual(["'self'"]);
});
it("keeps the schemes playback and thumbnails depend on", () => {
// The asset protocol under both names convertFileSrc emits.
expect(csp["img-src"]).toContain("asset:");
expect(csp["img-src"]).toContain("http://asset.localhost");
expect(csp["media-src"]).toContain("asset:");
// hls.js: MSE object URLs, and its demuxer worker built from a blob.
expect(csp["media-src"]).toContain("blob:");
expect(csp["worker-src"]).toContain("blob:");
// The token-guarded loopback media server (DR-137).
expect(csp["media-src"]).toContain("http://127.0.0.1:*");
// Tauri's invoke transport.
expect(csp["connect-src"]).toContain("ipc:");
expect(csp["connect-src"]).toContain("http://ipc.localhost");
// The user's Jellyfin server: an arbitrary run-time origin, http on a LAN.
for (const directive of ["img-src", "media-src", "connect-src"]) {
expect(csp[directive]).toContain("http:");
expect(csp[directive]).toContain("https:");
}
});
it("never widens a data directive into script execution", () => {
for (const [name, sources] of Object.entries(csp)) {
if (name === "script-src" || name === "worker-src") {
expect(sources).not.toContain("'unsafe-eval'");
expect(sources).not.toContain("'unsafe-inline'");
}
// A bare `*` would re-admit every scheme, including file:.
expect(sources).not.toContain("*");
}
});
});
describe("tauri.conf.json asset protocol scope", () => {
const scope: string[] = security.assetProtocol.scope;
it("covers only the thumbnail cache, not the storage root", () => {
expect(scope).toEqual(["$APPDATA/thumbnails/**"]);
// The database and the encrypted-token fallback live directly in $APPDATA.
expect(scope).not.toContain("$APPDATA/**");
});
});
+9 -5
View File
@@ -23,11 +23,15 @@ debug = "line-tables-only"
tauri-build = { version = "2", features = [] } tauri-build = { version = "2", features = [] }
[dependencies] [dependencies]
# protocol-asset serves downloaded media and cached thumbnails to the webview # protocol-asset serves cached thumbnails to the webview (asset://localhost on
# over http://asset.localhost; without it convertFileSrc yields a URL nothing # Linux/macOS, http://asset.localhost on Windows/Android); without it
# answers. Paired with app.security.assetProtocol in tauri.conf.json, which # convertFileSrc yields a URL nothing answers. Paired with
# scopes it to $APPDATA/**. # app.security.assetProtocol in tauri.conf.json, which scopes it to
# TRACES: UR-071 | DR-134 # $APPDATA/thumbnails/** — the one directory still read through this protocol.
# Downloaded media went the same way until DR-137 moved it to the loopback media
# server, so the database, the encrypted-token fallback file and downloads/ are
# all outside the grant now.
# TRACES: UR-012, UR-071 | DR-134, DR-137, DR-198
tauri = { version = "2", features = ["protocol-asset"] } tauri = { version = "2", features = ["protocol-asset"] }
tauri-plugin-opener = "2" tauri-plugin-opener = "2"
tauri-plugin-os = "2" tauri-plugin-os = "2"
+15 -8
View File
@@ -1029,16 +1029,23 @@ fn set_env_if_unset(key: &str, value: &str) {
} }
} }
/// Downloaded media and cached thumbnails are handed to the webview as /// Cached thumbnails are handed to the webview as asset-protocol URLs by
/// `http://asset.localhost/…` URLs by `convertFileSrc`. Tauri only answers that /// `convertFileSrc` (`asset://localhost/…` on Linux/macOS,
/// `http://asset.localhost/…` on Windows/Android). Tauri only answers that
/// origin when the `protocol-asset` cargo feature is compiled in *and* /// origin when the `protocol-asset` cargo feature is compiled in *and*
/// `app.security.assetProtocol.enable` is set in `tauri.conf.json`, which also /// `app.security.assetProtocol.enable` is set in `tauri.conf.json`. Both are
/// scopes it to `$APPDATA/**` — the storage root holding the database, /// required together: with either missing the URL resolves to nothing and the
/// `downloads/` and the thumbnail cache. Both are required together: with either /// webview reports `NETWORK_NO_SOURCE`, which is how offline video came to fail
/// missing the URL resolves to nothing and the webview reports /// silently.
/// `NETWORK_NO_SOURCE`, which is how offline video came to fail silently.
/// ///
/// TRACES: UR-071 | DR-134 /// The scope is `$APPDATA/thumbnails/**`, not the storage root: downloaded media
/// moved to the loopback media server in DR-137, so `imageCache` is the only
/// remaining `convertFileSrc` caller and the database and the encrypted-token
/// fallback file — which share that root — never need to be readable by the
/// webview. Widen it only if something other than thumbnails starts resolving
/// through `convertFileSrc` again.
///
/// TRACES: UR-012, UR-071 | DR-134, DR-137, DR-198
#[cfg_attr(mobile, tauri::mobile_entry_point)] #[cfg_attr(mobile, tauri::mobile_entry_point)]
pub fn run() { pub fn run() {
// Initialize logger // Initialize logger
+3 -2
View File
@@ -18,10 +18,11 @@
} }
], ],
"security": { "security": {
"csp": null, "csp": "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; font-src 'self' data:; img-src 'self' data: blob: asset: http://asset.localhost http: https:; media-src 'self' blob: asset: http://asset.localhost http://127.0.0.1:* http: https:; connect-src 'self' ipc: http://ipc.localhost http: https:; worker-src 'self' blob:; object-src 'none'; frame-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'",
"devCsp": "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; font-src 'self' data:; img-src 'self' data: blob: asset: http://asset.localhost http: https:; media-src 'self' blob: asset: http://asset.localhost http://127.0.0.1:* http: https:; connect-src 'self' ipc: http://ipc.localhost http: https: ws: wss:; worker-src 'self' blob:; object-src 'none'; frame-src 'none'; base-uri 'self'; form-action 'self'",
"assetProtocol": { "assetProtocol": {
"enable": true, "enable": true,
"scope": ["$APPDATA/**"] "scope": ["$APPDATA/thumbnails/**"]
} }
} }
}, },
+4 -1
View File
@@ -41,7 +41,10 @@ export async function getCachedImageUrl(
const cachedPath = await commands.thumbnailGetCached(itemId, imageType, tag); const cachedPath = await commands.thumbnailGetCached(itemId, imageType, tag);
if (cachedPath) { if (cachedPath) {
// Convert file path to asset URL for Tauri // Convert file path to asset URL for Tauri. This is the only remaining
// convertFileSrc caller, which is why the asset-protocol scope is narrowed
// to $APPDATA/thumbnails/** — a path outside it resolves to nothing.
// TRACES: UR-012 | DR-134, DR-198
return convertFileSrc(cachedPath); return convertFileSrc(cachedPath);
} }
} catch (e) { } catch (e) {