Skip to content

feat: Add GitHub DIFC enforcement with WASM guards, guard policies, and LabelAgent integration#1674

Merged
lpcox merged 58 commits intomainfrom
lpcox/github-difc
Mar 8, 2026
Merged

feat: Add GitHub DIFC enforcement with WASM guards, guard policies, and LabelAgent integration#1674
lpcox merged 58 commits intomainfrom
lpcox/github-difc

Conversation

@lpcox
Copy link
Collaborator

@lpcox lpcox commented Mar 8, 2026

  • Update PR title and description
  • Fix data race: AddSecrecyTags/DropIntegrityTag/DropIntegrityTags bypass Label.mu — add Label.Remove/RemoveAll, use thread-safe methods throughout
  • Add unit tests for Label.Remove and Label.RemoveAll
  • Add concurrent race test TestAgentLabels_ConcurrentBulkMutations
  • Fix flaky port collision in TestDIFCEnvironmentVariables — use ephemeral ports via getFreePort(t) helper
  • Fix requireGuardPolicyIfGuardEnabled to use resolveGuardPolicy covering all policy sources (global override, server-specific, guard config)
  • Deprecate sys tools exposure — keep handler implementations but remove SDK registration so sys___init/sys___list_servers never appear in tools/list; remove /mcp/sys route from routed mode regardless of DIFC state
  • Fix guard policy validation to accept "allow-only" (with dash) as the canonical key; legacy "allowonly" (no dash) still accepted for backward compatibility; MarshalJSON outputs "allow-only"; all error messages updated

💡 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 and others added 30 commits March 8, 2026 11:34
Restore config extension features for DIFC session label configuration:

New CLI flags:
- --enable-config-extensions: Gate for enabling DIFC-related config features
- --session-secrecy: Comma-separated initial secrecy labels for agent sessions
- --session-integrity: Comma-separated initial integrity labels for agent sessions

New environment variables:
- MCP_GATEWAY_CONFIG_EXTENSIONS: Enable config extensions
- MCP_GATEWAY_SESSION_SECRECY: Initial secrecy labels
- MCP_GATEWAY_SESSION_INTEGRITY: Initial integrity labels

Features:
- SessionConfig struct added to GatewayConfig for TOML/JSON config support
- Validation that session label flags require --enable-config-extensions
- NewAgentRegistryWithDefaults() called with configured labels
- Tests for all new helper functions
T_EDITOR=true git rebase --continue && git status --short --branch
Restores WASM guard implementation and path labeling from commit 0f1873a
that was inadvertently reverted in commit 04f678e.

Restored files:
- internal/guard/wasm.go: WASM guard with mutex for thread safety
- internal/difc/path_labels.go: Path-based labeling (JSON Pointer RFC 6901)
- internal/difc/path_labels_test.go: Comprehensive tests for path labeling

This enables loading WASM guards from config:
{
  "guards": {
    "github-guard": {
      "type": "wasm",
      "path": "/path/to/guard.wasm"
    }
  }
}
Tests the GitHub guard WASM file at /Users/lpcox/Desktop/ai/github-guard/github-guard-rust.wasm

Verifies:
- Read operations: search_repositories, get_issue, list_commits,
  get_file_contents, list_branches
- Write operations: create_issue, push_files, create_or_update_file,
  create_pull_request
- LabelResponse for search results with path-based labeling
- Concurrent access (WASM module serialization)

Tests are skipped if the WASM file is not present.
The createGuardFromConfig function was calling guard.CreateGuard('wasm')
which looks for a registered factory, but the path to the WASM file was
never passed. Now directly calls guard.NewWasmGuard() with the config path.

This enables guards configured in JSON like:
  "guards": {
    "github-guard": {
      "type": "wasm",
      "path": "/guard/github-guard-rust.wasm"
    }
  }
In filter/propagate mode, read operations should proceed to the backend
even if coarse-grained checks fail, because LabelResponse() provides
per-item labels and Phase 5 filtering can then filter/propagate those
specific items based on their actual data labels.

