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:
@@ -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).
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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() };
|
||||
|
||||
@@ -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() {
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user