[failproofai] Preserve events and redact telemetry credentials

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## 1.0.5-beta.0 — 2026-09-12
+
+### Fixes
+
+- `failproofaid` now applies `collector.redact` to externally written SDK spool batches immediately before upload. SDK JSONL files previously bypassed the daemon's redaction path entirely because redaction only ran while the daemon created its own session and hook events. A batch written by an older SDK could therefore send a captured API key verbatim even with the default `minimal` setting. The uploader now scrubs every valid JSON event with the existing deterministic rules, leaves malformed lines untouched for ingest to reject and preserve through the failed-batch path, and still honors `collector.redact: off` (#791)
+
 ## 1.0.4-beta.0 — 2026-09-02
 
 ### Fixes

crates/failproofaid/src/main.rs

@@ -789,7 +789,9 @@ fn collector_tasks() -> Vec<fpai_collect::TaskSpec> {
         ingest.url.clone(),
         ingest.key.clone(),
         cfg.failed_dir.clone(),
-    ) {
+    )
+    .map(|u| u.with_redact(cfg.settings.redact))
+    {
         Ok(u) => std::sync::Arc::new(u),
         Err(err) => {
             eprintln!("[failproofaid] collector disabled: {err}");

crates/fpai-collect/src/redact.rs

@@ -156,7 +156,7 @@ const WEAK_SECRET_NAMES: &[&str] = &["key", "token"];
 /// to be a placeholder or a flag than a credential.
 const MIN_ASSIGNMENT_VALUE: usize = 12;
 
-/// Scrub every string leaf of an event in place.
+/// Scrub credential-shaped object keys and string values in place.
 ///
 /// Returns the number of replacements, so a caller can log that redaction
 /// actually did something without logging what it removed.
@@ -165,20 +165,60 @@ pub fn scrub_value(v: &mut Value, mode: Redact) -> usize {
         return 0;
     }
     let mut n = 0;
-    scrub_in_place(v, &mut n);
+    scrub_in_place(v, None, &mut n);
     n
 }
 
-fn scrub_in_place(v: &mut Value, n: &mut usize) {
+fn is_secret_name(name: &str) -> bool {
+    let raw = name.trim_matches('-');
+    let lower = raw.to_ascii_lowercase();
+    let compound = raw.contains('_')
+        || raw.contains('-')
+        || raw.chars().skip(1).any(|c| c.is_ascii_uppercase());
+    STRONG_SECRET_NAMES.iter().any(|part| lower.ends_with(part))
+        || (compound && WEAK_SECRET_NAMES.iter().any(|part| lower.ends_with(part)))
+}
+
+fn is_literal_secret(value: &str) -> bool {
+    value.len() >= MIN_ASSIGNMENT_VALUE
+        && !value.starts_with(['{', '$', '<', '(', '`'])
+        && !value.starts_with("[redacted:")
+}
+
+fn scrub_in_place(v: &mut Value, field_name: Option<&str>, n: &mut usize) {
     match v {
         Value::String(s) => {
             if let Some(replaced) = scrub_str(s) {
                 *n += replaced.1;
                 *s = replaced.0;
+            } else if field_name.is_some_and(is_secret_name) && is_literal_secret(s) {
+                *n += 1;
+                *s = "[redacted:secret-assignment]".to_string();
+            }
+        }
+        // Elements are more values for the same field, so its name still
+        // decides whether an opaque string among them is a secret.
+        Value::Array(a) => a.iter_mut().for_each(|e| scrub_in_place(e, field_name, n)),
+        Value::Object(o) => {
+            let entries = std::mem::take(o);
+            for (key, mut value) in entries {
+                scrub_in_place(&mut value, Some(&key), n);
+                let redacted_key = match scrub_str(&key) {
+                    Some((key, count)) => {
+                        *n += count;
+                        key
+                    }
+                    None => key,
+                };
+                let mut unique_key = redacted_key.clone();
+                let mut suffix = 2;
+                while o.contains_key(&unique_key) {
+                    unique_key = format!("{redacted_key}#{suffix}");
+                    suffix += 1;
+                }
+                o.insert(unique_key, value);
             }
         }
-        Value::Array(a) => a.iter_mut().for_each(|e| scrub_in_place(e, n)),
-        Value::Object(o) => o.values_mut().for_each(|e| scrub_in_place(e, n)),
         _ => {}
     }
 }
@@ -344,12 +384,8 @@ fn match_assignment(s: &str, i: usize, rest: &str) -> Option<(usize, &'static st
     if name_len == 0 {
         return None;
     }
-    let raw = name_part[name_part.len() - name_len..].to_ascii_lowercase();
-    let name = raw.trim_matches('-');
-    let compound = name.contains('_') || name.contains('-');
-    let convincing = STRONG_SECRET_NAMES.iter().any(|n| name.ends_with(n))
-        || (compound && WEAK_SECRET_NAMES.iter().any(|n| name.ends_with(n)));
-    if !convincing {
+    let name = &name_part[name_part.len() - name_len..];
+    if !is_secret_name(name) {
         return None;
     }
 
@@ -362,15 +398,18 @@ fn match_assignment(s: &str, i: usize, rest: &str) -> Option<(usize, &'static st
 
     // The value runs to the closing quote, or to whitespace / a shell
     // separator when unquoted. The closing quote is left in place.
-    let quoted = matches!(s[..i].chars().next_back(), Some('"') | Some('\''));
+    let quote = match s[..i].chars().next_back() {
+        Some(c @ ('"' | '\'')) => Some(c),
+        _ => None,
+    };
     // Bytes, not characters — see the note on `match_bearer`. This predicate
     // also accepts non-ASCII, so a char count under-reports the span and
     // `scrub_str`'s `i += len` leaves the cursor inside the value.
     let value_len: usize = rest
         .chars()
         .take_while(|c| {
-            if quoted {
-                *c != '"' && *c != '\''
+            if let Some(quote) = quote {
+                *c != quote
             } else {
                 // Quotes end an unquoted value too, matching `match_bearer`'s
                 // token run. An unquoted shell word does not contain a bare
@@ -557,6 +596,16 @@ mod tests {
             scrub(r#"--api-token=abcdefghijklmnop"trailing""#),
             r#"--api-token=[redacted:secret-assignment]"trailing""#
         );
+        // The opposite quote is valid inside a quoted shell value and must not
+        // terminate the secret early.
+        assert_eq!(
+            scrub(r#"PASSWORD='abcdefghijkL"mnopQRST'"#),
+            r#"PASSWORD='[redacted:secret-assignment]'"#
+        );
+        assert_eq!(
+            scrub(r#"PASSWORD="abcdefghijkL'mnopQRST""#),
+            r#"PASSWORD="[redacted:secret-assignment]""#
+        );
     }
 
     /// Redaction must never be a net data loss beyond the secret itself: every
@@ -643,6 +692,61 @@ mod tests {
         assert_eq!(v["type"], "tool_use");
     }
 
+    #[test]
+    fn secret_named_fields_and_credential_shaped_keys_are_scrubbed() {
+        let first = "API_KEY=abcdefghijklmnop";
+        let second = "API_KEY=qrstuvwxyzabcdef";
+        let mut nested = serde_json::Map::new();
+        nested.insert(first.to_string(), json!(1));
+        nested.insert(second.to_string(), json!(2));
+        let mut v = json!({
+            "password": "abcdefghijklmnop",
+            "client_secret": "abcdefghijklmnop",
+            "api_key": "abcdefghijklmnop",
+            "accessToken": "abcdefghijklmnop",
+            "nested": Value::Object(nested),
+        });
+
+        let n = scrub_value(&mut v, Redact::Minimal);
+        assert_eq!(v["password"], "[redacted:secret-assignment]");
+        assert_eq!(v["client_secret"], "[redacted:secret-assignment]");
+        assert_eq!(v["api_key"], "[redacted:secret-assignment]");
+        assert_eq!(v["accessToken"], "[redacted:secret-assignment]");
+        let nested = v["nested"].as_object().unwrap();
+        assert_eq!(nested.len(), 2, "redacted keys must not collapse fields");
+        assert!(!nested.contains_key(first));
+        assert!(!nested.contains_key(second));
+        assert!(n >= 6, "expected fields and keys to be scrubbed, got {n}");
+    }
+
+    /// An array's elements are values of the field that holds it.
+    ///
+    /// The field name used to be dropped on the way into an array, so
+    /// `{"password": ["hunter2hunter2"]}` reached the wire verbatim while the
+    /// same value as a plain string was redacted. Form bodies parsed with
+    /// `parse_qs` and multi-value header maps put every value in a list, so
+    /// this is an ordinary shape for a captured credential, not an exotic one.
+    #[test]
+    fn secret_named_arrays_are_scrubbed() {
+        let mut v = json!({
+            "password": ["abcdefghijklmnop"],
+            "client_secret": ["abcdefghijklmnop", "short", 7, null],
+            "api_key": [["abcdefghijklmnop"]],
+            "access_token": ["abcdefghijklmnop"],
+            "messages": ["an ordinary sentence of text"],
+        });
+
+        let n = scrub_value(&mut v, Redact::Minimal);
+        let marker = "[redacted:secret-assignment]";
+        assert_eq!(v["password"], json!([marker]));
+        assert_eq!(v["client_secret"], json!([marker, "short", 7, null]));
+        assert_eq!(v["api_key"], json!([[marker]]));
+        assert_eq!(v["access_token"], json!([marker]));
+        // The name is what makes a value secret: an ordinary array is untouched.
+        assert_eq!(v["messages"], json!(["an ordinary sentence of text"]));
+        assert_eq!(n, 4);
+    }
+
     #[test]
     fn off_mode_changes_nothing() {
         let mut v = json!({"output": "ghp_abcdefghijklmnopqrstuvwxyz0123"});

crates/fpai-collect/src/uploader.rs

@@ -34,7 +34,9 @@ use std::sync::Arc;
 use std::sync::atomic::{AtomicU64, Ordering};
 use std::time::{Duration, SystemTime, UNIX_EPOCH};
 
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
+
+use crate::config::Redact;
 
 /// Suffix marking a batch that exhausted its retry budget. Deliberately NOT
 /// `.jsonl`, so every directory scan and the watcher skip it for free rather
@@ -165,6 +167,7 @@ pub struct Uploader {
     max_retries: u32,
     retry_base: Duration,
     failed_retries_max: u32,
+    redact: Redact,
     metrics: Arc<UploadMetrics>,
 }
 
@@ -197,10 +200,16 @@ impl Uploader {
             max_retries: DEFAULT_MAX_RETRIES,
             retry_base: DEFAULT_RETRY_BASE,
             failed_retries_max: DEFAULT_FAILED_RETRIES_MAX,
+            redact: Redact::default(),
             metrics: Arc::new(UploadMetrics::default()),
         })
     }
 
+    pub fn with_redact(mut self, redact: Redact) -> Self {
+        self.redact = redact;
+        self
+    }
+
     /// Shorten every delay. Tests only — without it each retry test would wait
     /// out a real multi-second backoff.
     #[doc(hidden)]
@@ -232,6 +241,7 @@ impl Uploader {
             Err(e) => return Err(UploadError::Io(e)),
         };
 
+        let bytes = redact_batch(&bytes, self.redact);
         for chunk in split_lines(&bytes, self.max_upload_bytes) {
             self.post_batch(path, chunk).await?;
         }
@@ -489,6 +499,35 @@ impl Uploader {
     }
 }
 
+fn redact_batch(bytes: &[u8], mode: Redact) -> Vec<u8> {
+    if mode == Redact::Off {
+        return bytes.to_vec();
+    }
+
+    let mut out = Vec::with_capacity(bytes.len());
+    for line in bytes.split_inclusive(|byte| *byte == b'\n') {
+        let (body, newline) = line
+            .strip_suffix(b"\n")
+            .map_or((line, false), |body| (body, true));
+        match serde_json::from_slice::<serde_json::Value>(body) {
+            Ok(mut event) => {
+                if crate::redact::scrub_value(&mut event, mode) > 0 {
+                    event
+                        .serialize(&mut serde_json::Serializer::new(&mut out))
+                        .expect("serializing JSON into Vec cannot fail");
+                } else {
+                    out.extend_from_slice(body);
+                }
+            }
+            Err(_) => out.extend_from_slice(body),
+        }
+        if newline {
+            out.push(b'\n');
+        }
+    }
+    out
+}
+
 /// Retry state carried in a parked batch's filename:
 /// `<base>.a<N>[.c<STATUS>].jsonl[.poison]`.
 ///