From a4614d452b689f959efd4d5039c8c3011dd2a266 Mon Sep 17 00:00:00 2001 From: saif <11242541+saifsmailbox98@users.noreply.github.com> Date: Mon, 10 Aug 2026 06:33:22 +0100 Subject: [PATCH 1/6] feat(agent-proxy): support all machine identity auth methods start and connect could only authenticate with Universal Auth, so a proxy on a host that already had its own identity still needed a client id and secret provisioned on it. Both now accept every machine identity auth method the CLI supports, via --auth-method or INFISICAL_AUTH_METHOD. The strategy table moves from cmd/pam.go into util so all three commands offering --auth-method share it. Resolution order now matches pam agentic-access: ready-made token first, then machine identity auth, with pam's service-token and expiry guards on both commands. refreshProxyToken re-authenticates through the resolved strategy instead of calling UniversalAuthLogin directly, and credentialEnvKeys is derived from util.MachineIdentityAuthEnvVars so a new method cannot silently widen what a child agent inherits. That list is shared with agent-proxy run, whose scrub widens too. --- packages/cmd/agent_proxy.go | 172 ++++++++++++++++++++++++------ packages/cmd/agent_proxy_start.go | 61 +++++++---- packages/cmd/agent_proxy_test.go | 127 ++++++++++++++++++++++ packages/cmd/pam.go | 39 ++----- packages/util/auth.go | 130 ++++++++++++++++++++++ 5 files changed, 445 insertions(+), 84 deletions(-) diff --git a/packages/cmd/agent_proxy.go b/packages/cmd/agent_proxy.go index ad908bce..24c17772 100644 --- a/packages/cmd/agent_proxy.go +++ b/packages/cmd/agent_proxy.go @@ -1,6 +1,7 @@ package cmd import ( + "context" "errors" "fmt" "net/url" @@ -18,6 +19,7 @@ import ( "github.com/Infisical/infisical-merge/packages/util" "github.com/fatih/color" "github.com/go-resty/resty/v2" + infisicalSdk "github.com/infisical/go-sdk" "github.com/posthog/posthog-go" "github.com/rs/zerolog/log" "github.com/spf13/cobra" @@ -78,12 +80,24 @@ var proxyEnvKeys = []string{ "OPENCLAW_PROXY_URL", } -// Stripped so the agent never sees the long-lived MI credentials, only the scoped short-lived JWT set below. -var credentialEnvKeys = []string{ - util.INFISICAL_UNIVERSAL_AUTH_CLIENT_ID_NAME, - util.INFISICAL_UNIVERSAL_AUTH_CLIENT_SECRET_NAME, +// Stripped so the agent is handed only the scoped short-lived JWT set below, and not whatever the +// parent authenticated with. +// +// Derived from util.MachineIdentityAuthEnvVars rather than listed here, so that a machine identity +// auth method added to the CLI cannot quietly widen what the agent inherits. INFISICAL_AUTH_METHOD is +// in that list and matters as much as the credentials: connect deliberately sets INFISICAL_TOKEN for +// the agent, and a stray auth method would send a CLI call inside the agent down a machine identity +// login instead of using that token. +// +// How much this is worth depends on the method. For universal-auth, ldap-auth and the jwt-based +// methods the credential is a value that lives only in the environment, so removing it is a real +// boundary. For kubernetes, aws-iam, azure and gcp-id-token the credential is an ambient capability of +// the host (a service account token file, an instance metadata endpoint) that the agent can reach +// whether or not it inherits these; there the isolation comes from the agent running on a different +// host to the proxy, and from the agent identity holding Proxy and nothing else. +var credentialEnvKeys = append([]string{ util.INFISICAL_UNIVERSAL_AUTH_ACCESS_TOKEN_NAME, -} +}, util.MachineIdentityAuthEnvVars...) // Addresses of host IPC endpoints. Not secret values, but handing them to the agent points it at // sockets it can use: the SSH/GPG agents as a signing oracle, and the session bus, where @@ -210,37 +224,134 @@ func telemetryAgentName(args []string) string { return filepath.Base(args[0]) } -func universalAuthCredentialSource(cmd *cobra.Command) string { - if cmd.Flags().Changed("client-id") { - return "universal-auth-flag" +// resolveAgentProxyStaticToken returns a token the operator fetched elsewhere, or nil if none was +// given. The two guards match what `pam agentic-access` applies to the same input: a service token +// authenticates to the secrets API but not as a machine identity, so it cannot stand in for one here, +// and an expired token is worth catching now rather than at the agent's first proxied request, which +// is where it would otherwise surface as an unexplained 403. +func resolveAgentProxyStaticToken(cmd *cobra.Command, subject string) *models.TokenDetails { + token, err := util.GetInfisicalToken(cmd) + if err != nil { + util.HandleError(err, "Unable to resolve authentication") } - return "universal-auth-env" + if token == nil { + return nil + } + if token.Type == util.SERVICE_TOKEN_IDENTIFIER { + util.PrintErrorMessageAndExit("The agent proxy does not support service tokens. Use a machine identity access token, or authenticate with --auth-method.") + } + failIfTokenExpired(token.Token, subject) + return token } -// Returns the token and a label for the branch that produced it. -func resolveAgentToken(cmd *cobra.Command) (*models.TokenDetails, string) { - clientID, _ := util.GetCmdFlagOrEnvWithDefaultValue(cmd, "client-id", []string{util.INFISICAL_UNIVERSAL_AUTH_CLIENT_ID_NAME}, "") - clientSecret, _ := util.GetCmdFlagOrEnvWithDefaultValue(cmd, "client-secret", []string{util.INFISICAL_UNIVERSAL_AUTH_CLIENT_SECRET_NAME}, "") +// resolveAgentProxyLogin works out how this command's machine identity authenticates, and returns a +// function that performs one authentication per call along with a label naming the branch for +// telemetry. Both are nil when nothing was configured. +// +// Callers reach this only after resolveAgentProxyStaticToken has come up empty, which is the order +// `pam agentic-access` resolves in too: a ready-made token first, then a machine identity +// authenticating with its own credentials. Within this function the order is an explicit +// --auth-method, then client credentials on their own as the shorthand for universal-auth. +// +// Each call builds its own SDK client and lets it go again. The SDK cannot be told to skip its +// background token lifecycle (Config.AutoTokenRefresh is a bool tagged `default:"true"`, and +// setDefaults rewrites any false bool back to its default). Left alive, that goroutine would +// re-authenticate on its own schedule alongside the renewal the caller is already driving, doubling +// this identity's authentication events; a client that dies with its login cannot. Separately, and +// the reason nothing here returns the SDK's own getter: that goroutine renews into a field +// Auth().GetAccessToken reads without taking the client's mutex, and both commands read the token +// from a per-request path. +func resolveAgentProxyLogin(cmd *cobra.Command) (login func() (infisicalSdk.MachineIdentityCredential, error), source string) { + authMethod, err := util.ResolveAuthMethod(cmd) + if err != nil { + util.HandleError(err, "Unable to parse auth-method flag") + } - if clientID != "" && clientSecret != "" { - loginResp, err := util.UniversalAuthLogin(clientID, clientSecret) - if err != nil { - util.HandleError(err, "Failed to authenticate the agent machine identity") + if authMethod == "" { + clientID, _ := util.GetCmdFlagOrEnvWithDefaultValue(cmd, "client-id", []string{util.INFISICAL_UNIVERSAL_AUTH_CLIENT_ID_NAME}, "") + clientSecret, _ := util.GetCmdFlagOrEnvWithDefaultValue(cmd, "client-secret", []string{util.INFISICAL_UNIVERSAL_AUTH_CLIENT_SECRET_NAME}, "") + if clientID == "" && clientSecret == "" { + return nil, "" + } + // Half a credential is a mistake rather than a request for a different method, and saying which + // half is missing beats listing every method the command accepts. + if clientID == "" { + util.HandleError(fmt.Errorf("client id required; pass --client-id or set %s", util.INFISICAL_UNIVERSAL_AUTH_CLIENT_ID_NAME)) + } + if clientSecret == "" { + util.HandleError(fmt.Errorf("client secret required; pass --client-secret or set %s", util.INFISICAL_UNIVERSAL_AUTH_CLIENT_SECRET_NAME)) } - return &models.TokenDetails{ - Type: util.UNIVERSAL_AUTH_TOKEN_IDENTIFIER, - Token: loginResp.AccessToken, - }, universalAuthCredentialSource(cmd) + authMethod = string(util.AuthStrategy.UNIVERSAL_AUTH) } - token, err := util.GetInfisicalToken(cmd) + // Rejected here rather than at the first login, so that a typo fails before the command has done + // anything else. + if err := util.ValidateAuthMethod(authMethod); err != nil { + util.PrintErrorMessageAndExit(err.Error()) + } + + customHeaders, err := util.GetInfisicalCustomHeadersMap() if err != nil { - util.HandleError(err, "Unable to resolve authentication") + util.HandleError(err, "Unable to get custom headers") } - if token == nil { - util.HandleError(fmt.Errorf("authentication required; provide --client-id/--client-secret, env vars, or a token")) + + login = func() (infisicalSdk.MachineIdentityCredential, error) { + ctx, cancel := context.WithCancel(cmd.Context()) + defer cancel() + + client := infisicalSdk.NewInfisicalClient(ctx, infisicalSdk.Config{ + SiteUrl: config.INFISICAL_URL, + UserAgent: api.USER_AGENT, + CustomHeaders: customHeaders, + }) + authenticate, err := util.MachineIdentityLoginFunc(cmd, client, authMethod) + if err != nil { + return infisicalSdk.MachineIdentityCredential{}, err + } + credential, err := authenticate() + if err != nil { + return infisicalSdk.MachineIdentityCredential{}, err + } + if credential.AccessToken == "" { + return infisicalSdk.MachineIdentityCredential{}, fmt.Errorf("authenticating with %s returned no access token", authMethod) + } + return credential, nil + } + return login, authMethodCredentialSource(cmd, authMethod) +} + +// authMethodCredentialSource labels how the identity was configured, for telemetry. The method alone +// would not distinguish the two ways of asking for the same one. +func authMethodCredentialSource(cmd *cobra.Command, authMethod string) string { + if cmd.Flags().Changed("auth-method") || cmd.Flags().Changed("client-id") { + return authMethod + "-flag" + } + return authMethod + "-env" +} + +// Returns the token and a label for the branch that produced it. +// +// Whichever branch wins, the token is frozen here: connect writes it into the agent's environment and +// execs, and nothing can reach in and rewrite it afterwards. When it expires the agent's requests +// start coming back 403 and the agent has to be relaunched. +func resolveAgentToken(cmd *cobra.Command) (*models.TokenDetails, string) { + if token := resolveAgentProxyStaticToken(cmd, "the provided token"); token != nil { + return token, "token" + } + + login, source := resolveAgentProxyLogin(cmd) + if login == nil { + util.HandleError(fmt.Errorf("authentication required; pass --auth-method [%s] with that method's credentials, --client-id/--client-secret, or a token", util.MachineIdentityAuthMethods)) + } + + credential, err := login() + if err != nil { + util.HandleError(err, "Failed to authenticate the agent machine identity") } - return token, "token" + return &models.TokenDetails{ + Type: util.UNIVERSAL_AUTH_TOKEN_IDENTIFIER, + Token: credential.AccessToken, + }, source } // Builds http://:/:@host:port (username=projectId, password="/:", jwt last). @@ -474,17 +585,16 @@ func init() { agentProxyConnectCmd.Flags().StringP("env", "e", "", "environment slug to fetch proxied services and secrets from (falls back to INFISICAL_ENVIRONMENT or .infisical.json)") agentProxyConnectCmd.Flags().String("path", "/", "secret path (folder) scope (falls back to INFISICAL_SECRET_PATH or defaultSecretPath in .infisical.json)") agentProxyConnectCmd.Flags().String("projectId", "", "project id (falls back to INFISICAL_PROJECT_ID or .infisical.json)") - agentProxyConnectCmd.Flags().String("client-id", "", "universal auth client id for the agent machine identity") - agentProxyConnectCmd.Flags().String("client-secret", "", "universal auth client secret for the agent machine identity") - agentProxyConnectCmd.Flags().String("token", "", "Fetch secrets using service token or machine identity access token") + util.RegisterMachineIdentityAuthFlags(agentProxyConnectCmd, "agent") + agentProxyConnectCmd.Flags().String("token", "", "machine identity access token to use instead of authenticating; takes precedence over --auth-method") agentProxyConnectCmd.Flags().String("no-proxy", "", "additional comma-separated hosts to bypass the proxy (always merged with localhost,127.0.0.1)") agentProxyConnectCmd.Flags().Bool("allow-readable-brokered-secrets", false, "start even if the agent can read secrets that proxied services broker to it (bypasses a misconfiguration guardrail; falls back to INFISICAL_AGENT_PROXY_ALLOW_READABLE_BROKERED_SECRETS)") agentProxyStartCmd.Flags().Int("port", 17322, "port for the agent proxy to listen on") agentProxyStartCmd.Flags().String("unmatched-host", "allow", "policy for hosts with no proxied service: allow | block") agentProxyStartCmd.Flags().Int("poll-interval", 60, "seconds between permission/credential refreshes for active agents") - agentProxyStartCmd.Flags().String("client-id", "", "universal auth client id for the agent proxy machine identity") - agentProxyStartCmd.Flags().String("client-secret", "", "universal auth client secret for the agent proxy machine identity") + util.RegisterMachineIdentityAuthFlags(agentProxyStartCmd, "agent proxy") + agentProxyStartCmd.Flags().String("token", "", "machine identity access token to use instead of authenticating; takes precedence over --auth-method, and the proxy cannot renew it") agentProxyStartCmd.Flags().String("log-format", "console", "log output format: console | json") agentProxyStartCmd.Flags().String("log-file", "", "also write json logs to this file (in addition to the console/json stream)") diff --git a/packages/cmd/agent_proxy_start.go b/packages/cmd/agent_proxy_start.go index 9185cb2f..3676654a 100644 --- a/packages/cmd/agent_proxy_start.go +++ b/packages/cmd/agent_proxy_start.go @@ -9,6 +9,7 @@ import ( "github.com/Infisical/infisical-merge/packages/telemetry" "github.com/Infisical/infisical-merge/packages/util" "github.com/fatih/color" + infisicalSdk "github.com/infisical/go-sdk" "github.com/posthog/posthog-go" "github.com/rs/zerolog/log" "github.com/spf13/cobra" @@ -33,32 +34,49 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { } log.Logger = log.Output(logWriter) - clientID, err := util.GetCmdFlagOrEnvWithDefaultValue(cmd, "client-id", []string{util.INFISICAL_UNIVERSAL_AUTH_CLIENT_ID_NAME}, "") - if err != nil || clientID == "" { - util.HandleError(fmt.Errorf("agent proxy credentials required; set INFISICAL_UNIVERSAL_AUTH_CLIENT_ID / _SECRET or pass --client-id / --client-secret")) - } - clientSecret, err := util.GetCmdFlagOrEnvWithDefaultValue(cmd, "client-secret", []string{util.INFISICAL_UNIVERSAL_AUTH_CLIENT_SECRET_NAME}, "") - if err != nil || clientSecret == "" { - util.HandleError(fmt.Errorf("agent proxy client secret required")) - } + // Same order as connect and as `pam agentic-access`: a ready-made token first, then a machine + // identity authenticating with its own credentials. + var accessToken string + var accessTokenTTL int + var login func() (infisicalSdk.MachineIdentityCredential, error) + credentialSource := "token" - loginResp, err := util.UniversalAuthLogin(clientID, clientSecret) - if err != nil { - util.HandleError(err, "Failed to authenticate the agent proxy machine identity") + if token := resolveAgentProxyStaticToken(cmd, "the provided token"); token != nil { + // A token minted elsewhere. Nothing here can renew it, and the proxy is meant to outlive any + // single token, so this is worth saying out loud rather than leaving to be discovered when every + // request starts failing at once. + accessToken = token.Token + log.Warn().Msg("The agent proxy is running on a fixed token, which it cannot renew. It will stop working when that token expires; use --auth-method or client credentials to have it re-authenticate on its own.") + } else { + login, credentialSource = resolveAgentProxyLogin(cmd) + if login == nil { + util.HandleError(fmt.Errorf("agent proxy credentials required; pass --auth-method [%s] with that method's credentials, --client-id/--client-secret, or a token", util.MachineIdentityAuthMethods)) + } + credential, err := login() + if err != nil { + util.HandleError(err, "Failed to authenticate the agent proxy machine identity") + } + accessToken = credential.AccessToken + accessTokenTTL = int(credential.ExpiresIn) } - Telemetry.SetActor(telemetry.IdentityClaimsFromToken(loginResp.AccessToken)) + Telemetry.SetActor(telemetry.IdentityClaimsFromToken(accessToken)) Telemetry.CaptureEvent("cli-command:agent-proxy start", posthog.NewProperties(). Set("version", util.CLI_VERSION). Set("unmatchedHost", unmatchedHost). Set("pollInterval", pollInterval). - Set("credentialSource", universalAuthCredentialSource(cmd))) + Set("credentialSource", credentialSource)) log.Info().Msg(color.GreenString("Agent proxy authenticated; starting MITM proxy")) + // atomic.Value rather than the SDK's own getter: the proxy reads this on the per-request path while + // the refresher writes it, and Auth().GetAccessToken reads the SDK's token field without holding + // the client's mutex. See resolveAgentProxyLogin. var proxyToken atomic.Value - proxyToken.Store(loginResp.AccessToken) - go refreshProxyToken(&proxyToken, clientID, clientSecret, loginResp.AccessTokenTTL) + proxyToken.Store(accessToken) + if login != nil { + go refreshProxyToken(&proxyToken, login, accessTokenTTL) + } err = agentproxy.Start(agentproxy.Options{ Port: port, @@ -71,7 +89,10 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { } } -func refreshProxyToken(token *atomic.Value, clientID, clientSecret string, ttlSeconds int) { +// refreshProxyToken re-authenticates the proxy's machine identity on a schedule, whatever method it +// uses: login performs a full authentication rather than a renewal, so nothing here depends on which +// one it is. +func refreshProxyToken(token *atomic.Value, login func() (infisicalSdk.MachineIdentityCredential, error), ttlSeconds int) { const retryInterval = 30 * time.Second halfTTL := func() time.Duration { @@ -86,15 +107,15 @@ func refreshProxyToken(token *atomic.Value, clientID, clientSecret string, ttlSe for { time.Sleep(wait) - loginResp, err := util.UniversalAuthLogin(clientID, clientSecret) + credential, err := login() if err != nil { log.Warn().Err(err).Msgf("Failed to refresh agent proxy token, retrying in %s", retryInterval) wait = retryInterval continue } - token.Store(loginResp.AccessToken) - if loginResp.AccessTokenTTL > 0 { - ttlSeconds = loginResp.AccessTokenTTL + token.Store(credential.AccessToken) + if credential.ExpiresIn > 0 { + ttlSeconds = int(credential.ExpiresIn) } wait = halfTTL() } diff --git a/packages/cmd/agent_proxy_test.go b/packages/cmd/agent_proxy_test.go index e5a78bd0..7ba88aad 100644 --- a/packages/cmd/agent_proxy_test.go +++ b/packages/cmd/agent_proxy_test.go @@ -2,11 +2,138 @@ package cmd import ( "reflect" + "strings" "testing" "github.com/Infisical/infisical-merge/packages/models" + "github.com/Infisical/infisical-merge/packages/util" + "github.com/spf13/cobra" ) +// Every command offering --auth-method has to declare all of the flags the strategies read, including +// the ones only some methods use. GetCmdFlagOrEnv looks a flag up before it looks at the environment +// and errors on a name the command never declared, so a missing registration breaks that method even +// for someone who only ever set its environment variable, and does it at authentication time rather +// than at startup. +func TestMachineIdentityAuthFlagsAreRegistered(t *testing.T) { + commands := map[string]*cobra.Command{ + "agent-proxy start": agentProxyStartCmd, + "agent-proxy connect": agentProxyConnectCmd, + "pam agentic-access": pamAgenticAccessCmd, + } + + for name, cmd := range commands { + t.Run(name, func(t *testing.T) { + for _, flag := range util.MachineIdentityAuthFlags { + if cmd.Flags().Lookup(flag) == nil { + t.Errorf("%s does not register --%s", name, flag) + } + } + }) + } +} + +// Which credentials win when more than one is present, asserted on the branch resolveAgentProxyLogin +// picks rather than on a login it cannot perform in a test. Only the non-erroring paths are covered: +// the rest of packages/cmd exits the process on a bad input, and this follows that. +func TestResolveAgentProxyLoginPrecedence(t *testing.T) { + tests := []struct { + name string + env map[string]string + wantLogin bool + wantSource string + }{ + { + name: "nothing configured leaves the caller on its token fallback", + wantLogin: false, + }, + { + name: "client credentials on their own mean universal-auth", + env: map[string]string{ + util.INFISICAL_UNIVERSAL_AUTH_CLIENT_ID_NAME: "id", + util.INFISICAL_UNIVERSAL_AUTH_CLIENT_SECRET_NAME: "secret", + }, + wantLogin: true, + wantSource: "universal-auth-env", + }, + { + name: "an explicit auth method is used", + env: map[string]string{util.INFISICAL_AUTH_METHOD_NAME: "aws-iam"}, + wantLogin: true, + wantSource: "aws-iam-env", + }, + { + name: "an explicit auth method beats client credentials", + env: map[string]string{ + util.INFISICAL_AUTH_METHOD_NAME: "aws-iam", + util.INFISICAL_UNIVERSAL_AUTH_CLIENT_ID_NAME: "id", + util.INFISICAL_UNIVERSAL_AUTH_CLIENT_SECRET_NAME: "secret", + }, + wantLogin: true, + wantSource: "aws-iam-env", + }, + { + // "user" is how some callers spell the human login, and there is no such thing here, so it + // has to fall through rather than be treated as a method name. + name: "the user pseudo-method is not a machine identity", + env: map[string]string{util.INFISICAL_AUTH_METHOD_NAME: "user"}, + wantLogin: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for _, key := range append([]string{}, util.MachineIdentityAuthEnvVars...) { + t.Setenv(key, "") + } + for key, value := range tt.env { + t.Setenv(key, value) + } + + login, source := resolveAgentProxyLogin(agentProxyConnectCmd) + + if (login != nil) != tt.wantLogin { + t.Fatalf("login != nil = %v, want %v", login != nil, tt.wantLogin) + } + if source != tt.wantSource { + t.Errorf("source = %q, want %q", source, tt.wantSource) + } + }) + } +} + +// The scrub list is derived from the auth env vars rather than written out, so that adding a machine +// identity auth method cannot quietly widen what the agent inherits. This checks the derivation held. +func TestCredentialEnvKeysCoverEveryAuthEnvVar(t *testing.T) { + scrubbed := make(map[string]bool, len(credentialEnvKeys)) + for _, key := range credentialEnvKeys { + scrubbed[key] = true + } + + for _, envVar := range util.MachineIdentityAuthEnvVars { + if !scrubbed[envVar] { + t.Errorf("%s reaches the agent; add it to credentialEnvKeys", envVar) + } + } +} + +// buildAgentEnv is what stands between the parent's credentials and the agent, so the guarantee is +// worth asserting on the built environment and not just on the list feeding it. +func TestBuildAgentEnvScrubsMachineIdentityCredentials(t *testing.T) { + for _, envVar := range append([]string{util.INFISICAL_UNIVERSAL_AUTH_ACCESS_TOKEN_NAME}, util.MachineIdentityAuthEnvVars...) { + t.Setenv(envVar, "leaked-"+envVar) + } + + env := buildAgentEnv("http://proxy:17322", "/tmp/ca.pem", "agent-jwt", "", nil, nil) + + for _, entry := range env { + key, value, _ := strings.Cut(entry, "=") + if strings.HasPrefix(value, "leaked-") { + t.Errorf("%s reached the agent's environment", key) + } + } +} + func TestReadableBrokeredSecrets(t *testing.T) { brokered := map[string]struct{}{"STRIPE_API_KEY": {}, "GITHUB_TOKEN": {}} real := func(keys ...string) []models.SingleEnvironmentVariable { diff --git a/packages/cmd/pam.go b/packages/cmd/pam.go index 6799b72d..738c4034 100644 --- a/packages/cmd/pam.go +++ b/packages/cmd/pam.go @@ -268,30 +268,19 @@ func resolveAgentAccessToken(cmd *cobra.Command) (accessToken func() string, sto return func() string { return jwt }, func() {} } -// agenticAuthMethods lists what --auth-method accepts, for the flag help and for the error when it -// doesn't. Kept next to the strategy table below so the two cannot drift apart. -const agenticAuthMethods = "universal-auth, kubernetes, azure, gcp-id-token, gcp-iam, aws-iam, oidc-auth, jwt-auth, ldap-auth" - // authenticateMachineIdentity authenticates a machine identity with its own credentials when // --auth-method (or INFISICAL_AUTH_METHOD) names one, returning a getter for its current access token // and a stop function. Both are nil when no method was asked for, which leaves the caller on its // stored-login path. func authenticateMachineIdentity(cmd *cobra.Command) (accessToken func() string, stop func()) { - authMethod, err := util.GetCmdFlagOrEnvWithDefaultValue(cmd, "auth-method", []string{util.INFISICAL_AUTH_METHOD_NAME}, "") + authMethod, err := util.ResolveAuthMethod(cmd) if err != nil { util.HandleError(err, "Unable to parse auth-method flag") } - - // "user" is spelled out by some callers to mean the human login, which is already the fallback. - if authMethod == "" || authMethod == "user" { + if authMethod == "" { return nil, nil } - valid, strategy := util.IsAuthMethodValid(authMethod, false) - if !valid { - util.PrintErrorMessageAndExit(fmt.Sprintf("Invalid auth method %q. Supported: %s.", authMethod, agenticAuthMethods)) - } - customHeaders, err := util.GetInfisicalCustomHeadersMap() if err != nil { util.HandleError(err, "Unable to get custom headers") @@ -308,26 +297,10 @@ func authenticateMachineIdentity(cmd *cobra.Command) (accessToken func() string, CustomHeaders: customHeaders, }) - authenticator := util.NewSdkAuthenticator(client, cmd) - strategies := map[util.AuthStrategyType]func() (infisicalSdk.MachineIdentityCredential, error){ - util.AuthStrategy.UNIVERSAL_AUTH: authenticator.HandleUniversalAuthLogin, - util.AuthStrategy.KUBERNETES_AUTH: authenticator.HandleKubernetesAuthLogin, - util.AuthStrategy.AZURE_AUTH: authenticator.HandleAzureAuthLogin, - util.AuthStrategy.GCP_ID_TOKEN_AUTH: authenticator.HandleGcpIdTokenAuthLogin, - util.AuthStrategy.GCP_IAM_AUTH: authenticator.HandleGcpIamAuthLogin, - util.AuthStrategy.AWS_IAM_AUTH: authenticator.HandleAwsIamAuthLogin, - util.AuthStrategy.OIDC_AUTH: authenticator.HandleOidcAuthLogin, - util.AuthStrategy.JWT_AUTH: authenticator.HandleJwtAuthLogin, - util.AuthStrategy.LDAP_AUTH: authenticator.HandleLdapAuthLogin, - } - - // IsAuthMethodValid accepts every strategy in util.AVAILABLE_AUTH_STRATEGIES, so a method added - // there without a handler here has to be reported. Indexing a missing key yields a nil func, and - // calling that panics, which is what the same table does elsewhere in the CLI for ldap-auth. - login, supported := strategies[strategy] - if !supported { + login, err := util.MachineIdentityLoginFunc(cmd, client, authMethod) + if err != nil { cancel() - util.PrintErrorMessageAndExit(fmt.Sprintf("Auth method %q is not supported here. Supported: %s.", authMethod, agenticAuthMethods)) + util.PrintErrorMessageAndExit(err.Error()) } credential, err := login() @@ -360,7 +333,7 @@ func init() { // Machine identity auth. Every input the util.SdkAuthenticator handlers read has to be declared // here, not just documented: GetCmdFlagOrEnv asks cobra for the flag first and fails on an // undeclared one, so the environment variable fallbacks are unreachable without these. - pamAgenticAccessCmd.Flags().String("auth-method", "", "Authenticate as a machine identity with its own credentials instead of a ready-made --token ["+agenticAuthMethods+"]") + pamAgenticAccessCmd.Flags().String("auth-method", "", "Authenticate as a machine identity with its own credentials instead of a ready-made --token ["+util.MachineIdentityAuthMethods+"]") pamAgenticAccessCmd.Flags().String("client-id", "", "Client id for universal auth") pamAgenticAccessCmd.Flags().String("client-secret", "", "Client secret for universal auth") pamAgenticAccessCmd.Flags().String("machine-identity-id", "", "Machine identity id for the kubernetes, azure, gcp-id-token, gcp-iam, aws-iam, oidc-auth, jwt-auth and ldap-auth methods") diff --git a/packages/util/auth.go b/packages/util/auth.go index a40f6022..7cb1ddec 100644 --- a/packages/util/auth.go +++ b/packages/util/auth.go @@ -275,3 +275,133 @@ func (a *SdkAuthenticator) HandleLdapAuthLogin() (credential infisicalSdk.Machin return a.infisicalClient.Auth().WithOrganizationSlug(organizationSlug).LdapAuthLogin(identityId, ldapUsername, ldapPassword) } + +// MachineIdentityAuthMethods lists what --auth-method accepts, for flag help and for the error when +// it is given something else. Kept beside the strategy table in MachineIdentityLoginFunc so the two +// cannot drift apart. +const MachineIdentityAuthMethods = "universal-auth, kubernetes, azure, gcp-id-token, gcp-iam, aws-iam, oidc-auth, jwt-auth, ldap-auth" + +// MachineIdentityAuthFlags are the flags the strategies above read. Every one has to be registered on +// a command that offers --auth-method: GetCmdFlagOrEnv looks the flag up before it looks at the +// environment and returns an error for a name the command never declared, so a missing registration +// breaks that method even for someone who only ever set its environment variable. +var MachineIdentityAuthFlags = []string{ + "auth-method", + "client-id", + "client-secret", + "machine-identity-id", + "service-account-token-path", + "service-account-key-file-path", + "jwt", + "ldap-username", + "ldap-password", + "organization-slug", +} + +// MachineIdentityAuthEnvVars are the environment variables the strategies read. A command that hands +// an environment to a child process it does not trust scrubs these, so that adding a strategy cannot +// quietly widen what the child inherits. +var MachineIdentityAuthEnvVars = []string{ + INFISICAL_AUTH_METHOD_NAME, + INFISICAL_UNIVERSAL_AUTH_CLIENT_ID_NAME, + INFISICAL_UNIVERSAL_AUTH_CLIENT_SECRET_NAME, + INFISICAL_MACHINE_IDENTITY_ID_NAME, + INFISICAL_KUBERNETES_SERVICE_ACCOUNT_TOKEN_NAME, + INFISICAL_GCP_IAM_SERVICE_ACCOUNT_KEY_FILE_PATH_NAME, + INFISICAL_JWT_NAME, + INFISICAL_OIDC_AUTH_JWT_NAME, + INFISICAL_LDAP_USERNAME, + INFISICAL_LDAP_PASSWORD, +} + +// RegisterMachineIdentityAuthFlags declares every flag in MachineIdentityAuthFlags on cmd. identity +// names whose credentials these are, since a command may authenticate more than one identity and the +// help is the only place that distinction shows up. +func RegisterMachineIdentityAuthFlags(cmd *cobra.Command, identity string) { + cmd.Flags().String("auth-method", "", fmt.Sprintf("how to authenticate the %s machine identity ["+MachineIdentityAuthMethods+"]", identity)) + cmd.Flags().String("client-id", "", fmt.Sprintf("client id for universal auth, for the %s machine identity", identity)) + cmd.Flags().String("client-secret", "", fmt.Sprintf("client secret for universal auth, for the %s machine identity", identity)) + cmd.Flags().String("machine-identity-id", "", "machine identity id, for every method except universal-auth") + cmd.Flags().String("service-account-token-path", "", "service account token path for kubernetes auth (on a pod, /var/run/secrets/kubernetes.io/serviceaccount/token)") + cmd.Flags().String("service-account-key-file-path", "", "service account key file path for gcp-iam auth") + cmd.Flags().String("jwt", "", "JWT for the jwt-based methods [oidc-auth, jwt-auth]") + cmd.Flags().String("ldap-username", "", "username for ldap-auth") + cmd.Flags().String("ldap-password", "", "password for ldap-auth") + cmd.Flags().String("organization-slug", "", "scope the identity to this sub-organization. Defaults to the organization the identity was created in") +} + +// ResolveAuthMethod returns the machine identity auth method named by --auth-method or +// INFISICAL_AUTH_METHOD. It returns "" when none was named, and also for "user", which some callers +// spell out to mean the human login; both leave the caller on whatever fallback it has. +func ResolveAuthMethod(cmd *cobra.Command) (string, error) { + authMethod, err := GetCmdFlagOrEnvWithDefaultValue(cmd, "auth-method", []string{INFISICAL_AUTH_METHOD_NAME}, "") + if err != nil { + return "", err + } + if authMethod == "user" { + return "", nil + } + return authMethod, nil +} + +// ValidateAuthMethod reports whether authMethod names a strategy MachineIdentityLoginFunc can +// authenticate, so that a caller can reject a typo before building anything. +func ValidateAuthMethod(authMethod string) error { + valid, strategy := IsAuthMethodValid(authMethod, false) + if !valid { + return fmt.Errorf("invalid auth method %q. Supported: %s", authMethod, MachineIdentityAuthMethods) + } + if !machineIdentityStrategies[strategy] { + return fmt.Errorf("auth method %q is not supported here. Supported: %s", authMethod, MachineIdentityAuthMethods) + } + return nil +} + +// machineIdentityStrategies is which strategies MachineIdentityLoginFunc has a handler for. +// IsAuthMethodValid accepts every strategy in AVAILABLE_AUTH_STRATEGIES, and one added there without +// a handler here has to be reported rather than indexed: a missing key yields a nil func, and calling +// that panics. +var machineIdentityStrategies = map[AuthStrategyType]bool{ + AuthStrategy.UNIVERSAL_AUTH: true, + AuthStrategy.KUBERNETES_AUTH: true, + AuthStrategy.AZURE_AUTH: true, + AuthStrategy.GCP_ID_TOKEN_AUTH: true, + AuthStrategy.GCP_IAM_AUTH: true, + AuthStrategy.AWS_IAM_AUTH: true, + AuthStrategy.OIDC_AUTH: true, + AuthStrategy.JWT_AUTH: true, + AuthStrategy.LDAP_AUTH: true, +} + +// MachineIdentityLoginFunc returns the function that authenticates authMethod, which must have come +// from ResolveAuthMethod. The function performs one authentication per call and returns that +// credential, so a caller that needs a fresh token later calls it again; nothing here renews on its +// own. +func MachineIdentityLoginFunc(cmd *cobra.Command, client infisicalSdk.InfisicalClientInterface, authMethod string) (func() (infisicalSdk.MachineIdentityCredential, error), error) { + if err := ValidateAuthMethod(authMethod); err != nil { + return nil, err + } + _, strategy := IsAuthMethodValid(authMethod, false) + + authenticator := NewSdkAuthenticator(client, cmd) + strategies := map[AuthStrategyType]func() (infisicalSdk.MachineIdentityCredential, error){ + AuthStrategy.UNIVERSAL_AUTH: authenticator.HandleUniversalAuthLogin, + AuthStrategy.KUBERNETES_AUTH: authenticator.HandleKubernetesAuthLogin, + AuthStrategy.AZURE_AUTH: authenticator.HandleAzureAuthLogin, + AuthStrategy.GCP_ID_TOKEN_AUTH: authenticator.HandleGcpIdTokenAuthLogin, + AuthStrategy.GCP_IAM_AUTH: authenticator.HandleGcpIamAuthLogin, + AuthStrategy.AWS_IAM_AUTH: authenticator.HandleAwsIamAuthLogin, + AuthStrategy.OIDC_AUTH: authenticator.HandleOidcAuthLogin, + AuthStrategy.JWT_AUTH: authenticator.HandleJwtAuthLogin, + AuthStrategy.LDAP_AUTH: authenticator.HandleLdapAuthLogin, + } + + // Not indexed blind, even though ValidateAuthMethod has already passed: it consults + // machineIdentityStrategies, and if that list and this table ever disagree the lookup yields a nil + // func that panics at the call site rather than here. + login, supported := strategies[strategy] + if !supported { + return nil, fmt.Errorf("auth method %q is not supported here. Supported: %s", authMethod, MachineIdentityAuthMethods) + } + return login, nil +} From b1d0f86d52a5d9696b7efdd438a94f2eeecfef9d Mon Sep 17 00:00:00 2001 From: saif <11242541+saifsmailbox98@users.noreply.github.com> Date: Mon, 10 Aug 2026 06:53:10 +0100 Subject: [PATCH 2/6] fix(agent-proxy): let a typed auth flag beat a token from the environment Resolving a ready-made token before machine identity auth matched pam agentic-access, but it meant an explicit --auth-method lost to INFISICAL_TOKEN. That is the variable most likely to be exported for something else, and connect sets it in every agent environment it launches, so a flag the operator typed was being ignored. resolveAgentProxyCredential keeps the token-first default and makes one exception: --auth-method or client credentials on the command line beat a token that came only from the environment. An explicit --token still wins over both. Covered by a precedence test. Also drops the service-token branch in fetchAgentRealSecrets, which became unreachable once both commands started rejecting service tokens, and adds a test asserting PAM's own scrub list stays a superset of util.MachineIdentityAuthEnvVars so the two cannot drift. --- packages/cmd/agent_proxy.go | 76 +++++++++++++++----- packages/cmd/agent_proxy_start.go | 24 +++---- packages/cmd/agent_proxy_test.go | 115 ++++++++++++++++++++++++++++-- packages/pam/agent/run_test.go | 28 ++++++++ 4 files changed, 210 insertions(+), 33 deletions(-) create mode 100644 packages/pam/agent/run_test.go diff --git a/packages/cmd/agent_proxy.go b/packages/cmd/agent_proxy.go index 24c17772..5ccfe428 100644 --- a/packages/cmd/agent_proxy.go +++ b/packages/cmd/agent_proxy.go @@ -244,14 +244,61 @@ func resolveAgentProxyStaticToken(cmd *cobra.Command, subject string) *models.To return token } +// agentProxyCredential is how a command authenticates. Exactly one of token and login is set, unless +// nothing was configured at all, in which case neither is. +type agentProxyCredential struct { + token *models.TokenDetails + login func() (infisicalSdk.MachineIdentityCredential, error) + source string +} + +// machineIdentityGivenAsFlag reports whether this invocation named a machine identity on the command +// line, rather than inheriting one from the environment. +func machineIdentityGivenAsFlag(cmd *cobra.Command) bool { + return cmd.Flags().Changed("auth-method") || + cmd.Flags().Changed("client-id") || + cmd.Flags().Changed("client-secret") +} + +// resolveAgentProxyCredential chooses between a token the operator already has and a machine identity +// that authenticates itself. +// +// A ready-made token comes first, as it does in `pam agentic-access`. The exception is a flag that was +// actually typed: --auth-method or client credentials on the command line beat a token that came only +// from the environment. Without that exception INFISICAL_TOKEN would win, and it is the variable most +// likely to be exported for something else entirely, not least because `connect` sets it in every +// agent environment it launches. +func resolveAgentProxyCredential(cmd *cobra.Command) agentProxyCredential { + tokenFirst := cmd.Flags().Changed("token") || !machineIdentityGivenAsFlag(cmd) + + if tokenFirst { + if token := resolveAgentProxyStaticToken(cmd, "the provided token"); token != nil { + return agentProxyCredential{token: token, source: "token"} + } + } + + if login, source := resolveAgentProxyLogin(cmd); login != nil { + return agentProxyCredential{login: login, source: source} + } + + // Only reachable when a machine identity was named on the command line but resolved to nothing, + // which resolveAgentProxyLogin already rejects. Kept so the token is never dropped silently. + if !tokenFirst { + if token := resolveAgentProxyStaticToken(cmd, "the provided token"); token != nil { + return agentProxyCredential{token: token, source: "token"} + } + } + + return agentProxyCredential{} +} + // resolveAgentProxyLogin works out how this command's machine identity authenticates, and returns a // function that performs one authentication per call along with a label naming the branch for // telemetry. Both are nil when nothing was configured. // -// Callers reach this only after resolveAgentProxyStaticToken has come up empty, which is the order -// `pam agentic-access` resolves in too: a ready-made token first, then a machine identity -// authenticating with its own credentials. Within this function the order is an explicit -// --auth-method, then client credentials on their own as the shorthand for universal-auth. +// The order within this function is an explicit --auth-method, then client credentials on their own as +// the shorthand for universal-auth. resolveAgentProxyCredential decides how this ranks against a +// ready-made token. // // Each call builds its own SDK client and lets it go again. The SDK cannot be told to skip its // background token lifecycle (Config.AutoTokenRefresh is a bool tagged `default:"true"`, and @@ -335,23 +382,22 @@ func authMethodCredentialSource(cmd *cobra.Command, authMethod string) string { // execs, and nothing can reach in and rewrite it afterwards. When it expires the agent's requests // start coming back 403 and the agent has to be relaunched. func resolveAgentToken(cmd *cobra.Command) (*models.TokenDetails, string) { - if token := resolveAgentProxyStaticToken(cmd, "the provided token"); token != nil { - return token, "token" + resolved := resolveAgentProxyCredential(cmd) + if resolved.token != nil { + return resolved.token, resolved.source } - - login, source := resolveAgentProxyLogin(cmd) - if login == nil { + if resolved.login == nil { util.HandleError(fmt.Errorf("authentication required; pass --auth-method [%s] with that method's credentials, --client-id/--client-secret, or a token", util.MachineIdentityAuthMethods)) } - credential, err := login() + credential, err := resolved.login() if err != nil { util.HandleError(err, "Failed to authenticate the agent machine identity") } return &models.TokenDetails{ Type: util.UNIVERSAL_AUTH_TOKEN_IDENTIFIER, Token: credential.AccessToken, - }, source + }, resolved.source } // Builds http://:/:@host:port (username=projectId, password="/:", jwt last). @@ -481,11 +527,9 @@ func fetchAgentRealSecrets(token *models.TokenDetails, projectID, environment, s ExpandSecretReferences: true, IncludeImport: true, } - if token.Type == util.SERVICE_TOKEN_IDENTIFIER { - params.InfisicalToken = token.Token - } else if token.Type == util.UNIVERSAL_AUTH_TOKEN_IDENTIFIER { - params.UniversalAuthAccessToken = token.Token - } + // Always an identity access token: resolveAgentProxyStaticToken turns a service token away, and + // every auth method produces one of these. + params.UniversalAuthAccessToken = token.Token secrets, err := util.GetAllEnvironmentVariables(params, "") if err != nil { diff --git a/packages/cmd/agent_proxy_start.go b/packages/cmd/agent_proxy_start.go index 3676654a..de32e844 100644 --- a/packages/cmd/agent_proxy_start.go +++ b/packages/cmd/agent_proxy_start.go @@ -34,31 +34,29 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { } log.Logger = log.Output(logWriter) - // Same order as connect and as `pam agentic-access`: a ready-made token first, then a machine - // identity authenticating with its own credentials. + resolved := resolveAgentProxyCredential(cmd) + var accessToken string var accessTokenTTL int - var login func() (infisicalSdk.MachineIdentityCredential, error) - credentialSource := "token" - - if token := resolveAgentProxyStaticToken(cmd, "the provided token"); token != nil { + switch { + case resolved.token != nil: // A token minted elsewhere. Nothing here can renew it, and the proxy is meant to outlive any // single token, so this is worth saying out loud rather than leaving to be discovered when every // request starts failing at once. - accessToken = token.Token + accessToken = resolved.token.Token log.Warn().Msg("The agent proxy is running on a fixed token, which it cannot renew. It will stop working when that token expires; use --auth-method or client credentials to have it re-authenticate on its own.") - } else { - login, credentialSource = resolveAgentProxyLogin(cmd) - if login == nil { - util.HandleError(fmt.Errorf("agent proxy credentials required; pass --auth-method [%s] with that method's credentials, --client-id/--client-secret, or a token", util.MachineIdentityAuthMethods)) - } - credential, err := login() + case resolved.login != nil: + credential, err := resolved.login() if err != nil { util.HandleError(err, "Failed to authenticate the agent proxy machine identity") } accessToken = credential.AccessToken accessTokenTTL = int(credential.ExpiresIn) + default: + util.HandleError(fmt.Errorf("agent proxy credentials required; pass --auth-method [%s] with that method's credentials, --client-id/--client-secret, or a token", util.MachineIdentityAuthMethods)) } + credentialSource := resolved.source + login := resolved.login Telemetry.SetActor(telemetry.IdentityClaimsFromToken(accessToken)) Telemetry.CaptureEvent("cli-command:agent-proxy start", posthog.NewProperties(). diff --git a/packages/cmd/agent_proxy_test.go b/packages/cmd/agent_proxy_test.go index 7ba88aad..8db553f5 100644 --- a/packages/cmd/agent_proxy_test.go +++ b/packages/cmd/agent_proxy_test.go @@ -33,6 +33,115 @@ func TestMachineIdentityAuthFlagsAreRegistered(t *testing.T) { } } +// newAgentProxyAuthCmd is a stand-in for the real subcommands, carrying the same auth flags. Tests +// build one each so that setting a flag in one case cannot change what another case observes, which +// matters because the source label distinguishes a flag from an environment variable. +func newAgentProxyAuthCmd() *cobra.Command { + cmd := &cobra.Command{} + util.RegisterMachineIdentityAuthFlags(cmd, "test") + cmd.Flags().String("token", "", "") + return cmd +} + +// clearAgentProxyAuthEnv removes everything either resolver reads, so a case sees only what it sets +// and the developer's own environment cannot decide the outcome. +func clearAgentProxyAuthEnv(t *testing.T) { + t.Helper() + for _, key := range util.MachineIdentityAuthEnvVars { + t.Setenv(key, "") + } + for _, key := range []string{ + util.INFISICAL_TOKEN_NAME, + util.INFISICAL_UNIVERSAL_AUTH_ACCESS_TOKEN_NAME, + util.INFISICAL_GATEWAY_TOKEN_NAME_LEGACY, + } { + t.Setenv(key, "") + } +} + +// A ready-made token normally wins, matching `pam agentic-access`. The exception is a machine identity +// named on the command line, which beats a token that came only from the environment: INFISICAL_TOKEN +// is the variable most likely to be exported for something else, and connect itself sets it in every +// agent environment it launches, so a flag the operator typed must not lose to it. +// +// The token values here are deliberately not JWTs, so that the expiry guard reads no claims and the +// resolver returns instead of exiting. +func TestResolveAgentProxyCredentialPrecedence(t *testing.T) { + tests := []struct { + name string + env map[string]string + flags map[string]string + wantToken bool + wantSource string + }{ + { + name: "a token in the environment is used on its own", + env: map[string]string{util.INFISICAL_TOKEN_NAME: "opaque-token"}, + wantToken: true, + wantSource: "token", + }, + { + name: "a token beats an auth method that also came from the environment", + env: map[string]string{ + util.INFISICAL_TOKEN_NAME: "opaque-token", + util.INFISICAL_AUTH_METHOD_NAME: "aws-iam", + }, + wantToken: true, + wantSource: "token", + }, + { + name: "an auth method passed as a flag beats a token from the environment", + env: map[string]string{util.INFISICAL_TOKEN_NAME: "opaque-token"}, + flags: map[string]string{"auth-method": "aws-iam"}, + wantToken: false, + wantSource: "aws-iam-flag", + }, + { + name: "a token passed as a flag beats an auth method passed as a flag", + flags: map[string]string{"token": "opaque-token", "auth-method": "aws-iam"}, + wantToken: true, + wantSource: "token", + }, + { + name: "client credentials on the command line beat a token from the environment", + env: map[string]string{util.INFISICAL_TOKEN_NAME: "opaque-token"}, + flags: map[string]string{ + "client-id": "id", + "client-secret": "secret", + }, + wantToken: false, + wantSource: "universal-auth-flag", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clearAgentProxyAuthEnv(t) + for key, value := range tt.env { + t.Setenv(key, value) + } + cmd := newAgentProxyAuthCmd() + for name, value := range tt.flags { + if err := cmd.Flags().Set(name, value); err != nil { + t.Fatalf("setting --%s: %v", name, err) + } + } + + resolved := resolveAgentProxyCredential(cmd) + + if (resolved.token != nil) != tt.wantToken { + t.Fatalf("token != nil = %v, want %v", resolved.token != nil, tt.wantToken) + } + if !tt.wantToken && resolved.login == nil { + t.Fatal("expected a login function, got none") + } + if resolved.source != tt.wantSource { + t.Errorf("source = %q, want %q", resolved.source, tt.wantSource) + } + }) + } +} + // Which credentials win when more than one is present, asserted on the branch resolveAgentProxyLogin // picks rather than on a login it cannot perform in a test. Only the non-erroring paths are covered: // the rest of packages/cmd exits the process on a bad input, and this follows that. @@ -83,14 +192,12 @@ func TestResolveAgentProxyLoginPrecedence(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - for _, key := range append([]string{}, util.MachineIdentityAuthEnvVars...) { - t.Setenv(key, "") - } + clearAgentProxyAuthEnv(t) for key, value := range tt.env { t.Setenv(key, value) } - login, source := resolveAgentProxyLogin(agentProxyConnectCmd) + login, source := resolveAgentProxyLogin(newAgentProxyAuthCmd()) if (login != nil) != tt.wantLogin { t.Fatalf("login != nil = %v, want %v", login != nil, tt.wantLogin) diff --git a/packages/pam/agent/run_test.go b/packages/pam/agent/run_test.go new file mode 100644 index 00000000..1cfbd1cd --- /dev/null +++ b/packages/pam/agent/run_test.go @@ -0,0 +1,28 @@ +package agent + +import ( + "testing" + + "github.com/Infisical/infisical-merge/packages/util" +) + +// infisicalAuthEnvKeys is maintained here by hand, while the agent proxy derives its own scrub list +// from util.MachineIdentityAuthEnvVars. That is fine as long as this list stays a superset: an agent +// launched by `pam agentic access` must not inherit a variable the CLI's auth resolution reads, or it +// could authenticate to the API directly and open sessions outside the accounts, duration and approval +// gates the run was launched with. +// +// Without this test the two lists drift silently. Adding an auth method to the CLI updates the shared +// list, the agent proxy picks it up automatically, and PAM would not. +func TestInfisicalAuthEnvKeysCoverMachineIdentityAuthEnvVars(t *testing.T) { + stripped := make(map[string]bool, len(infisicalAuthEnvKeys)) + for _, key := range infisicalAuthEnvKeys { + stripped[key] = true + } + + for _, key := range util.MachineIdentityAuthEnvVars { + if !stripped[key] { + t.Errorf("%s reaches the agent; add it to infisicalAuthEnvKeys", key) + } + } +} From 1d8ca02bcf3e104013bfb844536ba8ebbdb2251f Mon Sep 17 00:00:00 2001 From: saif <11242541+saifsmailbox98@users.noreply.github.com> Date: Mon, 10 Aug 2026 07:36:34 +0100 Subject: [PATCH 3/6] fix(agent-proxy): reject access token TTLs the proxy cannot refresh ahead of refreshProxyToken never waits less than 30 seconds, so an access token that expires inside that window is renewed only after it has already died, and every request in the gap fails. Rejected at startup instead, on the same threshold and for the same reason as infisical agent. The floor and the threshold are now one constant, so they cannot drift apart. --- packages/cmd/agent_proxy_start.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/cmd/agent_proxy_start.go b/packages/cmd/agent_proxy_start.go index de32e844..ac5761bd 100644 --- a/packages/cmd/agent_proxy_start.go +++ b/packages/cmd/agent_proxy_start.go @@ -15,6 +15,11 @@ import ( "github.com/spf13/cobra" ) +// minRefreshableTTL is both the floor on how long refreshProxyToken waits between attempts and, for +// that reason, the shortest access token lifetime the proxy can keep ahead of. A token that expires +// sooner is rejected at startup rather than renewed too late. +const minRefreshableTTL = 30 * time.Second + func runAgentProxyStart(cmd *cobra.Command, args []string) { port, _ := cmd.Flags().GetInt("port") unmatchedHost, _ := cmd.Flags().GetString("unmatched-host") @@ -52,6 +57,12 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { } accessToken = credential.AccessToken accessTokenTTL = int(credential.ExpiresIn) + // refreshProxyToken never waits less than its retry interval, so a token that expires inside that + // window is renewed only after it is already dead, and every request in the gap fails. Refused + // rather than half-served, on the same threshold and for the same reason as `infisical agent`. + if accessTokenTTL > 0 && accessTokenTTL <= int(minRefreshableTTL.Seconds()) { + util.HandleError(fmt.Errorf("the agent proxy cannot refresh an access token with a TTL of %s or less; raise the TTL on this identity's auth method", minRefreshableTTL)) + } default: util.HandleError(fmt.Errorf("agent proxy credentials required; pass --auth-method [%s] with that method's credentials, --client-id/--client-secret, or a token", util.MachineIdentityAuthMethods)) } @@ -91,7 +102,7 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { // uses: login performs a full authentication rather than a renewal, so nothing here depends on which // one it is. func refreshProxyToken(token *atomic.Value, login func() (infisicalSdk.MachineIdentityCredential, error), ttlSeconds int) { - const retryInterval = 30 * time.Second + const retryInterval = minRefreshableTTL halfTTL := func() time.Duration { wait := time.Duration(ttlSeconds) * time.Second / 2 From 7a00cda003eede09f0a60d2afd1df5efe3841c74 Mon Sep 17 00:00:00 2001 From: saif <11242541+saifsmailbox98@users.noreply.github.com> Date: Mon, 10 Aug 2026 09:09:12 +0100 Subject: [PATCH 4/6] chore(agent-proxy): trim comments to the ones explaining why Follows the code comment rule in the monorepo's CLAUDE.md, which the CLI repo has no copy of: default to none, and keep only what explains a non-obvious constraint or a decision the code cannot show. Drops docstrings restating signatures and narration, and shortens the rest. What stays is the flag registration trap in GetCmdFlagOrEnv, why each login builds and drops its own SDK client, why the token is frozen in connect, and why the scrub list is derived rather than listed. --- packages/cmd/agent_proxy.go | 81 ++++++++++--------------------- packages/cmd/agent_proxy_start.go | 24 ++++----- packages/cmd/agent_proxy_test.go | 34 ++++--------- packages/pam/agent/run_test.go | 10 +--- packages/util/auth.go | 44 ++++++----------- 5 files changed, 62 insertions(+), 131 deletions(-) diff --git a/packages/cmd/agent_proxy.go b/packages/cmd/agent_proxy.go index 5ccfe428..8c7ab89d 100644 --- a/packages/cmd/agent_proxy.go +++ b/packages/cmd/agent_proxy.go @@ -80,21 +80,12 @@ var proxyEnvKeys = []string{ "OPENCLAW_PROXY_URL", } -// Stripped so the agent is handed only the scoped short-lived JWT set below, and not whatever the -// parent authenticated with. +// Stripped so the agent is handed only the scoped short-lived JWT set below. Derived rather than +// listed, so that an auth method added to the CLI cannot quietly widen what the agent inherits. // -// Derived from util.MachineIdentityAuthEnvVars rather than listed here, so that a machine identity -// auth method added to the CLI cannot quietly widen what the agent inherits. INFISICAL_AUTH_METHOD is -// in that list and matters as much as the credentials: connect deliberately sets INFISICAL_TOKEN for -// the agent, and a stray auth method would send a CLI call inside the agent down a machine identity -// login instead of using that token. -// -// How much this is worth depends on the method. For universal-auth, ldap-auth and the jwt-based -// methods the credential is a value that lives only in the environment, so removing it is a real -// boundary. For kubernetes, aws-iam, azure and gcp-id-token the credential is an ambient capability of -// the host (a service account token file, an instance metadata endpoint) that the agent can reach -// whether or not it inherits these; there the isolation comes from the agent running on a different -// host to the proxy, and from the agent identity holding Proxy and nothing else. +// Worth less than it looks for kubernetes, aws-iam, azure and gcp-id-token, whose credential is an +// ambient capability of the host the agent can reach anyway; there the isolation comes from the agent +// running on a different host and holding Proxy and nothing else. var credentialEnvKeys = append([]string{ util.INFISICAL_UNIVERSAL_AUTH_ACCESS_TOKEN_NAME, }, util.MachineIdentityAuthEnvVars...) @@ -224,11 +215,9 @@ func telemetryAgentName(args []string) string { return filepath.Base(args[0]) } -// resolveAgentProxyStaticToken returns a token the operator fetched elsewhere, or nil if none was -// given. The two guards match what `pam agentic-access` applies to the same input: a service token -// authenticates to the secrets API but not as a machine identity, so it cannot stand in for one here, -// and an expired token is worth catching now rather than at the agent's first proxied request, which -// is where it would otherwise surface as an unexplained 403. +// Both guards match `pam agentic-access`: a service token authenticates to the secrets API but not as +// a machine identity, and an expired one would otherwise surface as an unexplained 403 on the agent's +// first proxied request. func resolveAgentProxyStaticToken(cmd *cobra.Command, subject string) *models.TokenDetails { token, err := util.GetInfisicalToken(cmd) if err != nil { @@ -244,30 +233,23 @@ func resolveAgentProxyStaticToken(cmd *cobra.Command, subject string) *models.To return token } -// agentProxyCredential is how a command authenticates. Exactly one of token and login is set, unless -// nothing was configured at all, in which case neither is. +// Exactly one of token and login is set, or neither when nothing was configured. type agentProxyCredential struct { token *models.TokenDetails login func() (infisicalSdk.MachineIdentityCredential, error) source string } -// machineIdentityGivenAsFlag reports whether this invocation named a machine identity on the command -// line, rather than inheriting one from the environment. func machineIdentityGivenAsFlag(cmd *cobra.Command) bool { return cmd.Flags().Changed("auth-method") || cmd.Flags().Changed("client-id") || cmd.Flags().Changed("client-secret") } -// resolveAgentProxyCredential chooses between a token the operator already has and a machine identity -// that authenticates itself. -// -// A ready-made token comes first, as it does in `pam agentic-access`. The exception is a flag that was -// actually typed: --auth-method or client credentials on the command line beat a token that came only -// from the environment. Without that exception INFISICAL_TOKEN would win, and it is the variable most -// likely to be exported for something else entirely, not least because `connect` sets it in every -// agent environment it launches. +// A ready-made token comes first, as in `pam agentic-access`, except when a machine identity was +// typed on the command line and the token came only from the environment. Without that exception +// INFISICAL_TOKEN would win, and it is the variable most likely to be exported for something else, +// not least because `connect` sets it in every agent environment it launches. func resolveAgentProxyCredential(cmd *cobra.Command) agentProxyCredential { tokenFirst := cmd.Flags().Changed("token") || !machineIdentityGivenAsFlag(cmd) @@ -281,8 +263,8 @@ func resolveAgentProxyCredential(cmd *cobra.Command) agentProxyCredential { return agentProxyCredential{login: login, source: source} } - // Only reachable when a machine identity was named on the command line but resolved to nothing, - // which resolveAgentProxyLogin already rejects. Kept so the token is never dropped silently. + // Only reachable if a machine identity was named but resolved to nothing, which + // resolveAgentProxyLogin already rejects. Kept so the token is never dropped silently. if !tokenFirst { if token := resolveAgentProxyStaticToken(cmd, "the provided token"); token != nil { return agentProxyCredential{token: token, source: "token"} @@ -292,22 +274,15 @@ func resolveAgentProxyCredential(cmd *cobra.Command) agentProxyCredential { return agentProxyCredential{} } -// resolveAgentProxyLogin works out how this command's machine identity authenticates, and returns a -// function that performs one authentication per call along with a label naming the branch for -// telemetry. Both are nil when nothing was configured. -// -// The order within this function is an explicit --auth-method, then client credentials on their own as -// the shorthand for universal-auth. resolveAgentProxyCredential decides how this ranks against a -// ready-made token. +// An explicit --auth-method first, then client credentials on their own as the shorthand for +// universal-auth. Both return values are nil when nothing was configured. // // Each call builds its own SDK client and lets it go again. The SDK cannot be told to skip its // background token lifecycle (Config.AutoTokenRefresh is a bool tagged `default:"true"`, and -// setDefaults rewrites any false bool back to its default). Left alive, that goroutine would +// setDefaults rewrites any false bool back to its default), and left alive that goroutine would // re-authenticate on its own schedule alongside the renewal the caller is already driving, doubling -// this identity's authentication events; a client that dies with its login cannot. Separately, and -// the reason nothing here returns the SDK's own getter: that goroutine renews into a field -// Auth().GetAccessToken reads without taking the client's mutex, and both commands read the token -// from a per-request path. +// this identity's authentication events. Nothing here returns the SDK's getter either: it reads the +// renewed field without taking the client's mutex, and both commands read the token per request. func resolveAgentProxyLogin(cmd *cobra.Command) (login func() (infisicalSdk.MachineIdentityCredential, error), source string) { authMethod, err := util.ResolveAuthMethod(cmd) if err != nil { @@ -320,8 +295,7 @@ func resolveAgentProxyLogin(cmd *cobra.Command) (login func() (infisicalSdk.Mach if clientID == "" && clientSecret == "" { return nil, "" } - // Half a credential is a mistake rather than a request for a different method, and saying which - // half is missing beats listing every method the command accepts. + // Half a credential is a mistake, not a request for a different method. if clientID == "" { util.HandleError(fmt.Errorf("client id required; pass --client-id or set %s", util.INFISICAL_UNIVERSAL_AUTH_CLIENT_ID_NAME)) } @@ -331,8 +305,7 @@ func resolveAgentProxyLogin(cmd *cobra.Command) (login func() (infisicalSdk.Mach authMethod = string(util.AuthStrategy.UNIVERSAL_AUTH) } - // Rejected here rather than at the first login, so that a typo fails before the command has done - // anything else. + // Before anything is built, so a typo fails immediately rather than at the first login. if err := util.ValidateAuthMethod(authMethod); err != nil { util.PrintErrorMessageAndExit(err.Error()) } @@ -367,8 +340,7 @@ func resolveAgentProxyLogin(cmd *cobra.Command) (login func() (infisicalSdk.Mach return login, authMethodCredentialSource(cmd, authMethod) } -// authMethodCredentialSource labels how the identity was configured, for telemetry. The method alone -// would not distinguish the two ways of asking for the same one. +// The method alone would not distinguish the two ways of asking for the same one. func authMethodCredentialSource(cmd *cobra.Command, authMethod string) string { if cmd.Flags().Changed("auth-method") || cmd.Flags().Changed("client-id") { return authMethod + "-flag" @@ -376,11 +348,8 @@ func authMethodCredentialSource(cmd *cobra.Command, authMethod string) string { return authMethod + "-env" } -// Returns the token and a label for the branch that produced it. -// -// Whichever branch wins, the token is frozen here: connect writes it into the agent's environment and -// execs, and nothing can reach in and rewrite it afterwards. When it expires the agent's requests -// start coming back 403 and the agent has to be relaunched. +// Returns the token and a label for the branch that produced it. The token is frozen here: connect +// writes it into the agent's environment and execs, so an expiry mid-run means 403s until relaunch. func resolveAgentToken(cmd *cobra.Command) (*models.TokenDetails, string) { resolved := resolveAgentProxyCredential(cmd) if resolved.token != nil { diff --git a/packages/cmd/agent_proxy_start.go b/packages/cmd/agent_proxy_start.go index ac5761bd..98e1e2e1 100644 --- a/packages/cmd/agent_proxy_start.go +++ b/packages/cmd/agent_proxy_start.go @@ -15,9 +15,8 @@ import ( "github.com/spf13/cobra" ) -// minRefreshableTTL is both the floor on how long refreshProxyToken waits between attempts and, for -// that reason, the shortest access token lifetime the proxy can keep ahead of. A token that expires -// sooner is rejected at startup rather than renewed too late. +// Both the floor on how long refreshProxyToken waits and, for that reason, the shortest token +// lifetime the proxy can stay ahead of. const minRefreshableTTL = 30 * time.Second func runAgentProxyStart(cmd *cobra.Command, args []string) { @@ -45,9 +44,8 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { var accessTokenTTL int switch { case resolved.token != nil: - // A token minted elsewhere. Nothing here can renew it, and the proxy is meant to outlive any - // single token, so this is worth saying out loud rather than leaving to be discovered when every - // request starts failing at once. + // Nothing here can renew a token it did not fetch, and the proxy is meant to outlive any single + // one, so say so rather than let it be discovered when every request fails at once. accessToken = resolved.token.Token log.Warn().Msg("The agent proxy is running on a fixed token, which it cannot renew. It will stop working when that token expires; use --auth-method or client credentials to have it re-authenticate on its own.") case resolved.login != nil: @@ -57,9 +55,8 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { } accessToken = credential.AccessToken accessTokenTTL = int(credential.ExpiresIn) - // refreshProxyToken never waits less than its retry interval, so a token that expires inside that - // window is renewed only after it is already dead, and every request in the gap fails. Refused - // rather than half-served, on the same threshold and for the same reason as `infisical agent`. + // Renewed only after it is already dead otherwise, failing every request in the gap. Same + // threshold and reason as `infisical agent`. if accessTokenTTL > 0 && accessTokenTTL <= int(minRefreshableTTL.Seconds()) { util.HandleError(fmt.Errorf("the agent proxy cannot refresh an access token with a TTL of %s or less; raise the TTL on this identity's auth method", minRefreshableTTL)) } @@ -78,9 +75,8 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { log.Info().Msg(color.GreenString("Agent proxy authenticated; starting MITM proxy")) - // atomic.Value rather than the SDK's own getter: the proxy reads this on the per-request path while - // the refresher writes it, and Auth().GetAccessToken reads the SDK's token field without holding - // the client's mutex. See resolveAgentProxyLogin. + // atomic.Value rather than the SDK's getter, which reads its token field without the client's + // mutex while the proxy reads on the per-request path. See resolveAgentProxyLogin. var proxyToken atomic.Value proxyToken.Store(accessToken) if login != nil { @@ -98,9 +94,7 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { } } -// refreshProxyToken re-authenticates the proxy's machine identity on a schedule, whatever method it -// uses: login performs a full authentication rather than a renewal, so nothing here depends on which -// one it is. +// login authenticates in full rather than renewing, so nothing here depends on which method it uses. func refreshProxyToken(token *atomic.Value, login func() (infisicalSdk.MachineIdentityCredential, error), ttlSeconds int) { const retryInterval = minRefreshableTTL diff --git a/packages/cmd/agent_proxy_test.go b/packages/cmd/agent_proxy_test.go index 8db553f5..9ee2fcd7 100644 --- a/packages/cmd/agent_proxy_test.go +++ b/packages/cmd/agent_proxy_test.go @@ -10,11 +10,8 @@ import ( "github.com/spf13/cobra" ) -// Every command offering --auth-method has to declare all of the flags the strategies read, including -// the ones only some methods use. GetCmdFlagOrEnv looks a flag up before it looks at the environment -// and errors on a name the command never declared, so a missing registration breaks that method even -// for someone who only ever set its environment variable, and does it at authentication time rather -// than at startup. +// A missing registration breaks that method even for someone who only set its environment variable, +// and does it at authentication time rather than at startup. func TestMachineIdentityAuthFlagsAreRegistered(t *testing.T) { commands := map[string]*cobra.Command{ "agent-proxy start": agentProxyStartCmd, @@ -33,9 +30,8 @@ func TestMachineIdentityAuthFlagsAreRegistered(t *testing.T) { } } -// newAgentProxyAuthCmd is a stand-in for the real subcommands, carrying the same auth flags. Tests -// build one each so that setting a flag in one case cannot change what another case observes, which -// matters because the source label distinguishes a flag from an environment variable. +// One per case, so a flag set in one cannot change what another observes: the source label +// distinguishes a flag from an environment variable. func newAgentProxyAuthCmd() *cobra.Command { cmd := &cobra.Command{} util.RegisterMachineIdentityAuthFlags(cmd, "test") @@ -43,8 +39,7 @@ func newAgentProxyAuthCmd() *cobra.Command { return cmd } -// clearAgentProxyAuthEnv removes everything either resolver reads, so a case sees only what it sets -// and the developer's own environment cannot decide the outcome. +// So a case sees only what it sets, and the developer's own environment cannot decide the outcome. func clearAgentProxyAuthEnv(t *testing.T) { t.Helper() for _, key := range util.MachineIdentityAuthEnvVars { @@ -59,13 +54,8 @@ func clearAgentProxyAuthEnv(t *testing.T) { } } -// A ready-made token normally wins, matching `pam agentic-access`. The exception is a machine identity -// named on the command line, which beats a token that came only from the environment: INFISICAL_TOKEN -// is the variable most likely to be exported for something else, and connect itself sets it in every -// agent environment it launches, so a flag the operator typed must not lose to it. -// -// The token values here are deliberately not JWTs, so that the expiry guard reads no claims and the -// resolver returns instead of exiting. +// The token values are deliberately not JWTs, so the expiry guard reads no claims and the resolver +// returns instead of exiting. func TestResolveAgentProxyCredentialPrecedence(t *testing.T) { tests := []struct { name string @@ -142,9 +132,7 @@ func TestResolveAgentProxyCredentialPrecedence(t *testing.T) { } } -// Which credentials win when more than one is present, asserted on the branch resolveAgentProxyLogin -// picks rather than on a login it cannot perform in a test. Only the non-erroring paths are covered: -// the rest of packages/cmd exits the process on a bad input, and this follows that. +// Only the non-erroring paths: the rest of packages/cmd exits the process on bad input. func TestResolveAgentProxyLoginPrecedence(t *testing.T) { tests := []struct { name string @@ -209,8 +197,7 @@ func TestResolveAgentProxyLoginPrecedence(t *testing.T) { } } -// The scrub list is derived from the auth env vars rather than written out, so that adding a machine -// identity auth method cannot quietly widen what the agent inherits. This checks the derivation held. +// Checks the derivation held, so adding an auth method cannot quietly widen what the agent inherits. func TestCredentialEnvKeysCoverEveryAuthEnvVar(t *testing.T) { scrubbed := make(map[string]bool, len(credentialEnvKeys)) for _, key := range credentialEnvKeys { @@ -224,8 +211,7 @@ func TestCredentialEnvKeysCoverEveryAuthEnvVar(t *testing.T) { } } -// buildAgentEnv is what stands between the parent's credentials and the agent, so the guarantee is -// worth asserting on the built environment and not just on the list feeding it. +// Asserted on the built environment, not just the list feeding it. func TestBuildAgentEnvScrubsMachineIdentityCredentials(t *testing.T) { for _, envVar := range append([]string{util.INFISICAL_UNIVERSAL_AUTH_ACCESS_TOKEN_NAME}, util.MachineIdentityAuthEnvVars...) { t.Setenv(envVar, "leaked-"+envVar) diff --git a/packages/pam/agent/run_test.go b/packages/pam/agent/run_test.go index 1cfbd1cd..6f1207ff 100644 --- a/packages/pam/agent/run_test.go +++ b/packages/pam/agent/run_test.go @@ -6,14 +6,8 @@ import ( "github.com/Infisical/infisical-merge/packages/util" ) -// infisicalAuthEnvKeys is maintained here by hand, while the agent proxy derives its own scrub list -// from util.MachineIdentityAuthEnvVars. That is fine as long as this list stays a superset: an agent -// launched by `pam agentic access` must not inherit a variable the CLI's auth resolution reads, or it -// could authenticate to the API directly and open sessions outside the accounts, duration and approval -// gates the run was launched with. -// -// Without this test the two lists drift silently. Adding an auth method to the CLI updates the shared -// list, the agent proxy picks it up automatically, and PAM would not. +// This list is maintained by hand while the agent proxy derives its own from the shared one, so +// adding an auth method to the CLI would update that one and silently leave this behind. func TestInfisicalAuthEnvKeysCoverMachineIdentityAuthEnvVars(t *testing.T) { stripped := make(map[string]bool, len(infisicalAuthEnvKeys)) for _, key := range infisicalAuthEnvKeys { diff --git a/packages/util/auth.go b/packages/util/auth.go index 7cb1ddec..9548c721 100644 --- a/packages/util/auth.go +++ b/packages/util/auth.go @@ -276,15 +276,11 @@ func (a *SdkAuthenticator) HandleLdapAuthLogin() (credential infisicalSdk.Machin return a.infisicalClient.Auth().WithOrganizationSlug(organizationSlug).LdapAuthLogin(identityId, ldapUsername, ldapPassword) } -// MachineIdentityAuthMethods lists what --auth-method accepts, for flag help and for the error when -// it is given something else. Kept beside the strategy table in MachineIdentityLoginFunc so the two -// cannot drift apart. const MachineIdentityAuthMethods = "universal-auth, kubernetes, azure, gcp-id-token, gcp-iam, aws-iam, oidc-auth, jwt-auth, ldap-auth" -// MachineIdentityAuthFlags are the flags the strategies above read. Every one has to be registered on -// a command that offers --auth-method: GetCmdFlagOrEnv looks the flag up before it looks at the -// environment and returns an error for a name the command never declared, so a missing registration -// breaks that method even for someone who only ever set its environment variable. +// Every one of these has to be registered on a command that offers --auth-method: GetCmdFlagOrEnv +// looks the flag up before the environment and errors on a name the command never declared, so a +// missing registration breaks that method even for someone who only set its environment variable. var MachineIdentityAuthFlags = []string{ "auth-method", "client-id", @@ -298,9 +294,8 @@ var MachineIdentityAuthFlags = []string{ "organization-slug", } -// MachineIdentityAuthEnvVars are the environment variables the strategies read. A command that hands -// an environment to a child process it does not trust scrubs these, so that adding a strategy cannot -// quietly widen what the child inherits. +// A command that hands an environment to a child it does not trust scrubs these, so that adding a +// strategy cannot quietly widen what the child inherits. var MachineIdentityAuthEnvVars = []string{ INFISICAL_AUTH_METHOD_NAME, INFISICAL_UNIVERSAL_AUTH_CLIENT_ID_NAME, @@ -314,8 +309,7 @@ var MachineIdentityAuthEnvVars = []string{ INFISICAL_LDAP_PASSWORD, } -// RegisterMachineIdentityAuthFlags declares every flag in MachineIdentityAuthFlags on cmd. identity -// names whose credentials these are, since a command may authenticate more than one identity and the +// identity names whose credentials these are, since a command may authenticate more than one and the // help is the only place that distinction shows up. func RegisterMachineIdentityAuthFlags(cmd *cobra.Command, identity string) { cmd.Flags().String("auth-method", "", fmt.Sprintf("how to authenticate the %s machine identity ["+MachineIdentityAuthMethods+"]", identity)) @@ -330,9 +324,8 @@ func RegisterMachineIdentityAuthFlags(cmd *cobra.Command, identity string) { cmd.Flags().String("organization-slug", "", "scope the identity to this sub-organization. Defaults to the organization the identity was created in") } -// ResolveAuthMethod returns the machine identity auth method named by --auth-method or -// INFISICAL_AUTH_METHOD. It returns "" when none was named, and also for "user", which some callers -// spell out to mean the human login; both leave the caller on whatever fallback it has. +// Returns "" both when no method was named and for "user", which some callers spell out to mean the +// human login; either way the caller falls through to whatever it has. func ResolveAuthMethod(cmd *cobra.Command) (string, error) { authMethod, err := GetCmdFlagOrEnvWithDefaultValue(cmd, "auth-method", []string{INFISICAL_AUTH_METHOD_NAME}, "") if err != nil { @@ -344,8 +337,7 @@ func ResolveAuthMethod(cmd *cobra.Command) (string, error) { return authMethod, nil } -// ValidateAuthMethod reports whether authMethod names a strategy MachineIdentityLoginFunc can -// authenticate, so that a caller can reject a typo before building anything. +// Separate from MachineIdentityLoginFunc so a caller can reject a typo before building a client. func ValidateAuthMethod(authMethod string) error { valid, strategy := IsAuthMethodValid(authMethod, false) if !valid { @@ -357,10 +349,9 @@ func ValidateAuthMethod(authMethod string) error { return nil } -// machineIdentityStrategies is which strategies MachineIdentityLoginFunc has a handler for. -// IsAuthMethodValid accepts every strategy in AVAILABLE_AUTH_STRATEGIES, and one added there without -// a handler here has to be reported rather than indexed: a missing key yields a nil func, and calling -// that panics. +// IsAuthMethodValid accepts every strategy in AVAILABLE_AUTH_STRATEGIES, including ones with no +// handler below. Those have to be reported rather than indexed: a missing key yields a nil func, and +// calling that panics, which is what `infisical login` does today for ldap-auth. var machineIdentityStrategies = map[AuthStrategyType]bool{ AuthStrategy.UNIVERSAL_AUTH: true, AuthStrategy.KUBERNETES_AUTH: true, @@ -373,10 +364,8 @@ var machineIdentityStrategies = map[AuthStrategyType]bool{ AuthStrategy.LDAP_AUTH: true, } -// MachineIdentityLoginFunc returns the function that authenticates authMethod, which must have come -// from ResolveAuthMethod. The function performs one authentication per call and returns that -// credential, so a caller that needs a fresh token later calls it again; nothing here renews on its -// own. +// The returned function authenticates once per call and renews nothing, so a caller that needs a +// fresh token later calls it again. func MachineIdentityLoginFunc(cmd *cobra.Command, client infisicalSdk.InfisicalClientInterface, authMethod string) (func() (infisicalSdk.MachineIdentityCredential, error), error) { if err := ValidateAuthMethod(authMethod); err != nil { return nil, err @@ -396,9 +385,8 @@ func MachineIdentityLoginFunc(cmd *cobra.Command, client infisicalSdk.InfisicalC AuthStrategy.LDAP_AUTH: authenticator.HandleLdapAuthLogin, } - // Not indexed blind, even though ValidateAuthMethod has already passed: it consults - // machineIdentityStrategies, and if that list and this table ever disagree the lookup yields a nil - // func that panics at the call site rather than here. + // Not indexed blind despite ValidateAuthMethod passing: it consults machineIdentityStrategies, and + // if that list and this table disagree the nil func would panic at the call site instead. login, supported := strategies[strategy] if !supported { return nil, fmt.Errorf("auth method %q is not supported here. Supported: %s", authMethod, MachineIdentityAuthMethods) From fb5c8223f68fb38f20c0762ac01e791b8689fdfe Mon Sep 17 00:00:00 2001 From: saif <11242541+saifsmailbox98@users.noreply.github.com> Date: Mon, 10 Aug 2026 09:15:10 +0100 Subject: [PATCH 5/6] chore(agent-proxy): cut the remaining comments back to load-bearing ones Drops the rest of the narration and the threat-model prose, leaving only what a reader could not work out from the code: the GetCmdFlagOrEnv flag trap, why each login builds and drops its own SDK client, why the token is frozen in connect, and why the strategy table is not indexed blind. Also removes the unreachable second token lookup in resolveAgentProxyCredential rather than explaining it. Ordering the two resolvers up front says the same thing without the dead branch. --- packages/cmd/agent_proxy.go | 71 ++++++++++++------------------- packages/cmd/agent_proxy_start.go | 13 ++---- packages/cmd/agent_proxy_test.go | 15 ++----- packages/util/auth.go | 22 +++------- 4 files changed, 42 insertions(+), 79 deletions(-) diff --git a/packages/cmd/agent_proxy.go b/packages/cmd/agent_proxy.go index 8c7ab89d..88b1d264 100644 --- a/packages/cmd/agent_proxy.go +++ b/packages/cmd/agent_proxy.go @@ -80,12 +80,8 @@ var proxyEnvKeys = []string{ "OPENCLAW_PROXY_URL", } -// Stripped so the agent is handed only the scoped short-lived JWT set below. Derived rather than -// listed, so that an auth method added to the CLI cannot quietly widen what the agent inherits. -// -// Worth less than it looks for kubernetes, aws-iam, azure and gcp-id-token, whose credential is an -// ambient capability of the host the agent can reach anyway; there the isolation comes from the agent -// running on a different host and holding Proxy and nothing else. +// Derived rather than listed, so an auth method added to the CLI cannot quietly widen what the agent +// inherits. Buys little for the methods whose credential is ambient to the host anyway. var credentialEnvKeys = append([]string{ util.INFISICAL_UNIVERSAL_AUTH_ACCESS_TOKEN_NAME, }, util.MachineIdentityAuthEnvVars...) @@ -215,9 +211,8 @@ func telemetryAgentName(args []string) string { return filepath.Base(args[0]) } -// Both guards match `pam agentic-access`: a service token authenticates to the secrets API but not as -// a machine identity, and an expired one would otherwise surface as an unexplained 403 on the agent's -// first proxied request. +// A service token authenticates to the secrets API but not as a machine identity, and an expired one +// would otherwise surface as an unexplained 403 on the agent's first proxied request. func resolveAgentProxyStaticToken(cmd *cobra.Command, subject string) *models.TokenDetails { token, err := util.GetInfisicalToken(cmd) if err != nil { @@ -233,7 +228,6 @@ func resolveAgentProxyStaticToken(cmd *cobra.Command, subject string) *models.To return token } -// Exactly one of token and login is set, or neither when nothing was configured. type agentProxyCredential struct { token *models.TokenDetails login func() (infisicalSdk.MachineIdentityCredential, error) @@ -246,43 +240,38 @@ func machineIdentityGivenAsFlag(cmd *cobra.Command) bool { cmd.Flags().Changed("client-secret") } -// A ready-made token comes first, as in `pam agentic-access`, except when a machine identity was -// typed on the command line and the token came only from the environment. Without that exception -// INFISICAL_TOKEN would win, and it is the variable most likely to be exported for something else, -// not least because `connect` sets it in every agent environment it launches. +// A ready-made token comes first, as in `pam agentic-access`, except when a machine identity was typed +// on the command line and the token came only from the environment. Without that exception +// INFISICAL_TOKEN would win, and `connect` sets it in every agent environment it launches. func resolveAgentProxyCredential(cmd *cobra.Command) agentProxyCredential { - tokenFirst := cmd.Flags().Changed("token") || !machineIdentityGivenAsFlag(cmd) - - if tokenFirst { + staticToken := func() agentProxyCredential { if token := resolveAgentProxyStaticToken(cmd, "the provided token"); token != nil { return agentProxyCredential{token: token, source: "token"} } + return agentProxyCredential{} } - - if login, source := resolveAgentProxyLogin(cmd); login != nil { - return agentProxyCredential{login: login, source: source} + machineIdentity := func() agentProxyCredential { + if login, source := resolveAgentProxyLogin(cmd); login != nil { + return agentProxyCredential{login: login, source: source} + } + return agentProxyCredential{} } - // Only reachable if a machine identity was named but resolved to nothing, which - // resolveAgentProxyLogin already rejects. Kept so the token is never dropped silently. - if !tokenFirst { - if token := resolveAgentProxyStaticToken(cmd, "the provided token"); token != nil { - return agentProxyCredential{token: token, source: "token"} - } + first, second := staticToken, machineIdentity + if machineIdentityGivenAsFlag(cmd) && !cmd.Flags().Changed("token") { + first, second = machineIdentity, staticToken } - return agentProxyCredential{} + if resolved := first(); resolved.token != nil || resolved.login != nil { + return resolved + } + return second() } -// An explicit --auth-method first, then client credentials on their own as the shorthand for -// universal-auth. Both return values are nil when nothing was configured. -// -// Each call builds its own SDK client and lets it go again. The SDK cannot be told to skip its -// background token lifecycle (Config.AutoTokenRefresh is a bool tagged `default:"true"`, and -// setDefaults rewrites any false bool back to its default), and left alive that goroutine would -// re-authenticate on its own schedule alongside the renewal the caller is already driving, doubling -// this identity's authentication events. Nothing here returns the SDK's getter either: it reads the -// renewed field without taking the client's mutex, and both commands read the token per request. +// Each call builds its own SDK client and drops it. AutoTokenRefresh cannot be switched off (it is a +// bool tagged `default:"true"`, and setDefaults rewrites any false bool back), so a client kept alive +// would re-authenticate on its own schedule alongside the caller's, doubling this identity's auth +// events. Its getter goes unused for a second reason: it reads the renewed field without the mutex. func resolveAgentProxyLogin(cmd *cobra.Command) (login func() (infisicalSdk.MachineIdentityCredential, error), source string) { authMethod, err := util.ResolveAuthMethod(cmd) if err != nil { @@ -295,7 +284,6 @@ func resolveAgentProxyLogin(cmd *cobra.Command) (login func() (infisicalSdk.Mach if clientID == "" && clientSecret == "" { return nil, "" } - // Half a credential is a mistake, not a request for a different method. if clientID == "" { util.HandleError(fmt.Errorf("client id required; pass --client-id or set %s", util.INFISICAL_UNIVERSAL_AUTH_CLIENT_ID_NAME)) } @@ -305,7 +293,6 @@ func resolveAgentProxyLogin(cmd *cobra.Command) (login func() (infisicalSdk.Mach authMethod = string(util.AuthStrategy.UNIVERSAL_AUTH) } - // Before anything is built, so a typo fails immediately rather than at the first login. if err := util.ValidateAuthMethod(authMethod); err != nil { util.PrintErrorMessageAndExit(err.Error()) } @@ -340,7 +327,6 @@ func resolveAgentProxyLogin(cmd *cobra.Command) (login func() (infisicalSdk.Mach return login, authMethodCredentialSource(cmd, authMethod) } -// The method alone would not distinguish the two ways of asking for the same one. func authMethodCredentialSource(cmd *cobra.Command, authMethod string) string { if cmd.Flags().Changed("auth-method") || cmd.Flags().Changed("client-id") { return authMethod + "-flag" @@ -348,8 +334,8 @@ func authMethodCredentialSource(cmd *cobra.Command, authMethod string) string { return authMethod + "-env" } -// Returns the token and a label for the branch that produced it. The token is frozen here: connect -// writes it into the agent's environment and execs, so an expiry mid-run means 403s until relaunch. +// The token is frozen here: connect writes it into the agent's environment and execs, so an expiry +// mid-run means 403s until relaunch. func resolveAgentToken(cmd *cobra.Command) (*models.TokenDetails, string) { resolved := resolveAgentProxyCredential(cmd) if resolved.token != nil { @@ -496,8 +482,7 @@ func fetchAgentRealSecrets(token *models.TokenDetails, projectID, environment, s ExpandSecretReferences: true, IncludeImport: true, } - // Always an identity access token: resolveAgentProxyStaticToken turns a service token away, and - // every auth method produces one of these. + // Always an identity access token: service tokens are turned away before this. params.UniversalAuthAccessToken = token.Token secrets, err := util.GetAllEnvironmentVariables(params, "") diff --git a/packages/cmd/agent_proxy_start.go b/packages/cmd/agent_proxy_start.go index 98e1e2e1..3acbe6e3 100644 --- a/packages/cmd/agent_proxy_start.go +++ b/packages/cmd/agent_proxy_start.go @@ -15,8 +15,7 @@ import ( "github.com/spf13/cobra" ) -// Both the floor on how long refreshProxyToken waits and, for that reason, the shortest token -// lifetime the proxy can stay ahead of. +// The floor on refreshProxyToken's wait, and so the shortest TTL it can stay ahead of. const minRefreshableTTL = 30 * time.Second func runAgentProxyStart(cmd *cobra.Command, args []string) { @@ -44,8 +43,7 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { var accessTokenTTL int switch { case resolved.token != nil: - // Nothing here can renew a token it did not fetch, and the proxy is meant to outlive any single - // one, so say so rather than let it be discovered when every request fails at once. + // Unrenewable, and the proxy is meant to outlive any single token. accessToken = resolved.token.Token log.Warn().Msg("The agent proxy is running on a fixed token, which it cannot renew. It will stop working when that token expires; use --auth-method or client credentials to have it re-authenticate on its own.") case resolved.login != nil: @@ -55,8 +53,7 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { } accessToken = credential.AccessToken accessTokenTTL = int(credential.ExpiresIn) - // Renewed only after it is already dead otherwise, failing every request in the gap. Same - // threshold and reason as `infisical agent`. + // Otherwise renewed only after it is already dead, failing every request in the gap. if accessTokenTTL > 0 && accessTokenTTL <= int(minRefreshableTTL.Seconds()) { util.HandleError(fmt.Errorf("the agent proxy cannot refresh an access token with a TTL of %s or less; raise the TTL on this identity's auth method", minRefreshableTTL)) } @@ -75,8 +72,7 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { log.Info().Msg(color.GreenString("Agent proxy authenticated; starting MITM proxy")) - // atomic.Value rather than the SDK's getter, which reads its token field without the client's - // mutex while the proxy reads on the per-request path. See resolveAgentProxyLogin. + // Not the SDK's getter: it reads the renewed field without the mutex, and this is read per request. var proxyToken atomic.Value proxyToken.Store(accessToken) if login != nil { @@ -94,7 +90,6 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { } } -// login authenticates in full rather than renewing, so nothing here depends on which method it uses. func refreshProxyToken(token *atomic.Value, login func() (infisicalSdk.MachineIdentityCredential, error), ttlSeconds int) { const retryInterval = minRefreshableTTL diff --git a/packages/cmd/agent_proxy_test.go b/packages/cmd/agent_proxy_test.go index 9ee2fcd7..95601a06 100644 --- a/packages/cmd/agent_proxy_test.go +++ b/packages/cmd/agent_proxy_test.go @@ -10,8 +10,7 @@ import ( "github.com/spf13/cobra" ) -// A missing registration breaks that method even for someone who only set its environment variable, -// and does it at authentication time rather than at startup. +// A missing registration breaks that method at authentication time, not at startup. func TestMachineIdentityAuthFlagsAreRegistered(t *testing.T) { commands := map[string]*cobra.Command{ "agent-proxy start": agentProxyStartCmd, @@ -30,8 +29,7 @@ func TestMachineIdentityAuthFlagsAreRegistered(t *testing.T) { } } -// One per case, so a flag set in one cannot change what another observes: the source label -// distinguishes a flag from an environment variable. +// One per case: the source label distinguishes a flag from an environment variable. func newAgentProxyAuthCmd() *cobra.Command { cmd := &cobra.Command{} util.RegisterMachineIdentityAuthFlags(cmd, "test") @@ -39,7 +37,6 @@ func newAgentProxyAuthCmd() *cobra.Command { return cmd } -// So a case sees only what it sets, and the developer's own environment cannot decide the outcome. func clearAgentProxyAuthEnv(t *testing.T) { t.Helper() for _, key := range util.MachineIdentityAuthEnvVars { @@ -54,8 +51,7 @@ func clearAgentProxyAuthEnv(t *testing.T) { } } -// The token values are deliberately not JWTs, so the expiry guard reads no claims and the resolver -// returns instead of exiting. +// The token values are not JWTs, so the expiry guard reads no claims and returns instead of exiting. func TestResolveAgentProxyCredentialPrecedence(t *testing.T) { tests := []struct { name string @@ -132,7 +128,6 @@ func TestResolveAgentProxyCredentialPrecedence(t *testing.T) { } } -// Only the non-erroring paths: the rest of packages/cmd exits the process on bad input. func TestResolveAgentProxyLoginPrecedence(t *testing.T) { tests := []struct { name string @@ -170,8 +165,6 @@ func TestResolveAgentProxyLoginPrecedence(t *testing.T) { wantSource: "aws-iam-env", }, { - // "user" is how some callers spell the human login, and there is no such thing here, so it - // has to fall through rather than be treated as a method name. name: "the user pseudo-method is not a machine identity", env: map[string]string{util.INFISICAL_AUTH_METHOD_NAME: "user"}, wantLogin: false, @@ -197,7 +190,6 @@ func TestResolveAgentProxyLoginPrecedence(t *testing.T) { } } -// Checks the derivation held, so adding an auth method cannot quietly widen what the agent inherits. func TestCredentialEnvKeysCoverEveryAuthEnvVar(t *testing.T) { scrubbed := make(map[string]bool, len(credentialEnvKeys)) for _, key := range credentialEnvKeys { @@ -211,7 +203,6 @@ func TestCredentialEnvKeysCoverEveryAuthEnvVar(t *testing.T) { } } -// Asserted on the built environment, not just the list feeding it. func TestBuildAgentEnvScrubsMachineIdentityCredentials(t *testing.T) { for _, envVar := range append([]string{util.INFISICAL_UNIVERSAL_AUTH_ACCESS_TOKEN_NAME}, util.MachineIdentityAuthEnvVars...) { t.Setenv(envVar, "leaked-"+envVar) diff --git a/packages/util/auth.go b/packages/util/auth.go index 9548c721..53ab0301 100644 --- a/packages/util/auth.go +++ b/packages/util/auth.go @@ -278,9 +278,8 @@ func (a *SdkAuthenticator) HandleLdapAuthLogin() (credential infisicalSdk.Machin const MachineIdentityAuthMethods = "universal-auth, kubernetes, azure, gcp-id-token, gcp-iam, aws-iam, oidc-auth, jwt-auth, ldap-auth" -// Every one of these has to be registered on a command that offers --auth-method: GetCmdFlagOrEnv -// looks the flag up before the environment and errors on a name the command never declared, so a -// missing registration breaks that method even for someone who only set its environment variable. +// GetCmdFlagOrEnv looks a flag up before the environment and errors on a name the command never +// declared, so any command offering --auth-method must register all of these. var MachineIdentityAuthFlags = []string{ "auth-method", "client-id", @@ -309,8 +308,6 @@ var MachineIdentityAuthEnvVars = []string{ INFISICAL_LDAP_PASSWORD, } -// identity names whose credentials these are, since a command may authenticate more than one and the -// help is the only place that distinction shows up. func RegisterMachineIdentityAuthFlags(cmd *cobra.Command, identity string) { cmd.Flags().String("auth-method", "", fmt.Sprintf("how to authenticate the %s machine identity ["+MachineIdentityAuthMethods+"]", identity)) cmd.Flags().String("client-id", "", fmt.Sprintf("client id for universal auth, for the %s machine identity", identity)) @@ -324,8 +321,7 @@ func RegisterMachineIdentityAuthFlags(cmd *cobra.Command, identity string) { cmd.Flags().String("organization-slug", "", "scope the identity to this sub-organization. Defaults to the organization the identity was created in") } -// Returns "" both when no method was named and for "user", which some callers spell out to mean the -// human login; either way the caller falls through to whatever it has. +// Returns "" for "user" as well as for nothing: some callers spell out the human login. func ResolveAuthMethod(cmd *cobra.Command) (string, error) { authMethod, err := GetCmdFlagOrEnvWithDefaultValue(cmd, "auth-method", []string{INFISICAL_AUTH_METHOD_NAME}, "") if err != nil { @@ -337,7 +333,6 @@ func ResolveAuthMethod(cmd *cobra.Command) (string, error) { return authMethod, nil } -// Separate from MachineIdentityLoginFunc so a caller can reject a typo before building a client. func ValidateAuthMethod(authMethod string) error { valid, strategy := IsAuthMethodValid(authMethod, false) if !valid { @@ -349,9 +344,8 @@ func ValidateAuthMethod(authMethod string) error { return nil } -// IsAuthMethodValid accepts every strategy in AVAILABLE_AUTH_STRATEGIES, including ones with no -// handler below. Those have to be reported rather than indexed: a missing key yields a nil func, and -// calling that panics, which is what `infisical login` does today for ldap-auth. +// IsAuthMethodValid accepts strategies with no handler below. Those must be reported rather than +// indexed: a nil func panics, which is what `infisical login` does today for ldap-auth. var machineIdentityStrategies = map[AuthStrategyType]bool{ AuthStrategy.UNIVERSAL_AUTH: true, AuthStrategy.KUBERNETES_AUTH: true, @@ -364,8 +358,7 @@ var machineIdentityStrategies = map[AuthStrategyType]bool{ AuthStrategy.LDAP_AUTH: true, } -// The returned function authenticates once per call and renews nothing, so a caller that needs a -// fresh token later calls it again. +// The returned function authenticates once per call and renews nothing. func MachineIdentityLoginFunc(cmd *cobra.Command, client infisicalSdk.InfisicalClientInterface, authMethod string) (func() (infisicalSdk.MachineIdentityCredential, error), error) { if err := ValidateAuthMethod(authMethod); err != nil { return nil, err @@ -385,8 +378,7 @@ func MachineIdentityLoginFunc(cmd *cobra.Command, client infisicalSdk.InfisicalC AuthStrategy.LDAP_AUTH: authenticator.HandleLdapAuthLogin, } - // Not indexed blind despite ValidateAuthMethod passing: it consults machineIdentityStrategies, and - // if that list and this table disagree the nil func would panic at the call site instead. + // ValidateAuthMethod consults machineIdentityStrategies, not this table. login, supported := strategies[strategy] if !supported { return nil, fmt.Errorf("auth method %q is not supported here. Supported: %s", authMethod, MachineIdentityAuthMethods) From 1df26b453a9e04d270b29c6982fd641eeb23041f Mon Sep 17 00:00:00 2001 From: saif <11242541+saifsmailbox98@users.noreply.github.com> Date: Mon, 10 Aug 2026 09:18:48 +0100 Subject: [PATCH 6/6] chore(agent-proxy): drop comments that repeat the code, a test or an error Several of the remaining ones restated something already stated nearby: the scrub list has a test asserting exactly what its comment claimed, the fixed-token warning says in its message what the comment above it said, and the SDK getter reason was written out in two files. Nineteen lines left, each one a constraint a reader cannot see: the GetCmdFlagOrEnv flag trap, the token precedence exception, why a client is built per login, and why the strategy table is not indexed blind. --- packages/cmd/agent_proxy.go | 15 +++++---------- packages/cmd/agent_proxy_start.go | 2 -- packages/cmd/agent_proxy_test.go | 2 -- packages/pam/agent/run_test.go | 3 +-- packages/util/auth.go | 3 --- 5 files changed, 6 insertions(+), 19 deletions(-) diff --git a/packages/cmd/agent_proxy.go b/packages/cmd/agent_proxy.go index 88b1d264..8fbbf078 100644 --- a/packages/cmd/agent_proxy.go +++ b/packages/cmd/agent_proxy.go @@ -80,8 +80,6 @@ var proxyEnvKeys = []string{ "OPENCLAW_PROXY_URL", } -// Derived rather than listed, so an auth method added to the CLI cannot quietly widen what the agent -// inherits. Buys little for the methods whose credential is ambient to the host anyway. var credentialEnvKeys = append([]string{ util.INFISICAL_UNIVERSAL_AUTH_ACCESS_TOKEN_NAME, }, util.MachineIdentityAuthEnvVars...) @@ -211,8 +209,7 @@ func telemetryAgentName(args []string) string { return filepath.Base(args[0]) } -// A service token authenticates to the secrets API but not as a machine identity, and an expired one -// would otherwise surface as an unexplained 403 on the agent's first proxied request. +// A service token authenticates to the secrets API but not as a machine identity. func resolveAgentProxyStaticToken(cmd *cobra.Command, subject string) *models.TokenDetails { token, err := util.GetInfisicalToken(cmd) if err != nil { @@ -268,10 +265,10 @@ func resolveAgentProxyCredential(cmd *cobra.Command) agentProxyCredential { return second() } -// Each call builds its own SDK client and drops it. AutoTokenRefresh cannot be switched off (it is a -// bool tagged `default:"true"`, and setDefaults rewrites any false bool back), so a client kept alive -// would re-authenticate on its own schedule alongside the caller's, doubling this identity's auth -// events. Its getter goes unused for a second reason: it reads the renewed field without the mutex. +// A client per login, dropped after. AutoTokenRefresh cannot be switched off (a bool tagged +// `default:"true"`, and setDefaults rewrites any false bool back), so one kept alive would +// re-authenticate alongside the caller's own renewal. Its getter reads that field without the mutex, +// which is why callers take the returned credential instead. func resolveAgentProxyLogin(cmd *cobra.Command) (login func() (infisicalSdk.MachineIdentityCredential, error), source string) { authMethod, err := util.ResolveAuthMethod(cmd) if err != nil { @@ -334,8 +331,6 @@ func authMethodCredentialSource(cmd *cobra.Command, authMethod string) string { return authMethod + "-env" } -// The token is frozen here: connect writes it into the agent's environment and execs, so an expiry -// mid-run means 403s until relaunch. func resolveAgentToken(cmd *cobra.Command) (*models.TokenDetails, string) { resolved := resolveAgentProxyCredential(cmd) if resolved.token != nil { diff --git a/packages/cmd/agent_proxy_start.go b/packages/cmd/agent_proxy_start.go index 3acbe6e3..4c2f1d6d 100644 --- a/packages/cmd/agent_proxy_start.go +++ b/packages/cmd/agent_proxy_start.go @@ -43,7 +43,6 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { var accessTokenTTL int switch { case resolved.token != nil: - // Unrenewable, and the proxy is meant to outlive any single token. accessToken = resolved.token.Token log.Warn().Msg("The agent proxy is running on a fixed token, which it cannot renew. It will stop working when that token expires; use --auth-method or client credentials to have it re-authenticate on its own.") case resolved.login != nil: @@ -72,7 +71,6 @@ func runAgentProxyStart(cmd *cobra.Command, args []string) { log.Info().Msg(color.GreenString("Agent proxy authenticated; starting MITM proxy")) - // Not the SDK's getter: it reads the renewed field without the mutex, and this is read per request. var proxyToken atomic.Value proxyToken.Store(accessToken) if login != nil { diff --git a/packages/cmd/agent_proxy_test.go b/packages/cmd/agent_proxy_test.go index 95601a06..2504521d 100644 --- a/packages/cmd/agent_proxy_test.go +++ b/packages/cmd/agent_proxy_test.go @@ -10,7 +10,6 @@ import ( "github.com/spf13/cobra" ) -// A missing registration breaks that method at authentication time, not at startup. func TestMachineIdentityAuthFlagsAreRegistered(t *testing.T) { commands := map[string]*cobra.Command{ "agent-proxy start": agentProxyStartCmd, @@ -29,7 +28,6 @@ func TestMachineIdentityAuthFlagsAreRegistered(t *testing.T) { } } -// One per case: the source label distinguishes a flag from an environment variable. func newAgentProxyAuthCmd() *cobra.Command { cmd := &cobra.Command{} util.RegisterMachineIdentityAuthFlags(cmd, "test") diff --git a/packages/pam/agent/run_test.go b/packages/pam/agent/run_test.go index 6f1207ff..4eae3899 100644 --- a/packages/pam/agent/run_test.go +++ b/packages/pam/agent/run_test.go @@ -6,8 +6,7 @@ import ( "github.com/Infisical/infisical-merge/packages/util" ) -// This list is maintained by hand while the agent proxy derives its own from the shared one, so -// adding an auth method to the CLI would update that one and silently leave this behind. +// Maintained by hand, while the agent proxy derives its own from the shared list. func TestInfisicalAuthEnvKeysCoverMachineIdentityAuthEnvVars(t *testing.T) { stripped := make(map[string]bool, len(infisicalAuthEnvKeys)) for _, key := range infisicalAuthEnvKeys { diff --git a/packages/util/auth.go b/packages/util/auth.go index 53ab0301..eef2fef7 100644 --- a/packages/util/auth.go +++ b/packages/util/auth.go @@ -293,8 +293,6 @@ var MachineIdentityAuthFlags = []string{ "organization-slug", } -// A command that hands an environment to a child it does not trust scrubs these, so that adding a -// strategy cannot quietly widen what the child inherits. var MachineIdentityAuthEnvVars = []string{ INFISICAL_AUTH_METHOD_NAME, INFISICAL_UNIVERSAL_AUTH_CLIENT_ID_NAME, @@ -358,7 +356,6 @@ var machineIdentityStrategies = map[AuthStrategyType]bool{ AuthStrategy.LDAP_AUTH: true, } -// The returned function authenticates once per call and renews nothing. func MachineIdentityLoginFunc(cmd *cobra.Command, client infisicalSdk.InfisicalClientInterface, authMethod string) (func() (infisicalSdk.MachineIdentityCredential, error), error) { if err := ValidateAuthMethod(authMethod); err != nil { return nil, err