fix(config): reject unsupported env placeholders

[goutamadwant]

Closes #323

Summary

• reject unsupported ${env:VAR} placeholders before HTTP requests or stdio process launches, including inherited environment keys and direct SDK runtime definitions
• show the accepted ${VAR}, ${VAR:-fallback}, and whole-value $env:VAR forms in CLI help, the packaged README, import guidance, and generated schema
• preserve the existing placeholder grammar and literal non-placeholder text

Behavior proof

On current main, resolving Bearer ${env:MCPORTER_TEST_TOKEN} with MCPORTER_TEST_TOKEN set returned the same literal string, allowing it to reach an authorization header unchanged. A whole-value ${env:MCPORTER_TEST_TOKEN} also entered the legacy default parser instead of reporting unsupported syntax.

After the fix, two isolated configurations were exercised through the built CLI. Paths are redacted below. The HTTP configuration failed while resolving its header, before any request:

$ node dist/cli.js list invalid-header --config <redacted-config>
invalid-header
  tools unavailable ... HTTP http://127.0.0.1:9/mcp

  Tools: <unavailable>
  Reason: Failed to resolve header 'Authorization' for server 'invalid-header': Unsupported environment placeholder '${env:MCPORTER_PROOF}'. Use '${VAR}', '${VAR:-fallback}', or whole-value '$env:VAR'.

The stdio configuration used inherited PATH and a child command that would create a sentinel if launched. It failed on the configured value before inherited-key precedence, and the sentinel remained absent:

$ env PATH="$PATH" node dist/cli.js list invalid-stdio --config <redacted-config>
invalid-stdio
  tools unavailable ... STDIO node -e <sentinel writer>

  Tools: <unavailable>
  Reason: Unsupported environment placeholder '${env:PATH}'. Use '${VAR}', '${VAR:-fallback}', or whole-value '$env:VAR'.
$ test ! -e "$sentinel"
$ echo $?
0

A direct createRuntime({ servers }) regression also supplies ${env:MCPORTER_COMMAND} as the stdio executable and verifies that runtime.connect() returns the same corrective error before transport creation.

Validation

pnpm exec vitest run tests/runtime-utils.test.ts tests/env-and-daemon-utils.test.ts tests/runtime.test.ts tests/config-help.test.ts
pnpm docs:list
pnpm check
pnpm test
pnpm docs:site
git diff --check

Focused regression coverage passed with 4 files and 29 tests. The full suite passed with 194 files and 1,639 tests; 4 files and 26 tests were skipped by the existing suite configuration. Documentation listing, formatting, lint, type checking, the build, the documentation site build, and the diff check also passed on Node.js 24.19.0 with pnpm 10.34.5.

Limitations

This rejects ${env:VAR} with corrective guidance rather than translating it. Supported placeholder behavior is unchanged, and no live provider credentials are required for this parser and help-surface fix.

Disclosure: AI was used to understand the codebase and review the fix.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 24, 2026, 2:33 PM ET / 18:33 UTC.

ClawSweeper review

What this changes

The PR rejects unsupported ${env:VAR} placeholders before HTTP or stdio use, documents accepted syntax, and adds regressions for configuration and direct-runtime paths.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

Keep open: the PR remains a focused, evidence-backed fix for the linked configuration bug, and review found no actionable correctness defect in the current head.

Priority: P2
Reviewed head: 7cc7b43b53cf132069c0cad9271364fd1ee2fbe3

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused fix with credible runtime proof, proportional regression coverage, and no identified patch defect.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The PR body provides redacted after-fix terminal traces showing rejection before HTTP dispatch and before stdio child-process launch.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR body provides redacted after-fix terminal traces showing rejection before HTTP dispatch and before stdio child-process launch.
Evidence reviewed	5 items	Shared resolver rejects unsupported syntax: The added rejection runs in both generic placeholder resolution and env override resolution, preserving the existing accepted-placeholder parser. Direct stdio runtime path is covered: Command preparation detects the unsupported braced form before passing it to the shared resolver; the direct-runtime regression asserts connection fails before launch. Current main does not already contain the patch: The tree diff from current main to the PR head contains the resolver, runtime, documentation, schema, and regression changes, so the central behavior is not implemented on main.
Findings	None	None.
Security	None	None.

Live Verification

Command: pnpm run mcporter -- config add --help

Result: FAIL (partial) — step 2 expect_output whole-value $env:VAR: expected terminal output was not visible within 30 seconds: "whole-value $env:VAR"

pnpm run mcporter -- config add --help
runner@runnervm76f27:/tmp/clawsweeper-live-proof-324-DwlQja/target$ pnpm run mcporter -- config add --help

› mcporter@0.13.7 mcporter /tmp/clawsweeper-live-proof-324-DwlQja/target
› tsx src/cli.ts -- config add --help

[mcporter] Did you mean iterm?
Unknown MCP server '--'.
 ELIFECYCLE  Command failed with exit code 1.
