diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 529b3434..9c509570 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -119,8 +119,8 @@ jobs: - name: Test Windows auth and DPAPI packages run: go test -v -count=1 -timeout=10m ./internal/keychain ./internal/auth - - name: Test Windows auth migration diagnostics - run: go test -v -count=1 -timeout=5m ./internal/app -run '^TestAuth(MigrateKeychain|StatusDiagnosticReportsCiphertextKeyMismatch)' + - name: Test Windows auth migration and portable auth diagnostics + run: go test -v -count=1 -timeout=5m ./internal/app -run '^Test(CrossPlatformCoverage)?Auth(MigrateKeychain|StatusDiagnosticReportsCiphertextKeyMismatch|ExportRejectsWindowsDPAPIBackend|ImportRejectsWindowsDPAPIBackend)' coverage-darwin: name: Coverage (macOS) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8392b622..8540d78c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ This beta validates the accumulated post-v1.0.52 command surface, release automa - **PAT organization-policy denials stop immediately** — `PAT_ORG_POLICY_DENIED` now remains terminal even if a backend also returns `flowId`, authorization URLs, or client credentials; the CLI does not mutate process credentials, open a browser, poll, or retry until an organization administrator changes the policy. - **Sheet and task invalid-target failures** — `sheet range read/get` now rejects a null cell-info response instead of printing `null` and exiting successfully, while task completion and attachment listing verify that a task exists before calling lenient backend endpoints. Attachment listing is also published through Runtime Schema for schema-first Agent discovery. +- **Windows portable-auth contract** — `dws auth export` and `dws auth import` now fail early without reading credentials, bundles, or writing files instead of claiming portable-bundle support for DPAPI-protected HKCU Registry credentials. - **Concurrent credential writes and reentrant CLI execution** — secure-token writers now use isolated, exclusive temporary files before atomic replacement so concurrent processes cannot remove each other's in-flight data, and repeated in-process CLI runs close the previous file logger before replacing it instead of retaining the prior log-file handle. ## [1.0.52] - 2026-07-14 diff --git a/README.md b/README.md index 024932e6..0380025f 100644 --- a/README.md +++ b/README.md @@ -323,6 +323,9 @@ dws auth status # confirm "Refresh Token: valid" ``` The bundle includes the encrypted keychain under `~/.local/share/dws-cli` (with `auth-token.enc` and `dek`) plus required `~/.dws` config files. +Windows export and import are intentionally rejected before credentials or +bundles are read: Windows stores credentials as DPAPI-protected HKCU Registry +values, and the current file-DEK bundle has no safe DPAPI-to-portable conversion. diff --git a/internal/app/auth_command.go b/internal/app/auth_command.go index d96d60a5..4ec04025 100644 --- a/internal/app/auth_command.go +++ b/internal/app/auth_command.go @@ -666,6 +666,10 @@ func pushRuntimeProfile(selector string) func() { } func newAuthExportCommand() *cobra.Command { + return newAuthExportCommandWithSupport(authpkg.PortableExportSupportError) +} + +func newAuthExportCommandWithSupport(supportError func() error) *cobra.Command { cmd := &cobra.Command{ Use: "export", Short: "导出可迁移认证包", @@ -690,6 +694,9 @@ func newAuthExportCommand() *cobra.Command { if !asBase64 && output == "" { return apperrors.NewValidation("--output is required unless --base64 is used") } + if err := supportError(); err != nil { + return apperrors.NewValidation(err.Error()) + } if !authPortableExportSupported() { return apperrors.NewValidation(fmt.Sprintf( "macOS 导出认证包需要 file-DEK 模式；请先设置 %s=1 并运行 dws auth status 验证，只有提示密钥不匹配且确认可丢弃旧登录态时，才执行 dws auth reset 后重新登录", @@ -733,6 +740,10 @@ func newAuthExportCommand() *cobra.Command { } func newAuthImportCommand() *cobra.Command { + return newAuthImportCommandWithSupport(authpkg.PortableImportSupportError) +} + +func newAuthImportCommandWithSupport(supportError func() error) *cobra.Command { cmd := &cobra.Command{ Use: "import", Short: "导入可迁移认证包", @@ -761,7 +772,9 @@ func newAuthImportCommand() *cobra.Command { if err != nil { return apperrors.NewInternal("failed to read --force") } - + if err := supportError(); err != nil { + return apperrors.NewValidation(err.Error()) + } configDir := defaultConfigDir() if !force && authPortableTargetPopulated(configDir) { return apperrors.NewValidation("检测到已有登录态，请使用 --force 确认覆盖") diff --git a/internal/app/auth_command_full_coverage_test.go b/internal/app/auth_command_full_coverage_test.go index 497dc38c..c6143589 100644 --- a/internal/app/auth_command_full_coverage_test.go +++ b/internal/app/auth_command_full_coverage_test.go @@ -618,7 +618,7 @@ func TestCrossPlatformCoverageAuthCoveragePortableExchangeAndReset(t *testing.T) authDeleteAppConfig = oldDeleteConfig }) - export := newAuthExportCommand() + export := newAuthExportCommandWithSupport(func() error { return nil }) badString := &cobra.Command{} badString.Flags().Bool("output", false, "") if err := export.RunE(badString, nil); err == nil { @@ -658,7 +658,7 @@ func TestCrossPlatformCoverageAuthCoveragePortableExchangeAndReset(t *testing.T) if err := export.RunE(export, nil); err != nil { t.Fatal(err) } - export = newAuthExportCommand() + export = newAuthExportCommandWithSupport(func() error { return nil }) _, out, _ := authCoverageRoot(export, "table", false) _ = export.Flags().Set("base64", "true") export.SetOut(&appFailWriter{err: errors.New("stdout")}) @@ -676,7 +676,7 @@ func TestCrossPlatformCoverageAuthCoveragePortableExchangeAndReset(t *testing.T) t.Fatal(err) } - importCmd := newAuthImportCommand() + importCmd := newAuthImportCommandWithSupport(func() error { return nil }) badInput := &cobra.Command{} badInput.Flags().Bool("input", false, "") if err := importCmd.RunE(badInput, nil); err == nil { diff --git a/internal/app/auth_command_test.go b/internal/app/auth_command_test.go index 49fb18b9..0dbac02e 100644 --- a/internal/app/auth_command_test.go +++ b/internal/app/auth_command_test.go @@ -23,6 +23,7 @@ import ( "net/http" "os" "path/filepath" + "runtime" "strings" "testing" "time" @@ -58,7 +59,7 @@ func TestAuthExportImportBase64RoundTrip(t *testing.T) { return err } - exportCmd := newAuthExportCommand() + exportCmd := newAuthExportCommandWithSupport(func() error { return nil }) var exported bytes.Buffer exportCmd.SetOut(&exported) exportCmd.SetErr(&bytes.Buffer{}) @@ -84,7 +85,7 @@ func TestAuthExportImportBase64RoundTrip(t *testing.T) { return authpkg.PortableImportReport{}, err } - importCmd := newAuthImportCommand() + importCmd := newAuthImportCommandWithSupport(func() error { return nil }) importCmd.SetOut(&bytes.Buffer{}) importCmd.SetErr(&bytes.Buffer{}) importCmd.SetArgs([]string{"--input", inputPath, "--base64"}) @@ -96,6 +97,260 @@ func TestAuthExportImportBase64RoundTrip(t *testing.T) { } } +func TestCrossPlatformCoverageAuthExportUnsupportedBackendIsValidationError(t *testing.T) { + exportCmd := newAuthExportCommandWithSupport(func() error { + return errors.New("portable auth export is unavailable for the test backend") + }) + exportCmd.SetOut(&bytes.Buffer{}) + exportCmd.SetErr(&bytes.Buffer{}) + exportCmd.SetArgs([]string{"--base64"}) + + err := exportCmd.Execute() + if err == nil { + t.Fatal("auth export should reject an unsupported credential backend") + } + var appErr *apperrors.Error + if !errors.As(err, &appErr) || appErr.Category != apperrors.CategoryValidation { + t.Fatalf("expected validation error, got %T: %v", err, err) + } + if !strings.Contains(err.Error(), "test backend") { + t.Fatalf("error = %v, want backend-specific reason", err) + } +} + +func TestCrossPlatformCoverageAuthExportRejectsWindowsDPAPIBackend(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Windows DPAPI contract requires a native Windows runner") + } + t.Cleanup(CloseFileLogger) + + exportCmd := NewRootCommand() + exportCmd.SetOut(&bytes.Buffer{}) + exportCmd.SetErr(&bytes.Buffer{}) + exportCmd.SetArgs([]string{"auth", "export", "--base64"}) + + err := exportCmd.Execute() + if err == nil { + t.Fatal("auth export should reject the Windows DPAPI backend") + } + var appErr *apperrors.Error + if !errors.As(err, &appErr) || appErr.Category != apperrors.CategoryValidation { + t.Fatalf("expected validation error, got %T: %v", err, err) + } + for _, want := range []string{"Windows", "DPAPI", "HKCU"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("error = %v, want substring %q", err, want) + } + } +} + +func TestCrossPlatformCoverageAuthImportUnsupportedBackendIsValidationErrorBeforeReadingInput(t *testing.T) { + root := t.TempDir() + configDir := filepath.Join(root, ".dws") + keychainDir := filepath.Join(root, "keychain") + t.Setenv("DWS_CONFIG_DIR", configDir) + t.Setenv(keychain.StorageDirEnv, keychainDir) + + importCmd := newAuthImportCommandWithSupport(func() error { + return errors.New("portable auth import is unavailable for the test backend") + }) + importCmd.SetOut(&bytes.Buffer{}) + importCmd.SetErr(&bytes.Buffer{}) + importCmd.SetArgs([]string{"--input", filepath.Join(root, "missing-bundle.tar.gz")}) + + err := importCmd.Execute() + if err == nil { + t.Fatal("auth import should reject an unsupported credential backend") + } + var appErr *apperrors.Error + if !errors.As(err, &appErr) || appErr.Category != apperrors.CategoryValidation { + t.Fatalf("expected validation error, got %T: %v", err, err) + } + if !strings.Contains(err.Error(), "test backend") { + t.Fatalf("error = %v, want backend-specific reason", err) + } + for _, path := range []string{configDir, keychainDir} { + if _, statErr := os.Stat(path); !os.IsNotExist(statErr) { + t.Fatalf("unsupported import touched %s: stat error = %v", path, statErr) + } + } +} + +func TestCrossPlatformCoverageAuthImportRejectsWindowsDPAPIBackend(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Windows DPAPI contract requires a native Windows runner") + } + + root := t.TempDir() + // NewRootCommand initializes the normal CLI file logger below configDir. + // Register its cleanup after TempDir so the Windows handle is closed before + // testing removes the temporary directory. + t.Cleanup(CloseFileLogger) + configDir := filepath.Join(root, ".dws") + keychainDir := filepath.Join(root, "keychain") + inputPath := filepath.Join(root, "bundle.tar.gz") + if err := os.WriteFile(inputPath, []byte("the capability guard must run before this input is read"), 0o600); err != nil { + t.Fatalf("write input sentinel error = %v", err) + } + t.Setenv("DWS_CONFIG_DIR", configDir) + t.Setenv(keychain.StorageDirEnv, keychainDir) + + importCmd := NewRootCommand() + importCmd.SetOut(&bytes.Buffer{}) + importCmd.SetErr(&bytes.Buffer{}) + importCmd.SetArgs([]string{"auth", "import", "--input", inputPath}) + + err := importCmd.Execute() + if err == nil { + t.Fatal("auth import should reject the Windows DPAPI backend") + } + var appErr *apperrors.Error + if !errors.As(err, &appErr) || appErr.Category != apperrors.CategoryValidation { + t.Fatalf("expected validation error, got %T: %v", err, err) + } + for _, want := range []string{"Windows", "DPAPI", "HKCU"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("error = %v, want substring %q", err, want) + } + } + // The root command may create configDir/logs as part of normal CLI startup. + // The capability guard must still run before any auth state is imported. + for _, path := range []string{ + keychainDir, + authpkg.ProfilesPath(configDir), + filepath.Join(configDir, "app.json"), + filepath.Join(configDir, "token.json"), + } { + if _, statErr := os.Stat(path); !os.IsNotExist(statErr) { + t.Fatalf("unsupported Windows import touched %s: stat error = %v", path, statErr) + } + } +} + +func TestCrossPlatformCoverageAuthImportRejectsWindowsDPAPIBackendWithPopulatedCredentialBeforeRead(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Windows DPAPI contract requires a native Windows runner") + } + + previous, previousErr := authpkg.LoadTokenDataKeychain() + if previousErr != nil && !errors.Is(previousErr, authpkg.ErrTokenDataNotFound) { + t.Fatalf("capture existing Windows credential: %v", previousErr) + } + hadPrevious := previousErr == nil + t.Cleanup(func() { + if hadPrevious { + _ = authpkg.SaveTokenDataKeychain(previous) + } else { + _ = authpkg.DeleteTokenDataKeychain() + } + }) + + want := &authpkg.TokenData{ + AccessToken: "windows-existing-access", + RefreshToken: "windows-existing-refresh", + RefreshExpAt: time.Now().Add(24 * time.Hour), + CorpID: "windows-existing-corp", + } + if err := authpkg.SaveTokenDataKeychain(want); err != nil { + t.Fatalf("seed cleanup-scoped Windows DPAPI credential: %v", err) + } + + originalTarget := authPortableTargetPopulated + originalRead := authReadFile + targetChecks := 0 + bundleReads := 0 + authPortableTargetPopulated = func(configDir string) bool { + targetChecks++ + return originalTarget(configDir) + } + authReadFile = func(path string) ([]byte, error) { + bundleReads++ + return originalRead(path) + } + t.Cleanup(func() { + authPortableTargetPopulated = originalTarget + authReadFile = originalRead + }) + + root := t.TempDir() + inputPath := filepath.Join(root, "bundle.tar.gz") + if err := os.WriteFile(inputPath, []byte("unsupported Windows import must not read this bundle"), 0o600); err != nil { + t.Fatalf("write bundle sentinel: %v", err) + } + t.Setenv("DWS_CONFIG_DIR", filepath.Join(root, ".dws")) + + importCmd := newAuthImportCommand() + importCmd.SetOut(&bytes.Buffer{}) + importCmd.SetErr(&bytes.Buffer{}) + importCmd.SetArgs([]string{"--input", inputPath}) + err := importCmd.Execute() + if err == nil { + t.Fatal("auth import should reject a populated Windows DPAPI backend") + } + var appErr *apperrors.Error + if !errors.As(err, &appErr) || appErr.Category != apperrors.CategoryValidation { + t.Fatalf("expected validation error, got %T: %v", err, err) + } + for _, required := range []string{"Windows", "DPAPI", "HKCU"} { + if !strings.Contains(err.Error(), required) { + t.Fatalf("error = %v, want substring %q", err, required) + } + } + if strings.Contains(err.Error(), "--force") { + t.Fatalf("unsupported Windows import suggested impossible --force remediation: %v", err) + } + if targetChecks != 0 || bundleReads != 0 { + t.Fatalf("unsupported Windows import inspected credentials/bundle: target_checks=%d bundle_reads=%d", targetChecks, bundleReads) + } + + got, err := authpkg.LoadTokenDataKeychain() + if err != nil { + t.Fatalf("reload Windows DPAPI credential after rejection: %v", err) + } + if got.AccessToken != want.AccessToken || got.RefreshToken != want.RefreshToken || got.CorpID != want.CorpID { + t.Fatalf("Windows auth state changed after rejected import: got=%#v want=%#v", got, want) + } +} + +func TestCrossPlatformCoverageAuthImportRequiresForceWhenPopulated(t *testing.T) { + t.Setenv(keychain.DisableKeychainEnv, "1") + root := t.TempDir() + t.Cleanup(CloseFileLogger) + configDir := filepath.Join(root, ".dws") + t.Setenv(keychain.StorageDirEnv, filepath.Join(root, "keychain")) + t.Setenv("DWS_CONFIG_DIR", configDir) + + if err := authpkg.SaveTokenData(configDir, &authpkg.TokenData{ + AccessToken: "existing", + RefreshToken: "existing-refresh", + RefreshExpAt: time.Now().Add(24 * time.Hour), + }); err != nil { + t.Fatalf("SaveTokenData() error = %v", err) + } + + bundlePath := filepath.Join(root, "bundle.tar.gz") + if err := os.WriteFile(bundlePath, []byte("not-a-real-bundle"), 0o600); err != nil { + t.Fatalf("write bundle stub error = %v", err) + } + + importCmd := newAuthImportCommandWithSupport(func() error { return nil }) + var stderr bytes.Buffer + importCmd.SetOut(&bytes.Buffer{}) + importCmd.SetErr(&stderr) + importCmd.SetArgs([]string{"--input", bundlePath}) + err := importCmd.Execute() + if err == nil { + t.Fatal("auth import without --force should fail when auth exists") + } + var appErr *apperrors.Error + if !errors.As(err, &appErr) || appErr.Category != apperrors.CategoryValidation { + t.Fatalf("expected validation error, got %T: %v", err, err) + } + if !strings.Contains(err.Error(), "--force") { + t.Fatalf("error = %v, want --force hint", err) + } +} + func TestAuthImportRequiresForceWhenPopulated(t *testing.T) { originalTarget := authPortableTargetPopulated authPortableTargetPopulated = func(string) bool { return true } @@ -110,10 +365,9 @@ func TestAuthImportRequiresForceWhenPopulated(t *testing.T) { t.Fatalf("write bundle stub error = %v", err) } - importCmd := newAuthImportCommand() - var stderr bytes.Buffer + importCmd := newAuthImportCommandWithSupport(func() error { return nil }) importCmd.SetOut(&bytes.Buffer{}) - importCmd.SetErr(&stderr) + importCmd.SetErr(&bytes.Buffer{}) importCmd.SetArgs([]string{"--input", bundlePath}) err := importCmd.Execute() if err == nil { diff --git a/internal/app/root.go b/internal/app/root.go index d5f11139..ef99c21b 100644 --- a/internal/app/root.go +++ b/internal/app/root.go @@ -24,6 +24,7 @@ import ( "os/signal" "path/filepath" "strings" + "sync" "syscall" "time" @@ -748,7 +749,10 @@ func validateOptionalPath(flagName, path string) error { // fileLogger holds the package-level file logger for diagnostics. // It is initialized by configureLogLevel and closed by CloseFileLogger. -var fileLogger *logging.FileLogger +var ( + fileLoggerMu sync.Mutex + fileLogger *logging.FileLogger +) // configureLogLevel sets the global slog level based on --debug and --verbose flags // and initializes the file logger for diagnostics. @@ -770,18 +774,24 @@ func configureLogLevel(flags *GlobalFlags) { // Initialize file logger — writes to ~/.dws/logs/dws.log at DEBUG level // regardless of stderr level. All slog calls are captured for diagnostics. - // A previously closed rotating writer may have been reopened by slog calls - // made while constructing another root command, so close it before replacing - // the package pointer during reentrant executions. - CloseFileLogger() - fileLogger = logging.Setup(defaultConfigDir()) - fileHandler := slog.NewJSONHandler(fileLogger.Writer(), &slog.HandlerOptions{Level: slog.LevelDebug}) + logger := logging.Setup(defaultConfigDir()) + fileHandler := slog.NewJSONHandler(logger.Writer(), &slog.HandlerOptions{Level: slog.LevelDebug}) + defaultLogger := slog.New(logging.NewMultiHandler(stderrHandler, fileHandler)) - slog.SetDefault(slog.New(logging.NewMultiHandler(stderrHandler, fileHandler))) + fileLoggerMu.Lock() + defer fileLoggerMu.Unlock() + previous := fileLogger + fileLogger = logger + slog.SetDefault(defaultLogger) + if previous != nil { + _ = previous.Close() + } } // FileLoggerInstance returns the package-level file logger, or nil if not initialized. func FileLoggerInstance() *slog.Logger { + fileLoggerMu.Lock() + defer fileLoggerMu.Unlock() if fileLogger == nil { return nil } @@ -790,8 +800,11 @@ func FileLoggerInstance() *slog.Logger { // CloseFileLogger flushes and closes the file logger. func CloseFileLogger() { + fileLoggerMu.Lock() + defer fileLoggerMu.Unlock() if fileLogger != nil { - fileLogger.Close() + _ = fileLogger.Close() + fileLogger = nil } } diff --git a/internal/app/schema_agent_examples_dry_run_test.go b/internal/app/schema_agent_examples_dry_run_test.go index b3284eb6..fa4eca28 100644 --- a/internal/app/schema_agent_examples_dry_run_test.go +++ b/internal/app/schema_agent_examples_dry_run_test.go @@ -67,7 +67,7 @@ func TestManualAgentExamplesDryRun(t *testing.T) { t.Fatalf("create isolated test directory %s: %v", dir, err) } } - t.Setenv("HOME", homeDir) + setTestHome(t, homeDir) t.Setenv("DWS_CONFIG_DIR", configDir) t.Setenv("HTTP_PROXY", "http://127.0.0.1:1") t.Setenv("HTTPS_PROXY", "http://127.0.0.1:1") @@ -470,7 +470,7 @@ func TestManualAgentExamplePromptObservedRejectsInteractiveConfirmation(t *testi } func TestAitableAdvpermDisableDryRunSkipsConfirmationAndToolCall(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + setTestHome(t, t.TempDir()) t.Setenv("DWS_CONFIG_DIR", t.TempDir()) args := []string{ @@ -518,7 +518,7 @@ func TestManualAgentExampleChatGroupMuteMemberUsesCommandDryRunPreview(t *testin if err := os.MkdirAll(configDir, 0o700); err != nil { t.Fatalf("create isolated config directory: %v", err) } - t.Setenv("HOME", sandboxRoot) + setTestHome(t, sandboxRoot) t.Setenv("DWS_CONFIG_DIR", configDir) capture, err := executeManualAgentExampleCapture(t, []string{ diff --git a/internal/app/skill_command_test.go b/internal/app/skill_command_test.go index efd294f4..7caa94b3 100644 --- a/internal/app/skill_command_test.go +++ b/internal/app/skill_command_test.go @@ -369,6 +369,7 @@ func TestSkillInstallCommandValidation(t *testing.T) { func TestSkillInstallInvalidTarget(t *testing.T) { tempDir := t.TempDir() + t.Cleanup(CloseFileLogger) configDir := filepath.Join(tempDir, "config") t.Setenv("DWS_CONFIG_DIR", configDir) t.Cleanup(CloseFileLogger) @@ -392,6 +393,7 @@ func TestSkillInstallInvalidTarget(t *testing.T) { func TestSkillInstallRequiresAuth(t *testing.T) { // Setup: Create config directory without token tempDir := t.TempDir() + t.Cleanup(CloseFileLogger) configDir := filepath.Join(tempDir, "config") t.Setenv("DWS_CONFIG_DIR", configDir) t.Cleanup(CloseFileLogger) diff --git a/internal/app/skill_setup_test.go b/internal/app/skill_setup_test.go index 5a183563..aa8a6ed8 100644 --- a/internal/app/skill_setup_test.go +++ b/internal/app/skill_setup_test.go @@ -88,7 +88,7 @@ func TestResolveSkillSetupSourceErrorWhenMissing(t *testing.T) { // Isolate HOME so the ~/.dws/skills// fallback (added by the release // pipeline cache work) does not pick up real cached content on the // developer machine. - t.Setenv("HOME", t.TempDir()) + setTestHome(t, t.TempDir()) _, err := resolveSkillSetupSource(tmp, skillSetupModeMono) if err == nil { t.Fatalf("expected error when source missing") @@ -110,8 +110,9 @@ func TestResolveSkillSetupTargetsSingleAgent(t *testing.T) { if len(got) != 1 { t.Fatalf("expected 1 dest, got %d", len(got)) } - if !strings.Contains(got[0], filepath.Join(".claude", "skills", "dws")) { - t.Fatalf("expected .claude/skills/dws path, got %s", got[0]) + want := filepath.Join(home, ".claude", "skills", "dws") + if filepath.Clean(got[0]) != filepath.Clean(want) { + t.Fatalf("expected %s, got %s", want, got[0]) } } @@ -133,11 +134,9 @@ func TestResolveSkillSetupTargetsMultiOmitsDwsTail(t *testing.T) { if len(got) != 1 { t.Fatalf("expected 1 dest, got %d", len(got)) } - if filepath.Base(got[0]) == "dws" { - t.Fatalf("multi target must not end with /dws, got %s", got[0]) - } - if !strings.HasSuffix(got[0], filepath.Join(".claude", "skills")) { - t.Fatalf("expected suffix .claude/skills, got %s", got[0]) + want := filepath.Join(home, ".claude", "skills") + if filepath.Clean(got[0]) != filepath.Clean(want) { + t.Fatalf("expected %s, got %s", want, got[0]) } } diff --git a/internal/app/test_home_test.go b/internal/app/test_home_test.go new file mode 100644 index 00000000..51a534e7 --- /dev/null +++ b/internal/app/test_home_test.go @@ -0,0 +1,65 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package app + +import ( + "os" + "path/filepath" + "runtime" + "testing" +) + +func setTestHome(t *testing.T, home string) { + t.Helper() + t.Setenv("HOME", home) + if runtime.GOOS == "windows" { + // os.UserHomeDir uses USERPROFILE on Windows rather than HOME. + t.Setenv("USERPROFILE", home) + } +} + +func TestCrossPlatformCoverageConfigureLogLevelTerminatesReplacedFileLoggers(t *testing.T) { + firstConfig := t.TempDir() + secondConfig := t.TempDir() + t.Cleanup(CloseFileLogger) + + t.Setenv("DWS_CONFIG_DIR", firstConfig) + configureLogLevel(&GlobalFlags{}) + firstLogger := FileLoggerInstance() + t.Setenv("DWS_CONFIG_DIR", secondConfig) + configureLogLevel(&GlobalFlags{}) + + firstLogPath := filepath.Join(firstConfig, "logs", "dws.log") + if err := os.Remove(firstLogPath); err != nil { + t.Fatalf("remove previous logger file: %v", err) + } + firstLogger.Info("late write to replaced logger") + if _, err := os.Stat(firstLogPath); !os.IsNotExist(err) { + t.Fatalf("replaced logger recreated its log file: %v", err) + } + + previousSameDirLogger := FileLoggerInstance() + configureLogLevel(&GlobalFlags{}) + currentLogger := FileLoggerInstance() + CloseFileLogger() + secondLogPath := filepath.Join(secondConfig, "logs", "dws.log") + if err := os.Remove(secondLogPath); err != nil { + t.Fatalf("remove current logger file: %v", err) + } + previousSameDirLogger.Info("late write to same-directory replaced logger") + currentLogger.Info("late write to closed current logger") + if _, err := os.Stat(secondLogPath); !os.IsNotExist(err) { + t.Fatalf("closed logger recreated same-directory log file: %v", err) + } +} diff --git a/internal/app/testmain_test.go b/internal/app/testmain_test.go index 76d74c17..77b212ac 100644 --- a/internal/app/testmain_test.go +++ b/internal/app/testmain_test.go @@ -14,9 +14,12 @@ package app import ( + "fmt" "os" + "path/filepath" "testing" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/audit" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/keychain" ) @@ -49,8 +52,38 @@ func TestMain(m *testing.M) { _ = os.RemoveAll(tmpDir) panic("set " + keychain.StorageDirEnv + ": " + err.Error()) } + if err := os.Setenv("DWS_CONFIG_DIR", filepath.Join(tmpDir, "config")); err != nil { + _ = os.RemoveAll(tmpDir) + panic("set DWS_CONFIG_DIR: " + err.Error()) + } + for key, value := range map[string]string{ + audit.EnvAudit: "1", + audit.EnvAuditDir: filepath.Join(tmpDir, "audit"), + audit.EnvRetentionDays: "1", + audit.EnvForwardURL: "", + audit.EnvForwardToken: "", + audit.EnvForwardRedact: "none", + audit.EnvAuditDebug: "", + } { + if err := os.Setenv(key, value); err != nil { + _ = os.RemoveAll(tmpDir) + panic("set " + key + ": " + err.Error()) + } + } + // Keep the process-wide audit sink outside per-test TempDir trees. Cobra + // skips PersistentPostRunE on expected command errors, and Windows cannot + // remove a TempDir while the audit lock is still open. + setupAuditSink() openBrowserFunc = func(string) error { return nil } code := m.Run() - _ = os.RemoveAll(tmpDir) + StopAllStdioClients() + CloseAuditSink() + CloseFileLogger() + if err := os.RemoveAll(tmpDir); err != nil { + fmt.Fprintf(os.Stderr, "internal/app test cleanup %s: %v\n", tmpDir, err) + if code == 0 { + code = 1 + } + } os.Exit(code) } diff --git a/internal/auth/coverage_edges_test.go b/internal/auth/coverage_edges_test.go index e5825415..1fa26b34 100644 --- a/internal/auth/coverage_edges_test.go +++ b/internal/auth/coverage_edges_test.go @@ -1234,6 +1234,12 @@ func TestCrossPlatformCoveragePortableAuthBundleCoverageEdges(t *testing.T) { }) t.Run("import validation", func(t *testing.T) { + oldGOOS := portableRuntimeGOOS + t.Cleanup(func() { + portableRuntimeGOOS = oldGOOS + }) + portableRuntimeGOOS = func() string { return "linux" } + t.Setenv(keychain.StorageDirEnv, t.TempDir()) if _, err := ImportPortableAuthBundle(t.TempDir(), nil); err == nil { t.Fatal("nil portable reader succeeded") diff --git a/internal/auth/portable_store.go b/internal/auth/portable_store.go index 2ffb5974..3be3f85a 100644 --- a/internal/auth/portable_store.go +++ b/internal/auth/portable_store.go @@ -69,13 +69,44 @@ type PortableImportReport struct { OSMismatch bool } -// PortableExportSupported reports whether the current platform can produce a -// bundle that includes the file-based DEK required for import elsewhere. +// PortableExportSupported reports whether the current credential backend can +// produce a bundle that includes the file-based DEK required for import +// elsewhere. func PortableExportSupported() bool { - if portableRuntimeGOOS() != "darwin" { - return true + return PortableExportSupportError() == nil +} + +// PortableExportSupportError explains why the current credential backend +// cannot produce a portable auth bundle. A nil error means export is +// supported. +func PortableExportSupportError() error { + return portableExportSupportError(portableRuntimeGOOS(), os.Getenv(keychain.DisableKeychainEnv)) +} + +func portableExportSupportError(goos, disableKeychain string) error { + switch goos { + case "windows": + return fmt.Errorf("portable auth export is not supported on Windows because credentials are stored as DPAPI-protected HKCU Registry values; a portable conversion is not implemented") + case "darwin": + if disableKeychain == "" { + return fmt.Errorf("portable export requires file-DEK mode on macOS; set %s=1 and verify auth first, resetting and re-logging in only if the existing token cannot be decrypted", keychain.DisableKeychainEnv) + } } - return os.Getenv(keychain.DisableKeychainEnv) != "" + return nil +} + +// PortableImportSupportError explains why the current credential backend +// cannot consume a portable auth bundle. A nil error means import is +// supported. +func PortableImportSupportError() error { + return portableImportSupportError(portableRuntimeGOOS()) +} + +func portableImportSupportError(goos string) error { + if goos == "windows" { + return fmt.Errorf("portable auth import is not supported on Windows because file-DEK credentials cannot be represented as DPAPI-protected HKCU Registry values; a portable conversion is not implemented") + } + return nil } // PortableAuthTargetPopulated reports whether local auth files would be @@ -128,8 +159,8 @@ func ExportPortableAuthBundle(configDir string, w io.Writer) error { if w == nil { return fmt.Errorf("missing output writer") } - if !PortableExportSupported() { - return fmt.Errorf("portable export requires file-DEK mode on macOS; set %s=1 and verify auth first, resetting and re-logging in only if the existing token cannot be decrypted", keychain.DisableKeychainEnv) + if err := PortableExportSupportError(); err != nil { + return err } keychainDir := keychain.StorageDir(keychain.Service) if _, err := portableStat(keychainDir); err != nil { @@ -180,6 +211,9 @@ func ImportPortableAuthBundle(configDir string, r io.Reader) (PortableImportRepo if r == nil { return PortableImportReport{}, fmt.Errorf("missing input reader") } + if err := PortableImportSupportError(); err != nil { + return PortableImportReport{}, err + } gz, err := gzip.NewReader(r) if err != nil { return PortableImportReport{}, fmt.Errorf("open auth bundle: %w", err) diff --git a/internal/auth/portable_store_test.go b/internal/auth/portable_store_test.go index f3c7764c..9fc98ec5 100644 --- a/internal/auth/portable_store_test.go +++ b/internal/auth/portable_store_test.go @@ -14,30 +14,126 @@ package auth import ( + "archive/tar" "bytes" + "compress/gzip" "os" + "path" "path/filepath" "runtime" + "strings" "testing" "time" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/keychain" ) -func TestPortableExportSupported(t *testing.T) { - if runtime.GOOS != "darwin" { - if !PortableExportSupported() { - t.Fatal("PortableExportSupported() should be true on non-darwin") +func TestCrossPlatformCoveragePortableExportSupported(t *testing.T) { + switch runtime.GOOS { + case "windows": + if PortableExportSupported() { + t.Fatal("PortableExportSupported() should be false for the Windows DPAPI backend") + } + var bundle bytes.Buffer + err := ExportPortableAuthBundle(t.TempDir(), &bundle) + if err == nil || !strings.Contains(err.Error(), "DPAPI") { + t.Fatalf("ExportPortableAuthBundle() error = %v, want Windows DPAPI unsupported error", err) + } + if bundle.Len() != 0 { + t.Fatalf("ExportPortableAuthBundle() wrote %d bytes on unsupported Windows backend", bundle.Len()) } return + case "darwin": + t.Setenv(keychain.DisableKeychainEnv, "") + if PortableExportSupported() { + t.Fatal("PortableExportSupported() should be false on darwin without file DEK") + } + t.Setenv(keychain.DisableKeychainEnv, "1") + if !PortableExportSupported() { + t.Fatal("PortableExportSupported() should be true when file DEK is enabled") + } + return + default: + if !PortableExportSupported() { + t.Fatal("PortableExportSupported() should be true for file-DEK platforms") + } } - t.Setenv(keychain.DisableKeychainEnv, "") - if PortableExportSupported() { - t.Fatal("PortableExportSupported() should be false on darwin without file DEK") +} + +func TestCrossPlatformCoveragePortableExportSupportError(t *testing.T) { + tests := []struct { + name string + goos string + disableKeychain string + wantError string + }{ + {name: "windows dpapi", goos: "windows", disableKeychain: "1", wantError: "DPAPI"}, + {name: "darwin keychain", goos: "darwin", wantError: keychain.DisableKeychainEnv}, + {name: "darwin file dek", goos: "darwin", disableKeychain: "1"}, + {name: "linux file dek", goos: "linux"}, } - t.Setenv(keychain.DisableKeychainEnv, "1") - if !PortableExportSupported() { - t.Fatal("PortableExportSupported() should be true when file DEK is enabled") + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := portableExportSupportError(test.goos, test.disableKeychain) + if test.wantError == "" { + if err != nil { + t.Fatalf("portableExportSupportError() error = %v, want nil", err) + } + return + } + if err == nil || !strings.Contains(err.Error(), test.wantError) { + t.Fatalf("portableExportSupportError() error = %v, want substring %q", err, test.wantError) + } + }) + } +} + +func TestCrossPlatformCoveragePortableImportSupportError(t *testing.T) { + tests := []struct { + name string + goos string + wantError string + }{ + {name: "windows dpapi", goos: "windows", wantError: "DPAPI"}, + {name: "darwin", goos: "darwin"}, + {name: "linux", goos: "linux"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := portableImportSupportError(test.goos) + if test.wantError == "" { + if err != nil { + t.Fatalf("portableImportSupportError() error = %v, want nil", err) + } + return + } + if err == nil || !strings.Contains(err.Error(), test.wantError) { + t.Fatalf("portableImportSupportError() error = %v, want substring %q", err, test.wantError) + } + }) + } +} + +func TestCrossPlatformCoverageImportPortableAuthBundleRejectsWindowsDPAPIWithoutMutation(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Windows DPAPI contract requires a native Windows runner") + } + + root := t.TempDir() + configDir := filepath.Join(root, ".dws") + keychainDir := filepath.Join(root, "keychain") + t.Setenv(keychain.StorageDirEnv, keychainDir) + bundle := validPortableAuthBundleForTest(t) + + if _, err := ImportPortableAuthBundle(configDir, bytes.NewReader(bundle)); err == nil || !strings.Contains(err.Error(), "DPAPI") { + t.Fatalf("ImportPortableAuthBundle() error = %v, want Windows DPAPI unsupported error", err) + } + for _, path := range []string{configDir, keychainDir} { + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("unsupported Windows import touched %s: stat error = %v", path, err) + } } } @@ -221,3 +317,41 @@ func requirePortableFileBackend(t *testing.T) { t.Skip("portable bundle round trips require a file-DEK backend; Windows uses DPAPI registry storage") } } + +func validPortableAuthBundleForTest(t *testing.T) []byte { + t.Helper() + + var bundle bytes.Buffer + gz := gzip.NewWriter(&bundle) + tw := tar.NewWriter(gz) + manifest := portableAuthBundleManifest{ + Version: 1, + CreatedAt: time.Now().UTC().Format(time.RFC3339), + OS: "linux", + KeychainService: keychain.Service, + ConfigFiles: []string{"app.json"}, + } + if err := writePortableManifest(tw, manifest); err != nil { + t.Fatalf("writePortableManifest() error = %v", err) + } + entries := map[string]string{ + path.Join("keychain", keychain.Service, keychain.AccountToken+".enc"): "encrypted-token", + "config/app.json": "{}", + } + for name, content := range entries { + hdr := &tar.Header{Name: name, Mode: 0o600, Size: int64(len(content))} + if err := tw.WriteHeader(hdr); err != nil { + t.Fatalf("WriteHeader(%s) error = %v", name, err) + } + if _, err := tw.Write([]byte(content)); err != nil { + t.Fatalf("Write(%s) error = %v", name, err) + } + } + if err := tw.Close(); err != nil { + t.Fatalf("tar.Close() error = %v", err) + } + if err := gz.Close(); err != nil { + t.Fatalf("gzip.Close() error = %v", err) + } + return bundle.Bytes() +} diff --git a/internal/auth/remaining_coverage_test.go b/internal/auth/remaining_coverage_test.go index 5044bfbc..828d635f 100644 --- a/internal/auth/remaining_coverage_test.go +++ b/internal/auth/remaining_coverage_test.go @@ -91,6 +91,7 @@ func TestCrossPlatformCoverageTokenPersistencePreflightRemainingEdges(t *testing } func TestCrossPlatformCoveragePortableExportRemainingEdges(t *testing.T) { + origGOOS := portableRuntimeGOOS origStat := portableStat origConfigFiles := portableConfigFilesForExport origManifest := portableWriteManifest @@ -99,6 +100,7 @@ func TestCrossPlatformCoveragePortableExportRemainingEdges(t *testing.T) { origKeychainGet := authKeychainGet origKeychainExists := authKeychainExists t.Cleanup(func() { + portableRuntimeGOOS = origGOOS portableStat = origStat portableConfigFilesForExport = origConfigFiles portableWriteManifest = origManifest @@ -107,6 +109,7 @@ func TestCrossPlatformCoveragePortableExportRemainingEdges(t *testing.T) { authKeychainGet = origKeychainGet authKeychainExists = origKeychainExists }) + portableRuntimeGOOS = func() string { return "linux" } t.Setenv(keychain.StorageDirEnv, t.TempDir()) t.Setenv(keychain.DisableKeychainEnv, "1") diff --git a/internal/logging/logger.go b/internal/logging/logger.go index b9dad102..295b4e70 100644 --- a/internal/logging/logger.go +++ b/internal/logging/logger.go @@ -79,7 +79,8 @@ func (fl *FileLogger) Writer() io.Writer { return fl.writer } -// Close flushes and closes the underlying log file. +// Close flushes and permanently closes the underlying log file. Later writes +// fail with os.ErrClosed instead of reopening the file. func (fl *FileLogger) Close() error { if fl == nil || fl.writer == nil { return nil @@ -122,6 +123,7 @@ type rotatingWriter struct { mu sync.Mutex file *os.File written int64 + closed bool } func newRotatingWriter(path string, maxSize int64, maxBackups int) *rotatingWriter { @@ -135,6 +137,9 @@ func newRotatingWriter(path string, maxSize int64, maxBackups int) *rotatingWrit func (w *rotatingWriter) open() error { w.mu.Lock() defer w.mu.Unlock() + if w.closed { + return os.ErrClosed + } f, err := openLogFile(w.path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, config.FilePerm) if err != nil { @@ -153,6 +158,9 @@ func (w *rotatingWriter) open() error { func (w *rotatingWriter) Write(p []byte) (int, error) { w.mu.Lock() defer w.mu.Unlock() + if w.closed { + return 0, os.ErrClosed + } // Attempt to recover if file was lost (e.g. rotation failed due to full disk). if w.file == nil { @@ -177,6 +185,9 @@ func (w *rotatingWriter) Write(p []byte) (int, error) { // reopenLocked tries to reopen the log file. Caller must hold w.mu. func (w *rotatingWriter) reopenLocked() error { + if w.closed { + return os.ErrClosed + } f, err := openLogFile(w.path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, config.FilePerm) if err != nil { return err @@ -199,6 +210,10 @@ func (w *rotatingWriter) close() error { w.mu.Lock() defer w.mu.Unlock() + if w.closed { + return nil + } + w.closed = true if w.file == nil { return nil } diff --git a/internal/logging/logger_test.go b/internal/logging/logger_test.go index 18e05489..f5bf7dfe 100644 --- a/internal/logging/logger_test.go +++ b/internal/logging/logger_test.go @@ -14,6 +14,7 @@ package logging import ( + "errors" "os" "path/filepath" "strings" @@ -121,3 +122,39 @@ func TestCloseOnNilLogger(t *testing.T) { t.Errorf("Close on nil should not error: %v", err) } } + +func TestCrossPlatformCoverageFileLoggerCloseIsTerminal(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + fl := Setup(dir) + logPath := filepath.Join(dir, logSubDir, logFileName) + fl.Logger.Info("before close") + if err := fl.Close(); err != nil { + t.Fatalf("close logger: %v", err) + } + if err := os.Remove(logPath); err != nil { + t.Fatalf("remove closed log file: %v", err) + } + if err := fl.writer.open(); !errors.Is(err, os.ErrClosed) { + t.Fatalf("open after close = %v, want os.ErrClosed", err) + } + fl.writer.mu.Lock() + err := fl.writer.reopenLocked() + fl.writer.mu.Unlock() + if !errors.Is(err, os.ErrClosed) { + t.Fatalf("reopen after close = %v, want os.ErrClosed", err) + } + + n, err := fl.Writer().Write([]byte("late write")) + if n != 0 || !errors.Is(err, os.ErrClosed) { + t.Fatalf("write after close = (%d, %v), want (0, os.ErrClosed)", n, err) + } + fl.Logger.Info("late structured write") + if _, err := os.Stat(logPath); !os.IsNotExist(err) { + t.Fatalf("closed logger recreated log file: %v", err) + } + if err := fl.Close(); err != nil { + t.Fatalf("second close: %v", err) + } +}