Skip to content

fix(cloud-agent-next): redact secrets by key name in setup output - #7237

Merged
eshurakov merged 1 commit into
mainfrom
eshurakov/redact-named-secrets
Oct 7, 2026
Merged

eshurakov merged 1 commit into
mainfrom
eshurakov/redact-named-secrets

Conversation

@eshurakov

Copy link
Copy Markdown
Contributor

Summary

Setup commands and the automatic git diagnostics could print a decrypted profile secret in clear when its name did not match the SECRET_NAME heuristic, for example DATABASE_URL. The wrapper now also redacts the value of every secret it is told about by key name.

  • Legacy plane: getSaferEnvVars returns the names of the secrets it decrypted (excluding a key a later credential step overwrote). The bootstrap carries them as materialized.secretEnvKeys, and the wrapper redacts them in setup output and auto-commit diagnostics.
  • Control plane: session.prepare may carry frame-only secretEnvKeys, validated in the shared protocol and rejected in a prepare input so it can never reach a route row. The wrapper advertises optional redactsNamedSecrets in hello. A frame without the field stays valid for a version-3 wrapper. Nothing sends the field yet; the follow-up PR delivers profile secrets to control-plane sessions and uses this.

Agent output and terminals are not redacted.

Verification

  • Not exercised manually in this PR. Behavior is covered by wrapper unit tests (redact-output, prepare, turn, session-bootstrap, auto-commit, connection) and session-service tests for the legacy plane; pnpm --filter cloud-agent-next typecheck, lint, test (5929 passed) and test:wrapper (968 passed) are green.

Visual Changes

N/A

Reviewer Notes

First of two stacked PRs; the second (eshurakov/deliver-profile-secrets) depends on the protocol fields added here.

@kilo-code-bot

kilo-code-bot Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Full re-review against HEAD b29a062e7: the branch was rewritten and the previous incremental base (30d3634) is no longer present in the clone, so all 23 changed files were reviewed in full. Both previously reported findings are resolved, and no new issues were found in the named-secret redaction wiring.

  • wrapper/src/redact-output.ts now guards the sources[index][key] lookup with typeof value === 'string', so inherited names like constructor are skipped instead of crashing remember.
  • src/shared/control-plane-protocol.ts adds controlPlaneRegistrationRouteSpecSchema, which rejects frame-only secretEnvKeys at the session-registration boundary; session-do.ts uses it, so a frame-only value can no longer be persisted and fail every later prepare.
  • Verified: all createSecretRedactor call sites use the new array/keys signature; the legacy secretEnvKeys filter correctly excludes a decrypted secret later overwritten (e.g. KILOCODE_TOKEN); the redactor's named-key lookup falls back across layered sources without matching non-string properties; WrapperState._secretEnvKeys is cleared on reset; no new memory retention was introduced.
Files Reviewed (23 files)
  • services/cloud-agent-next/docs/control-plane.md
  • services/cloud-agent-next/src/control-plane/session/registration.test.ts
  • services/cloud-agent-next/src/control-plane/session/session-do.ts
  • services/cloud-agent-next/src/session-service.test.ts
  • services/cloud-agent-next/src/session-service.ts
  • services/cloud-agent-next/src/shared/control-plane-protocol.ts
  • services/cloud-agent-next/src/shared/wrapper-bootstrap.ts
  • services/cloud-agent-next/test/unit/wrapper/auto-commit.test.ts
  • services/cloud-agent-next/wrapper/src/auto-commit.ts
  • services/cloud-agent-next/wrapper/src/control-plane/connection.test.ts
  • services/cloud-agent-next/wrapper/src/control-plane/connection.ts
  • services/cloud-agent-next/wrapper/src/control-plane/prepare.test.ts
  • services/cloud-agent-next/wrapper/src/control-plane/prepare.ts
  • services/cloud-agent-next/wrapper/src/control-plane/turn.test.ts
  • services/cloud-agent-next/wrapper/src/control-plane/turn.ts
  • services/cloud-agent-next/wrapper/src/control/apply-attach.ts
  • services/cloud-agent-next/wrapper/src/lifecycle.ts
  • services/cloud-agent-next/wrapper/src/main.ts
  • services/cloud-agent-next/wrapper/src/redact-output.test.ts
  • services/cloud-agent-next/wrapper/src/redact-output.ts
  • services/cloud-agent-next/wrapper/src/session-bootstrap.test.ts
  • services/cloud-agent-next/wrapper/src/session-bootstrap.ts
  • services/cloud-agent-next/wrapper/src/state.ts
