diff --git a/CHANGELOG.md b/CHANGELOG.md index 9282faee..bb64664c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ ### 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`. - The two PyPI publish workflows open the next version's `CHANGELOG` section in the same `bump` commit that moves `_version.py`, via a new `scripts/changelog-open.py`. `bump` used to move the version alone, leaving `main` on a version with no section — the exact state `scripts/changelog-section.py` refuses at release time and `__tests__/ci/python-version-pipeline.test.ts` asserts against. Because bump commits carry a skip-ci marker, that never went red on itself: it went red on the next unrelated PR to run CI, which is how `main` broke after the 0.0.1b1 publish (repaired by hand in #755, which named the recurrence and left it) and again after 0.0.1b2. `sdk/python/CHANGELOG.md` gets the 0.0.1b3 section that was missing. The opener is idempotent — a re-run of `bump` against a `main` that already carries the section is a no-op rather than a second heading, which would put only the first one's body on the GitHub Release — and it matches the version with the same trailing word boundary the extractor uses, so opening `0.0.1b1` is not satisfied by an existing `0.0.1b10` (#787) - `fp-cloud-cli`'s Click shim survives typer 0.27.2, which moved `Abort` out of its vendored Click. `_click_compat` wrapped all six vendored imports in one `try: … except ImportError: from click import …`, so that single missing name rebound **every** symbol to pip Click — the exact silent failure the module exists to prevent. Typer catches only its own Click's exceptions, so every typed error escaped uncaught: `fp alerts show ghost` exited 1 with an empty stderr instead of 6 with a message, and the same for exits 2, 3, 4 and 5. 105 tests went red on the dependabot bump that first installed 0.27.2. The Click is now chosen once — on whether `typer._click` exists at all — and each symbol imported from that choice, so a name that goes missing raises at import (a CLI that will not start) rather than silently downgrading every error to exit 1. `Abort` alone is resolved from `typer.Abort`, which tracks the move by construction: pip Click's before typer 0.26, the vendored class through 0.27.1, `typer.exceptions.Abort` from 0.27.2 (#771) diff --git a/crates/failproofaid/src/main.rs b/crates/failproofaid/src/main.rs index 0f88e496..62907aa4 100644 --- a/crates/failproofaid/src/main.rs +++ b/crates/failproofaid/src/main.rs @@ -789,7 +789,9 @@ fn collector_tasks() -> Vec { 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}"); diff --git a/crates/fpai-collect/src/redact.rs b/crates/fpai-collect/src/redact.rs index f955cb00..90b48235 100644 --- a/crates/fpai-collect/src/redact.rs +++ b/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,58 @@ 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(); + } + } + Value::Array(a) => a.iter_mut().for_each(|e| scrub_in_place(e, None, 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 +382,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 +396,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 +594,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 +690,33 @@ 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}"); + } + #[test] fn off_mode_changes_nothing() { let mut v = json!({"output": "ghp_abcdefghijklmnopqrstuvwxyz0123"}); diff --git a/crates/fpai-collect/src/uploader.rs b/crates/fpai-collect/src/uploader.rs index 6ec55046..5d9251e7 100644 --- a/crates/fpai-collect/src/uploader.rs +++ b/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, } @@ -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 { + 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::(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: /// `.a[.c].jsonl[.poison]`. /// diff --git a/crates/fpai-collect/tests/uploader.rs b/crates/fpai-collect/tests/uploader.rs index 1bffbf1e..4a897b57 100644 --- a/crates/fpai-collect/tests/uploader.rs +++ b/crates/fpai-collect/tests/uploader.rs @@ -9,7 +9,7 @@ use std::path::{Path, PathBuf}; use std::sync::atomic::Ordering; use std::time::{Duration, SystemTime, UNIX_EPOCH}; -use fpai_collect::{UploadError, Uploader}; +use fpai_collect::{Redact, UploadError, Uploader}; use wiremock::matchers::{header, method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; @@ -88,6 +88,74 @@ async fn a_2xx_with_an_accepting_ack_deletes_the_batch() { fs::remove_dir_all(&failed).ok(); } +#[tokio::test] +async fn sdk_batches_are_redacted_before_upload() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "accepted": 1, "skipped": 0 + }))) + .mount(&server) + .await; + + let spool = tmpdir("redact-spool"); + let failed = tmpdir("redact-failed"); + let batch = spool.join("event-s-1-0.jsonl"); + fs::write( + &batch, + r#"{"type":"tool_use","input":{"command":"API_KEY=abcdefghijklmnop","password":"qrstuvwxyzabcdef","API_KEY=secretvalue123456":true}} +"#, + ) + .unwrap(); + + uploader(&server, &failed) + .upload_file(&batch) + .await + .unwrap(); + + let requests = server.received_requests().await.unwrap(); + let body = String::from_utf8(requests[0].body.clone()).unwrap(); + assert!( + !body.contains("abcdefghijklmnop"), + "credential reached the wire" + ); + assert!(!body.contains("qrstuvwxyzabcdef")); + assert!(!body.contains("API_KEY=secretvalue123456")); + assert!(body.contains("[redacted:secret-assignment]")); + + fs::remove_dir_all(&spool).ok(); + fs::remove_dir_all(&failed).ok(); +} + +#[tokio::test] +async fn uploader_redaction_can_be_disabled() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "accepted": 1, "skipped": 0 + }))) + .mount(&server) + .await; + + let spool = tmpdir("redact-off-spool"); + let failed = tmpdir("redact-off-failed"); + let batch = spool.join("event-s-1-0.jsonl"); + fs::write(&batch, "{\"output\":\"API_KEY=abcdefghijklmnop\"}\n").unwrap(); + + uploader(&server, &failed) + .with_redact(Redact::Off) + .upload_file(&batch) + .await + .unwrap(); + + let requests = server.received_requests().await.unwrap(); + let body = String::from_utf8(requests[0].body.clone()).unwrap(); + assert!(body.contains("abcdefghijklmnop")); + + fs::remove_dir_all(&spool).ok(); + fs::remove_dir_all(&failed).ok(); +} + #[tokio::test] async fn a_200_that_stored_nothing_is_counted_as_fully_skipped() { // The failure this exists for: a systematically malformed transform gets diff --git a/docs/start/integrations/custom-agents.mdx b/docs/start/integrations/custom-agents.mdx index fd3deb94..778a6970 100644 --- a/docs/start/integrations/custom-agents.mdx +++ b/docs/start/integrations/custom-agents.mdx @@ -643,20 +643,24 @@ Each flush writes one batch file, `.tmp` first, then `fsync`, then an atomic ren The daemon only picks up `.jsonl`, so it can never read a half-written file. The stem carries a timestamp, process id and sequence number, so two processes flushing in the same millisecond cannot collide. The queue is capped at 10,000 events; past that it drops the oldest and logs. - **`collector.redact` does not apply to your SDK events.** It never sees them. + **`collector.redact` defaults to `minimal` for SDK events too.** The SDK + scrubs before writing a batch to disk, and the daemon repeats the same + deterministic pass before upload so batches from older SDKs are protected. -The daemon **ships** your batches. It does not open or rewrite them. +The daemon reads each batch and applies redaction in memory before upload. It +does not rewrite the spool file it read. -| Events | Written by | Redacted by `collector.redact`? | +| Events | Written by | Where minimal redaction runs | | --- | --- | --- | -| CLI session transcripts | The daemon | Yes | -| Hook activity | The daemon | Yes | -| **Everything the SDK emits** | **Your process** | **No** | +| CLI session transcripts | The daemon | Before the daemon writes the batch | +| Hook activity | The daemon | Before the daemon writes the batch | +| **Everything the SDK emits** | **Your process** | **Before the SDK writes the batch and again before daemon upload** | -Redaction runs where the daemon *writes* its own events — not where batches are *shipped*. So a prompt or a tool argument holding an API key still holds it on arrival. - -That is deliberate. These are your own instrumentation calls, and rewriting them in transit would mean the events you receive are not the events you emitted. +Set `collector.redact` to `off` only when verbatim payloads are an explicit +requirement; the SDK and daemon both honor that setting. Minimal redaction +catches common API keys, bearer tokens, JWTs, and secret assignments. It cannot +identify arbitrary sensitive prose. **You control payloads at the source, in two places:** @@ -669,7 +673,7 @@ That is deliberate. These are your own instrumentation calls, and rewriting them `instrument()` drops options an adapter does not read, so passing the wrong name raises nothing and changes nothing. - Don't hand the secret to `input=` in the first place. - `collector.redact` is not a substitute for either. + `collector.redact` is defence in depth, not a substitute for either. diff --git a/sdk/python/CHANGELOG.md b/sdk/python/CHANGELOG.md index e6253a17..f320c16d 100644 --- a/sdk/python/CHANGELOG.md +++ b/sdk/python/CHANGELOG.md @@ -11,17 +11,40 @@ see `scripts/changelog-section.py`. ## 0.0.1b3 — 2026-09-08 -Open for the next release. `0.0.1b2` published on 2026-09-08 and the `bump` job -moved the version here automatically; nothing has landed against `0.0.1b3` yet. -Add entries as changes merge — this section becomes the GitHub Release body when -it ships. +### Fixes -## 0.0.1b2 — 2026-08-25 +- Redact common credential shapes before SDK events reach the on-disk spool. + The daemon already exposes `collector.redact`, but SDK-written batches could + contain raw API keys, bearer tokens, JWTs, or secret assignments at rest until + upload. The SDK now applies the same deterministic minimal rules before each + JSONL write, defaults safely to redaction when configuration is absent or + malformed, and honors `collector.redact: off` when verbatim capture is + explicitly required. The daemon repeats the pass before upload as defence in + depth for batches written by older SDK versions. +- **`None` on a promoted column is now dropped and warned about, not refused.** + A promoted key left at `None` in `**fields` reached the wire as an explicit + JSON `null`, so `_validate_promoted_string` refused it outright. But `None` is + how a caller says *I have no value*, and the refusal landed inside their emit + helper — which swallows telemetry errors, because telemetry must not break a + run. The event vanished with nothing logged. + + `agent_end(error_type=None)` is the shape **every successful run** produces: + `error_type` is populated only on a failing outcome. Found against a real + multi-agent app, where it silently dropped `agent_end` for every session that + succeeded — leaving each one with a dangling `agent_start`, no outcome, and no + evaluation, since the server triggers evaluation on `agent_end`. + +- **The same fix closes the mirror bug on promoted numerics.** `_build` omits + `None` only from a dataclass's named `specifics`; `extra` is merged verbatim. + So `duration_ms=None` was dropped when passed as a named parameter and written + as an explicit `null` when passed through `**fields` — the same value, two + outcomes, decided by which door it came through. -Open for the next release. `0.0.1b1` published on 2026-08-24 and the `bump` job -moved the version here automatically; nothing has landed against `0.0.1b2` yet. -Add entries as changes merge — this section becomes the GitHub Release body when -it ships. + Both paths now agree: for a promoted column, no value means no key. Nothing + that worked before changes, and no explicit `null` reaches a promoted column + from either direction. + +## 0.0.1b2 — 2026-08-25 - Retire the old inbound evaluator boundary and add evaluator authoring plus the outbound-only v2 worker runtime under the lazy `failproofai_sdk.evaluator` diff --git a/sdk/python/failproofai_sdk/_events.py b/sdk/python/failproofai_sdk/_events.py index 52bf4742..0f8ade53 100644 --- a/sdk/python/failproofai_sdk/_events.py +++ b/sdk/python/failproofai_sdk/_events.py @@ -99,6 +99,11 @@ def _validate_promoted_string(name: str, value) -> None: own call. `_build` copies the base dict verbatim, so a `None` here reached the wire as an explicit JSON `null`, the row was accepted at 200 OK, and the column was empty for some events and not others with nothing logged anywhere. + + `**fields` no longer reaches this holding a `None`: `_validate_fields` drops + the key and warns, so an optional column the caller simply does not have + costs a log line rather than the whole event. The raise below stays as the + backstop for any direct caller. """ if value is None: raise ValueError( @@ -320,6 +325,21 @@ def _validate_fields(self, fields: dict) -> None: bad = _RESERVED & fields.keys() if bad: raise ValueError(f"Reserved field names cannot be used as custom fields: {sorted(bad)}") + # `_build` omits None only from a dataclass's named `specifics`; `extra` + # is merged verbatim, so a promoted key left at None reaches the wire as + # an explicit JSON null — accepted at 200 OK, stored as NULL, invisible + # to every filter on that column. For a promoted column "no value" has to + # mean "no key", so drop it here, the one place holding the caller's own + # dict. Warned rather than silent: passing None is still a mistake worth + # hearing about, it just must not cost the event. + for name in (_PROMOTED_NUMERIC | _PROMOTED_STRING) & fields.keys(): + if fields[name] is None: + logger.warning( + "%s was passed as None and has been omitted from the event; " + "pass a value, or omit the argument entirely to silence this.", + name, + ) + del fields[name] for name in _PROMOTED_NUMERIC & fields.keys(): _validate_promoted_numeric(name, fields[name]) for name in _PROMOTED_STRING & fields.keys(): diff --git a/sdk/python/failproofai_sdk/_redact.py b/sdk/python/failproofai_sdk/_redact.py new file mode 100644 index 00000000..2ca7ca13 --- /dev/null +++ b/sdk/python/failproofai_sdk/_redact.py @@ -0,0 +1,232 @@ +"""Deterministic credential scrubbing for SDK spool files. + +This mirrors the daemon's minimal redaction boundary. The SDK applies it +before bytes reach disk; the daemon applies it again before upload so batches +written by older SDKs receive the same protection. +""" + +import json +import os +from pathlib import Path + +_PREFIX_RULES = ( + ("sk-ant-api", 16, "anthropic-key"), + ("sk-ant-", 16, "anthropic-key"), + ("sk-proj-", 16, "openai-key"), + ("sk-", 16, "api-key"), + ("ghp_", 20, "github-token"), + ("gho_", 20, "github-token"), + ("ghu_", 20, "github-token"), + ("ghs_", 20, "github-token"), + ("ghr_", 20, "github-token"), + ("github_pat_", 20, "github-token"), + ("sb_secret_", 16, "supabase-key"), + ("sbp_", 20, "supabase-key"), + ("xoxb-", 16, "slack-token"), + ("xoxp-", 16, "slack-token"), + ("AKIA", 16, "aws-access-key-id"), + ("ASIA", 16, "aws-access-key-id"), +) +_STRONG_SECRET_NAMES = ("secret", "password", "passwd", "credential") +_WEAK_SECRET_NAMES = ("key", "token") +_MIN_ASSIGNMENT_VALUE = 12 + + +def _is_token_char(char: str) -> bool: + return char.isascii() and (char.isalnum() or char in "_-") + + +def _at_boundary(value: str, start: int) -> bool: + return start == 0 or not _is_token_char(value[start - 1]) + + +def _match_prefix(value: str, start: int): + if not _at_boundary(value, start): + return None + for prefix, minimum, label in _PREFIX_RULES: + if not value.startswith(prefix, start): + continue + end = start + len(prefix) + while end < len(value) and _is_token_char(value[end]): + end += 1 + if end - start - len(prefix) >= minimum: + return end - start, label + return None + + +def _match_jwt(value: str, start: int): + if not _at_boundary(value, start) or not value.startswith("eyJ", start): + return None + end = start + segments = 0 + while segments < 3: + segment_start = end + while end < len(value): + char = value[end] + if not (char.isascii() and (char.isalnum() or char in "-_=")): + break + end += 1 + if end == segment_start: + break + segments += 1 + if segments < 3 and end < len(value) and value[end] == ".": + end += 1 + elif segments < 3: + break + length = end - start + if segments == 3 and length >= 40: + return length, "jwt" + return None + + +def _match_bearer(value: str, start: int): + if value[start : start + 7].lower() != "bearer ": + return None + end = start + 7 + token_bytes = 0 + while end < len(value): + char = value[end] + if char.isspace() or char in "\"'": + break + token_bytes += len(char.encode("utf-8")) + end += 1 + if token_bytes >= 8: + return end - start, "bearer-token" + return None + + +def _is_secret_name(name: str) -> bool: + raw = name.strip("-") + lowered = raw.lower() + compound = "_" in raw or "-" in raw or any(char.isupper() for char in raw[1:]) + return any(lowered.endswith(part) for part in _STRONG_SECRET_NAMES) or ( + compound and any(lowered.endswith(part) for part in _WEAK_SECRET_NAMES) + ) + + +def _is_literal_secret(value: str) -> bool: + return ( + len(value.encode("utf-8")) >= _MIN_ASSIGNMENT_VALUE + and not value.startswith(("{", "$", "<", "(", "`", "[redacted:")) + ) + + +def _match_assignment(value: str, start: int): + if start == 0: + return None + if value[start - 1] == "=" and value[start] in "\"'": + return None + + quote = value[start - 1] if value[start - 1] in "\"'" else None + equals = start - 2 if quote else start - 1 + if equals < 0 or value[equals] != "=": + return None + + name_start = equals + while name_start > 0: + char = value[name_start - 1] + if not (char.isascii() and (char.isalnum() or char in "_-")): + break + name_start -= 1 + if name_start == equals: + return None + if not _is_secret_name(value[name_start:equals]) or value.startswith( + ("{", "$", "<", "(", "`"), start + ): + return None + + end = start + value_bytes = 0 + while end < len(value): + char = value[end] + if (quote and char == quote) or ( + not quote and (char.isspace() or char in ";&\"'") + ): + break + value_bytes += len(char.encode("utf-8")) + end += 1 + if value_bytes >= _MIN_ASSIGNMENT_VALUE: + return end - start, "secret-assignment" + return None + + +def scrub_string(value: str) -> tuple[str, int]: + """Return the minimally redacted string and replacement count.""" + out = [] + cursor = 0 + copied_through = 0 + hits = 0 + while cursor < len(value): + match = ( + _match_prefix(value, cursor) + or _match_jwt(value, cursor) + or _match_bearer(value, cursor) + or _match_assignment(value, cursor) + ) + if match is None: + cursor += 1 + continue + length, label = match + out.append(value[copied_through:cursor]) + out.append(f"[redacted:{label}]") + cursor += length + copied_through = cursor + hits += 1 + if not hits: + return value, 0 + out.append(value[copied_through:]) + return "".join(out), hits + + +def redaction_enabled(base_dir: Path) -> bool: + """Read the daemon's redaction switch, defaulting safely to minimal.""" + configured_home = os.environ.get("FAILPROOFAI_HOME") + if base_dir.name == "custom-agents": + config_path = base_dir.parent / "config.json" + elif configured_home: + config_path = Path(configured_home) / "config.json" + else: + config_path = Path.home() / ".failproofai" / "config.json" + try: + config = json.loads(config_path.read_text(encoding="utf-8")) + if not isinstance(config, dict): + return True + collector = config.get("collector") + return not isinstance(collector, dict) or collector.get("redact") != "off" + except (OSError, TypeError, ValueError): + return True + + +def redact_json_line(encoded: str) -> str: + """Redact credential-shaped keys and string values in a valid JSON event.""" + event = json.loads(encoded) + hits = 0 + + def scrub(value, field_name=None): + nonlocal hits + if isinstance(value, str): + value, count = scrub_string(value) + hits += count + if count == 0 and isinstance(field_name, str) and _is_secret_name(field_name): + if _is_literal_secret(value): + hits += 1 + return "[redacted:secret-assignment]" + return value + if isinstance(value, list): + return [scrub(item) for item in value] + if isinstance(value, dict): + result = {} + for key, item in value.items(): + redacted_key, count = scrub_string(key) + hits += count + unique_key = redacted_key + suffix = 2 + while unique_key in result: + unique_key = f"{redacted_key}#{suffix}" + suffix += 1 + result[unique_key] = scrub(item, key) + return result + return value + + redacted = scrub(event) + return json.dumps(redacted) if hits else encoded diff --git a/sdk/python/failproofai_sdk/_writer.py b/sdk/python/failproofai_sdk/_writer.py index 3b1d5b6e..e4197ed2 100644 --- a/sdk/python/failproofai_sdk/_writer.py +++ b/sdk/python/failproofai_sdk/_writer.py @@ -9,9 +9,9 @@ import weakref from datetime import datetime, timezone +from failproofai_sdk._redact import redact_json_line, redaction_enabled from failproofai_sdk._resolver import get_base_dir - logger = logging.getLogger(__name__) #: Per-process batch counter, so two batches written inside the same millisecond @@ -638,11 +638,14 @@ def _write_batch(self, entries: list[dict]) -> None: # whole batch goes back on the queue to be retried. lines = [] dropped = 0 + redact = redaction_enabled(get_base_dir()) for entry in entries: encoded = _encode_entry(entry) if encoded is None: dropped += 1 continue + if redact: + encoded = redact_json_line(encoded) lines.append(encoded) if dropped: diff --git a/sdk/python/failproofai_sdk/integrations/llama_index.py b/sdk/python/failproofai_sdk/integrations/llama_index.py index 0f82796e..827015b8 100644 --- a/sdk/python/failproofai_sdk/integrations/llama_index.py +++ b/sdk/python/failproofai_sdk/integrations/llama_index.py @@ -498,11 +498,11 @@ def capture(self, value, limit: int | None = None): recorded, so the setting looked like it had worked. That is the switch `docs/start/integrations/llamaindex.mdx` presents as - the control for regulated data, and `collector.redact` explicitly does - not apply to SDK events — so there is no second line of defence behind - it. The sibling adapters route every payload through one helper - (LangChain's `_shrink`, Pydantic AI's `capture_content` checks); this is - that helper. + the control for regulated data. Minimal credential redaction is defence + in depth, not a replacement for disabling content capture: it catches + known secret shapes, not arbitrary regulated content. The sibling + adapters route every payload through one helper (LangChain's `_shrink`, + Pydantic AI's `capture_content` checks); this is that helper. """ if not self.capture_messages: return None diff --git a/sdk/python/tests/integrations/test_llama_index.py b/sdk/python/tests/integrations/test_llama_index.py index bbdb8280..50361f21 100644 --- a/sdk/python/tests/integrations/test_llama_index.py +++ b/sdk/python/tests/integrations/test_llama_index.py @@ -1524,8 +1524,8 @@ def test_capture_messages_off_records_no_payload_anywhere( answers, the arguments and the outputs did not. `docs/start/integrations/llamaindex.mdx` presents this as the control for - regulated data, and `collector.redact` explicitly does not apply to SDK - events, so there was no second line of defence behind it. + regulated data. Minimal credential redaction cannot replace it: arbitrary + prompts and completions do not necessarily look like credentials. """ llm = StubLLM(script=[("add", {"a": 987654321, "b": 123456789})], final="SECRET-COMPLETION") run_agent(calculator(llm), "SECRET-PROMPT: add them") diff --git a/sdk/python/tests/test_redaction.py b/sdk/python/tests/test_redaction.py new file mode 100644 index 00000000..6f0f977d --- /dev/null +++ b/sdk/python/tests/test_redaction.py @@ -0,0 +1,117 @@ +import json + +import pytest + +from failproofai_sdk._redact import redact_json_line, redaction_enabled, scrub_string + + +@pytest.mark.parametrize( + ("raw", "marker"), + [ + ("sk-ant-api03-AbCdEfGhIjKlMnOpQrStUv", "anthropic-key"), + ("sk-proj-abcdefghijklmnopqrstuvwxyz", "openai-key"), + ("ghp_abcdefghijklmnopqrstuvwxyz0123", "github-token"), + ("AKIAIOSFODNN7EXAMPLE0000", "aws-access-key-id"), + ( + "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.sIgNaTuRe0123456789ab", + "jwt", + ), + ("Authorization: Bearer tok-🔑🔑🔑-SECRETTAIL", "bearer-token"), + ("Authorization: Bearer 🔑🔑", "bearer-token"), + ("API_TOKEN=пароль-очень-ENDOFSECRET", "secret-assignment"), + ("API_TOKEN=密码密码密码", "secret-assignment"), + ], +) +def test_minimal_redaction_matches_daemon_secret_shapes(raw, marker): + scrubbed, count = scrub_string(raw) + assert count == 1 + assert scrubbed == f"[redacted:{marker}]" or scrubbed.endswith(f"[redacted:{marker}]") + assert "SECRETTAIL" not in scrubbed + assert "ENDOFSECRET" not in scrubbed + + +@pytest.mark.parametrize( + "value", + [ + "this is a risk-averse approach", + "AWS_REGION=us-east-1", + "key=someLongIdentifier", + "api_key=$OPENAI_API_KEY", + "--token=", + ], +) +def test_minimal_redaction_leaves_known_false_positives_alone(value): + assert scrub_string(value) == (value, 0) + + +def test_redaction_preserves_json_structure_and_is_deterministic(): + encoded = json.dumps( + { + "type": "tool_use", + "input": {"command": 'API_KEY="abcdefghijklmnop" && echo done'}, + "nested": [{"output": "ghp_abcdefghijklmnopqrstuvwxyz0123"}], + } + ) + first = redact_json_line(encoded) + second = redact_json_line(encoded) + assert first == second + event = json.loads(first) + assert event["type"] == "tool_use" + assert event["input"]["command"] == 'API_KEY="[redacted:secret-assignment]" && echo done' + assert event["nested"][0]["output"] == "[redacted:github-token]" + + +@pytest.mark.parametrize( + "field", + ["password", "client_secret", "api_key", "access_token", "apiKey", "accessToken"], +) +def test_secret_named_fields_redact_opaque_values(field): + encoded = json.dumps({"nested": {field: "abcdefghijklmnop"}}) + event = json.loads(redact_json_line(encoded)) + assert event["nested"][field] == "[redacted:secret-assignment]" + + +def test_credential_shaped_dictionary_keys_are_redacted_without_colliding(): + first = "API_KEY=abcdefghijklmnop" + second = "API_KEY=qrstuvwxyzabcdef" + event = json.loads(redact_json_line(json.dumps({"nested": {first: 1, second: 2}}))) + keys = list(event["nested"]) + assert first not in keys + assert second not in keys + assert len(keys) == 2 + assert sorted(event["nested"].values()) == [1, 2] + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + ( + "PASSWORD='abcdefghijkL\"mnopQRST'", + "PASSWORD='[redacted:secret-assignment]'", + ), + ( + 'PASSWORD="abcdefghijkL\'mnopQRST"', + 'PASSWORD="[redacted:secret-assignment]"', + ), + ], +) +def test_quoted_assignment_stops_only_at_its_matching_quote(raw, expected): + assert scrub_string(raw) == (expected, 1) + + +def test_plain_string_scan_does_not_copy_every_remaining_suffix(): + class NoTailSlices(str): + def __getitem__(self, item): + if isinstance(item, slice) and item.stop is None and (item.start or 0) > 0: + raise AssertionError(f"copied the remaining tail at {item.start}") + return super().__getitem__(item) + + value = NoTailSlices("ordinary payload text " * 100) + assert scrub_string(value) == (value, 0) + + +@pytest.mark.parametrize("config", [None, [], {"collector": None}, {"collector": "minimal"}]) +def test_malformed_redaction_config_fails_closed(tmp_path, config): + (tmp_path / "config.json").write_text(json.dumps(config), encoding="utf-8") + + assert redaction_enabled(tmp_path / "custom-agents") is True diff --git a/sdk/python/tests/test_sdk.py b/sdk/python/tests/test_sdk.py index 8f5f837d..af43cea5 100644 --- a/sdk/python/tests/test_sdk.py +++ b/sdk/python/tests/test_sdk.py @@ -622,6 +622,64 @@ def test_writer_multiple_events_in_one_file(tmp_path): resolver.set_base_dir(original) +def test_writer_redacts_credentials_before_the_spool_reaches_disk(tmp_path): + import failproofai_sdk._resolver as resolver + original = resolver._base_dir + try: + resolver.set_base_dir(tmp_path) + writer = EventWriter(flush_interval=60) + writer.submit( + { + "timestamp": "t", + "session_id": "s1", + "agent_id": "a1", + "type": "tool_use", + "input": { + "command": "API_KEY=abcdefghijklmnop", + "password": "qrstuvwxyzabcdef", + "API_KEY=secretvalue123456": True, + }, + } + ) + writer.flush_now() + + raw = next((tmp_path / "events").glob("*.jsonl")).read_text() + assert "abcdefghijklmnop" not in raw + assert "qrstuvwxyzabcdef" not in raw + assert "API_KEY=secretvalue123456" not in raw + assert "API_KEY=[redacted:secret-assignment]" in raw + finally: + resolver.set_base_dir(original) + + +def test_writer_honours_collector_redact_off(tmp_path): + import failproofai_sdk._resolver as resolver + original = resolver._base_dir + try: + base_dir = tmp_path / "custom-agents" + (tmp_path / "config.json").write_text( + json.dumps({"collector": {"redact": "off"}}), encoding="utf-8" + ) + resolver.set_base_dir(base_dir) + writer = EventWriter(flush_interval=60) + writer.submit( + { + "timestamp": "t", + "session_id": "s1", + "agent_id": "a1", + "type": "tool_use", + "input": {"command": "API_KEY=abcdefghijklmnop"}, + } + ) + writer.flush_now() + + raw = next((base_dir / "events").glob("*.jsonl")).read_text() + assert "API_KEY=abcdefghijklmnop" in raw + assert "[redacted:" not in raw + finally: + resolver.set_base_dir(original) + + def test_writer_coerces_unserializable_payload_values(tmp_path): import failproofai_sdk._resolver as resolver original = resolver._base_dir diff --git a/sdk/python/tests/test_server_contract.py b/sdk/python/tests/test_server_contract.py index 3096b7df..794404db 100644 --- a/sdk/python/tests/test_server_contract.py +++ b/sdk/python/tests/test_server_contract.py @@ -427,6 +427,66 @@ def test_the_promoted_numeric_set_matches_the_one_ingest_lifts(): assert _events._PROMOTED_NUMERIC == PROMOTED_NUMERIC +# ───────────────────────────────────────────────────────────────────────────── +# A promoted column the caller does not have costs a warning, not the event +# ───────────────────────────────────────────────────────────────────────────── + + +@pytest.mark.parametrize("name", sorted(_events._PROMOTED_STRING)) +def test_a_promoted_string_passed_as_none_is_omitted_not_refused(name): + """None means "I have no value", and that must not cost the whole event. + + `agent_end(error_type=None)` is the shape every successful run produces: the + field is populated only on a failing outcome, so the ordinary success path + passes None. Refusing it raised inside the caller's emit helper, and a helper + that swallows telemetry errors — which every one of them does, because + telemetry must not break a run — turned that into a silently missing event. + """ + recorder = _Recorder() + EventNamespace(recorder).agent_start(session_id="s", agent_id="a", **{name: None}) + assert name not in recorder.entries[0] + + +@pytest.mark.parametrize("name", sorted(_events._PROMOTED_NUMERIC)) +def test_a_promoted_numeric_passed_as_none_in_fields_is_omitted_too(name): + """The named-parameter path already dropped None; `**fields` did not. + + `_build` omits None only from a dataclass's own `specifics` — `extra` is + merged verbatim — so the same value went to the wire as an explicit JSON + null depending only on which door it came through. + """ + recorder = _Recorder() + EventNamespace(recorder).agent_start(session_id="s", agent_id="a", **{name: None}) + assert name not in recorder.entries[0] + + +def test_a_dropped_promoted_column_is_warned_about(caplog): + """Silently dropping it would hide a real mistake; refusing it costs the event.""" + recorder = _Recorder() + with caplog.at_level("WARNING", logger="failproofai_sdk._events"): + EventNamespace(recorder).agent_end(session_id="s", agent_id="a", error_type=None) + assert "error_type" in caplog.text + + +def test_agent_end_on_a_successful_run_is_still_emitted(): + """The regression this all exists for, in the shape the caller actually sends.""" + recorder = _Recorder() + EventNamespace(recorder).agent_end( + session_id="s", agent_id="a", outcome="success", summary=None, error_type=None + ) + assert [e["type"] for e in recorder.entries] == ["agent_end"] + assert recorder.entries[0]["outcome"] == "success" + assert "error_type" not in recorder.entries[0] + + +@pytest.mark.parametrize("name", sorted(_events._PROMOTED_STRING)) +def test_a_promoted_string_with_a_real_value_still_goes_through(name): + """Dropping None must not also drop the values that matter.""" + recorder = _Recorder() + EventNamespace(recorder).agent_start(session_id="s", agent_id="a", **{name: "real"}) + assert recorder.entries[0][name] == "real" + + # ───────────────────────────────────────────────────────────────────────────── # The MEASURED duration is bound by the same u32 range a caller is held to # ───────────────────────────────────────────────────────────────────────────── diff --git a/sdk/python/tests/test_site_docs.py b/sdk/python/tests/test_site_docs.py index abd928d3..2a467878 100644 --- a/sdk/python/tests/test_site_docs.py +++ b/sdk/python/tests/test_site_docs.py @@ -315,8 +315,8 @@ def test_no_cross_adapter_page_presents_one_adapters_option_as_universal(): and CrewAI has none — and `instrument()` drops options an adapter does not read, so `instrument("crewai", capture_content=False)` raised nothing and changed nothing. A reader on regulated data shipped believing prompts and - completions had stopped being recorded, and `collector.redact` explicitly - does not apply to SDK events, so nothing was behind it. + completions had stopped being recorded. Minimal credential redaction is not + a substitute: arbitrary regulated content need not resemble a secret. The four per-framework pages are checked elsewhere; these are the pages that speak about all of them at once and so must name the difference.