fix: atomically write config.json via temp+rename

config/configuration.go

@@ -17,6 +17,7 @@ package config
 import (
 	"encoding/json"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -217,13 +218,12 @@ func LoadConfigurationWithContext(ctx *cli.Context) (conf *Configuration, err er
 }
 
 func SaveConfiguration(config *Configuration) (err error) {
-	// fmt.Printf("conf %v\n", config)
 	bytes, err := json.MarshalIndent(config, "", "\t")
 	if err != nil {
 		return
 	}
-	path := GetConfigPath() + "/" + configFile
-	err = os.WriteFile(path, bytes, 0600)
+	path := filepath.Join(GetConfigPath(), configFile)
+	err = atomicWriteFile(path, bytes, 0600)
 	return
 }
 
@@ -232,7 +232,7 @@ func SaveConfigurationWithContext(ctx *cli.Context, config *Configuration) (err
 	if err != nil {
 		return
 	}
-	confFilePath := hookGetHomePath(GetHomePath)() + configPath + "/" + configFile
+	confFilePath := filepath.Join(hookGetHomePath(GetHomePath)()+configPath, configFile)
 	if customPath, ok := ConfigurePathFlag(ctx.Flags()).GetValue(); ok {
 		confFilePath = customPath
 	}
@@ -243,10 +243,79 @@ func SaveConfigurationWithContext(ctx *cli.Context, config *Configuration) (err
 			panic(fmt.Errorf("failed to create config directory %q: %w", dir, err))
 		}
 	}
-	err = os.WriteFile(confFilePath, bytes, 0600)
+	err = atomicWriteFile(confFilePath, bytes, 0600)
 	return
 }
 
