diff --git a/cli/azd/extensions/azure.ai.agents/cspell.yaml b/cli/azd/extensions/azure.ai.agents/cspell.yaml index 1545a88bed3..2d5c10d89d6 100644 --- a/cli/azd/extensions/azure.ai.agents/cspell.yaml +++ b/cli/azd/extensions/azure.ai.agents/cspell.yaml @@ -28,6 +28,7 @@ words: - westeurope # Project terms - ABAC + - acrpush - agentignore - ADLS - agentserver @@ -35,6 +36,7 @@ words: - Alphanum - anonymousconnection - aoai + - authorizationfailed - azdaiagent - CLIENTSECRET - curr @@ -46,6 +48,7 @@ words: - hostedagent - hostedagents - kval + - listbuildsourceuploadurl - logstream - mcpservertoolalwaysrequireapprovalmode - mcpservertoolneverrequireapprovalmode @@ -58,6 +61,7 @@ words: - projectpkg - protocolversionrecord - Qdrant + - schedulerun - Toolsets - underscoped - Vnext diff --git a/cli/azd/extensions/azure.ai.agents/internal/exterrors/codes.go b/cli/azd/extensions/azure.ai.agents/internal/exterrors/codes.go index 2bcc78683d6..449dff025fd 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/exterrors/codes.go +++ b/cli/azd/extensions/azure.ai.agents/internal/exterrors/codes.go @@ -62,6 +62,7 @@ const ( // Error codes for ACR dependency errors. const ( CodePrivateACRNetworkAccessFailed = "private_acr_network_access_failed" + CodeACRPermissionDenied = "acr_permission_denied" ) // Error codes commonly used for auth errors. diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go b/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go index 042204f2922..1aa2c2631ea 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go @@ -618,6 +618,18 @@ func classifyContainerPublishError(err error) error { ) } + if isACRPermissionError(err) { + return exterrors.Dependency( + exterrors.CodeACRPermissionDenied, + fmt.Sprintf( + "container publish failed because your identity does not have permission to push "+ + "to the Azure Container Registry: %s", + err, + ), + acrPermissionSuggestionFor(err), + ) + } + if actionable := azdext.ActionableErrorDetailFromError(err); actionable != nil && actionable.GetSuggestion() != "" { return err } @@ -625,26 +637,120 @@ func classifyContainerPublishError(err error) error { return exterrors.Internal(exterrors.OpContainerPublish, fmt.Sprintf("container publish failed: %s", err)) } +// acrPermissionSuggestionFor is the user-facing remediation text for +// CodeACRPermissionDenied. It offers a primary RBAC fix and an in-place +// fallback that switches the service to code (zip) deploy without re-running +// `azd ai agent init`. +// +// The recommended role depends on which API was denied: +// - Remote-build path (docker.remoteBuild: true -- the new container deploy +// default): the failing action is typically +// Microsoft.ContainerRegistry/registries/listBuildSourceUploadUrl/action +// or .../scheduleRun/action. AcrPush is data-plane only and does NOT grant +// these; the correct role is "Container Registry Tasks Contributor". +// - Local-push path (docker.remoteBuild: false): the failing action is the +// docker push itself; AcrPush is sufficient. +// +// The emitted `az role assignment create` command uses the role definition +// GUID (not the display name) for the --role argument. GUIDs are guaranteed +// stable; display names could in principle be renamed by Azure. The human +// role name is still shown in the surrounding prose so the user understands +// what they are assigning. +// +// When the underlying error includes the principal's object id and/or the ACR +// resource scope (typical of ARM 403 responses), those values are substituted +// into the command so the user can paste it as-is. Otherwise placeholders +// are shown. ASCII-only per repo style. +func acrPermissionSuggestionFor(err error) string { + assignee := "" + scope := "" + msgRaw := "" + if err != nil { + msgRaw = err.Error() + if m := armObjectIDRe.FindStringSubmatch(msgRaw); len(m) == 2 { + assignee = m[1] + } + if m := armACRScopeRe.FindStringSubmatch(msgRaw); len(m) == 2 { + scope = m[1] + } + } + + isRemoteBuildPath := false + if msgRaw != "" { + lower := strings.ToLower(msgRaw) + isRemoteBuildPath = containsAny(lower, + "listbuildsourceuploadurl", + "schedulerun", + "remote build failed", + ) + } + + // Role identifiers come from developer_rbac_check.go (same package). + // Names are for prose; IDs are what the `az` command actually uses. + primaryRoleName := "AcrPush" + primaryRoleID := roleAcrPush + pathContext := "data-plane push (used when docker.remoteBuild: false)" + abacLine := fmt.Sprintf( + " - Container Registry Repository Writer (role ID: %s) for ABAC-mode registries", + roleAcrRepositoryWriter, + ) + if isRemoteBuildPath { + primaryRoleName = "Container Registry Tasks Contributor" + primaryRoleID = roleContainerRegistryTasksContributor + pathContext = "ACR Tasks remote build (used when docker.remoteBuild: true)" + // For Tasks-based builds on ABAC-mode registries, RepositoryWriter alone + // does not cover Tasks actions. Owner / Contributor remain the broad + // options. + abacLine = " - For ABAC-mode registries, an Owner or Contributor assignment may also be needed" + } + + primaryLine := fmt.Sprintf(" - %s (role ID: %s)", primaryRoleName, primaryRoleID) + azCommand := fmt.Sprintf( + `az role assignment create --assignee %s --role %s --scope %s`, + assignee, primaryRoleID, scope, + ) + + return "Your identity needs permission to push container images to the Azure Container Registry.\n\n" + + "This deployment failed on the " + pathContext + " path.\n\n" + + "Recommended fix (keep container deploy):\n" + + " Ask a subscription Owner or User Access Administrator to assign one of these roles to your\n" + + " identity, then re-run `azd up`:\n" + + primaryLine + "\n" + + abacLine + "\n\n" + + " Example (run as a subscription Owner or User Access Administrator):\n" + + " " + azCommand + "\n\n" + + "Alternative (switch this service to code (zip) deploy; no ACR push required):\n" + + " Code (zip) deploy uploads your source code directly to Foundry Agent Service.\n" + + " The service runs your agent in a Microsoft-managed platform container -- you do\n" + + " NOT need a Dockerfile or a custom container image. No container is built or\n" + + " pushed, so no ACR permissions are needed.\n\n" + + " Supported runtimes: python_3_13, python_3_14, dotnet_10\n\n" + + " Learn more: https://learn.microsoft.com/azure/foundry/agents/how-to/deploy-hosted-agent-code\n\n" + + " To switch (no need to re-run `azd ai agent init`):\n" + + " 1. Open the service's agent.yaml and add a `code_configuration:` block under\n" + + " the hosted agent, for example:\n" + + " code_configuration:\n" + + " runtime: python_3_13 # or dotnet_10\n" + + " entry_point: app.py # or MyAgent.dll\n" + + " 2. Run: azd env set AZD_AGENT_SKIP_ACR true\n" + + " (subsequent provisioning will skip creating ACR; an already-provisioned\n" + + " ACR is not deleted automatically)\n" + + " 3. Re-run: azd up" +} + func isPrivateACRNetworkAccessError(err error) bool { if err == nil { return false } message := strings.ToLower(err.Error()) - acrContext := []string{ - ".azurecr.io", - "azure container registry", - "container registry", - } - hasACRContext := containsAny(message, acrContext...) + hasACRContext := containsAny(message, acrContextSignals...) networkSignals := []string{ "public network access", "private endpoint", "network rule", "firewall", - "not allowed access", - "forbidden", "i/o timeout", "connection timed out", "tls handshake timeout", @@ -653,19 +759,95 @@ func isPrivateACRNetworkAccessError(err error) bool { } hasNetworkSignal := containsAny(message, networkSignals...) + // Specific signal: ACR firewall block list. The "client with ip address ... + // not allowed access" wording is unambiguous so we accept it standalone. if strings.Contains(message, "client with ip address") && strings.Contains(message, "not allowed access") { return true } + // Remote-build wrapper: require BOTH an explicit network signal AND ACR + // context. The previous OR variant false-classified RBAC failures whose + // only "signal" was the word "forbidden" -- those are now handled by + // isACRPermissionError. if strings.Contains(message, "remote build failed") && strings.Contains(message, "local fallback unavailable") { - return hasNetworkSignal || hasACRContext + return hasNetworkSignal && hasACRContext } return hasACRContext && hasNetworkSignal } +// isACRPermissionError reports whether err is an ACR push/build failure caused +// by missing RBAC or auth (as opposed to network access). Predicate is AND: +// the error must reference ACR (by login server, ARM resource type, or +// human-readable name) AND carry an explicit permission signal. +func isACRPermissionError(err error) bool { + if err == nil { + return false + } + message := strings.ToLower(err.Error()) + + if !containsAny(message, acrContextSignals...) { + return false + } + + permissionSignals := []string{ + "denied: requested access to the resource is denied", + "unauthorized", + "authentication required", + "authorization failed", + "authorizationfailed", // ARM ErrorCode (no space) + "does not have authorization", + "does not have rbac permission", + "acrpush", + "insufficient_scope", + "repository access not allowed", + "failed to fetch oauth token", + "acr_token", + "token exchange", + } + if containsAny(message, permissionSignals...) { + return true + } + + // Word-bounded 401/403 plus an explicit permission noun nearby. Avoid + // bare "forbidden" / "token" matches that overlap with other failure modes. + if has40xRe.MatchString(message) && + containsAny(message, "denied", "forbidden", "permission", "not authorized") { + return true + } + + return false +} + +// acrContextSignals are substrings that indicate the error references an +// Azure Container Registry. ".azurecr.io" covers docker-push errors; +// "microsoft.containerregistry" covers ARM-side errors from the remote-build +// path (e.g. listBuildSourceUploadUrl/scheduleRun) which do NOT include the +// login server in the URL. The human-readable variants catch wrapper text. +var acrContextSignals = []string{ + ".azurecr.io", + "microsoft.containerregistry", + "azure container registry", + "container registry", +} + +// has40xRe matches a bare 401 or 403 status code with word boundaries to avoid +// false positives on arbitrary digit runs. +var has40xRe = regexp.MustCompile(`(?i)\b40[13]\b`) + +// armObjectIDRe extracts the principal object id from an ARM AuthorizationFailed +// error message of the form: ... with object id '' does not have authorization ... +var armObjectIDRe = regexp.MustCompile(`(?i)with object id '([0-9a-f-]{36})'`) + +// armACRScopeRe extracts the ACR resource scope from an ARM AuthorizationFailed +// error message of the form: ... over scope '/subscriptions/.../Microsoft.ContainerRegistry/registries/' ... +// Anchored to Microsoft.ContainerRegistry so we don't match unrelated scopes. +var armACRScopeRe = regexp.MustCompile( + `(?i)over scope '(/subscriptions/[^']+/providers/Microsoft\.ContainerRegistry/registries/[^']+)'`, +) + func containsAny(s string, values ...string) bool { for _, value := range values { if strings.Contains(s, value) { diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go index 24818f4c4c3..90ed4a4b46b 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go @@ -1182,9 +1182,9 @@ func TestPublish_PrivateACRNetworkAccessGuidance(t *testing.T) { err error }{ { - name: "remote build failed and local fallback unavailable", + name: "remote build failed and local fallback unavailable with acr context", err: errors.New( - "remote build failed: registry firewall blocked source upload\n\n" + + "remote build failed: myregistry.azurecr.io firewall blocked source upload\n\n" + "Local fallback unavailable: Docker is not installed", ), }, @@ -1196,11 +1196,10 @@ func TestPublish_PrivateACRNetworkAccessGuidance(t *testing.T) { ), }, { - name: "generic host docker login suggestion is overridden", - err: actionableStatusError( - t, - "pushing image to myregistry.azurecr.io failed: Forbidden", - "When pushing to an external registry, run 'docker login' and try again", + name: "private endpoint without public access on ARM call", + err: errors.New( + "POST https://management.azure.com/.../Microsoft.ContainerRegistry/registries/myregistry/" + + "listBuildSourceUploadUrl: private endpoint required; public network access disabled", ), }, } @@ -1224,6 +1223,187 @@ func TestPublish_PrivateACRNetworkAccessGuidance(t *testing.T) { } } +// initTest15RemoteBuildRBACError is the wire-format error captured on +// 2026-05-29 from a repro on the init-test-15 project when running +// `azd deploy` as a service principal with Reader-only access. It exercises +// the new default container deploy path (docker.remoteBuild: true) hitting +// ARM's listBuildSourceUploadUrl with no ACR push role. The error is +// preserved verbatim so future refactors cannot silently re-introduce the +// pre-2026-05 misclassification ("Container Registry may be blocking network +// access") on what is actually an RBAC failure. +const initTest15RemoteBuildRBACError = "rpc error: code = Unknown desc = remote build failed: " + + "POST https://management.azure.com/subscriptions/5f416acb-98a5-411a-808e-f37c0fbbbdb5/" + + "resourceGroups/rg-init-test-15-dev/providers/Microsoft.ContainerRegistry/" + + "registries/crpjhtjmfdtwcau/listBuildSourceUploadUrl\n" + + "--------------------------------------------------------------------------------\n" + + "RESPONSE 403: 403 Forbidden\n" + + "ERROR CODE: AuthorizationFailed\n" + + "--------------------------------------------------------------------------------\n" + + "{\n" + + " \"error\": {\n" + + " \"code\": \"AuthorizationFailed\",\n" + + " \"message\": \"The client 'cdd73b03-a291-42d1-8fd5-903957338f08' with object id " + + "'209256b0-0f0c-41f4-a7e2-bceaba1ca711' does not have authorization to perform action " + + "'Microsoft.ContainerRegistry/registries/listBuildSourceUploadUrl/action' over scope " + + "'/subscriptions/5f416acb-98a5-411a-808e-f37c0fbbbdb5/resourceGroups/rg-init-test-15-dev/" + + "providers/Microsoft.ContainerRegistry/registries/crpjhtjmfdtwcau' or the scope is invalid. " + + "If access was recently granted, please refresh your credentials.\"\n" + + " }\n" + + "}\n" + + "--------------------------------------------------------------------------------\n\n" + + "Local fallback unavailable: the docker service is not running, please start it: exit code: 1" + +func TestPublish_ACRPermissionGuidance(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + err error + expectedPrimaryRole string + expectedRoleID string + expectedPathContext string + }{ + { + name: "init-test-15 remote build RBAC fixture", + err: errors.New(initTest15RemoteBuildRBACError), + expectedPrimaryRole: "Container Registry Tasks Contributor", + expectedRoleID: roleContainerRegistryTasksContributor, + expectedPathContext: "ACR Tasks remote build", + }, + { + name: "docker push denied requested access", + err: errors.New( + "failed to push image to myregistry.azurecr.io/app:v1: " + + "denied: requested access to the resource is denied", + ), + expectedPrimaryRole: "AcrPush", + expectedRoleID: roleAcrPush, + expectedPathContext: "data-plane push", + }, + { + name: "docker push 401 unauthorized authentication required", + err: errors.New( + "pushing to myregistry.azurecr.io: 401 Unauthorized: authentication required", + ), + expectedPrimaryRole: "AcrPush", + expectedRoleID: roleAcrPush, + expectedPathContext: "data-plane push", + }, + { + name: "acr token exchange failure", + err: errors.New( + "failed to fetch oauth token for myregistry.azurecr.io: insufficient_scope", + ), + expectedPrimaryRole: "AcrPush", + expectedRoleID: roleAcrPush, + expectedPathContext: "data-plane push", + }, + { + name: "actionable docker login suggestion is overridden", + err: actionableStatusError( + t, + "pushing image to myregistry.azurecr.io failed: 403 Forbidden", + "When pushing to an external registry, run 'docker login' and try again", + ), + expectedPrimaryRole: "AcrPush", + expectedRoleID: roleAcrPush, + expectedPathContext: "data-plane push", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := publishWithContainerError(t, tt.err) + + localErr, ok := errors.AsType[*azdext.LocalError](err) + require.True(t, ok, "expected LocalError, got %T: %v", err, err) + require.Equal(t, exterrors.CodeACRPermissionDenied, localErr.Code, + "should classify as permission denied, not %q", localErr.Code) + require.Equal(t, azdext.LocalErrorCategoryDependency, localErr.Category) + require.Contains(t, localErr.Message, + "does not have permission to push") + require.Contains(t, localErr.Suggestion, tt.expectedPathContext, + "suggestion should identify the failing path") + require.Contains(t, localErr.Suggestion, tt.expectedPrimaryRole, + "suggestion prose should name the role") + require.Contains(t, localErr.Suggestion, tt.expectedRoleID, + "suggestion should include the role ID alongside the name") + require.Contains(t, localErr.Suggestion, + fmt.Sprintf(`--role %s`, tt.expectedRoleID), + "az command should use the role GUID for stability") + require.NotContains(t, localErr.Suggestion, + fmt.Sprintf(`--role "%s"`, tt.expectedPrimaryRole), + "az command should not use the display name -- use the GUID instead") + require.Contains(t, localErr.Suggestion, "AZD_AGENT_SKIP_ACR") + require.Contains(t, localErr.Suggestion, "code_configuration") + require.Contains(t, localErr.Suggestion, "azd up") + require.NotContains(t, localErr.Suggestion, "docker login") + require.NotContains(t, localErr.Suggestion, "allowlist the public outbound IP/CIDR") + }) + } +} + +// TestPublish_ACRPermissionGuidance_DynamicSubstitution verifies that when the +// underlying ARM error includes the principal object id and ACR resource scope +// (typical of remoteBuild=true RBAC failures), those values are substituted +// into the example `az role assignment create` command so the user can copy +// and paste it directly. Also confirms the remote-build path triggers the +// "Container Registry Tasks Contributor" recommendation (via GUID), not +// "AcrPush" (AcrPush is data-plane only and does NOT grant +// listBuildSourceUploadUrl). +func TestPublish_ACRPermissionGuidance_DynamicSubstitution(t *testing.T) { + t.Parallel() + + err := publishWithContainerError(t, errors.New(initTest15RemoteBuildRBACError)) + + localErr, ok := errors.AsType[*azdext.LocalError](err) + require.True(t, ok, "expected LocalError, got %T: %v", err, err) + require.Equal(t, exterrors.CodeACRPermissionDenied, localErr.Code) + + // Both identifiers from the captured fixture must appear inline in the + // command -- not as placeholders -- so the user can paste it as-is. + // The role is identified by its definition GUID, not its display name, + // so the command remains valid even if Azure ever renames the role. + expectedCmd := fmt.Sprintf( + `az role assignment create `+ + `--assignee 209256b0-0f0c-41f4-a7e2-bceaba1ca711 `+ + `--role %s `+ + `--scope /subscriptions/5f416acb-98a5-411a-808e-f37c0fbbbdb5/`+ + `resourceGroups/rg-init-test-15-dev/providers/Microsoft.ContainerRegistry/`+ + `registries/crpjhtjmfdtwcau`, + roleContainerRegistryTasksContributor, + ) + require.Contains(t, localErr.Suggestion, expectedCmd) + require.NotContains(t, localErr.Suggestion, "") + require.NotContains(t, localErr.Suggestion, "") +} + +// TestPublish_ACRPermissionGuidance_PlaceholderFallback verifies that when the +// error shape lacks an object id and/or ACR scope (e.g. docker-push errors +// from the local-build path), the suggestion gracefully falls back to +// placeholder tokens rather than emitting a broken command. The local-push +// path also gets the AcrPush GUID recommendation, not Tasks Contributor. +func TestPublish_ACRPermissionGuidance_PlaceholderFallback(t *testing.T) { + t.Parallel() + + err := publishWithContainerError(t, errors.New( + "failed to push image to myregistry.azurecr.io/app:v1: "+ + "denied: requested access to the resource is denied", + )) + + localErr, ok := errors.AsType[*azdext.LocalError](err) + require.True(t, ok, "expected LocalError, got %T: %v", err, err) + require.Equal(t, exterrors.CodeACRPermissionDenied, localErr.Code) + require.Contains(t, localErr.Suggestion, "") + require.Contains(t, localErr.Suggestion, "") + require.Contains(t, localErr.Suggestion, fmt.Sprintf( + `az role assignment create --assignee --role %s --scope `, + roleAcrPush, + )) +} + func TestPublish_GenericPublishErrorsAreNotClassifiedAsPrivateACR(t *testing.T) { t.Parallel() @@ -1240,7 +1420,11 @@ func TestPublish_GenericPublishErrorsAreNotClassifiedAsPrivateACR(t *testing.T) err: errors.New("denied: requested access to the resource is denied"), }, { - name: "remote build dockerfile failure without acr network signal", + name: "non-acr 403 forbidden", + err: errors.New("403 forbidden from foundry control plane"), + }, + { + name: "remote build dockerfile failure without acr context", err: errors.New( "remote build failed: Dockerfile parse error\n\n" + "Local fallback unavailable: Docker is not installed", @@ -1256,7 +1440,8 @@ func TestPublish_GenericPublishErrorsAreNotClassifiedAsPrivateACR(t *testing.T) localErr, ok := errors.AsType[*azdext.LocalError](err) require.True(t, ok, "expected LocalError, got %T: %v", err, err) - require.Equal(t, exterrors.OpContainerPublish, localErr.Code) + require.Equal(t, exterrors.OpContainerPublish, localErr.Code, + "should fall through to internal, not %q", localErr.Code) require.Equal(t, azdext.LocalErrorCategoryInternal, localErr.Category) require.Contains(t, localErr.Message, "container publish failed") })