feat(agent-vault): the proxy and infisical av commands by saifsmailbox98 · Pull Request #386 · Infisical/cli

saifsmailbox98 · 2026-09-04T22:27:56Z

Context

The command-line half of Agent Vault, a new product whose dashboard and API are in Infisical/infisical#8000. The two merge together. Agents run holding no credentials: their traffic goes through a proxy, and the proxy attaches the real credential on the way out, only for the hosts that agent is allowed to reach.

Two commands, both under infisical av.

infisical av proxy runs the proxy. You create it in the dashboard, paste the one-time token it gives you, and the proxy takes it from there — it sets itself up, then keeps asking Infisical what each running agent is allowed to reach. Settings changed in the dashboard reach it on its own, without a restart, and so does a revoked session.

infisical av run starts an agent behind that proxy. It gets a session for the bundles you name, points the agent's traffic at the proxy, and runs your command. When the agent exits, the session it created is revoked.

What it does not do: it does not sandbox the agent. It sets a few environment variables and starts your command, so the agent can read files and reach the network like any program you run yourself. What it cannot do is hold a credential, because it is never given one.

Steps to verify the change

Create a proxy in the dashboard under Agent Vault → Proxies and copy the one-time token.
infisical av proxy --enrollment-token <token> on the machine that will carry agent traffic.
Create an access bundle with a credential, then run something through it: infisical av run --access-bundle <name> --proxy <host:port> -- curl -sS https://api.github.com/user. GitHub answers. The same command outside the agent answers 401.
Point the agent at a host no bundle covers. Under Deny the proxy refuses it; under Allow it goes through with no credential.
Revoke the session in the dashboard. The agent stops getting credentials within a minute.

Type

Checklist

Title follows the conventional commit format
Tested locally
Updated docs (if needed) — the docs live in the Infisical repo PR
Updated CLAUDE.md files (if needed)
Read the contributing guide

packages/agentvault is a sibling of packages/agentproxy, copied rather than imported so the old package stays independently deletable. The matcher drops paths and defaults a portless pattern to 443, which collapses the inherited three-rung ladder to one: exact host beats wildcard, then slice order, which is the order resolve returned - bundle position first. testdata/host-pattern-fixture.json is the same file the backend's Vitest suite reads, so both grammars are held to one contract. The cache is keyed by the sha256 of the session token rather than the token itself (the shipped agentproxy cache holds the raw JWT as both map key and entry field), zeroes credential bytes on every eviction path, and gives 404 its own arm so a deleted actor's session stops immediately instead of riding out the unreachable-Infisical grace window.

