feat(security): add redact secretScan mode with placeholder-aware copy

[chrisleekr]

Summary

Completes #181: adds a secretScan = "redact" mode so a literal API token in a structured config (.claude.json, mcp.json, Codex config.toml, hook settings) is replaced with a $AGENTSYNC_REDACTED_<FIELD> placeholder and pushed, instead of aborting the whole push. Pairs it with a placeholder-aware copy so the round trip is safe, and surfaces it in the TUI.

Builds on #190 (the catastrophic always-block tier), which still applies in every mode — redact and off included.

How it works (the round trip)

1. Push (redact mode): redactSecretLiterals replaces an ordinary token with $AGENTSYNC_REDACTED_<FIELD> and emits a non-fatal Redacted literal secret notice (vs the fatal Detected literal secret). A catastrophic value (age key, PEM) is left untouched so the central scanForSecrets still blocks it. A secret in prose (markdown) has no field to replace, so it still aborts.
2. Copy (mergePreservingSecrets, new src/core/secret-merge.ts): merges incoming over the local file — an incoming placeholder never overwrites a real local value, local-only entries survive, and array elements (e.g. a token in MCP args) merge element-wise. Wired into all four adapters that redact structured config: claude, codex, cursor, vscode.
3. TUI Config tab: redact joins the ←/→ cycle, a "What this means" panel explains the selected mode, and switching to off prompts a y/n confirm (it pushes live secrets). The confirm is a blocking modal so a global p can't push behind it.

Notes from the senior-review gate

The pre-commit review caught and this PR fixes:

• (High) copy for cursor and vscode wholesale-overwrote the local MCP file — a placeholder would clobber a working local key. Now merged like claude/codex.
• (High) the merge didn't handle placeholders inside arrays (MCP args); now element-wise.
• (Medium) doc/comment wording overstated env-var auto-expansion — the placeholder is a literal marker AgentSync does not expand; corrected.
• Added e2e (prose-still-aborts), applyClaudeMcp/cursor/vscode placeholder-preservation, and explainer tests.

Tests

1003 pass / 0 fail. New coverage: redact-mode redactor, mergePreservingSecrets (incl. arrays + __proto__), placeholder preservation across claude/codex/cursor/vscode apply, TUI confirm/cancel/redact-applies + explainer, redact prose-abort e2e.

Closes #181.

🤖 Generated with Claude Code

https://claude.ai/code/session_011L7s33UCjpQkreXW7amAw2

Summary by CodeRabbit

• New Features

  • Added "redact" mode for secret scanning that masks API tokens with placeholders instead of blocking pushes.
  • Added confirmation dialog in the config tab when switching secret scan to "off" mode.

• Improvements

  • Local secrets are now preserved during sync operations instead of being overwritten by vault placeholders.

• Documentation

  • Expanded secret-handling policy documentation across guides with details on the new redact mode and tiered secret classifications.
  • Updated CLI and TUI config documentation to reflect new behavior modes.

[coderabbitai]

Warning

Review limit reached

@chrisleekr, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 48 minutes and 26 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d6a231ba-0b3e-43aa-a7a8-6e0e8d9237f9

📥 Commits

Reviewing files that changed from the base of the PR and between ddec446 and e9cf630.

📒 Files selected for processing (2)

• src/agents/__tests__/_utils.test.ts
• src/agents/_utils.ts

📝 Walkthrough

Walkthrough

Adds a redact mode to security.secretScan that substitutes ordinary API tokens with $AGENTSYNC_REDACTED_<FIELD> placeholders on push instead of aborting. On apply, a new mergePreservingSecrets module prevents those placeholders from overwriting real local values. All four agent apply paths (Claude, Codex, Cursor, VS Code) are updated to use this merge. The TUI gains a y/n confirmation gate for switching secretScan to off.

Changes

secretScan redact mode end-to-end

