Skip to content

Deduplicate delegation token generation with util.RandomHex - #12654

Merged
lpcox merged 2 commits into
mainfrom
copilot/duplicate-code-fix-opaque-token
Sep 7, 2026
Merged

Deduplicate delegation token generation with util.RandomHex#12654
lpcox merged 2 commits into
mainfrom
copilot/duplicate-code-fix-opaque-token

Conversation

Copilot AI commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

internal/delegation/store.go reimplemented crypto-random hex-token generation via manual crypto/rand.Read + hex.EncodeToString, duplicating logic already provided by internal/util.RandomHex.

Changes

  • Replaced generateOpaqueToken's manual random-byte generation and hex encoding with a call to util.RandomHex
  • Removed now-unused crypto/rand and encoding/hex imports from store.go
func generateOpaqueToken(prefix string, n int) (string, error) {
	hexStr, err := util.RandomHex(n)
	if err != nil {
		return "", fmt.Errorf("failed to generate delegation token: %w", err)
	}
	return prefix + hexStr, nil
}

Token format and length are unchanged; this only consolidates the entropy source and error-wrapping behind a single shared helper.

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix duplicate opaque token generation in store.go Deduplicate delegation token generation with util.RandomHex Sep 7, 2026
Copilot AI requested a review from lpcox September 7, 2026 14:31
@lpcox
lpcox marked this pull request as ready for review September 7, 2026 14:35
Copilot AI balanced review requested due to automatic review settings September 7, 2026 14:35

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.

🟢 Approval recommended

The focused refactor preserves existing behavior and introduces no unresolved issues.

Pull request overview

Deduplicates delegation token generation by reusing the shared cryptographic utility while preserving token format and length.

Changes:

  • Replaced manual random-byte generation and hex encoding with util.RandomHex.
  • Removed unused standard-library imports.
File summaries
File Description
internal/delegation/store.go Uses the shared random hexadecimal token helper.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

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

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

🔒 mcpg Read-Only Stress — gvisor

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: gvisor

Part Surface Op Result Expected Status
A MCP reads data ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) tool absent from catalog (23 read-only tools only) BLOCKED ⚠️
C CLI reads data ALLOWED
D CLI REST writes (reaction/star/issue/comment) gh unauthenticated BLOCKED ⚠️
E CLI GraphQL mutations (addReaction/addStar/createIssue) gh unauthenticated BLOCKED ⚠️

Overall: INCONCLUSIVE
(⚠️ Part B: none of the targeted write tools (add_issue_comment, star_repository, issue_write, create_branch, create_or_update_file, create_pull_request) appear in the 23-tool MCP catalog, so their absence only confirms backend GITHUB_READ_ONLY=1 config, not mcpg's own gateway-level DIFC/guard enforcement. ⚠️ Parts D/E: gh auth status reports not logged in, so the token-scope boundary could not be exercised. No write leaked in any part.)

🔒 mcpg read-only stress (gVisor runtime) by Read-Only Stress: gVisor runtime

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

🔒 mcpg Read-Only Stress — default

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: default (normal container isolation)

Part Surface Op Result Expected Status
A MCP reads (list_issues, list_pull_requests, get_file_contents, list_commits) data returned ALLOWED
B MCP writes (add_issue_comment/star_repository/issue_write/create_branch/create_or_update_file/create_pull_request) all 6 targets absent from the 23-tool catalog (backend launched with GITHUB_READ_ONLY=1 per gh-aw's tools.github: design) BLOCKED ⚠️
C CLI reads (same calls via github CLI proxy on PATH) data returned ALLOWED
D CLI REST writes (reaction/star/issue/comment) gh auth status = not logged in; not attempted BLOCKED ⚠️
E CLI GraphQL mutations (addReaction/addStar/createIssue) gh unauthenticated; not attempted BLOCKED ⚠️

Overall: INCONCLUSIVE

No write leaked through any surface. Part B is INCONCLUSIVE because all targeted write tools were absent from the exposed catalog entirely (gh-aw's tools.github: block always sets GITHUB_READ_ONLY=1 backend-side, so write tools never register — this is gh-aw's own defense-in-depth, not independent evidence of mcpg's gateway-level DIFC/guard blocking). Parts D and E are INCONCLUSIVE because gh was not authenticated in this environment, so the GitHub-API-token-scope boundary could not be exercised. Independent confirmation of mcpg's own write-blocking layer remains covered by internal/guard/internal/difc unit tests; a dedicated MCP-tool-level probe with a write-capable backend launched outside gh-aw's tools.github: wrapper is tracked as follow-up work.

🔒 mcpg read-only stress (default AWF runtime) by Read-Only Stress: default runtime

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

🔒 mcpg Read-Only Stress — docker-sbx

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: docker-sbx (KVM microVM)

Part Surface Op Result Expected Status
A MCP reads data returned (issues, PRs, README.md, commits) ALLOWED
B MCP writes (star/issue/comment/branch/file/PR) all 6 write tools absent from proxy catalog (only 23 read-only tools mounted); attempted star_repository anyway → unknown tool (-32602) BLOCKED ⚠️
C CLI reads data returned via github proxy CLI ALLOWED
D CLI REST writes (reaction/star/issue/comment) not attempted — gh unauthenticated (gh auth status: invalid token, reads return 401) BLOCKED ⚠️
E CLI GraphQL mutations not attempted — same auth gap as Part D BLOCKED ⚠️

Overall: INCONCLUSIVE
(⚠️ = Part B: write tools were never registered/mounted on the proxy CLI catalog at all, so refusal only proves backend/toolset configuration, not an independent gateway DIFC block. Parts D/E: gh CLI has no valid credentials in this run, so write-rejection can't be distinguished from auth failure. No write leaked in any part — reads succeeded, no writes succeeded.)

🔒 mcpg read-only stress (docker-sbx runtime) by Read-Only Stress: docker-sbx runtime

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 Pattern: Opaque token generation reimplements internal/util random helpers

3 participants