Skip to content

fix(core): mask fine-grained GitHub PATs in both shared redactors - #6062

Merged
Astro-Han merged 1 commit into
apache:mainfrom
JasonBuildAI:fix/fine-grained-github-pat-redaction-z9k3
Oct 13, 2026
Merged

Astro-Han merged 1 commit into
apache:mainfrom
JasonBuildAI:fix/fine-grained-github-pat-redaction-z9k3

Conversation

@JasonBuildAI

@JasonBuildAI JasonBuildAI commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Users whose session output prints a fine-grained GitHub PAT (github_pat_...) now get that token masked everywhere Maka renders or stores text - explicitly including GitHub Copilot subscription users, because github_pat_ is one of the three token families Maka imports as a Copilot credential (isSupportedGitHubCopilotAccountToken accepts gho_, ghu_, and github_pat_; classic ghp_ is refused) - where before the quiet panel/TUI, the model/transcript path, and diagnostics exposed it in plain text, so sharing logs or a screenshot could leak a live repository credential.

The problem: redactSecrets (packages/core/src/redaction.ts) and the display redactor (packages/core/src/display-redaction.ts) enumerated the classic GitHub families ghp_, gho_, ghu_, ghs_, and ghr_, but not fine-grained PATs, so a bare printed PAT passed both unmasked. The display redactor also leaked keyed lines such as env output (GITHUB_TOKEN=github_pat_...) that the core redactor's sensitive-key assignment rule already masked. The PTY screen collector suppresses a line only when redaction changes it (packages/runtime/src/pty-screen-collector.ts:501), the diagnostic log stores the redacted text (packages/core/src/diagnostic-log.ts:58), and MCP secret scrubbing uses isPatternSecret as the only signal for free-form positions (packages/core/src/mcp-secrets.ts:93).

The change: one alternative added to each pattern list (github_pat_[0-9A-Za-z_]{20,}), plus regression tests in the three suites that cover these paths. The display pattern's reversible streaming contract is unchanged; the token body's _ is already inside the default continuation set.

Fixes #6061

Verification

Minimal repro with fake bare tokens (no key prefix), run against the built @maka/core (script imports packages/core/dist/redaction.js, display-redaction.js, and mcp-secrets.js).

Before (only the two pattern lines reverted):

family                      | core redactSecrets        | display redactSecrets     | pty line suppression
github_pat_ (fine-grained)  | LEAKS UNCHANGED           | LEAKS UNCHANGED           | does NOT suppress
ghp_ (classic PAT)          | [redacted]                | <redacted>                | suppresses
gho_ (OAuth)                | [redacted]                | <redacted>                | suppresses
ghu_ (user-to-server)       | [redacted]                | <redacted>                | suppresses
ghs_ (server-to-server)     | [redacted]                | <redacted>                | suppresses
ghr_ (refresh)              | [redacted]                | <redacted>                | suppresses
isPatternSecret(github_pat_): false
isPatternSecret(ghp_):        true

After:

family                      | core redactSecrets        | display redactSecrets     | pty line suppression
github_pat_ (fine-grained)  | [redacted]                | <redacted>                | suppresses
ghp_ (classic PAT)          | [redacted]                | <redacted>                | suppresses
gho_ (OAuth)                | [redacted]                | <redacted>                | suppresses
ghu_ (user-to-server)       | [redacted]                | <redacted>                | suppresses
ghs_ (server-to-server)     | [redacted]                | <redacted>                | suppresses
ghr_ (refresh)              | [redacted]                | <redacted>                | suppresses
isPatternSecret(github_pat_): true
isPatternSecret(ghp_):        true

Shape coverage around the same change (fake token, core vs. display redactor with only the two pattern lines reverted vs. this branch):

shape                        | core before | display before | after (both)
bare token                   | LEAKS RAW   | LEAKS RAW      | masked
env GITHUB_TOKEN=...         | masked      | LEAKS RAW      | masked
env COPILOT_GITHUB_TOKEN=... | masked      | LEAKS RAW      | masked
json {"token": "..."}        | masked      | LEAKS RAW      | masked
gh auth token (bare line)    | LEAKS RAW   | LEAKS RAW      | masked

Checks actually run locally (Node 24.21.0, Windows 11):

  • node --test on redaction.test.js, display-redaction.test.js, mcp-secrets.test.js: with the two pattern lines reverted and the new tests in place, the four new assertions fail (4 of 41); with the change, 41/41 pass. This is the "tests fail without the change" evidence.
  • npm --workspace @maka/core run test:dist: 934/934 pass.
  • npm --workspace @maka/ui run test:dist: 733/733 pass (the display redactor is re-exported through @maka/ui).
  • npm run build: exit 0.
  • npm run lint: exit 0.
  • npm run format:check: exit 0.
  • npm run typecheck: exit 0.
  • npx knip --workspace apps/desktop: exit 0.

Not run: the full monorepo npm test and the @maka/eval suite, which need a live Runtime Host/desktop environment this machine cannot host; the changed package's own suite is green.

AI use

  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex (OpenAI) drafted the pattern additions, the regression tests, and the reproduction under my direction; I reviewed the cited code paths, ran every check above, and own the change.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes - described under Summary above

