Skip to content

fix: extend offline secret caching to token-based auth and fix connection check#272

Open
devin-ai-integration[bot] wants to merge 5 commits into
mainfrom
devin/1781886077-fix-offline-fallback
Open

fix: extend offline secret caching to token-based auth and fix connection check#272
devin-ai-integration[bot] wants to merge 5 commits into
mainfrom
devin/1781886077-fix-offline-fallback

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

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 codes

When a load balancer or reverse proxy returns HTTP 503 (server down), the Go http.Get succeeds without error — so the CLI treated the server as "connected" and never fell back to cached secrets. Now checks for a 2xx response:

// Before: 503 from a proxy → err == nil → isConnected = true → no fallback
_, err := http.Get(...)
return err == nil

// After: 503 → resp.StatusCode == 503 → isConnected = false → fallback triggers
resp, err := http.Get(...)
defer resp.Body.Close()
return resp.StatusCode >= 200 && resp.StatusCode < 300

2. Token-based auth (service tokens, Universal Auth / machine identity) had zero offline caching

GetAllEnvironmentVariables() only wrote/read secret backups in the infisical login user-auth code path. The else branch for InfisicalToken and UniversalAuthAccessToken had 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/GetBackupEncryptionKey helpers. The fallback only triggers on connection errors or server errors (5xx) — client errors like 401/403 (revoked token, permission denied) propagate correctly so users see actionable auth failure messages.

Type ✨

  • Bug fix
  • New feature
  • Improvement
  • Breaking change
  • Documentation

Tests 🛠️

Scenario to verify:

  1. Authenticate with a machine identity (Universal Auth) and run infisical run --env=prod --path=/test --projectId=<id> -- env — secrets should be fetched and injected as before
  2. Verify ~/.infisical/secrets-backup/ contains an encrypted backup file
  3. Make the Infisical server unreachable (stop it, or have a proxy return 503)
  4. Re-run the same command — should print a warning and serve cached secrets instead of failing
  5. Revoke/invalidate the token and re-run — should show an auth error (NOT fall back to cache)

Link to Devin session: https://app.devin.ai/sessions/2dfab5dd573b484f84cce72d5f38c757

…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 <[email protected]>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@greptile-apps

greptile-apps Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes two gaps in the CLI's offline secret caching: ValidateInfisicalAPIConnection now checks for a 2xx HTTP status (catching load balancer 503s), and token-based auth (InfisicalToken / UniversalAuthAccessToken) gains the same write-on-success / read-on-failure backup caching that previously only existed for the interactive login path.

  • common.go: http.Get result is now checked for StatusCode >= 200 && StatusCode < 300, with defer resp.Body.Close() to prevent connection leaks.
  • secrets.go: GetPlainTextSecretsViaServiceToken signature extended to return the workspace ID so the caller can derive a cache key without requiring --projectId; GetAllEnvironmentVariables token-branch now writes the encrypted backup on success and reads it on connection/5xx failures, with an errors.As guard that prevents 4xx errors from being silently swallowed by the fallback. Error wrapping for the service token path was also changed from %v to %w so the *api.APIError chain survives for the 4xx guard.

Confidence Score: 5/5

Safe to merge — both changes are well-scoped, the caching logic mirrors the existing user-auth pattern, and all previously identified issues (body leak, 4xx masking, error chain stripping) have been addressed in this revision.

The connection-check fix is a two-line correction that closes a well-understood gap. The caching extension is more complex but follows the established pattern exactly, the errors.As guard correctly prevents auth errors from being swallowed, and %w wrapping was applied consistently so the error chain reaches the guard for all service-token failure modes. No new logic paths were introduced that aren't covered by the existing helpers.

No files require special attention.

Important Files Changed

Filename Overview
packages/util/common.go ValidateInfisicalAPIConnection now checks the HTTP status code (2xx) instead of just the absence of a transport error, and properly closes the response body with defer.
packages/util/secrets.go GetPlainTextSecretsViaServiceToken updated to return workspaceId so the caller can cache secrets; GetAllEnvironmentVariables now writes/reads the backup cache for token-based auth with a proper 4xx guard to prevent masking auth errors.

Reviews (4): Last reviewed commit: "fix: expose workspace ID from service to..." | Re-trigger Greptile

Comment thread packages/util/common.go
Comment thread packages/util/secrets.go Outdated
Comment thread packages/util/common.go
…to non-auth errors

Address review feedback:
- Close resp.Body in ValidateInfisicalAPIConnection to prevent connection leak
- Only fall back to cached secrets on connection/server errors (5xx), not on
  client errors (4xx) like 401/403 which indicate auth issues (revoked token,
  permission denied). This prevents silently serving stale secrets when the
  real problem is invalid credentials.

Co-Authored-By: jake <[email protected]>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Addressed all Greptile feedback in d7e51f3:

  1. resp.Body leak — Fixed with defer resp.Body.Close()
  2. Fallback on all errors — Fixed. Now only falls back to cache on connection errors (GenericRequestError) or server errors (5xx). Client errors (4xx like 401/403) propagate correctly so users see actionable auth failure messages.
  3. Service token + WorkspaceId — By design. WorkspaceId is set by the caller via --projectId flag or INFISICAL_PROJECT_ID env var (see run.go:80). The community user's command includes --projectId=xxxxx. For Universal Auth, WorkspaceId is already enforced (line 367). For service tokens without --projectId, caching gracefully degrades to a no-op.
  4. SSRF — Pre-existing pattern, out of scope for this bugfix.

@jakehulberg

Copy link
Copy Markdown

@greptile re-review please

Comment thread packages/util/secrets.go
…r chain

Change fmt.Errorf wrapping from %v to %w in GetPlainTextSecretsViaServiceToken
so errors.As can unwrap the underlying *api.APIError. Without this, a revoked
service token returning 401 would bypass the 4xx guard and silently serve
stale cached secrets.

Co-Authored-By: jake <[email protected]>
@jakehulberg

Copy link
Copy Markdown

@greptile re-review pls

…thout --projectId

GetPlainTextSecretsViaServiceToken now returns the workspace ID it discovers
from the token details, so the caller can use it for caching even when
--projectId is not explicitly passed. Previously, service token users without
--projectId would silently get no offline caching because params.WorkspaceId
was empty.

Co-Authored-By: jake <[email protected]>
@jakehulberg

Copy link
Copy Markdown

@greptile re review pls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant