Skip to content

Commit 5f989ee

Browse files
jongioCopilot
andauthored
Fix environment variable leak and flag propagation for extensions (#7314)
* Test aks * Fix environment variable leak and flag propagation for extensions 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> * fix: add AZD_DISABLE_AGENT_DETECT for functional tests 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> * fix: remove tainted env var value from log to satisfy gosec G706 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> * ci: retrigger pipeline Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: exclude --environment from workflow step global args 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> * fix: add missing color import in detect_confirm_apphost.go * Use lenient validation for -e/--environment in global flag parsing Change ParseGlobalFlags to silently ignore invalid environment names instead of returning an error. Valid environment names match the regex [a-zA-Z0-9-()_.]{1,64} which can never match URLs containing ://, so there is natural disambiguation between azd env names and extension flags that reuse -e for other purposes (e.g., --project-endpoint). This fixes the environment variable leak (azd env name not propagating to workflow steps) while preserving backward compatibility with all extensions — including third-party ones that use -e for their own flags. PR 7313 (migrating first-party extensions off -e) is no longer a hard prerequisite since invalid values are silently skipped, not rejected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update TestFigSpec snapshot to match current extension output Remove stale --host option from ai agents init that was incorrectly kept during rebase conflict resolution. The current azure.ai.agents extension no longer exposes this option. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add extension compatibility regression test for -e flag Add TestParseGlobalFlags_ExtensionCompatibility that verifies real extension scenarios (azure.ai.models, azure.ai.finetune) using -e for --project-endpoint URLs are not broken by global -e/--environment flag parsing. Covers: URL endpoints, --project-endpoint passthrough, valid env before extension args, mixed global flags, and pflag last-value-wins behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix line-length lint violations in extension compatibility tests Break long args slices across multiple lines to comply with the 125-char lll linter rule enforced by golangci-lint. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: retrigger CI after flaky test failures * fix: use EnvironmentNameFlagName constant and add debug logging - Replace string literal 'environment' with internal.EnvironmentNameFlagName constant in CreateGlobalFlagSet and ParseGlobalFlags to prevent drift - Add debug-level log when an invalid environment name is silently ignored, making the lenient skip observable when --debug is enabled Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: remove accidental dev/null file Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 948f710 commit 5f989ee

76 files changed

Lines changed: 689 additions & 377 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

cli/azd/cmd/auto_install.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/azure/azure-dev/cli/azd/internal"
1818
"github.com/azure/azure-dev/cli/azd/internal/runcontext/agentdetect"
1919
"github.com/azure/azure-dev/cli/azd/internal/tracing/resource"
20+
"github.com/azure/azure-dev/cli/azd/pkg/environment"
2021
"github.com/azure/azure-dev/cli/azd/pkg/extensions"
2122
"github.com/azure/azure-dev/cli/azd/pkg/input"
2223
"github.com/azure/azure-dev/cli/azd/pkg/ioc"
@@ -623,6 +624,7 @@ func CreateGlobalFlagSet() *pflag.FlagSet {
623624
false,
624625
"Alias for --no-prompt.")
625626
_ = globalFlags.MarkHidden("non-interactive")
627+
globalFlags.StringP(internal.EnvironmentNameFlagName, "e", "", "The name of the environment to use.")
626628

627629
// The telemetry system is responsible for reading these flags value and using it to configure the telemetry
628630
// system, but we still need to add it to our flag set so that when we parse the command line with Cobra we
@@ -705,6 +707,24 @@ func ParseGlobalFlags(args []string, opts *internal.GlobalCommandOptions) error
705707
}
706708
}
707709