pnpm run mcporter -- config add --help
runner@runnervm76f27:/tmp/clawsweeper-live-proof-324-DwlQja/target$ pnpm run mcporter -- config add --help

› mcporter@0.13.7 mcporter /tmp/clawsweeper-live-proof-324-DwlQja/target
› tsx src/cli.ts -- config add --help

[mcporter] Did you mean iterm?
Unknown MCP server '--'.
 ELIFECYCLE  Command failed with exit code 1.
runner@runnervm76f27:/tmp/clawsweeper-live-proof-324-DwlQja/target$

Assertions:

• FAIL expect_output: whole-value $env:VAR

How this fits together

MCPorter reads server definitions from configuration, imports, and SDK callers, resolves environment values, then creates HTTP or stdio transports. Invalid placeholder handling must stop before headers are sent or child processes start.

flowchart LR
  A[Configuration and SDK definitions] --> B[Environment value resolver]
  B --> C[Placeholder validation]
  C --> D[HTTP header construction]
  C --> E[Stdio command preparation]
  D --> F[Remote MCP server]
  E --> G[Child MCP process]

Loading

Before merge

• Resolve merge risk (P1) - Existing configurations that use ${env:VAR} will change from sending a literal value to receiving a corrective error; users must replace it with one of the documented forms.
• Complete next step (P2) - No repair lane is needed: the patch is coherent and the remaining action is ordinary maintainer merge review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	production +40/-10; tests +35/-1	The compact parser and help-surface change is accompanied by regression coverage for resolver, inherited-env, and direct-runtime paths.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #323
Summary: This PR is the explicit candidate fix for the still-open report about unsupported environment placeholder syntax.

Members:

• canonical: Env placeholder syntax for --header/--env is undiscoverable from CLI or npm package; ${env:VAR} silently passes through verbatim #323 - The PR body explicitly closes it and the reported header/env pass-through is covered by this patch.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

1. Accept the intentional compatibility change (recommended)
   Land the explicit rejection so affected users receive a corrective error rather than a literal credential-like value reaching a transport.

Technical review

Best possible solution:

Merge the narrow fail-fast validation and documentation update so invalid imported or authored placeholders cannot reach authentication headers or child-process launch.

Do we have a high-confidence way to reproduce the issue?

Yes. The resolver and its HTTP and stdio callers make the failing path clear, and the PR supplies redacted after-fix CLI traces plus focused regression coverage.

Is this the best way to solve the issue?

Yes. Rejecting the unsupported syntax at the shared resolver is narrower and safer than silently translating a grammar MCPorter has not documented as supported.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against ae3d9000c320.

Labels

Label justifications:

• P2: This is a bounded configuration defect that can cause misleading authentication or launch failures for affected users.
• merge-risk: 🚨 compatibility: Merging intentionally changes unsupported existing placeholder syntax from literal pass-through to a clear runtime error.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body provides redacted after-fix terminal traces showing rejection before HTTP dispatch and before stdio child-process launch.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides redacted after-fix terminal traces showing rejection before HTTP dispatch and before stdio child-process launch.

Evidence

What I checked:

• Shared resolver rejects unsupported syntax: The added rejection runs in both generic placeholder resolution and env override resolution, preserving the existing accepted-placeholder parser. (src/env.ts:53, 7cc7b43b53cf)
• Direct stdio runtime path is covered: Command preparation detects the unsupported braced form before passing it to the shared resolver; the direct-runtime regression asserts connection fails before launch. (src/runtime/utils.ts:3, 7cc7b43b53cf)
• Current main does not already contain the patch: The tree diff from current main to the PR head contains the resolver, runtime, documentation, schema, and regression changes, so the central behavior is not implemented on main. (src/env.ts:6, ae3d9000c320)
• Feature-history routing: The environment-resolution history includes the original config-resolution and escaped-placeholder work, predominantly authored by steipete. (src/env.ts:5, 17c51a845dd3)
• Post-fix behavior evidence: The PR body records redacted built-CLI runs showing an invalid HTTP header fails before request dispatch and an invalid stdio definition fails before its sentinel-writing child can launch. (tests/env-and-daemon-utils.test.ts:45, 7cc7b43b53cf)

Likely related people:

• steipete: History for the environment resolver includes the original config-resolution and escaped-placeholder work, and the area has substantial authored history under this identity. (role: long-term environment and runtime contributor; confidence: high; commits: ea9108627350, 17c51a845dd3; files: src/env.ts, src/runtime/utils.ts, src/runtime/transport.ts)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)

• reviewed 2026-08-16T22:01:22.795Z sha 3bd3515 :: needs real behavior proof before merge. :: [P2] Validate configured env values before skipping inherited keys
• reviewed 2026-08-17T00:54:08.858Z sha 37326ce :: needs maintainer review before merge. :: none
• reviewed 2026-08-22T22:56:12.627Z sha 37326ce :: needs maintainer review before merge. :: none

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3bd3515b5a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".