Repository navigation
Make history-redact output disclosure-safe across SDK, CLI, MCP, and telemetry - #869
Merged
Merged
Conversation
Replace raw history-redact matcher and replacement fields with non-reversible counts across the public SDK contract. Sanitize CLI recovery guidance and telemetry inputs, cover CLI/SDK/MCP success and failure paths, document the contract, and record GH-864 in pm and the generated changelog.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. 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, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis change prevents ChangesHistory redaction disclosure safety
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant CommandLine as redactSensitiveCommandArgs
participant HistoryRedact as history-redact
participant Telemetry
participant MCP
CLI->>CommandLine: Sanitize matcher and replacement arguments
CommandLine-->>CLI: Return arguments with [redacted] values
CLI->>HistoryRedact: Execute with redaction options
HistoryRedact-->>CLI: Return counts and replacement_is_default
CLI->>Telemetry: Record sanitized arguments and metadata
MCP->>HistoryRedact: Submit history-redact request
HistoryRedact-->>MCP: Return disclosure-safe result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
unbraind
marked this pull request as ready for review
August 2, 2026 01:30
Owner
Author
|
@coderabbitai full review |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Owner
Author
✅ Action performedFull review finished. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideMakes history-redact disclosure-safe across SDK, CLI, MCP, and telemetry by removing echoing of matcher/replacement material, replacing it with aggregated counts, and ensuring all user-facing and telemetry paths sanitize sensitive history-redact inputs, with added tests and documentation to enforce the new contract. Sequence diagram for sanitized history-redact CLI recovery and typo guidancesequenceDiagram
actor User
participant CliMain
participant ArgvUtils
User->>CliMain: invoke history-redact with sensitive --literal/--regex
CliMain->>ArgvUtils: redactSensitiveCommandArgs(argv)
ArgvUtils-->>CliMain: safeInvocationArgv
CliMain->>CliMain: buildPmCliRecoveryContext(safeInvocationArgv)
CliMain->>ArgvUtils: renderPmCommand(safeInvocationArgv)
CliMain-->>User: error + recovery + examples (with redacted args)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/core/telemetry/runtime.ts (1)
133-141: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winInclude plural redaction keys in inline assignment matching.
isSensitiveKeyprotectsliterals,regexes, andreplacements, butSENSITIVE_INLINE_KEY_PATTERNonly matches singular names. A telemetry string such asregexes=telemetry-canarycan retain its value.Add plural forms to this pattern and add
sanitizeStringcoverage for each plural assignment form.🤖 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 `@src/core/telemetry/runtime.ts` around lines 133 - 141, The inline sensitive-key pattern used by INLINE_SENSITIVE_ASSIGNMENT_PATTERN and INLINE_SENSITIVE_FLAG_PATTERN must recognize plural forms literals, regexes, and replacements, matching the keys protected by isSensitiveKey. Extend SENSITIVE_INLINE_KEY_PATTERN accordingly and add sanitizeString coverage for each plural assignment form, preserving existing singular-key behavior.src/cli/main.ts (1)
412-427: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSanitize existing recovery fields before returning them.
The compact branch returns
existingRecoveryunchanged. The normal path also lets existingattempted_command,normalized_args, andsuggested_retryoverride the sanitized values inbuildRecoveryPayload.An upstream error context can therefore retain matcher or replacement canaries in the error response. Sanitize these fields before the compact return and before merging existing recovery data. Add a regression case with raw sensitive values in
context.recovery.🤖 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 `@src/cli/main.ts` around lines 412 - 427, Sanitize the existing recovery payload before it is returned or merged: update the recovery handling around `existingRecovery`, the compact-mode early return, and `buildRecoveryPayload` so `attempted_command`, `normalized_args`, and `suggested_retry` cannot override their sanitized equivalents. Preserve the current recovery behavior while ensuring upstream `context.recovery` values are redacted, and add a regression case covering raw sensitive values in `context.recovery`.
🤖 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/cli/main.ts`:
- Around line 2421-2431: Update the typo-handling flow around typoEvent.from and
redactSensitiveCommandArgs so recognized sensitive-flag typos redact inline
values in both the source token used by the PmCliError message and the matching
bootstrapInvocation.argv token before renderPmCommand(safeArgv). Use typoEvent’s
canonical target to identify the sensitive flag, preserve non-sensitive and
non-equals arguments, and add canary coverage for an equals-form typo for each
sensitive flag.
In `@src/sdk/command-line.ts`:
- Around line 69-72: Update the redaction logic around redactNext so a following
option token is not treated as a matcher value: only consume and replace the
next token when it is a value token, otherwise preserve the option and continue
normal processing. Add coverage for a value-less sensitive flag followed by
another option, such as --literal followed by --dry-run.
---
Outside diff comments:
In `@src/cli/main.ts`:
- Around line 412-427: Sanitize the existing recovery payload before it is
returned or merged: update the recovery handling around `existingRecovery`, the
compact-mode early return, and `buildRecoveryPayload` so `attempted_command`,
`normalized_args`, and `suggested_retry` cannot override their sanitized
equivalents. Preserve the current recovery behavior while ensuring upstream
`context.recovery` values are redacted, and add a regression case covering raw
sensitive values in `context.recovery`.
In `@src/core/telemetry/runtime.ts`:
- Around line 133-141: The inline sensitive-key pattern used by
INLINE_SENSITIVE_ASSIGNMENT_PATTERN and INLINE_SENSITIVE_FLAG_PATTERN must
recognize plural forms literals, regexes, and replacements, matching the keys
protected by isSensitiveKey. Extend SENSITIVE_INLINE_KEY_PATTERN accordingly and
add sanitizeString coverage for each plural assignment form, preserving existing
singular-key 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e2e4f607-35f1-4fa4-931b-1afcee9e4026
📒 Files selected for processing (16)
.agents/pm/extensions/.managed-extensions.json.agents/pm/history/pm-y3w0ld.jsonl.agents/pm/issues/pm-y3w0ld.toonCHANGELOG.mddocs/COMMANDS.mdsdk/public-surface.jsonsrc/cli/argv-utils.tssrc/cli/main.tssrc/core/telemetry/runtime.tssrc/sdk/command-line.tssrc/sdk/history-redact.tstests/unit/cli/argv-utils.spec.tstests/unit/commands/history-activity-command.spec.tstests/unit/core/telemetry/history-redact-sensitive-options.spec.tstests/unit/mcp/mcp-server-nested-options.spec.tstests/unit/sdk/history-read-primitives.spec.ts
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Address CodeRabbit findings by sanitizing inherited recovery payloads and sensitive flag typos, recognizing plural telemetry assignments, and preserving option tokens after value-less sensitive flags. Extend canary coverage and record the exact review evidence in pm.
Owner
Author
|
Review-round acknowledgment for exact head
|
Owner
Author
|
@coderabbitai full review |
Owner
Author
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 27 minutes. |
Owner
Author
|
Final bot-artifact ledger at exact head
Every current bot comment/review has now been inspected, usefulness-voted, and addressed or explicitly classified. |
Owner
Author
|
Tracked in PM: pm-y3w0ld. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Project management
Verification
Closes #864
Summary by Sourcery
Ensure history-redact never discloses matcher or replacement material across SDK, CLI, MCP, and telemetry, and update outputs to report only non-sensitive counts.
New Features:
Bug Fixes:
Enhancements:
Tests:
Summary by cubic
Make history-redact outputs disclosure-safe by replacing raw matchers/replacements with non-reversible counts and scrubbing sensitive inputs across CLI, SDK, MCP, recovery, and telemetry. Fixes GH-864 and closes diagnostic gaps so secrets never appear in success, failure, or diagnostics.
Bug Fixes
HistoryRedactResult.patternsnow reportsliteral_count,regex_count,total_count, andreplacement_is_default; removedliterals,regex, andreplacement.--literal,--regex,--replacement; inherited recovery fields are sanitized; equals-form typos are redacted; value-less sensitive flags preserve following option tokens.Migration
patterns.literals,patterns.regex, andpatterns.replacementwithliteral_count,regex_count,total_count, andreplacement_is_default.history.replacementsand the new counts for reporting.Written for commit 192f258. Summary will update on new commits.