Skip to content

OAuth config fields not wired to CLI flags / env vars (30 missing flags) #96

@BorisTyshkevich

Description

@BorisTyshkevich

Summary

The flag:"..." struct tags on every field of config.OAuthConfig claim a CLI flag and matching env-var source exists, and the README states "All configuration options can be set via environment variables." In reality, the entire OAuthConfig struct is only loadable via the YAML config file. None of the 30 OAuth fields have a CLI flag registered in cmd/altinity-mcp/main.go, and overrideWithCLIFlags does not override any OAuth field.

This forces operators to commit OAuth secrets like gating_secret_key to the rendered Helm Secret instead of injecting them from a separate Kubernetes Secret via valueFrom.secretKeyRef, which is the standard pattern.

Evidence

pkg/config/config.go (OAuthConfig, lines ~76–179) defines 30 fields, every one tagged like:

GatingSecretKey string `json:"gating_secret_key" yaml:"gating_secret_key" flag:"oauth-gating-secret-key" desc:"..."`

cmd/altinity-mcp/main.go registers ~30 CLI flags (ClickHouse, JWE, transport, TLS, logging, openapi, cors, tool-input-settings, blocked-query-clauses) — zero are OAuth flags.

overrideWithCLIFlags overrides ClickHouse / Server.TLS / Server.JWE / Logging / etc. — zero OAuth field overrides.

The flag:"..." struct tag is unread — no reflection-based loader (no envconfig, no viper; only urfave/cli/v3 with explicit per-flag enumeration).

Missing fields (the gap, today)

All 30 are missing from Flags: []cli.Flag{...} and from overrideWithCLIFlags:

oauth-access-token-ttl-seconds       oauth-mode
oauth-allowed-email-domains          oauth-openid-configuration-path
oauth-allowed-hosted-domains         oauth-protected-resource-metadata-path
oauth-audience                       oauth-public-auth-server-url
oauth-auth-code-ttl-seconds          oauth-public-resource-url
oauth-auth-url                       oauth-refresh-token-ttl-seconds
oauth-authorization-path             oauth-registration-path
oauth-authorization-server-metadata-path  oauth-require-email-verified
oauth-callback-path                  oauth-required-scopes
oauth-clickhouse-header-name         oauth-scopes
oauth-client-id                      oauth-token-path
oauth-client-secret                  oauth-token-url
oauth-enabled                        oauth-upstream-issuer-allowlist
oauth-gating-secret-key              oauth-userinfo-url
oauth-issuer
oauth-jwks-url

A few fields lack a flag: tag at all (OAuthConfig.ClaimsToHeaders, ServerConfig.Tools, ServerConfig.OpenAPI.Enabled, ServerConfig.OpenAPI.TLS, ServerConfig.DynamicTools) — they should be covered by the same fix so "every option overridable via env" is actually true.

Proposed fix — generic, not 30 boilerplate blocks

Adding 30 &cli.StringFlag{Name: "...", Sources: cli.EnvVars("MCP_...")} blocks plus 30 if cmd.IsSet(...) { cfg.Server.OAuth.X = cmd.String(...) } blocks would work but is ugly and still leaves the next added field unwired. A generic mechanism is preferable. Three options ordered by power:

Option A — YAML env-var interpolation (minimal, ~10 lines)

In LoadConfigFromFile, before yaml.Unmarshal:

data = []byte(os.ExpandEnv(string(data)))

Then operators write gating_secret_key: ${MCP_OAUTH_GATING_SECRET_KEY} in the YAML. Solves the immediate "keep secrets out of files" need without touching CLI plumbing. Doesn't allow running with no YAML at all.

Option B — kelseyhightower/envconfig (or caarlos0/env) for env loading

Add the dep, then:

// after LoadConfigFromFile and overrideWithCLIFlags
if err := envconfig.Process("MCP", &cfg); err != nil { return cfg, err }

The library walks the struct via reflection and binds env vars by struct path (e.g. MCP_SERVER_OAUTH_GATING_SECRET_KEY). Handles strings, ints, bools, time.Duration, slices (CSV), maps. Every existing and future tagged field gets env support automatically — zero per-field code. No CLI flag added; env-only.

Option C — reflection-based CLI flag auto-generation (most ambitious)

Walk config.Config{} recursively in main.go, and for every field with a flag:"..." tag build the right cli.*Flag value dynamically (mapping Go type → cli flag type) with Sources: cli.EnvVars(MCP_<UPPER_SNAKE>). The override step is similarly generic — set the field via reflection if the flag was set. This deletes the existing ~30 explicit flag blocks too. Net code shrinks. Future-proof: any new flag:"..." tag works automatically.

Recommended path: Option C if the project commits to "config struct is the single source of truth," Option B if a third-party dep is acceptable and CLI parity isn't needed, Option A as a one-day fix if neither is in scope today.

Whichever is chosen, the README claim ("All configuration options can be set via environment variables") becomes accurate.

Why this matters

Operational impact for the altinity-claude-otel deployment: gating_secret_key for the otel/github MCP servers currently must live as plaintext in deploy/<cluster>/mcp-values.yaml. With env-var support, the Helm chart's existing env: array can use valueFrom.secretKeyRef to pull the key from a separate Kubernetes Secret — keeping secrets out of any local YAML.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions