Refactor DIFC pre-phase error handling into a shared guard helper#7760
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors DIFC pre-phase denial handling to eliminate duplicated *guard.PipelineAccessDenied unpacking/formatting logic across the unified MCP server and the HTTP proxy, by centralizing classification + detailed violation formatting in guard.HandlePrePhaseError.
Changes:
- Added
guard.HandlePrePhaseError(err)to classifyRunPipelinePrePhaseserrors viaerrors.Asand produce a formatted DIFC violation error. - Updated unified MCP server denial handling to use the shared helper while continuing to return the detailed MCP error payload.
- Updated proxy denial handling to use the shared helper while preserving the existing short 403 body, and added guard-level tests for denied vs non-denied pre-phase errors.
Show a summary per file
| File | Description |
|---|---|
| internal/server/unified.go | Replaces inline PipelineAccessDenied handling with the shared guard helper for consistent denial formatting. |
| internal/proxy/handler.go | Uses the shared guard helper for denial classification while keeping the proxy’s short 403 response behavior. |
| internal/guard/pipeline.go | Introduces HandlePrePhaseError to centralize pre-phase denial detection and detailed violation formatting. |
| internal/guard/pipeline_test.go | Adds unit tests covering denied vs non-denied behavior for the new helper, including wrapped denial errors. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 1
| if denied, detailedErr := guard.HandlePrePhaseError(err); denied != nil { | ||
| logHandler.Printf("[DIFC] Phase 2: BLOCKED %s %s — %s", r.Method, path, denied.EvalResult.Reason) | ||
| deniedErr := fmt.Errorf("DIFC policy violation: %s", denied.EvalResult.Reason) | ||
| tracing.RecordSpanError(difcSpan, deniedErr, "access denied: "+denied.EvalResult.Reason) | ||
| tracing.RecordSpanError(difcSpan, detailedErr, "access denied: "+denied.EvalResult.Reason) | ||
| writeDIFCForbidden(w, deniedErr.Error()) |
|
@copilot address review feedback |
Fixed in the latest commit. The proxy span now records the short |
RunPipelinePrePhasesdenial handling had drift-prone duplication in the unified MCP server and HTTP proxy. Both callers were separately unpacking*PipelineAccessDeniedand formatting denial responses from the same guard contract.Centralize pre-phase error classification
guard.HandlePrePhaseError(err)ininternal/guard/pipeline.goPipelineAccessDenieddetection and detailed DIFC violation formatting behind one helpererrors.Asso wrapped denial errors still resolve consistentlyKeep caller-specific response behavior
internal/server/unified.gonow uses the shared helper and continues returning the detailed formatted MCP errorinternal/proxy/handler.gonow uses the same helper while preserving the existing short HTTP 403 bodyTighten the guard contract with focused tests