Repository navigation
Sentry: scrub file paths / PII / secrets from events before send (beforeSend/beforeBreadcrumb) - #5598
Conversation
Add a content scrubber to cmux's Sentry config (macOS app + CLI) so error messages, exception values, breadcrumbs, request/transaction data, the user context, and stack-frame file paths are redacted before they leave the device for sentry.io. The pure SentryScrubber (CmuxFoundation) redacts home/user paths (the real home dir plus any /Users/<name>/ or /home/<name>/ prefix, which also covers build-machine stack-frame paths), emails, and secrets (Bearer tokens, sk-/ghp- style keys, JWTs, token=/password= assignments, AWS access keys). It leaves grouping-relevant fields (exception type, fingerprint, frame function/module/ lineNumber) intact. SentryEventScrubber wires it into beforeSend/beforeBreadcrumb and walks message.formatted, exception values, exception/thread/event stack frames, request URL/query/cookies/headers, transaction, serverName, user fields, tags, extra, and context. The CLI previously set sendDefaultPii = true; flipped to false for defense-in-depth (the scrubber also redacts any user fields that slip in). iOS ships no Sentry SDK, so there is nothing to scrub there. Tests: 24 SentryScrubber unit tests (CmuxFoundation, swift test) plus SentryEventScrubberTests covering the Event/Breadcrumb glue including the formatted-only message and context-dictionary leak paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a compiled regex helper and text scrubber, applies it to Sentry Event and Breadcrumb payloads to remove PII, and wires the scrubber into app startup and Sentry hooks. ChangesDefense-in-Depth Sentry Event Scrubbing
Sequence DiagramsequenceDiagram
participant Event as Sentry Event/Breadcrumb
participant SentryEventScrubber as SentryEventScrubber
participant SentryScrubber as SentryScrubber
participant SentrySDK as Sentry SDK
Event->>SentryEventScrubber: scrub(event) / scrub(breadcrumb)
SentryEventScrubber->>SentryScrubber: scrub(message/path/email/token)
SentryScrubber->>SentryScrubber: apply redaction patterns
SentryScrubber-->>SentryEventScrubber: redacted text
SentryEventScrubber->>SentryEventScrubber: rebuild event/breadcrumb
SentryEventScrubber-->>SentrySDK: scrubbed event/breadcrumb
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (17 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d960ea197d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (key, value) in dictionary { | ||
| output[key] = scrub(value: value) |
There was a problem hiding this comment.
Because the dictionary scrubber ignores key and only scans the value string, structured Sentry payloads like scope.setContext(value: ["token": "abcdef0123456789secretvalue"], key: ...), extra, or breadcrumb data still send the raw secret when the value does not independently match a prefixed token/JWT/API-key regex. This directly affects the new event scrubber paths that pass context/extra/data through this method, so key names such as token, password, or api_key need to force the corresponding value to <redacted-secret>.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR wires a defense-in-depth Sentry scrubbing pipeline into both the macOS app and CLI before any telemetry leaves the device, preventing local paths, emails, tokens, and user-identity fields from reaching hosted sentry.io.
Confidence Score: 5/5Safe to merge — scrubbing pipeline is comprehensive, all documented fields are covered, and the tracing-disable decision is clearly justified. Every sensitive Sentry field documented in the PR contract is wired through the scrubber. The pure SentryScrubber is testable in isolation; 35 unit tests plus the event-glue suite give strong behavioral coverage. Regex patterns carry upstream provenance pins. The only open item is the captureGroup lower-bound guard (flagged in a prior thread), which has no current callsite passing 0. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant App as AppDelegate / CLISocketSentryTelemetry
participant SDK as Sentry SDK
participant EScrubber as SentryEventScrubber
participant SScrubber as SentryScrubber
participant Sentry as sentry.io
App->>SDK: SentrySDK.start(options)
Note over App,SDK: registers beforeSend, beforeBreadcrumb, beforeSendSpan
SDK->>EScrubber: beforeBreadcrumb(breadcrumb)
EScrubber->>SScrubber: scrub message + data
SScrubber-->>EScrubber: redacted values
EScrubber-->>SDK: scrubbed breadcrumb
SDK->>EScrubber: beforeSend(event)
EScrubber->>SScrubber: scrub message, exception, frames, request, tags/extra/context
EScrubber->>EScrubber: drop user identity fields
SScrubber-->>EScrubber: redacted values
EScrubber-->>SDK: scrubbed event
SDK->>Sentry: POST scrubbed payload
Reviews (13): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /// `tags`, `extra`, and breadcrumb `message` / `data` — while leaving | ||
| /// grouping-relevant fields (exception `type`, fingerprint, frame `function` / | ||
| /// `module` / `lineNumber`) untouched so Sentry issue grouping is unaffected. | ||
| struct SentryEventScrubber { |
There was a problem hiding this comment.
Sendable conformance on captured type
SentryEventScrubber is captured by both beforeSend and beforeBreadcrumb closures, which the Sentry SDK calls from whatever dispatch queue produced the event — potentially crossing actor boundaries from the main-actor context where SentrySDK.start runs. In Swift 6 strict-concurrency mode this will produce a warning (or error) because the closure capture requires the value to be Sendable. Adding the conformance is safe: the single stored property is SentryScrubber which is already declared Sendable, so the struct synthesises it without changes.
| struct SentryEventScrubber { | |
| struct SentryEventScrubber: Sendable { |
| user.email = scrubber.scrub(optional: user.email) | ||
| user.username = scrubber.scrub(optional: user.username) | ||
| user.name = scrubber.scrub(optional: user.name) |
There was a problem hiding this comment.
user.userId not scrubbed
The Sentry User type exposes a userId: String? field (distinct from username). The PR description lists the scrubbed user fields as "email/username/name/data, ipAddress dropped" but userId is absent. If cmux ever sets userId to an email address or a path-bearing value (e.g. via scope.setUser), it will leave the device unscrubbed. Given that the rest of the user object is being walked for defense-in-depth, routing userId through the scrubber keeps the coverage consistent.
| user.email = scrubber.scrub(optional: user.email) | |
| user.username = scrubber.scrub(optional: user.username) | |
| user.name = scrubber.scrub(optional: user.name) | |
| user.userId = scrubber.scrub(optional: user.userId) | |
| user.email = scrubber.scrub(optional: user.email) | |
| user.username = scrubber.scrub(optional: user.username) | |
| user.name = scrubber.scrub(optional: user.name) |
| public func captureGroup(_ index: Int) -> String? { | ||
| guard index < result.numberOfRanges else { return nil } |
There was a problem hiding this comment.
captureGroup guard doesn't enforce the documented 1-based contract
The doc-comment declares the parameter as a "1-based capture group index", but the guard index < result.numberOfRanges lets index = 0 through — which returns the text of the entire match rather than nil. While captureGroup(0) is never called today, the mismatch between contract and implementation is a maintenance hazard: a future caller following the docs would expect nil and receive unexpected match text. Adding an explicit lower-bound guard makes the 1-based promise machine-checked.
| public func captureGroup(_ index: Int) -> String? { | |
| guard index < result.numberOfRanges else { return nil } | |
| public func captureGroup(_ index: Int) -> String? { | |
| guard index >= 1, index < result.numberOfRanges else { return nil } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Packages/CmuxFoundation/Sources/CmuxFoundation/SentryRegexPattern.swift`:
- Around line 51-54: The public initializer SentryRegexPattern.init currently
uses try! to create self.regex which can crash on invalid patterns; change the
API to either make the initializer throwing (declare public init(_ pattern:
String, options: NSRegularExpression.Options = [.caseInsensitive]) throws and
use try to construct NSRegularExpression then assign to regex, propagating the
error to callers) or, if external construction isn’t intended, reduce the
initializer’s access level (make it internal or fileprivate) and handle the
NSRegularExpression construction with a failable/guard path instead of try!;
update any callers of SentryRegexPattern.init to either catch/propagate the
thrown error or to use the new internal factory so the code no longer
force-unwraps NSRegularExpression.
- Around line 31-35: SentryRegexPattern.Match.captureGroup(_:) must validate
that index is within 0 ..< result.numberOfRanges (not just <) to avoid passing
negative indexes into NSTextCheckingResult.range(at:); add a guard that index >=
0 && index < result.numberOfRanges before calling range(at:) and return nil for
out-of-range values. For SentryRegexPattern.init(_:options:) remove the
force-try and either make the initializer throw (public init(...) throws) and
propagate NSRegularExpression initialization errors, or make the initializer
non-public if it must remain infallible; replace try! NSRegularExpression(...)
with a throwing call and update callers accordingly.
In `@Sources/SentryEventScrubber.swift`:
- Around line 40-46: serverName and user identity fields are being
pattern-scrubbed via scrubber.scrub(optional:) but must be fully redacted or
replaced with a stable placeholder; update the SentryEventScrubber logic to drop
or replace event.serverName and user identity fields (event.user?.username and
event.user?.name) rather than calling scrubber.scrub(optional:), e.g. set them
to nil or a constant placeholder inside the same branch where exceptions and
other fields are scrubbed (refer to event.serverName, event.user, and
scrubber.scrub(optional:) to locate the code), and update the expectations in
cmuxTests/SentryEventScrubberTests.swift to reflect the new redaction behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 49821527-4afe-48bd-8095-4a5c9f3dae32
📒 Files selected for processing (8)
CLI/cmux.swiftPackages/CmuxFoundation/Sources/CmuxFoundation/SentryRegexPattern.swiftPackages/CmuxFoundation/Sources/CmuxFoundation/SentryScrubber.swiftPackages/CmuxFoundation/Tests/CmuxFoundationTests/SentryScrubberTests.swiftSources/AppDelegate.swiftSources/SentryEventScrubber.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SentryEventScrubberTests.swift
| public init(_ pattern: String, options: NSRegularExpression.Options = [.caseInsensitive]) { | ||
| // swiftlint:disable:next force_try | ||
| self.regex = try! NSRegularExpression(pattern: pattern, options: options) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant portion of the file with line numbers
FILE="Packages/CmuxFoundation/Sources/CmuxFoundation/SentryRegexPattern.swift"
if [ -f "$FILE" ]; then
echo "=== $FILE (lines 1-140) ==="
nl -ba "$FILE" | sed -n '1,140p'
else
echo "Missing: $FILE"
fi
# Find all initializations / uses
echo "=== rg: SentryRegexPattern init/usage ==="
rg -n "SentryRegexPattern\s*\(" -S . || true
rg -n "init\(_ pattern: String, options:" -S "$FILE" || trueRepository: manaflow-ai/cmux
Length of output: 215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Packages/CmuxFoundation/Sources/CmuxFoundation/SentryRegexPattern.swift"
nl -ba "$FILE" | sed -n '1,200p'
echo "---- usages ----"
rg -n "SentryRegexPattern\s*\(" -S . || trueRepository: manaflow-ai/cmux
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Packages/CmuxFoundation/Sources/CmuxFoundation/SentryRegexPattern.swift"
nl -ba "$FILE" | sed -n '1,200p'
echo "---- usages ----"
rg -n "SentryRegexPattern\s*\(" -S . || trueRepository: manaflow-ai/cmux
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Packages/CmuxFoundation/Sources/CmuxFoundation/SentryRegexPattern.swift"
echo "=== file exists? ==="
ls -l "$FILE" || true
echo
echo "=== file contents (1-120) ==="
sed -n '1,120p' "$FILE" || true
echo
echo "=== try! usage in file ==="
rg -n "try!" "$FILE" || true
echo
echo "=== initializer signature in file ==="
rg -n "init\\(.*pattern: String" "$FILE" || true
echo
echo "=== usages across repo (max 200 lines) ==="
rg -n "SentryRegexPattern\\s*\\(" -S . | head -n 200 || trueRepository: manaflow-ai/cmux
Length of output: 5524
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Packages/CmuxFoundation/Sources/CmuxFoundation/SentryRegexPattern.swift"
echo "=== file exists? ==="
ls -l "$FILE" || true
echo
echo "=== file contents (1-120) ==="
sed -n '1,120p' "$FILE" || true
echo
echo "=== try! usage in file ==="
rg -n "try!" "$FILE" || true
echo
echo "=== initializer signature in file ==="
rg -n "init\\(.*pattern: String" "$FILE" || true
echo
echo "=== usages across repo (max 200 lines) ==="
rg -n "SentryRegexPattern\\s*\\(" -S . | head -n 200 || trueRepository: manaflow-ai/cmux
Length of output: 5524
Avoid try! in SentryRegexPattern’s public initializer
public init(_ pattern: String, ...) force-unwraps NSRegularExpression, so invalid runtime input can crash the process. Change the initializer to throws or make the initializer non-public/internal if external callers aren’t meant to construct arbitrary patterns.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Packages/CmuxFoundation/Sources/CmuxFoundation/SentryRegexPattern.swift`
around lines 51 - 54, The public initializer SentryRegexPattern.init currently
uses try! to create self.regex which can crash on invalid patterns; change the
API to either make the initializer throwing (declare public init(_ pattern:
String, options: NSRegularExpression.Options = [.caseInsensitive]) throws and
use try to construct NSRegularExpression then assign to regex, propagating the
error to callers) or, if external construction isn’t intended, reduce the
initializer’s access level (make it internal or fileprivate) and handle the
NSRegularExpression construction with a failable/guard path instead of try!;
update any callers of SentryRegexPattern.init to either catch/propagate the
thrown error or to use the new internal factory so the code no longer
force-unwraps NSRegularExpression.
Source: Coding guidelines
Autoreview P1: the user context and request cookies are fields the scrubber explicitly claims to protect, but pattern-scrubbing them leaks the common case. A username/display name rarely looks like an email/path/secret, and a normal session cookie (session=, sid=, auth=) has no secret-pattern name. Drop user.userId/email/username/name/ipAddress/geo and request.cookies wholesale instead of pattern-scrubbing. User.data and request URL/query/headers still go through the scrubber. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7fbe98aa0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // …), so pattern-scrubbing the value cannot reliably catch every secret. | ||
| request.cookies = nil | ||
| if let headers = request.headers { | ||
| request.headers = headers.mapValues { scrubber.scrub($0) } |
There was a problem hiding this comment.
When event.request.headers contains an Authorization header whose value is a non-Bearer scheme such as Basic dXNlcjpwYXNz, this code passes only the header value to the scrubber. The scrubber's authorization regex expects the literal Authorization: prefix, so these credentials remain in the Sentry payload even though this path is supposed to scrub request headers; use the header key to drop/redact sensitive headers before applying value-only patterns.
Useful? React with 👍 / 👎.
Autoreview P2: two more egress paths the scrubber claimed to cover but missed. capture(error:) copies NSError.userInfo (NSFilePathErrorKey, URLs, etc.) into exception.mechanism.data, and event.debugMeta[].codeFile is the on-disk path to the loaded binary, which carries /Users/<name>/ for dev/home-launched builds. Scrub mechanism desc/helpLink/data (preserving mechanism.type for grouping) and debug-image codeFile. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/SentryEventScrubber.swift`:
- Around line 136-138: The request header scrubbing currently maps all headers
through scrubber.scrub but does not remove "Cookie" headers, so Cookie: ...
values can leak; update the logic in SentryEventScrubber (the block handling
request.headers and using mapValues { scrubber.scrub($0) }) to first remove any
header whose name equals "cookie" case-insensitively, then apply scrubber.scrub
to the remaining headers, and ensure request.headers is set to that
filtered+scrubbed dictionary. Add a unit test that supplies a Cookie header via
request.headers (e.g., "Cookie": "session=abc...") and asserts the Cookie header
has been dropped from the scrubbed request (header-based test to complement
existing request.cookies tests).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 24e23526-a6ea-4232-8b0b-1fe53d05e77e
📒 Files selected for processing (2)
Sources/SentryEventScrubber.swiftcmuxTests/SentryEventScrubberTests.swift
Autoreview P1: the recursive dictionary scrubber only applied value regexes, so a secret keyed by a sensitive name (context["auth"]["token"], extra, breadcrumb data, or a Cookie/Authorization header) passed through when its raw value did not match a standalone secret pattern. Treat sensitive keys (token/password/secret/api_key/authorization/cookie/…, case- and separator-insensitive) as a trust boundary: redact their string values wholesale before recursing. Route request headers through the key-aware dictionary scrubber too. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/SentryEventScrubber.swift (1)
44-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
serverNameshould be nil'd, not pattern-scrubbed.The previous review flagged that
serverName(likeuser.username/user.name) is an inherently identifying value that rarely matches secret/email/path patterns. A hostname like"lawrence-macbook.local"passes through unredacted. The user fields at lines 173-178 were fixed, butserverNamewas not.🛡️ Suggested fix
- event.serverName = scrubber.scrub(optional: event.serverName) + event.serverName = nil🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SentryEventScrubber.swift` at line 44, Replace pattern-scrubbing of serverName with niling: in SentryEventScrubber (where event.serverName is currently set via scrubber.scrub(optional: event.serverName)) change the assignment to set event.serverName = nil so hostnames like "lawrence-macbook.local" are removed entirely rather than passed through; mirror the same approach used for user.username/user.name scrub niling to keep behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/SentryEventScrubber.swift`:
- Line 44: Replace pattern-scrubbing of serverName with niling: in
SentryEventScrubber (where event.serverName is currently set via
scrubber.scrub(optional: event.serverName)) change the assignment to set
event.serverName = nil so hostnames like "lawrence-macbook.local" are removed
entirely rather than passed through; mirror the same approach used for
user.username/user.name scrub niling to keep behavior consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 133dbf27-5325-43a0-b74b-d62bb2ed7ba0
📒 Files selected for processing (2)
Sources/SentryEventScrubber.swiftcmuxTests/SentryEventScrubberTests.swift
…llowlist Autoreview P1 x2, fixed by inverting the default instead of chasing fields: - Sensitive dictionary keys now drop their value wholesale regardless of shape (string, array, or nested dict), not just strings. - Stack frames now scrub contextLine / preContext / postContext / vars (source snippets and local variable values), plus thread.name and event.logger. The contract is now "scrub every free-text field; preserve only an explicit grouping allowlist" (exception/mechanism type, fingerprint, frame function/module/lineNumber/addresses, level, environment, release, dist, modules), documented on the type. event.error is not serialized by the SDK (converted to exceptions/mechanism.data first), so it needs no handling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cmux-policy P2 (Aziz file-organization): one major type per file. Extract the nested capture-aware match type out of SentryRegexPattern into SentryRegexMatch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/SentryEventScrubber.swift (1)
85-87:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
event.tagsbypasses sensitive-key redaction.This path only scrubs tag values by content. A tag such as
["token": "session123"]or["authorization": "abc"]survives if the value itself does not match a secret regex, even thoughSentryScrubber.scrub(dictionary:)already treats those keys as the redaction boundary. Route tags through the same key-aware dictionary scrubber used forextra,context, and headers, and add a matching regression case.Suggested fix
if let tags = event.tags { - event.tags = tags.mapValues { scrubber.scrub($0) } + event.tags = scrubber.scrub(dictionary: tags) + .compactMapValues { $0 as? String } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SentryEventScrubber.swift` around lines 85 - 87, The tags branch in SentryEventScrubber.swift currently only maps values with scrubber.scrub($0) which ignores sensitive tag keys; change it to route event.tags through the same key-aware dictionary scrubber (call SentryScrubber.scrub(dictionary:) or the instance method scrub(dictionary:) on scrubber) so keys like "token" or "authorization" are redacted, i.e., replace the tags.mapValues usage with a call that scrubber.scrub(dictionary: event.tags) (or equivalent instance method) and update/ add a regression test that submits an event with sensitive tag keys (e.g., ["token":"session123"]) and asserts the tag value is redacted.
♻️ Duplicate comments (1)
Sources/SentryEventScrubber.swift (1)
53-53:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDrop
event.serverNamewholesale.
serverNameis identifying even when it contains no path, email, or token pattern, so hostnames like a user-named Mac or internal node name still leave the device throughbeforeSend. Pattern-scrubbing is not enough here; treat it like the other identity fields and clear it.Suggested fix
- event.serverName = scrubber.scrub(optional: event.serverName) + event.serverName = nil🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SentryEventScrubber.swift` at line 53, The Sentry event's serverName is still being scrubbed instead of removed, so hostnames can leak; update the SentryEventScrubber logic to drop serverName entirely by setting event.serverName = nil (instead of calling scrubber.scrub(optional: event.serverName)) so it is cleared like other identity fields; locate the assignment to event.serverName in Sources/SentryEventScrubber.swift and replace the scrub call with a nil assignment ensuring no residual pattern-scrubbing remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Packages/CmuxFoundation/Sources/CmuxFoundation/SentryScrubber.swift`:
- Around line 150-156: The scrub(dictionary:) path uses Self.isSensitiveKey(key)
which currently relies on substring contains(marker) and thus over-redacts keys
like "author" or "tokenizer"; update isSensitiveKey to match against key
components and exact alias forms only (e.g., split camelCase/snake/kebab and
compare whole components and canonical exact names such as "password", "token",
"api_key", "auth-token", etc.) instead of raw substring containment, and ensure
scrub(dictionary:) and the same logic used in scrub(value:) call the revised
isSensitiveKey; add a regression unit test verifying a non-secret key like
"author" (and variations) is not redacted while actual secret keys are redacted.
---
Outside diff comments:
In `@Sources/SentryEventScrubber.swift`:
- Around line 85-87: The tags branch in SentryEventScrubber.swift currently only
maps values with scrubber.scrub($0) which ignores sensitive tag keys; change it
to route event.tags through the same key-aware dictionary scrubber (call
SentryScrubber.scrub(dictionary:) or the instance method scrub(dictionary:) on
scrubber) so keys like "token" or "authorization" are redacted, i.e., replace
the tags.mapValues usage with a call that scrubber.scrub(dictionary: event.tags)
(or equivalent instance method) and update/ add a regression test that submits
an event with sensitive tag keys (e.g., ["token":"session123"]) and asserts the
tag value is redacted.
---
Duplicate comments:
In `@Sources/SentryEventScrubber.swift`:
- Line 53: The Sentry event's serverName is still being scrubbed instead of
removed, so hostnames can leak; update the SentryEventScrubber logic to drop
serverName entirely by setting event.serverName = nil (instead of calling
scrubber.scrub(optional: event.serverName)) so it is cleared like other identity
fields; locate the assignment to event.serverName in
Sources/SentryEventScrubber.swift and replace the scrub call with a nil
assignment ensuring no residual pattern-scrubbing remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d198aa7c-bced-4b89-91e2-88324d974737
📒 Files selected for processing (4)
Packages/CmuxFoundation/Sources/CmuxFoundation/SentryScrubber.swiftPackages/CmuxFoundation/Tests/CmuxFoundationTests/SentryScrubberTests.swiftSources/SentryEventScrubber.swiftcmuxTests/SentryEventScrubberTests.swift
Autoreview P2 x2: - event.tags went through content-only scrubbing, so a tag keyed `access_token` with an opaque value leaked. Route tags through the key-aware dictionary scrubber. - The free-text assignment regex only matched bare markers (`secret=`, `api_key=`); env/config names embedding the marker (AWS_SECRET_ACCESS_KEY=, SECRET_ACCESS_KEY=, MY_API_KEY=) slipped through in messages / breadcrumbs / stack context. Allow identifier characters around the marker word. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4db5223d54
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case let dictionary as [String: Any]: | ||
| return scrub(dictionary: dictionary) | ||
| case let array as [Any]: | ||
| return array.map { scrub(value: $0) } | ||
| default: | ||
| return value |
There was a problem hiding this comment.
When Sentry payload dictionaries contain URL/NSURL objects rather than strings (for example NSError.userInfo entries such as a failing file/HTTP URL copied into mechanism.data), this switch falls through to default and leaves the object untouched, so Sentry can still serialize an absolute URL containing /Users/<name>/... or token query parameters. Please normalize URL values to their string form and run them through the same scrubber before returning.
Useful? React with 👍 / 👎.
Autoreview P1: the macOS app samples transactions (tracesSampleRate = 0.1) and Sentry processes child spans through beforeSendSpan, not beforeSend. Auto network/file-I/O spans serialize URLs (http.query, url) and file paths in spanDescription and data, which bypassed the event/breadcrumb scrubber. Add a span scrubber (description + key-aware data/tags) and wire options.beforeSendSpan in the macOS init. The CLI sets tracesSampleRate = 0, so it emits no spans and needs no span hook. Operation/origin (span kind) are preserved. A dedicated span unit test is not practical: the concrete Span is only constructable via a live SDK transaction, which is global SDK state. The span path routes through scrub(dictionary:)/scrub(optional:), both covered by the SentryScrubber package tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e031784. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e031784b19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| static let secretPatterns: [SentryRegexPattern] = [ | ||
| // Bearer <token> | ||
| SentryRegexPattern(#"(Bearer\s+)[A-Za-z0-9\-._~+/]+=*"#), | ||
| // Authorization: <scheme> <token> (Basic / Digest / token / etc.) | ||
| SentryRegexPattern(#"(Authorization:\s*\w+\s+)\S+"#), |
There was a problem hiding this comment.
When a Sentry field routed through scrub(_:) contains a URL with RFC 3986 user-info, such as https://alice:hunter2@example.com/path, none of these secret patterns match because there is no password= key or Authorization: prefix. This leaves credentials in event.request.url, breadcrumb data, or context URL strings even though this scrubber is intended to sanitize URLs before send; add URL-aware redaction for the user[:password]@host component.
Useful? React with 👍 / 👎.
…y set Autoreview P1: the string assignment regex used a narrower marker set than the key-aware dictionary path, so credentials like auth=, session_id=, cookie=, or private_key= in raw query strings / http.query span fields slipped through while the same names were redacted as dictionary keys. Expand the assignment alternation to match the dictionary sensitive markers (auth, cookie, credential(s), session_id, private_key, bearer, …) so a credential is redacted whether it arrives as a dict entry or as raw text. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…race path Autoreview P1 (verified against sentry-cocoa 9.3 source): SentryTransaction serialize() merges the root trace's data/tags/description into the payload AFTER beforeSend runs, and beforeSendSpan only iterates child spans, not the root tracer. SentryTransaction/SentryTracer live in the SDK's internal include/ headers (not the public umbrella), so the root trace is not reachable to scrub through the public Sentry API. cmux does not consume these performance traces (no manual transactions or span.setData), so disable transactions on macOS (tracesSampleRate = 0). This removes the un-scrubbable egress path while keeping crash, error, and app-hang reporting, which are independent of the trace sample rate. The beforeSendSpan hook stays as belt-and-suspenders if tracing is ever re-enabled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e9cbb5570
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // `context` carries the per-key dictionaries set via | ||
| // `scope.setContext(value:key:)`, where cmux puts cwd / path / URL | ||
| // data; scrub every nested value. | ||
| event.context = context.mapValues { scrubber.scrub(dictionary: $0) } |
There was a problem hiding this comment.
When a caller uses the Sentry context key itself to describe a secret-bearing payload, e.g. scope.setContext(value: ["value": "dXNlcjpwYXNz"], key: "auth") or "credentials", this path ignores that top-level key and only scrubs the nested dictionary by its inner keys/content. Opaque credentials under generic inner names therefore still leave the device even though the scrubber already treats auth/credential as sensitive keys elsewhere; apply the same key-aware boundary to the context key before recursing.
Useful? React with 👍 / 👎.
Autoreview P2: scrub(value:) passed non-string/dict/array values through unchanged, but Sentry serializes unsupported Foundation objects (notably URL/NSURL, which carry a file path) to their description after beforeSend. A URL(fileURLWithPath: "/Users/alice/secret.txt") in context/extra/breadcrumb data / frame vars / span data would emit the unscrubbed path. Preserve safe scalars (NSNumber/Bool/Int/Double, Date, Data, NSNull); convert URL/NSURL and any other object to its string form and scrub that. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62b4370ec1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private func redactPaths(in text: String) -> String { | ||
| var result = text | ||
| if !homeDirectory.isEmpty, homeDirectory != "/" { | ||
| result = result.replacingOccurrences(of: homeDirectory, with: Self.redactedHomePath(for: homeDirectory)) |
There was a problem hiding this comment.
When the runtime home is a prefix of another username (for example homeDirectory == "/Users/ann" and a stack frame or NSError path contains /Users/anna/project), this unbounded string replacement rewrites it to /Users/<redacted>a/project before the generic /Users/<name>/ rule can run, leaving part of the other username in the outgoing Sentry payload. Match only the exact home path component (or let the generic user-home regex handle other /Users/.../ paths) so similarly prefixed usernames are fully redacted.
Useful? React with 👍 / 👎.
The free-text + dictionary sensitive-key sets only knew session_id, so request query strings like session=… or sid=… (common session credentials) left the device unscrubbed. Add 'session' to the embeddable marker set + a word-boundary \bsid pattern (so it never matches inside/aside), and an exact 'sid' dictionary-key marker (substring would over-redact innocuous keys). +2 regression tests; full CmuxFoundation scrubber suite green (32 tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sentry has no JSON binary type and serializes NSData to its hex description
AFTER beforeSend, so a Data value (e.g. Data("token=...")) under a
non-sensitive key leaked as hex. Stop treating Data as a safe scalar; drop it
wholesale to <redacted-data>. +1 regression test; suite green (33 tests).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…w P1+P2) - URL user:pass@host authority (e.g. http://alice:secret@localhost) matched no rule; add a userinfo redactor that keeps scheme+host, run FIRST so it beats the email rule (which would match pass@host.tld and leak the username). - Home-path rule required a trailing slash, so /Users/buildbot and file:///Users/alice leaked the username; drop the required slash (username component stops at the next delimiter/end). +4 regression tests; suite green (35 tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…common values) Move the hand-authored key markers and value regexes out of SentryScrubber into ScrubberDenylists, ported from maintained upstream sources with the exact commit SHA + file:line pinned per block so drift is auditable: - Dictionary-key markers from sentry-python DEFAULT_DENYLIST + DEFAULT_PII_DENYLIST (@ 9e54e149a0, sentry_sdk/scrubber.py:15-60), reconciled with cmux's existing substring markers. Adds the session/ CSRF/IP aliases the hand-rolled list missed (connect.sid, phpsessid, symfony, mysql_pwd, _csrf/_xsrf, x_forwarded_for/x_real_ip/ip_address/ remote_addr) plus relay SENSITIVE_COOKIES aliases (sentrysid, _vercel_jwt, fasthttpsessionid, ...). Short aliases stored pre-normalized so they fire. - Value regexes from relay @common (@ 99c91d9284, relay-pii/src/regexes.rs): adds @pemkey (PEM private/public key blocks), @creditcard, @iban, @usssn, ported with no capture group so cmux's keep-group-1-prefix loop redacts the whole match (relay's group-1 = redact convention is inverted from cmux's). Keeps cmux's existing bearer/key=value/provider-key/JWT/AWS and the existing urlUserInfo (@urlauth) rule; skips relay @ip/@uuid to avoid redacting version strings and the workspace/surface UUIDs cmux logs. SentryScrubber's public API and all behavior are unchanged; the 35 existing SentryScrubberTests pass untouched. Adds table-driven ScrubberDenylistsTests (@test(arguments:)) asserting the ported keys/patterns redact representative upstream-corpus secrets and that UUIDs/build numbers are not over-redacted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ed @ (autoreview P1) The URL credential class excluded @, so redis://user:p@ss@host/db redacted only user:p@ and leaked ss@host (the host has no dotted domain, so the email rule doesn't catch it either). Drop @ from the userinfo exclusion class so the match consumes greedily through the last @ before a path/query/fragment/ whitespace terminator; the terminators still bound each match to one URL's authority, so a later URL's host isn't swallowed. Adds regression coverage for @-in-password, multi-credential strings, and the no-swallow case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…(autoreview P1)
The free-text key=value rule stopped the value at the first whitespace, quote,
&, comma, or }, so a quoted JSON secret containing one of those characters
leaked its tail: {"password":"abc&def"} -> {"password":"<redacted-secret>&def"}.
Raw event messages, breadcrumbs, stack context lines, and NSError text arrive
as such strings and never hit the wholesale dict-key layer.
The terminator is quote-context-dependent: & must end an UNQUOTED query value
(?token=X&page=2 keeps &page=2) but not a QUOTED JSON value. Split the rule
(and the sid alias) into a quoted-value branch that captures the opening quote
and consumes through the closing quote ([^"']*), and an unquoted branch that
keeps the delimiter-bounded value. Quoted rules run first. Does not model
escaped quotes inside a quoted value (a regex JSON parser); the structured
dict-key layer plus the wholesale Data/user/cookie drops are the real boundary
and this free-text pass is best-effort defense-in-depth. Adds regression cases
for both branches.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lace (autoreview P1+P2) P1: query strings drift from the key denylist. The free-text key=value regex hand-lists a subset of markers, so query params whose key is in the denylist's EXACT set (csrf, _csrf, _vercel_jwt, su, sentrysid, phpsessid, sid, ...) leaked. Query strings are structured, so redact them structurally: new SentryScrubber.scrubQueryString(_:) splits on &/; , parses each key=value, and replaces the value with redactedSecret when isSensitiveKey(key) (URL-decoding the key for the check). isSensitiveKey is now the single source of truth for query strings. Glue routes request.queryString and request.url's query component through it; the free-text scrub(_:) is documented as irreducibly best-effort. P2: redactPaths did an unbounded substring replace of homeDirectory, so homeDirectory=/Users/al over /Users/alice/x leaked the "ice" suffix. Precompute a component-bounded SentryRegexPattern (escaped home + (?=[/\s"']|$) lookahead) in init and use it instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…utoreview P2) event.context is a [contextName: [key: value]] map. The glue mapped each inner dict through scrub(dictionary:) but never checked the OUTER context name, so a scope.setContext(value:["raw":"plainsecret"], key:"credentials") leaked when the inner value matched no standalone secret pattern. tags/extra already get key-aware scrubbing at their outer boundary; context now matches. New SentryScrubber.scrub(context:) applies isSensitiveKey to the outer name (redacting the whole context wholesale) and recurses through scrub(dictionary:) otherwise. Glue routes event.context through it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d96e1a224
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| let key = String(segment[segment.startIndex..<equalsIndex]) | ||
| let decodedKey = key.removingPercentEncoding ?? key | ||
| guard Self.isSensitiveKey(decodedKey) else { return segment } |
There was a problem hiding this comment.
When event.request.queryString or a URL routed through scrubURL contains PII under an innocuous parameter name, this early return preserves the entire key=value segment without running the value through the normal string scrubber. For example file=/Users/alice/db.sqlite or next=https://host/p?token=opaque under a non-denylisted key leaves the user path or nested token in the Sentry payload; only keys on the denylist get redacted. Preserve non-sensitive keys, but still scrub their values by content before returning the segment unchanged.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fa9ba7a6e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case let string as String: | ||
| return scrub(string) |
There was a problem hiding this comment.
When structured Sentry payloads carry a URL as a string under a non-sensitive key such as breadcrumb data["url"] or mechanism.data["url"], this dispatch sends it only through the free-text regex scrubber. Exact denylist query aliases like _csrf, sid, su, or _vercel_jwt are intentionally handled by scrubQueryString(_:) and are not covered by the free-text assignment patterns, so https://host/callback?_csrf=opaque remains unsanitized outside event.request.url; detect URL-like strings (or URL-valued keys) and route their query component through the structured query scrubber before returning.
Useful? React with 👍 / 👎.
This branch was successfully deployed
1 participant
cmux's Sentry config (
Sources/AppDelegate.swift,CLI/cmux.swift) sends to hostedsentry.io(third-party US service) withsendDefaultPii = falsebut no content scrubber, so error messages, exception values, breadcrumbs, request/transaction data, the user context, and stack-frame file paths went out unscrubbed. That leaked local paths like/Users/<name>/...(revealing the username), project/workspace names, URLs, and possibly command/secret text.This adds a
beforeSend+beforeBreadcrumb(+beforeSendSpan) content scrubber to both Sentry inits.Coverage contract
The pure
SentryScrubber(inCmuxFoundation, no Sentry dependency, unit-tested) redacts on every string:/Users/<name>/or/home/<name>/prefix. The generic rule also covers build-machine stack-frame paths whose home differs from the runtime one.Bearer …,sk-/pk-/ghp_/xoxb-keys, JWTs, AWS access key IDs, and<sensitive-key>=valueassignments (token/password/secret/api_key/access_key/auth/cookie/credential/session_id/private_key/bearer, including embedded env names likeAWS_SECRET_ACCESS_KEY=). The key marker set is shared between the free-text regex and the dictionary path so a credential is caught whether it arrives as raw text or a dict key.URL/NSURLand other non-scalar values are stringified and scrubbed (Sentry serializes them to their description afterbeforeSend); genuine scalars (numbers, bools, dates, Data, NSNull) pass through.The contract is scrub every reachable free-text field; preserve only an explicit grouping allowlist, so a new free-text field is covered by default.
SentryEventScrubber(app + CLI target, where Sentry is linked) applies it to: eventmessage.formatted(rebuilt —capture(message:)populatesformattedand leaves the template nil),transaction,serverName,logger; exceptionvalue; exception mechanismdesc/helpLink/data(wherecapture(error:)copiesNSError.userInfo); every stack frame'sfileName/package/contextLine/preContext/postContext/varsacross exception, thread, and event stack traces; threadname; debug-imagecodeFile; requesturl/queryString/fragment/headers(cookies dropped);tags/extra/context; breadcrumbmessage/data; and child performance spanspanDescription/data/tags. Theuseridentity fields (userId/email/username/name/ipAddress/geo) are dropped wholesale.Preserved for grouping (the allowlist): exception
type, mechanismtype,fingerprint, framefunction/module/lineNumber/addresses,level,environment,releaseName,dist,modules.Behavior change: macOS performance tracing disabled
The macOS app had
tracesSampleRate = 0.1. The auto-instrumented rootSentryTransaction.traceserializes itsdata/tags/descriptioninto the payload afterbeforeSendruns, and the root tracer is not reachable through the public Sentry API (it lives in the SDK's internalinclude/headers), so it cannot be scrubbed. cmux does not consume these performance traces (no manual transactions orspan.setData), so this PR setstracesSampleRate = 0on macOS to remove that un-scrubbable egress path. Crash, error, and app-hang reporting are unaffected (independent of the trace sample rate). The CLI already hadtracesSampleRate = 0. The CLI also flipssendDefaultPii = true → falsefor defense-in-depth.macOS + iOS
Both Sentry inits (macOS app and the bundled CLI) share one DSN and both got the scrubber. iOS ships no Sentry SDK at all (no
import Sentry, noSentrySDK, no DSN anywhere underios/), so it sends nothing to sentry.io and there is nothing to scrub there.Tests
SentryScrubberunit tests inCmuxFoundation(swift test --package-path Packages/CmuxFoundation): paths, emails, tokens, env-style and broad-marker assignments, key-aware dict redaction, URL/object stringification, scalar passthrough; plain text and grouping fields preserved.SentryEventScrubberTests(cmuxTests) for the Event/Breadcrumb glue: formatted-only message, exception mechanism, frame paths/context/vars, thread frames, debugMeta, request/cookies, user-field drop, tags/extra/context.Builds verified
macOS
cmuxscheme,cmux-cli, andcmux-iossimulator all build clean. Structured autoreview (Codex) is clean.🤖 Generated with Claude Code
Note
Medium Risk
Touches all outbound Sentry payloads and changes macOS tracing sampling; scrubbing could over-redact debugging context or miss edge-case leaks despite broad tests.
Overview
Adds a defense-in-depth Sentry egress scrubber so crash/error telemetry to hosted Sentry no longer ships local paths, emails, tokens, and other PII in the clear.
CmuxFoundationgains a Sentry-freeSentryScrubber(regex helpers,ScrubberDenylistsaligned with sentry-python / relay) that redacts secrets, emails,/Usersand/homepaths, URL userinfo, key-aware dict/query handling, and wholesale drops forDataunder sensitive keys.SentryEventScrubberwires that intobeforeSend,beforeBreadcrumb, andbeforeSendSpanon the macOS app and CLI: scrub free-text event fields (messages, stack frame paths/lines/vars, request URL/query/headers, tags/extra/context, breadcrumbs) while preserving grouping fields (exception type, frame symbols, fingerprint). User identity and request cookies are cleared; CLIsendDefaultPiigoestrue→false.macOS sets
tracesSampleRatefrom0.1to0because root transaction payload fields are serialized afterbeforeSendand cannot be scrubbed via the public API. Crash/error/hang reporting is unchanged.Extensive unit tests cover denylists, scrubber behavior, and event glue.
Reviewed by Cursor Bugbot for commit 4fa9ba7. Bugbot is set up for automated code reviews on this repo. Configure here.