fix(mcp): redact Octokit error tool-results and widen GitHub token regex by chrisleekr · Pull Request #238 · chrisleekr/github-app-playground

chrisleekr · 2026-06-20T11:44:10Z

Summary

Five GitHub-touching MCP servers (comment, inline-comment, github-state, resolve-review-thread, merge-readiness) and the shared state-fetchers.ts fetcher returned raw Octokit error messages directly to the agent's tool-result channel. Those messages echo the full credential URL (https://x-access-token:<ghs_token>@api.github.com/...) and Authorization: Bearer <token> headers with no redaction — OWASP MCP01 tool-result token leakage. merge-readiness had a hand-rolled inline regex that had already drifted from the canonical redactor; the other four did nothing.

A second, live gap was closed during review: the canonical redactGitHubTokens / redactSecrets regexes in src/utils/sanitize.ts only matched the legacy 36-char ghs_ body. GitHub's 2026-04-24 changelog announces a new ~520-char stateless ghs_APPID_JWT installation-token format, rolling out to all App installation tokens mid-May–late-June 2026, and explicitly names ghs_[A-Za-z0-9]{36} as a regex that will break. This app mints installation tokens itself, so its own live token was about to become unredactable. The ghp_/gho_/ghs_/ghr_ patterns are widened to GitHub's recommended open-ended [A-Za-z0-9._-]{36,} shape so both the legacy and new-format tokens are scrubbed.

Flow

flowchart TD
    OctErr["Octokit error<br/>echoes credential URL and Bearer header"]:::new --> Redact["redactErrorMessage<br/>redactCredentialUrls then redactSecrets<br/>byte-deletion only, no marker"]:::new
    Redact --> AgentResult["agent tool-result<br/>credential-free"]:::keep
    TokenRegex["GitHub token patterns widened<br/>open-ended body, 36 or more chars<br/>covers legacy and new stateless token"]:::new --> SanitizePipe["redactSecrets pipeline"]:::keep
classDef new fill:#1a5276,color:#ffffff
classDef keep fill:#ecf0f1,color:#2c3e50

Before: Octokit error → raw string (credential URL / Bearer header) → agent tool-result. redactGitHubTokens matched only exactly-36-char ghs_ tokens; new ~520-char installation tokens passed through unredacted.

Changes

