diff --git a/packages/core/src/api.js b/packages/core/src/api.js index 21652835b..8fae6d516 100644 --- a/packages/core/src/api.js +++ b/packages/core/src/api.js @@ -422,13 +422,34 @@ export function createPercyServer(percy, port) { // post a comparison by reading a Maestro screenshot from disk .route('post', '/percy/maestro-screenshot', async (req, res) => { /* istanbul ignore next — req.body falsy guard; tests always pass a body. */ - let { name, sessionId } = req.body || {}; + let { name, sessionId, runtime } = req.body || {}; if (!name) throw new ServerError(400, 'Missing required field: name'); - // `sessionId` is host-injected on BrowserStack; its absence signals - // self-hosted mode (gated separately on PERCY_MAESTRO_SCREENSHOT_DIR - // below). When present, the BS path runs byte-identical. - let selfHosted = !sessionId; + + // Validate `runtime` if present: must be a string, but unknown values + // are NOT rejected — we treat anything other than "selfhosted" / + // "browserstack" as "fall back to sessionId-absent detection". This + // is defensive against future runtime values (maestro-cloud, + // saucelabs, etc.) the SDK might introduce ahead of relay support. + if (runtime !== undefined && typeof runtime !== 'string') { + throw new ServerError(400, 'Invalid runtime: must be a string'); + } + + // Self-hosted detection — prefer the SDK's explicit declaration when + // present, fall back to sessionId absence (the implicit signal used + // by SDKs that predate the runtime field). The fallback is what + // ships in cli#2261 today; the explicit runtime field arrives with + // @percy/maestro-app >= v1.0.0-Beta.1 (cf. percy-maestro-app#7+R2). + let selfHosted = (runtime === 'selfhosted') || (!runtime && !sessionId); + + // Diagnostic surface for any inconsistency between the two signals + // (debug-only — never breaks the request). + if (runtime === 'browserstack' && !sessionId) { + percy.log.debug('maestro-screenshot: runtime=browserstack but sessionId missing — trusting runtime field'); + } + if (runtime === 'selfhosted' && sessionId) { + percy.log.debug('maestro-screenshot: runtime=selfhosted but sessionId present — trusting runtime field'); + } // Strict character-class validation — rejects path separators, shell metacharacters, // NUL, newlines, and anything else that could confuse the glob or the filesystem. diff --git a/packages/core/test/api.test.js b/packages/core/test/api.test.js index e432f9d2c..9dadae5ad 100644 --- a/packages/core/test/api.test.js +++ b/packages/core/test/api.test.js @@ -1255,6 +1255,103 @@ describe('API Server', () => { .toBeRejectedWithError(/Invalid platform: must be a string/); }); + describe('runtime field gating', () => { + // The relay's self-hosted detection prefers an explicit `runtime` field + // from the SDK when present, falling back to sessionId-absence for older + // SDKs. These tests cover the eight-way matrix of (runtime ∈ + // {undefined, "selfhosted", "browserstack", "unknown"}) × (sessionId ∈ + // {present, absent}) — plus the type-validation 400. + + it('rejects non-string runtime type with 400', async () => { + await percy.start(); + await expectAsync(postMaestro({ name: SS_NAME, sessionId: SID, runtime: 123 })) + .toBeRejectedWithError(/Invalid runtime: must be a string/); + }); + + it('runtime=selfhosted + no sessionId → self-hosted branch (400s on missing env)', async () => { + let prior = process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + delete process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + try { + await percy.start(); + await expectAsync(postMaestro({ name: SS_NAME, runtime: 'selfhosted' })) + .toBeRejectedWithError(/Missing required env: PERCY_MAESTRO_SCREENSHOT_DIR/); + } finally { + if (prior === undefined) delete process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + else process.env.PERCY_MAESTRO_SCREENSHOT_DIR = prior; + } + }); + + it('runtime=selfhosted + sessionId PRESENT → still self-hosted branch (trust runtime field)', async () => { + // Bidirectional-consistency case: SDK explicitly says self-hosted but + // a sessionId leaked into the payload. Trust the SDK's declaration. + let prior = process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + delete process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + try { + await percy.start(); + await expectAsync(postMaestro({ name: SS_NAME, sessionId: SID, runtime: 'selfhosted' })) + .toBeRejectedWithError(/Missing required env: PERCY_MAESTRO_SCREENSHOT_DIR/); + } finally { + if (prior === undefined) delete process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + else process.env.PERCY_MAESTRO_SCREENSHOT_DIR = prior; + } + }); + + it('runtime=browserstack + sessionId present → BS branch finds file under /tmp//...', async () => { + await percy.start(); + const res = await postMaestro({ name: SS_NAME, sessionId: SID, runtime: 'browserstack' }); + expect(res.status).toBe(200); + }); + + it('runtime=browserstack + sessionId ABSENT → BS branch (trust runtime), but file-find fails → 404', async () => { + // Bidirectional-consistency case: SDK explicitly says browserstack + // but sessionId is missing. BS branch runs, can't locate the file + // because `/tmp//...` doesn't resolve. + await percy.start(); + await expectAsync(postMaestro({ name: SS_NAME, runtime: 'browserstack' })) + .toBeRejectedWithError(/Screenshot not found|sessionId/i); + }); + + it('runtime=unknown-value + no sessionId → falls back to sessionId-absent → self-hosted branch', async () => { + // Defensive: unknown runtime values are NOT rejected. The relay falls + // back to sessionId-absence detection, so this lands in self-hosted. + let prior = process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + delete process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + try { + await percy.start(); + await expectAsync(postMaestro({ name: SS_NAME, runtime: 'maestro-cloud' })) + .toBeRejectedWithError(/Missing required env: PERCY_MAESTRO_SCREENSHOT_DIR/); + } finally { + if (prior === undefined) delete process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + else process.env.PERCY_MAESTRO_SCREENSHOT_DIR = prior; + } + }); + + it('runtime=unknown-value + sessionId present → falls back to sessionId-present → BS branch', async () => { + await percy.start(); + const res = await postMaestro({ name: SS_NAME, sessionId: SID, runtime: 'maestro-cloud' }); + expect(res.status).toBe(200); + }); + + it('no runtime + sessionId present → BS branch (backward-compat with pre-runtime SDKs)', async () => { + await percy.start(); + const res = await postMaestro({ name: SS_NAME, sessionId: SID }); + expect(res.status).toBe(200); + }); + + it('no runtime + no sessionId → self-hosted branch (backward-compat with pre-runtime SDKs)', async () => { + let prior = process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + delete process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + try { + await percy.start(); + await expectAsync(postMaestro({ name: SS_NAME })) + .toBeRejectedWithError(/Missing required env: PERCY_MAESTRO_SCREENSHOT_DIR/); + } finally { + if (prior === undefined) delete process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + else process.env.PERCY_MAESTRO_SCREENSHOT_DIR = prior; + } + }); + }); + it('rejects non-object element selector with 400', async () => { await percy.start(); await expectAsync(postMaestro({