You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat: PII scrubber and rate limiting, on by default (#11) - #30
Scrubber (bugsee-core/src/scrub.rs): hand-written, regex-free matcher run at report assembly (after before_send) on every channel, crash.json, summary/description/labels/attributes, request.json and the registration environment — on the disk, in-memory (Read-only / ephemeral file systems: defined data-directory fallbacks #9) and recovery paths. Detects emails, known token shapes, secret-keyed values (text and JSON strings only), Luhn-valid card layouts, PEM blocks, URL userinfo, home-directory usernames. Never rewrites :: paths, UUID/code_id shapes, URL routes, signatures or numbers. ScrubConfig (allow/deny/sensitive keys/toggles), LaunchOptions::disable_scrubbing(). One shared key matcher with bugsee-reqwest.
Rate limiting (ratelimit.rs): per-signature + global token buckets, separate crash-floor bucket (never zero), manual bug reports exempt, DropReason::RateLimited, suppressed count attached to the next admitted non-crash report and exposed via Bugsee::suppressed_reports(). LaunchOptions::disable_rate_limiting().
Both on by default — behaviour change documented in PROGRESS.md.
Review
Opus review (14 findings, incl. :: path corruption and quadratic-time detectors) fully addressed; adversarial 1 MB inputs now finish in ~0.13 s (release). A second Opus pass is in progress; follow-up commits will land here.
Known limits
Attachments/minidumps, the reporter's own email field and dedup signatures are not scrubbed; the FFI doesn't expose the new options yet; scrubbing is not idempotent; no entropy-based detection by design.
Test plan
fmt, clippy -D warnings, cargo test --workspace --locked on macOS (skipping the pre-existing hanging fork_safety test)
…ems (#9)
Resolve the data directory as explicit option, BUGSEE_DATA_DIR, platform
default, then temp dir, taking the first that is actually writable. If none is,
or the disk fails mid-run (full, read-only, removed), launch still succeeds and
handled errors / manual reports are assembled without capture context and
delivered from a bounded in-memory backlog. Native crash handler and panic
snapshots are skipped in memory mode. Adds Bugsee::storage_status() so a host
can check crash recovery is available and whether the dir may be ephemeral.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
- memory backlog: entries stay queued while in flight so the cap is real; cache
the presigned endpoint (no duplicate issue on a retried PUT); honour the flush
deadline; clear the backlog in fork children
- a requested data_dir / BUGSEE_DATA_DIR is final: unusable means memory mode
(explicit_rejected), never another disk location
- temp fallback is bugsee-<uid>/<token>, 0700, symlink/owner checked; probe uses
create_new with a random name
- recover and deliver leftovers from the previous default location once
- mid-run degradation is visible (StorageStatus::degraded); fork child falls
back to memory instead of failing; warn_once per message, never panics
- #[non_exhaustive] on the new public types; ffi test no longer writes to the
real data directory
🤖 Generated with [Claude Code](https://claude.com/claude-code)
- fork child that falls back to memory clears the panic snapshot path and
updates the published storage resolution (dir None, source None)
- legacy location is adopted only if <temp>/bugsee and the token dir are real
directories owned by us and not our own directory (canonicalised compare)
- legacy queue is included in flush, gc'd for orphans, and retired from polling
once recovery has run and its queue is empty
- do not resolve temp_dir() unless a default location is actually needed
- tests: fork memory-fallback, symlinked/aliased legacy dir, end-to-end legacy
crashed session + queued report delivered once and cleaned up
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Adds crates/bugsee/src/hostinfo.rs with per-platform lookups (libc on unix,
hand-declared Win32 calls on Windows; no new dependencies) and threads the
results through HostFacts into platform.utc_offset and hardware.model.
Closes#16
🤖 Generated with [Claude Code](https://claude.com/claude-code)
…sharing a data dir
Several processes on one data directory could double-deliver (or lose) reports:
- A session was recovered twice: a peer that listed it before another finished
claimed it after the winner had discarded it, producing a phantom report. The
marker is now re-checked after the claim is won.
- Stale-claim takeover was remove-then-create, so two takers could each remove the
other's fresh claim and both hold it. Takeover is now serialised by a guard file
and re-validated under it; release only removes the claim's own content.
- Claims and session markers identified owners by bare pid: a recycled pid (or a
restarted container's pid 1) kept a dead owner's work blocked forever. Owners
now carry a process start time (Linux /proc, macOS proc_pidinfo, Windows
GetProcessTimes), with a 30 minute lease as a backstop.
- On Windows process liveness always answered "dead", so live peers' claims and
markers were taken over. Implemented via OpenProcess/GetExitCodeProcess.
- queue::gc_orphans at a peer's launch reaped another process's in-flight enqueue
(sidecars before the bundle rename), stranding the bundle and losing the report.
Only files untouched for 60 s are reaped now.
- Leftover claims of discarded sessions are swept.
Adds tests/multiprocess.rs (real child processes racing; SIGKILLed holder) plus unit
tests for claims, process identity and queue gc.
Closes#14
🤖 Generated with [Claude Code](https://claude.com/claude-code)
- Document per exit path what is persisted, delivered and recovered
(Bugsee::stop docs, DESIGN.md section 8).
- Bugsee::shutdown(timeout) = flush then stop. A stop during a flush now
aborts the flush so the clean-stop acknowledgement bound holds.
- Bugsee::install_termination_flush: opt-in, chaining SIGTERM/SIGINT hook
(self-pipe; flush on a normal thread; re-raises with the default
disposition so the exit status is unchanged) and a Windows console
control handler.
- LaunchOptions::flush_at_exit: atexit-based clean stop for deliberate
process::exit.
- Subprocess tests for every exit path.
Closes#8
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Add a regex-free scrubber (emails, token shapes, keyed secrets, card-like
numbers, home-dir usernames; allow/deny/sensitive_keys; off switch) applied
at report assembly over every JSON section after before_send, and token
bucket rate limiting (per-signature + global, crash floor, bounded memory)
applied before the durable queue, surfaced via DropReason::RateLimited and
a bugsee.reports_suppressed attribute. Both on by default, opt-out via
LaunchOptions::disable_scrubbing / disable_rate_limiting.
Closes#11
🤖 Generated with [Claude Code](https://claude.com/claude-code)
…S file locks
Review of the pid/start-time claim scheme found it unsound where #14 matters most:
containers with separate pid namespaces sharing a volume (both apps pid 1) cannot
compare pids or start times, so a peer's live claim was judged stale and its live
session reported as a crash. Pid-in-file schemes also needed a lease, a takeover
guard with a timer, and identity checks, each with its own race.
Replace them with kernel locks: fcntl(F_SETLK) on unix (per process, released on
death, not inherited by fork) and LockFileEx on Windows, on dedicated *.lock files
(queue/<bundle>.lock, sessions/<gen>.recover.lock, and sessions/<gen>.lock held
for a session's life). Nothing can go stale, so there is no lease, takeover or
start-time logic. An in-process registry (pid-keyed, spin-locked with the holder's
pid so a fork child cannot inherit a held lock) excludes threads and keeps a
process from ever opening one lock file twice, which POSIX would punish by
dropping the lock.
- .alive stays a bare pid and lock files are new names, so older builds sharing a
directory keep working (no exactly-once between old and new; documented).
Markers with no .lock beside them still use the pid probe.
- Liveness probe: zombies count as dead (Linux /proc state, macOS proc_pidinfo);
Windows uses WaitForSingleObject.
- Leftover lock files of finished items and dead sessions are swept when free.
- Tests: a live peer whose marker names a colliding pid (parent's, init's) is not
recovered until it really dies, then exactly once; concurrent claimers; probing
never drops a held lock; zombie probe.
Closes#14
🤖 Generated with [Claude Code](https://claude.com/claude-code)
- Detect a handler installed after ours (tokio, signal-hook) by asking the
kernel what is in force; flush only, never terminate the host.
- A stop no longer waits on the uploader to end the session cleanly (an HTTP
call cannot be interrupted); delivery is documented at-least-once.
- Fork-child re-install ordering, SIG_IGN left alone, watcher-alive and
pending-signal tracking, host sa_mask/SA_ONSTACK/SA_NODEFER/SA_RESETHAND
carried over, fork-aware install lock, pipe2, clean rollback on failure.
- flush_at_exit is unix only and skipped in fork children.
- Flush aborts between deliveries on stop (memory and every queue dir).
- Tests: late/SA_SIGINFO/ignored handlers, SIGINT, double signal, fork child,
hanging transport; no fixed sleeps; temp dirs cleaned.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Deep review: PII scrubber + rate limiting (on by default)
Closes #11. Stacked on #24. The choke-point wiring is right: every delivery path (disk, in-memory, recovery, snapshot) goes through assemble_docs, rate limiting sits after before_send/sampling and before snapshot/ZIP/queue, and the known limits (attachments, reporter email, FFI, non-idempotent, no entropy) are documented in PROGRESS.md.
No P0/P1. Four P2s below are real failure modes under the default-on config, not style. I did not run the test suite here (environment rustc 1.83 vs MSRV 1.88).
P2 — Free-text keyed values stop at the first ", leaking the tail of a secret (scrub.rskeyed)
P2 — request.json scrubs signatures despite the documented exception (reporting.rs)
P2 — Denied reports still hit a blacklist disk read (runtime.rsprocess_report)
Overall risk: Medium
Merge recommendation: Request changes
Fix the four P2s (or explicitly accept them with tests) before merge. The default-on behaviour change is otherwise in good shape.
Most important to fix
Stop treating auth/token/session as unstructured substrings on JSON keys — author, cancellation_token, and the reqwest www-authenticate exception are the concrete casualties.
Scan quoted keyed values like JSON strings (escaped quotes), not find(quote).
Pluck signatures out of request.json the same way email is plucked.
Call admit before any_blacklisted so a storm’s deny path is actually lock + hash.
Positive
Regex-free detectors with run caches and a 1 MB pathological bound; :: paths, UUID/code_id, and crash.json structure have tests; KEY_EXCEPTIONS for signatures on the structured walk; crash floor clamped to ≥1; limiter joins the fork() protocol; restore_suppressed on assemble failure; e2e covers disk, in-memory, facade, and a storm with crashes still delivered.
- an existing directory that fails the write probe / session open is still
recovered from and drained (read + delete only); claims are lenient there and
a crash that cannot be re-queued on disk is delivered from memory; delivered
but undeletable bundles are remembered so they are not resent every drain
- in-memory TooManySimilar blacklist (cleared in fork children)
- a clean stop gives the in-memory backlog a last bounded delivery attempt
- legacy temp location adopted whenever it exists and is trusted, even if no
new default directory was usable
- rotation tick rewrites a missing liveness marker after a wipe and marks the
recorder degraded
- DESIGN.md: failure table, legacy/flush wording
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Numeric secrets under secret keys and integer card numbers are redacted;
colon-form prose ("invalid token: expected ident") and "my_app::token:"
are left alone; Cookie headers take the whole line; escaped quotes inside
quoted values are honoured; the "signatures" exemption moves out of the
shared key matcher into crash.json handling; tighter token shapes (AWS,
hf_, sk-, prose after Bearer); contextual stem exclusions (author,
tokenizer, session_state, method_signature); exact key/sig only redact
credential-looking strings; request.json no longer double-scrubs
metadata; file names (icon@2x.png) and /Users/Shared are not PII; the
pre-check handles \u escapes and separated key stems. Test secrets are
assembled from halves for push protection.
Refs #11
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Keep both behaviours: the new in-memory blacklist and stuck-delivery
locks join the fork protocol alongside the rate limiter; recovered
crashes are scrubbed once in assemble_docs and delivered through the
new memory fallback; process_report order is blacklist -> limiter ->
assemble. Adds a recovered-crash scrubbing test.
Refs #11
🤖 Generated with [Claude Code](https://claude.com/claude-code)
- A launch that cannot hold <gen>.lock removes the file instead of leaving an
unheld one, which every peer read as "that session died" and recovered (and
deleted) the live session. Contended claims are retried briefly; an empty,
fresh marker with a free lock is skipped (the pid is written after locking);
on unix a probe uses F_GETLK and never takes the lock.
- Filesystems that reject locks (ENOLCK, EOPNOTSUPP, ENOSYS, EINVAL) degrade to
thread-only claims with a one-time message, and probes report unknown so
recovery falls back to the marker pid, instead of blocking the queue forever.
- An unclean shutdown (detached worker/uploader) keeps the session lock until the
process exits so peers cannot recover a session still being written.
- Registry keys are canonicalised; a fork child resets the registry from the fork
hook; old-scheme .claim/.claim.takeover files are swept.
- Tests: the real pid-namespace hazard (marker pid that does not exist here,
reaped pid), crashed sessions with free lock files, the lock-race launch,
unsupported-lock injection, and the probe/launch ordering cases.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
The stop-time memory drain relies on the scheduled pass having had its one
attempt; only an explicit flush aborts on stop.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Resolves conflicts in claim.rs (keep the file-lock claims, add the lenient
variant #9 introduced: a directory that cannot take a lock file proceeds
unclaimed) and recovery.rs (claim_pending is lenient and still re-checks the
marker).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
A producer only queues an entry; the capture worker appends it later, so the
last events before a fault (still in the channel) were lost. Measured: a
1500-event burst followed by SIGSEGV recovered ~50-60 events.
After the marker is complete, the fatal-crash handler now does one atomic
ticket, one non-blocking write(2) (Windows: SetEvent) to a pre-created wake
handle and polls an atomic for at most 50 ms. A helper thread made at launch
posts a Flush through the normal FIFO channel and publishes the worker's ack.
No allocation or lock on the crashing thread; a stuck worker costs only the
budget. A main-thread panic nothing will catch runs the same bounded flush.
Tests: core handshake/stuck-worker/deregistration, torn part-tail recovery,
and subprocess crash tests (burst then SIGSEGV recovers every event; wedged
worker still dies promptly and reports). fork_safety's FileTransport now
writes atomically since a dying worker can linger mid-upload.
Closes#13
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Closes #11. Stacked on #24. Head is f8162d7 (scrubber hardening) + merge of fix/issue-9.
The choke-point wiring is still right: every delivery path goes through assemble_docs, limiter sits after before_send/sampling, recovered crashes are scrubbed once, registration environment is scrubbed.
Prior P2s: escaped-quote values and request.jsonsignatures are fixed. Stem exclusions and blacklist-before-admit are not (stems only partly: author/authority are out, the originally cited cancellation_token / www-authenticate / token_endpoint still match). One new P2 from the hardening pass: the colon-prose skip leaks all-lowercase password: … values.
No P0/P1. I did not run the test suite (environment rustc 1.83 vs MSRV 1.88).
P2 — Denied storm reports still read .blacklist from disk (runtime.rsprocess_report) — unchanged from last review
Overall risk: Medium
Merge recommendation: Request changes
Most important to fix
Extend stem_hit (or a small denylist next to reqwest’s) so cancellation_token, www-authenticate, and token_endpoint are not treated as secret keys. Negative tests for those names as JSON keys / headers, not only cancellation_token::CancellationToken as exception text.
Apply the colon-prose skip only to token (compiler-error wording), not to password/passwd/secret. Add password: secret / invalid password: letmein.
Admit before any_blacklisted, or cache .blacklist in memory so a rate-limited storm is actually lock + hash.
Positive
author/authority/tokenizer/session_state exclusions and the looks_like_secret gate on exact key/sig are the right idea. find_close fixes the reported foo\"bar leak. request.json now scrubs only environment, with scrub_json_bytes_keeping for crash signatures. Cookie takes the whole line; numeric secrets and card integers are covered; limiter still joins fork().
…returns
A host that crashes again right after launch (crash loop) could die before the
worker thread recovered the previous crash. launch now runs a bounded,
newest-first recovery pass (8 sessions, 500 ms budget via
LaunchOptions::launch_recovery_budget, 4096-entry scan cap) that enqueues the
reports durably before returning; upload stays async and the worker's pass picks
up the rest, skipping what the launch pass attempted.
Closes#10
🤖 Generated with [Claude Code](https://claude.com/claude-code)
… unlinkable claims proceed (#9)
- recovery_dirs: all rejected existing candidates (incl. the private temp tree)
are drained/recovered even when a later default wins; core takes extra_data_dirs
- a stale claim that cannot be unlinked is Unavailable, not Taken
- seed the in-memory blacklist from the on-disk one
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Stem exclusions for cancellation_token, www-authenticate/authentication,
token_endpoint/url/uri/expiry/ttl; the colon-prose leniency applies only to
bare 'token', so password/passwd/secret/api_key values are always redacted;
the rate limiter now runs before the on-disk blacklist read so a denied
storm is lock + hash only (test counts disk reads).
Refs #11
🤖 Generated with [Claude Code](https://claude.com/claude-code)
- Android: errno via libc::__errno (and the BSD variants), not __errno_location.
- Gate the flush_at_exit statics and import on unix (Windows clippy).
- SA_RESETHAND: the handler decides "terminate" before emulating the reset
and ships it in the pipe byte; the watcher no longer re-reads the slot.
New test with a one-shot host handler.
- Windows Ctrl+C/Break now run shutdown like close/logoff/shutdown; document
that a host which swallows the event registers its handler after ours.
- DESIGN section 6 no longer says guard Drop flushes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Keeps the file-lock claim.rs (the pid-claim changes from #9 no longer apply; its
lenient variant is retained). Writes the F_UNLCK comparison portably and gates the
unix-only Lock::Unsupported variant so non-unix clippy is clean.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
…ash handlers
Address review of the launch-path recovery pass:
- the pass runs on a helper thread and launch waits at most the budget, so a
blocked before_send cannot stall or deadlock launch; the worker holds its own
recovery until the pass is done (bounded) and skips what the pass claimed
- Recorder::recover_on_launch_path is called by launch_with after the native
handler and panic hook are installed, outside all locks
- dedup repeated identical recovered crashes against the queue and cap it
- recovered crashes carry no scope on either path; claimed-only attempted set;
scan cap counts session markers only; docs, latency numbers, deterministic
tests, facade test
🤖 Generated with [Claude Code](https://claude.com/claude-code)
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Deep review (HEAD e86e426): PII scrubber + rate limiting
Closes #11. Base is #8 (fix/issue-8 @ 50da593); unique #11 vs that base is still the same 16 files. Head is another merge of shutdown/flush (and the stacked #10/#12/#13/#14/#25 tree) into this branch. Unique #11 code is unchanged in substance from last review (605126d / 1881878). The remarge’s only touch on those files is manual_upload_meta now filling source.origin (from #25) and the crate table in PROGRESS.md.
Last round’s PKCE P2 stays fully addressed. The merge kept the choke points:
every delivery path still goes through assemble_docs (recover_one, deliver_snapshot, assemble_from_disk, assemble_in_memory)
limiter still sits after before_send/sampling and before the on-disk .blacklist read
recovered crashes still skip the limiter (the crash floor is for in-process loops, not next-launch recovery)
the limiter is last in fork_lock / first in fork_unlock, after health
default source.origin is path::to::function:line; scrub_meta does not rewrite it, which matches the ::-path exception. Session registration still uses the pre-scrubbed environment_json.
No remaining P0–P3. CI 38577927241 has ubuntu/windows + MSRV + android + ios aarch64 + macOS private-API green (macos tests and ios-sim still pending at review). I did not run the suite here (rustc 1.83 vs MSRV 1.88).
Findings
None.
Overall
Overall risk: Low (default-on PII/secret stripping and volume caps; remaining leftovers are diagnostic over-redaction, not leaks).
Merge recommendation: Approve — unique #11 is ready on top of #8; retarget to main after #8 (and its stack) land. The PR body still says stacked on #24.
Most important issues to fix: Nothing blocking.
What looks solid: PKCE code_verifier is a structured stem and a free-text Strong name, with reqwest header/query coverage; last round’s free-text oauth_verifier / short pin/otp path stays green; auth_url survives; limiter-before-disk order survived the remarge; colon-prose leniency stays on bare token; crash.json signatures stay intact; recovered crashes are still scrubbed once.
Residual (same matcher class, not re-filed): OIDC discovery still loses end_session_endpoint, check_session_iframe, id_token_signing_alg_values_supported (and encryption siblings), pushed_authorization_request_endpoint. might_match still has no verifier needle — the documented prefilter invariant is false for a timestamp-free {"code_verifier":…} record, but every live capture entry has epoch-ms so it is not a production leak.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Deep review (HEAD 75dbf32): PII scrubber + rate limiting
Closes #11. Base is #8 (fix/issue-8 @ f5dcac4). This remarge is a no-op tree vs last review (e86e426); unique #11 is still the same 16 files. crates/bugsee/Cargo.toml still has a single unix libc table.
All 11 prior P2 threads stay fully addressed. The remarge kept the choke points: every delivery path still goes through assemble_docs; limiter still sits after before_send/sampling and before the on-disk .blacklist read; recovered crashes still skip the limiter; the limiter is last in fork_lock / first in fork_unlock; registration still uses the pre-scrubbed environment_json.
One remaining P2 under the default-on config, missed on earlier passes of the Pin class added in 7e1adab. No P0/P1. CI 38579297811 has MSRV + android green (ubuntu/windows running, macos/ios queued at review). I did not run the suite here (rustc 1.83 vs MSRV 1.88).
Overall risk: Medium (default-on MFA-code leak from log/breadcrumb string leaves; structured {"otp": …} and otp= are already clean).
Merge recommendation: Request changes — drop the extra separator gate for KeyClass::Pin and add OTP: 123456 / pin: 4829 tests.
Most important issues to fix: The Pin-class colon-form hole below.
What looks solid: Prior stem/OIDC/oauth_verifier/PKCE/escaped-quote/request.json signatures/limiter-before-disk fixes held through the remarge; crash.json signatures stay intact; recovered crashes are still scrubbed once; colon-prose leniency stays on bare token only.
Residual (same matcher class, not re-filed): OIDC discovery still loses end_session_endpoint, check_session_iframe, id_token_signing_alg_values_supported (and encryption siblings), pushed_authorization_request_endpoint. might_match still has no verifier needle (live capture records carry epoch-ms). Compound MFA keys (sms_otp / totp) stay exact-otp/pin by design. PR body still says stacked on #24.
The reason will be displayed to describe this comment to others. Learn more.
P2 — KeyClass::Pin requires = or a quoted key, so the common log form OTP: 847291 is not redacted.
7e1adab added this class so short numeric codes redact in free text. codeish already excludes the documented prose (otp: sent, pin=home). The extra sep == '=' || quoted_key gate then drops the unquoted colon form that Strong keys already allow (password: secret).
Traced through keyed:
input
class
sep
quoted_key
codeish
result
otp=654321
Pin
=
no
yes
[redacted] (tested)
{"otp": "654321"}
Pin
:
yes
yes
[redacted] (tested)
otp: sent
Pin
:
no
no
kept (prose)
OTP: 847291
Pin
:
no
yes
unchanged
pin: 4829
Pin
:
no
yes
unchanged
might_match admits the record (otp needle + epoch-ms). Path: assemble_docs → scrub_envelope → scrub_str on the log/breadcrumb message → here → continue.
Impact: Default-on. MFA/OTP codes in log lines and breadcrumb messages ship in every report section that stores free text. Structured object keys and otp= query strings are already clean.
Scenario:log!("MFA challenge OTP: {}", code) with a 6-digit TOTP, or a breadcrumb message of pin: 4829. The code leaves the device.
Fix: When codeish, accept :, =, or a quoted key (drop the separator restriction). Keep codeish as the prose filter. Add scrub_str tests for OTP: 123456 / pin: 4829; keep otp: sent / pin=home.
Root cause of the windows flake in a_stuck_worker_costs_only_the_budget: the helper's ack wait (250 ms) was shorter than the 5 s budget the test asked for. After un-wedging, the worker had to drain a backlog (file creation on Windows is slow) and the helper gave up before the ack, never retrying, so the handler waited out its whole budget. The helper now waits 10 s (it only bounds a permanently wedged worker). The test also stopped assuming a 150 ms sleep wedges the worker (it waits for the sampler to report entry), uses generous budgets and bounds, and no longer relies on sleeps.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Generalise the mobile native capture and out-of-process monitor design
notes, the FFI audit and a few code comments so they describe behaviour in
generic terms (the backend, the ingestion pipeline, existing report
variants) instead of naming components of other projects. No behaviour
change.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Tree unchanged here; the clean-up from #37 is already included by the parent merge or applied next.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
The reason will be displayed to describe this comment to others. Learn more.
Deep review (HEAD 2e1b442): PII scrubber + rate limiting
Closes #11. Base is now main (bf90a57, including landed #8/#12/#13/#14/#10/#37). This push is a remarge of those stacks plus #37's comment-only drop of numeric server codes. Unique #11 production code (scrub.rs, ratelimit.rs, assemble/admit wiring) is unchanged vs last review (75dbf32).
Choke-point wiring still holds after the remarge:
every delivery path (disk, in-memory, recovery, snapshot) goes through assemble_docs
limiter sits after before_send/sampling and before snapshot/ZIP/queue
limiter is last in fork_lock / first in fork_unlock (after health)
recovered crashes skip the limiter (launch-path cap still applies)
eager session registration (#12) still uses the scrubbed environment_json
crates/bugsee/Cargo.toml still has a single unix libc table
Prior P2s from earlier rounds stay fully addressed. Last round's P2 is still open: KeyClass::Pin rejects the unquoted colon form (OTP: 847291).
No P0/P1. CI 38600009536 is green (ubuntu/macOS/windows + MSRV 1.88 + Android/iOS lint). I did not run the suite here (rustc 1.83 vs MSRV 1.88).
Findings
P2 — KeyClass::Pin requires = or a quoted key, so OTP: 847291 survives (scrub.rskeyed)
Overall risk: Medium
Merge recommendation: Request changes
Same P2 as last round. Unique #11 did not move; the remarge did not introduce a new P0–P3.
Most important to fix
When codeish, accept :, =, or a quoted key. Keep codeish as the prose filter (otp: sent, pin=home). Tests for OTP: 123456 / pin: 4829.
Residual (not re-filed)
OIDC discovery over-redaction of end_session_endpoint, check_session_iframe, id_token_signing_alg_values_supported (and encryption siblings), pushed_authorization_request_endpoint. might_match still lacks a verifier needle (timestamp-free synthetic records only). Compound MFA keys (sms_otp / totp) are exact-otp/pin by design. FFI still does not expose the new options (documented).
Positive
The #8/#12/#13 remarge did not skip the scrubber or reorder the limiter relative to disk work. #12 health still increments on errors that are later rate-limited (intentional). Regex-free detectors, crash-floor clamp, and e2e coverage of disk / in-memory / facade / recovered crashes remain intact.
The reason will be displayed to describe this comment to others. Learn more.
P2 — KeyClass::Pin requires = or a quoted key, so the common log form OTP: 847291 is not redacted.
Still present at 2e1b442 (unique #11 unchanged vs 75dbf32). 7e1adab added this class so short numeric codes redact in free text. codeish already excludes the documented prose (otp: sent, pin=home). The extra sep == '=' || quoted_key gate then drops the unquoted colon form that Strong keys already allow (password: secret).
Traced through keyed:
input
class
sep
quoted_key
codeish
result
otp=654321
Pin
=
no
yes
[redacted] (tested)
{"otp": "654321"}
Pin
:
yes
yes
[redacted] (tested)
otp: sent
Pin
:
no
no
kept (prose)
OTP: 847291
Pin
:
no
yes
unchanged
pin: 4829
Pin
:
no
yes
unchanged
might_match admits the record (otp needle + epoch-ms). Path: assemble_docs → scrub_envelope → scrub_str on the log/breadcrumb message → here → continue.
Impact: Default-on. MFA/OTP codes in log lines and breadcrumb messages ship in every report section that stores free text. Structured object keys and otp= query strings are already clean.
Scenario:log!("MFA challenge OTP: {}", code) with a 6-digit TOTP, or a breadcrumb message of pin: 4829. The code leaves the device.
Fix: When codeish, accept :, =, or a quoted key (drop the separator restriction). Keep codeish as the prose filter. Add scrub_str tests for OTP: 123456 / pin: 4829; keep otp: sent / pin=home.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Closes #11.
Summary
bugsee-core/src/scrub.rs): hand-written, regex-free matcher run at report assembly (afterbefore_send) on every channel,crash.json, summary/description/labels/attributes,request.jsonand the registration environment — on the disk, in-memory (Read-only / ephemeral file systems: defined data-directory fallbacks #9) and recovery paths. Detects emails, known token shapes, secret-keyed values (text and JSON strings only), Luhn-valid card layouts, PEM blocks, URL userinfo, home-directory usernames. Never rewrites::paths, UUID/code_id shapes, URL routes, signatures or numbers.ScrubConfig(allow/deny/sensitive keys/toggles),LaunchOptions::disable_scrubbing(). One shared key matcher withbugsee-reqwest.ratelimit.rs): per-signature + global token buckets, separate crash-floor bucket (never zero), manual bug reports exempt,DropReason::RateLimited, suppressed count attached to the next admitted non-crash report and exposed viaBugsee::suppressed_reports().LaunchOptions::disable_rate_limiting().Review
Opus review (14 findings, incl.
::path corruption and quadratic-time detectors) fully addressed; adversarial 1 MB inputs now finish in ~0.13 s (release). A second Opus pass is in progress; follow-up commits will land here.Known limits
Attachments/minidumps, the reporter's own email field and dedup signatures are not scrubbed; the FFI doesn't expose the new options yet; scrubbing is not idempotent; no entropy-based detection by design.
Test plan
-D warnings,cargo test --workspace --lockedon macOS (skipping the pre-existing hangingfork_safetytest)🤖 Generated with Claude Code