Conversation
There was a problem hiding this comment.
Pull request overview
Adds the first-party microsoft.azd.exec extension to the azure-dev repo, enabling azd exec to run scripts/inline commands with azd environment context and optional Key Vault secret resolution, plus MCP tooling. It also introduces/extends Key Vault reference parsing and host-side secret resolution so extensions receive resolved env vars.
Changes:
- Introduces the
cli/azd/extensions/microsoft.azd.execextension module (cobra command, executor, MCP server, skills, build scripts, metadata). - Adds Key Vault secret reference parsing/resolution helpers in core (
pkg/keyvault) and a new resolver in the extension SDK (pkg/azdext). - Wires Key Vault env var resolution into extension invocation and adds CI/release pipeline + registry/workflow entries for the new extension.
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/pipelines/release-ext-microsoft-azd-exec.yml | Adds ADO release pipeline definition for the new extension. |
| .github/workflows/lint-ext-microsoft-azd-exec.yml | Adds GitHub Actions lint workflow scoped to the new extension module. |
| cli/azd/cmd/extensions.go | Resolves Key Vault secret references in azd-managed env vars before invoking extensions. |
| cli/azd/pkg/keyvault/keyvault.go | Adds support for @Microsoft.KeyVault(SecretUri=...) parsing + unified reference resolution + env var list resolver. |
| cli/azd/pkg/keyvault/keyvault_test.go | Adds unit tests for new reference parsing and env var secret resolution behavior. |
| cli/azd/pkg/cmdsubst/cmdsubst_additional_test.go | Updates KeyVaultService mock to satisfy the expanded interface. |
| cli/azd/pkg/azdext/keyvault_resolver.go | Adds SDK-side Key Vault resolver supporting multiple reference formats and caching. |
| cli/azd/pkg/azdext/keyvault_resolver_test.go | Adds comprehensive resolver tests (parsing, error classification, concurrency, env/map helpers). |
| cli/azd/extensions/registry.json | Registers the new microsoft.azd.exec extension (id/namespace/version/capabilities). |
| cli/azd/extensions/microsoft.azd.exec/version.txt | Introduces extension version tracking (0.5.0). |
| cli/azd/extensions/microsoft.azd.exec/README.md | Adds end-user docs for install/usage/features of azd exec. |
| cli/azd/extensions/microsoft.azd.exec/main.go | Adds extension entrypoint and error rendering. |
| cli/azd/extensions/microsoft.azd.exec/go.mod | Adds Pattern B module for the extension with replace to core azd module. |
| cli/azd/extensions/microsoft.azd.exec/go.sum | Adds extension module dependency lockfile. |
| cli/azd/extensions/microsoft.azd.exec/extension.yaml | Defines extension metadata/capabilities and MCP serve args/env mapping. |
| cli/azd/extensions/microsoft.azd.exec/.golangci.yaml | Adds extension-specific golangci-lint configuration. |
| cli/azd/extensions/microsoft.azd.exec/CHANGELOG.md | Adds initial changelog for the extension. |
| cli/azd/extensions/microsoft.azd.exec/build.sh | Adds cross-platform build script for producing extension binaries. |
| cli/azd/extensions/microsoft.azd.exec/build.ps1 | Adds PowerShell cross-platform build script for producing extension binaries. |
| cli/azd/extensions/microsoft.azd.exec/ci-build.ps1 | Adds CI build wrapper for the extension (flags/tags/ldflags). |
| cli/azd/extensions/microsoft.azd.exec/internal/cmd/root.go | Implements azd exec root command wiring, flags, env loading, and subcommands. |
| cli/azd/extensions/microsoft.azd.exec/internal/cmd/root_test.go | Adds basic tests for root/version command construction. |
| cli/azd/extensions/microsoft.azd.exec/internal/cmd/version.go | Implements version subcommand output. |
| cli/azd/extensions/microsoft.azd.exec/internal/cmd/mcp.go | Implements hidden MCP server command and tool handlers (exec/list shells/env). |
| cli/azd/extensions/microsoft.azd.exec/internal/cmd/mcp_test.go | Adds unit tests for MCP handlers, validation, and helper functions. |
| cli/azd/extensions/microsoft.azd.exec/internal/executor/executor.go | Implements script/inline execution and env preparation with optional Key Vault resolution. |
| cli/azd/extensions/microsoft.azd.exec/internal/executor/command_builder.go | Builds shell-specific exec command lines (including PowerShell quoting). |
| cli/azd/extensions/microsoft.azd.exec/internal/executor/command_builder_test.go | Adds tests for command construction and PowerShell arg quoting. |
| cli/azd/extensions/microsoft.azd.exec/internal/executor/errors.go | Adds typed executor errors (validation/shell/exit code). |
| cli/azd/extensions/microsoft.azd.exec/internal/executor/errors_test.go | Adds tests validating error messages and config validation. |
| cli/azd/extensions/microsoft.azd.exec/internal/skills/skills.go | Implements embedded Copilot skill installation to ~/.copilot/skills/azd-exec. |
| cli/azd/extensions/microsoft.azd.exec/internal/skills/azd-exec/SKILL.md | Adds embedded skill definition/documentation for the extension. |
cli/azd/extensions/microsoft.azd.exec/internal/executor/executor.go
Outdated
Show resolved
Hide resolved
cli/azd/extensions/microsoft.azd.exec/internal/skills/azd-exec/SKILL.md
Outdated
Show resolved
Hide resolved
cli/azd/extensions/microsoft.azd.exec/internal/skills/azd-exec/SKILL.md
Outdated
Show resolved
Hide resolved
81565b7 to
b721a0a
Compare
|
Dependent on #7314 |
Known Issue: Environment Value MungingWhen azd core already passes env vars directly to the extension process environment, so this roundtrip is only needed when PR #7314 addresses the core Tracking: depends on #7314 |
wbreza
left a comment
There was a problem hiding this comment.
Code Review — microsoft.azd.exec Extension
Good work on the clean architecture and comprehensive test coverage! A few items to consider before this is ready to merge.
What Looks Good ✅
- Clean package structure — 3 focused packages (
cmd,executor,shellutil) with clear separation of concerns - Strong error hierarchy — Typed errors enable programmatic handling and clear user messages
- Comprehensive table-driven tests — Platform-aware scaffolding, validation edge cases, and PowerShell quoting coverage
- Faithful exit code propagation — Critical for CI/CD usage, well-implemented
- Correct SDK usage —
NewExtensionRootCommand,PersistentPreRunEchain preservation,NewMetadataCommand
Summary
| Priority | Count |
|---|---|
| Critical | 0 |
| High | 2 |
| Medium | 3 |
| Low | 2 |
| Total | 7 |
Overall Assessment: Comment — see inline comments for details.
b721a0a to
b2cf453
Compare
|
How are we logging failures, preflight on the script (if needed), if the script itself fails does it use azd logs so we can report when a user has used exec and passed or failed and why Also how does this differ from hooks, pre and post? |
|
Good questions. On logging and error reporting: We're using the SDK's telemetry stack. On how this differs from hooks: Hooks are tied to azd lifecycle events. You define them in |
b2cf453 to
7cf929e
Compare
b32ee0e to
65c0aff
Compare
1e7bc49 to
b089007
Compare
b089007 to
b526874
Compare
Add the azd-exec extension as microsoft.azd.exec to the official extensions directory. This extension enables script execution with Azure Developer CLI context, environment variables, and automatic Azure Key Vault secret resolution. Features: - Execute script files (-s) or inline commands (-i) with shell selection - Automatic azd environment variable loading via azd env get-values - Azure Key Vault secret reference resolution in environment variables - Cross-platform shell support (bash, sh, zsh, pwsh, powershell, cmd) - Child process exit code propagation for CI/CD pipelines - Interactive mode with stdin passthrough Architecture: - internal/cmd: Cobra CLI commands (root, version, listen) - internal/executor: Script execution engine with command builder - internal/envutil: Shared Key Vault reference resolution - internal/shellutil: Shared shell detection and validation Test coverage: 82-94% across all packages (42 tests). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
b526874 to
a171856
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
wbreza
left a comment
There was a problem hiding this comment.
Code Review — microsoft.azd.exec Extension
Great extension contribution! The architecture is clean and well-thought-out — 3 focused packages with clear separation of concerns, comprehensive test coverage, and proper use of the extension SDK patterns. Looking forward to porting more extensions over to the azd official registry.
✅ What Looks Good
- Clean use of
azdext.Run()with the newWithExitCodeoption — elegant solution for exit code propagation - Thorough cmd.exe injection tests covering CWE-78 and CWE-93 edge cases
errors.AsType[*T]()used consistently throughout (Go 1.26 convention)- Security hardening flags in build scripts (
-trimpath,-buildmode=pie,-tags=cfi,cfg,osusergo) requiredAzdVersionproperly declared- Good error type hierarchy enabling programmatic handling
Summary
| Priority | Count |
|---|---|
| Critical | 0 |
| High | 0 |
| Medium | 2 |
| Low | 3 |
| Total | 5 |
Overall Assessment: Approve — well-structured extension with no blocking issues. See inline comments for minor suggestions.
| }) | ||
| if err != nil { | ||
| return fmt.Errorf("invalid configuration: %w", err) | ||
| } |
There was a problem hiding this comment.
[Medium] Silent fallback to inline execution on non-NotExist stat errors
When os.Stat(absPath) fails with something other than "not found" (e.g., EACCES, broken symlinks), the code silently falls through to ExecuteInline, producing a confusing failure instead of the real error.
Consider trying Execute first and falling back to ExecuteInline only on ScriptNotFoundError — this also eliminates the duplicate filepath.Abs+os.Stat between here and executor.go:
| } | |
| if err := exec.Execute(cmd.Context(), scriptInput); err != nil { | |
| if _, ok := errors.AsType[*executor.ScriptNotFoundError](err); ok { | |
| return exec.ExecuteInline(cmd.Context(), scriptInput) | |
| } | |
| return err | |
| } | |
| return nil |
| } | ||
|
|
||
| // quoteCmdArg quotes a single argument for cmd.exe if it contains spaces | ||
| // or metacharacters. Embedded double quotes are escaped by doubling them. |
There was a problem hiding this comment.
[Medium] quoteCmdArg doesn't treat tab characters as word separators
Tab characters (\t) also function as argument delimiters in Windows command lines. An embedded tab would pass through unquoted, causing unintended argument splitting.
| // or metacharacters. Embedded double quotes are escaped by doubling them. | |
| if strings.ContainsAny(escaped, " \t&|<>^%\"") { |
|
|
||
| if cfg.exitCodeFunc != nil { | ||
| if code, ok := cfg.exitCodeFunc(err); ok { | ||
| os.Exit(code) |
There was a problem hiding this comment.
[Low] WithExitCode allows exit code 0 in the error path
If a caller's exitCodeFunc returns (0, true), Run calls os.Exit(0) despite having a real error, masking failures in CI. Current usage won't trigger this, but the API is generic.
Consider adding a defensive guard or documenting in the godoc that returning (0, true) is a contract violation.
| if useCmdLineOverride { | ||
| setCmdLineOverride(cmd, cmdArgs, cmdWrapOuter) | ||
| } | ||
| return cmd |
There was a problem hiding this comment.
[Low] strings.NewReplacer allocated on every call
Not a hot path, but a free optimization — consider hoisting to a package-level variable:
var controlCharReplacer = strings.NewReplacer("\n", "", "\r", "", "\x00", "")|
|
||
| // quoteCmdArg quotes a single argument for cmd.exe if it contains spaces | ||
| // or metacharacters. Embedded double quotes are escaped by doubling them. | ||
| // Newline/CR/null bytes are stripped as they act as command separators. |
There was a problem hiding this comment.
[Low] Consider documenting the %-variable expansion limitation
cmd.exe expands %VAR% patterns even inside double quotes — an inherent limitation with no workaround. A brief code comment would prevent future maintainers from thinking the quoting prevents expansion.
Description
Add the
microsoft.azd.execextension to the official extensions directory. This extension enables script execution with Azure Developer CLI context and environment variables.Example Usage
Features
Architecture
3 focused internal packages, no circular dependencies, structured error types for programmatic handling.
Test Coverage
cmdexecutorshellutilAll tests passing. Table-driven tests with platform-aware scaffolding.
What's Included
cli/azd/extensions/microsoft.azd.exec/).github/workflows/lint-ext-microsoft-azd-exec.yml)eng/pipelines/release-ext-microsoft-azd-exec.yml)build.ps1,build.sh,ci-build.ps1)What's NOT Included (Follow-Up)
Dependencies
pkg/azdextSDK for extension bootstrap and gRPC clientTesting