Skip to content

fix: redact clipboard and CDP secrets in recorded steps (AIENG-228) - #52

Merged
cryptotavares merged 7 commits into
mainfrom
cryptotavares/aieng-228-redact-recorded-secrets
Oct 16, 2026
Merged

cryptotavares merged 7 commits into
mainfrom
cryptotavares/aieng-228-redact-recorded-secrets

Conversation

@cryptotavares

@cryptotavares cryptotavares commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Addresses the upstream fix for AIENG-228, under AIENG-128.

Summary: Recorded steps redact browser/mobile clipboard text and all CDP params, including nested batches. Targeted echo scrubbing protects diagnostic content while preserving metadata and original tool execution.

Problem

Onboarding sends the SRP through clipboard.text and the password through cdp.params.text. Neither field matches the knowledge store's sensitive-name policy. Step files retain credentials that an execute agent can read. Pool wallets are reused across runs.

Errors and observations can also copy the input into the same step.

sequenceDiagram
    participant O as Onboarding
    participant T as Tool handler
    participant K as Knowledge store
    participant F as Step JSON
    O->>T: clipboard write SRP / CDP insert password
    T-->>O: Original execution result
    T->>K: Input, outcome, observation
    K->>K: Check input field names
    Note over K: text and params do not match
    K->>F: Persist plaintext input and echoed copies
Loading

Solution

Apply the recording policy before the knowledge store writes each step.

  1. Redact inputs. Replace clipboard text and CDP params with [REDACTED]. Recurse through run_steps, including the mobile device-clipboard alias.
  2. Scrub echoes. Match raw and JSON-escaped strings in errors, targets, observations, and screenshot-path metadata.
  3. Preserve structure. Keep timestamps, session IDs, environment fields, labels, accessibility roles/references, and error codes intact.
  4. Validate recording paths. Parse session IDs and tool names before filesystem operations. Use only checked values in paths and metadata, addressing CodeQL alerts 70 and 71.
flowchart TD
    A[Validated tool input] --> B[Execute tool with original input]
    B --> C[Collect outcome and observation]
    C --> P{Safe session ID and tool name?}
    P -->|yes| D[Redact clipboard text and CDP params]
    P -->|no| X[Reject recording. No filesystem operations.]
    D --> E[Recurse through nested batch inputs]
    E --> F[Scrub copied input in diagnostic content]
    F --> G[Keep structural metadata intact]
    G --> H[Write recorded step JSON]
    B --> I[Return original result to caller]

    classDef new fill:#fff3cd,stroke:#d39e00
    classDef stop fill:#f8d7da,stroke:#c82333
    class P,D,E,F,G new
    class X stop
Loading

Yellow boxes are new. Red boxes stop the recording.

What changes

Success and error recordings use this policy. Tools and HTTP responses retain original values.

Call or condition Before After
clipboard write Records plaintext text Records [REDACTED], textRedacted: true, and textLength
device_clipboard write Records plaintext text Applies the same clipboard policy
CDP call with params Records the parameter object Records params: "[REDACTED]"; retains method and call metadata
Nested run_steps Redacts named-sensitive fields and password-targeted typing, but retains clipboard/CDP input Extends recursive redaction to clipboard/CDP inputs and collects their echoes
Raw or escaped echo, at least eight characters Persists the copied string Replaces matching substrings in targeted diagnostic content
Echo shorter than eight characters Persists the copied string Redacts whole-value matches; retains embedded substrings
CDP keystrokes 2 or o Records original parameters Redacts parameters; preserves timestamps, session IDs, and ordinary observation substrings
Error recording Persists the original message/details Scrubs matching copies; retains the error code
Clipboard read or parameterless CDP call Retains input metadata Retains input metadata; adds no redaction flag
Existing sensitive-field or password-targeted type input Redacts sensitive input Retains that policy and adds targeted echo scrubbing
Unsafe session ID or tool name Uses the value in the recording path Rejects the recording before directory creation or file writes

Decisions for reviewers

Decision Reason
Deliver the upstream portion of AIENG-228 Consumer updates depend on this recording fix and its release.
Include device_clipboard and its batch alias User-approved extension. Mobile clipboard writes carry the same sensitive text.
Redact all CDP params Credentials can occur in nested parameters or evaluation calls. An allowlist could miss another input path.
Target echo scrubbing; use the eight-character boundary User-approved policy. Global substring replacement corrupts metadata for common letters and digits.
Preserve structural fields even when they match input Scrubbing record identity can break sorting, lookup, and error interpretation.
Preserve execution and response values Protect persisted knowledge without changing browser/device actions or caller responses.
Keep persistence tests separate from matching-policy tests Seven HTTP-to-disk cases cover recording boundaries. Focused unit tests cover matching and metadata without repeated server setup.
Restrict recording components to ASCII letters, digits, ., _, and - Blocks path separators and control characters. Empty values, . and .. are rejected. Custom names with spaces or other punctuation now fail recording validation.
Separate secret matching from record-field policy Named helpers isolate recursive values, observations, prior knowledge, and targets. Plain conditional updates replace nested conditional spreads.

