Fix #3355: Add validation for environment names in azd env list #6463

spboyer · 2026-01-07T14:12:08Z

Add IsValid field to EnvListEnvironment contract
Update LocalFileDataStore.List to validate environment names
Add STATUS column to env list table output
Display warning message for invalid environment names
Add validation to manager.Get() to reject invalid names
Add tests for invalid environment name scenarios

- Add IsValid field to EnvListEnvironment contract - Update LocalFileDataStore.List to validate environment names - Add STATUS column to env list table output - Display warning message for invalid environment names - Add validation to manager.Get() to reject invalid names - Add tests for invalid environment name scenarios

Copilot

Pull request overview

This PR addresses issue #3355 by adding validation for environment names in the azd env list command. The implementation enhances user experience by clearly identifying invalid environment names that may cause issues with azd commands.

Key changes:

Adds IsValid field to the environment list contract to track validation status
Implements early validation in manager.Get() to reject operations on invalid environment names
Enhances azd env list output with a STATUS column showing validation status and a warning message for invalid environments

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
cli/azd/pkg/contracts/env_list.go	Adds `IsValid` boolean field to the `EnvListEnvironment` contract to indicate whether an environment name passes validation
cli/azd/pkg/environment/local_file_data_store.go	Updates `List()` method to populate the `IsValid` field by calling `IsValidEnvironmentName()` for each environment directory
cli/azd/pkg/environment/local_file_data_store_test.go	Adds comprehensive test validating that environments with valid and invalid names are correctly marked in the list results
cli/azd/pkg/environment/manager.go	Adds validation check in `Get()` to reject invalid environment names early, and propagates `IsValid` field in `List()` for both local and remote environments
cli/azd/pkg/environment/manager_test.go	Adds test cases verifying that `manager.Get()` rejects various invalid environment name patterns
cli/azd/cmd/env.go	Adds STATUS column to table output showing validation status with icons, and displays warning message when invalid environments are detected (table format only)

cli/azd/pkg/environment/manager.go

Copilot · 2026-01-07T14:16:48Z

cli/azd/pkg/environment/manager_test.go

+	t.Run("InvalidEnvironmentName", func(t *testing.T) {
+		localDataStore := &MockDataStore{}
+
+		manager := newManagerForTest(azdContext, mockContext.Console, localDataStore, nil)
+
+		// Test various invalid names
+		invalidNames := []string{
+			"#invalid",
+			"$bad%name",
+			"no spaces",
+			"no*asterisk",
+		}
+
+		for _, invalidName := range invalidNames {
+			env, err := manager.Get(*mockContext.Context, invalidName)
+			require.Error(t, err, "Should error for invalid name: %s", invalidName)
+			require.Nil(t, env)
+			require.Contains(t, err.Error(), "invalid")
+			require.Contains(t, err.Error(), "Valid names can only contain")
+		}
+
+		// localDataStore.Get should never be called for invalid names
+		localDataStore.AssertNotCalled(t, "Get")
+	})


The test implementation doesn't follow the table-driven testing pattern which is a standard practice in this codebase (as mentioned in the coding guidelines). Consider refactoring this test to use a table-driven approach for better maintainability and easier addition of new test cases. The test should be structured with a slice of test cases containing name, input, and expected error patterns.

cli/azd/cmd/env.go

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

vhvb1989

tks. We might need to look into azd env select as well.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Copilot · 2026-01-08T17:44:26Z

cli/azd/pkg/environment/manager.go

+
 	// Check cache first
 	cached, err := m.getFromCache(ctx, name)


This code contains duplicate lines. Lines 414-415 and lines 417-418 are identical. The comment "Check cache first" and the variable assignment "cached, err := m.getFromCache(ctx, name)" appear twice. Please remove lines 414-415 to eliminate the duplication.

Suggested change

// Check cache first

cached, err := m.getFromCache(ctx, name)

Copilot · 2026-01-08T17:44:27Z

cli/azd/pkg/environment/manager_test.go

+			require.Error(t, err, "Should error for invalid name: %s", invalidName)
+			require.Nil(t, env)
+			require.Contains(t, err.Error(), "invalid")
+			require.Contains(t, err.Error(), "Valid names can only contain")


The test expects the error message to contain "Valid names can only contain", but the actual error message returned by invalidEnvironmentNameMsg() says "it should contain only alphanumeric characters and hyphens". This test will fail because the error message doesn't match the expected substring. The test expectation should be updated to match the actual error message, for example by checking for "invalid" and "alphanumeric" instead.

Suggested change

require.Contains(t, err.Error(), "Valid names can only contain")

require.Contains(t, err.Error(), "alphanumeric")

wbreza

I'm not quite sure this is solving some customer pain point... Yes - users could manually create an environment that is invalid - but is this something users are strugging with?

vhvb1989 · 2026-01-12T19:44:59Z

I'm not quite sure this is solving some customer pain point... Yes - users could manually create an environment that is invalid - but is this something users are strugging with?

I think that in most cases it is not the user manually creating the environment, but some automation. Like AI agents skipping the call to azd env new .... and directly trying to write the expected files. I see this change as a protection against nils

Copilot AI review requested due to automatic review settings January 7, 2026 14:12

spboyer requested review from JeffreyCA, hemarina, rajeshkamal5050, tg-msft, vhvb1989, wbreza and weikanglim as code owners January 7, 2026 14:12

microsoft-github-policy-service bot assigned spboyer Jan 7, 2026

Copilot started reviewing on behalf of spboyer January 7, 2026 14:12 View session

Copilot AI reviewed Jan 7, 2026

View reviewed changes

Copilot AI mentioned this pull request Jan 7, 2026

Standardize environment name validation messages using DRY principle spboyer/azure-dev#3

Draft

spboyer and others added 2 commits January 8, 2026 12:38

Update cli/azd/pkg/environment/manager.go

827a080

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update cli/azd/cmd/env.go

d7d4098

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

spboyer requested a review from Copilot January 8, 2026 17:39

Copilot started reviewing on behalf of spboyer January 8, 2026 17:40 View session

vhvb1989 approved these changes Jan 8, 2026

View reviewed changes

Copilot AI reviewed Jan 8, 2026

View reviewed changes

wbreza reviewed Jan 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix #3355: Add validation for environment names in azd env list #6463

Fix #3355: Add validation for environment names in azd env list #6463

Uh oh!

spboyer commented Jan 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Jan 7, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

vhvb1989 left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

wbreza left a comment

Uh oh!

vhvb1989 commented Jan 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		// Check cache first
		cached, err := m.getFromCache(ctx, name)

	require.Contains(t, err.Error(), "Valid names can only contain")
	require.Contains(t, err.Error(), "alphanumeric")

Fix #3355: Add validation for environment names in azd env list #6463

Are you sure you want to change the base?

Fix #3355: Add validation for environment names in azd env list #6463

Uh oh!

Conversation

spboyer commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

vhvb1989 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

wbreza left a comment

Choose a reason for hiding this comment

Uh oh!

vhvb1989 commented Jan 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

spboyer commented Jan 7, 2026 •

edited

Loading