fix(auth): propagate TTY state to subprocesses for SSO device flow in workflows by Benbentwo · Pull Request #2126 · cloudposse/atmos

Benbentwo · 2026-03-02T22:09:28Z

What

Fixes atmos auth login failing with "AWS SSO device flow requires an interactive terminal (TTY)" when called as a step inside an atmos workflow.

Why

When atmos runs workflow command steps via ExecuteShellCommand, cmd.Stderr is wrapped in a MaskWriter (for secret masking). This creates a pipe between the parent and subprocess, so the subprocess's term.IsTerminal(os.Stderr.Fd()) returns false — even when the user is sitting at an interactive terminal. As a result, the SSO device auth flow's isInteractive() guard blocks authentication.

Two-layer fix:

pkg/auth/providers/aws/sso.go — isInteractive() now checks --force-tty / ATMOS_FORCE_TTY via viper before falling back to raw TTY detection. This allows any caller to explicitly signal an interactive context.
internal/exec/shell_utils.go — ExecuteShellCommand now checks whether the parent process has a real TTY on stderr. When it does (and ATMOS_FORCE_TTY is not already set), it injects ATMOS_FORCE_TTY=true into the subprocess environment. This propagates the interactive context through the MaskWriter pipe automatically.

Behavior matrix after fix:

Scenario	Before	After
Direct `atmos auth login` from terminal	✅ works	✅ works
Workflow `command: auth login` from terminal	❌ fails	✅ works
Workflow in CI/CD (no TTY)	❌ fails (correct)	❌ fails (correct)
`ATMOS_FORCE_TTY=true` set manually	❌ ignored	✅ respected
Cached SSO token exists	✅ works	✅ works

Tests

TestIsInteractive_ForceTTY — verifies force-tty viper key enables interactive mode
TestEnvKeyIsSet — table-driven tests for the envKeyIsSet helper (6 cases)
TestExecuteShellCommand — two new subtests:
- ATMOS_FORCE_TTY preserved when explicitly set in step env — user-set value is never overridden
- ATMOS_FORCE_TTY not auto-injected in non-TTY environment — CI/pipe environments stay non-interactive

References

Root cause: MaskWriter wraps os.Stderr as a pipe in ExecuteShellCommand, breaking term.IsTerminal() in subprocess even when the parent process has a real TTY

Summary by CodeRabbit

Bug Fixes
- More reliable TTY detection and propagation so subprocesses (e.g., interactive auth flows) behave correctly across CI, non‑TTY, and parent‑TTY environments.
- Prevents unintended overrides when a TTY-related environment flag is explicitly set.
Tests
- Expanded coverage validating TTY behavior and environment‑flag handling across multiple scenarios.
Chores
- Updated displayed version list content in CLI output snapshots.

… workflows When atmos runs workflow steps via ExecuteShellCommand, stderr is wrapped in a MaskWriter pipe. This causes subprocess TTY detection (os.Stderr.Fd()) to return a pipe descriptor instead of a terminal, breaking AWS SSO device authorization flow which requires an interactive TTY to display the verification URL and code to the user. Fix 1 (pkg/auth/providers/aws/sso.go): - isInteractive() now respects --force-tty / ATMOS_FORCE_TTY via viper, allowing callers to explicitly override TTY detection when needed. Fix 2 (internal/exec/shell_utils.go): - ExecuteShellCommand now detects when the parent process has a real TTY on stderr and automatically injects ATMOS_FORCE_TTY=true into the subprocess environment when the variable is not already set. - This ensures workflow steps like `command: auth login` behave interactively when the user is running atmos from a terminal. Tests: - TestIsInteractive_ForceTTY: verifies force-tty viper key is respected - TestEnvKeyIsSet: table-driven tests for the new envKeyIsSet helper - TestExecuteShellCommand: two new subtests covering ATMOS_FORCE_TTY preservation and non-injection in non-TTY environments Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-actions · 2026-03-02T22:10:41Z

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

coderabbitai · 2026-03-02T22:27:39Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 717ff76 and f9befc1.

📒 Files selected for processing (3)

internal/exec/shell_utils_test.go
pkg/auth/providers/aws/sso_test.go
tests/snapshots/TestCLICommands_atmos_toolchain_info_tar.gz_format_tool.stderr.golden

🚧 Files skipped from review as they are similar to previous changes (2)

internal/exec/shell_utils_test.go
pkg/auth/providers/aws/sso_test.go

📝 Walkthrough

Walkthrough

Exec now propagates TTY state to subprocesses by injecting ATMOS_FORCE_TTY into merged env when the parent has a real TTY and the key isn't present; AWS SSO's isInteractive() can be forced via a viper force-tty setting. Tests and an env-key helper were added.

Changes

