feat(diagnostics): persistent redacted logging and an exportable bundle
🏗️ Build and Test JellyTau / Run Tests (pull_request) Successful in 22m12s
🏗️ Build and Test JellyTau / Supply Chain (pull_request) Successful in 37s
Traceability Validation / Check Requirement Traces (pull_request) Successful in 11s
🏗️ Build and Test JellyTau / Android Compile Check (pull_request) Successful in 4m10s
🏗️ Build and Test JellyTau / Run Tests (pull_request) Successful in 22m12s
🏗️ Build and Test JellyTau / Supply Chain (pull_request) Successful in 37s
Traceability Validation / Check Requirement Traces (pull_request) Successful in 11s
🏗️ Build and Test JellyTau / Android Compile Check (pull_request) Successful in 4m10s
The app forgot everything it did the moment it exited. The Rust half
logged through env_logger to stdout only -- invisible to anyone who
launched from a desktop icon, and on Android worse than that: stdout is
not logcat, so the backend produced no visible output at all on the
platform carrying this project's hardest bugs. The autoplay deadlock,
the truncated-stream restart and the background-audio stall were all
diagnosed by talking a user through `adb logcat`, because there was no
other way to see anything. A panic left nothing behind at all.
Logs now go to a size-capped rotating file, to logcat on Android, and to
the webview console in dev. A panic is recorded with its backtrace before
the process dies. The frontend's messages are forwarded into the same
file, so one timeline holds both halves of the app in order -- which is
what makes a race between them legible after the fact, and races between
them are the expensive bug class here.
Redaction runs in the log FORMATTER, not at export time. A credential
sitting in a file on the device is already a disclosure; stripping it on
the way out would be too late. The exporter redacts a second time to
cover files written by builds that predate this. api_key, X-Emby-Token,
Authorization, "AccessToken" and Token="..." all reduce to [REDACTED],
while host, item ids and filenames are deliberately kept -- a log scrubbed
of those is one nobody can debug anything from. Server URLs keep scheme
and host and drop any embedded user:pass@.
Two things the tests caught that review would not have:
- redact_headers recursed on its own output. The replacement keeps the
header NAME, so the next call matched the same header forever; the
test died with a stack overflow. It is a forward scan now.
- The frontend forwarder used `void plugin.error(...)`. `void` discards
a promise's value but not its rejection, so in any webview without
IPC -- a unit test, SSR, a browser preview -- every log line became an
unhandled rejection. 20 of them showed up the first time coverage
ran. Each call now attaches a catch.
Only info and above cross the IPC boundary: debug is per-tick player
state and forwarding it would be thousands of calls a minute for output
nobody reads. A failing forwarder never propagates and never prevents the
console write.
Nothing is transmitted anywhere. The export writes a zip and reports its
path; the user attaches it themselves, which is also what keeps this from
becoming telemetry. An Android share intent is explicitly out of scope --
it is Kotlin work that belongs with the other native code.
The panic hook chains to the previous hook rather than replacing it,
because utils/lock.rs installs a silencing hook around tests that provoke
poisoned locks on purpose.
Spec in docs/specs/diagnostics-and-logging.md; UR-078 / DR-218 / UT-209.
Verified: 1079 frontend tests and the coverage gate, 759 Rust tests,
clippy -D warnings, svelte-check 0 errors, and cargo check for
aarch64-linux-android.
This commit is contained in:
@@ -0,0 +1,399 @@
|
||||
//! Credential redaction and crash capture for diagnostic logs.
|
||||
//!
|
||||
//! TRACES: UR-078 | DR-218
|
||||
//!
|
||||
//! ## Why redaction lives here and not at the export
|
||||
//!
|
||||
//! A diagnostic bundle is something a user attaches to a public bug report. If a
|
||||
//! Jellyfin access token can reach it, this feature is a credential-disclosure
|
||||
//! bug with a friendly button on it.
|
||||
//!
|
||||
//! So [`redact`] runs in the log *formatter* — the token never reaches disk —
|
||||
//! and again over every line the exporter copies, which covers files written by
|
||||
//! an older build that lacked the formatter pass. Redacting only at export would
|
||||
//! leave the secret sitting in a file on the device, which is exactly the thing
|
||||
//! we are trying not to do.
|
||||
//!
|
||||
//! ## What is deliberately NOT redacted
|
||||
//!
|
||||
//! Server host, item ids, filenames and paths inside the app's own directories
|
||||
//! all stay. They are not secrets and they are the entire diagnostic value of a
|
||||
//! log: a bundle scrubbed of them is one nobody can debug anything from.
|
||||
|
||||
use std::borrow::Cow;
|
||||
|
||||
/// Replacement for a redacted value.
|
||||
pub const REDACTED: &str = "[REDACTED]";
|
||||
|
||||
/// Query-string parameters whose value is a credential.
|
||||
///
|
||||
/// Jellyfin accepts the API key under several spellings depending on the
|
||||
/// endpoint and client generation, and this codebase has emitted more than one
|
||||
/// of them over time.
|
||||
const SECRET_QUERY_KEYS: &[&str] = &["api_key", "apikey", "x-emby-token", "accesstoken"];
|
||||
|
||||
/// Header names whose value is a credential.
|
||||
const SECRET_HEADERS: &[&str] = &[
|
||||
"x-emby-token",
|
||||
"x-mediabrowser-token",
|
||||
"authorization",
|
||||
"x-emby-authorization",
|
||||
];
|
||||
|
||||
/// JSON keys whose value is a credential.
|
||||
const SECRET_JSON_KEYS: &[&str] = &["accesstoken", "password", "token"];
|
||||
|
||||
/// Strip credentials from one log line.
|
||||
///
|
||||
/// Idempotent: redacting an already-redacted line changes nothing, which matters
|
||||
/// because the exporter may re-process a file the formatter already cleaned.
|
||||
pub fn redact(line: &str) -> String {
|
||||
let mut out = redact_query_params(line);
|
||||
out = redact_headers(&out);
|
||||
out = redact_json_values(&out);
|
||||
out = redact_emby_auth(&out);
|
||||
out
|
||||
}
|
||||
|
||||
/// `?api_key=abc&x=1` -> `?api_key=[REDACTED]&x=1`
|
||||
///
|
||||
/// The value ends at the first character that cannot be part of one: `&`
|
||||
/// separates parameters, and whitespace/quotes mean the URL itself ended.
|
||||
fn redact_query_params(line: &str) -> String {
|
||||
let mut result = String::with_capacity(line.len());
|
||||
let lower = line.to_ascii_lowercase();
|
||||
let bytes = line.as_bytes();
|
||||
let mut i = 0;
|
||||
|
||||
while i < bytes.len() {
|
||||
let mut matched = None;
|
||||
for key in SECRET_QUERY_KEYS {
|
||||
// A key only counts when it is preceded by ? or & (or starts the
|
||||
// line), so a *word* like "token" inside prose is left alone.
|
||||
if lower[i..].starts_with(key) {
|
||||
let prev = if i == 0 { None } else { Some(bytes[i - 1]) };
|
||||
let is_param_start = matches!(prev, None | Some(b'?') | Some(b'&'));
|
||||
let after = i + key.len();
|
||||
if is_param_start && after < bytes.len() && bytes[after] == b'=' {
|
||||
matched = Some((*key, after + 1));
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
match matched {
|
||||
Some((key, value_start)) => {
|
||||
result.push_str(&line[i..i + key.len()]);
|
||||
result.push('=');
|
||||
result.push_str(REDACTED);
|
||||
let mut end = value_start;
|
||||
while end < bytes.len()
|
||||
&& !matches!(bytes[end], b'&' | b' ' | b'"' | b'\'' | b'\t' | b')')
|
||||
{
|
||||
end += 1;
|
||||
}
|
||||
i = end;
|
||||
}
|
||||
None => {
|
||||
// Advance one whole char, not one byte: a UTF-8 boundary split
|
||||
// would panic on the slice above.
|
||||
let ch = line[i..].chars().next().unwrap_or('\0');
|
||||
result.push(ch);
|
||||
i += ch.len_utf8();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
result
|
||||
}
|
||||
|
||||
/// `X-Emby-Token: abc` -> `X-Emby-Token: [REDACTED]`
|
||||
///
|
||||
/// Scans forward from a cursor rather than recursing on the rewritten string.
|
||||
/// The obvious recursive version does not terminate: the replacement keeps the
|
||||
/// header *name*, so the next call finds the same header again and recurses
|
||||
/// until the stack is gone. A test provoked exactly that.
|
||||
fn redact_headers(line: &str) -> String {
|
||||
let mut out = String::with_capacity(line.len());
|
||||
let mut rest = line;
|
||||
|
||||
'outer: loop {
|
||||
let lower = rest.to_ascii_lowercase();
|
||||
|
||||
// Earliest header match in what remains, so several headers on one line
|
||||
// are handled left to right.
|
||||
let mut best: Option<(usize, usize)> = None;
|
||||
for header in SECRET_HEADERS {
|
||||
let needle = format!("{header}:");
|
||||
if let Some(pos) = lower.find(&needle) {
|
||||
let candidate = (pos, needle.len());
|
||||
if best.is_none_or(|(best_pos, _)| pos < best_pos) {
|
||||
best = Some(candidate);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let Some((pos, needle_len)) = best else {
|
||||
break 'outer;
|
||||
};
|
||||
|
||||
let value_start = pos + needle_len;
|
||||
// The value runs to the next comma or the end of the line: reqwest's
|
||||
// debug output prints several headers comma-separated on one line.
|
||||
let value_end = rest[value_start..]
|
||||
.find(',')
|
||||
.map_or(rest.len(), |c| value_start + c);
|
||||
|
||||
out.push_str(&rest[..value_start]);
|
||||
out.push(' ');
|
||||
out.push_str(REDACTED);
|
||||
|
||||
// Continue strictly *after* the value just handled -- this is what makes
|
||||
// the loop terminate.
|
||||
rest = &rest[value_end..];
|
||||
}
|
||||
|
||||
out.push_str(rest);
|
||||
out
|
||||
}
|
||||
|
||||
/// `"AccessToken":"abc"` -> `"AccessToken":"[REDACTED]"`
|
||||
fn redact_json_values(line: &str) -> String {
|
||||
let mut out = Cow::Borrowed(line);
|
||||
|
||||
for key in SECRET_JSON_KEYS {
|
||||
loop {
|
||||
let lower = out.to_ascii_lowercase();
|
||||
let pattern = format!("\"{key}\"");
|
||||
let Some(key_pos) = lower.find(&pattern) else {
|
||||
break;
|
||||
};
|
||||
|
||||
// Find the opening quote of the value after the colon.
|
||||
let after_key = key_pos + pattern.len();
|
||||
let Some(colon_rel) = out[after_key..].find(':') else {
|
||||
break;
|
||||
};
|
||||
let value_region = after_key + colon_rel + 1;
|
||||
let Some(open_rel) = out[value_region..].find('"') else {
|
||||
break;
|
||||
};
|
||||
let open = value_region + open_rel;
|
||||
let Some(close_rel) = out[open + 1..].find('"') else {
|
||||
break;
|
||||
};
|
||||
let close = open + 1 + close_rel;
|
||||
|
||||
// Already redacted: stop, or this loops forever.
|
||||
if &out[open + 1..close] == REDACTED {
|
||||
break;
|
||||
}
|
||||
|
||||
let mut replaced = String::with_capacity(out.len());
|
||||
replaced.push_str(&out[..open + 1]);
|
||||
replaced.push_str(REDACTED);
|
||||
replaced.push_str(&out[close..]);
|
||||
out = Cow::Owned(replaced);
|
||||
}
|
||||
}
|
||||
|
||||
out.into_owned()
|
||||
}
|
||||
|
||||
/// `MediaBrowser Token="abc"` -> `MediaBrowser Token="[REDACTED]"`
|
||||
///
|
||||
/// Jellyfin's own auth header format, which is not JSON and not a query param.
|
||||
fn redact_emby_auth(line: &str) -> String {
|
||||
let lower = line.to_ascii_lowercase();
|
||||
let Some(pos) = lower.find("token=\"") else {
|
||||
return line.to_string();
|
||||
};
|
||||
let open = pos + "token=\"".len();
|
||||
let Some(close_rel) = line[open..].find('"') else {
|
||||
return line.to_string();
|
||||
};
|
||||
let close = open + close_rel;
|
||||
if &line[open..close] == REDACTED {
|
||||
return line.to_string();
|
||||
}
|
||||
let mut out = String::with_capacity(line.len());
|
||||
out.push_str(&line[..open]);
|
||||
out.push_str(REDACTED);
|
||||
out.push_str(&line[close..]);
|
||||
out
|
||||
}
|
||||
|
||||
/// Reduce a server URL to scheme and host.
|
||||
///
|
||||
/// The host is diagnostic (is it https? a LAN address? a reverse proxy?); the
|
||||
/// path and any query on it are not, and a configured URL has been seen to carry
|
||||
/// a token.
|
||||
pub fn redact_server_url(url: &str) -> String {
|
||||
let Some(scheme_end) = url.find("://") else {
|
||||
return REDACTED.to_string();
|
||||
};
|
||||
let after_scheme = scheme_end + 3;
|
||||
let host_end = url[after_scheme..]
|
||||
.find('/')
|
||||
.map_or(url.len(), |slash| after_scheme + slash);
|
||||
// Credentials embedded as user:pass@host must not survive.
|
||||
let host = &url[after_scheme..host_end];
|
||||
let host = host.rsplit('@').next().unwrap_or(host);
|
||||
format!("{}://{}", &url[..scheme_end], host)
|
||||
}
|
||||
|
||||
/// Install a panic hook that records the panic through `log::error!` before the
|
||||
/// default hook runs.
|
||||
///
|
||||
/// # Why it chains rather than replaces
|
||||
///
|
||||
/// `utils::lock` installs a silencing hook around its own tests, which
|
||||
/// deliberately provoke poisoned locks. Replacing the current hook here would
|
||||
/// make that test output scream about panics it is intentionally causing — and,
|
||||
/// more importantly, replacing whatever hook is present is how you lose the
|
||||
/// backtrace the runtime would otherwise print.
|
||||
pub fn install_panic_hook() {
|
||||
let previous = std::panic::take_hook();
|
||||
std::panic::set_hook(Box::new(move |info| {
|
||||
// The payload is very often the formatted message of a `panic!`, so it
|
||||
// goes through redaction like any other line: a panic inside the HTTP
|
||||
// layer can carry a URL.
|
||||
let payload = panic_payload_string(info);
|
||||
let location = info
|
||||
.location()
|
||||
.map(|l| format!("{}:{}", l.file(), l.line()))
|
||||
.unwrap_or_else(|| "unknown location".to_string());
|
||||
|
||||
log::error!("PANIC at {location}: {}", redact(&payload));
|
||||
log::error!("backtrace:\n{}", std::backtrace::Backtrace::force_capture());
|
||||
|
||||
previous(info);
|
||||
}));
|
||||
}
|
||||
|
||||
/// Extract a printable message from a panic payload.
|
||||
fn panic_payload_string(info: &std::panic::PanicHookInfo<'_>) -> String {
|
||||
let payload = info.payload();
|
||||
if let Some(s) = payload.downcast_ref::<&str>() {
|
||||
(*s).to_string()
|
||||
} else if let Some(s) = payload.downcast_ref::<String>() {
|
||||
s.clone()
|
||||
} else {
|
||||
"non-string panic payload".to_string()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn redacts_api_key_query_parameter() {
|
||||
let line = "GET https://media.example.com/Items?api_key=abc123def&Limit=50";
|
||||
let out = redact(line);
|
||||
assert!(!out.contains("abc123def"), "token survived: {out}");
|
||||
assert!(out.contains("api_key=[REDACTED]"));
|
||||
// The rest of the URL is what makes the line worth keeping.
|
||||
assert!(out.contains("media.example.com"));
|
||||
assert!(out.contains("Limit=50"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redacts_every_spelling_of_the_key_parameter() {
|
||||
for key in ["api_key", "ApiKey", "X-Emby-Token", "AccessToken"] {
|
||||
let line = format!("https://h/Items?{key}=SECRETVALUE&x=1");
|
||||
let out = redact(&line);
|
||||
assert!(!out.contains("SECRETVALUE"), "{key} survived: {out}");
|
||||
assert!(out.contains("x=1"), "{key} ate the next parameter: {out}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redacts_auth_headers() {
|
||||
let out = redact("request headers: X-Emby-Token: abc123, Accept: application/json");
|
||||
assert!(!out.contains("abc123"), "{out}");
|
||||
// A following header must survive -- the value stops at the comma.
|
||||
assert!(out.contains("Accept: application/json"), "{out}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redacts_authorization_header() {
|
||||
let out = redact("Authorization: Bearer verysecrettoken");
|
||||
assert!(!out.contains("verysecrettoken"), "{out}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redacts_json_access_token() {
|
||||
let out = redact(r#"login response {"User":{"Name":"duncan"},"AccessToken":"abc123"}"#);
|
||||
assert!(!out.contains("abc123"), "{out}");
|
||||
// The username is not a credential and is diagnostic.
|
||||
assert!(out.contains("duncan"), "{out}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redacts_the_emby_auth_header_form() {
|
||||
let line = r#"MediaBrowser Client="JellyTau", Token="abc123xyz""#;
|
||||
let out = redact(line);
|
||||
assert!(!out.contains("abc123xyz"), "{out}");
|
||||
assert!(out.contains("JellyTau"), "{out}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_idempotent() {
|
||||
// The exporter re-processes files the formatter already cleaned; a
|
||||
// second pass must not corrupt them or loop.
|
||||
let once = redact("https://h/Items?api_key=abc&z=1");
|
||||
let twice = redact(&once);
|
||||
assert_eq!(once, twice);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn leaves_ordinary_lines_untouched() {
|
||||
let line = "player: advancing to next episode (item 4f2a, position 0)";
|
||||
assert_eq!(redact(line), line);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn does_not_redact_the_word_token_in_prose() {
|
||||
// "token" appears in comments and messages constantly. Only a real
|
||||
// parameter or header assignment should trigger.
|
||||
let line = "refreshing the access token because the session expired";
|
||||
assert_eq!(redact(line), line);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn handles_multibyte_characters_without_panicking() {
|
||||
// The scanner walks bytes; a naive implementation slices mid-character.
|
||||
let line = "playing “Où est le café” from https://h/Items?api_key=abc";
|
||||
let out = redact(line);
|
||||
assert!(!out.contains("abc"), "{out}");
|
||||
assert!(out.contains("café"), "{out}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn server_url_keeps_scheme_and_host_only() {
|
||||
assert_eq!(
|
||||
redact_server_url("https://media.example.com/jellyfin?api_key=abc"),
|
||||
"https://media.example.com"
|
||||
);
|
||||
assert_eq!(
|
||||
redact_server_url("http://192.168.1.10:8096/"),
|
||||
"http://192.168.1.10:8096"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn server_url_drops_embedded_credentials() {
|
||||
// http://user:password@host is a valid URL and has been pasted into
|
||||
// server-address fields before.
|
||||
assert_eq!(
|
||||
redact_server_url("https://duncan:hunter2@media.example.com/"),
|
||||
"https://media.example.com"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn server_url_without_a_scheme_is_refused_rather_than_guessed() {
|
||||
assert_eq!(redact_server_url("media.example.com"), REDACTED);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user