feat(agent-proxy): support all machine identity auth methods - #357
feat(agent-proxy): support all machine identity auth methods#357saifsmailbox98 wants to merge 6 commits into
Conversation
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.
|
💬 Discussion in Slack: #pr-review-cli-357-feat-agent-proxy-support-all-machine-identity-auth-methods Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
|
| Filename | Overview |
|---|---|
| packages/cmd/agent_proxy.go | Adds shared token/login resolution, all machine-identity flags, and expanded child credential scrubbing without an accepted defect. |
| packages/cmd/agent_proxy_start.go | Adds static-token and strategy-based authentication, but its minimum refresh delay can outlive short access-token TTLs. |
| packages/util/auth.go | Centralizes supported machine-identity methods, inputs, validation, and SDK handler selection with complete current input coverage. |
| packages/cmd/pam.go | Reuses the shared authentication resolution and strategy table while preserving the existing long-lived SDK lifecycle. |
| packages/cmd/agent_proxy_test.go | Covers strategy flag registration, precedence, and environment scrubbing, but does not exercise short-TTL refresh scheduling. |
Comments Outside Diff (1)
-
packages/cmd/agent_proxy_start.go, line 98-103 (link)Refresh can follow token expiry
When a machine identity issues an access token with a TTL below 60 seconds, the 30-second minimum wait delays re-authentication until near or after expiry, causing the proxy to use an expired token and backend requests to fail.
Knowledge Base Used:
Reviews (1): Last reviewed commit: "feat(agent-proxy): support all machine i..." | Re-trigger Greptile
…ment 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.
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| 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") |
There was a problem hiding this comment.
Low: Authentication secrets exposed in process arguments
The newly supported --jwt and --ldap-password values remain in the process command line for the lifetime of agent-proxy start or connect. On multi-user systems, a local user who can inspect process arguments can copy these credentials and authenticate as the machine identity. Keep these flags registered if required for environment fallback, but reject command-line values for secret-bearing options and require environment variables, protected files, or stdin instead.
There was a problem hiding this comment.
Valid concern, but out of scope here. --client-secret was already a flag on both these commands, and login, gateway, relay, proxy and pam agentic access all take the same secret flags. Rejecting them only in agent-proxy would be inconsistent and would not close the gap anywhere else. The docs already say to prefer the env vars. Better as a separate CLI-wide change if we want to drop secret flags entirely.
There was a problem hiding this comment.
Thanks for the detail. I can't automatically confirm this is safe to dismiss, so I'm leaving the thread open for a maintainer to make the call.
PR overviewThis pull request expands Two security issues remain open. Secret-bearing command-line flags can expose machine identity credentials to other local users through process arguments, while incomplete credential precedence handling can cause an existing environment token to override the selected identity and be passed to the launched agent. No issues have yet been addressed. Open issues (2)
Fixed/addressed: 0 · PR risk: 7/10 |
…head 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.
| // 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") || |
There was a problem hiding this comment.
Low: Method-specific flags do not override an environment token
This only recognizes --auth-method, --client-id, and --client-secret. If the auth method comes from INFISICAL_AUTH_METHOD while credentials such as --jwt, --machine-identity-id, or --service-account-token-path are supplied as flags, an existing INFISICAL_TOKEN still wins. For connect, that potentially broader token is then exposed to the launched agent through both INFISICAL_TOKEN and the proxy URL, allowing the agent to use permissions from the wrong identity. Treat every changed method-specific credential flag as an explicit machine-identity selection while preserving an explicit --token as the highest priority.
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.
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.
…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.
1 participant
Description 📣
Before this,
agent-proxy startandagent-proxy connectcould only authenticate using Universal Auth. So even if you run the proxy on EKS or EC2, where the host already has its own identity, you still had to create a client id and secret and put them on the host.Now both commands support all nine machine identity auth methods the CLI already has:
universal-auth,kubernetes,azure,gcp-id-token,gcp-iam,aws-iam,oidc-auth,jwt-authandldap-auth. You pick one with--auth-methodorINFISICAL_AUTH_METHOD.On a Kubernetes pod it looks like this, with no secret to create anywhere:
If you are already on Universal Auth, nothing changes for you.
--client-idwith--client-secretstill works exactly as before.startalso accepts--tokennow, which it did not accept earlier.Implementation notes
Shared strategy table. The auth method table moves from
cmd/pam.gointoutil, andpam agentic-access,agent-proxy startandagent-proxy connectall use it now. A test asserts every command registers all the flags the strategies read, becauseGetCmdFlagOrEnvlooks up the flag before the environment variable and errors on a flag the command never declared. Without that, a method would break even for someone who only set its environment variable.Resolution order. A ready-made token comes first, then machine identity auth, same default as
pam agentic-access. One exception:--auth-methodor client credentials typed on the command line beat a token that came only from the environment, becauseINFISICAL_TOKENis the variable most likely to be exported for something else, andconnectsets it in every agent environment it launches. An explicit--tokenstill wins over both. Earlierstarthad no token path at all, andconnectchecked client credentials before--token. Covered by a precedence test.Token guards on both commands, same as pam: service tokens are rejected up front, and an expired token fails immediately instead of surfacing later as a 403 on the agent's first request.
startalso warns that it cannot renew a token it did not fetch itself.Refresh.
refreshProxyTokennow re-authenticates through whichever strategy was resolved, instead of callingUniversalAuthLogindirectly.One SDK client per login, then dropped.
Config.AutoTokenRefreshis a bool taggeddefault:"true", andsetDefaultsrewrites any false bool back to its default, so the SDK's background refresh cannot be switched off. A long-lived client would keep re-authenticating on its own schedule alongside ours and double this identity's auth events. Nothing readsAuth().GetAccessTokeneither, because it reads the SDK's token field without taking the client's mutex while the proxy reads the token on every request. We keep usingatomic.Valuefor that.Credential scrubbing.
credentialEnvKeysis now derived fromutil.MachineIdentityAuthEnvVars, so adding an auth method cannot silently widen what a child agent inherits.INFISICAL_AUTH_METHODis scrubbed too, sinceconnectdeliberately setsINFISICAL_TOKENfor the agent and a stray auth method would push a CLI call inside the agent down a login path instead. Note this also widensagent-proxy run's scrub, because it shares the same list.Known limitation.
jwt-authandoidc-authonstartreuse the JWT you passed at every refresh, so a short-lived CI token will already be expired by the first refresh. Documented instead of blocked, since a long-lived internal JWT is a fair use case.Error messages. Giving only half a credential now tells you which half is missing, instead of listing all nine methods.
PAM.
pam agentic accesskeeps its own hand-maintained env scrub list, which is already a superset of the shared one, so nothing there changes. A test asserts it stays a superset, so adding an auth method to the CLI cannot silently leave PAM behind.Docs PR: Infisical/infisical#7606
Type ✨
Tests 🛠️
# Here's some code block to paste some code snippets