Skip to content

ENG-8589: Don't log the MCP client secret under --debug - #463

Closed
p0-agentic-workflow-alpha[bot] wants to merge 4 commits into
mainfrom
first-pass/eng-8589-don-t-log-the-mcp-client-secret-7407cff3
Closed

p0-agentic-workflow-alpha[bot] wants to merge 4 commits into
mainfrom
first-pass/eng-8589-don-t-log-the-mcp-client-secret-7407cff3

Conversation

@p0-agentic-workflow-alpha

Copy link
Copy Markdown

Opened by first-pass-pipeline from thread 7407cff3-942d-400f-8d0c-4ebade124008.

Issue

ENG-8589: Don't log the MCP client secret under --debug

Problem

p0 claude mcp add prints the OAuth client secret when the user passes --debug:

debug(argv, "Client secret", client.secret);

in src/commands/claude/mcp.ts.

The secret is otherwise handled carefully — it goes to the child process through the MCP_CLIENT_SECRET environment variable specifically so it never lands on disk. The debug line undoes that. Anything that captures the terminal keeps the credential: scrollback, a CI job's log, or the output a user pastes into a support ticket when asking why the command failed.

The value isn't needed to debug the command. Knowing whether a secret was set is enough, so the line can say that instead of printing it.

Found by CodeRabbit while reviewing Report failures from "claude mcp add". It's outside that PR's diff — the line has been there since the claude command suite was added in p0-security/p0-docs#406 — so it's tracked here rather than fixed in passing.

repo: p0cli

Definition of done

  • Debug output for p0 claude mcp add does not print the OAuth client secret.
  • Debug output can still indicate whether a client secret is set.
  • The secret continues to be passed to the child process only through the MCP_CLIENT_SECRET environment variable.

Linear: https://linear.app/p0-security/issue/ENG-8589/dont-log-the-mcp-client-secret-under-debug

Plan

Plan: Redact the MCP client secret from debug output

Steps

  1. Replace the sensitive debug argument in src/commands/claude/mcp.ts.
    Change the provisionServer debug call from passing client.secret to reporting the literal status "set": debug(argv, "Client secret", "set"). Do not alter the OAuth arguments or the environment construction; MCP_CLIENT_SECRET must continue receiving the exact secret.

  2. Extend coverage in src/commands/claude/__tests__/mcp.test.ts.
    Add a debug-output test using a sentinel secret and captured console.error output. Verify that debug output reports Client secret set (or the equivalent argument formatting), does not contain the sentinel value, and that the existing child-process environment test still proves the exact secret reaches MCP_CLIENT_SECRET. If needed for direct coverage, expose the provisioning function or use the project’s existing test seams without changing runtime behavior.

  3. Run the relevant validation.
    Execute the MCP unit tests, then the repository type-check/lint or full unit-test commands as appropriate. Confirm formatting remains compliant.

Acceptance criteria

  • [Step 1] p0 claude mcp add --debug never writes the OAuth client secret to debug output.
  • [Step 1, Step 2] Debug output still indicates that the client secret is “set”.
  • [Step 1, Step 2] The child process continues to receive the unchanged secret exclusively through MCP_CLIENT_SECRET; no secret value is added to Claude’s argument list or debug arguments.
  • [Step 2, Step 3] Tests fail if the sentinel secret appears in captured debug output and pass when the status-only message is emitted.
  • [Step 3] Existing MCP behavior and repository checks remain passing.

Files

  • src/commands/claude/mcp.ts
  • src/commands/claude/__tests__/mcp.test.ts

Tests

  • Add coverage for debug mode with a sentinel OAuth secret:
    • assert the output reports Client secret as set;
    • assert the sentinel secret is absent from captured stderr/debug output.
  • Preserve and run the existing test asserting MCP_CLIENT_SECRET reaches the spawned child unchanged.
  • Run yarn test:unit (or the focused MCP test), plus type/lint checks if required by the repository workflow.

Out of scope

  • Do not change MCP client cache behavior, including the existing cached clientData handling.
  • Do not modify src/drivers/stdio.ts or the Claude command’s --debug option.
  • Do not alter OAuth provisioning, Claude arguments, child-process inheritance, or secret storage beyond redacting this debug log.

Review

  • Plan review: 2 findings
  • Implementation review: no findings after 3 remediations
  • Local CI: passed (Install dependencies, Verify format, Lint, Build, Run unit tests)
  • Still requires GitHub: semgrep.yml / semgrep
  • Risk: low
  • Outcome: escalated. Escalated: Publish failed: git push failed: fatal: could not read Username for 'https://github.com': No such device or address. Published by a human.

Implementation notes

Redacted the MCP OAuth client secret from debug output by logging only "set". Added coverage confirming the secret is absent from debug output and Claude arguments while remaining unchanged in MCP_CLIENT_SECRET. No findings were disputed. Tests and type/format checks could not run because dependencies are unavailable; yarn install failed with ENOSPC, and vitest/tsc/prettier were not found.

first-pass-pipeline added 4 commits September 18, 2026 18:39
Thread 7407cff3-942d-400f-8d0c-4ebade124008

Redacted the MCP OAuth client secret from debug output by reporting only "set" while preserving MCP_CLIENT_SECRET environment handling. Added coverage for the status-only debug output and sentinel absence. Focused tests and formatting checks were attempted but could not run because dependencies (vitest and prettier) are not installed.
Thread 7407cff3-942d-400f-8d0c-4ebade124008

Redacted the MCP OAuth client secret from debug output, reporting only "set" while preserving the exact MCP_CLIENT_SECRET environment value. Added direct provisioning coverage verifying the secret is absent from debug output and Claude arguments. Tests could not run because dependency installation failed with ENOSPC and vitest was unavailable; cleared the yarn cache to recover workspace space, but installation still exceeded available space. No findings were disputed.
Thread 7407cff3-942d-400f-8d0c-4ebade124008

Corrected MCP coverage to exercise provisionServer directly, verify debug output reports only "set", ensure the sentinel secret is absent from logs and Claude arguments, and confirm it remains in MCP_CLIENT_SECRET. Existing source redaction is preserved. Focused tests, TypeScript, and Prettier checks were attempted but could not run because dependencies were unavailable after yarn install failed with ENOSPC; git diff validation passed.
Thread 7407cff3-942d-400f-8d0c-4ebade124008

Added MCP coverage using a sentinel OAuth secret to verify debug output reports only "Client secret set", excludes the secret from logs and Claude arguments, and preserves delivery through MCP_CLIENT_SECRET. The source redaction was already present and remains unchanged. Focused tests and type/format checks could not execute because dependencies are unavailable (vitest, tsc, and prettier not found); dependency installation previously failed with corrupt cache entries and ENOSPC. git diff --check passed.
@varunalla varunalla closed this Sep 18, 2026
@varunalla
varunalla deleted the first-pass/eng-8589-don-t-log-the-mcp-client-secret-7407cff3 branch September 18, 2026 19:41
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