From 81fb01e511b087c1da86137891a653f26613016d Mon Sep 17 00:00:00 2001 From: Siddartha Aralakuppe Yogesha Date: Wed, 9 Sep 2026 13:20:53 +0530 Subject: [PATCH 1/5] 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 d1ab194ae9fd7714f22cec4c88d81d62a6c8ddaa Mon Sep 17 00:00:00 2001 From: Siddartha Aralakuppe Yogesha Date: Thu, 10 Sep 2026 15:10:24 +0530 Subject: [PATCH 2/5] 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. From 5b55de24e5b483bde77f21cdbd9d835a0bc7a636 Mon Sep 17 00:00:00 2001 From: Siddartha Aralakuppe Yogesha Date: Sat, 12 Sep 2026 01:07:13 +0530 Subject: [PATCH 3/5] fix(telemetry): close redaction gaps --- crates/fpai-collect/src/redact.rs | 102 +++++++++++++++--- crates/fpai-collect/tests/uploader.rs | 4 +- sdk/python/CHANGELOG.md | 7 +- sdk/python/failproofai_sdk/_redact.py | 145 ++++++++++++++++---------- sdk/python/tests/test_redaction.py | 49 +++++++++ sdk/python/tests/test_sdk.py | 8 +- 6 files changed, 237 insertions(+), 78 deletions(-) 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/tests/uploader.rs b/crates/fpai-collect/tests/uploader.rs index 7d9e07fa..4a897b57 100644 --- a/crates/fpai-collect/tests/uploader.rs +++ b/crates/fpai-collect/tests/uploader.rs @@ -103,7 +103,7 @@ async fn sdk_batches_are_redacted_before_upload() { let batch = spool.join("event-s-1-0.jsonl"); fs::write( &batch, - r#"{"type":"tool_use","input":{"command":"API_KEY=abcdefghijklmnop"}} + r#"{"type":"tool_use","input":{"command":"API_KEY=abcdefghijklmnop","password":"qrstuvwxyzabcdef","API_KEY=secretvalue123456":true}} "#, ) .unwrap(); @@ -119,6 +119,8 @@ async fn sdk_batches_are_redacted_before_upload() { !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(); diff --git a/sdk/python/CHANGELOG.md b/sdk/python/CHANGELOG.md index 951e1d8f..f320c16d 100644 --- a/sdk/python/CHANGELOG.md +++ b/sdk/python/CHANGELOG.md @@ -21,11 +21,6 @@ see `scripts/changelog-section.py`. 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 - -### 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 @@ -49,6 +44,8 @@ see `scripts/changelog-section.py`. 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` namespace. diff --git a/sdk/python/failproofai_sdk/_redact.py b/sdk/python/failproofai_sdk/_redact.py index 40cc45ad..2ca7ca13 100644 --- a/sdk/python/failproofai_sdk/_redact.py +++ b/sdk/python/failproofai_sdk/_redact.py @@ -43,98 +43,110 @@ def _at_boundary(value: str, start: int) -> bool: 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): + if not value.startswith(prefix, start): 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 + 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 - rest = value[start:] - length = 0 + end = start segments = 0 while segments < 3: - segment = 0 - for char in rest[length:]: + segment_start = end + while end < len(value): + char = value[end] if not (char.isascii() and (char.isalnum() or char in "-_=")): break - segment += 1 - if segment == 0: + end += 1 + if end == segment_start: break - length += segment segments += 1 - if segments < 3 and length < len(rest) and rest[length] == ".": - length += 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): - rest = value[start:] - if rest[:7].lower() != "bearer ": + if value[start : start + 7].lower() != "bearer ": return None - token_length = 0 - for char in rest[7:]: + end = start + 7 + token_bytes = 0 + while end < len(value): + char = value[end] 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" + 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 - before = value[:start] - rest = value[start:] - if before.endswith("=") and rest.startswith(("\"", "'")): + if value[start - 1] == "=" and value[start] in "\"'": return None - without_quote = before[:-1] if before[-1:] in ("\"", "'") else before - if not without_quote.endswith("="): + + 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_part = without_quote[:-1] - name_len = 0 - for char in reversed(name_part): + name_start = equals + while name_start > 0: + char = value[name_start - 1] if not (char.isascii() and (char.isalnum() or char in "_-")): break - name_len += 1 - if not name_len: + name_start -= 1 + if name_start == equals: 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(("{", "$", "<", "(", "`")): + if not _is_secret_name(value[name_start:equals]) or value.startswith( + ("{", "$", "<", "(", "`"), start + ): return None - quoted = before[-1:] in ("\"", "'") - length = 0 - for char in rest: - if char in "\"'" or (not quoted and (char.isspace() or char in ";&")): + 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 - 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" + value_bytes += len(char.encode("utf-8")) + end += 1 + if value_bytes >= _MIN_ASSIGNMENT_VALUE: + return end - start, "secret-assignment" return None @@ -142,6 +154,7 @@ 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 = ( @@ -151,14 +164,18 @@ def scrub_string(value: str) -> tuple[str, int]: or _match_assignment(value, cursor) ) if match is None: - out.append(value[cursor]) cursor += 1 continue length, label = match + out.append(value[copied_through:cursor]) out.append(f"[redacted:{label}]") cursor += length + copied_through = cursor hits += 1 - return ("".join(out), hits) if hits else (value, 0) + if not hits: + return value, 0 + out.append(value[copied_through:]) + return "".join(out), hits def redaction_enabled(base_dir: Path) -> bool: @@ -181,20 +198,34 @@ def redaction_enabled(base_dir: Path) -> bool: def redact_json_line(encoded: str) -> str: - """Redact every string value in one already-valid JSON event.""" + """Redact credential-shaped keys and string values in a valid JSON event.""" event = json.loads(encoded) hits = 0 - def scrub(value): + 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): - return {key: scrub(item) for key, item in value.items()} + 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) diff --git a/sdk/python/tests/test_redaction.py b/sdk/python/tests/test_redaction.py index fdacb64e..6f0f977d 100644 --- a/sdk/python/tests/test_redaction.py +++ b/sdk/python/tests/test_redaction.py @@ -61,6 +61,55 @@ def test_redaction_preserves_json_structure_and_is_deterministic(): 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") diff --git a/sdk/python/tests/test_sdk.py b/sdk/python/tests/test_sdk.py index 16300ca9..af43cea5 100644 --- a/sdk/python/tests/test_sdk.py +++ b/sdk/python/tests/test_sdk.py @@ -634,13 +634,19 @@ def test_writer_redacts_credentials_before_the_spool_reaches_disk(tmp_path): "session_id": "s1", "agent_id": "a1", "type": "tool_use", - "input": {"command": "API_KEY=abcdefghijklmnop"}, + "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) From 2563c8364ffe74cd3c1acf580bdf20a70ed99422 Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Sat, 12 Sep 2026 15:15:27 +0530 Subject: [PATCH 4/5] docs(changelog): move the SDK-batch redaction entry to 1.0.5-beta.0 1.0.4 shipped before this merged and main moved to 1.0.5-beta.0, so the entry was sitting under a release it is not part of. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MT7r7rUgssiLQcq6D9M6vF --- CHANGELOG.md | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb64664c..e5036129 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,18 +1,15 @@ # 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 -- `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) From 5359f13588f3e861605e665573cb4f05a968e58d Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Sat, 12 Sep 2026 15:52:30 +0530 Subject: [PATCH 5/5] fix(telemetry): redact opaque secrets inside secret-named arrays Both redactors dropped the parent field name when recursing into an array, so {"password": ["hunter2hunter2"]} reached the SDK spool and the upload 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. Array elements now inherit the field name in the Python SDK and in the daemon, whose pass is the only one a batch from an older SDK ever gets. Regression tests cover arrays (including nested ones) under password, client_secret, api_key and access_token in the redactor units, the SDK spool writer, and the daemon uploader. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TUAqtRvddQeVW7443bLz2E --- crates/fpai-collect/src/redact.rs | 32 +++++++++++++++++++++- crates/fpai-collect/tests/uploader.rs | 39 +++++++++++++++++++++++++++ sdk/python/failproofai_sdk/_redact.py | 3 ++- sdk/python/tests/test_redaction.py | 23 ++++++++++++++++ sdk/python/tests/test_sdk.py | 29 ++++++++++++++++++++ 5 files changed, 124 insertions(+), 2 deletions(-) diff --git a/crates/fpai-collect/src/redact.rs b/crates/fpai-collect/src/redact.rs index 90b48235..5dc352ce 100644 --- a/crates/fpai-collect/src/redact.rs +++ b/crates/fpai-collect/src/redact.rs @@ -196,7 +196,9 @@ fn scrub_in_place(v: &mut Value, field_name: Option<&str>, n: &mut usize) { *s = "[redacted:secret-assignment]".to_string(); } } - Value::Array(a) => a.iter_mut().for_each(|e| scrub_in_place(e, None, n)), + // 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 { @@ -717,6 +719,34 @@ mod tests { 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"}); diff --git a/crates/fpai-collect/tests/uploader.rs b/crates/fpai-collect/tests/uploader.rs index 4a897b57..1191143b 100644 --- a/crates/fpai-collect/tests/uploader.rs +++ b/crates/fpai-collect/tests/uploader.rs @@ -127,6 +127,45 @@ async fn sdk_batches_are_redacted_before_upload() { fs::remove_dir_all(&failed).ok(); } +/// A batch from an SDK that predates its own redaction gets exactly one pass — +/// this one — so it has to reach credentials held in secret-named arrays too. +#[tokio::test] +async fn secret_named_arrays_in_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-array-spool"); + let failed = tmpdir("redact-array-failed"); + let batch = spool.join("event-s-1-0.jsonl"); + fs::write( + &batch, + r#"{"type":"tool_use","input":{"password":["ordinarysecretvalue1"],"client_secret":["ordinarysecretvalue2"],"api_key":["ordinarysecretvalue3"],"access_token":[["ordinarysecretvalue4"]]}} +"#, + ) + .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("ordinarysecretvalue"), + "credential reached the wire" + ); + assert_eq!(body.matches("[redacted:secret-assignment]").count(), 4); + + 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; diff --git a/sdk/python/failproofai_sdk/_redact.py b/sdk/python/failproofai_sdk/_redact.py index 2ca7ca13..b76f9674 100644 --- a/sdk/python/failproofai_sdk/_redact.py +++ b/sdk/python/failproofai_sdk/_redact.py @@ -213,7 +213,8 @@ def scrub(value, field_name=None): return "[redacted:secret-assignment]" return value if isinstance(value, list): - return [scrub(item) for item in value] + # Elements are more values for the same field, so its name still applies. + return [scrub(item, field_name) for item in value] if isinstance(value, dict): result = {} for key, item in value.items(): diff --git a/sdk/python/tests/test_redaction.py b/sdk/python/tests/test_redaction.py index 6f0f977d..16e31ae9 100644 --- a/sdk/python/tests/test_redaction.py +++ b/sdk/python/tests/test_redaction.py @@ -71,6 +71,29 @@ def test_secret_named_fields_redact_opaque_values(field): assert event["nested"][field] == "[redacted:secret-assignment]" +@pytest.mark.parametrize( + "field", + ["password", "client_secret", "api_key", "access_token", "apiKey", "accessToken"], +) +def test_secret_named_arrays_redact_opaque_elements(field): + encoded = json.dumps( + {"nested": {field: ["abcdefghijklmnop", ["qrstuvwxyzabcdef"], "short", 7, None]}} + ) + event = json.loads(redact_json_line(encoded)) + assert event["nested"][field] == [ + "[redacted:secret-assignment]", + ["[redacted:secret-assignment]"], + "short", + 7, + None, + ] + + +def test_arrays_under_ordinary_fields_are_left_alone(): + encoded = json.dumps({"messages": ["an ordinary sentence of text", ["another long value"]]}) + assert redact_json_line(encoded) == encoded + + def test_credential_shaped_dictionary_keys_are_redacted_without_colliding(): first = "API_KEY=abcdefghijklmnop" second = "API_KEY=qrstuvwxyzabcdef" diff --git a/sdk/python/tests/test_sdk.py b/sdk/python/tests/test_sdk.py index af43cea5..fba3c2c7 100644 --- a/sdk/python/tests/test_sdk.py +++ b/sdk/python/tests/test_sdk.py @@ -652,6 +652,35 @@ def test_writer_redacts_credentials_before_the_spool_reaches_disk(tmp_path): resolver.set_base_dir(original) +def test_writer_redacts_opaque_secrets_in_secret_named_arrays(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": { + "password": ["ordinarysecretvalue1"], + "client_secret": ["ordinarysecretvalue2"], + "api_key": ["ordinarysecretvalue3"], + "access_token": [["ordinarysecretvalue4"]], + }, + } + ) + writer.flush_now() + + raw = next((tmp_path / "events").glob("*.jsonl")).read_text() + assert "ordinarysecretvalue" not in raw + assert raw.count("[redacted:secret-assignment]") == 4 + 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