Skip to content

Validate environment variable names for Foundry hosted agents#16884

Merged
eerhardt merged 2 commits into
mainfrom
eerhardt/foundry-hosted-agents-validate-env-vars
May 11, 2026
Merged

Validate environment variable names for Foundry hosted agents#16884
eerhardt merged 2 commits into
mainfrom
eerhardt/foundry-hosted-agents-validate-env-vars

Conversation

@eerhardt
Copy link
Copy Markdown
Member

@eerhardt eerhardt commented May 8, 2026

Description

Foundry Hosted Agents only accept environment variable names containing ASCII letters, digits, and underscores. Other characters (dashes, dots, etc.) are silently rejected at deploy time, producing a confusing failure. This change validates names up-front.

Changes

  • HostedAgentConfiguration.ToProjectsAgentVersionCreationOptions now takes the target resource name and validates each environment variable name against a source-generated regex (^[A-Za-z0-9_]+$).
  • Invalid names cause a DistributedApplicationException listing the offending names and the target resource.
  • Method visibility tightened to internal (only the resource-side deploy path needs it).
  • Renamed the target resources in PublishAsHostedAgent_ResolvesExternalContainerAppReference to non-dashed names so the existing happy-path test continues to pass.
  • Added a unit test covering the new validation.

Fixes

Fixes #16858

Checklist

  • Tests added/updated and passing locally (Aspire.Hosting.Foundry.Tests 84/84, FoundryExtensionsTests 19/19)
  • Build clean

Copilot AI review requested due to automatic review settings May 8, 2026 18:19
@eerhardt eerhardt requested a review from sebastienros as a code owner May 8, 2026 18:19
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16884

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16884"

Foundry Hosted Agents only support environment variable names containing ASCII letters, digits, and underscores. Service discovery names (with dashes/dots) and other generated names would silently produce invalid agent versions at deploy time.

Add validation in HostedAgentConfiguration.ToProjectsAgentVersionCreationOptions using a source-generated regex; throw DistributedApplicationException with the offending names if any are invalid. Rename target resources in the existing publish test to non-dashed names so the test exercises the happy path.

Fixes #16858

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@eerhardt eerhardt force-pushed the eerhardt/foundry-hosted-agents-validate-env-vars branch from 30df0c9 to 79ae822 Compare May 8, 2026 18:20
@eerhardt eerhardt requested a review from tommasodotNET May 8, 2026 18:20
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds proactive validation for environment variable names used by Foundry Hosted Agents so deploy-time silent rejection becomes an actionable, early failure with clear diagnostics.

Changes:

  • Validate hosted-agent environment variable names against ^[A-Za-z0-9_]+$ and throw a DistributedApplicationException that includes the target resource name and offending variables.
  • Thread the target resource name into the conversion path used during deployment.
  • Update and add tests to cover the new validation and keep existing publish-path tests passing.

Reviewed changes

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

File Description
tests/Aspire.Hosting.Foundry.Tests/HostedAgentConfigurationTests.cs Adds coverage ensuring invalid env-var names cause a clear exception and valid names aren’t reported.
tests/Aspire.Hosting.Azure.Tests/FoundryExtensionsTests.cs Renames test resource names to avoid dashes so generated env-var keys remain valid under the new restriction.
src/Aspire.Hosting.Foundry/HostedAgent/HostedAgentConfiguration.cs Implements source-generated-regex validation and improves error reporting by including target resource context.
src/Aspire.Hosting.Foundry/HostedAgent/AzureHostedAgentResource.cs Passes the target resource name into the options-conversion method during deploy.

Comment thread src/Aspire.Hosting.Foundry/HostedAgent/HostedAgentConfiguration.cs
Copy link
Copy Markdown
Member

@joperezr joperezr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Left a few thoughts inline.

Comment thread src/Aspire.Hosting.Foundry/HostedAgent/HostedAgentConfiguration.cs
Comment thread src/Aspire.Hosting.Foundry/HostedAgent/HostedAgentConfiguration.cs
Comment thread src/Aspire.Hosting.Foundry/HostedAgent/HostedAgentConfiguration.cs
Comment on lines +96 to +100
Assert.Contains("Foundry hosted agent for target resource 'target'", ex.Message);
Assert.Contains("Environment variable names must contain only ASCII letters, digits, or underscores.", ex.Message);
Assert.Contains("'INVALID-NAME'", ex.Message);
Assert.Contains("'invalid.name'", ex.Message);
Assert.DoesNotContain("VALID_NAME_1", ex.Message);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Use Assert.Equal + raw string literal to exactly test the results.