710+
// Parse -e/--environment with lenient validation.
711+
// Only accept values that look like valid environment names (alphanumeric, hyphens, dots,
712+
// underscores). Values that don't match (e.g., URLs from extensions reusing -e for
713+
// --project-endpoint) are silently ignored — the extension still receives the raw args
714+
// and can parse -e itself. This avoids breaking third-party extensions that use -e
715+
// for their own flags while still fixing the environment leak for valid env names.
716+
if strVal, err := globalFlagSet.GetString(internal.EnvironmentNameFlagName); err == nil && strVal != "" {
717+
if environment.IsValidEnvironmentName(strVal) {
718+
opts.EnvironmentName = strVal
719+
} else if opts.EnableDebugLogging {
720+
log.Printf(
721+
"debug: ignoring invalid environment name %q from -e/--environment flag"+
722+
" (does not match %s pattern)",
723+
strVal, environment.EnvironmentNameRegexp,
724+
)
725+
}
726+
}
727+
708728
// Agent Detection: If no explicit flag or env var was set and we detect an AI coding
709729
// agent as the caller, automatically enable no-prompt mode for non-interactive execution.
710730
if !flagExplicitlySet && !envVarPresent && agentdetect.IsRunningInAgent() {

cli/azd/cmd/auto_install_test.go

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,3 +568,189 @@ func TestParseGlobalFlags_NonInteractiveAliasAndEnvVar(t *testing.T) {
568568
agentdetect.ResetDetection()
569569
})
570570
}
571+
572+
func TestParseGlobalFlags_EnvironmentName(t *testing.T) {
573+
tests := []struct {
574+
name string
575+
args []string
576+
expectedEnvName string
577+
}{
578+
{
579+
name: "valid env name with -e",
580+
args: []string{"-e", "dev", "up"},
581+
expectedEnvName: "dev",
582+
},
583+
{
584+
name: "valid env name with --environment",
585+
args: []string{"--environment", "production", "deploy"},
586+
expectedEnvName: "production",
587+
},
588+
{
589+
name: "valid env name with equals syntax",
590+
args: []string{"--environment=staging", "deploy"},
591+
expectedEnvName: "staging",
592+
},
593+
{
594+
name: "env name with dots and hyphens",
595+
args: []string{"-e", "my-env.v2", "up"},
596+
expectedEnvName: "my-env.v2",
597+
},
598+
{
599+
name: "empty value",
600+
args: []string{"up"},
601+
expectedEnvName: "",
602+
},
603+
{
604+
name: "env name alongside other global flags",
605+
args: []string{"--debug", "-e", "myenv", "--no-prompt", "deploy"},
606+
expectedEnvName: "myenv",
607+
},
608+
}
609+
610+
for _, tt := range tests {
611+
t.Run(tt.name, func(t *testing.T) {
612+
// Clear agent detection to avoid NoPrompt side effects
613+
clearAgentEnvVarsForTest(t)
614+
agentdetect.ResetDetection()
615+
616+
opts := &internal.GlobalCommandOptions{}
617+
err := ParseGlobalFlags(tt.args, opts)
618+
require.NoError(t, err)
619+
620+
assert.Equal(t, tt.expectedEnvName, opts.EnvironmentName,
621+
"EnvironmentName should be %q for test case: %s", tt.expectedEnvName, tt.name)
622+
623+
agentdetect.ResetDetection()
624+
})
625+
}
626+
}
627+
628+
func TestParseGlobalFlags_InvalidEnvironmentName(t *testing.T) {
629+
// Invalid environment names are silently ignored (not errors) so that
630+
// third-party extensions reusing -e for their own flags (e.g., URLs)
631+
// are not broken by azd's global flag parsing.
632+
tests := []struct {
633+
name string
634+
args []string
635+
}{
636+
{
637+
name: "URL value",
638+
args: []string{"-e", "https://foo.services.ai.azure.com/api/projects/bar", "model", "custom", "create"},
639+
},
640+
{
641+
name: "value with colons",
642+
args: []string{"-e", "host:port", "model", "custom", "create"},
643+
},
644+
{
645+
name: "value with slashes",
646+
args: []string{"-e", "path/to/thing", "model", "custom", "create"},
647+
},
648+
{
649+
name: "value with spaces",
650+
args: []string{"-e", "env name with spaces"},
651+
},
652+
{
653+
name: "special characters",
654+
args: []string{"-e", "env@#$%"},
655+
},
656+
}
657+
658+
for _, tt := range tests {
659+
t.Run(tt.name, func(t *testing.T) {
660+
clearAgentEnvVarsForTest(t)
661+
agentdetect.ResetDetection()
662+
663+
opts := &internal.GlobalCommandOptions{}
664+
err := ParseGlobalFlags(tt.args, opts)
665+
require.NoError(t, err, "invalid env names should be silently ignored, not rejected")
666+
assert.Empty(t, opts.EnvironmentName,
667+
"EnvironmentName should be empty when -e value is not a valid env name")
668+
669+
agentdetect.ResetDetection()
670+
})
671+
}
672+
}
673+
674+
// TestParseGlobalFlags_ExtensionCompatibility verifies that extensions reusing -e for their
675+
// own flags (e.g., azure.ai.models uses -e/--project-endpoint) work correctly alongside
676+
// azd's global -e/--environment flag. This is a regression test for the bug that caused
677+
// PR #7035 to be reverted (PR #7274): strict validation of -e values rejected URLs passed
678+
// by extensions, breaking commands like `azd ai models custom create -e https://...`.
679+
func TestParseGlobalFlags_ExtensionCompatibility(t *testing.T) {
680+
tests := []struct {
681+
name string
682+
args []string
683+
expectedEnvName string
684+
description string
685+
}{
686+
{
687+
name: "azure.ai.models: -e with project endpoint URL",
688+
args: []string{
689+
"ai", "models", "custom", "create",
690+
"-e", "https://myaccount.services.ai.azure.com/api/projects/myproject",
691+
},
692+
expectedEnvName: "",
693+
description: "URL must not be captured as env name; extension receives raw args",
694+
},
695+
{
696+
name: "azure.ai.models: --project-endpoint with URL",
697+
args: []string{
698+
"ai", "models", "custom", "create",
699+
"--project-endpoint", "https://myaccount.services.ai.azure.com/api/projects/myproject",
700+
},
701+
expectedEnvName: "",
702+
description: "--project-endpoint is not a global flag, should be ignored",
703+
},
704+
{
705+
name: "valid env name before extension args",
706+
args: []string{
707+
"-e", "dev", "ai", "models", "custom", "create",
708+
"--project-endpoint", "https://endpoint.com",
709+
},
710+
expectedEnvName: "dev",
711+
description: "valid -e before extension subcommand should be captured",
712+
},
713+
{
714+
name: "extension -e URL with other global flags",
715+
args: []string{
716+
"--debug", "ai", "models", "custom", "create",
717+
"-e", "https://endpoint.com", "--no-prompt",
718+
},
719+
expectedEnvName: "",
720+
description: "URL via -e among global flags must not error or capture",
721+
},
722+
{
723+
name: "azure.ai.finetune: -e with endpoint URL",
724+
args: []string{
725+
"ai", "fine-tuning", "init",
726+
"-e", "https://ai-endpoint.azure.com/v1",
727+
},
728+
expectedEnvName: "",
729+
description: "fine-tuning extension's -e must not be captured",
730+
},
731+
{
732+
name: "both --environment and -e URL: last value wins per pflag",
733+
args: []string{
734+
"--environment", "staging", "ai", "models", "custom", "create",
735+
"-e", "https://endpoint.com",
736+
},
737+
expectedEnvName: "",
738+
description: "pflag takes last -e value (the URL), " +
739+
"which is invalid so env name stays empty",
740+
},
741+
}
742+
743+
for _, tt := range tests {
744+
t.Run(tt.name, func(t *testing.T) {
745+
clearAgentEnvVarsForTest(t)
746+
agentdetect.ResetDetection()
747+
748+
opts := &internal.GlobalCommandOptions{}
749+
err := ParseGlobalFlags(tt.args, opts)
750+
require.NoError(t, err, "ParseGlobalFlags must not error for extension args: %s", tt.description)
751+
assert.Equal(t, tt.expectedEnvName, opts.EnvironmentName, tt.description)
752+
753+
agentdetect.ResetDetection()
754+
})
755+
}
756+
}