Previous Review Summaries (4 snapshots, latest commit 30d3634)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 30d3634)

Status: No Issues Found | Recommendation: Merge

This incremental commit resolves both prior findings: sources[index][key] is now type-guarded against inherited non-string properties before remember, and a new controlPlaneRegistrationRouteSpecSchema rejects frame-only secretEnvKeys at the registration boundary. No new issues found in the changed lines.

Files Reviewed (5 files)
  • services/cloud-agent-next/src/control-plane/session/registration.test.ts
  • services/cloud-agent-next/src/control-plane/session/session-do.ts
  • services/cloud-agent-next/src/shared/control-plane-protocol.ts
  • services/cloud-agent-next/wrapper/src/redact-output.test.ts
  • services/cloud-agent-next/wrapper/src/redact-output.ts

Previous review (commit 03baf4d)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The refactor that threads named secret keys into the output redactor is sound and all call sites were updated, but the new sources[index][key] lookup can hit an inherited non-string property and abort wrapper bootstrap, and the new secretEnvKeys field is enforced frame-only only at a later boundary.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
services/cloud-agent-next/wrapper/src/redact-output.ts 117 sources[index][key] can return an inherited non-string property (e.g. a profile secret named constructor), crashing remember during redactor construction

SUGGESTION

File Line Issue
services/cloud-agent-next/src/shared/control-plane-protocol.ts 265 secretEnvKeys is persisted at the session-registration boundary rather than enforced frame-only there
Files Reviewed (21 files)
  • services/cloud-agent-next/docs/control-plane.md
  • services/cloud-agent-next/src/session-service.test.ts
  • services/cloud-agent-next/src/session-service.ts
  • services/cloud-agent-next/src/shared/control-plane-protocol.ts
  • services/cloud-agent-next/src/shared/wrapper-bootstrap.ts
  • services/cloud-agent-next/test/unit/wrapper/auto-commit.test.ts
  • services/cloud-agent-next/wrapper/src/auto-commit.ts
  • services/cloud-agent-next/wrapper/src/control-plane/connection.test.ts
  • services/cloud-agent-next/wrapper/src/control-plane/connection.ts
  • services/cloud-agent-next/wrapper/src/control-plane/prepare.test.ts
  • services/cloud-agent-next/wrapper/src/control-plane/prepare.ts
  • services/cloud-agent-next/wrapper/src/control-plane/turn.test.ts
  • services/cloud-agent-next/wrapper/src/control-plane/turn.ts
  • services/cloud-agent-next/wrapper/src/control/apply-attach.ts
  • services/cloud-agent-next/wrapper/src/lifecycle.ts
  • services/cloud-agent-next/wrapper/src/main.ts
  • services/cloud-agent-next/wrapper/src/redact-output.test.ts
  • services/cloud-agent-next/wrapper/src/redact-output.ts
  • services/cloud-agent-next/wrapper/src/session-bootstrap.test.ts
  • services/cloud-agent-next/wrapper/src/session-bootstrap.ts
  • services/cloud-agent-next/wrapper/src/state.ts

Note: the incremental base commit b8fe8ea569a4a4dffd9791a68dd5567fc102e38f was not present in the local clone, so this review was published as a full re-review against HEAD 03baf4d58.

Fix these issues in Kilo Cloud

Previous review (commit b8fe8ea)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

This incremental commit only sanitizes test fixtures (db.internal → localhost) to avoid secret-scanner flags; it adds no new issues, and the two prior findings remain unresolved.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
services/cloud-agent-next/wrapper/src/redact-output.ts 117 sources[index][key] can return an inherited non-string property (e.g. a profile secret named constructor), crashing remember during redactor construction

SUGGESTION

