diff --git a/rust/cymbal/README.md b/rust/cymbal/README.md index b21c3bcb24b9..bcf3046b3d75 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. + +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 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..33b54d8f634a --- /dev/null +++ b/rust/cymbal/src/core/code_variables.rs @@ -0,0 +1,526 @@ +//! 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 base64::Engine; +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 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-----"; +// 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. 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|(?:^|[^a-z0-9])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. 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()); + +// 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 +// 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}$") + .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(); + 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, &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, &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, &mut next_placeholder); + } + if key_matches_mask { + value = Value::String(REDACTED.to_string()); + } else { + mask_value(&mut value, depth + 1); + } + result.insert(out_key, value); + } + result +} + +// 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; + } + } +} + +// 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 = if value.contains("://") { + URL_CREDENTIALS.replace_all(value, redact_url_credential) + } else { + Cow::Borrowed(value) + }; + 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) +} + +// 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]; + 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()); + 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 { + 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]; + 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}")}), + ), + ( + "credential of lowercase letters only", + 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")}), + ), + ( + "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}")}), + 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"}), + 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 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}}), + 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", + "bearer transportation services", + "Basic Configuration loaded", + "basicConfig(level=10)", + "550e8400-e29b-41d4-a716-446655440000", + "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}"); + } + } +} 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/stages/http_pipeline.rs b/rust/cymbal/src/modes/processing/stages/http_pipeline.rs index 86286925f66e..f2452082adbe 100644 --- a/rust/cymbal/src/modes/processing/stages/http_pipeline.rs +++ b/rust/cymbal/src/modes/processing/stages/http_pipeline.rs @@ -34,12 +34,14 @@ 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 batch = batch.map( |mut event, ()| { if drop_team_ids.contains(&event.team_id) { event.drop_code_variables(); + } 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 0811e27c0155..14bfca2586c5 100644 --- a/rust/cymbal/src/modes/processing/stages/resolution/mod.rs +++ b/rust/cymbal/src/modes/processing/stages/resolution/mod.rs @@ -81,6 +81,8 @@ impl Stage for ResolutionStage { item.map(|mut event| { if drop_team_ids.contains(&event.team_id()) { event.drop_code_variables(); + } else { + 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/event.rs b/rust/cymbal/tests/event.rs index b35aaa9b4415..0c6f14b68bcc 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,84 @@ 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.clone()); + + let (status, _): (_, SuccessResponse) = harness + .post_event_with_config(&python_event_with_database_url(), |_| {}) + .await; + assert!(status.is_success()); + // 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"); + 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());