@eerhardt eerhardt enabled auto-merge (squash) May 11, 2026 19:59
@github-actions
Copy link
Copy Markdown
Contributor

Re-running the failed jobs in the CI workflow for this pull request because 2 jobs were identified as retry-safe transient failures in the CI run attempt.
GitHub was asked to rerun all failed jobs for that attempt, and the rerun is being tracked in the rerun attempt.
The job links below point to the failed attempt jobs that matched the retry-safe transient failure rules.

@eerhardt eerhardt merged commit 124cad0 into main May 11, 2026
572 of 577 checks passed
@github-actions github-actions Bot added this to the 13.4 milestone May 11, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🎬 CLI E2E Test Recordings — 78 recordings uploaded (commit c8cca89)

View all recordings
Status Test Recording
AddPackageInteractiveWhileAppHostRunningDetached ▶️ View Recording
AddPackageWhileAppHostRunningDetached ▶️ View Recording
AgentCommands_AllHelpOutputs_AreCorrect ▶️ View Recording
AgentInitCommand_DefaultSelection_InstallsSkillOnly ▶️ View Recording
AgentInitCommand_MigratesDeprecatedConfig ▶️ View Recording
AspireAddPackageVersionToDirectoryPackagesProps ▶️ View Recording
AspireInitSingleFileAppHostRunsViaDotnetRunAppHost ▶️ View Recording
AspireUpdateRemovesAppHostPackageVersionFromDirectoryPackagesProps ▶️ View Recording
Banner_DisplayedOnFirstRun ▶️ View Recording
Banner_DisplayedWithExplicitFlag ▶️ View Recording
Banner_NotDisplayedWithNoLogoFlag ▶️ View Recording
CertificatesClean_RemovesCertificates ▶️ View Recording
CertificatesTrust_WithNoCert_CreatesAndTrustsCertificate ▶️ View Recording
CertificatesTrust_WithUntrustedCert_TrustsCertificate ▶️ View Recording
ConfigSetGet_CreatesNestedJsonFormat ▶️ View Recording
CreateAndRunAspireStarterProject ▶️ View Recording
CreateAndRunAspireStarterProjectWithBundle ▶️ View Recording
CreateAndRunEmptyAppHostProject ▶️ View Recording
CreateAndRunJavaEmptyAppHostProject ▶️ View Recording
CreateAndRunJsReactProject ▶️ View Recording
CreateAndRunPythonReactProject ▶️ View Recording
CreateAndRunTypeScriptEmptyAppHostProject ▶️ View Recording
CreateAndRunTypeScriptStarterProject ▶️ View Recording
CreateJavaAppHostWithViteApp ▶️ View Recording
CreateTypeScriptAppHostWithViteApp_UsesConfiguredToolchain ▶️ View Recording
DashboardRunWithOtelTracesReturnsNoTraces ▶️ View Recording
DeployK8sBasicApiService ▶️ View Recording
DeployK8sWithExternalHelmChart ▶️ View Recording
DeployK8sWithGarnet ▶️ View Recording
DeployK8sWithMongoDB ▶️ View Recording
DeployK8sWithMySql ▶️ View Recording
DeployK8sWithPostgres ▶️ View Recording
DeployK8sWithRabbitMQ ▶️ View Recording
DeployK8sWithRedis ▶️ View Recording
DeployK8sWithSqlServer ▶️ View Recording
DeployK8sWithValkey ▶️ View Recording
DeployTypeScriptAppToKubernetes ▶️ View Recording
DescribeCommandResolvesReplicaNames ▶️ View Recording
DescribeCommandShowsRunningResources ▶️ View Recording
DetachFormatJsonProducesValidJson ▶️ View Recording
DetachFormatJsonProducesValidJsonWhenRestartingExistingInstance ▶️ View Recording
DoListStepsShowsPipelineSteps ▶️ View Recording
DocsCommand_RendersInteractiveMarkdownFromLocalSource ▶️ View Recording
DoctorCommand_DetectsDeprecatedAgentConfig ▶️ View Recording
DoctorCommand_TypeScriptAppHostReportsMissingConfiguredToolchain ▶️ View Recording
DoctorCommand_WithSslCertDir_ShowsTrusted ▶️ View Recording
DoctorCommand_WithoutSslCertDir_ShowsPartiallyTrusted ▶️ View Recording
GlobalMigration_HandlesCommentsAndTrailingCommas ▶️ View Recording
GlobalMigration_HandlesMalformedLegacyJson ▶️ View Recording
GlobalMigration_PreservesAllValueTypes ▶️ View Recording
GlobalMigration_SkipsWhenNewConfigExists ▶️ View Recording
GlobalSettings_MigratedFromLegacyFormat ▶️ View Recording
InitTypeScriptAppHost_AugmentsExistingViteRepoAtRoot ▶️ View Recording
InteractiveCSharpInitCreatesExpectedFiles ▶️ View Recording
InvalidAppHostPathWithComments_IsHealedOnRun ▶️ View Recording
LatestCliCanStartStableChannelAppHost ▶️ View Recording
LatestCliCanStartStableChannelTypeScriptAppHost ▶️ View Recording
LegacySettingsMigration_AdjustsRelativeAppHostPath ▶️ View Recording
LogsCommandShowsResourceLogs ▶️ View Recording
OtelLogsReturnsStructuredLogsFromStarterAppCore ▶️ View Recording
PsCommandListsRunningAppHost ▶️ View Recording
PsFormatJsonOutputsOnlyJsonToStdout ▶️ View Recording
PublishWithConfigureEnvFileUpdatesEnvOutput ▶️ View Recording
PublishWithDockerComposeServiceCallbackSucceeds ▶️ View Recording
PublishWithoutOutputPathUsesAppHostDirectoryDefault ▶️ View Recording
RestoreGeneratesSdkFiles ▶️ View Recording
RestoreGeneratesSdkFiles_WithConfiguredToolchain ▶️ View Recording
RestoreRefreshesGeneratedSdkAfterAddingIntegration ▶️ View Recording
RestoreSupportsConfigOnlyHelperPackageAndCrossPackageTypes ▶️ View Recording
RunFromParentDirectory_UsesExistingConfigNearAppHost ▶️ View Recording
SecretCrudOnDotNetAppHost ▶️ View Recording
SecretCrudOnTypeScriptAppHost ▶️ View Recording
StagingChannel_ConfigureAndVerifySettings_ThenSwitchChannels ▶️ View Recording
StartAndWaitForTypeScriptSqlServerAppHostWithNativeAssets ▶️ View Recording
StopAllAppHostsFromAppHostDirectory ▶️ View Recording
StopNonInteractiveSingleAppHost ▶️ View Recording
StopWithNoRunningAppHostExitsSuccessfully ▶️ View Recording
UnAwaitedChainsCompileWithAutoResolvePromises ▶️ View Recording

📹 Recordings uploaded automatically from CI run #25693901621

@aspire-repo-bot
Copy link
Copy Markdown
Contributor

✅ No documentation update needed.

Bug fix only — this PR adds up-front validation that environment variable names for Foundry hosted agents contain only ASCII letters, digits, and underscores, surfacing an existing Foundry service constraint as a clear DistributedApplicationException instead of a silent deploy-time failure. No new user-facing features, public APIs, or configuration options were introduced; the only public-API change was narrowing ToProjectsAgentVersionCreationOptions from public to internal.

nellshamrell pushed a commit to nellshamrell/aspire that referenced this pull request May 18, 2026
…oft#16884)

* Validate environment variable names for Foundry hosted agents

Foundry Hosted Agents only support environment variable names containing ASCII letters, digits, and underscores. Service discovery names (with dashes/dots) and other generated names would silently produce invalid agent versions at deploy time.

Add validation in HostedAgentConfiguration.ToProjectsAgentVersionCreationOptions using a source-generated regex; throw DistributedApplicationException with the offending names if any are invalid. Rename target resources in the existing publish test to non-dashed names so the test exercises the happy path.

Fixes microsoft#16858

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

* PR feedback

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Foundry Hosted Agents needs to validate environment variables

5 participants