Repository navigation
feat(agent-vault): pick the Agent Vault project and add sessions create/revoke - #488
Conversation
…te/revoke Agent Vault orgs can now hold several projects. Commands that mint sessions or push skills take --project-id, INFISICAL_AGENT_VAULT_PROJECT_ID, the agent-vault.project-id setting in .infisical.yaml, or INFISICAL_PROJECT_ID, in that order. New sessions create and sessions revoke (by ID or --session-token).
|
💬 Discussion in Slack: #pr-review-cli-488-feat-agent-vault-pick-the-agent-vault-project-and-add-sessions Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 4 · PR risk: 0/10 |
 [High impact] The latest changes appear safe to merge; no new blocking issues were found.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings marked 🟡 are optional suggestions and need no follow-up push.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🔴
packages/cmd/agent_vault_project.go— Users upgrading who mint withagent-vault run --access-bundleor runskills pushnow fail in two cases that work on base. The first is when no project is configured. The second is whenINFISICAL_PROJECT_IDis already set for Secrets Management, which is common in CI. With no source, resolveAgentVaultProject errors before any request. When the ID comes fromINFISICAL_PROJECT_ID, the CLI sends that Secrets Management ID and the server answers 404. Fix: when no Agent Vault-specific source is set, leaveprojectIdout (the request tag is alreadyomitempty) so the server falls back to the org's oldest Agent Vault project.Why this was flagged
On base, CallCreateAgentVaultSession and CallListAgentVaultSkillsByName send no project. The PR says the server then falls back to the org's oldest Agent Vault project, so a single-project org works with no setup. After the change, runAgentVaultRun (agent_vault_run.go:321-326), the skills push Run and sessions create all call resolveAgentVaultProject. With none of the four sources set, it returns "no Agent Vault project is set..." at agent_vault_project.go:49-53 and the command exits. If only INFISICAL_PROJECT_ID is set (agent_vault_project.go:184-186), which CI jobs set for
infisical run/export, that Secrets Management UUID goes out as projectId and the mint or skill lookup fails with 404. agentVaultProjectHints only adds a hint and does not recover. The PR accepts this as breaking, but the server fallback it cites would keep these existing scripts working if projectId were omitted. The 404 hint does not prevent the failure.Verification: When no source is set, resolveAgentVaultProject returns "no Agent Vault project is set. Pass --project-id, ..." (agent_vault_project.go:49-53). resolveAgentVaultProjectFromEnvironment falls back to
os.Getenv(util.INFISICAL_PROJECT_ID_NAME)(agent_vault_project.go:75-78). agentVaultProjectHints only adds a hint; the command still fails. Both paths break workflows that work on base. -
🟡
packages/cmd/init.go— Re-runninginfisical initsilently deletes a user'sagent-vault.project-idfrom .infisical.yaml. Later Agent Vault commands then fail or fall back to another project. writeWorkspaceFile builds a freshmodels.WorkspaceConfigFileYamlwith only Version and SecretsManagement, then overwrites the file. The newAgentVaultsection this PR adds to that struct is never carried over. Fix: when .infisical.yaml already exists, read it and keep itsAgentVaultsection (andGeneral) while replacing onlysecrets-management.project-id. The same pre-existing loss already hitsgeneral.domain.Why this was flagged
A user adds
agent-vault: project-id: <uuid>to .infisical.yaml, which this PR makes a documented project source (models/cli.goAgentVaultConfigSection, util/config.go:123). Later they runinfisical initin that directory to link a different Secrets Management project and confirm the overwrite prompt. writeWorkspaceFile (packages/cmd/init.go:341-353) marshals a new struct with only Version and SecretsManagement and writes it with WriteFileAtomic, so the agent-vault section is dropped without warning. The nextagent-vault run --access-bundle,skills pushorsessions creategoes to the next source in resolveAgentVaultProjectFromEnvironment (packages/cmd/agent_vault_project.go:75). That is INFISICAL_PROJECT_ID, often a Secrets Management project, which gives a 404 or a skill pushed to the wrong project; if it is unset, the command fails with "no Agent Vault project is set". Nothing on the init path reads or merges the existing file.Verification: writeWorkspaceFile (packages/cmd/init.go:341-346) builds
models.WorkspaceConfigFileYaml{Version: ..., SecretsManagement: {ProjectID: selectedWorkspace.ID}}and writes it at line 353 without reading the existing file first. Nothing in writeWorkspaceFile copies the new AgentVault section over, so the agent-vault section is erased when the file is rewritten. Why nit and not normal: init already overwrites the whole file on the base branch.
… and timeout; drop .infisical.yaml project - sessions revoke falls back to INFISICAL_AGENT_VAULT_SESSION_TOKEN - revoke by token ignores the domain in .infisical.yaml, like run --session-token - both revoke calls time out after 30s - drop the agent-vault project-id setting in .infisical.yaml
|
On the two points in the review summary:
|
|
@greptile review |
…-multiple-agent-vault-projects-per-organization # Conflicts: # packages/api/agent_vault.go # packages/cmd/agent_vault_run.go
scott-ray-wilson
left a comment
scott-ray-wilson
left a comment
There was a problem hiding this comment.
Ran a4ab1d7 against Infisical/infisical#9160 locally. The project lookup order, the empty and non-UUID refusals, the Secrets Management 404 hint, sessions create -o json and every sessions revoke path behave as described. The comments below are what's left. The server-side ones are on Infisical/infisical#9160.
| } | ||
| minted = &created.Session | ||
| sessionToken = created.Session.Token | ||
| minted = mintAgentVaultSessionOrExit(identity, project, accessBundles, ttl, label) |
There was a problem hiding this comment.
run can mint in a project that doesn't own the proxy
run mints the session in the project from --project-id or the environment without checking that this project owns the --proxy it's about to use. On a mismatch, the agent starts and then every request is refused as an invalid session.
Reproduced against Infisical/infisical#9160 with a real proxy registered in Apollo. With --project-id <Apollo>, run injected Bearer apollo-secret-token and httpbin answered 200. With --project-id <Hermes> (Hermes also has a bundle named apollo-bundle), the same command minted, printed its summary and started the agent with no warning. curl then got http 000 and the proxy logged refused, the session is no longer valid ... status=403. The server's resolve treats proxy.projectId !== session.projectId as "Session not found", which the proxy reports as a dead session. run already has the proxy id from the CA fetch (line 369), so it could refuse the mismatch, or default the project from the proxy, before minting. Released CLIs hit this with any proxy outside the org's oldest project.
There was a problem hiding this comment.
I'd keep this out of the CLI. run only knows the address it's given and what the proxy says about itself, and the server already refuses a session from another project, so the rule stays in one place. A wrong proxy just doesn't work, so I added a troubleshooting note to the docs for that error instead.
There was a problem hiding this comment.
Agreed the rule belongs on the server. The confusing part is the message: a session minted a second earlier is refused with "the session is no longer valid". Could the server's resolve report a project mismatch separately from a dead session, so the proxy says the session is for another project? That keeps the rule in one place.
There was a problem hiding this comment.
Kept the server's 404, since it's deliberate. The proxy's message now says the session is no longer valid, or belongs to a different Agent Vault project than this proxy.
| if err != nil { | ||
| return agentVaultProject{}, err | ||
| } | ||
| if !found { |
There was a problem hiding this comment.
Requiring a project breaks the CLI-first release window
The new CLI refuses run --access-bundle, skills push and sessions create when no project is set. Since the CLI ships before Infisical/infisical#9160, users on servers without it must supply a project ID their server never shows them.
With no project set, sessions create exits with no Agent Vault project is set. Pass --project-id, or set INFISICAL_AGENT_VAULT_PROJECT_ID or INFISICAL_PROJECT_ID. The published docs' GitHub Actions and Dockerfile examples install the latest CLI and run agent-vault run --access-bundle with no project, so they fail on their first run after the CLI release, even against today's prod. On servers without the new release, the Agent Vault UI is org-scoped and never shows a project ID (the old layout resolved it through the hidden GET /api/v1/agent-vault/project), and the error's advice ("copy the ID from the Agent Vault project's settings") points at a page that doesn't exist there. Meanwhile any UUID would "work", since the old server drops the field. Omitting projectId when none is set (the new server falls back anyway) would keep old and new servers working through the release window.
There was a problem hiding this comment.
Agreed there's a gap, but adoption of the Agent Vault CLI commands is still low, and we'll release the CLI and the server on the same day, so the window is a few hours. Anyone caught in it only needs to set the project ID, which they need on the new server anyway, so I'd keep the CLI strict rather than make it lean on the fallback.
There was a problem hiding this comment.
Same-day release works for me. Worth a line in the CLI release notes saying Agent Vault commands now need INFISICAL_AGENT_VAULT_PROJECT_ID or --project-id, and that the ID is under Settings > General after the server update.
There was a problem hiding this comment.
Our CLI release notes are generated from PR titles. The CLI's error names --project-id and the two variables, and the docs say where to find the ID.
- Agent Vault commands ignore the domain in .infisical.yaml - show the real login error instead of 'expired' - never echo a session token passed as the revoke argument - lowercase project IDs
|
@greptile review |
scott-ray-wilson
left a comment
scott-ray-wilson
left a comment
There was a problem hiding this comment.
Verified the fixes on 8b8d3a7 against Infisical/infisical#9160: a token passed as the argument is refused without being printed, uppercase project IDs work, and create and revoke now use the same server. One follow-up on the domain change below.
| } | ||
| minted = &created.Session | ||
| sessionToken = created.Session.Token | ||
| minted = mintAgentVaultSessionOrExit(identity, project, accessBundles, ttl, label) |
There was a problem hiding this comment.
Agreed the rule belongs on the server. The confusing part is the message: a session minted a second earlier is refused with "the session is no longer valid". Could the server's resolve report a project mismatch separately from a dead session, so the proxy says the session is for another project? That keeps the rule in one place.
| if err != nil { | ||
| return agentVaultProject{}, err | ||
| } | ||
| if !found { |
There was a problem hiding this comment.
Same-day release works for me. Worth a line in the CLI release notes saying Agent Vault commands now need INFISICAL_AGENT_VAULT_PROJECT_ID or --project-id, and that the ID is under Settings > General after the server update.
… say a session can be for another project
…-multiple-agent-vault-projects-per-organization # Conflicts: # packages/cmd/profile.go
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 39278625 | Triggered | JSON Web Token | 5b081ff | packages/util/cli_error_test.go | View secret |
| 39278625 | Triggered | JSON Web Token | 5b081ff | packages/util/cli_error_test.go | View secret |
| 39723066 | Triggered | Generic High Entropy Secret | fa6f110 | packages/cmd/agent_vault_project_test.go | View secret |
| 39723066 | Triggered | Generic High Entropy Secret | 2a490c4 | packages/cmd/agent_vault_errors_test.go | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
…e quiet under --silent and -o
… that start with infisical
…p the project ID out of the non-member error
… runs through, and code access bundle refusals
|
@scott-ray-wilson thanks, all addressed:
|
Description 📣
Agent Vault orgs can now hold several projects, so the commands that need one take a project.
agent-vault run --access-bundle,agent-vault skills pushand the newsessions createresolve the project from--project-id, thenINFISICAL_AGENT_VAULT_PROJECT_ID, thenINFISICAL_PROJECT_ID. With none set they stop with a message naming all three. An empty flag or a non-uuid is refused, and a 404 when the id came fromINFISICAL_PROJECT_IDhints that it may be a Secrets Management projectinfisical agent-vault sessions create: mints a session and prints only the token,-o json|yamlfor id, token, projectId, accessBundle, label and expiresAt.--ttldefaults toneverlikerun. Doesn't open the login wizard when stdout isn't a terminalinfisical agent-vault sessions revoke <session-id>or by token (calls the new token-authenticated revoke route), no prompt. The token comes from--session-token, elseINFISICAL_AGENT_VAULT_SESSION_TOKEN, so scripts can keep it off the command line.--session-tokenrather than--tokenso the root command's token handling never reads it.infisical.yamlat all (that file is for Secrets Management), so a cloned repo can't redirect credentials or tokens andsessions createandrevokealways talk to the same server. They use--domain,INFISICAL_DOMAINor the login. Both revoke calls time out after 30s, likerun's cleanuprunresolves the project only when minting, refuses--project-idwith--session-token, and shows the project and its source in the run summary.run --session-tokenis unchangedprofile currentshows the Agent Vault project and where it came fromOlder CLIs keep working: the server falls back to the org's original Agent Vault project when no project is sent.
Infisical PR: Infisical/infisical#9160
Python SDK PR: Infisical/python-sdk-official#68
Node SDK PR: Infisical/node-sdk-v2#47
Type ✨
Tests 🛠️
# Here's some code block to paste some code snippets