Repository navigation
feat(terminal): redact copied command blocks - #222
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis change adds command-block redaction storage, validation, APIs, synchronization, settings, and clipboard processing for text and images. It also adds prompt detection, soft-wrap handling, migration coverage, and localized configuration messages. ChangesCommand-block redaction
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalPane
participant appStore
participant commandBlockRedaction
participant ImageRenderer
participant Clipboard
TerminalPane->>appStore: load command-block redaction settings
appStore-->>TerminalPane: return settings and rules
TerminalPane->>commandBlockRedaction: redact block text
commandBlockRedaction-->>TerminalPane: return redacted text
TerminalPane->>ImageRenderer: render blocks with redaction
ImageRenderer-->>TerminalPane: return redacted image blob
TerminalPane->>Clipboard: write redacted output
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
There was a problem hiding this comment.
Pull request overview
This PR adds a new “command-block copy redaction” feature for terminal copy-as-text and copy-as-image flows, including prompt recognition, per-block redaction rules, UI controls, persistence/migration, and sync import/export (with legacy payload compatibility).
Changes:
- Introduce shell prompt detection and apply prompt replacement + regex-based redaction per command block for text and image copying.
- Add UI and frontend store plumbing to manage prompt/redaction settings and CRUD redaction rules.
- Persist/sync the new redaction category via DB schema v26, new command APIs (Tauri + headless server dispatcher), and sync payload import/export (including legacy import key support).
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/lib/terminal/prompt.ts | Add prompt-recognition heuristics for common shells/prompts. |
| src/lib/terminal/prompt.test.ts | Add unit tests for prompt detection. |
| src/lib/terminal/command-block-redaction.ts | Implement rule compilation and per-block text redaction (prompt + regex rules). |
| src/lib/terminal/command-block-redaction.test.ts | Add unit tests for command-block text redaction behavior and fail-closed semantics. |
| src/lib/terminal/block-to-image.ts | Add optional command-block redaction to image rendering path; implement row/cell-level redaction. |
| src/lib/terminal/block-to-image.test.ts | Add tests for image-row redaction behavior. |
| src/lib/terminal/block-content.ts | Add extractBlockTexts to preserve per-block boundaries for copy-time policies. |
| src/lib/terminal/block-content.test.ts | Add coverage for multi-block boundary extraction. |
| src/lib/stores/app.svelte.ts | Add store state + loading/saving APIs for command-block copy redaction settings and rule CRUD. |
| src/lib/i18n/locales/zh.ts | Add Chinese strings for the new settings UI and sync category label. |
| src/lib/i18n/locales/en.ts | Add English strings for the new settings UI and sync category label. |
| src/lib/components/TerminalPane.svelte | Apply command-block redaction during “copy as text” and “copy as image”. |
| src/lib/components/SyncScreen.svelte | Add sync category toggle for command-block redaction rules. |
| src/lib/components/CommandBlockSettings.svelte | Add settings UI to edit prompt redaction + redaction rules with CRUD actions. |
| src-tauri/src/sync/config.rs | Add import/export + sync prefs support for command_block_redaction (and legacy import key). |
| src-tauri/src/server.rs | Expose command-block rule CRUD in the headless/JetBrains dispatcher. |
| src-tauri/src/redaction.rs | Introduce shared regex validation (invalid + zero-width) for user-managed redaction rules. |
| src-tauri/src/lib.rs | Wire up new module + register new Tauri commands. |
| src-tauri/src/db/schema.rs | Bump schema to v26; add command_block_redact_rules + seed/snapshot migration logic. |
| src-tauri/src/db/mod.rs | Export the new command_block_redact_rule DB module. |
| src-tauri/src/db/command_block_redact_rule.rs | Add DB persistence layer for command-block redaction rules. |
| src-tauri/src/commands/sync.rs | Ensure sync command tests accept the new payload category. |
| src-tauri/src/commands/mod.rs | Register the new command-block command module. |
| src-tauri/src/commands/command_block.rs | Add list/save/delete APIs + Tauri commands for command-block rule CRUD. |
| src-tauri/src/ai/redact_rules.rs | Reuse shared regex validation for AI redaction rule saving. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export interface CommandBlockRedactionRule { | ||
| pattern: string; | ||
| replacement: string; | ||
| } |
There was a problem hiding this comment.
Verified. The redaction engine intentionally accepts the minimal pattern/replacement shape; persisted rules extend it with id. I removed persistence-only ids from the engine test literals and also pass explicit fields to the Tauri invoke so the feature adds no TypeScript errors.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
src/lib/terminal/block-to-image.ts (2)
175-180: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
applyRulesreturns the caller's cell objects when the rule list is empty.
reduceover an emptyrulesarray returns the seed. The seed[...cells]is a new array, but it holds the originalImageCellreferences. Every other path in this file returns fresh objects. A caller that mutates a returned row then mutates the input row.No current caller does this, so this is defensive only.
♻️ Proposed change
- return rules.reduce<ImageCell[]>((current, rule) => applyRule(current, rule), [...cells]); + return rules.reduce<ImageCell[]>( + (current, rule) => applyRule(current, rule), + cells.map((cell) => ({ ...cell })), + );🤖 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/lib/terminal/block-to-image.ts` around lines 175 - 180, Update applyRules to return fresh ImageCell objects even when rules is empty, not merely a shallow-copied array containing the original references. Preserve the existing rule-application behavior while ensuring every returned cell is cloned independently from the input cells.
91-113: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
cellsFromTextsplits grapheme clusters into separate cells.
Array.fromiterates code points, not grapheme clusters. A replacement string that contains a combining mark, a variation selector, or a ZWJ emoji sequence becomes several cells.graphemeWidththen assigns a width to each part, so a ZWJ family emoji renders as several wide cells instead of one. xterm stores such a cluster in a single cell.This affects only the replacement text, which the user controls. If you want to keep it simple, document the limitation next to
graphemeWidth. Otherwise useIntl.Segmenterwithgranularity: "grapheme".🤖 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/lib/terminal/block-to-image.ts` around lines 91 - 113, Update cellsFromText to segment replacement text by grapheme clusters using Intl.Segmenter with granularity "grapheme" rather than Array.from code-point iteration, then assign one cell and one graphemeWidth result per segment. Keep the existing style and character storage behavior, and preserve the current width fallback or document the limitation beside graphemeWidth if segmentation cannot be supported.src/lib/components/CommandBlockSettings.svelte (1)
116-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider validating the pattern as a regular expression before saving.
saveRulerejects only empty strings. A syntactically invalid pattern is accepted here and then rejected later. BecausecompileCommandBlockRedactionfails closed, one bad rule blocks every command-block copy until the user finds and fixes it. A localnew RegExp(pattern)check gives immediate feedback at the point of entry.♻️ Proposed change
if (editingRule.pattern === "" || editingRule.replacement === "") { redactNote = t("settings.shell.command_block_redact_error_empty"); return; } + try { + new RegExp(editingRule.pattern.replace(/^\(\?[ims]+\)/, "")); + } catch (error) { + redactNote = t("settings.shell.command_block_redact_error_save", { error: errMsg(error) }); + return; + }🤖 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/lib/components/CommandBlockSettings.svelte` around lines 116 - 134, Update saveRule to validate editingRule.pattern with new RegExp before setting savingRule or calling app.saveCommandBlockRedactRule; catch syntax errors, set the existing redactNote using the appropriate validation message, and return while preserving the current empty-value and successful-save flows.src/lib/components/TerminalPane.svelte (1)
411-419: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm the silent return on an empty redacted result is intended.
If a rule redacts a block to an empty string,
copyBlocksAsTextreturns beforewriteClipboardand beforeclearBlockSelection. The user sees no clipboard change and no message. The old clipboard content stays in place, which is the safe outcome, but the copy action appears to do nothing.Consider a short toast so the user knows the copy was suppressed rather than failed silently.
🤖 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/lib/components/TerminalPane.svelte` around lines 411 - 419, Update copyBlocksAsText around the empty redacted text check so suppressed copies provide user feedback before returning. Add a brief toast indicating that copying was skipped because the selected content was redacted, while preserving the existing behavior of avoiding writeClipboard and clearBlockSelection.src/lib/terminal/command-block-redaction.test.ts (1)
48-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the zero-width rejection contract.
redactionRuleMatchesthrowsredact_zero_width_patternfor a pattern that matches an empty string. That guard prevents an infinite loop, and no test covers it. A pattern such asa*is valid JavaScript regex, so it passes compilation and reaches the guard.💚 Proposed test
+ it("rejects a zero-width pattern instead of looping", () => { + expect(() => redactCommandBlockTexts(["$ id"], { + promptEnabled: true, + promptReplacement: "anonymous@rssh", + rules: [{ pattern: "x*", replacement: "<X>" }], + })).toThrow(/redact_zero_width_pattern/); + });🤖 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/lib/terminal/command-block-redaction.test.ts` around lines 48 - 54, Add a test alongside the existing invalid-rule test in command-block-redaction.test.ts that calls redactCommandBlockTexts with a valid zero-width-capable rule such as pattern “a*”, then asserts it throws the redact_zero_width_pattern error. Keep the test focused on the zero-width rejection contract and preserve the current rule configuration style.src/lib/terminal/command-block-redaction.ts (1)
30-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInline-flag translation accepts only a prefix group and drops nothing silently.
The translation handles the common
(?ims)prefix. Two Rust forms are not handled: a negated group such as(?-i)and a mid-pattern group such asfoo(?i)bar. Both remain insourceand then failnew RegExp, which fails closed. That behavior is correct, but the failure message reaches the user as a generic regex error at copy time.Consider rejecting these forms in the shared backend validator so the user sees the error while editing the rule, not while copying.
🤖 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/lib/terminal/command-block-redaction.ts` around lines 30 - 44, Update the shared backend validator for CommandBlockRedactionRule patterns to reject unsupported Rust inline-flag forms, including negated groups such as (?-i) and non-prefix groups such as foo(?i)bar, before compileRule is called. Reuse the validator’s existing validation-error mechanism so users receive the actionable error while editing, while preserving compileRule’s current supported-prefix translation and fail-closed behavior.
🤖 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/lib/stores/app.svelte.ts`:
- Around line 825-841: Remove the `_commandBlockRedactionLoaded = true`
assignments from `setCommandBlockPromptRedactEnabled` and
`setCommandBlockPromptReplacement`; these setters should update persisted and
in-memory values without claiming the cache was loaded. Set
`_commandBlockRedactionLoaded` only after `loadCommandBlockRedaction()`
successfully reads the backend, unless the setters explicitly merge into an
already loaded snapshot.
In `@src/lib/terminal/block-to-image.test.ts`:
- Around line 345-348: Remove the id properties from the redaction rule objects
in the test rules array, keeping only pattern and replacement so they conform to
CommandBlockRedactionRule and remain compatible with compileRule.
In `@src/lib/terminal/block-to-image.ts`:
- Around line 182-197: Update ImageRow and extractImageRows to preserve each
buffer line’s isWrapped state, then revise redactImageRows to combine
continuation rows into logical text before calling commandBlockPromptEnd. Apply
the prompt replacement to the originating prompt row while preserving the
remaining cells and row structure, and add a regression test covering a prompt
split across an isWrapped continuation row.
In `@src/lib/terminal/prompt.ts`:
- Around line 28-31: Update the prompt-detection regexes in the terminal prompt
patterns so their leading prefix quantifiers are lazy, ensuring matching stops
at the first prompt marker rather than the last. Decide and preserve the
intended behavior for multi-segment Powerline prompts, then add coverage for
that case and for command text containing prompt markers.
---
Nitpick comments:
In `@src/lib/components/CommandBlockSettings.svelte`:
- Around line 116-134: Update saveRule to validate editingRule.pattern with new
RegExp before setting savingRule or calling app.saveCommandBlockRedactRule;
catch syntax errors, set the existing redactNote using the appropriate
validation message, and return while preserving the current empty-value and
successful-save flows.
In `@src/lib/components/TerminalPane.svelte`:
- Around line 411-419: Update copyBlocksAsText around the empty redacted text
check so suppressed copies provide user feedback before returning. Add a brief
toast indicating that copying was skipped because the selected content was
redacted, while preserving the existing behavior of avoiding writeClipboard and
clearBlockSelection.
In `@src/lib/terminal/block-to-image.ts`:
- Around line 175-180: Update applyRules to return fresh ImageCell objects even
when rules is empty, not merely a shallow-copied array containing the original
references. Preserve the existing rule-application behavior while ensuring every
returned cell is cloned independently from the input cells.
- Around line 91-113: Update cellsFromText to segment replacement text by
grapheme clusters using Intl.Segmenter with granularity "grapheme" rather than
Array.from code-point iteration, then assign one cell and one graphemeWidth
result per segment. Keep the existing style and character storage behavior, and
preserve the current width fallback or document the limitation beside
graphemeWidth if segmentation cannot be supported.
In `@src/lib/terminal/command-block-redaction.test.ts`:
- Around line 48-54: Add a test alongside the existing invalid-rule test in
command-block-redaction.test.ts that calls redactCommandBlockTexts with a valid
zero-width-capable rule such as pattern “a*”, then asserts it throws the
redact_zero_width_pattern error. Keep the test focused on the zero-width
rejection contract and preserve the current rule configuration style.
In `@src/lib/terminal/command-block-redaction.ts`:
- Around line 30-44: Update the shared backend validator for
CommandBlockRedactionRule patterns to reject unsupported Rust inline-flag forms,
including negated groups such as (?-i) and non-prefix groups such as foo(?i)bar,
before compileRule is called. Reuse the validator’s existing validation-error
mechanism so users receive the actionable error while editing, while preserving
compileRule’s current supported-prefix translation and fail-closed behavior.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1dd65558-3642-4b68-bb57-381b106a6ae3
📒 Files selected for processing (25)
src-tauri/src/ai/redact_rules.rssrc-tauri/src/commands/command_block.rssrc-tauri/src/commands/mod.rssrc-tauri/src/commands/sync.rssrc-tauri/src/db/command_block_redact_rule.rssrc-tauri/src/db/mod.rssrc-tauri/src/db/schema.rssrc-tauri/src/lib.rssrc-tauri/src/redaction.rssrc-tauri/src/server.rssrc-tauri/src/sync/config.rssrc/lib/components/CommandBlockSettings.sveltesrc/lib/components/SyncScreen.sveltesrc/lib/components/TerminalPane.sveltesrc/lib/i18n/locales/en.tssrc/lib/i18n/locales/zh.tssrc/lib/stores/app.svelte.tssrc/lib/terminal/block-content.test.tssrc/lib/terminal/block-content.tssrc/lib/terminal/block-to-image.test.tssrc/lib/terminal/block-to-image.tssrc/lib/terminal/command-block-redaction.test.tssrc/lib/terminal/command-block-redaction.tssrc/lib/terminal/prompt.test.tssrc/lib/terminal/prompt.ts
Summary
Compatibility
New exports write
command_block_redaction; imports also accept the legacycommand_block_image_redactionfield. AI handoff keeps its existing independent redaction path.Tests
npm test— 621 passednpm run buildcargo fmt --manifest-path src-tauri/Cargo.toml --checkcargo test --manifest-path src-tauri/Cargo.toml— 702 passedcargo check --manifest-path src-tauri/Cargo.toml --features cli --bin rssh-clicargo check --manifest-path src-tauri/Cargo.toml --features server --bin rssh-serverSummary by CodeRabbit