Cohort / File(s)	Summary
Shell execution TTY propagation `internal/exec/shell_utils.go`, `internal/exec/shell_utils_test.go`	ExecuteShellCommand merges envs and injects `ATMOS_FORCE_TTY=true` when parent is a real TTY and the key is not already present. Adds `envKeyIsSet` helper and tests for explicit-preserve, parent-env propagation, non-TTY behavior, and env-key detection.
AWS SSO TTY handling `pkg/auth/providers/aws/sso.go`, `pkg/auth/providers/aws/sso_test.go`	`isInteractive()` now returns true if viper `force-tty` is set; otherwise falls back to existing TTY detection. Added tests to validate the viper-driven override and adjusted imports.
Snapshots / CLI output `tests/snapshots/.../TestCLICommands_atmos_toolchain_info_tar.gz_format_tool.stderr.golden`	Updated available-versions display content (added version entry; removed "Screenshot" line).

Sequence Diagram(s)

sequenceDiagram
    participant Parent as Parent Process
    participant Exec as ExecuteShellCommand
    participant Sub as Subprocess
    Parent->>Exec: call ExecuteShellCommand(cmd, stepEnv)
    Exec->>Exec: merge envs (parent + stepEnv)
    Exec->>Exec: isTTY() -> true?
    alt parent has TTY and env key not set
        Exec->>Exec: add ATMOS_FORCE_TTY=true to env
    end
    Exec->>Sub: spawn subprocess with merged env
    Sub->>Sub: detects ATMOS_FORCE_TTY in env

sequenceDiagram
    participant SSO as AWS SSO module
    participant Config as Viper config
    participant Term as isTTY()
    SSO->>Config: read "force-tty"
    alt force-tty set
        Config-->>SSO: true
        SSO->>SSO: isInteractive() returns true
    else
        Config-->>SSO: not set
        SSO->>Term: call isTTY()
        Term-->>SSO: true/false
        SSO->>SSO: isInteractive() returns Term result
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Fix: atmos auth login "hangs" when run in make targets #1671: Overlapping changes to AWS SSO interactivity and TTY detection/overrides.

Suggested reviewers

osterman
Benbentwo

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main fix: propagating TTY state to subprocesses to enable SSO device flow in workflows, which directly addresses the root cause and primary objective of the PR.

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

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/sso-auth-login-workflow-tty

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

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

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

pkg/auth/providers/aws/sso_test.go (1)

818-819: Restore prior Viper state in cleanup (avoid cross-test coupling).

This test currently forces cleanup to false, which can mutate global config state for later tests. Restore the previous value instead.

