Azure / Azure/azure-dev

Apply ValidateEnvReferences to the init and service env discovery paths

Open
#9,428 0 comments 0 reactions 1 assignee Claimed by @glharper View on GitHub
area/extensions area/validation bug ext-agents ext-connections ext-foundry ext-projects ext-routines ext-toolboxes
Dominant language
Go
Stars
569
Forks
364
Avg merge
2d 19h
Merged PRs (30d)
136

Description

Follow-up from review on #9367.

## Context

`ValidateEnvReferences` (added in #9367, `internal/synthesis/envrefs.go`) refuses any `$` form outside `${VAR}`, `${VAR:-default}`, `$${VAR}` and `${{...}}`. It exists because drone/envsubst — which backs `foundry.ExpandEnv` — implements the full shell parameter grammar, so `${BAR:=x}`, `${BAR:+alt}`, `${BAR:?msg}`, `${BAR#p}` and `${BAR:0:3}` all expand even though `FindEnvReferences` reports none of them. It also refuses a reference nested in a `:-` default, which the expander resolves but the scanner deliberately does not report.

Where it runs, it makes the scan complete: every occurrence the expander acts on is one the scanner saw.

## Problem

It only runs on the three project network fields: `network.agentSubnet.vnet`, `network.peSubnet.vnet`, `network.dns.subscription`.

Every other Foundry field is scanned without it. `FindEnvReferences` is also the discovery source for:

- init prompting (`azure.ai.agents/internal/cmd/init_env.go`)
- the generated service env block (`azure.ai.agents/internal/cmd/resource_services.go`)

which cover agent `environmentVariables[*].value`, connection `target` / `credentials` / `metadata`, routine `action.input`, and toolbox `endpoint` / `tools`.

So `${BAR:=x}` in an agent `env:` value returns no references at all. init never prompts for `BAR`, the value gets no entry in the service env block, and `ExpandEnv` quietly rewrites it to `x` at deploy. Same one-character slip as the network case, different field, no diagnostic.

## Why it was not just switched on

Those paths are discovery, not validation:

- `collectAzureYamlEnvironmentReferences` and `collectStringEnvironmentTemplates` are collectors with no error return, so surfacing a refusal means threading an error out to the init/deploy callers and choosing where it is reported.
- The blast radius is every Foundry field in every extension, not three fields in one. An azure.yaml using envsubst grammar today *works* — it is just undiscovered — so refusing it turns a silent gap into a hard failure on manifests that currently deploy. That needs to be a deliberate, announced change, not a side effect of a `resolveVars` fix.

## Proposed work

1. Decide the policy: refuse, or warn and continue, for unsupported forms found during discovery.
2. Thread the result out of `collectAzureYamlEnvironmentReferences` / `collectStringEnvironmentTemplates` to the init and deploy call sites.
3. Report it with the service and field path, matching the network-field message.
4. Decide whether nested `:-` references are refused everywhere or stay tolerated outside the network fields, where they currently resolve at deploy for consumers that keep an azd environment fallback.

Related: #9427 (move the scanner into `pkg/foundry`).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.