From 7503d88b4f190e64aa20f56cd8f4b371b75a5a26 Mon Sep 17 00:00:00 2001 From: Siddartha Aralakuppe Yogesha Date: Wed, 9 Sep 2026 13:20:53 +0530 Subject: [PATCH 1/2] fix(sdk): drop a promoted column passed as None instead of refusing the event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A promoted key left at None in **fields reached the wire as an explicit JSON null, so _validate_promoted_string refused it. 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 dropped agent_end for every session that succeeded, leaving a dangling agent_start, no outcome, and no evaluation — 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 as a named parameter and written as an explicit null through **fields — same value, two outcomes, decided by which door it came through. Both paths now agree: for a promoted column, no value means no key. Dropped with a warning rather than silently: passing None is still a mistake worth hearing about, it just must not cost the event. Co-Authored-By: Claude Opus 5 (1M context) --- sdk/python/CHANGELOG.md | 28 +++++++++-- sdk/python/failproofai_sdk/_events.py | 20 ++++++++ sdk/python/tests/test_server_contract.py | 60 ++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 4 deletions(-) diff --git a/sdk/python/CHANGELOG.md b/sdk/python/CHANGELOG.md index e6253a17..3e7fe84d 100644 --- a/sdk/python/CHANGELOG.md +++ b/sdk/python/CHANGELOG.md @@ -18,10 +18,30 @@ it ships. ## 0.0.1b2 — 2026-08-25 -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. +### A promoted column passed as `None` no longer costs the event + +- **`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. + + 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. - 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/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 # ───────────────────────────────────────────────────────────────────────────── From 98e654cb0e5792bb4efe6997c49dfa2586750ed2 Mon Sep 17 00:00:00 2001 From: Siddartha Aralakuppe Yogesha Date: Thu, 10 Sep 2026 15:10:24 +0530 Subject: [PATCH 2/2] fix(telemetry): redact SDK batches before upload --- CHANGELOG.md | 9 + crates/failproofaid/src/main.rs | 4 +- crates/fpai-collect/src/uploader.rs | 41 +++- crates/fpai-collect/tests/uploader.rs | 68 +++++- docs/start/integrations/custom-agents.mdx | 24 ++- sdk/python/CHANGELOG.md | 14 +- sdk/python/failproofai_sdk/_redact.py | 201 ++++++++++++++++++ sdk/python/failproofai_sdk/_writer.py | 5 +- .../integrations/llama_index.py | 10 +- .../tests/integrations/test_llama_index.py | 4 +- sdk/python/tests/test_redaction.py | 68 ++++++ sdk/python/tests/test_sdk.py | 52 +++++ sdk/python/tests/test_site_docs.py | 4 +- 13 files changed, 477 insertions(+), 27 deletions(-) create mode 100644 sdk/python/failproofai_sdk/_redact.py create mode 100644 sdk/python/tests/test_redaction.py 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/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..7d9e07fa 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,72 @@ 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"}} +"#, + ) + .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("[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 3e7fe84d..951e1d8f 100644 --- a/sdk/python/CHANGELOG.md +++ b/sdk/python/CHANGELOG.md @@ -11,10 +11,16 @@ 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 + +- 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. ## 0.0.1b2 — 2026-08-25 diff --git a/sdk/python/failproofai_sdk/_redact.py b/sdk/python/failproofai_sdk/_redact.py new file mode 100644 index 00000000..40cc45ad --- /dev/null +++ b/sdk/python/failproofai_sdk/_redact.py @@ -0,0 +1,201 @@ +"""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 + rest = value[start:] + for prefix, minimum, label in _PREFIX_RULES: + if not rest.startswith(prefix): + continue + length = 0 + for char in rest[len(prefix) :]: + if not _is_token_char(char): + break + length += 1 + if length >= minimum: + return len(prefix) + length, label + return None + + +def _match_jwt(value: str, start: int): + if not _at_boundary(value, start) or not value.startswith("eyJ", start): + return None + rest = value[start:] + length = 0 + segments = 0 + while segments < 3: + segment = 0 + for char in rest[length:]: + if not (char.isascii() and (char.isalnum() or char in "-_=")): + break + segment += 1 + if segment == 0: + break + length += segment + segments += 1 + if segments < 3 and length < len(rest) and rest[length] == ".": + length += 1 + elif segments < 3: + break + if segments == 3 and length >= 40: + return length, "jwt" + return None + + +def _match_bearer(value: str, start: int): + rest = value[start:] + if rest[:7].lower() != "bearer ": + return None + token_length = 0 + for char in rest[7:]: + if char.isspace() or char in "\"'": + break + token_length += 1 + token = rest[7 : 7 + token_length] + # The daemon's threshold is bytes; keep short multibyte tokens in parity. + if len(token.encode("utf-8")) >= 8: + return 7 + token_length, "bearer-token" + return None + + +def _match_assignment(value: str, start: int): + if start == 0: + return None + before = value[:start] + rest = value[start:] + if before.endswith("=") and rest.startswith(("\"", "'")): + return None + without_quote = before[:-1] if before[-1:] in ("\"", "'") else before + if not without_quote.endswith("="): + return None + + name_part = without_quote[:-1] + name_len = 0 + for char in reversed(name_part): + if not (char.isascii() and (char.isalnum() or char in "_-")): + break + name_len += 1 + if not name_len: + return None + name = name_part[-name_len:].lower().strip("-") + compound = "_" in name or "-" in name + convincing = any(name.endswith(part) for part in _STRONG_SECRET_NAMES) or ( + compound and any(name.endswith(part) for part in _WEAK_SECRET_NAMES) + ) + if not convincing or rest.startswith(("{", "$", "<", "(", "`")): + return None + + quoted = before[-1:] in ("\"", "'") + length = 0 + for char in rest: + if char in "\"'" or (not quoted and (char.isspace() or char in ";&")): + break + length += 1 + # The daemon measures byte length but advances by bytes; Python advances by + # characters, so use bytes only for the threshold and return characters. + if len(rest[:length].encode("utf-8")) >= _MIN_ASSIGNMENT_VALUE: + return length, "secret-assignment" + return None + + +def scrub_string(value: str) -> tuple[str, int]: + """Return the minimally redacted string and replacement count.""" + out = [] + cursor = 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: + out.append(value[cursor]) + cursor += 1 + continue + length, label = match + out.append(f"[redacted:{label}]") + cursor += length + hits += 1 + return ("".join(out), hits) if hits else (value, 0) + + +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 every string value in one already-valid JSON event.""" + event = json.loads(encoded) + hits = 0 + + def scrub(value): + nonlocal hits + if isinstance(value, str): + value, count = scrub_string(value) + hits += count + return value + if isinstance(value, list): + return [scrub(item) for item in value] + if isinstance(value, dict): + return {key: scrub(item) for key, item in value.items()} + 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..fdacb64e --- /dev/null +++ b/sdk/python/tests/test_redaction.py @@ -0,0 +1,68 @@ +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("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..16300ca9 100644 --- a/sdk/python/tests/test_sdk.py +++ b/sdk/python/tests/test_sdk.py @@ -622,6 +622,58 @@ 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"}, + } + ) + writer.flush_now() + + raw = next((tmp_path / "events").glob("*.jsonl")).read_text() + assert "abcdefghijklmnop" 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_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.