cli/azd/cmd/container.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,12 @@ func registerCommonDependencies(container *ioc.NestedContainer) {
189189
return writer
190190
})
191191

192-
container.MustRegisterScoped(func(ctx context.Context, cmd *cobra.Command) internal.EnvFlag {
193-
// The env flag `-e, --environment` is available on most azd commands but not all
192+
container.MustRegisterScoped(func(
193+
ctx context.Context,
194+
cmd *cobra.Command,
195+
globalOptions *internal.GlobalCommandOptions,
196+
) internal.EnvFlag {
197+
// The env flag `-e, --environment` is available on most azd commands but not all.
194198
// This is typically used to override the default environment and is used for bootstrapping other components
195199
// such as the azd environment.
196200
// If the flag is not available, don't panic, just return an empty string which will then allow for our default
@@ -201,6 +205,13 @@ func registerCommonDependencies(container *ioc.NestedContainer) {
201205
envValue = ""
202206
}
203207

208+
// For extension commands (DisableFlagParsing=true), cobra never parses -e so
209+
// cmd.Flags().GetString always returns "". Fall back to the value that was
210+
// pre-parsed in ParseGlobalFlags before the command tree was built.
211+
if envValue == "" && globalOptions.EnvironmentName != "" {
212+
envValue = globalOptions.EnvironmentName
213+
}
214+
204215
if envValue == "" {
205216
// If no explicit environment flag was set, but one was provided
206217
// in the context, use that instead.
@@ -981,6 +992,11 @@ func (w *workflowCmdAdapter) ExecuteContext(ctx context.Context, args []string)
981992
// extractGlobalArgs extracts global flag arguments from the process command line.
982993
// It parses os.Args against the global flag set and returns only the flags that were
983994
// explicitly set by the user, formatted as command-line arguments.
995+
//
996+
// The "environment" flag is intentionally excluded: workflow steps may define their own
997+
// -e/--environment (e.g. `azd: env set KEY VALUE -e env1`), and appending the parent's
998+
// --environment would override the step-level value. Environment propagation to workflow
999+
// steps is handled by the globalOptions DI fallback in the EnvFlag resolver instead.
9841000
func extractGlobalArgs() []string {
9851001
globalFlagSet := CreateGlobalFlagSet()
9861002
globalFlagSet.SetOutput(io.Discard)
@@ -989,7 +1005,7 @@ func extractGlobalArgs() []string {
9891005

9901006
var result []string
9911007
globalFlagSet.VisitAll(func(f *pflag.Flag) {
992-
if f.Changed {
1008+
if f.Changed && f.Name != internal.EnvironmentNameFlagName {
9931009
// Use --flag=value syntax to avoid ambiguity. The two-arg form (--flag value)
9941010
// doesn't work for boolean flags, where the value is treated as a positional arg.
9951011
result = append(result, fmt.Sprintf("--%s=%s", f.Name, f.Value.String()))

cli/azd/cmd/extensions.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -259,22 +259,22 @@ func (a *extensionAction) Run(ctx context.Context) (*actions.ActionResult, error
259259
allEnv = append(allEnv, traceEnv...)
260260
}
261261

262-
// Read global flags for propagation via InvokeOptions
263-
debugEnabled, _ := a.cmd.Flags().GetBool("debug")
264-
cwd, _ := a.cmd.Flags().GetString("cwd")
265-
envName, _ := a.cmd.Flags().GetString("environment")
266-
262+
// Use globalOptions for flag propagation instead of cmd.Flags().
263+
// Extension commands use DisableFlagParsing=true, so cobra never parses
264+
// global flags like --debug, --cwd, or -e. The globalOptions were populated
265+
// by ParseGlobalFlags() before command tree construction and are the only
266+
// reliable source for these values.
267267
options := &extensions.InvokeOptions{
268268
Args: a.args,
269269
Env: allEnv,
270270
// cmd extensions are always interactive (connected to terminal)
271271
Interactive: true,
272-
Debug: debugEnabled,
272+
Debug: a.globalOptions.EnableDebugLogging,
273273
// Use globalOptions.NoPrompt which includes agent detection,
274274
// not just the --no-prompt CLI flag
275275
NoPrompt: a.globalOptions.NoPrompt,
276-
Cwd: cwd,
277-
Environment: envName,
276+
Cwd: a.globalOptions.Cwd,
277+
Environment: a.globalOptions.EnvironmentName,
278278
}
279279

280280
_, invokeErr := a.extensionRunner.Invoke(ctx, extension, options)

cli/azd/cmd/testdata/TestFigSpec.ts

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -226,15 +226,6 @@ const completionSpec: Fig.Spec = {
226226
name: ['init'],
227227
description: 'Initialize a new AI agent project. (Preview)',
228228
options: [
229-
{
230-
name: ['--environment', '-e'],
231-
description: 'The name of the azd environment to use.',
232-
args: [
233-
{
234-
name: 'environment',
235-
},
236-
],
237-
},
238229
{
239230
name: ['--manifest', '-m'],
240231
description: 'Path or URI to an agent manifest to add to your azd project',
@@ -426,15 +417,6 @@ const completionSpec: Fig.Spec = {
426417
name: ['init'],
427418
description: 'Initialize a new AI Fine-tuning project. (Preview)',
428419
options: [
429-
{
430-
name: ['--environment', '-n'],
431-
description: 'The name of the azd environment to use.',
432-
args: [
433-
{
434-
name: 'environment',
435-
},
436-
],
437-
},
438420
{
439421
name: ['--from-job', '-j'],
440422
description: 'Clone configuration from an existing job ID',
@@ -1149,15 +1131,6 @@ const completionSpec: Fig.Spec = {
11491131
name: ['init'],
11501132
description: 'Initialize a new AI models project. (Preview)',
11511133
options: [
1152-
{
1153-
name: ['--environment', '-n'],
1154-
description: 'The name of the azd environment to use',
1155-
args: [
1156-
{
1157-
name: 'environment',
1158-
},
1159-
],
1160-
},
11611134
{
11621135
name: ['--project-endpoint', '-e'],
11631136
description: 'Azure AI Foundry project endpoint URL (e.g., https://account.services.ai.azure.com/api/projects/project-name)',
@@ -3689,6 +3662,16 @@ const completionSpec: Fig.Spec = {
36893662
description: 'Enables debugging and diagnostics logging.',
36903663
isPersistent: true,
36913664
},
3665+
{
3666+
name: ['--environment', '-e'],
3667+
description: 'The name of the environment to use.',
3668+
isPersistent: true,
3669+
args: [
3670+
{
3671+
name: 'environment',
3672+
},
3673+
],
3674+
},
36923675
{
36933676
name: ['--no-prompt'],
36943677
description: 'Accepts the default value instead of prompting, or it fails if there is no default.',

cli/azd/cmd/testdata/TestUsage-azd-add.snap

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@ Usage
55
azd add [flags]
66

77
Global Flags
8-
-C, --cwd string : Sets the current working directory.
9-
--debug : Enables debugging and diagnostics logging.
10-
--docs : Opens the documentation for azd add in your web browser.
11-
-h, --help : Gets help for add.
12-
--no-prompt : Accepts the default value instead of prompting, or it fails if there is no default.
8+
-C, --cwd string : Sets the current working directory.
9+
--debug : Enables debugging and diagnostics logging.
10+
--docs : Opens the documentation for azd add in your web browser.
11+
-e, --environment string : The name of the environment to use.
12+
-h, --help : Gets help for add.
13+
--no-prompt : Accepts the default value instead of prompting, or it fails if there is no default.
1314

1415
Find a bug? Want to let us know how we're doing? Fill out this brief survey: https://aka.ms/azure-dev/hats.
1516

0 commit comments

Comments
 (0)