diff --git a/internal/proxy/graphql.go b/internal/proxy/graphql.go index 623a0e7d1..9ce3edb78 100644 --- a/internal/proxy/graphql.go +++ b/internal/proxy/graphql.go @@ -52,8 +52,9 @@ var graphqlPatterns = []graphqlPattern{ // Repository info {queryPattern: regexp.MustCompile(`(?i)\brepository\s*\(`), toolName: "get_file_contents"}, - // User/viewer - {queryPattern: regexp.MustCompile(`(?i)\bviewer\s*\{`), toolName: "get_me"}, + // viewer { ... } is intentionally not mapped — the guard does not recognize a tool name + // with equivalent semantics for user/account data, and it may include private fields. + // Unknown GraphQL queries are blocked by the handler. } // ownerRepoPattern extracts owner and repo from GraphQL variables or query text. diff --git a/internal/proxy/handler.go b/internal/proxy/handler.go index 3f312a967..e19f325b4 100644 --- a/internal/proxy/handler.go +++ b/internal/proxy/handler.go @@ -64,8 +64,14 @@ func (h *proxyHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { match := MatchGraphQL(graphQLBody) if match == nil { - // Unknown GraphQL query — pass through with conservative labeling - h.forwardGraphQL(w, r, fullPath, graphQLBody) + // Unknown GraphQL query — fail closed: deny rather than risk leaking unfiltered data + logHandler.Printf("unknown GraphQL query, blocking request") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusForbidden) + json.NewEncoder(w).Encode(map[string]interface{}{ + "errors": []map[string]string{{"message": "access denied: unrecognized GraphQL operation"}}, + "data": nil, + }) return } toolName = match.ToolName @@ -73,8 +79,9 @@ func (h *proxyHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } else { match := MatchRoute(rawPath) if match == nil { - // Unknown REST endpoint — pass through - h.passthrough(w, r, fullPath) + // Unknown REST endpoint — fail closed: deny rather than risk leaking unfiltered data + logHandler.Printf("unknown REST endpoint %s, blocking request", rawPath) + http.Error(w, "access denied: unrecognized endpoint", http.StatusForbidden) return } toolName = match.ToolName @@ -92,12 +99,8 @@ func (h *proxyHandler) handleWithDIFC(w http.ResponseWriter, r *http.Request, pa backend := &stubBackendCaller{} if !s.guardInitialized { - log.Printf("[proxy] WARNING: guard not initialized, passing through") - if graphQLBody != nil { - h.forwardGraphQL(w, r, path, graphQLBody) - } else { - h.passthrough(w, r, path) - } + log.Printf("[proxy] WARNING: guard not initialized, blocking request") + http.Error(w, "proxy enforcement not configured", http.StatusServiceUnavailable) return } @@ -110,12 +113,8 @@ func (h *proxyHandler) handleWithDIFC(w http.ResponseWriter, r *http.Request, pa resource, operation, err := s.guard.LabelResource(ctx, toolName, args, backend, s.capabilities) if err != nil { logHandler.Printf("[DIFC] Phase 1 failed: %v", err) - // On labeling failure, pass through (fail-open for read operations) - if graphQLBody != nil { - h.forwardGraphQL(w, r, path, graphQLBody) - } else { - h.passthrough(w, r, path) - } + // On labeling failure, fail closed to prevent enforcement bypass + http.Error(w, "resource labeling failed", http.StatusBadGateway) return } @@ -186,7 +185,7 @@ func (h *proxyHandler) handleWithDIFC(w http.ResponseWriter, r *http.Request, pa if evalResult.IsAllowed() { h.writeResponse(w, resp, respBody) } else { - h.writeEmptyResponse(w, resp) + h.writeEmptyResponse(w, resp, responseData) } return } @@ -223,7 +222,7 @@ func (h *proxyHandler) handleWithDIFC(w http.ResponseWriter, r *http.Request, pa finalData, err = filtered.ToResult() if err != nil { logHandler.Printf("[DIFC] Phase 5 ToResult failed: %v", err) - h.writeEmptyResponse(w, resp) + h.writeEmptyResponse(w, resp, responseData) return } } else { @@ -231,7 +230,7 @@ func (h *proxyHandler) handleWithDIFC(w http.ResponseWriter, r *http.Request, pa finalData, err = labeledData.ToResult() if err != nil { logHandler.Printf("[DIFC] Phase 5 ToResult failed: %v", err) - h.writeEmptyResponse(w, resp) + h.writeEmptyResponse(w, resp, responseData) return } } @@ -240,7 +239,7 @@ func (h *proxyHandler) handleWithDIFC(w http.ResponseWriter, r *http.Request, pa if evalResult.IsAllowed() { finalData = responseData } else { - h.writeEmptyResponse(w, resp) + h.writeEmptyResponse(w, resp, responseData) return } } @@ -317,12 +316,31 @@ func (h *proxyHandler) writeResponse(w http.ResponseWriter, resp *http.Response, w.Write(body) } -// writeEmptyResponse writes an empty JSON array or object response. -func (h *proxyHandler) writeEmptyResponse(w http.ResponseWriter, resp *http.Response) { +// writeEmptyResponse writes an empty JSON response matching the shape of the original data. +// originalData should be the parsed upstream response; nil or unrecognized types fall back to "[]". +// For JSON arrays it writes "[]", for GraphQL objects with a "data" key it writes {"data":null}, +// and for other JSON objects it writes "{}". +func (h *proxyHandler) writeEmptyResponse(w http.ResponseWriter, resp *http.Response, originalData interface{}) { copyResponseHeaders(w, resp) w.Header().Set("Content-Type", "application/json") w.WriteHeader(resp.StatusCode) - w.Write([]byte("[]")) + + var empty string + switch originalData.(type) { + case []interface{}: + empty = "[]" + case map[string]interface{}: + obj := originalData.(map[string]interface{}) + // GraphQL responses wrap their payload in a "data" key + if _, ok := obj["data"]; ok { + empty = `{"data":null}` + } else { + empty = "{}" + } + default: + empty = "[]" // safe default for nil or unknown types + } + w.Write([]byte(empty)) } // copyResponseHeaders copies relevant headers from upstream to the client response. diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index a5ba079b1..2c2e71c2b 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -86,6 +86,9 @@ func New(ctx context.Context, cfg Config) (*Server, error) { // Parse enforcement mode difcMode, err := difc.ParseEnforcementMode(cfg.DIFCMode) if err != nil { + if cfg.DIFCMode != "" { + log.Printf("[proxy] WARNING: invalid DIFC mode %q, defaulting to filter", cfg.DIFCMode) + } difcMode = difc.EnforcementFilter // default to filter for proxy } @@ -96,12 +99,13 @@ func New(ctx context.Context, cfg Config) (*Server, error) { } s := &Server{ - guard: g, - evaluator: difc.NewEvaluatorWithMode(difcMode), - agentRegistry: difc.NewAgentRegistryWithDefaults(nil, nil), - capabilities: difc.NewCapabilities(), - githubToken: cfg.GitHubToken, - githubAPIURL: apiURL, + guard: g, + evaluator: difc.NewEvaluatorWithMode(difcMode), + agentRegistry: difc.NewAgentRegistryWithDefaults(nil, nil), + capabilities: difc.NewCapabilities(), + githubToken: cfg.GitHubToken, + githubAPIURL: apiURL, + enforcementMode: difcMode, httpClient: &http.Client{ Timeout: 60 * time.Second, Transport: &http.Transport{ diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go index 59984ad41..ef61aed29 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -31,14 +31,14 @@ func TestMatchRoute(t *testing.T) { { name: "issue comments", path: "/repos/octocat/hello-world/issues/42/comments", - wantTool: "get_comments", - wantArgs: map[string]interface{}{"owner": "octocat", "repo": "hello-world", "issue_number": "42"}, + wantTool: "issue_read", + wantArgs: map[string]interface{}{"owner": "octocat", "repo": "hello-world", "issue_number": "42", "method": "get_comments"}, }, { name: "issue labels", path: "/repos/octocat/hello-world/issues/42/labels", - wantTool: "get_labels", - wantArgs: map[string]interface{}{"owner": "octocat", "repo": "hello-world", "issue_number": "42"}, + wantTool: "issue_read", + wantArgs: map[string]interface{}{"owner": "octocat", "repo": "hello-world", "issue_number": "42", "method": "get_labels"}, }, // Pull Requests @@ -153,12 +153,11 @@ func TestMatchRoute(t *testing.T) { wantArgs: map[string]interface{}{}, }, - // User + // User — not mapped; unknown paths are blocked (fail closed) { - name: "get me", - path: "/user", - wantTool: "get_me", - wantArgs: map[string]interface{}{}, + name: "get me", + path: "/user", + wantNil: true, }, // Query string stripping @@ -236,9 +235,9 @@ func TestMatchGraphQL(t *testing.T) { wantTool: "search_issues", }, { - name: "viewer query", - body: `{"query":"query { viewer { login name email } }"}`, - wantTool: "get_me", + name: "viewer query", + body: `{"query":"query { viewer { login name email } }"}`, + wantNil: true, }, { name: "empty query", diff --git a/internal/proxy/router.go b/internal/proxy/router.go index 9ed3ca3d8..a9108d1bd 100644 --- a/internal/proxy/router.go +++ b/internal/proxy/router.go @@ -39,16 +39,16 @@ var routes = []route{ // Issues { pattern: regexp.MustCompile(`^/repos/([^/]+)/([^/]+)/issues/(\d+)/comments$`), - toolName: "get_comments", + toolName: "issue_read", extractArgs: func(m []string) map[string]interface{} { - return map[string]interface{}{"owner": m[1], "repo": m[2], "issue_number": m[3]} + return map[string]interface{}{"owner": m[1], "repo": m[2], "issue_number": m[3], "method": "get_comments"} }, }, { pattern: regexp.MustCompile(`^/repos/([^/]+)/([^/]+)/issues/(\d+)/labels$`), - toolName: "get_labels", + toolName: "issue_read", extractArgs: func(m []string) map[string]interface{} { - return map[string]interface{}{"owner": m[1], "repo": m[2], "issue_number": m[3]} + return map[string]interface{}{"owner": m[1], "repo": m[2], "issue_number": m[3], "method": "get_labels"} }, }, { @@ -236,14 +236,9 @@ var routes = []route{ }, }, - // User API - { - pattern: regexp.MustCompile(`^/user$`), - toolName: "get_me", - extractArgs: func(_ []string) map[string]interface{} { - return map[string]interface{}{} - }, - }, + // User API (/user) is intentionally not mapped — it cannot be correctly labeled + // by the guard (no recognized tool name with equivalent semantics) and may contain + // private account data (e.g., email). Unknown paths are blocked by the handler. // Generic repo-scoped fallback (must be last) {