From dab1d3c81e60da5f4f7802d2bc473993edab85a5 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 10 Jun 2026 14:21:54 -0700 Subject: [PATCH] fix(cli): sort external auth env vars by numeric index (#26230) --- cli/server.go | 17 ++++++++++++++--- cli/server_test.go | 22 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/cli/server.go b/cli/server.go index 2a1fd623cd..4508db623e 100644 --- a/cli/server.go +++ b/cli/server.go @@ -2904,11 +2904,22 @@ func ReadExternalAuthProvidersFromEnv(environ []string) ([]codersdk.ExternalAuth // external auth providers. A prefix is provided to support the legacy // parsing of `GITAUTH` environment variables. func parseExternalAuthProvidersFromEnv(prefix string, environ []string) ([]codersdk.ExternalAuthConfig, error) { - // The index numbers must be in-order. - slices.Sort(environ) + parsed := serpent.ParseEnviron(environ, prefix) + + // Sort by numeric index so that PROVIDER_2 comes before PROVIDER_10. + // A lexicographic sort would order PROVIDER_10 between PROVIDER_1 and + // PROVIDER_2 and trip the "provider num skipped" check below. + slices.SortFunc(parsed, func(a, b serpent.EnvVar) int { + aIdx, _ := strconv.Atoi(strings.SplitN(a.Name, "_", 2)[0]) + bIdx, _ := strconv.Atoi(strings.SplitN(b.Name, "_", 2)[0]) + if aIdx != bIdx { + return aIdx - bIdx + } + return strings.Compare(a.Name, b.Name) + }) var providers []codersdk.ExternalAuthConfig - for _, v := range serpent.ParseEnviron(environ, prefix) { + for _, v := range parsed { tokens := strings.SplitN(v.Name, "_", 2) if len(tokens) != 2 { return nil, xerrors.Errorf("invalid env var: %s", v.Name) diff --git a/cli/server_test.go b/cli/server_test.go index 5b68c68588..3a7d8be4c8 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -107,6 +107,28 @@ func TestReadExternalAuthProvidersFromEnv(t *testing.T) { assert.Equal(t, "Google", providers[1].DisplayName) assert.Equal(t, "/icon/google.svg", providers[1].DisplayIcon) }) + + // Regression test: when more than 10 providers are configured the + // previous lexicographic sort placed PROVIDER_10 between PROVIDER_1 + // and PROVIDER_2 and the parser failed with "provider num skipped". + t.Run("MoreThan10Providers", func(t *testing.T) { + t.Parallel() + const count = 12 + environ := make([]string, 0, count*2) + for i := 0; i < count; i++ { + environ = append(environ, + fmt.Sprintf("CODER_EXTERNAL_AUTH_%d_ID=id-%d", i, i), + fmt.Sprintf("CODER_EXTERNAL_AUTH_%d_TYPE=type-%d", i, i), + ) + } + providers, err := cli.ReadExternalAuthProvidersFromEnv(environ) + require.NoError(t, err) + require.Len(t, providers, count) + for i := 0; i < count; i++ { + assert.Equal(t, fmt.Sprintf("id-%d", i), providers[i].ID) + assert.Equal(t, fmt.Sprintf("type-%d", i), providers[i].Type) + } + }) } func TestReadExternalAuthProvidersFromEnv_APIBaseURL(t *testing.T) {