Skip to content

Commit 39f7202

Browse files
jongioCopilot
andauthored
feat(azdext): Add KeyVault resolver for extension secret resolution (#7043)
* feat(azdext): Add KeyVaultResolver for extension Key Vault secret resolution Add KeyVaultResolver to the Extension SDK for resolving Azure Key Vault secret references embedded in environment variables. Supports three reference formats: - akvs://<subscription>/<vault>/<secret> - @Microsoft.KeyVault(SecretUri=https://<vault>.vault.azure.net/secrets/<name>[/<version>]) - @Microsoft.KeyVault(VaultName=...;SecretName=...) Features: - Thread-safe per-vault client caching - Batch resolution via ResolveMap - Structured error types with ResolveReason classification - Configurable vault suffix for sovereign clouds - Helper functions: IsSecretReference, ParseKeyVaultAppReference, ResolveSecretEnvironment for bulk env var resolution Also adds supporting functions to pkg/keyvault: - IsKeyVaultAppReference, IsSecretReference - ParseKeyVaultAppReference for @Microsoft.KeyVault format - SecretFromKeyVaultReference for unified resolution - ResolveSecretEnvironment for bulk env var processing Evidence: Extensions like azd-exec, azd-app duplicate 100+ lines of KV resolution logic each. This centralizes the pattern in the SDK. Related: #6945, #6853 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove duplicate stubCredential (already in token_provider_test.go) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove stale files from KV resolver branch Reverted detect_confirm_apphost.go (had color.MagentaString() without import, causing build failure) and show.go (KV comments belong on improvements branch) to match upstream/main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: suppress gosec G101 false positives in test fixtures The 'secret' map key in test data triggers gosec G101 (hardcoded credentials) but these are fake akvs:// URIs in test fixtures, not real credentials. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix security and code quality audit findings - SSRF: Validate KeyVault SecretUri hostname against Azure vault suffixes - Scoping: Only resolve azd env vars through KV resolver, not os.Environ() - Errors: Use ServiceError reason for non-ResponseError failures - Security: Explicit DisableChallengeResourceVerification: false - Testing: Add 8 tests for @Microsoft.KeyVault reference format - Reliability: Return empty string on resolution failure, not raw reference - Determinism: Sort keys in ResolveMap, collect all errors with errors.Join Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove stray coverage artifact Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: resolve cspell and gosec CI failures - Add 'managedhsm' and 'microsoftazure' to cspell dictionary - Suppress gosec G101 false positive on test data string Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: apply go fix modernization (strings.Cut) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address Copilot review feedback on KeyVaultResolver - Call ResolveSecretEnvironment unconditionally so akvs:// refs work without AZURE_SUBSCRIPTION_ID being set - Add per-vault client cache (sync.Map) to avoid N client creations when resolving N secrets from the same vault - Preserve original reference in ResolveMap output on failure so callers see all input keys - Tighten IsKeyVaultAppReference to only match SecretUri= variant - Use u.Hostname() for port-safe host validation in SSRF checks - Return ([]string, error) from ResolveSecretEnvironment so callers can handle failures explicitly - Update docs to clarify only SecretUri= format is supported Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove sort.Strings(result) that breaks env var override semantics ResolveSecretEnvironment appends system env vars first then azd vars, so last-wins ordering matters for duplicate keys. Sorting alphabetically destroys this override semantics. Addresses review thread 1 (keyvault.go:651). * fix: use case-insensitive matching for @Microsoft.KeyVault prefix Azure App Service handles @Microsoft.KeyVault(...) references case-insensitively. Switch from strings.HasPrefix to strings.EqualFold for the prefix and SecretUri= parameter checks so that variations like @microsoft.keyvault(secreturi=...) are not silently skipped. Addresses review thread 6 (keyvault.go:540). * fix: reject non-standard ports in @Microsoft.KeyVault SecretUri A URI like https://myvault.vault.azure.net:9999/secrets/foo would pass host validation but send the bearer token to an unexpected port. Reject any port other than 443 (or no port) for SSRF protection. Addresses review thread 4 (keyvault.go:617). * fix: reject empty vault name when hostname equals a bare suffix If the SecretUri hostname has no subdomain prefix (e.g., 'vault.azure.net' instead of 'myvault.vault.azure.net'), vault name extraction would yield an incorrect or empty value. Add a guard to ensure the extracted vault name is non-empty and distinct from the full host. Addresses review thread 5 (keyvault.go:625). * refactor: use slices.Sorted(maps.Keys(...)) in ResolveMap Replace the manual key collection + sort.Strings pattern with the idiomatic Go 1.26 equivalent. This is cleaner and consistent with the rest of the codebase. Addresses review thread 9 (keyvault_resolver.go:299). * refactor: use t.Context() instead of context.Background() in tests Per Go 1.24+ conventions and repo patterns, use t.Context() to get a test-scoped context that is automatically cancelled when the test ends. Addresses review thread 8 (keyvault_resolver_test.go:548). * test: add @Microsoft.KeyVault cases to IsSecretReference test The test only covered akvs:// variants. Add cases for the @Microsoft.KeyVault(SecretUri=...) format (true), case-insensitive prefix matching (true), and the unsupported VaultName/SecretName form (false). Addresses review thread 3 (keyvault_resolver_test.go:534). * test: consolidate HTTP error tests, add recording stub, test error collection - Replace 4 structurally identical HTTP error tests with a single table-driven TestResolve_HTTPErrorClassification - Make stubSecretGetter record name/version args and verify dispatch in TestResolve_Success and TestResolve_AppRefWithVersion - Add 3 failing refs (+ 1 plain value) to TestResolveMap_ErrorCollectsAllFailures to verify error collection and partial results Addresses review thread 10 (keyvault_resolver_test.go:766). * test: add tests for ResolveSecretEnvironment and SecretFromKeyVaultReference Cover the key untested paths: - Mixed akvs:// and @Microsoft.KeyVault refs (both resolved) - Malformed env vars (no = sign) passed through unchanged - nil kvService passthrough (returns input as-is) - Error collection when multiple refs fail - Ordering preserved (no alphabetical sort) - Non-standard port rejection - Port 443 allowed - Empty vault name rejection - Case-insensitive prefix matching - Comprehensive IsSecretReference coverage Addresses review thread 2 (keyvault.go:424). * refactor: use errors.AsType per Go 1.26+ conventions Replace errors.As + var declaration with errors.AsType generic pattern for cleaner, more idiomatic error type assertion. Addresses review thread 11 (keyvault_resolver.go:231). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: resolve CI failures — gofmt and missing mock method - Fix gofmt formatting in keyvault_test.go (tabs vs spaces in imports) - Add SecretFromKeyVaultReference to mockKeyVaultService in cmdsubst tests to satisfy updated KeyVaultService interface Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: add ResolveEnvironment, VaultName format, and quote stripping to KeyVault resolver - Add ResolveEnvironment(ctx, env) method that scans env map, resolves only secret references, and passes through plain values unchanged - Add VaultName=;SecretName= format support to IsSecretReference and ParseSecretReference (third format alongside SecretUri and akvs://) - Add quote/whitespace stripping to IsSecretReference so quoted values like "akvs://..." and 'akvs://...' are detected - Add vault name validation for VaultName format (matches akvs:// rules) - Add 24 new tests covering all new functionality and edge cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: address wbreza re-review — fix data race, modernize test patterns (t.Context, errors.AsType, wg.Go) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f9ce827 commit 39f7202

7 files changed

Lines changed: 2505 additions & 1 deletion

File tree

cli/azd/.vscode/cspell.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ words:
8585
- yarnpkg
8686
- azconfig
8787
- hostnames
88+
- managedhsm
89+
- microsoftazure
8890
- seekable
8991
- seekability
9092
- APFS
@@ -108,7 +110,10 @@ words:
108110
- preconfigured
109111
- Println
110112
- sctx
113+
- secretname
114+
- secretversion
111115
- TTLs
116+
- vaultname
112117
languageSettings:
113118
- languageId: go
114119
ignoreRegExpList:

cli/azd/cmd/extensions.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/azure/azure-dev/cli/azd/pkg/exec"
2020
"github.com/azure/azure-dev/cli/azd/pkg/extensions"
2121
"github.com/azure/azure-dev/cli/azd/pkg/input"
22+
kv "github.com/azure/azure-dev/cli/azd/pkg/keyvault"
2223
"github.com/azure/azure-dev/cli/azd/pkg/lazy"
2324
"github.com/azure/azure-dev/cli/azd/pkg/output/ux"
2425
pkgux "github.com/azure/azure-dev/cli/azd/pkg/ux"
@@ -119,6 +120,7 @@ type extensionAction struct {
119120
extensionManager *extensions.Manager
120121
azdServer *grpcserver.Server
121122
globalOptions *internal.GlobalCommandOptions
123+
kvService kv.KeyVaultService
122124
cmd *cobra.Command
123125
args []string
124126
}
@@ -132,6 +134,7 @@ func newExtensionAction(
132134
cmd *cobra.Command,
133135
azdServer *grpcserver.Server,
134136
globalOptions *internal.GlobalCommandOptions,
137+
kvService kv.KeyVaultService,
135138
args []string,
136139
) actions.Action {
137140
return &extensionAction{
@@ -141,6 +144,7 @@ func newExtensionAction(
141144
extensionManager: extensionManager,
142145
azdServer: azdServer,
143146
globalOptions: globalOptions,
147+
kvService: kvService,
144148
cmd: cmd,
145149
args: args,
146150
}
@@ -216,7 +220,18 @@ func (a *extensionAction) Run(ctx context.Context) (*actions.ActionResult, error
216220

217221
env, err := a.lazyEnv.GetValue()
218222
if err == nil && env != nil {
219-
allEnv = append(allEnv, env.Environ()...)
223+
// Resolve Key Vault secret references only in azd-managed environment
224+
// variables (akvs:// and @Microsoft.KeyVault formats). System env vars
225+
// from os.Environ() are NOT processed — only the azd environment's
226+
// variables may contain KV references.
227+
azdEnvVars := env.Environ()
228+
subId := env.Getenv("AZURE_SUBSCRIPTION_ID")
229+
azdEnvVars, kvErr := kv.ResolveSecretEnvironment(ctx, a.kvService, azdEnvVars, subId)
230+
if kvErr != nil {
231+
log.Printf("warning: %v", kvErr)
232+
}
233+
234+
allEnv = append(allEnv, azdEnvVars...)
220235
}
221236

222237
serverInfo, err := a.azdServer.Start()

0 commit comments

Comments
 (0)