diff --git a/.changeset/staging-e2e-validate-gate.md b/.changeset/staging-e2e-validate-gate.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/staging-e2e-validate-gate.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.github/workflows/e2e-staging.yml b/.github/workflows/e2e-staging.yml index 2e811ddac1b..0a2257779ca 100644 --- a/.github/workflows/e2e-staging.yml +++ b/.github/workflows/e2e-staging.yml @@ -111,13 +111,21 @@ jobs: - name: Validate staging instance settings run: node scripts/validate-staging-instances.mjs env: + # Report-only unless the `STAGING_VALIDATE_STRICT` repo variable is set to "true"/"1". + # When strict, a mismatch on critical config (see CRITICAL_PATHS in the script) fails + # this job, which gates the integration-tests job below so the run stops fast with a + # clear diagnostic instead of letting a drifted staging mirror produce opaque failures. + STAGING_VALIDATE_STRICT: ${{ vars.STAGING_VALIDATE_STRICT }} INTEGRATION_INSTANCE_KEYS: ${{ secrets.INTEGRATION_INSTANCE_KEYS }} INTEGRATION_STAGING_INSTANCE_KEYS: ${{ secrets.INTEGRATION_STAGING_INSTANCE_KEYS }} integration-tests: name: Integration Tests (${{ matrix.test-name }}, ${{ matrix.test-project }}) - needs: [permissions-check] - if: ${{ always() && (needs.permissions-check.result == 'success' || needs.permissions-check.result == 'skipped') }} + needs: [permissions-check, validate-instances] + # Run when permissions passed/skipped AND the staging-config validation did not block. + # validate-instances only fails when strict gating is enabled and critical config drifted, + # so by default (report-only) this is a no-op and tests run as before. + if: ${{ always() && (needs.permissions-check.result == 'success' || needs.permissions-check.result == 'skipped') && (needs.validate-instances.result == 'success' || needs.validate-instances.result == 'skipped') }} runs-on: 'blacksmith-8vcpu-ubuntu-2204' defaults: run: @@ -321,7 +329,9 @@ jobs: report: name: Report Results - needs: [integration-tests] + # validate-instances is needed so a strict-mode gate failure (which SKIPS all + # test legs rather than failing them) still reaches the Slack notification. + needs: [integration-tests, validate-instances] if: always() runs-on: 'blacksmith-8vcpu-ubuntu-2204' defaults: @@ -355,7 +365,7 @@ jobs: fi - name: Notify Slack on failure - if: ${{ needs.integration-tests.result == 'failure' && steps.inputs.outputs.notify-slack == 'true' }} + if: ${{ (needs.integration-tests.result == 'failure' || needs.validate-instances.result == 'failure') && steps.inputs.outputs.notify-slack == 'true' }} uses: slackapi/slack-github-action@e28cf165c92ffef168d23c5c9000cffc8a25e117 # v1.24.0 with: payload: | diff --git a/scripts/validate-staging-instances.mjs b/scripts/validate-staging-instances.mjs index 5afb0178617..3d01cd508e7 100644 --- a/scripts/validate-staging-instances.mjs +++ b/scripts/validate-staging-instances.mjs @@ -21,20 +21,77 @@ const STAGING_KEY_PREFIX = 'clerkstage-'; * Paths to ignore during comparison — these are expected to differ between * production and staging environments. */ -const IGNORED_PATHS = [ - /\.id$/, - /^auth_config\.id$/, - /\.logo_url$/, - /\.captcha_enabled$/, - /\.captcha_widget_type$/, - /\.enforce_hibp_on_sign_in$/, - /\.disable_hibp$/, -]; +const IGNORED_PATHS = [/\.id$/, /^auth_config\.id$/, /\.logo_url$/, /\.enforce_hibp_on_sign_in$/, /\.disable_hibp$/]; function isIgnored(path) { return IGNORED_PATHS.some(pattern => pattern.test(path)); } +// ── Gating policy ──────────────────────────────────────────────────────────── + +/** + * Functional configuration that must match between a production instance and its + * staging mirror for the e2e suite to be meaningful. A mismatch on any of these + * paths fails the gate in strict mode; every other difference is reported as + * informational drift and never blocks. Keep this list tight: only config that + * actually changes which auth flows are possible belongs here. + */ +const CRITICAL_PATHS = [ + // An auth attribute (email_address, phone_number, username, ...) toggled on/off. + /^user_settings\.attributes\.[^.]+\.enabled$/, + // The phone-code channel set (sms / whatsapp), which drives alternate-channel UIs. + /^user_settings\.attributes\.phone_number\.channels$/, + // Enabled auth strategies / factors for an attribute. + /^user_settings\.attributes\.[^.]+\.(first_factors|second_factors|verifications)$/, + // A social provider enabled/disabled, or wholly added/removed. + /^user_settings\.social\.[^.]+(\.enabled)?$/, + // Password policy, which affects password sign-in / sign-up flows. + /^user_settings\.password_settings\..+/, + // Bot protection: an enabled captcha blocks every in-browser sign-up in + // headless CI unless the test carries a bypass token (widget type alone is + // inert while captcha is disabled, so it stays informational). + /^user_settings\.sign_up\.captcha_enabled$/, +]; + +/** + * Known, intentionally-tolerated critical drift that should NOT fail the gate, so + * that NEW drift still does. Each entry needs a `path` (string or RegExp), an + * optional `instance` name to scope it, and a `reason` (ideally a tracking link). + * Prefer fixing the staging instance over adding entries here. + */ +const ACCEPTED_DRIFT = [ + // e.g. { instance: 'with-whatsapp-phone-code', path: 'user_settings.attributes.phone_number.channels', + // reason: 'WhatsApp channel not yet provisioned on staging (CLERK-XXXX)' }, +]; + +function isCriticalPath(path) { + return CRITICAL_PATHS.some(pattern => pattern.test(path)); +} + +function isAcceptedDrift(instanceName, path, acceptedDrift = ACCEPTED_DRIFT) { + return acceptedDrift.some(entry => { + if (entry.instance !== undefined && entry.instance !== instanceName) return false; + return typeof entry.path === 'string' ? entry.path === path : entry.path.test(path); + }); +} + +/** + * Split a pair's mismatches into blocking (critical and not accepted) and + * informational. Pure and side-effect free for testability. + */ +function classifyMismatches(instanceName, mismatches, acceptedDrift = ACCEPTED_DRIFT) { + const blocking = []; + const informational = []; + for (const m of mismatches) { + if (isCriticalPath(m.path) && !isAcceptedDrift(instanceName, m.path, acceptedDrift)) { + blocking.push(m); + } else { + informational.push(m); + } + } + return { blocking, informational }; +} + // ── Key loading ────────────────────────────────────────────────────────────── function loadKeys(envVar, filePath) { @@ -311,7 +368,7 @@ function printReport(name, mismatches) { // ── Main ───────────────────────────────────────────────────────────────────── -async function main() { +async function main({ strict = ['1', 'true'].includes(process.env.STAGING_VALIDATE_STRICT) } = {}) { const { keys: prodKeys, errors: prodErrors } = loadKeys('INTEGRATION_INSTANCE_KEYS', 'integration/.keys.json'); for (const err of prodErrors) console.error(`⚠️ Production keys: ${err}`); if (!prodKeys) { @@ -367,6 +424,8 @@ async function main() { let mismatchCount = 0; let fetchFailCount = 0; + let blockingTotal = 0; + const blockingByInstance = []; for (const pair of validPairs) { const prodDomain = parseFapiDomain(pair.prod.pk); @@ -386,6 +445,12 @@ async function main() { mismatches = collapseAttributeMismatches(mismatches); mismatches = collapseSocialMismatches(mismatches); + const { blocking } = classifyMismatches(pair.name, mismatches); + if (blocking.length > 0) { + blockingTotal += blocking.length; + blockingByInstance.push({ name: pair.name, paths: blocking.map(m => m.path) }); + } + if (mismatches.length > 0) mismatchCount++; printReport(pair.name, mismatches); } @@ -397,12 +462,32 @@ async function main() { const matchedCount = validPairs.length - mismatchCount - fetchFailCount; if (matchedCount > 0) parts.push(`${matchedCount} matched`); console.log(`Summary: ${parts.join(', ')} (${validPairs.length} total)`); + + // Gating: only mismatches on critical config block, and only in strict mode. + // Fetch failures and cosmetic drift never fail the build, to avoid false reds. + if (blockingTotal > 0) { + console.log(''); + console.log( + `❌ ${blockingTotal} blocking mismatch(es) on critical config across ${blockingByInstance.length} instance(s):`, + ); + for (const { name, paths } of blockingByInstance) { + for (const p of paths) console.log(` - ${name}: ${p}`); + } + if (strict) { + console.error( + '\nStaging instance config has drifted on critical paths. Fix the staging instance(s) or add an accepted-drift entry.', + ); + process.exit(1); + } + console.log('\n(Report-only: set STAGING_VALIDATE_STRICT=1 or pass --strict to fail the build on the above.)'); + } } // Allow importing functions for testing while still being executable const isDirectRun = process.argv[1] === fileURLToPath(import.meta.url); if (isDirectRun) { - main().catch(err => { + const strict = ['1', 'true'].includes(process.env.STAGING_VALIDATE_STRICT) || process.argv.includes('--strict'); + main({ strict }).catch(err => { console.error('Unexpected error:', err); process.exit(0); }); @@ -416,5 +501,9 @@ export { collapseAttributeMismatches, collapseSocialMismatches, compareEnvironments, + isIgnored, + isCriticalPath, + isAcceptedDrift, + classifyMismatches, main, }; diff --git a/scripts/validate-staging-instances.test.mjs b/scripts/validate-staging-instances.test.mjs index 57c86a53801..5983d8753c5 100644 --- a/scripts/validate-staging-instances.test.mjs +++ b/scripts/validate-staging-instances.test.mjs @@ -7,15 +7,36 @@ vi.mock('node:fs', async importOriginal => { }); import { + classifyMismatches, collapseAttributeMismatches, collapseSocialMismatches, + compareEnvironments, diffObjects, fetchEnvironment, + isCriticalPath, + isIgnored, loadKeys, main, parseFapiDomain, } from './validate-staging-instances.mjs'; +// ── isIgnored ─────────────────────────────────────────────────────────────── + +describe('isIgnored', () => { + it('compares captcha settings (behavior-changing for headless e2e)', () => { + expect(isIgnored('user_settings.sign_up.captcha_enabled')).toBe(false); + expect(isIgnored('user_settings.sign_up.captcha_widget_type')).toBe(false); + }); + + it('still ignores ids, logo_url and hibp settings', () => { + expect(isIgnored('user_settings.social.oauth_google.id')).toBe(true); + expect(isIgnored('auth_config.id')).toBe(true); + expect(isIgnored('user_settings.social.oauth_google.logo_url')).toBe(true); + expect(isIgnored('user_settings.sign_in.enforce_hibp_on_sign_in')).toBe(true); + expect(isIgnored('user_settings.password_settings.disable_hibp')).toBe(true); + }); +}); + // ── loadKeys ──────────────────────────────────────────────────────────────── describe('loadKeys', () => { @@ -317,6 +338,116 @@ describe('collapseSocialMismatches', () => { }); }); +// ── isCriticalPath ────────────────────────────────────────────────────────── + +describe('isCriticalPath', () => { + it('flags attribute enabled toggles', () => { + expect(isCriticalPath('user_settings.attributes.phone_number.enabled')).toBe(true); + expect(isCriticalPath('user_settings.attributes.email_address.enabled')).toBe(true); + }); + + it('flags phone channel changes', () => { + expect(isCriticalPath('user_settings.attributes.phone_number.channels')).toBe(true); + }); + + it('flags factor / verification strategy changes', () => { + expect(isCriticalPath('user_settings.attributes.email_address.first_factors')).toBe(true); + expect(isCriticalPath('user_settings.attributes.phone_number.second_factors')).toBe(true); + expect(isCriticalPath('user_settings.attributes.email_address.verifications')).toBe(true); + }); + + it('flags social provider enable/disable and wholly added/removed', () => { + expect(isCriticalPath('user_settings.social.google')).toBe(true); + expect(isCriticalPath('user_settings.social.google.enabled')).toBe(true); + }); + + it('flags password settings', () => { + expect(isCriticalPath('user_settings.password_settings.min_length')).toBe(true); + }); + + it('flags captcha_enabled but not captcha_widget_type', () => { + expect(isCriticalPath('user_settings.sign_up.captcha_enabled')).toBe(true); + expect(isCriticalPath('user_settings.sign_up.captcha_widget_type')).toBe(false); + }); + + it('no critical path is swallowed by the ignore filter before classification', () => { + const samples = [ + 'user_settings.attributes.phone_number.enabled', + 'user_settings.attributes.phone_number.channels', + 'user_settings.attributes.email_address.first_factors', + 'user_settings.social.google.enabled', + 'user_settings.password_settings.min_length', + 'user_settings.sign_up.captcha_enabled', + ]; + for (const path of samples) { + expect(isCriticalPath(path), path).toBe(true); + expect(isIgnored(path), path).toBe(false); + } + }); + + it('does not flag cosmetic / non-critical paths', () => { + expect(isCriticalPath('auth_config.single_session_mode')).toBe(false); + expect(isCriticalPath('organization_settings.enabled')).toBe(false); + expect(isCriticalPath('user_settings.social.google.strategy')).toBe(false); + expect(isCriticalPath('user_settings.sign_in.second_factor.required')).toBe(false); + }); +}); + +// ── classifyMismatches ────────────────────────────────────────────────────── + +describe('classifyMismatches', () => { + it('separates blocking (critical) from informational drift', () => { + const mismatches = [ + { path: 'user_settings.attributes.email_address.enabled', prod: true, staging: false }, + { path: 'auth_config.single_session_mode', prod: true, staging: false }, + ]; + const { blocking, informational } = classifyMismatches('myapp', mismatches); + expect(blocking.map(m => m.path)).toEqual(['user_settings.attributes.email_address.enabled']); + expect(informational.map(m => m.path)).toEqual(['auth_config.single_session_mode']); + }); + + it('respects accepted drift scoped to a specific instance', () => { + const mismatches = [ + { + path: 'user_settings.attributes.phone_number.channels', + prod: ['sms', 'whatsapp'], + staging: ['sms'], + missingOnStaging: ['whatsapp'], + }, + ]; + const accepted = [ + { instance: 'with-whatsapp-phone-code', path: 'user_settings.attributes.phone_number.channels', reason: 'x' }, + ]; + expect(classifyMismatches('with-whatsapp-phone-code', mismatches, accepted).blocking).toHaveLength(0); + // The same drift on a different instance is NOT accepted. + expect(classifyMismatches('other-instance', mismatches, accepted).blocking).toHaveLength(1); + }); + + it('accepts drift matched by a RegExp path with no instance scope', () => { + const mismatches = [{ path: 'user_settings.password_settings.min_length', prod: 8, staging: 6 }]; + const accepted = [{ path: /^user_settings\.password_settings\./, reason: 'x' }]; + expect(classifyMismatches('any', mismatches, accepted).blocking).toHaveLength(0); + }); + + it('blocks on captcha_enabled drift but not captcha_widget_type', () => { + const mismatches = [ + { path: 'user_settings.sign_up.captcha_enabled', prod: false, staging: true }, + { path: 'user_settings.sign_up.captcha_widget_type', prod: 'smart', staging: '' }, + ]; + const { blocking, informational } = classifyMismatches('with-legal-consent', mismatches); + expect(blocking.map(m => m.path)).toEqual(['user_settings.sign_up.captcha_enabled']); + expect(informational.map(m => m.path)).toEqual(['user_settings.sign_up.captcha_widget_type']); + }); + + it('captcha_enabled drift survives the full compare-filter-classify pipeline', () => { + const prodEnv = { user_settings: { sign_up: { captcha_enabled: false } } }; + const stagingEnv = { user_settings: { sign_up: { captcha_enabled: true } } }; + const mismatches = compareEnvironments(prodEnv, stagingEnv).filter(m => !isIgnored(m.path)); + const { blocking } = classifyMismatches('with-legal-consent', mismatches); + expect(blocking.map(m => m.path)).toEqual(['user_settings.sign_up.captcha_enabled']); + }); +}); + // ── fetchEnvironment ──────────────────────────────────────────────────────── describe('fetchEnvironment', () => { @@ -387,6 +518,7 @@ describe('main', () => { // Clean up env vars delete process.env.INTEGRATION_INSTANCE_KEYS; delete process.env.INTEGRATION_STAGING_INSTANCE_KEYS; + delete process.env.STAGING_VALIDATE_STRICT; }); afterEach(() => { @@ -535,4 +667,94 @@ describe('main', () => { expect(consoleErrors.some(m => m.includes('bad_entry'))).toBe(true); expect(consoleLogs.some(m => m.includes('1 key load errors'))).toBe(true); }); + + // ── strict gating ────────────────────────────────────────────────────────── + + function mockEnvPair(prodEnv, stagingEnv) { + let callCount = 0; + vi.spyOn(globalThis, 'fetch').mockImplementation(() => { + callCount++; + const env = callCount % 2 === 1 ? prodEnv : stagingEnv; + return Promise.resolve({ ok: true, json: () => Promise.resolve(env) }); + }); + } + + const emptyUserSettings = () => ({ attributes: {}, social: {}, sign_in: {}, sign_up: {}, password_settings: {} }); + + function setPair() { + process.env.INTEGRATION_INSTANCE_KEYS = JSON.stringify({ myapp: { pk: PROD_PK } }); + process.env.INTEGRATION_STAGING_INSTANCE_KEYS = JSON.stringify({ 'clerkstage-myapp': { pk: STAGING_PK } }); + } + + it('exits non-zero in strict mode when a critical config path drifts', async () => { + setPair(); + mockEnvPair( + { + auth_config: {}, + organization_settings: {}, + user_settings: { ...emptyUserSettings(), attributes: { email_address: { enabled: true } } }, + }, + { + auth_config: {}, + organization_settings: {}, + user_settings: { ...emptyUserSettings(), attributes: { email_address: { enabled: false } } }, + }, + ); + + await expect(main({ strict: true })).rejects.toThrow('process.exit(1)'); + expect(exitCode).toBe(1); + expect(consoleLogs.some(m => m.includes('blocking mismatch'))).toBe(true); + }); + + it('reports but does not exit non-zero on a critical mismatch when not strict', async () => { + setPair(); + mockEnvPair( + { + auth_config: {}, + organization_settings: {}, + user_settings: { ...emptyUserSettings(), attributes: { email_address: { enabled: true } } }, + }, + { + auth_config: {}, + organization_settings: {}, + user_settings: { ...emptyUserSettings(), attributes: { email_address: { enabled: false } } }, + }, + ); + + await main({ strict: false }); + expect(exitCode).toBeUndefined(); + expect(consoleLogs.some(m => m.includes('Report-only'))).toBe(true); + }); + + it('does not block on non-critical drift even in strict mode', async () => { + setPair(); + mockEnvPair( + { auth_config: { single_session_mode: true }, organization_settings: {}, user_settings: emptyUserSettings() }, + { auth_config: { single_session_mode: false }, organization_settings: {}, user_settings: emptyUserSettings() }, + ); + + await main({ strict: true }); + expect(exitCode).toBeUndefined(); + expect(consoleLogs.some(m => m.includes('1 mismatched'))).toBe(true); + }); + + it('defaults strict from STAGING_VALIDATE_STRICT=1 and blocks on critical drift', async () => { + process.env.STAGING_VALIDATE_STRICT = '1'; + setPair(); + mockEnvPair( + { + auth_config: {}, + organization_settings: {}, + user_settings: { ...emptyUserSettings(), attributes: { phone_number: { enabled: true } } }, + }, + { + auth_config: {}, + organization_settings: {}, + user_settings: { ...emptyUserSettings(), attributes: { phone_number: { enabled: false } } }, + }, + ); + + await expect(main()).rejects.toThrow('process.exit(1)'); + expect(exitCode).toBe(1); + }); });