@github-actions github-actions Bot added the effort/S Under 100 readable lines label Oct 11, 2026
@JasonBuildAI
JasonBuildAI force-pushed the fix/fine-grained-github-pat-redaction-z9k3 branch from c36f0b2 to 2d11d1b Compare October 11, 2026 12:29

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

I reviewed 2d11d1b8fe72e47b22c28bef9ff652952cec818e. I found no new P0–P2 issue in this change.

Both new alternatives match github_pat_ followed by at least 20 ASCII letters, digits or underscores, with word boundaries (packages/core/src/redaction.ts:74 and packages/core/src/display-redaction.ts:103). Their fine-grained PAT behavior agrees; the pre-existing differences between classic-token families are unchanged. GitHub's credential reference confirms the github_pat_ prefix. These patterns are defensive masking, not token-validity checks: a sufficiently long non-secret identifier with that prefix is also hidden. I did not establish an official fixed-length guarantee or test a live credential.

After a fresh dependency install and the normal build:test, the three affected suites passed all 41 tests, the complete core suite passed all 934 tests, and the complete UI suite passed all 733 tests. An additional synthetic probe passed 3,262 assertions covering the 19/20-character threshold, mixed-case bodies and underscores, surrounding delimiters, bare and keyed values, MCP free-form arguments, character-by-character rendering and bounded streaming recovery. Removing the two new alternatives makes the synthetic bare PAT pass through unchanged again.

There is no schema or wire-protocol change. The local merge with main 9a32d423b is clean and retains main's epoch 217. CI for this exact head is now running (run 38139463822); it has not finished, so I cannot report it as green. I did not run the full monorepo suite, a live Copilot account, external MCP integration or interactive PTY/screenshot checks.

Both `redactSecrets` and the display redactor covered every classic GitHub
token family (`ghp_`, `gho_`, `ghu_`, `ghs_`, `ghr_`) but not fine-grained
PATs (`github_pat_...`) - the format GitHub has issued by default since 2022,
and one of the token families Maka imports as a GitHub Copilot credential
(`isSupportedGitHubCopilotAccountToken` accepts `gho_`, `ghu_`, and
`github_pat_`; classic `ghp_` is refused). A bare printed fine-grained PAT
therefore passed both redactors unmasked: it reached the model and the
transcript (the PTY screen collector suppresses a line only when redaction
changes it), was shown in the quiet panel/TUI, and was stored in the
diagnostic log, so shared transcripts, screenshots, or diagnostics bundles
leaked a live repository credential. The display redactor also kept leaking
keyed lines such as `env` output (`GITHUB_TOKEN=github_pat_...`) that the
core redactor's sensitive-key assignment rule already masked.

Add the `github_pat_` alternative to both pattern lists. The display
pattern keeps its reversible streaming contract: the token body's `_` is
already inside the default continuation set.

Fixes apache#6061

Generated-by: Codex
@JasonBuildAI
JasonBuildAI force-pushed the fix/fine-grained-github-pat-redaction-z9k3 branch from 2d11d1b to c895e0f Compare October 11, 2026 14:45
@JasonBuildAI

Copy link
Copy Markdown
Contributor Author

CI note: the test run on the previous head failed in apps/desktop's "Windows opens the store page in installed Chrome, not the default browser" (src/main/__tests__/opencli-chrome.test.ts). The fixture polls for the stand-in Chrome's output file and reads it as soon as it exists, so it can observe the file in the window between its creation and the shell's printf — that run recorded an empty read where the store URL was expected. Nothing in this change touches that path.

I force-pushed the identical tree (same diff) to re-trigger test. The new run is waiting for approval before it starts.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

I compared c895e0fba4282ed5beb781d60d3ec5a17dd285bf with my previous review at 2d11d1b8fe72e47b22c28bef9ff652952cec818e. The single-commit patches are identical in git range-diff; both redactors and all three changed test files are unchanged. I found no new P0–P2 issue in this update.

The behavior remains the additional github_pat_ alternative in packages/core/src/redaction.ts:74 and packages/core/src/display-redaction.ts:103, with a 20-character minimum body and word boundaries. MCP free-form secret detection inherits the core redactor. This is a masking heuristic, not validation of a live token; sufficiently long non-secret identifiers with that prefix are also masked.

On this exact head, after a fresh dependency install and normal test build, I reran the 41 focused tests, all 934 core tests, all 733 UI tests, and 3,262 assertions covering synthetic lengths, delimiters, keyed/bare values, MCP scrubbing, character-by-character streaming and bounded recovery. All passed. Removing the two PAT alternatives in an isolated negative control restores the original bare-token leak.

The merge with main 0a8e29d2ed6a2703052ecd961ee4e3044c58af13 is conflict-free and makes no protocol change; the merged tree passes the epoch guard at 217. GitHub reports this head mergeable, but CI run 38148445125 is still running at this check, so I am not claiming all checks have passed. I did not exercise real credentials, a live Copilot account, external MCP services, PTY interaction, or the full monorepo test suite.

@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the contribution! Merging now.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at @Astro-Han's explicit request: no blocking findings in the automated review of this head, green CI, and no conflicts.

@Astro-Han
Astro-Han merged commit ff1db37 into apache:main Oct 13, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/S Under 100 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(core): fine-grained GitHub PATs (github_pat_) pass both shared redactors unmasked

3 participants