Skip to content

Commit a890d17

Browse files
lpcoxlpcox
authored
Fail-fast OIDC env var validation at config load time (#3367)
When an HTTP server uses `auth.type: "github-oidc"`, missing `ACTIONS_ID_TOKEN_REQUEST_URL` was only surfaced on first request. The gateway would start, report healthy, then fail when a client actually hit the OIDC server. ### Changes - **`internal/config/validation.go`** — Check `ACTIONS_ID_TOKEN_REQUEST_URL` in `validateAuthConfig()` after confirming `auth.type == "github-oidc"`, returning a fail-fast validation error at config load time - **`internal/launcher/launcher.go`** — Pre-populate `serverErrors` during `New()` for OIDC-misconfigured servers so `/health` immediately reflects error state (defensive fallback for configs that bypass validation, e.g. tests) - **Tests** — Added cases for missing env var validation and early error recording; set env var in existing tests that construct valid OIDC configs ```go // validateAuthConfig now fails fast for missing OIDC env vars if auth.Type == "github-oidc" { if os.Getenv("ACTIONS_ID_TOKEN_REQUEST_URL") == "" { return rules.MissingRequired( "ACTIONS_ID_TOKEN_REQUEST_URL", "environment", authPath, "Server requires OIDC authentication but ACTIONS_ID_TOKEN_REQUEST_URL is not set. "+ "OIDC auth requires running in GitHub Actions with `permissions: { id-token: write }`") } } ``` > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `example.com` > - Triggering command: `/tmp/go-build1150468762/b514/launcher.test /tmp/go-build1150468762/b514/launcher.test -test.testlogfile=/tmp/go-build1150468762/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s insp�� uf@v1.36.11/type-errorsas om/tetratelabs/w-ifaceassert x_amd64/vet g_.a ce 64/pkg/tool/linu--format x_amd64/vet /usr�� g_.a 64/pkg/tool/linux_amd64/vet x_amd64/vet -I /known/anypb 64/pkg/tool/linux_amd64/vet x_amd64/vet` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build3631266208/b001/config.test /tmp/go-build3631266208/b001/config.test -test.testlogfile=/tmp/go-build3631266208/b001/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true ache/go/1.25.8/x64/src/runtime/c--gdwarf-5 YVt4/qIWeKIr-djJe-CXpYVt4 x_amd64/compile -I /tmp/go-build102-unsafeptr=false -I x_amd64/compile 7443�� ache/go/1.25.8/x64/src/runtime/c/tmp/go-build1027443045/b148/ ache/go/1.25.8/x64/src/crypto/in-imultiarch .13/x64/bin/as --gdwarf-5 --64 -o 9964951/b135/_x012.o` (dns block) > - Triggering command: `/tmp/go-build2092181885/b001/config.test /tmp/go-build2092181885/b001/config.test -test.testlogfile=/tmp/go-build2092181885/b001/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/net -trimpath x_amd64/vet -I /tmp/go-build102-qE /x64=/_/GOROOT x_amd64/vet ut-2�� 7443045/b147/_pkg_.a -I x_amd64/vet ctor --64 E=3 x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build1129750467/b001/config.test /tmp/go-build1129750467/b001/config.test -test.testlogfile=/tmp/go-build1129750467/b001/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true g_.a Wn3v/y-pYGV7LMnpJY6ujWn3v 64/pkg/tool/linux_amd64/vet . on --64 64/pkg/tool/linux_amd64/vet 9964�� 5vRKptRn1 .cfg 64/pkg/tool/linux_amd64/compile . 9964951/b187/ --64 64/pkg/tool/linux_amd64/compile` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build1150468762/b514/launcher.test /tmp/go-build1150468762/b514/launcher.test -test.testlogfile=/tmp/go-build1150468762/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s insp�� uf@v1.36.11/type-errorsas om/tetratelabs/w-ifaceassert x_amd64/vet g_.a ce 64/pkg/tool/linu--format x_amd64/vet /usr�� g_.a 64/pkg/tool/linux_amd64/vet x_amd64/vet -I /known/anypb 64/pkg/tool/linux_amd64/vet x_amd64/vet` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build1150468762/b514/launcher.test /tmp/go-build1150468762/b514/launcher.test -test.testlogfile=/tmp/go-build1150468762/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s insp�� uf@v1.36.11/type-errorsas om/tetratelabs/w-ifaceassert x_amd64/vet g_.a ce 64/pkg/tool/linu--format x_amd64/vet /usr�� g_.a 64/pkg/tool/linux_amd64/vet x_amd64/vet -I /known/anypb 64/pkg/tool/linux_amd64/vet x_amd64/vet` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build1150468762/b523/mcp.test /tmp/go-build1150468762/b523/mcp.test -test.testlogfile=/tmp/go-build1150468762/b523/testlog.txt -test.paniconexit0 -test.timeout=10m0s -goversion go1.25.8 -c=4 -nolocalimports -importcfg /tmp/go-build1150468762/b517/importcfg -pack /tmp/go-build1150468762/b517/_testmain.go /usr�� fg olang.org/protobuf@v1.36.11/inte-ifaceassert x_amd64/vet ache/go/1.25.8/x/usr/libexec/docker/docker-init .cfg de/node/bin/bash x_amd64/vet` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent) (admins only) > > </details>
2 parents 6333a1c + 361f687 commit a890d17

File tree

5 files changed

+70
-3
lines changed

5 files changed

+70
-3
lines changed

‎internal/config/config_stdin_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,6 +1027,9 @@ func TestConvertStdinConfig_TrustedBots(t *testing.T) {
10271027

10281028
// TestConvertStdinServerConfig_HTTPWithAuth tests that auth config is properly converted.
10291029
func TestConvertStdinServerConfig_HTTPWithAuth(t *testing.T) {
1030+
// OIDC validation now checks that ACTIONS_ID_TOKEN_REQUEST_URL is set
1031+
t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "https://token.actions.example.com")
1032+
10301033
t.Run("auth with explicit audience", func(t *testing.T) {
10311034
server := &StdinServerConfig{
10321035
Type: "http",

‎internal/config/validation.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,17 @@ func validateAuthConfig(auth *AuthConfig, serverName, jsonPath string) error {
258258
return rules.UnsupportedType("type", auth.Type, authPath, fmt.Sprintf("Unsupported auth type %q. Currently only \"github-oidc\" is supported", auth.Type))
259259
}
260260

261+
// Fail-fast: check that required OIDC environment variables are present.
262+
// This catches misconfigurations at config-load time rather than deferring
263+
// the error to the first request against this server.
264+
if os.Getenv("ACTIONS_ID_TOKEN_REQUEST_URL") == "" {
265+
logValidateServerFailed(serverName, "ACTIONS_ID_TOKEN_REQUEST_URL is not set")
266+
return rules.MissingRequired(
267+
"ACTIONS_ID_TOKEN_REQUEST_URL", "github-oidc", authPath,
268+
"Server requires OIDC authentication but ACTIONS_ID_TOKEN_REQUEST_URL is not set. "+
269+
"OIDC auth requires running in GitHub Actions with `permissions: { id-token: write }`")
270+
}
271+
261272
logValidation.Printf("Auth config validated: server=%s, type=%s", serverName, auth.Type)
262273
return nil
263274
}

‎internal/config/validation_test.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -956,6 +956,7 @@ func TestValidateAuthConfig(t *testing.T) {
956956
server *StdinServerConfig
957957
shouldErr bool
958958
errMsg string
959+
clearEnv bool // when true, ensure ACTIONS_ID_TOKEN_REQUEST_URL is unset
959960
}{
960961
{
961962
name: "valid github-oidc auth on http server",
@@ -1016,10 +1017,30 @@ func TestValidateAuthConfig(t *testing.T) {
10161017
shouldErr: true,
10171018
errMsg: "type",
10181019
},
1020+
{
1021+
name: "github-oidc rejected when ACTIONS_ID_TOKEN_REQUEST_URL is not set",
1022+
server: &StdinServerConfig{
1023+
Type: "http",
1024+
URL: "https://example.com/mcp",
1025+
Auth: &AuthConfig{
1026+
Type: "github-oidc",
1027+
Audience: "https://example.com",
1028+
},
1029+
},
1030+
shouldErr: true,
1031+
errMsg: "ACTIONS_ID_TOKEN_REQUEST_URL",
1032+
clearEnv: true,
1033+
},
10191034
}
10201035

10211036
for _, tt := range tests {
10221037
t.Run(tt.name, func(t *testing.T) {
1038+
if tt.clearEnv {
1039+
t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "")
1040+
} else {
1041+
// Ensure OIDC env var is set for tests that expect valid config
1042+
t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "https://token.actions.example.com")
1043+
}
10231044
err := validateStandardServerConfig("test-server", tt.server, "mcpServers.test-server")
10241045
if tt.shouldErr {
10251046
require.Error(t, err)

‎internal/launcher/launcher.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,19 @@ func New(ctx context.Context, cfg *config.Config) *Launcher {
7272
logger.LogInfo("startup", "GitHub Actions OIDC provider initialized")
7373
}
7474

75-
// Validate that all servers requiring OIDC have the provider available
75+
// Defensive fallback: pre-populate serverErrors for OIDC-misconfigured servers
76+
// so /health reports them as "error" immediately at startup. Normal config
77+
// validation (validateAuthConfig) catches this earlier; this handles cases
78+
// where validation was bypassed (e.g., tests constructing configs manually).
79+
serverErrors := make(map[string]string)
7680
for serverID, serverCfg := range cfg.Servers {
7781
if serverCfg.Auth != nil && serverCfg.Auth.Type == "github-oidc" && oidcProvider == nil {
78-
logger.LogError("startup",
82+
errMsg := fmt.Sprintf(
7983
"Server %q requires OIDC authentication but ACTIONS_ID_TOKEN_REQUEST_URL is not set. "+
8084
"OIDC auth is only available when running in GitHub Actions with `permissions: { id-token: write }`.",
8185
serverID)
86+
logger.LogError("startup", "%s", errMsg)
87+
serverErrors[serverID] = errMsg
8288
}
8389
}
8490

@@ -91,7 +97,7 @@ func New(ctx context.Context, cfg *config.Config) *Launcher {
9197
startupTimeout: startupTimeout,
9298
oidcProvider: oidcProvider,
9399
serverStartTimes: make(map[string]time.Time),
94-
serverErrors: make(map[string]string),
100+
serverErrors: serverErrors,
95101
}
96102
}
97103

‎internal/launcher/launcher_test.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -776,6 +776,32 @@ func TestLauncher_MixedAuthTypes(t *testing.T) {
776776
assert.Nil(t, l.oidcProvider, "OIDC provider should be nil without env vars")
777777
}
778778

779+
func TestLauncher_OIDCMissingEnvRecordsErrorAtStartup(t *testing.T) {
780+
ctx := context.Background()
781+
t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "")
782+
783+
cfg := &config.Config{
784+
Servers: map[string]*config.ServerConfig{
785+
"oidc-http": {
786+
Type: "http",
787+
URL: "https://secure.example.com/mcp",
788+
Auth: &config.AuthConfig{
789+
Type: "github-oidc",
790+
Audience: "https://secure.example.com",
791+
},
792+
},
793+
},
794+
}
795+
796+
l := New(ctx, cfg)
797+
defer l.Close()
798+
799+
// The launcher should record an error for the OIDC server at startup
800+
state := l.GetServerState("oidc-http")
801+
assert.Equal(t, "error", state.Status, "OIDC server with missing env should be in error state at startup")
802+
assert.Contains(t, state.LastError, "ACTIONS_ID_TOKEN_REQUEST_URL")
803+
}
804+
779805
func TestGetServerState_StoppedByDefault(t *testing.T) {
780806
cfg := newTestConfig(map[string]*config.ServerConfig{
781807
"test-server": {Type: "stdio", Command: "echo"},

0 commit comments

Comments
 (0)