From 9040ea820523e95aa10af917f17a6770c308e0d4 Mon Sep 17 00:00:00 2001 From: ablaszkiewicz Date: Tue, 6 Oct 2026 12:41:34 +0200 Subject: [PATCH 1/7] feat(cymbal): mask python code variables with the sdk rules Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MxaTpjkJ3QxjaCSrYxXFHW --- rust/cymbal/README.md | 24 ++ rust/cymbal/src/core/code_variables.rs | 405 ++++++++++++++++++ rust/cymbal/src/core/mod.rs | 1 + rust/cymbal/src/modes/processing/config.rs | 5 + .../modes/processing/stages/http_pipeline.rs | 5 +- .../modes/processing/stages/resolution/mod.rs | 5 + .../src/modes/processing/types/event.rs | 23 +- .../modes/processing/types/exception_event.rs | 17 +- rust/cymbal/tests/common/mod.rs | 1 + rust/cymbal/tests/event.rs | 69 +++ 10 files changed, 549 insertions(+), 6 deletions(-) create mode 100644 rust/cymbal/src/core/code_variables.rs diff --git a/rust/cymbal/README.md b/rust/cymbal/README.md index b21c3bcb24b9..ba6f6c8da5b6 100644 --- a/rust/cymbal/README.md +++ b/rust/cymbal/README.md @@ -98,6 +98,30 @@ two times: The setting does not change frame records that are already in Postgres. Remove their variables separately if they must not stay there. +## Masking code variables (processing mode) + +Processing mode masks `code_variables` in Python frames for every team that +`ERROR_TRACKING_DROP_CODE_VARIABLES_TEAM_IDS` does not list. The rules are the +default masking rules of the posthog-python SDK, ported to +`src/core/code_variables.rs`. Older SDK versions send values that the current +rules redact, so applying the rules on the server covers those senders. + +- A variable, key or string value that contains a sensitive name, such as + `password`, `token` or `auth`, is redacted whole. +- A value in a known vendor format, a high-entropy token, a PEM private key and + a signed URL with a `sig` parameter are redacted whole. +- URL credentials and `Bearer` or `Basic` credentials are replaced in place, so + the rest of the text stays. +- A string that holds a JSON object or array, which is how the SDK sends a dict + or an object, is masked field by field. + +Masking runs at the same two points as dropping, so new frame records hold +masked values, and records stored earlier are masked when they are replayed. +It does not change records that are already in Postgres. + +Set `ERROR_TRACKING_MASK_CODE_VARIABLES=false` to turn masking off. When the SDK +changes its default rules, change `src/core/code_variables.rs` to match. + ## Remote resolution behavior The public HTTP contract stays `POST /process`: callers send an array of diff --git a/rust/cymbal/src/core/code_variables.rs b/rust/cymbal/src/core/code_variables.rs new file mode 100644 index 000000000000..0ad072a33eed --- /dev/null +++ b/rust/cymbal/src/core/code_variables.rs @@ -0,0 +1,405 @@ +//! Server-side masking of Python frame `code_variables`. +//! +//! The rules port the default masking of the posthog-python SDK +//! (`posthog/exception_utils.py`). Older SDK versions send values that the current rules +//! redact, so cymbal applies the current rules to every event it processes. Keep the +//! pattern lists in step with the SDK. + +use std::{borrow::Cow, collections::HashMap, sync::LazyLock}; + +use regex::{Captures, Regex}; +use serde_json::{Map, Value}; + +pub const REDACTED: &str = "$$_posthog_redacted_based_on_masking_rules_$$"; +pub const TOO_LONG: &str = "$$_posthog_value_too_long_$$"; + +const MAX_LENGTH_FOR_PATTERN_MATCH: usize = 2_048; +const MAX_DEPTH: usize = 12; +const SECRET_MIN_LENGTH: usize = 16; +const SECRET_MIN_ENTROPY_BITS: f64 = 3.8; +const SECRET_MIN_CHAR_CLASSES: u8 = 3; +// Shorter values, and lowercase words, are prose such as "the bearer of". +const AUTH_CREDENTIAL_MIN_LENGTH: usize = 8; +const PEM_PRIVATE_KEY_MARKER: &str = "PRIVATE KEY-----"; +// Punctuation of reprs and structured strings. A bare token never holds it. +const SECRET_REJECT_CHARS: &str = "()[]{}<>'\"`,;"; + +// The SDK's `DEFAULT_CODE_VARIABLES_MASK_PATTERNS`. The SDK matches them against names and +// string values alike, so a value that contains `token` is redacted whole. +static MASK_PATTERNS: LazyLock = LazyLock::new(|| { + Regex::new(concat!( + r"(?i)password|secret|passwd|pwd|api_key|apikey|auth|credentials|privatekey|", + r"private_key|token|aws_access_key_id|_pass|sk_|jwt|connection_string|", + r"connectionstring|conn_str|connstr|dsn|[?&]sig=" + )) + .unwrap() +}); + +// The SDK's `_KNOWN_SECRET_PATTERNS`. +static KNOWN_SECRET: LazyLock = LazyLock::new(|| { + Regex::new( + &[ + r"sk-ant-[A-Za-z0-9_-]{16,}", + r"sk-(?:proj-)?[A-Za-z0-9_-]{20,}", + r"hf_[A-Za-z0-9]{34}", + r"AKIA[0-9A-Z]{16}", + r"(?:ASIA|AGPA|AIDA|AROA|AIPA|ANPA|ANVA|ABIA|ACCA)[0-9A-Z]{16}", + r"AIza[A-Za-z0-9_-]{35}", + r"ya29\.[A-Za-z0-9_-]{20,}", + r"do[opr]_v1_[a-f0-9]{64}", + r"(?:sk|pk|rk)_(?:live|test)_[A-Za-z0-9]{16,}", + r"sq0[a-z]{3}-[A-Za-z0-9_-]{22,43}", + r"gh[pousr]_[A-Za-z0-9]{36}", + r"github_pat_[A-Za-z0-9_]{20,}", + r"gl(?:pat|ptt|rt|soat)-[A-Za-z0-9_-]{20}", + r"glsa_[A-Za-z0-9]{32}_[A-Fa-f0-9]{8}", + r"xox[abeoprs]-[A-Za-z0-9-]{10,}", + r"xapp-[0-9]-[A-Za-z0-9-]{10,}", + r"SK[0-9a-fA-F]{32}", + r"SG\.[A-Za-z0-9_-]{22}\.[A-Za-z0-9_-]{43}", + r"key-[0-9a-f]{32}", + r"[0-9a-f]{32}-us[0-9]{1,2}", + r"npm_[A-Za-z0-9]{36}", + r"pypi-AgEI[A-Za-z0-9_-]{50,}", + r"dapi[0-9a-f]{32}", + r"dp\.pt\.[A-Za-z0-9]{40,}", + r"PMAK-[a-f0-9]{24}-[a-f0-9]{34}", + r"lin_api_[A-Za-z0-9]{40}", + r"ntn_[A-Za-z0-9]{40,}", + r"shp(?:at|ca|pa|ss)_[a-fA-F0-9]{32}", + r"NR(?:AK|JS|II|MA|RA)-[A-Za-z0-9]{27}", + r"eyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{6,}", + ] + .join("|"), + ) + .unwrap() +}); + +// The SDK uses a lookahead that the regex crate lacks, so `redact_url_credential` checks +// for the `:` instead. +static URL_CREDENTIALS: LazyLock = + LazyLock::new(|| Regex::new(r"(?i)([a-z][a-z0-9+.\-]{0,30}://)([^/\s]*)@").unwrap()); + +// A header pair list or an ASGI scope holds an `Authorization` value apart from its header +// name, so the name patterns never see it. +static AUTH_HEADER_CREDENTIALS: LazyLock = + LazyLock::new(|| Regex::new(r"(?i)\b(bearer|basic)(\s+)([A-Za-z0-9._~+/-]+=*)").unwrap()); + +static UUID: LazyLock = LazyLock::new(|| { + Regex::new(r"^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$") + .unwrap() +}); + +static PATH_WORD: LazyLock = LazyLock::new(|| Regex::new(r"^[a-z][a-z.]*$").unwrap()); + +// A key that matches a mask pattern is kept only in this shape. Other text, such as +// `password=hunter2`, can hold the value itself. +static FIELD_NAME: LazyLock = LazyLock::new(|| Regex::new(r"^[\w.\-]+$").unwrap()); + +pub fn mask_code_variables(code_variables: &mut Value) { + mask_value(code_variables, 0); +} + +fn mask_value(value: &mut Value, depth: usize) { + if depth >= MAX_DEPTH && (value.is_object() || value.is_array()) { + *value = Value::String(TOO_LONG.to_string()); + return; + } + match value { + Value::String(text) => { + if let Some(masked) = mask_string(text, depth) { + *text = masked; + } + } + Value::Array(items) => { + for item in items { + mask_value(item, depth + 1); + } + } + Value::Object(map) => { + let entries = std::mem::take(map); + *map = mask_mapping(entries, depth); + } + _ => {} + } +} + +fn mask_string(value: &str, depth: usize) -> Option { + if value.chars().count() > MAX_LENGTH_FOR_PATTERN_MATCH { + return Some(TOO_LONG.to_string()); + } + // The SDK serializes a dict or an object to a JSON string after it masks it. The string + // rules would redact all of it for any sensitive key name, so mask the structure instead. + if let Some(parsed) = parse_json_container(value) { + let mut masked = parsed.clone(); + mask_value(&mut masked, depth + 1); + return (masked != parsed).then(|| masked.to_string()); + } + if MASK_PATTERNS.is_match(value) || looks_like_secret(value) { + return Some(REDACTED.to_string()); + } + match redact_embedded_credentials(value) { + Cow::Owned(redacted) => Some(redacted), + Cow::Borrowed(_) => None, + } +} + +fn parse_json_container(value: &str) -> Option { + if !matches!(value.trim_start().as_bytes().first(), Some(b'{' | b'[')) { + return None; + } + serde_json::from_str(value).ok() +} + +fn mask_mapping(entries: Map, depth: usize) -> Map { + let mut result = Map::new(); + for (key, mut value) in entries { + if key.chars().count() > MAX_LENGTH_FOR_PATTERN_MATCH { + let placeholder = redacted_key(&result); + result.insert(placeholder, Value::String(TOO_LONG.to_string())); + continue; + } + let key_matches_mask = MASK_PATTERNS.is_match(&key); + let mut out_key = + if (key_matches_mask && !FIELD_NAME.is_match(&key)) || looks_like_secret(&key) { + redacted_key(&result) + } else { + redact_embedded_credentials(&key).into_owned() + }; + // Two keys can mask to the same text, e.g. URLs that differ only in their credentials. + if result.contains_key(&out_key) { + out_key = redacted_key(&result); + } + if key_matches_mask { + value = Value::String(REDACTED.to_string()); + } else { + mask_value(&mut value, depth + 1); + } + result.insert(out_key, value); + } + result +} + +fn redacted_key(result: &Map) -> String { + (0..) + .map(|n| format!("$$_posthog_redacted_key_{n}_$$")) + .find(|candidate| !result.contains_key(candidate)) + .expect("an unused placeholder exists") +} + +fn redact_embedded_credentials(value: &str) -> Cow<'_, str> { + let value = AUTH_HEADER_CREDENTIALS.replace_all(value, redact_auth_credential); + if !value.contains("://") { + return value; + } + let redacted = match URL_CREDENTIALS.replace_all(&value, redact_url_credential) { + Cow::Owned(redacted) => Some(redacted), + Cow::Borrowed(_) => None, + }; + redacted.map_or(value, Cow::Owned) +} + +fn redact_auth_credential(caps: &Captures) -> String { + let credential = &caps[3]; + if credential.len() < AUTH_CREDENTIAL_MIN_LENGTH + || credential.bytes().all(|b| b.is_ascii_lowercase()) + { + return caps[0].to_string(); + } + format!("{}{}{REDACTED}", &caps[1], &caps[2]) +} + +fn redact_url_credential(caps: &Captures) -> String { + // Only userinfo with a password is a credential: `ssh://git@host` keeps its username. + let userinfo = &caps[2]; + if userinfo + .split('@') + .next() + .is_some_and(|user| user.contains(':')) + { + format!("{}{REDACTED}@", &caps[1]) + } else { + caps[0].to_string() + } +} + +fn looks_like_secret(value: &str) -> bool { + if value.contains(PEM_PRIVATE_KEY_MARKER) { + return true; + } + let length = value.chars().count(); + if length < SECRET_MIN_LENGTH { + return false; + } + if is_high_entropy_secret(value, length) { + return true; + } + // Known formats that the entropy check misses, e.g. AWS key ids with two char classes. + length <= MAX_LENGTH_FOR_PATTERN_MATCH && KNOWN_SECRET.is_match(value) +} + +fn is_high_entropy_secret(value: &str, length: usize) -> bool { + if value.contains(' ') || looks_like_path_or_url(value) || UUID.is_match(value) { + return false; + } + let mut counts: HashMap = HashMap::new(); + for ch in value.chars() { + *counts.entry(ch).or_default() += 1; + } + + let (mut lower, mut upper, mut digit, mut symbol) = (false, false, false, false); + let mut hex_only = true; + for &ch in counts.keys() { + if ch.is_whitespace() || SECRET_REJECT_CHARS.contains(ch) { + return false; + } + if ch.is_lowercase() { + lower = true; + hex_only &= ch.is_ascii_hexdigit(); + } else if ch.is_uppercase() { + upper = true; + hex_only &= ch.is_ascii_hexdigit(); + } else if ch.is_numeric() { + digit = true; + } else { + symbol = true; + hex_only = false; + } + } + // A hex string is an id or a digest, such as a SHA or an ObjectId. + let classes = [lower, upper, digit, symbol].iter().filter(|&&c| c).count() as u8; + if hex_only || classes < SECRET_MIN_CHAR_CLASSES { + return false; + } + + let total = length as f64; + let entropy: f64 = counts + .values() + .map(|&n| { + let p = n as f64 / total; + -p * p.log2() + }) + .sum(); + entropy >= SECRET_MIN_ENTROPY_BITS +} + +fn looks_like_path_or_url(value: &str) -> bool { + if value.contains("://") || value.contains('\\') { + return true; + } + value.contains('/') + && value + .split('/') + .filter(|segment| !segment.is_empty() && PATH_WORD.is_match(segment)) + .nth(1) + .is_some() +} + +#[cfg(test)] +mod tests { + use serde_json::{json, Value}; + + use super::*; + + // Synthetic fakes, assembled at runtime so no complete credential literal sits in source. + fn fake(prefix: &str, body: &str) -> String { + format!("{prefix}{body}") + } + + fn masked(value: &Value) -> Value { + let mut value = value.clone(); + mask_code_variables(&mut value); + value + } + + #[test] + fn masks_what_the_python_sdk_masks() { + let stripe_key = fake("sk_live_", "Zx81Qm7Lp2Vb9Nc4Rt6Yh3Kd"); + let bearer_token = fake("tok_", "Zx81Qm7Lp2Vb9Nc4"); + let basic_credential = fake("c3ZjOmZha2Ut", "cGFzcy0xMjM="); + let cases = [ + ( + "variable with a secret name", + json!({"api_key": "abc"}), + json!({"api_key": REDACTED}), + ), + ( + "known format under a neutral name", + json!({"value": stripe_key}), + json!({"value": REDACTED}), + ), + ( + "high-entropy value", + json!({"value": "Zx81Qm7Lp2Vb9Nc4Rt6Yh3Kd"}), + json!({"value": REDACTED}), + ), + ( + "PEM private key", + json!({"value": "-----BEGIN PRIVATE KEY-----\nMIIEvQ"}), + json!({"value": REDACTED}), + ), + ( + "URL credentials", + json!({"url": "postgresql://app:hunter22@db.example.com/app"}), + json!({"url": format!("postgresql://{REDACTED}@db.example.com/app")}), + ), + ( + "Bearer value in a header pair list", + json!({"headers": [["authorization", format!("Bearer {bearer_token}")]]}), + json!({"headers": [[REDACTED, format!("Bearer {REDACTED}")]]}), + ), + ( + "Basic value under a neutral name", + json!({"value": format!("Basic {basic_credential}")}), + json!({"value": format!("Basic {REDACTED}")}), + ), + ( + "signed URL", + json!({"url": "https://acct.blob.core.windows.net/c/f?sv=2022-11-02&sig=q2VxT8fKz1aB3dE%3D"}), + json!({"url": REDACTED}), + ), + ( + "object repr from an old SDK", + json!({"settings": "Settings(AWS_SECRET_ACCESS_KEY='abc')"}), + json!({"settings": REDACTED}), + ), + ( + "JSON string from the current SDK keeps its safe fields", + json!({"user": "{\"name\": \"bob\", \"password\": \"hunter22\"}"}), + json!({"user": format!("{{\"name\":\"bob\",\"password\":\"{REDACTED}\"}}")}), + ), + ( + "URL credentials in a key", + json!({"pools": {"postgresql://app:hunter22@db.example.com/app": 1}}), + json!({"pools": Map::from_iter([( + format!("postgresql://{REDACTED}@db.example.com/app"), + json!(1), + )])}), + ), + ]; + + for (name, input, expected) in cases { + let once = masked(&input); + assert_eq!(once, expected, "{name}"); + // Processing masks a frame on arrival and again after resolution. + assert_eq!(masked(&once), once, "{name} is not idempotent"); + } + } + + #[test] + fn leaves_benign_values_alone() { + for value in [ + "hello world", + "design", + "/signup?step=2", + "the bearer of bad news", + "550e8400-e29b-41d4-a716-446655440000", + "da39a3ee5e6b4b0d3255bfef95601890afd80709", + "/usr/local/lib/python3.12/site-packages/app/views.py", + "{\"name\": \"bob\"}", + ] { + let input = json!({"value": value}); + assert_eq!(masked(&input), input, "{value}"); + } + } +} diff --git a/rust/cymbal/src/core/mod.rs b/rust/cymbal/src/core/mod.rs index 5b935312b36f..82a967ff7767 100644 --- a/rust/cymbal/src/core/mod.rs +++ b/rust/cymbal/src/core/mod.rs @@ -4,6 +4,7 @@ //! this subtree can later be lifted into its own `cymbal-core` crate. pub mod analytics; +pub mod code_variables; pub mod config; pub mod error; pub mod ids; diff --git a/rust/cymbal/src/modes/processing/config.rs b/rust/cymbal/src/modes/processing/config.rs index 68b051e43398..4c5797fff7d5 100644 --- a/rust/cymbal/src/modes/processing/config.rs +++ b/rust/cymbal/src/modes/processing/config.rs @@ -204,6 +204,11 @@ pub struct ProcessingConfig { #[envconfig(from = "ERROR_TRACKING_DROP_CODE_VARIABLES_TEAM_IDS", default = "")] pub drop_code_variables_team_ids: String, + // Masks frame code variables with the posthog-python SDK's default rules, for teams not + // listed above. Older SDK versions send values that the current rules redact. + #[envconfig(from = "ERROR_TRACKING_MASK_CODE_VARIABLES", default = "true")] + pub mask_code_variables: bool, + // ---------------------------------------------------------------------- // Remote resolution (cymbal.resolution.v1). // ---------------------------------------------------------------------- diff --git a/rust/cymbal/src/modes/processing/stages/http_pipeline.rs b/rust/cymbal/src/modes/processing/stages/http_pipeline.rs index 86286925f66e..7dbf54c5e860 100644 --- a/rust/cymbal/src/modes/processing/stages/http_pipeline.rs +++ b/rust/cymbal/src/modes/processing/stages/http_pipeline.rs @@ -34,12 +34,15 @@ impl Stage for HttpEventPipeline { async fn process(self, batch: Batch) -> StageResult { let (preprocess, postprocess) = create_pre_post_processing(batch.len(), Box::new(handle_result)); - // Strip first: pre-processing keeps this copy and returns it unchanged on failure. + // Drop or mask first: pre-processing keeps this copy and returns it unchanged on failure. let drop_team_ids = self.app_context.drop_code_variables_team_ids.clone(); + let mask = self.app_context.config.mask_code_variables; let batch = batch.map( |mut event, ()| { if drop_team_ids.contains(&event.team_id) { event.drop_code_variables(); + } else if mask { + event.mask_code_variables(); } event }, diff --git a/rust/cymbal/src/modes/processing/stages/resolution/mod.rs b/rust/cymbal/src/modes/processing/stages/resolution/mod.rs index 0811e27c0155..55b3f54afeda 100644 --- a/rust/cymbal/src/modes/processing/stages/resolution/mod.rs +++ b/rust/cymbal/src/modes/processing/stages/resolution/mod.rs @@ -28,6 +28,7 @@ pub struct ResolutionStage { pub posthog_pool: PgPool, pub release_cache: ReleaseCache, pub drop_code_variables_team_ids: Arc>, + pub mask_code_variables: bool, } #[derive(Clone)] @@ -47,6 +48,7 @@ impl From<&Arc> for ResolutionStage { posthog_pool: app_context.posthog_pool.clone(), release_cache: app_context.release_cache.clone(), drop_code_variables_team_ids: app_context.drop_code_variables_team_ids.clone(), + mask_code_variables: app_context.config.mask_code_variables, } } } @@ -76,11 +78,14 @@ impl Stage for ResolutionStage { // frames' symbol sets for legacy events. let resolved = resolve_batch(batch, self.remote.clone()).await?; let drop_team_ids = self.drop_code_variables_team_ids.clone(); + let mask = self.mask_code_variables; let resolved = resolved.map( |item, ()| { item.map(|mut event| { if drop_team_ids.contains(&event.team_id()) { event.drop_code_variables(); + } else if mask { + event.mask_code_variables(); } event }) diff --git a/rust/cymbal/src/modes/processing/types/event.rs b/rust/cymbal/src/modes/processing/types/event.rs index 3d8f7ecccc62..ba3922cf2ccb 100644 --- a/rust/cymbal/src/modes/processing/types/event.rs +++ b/rust/cymbal/src/modes/processing/types/event.rs @@ -2,10 +2,13 @@ use std::collections::HashMap; use common_types::ClickHouseEvent; use serde::{Deserialize, Serialize}; -use serde_json::Value; +use serde_json::{Map, Value}; use uuid::Uuid; -use crate::error::{EventError, UnhandledError}; +use crate::{ + core::code_variables::mask_code_variables, + error::{EventError, UnhandledError}, +}; #[derive(Serialize, Deserialize, Clone, Debug)] pub struct AnyEvent { @@ -22,6 +25,20 @@ pub struct AnyEvent { impl AnyEvent { pub fn drop_code_variables(&mut self) { + self.for_each_frame(|frame| { + frame.remove("code_variables"); + }); + } + + pub fn mask_code_variables(&mut self) { + self.for_each_frame(|frame| { + if let Some(code_variables) = frame.get_mut("code_variables") { + mask_code_variables(code_variables); + } + }); + } + + fn for_each_frame(&mut self, mut apply: impl FnMut(&mut Map)) { let Some(exceptions) = self .properties .get_mut("$exception_list") @@ -37,7 +54,7 @@ impl AnyEvent { continue; }; for frame in frames.iter_mut().filter_map(Value::as_object_mut) { - frame.remove("code_variables"); + apply(frame); } } } diff --git a/rust/cymbal/src/modes/processing/types/exception_event.rs b/rust/cymbal/src/modes/processing/types/exception_event.rs index 3068fa13e321..8d3cc41610ac 100644 --- a/rust/cymbal/src/modes/processing/types/exception_event.rs +++ b/rust/cymbal/src/modes/processing/types/exception_event.rs @@ -5,6 +5,7 @@ use serde_json::{Map, Value}; use uuid::Uuid; use crate::{ + core::code_variables::mask_code_variables, error::EventError, fingerprinting::{Fingerprint, FingerprintRecordPart, FingerprintVersion}, frames::{ @@ -211,18 +212,30 @@ impl ExceptionEvent { /// Resolved frames need this even when the event sent none: stored records replay old ones. pub fn drop_code_variables(&mut self) { + self.for_each_code_variables(|code_variables| *code_variables = None); + } + + pub fn mask_code_variables(&mut self) { + self.for_each_code_variables(|code_variables| { + if let Some(code_variables) = code_variables { + mask_code_variables(code_variables); + } + }); + } + + fn for_each_code_variables(&mut self, mut apply: impl FnMut(&mut Option)) { for exception in self.exception_list.iter_mut() { match &mut exception.stack { Some(Stacktrace::Raw { frames }) => { for frame in frames.iter_mut() { if let RawFrame::Python(python) = frame { - python.code_variables = None; + apply(&mut python.code_variables); } } } Some(Stacktrace::Resolved { frames }) => { for frame in frames.iter_mut() { - frame.code_variables = None; + apply(&mut frame.code_variables); } } None => {} diff --git a/rust/cymbal/tests/common/mod.rs b/rust/cymbal/tests/common/mod.rs index 45dc09812136..107aa012baab 100644 --- a/rust/cymbal/tests/common/mod.rs +++ b/rust/cymbal/tests/common/mod.rs @@ -435,6 +435,7 @@ pub fn remote_stage(ctx: RemoteResolutionContext) -> ResolutionStage { .expect("lazy pool construction does not connect"), release_cache: ReleaseCache::new(0, Duration::from_secs(0)), drop_code_variables_team_ids: Default::default(), + mask_code_variables: true, } } diff --git a/rust/cymbal/tests/event.rs b/rust/cymbal/tests/event.rs index b35aaa9b4415..51b965c9afb8 100644 --- a/rust/cymbal/tests/event.rs +++ b/rust/cymbal/tests/event.rs @@ -4,6 +4,7 @@ use axum::{body::Body, http::Request}; use chrono::{DateTime, Utc}; use common_types::error_tracking::FrameId; use cymbal::{ + core::code_variables::REDACTED, error::UnhandledError, fingerprinting::{Fingerprint, FingerprintVersion}, frames::Frame, @@ -841,6 +842,74 @@ async fn drops_code_variables_from_events_that_fail_to_parse(db: PgPool) { assert_eq!(frames_with_code_variables(&body), 0); } +const FAKE_DB_PASSWORD: &str = "fake-pass-for-tests"; + +fn python_event_with_database_url() -> AnyEvent { + let mut event = load_static_event("python"); + for frame in event.properties["$exception_list"][0]["stacktrace"]["frames"] + .as_array_mut() + .unwrap() + { + frame["code_variables"] = + json!({"url": format!("postgresql://app:{FAKE_DB_PASSWORD}@db.example.com/app")}); + } + event +} + +fn exception_list_contains(body: &SuccessResponse, needle: &str) -> bool { + let event = body + .first_event() + .as_ref() + .expect("event should be returned"); + event.properties["$exception_list"] + .to_string() + .contains(needle) +} + +#[sqlx::test(migrations = "./tests/test_migrations")] +async fn stores_masked_code_variables(db: PgPool) { + let harness = TestHarness::new(db.clone()); + + let (status, body): (_, SuccessResponse) = harness + .post_event_with_config(&python_event_with_database_url(), |_| {}) + .await; + + assert!(status.is_success()); + assert!(!exception_list_contains(&body, FAKE_DB_PASSWORD)); + let (stored, stored_unmasked): (i64, i64) = sqlx::query_as( + "SELECT count(*), count(*) FILTER (WHERE contents::text LIKE '%' || $1 || '%') + FROM posthog_errortrackingstackframe WHERE team_id = 1", + ) + .bind(FAKE_DB_PASSWORD) + .fetch_one(&db) + .await + .unwrap(); + assert!(stored > 0); + assert_eq!(stored_unmasked, 0); +} + +#[sqlx::test(migrations = "./tests/test_migrations")] +async fn masks_code_variables_replayed_from_stored_frames(db: PgPool) { + let harness = TestHarness::new(db); + + // Masking off stands in for frame records stored before masking existed. + let (status, body): (_, SuccessResponse) = harness + .post_event_with_config(&python_event_with_database_url(), |config| { + config.mask_code_variables = false; + }) + .await; + assert!(status.is_success()); + assert!(exception_list_contains(&body, FAKE_DB_PASSWORD)); + + // The second event carries no code variables, so any that come back are replayed records. + let mut event = load_static_event("python"); + event.uuid = Uuid::now_v7(); + let (status, body): (_, SuccessResponse) = harness.post_event_with_config(&event, |_| {}).await; + assert!(status.is_success()); + assert!(!exception_list_contains(&body, FAKE_DB_PASSWORD)); + assert!(exception_list_contains(&body, REDACTED)); +} + #[sqlx::test(migrations = "./tests/test_migrations")] async fn resolves_javascript_with_sourcemaps(db: PgPool) { let harness = TestHarness::new(db.clone()); From 930807ae5aa35e2c1279316fa9c6e3d9eb823bd4 Mon Sep 17 00:00:00 2001 From: ablaszkiewicz Date: Tue, 6 Oct 2026 13:08:39 +0200 Subject: [PATCH 2/7] fix(cymbal): mask lowercase and colon-separated authorization credentials Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MxaTpjkJ3QxjaCSrYxXFHW --- rust/cymbal/src/core/code_variables.rs | 32 +++++++++++++++++++------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/rust/cymbal/src/core/code_variables.rs b/rust/cymbal/src/core/code_variables.rs index 0ad072a33eed..1375318fd77c 100644 --- a/rust/cymbal/src/core/code_variables.rs +++ b/rust/cymbal/src/core/code_variables.rs @@ -18,7 +18,7 @@ const MAX_DEPTH: usize = 12; const SECRET_MIN_LENGTH: usize = 16; const SECRET_MIN_ENTROPY_BITS: f64 = 3.8; const SECRET_MIN_CHAR_CLASSES: u8 = 3; -// Shorter values, and lowercase words, are prose such as "the bearer of". +// Shorter values are prose, such as "the bearer of". const AUTH_CREDENTIAL_MIN_LENGTH: usize = 8; const PEM_PRIVATE_KEY_MARKER: &str = "PRIVATE KEY-----"; // Punctuation of reprs and structured strings. A bare token never holds it. @@ -81,9 +81,12 @@ static URL_CREDENTIALS: LazyLock = LazyLock::new(|| Regex::new(r"(?i)([a-z][a-z0-9+.\-]{0,30}://)([^/\s]*)@").unwrap()); // A header pair list or an ASGI scope holds an `Authorization` value apart from its header -// name, so the name patterns never see it. -static AUTH_HEADER_CREDENTIALS: LazyLock = - LazyLock::new(|| Regex::new(r"(?i)\b(bearer|basic)(\s+)([A-Za-z0-9._~+/-]+=*)").unwrap()); +// name, so the name patterns never see it. The separator also accepts a colon or an opening +// quote, as in `Bearer: `, but it must not be empty, so that names such as +// `basicConfig` stay untouched. +static AUTH_HEADER_CREDENTIALS: LazyLock = LazyLock::new(|| { + Regex::new(r#"(?i)\b(bearer|basic)((?:\s*:\s*|\s+)['"]?)([A-Za-z0-9._~+/-]+=*)"#).unwrap() +}); static UUID: LazyLock = LazyLock::new(|| { Regex::new(r"^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$") @@ -200,10 +203,7 @@ fn redact_embedded_credentials(value: &str) -> Cow<'_, str> { } fn redact_auth_credential(caps: &Captures) -> String { - let credential = &caps[3]; - if credential.len() < AUTH_CREDENTIAL_MIN_LENGTH - || credential.bytes().all(|b| b.is_ascii_lowercase()) - { + if caps[3].len() < AUTH_CREDENTIAL_MIN_LENGTH { return caps[0].to_string(); } format!("{}{}{REDACTED}", &caps[1], &caps[2]) @@ -353,6 +353,21 @@ mod tests { json!({"value": format!("Basic {basic_credential}")}), json!({"value": format!("Basic {REDACTED}")}), ), + ( + "credential of lowercase letters only", + json!({"value": format!("Bearer {}", fake("zqwklmno", "pxyzrstu"))}), + json!({"value": format!("Bearer {REDACTED}")}), + ), + ( + "colon between the scheme and the credential", + json!({"value": format!("Bearer: {bearer_token}")}), + json!({"value": format!("Bearer: {REDACTED}")}), + ), + ( + "quote between the scheme and the credential", + json!({"value": format!("Bearer '{bearer_token}'")}), + json!({"value": format!("Bearer '{REDACTED}'")}), + ), ( "signed URL", json!({"url": "https://acct.blob.core.windows.net/c/f?sv=2022-11-02&sig=q2VxT8fKz1aB3dE%3D"}), @@ -393,6 +408,7 @@ mod tests { "design", "/signup?step=2", "the bearer of bad news", + "basicConfig(level=10)", "550e8400-e29b-41d4-a716-446655440000", "da39a3ee5e6b4b0d3255bfef95601890afd80709", "/usr/local/lib/python3.12/site-packages/app/views.py", From 2e9f86213c14e19640baf2fe7787247e3a247cf7 Mon Sep 17 00:00:00 2001 From: ablaszkiewicz Date: Tue, 6 Oct 2026 13:47:40 +0200 Subject: [PATCH 3/7] feat(cymbal): remove the code variables masking setting Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MxaTpjkJ3QxjaCSrYxXFHW --- rust/cymbal/README.md | 4 ++-- rust/cymbal/src/modes/processing/config.rs | 5 ---- .../modes/processing/stages/http_pipeline.rs | 3 +-- .../modes/processing/stages/resolution/mod.rs | 5 +--- rust/cymbal/tests/common/mod.rs | 1 - rust/cymbal/tests/event.rs | 24 +++++++++++++------ 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/rust/cymbal/README.md b/rust/cymbal/README.md index ba6f6c8da5b6..bcf3046b3d75 100644 --- a/rust/cymbal/README.md +++ b/rust/cymbal/README.md @@ -119,8 +119,8 @@ Masking runs at the same two points as dropping, so new frame records hold masked values, and records stored earlier are masked when they are replayed. It does not change records that are already in Postgres. -Set `ERROR_TRACKING_MASK_CODE_VARIABLES=false` to turn masking off. When the SDK -changes its default rules, change `src/core/code_variables.rs` to match. +Masking has no setting and is always on. When the SDK changes its default +rules, change `src/core/code_variables.rs` to match. ## Remote resolution behavior diff --git a/rust/cymbal/src/modes/processing/config.rs b/rust/cymbal/src/modes/processing/config.rs index 4c5797fff7d5..68b051e43398 100644 --- a/rust/cymbal/src/modes/processing/config.rs +++ b/rust/cymbal/src/modes/processing/config.rs @@ -204,11 +204,6 @@ pub struct ProcessingConfig { #[envconfig(from = "ERROR_TRACKING_DROP_CODE_VARIABLES_TEAM_IDS", default = "")] pub drop_code_variables_team_ids: String, - // Masks frame code variables with the posthog-python SDK's default rules, for teams not - // listed above. Older SDK versions send values that the current rules redact. - #[envconfig(from = "ERROR_TRACKING_MASK_CODE_VARIABLES", default = "true")] - pub mask_code_variables: bool, - // ---------------------------------------------------------------------- // Remote resolution (cymbal.resolution.v1). // ---------------------------------------------------------------------- diff --git a/rust/cymbal/src/modes/processing/stages/http_pipeline.rs b/rust/cymbal/src/modes/processing/stages/http_pipeline.rs index 7dbf54c5e860..f2452082adbe 100644 --- a/rust/cymbal/src/modes/processing/stages/http_pipeline.rs +++ b/rust/cymbal/src/modes/processing/stages/http_pipeline.rs @@ -36,12 +36,11 @@ impl Stage for HttpEventPipeline { create_pre_post_processing(batch.len(), Box::new(handle_result)); // Drop or mask first: pre-processing keeps this copy and returns it unchanged on failure. let drop_team_ids = self.app_context.drop_code_variables_team_ids.clone(); - let mask = self.app_context.config.mask_code_variables; let batch = batch.map( |mut event, ()| { if drop_team_ids.contains(&event.team_id) { event.drop_code_variables(); - } else if mask { + } else { event.mask_code_variables(); } event diff --git a/rust/cymbal/src/modes/processing/stages/resolution/mod.rs b/rust/cymbal/src/modes/processing/stages/resolution/mod.rs index 55b3f54afeda..14bfca2586c5 100644 --- a/rust/cymbal/src/modes/processing/stages/resolution/mod.rs +++ b/rust/cymbal/src/modes/processing/stages/resolution/mod.rs @@ -28,7 +28,6 @@ pub struct ResolutionStage { pub posthog_pool: PgPool, pub release_cache: ReleaseCache, pub drop_code_variables_team_ids: Arc>, - pub mask_code_variables: bool, } #[derive(Clone)] @@ -48,7 +47,6 @@ impl From<&Arc> for ResolutionStage { posthog_pool: app_context.posthog_pool.clone(), release_cache: app_context.release_cache.clone(), drop_code_variables_team_ids: app_context.drop_code_variables_team_ids.clone(), - mask_code_variables: app_context.config.mask_code_variables, } } } @@ -78,13 +76,12 @@ impl Stage for ResolutionStage { // frames' symbol sets for legacy events. let resolved = resolve_batch(batch, self.remote.clone()).await?; let drop_team_ids = self.drop_code_variables_team_ids.clone(); - let mask = self.mask_code_variables; let resolved = resolved.map( |item, ()| { item.map(|mut event| { if drop_team_ids.contains(&event.team_id()) { event.drop_code_variables(); - } else if mask { + } else { event.mask_code_variables(); } event diff --git a/rust/cymbal/tests/common/mod.rs b/rust/cymbal/tests/common/mod.rs index 107aa012baab..45dc09812136 100644 --- a/rust/cymbal/tests/common/mod.rs +++ b/rust/cymbal/tests/common/mod.rs @@ -435,7 +435,6 @@ pub fn remote_stage(ctx: RemoteResolutionContext) -> ResolutionStage { .expect("lazy pool construction does not connect"), release_cache: ReleaseCache::new(0, Duration::from_secs(0)), drop_code_variables_team_ids: Default::default(), - mask_code_variables: true, } } diff --git a/rust/cymbal/tests/event.rs b/rust/cymbal/tests/event.rs index 51b965c9afb8..0c6f14b68bcc 100644 --- a/rust/cymbal/tests/event.rs +++ b/rust/cymbal/tests/event.rs @@ -890,16 +890,26 @@ async fn stores_masked_code_variables(db: PgPool) { #[sqlx::test(migrations = "./tests/test_migrations")] async fn masks_code_variables_replayed_from_stored_frames(db: PgPool) { - let harness = TestHarness::new(db); + let harness = TestHarness::new(db.clone()); - // Masking off stands in for frame records stored before masking existed. - let (status, body): (_, SuccessResponse) = harness - .post_event_with_config(&python_event_with_database_url(), |config| { - config.mask_code_variables = false; - }) + let (status, _): (_, SuccessResponse) = harness + .post_event_with_config(&python_event_with_database_url(), |_| {}) .await; assert!(status.is_success()); - assert!(exception_list_contains(&body, FAKE_DB_PASSWORD)); + // Stands in for frame records stored before masking existed. + let unmasked = + json!({"url": format!("postgresql://app:{FAKE_DB_PASSWORD}@db.example.com/app")}); + let rewritten = sqlx::query( + "UPDATE posthog_errortrackingstackframe + SET contents = jsonb_set(contents, '{code_variables}', $1) + WHERE team_id = 1 AND contents ? 'code_variables'", + ) + .bind(unmasked) + .execute(&db) + .await + .unwrap() + .rows_affected(); + assert!(rewritten > 0); // The second event carries no code variables, so any that come back are replayed records. let mut event = load_static_event("python"); From b15a18f20baec7c032d97904f0793515241e65e9 Mon Sep 17 00:00:00 2001 From: ablaszkiewicz Date: Tue, 6 Oct 2026 14:46:00 +0200 Subject: [PATCH 4/7] fix(cymbal): narrow sk_ masking, stop url userinfo at the authority Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MxaTpjkJ3QxjaCSrYxXFHW --- rust/cymbal/src/core/code_variables.rs | 55 ++++++++++++++++++++------ 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/rust/cymbal/src/core/code_variables.rs b/rust/cymbal/src/core/code_variables.rs index 1375318fd77c..1a7127fa09f8 100644 --- a/rust/cymbal/src/core/code_variables.rs +++ b/rust/cymbal/src/core/code_variables.rs @@ -25,11 +25,13 @@ const PEM_PRIVATE_KEY_MARKER: &str = "PRIVATE KEY-----"; const SECRET_REJECT_CHARS: &str = "()[]{}<>'\"`,;"; // The SDK's `DEFAULT_CODE_VARIABLES_MASK_PATTERNS`. The SDK matches them against names and -// string values alike, so a value that contains `token` is redacted whole. +// string values alike, so a value that contains `token` is redacted whole. One difference: +// `sk_` must start a word here, so `task_id` and `disk_usage` keep their values. A team can +// change the SDK patterns in its own code, but not these. static MASK_PATTERNS: LazyLock = LazyLock::new(|| { Regex::new(concat!( r"(?i)password|secret|passwd|pwd|api_key|apikey|auth|credentials|privatekey|", - r"private_key|token|aws_access_key_id|_pass|sk_|jwt|connection_string|", + r"private_key|token|aws_access_key_id|_pass|(?:^|[^a-z0-9])sk_|jwt|connection_string|", r"connectionstring|conn_str|connstr|dsn|[?&]sig=" )) .unwrap() @@ -76,9 +78,9 @@ static KNOWN_SECRET: LazyLock = LazyLock::new(|| { }); // The SDK uses a lookahead that the regex crate lacks, so `redact_url_credential` checks -// for the `:` instead. +// for the `:` instead. The userinfo ends where the authority ends, at `/`, `?` or `#`. static URL_CREDENTIALS: LazyLock = - LazyLock::new(|| Regex::new(r"(?i)([a-z][a-z0-9+.\-]{0,30}://)([^/\s]*)@").unwrap()); + LazyLock::new(|| Regex::new(r"(?i)([a-z][a-z0-9+.\-]{0,30}://)([^/?#\s]*)@").unwrap()); // A header pair list or an ASGI scope holds an `Authorization` value apart from its header // name, so the name patterns never see it. The separator also accepts a colon or an opening @@ -156,22 +158,23 @@ fn parse_json_container(value: &str) -> Option { fn mask_mapping(entries: Map, depth: usize) -> Map { let mut result = Map::new(); + let mut next_placeholder = 0; for (key, mut value) in entries { if key.chars().count() > MAX_LENGTH_FOR_PATTERN_MATCH { - let placeholder = redacted_key(&result); + let placeholder = redacted_key(&result, &mut next_placeholder); result.insert(placeholder, Value::String(TOO_LONG.to_string())); continue; } let key_matches_mask = MASK_PATTERNS.is_match(&key); let mut out_key = if (key_matches_mask && !FIELD_NAME.is_match(&key)) || looks_like_secret(&key) { - redacted_key(&result) + redacted_key(&result, &mut next_placeholder) } else { redact_embedded_credentials(&key).into_owned() }; // Two keys can mask to the same text, e.g. URLs that differ only in their credentials. if result.contains_key(&out_key) { - out_key = redacted_key(&result); + out_key = redacted_key(&result, &mut next_placeholder); } if key_matches_mask { value = Value::String(REDACTED.to_string()); @@ -183,11 +186,15 @@ fn mask_mapping(entries: Map, depth: usize) -> Map result } -fn redacted_key(result: &Map) -> String { - (0..) - .map(|n| format!("$$_posthog_redacted_key_{n}_$$")) - .find(|candidate| !result.contains_key(candidate)) - .expect("an unused placeholder exists") +// Placeholders are only added, so the search resumes from the last one instead of zero. +fn redacted_key(result: &Map, next: &mut usize) -> String { + loop { + let candidate = format!("$$_posthog_redacted_key_{next}_$$"); + *next += 1; + if !result.contains_key(&candidate) { + return candidate; + } + } } fn redact_embedded_credentials(value: &str) -> Cow<'_, str> { @@ -383,6 +390,24 @@ mod tests { json!({"user": "{\"name\": \"bob\", \"password\": \"hunter22\"}"}), json!({"user": format!("{{\"name\":\"bob\",\"password\":\"{REDACTED}\"}}")}), ), + ( + "URL credentials end at the authority", + json!({"url": "https://app:hunter22@db.example.com?next=a@b"}), + json!({"url": format!("https://{REDACTED}@db.example.com?next=a@b")}), + ), + ( + "sk_ at the start of a word", + json!({"stripe_sk_key": "abc"}), + json!({"stripe_sk_key": REDACTED}), + ), + ( + "several keys that hold text get distinct placeholders", + json!({"pools": {"password=a": 1, "password=b": 2}}), + json!({"pools": { + "$$_posthog_redacted_key_0_$$": REDACTED, + "$$_posthog_redacted_key_1_$$": REDACTED, + }}), + ), ( "URL credentials in a key", json!({"pools": {"postgresql://app:hunter22@db.example.com/app": 1}}), @@ -413,9 +438,15 @@ mod tests { "da39a3ee5e6b4b0d3255bfef95601890afd80709", "/usr/local/lib/python3.12/site-packages/app/views.py", "{\"name\": \"bob\"}", + "disk_usage at 91%", + "https://db.example.com?next=a:b@c", ] { let input = json!({"value": value}); assert_eq!(masked(&input), input, "{value}"); } + for key in ["task_id", "disk_usage"] { + let input = json!({key: "42"}); + assert_eq!(masked(&input), input, "{key}"); + } } } From f46cc04d457e0ca360ba2cece74ea59d7e95f821 Mon Sep 17 00:00:00 2001 From: ablaszkiewicz Date: Tue, 6 Oct 2026 15:36:58 +0200 Subject: [PATCH 5/7] fix(cymbal): port the sdk url-first, prose and short basic rules Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MxaTpjkJ3QxjaCSrYxXFHW --- rust/cymbal/src/core/code_variables.rs | 46 ++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/rust/cymbal/src/core/code_variables.rs b/rust/cymbal/src/core/code_variables.rs index 1a7127fa09f8..d06f5fb28e59 100644 --- a/rust/cymbal/src/core/code_variables.rs +++ b/rust/cymbal/src/core/code_variables.rs @@ -7,6 +7,7 @@ use std::{borrow::Cow, collections::HashMap, sync::LazyLock}; +use base64::Engine; use regex::{Captures, Regex}; use serde_json::{Map, Value}; @@ -18,8 +19,12 @@ const MAX_DEPTH: usize = 12; const SECRET_MIN_LENGTH: usize = 16; const SECRET_MIN_ENTROPY_BITS: f64 = 3.8; const SECRET_MIN_CHAR_CLASSES: u8 = 3; -// Shorter values are prose, such as "the bearer of". +// Shorter values are prose, such as "the bearer of", and so is a lowercase word of up to +// 15 letters, such as "bearer transportation". A random lowercase token is longer than +// that. A `Basic` credential of any length is still redacted when it decodes to +// `user:password`, e.g. `YTpi` for `a:b`. const AUTH_CREDENTIAL_MIN_LENGTH: usize = 8; +const AUTH_PROSE_WORD_MAX_LENGTH: usize = 15; const PEM_PRIVATE_KEY_MARKER: &str = "PRIVATE KEY-----"; // Punctuation of reprs and structured strings. A bare token never holds it. const SECRET_REJECT_CHARS: &str = "()[]{}<>'\"`,;"; @@ -197,12 +202,15 @@ fn redacted_key(result: &Map, next: &mut usize) -> String { } } +// URLs go first, because the `Authorization` pass can consume a URL scheme, as in +// `Bearer postgresql://user:pass@host`. fn redact_embedded_credentials(value: &str) -> Cow<'_, str> { - let value = AUTH_HEADER_CREDENTIALS.replace_all(value, redact_auth_credential); - if !value.contains("://") { - return value; - } - let redacted = match URL_CREDENTIALS.replace_all(&value, redact_url_credential) { + let value = if value.contains("://") { + URL_CREDENTIALS.replace_all(value, redact_url_credential) + } else { + Cow::Borrowed(value) + }; + let redacted = match AUTH_HEADER_CREDENTIALS.replace_all(&value, redact_auth_credential) { Cow::Owned(redacted) => Some(redacted), Cow::Borrowed(_) => None, }; @@ -210,12 +218,24 @@ fn redact_embedded_credentials(value: &str) -> Cow<'_, str> { } fn redact_auth_credential(caps: &Captures) -> String { - if caps[3].len() < AUTH_CREDENTIAL_MIN_LENGTH { + let credential = &caps[3]; + let is_basic_pair = caps[1].eq_ignore_ascii_case("basic") && is_basic_credential(credential); + let is_prose_word = credential.len() <= AUTH_PROSE_WORD_MAX_LENGTH + && credential.bytes().all(|b| b.is_ascii_lowercase()); + if !is_basic_pair && (credential.len() < AUTH_CREDENTIAL_MIN_LENGTH || is_prose_word) { return caps[0].to_string(); } format!("{}{}{REDACTED}", &caps[1], &caps[2]) } +fn is_basic_credential(credential: &str) -> bool { + base64::engine::general_purpose::STANDARD + .decode(credential) + .ok() + .and_then(|decoded| String::from_utf8(decoded).ok()) + .is_some_and(|decoded| decoded.contains(':')) +} + fn redact_url_credential(caps: &Captures) -> String { // Only userinfo with a password is a credential: `ssh://git@host` keeps its username. let userinfo = &caps[2]; @@ -365,6 +385,16 @@ mod tests { json!({"value": format!("Bearer {}", fake("zqwklmno", "pxyzrstu"))}), json!({"value": format!("Bearer {REDACTED}")}), ), + ( + "short Basic credential that decodes to user:password", + json!({"value": "Basic YTpi"}), + json!({"value": format!("Basic {REDACTED}")}), + ), + ( + "DSN after the scheme keeps its own credential redacted", + json!({"value": "Bearer mongodb+srv://alice:sunflower99@db.example.com/app"}), + json!({"value": format!("Bearer {REDACTED}://{REDACTED}@db.example.com/app")}), + ), ( "colon between the scheme and the credential", json!({"value": format!("Bearer: {bearer_token}")}), @@ -433,6 +463,8 @@ mod tests { "design", "/signup?step=2", "the bearer of bad news", + "bearer transportation", + "basic: configuration", "basicConfig(level=10)", "550e8400-e29b-41d4-a716-446655440000", "da39a3ee5e6b4b0d3255bfef95601890afd80709", From 45e8fbf24dafb3717cbcea66c0cfc8a51888b3de Mon Sep 17 00:00:00 2001 From: ablaszkiewicz Date: Tue, 6 Oct 2026 16:25:54 +0200 Subject: [PATCH 6/7] fix(cymbal): keep capitalized words after bearer and basic as prose Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MxaTpjkJ3QxjaCSrYxXFHW --- rust/cymbal/src/core/code_variables.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/rust/cymbal/src/core/code_variables.rs b/rust/cymbal/src/core/code_variables.rs index d06f5fb28e59..2f9d506f6bfe 100644 --- a/rust/cymbal/src/core/code_variables.rs +++ b/rust/cymbal/src/core/code_variables.rs @@ -19,10 +19,11 @@ const MAX_DEPTH: usize = 12; const SECRET_MIN_LENGTH: usize = 16; const SECRET_MIN_ENTROPY_BITS: f64 = 3.8; const SECRET_MIN_CHAR_CLASSES: u8 = 3; -// Shorter values are prose, such as "the bearer of", and so is a lowercase word of up to -// 15 letters, such as "bearer transportation". A random lowercase token is longer than -// that. A `Basic` credential of any length is still redacted when it decodes to -// `user:password`, e.g. `YTpi` for `a:b`. +// Shorter values are prose, such as "the bearer of", and so is a word of up to 15 letters +// that is lowercase or starts with a capital, such as "bearer transportation" or +// "basic: Configuration". A random token mixes case or is longer than that. A `Basic` +// credential of any length is still redacted when it decodes to `user:password`, e.g. +// `YTpi` for `a:b`. const AUTH_CREDENTIAL_MIN_LENGTH: usize = 8; const AUTH_PROSE_WORD_MAX_LENGTH: usize = 15; const PEM_PRIVATE_KEY_MARKER: &str = "PRIVATE KEY-----"; @@ -221,7 +222,11 @@ fn redact_auth_credential(caps: &Captures) -> String { let credential = &caps[3]; let is_basic_pair = caps[1].eq_ignore_ascii_case("basic") && is_basic_credential(credential); let is_prose_word = credential.len() <= AUTH_PROSE_WORD_MAX_LENGTH - && credential.bytes().all(|b| b.is_ascii_lowercase()); + && credential + .bytes() + .next() + .is_some_and(|b| b.is_ascii_alphabetic()) + && credential.bytes().skip(1).all(|b| b.is_ascii_lowercase()); if !is_basic_pair && (credential.len() < AUTH_CREDENTIAL_MIN_LENGTH || is_prose_word) { return caps[0].to_string(); } @@ -465,6 +470,8 @@ mod tests { "the bearer of bad news", "bearer transportation", "basic: configuration", + "basic: Configuration", + "Basic Configuration loaded", "basicConfig(level=10)", "550e8400-e29b-41d4-a716-446655440000", "da39a3ee5e6b4b0d3255bfef95601890afd80709", From f9eefb9d45ab4b49e8cc277bdf33f3d8528ca9bc Mon Sep 17 00:00:00 2001 From: ablaszkiewicz Date: Tue, 6 Oct 2026 16:40:02 +0200 Subject: [PATCH 7/7] fix(cymbal): treat a word that ends an auth value as a credential Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MxaTpjkJ3QxjaCSrYxXFHW --- rust/cymbal/src/core/code_variables.rs | 75 +++++++++++++++++++------- 1 file changed, 55 insertions(+), 20 deletions(-) diff --git a/rust/cymbal/src/core/code_variables.rs b/rust/cymbal/src/core/code_variables.rs index 2f9d506f6bfe..33b54d8f634a 100644 --- a/rust/cymbal/src/core/code_variables.rs +++ b/rust/cymbal/src/core/code_variables.rs @@ -19,11 +19,8 @@ const MAX_DEPTH: usize = 12; const SECRET_MIN_LENGTH: usize = 16; const SECRET_MIN_ENTROPY_BITS: f64 = 3.8; const SECRET_MIN_CHAR_CLASSES: u8 = 3; -// Shorter values are prose, such as "the bearer of", and so is a word of up to 15 letters -// that is lowercase or starts with a capital, such as "bearer transportation" or -// "basic: Configuration". A random token mixes case or is longer than that. A `Basic` -// credential of any length is still redacted when it decodes to `user:password`, e.g. -// `YTpi` for `a:b`. +// Shorter values are prose, such as "the bearer of". A `Basic` credential of any length is +// still redacted when it decodes to `user:password`, e.g. `YTpi` for `a:b`. const AUTH_CREDENTIAL_MIN_LENGTH: usize = 8; const AUTH_PROSE_WORD_MAX_LENGTH: usize = 15; const PEM_PRIVATE_KEY_MARKER: &str = "PRIVATE KEY-----"; @@ -211,26 +208,56 @@ fn redact_embedded_credentials(value: &str) -> Cow<'_, str> { } else { Cow::Borrowed(value) }; - let redacted = match AUTH_HEADER_CREDENTIALS.replace_all(&value, redact_auth_credential) { - Cow::Owned(redacted) => Some(redacted), - Cow::Borrowed(_) => None, - }; - redacted.map_or(value, Cow::Owned) + match redact_auth_credentials(&value) { + Some(redacted) => Cow::Owned(redacted), + None => value, + } +} + +// `replace_all` cannot see the text after a match, and `is_auth_credential` needs it. +fn redact_auth_credentials(value: &str) -> Option { + let mut redacted = String::new(); + let mut copied_up_to = 0; + for caps in AUTH_HEADER_CREDENTIALS.captures_iter(value) { + let whole = caps.get(0).expect("group 0 is the whole match"); + if !is_auth_credential(&caps, &value[whole.end()..]) { + continue; + } + redacted.push_str(&value[copied_up_to..whole.start()]); + redacted.push_str(&caps[1]); + redacted.push_str(&caps[2]); + redacted.push_str(REDACTED); + copied_up_to = whole.end(); + } + if copied_up_to == 0 { + return None; + } + redacted.push_str(&value[copied_up_to..]); + Some(redacted) } -fn redact_auth_credential(caps: &Captures) -> String { +// A plain word of up to 15 letters is prose only when more text follows it, as in "Basic +// Configuration loaded". At the end of a value, the same word looks exactly like a header +// value such as "Bearer Sunflower", so it counts as a credential. +fn is_auth_credential(caps: &Captures, rest: &str) -> bool { let credential = &caps[3]; - let is_basic_pair = caps[1].eq_ignore_ascii_case("basic") && is_basic_credential(credential); - let is_prose_word = credential.len() <= AUTH_PROSE_WORD_MAX_LENGTH + if caps[1].eq_ignore_ascii_case("basic") && is_basic_credential(credential) { + return true; + } + if credential.len() < AUTH_CREDENTIAL_MIN_LENGTH { + return false; + } + let is_plain_word = credential.len() <= AUTH_PROSE_WORD_MAX_LENGTH && credential .bytes() .next() .is_some_and(|b| b.is_ascii_alphabetic()) && credential.bytes().skip(1).all(|b| b.is_ascii_lowercase()); - if !is_basic_pair && (credential.len() < AUTH_CREDENTIAL_MIN_LENGTH || is_prose_word) { - return caps[0].to_string(); - } - format!("{}{}{REDACTED}", &caps[1], &caps[2]) + let more_text_follows = rest.starts_with(char::is_whitespace) + && rest + .trim_start() + .starts_with(|c: char| c.is_ascii_alphabetic()); + !(is_plain_word && more_text_follows) } fn is_basic_credential(credential: &str) -> bool { @@ -400,6 +427,16 @@ mod tests { json!({"value": "Bearer mongodb+srv://alice:sunflower99@db.example.com/app"}), json!({"value": format!("Bearer {REDACTED}://{REDACTED}@db.example.com/app")}), ), + ( + "one word that ends the value, like a header", + json!({"value": "Bearer Sunflower"}), + json!({"value": format!("Bearer {REDACTED}")}), + ), + ( + "short fragment of a scheme and one word", + json!({"value": "basic: configuration"}), + json!({"value": format!("basic: {REDACTED}")}), + ), ( "colon between the scheme and the credential", json!({"value": format!("Bearer: {bearer_token}")}), @@ -468,9 +505,7 @@ mod tests { "design", "/signup?step=2", "the bearer of bad news", - "bearer transportation", - "basic: configuration", - "basic: Configuration", + "bearer transportation services", "Basic Configuration loaded", "basicConfig(level=10)", "550e8400-e29b-41d4-a716-446655440000",