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 <noreply@anthropic.com>
This commit is contained in:
2026-10-15 13:18:08 +00:00
parent a9991545f9
commit 4e0b5eed5d
7 changed files with 81 additions and 3 deletions

View File

@@ -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).

View File

@@ -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.

View File

@@ -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]

View File

@@ -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).

View File

@@ -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() };

View File

@@ -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() {

View File

@@ -1907,7 +1907,7 @@ async fn art(State(state): State<WebState>, Query(q): Query<ArtQuery>) -> 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()
}