diff --git a/internal/cmd/flags_core.go b/internal/cmd/flags_core.go index 52f53352a..99bc17b31 100644 --- a/internal/cmd/flags_core.go +++ b/internal/cmd/flags_core.go @@ -41,7 +41,11 @@ func registerCoreFlags(cmd *cobra.Command) { cmd.Flags().BoolVar(&validateEnv, "validate-env", false, "Validate execution environment (Docker, env vars) before starting") cmd.Flags().CountVarP(&verbosity, "verbose", "v", "Increase verbosity level: -v (info), -vv (debug), -vvv (trace)") - // Flag validation groups + // Flag validation groups. + // Note: MarkFlagsMutuallyExclusive only fires when flags are explicitly set on + // the command line. Neither --routed nor --unified has an env-var default, so + // there is no env-var bypass risk here; runtime logic defaults to routed mode + // when neither flag is set. cmd.MarkFlagsMutuallyExclusive("routed", "unified") cmd.MarkFlagsOneRequired("config", "config-stdin") } diff --git a/internal/cmd/flags_difc.go b/internal/cmd/flags_difc.go index 24aff8eaa..c56f65fd2 100644 --- a/internal/cmd/flags_difc.go +++ b/internal/cmd/flags_difc.go @@ -8,6 +8,7 @@ import ( "github.com/github/gh-aw-mcpg/internal/config" "github.com/github/gh-aw-mcpg/internal/difc" "github.com/github/gh-aw-mcpg/internal/envutil" + "github.com/github/gh-aw-mcpg/internal/guard" "github.com/spf13/cobra" ) @@ -37,14 +38,32 @@ func init() { }) } -// detectGuardWasm returns the baked-in container guard path if it exists, -// or empty string if not found (requiring the user to specify --guard-wasm). +// detectGuardWasm returns the path to the WASM guard module to use as the +// default for the --guard-wasm flag. It checks in order: +// 1. The baked-in container guard at containerGuardWasmPath. +// 2. The first .wasm file under $MCP_GATEWAY_WASM_GUARDS_DIR/github/. +// +// Returns an empty string when no guard can be auto-detected, which causes +// --guard-wasm to be marked as required. func detectGuardWasm() string { debugLog.Printf("Checking for baked-in guard at %s", containerGuardWasmPath) if _, err := os.Stat(containerGuardWasmPath); err == nil { debugLog.Printf("Auto-detected baked-in guard: %s", containerGuardWasmPath) return containerGuardWasmPath } + + // Fall back to MCP_GATEWAY_WASM_GUARDS_DIR/github/*.wasm if the env var is set. + // This allows operators who set MCP_GATEWAY_WASM_GUARDS_DIR to satisfy the + // --guard-wasm requirement without passing it explicitly on the CLI. + // Note: MarkFlagRequired only fires on CLI-set flags, so the env var must be + // translated to a concrete default here at flag-registration time. + if wasmPath, found, err := guard.FindServerWASMGuardFile("github"); err != nil { + debugLog.Printf("WASM guard discovery via %s failed: %v", guard.WASMGuardsDirEnvVar, err) + } else if found { + debugLog.Printf("Auto-detected guard via %s: %s", guard.WASMGuardsDirEnvVar, wasmPath) + return wasmPath + } + debugLog.Print("Baked-in guard not found, --guard-wasm flag required") return "" } diff --git a/internal/cmd/proxy_test.go b/internal/cmd/proxy_test.go index 63a1eb00f..2f8ac3406 100644 --- a/internal/cmd/proxy_test.go +++ b/internal/cmd/proxy_test.go @@ -2,6 +2,7 @@ package cmd import ( "os" + "path/filepath" "strings" "testing" @@ -14,7 +15,8 @@ import ( ) // TestDetectGuardWasm_FileNotFound tests that detectGuardWasm returns empty string -// when the baked-in guard at containerGuardWasmPath does not exist. +// when the baked-in guard at containerGuardWasmPath does not exist and +// MCP_GATEWAY_WASM_GUARDS_DIR is not set. // In standard test environments (non-container), the baked-in guard is absent. func TestDetectGuardWasm_FileNotFound(t *testing.T) { // Confirm the baked-in path does not exist in this environment @@ -23,13 +25,56 @@ func TestDetectGuardWasm_FileNotFound(t *testing.T) { t.Skipf("baked-in guard found at %s (running in container) — skipping 'not found' test", containerGuardWasmPath) } + // Ensure the env-var fallback is disabled too. + t.Setenv("MCP_GATEWAY_WASM_GUARDS_DIR", "") + result := detectGuardWasm() assert.Empty(t, result, "detectGuardWasm should return empty string when guard file does not exist") } +// TestDetectGuardWasm_ViaWasmGuardsDir verifies that detectGuardWasm falls back to +// MCP_GATEWAY_WASM_GUARDS_DIR/github/*.wasm when the baked-in container guard is absent. +func TestDetectGuardWasm_ViaWasmGuardsDir(t *testing.T) { + // Only meaningful when not running in a container with the baked-in guard. + if _, err := os.Stat(containerGuardWasmPath); err == nil { + t.Skipf("baked-in guard found at %s — fallback test not applicable", containerGuardWasmPath) + } + + // Create a temporary directory that mimics the MCP_GATEWAY_WASM_GUARDS_DIR layout: + // /github/00-github-guard.wasm + rootDir := t.TempDir() + githubDir := filepath.Join(rootDir, "github") + require.NoError(t, os.MkdirAll(githubDir, 0o755)) + wasmFile := filepath.Join(githubDir, "00-github-guard.wasm") + require.NoError(t, os.WriteFile(wasmFile, []byte("fake wasm"), 0o644)) + + t.Setenv("MCP_GATEWAY_WASM_GUARDS_DIR", rootDir) + + result := detectGuardWasm() + assert.Equal(t, wasmFile, result, + "detectGuardWasm should return the guard found under MCP_GATEWAY_WASM_GUARDS_DIR/github/") +} + +// TestDetectGuardWasm_WasmGuardsDirEmpty verifies that detectGuardWasm returns empty +// when MCP_GATEWAY_WASM_GUARDS_DIR is set but contains no .wasm files for github. +func TestDetectGuardWasm_WasmGuardsDirEmpty(t *testing.T) { + if _, err := os.Stat(containerGuardWasmPath); err == nil { + t.Skipf("baked-in guard found at %s — fallback test not applicable", containerGuardWasmPath) + } + + // Create a directory structure with no .wasm files. + rootDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(rootDir, "github"), 0o755)) + + t.Setenv("MCP_GATEWAY_WASM_GUARDS_DIR", rootDir) + + result := detectGuardWasm() + assert.Empty(t, result, + "detectGuardWasm should return empty when WASM_GUARDS_DIR/github/ contains no .wasm files") +} + // TestDetectGuardWasm_FileExists verifies that detectGuardWasm returns the // containerGuardWasmPath when that file is present on the filesystem. -// This test creates a temporary file at the expected path to simulate the container environment. func TestDetectGuardWasm_FileExists(t *testing.T) { // Skip if we cannot write to /guards/github/; test can only run where the // directory is pre-created (e.g. the production container image). @@ -265,13 +310,17 @@ func TestNewProxyCmd_OTLPServiceNameDefaultFromEnv(t *testing.T) { } // TestNewProxyCmd_GuardWasmRequiredWhenNoBakedInGuard verifies that --guard-wasm is -// marked as required when the baked-in container guard does not exist. +// marked as required when the baked-in container guard does not exist and +// MCP_GATEWAY_WASM_GUARDS_DIR is not set. func TestNewProxyCmd_GuardWasmRequiredWhenNoBakedInGuard(t *testing.T) { // This test is only meaningful when running outside a container. if _, err := os.Stat(containerGuardWasmPath); err == nil { t.Skipf("baked-in guard found at %s — in container, --guard-wasm is optional", containerGuardWasmPath) } + // Disable the env-var fallback so the flag is truly required. + t.Setenv("MCP_GATEWAY_WASM_GUARDS_DIR", "") + cmd := newProxyCmd() require.NotNil(t, cmd) diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 2664e33bf..61c5357b9 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -293,10 +293,7 @@ func run(cmd *cobra.Command, args []string) error { } // Determine mode (default to routed if neither flag is set) - mode := "routed" - if unifiedMode { - mode = "unified" - } + mode := resolveServerMode(routedMode, unifiedMode) debugLog.Printf("Server mode: %s, guards mode: %s", mode, cfg.DIFCMode) @@ -493,6 +490,17 @@ func run(cmd *cobra.Command, args []string) error { return nil } +func resolveServerMode(routed, unified bool) string { + switch { + case unified: + return "unified" + case routed: + return "routed" + default: + return "routed" + } +} + // Execute runs the root command func Execute() { if err := rootCmd.Execute(); err != nil { diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index e5aae89c9..86b016710 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -435,6 +435,40 @@ func TestPostRunCleanup(t *testing.T) { }) } +func TestResolveServerMode(t *testing.T) { + tests := []struct { + name string + routed bool + unified bool + want string + }{ + { + name: "defaults to routed when no flags are set", + routed: false, + unified: false, + want: "routed", + }, + { + name: "uses routed mode when routed flag is set", + routed: true, + unified: false, + want: "routed", + }, + { + name: "uses unified mode when unified flag is set", + routed: false, + unified: true, + want: "unified", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, resolveServerMode(tt.routed, tt.unified)) + }) + } +} + // TestWriteGatewayConfig_WildcardAddresses tests that wildcard bind addresses // (0.0.0.0 and ::) are replaced with 127.0.0.1 in the output client URLs, // since clients cannot connect to wildcard addresses directly.