Skip to content

Support environment variable placeholders in config and data files - #2031

Open
waldekmastykarz wants to merge 7 commits into
dotnet:mainfrom
waldekmastykarz:env-var-placeholders
Open

waldekmastykarz wants to merge 7 commits into
dotnet:mainfrom
waldekmastykarz:env-var-placeholders

Conversation

@waldekmastykarz

Copy link
Copy Markdown
Collaborator

Closes #1667

Adds environment variable placeholders to Dev Proxy config files and plugin data files.

  • ${VAR} (and ${env:VAR} alias) resolved at file load, per string value (property names untouched)
  • Config files resolve against --env, the config env section, then OS environment
  • Unset variables: placeholder left as-is with a warning
  • $${VAR} escapes a literal ${VAR}
  • Legacy @VAR (API Center) and {VAR} (minimal permissions API specs) still work but log deprecation warnings

Notes for review:

  • Rewrites files opt out of load-time resolution because ${name} is a regex capture-group reference in rewrite targets; they keep the existing per-request ${env:NAME} handling
  • Not covered: .http mocks, @file bodies, prompt files; config validate and plugin-config schema validation still see raw text

Tests added for the resolver, config providers and BaseLoader. Docs (skills references) and v4 rc schema env description updated.

…loses dotnet#1667

Resolve ${VAR} (and ${env:VAR}) placeholders when loading Dev Proxy config
files and plugin data files. Unset variables are left as-is with a warning;
$${VAR} escapes a literal placeholder. Legacy @var (API Center) and {VAR}
(minimal permissions API specs) keep working but log deprecation warnings.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 31, 2026 09:08
@waldekmastykarz
waldekmastykarz requested a review from a team as a code owner October 31, 2026 09:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved findings show placeholder resolution can corrupt YAML and mishandle escaped or Unicode-encoded tokens, while case-variant CLI overrides can cause Windows startup failures.

4 open issues
What changed in this PR

Adds environment-variable placeholders to Dev Proxy configuration and plugin data files, addressing #1667 by reducing the need for scripts that rewrite environment-specific values.

Changes:

  • Adds shared placeholder resolution, escaping, precedence rules, and unresolved-variable warnings.
  • Integrates resolution into configuration providers and plugin loaders, preserves RewritePlugin handling, and warns on legacy syntax.
  • Adds resolver and loader tests, updates examples, and documents supported behavior.
File Description
skills/​dev-proxy/​references/​configuration.md Documents syntax, precedence, escaping, and exceptions.
skills/​dev-proxy/​references/​ci-cd-integration.md Updates API Center placeholder examples.
skills/​dev-proxy/​references/​analyze-api-usage.md Updates API usage configuration examples.
schemas/​v4.0.0/​rc.schema.json Describes placeholder support in env.
DevProxy/​Program.cs Logs unresolved configuration variables.
DevProxy/​Extensions/​ConfigurationManagerExtensions.cs Passes CLI environment overrides to providers.
DevProxy.Tests/​BaseLoaderPlaceholderTests.cs Tests loader resolution and opt-out behavior.
DevProxy.Plugins/​Utils/​MinimalPermissionsUtils.cs Adds shared API-spec variable resolution.
DevProxy.Plugins/​Reporting/​MinimalPermissionsPlugin.cs Uses the shared API-spec resolver.
DevProxy.Plugins/​Reporting/​MinimalPermissionsGuidancePlugin.cs Uses the shared API-spec resolver.
DevProxy.Plugins/​Manipulation/​RewritesLoader.cs Excludes rewrite files from load-time resolution.
DevProxy.Plugins/​ApiCenter/​ApiCenterClient.cs Warns about deprecated @VAR syntax.
DevProxy.Abstractions/​Utils/​EnvironmentVariablePlaceholders.cs Implements string and JSON placeholder resolution.
DevProxy.Abstractions/​Plugins/​BaseLoader.cs Resolves placeholders before validation and loading.
DevProxy.Abstractions/​Extensions/​YamlConfigurationExtensions.cs Integrates resolution into configuration loading.
DevProxy.Abstractions/​Extensions/​EnvironmentVariableConfigurationExtensions.cs Adds JSON provider and precedence handling.
DevProxy.Abstractions.Tests/​Utils/​EnvironmentVariablePlaceholdersTests.cs Tests substitution, escaping, and JSON preservation.
DevProxy.Abstractions.Tests/​Extensions/​ConfigurationPlaceholderTests.cs Tests JSON/YAML providers and override precedence.

