From b86062b97a0ec1d85bab7f5d094bfe60a9fed85f Mon Sep 17 00:00:00 2001 From: rays Date: Mon, 5 Oct 2026 16:19:38 +0000 Subject: [PATCH] Keep a feed's items and files to the people who subscribe to it (#129) Routes that take a feed or an enclosure id did not check who was asking. Anyone signed in could read any feed's items through GET /api/feeds/{id}/entries, a paid feed's included, with the addresses of its files, which can carry the subscriber's key: the Directory leaves such feeds out for that reason, and this route handed them back to whoever guessed the id, a slug of the title. In production it answered 25 items of a feed the asking account does not subscribe to. /media/{id} served any downloaded file by its sequential id, POST /api/enclosures/{id}/download and /api/feeds/{id}/download-latest queued any feed's downloads, and DELETE /api/enclosures/{id}?force=true deleted any file. Each now answers 404, "you do not subscribe to that feed", unless the person subscribes to it. A feed inside an OPML has a subscription row of its own for everyone subscribed to the OPML, so that holds for those feeds too. Found while adding the Directory's feed page (#128), which has its own route that answers only for listed feeds and carries no files. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++++ docs/architecture.md | 3 ++- src/web.rs | 22 ++++++++++++++++++++++ tests/ui/app.spec.js | 14 +++++++++++++- 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d4c2ed..2d66caf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - An iPhone adding the site to its home screen finds the icon at the first address it tries. - A feed whose server hangs no longer holds up every scan: a feed gets 30 seconds, and connecting anywhere 10. +### Security + +- A feed's items and files reach only the people who subscribe to it. Anyone signed in could read any feed's items, a paid feed's included, with the addresses of its files, which can carry the subscriber's key, and could play, download or delete its files. + ## [0.9.1] - 2026-09-29 ### Added diff --git a/docs/architecture.md b/docs/architecture.md index d5e0783..064247d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -110,7 +110,8 @@ so a long download had to be able to notice the signal itself. ## HTTP API Everything below `/api` needs a signed-in user; the browser gets a redirect to `/login`, anything -else a `401`. +else a `401`. A feed's items and files (its entries, `download-latest`, `/api/enclosures/{id}` and +`/media/{id}`) answer only someone who subscribes to the feed; anyone else gets a `404`. | Route | | |---|---| diff --git a/src/web.rs b/src/web.rs index de613d4..0937d02 100644 --- a/src/web.rs +++ b/src/web.rs @@ -1325,9 +1325,22 @@ async fn entries( user: crate::db::User, Query(page): Query, ) -> Result, ApiError> { + subscribed(&state, &user, &id).await?; entry_page(&state, user.id, Some(&id), &page).await } +/// A feed's items and files are for the people who subscribe to it (#129). Anyone signed in +/// could read any feed's by its id, a paid one's included, with its enclosure addresses, which +/// can carry the subscriber's key, and fetch, queue or delete its files by theirs. A feed in +/// an OPML has a subscription of its own for everyone subscribed to the OPML, so this holds +/// for those too. Not found, as for any other feed that is not yours. +async fn subscribed(state: &WebState, user: &crate::db::User, feed_id: &str) -> Result<(), ApiError> { + match state.ctx.db.subscription(user.id, feed_id).await? { + Some(_) => Ok(()), + None => Err(ApiError::not_found("you do not subscribe to that feed")), + } +} + /// Every subscribed feed's items together, newest first: All Subscriptions. async fn all_entries( State(state): State, @@ -1676,12 +1689,14 @@ async fn set_flags( async fn download_now( State(state): State, Path(id): Path, + user: crate::db::User, ) -> Result { let enc = state .ctx .db .enclosure(id).await? .ok_or_else(|| anyhow::anyhow!("no enclosure {id}"))?; + subscribed(&state, &user, &enc.feed_id).await?; if enc.path.is_some() { return Ok(StatusCode::NO_CONTENT); // Already here. } @@ -1711,6 +1726,7 @@ async fn delete_file( .db .enclosure(id).await? .ok_or_else(|| anyhow::anyhow!("no enclosure {id}"))?; + subscribed(&state, &user, &enc.feed_id).await?; // There is one copy of the file: deleting it deletes everyone's. Say so before doing // it, once, and let them decide. @@ -1828,11 +1844,15 @@ fn changed_feed(ev: &Event) -> Option<&str> { async fn media( State(state): State, Path(id): Path, + user: crate::db::User, req: Request, ) -> Response { let Ok(Some(enc)) = state.ctx.db.enclosure(id).await else { return (StatusCode::NOT_FOUND, "no such enclosure").into_response(); }; + if let Err(e) = subscribed(&state, &user, &enc.feed_id).await { + return e.into_response(); + } let Some(path) = enc.path else { return (StatusCode::NOT_FOUND, "not downloaded").into_response(); }; @@ -1962,8 +1982,10 @@ fn five() -> i64 { async fn download_latest( State(state): State, Path(id): Path, + user: crate::db::User, Json(body): Json, ) -> Result, ApiError> { + subscribed(&state, &user, &id).await?; let ids = state.ctx.db.undownloaded(&id, body.count.clamp(1, 100)).await?; for enc in &ids { state.ctx.db.requeue(*enc).await?; diff --git a/tests/ui/app.spec.js b/tests/ui/app.spec.js index 3ceca82..dad0363 100644 --- a/tests/ui/app.spec.js +++ b/tests/ui/app.spec.js @@ -812,7 +812,7 @@ test('ipx import subscribes the admin, and ipx export writes the feeds out', asy expect(xml).toContain('http://127.0.0.1:8792/two.xml'); }); -test('the Directory lists what everyone here reads, the most subscribed first, but never a private feed', async ({ browser }) => { +test('the Directory lists what everyone here reads, the most subscribed first, but never a private feed', async ({ browser, page }) => { const { execFileSync } = require('child_process'); const setup = require('./global-setup'); const env = { @@ -919,6 +919,17 @@ test('the Directory lists what everyone here reads, the most subscribed first, b expect(preview).toContain('First Episode'); expect(preview).not.toContain('.mp3'); expect((await piper.request.get('/api/directory/paid-show')).status()).toBe(404); + // Nor does anything else give a feed's items or files to someone who does not subscribe (#129). + expect((await piper.request.get('/api/feeds/test-show/entries')).status()).toBe(404); + expect((await piper.request.get('/api/feeds/paid-show/entries')).status()).toBe(404); + const pics = await page.evaluate(() => api('/api/feeds/picture-blog/entries')); + const file = pics.entries.flatMap(e => e.enclosures).find(x => x.path); + expect(file).toBeTruthy(); + expect((await page.request.get(`/media/${file.id}`)).status()).toBe(200); + expect((await piper.request.get(`/media/${file.id}`)).status()).toBe(404); + expect((await piper.request.post(`/api/enclosures/${file.id}/download`)).status()).toBe(404); + expect((await piper.request.delete(`/api/enclosures/${file.id}?force=true`)).status()).toBe(404); + expect((await piper.request.post('/api/feeds/picture-blog/download-latest', { data: { count: 1 } })).status()).toBe(404); // Add a feed is for an address; the Directory is where you browse (issue #30). await piper.locator('#addFeed').click(); @@ -935,6 +946,7 @@ test('the Directory lists what everyone here reads, the most subscribed first, b // Everyone counts, you included: it stays listed, marked as yours, with one more subscriber. expect(await row()).toMatchObject({ subscribed: true, subscribers: before.subscribers + 1 }); + expect((await piper.request.get('/api/feeds/test-show/entries')).status()).toBe(200); await piper.locator('#feedlist .place', { hasText: 'Directory' }).click(); await expect(chart.filter({ hasText: 'Test Show' }).locator('[title^="Subscribed"]')).toBeVisible(); await expect(chart.filter({ hasText: 'Test Show' }).locator('button[title="Subscribe"]')).toHaveCount(0);