2.5.0: unresolved env placeholder on an mcp boolean gate now drops to its default (behavior change not in CHANGELOG)
- Dominant language
- JavaScript
- Stars
- 1
- Forks
- 1
- Avg merge
- 2d 16h
- Merged PRs (30d)
- 12
Description
## Summary
Upgrading `@harperfast/oauth` 2.4.0 → 2.5.0 changed how an **unresolved** env placeholder on an `mcp` boolean gate is treated. Given a config where the value is a `${VAR}` placeholder and `VAR` is unset (e.g. `mcp.enabled: "${SOME_VAR}"`):
- **2.4.0 and earlier** left the literal string `"${SOME_VAR}"` in place. For `mcp.enabled` that string is truthy, so MCP was treated as **enabled**; if `mcp.issuer` was also an unresolved placeholder, startup then failed loudly in issuer validation (`mcp.issuer must be an absolute http(s) origin …`) and the instance booted degraded.
- **2.5.0** detects the unresolved `${…}` placeholder, logs a warning, and **removes the field** so the documented default applies. For `mcp.enabled` that means MCP is **disabled** and the instance boots clean — the `/mcp` route is simply not mounted.
This is a strictly safer default (a mistyped or unset gate can no longer silently flip on), and it reads as intentional: the `normalizeBooleanField` implementation special-cases the placeholder, and the type declaration documents each documented boolean as "coerced to a real boolean, or removed with a warning so the documented default applies."
## Why I'm filing
The change isn't called out in the 2.5.0 CHANGELOG under Security / Added / Fixed — it lives only in code comments and the `.d.ts`. A downstream integration test of mine asserted the **old** surface (an env-interpolated `mcp.enabled` with the var unset → degraded boot / `500`), and it broke on the bump with no changelog signal pointing at the cause; it took a source diff between the two tarballs to attribute it.
A one-line **Changed** entry (or a migration note) would save the next integrator that diff. Suggested wording:
> Unresolved `${ENV}` placeholders on documented `mcp` boolean options (`mcp.enabled`, `mcp.clientCredentials.enabled`, `mcp.clientIdMetadataDocuments.enabled`, `mcp.dynamicClientRegistration.enabled`, `mcp.refreshTokenRequiresOfflineAccess`) are now treated as absent — a warning is logged and the documented default applies — rather than being left as a truthy string.
One observability point worth folding into that note: an operator who *intended* `mcp.enabled: "${VAR}"` to be **on** but left `VAR` unset now gets a green boot with MCP silently **off** (only a log warning), where before they'd have hit a loud startup error. That's the right fail-safe for a security gate — just worth telling operators to check for the warning if MCP isn't mounting.
## Repro
`config.yaml`:
```yaml
mcp:
enabled: "${MCP_ENABLED}" # MCP_ENABLED unset
issuer: "${MCP_ISSUER}" # unset
```
- **2.4.0:** `/health` and `/mcp` return `500`; body contains `mcp.issuer must be an absolute http(s) origin`.
- **2.5.0:** MCP disabled; `/mcp` not mounted; no degraded boot; a warning is logged that the placeholder was treated as absent.
## Evidence (from the published tarballs)
- `dist/lib/config.js` — `normalizeBooleanField`: the `isUnresolvedPlaceholder = /^\$\{[^}]*\}$/` branch that warns and `delete obj[field]`. (2.4.0 had no equivalent — a non-coercible value on `mcp.enabled` was left unchanged.)
- `dist/index.js` — the `normalizeMcpSecurityConfig(mcpConfig, logger)` call site; the comment now names the "unresolved `${FLAG}` placeholder" case.
Happy to open a CHANGELOG PR if that's useful.
Contributor guide
Research direction
Start by reviewing the 2.5.0 CHANGELOG sections named under Security, Added, and Fixed, then read dist/lib/config.js and dist/index.js for the unresolved-placeholder behavior and call site. Done means adding a concise Changed or migration entry that explains the new defaulting behavior and warning for the listed mcp boolean options.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- authentication, documentation
- Issue type
- Documentation
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 82/100