feat: Add GitHub Actions format to atmos auth env#1984
Merged
Conversation
Dependency Review✅ No vulnerabilities or license issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds GitHub Actions format support to Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI Command
participant Cmd as cmd/auth_env
participant Output as pkg/env/output
participant Fmt as env/format
participant GitHub as GitHub Actions
participant File as File System
CLI->>Cmd: Execute auth env --format=github
Cmd->>Cmd: Parse flags via Viper
Cmd->>Output: Output(envVars, "github", outputFile?, opts)
Output->>Output: Check if format is github & no outputFile
alt GitHub format with auto-detection
Output->>Output: actions.GetEnvPath() → $GITHUB_ENV
Output->>File: Append to detected GITHUB_ENV file
else GitHub format with explicit file
Output->>File: Append to specified outputFile
else Other formats
Output->>Fmt: FormatData(envVars, format)
Fmt-->>Output: Formatted output
Output->>File: writeToFileWithMode or stdout
end
File-->>GitHub: Environment variables available
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Contributor
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@cmd/auth_env_test.go`:
- Around line 388-555: Add a RunE‑level test that uses t.Setenv("GITHUB_ENV",
filePath) to verify auto‑detect and append behavior: create a temp file with
initial content, call the CLI entrypoint (invoke the command's RunE or root
command Execute so the code path that would call
writeAuthEnvToFile/formatAuthGitHub runs), then read the file and assert it was
appended to (not overwritten) and that multiline values are formatted the same
as writeAuthEnvToFile when using formatAuthGitHub; use t.Setenv for
setup/teardown and reuse writeAuthEnvToFile and formatAuthGitHub as references
to expected formatting.
In `@cmd/auth_env.go`:
- Around line 21-30: The default file mode constant defaultAuthFileMode
currently uses 0o644 which leaves created auth files world-readable; change
defaultAuthFileMode to an owner-only permission (e.g., 0o600) and update its
comment to reflect owner-only access so newly created credential files are not
world-readable; ensure any code that relies on defaultAuthFileMode (e.g., file
creation or write routines) continues to use this constant.
- Around line 228-239: In formatAuthGitHub, the heredoc delimiter
ATMOS_EOF_<key> can collide with the value; change the logic to generate a
delimiter that is guaranteed not to appear in the value (e.g., append a
random/unique token or UUID to the delimiter and retry if
strings.Contains(value, delimiter) is true) and then use that safe delimiter for
both the opening marker and closing marker; update the heredoc construction in
formatAuthGitHub to use the computed safe delimiter and ensure the containment
check uses that same delimiter before writing the heredoc.
- Around line 40-48: The current code reads flags directly using
cmd.Flags().GetString for "format" and "output" which bypasses Viper bindings
and ignores ATMOS_AUTH_ENV_* env overrides; replace those reads with
viper.GetString("auth_env_format") and viper.GetString("auth_env_output")
(preserving the validation against SupportedFormats and the error return using
errUtils.ErrInvalidArgumentError) so the values bound in init() are honored
(refer to the format/output variables and the SupportedFormats check in the
auth_env command).
🧹 Nitpick comments (1)
cmd/auth_env.go (1)
272-288: Consider migrating new flag bindings to StandardParser.This command still wires flags via
viper.BindEnv/BindPFlag. The repo convention isflags.NewStandardParser()to avoid manual binding and keep precedence behavior consistent. As per coding guidelines, follow the standard parser pattern for cmd flags.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/auth_env.go (1)
277-287: Refactor to useflags.NewStandardParser()instead of direct Viper binds.The direct
viper.BindEnv()andviper.BindPFlag()calls in init() violate the mandatory StandardParser pattern. Refactor the command to define flags viaStandardParserwith.WithStringFlag(), register them viaparser.RegisterFlags(), and bind them in RunE viaparser.BindFlagsToViper()(seecmd/toolchain/get.gofor the pattern).
🤖 Fix all issues with AI agents
In `@cmd/auth_env.go`:
- Around line 21-27: The constant defaultAuthFileMode currently sets new
credential file permissions to 0o644 which allows world-readable access; change
defaultAuthFileMode in cmd/auth_env.go to 0o600 (owner read/write only) so
exported credentials are created with restrictive permissions suitable for
secret material.
- Around line 251-258: The file I/O error returns in cmd/auth_env.go should use
static sentinel errors from errors/errors.go instead of raw string literals; add
two new sentinel errors (e.g., ErrFileOpen and ErrFileWrite) to
errors/errors.go, import that package into cmd/auth_env.go, and replace the
current fmt.Errorf calls (the open error and the f.WriteString error) to wrap
the sentinel with the underlying error and context using %w (for example:
fmt.Errorf("%w: failed to open file '%s': %v", errors.ErrFileOpen, filePath,
err) and similarly for ErrFileWrite), keeping the existing message/context and
preserving error wrapping so callers can check errors.Is(err,
errors.ErrFileOpen) or errors.Is(err, errors.ErrFileWrite).
- Around line 236-239: The heredoc currently uses a fixed delimiter
"ATMOS_EOF_<key>" which can collide if value contains that line; change the
logic in the block handling multiline values (where variables key, value, sb are
used) to generate a unique delimiter not present in value by e.g. appending a
random or incremental suffix and looping until strings.Contains(value,
delimiter) is false, then use that delimiter in the fmt.Sprintf write instead of
the fixed "ATMOS_EOF_%s"; ensure the new delimiter variable replaces the
existing hard-coded delimiter in the sb.WriteString call.
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@website/docs/cli/commands/auth/auth-env.mdx`:
- Around line 99-112: Update the documentation text to use the proper platform
capitalization "GitHub Actions" (capital "G" and "H") wherever the platform is
referenced in this section; specifically edit the sentence that currently says
"In GitHub Actions, use `--format=github`..." to ensure "GitHub Actions" is
capitalized while leaving the flag `--format=github` and the commands `atmos
auth env` and `--login` unchanged.
🧹 Nitpick comments (4)
cmd/env/env_test.go (1)
293-309: JSON validation improved.The test now reads file content and verifies keys/values are present. Previous review suggested using
json.Unmarshalfor stricter validation - current approach withassert.Containsis simpler but less rigorous.Consider unmarshalling for full round-trip validation:
♻️ Optional enhancement
// Verify JSON structure matches input. - for key, value := range tt.envVars { - assert.Contains(t, string(content), `"`+key+`"`) - assert.Contains(t, string(content), `"`+value+`"`) - } + var got map[string]string + require.NoError(t, json.Unmarshal(content, &got)) + assert.Equal(t, tt.envVars, got)cmd/env/env.go (1)
20-24: Consider derivingSupportedFormatsfrompkg/env.You're maintaining a separate format list here while
pkg/env.SupportedFormatsexists. Similar pattern is used incmd/auth_env.go(lines 138-144) where formats are built dynamically. This could drift.♻️ Suggested approach
// SupportedFormats lists all supported output formats. var SupportedFormats = append([]string{"json"}, envfmt.FormatStrings()...)Or derive directly from
envfmt.SupportedFormatsplus "json" likecmd/auth_env.godoes.pkg/env/output_test.go (1)
20-33: Consider checking pipe errors.The
os.Pipe()error is silently ignored. While rare, pipe creation can fail. Also, the buffer read (line 32) discards errors.♻️ Safer capture
- r, w, _ := os.Pipe() + r, w, err := os.Pipe() + require.NoError(t, err)- n, _ := r.Read(buf) + n, err := r.Read(buf) + require.NoError(t, err)pkg/env/output.go (1)
31-58:perf.Trackin trivial option functions is unnecessary overhead.Per coding guidelines, trivial getters/setters should skip performance tracking. These option functions are simple closure factories.
♻️ Remove perf.Track from option functions
func WithFileMode(mode os.FileMode) OutputOption { - defer perf.Track(nil, "env.WithFileMode")() - return func(c *outputConfig) { c.fileMode = mode } } func WithAtmosConfig(cfg *schema.AtmosConfiguration) OutputOption { - defer perf.Track(nil, "env.WithAtmosConfig")() - return func(c *outputConfig) { c.atmosConfig = cfg } } func WithFormatOptions(opts ...Option) OutputOption { - defer perf.Track(nil, "env.WithFormatOptions")() - return func(c *outputConfig) { c.formatOpts = opts } }
Add "in GitHub Actions" to clarify that subsequent steps refers to GitHub Actions workflow steps. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
Resolve merge conflicts: - cmd/auth_env.go: Accept cliConfigPath parameter for createAuthManager - cmd/auth_integration_test.go: Keep explicit format assertions for github format support Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cmd/auth_env.go (1)
138-139: Fix the stale JSON comment.
jsonis no longer handled in this command; it is handled insideenv.Output. The current comment points readers at the wrong layer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/auth_env.go` around lines 138 - 139, The comment above the format completion registration is stale: remove the line claiming "JSON is handled separately in the command, not via pkg/env" and replace it with a brief note that JSON formatting is handled by env.Output (so this registration only covers pkg/env-supported formats); update the comment near the "Register format completion" text to reference pkg/env and env.Output so readers are directed to the correct layer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/auth_env.go`:
- Around line 121-123: The --format flag help on authEnvCmd currently lists
"bash, dotenv, github, json" but omits the supported "env" option; update the
StringP call for the "format" flag (authEnvCmd.Flags().StringP("format", "f",
...)) to include "env" in the help string so the flag description reads
something like "Output format: bash, dotenv, env, github, json" (adjust ordering
to match completion if desired).
- Around line 131-136: Replace the direct viper.BindEnv/BindPFlag calls for the
auth_env command with the standard flag parser pattern: create a parser via
flags.NewStandardParser(), register the "output-file" and "format" flags with
WithStringFlag (and any boolean flags with WithBoolFlag if present), call
WithEnvVars to map ATMOS_AUTH_ENV_OUTPUT_FILE (and the existing ENV for format)
and then call BindToViper() to bind into viper; update the code that currently
references authEnvCmd.Flags().Lookup("output-file") to use the parser
registration flow and ensure the parser is invoked before relying on viper
values.
---
Nitpick comments:
In `@cmd/auth_env.go`:
- Around line 138-139: The comment above the format completion registration is
stale: remove the line claiming "JSON is handled separately in the command, not
via pkg/env" and replace it with a brief note that JSON formatting is handled by
env.Output (so this registration only covers pkg/env-supported formats); update
the comment near the "Register format completion" text to reference pkg/env and
env.Output so readers are directed to the correct layer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b0b8b547-a679-408f-9cbe-39f693c6d395
📒 Files selected for processing (2)
cmd/auth_env.gocmd/auth_integration_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/auth_integration_test.go
…rage Address PR review feedback: - Refactor cmd/auth_env.go to use flags.NewStandardParser() instead of direct viper.BindEnv/BindPFlag calls (mandatory per coding guidelines) - Add missing "env" format to --format flag help text - Add tests for JSON output with/without AtmosConfig, env format, writeToFileWithMode error path, and WithFormatOptions passthrough Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update snapshot to reflect StandardParser flag output: - Added "env" to format list - Boolean flag default display matches Cobra convention Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
aknysh
approved these changes
Apr 7, 2026
|
These changes were released in v1.215.0-rc.4. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
--format=githuboption toatmos auth envcommand--outputflag for explicit file output (appends mode)$GITHUB_ENVenvironment variable when using github formatKEY=valueand multiline values with heredoc syntaxwhy
This eliminates the need for complex shell pipelines like
atmos auth env ... | grep "^export " | sed 's/^export //' >> $GITHUB_ENV. Users can now directly output to GitHub Actions$GITHUB_ENVfile with proper formatting and multiline value handling.references
Closes issue related to GitHub Actions integration workflow simplification.
Summary by CodeRabbit
New Features
atmos auth env: Export credentials directly to $GITHUB_ENV with--format=github--output-fileflag to redirect output to a fileDocumentation