From f3c3a4c49f36482d7b93bb15cccd1780a990ff3d Mon Sep 17 00:00:00 2001 From: JackZhao10086 Date: Wed, 8 Apr 2026 11:06:58 +0800 Subject: [PATCH] feat: support custom data dir and log directories (#302) * feat: linux support custom data dir via environment variable * feat(keychain): support custom log directory via LARKSUITE_CLI_LOG_DIR * feat(security): validate env dir paths for security Add validation for environment variable directory paths to ensure they are absolute and safe. This prevents potential security issues from malformed paths. Also add corresponding tests to verify the validation behavior. * docs(validate): add function and test documentation comments Add missing documentation comments for SafeEnvDirPath function and related test cases to improve code clarity and maintainability * refactor(keychain): remove warning logs for invalid env vars --- internal/keychain/auth_log.go | 8 +++++ internal/keychain/auth_log_test.go | 35 ++++++++++++++++++++++ internal/keychain/keychain_other.go | 7 +++++ internal/keychain/keychain_other_test.go | 37 ++++++++++++++++++++++++ internal/validate/path.go | 21 ++++++++++++++ internal/validate/path_test.go | 27 +++++++++++++++++ 6 files changed, 135 insertions(+) create mode 100644 internal/keychain/auth_log_test.go create mode 100644 internal/keychain/keychain_other_test.go diff --git a/internal/keychain/auth_log.go b/internal/keychain/auth_log.go index 079f8cd9..8bc9e947 100644 --- a/internal/keychain/auth_log.go +++ b/internal/keychain/auth_log.go @@ -9,6 +9,7 @@ import ( "sync" "time" + "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/internal/vfs" ) @@ -21,6 +22,13 @@ var ( ) func authLogDir() string { + if dir := os.Getenv("LARKSUITE_CLI_LOG_DIR"); dir != "" { + safeDir, err := validate.SafeEnvDirPath(dir, "LARKSUITE_CLI_LOG_DIR") + if err == nil { + return safeDir + } + } + if dir := os.Getenv("LARKSUITE_CLI_CONFIG_DIR"); dir != "" { return filepath.Join(dir, "logs") } diff --git a/internal/keychain/auth_log_test.go b/internal/keychain/auth_log_test.go new file mode 100644 index 00000000..c81fc1ff --- /dev/null +++ b/internal/keychain/auth_log_test.go @@ -0,0 +1,35 @@ +package keychain + +import ( + "path/filepath" + "testing" +) + +// TestAuthLogDir_UsesValidatedLogDirEnv verifies that a valid absolute +// LARKSUITE_CLI_LOG_DIR is normalized and used as the auth log directory. +func TestAuthLogDir_UsesValidatedLogDirEnv(t *testing.T) { + base := t.TempDir() + base, _ = filepath.EvalSymlinks(base) + t.Setenv("LARKSUITE_CLI_LOG_DIR", filepath.Join(base, "logs", "..", "auth")) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", "") + + got := authLogDir() + want := filepath.Join(base, "auth") + if got != want { + t.Fatalf("authLogDir() = %q, want %q", got, want) + } +} + +// TestAuthLogDir_InvalidLogDirFallsBackToConfigDir verifies that an invalid +// LARKSUITE_CLI_LOG_DIR falls back to LARKSUITE_CLI_CONFIG_DIR/logs. +func TestAuthLogDir_InvalidLogDirFallsBackToConfigDir(t *testing.T) { + t.Setenv("LARKSUITE_CLI_LOG_DIR", "relative-logs") + configDir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", configDir) + + got := authLogDir() + want := filepath.Join(configDir, "logs") + if got != want { + t.Fatalf("authLogDir() = %q, want %q", got, want) + } +} diff --git a/internal/keychain/keychain_other.go b/internal/keychain/keychain_other.go index d84ad84b..d0d094dd 100644 --- a/internal/keychain/keychain_other.go +++ b/internal/keychain/keychain_other.go @@ -16,6 +16,7 @@ import ( "regexp" "github.com/google/uuid" + "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/internal/vfs" ) @@ -25,6 +26,12 @@ 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 != "" { + safeDir, err := validate.SafeEnvDirPath(dir, "LARKSUITE_CLI_DATA_DIR") + if err == nil { + return filepath.Join(safeDir, service) + } + } home, err := vfs.UserHomeDir() if err != nil || home == "" { // If home is missing, fallback to relative path and print warning. diff --git a/internal/keychain/keychain_other_test.go b/internal/keychain/keychain_other_test.go new file mode 100644 index 00000000..a07cd881 --- /dev/null +++ b/internal/keychain/keychain_other_test.go @@ -0,0 +1,37 @@ +//go:build linux + +package keychain + +import ( + "path/filepath" + "testing" +) + +// TestStorageDir_UsesValidatedDataDirEnv verifies that a valid absolute +// LARKSUITE_CLI_DATA_DIR is normalized and still preserves service isolation. +func TestStorageDir_UsesValidatedDataDirEnv(t *testing.T) { + base := t.TempDir() + base, _ = filepath.EvalSymlinks(base) + t.Setenv("LARKSUITE_CLI_DATA_DIR", filepath.Join(base, "data", "..", "store")) + + got := StorageDir("svc") + want := filepath.Join(base, "store", "svc") + if got != want { + t.Fatalf("StorageDir() = %q, want %q", got, want) + } +} + +// TestStorageDir_InvalidDataDirFallsBackToDefault verifies that an invalid +// LARKSUITE_CLI_DATA_DIR falls back to the default per-service storage path. +func TestStorageDir_InvalidDataDirFallsBackToDefault(t *testing.T) { + home := t.TempDir() + home, _ = filepath.EvalSymlinks(home) + t.Setenv("LARKSUITE_CLI_DATA_DIR", "relative-data") + t.Setenv("HOME", home) + + got := StorageDir("svc") + want := filepath.Join(home, ".local", "share", "svc") + if got != want { + t.Fatalf("StorageDir() = %q, want %q", got, want) + } +} diff --git a/internal/validate/path.go b/internal/validate/path.go index ecc6e973..d46f3571 100644 --- a/internal/validate/path.go +++ b/internal/validate/path.go @@ -32,6 +32,27 @@ func SafeInputPath(path string) (string, error) { return safePath(path, "--file") } +// SafeEnvDirPath validates an environment-provided application directory path. +// It requires an absolute path, rejects control characters, normalizes the +// input, and resolves symlinks through the nearest existing ancestor so callers +// receive a canonical path for subsequent filesystem operations. +func SafeEnvDirPath(path, envName string) (string, error) { + if err := RejectControlChars(path, envName); err != nil { + return "", err + } + + path = filepath.Clean(path) + if !filepath.IsAbs(path) { + return "", fmt.Errorf("%s must be an absolute path, got %q", envName, path) + } + + resolved, err := resolveNearestAncestor(path) + if err != nil { + return "", fmt.Errorf("cannot resolve symlinks: %w", err) + } + return resolved, nil +} + // SafeLocalFlagPath validates a flag value as a local file path. // Empty values and http/https URLs are returned unchanged without validation, // allowing the caller to handle non-path inputs (e.g. API keys, URLs) upstream. diff --git a/internal/validate/path_test.go b/internal/validate/path_test.go index bc6b1f48..b99a16a8 100644 --- a/internal/validate/path_test.go +++ b/internal/validate/path_test.go @@ -283,3 +283,30 @@ func TestSafeInputPath_ErrorMessageContainsCorrectFlagName(t *testing.T) { t.Errorf("error should mention --output, got: %s", err.Error()) } } + +// TestSafeEnvDirPath_RequiresAbsolutePath verifies that environment-provided +// directory paths must be absolute. +func TestSafeEnvDirPath_RequiresAbsolutePath(t *testing.T) { + _, err := SafeEnvDirPath("logs", "LARKSUITE_CLI_LOG_DIR") + if err == nil { + t.Fatal("expected error for relative path") + } + if !strings.Contains(err.Error(), "LARKSUITE_CLI_LOG_DIR") { + t.Fatalf("error should mention env name, got %v", err) + } +} + +// TestSafeEnvDirPath_ReturnsNormalizedAbsolutePath verifies that a valid +// absolute environment directory is cleaned and resolved to its canonical path. +func TestSafeEnvDirPath_ReturnsNormalizedAbsolutePath(t *testing.T) { + base := t.TempDir() + base, _ = filepath.EvalSymlinks(base) + got, err := SafeEnvDirPath(filepath.Join(base, "logs", "..", "auth"), "LARKSUITE_CLI_LOG_DIR") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := filepath.Join(base, "auth") + if got != want { + t.Fatalf("SafeEnvDirPath() = %q, want %q", got, want) + } +}