+// atomicWriteFile writes data via a same-directory temp file then os.Rename.
+// On Windows, os.Rename replaces an existing destination (MoveFileEx REPLACE_EXISTING),
+// matching credentials-go / mcpproxy behavior and avoiding truncated config.json on crash.
+func atomicWriteFile(path string, data []byte, perm os.FileMode) error {
+	return atomicWriteFileWithRename(path, data, perm, os.Rename)
+}
+
+type atomicTempFile interface {
+	Name() string
+	Chmod(os.FileMode) error
+	Write([]byte) (int, error)
+	Sync() error
+	Close() error
+}
+
+var createAtomicTempFile = func(dir, pattern string) (atomicTempFile, error) {
+	return os.CreateTemp(dir, pattern)
+}
+
+var lstatAtomicPath = os.Lstat
+
+func atomicWriteFileWithRename(path string, data []byte, perm os.FileMode, rename func(string, string) error) error {
+	if info, err := lstatAtomicPath(path); err == nil && info.Mode()&os.ModeSymlink != 0 {
+		resolvedPath, err := filepath.EvalSymlinks(path)
+		if err != nil {
+			return fmt.Errorf("failed to resolve config symlink %q: %w", path, err)
+		}
+		path = resolvedPath
+	} else if err != nil && !os.IsNotExist(err) {
+		return fmt.Errorf("failed to inspect config path %q: %w", path, err)
+	}
+
+	dir := filepath.Dir(path)
+	base := filepath.Base(path)
+	temp, err := createAtomicTempFile(dir, "."+base+".tmp-*")
+	if err != nil {
+		return fmt.Errorf("failed to create temp config in %q: %w", dir, err)
+	}
+	tempPath := temp.Name()
+	closed := false
+	defer func() {
+		if !closed {
+			_ = temp.Close()
+		}
+		_ = os.Remove(tempPath)
+	}()
+
+	if err := temp.Chmod(perm); err != nil {
+		return fmt.Errorf("failed to set temp config permissions %q: %w", tempPath, err)
+	}
+	if n, err := temp.Write(data); err != nil {
+		return fmt.Errorf("failed to write temp config %q: %w", tempPath, err)
+	} else if n != len(data) {
+		return fmt.Errorf("failed to write temp config %q: %w", tempPath, io.ErrShortWrite)
+	}
+	if err := temp.Sync(); err != nil {
+		return fmt.Errorf("failed to sync temp config %q: %w", tempPath, err)
+	}
+	if err := temp.Close(); err != nil {
+		return fmt.Errorf("failed to close temp config %q: %w", tempPath, err)
+	}
+	closed = true
+
+	if err := rename(tempPath, path); err != nil {
+		return fmt.Errorf("failed to rename temp config to %q: %w", path, err)
+	}
+	return nil
+}
+
 func NewConfigFromBytes(bytes []byte) (conf *Configuration, err error) {
 	conf = NewConfiguration()
 	err = json.Unmarshal(bytes, conf)

config/configuration_test.go

@@ -18,7 +18,9 @@ import (
 	"encoding/json"
 	"errors"
 	"os"
+	"path/filepath"
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/aliyun/aliyun-cli/v3/cli"
@@ -217,14 +219,232 @@ func TestSaveConfiguration(t *testing.T) {
 	assert.Nil(t, err)
 	err = SaveConfiguration(conf)
 	assert.Nil(t, err)
-	file, err := os.Open(GetConfigPath() + "/" + configFile)
+	file, err := os.Open(filepath.Join(GetConfigPath(), configFile))
 	assert.Nil(t, err)
 	buf := make([]byte, 1024)
 	n, _ := file.Read(buf)
 	file.Close()
 	assert.Equal(t, string(bytes), string(buf[:n]))
 }
 
+func TestAtomicWriteFile_OverwriteExisting(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "config.json")
+
+	err := os.WriteFile(path, []byte(`{"current":"old"}`), 0600)
+	assert.NoError(t, err)
+
+	newContent := []byte(`{"current":"new","profiles":[]}`)
+	err = atomicWriteFile(path, newContent, 0600)
+	assert.NoError(t, err)
+
+	got, err := os.ReadFile(path)
+	assert.NoError(t, err)
+	assert.Equal(t, string(newContent), string(got))
+
+	entries, err := os.ReadDir(dir)
+	assert.NoError(t, err)
+	for _, e := range entries {
+		assert.False(t, strings.Contains(e.Name(), ".tmp-"), "temp file should be cleaned: %s", e.Name())
+	}
+}
+
+func TestAtomicWriteFile_RenameFailurePreservesExisting(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "config.json")
+	oldContent := []byte(`{"current":"old"}`)
+	assert.NoError(t, os.WriteFile(path, oldContent, 0600))
+
+	rename := func(oldPath, newPath string) error {
+		return errors.New("injected rename failure")
+	}
+
+	err := atomicWriteFileWithRename(path, []byte(`{"current":"new"}`), 0600, rename)
+	assert.ErrorContains(t, err, "injected rename failure")
+
+	got, err := os.ReadFile(path)
+	assert.NoError(t, err)
+	assert.Equal(t, oldContent, got)
+
+	entries, err := os.ReadDir(dir)
+	assert.NoError(t, err)
+	for _, entry := range entries {
+		assert.False(t, strings.Contains(entry.Name(), ".tmp-"), "temp file should be cleaned: %s", entry.Name())
+	}
+}
+
+func TestAtomicWriteFile_PreservesSymlink(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("creating symlinks may require elevated privileges on Windows")
+	}
+
+	targetDir := t.TempDir()
+	targetPath := filepath.Join(targetDir, "config.json")
+	assert.NoError(t, os.WriteFile(targetPath, []byte(`{"current":"old"}`), 0600))
+
+	linkDir := t.TempDir()
+	linkPath := filepath.Join(linkDir, "config.json")
+	assert.NoError(t, os.Symlink(targetPath, linkPath))
+
+	newContent := []byte(`{"current":"new"}`)
+	assert.NoError(t, atomicWriteFile(linkPath, newContent, 0600))
+
+	info, err := os.Lstat(linkPath)
+	assert.NoError(t, err)
+	assert.NotZero(t, info.Mode()&os.ModeSymlink)
+	got, err := os.ReadFile(targetPath)
+	assert.NoError(t, err)
+	assert.Equal(t, newContent, got)
+}
+
+func TestAtomicWriteFile_DanglingSymlinkFails(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("creating symlinks may require elevated privileges on Windows")
+	}
+
+	dir := t.TempDir()
+	linkPath := filepath.Join(dir, "config.json")
+	assert.NoError(t, os.Symlink(filepath.Join(dir, "missing.json"), linkPath))
+
+	err := atomicWriteFile(linkPath, []byte(`{"current":"new"}`), 0600)
+	assert.ErrorContains(t, err, "failed to resolve config symlink")
+}
+
+func TestAtomicWriteFile_CreateTempFailure(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "missing", "config.json")
+
+	err := atomicWriteFile(path, []byte(`{"current":"new"}`), 0600)
+	assert.ErrorContains(t, err, "failed to create temp config")
+}
+
+func TestAtomicWriteFile_InspectPathFailure(t *testing.T) {
+	// Do not rely on "parent path is a file" to trigger Lstat failure:
+	// on Windows that yields a not-exist-style error (IsNotExist=true), so the
+	// inspect branch is skipped and CreateTemp fails instead. Inject Lstat.
+	originLstat := lstatAtomicPath
+	defer func() {
+		lstatAtomicPath = originLstat
+	}()
+	lstatAtomicPath = func(name string) (os.FileInfo, error) {
+		return nil, errors.New("permission denied")
+	}
+
+	err := atomicWriteFile(filepath.Join(t.TempDir(), "config.json"), []byte(`{"current":"new"}`), 0600)
+	assert.ErrorContains(t, err, "failed to inspect config path")
+	assert.ErrorContains(t, err, "permission denied")
+}
+
+func TestAtomicWriteFile_TempFileFailures(t *testing.T) {
+	originCreateTemp := createAtomicTempFile
+	defer func() {
+		createAtomicTempFile = originCreateTemp
+	}()
+
+	tests := []struct {
+		name    string
+		file    *fakeAtomicTempFile
+		wantErr string
+	}{
+		{
+			name:    "chmod",
+			file:    &fakeAtomicTempFile{chmodErr: errors.New("chmod failed")},
+			wantErr: "chmod failed",
+		},
+		{
+			name:    "write",
+			file:    &fakeAtomicTempFile{writeErr: errors.New("write failed")},
+			wantErr: "write failed",
+		},
+		{
+			name:    "short write",
+			file:    &fakeAtomicTempFile{writeN: 1},
+			wantErr: "short write",
+		},
+		{
+			name:    "sync",
+			file:    &fakeAtomicTempFile{syncErr: errors.New("sync failed")},
+			wantErr: "sync failed",
+		},
+		{
+			name:    "close",
+			file:    &fakeAtomicTempFile{closeErr: errors.New("close failed")},
+			wantErr: "close failed",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tempPath := filepath.Join(t.TempDir(), "config.json.tmp")
+			tt.file.name = tempPath
+			createAtomicTempFile = func(dir, pattern string) (atomicTempFile, error) {
+				return tt.file, nil
+			}
+
+			err := atomicWriteFile(filepath.Join(t.TempDir(), "config.json"), []byte(`{"current":"new"}`), 0600)
+			assert.ErrorContains(t, err, tt.wantErr)
+		})
+	}
+}
+
+func TestSaveConfiguration_OverwriteExisting(t *testing.T) {
+	orighookGetHomePath := hookGetHomePath
+	defer func() {
+		os.RemoveAll("./.aliyun")
+		hookGetHomePath = orighookGetHomePath
+	}()
+	hookGetHomePath = func(fn func() string) func() string {
+		return func() string {
+			return "."
+		}
+	}
+
+	oldConf := &Configuration{
+		CurrentProfile: "old",
+		Profiles:       []Profile{{Language: "en", Name: "old", Mode: "AK", AccessKeyId: "old_id", AccessKeySecret: "old_secret", RegionId: "cn-hangzhou", OutputFormat: "json"}},
+	}
+	assert.NoError(t, SaveConfiguration(oldConf))
+
+	newConf := &Configuration{
+		CurrentProfile: "default",
+		Profiles:       []Profile{{Language: "en", Name: "default", Mode: "AK", AccessKeyId: "new_id", AccessKeySecret: "new_secret", RegionId: "cn-beijing", OutputFormat: "json"}},
+	}
+	assert.NoError(t, SaveConfiguration(newConf))
+
+	path := filepath.Join(GetConfigPath(), configFile)
+	loaded, err := LoadConfigurationFromFile(path)
+	assert.NoError(t, err)
+	assert.Equal(t, "default", loaded.CurrentProfile)
+	assert.Equal(t, "new_id", loaded.Profiles[0].AccessKeyId)
+	assert.Equal(t, "cn-beijing", loaded.Profiles[0].RegionId)
+
+	entries, err := os.ReadDir(GetConfigPath())
+	assert.NoError(t, err)
+	for _, e := range entries {
+		assert.False(t, strings.Contains(e.Name(), ".tmp-"), "temp file should be cleaned: %s", e.Name())
+	}
+}
+
+func TestSaveConfigurationWithContext_CustomPathCreatesDir(t *testing.T) {
+	stdout := new(bytes.Buffer)
+	stderr := new(bytes.Buffer)
+	ctx := cli.NewCommandContext(stdout, stderr)
+	AddFlags(ctx.Flags())
+	customPath := filepath.Join(t.TempDir(), "nested", "config.json")
+	ConfigurePathFlag(ctx.Flags()).SetAssigned(true)
+	ConfigurePathFlag(ctx.Flags()).SetValue(customPath)
+
+	conf := &Configuration{
+		CurrentProfile: "default",
+		Profiles:       []Profile{{Language: "en", Name: "default", Mode: "AK", AccessKeyId: "new_id", AccessKeySecret: "new_secret", RegionId: "cn-beijing", OutputFormat: "json"}},
+	}
+	assert.NoError(t, SaveConfigurationWithContext(ctx, conf))
+
+	loaded, err := LoadConfigurationFromFile(customPath)
+	assert.NoError(t, err)
+	assert.Equal(t, "default", loaded.CurrentProfile)
+	assert.Equal(t, "new_id", loaded.Profiles[0].AccessKeyId)
+}
+
 func TestLoadOrCreateConfiguration(t *testing.T) {
 	orighookGetHomePath := hookGetHomePath
 	defer func() {
@@ -401,3 +621,38 @@ func TestGetConfigurePath(t *testing.T) {
 	p = getConfigurePath(ctx)
 	assert.Equal(t, p, "/path/to/config.json")
 }
+
+type fakeAtomicTempFile struct {
+	name     string
+	writeN   int
+	chmodErr error
+	writeErr error
+	syncErr  error
+	closeErr error
+}
+
+func (f *fakeAtomicTempFile) Name() string {
+	return f.name
+}
+
+func (f *fakeAtomicTempFile) Chmod(os.FileMode) error {
+	return f.chmodErr
+}
+
+func (f *fakeAtomicTempFile) Write(data []byte) (int, error) {
+	if f.writeErr != nil {
+		return 0, f.writeErr
+	}
+	if f.writeN != 0 {
+		return f.writeN, nil
+	}
+	return len(data), nil
+}
+
+func (f *fakeAtomicTempFile) Sync() error {
+	return f.syncErr
+}
+
+func (f *fakeAtomicTempFile) Close() error {
+	return f.closeErr
+}