Skip to content

feat(tui): confirmed opt-out for model-bound key redaction ([redaction] model_bound) - #5982

Open
SparkofSpike wants to merge 1 commit into
Hmbown:mainfrom
SparkofSpike:codex/feat-model-bound-redaction-gate
Open

feat(tui): confirmed opt-out for model-bound key redaction ([redaction] model_bound)#5982
SparkofSpike wants to merge 1 commit into
Hmbown:mainfrom
SparkofSpike:codex/feat-model-bound-redaction-gate

Conversation

@SparkofSpike

Copy link
Copy Markdown
Contributor

Why

CodeWhale already has a mandatory algorithm that redacts credential‑like information. However, this redaction causes unnecessary trouble during development. For example, I'm currently building a browser extension: the server backend generates an API key, which I need to paste into the extension's settings page to use it. But CodeWhale keeps redacting the key, wasting many tokens and even overwriting the API key with [redacted] when modifying files, which causes the key to be lost.

We should add a configurable option to disable redaction. I admit that while this design is inconvenient, it is there for privacy and security. In this PR’s changes, we’ve paid special attention to that: the warning will be shown twice, clearly stating the possible serious consequences.

Summary

Codewhale masks credential-shaped values in tool output before it is sent to an upstream model (configured secrets plus the CredentialShaped structural pass, see crates/config/src/persistence.rs). That boundary makes it impossible for the model to read and then quote files that contain real credentials: a read_file result is redacted, and a whole-file rewrite can overwrite the true token with [redacted].

This change adds a deliberate, documented opt-out that treats lowering that boundary as a security decision rather than a plain boolean:

  • New config table [redaction] model_bound = "enabled" (default) / "disabled". The value parses forgivingly: true/false, "on"/off", and any casing of "enabled"/"disabled" all work (false/"off" mean "disabled").
  • "disabled" only records a request. After a restart the interactive TUI shows a two-stage startup gate: the first screen explains the opt-out and shows a bold red warning; pressing 1/Y advances to a final-confirmation screen ("are you really sure?") that repeats the red warning, and only a second 1/Y persists a receipt to ~/.codewhale/redaction-state.json and takes effect.
  • The receipt is bound to the config.toml it was made against: setting the field back to "enabled", or rewriting config.toml after the confirmation (including an enabled→disabled round trip with zero processes in between), invalidates it, so a later "disabled" request always asks for a fresh confirmation.
  • Fail-closed everywhere: any unconfirmed request resolves to Enabled in every process, including headless/exec runs, hooks, and automations, which never confirm anything.
  • The opt-out only relaxes masking on structured ToolResult blocks of the main conversation. Routing/classification summaries and durable goal-state text keep always-on redaction.

Changes

  • crates/config/src/redaction.rs (new): [redaction] types, forgiving ModelBoundMasking deserialization, the receipt state file, confirmation_required / effective_masking decisions, and the config-mtime-bound receipt invalidation with best-effort sweep.
  • crates/config/src/lib.rs: ConfigToml.redaction field plus redaction_model_bound_masking() accessor.
  • crates/tui/src/config.rs: TUI Config accepts [redaction] and exposes model_bound_redaction(); merge_config carries the table.
  • crates/tui/src/client.rs: DeepSeekClient.model_bound_masking — set only when the opt-out is confirmed — gates the ToolResult redaction in prepare_model_bound_request; other redaction paths are untouched.
  • crates/tui/src/tui/redaction_gate.rs (new): two-stage gate rendering with the red bold warning (palette STATUS_ERROR + BOLD) and explicit-key actions.
  • crates/tui/src/tui/app.rs / app/init.rs: redaction_gate / redaction_gate_confirming state on App.
  • crates/tui/src/tui/ui/event_loop.rs: arms the gate at startup before the engine spawns; two-stage key handling (1/Y confirm, 2/U back-or-keep, 3/N and Esc quit, Enter never confirms); on final confirm the engine is rebuilt (Shutdown + spawn_tui_engine) so the client picks up the effective mode, same pattern as the provider-rollback respawn.
  • crates/tui/src/tui/ui/frame.rs: gate renders full-screen above the launch surface while active.
  • Localization: 13 new RedactionGate* MessageIds added to the enum and ALL_MESSAGE_IDS, translated in all 15 complete locale packs (the zh packs use the polite 您 and include the requested red warning).
  • Docs: docs/CONFIGURATION.md section and config.example.toml comments document the table, the two-stage confirmation, the receipt-binding rule, and the forgiving value spellings.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Tests

