From 4e0b5eed5df1c4752d854724c04883ef6d0897ca Mon Sep 17 00:00:00 2001 From: rays Date: Thu, 15 Oct 2026 13:18:08 +0000 Subject: [PATCH] Failures and events go out without a private feed's key (#238) A fetch that failed logged reqwest's words, which name the whole address: Patreon's auth key went to Loki on 2026-10-15, and, since scan events go to every open page, to everyone's browser too. #225 had taken queries out of the traces' address fields only. Every event is redacted where it is emitted, for the log, the live stream and a terminal alike: address fields lose their query and password, and so does any address in an error's words (feed::redact_text). The artwork proxy's warnings and the directory's failed checks the same. The error kept on the feed, shown to its own subscribers, keeps the detail. Co-Authored-By: Claude Opus 5.5 --- crates/directory/CHANGELOG.md | 4 +++ crates/directory/src/lib.rs | 2 +- crates/feed/src/lib.rs | 22 ++++++++++++++++ crates/ipx/CHANGELOG.md | 4 +++ crates/ipx/src/ipc.rs | 48 +++++++++++++++++++++++++++++++++++ crates/ipx/src/main.rs | 2 +- crates/ipx/src/web.rs | 2 +- 7 files changed, 81 insertions(+), 3 deletions(-) diff --git a/crates/directory/CHANGELOG.md b/crates/directory/CHANGELOG.md index 03263c5..8a9fe13 100644 --- a/crates/directory/CHANGELOG.md +++ b/crates/directory/CHANGELOG.md @@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security + +- A failed feed check is logged without its address's query or password (#238). + ### Changed - The directory checks five feeds at a time, so a publisher that does not answer no longer holds up the rest of its round (#227). diff --git a/crates/directory/src/lib.rs b/crates/directory/src/lib.rs index e0d3d13..081697b 100644 --- a/crates/directory/src/lib.rs +++ b/crates/directory/src/lib.rs @@ -177,7 +177,7 @@ impl Directory { Err(e) => Refreshed::Failed(format!("{e:#}")), }; if let Refreshed::Failed(e) = &got { - tracing::info!(feed = %due.id, error = %e, "feed check failed"); + tracing::info!(feed = %due.id, error = %ipx_feed::redact_text(e), "feed check failed"); } // ponytail: a feed that moved for good is still asked for at its old address, which // redirects; follow the move when a feed's id can change without losing its counts. diff --git a/crates/feed/src/lib.rs b/crates/feed/src/lib.rs index 6b638c1..18727fb 100644 --- a/crates/feed/src/lib.rs +++ b/crates/feed/src/lib.rs @@ -96,6 +96,21 @@ pub fn redact(url: &str) -> String { } } +/// Text with each address in it redacted, for what is logged and sent to every open page: an +/// error from reqwest names the whole address it fetched, a private feed's key with it (#238). +pub fn redact_text(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + let mut rest = text; + while let Some(i) = rest.find("http://").into_iter().chain(rest.find("https://")).min() { + out.push_str(&rest[..i]); + let end = rest[i..].find(|c: char| c.is_whitespace() || "()<>\"'".contains(c)).map_or(rest.len(), |n| i + n); + out.push_str(&redact(&rest[i..end])); + rest = &rest[end..]; + } + out.push_str(rest); + out +} + #[tracing::instrument(skip_all, fields(url = %redact(feed_url), http.response.body.size))] pub async fn fetch( client: &reqwest::Client, @@ -1202,6 +1217,13 @@ mod tests { assert_eq!(super::redact("https://me:pw@example.com/feed.xml#top"), "https://example.com/feed.xml"); assert_eq!(super::redact("https://example.com/feed.xml"), "https://example.com/feed.xml"); assert_eq!(super::redact("not a url?token=SECRET"), "not a url"); + // As reqwest words a failure, and with two addresses and none. + assert_eq!( + super::redact_text("connecting: error sending request for url (https://www.patreon.com/rss/x?auth=SECRET&show=1): operation timed out"), + "connecting: error sending request for url (https://www.patreon.com/rss/x): operation timed out" + ); + assert_eq!(super::redact_text("from http://a/f?k=1 to https://u:p@b/g"), "from http://a/f to https://b/g"); + assert_eq!(super::redact_text("HTTP 404 Not Found"), "HTTP 404 Not Found"); } #[test] diff --git a/crates/ipx/CHANGELOG.md b/crates/ipx/CHANGELOG.md index c06762a..45301f1 100644 --- a/crates/ipx/CHANGELOG.md +++ b/crates/ipx/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security + +- A failure's words, and the addresses in an event, go to the log and to every open page without their query or password, where a private feed keeps its key: a failed fetch of a Patreon feed logged its `auth` key (#238). + ### Changed - Check every feed looks up the site icons of feeds without artwork a few at a time, alongside the fetches, instead of one after another (#228). diff --git a/crates/ipx/src/ipc.rs b/crates/ipx/src/ipc.rs index 2e8c152..2bf1427 100644 --- a/crates/ipx/src/ipc.rs +++ b/crates/ipx/src/ipc.rs @@ -144,6 +144,7 @@ impl Emitter { } pub fn emit(&self, e: Event) { + let e = e.redacted(); // Also log it. Scans and downloads travel as events, not tracing calls, so // without this the log view shows only startup and HTTP lines and none of the // work the daemon is actually doing. Progress goes to debug: it fires on every @@ -167,6 +168,29 @@ impl Emitter { } } +impl Event { + /// The event as it may go out: to the log, to every open page whoever's feed it is, to a + /// terminal. Addresses lose their query and password, where a private feed keeps its key, and + /// so do the ones in an error's words (#238). + fn redacted(mut self) -> Self { + use crate::feed::{redact, redact_text}; + match &mut self { + Event::DownloadError { url, msg, .. } => { + *url = redact(url); + *msg = redact_text(msg); + } + Event::TorrentDeferred { url, .. } => *url = redact(url), + Event::FeedMoved { from, to, .. } => { + *from = redact(from); + *to = redact(to); + } + Event::FeedError { msg, .. } | Event::Error { msg } => *msg = redact_text(msg), + _ => {} + } + self + } +} + /// An event, logged once (#91): its words as the message, and `ev`, `feed`, `new` and the rest as /// fields, so Loki reads them without parsing the message. A failure also gets `error.type` and, /// from an HTTP error, `http.response.status_code`, so failures group by kind without a regex. @@ -386,6 +410,30 @@ mod tests { assert!(matches!(rx.try_recv(), Ok(Event::FeedSkip { feed, .. }) if feed == "g"), "nothing missed"); } + #[test] + fn what_goes_out_names_no_private_feeds_key() { + let (tx, mut rx) = broadcast::channel(8); + let out = Emitter::socket(tx, false); + let key = "auth=SECRET"; + out.emit(Event::FeedError { + feed: "f".into(), + msg: format!("connecting: error sending request for url (https://x/rss?{key}&show=1): operation timed out"), + }); + out.emit(Event::DownloadError { + feed: "f".into(), + enclosure: 1, + url: format!("https://cdn/ep.mp3?{key}"), + msg: format!("connecting: error sending request for url (https://cdn/ep.mp3?{key})"), + }); + out.emit(Event::FeedMoved { feed: "f".into(), from: format!("https://x/rss?{key}"), to: format!("https://y/rss?{key}") }); + // What every open page is sent, whoever's feed it is (#238). + for _ in 0..3 { + let sent = serde_json::to_string(&rx.try_recv().unwrap()).unwrap(); + assert!(!sent.contains("SECRET"), "{sent}"); + assert!(sent.contains("https://"), "the address is still there, without its key: {sent}"); + } + } + #[test] fn a_feed_that_moved_says_so_on_the_wire_and_in_words() { let e = Event::FeedMoved { feed: "x".into(), from: "http://a/f".into(), to: "https://a/f".into() }; diff --git a/crates/ipx/src/main.rs b/crates/ipx/src/main.rs index 9c8a8aa..7cd5df5 100644 --- a/crates/ipx/src/main.rs +++ b/crates/ipx/src/main.rs @@ -1753,7 +1753,7 @@ async fn scan_one( && stored.image.as_deref() != Some(art.as_str()) && !feed::is_image(&ctx.client, art).await { - tracing::info!(feed = id, art = %art, "the feed's artwork is not an image; trying its site's icon"); + tracing::info!(feed = id, art = %feed::redact(art), "the feed's artwork is not an image; trying its site's icon"); parsed.image = None; } if parsed.image.is_none() { diff --git a/crates/ipx/src/web.rs b/crates/ipx/src/web.rs index 6f0a6e4..c554137 100644 --- a/crates/ipx/src/web.rs +++ b/crates/ipx/src/web.rs @@ -1907,7 +1907,7 @@ async fn art(State(state): State, Query(q): Query) -> Respon Err(e) => { let why = format!("{e:#}"); // The access line says only that it failed, not which image or why (#169). - tracing::warn!(url = %q.u, error = %why, "could not fetch artwork"); + tracing::warn!(url = %crate::feed::redact(&q.u), error = %crate::feed::redact_text(&why), "could not fetch artwork"); crate::art::fail(&q.u, &why); (StatusCode::BAD_GATEWAY, why).into_response() }