Suggested patch

 func TestIsInteractive_ForceTTY(t *testing.T) {
 	// Test that force-tty viper setting overrides the TTY check.
+	prevForceTTY := viper.GetBool("force-tty")
 	viper.Set("force-tty", true)
-	t.Cleanup(func() { viper.Set("force-tty", false) })
+	t.Cleanup(func() { viper.Set("force-tty", prevForceTTY) })

 	result := isInteractive()
 	assert.True(t, result, "isInteractive should return true when force-tty is set")
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/auth/providers/aws/sso_test.go` around lines 818 - 819, The cleanup
currently unconditionally sets viper "force-tty" to false which mutates global
state; instead capture the prior value before changing it (e.g., prev :=
viper.Get("force-tty") or viper.GetBool("force-tty")), set
viper.Set("force-tty", true) for the test, and in the t.Cleanup restore the
original value using viper.Set("force-tty", prev) (or cast prev back to bool) so
the global Viper state is returned exactly to its prior value; update the code
around viper.Set and t.Cleanup in the aws SSO test to use this save-and-restore
pattern.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/exec/shell_utils_test.go`:
- Around line 625-626: The test currently sets ATMOS_FORCE_TTY to an empty
string (t.Setenv("ATMOS_FORCE_TTY", "")) which leaves the key present and yields
a false positive; replace those empty-set calls with removal of the env var
(e.g., call os.Unsetenv("ATMOS_FORCE_TTY") or ensure the env key is absent)
before invoking ExecuteShellCommand so the shell expansion
`${ATMOS_FORCE_TTY:-not-set}` truly falls back to the default; apply the same
fix for the other occurrences around lines 630-642 and keep assertions against
ExecuteShellCommand's output (behavior) rather than presence of the variable
(implementation).

---

Nitpick comments:
In `@pkg/auth/providers/aws/sso_test.go`:
- Around line 818-819: The cleanup currently unconditionally sets viper
"force-tty" to false which mutates global state; instead capture the prior value
before changing it (e.g., prev := viper.Get("force-tty") or
viper.GetBool("force-tty")), set viper.Set("force-tty", true) for the test, and
in the t.Cleanup restore the original value using viper.Set("force-tty", prev)
(or cast prev back to bool) so the global Viper state is returned exactly to its
prior value; update the code around viper.Set and t.Cleanup in the aws SSO test
to use this save-and-restore pattern.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 314b3d5 and d72b7a7.

📒 Files selected for processing (4)

internal/exec/shell_utils.go
internal/exec/shell_utils_test.go
pkg/auth/providers/aws/sso.go
pkg/auth/providers/aws/sso_test.go

internal/exec/shell_utils_test.go

…ar properly t.Setenv("ATMOS_FORCE_TTY", "") leaves the key present in the environment. envKeyIsSet then matches it and skips auto-injection — not because there is no TTY, but because the key exists. This caused ${VAR:-default} to expand to the fallback for the wrong reason (empty value, not absent key), masking the incorrect skip path. Replace both t.Setenv("ATMOS_FORCE_TTY", "") calls with os.LookupEnv + os.Unsetenv + t.Cleanup so the key is truly absent before the subprocess runs. Tests now verify the correct behavior: auto-injection is skipped because the test runner has no TTY, not because a stale empty value was present in the parent environment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

codecov · 2026-03-03T01:45:44Z

Codecov Report

❌ Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.55%. Comparing base (314b3d5) to head (f9befc1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/exec/shell_utils.go	75.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2126      +/-   ##
==========================================
+ Coverage   76.52%   76.55%   +0.02%     
==========================================
  Files         832      832              
  Lines       79441    79451      +10     
==========================================
+ Hits        60792    60820      +28     
+ Misses      14856    14837      -19     
- Partials     3793     3794       +1

Flag	Coverage Δ
unittests	`76.55% <81.81%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pkg/auth/providers/aws/sso.go	`51.80% <100.00%> (+0.24%)`	⬆️
internal/exec/shell_utils.go	`56.39% <75.00%> (+0.57%)`	⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2026-03-03T22:35:11Z

These changes were released in v1.208.0.

…raform-plan * osterman/native-ci-terraform: (28 commits) feat: Add source cache TTL for JIT-vendored components (#2138) feat: Per-target version overrides in vendor manifests (#2141) docs: Add PRD for browser-based auth in aws/user identity (#1887) docs: Add EKS kubeconfig authentication integration PRD (#1884) fix: correct marketplace.json schema and update docs with install/uninstall commands (#2142) fix: propagate auth to all YAML functions in multi-component execution (#2140) fix: Use atmos_component for source provisioner workdir paths (#2137) Fix identity prompts to respect --interactive flag (#2130) Increase PR size thresholds to accommodate AI-assisted development (#2136) docs: Add Azure authentication provider documentation (#2132) fix: propagate component-type level dependencies through stack processor (#2127) fix: Add retry and missing workflow step properties to all schema copies (#2113) Exclude unsupported windows/arm from goreleaser build matrix (#2133) Add AI Agent Skills for LLM-Powered Infrastructure Development (#2121) Fix: Convert toolchain paths to absolute in PATH to resolve exec.LookPath failures (#2095) Fix workdir collision for component instances sharing base component (#2093) fix(auth): propagate TTY state to subprocesses for SSO device flow in workflows (#2126) fix(security): prevent SSRF in GitHub OIDC token URL handling (CWE-918) (#2106) Fix #2112: add workflow_retry definition and retry property to workflow step schema (#2114) fix(auth): auto-detect GitHub Actions WIF with proper audience, host validation, and lazy GSM init (#2109) ...

github-actions · 2026-03-09T13:27:34Z

These changes were released in v1.208.1-test.9.

github-actions · 2026-03-09T17:35:16Z

These changes were released in v1.208.1-test.10.

Benbentwo requested a review from a team as a code owner March 2, 2026 22:09

github-actions bot added the size/m Medium size PR label Mar 2, 2026

Benbentwo added the patch A minor, backward compatible change label Mar 2, 2026

coderabbitai bot requested changes Mar 2, 2026

View reviewed changes

internal/exec/shell_utils_test.go Outdated Show resolved Hide resolved

coderabbitai bot previously approved these changes Mar 3, 2026

View reviewed changes

address comments, fix tests, increase test coverage

f9befc1

aknysh dismissed coderabbitai[bot]’s stale review via f9befc1 March 3, 2026 01:22

coderabbitai bot approved these changes Mar 3, 2026

View reviewed changes

aknysh approved these changes Mar 3, 2026

View reviewed changes

aknysh merged commit 6edec69 into main Mar 3, 2026
56 checks passed

aknysh deleted the fix/sso-auth-login-workflow-tty branch March 3, 2026 01:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(auth): propagate TTY state to subprocesses for SSO device flow in workflows#2126

fix(auth): propagate TTY state to subprocesses for SSO device flow in workflows#2126
aknysh merged 3 commits intomainfrom
fix/sso-auth-login-workflow-tty

Benbentwo commented Mar 2, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

github-actions bot commented Mar 2, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Mar 2, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

codecov bot commented Mar 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

github-actions bot commented Mar 3, 2026

Uh oh!

github-actions bot commented Mar 9, 2026

Uh oh!

github-actions bot commented Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Benbentwo commented Mar 2, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Tests

References

Summary by CodeRabbit

Uh oh!

github-actions bot commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Review

Scanned Files

Uh oh!

coderabbitai bot commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

github-actions bot commented Mar 3, 2026

Uh oh!

github-actions bot commented Mar 9, 2026

Uh oh!

github-actions bot commented Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Benbentwo commented Mar 2, 2026 •

edited by coderabbitai bot

Loading

github-actions bot commented Mar 2, 2026 •

edited

Loading

coderabbitai bot commented Mar 2, 2026 •

edited

Loading

codecov bot commented Mar 3, 2026 •

edited

Loading