File Line Issue
services/cloud-agent-next/src/shared/control-plane-protocol.ts 265 secretEnvKeys is persisted at the session-registration boundary rather than enforced frame-only there
Files Reviewed (4 files + 2 re-checked)
  • services/cloud-agent-next/test/unit/wrapper/auto-commit.test.ts
  • services/cloud-agent-next/wrapper/src/control-plane/prepare.test.ts
  • services/cloud-agent-next/wrapper/src/redact-output.test.ts
  • services/cloud-agent-next/wrapper/src/session-bootstrap.test.ts
  • re-checked: services/cloud-agent-next/wrapper/src/redact-output.ts
  • re-checked: services/cloud-agent-next/src/shared/control-plane-protocol.ts

Fix these issues in Kilo Cloud

Previous review (commit 2d0f3b9)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The named-secret redaction wiring is sound, but the new lookup in createSecretRedactor can crash on an Object.prototype-named secret (constructor), aborting wrapper bootstrap.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
services/cloud-agent-next/wrapper/src/redact-output.ts 117 sources[index][key] can return an inherited non-string property, crashing remember (reachable via a profile secret named e.g. constructor)

SUGGESTION

File Line Issue
services/cloud-agent-next/src/shared/control-plane-protocol.ts 265 secretEnvKeys is persisted at the session-registration boundary, not enforced frame-only there
Files Reviewed (21 files)
  • services/cloud-agent-next/docs/control-plane.md
  • services/cloud-agent-next/src/session-service.test.ts
  • services/cloud-agent-next/src/session-service.ts
  • services/cloud-agent-next/src/shared/control-plane-protocol.ts
  • services/cloud-agent-next/src/shared/wrapper-bootstrap.ts
  • services/cloud-agent-next/test/unit/wrapper/auto-commit.test.ts
  • services/cloud-agent-next/wrapper/src/auto-commit.ts
  • services/cloud-agent-next/wrapper/src/control-plane/connection.test.ts
  • services/cloud-agent-next/wrapper/src/control-plane/connection.ts
  • services/cloud-agent-next/wrapper/src/control-plane/prepare.test.ts
  • services/cloud-agent-next/wrapper/src/control-plane/prepare.ts
  • services/cloud-agent-next/wrapper/src/control-plane/turn.test.ts
  • services/cloud-agent-next/wrapper/src/control-plane/turn.ts
  • services/cloud-agent-next/wrapper/src/control/apply-attach.ts
  • services/cloud-agent-next/wrapper/src/lifecycle.ts
  • services/cloud-agent-next/wrapper/src/main.ts
  • services/cloud-agent-next/wrapper/src/redact-output.test.ts
  • services/cloud-agent-next/wrapper/src/redact-output.ts
  • services/cloud-agent-next/wrapper/src/session-bootstrap.test.ts
  • services/cloud-agent-next/wrapper/src/session-bootstrap.ts
  • services/cloud-agent-next/wrapper/src/state.ts

Verified clean

  • All createSecretRedactor callers were migrated to the new signature; the array form is behaviorally identical to the old variadic form (apply-attach.ts included).
  • The legacy getSaferEnvVars filter (session-service.ts:1511) cannot advertise a key whose decrypted value was overwritten, and decryption is fail-closed, so no undefined === undefined false advertisement.
  • WrapperState._secretEnvKeys is cleared on session change and set on every ready request; no stale cross-session keys and no ordering race with auto-commit.
  • The control-plane secretEnvKeys producer is intentionally absent in this PR (documented as a follow-up), and the prepare-input rejection is correct; redactsNamedSecrets version negotiation with older peers is handled.
  • No memory leak introduced; the new sets are per-call and not retained.

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 109.6K · Output: 26.2K · Cached: 1.7M

Review guidance: REVIEW.md from base branch main

@eshurakov
eshurakov force-pushed the eshurakov/redact-named-secrets branch from b8fe8ea to 03baf4d Compare October 7, 2026 08:02
@eshurakov
eshurakov added this pull request to stack #7257 October 7, 2026 15:01
- Redact setup output secrets identified by key name.
- Harden named-secret redaction and its boundaries.
@eshurakov
eshurakov force-pushed the eshurakov/redact-named-secrets branch from 30d3634 to b29a062 Compare October 7, 2026 19:27
@eshurakov
eshurakov merged commit f7fc248 into main Oct 7, 2026
28 checks passed
@eshurakov
eshurakov deleted the eshurakov/redact-named-secrets branch October 7, 2026 20:01
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