Skip to content

Extract shared HMAC-SHA256 signing utility - #12034

Merged
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-hmac-sha256
Aug 28, 2026
Merged

Extract shared HMAC-SHA256 signing utility#12034
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-hmac-sha256

Conversation

Copilot AI commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Request authentication and enclave capability validation independently implemented HMAC-SHA256 signing and constant-time verification. This consolidates the cryptographic primitive while preserving each protocol’s canonical message format and encoding.

  • Shared crypto utility

    • Add internal/hmacutil with Sign and constant-time Verify.
  • Request HMAC authentication

    • Use the shared utility for signing canonical request messages.
    • Decode hex request signatures before shared verification.
  • Enclave capabilities

    • Replace inline HMAC construction and subtle.ConstantTimeCompare with the shared verifier.
if !hmacutil.Verify(v.key, signingInput, providedSignature) {
    return nil, fmt.Errorf("invalid enclave capability")
}

Copilot AI and others added 2 commits August 28, 2026 15:01
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor duplicate HMAC-SHA256 logic in server and enclavegithub Extract shared HMAC-SHA256 signing utility Aug 28, 2026
Copilot AI requested a review from lpcox August 28, 2026 15:06
@lpcox
lpcox marked this pull request as ready for review August 28, 2026 15:15
Copilot AI balanced review requested due to automatic review settings August 28, 2026 15:15

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

Extracts shared HMAC-SHA256 signing and constant-time verification while preserving protocol-specific formats.

Changes:

  • Adds tested hmacutil helpers.
  • Migrates request and enclave capability verification.
Show a summary per file
File Description
internal/hmacutil/hmacutil.go Implements shared HMAC helpers.
internal/hmacutil/hmacutil_test.go Tests signing and verification failures.
internal/server/hmac.go Uses shared helpers for request authentication.
internal/enclavegithub/capability.go Uses shared verification for capabilities.

Review details

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

  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

🔒 mcpg Read-Only Stress — gVisor

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: gVisor (runsc) kernel-level isolation

Part Surface Op Result Expected Status
A MCP reads (issues/PRs/file/commits) data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) all tools absent from catalog BLOCKED ⚠️
C CLI reads (list_issues, get_file_contents) data returned ALLOWED
D CLI REST writes gh unauthenticated BLOCKED ⚠️
E CLI GraphQL mutations (addReaction/addStar/createIssue) gh unauthenticated BLOCKED ⚠️

Overall: INCONCLUSIVE

⚠️ Part B: All 7 write tools (add_issue_comment, star_repository, issue_write, create_branch, create_or_update_file, create_pull_request) were absent from the MCP tool catalog (backend launched with GITHUB_READ_ONLY=1). This confirms gh-aw's defense-in-depth guarantee but cannot independently confirm mcpg's own DIFC/guard enforcement layer since write tools never reached a write-capable backend.

⚠️ Parts D & E: gh CLI is not authenticated in this environment. All REST and GraphQL write attempts would return 401 — this cannot distinguish token-scope blocking from unauthenticated rejection. No writes were attempted or leaked.

No write leaked through mcpg in this run. The inconclusive status reflects methodology gaps (read-only backend + unauthenticated CLI), not evidence of enforcement failure.

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

@github-actions

Copy link
Copy Markdown
Contributor

🔒 mcpg Read-Only Stress — default AWF

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

Part Surface Op Result Expected Status
A MCP reads (list_issues, list_prs, get_file, list_commits) data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) all tools absent from catalog BLOCKED ⚠️
C CLI reads (list_issues, get_file_contents) data returned ALLOWED
D CLI REST writes (reaction/star/issue/comment/file) gh not authenticated BLOCKED ⚠️
E CLI GraphQL mutations (addReaction/addStar/createIssue) gh not authenticated BLOCKED ⚠️

Overall: INCONCLUSIVE

⚠️ Methodology gaps (no writes leaked, but gateway enforcement not independently confirmed):

  • Part B: All 6 write tool targets returned unknown tool [-32602] — they are absent from the exposed catalog because the backend runs with GITHUB_READ_ONLY=1 (write tools never registered). This is the gh-aw framework's own defense-in-depth. No tool was present in the catalog and then refused at call-time, so mcpg's own DIFC/guard enforcement layer was not independently exercised on this surface.

  • Part D/E: gh is not authenticated in this environment (gh auth status → "not logged into any GitHub hosts"). All REST write and GraphQL mutation attempts would produce 401, not evidence of gateway enforcement. These rows cannot distinguish token-scope blocking from gateway blocking.

No writes leaked. The read-only guarantee held in practice for this runtime. The INCONCLUSIVE rating reflects the methodology limits above, not a security failure.

Full Report

MCP Tool Catalog (23 tools, all read-only):
get_commit, get_file_contents, get_label, get_latest_release, get_release_by_tag, get_tag, issue_read, list_branches, list_commits, list_issue_fields, list_issue_types, list_issues, list_pull_requests, list_releases, list_repository_collaborators, list_starred_repositories, list_tags, pull_request_read, search_code, search_commits, search_issues, search_pull_requests, search_repositories

Part B write attempts (all → Error [-32602]: unknown tool):

  • add_issue_comment, star_repository, issue_write, create_branch, create_pull_request

Result file: readonly-stress-result-33183181839.txtRESULT=INCONCLUSIVE RUNTIME=default RUNID=33183181839

References: §33183181839

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

@github-actions

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-isolated microVM)

Part Surface Op Result Expected Status
A MCP list_issues 3 issues returned ALLOWED
A MCP list_pull_requests 3 PRs returned ALLOWED
A MCP get_file_contents (README.md) content returned ALLOWED
A MCP list_commits 3 commits returned ALLOWED
B MCP add_issue_comment absent from catalog BLOCKED ⚠️
B MCP star_repository absent from catalog BLOCKED ⚠️
B MCP issue_write absent from catalog BLOCKED ⚠️
B MCP create_branch absent from catalog BLOCKED ⚠️
B MCP create_or_update_file absent from catalog BLOCKED ⚠️
B MCP create_pull_request absent from catalog BLOCKED ⚠️
C CLI github list_issues data returned ALLOWED
C CLI github get_file_contents data returned ALLOWED
D CLI REST writes (all 6) gh unauthenticated (401) BLOCKED ⚠️
E CLI GraphQL mutations (all 3) gh unauthenticated (401) BLOCKED ⚠️

Overall: INCONCLUSIVE

⚠️ Gaps (not FAIL — no write succeeded):

  • Part B: All 6 write tool targets absent from catalog (23 read-only tools exposed). Backend launched with GITHUB_READ_ONLY=1; write tools never registered. Gateway-layer DIFC enforcement not independently confirmed for MCP tool writes.
  • Parts D & E: gh token invalid (GH_TOKEN reports 401 Bad credentials). REST/GraphQL write blocking could not be independently confirmed via CLI this run.

No writes succeeded. No security regression detected.

References: §33183181791

🔒 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] Duplicate Code Pattern: HMAC-SHA256 Sign/Verify Logic in server and enclavegithub

3 participants