The MITM server, its own self-signed per-proxy CA, the data directory, and the poll loop that heartbeats and refreshes every live session. Traffic policy is server-owned and arrives on every poll, so --unmatched-host and --poll-interval do not exist here; only what the server cannot know stays as a flag. The settings block is persisted, so a restart during an Infisical outage keeps a deny policy instead of silently coming back up allowing. Two things the inherited proxy got wrong are fixed rather than carried over: private and link-local ranges are blocked unconditionally, not as a side effect of deny mode (under the allow default the old shape leaves the proxy an SSRF pivot into its own host's metadata service), and listener saturation logs a warning instead of blocking silently while the agent hangs. Port 17323, not 17322: the shipped secrets agent-proxy start default is not being removed and both are expected to run on one box.

refresh() zeroed the old credential bytes before swapping in the new ones, but get() hands requests the slice by reference, so a request that resolved just before the tick injected \x00 bytes upstream. Reproduced with go test -race. Zeroing is dropped rather than narrowed: it defended only against someone reading the proxy's memory, and anyone there has the proxy token on disk and can resolve every session directly. The blocked-address DNS lookup now runs after the session resolves, on both CONNECT and plain forwards, so a well-formed but bogus Proxy-Authorization header can no longer make the proxy resolve arbitrary names.

Bypass was evaluated before anything else, so a host on the list was tunnelled untouched even when a connection in the session covered it. The credential was silently never attached, which reads as a broken credential rather than a bypassed host. The session is already resolved a few lines above for its own reasons, so the match now happens first and costs nothing. Bypass governs the hosts nothing else covers, which is the case it exists for: clients that pin certificates.

There was no coverage of the bypass list at all, on either side of the change. Asserts the three cases handleConnect distinguishes: covered and bypassed is opened so its credential still lands, bypassed and uncovered is tunnelled untouched, and neither falls through to the unmatched-host policy.

…hing more Bypass used to skip interception entirely: no certificate, no inspection, and the unmatched-host policy ignored. That made a bypassed host behave unlike every other reachable host for a reason nobody using the product asked about. It is now a proxy-wide exception to deny and only that. A host on the list is opened, given the proxy's certificate and forwarded without a credential, exactly as an allowed host is. It saves naming every such host as a pass-through connection in a bundle, and it belongs to whoever runs the proxy rather than whoever owns the bundle. Under allow it changes nothing. The guard in forward() already did this, so the change is the deletion of the CONNECT branch and tunnelOpaque with it. Raw tunnelling was the only way WebSocket, HTTP/2-only and long-stream traffic reached an upstream through the proxy; those now have to go around it until the proxy carries them.

The write deadline on a forwarded response is absolute and set at request start, so a stream was cut at exactly 30 minutes with data still flowing. The client saw the response simply end, with no error - the shape SSE, MCP over server-sent events and log tails all take. flushingWriter already sits on every byte we hand back, so it refreshes the deadline on each flushed chunk. A response now lives as long as it keeps producing, and one that goes quiet for five minutes is still closed. The absolute timeouts stay as they were: they now only bound a response that has produced nothing yet, which matters because the upstream transport sets no ResponseHeaderTimeout and a slow API is not a stalled one.

av run does not isolate the agent: it sets variables and execs, sharing the filesystem and the network with whatever launched it. Removing a handful of Infisical variable names on the way through suggested a boundary that isn't there, while the agent could read the same values off disk regardless. The proxy settings are still replaced, since ours have to win, and the CA-trust variables are still added. Nothing else is touched.

Trusting the proxy's certificate authority puts a macOS dialog on screen, and av run had printed nothing by the time it appeared. The first command a new user runs looked hung rather than blocked on a question, and over SSH or in CI, where the dialog can never be answered, it waited forever. It returns for every new proxy, since each has its own authority. The step now says what it is about to ask for before it asks, gives up after thirty seconds with the run continuing, and is skipped entirely when stdin is not a terminal and nothing could answer. Agent Vault gets its own trust helper rather than changing ensureCATrusted, which the agent-proxy command shares; it reuses that file's already-trusted check and error type.

The proxy refused every private and link-local address before any policy check, so an internal API inside the operator's own network was unreachable - which is half of what people want a credential-injecting egress proxy for, and self-hosted is the common case. netguard.go and its two call sites are gone. unmatchedHost is the control instead of an unconditional rule: under deny the proxy reaches only configured hosts, wherever they live. The accepted consequence is that under the allow default an agent can reach the metadata endpoint of the machine the proxy runs on, and with it that machine's cloud role. That was the reason the block existed; the trade was made deliberately in favour of the product being usable inside a network. Also drops a DNS lookup from every CONNECT, since the rebinding check resolved the name before dialling it.

Matching lowercased the host and dropped a trailing dot; the certificate, its cache key, the Host header and the dial used whatever the client typed. So the two disagreed, three ways: - a host reached in unusual case was brokered and passed upstream in that case - a trailing dot matched, then minted a leaf whose SAN carried the dot, which is not valid name syntax, so the client got an opaque TLS error for a host the proxy had already decided to broker - every capitalisation minted and cached its own certificate, a P-256 keygen and a signature each, so a session holder could churn the 8192-entry leaf cache with nothing but case parseConnectTarget and parseForwardTarget now normalise, which is where HTTP says to do it and where every other proxy does. req.Host is derived from req.URL.Host, so the pinned authority follows without a second change. IP literals are unaffected: matching compares them by value. Two tests, both failing without the change: four spellings parse to one host, and four capitalisations reuse one certificate.

The version had one consumer, a column on the Proxies page, and nothing branched on it: no compatibility gate, no upgrade prompt, no alerting. The gateway and relay track no self-reported version either - the gateway's analogue is its capabilities blob, which exists because it is consulted. The heartbeat now carries no body at all: the token says which proxy it is and the timestamp says it is alive.

A spent token in a spec is refused, not specifically 401: the enrollment path is shared with gateway, relay and KMIP and answers 400. Nothing branches on the status, so the comment states the behaviour and leaves the number to the API.

Port 0 was treated as "unset" and replaced with the default, so a deployment script interpolating an empty variable landed on the standard port instead of failing - and the conventional meaning of 0, bind anything free, was unavailable. The flag's own default already covers an omitted --port, so the substitution only ever caught a deliberate 0. The startup line now reports the port actually bound rather than the one asked for, which is the only way an operator learns where a 0 landed. Out-of-range values say so instead of surfacing a listen error. Verified: --port 0 bound 63316, logged it, and served on it; 99999 and -1 are refused with the range.

Both describe a session this command mints, and neither can reach one minted in the dashboard: its expiry was fixed at creation, and av run never revokes a session it was handed. --ttl was even validated against the allowed values on that path, so it looked understood while doing nothing - someone shortening a long-lived token this way would believe they had. Refused rather than warned past: what the operator is asking for does not happen either way, so continuing would be the surprise. Keyed on whether the flag was typed, since both carry a default.

A pass-through connection attaches nothing, but any match was logged as brokered, so the line claimed a secret left the machine when none did - and that line is the only record of it, since resolve is not audited and there is no request stream. It reads as passthrough now, like any other forwarded request that carried no credential. The two cases stay distinguishable without a fourth value: the connection and accessBundle fields appear only when a connection matched.

Every allowed request was logged at debug while the proxy runs at info, so the one line recording that a credential left the machine was invisible by default. Nothing else records it: resolve is not audited and there is no request stream, so an operator who wanted to know which host got a secret had to have known to start with -l debug and to have kept stdout. Brokered is now info. Uncovered-host traffic stays at debug, so a default run reads as one line per credential handed out rather than one per request. Blocked and errors are unchanged. The line carries the path without its query string, so promoting it does not start writing query-string tokens into operator logs.

It answered "what can this session reach right now" to anyone holding a session token, and av run printed the result before starting the agent. The dashboard already shows a bundle's connections and hosts to whoever holds it, so this was a convenience that saved a context switch, not a source of anything else. Removing it also settles the disagreement it created: it handed a member the proxy's unmatched-host setting and bypass list, two of the columns the Proxies page deliberately shows only to administrators. The run summary keeps the proxy name, its CA fingerprint and the session expiry. /_agent-vault/ca stays: a public certificate is public, and fetching it is how an agent trusts the proxy.

infisical-review-police · 2026-09-04T22:28:00Z

💬 Discussion in Slack: #pr-review-cli-386-feat-agent-vault-the-proxy-and-infisical-av-commands

Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel.

veria-ai · 2026-09-04T22:30:57Z

PR overview

This pull request adds the agent-vault proxy and Infisical AV commands, including CONNECT handling and certificate generation for proxied targets.

Six issues have been addressed, with one availability concern remaining. A client with a valid session token can submit oversized, unique CONNECT targets that consume substantial proxy memory through cached certificates, potentially disrupting the proxy. Enforcing DNS length limits before certificate generation would close the remaining gap.

Open issues (1)

Low: Oversized CONNECT targets can exhaust proxy memory — packages/agentvault/proxy.go:173

Fixed/addressed: 6 · PR risk: 5/10

greptile-apps · 2026-09-04T22:31:53Z

Greptile Summary

This PR introduces the Agent Vault CLI workflow for running a credential-brokering proxy and launching agents through it.

Adds proxy enrollment, persisted state, CA generation, TLS interception, credential injection, host-policy matching, and session refresh behavior.
Adds infisical av proxy and infisical av run, including environment configuration and best-effort session revocation.
Adds tests for matching, cache behavior, proxy streams and hostnames, command execution, and Darwin trust handling.
The changes since the previous review make session cleanup nonfatal and avoid prompting or exiting when revocation credentials are no longer valid.

Confidence Score: 5/5

The PR appears safe to merge because the latest cleanup change introduces no new actionable failure and no previous blocking finding remains outstanding.

The cleanup path now reports revocation failures without replacing the child’s exit result or triggering an unattended login flow. The remaining inability to revoke after identity expiry is the already reviewed cleanup limitation, while the proxy transport and internal-destination behavior were explicitly discussed and withdrawn or resolved in their existing threads.

Important Files Changed

Filename	Overview
packages/cmd/agent_vault_run.go	Cleanup now resolves revocation credentials without prompting or terminating the command and warns when revocation cannot be completed.
packages/agentvault/proxy.go	Implements authenticated forwarding, TLS interception, host-policy enforcement, credential injection, and bounded proxy behavior.
packages/agentvault/cache.go	Caches resolved sessions, refreshes policy and credentials, and fails closed after a bounded control-plane outage.
packages/agentvault/store.go	Persists enrollment, proxy configuration, and CA state; the previously reviewed non-atomic update concern was manually resolved.
packages/agentvault/match.go	Implements normalized host and port matching with exact-host precedence and one-label wildcard semantics.

_{Reviews (2): Last reviewed commit: "fix(agent-vault): say so when a session ..." | Re-trigger Greptile}

av run revokes the session it minted once the agent exits, and resolved the identity again to do it. A run long enough to outlive the login exited on the expiry check, or opened the login wizard on a terminal nobody is watching, and either way the session was left behind - indefinitely with --ttl never. Nothing can be done about an expired login at that point, so the cleanup path now reports it and names where to clear the session instead of ending the command. Runs that pass --token are unaffected: they mint nothing and revoke nothing.

saifsmailbox98 · 2026-09-05T01:09:03Z

@greptile review

…tificate saveCa writes the key and then the certificate, so a re-enrollment interrupted between the two leaves a new key beside the old certificate. Both parse, resolveState calls the proxy enrolled, and every request then fails to mint a leaf with nothing at the proxy to say why. loadCa now checks the pair and refuses with a message naming the directory and the way out.

The proxy's CA arrives as PEM text. The fingerprint was taken from the first certificate in it, but the whole text was written to the CA file, so anything appended after the genuine certificate was trusted without ever being checked. Only the first certificate is kept now.

The shared CLI client sets retries but no deadline, so a control plane that accepted the connection and never answered blocked the poll loop on one call forever, and every session was served from the cache meanwhile. The grace window only applied when a refresh failed, never when one hung. Every call the proxy makes to Infisical now has a 15 second deadline, and a read past the grace window treats the entry as a miss and re-resolves rather than serving it.

…esty client The CA endpoint is served by the proxy, not by Infisical, so the shared resty builder is wrong for it: it attaches INFISICAL_CUSTOM_HEADERS, which are credentials for a gateway in front of Infisical. A bare resty client avoided that but tripped the guard test that keeps every resty client on the shared retry policy. Every other call to a non-Infisical host in the CLI uses net/http directly, so this one now does too, and the call moves out of the Infisical API package.

The backend now compares an IPv4-mapped IPv6 pattern as the IPv4 host it names, which is what net.ParseIP already did here. The fixture records both directions.

The root command reads --token as an Infisical token and warned that the logged-in session was being overwritten on every dashboard-minted run. The session token is a different thing and now has a name the root never reads.

A revoked or deleted proxy restarted and kept running, logging a 401 heartbeat once a poll with no sign that anything was wrong, while every session on it resolved to nothing. A heartbeat 401 is never transient (an outage is a timeout or a 5xx and rides out the grace window), so after two consecutive rejections the proxy shuts down with a message that says what to do.

gitguardian · 2026-09-07T04:13:17Z

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
37048879	Triggered	Generic CLI Secret	`fe67adb`	packages/cmd/agent_vault_run.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 secret safely. Learn here the best practices.
Revoke and rotate this secret.
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.}

