[duplicate-code] Duplicate Code Pattern: Enclave and Delegated Request Handling Skeleton

[github-actions]

Part of duplicate code analysis: #13679

Summary

handleDelegatedRequest in internal/proxy/delegation.go and the enclave request handler in internal/proxy/enclave.go implement structurally identical request-parsing/routing/error-handling flows (path parse → query parse → route match → tool/args extraction → DIFC context bind → dispatch), differing mainly in the authorization mechanism (delegation store vs. capability verifier).

Duplication Details

Pattern: Repeated enclave-request parse/route/dispatch skeleton

• Severity: Medium
• Occurrences: 2 (one per handler)
• Locations:
  • internal/proxy/delegation.go (lines 29-63, handleDelegatedRequest)
  • internal/proxy/enclave.go (lines ~200-247, enclave GET handler)
• Code Sample:

  // delegation.go
  path, ok := enclavePath(r.URL.Path, r.URL.RawPath)
  if !ok || r.Method != http.MethodGet || hasEnclaveGETBody(r) {
      writeEnclaveDenied(w)
      return
  }
  query, err := url.ParseQuery(r.URL.RawQuery)
  if err != nil { writeEnclaveDenied(w); return }
  route, err := enclavegithub.MatchEnclaveRoute(path, query)
  if err != nil { ...; writeEnclaveDenied(w); return }
  toolName, args := enclaveToolAndArgs(route)
  if toolName == "" { writeEnclaveDenied(w); return }
  // ... authorize ...
  fullPath := path
  if r.URL.RawQuery != "" { fullPath += "?" + r.URL.RawQuery }
  ctx := withEnclaveAuthorization(r.Context(), ..., route.FullRepo())
  h.handleWithDIFC(w, r.WithContext(ctx), fullPath, toolName, args, nil)
  
  // enclave.go
  path, ok := enclavePath(r.URL.Path, r.URL.RawPath)
  if !ok { ...; writeEnclaveDenied(w); return }
  claims, err := h.server.enclave.verifier.VerifyAuthorization(...)
  if err != nil { ...; writeEnclaveDenied(w); return }
  if r.Method != http.MethodGet || hasEnclaveGETBody(r) { writeEnclaveDenied(w); return }
  query, err := url.ParseQuery(r.URL.RawQuery)
  if err != nil { writeEnclaveDenied(w); return }
  route, err := enclavegithub.MatchEnclaveRoute(path, query)
  if err != nil || !claims.AllowsOperation(route.Operation) { ...; writeEnclaveDenied(w); return }
  // ... additional cross-repo checks ...
  toolName, args := enclaveToolAndArgs(route)
  if toolName == "" { writeEnclaveDenied(w); return }
  fullPath := path
  if r.URL.RawQuery != "" { fullPath += "?" + r.URL.RawQuery }
  ctx := withEnclaveAuthorization(r.Context(), claims.AgentID(), claims.Repo)
  h.handleWithDIFC(w, r.WithContext(ctx), fullPath, toolName, args, nil)

Impact Analysis

• Maintainability: The path/query parsing, route matching, tool/args extraction, fullPath reconstruction, and final handleWithDIFC dispatch are duplicated verbatim across both handlers. A change to the shared request-shape rules (e.g., adding a new denial condition or changing how fullPath is built) must be applied twice.
• Bug Risk: High for authorization-adjacent logic — since these are both security enforcement points (enclave/delegation admission), divergence between the two copies over time could allow one path to skip a check the other has (as already evidenced: enclave.go has an extra cross-repo/public-repo check that delegation.go does not, and it isn't obvious from the code whether that's an intentional difference or a latent gap).
• Code Bloat: ~25-30 lines of duplicated control flow.

Refactoring Recommendations

1. Extract a shared parseAndRouteEnclaveRequest helper

   • Factor the common path/query parse + MatchEnclaveRoute + tool/args extraction + fullPath reconstruction into a single helper function returning (route, toolName, args, fullPath, error), used by both handleDelegatedRequest and the enclave handler, each supplying its own authorization callback.
   • Suggested location: internal/proxy/enclave_common.go (new file) or as a method on proxyHandler.
   • Estimated effort: 3-4 hours (needs careful handling of the differing error-denial branch order — enclave.go checks method/body after auth, delegation.go checks it before route match)
   • Benefits: Single code path for the security-critical routing logic, eliminating risk of the two handlers drifting apart on validation order or denial conditions.

2. Document (or eliminate) the intentional differences

   • If the differing check order and additional cross-repo check in enclave.go are intentional, add an explicit comment in the shared helper (once extracted) noting which caller enables which optional check, so future readers don't assume the two are meant to be identical.
   • Estimated effort: <1 hour
   • Benefits: Prevents future refactors from "fixing" an intentional difference in one file without updating the other by accident.

Implementation Checklist

• Review duplication findings
• Prioritize refactoring tasks
• Create refactoring plan
• Implement changes
• Update tests
• Verify no functionality broken

Parent Issue

See parent analysis report: #13679
Related to #13679

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "proxy.golang.org"

See Network Configuration for more information.

Generated by Duplicate Code Detector · copilot · auto · 123.3 AIC · ⊞ 13.4K · ◷

• expires on Sep 30, 2026, 2:45 AM UTC