Skip to content

MCP server headers: unresolved $VAR placeholders are sent verbatim instead of failing closed #146

Open

Description

@larohra

This issue is for a: (mark with an x)

- [x] bug report -> please search issues before submitting
- [ ] feature request
- [ ] documentation issue or request
- [ ] regression (a behavior that used to work and stopped in a new release)

Summary

discovery/mcp.py validates unresolved $VAR / %VAR% placeholders on an MCP server's url and on auth.client_id, but never on header values. A typo'd or missing app setting therefore ships the placeholder text to the remote server verbatim, with no warning.

This affects the default in_lang_worker backend today — it is not specific to session_runtime.aca_sandbox.

Minimal steps to reproduce

  1. Add an mcp.json with a header referencing an environment variable that is not set:
{
  "servers": {
    "example": {
      "type": "http",
      "url": "https://api.example.com/mcp",
      "headers": { "Authorization": "Bearer $MY_TOKEN_TYPO" }
    }
  }
}
  1. Start the app without defining MY_TOKEN_TYPO.
  2. Invoke an agent that uses the example MCP server.

Any log messages given by the failure

None — that is the bug. No warning, no skipped-server message. The server loads normally and the request goes out with the literal header value.

Contrast with the existing, correct handling for the other two fields:

  • discovery/mcp.py:134 — if has_unresolved_placeholders(url): ... skipping
  • discovery/mcp.py:67 — if has_unresolved_placeholders(client_id): client_id = ""

_build_header_provider (discovery/mcp.py:37-53) applies no such check; it stringifies whatever is present:

static_headers = (
    {str(key): str(value) for key, value in headers.items()}
    if isinstance(headers, dict)
    else {}
)

Why the value is a literal rather than empty

config/env.py:29-34 returns the original match when the variable is unset:

def _dollar_replacer(match: re.Match[str]) -> str:
    return os.environ.get(match.group(1), match.group(0))

This is documented behavior (docs/front-matter-spec.md:928): "If a referenced environment variable is not set, the original placeholder text is left literal." So the outbound header is exactly Authorization: Bearer $MY_TOKEN_TYPO.

Expected/desired behavior

Fail closed and loudly, consistent with how url and auth.client_id are already treated. Either:

  • skip the server with a warning naming the server and the unresolved variable (matches the url precedent), or
  • fail startup.

The url precedent seems the closer match.

Secondary concern: information disclosure

Because the placeholder is sent verbatim, the name of the app setting is disclosed to the remote MCP host on every request. Minor, but it is unnecessary and avoidable, and it will typically end up in the remote's request logs.

Versions

Observed on feature/aca-sandboxes @ 343e3e0; the affected code is not branch-specific and applies to main.

Mention any other details that might be useful

  • The helper needed already exists and is already imported in this module — has_unresolved_placeholders from config.env. The fix is a check in _build_header_provider, plus a regression test.
  • Worth deciding whether the check applies to header names as well as values (dictionary keys are never substituted, per docs/front-matter-spec.md:931, so a $VAR-shaped key is always literal and is probably an authoring mistake worth warning about).
  • Found while investigating secret handling under the ACA sandbox backend, but it is independent of that work and worth fixing on its own merits.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions