fix: prevent SSO temp config leaks

[YoungJinJung]

Summary

• validate incomplete non-profile SSO login config before creating the temporary AWS config directory
• keep temp config write errors internally cleaned up before returning
• add a regression test that simulates config write failure and asserts no temp directory remains

Validation

• env -u GOROOT go test ./internal/services/aws -run 'TestBuildSSOLoginCmd|TestEnsureSSOLogin' -count=1
• env -u GOROOT make test
• env -u GOROOT go vet ./...
• env -u GOROOT make build

Closes #207

Summary by CodeRabbit

• Bug Fixes
  • Improved AWS SSO login reliability with enhanced error handling and validation for profile configuration parameters.
  • Fixed temporary directory cleanup to properly remove files when SSO configuration write operations fail.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 632faeb9-f16c-4117-ab7e-1fd7b1d76aa7

📥 Commits

Reviewing files that changed from the base of the PR and between 0b33442 and be6b952.

📒 Files selected for processing (2)

• internal/services/aws/sso.go
• internal/services/aws/sso_test.go

📜 Recent review details

🧰 Additional context used

📓 Path-based instructions (4)

**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.go: Use lipgloss for styled TUI output — column-aligned tables with dimmed labels in Go implementation files
Implement scroll windowing with formula: visibleLines := max(m.height-N, 5) in Go TUI implementation

Files:

• internal/services/aws/sso_test.go
• internal/services/aws/sso.go

---

⚙️ CodeRabbit configuration file

**/*.go: For Go reviews, look beyond compilation and prioritize nil pointer risks,
context propagation, AWS SDK pagination, error wrapping, deterministic
sorting, and stable table/detail rendering. For new AWS service work,
verify that repository interfaces, model mapping, app integration, and
tests are updated together.

Files:

• internal/services/aws/sso_test.go
• internal/services/aws/sso.go

**/*_test.go

📄 CodeRabbit inference engine (CLAUDE.md)

Tests use mock client interfaces (see rds_test.go pattern) in Go test files

Files:

• internal/services/aws/sso_test.go

---

⚙️ CodeRabbit configuration file

**/*_test.go: Check that tests cover API errors, mapping edge cases, and navigation
state transitions, not only happy paths. Prefer mock-based tests that do
not depend on external AWS calls.

Files:

• internal/services/aws/sso_test.go

internal/services/aws/**

⚙️ CodeRabbit configuration file

internal/services/aws/**: For AWS integration code, focus on SDK client interface mockability,
paginator usage, nil/empty response handling, AWS pointer conversion,
stable list ordering, and user-facing error messages.

Files:

• internal/services/aws/sso_test.go
• internal/services/aws/sso.go

**

⚙️ CodeRabbit configuration file

**: # unic — Roles

Developer

You — All implementation, code review, testing, and releases.

Advisor (Kiro)

Senior engineer role. Responsibilities:

• Architecture decisions and trade-off analysis
• Code review guidance when asked
• Debugging help and troubleshooting
• AWS SDK / API usage advice
• Bubbletea / TUI pattern recommendations
• Suggest approaches, never write code autonomously

Rule: Advisor does not write or modify code unless explicitly asked. All code is written by the developer.

Documentation Harness

When implementation changes affect user-visible behavior, config/auth behavior, service coverage, TUI flow, or contributor workflow:

• update README.md
• update the relevant file under docs/
• use docs/documentation-harness.md as the minimum checklist

A feature change is not considered complete until the related docs are reviewed and updated when needed.

Branch Naming Harness

When creating a working branch for repository work, prefer the convention defined in docs/branch-naming-harness.md.

Expected format:

• <work-type>/<issue-number>-<short-description>

Examples:

• feature/58-s3-browser
• bugfix/76-s3-region-error-handling
• docs/79-documentation-harness

Worktree Isolation Rule

All repository work must start from main in a fresh git worktree.

• Before editing files, fetch or otherwise verify the intended main base.
• Create a new worktree and branch from main or origin/main.
• Do not implement new work directly in the primary checkout or on an existing
  feature branch.
• Keep one worktree per task, issue, or PR-sized change.
• If follow-up work appears to depend on another unmerged branch, still create a
  fresh worktree from main first, then explicitly document and apply the
  dependency only when it is unavoidable.

Files:

• internal/services/aws/sso_test.go
• internal/services/aws/sso.go

🔇 Additional comments (2)

internal/services/aws/sso.go (1)

48-48: LGTM!

Also applies to: 310-312, 331-333, 338-339

internal/services/aws/sso_test.go (1)

7-7: LGTM!

Also applies to: 146-174

---

Walkthrough

BuildSSOLoginCmd in sso.go gains a package-level writeSSOConfigFile variable (defaulting to os.WriteFile) to allow test substitution, adds a guard that returns an error when both SSOAccountID and SSORoleName are missing, and routes the config write through that variable with existing cleanup-on-failure logic. A new test verifies the temp directory is deleted when the write fails.

Changes

SSO login cmd validation and temp dir cleanup

Layer / File(s)	Summary
Mockable write variable, validation guard, and cleanup on failure internal/services/aws/sso.go	Declares writeSSOConfigFile as var writeSSOConfigFile = os.WriteFile, inserts an early return when cfg.SSOAccountID or cfg.SSORoleName is empty, and calls writeSSOConfigFile instead of os.WriteFile directly, preserving cleanup-on-failure and adding a success-path comment.
Temp dir cleanup test internal/services/aws/sso_test.go	Adds errors import and TestBuildSSOLoginCmdCleansTempDirOnConfigWriteError, which injects a failing writeSSOConfigFile, asserts nil returns from BuildSSOLoginCmd, and uses filesystem globbing to confirm the unic-sso-* temp dir was removed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title uses the 'fix:' prefix as required and clearly summarizes the main change: preventing SSO temp config leaks.
Description check	✅ Passed	The description covers all critical sections: Summary explains what changed and why, Validation documents testing steps, and it closes issue #207. However, some optional sections like README/docs updates are not addressed.
Linked Issues check	✅ Passed	The code changes address all four checklist items from issue #207: moved profile validation before temp dir creation, ensured cleanup on errors, added regression test, and the changes verify temp directory is removed on write failure.
Out of Scope Changes check	✅ Passed	All changes are directly aligned with issue #207 objectives: profile validation reordering, error handling with cleanup, test coverage for failure scenarios, and no unrelated modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch bugfix/207-sso-tempdir-cleanup

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[amazon-q-developer]

The changes effectively prevent SSO temp config directory leaks by validating incomplete configurations before creating temporary directories and ensuring proper cleanup on write errors. The implementation is solid with appropriate test coverage for the regression scenario.

---

You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.