Testing

  • cargo fmt --all -- --check — clean
  • cargo clippy -p codewhale-config --all-targets — no warnings
  • cargo test -p codewhale-config — 640 passed, 0 failed (includes the redaction decision/lifecycle tests: default-path lifecycle, forgiving boolean/on-off spellings, corrupt receipt, enabled-invalidates-receipt, config-rewrite-invalidates-receipt)
  • cargo test -p codewhale-tui --lib opt_out — 9 passed (includes the two new regression tests: confirmed opt-out keeps tool output byte-exact; unconfirmed request stays masked)
  • cargo test -p codewhale-tui --lib redaction_gate — 3 passed (two-stage rails and red warning, narrow-terminal rendering for both stages)
  • cargo test -p codewhale-tui --lib localization::tests — 50 passed (15-locale key parity)
  • cargo build --release -p codewhale-tui — succeeds; the executable was exercised (--version, config parsing with model_bound = "disabled")
  • Manual TUI verification of the two-stage gate flow by the author

Note: cargo clippy --workspace and cargo test --workspace were not run in full (Windows, long release builds); the focused crate suites above are green and the PR CI gate will run the workspace gates. On Windows the tui unit-test binary can hit a stack overflow under the default 2 MB thread stack with the v0.9.12 codebase; the affected suites were verified with RUST_MIN_STACK=16MB, and the config lifecycle test includes a 300 ms backoff for the transient exclusive file lock Windows real-time AV holds on freshly written files (commented in the test).

Checklist

  • Updated docs or comments as needed
  • Added or updated tests where relevant
  • Verified TUI behavior manually if UI changes
  • Harvested/co-authored credit uses a GitHub numeric noreply address

Related Issues

No-Issue: feature request from the author (editing files that contain real credentials); no issue number exists yet.

@SparkofSpike
SparkofSpike force-pushed the codex/feat-model-bound-redaction-gate branch from 34d1e2b to 59fb0c7 Compare September 7, 2026 06:54
@SparkofSpike
SparkofSpike marked this pull request as ready for review September 7, 2026 12:10
@SparkofSpike
SparkofSpike requested a review from Hmbown as a code owner September 7, 2026 12:10
@Hmbown

Hmbown commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Why

CodeWhale already has a mandatory algorithm that redacts credential‑like information. However, this redaction causes unnecessary trouble during development. For example, I'm currently building a browser extension: the server backend generates an API key, which I need to paste into the extension's settings page to use it. But CodeWhale keeps redacting the key, wasting many tokens and even overwriting the API key with [redacted] when modifying files, which causes the key to be lost.

We should add a configurable option to disable redaction. I admit that while this design is inconvenient, it is there for privacy and security. In this PR’s changes, we’ve paid special attention to that: the warning will be shown twice, clearly stating the possible serious consequences.

Summary

Codewhale masks credential-shaped values in tool output before it is sent to an upstream model (configured secrets plus the CredentialShaped structural pass, see crates/config/src/persistence.rs). That boundary makes it impossible for the model to read and then quote files that contain real credentials: a read_file result is redacted, and a whole-file rewrite can overwrite the true token with [redacted].