--access-bundle is accepted once. The flag stays a StringArray so a repeat is an error rather than a silent last-wins, and the backticked usage word makes help print it as a single name. Matches the backend's V1 cap.

scott-ray-wilson

Deep review of the branch at 63113ec. The 15 inline comments below are ordered by severity here; each was verified against the built code with live probes, except the store.go one, which is marked as plausible because I could not confirm the server ever emits the triggering value.

Proxy URL has no password, so Node and Python clients get 407 (packages/cmd/agent_vault_run.go:383)
Infisical credentials are passed through to the agent's environment (packages/cmd/agent_vault_run.go:346)
The 401/404 classifier is wrong in both directions (packages/agentvault/cache.go:87)
Deny policy is a fail-open exact string match (packages/agentvault/proxy.go:338)
Bypass hosts default to port 443, so plain http is still blocked under deny (packages/agentvault/proxy.go:377)
In-tunnel session loss returns 502 with the raw error text (packages/agentvault/proxy.go:285)
CA trust variables replace the system store, breaking every NO_PROXY host (packages/cmd/agent_vault_run.go:370)
Enrollment spends the one-time token before any disk write, and writes are not atomic (packages/agentvault/run.go:34)
IPv6 Host header loses its brackets on the default port (packages/agentvault/proxy.go:455)
One fixed CA path is clobbered by a second run (packages/cmd/agent_vault_run.go:149)
Session is minted before the CA write, and a write failure skips the revoke (packages/cmd/agent_vault_run.go:208)
A newline in a persisted value overrides earlier keys (plausible, trigger unconfirmed) (packages/agentvault/store.go:185)
ForceAttemptHTTP2: false does not disable HTTP/2 (packages/agentvault/proxy.go:111)
The "bare" CA fetch client honours the shell's HTTP_PROXY (packages/cmd/agent_vault_run.go:35)
The resolver is a one-shot client on the hot path, with retries and no coalescing (packages/agentvault/resolve.go:36)

