diff --git a/.semgrepignore b/.semgrepignore index 854bd41f2..fc58ffd32 100644 --- a/.semgrepignore +++ b/.semgrepignore @@ -20,16 +20,24 @@ packages/core/src/lock.js # every join. Suppress at the file level with this rationale. packages/core/src/archive.js -# api.js path-traversal guards live at the top of the /percy/maestro-screenshot -# route handler: `name` and `sessionId` are both validated against -# /^[a-zA-Z0-9_-]+$/ (SAFE_ID, line ~322) BEFORE any path.join() runs. -# Traversal sequences (`..`, `/`, `\0`) are rejected with 400 there. The -# fallback walker is also depth-capped at 15 levels. semgrep's -# path-join-resolve-traversal rule cannot follow the regex validation -# chain across the function body, so it flags the joins on lines 445 -# and 462. Inline `// nosemgrep` directives (preceding and same-line) -# were not honored by the semgrep CI version in use — suppress at the -# file level with this rationale. +# maestro-screenshot-file.js owns the screenshot-location path joins. The +# handleMaestroScreenshot handler validates `name`/`sessionId` against +# /^[a-zA-Z0-9_-]+$/ (SAFE_ID) and rejects traversal sequences with 400 BEFORE +# calling locateScreenshot(); the fallback walker (manualScreenshotWalk) is +# additionally depth-capped at 15 levels, and the realpath + scopeRoot prefix +# check enforces containment. semgrep's path-join-resolve-traversal rule cannot +# follow that validation chain across the module boundary, so it flags the joins +# inside manualScreenshotWalk. Inline `// nosemgrep` is not honored by the CI +# semgrep version — suppress at the file level with this rationale. +packages/core/src/maestro-screenshot-file.js + +# api.js: createStaticServer builds an automatic sitemap by joining the +# operator-provided `baseUrl` config with locally-globbed `*.html` filenames via +# `path.posix.join('/', baseUrl, …)` (sitemap-URL construction, not a filesystem +# read of external request input). semgrep's path-join-resolve-traversal rule +# flags the join regardless of the trusted sources. Inline `// nosemgrep` is not +# honored by the CI semgrep version — suppress at the file level with this +# rationale. packages/core/src/api.js # Test files load fixtures from hardcoded subpaths under diff --git a/packages/cli-app/src/exec.js b/packages/cli-app/src/exec.js index 10b54de21..1cc8261d2 100644 --- a/packages/cli-app/src/exec.js +++ b/packages/cli-app/src/exec.js @@ -1,8 +1,6 @@ -import fs from 'fs'; -import os from 'os'; -import path from 'path'; import command from '@percy/cli-command'; import * as ExecPlugin from '@percy/cli-exec'; +import { maybeInjectMaestroServer, maybeInjectScreenshotDir, maybeInjectDriverHostPort } from './maestro-inject.js'; export const ping = ExecPlugin.ping; export const stop = ExecPlugin.stop; @@ -18,168 +16,6 @@ export const start = command('start', { } }, ExecPlugin.start.callback); -// Locate the `test` subcommand in argv. Maestro accepts global parent -// flags before the subcommand, e.g.: -// maestro test flow.yaml -// maestro --udid test flow.yaml -// maestro --platform=android test flow.yaml -// maestro --verbose --no-ansi test flow.yaml -// We must find `test` by scanning rather than checking args[1] === 'test', -// or our injects silently no-op when the customer pins a device. -// -// Returns the index of the `test` literal, or -1 if not present. Skips -// over the value of known value-taking parent flags so a literal `test` -// supplied as a flag value (e.g. `--udid test`) isn't mistaken for the -// subcommand. Equals-form (`--flag=value`) doesn't need a skip — the -// value is part of the same argv token. -const MAESTRO_PARENT_VALUE_FLAGS = new Set([ - '--udid', '--device', '-p', '--platform', - '--host', '--port', '--driver-host-port' -]); -function findTestSubcommandIdx(args) { - for (let i = 1; i < args.length; i++) { - const tok = args[i]; - if (tok === 'test') return i; - if (typeof tok === 'string' && MAESTRO_PARENT_VALUE_FLAGS.has(tok)) { - i++; // skip the value of this flag - } - } - return -1; -} - -function hasExistingPercyServerFlag(args, testIdx) { - for (let i = testIdx + 1; i < args.length - 1; i++) { - if (args[i] === '-e' && /^PERCY_SERVER=/.test(args[i + 1])) return true; - } - return false; -} - -// Returns the customer-supplied value of `--test-output-dir` (whether the -// space-form `--test-output-dir ` or the equals-form -// `--test-output-dir=` — both are valid picocli syntax), or null if -// absent. Returning the value (not just an index) lets the screenshot-dir -// helper align PERCY_MAESTRO_SCREENSHOT_DIR with the customer's value in -// either form without re-deriving it. -// -// An empty value (space-form value === '' or equals-form `--test-output-dir=` -// with nothing after the `=`) is treated as ABSENT — returning the empty -// string would tell the caller "customer set this" and bypass the -// auto-resolve fallback, leaving Maestro to default its output location -// while PERCY_MAESTRO_SCREENSHOT_DIR stays unset. That mismatch silently -// produces all-404 snapshots, so fall through to the auto-resolve path -// instead. -const TEST_OUTPUT_DIR_EQ_PREFIX = '--test-output-dir='; -function findTestOutputDirValue(args, testIdx) { - for (let i = testIdx + 1; i < args.length; i++) { - const tok = args[i]; - if (tok === '--test-output-dir' && i + 1 < args.length) { - const val = args[i + 1]; - if (typeof val === 'string' && val.length > 0) return val; - } else if (typeof tok === 'string' && tok.startsWith(TEST_OUTPUT_DIR_EQ_PREFIX)) { - const val = tok.slice(TEST_OUTPUT_DIR_EQ_PREFIX.length); - if (val.length > 0) return val; - } - } - return null; -} - -// Maestro's GraalJS sandbox does NOT inherit the parent process's env, -// so `PERCY_SERVER_ADDRESS` exported by app:exec is invisible to the -// SDK. When wrapping `maestro test`, surface the CLI address through -// Maestro's only env channel — `-e KEY=VALUE` flags — so the SDK -// healthcheck can find the local CLI without the customer having to -// pair ports manually. No-op when the customer already supplied their -// own `-e PERCY_SERVER=...`. -// -// When percy?.address() is falsy (percy disabled, start failed), emit a -// WARN so the customer is not surprised by a silent zero-snapshot build. -// The customer-override skip case (their own `-e PERCY_SERVER=...` is in -// argv) does NOT warn — that's intentional flow control, not a problem. -export function maybeInjectMaestroServer(ctx, log) { - const args = ctx?.argv; - if (!Array.isArray(args) || args.length < 2) return; - if (path.basename(args[0]) !== 'maestro') return; - const testIdx = findTestSubcommandIdx(args); - if (testIdx < 0) return; - if (hasExistingPercyServerFlag(args, testIdx)) return; - const addr = ctx.percy?.address(); - if (!addr) { - log?.warn( - 'app:exec did not start the Percy CLI server (percy disabled or start ' + - 'failed); -e PERCY_SERVER not injected into maestro test. Snapshots will ' + - 'NOT be uploaded. Set PERCY_TOKEN and re-run, or check the percy log above.' - ); - return; - } - // Inject after `test` so `-e KEY=VAL` is bound to the `test` subcommand - // (the only Maestro subcommand that accepts `-e`). - args.splice(testIdx + 1, 0, '-e', `PERCY_SERVER=${addr}`); -} - -// Auto-resolve the Maestro screenshot output directory so customers don't -// have to pair `export PERCY_MAESTRO_SCREENSHOT_DIR=...` with a matching -// `--test-output-dir ` in their maestro test command. -// -// Resolution order: -// 1. Customer set BOTH process.env.PERCY_MAESTRO_SCREENSHOT_DIR and -// --test-output-dir in argv → trust them, do nothing. -// 2. Customer set PERCY_MAESTRO_SCREENSHOT_DIR only → use it, inject -// `--test-output-dir ` into argv. -// 3. Customer set --test-output-dir only → use that value, mirror it -// into process.env.PERCY_MAESTRO_SCREENSHOT_DIR (so the SDK + -// CLI relay see the same path). -// 4. Neither set → pick `${process.cwd()}/.percy-out`. On any mkdir -// failure (read-only CWD, EACCES, EEXIST as a file), fall back to -// `${os.tmpdir()}/percy-maestro-` with a WARN log. -// -// The env-var update and argv splice always keep both sources of truth -// (SDK reads env var; Maestro reads the flag) aligned to the same path. -export function maybeInjectScreenshotDir(ctx, log) { - const args = ctx?.argv; - if (!Array.isArray(args) || args.length < 2) return; - if (path.basename(args[0]) !== 'maestro') return; - const testIdx = findTestSubcommandIdx(args); - if (testIdx < 0) return; - - const envSet = !!process.env.PERCY_MAESTRO_SCREENSHOT_DIR; - const existingFlagValue = findTestOutputDirValue(args, testIdx); - const flagSet = existingFlagValue !== null; - - // Fully customer-controlled — nothing to do. - if (envSet && flagSet) return; - - let resolved; - if (envSet) { - resolved = process.env.PERCY_MAESTRO_SCREENSHOT_DIR; - } else if (flagSet) { - resolved = existingFlagValue; - } else { - const preferred = path.join(process.cwd(), '.percy-out'); - try { - fs.mkdirSync(preferred, { recursive: true }); - resolved = preferred; - } catch (err) { - const fallback = path.join(os.tmpdir(), `percy-maestro-${process.pid}`); - try { - fs.mkdirSync(fallback, { recursive: true }); - } catch (_) { - // tmpdir mkdir failure is exceedingly rare; fall through and let - // downstream code surface a clearer error than this helper can. - } - resolved = fallback; - log?.warn( - `Could not create ${preferred} (${err.code || err.message}); ` + - `falling back to ${fallback}. Set PERCY_MAESTRO_SCREENSHOT_DIR to ` + - 'pick a specific location.' - ); - } - } - - if (!envSet) process.env.PERCY_MAESTRO_SCREENSHOT_DIR = resolved; - // Inject right after `test` (the subcommand that owns `--test-output-dir`). - if (!flagSet) args.splice(testIdx + 1, 0, '--test-output-dir', resolved); -} - export const exec = command('exec', { description: 'Start and stop Percy around a supplied command for native apps', usage: '[options] -- ', @@ -195,16 +31,21 @@ export const exec = command('exec', { skipDiscovery: true } }, async function*(ctx) { - // The two helpers splice their flag groups at argv index 2 (between `test` - // and the flow file) because `-e` and `--test-output-dir` are - // `test`-subcommand options. Resulting argv for `maestro test flow.yaml`: - // maestro test --test-output-dir -e PERCY_SERVER= flow.yaml - // iOS driver port: not prescribed from this side — the @percy/core relay - // reads `PERCY_IOS_DRIVER_HOST_PORT` (BS-host-injected on production - // hosts) and probes the documented Maestro 2.4.0 single-simulator default - // (`127.0.0.1:7001`) when it isn't set. See `packages/core/src/maestro-hierarchy.js`. + // The helpers splice their flag groups right after `test` because `-e` and + // `--test-output-dir` are `test`-subcommand options; on iOS + Maestro >= 2.6 a + // third adds `--driver-host-port `. Resulting argv for + // `maestro --platform=ios test flow.yaml`: + // maestro --platform=ios test --driver-host-port --test-output-dir -e PERCY_SERVER= flow.yaml + // iOS driver port: on Maestro >= 2.6 (ephemeral driver port) we prescribe a + // free port via `--driver-host-port` and mirror it to PERCY_IOS_DRIVER_HOST_PORT + // so the @percy/core relay reaches `/viewHierarchy` deterministically for + // element regions and device insets. On Maestro <= 2.4 the helper no-ops and + // the relay's `127.0.0.1:7001` probe (the deterministic 2.4.0 default) serves + // those customers. On BrowserStack the port is host-injected and this path + // never runs. See `packages/core/src/maestro-hierarchy.js`. maybeInjectMaestroServer(ctx, ctx.log); maybeInjectScreenshotDir(ctx, ctx.log); + await maybeInjectDriverHostPort(ctx, ctx.log); yield* ExecPlugin.default.callback(ctx); }); diff --git a/packages/cli-app/src/index.js b/packages/cli-app/src/index.js index d1c8d4ffd..6377e032c 100644 --- a/packages/cli-app/src/index.js +++ b/packages/cli-app/src/index.js @@ -1,2 +1,3 @@ export { default, app } from './app.js'; -export { exec, start, stop, ping, maybeInjectMaestroServer, maybeInjectScreenshotDir } from './exec.js'; +export { exec, start, stop, ping } from './exec.js'; +export { maybeInjectMaestroServer, maybeInjectScreenshotDir, maybeInjectDriverHostPort } from './maestro-inject.js'; diff --git a/packages/cli-app/src/maestro-inject.js b/packages/cli-app/src/maestro-inject.js new file mode 100644 index 000000000..190f6f8f3 --- /dev/null +++ b/packages/cli-app/src/maestro-inject.js @@ -0,0 +1,350 @@ +import fs from 'fs'; +import os from 'os'; +import net from 'net'; +import path from 'path'; +import { spawnSync } from 'child_process'; + +// Locate the `test` subcommand in argv. Maestro accepts global parent +// flags before the subcommand, e.g.: +// maestro test flow.yaml +// maestro --udid test flow.yaml +// maestro --platform=android test flow.yaml +// maestro --verbose --no-ansi test flow.yaml +// We must find `test` by scanning rather than checking args[1] === 'test', +// or our injects silently no-op when the customer pins a device. +// +// Returns the index of the `test` literal, or -1 if not present. Skips +// over the value of known value-taking parent flags so a literal `test` +// supplied as a flag value (e.g. `--udid test`) isn't mistaken for the +// subcommand. Equals-form (`--flag=value`) doesn't need a skip — the +// value is part of the same argv token. +const MAESTRO_PARENT_VALUE_FLAGS = new Set([ + '--udid', '--device', '-p', '--platform', + '--host', '--port', '--driver-host-port' +]); +function findTestSubcommandIdx(args) { + for (let i = 1; i < args.length; i++) { + const tok = args[i]; + if (tok === 'test') return i; + if (typeof tok === 'string' && MAESTRO_PARENT_VALUE_FLAGS.has(tok)) { + i++; // skip the value of this flag + } + } + return -1; +} + +function hasExistingPercyServerFlag(args, testIdx) { + for (let i = testIdx + 1; i < args.length - 1; i++) { + if (args[i] === '-e' && /^PERCY_SERVER=/.test(args[i + 1])) return true; + } + return false; +} + +// Returns the customer-supplied value of `--test-output-dir` (whether the +// space-form `--test-output-dir ` or the equals-form +// `--test-output-dir=` — both are valid picocli syntax), or null if +// absent. Returning the value (not just an index) lets the screenshot-dir +// helper align PERCY_MAESTRO_SCREENSHOT_DIR with the customer's value in +// either form without re-deriving it. +// +// An empty value (space-form value === '' or equals-form `--test-output-dir=` +// with nothing after the `=`) is treated as ABSENT — returning the empty +// string would tell the caller "customer set this" and bypass the +// auto-resolve fallback, leaving Maestro to default its output location +// while PERCY_MAESTRO_SCREENSHOT_DIR stays unset. That mismatch silently +// produces all-404 snapshots, so fall through to the auto-resolve path +// instead. +const TEST_OUTPUT_DIR_EQ_PREFIX = '--test-output-dir='; +function findTestOutputDirValue(args, testIdx) { + for (let i = testIdx + 1; i < args.length; i++) { + const tok = args[i]; + if (tok === '--test-output-dir' && i + 1 < args.length) { + const val = args[i + 1]; + if (typeof val === 'string' && val.length > 0) return val; + } else if (typeof tok === 'string' && tok.startsWith(TEST_OUTPUT_DIR_EQ_PREFIX)) { + const val = tok.slice(TEST_OUTPUT_DIR_EQ_PREFIX.length); + if (val.length > 0) return val; + } + } + return null; +} + +// Maestro's GraalJS sandbox does NOT inherit the parent process's env, +// so `PERCY_SERVER_ADDRESS` exported by app:exec is invisible to the +// SDK. When wrapping `maestro test`, surface the CLI address through +// Maestro's only env channel — `-e KEY=VALUE` flags — so the SDK +// healthcheck can find the local CLI without the customer having to +// pair ports manually. No-op when the customer already supplied their +// own `-e PERCY_SERVER=...`. +// +// When percy?.address() is falsy (percy disabled, start failed), emit a +// WARN so the customer is not surprised by a silent zero-snapshot build. +// The customer-override skip case (their own `-e PERCY_SERVER=...` is in +// argv) does NOT warn — that's intentional flow control, not a problem. +export function maybeInjectMaestroServer(ctx, log) { + const args = ctx?.argv; + if (!Array.isArray(args) || args.length < 2) return; + if (path.basename(args[0]) !== 'maestro') return; + const testIdx = findTestSubcommandIdx(args); + if (testIdx < 0) return; + if (hasExistingPercyServerFlag(args, testIdx)) return; + const addr = ctx.percy?.address(); + if (!addr) { + log?.warn( + 'app:exec did not start the Percy CLI server (percy disabled or start ' + + 'failed); -e PERCY_SERVER not injected into maestro test. Snapshots will ' + + 'NOT be uploaded. Set PERCY_TOKEN and re-run, or check the percy log above.' + ); + return; + } + // Inject after `test` so `-e KEY=VAL` is bound to the `test` subcommand + // (the only Maestro subcommand that accepts `-e`). + args.splice(testIdx + 1, 0, '-e', `PERCY_SERVER=${addr}`); +} + +// Auto-resolve the Maestro screenshot output directory so customers don't +// have to pair `export PERCY_MAESTRO_SCREENSHOT_DIR=...` with a matching +// `--test-output-dir ` in their maestro test command. +// +// Resolution order: +// 1. Customer set BOTH process.env.PERCY_MAESTRO_SCREENSHOT_DIR and +// --test-output-dir in argv → trust them, do nothing. +// 2. Customer set PERCY_MAESTRO_SCREENSHOT_DIR only → use it, inject +// `--test-output-dir ` into argv. +// 3. Customer set --test-output-dir only → use that value, mirror it +// into process.env.PERCY_MAESTRO_SCREENSHOT_DIR (so the SDK + +// CLI relay see the same path). +// 4. Neither set → pick `${process.cwd()}/.percy-out`. On any mkdir +// failure (read-only CWD, EACCES, EEXIST as a file), fall back to +// `${os.tmpdir()}/percy-maestro-` with a WARN log. +// +// The env-var update and argv splice always keep both sources of truth +// (SDK reads env var; Maestro reads the flag) aligned to the same path. +export function maybeInjectScreenshotDir(ctx, log) { + const args = ctx?.argv; + if (!Array.isArray(args) || args.length < 2) return; + if (path.basename(args[0]) !== 'maestro') return; + const testIdx = findTestSubcommandIdx(args); + if (testIdx < 0) return; + + const envSet = !!process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + const existingFlagValue = findTestOutputDirValue(args, testIdx); + const flagSet = existingFlagValue !== null; + + // Fully customer-controlled — nothing to do. + if (envSet && flagSet) return; + + let resolved; + if (envSet) { + resolved = process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + } else if (flagSet) { + resolved = existingFlagValue; + } else { + const preferred = path.join(process.cwd(), '.percy-out'); + try { + fs.mkdirSync(preferred, { recursive: true }); + resolved = preferred; + } catch (err) { + const fallback = path.join(os.tmpdir(), `percy-maestro-${process.pid}`); + try { + fs.mkdirSync(fallback, { recursive: true }); + } catch (_) { + // tmpdir mkdir failure is exceedingly rare; fall through and let + // downstream code surface a clearer error than this helper can. + } + resolved = fallback; + log?.warn( + `Could not create ${preferred} (${err.code || err.message}); ` + + `falling back to ${fallback}. Set PERCY_MAESTRO_SCREENSHOT_DIR to ` + + 'pick a specific location.' + ); + } + } + + if (!envSet) process.env.PERCY_MAESTRO_SCREENSHOT_DIR = resolved; + // Inject right after `test` (the subcommand that owns `--test-output-dir`). + if (!flagSet) args.splice(testIdx + 1, 0, '--test-output-dir', resolved); +} + +// ─── iOS driver-host-port prescription ────────────────────────────────────── +// +// Maestro 2.6.0 changed the iOS driver to bind an OS-assigned ephemeral port +// (`ServerSocket(0)`); Maestro ≤ 2.4.0 bound the deterministic `7001`. The +// @percy/core relay resolves the iOS `/viewHierarchy` port by reading +// `PERCY_IOS_DRIVER_HOST_PORT` first, then probing `127.0.0.1:7001`. On Maestro +// 2.6+ the probe finds nothing (ephemeral), so self-hosted iOS element regions +// (and device insets, whose relay path is env-only with no probe) degrade. +// +// Fix: pick a free port, pin Maestro's iOS driver to it via `--driver-host-port +// `, and mirror it to `process.env.PERCY_IOS_DRIVER_HOST_PORT` so the +// relay (same Node process — env IS inherited there) targets it. This only fires +// on Maestro ≥ 2.6.0: the flag does not exist on 2.4.0 and passing it there is a +// fatal `Unknown option` (the reason commit 13616a87 was reverted). On ≤ 2.4.0 +// the helper no-ops and the relay's 7001 probe serves those customers. +// +// Gated conservatively to iOS (`--platform=ios`/`-p ios`), non-sharded runs (a +// single pinned port collides across shards), and no-ops on any version-detection +// failure — degrading to the existing 7001-probe + warn-skip path. +const DRIVER_HOST_PORT_EQ_PREFIX = '--driver-host-port='; +const DRIVER_HOST_PORT_MIN_MAJOR = 2; +const DRIVER_HOST_PORT_MIN_MINOR = 6; +// Sharding flags — a single prescribed port cannot serve parallel shards +// (Maestro 2.6+ throws CliError on the second shard's bind). +const MAESTRO_SHARDING_FLAGS = new Set(['--shards', '--shard-split', '--shard-all', '-s']); +const MAESTRO_SHARDING_EQ_PREFIXES = ['--shards=', '--shard-split=', '--shard-all=']; + +/* istanbul ignore next — production default; tests inject deps.execMaestro. */ +function defaultExecMaestro() { + return spawnSync('maestro', ['--version'], { encoding: 'utf8' }); +} + +/* istanbul ignore next — production default; tests inject deps.pickFreePort. */ +function defaultPickFreePort() { + return new Promise((resolve, reject) => { + const srv = net.createServer(); + srv.on('error', reject); + srv.listen(0, '127.0.0.1', () => { + const { port } = srv.address(); + srv.close(() => resolve(port)); + }); + }); +} + +// Explicit iOS platform signal — `--platform=ios`, `--platform ios`, or `-p ios`. +// Case-sensitive (Maestro is). Scans the whole argv so a parent-position flag +// (the usual place) or an after-`test` one is both detected. +function isIosPlatform(args) { + for (let i = 1; i < args.length; i++) { + const tok = args[i]; + if ((tok === '--platform' || tok === '-p') && args[i + 1] === 'ios') return true; + if (tok === '--platform=ios') return true; + } + return false; +} + +// True when argv requests sharded/parallel execution (space- or equals-form). +// argv tokens are always strings (process argv / spliced literals). +function hasShardingFlag(args) { + for (let i = 1; i < args.length; i++) { + const tok = args[i]; + if (MAESTRO_SHARDING_FLAGS.has(tok)) return true; + if (MAESTRO_SHARDING_EQ_PREFIXES.some(p => tok.startsWith(p))) return true; + } + return false; +} + +// Returns the customer-supplied `--driver-host-port` value (space- or +// equals-form), or null if absent. An empty value is treated as ABSENT (mirrors +// findTestOutputDirValue) so `--driver-host-port=` falls through to our own pick. +function findDriverHostPortValue(args) { + for (let i = 1; i < args.length; i++) { + const tok = args[i]; + if (tok === '--driver-host-port' && i + 1 < args.length) { + const val = args[i + 1]; + if (typeof val === 'string' && val.length > 0) return val; + } else if (typeof tok === 'string' && tok.startsWith(DRIVER_HOST_PORT_EQ_PREFIX)) { + const val = tok.slice(DRIVER_HOST_PORT_EQ_PREFIX.length); + if (val.length > 0) return val; + } + } + return null; +} + +// Validate a port string as an integer in 1..65535 (mirrors @percy/core's +// parseIosDriverHostPort). Returns the number, or null when absent/invalid so a +// stale/garbage PERCY_IOS_DRIVER_HOST_PORT export is treated as unset. +function validDriverHostPort(raw) { + if (typeof raw !== 'string') return null; + const trimmed = raw.trim(); + if (!/^\d+$/.test(trimmed)) return null; + const n = Number(trimmed); + return (n >= 1 && n <= 65535) ? n : null; +} + +// Detect the installed Maestro MAJOR.MINOR via `maestro --version`. Returns +// { major, minor } or null on any spawn/parse failure (helper then no-ops). +// The flag is `hidden` in Maestro's CLI, so version — not `--help` — is the +// only reliable capability signal. +function detectMaestroVersion(execMaestro, log) { + let result; + try { + result = execMaestro(); + } catch (err) { + log?.debug(`maestro --version failed (${err.code || err.message}); skipping --driver-host-port injection`); + return null; + } + if (!result || result.error || result.status !== 0) { + log?.debug('maestro --version returned no usable output; skipping --driver-host-port injection'); + return null; + } + const out = `${result.stdout || ''}${result.stderr || ''}`; + const m = out.match(/(\d+)\.(\d+)/); + if (!m) { + log?.debug('could not parse maestro version; skipping --driver-host-port injection'); + return null; + } + return { major: Number(m[1]), minor: Number(m[2]) }; +} + +// Prescribe the iOS Maestro driver port on Maestro ≥ 2.6.0 so the @percy/core +// relay can reach `/viewHierarchy` deterministically (regions + insets) without +// customer-side `PERCY_IOS_DRIVER_HOST_PORT` configuration. No-ops on every path +// that would be unsafe or unnecessary (non-maestro, non-`test`, sharded, +// non-iOS, customer already pinned, Maestro < 2.6, detection failure). Async +// because picking a free port (`net`) is inherently async. +export async function maybeInjectDriverHostPort(ctx, log, deps = {}) { + const args = ctx?.argv; + if (!Array.isArray(args) || args.length < 2) return; + if (path.basename(args[0]) !== 'maestro') return; + const testIdx = findTestSubcommandIdx(args); + if (testIdx < 0) return; + + // Sharded runs can't share one pinned port — leave them to the relay probe. + if (hasShardingFlag(args)) return; + // Conservative: the flag is an iOS-driver concern; require an explicit signal. + if (!isIosPlatform(args)) return; + // Customer already pinned the port — stay fully passive (argv + env untouched). + if (findDriverHostPortValue(args) !== null) return; + + // Capability gate: the flag only exists on Maestro ≥ 2.6.0. Injecting it on + // 2.4.0 is a fatal `Unknown option`, so detect first and bail otherwise. + /* istanbul ignore next -- production DI default; unit tests always inject deps.execMaestro. */ + const execMaestro = deps.execMaestro || defaultExecMaestro; + const version = detectMaestroVersion(execMaestro, log); + if (!version) return; + if (version.major < DRIVER_HOST_PORT_MIN_MAJOR || + (version.major === DRIVER_HOST_PORT_MIN_MAJOR && version.minor < DRIVER_HOST_PORT_MIN_MINOR)) { + log?.debug( + `Maestro ${version.major}.${version.minor} predates --driver-host-port ` + + '(>= 2.6.0); relying on the 127.0.0.1:7001 relay probe' + ); + return; + } + + // A valid customer-exported port wins its value; otherwise pick a free one. + // Either way we splice the argv flag (so Maestro binds it) and mirror env (so + // the relay targets it). Stale/invalid env values fall through to a fresh pick. + /* istanbul ignore next -- production DI default; unit tests always inject deps.pickFreePort. */ + const pickFreePort = deps.pickFreePort || defaultPickFreePort; + const envPort = validDriverHostPort(process.env.PERCY_IOS_DRIVER_HOST_PORT); + let port = envPort; + if (port === null) { + // Picking a free port can reject (OS resource exhaustion on listen(0), etc.). + // Degrade gracefully like every other failure path here — no-op and let the + // relay's 127.0.0.1:7001 probe handle it — rather than crashing app:exec with + // an unattributed rejection AND leaving Maestro 2.6+ with no port at all. + try { + port = await pickFreePort(); + } catch (err) { + log?.warn( + `[percy] could not reserve a free port for --driver-host-port (${err.message}); ` + + 'relying on the 127.0.0.1:7001 relay probe' + ); + return; + } + } + + args.splice(testIdx + 1, 0, '--driver-host-port', String(port)); + process.env.PERCY_IOS_DRIVER_HOST_PORT = String(port); + log?.info(`[percy] iOS Maestro driver port prescribed → ${port} (Maestro ${version.major}.${version.minor})`); +} diff --git a/packages/cli-app/test/exec.test.js b/packages/cli-app/test/exec.test.js index b114676af..98d4c98c9 100644 --- a/packages/cli-app/test/exec.test.js +++ b/packages/cli-app/test/exec.test.js @@ -5,7 +5,7 @@ import { setupTest } from '@percy/cli-command/test/helpers'; import * as ExecPlugin from '@percy/cli-exec'; import { exec, start, stop, ping, - maybeInjectMaestroServer, maybeInjectScreenshotDir + maybeInjectMaestroServer, maybeInjectScreenshotDir, maybeInjectDriverHostPort } from '@percy/cli-app'; describe('percy app:exec', () => { @@ -532,4 +532,349 @@ describe('percy app:exec', () => { ]); }); }); + + describe('maybeInjectDriverHostPort', () => { + let originalEnvValue; + + beforeEach(() => { + originalEnvValue = process.env.PERCY_IOS_DRIVER_HOST_PORT; + delete process.env.PERCY_IOS_DRIVER_HOST_PORT; + }); + + afterEach(() => { + if (originalEnvValue === undefined) { + delete process.env.PERCY_IOS_DRIVER_HOST_PORT; + } else { + process.env.PERCY_IOS_DRIVER_HOST_PORT = originalEnvValue; + } + }); + + function ctxFor(argv) { + return { argv: [...argv] }; + } + + // Fake `maestro --version` result at the requested version. + function versionDeps(version, port = 9555) { + return { + execMaestro: () => ({ stdout: `${version}\n`, stderr: '', status: 0 }), + pickFreePort: () => port + }; + } + + it('injects --driver-host-port after test and mirrors env on Maestro 2.6.1 + --platform=ios', async () => { + const log = { info: jasmine.createSpy('info'), debug: jasmine.createSpy('debug') }; + const ctx = ctxFor(['maestro', '--platform=ios', 'test', 'flow.yaml']); + await maybeInjectDriverHostPort(ctx, log, versionDeps('2.6.1')); + expect(ctx.argv).toEqual([ + 'maestro', '--platform=ios', 'test', '--driver-host-port', '9555', 'flow.yaml' + ]); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBe('9555'); + expect(log.info).toHaveBeenCalledTimes(1); + }); + + it('injects at the 2.6.0 boundary', async () => { + const ctx = ctxFor(['maestro', '--platform=ios', 'test', 'flow.yaml']); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.0')); + expect(ctx.argv).toContain('--driver-host-port'); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBe('9555'); + }); + + it('detects --platform ios space-form', async () => { + const ctx = ctxFor(['maestro', '--platform', 'ios', 'test', 'flow.yaml']); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual([ + 'maestro', '--platform', 'ios', 'test', '--driver-host-port', '9555', 'flow.yaml' + ]); + }); + + it('detects the -p ios alias', async () => { + const ctx = ctxFor(['maestro', '-p', 'ios', 'test', 'flow.yaml']); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toContain('--driver-host-port'); + }); + + it('no-ops for --platform=android', async () => { + const argv = ['maestro', '--platform=android', 'test', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual(argv); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBeUndefined(); + }); + + it('no-ops when no --platform is given (conservative gate)', async () => { + const argv = ['maestro', 'test', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual(argv); + }); + + it('no-ops on Maestro 2.4.0 (flag would be a fatal Unknown option)', async () => { + const log = { debug: jasmine.createSpy('debug') }; + const argv = ['maestro', '--platform=ios', 'test', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, log, versionDeps('2.4.0')); + expect(ctx.argv).toEqual(argv); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBeUndefined(); + expect(log.debug).toHaveBeenCalled(); + }); + + it('no-ops on legacy Maestro 1.40.0', async () => { + const argv = ['maestro', '--platform=ios', 'test', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, null, versionDeps('1.40.0')); + expect(ctx.argv).toEqual(argv); + }); + + it('injects on a future major (Maestro 3.0.0, forward-compat)', async () => { + const ctx = ctxFor(['maestro', '--platform=ios', 'test', 'flow.yaml']); + await maybeInjectDriverHostPort(ctx, null, versionDeps('3.0.0')); + expect(ctx.argv).toContain('--driver-host-port'); + }); + + it('no-ops for --platform android space-form (non-ios)', async () => { + const argv = ['maestro', '--platform', 'android', 'test', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual(argv); + }); + + it('treats an empty space-form --driver-host-port "" as absent and picks fresh', async () => { + const ctx = ctxFor(['maestro', '--platform=ios', 'test', '--driver-host-port', '', 'flow.yaml']); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + // customer's empty flag is left in place; our own pinned flag is spliced after test + expect(ctx.argv).toEqual([ + 'maestro', '--platform=ios', 'test', '--driver-host-port', '9555', '--driver-host-port', '', 'flow.yaml' + ]); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBe('9555'); + }); + + it('treats a trailing --driver-host-port with no value as absent and picks fresh', async () => { + const ctx = ctxFor(['maestro', '--platform=ios', 'test', 'flow.yaml', '--driver-host-port']); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toContain('9555'); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBe('9555'); + }); + + it('no-ops (warn) when pickFreePort rejects, degrading to the relay probe', async () => { + const log = { info: jasmine.createSpy('info'), warn: jasmine.createSpy('warn'), debug: jasmine.createSpy('debug') }; + const argv = ['maestro', '--platform=ios', 'test', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, log, { + execMaestro: () => ({ stdout: '2.6.1', stderr: '', status: 0 }), + pickFreePort: () => Promise.reject(new Error('EMFILE')) + }); + expect(ctx.argv).toEqual(argv); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBeUndefined(); + expect(log.warn).toHaveBeenCalled(); + expect(log.info).not.toHaveBeenCalled(); + }); + + it('no-ops (debug) when execMaestro returns nothing', async () => { + const log = { debug: jasmine.createSpy('debug') }; + const argv = ['maestro', '--platform=ios', 'test', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, log, { + execMaestro: () => undefined, + pickFreePort: () => 9555 + }); + expect(ctx.argv).toEqual(argv); + expect(log.debug).toHaveBeenCalled(); + }); + + it('stays fully passive when the customer pinned --driver-host-port (space-form)', async () => { + const argv = ['maestro', '--platform=ios', 'test', '--driver-host-port', '7777', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual(argv); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBeUndefined(); + }); + + it('stays fully passive when the customer pinned --driver-host-port= (equals-form)', async () => { + const argv = ['maestro', '--platform=ios', 'test', '--driver-host-port=7777', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual(argv); + }); + + it('reuses a valid customer PERCY_IOS_DRIVER_HOST_PORT env value and still splices argv', async () => { + process.env.PERCY_IOS_DRIVER_HOST_PORT = '8888'; + const ctx = ctxFor(['maestro', '--platform=ios', 'test', 'flow.yaml']); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1', 9555)); + expect(ctx.argv).toEqual([ + 'maestro', '--platform=ios', 'test', '--driver-host-port', '8888', 'flow.yaml' + ]); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBe('8888'); + }); + + it('stays passive when BOTH customer argv flag and env are set', async () => { + process.env.PERCY_IOS_DRIVER_HOST_PORT = '8888'; + const argv = ['maestro', '--platform=ios', 'test', '--driver-host-port', '7777', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual(argv); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBe('8888'); + }); + + it('injects past a parent flag (--udid X test --platform=ios)', async () => { + const ctx = ctxFor(['maestro', '--udid', 'X', 'test', '--platform=ios', 'flow.yaml']); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual([ + 'maestro', '--udid', 'X', 'test', '--driver-host-port', '9555', '--platform=ios', 'flow.yaml' + ]); + }); + + it('no-ops (debug) when execMaestro throws ENOENT', async () => { + const log = { debug: jasmine.createSpy('debug') }; + const argv = ['maestro', '--platform=ios', 'test', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, log, { + execMaestro: () => { const e = new Error('ENOENT'); e.code = 'ENOENT'; throw e; }, + pickFreePort: () => 9555 + }); + expect(ctx.argv).toEqual(argv); + expect(log.debug).toHaveBeenCalled(); + }); + + it('no-ops (debug) when execMaestro returns a non-zero status', async () => { + const log = { debug: jasmine.createSpy('debug') }; + const argv = ['maestro', '--platform=ios', 'test', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, log, { + execMaestro: () => ({ stdout: '', stderr: 'boom', status: 1 }), + pickFreePort: () => 9555 + }); + expect(ctx.argv).toEqual(argv); + expect(log.debug).toHaveBeenCalled(); + }); + + it('no-ops (debug) when execMaestro returns an error object (ENOENT, no throw)', async () => { + const log = { debug: jasmine.createSpy('debug') }; + const argv = ['maestro', '--platform=ios', 'test', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, log, { + execMaestro: () => ({ error: new Error('spawn ENOENT'), status: null }), + pickFreePort: () => 9555 + }); + expect(ctx.argv).toEqual(argv); + expect(log.debug).toHaveBeenCalled(); + }); + + it('no-ops (debug) when version output is unparseable', async () => { + const log = { debug: jasmine.createSpy('debug') }; + const argv = ['maestro', '--platform=ios', 'test', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, log, { + execMaestro: () => ({ stdout: 'no version here', stderr: '', status: 0 }), + pickFreePort: () => 9555 + }); + expect(ctx.argv).toEqual(argv); + expect(log.debug).toHaveBeenCalled(); + }); + + it('parses a version emitted on stderr', async () => { + const ctx = ctxFor(['maestro', '--platform=ios', 'test', 'flow.yaml']); + await maybeInjectDriverHostPort(ctx, null, { + execMaestro: () => ({ stdout: '', stderr: '2.6.3', status: 0 }), + pickFreePort: () => 9555 + }); + expect(ctx.argv).toContain('--driver-host-port'); + }); + + it('no-ops for `maestro hierarchy` (not a test command)', async () => { + const argv = ['maestro', 'hierarchy', '--udid', 'X']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual(argv); + }); + + it('no-ops for `npx maestro test` (argv[0] is npx, not maestro)', async () => { + const argv = ['npx', 'maestro', 'test', '--platform=ios', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual(argv); + }); + + it('no-ops when args has fewer than two elements', async () => { + const argv = ['maestro']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual(argv); + }); + + it('matches by basename when the command is an absolute path', async () => { + const ctx = ctxFor(['/Users/foo/.maestro/bin/maestro', '--platform=ios', 'test', 'flow.yaml']); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toContain('--driver-host-port'); + }); + + it('treats an empty equals-form --driver-host-port= as absent and picks fresh', async () => { + const ctx = ctxFor(['maestro', '--platform=ios', 'test', '--driver-host-port=', 'flow.yaml']); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual([ + 'maestro', '--platform=ios', 'test', '--driver-host-port', '9555', '--driver-host-port=', 'flow.yaml' + ]); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBe('9555'); + }); + + it('treats a non-numeric env value as absent and picks fresh', async () => { + process.env.PERCY_IOS_DRIVER_HOST_PORT = 'not-a-port'; + const ctx = ctxFor(['maestro', '--platform=ios', 'test', 'flow.yaml']); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1', 9555)); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBe('9555'); + }); + + it('treats an out-of-range env value as absent and picks fresh', async () => { + process.env.PERCY_IOS_DRIVER_HOST_PORT = '70000'; + const ctx = ctxFor(['maestro', '--platform=ios', 'test', 'flow.yaml']); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1', 9556)); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBe('9556'); + }); + + it('gives concurrent invocations distinct ports', async () => { + // Real concurrent app:exec runs are separate processes with separate + // process.env; clear the env between calls to model that (otherwise the + // first call's env mirror is reused by the second — itself correct, but a + // different property than the picker producing distinct ports). + const picker = jasmine.createSpy('pickFreePort').and.returnValues(9555, 9556); + const deps = { execMaestro: () => ({ stdout: '2.6.1', stderr: '', status: 0 }), pickFreePort: picker }; + const a = ctxFor(['maestro', '--platform=ios', 'test', 'a.yaml']); + const b = ctxFor(['maestro', '--platform=ios', 'test', 'b.yaml']); + await maybeInjectDriverHostPort(a, null, deps); + delete process.env.PERCY_IOS_DRIVER_HOST_PORT; + await maybeInjectDriverHostPort(b, null, deps); + expect(a.argv).toContain('9555'); + expect(b.argv).toContain('9556'); + }); + + it('no-ops on sharded runs (--shards), leaving the port to the relay probe', async () => { + const argv = ['maestro', '--platform=ios', 'test', '--shards', '2', 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual(argv); + expect(process.env.PERCY_IOS_DRIVER_HOST_PORT).toBeUndefined(); + }); + + it('no-ops on sharded runs (equals-form --shards=2 and other shard flags)', async () => { + for (const shardFlag of [['--shards=2'], ['--shard-split', '2'], ['--shard-all'], ['-s', '2']]) { + const argv = ['maestro', '--platform=ios', 'test', ...shardFlag, 'flow.yaml']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual(argv); + } + }); + + it('no-ops for non-maestro commands', async () => { + const argv = ['python', 'test.py']; + const ctx = ctxFor(argv); + await maybeInjectDriverHostPort(ctx, null, versionDeps('2.6.1')); + expect(ctx.argv).toEqual(argv); + }); + + it('tolerates a missing ctx / argv', async () => { + await maybeInjectDriverHostPort(undefined, null, versionDeps('2.6.1')); + await maybeInjectDriverHostPort({}, null, versionDeps('2.6.1')); + // no throw + expect(true).toBe(true); + }); + }); }); diff --git a/packages/core/src/api.js b/packages/core/src/api.js index f6c9bfa25..1384f19da 100644 --- a/packages/core/src/api.js +++ b/packages/core/src/api.js @@ -2,13 +2,12 @@ import fs from 'fs'; import path, { dirname, resolve } from 'path'; import logger from '@percy/logger'; import { normalize } from '@percy/config/utils'; -import { getPackageJSON, Server, percyAutomateRequestHandler, percyBuildEventHandler, computeResponsiveWidths } from './utils.js'; -import { ServerError } from './server.js'; +import { getPackageJSON, Server, percyAutomateRequestHandler, percyBuildEventHandler, computeResponsiveWidths, encodeURLSearchParams } from './utils.js'; import WebdriverUtils from '@percy/webdriver-utils'; import { handleSyncJob } from './snapshot.js'; -import { dump as maestroDump, firstMatch as maestroFirstMatch, SELECTOR_KEYS_WHITELIST, getMaestroHierarchyDrift } from './maestro-hierarchy.js'; -import Busboy from 'busboy'; -import { Readable } from 'stream'; +import { getMaestroHierarchyDrift } from './maestro-hierarchy.js'; +import { handleComparisonUpload } from './comparison-upload.js'; +import { handleMaestroScreenshot } from './maestro-screenshot.js'; // Previously, we used `createRequire(import.meta.url).resolve` to resolve the path to the module. // This approach relied on `createRequire`, which is Node.js-specific and less compatible with modern ESM (ECMAScript Module) standards. // This was leading to hard coded paths when CLI is used as a dependency in another project. @@ -35,13 +34,6 @@ export const getPercyDomPath = (url) => { // Resolved path for PERCY_DOM export const PERCY_DOM = getPercyDomPath(import.meta.url); -// Returns a URL encoded string of nested query params -function encodeURLSearchParams(subj, prefix) { - return typeof subj === 'object' ? Object.entries(subj).map(([key, value]) => ( - encodeURLSearchParams(value, prefix ? `${prefix}[${key}]` : key) - )).join('&') : `${prefix}=${encodeURIComponent(subj)}`; -} - // Walks the config schema and collects dot-paths of any fields marked `httpReadOnly: true` // that are present in `body`. Driving this from the schema means new HTTP-blocked fields // only need a one-line annotation next to their definition — no list to keep in sync here. @@ -91,174 +83,7 @@ function stripBlockedConfigFields(body, log) { return _applyHttpReadOnlyStripping(body, findHttpReadOnlyPaths(body, ROOT_CONFIG_SCHEMA), log); } -// Parse PNG IHDR chunk for the screenshot's actual rendered dimensions. -// Returns { width, height } when the buffer is a valid PNG with non-zero -// dimensions, or null otherwise (non-PNG signature, truncated file, zero -// IHDR values). PNG layout per W3C spec: -// bytes 0..7 PNG signature (89 50 4E 47 0D 0A 1A 0A) -// bytes 8..15 IHDR chunk header (length + type, fixed) -// bytes 16..19 width (big-endian uint32) -// bytes 20..23 height (big-endian uint32) -// No library dependency — pure stdlib Buffer access on the bytes the relay -// has already read. -export function parsePngDimensions(buffer) { - if (!buffer || buffer.length < 24) return null; - if (buffer[0] !== 0x89 || buffer[1] !== 0x50 || buffer[2] !== 0x4E || buffer[3] !== 0x47 || - buffer[4] !== 0x0D || buffer[5] !== 0x0A || buffer[6] !== 0x1A || buffer[7] !== 0x0A) { - return null; - } - const width = buffer.readUInt32BE(16); - const height = buffer.readUInt32BE(20); - if (width <= 0 || height <= 0) return null; - return { width, height }; -} - // Create a Percy CLI API server instance -/* istanbul ignore next — defensive manual directory walker invoked only when - fast-glob import fails (broken install / FS corruption). Unit tests - exercise the primary glob path; integration tests on BS hosts exercise - the walker against real session layouts. Path-traversal sinks inside this - function are suppressed at file level in .semgrepignore with the same - rationale (upstream SAFE_ID validation, depth cap, exact filename match). */ -async function manualScreenshotWalk(platform, sessionId, name) { - const files = []; - try { - if (platform === 'ios') { - const sessionDir = `/tmp/${sessionId}`; - const walk = async (dir, depth) => { - if (depth > 15) return; // sanity cap - let entries; - try { entries = await fs.promises.readdir(dir, { withFileTypes: true }); } catch { return; } - for (const entry of entries) { - const full = path.join(dir, entry.name); - if (entry.isDirectory()) { - await walk(full, depth + 1); - } else if (entry.isFile() && entry.name === `${name}.png` && full.includes('_maestro_debug_')) { - files.push(full); - } - } - }; - await walk(sessionDir, 0); - } else { - const baseDir = `/tmp/${sessionId}_test_suite/logs`; - const logDirs = await fs.promises.readdir(baseDir); - for (const dir of logDirs) { - const screenshotPath = path.join(baseDir, dir, 'screenshots', `${name}.png`); - try { - await fs.promises.access(screenshotPath); - files.push(screenshotPath); - } catch { /* not found, continue */ } - } - } - } catch { /* base dir not found */ } - return files; -} - -/* istanbul ignore next — multipart /percy/comparison/upload handler; - exercises Busboy stream parsing + PNG magic-byte validation + base64 - encoding + percy.upload. Integration-tested via the regression suite - (real multipart POST) rather than the unit suite, which would require - constructing valid multipart bodies. */ -async function handleComparisonUpload(req, res, percy) { - const MAX_FILE_SIZE = 50 * 1024 * 1024; // 50MB - const PNG_MAGIC_BYTES = Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); - - let contentType = req.headers['content-type'] || ''; - if (!contentType.startsWith('multipart/form-data')) { - throw new ServerError(400, 'Content-Type must be multipart/form-data'); - } - - if (!req.body) { - throw new ServerError(400, 'Empty request body'); - } - - let fields = Object.create(null); - let fileBuffer = null; - - await new Promise((resolve, reject) => { - let bb = Busboy({ - headers: req.headers, - limits: { fileSize: MAX_FILE_SIZE } - }); - - bb.on('file', (fieldname, stream, info) => { - let chunks = []; - stream.on('data', (chunk) => chunks.push(chunk)); - stream.on('limit', () => { - reject(new ServerError(413, 'File size exceeds maximum of 50MB')); - }); - stream.on('end', () => { - if (fieldname === 'screenshot') { - fileBuffer = Buffer.concat(chunks); - } - }); - }); - - bb.on('field', (fieldname, value) => { - if (['name', 'tag', 'clientInfo', 'environmentInfo', 'testCase', 'labels'].includes(fieldname)) { - fields[fieldname] = value; - } - }); - - bb.on('close', resolve); - bb.on('error', reject); - - let stream = Readable.from(req.body); - stream.on('error', reject); - stream.pipe(bb); - }); - - if (!fileBuffer) { - throw new ServerError(400, 'Missing required file part: screenshot'); - } - - if (fileBuffer.length < 8 || !fileBuffer.subarray(0, 8).equals(PNG_MAGIC_BYTES)) { - throw new ServerError(400, 'File is not a valid PNG image'); - } - - if (!fields.name) throw new ServerError(400, 'Missing required field: name'); - if (!fields.tag) throw new ServerError(400, 'Missing required field: tag'); - - let tag; - try { - tag = JSON.parse(fields.tag); - } catch { - throw new ServerError(400, 'Invalid JSON in tag field'); - } - - let base64Content = fileBuffer.toString('base64'); - - let payload = { - name: fields.name, - tag, - tiles: [{ - content: base64Content, - statusBarHeight: 0, - navBarHeight: 0, - headerHeight: 0, - footerHeight: 0, - fullscreen: false - }], - clientInfo: fields.clientInfo || '', - environmentInfo: fields.environmentInfo || '' - }; - - if (fields.testCase) payload.testCase = fields.testCase; - if (fields.labels) payload.labels = fields.labels; - - let upload = percy.upload(payload, null, 'app'); - if (req.url.searchParams.has('await')) await upload; - - let link = [ - percy.client.apiUrl, '/comparisons/redirect?', - encodeURLSearchParams(normalize({ - buildId: percy.build?.id, snapshot: { name: payload.name }, tag - }, { snake: true })) - ].join(''); - - return res.json(200, { success: true, link }); -} - export function createPercyServer(percy, port) { let pkg = getPackageJSON(import.meta.url); @@ -423,519 +248,7 @@ export function createPercyServer(percy, port) { // post a comparison via multipart file upload .route('post', '/percy/comparison/upload', /* istanbul ignore next */ (req, res) => handleComparisonUpload(req, res, percy)) // 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 || {}; - - if (!name) throw new ServerError(400, 'Missing required field: name'); - - // Self-hosted vs BrowserStack is signaled by sessionId presence: BS - // host-injection always supplies it; self-hosted runs never do. - let selfHosted = !sessionId; - - // Strict character-class validation — rejects path separators, shell metacharacters, - // NUL, newlines, and anything else that could confuse the glob or the filesystem. - // `name` is load-bearing for the recursive glob — must not be loosened. - const SAFE_ID = /^[a-zA-Z0-9_-]+$/; - if (!SAFE_ID.test(name)) { - throw new ServerError(400, 'Invalid screenshot name'); - } - if (sessionId && !SAFE_ID.test(sessionId)) { - throw new ServerError(400, 'Invalid sessionId'); - } - - // Resolve platform signal: strict whitelist on `platform` when present; default Android when absent. - // Backward compatible with SDK v0.2.0 (no platform field → Android glob). - let platform = 'android'; - if (req.body.platform !== undefined) { - if (typeof req.body.platform !== 'string') { - throw new ServerError(400, 'Invalid platform: must be a string'); - } - let normalized = req.body.platform.toLowerCase(); - if (normalized !== 'ios' && normalized !== 'android') { - throw new ServerError(400, `Invalid platform: must be "ios" or "android", got "${req.body.platform}"`); - } - platform = normalized; - } - - // Optional caller-supplied absolute path. When present, the relay reads - // the file directly and skips the legacy glob — the SDK has already - // chosen the path under the BS session root. Shape errors (non-string, - // non-absolute, too long) are 400. Existence and session-root scoping - // are enforced by the shared realpath + prefix check below, which - // returns 404 — same shape as the glob path. Treat empty string as - // absent so older SDKs that emit the field unconditionally still fall - // through to the glob. - let suppliedFilePath = null; - if (req.body.filePath !== undefined && req.body.filePath !== null && req.body.filePath !== '') { - if (typeof req.body.filePath !== 'string') { - throw new ServerError(400, 'Invalid filePath: must be a string'); - } - if (req.body.filePath.length > 1024) { - throw new ServerError(400, 'Invalid filePath: exceeds maximum length of 1024'); - } - if (!path.isAbsolute(req.body.filePath)) { - throw new ServerError(400, 'Invalid filePath: must be an absolute path'); - } - suppliedFilePath = req.body.filePath; - } - - // Resolve the file-find scope root. On BrowserStack (sessionId present), - // the root is the BS host's /tmp/{sessionId}{_test_suite} convention. - // Self-hosted (sessionId absent) requires PERCY_MAESTRO_SCREENSHOT_DIR - // (read from process.env, never the request body) to be an absolute, - // existing directory — typically the customer's - // `maestro test --test-output-dir ` path. The realpath + prefix - // check below enforces the security invariant at whichever root applies; - // the boundary is relocated, not removed. - let scopeRoot; - if (selfHosted) { - // Reject filePath outright in self-hosted mode. The SDK never emits - // it (it sends a relative SCREENSHOT_NAME); honoring an absolute - // filePath against a caller-influenceable root would re-open - // arbitrary in-root reads. - if (suppliedFilePath) { - throw new ServerError(400, 'filePath is not accepted in self-hosted mode (omit it; PERCY_MAESTRO_SCREENSHOT_DIR + relative SCREENSHOT_NAME is the supported path)'); - } - let dir = process.env.PERCY_MAESTRO_SCREENSHOT_DIR; - if (!dir) { - throw new ServerError(400, 'Missing required env: PERCY_MAESTRO_SCREENSHOT_DIR (set it to your `maestro test --test-output-dir` path)'); - } - if (!path.isAbsolute(dir)) { - throw new ServerError(400, 'PERCY_MAESTRO_SCREENSHOT_DIR must be an absolute path'); - } - // UX guard ONLY: surface an actionable 400 ("dir not found") instead - // of the opaque 404 the realpath+prefix containment check below would - // emit for a missing dir. There is a small TOCTOU window between this - // stat and the realpath at line 647 — that window is acceptable here - // because realpath (not stat) is the security invariant: even if the - // dir is replaced with a symlink in between, realpath resolves the - // target and the sep-prefix check rejects anything outside scopeRoot. - let stat; - try { stat = await fs.promises.stat(dir); } catch { stat = null; } - if (!stat || !stat.isDirectory()) { - throw new ServerError(400, `PERCY_MAESTRO_SCREENSHOT_DIR is not an existing directory: ${dir}`); - } - scopeRoot = dir; - } else { - scopeRoot = platform === 'ios' - ? `/tmp/${sessionId}` - : `/tmp/${sessionId}_test_suite`; - } - - // Validate regions input shape early (before file I/O and ADB work) so - // malformed requests don't consume resolver/relay work. Three parallel - // input arrays share the same per-item shape; algorithm semantics differ - // per array (regions only — ignoreRegions/considerRegions are implicit). - const REGION_INPUT_FIELDS = ['regions', 'ignoreRegions', 'considerRegions']; - for (let fieldName of REGION_INPUT_FIELDS) { - let input = req.body[fieldName]; - if (input === undefined) continue; - if (!Array.isArray(input)) { - throw new ServerError(400, `${fieldName} must be an array`); - } - if (input.length > 50) { - throw new ServerError(400, `${fieldName} exceeds maximum of 50`); - } - for (let [idx, region] of input.entries()) { - if (region && region.element !== undefined) { - if (typeof region.element !== 'object' || region.element === null || Array.isArray(region.element)) { - throw new ServerError(400, `${fieldName}[${idx}].element must be an object`); - } - let keys = Object.keys(region.element); - if (keys.length !== 1) { - throw new ServerError(400, `${fieldName}[${idx}].element must have exactly one selector key`); - } - let [key] = keys; - if (!SELECTOR_KEYS_WHITELIST.includes(key)) { - throw new ServerError(400, `${fieldName}[${idx}].element: unsupported selector key "${key}" (allowed: ${SELECTOR_KEYS_WHITELIST.join(', ')})`); - } - let value = region.element[key]; - if (typeof value !== 'string' || value.length === 0) { - throw new ServerError(400, `${fieldName}[${idx}].element.${key} must be a non-empty string`); - } - if (value.length > 512) { - throw new ServerError(400, `${fieldName}[${idx}].element.${key} exceeds maximum length of 512`); - } - } - } - } - - // Locate the screenshot on disk. Two paths converge on `chosenFile`: - // 1. `filePath` supplied (new SDK ≥ v0.4 — the SDK chose an absolute - // path under the BS session root and saved Maestro's PNG there). - // 2. Legacy glob (older SDKs — file lives at the BS-infra-chosen - // SCREENSHOTS_DIR layout). Either way, the shared realpath + - // session-root prefix check below enforces the security invariant. - let chosenFile; - if (suppliedFilePath) { - chosenFile = suppliedFilePath; - } else { - // Legacy glob. Pattern depends on platform: - // Android (BrowserStack mobile): /tmp/{sid}_test_suite/logs/*/screenshots/{name}.png - // iOS (BrowserStack realmobile): /tmp/{sid}//**/{name}.png - // realmobile builds SCREENSHOTS_DIR with literal slashes from the flow-path - // concatenation, causing Maestro to mkdir a deeply nested structure under the - // {device}_maestro_debug_ root. The `**` recursive match handles any depth. - // Exact {name}.png match at the leaf filters out Maestro's emoji-prefixed - // debug frames (e.g., `screenshot-❌--(flow).png`). - let searchPattern; - if (selfHosted) { - // Self-hosted: recursive glob under the customer's --test-output-dir - // (PERCY_MAESTRO_SCREENSHOT_DIR). Recursive depth handles arbitrary - // Maestro layouts; `name` is SAFE_ID-validated above so it cannot - // contain separators or traversal characters. - // - // fast-glob requires forward-slashes in patterns on every platform - // (per its docs: "Always use forward-slashes in glob expressions"). - // On Windows the scopeRoot from path.resolve contains backslashes, - // so we normalize before embedding into the pattern. Production- - // code Windows portability — verified by the CI Windows runner. - const globRoot = scopeRoot.replace(/\\/g, '/'); - searchPattern = `${globRoot}/**/${name}.png`; - } else { - searchPattern = platform === 'ios' - ? `/tmp/${sessionId}/*_maestro_debug_*/**/${name}.png` - : `/tmp/${sessionId}_test_suite/logs/*/screenshots/${name}.png`; - } - - let files; - try { - let { default: glob } = await import('fast-glob'); - // Self-hosted needs `dot: true` because Maestro's default output - // directory is `.maestro/` — a dot-prefixed entry that fast-glob - // hides by default. BS layouts have no dot-prefixed segments, so - // omitting the option there keeps the byte-identical behavior. - files = await glob(searchPattern, selfHosted ? { dot: true } : undefined); - } catch { - // Fast-glob import / glob call failed — fall back to manual walker. - // See manualScreenshotWalk() at file top for the rationale + the - // file-level .semgrepignore covering path-traversal sinks inside. - // Self-hosted has no walker fallback (no fixed-layout convention) — - // empty files → 404 with the actionable PERCY_MAESTRO_SCREENSHOT_DIR - // guidance above. - /* istanbul ignore next — only fires when fast-glob import throws - (broken install / FS corruption); integration-test territory. */ - files = selfHosted ? [] : await manualScreenshotWalk(platform, sessionId, name); - } - - if (!files || files.length === 0) { - throw new ServerError(404, `Screenshot not found: ${name}.png (searched ${searchPattern})`); - } - - // If multiple files match (iOS — same name reused across flows), pick the most recently modified - // for determinism. The else branch only fires when a snapshot name - // is reused across two flows in the same session; the realmobile - // layout normally writes one file per snapshot per session, so the - // multi-match path is exercised by integration tests on BS hosts - // rather than the unit suite. - /* istanbul ignore else */ - if (files.length === 1) { - chosenFile = files[0]; - } else { - let mtimes = await Promise.all(files.map(async f => { - try { return { f, mtime: (await fs.promises.stat(f)).mtimeMs }; } catch { return { f, mtime: 0 }; } - })); - mtimes.sort((a, b) => b.mtime - a.mtime); - chosenFile = mtimes[0].f; - } - } - - // Canonicalize and confirm the resolved path still lives under scopeRoot. - // Defeats symlink swaps where the root points elsewhere. Both ends are - // realpath'd because /tmp is a symlink on macOS (where iOS hosts run). - // The trailing `/` on the prefix is load-bearing — it prevents - // sibling-prefix bypass (e.g. /x/.maestro vs /x/.maestro-secrets). - // - // Normalize both sides to forward-slashes before the prefix check so - // the same code works on Windows (real-fs returns backslashes) AND on - // POSIX (no-op) AND on memfs in tests (POSIX-style virtual paths - // regardless of host OS). - let realPath, realPrefix; - try { - realPath = await fs.promises.realpath(chosenFile); - realPrefix = await fs.promises.realpath(scopeRoot); - } catch { - throw new ServerError(404, `Screenshot not found: ${name}.png (path resolution failed)`); - } - const realPathFwd = realPath.replace(/\\/g, '/'); - const realPrefixFwd = realPrefix.replace(/\\/g, '/'); - if (!realPathFwd.startsWith(`${realPrefixFwd}/`)) { - throw new ServerError(404, `Screenshot not found: ${name}.png (resolved outside ${selfHosted ? 'PERCY_MAESTRO_SCREENSHOT_DIR' : 'session dir'})`); - } - - // Read and base64-encode the screenshot - let fileContent = await fs.promises.readFile(realPath); - let base64Content = fileContent.toString('base64'); - - // Parse the PNG header for actual rendered dimensions. The PNG bytes - // ARE the source of truth — what Percy stores and compares against. - // Fills tag.width/height when the customer didn't supply them (or - // supplied invalid values); customer-supplied values continue to win - // for backward compat with any flow that pins a specific tag dim. - let pngDims = parsePngDimensions(fileContent); - - // Build tag from optional request body fields - let tag = req.body.tag || { name: 'Unknown Device', osName: 'Android' }; - /* istanbul ignore if — fallback when tag.name is missing; tests always - pass a complete tag object. */ - if (!tag.name) tag.name = 'Unknown Device'; - if (pngDims) { - if (typeof tag.width !== 'number' || tag.width <= 0 || isNaN(tag.width)) { - tag.width = pngDims.width; - } - if (typeof tag.height !== 'number' || tag.height <= 0 || isNaN(tag.height)) { - tag.height = pngDims.height; - } - } - - // Construct comparison payload with tile metadata from request - let payload = { - name, - tag, - tiles: [{ - content: base64Content, - statusBarHeight: req.body.statusBarHeight || 0, - navBarHeight: req.body.navBarHeight || 0, - headerHeight: 0, - footerHeight: 0, - fullscreen: req.body.fullscreen || false - }], - clientInfo: req.body.clientInfo || 'percy-maestro/0.1.0', - environmentInfo: req.body.environmentInfo || 'percy-maestro' - }; - - if (req.body.testCase) payload.testCase = req.body.testCase; - if (req.body.labels) payload.labels = req.body.labels; - if (req.body.thTestCaseExecutionId) payload.thTestCaseExecutionId = req.body.thTestCaseExecutionId; - - // ─────────────────────────────────────────────────────────────────── - // REGIONS — end-to-end architecture - // ─────────────────────────────────────────────────────────────────── - // - // Regions tell Percy's diff engine which parts of a mobile screenshot - // to ignore / consider / layout-compare. Two ways to specify one: - // - // 1. Coordinate region — caller already knows the pixel rectangle. - // Shape: { top, left, right, bottom }. Forwarded as-is after - // transform to `{x, y, width, height}` boundingBox. - // - // 2. Element region — caller knows a selector (`resource-id`, `text`, - // `content-desc`, `class`, `id`) but not the on-screen bounds. - // Resolved at relay-time against the live device's view hierarchy. - // - // ── Data flow (element region case) ──────────────────────────────── - // - // SDK (percy-screenshot.js) - // │ POST /percy/maestro-screenshot - // │ { name, sessionId, platform, regions:[{element:{...}}], ... } - // ▼ - // Relay (this handler) - // │ validate selector shape (SELECTOR_KEYS_WHITELIST) - // │ maestroDump({ platform, sessionId, grpcClientCache }) ← lazy + memoized per request - // │ │ - // │ ├─ Android cascade (maestro-hierarchy.js) - // │ │ gRPC primary → maestro-CLI → adb uiautomator - // │ │ Three-class taxonomy: schema-class (drift bit, no - // │ │ fallback) / channel-broken (evict cache, fall back) / - // │ │ contention-class (keep cache, skip CLI → adb). - // │ │ - // │ └─ iOS cascade - // │ HTTP primary (Maestro XCTestRunner /viewHierarchy) - // │ → maestro-CLI shell-out. AUT-root detection skips - // │ SpringBoard frames. - // │ - // │ firstMatch(nodes, selector) → bbox or null (warn-skip). - // │ payload.regions[i].elementSelector.boundingBox = bbox - // ▼ - // Percy backend — compares masked regions across builds. - // - // ── Observability ────────────────────────────────────────────────── - // - // /percy/healthcheck exposes maestroHierarchyDrift per platform: - // { lastFailureClass, fallbackCount, succeededVia, code?, reason?, firstSeenAt? } - // Every primary→fallback transition also emits one info-level line: - // [percy] hierarchy: failed (: ) → falling back to - // - // ── Failure shape ────────────────────────────────────────────────── - // - // Element regions degrade gracefully: resolver failure → warn-skip - // those regions only; the snapshot itself still uploads. Coordinate - // regions don't depend on the resolver and always pass through. - // - // ─────────────────────────────────────────────────────────────────── - // Shared resolver state across regions/ignoreRegions/considerRegions — - // one hierarchy dump per request, one warn-once skip notice. - let cachedDump = null; - let elementSkipWarned = false; - const totalElementRegionCount = REGION_INPUT_FIELDS.reduce((sum, f) => { - let arr = req.body[f]; - return sum + (Array.isArray(arr) ? arr.filter(r => r && r.element).length : 0); - }, 0); - - // Resolve one region input to {x, y, width, height}, or null when the - // region is invalid or the resolver couldn't match it. Mutates the - // shared cachedDump / warn-flag state above. - async function resolveBbox(region) { - if (region.top != null && region.bottom != null && region.left != null && region.right != null) { - return { - x: region.left, - y: region.top, - width: region.right - region.left, - height: region.bottom - region.top - }; - } - /* istanbul ignore else — region.element false branch falls through - to the istanbul-ignored "Invalid region format" warn below. */ - if (region.element) { - /* istanbul ignore else — cachedDump === null only on first - element-region per request; subsequent regions hit the cache. */ - if (cachedDump === null) { - // Thread the per-Percy gRPC client cache so the Android gRPC - // primary path can reuse channels across snapshots in the same - // session (D9 of 2026-05-07-002 plan). iOS path ignores it - // (the iOS resolver reads PERCY_IOS_DRIVER_HOST_PORT directly; - // no per-session port cache needed since the port is prescribed - // upstream by `@percy/cli-app`'s `maybeInjectDriverHostPort`). - cachedDump = await maestroDump({ - platform, - sessionId, - grpcClientCache: percy.grpcClientCache - }); - } - /* istanbul ignore else — branch where dump resolves to hierarchy is - happy-path element-region territory, integration-tested only. */ - if (cachedDump.kind !== 'hierarchy') { - /* istanbul ignore else — elementSkipWarned latches after first - warn; second+ iterations take the no-op branch. */ - if (!elementSkipWarned) { - percy.log.warn( - `Element-region resolver ${cachedDump.kind} (${cachedDump.reason}) — skipping ${totalElementRegionCount} element regions` - ); - elementSkipWarned = true; - } - return null; - } - /* istanbul ignore next */ - let bbox = maestroFirstMatch(cachedDump.nodes, region.element); - /* istanbul ignore next */ - if (!bbox) { - percy.log.warn(`Element region not found: ${JSON.stringify(region.element)} — skipping`); - return null; - } - /* istanbul ignore next — element-region happy path requires a - non-stub maestroDump returning hierarchy nodes; unit tests run - with stubbed resolver (env-missing), happy path covered by the - cross-platform-parity integration harness against fixture data. */ - return bbox; - } - /* istanbul ignore next */ - percy.log.warn('Invalid region format, skipping'); - /* istanbul ignore next — region shape is validated upstream by the - SDK before posting; this is a defensive catch-all for regions that - lack both coordinate fields AND an element selector. */ - return null; - } - - // regions[]: comparison-shape items with algorithm. Default algorithm is - // 'ignore' (back-compat with SDK ≤ 0.3). - if (Array.isArray(req.body.regions)) { - let resolvedRegions = []; - for (let region of req.body.regions) { - let bbox = await resolveBbox(region); - if (!bbox) continue; - let resolved = { - elementSelector: { boundingBox: bbox }, - algorithm: region.algorithm || 'ignore' - }; - /* istanbul ignore if — region.configuration optional field; only - passed when SDK opts in to per-region config overrides. */ - if (region.configuration) resolved.configuration = region.configuration; - /* istanbul ignore if — region.padding optional field. */ - if (region.padding) resolved.padding = region.padding; - /* istanbul ignore if — region.assertion optional field. */ - if (region.assertion) resolved.assertion = region.assertion; - resolvedRegions.push(resolved); - } - /* istanbul ignore else — empty resolvedRegions branch only fires when - ALL regions failed to resolve; happy path resolves at least one. */ - if (resolvedRegions.length > 0) payload.regions = resolvedRegions; - } - - // ignoreRegions[] and considerRegions[]: parallel top-level payload - // fields. Each item is shaped per regionsSchema (config.js:792) — - // { coOrdinates: {top, left, bottom, right} } with an optional selector - // hint preserved when the caller supplied an element selector. - const REGION_OUTPUT_MAP = { - ignoreRegions: { payloadKey: 'ignoredElementsData', innerKey: 'ignoreElementsData' }, - considerRegions: { payloadKey: 'consideredElementsData', innerKey: 'considerElementsData' } - }; - for (let [inputField, { payloadKey, innerKey }] of Object.entries(REGION_OUTPUT_MAP)) { - let input = req.body[inputField]; - if (!Array.isArray(input)) continue; - let resolved = []; - for (let region of input) { - let bbox = await resolveBbox(region); - /* istanbul ignore if — null bbox skip in ignoreRegions/considerRegions - loop; tests cover the happy path where every region resolves. */ - if (!bbox) continue; - let item = { - coOrdinates: { - top: bbox.y, - left: bbox.x, - bottom: bbox.y + bbox.height, - right: bbox.x + bbox.width - } - }; - /* istanbul ignore if — element selector echo on resolved region; - only fires when resolveBbox returned a bbox for an element region, - which itself is integration-test territory (see resolveBbox - above for the resolver-mock rationale). */ - if (region.element) { - let [key] = Object.keys(region.element); - item.selector = `${key}=${region.element[key]}`; - } - resolved.push(item); - } - /* istanbul ignore else — empty resolved branch only fires when ALL - regions in this category failed to resolve; happy path resolves - at least one. */ - if (resolved.length > 0) payload[payloadKey] = { [innerKey]: resolved }; - } - - // Upload via percy — sync or fire-and-forget - if (req.body.sync === true) payload.sync = true; - - let data; - if (percy.syncMode(payload)) { - // See the /percy/comparison route: percy.upload() is the Promise-wrapped method; - // calling it drives the generator and the sync queue resolves/rejects the callback. - // The .catch(reject) surfaces generator errors that bypass that callback. - const snapshotPromise = new Promise((resolve, reject) => { - percy.upload(payload, { resolve, reject }, 'app').catch(reject); - }); - data = await handleSyncJob(snapshotPromise, percy, 'comparison'); - return res.json(200, { success: true, data }); - } - - let upload = percy.upload(payload, null, 'app'); - /* istanbul ignore if — ?await=true URL flag triggers fire-and-wait; - tests cover both syncMode and fire-and-forget but not the explicit - ?await query-param variant. */ - if (req.url.searchParams.has('await')) await upload; - - // Generate redirect link - let link = [ - percy.client.apiUrl, '/comparisons/redirect?', - encodeURLSearchParams(normalize({ - buildId: percy.build?.id, - snapshot: { name }, - tag - }, { snake: true })) - ].join(''); - - return res.json(200, { success: true, link }); - }) + .route('post', '/percy/maestro-screenshot', (req, res) => handleMaestroScreenshot(req, res, percy)) // flushes one or more snapshots from the internal queue .route('post', '/percy/flush', async (req, res) => res.json(200, { success: await percy.flush(req.body).then(() => true) diff --git a/packages/core/src/comparison-upload.js b/packages/core/src/comparison-upload.js new file mode 100644 index 000000000..847580a72 --- /dev/null +++ b/packages/core/src/comparison-upload.js @@ -0,0 +1,110 @@ +import { normalize } from '@percy/config/utils'; +import { ServerError } from './server.js'; +import { encodeURLSearchParams } from './utils.js'; +import Busboy from 'busboy'; +import { Readable } from 'stream'; + +/* istanbul ignore next — multipart /percy/comparison/upload handler; + exercises Busboy stream parsing + PNG magic-byte validation + base64 + encoding + percy.upload. Integration-tested via the regression suite + (real multipart POST) rather than the unit suite, which would require + constructing valid multipart bodies. */ +export async function handleComparisonUpload(req, res, percy) { + const MAX_FILE_SIZE = 50 * 1024 * 1024; // 50MB + const PNG_MAGIC_BYTES = Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); + + let contentType = req.headers['content-type'] || ''; + if (!contentType.startsWith('multipart/form-data')) { + throw new ServerError(400, 'Content-Type must be multipart/form-data'); + } + + if (!req.body) { + throw new ServerError(400, 'Empty request body'); + } + + let fields = Object.create(null); + let fileBuffer = null; + + await new Promise((resolve, reject) => { + let bb = Busboy({ + headers: req.headers, + limits: { fileSize: MAX_FILE_SIZE } + }); + + bb.on('file', (fieldname, stream, info) => { + let chunks = []; + stream.on('data', (chunk) => chunks.push(chunk)); + stream.on('limit', () => { + reject(new ServerError(413, 'File size exceeds maximum of 50MB')); + }); + stream.on('end', () => { + if (fieldname === 'screenshot') { + fileBuffer = Buffer.concat(chunks); + } + }); + }); + + bb.on('field', (fieldname, value) => { + if (['name', 'tag', 'clientInfo', 'environmentInfo', 'testCase', 'labels'].includes(fieldname)) { + fields[fieldname] = value; + } + }); + + bb.on('close', resolve); + bb.on('error', reject); + + let stream = Readable.from(req.body); + stream.on('error', reject); + stream.pipe(bb); + }); + + if (!fileBuffer) { + throw new ServerError(400, 'Missing required file part: screenshot'); + } + + if (fileBuffer.length < 8 || !fileBuffer.subarray(0, 8).equals(PNG_MAGIC_BYTES)) { + throw new ServerError(400, 'File is not a valid PNG image'); + } + + if (!fields.name) throw new ServerError(400, 'Missing required field: name'); + if (!fields.tag) throw new ServerError(400, 'Missing required field: tag'); + + let tag; + try { + tag = JSON.parse(fields.tag); + } catch { + throw new ServerError(400, 'Invalid JSON in tag field'); + } + + let base64Content = fileBuffer.toString('base64'); + + let payload = { + name: fields.name, + tag, + tiles: [{ + content: base64Content, + statusBarHeight: 0, + navBarHeight: 0, + headerHeight: 0, + footerHeight: 0, + fullscreen: false + }], + clientInfo: fields.clientInfo || '', + environmentInfo: fields.environmentInfo || '' + }; + + if (fields.testCase) payload.testCase = fields.testCase; + if (fields.labels) payload.labels = fields.labels; + + let upload = percy.upload(payload, null, 'app'); + if (req.url.searchParams.has('await')) await upload; + + let link = [ + percy.client.apiUrl, '/comparisons/redirect?', + encodeURLSearchParams(normalize({ + buildId: percy.build?.id, snapshot: { name: payload.name }, tag + }, { snake: true })) + ].join(''); + + return res.json(200, { success: true, link }); +} diff --git a/packages/core/src/maestro-hierarchy.js b/packages/core/src/maestro-hierarchy.js index aec35f696..9ba081d5a 100644 --- a/packages/core/src/maestro-hierarchy.js +++ b/packages/core/src/maestro-hierarchy.js @@ -114,6 +114,25 @@ const IOS_HTTP_CIRCUIT_BREAKER_MS = 5000; // HTTP response cap before parse — sized for WebView-heavy iOS apps. const IOS_HTTP_RESPONSE_MAX_BYTES = 20 * 1024 * 1024; +// Device system-bar inset derivation tunables. +// +// iOS: the `/viewHierarchy` AXElement tree exposes the status bar as a node +// with `elementType === 26` (XCUIElementTypeStatusBar — a stable XCUITest +// enum constant). Its `frame.Height` is in logical POINTS; the comparison +// tile expects PIXELS, so we scale by the device scale factor derived +// empirically as PNG-pixel-height ÷ AUT-root-point-height (avoids the +// Plus-class `nativeScale ≠ scale` foot-gun). Apple scale factors are 1/2/3; +// we snap the ratio to the nearest integer and reject anything implausible +// (a wrong root-frame height would otherwise yield a bogus inset). +const IOS_STATUS_BAR_ELEMENT_TYPE = 26; +const DEVICE_SCALE_MIN = 1; +const DEVICE_SCALE_MAX = 3; +// Max distance the raw PNG/point ratio may sit from an integer before we treat +// the root-frame height as unreliable and fall back (e.g. a non-full-screen +// AUT frame). 0.15 comfortably admits real rounding (2532/844 = 3.000) while +// rejecting a half-screen root (e.g. 2532/667 = 3.79 → 0.21 off). +const DEVICE_SCALE_TOLERANCE = 0.15; + // Android gRPC transport tunables. Symmetric with iOS HTTP (D11): same // healthy-deadline + circuit-breaker pair. gRPC's `deadline` option is // client-library-enforced, not kernel-enforced — the outer Promise.race is @@ -959,6 +978,33 @@ function findAxAutRoot(axElement) { return null; } +// Walk the FULL AXElement tree (not just the AUT subtree) for the status-bar +// element (`elementType === IOS_STATUS_BAR_ELEMENT_TYPE`) and return its +// `frame.Height` in points, or null when absent. The status bar is a sibling +// of the AUT app node (cli-2.0.7 wraps as `[appHierarchy, statusBarsContainer]`), +// so callers must pass the raw `axElement` root, not `findAxAutRoot(...)`. +function findStatusBarFrameHeight(node) { + /* istanbul ignore if — defensive guard; deriveIosInsets passes a parsed + object and recursion only descends into well-formed array children. A + malformed child instead surfaces via deriveDeviceInsets' catch → null. */ + if (!node || typeof node !== 'object') return null; + if (node.elementType === IOS_STATUS_BAR_ELEMENT_TYPE) { + // `|| null` collapses a missing/zero/non-numeric height to null so the + // caller's single `== null` check covers every malformed-frame case. + /* istanbul ignore next — frame optional-chain guards a malformed status-bar + frame; real /viewHierarchy responses always carry a positive frame.Height. */ + return node.frame?.Height || null; + } + const children = node.children; + if (Array.isArray(children)) { + for (const child of children) { + const found = findStatusBarFrameHeight(child); + if (found != null) return found; + } + } + return null; +} + // Adapter: walk an AXElement subtree (HTTP /viewHierarchy path) and emit nodes // in the canonical shape that firstMatch consumes for Android. Specifically: // { 'resource-id': identifier, id: identifier, bounds: '[X,Y][X+W,Y+H]' } @@ -1156,6 +1202,89 @@ async function runIosHttpDump({ port, sessionId, httpRequest }) { return { kind: 'hierarchy', nodes }; } +// Derive the iOS status-bar inset (in pixels) from a fresh `/viewHierarchy` +// fetch. Returns `{ statusBarHeight, navBarHeight: 0 }` on success, or null on +// any failure (transport error, non-JSON/missing root, no AUT root, no status +// bar element, implausible scale). navBarHeight is always 0 on iOS — the home +// indicator is static and unmeasured, matching the rest of the Percy SDK fleet. +// +// This is a separate, lighter parse than runIosHttpDump: that path flattens the +// AUT subtree for element-region matching and DISCARDS the status-bar sibling, +// so the status-bar frame is not reachable through dump(). Here we retain the +// raw response, read the AUT root frame height (points) for the scale factor, +// and the status-bar element frame height (points), then convert to pixels. +async function deriveIosInsets({ port, pngDims, httpRequest, sessionId }) { + /* istanbul ignore next — production-only default; unit suite injects a stub. */ + httpRequest = httpRequest || defaultHttpRequest; + // Need PNG pixel height to derive the points→pixels scale. parsePngDimensions + // only ever yields null or a fully-valid {width>0, height>0}, so a single + // truthiness check suffices; a degenerate height is additionally caught by + // the scale sanity bounds below. + if (!pngDims) return null; + + const requestBody = JSON.stringify({ appIds: [], excludeKeyboardElements: false }); + let response; + try { + // Best-effort, cached, fallback-safe: a single request with the transport's + // own timeout is sufficient (no outer circuit-breaker race needed — that's + // defense-in-depth the resolver cascade owns). + response = await httpRequest({ + host: '127.0.0.1', + port, + method: 'POST', + path: '/viewHierarchy', + headers: { + 'content-type': 'application/json', + 'content-length': Buffer.byteLength(requestBody) + }, + body: requestBody, + timeoutMs: IOS_HTTP_HEALTHY_DEADLINE_MS + }); + } catch { + // Transport failure — caller falls back to the SDK default. + return null; + } + + /* istanbul ignore if — defensive; httpRequest resolves to a response object + or the race rejects (caught above). */ + if (!response) return null; + if (response.statusCode !== 200) return null; + + let parsed; + try { + parsed = JSON.parse(response.body); + } catch { + // Non-JSON body, or a non-string body that JSON.parse rejects. + return null; + } + + // Root point-height for scale = the AUT app frame (full-screen on iOS; the + // app draws under the status bar). SpringBoard-only / malformed responses → + // no AUT → null. (findAxAutRoot guards a null/undefined argument.) + const aut = findAxAutRoot(parsed && parsed.axElement); + /* istanbul ignore next — frame optional-chain guards a malformed AUT frame; + real AUT nodes always carry a positive frame.Height. */ + const rootPointHeight = aut?.frame?.Height; + if (!rootPointHeight) return null; + + // Empirical scale: PNG pixel height ÷ AUT root point height, snapped to an + // integer and sanity-checked. Guards the Plus-class nativeScale≠scale gap and + // a non-full-screen root frame. + const ratio = pngDims.height / rootPointHeight; + const scale = Math.round(ratio); + if (scale < DEVICE_SCALE_MIN || scale > DEVICE_SCALE_MAX) return null; + if (Math.abs(ratio - scale) > DEVICE_SCALE_TOLERANCE) return null; + + const statusBarPoints = findStatusBarFrameHeight(parsed.axElement); + if (statusBarPoints == null) return null; + + /* istanbul ignore next — sid log tag ternary; relay always passes a sessionId. */ + const sidTag = sessionId ? `sid=${String(sessionId).slice(0, 8)}…` : 'sid=none'; + const statusBarHeight = Math.round(statusBarPoints * scale); + log.debug(`deriveIosInsets ok ${sidTag} statusBar=${statusBarHeight}px (${statusBarPoints}pt × ${scale})`); + return { statusBarHeight, navBarHeight: 0 }; +} + // iOS maestro-CLI fallback path. Spawns // `maestro --udid --driver-host-port hierarchy` and parses // stdout (Maestro's normalized TreeNode shape, identical to Android). @@ -1250,6 +1379,78 @@ async function runAdbFallback(serial, execAdb) { return result; } +// Parse the first `mStableInsets=Rect(left, top - right, bottom)` from +// `dumpsys window` output. Android's Rect.toString() prints +// `Rect(L, T - R, B)`, so top = status-bar inset, bottom = navigation-bar +// inset (both in pixels — same space as the screenshot). Returns +// `{ statusBarHeight, navBarHeight }` or null when the line is absent. +// Validated against real-device `dumpsys window` output during BS validation; +// gesture-nav and 3-button-nav both surface here, differing only in the +// bottom value. +function parseStableInsets(stdout) { + // execAdb always yields a string stdout (''); the regex returns null on no + // match, so empty input needs no separate guard. + const m = /mStableInsets=Rect\(\s*\d+,\s*(\d+)\s*-\s*\d+,\s*(\d+)\s*\)/.exec(stdout); + if (!m) return null; + const statusBarHeight = Number(m[1]); + const navBarHeight = Number(m[2]); + /* istanbul ignore if — defensive NaN guard; the regex only matches digit + runs, so Number() always parses. */ + if (!Number.isFinite(statusBarHeight) || !Number.isFinite(navBarHeight)) return null; + return { statusBarHeight, navBarHeight }; +} + +// Derive Android status + navigation bar insets (in pixels) via adb. System +// bars are not present in the uiautomator hierarchy dump, so this is a distinct +// `dumpsys window` read. Reuses the resolver's serial-resolution + execAdb +// path (ANDROID_SERIAL on BS multi-device hosts, else `adb devices`). Returns +// `{ statusBarHeight, navBarHeight }` or null on any failure (no/ambiguous +// device, adb error, unparseable output) — caller falls back to SDK defaults. +async function deriveAndroidInsets({ execAdb, getEnv }) { + /* istanbul ignore next — production-only defaults; unit suite injects stubs. */ + execAdb = execAdb || defaultExecAdb; + /* istanbul ignore next */ + getEnv = getEnv || defaultGetEnv; + + const { serial, classification } = await resolveSerial({ execAdb, getEnv }); + if (classification) return null; + + const result = await execAdb(['-s', serial, 'shell', 'dumpsys', 'window']); + if (classifyAdbFailure(result)) return null; + /* istanbul ignore next — `?? 1` fallback branch; spawn helpers always set an + exitCode. Non-zero exit with no recognized failure → treat as unparseable. */ + if ((result.exitCode ?? 1) !== 0) return null; + + return parseStableInsets(result.stdout); +} + +// Derive exact device system-bar insets for the comparison tile, dispatching by +// platform. Returns `{ statusBarHeight, navBarHeight }` (pixels) or null on any +// failure. Never throws — the relay treats null as "use the SDK default". iOS +// navBarHeight is always 0 (static home indicator, fleet-consistent); the iOS +// path additionally needs the PNG pixel height for the points→pixels scale. +export async function deriveDeviceInsets(options) { + /* istanbul ignore next — options-omitted default; callers always pass an object. */ + options = options || {}; + let { platform, sessionId, pngDims, execAdb, httpRequest, getEnv } = options; + /* istanbul ignore next — defaults applied only when caller omits them; tests + inject every dependency, production binds them at runtime. */ + getEnv = getEnv || defaultGetEnv; + + try { + if (platform === 'ios') { + const driverHostPort = parseIosDriverHostPort(getEnv('PERCY_IOS_DRIVER_HOST_PORT')); + if (driverHostPort === null) return null; + return await deriveIosInsets({ port: driverHostPort, pngDims, httpRequest, sessionId }); + } + return await deriveAndroidInsets({ execAdb, getEnv }); + } catch { + // Defensive: derive paths return null on their own failures; this only + // fires if a transport (e.g. getEnv) throws unexpectedly. + return null; + } +} + export async function dump(options) { /* istanbul ignore next — options-omitted default; callers always pass an object (tests inject every dependency; production code binds them). */ diff --git a/packages/core/src/maestro-regions.js b/packages/core/src/maestro-regions.js new file mode 100644 index 000000000..4a4c8b596 --- /dev/null +++ b/packages/core/src/maestro-regions.js @@ -0,0 +1,245 @@ +import { ServerError } from './server.js'; +import { dump as maestroDump, firstMatch as maestroFirstMatch, SELECTOR_KEYS_WHITELIST } from './maestro-hierarchy.js'; + +// Three parallel region input arrays share the same per-item shape; algorithm +// semantics differ per array (regions only — ignoreRegions/considerRegions are +// implicit). +const REGION_INPUT_FIELDS = ['regions', 'ignoreRegions', 'considerRegions']; + +// Validate regions input shape early (before file I/O and ADB work) so +// malformed requests don't consume resolver/relay work. Throws ServerError(400) +// on the first shape violation. +export function validateRegionInputs(body) { + for (let fieldName of REGION_INPUT_FIELDS) { + let input = body[fieldName]; + if (input === undefined) continue; + if (!Array.isArray(input)) { + throw new ServerError(400, `${fieldName} must be an array`); + } + if (input.length > 50) { + throw new ServerError(400, `${fieldName} exceeds maximum of 50`); + } + for (let [idx, region] of input.entries()) { + if (region && region.element !== undefined) { + if (typeof region.element !== 'object' || region.element === null || Array.isArray(region.element)) { + throw new ServerError(400, `${fieldName}[${idx}].element must be an object`); + } + let keys = Object.keys(region.element); + if (keys.length !== 1) { + throw new ServerError(400, `${fieldName}[${idx}].element must have exactly one selector key`); + } + let [key] = keys; + if (!SELECTOR_KEYS_WHITELIST.includes(key)) { + throw new ServerError(400, `${fieldName}[${idx}].element: unsupported selector key "${key}" (allowed: ${SELECTOR_KEYS_WHITELIST.join(', ')})`); + } + let value = region.element[key]; + if (typeof value !== 'string' || value.length === 0) { + throw new ServerError(400, `${fieldName}[${idx}].element.${key} must be a non-empty string`); + } + if (value.length > 512) { + throw new ServerError(400, `${fieldName}[${idx}].element.${key} exceeds maximum length of 512`); + } + } + } + } +} + +// ─────────────────────────────────────────────────────────────────── +// REGIONS — end-to-end architecture +// ─────────────────────────────────────────────────────────────────── +// +// Regions tell Percy's diff engine which parts of a mobile screenshot +// to ignore / consider / layout-compare. Two ways to specify one: +// +// 1. Coordinate region — caller already knows the pixel rectangle. +// Shape: { top, left, right, bottom }. Forwarded as-is after +// transform to `{x, y, width, height}` boundingBox. +// +// 2. Element region — caller knows a selector (`resource-id`, `text`, +// `content-desc`, `class`, `id`) but not the on-screen bounds. +// Resolved at relay-time against the live device's view hierarchy. +// +// ── Data flow (element region case) ──────────────────────────────── +// +// SDK (percy-screenshot.js) +// │ POST /percy/maestro-screenshot +// │ { name, sessionId, platform, regions:[{element:{...}}], ... } +// ▼ +// Relay (maestro-screenshot.js → resolveRegions) +// │ validate selector shape (SELECTOR_KEYS_WHITELIST) +// │ maestroDump({ platform, sessionId, grpcClientCache }) ← lazy + memoized per request +// │ │ +// │ ├─ Android cascade (maestro-hierarchy.js) +// │ │ gRPC primary → maestro-CLI → adb uiautomator +// │ │ Three-class taxonomy: schema-class (drift bit, no +// │ │ fallback) / channel-broken (evict cache, fall back) / +// │ │ contention-class (keep cache, skip CLI → adb). +// │ │ +// │ └─ iOS cascade +// │ HTTP primary (Maestro XCTestRunner /viewHierarchy) +// │ → maestro-CLI shell-out. AUT-root detection skips +// │ SpringBoard frames. +// │ +// │ firstMatch(nodes, selector) → bbox or null (warn-skip). +// │ payload.regions[i].elementSelector.boundingBox = bbox +// ▼ +// Percy backend — compares masked regions across builds. +// +// ── Observability ────────────────────────────────────────────────── +// +// /percy/healthcheck exposes maestroHierarchyDrift per platform: +// { lastFailureClass, fallbackCount, succeededVia, code?, reason?, firstSeenAt? } +// Every primary→fallback transition also emits one info-level line: +// [percy] hierarchy: failed (: ) → falling back to +// +// ── Failure shape ────────────────────────────────────────────────── +// +// Element regions degrade gracefully: resolver failure → warn-skip +// those regions only; the snapshot itself still uploads. Coordinate +// regions don't depend on the resolver and always pass through. +// +// ─────────────────────────────────────────────────────────────────── +// Resolve all region inputs to comparison-payload fragments. Returns an object +// with only the populated keys among { regions, ignoredElementsData, +// consideredElementsData }; merge it into the comparison payload. The hierarchy +// dump and the warn-once flag are request-scoped (call-local) — one dump per +// request, one warn-once skip notice. +export async function resolveRegions({ body, platform, sessionId, percy }) { + let out = {}; + + let cachedDump = null; + let elementSkipWarned = false; + const totalElementRegionCount = REGION_INPUT_FIELDS.reduce((sum, f) => { + let arr = body[f]; + return sum + (Array.isArray(arr) ? arr.filter(r => r && r.element).length : 0); + }, 0); + + // Resolve one region input to {x, y, width, height}, or null when the + // region is invalid or the resolver couldn't match it. Mutates the + // shared cachedDump / warn-flag state above. + async function resolveBbox(region) { + if (region.top != null && region.bottom != null && region.left != null && region.right != null) { + return { + x: region.left, + y: region.top, + width: region.right - region.left, + height: region.bottom - region.top + }; + } + /* istanbul ignore else — region.element false branch falls through + to the istanbul-ignored "Invalid region format" warn below. */ + if (region.element) { + /* istanbul ignore else — cachedDump === null only on first + element-region per request; subsequent regions hit the cache. */ + if (cachedDump === null) { + // Thread the per-Percy gRPC client cache so the Android gRPC + // primary path can reuse channels across snapshots in the same + // session (D9 of 2026-05-07-002 plan). iOS path ignores it. + cachedDump = await maestroDump({ + platform, + sessionId, + grpcClientCache: percy.grpcClientCache + }); + } + /* istanbul ignore else — branch where dump resolves to hierarchy is + happy-path element-region territory, integration-tested only. */ + if (cachedDump.kind !== 'hierarchy') { + /* istanbul ignore else — elementSkipWarned latches after first + warn; second+ iterations take the no-op branch. */ + if (!elementSkipWarned) { + percy.log.warn( + `Element-region resolver ${cachedDump.kind} (${cachedDump.reason}) — skipping ${totalElementRegionCount} element regions` + ); + elementSkipWarned = true; + } + return null; + } + /* istanbul ignore next */ + let bbox = maestroFirstMatch(cachedDump.nodes, region.element); + /* istanbul ignore next */ + if (!bbox) { + percy.log.warn(`Element region not found: ${JSON.stringify(region.element)} — skipping`); + return null; + } + /* istanbul ignore next — element-region happy path requires a + non-stub maestroDump returning hierarchy nodes; unit tests run + with stubbed resolver (env-missing), happy path covered by the + cross-platform-parity integration harness against fixture data. */ + return bbox; + } + /* istanbul ignore next */ + percy.log.warn('Invalid region format, skipping'); + /* istanbul ignore next — region shape is validated upstream by the + SDK before posting; this is a defensive catch-all for regions that + lack both coordinate fields AND an element selector. */ + return null; + } + + // regions[]: comparison-shape items with algorithm. Default algorithm is + // 'ignore' (back-compat with SDK ≤ 0.3). + if (Array.isArray(body.regions)) { + let resolvedRegions = []; + for (let region of body.regions) { + let bbox = await resolveBbox(region); + if (!bbox) continue; + let resolved = { + elementSelector: { boundingBox: bbox }, + algorithm: region.algorithm || 'ignore' + }; + /* istanbul ignore if — region.configuration optional field; only + passed when SDK opts in to per-region config overrides. */ + if (region.configuration) resolved.configuration = region.configuration; + /* istanbul ignore if — region.padding optional field. */ + if (region.padding) resolved.padding = region.padding; + /* istanbul ignore if — region.assertion optional field. */ + if (region.assertion) resolved.assertion = region.assertion; + resolvedRegions.push(resolved); + } + /* istanbul ignore else — empty resolvedRegions branch only fires when + ALL regions failed to resolve; happy path resolves at least one. */ + if (resolvedRegions.length > 0) out.regions = resolvedRegions; + } + + // ignoreRegions[] and considerRegions[]: parallel top-level payload + // fields. Each item is shaped per regionsSchema (config.js:792) — + // { coOrdinates: {top, left, bottom, right} } with an optional selector + // hint preserved when the caller supplied an element selector. + const REGION_OUTPUT_MAP = { + ignoreRegions: { payloadKey: 'ignoredElementsData', innerKey: 'ignoreElementsData' }, + considerRegions: { payloadKey: 'consideredElementsData', innerKey: 'considerElementsData' } + }; + for (let [inputField, { payloadKey, innerKey }] of Object.entries(REGION_OUTPUT_MAP)) { + let input = body[inputField]; + if (!Array.isArray(input)) continue; + let resolved = []; + for (let region of input) { + let bbox = await resolveBbox(region); + /* istanbul ignore if — null bbox skip in ignoreRegions/considerRegions + loop; tests cover the happy path where every region resolves. */ + if (!bbox) continue; + let item = { + coOrdinates: { + top: bbox.y, + left: bbox.x, + bottom: bbox.y + bbox.height, + right: bbox.x + bbox.width + } + }; + /* istanbul ignore if — element selector echo on resolved region; + only fires when resolveBbox returned a bbox for an element region, + which itself is integration-test territory (see resolveBbox + above for the resolver-mock rationale). */ + if (region.element) { + let [key] = Object.keys(region.element); + item.selector = `${key}=${region.element[key]}`; + } + resolved.push(item); + } + /* istanbul ignore else — empty resolved branch only fires when ALL + regions in this category failed to resolve; happy path resolves + at least one. */ + if (resolved.length > 0) out[payloadKey] = { [innerKey]: resolved }; + } + + return out; +} diff --git a/packages/core/src/maestro-screenshot-file.js b/packages/core/src/maestro-screenshot-file.js new file mode 100644 index 000000000..9ae8c134a --- /dev/null +++ b/packages/core/src/maestro-screenshot-file.js @@ -0,0 +1,144 @@ +import fs from 'fs'; +import path from 'path'; +import { ServerError } from './server.js'; + +/* istanbul ignore next — defensive manual directory walker invoked only when + fast-glob import fails (broken install / FS corruption). Unit tests + exercise the primary glob path; integration tests on BS hosts exercise + the walker against real session layouts. Path-traversal sinks inside this + function are suppressed at file level in .semgrepignore with the same + rationale (upstream SAFE_ID validation, depth cap, exact filename match). */ +async function manualScreenshotWalk(platform, sessionId, name) { + const files = []; + try { + if (platform === 'ios') { + const sessionDir = `/tmp/${sessionId}`; + const walk = async (dir, depth) => { + if (depth > 15) return; // sanity cap + let entries; + try { entries = await fs.promises.readdir(dir, { withFileTypes: true }); } catch { return; } + for (const entry of entries) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + await walk(full, depth + 1); + } else if (entry.isFile() && entry.name === `${name}.png` && full.includes('_maestro_debug_')) { + files.push(full); + } + } + }; + await walk(sessionDir, 0); + } else { + const baseDir = `/tmp/${sessionId}_test_suite/logs`; + const logDirs = await fs.promises.readdir(baseDir); + for (const dir of logDirs) { + const screenshotPath = path.join(baseDir, dir, 'screenshots', `${name}.png`); + try { + await fs.promises.access(screenshotPath); + files.push(screenshotPath); + } catch { /* not found, continue */ } + } + } + } catch { /* base dir not found */ } + return files; +} + +// Locate the screenshot on disk and confirm it lives under `scopeRoot`. Three +// paths converge on `chosenFile`: +// 1. `filePath` supplied (BrowserStack new SDK — absolute path under the BS +// session root; rejected upstream in self-hosted mode). +// 2. BrowserStack glob (the BS-infra SCREENSHOTS_DIR layout). +// 3. Self-hosted recursive glob under scopeRoot (PERCY_MAESTRO_SCREENSHOT_DIR). +// Either way, the shared realpath + scopeRoot prefix check below enforces the +// security invariant. Returns the canonicalized absolute path, or throws +// ServerError(404) when the file is missing or resolves outside scopeRoot. +// Callers pass `filePath` already shape-validated, plus the resolved `scopeRoot` +// and `selfHosted` flag. +export async function locateScreenshot({ platform, sessionId, name, filePath, scopeRoot, selfHosted }) { + let chosenFile; + if (filePath) { + chosenFile = filePath; + } else { + // Glob pattern depends on deployment shape: + // BrowserStack Android: /tmp/{sid}_test_suite/logs/*/screenshots/{name}.png + // BrowserStack iOS: /tmp/{sid}//**/{name}.png + // (realmobile builds a deeply nested {device}_maestro_debug_ tree; `**` + // handles any depth, exact {name}.png filters Maestro's emoji-prefixed + // debug frames, e.g. `screenshot-❌--(flow).png`). + // Self-hosted: recursive glob under the customer's --test-output-dir + // (scopeRoot = PERCY_MAESTRO_SCREENSHOT_DIR). `name` is SAFE_ID-validated + // by the caller, so it cannot contain separators or traversal chars. + let searchPattern; + if (selfHosted) { + // fast-glob requires forward-slashes in patterns on every platform; on + // Windows scopeRoot contains backslashes, so normalize before embedding. + // Production-code Windows portability — verified by the CI Windows runner. + const globRoot = scopeRoot.replace(/\\/g, '/'); + searchPattern = `${globRoot}/**/${name}.png`; + } else { + searchPattern = platform === 'ios' + ? `/tmp/${sessionId}/*_maestro_debug_*/**/${name}.png` + : `/tmp/${sessionId}_test_suite/logs/*/screenshots/${name}.png`; + } + + let files; + try { + let { default: glob } = await import('fast-glob'); + // Self-hosted needs `dot: true` because Maestro's default output dir is + // `.maestro/` — a dot-prefixed entry fast-glob hides by default. BS + // layouts have no dot-prefixed segments, so omitting it there keeps the + // byte-identical behavior. + files = await glob(searchPattern, selfHosted ? { dot: true } : undefined); + } catch { + // Fast-glob import / glob call failed — fall back to manual walker (BS + // only; self-hosted has no fixed-layout convention, so empty → 404 with + // the actionable PERCY_MAESTRO_SCREENSHOT_DIR guidance from the caller). + // See manualScreenshotWalk() at file top + the file-level .semgrepignore. + /* istanbul ignore next — only fires when fast-glob import throws + (broken install / FS corruption); integration-test territory. */ + files = selfHosted ? [] : await manualScreenshotWalk(platform, sessionId, name); + } + + if (!files || files.length === 0) { + throw new ServerError(404, `Screenshot not found: ${name}.png (searched ${searchPattern})`); + } + + // If multiple files match (iOS — same name reused across flows), pick the most recently modified + // for determinism. The else branch only fires when a snapshot name + // is reused across two flows in the same session; the realmobile + // layout normally writes one file per snapshot per session, so the + // multi-match path is exercised by integration tests on BS hosts + // rather than the unit suite. + /* istanbul ignore else */ + if (files.length === 1) { + chosenFile = files[0]; + } else { + let mtimes = await Promise.all(files.map(async f => { + try { return { f, mtime: (await fs.promises.stat(f)).mtimeMs }; } catch { return { f, mtime: 0 }; } + })); + mtimes.sort((a, b) => b.mtime - a.mtime); + chosenFile = mtimes[0].f; + } + } + + // Canonicalize and confirm the resolved path still lives under scopeRoot. + // Defeats symlink swaps where the root points elsewhere. Both ends are + // realpath'd because /tmp is a symlink on macOS (where iOS hosts run). The + // trailing `/` on the prefix is load-bearing — it prevents sibling-prefix + // bypass (e.g. /x/.maestro vs /x/.maestro-secrets). Both sides are + // normalized to forward-slashes so the check works on Windows (real-fs + // returns backslashes), POSIX (no-op), and memfs (POSIX-style virtual paths). + let realPath, realPrefix; + try { + realPath = await fs.promises.realpath(chosenFile); + realPrefix = await fs.promises.realpath(scopeRoot); + } catch { + throw new ServerError(404, `Screenshot not found: ${name}.png (path resolution failed)`); + } + const realPathFwd = realPath.replace(/\\/g, '/'); + const realPrefixFwd = realPrefix.replace(/\\/g, '/'); + if (!realPathFwd.startsWith(`${realPrefixFwd}/`)) { + throw new ServerError(404, `Screenshot not found: ${name}.png (resolved outside ${selfHosted ? 'PERCY_MAESTRO_SCREENSHOT_DIR' : 'session dir'})`); + } + + return realPath; +} diff --git a/packages/core/src/maestro-screenshot.js b/packages/core/src/maestro-screenshot.js new file mode 100644 index 000000000..d1843ba75 --- /dev/null +++ b/packages/core/src/maestro-screenshot.js @@ -0,0 +1,252 @@ +import fs from 'fs'; +import path from 'path'; +import { normalize } from '@percy/config/utils'; +import { ServerError } from './server.js'; +import { encodeURLSearchParams } from './utils.js'; +import { handleSyncJob } from './snapshot.js'; +import { locateScreenshot } from './maestro-screenshot-file.js'; +import { validateRegionInputs, resolveRegions } from './maestro-regions.js'; +import { deriveDeviceInsets } from './maestro-hierarchy.js'; + +// Parse PNG IHDR chunk for the screenshot's actual rendered dimensions. +// Returns { width, height } when the buffer is a valid PNG with non-zero +// dimensions, or null otherwise (non-PNG signature, truncated file, zero +// IHDR values). PNG layout per W3C spec: +// bytes 0..7 PNG signature (89 50 4E 47 0D 0A 1A 0A) +// bytes 8..15 IHDR chunk header (length + type, fixed) +// bytes 16..19 width (big-endian uint32) +// bytes 20..23 height (big-endian uint32) +// No library dependency — pure stdlib Buffer access on the bytes the relay +// has already read. +export function parsePngDimensions(buffer) { + if (!buffer || buffer.length < 24) return null; + if (buffer[0] !== 0x89 || buffer[1] !== 0x50 || buffer[2] !== 0x4E || buffer[3] !== 0x47 || + buffer[4] !== 0x0D || buffer[5] !== 0x0A || buffer[6] !== 0x1A || buffer[7] !== 0x0A) { + return null; + } + const width = buffer.readUInt32BE(16); + const height = buffer.readUInt32BE(20); + if (width <= 0 || height <= 0) return null; + return { width, height }; +} + +// Handler for `post /percy/maestro-screenshot`: post a comparison by reading +// a Maestro screenshot from disk. +export async function handleMaestroScreenshot(req, res, percy) { + /* istanbul ignore next — req.body falsy guard; tests always pass a body. */ + let { name, sessionId } = req.body || {}; + + if (!name) throw new ServerError(400, 'Missing required field: name'); + + // Self-hosted vs BrowserStack is signaled by sessionId presence: BS + // host-injection always supplies it; self-hosted runs never do. + let selfHosted = !sessionId; + + // Strict character-class validation — rejects path separators, shell metacharacters, + // NUL, newlines, and anything else that could confuse the glob or the filesystem. + // `name` is load-bearing for the recursive glob — must not be loosened. + const SAFE_ID = /^[a-zA-Z0-9_-]+$/; + if (!SAFE_ID.test(name)) { + throw new ServerError(400, 'Invalid screenshot name'); + } + if (sessionId && !SAFE_ID.test(sessionId)) { + throw new ServerError(400, 'Invalid sessionId'); + } + + // Resolve platform signal: strict whitelist on `platform` when present; default Android when absent. + // Backward compatible with SDK v0.2.0 (no platform field → Android glob). + let platform = 'android'; + if (req.body.platform !== undefined) { + if (typeof req.body.platform !== 'string') { + throw new ServerError(400, 'Invalid platform: must be a string'); + } + let normalized = req.body.platform.toLowerCase(); + if (normalized !== 'ios' && normalized !== 'android') { + throw new ServerError(400, `Invalid platform: must be "ios" or "android", got "${req.body.platform}"`); + } + platform = normalized; + } + + // Optional caller-supplied absolute path. When present, the relay reads + // the file directly and skips the legacy glob — the SDK has already + // chosen the path under the BS session root. Shape errors (non-string, + // non-absolute, too long) are 400. Existence and session-root scoping + // are enforced by the shared realpath + prefix check below, which + // returns 404 — same shape as the glob path. Treat empty string as + // absent so older SDKs that emit the field unconditionally still fall + // through to the glob. + let suppliedFilePath = null; + if (req.body.filePath !== undefined && req.body.filePath !== null && req.body.filePath !== '') { + if (typeof req.body.filePath !== 'string') { + throw new ServerError(400, 'Invalid filePath: must be a string'); + } + if (req.body.filePath.length > 1024) { + throw new ServerError(400, 'Invalid filePath: exceeds maximum length of 1024'); + } + if (!path.isAbsolute(req.body.filePath)) { + throw new ServerError(400, 'Invalid filePath: must be an absolute path'); + } + suppliedFilePath = req.body.filePath; + } + + // Resolve the file-find scope root. On BrowserStack (sessionId present), the + // root is the BS host's /tmp/{sessionId}{_test_suite} convention. Self-hosted + // (sessionId absent) requires PERCY_MAESTRO_SCREENSHOT_DIR (read from + // process.env, never the request body) to be an absolute, existing directory + // — typically the customer's `maestro test --test-output-dir ` path. The + // realpath + prefix check inside locateScreenshot enforces the security + // invariant at whichever root applies; the boundary is relocated, not removed. + let scopeRoot; + if (selfHosted) { + // Reject filePath outright in self-hosted mode. The SDK never emits it (it + // sends a relative SCREENSHOT_NAME); honoring an absolute filePath against + // a caller-influenceable root would re-open arbitrary in-root reads. + if (suppliedFilePath) { + throw new ServerError(400, 'filePath is not accepted in self-hosted mode (omit it; PERCY_MAESTRO_SCREENSHOT_DIR + relative SCREENSHOT_NAME is the supported path)'); + } + let dir = process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + if (!dir) { + throw new ServerError(400, 'Missing required env: PERCY_MAESTRO_SCREENSHOT_DIR (set it to your `maestro test --test-output-dir` path)'); + } + if (!path.isAbsolute(dir)) { + throw new ServerError(400, 'PERCY_MAESTRO_SCREENSHOT_DIR must be an absolute path'); + } + // UX guard ONLY: surface an actionable 400 ("dir not found") instead of the + // opaque 404 the realpath+prefix containment check would emit for a missing + // dir. A small TOCTOU window exists between this stat and locateScreenshot's + // realpath — acceptable because realpath (not stat) is the security + // invariant: even if the dir is swapped for a symlink in between, realpath + // resolves the target and the sep-prefix check rejects anything outside it. + let stat; + try { stat = await fs.promises.stat(dir); } catch { stat = null; } + if (!stat || !stat.isDirectory()) { + throw new ServerError(400, `PERCY_MAESTRO_SCREENSHOT_DIR is not an existing directory: ${dir}`); + } + scopeRoot = dir; + } else { + scopeRoot = platform === 'ios' + ? `/tmp/${sessionId}` + : `/tmp/${sessionId}_test_suite`; + } + + // Validate regions input shape early (before file I/O and ADB work) so + // malformed requests don't consume resolver/relay work. + validateRegionInputs(req.body); + + // Locate the screenshot on disk (supplied filePath, BS session glob, or + // self-hosted PERCY_MAESTRO_SCREENSHOT_DIR recursive glob) and confirm it + // resolves under scopeRoot. Throws ServerError(404) when missing/out-of-root. + let realPath = await locateScreenshot({ platform, sessionId, name, filePath: suppliedFilePath, scopeRoot, selfHosted }); + + // Read and base64-encode the screenshot + let fileContent = await fs.promises.readFile(realPath); + let base64Content = fileContent.toString('base64'); + + // Parse the PNG header for actual rendered dimensions. The PNG bytes + // ARE the source of truth — what Percy stores and compares against. + // Fills tag.width/height when the customer didn't supply them (or + // supplied invalid values); customer-supplied values continue to win + // for backward compat with any flow that pins a specific tag dim. + let pngDims = parsePngDimensions(fileContent); + + // Build tag from optional request body fields + let tag = req.body.tag || { name: 'Unknown Device', osName: 'Android' }; + /* istanbul ignore if — fallback when tag.name is missing; tests always + pass a complete tag object. */ + if (!tag.name) tag.name = 'Unknown Device'; + if (pngDims) { + if (typeof tag.width !== 'number' || tag.width <= 0 || isNaN(tag.width)) { + tag.width = pngDims.width; + } + if (typeof tag.height !== 'number' || tag.height <= 0 || isNaN(tag.height)) { + tag.height = pngDims.height; + } + } + + // Derive exact device system-bar insets (pixels), once per session — they're + // device-constant, so the first snapshot pays one /viewHierarchy (iOS) or + // `dumpsys` (Android) call and the rest reuse the cached result (incl. a null + // "use SDK default" outcome). CLI-derived values are authoritative over the + // SDK's static defaults (those are SDK internal constants, not customer-set); + // any derivation failure falls back to the SDK value. iOS navBarHeight is + // always 0 — the home indicator is static and unmeasured, fleet-consistent. + let insets = percy.maestroInsetCache.get(sessionId); + if (insets === undefined) { + insets = await deriveDeviceInsets({ platform, sessionId, pngDims }); + percy.maestroInsetCache.set(sessionId, insets); + percy.log.debug(`maestro device insets (${platform}): ${JSON.stringify(insets)}`); + } + let statusBarHeight = insets?.statusBarHeight ?? (req.body.statusBarHeight || 0); + let navBarHeight = platform === 'ios' + ? 0 + : (insets?.navBarHeight ?? (req.body.navBarHeight || 0)); + + // Construct comparison payload with tile metadata from request + let payload = { + name, + tag, + tiles: [{ + content: base64Content, + statusBarHeight, + navBarHeight, + headerHeight: 0, + footerHeight: 0, + fullscreen: req.body.fullscreen || false + }], + clientInfo: req.body.clientInfo || 'percy-maestro/0.1.0', + environmentInfo: req.body.environmentInfo || 'percy-maestro' + }; + + if (req.body.testCase) payload.testCase = req.body.testCase; + if (req.body.labels) payload.labels = req.body.labels; + if (req.body.thTestCaseExecutionId) payload.thTestCaseExecutionId = req.body.thTestCaseExecutionId; + + // Resolve element/coordinate regions to comparison-payload fragments. + // Element regions degrade gracefully — a resolver failure warn-skips those + // regions only; the snapshot still uploads. The hierarchy dump is memoized + // per request inside resolveRegions. Assign the three known comparison keys + // explicitly (never Object.assign of request-derived data) so only these + // fields can ever be set here. + let regionData = await resolveRegions({ body: req.body, platform, sessionId, percy }); + if (regionData.regions) payload.regions = regionData.regions; + if (regionData.ignoredElementsData) payload.ignoredElementsData = regionData.ignoredElementsData; + if (regionData.consideredElementsData) payload.consideredElementsData = regionData.consideredElementsData; + + // Upload via percy — sync or fire-and-forget + if (req.body.sync === true) payload.sync = true; + + let data; + if (percy.syncMode(payload)) { + // percy.upload() is the generatePromise-wrapped method: calling it drives the + // underlying async generator to completion (enqueuing #snapshots) and the sync + // queue resolves/rejects the attached callback. Do NOT `for await` the return + // value — it is a Promise, not an async iterable (that throws "upload is not + // async iterable"). The raw generator lives at percy.yield.upload() if direct + // iteration is ever needed. The trailing .catch(reject) surfaces generator + // errors that bypass the sync-queue callback (e.g. a throw before the queue task + // runs) instead of leaking an unhandled rejection and hanging the request. + const snapshotPromise = new Promise((resolve, reject) => { + percy.upload(payload, { resolve, reject }, 'app').catch(reject); + }); + data = await handleSyncJob(snapshotPromise, percy, 'comparison'); + return res.json(200, { success: true, data }); + } + + let upload = percy.upload(payload, null, 'app'); + /* istanbul ignore if — ?await=true URL flag triggers fire-and-wait; + tests cover both syncMode and fire-and-forget but not the explicit + ?await query-param variant. */ + if (req.url.searchParams.has('await')) await upload; + + // Generate redirect link + let link = [ + percy.client.apiUrl, '/comparisons/redirect?', + encodeURLSearchParams(normalize({ + buildId: percy.build?.id, + snapshot: { name }, + tag + }, { snake: true })) + ].join(''); + + return res.json(200, { success: true, link }); +} diff --git a/packages/core/src/percy.js b/packages/core/src/percy.js index 1bab43d7a..1e8319ae4 100644 --- a/packages/core/src/percy.js +++ b/packages/core/src/percy.js @@ -145,6 +145,15 @@ export class Percy { this.grpcClientCache = new Map(); this.grpcClientCache.shutdownInProgress = false; + // Per-Percy cache of derived device system-bar insets, keyed by sessionId. + // Insets are device-constant within a session, so the Maestro relay derives + // them once (one /viewHierarchy or `dumpsys` call) and reuses the result — + // including a null "derivation failed, use SDK default" outcome — for every + // subsequent snapshot in that session. Per-instance (not module-scoped) so + // concurrent Percy instances don't share session state; holds plain data + // (no sockets), so stop() just clears it. + this.maestroInsetCache = new Map(); + // Domain validation state for auto domain allow-listing this.domainValidation = { autoConfiguredHosts: new Set(), // Domains from project config @@ -451,6 +460,9 @@ export class Percy { this.grpcClientCache.shutdownInProgress = true; closeGrpcClientCache(this.grpcClientCache); + // Drop the per-session device-inset cache (plain data, no sockets). + this.maestroInsetCache.clear(); + // mark instance as stopped this.readyState = 3; } catch (err) { diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index bdc206587..328e46bf8 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -85,6 +85,13 @@ export function buildSyntheticFrameResourceUrl(rootUrl, percyElementId) { } } +// Returns a URL encoded string of nested query params +export function encodeURLSearchParams(subj, prefix) { + return typeof subj === 'object' ? Object.entries(subj).map(([key, value]) => ( + encodeURLSearchParams(value, prefix ? `${prefix}[${key}]` : key) + )).join('&') : `${prefix}=${encodeURIComponent(subj)}`; +} + // Process CORS iframes in a single domSnapshot object. `rootUrl` is the page // URL of the snapshot, used as the base for synthetic resource URLs. export function processCorsIframesInDomSnapshot(domSnapshot, rootUrl) { diff --git a/packages/core/test/api.test.js b/packages/core/test/api.test.js index 35b49aead..fb20c89e3 100644 --- a/packages/core/test/api.test.js +++ b/packages/core/test/api.test.js @@ -1228,6 +1228,12 @@ describe('API Server', () => { fs.writeFileSync(path.join(ANDROID_FILEPATH_DIR, `${FILEPATH_NAME}.png`), 'PNGBYTES-FILEPATH-ANDROID'); fs.mkdirSync(IOS_FILEPATH_DIR, { recursive: true }); fs.writeFileSync(path.join(IOS_FILEPATH_DIR, `${FILEPATH_NAME}.png`), 'PNGBYTES-FILEPATH-IOS'); + + // Short-circuit device system-bar inset derivation in the unit env (no + // real device/adb): seeding the per-session cache makes the relay skip + // deriveDeviceInsets and fall back to the request's statusBarHeight/ + // navBarHeight. Tests that assert derived behavior override this seed. + percy.maestroInsetCache.set(SID, null); }); async function postMaestro(body) { @@ -2064,6 +2070,82 @@ describe('API Server', () => { expect(payload.tag.height).toBe(2400); }); }); + + describe('device system-bar inset derivation (relay wiring)', () => { + it('CLI-derived insets are authoritative over the SDK-sent defaults (Android)', async () => { + // Override the beforeEach null seed with a derived result. + percy.maestroInsetCache.set(SID, { statusBarHeight: 141, navBarHeight: 168 }); + spyOn(percy, 'upload').and.resolveTo(); + await percy.start(); + + await expectAsync(postMaestro({ + name: SS_NAME, + sessionId: SID, + platform: 'android', + statusBarHeight: 50, + navBarHeight: 48 + })).toBeResolvedTo(jasmine.objectContaining({ success: true })); + + let [payload] = percy.upload.calls.mostRecent().args; + expect(payload.tiles[0]).toEqual(jasmine.objectContaining({ statusBarHeight: 141, navBarHeight: 168 })); + }); + + it('falls back to the SDK-sent values when derivation yields null (Android)', async () => { + percy.maestroInsetCache.set(SID, null); + spyOn(percy, 'upload').and.resolveTo(); + await percy.start(); + + await expectAsync(postMaestro({ + name: SS_NAME, + sessionId: SID, + platform: 'android', + statusBarHeight: 50, + navBarHeight: 48 + })).toBeResolvedTo(jasmine.objectContaining({ success: true })); + + let [payload] = percy.upload.calls.mostRecent().args; + expect(payload.tiles[0]).toEqual(jasmine.objectContaining({ statusBarHeight: 50, navBarHeight: 48 })); + }); + + it('iOS navBarHeight is always 0, even when the SDK sends a value', async () => { + percy.maestroInsetCache.set(SID, { statusBarHeight: 141, navBarHeight: 0 }); + spyOn(percy, 'upload').and.resolveTo(); + await percy.start(); + + await expectAsync(postMaestro({ + name: SS_NAME, + sessionId: SID, + platform: 'ios', + statusBarHeight: 47, + navBarHeight: 80 + })).toBeResolvedTo(jasmine.objectContaining({ success: true })); + + let [payload] = percy.upload.calls.mostRecent().args; + expect(payload.tiles[0].statusBarHeight).toBe(141); + expect(payload.tiles[0].navBarHeight).toBe(0); + }); + + it('derives once and caches the outcome (incl. null) per session', async () => { + // Cache miss → derive. iOS with no PERCY_IOS_DRIVER_HOST_PORT env yields + // null deterministically (no transport spawn). Outcome is cached. + percy.maestroInsetCache.delete(SID); + spyOn(percy, 'upload').and.resolveTo(); + await percy.start(); + + await expectAsync(postMaestro({ + name: SS_NAME, + sessionId: SID, + platform: 'ios', + statusBarHeight: 47 + })).toBeResolvedTo(jasmine.objectContaining({ success: true })); + + // Null outcome cached, and the tile fell back to the SDK-sent value. + expect(percy.maestroInsetCache.has(SID)).toBe(true); + expect(percy.maestroInsetCache.get(SID)).toBeNull(); + let [payload] = percy.upload.calls.mostRecent().args; + expect(payload.tiles[0].statusBarHeight).toBe(47); + }); + }); }); }); diff --git a/packages/core/test/percy.test.js b/packages/core/test/percy.test.js index 15538dd60..2001b65a1 100644 --- a/packages/core/test/percy.test.js +++ b/packages/core/test/percy.test.js @@ -885,6 +885,12 @@ describe('Percy', () => { expect(percy.browser.isConnected()).toBe(false); }); + it('clears the per-session device-inset cache', async () => { + percy.maestroInsetCache.set('some-session', { statusBarHeight: 141, navBarHeight: 0 }); + await expectAsync(percy.stop()).toBeResolved(); + expect(percy.maestroInsetCache.size).toBe(0); + }); + it('clears pending tasks and logs when force stopping', async () => { await reset({ deferUploads: true }); await expectAsync(percy.stop(true)).toBeResolved(); diff --git a/packages/core/test/unit/maestro-hierarchy.test.js b/packages/core/test/unit/maestro-hierarchy.test.js index 59145b975..7e45a6eb8 100644 --- a/packages/core/test/unit/maestro-hierarchy.test.js +++ b/packages/core/test/unit/maestro-hierarchy.test.js @@ -8,6 +8,7 @@ import { runAndroidGrpcDump, classifyGrpcFailure, closeGrpcClientCache, + deriveDeviceInsets, __testing } from '../../src/maestro-hierarchy.js'; import { logger, setupTest } from '../helpers/index.js'; @@ -2077,4 +2078,190 @@ describe('Unit / maestro-hierarchy', () => { }); }); }); + + describe('deriveDeviceInsets', () => { + const iosFixtureDir = path.resolve(url.fileURLToPath(import.meta.url), '../../fixtures/maestro-ios-hierarchy'); + const loadIosFixture = name => fs.readFileSync(path.join(iosFixtureDir, name), 'utf8'); + + const iosEnv = key => (key === 'PERCY_IOS_DRIVER_HOST_PORT' ? '11100' : undefined); + + const makeHttp = handler => { + const fn = async opts => handler(opts); + return fn; + }; + const httpOk = body => makeHttp(() => ({ statusCode: 200, headers: { 'content-type': 'application/json' }, body })); + + // Build a minimal cli-2.0.7-shape wrap: AUT app (elementType 1) at + // children[0] with the given root point-height, and a statusBars sibling + // (elementType 0) at children[1] whose child is the status bar + // (elementType 26) with the given point-height. + const makeIosBody = (rootPt, statusBarPt) => { + const statusBar = { identifier: '', frame: { X: 0, Y: 0, Width: 390, Height: statusBarPt }, elementType: 26 }; + const statusContainer = { identifier: '', frame: { X: 0, Y: 0, Width: 0, Height: 0 }, elementType: 0, children: [statusBar] }; + const aut = { identifier: 'com.example.app', frame: { X: 0, Y: 0, Width: 390, Height: rootPt }, elementType: 1, children: [] }; + const root = { identifier: '', frame: { X: 0, Y: 0, Width: 0, Height: 0 }, elementType: 0, children: [aut, statusContainer] }; + return JSON.stringify({ axElement: root }); + }; + + describe('iOS', () => { + it('derives status bar in pixels from the statusBars frame and PNG scale (iPhone 14 → 141px)', async () => { + const httpRequest = httpOk(loadIosFixture('viewHierarchy-response.json')); // 844pt root, 47pt SB + const res = await deriveDeviceInsets({ platform: 'ios', sessionId: 'sid', pngDims: { width: 1170, height: 2532 }, httpRequest, getEnv: iosEnv }); + expect(res).toEqual({ statusBarHeight: 141, navBarHeight: 0 }); + }); + + it('snaps a 2x device scale correctly', async () => { + const httpRequest = httpOk(makeIosBody(800, 20)); // 1600/800 = 2 → 20*2 + const res = await deriveDeviceInsets({ platform: 'ios', pngDims: { width: 750, height: 1600 }, httpRequest, getEnv: iosEnv }); + expect(res).toEqual({ statusBarHeight: 40, navBarHeight: 0 }); + }); + + it('returns null when the derived scale is out of the plausible range', async () => { + const httpRequest = httpOk(makeIosBody(844, 47)); // 4220/844 = 5 → reject + const res = await deriveDeviceInsets({ platform: 'ios', pngDims: { width: 1170, height: 4220 }, httpRequest, getEnv: iosEnv }); + expect(res).toBeNull(); + }); + + it('returns null when the PNG/point ratio is too far from an integer (unreliable root frame)', async () => { + const httpRequest = httpOk(makeIosBody(844, 47)); // 2279/844 = 2.70 → 0.30 off → reject + const res = await deriveDeviceInsets({ platform: 'ios', pngDims: { width: 1170, height: 2279 }, httpRequest, getEnv: iosEnv }); + expect(res).toBeNull(); + }); + + it('returns null when no status-bar element is present', async () => { + const aut = { identifier: 'com.example.app', frame: { X: 0, Y: 0, Width: 390, Height: 844 }, elementType: 1, children: [] }; + const body = JSON.stringify({ axElement: { identifier: '', frame: { X: 0, Y: 0, Width: 0, Height: 0 }, elementType: 0, children: [aut] } }); + const httpRequest = httpOk(body); + const res = await deriveDeviceInsets({ platform: 'ios', pngDims: { width: 1170, height: 2532 }, httpRequest, getEnv: iosEnv }); + expect(res).toBeNull(); + }); + + it('returns null on a SpringBoard-only response (no AUT root)', async () => { + const httpRequest = httpOk(loadIosFixture('viewHierarchy-response-springboard-only.json')); + const res = await deriveDeviceInsets({ platform: 'ios', pngDims: { width: 1170, height: 2532 }, httpRequest, getEnv: iosEnv }); + expect(res).toBeNull(); + }); + + it('returns null on non-JSON body', async () => { + const httpRequest = httpOk('not json'); + const res = await deriveDeviceInsets({ platform: 'ios', pngDims: { width: 1170, height: 2532 }, httpRequest, getEnv: iosEnv }); + expect(res).toBeNull(); + }); + + it('returns null on a missing axElement root', async () => { + const httpRequest = httpOk(JSON.stringify({ depth: 1 })); + const res = await deriveDeviceInsets({ platform: 'ios', pngDims: { width: 1170, height: 2532 }, httpRequest, getEnv: iosEnv }); + expect(res).toBeNull(); + }); + + it('returns null when the parsed body is the JSON literal null', async () => { + const httpRequest = httpOk('null'); + const res = await deriveDeviceInsets({ platform: 'ios', pngDims: { width: 1170, height: 2532 }, httpRequest, getEnv: iosEnv }); + expect(res).toBeNull(); + }); + + it('returns null on a non-200 response', async () => { + const httpRequest = makeHttp(() => ({ statusCode: 500, headers: {}, body: '' })); + const res = await deriveDeviceInsets({ platform: 'ios', pngDims: { width: 1170, height: 2532 }, httpRequest, getEnv: iosEnv }); + expect(res).toBeNull(); + }); + + it('returns null on a transport throw', async () => { + const httpRequest = makeHttp(() => { throw Object.assign(new Error('refused'), { code: 'ECONNREFUSED' }); }); + const res = await deriveDeviceInsets({ platform: 'ios', pngDims: { width: 1170, height: 2532 }, httpRequest, getEnv: iosEnv }); + expect(res).toBeNull(); + }); + + it('returns null when pngDims is missing', async () => { + const httpRequest = httpOk(loadIosFixture('viewHierarchy-response.json')); + const res = await deriveDeviceInsets({ platform: 'ios', pngDims: null, httpRequest, getEnv: iosEnv }); + expect(res).toBeNull(); + }); + + it('returns null when the driver-host-port env is absent', async () => { + const httpRequest = httpOk(loadIosFixture('viewHierarchy-response.json')); + const res = await deriveDeviceInsets({ platform: 'ios', pngDims: { width: 1170, height: 2532 }, httpRequest, getEnv: () => undefined }); + expect(res).toBeNull(); + }); + + it('returns null (never throws) when a transport throws unexpectedly', async () => { + const res = await deriveDeviceInsets({ + platform: 'ios', + pngDims: { width: 1170, height: 2532 }, + getEnv: () => { throw new Error('boom'); } + }); + expect(res).toBeNull(); + }); + }); + + describe('Android', () => { + const dumpsysOut = (top, bottom) => `WINDOW MANAGER WINDOWS\n Display: mStableInsets=Rect(0, ${top} - 0, ${bottom})\n`; + + it('derives status + nav insets via dumpsys (3-button nav)', async () => { + const execAdb = makeFakeExecAdb([ + { match: args => args.includes('dumpsys'), result: { stdout: dumpsysOut(132, 168), stderr: '', exitCode: 0 } } + ]); + const res = await deriveDeviceInsets({ platform: 'android', execAdb, getEnv: k => (k === 'ANDROID_SERIAL' ? 'serial-1' : undefined) }); + expect(res).toEqual({ statusBarHeight: 132, navBarHeight: 168 }); + }); + + it('derives a small bottom inset for gesture nav', async () => { + const execAdb = makeFakeExecAdb([ + { match: args => args.includes('dumpsys'), result: { stdout: dumpsysOut(72, 63), stderr: '', exitCode: 0 } } + ]); + const res = await deriveDeviceInsets({ platform: 'android', execAdb, getEnv: k => (k === 'ANDROID_SERIAL' ? 'serial-1' : undefined) }); + expect(res).toEqual({ statusBarHeight: 72, navBarHeight: 63 }); + }); + + it('targets the resolved serial with -s', async () => { + const execAdb = makeFakeExecAdb([ + { match: args => args.includes('dumpsys'), result: { stdout: dumpsysOut(72, 144), stderr: '', exitCode: 0 } } + ]); + await deriveDeviceInsets({ platform: 'android', execAdb, getEnv: k => (k === 'ANDROID_SERIAL' ? 'env-serial-9' : undefined) }); + const dumpsysCall = execAdb.calls.find(a => a.includes('dumpsys')); + expect(dumpsysCall.slice(0, 2)).toEqual(['-s', 'env-serial-9']); + }); + + it('resolves the serial via `adb devices` when ANDROID_SERIAL is unset', async () => { + const execAdb = makeFakeExecAdb([ + { match: args => args[0] === 'devices', result: okDevices }, + { match: args => args.includes('dumpsys'), result: { stdout: dumpsysOut(72, 144), stderr: '', exitCode: 0 } } + ]); + const res = await deriveDeviceInsets({ platform: 'android', execAdb, getEnv: () => undefined }); + expect(res).toEqual({ statusBarHeight: 72, navBarHeight: 144 }); + }); + + it('returns null when no device is available', async () => { + const execAdb = makeFakeExecAdb([ + { match: args => args[0] === 'devices', result: { stdout: 'List of devices attached\n\n', stderr: '', exitCode: 0 } } + ]); + const res = await deriveDeviceInsets({ platform: 'android', execAdb, getEnv: () => undefined }); + expect(res).toBeNull(); + }); + + it('returns null on an adb spawn error', async () => { + const execAdb = makeFakeExecAdb([ + { match: args => args.includes('dumpsys'), result: { spawnError: Object.assign(new Error('nope'), { code: 'ENOENT' }) } } + ]); + const res = await deriveDeviceInsets({ platform: 'android', execAdb, getEnv: k => (k === 'ANDROID_SERIAL' ? 'serial-1' : undefined) }); + expect(res).toBeNull(); + }); + + it('returns null on a non-zero dumpsys exit', async () => { + const execAdb = makeFakeExecAdb([ + { match: args => args.includes('dumpsys'), result: { stdout: '', stderr: '', exitCode: 1 } } + ]); + const res = await deriveDeviceInsets({ platform: 'android', execAdb, getEnv: k => (k === 'ANDROID_SERIAL' ? 'serial-1' : undefined) }); + expect(res).toBeNull(); + }); + + it('returns null when dumpsys output has no mStableInsets line', async () => { + const execAdb = makeFakeExecAdb([ + { match: args => args.includes('dumpsys'), result: { stdout: 'WINDOW MANAGER WINDOWS\n (no insets here)\n', stderr: '', exitCode: 0 } } + ]); + const res = await deriveDeviceInsets({ platform: 'android', execAdb, getEnv: k => (k === 'ANDROID_SERIAL' ? 'serial-1' : undefined) }); + expect(res).toBeNull(); + }); + }); + }); });