Fix environment variable leak and flag propagation for extensions#7314
Open
Fix environment variable leak and flag propagation for extensions#7314
Conversation
jongio
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
This PR fixes global flag propagation (notably -e/--environment, --debug, --cwd) to extension commands that run with DisableFlagParsing: true, and standardizes invalid environment-name errors.
Changes:
- Introduces
GlobalCommandOptions.EnvironmentNameand parses-e/--environmentearly viaParseGlobalFlags(). - Updates extension invocation and DI env resolver to read from pre-parsed
globalOptionsrather thancmd.Flags(). - Centralizes invalid environment-name error formatting and updates help/usage snapshots to include the new global flag.
Reviewed changes
Copilot reviewed 71 out of 71 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/pkg/environment/manager.go | Replaces ad-hoc invalid env name messaging with shared InvalidEnvironmentNameError() |
| cli/azd/pkg/environment/environment.go | Adds shared exported invalid env name error helper |
| cli/azd/internal/global_command_options.go | Adds EnvironmentName to carry pre-parsed -e/--environment value |
| cli/azd/cmd/extensions.go | Propagates debug/cwd/env/no-prompt to extensions via globalOptions |
| cli/azd/cmd/container.go | DI resolver for EnvFlag falls back to globalOptions.EnvironmentName |
| cli/azd/cmd/auto_install.go | Adds global -e/--environment and validates it in ParseGlobalFlags() |
| cli/azd/cmd/auto_install_test.go | Adds tests for parsing/validating -e/--environment |
| cli/azd/cmd/testdata/TestFigSpec.ts | Moves --environment/-e to persistent options; removes per-command env options in a few places |
| cli/azd/cmd/testdata/TestUsage-azd.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-x.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-version.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-template.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-template-source.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-template-source-remove.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-template-source-list.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-template-source-add.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-template-show.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-template-list.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-pipeline.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-mcp.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-mcp-start.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-infra.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-hooks.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-extension.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-extension-upgrade.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-extension-uninstall.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-extension-source.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-extension-source-validate.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-extension-source-remove.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-extension-source-list.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-extension-source-add.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-extension-show.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-extension-list.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-extension-install.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-env.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-env-select.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-env-new.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-env-list.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-env-config.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-demo.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-copilot.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-copilot-consent.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-copilot-consent-revoke.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-copilot-consent-list.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-copilot-consent-grant.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-config.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-config-unset.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-config-show.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-config-set.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-config-reset.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-config-options.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-config-list-alpha.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-config-get.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-concurx.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-completion.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-completion-zsh.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-completion-powershell.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-completion-fish.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-completion-fig.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-completion-bash.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-coding-agent.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-auth.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-auth-status.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-auth-logout.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-auth-login.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-appservice.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-ai.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-ai-models.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-ai-finetuning.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-ai-agent.snap | Updates help snapshot to include -e, --environment |
| cli/azd/cmd/testdata/TestUsage-azd-add.snap | Updates help snapshot to include -e, --environment |
Comments suppressed due to low confidence (2)
cli/azd/internal/global_command_options.go:1
- This comment says
EnvironmentNameis empty when the passed-evalue is not a valid environment name (e.g., extensions reuse-efor URLs), butParseGlobalFlags()now returns an error for invalid values. Update the comment to match the new strict-validation behavior (or relax validation if the intent is still to allow extensions to reuse-e).
cli/azd/pkg/environment/environment.go:1 - The standardized error message hard-codes the allowed character set as 'only alphanumeric characters and hyphens'. In this PR,
TestParseGlobalFlags_EnvironmentNametreats a name containing a dot (my-env.v2) as valid. Either adjust the test expectations/validation to disallow dots, or update the error message to accurately describe whatIsValidEnvironmentNamepermits so users get correct guidance.
spboyer
approved these changes
Mar 26, 2026
Member
spboyer
left a comment
spboyer
left a comment
There was a problem hiding this comment.
Reviewed the core fix (switching extensionAction from cmd.Flags() to globalOptions for DisableFlagParsing extensions), IoC plumbing, AZD_DISABLE_AGENT_DETECT kill switch, InvalidEnvironmentNameError refactor, and require.Fail -> require.Failf fix. No issues found.
jongio
force-pushed
the
fix/env-leak-extensions
branch
2 times, most recently
from
c753524 to
14fb280
Compare
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
|
…nt (#7076) Just a little unprotected access to a variable. This should just happen if you run `go test -race` This is the test I used to repro it, but I'm unsure if you already have `-race` turned on in testing, etc... Let me know, I'll move this into the right spot. ```go package bug_test import ( "testing" "github.com/azure/azure-dev/cli/azd/pkg/ux" "github.com/stretchr/testify/require" ) func TestUseUXTaskList_RaceCondition(t *testing.T) { taskList := ux.NewTaskList(&ux.TaskListOptions{}) taskList.AddTask(ux.TaskOptions{ Title: "hello", Action: func(spf ux.SetProgressFunc) (ux.TaskState, error) { return ux.Success, nil }, }) taskList.AddTask(ux.TaskOptions{ Title: "hello2", Action: func(spf ux.SetProgressFunc) (ux.TaskState, error) { return ux.Success, nil }, }) err := taskList.Run() require.NoError(t, err) } ```
The extension was using Subscription.TenantId (the resource tenant) to create the AzureDeveloperCLICredential after subscription selection. For multi-tenant/guest users, this differs from Subscription.UserTenantId (the user access tenant), causing 'refresh token expired' errors. This aligns the extension with how azd core resolves credentials via SubscriptionsManager.LookupTenant(), which returns UserAccessTenantId. Fixes #7077 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…7079) The test used a single 100ms context for both phases: confirming Ready() blocks (50ms) and then waiting for it to unblock after Run() starts (remaining ~50ms). On slow CI machines, goroutine scheduling could consume the remaining budget. Split into two independent phases: a 50ms context expected to timeout (proving Ready() blocks), then a fresh 5s context for the second Ready() call that completes after Run() starts.
…7018) * update UI and add version check for failed ext * Update cli/azd/cmd/middleware/extensions.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * address feedback * address feedback * address feedback * clean error message --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…l extensions (#7080) * fix(extensions): apply same UserTenantId fix to all affected extensions Audit of all extensions found the same bug in 5 more extensions: - azure.ai.models (custom.go, init.go) - azure.ai.agents (init.go, init_from_code.go) - azure.ai.finetune (init.go) - microsoft.azd.ai.builder (start.go) - microsoft.azd.demo (prompt.go) Extensions using LookupTenant() (azure.appservice, azure.ai.agents parser.go/service_target_agent.go) are already correct since the server resolves to UserAccessTenantId. Fixes #7077 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: add Copilot review instruction for extension tenant usage Adds a path-scoped Copilot instruction for cli/azd/extensions/** that flags use of Subscription.TenantId (resource tenant) instead of Subscription.UserTenantId (user access tenant) for credential creation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rounds (#7063) * add cases for provision errors and make sure infra fix over az command * Update cli/azd/internal/mcp/tools/prompts/azd_provision_common_error.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update cli/azd/internal/mcp/tools/prompts/azd_provision_common_error.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * address feedback --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* add error suggestion yml rule and actions in error message * address feedback
* feat: add localFallback option for Docker remote build When remoteBuild is true and localFallback is true in azure.yaml, azd automatically falls back to a local Docker build if the remote ACR build fails. Displays a WARNING message when fallback triggers. This helps users on subscriptions that don't support ACR Tasks (e.g., free trial) by gracefully degrading to local Docker builds instead of failing outright. Changes: - Add localFallback field to DockerProjectOptions struct - Add fallback logic in ContainerHelper.Publish() - Update proto definition and generated code - Update azure.yaml JSON schema (service-level and docker-level) - Add mapper registry mappings - Add unit test for fallback behavior Fixes #4618 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * address PR review feedback - Include original error in fallback warning message - Fix proto field ordering (local_fallback after build_args) - Align struct tag spacing for LocalFallback - Remove service-level localFallback from schema (only docker-level) - Fix lint: break long test line under 125 chars Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: retrigger pipeline checks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: retrigger Windows build check Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: make local fallback the default when remoteBuild fails Remove the localFallback field from DockerProjectOptions and make fallback-to-local the default behavior when remoteBuild is true and the remote build fails. Before attempting the local build, azd now checks if Docker or Podman is installed and running via CheckInstalled, providing a clear error if neither is available. Changes: - Remove LocalFallback from DockerProjectOptions struct, proto, schema - Always fall back to local build on remote build failure - Validate Docker/Podman availability before local fallback - Reserve proto field number 10 to prevent reuse - Update remoteBuild schema description to document fallback behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: resolve lint errors (errorlint, gofmt) - Use %w instead of %s for dockerErr in container_helper.go (errorlint) - Fix struct field alignment in DockerProjectOptions (gofmt) - Fix struct literal alignment in mapper_registry.go (gofmt) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: correct gofmt struct field alignment after rebase Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* chore: apply go fix modernizers for Go 1.26 Run `go fix ./...` and `gofmt -s -w .` to apply automated Go 1.26 modernizations across the cli/azd codebase. Fixes #7082 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: add Qwen to cspell dictionary Add "Qwen" (AI model family name) to the cspell word list to fix the pre-existing spell check failure in azure.ai.models extension. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: retrigger build --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Initial plan * Fix: preserve Dapr configuration during container app deployment Co-authored-by: spboyer <7681382+spboyer@users.noreply.github.com> * fix: improve Dapr preservation error handling and add 404 test - Handle 404 (first deploy) explicitly in persistSettings instead of swallowing all errors — proceed without persisting when app does not exist yet - Fail on non-404 errors when Dapr preservation is needed to prevent silent config wipe (correctness-critical path) - Add test for first-deploy scenario (GET 404) verifying no Dapr config is injected - Fix pre-existing cspell issues (projectpkg, agentserver) - Apply go fix modernizations (to.Ptr -> new) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: spboyer <7681382+spboyer@users.noreply.github.com> Co-authored-by: Shayne Boyer <spboyer@live.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Update brew install/upgrade instructions * Include update module
* docs: add PR review patterns to AGENTS.md Add lessons learned from team and Copilot reviews across PRs #7290, #7251, #7250, #7247, #7236, #7235, #7202, #7039 as agent instructions to prevent recurring review findings. New/expanded sections: - Error handling: ErrorWithSuggestion field completeness, telemetry service attribution, scope-agnostic messages, link/suggestion parity, stale data in polling loops - Architecture boundaries: pkg/project target-agnostic, extension docs separation, env var verification against source code - Output formatting: shell-safe quoted paths, consistent JSON types - Path safety: traversal validation, quoted paths in messages - Code organization: extract shared logic across scopes - Documentation standards: help text consistency, no dead references, PR description accuracy - Testing best practices: test YAML rules e2e, extract shared helpers, correct env vars (AZD_FORCE_TTY, NO_COLOR), TypeScript patterns, reasonable timeouts, cross-platform paths, test new JSON fields - CI / GitHub Actions: permissions blocks, PATH handling, cross-workflow artifacts, prefer ADO for secrets, no placeholder steps Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix Copilot instructions for code review and strengthen guidance on Go patterns (#7320) * Fix Copilot instructions for code review and strengthen guidance on Go patterns * Update wording --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: JeffreyCA <jeffreychen@microsoft.com>
Bumps and [picomatch](https://github.com/micromatch/picomatch). These dependencies needed to be updated together. Updates `picomatch` from 2.3.1 to 2.3.2 - [Release notes](https://github.com/micromatch/picomatch/releases) - [Changelog](https://github.com/micromatch/picomatch/blob/master/CHANGELOG.md) - [Commits](micromatch/picomatch@2.3.1...2.3.2) Updates `picomatch` from 4.0.3 to 4.0.4 - [Release notes](https://github.com/micromatch/picomatch/releases) - [Changelog](https://github.com/micromatch/picomatch/blob/master/CHANGELOG.md) - [Commits](micromatch/picomatch@2.3.1...2.3.2) --- updated-dependencies: - dependency-name: picomatch dependency-version: 2.3.2 dependency-type: indirect - dependency-name: picomatch dependency-version: 4.0.4 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
When the user selects 'N' to preflight validation warnings, azd now stops immediately with exit code 0 instead of continuing (which caused a nil panic with custom service targets or proceeded to deploy without provisioned resources). Changes: - Add ErrAbortedByUser sentinel error for user-initiated aborts - ProvisionAction detects PreflightAbortedSkipped and returns the error - UX middleware swallows ErrAbortedByUser to produce exit code 0 - Workflow runner returns ErrAbortedByUser unwrapped to stop workflows - Error middleware skips AI error analysis for user aborts - Telemetry maps abort to internal.operation_aborted Fixes #7305 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bumps [yaml](https://github.com/eemeli/yaml) from 2.8.2 to 2.8.3. - [Release notes](https://github.com/eemeli/yaml/releases) - [Commits](eemeli/yaml@v2.8.2...v2.8.3) --- updated-dependencies: - dependency-name: yaml dependency-version: 2.8.3 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…eout (#7346) * fix: use 127.0.0.1 for gRPC server address and increase extension timeout The gRPC server binds to 127.0.0.1 (IPv4) but reported its address as "localhost", which on Windows can resolve to ::1 (IPv6). This mismatch causes the extension gRPC client to connect to the wrong address, hanging indefinitely until the timeout fires. Fix: report 127.0.0.1:PORT to match the actual bind address. Also increase the default extension startup timeout from 5s to 15s to accommodate Windows cold-start overhead (Defender scanning, process creation, gRPC handshake). Fixes #7304 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * address review: return shared serverInfo, fix stale comment - Return &serverInfo instead of constructing a duplicate ServerInfo, preventing address drift between auth interceptors and token generation. - Update inline comment from "5 seconds" to "15 seconds" to match the new default. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Initial plan * Create changelog for azd 1.23.13 Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/ec18918d-8b3a-40c9-b15a-4d9b273c0270 Co-authored-by: rajeshkamal5050 <11532743+rajeshkamal5050@users.noreply.github.com> * Remove PR #7293 from 1.23.13 changelog Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/c248a61d-9e5c-4091-b002-953c5c7ea4e7 Co-authored-by: rajeshkamal5050 <11532743+rajeshkamal5050@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: rajeshkamal5050 <11532743+rajeshkamal5050@users.noreply.github.com>
…ibutes (#7299) * feat: comprehensive telemetry audit - add command-specific usage attributes - Add telemetry to auth, config, env, hooks, templates, pipeline, monitor, show, infra commands - Add 16 new telemetry field constants for command-specific attributes - Fix user identity tracking with Anonymous account type fallback - Fix flaky TestStateCacheManager_TTL timing issue - Add audit documentation: feature matrix, schema, privacy checklist, audit process - Add telemetry field contract tests and CI coverage check Resolves #1772 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address PR review feedback (threads 1-8) - Rename test: TestTelemetryFieldConstants with clarified allowlist approach - pipeline.go: skip SetUsageAttributes for empty provider/auth values - docs: fix internal/telemetry/ -> cli/azd/internal/tracing/ paths - docs: add Anonymous to ad.account.type allowed values - docs: add missing legend symbol in feature-telemetry-matrix.md - docs: pick CODEOWNERS over GitHub Actions for telemetry PR labeling - docs: add opt-out rate estimation section with @AngelosP question - cspell: add metrics-audit word list for docs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: hash hook names since extensions can define arbitrary names Extensions register custom hooks via WithProjectEventHandler/WithServiceEventHandler with arbitrary string names that are not validated against a fixed set. Hash the hook name to prevent potential PII leakage in telemetry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: emit default values for pipeline/infra telemetry, revert hook hashing - pipeline.provider: emit 'auto' when user doesn't specify --provider - pipeline.auth.type: emit 'auto' when user doesn't specify --auth-type - infra.provider: emit 'auto' when provider not set in project config - hooks.name: revert to raw string (not hashed) for telemetry readability - audit-process.md: add telemetry validation pipeline section - telemetry-schema.md: document 'auto' as valid value for provider fields Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: add CODEOWNERS to cspell word list Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: remove redundant telemetry attributes per review feedback Remove attributes that duplicate data already captured by command span names (config.operation, env.operation, template.operation), OTel span status (auth.result), or cmd.flags flag names (monitor.type). Remove show.output.format (should be global, tracked as follow-up). Remove dead code Anonymous fallback in manager.go. 8 unique attributes remain: auth.method, auth.tenant.id.hashed, env.count, hooks.name, hooks.type, infra.provider, pipeline.provider, pipeline.auth.type. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: gofmt formatting in telemetry coverage test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: add weikanglim to cspell word list Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: log resolved pipeline provider instead of 'auto' sentinel Use CiProviderName() to log the actual resolved provider name after auto-detection instead of the 'auto' placeholder. For auth type, only log when explicitly specified — cmd.flags absence indicates auto-detection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address Wei pass-2 review feedback - Remove 'logout' as auth.method value (not an auth method) - Unhash tenant ID (infrastructure GUID, not PII) - Revert unrelated cosmetic change in auth_status.go - Revert unrelated if/else logic change in manager.go - Remove redundant TestFieldKeyValues test - Rename tenant key from ad.tenant.id to auth.tenant.id Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: re-trigger pipeline (Mac build flake) * fix: address spboyer review - schema doc mismatch, missing infra generate, undocumented check-status - Update telemetry-schema.md: ad.tenant.id -> auth.tenant.id to match code - Add check-status to auth.method allowed values, remove stale logout - Add missing 'infra generate' to commandsWithSpecificTelemetry manifest Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: validate hook names before logging to telemetry Known built-in hook names (pre/post build, deploy, etc.) are logged raw. Unknown/extension-defined hook names are hashed via SHA-256 to avoid logging arbitrary user input as customer content. Addresses review feedback from @weikanglim on hook name telemetry. Tracks extension hook telemetry gap in issue #7326. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove audit test and revert TenantIdKey to ad.tenant.id - Remove fields_audit_test.go per Wei's feedback — test duplicated field constants without clear value; snapshot testing would be preferred. - Revert TenantIdKey from auth.tenant.id back to ad.tenant.id to avoid data contract change on existing context-level field. - Update telemetry-schema.md to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…7343) Add post-unmarshal validation that catches nil service, resource, and hook definitions before they can cause nil-pointer panics. All problems are collected into a single ConfigValidationError with actionable messages so users can fix everything in one pass. Validation covers: - Services with empty definitions (nil *ServiceConfig) - Resources with empty definitions (nil *ResourceConfig) - Hooks with nil slices or nil entries at project and service levels - Hooks merged from *.hooks.yaml infra module files in Load() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* update design docs and hide auto update from help * update message * address copilot feedback * golangci-lint * golangci * remove auto update part instead of comment out * gofmt
* Initial plan * fix: reuse existing env during ai agent init reruns Co-authored-by: JeffreyCA <9157833+JeffreyCA@users.noreply.github.com> Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/83ae2408-eb24-4ff5-b22a-67ad44375e09 * test: tighten env reuse helper coverage Co-authored-by: JeffreyCA <9157833+JeffreyCA@users.noreply.github.com> Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/83ae2408-eb24-4ff5-b22a-67ad44375e09 * chore: revert unrelated concurx module changes Co-authored-by: JeffreyCA <9157833+JeffreyCA@users.noreply.github.com> Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/83ae2408-eb24-4ff5-b22a-67ad44375e09 * Address feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JeffreyCA <9157833+JeffreyCA@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Copilot PR reviewer flagged 5 false positives on PR #7223 because it didn't know about Go 1.26 features (e.g. new(expr) for pointer literals) and incorrectly flagged missing imports that existed outside the diff context. These patterns are documented in cli/azd/AGENTS.md but the PR reviewer reads .github/instructions/*.instructions.md files, not AGENTS.md. This adds a focused Go-specific instruction file that covers: - new(expr) pointer literal syntax (Go 1.26) - Other modern Go patterns that should not be flagged - Guidance to check full file context, not just diff hunks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bumps [github.com/buger/jsonparser](https://github.com/buger/jsonparser) from 1.1.1 to 1.1.2. - [Release notes](https://github.com/buger/jsonparser/releases) - [Commits](buger/jsonparser@v1.1.1...v1.1.2) --- updated-dependencies: - dependency-name: github.com/buger/jsonparser dependency-version: 1.1.2 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/buger/jsonparser](https://github.com/buger/jsonparser) from 1.1.1 to 1.1.2. - [Release notes](https://github.com/buger/jsonparser/releases) - [Commits](buger/jsonparser@v1.1.1...v1.1.2) --- updated-dependencies: - dependency-name: github.com/buger/jsonparser dependency-version: 1.1.2 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Extension commands use DisableFlagParsing, so cobra never parses global flags like -e/--environment, --debug, or --cwd. This caused two problems: 1. The DI-resolved environment always loaded the default instead of the one specified with -e, leaking wrong env vars into extension processes and never setting AZD_ENVIRONMENT (#7034). 2. --debug and --cwd were also not propagated to extensions because extensions.go read them from cmd.Flags() which returns defaults. Fix by: - Adding -e/--environment to ParseGlobalFlags() with lenient validation: valid env names are accepted, non-env values (like URLs that extensions pass via -e) are silently skipped so extensions still work. - Adding EnvironmentName to GlobalCommandOptions so the pre-parsed value is available to the DI container and extension runner. - Updating container.go EnvFlag resolver to fall back to globalOptions when cmd.Flags() returns empty (extension commands). - Updating extensions.go to use globalOptions for all InvokeOptions fields (debug, cwd, environment, no-prompt) instead of cmd.Flags(). Closes #7034 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Agent detection (agentdetect package) walks the parent process tree and auto-enables --no-prompt when it finds an AI coding agent. In CI and local dev under Copilot CLI, this causes functional tests to fail because piped stdin is ignored when no-prompt is active. Changes: - detect.go: Early return from detectAgent() when AZD_DISABLE_AGENT_DETECT is set, suppressing both env var and parent process detection - cli.go: Set AZD_DISABLE_AGENT_DETECT=1 on all child azd processes in RunCommandWithStdIn(), with nil-Env safety (nil means inherit-all in Go) - detect_test.go: Test that AZD_DISABLE_AGENT_DETECT suppresses detection - env_test.go: Fix require.Fail -> require.Failf format string bug Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The gosec linter flags os.LookupEnv values as tainted input for log injection (G706). Remove the env var value from the log message since only the presence of the env var matters, not its value. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Workflow steps that specify their own -e/--environment flag (e.g. 'azd: env set KEY VALUE -e env1') were getting the parent command's --environment appended via extractGlobalArgs(), causing the parent's value to override the step's explicit value. The environment flag is now excluded from extractGlobalArgs() since environment propagation to workflow steps is already handled by the globalOptions DI fallback in the EnvFlag resolver. Fixes Test_CLI_Up_EnvironmentFlags. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
force-pushed
the
fix/env-leak-extensions
branch
from
14fb280 to
ff5d8a2
Compare
jongio
requested review from
achauhan-scc,
bwateratmsft,
danieljurek,
karolz-ms,
kingernupur,
rabollin,
saanikaguptamicrosoft,
therealjohn,
trangevi and
trrwilson
as code owners
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
This is a redo of #7035 (which was reverted by #7274) with the prerequisite work done first. It fixes two problems:
Environment variable leak (azd <extension> -e <env> leaks default environment variables into extension process #7034): Extensions never received the correct
-e/--environmentvalue because extension commands useDisableFlagParsing: true, so cobra never parsed the flag. The DI resolver always fell back to the default environment.Flag propagation broken by revert (Revert: Fix env var leak when running extension commands with -e flag (#7035) #7274): The revert also broke
--debugand--cwdpropagation to extensions, since it changedextensions.goback to usingcmd.Flags()which returns defaults for extension commands.What changed
global_command_options.go: AddedEnvironmentNamefieldauto_install.go: Added-e/--environmenttoCreateGlobalFlagSet()with strict validation inParseGlobalFlags()(rejects invalid env names with clear error)container.go: UpdatedEnvFlagDI resolver to fall back toglobalOptions.EnvironmentNameextensions.go: UsesglobalOptions(populated before cobra) for ALLInvokeOptionsfields (debug, cwd, environment, no-prompt)environment.go: Added exportedInvalidEnvironmentNameError()for shared validation across all call sitesmanager.go: Replaced 3 inconsistent error message formats with the sharedInvalidEnvironmentNameError()-e, --environmentflag in help textauto_install_test.go: 11 new subtests (6 valid env name + 5 invalid env name)Key difference from #7035
PR #7035 added strict
-evalidation which broke extensions that reused-efor URLs. This PR is safe because PR #7313 migrates extensions off-efirst.How
globalOptionssolves itParseGlobalFlags()runs before cobra, manually parsing the rawos.Args. For extension commands (DisableFlagParsing: true), cobra skips all flag parsing, butglobalOptionsalready has the correct values. Both the DI resolver and extension invocation now read fromglobalOptionsinstead ofcmd.Flags().Closes #7034
Closes #7271
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com