Skip to content

Deduplicate auth scheme parsing and random hex generation#3450

Merged
lpcox merged 4 commits intomainfrom
copilot/duplicate-code-analysis-report
Apr 9, 2026
Merged

Deduplicate auth scheme parsing and random hex generation#3450
lpcox merged 4 commits intomainfrom
copilot/duplicate-code-analysis-report

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 9, 2026

Two duplicate code patterns identified across internal/auth/header.go, internal/auth/apikey.go, and internal/middleware/jqschema.go: repeated Bearer/Agent prefix stripping logic in a security-critical path, and identical crypto/randhex.EncodeToString boilerplate.

Auth scheme parsing (internal/auth/header.go)

  • Extract stripAuthScheme(authHeader) (scheme, value, matched) helper backed by a supportedAuthSchemes slice
  • ParseAuthHeader and ExtractSessionID now both delegate to it; ExtractSessionID retains its TrimSpace on ******

Adding a new scheme (e.g. "Token ") is now a one-line change to the slice.

Random hex utility (internal/strutil/random_hex.go)

  • Add strutil.RandomHex(n int) (string, error) — returns 2*n hex chars from crypto/rand
  • auth.GenerateRandomAPIKey()strutil.RandomHex(32)
  • middleware.generateRandomID()strutil.RandomHex(16) with existing PID fallback preserved
// before (duplicated in 2 packages)
bytes := make([]byte, 32)
rand.Read(bytes)
return hex.EncodeToString(bytes)

// after
return strutil.RandomHex(32)

Tests added for both stripAuthScheme and RandomHex. All existing auth/middleware/strutil tests pass unchanged.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • example.com
    • Triggering command: /tmp/go-build1226932992/b514/launcher.test /tmp/go-build1226932992/b514/launcher.test -test.testlogfile=/tmp/go-build1226932992/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1226932992/b496/_pkg_.a -I /tmp/go-build167github.qkg1.top/github/gh-aw-mcpg x_amd64/vet . --gdwarf2 --64 x_amd64/vet o_.o�� 1.80.0/keepalive-errorsas -I x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build1226932992/b496/config.test /tmp/go-build1226932992/b496/config.test -test.testlogfile=/tmp/go-build1226932992/b496/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1226932992/b369/vet.cfg _.a 8150265/b159/vet.cfg x_amd64/vet /tmp/go-build167/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet ernal/envutil -fno-stack-prote-bool x_amd64/vet -uns�� eUOvqdOXy /tmp/go-build167-ifaceassert x_amd64/vet (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build1226932992/b514/launcher.test /tmp/go-build1226932992/b514/launcher.test -test.testlogfile=/tmp/go-build1226932992/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1226932992/b496/_pkg_.a -I /tmp/go-build167github.qkg1.top/github/gh-aw-mcpg x_amd64/vet . --gdwarf2 --64 x_amd64/vet o_.o�� 1.80.0/keepalive-errorsas -I x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build1226932992/b514/launcher.test /tmp/go-build1226932992/b514/launcher.test -test.testlogfile=/tmp/go-build1226932992/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1226932992/b496/_pkg_.a -I /tmp/go-build167github.qkg1.top/github/gh-aw-mcpg x_amd64/vet . --gdwarf2 --64 x_amd64/vet o_.o�� 1.80.0/keepalive-errorsas -I x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build1226932992/b523/mcp.test /tmp/go-build1226932992/b523/mcp.test -test.testlogfile=/tmp/go-build1226932992/b523/testlog.txt -test.paniconexit0 -test.timeout=10m0s -uns�� _.a /tmp/go-build1678150265/b188/vet.cfg x_amd64/vet --global eapis/rpc/status--version aw-mcpg/.git/cop--64 x_amd64/vet Tjxo�� fg 670252/b286/ x_amd64/vet 670252/b263/symaecho -w -buildmode=exe x_amd64/vet (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI linked an issue Apr 9, 2026 that may be closed by this pull request
Fixes #3108, fixes #3109, fixes #3110

- Extract stripAuthScheme helper in internal/auth/header.go to eliminate
  duplicate Bearer/Agent prefix parsing between ParseAuthHeader and
  ExtractSessionID
- Add strutil.RandomHex(n) shared utility for cryptographic random hex
  generation
- Refactor GenerateRandomAPIKey and generateRandomID to use RandomHex
- Add tests for stripAuthScheme and RandomHex

Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/8badc06f-16b9-4cd3-9b52-98ad76d23f1d

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix duplicate code in Go source files Deduplicate auth scheme parsing and random hex generation Apr 9, 2026
Copilot AI requested a review from lpcox April 9, 2026 17:16
Copilot finished work on behalf of lpcox April 9, 2026 17:16
@lpcox lpcox marked this pull request as ready for review April 9, 2026 17:17
Copilot AI review requested due to automatic review settings April 9, 2026 17:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 refactors duplicated security-sensitive parsing and random-ID generation logic into shared helpers to reduce repetition and make future changes safer and more consistent.

Changes:

  • Added strutil.RandomHex(n) and updated API key generation + jq payload IDs to use it.
  • Deduplicated Authorization scheme parsing via stripAuthScheme and reused it in ParseAuthHeader and ExtractSessionID.
  • Added unit tests for RandomHex and stripAuthScheme.
Show a summary per file
File Description
internal/strutil/random_hex.go Introduces shared crypto-random hex generator helper.
internal/strutil/random_hex_test.go Adds tests validating output length and basic uniqueness properties.
internal/middleware/jqschema.go Switches payload query ID generation to strutil.RandomHex(16).
internal/auth/header.go Centralizes Bearer/Agent scheme stripping and reuses it across parsing functions.
internal/auth/header_test.go Adds direct tests for stripAuthScheme.
internal/auth/apikey.go Switches API key generation to strutil.RandomHex(32).

Copilot's findings

Tip

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

  • Files reviewed: 6/6 changed files
  • Comments generated: 2

lpcox and others added 2 commits April 9, 2026 10:53
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@lpcox lpcox merged commit 7209a3b into main Apr 9, 2026
14 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-analysis-report branch April 9, 2026 17:58
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.

[duplicate-code] Duplicate Code Analysis Report

3 participants