feat(cymbal): mask python code variables with the sdk rules

[ablaszkiewicz]

Problem

Error tracking shows credentials in Python code_variables when the sending SDK predates a masking rule, and stored frame records keep them.

• An SDK masking fix reaches a team only after that team upgrades.
• Cymbal can only drop code variables, and only for teams in ERROR_TRACKING_DROP_CODE_VARIABLES_TEAM_IDS. Every other team gets them as sent.
• A stored frame record replays the variables of the event that first resolved the frame, so a value stays visible after the sender upgrades.

Changes

• Cymbal masks Python frame code_variables for every team not on the drop list, with the posthog-python SDK's default rules. Users see $$_posthog_redacted_based_on_masking_rules_$$ where an older SDK sent a credential.
• The ported rules cover sensitive names in variables, keys and string values, known vendor formats, high-entropy tokens, PEM keys and URL credentials.
• The port matches fix: mask authorization credentials and signed urls in code variables posthog-python#1015: Bearer/Basic values, the signed-URL sig parameter, URLs scrubbed before Authorization values, and short Basic credentials that decode to user:password.
• One rule is narrower than the SDK's: sk_ must start a word, so task_id and disk_usage keep their values. A team can tune the SDK patterns in its own code but not cymbal's, so a false positive costs more here.
• One rule is stricter than the SDK's: after Bearer/Basic, a plain word of up to 15 letters counts as prose only when more text follows it. A word at the end of a value, such as Bearer Sunflower, looks exactly like a header value, so it is redacted.
• The URL credential match stops at /, ? or #, so a : or @ in a query string no longer wipes the host.
• A string that holds a JSON object or array is masked field by field. The current SDK sends dicts that way, and the plain string rule would blank a whole dict for one password key the SDK already redacted.
• Masking runs at the two existing drop points: on arrival, so new stored frames hold masked values, and after resolution, so frames stored earlier are masked on replay.
• Masking has no setting and is always on.
• Mechanical: drop and mask share one frame loop in each event type.

Note

Frame records already in Postgres keep their raw values until they are replayed. #110760 adds a command that drops them.

How did you test this code?

Test rationale:

• masks_what_the_python_sdk_masks and leaves_benign_values_alone fail when a ported rule stops matching, or starts touching UUIDs, hex digests, paths or prose. No test covered masking, and it is a pure function, so these are unit tests.
• Each unit case also asserts idempotence, because a frame passes through both hooks.
• stores_masked_code_variables and masks_code_variables_replayed_from_stored_frames extend the existing drop tests in tests/event.rs with the same harness. Removing either hook fails exactly its own test.
• The replay test rewrites the stored frame rows to raw values, which stands in for records stored before masking existed.
• Not measured: the cost at production volume. Masking runs a few regex scans per string value, and the SDK caps a value at 1,024 characters.

Performance

Before this PR, cymbal did not mask, so all of the cost below is new. It falls only on Python exception events that carry code variables.

Per event (6 frames, 4.9 KB)	Time
Mask on arrival	12.6 µs
Mask again after resolution, values already masked	9.8 µs
Added per event	≈ 22 µs
Reference: parse the same event JSON, which cymbal already does	10.0 µs

Per variable	Time
Short plain string	0.40 µs
2 KB plain string	6.4 µs
Header pair list	3.3 µs
ASGI scope, 10 headers	7.4 µs
Dict with 40 string keys	15.9 µs
Frame with 11 locals	10.3 µs

• Masking an event takes about 2.2 times as long as parsing its JSON, about 22 µs against 10 µs.
• The dict case costs most, because every key and every value runs the name, secret and credential checks.
• Complex values arrive as JSON strings, so those cases include parsing them.

Method and harness

Release build of f46cc04 on an Apple M4 Pro, rustc 1.91.1. The inputs are the wire form that posthog-python main produces for the same cases as the SDK PR's benchmark, so cymbal masks what it would receive. Median of 7 samples per case. The harness ran as a temporary ignored integration test and is not part of this PR.

use std::{hint::black_box, time::Instant};

use cymbal::{core::code_variables::mask_code_variables, types::event::AnyEvent};
use serde_json::Value;

// Each sample masks a batch of clones, so cloning is not timed. 7 samples per case.
fn per_op<T: Clone>(input: &T, n: usize, mut f: impl FnMut(&mut T)) -> Vec<f64> {
    (0..7)
        .map(|_| {
            let mut batch: Vec<T> = (0..n).map(|_| input.clone()).collect();
            let start = Instant::now();
            for item in batch.iter_mut() {
                f(black_box(item));
            }
            let elapsed = start.elapsed().as_secs_f64();
            black_box(&batch);
            elapsed / n as f64
        })
        .collect()
}

