fix(infisical): support CLI session auth as fallback#375
fix(infisical): support CLI session auth as fallback#375
Conversation
Greptile SummaryThis PR adds CLI session auth as a third fallback for the Infisical provider: when neither Two minor P2 notes: the Confidence Score: 5/5Safe to merge; the change is minimal, well-scoped, and the prior P0/P1 concerns from earlier rounds are fully resolved. All remaining findings are P2: one verification suggestion about an undocumented CLI flag and one test coverage gap. Neither blocks merge nor introduces correctness issues.
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant fnox
participant InfisicalProvider
participant InfisicalCLI
User->>fnox: fnox get SECRET
fnox->>InfisicalProvider: get_secret("SECRET")
InfisicalProvider->>InfisicalProvider: execute_infisical_command(args)
InfisicalProvider->>InfisicalProvider: get_auth_token()
alt INFISICAL_TOKEN set
InfisicalProvider-->>InfisicalProvider: Ok(Some(token))
InfisicalProvider->>InfisicalCLI: infisical secrets get ... --token token
else CLIENT_ID + CLIENT_SECRET set
InfisicalProvider->>InfisicalCLI: infisical login --method universal-auth ...
InfisicalCLI-->>InfisicalProvider: cached token
InfisicalProvider->>InfisicalCLI: infisical secrets get ... --token cached_token
else No explicit credentials (new)
InfisicalProvider-->>InfisicalProvider: Ok(None)
InfisicalProvider->>InfisicalCLI: infisical secrets get ... (no --token)
Note over InfisicalCLI: CLI uses cached session from infisical login
end
InfisicalCLI-->>InfisicalProvider: JSON output
InfisicalProvider-->>fnox: secret value
fnox-->>User: secret value
Reviews (5): Last reviewed commit: "fix(infisical): support CLI session auth..." | Re-trigger Greptile |
There was a problem hiding this comment.
Code Review
This pull request updates the Infisical provider to support CLI session authentication when explicit credentials (like environment variables or client secrets) are not provided. It modifies get_auth_token to return an optional token and updates test_connection to perform a connectivity check using the secrets list command when falling back to the CLI session. A review comment suggests that test_connection should be refactored to always execute this probe command, ensuring that explicit credentials are also verified for validity and project accessibility rather than assuming success immediately.
6f15b4e to
6c63759
Compare
jdx
left a comment
There was a problem hiding this comment.
The code change looks good! One thing — the docs at docs/providers/infisical.md should be updated to reflect that CLI session auth now works as a fallback. Currently the docs present INFISICAL_TOKEN or universal auth as required steps.
Key spots:
- Quick Start (step 3): Says "Get a service token or universal auth token" as if mandatory — with this change,
infisical loginalone is sufficient - "Get Authentication Token" section: Could use an "Option C: CLI Session Auth" noting that no env vars are needed if you've run
infisical login - Usage section: Shows
export INFISICAL_TOKEN=...as a required first step - Token Management: Implies a token is always needed
This comment was generated by Claude Code.
Previously, the Infisical provider required explicit credentials (INFISICAL_TOKEN or INFISICAL_CLIENT_ID/SECRET). This change makes get_auth_token() return None when no credentials are found, allowing the CLI to use its own cached session from 'infisical login'. - Update get_auth_token() to return Option<String>, falling back to CLI session auth - Update docs to recommend CLI session auth as simplest option for local development - Update bats test to accept either auth_failed or cli_failed for missing credentials - Add rust to mise.toml tools
951fc97 to
c7f3496
Compare
|
Updated the docs per your review. Changes:
Also fixed the |
The Infisical provider currently requires explicit credentials (
INFISICAL_TOKENorINFISICAL_CLIENT_ID+INFISICAL_CLIENT_SECRET). If neither is set, it fails with "Authentication not found", even when the user has an active CLI session frominfisical login.This is inconsistent with how the Infisical CLI itself works. Running
infisical secrets get ...without--tokenuses the cached session just fine.The fix: when no explicit credentials are configured, skip passing
--tokenand let the CLI use its own session. The priority is:INFISICAL_TOKEN/FNOX_INFISICAL_TOKEN(explicit token, used as-is)INFISICAL_CLIENT_ID+INFISICAL_CLIENT_SECRET(universal auth login, token cached)--tokenpassed, CLI manages its own auth)test_connection()also needed updating since it previously just checked for a token. When using CLI session auth, it now runs asecrets listcommand as a connectivity check instead.All existing cargo tests pass. I've tested this against a self-hosted Infisical instance with
infisical loginsession auth.