HarperFast / HarperFast/oauth

2.5.0: unresolved env placeholder on an mcp boolean gate now drops to its default (behavior change not in CHANGELOG)

Open Beginner friendly
#207 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.