improvement(agent-vault): read the session token from the environment - #395
improvement(agent-vault): read the session token from the environment#395saifsmailbox98 wants to merge 2 commits into
Conversation
|
💬 Discussion in Slack: #pr-review-cli-395-improvement-agent-vault-read-the-session-token-from-the-enviro Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| util.PrintWarning(fmt.Sprintf("%s is set, but --access-bundle was passed: minting a new session and ignoring the token in the environment.", agentVaultSessionTokenEnv)) | ||
| sessionToken = "" |
There was a problem hiding this comment.
When --access-bundle overrides an ambient session token, this branch clears only the local variable. buildAgentVaultRunEnv still copies INFISICAL_AGENT_VAULT_SESSION_TOKEN into the child environment, so the launched process and anything it spawns receive the supposedly ignored bearer credential. If the stale token grants broader access than the newly minted bundle, the child retains those broader privileges. Remove the variable from the child environment when constructing an access-bundle run.
How this was verified: The access-bundle branch clears only the local token, while buildAgentVaultRunEnv copies every non-proxy parent variable, including the ambient session token, into the child environment.
--session-token was the only Agent Vault flag without an environment fallback, so the one credential that carries the whole access bundle had to sit on the command line, in shell history and in process listings. It now falls back to INFISICAL_AGENT_VAULT_SESSION_TOKEN, the same shape --proxy, --client-id and the proxy's --enrollment-token already have. Resolution moves into one helper so the two rules that turn on the token's source are testable. An explicit but empty --session-token stays an error rather than falling back to the environment: that shape is "$AV_TOKEN" with the variable unset, and running on a session the operator did not name is the outcome worth refusing. --access-bundle alongside a flag token is still the "pass one of them, not both" error, but alongside an environment token it warns and mints, because an exported variable is ambient and may be left over from an unrelated run while the flag was typed for this one. The agent already receives the token either way, as the password in the HTTPS_PROXY URL, so nothing new reaches the child process.
6a5d0e9 to
217a810
Compare
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| return "", fromFlag, fmt.Errorf( | ||
| "--session-token was given but is empty; pass a session token from the dashboard, or drop the flag and set %s", agentVaultSessionTokenEnv) | ||
| } | ||
| token, err = util.GetCmdFlagOrEnvWithDefaultValue(cmd, "session-token", []string{agentVaultSessionTokenEnv}, "") |
There was a problem hiding this comment.
Medium: Overridden session token remains exposed to the child
buildAgentVaultRunEnv copies unrelated entries from os.Environ(), so the launched command still receives INFISICAL_AGENT_VAULT_SESSION_TOKEN. When a flag takes precedence or --access-bundle mints a narrower session, the child can read this stale token and use its potentially broader access despite the CLI claiming to ignore it. Filter agentVaultSessionTokenEnv from the environment passed to the child after resolving the token.
PR overviewThis pull request updates the agent-vault command to read its session token from the environment while supporting flag and access-bundle token resolution when launching child commands. One security issue remains open: a child process can still inherit an overridden session token from the environment. If that stale token has broader permissions than the selected or newly minted token, the child could use it to obtain unintended vault access; no issues have yet been addressed. Open issues (1)
Fixed/addressed: 0 · PR risk: 6/10 |
buildAgentVaultRunEnv copies the parent environment wholesale, so an exported INFISICAL_AGENT_VAULT_SESSION_TOKEN reached the agent. On an --access-bundle run that is the token the command just said it was ignoring: the agent could read it and use it as the proxy password itself, getting whatever the older bundle allowed instead of the bundle that was named. Stripped on every run rather than only on that path. When the variable is the session, the agent already has it as the proxy password, so dropping it costs nothing there and leaves one rule: the agent's session comes from the proxy URL and nowhere else.
1 participant
Description 📣
--session-tokenwas the only Agent Vault flag with no environment fallback.--proxy,--client-id,--client-secretand the proxy's--enrollment-tokenand--data-dirall go throughGetCmdFlagOrEnvWithDefaultValue, so the one credential that carries the whole access bundle had to sit on the command line, where it lands in shell history and in process listings. It now falls back toINFISICAL_AGENT_VAULT_SESSION_TOKEN.Resolution moves into a single helper so the two rules that turn on where the token came from are testable:
--session-token ""stays a hard error rather than falling back to the environment. That shape is"$AV_TOKEN"with the variable unset, and running the agent on a session the operator did not name is the outcome worth refusing.--access-bundlealongside a flag token is still the existing "pass one of them, not both" error. Alongside an environment token it warns and mints instead, because an exported variable is ambient and may be left over from an unrelated run, while the flag was typed for this run.Companion docs PR: Infisical/infisical#8104
The agent process already receives the session token either way, as the password in the
HTTPS_PROXYURL, so this does not widen what the child sees. It only keeps the token out of history and out ofps.Type ✨
Tests 🛠️
# Here's some code block to paste some code snippets