diff --git a/ghost/core/core/boot.js b/ghost/core/core/boot.js index e1874cffecb..4a7db596e17 100644 --- a/ghost/core/core/boot.js +++ b/ghost/core/core/boot.js @@ -443,6 +443,10 @@ async function initBackgroundServices({config}) { const updateCheck = require('./server/services/update-check'); updateCheck.scheduleRecurringJobs(); + // Remote feature-flag overrides (Pro-only; inert unless explicitly configured). + const remoteFlags = require('./server/services/remote-flags'); + remoteFlags.init(); + const milestonesService = require('./server/services/milestones'); milestonesService.initAndRun(); diff --git a/ghost/core/core/server/services/remote-flags/index.js b/ghost/core/core/server/services/remote-flags/index.js new file mode 100644 index 00000000000..508400388e5 --- /dev/null +++ b/ghost/core/core/server/services/remote-flags/index.js @@ -0,0 +1,78 @@ +const config = require('../../../shared/config'); +const labs = require('../../../shared/labs'); +const logging = require('@tryghost/logging'); +const request = require('@tryghost/request'); +const RemoteFlagsService = require('./remote-flags-service'); + +let instance = null; + +/** + * Start the remote feature-flag poller, if it is enabled for this instance. + * + * Pro-only and opt-in: the service stays completely inert unless the `remoteFlags` + * config block is explicitly enabled with a manifest `url`, on a container that has + * a `hostSettings:siteId`. Self-hosted and dev installs have neither by default, so + * this is a no-op there and labs behaves exactly as before. + * + * Polling is started fire-and-forget so boot is never blocked on (or failed by) the + * first manifest fetch; the service is fail-open and applies overrides once the + * fetch completes. + * + * @returns {RemoteFlagsService|null} the running service, or null when inert + */ +module.exports.init = function init() { + if (instance) { + return instance; + } + + const remoteFlags = config.get('remoteFlags') || {}; + const siteId = config.get('hostSettings:siteId'); + + if (remoteFlags.enabled !== true || !remoteFlags.url || siteId === undefined || siteId === null) { + return null; + } + + try { + // Validate the URL once here so a misconfigured manifest url fails loudly at + // start rather than silently warning on every poll for the life of the process. + // eslint-disable-next-line no-new + new URL(remoteFlags.url); + } catch (err) { + logging.warn({ + system: {event: 'remote_flags.invalid_url', siteId} + }, `Remote feature flags url is not a valid URL, not starting: ${remoteFlags.url}`); + return null; + } + + instance = new RemoteFlagsService({ + url: remoteFlags.url, + siteId, + getKnownFlags: () => labs.getAllFlags(), + applyOverrides: overrides => labs.setRemoteOverrides(overrides), + request + }); + + // Fire-and-forget: start() is fail-open and never rejects, so this neither + // blocks boot nor produces an unhandled rejection. + instance.start(); + + return instance; +}; + +/** + * Stop the poller. This only halts polling; it intentionally leaves the + * last-applied overrides in place rather than clearing them. + */ +module.exports.stop = function stop() { + if (instance) { + instance.stop(); + instance = null; + } +}; + +/** + * @returns {RemoteFlagsService|null} + */ +module.exports.getInstance = function getInstance() { + return instance; +}; diff --git a/ghost/core/core/server/services/remote-flags/remote-flags-service.js b/ghost/core/core/server/services/remote-flags/remote-flags-service.js new file mode 100644 index 00000000000..541f3399100 --- /dev/null +++ b/ghost/core/core/server/services/remote-flags/remote-flags-service.js @@ -0,0 +1,218 @@ +const logging = require('@tryghost/logging'); +const resolve = require('./resolve'); + +const DEFAULT_POLL_INTERVAL_MS = 5 * 60 * 1000; // 5 minutes +const DEFAULT_JITTER_MS = 60 * 1000; // up to 1 minute of spread per poll +const REQUEST_TIMEOUT_MS = 10 * 1000; + +/** + * Polls a remote JSON manifest of feature-flag overrides, resolves it for this + * site, and pushes the result into the labs service. Designed to be unconditionally + * fail-open: a missing, broken, or unreachable manifest must never change live flag + * state in a way that takes the site down, so every failure path either keeps the + * last-known-good overrides or clears them, and nothing here ever throws out. + * + * The HTTP client, the known-flags source, and the override sink are all injected so + * the polling/caching/fail-open logic can be unit-tested without real I/O or timers. + */ +class RemoteFlagsService { + /** + * @param {object} deps + * @param {string} deps.url - canonical manifest URL (one per environment) + * @param {number|string} deps.siteId - this container's Pro site id + * @param {function(): string[]} deps.getKnownFlags - returns the overridable flag keys + * @param {function(Object): void} deps.applyOverrides - sink for resolved overrides (labs.setRemoteOverrides) + * @param {function(string, object): Promise} deps.request - @tryghost/request-compatible client + * @param {number} [deps.pollInterval] - base poll interval in ms + * @param {number} [deps.jitter] - max random extra delay per poll in ms + * @param {function(): number} [deps.getRandom] - returns 0..1 (injectable for tests) + */ + constructor(deps) { + this.url = deps.url; + this.siteId = deps.siteId; + this.getKnownFlags = deps.getKnownFlags; + this.applyOverrides = deps.applyOverrides; + this.request = deps.request; + this.pollInterval = deps.pollInterval || DEFAULT_POLL_INTERVAL_MS; + this.jitter = deps.jitter === undefined ? DEFAULT_JITTER_MS : deps.jitter; + this.getRandom = deps.getRandom || Math.random; + + this._etag = null; // last seen ETag, for conditional GETs + this._resolvedKey = null; // serialized last-applied resolved map, for change detection + this._timer = null; + this._started = false; + this._refreshing = false; + } + + /** + * Fetch the manifest once, resolve it for this site, and apply it. Always + * fail-open; never rejects. Not re-entrant: overlapping calls are coalesced so + * concurrent refreshes can never race the cached ETag. + * @returns {Promise} + */ + async refresh() { + if (this._refreshing) { + return; + } + this._refreshing = true; + try { + await this._doRefresh(); + } finally { + this._refreshing = false; + } + } + + /** @private */ + async _doRefresh() { + let response; + try { + const headers = {}; + if (this._etag) { + headers['if-none-match'] = this._etag; + } + response = await this.request(this.url, { + method: 'GET', + headers, + throwHttpErrors: false, + responseType: 'text', + followRedirect: false, + retry: {limit: 0}, + timeout: {request: REQUEST_TIMEOUT_MS} + }); + } catch (err) { + // Network/timeout error: keep last-known-good, change nothing. + logging.warn({ + system: {event: 'remote_flags.fetch_failed', siteId: this.siteId}, + err + }, 'Remote feature flags fetch failed; keeping last-known-good'); + return; + } + + try { + const status = response && response.statusCode; + + if (status === 304) { + // Not modified: current overrides are still correct. + return; + } + + if (status === 404) { + // No manifest published: no opinion for anyone, fail open to empty. + this._etag = null; + this._applyAndMaybeLog({}, null); + return; + } + + if (!status || status < 200 || status >= 300) { + logging.warn({ + system: {event: 'remote_flags.fetch_bad_status', siteId: this.siteId, statusCode: status || null} + }, 'Remote feature flags fetch returned an unexpected status; keeping last-known-good'); + return; + } + + let manifest; + try { + manifest = JSON.parse(response.body); + } catch (parseErr) { + logging.warn({ + system: {event: 'remote_flags.parse_failed', siteId: this.siteId}, + err: parseErr + }, 'Remote feature flags manifest was not valid JSON; keeping last-known-good'); + return; + } + + // got lowercases response header keys, so `.etag` is the only spelling. + const etag = (response.headers && response.headers.etag) || null; + // Commit the ETag only after the manifest has actually been applied: if + // apply throws, we keep the old ETag so the next poll re-fetches and + // retries instead of getting a 304 for a manifest we never applied. + this._applyAndMaybeLog(manifest, etag); + this._etag = etag; + } catch (err) { + // Defensive backstop: resolve()/applyOverrides should not throw, but if + // anything does, fail open rather than letting it escape the poll loop. + logging.warn({ + system: {event: 'remote_flags.apply_failed', siteId: this.siteId}, + err + }, 'Remote feature flags could not be applied; keeping last-known-good'); + } + } + + /** + * Resolve a manifest for this site, push it to the override sink, and emit a + * structured log only when the resolved set actually changes (so a steady fleet + * does not log on every poll). + * @private + */ + _applyAndMaybeLog(manifest, etag) { + const resolved = resolve(manifest, { + siteId: this.siteId, + knownFlags: this.getKnownFlags() + }); + + this.applyOverrides(resolved); + + // Canonicalise with sorted keys so a manifest that only reorders its keys + // does not look "changed" and emit a spurious applied log on every container. + const key = JSON.stringify(resolved, Object.keys(resolved).sort()); + if (key !== this._resolvedKey) { + this._resolvedKey = key; + logging.info({ + system: { + event: 'remote_flags.applied', + siteId: this.siteId, + etag: etag || null, + flags: resolved + } + }, 'Remote feature flags applied'); + } + } + + _scheduleNext() { + if (!this._started) { + return; + } + // Ensure a single outstanding timer even across a stop/start cycle where an + // in-flight callback could otherwise schedule a second chain. + if (this._timer) { + clearTimeout(this._timer); + this._timer = null; + } + const delay = this.pollInterval + Math.floor(this.getRandom() * this.jitter); + this._timer = setTimeout(async () => { + await this.refresh(); + this._scheduleNext(); + }, delay); + // Never hold the process open just for the poll timer. + if (this._timer && typeof this._timer.unref === 'function') { + this._timer.unref(); + } + } + + /** + * Start polling: an immediate refresh (so boot reflects current flag state), + * then a jittered recurring poll. Idempotent. + * @returns {Promise} + */ + async start() { + if (this._started) { + return; + } + this._started = true; + await this.refresh(); + this._scheduleNext(); + } + + /** + * Stop polling. Does not clear already-applied overrides. + */ + stop() { + this._started = false; + if (this._timer) { + clearTimeout(this._timer); + this._timer = null; + } + } +} + +module.exports = RemoteFlagsService; diff --git a/ghost/core/core/server/services/remote-flags/resolve.js b/ghost/core/core/server/services/remote-flags/resolve.js new file mode 100644 index 00000000000..6d7fe20849f --- /dev/null +++ b/ghost/core/core/server/services/remote-flags/resolve.js @@ -0,0 +1,131 @@ +const crypto = require('crypto'); +const {z} = require('zod'); + +/** + * Deterministic bucket in the range [0, 99] for a (flag, siteId) pair. + * + * The flag name is part of the hash input so that ramps are decorrelated across + * flags: a site that lands in the enabled bucket for one flag is not biased + * towards being enabled for another. Because the same input always produces the + * same bucket, raising a flag's percent only ever adds sites (monotonic ramp). + * + * md5 is used purely as a fast, uniform, stable bucketing hash, not for any + * security property. `% 100` carries a negligible modulo bias (the 2^32 hash + * space is not a multiple of 100, so buckets 0-95 are over-represented by ~1 part + * in 43 million), which is immaterial at fleet scale. The mapping must stay + * stable: changing the algorithm, byte offset, or separator would silently + * re-bucket every in-flight ramp, so a golden value is pinned in the tests. + * + * @param {string} flag + * @param {number|string} siteId + * @returns {number} integer in [0, 99] + */ +function bucketFor(flag, siteId) { + const digest = crypto.createHash('md5').update(`${flag}:${siteId}`).digest(); + return digest.readUInt32BE(0) % 100; +} + +/** + * The shape of a single manifest entry: either a bare boolean (a full override) or + * a `{value, percent?}` ramp. Validation is delegated to Zod so the contract is + * declarative; note that `z.number()` rejects NaN/Infinity, so a non-finite or + * non-numeric percent is treated as a malformed entry and skipped (such values + * cannot occur in valid JSON anyway). Unknown keys inside a ramp object are stripped. + */ +const entrySchema = z.union([ + z.boolean(), + z.object({ + value: z.boolean(), + percent: z.number().optional() + }) +]); + +/** + * Resolve a sparse remote manifest into a flat `{flag: boolean}` override map for + * a single site. + * + * The manifest is sparse: an absent flag means "no opinion" (fall through to the + * normal labs precedence), not "off". Each present entry is either: + * - a bare boolean: a full override applied to every site, or + * - `{value: boolean, percent?: number}`: a ramp. With no percent (or percent + * >= 100) it is a full override; otherwise the override applies only to sites + * whose deterministic bucket falls below `percent`. + * + * Only keys present in `knownFlags` are honored, and each entry is validated and + * skipped individually (per-entry skip): one bad entry can never discard the rest + * of the manifest, and the manifest can never introduce a flag that core does not + * already define. The flag allowlist is supplied by the caller (the labs service), + * not duplicated here. + * + * @param {*} manifest - parsed manifest; anything that is not a plain object yields `{}` + * @param {object} [options] + * @param {number|string} [options.siteId] - this container's site id; required to resolve ramps + * @param {string[]} [options.knownFlags] - the only flag keys that may be overridden + * @returns {Object} resolved overrides for this site + */ +function resolve(manifest, options) { + const result = {}; + + if (!manifest || typeof manifest !== 'object' || Array.isArray(manifest)) { + return result; + } + + // Normalise options defensively: this function must never throw on bad input, + // so a null/omitted options object or a non-array knownFlags is tolerated. + const {siteId, knownFlags} = options || {}; + const known = new Set(Array.isArray(knownFlags) ? knownFlags : []); + + for (const flag of Object.keys(manifest)) { + if (!known.has(flag)) { + // Unknown flag: never let the manifest invent a flag core doesn't define. + continue; + } + + const parsed = entrySchema.safeParse(manifest[flag]); + if (!parsed.success) { + // Malformed entry: skip it individually, keeping every valid sibling. + continue; + } + const entry = parsed.data; + + // Bare boolean: full override for every site. + if (typeof entry === 'boolean') { + result[flag] = entry; + continue; + } + + // Ramp object. A missing percent is a full override; clamp so a typo can + // never widen beyond the whole fleet or go negative. + let percent = entry.percent === undefined ? 100 : entry.percent; + percent = Math.max(0, Math.min(100, percent)); + + if (percent <= 0) { + // 0% is "no opinion" for this site. + continue; + } + if (percent >= 100) { + result[flag] = entry.value; + continue; + } + + // A ramp needs a stable, scalar site id to bucket against. Without a + // usable id (a non-Pro container with no siteId, or an unexpected + // non-scalar/empty value) we cannot place the site, so we skip the ramp + // while still having applied any full overrides above. Guarding the type + // here also means a hostile siteId can never make bucketFor throw. + const canBucket = (typeof siteId === 'number' && Number.isFinite(siteId)) + || (typeof siteId === 'string' && siteId !== ''); + if (!canBucket) { + continue; + } + + if (bucketFor(flag, siteId) < percent) { + result[flag] = entry.value; + } + } + + return result; +} + +module.exports = resolve; +module.exports.bucketFor = bucketFor; diff --git a/ghost/core/core/shared/config/defaults.json b/ghost/core/core/shared/config/defaults.json index ccb1016bf89..f04ccf441f9 100644 --- a/ghost/core/core/shared/config/defaults.json +++ b/ghost/core/core/shared/config/defaults.json @@ -12,6 +12,10 @@ "url": "https://updates.ghost.org", "forceUpdate": false }, + "remoteFlags": { + "enabled": false, + "url": null + }, "privacy": false, "security": { "allowWebhookInternalIPs": false, diff --git a/ghost/core/core/shared/labs.js b/ghost/core/core/shared/labs.js index 6313e187c5b..058fd9af05a 100644 --- a/ghost/core/core/shared/labs.js +++ b/ghost/core/core/shared/labs.js @@ -56,6 +56,40 @@ const PRIVATE_FEATURES = [ module.exports.GA_KEYS = [...GA_FEATURES]; module.exports.WRITABLE_KEYS_ALLOWLIST = [...PUBLIC_BETA_FEATURES, ...PRIVATE_FEATURES]; +// Resolved remote feature-flag overrides for this instance, pushed in by the +// remote-flags service (Pro-only). A flat `{flag: boolean}` map; empty when the +// feature is disabled or no manifest has been applied, which makes the overlay in +// getAll() a no-op. The store lives here so getAll() stays synchronous and reads a +// plain in-memory object on the hot path, and so this shared module never has to +// depend on a server-side service. +let remoteOverrides = {}; + +/** + * Replace the active remote overrides. Called by the remote-flags service after it + * resolves a manifest for this site. A non-object payload is treated as "none". + * @param {Object} overrides + */ +module.exports.setRemoteOverrides = function setRemoteOverrides(overrides) { + // Store a shallow copy so the caller cannot mutate live flag state by holding + // onto the passed object (values are primitive booleans, so this fully isolates). + remoteOverrides = (overrides && typeof overrides === 'object' && !Array.isArray(overrides)) ? {...overrides} : {}; +}; + +/** + * Drop all remote overrides, returning to purely local flag state. + */ +module.exports.clearRemoteOverrides = function clearRemoteOverrides() { + remoteOverrides = {}; +}; + +/** + * The currently applied remote overrides (read-only copy for visibility/tests). + * @returns {Object} + */ +module.exports.getRemoteOverrides = function getRemoteOverrides() { + return {...remoteOverrides}; +}; + module.exports.getAll = () => { const labs = _.cloneDeep(settingsCache.get('labs')) || {}; @@ -63,6 +97,13 @@ module.exports.getAll = () => { labs[gaKey] = true; }); + // Remote overrides sit above GA_FEATURES (so a remote entry can kill a GA flag + // fleet-wide) but below local config (so an explicit `config.labs` pin always + // wins): config.labs > remote > GA_FEATURES > DB settings. + Object.keys(remoteOverrides).forEach((key) => { + labs[key] = remoteOverrides[key]; + }); + const labsConfig = config.get('labs') || {}; Object.keys(labsConfig).forEach((key) => { labs[key] = labsConfig[key]; diff --git a/ghost/core/package.json b/ghost/core/package.json index e2cdb8e3544..01e145e33ea 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -240,7 +240,8 @@ "terser": "5.46.1", "tiny-glob": "0.2.9", "ua-parser-js": "1.0.41", - "xml": "1.0.1" + "xml": "1.0.1", + "zod": "4.1.12" }, "overrides": { "lodash.template": "4.5.0" diff --git a/ghost/core/test/unit/server/services/remote-flags/index.test.js b/ghost/core/test/unit/server/services/remote-flags/index.test.js new file mode 100644 index 00000000000..b51e6062d82 --- /dev/null +++ b/ghost/core/test/unit/server/services/remote-flags/index.test.js @@ -0,0 +1,155 @@ +const assert = require('node:assert/strict'); +const sinon = require('sinon'); +const logging = require('@tryghost/logging'); + +const configUtils = require('../../../../utils/config-utils'); +const labs = require('../../../../../core/shared/labs'); +const resolve = require('../../../../../core/server/services/remote-flags/resolve'); +const RemoteFlagsService = require('../../../../../core/server/services/remote-flags/remote-flags-service'); +const remoteFlags = require('../../../../../core/server/services/remote-flags'); + +const URL = 'https://assets.example.com/platform/flags.json'; + +describe('remote-flags service index (gating)', function () { + let startStub; + + beforeEach(function () { + // Never make a real request during these tests. + startStub = sinon.stub(RemoteFlagsService.prototype, 'start').resolves(); + }); + + afterEach(async function () { + remoteFlags.stop(); + sinon.restore(); + await configUtils.restore(); + }); + + it('is inert when disabled, even with a url and a site id', function () { + configUtils.set('remoteFlags', {enabled: false, url: URL}); + configUtils.set('hostSettings', {siteId: 42}); + + const instance = remoteFlags.init(); + + assert.equal(instance, null); + assert.equal(startStub.called, false); + assert.equal(remoteFlags.getInstance(), null); + }); + + it('is inert when enabled but no manifest url is configured', function () { + configUtils.set('remoteFlags', {enabled: true}); + configUtils.set('hostSettings', {siteId: 42}); + + assert.equal(remoteFlags.init(), null); + assert.equal(startStub.called, false); + }); + + it('is inert on a non-Pro instance with no site id (self-hosted)', function () { + configUtils.set('remoteFlags', {enabled: true, url: URL}); + // no hostSettings:siteId + + assert.equal(remoteFlags.init(), null); + assert.equal(startStub.called, false); + }); + + it('is inert and warns when the configured url is not a valid URL', function () { + const warnStub = sinon.stub(logging, 'warn'); + configUtils.set('remoteFlags', {enabled: true, url: 'not-a-url'}); + configUtils.set('hostSettings', {siteId: 42}); + + assert.equal(remoteFlags.init(), null); + assert.equal(startStub.called, false); + assert.ok(warnStub.getCalls().some(c => c.args[0]?.system?.event === 'remote_flags.invalid_url')); + }); + + it('constructs and starts the service when enabled with a url and site id', function () { + configUtils.set('remoteFlags', {enabled: true, url: URL}); + configUtils.set('hostSettings', {siteId: 42}); + + const instance = remoteFlags.init(); + + assert.ok(instance instanceof RemoteFlagsService); + assert.equal(instance.url, URL); + assert.equal(instance.siteId, 42); + assert.equal(startStub.calledOnce, true); + // wired to labs + assert.deepEqual(instance.getKnownFlags(), labs.getAllFlags()); + }); + + it('is idempotent: a second init returns the same instance and starts once', function () { + configUtils.set('remoteFlags', {enabled: true, url: URL}); + configUtils.set('hostSettings', {siteId: 42}); + + const first = remoteFlags.init(); + const second = remoteFlags.init(); + + assert.equal(first, second); + assert.equal(startStub.calledOnce, true); + }); + + it('stop() stops the running service and allows a fresh init afterwards', function () { + configUtils.set('remoteFlags', {enabled: true, url: URL}); + configUtils.set('hostSettings', {siteId: 42}); + + const stopStub = sinon.stub(RemoteFlagsService.prototype, 'stop'); + const first = remoteFlags.init(); + remoteFlags.stop(); + + assert.equal(stopStub.calledOnce, true); + assert.equal(remoteFlags.getInstance(), null); + + const second = remoteFlags.init(); + assert.notEqual(second, first); + assert.equal(startStub.calledTwice, true); + }); + + it('applyOverrides is wired to labs.setRemoteOverrides', function () { + configUtils.set('remoteFlags', {enabled: true, url: URL}); + configUtils.set('hostSettings', {siteId: 42}); + + const setStub = sinon.stub(labs, 'setRemoteOverrides'); + const instance = remoteFlags.init(); + instance.applyOverrides({flagA: true}); + + assert.equal(setStub.calledOnceWithExactly({flagA: true}), true); + }); +}); + +describe('remote-flags integration: kill-switch through the real labs flag set', function () { + afterEach(function () { + labs.clearRemoteOverrides(); + }); + + it('honors a remote kill of a real GA flag end-to-end', function () { + if (labs.GA_KEYS.length === 0) { + this.skip(); + return; + } + const gaKey = labs.GA_KEYS[0]; + + // Guards against a future change that drops GA keys from getAllFlags(): if + // that happened, resolve() would skip the kill (unknown flag) and this fails. + assert.equal(labs.isSet(gaKey), true); + + const resolved = resolve({[gaKey]: false}, {siteId: 1, knownFlags: labs.getAllFlags()}); + assert.deepEqual(resolved, {[gaKey]: false}, 'resolver must honor a GA flag key'); + + labs.setRemoteOverrides(resolved); + assert.equal(labs.isSet(gaKey), false, 'remote kill must flip a GA flag off end-to-end'); + }); + + it('honors a remote enable of a real private/beta flag end-to-end', function () { + if (labs.WRITABLE_KEYS_ALLOWLIST.length === 0) { + this.skip(); + return; + } + const flag = labs.WRITABLE_KEYS_ALLOWLIST[0]; + + assert.equal(labs.isSet(flag), false); + + const resolved = resolve({[flag]: {value: true, percent: 100}}, {siteId: 1, knownFlags: labs.getAllFlags()}); + assert.deepEqual(resolved, {[flag]: true}); + + labs.setRemoteOverrides(resolved); + assert.equal(labs.isSet(flag), true); + }); +}); diff --git a/ghost/core/test/unit/server/services/remote-flags/remote-flags-service.test.js b/ghost/core/test/unit/server/services/remote-flags/remote-flags-service.test.js new file mode 100644 index 00000000000..b0f5bb9a7b9 --- /dev/null +++ b/ghost/core/test/unit/server/services/remote-flags/remote-flags-service.test.js @@ -0,0 +1,321 @@ +const assert = require('node:assert/strict'); +const sinon = require('sinon'); +const logging = require('@tryghost/logging'); + +const RemoteFlagsService = require('../../../../../core/server/services/remote-flags/remote-flags-service'); + +const KNOWN = ['flagA', 'flagB', 'commentModeration']; + +function buildService(overrides = {}) { + const request = overrides.request || sinon.stub(); + const applyOverrides = overrides.applyOverrides || sinon.stub(); + const service = new RemoteFlagsService({ + url: 'https://assets.example.com/platform/flags.json', + siteId: 42, + getKnownFlags: () => KNOWN, + applyOverrides, + request, + pollInterval: 1000, + jitter: 0, + getRandom: () => 0, + ...overrides + }); + return {service, request, applyOverrides}; +} + +function ok(body, headers = {}) { + return {statusCode: 200, body: JSON.stringify(body), headers}; +} + +describe('RemoteFlagsService', function () { + let logInfo; + let logWarn; + + beforeEach(function () { + logInfo = sinon.stub(logging, 'info'); + logWarn = sinon.stub(logging, 'warn'); + }); + + afterEach(function () { + sinon.restore(); + }); + + describe('refresh - happy path', function () { + it('fetches, resolves and applies a manifest, and logs remote_flags.applied', async function () { + const {service, request, applyOverrides} = buildService(); + request.resolves(ok({flagA: true, commentModeration: false}, {etag: '"v1"'})); + + await service.refresh(); + + assert.equal(applyOverrides.calledOnce, true); + assert.deepEqual(applyOverrides.firstCall.args[0], {flagA: true, commentModeration: false}); + + const appliedLog = logInfo.getCalls().find(c => c.args[0]?.system?.event === 'remote_flags.applied'); + assert.ok(appliedLog, 'expected a remote_flags.applied log'); + assert.equal(appliedLog.args[0].system.siteId, 42); + assert.deepEqual(appliedLog.args[0].system.flags, {flagA: true, commentModeration: false}); + }); + + it('only honors known flags and resolves percentage ramps via siteId', async function () { + const {service, applyOverrides} = buildService({ + request: sinon.stub().resolves(ok({ + flagA: {value: true, percent: 100}, + flagB: {value: true, percent: 0}, + unknownFlag: true + })) + }); + + await service.refresh(); + + assert.deepEqual(applyOverrides.firstCall.args[0], {flagA: true}); + }); + + it('sends a conditional GET (If-None-Match) once an ETag is known', async function () { + const {service, request} = buildService(); + request.onFirstCall().resolves(ok({flagA: true}, {etag: '"v1"'})); + request.onSecondCall().resolves({statusCode: 304, body: '', headers: {}}); + + await service.refresh(); + await service.refresh(); + + const firstHeaders = request.firstCall.args[1].headers || {}; + const secondHeaders = request.secondCall.args[1].headers || {}; + assert.equal(firstHeaders['if-none-match'], undefined); + assert.equal(secondHeaders['if-none-match'], '"v1"'); + }); + }); + + describe('refresh - 304 not modified', function () { + it('keeps last-known-good and does not re-log on 304', async function () { + const {service, request, applyOverrides} = buildService(); + request.onFirstCall().resolves(ok({flagA: true}, {etag: '"v1"'})); + request.onSecondCall().resolves({statusCode: 304, body: '', headers: {}}); + + await service.refresh(); + const appliedCountAfterFirst = logInfo.getCalls().filter(c => c.args[0]?.system?.event === 'remote_flags.applied').length; + applyOverrides.resetHistory(); + + await service.refresh(); + + assert.equal(applyOverrides.called, false, 'should not re-apply on 304'); + const appliedCountAfterSecond = logInfo.getCalls().filter(c => c.args[0]?.system?.event === 'remote_flags.applied').length; + assert.equal(appliedCountAfterSecond, appliedCountAfterFirst, 'should not re-log applied on 304'); + }); + }); + + describe('refresh - fail open', function () { + it('applies an empty override map on 404 (no manifest)', async function () { + const {service, applyOverrides} = buildService({ + request: sinon.stub().resolves({statusCode: 404, body: 'Not Found', headers: {}}) + }); + + await service.refresh(); + + assert.equal(applyOverrides.calledOnce, true); + assert.deepEqual(applyOverrides.firstCall.args[0], {}); + }); + + it('keeps last-known-good and warns on a network error', async function () { + const {service, request, applyOverrides} = buildService(); + request.onFirstCall().resolves(ok({flagA: true}, {etag: '"v1"'})); + request.onSecondCall().rejects(new Error('ECONNRESET')); + + await service.refresh(); + applyOverrides.resetHistory(); + await service.refresh(); + + assert.equal(applyOverrides.called, false, 'must not change overrides on network error'); + assert.ok(logWarn.getCalls().some(c => c.args[0]?.system?.event === 'remote_flags.fetch_failed')); + }); + + it('keeps last-known-good and warns on a 5xx status', async function () { + const {service, request, applyOverrides} = buildService(); + request.onFirstCall().resolves(ok({flagA: true}, {etag: '"v1"'})); + request.onSecondCall().resolves({statusCode: 503, body: 'oops', headers: {}}); + + await service.refresh(); + applyOverrides.resetHistory(); + await service.refresh(); + + assert.equal(applyOverrides.called, false); + assert.ok(logWarn.called); + }); + + it('keeps last-known-good and warns when the body is not valid JSON', async function () { + const {service, request, applyOverrides} = buildService(); + request.onFirstCall().resolves(ok({flagA: true}, {etag: '"v1"'})); + request.onSecondCall().resolves({statusCode: 200, body: 'not json', headers: {etag: '"v2"'}}); + + await service.refresh(); + applyOverrides.resetHistory(); + await service.refresh(); + + assert.equal(applyOverrides.called, false); + assert.ok(logWarn.getCalls().some(c => c.args[0]?.system?.event === 'remote_flags.parse_failed')); + }); + + it('never throws even if applyOverrides throws', async function () { + const {service} = buildService({ + request: sinon.stub().resolves(ok({flagA: true})), + applyOverrides: sinon.stub().throws(new Error('boom')) + }); + + await assert.doesNotReject(service.refresh()); + }); + }); + + describe('change detection', function () { + it('applies every successful fetch but logs applied only when the resolved set changes', async function () { + const {service, request, applyOverrides} = buildService(); + // Two 200s with the same content (different etag) -> resolved set unchanged. + request.onFirstCall().resolves(ok({flagA: true}, {etag: '"v1"'})); + request.onSecondCall().resolves(ok({flagA: true}, {etag: '"v2"'})); + + await service.refresh(); + await service.refresh(); + + assert.equal(applyOverrides.callCount, 2, 'applies on each successful fetch'); + const appliedLogs = logInfo.getCalls().filter(c => c.args[0]?.system?.event === 'remote_flags.applied'); + assert.equal(appliedLogs.length, 1, 'logs applied only once when nothing changed'); + }); + }); + + describe('review hardening - etag, ordering, re-entrancy', function () { + it('does not re-log applied when the manifest only reorders its keys', async function () { + const {service, request} = buildService(); + request.onFirstCall().resolves(ok({flagA: true, commentModeration: false}, {etag: '"v1"'})); + request.onSecondCall().resolves(ok({commentModeration: false, flagA: true}, {etag: '"v2"'})); + + await service.refresh(); + await service.refresh(); + + const appliedLogs = logInfo.getCalls().filter(c => c.args[0]?.system?.event === 'remote_flags.applied'); + assert.equal(appliedLogs.length, 1, 'reordered-but-identical manifest must not re-log'); + }); + + it('clears the ETag after a 200 with no ETag header, so the next poll re-fetches', async function () { + const {service, request} = buildService(); + request.onFirstCall().resolves(ok({flagA: true}, {})); // no etag + request.onSecondCall().resolves(ok({flagA: true}, {})); + + await service.refresh(); + await service.refresh(); + + assert.equal((request.secondCall.args[1].headers || {})['if-none-match'], undefined); + assert.equal(request.callCount, 2); + }); + + it('does not commit the ETag when applying fails, so the next poll retries instead of getting a 304', async function () { + const request = sinon.stub(); + request.onFirstCall().resolves(ok({flagA: true}, {etag: '"v1"'})); + request.onSecondCall().resolves(ok({flagA: true}, {etag: '"v2"'})); + const applyOverrides = sinon.stub(); + applyOverrides.onFirstCall().throws(new Error('boom')); + const {service} = buildService({request, applyOverrides}); + + await service.refresh(); // apply throws -> ETag must NOT be committed + await service.refresh(); // must re-fetch (no If-None-Match) and apply + + assert.equal((request.secondCall.args[1].headers || {})['if-none-match'], undefined, 'must not send a stale ETag for an unapplied manifest'); + assert.equal(applyOverrides.callCount, 2); + assert.ok(logWarn.getCalls().some(c => c.args[0]?.system?.event === 'remote_flags.apply_failed')); + }); + + it('never throws and warns if getKnownFlags throws', async function () { + const {service} = buildService({ + request: sinon.stub().resolves(ok({flagA: true})), + getKnownFlags: () => { + throw new Error('nope'); + } + }); + + await assert.doesNotReject(service.refresh()); + assert.ok(logWarn.getCalls().some(c => c.args[0]?.system?.event === 'remote_flags.apply_failed')); + }); + + it('treats a 3xx as an unexpected status and keeps last-known-good', async function () { + const {service, request, applyOverrides} = buildService(); + request.onFirstCall().resolves(ok({flagA: true}, {etag: '"v1"'})); + request.onSecondCall().resolves({statusCode: 301, body: '', headers: {location: '/elsewhere'}}); + + await service.refresh(); + applyOverrides.resetHistory(); + await service.refresh(); + + assert.equal(applyOverrides.called, false); + assert.ok(logWarn.getCalls().some(c => c.args[0]?.system?.event === 'remote_flags.fetch_bad_status')); + }); + + it('logs applied on each real transition through 404 -> 200 -> 404 but not on a repeated 404', async function () { + const {service, request} = buildService(); + request.onCall(0).resolves({statusCode: 404, body: '', headers: {}}); + request.onCall(1).resolves(ok({flagA: true}, {etag: '"v1"'})); + request.onCall(2).resolves({statusCode: 404, body: '', headers: {}}); + request.onCall(3).resolves({statusCode: 404, body: '', headers: {}}); + + await service.refresh(); // {} applied (first application) + await service.refresh(); // {flagA:true} + await service.refresh(); // {} again (real transition) + await service.refresh(); // {} repeated (no change) + + const appliedLogs = logInfo.getCalls().filter(c => c.args[0]?.system?.event === 'remote_flags.applied'); + assert.equal(appliedLogs.length, 3); + }); + + it('coalesces overlapping refresh calls into a single fetch', async function () { + let resolveReq; + const request = sinon.stub().returns(new Promise((res) => { + resolveReq = res; + })); + const {service} = buildService({request}); + + const p1 = service.refresh(); + const p2 = service.refresh(); // must early-return while p1 is in flight + resolveReq(ok({flagA: true})); + await Promise.all([p1, p2]); + + assert.equal(request.callCount, 1); + }); + }); + + describe('start / stop scheduling', function () { + it('refreshes immediately on start and reschedules, until stopped', async function () { + const clock = sinon.useFakeTimers(); + try { + const request = sinon.stub().resolves(ok({flagA: true})); + const {service} = buildService({request}); + + await service.start(); + assert.equal(request.callCount, 1, 'refreshes once immediately on start'); + + await clock.tickAsync(1000); // pollInterval, jitter 0 + assert.equal(request.callCount, 2, 'reschedules and polls again'); + + service.stop(); + await clock.tickAsync(5000); + assert.equal(request.callCount, 2, 'no further polls after stop'); + } finally { + clock.restore(); + } + }); + + it('is idempotent: calling start twice does not double-schedule', async function () { + const clock = sinon.useFakeTimers(); + try { + const request = sinon.stub().resolves(ok({flagA: true})); + const {service} = buildService({request}); + + await service.start(); + await service.start(); + assert.equal(request.callCount, 1, 'only one immediate refresh'); + + await clock.tickAsync(1000); + assert.equal(request.callCount, 2, 'only one scheduled poll, not two'); + + service.stop(); + } finally { + clock.restore(); + } + }); + }); +}); diff --git a/ghost/core/test/unit/server/services/remote-flags/resolve.test.js b/ghost/core/test/unit/server/services/remote-flags/resolve.test.js new file mode 100644 index 00000000000..3c69d44db2c --- /dev/null +++ b/ghost/core/test/unit/server/services/remote-flags/resolve.test.js @@ -0,0 +1,252 @@ +const assert = require('node:assert/strict'); + +const resolve = require('../../../../../core/server/services/remote-flags/resolve'); + +// A fixed, dependency-free set of "known" flags for these pure tests. +const KNOWN = ['flagA', 'flagB', 'flagC', 'commentModeration']; + +describe('remote-flags resolve', function () { + describe('input guarding', function () { + it('returns an empty map for a missing manifest', function () { + assert.deepEqual(resolve(undefined, {siteId: 1, knownFlags: KNOWN}), {}); + assert.deepEqual(resolve(null, {siteId: 1, knownFlags: KNOWN}), {}); + }); + + it('returns an empty map for a non-object manifest', function () { + assert.deepEqual(resolve('flagA', {siteId: 1, knownFlags: KNOWN}), {}); + assert.deepEqual(resolve(42, {siteId: 1, knownFlags: KNOWN}), {}); + assert.deepEqual(resolve(['flagA'], {siteId: 1, knownFlags: KNOWN}), {}); + }); + + it('returns an empty map when there are no known flags', function () { + assert.deepEqual(resolve({flagA: true}, {siteId: 1, knownFlags: []}), {}); + assert.deepEqual(resolve({flagA: true}, {siteId: 1}), {}); + }); + + it('never throws when options is null or omitted', function () { + assert.deepEqual(resolve({flagA: true}, null), {}); + assert.deepEqual(resolve({flagA: true}), {}); + }); + + it('never throws when knownFlags is not an array', function () { + assert.deepEqual(resolve({flagA: true}, {siteId: 1, knownFlags: {}}), {}); + assert.deepEqual(resolve({flagA: true}, {siteId: 1, knownFlags: 'flagA'}), {}); + }); + + it('does not mutate the input manifest', function () { + const manifest = {flagA: true, flagB: {value: true, percent: 100}}; + const snapshot = JSON.parse(JSON.stringify(manifest)); + resolve(manifest, {siteId: 1, knownFlags: KNOWN}); + assert.deepEqual(manifest, snapshot); + }); + }); + + describe('bare boolean entries (full override)', function () { + it('applies a bare true to every site', function () { + assert.deepEqual(resolve({flagA: true}, {siteId: 1, knownFlags: KNOWN}), {flagA: true}); + assert.deepEqual(resolve({flagA: true}, {siteId: 99999, knownFlags: KNOWN}), {flagA: true}); + }); + + it('applies a bare false to every site (kill switch)', function () { + assert.deepEqual(resolve({commentModeration: false}, {siteId: 7, knownFlags: KNOWN}), {commentModeration: false}); + }); + }); + + describe('per-entry skip (validation)', function () { + it('skips flags that are not in the known set', function () { + assert.deepEqual(resolve({unknownFlag: true, flagA: true}, {siteId: 1, knownFlags: KNOWN}), {flagA: true}); + }); + + it('skips the special members key (not a labs flag)', function () { + assert.deepEqual(resolve({members: true}, {siteId: 1, knownFlags: KNOWN}), {}); + }); + + it('skips malformed entries but keeps valid siblings', function () { + const manifest = { + flagA: 'yes', // string -> skip + flagB: 123, // number -> skip + flagC: null, // null -> skip + commentModeration: true // valid -> keep + }; + assert.deepEqual(resolve(manifest, {siteId: 1, knownFlags: KNOWN}), {commentModeration: true}); + }); + + it('skips object entries without a boolean value', function () { + const manifest = { + flagA: {percent: 50}, // no value -> skip + flagB: {value: 'yes', percent: 50}, // non-bool value -> skip + flagC: {} // empty -> skip + }; + assert.deepEqual(resolve(manifest, {siteId: 1, knownFlags: KNOWN}), {}); + }); + + it('skips object entries with a non-numeric percent', function () { + const manifest = { + flagA: {value: true, percent: '50'}, // string percent -> skip + flagB: {value: true, percent: NaN} // NaN percent -> skip + }; + assert.deepEqual(resolve(manifest, {siteId: 1, knownFlags: KNOWN}), {}); + }); + }); + + describe('percentage ramps', function () { + it('treats an object value with no percent as a full override', function () { + assert.deepEqual(resolve({flagA: {value: true}}, {siteId: 1, knownFlags: KNOWN}), {flagA: true}); + assert.deepEqual(resolve({flagA: {value: false}}, {siteId: 1, knownFlags: KNOWN}), {flagA: false}); + }); + + it('applies percent: 100 to every site', function () { + assert.deepEqual(resolve({flagA: {value: true, percent: 100}}, {siteId: 123, knownFlags: KNOWN}), {flagA: true}); + }); + + it('applies percent: 0 to no site', function () { + assert.deepEqual(resolve({flagA: {value: true, percent: 0}}, {siteId: 123, knownFlags: KNOWN}), {}); + }); + + it('clamps a negative percent to 0 (no opinion)', function () { + assert.deepEqual(resolve({flagA: {value: true, percent: -10}}, {siteId: 123, knownFlags: KNOWN}), {}); + }); + + it('clamps a percent above 100 to 100 (all sites)', function () { + assert.deepEqual(resolve({flagA: {value: true, percent: 150}}, {siteId: 123, knownFlags: KNOWN}), {flagA: true}); + }); + + it('produces a stable, deterministic 0-99 bucket for a (flag, siteId) pair', function () { + const b1 = resolve.bucketFor('flagA', 42); + const b2 = resolve.bucketFor('flagA', 42); + assert.equal(b1, b2); + assert.ok(b1 >= 0 && b1 < 100, `bucket ${b1} out of range`); + }); + + it('pins the bucket mapping to a golden value (guards cross-deploy ramp stability)', function () { + // If this fails, the hash changed and every in-flight ramp re-buckets: + // sites mid-ramp could flip off. Only update with a deliberate decision. + assert.equal(resolve.bucketFor('flagA', 42), 53); + assert.equal(resolve.bucketFor('commentModeration', 1), 40); + }); + + it('honors a fractional percent at the bucket boundary', function () { + const siteId = 99; + const bucket = resolve.bucketFor('flagA', siteId); + assert.deepEqual(resolve({flagA: {value: true, percent: bucket + 0.5}}, {siteId, knownFlags: KNOWN}), {flagA: true}); + assert.deepEqual(resolve({flagA: {value: true, percent: bucket - 0.5}}, {siteId, knownFlags: KNOWN}), {}); + }); + + it('skips entries whose percent is non-finite (Infinity/NaN are not valid numbers)', function () { + // The Zod schema rejects non-finite percents, so the entry is skipped + // (no opinion) rather than applied. These cannot occur in valid JSON. + assert.deepEqual(resolve({flagA: {value: true, percent: Infinity}}, {siteId: 5, knownFlags: KNOWN}), {}); + assert.deepEqual(resolve({flagA: {value: true, percent: -Infinity}}, {siteId: 5, knownFlags: KNOWN}), {}); + assert.deepEqual(resolve({flagA: {value: true, percent: NaN}}, {siteId: 5, knownFlags: KNOWN}), {}); + }); + + it('omits (does not set false) when a value:false ramp does not apply to this site', function () { + // "no opinion" must mean the key is absent, not present-and-false. + assert.deepEqual(resolve({flagA: {value: false, percent: 0}}, {siteId: 5, knownFlags: KNOWN}), {}); + const siteId = 5; + const bucket = resolve.bucketFor('flagA', siteId); + // out-of-bucket value:false -> omitted + assert.deepEqual(resolve({flagA: {value: false, percent: bucket}}, {siteId, knownFlags: KNOWN}), {}); + }); + + it('decorrelates buckets across flags for the same site', function () { + // Different flag names should not all map to the same bucket for one site. + const buckets = KNOWN.map(flag => resolve.bucketFor(flag, 42)); + const unique = new Set(buckets); + assert.ok(unique.size > 1, `expected varied buckets, got ${JSON.stringify(buckets)}`); + }); + + it('includes a site iff its bucket is below the percent threshold', function () { + const siteId = 314; + const bucket = resolve.bucketFor('flagA', siteId); + + // Just above the bucket -> included. + const included = resolve({flagA: {value: true, percent: bucket + 1}}, {siteId, knownFlags: KNOWN}); + assert.deepEqual(included, {flagA: true}); + + // At/below the bucket -> excluded. + const excluded = resolve({flagA: {value: true, percent: bucket}}, {siteId, knownFlags: KNOWN}); + assert.deepEqual(excluded, {}); + }); + + it('is monotonic: a site enabled at a lower percent stays enabled at a higher percent', function () { + const siteId = 2718; + const bucket = resolve.bucketFor('flagB', siteId); + // Pick a percent that includes this site, then a strictly higher one. + const low = bucket + 1; + const high = Math.min(100, bucket + 5); + assert.deepEqual(resolve({flagB: {value: true, percent: low}}, {siteId, knownFlags: KNOWN}), {flagB: true}); + assert.deepEqual(resolve({flagB: {value: true, percent: high}}, {siteId, knownFlags: KNOWN}), {flagB: true}); + }); + + it('supports ramped kill switches (value: false at a percent)', function () { + const siteId = 555; + const bucket = resolve.bucketFor('commentModeration', siteId); + const result = resolve({commentModeration: {value: false, percent: bucket + 1}}, {siteId, knownFlags: KNOWN}); + assert.deepEqual(result, {commentModeration: false}); + }); + }); + + describe('missing or non-scalar siteId', function () { + it('still applies full overrides but skips ramps when siteId is absent', function () { + const manifest = { + flagA: true, // full -> applies + flagB: {value: true, percent: 100}, // full -> applies + flagC: {value: true, percent: 50} // ramp -> cannot bucket -> skip + }; + assert.deepEqual(resolve(manifest, {knownFlags: KNOWN}), {flagA: true, flagB: true}); + }); + + it('treats siteId 0 as a valid id but empty-string as no id', function () { + // 0 is a finite number -> bucketable; '' -> not bucketable (ramp skipped). + const withZero = resolve({flagA: {value: true, percent: 100}}, {siteId: 0, knownFlags: KNOWN}); + assert.deepEqual(withZero, {flagA: true}); + const zeroBucket = resolve.bucketFor('flagA', 0); + assert.deepEqual(resolve({flagA: {value: true, percent: zeroBucket + 1}}, {siteId: 0, knownFlags: KNOWN}), {flagA: true}); + assert.deepEqual(resolve({flagA: {value: true, percent: 50}}, {siteId: '', knownFlags: KNOWN}), {}); + }); + + it('never throws on a hostile non-scalar siteId, just skips ramps', function () { + const manifest = {flagA: true, flagB: {value: true, percent: 50}}; + // Symbol, object, NaN, Infinity, boolean: full override still applies, ramp skipped. + assert.deepEqual(resolve(manifest, {siteId: Symbol('x'), knownFlags: KNOWN}), {flagA: true}); + assert.deepEqual(resolve(manifest, {siteId: {}, knownFlags: KNOWN}), {flagA: true}); + assert.deepEqual(resolve(manifest, {siteId: NaN, knownFlags: KNOWN}), {flagA: true}); + assert.deepEqual(resolve(manifest, {siteId: Infinity, knownFlags: KNOWN}), {flagA: true}); + assert.deepEqual(resolve(manifest, {siteId: true, knownFlags: KNOWN}), {flagA: true}); + }); + }); + + describe('security: dangerous manifest keys', function () { + it('ignores __proto__/constructor keys from a parsed manifest and never pollutes Object.prototype', function () { + // A real manifest is JSON.parsed from the CDN; JSON.parse creates a real + // own "__proto__" key (unlike an object literal). The knownFlags allowlist + // must drop these and nothing may leak onto the global prototype. + const manifest = JSON.parse('{"__proto__": {"value": true}, "constructor": true, "polluted": true, "flagA": true}'); + const result = resolve(manifest, {siteId: 1, knownFlags: KNOWN}); + assert.deepEqual(result, {flagA: true}); + assert.equal({}.polluted, undefined); + assert.equal({}.value, undefined); + assert.equal(Object.prototype.polluted, undefined); + }); + }); + + describe('mixed manifest', function () { + it('resolves a realistic mix of entries', function () { + const siteId = 4242; + const ramped = resolve.bucketFor('flagC', siteId); + const manifest = { + commentModeration: false, // kill GA flag everywhere + flagA: {value: true, percent: 100}, // enable everywhere + flagB: {value: true, percent: 0}, // enable nowhere + flagC: {value: true, percent: ramped + 1}, // enable for this site + unknownFlag: true // skip + }; + assert.deepEqual(resolve(manifest, {siteId, knownFlags: KNOWN}), { + commentModeration: false, + flagA: true, + flagC: true + }); + }); + }); +}); diff --git a/ghost/core/test/unit/shared/labs.test.js b/ghost/core/test/unit/shared/labs.test.js index fc9832c7e87..5166cfa3027 100644 --- a/ghost/core/test/unit/shared/labs.test.js +++ b/ghost/core/test/unit/shared/labs.test.js @@ -128,6 +128,170 @@ describe('Labs Service', function () { }); }); +describe('Labs Service - remote overrides', function () { + afterEach(async function () { + labs.clearRemoteOverrides(); + sinon.restore(); + await configUtils.restore(); + }); + + it('overlays a remote override so an otherwise-off flag reads on', function () { + if (labs.WRITABLE_KEYS_ALLOWLIST.length === 0) { + this.skip(); + return; + } + const flag = labs.WRITABLE_KEYS_ALLOWLIST[0]; + + assert.equal(labs.isSet(flag), false); + labs.setRemoteOverrides({[flag]: true}); + assert.equal(labs.isSet(flag), true); + assert.equal(labs.getAll()[flag], true); + }); + + it('lets a remote override kill a GA flag (kill switch)', function () { + if (labs.GA_KEYS.length === 0) { + this.skip(); + return; + } + const gaKey = labs.GA_KEYS[0]; + + // GA forces the flag on by default. + assert.equal(labs.isSet(gaKey), true); + + labs.setRemoteOverrides({[gaKey]: false}); + assert.equal(labs.isSet(gaKey), false); + assert.equal(labs.getAll()[gaKey], false); + }); + + it('lets a remote override beat the DB settings value', function () { + if (labs.WRITABLE_KEYS_ALLOWLIST.length === 0) { + this.skip(); + return; + } + const flag = labs.WRITABLE_KEYS_ALLOWLIST[0]; + + const getSpy = sinon.stub(settingsCache, 'get'); + getSpy.withArgs('labs').returns({[flag]: false}); + + labs.setRemoteOverrides({[flag]: true}); + assert.equal(labs.isSet(flag), true); + }); + + it('lets a local config.labs pin beat a remote override', function () { + if (labs.WRITABLE_KEYS_ALLOWLIST.length === 0) { + this.skip(); + return; + } + const flag = labs.WRITABLE_KEYS_ALLOWLIST[0]; + + // config pins the flag OFF; remote tries to turn it ON; config must win. + configUtils.set('labs', {[flag]: false}); + labs.setRemoteOverrides({[flag]: true}); + assert.equal(labs.isSet(flag), false); + }); + + it('never lets a remote override change the members flag', function () { + const getSpy = sinon.stub(settingsCache, 'get'); + getSpy.withArgs('members_signup_access').returns('all'); + + labs.setRemoteOverrides({members: false}); + + // members is always recomputed from settings after the remote overlay. + assert.equal(labs.isSet('members'), true); + }); + + it('is inert with no overrides set and after clearing', function () { + if (labs.GA_KEYS.length === 0) { + this.skip(); + return; + } + const gaKey = labs.GA_KEYS[0]; + + labs.setRemoteOverrides({[gaKey]: false}); + labs.clearRemoteOverrides(); + assert.equal(labs.isSet(gaKey), true); + }); + + it('treats a non-object override payload as empty without throwing', function () { + if (labs.GA_KEYS.length === 0) { + this.skip(); + return; + } + const gaKey = labs.GA_KEYS[0]; + + labs.setRemoteOverrides(null); + labs.setRemoteOverrides('nope'); + labs.setRemoteOverrides(['x']); + labs.setRemoteOverrides(42); + + assert.equal(labs.isSet(gaKey), true); + }); + + it('lets config.labs beat a remote override for a GA key', function () { + if (labs.GA_KEYS.length === 0) { + this.skip(); + return; + } + const gaKey = labs.GA_KEYS[0]; + + // Without config, the remote `false` would kill this GA flag; the config + // pin must override the remote entry and keep it on. + configUtils.set('labs', {[gaKey]: true}); + labs.setRemoteOverrides({[gaKey]: false}); + assert.equal(labs.isSet(gaKey), true); + }); + + it('applies multiple overrides in one payload key-by-key', function () { + if (labs.GA_KEYS.length === 0 || labs.WRITABLE_KEYS_ALLOWLIST.length === 0) { + this.skip(); + return; + } + const gaKey = labs.GA_KEYS[0]; + const writable = labs.WRITABLE_KEYS_ALLOWLIST[0]; + + labs.setRemoteOverrides({[gaKey]: false, [writable]: true}); + assert.equal(labs.isSet(gaKey), false); + assert.equal(labs.isSet(writable), true); + }); + + it('isolates stored overrides from later caller and getAll() mutation', function () { + if (labs.GA_KEYS.length === 0) { + this.skip(); + return; + } + const gaKey = labs.GA_KEYS[0]; + + const payload = {[gaKey]: false}; + labs.setRemoteOverrides(payload); + payload[gaKey] = true; // mutating the caller's object must not affect stored state + assert.equal(labs.isSet(gaKey), false); + + const all = labs.getAll(); + all[gaKey] = true; // mutating getAll() output must not affect stored state + assert.equal(labs.isSet(gaKey), false); + }); + + it('getRemoteOverrides returns a copy and reflects set/clear', function () { + if (labs.WRITABLE_KEYS_ALLOWLIST.length === 0) { + this.skip(); + return; + } + const flag = labs.WRITABLE_KEYS_ALLOWLIST[0]; + + assert.deepEqual(labs.getRemoteOverrides(), {}); + + labs.setRemoteOverrides({[flag]: true}); + assert.deepEqual(labs.getRemoteOverrides(), {[flag]: true}); + + const copy = labs.getRemoteOverrides(); + copy[flag] = false; // mutating the returned copy must not affect stored state + assert.equal(labs.getRemoteOverrides()[flag], true); + + labs.clearRemoteOverrides(); + assert.deepEqual(labs.getRemoteOverrides(), {}); + }); +}); + describe('Labs Service - Flag Integrity', function () { it('should have no duplicate flags across categories', function () { const allFlags = labs.getAllFlags(); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a7ff0be6e64..d127575bfe7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2437,6 +2437,9 @@ importers: xml: specifier: 1.0.1 version: 1.0.1 + zod: + specifier: 4.1.12 + version: 4.1.12 devDependencies: '@actions/core': specifier: 3.0.0