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

packages/plugin/package.json

@@ -1,6 +1,6 @@
 {
 	"name": "@harperfast/prerender",
-	"version": "0.30.0",
+	"version": "0.32.0",
 	"type": "module",
 	"description": "Configurable Harper plugin for prerendering pages for bots and crawlers",
 	"license": "Apache-2.0",

packages/plugin/src/config.js

@@ -362,10 +362,16 @@ export const collectConfigWarnings = () => {
 		if (!config.renderNow.header) {
 			add('warn', 'renderNow.header', 'renderNow.enabled but renderNow.header is empty — on-demand render is disabled');
 		} else if (!config.renderNow.token) {
+			// Inert, not open: isRenderNowAuthorized fails closed without a token. Still worth
+			// reporting, because the operator asked for a feature that is not actually on — and
+			// naming the unresolved variable is the difference between a five-second fix and a hunt.
+			const { valueEnv } = config.renderNow;
 			add(
 				'warn',
 				'renderNow.token',
-				`renderNow ENABLED WITHOUT A TOKEN — any client sending "${config.renderNow.header}" can force cache/origin-bypassing renders (DoS risk); set renderNow.token or renderNow.valueEnv`
+				valueEnv
+					? `renderNow.enabled is true but renderNow.valueEnv ("${valueEnv}") is not set in the environment and no renderNow.token is configured — renderNow is DISABLED (the levers fail closed rather than authorizing anyone)`
+					: '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'
 			);
 		}
 	}

packages/plugin/src/configSchema.js

@@ -289,17 +289,27 @@ export const configSchema = group('Prerender plugin configuration.', {
 			'So `defaultMissMode: prerender` + no Cache-Control = "serve cache, else render now" ' +
 			'(warm-on-demand); adding `Cache-Control: no-cache` = "always render fresh now".',
 		{
-			enabled: option(false, 'Enable the on-demand render levers.'),
+			enabled: option(
+				false,
+				'Enable the on-demand render levers. Enabling is necessary but not sufficient — a non-empty ' +
+					'`token` (or a `valueEnv` that resolves to one) is also required, so this cannot open the ' +
+					'levers on its own.'
+			),
 			header: option(
 				'x-harper-render-now',
-				'Request header that authorizes the on-demand levers. Authorization is gated on its presence; ' +
-					'when a `token` is set the header VALUE must equal it.'
+				'Request header that authorizes the on-demand levers. The header VALUE must equal the ' +
+					'configured `token`; presence alone never authorizes.'
 			),
 			token: option(
 				'',
-				'Expected value of `header`. An empty token leaves the feature unauthenticated (any client ' +
-					'sending the header can force renders — a DoS vector), which is warned about at config-apply ' +
-					'time.',
+				'Expected value of `header`. **Required** — there is no unauthenticated mode: an empty token ' +
+					'leaves renderNow DISABLED (the levers stay off even when `enabled` is true) rather than ' +
+					'authorizing anyone who sends the header, and is reported at config-apply time.\n\n' +
+					'This fails CLOSED deliberately. The levers let a caller bypass the served cache and force ' +
+					'a synchronous render that occupies the request for up to `timeoutMs`, so on a path that ' +
+					'takes public crawler traffic an absent or unresolved token must not degrade to "authorize ' +
+					'everyone". Prefer `valueEnv` so the secret stays out of config.yaml, and never commit a ' +
+					'guessable placeholder — a value like "true" is not meaningfully better than none.',
 				{ secret: true }
 			),
 			valueEnv: option(

packages/plugin/src/util/renderNow.js

@@ -29,10 +29,16 @@ export const resolveServingPolicy = (routeClass, method, headers) => {
 /**
  * Whether a request is an authorized on-demand render ("render now") request.
  *
- * The feature must be enabled and a header name configured; the request must
- * carry that header. When a `token` is configured the header value must equal it
- * (the shared secret gate). When no token is configured, mere presence of the
- * header authorizes — the feature is then unauthenticated (see config warning).
+ * Requires the feature enabled, a header name configured, AND a non-empty token;
+ * the request must carry that header with a value equal to the token.
+ *
+ * This fails CLOSED on a missing token rather than treating "no token" as
+ * "authorize anyone". The levers bypass the served cache and can occupy a request
+ * for up to `timeoutMs` forcing a synchronous render, so on a path that takes
+ * public crawler traffic the unauthenticated reading is a DoS lever, not a
+ * convenience. It is also the state a misconfiguration lands in: `valueEnv`
+ * pointing at an unset variable leaves `token` at its empty default, so the
+ * permissive reading would turn a typo in a variable name into an open door.
  *
  * `headers` is anything with a `.get(name)` accessor (Harper request headers or a
  * `Headers` instance). An unauthorized-but-present header returns false so the
@@ -41,10 +47,10 @@ export const resolveServingPolicy = (routeClass, method, headers) => {
  */
 export const isRenderNowAuthorized = (headers) => {
 	const { enabled, header, token } = config.renderNow;
-	if (!enabled || !header) return false;
+	if (!enabled || !header || !token) return false;
 	const value = headers.get(header);
 	if (value === null || value === undefined) return false;
-	return token ? value === token : true;
+	return value === token;
 };
 
 /**

packages/plugin/test/config.test.js

@@ -141,6 +141,34 @@ test('prefix mode is not held to the prerender-route requirement', () => {
 	assert.equal(findingKeys().includes('ingress.routes'), false);
 });
 
+test('renderNow enabled without a token reports the feature as disabled', () => {
+	// The warning is the operator's only signal that a feature they switched on is not actually
+	// running, so it has to mirror the runtime gate: say DISABLED, not "DoS risk". The old wording
+	// would send someone hunting an exposure that no longer exists, and anything that reads as
+	// "enabled" would hide the fact that nothing is on.
+	applyOptions({ renderNow: { enabled: true } });
+	const finding = collectConfigWarnings().find((f) => f.key === 'renderNow.token');
+	assert.ok(finding, 'expected a renderNow.token finding');
+	assert.match(finding.message, /DISABLED/);
+	assert.match(finding.message, /fail closed/);
+});
+
+test('an unresolved renderNow valueEnv is named in the warning', () => {
+	// Naming the variable is the difference between a five-second fix and a hunt: config.js only
+	// assigns from the environment when the variable is set, so a typo silently leaves token empty.
+	delete process.env.__TEST_RENDER_NOW_ABSENT;
+	applyOptions({ renderNow: { enabled: true, valueEnv: '__TEST_RENDER_NOW_ABSENT' } });
+	const finding = collectConfigWarnings().find((f) => f.key === 'renderNow.token');
+	assert.ok(finding, 'expected a renderNow.token finding');
+	assert.match(finding.message, /__TEST_RENDER_NOW_ABSENT/);
+	assert.match(finding.message, /DISABLED/);
+});
+
+test('renderNow with a token reports nothing', () => {
+	applyOptions({ renderNow: { enabled: true, token: 'a-real-secret' } });
+	assert.equal(findingKeys().includes('renderNow.token'), false);
+});
+
 test('applyOptions sources the security token from valueEnv (overriding the literal)', () => {
 	process.env.__TEST_PR_TOKEN = 'env-secret';
 	try {

packages/plugin/test/renderNow.test.js

@@ -38,13 +38,40 @@ test('isRenderNowAuthorized requires the header value to match the token', () =>
 	assert.equal(isRenderNowAuthorized(headersWith({ 'x-harper-render-now': 'wrong' })), false);
 });
 
-test('isRenderNowAuthorized authorizes on presence when no token is configured', () => {
+test('isRenderNowAuthorized fails CLOSED when no token is configured', () => {
+	// Previously presence alone authorized here, which made an absent token an open door on a
+	// path that takes public crawler traffic: any caller could skip the cache and force a
+	// synchronous render occupying the request for up to timeoutMs. Enabling is now necessary
+	// but not sufficient — without a token the levers are inert.
 	applyOptions({ renderNow: { enabled: true } });
-	assert.equal(isRenderNowAuthorized(headersWith({ 'x-harper-render-now': 'anything' })), true);
-	assert.equal(isRenderNowAuthorized(headersWith({ 'x-harper-render-now': '' })), true);
+	assert.equal(config.renderNow.token, '');
+	assert.equal(isRenderNowAuthorized(headersWith({ 'x-harper-render-now': 'anything' })), false);
+	assert.equal(isRenderNowAuthorized(headersWith({ 'x-harper-render-now': '' })), false);
 	assert.equal(isRenderNowAuthorized(headersWith({})), false);
 });
 
+test('an unresolved valueEnv leaves the levers inert, not open', () => {
+	// The misconfiguration this exists for: config.js only assigns from the environment when the
+	// variable is actually set, so a typo in the variable name leaves token at its empty default.
+	// Under the old permissive reading that typo silently became an open door.
+	delete process.env.RENDER_NOW_TOKEN_ABSENT;
+	applyOptions({ renderNow: { enabled: true, valueEnv: 'RENDER_NOW_TOKEN_ABSENT' } });
+	assert.equal(config.renderNow.token, '');
+	assert.equal(isRenderNowAuthorized(headersWith({ 'x-harper-render-now': 'anything' })), false);
+});
+
+test('resolveServingPolicy grants no levers when the token is missing', () => {
+	// The end-to-end consequence: a miss must proxy the origin as normal rather than forcing a
+	// render, and Cache-Control must not be honored as a cache skip.
+	applyOptions({ renderNow: { enabled: true, defaultMissMode: 'prerender' } });
+	const policy = resolveServingPolicy(
+		PRERENDER,
+		'GET',
+		headersWith({ 'x-harper-render-now': 'anything', 'cache-control': 'no-cache' })
+	);
+	assert.deepEqual(policy, { skipCache: false, missMode: 'origin' });
+});
+
 test('isRenderNowAuthorized honors a custom header name', () => {
 	applyOptions({ renderNow: { enabled: true, header: 'x-acme-render', token: 't' } });
 	assert.equal(isRenderNowAuthorized(headersWith({ 'x-acme-render': 't' })), true);