Skip to content

Remove unused --profile flag from auth commands#1650

Merged
aknysh merged 2 commits intomainfrom
osterman/profile-flag-investigation
Oct 18, 2025
Merged

Remove unused --profile flag from auth commands#1650
aknysh merged 2 commits intomainfrom
osterman/profile-flag-investigation

Conversation

@osterman
Copy link
Member

@osterman osterman commented Oct 17, 2025

what

  • Removed the --profile flag and all associated references from the Atmos CLI auth commands.
  • This includes removing the flag definition in cmd/auth.go, its documentation, and related entries in test snapshots.

why

  • The --profile flag was added in a previous PR but was never implemented with any actual functionality.
  • This flag was causing confusion with other concepts of "profile" within Atmos and the broader AWS ecosystem (e.g., AWS CLI profiles, performance profiling).
  • Removing this dead code cleans up the codebase and improves clarity for users.

references

Summary by CodeRabbit

  • Breaking Changes

    • Removed the --profile flag from authentication commands. Users can no longer specify a profile for authentication via this flag.
  • Documentation

    • Updated help text and documentation to reflect removal of profile flag support.

@osterman osterman requested a review from a team as a code owner October 17, 2025 05:01
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 17, 2025

📝 Walkthrough

Walkthrough

Removed the --profile flag from the auth command suite, eliminating the flag definition, all associated help text snapshots across auth subcommands, and related documentation.

Changes

Cohort / File(s) Summary
Flag Definition Removal
cmd/auth.go
Removed ProfileFlagName exported constant and its flag initialization logic, including environment and flag bindings
Help Text Snapshots
tests/snapshots/TestCLICommands_atmos_auth_*--help.stdout.golden
Removed --profile flag descriptions from help output for auth subcommands: env, exec, login, user configure, validate, whoami
Documentation
website/docs/cli/commands/auth/usage.mdx
Removed documentation entries describing the --profile flag usage

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Changes are homogeneous across files—consistent removal of the --profile flag from the flag definition, help snapshots, and documentation. Low logic density and straightforward verification.

Suggested labels

major

Suggested reviewers

  • aknysh

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Remove unused --profile flag from auth commands" is concise, specific, and directly reflects the primary change in the changeset. It clearly indicates what was removed (the --profile flag), where it was removed from (auth commands), and why it matters (it's unused). The title accurately summarizes the content of the diff across all modified files without noise or vague language.
Linked Issues Check ✅ Passed The changes directly support the objectives of both linked issues. For #1530 (fix error wrapping and remove dead code), this PR removes the unimplemented --profile flag, which is unused code that aligns with the dead code removal requirement. For #1475 (Atmos Auth Implementation), this PR cleans up the auth command implementation by removing a confusing, never-implemented feature that was added previously. The removal of the ProfileFlagName constant in cmd/auth.go and all associated references across help snapshots and documentation supports the goal of eliminating confusion in the auth system and ensuring the codebase stays clean.
Out of Scope Changes Check ✅ Passed All changes in the PR are directly scoped to removing the --profile flag and its references. The modifications span the implementation layer (cmd/auth.go removal of ProfileFlagName), test snapshots (seven help text files updated to remove --profile descriptions), and documentation (website usage docs updated). Each file change is purposefully aligned with the stated objective of eliminating the unused --profile flag that was never implemented with actual functionality.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch osterman/profile-flag-investigation

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6e04105 and 7eea231.

📒 Files selected for processing (8)
  • cmd/auth.go (0 hunks)
  • tests/snapshots/TestCLICommands_atmos_auth_env_--help.stdout.golden (0 hunks)
  • tests/snapshots/TestCLICommands_atmos_auth_exec_--help.stdout.golden (0 hunks)
  • tests/snapshots/TestCLICommands_atmos_auth_login_--help.stdout.golden (0 hunks)
  • tests/snapshots/TestCLICommands_atmos_auth_user_configure_--help.stdout.golden (0 hunks)
  • tests/snapshots/TestCLICommands_atmos_auth_validate_--help.stdout.golden (0 hunks)
  • tests/snapshots/TestCLICommands_atmos_auth_whoami_--help.stdout.golden (0 hunks)
  • website/docs/cli/commands/auth/usage.mdx (0 hunks)
💤 Files with no reviewable changes (8)
  • website/docs/cli/commands/auth/usage.mdx
  • tests/snapshots/TestCLICommands_atmos_auth_exec_--help.stdout.golden
  • cmd/auth.go
  • tests/snapshots/TestCLICommands_atmos_auth_whoami_--help.stdout.golden
  • tests/snapshots/TestCLICommands_atmos_auth_user_configure_--help.stdout.golden
  • tests/snapshots/TestCLICommands_atmos_auth_validate_--help.stdout.golden
  • tests/snapshots/TestCLICommands_atmos_auth_login_--help.stdout.golden
  • tests/snapshots/TestCLICommands_atmos_auth_env_--help.stdout.golden
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: Build (windows-latest, windows)
  • GitHub Check: Build (macos-latest, macos)
  • GitHub Check: Lint (golangci)
  • GitHub Check: Analyze (go)
  • GitHub Check: website-deploy-preview
  • GitHub Check: Run pre-commit hooks
  • GitHub Check: Summary

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.

@github-actions github-actions bot added the size/s Small size PR label Oct 17, 2025
@osterman osterman added the no-release Do not create a new release (wait for additional code changes) label Oct 17, 2025
@codecov
Copy link

