fix: extend offline secret caching to token-based auth and fix connection check#272
fix: extend offline secret caching to token-based auth and fix connection check#272devin-ai-integration[bot] wants to merge 1 commit into
Conversation
…tion check - ValidateInfisicalAPIConnection now checks HTTP status code, not just transport errors. A 503 from a load balancer/proxy no longer counts as 'connected', allowing the cache fallback to trigger correctly. - Added secret backup/restore for service token and Universal Auth (machine identity) code paths. Previously, offline caching only worked for infisical-login user auth. Now all auth methods write secrets to the encrypted backup on success and fall back to cached secrets on fetch failure. Co-Authored-By: jake <jake@infisical.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
| Filename | Overview |
|---|---|
| packages/util/common.go | Adds HTTP status code check to ValidateInfisicalAPIConnection, but leaks the response body — resp.Body is never closed after a successful GET. |
| packages/util/secrets.go | Adds write-on-success / read-on-failure caching to the token-based auth branch, but the fallback fires on all errors (not just connection failures), and the service-token path never populates WorkspaceId so caching is silently skipped for it. |
Comments Outside Diff (1)
-
packages/util/secrets.go, line 362-364 (link)Caching silently skipped for service tokens
GetPlainTextSecretsViaServiceTokenis called forInfisicalTokenwithout ever settingparams.WorkspaceId. The caching block below is guarded byparams.WorkspaceId != "", so for service token auth both write-on-success and read-on-failure are silently no-ops. The offline fallback feature advertised by this PR has no effect forInfisicalTokenusers unlessWorkspaceIdis separately provided via--projectId.
Reviews (1): Last reviewed commit: "fix: extend offline secret caching to to..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| resp, err := http.Get(fmt.Sprintf("%v/status", config.INFISICAL_URL)) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| return resp.StatusCode >= 200 && resp.StatusCode < 300 |
There was a problem hiding this comment.
http.Get succeeds, leaking the underlying TCP connection. Per Go's net/http docs, callers must always close resp.Body, even if they don't read from it — otherwise the HTTP client cannot reuse the connection and the goroutine/socket is leaked. Since ValidateInfisicalAPIConnection is called on every CLI invocation that touches secrets, repeated calls will accumulate leaks.
| resp, err := http.Get(fmt.Sprintf("%v/status", config.INFISICAL_URL)) | |
| if err != nil { | |
| return false | |
| } | |
| return resp.StatusCode >= 200 && resp.StatusCode < 300 | |
| resp, err := http.Get(fmt.Sprintf("%v/status", config.INFISICAL_URL)) | |
| if err != nil { | |
| return false | |
| } | |
| defer resp.Body.Close() | |
| return resp.StatusCode >= 200 && resp.StatusCode < 300 |
| } else if errorToReturn != nil && params.WorkspaceId != "" { | ||
| backupEncryptionKey, _ := GetBackupEncryptionKey() | ||
| if backupEncryptionKey != nil { | ||
| backedUpSecrets, err := ReadBackupSecrets(params.WorkspaceId, params.Environment, params.SecretsPath, backupEncryptionKey) | ||
| if len(backedUpSecrets) > 0 { | ||
| PrintWarning("Unable to fetch the latest secret(s) due to connection error, serving secrets from last successful fetch. For more info, run with --debug") | ||
| secretsToReturn = backedUpSecrets | ||
| errorToReturn = err | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The user-auth path (line 349) only reads from cache when !isConnected, so auth errors (401/403) propagate correctly. The new token-based path falls back on any errorToReturn != nil, meaning a revoked token or permission error will silently serve stale cached secrets instead of surfacing the error to the caller. A user whose token is rotated would see old secrets with only a vague "connection error" warning, with no indication that their token is actually invalid.
| func ValidateInfisicalAPIConnection() (ok bool) { | ||
| _, err := http.Get(fmt.Sprintf("%v/status", config.INFISICAL_URL)) | ||
| return err == nil | ||
| resp, err := http.Get(fmt.Sprintf("%v/status", config.INFISICAL_URL)) |
There was a problem hiding this comment.
INFISICAL_URL
config.INFISICAL_URL is user-supplied and passed directly to http.Get. An attacker who can influence this value (e.g., via a misconfigured environment variable) could use it to probe internal network endpoints — making the CLI an SSRF oracle. Consider validating that the URL is a known-safe external host, or at minimum ensuring the scheme is https and the host is not a private/loopback address. This vector existed before this PR but the change makes it more prominent as the health-check path is now load-bearing for the offline fallback decision.
Context Used: Flag SSRF risks (source)
Description 📣
Fixes two issues that prevent the CLI's offline secret caching from working in common scenarios:
1.
ValidateInfisicalAPIConnection()only checked for transport errors, not HTTP status codesWhen a load balancer or reverse proxy returns HTTP 503 (server down), the Go
http.Getsucceeds without error — so the CLI treated the server as "connected" and never fell back to cached secrets. Now checks for a 2xx response:2. Token-based auth (service tokens, Universal Auth / machine identity) had zero offline caching
GetAllEnvironmentVariables()only wrote/read secret backups in theinfisical loginuser-auth code path. Theelsebranch forInfisicalTokenandUniversalAuthAccessTokenhad no caching logic at all — any fetch failure returned an error immediately with no fallback.Added the same backup write-on-success / read-on-failure pattern to the token-based auth branch, using the existing
WriteBackupSecrets/ReadBackupSecrets/GetBackupEncryptionKeyhelpers.Type ✨
Tests 🛠️
Scenario to verify:
infisical run --env=prod --path=/test --projectId=<id> -- env— secrets should be fetched and injected as before~/.infisical/secrets-backup/contains an encrypted backup fileLink to Devin session: https://app.devin.ai/sessions/2dfab5dd573b484f84cce72d5f38c757