// variables.json, frame.json and event.json hold the wire form that posthog-python
// `main` (2273c7a) produces for the cases in the SDK PR's benchmark (PostHog/posthog-python#1015).
// Cases: mask_code_variables on each variable and on the frame (20,000 per sample),
// AnyEvent::mask_code_variables on the event, fresh and already masked (5,000 per sample),
// and serde_json::from_str::<AnyEvent> on the event JSON as the reference.

Release status

• No feature flag controls this change

Docs update

rust/cymbal/README.md documents the masking and its setting. No public docs change in this PR.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5 (claude-opus-5-5)

• Skills: /writing-tests, /writing-code-comments, /writing-pr-descriptions, /reviewing-with-coderabbit.
• CodeRabbit CLI: skipped. The CLI was signed out, and the person chose to skip setup.
• The first version had a ERROR_TRACKING_MASK_CODE_VARIABLES kill switch. It was removed on request, so masking is unconditional.
• Duplicate search: feat(error-tracking): add command to drop stored frame code variables #110760 drops stored frame code variables with a command. It complements this PR, which masks incoming events and replays.
• The comment-density check reports 6% comment lines. The remaining comments carry rule rationale and pointers that keep the port in step with the SDK.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MxaTpjkJ3QxjaCSrYxXFHW

[trunk-io]

😎 Merged successfully - details.

[github-actions]

🤖 CI report

✅ Trunk lane — non-backend lane (rust:crate:cymbal)

This PR is assigned to the non-backend lane (rust:crate:cymbal). It does not run backend Python tests and may merge in parallel with PRs in other lanes.

🚨 Comment density — 7% of added code lines are comments (40 of 599)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File	Comment lines	Added lines
rust/cymbal/src/core/code_variables.rs	37	492
rust/cymbal/tests/event.rs	2	70
rust/cymbal/src/modes/processing/stages/http_pipeline.rs	1	3

This check does not block merging. It updates on every push and clears when the share drops.

[greptile-apps]

[Medium risk] Adds server-side masking of Python exception variables.

The PR appears safe to merge, though the Authorization matcher can unnecessarily remove diagnostic text.

_{Reviews (2) · Last reviewed commit: "fix(cymbal): port the sdk url-first, pro..."}

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The change adds recursive masking for sensitive values in Python-frame code_variables. Events for teams configured to drop these values still have them removed; other eligible events are masked before preprocessing and after remote resolution. Tests cover initial event processing, stored frame records, and replayed frame data. The documentation states that existing Postgres records are not changed.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to 45e8f

Masking of Python code variables can leave short, alphabetic Bearer tokens visible in error-tracking frames, which weakens the privacy guarantee this change is meant to provide. Tighten the prose exemption before merging. Also confirm that the URL credential pattern stops at query and fragment characters.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 45e8f

The change reduces credential exposure without expanding access or privileges. Some credential forms still pass through, as they did before, and previously stored secrets are not removed. These limitations prevent treating masking as complete credential protection.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The masking policy applies to every non-drop-list team. Incoming traversal is platform-agnostic for object frames containing code_variables, while typed raw-frame traversal selects Python frames and resolved traversal covers all resolved frames. The inspected change adds sanitization, not access to another team's records or new privileges.

Security Findings and Attack Paths

• observed — The retained sensitive-data finding remains supported. An event-supplied value such as Bearer abcdefgh under a neutral variable name survives: name and known-format rules do not match, the whole string is below the secret-length threshold, and the authentication callback classifies the credential as prose. Such credentials can continue into event output and frame storage. Short Basic values decoding to user:password are redacted, but that countercontrol does not cover this Bearer case. The same value already passed through at the PR base, so the demonstrated exposure predates this PR rather than being introduced or widened by it.

Trust Boundaries and Controls

• observed — Sender-provided frame values and reused stored-frame values are both treated as inputs requiring sanitization. Replay masking protects the emitted event but does not remove raw credentials from historical database rows, so it is not a substitute for storage cleanup or database access controls.

Resilience and Maintainability Implications

• observed — Masking mutates the owned event synchronously before preprocessing clones its fallback copy. Handled failures return that masked copy, suppression returns no event, and stage errors propagate. Tests assert stable repeated masking for covered cases. No cancellation or process-crash integration validation was supplied.