Review guide

Read the files in this order:

# File What to check
1 src/knowledge-store/knowledge-store.ts Input policy, recursive batch handling, and sanitization immediately before persistence
2 src/knowledge-store/utils/redaction.ts Start with redactRecordedSecrets; follow the matching, observation, and prior-knowledge helpers. Check preserved metadata beside each field policy.
3 Tests: src/server/recorded-secrets.test.ts, src/knowledge-store/utils/redaction.test.ts, src/knowledge-store/knowledge-store.test.ts Persistence/execution boundaries first; pure matching rules next; recorder compatibility last
4 README.md Recorded-input policy and echo-matching behavior
Testing
  • CI=true yarn lint passes all lint stages.
  • CI=true yarn build passes.
  • yarn tsc --project tsconfig.build.json --noEmit passes.
  • CI=true yarn test passes: 48 files, 1,605 tests, coverage thresholds, build/packing, and attw --pack.
  • CI=true yarn vitest run src/server/recorded-secrets.test.ts src/knowledge-store/utils/redaction.test.ts src/knowledge-store/knowledge-store.test.ts --coverage.enabled=false passes: three files, 163 cases, including existing tests.
  • git diff --check passes.

The acceptance regression sends synthetic onboarding inputs through real HTTP handlers and scans every generated step file. Browser/device I/O is mocked; this verifies the recording boundary, not a live wallet import.

Redaction coverage has 16 cases: seven HTTP-to-disk cases, seven pure-policy cases, and two recorder-compatibility cases. Integration cases cover onboarding, nested batches and aliases, normal errors, server exceptions, and a non-redaction control. Unit cases cover short strings, escaped matching, scoped content, metadata, idempotence, and nonmutation.

Path-validation tables check ten unsafe values against both recording components and three legitimate name combinations. Rejected values trigger neither mkdir nor writeFile. Direct callers receive Invalid sessionId: expected a safe filename component or the equivalent toolName error. The server preserves tool responses when recording fails.

The audit reduces feature-test source from 1,003 to 646 lines. The integration file falls to 264 lines. The subsequent merge adds one focused regression for defensive batch handling together with recursive echo scrubbing. Existing tests remain intact.

Mutation checks on the leaner suite:

  • Bypass recording sanitization: six of seven integration cases fail; the non-redaction control passes.
  • Restore global short-string substring scrubbing: both 2/o regression cases fail on corrupted metadata.
  • Remove escaped matching: the scoped-content test fails on copied credentials.
  • Bypass either recording-component guard: all ten path-rejection cases fail.

Production mutations are restored before final verification. Production and configuration files are unchanged by the test audit. Current coverage is 96.29% statements, 91.60% branches, 94.36% functions, and 96.53% lines. The branch retains main's raised coverage thresholds.

The readability refactor reuses the existing tests. A temporary old/new comparison verifies equivalent behavior across 4,246 cases, including escaping, boundaries, optional fields, Dates, overlaps, and nonmutation.

Measured with the same ESLint complexity rule, redactRecordedSecrets drops from 11 to 7, and from 167 to 41 lines. No function exceeds complexity 7 or 47 lines. The file grows from 200 to 314 lines because helpers have explicit signatures and documentation.

tsconfig.test.json has eight pre-existing errors in mobile-driver, mock-network, and screenshot tests, reproduced against the base recorder. Production checks and the full test command pass.

Independent code review confirms the revision has no remaining blocking issues.

Docs and follow-ups
  • README.md documents clipboard/CDP recording and the targeted echo policy.
  • AIENG-228 still tracks downstream package updates and full onboarding validation after the upstream release.
  • Echo matching uses inputs from the current step. It does not track secrets across later steps or clean existing artifacts.
  • Short strings embedded in longer diagnostic text remain visible under the approved whole-value policy.
  • Screenshot-path metadata is scrubbed; image contents are outside this step-JSON change.

Comment thread src/knowledge-store/knowledge-store.ts Fixed
Comment thread src/knowledge-store/knowledge-store.ts Fixed
@cryptotavares
cryptotavares marked this pull request as ready for review October 15, 2026 13:22
@cryptotavares
cryptotavares requested a review from a team as a code owner October 15, 2026 13:22
@cryptotavares
cryptotavares enabled auto-merge (squash) October 15, 2026 13:43
@cryptotavares
cryptotavares merged commit 9f2eb51 into main Oct 16, 2026
27 checks passed
@cryptotavares
cryptotavares deleted the cryptotavares/aieng-228-redact-recorded-secrets branch October 16, 2026 09:14
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.

3 participants