Skip to content

fix: harden proxy DIFC enforcement — fail closed throughout pipeline#2188

Merged
lpcox merged 2 commits intofeat/proxy-modefrom
copilot/sub-pr-2176
Mar 19, 2026
Merged

fix: harden proxy DIFC enforcement — fail closed throughout pipeline#2188
lpcox merged 2 commits intofeat/proxy-modefrom
copilot/sub-pr-2176

Conversation

Copy link
Contributor

Copilot AI commented Mar 19, 2026

The proxy mode had multiple fail-open paths that could bypass DIFC enforcement: unknown endpoints passed through unfiltered, an uninitialized guard silently forwarded all traffic, LabelResource failures forwarded the request, writeEmptyResponse always returned [] regardless of response shape, and enforcementMode was never seeded from the CLI flag.

Fail-closed enforcement

  • Unknown REST endpoints403 Forbidden (was: passthrough)
  • Unknown GraphQL queries{"errors":[{"message":"access denied: ..."}],"data":null} with 403 (was: passthrough)
  • Guard not initialized503 Service Unavailable (was: passthrough)
  • Phase 1 LabelResource failure502 Bad Gateway (was: passthrough)

Tool name mapping

  • Issue comments/labels (/issues/{n}/comments, /issues/{n}/labels) remapped from unrecognized get_comments/get_labelsissue_read with a method arg, consistent with MCP server tool naming and guard recognition
  • /user REST route and viewer GraphQL pattern removedget_me is not recognized by the guard, risking under-labeling of private account data (email, etc.); these paths now hit the fail-closed handler

Enforcement mode

  • Server.enforcementMode now initialized from parsed CLI/config mode at construction; previously left at zero value (strict) unless LabelAgent returned an override
  • Invalid --guards-mode values log a warning instead of silently defaulting to filter

Response shape

  • writeEmptyResponse now returns the correct empty shape per upstream response type: [] for arrays, {"data":null} for GraphQL responses (detected by "data" key presence), {} for other objects

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@lpcox lpcox marked this pull request as ready for review March 19, 2026 21:22
Copilot AI review requested due to automatic review settings March 19, 2026 21:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Fail closed (403/502/503) for unknown endpoints, uninitialized guard,
  and LabelResource failures instead of passing through unfiltered
- Initialize Server.enforcementMode from parsed CLI/config mode at
  construction time; log warning for invalid DIFC mode values
- Fix writeEmptyResponse to return shape-matched empty response:
  [] for arrays, {"data":null} for GraphQL objects, {} for other objects
- Map issue comments/labels routes to guard-recognized tool "issue_read"
  (was "get_comments"/"get_labels") with method arg
- Remove /user route and viewer GraphQL pattern (guard does not recognize
  "get_me"; these now fail closed via unknown-endpoint handler)

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/0b55b205-bccb-40cb-af8f-963cb6aae51f
Copy link
Contributor Author

Copilot AI commented Mar 19, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • example.com
    • Triggering command: /tmp/go-build3276356306/b333/launcher.test /tmp/go-build3276356306/b333/launcher.test -test.testlogfile=/tmp/go-build3276356306/b333/testlog.txt -test.paniconexit0 -test.timeout=10m0s 8098�� ache/go/1.25.8/x-D RaWZ/z704niHGLlLGOAMD64_v1 x_amd64/vet (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build3276356306/b318/config.test /tmp/go-build3276356306/b318/config.test -test.testlogfile=/tmp/go-build3276356306/b318/testlog.txt -test.paniconexit0 -test.timeout=10m0s rev-�� g_.a (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build3276356306/b333/launcher.test /tmp/go-build3276356306/b333/launcher.test -test.testlogfile=/tmp/go-build3276356306/b333/testlog.txt -test.paniconexit0 -test.timeout=10m0s 8098�� ache/go/1.25.8/x-D RaWZ/z704niHGLlLGOAMD64_v1 x_amd64/vet (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build3276356306/b333/launcher.test /tmp/go-build3276356306/b333/launcher.test -test.testlogfile=/tmp/go-build3276356306/b333/testlog.txt -test.paniconexit0 -test.timeout=10m0s 8098�� ache/go/1.25.8/x-D RaWZ/z704niHGLlLGOAMD64_v1 x_amd64/vet (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build3276356306/b342/mcp.test /tmp/go-build3276356306/b342/mcp.test -test.testlogfile=/tmp/go-build3276356306/b342/testlog.txt -test.paniconexit0 -test.timeout=10m0s -I g_.a om/!burnt!sushi/main x_amd64/vet 8098757/b149/ 8098757/b138/sym--version ctor x_amd64/vet -I aw-mcpg/internalgo1.25.8 cfg x_amd64/vet --gdwarf-5 --64 8098757/b149/ x_amd64/vet (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Add proxy mode for GitHub API DIFC filtering fix: harden proxy DIFC enforcement — fail closed throughout pipeline Mar 19, 2026
Copilot AI requested a review from lpcox March 19, 2026 21:24
@lpcox lpcox merged commit b613ed0 into feat/proxy-mode Mar 19, 2026
8 of 9 checks passed
@lpcox lpcox deleted the copilot/sub-pr-2176 branch March 19, 2026 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants