From be6b9526472dab531113b76b6e0f23ea17137376 Mon Sep 17 00:00:00 2001
From: "kuma.jung" <yj.jung5078@gmail.com>
Date: Tue, 16 Jun 2026 14:43:03 +0900
Subject: [PATCH] fix: prevent SSO temp config leaks

BuildSSOLoginCmd now validates incomplete non-profile SSO config before creating the temporary AWS config directory, and the write-error path is covered by a cleanup regression test.

Constraint: Issue #207 requires temp config setup errors to clean up internally while success callers still own the returned cleanup function
Confidence: high
Scope-risk: narrow
Directive: Keep new error paths after temp directory creation paired with cleanup before returning
Tested: env -u GOROOT go test ./internal/services/aws -run 'TestBuildSSOLoginCmd|TestEnsureSSOLogin' -count=1
Tested: env -u GOROOT make test
Tested: env -u GOROOT go vet ./...
Tested: env -u GOROOT make build
Related: #207
---
 internal/services/aws/sso.go      |  7 ++++++-
 internal/services/aws/sso_test.go | 31 +++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/internal/services/aws/sso.go b/internal/services/aws/sso.go
index f8d041c..959e8fd 100644
--- a/internal/services/aws/sso.go
+++ b/internal/services/aws/sso.go
@@ -45,6 +45,7 @@ var newSSOClient = func(cfg aws.Config) SSOClientAPI {
 }
 
 var runSSOLoginFn = RunSSOLogin
+var writeSSOConfigFile = os.WriteFile
 
 // ssoTokenCache represents the cached SSO token file structure.
 type ssoTokenCache struct {
@@ -306,6 +307,9 @@ func BuildSSOLoginCmd(cfg *config.Config) (*exec.Cmd, func(), error) {
 		cmd := exec.Command("aws", "sso", "login", "--profile", cfg.Profile)
 		return cmd, func() {}, nil
 	}
+	if cfg.SSOAccountID == "" || cfg.SSORoleName == "" {
+		return nil, nil, fmt.Errorf("SSO login requires sso_account_id and sso_role_name when profile is not set")
+	}
 
 	tmpDir, err := os.MkdirTemp("", "unic-sso-*")
 	if err != nil {
@@ -324,13 +328,14 @@ func BuildSSOLoginCmd(cfg *config.Config) (*exec.Cmd, func(), error) {
 	)
 
 	configPath := filepath.Join(tmpDir, "config")
-	if err := os.WriteFile(configPath, []byte(configContent), 0600); err != nil {
+	if err := writeSSOConfigFile(configPath, []byte(configContent), 0600); err != nil {
 		cleanup()
 		return nil, nil, fmt.Errorf("failed to write temp SSO config: %w", err)
 	}
 
 	cmd := exec.Command("aws", "sso", "login", "--profile", profileName)
 	cmd.Env = append(os.Environ(), "AWS_CONFIG_FILE="+configPath)
+	// On success, callers own the temporary config directory and must defer cleanup().
 	return cmd, cleanup, nil
 }
 
diff --git a/internal/services/aws/sso_test.go b/internal/services/aws/sso_test.go
index b0006b5..67579ca 100644
--- a/internal/services/aws/sso_test.go
+++ b/internal/services/aws/sso_test.go
@@ -4,6 +4,7 @@ import (
 	"crypto/sha1"
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"os"
 	"path/filepath"
 	"testing"
@@ -141,3 +142,33 @@ func TestEnsureSSOLoginRunsLoginWhenTokenExpired(t *testing.T) {
 		t.Fatal("expected login result to report a refreshed session")
 	}
 }
+
+func TestBuildSSOLoginCmdCleansTempDirOnConfigWriteError(t *testing.T) {
+	tempRoot := t.TempDir()
+	t.Setenv("TMPDIR", tempRoot)
+
+	origWriteSSOConfigFile := writeSSOConfigFile
+	defer func() { writeSSOConfigFile = origWriteSSOConfigFile }()
+	writeSSOConfigFile = func(string, []byte, os.FileMode) error {
+		return errors.New("write failed")
+	}
+
+	cmd, cleanup, err := BuildSSOLoginCmd(testSSOConfig())
+	if err == nil {
+		t.Fatal("expected config write error")
+	}
+	if cmd != nil {
+		t.Fatalf("expected nil command on error, got %v", cmd)
+	}
+	if cleanup != nil {
+		t.Fatal("expected nil cleanup when setup fails")
+	}
+
+	matches, globErr := filepath.Glob(filepath.Join(tempRoot, "unic-sso-*"))
+	if globErr != nil {
+		t.Fatalf("failed to inspect temp dirs: %v", globErr)
+	}
+	if len(matches) != 0 {
+		t.Fatalf("expected temp config directory to be removed, found %v", matches)
+	}
+}