Review effort: Balanced

- Don't throw on --env names differing only in casing on Windows
- Resolve JSON values containing Unicode-escaped placeholders
- Resolve YAML API specs per scalar value instead of raw text
- Apply legacy {NAME} variables only outside ${NAME} placeholders

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Load-time expansion incorrectly replaces inline rewrite capture-group placeholders with environment values, while legacy unquoted YAML URL placeholders remain objects instead of URL strings.

2 open issues
4 resolved since last review

Review effort: Balanced

- Exclude RewritePlugin inline rewrite rules from load-time placeholder
  resolution, matching rewrites files
- Replace legacy {NAME} variables in YAML API specs in raw text (outside
  ${NAME} placeholders) so unquoted values keep working

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Placeholder handling can overwrite rewrite capture-group references when plugin metadata contains placeholders and corrupt Unicode-escaped YAML literals while emitting spurious deprecation warnings.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

Medium severity Resolve plugin metadata before computing rewrite exclusions

DevProxy.Abstractions/​Extensions/​EnvironmentVariableConfigurationExtensions.cs:134

Rewrite exclusions are computed before plugin metadata is expanded: with SECTION=myRewrites and USER=alice, "configSection": "${SECTION}" causes an inline /users/${USER} target to become /users/alice, losing its capture-group reference. A placeholder in the plugin name also bypasses the exclusion. Compute the prefixes from resolved metadata, while preserving single-pass expansion of the stored values.

2 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Rewrite exclusions are computed before plugin metadata expansion, allowing environment-variable substitution to alter inline rewrite targets and capture-group references when plugin names or section references contain placeholders.

1 open issue
1 resolved since last review

Review effort: Balanced

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Human assessment is needed because YAML alias mutation can alter mapping keys, while placeholder matching can silently skip valid Windows environment-variable names.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

${env:NAME} now matches any characters except } so names like
ProgramFiles(x86) resolve. Bare ${NAME} stays identifier-only. Pin
YAML alias-as-key behavior with a test.

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

Copy link
Copy Markdown
Collaborator Author

Addressing the latest Copilot review summary (no inline threads) in 596a102:

  • Windows variable names skipped: fixed. ${env:NAME} now accepts any characters except }, so ${env:ProgramFiles(x86)}, ${env:FOO-BAR}, and ${env:FOO.BAR} resolve, and unset ones log a warning. Bare ${NAME} stays identifier-only on purpose so that unrelated ${...} text in data files isn't treated as a variable. Added tests covering $$ escaping and legacy {NAME} handling alongside the new names.
  • YAML alias used as a mapping key gets mutated: pushing back. An alias reuses the anchored node, so a key that aliases a resolved value changes with it. This is very unlikely in Dev Proxy configs and data files. Added a test that pins this behavior so it's intentional.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Human assessment is needed because legacy JSON-key substitution changes can report permissions for unrelated paths, while startup failures can suppress missing-variable diagnostics.

No open issues 🎉

Review effort: Balanced

waldekmastykarz and others added 2 commits October 31, 2026 14:23
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…startup failure

- Replace legacy {NAME} variables in raw text for JSON API specs too, so
  keys like OpenAPI paths keep being substituted as before
- Write unresolved placeholder warnings to stderr when startup fails
  before the logger is available

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

Copy link
Copy Markdown
Collaborator Author

Addressing the latest Copilot review summary (no inline threads):

  • Legacy JSON key substitution: fixed in e890062. Before this PR, legacy {NAME} variables were replaced in the raw text of JSON specs, including keys such as OpenAPI paths. The JSON pass only looked at values, which broke that. Both JSON and YAML specs now apply the legacy replacement to the raw text again, skipping only ${...} placeholders, and then resolve new-style placeholders in values. Added a test.
  • Startup failures hide missing-variable warnings: fixed in e890062. If building the app throws, for example on an invalid apiPort: "${API_PORT}", the unresolved-variable warnings are now written to stderr before the exception propagates. In JSON output mode they're written as JSONL. Verified manually.
  • Also in c6e03d6: restored the original CRLF line endings in files that had been accidentally converted to LF, so the PR diff only shows real changes.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues are recorded for the environment-variable placeholder changes.

No open issues 🎉

Review effort: Balanced

This branch has not been deployed

No deployments
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.

Support environment variable placeholders in configuration files

2 participants