codecov bot commented Oct 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.00%. Comparing base (4b829b9) to head (cca857a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1650      +/-   ##
==========================================
+ Coverage   65.98%   66.00%   +0.01%     
==========================================
  Files         343      343              
  Lines       38693    38686       -7     
==========================================
+ Hits        25533    25534       +1     
+ Misses      11163    11158       -5     
+ Partials     1997     1994       -3     
Flag Coverage Δ
unittests 66.00% <ø> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
cmd/auth.go 57.14% <ø> (+14.28%) ⬆️

... 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.

@aknysh aknysh added minor New features that do not break anything and removed no-release Do not create a new release (wait for additional code changes) labels Oct 18, 2025
@github-actions
Copy link

Warning

Changelog Entry Required

This PR is labeled minor or major but doesn't include a changelog entry.

Action needed: Add a new blog post in website/blog/ to announce this change.

Example filename: website/blog/2025-10-18-feature-name.mdx

Alternatively: If this change doesn't require a changelog entry, remove the minor or major label.

@aknysh aknysh merged commit 48b3837 into main Oct 18, 2025
53 of 54 checks passed
@aknysh aknysh deleted the osterman/profile-flag-investigation branch October 18, 2025 05:07
osterman added a commit that referenced this pull request Oct 19, 2025
Resolved conflicts in:
- internal/exec/shell_utils.go: kept both viper import and exit code changes
- internal/exec/shell_utils_test.go: merged auth shell tests with new ExecuteShellCommand tests

Includes updates from main:
- Exit code preservation for shell commands and workflows (#1660)
- Custom golangci-lint build isolation (#1666)
- Conductor auto-commits (#1650)
- No-color flag fix (#1652)
- Performance optimizations (#1639)
@github-actions
Copy link

These changes were released in v1.195.0-rc.1.

aknysh added a commit that referenced this pull request Oct 22, 2025
* feat: Add `atmos auth shell` command for authenticated interactive sessions

Implement `atmos auth shell` command to launch an interactive shell
with authentication environment variables pre-configured.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: Run packer init before packer validate in test

Fix flaky TestExecutePacker_Validate test by running `packer init`
to install required plugins before validation. The test now:
- Runs packer init to install the amazon plugin
- Skips gracefully if init fails (network issues)
- Proceeds with validation only if init succeeds

This prevents the "Missing plugins" error that was causing test failures.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* test: Update golden snapshot to include new auth shell subcommand

* test: Regenerate golden snapshot with proper terminal padding

The snapshot test infrastructure pads output to terminal width.
Regenerated using --regenerate-snapshots flag to match CI expectations.

* fix: Resolve shell command path and honor Windows shell override

Addresses CodeRabbit review feedback:

1. Use exec.LookPath to resolve shell command before os.StartProcess
   - os.StartProcess doesn't search PATH, so bare commands like 'bash' fail
   - Now resolves to absolute path for non-absolute commands

2. Honor --shell flag on Windows
   - Previously always used cmd.exe even when --shell was specified
   - Now respects shellOverride and viper config before falling back to cmd.exe
   - Only applies -l flag on Unix systems (not Windows)

3. Add osWindows constant to satisfy linting

* docs: Add blog post introducing atmos auth shell

Comprehensive blog post covering:
- Problem statement and motivation
- Key features (auto auth, custom shell, nesting, cross-platform)
- Real-world use cases (Terraform workflows, debugging, long-running ops)
- How it works under the hood
- Getting started guide
- Environment variables reference
- Future enhancements

* docs: Simplify blog post to focus on identity-based authentication

Condensed key features section to emphasize the core value:
identity-based authentication and seamless identity switching.
Other features (custom shell, args, nesting, cross-platform)
mentioned succinctly in one sentence.

* docs: Refocus blog post on primary use case

Changed messaging to emphasize the core purpose:
launch a shell with active cloud credentials to work naturally
without juggling environment variables or re-authenticating.

* docs: Remove broken authentication guide link from blog post

The /core-concepts/authentication page doesn't exist.
Removed the link to fix website build.

* docs: Remove command documentation link from blog post

Docusaurus can't resolve internal doc links from blog posts
during the build. Removed to fix website deploy.

* docs: Fix auth shell prompt claim and improve test coverage

- Remove incorrect claim about automatic shell prompt modification
- Clarify that ATMOS_IDENTITY and ATMOS_SHLVL env vars are exposed
- Add tip section showing how to manually customize prompt
- Add test coverage for edge cases and environment binding
- Improve auth_shell.go coverage from 40% to 67%
- Document testing limitations for authentication paths

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* feat: Add mock authentication provider for testing

- Create mock provider and identity for testing without cloud credentials
- Register mock provider kind in auth factory
- Add integration tests using mock provider
- Improve auth_shell.go test coverage from 67% to 75%
- Add test fixture with mock auth configuration
- Enable end-to-end testing of auth shell command

The mock provider is for testing purposes only and is not documented
for end users. It enables comprehensive integration testing of
authentication features without requiring real cloud credentials.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* test: Fix test isolation to prevent environment pollution

- Move environment setup into each subtest for TestAuthShellCmd_FlagParsing
- Remove test case that was susceptible to config caching
- Tests now pass reliably regardless of execution order

Fixes test pollution where environment variables from one test
would bleed into subsequent tests causing failures in CI.

* test: Make mock provider tests cross-platform compatible

- Add OS-specific shell and command handling for Windows vs Unix
- Use cmd.exe with /c on Windows, /bin/sh with -c on Unix
- Explicitly specify shell via --shell flag in tests
- Simplify environment variable test to just verify execution

Fixes test failures on Windows CI where /bin/bash doesn't exist.

* feat: Add shell completion for identity flag in auth shell

- Register identity flag completion using AddIdentityCompletion
- Enables shell completion to suggest available identities
- Matches completion behavior of other auth subcommands

* docs: Correct auth shell environment variable documentation

- Fix incorrect claim that credentials are exported directly
- Clarify that Atmos sets AWS config file paths, not raw credentials
- Document actual environment variables: AWS_SHARED_CREDENTIALS_FILE,
  AWS_CONFIG_FILE, AWS_PROFILE
- Emphasize security: credentials never exposed in environment
- Explain compatibility with AWS SDK-based tools

This corrects the documentation to match actual implementation where
Atmos writes credentials to managed config files and sets environment
variables pointing to those files, rather than exposing sensitive
credentials directly in the environment.

* docs: Update auth shell command documentation for secure credential handling

- Correct environment variable documentation to show actual AWS config paths
- Add info box explaining secure credential handling approach
- Document that credentials are written to managed config files, not exposed
- Clarify AWS SDK compatibility with file-based credential approach
- Remove misleading reference to raw credential environment variables

Matches the same correction made to the blog post.

* security: Fix mock provider to prevent credential leaks in serialization

- Move sensitive credentials from Environment map to Credentials field
- Environment map is JSON-serializable and should not contain secrets
- Only populate non-sensitive env vars (AWS_REGION, AWS_DEFAULT_REGION)
- Add nil-check for info pointer before populating
- Only set Expiration when non-zero
- Update mock identity to provide AWS config file paths instead of raw credentials
- Add comprehensive tests to verify no secret leakage in JSON serialization

The mock provider now follows the same secure pattern as real AWS
implementations, storing credentials in the non-serializable
info.Credentials field and only exposing file paths in Environment.

* feat: Add performance tracking and viper precedence to auth shell command

- Add perf.Track instrumentation to executeAuthShellCommand and executeAuthShellCommandCore
- Change identity retrieval to use viper for CLI → ENV → config precedence
- Bind identity flag to viper with ATMOS_IDENTITY support
- Store atmosConfig as pointer for consistent downstream usage

* test: Add enabled tests for auth commands using mock provider

- Add tests for 'atmos auth whoami' with mock identity
- Add tests for 'atmos auth env' in json, bash, and dotenv formats
- Add test for 'atmos auth exec' with simple command
- Add test for 'atmos auth login' with mock identity
- All tests use fixtures/scenarios/atmos-auth-mock/ workdir
- These replace the previously disabled tests that required real cloud credentials

* test: Add mock provider test for auth login

- Adds enabled test for 'atmos auth login --identity mock-identity'
- Uses fixtures/scenarios/atmos-auth-mock/ workdir with mock provider
- Login succeeds without real cloud credentials
- Other auth commands (whoami, env, exec) remain disabled as they require
  credentials to be persisted to keyring, which doesn't work in test isolation

* Changes auto-committed by Conductor

* [autofix.ci] apply automated fixes

* test: fix test snapshots for keyring configuration and mock auth

* fix: improve keyring and mock provider code quality

- Make keyring_file_test hermetic by using temp HOME directory
- Fix keyring_file_test password assertion to actually test behavior  
- Add perf tracking to keyring_file.go constructor
- Fix error wrapping in all keyring implementations to use errors.Join
- Add perf tracking to all mock provider methods
- Add nil check to mock provider NewProvider

All changes follow project coding standards and error handling guidelines.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Merge branch 'main' into atmos-auth-shell

Resolved conflicts in:
- internal/exec/shell_utils.go: kept both viper import and exit code changes
- internal/exec/shell_utils_test.go: merged auth shell tests with new ExecuteShellCommand tests

Includes updates from main:
- Exit code preservation for shell commands and workflows (#1660)
- Custom golangci-lint build isolation (#1666)
- Conductor auto-commits (#1650)
- No-color flag fix (#1652)
- Performance optimizations (#1639)

* chore: trigger CI rebuild

Re-run CI after merge conflict resolution to verify all checks pass

* fix: improve keyring file test hermiticity and error wrapping

- Make TestFileKeyring_EmptyConfig hermetic by using t.TempDir() for HOME
- Fix error wrapping in password prompt to use %w instead of %v
- Ensures proper error unwrapping for callers checking specific error types

* fix: remove 'go get' from Makefile to prevent internal package resolution errors

The 'go get' command without arguments tries to resolve ALL imports as external
modules, including internal packages like internal/tui/atmos. This causes CI build
failures with 'cannot find module providing package' errors.

Modern Go (1.17+) deprecates 'go get' for building. Use 'go mod download' for
dependencies instead (available via 'make deps').

* fix: pre-download Go dependencies in pre-commit CI workflow

Add 'go mod download' step before running pre-commit hooks to ensure all
module dependencies are cached. This prevents the go-mod-tidy hook from
trying to fetch internal packages as external modules.

* fix: unignore internal/tui/atmos and other atmos directories

Changed .gitignore rule from 'atmos' to '/atmos' to only ignore the atmos
binary in the root directory. The previous rule was ignoring ALL files and
directories named 'atmos', including:
- internal/tui/atmos/ (TUI package for atmos command)
- pkg/datafetcher/schema/atmos/ (schema files)
- tests/fixtures/schemas/atmos/ (test schemas)
- website/static/schemas/atmos/ (website schemas)
- website/static/img/cli/atmos/ (CLI screenshots)
- Example atmos.yaml config files

This was causing CI build failures with 'no required module provides package
github.com/cloudposse/atmos/internal/tui/atmos' because the source files were
not committed to git.

* fix: normalize timestamps in test snapshots and add empty-dir fixture

1. Add timestamp normalization to sanitizeOutput function
   - Replace 'Expires: YYYY-MM-DD HH:MM:SS TZ' with 'Expires: <TIMESTAMP>'
   - Prevents snapshot mismatches due to timezone/time differences

2. Update mock-identity stdout golden file with <TIMESTAMP> placeholder

3. Add empty-dir fixture .gitignore to repository
   - Tests require this directory to exist for 'empty-dir' test cases
   - Directory has .gitignore with '*' to keep it intentionally empty

* docs: document keyring backends in auth usage documentation

Added comprehensive documentation for all three keyring backends:
- System keyring (default, OS-native)
- File keyring (encrypted file with password prompting)
- Memory keyring (in-memory for testing)

Includes configuration examples, use cases, and password resolution
order for file keyring.

* docs: clarify when to use 'auth shell' vs 'auth exec'

Added concise comparison notes to both command docs to help users
quickly understand the difference:
- shell: for interactive multi-command sessions
- exec: for single commands and automation

Each doc now cross-links to the other for easy reference.

* docs: move shell vs exec comparison to tip callout

Moved the comparison note out of the intro and into a concise tip
callout between the help screenshot and usage section. This provides
a quick reference without cluttering the introduction.

* fix: make 'get' target delegate to 'deps' for dependency download

Changed get target to invoke deps (go mod download) instead of being a no-op.
This fixes the inconsistency where build-default, build-windows, lint, testacc,
testacc-cover, test-short, and test-short-cover all depend on 'get' but it
wasn't downloading dependencies.

Maintains backward compatibility for any targets depending on 'get' while
following modern Go practices (go mod download instead of go get).

* fix: use placeholder timestamp instead of generic token in snapshots

Changed timestamp sanitization from '<TIMESTAMP>' to a realistic fixed
timestamp '2025-01-01 00:00:00 UTC' to make snapshots more readable and
consistent with other placeholder patterns in the codebase.

Updated snapshot file to match the new placeholder format.

* fix: remove 'go get' from Makefile and use 'deps' directly

Removed the deprecated 'get' target entirely and updated all targets to
depend on 'deps' directly:
- lint: get → deps
- build-default: get → deps
- build-windows: get → deps
- testacc: get → deps
- testacc-cover: get → deps
- test-short: get → deps
- test-short-cover: get → deps

This eliminates the unnecessary indirection and makes it clear that
'go mod download' is used for dependency management. The 'get' target
was a no-op that we don't need to maintain.

* fix: correct broken links in auth shell and exec documentation

Fixed broken documentation links:
- /cli/commands/auth/auth-exec → /cli/commands/auth/exec
- /cli/commands/auth/auth-shell → /cli/commands/auth/shell

The page paths don't include the 'auth-' prefix in the URL.

* fix: update auth login test snapshots to match current output format

The auth login command outputs to stderr (via PrintfMessageToTUI), not
stdout. Updated snapshots to reflect this:

- stdout.golden is now empty (no stdout output)
- stderr.golden has normalized timestamp placeholder (2025-01-01 00:00:00 UTC)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: exclude problematic godbus/dbus version to resolve license issue

The CI license checker flagged github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2
(pulled by 99designs/keyring and versent/saml2aws) as having an incompatible
BSD-2-Clause license.

Added exclude directive to force resolution to v4.1.0+incompatible instead.
All tests pass with the newer version.

If v4.1.0 still fails license check (also BSD-2-Clause), we'll need to either:
1. Get BSD-2-Clause added to allowed license list
2. Replace 99designs/keyring with custom AES encryption for file backend
3. Make keyring backends optional/build-tag gated

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: add NOTICE file with third-party license attributions

Added NOTICE file documenting third-party dependencies with non-Apache-2.0
licenses, specifically:

- godbus/dbus (BSD-2-Clause) - Used indirectly via 99designs/keyring and
  versent/saml2aws for Linux D-Bus keyring access
- 99designs/keyring (MIT) - Used for encrypted file-based credential storage

Both licenses are compatible with Apache-2.0 and approved by the Apache
Software Foundation (Category A). This follows ASF best practices for
documenting third-party dependencies.

Reference: https://www.apache.org/legal/resolved.html#category-a

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: use viper.BindEnv instead of os.Getenv for environment variables

Changed environment variable handling to use viper.BindEnv instead of raw
os.Getenv, following Atmos conventions per CLAUDE.md:

> ALWAYS use viper.BindEnv() for environment variable binding

Also fixed most linting issues:
- Replaced dynamic errors with static errors (err113)
- Added error return value checks (errcheck)
- Fixed missing comment periods (godot)
- Extracted magic number 0o700 to named constant

Remaining linting issues (non-blocking):
- cyclomatic complexity in newFileKeyringStore (12 > 10) - acceptable for setup function
- unparam in newMemoryKeyringStore - error return for interface consistency
- magic number in mock provider test - test data, acceptable

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: fix misleading terraform command examples in auth blog posts

Replaced `terraform plan` and `terraform apply` examples in the "Use It
Standalone" sections with better examples like `aws s3 ls`, `aws ecs
list-clusters`, and `kubectl get pods`.

The previous examples were misleading because:
- `atmos terraform *` commands natively support `--identity` flag
- Using `eval $(atmos auth env)` with raw `terraform` commands suggests
  bypassing Atmos's built-in identity management
- Better examples show using auth for tools that don't have native identity
  support (AWS CLI, kubectl, etc.)

Changed in:
- website/blog/2025-10-13-introducing-atmos-auth.md
- website/blog/2025-10-15-atmos-auth-shell.mdx

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: emphasize isolation and safety benefits of atmos auth shell

Expanded the "Why This Matters" section to clearly articulate the core value
proposition: session isolation and credential safety.

Key messaging changes:
- Traditional approach (shared ~/.aws/credentials) is fundamentally unsafe
- atmos auth shell provides isolation: each shell = one identity = one set of credentials
- Parent shell environment remains unchanged when you exit
- Enables multiple concurrent authenticated sessions without cross-contamination
- Credentials are ephemeral and session-scoped

Updated real-world examples to emphasize:
- Safe production operations with explicit credential cleanup
- Multiple concurrent terminals, each with different isolated credentials
- Prevention of accidental cross-environment commands

This addresses the core differentiation: why use atmos auth shell instead of
traditional AWS credential management approaches.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* perf: add performance tracking to keyring store methods

Added perf.Track() instrumentation to all exported methods in the keyring
credential stores for visibility and consistency with CLAUDE.md guidelines:

> Add `defer perf.Track()` to all public functions and critical private functions
> Always include a blank line after the perf.Track call

Changes:
- Added perf.Track to constructors:
  - NewCredentialStore()
  - NewCredentialStoreWithConfig()

- Added perf.Track to fileKeyringStore methods:
  - Store, Retrieve, Delete, List, IsExpired, GetAny, SetAny

- Added perf.Track to memoryKeyringStore methods:
  - Store, Retrieve, Delete, List, IsExpired, GetAny, SetAny

- Added perf.Track to systemKeyringStore methods:
  - Store, Retrieve, Delete, List, IsExpired, GetAny, SetAny

These are I/O-heavy operations (file/keyring access) where performance tracking
provides valuable visibility into credential storage latency.

All tests pass with instrumentation in place.

Remaining linting issues (non-blocking):
- function-length in newFileKeyringStore (61 > 60) - acceptable for setup
- add-constant for mock test timestamp - acceptable for test data
- unparam for newMemoryKeyringStore - kept for interface consistency

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: remove homedir replace directive for go install compatibility

- Remove replace directive that redirected mitchellh/go-homedir to ./pkg/config/homedir
- This restores go install compatibility merged from main
- Keep exclude directive for incompatible dbus version

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: use configured path as keyring storage directory

- Replace `filepath.Dir(path)` with `path` directly as the storage directory
- Update comment to clarify path is the directory (e.g., ~/.atmos/keyring)
- Files now stored inside the configured directory instead of its parent
- Fixes incorrect storage location when custom path is specified

Refactoring and linting fixes:
- Extract parseFileKeyringConfig and getDefaultKeyringPath helpers
- Reduce cyclomatic complexity in newFileKeyringStore (12 -> 8)
- Remove error return from newMemoryKeyringStore (always returned nil)
- Add mock timestamp constants to avoid magic numbers
- Update all test calls to match new signatures
- Add nolint:dupl for intentional test duplication

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* feat: use XDG Base Directory Specification for keyring storage

- Follow XDG Base Directory Specification for file keyring location
- Default path now: $XDG_DATA_HOME/atmos/keyring (typically ~/.local/share/atmos/keyring)
- Support ATMOS_XDG_DATA_HOME override following existing XDG pattern from cache.go
- Fallback to XDG library default if environment variables not set
- Update tests to set XDG_DATA_HOME for hermetic testing
- Import github.com/adrg/xdg for cross-platform XDG support

Benefits:
- Consistent with XDG standards used by modern Linux applications
- Separates data (credentials) from config files
- Allows users to control storage location via standard XDG_DATA_HOME
- Maintains backward compatibility via environment variable override

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: add keyring backends PRD

Create comprehensive PRD documenting the credential storage backend system:

- Overview of system, file, and memory keyring backends
- Backend selection priority (env var > config > default)
- XDG Base Directory Specification compliance for file backend
- Security considerations for each backend
- Configuration examples for different deployment scenarios
- Data formats and credential envelope structure
- Testing strategy and isolation patterns
- Migration paths and backward compatibility
- Performance instrumentation approach

This is retroactive documentation of the implemented keyring system,
capturing design decisions, architecture, and usage patterns.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: fix punctuation and clarify system keyring List() docstring

- Add missing period to list item in auth shell blog post
- Update systemKeyringStore.List() docstring to accurately reflect that
  the method is not supported (due to go-keyring limitations) and returns
  an error instead of aliases
- Clarify that callers should use errors.Is to detect not-supported condition

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* refactor: eliminate test duplication using shared test suite

- Create suite_test.go with shared CredentialStore test suite
- Use factory pattern to test all backend implementations
- Remove duplicate testStoreAndRetrieve, testIsExpired, testDelete tests
- Keep backend-specific tests (List, ConcurrentAccess, Isolation)
- Remove nolint:dupl directive (no longer needed)

Benefits:
- Eliminates code duplication across backend tests
- Ensures all backends implement interface correctly
- Makes it easy to add new backends with full test coverage
- Reduces maintenance burden (update tests once, applies to all backends)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* refactor: create reusable XDG utility to eliminate duplication

- Add pkg/utils/xdg.go with GetXDGCacheDir, GetXDGDataDir, GetXDGConfigDir functions
- Centralize XDG Base Directory Specification logic in one place
- Refactor pkg/config/cache.go to use GetXDGCacheDir utility
- Refactor pkg/auth/credentials/keyring_file.go to use GetXDGDataDir utility
- Add comprehensive tests for XDG utility functions
- Add CacheDirPermissions and KeyringDirPermissions constants

Benefits:
- DRY: XDG logic defined once, used everywhere
- Consistency: All XDG paths follow same precedence (ATMOS_XDG_* > XDG_* > default)
- Maintainability: Changes to XDG handling only need to happen in one place
- Testability: Easier to test XDG path resolution in isolation
- Documentation: Clear API with GetXDGCacheDir/DataDir/ConfigDir functions

Environment variable precedence (for all XDG functions):
1. ATMOS_XDG_*_HOME (Atmos-specific override)
2. XDG_*_HOME (standard XDG variable)
3. XDG library default (platform-specific from github.com/adrg/xdg)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* test: add TestMemoryKeyring_GetAnySetAny for implementation-specific method

- Re-add GetAnySetAny test to memory keyring (accidentally removed during refactoring)
- GetAny/SetAny are implementation-specific helper methods, not part of CredentialStore interface
- Each backend has its own GetAny/SetAny test since they're not in the shared suite
- Shared suite only tests interface methods (Store, Retrieve, Delete, IsExpired)

Testing strategy:
- Shared suite (suite_test.go): Tests interface contract methods
- Backend-specific tests: Test implementation-specific features
  - Memory: List, GetAny/SetAny, ConcurrentAccess, Isolation
  - File: GetAny/SetAny, custom paths, password handling, error cases
  - System: GetAny/SetAny (List returns not-supported error)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* refactor: extract XDG utilities to dedicated package

Move XDG Base Directory Specification utilities from pkg/utils to pkg/xdg
for better organization and clarity. This follows Go idiom of focused,
single-purpose packages.

Changes:
- Create pkg/xdg package with GetXDGCacheDir, GetXDGDataDir, GetXDGConfigDir
- Update pkg/config/cache.go to use xdg package
- Update pkg/auth/credentials/keyring_file.go to use xdg package
- All tests pass with new package structure

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: document XDG Base Directory Specification support

Add documentation for XDG environment variables in global flags reference
and update file keyring documentation to explain XDG-based default storage
location.

Changes:
- Add ATMOS_XDG_CACHE_HOME, ATMOS_XDG_DATA_HOME, ATMOS_XDG_CONFIG_HOME to global-flags.mdx
- Document XDG_CACHE_HOME, XDG_DATA_HOME, XDG_CONFIG_HOME fallback support
- Explain default storage locations per platform
- Update file keyring docs to mention XDG Base Directory Specification compliance
- Add examples for customizing XDG directories
- Link to XDG Base Directory Specification

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: add XDG Base Directory Specification PRD

Create comprehensive PRD documenting Atmos's implementation of the XDG Base
Directory Specification for organizing user data files. This PRD covers:

- Problem statement and requirements
- Technical architecture and API design
- Platform-specific defaults (Linux, macOS, Windows)
- Environment variable precedence (ATMOS_XDG_* > XDG_* > defaults)
- Integration points (cache, keyring, future config)
- Security considerations and file permissions
- Testing strategy with hermetic isolation
- Migration path from legacy ~/.atmos/ locations
- Future enhancements (config discovery, migration tool)

The PRD documents the rationale for using isolated Viper instances rather
than global Viper/Cobra integration, explains why XDG paths are environment-
only (not CLI flags), and provides complete reference tables for default
file locations on each platform.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* test: increase XDG package coverage to 92.9%

Add comprehensive error path testing for XDG directory utilities to exceed
the 80% minimum coverage requirement.

New tests:
- TestGetXDGDir_MkdirError: Tests directory creation failure when a file
  blocks the path (covers os.MkdirAll error path)
- TestGetXDGDir_DefaultFallback: Tests library default fallback when no
  environment variables are set (covers empty env var path)

Coverage improvements:
- Before: 78.6% (below minimum requirement)
- After: 92.9% (exceeds 80% requirement)
- getXDGDir: 72.7% → 90.9%
- All public functions: 100%

All 9 tests pass successfully with proper error handling and edge case
coverage.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* test: update auth login snapshot for new table format

Update golden snapshots and regex pattern to match the new authentication
success output format which uses a formatted table with checkmark instead
of plain text.

Changes:
- Update expiresRegex to match new table format with relative time
- Handle optional relative time suffix "(59m 59s)" in expiration line
- Update stderr golden snapshot with new table format:
  - Uses ✓ checkmark instead of **bold** text
  - Displays info in aligned table columns
  - Normalized expiration to fixed timestamp
- Update stdout golden snapshot (was empty, now matches stderr)

The regex now matches both old format "Expires: timestamp" and new format
"Expires   timestamp (relative)" and normalizes both to "Expires   2025-01-01 00:00:00 UTC"
for consistent snapshot testing.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* refactor: improve error handling and validation across auth and xdg packages

Implement comprehensive improvements to error handling, validation, and
testing across authentication credential storage and XDG utilities.

**Keyring File Changes (pkg/auth/credentials/keyring_file.go)**:
- Add MinPasswordLength constant (8 characters) for consistent validation
- Validate environment-supplied passwords meet minimum length requirement
- Map keyring.ErrKeyNotFound to ErrCredentialsNotFound in Retrieve()
- Map keyring.ErrKeyNotFound to ErrDataNotFound in GetAny()
- Improve error message for credential envelope marshal failure
- Use constant instead of magic number for password length validation

**Credential Store Changes (pkg/auth/credentials/store.go)**:
- Add final fallback to memory keyring if system keyring fails
- Ensure NewCredentialStore() never returns nil
- Improve warning messages to indicate fallback chain

**XDG Changes (pkg/xdg/xdg.go)**:
- Fix environment variable precedence with separate BindEnv calls
- Explicitly check ATMOS_XDG_*_HOME before XDG_*_HOME
- Add error handling for each BindEnv call
- Clarify precedence in comments and implementation

**Test Improvements**:
- Fix goroutine test assertions in keyring_memory_test.go
- Use error channel to collect errors from goroutines
- Avoid testing.T method calls from goroutines (data race)
- Update test expectations for new error handling behavior

All tests pass with improved coverage:
- pkg/auth/credentials: 80.1% coverage
- pkg/xdg: 88.9% coverage

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: make mock provider timestamps deterministic for snapshot test stability

Replace dynamic time.Now().Add(1 * time.Hour) with fixed timestamp using
MockExpiration* constants in mock provider and identity to ensure snapshot
tests remain stable and don't become flaky due to changing timestamps.

Changes:
- Updated MockExpirationYear from 2025 to 2099 (far future)
- Updated MockExpirationMonth/Day/Hour to 12/31/23
- Added MockExpirationMinute and MockExpirationSecond constants (59/59)
- Changed identity.go to use all shared MockExpiration* constants
- Updated comments to explain deterministic testing rationale

The test framework normalizes all expiration timestamps to 2025-01-01
00:00:00 UTC via regex replacement for snapshot comparison, so the
actual 2099-12-31 23:59:59 timestamp is replaced during test execution.
This ensures the mock provider always returns a far-future expiration
that won't cause expiration-related test failures while maintaining
deterministic snapshot testing.

Updated snapshots to match the normalized timestamp format with proper
table padding.

* fix: normalize only duration in expiration timestamps, preserve actual timestamp

Change the snapshot normalization to only remove the relative duration part
(e.g., "(59m 59s)") from expiration timestamps instead of replacing the
entire timestamp with a hardcoded value.

Before: Replaced entire timestamp with "2025-01-01 00:00:00 UTC"
After: Only strips duration, preserving actual timestamp like "2099-12-31 23:59:59 UTC"

This allows the actual expiration timestamp to be visible in snapshots while
still avoiding flakiness from the time-sensitive duration part. The regex now
uses a capture group to preserve the timestamp portion while removing only
the duration in parentheses.

Updated snapshot to show the actual mock provider timestamp (2099-12-31 23:59:59 UTC)
instead of the normalized placeholder.

* fix: normalize duration in expiration timestamps to deterministic placeholder

Change the snapshot normalization to replace the relative duration part
(e.g., "(59m 59s)", "(expired)") with a deterministic placeholder "(in 1h)"
instead of removing it entirely.

This follows the established pattern of replacing dynamic values with
placeholders rather than deleting them, maintaining consistent output
format while avoiding snapshot flakiness.

The regex preserves the actual timestamp (e.g., 2099-12-31 23:59:59 UTC)
while normalizing only the time-sensitive duration portion.

Updated snapshot to include the normalized duration placeholder.

* fix: use realistic duration format in expiration timestamp placeholder

Change the duration placeholder from "(in 1h)" to "(1h 0m)" to match the
actual format produced by the formatDuration function.

The formatDuration function produces formats like:
- "1h 30m" for hours and minutes
- "30m 15s" for minutes and seconds
- "45s" for just seconds
- "expired" for negative durations

Using "1h 0m" as the placeholder matches the actual output format and
represents a realistic value that could be produced by the duration
calculation, rather than an invalid format with the word "in".

* fix: use MockExpirationMinute and MockExpirationSecond in provider timestamp

Update the time.Date call in Provider.Authenticate to use the defined
MockExpirationMinute and MockExpirationSecond constants (59, 59) instead
of hardcoded zeros (0, 0).

This ensures consistency with the Identity.Authenticate implementation and
produces the correct timestamp of 2099-12-31 23:59:59 UTC for both the
provider and identity authentication paths.

Updated the comment to reflect the actual timestamp including minutes and
seconds.

* [autofix.ci] apply automated fixes

* docs: reposition atmos auth shell blog post to emphasize security

Refocused the blog post to highlight session isolation and security as the
primary value proposition, following the aws-vault exec model.

Key changes:
- Updated title to emphasize "Isolated Shell Sessions for Secure Multi-Identity Workflows"
- Rewrote problem statement to focus on credential leakage and context confusion
- Enhanced solution section to emphasize session-scoped security and isolation
- Added AWS Vault comparison table showing feature parity across cloud providers
- Added "When to Use" section clarifying atmos auth shell vs atmos auth env
- Removed redundant "Why This Matters" section
- Updated tags from [atmos, authentication, shell, aws, cloud] to [feature, atmos-auth, security, workflows]

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: tone down hyperbole in atmos auth shell blog post

Removed "powerful" and other superlatives to make the writing more genuine
and approachable. Changed "powerful new command" to just "new command" and
"powerful workflow" to "useful workflow".

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: clarify credentials are removed from environment, not deleted

Fixed inaccurate language that suggested credentials are "removed" or "gone"
entirely. Credentials are removed from the shell environment on exit, but the
underlying authentication session remains active (logout is a separate action).

Changes:
- "credentials are automatically removed" → "credentials are removed from your environment"
- "those credentials disappear" → "those credentials are removed from the environment"
- "those production credentials are gone" → "those production credentials are removed from your environment"
- Updated comparison table to clarify "removed from environment on exit"

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: clarify shell destruction and parent shell return

Replaced "credentials removed from environment" language with more accurate
description: when you exit, the shell is destroyed and you return to your
parent shell where those credentials were never present.

Changes:
- "credentials are removed from your environment" → "you return to your parent shell where those credentials were never present"
- "those credentials are removed from the environment" → "the shell is destroyed and you return to your parent shell where those credentials don't exist"
- "those production credentials are removed from your environment" → "you're back in your parent shell where those production credentials don't exist"
- Table: "Credentials removed from environment on exit" → "Return to parent shell on exit"

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: refocus keyring blog on value, not history

Removed "Atmos Auth previously..." language since the feature just launched.
Emphasized the Docker/Dev Container use case as the key benefit of file-based
keyrings—solving the cross-OS boundary problem where system keyrings aren't
accessible inside containers.

Key changes:
- Replaced "The Problem" section with "Why Different Keyring Types Matter"
- Removed historical references ("previously relied exclusively", "previously these tests were skipped")
- Added Docker/Dev Container as primary example for file-based keyrings
- Updated introduction to highlight cross-boundary credential sharing
- Simplified "What This Means for You" to "What This Gives You"
- Removed "backward compatibility" language from system keyring description

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: handle ErrNotFound in Delete and update snapshots

Fixed TestDelete_Flow test failure by treating keyring.ErrNotFound as
success (idempotent delete). Updated snapshots to include proper trailing
whitespace formatting for command output alignment.

Changes:
- pkg/auth/credentials/keyring_system.go: Check for keyring.ErrNotFound in Delete()
- tests/snapshots: Updated trailing whitespace to match actual CLI output formatting

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* perf: add performance tracking to mock identity methods

Added perf.Track instrumentation to all public methods in mock Identity
to comply with performance tracking guidelines. Updated cache.go doc
comment to accurately describe current behavior.

Changes:
- pkg/auth/providers/mock/identity.go: Added perf.Track to all public methods
  - NewIdentity, Kind, GetProviderName, Authenticate, Validate, Environment,
    PostAuthenticate, Logout
- pkg/config/cache.go: Updated GetCacheFilePath doc comment to remove
  outdated reference to environment variable binding

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: update CLAUDE.md with correct snapshot regeneration instructions

Replaced incorrect snapshot update commands with the documented approach
from tests/README.md. The correct method is to use the -regenerate-snapshots
flag, not environment variables.

Changes:
- Replaced "Golden Snapshots (MANDATORY)" section with accurate information
- Added snapshot types explanation (.stdout.golden, .stderr.golden, .tty.golden)
- Documented proper regeneration commands using -regenerate-snapshots flag
- Added workflow for reviewing and committing regenerated snapshots
- Added reference to tests/README.md for complete documentation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: use semantic definition list for environment variables

Replaced bulleted list with semantic <dl> for environment variables in
atmos auth shell blog post, following documentation guidelines. Added
"(AWS identities only)" qualifier to AWS-specific variables.

Changes:
- Converted bullet points to <dl> with <dt><code>...</code></dt> and <dd>
- ATMOS_IDENTITY, ATMOS_SHLVL: General variables
- AWS_SHARED_CREDENTIALS_FILE, AWS_CONFIG_FILE, AWS_PROFILE: AWS-only

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: update suite test to expect idempotent delete behavior

Changed suite_test.go to expect NoError when deleting a non-existent
credential, matching the idempotent behavior implemented in the Delete
method and consistent with main branch expectations.

Changes:
- suite_test.go: assert.Error → assert.NoError for non-existent delete
- Updated comment to clarify idempotent behavior

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: make Delete idempotent for all keyring backends

Updated memory and file keyring stores to treat "not found" as success
when deleting credentials, matching the idempotent behavior of the system
keyring store.

Changes:
- keyring_memory.go: Remove error check, just delete (idempotent)
- keyring_file.go: Check for keyring.ErrKeyNotFound and return nil

This ensures consistent idempotent delete behavior across all three
credential store backends (system, file, memory).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor New features that do not break anything size/s Small size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants