From 5e86cb59b07d082078219c549ddbbc626310a876 Mon Sep 17 00:00:00 2001 From: Brummel Date: Mon, 1 Jun 2026 01:42:25 +0200 Subject: [PATCH] fix(web): make closed cases fully viewable (discovery-gate, not access-gate) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to 6d1ca71: that commit made the recordings sub-page render for a closed case by threading ?show_closed=1, but the page's own links and the audio player still 404'd — playback was dead and the back link led nowhere. Root cause: the `.closed` marker was implemented as an ACCESS gate (every case-scoped read 404s a closed case unless the opt-in flag is threaded through), while it is documented as a DISCOVERY gate ("hidden from the default listing"). Threading the flag through every link/redirect is whack-a-mole; the next new link forgets it. Resolved in favour of discovery-gate semantics: the `.closed` marker only filters the default case LIST (scan_user_cases). Direct/deep-link access to a known, owned case and all its sub-resources is closed-agnostic. IDOR is unchanged — it comes from the per-user root + existence check, orthogonal to closed-ness. Scope 1 — closed cases are fully viewable: - handle_audio: drop the is_closed -> 404 gate (web.rs). Audio is a capability URL behind the same per-user path + slug/UUID/filename validation as an open case. - handle_case_page / handle_case_recordings: always resolve via locate_closed_case_or_404; show_closed is no longer an access gate, so back links (recordings -> case, case -> list) reach live pages without threading the flag. - the "back to list" links carry ?show_closed=1 when the case is closed (computed server-side from the marker) so the user returns to the closed-inclusive list, not the default list that hides the case. Scope 2 — closed means read-only: - case_page.html withholds the analyze / reset / re-analyze / retry forms on closed cases; only the reopen affordance remains. - case_recordings.html withholds the per-recording delete form on closed cases (CaseRecordingsTemplate gains is_closed). - the mutating handlers (analyze/reset/delete/oneliner) keep rejecting closed cases via locate_case_or_404 as defense-in-depth behind the now-hidden buttons. Tests (RED-first): - web_test: audio serves a closed case's file (200, not 404). - case_page_test: closed case page + recordings render without ?show_closed=1; closed case hides analyze/reset; open case still shows delete; closed case hides delete. - analyze_test: closed_case_returns_404_on_detail rewritten to closed_case_detail_renders_read_only — the old test pinned the now-superseded access-gate contract. cargo test (440 passed), clippy, and fmt all clean. closes #17 --- server/src/routes/user_web.rs | 69 ++++------ server/src/routes/web.rs | 8 +- server/templates/case_page.html | 14 ++- server/templates/case_recordings.html | 4 +- server/tests/analyze_test.rs | 24 +++- server/tests/case_page_test.rs | 174 ++++++++++++++++++++++++++ server/tests/web_test.rs | 35 ++++++ 7 files changed, 272 insertions(+), 56 deletions(-) diff --git a/server/src/routes/user_web.rs b/server/src/routes/user_web.rs index c3b3d44..5a8945a 100644 --- a/server/src/routes/user_web.rs +++ b/server/src/routes/user_web.rs @@ -409,6 +409,10 @@ struct CaseRecordingsTemplate { recordings: Vec, transcribe_busy: bool, is_admin: bool, + /// True when the case carries the `.closed` marker. Drives the read-only + /// view: the per-recording delete form is withheld and the back-to-list + /// link returns to the closed-inclusive list (`?show_closed=1`). + is_closed: bool, /// Session CSRF token — rendered as a hidden field into the /// per-recording delete forms and the logout form. csrf_token: String, @@ -736,7 +740,6 @@ pub async fn handle_case_page( State(boot_time): State, State(in_flight): State, CaseIdPath(case_id): CaseIdPath, - Query(query): Query, ) -> Result, AppError> { let user_root = config.data_path.join(&user.slug); pipeline @@ -750,24 +753,13 @@ pub async fn handle_case_page( ) .await; - let show_closed = query.include_closed(); - let case_dir = if show_closed { - locate_closed_case_or_404( - &user_root, - &case_id, - &user.slug, - "case page (show_closed=1)", - ) - .await? - } else { - locate_case_or_404( - &user_root, - &case_id, - &user.slug, - "case page (possible IDOR probe)", - ) - .await? - }; + // Direct case access is closed-agnostic: the `.closed` marker only hides + // a case from the default LIST (the discovery gate lives in + // `scan_user_cases`). A deep link or back link to a known, owned case — + // open or closed — must resolve, so the recordings page's back link works + // without threading `?show_closed=1`. IDOR is still guarded by the + // per-user root plus the existence check inside the resolver. + let case_dir = locate_closed_case_or_404(&user_root, &case_id, &user.slug, "case page").await?; // Auto-analysis trigger for deep-link navigation: same evaluation as // the list view. Document render below still wins the race if the // worker happens to finish synchronously. @@ -892,7 +884,6 @@ pub async fn handle_case_recordings( State(http_client): State, State(vocab): State>, CaseIdPath(case_id): CaseIdPath, - Query(query): Query, ) -> Result, AppError> { let user_root = config.data_path.join(&user.slug); pipeline @@ -906,27 +897,12 @@ pub async fn handle_case_recordings( ) .await; - // Closing a case only writes a `.closed` marker; the `.m4a` recordings - // stay on disk. A user who opted into viewing closed cases must still - // reach this list, so branch on `show_closed=1` exactly like - // `handle_case_page` does. - let case_dir = if query.include_closed() { - locate_closed_case_or_404( - &user_root, - &case_id, - &user.slug, - "case recordings (show_closed=1)", - ) - .await? - } else { - locate_case_or_404( - &user_root, - &case_id, - &user.slug, - "case recordings (possible IDOR probe)", - ) - .await? - }; + // Closed-agnostic, like `handle_case_page`: closing only writes a + // `.closed` marker and the `.m4a` recordings stay on disk, so the + // recordings sub-page must resolve for a closed case too. The discovery + // gate is the case LIST, not this view. + let case_dir = + locate_closed_case_or_404(&user_root, &case_id, &user.slug, "case recordings").await?; let case_id_str = case_id.to_string(); info!(slug = %user.slug, case_id = %case_id, "case recordings viewed"); @@ -936,6 +912,7 @@ pub async fn handle_case_recordings( let t_busy = pipeline.transcribe_busy.0.load(Ordering::Acquire); let case_id_short = case_id_str.chars().take(8).collect(); let is_admin = user.is_admin(); + let is_closed = crate::paths::is_closed(&case_dir).await; CaseRecordingsTemplate { csrf_token: user.csrf_token, @@ -946,6 +923,7 @@ pub async fn handle_case_recordings( recordings, transcribe_busy: t_busy, is_admin, + is_closed, } .render() .map(Html) @@ -992,9 +970,12 @@ pub(crate) async fn locate_case_or_404( } } -/// Like `locate_case_or_404` but accepts closed cases. Only the detail -/// view uses this, and only when the user navigated there with -/// `?show_closed=1`. A non-existent directory still 404s as expected. +/// Like `locate_case_or_404` but accepts closed cases. The read-only VIEW +/// handlers (case page, recordings sub-page) use this: the `.closed` marker +/// is a discovery filter for the case LIST, not an access gate on a known +/// case, so direct/deep-link access resolves regardless of closed-ness. The +/// mutating handlers keep using `locate_case_or_404` (which rejects closed) +/// since a closed case is read-only. A non-existent directory still 404s. pub(crate) async fn locate_closed_case_or_404( user_root: &Path, case_id: &CaseId, diff --git a/server/src/routes/web.rs b/server/src/routes/web.rs index cbbf433..60eba68 100644 --- a/server/src/routes/web.rs +++ b/server/src/routes/web.rs @@ -56,10 +56,12 @@ pub async fn handle_audio( .map_err(|_| AppError::BadRequest("Invalid case_id (not a UUID)".into()))?; validate_filename(&filename)?; + // The `.closed` marker is a discovery filter for the case LIST, not an + // access gate on the audio file server: a closed case keeps its `.m4a` + // files on disk and its recordings page must still play them. IDOR is + // guarded by the per-user path plus the slug/UUID/filename validation + // above, independent of whether the case is closed. let case_path = crate::paths::case_dir(&config.data_path, &user, &case_id); - if crate::paths::is_closed(&case_path).await { - return Err(AppError::NotFound("Audio file not found".into())); - } let file_path = case_path.join(&filename); let metadata = match tokio::fs::metadata(&file_path).await { Ok(m) => m, diff --git a/server/templates/case_page.html b/server/templates/case_page.html index 3ad2c7e..d3025dd 100644 --- a/server/templates/case_page.html +++ b/server/templates/case_page.html @@ -377,7 +377,7 @@
- +
{% if is_admin %}