From 969566a773dda955caf643f208239df8d77fe708 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Sun, 15 Mar 2026 11:50:12 -0700 Subject: [PATCH] chore: cleanup stale docs and trim README - Delete 6 stale tracked files: ISSUE_18295_FIX.md, PAYLOAD_THRESHOLD_IMPLEMENTATION.md, SESSION_PERSISTENCE_ANALYSIS.md, docs/DIFC_INTEGRATION_PROPOSAL.md, docs/TOML_MODULE_REVIEW.md, docs/STREAMABLE_HTTP_HANDLER_EXPLAINED.md - Trim README.md from 929 to 333 lines (64% reduction) - Extract detailed config reference to docs/CONFIGURATION.md - Extract environment variables to docs/ENVIRONMENT_VARIABLES.md - Condense logging, auth, architecture, and containerized mode sections - Replace verbose CLI help dump with brief usage examples - Remove stale MCP server test result tables Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ISSUE_18295_FIX.md | 155 --- PAYLOAD_THRESHOLD_IMPLEMENTATION.md | 365 ------ README.md | 724 +---------- SESSION_PERSISTENCE_ANALYSIS.md | 292 ----- docs/CONFIGURATION.md | 262 ++++ docs/DIFC_INTEGRATION_PROPOSAL.md | 1362 --------------------- docs/ENVIRONMENT_VARIABLES.md | 63 + docs/STREAMABLE_HTTP_HANDLER_EXPLAINED.md | 343 ------ docs/TOML_MODULE_REVIEW.md | 320 ----- 9 files changed, 389 insertions(+), 3497 deletions(-) delete mode 100644 ISSUE_18295_FIX.md delete mode 100644 PAYLOAD_THRESHOLD_IMPLEMENTATION.md delete mode 100644 SESSION_PERSISTENCE_ANALYSIS.md create mode 100644 docs/CONFIGURATION.md delete mode 100644 docs/DIFC_INTEGRATION_PROPOSAL.md create mode 100644 docs/ENVIRONMENT_VARIABLES.md delete mode 100644 docs/STREAMABLE_HTTP_HANDLER_EXPLAINED.md delete mode 100644 docs/TOML_MODULE_REVIEW.md diff --git a/ISSUE_18295_FIX.md b/ISSUE_18295_FIX.md deleted file mode 100644 index f5085079..00000000 --- a/ISSUE_18295_FIX.md +++ /dev/null @@ -1,155 +0,0 @@ -# Fix for GitHub Issue #18295: MCP Tool Calling Loop Issues - -## Problem Statement - -Agents were getting stuck in infinite loops when calling MCP tools like `github-list_commits`. The agent would repeatedly make the same tool call, receive a response indicating the payload was too large, attempt to read the payload file, fail (FILE_NOT_FOUND), and retry - creating an infinite loop that would only stop when the workflow timed out. - -## Root Cause Analysis - -The issue was caused by the interaction between three factors: - -1. **Low Default Threshold**: The default payload size threshold was set to 10KB (10,240 bytes) -2. **Typical GitHub API Response Sizes**: GitHub API responses (especially `list_commits` over 3 days) frequently exceed 10KB: - - Small query (1-5 commits): ~2-5KB - - Medium query (10-30 commits): **10-50KB** ← Often exceeds threshold - - Large query (100+ commits): 100KB-1MB - -3. **Inaccessible Payload Path**: When a response exceeded 10KB: - - The middleware would save the payload to disk at an **absolute host path**: `/tmp/jq-payloads/{sessionID}/{queryID}/payload.json` - - The middleware would return metadata with `payloadPath` pointing to this host path - - The agent would try to read this path, but it **doesn't exist in the agent's container filesystem** - - The agent would see `FILE_NOT_FOUND` and retry the tool call - - This created an infinite loop - -## Example from Issue #18295 - -From the user's logs: -``` -github-list_commits - └ {"agentInstructions":"The payload was too large for an MCP response. The comp... - -✗ bash: cat /tmp/gh-aw/mcp-payloads/***/a47e03f1b3561c858a06b84d5e02eb38/payload.json 2>/dev/null || echo "FILE_NOT_FOUND" - "description": Required -``` - -The agent received: -- `agentInstructions`: "The payload was too large..." -- `payloadPath`: `/tmp/jq-payloads/{session}/{query}/payload.json` - -Then tried to read the file, got `FILE_NOT_FOUND`, and retried the tool call. - -## Solution - -**Increase the default payload size threshold from 10KB to 512KB (524,288 bytes)** - -This ensures that: -1. Typical GitHub API responses (10-50KB) are returned **inline** without disk storage -2. Only truly large responses (>512KB) trigger the payload-to-disk mechanism -3. Agents don't encounter the inaccessible file path issue for normal operations -4. The threshold can still be overridden via: - - Environment variable: `MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD=` - - Command-line flag: `--payload-size-threshold ` - - Config file: `payload_size_threshold = ` - -## Changes Made - -### Code Changes - -1. **internal/config/config_payload.go** - - Changed `DefaultPayloadSizeThreshold` from `10240` to `524288` - - Updated comment to explain rationale - -2. **internal/cmd/flags_logging.go** - - Changed `defaultPayloadSizeThreshold` from `10240` to `524288` - - Updated comment - -3. **internal/config/config_core.go** - - Updated comment from "10KB" to "512KB" - -4. **internal/cmd/flags_logging_test.go** - - Updated test assertion from `10240` to `524288` - -### Documentation Changes - -1. **README.md** - - Updated CLI flag default from `10240` to `524288` - - Updated environment variable table default from `10240` to `524288` - - Updated configuration alternative default from `10240` to `524288` - -2. **config.example-payload-threshold.toml** - - Updated default from `10240` to `524288` - - Updated examples to use larger, more realistic values: - - 256KB (more aggressive storage) - - 512KB (default) - - 1MB (minimize disk storage) - -## Testing - -All tests pass with the new default: -- Unit tests: ✅ PASS (all packages) -- Integration tests: ✅ PASS -- Configuration tests: ✅ PASS - -## Impact - -### Before Fix (10KB threshold) -- GitHub `list_commits` responses frequently exceeded threshold -- Agents got stuck in infinite loops trying to read inaccessible files -- Workflows would timeout after repeatedly calling the same tool -- Poor user experience - -### After Fix (512KB threshold) -- GitHub `list_commits` responses are returned inline (typical size 10-50KB) -- Agents receive complete data without file system access issues -- No more infinite loops for typical use cases -- Greatly improved user experience - -### Performance Considerations - -**Memory Impact**: Minimal. Most responses are still well under 512KB. - -**Network Impact**: Reduced. Returning data inline is faster than writing to disk and returning metadata. - -**Disk I/O Impact**: Significantly reduced. Fewer responses trigger disk storage. - -## Configuration Options - -Users can still customize the threshold for their specific needs: - -```bash -# Lower threshold (more aggressive disk storage) -MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD=262144 ./awmg --config config.toml - -# Higher threshold (minimize disk storage) -MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD=1048576 ./awmg --config config.toml - -# Or via config file -[gateway] -payload_size_threshold = 524288 -``` - -## Related Issue - -GitHub Issue: https://github.com/github/gh-aw/issues/18295 - -## Backward Compatibility - -✅ **Fully backward compatible** - -- Existing configurations continue to work -- Environment variable and CLI flag still functional -- Users can explicitly set the old 10KB threshold if desired -- New default is a **quality-of-life improvement** that makes the gateway work better out-of-the-box - -## Future Considerations - -If payload path accessibility issues persist for responses >512KB: - -1. Consider adding a mount configuration to make payload paths accessible to agents -2. Consider adding a flag to return large payloads inline (disable disk storage) -3. Consider implementing payload compression to reduce size before threshold check -4. Consider per-tool threshold configuration for tools known to return large responses - -## Summary - -The fix addresses the root cause of the infinite loop issue by ensuring that typical MCP tool responses are small enough to be returned inline, avoiding the inaccessible file path problem entirely. The threshold remains configurable for advanced use cases, maintaining flexibility while providing sensible defaults. diff --git a/PAYLOAD_THRESHOLD_IMPLEMENTATION.md b/PAYLOAD_THRESHOLD_IMPLEMENTATION.md deleted file mode 100644 index 1ecaf966..00000000 --- a/PAYLOAD_THRESHOLD_IMPLEMENTATION.md +++ /dev/null @@ -1,365 +0,0 @@ -# Payload Size Threshold Implementation Summary - -## Overview -Implemented a fully configurable size threshold for the jq middleware to optimize performance by only storing large payloads to disk while returning small payloads inline. The threshold can be configured via config file, environment variable, or command-line flag. - -## Problem Statement -The jq middleware was storing ALL payloads to disk regardless of size, creating: -- Unnecessary file I/O overhead for small responses -- Increased latency for simple tool calls -- Extra filesystem pressure - -## Solution -Added a configurable `payload_size_threshold` (default: 1024 bytes / 1KB) with multiple configuration methods: -- **Config file**: TOML or JSON format -- **Environment variable**: `MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD` -- **Command-line flag**: `--payload-size-threshold` -- **Default**: 1024 bytes (1KB) - -### Configuration Priority (Highest to Lowest) -1. Command-line flag: `--payload-size-threshold 2048` -2. Environment variable: `MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD=2048` -3. Config file: `payload_size_threshold = 2048` -4. Default: `1024 bytes` - -### Behavior -- **Payloads ≤ threshold**: Returned inline (no file storage, no transformation) -- **Payloads > threshold**: Stored to disk with metadata response - -## Configuration - -### Command-Line Flag -```bash -# Set threshold to 2KB -./awmg --config config.toml --payload-size-threshold 2048 - -# Set threshold to 512 bytes -./awmg --config config.toml --payload-size-threshold 512 - -# View help -./awmg --help | grep payload-size-threshold -``` - -### Environment Variable -```bash -# Set threshold via environment -export MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD=2048 -./awmg --config config.toml - -# One-liner -MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD=4096 ./awmg --config config.toml -``` - -### TOML Format -```toml -[gateway] -payload_dir = "/tmp/jq-payloads" -payload_size_threshold = 1024 # 1KB default -``` - -### JSON Format -```json -{ - "gateway": { - "payloadDir": "/tmp/jq-payloads", - "payloadSizeThreshold": 1024 - } -} -``` - -## Implementation Details - -### Files Modified -1. **internal/config/config_core.go** - - Added `PayloadSizeThreshold` field to `GatewayConfig` - -2. **internal/config/config_payload.go** - - Added `DefaultPayloadSizeThreshold` constant (1024 bytes) - - Updated config initialization to set default - -3. **internal/middleware/jqschema.go** - - Modified `WrapToolHandler` signature to accept `sizeThreshold` parameter - - Added size check logic before file storage - - Returns original data if size ≤ threshold - - Stores to disk and returns metadata if size > threshold - -4. **internal/server/unified.go** - - Added `payloadSizeThreshold` field to `UnifiedServer` - - Added `GetPayloadSizeThreshold()` public getter method - - Updated middleware wrapper call to pass threshold - -5. **internal/cmd/flags_logging.go** ✨ NEW - - Added `--payload-size-threshold` command-line flag - - Added `MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD` environment variable support - - Implemented `getDefaultPayloadSizeThreshold()` with validation - - Invalid values (negative, zero, non-numeric) fall back to default - -6. **internal/cmd/root.go** ✨ NEW - - Apply flag/env overrides to config after loading - - Priority: CLI flag > Env var > Config > Default - -7. **internal/cmd/flags_logging_test.go** ✨ NEW - - 10+ comprehensive tests for flag and env var behavior - - Tests default values, valid inputs, invalid inputs - - Tests override priority - -### Test Coverage -Created comprehensive test suite with 50+ tests total: - -**Middleware Tests** (41 tests): -- Small payloads returned inline -- Large payloads use disk storage -- Exact boundary conditions -- Custom threshold configurations -- Mixed payload scenarios - -**CLI/Env Tests** (10 tests): -- Default value (no env var) -- Valid environment variables (512, 2048, 10240) -- Invalid values (non-numeric, negative, zero) -- Environment variable override -- Flag default behavior - -All tests pass with 100% success rate ✅ - -## Public API - -### Accessing the Threshold -```go -// Get threshold from UnifiedServer instance -threshold := server.GetPayloadSizeThreshold() -``` - -### Command-Line Help -```bash -$ ./awmg --help | grep -A1 payload-size-threshold - --payload-size-threshold int Size threshold (in bytes) for storing payloads to disk. - Payloads larger than this are stored, smaller ones returned inline - (default 1024) -``` - -## Performance Impact - -### Before (All Payloads Stored) -- Every tool response → file I/O -- Small 50-byte responses: ~1-2ms file overhead -- 1000 small requests: ~1-2 seconds overhead - -### After (Threshold-Based) -- Small responses (≤1KB): No file I/O, ~0.01ms -- Large responses (>1KB): File I/O, ~1-2ms -- 1000 small requests: ~10ms overhead (200x improvement) - -## Common Threshold Values - -| Threshold | Use Case | File I/O Frequency | Command | -|-----------|----------|-------------------|---------| -| 512 bytes | Aggressive storage | High - most payloads stored | `--payload-size-threshold 512` | -| 1024 bytes | Default (balanced) | Medium - typical responses inline | Default or `--payload-size-threshold 1024` | -| 2048 bytes | Minimal storage | Low - only large responses stored | `--payload-size-threshold 2048` | -| 10240 bytes | Development/testing | Very low - almost everything inline | `--payload-size-threshold 10240` | - -## Environment Variables Reference - -| Variable | Description | Default | -|----------|-------------|---------| -| `MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD` | Size threshold in bytes for payload storage | `1024` | -| `MCP_GATEWAY_PAYLOAD_DIR` | Directory for storing large payloads | `/tmp/jq-payloads` | - -## Migration Path - -### Backward Compatibility -- Default threshold (1024 bytes) maintains similar behavior for most use cases -- Existing configurations work without changes -- Payloads >1KB still stored to disk as before -- New behavior: small payloads now returned inline (performance improvement) -- CLI flag and env var are optional - no breaking changes - -### Upgrading -1. No configuration changes required (uses default 1024 bytes) -2. Optional: Tune threshold via CLI flag for quick testing -3. Optional: Set environment variable for deployment-wide configuration -4. Optional: Add to config file for permanent setting -5. Optional: Monitor payload sizes and adjust threshold - -## Testing Verification - -```bash -# Run all tests -make test-unit - -# Run middleware tests specifically -go test ./internal/middleware/... -v - -# Run CLI flag tests -go test ./internal/cmd/... -v -run "TestPayloadSizeThreshold" - -# Run complete verification -make agent-finished -``` - -## Documentation -- README.md updated with flag, env var, and configuration examples -- config.example-payload-threshold.toml created with all configuration methods -- Gateway Configuration Fields section updated -- Environment Variables table updated -- Command-line flags help output updated - -## Usage Examples - -### Quick Testing -```bash -# Try different thresholds without editing config -./awmg --config config.toml --payload-size-threshold 512 # Aggressive -./awmg --config config.toml --payload-size-threshold 2048 # Relaxed -./awmg --config config.toml --payload-size-threshold 10240 # Minimal -``` - -### Production Deployment -```bash -# Set via environment variable in systemd service -Environment="MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD=2048" - -# Set via Docker environment -docker run -e MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD=2048 ghcr.io/github/gh-aw-mcpg - -# Set via config file (permanent) -# Edit config.toml: -[gateway] -payload_size_threshold = 2048 -``` - -## Future Enhancements - -Potential improvements for future iterations: -1. Add metrics for inline vs. disk storage rates -2. Add per-tool threshold configuration -3. Add dynamic threshold adjustment based on load -4. Add payload size distribution logging -5. Add compression for stored payloads - -## References -- Original Issue: "The jq middleware is sharing all payloads through payloadDir instead only large ones" -- New Requirement 1: "Store the max payload size in a variable that can be accessed by other modules" -- New Requirement 2: "Make the payload size threshold configurable through a command line flag and environment variable" -- Default threshold: 1024 bytes (1KB) -- Files: `internal/middleware/jqschema.go`, `internal/config/config_payload.go`, `internal/cmd/flags_logging.go` -- Tests: `internal/middleware/jqschema_test.go`, `internal/cmd/flags_logging_test.go` - -## Implementation Details - -### Files Modified -1. **internal/config/config_core.go** - - Added `PayloadSizeThreshold` field to `GatewayConfig` - -2. **internal/config/config_payload.go** - - Added `DefaultPayloadSizeThreshold` constant (1024 bytes) - - Updated config initialization to set default - -3. **internal/middleware/jqschema.go** - - Modified `WrapToolHandler` signature to accept `sizeThreshold` parameter - - Added size check logic before file storage - - Returns original data if size ≤ threshold - - Stores to disk and returns metadata if size > threshold - -4. **internal/server/unified.go** - - Added `payloadSizeThreshold` field to `UnifiedServer` - - Added `GetPayloadSizeThreshold()` public getter method - - Updated middleware wrapper call to pass threshold - -### Test Coverage -Created comprehensive test suite with 41+ tests: -- `TestPayloadSizeThreshold_SmallPayload` - Verifies inline return for small payloads -- `TestPayloadSizeThreshold_LargePayload` - Verifies disk storage for large payloads -- `TestPayloadSizeThreshold_ExactBoundary` - Tests exact threshold boundaries -- `TestPayloadSizeThreshold_CustomThreshold` - Tests various threshold values -- `TestThresholdBehavior_SmallPayloadsAsIs` - Multiple small payload scenarios -- `TestThresholdBehavior_LargePayloadsUsePayloadDir` - Multiple large payload scenarios -- `TestThresholdBehavior_MixedPayloads` - Same handler with mixed sizes -- `TestThresholdBehavior_ConfigurableThresholds` - Threshold configuration impact - -All tests pass with 100% success rate. - -## Public API - -### Accessing the Threshold -```go -// Get threshold from UnifiedServer instance -threshold := server.GetPayloadSizeThreshold() -``` - -This allows other modules to: -- Display current configuration -- Make decisions based on threshold -- Log configuration values -- Implement monitoring/metrics - -## Performance Impact - -### Before (All Payloads Stored) -- Every tool response → file I/O -- Small 50-byte responses: ~1-2ms file overhead -- 1000 small requests: ~1-2 seconds overhead - -### After (Threshold-Based) -- Small responses (≤1KB): No file I/O, ~0.01ms -- Large responses (>1KB): File I/O, ~1-2ms -- 1000 small requests: ~10ms overhead (200x improvement) - -## Common Threshold Values - -| Threshold | Use Case | File I/O Frequency | -|-----------|----------|-------------------| -| 512 bytes | Aggressive storage | High - most payloads stored | -| 1024 bytes | Default (balanced) | Medium - typical tool responses inline | -| 2048 bytes | Minimal storage | Low - only large responses stored | -| 10240 bytes | Development/testing | Very low - almost everything inline | - -## Migration Path - -### Backward Compatibility -- Default threshold (1024 bytes) maintains similar behavior for most use cases -- Existing configurations work without changes -- Payloads >1KB still stored to disk as before -- New behavior: small payloads now returned inline (performance improvement) - -### Upgrading -1. No configuration changes required (uses default 1024 bytes) -2. Optional: Tune threshold based on your workload -3. Optional: Monitor payload sizes and adjust threshold - -## Testing Verification - -```bash -# Run all tests -make test-unit - -# Run middleware tests specifically -go test ./internal/middleware/... -v - -# Run threshold behavior tests -go test ./internal/middleware/... -v -run "TestThresholdBehavior" - -# Run complete verification -make agent-finished -``` - -## Documentation -- README.md updated with configuration examples -- config.example-payload-threshold.toml created with detailed comments -- Gateway Configuration Fields section updated - -## Future Enhancements - -Potential improvements for future iterations: -1. Add metrics for inline vs. disk storage rates -2. Add environment variable override for threshold -3. Add per-tool threshold configuration -4. Add dynamic threshold adjustment based on load -5. Add payload size distribution logging - -## References -- Issue: "The jq middleware is sharing all payloads through payloadDir instead only large ones" -- Default threshold: 1024 bytes (1KB) -- Files: `internal/middleware/jqschema.go`, `internal/config/config_payload.go` -- Tests: `internal/middleware/jqschema_test.go` diff --git a/README.md b/README.md index 04a5852e..9f22b67a 100644 --- a/README.md +++ b/README.md @@ -163,466 +163,71 @@ For the complete JSON configuration specification with all validation rules, see } ``` -#### Server Configuration Fields - -- **`type`** (optional): Server transport type - - `"stdio"` - Standard input/output transport (default) - - `"http"` - HTTP transport (fully supported) - - `"local"` - Alias for `"stdio"` (backward compatibility) - -- **`container`** (required for stdio in JSON format): Docker container image (e.g., `"ghcr.io/github/github-mcp-server:latest"`) - - Automatically wraps as `docker run --rm -i ` - - **Note**: The `command` field is NOT supported in JSON stdin format (stdio servers must use `container` instead) - - **TOML format uses `command` and `args` fields - `command` must be `"docker"` for stdio servers** - -- **`entrypoint`** (optional): Custom entrypoint for the container - - Overrides the default container entrypoint - - Applied as `--entrypoint` flag to Docker - -- **`entrypointArgs`** (optional): Arguments passed to container entrypoint - - Array of strings passed after the container image - -- **`args`** (optional): Additional Docker runtime arguments inserted before the container image name - - Array of strings passed to `docker run` before the container image - - Example: `["--network", "host", "--privileged"]` - - Useful for advanced Docker configurations - -- **`mounts`** (optional): Volume mounts for the container - - Array of strings in format `"source:dest:mode"` - - `source` - Host path to mount (can use environment variables with `${VAR}` syntax) - - `dest` - Container path where the volume is mounted - - `mode` - Either `"ro"` (read-only) or `"rw"` (read-write) - - Example: `["/host/config:/app/config:ro", "/host/data:/app/data:rw"]` - -- **`env`** (optional): Environment variables - - Set to `""` (empty string) for passthrough from host environment - - Set to `"value"` for explicit value - - Use `"${VAR_NAME}"` for environment variable expansion (fails if undefined) - -- **`url`** (required for http): HTTP endpoint URL for `type: "http"` servers - -- **`headers`** (optional): HTTP headers to include in requests (for `type: "http"` servers) - - Map of header name to value (e.g., `{"Authorization": "Bearer token"}`) - -- **`tools`** (optional): List of tool names to expose from this server - - If omitted or empty, all tools are exposed - - Example: `["get_file_contents", "search_code"]` - -- **`registry`** (optional): Informational URI to the server's entry in an MCP registry - - Used for documentation and discoverability purposes only; not used at runtime - -- **`guard`** (optional): Name of the guard to use for this server (DIFC) - - References a guard defined in the top-level `[guards]` section - - Enables per-server DIFC guard assignment independent of `guard-policies` - - Example: `guard = "github"` (uses the guard named `github` from `[guards.github]`) - -- **`working_directory`** (optional, TOML format only): Working directory for the server process - - **Note**: This field is parsed and stored but not yet implemented in the launcher; it has no runtime effect currently - -- **`guard-policies`** (optional): Guard policies for access control at the MCP gateway level - - **`allow-only`**: Restricts which repositories a guard allows (used for GitHub MCP server) - - **`write-sink`**: Marks a server as a write-only output channel that accepts writes from agents with matching secrecy labels (used for safe-outputs, buffered update servers) - - A server's guard-policies must contain **either** `allow-only` **or** `write-sink`, not both. - - ##### allow-only (GitHub MCP server) - - Controls repository access with the following structure: - ```json - "guard-policies": { - "allow-only": { - "repos": ["github/gh-aw-mcpg", "github/gh-aw"], - "min-integrity": "unapproved" - } - } - ``` - TOML equivalent: - ```toml - [servers.github.guard_policies.allow-only] - repos = ["github/gh-aw-mcpg", "github/gh-aw"] - min-integrity = "unapproved" - ``` - - **`repos`**: Repository access scope - - `"all"` - All repositories accessible by the token - - `"public"` - Public repositories only - - Array of patterns: - - `"owner/repo"` - Exact repository match - - `"owner/*"` - All repositories under owner - - `"owner/prefix*"` - Repositories with name prefix under owner - - **`min-integrity`**: Minimum integrity level required. Integrity levels are determined by the GitHub MCP server based on the `author_association` field of GitHub objects and whether the object is reachable from the main branch: - - `"none"` - No integrity requirements (includes objects with author_association: FIRST_TIME_CONTRIBUTOR, FIRST_TIMER, NONE) - - `"unapproved"` - Unapproved contributor level (includes objects with author_association: CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR) - - `"approved"` - Approved contributor level (includes objects with author_association: OWNER, MEMBER, COLLABORATOR) - - `"merged"` - Merged to main branch (any object reachable from the main branch, regardless of authorship) - - **Meaning**: Restricts the GitHub MCP server to only access specified repositories - - Tools like `get_file_contents`, `search_code`, etc. will only work on allowed repositories - - Attempts to access other repositories will be denied by the guard policy - - ##### write-sink (output servers) - - Marks a server as a write-only output channel. **Write-sink is required for ALL output - servers** (e.g., `safeoutputs`) when DIFC guards are enabled on any other server. Without - it, the output server gets a noop guard that classifies operations as reads with empty - labels, causing integrity violations when the agent has integrity tags from other guards. - - When an agent reads from a guarded server (e.g., GitHub with `allow-only`), it acquires - secrecy and integrity labels. The write-sink guard solves this by classifying all - operations as writes and accepting writes from agents whose secrecy labels match the - configured `accept` patterns. - - For exact repos (`repos=["owner/repo1", "owner/repo2"]`): - ```json - "guard-policies": { - "write-sink": { - "accept": ["private:owner/repo1", "private:owner/repo2"] - } - } - ``` +For complete field-by-field reference, see **[docs/CONFIGURATION.md](docs/CONFIGURATION.md)**. - For prefix wildcard repos (`repos=["owner/prefix*"]`): - ```json - "guard-policies": { - "write-sink": { - "accept": ["private:owner/prefix*"] - } - } - ``` +Key server fields: `type` (stdio/http), `container` (Docker image), `env` (environment variables with `${VAR}` expansion), `url` (HTTP endpoint), `tools` (tool filter list), `guard` (DIFC guard name), `guard-policies` (access control). - For broad access (`repos="all"` or `repos="public"`): - ```json - "guard-policies": { - "write-sink": { - "accept": ["*"] - } - } - ``` - - TOML equivalents: - ```toml - # Exact repos - [servers.safeoutputs.guard_policies.write-sink] - Accept = ["private:owner/repo1", "private:owner/repo2"] - - # Prefix wildcard repos - [servers.safeoutputs.guard_policies.write-sink] - Accept = ["private:owner/prefix*"] - - # Broad access (repos="all" or repos="public") - [servers.safeoutputs.guard_policies.write-sink] - Accept = ["*"] - ``` - - **`accept`**: Array of secrecy tags the sink accepts (exact string match against agent secrecy tags — not glob patterns) - - `"*"` - **Wildcard**: Accept writes from agents with any secrecy (must be the sole entry). Use for `repos="all"` or `repos="public"`. - - `"private:owner/repo"` - Matches agent secrecy tag from `repos=["owner/repo"]` (exact repo) - - `"private:owner/prefix*"` - Matches agent secrecy tag from `repos=["owner/prefix*"]` (prefix wildcard — the `*` is a literal character in the tag) - - `"private:owner"` - Matches agent secrecy tag from `repos=["owner/*"]` (owner wildcard — bare owner, no `/*` suffix) - - `"public:owner/repo*"` - Matches agent secrecy tag for public repos matching a prefix - - `"internal:owner/repo*"` - Matches agent secrecy tag for internal repos matching a prefix - - **How it works**: The write-sink classifies all operations as writes. For DIFC write checks: - - Resource secrecy is set to the `accept` patterns → agent secrecy ⊆ resource secrecy passes - - Resource integrity is left empty → no integrity requirements for writes - - **When to use**: Required for **all** output servers (`safeoutputs`, etc.) when DIFC guards are enabled on any server in the configuration - - ##### Mapping allow-only repos to write-sink accept - - The write-sink `accept` entries must match the secrecy tags the GitHub guard assigns - to the agent via `label_agent`. The mapping depends on the `repos` configuration: - - | `allow-only.repos` | Agent secrecy tags | `write-sink.accept` | - |---|---|---| - | `"all"` | `[]` (none) | `["*"]` (wildcard) | - | `"public"` | `[]` (none) | `["*"]` (wildcard) | - | `["owner/repo"]` | `["private:owner/repo"]` | `["private:owner/repo"]` | - | `["owner/*"]` | `["private:owner"]` | `["private:owner"]` | - | `["owner/prefix*"]` | `["private:owner/prefix*"]` | `["private:owner/prefix*"]` | - | `["O/R1", "O/R2"]` | `["private:O/R1", "private:O/R2"]` | `["private:O/R1", "private:O/R2"]` | - | `["O1/*", "O2/R"]` | `["private:O1", "private:O2/R"]` | `["private:O1", "private:O2/R"]` | - - **Key rules**: - - `repos="all"` or `repos="public"` → no secrecy tags → use `accept: ["*"]` (wildcard) - - Write-sink is **required for ALL output servers** when DIFC guards are enabled (prevents noop guard integrity violations) - - `accept: ["*"]` is a special wildcard that accepts writes from agents with any secrecy; it must be the sole entry - - `repos=["owner/*"]` (owner wildcard) → bare owner tag `"private:owner"` (no `/*` suffix) - - `repos=["owner/prefix*"]` (prefix wildcard) → `"private:owner/prefix*"` (suffix preserved) - - `repos=["owner/repo"]` (exact) → `"private:owner/repo"` - - Multi-entry repos produce one tag per entry; `accept` must include all of them - - `accept` can be a superset of the agent's secrecy (extra entries are harmless) - - `min-integrity` does not affect these rules (it only changes integrity labels) - - - For **other MCP servers** (Jira, WorkIQ, etc.), different policy schemas apply - - JSON format uses `"guard-policies"` (with hyphen), TOML uses `guard_policies` (with underscore) +##### guard-policies -#### Custom Schemas (`customSchemas`) +- **`allow-only`**: Restricts repository access for GitHub MCP servers — configures `repos` (scope) and `min-integrity` (none/unapproved/approved/merged) +- **`write-sink`**: Required for ALL output servers when DIFC guards are enabled — configures `accept` patterns matching agent secrecy tags -The `customSchemas` top-level field allows you to define custom server types beyond the built-in `"stdio"` and `"http"` types. Each custom type maps to an HTTPS schema URL that describes its configuration format. +See **[docs/CONFIGURATION.md](docs/CONFIGURATION.md)** for guard-policy details including the allow-only → write-sink accept mapping table. -```json -{ - "customSchemas": { - "myCustomType": "https://example.com/schemas/my-custom-type.json" - }, - "mcpServers": { - "myServer": { - "type": "myCustomType" - } - } -} -``` +#### Custom Schemas (`customSchemas`) -**Validation Rules for `customSchemas`:** -- Custom type names must not conflict with reserved types (`stdio`, `http`) -- Schema URLs must use `https://` (HTTP URLs are not permitted) -- If a server's `type` references a custom type not listed in `customSchemas`, validation fails with a helpful error message - -**Validation Rules:** - -- **JSON stdin format**: - - **Stdio servers** must specify `container` (required) - - **HTTP servers** must specify `url` (required) - - **The `command` field is not supported** - stdio servers must use `container` -- **TOML format**: - - Uses `command` and `args` fields directly (e.g., `command = "docker"`) -- **Common rules** (both formats): - - Empty/"local" type automatically normalized to "stdio" - - Variable expansion with `${VAR_NAME}` fails fast on undefined variables - - All validation errors include JSONPath and helpful suggestions - - **Mount specifications** must follow `"source:dest:mode"` format - - `source` must be an absolute path (e.g., `/host/data`) - - `dest` must be an absolute path (e.g., `/app/data`) - - `mode` must be either `"ro"` or `"rw"` - - Both source and destination paths are required (cannot be empty) - -See **[Configuration Specification](https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/mcp-gateway.md)** for complete validation rules. +Define custom server types beyond `"stdio"` and `"http"` by mapping type names to HTTPS schema URLs. See **[docs/CONFIGURATION.md](docs/CONFIGURATION.md)** for details. #### Gateway Configuration Fields (Reserved) -- **`port`** (optional): Gateway HTTP port (default: from `--listen` flag) - - Valid range: 1-65535 -- **`apiKey`** (optional): API key for authentication -- **`domain`** (optional): Domain name for the gateway - - Allowed values: `"localhost"`, `"host.docker.internal"`, or a variable expression (e.g., `"${MCP_GATEWAY_DOMAIN}"`) - - **Note**: Only **uppercase** variable names are accepted in variable expressions (e.g., `"${MY_VAR}"` is valid, `"${my_var}"` is not) -- **`startupTimeout`** (optional): Seconds to wait for backend startup (default: 60) - - Must be positive integer -- **`toolTimeout`** (optional): Seconds to wait for tool execution (default: 120) - - Must be positive integer -- **`payloadDir`** (optional): Directory for storing large payload files (default: `/tmp/jq-payloads`) - - Payloads are organized by session: `{payloadDir}/{sessionID}/{queryID}/payload.json` - -**Note**: Gateway configuration fields are validated and parsed but not yet fully implemented. - -**Configuration Alternatives**: -- **`payloadSizeThreshold`** is not supported in JSON stdin format. Use: - - CLI flag: `--payload-size-threshold ` (default: 524288) - - Environment variable: `MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD=` - - TOML config file: `payload_size_threshold = ` in `[gateway]` section - - Payloads **larger** than this threshold are stored to disk and return metadata - - Payloads **smaller than or equal** to this threshold are returned inline -- **`payloadPathPrefix`** is not supported in JSON stdin format. Use: - - CLI flag: `--payload-path-prefix ` (default: empty - use actual filesystem path) - - Environment variable: `MCP_GATEWAY_PAYLOAD_PATH_PREFIX=` - - TOML config file: `payload_path_prefix = ""` in `[gateway]` section - - When set, the `payloadPath` returned to clients uses this prefix instead of the actual filesystem path - - Example: Gateway saves to `/tmp/jq-payloads/session/query/payload.json`, but returns `/workspace/payloads/session/query/payload.json` to clients if `payload_path_prefix = "/workspace/payloads"` - - This allows agents running in containers to access payload files via mounted volumes -- **`sequential_launch`** is only supported in TOML config and CLI. Use: - - CLI flag: `--sequential-launch` (default: parallel launch) - - TOML config file: `sequential_launch = true` at the **top level** (outside of any `[section]`) - - When enabled, MCP servers are started one-by-one instead of in parallel; useful when servers have ordering dependencies or for debugging startup issues - - Example (top-level TOML placement): - ```toml - sequential_launch = true - - [gateway] - port = 3000 - - [servers.github] - command = "docker" - # ... - ``` -- **`guards_mode`** is only supported in TOML config and CLI. Use: - - CLI flag: `--guards-mode ` (default: `strict`) - - Environment variable: `MCP_GATEWAY_GUARDS_MODE=` - - TOML config file: `guards_mode = "strict"` at the **top level** (outside of any `[section]`) - - Valid values: `strict` (deny violations), `filter` (silently remove denied tools/resources), `propagate` (auto-adjust agent labels on reads to allow access) - - Example (top-level TOML placement): - ```toml - guards_mode = "filter" - - [gateway] - port = 3000 - ``` - -**Environment Variable Features**: -- **Passthrough**: Set value to empty string (`""`) to pass through from host -- **Expansion**: Use `${VAR_NAME}` syntax for dynamic substitution (fails if undefined) - -### Configuration Validation and Error Handling - -MCP Gateway provides detailed error messages and validation to help catch configuration issues early: - -#### Parse Errors with Precise Location - -When there's a syntax error in your TOML configuration, the gateway reports the exact line and column: - -```bash -$ awmg --config config.toml -Error: failed to parse TOML at line 2, column 6: expected '.' or '=', but got '3' instead -``` - -This helps you quickly identify and fix syntax issues. - -#### Unknown Key Detection (Typo Detection) - -The gateway detects and warns about unknown configuration keys, helping catch typos and deprecated options: - -```toml -[gateway] -prot = 3000 # Typo: should be 'port' -startup_timout = 30 # Typo: should be 'startup_timeout' -``` - -When you run the gateway with these typos, you'll see warnings in the log file: - -``` -[2026-02-07T17:46:51Z] [WARN] [config] Unknown configuration key 'gateway.prot' - check for typos or deprecated options -[2026-02-07T17:46:51Z] [WARN] [config] Unknown configuration key 'gateway.startup_timout' - check for typos or deprecated options -``` - -The gateway will use default values for unrecognized keys, so it will still start, but the warnings help you identify and fix configuration issues. - -#### Memory-Efficient Parsing +| Field | Description | Default | +|-------|-------------|---------| +| `port` | HTTP port (1-65535) | From `--listen` flag | +| `apiKey` | API key for authentication | (disabled) | +| `domain` | Gateway domain | `localhost` | +| `startupTimeout` | Backend startup timeout (seconds) | `60` | +| `toolTimeout` | Tool execution timeout (seconds) | `120` | +| `payloadDir` | Large payload storage directory | `/tmp/jq-payloads` | -The gateway uses streaming parsing for configuration files, making it efficient even with large configuration files containing many servers. +See **[docs/CONFIGURATION.md](docs/CONFIGURATION.md)** for TOML-only/CLI-only options and variable expansion features. -#### Best Practices +### Configuration Validation -1. **Check logs for warnings**: After starting the gateway, check the log file for any warnings about unknown keys -2. **Use precise error messages**: When you see a parse error, the line and column numbers point exactly to the problem -3. **Validate configuration**: Test your configuration changes by running the gateway and checking for warnings -4. **Keep configuration clean**: Remove any deprecated or unused configuration options +The gateway provides fail-fast validation with precise error locations (line/column for TOML parse errors), unknown key detection (catches typos like `prot` instead of `port`), and environment variable expansion validation. Check log files for warnings after startup. ## Usage -``` -MCPG is a proxy server for Model Context Protocol (MCP) servers. -It provides routing, aggregation, and management of multiple MCP backend servers. - -Usage: - awmg [flags] - awmg [command] - -Available Commands: - completion Generate completion script - help Help about any command - -Flags: - --allowonly-min-integrity string AllowOnly integrity: none|unapproved|approved|merged - --allowonly-scope-owner string AllowOnly owner scope value - --allowonly-scope-public Use public AllowOnly scope - --allowonly-scope-repo string AllowOnly repo name (requires owner) - -c, --config string Path to config file - --config-stdin Read MCP server configuration from stdin (JSON format). When enabled, overrides --config - --env string Path to .env file to load environment variables - --guard-policy-json string Guard policy JSON (e.g. {"allow-only":{"repos":"public","min-integrity":"none"}}) - --guards-mode string Guards enforcement mode: strict (deny violations), filter (remove denied tools), or propagate (auto-adjust agent labels on reads) (default "strict") - --guards-sink-server-ids string Comma-separated server IDs whose RPC JSONL logs should include agent secrecy/integrity tag snapshots - -h, --help help for awmg - -l, --listen string HTTP server listen address (default "127.0.0.1:3000") - --log-dir string Directory for log files (falls back to stdout if directory cannot be created) (default "/tmp/gh-aw/mcp-logs") - --payload-dir string Directory for storing large payload files (segmented by session ID) (default "/tmp/jq-payloads") - --payload-path-prefix string Path prefix to use when returning payloadPath to clients (allows remapping host paths to client/agent container paths) - --payload-size-threshold int Size threshold (in bytes) for storing payloads to disk. Payloads larger than this are stored, smaller ones returned inline (default 524288) - --routed Run in routed mode (each backend at /mcp/) - --sequential-launch Launch MCP servers sequentially during startup (parallel launch is default) - --unified Run in unified mode (all backends at /mcp) - --validate-env Validate execution environment (Docker, env vars) before starting - -v, --verbose count Increase verbosity level (use -v for info, -vv for debug, -vvv for trace) - --version version for awmg - -Use "awmg [command] --help" for more information about a command. +Run `./awmg --help` for full CLI options. Key flags: + +```bash +./awmg --config config.toml # TOML config file +./awmg --config-stdin < config.json # JSON stdin +./awmg --config config.toml --routed # Routed mode (default) +./awmg --config config.toml --unified # Unified mode +./awmg --config config.toml --log-dir /path # Custom log directory ``` ## Environment Variables -The following environment variables are used by the MCP Gateway: - -### Required for Production (Containerized Mode) - -When running in a container (`run_containerized.sh`), these variables **must** be set: - -| Variable | Description | Example | -|----------|-------------|---------| -| `MCP_GATEWAY_PORT` | The port the gateway listens on (used for `--listen` address) | `8080` | -| `MCP_GATEWAY_DOMAIN` | The domain name for the gateway | `localhost` | -| `MCP_GATEWAY_API_KEY` | API key checked by `run_containerized.sh` as a deployment gate; must be referenced in your JSON config via `"${MCP_GATEWAY_API_KEY}"` to enable authentication | `your-secret-key` | - -### Optional (Non-Containerized Mode) +For complete reference, see **[docs/ENVIRONMENT_VARIABLES.md](docs/ENVIRONMENT_VARIABLES.md)**. -When running locally (`run.sh`), these variables are optional (warnings shown if missing): +Key variables: | Variable | Description | Default | |----------|-------------|---------| | `MCP_GATEWAY_PORT` | Gateway listening port | `8000` | -| `MCP_GATEWAY_DOMAIN` | Gateway domain | `localhost` | -| `MCP_GATEWAY_API_KEY` | Informational only — not read directly by the binary; must be referenced in your config via `"${MCP_GATEWAY_API_KEY}"` to enable authentication | (disabled) | -| `MCP_GATEWAY_LOG_DIR` | Log file directory (sets default for `--log-dir` flag) | `/tmp/gh-aw/mcp-logs` | -| `MCP_GATEWAY_PAYLOAD_DIR` | Large payload storage directory (sets default for `--payload-dir` flag) | `/tmp/jq-payloads` | -| `MCP_GATEWAY_PAYLOAD_PATH_PREFIX` | Path prefix for remapping payloadPath returned to clients (sets default for `--payload-path-prefix` flag) | (empty - use actual filesystem path) | -| `MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD` | Size threshold in bytes for payload storage (sets default for `--payload-size-threshold` flag) | `524288` | -| `MCP_GATEWAY_WASM_GUARDS_DIR` | Root directory for per-server WASM guards (`//*.wasm`, first match is loaded) | (disabled) | -| `MCP_GATEWAY_GUARDS_MODE` | Guards enforcement mode: `strict` (deny violations), `filter` (remove denied tools), `propagate` (auto-adjust agent labels) (sets default for `--guards-mode`) | `strict` | -| `MCP_GATEWAY_GUARDS_SINK_SERVER_IDS` | Comma-separated sink server IDs for JSONL guards tag enrichment (sets default for `--guards-sink-server-ids`) | (disabled) | -| `DEBUG` | Enable debug logging with pattern matching (e.g., `*`, `server:*,launcher:*`) | (disabled) | -| `DEBUG_COLORS` | Control colored debug output (0 to disable, auto-disabled when piping) | Auto-detect | -| `RUNNING_IN_CONTAINER` | Manual override; set to `"true"` to force container detection when `/.dockerenv` and cgroup detection are unavailable | (unset) | - -**Note:** `PORT`, `HOST`, and `MODE` are not read by the `awmg` binary directly. However, `run.sh` does use `HOST` (default: `0.0.0.0`) and `MODE` (default: `--routed`) to set the bind address and routing mode. Use the `--listen` and `--routed`/`--unified` flags when running `awmg` directly. - -### Containerized Deployment Variables - -When using `run_containerized.sh`, these additional variables are available: - -| Variable | Description | Default | -|----------|-------------|---------| -| `MCP_GATEWAY_HOST` | Bind address for the gateway | `0.0.0.0` | -| `MCP_GATEWAY_MODE` | Routing mode flag passed to `awmg` (e.g., `--routed`, `--unified`) | `--routed` | - -### Docker Configuration - -| Variable | Description | Default | -|----------|-------------|---------| -| `DOCKER_HOST` | Docker daemon socket path | `/var/run/docker.sock` | -| `DOCKER_API_VERSION` | Docker API version (set by helper scripts, Docker client auto-negotiates) | Set by querying Docker daemon's current API version; falls back to `1.44` if detection fails | - -### DIFC / Guard Policy Configuration - -These environment variables configure guard policies (e.g., AllowOnly policies for restricting tool access to specific GitHub repositories): - -| Variable | Description | Default | -|----------|-------------|---------| -| `MCP_GATEWAY_GUARD_POLICY_JSON` | Guard policy JSON (e.g., `{"allow-only":{"repos":"public","min-integrity":"none"}}`) (sets default for `--guard-policy-json`) | (disabled) | -| `MCP_GATEWAY_ALLOWONLY_SCOPE_PUBLIC` | Use public AllowOnly scope (sets default for `--allowonly-scope-public`) | `false` | -| `MCP_GATEWAY_ALLOWONLY_SCOPE_OWNER` | AllowOnly owner scope value (sets default for `--allowonly-scope-owner`) | (disabled) | -| `MCP_GATEWAY_ALLOWONLY_SCOPE_REPO` | AllowOnly repo name (requires owner) (sets default for `--allowonly-scope-repo`) | (disabled) | -| `MCP_GATEWAY_ALLOWONLY_MIN_INTEGRITY` | AllowOnly integrity level: `none`, `unapproved`, `approved`, `merged` (sets default for `--allowonly-min-integrity`) | (disabled) | +| `MCP_GATEWAY_API_KEY` | API key (reference in config via `"${MCP_GATEWAY_API_KEY}"`) | (disabled) | +| `MCP_GATEWAY_LOG_DIR` | Log file directory | `/tmp/gh-aw/mcp-logs` | +| `MCP_GATEWAY_PAYLOAD_DIR` | Payload storage directory | `/tmp/jq-payloads` | +| `MCP_GATEWAY_GUARDS_MODE` | DIFC mode: `strict`/`filter`/`propagate` | `strict` | +| `MCP_GATEWAY_WASM_GUARDS_DIR` | WASM guard directory | (disabled) | +| `DEBUG` | Debug logging pattern (e.g., `*`, `server:*`) | (disabled) | ## Containerized Mode -### Running in Docker - -For production deployments in Docker containers, use `run_containerized.sh` which: - -1. **Validates the container environment** before starting -2. **Requires** all essential environment variables -3. **Requires** stdin input (`-i` flag) for JSON configuration -4. **Validates** Docker socket accessibility -5. **Validates** port mapping configuration +For production deployments, use `run_containerized.sh` which validates the environment, requires essential env vars, and checks Docker socket access: ```bash -# Correct way to run the gateway in a container: -# Note: MCP_GATEWAY_API_KEY is checked by run_containerized.sh as a deployment gate. -# For authentication to be active, your config.json must reference it via variable expansion: -# "gateway": { "apiKey": "${MCP_GATEWAY_API_KEY}" } docker run -i \ -e MCP_GATEWAY_PORT=8080 \ -e MCP_GATEWAY_DOMAIN=localhost \ @@ -633,145 +238,25 @@ docker run -i \ ghcr.io/github/gh-aw-mcpg:latest < config.json ``` -**Important flags:** -- `-i`: Required for passing configuration via stdin -- `-v /var/run/docker.sock:/var/run/docker.sock`: Required for spawning backend MCP servers -- `-v /path/to/logs:/tmp/gh-aw/mcp-logs`: Optional but recommended for persistent logs (path matches default or `MCP_GATEWAY_LOG_DIR`) -- `-p :`: Port mapping must match `MCP_GATEWAY_PORT` - -### Validation Checks - -The containerized startup script performs these validations: - -| Check | Description | Action on Failure | -|-------|-------------|-------------------| -| Docker Socket | Verifies Docker daemon is accessible | Exit with error | -| Environment Variables | Checks required env vars are set | Exit with error | -| Port Mapping | Verifies container port is mapped to host | Exit with error | -| Stdin Interactive | Ensures `-i` flag was used | Exit with error | -| Log Directory Mount | Verifies log directory is mounted to host | Warning (logs won't persist) | - -### Non-Containerized Mode - -For local development, use `run.sh` which: - -1. **Warns** about missing environment variables (but continues) -2. **Provides** default configuration if no config file specified -3. **Auto-detects** containerized environments and redirects to `run_containerized.sh` - -```bash -# Run locally with defaults: -./run.sh - -# Run with custom config: -CONFIG=my-config.toml ./run.sh +Key flags: `-i` (required for stdin config), `-v .../docker.sock` (required for spawning backends), `-p` (must match `MCP_GATEWAY_PORT`). -# Run with environment variables: -MCP_GATEWAY_PORT=3000 ./run.sh -``` +For local development, use `run.sh` which provides defaults and warns about missing env vars. ## Logging -MCPG provides comprehensive logging of all gateway operations to help diagnose issues and monitor activity. - -### Log Files - -The gateway creates multiple log files for different purposes: - -1. **`mcp-gateway.log`** - Unified log with all gateway messages -2. **`{serverID}.log`** - Per-server logs (e.g., `github.log`, `slack.log`) for easier troubleshooting of specific backend servers -3. **`gateway.md`** - Markdown-formatted logs for GitHub workflow previews -4. **`rpc-messages.jsonl`** - Machine-readable JSONL format for RPC message analysis -5. **`tools.json`** - Available tools from all backend MCP servers (mapping server IDs to their tool names and descriptions) - -### Log File Location - -By default, logs are written to `/tmp/gh-aw/mcp-logs/`. This location can be configured using either: - -1. **`MCP_GATEWAY_LOG_DIR` environment variable** - Sets the default log directory -2. **`--log-dir` flag** - Overrides the environment variable and default +The gateway creates log files in the configured log directory (default: `/tmp/gh-aw/mcp-logs`): -The precedence order is: `--log-dir` flag → `MCP_GATEWAY_LOG_DIR` env var → default (`/tmp/gh-aw/mcp-logs`) - -### Per-ServerID Logs - -Each backend MCP server gets its own log file (e.g., `github.log`, `slack.log`) in addition to the unified `mcp-gateway.log`. This makes it much easier to: - -- Debug issues with a specific backend server -- View all activity for one server without filtering -- Identify which server is causing problems -- Troubleshoot server-specific configuration issues - -Example log directory structure: -``` -/tmp/gh-aw/mcp-logs/ -├── mcp-gateway.log # All messages -├── github.log # Only GitHub server logs -├── slack.log # Only Slack server logs -├── notion.log # Only Notion server logs -├── gateway.md # Markdown format -├── rpc-messages.jsonl # RPC messages -└── tools.json # Available tools -``` - -**Using the environment variable:** -```bash -export MCP_GATEWAY_LOG_DIR=/var/log/mcp-gateway -./awmg --config config.toml -``` - -**Using the command-line flag:** -```bash -./awmg --config config.toml --log-dir /var/log/mcp-gateway -``` - -**Important for containerized mode:** Mount the log directory to persist logs outside the container: -```bash -docker run -v /path/on/host:/tmp/gh-aw/mcp-logs ... -``` - -If the log directory cannot be created or accessed, MCPG automatically falls back to logging to stdout. - -### What Gets Logged - -MCPG logs all important gateway events including: - -- **Startup and Shutdown**: Gateway initialization, configuration loading, and graceful shutdown -- **MCP Client Interactions**: Client connection events, request/response details, session management -- **Backend Server Interactions**: Backend server launches, connection establishment, communication events -- **Authentication Events**: Successful authentications and authentication failures (missing/invalid tokens) -- **Connectivity Errors**: Connection failures, timeouts, protocol errors, and command execution issues -- **Debug Information**: Optional detailed debugging via the `DEBUG` environment variable - -### Log Format - -Each log entry includes: -- **Timestamp** (RFC3339 format) -- **Log Level** (INFO, WARN, ERROR, DEBUG) -- **Category** (startup, client, backend, auth, shutdown) -- **Message** with contextual details - -Example log entries: -``` -[2026-01-08T23:00:00Z] [INFO] [startup] Starting MCPG with config: config.toml, listen: 127.0.0.1:3000, log-dir: /tmp/gh-aw/mcp-logs -[2026-01-08T23:00:01Z] [INFO] [backend] Launching MCP backend server: github, command=docker, args=[run --rm -i ghcr.io/github/github-mcp-server:latest] -[2026-01-08T23:00:02Z] [INFO] [client] New MCP client connection, remote=127.0.0.1:54321, method=POST, path=/mcp/github, backend=github, session=abc123 -[2026-01-08T23:00:03Z] [ERROR] [auth] Authentication failed: invalid API key, remote=127.0.0.1:54322, path=/mcp/github -``` +| File | Purpose | +|------|---------| +| `mcp-gateway.log` | Unified log with all messages | +| `{serverID}.log` | Per-server logs (e.g., `github.log`) | +| `gateway.md` | Markdown-formatted logs for workflow previews | +| `rpc-messages.jsonl` | Machine-readable RPC messages | +| `tools.json` | Available tools from all backends | -### Debug Logging +Configure log location with `--log-dir` flag or `MCP_GATEWAY_LOG_DIR` env var. Logs include timestamps, levels (INFO/WARN/ERROR/DEBUG), categories, and contextual details. -For development and troubleshooting, enable debug logging using the `DEBUG` environment variable: - -```bash -# Enable all debug logs -DEBUG=* ./awmg --config config.toml - -# Enable specific categories -DEBUG=server:*,launcher:* ./awmg --config config.toml -``` - -Debug logs are written to stderr and follow the same pattern-matching syntax as the file logger. +For debug logging: `DEBUG=* ./awmg --config config.toml` (supports pattern matching: `DEBUG=server:*,launcher:*`) ## API Endpoints ### Routed Mode (default) @@ -799,127 +284,46 @@ Supported JSON-RPC 2.0 methods: ### Authentication -MCPG implements MCP specification 7.1 for API key authentication. - -**Authentication Header Format:** -- Per MCP spec 7.1: Authorization header MUST contain the API key directly -- Format: `Authorization: ` (plain API key, NOT Bearer scheme) -- Example: `Authorization: my-secret-api-key-123` - -**Configuration:** -- The binary reads the API key from the config (`[gateway] api_key` in TOML, or `"gateway": {"apiKey": "..."}` in JSON stdin) -- To configure via environment variable, reference it in your JSON config using variable expansion: - ```json - { - "gateway": { - "apiKey": "${MCP_GATEWAY_API_KEY}" - } - } - ``` +Per MCP spec 7.1, the gateway uses plain API key authentication: +- Header format: `Authorization: ` (NOT Bearer scheme) +- Configure via `[gateway] api_key` in TOML, or `"gateway": {"apiKey": "${MCP_GATEWAY_API_KEY}"}` in JSON - When configured, all endpoints except `/health` require authentication - When not configured, authentication is disabled -**Implementation:** -- The `internal/auth` package provides centralized authentication logic -- `auth.ParseAuthHeader()` - Parses Authorization headers per MCP spec 7.1 -- `auth.ValidateAPIKey()` - Validates provided API keys -- Backward compatibility for Bearer tokens is maintained - -**Example Request:** -```bash -curl -X POST http://localhost:8000/mcp/github \ - -H "Authorization: my-api-key" \ - -H "Content-Type: application/json" \ - -d '{"jsonrpc": "2.0", "method": "tools/list", "id": 1}' -``` - ### Enhanced Error Debugging -Command failures now include extensive debugging information: - -- Full command, arguments, and environment variables -- Context-specific troubleshooting suggestions: - - Docker daemon connectivity checks - - Container image availability - - Network connectivity issues - - MCP protocol compatibility checks +Command failures include full command details, environment variables, and context-specific troubleshooting suggestions (Docker connectivity, image availability, network issues, MCP compatibility). ## Architecture -This Go port focuses on core MCP proxy functionality with optional security features: +Core MCP proxy with optional DIFC security: -### Core Features (Enabled) - -- ✅ TOML and JSON stdin configuration with spec-compliant validation -- ✅ Environment variable expansion (`${VAR_NAME}`) with fail-fast behavior -- ✅ Stdio transport for backend servers (containerized execution only) -- ✅ HTTP transport for backend servers (session state preserved across requests) -- ✅ Docker container launching -- ✅ Routed and unified modes -- ✅ Basic request/response proxying -- ✅ Enhanced error debugging and troubleshooting +- TOML and JSON stdin configuration with spec-compliant validation +- Environment variable expansion (`${VAR_NAME}`) with fail-fast behavior +- Stdio transport (containerized) and HTTP transport (session state preserved) +- Routed (`/mcp/{serverID}`) and unified (`/mcp`) modes +- Docker container launching for backend servers +- DIFC guard system with WASM-based guards ## MCP Server Compatibility -**The gateway supports MCP servers via stdio transport using Docker containers.** All properly configured MCP servers work through direct stdio connections. - -### Test Results - -Both GitHub MCP and Serena MCP servers pass comprehensive test suites including gateway tests: - -| Server | Transport | Direct Tests | Gateway Tests | Status | -|--------|-----------|--------------|---------------|--------| -| **GitHub MCP** | Stdio (Docker) | ✅ All passed | ✅ All passed | Production ready | -| **Serena MCP** | Stdio (Docker) | ✅ 68/68 passed (100%) | ✅ All passed | Production ready | - -**Configuration:** -```bash -# Both servers use stdio transport via Docker containers -docker run -i ghcr.io/github/github-mcp-server # GitHub MCP -docker run -i ghcr.io/github/serena-mcp-server:latest # Serena MCP -``` - -### Using MCP Servers with the Gateway - -**Direct Connection (Recommended):** -Configure MCP servers to connect directly via stdio transport for optimal performance and full feature support: +The gateway supports MCP servers via stdio transport using Docker containers. Tested with GitHub MCP and Serena MCP servers. ```json { "mcpServers": { - "serena": { - "type": "stdio", - "container": "ghcr.io/github/serena-mcp-server:latest" - }, "github": { "type": "stdio", "container": "ghcr.io/github/github-mcp-server:latest" + }, + "serena": { + "type": "stdio", + "container": "ghcr.io/github/serena-mcp-server:latest" } } } ``` -**Architecture Considerations:** -- The gateway manages backend MCP servers using stdio transport via Docker containers -- Session connection pooling ensures efficient resource usage -- Backend processes are reused across multiple requests per session -- All MCP protocol features are fully supported - -### Test Coverage - -**Serena MCP Server Testing:** -- ✅ **Direct Connection Tests:** 68/68 tests passed (100%) -- ✅ **Gateway Tests:** All tests passed via `make test-serena-gateway` -- ✅ Multi-language support (Go, Java, JavaScript, Python) -- ✅ File operations, symbol operations, memory management -- ✅ Error handling and protocol compliance -- ✅ Detailed results available via `make test-serena-gateway` - -**GitHub MCP Server Testing:** -- ✅ Full test suite validation (direct and gateway) -- ✅ Repository operations, issue management, search functionality -- ✅ Production deployment validated - ## Contributing For development setup, build instructions, testing guidelines, and project architecture details, see [CONTRIBUTING.md](CONTRIBUTING.md). diff --git a/SESSION_PERSISTENCE_ANALYSIS.md b/SESSION_PERSISTENCE_ANALYSIS.md deleted file mode 100644 index 71b7c0bb..00000000 --- a/SESSION_PERSISTENCE_ANALYSIS.md +++ /dev/null @@ -1,292 +0,0 @@ -# Session Persistence Analysis for MCP Gateway - -## Investigation Summary - -This document provides a comprehensive analysis of the MCP Gateway's session persistence implementation in response to issue [language-support] Serena MCP Language Support Cannot Be Tested Through HTTP Gateway. - -## Key Finding - -**The gateway ALREADY fully implements the three recommendations from the issue:** - -1. ✅ Maintain persistent stdio connections to backend servers -2. ✅ Map multiple HTTP requests from the same session (via Authorization header) to the same backend connection -3. ✅ Keep the backend connection alive across multiple HTTP requests - -## Architecture Overview - -### Session Connection Pool - -**Location**: `internal/launcher/connection_pool.go` - -The `SessionConnectionPool` struct provides: -- **Connection Storage**: Maps `(BackendID, SessionID)` tuples to persistent connections -- **Metadata Tracking**: Monitors creation time, last use time, request count, and error count -- **Automatic Lifecycle Management**: - - Idle timeout: 30 minutes - - Error threshold: 10 errors before removal - - Cleanup interval: 5 minutes -- **Thread Safety**: RWMutex protection for concurrent access - -```go -type ConnectionKey struct { - BackendID string - SessionID string -} - -type SessionConnectionPool struct { - connections map[ConnectionKey]*ConnectionMetadata - mu sync.RWMutex - ctx context.Context - idleTimeout time.Duration - cleanupInterval time.Duration - maxErrorCount int -} -``` - -### Session-Aware Launcher - -**Location**: `internal/launcher/launcher.go:178-277` - -The `GetOrLaunchForSession()` function: -- Distinguishes between HTTP (stateless) and stdio (stateful) backends -- For stdio backends: - 1. Checks session pool for existing connection - 2. If not found, launches new backend with proper initialization - 3. Stores connection in pool keyed by `(serverID, sessionID)` -- Handles concurrent access with double-checked locking pattern - -```go -func GetOrLaunchForSession(l *Launcher, serverID, sessionID string) (*mcp.Connection, error) -``` - -### Backend Connection Initialization - -**Location**: `internal/mcp/connection.go:297-408` - -When a new stdio connection is created via `NewConnection()`: -1. Command transport is set up with proper environment variables -2. SDK client's `Connect()` method is called (line 352) -3. The SDK automatically handles the MCP initialization handshake: - - Sends `initialize` request - - Waits for `initialize` response - - Sends `notifications/initialized` notification - -This ensures every backend connection is properly initialized before accepting tool calls. - -### HTTP Session Management - -#### Routed Mode (`/mcp/{serverID}`) - -**Location**: `internal/server/routed.go:111-140` - -Flow for each HTTP request: -1. SDK StreamableHTTP handler callback fires -2. Session ID extracted from Authorization header (`extractAndValidateSession()`) -3. Session ID + Backend ID injected into request context (`injectSessionContext()`) -4. Filtered SDK Server cached per `(backend, session)` pair -5. Tool calls route through unified server handlers → `callBackendTool()` → `GetOrLaunchForSession()` - -```go -routeHandler := sdk.NewStreamableHTTPHandler(func(r *http.Request) *sdk.Server { - sessionID := extractAndValidateSession(r) - *r = *injectSessionContext(r, sessionID, backendID) - return serverCache.getOrCreate(backendID, sessionID, func() *sdk.Server { - return createFilteredServer(unifiedServer, backendID) - }) -}, &sdk.StreamableHTTPOptions{ - Stateless: false, -}) -``` - -#### Unified Mode (`/mcp`) - -**Location**: `internal/server/transport.go:74-109` - -Similar session handling: -- Session ID extracted from Authorization header (line 81) -- Context injection (line 100) -- SDK StreamableHTTP with `Stateless: false` (line 106) -- Session timeout: 30 minutes (line 108) - -### Tool Call Integration - -**Location**: `internal/server/unified.go:650-750` - -All backend tool calls use the session-aware launcher: - -```go -func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName string, args interface{}) { - sessionID := us.getSessionID(ctx) - conn, err := launcher.GetOrLaunchForSession(us.launcher, serverID, sessionID) - // ... make tool call on persistent connection -} -``` - -## How It Works: End-to-End Flow - -### Initial Request (HTTP Initialize) - -1. HTTP POST to `/mcp/serena` with Authorization header -2. SDK StreamableHTTP extracts session ID from Authorization -3. Session ID stored in request context -4. SDK server instance created and cached for this session -5. Backend stdio connection launched via `GetOrLaunchForSession()` -6. SDK's `client.Connect()` initializes backend: - - Sends `initialize` request to Serena - - Receives `initialize` response - - Sends `notifications/initialized` notification -7. Connection stored in session pool with key `("serena", sessionID)` - -### Subsequent Requests (HTTP Tool Calls) - -1. HTTP POST to `/mcp/serena` with **same Authorization header** -2. SDK StreamableHTTP extracts **same session ID** -3. Cached SDK server instance reused -4. Tool handler calls `GetOrLaunchForSession("serena", sessionID)` -5. Connection pool returns **existing persistent connection** (no new launch) -6. Tool call sent on **same stdio connection** used for initialize -7. Connection's LastUsedAt and RequestCount updated - -### Connection Lifecycle - -- **Creation**: Launched on first request for (backend, session) pair -- **Reuse**: All subsequent requests with same session ID use same connection -- **Idle Timeout**: Cleaned up after 30 minutes of inactivity -- **Error Threshold**: Removed after 10 consecutive errors -- **Cleanup**: Background goroutine runs every 5 minutes - -## Why Serena Still Fails - -Given that session persistence is fully implemented, the Serena failure must have a different root cause: - -### Error Message Analysis - -```json -{ - "jsonrpc": "2.0", - "id": 2, - "error": { - "code": 0, - "message": "method \"tools/list\" is invalid during session initialization" - } -} -``` - -This error: -- Comes from Serena itself (not the gateway) -- Indicates Serena received the request -- Shows Serena is rejecting it because it's **still in initialization state** - -### Possible Root Causes - -1. **Timing Issue**: There may be a race condition where tool calls arrive before Serena has fully transitioned out of initialization state, even though the MCP protocol handshake has completed. - -2. **SDK StreamableHTTP Behavior**: The SDK's StreamableHTTP implementation may allow subsequent requests to be processed before the backend stdio connection has fully stabilized. - -3. **Serena Internal State**: Serena may have additional internal initialization steps beyond the MCP protocol handshake (e.g., language server initialization, workspace indexing). - -4. **Protocol Mismatch**: Serena may expect a specific timing or ordering that isn't compatible with how the SDK's StreamableHTTP processes requests. - -### Evidence Supporting These Theories - -From `test/serena-mcp-tests/GATEWAY_TEST_FINDINGS.md`: -- **Passing**: MCP initialize (succeeds on each request) -- **Failing**: All `tools/list` and `tools/call` requests fail with "invalid during session initialization" - -This pattern suggests: -- The connection is being established correctly -- The MCP handshake is completing successfully -- But Serena isn't ready to accept tool calls yet - -## Recommendations - -### Do NOT Implement the Issue's Suggestions - -The three recommendations in the issue are **already fully implemented and working correctly**: -- ✅ Persistent stdio connections: `SessionConnectionPool` -- ✅ Session mapping: `GetOrLaunchForSession()` with `(backend, session)` keys -- ✅ Connection reuse: Connections survive across multiple HTTP requests - -### Instead, Investigate - -1. **Add Initialization Delay**: Consider adding a configurable delay after `notifications/initialized` before accepting tool calls - - This could be a per-backend configuration option - - Default to 0ms, allow Serena to configure a delay - -2. **Enhanced Logging**: Add detailed logging around: - - When backend initialization completes - - When first tool call arrives - - Timing between these events - -3. **Backend Readiness Check**: Implement a readiness probe that waits for Serena to signal it's ready: - - Could use a health check endpoint - - Or wait for a specific log message on stderr - - Or retry tool calls with exponential backoff - -4. **HTTP-Native Serena**: Consider developing an HTTP-native version of Serena: - - Designed for stateless HTTP requests - - Handles initialization differently - - More compatible with gateway architecture - -5. **SDK Investigation**: Review go-sdk's StreamableHTTP implementation: - - Check if there's a way to block subsequent requests until backend is ready - - Verify if there's a hook to signal backend readiness - - Consider if `Stateless: false` has the expected behavior - -## Code References - -| Component | File | Lines | Purpose | -|-----------|------|-------|---------| -| Session Pool | `internal/launcher/connection_pool.go` | 1-330 | Connection pool with lifecycle management | -| Session Launcher | `internal/launcher/launcher.go` | 178-277 | `GetOrLaunchForSession()` function | -| Backend Init | `internal/mcp/connection.go` | 297-408 | `NewConnection()` with SDK handshake | -| Routed Handler | `internal/server/routed.go` | 111-140 | StreamableHTTP callback with session extraction | -| Unified Handler | `internal/server/transport.go` | 74-109 | Unified mode StreamableHTTP setup | -| Tool Calls | `internal/server/unified.go` | 650-750 | `callBackendTool()` using session launcher | -| Session Helpers | `internal/server/http_helpers.go` | 18-87 | Session extraction and context injection | - -## Testing Recommendations - -To verify session persistence is working: - -1. **Add Session Pool Metrics**: - - Log connection pool size - - Log cache hits vs misses - - Log connection reuse counts - -2. **Add Request Correlation**: - - Log unique request IDs - - Track which backend connection handles each request - - Verify same connection used across session - -3. **Add Timing Metrics**: - - Measure time from `notifications/initialized` to first tool call - - Compare with direct stdio connection timing - - Identify if there's a timing difference causing the issue - -4. **Test with Delays**: - - Manually add delays between initialize and tool calls - - See if Serena succeeds with longer delays - - Determine minimum delay needed - -## Conclusion - -The MCP Gateway's session persistence implementation is **complete, correct, and working as designed**. The architecture properly: - -- Creates persistent stdio connections per session -- Maps HTTP requests to backend connections via session ID -- Reuses connections across multiple requests -- Manages connection lifecycle automatically - -The Serena failure is **not due to missing session persistence**, but rather due to a timing or compatibility issue between: -- Serena's internal initialization state machine -- The SDK's StreamableHTTP request processing -- The gateway's backend connection lifecycle - -Further investigation should focus on **timing and readiness signaling** rather than session persistence architecture. - -## Related Files - -- Issue documentation: `test/serena-mcp-tests/GATEWAY_TEST_FINDINGS.md` -- Test scripts: `test/serena-mcp-tests/test_serena.sh` (direct), `test_serena_via_gateway.sh` (gateway) -- Configuration: `config.toml` (Serena server config on lines 22-27) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md new file mode 100644 index 00000000..2e60f6a7 --- /dev/null +++ b/docs/CONFIGURATION.md @@ -0,0 +1,262 @@ +# Configuration Reference + +This document provides the complete field-by-field reference for MCP Gateway configuration. + +For the upstream specification, see the **[MCP Gateway Configuration Reference](https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/mcp-gateway.md)**. + +## Server Configuration Fields + +- **`type`** (optional): Server transport type + - `"stdio"` - Standard input/output transport (default) + - `"http"` - HTTP transport (fully supported) + - `"local"` - Alias for `"stdio"` (backward compatibility) + +- **`container`** (required for stdio in JSON format): Docker container image (e.g., `"ghcr.io/github/github-mcp-server:latest"`) + - Automatically wraps as `docker run --rm -i ` + - **Note**: The `command` field is NOT supported in JSON stdin format (stdio servers must use `container` instead) + - **TOML format uses `command` and `args` fields - `command` must be `"docker"` for stdio servers** + +- **`entrypoint`** (optional): Custom entrypoint for the container + - Overrides the default container entrypoint + - Applied as `--entrypoint` flag to Docker + +- **`entrypointArgs`** (optional): Arguments passed to container entrypoint + - Array of strings passed after the container image + +- **`args`** (optional): Additional Docker runtime arguments inserted before the container image name + - Array of strings passed to `docker run` before the container image + - Example: `["--network", "host", "--privileged"]` + - Useful for advanced Docker configurations + +- **`mounts`** (optional): Volume mounts for the container + - Array of strings in format `"source:dest:mode"` + - `source` - Host path to mount (can use environment variables with `${VAR}` syntax) + - `dest` - Container path where the volume is mounted + - `mode` - Either `"ro"` (read-only) or `"rw"` (read-write) + - Example: `["/host/config:/app/config:ro", "/host/data:/app/data:rw"]` + +- **`env`** (optional): Environment variables + - Set to `""` (empty string) for passthrough from host environment + - Set to `"value"` for explicit value + - Use `"${VAR_NAME}"` for environment variable expansion (fails if undefined) + +- **`url`** (required for http): HTTP endpoint URL for `type: "http"` servers + +- **`headers`** (optional): HTTP headers to include in requests (for `type: "http"` servers) + - Map of header name to value (e.g., `{"Authorization": "Bearer token"}`) + +- **`tools`** (optional): List of tool names to expose from this server + - If omitted or empty, all tools are exposed + - Example: `["get_file_contents", "search_code"]` + +- **`registry`** (optional): Informational URI to the server's entry in an MCP registry + - Used for documentation and discoverability purposes only; not used at runtime + +- **`guard`** (optional): Name of the guard to use for this server (DIFC) + - References a guard defined in the top-level `[guards]` section + - Enables per-server DIFC guard assignment independent of `guard-policies` + - Example: `guard = "github"` (uses the guard named `github` from `[guards.github]`) + +- **`working_directory`** (optional, TOML format only): Working directory for the server process + - **Note**: This field is parsed and stored but not yet implemented in the launcher; it has no runtime effect currently + +## Guard Policies (`guard-policies`) + +Guard policies provide access control at the MCP gateway level. A server's guard-policies must contain **either** `allow-only` **or** `write-sink`, not both. + +- **`allow-only`**: Restricts which repositories a guard allows (used for GitHub MCP server) +- **`write-sink`**: Marks a server as a write-only output channel that accepts writes from agents with matching secrecy labels + +> **Format note**: JSON format uses `"guard-policies"` (with hyphen), TOML uses `guard_policies` (with underscore). + +### allow-only (GitHub MCP server) + +Controls repository access with the following structure: +```json +"guard-policies": { + "allow-only": { + "repos": ["github/gh-aw-mcpg", "github/gh-aw"], + "min-integrity": "unapproved" + } +} +``` +TOML equivalent: +```toml +[servers.github.guard_policies.allow-only] +repos = ["github/gh-aw-mcpg", "github/gh-aw"] +min-integrity = "unapproved" +``` + +- **`repos`**: Repository access scope + - `"all"` - All repositories accessible by the token + - `"public"` - Public repositories only + - Array of patterns: + - `"owner/repo"` - Exact repository match + - `"owner/*"` - All repositories under owner + - `"owner/prefix*"` - Repositories with name prefix under owner + +- **`min-integrity`**: Minimum integrity level required. Integrity levels are determined by the GitHub MCP server based on the `author_association` field of GitHub objects and whether the object is reachable from the main branch: + - `"none"` - No integrity requirements (includes objects with author_association: FIRST_TIME_CONTRIBUTOR, FIRST_TIMER, NONE) + - `"unapproved"` - Unapproved contributor level (includes objects with author_association: CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR) + - `"approved"` - Approved contributor level (includes objects with author_association: OWNER, MEMBER, COLLABORATOR) + - `"merged"` - Merged to main branch (any object reachable from the main branch, regardless of authorship) + +- **Meaning**: Restricts the GitHub MCP server to only access specified repositories. Tools like `get_file_contents`, `search_code`, etc. will only work on allowed repositories. Attempts to access other repositories will be denied by the guard policy. + +### write-sink (output servers) + +Marks a server as a write-only output channel. **Write-sink is required for ALL output +servers** (e.g., `safeoutputs`) when DIFC guards are enabled on any other server. Without +it, the output server gets a noop guard that classifies operations as reads with empty +labels, causing integrity violations when the agent has integrity tags from other guards. + +When an agent reads from a guarded server (e.g., GitHub with `allow-only`), it acquires +secrecy and integrity labels. The write-sink guard solves this by classifying all +operations as writes and accepting writes from agents whose secrecy labels match the +configured `accept` patterns. + +For exact repos (`repos=["owner/repo1", "owner/repo2"]`): +```json +"guard-policies": { + "write-sink": { + "accept": ["private:owner/repo1", "private:owner/repo2"] + } +} +``` + +For prefix wildcard repos (`repos=["owner/prefix*"]`): +```json +"guard-policies": { + "write-sink": { + "accept": ["private:owner/prefix*"] + } +} +``` + +For broad access (`repos="all"` or `repos="public"`): +```json +"guard-policies": { + "write-sink": { + "accept": ["*"] + } +} +``` + +TOML equivalents: +```toml +# Exact repos +[servers.safeoutputs.guard_policies.write-sink] +Accept = ["private:owner/repo1", "private:owner/repo2"] + +# Prefix wildcard repos +[servers.safeoutputs.guard_policies.write-sink] +Accept = ["private:owner/prefix*"] + +# Broad access (repos="all" or repos="public") +[servers.safeoutputs.guard_policies.write-sink] +Accept = ["*"] +``` + +- **`accept`**: Array of secrecy tags the sink accepts (exact string match against agent secrecy tags — not glob patterns) + - `"*"` - **Wildcard**: Accept writes from agents with any secrecy (must be the sole entry). Use for `repos="all"` or `repos="public"`. + - `"private:owner/repo"` - Matches agent secrecy tag from `repos=["owner/repo"]` (exact repo) + - `"private:owner/prefix*"` - Matches agent secrecy tag from `repos=["owner/prefix*"]` (prefix wildcard — the `*` is a literal character in the tag) + - `"private:owner"` - Matches agent secrecy tag from `repos=["owner/*"]` (owner wildcard — bare owner, no `/*` suffix) + - `"public:owner/repo*"` - Matches agent secrecy tag for public repos matching a prefix + - `"internal:owner/repo*"` - Matches agent secrecy tag for internal repos matching a prefix + +- **How it works**: The write-sink classifies all operations as writes. For DIFC write checks: + - Resource secrecy is set to the `accept` patterns → agent secrecy ⊆ resource secrecy passes + - Resource integrity is left empty → no integrity requirements for writes + +- **When to use**: Required for **all** output servers (`safeoutputs`, etc.) when DIFC guards are enabled on any server in the configuration + +### Mapping allow-only repos to write-sink accept + +The write-sink `accept` entries must match the secrecy tags the GitHub guard assigns +to the agent via `label_agent`. The mapping depends on the `repos` configuration: + +| `allow-only.repos` | Agent secrecy tags | `write-sink.accept` | +|---|---|---| +| `"all"` | `[]` (none) | `["*"]` (wildcard) | +| `"public"` | `[]` (none) | `["*"]` (wildcard) | +| `["owner/repo"]` | `["private:owner/repo"]` | `["private:owner/repo"]` | +| `["owner/*"]` | `["private:owner"]` | `["private:owner"]` | +| `["owner/prefix*"]` | `["private:owner/prefix*"]` | `["private:owner/prefix*"]` | +| `["O/R1", "O/R2"]` | `["private:O/R1", "private:O/R2"]` | `["private:O/R1", "private:O/R2"]` | +| `["O1/*", "O2/R"]` | `["private:O1", "private:O2/R"]` | `["private:O1", "private:O2/R"]` | + +**Key rules**: +- `repos="all"` or `repos="public"` → no secrecy tags → use `accept: ["*"]` (wildcard) +- Write-sink is **required for ALL output servers** when DIFC guards are enabled (prevents noop guard integrity violations) +- `accept: ["*"]` is a special wildcard that accepts writes from agents with any secrecy; it must be the sole entry +- `repos=["owner/*"]` (owner wildcard) → bare owner tag `"private:owner"` (no `/*` suffix) +- `repos=["owner/prefix*"]` (prefix wildcard) → `"private:owner/prefix*"` (suffix preserved) +- `repos=["owner/repo"]` (exact) → `"private:owner/repo"` +- Multi-entry repos produce one tag per entry; `accept` must include all of them +- `accept` can be a superset of the agent's secrecy (extra entries are harmless) +- `min-integrity` does not affect these rules (it only changes integrity labels) + +## Custom Schemas (`customSchemas`) + +The `customSchemas` top-level field allows you to define custom server types beyond the built-in `"stdio"` and `"http"` types. Each custom type maps to an HTTPS schema URL that describes its configuration format. + +```json +{ + "customSchemas": { + "myCustomType": "https://example.com/schemas/my-custom-type.json" + }, + "mcpServers": { + "myServer": { + "type": "myCustomType" + } + } +} +``` + +**Validation Rules for `customSchemas`:** +- Custom type names must not conflict with reserved types (`stdio`, `http`) +- Schema URLs must use `https://` (HTTP URLs are not permitted) +- If a server's `type` references a custom type not listed in `customSchemas`, validation fails with a helpful error message + +## Validation Rules + +- **JSON stdin format**: + - **Stdio servers** must specify `container` (required) + - **HTTP servers** must specify `url` (required) + - **The `command` field is not supported** - stdio servers must use `container` +- **TOML format**: + - Uses `command` and `args` fields directly (e.g., `command = "docker"`) +- **Common rules** (both formats): + - Empty/"local" type automatically normalized to "stdio" + - Variable expansion with `${VAR_NAME}` fails fast on undefined variables + - All validation errors include JSONPath and helpful suggestions + - **Mount specifications** must follow `"source:dest:mode"` format + - `source` must be an absolute path (e.g., `/host/data`) + - `dest` must be an absolute path (e.g., `/app/data`) + - `mode` must be either `"ro"` or `"rw"` + - Both source and destination paths are required (cannot be empty) + +## Gateway Configuration Fields + +| Field | Description | Default | +|-------|-------------|---------| +| `port` | HTTP port (1-65535) | From `--listen` flag | +| `apiKey` | API key for authentication | (disabled) | +| `domain` | Gateway domain (`"localhost"`, `"host.docker.internal"`, or `"${VAR}"`) | `localhost` | +| `startupTimeout` | Seconds to wait for backend startup | `60` | +| `toolTimeout` | Seconds to wait for tool execution | `120` | +| `payloadDir` | Directory for large payload files | `/tmp/jq-payloads` | + +**TOML-only / CLI-only options** (not available in JSON stdin): + +| Option | CLI Flag | Env Var | Default | +|--------|----------|---------|---------| +| Payload size threshold | `--payload-size-threshold` | `MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD` | `524288` | +| Payload path prefix | `--payload-path-prefix` | `MCP_GATEWAY_PAYLOAD_PATH_PREFIX` | (empty) | +| Sequential launch | `--sequential-launch` | — | `false` | +| Guards mode | `--guards-mode` | `MCP_GATEWAY_GUARDS_MODE` | `strict` | + +**Environment Variable Features**: +- **Passthrough**: Set value to empty string (`""`) to pass through from host +- **Expansion**: Use `${VAR_NAME}` syntax for dynamic substitution (fails if undefined) diff --git a/docs/DIFC_INTEGRATION_PROPOSAL.md b/docs/DIFC_INTEGRATION_PROPOSAL.md deleted file mode 100644 index 12b8fe20..00000000 --- a/docs/DIFC_INTEGRATION_PROPOSAL.md +++ /dev/null @@ -1,1362 +0,0 @@ -# DIFC Integration Proposal for MCPG - -> **📝 IMPLEMENTATION NOTE:** -> -> This is the original proposal document. The actual implementation largely aligns with this proposal, with the following key differences and enhancements: -> -> - **Interface changes**: Guard methods use `toolName string, args interface{}` for better WASM compatibility -> - **New `LabelAgent()` method**: Enables policy-driven session initialization (not in proposal) -> - **Path-based labeling**: RFC 6901 JSON Pointer support for fine-grained filtering (major enhancement) -> - **WASM guard support**: Dynamic guard loading from WASM modules -> - **Auto-activation**: DIFC automatically enables when non-noop guards are configured -> -> For implementation details, see: -> - Core implementation: `internal/difc/`, `internal/guard/` -> - Server integration: `internal/server/unified.go` (lines 923-1110) -> - Documentation: `docs/GUARD_RESPONSE_LABELING.md` (covers actual implementation) - -## Overview - -This document proposes an approach to integrate Decentralized Information Flow Control (DIFC) checks and labeling into the Go implementation of MCPG, following the patterns established in the Rust implementation. - -## Core Concepts - -### DIFC Labels -- **Secrecy Labels**: Control information disclosure (who can read data) -- **Integrity Labels**: Control information trust (who can write/influence data) -- **Resources**: Represent external systems with their own label requirements - -### Guard Pattern -- Each MCP server has an associated **Guard** that understands its domain -- Guards **ONLY label** resources and response data - they do NOT make access control decisions -- The **Reference Monitor** (in the server) uses guard-provided labels to enforce DIFC policies -- Reference Monitor decides whether operations are allowed and filters response data -- Default to a **NoopGuard** for servers without custom guards - -## Architecture - -### 1. Package Structure - -``` -awmg/ -├── internal/ -│ ├── difc/ # DIFC label system -│ │ ├── labels.go # Label types and operations -│ │ ├── resource.go # Resource representation -│ │ └── capabilities.go # Global capabilities -│ ├── guard/ # Guard framework -│ │ ├── guard.go # Guard interface and types -│ │ ├── noop.go # Default noop guard -│ │ ├── registry.go # Guard registration -│ │ └── context.go # DIFC context per request -│ ├── guards/ # Specific guard implementations -│ │ ├── github/ # GitHub MCP guard -│ │ │ ├── guard.go -│ │ │ ├── requests.go -│ │ │ └── policy.go -│ │ └── ... # Other guards -│ └── server/ -│ ├── unified.go # Integrate DIFC checks -│ └── routed.go # Integrate DIFC checks -``` - -### 2. Core Interfaces - -#### Guard Interface - -```go -package guard - -import ( - "context" - "github.com/github/gh-aw-mcpg/internal/difc" - sdk "github.com/modelcontextprotocol/go-sdk/mcp" -) - -// BackendCaller provides a way for guards to make read-only calls to the backend -// to gather information needed for labeling (e.g., fetching issue author) -type BackendCaller interface { - // CallTool makes a read-only call to the backend MCP server - // This is used by guards to gather metadata for labeling - CallTool(ctx context.Context, toolName string, args interface{}) (*sdk.CallToolResult, error) -} - -// Guard handles DIFC labeling for a specific MCP server -// Guards ONLY label resources - they do NOT make access control decisions -type Guard interface { - // Name returns the identifier for this guard (e.g., "github", "noop") - Name() string - - // LabelResource determines the resource being accessed and its labels - // This may call the backend (via BackendCaller) to gather metadata needed for labeling - // Returns: - // - resource: The labeled resource (simple or nested structure for fine-grained filtering) - // - operation: The type of operation (Read, Write, or ReadWrite) - LabelResource(ctx context.Context, req *sdk.CallToolRequest, backend BackendCaller, caps *difc.Capabilities) (*difc.LabeledResource, difc.OperationType, error) -} - -// OperationType indicates the nature of the resource access -type OperationType int - -const ( - OperationRead OperationType = iota - OperationWrite - OperationReadWrite -) -``` - -#### DIFC Label System - -```go -package difc - -import "sync" - -// Tag represents a single tag (e.g., "repo:owner/name", "agent:demo-agent") -type Tag string - -// Label represents a set of DIFC tags -type Label struct { - tags map[Tag]struct{} - mu sync.RWMutex -} - -// Resource represents an external system with label requirements (deprecated - use LabeledResource) -type Resource struct { - Description string // Human-readable description of what this resource represents - Secrecy SecrecyLabel - Integrity IntegrityLabel -} - -// LabeledResource represents a resource with DIFC labels -// This can be a simple label pair or a complex nested structure for fine-grained filtering -type LabeledResource struct { - Description string // Human-readable description of the resource - Secrecy SecrecyLabel // Secrecy requirements for this resource - Integrity IntegrityLabel // Integrity requirements for this resource - - // Structure is an optional nested map for fine-grained labeling of response fields - // Maps JSON paths to their labels (e.g., "items[*].private" -> specific labels) - // If nil, labels apply uniformly to entire resource - Structure *ResourceStructure -} - -// ResourceStructure defines fine-grained labels for nested data structures -type ResourceStructure struct { - // Fields maps field names/paths to their labels - // For collections, use "items[*]" to indicate per-item labeling - Fields map[string]*FieldLabels -} - -// FieldLabels defines labels for a specific field in the response -type FieldLabels struct { - Secrecy *SecrecyLabel - Integrity *IntegrityLabel - - // Predicate is an optional function to determine labels based on field value - // For example: label repo as private if repo.Private == true - Predicate func(value interface{}) (*SecrecyLabel, *IntegrityLabel) -} - -// Capabilities represents the global set of tags available to agents -type Capabilities struct { - tags map[Tag]struct{} - mu sync.RWMutex -} - -// AccessDecision represents the result of a DIFC evaluation -type AccessDecision int - -const ( - AccessAllow AccessDecision = iota - AccessDeny -) - -// EvaluationResult contains the decision and required label changes -type EvaluationResult struct { - Decision AccessDecision - SecrecyToAdd []Tag // Secrecy tags agent must add to proceed - IntegrityToDrop []Tag // Integrity tags agent must drop to proceed - Reason string // Human-readable reason for denial -} - -// Evaluator performs DIFC policy evaluation -type Evaluator struct{} - -// Evaluate checks if an agent can perform an operation on a resource -func (e *Evaluator) Evaluate( - agent *AgentLabels, - resource *LabeledResource, - operation OperationType, -) *EvaluationResult { - result := &EvaluationResult{ - Decision: AccessAllow, - SecrecyToAdd: []Tag{}, - IntegrityToDrop: []Tag{}, - } - - switch operation { - case OperationRead: - // For reads: resource integrity must flow to agent (trust check) - ok, missingTags := resource.Integrity.CheckFlow(agent.Integrity) - if !ok { - result.Decision = AccessDeny - result.IntegrityToDrop = missingTags - result.Reason = fmt.Sprintf("Resource '%s' has lower integrity than agent requires", resource.Description) - return result - } - - // For reads: check if agent can handle resource's secrecy - ok, extraTags := agent.Secrecy.CheckFlow(&resource.Secrecy) - if !ok { - result.Decision = AccessDeny - result.SecrecyToAdd = extraTags - result.Reason = fmt.Sprintf("Resource '%s' requires additional secrecy handling", resource.Description) - return result - } - - case OperationWrite: - // For writes: agent integrity must flow to resource (agent must be trustworthy enough) - ok, missingTags := agent.Integrity.CheckFlow(&resource.Integrity) - if !ok { - result.Decision = AccessDeny - result.IntegrityToDrop = missingTags - result.Reason = fmt.Sprintf("Agent lacks required integrity to write to '%s'", resource.Description) - return result - } - - // For writes: agent secrecy must flow to resource secrecy - ok, extraTags := agent.Secrecy.CheckFlow(&resource.Secrecy) - if !ok { - result.Decision = AccessDeny - result.SecrecyToAdd = extraTags - result.Reason = fmt.Sprintf("Agent has secrecy tags that cannot flow to '%s'", resource.Description) - return result - } - - case OperationReadWrite: - // For read-write, must satisfy both read and write constraints - readResult := e.Evaluate(agent, resource, OperationRead) - if readResult.Decision == AccessDeny { - return readResult - } - - writeResult := e.Evaluate(agent, resource, OperationWrite) - if writeResult.Decision == AccessDeny { - return writeResult - } - } - - return result -} - -// FormatViolationError creates a detailed error message explaining the violation and its implications -func FormatViolationError(result *EvaluationResult, agent *AgentLabels, resource *LabeledResource) error { - if result.Decision == AccessAllow { - return nil - } - - var msg strings.Builder - msg.WriteString(fmt.Sprintf("DIFC Violation: %s\n\n", result.Reason)) - - if len(result.SecrecyToAdd) > 0 { - msg.WriteString(fmt.Sprintf("Required Action: Add secrecy tags %v\n", result.SecrecyToAdd)) - msg.WriteString("\nImplications of adding secrecy tags:\n") - msg.WriteString(" - Agent will be restricted from writing to resources that lack these tags\n") - msg.WriteString(" - This includes public resources (e.g., public repositories, public internet)\n") - msg.WriteString(" - Agent will be marked as handling sensitive information\n") - msg.WriteString(fmt.Sprintf(" - Future writes must target resources with tags: %v\n", result.SecrecyToAdd)) - } - - if len(result.IntegrityToDrop) > 0 { - msg.WriteString(fmt.Sprintf("\nRequired Action: Drop integrity tags %v\n", result.IntegrityToDrop)) - msg.WriteString("\nImplications of dropping integrity tags:\n") - msg.WriteString(" - Agent will no longer be able to write to high-integrity resources\n") - msg.WriteString(fmt.Sprintf(" - Specifically, agent cannot write to resources requiring tags: %v\n", result.IntegrityToDrop)) - msg.WriteString(" - This action acknowledges that agent has been influenced by lower-integrity data\n") - msg.WriteString(" - Agent's outputs will be considered less trustworthy\n") - } - - msg.WriteString("\nCurrent Agent Labels:\n") - msg.WriteString(fmt.Sprintf(" Secrecy: %v\n", agent.Secrecy.GetTags())) - msg.WriteString(fmt.Sprintf(" Integrity: %v\n", agent.Integrity.GetTags())) - - msg.WriteString("\nResource Requirements:\n") - msg.WriteString(fmt.Sprintf(" Secrecy: %v\n", resource.Secrecy.GetTags())) - msg.WriteString(fmt.Sprintf(" Integrity: %v\n", resource.Integrity.GetTags())) - - return errors.New(msg.String()) -} - -// Methods for creating and manipulating labels -func NewLabel() *Label { - return &Label{tags: make(map[Tag]struct{})} -} - -func (l *Label) Add(tag Tag) { - l.mu.Lock() - defer l.mu.Unlock() - l.tags[tag] = struct{}{} -} - -func (l *Label) Contains(tag Tag) bool { - l.mu.RLock() - defer l.mu.RUnlock() - _, ok := l.tags[tag] - return ok -} - -func (l *Label) Union(other *Label) { - other.mu.RLock() - defer other.mu.RUnlock() - l.mu.Lock() - defer l.mu.Unlock() - for tag := range other.tags { - l.tags[tag] = struct{}{} - } -} - -func (l *Label) Clone() *Label { - l.mu.RLock() - defer l.mu.RUnlock() - newLabel := NewLabel() - for tag := range l.tags { - newLabel.tags[tag] = struct{}{} - } - return newLabel -} - -// SecrecyLabel wraps Label with secrecy-specific flow semantics -type SecrecyLabel struct { - *Label -} - -// IntegrityLabel wraps Label with integrity-specific flow semantics -type IntegrityLabel struct { - *Label -} - -// NewSecrecyLabel creates a new secrecy label -func NewSecrecyLabel() *SecrecyLabel { - return &SecrecyLabel{Label: NewLabel()} -} - -// NewIntegrityLabel creates a new integrity label -func NewIntegrityLabel() *IntegrityLabel { - return &IntegrityLabel{Label: NewLabel()} -} - -// CanFlowTo checks if this secrecy label can flow to target -// Secrecy semantics: l ⊆ target (this has no tags that target doesn't have) -// Data can only flow to contexts with equal or more secrecy tags -func (l *SecrecyLabel) CanFlowTo(target *SecrecyLabel) bool { - l.mu.RLock() - defer l.mu.RUnlock() - target.mu.RLock() - defer target.mu.RUnlock() - - // Check if all tags in l are in target - for tag := range l.tags { - if _, ok := target.tags[tag]; !ok { - return false - } - } - return true -} - -// CheckFlow checks if this secrecy label can flow to target and returns violation details if not -func (l *SecrecyLabel) CheckFlow(target *SecrecyLabel) (bool, []Tag) { - l.mu.RLock() - defer l.mu.RUnlock() - target.mu.RLock() - defer target.mu.RUnlock() - - var extraTags []Tag - // Check if all tags in l are in target - for tag := range l.tags { - if _, ok := target.tags[tag]; !ok { - extraTags = append(extraTags, tag) - } - } - - return len(extraTags) == 0, extraTags -} - -// GetTags returns all tags in this label -func (l *SecrecyLabel) GetTags() []Tag { - l.mu.RLock() - defer l.mu.RUnlock() - - tags := make([]Tag, 0, len(l.tags)) - for tag := range l.tags { - tags = append(tags, tag) - } - return tags -} - -// CanFlowTo checks if this integrity label can flow to target -// Integrity semantics: l ⊇ target (this has all tags that target has) -// For writes: agent must have >= integrity than endpoint -// For reads: endpoint must have >= integrity than agent -func (l *IntegrityLabel) CanFlowTo(target *IntegrityLabel) bool { - l.mu.RLock() - defer l.mu.RUnlock() - target.mu.RLock() - defer target.mu.RUnlock() - - // Check if all tags in target are in l - for tag := range target.tags { - if _, ok := l.tags[tag]; !ok { - return false - } - } - return true -} - -// CheckFlow checks if this integrity label can flow to target and returns violation details if not -func (l *IntegrityLabel) CheckFlow(target *IntegrityLabel) (bool, []Tag) { - l.mu.RLock() - defer l.mu.RUnlock() - target.mu.RLock() - defer target.mu.RUnlock() - - var missingTags []Tag - // Check if all tags in target are in l - for tag := range target.tags { - if _, ok := l.tags[tag]; !ok { - missingTags = append(missingTags, tag) - } - } - - return len(missingTags) == 0, missingTags -} - -// GetTags returns all tags in this label -func (l *IntegrityLabel) GetTags() []Tag { - l.mu.RLock() - defer l.mu.RUnlock() - - tags := make([]Tag, 0, len(l.tags)) - for tag := range l.tags { - tags = append(tags, tag) - } - return tags -} - -// Clone creates a copy of the secrecy label -func (l *SecrecyLabel) Clone() *SecrecyLabel { - return &SecrecyLabel{Label: l.Label.Clone()} -} - -// Clone creates a copy of the integrity label -func (l *IntegrityLabel) Clone() *IntegrityLabel { - return &IntegrityLabel{Label: l.Label.Clone()} -} - -// NewResource creates a new resource with the given description -func NewResource(description string) *Resource { - return &Resource{ - Description: description, - Secrecy: *NewSecrecyLabel(), - Integrity: *NewIntegrityLabel(), - } -} - -// Empty returns a resource with no label requirements -func (r *Resource) Empty() *Resource { - return &Resource{ - Description: "empty resource", - secrecy: *NewSecrecyLabel(), - integrity: *NewIntegrityLabel(), - } -} - -// ViolationType indicates what kind of DIFC violation occurred -type ViolationType string - -const ( - SecrecyViolation ViolationType = "secrecy" - IntegrityViolation ViolationType = "integrity" -) - -// ViolationError provides detailed information about a DIFC violation -type ViolationError struct { - Type ViolationType - Resource string // Resource description - IsWrite bool // true for write, false for read - MissingTags []Tag // Tags the agent needs but doesn't have - ExtraTags []Tag // Tags the agent has but shouldn't - AgentTags []Tag // All agent tags (for context) - ResourceTags []Tag // All resource tags (for context) -} - -func (e *ViolationError) Error() string { - var msg string - - if e.Type == SecrecyViolation { - msg = fmt.Sprintf("Secrecy violation for resource '%s': ", e.Resource) - if len(e.ExtraTags) > 0 { - msg += fmt.Sprintf("agent has secrecy tags %v that cannot flow to resource. ", e.ExtraTags) - msg += fmt.Sprintf("Remediation: remove these tags from agent's secrecy label or add them to the resource's secrecy requirements.") - } - } else { - if e.IsWrite { - msg = fmt.Sprintf("Integrity violation for write to resource '%s': ", e.Resource) - if len(e.MissingTags) > 0 { - msg += fmt.Sprintf("agent is missing required integrity tags %v. ", e.MissingTags) - msg += fmt.Sprintf("Remediation: agent must gain integrity tags %v to write to this resource.", e.MissingTags) - } - } else { - msg = fmt.Sprintf("Integrity violation for read from resource '%s': ", e.Resource) - if len(e.MissingTags) > 0 { - msg += fmt.Sprintf("resource is missing integrity tags %v that agent requires. ", e.MissingTags) - msg += fmt.Sprintf("Remediation: agent should drop integrity tags %v to trust this resource, or verify resource has higher integrity.", e.MissingTags) - } - } - } - - return msg -} - -// Detailed returns a detailed error message with full context -func (e *ViolationError) Detailed() string { - msg := e.Error() - msg += fmt.Sprintf("\n Agent %s tags: %v", e.Type, e.AgentTags) - msg += fmt.Sprintf("\n Resource %s tags: %v", e.Type, e.ResourceTags) - return msg -} -``` - -#### Agent Labels - -```go -// AgentLabels associates each agent with their DIFC labels -type AgentLabels struct { - AgentID string - Secrecy *SecrecyLabel - Integrity *IntegrityLabel -} - -// AgentRegistry manages agent labels -type AgentRegistry struct { - agents map[string]*AgentLabels - mu sync.RWMutex -} - -func (r *AgentRegistry) GetOrCreate(agentID string) *AgentLabels { - r.mu.Lock() - defer r.mu.Unlock() - - if labels, ok := r.agents[agentID]; ok { - return labels - } - - // Initialize new agent with empty labels - labels := &AgentLabels{ - AgentID: agentID, - Secrecy: NewSecrecyLabel(), - Integrity: NewIntegrityLabel(), - } - r.agents[agentID] = labels - return labels -} -``` - -### 3. Noop Guard Implementation - -```go -package guard - -import ( - "context" - "github.com/github/gh-aw-mcpg/internal/difc" - sdk "github.com/modelcontextprotocol/go-sdk/mcp" -) - -// NoopGuard is the default guard that performs no DIFC labeling -type NoopGuard struct{} - -func NewNoopGuard() *NoopGuard { - return &NoopGuard{} -} - -func (g *NoopGuard) Name() string { - return "noop" -} - -func (g *NoopGuard) LabelResource(ctx context.Context, req *sdk.CallToolRequest, backend BackendCaller, caps *difc.Capabilities) (*difc.LabeledResource, difc.OperationType, error) { - // Empty resource = no label requirements - // Conservatively assume all operations are writes - resource := &difc.LabeledResource{ - Description: "noop resource (no restrictions)", - Secrecy: *difc.NewSecrecyLabel(), - Integrity: *difc.NewIntegrityLabel(), - Structure: nil, // No fine-grained labeling - } - return resource, difc.OperationWrite, nil -} -``` - -### 4. GitHub Guard Example - -```go -package github - -import ( - "context" - "encoding/json" - "fmt" - "github.com/github/gh-aw-mcpg/internal/difc" - "github.com/github/gh-aw-mcpg/internal/guard" - sdk "github.com/modelcontextprotocol/go-sdk/mcp" -) - -type GitHubGuard struct{} - -func NewGitHubGuard() *GitHubGuard { - return &GitHubGuard{} -} - -func (g *GitHubGuard) Name() string { - return "github" -} - -func (g *GitHubGuard) LabelRequest(ctx context.Context, req *sdk.CallToolRequest, backend BackendCaller, caps *difc.Capabilities) (*difc.Resource, bool, guard.RequestState, error) { - // Parse based on tool name - switch req.Params.Name { - case "get_file_contents": - return g.labelGetFileContentsRequest(ctx, req, backend, caps) - case "get_issue": - return g.labelGetIssueRequest(ctx, req, backend, caps) - case "list_repos": - return g.labelListReposRequest(ctx, req, backend, caps) - case "push_files": - return g.labelPushFilesRequest(ctx, req, backend, caps) - // ... other tools - default: - // Unknown tool, treat as write with basic repo labeling - return g.labelUnknownToolRequest(ctx, req, backend, caps) - } -} - -func (g *GitHubGuard) labelGetFileContentsRequest(ctx context.Context, req *sdk.CallToolRequest, backend BackendCaller, caps *difc.Capabilities) (*difc.Resource, bool, guard.RequestState, error) { - // Parse request args - var args struct { - Owner string `json:"owner"` - Repo string `json:"repo"` - Path string `json:"path"` - Ref string `json:"ref"` - } - if err := json.Unmarshal(req.Params.Arguments, &args); err != nil { - return nil, false, nil, err - } - - repoName := fmt.Sprintf("%s/%s", args.Owner, args.Repo) - resource := difc.NewResource(fmt.Sprintf("GitHub file %s in %s@%s", args.Path, repoName, args.Ref)) - - // Add repo tag - data comes from this repository - repoTag := difc.Tag(fmt.Sprintf("repo:%s", repoName)) - resource.Integrity.Add(repoTag) - - // Could optionally call backend here to check if file contains secrets, etc. - // and add additional secrecy labels - - return resource, false, args, nil -} - -func (g *GitHubGuard) labelGetIssueRequest(ctx context.Context, req *sdk.CallToolRequest, backend BackendCaller, caps *difc.Capabilities) (*difc.Resource, bool, guard.RequestState, error) { - // Parse request args - var args struct { - Owner string `json:"owner"` - Repo string `json:"repo"` - Number int `json:"issue_number"` - } - if err := json.Unmarshal(req.Params.Arguments, &args); err != nil { - return nil, false, nil, err - } - - repoName := fmt.Sprintf("%s/%s", args.Owner, args.Repo) - - // **KEY POINT**: Call backend to get issue metadata for labeling - // This happens BEFORE the DIFC check, but we only fetch metadata, not the full content - issueResp, err := backend.CallTool(ctx, "get_issue", map[string]interface{}{ - "owner": args.Owner, - "repo": args.Repo, - "issue_number": args.Number, - }) - if err != nil { - return nil, false, nil, fmt.Errorf("failed to fetch issue metadata: %w", err) - } - - // Parse issue to extract author - var issue struct { - Author string `json:"author"` - Title string `json:"title"` - } - if err := json.Unmarshal(issueResp.Content, &issue); err != nil { - return nil, false, nil, err - } - - resource := difc.NewResource(fmt.Sprintf("GitHub issue #%d in %s", args.Number, repoName)) - - // Label with repo and author - repoTag := difc.Tag(fmt.Sprintf("repo:%s", repoName)) - authorTag := difc.Tag(fmt.Sprintf("user:%s", issue.Author)) - resource.Integrity.Add(repoTag) - resource.Integrity.Add(authorTag) - - // Cache the issue data in state so we don't fetch it again - state := &IssueRequestState{ - Args: args, - IssueData: issueResp, - } - - return resource, false, state, nil -} - -func (g *GitHubGuard) labelListReposRequest(ctx context.Context, req *sdk.CallToolRequest, backend BackendCaller, caps *difc.Capabilities) (*difc.Resource, bool, guard.RequestState, error) { - var args struct { - Username string `json:"username"` - } - if err := json.Unmarshal(req.Params.Arguments, &args); err != nil { - return nil, false, nil, err - } - - // For list operations, the resource represents the query itself, not individual items - // Individual items will be labeled and filtered by the reference monitor - resource := difc.NewResource(fmt.Sprintf("GitHub repositories for user %s", args.Username)) - - // No specific integrity requirements for listing (but items may be filtered) - return resource, false, args, nil -} - -func (g *GitHubGuard) labelPushFilesRequest(ctx context.Context, req *sdk.CallToolRequest, backend BackendCaller, caps *difc.Capabilities) (*difc.Resource, bool, guard.RequestState, error) { - var args struct { - Owner string `json:"owner"` - Repo string `json:"repo"` - Branch string `json:"branch"` - } - if err := json.Unmarshal(req.Params.Arguments, &args); err != nil { - return nil, false, nil, err - } - - repoName := fmt.Sprintf("%s/%s", args.Owner, args.Repo) - resource := difc.NewResource(fmt.Sprintf("GitHub repository %s (branch %s)", repoName, args.Branch)) - - // Writing requires repo integrity - repoTag := difc.Tag(fmt.Sprintf("repo:%s", repoName)) - resource.Integrity.Add(repoTag) - - return resource, true, args, nil -} - -type IssueRequestState struct { - Args interface{} - IssueData *sdk.CallToolResult // Cached issue data from labeling phase -} - -// LabelResponse labels the data returned from the backend -func (g *GitHubGuard) LabelResponse(ctx context.Context, resp *sdk.CallToolResult, resource *difc.Resource, state guard.RequestState) (*guard.LabeledData, error) { - // Check if we cached the response during request labeling - if issueState, ok := state.(*IssueRequestState); ok && issueState.IssueData != nil { - // We already fetched the issue during labeling, use cached data - resp = issueState.IssueData - } - - // Check if this is a list_repos response (collection that can be filtered) - if listReposArgs, ok := state.(struct{ Username string }); ok { - return g.labelListReposResponse(ctx, resp, listReposArgs.Username) - } - - // For single-item responses, return with resource labels - return &SimpleLabeledData{ - result: resp, - labels: &difc.Labels{ - Secrecy: resource.Secrecy.Clone(), - Integrity: resource.Integrity.Clone(), - }, - }, nil -} - -// labelListReposResponse creates a labeled collection for repository lists -func (g *GitHubGuard) labelListReposResponse(ctx context.Context, resp *sdk.CallToolResult, username string) (*guard.LabeledData, error) { - // Parse response to get list of repos - var repos []struct { - Name string `json:"name"` - Owner string `json:"owner"` - Private bool `json:"private"` - // ... other fields - } - - if err := json.Unmarshal(resp.Content, &repos); err != nil { - return nil, fmt.Errorf("failed to parse repos response: %w", err) - } - - // Create labeled items - items := make([]LabeledItem, len(repos)) - for i, repo := range repos { - repoName := fmt.Sprintf("%s/%s", repo.Owner, repo.Name) - - // Create labels for this specific repo - secrecy := difc.NewSecrecyLabel() - integrity := difc.NewIntegrityLabel() - - // Add repo tag - repoTag := difc.Tag(fmt.Sprintf("repo:%s", repoName)) - integrity.Add(repoTag) - - // Private repos need secrecy label - if repo.Private { - privateTag := difc.Tag(fmt.Sprintf("private:%s", repoName)) - secrecy.Add(privateTag) - } - - items[i] = LabeledItem{ - Data: repo, - Labels: &difc.Labels{ - Secrecy: secrecy, - Integrity: integrity, - }, - Description: fmt.Sprintf("repo %s (private=%v)", repoName, repo.Private), - } - } - - return &CollectionLabeledData{ - items: items, - }, nil -} - -// SimpleLabeledData represents non-collection data with uniform labels -type SimpleLabeledData struct { - result *sdk.CallToolResult - labels *difc.Labels -} - -func (d *SimpleLabeledData) Overall() *difc.Labels { - return d.labels -} - -func (d *SimpleLabeledData) IsCollection() bool { - return false -} - -func (d *SimpleLabeledData) FilterCollection(agentLabels *difc.Labels) (interface{}, []string, error) { - return nil, nil, fmt.Errorf("not a collection") -} - -func (d *SimpleLabeledData) ToResult() (*sdk.CallToolResult, error) { - return d.result, nil -} - -// LabeledItem represents a single item in a collection with its labels -type LabeledItem struct { - Data interface{} - Labels *difc.Labels - Description string // For audit logging -} - -// CollectionLabeledData represents a collection of individually labeled items -type CollectionLabeledData struct { - items []LabeledItem -} - -func (d *CollectionLabeledData) Overall() *difc.Labels { - // Union of all item labels - overall := &difc.Labels{ - Secrecy: difc.NewSecrecyLabel(), - Integrity: difc.NewIntegrityLabel(), - } - for _, item := range d.items { - overall.Secrecy.Union(item.Labels.Secrecy.Label) - overall.Integrity.Union(item.Labels.Integrity.Label) - } - return overall -} - -func (d *CollectionLabeledData) IsCollection() bool { - return true -} - -func (d *CollectionLabeledData) FilterCollection(agentLabels *difc.Labels) (interface{}, []string, error) { - var filtered []interface{} - var violations []string - - for _, item := range d.items { - // Check if agent can access this item - // For reads: item integrity must flow to agent (trust check) - canAccessIntegrity, _ := item.Labels.Integrity.CheckFlow(agentLabels.Integrity) - - // For reads: agent secrecy must flow to item secrecy (can the agent handle this data?) - canAccessSecrecy, _ := agentLabels.Secrecy.CheckFlow(item.Labels.Secrecy) - - if canAccessIntegrity && canAccessSecrecy { - filtered = append(filtered, item.Data) - } else { - violations = append(violations, item.Description) - } - } - - return filtered, violations, nil -} - -func (d *CollectionLabeledData) ToResult() (*sdk.CallToolResult, error) { - // Collect all item data - data := make([]interface{}, len(d.items)) - for i, item := range d.items { - data[i] = item.Data - } - - content, err := json.Marshal(data) - if err != nil { - return nil, fmt.Errorf("failed to marshal collection: %w", err) - } - - return &sdk.CallToolResult{ - Content: content, - IsError: false, - }, nil -} -``` - -### 5. Integration into MCP Server - -```go -package server - -import ( - "context" - "fmt" - "log" - "github.com/github/gh-aw-mcpg/internal/difc" - "github.com/github/gh-aw-mcpg/internal/guard" - sdk "github.com/modelcontextprotocol/go-sdk/mcp" -) - -type UnifiedServer struct { - // ... existing fields - - // DIFC additions - guards map[string]guard.Guard // serverID -> Guard - agentRegistry *difc.AgentRegistry - capabilities *difc.Capabilities -} - -func NewUnified(ctx context.Context, cfg *config.Config) (*UnifiedServer, error) { - // ... existing initialization - - us := &UnifiedServer{ - // ... existing fields - - guards: make(map[string]guard.Guard), - agentRegistry: difc.NewAgentRegistry(), - capabilities: difc.NewCapabilities(), - } - - // Register guards for each backend - for _, serverID := range cfg.ServerIDs() { - us.registerGuard(serverID) - } - - return us, nil -} - -func (us *UnifiedServer) registerGuard(serverID string) { - // Look up guard implementation, default to noop - var g guard.Guard - - switch serverID { - case "github": - g = github.NewGitHubGuard() - // ... other guards - default: - g = guard.NewNoopGuard() - log.Printf("No guard implementation for %s, using noop guard", serverID) - } - - us.guards[serverID] = g - log.Printf("Registered guard %s for server %s", g.Name(), serverID) -} - -// Modified tool call handler with DIFC checks -func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName string, args interface{}) (*sdk.CallToolResult, interface{}, error) { - // Get agent ID from context (from Authorization header) - agentID := getAgentIDFromContext(ctx) - agentLabels := us.agentRegistry.GetOrCreate(agentID) - - // Get guard for this backend - g := us.guards[serverID] - - // Create MCP request - mcpReq := &sdk.CallToolRequest{ - Params: sdk.CallToolRequestParams{ - Name: toolName, - Arguments: args, - }, - } - - // Create backend caller for the guard - backendCaller := &guardBackendCaller{ - server: us, - serverID: serverID, - ctx: ctx, - } - - // **Phase 1: Label the request resource (guard labels, doesn't decide)** - resource, isWrite, state, err := g.LabelRequest(ctx, mcpReq, backendCaller, us.capabilities) - if err != nil { - return &sdk.CallToolResult{IsError: true}, nil, fmt.Errorf("failed to label request: %w", err) - } - - // **Phase 2: Reference monitor checks DIFC constraints (coarse-grained)** - if err := us.checkDIFCConstraints(agentLabels, resource, isWrite); err != nil { - return &sdk.CallToolResult{IsError: true}, nil, err - } - - // **Phase 3: Call backend (unless cached in state)** - var mcpResp *sdk.CallToolResult - - // Check if response is cached in state from request labeling - if cachedResp := extractCachedResponse(state); cachedResp != nil { - mcpResp = cachedResp - } else { - // Make the actual backend call - conn, err := launcher.GetOrLaunch(us.launcher, serverID) - if err != nil { - return &sdk.CallToolResult{IsError: true}, nil, err - } - - result, err := conn.SendRequest("tools/call", mcpReq) - if err != nil { - return &sdk.CallToolResult{IsError: true}, nil, err - } - mcpResp = parseCallToolResult(result) - } - - // **Phase 4: Guard labels the response data** - labeledData, err := g.LabelResponse(ctx, mcpResp, resource, state) - if err != nil { - return &sdk.CallToolResult{IsError: true}, nil, fmt.Errorf("failed to label response: %w", err) - } - - // **Phase 5: Reference monitor filters/validates response based on labels** - var finalResp *sdk.CallToolResult - - if labeledData.IsCollection() { - // Filter collection items based on agent labels - filteredData, violations, err := labeledData.FilterCollection(&difc.Labels{ - Secrecy: agentLabels.Secrecy, - Integrity: agentLabels.Integrity, - }) - if err != nil { - return &sdk.CallToolResult{IsError: true}, nil, fmt.Errorf("failed to filter collection: %w", err) - } - - // Log filtered items for audit - if len(violations) > 0 { - log.Printf("Agent %s: Filtered %d items: %v", agentID, len(violations), violations) - } - - // Create response with filtered data - filteredContent, err := json.Marshal(filteredData) - if err != nil { - return &sdk.CallToolResult{IsError: true}, nil, fmt.Errorf("failed to marshal filtered data: %w", err) - } - - finalResp = &sdk.CallToolResult{ - Content: filteredContent, - IsError: false, - } - } else { - // Single item - already passed coarse-grained check - finalResp, err = labeledData.ToResult() - if err != nil { - return &sdk.CallToolResult{IsError: true}, nil, fmt.Errorf("failed to convert labeled data: %w", err) - } - } - - // **Phase 6: Accumulate labels from this operation** - // For reads, agent gains labels from the data they read - if !isWrite { - overall := labeledData.Overall() - agentLabels.Integrity.Union(overall.Integrity.Label) - agentLabels.Secrecy.Union(overall.Secrecy.Label) - } - - return finalResp, nil, nil -} - -// guardBackendCaller implements BackendCaller for guards -type guardBackendCaller struct { - server *UnifiedServer - serverID string - ctx context.Context -} - -func (gbc *guardBackendCaller) CallTool(ctx context.Context, toolName string, args interface{}) (*sdk.CallToolResult, error) { - // Make a direct backend call without DIFC checks - // This is only used by guards for metadata gathering during labeling - conn, err := launcher.GetOrLaunch(gbc.server.launcher, gbc.serverID) - if err != nil { - return nil, err - } - - mcpReq := &sdk.CallToolRequest{ - Params: sdk.CallToolRequestParams{ - Name: toolName, - Arguments: args, - }, - } - - result, err := conn.SendRequest("tools/call", mcpReq) - if err != nil { - return nil, err - } - - return parseCallToolResult(result), nil -} - -func extractCachedResponse(state guard.RequestState) *sdk.CallToolResult { - // Check common state types for cached responses - type cacheableState interface { - CachedResponse() *sdk.CallToolResult - } - - if cs, ok := state.(cacheableState); ok { - return cs.CachedResponse() - } - return nil -} - -func (us *UnifiedServer) checkDIFCConstraints(agent *difc.AgentLabels, resource *difc.Resource, isWrite bool) error { - if isWrite { - // Write operation: agent can only write to resources with lower or equal integrity - // (agent's integrity must be >= resource's integrity) - ok, missingTags := agent.Integrity.CheckFlow(&resource.Integrity) - if !ok { - return &difc.ViolationError{ - Type: difc.IntegrityViolation, - Resource: resource.Description, - IsWrite: true, - MissingTags: missingTags, - AgentTags: agent.Integrity.GetTags(), - ResourceTags: resource.Integrity.GetTags(), - } - } - } else { - // Read operation: agent can only read from resources with higher or equal integrity - // (resource's integrity must be >= agent's integrity to trust the data) - ok, missingTags := resource.Integrity.CheckFlow(agent.Integrity) - if !ok { - return &difc.ViolationError{ - Type: difc.IntegrityViolation, - Resource: resource.Description, - IsWrite: false, - MissingTags: missingTags, - AgentTags: agent.Integrity.GetTags(), - ResourceTags: resource.Integrity.GetTags(), - } - } - } - - // Secrecy check: data can only flow where secrecy allows - ok, extraTags := agent.Secrecy.CheckFlow(&resource.Secrecy) - if !ok { - return &difc.ViolationError{ - Type: difc.SecrecyViolation, - Resource: resource.Description, - IsWrite: isWrite, - ExtraTags: extraTags, - AgentTags: agent.Secrecy.GetTags(), - ResourceTags: resource.Secrecy.GetTags(), - } - } - - return nil -} -``` - -## Implementation Phases - -### Phase 1: Foundation (2-3 weeks) -1. Implement `internal/difc` package - - Label types (Secrecy, Integrity) - - Endpoint representation - - Capabilities - - Label operations (union, intersection, canFlowTo) - -2. Implement `internal/guard` package - - Guard interface - - Noop guard implementation - - Guard registry - -3. Add agent label management - - AgentLabels struct - - AgentRegistry for tracking agents - -### Phase 2: Integration (2-3 weeks) -1. Integrate guards into UnifiedServer - - Register guards for each backend - - Add DIFC checks to callBackendTool - - Pass agent context through requests - -2. Extract agent ID from requests - - Parse Authorization header - - Create/retrieve AgentLabels - -3. Add DIFC constraint checking - - Read/write operation detection - - Integrity and secrecy flow checks - -### Phase 3: Guard Implementations (3-4 weeks) -1. Implement GitHub guard - - Parse common tools (get_file_contents, push_files, etc.) - - Add repo labels - - Implement policies - -2. Implement other guards as needed - - Filesystem guard - - Memory guard - - Custom guards - -3. Testing and refinement - - Unit tests for guards - - Integration tests for DIFC flow - - Policy validation - -### Phase 4: Configuration (1-2 weeks) -1. Add agent configuration - - Initial labels per agent - - Label inheritance rules - - Policy overrides - -2. Add guard configuration - - Per-server guard selection - - Guard-specific settings - -3. Logging and debugging - - Log DIFC decisions - - Debug mode for label tracking - - Violation reporting - -## Configuration Format - -```toml -# config.toml - -[agents] -[agents.default] -secrecy_labels = [] -integrity_labels = [] - -[agents."demo-agent"] -secrecy_labels = ["agent:demo-agent"] -integrity_labels = ["agent:demo-agent"] - -[agents."production-agent"] -secrecy_labels = ["agent:production-agent", "env:production"] -integrity_labels = ["agent:production-agent", "env:production"] - -[servers.github] -command = "docker" -args = ["run", "--rm", "-i", "ghcr.io/github/github-mcp-server:latest"] -guard = "github" # Use github guard - -[servers.github.guard_config] -# GitHub-specific guard configuration - -[servers.custom] -command = "node" -args = ["custom-server.js"] -guard = "noop" # Explicitly use noop guard -``` - -## Benefits - -1. **Security**: Tracks information flow through the system -2. **Transparency**: Clear labeling of data sources and integrity -3. **Flexibility**: Easy to add new guards for new MCP servers -4. **Compatibility**: Works with existing MCP servers via noop guard -5. **Gradual Adoption**: Can deploy without guards, add them incrementally -6. **Fine-Grained Filtering**: Can filter individual items in collections based on labels - -## Fine-Grained Filtering - -The DIFC system supports two levels of access control, both enforced by the **Reference Monitor**: - -### 1. Coarse-Grained (Resource-Level) -Guard's `LabelRequest` returns labels for the entire operation. The **reference monitor** checks these labels before allowing the operation. If the check fails, the entire operation is rejected. - -**Example**: Reading a specific issue requires integrity tag `user:issue-author`. If the agent lacks this tag, the **reference monitor** rejects the request. - -**Responsibility Split**: -- **Guard**: Labels the resource (e.g., "issue requires `user:alice` tag") -- **Reference Monitor**: Checks if agent has required tags, rejects if not - -### 2. Fine-Grained (Item-Level) -Guard's `LabelResponse` returns a `LabeledData` structure where individual items in a collection have their own labels. The **reference monitor** filters items based on agent labels. - -**Example**: Listing repositories for a user returns a mix of public and private repos: -1. **Guard** `LabelRequest`: Returns minimal-requirement resource representing the list query -2. Backend call fetches all repos -3. **Reference Monitor**: Checks coarse-grained labels (passes because listing is allowed) -4. **Guard** `LabelResponse`: Returns `CollectionLabeledData` with per-repo labels: - - Public repos: No secrecy requirements - - Private repos: Require `private:owner/repo` secrecy tag -5. **Reference Monitor** `FilterCollection`: Iterates through items, includes accessible ones, filters out inaccessible ones, logs violations for audit - -**Responsibility Split**: -- **Guard**: Labels each item in the collection (e.g., "this repo requires `private:foo/bar` tag") -- **Reference Monitor**: Decides which items agent can access, filters accordingly, logs audit trail - -### When to Use Each Approach - -**Coarse-Grained (fail entire request)**: -- Single-item operations (get specific file, get specific issue) -- Write operations (push files, create issue) -- Operations where partial access doesn't make sense - -**Fine-Grained (filter items)**: -- List operations (list repos, list issues, search) -- Aggregate operations (get repository statistics) -- Any operation returning collections where partial access is acceptable - -### Key Principle: Guard Labels, Reference Monitor Decides - -The guard is a **domain expert** that understands how to label resources and data in its domain (e.g., GitHub knows that private repos need privacy labels, issues should be labeled with author tags). - -The reference monitor is the **security policy enforcer** that makes all access control decisions based on those labels. This separation ensures: -- Guards can be written by domain experts without security expertise -- Security policy is centralized and consistent -- Guards are simpler and easier to test -- Policy changes don't require modifying guards - -### Implementation Notes - -1. **Audit Logging**: Reference monitor logs all filtered items for security audit -2. **Performance**: Filtering happens after backend call, so all data is fetched but only accessible items are returned -3. **Metadata**: Reference monitor can include filtered count in response metadata (e.g., "showing 5 of 12 repositories") -4. **Configuration**: Reference monitor can be configured per-agent for strict mode (reject if any item inaccessible) vs filter mode (remove inaccessible items) - -## Migration Path - -1. **Initial**: Deploy with all noop guards -2. **Add Guards**: Implement guards for critical servers (e.g., GitHub) -3. **Configure Agents**: Set initial labels for different agent types -4. **Enforce Policies**: Enable DIFC constraint checking -5. **Refine**: Adjust policies based on usage patterns - -## Testing Strategy - -1. **Unit Tests**: Test each guard implementation independently -2. **Integration Tests**: Test DIFC flow through full request cycle -3. **Policy Tests**: Verify constraints are enforced correctly -4. **Performance Tests**: Ensure DIFC overhead is acceptable -5. **Compatibility Tests**: Ensure existing functionality works with noop guards - -## Open Questions - -1. **Label Persistence**: Should agent labels persist across sessions? -2. **Label Discovery**: How do agents discover available labels? -3. **Policy Language**: Should we support a DSL for complex policies? -4. **Audit Logging**: What level of DIFC decision logging is needed? -5. **Performance**: What is acceptable overhead for DIFC checks? diff --git a/docs/ENVIRONMENT_VARIABLES.md b/docs/ENVIRONMENT_VARIABLES.md new file mode 100644 index 00000000..032ec39e --- /dev/null +++ b/docs/ENVIRONMENT_VARIABLES.md @@ -0,0 +1,63 @@ +# Environment Variables + +Complete reference for MCP Gateway environment variables. + +## Required for Production (Containerized Mode) + +When running in a container (`run_containerized.sh`), these variables **must** be set: + +| Variable | Description | Example | +|----------|-------------|---------| +| `MCP_GATEWAY_PORT` | The port the gateway listens on (used for `--listen` address) | `8080` | +| `MCP_GATEWAY_DOMAIN` | The domain name for the gateway | `localhost` | +| `MCP_GATEWAY_API_KEY` | API key checked by `run_containerized.sh` as a deployment gate; must be referenced in your JSON config via `"${MCP_GATEWAY_API_KEY}"` to enable authentication | `your-secret-key` | + +## Optional (Non-Containerized Mode) + +When running locally (`run.sh`), these variables are optional (warnings shown if missing): + +| Variable | Description | Default | +|----------|-------------|---------| +| `MCP_GATEWAY_PORT` | Gateway listening port | `8000` | +| `MCP_GATEWAY_DOMAIN` | Gateway domain | `localhost` | +| `MCP_GATEWAY_API_KEY` | Informational only — not read directly by the binary; must be referenced in your config via `"${MCP_GATEWAY_API_KEY}"` to enable authentication | (disabled) | +| `MCP_GATEWAY_LOG_DIR` | Log file directory (sets default for `--log-dir` flag) | `/tmp/gh-aw/mcp-logs` | +| `MCP_GATEWAY_PAYLOAD_DIR` | Large payload storage directory (sets default for `--payload-dir` flag) | `/tmp/jq-payloads` | +| `MCP_GATEWAY_PAYLOAD_PATH_PREFIX` | Path prefix for remapping payloadPath returned to clients (sets default for `--payload-path-prefix` flag) | (empty - use actual filesystem path) | +| `MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD` | Size threshold in bytes for payload storage (sets default for `--payload-size-threshold` flag) | `524288` | +| `MCP_GATEWAY_WASM_GUARDS_DIR` | Root directory for per-server WASM guards (`//*.wasm`, first match is loaded) | (disabled) | +| `MCP_GATEWAY_GUARDS_MODE` | Guards enforcement mode: `strict` (deny violations), `filter` (remove denied tools), `propagate` (auto-adjust agent labels) (sets default for `--guards-mode`) | `strict` | +| `MCP_GATEWAY_GUARDS_SINK_SERVER_IDS` | Comma-separated sink server IDs for JSONL guards tag enrichment (sets default for `--guards-sink-server-ids`) | (disabled) | +| `DEBUG` | Enable debug logging with pattern matching (e.g., `*`, `server:*,launcher:*`) | (disabled) | +| `DEBUG_COLORS` | Control colored debug output (0 to disable, auto-disabled when piping) | Auto-detect | +| `RUNNING_IN_CONTAINER` | Manual override; set to `"true"` to force container detection when `/.dockerenv` and cgroup detection are unavailable | (unset) | + +**Note:** `PORT`, `HOST`, and `MODE` are not read by the `awmg` binary directly. However, `run.sh` does use `HOST` (default: `0.0.0.0`) and `MODE` (default: `--routed`) to set the bind address and routing mode. Use the `--listen` and `--routed`/`--unified` flags when running `awmg` directly. + +## Containerized Deployment Variables + +When using `run_containerized.sh`, these additional variables are available: + +| Variable | Description | Default | +|----------|-------------|---------| +| `MCP_GATEWAY_HOST` | Bind address for the gateway | `0.0.0.0` | +| `MCP_GATEWAY_MODE` | Routing mode flag passed to `awmg` (e.g., `--routed`, `--unified`) | `--routed` | + +## Docker Configuration + +| Variable | Description | Default | +|----------|-------------|---------| +| `DOCKER_HOST` | Docker daemon socket path | `/var/run/docker.sock` | +| `DOCKER_API_VERSION` | Docker API version (set by helper scripts, Docker client auto-negotiates) | Set by querying Docker daemon's current API version; falls back to `1.44` if detection fails | + +## DIFC / Guard Policy Configuration + +These environment variables configure guard policies (e.g., AllowOnly policies for restricting tool access to specific GitHub repositories): + +| Variable | Description | Default | +|----------|-------------|---------| +| `MCP_GATEWAY_GUARD_POLICY_JSON` | Guard policy JSON (e.g., `{"allow-only":{"repos":"public","min-integrity":"none"}}`) (sets default for `--guard-policy-json`) | (disabled) | +| `MCP_GATEWAY_ALLOWONLY_SCOPE_PUBLIC` | Use public AllowOnly scope (sets default for `--allowonly-scope-public`) | `false` | +| `MCP_GATEWAY_ALLOWONLY_SCOPE_OWNER` | AllowOnly owner scope value (sets default for `--allowonly-scope-owner`) | (disabled) | +| `MCP_GATEWAY_ALLOWONLY_SCOPE_REPO` | AllowOnly repo name (requires owner) (sets default for `--allowonly-scope-repo`) | (disabled) | +| `MCP_GATEWAY_ALLOWONLY_MIN_INTEGRITY` | AllowOnly integrity level: `none`, `unapproved`, `approved`, `merged` (sets default for `--allowonly-min-integrity`) | (disabled) | diff --git a/docs/STREAMABLE_HTTP_HANDLER_EXPLAINED.md b/docs/STREAMABLE_HTTP_HANDLER_EXPLAINED.md deleted file mode 100644 index f60d616b..00000000 --- a/docs/STREAMABLE_HTTP_HANDLER_EXPLAINED.md +++ /dev/null @@ -1,343 +0,0 @@ -# StreamableHTTPHandler: How It Works - -> **⚠️ HISTORICAL DOCUMENT NOTE (March 2026):** -> -> This document describes architectural limitations that **have been solved** in the current implementation. The gateway now uses a `filteredServerCache` (see `internal/server/routed.go:33-74`) that preserves SDK protocol state across HTTP requests by caching server instances per (backend, session) pair. -> -> **Current Implementation Status:** -> - ✅ Protocol state IS preserved across HTTP requests -> - ✅ SDK Server instances are cached and reused per session -> - ✅ Session-scoped SDK Server reuse supports both stateless and stateful backend interaction patterns -> -> This document is maintained for historical context to show the problem that was solved. For current architecture, see the implementation in `internal/server/routed.go` and `internal/server/transport.go`. - ---- - -This document explains where the MCP SDK's `StreamableHTTPHandler` lives, what it does, and how it communicates with backend MCP servers. - -## TL;DR - -- **StreamableHTTPHandler**: Frontend only (gateway side), translates HTTP ↔ JSON-RPC -- **Backend**: Just a process receiving JSON-RPC via stdio, no HTTP awareness -- **What backend receives**: Plain JSON-RPC messages like `{"jsonrpc":"2.0","method":"tools/call",...}` -- **Protocol state**: Tracked separately on frontend (SDK Server) and backend (server code) -- **The issue**: New SDK Server instance per HTTP request = fresh protocol state, even though backend connection is reused - -## Where Does StreamableHTTPHandler Live? - -**Answer: Frontend only (gateway side)** - -``` -┌─────────────────────────────────────────┐ -│ Gateway Process │ -│ ┌────────────────────────────────────┐ │ -│ │ StreamableHTTPHandler (Frontend) │ │ -│ │ - Receives HTTP POST requests │ │ -│ │ - Translates to JSON-RPC │ │ -│ │ - Creates SDK Server instance │ │ -│ │ - Tracks protocol state │ │ -│ └────────────────────────────────────┘ │ -│ ↓ stdio pipes │ -└──────────────┼──────────────────────────┘ - │ JSON-RPC messages - ↓ -┌──────────────────────────────────────────┐ -│ Backend Process (e.g., Serena) │ -│ - Receives JSON-RPC via stdin │ -│ - Sends JSON-RPC via stdout │ -│ - NO awareness of HTTP │ -│ - NO awareness of StreamableHTTPHandler │ -│ - Tracks its own state machine │ -└──────────────────────────────────────────┘ -``` - -**Key Points:** -- Backend is just a process that speaks JSON-RPC over stdio -- Backend never sees HTTP requests, headers, or StreamableHTTPHandler -- Backend has no knowledge it's behind a gateway - -## What Does StreamableHTTPHandler Do? - -### Primary Function: HTTP ↔ JSON-RPC Translation - -``` -HTTP Request (from agent) - POST /mcp/serena - Body: {"jsonrpc":"2.0","method":"tools/call","params":{...}} - ↓ - StreamableHTTPHandler - - Creates SDK Server instance - - Parses JSON-RPC from HTTP body - - Routes to SDK Server methods - ↓ - SDK Server instance - - Validates protocol state (uninitialized → ready) - - Formats as JSON-RPC message - ↓ - Stdio pipes to backend - - Writes: {"jsonrpc":"2.0","method":"tools/call",...} - ↓ - Backend Process (Serena) - - Reads JSON-RPC from stdin - - Processes request - - Validates its own state - - Writes response to stdout - ↓ - SDK Server instance - - Reads JSON-RPC response from stdio - ↓ - StreamableHTTPHandler - - Translates to HTTP response - ↓ -HTTP Response (to agent) - Body: {"jsonrpc":"2.0","result":{...}} -``` - -## What Gets Passed to the Backend? - -**Answer: Only JSON-RPC messages, nothing about HTTP or protocol state** - -### Example Flow: - -**HTTP Request 1 (initialize):** -``` -Agent sends HTTP: - POST /mcp/serena - Authorization: session-123 - Body: { - "jsonrpc": "2.0", - "id": 1, - "method": "initialize", - "params": {"protocolVersion": "2024-11-05", ...} - } - -Gateway StreamableHTTPHandler: - - Extracts session ID from Authorization header - - Creates NEW SDK Server instance for this request - - SDK Server state: uninitialized - - Sees "initialize" method → Valid for uninitialized state - - Transitions state: uninitialized → ready - -Backend (Serena) receives via stdio: - { - "jsonrpc": "2.0", - "id": 1, - "method": "initialize", - "params": {"protocolVersion": "2024-11-05", ...} - } - -Backend does NOT receive: - ❌ HTTP headers (Authorization, Content-Type, etc.) - ❌ Session ID - ❌ Frontend SDK protocol state - ❌ Any indication this came via HTTP -``` - -**HTTP Request 2 (tools/call) - SAME session:** -``` -Agent sends HTTP: - POST /mcp/serena - Authorization: session-123 - Body: { - "jsonrpc": "2.0", - "id": 2, - "method": "tools/call", - "params": {"name": "search_code", ...} - } - -Gateway StreamableHTTPHandler: - - Extracts session ID: session-123 (same as before) - - Backend connection: ✅ REUSED (session pool works) - - Creates NEW SDK Server instance for this request ❌ - - SDK Server state: uninitialized ❌ - - Sees "tools/call" method → Invalid for uninitialized state ❌ - - ERROR: "method 'tools/call' is invalid during session initialization" - -Backend (Serena) NEVER receives this request - ❌ Request blocked by frontend SDK protocol validation -``` - -## Protocol State: Frontend vs Backend - -This is the critical distinction: - -### Frontend Protocol State (SDK Server) -``` -Location: Gateway process, SDK Server instance -Tracks: MCP protocol state machine -States: uninitialized → initializing → ready -Problem: NEW instance per HTTP request = always uninitialized -``` - -### Backend Protocol State (Server Implementation) -``` -Location: Backend process (Serena/GitHub) -Tracks: Backend's own state machine -GitHub: NO state validation (stateless) -Serena: ENFORCES state validation (stateful) -``` - -### The Disconnect: - -``` -┌─────────────────────────────────────────────────────────────┐ -│ Gateway (Frontend) │ -├─────────────────────────────────────────────────────────────┤ -│ Request 1: │ -│ SDK Server instance #1 (state: uninitialized) │ -│ Sees: initialize → Valid → State: ready ✅ │ -│ Sends to backend: {"method":"initialize",...} │ -│ │ -│ Request 2 (same session): │ -│ SDK Server instance #2 (state: uninitialized) ❌ │ -│ Sees: tools/call → Invalid → ERROR ❌ │ -│ Backend never receives this request │ -└─────────────────────────────────────────────────────────────┘ - ↓ stdio (persistent) -┌─────────────────────────────────────────────────────────────┐ -│ Backend Process (Same process, reused ✅) │ -├─────────────────────────────────────────────────────────────┤ -│ Received Request 1: │ -│ {"method":"initialize",...} │ -│ Backend state: uninitialized → ready ✅ │ -│ │ -│ Request 2 would have been fine: │ -│ Backend state: still ready ✅ │ -│ Would process {"method":"tools/call",...} successfully │ -│ But frontend SDK blocked it before backend saw it ❌ │ -└─────────────────────────────────────────────────────────────┘ -``` - -## Why GitHub Works But Serena Doesn't - -### GitHub MCP Server (Stateless) - -**Backend code doesn't validate protocol state:** -```typescript -// GitHub MCP Server -server.setRequestHandler(ListToolsRequestSchema, async () => { - // NO state check - just process the request - // Works regardless of whether initialize was called - return { tools: [...] }; -}); - -server.setRequestHandler(CallToolRequestSchema, async (request) => { - // NO state check - just execute the tool - return await executeTool(request.params.name, request.params.arguments); -}); -``` - -**Result:** -``` -Frontend SDK Server: uninitialized (wrong) ❌ -Backend doesn't care: processes request anyway ✅ -Works through gateway: ✅ -``` - -### Serena MCP Server (Stateful) - -**Backend code validates protocol state:** -```python -# Serena MCP Server -class SerenaServer: - def __init__(self): - self.state = "uninitialized" - - async def handle_initialize(self, params): - self.state = "ready" - return {"protocolVersion": "2024-11-05"} - - async def list_tools(self): - if self.state != "ready": # State validation - raise Error("method 'tools/list' is invalid during session initialization") - return {"tools": [...]} - - async def call_tool(self, name, arguments): - if self.state != "ready": # State validation - raise Error("method 'tools/call' is invalid during session initialization") - return await self._execute_tool(name, arguments) -``` - -**Result:** -``` -Frontend SDK Server: uninitialized (wrong) ❌ -Backend validates state: rejects request ❌ -Fails through gateway: ❌ -``` - -## Backend Connection vs Protocol State - -This is crucial to understand: - -### Backend Connection (Works Correctly ✅) -``` -- Managed by SessionConnectionPool -- One persistent stdio connection per (backend, session) -- Same Docker container process -- Same stdin/stdout/stderr pipes -- Connection IS reused across HTTP requests ✅ -``` - -### Protocol State (Doesn't Persist ❌) -``` -- Managed by SDK Server instances -- New instance created per HTTP request -- Each instance starts in "uninitialized" state -- Protocol state NOT preserved across HTTP requests ❌ -``` - -### Visual: -``` -HTTP Request 1 (Authorization: session-123) - → NEW SDK Server (state: uninitialized) - → REUSED backend connection ✅ - → Same backend process ✅ - → {"method":"initialize"} sent - -HTTP Request 2 (Authorization: session-123) - → NEW SDK Server (state: uninitialized) ❌ - → REUSED backend connection ✅ - → Same backend process ✅ - → {"method":"tools/call"} blocked by SDK ❌ -``` - -## The Architecture Issue - -The SDK's `StreamableHTTPHandler` was designed for **stateless HTTP scenarios** where: -- Each HTTP request is completely independent -- No session state needs to persist -- Backend doesn't validate protocol state - -It doesn't support **stateful backends** where: -- Protocol handshake must complete on the same session -- Backend validates that initialize was called before other methods -- Session state must persist across multiple HTTP requests - -## Summary - -**Where StreamableHTTPHandler lives:** -- Frontend only (gateway process) - -**What it does:** -- Translates HTTP requests to JSON-RPC messages -- Creates SDK Server instances to handle protocol -- Sends JSON-RPC to backend via stdio - -**What backend receives:** -- Plain JSON-RPC messages via stdin -- No HTTP, no headers, no session context -- No frontend protocol state information - -**The problem:** -- ✅ Backend stdio connection properly reused -- ✅ Backend process state maintained correctly -- ❌ Frontend SDK Server instance recreated per request -- ❌ Frontend protocol state reset to uninitialized -- ✅ Stateless backends (GitHub) work because they don't care -- ❌ Stateful backends (Serena) fail because they validate state - -**The limitation:** -- This is an SDK architectural pattern -- StreamableHTTPHandler doesn't support session persistence -- Backend connection pooling works, but SDK protocol state doesn't persist -- Would require SDK changes or bypassing StreamableHTTPHandler entirely diff --git a/docs/TOML_MODULE_REVIEW.md b/docs/TOML_MODULE_REVIEW.md deleted file mode 100644 index 04d7fbfb..00000000 --- a/docs/TOML_MODULE_REVIEW.md +++ /dev/null @@ -1,320 +0,0 @@ -# BurntSushi/toml Module Review - -**Date:** February 16, 2026 -**Module:** github.com/BurntSushi/toml v1.6.0 -**Status:** ✅ Implementation Excellent - Documentation Enhanced - -## Executive Summary - -The MCP Gateway's usage of the BurntSushi/toml module is **exemplary** and already implements nearly all best practices recommended in the Go Fan module review. This document confirms the implementation quality and documents the enhancement of in-code documentation. - -## Module Information - -- **Version:** v1.6.0 (latest, December 18, 2025) -- **Repository:** https://github.com/BurntSushi/toml -- **Stars:** 4,898 ⭐ -- **License:** MIT -- **Specification:** TOML 1.1 (default in v1.6.0+) -- **Last Update:** February 16, 2026 - -## Implementation Analysis - -### ✅ Already Implemented Features - -#### 1. TOML 1.1 Specification Support -- **Status:** Fully implemented -- **Location:** `internal/config/config_core.go` -- **Features Used:** - - Multi-line inline arrays (newlines in array definitions) - - Improved duplicate key detection - - Large float encoding with exponent syntax - -**Example from config files:** -```toml -[servers.github] -command = "docker" -args = [ - "run", "--rm", "-i", - "--name", "awmg-github-mcp" -] -``` - -#### 2. Column-Level Error Reporting (v1.5.0+ Feature) -- **Status:** Fully implemented -- **Location:** `internal/config/config_core.go:113-121` -- **Implementation:** - - Extracts both `Position.Line` and `Position.Col` from `ParseError` - - Dual type-check pattern (pointer and value) for compatibility - - Consistent error format: `"failed to parse TOML at line %d, column %d: %s"` - -**Code:** -```go -if perr, ok := err.(*toml.ParseError); ok { - return nil, fmt.Errorf("failed to parse TOML at line %d, column %d: %s", - perr.Position.Line, perr.Position.Col, perr.Message) -} -``` - -#### 3. Unknown Field Detection (Typo Detection) -- **Status:** Fully implemented with warning-based approach -- **Location:** `internal/config/config_core.go:145-161` -- **Implementation:** - - Uses `MetaData.Undecoded()` to detect keys not in struct - - Generates warnings instead of hard errors - - Logs to both debug logger and file logger - -**Design Rationale:** -- Maintains backward compatibility -- Allows gradual config migration -- Gateway starts even with typos (fail-soft behavior) -- Common typos like "prot" → "port" are caught and reported - -#### 4. Streaming Decoder Pattern -- **Status:** Fully implemented -- **Location:** `internal/config/config_core.go:104-106` -- **Benefits:** - - Memory-efficient for large configs - - Tests validate handling of 100+ server configs - -**Code:** -```go -decoder := toml.NewDecoder(file) -md, err := decoder.Decode(&cfg) -``` - -#### 5. Comprehensive Test Coverage -- **Status:** Excellent (31+ test cases) -- **Location:** `internal/config/config_test.go` -- **TOML-Specific Tests:** - - Parse error handling with line/column numbers - - Unknown key detection (typos) - - Multi-line array parsing - - Duplicate key detection (TOML 1.1 feature) - - Streaming large file handling - - Empty file validation - -**Example Test:** -```go -// TestLoadFromFile_InvalidTOMLDuplicateKey (line 1221) -// Tests TOML 1.1+ duplicate key detection -``` - -### Multi-Layer Validation Architecture - -The config package implements a sophisticated validation pipeline: - -1. **Parse-Time Validation** (config_core.go) - - TOML syntax validation - - Unknown field detection with warnings - -2. **Schema-Based Validation** (config_stdin.go + validation_schema.go) - - JSON schema validation against remote gh-aw schema - - Cached compilation for performance - -3. **Field-Level Validation** (validation.go + rules/rules.go) - - Port range (1-65535) - - Timeout positivity - - Mount format validation - - Absolute path validation - -4. **Variable Expansion Validation** (validation.go) - - `${VARIABLE_NAME}` expression expansion - - Environment variable existence checks - - Fails on undefined variables - -## What Was Changed - -### Documentation Enhancements Only - -**File:** `internal/config/config_core.go` - -#### 1. Package-Level Documentation -Added comprehensive package documentation explaining: -- TOML 1.1 specification support -- Column-level error reporting -- Duplicate key detection -- Metadata tracking -- Design patterns (streaming, error reporting, validation) -- Multi-layer validation approach - -#### 2. LoadFromFile() Function Documentation -Enhanced function documentation with: -- TOML 1.1 feature explanation -- Error handling capabilities -- Multi-line array usage example -- Metadata tracking description - -#### 3. Unknown Field Detection Comments -Added detailed comments explaining: -- Design decision rationale (warnings vs errors) -- Backward compatibility considerations -- User-friendliness balance -- MetaData.Undecoded() usage pattern - -### Memory Storage - -Stored three facts for future sessions: -1. TOML parsing implementation with v1.6.0+ features -2. Error reporting with line and column numbers -3. Validation philosophy (warnings vs hard errors) - -## Recommendations from Go Fan Report - -### ✅ Already Implemented - -1. **Verify TOML 1.1 Compatibility** - Confirmed using TOML 1.1 features -2. **Leverage Enhanced Error Reporting** - Column numbers included in errors -3. **Strict Decoding for Typo Detection** - Implemented with MetaData.Undecoded() -4. **Configuration Validation with Meta** - Comprehensive validation layers -5. **Enhanced Documentation** - Now enhanced with this review - -### ❌ Not Applicable / Not Needed - -1. **Use toml.Marshal() for Config Generation** - - Not needed: Gateway doesn't generate TOML configs programmatically - - Configs are user-authored TOML files - - If future feature requires marshaling, v1.4.0+ provides `toml.Marshal()` - -2. **Config Hot-Reload Support** - - Not needed: Gateway is designed for stable, startup-time configuration - - Would require complex synchronization with active connections - - Not requested by users - -## Best Practices Demonstrated - -### 1. Streaming for Memory Efficiency -Uses `toml.NewDecoder()` instead of `toml.DecodeFile()` for better control and memory usage with large configs. - -### 2. Metadata Utilization -Leverages `MetaData` return value for: -- Unknown field detection -- Validation tracking -- User-friendly error messages - -### 3. Dual Type-Check for ParseError -Handles both pointer and value types for robust error handling: -```go -if perr, ok := err.(*toml.ParseError); ok { ... } -if perr, ok := err.(toml.ParseError); ok { ... } -``` - -### 4. Warning-Based Validation -Balances strict validation with user-friendliness by using warnings for unknown fields instead of hard errors. - -### 5. Comprehensive Testing -31+ test cases covering: -- Valid configurations -- Parse errors -- Unknown fields -- Edge cases -- TOML 1.1 features -- Large file handling - -## TOML 1.1 Features Used - -### Multi-Line Inline Arrays -**Enabled by:** v1.6.0 making TOML 1.1 default - -**Example:** -```toml -[servers.github] -args = [ - "run", "--rm", "-i", - "--name", "awmg-github-mcp" -] -``` - -**Benefits:** -- Better readability for long arrays -- Easier maintenance -- Cleaner diffs in version control - -### Duplicate Key Detection -**Improved in:** v1.6.0 - -**Test Coverage:** `TestLoadFromFile_InvalidTOMLDuplicateKey` (line 1221) - -**Example Detected:** -```toml -[gateway] -port = 3000 -port = 8080 # Error: duplicate key detected -``` - -### Large Float Encoding -**Fixed in:** v1.6.0 - -Round-trip correctness for large floats using exponent syntax (e.g., `5e+22`). - -## Implementation Quality Assessment - -### Strengths -- ✅ **Modern:** Uses latest TOML 1.1 specification -- ✅ **Robust:** Comprehensive error handling with position information -- ✅ **User-Friendly:** Warning-based validation for typos -- ✅ **Efficient:** Streaming decoder for memory efficiency -- ✅ **Well-Tested:** 31+ test cases covering all scenarios -- ✅ **Documented:** Enhanced documentation of features and design decisions -- ✅ **Maintainable:** Clear separation of concerns across validation layers - -### No Weaknesses Identified -The implementation is exemplary and requires no functional changes. - -## Verification - -### Tests Executed -```bash -make agent-finished -``` - -**Results:** -- ✅ All 31+ config tests pass -- ✅ All validation tests pass -- ✅ No formatting issues -- ✅ No lint warnings - -### Files Modified -- `internal/config/config_core.go` (documentation only) - -### Zero Functional Changes -All changes are **documentation enhancements** only: -- Package-level comments -- Function documentation -- Inline comments explaining design decisions - -## Conclusion - -The MCP Gateway's implementation of BurntSushi/toml is **exemplary** and demonstrates: - -1. **Modern Best Practices:** Uses TOML 1.1 features appropriately -2. **Robust Error Handling:** Column-level position information in errors -3. **User-Friendly Design:** Warning-based typo detection -4. **Efficient Architecture:** Streaming decoder for large configs -5. **Comprehensive Testing:** 31+ test cases covering all scenarios -6. **Clear Documentation:** Enhanced with this review - -**No functional changes are needed.** The Go Fan report recommendations are nearly all implemented. This review enhanced the in-code documentation to make the excellent implementation patterns more visible to future maintainers. - -## Future Considerations - -If future requirements emerge, the module provides these unused capabilities: - -1. **toml.Marshal()** - For programmatic config generation (v1.4.0+) -2. **Custom Unmarshaler** - For complex type parsing (toml.Unmarshaler interface) -3. **Meta.Type()** - For type-specific validation if needed - -These features are available but not currently needed for the gateway's use cases. - -## References - -- **Module:** https://github.com/BurntSushi/toml -- **TOML Spec:** https://toml.io/en/v1.1.0 -- **Implementation:** `internal/config/config_core.go` -- **Tests:** `internal/config/config_test.go` -- **Go Fan Report:** Issue #[number] - BurntSushi/toml Module Review - ---- - -**Review Completed:** February 16, 2026 -**Reviewer:** Claude Agent (Go Fan Module Review) -**Status:** ✅ Implementation Excellent - No Changes Needed