This change adds a deliberate, documented opt-out that treats lowering that boundary as a security decision rather than a plain boolean:

  • New config table [redaction] model_bound = "enabled" (default) / "disabled". The value parses forgivingly: true/false, "on"/off", and any casing of "enabled"/"disabled" all work (false/"off" mean "disabled").

  • "disabled" only records a request. After a restart the interactive TUI shows a two-stage startup gate: the first screen explains the opt-out and shows a bold red warning; pressing 1/Y advances to a final-confirmation screen ("are you really sure?") that repeats the red warning, and only a second 1/Y persists a receipt to ~/.codewhale/redaction-state.json and takes effect.

  • The receipt is bound to the config.toml it was made against: setting the field back to "enabled", or rewriting config.toml after the confirmation (including an enabled→disabled round trip with zero processes in between), invalidates it, so a later "disabled" request always asks for a fresh confirmation.

  • Fail-closed everywhere: any unconfirmed request resolves to Enabled in every process, including headless/exec runs, hooks, and automations, which never confirm anything.

  • The opt-out only relaxes masking on structured ToolResult blocks of the main conversation. Routing/classification summaries and durable goal-state text keep always-on redaction.

Changes

  • crates/config/src/redaction.rs (new): [redaction] types, forgiving ModelBoundMasking deserialization, the receipt state file, confirmation_required / effective_masking decisions, and the config-mtime-bound receipt invalidation with best-effort sweep.

  • crates/config/src/lib.rs: ConfigToml.redaction field plus redaction_model_bound_masking() accessor.

  • crates/tui/src/config.rs: TUI Config accepts [redaction] and exposes model_bound_redaction(); merge_config carries the table.

  • crates/tui/src/client.rs: DeepSeekClient.model_bound_masking — set only when the opt-out is confirmed — gates the ToolResult redaction in prepare_model_bound_request; other redaction paths are untouched.

  • crates/tui/src/tui/redaction_gate.rs (new): two-stage gate rendering with the red bold warning (palette STATUS_ERROR + BOLD) and explicit-key actions.

  • crates/tui/src/tui/app.rs / app/init.rs: redaction_gate / redaction_gate_confirming state on App.

  • crates/tui/src/tui/ui/event_loop.rs: arms the gate at startup before the engine spawns; two-stage key handling (1/Y confirm, 2/U back-or-keep, 3/N and Esc quit, Enter never confirms); on final confirm the engine is rebuilt (Shutdown + spawn_tui_engine) so the client picks up the effective mode, same pattern as the provider-rollback respawn.

  • crates/tui/src/tui/ui/frame.rs: gate renders full-screen above the launch surface while active.

  • Localization: 13 new RedactionGate* MessageIds added to the enum and ALL_MESSAGE_IDS, translated in all 15 complete locale packs (the zh packs use the polite 您 and include the requested red warning).

  • Docs: docs/CONFIGURATION.md section and config.example.toml comments document the table, the two-stage confirmation, the receipt-binding rule, and the forgiving value spellings.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

  • New feature (non-breaking change which adds functionality)

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

  • Documentation update

  • Refactoring (no functional changes)

  • Tests

Testing

  • cargo fmt --all -- --check — clean

  • cargo clippy -p codewhale-config --all-targets — no warnings

  • cargo test -p codewhale-config — 640 passed, 0 failed (includes the redaction decision/lifecycle tests: default-path lifecycle, forgiving boolean/on-off spellings, corrupt receipt, enabled-invalidates-receipt, config-rewrite-invalidates-receipt)

  • cargo test -p codewhale-tui --lib opt_out — 9 passed (includes the two new regression tests: confirmed opt-out keeps tool output byte-exact; unconfirmed request stays masked)

  • cargo test -p codewhale-tui --lib redaction_gate — 3 passed (two-stage rails and red warning, narrow-terminal rendering for both stages)

  • cargo test -p codewhale-tui --lib localization::tests — 50 passed (15-locale key parity)

  • cargo build --release -p codewhale-tui — succeeds; the executable was exercised (--version, config parsing with model_bound = "disabled")

  • Manual TUI verification of the two-stage gate flow by the author

Note: cargo clippy --workspace and cargo test --workspace were not run in full (Windows, long release builds); the focused crate suites above are green and the PR CI gate will run the workspace gates. On Windows the tui unit-test binary can hit a stack overflow under the default 2 MB thread stack with the v0.9.12 codebase; the affected suites were verified with RUST_MIN_STACK=16MB, and the config lifecycle test includes a 300 ms backoff for the transient exclusive file lock Windows real-time AV holds on freshly written files (commented in the test).

