Skip to content

fix(app-server): redact command execution display - #583

Open
andrei-hasna wants to merge 7 commits into
mainfrom
hotfix/app-server-display-redaction
Open

fix(app-server): redact command execution display#583
andrei-hasna wants to merge 7 commits into
mainfrom
hotfix/app-server-display-redaction

Conversation

@andrei-hasna

@andrei-hasna andrei-hasna commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Why

App-server command-execution display items can expose bearer-shaped values embedded in rendered commands and parsed command actions. Client-facing live, completed, and replayed items must be sanitized without changing the original command or actions used for approval and execution.

This is the app-server display-projection fix tracked by Todos task 4ce43a71-c3c6-4a7b-b378-f6b59866451f (OPE2-00351). It does not overlap PR #580 or the separate exec-output and interactive-history tasks.

What changed

  • Expand the shared secrets sanitizer to recognize the bearer shapes exercised by the upstream regression.
  • Build redacted command-execution display items while retaining the original executable command and actions internally.
  • Apply the display projection consistently to live updates, completion, thread-history replay, and parsed search actions.
  • Add protocol and app-server regressions covering redaction, approval/execution preservation, rejection behavior, and round trips.
  • Document the redacted display contract in the app-server reference.

Verification

  • Fail-before candidate: 46ec165765e085ee23898e64d3718777a92c5c1d.
  • Blacksmith Testbox run 31675994660: expected regression failure at command_execution_display_redacts_secrets_and_round_trips, proving the display projection still serialized the credential before the fix.
  • Fix candidate: f17228c771dd1727f9aaf95cb84ad7f8f6fe5c1f.
  • Blacksmith Testbox run 31676996079: pending on the exact fix candidate when this PR was opened; this body will be updated with the terminal result.
  • Focused/affected remote gates, scans, and independent adversarial review are required before the candidate is frozen.

Landing state

Do not merge from this PR yet. The exact candidate remains subject to the recorded Blacksmith, scan, and independent-review gates.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Add Testbox regression coverage for bearer-token redaction in command execution display items and sanitizer support for bearer token alphabets.

Agent: Hortensia-404
Redact bearer-like secrets from app-server command execution display items across live events, terminal items, guardian projections, and replay reconstruction while leaving executable approval request parameters unchanged.

Task: 4ce43a71-c3c6-4a7b-b378-f6b59866451f

Agent: Hortensia-404
Tighten the bearer-token redaction order so bearer credentials with OpenAI-style prefixes and +suffix text are consumed as one bearer token before generic key redaction can split them.

Regenerate the affected app-server schema fixtures so commandExecution display fields are documented as redacted display values while approval request schemas keep the raw executable command contract.

Task: 4ce43a71-c3c6-4a7b-b378-f6b59866451f

Agent: Hortensia-404
Regenerate the command execution display schema fixture descriptions to match write-app-server-schema output after the redacted-display doc change.

Task: 4ce43a71-c3c6-4a7b-b378-f6b59866451f

Agent: Hortensia-404
Match the JSON commandActions schema descriptions to the current write-app-server-schema wrapping.

Task: 4ce43a71-c3c6-4a7b-b378-f6b59866451f

Agent: Hortensia-404
Keep the aggregate approval request schema aligned with write-app-server-schema while leaving ThreadItem command display docs redacted.

Task: 4ce43a71-c3c6-4a7b-b378-f6b59866451f

Agent: Hortensia-404
Preserve app-server display redaction hotfix after PR #581 and PR #582 landed on main.

Agent: Hortensia-404
@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #583 @ 2995fec — lens: correctness+security+gates, reviewer Trebius (1 of 1)

Reviewed:

  • git log --oneline origin/main..HEAD — exit 0; seven candidate commits read.
  • git diff origin/main...HEAD --stat — exit 0; 29 files, 361 insertions, 103 deletions.
  • Full diff of all 29 changed files against freshly fetched origin/main at 3f11b4286a48361739e245c6a8df60b77fd80bbf, plus surrounding app-server protocol builders, thread-history reconstruction, approval/event handling, ThreadItem and CommandAction types, parser paths, sanitizer, integration tests, generated schemas, Cargo metadata, and README contract.
  • bun install — exit 0; setup only, 521 packages installed. It created an untracked bun.lock; that setup residue was removed and is not part of the candidate.
  • This repo declares no test gate in package.json (0 declared test commands, 0 run).
  • This repo declares no typecheck script in package.json (0 declared typecheck commands, 0 run).
  • gh pr view 583 --repo hasna/codewith --json statusCheckRollup — exit 0; 26 checks read: 11 SUCCESS, 0 failed, 3 SKIPPED, 12 IN_PROGRESS with no conclusion yet.

Forge checks read by name and conclusion:

  • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS; no conclusion.
  • Build Codewith Linux CLI — IN_PROGRESS; no conclusion.
  • cla — SUCCESS.
  • Check for spelling errors — SUCCESS.
  • Blob size policy — SUCCESS.
  • cargo-deny — SUCCESS.
  • build-test — SUCCESS.
  • Detect changed areas — SUCCESS.
  • python-sdk — SUCCESS.
  • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-musl — IN_PROGRESS; no conclusion.
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 1/4 — IN_PROGRESS; no conclusion.
  • sdks — IN_PROGRESS; no conclusion.
  • Format / etc — SUCCESS.
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 2/4 — IN_PROGRESS; no conclusion.
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 3/4 — IN_PROGRESS; no conclusion.
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 4/4 — IN_PROGRESS; no conclusion.
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm (native main) — SKIPPED.
  • cargo shear — SUCCESS.
  • Bazel clippy on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS; no conclusion.
  • Argument comment lint package — SKIPPED.
  • Bazel clippy on windows-latest for x86_64-pc-windows-gnullvm — IN_PROGRESS; no conclusion.
  • Verify release build on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS; no conclusion.
  • Argument comment lint - Linux — IN_PROGRESS; no conclusion.
  • Verify release build on windows-latest for x86_64-pc-windows-gnullvm — SUCCESS.
  • Argument comment lint - Windows — SUCCESS.
  • [code]smith — SKIPPED.

Blocking P0/P1 finding:

  • P1 — codex-rs/app-server-protocol/src/protocol/item_builders.rs:77: redact_command_actions redacts command and Search.query but passes through Read.name, Read.path, ListFiles.path, and Search.path. All are client-serialized members of the field the protocol now documents as a redacted display parsing, and all are populated from supported raw command operands. A valid command such as cat <bearer-bearing-path> or rg <safe-query> <bearer-bearing-path> therefore emits the credential in commandActions even while the top-level command is redacted. This is currently reachable through ordinary command parsing and is material to the PR's security acceptance. Remedy: redact every string/path derived from the raw command in CommandAction, preserving the absolute-path shape, and add focused regression coverage for Read/ListFiles/Search operands.

Non-blocking follow-ups:

  • The separate guardian auto-approval review action still carries its original command/argv contract. That surface predates this PR and is not the commandExecution display contract changed here; audit it separately if guardian review notifications also require confidentiality.
  • No change-size follow-up: the diff is 464 changed lines including generated schemas and stays within the review-size guidance.

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.

1 participant