Skip to content

Improve redaction in mailsync-process to prevent secrets from appearing in connection error detail modal#2717

Merged
bengotow merged 4 commits into
masterfrom
claude/redact-mailsync-secrets-YA79v
May 8, 2026
Merged

Improve redaction in mailsync-process to prevent secrets from appearing in connection error detail modal#2717
bengotow merged 4 commits into
masterfrom
claude/redact-mailsync-secrets-YA79v

Conversation

@bengotow
Copy link
Copy Markdown
Collaborator

@bengotow bengotow commented May 8, 2026

Summary

Refactored and enhanced the secret redaction logic in the mailsync process to better protect sensitive credentials from appearing in UI messages.

Key Changes

  • Extracted redaction logic: Moved the inline stripSecrets function from the _spawnAndWait method into a new _stripSecrets instance method for reusability across the class
  • Enhanced redaction strategy: Implemented a two-pronged approach:
    1. Key-based redaction: Uses regex patterns to redact values of known sensitive JSON keys (refresh_token, access_token, imap_password, smtp_password) regardless of their current cached values, catching secrets that may have been rotated by the engine
    2. Value-based redaction: Continues to redact literal occurrences of cached secret values to handle cases where secrets appear outside JSON form
  • Expanded coverage: Applied the improved _stripSecrets method to additional error paths in the _spawnAndWait method and the _spawnProcess method, ensuring secrets are redacted in more error scenarios

Implementation Details

  • The regex pattern for key-based redaction (("key"\s*:\s*")(?:\\.|[^"\\])*(") ) properly handles escaped quotes within JSON string values
  • Cached values are filtered to only include non-empty strings before attempting redaction
  • The escape function properly escapes regex special characters to safely redact literal secret values
  • All error messages and logs now consistently use the centralized _stripSecrets method

https://claude.ai/code/session_01RUsz543z61nZAxsAEqHruD

The mailsync engine sometimes dumps Account JSON (e.g. on
ProcessAccountSecretsUpdated) into stderr/log buffers that get surfaced
as Error messages and rawLog strings. The previous redaction only ran in
_spawnAndWait and only stripped values cached on this.account.settings,
so rotated refresh/access tokens that the engine had just generated
would leak through unredacted.

Centralize redaction into _stripSecrets, which now also key-matches
known sensitive JSON fields (refresh_token, access_token, imap_password,
smtp_password) so any value for those keys is replaced regardless of
whether we have it cached. Apply it to sync()'s close handler too, which
previously constructed Errors directly from errBuffer with no scrubbing.
@indent-staging
Copy link
Copy Markdown
Contributor

indent-staging Bot commented May 8, 2026

PR Summary

Centralizes mailsync log/error redaction into a single _stripSecrets helper and applies it to the sync() close handler — which previously wrapped raw stderr in Error with no scrubbing — closing a leak path where engine-rotated refresh_token/access_token values (e.g. from ProcessAccountSecretsUpdated) could surface in Account.syncError, telemetry, and the onboarding "View Log" UI. The redaction now combines a key-based JSON regex (refresh_token/access_token/imap_password/smtp_password) with the existing value-based pass that scrubs cached secrets, so values that were rotated by the engine but not yet pushed back to the renderer are also stripped.

  • Added _stripSecrets(text) method on MailsyncProcess combining key-based JSON redaction and the existing value-based redaction of cached account.settings secrets.
  • Replaced the inline stripSecrets lambda inside _spawnAndWait's close handler with calls to this._stripSecrets, and extended scrubbing to also cover access_token.
  • Routed lastJSON.error and errBuffer in sync()'s close handler through _stripSecrets before constructing the emitted Error.

Issues

All clear! No issues remaining. 🎉

2 issues already resolved
  • Empty JSON values are rewritten to ********* — the key-based regex body matcher is (?:...)*, so \"refresh_token\":\"\" becomes \"refresh_token\":\"*********\", making it look like a secret was redacted when none was present. Cosmetic only, but confusing for log readers. (fixed by commit 75eb3fe)
  • sync()/migrate() still print raw buffers to console — console.warn('Failed to parse mailsync output as JSON:', outBuffer) (line 379), console.log('Skipping debug output from mailsync:', outBuffer) (line 373), and console.log(buffer.toString()) in migrate() (line 438) bypass _stripSecrets, so a rotated refresh_token/access_token flushed to stdout/stderr at the moment of a parse failure still appears verbatim in the renderer console and log files. (fixed by commit 4bb6250)

CI Checks

All CI checks passed on commit 4bb6250.

Custom Rules 3 rules evaluated, 3 passed, 0 failed

Passing This is a longer title to see what happens when they are too long to fit
Passing B
Passing Ben Rule

View all rules

@bengotow bengotow changed the title Improve secret redaction in mailsync error logs Improve secret redaction in mailsync error logs to prevent them from appearing in UI messages May 8, 2026
@bengotow bengotow changed the title Improve secret redaction in mailsync error logs to prevent them from appearing in UI messages Improve redaction in mailsync-process to prevent secrets from appearing in UI messages May 8, 2026
@bengotow bengotow changed the title Improve redaction in mailsync-process to prevent secrets from appearing in UI messages Improve redaction in mailsync-process to prevent secrets from appearing in connection error detail modal May 8, 2026
Comment thread app/src/mailsync-process.ts Outdated
@bengotow bengotow merged commit a4dc48e into master May 8, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants