Skip to content

fix(security): redact all GitHub token families - #84

Open
BunsDev wants to merge 6 commits into
mainfrom
fix/redact-all-github-token-families
Open

fix(security): redact all GitHub token families#84
BunsDev wants to merge 6 commits into
mainfrom
fix/redact-all-github-token-families

Conversation

@BunsDev

@BunsDev BunsDev commented Aug 22, 2026

Copy link
Copy Markdown
Member

Summary

Complete the credential-redaction boundary introduced by #81 by covering the classic GitHub token prefixes that remained unhandled:

  • ghp_ — personal access tokens
  • gho_ — OAuth access tokens
  • ghr_ — refresh tokens

The existing coverage for ghs_, ghu_, github_pat_, and x-access-token: is unchanged.

Why

Hosted task output, failure details, and publication comments all pass through redact_tokenish / redact_secrets. Before this change, classic GitHub token families could survive those paths even though newer installation, user, and fine-grained token forms were redacted.

Scope

  • one-line expansion of the centralized marker allowlist
  • focused unit coverage for every supported GitHub token family
  • publication-text coverage for the three previously missing classic prefixes

No trigger, authorization, task-execution, or publication behavior changes.

Validation

CI runs Python compilation and unittest discovery over deploy/coven-github/test_*.py, so the new regression suite is wired into the required check.

Historical follow-up to #81.

Copilot AI lite review requested due to automatic review settings August 22, 2026 23:58

Copilot AI 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.

Pull request overview

This PR completes the GitHub credential redaction boundary in the adapter by adding support for the classic GitHub token prefixes (ghp_, gho_, ghr_) and introducing focused unit tests to prevent regressions in token scrubbing paths used for published output.

Changes:

  • Expand redact_tokenish()’s marker allowlist to include ghp_, gho_, and ghr_.
  • Add a dedicated unit test suite validating redaction across all supported GitHub token families.
  • Add publication-text coverage intended to ensure classic token families are redacted via redact_secrets().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
deploy/coven-github/coven_github_adapter.py Extends the token marker list used by redact_tokenish() to cover classic GitHub token prefixes.
deploy/coven-github/test_github_token_redaction.py Adds new unit tests for GitHub token-family redaction and publication-text redaction behavior.
Suppressed comments (1)

deploy/coven-github/coven_github_adapter.py:1659

  • redact_tokenish() currently treats only whitespace and quotes as token terminators. That causes adjacent tokens or tokens followed by punctuation (e.g. ghp_…|gho_…) to be treated as one long token and over-redacted, which also makes the new publication-text test case incorrect/flaky depending on delimiters. Consider terminating the scan on the first non-token character (e.g. anything other than [A-Za-z0-9_]) so punctuation like |, &, ), etc. doesn’t get consumed.
    markers = ["ghp_", "gho_", "ghr_", "ghs_", "ghu_", "github_pat_", "x-access-token:"]
    redacted = text
    for marker in markers:
        while marker in redacted:
            index = redacted.find(marker)

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread deploy/coven-github/test_github_token_redaction.py
BunsDev and others added 2 commits August 30, 2026 09:16
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI and others added 2 commits August 30, 2026 14:22
Co-authored-by: BunsDev <68980965+BunsDev@users.noreply.github.com>
…ite loop

Co-authored-by: BunsDev <68980965+BunsDev@users.noreply.github.com>
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.

3 participants