You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Aims to fix issues (Infisical/infisical#1001) that several users pointed out regarding high latency CLI calls, especially for geographically distributed people using the CLI.
Removes a couple blocking Github calls to check for updates
Removes an infisical auth validation check from running every command
Attempts to validate JWT locally rather than using the API. In the case where the JWT is invalid the call will fail regardless.
This PR reduces CLI latency by (1) replacing the blocking per-command GitHub update check with an async background goroutine writing to a 24-hour disk cache, and (2) replacing the blocking server-side CallIsAuthenticated API round-trip with a local JWT expiry check using ParseUnverified. Both changes meaningfully reduce network calls on the hot path, which directly addresses the reported latency issues for geographically distributed users.
Key concerns found during review:
os.Exit bypasses the update-check defer (packages/cmd/root.go): Go's os.Exit does not run deferred functions. On the error path in Execute(), WaitForUpdateCheck() is never called, the background goroutine is killed mid-flight, and the cache write is abandoned. The WaitGroup pattern only works on the success path. A common fix is to wrap the logic in an inner function so that defer completes before os.Exit is called from the outer function.
ParseUnverified skips JWT signature validation (packages/util/credentials.go): The change from api.CallIsAuthenticated (server-validated) to IsJWTExpired (local exp claim only, no signature check) is a security model change. A crafted token with any valid-looking future exp claim will pass the local check regardless of its signature. The PR's reasoning ("the API call will fail anyway") is valid, but it widens the window in which partial command logic could execute before the first rejected API call.
Urgent updates spawn a GitHub request on every CLI invocation (packages/util/check-for-update.go): isCacheFresh returns false unconditionally for urgent updates, so every CLI run with a pending urgent update triggers a fresh background goroutine and GitHub API call. A short TTL (e.g. 5 minutes) for urgent entries would reduce churn while preserving prompt notification behaviour.
Confidence Score: 2/5
This PR should not be merged as-is due to the os.Exit / deferred WaitGroup bug and the security model change introduced by ParseUnverified.
Two concrete correctness/security issues reduce confidence: (1) the deferred WaitForUpdateCheck is silently skipped on all error paths due to Go's os.Exit behaviour, making the cache-persistence guarantee unreliable; (2) replacing a server-validated auth check with an unverified local JWT parse changes the security model in a meaningful way. The overall goal and the new caching architecture are well-designed, and test coverage is strong — once these issues are addressed the PR would be in good shape.
packages/cmd/root.go (deferred WaitGroup bypassed by os.Exit) and packages/util/credentials.go (ParseUnverified security model change) need the most attention before merging.
Important Files Changed
Filename
Overview
packages/cmd/root.go
Adds defer util.WaitForUpdateCheck() to block until the background update goroutine finishes — but os.Exit(1) on the error path bypasses all deferred functions in Go, so the cache write is abandoned on failures.
packages/util/credentials.go
Replaces the server-side CallIsAuthenticated API call with a local IsJWTExpired check using ParseUnverified, which skips signature validation entirely — a notable security model change that allows a crafted JWT with a valid exp claim to pass local auth checks.
packages/util/check-for-update.go
Refactors update checking to use a 24-hour disk cache with async background refresh via a goroutine and WaitGroup; atomic write with temp file + rename is well implemented, but urgent updates bypass the cache TTL entirely, causing a background GitHub request on every CLI invocation until the user upgrades.
packages/util/check-for-update_test.go
New test file with good coverage of isCacheFresh, displayCachedUpdateNotice, and atomic write/read behaviour; edge cases like urgent updates, 48h grace period, stale caches, and corrupt JSON are all covered.
packages/util/credentials_test.go
New test file with thorough coverage of IsJWTExpired including the 30-second buffer, missing exp claim, malformed JWTs, empty strings, and invalid base64 payloads.
packages/util/constants.go
Adds UPDATE_CHECK_CACHE_FILE_NAME constant for the update cache file; straightforward and correct.
go.mod
Adds github.com/golang-jwt/jwt/v5 v5.3.1 dependency for local JWT expiry checking.
In Go, os.Exit terminates the process immediately and does not run deferred functions. On any error path where RootCmd.Execute() returns an error, WaitForUpdateCheck() will never execute, the background goroutine will be killed mid-flight, and the cache write will be abandoned.
This defeats the purpose of the WaitGroup pattern: the cache is only guaranteed to persist on the success path.
A common Go pattern to address this is to use a helper that calls os.Exit after the deferred functions have run:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description 📣
Aims to fix issues (Infisical/infisical#1001) that several users pointed out regarding high latency CLI calls, especially for geographically distributed people using the CLI.
Type ✨
Tests 🛠️
Two new unit tests were added.