diff --git a/cmd/auth/login_scope_cache.go b/cmd/auth/login_scope_cache.go index ad8036bda..5407ead5e 100644 --- a/cmd/auth/login_scope_cache.go +++ b/cmd/auth/login_scope_cache.go @@ -10,8 +10,8 @@ import ( "path/filepath" "regexp" + "github.com/larksuite/cli/internal/appdir" larkauth "github.com/larksuite/cli/internal/auth" - "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/internal/vfs" ) @@ -25,7 +25,7 @@ type loginScopeCacheRecord struct { // loginScopeCacheDir returns the directory used to persist auth login --no-wait // requested scopes keyed by device_code. func loginScopeCacheDir() string { - return filepath.Join(core.GetConfigDir(), "cache", "auth_login_scopes") + return filepath.Join(appdir.CacheDir(), "auth_login_scopes") } // loginScopeCachePath returns the cache file path for a given device_code. @@ -44,14 +44,14 @@ func sanitizeLoginScopeCacheKey(deviceCode string) string { // saveLoginRequestedScope persists the requested scope string for a device_code. func saveLoginRequestedScope(deviceCode, requestedScope string) error { - if err := vfs.MkdirAll(loginScopeCacheDir(), 0700); err != nil { + if err := vfs.MkdirAll(loginScopeCacheDir(), 0o700); err != nil { return err } data, err := json.Marshal(loginScopeCacheRecord{RequestedScope: requestedScope}) if err != nil { return err } - return validate.AtomicWrite(loginScopeCachePath(deviceCode), data, 0600) + return validate.AtomicWrite(loginScopeCachePath(deviceCode), data, 0o600) } // loadLoginRequestedScope loads the cached requested scope string for a device_code. diff --git a/cmd/config/init_interactive.go b/cmd/config/init_interactive.go index f138a215c..c0ffdfd41 100644 --- a/cmd/config/init_interactive.go +++ b/cmd/config/init_interactive.go @@ -9,13 +9,12 @@ import ( "net/http" "github.com/charmbracelet/huh" - "github.com/larksuite/cli/internal/build" - qrcode "github.com/skip2/go-qrcode" - larkauth "github.com/larksuite/cli/internal/auth" + "github.com/larksuite/cli/internal/build" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/output" + qrcode "github.com/skip2/go-qrcode" ) // configInitResult holds the result of the interactive config init flow. @@ -169,7 +168,7 @@ func runCreateAppFlow(ctx context.Context, f *cmdutil.Factory, brandOverride cor // Step 1: Request app registration (begin) httpClient := &http.Client{} - authResp, err := larkauth.RequestAppRegistration(httpClient, larkBrand, f.IOStreams.ErrOut) + authResp, err := larkauth.RequestAppRegistration(httpClient, larkBrand) if err != nil { return nil, output.ErrAuth("app registration failed: %v", err) } diff --git a/internal/appdir/appdir.go b/internal/appdir/appdir.go new file mode 100644 index 000000000..a607d2d7f --- /dev/null +++ b/internal/appdir/appdir.go @@ -0,0 +1,180 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package appdir + +import ( + "fmt" + "log" + "os" + "path/filepath" + "runtime" + + "github.com/larksuite/cli/internal/validate" + "github.com/larksuite/cli/internal/vfs" +) + +const appName = "lark-cli" + +// ConfigDir returns the CLI config directory. +func ConfigDir() string { + if dir, ok := validatedEnvDir("LARKSUITE_CLI_CONFIG_DIR"); ok { + return dir + } + if dir, ok := xdgAppDir("XDG_CONFIG_HOME"); ok { + return dir + } + if dir, ok := legacyConfigDir(); ok { + return dir + } + return defaultConfigDir() +} + +// ConfigPath returns the CLI config file path. +func ConfigPath() string { + return filepath.Join(ConfigDir(), "config.json") +} + +// CacheDir returns the CLI cache directory. +func CacheDir() string { + if dir, ok := validatedEnvDir("LARKSUITE_CLI_CACHE_DIR"); ok { + return dir + } + if dir, ok := xdgAppDir("XDG_CACHE_HOME"); ok { + return dir + } + if dir, ok := legacyConfigDir(); ok { + return filepath.Join(dir, "cache") + } + return defaultCacheDir() +} + +// StateDir returns the CLI state directory. +func StateDir() string { + if dir, ok := validatedEnvDir("LARKSUITE_CLI_STATE_DIR"); ok { + return dir + } + if dir, ok := xdgAppDir("XDG_STATE_HOME"); ok { + return dir + } + if dir, ok := legacyConfigDir(); ok { + return dir + } + return defaultStateDir() +} + +// LogDir returns the CLI log directory. +func LogDir() string { + if dir, ok := validatedEnvDir("LARKSUITE_CLI_LOG_DIR"); ok { + return dir + } + return filepath.Join(StateDir(), "logs") +} + +// DataDir returns the directory used for service-specific stored data. +// It returns an error if service contains path separators, traversal sequences, +// or other unsafe characters. +func DataDir(service string) (string, error) { + if err := validate.SafeServiceName(service); err != nil { + return "", fmt.Errorf("appdir.DataDir: invalid service name: %w", err) + } + if dir, ok := validatedEnvDir("LARKSUITE_CLI_DATA_DIR"); ok { + return filepath.Join(dir, service), nil + } + if dir, ok := xdgDataDir(service); ok { + return dir, nil + } + if dir, ok := legacyDataDir(service); ok { + return dir, nil + } + return defaultDataDir(service), nil +} + +func validatedEnvDir(envName string) (string, bool) { + value := os.Getenv(envName) + if value == "" { + return "", false + } + dir, err := validate.SafeEnvDirPath(value, envName) + if err != nil { + fmt.Fprintf(log.Writer(), "warning: %s=%q is invalid (%v), using default\n", envName, value, err) + return "", false + } + return dir, true +} + +func xdgAppDir(envName string) (string, bool) { + base, ok := validatedEnvDir(envName) + if !ok { + return "", false + } + return filepath.Join(base, appName), true +} + +func xdgDataDir(service string) (string, bool) { + base, ok := validatedEnvDir("XDG_DATA_HOME") + if !ok { + return "", false + } + return filepath.Join(base, appName, service), true +} + +func legacyConfigDir() (string, bool) { + dir := filepath.Join(homeDir(), ".lark-cli") + return dir, dirExists(dir) +} + +func legacyDataDir(service string) (string, bool) { + var dir string + switch runtime.GOOS { + case "darwin": + dir = filepath.Join(homeDir(), "Library", "Application Support", service) + default: + return "", false + } + return dir, dirExists(dir) +} + +func defaultConfigDir() string { + if runtime.GOOS == "windows" { + return filepath.Join(homeDir(), ".lark-cli") + } + return filepath.Join(homeDir(), ".config", appName) +} + +func defaultCacheDir() string { + if runtime.GOOS == "windows" { + return filepath.Join(ConfigDir(), "cache") + } + return filepath.Join(homeDir(), ".cache", appName) +} + +func defaultStateDir() string { + if runtime.GOOS == "windows" { + return ConfigDir() + } + return filepath.Join(homeDir(), ".local", "state", appName) +} + +func defaultDataDir(service string) string { + if runtime.GOOS == "windows" { + return filepath.Join(homeDir(), ".lark-cli", "data", service) + } + return filepath.Join(homeDir(), ".local", "share", appName, service) +} + +func dirExists(path string) bool { + info, err := vfs.Stat(path) + if err != nil { + return false + } + return info.IsDir() +} + +func homeDir() string { + home, err := vfs.UserHomeDir() + if err != nil || home == "" { + fmt.Fprintf(log.Writer(), "warning: unable to determine home directory: %v\n", err) + } + return home +} diff --git a/internal/appdir/appdir_test.go b/internal/appdir/appdir_test.go new file mode 100644 index 000000000..768d7ef9a --- /dev/null +++ b/internal/appdir/appdir_test.go @@ -0,0 +1,197 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package appdir + +import ( + "os" + "path/filepath" + "runtime" + "testing" +) + +func TestConfigDir_PrefersExplicitOverride(t *testing.T) { + base := realPath(t, t.TempDir()) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", filepath.Join(base, "override")) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(base, "xdg-config")) + t.Setenv("HOME", base) + + got := ConfigDir() + want := filepath.Join(base, "override") + if got != want { + t.Fatalf("ConfigDir() = %q, want %q", got, want) + } +} + +func TestConfigDir_UsesLegacyDirWhenPresent(t *testing.T) { + home := realPath(t, t.TempDir()) + legacy := filepath.Join(home, ".lark-cli") + if err := osMkdirAll(legacy); err != nil { + t.Fatalf("create legacy dir: %v", err) + } + t.Setenv("HOME", home) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", "") + t.Setenv("XDG_CONFIG_HOME", "") + + got := ConfigDir() + if got != legacy { + t.Fatalf("ConfigDir() = %q, want %q", got, legacy) + } +} + +func TestConfigDir_UsesXDGWhenLegacyMissing(t *testing.T) { + home := realPath(t, t.TempDir()) + xdg := filepath.Join(home, "xdg-config") + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", xdg) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", "") + + got := ConfigDir() + want := filepath.Join(xdg, appName) + if got != want { + t.Fatalf("ConfigDir() = %q, want %q", got, want) + } +} + +func TestCacheDir_PrefersExplicitOverride(t *testing.T) { + base := realPath(t, t.TempDir()) + cacheDir := filepath.Join(base, "my-cache") + t.Setenv("LARKSUITE_CLI_CACHE_DIR", cacheDir) + t.Setenv("XDG_CACHE_HOME", filepath.Join(base, "xdg-cache")) + + if got := CacheDir(); got != cacheDir { + t.Fatalf("CacheDir() = %q, want %q", got, cacheDir) + } +} + +func TestStateDir_PrefersExplicitOverride(t *testing.T) { + base := realPath(t, t.TempDir()) + stateDir := filepath.Join(base, "my-state") + t.Setenv("LARKSUITE_CLI_STATE_DIR", stateDir) + t.Setenv("XDG_STATE_HOME", filepath.Join(base, "xdg-state")) + + if got := StateDir(); got != stateDir { + t.Fatalf("StateDir() = %q, want %q", got, stateDir) + } + if got, want := LogDir(), filepath.Join(stateDir, "logs"); got != want { + t.Fatalf("LogDir() = %q, want %q", got, want) + } +} + +func TestCacheStateAndLogDir_UseXDGBaseDirs(t *testing.T) { + base := realPath(t, t.TempDir()) + t.Setenv("HOME", base) + t.Setenv("XDG_CACHE_HOME", filepath.Join(base, "xdg-cache")) + t.Setenv("XDG_STATE_HOME", filepath.Join(base, "xdg-state")) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", "") + t.Setenv("LARKSUITE_CLI_CACHE_DIR", "") + t.Setenv("LARKSUITE_CLI_STATE_DIR", "") + t.Setenv("LARKSUITE_CLI_LOG_DIR", "") + + if got, want := CacheDir(), filepath.Join(base, "xdg-cache", appName); got != want { + t.Fatalf("CacheDir() = %q, want %q", got, want) + } + if got, want := StateDir(), filepath.Join(base, "xdg-state", appName); got != want { + t.Fatalf("StateDir() = %q, want %q", got, want) + } + if got, want := LogDir(), filepath.Join(base, "xdg-state", appName, "logs"); got != want { + t.Fatalf("LogDir() = %q, want %q", got, want) + } +} + +func TestLogDir_PrefersExplicitOverride(t *testing.T) { + base := realPath(t, t.TempDir()) + logDir := filepath.Join(base, "logs") + t.Setenv("HOME", base) + t.Setenv("LARKSUITE_CLI_LOG_DIR", logDir) + t.Setenv("LARKSUITE_CLI_STATE_DIR", filepath.Join(base, "state")) + t.Setenv("XDG_STATE_HOME", filepath.Join(base, "xdg-state")) + + got := LogDir() + if got != logDir { + t.Fatalf("LogDir() = %q, want %q", got, logDir) + } +} + +func TestDataDir_UsesXDGDataHome(t *testing.T) { + base := realPath(t, t.TempDir()) + t.Setenv("HOME", base) + t.Setenv("XDG_DATA_HOME", filepath.Join(base, "xdg-data")) + t.Setenv("LARKSUITE_CLI_DATA_DIR", "") + + got, err := DataDir("svc") + if err != nil { + t.Fatalf("DataDir() error = %v", err) + } + want := filepath.Join(base, "xdg-data", appName, "svc") + if got != want { + t.Fatalf("DataDir() = %q, want %q", got, want) + } +} + +func TestDataDir_DefaultPathIncludesAppNamespace(t *testing.T) { + base := realPath(t, t.TempDir()) + t.Setenv("HOME", base) + t.Setenv("XDG_DATA_HOME", "") + t.Setenv("LARKSUITE_CLI_DATA_DIR", "") + + got, err := DataDir("svc") + if err != nil { + t.Fatalf("DataDir() error = %v", err) + } + + want := filepath.Join(base, ".local", "share", appName, "svc") + if runtime.GOOS == "windows" { + want = filepath.Join(base, ".lark-cli", "data", "svc") + } + if got != want { + t.Fatalf("DataDir() = %q, want %q", got, want) + } +} + +func TestDataDir_PrefersDarwinLegacyDirWhenPresent(t *testing.T) { + if runtime.GOOS != "darwin" { + t.Skip("darwin-only legacy path") + } + + home := realPath(t, t.TempDir()) + legacy := filepath.Join(home, "Library", "Application Support", "svc") + if err := osMkdirAll(legacy); err != nil { + t.Fatalf("create legacy dir: %v", err) + } + t.Setenv("HOME", home) + t.Setenv("XDG_DATA_HOME", "") + t.Setenv("LARKSUITE_CLI_DATA_DIR", "") + + got, err := DataDir("svc") + if err != nil { + t.Fatalf("DataDir() error = %v", err) + } + if got != legacy { + t.Fatalf("DataDir() = %q, want %q", got, legacy) + } +} + +func TestDataDir_RejectsUnsafeServiceName(t *testing.T) { + for _, bad := range []string{"", ".", "..", "../etc", "foo/bar", "svc\x00name", "svc\tname", "svc\nname", "svc\rname"} { + t.Run(bad, func(t *testing.T) { + _, err := DataDir(bad) + if err == nil { + t.Errorf("DataDir(%q) returned no error, want error", bad) + } + }) + } +} + +func realPath(t *testing.T, path string) string { + t.Helper() + resolved, err := filepath.EvalSymlinks(path) + if err != nil { + t.Fatalf("EvalSymlinks(%q): %v", path, err) + } + return resolved +} + +func osMkdirAll(path string) error { + return os.MkdirAll(path, 0o700) +} diff --git a/internal/auth/app_registration.go b/internal/auth/app_registration.go index 2bdf50fba..afd16b0c9 100644 --- a/internal/auth/app_registration.go +++ b/internal/auth/app_registration.go @@ -40,11 +40,7 @@ type AppRegUserInfo struct { } // RequestAppRegistration initiates the app registration device flow. -func RequestAppRegistration(httpClient *http.Client, brand core.LarkBrand, errOut io.Writer) (*AppRegistrationResponse, error) { - if errOut == nil { - errOut = io.Discard - } - +func RequestAppRegistration(httpClient *http.Client, brand core.LarkBrand) (*AppRegistrationResponse, error) { ep := core.ResolveEndpoints(brand) regEp := core.ResolveEndpoints(core.BrandFeishu) // registration begin always uses feishu endpoint := regEp.Accounts + PathAppRegistration diff --git a/internal/auth/device_flow.go b/internal/auth/device_flow.go index 7d2c8ca41..fca171558 100644 --- a/internal/auth/device_flow.go +++ b/internal/auth/device_flow.go @@ -60,11 +60,7 @@ func ResolveOAuthEndpoints(brand core.LarkBrand) OAuthEndpoints { } // RequestDeviceAuthorization requests a device authorization code. -func RequestDeviceAuthorization(httpClient *http.Client, appId, appSecret string, brand core.LarkBrand, scope string, errOut io.Writer) (*DeviceAuthResponse, error) { - if errOut == nil { - errOut = io.Discard - } - +func RequestDeviceAuthorization(httpClient *http.Client, appId, appSecret string, brand core.LarkBrand, scope string, _ io.Writer) (*DeviceAuthResponse, error) { endpoints := ResolveOAuthEndpoints(brand) if !strings.Contains(scope, "offline_access") { diff --git a/internal/auth/transport.go b/internal/auth/transport.go index 567885418..ceb16339c 100644 --- a/internal/auth/transport.go +++ b/internal/auth/transport.go @@ -66,20 +66,18 @@ func (t *SecurityPolicyTransport) RoundTrip(req *http.Request) (*http.Response, // Try to parse it as JSON var result map[string]interface{} - if err := json.Unmarshal(bodyBytes, &result); err != nil { - return resp, nil - } - - // 1. Try to handle as MCP (JSON-RPC) format first - if err := t.tryHandleMCPResponse(result); err != nil { - resp.Body.Close() - return nil, err - } + if json.Unmarshal(bodyBytes, &result) == nil { + // 1. Try to handle as MCP (JSON-RPC) format first + if err := t.tryHandleMCPResponse(result); err != nil { + resp.Body.Close() + return nil, err + } - // 2. Try to handle as OpenAPI error format - if err := t.tryHandleOAPIResponse(result); err != nil { - resp.Body.Close() - return nil, err + // 2. Try to handle as OpenAPI error format + if err := t.tryHandleOAPIResponse(result); err != nil { + resp.Body.Close() + return nil, err + } } return resp, nil diff --git a/internal/auth/uat_client.go b/internal/auth/uat_client.go index 35bf4133a..a270fe467 100644 --- a/internal/auth/uat_client.go +++ b/internal/auth/uat_client.go @@ -10,7 +10,6 @@ import ( "io" "net/http" "net/url" - "os" "path/filepath" "regexp" "strings" @@ -18,12 +17,20 @@ import ( "time" "github.com/gofrs/flock" + "github.com/larksuite/cli/internal/appdir" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/vfs" ) var safeIDChars = regexp.MustCompile(`[^a-zA-Z0-9._-]`) +func diagnosticWriter(w io.Writer) io.Writer { + if w == nil { + return io.Discard + } + return w +} + // sanitizeID replaces empty IDs with "default" to prevent file path issues. func sanitizeID(id string) string { return safeIDChars.ReplaceAllString(id, "_") @@ -51,15 +58,12 @@ type UATStatus struct { // NewUATCallOptions creates UATCallOptions from a CLI config. func NewUATCallOptions(cfg *core.CliConfig, errOut io.Writer) UATCallOptions { - if errOut == nil { - errOut = os.Stderr - } return UATCallOptions{ UserOpenId: cfg.UserOpenId, AppId: cfg.AppID, AppSecret: cfg.AppSecret, Domain: cfg.Brand, - ErrOut: errOut, + ErrOut: diagnosticWriter(errOut), } } @@ -91,11 +95,7 @@ func GetValidAccessToken(httpClient *http.Client, opts UATCallOptions) (string, // expired if err := RemoveStoredToken(opts.AppId, opts.UserOpenId); err != nil { - if opts.ErrOut != nil { - fmt.Fprintf(opts.ErrOut, "[lark-cli] [WARN] uat-client: failed to remove token: %v\n", err) - } else { - fmt.Fprintf(os.Stderr, "[lark-cli] [WARN] uat-client: failed to remove token: %v\n", err) - } + fmt.Fprintf(diagnosticWriter(opts.ErrOut), "[lark-cli] [WARN] uat-client: failed to remove token: %v\n", err) } return "", &NeedAuthorizationError{UserOpenId: opts.UserOpenId} } @@ -126,10 +126,10 @@ func refreshWithLock(httpClient *http.Client, opts UATCallOptions, stored *Store // 2. Cross-process lock using flock // We use the same underlying storage directory resolution as keychain_other.go // to ensure locks are isolated properly alongside other sensitive data. - configDir := core.GetConfigDir() + configDir := appdir.StateDir() lockDir := filepath.Join(configDir, "locks") - if err := vfs.MkdirAll(lockDir, 0700); err != nil { + if err := vfs.MkdirAll(lockDir, 0o700); err != nil { return nil, fmt.Errorf("failed to create lock directory: %w", err) } @@ -170,10 +170,7 @@ func refreshWithLock(httpClient *http.Client, opts UATCallOptions, stored *Store // doRefreshToken performs the actual HTTP request to refresh the token. func doRefreshToken(httpClient *http.Client, opts UATCallOptions, stored *StoredUAToken) (*StoredUAToken, error) { - errOut := opts.ErrOut - if errOut == nil { - errOut = os.Stderr - } + errOut := diagnosticWriter(opts.ErrOut) now := time.Now().UnixMilli() if now >= stored.RefreshExpiresAt { diff --git a/internal/auth/uat_client_options_test.go b/internal/auth/uat_client_options_test.go index d1f825773..35211343f 100644 --- a/internal/auth/uat_client_options_test.go +++ b/internal/auth/uat_client_options_test.go @@ -5,6 +5,7 @@ package auth import ( "bytes" + "io" "testing" "github.com/larksuite/cli/internal/core" @@ -38,3 +39,13 @@ func TestNewUATCallOptions(t *testing.T) { t.Error("ErrOut not set correctly") } } + +func TestNewUATCallOptions_NilErrOutUsesDiscard(t *testing.T) { + cfg := &core.CliConfig{} + + opts := NewUATCallOptions(cfg, nil) + + if opts.ErrOut != io.Discard { + t.Fatalf("ErrOut = %v, want io.Discard", opts.ErrOut) + } +} diff --git a/internal/cmdutil/factory_default.go b/internal/cmdutil/factory_default.go index c9b4e92cf..ea183a7fa 100644 --- a/internal/cmdutil/factory_default.go +++ b/internal/cmdutil/factory_default.go @@ -12,10 +12,6 @@ import ( "sync" "time" - lark "github.com/larksuite/oapi-sdk-go/v3" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - "golang.org/x/term" - extcred "github.com/larksuite/cli/extension/credential" "github.com/larksuite/cli/extension/fileio" "github.com/larksuite/cli/internal/auth" @@ -25,6 +21,9 @@ import ( "github.com/larksuite/cli/internal/registry" "github.com/larksuite/cli/internal/util" _ "github.com/larksuite/cli/internal/vfs/localfileio" // register default FileIO provider + lark "github.com/larksuite/oapi-sdk-go/v3" + larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + "golang.org/x/term" ) // NewDefault creates a production Factory with cached closures. @@ -40,17 +39,17 @@ func NewDefault(inv InvocationContext) *Factory { Invocation: inv, } f.IOStreams = &IOStreams{ - In: os.Stdin, - Out: os.Stdout, - ErrOut: os.Stderr, - IsTerminal: term.IsTerminal(int(os.Stdin.Fd())), + In: os.Stdin, //nolint:forbidigo // production wiring point + Out: os.Stdout, //nolint:forbidigo // production wiring point + ErrOut: os.Stderr, //nolint:forbidigo // production wiring point + IsTerminal: term.IsTerminal(int(os.Stdin.Fd())), //nolint:forbidigo // production wiring point } // Phase 0: FileIO provider (no dependency) f.FileIOProvider = fileio.GetProvider() // Phase 1: HttpClient (no credential dependency) - f.HttpClient = cachedHttpClientFunc() + f.HttpClient = cachedHttpClientFunc(f.IOStreams.ErrOut) // Phase 2: Credential (sole data source) f.Credential = buildCredentialProvider(credentialDeps{ @@ -67,7 +66,7 @@ func NewDefault(inv InvocationContext) *Factory { return nil, err } cfg := acct.ToCliConfig() - registry.InitWithBrand(cfg.Brand) + registry.InitWithBrand(cfg.Brand, f.IOStreams.ErrOut) return cfg, nil }) @@ -93,9 +92,9 @@ func safeRedirectPolicy(req *http.Request, via []*http.Request) error { return nil } -func cachedHttpClientFunc() func() (*http.Client, error) { +func cachedHttpClientFunc(errOut io.Writer) func() (*http.Client, error) { return sync.OnceValues(func() (*http.Client, error) { - util.WarnIfProxied(os.Stderr) + util.WarnIfProxied(errOut) var transport http.RoundTripper = util.NewBaseTransport() transport = &RetryTransport{Base: transport} @@ -122,7 +121,7 @@ func cachedLarkClientFunc(f *Factory) func() (*lark.Client, error) { lark.WithLogLevel(larkcore.LogLevelError), lark.WithHeaders(BaseSecurityHeaders()), } - util.WarnIfProxied(os.Stderr) + util.WarnIfProxied(f.IOStreams.ErrOut) opts = append(opts, lark.WithHttpClient(&http.Client{ Transport: buildSDKTransport(), CheckRedirect: safeRedirectPolicy, diff --git a/internal/cmdutil/factory_http_test.go b/internal/cmdutil/factory_http_test.go index c27e9e696..9eaeb180b 100644 --- a/internal/cmdutil/factory_http_test.go +++ b/internal/cmdutil/factory_http_test.go @@ -4,11 +4,12 @@ package cmdutil import ( + "io" "testing" ) func TestCachedHttpClientFunc_ReturnsSameInstance(t *testing.T) { - fn := cachedHttpClientFunc() + fn := cachedHttpClientFunc(io.Discard) c1, err := fn() if err != nil { @@ -28,7 +29,7 @@ func TestCachedHttpClientFunc_ReturnsSameInstance(t *testing.T) { } func TestCachedHttpClientFunc_HasTimeout(t *testing.T) { - fn := cachedHttpClientFunc() + fn := cachedHttpClientFunc(io.Discard) c, _ := fn() if c.Timeout == 0 { t.Error("expected non-zero timeout") @@ -36,7 +37,7 @@ func TestCachedHttpClientFunc_HasTimeout(t *testing.T) { } func TestCachedHttpClientFunc_HasRedirectPolicy(t *testing.T) { - fn := cachedHttpClientFunc() + fn := cachedHttpClientFunc(io.Discard) c, _ := fn() if c.CheckRedirect == nil { t.Error("expected CheckRedirect to be set (safeRedirectPolicy)") diff --git a/internal/core/config.go b/internal/core/config.go index ddd428aa3..77bf9411e 100644 --- a/internal/core/config.go +++ b/internal/core/config.go @@ -7,11 +7,10 @@ import ( "encoding/json" "errors" "fmt" - "os" - "path/filepath" "strings" "unicode/utf8" + "github.com/larksuite/cli/internal/appdir" "github.com/larksuite/cli/internal/keychain" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" @@ -167,19 +166,12 @@ type CliConfig struct { // If the home directory cannot be determined, it falls back to a relative path // and prints a warning to stderr. func GetConfigDir() string { - if dir := os.Getenv("LARKSUITE_CLI_CONFIG_DIR"); dir != "" { - return dir - } - home, err := vfs.UserHomeDir() - if err != nil || home == "" { - fmt.Fprintf(os.Stderr, "warning: unable to determine home directory: %v\n", err) - } - return filepath.Join(home, ".lark-cli") + return appdir.ConfigDir() } // GetConfigPath returns the config file path. func GetConfigPath() string { - return filepath.Join(GetConfigDir(), "config.json") + return appdir.ConfigPath() } // LoadMultiAppConfig loads multi-app config from disk. @@ -202,14 +194,14 @@ func LoadMultiAppConfig() (*MultiAppConfig, error) { // SaveMultiAppConfig saves config to disk. func SaveMultiAppConfig(config *MultiAppConfig) error { dir := GetConfigDir() - if err := vfs.MkdirAll(dir, 0700); err != nil { + if err := vfs.MkdirAll(dir, 0o700); err != nil { return err } data, err := json.MarshalIndent(config, "", " ") if err != nil { return err } - return validate.AtomicWrite(GetConfigPath(), append(data, '\n'), 0600) + return validate.AtomicWrite(GetConfigPath(), append(data, '\n'), 0o600) } // RequireConfig loads the single-app config using the default profile resolution. @@ -241,9 +233,11 @@ func ResolveConfigFromMulti(raw *MultiAppConfig, kc keychain.KeychainAccess, pro } if err := ValidateSecretKeyMatch(app.AppId, app.AppSecret); err != nil { - return nil, &ConfigError{Code: 2, Type: "config", + return nil, &ConfigError{ + Code: 2, Type: "config", Message: "appId and appSecret keychain key are out of sync", - Hint: err.Error()} + Hint: err.Error(), + } } secret, err := ResolveSecretInput(app.AppSecret, kc) diff --git a/internal/keychain/auth_log.go b/internal/keychain/auth_log.go index c74791267..539947d32 100644 --- a/internal/keychain/auth_log.go +++ b/internal/keychain/auth_log.go @@ -12,7 +12,7 @@ import ( "sync" "time" - "github.com/larksuite/cli/internal/validate" + "github.com/larksuite/cli/internal/appdir" "github.com/larksuite/cli/internal/vfs" ) @@ -25,23 +25,7 @@ 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") - } - - home, err := vfs.UserHomeDir() - if err != nil || home == "" { - fmt.Fprintf(os.Stderr, "warning: unable to determine home directory: %v\n", err) - } - - return filepath.Join(home, ".lark-cli", "logs") + return appdir.LogDir() } func initAuthLogger() { @@ -52,13 +36,13 @@ func initAuthLogger() { dir := authLogDir() now := authResponseLogNow() - if err := vfs.MkdirAll(dir, 0700); err != nil { + if err := vfs.MkdirAll(dir, 0o700); err != nil { return } logName := fmt.Sprintf("auth-%s.log", now.Format("2006-01-02")) logPath := filepath.Join(dir, logName) - if f, err := vfs.OpenFile(logPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600); err == nil { + if f, err := vfs.OpenFile(logPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600); err == nil { authResponseLogger = log.New(f, "", 0) cleanupOldLogs(dir, now) } diff --git a/internal/keychain/auth_log_test.go b/internal/keychain/auth_log_test.go index 423d2b1cb..a88cba920 100644 --- a/internal/keychain/auth_log_test.go +++ b/internal/keychain/auth_log_test.go @@ -12,7 +12,11 @@ import ( // 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) + var err error + base, err = filepath.EvalSymlinks(base) + if err != nil { + t.Fatalf("EvalSymlinks(%q): %v", base, err) + } t.Setenv("LARKSUITE_CLI_LOG_DIR", filepath.Join(base, "logs", "..", "auth")) t.Setenv("LARKSUITE_CLI_CONFIG_DIR", "") @@ -23,15 +27,21 @@ func TestAuthLogDir_UsesValidatedLogDirEnv(t *testing.T) { } } -// TestAuthLogDir_InvalidLogDirFallsBackToConfigDir verifies that an invalid -// LARKSUITE_CLI_LOG_DIR falls back to LARKSUITE_CLI_CONFIG_DIR/logs. -func TestAuthLogDir_InvalidLogDirFallsBackToConfigDir(t *testing.T) { +// TestAuthLogDir_InvalidLogDirFallsBackToStateDir verifies that an invalid +// LARKSUITE_CLI_LOG_DIR falls back to LARKSUITE_CLI_STATE_DIR/logs. +func TestAuthLogDir_InvalidLogDirFallsBackToStateDir(t *testing.T) { t.Setenv("LARKSUITE_CLI_LOG_DIR", "relative-logs") - configDir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", configDir) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + stateDir := t.TempDir() + var err error + stateDir, err = filepath.EvalSymlinks(stateDir) + if err != nil { + t.Fatalf("EvalSymlinks(%q): %v", stateDir, err) + } + t.Setenv("LARKSUITE_CLI_STATE_DIR", stateDir) got := authLogDir() - want := filepath.Join(configDir, "logs") + want := filepath.Join(stateDir, "logs") if got != want { t.Fatalf("authLogDir() = %q, want %q", got, want) } diff --git a/internal/keychain/keychain_darwin.go b/internal/keychain/keychain_darwin.go index 8e92e2bcc..0c91158df 100644 --- a/internal/keychain/keychain_darwin.go +++ b/internal/keychain/keychain_darwin.go @@ -18,6 +18,7 @@ import ( "time" "github.com/google/uuid" + "github.com/larksuite/cli/internal/appdir" "github.com/larksuite/cli/internal/vfs" "github.com/zalando/go-keyring" ) @@ -44,12 +45,8 @@ var keyringGet = keyring.Get var keyringSet = keyring.Set // StorageDir returns the storage directory for a given service name on macOS. -func StorageDir(service string) string { - home, err := vfs.UserHomeDir() - if err != nil || home == "" { - return filepath.Join(".lark-cli", "keychain", service) - } - return filepath.Join(home, "Library", "Application Support", service) +func StorageDir(service string) (string, error) { + return appdir.DataDir(service) } var safeFileNameRe = regexp.MustCompile(`[^a-zA-Z0-9._-]`) @@ -124,7 +121,10 @@ func getMasterKey(service string, allowCreate bool) ([]byte, error) { // getFileMasterKey retrieves the fallback master key from local storage. // If allowCreate is true, it generates and stores a new fallback master key when missing. func getFileMasterKey(service string, allowCreate bool) ([]byte, error) { - dir := StorageDir(service) + dir, err := StorageDir(service) + if err != nil { + return nil, err + } keyPath := filepath.Join(dir, fileMasterKeyName) key, err := vfs.ReadFile(keyPath) @@ -140,7 +140,7 @@ func getFileMasterKey(service string, allowCreate bool) ([]byte, error) { if !allowCreate { return nil, errNotInitialized } - if err := vfs.MkdirAll(dir, 0700); err != nil { + if err := vfs.MkdirAll(dir, 0o700); err != nil { return nil, err } key = make([]byte, masterKeyBytes) @@ -148,7 +148,7 @@ func getFileMasterKey(service string, allowCreate bool) ([]byte, error) { return nil, err } - file, err := vfs.OpenFile(keyPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600) + file, err := vfs.OpenFile(keyPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) if err != nil { if errors.Is(err, os.ErrExist) { for i := 0; i < 3; i++ { @@ -248,7 +248,11 @@ func decryptData(data []byte, key []byte) (string, error) { // platformGet retrieves a value from the macOS keychain. func platformGet(service, account string) (string, error) { - path := filepath.Join(StorageDir(service), safeFileName(account)) + dir, err := StorageDir(service) + if err != nil { + return "", err + } + path := filepath.Join(dir, safeFileName(account)) data, err := vfs.ReadFile(path) if errors.Is(err, os.ErrNotExist) { return "", nil @@ -274,8 +278,15 @@ func platformGet(service, account string) (string, error) { // platformSet stores a value in the macOS keychain. func platformSet(service, account, data string) error { + dir, err := StorageDir(service) + if err != nil { + return err + } key, err := getFileMasterKey(service, false) if err != nil { + if err := vfs.MkdirAll(dir, 0o700); err != nil { + return err + } key, err = getMasterKey(service, true) if err != nil { key, err = getFileMasterKey(service, true) @@ -284,10 +295,6 @@ func platformSet(service, account, data string) error { } } } - dir := StorageDir(service) - if err := vfs.MkdirAll(dir, 0700); err != nil { - return err - } encrypted, err := encryptData(data, key) if err != nil { return err @@ -297,7 +304,7 @@ func platformSet(service, account, data string) error { tmpPath := filepath.Join(dir, safeFileName(account)+"."+uuid.New().String()+".tmp") defer vfs.Remove(tmpPath) - if err := vfs.WriteFile(tmpPath, encrypted, 0600); err != nil { + if err := vfs.WriteFile(tmpPath, encrypted, 0o600); err != nil { return err } @@ -310,7 +317,11 @@ func platformSet(service, account, data string) error { // platformRemove deletes a value from the macOS keychain. func platformRemove(service, account string) error { - err := vfs.Remove(filepath.Join(StorageDir(service), safeFileName(account))) + dir, err := StorageDir(service) + if err != nil { + return err + } + err = vfs.Remove(filepath.Join(dir, safeFileName(account))) if err != nil && !os.IsNotExist(err) { return err } diff --git a/internal/keychain/keychain_darwin_test.go b/internal/keychain/keychain_darwin_test.go index 5dc9ddb9a..50cd862a0 100644 --- a/internal/keychain/keychain_darwin_test.go +++ b/internal/keychain/keychain_darwin_test.go @@ -42,7 +42,11 @@ func TestPlatformSetFallsBackToFileMasterKey(t *testing.T) { t.Fatalf("platformSet() error = %v", err) } - if _, err := os.Stat(filepath.Join(StorageDir(service), fileMasterKeyName)); err != nil { + sdir, err := StorageDir(service) + if err != nil { + t.Fatalf("StorageDir() error = %v", err) + } + if _, err := os.Stat(filepath.Join(sdir, fileMasterKeyName)); err != nil { t.Fatalf("file master key not created: %v", err) } @@ -87,18 +91,21 @@ func TestPlatformGetPrefersFileMasterKey(t *testing.T) { account := "test-account" secret := "secret-value" - dir := StorageDir(service) - if err := os.MkdirAll(dir, 0700); err != nil { + dir, err := StorageDir(service) + if err != nil { + t.Fatalf("StorageDir() error = %v", err) + } + if err := os.MkdirAll(dir, 0o700); err != nil { t.Fatalf("MkdirAll() error = %v", err) } - if err := os.WriteFile(filepath.Join(dir, fileMasterKeyName), fileKey, 0600); err != nil { + if err := os.WriteFile(filepath.Join(dir, fileMasterKeyName), fileKey, 0o600); err != nil { t.Fatalf("WriteFile(master key) error = %v", err) } encrypted, err := encryptData(secret, fileKey) if err != nil { t.Fatalf("encryptData() error = %v", err) } - if err := os.WriteFile(filepath.Join(dir, safeFileName(account)), encrypted, 0600); err != nil { + if err := os.WriteFile(filepath.Join(dir, safeFileName(account)), encrypted, 0o600); err != nil { t.Fatalf("WriteFile(secret) error = %v", err) } @@ -136,8 +143,11 @@ func TestPlatformSetPrefersExistingFileMasterKey(t *testing.T) { account := "test-account" secret := "secret-value" - dir := StorageDir(service) - if err := os.MkdirAll(dir, 0700); err != nil { + dir, err := StorageDir(service) + if err != nil { + t.Fatalf("StorageDir() error = %v", err) + } + if err := os.MkdirAll(dir, 0o700); err != nil { t.Fatalf("MkdirAll() error = %v", err) } @@ -145,7 +155,7 @@ func TestPlatformSetPrefersExistingFileMasterKey(t *testing.T) { for i := range fileKey { fileKey[i] = byte(i + 1) } - if err := os.WriteFile(filepath.Join(dir, fileMasterKeyName), fileKey, 0600); err != nil { + if err := os.WriteFile(filepath.Join(dir, fileMasterKeyName), fileKey, 0o600); err != nil { t.Fatalf("WriteFile(master key) error = %v", err) } diff --git a/internal/keychain/keychain_other.go b/internal/keychain/keychain_other.go index d0d094dd5..775c9303b 100644 --- a/internal/keychain/keychain_other.go +++ b/internal/keychain/keychain_other.go @@ -10,36 +10,24 @@ import ( "crypto/cipher" "crypto/rand" "errors" - "fmt" "os" "path/filepath" "regexp" "github.com/google/uuid" - "github.com/larksuite/cli/internal/validate" + "github.com/larksuite/cli/internal/appdir" "github.com/larksuite/cli/internal/vfs" ) -const masterKeyBytes = 32 -const ivBytes = 12 -const tagBytes = 16 +const ( + masterKeyBytes = 32 + ivBytes = 12 + 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. - // This matches the behavior in internal/core/config.go. - fmt.Fprintf(os.Stderr, "warning: unable to determine home directory: %v\n", err) - } - xdgData := filepath.Join(home, ".local", "share") - return filepath.Join(xdgData, service) +func StorageDir(service string) (string, error) { + return appdir.DataDir(service) } var safeFileNameRe = regexp.MustCompile(`[^a-zA-Z0-9._-]`) @@ -52,7 +40,10 @@ func safeFileName(account string) string { // getMasterKey retrieves the master key from the file system. // If allowCreate is true, it generates and stores a new master key if one doesn't exist. func getMasterKey(service string, allowCreate bool) ([]byte, error) { - dir := StorageDir(service) + dir, err := StorageDir(service) + if err != nil { + return nil, err + } keyPath := filepath.Join(dir, "master.key") key, err := vfs.ReadFile(keyPath) @@ -72,7 +63,7 @@ func getMasterKey(service string, allowCreate bool) ([]byte, error) { return nil, errNotInitialized } - if err := vfs.MkdirAll(dir, 0700); err != nil { + if err := vfs.MkdirAll(dir, 0o700); err != nil { return nil, err } @@ -84,7 +75,7 @@ func getMasterKey(service string, allowCreate bool) ([]byte, error) { tmpKeyPath := filepath.Join(dir, "master.key."+uuid.New().String()+".tmp") defer vfs.Remove(tmpKeyPath) - if err := vfs.WriteFile(tmpKeyPath, key, 0600); err != nil { + if err := vfs.WriteFile(tmpKeyPath, key, 0o600); err != nil { return nil, err } @@ -149,7 +140,11 @@ func decryptData(data []byte, key []byte) (string, error) { // platformGet retrieves a value from the file system. func platformGet(service, account string) (string, error) { - path := filepath.Join(StorageDir(service), safeFileName(account)) + dir, err := StorageDir(service) + if err != nil { + return "", err + } + path := filepath.Join(dir, safeFileName(account)) data, err := vfs.ReadFile(path) if errors.Is(err, os.ErrNotExist) { return "", nil @@ -174,8 +169,11 @@ func platformSet(service, account, data string) error { if err != nil { return err } - dir := StorageDir(service) - if err := vfs.MkdirAll(dir, 0700); err != nil { + dir, err := StorageDir(service) + if err != nil { + return err + } + if err := vfs.MkdirAll(dir, 0o700); err != nil { return err } encrypted, err := encryptData(data, key) @@ -187,7 +185,7 @@ func platformSet(service, account, data string) error { tmpPath := filepath.Join(dir, safeFileName(account)+"."+uuid.New().String()+".tmp") defer vfs.Remove(tmpPath) - if err := vfs.WriteFile(tmpPath, encrypted, 0600); err != nil { + if err := vfs.WriteFile(tmpPath, encrypted, 0o600); err != nil { return err } @@ -200,7 +198,11 @@ func platformSet(service, account, data string) error { // platformRemove deletes a value from the file system. func platformRemove(service, account string) error { - err := vfs.Remove(filepath.Join(StorageDir(service), safeFileName(account))) + dir, err := StorageDir(service) + if err != nil { + return err + } + err = vfs.Remove(filepath.Join(dir, safeFileName(account))) if err != nil && !os.IsNotExist(err) { return err } diff --git a/internal/keychain/keychain_other_test.go b/internal/keychain/keychain_other_test.go index e89d16e47..c44780c14 100644 --- a/internal/keychain/keychain_other_test.go +++ b/internal/keychain/keychain_other_test.go @@ -17,7 +17,10 @@ func TestStorageDir_UsesValidatedDataDirEnv(t *testing.T) { base, _ = filepath.EvalSymlinks(base) t.Setenv("LARKSUITE_CLI_DATA_DIR", filepath.Join(base, "data", "..", "store")) - got := StorageDir("svc") + got, err := StorageDir("svc") + if err != nil { + t.Fatalf("StorageDir() error = %v", err) + } want := filepath.Join(base, "store", "svc") if got != want { t.Fatalf("StorageDir() = %q, want %q", got, want) @@ -28,12 +31,20 @@ func TestStorageDir_UsesValidatedDataDirEnv(t *testing.T) { // 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) + var err error + home, err = filepath.EvalSymlinks(home) + if err != nil { + t.Fatalf("EvalSymlinks(%q): %v", home, err) + } t.Setenv("LARKSUITE_CLI_DATA_DIR", "relative-data") + t.Setenv("XDG_DATA_HOME", "") t.Setenv("HOME", home) - got := StorageDir("svc") - want := filepath.Join(home, ".local", "share", "svc") + got, err := StorageDir("svc") + if err != nil { + t.Fatalf("StorageDir() error = %v", err) + } + want := filepath.Join(home, ".local", "share", "lark-cli", "svc") if got != want { t.Fatalf("StorageDir() = %q, want %q", got, want) } diff --git a/internal/lockfile/lockfile.go b/internal/lockfile/lockfile.go index d08820088..e960349ce 100644 --- a/internal/lockfile/lockfile.go +++ b/internal/lockfile/lockfile.go @@ -9,7 +9,7 @@ import ( "path/filepath" "regexp" - "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/appdir" "github.com/larksuite/cli/internal/vfs" ) @@ -29,7 +29,7 @@ func New(path string) *LockFile { } // ForSubscribe returns a LockFile scoped to the event subscribe command for a given App ID. -// Lock path: {configDir}/locks/subscribe_{appID}.lock +// Lock path: {stateDir}/locks/subscribe_{appID}.lock // // The appID is sanitized to prevent path traversal: any character outside // [a-zA-Z0-9._-] is replaced with "_", and filepath.Base strips directory @@ -39,8 +39,8 @@ func ForSubscribe(appID string) (*LockFile, error) { if appID == "" { return nil, fmt.Errorf("app ID must not be empty") } - dir := filepath.Join(core.GetConfigDir(), "locks") - if err := vfs.MkdirAll(dir, 0700); err != nil { + dir := filepath.Join(appdir.StateDir(), "locks") + if err := vfs.MkdirAll(dir, 0o700); err != nil { return nil, fmt.Errorf("create lock dir: %w", err) } safe := safeIDChars.ReplaceAllString(appID, "_") @@ -57,7 +57,7 @@ func (l *LockFile) TryLock() error { if l.file != nil { return fmt.Errorf("lock already held: %s", l.path) } - f, err := vfs.OpenFile(l.path, os.O_CREATE|os.O_RDWR, 0600) + f, err := vfs.OpenFile(l.path, os.O_CREATE|os.O_RDWR, 0o600) if err != nil { return fmt.Errorf("open lock file: %w", err) } diff --git a/internal/lockfile/lockfile_test.go b/internal/lockfile/lockfile_test.go index f6d50658e..a3fd810a9 100644 --- a/internal/lockfile/lockfile_test.go +++ b/internal/lockfile/lockfile_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/larksuite/cli/internal/appdir" ) func newTestLock(t *testing.T) *LockFile { @@ -139,19 +141,20 @@ func TestPath(t *testing.T) { func TestForSubscribe(t *testing.T) { dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + t.Setenv("LARKSUITE_CLI_STATE_DIR", dir) + stateDir := appdir.StateDir() l, err := ForSubscribe("cli_test123") if err != nil { t.Fatalf("ForSubscribe failed: %v", err) } - expected := filepath.Join(dir, "locks", "subscribe_cli_test123.lock") + expected := filepath.Join(stateDir, "locks", "subscribe_cli_test123.lock") if l.Path() != expected { t.Errorf("Path() = %q, want %q", l.Path(), expected) } - lockDir := filepath.Join(dir, "locks") + lockDir := filepath.Join(stateDir, "locks") if _, err := os.Stat(lockDir); os.IsNotExist(err) { t.Error("locks directory should be created by ForSubscribe") } @@ -159,7 +162,8 @@ func TestForSubscribe(t *testing.T) { func TestForSubscribe_SanitizesAppID(t *testing.T) { dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + t.Setenv("LARKSUITE_CLI_STATE_DIR", dir) + stateDir := appdir.StateDir() for _, tt := range []struct { name string @@ -181,7 +185,7 @@ func TestForSubscribe_SanitizesAppID(t *testing.T) { t.Errorf("Base(Path()) = %q, want %q", gotBase, tt.wantBase) } // Lock file must always be under the locks directory - locksDir := filepath.Join(dir, "locks") + locksDir := filepath.Join(stateDir, "locks") if !strings.HasPrefix(l.Path(), locksDir) { t.Errorf("path %q escapes locks dir %q", l.Path(), locksDir) } diff --git a/internal/registry/loader.go b/internal/registry/loader.go index a310326d6..275766b28 100644 --- a/internal/registry/loader.go +++ b/internal/registry/loader.go @@ -6,6 +6,7 @@ package registry import ( "embed" "encoding/json" + "io" "math" "path/filepath" "runtime" @@ -32,15 +33,19 @@ var ( // Init initializes the registry with default brand (feishu). // It is safe to call multiple times (sync.Once). func Init() { - InitWithBrand(core.BrandFeishu) + InitWithBrand(core.BrandFeishu, io.Discard) } // InitWithBrand initializes the registry by loading embedded data and optionally // overlaying cached remote data. The brand determines which remote API host to use. +// errOut receives progress/warning output (typically IOStreams.ErrOut). // It is safe to call multiple times (sync.Once). // Remote fetch errors are silently ignored when embedded data is available. // If no embedded data exists and no cache is found, a synchronous fetch is attempted. -func InitWithBrand(brand core.LarkBrand) { +func InitWithBrand(brand core.LarkBrand, errOut io.Writer) { + if errOut == nil { + errOut = io.Discard + } initOnce.Do(func() { configuredBrand = brand // 1. Load embedded meta_data.json as baseline (no-op if not compiled in) @@ -58,7 +63,7 @@ func InitWithBrand(brand core.LarkBrand) { } if len(mergedServices) == 0 || brandChanged { // No data at all or brand changed — must sync fetch - doSyncFetch() + doSyncFetch(errOut) } else if shouldRefresh(meta) || metaErr != nil { // Have embedded/cached data; refresh in background if TTL expired or first run triggerBackgroundRefresh() @@ -183,11 +188,13 @@ func ListFromMetaProjects() []string { // Higher score = more recommended. Unscored scopes get 0 (least preferred). const DefaultScopeScore = 0 -var cachedScopePriorities map[string]int -var cachedAutoApproveSet map[string]bool -var cachedPlatformAutoApprove map[string]bool // from scope_priorities.json only -var cachedOverrideAutoAllow map[string]bool // from scope_overrides.json allow only -var cachedOverrideAutoDeny map[string]bool // from scope_overrides.json deny only +var ( + cachedScopePriorities map[string]int + cachedAutoApproveSet map[string]bool + cachedPlatformAutoApprove map[string]bool // from scope_priorities.json only + cachedOverrideAutoAllow map[string]bool // from scope_overrides.json allow only + cachedOverrideAutoDeny map[string]bool // from scope_overrides.json deny only +) // scopePriorityEntry is used to parse scope_priorities.json entries. type scopePriorityEntry struct { diff --git a/internal/registry/remote.go b/internal/registry/remote.go index 4fcaa357c..b29bdbeff 100644 --- a/internal/registry/remote.go +++ b/internal/registry/remote.go @@ -15,6 +15,7 @@ import ( "sync" "time" + "github.com/larksuite/cli/internal/appdir" "github.com/larksuite/cli/internal/build" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/validate" @@ -95,7 +96,7 @@ func metaTTL() time.Duration { // --- cache path helpers --- func cacheDir() string { - return filepath.Join(core.GetConfigDir(), "cache") + return appdir.CacheDir() } func cachePath() string { @@ -110,11 +111,11 @@ func cacheMetaPath() string { // Returns false if the directory cannot be created or written to. func cacheWritable() bool { dir := cacheDir() - if err := vfs.MkdirAll(dir, 0700); err != nil { + if err := vfs.MkdirAll(dir, 0o700); err != nil { return false } probe := filepath.Join(dir, ".probe") - if err := vfs.WriteFile(probe, []byte{}, 0644); err != nil { + if err := vfs.WriteFile(probe, []byte{}, 0o644); err != nil { return false } vfs.Remove(probe) @@ -136,14 +137,14 @@ func loadCacheMeta() (CacheMeta, error) { } func saveCacheMeta(meta CacheMeta) error { - if err := vfs.MkdirAll(cacheDir(), 0700); err != nil { + if err := vfs.MkdirAll(cacheDir(), 0o700); err != nil { return err } data, err := json.Marshal(meta) if err != nil { return err } - return validate.AtomicWrite(cacheMetaPath(), data, 0644) + return validate.AtomicWrite(cacheMetaPath(), data, 0o644) } func loadCachedMerged() (*MergedRegistry, error) { @@ -163,10 +164,10 @@ func loadCachedMerged() (*MergedRegistry, error) { } func saveCachedMerged(data []byte, meta CacheMeta) error { - if err := vfs.MkdirAll(cacheDir(), 0700); err != nil { + if err := vfs.MkdirAll(cacheDir(), 0o700); err != nil { return err } - if err := validate.AtomicWrite(cachePath(), data, 0644); err != nil { + if err := validate.AtomicWrite(cachePath(), data, 0o644); err != nil { return err } return saveCacheMeta(meta) @@ -233,8 +234,8 @@ func (e *httpError) Error() string { // --- sync fetch (no embedded, no cache) --- // doSyncFetch performs a blocking fetch for first-run without embedded data. -func doSyncFetch() { - fmt.Fprintf(os.Stderr, "Fetching API metadata...\n") +func doSyncFetch(errOut io.Writer) { + fmt.Fprintln(errOut, "Fetching API metadata...") data, reg, err := fetchRemoteMerged(embeddedVersion) if err != nil || reg == nil { // Write meta even on failure so we don't retry every invocation within TTL diff --git a/internal/registry/remote_test.go b/internal/registry/remote_test.go index c8b2f77e8..3a04f40b1 100644 --- a/internal/registry/remote_test.go +++ b/internal/registry/remote_test.go @@ -5,6 +5,7 @@ package registry import ( "encoding/json" + "io" "net/http" "net/http/httptest" "os" @@ -110,6 +111,7 @@ func TestColdStart_NoEmbedded_SyncFetch(t *testing.T) { resetInit() tmp := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmp) + t.Setenv("LARKSUITE_CLI_CACHE_DIR", filepath.Join(tmp, "cache")) t.Setenv("LARKSUITE_CLI_REMOTE_META", "on") ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -130,12 +132,17 @@ func TestRemoteOff_SkipsRemoteLogic(t *testing.T) { resetInit() tmp := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmp) + t.Setenv("LARKSUITE_CLI_CACHE_DIR", filepath.Join(tmp, "cache")) t.Setenv("LARKSUITE_CLI_REMOTE_META", "off") // Create a fake cache that should NOT be loaded cDir := filepath.Join(tmp, "cache") - os.MkdirAll(cDir, 0700) - os.WriteFile(filepath.Join(cDir, "remote_meta.json"), testCacheJSON("fake_remote_svc"), 0644) + if err := os.MkdirAll(cDir, 0o700); err != nil { + t.Fatalf("os.MkdirAll(%q): %v", cDir, err) + } + if err := os.WriteFile(filepath.Join(cDir, "remote_meta.json"), testCacheJSON("fake_remote_svc"), 0o644); err != nil { + t.Fatalf("os.WriteFile(remote_meta.json): %v", err) + } Init() @@ -149,16 +156,23 @@ func TestCacheHit_WithinTTL(t *testing.T) { resetInit() tmp := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmp) + t.Setenv("LARKSUITE_CLI_CACHE_DIR", filepath.Join(tmp, "cache")) t.Setenv("LARKSUITE_CLI_REMOTE_META", "on") t.Setenv("LARKSUITE_CLI_META_TTL", "3600") // Pre-seed cache with a custom service cDir := filepath.Join(tmp, "cache") - os.MkdirAll(cDir, 0700) - os.WriteFile(filepath.Join(cDir, "remote_meta.json"), testCacheJSON("custom_svc"), 0644) + if err := os.MkdirAll(cDir, 0o700); err != nil { + t.Fatalf("os.MkdirAll(%q): %v", cDir, err) + } + if err := os.WriteFile(filepath.Join(cDir, "remote_meta.json"), testCacheJSON("custom_svc"), 0o644); err != nil { + t.Fatalf("os.WriteFile(remote_meta.json): %v", err) + } meta := CacheMeta{LastCheckAt: time.Now().Unix()} metaData, _ := json.Marshal(meta) - os.WriteFile(filepath.Join(cDir, "remote_meta.meta.json"), metaData, 0644) + if err := os.WriteFile(filepath.Join(cDir, "remote_meta.meta.json"), metaData, 0o644); err != nil { + t.Fatalf("os.WriteFile(remote_meta.meta.json): %v", err) + } // Point META_URL to a server that would fail if contacted ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -186,16 +200,23 @@ func TestNetworkError_SilentDegradation(t *testing.T) { resetInit() tmp := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmp) + t.Setenv("LARKSUITE_CLI_CACHE_DIR", filepath.Join(tmp, "cache")) t.Setenv("LARKSUITE_CLI_REMOTE_META", "on") t.Setenv("LARKSUITE_CLI_META_TTL", "0") // Always refresh // Pre-seed cache so we have data to fall back on cDir := filepath.Join(tmp, "cache") - os.MkdirAll(cDir, 0700) - os.WriteFile(filepath.Join(cDir, "remote_meta.json"), testCacheJSON("cached_svc"), 0644) + if err := os.MkdirAll(cDir, 0o700); err != nil { + t.Fatalf("os.MkdirAll(%q): %v", cDir, err) + } + if err := os.WriteFile(filepath.Join(cDir, "remote_meta.json"), testCacheJSON("cached_svc"), 0o644); err != nil { + t.Fatalf("os.WriteFile(remote_meta.json): %v", err) + } meta := CacheMeta{LastCheckAt: time.Now().Add(-2 * time.Hour).Unix()} metaData, _ := json.Marshal(meta) - os.WriteFile(filepath.Join(cDir, "remote_meta.meta.json"), metaData, 0644) + if err := os.WriteFile(filepath.Join(cDir, "remote_meta.meta.json"), metaData, 0o644); err != nil { + t.Fatalf("os.WriteFile(remote_meta.meta.json): %v", err) + } // Use a mock server that returns an error immediately (instead of 127.0.0.1:1 which // may hang up to fetchTimeout=5s, leaking the background goroutine into subsequent tests). @@ -406,14 +427,21 @@ func TestCorruptedCache_SelfHeals(t *testing.T) { resetInit() tmp := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmp) + t.Setenv("LARKSUITE_CLI_CACHE_DIR", filepath.Join(tmp, "cache")) // Write corrupted cache cDir := filepath.Join(tmp, "cache") - os.MkdirAll(cDir, 0700) - os.WriteFile(filepath.Join(cDir, "remote_meta.json"), []byte("not json{{{"), 0644) + if err := os.MkdirAll(cDir, 0o700); err != nil { + t.Fatalf("os.MkdirAll(%q): %v", cDir, err) + } + if err := os.WriteFile(filepath.Join(cDir, "remote_meta.json"), []byte("not json{{{"), 0o644); err != nil { + t.Fatalf("os.WriteFile(remote_meta.json): %v", err) + } meta := CacheMeta{LastCheckAt: time.Now().Unix()} metaData, _ := json.Marshal(meta) - os.WriteFile(filepath.Join(cDir, "remote_meta.meta.json"), metaData, 0644) + if err := os.WriteFile(filepath.Join(cDir, "remote_meta.meta.json"), metaData, 0o644); err != nil { + t.Fatalf("os.WriteFile(remote_meta.meta.json): %v", err) + } // loadCachedMerged should fail and remove the corrupted files _, err := loadCachedMerged() @@ -450,16 +478,23 @@ func TestBrandSwitchInvalidatesCache(t *testing.T) { resetInit() tmp := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmp) + t.Setenv("LARKSUITE_CLI_CACHE_DIR", filepath.Join(tmp, "cache")) t.Setenv("LARKSUITE_CLI_REMOTE_META", "on") t.Setenv("LARKSUITE_CLI_META_TTL", "3600") // Pre-seed cache with feishu brand cDir := filepath.Join(tmp, "cache") - os.MkdirAll(cDir, 0700) - os.WriteFile(filepath.Join(cDir, "remote_meta.json"), testCacheJSON("feishu_svc"), 0644) + if err := os.MkdirAll(cDir, 0o700); err != nil { + t.Fatalf("os.MkdirAll(%q): %v", cDir, err) + } + if err := os.WriteFile(filepath.Join(cDir, "remote_meta.json"), testCacheJSON("feishu_svc"), 0o644); err != nil { + t.Fatalf("os.WriteFile(remote_meta.json): %v", err) + } meta := CacheMeta{LastCheckAt: time.Now().Unix(), Version: "test-1.0", Brand: "feishu"} metaData, _ := json.Marshal(meta) - os.WriteFile(filepath.Join(cDir, "remote_meta.meta.json"), metaData, 0644) + if err := os.WriteFile(filepath.Join(cDir, "remote_meta.meta.json"), metaData, 0o644); err != nil { + t.Fatalf("os.WriteFile(remote_meta.meta.json): %v", err) + } // Server returns lark-specific data ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -470,7 +505,7 @@ func TestBrandSwitchInvalidatesCache(t *testing.T) { testMetaURL = ts.URL // Init with lark brand — should invalidate feishu cache and sync fetch - InitWithBrand(core.BrandLark) + InitWithBrand(core.BrandLark, io.Discard) // The old feishu_svc should NOT be loaded from stale cache // The new lark_svc from sync fetch should be available diff --git a/internal/update/update.go b/internal/update/update.go index c87bb59fc..2b33b66ee 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -16,7 +16,7 @@ import ( "sync/atomic" "time" - "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/appdir" "github.com/larksuite/cli/internal/util" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/internal/vfs" @@ -144,7 +144,7 @@ func isRelease(version string) bool { // --- state file I/O --- func statePath() string { - return filepath.Join(core.GetConfigDir(), stateFile) + return filepath.Join(appdir.StateDir(), stateFile) } func loadState() (*updateState, error) { @@ -160,15 +160,16 @@ func loadState() (*updateState, error) { } func saveState(s *updateState) error { - dir := core.GetConfigDir() - if err := vfs.MkdirAll(dir, 0700); err != nil { + dir := appdir.StateDir() + path := filepath.Join(dir, stateFile) + if err := vfs.MkdirAll(dir, 0o700); err != nil { return err } data, err := json.Marshal(s) if err != nil { return err } - return validate.AtomicWrite(statePath(), data, 0644) + return validate.AtomicWrite(path, data, 0o644) } // FetchLatest queries the npm registry and returns the latest published version. diff --git a/internal/update/update_test.go b/internal/update/update_test.go index 5ff356225..c8e9ac4c3 100644 --- a/internal/update/update_test.go +++ b/internal/update/update_test.go @@ -185,7 +185,7 @@ func TestUpdateInfoMethods(t *testing.T) { func TestCheckCached(t *testing.T) { clearSkipEnv(t) tmp := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmp) + t.Setenv("LARKSUITE_CLI_STATE_DIR", tmp) // No cache → nil info := CheckCached("1.0.0") @@ -196,7 +196,9 @@ func TestCheckCached(t *testing.T) { // Write cache with newer version state := &updateState{LatestVersion: "2.0.0", CheckedAt: time.Now().Unix()} data, _ := json.Marshal(state) - os.WriteFile(filepath.Join(tmp, stateFile), data, 0644) + if err := os.WriteFile(filepath.Join(tmp, stateFile), data, 0o644); err != nil { + t.Fatalf("WriteFile(%q): %v", filepath.Join(tmp, stateFile), err) + } info = CheckCached("1.0.0") if info == nil { @@ -216,7 +218,7 @@ func TestCheckCached(t *testing.T) { func TestRefreshCache(t *testing.T) { clearSkipEnv(t) tmp := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmp) + t.Setenv("LARKSUITE_CLI_STATE_DIR", tmp) // Set up mock npm registry via DefaultClient srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/validate/path.go b/internal/validate/path.go index 59d21c2c0..d2cb1cdc4 100644 --- a/internal/validate/path.go +++ b/internal/validate/path.go @@ -28,3 +28,9 @@ func SafeEnvDirPath(path, envName string) (string, error) { func SafeLocalFlagPath(flagName, value string) (string, error) { return localfileio.SafeLocalFlagPath(flagName, value) } + +// SafeServiceName validates that a service name is a single, safe path +// component. Delegates to localfileio.SafeServiceName. +func SafeServiceName(name string) error { + return localfileio.SafeServiceName(name) +} diff --git a/internal/vfs/localfileio/path.go b/internal/vfs/localfileio/path.go index 7a4b52ab8..080de4f00 100644 --- a/internal/vfs/localfileio/path.go +++ b/internal/vfs/localfileio/path.go @@ -122,6 +122,28 @@ func isUnderDir(child, parent string) bool { return !strings.HasPrefix(rel, ".."+string(filepath.Separator)) && rel != ".." } +// SafeServiceName validates that a service name is a single, safe path +// component. It rejects empty names, path traversal sequences, path +// separators, and control/dangerous Unicode characters. +func SafeServiceName(name string) error { + if name == "" { + return fmt.Errorf("service name must not be empty") + } + if name == "." || name == ".." { + return fmt.Errorf("service name %q is a path traversal sequence", name) + } + if strings.ContainsAny(name, `/\`) { + return fmt.Errorf("service name %q must not contain path separators", name) + } + if strings.ContainsAny(name, "\t\n\r") { + return fmt.Errorf("service name must not contain tabs or newlines") + } + if err := charcheck.RejectControlChars(name, "service name"); err != nil { + return err + } + return nil +} + // RejectControlChars delegates to charcheck.RejectControlChars. // Kept as a package-level alias for backward compatibility with callers // that import localfileio directly. diff --git a/internal/vfs/localfileio/path_test.go b/internal/vfs/localfileio/path_test.go index 946d1be1c..ed4cf46d7 100644 --- a/internal/vfs/localfileio/path_test.go +++ b/internal/vfs/localfileio/path_test.go @@ -73,7 +73,6 @@ func TestSafeOutputPath_ReturnsCanonicalAbsolutePath(t *testing.T) { // WHEN: SafeOutputPath validates a relative path got, err := SafeOutputPath("output/file.txt") - // THEN: returns the canonical absolute path for subsequent I/O if err != nil { t.Fatalf("unexpected error: %v", err) @@ -109,12 +108,13 @@ func TestSafeOutputPath_AllowsSymlinkWithinCWD(t *testing.T) { origDir, _ := os.Getwd() defer os.Chdir(origDir) os.Chdir(dir) - os.MkdirAll(filepath.Join(dir, "real"), 0755) + if err := os.MkdirAll(filepath.Join(dir, "real"), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } os.Symlink(filepath.Join(dir, "real"), filepath.Join(dir, "link")) // WHEN: SafeOutputPath validates a path through the internal symlink got, err := SafeOutputPath("link/file.txt") - // THEN: allowed, resolved to the real path within CWD if err != nil { t.Fatalf("symlink within CWD should be allowed: %v", err) @@ -156,7 +156,6 @@ func TestSafeOutputPath_DeepNonExistentPathStaysInCWD(t *testing.T) { // WHEN: SafeOutputPath validates "a/b/c/d/file.txt" (none of a/b/c/d exist) got, err := SafeOutputPath("a/b/c/d/file.txt") - // THEN: allowed, resolved to canonical path under CWD if err != nil { t.Fatalf("deep non-existent path within CWD should be allowed: %v", err) @@ -203,11 +202,12 @@ func TestSafeUploadPath_AcceptsRelativePath(t *testing.T) { defer os.Chdir(orig) os.Chdir(dir) - os.WriteFile(filepath.Join(dir, "upload.bin"), []byte("data"), 0600) + if err := os.WriteFile(filepath.Join(dir, "upload.bin"), []byte("data"), 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } // WHEN: SafeUploadPath validates a relative path to an existing file got, err := SafeInputPath("upload.bin") - // THEN: accepted and returned as absolute canonical path if err != nil { t.Fatalf("SafeUploadPath(relative) error = %v", err) @@ -218,6 +218,40 @@ func TestSafeUploadPath_AcceptsRelativePath(t *testing.T) { } } +func TestSafeServiceName(t *testing.T) { + for _, tt := range []struct { + name string + input string + wantErr bool + }{ + {"valid simple name", "my-service", false}, + {"valid with dots", "com.example.svc", false}, + {"valid with underscores", "my_service", false}, + {"valid alphanumeric", "svc123", false}, + + {"empty", "", true}, + {"dot only", ".", true}, + {"dot-dot", "..", true}, + {"forward slash", "foo/bar", true}, + {"backslash", `foo\bar`, true}, + {"traversal with slash", "../etc", true}, + {"null byte", "svc\x00name", true}, + {"tab", "svc\tname", true}, + {"newline", "svc\nname", true}, + {"carriage return", "svc\rname", true}, + {"control char", "svc\x07name", true}, + {"bidi override", "svc\u202Ename", true}, + {"zero width space", "svc\u200Bname", true}, + } { + t.Run(tt.name, func(t *testing.T) { + err := SafeServiceName(tt.input) + if (err != nil) != tt.wantErr { + t.Errorf("SafeServiceName(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr) + } + }) + } +} + func Test_SafeInputPath_ErrorMessageContainsCorrectFlagName(t *testing.T) { // GIVEN: an absolute path