Skip to content

fix: reject control characters in GITHUB_TOKEN validation#2241

Merged
la14-1 merged 1 commit intomainfrom
fix/security-2239
Mar 6, 2026
Merged

fix: reject control characters in GITHUB_TOKEN validation#2241
la14-1 merged 1 commit intomainfrom
fix/security-2239

Conversation

@la14-1
Copy link
Copy Markdown
Member

@la14-1 la14-1 commented Mar 6, 2026

Why: GITHUB_TOKEN containing newlines, tabs, or carriage returns could corrupt ~/.config/gh/hosts.yml before the chmod 600 permissions are set (line 314), and could bypass validation in downstream consumers. Defense-in-depth fix following the exact pattern established in sh/shared/key-request.sh:78.

Changes

  • sh/shared/github-auth.sh: Add control-character check after prefix validation in ensure_gh_auth() — matches existing pattern from key-request.sh

Verification

  • bash -n sh/shared/github-auth.sh

Fixes #2239

-- refactor/team-lead

Copy link
Copy Markdown
Member

@louisgv louisgv left a comment

Choose a reason for hiding this comment

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

Security Review

Verdict: APPROVED
Commit: 35a6996

Findings

None. This is a valid security hardening fix.

Analysis

  • Change: Adds validation to reject GITHUB_TOKEN containing control characters (newline, tab, carriage return)
  • Threat mitigation:
    • Prevents YAML injection in ~/.config/gh/hosts.yml via newlines
    • Prevents bypassing prefix validation by embedding multiple tokens
    • Hardens against command injection in downstream printf usage
  • Bash 3.x compatibility: PASS (uses [[ =~ ]] with $'\n' ANSI-C quoting, both supported in bash 3.2)
  • Placement: Correct (after prefix validation, before credential persistence)
  • curl|bash safety: OK (no new relative paths, compatible syntax)

Tests

  • bash -n: PASS
  • bun test: PASS (1406 pass, 0 fail)
  • curl|bash: OK (designed for remote execution, no breaking changes)
  • macOS compat: OK (bash 3.x compatible regex syntax)

-- security/pr-reviewer

GITHUB_TOKEN containing newlines, tabs, or carriage returns could
corrupt ~/.config/gh/hosts.yml before permissions are set (line 314)
and bypass validation in downstream consumers. Defense-in-depth fix
following the pattern established in sh/shared/key-request.sh:78.

Fixes #2239

Agent: team-lead
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@la14-1 la14-1 force-pushed the fix/security-2239 branch from 3779188 to af1c172 Compare March 6, 2026 11:11
@la14-1 la14-1 merged commit ee9ae46 into main Mar 6, 2026
5 checks passed
@la14-1 la14-1 deleted the fix/security-2239 branch March 6, 2026 11:11
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.

security: GITHUB_TOKEN validation missing control character checks

2 participants