mirror of
https://github.com/nimbold/Firelink.git
synced 2026-08-28 19:47:26 +00:00
fix(security): redact secrets from portable errors (#15)
Sanitize lastError at the portable SQLite boundary using the shared redactor.\nCover spaced credentials and non-HTTP URL queries while preserving standard-mode diagnostics.\n\nRefs #15
This commit is contained in:
+49
-1
@@ -724,6 +724,13 @@ fn remove_persisted_transfer_secrets(value: &mut Value) {
|
|||||||
object.remove(key);
|
object.remove(key);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if let Some(last_error) = object.get("lastError").and_then(Value::as_str) {
|
||||||
|
object.insert(
|
||||||
|
"lastError".to_string(),
|
||||||
|
Value::String(crate::redact_sensitive_text(last_error)),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
if let Some(url) = object.get("url").and_then(Value::as_str) {
|
if let Some(url) = object.get("url").and_then(Value::as_str) {
|
||||||
if let Ok(mut parsed) = url::Url::parse(url) {
|
if let Ok(mut parsed) = url::Url::parse(url) {
|
||||||
let had_userinfo = !parsed.username().is_empty() || parsed.password().is_some();
|
let had_userinfo = !parsed.username().is_empty() || parsed.password().is_some();
|
||||||
@@ -1482,6 +1489,45 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn portable_download_persistence_redacts_error_secrets_but_preserves_safe_errors_and_standard_details() {
|
||||||
|
let temp = TempDir::new().unwrap();
|
||||||
|
let state = init_at_path(temp.path()).unwrap();
|
||||||
|
let mut connection = state.lock().unwrap();
|
||||||
|
let data = json!([
|
||||||
|
{
|
||||||
|
"id": "download-secret-error",
|
||||||
|
"status": "failed",
|
||||||
|
"queueId": "main",
|
||||||
|
"url": "https://example.com/file",
|
||||||
|
"lastError": "HTTP 500 for https://example.com/file?token=PORTABLE_TEST_QUERY_TOKEN"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": "download-safe-error",
|
||||||
|
"status": "failed",
|
||||||
|
"queueId": "main",
|
||||||
|
"url": "https://example.com/other-file",
|
||||||
|
"lastError": "connection refused"
|
||||||
|
}
|
||||||
|
])
|
||||||
|
.to_string();
|
||||||
|
|
||||||
|
replace_downloads(&mut connection, &data, true).unwrap();
|
||||||
|
|
||||||
|
let saved = load_downloads(&connection).unwrap();
|
||||||
|
let secret_error: Value = serde_json::from_str(&saved[0]).unwrap();
|
||||||
|
let safe_error: Value = serde_json::from_str(&saved[1]).unwrap();
|
||||||
|
assert!(!secret_error
|
||||||
|
.to_string()
|
||||||
|
.contains("PORTABLE_TEST_QUERY_TOKEN"));
|
||||||
|
assert_eq!(safe_error["lastError"], "connection refused");
|
||||||
|
|
||||||
|
replace_downloads(&mut connection, &data, false).unwrap();
|
||||||
|
let standard: Value =
|
||||||
|
serde_json::from_str(&load_downloads(&connection).unwrap()[0]).unwrap();
|
||||||
|
assert!(standard.to_string().contains("PORTABLE_TEST_QUERY_TOKEN"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn portable_persistence_redacts_unparseable_download_urls() {
|
fn portable_persistence_redacts_unparseable_download_urls() {
|
||||||
let temp = TempDir::new().unwrap();
|
let temp = TempDir::new().unwrap();
|
||||||
@@ -1511,7 +1557,8 @@ mod tests {
|
|||||||
"id": "download-1",
|
"id": "download-1",
|
||||||
"status": "queued",
|
"status": "queued",
|
||||||
"url": "https://example.com/file",
|
"url": "https://example.com/file",
|
||||||
"password": "secret"
|
"password": "secret",
|
||||||
|
"lastError": "request failed with token=PORTABLE_EXISTING_QUERY_TOKEN"
|
||||||
}])
|
}])
|
||||||
.to_string();
|
.to_string();
|
||||||
replace_downloads(&mut connection, &data, false).unwrap();
|
replace_downloads(&mut connection, &data, false).unwrap();
|
||||||
@@ -1522,6 +1569,7 @@ mod tests {
|
|||||||
let connection = state.lock().unwrap();
|
let connection = state.lock().unwrap();
|
||||||
let saved: Value = serde_json::from_str(&load_downloads(&connection).unwrap()[0]).unwrap();
|
let saved: Value = serde_json::from_str(&load_downloads(&connection).unwrap()[0]).unwrap();
|
||||||
assert!(saved.get("password").is_none());
|
assert!(saved.get("password").is_none());
|
||||||
|
assert!(!saved.to_string().contains("PORTABLE_EXISTING_QUERY_TOKEN"));
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
+21
-3
@@ -4613,13 +4613,15 @@ async fn log_files(app_handle: &tauri::AppHandle) -> Result<Vec<std::path::PathB
|
|||||||
Ok(files)
|
Ok(files)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn redact_log_line(line: &str) -> String {
|
pub(crate) fn redact_sensitive_text(line: &str) -> String {
|
||||||
use std::sync::OnceLock;
|
use std::sync::OnceLock;
|
||||||
static SECRET: OnceLock<regex::Regex> = OnceLock::new();
|
static SECRET: OnceLock<regex::Regex> = OnceLock::new();
|
||||||
static HEADER: OnceLock<regex::Regex> = OnceLock::new();
|
static HEADER: OnceLock<regex::Regex> = OnceLock::new();
|
||||||
static QUERY: OnceLock<regex::Regex> = OnceLock::new();
|
static QUERY: OnceLock<regex::Regex> = OnceLock::new();
|
||||||
let secret = SECRET.get_or_init(|| {
|
let secret = SECRET.get_or_init(|| {
|
||||||
regex::Regex::new(r"(?i)(authorization|cookie|password|token|secret)\s*[:=]\s*([^\s,;]+)")
|
regex::Regex::new(
|
||||||
|
r"(?i)(authorization|cookie|password|token|secret)\s*[:=]\s*([^\r\n,;]+)",
|
||||||
|
)
|
||||||
.expect("valid secret redaction regex")
|
.expect("valid secret redaction regex")
|
||||||
});
|
});
|
||||||
let header = HEADER.get_or_init(|| {
|
let header = HEADER.get_or_init(|| {
|
||||||
@@ -4627,13 +4629,18 @@ fn redact_log_line(line: &str) -> String {
|
|||||||
.expect("valid sensitive header redaction regex")
|
.expect("valid sensitive header redaction regex")
|
||||||
});
|
});
|
||||||
let query = QUERY.get_or_init(|| {
|
let query = QUERY.get_or_init(|| {
|
||||||
regex::Regex::new(r"(https?://[^\s?]+)\?[^\s]+").expect("valid URL query redaction regex")
|
regex::Regex::new(r"([A-Za-z][A-Za-z0-9+.-]*://[^\s?]+)\?[^\s]+")
|
||||||
|
.expect("valid URL query redaction regex")
|
||||||
});
|
});
|
||||||
let redacted = header.replace_all(line, "$1: [redacted]");
|
let redacted = header.replace_all(line, "$1: [redacted]");
|
||||||
let redacted = secret.replace_all(&redacted, "$1=[redacted]");
|
let redacted = secret.replace_all(&redacted, "$1=[redacted]");
|
||||||
query.replace_all(&redacted, "$1?[redacted]").into_owned()
|
query.replace_all(&redacted, "$1?[redacted]").into_owned()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn redact_log_line(line: &str) -> String {
|
||||||
|
redact_sensitive_text(line)
|
||||||
|
}
|
||||||
|
|
||||||
fn redact_log_line_for_output(line: &str) -> String {
|
fn redact_log_line_for_output(line: &str) -> String {
|
||||||
static HOME_PATHS: OnceLock<(String, String)> = OnceLock::new();
|
static HOME_PATHS: OnceLock<(String, String)> = OnceLock::new();
|
||||||
let (home, escaped_home) = HOME_PATHS.get_or_init(|| {
|
let (home, escaped_home) = HOME_PATHS.get_or_init(|| {
|
||||||
@@ -5116,6 +5123,17 @@ mod tests {
|
|||||||
assert!(redacted.contains("[redacted]"));
|
assert!(redacted.contains("[redacted]"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn redacts_spaced_credentials_and_non_http_url_queries() {
|
||||||
|
let line =
|
||||||
|
"token: Bearer spaced secret; ftp://example.com/file?credential=spaced-secret";
|
||||||
|
let redacted = redact_log_line(line);
|
||||||
|
assert!(!redacted.contains("Bearer spaced secret"));
|
||||||
|
assert!(!redacted.contains("credential=spaced-secret"));
|
||||||
|
assert!(redacted.contains("token=[redacted]"));
|
||||||
|
assert!(redacted.contains("ftp://example.com/file?[redacted]"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn redacts_live_log_output_before_webview_delivery() {
|
fn redacts_live_log_output_before_webview_delivery() {
|
||||||
let line = "Cookie: session=abc https://example.com/file?signature=secret";
|
let line = "Cookie: session=abc https://example.com/file?signature=secret";
|
||||||
|
|||||||
Reference in New Issue
Block a user