This restores the behavior from commit 884c490 which was inadvertently
reverted in commit 04f678e.
Propagate mode now correctly handles labels:
- Secrecy: UNION - agent becomes tainted with all secret classifications
  from data it reads (agent_secrecy ∪ resource_secrecy)
- Integrity: INTERSECTION - agent's trustworthiness is reduced to minimum
  (agent_integrity ∩ resource_integrity)

Also fixed:
- Label accumulation only happens in propagate mode (not filter/strict)
- convertToCallToolResult now handles array responses and empty content
- Added Label.Intersect() method for integrity intersection
- Add DIFC Label Rules section explaining secrecy and integrity flow rules
- Add DIFC Modes section describing strict, filter, and propagate modes
- Clarify format detection behavior for WASM guards
- Update filtering behavior documentation to match code
- Fix NoopGuard to return OperationReadWrite instead of OperationWrite
- unified.go: Fix LabelResponse() calling logic per spec:
  - Skip LabelResponse for write operations (all modes)
  - Skip LabelResponse for read-write in strict mode
  - Block entire response in strict mode if any item filtered
  - Apply De Morgan's law per staticcheck QF1001

- guard_test.go: Update NoopGuard tests to expect OperationReadWrite

- config_difc_test.go: Use assert.Empty() instead of nil/len check (S1009)

- agent.go: Fix godoc comment formatting

- Remove wasm_github_test.go (pre-existing test data format issue)
Guards label the inner JSON structure of MCP responses (e.g., /items/0),
but the gateway was trying to resolve these paths against the MCP wrapper
({content:[{text:...}]}), causing 'path not found' errors.

Changes:
- Add unwrapMCPResponse() to detect and extract inner JSON from MCP format
- PathLabeledData now stores both OriginalData and UnwrappedData
- getItems() navigates paths using UnwrappedData
- Add comprehensive tests for MCP unwrapping
Added debug logging to track tool arguments being passed to guards:
- wasm.go: Log args value and full input JSON with byte count
- unified.go: Log args in callBackendTool

This helps diagnose issues where guards may not be receiving tool arguments correctly.
…n requireGuardPolicyIfGuardEnabled

- test/integration/difc_config_test.go: replace 13200+len(name)%100 with
  getFreePort(t) helper to eliminate port collisions between subtests of equal
  name length and with other processes
- internal/server/unified.go: requireGuardPolicyIfGuardEnabled now uses
  resolveGuardPolicy(serverID) so that policy provided via global override
  (cfg.GuardPolicy from CLI/env) or via GuardConfig.Policy is accepted, not
  just ServerConfig.GuardPolicies

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copy link
Contributor