Checklist

  • Updated docs or comments as needed

  • Added or updated tests where relevant

  • Verified TUI behavior manually if UI changes

  • Harvested/co-authored credit uses a GitHub numeric noreply address

Related Issues

No-Issue: feature request from the author (editing files that contain real credentials); no issue number exists yet.

Thank you so much for this!!

@Hmbown

Hmbown commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Hi @SparkofSpike — thank you for this, seriously. This is a well-shaped contribution to a genuinely hard boundary: the double-confirmation gate, the receipt bound to the exact config.toml bytes, the headless/exec fail-closed default, and the config-rewrite invalidation are exactly the properties we would have demanded in review. The doc comments and config.example.toml prose are excellent.

Status and next steps:

Thanks again — real users hitting real friction (dev-loop key pasting) with a security-careful fix is exactly the contribution that makes this project better.

The model boundary masks credential-shaped tool output before it reaches an
upstream model (configured secrets plus a CredentialShaped structural pass).
Editing files that contain real credentials has always been awkward because
the model cannot quote those bytes back. This adds a deliberate, documented
opt-out that treats lowering the boundary as a security decision:

- config.toml gains [redaction] model_bound = "enabled" (default) | "disabled";
  "disabled" only records a request.
- After a restart the interactive TUI shows a full-screen startup gate (same
  explicit-key discipline as workspace trust: 1/Y confirm, 2/U keep, 3/N/Esc
  quit; Enter never confirms by reflex).
- Confirming persists a receipt to ~/.codewhale/redaction-state.json and
  rebuilds the engine so its client applies the opt-out. Until a receipt
  exists every process - including headless/exec runs - stays masked.
- The client opt-out covers structured ToolResult blocks only; routing/
  classification summaries and durable goal-state text keep always-on
  redaction.
- Gate copy is localized across all 15 shipped locale packs; enum, JSON, and
  ALL_MESSAGE_IDS stay in sync.
@SparkofSpike

Copy link
Copy Markdown
Contributor Author

Thanks for the review pass and the scope call — next-release timing is fine on our side.

The branch is now rebased onto current main (b0c5241); the only conflict was the validate_base_url_security signature change, resolved by keeping both the new argument and the confirmation-gated masking decision. CI is green on the new base (all checks except the slow macos tail).

On the four review points, current state maps as follows:

  1. Fail-closed reads — read_state returns the safe default for absent/unreadable/malformed receipts (absent_state_is_not_confirmed, corrupt_state_reads_as_unconfirmed pin it); effective_masking is the single decision point every caller resolves through.
  2. No replay across config edits — the receipt is bound by config.toml mtime, so any rewrite after the confirmation (including an enabled→disabled round trip with zero processes in between) invalidates it and forces a fresh confirmation; default_path_lifecycle_requires_confirmation_before_disabling covers exactly that round trip.
  3. Non-interactive entries — record_model_bound_disabled_confirmation is reachable only from the interactive startup gate; exec/hooks/app-server resolve effective_masking to Enabled whenever no fresh receipt exists.
  4. Tests — each case above is pinned in crates/config/src/redaction.rs plus the gate and client opt-out tests.

Happy to adjust anything the review pass turns up.

@SparkofSpike

Copy link
Copy Markdown
Contributor Author

Well, the Test CI has timed out. Awaiting manual merge.

Hmbown pushed a commit that referenced this pull request Sep 8, 2026
…umed history

Keep selected config receipts isolated, defer initial and external input until
consent is answered, and restore the Engine conversation before dispatch.
Make gate copy scrollable and reuse the bounded onboarding text wrapper.
Follow up on contributor PR #5982 by @SparkofSpike.

Validation: 35 focused Rust tests passed, 0 failed (2 nextest leaky reports;
those 2 passed without leaks in isolated recheck). Rendered 90 combinations
of viewport, locale and gate stage. Original/repaired wrap probe: 9/0
overflowing lines. Offline config+TUI test check, fmt and diff checks passed.
Root npm test and npm run check:web unavailable: both scripts missing.
Only synthetic local fixtures; no live-provider or release claim.
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