scott-ray-wilson · 2026-09-08T01:33:34Z

+
+// The token rides as the Proxy-Authorization username on every CONNECT, in the clear on the hop to the proxy.
+func agentVaultProxyURL(proxyAddr, sessionToken string) string {
+	u := url.URL{Scheme: "http", User: url.User(sessionToken), Host: proxyAddr}


Proxy URL has no password, so Node and Python clients get 407

This URL carries a username and no password. undici (Node fetch, which NODE_USE_ENV_PROXY=1 switches on), Python urllib and requests only send Proxy-Authorization when both parts are present, so Claude Code and any Python agent get a 407 on every CONNECT; only curl works. url.UserPassword(sessionToken, "x") fixes it: the proxy's strings.Cut on the decoded credentials still yields the token, and agent_proxy.go:255 already does this for the sibling command. The two tests asserting the user-only URL (agent_vault_run_test.go:70 and :131) need updating.

Adding evidence from the end-to-end matrix: git is affected as well, not only Node and Python. Through the proxy, git ls-remote https://github.com/... fails with could not read Password for 'http://agv_...@127.0.0.1' because libcurl treats the username-only proxy URL as needing a password and never sends Proxy-Authorization; plain curl tolerates it. Since git and gh are the most common agent tools, url.UserPassword(sessionToken, "x") (any non-empty password half) fixes git, Node and Python in one line.

scott-ray-wilson · 2026-09-08T01:33:35Z

+// Nothing else is removed.
+func buildAgentVaultRunEnv(parent []string, proxyAddr, sessionToken, caPath, extraNoProxy string) []string {
+	stale := map[string]bool{}
+	for _, k := range proxyEnvKeys {


Infisical credentials are passed through to the agent's environment

Only the proxy variables are stripped, so INFISICAL_UNIVERSAL_AUTH_CLIENT_ID/_SECRET, INFISICAL_UNIVERSAL_AUTH_ACCESS_TOKEN and INFISICAL_TOKEN from the parent go straight into the agent's environment. That is the machine identity the CLI just used to mint the scoped session; with it the agent can call the API directly and mint sessions over any bundle, which defeats the per-host policy. The help text promises "nothing else from Infisical", and both sibling env builders strip credentialEnvKeys. Suggest adding those keys plus INFISICAL_TOKEN to stale. c6aa146 removed this on the grounds that the agent could read the same values off disk, which does not hold for env-only credentials.

scott-ray-wilson · 2026-09-08T01:33:35Z

+	if errors.As(err, &apiErr) {
+		// A 404 is neither a 401 nor a 5xx, so without its own arm it would fall into the unreachable-Infisical
+		// branch and keep brokering.
+		return apiErr.StatusCode == 401 || apiErr.StatusCode == 404


The 401/404 classifier is wrong in both directions

A 403, 400 or 422 from /proxy/resolve is treated as an outage, so handleRefreshFailure keeps injecting the cached credential for the full grace window (5 x pollInterval) and isTokenRejected never fires on the heartbeat. Conversely, a 401 caused by the proxy's own token being revoked is read as "session gone", so every cached session is dropped and agents are told their session is invalid. Suggest treating any definitive 4xx other than 408/429 as terminal for the session, the way the sibling's isAuthError treats 401 and 403 alike, and using APIError.Name to tell "proxy token rejected" apart from "session gone".

scott-ray-wilson · 2026-09-08T01:33:35Z

+
+	matched := bestMatch(connections, hostname, port)
+
+	if matched == nil && ps.currentConfig().UnmatchedHost == UnmatchedDeny && !ps.isBypassed(hostname, port) {


Deny policy is a fail-open exact string match

UnmatchedHost is stored raw from enrollment, every heartbeat and proxy.conf, with only "" normalised, so any value other than exactly deny silently means allow. I probed block (the old constant name recorded at resolve.go:11), Deny, DENY, deny and denied, and all reached the upstream. tick() then persists the bad value so it survives restarts. There is no CLI flag here, so this string is the only source. Parse it into a closed type at the boundary and fail closed (or at least Warn) on anything unrecognised, as agent_proxy_start.go:20 does for its flag.

scott-ray-wilson · 2026-09-08T01:33:35Z

+	if raw == "" {
+		return false
+	}
+	for _, pattern := range parseHostPatterns(raw) {


Bypass hosts default to port 443, so plain http is still blocked under deny

parseHostPatterns defaults a portless entry to 443 (match.go:27), so a bypass host written as a bare name only exempts HTTPS. Under deny, GET http://169.254.169.254/... still gets a 403 even though bypass entries carry no credential and forward() already refuses to inject over plaintext. The 443 default protects credentials, which is moot here. Suggest matching bypass entries on hostname only (the sibling's hostAllowlisted does), and adding a port-80 case to TestBypassIsAnExceptionToDeny, which currently only exercises 443.

scott-ray-wilson · 2026-09-08T01:33:35Z

+		if err := os.MkdirAll(filepath.Dir(caFile), 0o700); err != nil {
+			util.HandleError(err, "Unable to create the directory for the certificate authority file")
+		}
+		if err := os.WriteFile(caFile, caPEM, 0o600); err != nil {


Session is minted before the CA write, and a write failure skips the revoke

The session is minted at line 183, before these writes, and both util.HandleError calls here and at 206 exit via os.Exit(1), so the revoke block at 236-254 never runs. An unwritable --ca-file directory or a full disk leaves a live session (default TTL 7d, or never) with no message telling the operator to revoke it. The CA was already fetched and pin-checked at 152-163, so the simplest fix is to move this write block above the mint; alternatively route these failures through a fail closure that revokes first, as the sibling does at agent_proxy_run.go:109.

scott-ray-wilson · 2026-09-08T01:33:35Z

+		{confBypassHosts, state.Config.BypassHosts},
+		{confPollInterval, strconv.Itoa(state.Config.PollInterval)},
+	} {
+		fmt.Fprintf(&b, "%s=%s\n", pair[0], pair[1])


A newline in a persisted value overrides earlier keys (plausible, trigger unconfirmed)

Values are written unescaped, and loadState splits on newlines and cuts at the first '=' with last-wins assignment. A server-controlled value containing a newline (ProxyName, or a BypassHosts entry) does not round-trip: it is truncated and any KEY=VALUE text after the newline is parsed as a real key. BypassHosts is written 6th of 7, so a value like x.example.com\nINFISICAL_AGENT_VAULT_UNMATCHED_HOST=allow flips a deny proxy to allow on the next restart (probed with a hand-built file). I could not confirm the server ever emits a newline, so this is about robustness of the format rather than a live exploit. JSON, which the repo's other state files use, or escaping removes the class.

scott-ray-wilson · 2026-09-08T01:33:35Z

+		TLSHandshakeTimeout: tlsHandshakeTimeout,
+		// Deliberate: an h2 response has no HTTP/1.1 length framing, so re-serializing it into the tunnel
+		// would hang the client.
+		ForceAttemptHTTP2: false,


ForceAttemptHTTP2: false does not disable HTTP/2

With nil TLSClientConfig, dialers and TLSNextProto, this field is a no-op and Transport.protocols() enables h2 by default; probing the verbatim transport against an h2 upstream gives resp.ProtoMajor == 2. The sibling disables it with an empty non-nil TLSNextProto map (agentproxy/proxy.go:200) and that line was dropped here. Relaying still works because forwardHTTP re-frames via the ResponseWriter, but a GOAWAY on the shared multiplexed connection fails every in-flight brokered request at once, which the design never considered. Either add TLSNextProto: map[string]func(string, *tls.Conn) http.RoundTripper{} (or Protocols.SetHTTP2(false)), or fix the comment.

scott-ray-wilson · 2026-09-08T01:33:35Z

+const agentVaultCaFileName = "ca.pem"
+
+// Served by the proxy itself over plain HTTP, not by Infisical, so it uses a bare net/http client.
+var agentVaultProxyHTTPClient = &http.Client{Timeout: 10 * time.Second}


The "bare" CA fetch client honours the shell's HTTP_PROXY

A nil Transport means http.DefaultTransport, whose Proxy is ProxyFromEnvironment, so this client routes the CA fetch through any HTTP_PROXY in the operator's shell for every non-loopback proxy address. With a corporate proxy set and no NO_PROXY entry for the LAN, av run fails with "Unable to reach the Agent Vault proxy" even though the proxy is directly reachable; when it does succeed, the CA the agent will trust has transited a third party. Give it &http.Transport{Proxy: nil} (the proxy's own upstream transport already does this) and consider bounding the JSON decode with io.LimitReader.

scott-ray-wilson · 2026-09-08T01:33:35Z

+	if err != nil {
+		return nil, err
+	}
+	httpClient.SetAuthToken(r.proxyToken()).SetTimeout(controlPlaneTimeout)


The resolver is a one-shot client on the hot path, with retries and no coalescing

This builds a fresh resty client, transport and connection pool per call, on the request path, with DefaultRetryPolicy (429 retried up to 3 times honouring Retry-After up to 10s, 15s per attempt). Combined with get() having no singleflight and caching nothing on failure, any junk or concurrent CONNECT becomes an authenticated POST /proxy/resolve: three junk CONNECTs produced three control-plane POSTs, and one CONNECT against a 429 stub produced four. refresh() then resolves every cached session sequentially inside tick(), so 20 rate-limited sessions hold the poll loop for minutes and later entries age past grace() and get evicted on the request path. Suggest one long-lived client built at Start, a no-retry or bounded policy for in-request resolves, singleflight keyed by session (already in go.mod via the sibling's leases.go), and a bounded-parallel refresh.

linear · 2026-09-08T01:33:42Z

AGE2-86

scott-ray-wilson

Follow-up round: an exhaustive manual test matrix (8 slices: flag validation, enrollment lifecycle, credential injection, host matching, bypass/settings, session lifecycle, CA/env, protocol edge cases) driven against a local dev instance at 63113ec. These are the NEW items beyond the 15 inline comments already posted; each was reproduced live and confirmed against source. Two medium, ten low.

The matrix also produced strong positives worth recording: the host-pattern matcher matches the backend grammar exactly across exact/wildcard/IPv6/IPv4-mapped/precedence cases; basic-auth injection is byte-exact; Proxy-Authorization is always stripped; per-host isolation holds; session mint/revoke/keep/TTL all behave as documented; settings propagate within one poll interval; no panics anywhere.

Empty-host CONNECT / forward targets are accepted (packages/agentvault/proxy.go:406)
Losing only the access token reads as "never enrolled" and recovery rotates the CA (packages/agentvault/run.go:78)
Bearer prefix is always joined to the value with one inserted space (packages/agentvault/rewrite.go:24)
Connection-listed hop-by-hop headers are not stripped (packages/agentvault/rewrite.go:38)
Custom-header bearer gets a bogus "Bearer" prefix when the prefix is omitted (packages/agentvault/resolve.go:70)
Heartbeat token-rejection treats only 401 as terminal (packages/agentvault/run.go:210)
Session-gone / resolve-failure denials at the gate are never logged (packages/agentvault/proxy.go:163)
A signal-killed agent makes the CLI exit 255 instead of 128+signal (packages/cmd/agent_vault_run.go:446)
--log-format is unvalidated and case-sensitive (packages/cmd/agent_vault.go:90)
--proxy scheme trimming is lowercase-only (packages/cmd/agent_vault_run.go:136)
--proxy flag value is not trimmed, unlike the env var (packages/cmd/agent_vault_run.go:133)
Inconsistent handling of empty --access-bundle values (packages/cmd/agent_vault_run.go:108)
Non-canonical ports (leading zeros) validate but never match (packages/agentvault/match.go:85)

scott-ray-wilson · 2026-09-08T21:34:48Z

+	return strings.ToLower(strings.TrimSuffix(strings.TrimSpace(host), "."))
+}
+
+func parseConnectTarget(target string) (hostname, port string, err error) {


[medium] Empty-host CONNECT / forward targets are accepted

net.SplitHostPort(":443") returns an empty host with a nil error, and nothing here rejects it, so CONNECT :443, CONNECT :, and GET http://:18080/ are all accepted: the proxy mints a leaf for an empty CN and dials a loopback service on that port. Observed live through the proxy: CONNECT :443 returned 200 Connection Established and GET http://:18080/ was forwarded to the local echo. Under unmatchedHost=deny an empty-host bypass entry would also match. Reject an empty hostname with 400 in both parseConnectTarget and parseForwardTarget.

scott-ray-wilson · 2026-09-08T21:34:48Z

+	}
+	hasCa := key != nil && cert != nil
+
+	alreadyEnrolled := stored.AccessToken != "" && hasCa


[medium] Losing only the access token reads as "never enrolled" and recovery rotates the CA

alreadyEnrolled is derived from AccessToken != "" && hasCa, so if proxy.conf is truncated to zero bytes or just loses its ACCESS_TOKEN line while ca.key/ca.crt remain valid, a tokenless restart reports this proxy has not enrolled yet. The only recovery is to re-enroll with a new token, which replaces the certificate authority (the warn at run.go:88), so every agent that trusted the old CA breaks. Verified by blanking the token line and restarting. Consider treating a present, valid CA plus a missing token as a recoverable state, and failing loudly on partial/corrupt state rather than silently reporting "not enrolled".

scott-ray-wilson · 2026-09-08T21:34:48Z

+		}
+		value := string(cred.value)
+		if cred.headerPrefix != "" {
+			value = cred.headerPrefix + " " + value


[low] Bearer prefix is always joined to the value with one inserted space

value = cred.headerPrefix + " " + value forces exactly one space, so credential schemes whose prefix abuts the value cannot be expressed: prefix token= yields token= <value> (spurious space), sk- yields sk- <value>, and a no-space API-key prefix is impossible. Bearer/Token only work because the backend trims the trailing space and this re-adds one. This matches the documented one-space model and the backend, so it is an expressiveness gap rather than a spec deviation, but worth deciding deliberately (e.g. let the prefix carry its own separator).

scott-ray-wilson · 2026-09-08T21:34:48Z

+}
+
+// stripHopByHopHeaders also deletes Upgrade, which is why WebSocket upgrades cannot be forwarded.
+func stripHopByHopHeaders(header http.Header) {


[low] Connection-listed hop-by-hop headers are not stripped

stripHopByHopHeaders deletes only a fixed list and never parses the token list in the request's (or response's) Connection header, which RFC 7230 6.1 requires. Verified: curl -H 'Connection: X-Test' -H 'X-Test: leak' results in X-Test: leak reaching the upstream (the Connection header itself is dropped). The sibling agentproxy iterates the Connection list first; this copy dropped that. Same class as the earlier hop-by-hop review note.

scott-ray-wilson · 2026-09-08T21:34:48Z

+		return credential{
+			kind:         credentialBearer,
+			headerName:   wire.HeaderName,
+			headerPrefix: wire.HeaderPrefix,


[low] Custom-header bearer gets a bogus "Bearer" prefix when the prefix is omitted

Omitting headerPrefix on a bearer connection with a custom headerName (e.g. X-Api-Key) results in X-Api-Key: Bearer <value> instead of the value alone. The root cause is a backend default (the stored credential comes back with headerPrefix":"Bearer") that the CLI forwards verbatim here; setting headerPrefix: "" explicitly gives the correct value-alone result. Flagging on the CLI side because this is where the resolved credential is consumed; the fix likely belongs in the backend default, but the CLI could also treat a custom header with no explicit prefix as value-alone.

scott-ray-wilson · 2026-09-08T21:34:48Z

+	avProxyCmd.Flags().String("data-dir", "",
+		fmt.Sprintf("where to keep the certificate authority and proxy token (default: %s)", defaultDataDirHelp()))
+	avProxyCmd.Flags().Int("port", agentvault.DefaultPort, "port to listen on; 0 binds any free port, which the startup line then reports")
+	avProxyCmd.Flags().String("log-format", "console", "log output format: console | json")


[low] --log-format is unvalidated and case-sensitive

Only the exact lowercase json is honored; JSON, an empty string, or any other value is silently treated as console output with no error, though the help says console | json. Verified: av proxy --log-format JSON emits console-formatted lines. The parsing lives in the shared BuildAgentProxyLogWriter/root log-format handling (outside this diff), but the flag is defined here; validate the value or document that it is case-sensitive.

scott-ray-wilson · 2026-09-08T21:34:48Z

+	if proxyAddr == "" {
+		util.HandleError(fmt.Errorf("the proxy address is required; pass --proxy <host:port> or set INFISICAL_AGENT_VAULT_PROXY_ADDRESS. The same proxy has a different address from every network, so there is no name to look it up by"))
+	}
+	proxyAddr = strings.TrimPrefix(strings.TrimPrefix(proxyAddr, "http://"), "https://")


[low] --proxy scheme trimming is lowercase-only

The strings.TrimPrefix(..., "http://")/"https://" here only strips lowercase schemes, so --proxy HTTPS://127.0.0.1:17323 is not trimmed and mis-parses into http://HTTPS://..., dying with lookup HTTPS: no such host. Lowercase http:///https:// trim fine. Trim case-insensitively (or parse with url.Parse).

scott-ray-wilson · 2026-09-08T21:34:48Z

+	if err != nil {
+		util.HandleError(err, "Unable to read --proxy")
+	}
+	if proxyAddr == "" {


[low] --proxy flag value is not trimmed, unlike the env var

A whitespace-only --proxy ' ' is non-empty so it bypasses the friendly required-address check and dies with a raw url.Parse error, whereas INFISICAL_AGENT_VAULT_PROXY_ADDRESS is trimmed and yields the clean "proxy address is required" message. Trim the flag value for parity.

scott-ray-wilson · 2026-09-08T21:34:48Z

+	if len(accessBundles) > 0 && sessionToken != "" {
+		util.HandleError(fmt.Errorf("--access-bundle and --session-token are two ways to get one session; pass one of them, not both"))
+	}
+	if len(accessBundles) > 1 {


[low] Inconsistent handling of empty --access-bundle values

A single --access-bundle "" is silently swallowed and reported as "a session is required", but --access-bundle "" --access-bundle real is rejected as "a session carries one access bundle; pass --access-bundle once", which misleadingly implies two bundles were supplied. Reject an empty bundle name explicitly.

scott-ray-wilson · 2026-09-08T21:34:48Z

+		detail.exactHost = true
+	}
+
+	if p.port != port {


[low] Non-canonical ports (leading zeros) validate but never match

Ports are compared as literal strings here, and the backend validator (agent-vault-host-pattern.ts isValidPort) accepts 018080 because it range-checks via Number(), so a connection written as host:018080 is stored and then never matches a request on port 18080 under deny → the host is silently blocked and the connection is dead. Both sides compare literally, so the real fix is to reject or canonicalize non-canonical ports at write time (backend), but noting it here since the CLI is where the silent non-match happens.

scott-ray-wilson

Two more from reviewing an external report against the source. One medium (mid-stream truncation masked as 200), one low (bounded shutdown stall at the connection limit). The report's other three items (CA-file clobber, mint-before-CA-write, IPv6 Host brackets) are already covered by earlier inline comments.

scott-ray-wilson · 2026-09-08T21:39:05Z

+		}
+	}
+	w.WriteHeader(resp.StatusCode)
+	_, _ = io.Copy(flushingWriter{ResponseWriter: w, rc: http.NewResponseController(w)}, resp.Body)


[medium] Truncated streams are masked as a successful 200

w.WriteHeader(resp.StatusCode) commits the 200 and headers before the body copy, then this io.Copy discards its error. If the upstream fails mid-body on a chunked response (reset, deadline, dropped connection), the handler still returns normally, so Go emits the terminating zero-length chunk and the client receives a well-formed but truncated 200 with no error signalled. The status line is already on the wire, so the fix is to abort rather than return cleanly: panic(http.ErrAbortHandler) on the copy error drops the connection so the client detects the failure. Distinct from the read-deadline truncation around proxy.go:234 (that one is the 60s deadline; this is the ignored copy error masking any mid-stream failure).

scott-ray-wilson · 2026-09-08T21:39:05Z

+	case l.sem <- struct{}{}:
+	default:
+		l.fullOnce.Do(l.onFull)
+		l.sem <- struct{}{}


[low] Saturated limiter stalls graceful shutdown and leaks the accept goroutine

Accept sends into l.sem before calling the underlying Accept, so once all maxConcurrentConns (512) slots are occupied the serve goroutine parks on the channel send. Closing the listener unblocks a goroutine parked in Listener.Accept, but it cannot cancel a channel send, which is the classic netutil.LimitListener footgun. Impact is bounded, not indefinite: front.Shutdown(ctx) runs under a 10s context (run.go:188), so shutdown returns after that deadline, but a saturated proxy stalls the full 10s on shutdown, accepts no new connections during the drain, and leaks the parked accept goroutine until the process exits. Fix: make acquisition cancellable on close, e.g. a semaphore channel that Close closes (or select on a done channel while acquiring).

scott-ray-wilson

One more from the end-to-end matrix (reproduced with gh and docker through a live proxy).

scott-ray-wilson · 2026-09-09T01:16:51Z

+
+		// Go binaries such as gh and docker ignore the CA environment variables and read the system trust store,
+		// so macOS gets the keychain entry too.
+		if runtime.GOOS == "darwin" && isatty.IsTerminal(os.Stdin.Fd()) {


[medium] Go-based agents fail TLS through the proxy in every unattended run and on Linux. gh, docker and other Go binaries read the system trust store, not SSL_CERT_FILE, and the only system-trust install is this macOS keychain step, gated on runtime.GOOS == "darwin" and a TTY. In a non-TTY run (CI, a service, av run from a script) or on any Linux host they get x509: certificate signed by unknown authority, while agent_vault_run_trust_other.go:10 states that the CA environment variables are enough. proxies.mdx:53 lists gh and docker as supported. Either document the limitation and print a hint when the child is a known Go tool, or offer a --trust-system step for Linux (update-ca-certificates / SSL_CERT_DIR) and a non-interactive keychain path on macOS.

The server now takes bundle names on POST /agent-vault/sessions, so the run command posts the name the user typed instead of listing every bundle and matching it here first. That drops a request from every mint and leaves one place deciding what matching a name means. A name is checked against the slug grammar before the request goes out. The server would reject a bad one too, but a schema rejection arrives as a 422 whose whole body we print, and a typo is the most common way to get this wrong.

veria-ai · 2026-09-09T15:50:39Z

+	// Private and link-local addresses are reachable, deliberately: an internal API inside the operator's
+	// own network is a first-class destination.
+
+	leaf, err := ps.ca.mintLeaf(hostname)


Low: Oversized CONNECT targets can exhaust proxy memory

A client with any valid session token can send near-limit, unique CONNECT hostnames. Each hostname is copied into a newly generated certificate and retained in the 8,192-entry leaf cache, allowing one session holder to consume substantial memory and disrupt the proxy before the connection is matched or forwarded. Validate DNS names before minting, including the 253-byte total and 63-byte label limits, while continuing to permit valid IP literals.

saifsmailbox98 added 19 commits September 2, 2026 07:42

feat(agent-vault): infisical av run

2ffb2c7

veria-ai Bot reviewed Sep 4, 2026

View reviewed changes

Comment thread packages/cmd/agent_vault_run.go

Comment thread packages/cmd/agent_vault_run.go

greptile-apps Bot reviewed Sep 4, 2026

View reviewed changes

Comment thread packages/agentvault/run.go

Comment thread packages/agentvault/proxy.go Outdated

Comment thread packages/cmd/agent_vault_run.go Outdated

Comment thread packages/cmd/agent_vault_run.go

Comment thread packages/agentvault/store.go

saifsmailbox98 mentioned this pull request Sep 4, 2026

feat(agent-vault): agents that run without credentials Infisical/infisical#8000

Open

11 tasks

veria-ai Bot reviewed Sep 5, 2026

View reviewed changes

Comment thread packages/agentvault/proxy.go

fix(agent-vault): name the real cause when a session is dropped

f0a8e8b

veria-ai Bot reviewed Sep 5, 2026

View reviewed changes

Comment thread packages/cmd/agent_vault_run.go

saifsmailbox98 added 2 commits September 5, 2026 10:24

chore(agent-vault): name handleRefreshFailure for what it does

4c21a7d

veria-ai Bot reviewed Sep 5, 2026

View reviewed changes

Comment thread packages/agentvault/cache.go

saifsmailbox98 added 4 commits September 6, 2026 02:34

test(agent-vault): sync the host-pattern fixture with the backend

c28910b

The backend now compares an IPv4-mapped IPv6 pattern as the IPv4 host it names, which is what net.ParseIP already did here. The fixture records both directions.

veria-ai Bot reviewed Sep 5, 2026

View reviewed changes

Comment thread packages/agentvault/proxy.go

saifsmailbox98 added 2 commits September 7, 2026 07:40

fix(av): rename av run --token to --session-token

fe67adb

The root command reads --token as an Infisical token and warned that the logged-in session was being overwritten on every dashboard-minted run. The session token is a different thing and now has a name the root never reads.

saifsmailbox98 added 3 commits September 7, 2026 17:09

feat(av): one access bundle per session

4634d3c

--access-bundle is accepted once. The flag stays a StringArray so a repeat is an error rather than a silent last-wins, and the backticked usage word makes help print it as a single name. Matches the backend's V1 cap.

chore(agent-vault): cut the comments that only say what the code says

ae1e833

test(agent-vault): keep the host grammar cases in each repo's own tests

63113ec

saifsmailbox98 requested a review from scott-ray-wilson September 7, 2026 14:03

scott-ray-wilson reviewed Sep 8, 2026

View reviewed changes

scott-ray-wilson reviewed Sep 9, 2026

View reviewed changes

veria-ai Bot reviewed Sep 9, 2026

View reviewed changes


		matched := bestMatch(connections, hostname, port)

		if matched == nil && ps.currentConfig().UnmatchedHost == UnmatchedDeny && !ps.isBypassed(hostname, port) {

Uh oh!

Conversation

saifsmailbox98 commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Steps to verify the change

Type

Checklist

Uh oh!

infisical-review-police Bot commented Sep 4, 2026

Uh oh!

Uh oh!

Uh oh!

veria-ai Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR overview

Open issues (1)

Uh oh!

greptile-apps Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

saifsmailbox98 commented Sep 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gitguardian Bot commented Sep 7, 2026

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Uh oh!

scott-ray-wilson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linear Bot commented Sep 8, 2026

Uh oh!

scott-ray-wilson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saifsmailbox98 commented Sep 4, 2026 •

edited

Loading

veria-ai Bot commented Sep 4, 2026 •

edited

Loading

greptile-apps Bot commented Sep 4, 2026 •

edited

Loading