From f49e6e4648def9bd23fcfbbffbff51c897d6ed44 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Thu, 30 Jul 2026 10:30:55 +0200 Subject: [PATCH] fix(boundary): detect item-type arrays anywhere in src/ (DR-094) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check:boundary passed on the very leak it was written for. The pattern was anchored to `includeItemTypes:` at the query site, so searchScope.ts assigning the same array to a named const and dereferencing it one indirection away was invisible — through every green CI run. The check now matches an array literal naming two or more Jellyfin item types anywhere in src/, catching a const, a Record value, a function return, and an inline query alike. Deliberate limits kept: two adjacent literals required (single-type presentation stays legal), string literals required (item.type === "Audio" is display logic), explicit type list (so ["High","Low"] produces no noise). Verified all five cases: reintroducing the original SCOPE_ITEM_TYPES fails; a new const ["Movie","Series"] fails; the same array in a .test.ts passes; itemType: "Movie" / item.type === / ["High","Low"] pass; a 5th allowlist entry fails on the new cap. Allowlist 1→3 entries, capped at 4 so the next exception forces a conversation rather than a one-line append: - GenericMediaListPage: grid styling over a self-declared itemType — presentation, changes only with a UI redesign. - DownloadedBrowse: borderline, leans domain (the container set grows when Jellyfin adds a container type). Allowlisted with a TODO for a backend MediaItem.isContainer flag. The header now names what the check still cannot see — run-time-built sets, types split across variables, switch/|| taxonomy — and CLAUDE.md states that a green check:boundary is not proof. That matters given this check passed on its own founding violation for months. Also: both gates wired into test-all.sh, which called `bun run test` without --run and would have hung in watch mode. Corrected the Dockerfile comment describing the Windows toolchain as mingw/GNU — it is MSVC via cargo-xwin (GNU cannot bundle NSIS from Linux). --- CLAUDE.md | 10 +++- Dockerfile | 10 +++- docs/requirements.md | 4 +- docs/traceability.md | 27 +++++++-- scripts/check-frontend-boundary.sh | 93 ++++++++++++++++++++++++------ scripts/test-all.sh | 9 ++- 6 files changed, 122 insertions(+), 31 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e92bb7b0..482dabd9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -183,8 +183,14 @@ and [docs/build-release.md](docs/build-release.md). backend expand it. Single-type presentation (`itemType: "Movie"`, "this page shows albums") is fine; a *category → set of types* mapping in `src/` is a leak. `bun run check:boundary` is the tripwire; the real gate is the spec's layer - assignment. See [scoped-search-boundary.md](docs/specs/scoped-search-boundary.md) - for the incident this rule came from. + assignment. The canonical example lives in Rust: + `SearchScope::item_types()` in `repository/types.rs` expands an opaque scope the + frontend sends. See [scoped-search-boundary.md](docs/specs/scoped-search-boundary.md) + for the incident this rule came from — note the tripwire missed that leak for + months because the mapping was assigned to a named const rather than written + inline at the query, so **a green `check:boundary` is not proof**; it flags + item-type array literals only, not run-time-built sets or `switch`/`||` + taxonomy. ## Writing specs diff --git a/Dockerfile b/Dockerfile index 208a561b..24bc944e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -117,9 +117,13 @@ RUN cd src-tauri && cargo fetch && cd .. && \ # Desktop packaging stages build FROM the unified registry builder image (see the # BUILDER_IMAGE ARG at the top), which already carries every packaging tool -# (rpm/file for Linux, mingw-w64 + nsis + the x86_64-pc-windows-gnu rust target -# for Windows). ONE source of dependency truth, shared with CI — no per-stage -# apt/rustup here. +# (rpm/file for Linux, cargo-xwin + nsis + the x86_64-pc-windows-msvc rust +# target for Windows). ONE source of dependency truth, shared with CI — no +# per-stage apt/rustup here. +# +# NOTE: Windows uses the MSVC target via cargo-xwin, NOT mingw/GNU — the GNU +# toolchain cannot bundle an NSIS installer from Linux. See +# scripts/build-windows-cross.sh. # Linux desktop packaging environment (deb + rpm; Arch is Dockerfile.arch). # Thin layer over the builder — the actual build runs at container-run time on diff --git a/docs/requirements.md b/docs/requirements.md index 8055f6ff..04f9a0a7 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -220,7 +220,7 @@ Internal architecture, components, and application logic. | DR-060 | Multi-server store and active-account selection: save/get/delete server, save/get user, set/get active user (per-server), active-session resolution | Storage | UR-047 | Partial (store done; server-switcher UI pending) | | DR-061 | Episode Focus View: episode hero followed *immediately* by the "More Episodes" strip — a forward-biased window (~3 before / ~6 after) around the current episode, spanning season boundaries in series order, with the current episode present and badged, per-card resume progress and watched state, and click-to-swap focus (no playback) | UI | UR-048 | Done | | DR-062 | Detail-page section ordering: continuation content precedes discovery content — Episode Focus View renders hero → episode strip → cast → similar; Series renders hero → seasons/episodes → cast → similar | UI | UR-048 | Done | -| DR-063 | Search scope resolver mapping the originating route to an `includeItemTypes` set (All / Music / Movies / TV), defaulting to All for Home, `/library`, and the search tab | UI | UR-049 | Implemented | +| DR-063 | Search scope taxonomy owned by Rust: `SearchScope` (All / Music / Movies / TV) crosses IPC as an opaque enum and `SearchScope::item_types()` expands it to Jellyfin item types, resolved once in `repository_search` before the cache and server paths diverge so online and offline filter identically; `All` expands to *no* filter rather than the union of the other scopes (which would drop People and folders). The frontend maps the originating route to a scope (`resolveSearchScope`, presentation) and never names an item type for search | Backend | UR-049 | Implemented | | DR-064 | Scope chip row rendered under the search bar on both the search page and the in-library header search: preselected from context, horizontally scrollable, re-runs the search preserving the query on change | UI | UR-049 | Implemented | | DR-065 | Thread `SearchOptions.includeItemTypes` through `library.search()` so the global/header search honours scope (backend online + offline paths already support it) | UI | UR-049 | Implemented | | DR-066 | Persisted search result group order with a drag-and-drop settings list, keyboard-accessible reordering, a shipped default (see DR-091 for the current group set and order), and empty-group omission | Settings | UR-050 | Implemented | @@ -247,6 +247,8 @@ Internal architecture, components, and application logic. | DR-090 | Relevance ranking in Rust (`domain/search_rank.rs`): results sort by match position (prefix → word-start → mid-word substring → no name match) then by media kind (containers before their contents), stably so the backend's own relevance breaks ties. Applied in `repository_search` to both the instant cache result and the merged cache+server union, so the list does not reshuffle when server results land | Backend | UR-060 | Done | | DR-091 | Search result groups split TV into separate Shows and Episodes groups and add a People group (default order: Shows → Episodes → Movies → Songs → Albums → Artists → People); a stored `tvShows` order from before the split expands in place to shows+episodes so an upgrading user keeps their arrangement | UI | UR-060 | Done | | DR-092 | Video tap gestures resolve in `tapGestures.ts` (pure, unit-tested) rather than inline in `VideoPlayer.svelte`: `registerTap` returns `pending` for a first tap — the component defers `togglePlayPause` behind a `DOUBLE_TAP_WINDOW_MS` (300 ms) timer that a second tap cancels — or `seek` (+30 s right / −10 s left) for a second tap inside the window; a consumed second tap resets the state so a third tap starts fresh, and a swipe cancels the pending tap. The compatibility `click` the browser synthesizes after a touch tap is filtered in `handleVideoClick` so it cannot bypass the deferral. `resolveSeekTarget` converts the delta to the absolute position the facade requires, clamped to `[0, duration]` and chained off a still-in-flight `pendingSeekTarget` so back-to-back skips accumulate instead of all resolving against a not-yet-updated position | UI | UR-061 | Done | +| DR-094 | Frontend boundary tripwire (`scripts/check-frontend-boundary.sh`) detects Jellyfin item-type array literals **anywhere** in `src/` rather than only inline at an `includeItemTypes:` query site, so a category→type mapping cannot evade the check by being assigned to a named const (the evasion that let the `scoped-search` leak pass CI); requires two adjacent type literals so single-type presentation and `item.type ===` inspection stay legal, and caps the allowlist to force taxonomy into Rust instead of accumulating exceptions | 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 | --- diff --git a/docs/traceability.md b/docs/traceability.md index 45dea836..abacb8b3 100644 --- a/docs/traceability.md +++ b/docs/traceability.md @@ -1,15 +1,15 @@ # Code Traceability Matrix -**Generated:** 7/28/2026, 10:39:16 PM +**Generated:** 7/28/2026, 11:52:49 PM ## Summary -- **Total Files Scanned:** 299 -- **Total TRACES Found:** 318 +- **Total Files Scanned:** 302 +- **Total TRACES Found:** 322 - **Requirements Covered:** - User Requirements (UR): 58 - Integration Requirements (IR): 15 - - Development Requirements (DR): 84 + - Development Requirements (DR): 85 - Jellyfin API Requirements (JA): 24 ## Requirements by Type @@ -26,7 +26,7 @@ IR-003, IR-004, IR-009, IR-010, IR-011, IR-012, IR-013, IR-014, IR-015, IR-020, ### Development Requirements (DR) ``` -DR-001, DR-002, DR-003, DR-004, DR-005, DR-006, DR-007, DR-009, DR-010, DR-011, DR-012, DR-013, DR-014, DR-015, DR-016, DR-017, DR-018, DR-020, DR-021, DR-022, DR-023, DR-024, DR-025, DR-026, DR-027, DR-028, DR-029, DR-030, DR-032, DR-033, DR-034, DR-035, DR-036, DR-037, DR-038, DR-039, DR-040, DR-041, DR-043, DR-044, DR-045, DR-047, DR-048, DR-049, DR-050, DR-051, DR-052, DR-053, DR-054, DR-055, DR-056, DR-057, DR-058, DR-059, DR-060, DR-061, DR-062, DR-063, DR-064, DR-065, DR-066, DR-067, DR-068, DR-069, DR-070, DR-074, DR-075, DR-076, DR-077, DR-078, DR-079, DR-080, DR-081, DR-082, DR-083, DR-084, DR-085, DR-086, DR-087, DR-088, DR-089, DR-090, DR-091, DR-092 +DR-001, DR-002, DR-003, DR-004, DR-005, DR-006, DR-007, DR-009, DR-010, DR-011, DR-012, DR-013, DR-014, DR-015, DR-016, DR-017, DR-018, DR-020, DR-021, DR-022, DR-023, DR-024, DR-025, DR-026, DR-027, DR-028, DR-029, DR-030, DR-032, DR-033, DR-034, DR-035, DR-036, DR-037, DR-038, DR-039, DR-040, DR-041, DR-043, DR-044, DR-045, DR-047, DR-048, DR-049, DR-050, DR-051, DR-052, DR-053, DR-054, DR-055, DR-056, DR-057, DR-058, DR-059, DR-060, DR-061, DR-062, DR-063, DR-064, DR-065, DR-066, DR-067, DR-068, DR-069, DR-070, DR-074, DR-075, DR-076, DR-077, DR-078, DR-079, DR-080, DR-081, DR-082, DR-083, DR-084, DR-085, DR-086, DR-087, DR-088, DR-089, DR-090, DR-091, DR-092, DR-093 ``` ### Jellyfin API Requirements (JA) @@ -1881,6 +1881,23 @@ JA-001, JA-002, JA-003, JA-004, JA-005, JA-007, JA-010, JA-011, JA-012, JA-016, - **Line:** 10 - **Context:** `Unknown` +### DR-093 + +**Locations:** 4 file(s) + +- **File:** [`scripts/extract-traces.ts`](scripts/extract-traces.ts#L216) + - **Line:** 216 + - **Context:** `Unknown` +- **File:** [`scripts/extract-traces.ts`](scripts/extract-traces.ts#L246) + - **Line:** 246 + - **Context:** `Unknown` +- **File:** [`scripts/extract-traces.ts`](scripts/extract-traces.ts#L278) + - **Line:** 278 + - **Context:** `Unknown` +- **File:** [`scripts/extract-traces.ts`](scripts/extract-traces.ts#L387) + - **Line:** 387 + - **Context:** `function generateJson(data: TracesData): string {` + ### JA-001 **Locations:** 1 file(s) diff --git a/scripts/check-frontend-boundary.sh b/scripts/check-frontend-boundary.sh index e24e1817..5eb597e3 100755 --- a/scripts/check-frontend-boundary.sh +++ b/scripts/check-frontend-boundary.sh @@ -1,6 +1,8 @@ #!/usr/bin/env bash # Boundary tripwire: flag domain-taxonomy leaks in the Svelte frontend. # +# Implements DR-094 (see docs/requirements.md). +# # The project rule (CLAUDE.md, docs/architecture/02-svelte-frontend.md) is that # the frontend is presentation-only and the Rust backend owns domain logic — # including Jellyfin's item-type *taxonomy* (what the category "Music" means as a @@ -9,18 +11,29 @@ # # ⚠️ This is a TRIPWIRE, NOT A PROOF. A grep cannot distinguish taxonomy-as-policy # (a leak) from taxonomy-as-display (legitimate: "is this a music card?"). It -# targets the one machine-detectable signature of the leak class — a *query* that -# names a multi-type category — and defers everything subtler to the human -# spec-review checklist (docs/specs/SPEC-REVIEW-CHECKLIST.md). A clean run here -# does not mean the boundary is respected; it means the crudest violation isn't -# present. +# targets the machine-detectable signature of the leak class and defers +# everything subtler to the human spec-review checklist +# (docs/specs/SPEC-REVIEW-CHECKLIST.md). A clean run here does not mean the +# boundary is respected; it means the crudest violation isn't present. # -# What it flags: an `includeItemTypes: [ ... , ... ]` array literal with two or -# more types — i.e. the frontend deciding that a *category* maps to a *set* of -# Jellyfin types, which is domain knowledge the backend should own. Single-type -# query arrays (`includeItemTypes: ["Movie"]`) are a page saying "I show movies" -# and are allowed. Type *inspection* (`item.type === "Audio"`) is display logic -# and is not matched. +# What it flags: an array literal naming two or more Jellyfin item types, +# ANYWHERE in src/ — i.e. the frontend deciding that a *category* maps to a *set* +# of Jellyfin types, which is domain knowledge the backend should own. +# Single-type arrays (`includeItemTypes: ["Movie"]`) are a page saying "I show +# movies" and are allowed. Type *inspection* (`item.type === "Audio"`) is display +# logic and is not matched. +# +# 🔴 What it still CANNOT see (do not read a green run as proof): +# - a type set built at run time: [...musicTypes, "Playlist"] +# - types split across variables: const A = "Audio"; [A, B] +# - taxonomy as control flow: switch (t) { case "Audio": … } +# t === "Audio" || t === "MusicAlbum" +# - an item type absent from ITEM_TYPES below (false negative by design) +# +# This check was hardened in July 2026 after the audit found it passing on the +# very leak it was written for: the original pattern was anchored to +# `includeItemTypes:` at the query site, so assigning the same array to a named +# const evaded it entirely. See docs/specs/boundary-tripwire-hardening.md (DR-094). # # Escaping a genuine exception: add the file+reason to the ALLOWLIST below. @@ -28,7 +41,7 @@ set -euo pipefail cd "$(dirname "$0")/.." -# Files permitted to contain a multi-type includeItemTypes query, with the reason. +# Files permitted to contain a multi-type item-type array, with the reason. # Keep this SHORT. A growing allowlist means the boundary is eroding — that is a # signal to push taxonomy into Rust, not to keep appending here. ALLOWLIST=( @@ -36,8 +49,31 @@ ALLOWLIST=( # two-type filmography query with no category-configuration behind it. Tracked # as acceptable pending any person-scope work; revisit if it grows. "src/lib/components/library/PersonDetailView.svelte" + + # Grid styling predicate over `config.itemType`, a value the page already + # declares about itself. Selects a *look*, issues no query, and would only + # change if the UI were redesigned — presentation, not taxonomy-as-policy. + "src/lib/components/library/GenericMediaListPage.svelte" + + # "Is this item a container?" predicate for downloads browsing. + # BORDERLINE — leans domain: the container set grows when Jellyfin adds a + # container type. TODO: replace with a backend-supplied `MediaItem.isContainer` + # flag and remove this entry. Tracked in + # docs/specs/boundary-tripwire-hardening.md §Out of scope. + "src/lib/components/downloads/DownloadedBrowse.svelte" ) +# Hard cap so erosion is caught mechanically rather than by whoever notices. +# Deliberately just above the current count: the next exception forces a +# conversation instead of a one-line append. +MAX_ALLOWLIST=4 + +if [[ "${#ALLOWLIST[@]}" -gt "$MAX_ALLOWLIST" ]]; then + echo "❌ Allowlist has ${#ALLOWLIST[@]} entries (max $MAX_ALLOWLIST)." + echo " Push taxonomy into Rust instead of appending here." + exit 1 +fi + is_allowed() { local file="$1" for allowed in "${ALLOWLIST[@]}"; do @@ -46,11 +82,28 @@ is_allowed() { return 1 } -# Multi-element includeItemTypes array: `includeItemTypes: [ , ... ]`. -# The comma inside the brackets is what makes it multi-type. -PATTERN='includeItemTypes:[[:space:]]*\[[^]]*,[^]]*\]' +# Two or more adjacent Jellyfin item-type string literals inside a bracket. +# +# NOT anchored to `includeItemTypes:` — that was the original rule, and it missed +# the real leak: `searchScope.ts` assigned the same array to a named const and +# dereferenced it one indirection away from the query, so the grep never saw it +# while CI stayed green. Matching the array literal itself catches a const, a +# Record value, a function return, and an inline query alike. +# +# Deliberate limits: +# - requires TWO adjacent types, so single-type presentation +# (`itemType: "Movie"`) stays legal — the rule targets *category* taxonomy; +# - requires string literals, so `item.type === "Audio"` (display inspection) +# does not match; +# - uses an explicit type list rather than a generic capitalised-word pattern, +# so unrelated string arrays (`["High","Low"]`) produce no noise. +# +# An item type missing from this list is a false *negative*, never a false +# positive — the check degrades safely as Jellyfin adds types. +ITEM_TYPES='Movie|Series|Episode|Audio|MusicAlbum|MusicArtist|MusicVideo|Season|BoxSet|Playlist|Book|AudioBook|Video|Person|Folder|CollectionFolder|TvChannel|LiveTvChannel' +PATTERN="\[[[:space:]]*\"($ITEM_TYPES)\"[[:space:]]*,[[:space:]]*\"($ITEM_TYPES)\"" -echo "🔎 Checking frontend for domain-taxonomy leaks (multi-type query arrays)…" +echo "🔎 Checking frontend for domain-taxonomy leaks (item-type array literals)…" # Collect hits, excluding tests and the allowlist. violations="" @@ -69,9 +122,11 @@ done < <(grep -rInE "$PATTERN" src/ 2>/dev/null || true) if [[ -n "$violations" ]]; then echo "" - echo "❌ Frontend boundary violation: a multi-type includeItemTypes query defines" - echo " a category in the presentation layer. That taxonomy belongs in Rust —" - echo " send an opaque scope and let the backend expand it to item types." + echo "❌ Frontend boundary violation: an item-type array literal defines a" + echo " category in the presentation layer. That taxonomy belongs in Rust —" + echo " send an opaque scope/enum and let the backend expand it to item types" + echo " (see SearchScope::item_types() in src-tauri/src/repository/types.rs)." + echo " Assigning the array to a const does not make it presentation." echo " See docs/specs/scoped-search-boundary.md and CLAUDE.md." echo "" echo "$violations" | sed 's/^/ /' diff --git a/scripts/test-all.sh b/scripts/test-all.sh index 6b62842d..bd1849f3 100755 --- a/scripts/test-all.sh +++ b/scripts/test-all.sh @@ -7,7 +7,7 @@ echo "🧪 Running all tests..." echo "" echo "📦 Running frontend tests..." -bun run test +bun run test --run echo "" echo "🦀 Running Rust tests..." @@ -15,5 +15,12 @@ cd src-tauri cargo test cd .. +echo "" +echo "🚧 Checking architectural gates..." +# Boundary tripwire (DR-094): no Jellyfin taxonomy in the presentation layer. +bun run check:boundary +# Traceability coverage (DR-093): fails below 50%, or above 100% (miscount). +bun run traces:coverage + echo "" echo "✅ All tests passed!"