src/utils/log-redaction.ts — new config-free helper redactErrorMessage(err): extracts the message string, runs redactCredentialUrls then redactSecrets, returns the cleaned body. Byte-deletion only, no [REDACTED_X] marker (CLAUDE.md security invariant 2 — markers leak probing signal on output paths).
src/mcp/servers/{comment,inline-comment,github-state,resolve-review-thread,merge-readiness}.ts — every catch now routes through redactErrorMessage instead of raw err.message. merge-readiness's divergent inline regex is deleted.
src/github/state-fetchers.ts — sixth catch site (dispatchGithubStateTool) wired to redactErrorMessage. Stays config-free (log-redaction imports only pino + sanitize).
src/utils/sanitize.ts — ghp_/gho_/ghs_/ghr_ token regexes widened from {36} to open-ended [A-Za-z0-9._-]{36,} (input-side redactGitHubTokens and output-side redactSecrets in parity) so the new ~520-char ghs_APPID_JWT format is scrubbed. Ordering invariant documented: the GitHub-token patterns must stay above the JWT pattern, since a stateless token embeds a JWT.
test/utils/log-redaction.test.ts — unit tests for redactErrorMessage: bare token, Bearer header, credential URL, the new ~520-char format, and benign passthrough (Error + non-Error).
test/mcp/error-redaction-wiring.test.ts — source-assertion guard binding each of the 6 catch sites to = redactErrorMessage(, so the wiring cannot silently regress without a heavyweight MCP subprocess harness.
test/utils/sanitize.test.ts — new-format (~520-char) ghs_ case added to the canonical token-redaction suite.

Related issues

Closes #224

Test plan

redactErrorMessage unit tests incl. the new ~520-char ghs_APPID_JWT shape (bare / Bearer / credential URL)
Source-assertion wiring guard for all 6 catch sites
New-format token case in the canonical sanitize suite
typecheck / lint / format clean
The local full bun test shows pre-existing DB-skip noise only; the isolated CI runner (scripts/test-isolated.sh) reports 0 genuine assertion failures and all target test files pass

Five GitHub-touching MCP servers and the shared state-fetchers fetcher returned raw Octokit error messages to the agent tool-result channel, leaking the installation token echoed in the credential URL and Bearer header (OWASP MCP01). Add config-free redactErrorMessage() and wire it into all six catch sites; delete the divergent merge-readiness inline regex. Also widen the canonical ghp_/gho_/ghs_/ghr_ token regexes in sanitize.ts from {36} to open-ended [A-Za-z0-9._-]{36,} so the new ~520-char ghs_APPID_JWT installation token (GitHub rollout 2026-04-27) is scrubbed. Closes #224 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015v2bspPF1ZTTG9ZZrbBgA1

coderabbitai · 2026-06-20T11:44:18Z

Warning

Review limit reached

@chrisleekr, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 22 minutes and 42 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7982839f-264b-4571-aca8-878f77aca874

📥 Commits

Reviewing files that changed from the base of the PR and between c61cee5 and e9ab013.

📒 Files selected for processing (11)

src/github/state-fetchers.ts
src/mcp/servers/comment.ts
src/mcp/servers/github-state.ts
src/mcp/servers/inline-comment.ts
src/mcp/servers/merge-readiness.ts
src/mcp/servers/resolve-review-thread.ts
src/utils/log-redaction.ts
src/utils/sanitize.ts
test/mcp/error-redaction-wiring.test.ts
test/utils/log-redaction.test.ts
test/utils/sanitize.test.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This PR addresses secret leakage in MCP tool-result outputs by ensuring Octokit error messages are redacted before being returned to the agent, and updates GitHub token redaction to cover the new long ghs_APPID_JWT installation-token format.

Changes:

Introduces redactErrorMessage(err) and wires it into 6 known leak catch-sites so tool results don’t echo credentialed URLs / tokens.
Widens ghp_/gho_/ghs_/ghr_ token regexes to match GitHub’s new variable-length installation token format.
Adds unit tests for the new redaction helper, token-regex coverage, and a wiring regression guard.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`src/utils/log-redaction.ts`	Adds `redactErrorMessage` for tool-result-safe error sanitization.
`src/utils/sanitize.ts`	Widens GitHub token patterns in both input-side and output-side secret redactors; documents ordering vs JWT.
`src/mcp/servers/comment.ts`	Routes caught error messages through `redactErrorMessage`.
`src/mcp/servers/inline-comment.ts`	Routes caught error messages through `redactErrorMessage`.
`src/mcp/servers/github-state.ts`	Routes `fail()` error messages through `redactErrorMessage`.
`src/mcp/servers/resolve-review-thread.ts`	Routes caught error messages through `redactErrorMessage`.
`src/mcp/servers/merge-readiness.ts`	Replaces divergent inline regex with `redactErrorMessage`.
`src/github/state-fetchers.ts`	Routes dispatch tool error messages through `redactErrorMessage`.
`test/utils/log-redaction.test.ts`	Adds unit tests for `redactErrorMessage` across token / header / URL / DB URL cases.
`test/mcp/error-redaction-wiring.test.ts`	Adds source-assertion wiring guard for all 6 catch sites.
`test/utils/sanitize.test.ts`	Adds a new-format stateless `ghs_` token case to the canonical suite.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+export function redactErrorMessage(err: unknown): string {
+  const raw = err instanceof Error ? err.message : String(err);
+  return redactSecrets(redactCredentialUrls(raw)).body;
+}


+const repoRoot = join(import.meta.dir, "..", "..");
+const ASSIGN = "= redactErrorMessage(";
+


+  it("merge-readiness.ts uses redactErrorMessage and drops the inline regex", () => {
+    const src = readSource("src/mcp/servers/merge-readiness.ts");
+    expect(src).toContain(ASSIGN);
+    expect(src).not.toContain("x-access-token:");
+  });


Copilot AI review requested due to automatic review settings June 20, 2026 11:44

github-actions Bot added the type: fix 🐞 label Jun 20, 2026

Copilot started reviewing on behalf of chrisleekr June 20, 2026 11:44 View session

Copilot AI reviewed Jun 20, 2026

View reviewed changes

chrisleekr merged commit 675d610 into main Jun 20, 2026
16 checks passed

chrisleekr deleted the fix/issue-224 branch June 20, 2026 11:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(mcp): redact Octokit error tool-results and widen GitHub token regex#238

fix(mcp): redact Octokit error tool-results and widen GitHub token regex#238
chrisleekr merged 1 commit into
mainfrom
fix/issue-224

chrisleekr commented Jun 20, 2026

Uh oh!

coderabbitai Bot commented Jun 20, 2026

Review limit reached

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		const repoRoot = join(import.meta.dir, "..", "..");
		const ASSIGN = "= redactErrorMessage(";

Conversation

chrisleekr commented Jun 20, 2026

Summary

Flow

Changes

Related issues

Test plan

Uh oh!

coderabbitai Bot commented Jun 20, 2026

Review limit reached

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants