fix: redact env var values in agent debug manifest endpoint - #26904
Conversation
The unauthenticated /debug/manifest endpoint on the agent debug server (127.0.0.1:2113 by default) JSON-encoded the full manifest, including EnvironmentVariables values. Because the endpoint is reachable by any process in the workspace regardless of Unix user, template-provided values that may carry credentials were readable across users. Redact the environment variable values in the debug manifest response while preserving the keys, so operators can still confirm which variables are configured. This matches the existing support-bundle behavior, which already redacts these values via sanitizeEnv.
|
/coder-agents-review |
|
Chat: Review posted | View chat Review historydeep-review v0.9.0 | Round 2 | Last posted: Round 2, 6 findings (2 P3, 3 Nit, 1 Note), APPROVE. Review Finding inventoryFindings
Contested and acknowledged(none) Round logRound 1Panel: Bisky, Hisoka, Mafu-san, Mafuuu, Pariston, Gon, Leorio, Kurapika, Ging-Go, Komugi, Chopper, Ryosuke, Meruem (wildcard). Netero pre-panel. 2 P3, 3 Nit, 1 Note new. 1 Note dropped. Reviewed against 3d966d4..7bc3785. Round 2Panel: Bisky, Hisoka, Mafu-san, Mafuuu, Pariston, Gon, Leorio, Kurapika, Ging-Go, Komugi, Chopper, Ryosuke, Kite (wildcard). Netero pre-panel. CRF-1 through CRF-6 all verified fixed by multiple reviewers. 2 Gon Nits (marginal comment trim) dropped by orchestrator (12/13 reviewers found no issues; one-clause restatements within editorial judgment). No new findings. Reviewed against 3d966d4..8f4d8b3. About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
Documentation CheckUpdates Needed
Automated review via Coder Agents |
![coder-agents-review[bot]](https://avatars.githubusercontent.com/in/3454270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean security fix. The shallow copy + map replacement correctly avoids mutating the atomic-loaded manifest that other goroutines read. The test covers both directions: secret absent from body, key preserved with redacted value. Three reviewers independently verified the concurrency safety of the approach. Pariston built four adversarial framings and concluded the problem is correctly understood and the fix is at the right causal level.
2 P3, 3 Nit, 1 Note.
"Pulled the threads. The code does what it says. ♦" (Hisoka)
🤖 This review was automatically generated with Coder Agents.
Address review feedback on the debug manifest redaction: - Use the same "***REDACTED***" marker as sanitizeEnv in support/support.go so a support bundle and the debug endpoint agree. - Preserve empty env values as empty, matching sanitizeEnv, so an explicitly empty variable is not mistaken for one carrying a value. - Trim comments and use a distinct sentinel value in the test so a redaction failure cannot be confused with ManifestSecretsStripped.
sreya
changed the title
|
/coder-agents-review |
There was a problem hiding this comment.
All six R1 findings addressed in a single clean commit. Sentinel aligned with sanitizeEnv, comments trimmed, test sentinels made distinct, empty-value behavior matched.
Multiple reviewers independently verified the concurrency safety (shallow copy + new map, atomic load, no mutation of live manifest), confirmed redaction covers both the unauthenticated debug server and the tailnet API handler, and traced the edge case (empty env vars preserved, matching sanitizeEnv).
No new findings.
"I tried to build a case against this change and could not. The problem is correctly understood, the solution is proportional, and the fix is at the right causal level." (Pariston)
🤖 This review was automatically generated with Coder Agents.
2 participants
The unauthenticated /debug/manifest endpoint on the agent debug server (127.0.0.1:2113 by default) JSON-encoded the full manifest, including EnvironmentVariables values. Because the endpoint is reachable by any process in the workspace regardless of Unix user, template-provided values that may carry credentials were readable across users.
Redact the environment variable values in the debug manifest response while preserving the keys, so operators can still confirm which variables are configured. This matches the existing support-bundle behavior, which already redacts these values via sanitizeEnv.