Skip to content

feat: support custom data dir and log directories #302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JackZhao10086 wants to merge 2 commits into
base: main
Choose a base branch
from
+7 −0
Open

feat: support custom data dir and log directories #302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8061c4c
feat: linux support custom data dir via environment variable
JackZhao10086 Apr 7, 2026
f567cc9
feat(keychain): support custom log directory via LARKSUITE_CLI_LOG_DIR
JackZhao10086 Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions internal/keychain/auth_log.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ var (
)

func authLogDir() string {
if dir := os.Getenv("LARKSUITE_CLI_LOG_DIR"); dir != "" {
return dir
}
Comment on lines +24 to +26
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Validate the environment variable path before use.

The LARKSUITE_CLI_LOG_DIR environment variable is user-controlled input that is returned directly and later used in file I/O operations (line 48: vfs.MkdirAll, line 54: vfs.OpenFile). Without validation, malicious or malformed paths could cause path traversal vulnerabilities or unexpected filesystem behavior.

As per coding guidelines: Validate paths using validate.SafeInputPath before any file I/O operations.

🛡️ Proposed fix to add path validation 
+import (
+	"github.com/larksuite/cli/internal/validate"
+)
+
 func authLogDir() string {
 	if dir := os.Getenv("LARKSUITE_CLI_LOG_DIR"); dir != "" {
+		if err := validate.SafeInputPath(dir); err != nil {
+			fmt.Fprintf(os.Stderr, "warning: invalid LARKSUITE_CLI_LOG_DIR: %v, using default\n", err)
+			// Fall through to default logic
+		} else {
-		return dir
+			return dir
+		}
 	}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/keychain/auth_log.go` around lines 24 - 26, The returned
LARKSUITE_CLI_LOG_DIR value is used later by vfs.MkdirAll and vfs.OpenFile and
must be validated; update the function in auth_log.go that reads
os.Getenv("LARKSUITE_CLI_LOG_DIR") to call validate.SafeInputPath (or the
project’s equivalent) on the value and only return it if validation succeeds,
otherwise fall back to the existing default path or an empty string and
log/handle the invalid input; ensure the validation happens before any use in
vfs.MkdirAll or vfs.OpenFile so the path cannot be used without passing
validate.SafeInputPath.


if dir := os.Getenv("LARKSUITE_CLI_CONFIG_DIR"); dir != "" {
return filepath.Join(dir, "logs")
}
Expand Down
3 changes: 3 additions & 0 deletions internal/keychain/keychain_other.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ const tagBytes = 16

// StorageDir returns the directory where encrypted files are stored.
func StorageDir(service string) string {
if dir := os.Getenv("LARKSUITE_CLI_DATA_DIR"); dir != "" {
return dir
Comment on lines +28 to +29
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Append service name to avoid data conflicts between services.

When LARKSUITE_CLI_DATA_DIR is set, the function returns the raw environment variable value without appending the service parameter. This means different services (e.g., StorageDir("service1") and StorageDir("service2")) will share the same directory, which can lead to:

  1. Master key conflicts: Multiple services would share the same master.key file, breaking encryption isolation
  2. Credential overwrites: Encrypted credential files with identical account names would collide across services
  3. Security boundary violation: Service isolation is a security design principle

The default behavior (line 38) correctly appends the service name: filepath.Join(xdgData, service). The environment variable override must maintain this structure.

🔧 Proposed fix to append service name 
 func StorageDir(service string) string {
 	if dir := os.Getenv("LARKSUITE_CLI_DATA_DIR"); dir != "" {
-		return dir
+		return filepath.Join(dir, service)
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if dir := os.Getenv("LARKSUITE_CLI_DATA_DIR"); dir != "" {
return dir
if dir := os.Getenv("LARKSUITE_CLI_DATA_DIR"); dir != "" {
return filepath.Join(dir, service)
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/keychain/keychain_other.go` around lines 28 - 29, The function
currently returns the raw LARKSUITE_CLI_DATA_DIR env value
(os.Getenv("LARKSUITE_CLI_DATA_DIR")) without appending the service name,
causing data/credential collisions; change the branch in StorageDir to return
filepath.Join(dir, service) instead of dir so the environment override preserves
per-service isolation (mirror the default branch which uses
filepath.Join(xdgData, service)).

}
Comment on lines +28 to +30
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Validate the environment variable path before use.

The LARKSUITE_CLI_DATA_DIR environment variable is user-controlled input that is returned directly and later used in file I/O operations (lines 52, 71, 148, 174, 183, 199). Without validation, malicious or malformed paths could cause path traversal vulnerabilities or unexpected filesystem behavior.

As per coding guidelines: Validate paths using validate.SafeInputPath before any file I/O operations.

🛡️ Proposed fix to add path validation 
+import (
+	"github.com/larksuite/cli/internal/validate"
+)
+
 func StorageDir(service string) string {
 	if dir := os.Getenv("LARKSUITE_CLI_DATA_DIR"); dir != "" {
+		if err := validate.SafeInputPath(dir); err != nil {
+			fmt.Fprintf(os.Stderr, "warning: invalid LARKSUITE_CLI_DATA_DIR: %v, using default\n", err)
+			// Fall through to default logic
+		} else {
-		return dir
+			return dir
+		}
 	}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/keychain/keychain_other.go` around lines 28 - 30, The code returns
the LARKSUITE_CLI_DATA_DIR env value directly; validate this user-controlled
path with validate.SafeInputPath before returning/using it to prevent path
traversal or malformed paths. Modify the function that reads
os.Getenv("LARKSUITE_CLI_DATA_DIR") to call validate.SafeInputPath(dir) and
handle validation errors (fallback to default dir or return an error) so all
subsequent uses (the functions reading files in this package) receive a
validated path.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check if macOS StorageDir implementation supports LARKSUITE_CLI_DATA_DIR

rg -n "LARKSUITE_CLI_DATA_DIR" internal/keychain/keychain_darwin.go

Repository: larksuite/cli

Length of output: 39

🏁 Script executed:

# Check the macOS StorageDir implementation
cat -n internal/keychain/keychain_darwin.go | sed -n '40,60p'

Repository: larksuite/cli

Length of output: 967

🏁 Script executed:

# List all platform-specific keychain implementations
fd -e go internal/keychain/ | sort

Repository: larksuite/cli

Length of output: 434

🏁 Script executed:

# Check what keychain_other.go represents and its full context
head -n 40 internal/keychain/keychain_other.go

Repository: larksuite/cli

Length of output: 976

🏁 Script executed:

# Find all keychain implementation files
fd . internal/keychain/ --type f -e go | sort

Repository: larksuite/cli

Length of output: 298

🏁 Script executed:

# Search for any documentation or reasoning about platform-specific StorageDir
rg -n "StorageDir|LARKSUITE_CLI_DATA_DIR" internal/keychain/ -B2 -A2

Repository: larksuite/cli

Length of output: 5429

🏁 Script executed:

# Check if there's documentation in config or other places about this env var
rg -n "LARKSUITE_CLI_DATA_DIR" --type go | head -20

Repository: larksuite/cli

Length of output: 155

🏁 Script executed:

# Check Windows implementation for StorageDir
cat -n internal/keychain/keychain_windows.go | sed -n '1,80p'

Repository: larksuite/cli

Length of output: 3114

🏁 Script executed:

# Check if LARKSUITE_CLI_DATA_DIR is documented anywhere
rg -n "LARKSUITE_CLI_DATA_DIR" -B3 -A3 --type-add='markdown:*.md'

Repository: larksuite/cli

Length of output: 549

🏁 Script executed:

# Check tests for keychain_other to see if env var override is tested
cat internal/keychain/keychain_other.go | grep -n "func Test"

Repository: larksuite/cli

Length of output: 39

🏁 Script executed:

# Check if there's a test file for keychain_other
fd . internal/keychain/ --name '*other*test*'

Repository: larksuite/cli

Length of output: 289

🏁 Script executed:

# Look for any test that uses LARKSUITE_CLI_DATA_DIR
rg -n "LARKSUITE_CLI_DATA_DIR" --type go

Repository: larksuite/cli

Length of output: 155

🏁 Script executed:

# Check if there are tests that might use t.Setenv for this
rg -n "Setenv.*LARKSUITE_CLI" --type go

Repository: larksuite/cli

Length of output: 3772

Add environment variable override to macOS StorageDir for consistency, or document as Linux-only.

The Linux implementation in keychain_other.go:28-30 checks LARKSUITE_CLI_DATA_DIR to allow runtime override of the storage directory, but the macOS implementation in keychain_darwin.go:46-53 does not. This creates inconsistent behavior across platforms where users on Linux can override the storage location but users on macOS cannot.

Consider either:

  1. Adding the same LARKSUITE_CLI_DATA_DIR check to the macOS StorageDir function for feature parity
  2. Explicitly documenting this as a Linux-only feature with reasoning (e.g., if intentional by design)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/keychain/keychain_other.go` around lines 28 - 30, The macOS
StorageDir implementation (StorageDir in keychain_darwin.go) is missing the
LARKSUITE_CLI_DATA_DIR environment override present in the other implementation
(keychain_other.go); add the same env check at the start of StorageDir in
keychain_darwin.go to return that dir if set (or, alternatively, add a comment
in StorageDir documenting that the override is intentionally Linux-only). Update
the StorageDir function in keychain_darwin.go to mirror the logic: read
os.Getenv("LARKSUITE_CLI_DATA_DIR") and return it when non-empty before falling
back to the existing macOS directory resolution.

Comment on lines 27 to +30
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 service parameter silently dropped when env var is set

When LARKSUITE_CLI_DATA_DIR is set, StorageDir returns the directory as-is, discarding the service argument. The default code path returns filepath.Join(xdgData, service), so a user who sets LARKSUITE_CLI_DATA_DIR=/home/user/.local/share expecting the same layout would instead find master.key and all .enc credential files directly in /home/user/.local/share rather than /home/user/.local/share/lark-cli/.

Suggested change
func StorageDir(service string) string {
if dir := os.Getenv("LARKSUITE_CLI_DATA_DIR"); dir != "" {
return dir
}
if dir := os.Getenv("LARKSUITE_CLI_DATA_DIR"); dir != "" {
return filepath.Join(dir, service)
}

home, err := vfs.UserHomeDir()
if err != nil || home == "" {
// If home is missing, fallback to relative path and print warning.
Expand Down
Loading