Copilot AI commented Mar 8, 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-build1354663826/b336/launcher.test /tmp/go-build1354663826/b336/launcher.test -test.testlogfile=/tmp/go-build1354663826/b336/testlog.txt -test.paniconexit0 -test.timeout=10m0s -o azero@v1.11.0/internal/sock/sock-errorsas azero@v1.11.0/internal/sock/sock-ifaceassert x_amd64/vet -p vendor/golang.or-unsafeptr=false -lang=go1.25 xAxSkLHMevyv -I _.a 197463/b100/ 64/pkg/tool/linux_amd64/vet --gdwarf-5 --64 -o 64/pkg/tool/linu-trimpath (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build1354663826/b318/config.test /tmp/go-build1354663826/b318/config.test -test.testlogfile=/tmp/go-build1354663826/b318/testlog.txt -test.paniconexit0 -test.timeout=10m0s o_.o�� ache/go/1.25.7/x64/src/runtime/cgo cgo_.o x_amd64/vet (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build1354663826/b336/launcher.test /tmp/go-build1354663826/b336/launcher.test -test.testlogfile=/tmp/go-build1354663826/b336/testlog.txt -test.paniconexit0 -test.timeout=10m0s -o azero@v1.11.0/internal/sock/sock-errorsas azero@v1.11.0/internal/sock/sock-ifaceassert x_amd64/vet -p vendor/golang.or-unsafeptr=false -lang=go1.25 xAxSkLHMevyv -I _.a 197463/b100/ 64/pkg/tool/linux_amd64/vet --gdwarf-5 --64 -o 64/pkg/tool/linu-trimpath (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build1354663826/b336/launcher.test /tmp/go-build1354663826/b336/launcher.test -test.testlogfile=/tmp/go-build1354663826/b336/testlog.txt -test.paniconexit0 -test.timeout=10m0s -o azero@v1.11.0/internal/sock/sock-errorsas azero@v1.11.0/internal/sock/sock-ifaceassert x_amd64/vet -p vendor/golang.or-unsafeptr=false -lang=go1.25 xAxSkLHMevyv -I _.a 197463/b100/ 64/pkg/tool/linux_amd64/vet --gdwarf-5 --64 -o 64/pkg/tool/linu-trimpath (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build1354663826/b345/mcp.test /tmp/go-build1354663826/b345/mcp.test -test.testlogfile=/tmp/go-build1354663826/b345/testlog.txt -test.paniconexit0 -test.timeout=10m0s 1974�� VQN657Pe_ .cfg x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet 1974�� om/segmentio/enc-c=4 .cfg x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet (dns block)

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

Copilot finished work on behalf of lpcox March 8, 2026 20:39
@lpcox
Copy link
Collaborator Author

lpcox commented Mar 8, 2026

@copilot is the flag "--enable-difc" off by default, in other words is difc enforcement and loading mcp guards off by default?

Copy link
Contributor

Copilot AI commented Mar 8, 2026

@copilot is the flag "--enable-difc" off by default, in other words is difc enforcement and loading mcp guards off by default?

Yes, --enable-difc defaults to false (see defaultEnableDIFC = false in internal/cmd/flags_difc.go:18), and the flag is hidden. With DIFC disabled:

  • DIFC enforcement is off — the evaluator's flow checks, filter/strict/propagate mode behavior, and LabelAgent initialization are not applied meaningfully
  • Guards are still loaded at startup (in NewUnifiedServer) regardless of the flag, but servers with no guard configured (which is the default) get a noop guard that passes all calls through unconditionally
  • Sys tools are not registered when DIFC is disabled (the if us.enableDIFC block at unified.go:224 skips them)

So out of the box, with no configuration, DIFC enforcement and meaningful guard evaluation are both inactive. They only activate when --enable-difc (or MCP_GATEWAY_ENABLE_DIFC=true) is set AND a non-noop guard with a policy is configured for a server.

Copilot finished work on behalf of lpcox March 8, 2026 20:43
@lpcox
Copy link
Collaborator Author

lpcox commented Mar 8, 2026

@copilot what happens to sys tools if difc is enabled? they should remain off and unexposed at all times if they are not.

Copy link
Contributor

Copilot AI commented Mar 8, 2026

@copilot what happens to sys tools if difc is enabled? they should remain off and unexposed at all times if they are not.

When --enable-difc is set, sys___init and sys___list_servers are registered with the MCP SDK and appear in the tools/list response visible to clients. This is intentional — sys___init is the mechanism agents use to initialize their DIFC session (passing a token to register themselves), and sys___list_servers lets agents discover available backends.

When DIFC is disabled (the default), neither tool is registered or visible — the if us.enableDIFC guard at unified.go:224 ensures registerSysTools() is never called.

If the intent is that these tools should never appear in tools/list even when DIFC is on (i.e., DIFC session initialization should happen out-of-band rather than via a client-callable tool), that would be a design change — let me know and I can make it.

Copilot finished work on behalf of lpcox March 8, 2026 20:47
@lpcox
Copy link
Collaborator Author

lpcox commented Mar 8, 2026

@copilot the sys tools should be considered deprecated since the agent labels are set when a guard is initialized via the label_agent call. keep the implementation but otherwise they should not be exposed to the agent.

also please make sure that when validating an allow-only config the string "allow-only" (with a dash) is used and not "allowonly" (no dash) for example the following should pass validation

   "allow-only": {
       "repos": repos,
       "min-integrity": integrity,
   }

…anonical guard policy key

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copy link
Contributor

Copilot AI commented Mar 8, 2026

@copilot the sys tools should be considered deprecated since the agent labels are set when a guard is initialized via the label_agent call. keep the implementation but otherwise they should not be exp...

Done in commit b07b623:

Sys tools hidden from agents: sys___init and sys___list_servers handlers are kept in the internal tools map but are no longer registered with the MCP SDK. They never appear in tools/list and cannot be called by agents regardless of DIFC state. The /mcp/sys route is also removed from routed mode entirely.

"allow-only" (with dash) is now the canonical key: GuardPolicy.UnmarshalJSON now accepts both "allow-only" (canonical) and "allowonly" (backward compat). MarshalJSON outputs "allow-only". All error messages updated. Example that now passes:

"allow-only": {
    "repos": repos,
    "min-integrity": integrity
}

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-build3359572567/b336/launcher.test /tmp/go-build3359572567/b336/launcher.test -test.testlogfile=/tmp/go-build3359572567/b336/testlog.txt -test.paniconexit0 -test.timeout=10m0s -n .go th2.go x_amd64/compile (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build3359572567/b318/config.test /tmp/go-build3359572567/b318/config.test -test.testlogfile=/tmp/go-build3359572567/b318/testlog.txt -test.paniconexit0 -test.timeout=10m0s --no�� 64/src/runtime/c-c=4 64/src/internal/-nolocalimports ache/go/1.25.7/x-importcfg (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build3359572567/b336/launcher.test /tmp/go-build3359572567/b336/launcher.test -test.testlogfile=/tmp/go-build3359572567/b336/testlog.txt -test.paniconexit0 -test.timeout=10m0s -n .go th2.go x_amd64/compile (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build3359572567/b336/launcher.test /tmp/go-build3359572567/b336/launcher.test -test.testlogfile=/tmp/go-build3359572567/b336/testlog.txt -test.paniconexit0 -test.timeout=10m0s -n .go th2.go x_amd64/compile (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build3359572567/b345/mcp.test /tmp/go-build3359572567/b345/mcp.test -test.testlogfile=/tmp/go-build3359572567/b345/testlog.txt -test.paniconexit0 -test.timeout=10m0s ache�� g_.a udS0MYhRA x_amd64/vet (dns block)

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

Copilot finished work on behalf of lpcox March 8, 2026 21:01
@lpcox
Copy link
Collaborator Author

lpcox commented Mar 8, 2026

@copilot

The "allow-only" object in the comment below is not actually valid since values for "repos" and "min-integrity" are non-conformant with the schema and spec. Can you confirm that aside from changing "allowonly" to "allow-only" validation did not change?

#1674 (comment)

Copy link
Contributor

Copilot AI commented Mar 8, 2026

@copilot

The "allow-only" object in the comment below is not actually valid since values for "repos" and "min-integrity" are non-conformant with the schema and spec. Can you confirm that aside fro...

Confirmed — the validation logic is identical to before. The only changes in guard_policy.go were:

  1. GuardPolicy.UnmarshalJSON now matches on "allow-only" or "allowonly" (backward compat) — no other logic change
  2. Error message strings updated from allowonly.*allow-only.*
  3. MarshalJSON struct tag changed from json:"allowonly"json:"allow-only"

All of the following rules are completely unchanged:

  • repos accepts "all", "public", or a non-empty array of owner/repo-pattern strings
  • min-integrity must be one of none, unapproved, approved, merged
  • repos array entries must match owner/*, owner/repo, or owner/prefix* (alphanumeric + _ + - chars only, no dots, no duplicates)
  • Fields repos and min-integrity are both required inside allow-only

The example in my previous reply used placeholder variable names (repos, integrity) rather than literal values — those placeholders would not pass validation, but that was just shorthand in the comment, not actual test input.

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.

4 participants