chore(bindings): regenerate specta output for the new TRACES doc comments
tauri-specta propagates Rust doc comments into bindings.ts as JSDoc, so adding TRACES comments to command functions changes generated output. Regeneration happens at build time, so this was left dirty by the branch that added them. Doc-comment-only: no signature or exported-symbol changes. Also records the audit corrections made during device verification (B1 mechanism, B7 re-framing, B8, D3 magnitude).
This commit is contained in:
+55
-20
@@ -12,8 +12,8 @@ nothing here is inferred from documentation alone.
|
||||
| Severity | Count |
|
||||
|----------|-------|
|
||||
| High | 5 |
|
||||
| Medium | 10 |
|
||||
| Low | 5 |
|
||||
| Medium | 9 |
|
||||
| Low | 6 |
|
||||
| Tests passing | 1,719 |
|
||||
| Untraced requirements | 86 |
|
||||
| Traceability coverage | 86% (285/330) |
|
||||
@@ -26,6 +26,11 @@ nothing here is inferred from documentation alone.
|
||||
> 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.
|
||||
> - **D3 Medium → Low.** The "820 unwraps" figure was a measurement error; the
|
||||
> real number is 19, and none are in command handlers.
|
||||
> - **B1's stated mechanism was wrong** even though its conclusion held. FGS
|
||||
> notifications are *not* exempt from `POST_NOTIFICATIONS`; media-session
|
||||
> notifications are. See B1 — the distinction changes what the fix should be.
|
||||
>
|
||||
> Original ranking was 6 High / 8 Medium / 5 Low.
|
||||
|
||||
@@ -145,14 +150,25 @@ ServiceRecord{... com.dtourolle.jellytau/.player.JellyTauPlaybackService}
|
||||
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.**
|
||||
**`UR-006` is not at risk.** But the *reason* is not the one this audit first
|
||||
gave, and the correction is load-bearing rather than pedantic.
|
||||
|
||||
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 first explanation here was "foreground-service notifications are exempt." That
|
||||
is wrong. Android's own wording is that the permission covers "non-exempt
|
||||
(**including Foreground Services (FGS)**) notifications", and that users who deny
|
||||
it see FGS notices "in the Task Manager but [not] in the notification drawer" — an
|
||||
FGS notification is explicitly *not* exempt. What is exempt is **media-session**
|
||||
notifications. The platform predicate is `Notification.isMediaNotification()`,
|
||||
requiring `MediaStyle`/`DecoratedMediaCustomViewStyle` **and** a non-null
|
||||
`EXTRA_MEDIA_SESSION`; it is byte-identical across API 33–36, and
|
||||
`NotificationManagerService` has no FGS clause in either enforcement site.
|
||||
|
||||
Why the difference matters: under the FGS theory, anything the service posts is
|
||||
safe, and the code needs no care. Under the correct one, the exemption is earned
|
||||
per-notification by the token — so losing the token loses not just the shade entry
|
||||
but the lockscreen controls entirely, since SystemUI's media carousel
|
||||
(`MediaDataProcessor.onNotificationAdded`) gates on the *same* predicate. A
|
||||
token-less notification never even reaches the notification listener.
|
||||
|
||||
**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
|
||||
@@ -401,20 +417,39 @@ subscribers were never cleared, so every module instance discarded by
|
||||
|
||||
**Location:** `src/lib/services/offlineCatalog.test.ts:58`
|
||||
|
||||
### D3 · Medium · 820 `unwrap()`/`expect()` calls sit outside test code
|
||||
### D3 · Low · ~~820~~ **19** production `unwrap()`/`expect()` calls
|
||||
|
||||
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.
|
||||
*Downgraded from Medium. This audit substantially overstated the problem, and the
|
||||
correction is worth recording because the measurement error is instructive.*
|
||||
|
||||
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.
|
||||
The original 820 figure came from grepping for `unwrap()`/`expect()` and filtering
|
||||
lines containing "test". That does not exclude test *modules* — it only excludes
|
||||
lines with "test" in them. Scripting the actual `#[cfg(test)]` boundaries gives
|
||||
**19 real production sites**, not 820. `player/mod.rs`'s 154 hits, for instance,
|
||||
are *all* past its `#[cfg(test)]` at line 2183, as are the bulk of
|
||||
`repository/offline.rs`, `storage/mod.rs` and `commands/download/mod.rs`.
|
||||
|
||||
**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.
|
||||
**More importantly: zero bare unwraps exist in any `#[tauri::command]` handler.**
|
||||
The specific risk this finding was built around — a panic inside a command killing
|
||||
the task and stranding shared player state — is already absent.
|
||||
|
||||
The same correction applies to the lock half: all 33 raw `.lock().unwrap()` hits
|
||||
were in test modules (three weren't even code, but prose in `utils/lock.rs`'s doc
|
||||
comment). Production was already fully on `lock_safe()`/`read_safe()`/
|
||||
`write_safe()`. Converting them was consistency work, not a bug fix.
|
||||
|
||||
**What is genuinely worth doing** is a three-site cluster, all the same pattern —
|
||||
`Runtime::new().unwrap()` in threads owning playback-critical state:
|
||||
|
||||
| | Site | Consequence of a panic |
|
||||
|---|------|------------------------|
|
||||
| 1 | `session_poller/mod.rs:102` | Poller thread dies silently; it drives remote-mode state *and* offline→online recovery, so the app strands offline with nothing surfaced |
|
||||
| 2 | `player/mpv_backend.rs:424` | Position reporting stops mid-playback; the scrubber freezes while audio keeps going |
|
||||
| 3 | `player/android/mod.rs:761` | Same pattern across a JNI boundary; progress reporting dies and no resume points are written |
|
||||
|
||||
**Fix:** One shared helper returning `Option<Runtime>` and logging on failure
|
||||
retires all three. The remaining 16 are startup `expect()`s and two provably
|
||||
infallible calls.
|
||||
|
||||
### D4 · Low · Five files carry a disproportionate share of the complexity
|
||||
|
||||
|
||||
Reference in New Issue
Block a user