fix(plugin): renderNow fails closed without a token; v0.32.0

[harper-joseph]

Follow-up to a security-medium review finding on the consumer side (kohls-pr#49). The consumer fix there was to set enabled: false; this removes the underlying footgun for every consumer.

The problem

isRenderNowAuthorized treated "no token configured" as "authorize anyone who sends the header":

return token ? value === token : true;

The schema documented this as an intentional unauthenticated mode, warned about at config-apply time. On a path that takes public crawler traffic that is a DoS lever, not a convenience — an authorized GET can skip the served cache and force a synchronous render that occupies the request for up to timeoutMs (default 30s).

Why permissive is the wrong default here

It is the state a misconfiguration lands in. config.js assigns from the environment only when the variable is actually set:

const renderNowEnv = config.renderNow.valueEnv;
if (renderNowEnv && process.env[renderNowEnv]) {
  config.renderNow.token = process.env[renderNowEnv];
}

So valueEnv naming a variable that is unset — a typo, or an environment that was never provisioned — silently leaves token at its empty default. Under the old reading, a typo in a variable name became an open door. The failure mode of a secret-management mistake should never be "no secret required."

This is not hypothetical: the review suggestion that prompted this was to replace a hardcoded token with valueEnv, and the variable in question was not provisioned anywhere in that deployment. Applying it as suggested would have swapped a guessable token for no token at all — which, under the old semantics, was strictly worse.

The change

• isRenderNowAuthorized requires a non-empty token. Enabling is now necessary but not sufficient; presence of the header never authorizes on its own.
• Schema updated to match — the unauthenticated mode is withdrawn rather than documented. token is described as required, an empty one as making the feature inert, and a guessable placeholder ("true") called out as no better than none.
• Config warning rewritten. It previously reported a "DoS risk" that can no longer occur, which would send someone hunting an exposure that does not exist; it now says the levers are inert. It also names an unresolved valueEnv variable, which is the difference between a five-second fix and a hunt.

Breaking change

Anyone deliberately running enabled-with-no-token to leave the levers open to any caller loses that. It was documented as a DoS vector and warned about at startup, so it is being withdrawn rather than preserved. Such deployments must set renderNow.token (or a valueEnv that resolves) to keep the levers working. Nothing changes for a deployment that already sets a token, and nothing changes for organic crawler traffic either way — it never carried the header.

Tests

389/389 pass; lint and format clean.

Three behavioral tests, each verified to fail with the change reverted so they guard the semantics rather than restate them:

• isRenderNowAuthorized fails closed with no token (previously the case that authorized on presence)
• an unresolved valueEnv leaves the levers inert, not open
• resolveServingPolicy grants no levers when the token is missing — the end-to-end consequence: a miss proxies the origin and Cache-Control is not honored as a cache skip

Plus warning coverage for both branches (with and without valueEnv), and that a configured token reports nothing.

Notes

Version 0.32.0. 0.31.0 is left for #66, which still stamps 0.29.0 and needs a re-stamp since main is now at 0.30.0.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request updates the on-demand rendering feature (renderNow) to fail closed when no token is configured, eliminating a potential DoS risk where the feature previously defaulted to unauthenticated. It updates the configuration schema, authorization logic, warning messages, and tests to reflect this secure-by-default behavior. The reviewer suggests refining the warning message in config.js to explicitly state that the feature is disabled when a token is missing, ensuring the logs accurately mirror the runtime gate logic.

[gemini-code-assist]

Ensure configuration warning or validation logs accurately mirror the runtime gate logic. If a feature is disabled due to a missing or invalid dependent configuration, the logs should report it as disabled rather than claiming it is enabled. Please update the warning message to clearly state that renderNow is disabled.

Suggested change

	valueEnv
	? `renderNow.enabled but renderNow.valueEnv ("${valueEnv}") is not set in the environment and no renderNow.token is configured — the on-demand levers stay INERT (they fail closed rather than authorizing anyone)`
	: 'renderNow.enabled but no renderNow.token is configured — the on-demand levers stay INERT (they fail closed rather than authorizing anyone); set renderNow.token or renderNow.valueEnv'
	valueEnv
	? `renderNow is disabled because renderNow.valueEnv ("${valueEnv}") is not set in the environment and no renderNow.token is configured.`
	: 'renderNow is disabled because no renderNow.token is configured. Set renderNow.token or renderNow.valueEnv.'

References

1. Ensure configuration warning or validation logs accurately mirror the runtime gate logic. If a feature is disabled due to a missing or invalid dependent configuration, the logs should report it as disabled rather than claiming it is enabled.

[harper-joseph]

Taken — the principle is right, a warning must not read as though the feature is running.

I kept the enabled is true but clause rather than dropping to just "renderNow is disabled because...", because both facts matter to whoever is reading the log: that nothing is on, and why the enabled: true they wrote did not take effect. Someone who set the flag and then reads "renderNow is disabled" with no reference to their setting is likely to go check whether the flag took, which is the hunt the message should prevent.

So:

renderNow.enabled is true but no renderNow.token is configured — renderNow is DISABLED
(the levers fail closed rather than authorizing anyone); set renderNow.token or renderNow.valueEnv

and with an unresolved variable named:

renderNow.enabled is true but renderNow.valueEnv ("FOO") is not set in the environment and no
renderNow.token is configured — renderNow is DISABLED (the levers fail closed rather than
authorizing anyone)

Also aligned the schema description to say DISABLED rather than "inert", so the docs and the log use one vocabulary. Tests updated to assert the wording. 389/389 pass.