feat: add --export flag to azd env get-values for shell sourcing#7364
Closed
feat: add --export flag to azd env get-values for shell sourcing#7364
Conversation
spboyer
requested review from
JeffreyCA,
hemarina,
rajeshkamal5050,
tg-msft,
vhvb1989,
wbreza and
weikanglim
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new --export mode to azd env get-values so the command can emit shell-ready export KEY="VALUE" lines intended for direct sourcing/eval in POSIX-like shells, addressing the workflow described in #4384.
Changes:
- Introduces
--exportflag forazd env get-valuesand a helper to format/escape values asexport KEY="VALUE". - Adds table-driven unit tests for exported output vs existing dotenv output.
- Updates Fig spec and usage snapshots to reflect the new flag.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cli/azd/cmd/env.go | Adds --export flag, help text, and writeExportedEnv() implementation used by env get-values. |
| cli/azd/cmd/env_get_values_test.go | Adds unit tests validating export formatting and escaping behavior. |
| cli/azd/cmd/testdata/TestUsage-azd-env-get-values.snap | Updates usage snapshot to include --export flag. |
| cli/azd/cmd/testdata/TestFigSpec.ts | Updates generated Fig completion spec with the new --export option. |
Comments suppressed due to low confidence (1)
cli/azd/cmd/env.go:1385
--exportoutput is not safe toevalwhen an environment key is not a valid shell identifier. Keys are inserted unquoted intoexport %s=..., andazd env setcurrently accepts arbitrary keys, so a crafted key containing shell metacharacters/command substitutions could lead to command execution or a syntax error. Consider validating keys (e.g., POSIX identifier regex) and returning an error (or skipping with a warning) when a key is invalid before writing the export line.
keys := slices.Sorted(maps.Keys(values))
for _, key := range keys {
val := values[key]
escaped := strings.NewReplacer(
`\`, `\\`,
`"`, `\"`,
`$`, `\$`,
"`", "\\`",
"\n", `\n`,
"\r", `\r`,
).Replace(val)
line := fmt.Sprintf("export %s=\"%s\"\n", key, escaped)
if _, err := io.WriteString(writer, line); err != nil {
spboyer
commented
Mar 27, 2026
Member
Author
spboyer
left a comment
spboyer
left a comment
There was a problem hiding this comment.
All 4 review comments addressed:
- ✅
t.Context()— Replacedcontext.Background()witht.Context(), removed unused import. - ✅ Security test cases — Added tests for backslashes (
C:\path\to\dir), backticks + command substitution, and carriage returns. - ✅ Mutual exclusion —
--exportand--outputnow return an error when both specified. Added test. - ✅ Doc comment — Updated
writeExportedEnvcomment to list all escaped characters.
Please resolve the threads.
spboyer
force-pushed
the
feat/env-export
branch
2 times, most recently
from
dcaa317 to
50cc7be
Compare
Member
|
related: #1697 |
vhvb1989
reviewed
Mar 27, 2026
jongio
requested changes
Mar 28, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
PR Review - #7364
feat: add --export flag to azd env get-values for shell sourcing by @spboyer
Findings
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic | 0 | 1 | 0 | 0 |
| Security | 0 | 0 | 1 | 0 |
| Quality | 0 | 0 | 0 | 1 |
| Total | 0 | 1 | 1 | 1 |
Key Findings
- [HIGH] Logic: Newline/CR escaping doesn't survive bash
evalroundtrip -\ninside double quotes is literal in bash, so multiline values are silently corrupted - [MEDIUM] Security: Keys aren't validated as POSIX shell identifiers before interpolation into
exportstatements - shell metacharacters in keys could cause injection
Test Coverage Estimate
- Well covered: basic values, special character escaping (quotes, dollars, backticks, backslashes, CRs), empty values, mutual exclusion of --export/--output
- Likely missing: roundtrip validation (export -> eval -> verify value matches original), keys with non-identifier characters
What's Done Well
- Solid table-driven test structure with good edge case coverage
- Clean mutual exclusion check using existing
ErrInvalidFlagCombinationsentinel - Deterministic output via sorted keys
writeExportedEnvis a pure function taking map + writer - easy to test and reason about
3 inline comments below. Review converged after 2 analysis passes.
5 existing inline comments and 4 review submissions were checked - those issues aren't repeated here.
Add --export flag that outputs environment variables in shell-ready format (export KEY="VALUE" for bash/zsh). This enables easy shell integration: eval "$(azd env get-values --export)" Escapes backslashes, double quotes, dollar signs, backticks, newlines, and carriage returns for safe eval usage. Returns error when combined with --output flag (mutually exclusive). Fixes #4384 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use ANSI-C quoting ($'...') for values containing newlines so \n roundtrips correctly through eval. - Skip keys that are not valid shell identifiers to prevent injection. - Lift strings.NewReplacer out of the loop (allocated once). - Add test for invalid key skipping. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Avoids re-allocating strings.NewReplacer and regexp.MustCompile on every call to writeExportedEnv. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
spboyer
force-pushed
the
feat/env-export
branch
from
0633e0a to
9b3872f
Compare
wbreza
reviewed
Mar 31, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Code Review — PR #7364
feat: add --export flag to azd env get-values for shell sourcing by @spboyer
What's Done Well
- Prior findings fully resolved — all 3 of jongio's CHANGES_REQUESTED items (newline roundtrip via ANSI-C $'...', key validation, replacer allocation) are addressed in the current revision
- Clean architecture —
writeExportedEnvis a pure function takingmap[string]string+io.Writer, easy to test and reason about - Thorough tests — 9 table-driven cases covering special chars, newlines, backticks, command injection, CRs, empty values, and invalid keys. Plus mutual-exclusion test
- Efficient —
shellEscaperandvalidShellKeyare package-level vars built once - Deterministic — sorted keys ensure consistent output for scripting
Findings
| Priority | Count |
|---|---|
| Critical | 0 |
| High | 0 |
| Medium | 2 |
| Low | 2 |
| Total | 4 |
See inline comments for details.
Overall Assessment: Comment — the implementation is solid and production-ready for POSIX shells. The \r roundtrip bug is worth fixing before merge. Cross-platform shell support would make this feature significantly more useful given azd's Windows user base.
Review performed with GitHub Copilot CLI
jongio
requested changes
Mar 31, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
PR Review - #7364
feat: add --export flag to azd env get-values for shell sourcing by @spboyer
Summary
What: Adds --export flag to azd env get-values that outputs export KEY="VALUE" lines for direct shell sourcing via eval. Uses ANSI-C $'...' quoting for values with newlines, validates keys as POSIX identifiers, and rejects --export combined with non-default --output.
Why: Addresses #4384 - users want a quick way to source azd environment variables into their shell session.
Assessment: Solid implementation. Prior review findings (newline roundtrip, key validation, replacer allocation) are fully addressed. The ANSI-C quoting logic is correct - I verified empirically that bash 5.2 strips backslashes from unrecognized escape sequences in $'...', so shellEscaper's \$ and \` entries work in both double-quote and ANSI-C contexts. The open design discussion about --output export-bash/export-pwsh (from @vhvb1989 and @wbreza) is the main remaining question.
Findings
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Error Handling | 0 | 0 | 0 | 1 |
| Total | 0 | 0 | 0 | 1 |
Test Coverage Estimate
- Well covered: basic export, special chars, newlines (ANSI-C), backslashes, backticks,
$, carriage returns, empty values, invalid keys, mutual exclusion - Tested empirically:
$'...'quoting roundtrips correctly for\$,\`,\\,\n,\rvia bash 5.2
What's Done Well
- Clean
writeExportedEnvas a pure function takingmap[string]string+io.Writer- testable and composable slices.Sorted(maps.Keys(...))for deterministic output - idiomatic Go 1.26- Package-level
shellEscaperandvalidShellKeyavoid per-call allocation - 9 table-driven test cases with good edge case variety
1 inline comment below.
- Fix CR-only values to use ANSI-C quoting for correct roundtrip - Document POSIX shell limitation in help text - Emit stderr warning for skipped non-shell-safe keys - Use ErrorWithSuggestion for --export/--output conflict Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
requested changes
Mar 31, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
All 7 previous findings fixed (ErrorWithSuggestion, CR quoting, stderr warnings, POSIX docs). One new issue:
- cli/azd/cmd/env.go:1424 - shellEscaper escapes for double-quote context but ANSI-C $'...' path reuses the result, corrupting $ and backtick in multiline values
Extends the --export flag with a --shell option (bash | pwsh) so PowerShell users can export env vars natively: azd env get-values --export --shell pwsh | Invoke-Expression PowerShell output uses $env:KEY = "VALUE" syntax with proper backtick escaping for special characters. Includes tests for basic values, special characters, backticks, newlines, empty values, and invalid shell validation. Updates command snapshots for the new --shell flag. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wbreza
reviewed
Mar 31, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Re-Review — PR #7364 (4 new commits since Mar 31 review)
feat: add --export flag to azd env get-values for shell sourcing by @spboyer
Summary
What: Adds --export and --shell flags to azd env get-values for shell-ready output. Supports bash (export KEY="VALUE") and PowerShell ( = "VALUE") with proper escaping, key validation, and ANSI-C quoting for multiline bash values.
Why: Fixes #4384 — users need a quick way to source azd environment variables into their shell session.
Assessment: All 4 of my previous findings are resolved. PowerShell support is a solid addition. One new correctness bug in the PowerShell export path for multiline values. @jongio's ANSI-C escaping finding remains open and is valid.
Prior Findings: All 4 ✅ Resolved
| # | Priority | Finding | Status |
|---|---|---|---|
| 1 | Medium | \r without \n doesn't roundtrip through eval |
✅ Fixed — condition now includes \r |
| 2 | Medium | No PowerShell support | ✅ Fixed — added --shell pwsh with writePwshExportedEnv |
| 3 | Low | Silent key skipping without user feedback | ✅ Fixed — stderr warnings added |
| 4 | Low | ANSI-C $'...' not universal POSIX |
✅ Fixed — help text updated |
@jongio's Open Finding: Verified ✅ Agreed
[HIGH] shellEscaper produces double-quote escapes (\$, \``) reused in ANSI-C $'...'` context where they aren't recognized — confirmed this corrupts values.
New Finding
[HIGH] PowerShell export corrupts multiline values — see inline comment.
What's Done Well
- All prior findings thoroughly addressed with clean, correct fixes
- PowerShell support is a great addition —
--shellflag is extensible for future shells ErrorWithSuggestionadoption matches repo conventions- 15 table-driven test cases with excellent edge case coverage
- Package-level vars (
shellEscaper,pwshEscaper,validShellKey) are efficient and idiomatic
Review performed with GitHub Copilot CLI
weikanglim
reviewed
Mar 31, 2026
jongio
reviewed
Apr 1, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Previous ANSI-C escaping finding still open. PowerShell support is well-structured. Two new observations:
- cli/azd/cmd/env.go:1486 -
os.Stderrused directly; convention isconsole.Handles().Stderrfor testability - cli/azd/cmd/env.go:1353 -
--shellwithout--exportsilently accepted; inconsistent with validation when--exportis set
Minor test gaps: no case-insensitive shell test (--shell PWSH), no CRLF test for pwsh path, no test for --shell without --export.
…ell validation - Use separate ANSI-C escaper for $'...' quoting path so $ and backtick don't get extra backslashes. - Add newline/CR escaping to pwshEscaper using backtick sequences. - Accept warnWriter parameter instead of using os.Stderr directly. - Reject --shell (non-default) without --export. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
reviewed
Apr 1, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Incremental review - changes since dfea26d (1 new commit).
All previously flagged issues are addressed correctly. The ansiCEscaper separation, pwsh newline handling, warnWriter injection, and --shell validation are solid.
Two gaps remain in test coverage - see inline comments.
Use switch statement for TestPtr to keep nil-check and dereference in the same branch. Add nolint directive for TestMCPSecurityFluentBuilder where t.Fatal guarantees non-nil. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add combined newline + single-quote test case for ANSI-C path. - Add direct unit test verifying warnWriter receives skip warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace manual nil-check + t.Fatal with require.NotNil which makes staticcheck aware the pointer is non-nil for subsequent accesses. Convert all manual assertions to require.True/require.Len. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
jongio
reviewed
Apr 1, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Follow-up - 3 commits since last review (5436315, 318b415, 0ce924f).
Both flagged test gaps are now addressed:
- ANSI-C combined escaping test ("export value with newline and single quotes") exercises the ansiCEscaper path with characters that shellEscaper would wrongly escape
TestWriteExportedEnvWarningsvalidates warnWriter output for invalid keys
Two smaller items and a design question:
Scope creep - mcp_security_test.go (converting if/t.Fatal to require.*) and ux_additional_test.go (converting if chains to switch) aren't related to --export. Split these into a separate cleanup PR to keep the diff focused.
Warning format convention - The skipped-key warnings use lowercase "warning: skipping key..." via fmt.Fprintf. The established pattern in cmd/ (auth_login.go:303, config.go:245, util.go:38) is output.WithWarningFormat("WARNING: ...") through console.Handles().Stderr. The warnWriter parameterization was solid - the formatting just needs to match the rest of the codebase. This builds on the earlier os.Stderr comment - the abstraction was addressed but the visual consistency wasn't.
Design direction - @weikanglim's azd env exec proposal (#7423) avoids shell-specific escaping entirely since the OS propagates env vars natively. Both @spboyer and @weikanglim seem aligned on that path. This PR adds two shell escapers, ANSI-C quoting edge cases, and shell detection - all of which exec wouldn't need. Worth deciding: land this as an interim solution, or focus effort on exec?
hemarina
reviewed
Apr 8, 2026
Contributor
hemarina
left a comment
hemarina
left a comment
There was a problem hiding this comment.
Code Review: --export flag for azd env get-values
Overall: Shell escaping is fundamentally sound. No critical issues found.
🟡 MEDIUM
1. NUL byte values silently pass through (env.go)
NUL bytes (\x00) aren't escaped by any of the three replacers. Go's io.WriteString will write them to output, but when a shell evaluates export KEY="hello\x00world", it truncates at the NUL — the variable silently becomes "hello" instead of "hello\x00world". This is data corruption with no warning. Azure service values (certs, connection strings) could contain binary data. Fix: detect and warn, similar to the invalid-key warning.
2. Dead \r entry in shellEscaper (env.go)
The shellEscaper has "\r", \\r\ but it's unreachable. The routing logic is:
if strings.Contains(val, "\n") || strings.Contains(val, "\r") {
// → ANSI-C path using ansiCEscaper, NOT shellEscaper
} else {
escaped := shellEscaper.Replace(val) // only when NO \r present
}So \r values never reach shellEscaper. Remove the dead entry to avoid confusing future maintainers.
🟢 LOW
1. Missing test coverage — No pwsh warning test, no pwsh \r test, no backslash+newline combo test, no empty env map test.
2. --shell bash without --export silently ignored (env.go)
The validation checks if !eg.flags.export && shell != "bash" — so --shell pwsh without --export correctly errors, but --shell bash without --export silently passes because "bash" is the default value. A user explicitly typing azd env get-values --shell bash might expect export behavior. This is hard to fix cleanly since Cobra's pflag can't distinguish "user passed the default" from "not passed at all."
Member
Author
|
Closing this one out. After more thought, I have concerns about |
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
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.
Summary
Fixes #4384
Adds
--exportflag toazd env get-valuesthat outputs environment variables in shell-ready format for direct sourcing.Usage
Changes
cmd/env.go— Added--exportflag andwriteExportedEnv()helper with proper escaping of\\,",$, backticks, newlines, and carriage returnscmd/env_get_values_test.go— Table-driven tests covering basic values, special characters, newlines, empty values, and non-export modeSecurity
The
writeExportedEnv()function escapes all shell-dangerous characters:\\→\\\\"→\\"$→\\$\n→\\n(prevents newline injection)\r→\\rOutput is safe to
evalwithout risk of command execution from env var values.