Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions internal/proxy/graphql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
64 changes: 41 additions & 23 deletions internal/proxy/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,24 @@

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
args = match.Args
} 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
Expand All @@ -92,12 +99,8 @@
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
}

Expand All @@ -110,12 +113,8 @@
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
}

Expand Down Expand Up @@ -186,7 +185,7 @@
if evalResult.IsAllowed() {
h.writeResponse(w, resp, respBody)
} else {
h.writeEmptyResponse(w, resp)
h.writeEmptyResponse(w, resp, responseData)
}
return
}
Expand Down Expand Up @@ -223,15 +222,15 @@
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 {
// Simple labeled data — already passed coarse check
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
}
}
Expand All @@ -240,7 +239,7 @@
if evalResult.IsAllowed() {
finalData = responseData
} else {
h.writeEmptyResponse(w, resp)
h.writeEmptyResponse(w, resp, responseData)
return
}
}
Expand Down Expand Up @@ -293,7 +292,7 @@
}

// forwardGraphQL forwards a GraphQL request without DIFC filtering.
func (h *proxyHandler) forwardGraphQL(w http.ResponseWriter, r *http.Request, _ string, body []byte) {

Check failure on line 295 in internal/proxy/handler.go

View workflow job for this annotation

GitHub Actions / lint

func (*proxyHandler).forwardGraphQL is unused (unused)
resp, err := h.server.forwardToGitHub(r.Context(), http.MethodPost, "/graphql", bytes.NewReader(body), "application/json")
if err != nil {
http.Error(w, "upstream request failed", http.StatusBadGateway)
Expand All @@ -317,12 +316,31 @@
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) {

Check failure on line 329 in internal/proxy/handler.go

View workflow job for this annotation

GitHub Actions / lint

S1034: assigning the result of this type assertion to a variable (switch originalData := originalData.(type)) could eliminate type assertions in switch cases (staticcheck)
case []interface{}:
empty = "[]"
case map[string]interface{}:
obj := originalData.(map[string]interface{})

Check failure on line 333 in internal/proxy/handler.go

View workflow job for this annotation

GitHub Actions / lint

S1034(related information): could eliminate this type assertion (staticcheck)
// 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.
Expand Down
16 changes: 10 additions & 6 deletions internal/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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{
Expand Down
23 changes: 11 additions & 12 deletions internal/proxy/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
19 changes: 7 additions & 12 deletions internal/proxy/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
},
},
{
Expand Down Expand Up @@ -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)
{
Expand Down
Loading