feat: add jsonfile credential provider for JSON file credential injection#366
feat: add jsonfile credential provider for JSON file credential injection#366ychost wants to merge 1 commit intolarksuite:mainfrom
Conversation
…tion Add a new credential extension provider that reads app credentials from a JSON file specified via LARKSUITE_CLI_CREDENTIAL_FILE environment variable. This enables non-interactive credential injection by pointing to a JSON file containing app_id, app_secret, brand, access tokens, and/or refresh_token. Key features: - Reads credentials from a JSON file (absolute path required) - Supports app_id, app_secret, brand, default_as fields - Supports user_access_token and tenant_access_token for direct token injection - Supports refresh_token for automatic UAT refresh via OAuth endpoint - In-process token caching with 30s pre-expiry refresh - Path validation: rejects relative paths and control characters - Comprehensive error handling with BlockError for structured feedback Provider priority: env vars > jsonfile > default config (config init) Files changed: - extension/credential/jsonfile/jsonfile.go (new provider) - extension/credential/jsonfile/jsonfile_test.go (27 tests) - internal/envvars/envvars.go (new LARKSUITE_CLI_CREDENTIAL_FILE const) - main.go (blank import to activate provider) - README.md, README.zh.md (credential file documentation)
|
ychost.yc seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
github-actions
bot
added
the
size/M
📝 WalkthroughWalkthroughA new JSON credential file provider is introduced, enabling credentials to be loaded from an external file via the Changes
Sequence DiagramsequenceDiagram
actor CLI as CLI Request
participant JF as JSON File<br/>Provider
participant Cache as Refresh<br/>Cache
participant OAuth as OAuth Token<br/>Endpoint
CLI->>JF: ResolveToken(UAT or TAT)
alt Direct Token Available
JF->>JF: Return token from file
JF-->>CLI: Token
else Refresh Token Available
JF->>Cache: Check cached token
alt Cached & Valid
Cache-->>JF: Return cached token
JF-->>CLI: Token
else Cache Miss or Expired
JF->>OAuth: POST refresh_token grant
OAuth-->>JF: {access_token, expires_in}
JF->>Cache: Update cache + timestamp
JF-->>CLI: Token
end
else Token Missing
JF-->>CLI: nil
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested Labels
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds a
Confidence Score: 3/5Not safe to merge as-is: three P1 bugs in the refresh token path can cause indefinite hangs, stale credentials after rotation, and unresponsive behaviour on context cancellation. All three P1 findings are in the refreshUAT code path: context not propagated (breaking the Provider contract), cache not invalidated on credential rotation (silent stale data), and no HTTP timeout (indefinite hang in CI/CD). These are in a specific code path (refresh token flow), but since it is a primary feature of the new provider, they warrant a score of 3. extension/credential/jsonfile/jsonfile.go — specifically the refreshUAT function and the refreshCache global variable.
|
| Filename | Overview |
|---|---|
| extension/credential/jsonfile/jsonfile.go | New JSON file credential provider: 3 P1 issues — context dropped before HTTP call, refresh cache not keyed on the token value (stale cache after rotation), and http.DefaultClient used without a timeout. |
| extension/credential/jsonfile/jsonfile_test.go | 27 well-structured tests covering happy paths, validation errors, cache hits, and refresh token flows via an httptest server; no issues found. |
| internal/envvars/envvars.go | Adds LARKSUITE_CLI_CREDENTIAL_FILE constant; straightforward and correct. |
| main.go | Adds blank import to activate the jsonfile provider on startup; correct. |
Sequence Diagram
sequenceDiagram
participant CLI as lark-cli
participant Reg as credential.Registry
participant JP as jsonfile.Provider
participant FS as File System
participant OAuth as OAuth Token Endpoint
CLI->>Reg: ResolveAccount(ctx)
Reg->>JP: ResolveAccount(ctx)
JP->>FS: os.Getenv(LARKSUITE_CLI_CREDENTIAL_FILE)
JP->>FS: os.ReadFile(path)
FS-->>JP: credFile JSON
JP-->>Reg: *credential.Account
CLI->>Reg: ResolveToken(ctx, UAT)
Reg->>JP: ResolveToken(ctx, UAT)
JP->>FS: os.ReadFile(path)
FS-->>JP: credFile JSON
alt user_access_token set
JP-->>Reg: Token{Value: uat}
else refresh_token set
JP->>JP: refreshUAT(cf) [no ctx!]
alt cache hit
JP-->>Reg: Token{cached}
else cache miss
JP->>OAuth: POST /oauth/token (refresh_token grant)
OAuth-->>JP: access_token + expires_in
JP-->>Reg: Token{refreshed}
end
end
Reviews (1): Last reviewed commit: "feat: add jsonfile credential provider f..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func refreshUAT(cf *credFile) (string, error) { | ||
| refreshMu.Lock() | ||
| defer refreshMu.Unlock() | ||
|
|
||
| if refreshCache != nil && time.Now().Before(refreshCache.expiresAt.Add(-refreshAheadDuration)) { | ||
| return refreshCache.value, nil | ||
| } | ||
|
|
||
| tokenURL := tokenEndpointFunc(cf.Brand) | ||
|
|
||
| form := url.Values{} | ||
| form.Set("grant_type", "refresh_token") | ||
| form.Set("refresh_token", cf.RefreshToken) | ||
| form.Set("client_id", cf.AppID) | ||
| form.Set("client_secret", cf.AppSecret) | ||
|
|
||
| req, err := http.NewRequest("POST", tokenURL, strings.NewReader(form.Encode())) | ||
| if err != nil { | ||
| return "", &credential.BlockError{Provider: "jsonfile", Reason: fmt.Sprintf("failed to create refresh request: %v", err)} | ||
| } | ||
| req.Header.Set("Content-Type", "application/x-www-form-urlencoded") | ||
|
|
||
| resp, err := http.DefaultClient.Do(req) |
There was a problem hiding this comment.
The ctx received by ResolveToken is dropped before reaching refreshUAT. The HTTP request is created with http.NewRequest rather than http.NewRequestWithContext, so caller-initiated cancellations (Ctrl-C, deadline) are silently ignored and the network call can run indefinitely.
| func refreshUAT(cf *credFile) (string, error) { | |
| refreshMu.Lock() | |
| defer refreshMu.Unlock() | |
| if refreshCache != nil && time.Now().Before(refreshCache.expiresAt.Add(-refreshAheadDuration)) { | |
| return refreshCache.value, nil | |
| } | |
| tokenURL := tokenEndpointFunc(cf.Brand) | |
| form := url.Values{} | |
| form.Set("grant_type", "refresh_token") | |
| form.Set("refresh_token", cf.RefreshToken) | |
| form.Set("client_id", cf.AppID) | |
| form.Set("client_secret", cf.AppSecret) | |
| req, err := http.NewRequest("POST", tokenURL, strings.NewReader(form.Encode())) | |
| if err != nil { | |
| return "", &credential.BlockError{Provider: "jsonfile", Reason: fmt.Sprintf("failed to create refresh request: %v", err)} | |
| } | |
| req.Header.Set("Content-Type", "application/x-www-form-urlencoded") | |
| resp, err := http.DefaultClient.Do(req) | |
| func refreshUAT(ctx context.Context, cf *credFile) (string, error) { | |
| refreshMu.Lock() | |
| defer refreshMu.Unlock() | |
| if refreshCache != nil && time.Now().Before(refreshCache.expiresAt.Add(-refreshAheadDuration)) { | |
| return refreshCache.value, nil | |
| } | |
| tokenURL := tokenEndpointFunc(cf.Brand) | |
| form := url.Values{} | |
| form.Set("grant_type", "refresh_token") | |
| form.Set("refresh_token", cf.RefreshToken) | |
| form.Set("client_id", cf.AppID) | |
| form.Set("client_secret", cf.AppSecret) | |
| req, err := http.NewRequestWithContext(ctx, http.MethodPost, tokenURL, strings.NewReader(form.Encode())) |
You'll also need to update resolveUAT to accept and thread ctx through, and update the call-site in ResolveToken.
| } | ||
| req.Header.Set("Content-Type", "application/x-www-form-urlencoded") | ||
|
|
||
| resp, err := http.DefaultClient.Do(req) |
There was a problem hiding this comment.
http.DefaultClient has no timeout
http.DefaultClient has no Timeout set, so if the OAuth token endpoint is slow or unreachable the CLI will hang indefinitely. This is especially impactful for CI/CD pipelines where a stuck process blocks the pipeline.
| resp, err := http.DefaultClient.Do(req) | |
| resp, err := (&http.Client{Timeout: 10 * time.Second}).Do(req) |
| var ( | ||
| refreshMu sync.Mutex | ||
| refreshCache *cachedToken | ||
|
|
||
| tokenEndpointFunc = defaultTokenEndpoint | ||
| ) |
There was a problem hiding this comment.
refreshCache not keyed on refresh_token value
The in-process cache stores the last fetched access token but never records which refresh_token was used to obtain it. In CI/CD environments (the primary use case), credential rotation is common: if the file is updated with a new refresh_token, refreshUAT will keep returning the stale cached token for up to 2 hours rather than fetching a new one with the updated value.
The cachedToken struct should store the input token used to obtain it, and the cache-hit check should compare against cf.RefreshToken:
type cachedToken struct {
value string
expiresAt time.Time
refreshToken string // invalidate when input changes
}
// In refreshUAT, before returning cached value:
if refreshCache != nil &&
refreshCache.refreshToken == cf.RefreshToken &&
time.Now().Before(refreshCache.expiresAt.Add(-refreshAheadDuration)) {
return refreshCache.value, nil
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
README.md (1)
199-207: Missingdefault_asfield in JSON example.The JSON example omits the
default_asfield, but the implementation injsonfile.gosupports it (and tests verify it works). The fields table below mentions it should be documented to match the implementation's capabilities.Consider adding
"default_as": "user"to the example for completeness, or add a note that this optional field can be included.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 199 - 207, The README's JSON example is missing the optional "default_as" field that jsonfile.go supports; update the example JSON to include "default_as": "user" (or add a short note that this optional field is supported) so the docs match the implementation and tests referencing jsonfile.go's handling of default_as.extension/credential/jsonfile/jsonfile_test.go (2)
116-119: Consider checking error inTestResolveAccount_DefaultBrand.The error is ignored on line 116. If
ResolveAccountfails,acctwill be nil and the test will panic with a less informative message than an explicit error check would provide.Proposed fix
- acct, _ := (&Provider{}).ResolveAccount(context.Background()) + acct, err := (&Provider{}).ResolveAccount(context.Background()) + if err != nil { + t.Fatal(err) + } if acct.Brand != "feishu" {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/credential/jsonfile/jsonfile_test.go` around lines 116 - 119, The test TestResolveAccount_DefaultBrand ignores the error returned by (&Provider{}).ResolveAccount which can lead to a nil acct and a panic; modify the test to check the returned error from ResolveAccount (store both acct and err), fail the test with t.Fatalf or t.Fatalf-like message if err != nil, then continue to assert acct.Brand == "feishu" to ensure a clear failure message when ResolveAccount fails; reference the Provider type, ResolveAccount method, and acct variable in your change.
22-29: Consider checking error fromos.WriteFile.The error from
os.WriteFileis silently ignored. While test failures would likely surface as subsequent errors, explicitly checking would make debugging easier if file creation fails.Proposed fix
func writeTempJSON(t *testing.T, data map[string]string) string { t.Helper() dir := t.TempDir() path := filepath.Join(dir, "cred.json") b, _ := json.Marshal(data) - os.WriteFile(path, b, 0o600) + if err := os.WriteFile(path, b, 0o600); err != nil { + t.Fatalf("failed to write test credential file: %v", err) + } return path }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension/credential/jsonfile/jsonfile_test.go` around lines 22 - 29, In writeTempJSON(t *testing.T, data map[string]string) ensure you check and handle the error returned by os.WriteFile instead of ignoring it: capture the error (and optionally json.Marshal error) and call t.Fatalf or t.Fatalf with a clear message including the error and path so the test fails immediately if writing the temp file fails; reference the writeTempJSON function and the os.WriteFile call to locate where to add the error check.README.zh.md (1)
200-208: Missingdefault_asfield in JSON example (same as English README).For consistency with the implementation, consider adding the
default_asfield to both the JSON example and the fields table.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.zh.md` around lines 200 - 208, The JSON example shown (the snippet containing keys "app_id", "app_secret", "brand", "user_access_token", "tenant_access_token", "refresh_token") is missing the required "default_as" field; add "default_as" to that JSON example with an appropriate sample value and also update the corresponding fields table in the README.zh.md to include a row describing "default_as" (its purpose and expected values) so the Chinese README matches the English version and the implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extension/credential/jsonfile/jsonfile.go`:
- Around line 198-207: The code creates an HTTP request with http.NewRequest and
calls http.DefaultClient.Do(req) (around the token refresh logic using tokenURL
and form.Encode()) without a timeout, which can block indefinitely; change this
to use a context with timeout (e.g., context.WithTimeout) and attach it to the
request via req = req.WithContext(ctx) (or use a custom http.Client with
Timeout) and ensure you call cancel() after the request; keep the existing error
handling that returns &credential.BlockError but wrap/report context
deadline/cancel errors appropriately.
- Around line 259-262: Replace the os.ReadFile call with vfs.ReadFile in the
credential loading logic (the snippet that reads into data using variable
cleaned) and update imports to include "github.com/larksuite/cli/internal/vfs";
keep the existing error handling that returns &credential.BlockError{Provider:
"jsonfile", Reason: fmt.Sprintf(...)} but use the error returned by vfs.ReadFile
so behavior is unchanged except using the vfs layer for filesystem access.
---
Nitpick comments:
In `@extension/credential/jsonfile/jsonfile_test.go`:
- Around line 116-119: The test TestResolveAccount_DefaultBrand ignores the
error returned by (&Provider{}).ResolveAccount which can lead to a nil acct and
a panic; modify the test to check the returned error from ResolveAccount (store
both acct and err), fail the test with t.Fatalf or t.Fatalf-like message if err
!= nil, then continue to assert acct.Brand == "feishu" to ensure a clear failure
message when ResolveAccount fails; reference the Provider type, ResolveAccount
method, and acct variable in your change.
- Around line 22-29: In writeTempJSON(t *testing.T, data map[string]string)
ensure you check and handle the error returned by os.WriteFile instead of
ignoring it: capture the error (and optionally json.Marshal error) and call
t.Fatalf or t.Fatalf with a clear message including the error and path so the
test fails immediately if writing the temp file fails; reference the
writeTempJSON function and the os.WriteFile call to locate where to add the
error check.
In `@README.md`:
- Around line 199-207: The README's JSON example is missing the optional
"default_as" field that jsonfile.go supports; update the example JSON to include
"default_as": "user" (or add a short note that this optional field is supported)
so the docs match the implementation and tests referencing jsonfile.go's
handling of default_as.
In `@README.zh.md`:
- Around line 200-208: The JSON example shown (the snippet containing keys
"app_id", "app_secret", "brand", "user_access_token", "tenant_access_token",
"refresh_token") is missing the required "default_as" field; add "default_as" to
that JSON example with an appropriate sample value and also update the
corresponding fields table in the README.zh.md to include a row describing
"default_as" (its purpose and expected values) so the Chinese README matches the
English version and the implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b42a1ac1-62aa-4c3d-a5ba-265048d4c468
📒 Files selected for processing (6)
README.mdREADME.zh.mdextension/credential/jsonfile/jsonfile.goextension/credential/jsonfile/jsonfile_test.gointernal/envvars/envvars.gomain.go
| req, err := http.NewRequest("POST", tokenURL, strings.NewReader(form.Encode())) | ||
| if err != nil { | ||
| return "", &credential.BlockError{Provider: "jsonfile", Reason: fmt.Sprintf("failed to create refresh request: %v", err)} | ||
| } | ||
| req.Header.Set("Content-Type", "application/x-www-form-urlencoded") | ||
|
|
||
| resp, err := http.DefaultClient.Do(req) | ||
| if err != nil { | ||
| return "", &credential.BlockError{Provider: "jsonfile", Reason: fmt.Sprintf("refresh token request failed: %v", err)} | ||
| } |
There was a problem hiding this comment.
Add timeout to HTTP request to prevent indefinite blocking.
The HTTP request to the OAuth token endpoint uses http.DefaultClient without a timeout. If the server is slow or unresponsive, this could block indefinitely. Consider using a context with timeout or a custom http.Client.
Proposed fix using context timeout
-func refreshUAT(cf *credFile) (string, error) {
+func refreshUAT(ctx context.Context, cf *credFile) (string, error) {
refreshMu.Lock()
defer refreshMu.Unlock()
if refreshCache != nil && time.Now().Before(refreshCache.expiresAt.Add(-refreshAheadDuration)) {
return refreshCache.value, nil
}
tokenURL := tokenEndpointFunc(cf.Brand)
form := url.Values{}
form.Set("grant_type", "refresh_token")
form.Set("refresh_token", cf.RefreshToken)
form.Set("client_id", cf.AppID)
form.Set("client_secret", cf.AppSecret)
- req, err := http.NewRequest("POST", tokenURL, strings.NewReader(form.Encode()))
+ ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
+ defer cancel()
+ req, err := http.NewRequestWithContext(ctx, "POST", tokenURL, strings.NewReader(form.Encode()))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/credential/jsonfile/jsonfile.go` around lines 198 - 207, The code
creates an HTTP request with http.NewRequest and calls
http.DefaultClient.Do(req) (around the token refresh logic using tokenURL and
form.Encode()) without a timeout, which can block indefinitely; change this to
use a context with timeout (e.g., context.WithTimeout) and attach it to the
request via req = req.WithContext(ctx) (or use a custom http.Client with
Timeout) and ensure you call cancel() after the request; keep the existing error
handling that returns &credential.BlockError but wrap/report context
deadline/cancel errors appropriately.
| data, err := os.ReadFile(cleaned) | ||
| if err != nil { | ||
| return nil, &credential.BlockError{Provider: "jsonfile", Reason: fmt.Sprintf("cannot read credential file %s: %v", cleaned, err)} | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for vfs usage patterns in the codebase
rg -n "vfs\.(Read|Write|Open)" --type=go -C2
# Check if vfs package exists
fd -t f "vfs.go" --exec head -30 {}Repository: larksuite/cli
Length of output: 11049
🏁 Script executed:
# Examine the jsonfile.go file
cat -n extension/credential/jsonfile/jsonfile.go | head -270 | tail -50Repository: larksuite/cli
Length of output: 2174
🏁 Script executed:
# Check imports and overall structure
head -50 extension/credential/jsonfile/jsonfile.goRepository: larksuite/cli
Length of output: 1127
🏁 Script executed:
# Check for all os.* and vfs.* usage in the file
rg -n "os\.|vfs\." extension/credential/jsonfile/jsonfile.goRepository: larksuite/cli
Length of output: 202
🏁 Script executed:
# Check what validate.SafeInputPath looks like if present
rg -n "SafeInputPath|validate\." extension/credential/jsonfile/jsonfile.goRepository: larksuite/cli
Length of output: 39
🏁 Script executed:
# Check how validate.SafeInputPath is used in the codebase
rg -n "SafeInputPath" --type=go | head -20Repository: larksuite/cli
Length of output: 1912
🏁 Script executed:
# Check if vfs is imported anywhere and the pattern
rg -n "import.*vfs" --type=go | head -10Repository: larksuite/cli
Length of output: 224
🏁 Script executed:
# Look at examples of path validation with vfs in the codebase
rg -B3 "vfs.ReadFile" --type=go | head -40Repository: larksuite/cli
Length of output: 2076
🏁 Script executed:
# Check what validate module exports
head -50 internal/validate/path.goRepository: larksuite/cli
Length of output: 1078
🏁 Script executed:
# Confirm jsonfile.go is production code (not a test file)
file extension/credential/jsonfile/jsonfile.goRepository: larksuite/cli
Length of output: 101
🏁 Script executed:
# Check if there are any special exceptions for credential files
rg -n "credential" AGENTS.mdRepository: larksuite/cli
Length of output: 319
🏁 Script executed:
# Double-check the imports in jsonfile.go to see if vfs or validate are imported
grep -n "^import\|vfs\|validate" extension/credential/jsonfile/jsonfile.go | head -30Repository: larksuite/cli
Length of output: 67
🏁 Script executed:
# Look at how SafeEnvDirPath works since this is reading from an env-provided path
rg -A10 "SafeEnvDirPath" internal/vfs/localfileio/path.go | head -30Repository: larksuite/cli
Length of output: 619
🏁 Script executed:
# Check all filesystem-related os.* calls in jsonfile.go
rg -n "os\.(Read|Write|Open|Create|Mkdir|Remove|Stat)" extension/credential/jsonfile/jsonfile.goRepository: larksuite/cli
Length of output: 95
🏁 Script executed:
# Verify the pattern by looking at a complete example in another credential/auth file
fd -t f "auth.*\.go" cmd/ internal/ | head -5 | xargs grep -l "SafeEnvDirPath\|SafeInputPath" 2>/dev/nullRepository: larksuite/cli
Length of output: 86
🏁 Script executed:
# Look at a complete example from internal/core/config.go which also reads config
grep -B5 -A5 "vfs.ReadFile(GetConfigPath())" internal/core/config.goRepository: larksuite/cli
Length of output: 330
🏁 Script executed:
# Check what imports are already in jsonfile.go
sed -n '6,22p' extension/credential/jsonfile/jsonfile.goRepository: larksuite/cli
Length of output: 369
🏁 Script executed:
# Check if there are other credential providers that use similar patterns
fd -t f "credential.*\.go" extension/ --exec head -30 {} \; 2>/dev/null | grep -A5 "import"Repository: larksuite/cli
Length of output: 39
🏁 Script executed:
# Verify the exact path source - it comes from an env var
grep -n "CliCredentialFile" extension/credential/jsonfile/jsonfile.goRepository: larksuite/cli
Length of output: 364
🏁 Script executed:
# Check what SafeInputPath validation includes vs current manual validation
rg -A20 "func SafeInputPath" internal/vfs/localfileio/path.goRepository: larksuite/cli
Length of output: 951
🏁 Script executed:
# Confirm vfs is properly exported and available to import
head -20 internal/vfs/vfs.goRepository: larksuite/cli
Length of output: 135
🏁 Script executed:
# List what's in the vfs package
ls -la internal/vfs/Repository: larksuite/cli
Length of output: 470
🏁 Script executed:
# Find the vfs package entry point
fd "vfs" internal/ --type f | head -10Repository: larksuite/cli
Length of output: 39
🏁 Script executed:
# Check if vfs is imported from internal/core as shown in config.go
grep -n "import.*vfs" internal/core/config.goRepository: larksuite/cli
Length of output: 39
🏁 Script executed:
# Check the vfs package interface
cat internal/vfs/fs.goRepository: larksuite/cli
Length of output: 959
🏁 Script executed:
# Check the default implementation
cat internal/vfs/default.goRepository: larksuite/cli
Length of output: 1600
🏁 Script executed:
# Check how config.go imports vfs
grep -B2 -A10 "import" internal/core/config.go | head -30Repository: larksuite/cli
Length of output: 254
🏁 Script executed:
# Check how config.go actually imports and uses vfs
grep -n "vfs" internal/core/config.goRepository: larksuite/cli
Length of output: 237
🏁 Script executed:
# Verify the pattern - does config.go use validate.SafeInputPath or direct vfs?
sed -n '185,195p' internal/core/config.goRepository: larksuite/cli
Length of output: 390
🏁 Script executed:
# Check what GetConfigPath returns to understand the full flow
grep -B5 "func GetConfigPath" internal/core/config.goRepository: larksuite/cli
Length of output: 180
Replace os.ReadFile with vfs.ReadFile to comply with filesystem access guidelines.
Line 259 must use vfs.ReadFile instead of os.ReadFile per the coding guidelines for all filesystem operations in production code. This ensures testability and consistent behavior across the codebase. Add "github.com/larksuite/cli/internal/vfs" to imports and replace the call.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extension/credential/jsonfile/jsonfile.go` around lines 259 - 262, Replace
the os.ReadFile call with vfs.ReadFile in the credential loading logic (the
snippet that reads into data using variable cleaned) and update imports to
include "github.com/larksuite/cli/internal/vfs"; keep the existing error
handling that returns &credential.BlockError{Provider: "jsonfile", Reason:
fmt.Sprintf(...)} but use the error returned by vfs.ReadFile so behavior is
unchanged except using the vfs layer for filesystem access.
2 participants
Summary
Add a new credential extension provider (
jsonfile) that enables non-interactive credential injection via a JSON file, specified through theLARKSUITE_CLI_CREDENTIAL_FILEenvironment variable.This is useful for CI/CD pipelines, Docker containers, and scenarios where interactive
config initis not feasible.Changes
extension/credential/jsonfile/jsonfile.go— JSON file credential provider implementing thecredential.Providerinterfaceextension/credential/jsonfile/jsonfile_test.go— 27 comprehensive test cases with race detectioninternal/envvars/envvars.go— AddedLARKSUITE_CLI_CREDENTIAL_FILEconstantmain.go— Blank import to activate the jsonfile providerREADME.md,README.zh.md— Documentation for credential file usageKey Features
app_id,app_secret,brand,default_as,user_access_token,tenant_access_token, andrefresh_tokenfrom a JSON filerefresh_tokenis provided (with in-process caching)BlockErrorhandling for structured AI-agent-friendly error messagesconfig init)Test Plan
-race)go mod tidy— no changes togo.mod/go.sumgolangci-lint run --new-from-rev=origin/main— 0 issuesgo build ./...— compiles successfullyLARKSUITE_CLI_CREDENTIAL_FILE=/path/to/cred.json lark-cli --versionworksRelated Issues
Summary by CodeRabbit
New Features
LARKSUITE_CLI_CREDENTIAL_FILEenvironment variable, allowing users to bypass interactive credential setup.Documentation