Layer / File(s)	Summary
Core redact classification and placeholder emission in sanitizer src/core/sanitizer.ts, src/core/__tests__/sanitizer.test.ts, src/commands/__tests__/push.test.ts	Adds ORDINARY_EMBEDDED_PATTERNS, REDACTION_PLACEHOLDER_PREFIX, and classifyForRedact. In redact mode, redactSecretLiterals replaces ordinary tokens with $AGENTSYNC_REDACTED_<FIELD> and emits non-fatal warnings, while catastrophic-tier secrets are left intact for the existing abort path. Tests cover all classifications and confirm prose-body literals still abort in redact mode.
mergePreservingSecrets module src/core/secret-merge.ts, src/core/__tests__/secret-merge.test.ts	New module with SecretMergeResult, mergeNode, and mergePreservingSecrets. Recursively overlays vault incoming onto local existing, never overwriting real local values with placeholders, merging arrays element-wise, filtering prototype-pollution keys, and collecting dotted paths where placeholders were written. Full test suite included.
Schema enum extension and JSONC parsing helpers src/config/schema.ts, src/agents/_utils.ts	Extends AgentSyncConfigSchema.security.secretScan enum to include "redact". Adds getJsoncTopLevelKey and parseJsoncObject JSONC helper exports consumed by agent apply paths.
Agent apply paths wired with mergePreservingSecrets src/agents/claude/index.ts, src/agents/codex/index.ts, src/agents/cursor/index.ts, src/agents/vscode/index.ts, src/agents/claude/__tests__/..., src/agents/codex/__tests__/..., src/agents/cursor/__tests__/..., src/agents/vscode/__tests__/...	Reworks applyClaudeMcp, applyCodexConfig, applyCursorMcp, and applyVsCodeMcp from overwrite-on-apply to a read-merge-write flow using mergePreservingSecrets, with fallback to writing restored content on parse/merge failure. Agent tests assert placeholder non-overwrite and local-only key preservation.
TUI secretScan off-confirmation and redact explainer src/commands/tui/state.ts, src/commands/tui/tabs/config.ts, src/commands/tui/app.ts, src/commands/tui/__tests__/config-tab.test.ts	Adds pendingSecretScan: string | null to ConfigSlice. Adds secretScanExplainer and "redact" to SECRET_SCAN_OPTIONS. Cycling to "off" sets pendingSecretScan and requires a y/n confirmation before setConfig is called; all other transitions apply immediately. handleKey in app.ts routes all keypresses to the tab while confirmation is pending. Tests cover explainer output, confirm/cancel paths, and immediate redact application.
Documentation CLAUDE.md, docs/architecture.md, docs/commands.md, docs/operations.md	Updates CLAUDE.md convention block, architecture sanitiser bullet, commands TUI and CLI config references, and operations mode descriptions. Adds a new "The redact contract" subsection covering the push/copy placeholder lifecycle and multi-recipient vault guidance.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant performPush
    participant sanitizer as redactSecretLiterals
    participant Vault

    User->>performPush: push with secretScan=redact
    performPush->>sanitizer: scan structured config fields
    alt ordinary API token detected
        sanitizer-->>performPush: replace with $AGENTSYNC_REDACTED_FIELD (non-fatal)
        performPush->>Vault: write config with placeholder (no real secret)
    else catastrophic secret (age key / PEM)
        sanitizer-->>performPush: leave intact, no redaction warning
        performPush->>sanitizer: scanForSecrets on output
        sanitizer-->>performPush: Detected literal secret → fatal abort
    else secret in prose/markdown body
        sanitizer-->>performPush: Detected literal secret → fatal abort
    end

Loading

sequenceDiagram
    participant Vault
    participant applyAgent as applyXxxMcp / applyCodexConfig
    participant mergePreservingSecrets
    participant LocalFS as Local config file

    Vault->>applyAgent: restored incoming config (may contain $AGENTSYNC_REDACTED_* placeholders)
    applyAgent->>LocalFS: read existing file
    alt file absent
        applyAgent->>LocalFS: write restored content directly
    else file present
        applyAgent->>mergePreservingSecrets: merge(existing, incoming)
        note over mergePreservingSecrets: placeholder never overwrites real local value
        mergePreservingSecrets-->>applyAgent: merged result + placeholder paths
        applyAgent->>LocalFS: write merged result
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• chrisleekr/agentsync#53: Modifies redactSecretLiterals and the sanitizer policy surface that this PR extends with the redact mode classification.
• chrisleekr/agentsync#185: Introduced the [security] config section and securityToPolicy mapping that this PR extends with the new "redact" enum value.
• chrisleekr/agentsync#190: Both PRs enforce the catastrophic-tier (age secret key + PEM) block in every mode and share the same cannot-be-allow-listed contract in src/core/sanitizer.ts.

Suggested labels

bot:resolve

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title accurately describes the main change: adding a 'redact' secretScan mode with placeholder-aware copy functionality.
Description check	✅ Passed	PR description provides comprehensive context including summary, implementation details, and test results, though it doesn't follow the template structure (missing Architecture section and explicit Risks/Follow-ups and Checklist).
Linked Issues check	✅ Passed	The PR fully addresses issue #181 by implementing the 'redact' mode to replace literal secrets in structured config with placeholders and push instead of blocking, which was the core requirement.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to implementing the redact secretScan mode (#181) and supporting infrastructure: sanitizer enhancements, secret merge logic, adapter updates, TUI config support, and comprehensive test coverage.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

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 `@src/agents/_utils.ts`:
- Around line 112-127: The functions `getJsoncObjectAtKey` and
`parseJsoncObject` are both discarding parser errors by passing an empty array
to parse(). To fix this, create an errors array variable instead of passing an
empty array, pass this errors variable to the parse function to capture any
parsing errors, and then check if the errors array is non-empty after parsing —
if errors exist, return undefined instead of the parsed result to prevent
malformed JSONC from being treated as valid mergeable data. Apply this fix to
both functions where parse() is called.

🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7859824f-a9bf-45dc-8598-441632732a9f

📥 Commits

Reviewing files that changed from the base of the PR and between d8f9a82 and ddec446.

📒 Files selected for processing (23)

• CLAUDE.md
• docs/architecture.md
• docs/commands.md
• docs/operations.md
• src/agents/_utils.ts
• src/agents/claude/__tests__/index.test.ts
• src/agents/claude/index.ts
• src/agents/codex/__tests__/index.test.ts
• src/agents/codex/index.ts
• src/agents/cursor/__tests__/index.test.ts
• src/agents/cursor/index.ts
• src/agents/vscode/__tests__/index.test.ts
• src/agents/vscode/index.ts
• src/commands/__tests__/push.test.ts
• src/commands/tui/__tests__/config-tab.test.ts
• src/commands/tui/app.ts
• src/commands/tui/state.ts
• src/commands/tui/tabs/config.ts
• src/config/schema.ts
• src/core/__tests__/sanitizer.test.ts
• src/core/__tests__/secret-merge.test.ts
• src/core/sanitizer.ts
• src/core/secret-merge.ts