Hardening Proposals

• proposed — Consider distinguishing explicit authentication contexts from ordinary prose so short or lowercase credentials can be redacted without broadly removing benign text. Add regression cases for those credentials and make any intentional SDK-policy divergence explicit.

🚥 Pre-merge checks | ✅ 1

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description covers the problem, user-visible changes, test rationale, release status, documentation, and agent context. It does not include the required before-and-after Mermaid diagrams for the p…

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)

rust/cymbal/src/core/code_variables.rs-80-81 (1)

80-81: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Stop the URL userinfo match at ? and #.

[^/\s]*@ excludes / but not ? or #. Take https://host?next=a:b@c. The pattern captures host?next=a:b as userinfo. That text contains :, so the host and query are replaced with REDACTED. The opposite case also fails: for https://u:p@host?x=a@b, the greedy match runs to the last @. The redaction then removes the host, while the credential stays redacted. Exclude ? and # from the userinfo class. Keep the greedy match up to the last @ in the authority, so a password that contains @ is fully masked.

Proposed fix

-    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());

Based on learnings: the userinfo pattern "must not span past '/', '?', or '#'".

Source: Learnings

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
• Review profile: QUIET
• Plan: Enterprise
• Run ID: 8e1b3cda-9494-46e6-8f7b-ef7d7c4d9ae0

📥 Commits

Reviewing files that changed from the base of the PR and between aa436fd and 9040ea8.

📒 Files selected for processing (10)

• rust/cymbal/README.md
• rust/cymbal/src/core/code_variables.rs
• rust/cymbal/src/core/mod.rs
• rust/cymbal/src/modes/processing/config.rs
• rust/cymbal/src/modes/processing/stages/http_pipeline.rs
• rust/cymbal/src/modes/processing/stages/resolution/mod.rs
• rust/cymbal/src/modes/processing/types/event.rs
• rust/cymbal/src/modes/processing/types/exception_event.rs
• rust/cymbal/tests/common/mod.rs
• rust/cymbal/tests/event.rs

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

[hosthog]

HostHog preview — posthog-desktop-web

The previews for this PR have been torn down and no longer serve.

[trunk-io]

   

View Full Report ↗︎ ⋅ Docs

[ablaszkiewicz]

CodeRabbit's outside-diff note on URL_CREDENTIALS is fixed in b15a18f. The userinfo match now stops at /, ? or #, so https://db.example.com?next=a:b@c stays unchanged, and https://app:pass@db.example.com?next=a@b keeps its host and query while the credential is redacted. Both are new test cases.

[coderabbitai]

🧹 Nitpick comments (1)

rust/cymbal/src/core/code_variables.rs (1)

28-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the comment that records a change from the SDK patterns.

The sentence beginning "One difference" explains how this code differs from an earlier or external version. The guideline says comments must not record how the code got here. Keep only the durable fact: sk_ must start a word, so task_id and disk_usage keep their values. Put the SDK comparison in the PR description.

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
• Review profile: QUIET
• Plan: Enterprise
• Run ID: 02aecd37-0186-4814-8a33-f7532578d5f2

📥 Commits

Reviewing files that changed from the base of the PR and between 2e9f862 and b15a18f.

📒 Files selected for processing (1)

• rust/cymbal/src/core/code_variables.rs

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

[parameterai]

Risk: No findings

This delta fixes the prior finding: a single capitalized (or lowercase) word of 8-15 letters after Bearer/Basic is now redacted when it ends the value, and kept as prose only when more alphabetic text follows, via the new is_auth_credential/redact_auth_credentials rewrite loop. No new vulnerabilities found in the change; the only behavioral divergences I could construct are in the over-masking direction, which is safe.

_{Sentinel reviewed f9eefb9 · Review settings}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
• Review profile: QUIET
• Plan: Enterprise
• Run ID: 3df53658-fee1-4466-9b46-b091a9bcf4e2

📥 Commits

Reviewing files that changed from the base of the PR and between f46cc04 and 45e8fbf.

📒 Files selected for processing (1)

• rust/cymbal/src/core/code_variables.rs

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.

[deployment-status-posthog]

Deploy status

Environment	Status	Deployed At	Workflow
dev	✅ Deployed	2026-10-06 16:55 UTC	Run
prod-us	✅ Deployed	2026-10-06 17:10 UTC	Run
prod-eu	✅ Deployed	2026-10-06 17:10 UTC	Run