diff --git a/packages/core/src/maestro-screenshot-file.js b/packages/core/src/maestro-screenshot-file.js index 17a12e4d5..753e57ce9 100644 --- a/packages/core/src/maestro-screenshot-file.js +++ b/packages/core/src/maestro-screenshot-file.js @@ -15,31 +15,48 @@ export function appAutomateTmpDir() { return path.isAbsolute(dir) ? dir : '/tmp'; } +// Complete scope root for hosts whose layout {appAutomateTmpDir()}/{sessionId} +// can't express (realmobile's AAP-18965 iOS move). When set this IS the root: +// globbed recursively, and the realpath containment check anchors on it — so +// the boundary moves, never widens. Non-absolute is ignored, which drops '/'. +export function bsScopeRootOverride() { + let raw = process.env.PERCY_MAESTRO_BS_SCOPE_ROOT; + if (!raw) return null; + let dir = raw.replace(/[/\\]+$/, ''); + return path.isAbsolute(dir) ? dir : null; +} + /* 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) { +async function manualScreenshotWalk(platform, sessionId, name, scopeRoot) { const files = []; - try { - if (platform === 'ios') { - const sessionDir = `${appAutomateTmpDir()}/${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); - } + // `accept` gates matches: iOS keeps its `_maestro_debug_` guard, an explicit + // root (already narrowed to one session) takes anything. + const walkFrom = async (root, accept) => { + 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` && accept(full)) { + files.push(full); } - }; - await walk(sessionDir, 0); + } + }; + await walk(root, 0); + }; + try { + if (scopeRoot) { + await walkFrom(scopeRoot, () => true); + } else if (platform === 'ios') { + await walkFrom(`${appAutomateTmpDir()}/${sessionId}`, full => full.includes('_maestro_debug_')); } else { const baseDir = `${appAutomateTmpDir()}/${sessionId}_test_suite/logs`; const logDirs = await fs.promises.readdir(baseDir); @@ -60,13 +77,15 @@ async function manualScreenshotWalk(platform, sessionId, name) { // 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). +// 3. Recursive glob under scopeRoot — self-hosted, or a BS explicit root. // 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 }) { +export async function locateScreenshot({ platform, sessionId, name, filePath, scopeRoot, selfHosted, recursiveScope }) { + // Derived, not trusted from the caller, so `selfHosted` keeps its meaning. + let scopedGlob = selfHosted || !!recursiveScope; let chosenFile; if (filePath) { chosenFile = filePath; @@ -80,8 +99,10 @@ export async function locateScreenshot({ platform, sessionId, name, filePath, sc // 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. + // BS explicit root: same recursive glob — the host already narrowed + // scopeRoot to this session, so no convention is left to key on. let searchPattern; - if (selfHosted) { + if (scopedGlob) { // 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. @@ -108,10 +129,13 @@ export async function locateScreenshot({ platform, sessionId, name, filePath, sc // 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). + // The walker mirrors the glob: explicit root recurses it, else convention. // 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); + files = selfHosted + ? [] + : await manualScreenshotWalk(platform, sessionId, name, recursiveScope ? scopeRoot : null); } if (!files || files.length === 0) { diff --git a/packages/core/src/maestro-screenshot.js b/packages/core/src/maestro-screenshot.js index 3ae4e131a..aa90f4c20 100644 --- a/packages/core/src/maestro-screenshot.js +++ b/packages/core/src/maestro-screenshot.js @@ -4,7 +4,7 @@ import { normalize } from '@percy/config/utils'; import { ServerError } from './server.js'; import { encodeURLSearchParams } from './utils.js'; import { handleSyncJob } from './snapshot.js'; -import { locateScreenshot, appAutomateTmpDir } from './maestro-screenshot-file.js'; +import { locateScreenshot, appAutomateTmpDir, bsScopeRootOverride } from './maestro-screenshot-file.js'; import { validateRegionInputs, resolveRegions } from './maestro-regions.js'; import { deriveDeviceInsets } from './maestro-hierarchy.js'; @@ -91,13 +91,16 @@ export async function handleMaestroScreenshot(req, res, percy) { // Resolve the file-find scope root. On BrowserStack (sessionId present), the // root is the BS host's {appAutomateTmpDir()}/{sessionId}{_test_suite} - // convention (PERCY_APP_AUTOMATE_TMP_DIR, defaulting to /tmp). Self-hosted + // convention (PERCY_APP_AUTOMATE_TMP_DIR, defaulting to /tmp), unless the host + // injected a complete root via PERCY_MAESTRO_BS_SCOPE_ROOT. 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; + // Search the whole root — self-hosted, or a BS explicit root. + let recursiveScope = false; 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 @@ -124,10 +127,20 @@ export async function handleMaestroScreenshot(req, res, percy) { throw new ServerError(400, `PERCY_MAESTRO_SCREENSHOT_DIR is not an existing directory: ${dir}`); } scopeRoot = dir; + recursiveScope = true; } else { - scopeRoot = platform === 'ios' - ? `${appAutomateTmpDir()}/${sessionId}` - : `${appAutomateTmpDir()}/${sessionId}_test_suite`; + // No existence pre-check (unlike self-hosted): host config, not customer + // config, so a stale root should 404, not 400 at the customer. + let overrideRoot = bsScopeRootOverride(); + if (overrideRoot) { + scopeRoot = overrideRoot; + recursiveScope = true; + percy.log.debug(`maestro screenshot scope root overridden: ${scopeRoot}`); + } else { + scopeRoot = platform === 'ios' + ? `${appAutomateTmpDir()}/${sessionId}` + : `${appAutomateTmpDir()}/${sessionId}_test_suite`; + } } // Validate regions input shape early (before file I/O and ADB work) so @@ -137,7 +150,7 @@ export async function handleMaestroScreenshot(req, res, percy) { // 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 }); + let realPath = await locateScreenshot({ platform, sessionId, name, filePath: suppliedFilePath, scopeRoot, selfHosted, recursiveScope }); // Read and base64-encode the screenshot let fileContent = await fs.promises.readFile(realPath); diff --git a/packages/core/test/api.test.js b/packages/core/test/api.test.js index df0f6ec3d..cea66cff0 100644 --- a/packages/core/test/api.test.js +++ b/packages/core/test/api.test.js @@ -5,7 +5,7 @@ import { logger, setupTest, fs } from './helpers/index.js'; import Percy from '@percy/core'; import WebdriverUtils from '@percy/webdriver-utils'; import { getPercyDomPath, _applyHttpReadOnlyStripping } from '../src/api.js'; -import { appAutomateTmpDir } from '../src/maestro-screenshot-file.js'; +import { appAutomateTmpDir, bsScopeRootOverride } from '../src/maestro-screenshot-file.js'; describe('API Server', () => { let percy; @@ -25,7 +25,7 @@ describe('API Server', () => { // suite (works in isolation; returns [] mid-suite), so route the // self-hosted root to the REAL filesystem — this also tests the true // production glob path. Only paths under this unique root are affected. - await setupTest({ filesystem: { $bypass: [p => typeof p === 'string' && (p.includes('percy-self-hosted-real') || p.includes('percy-bs-tmp-real'))] } }); + await setupTest({ filesystem: { $bypass: [p => typeof p === 'string' && (p.includes('percy-self-hosted-real') || p.includes('percy-bs-tmp-real') || p.includes('percy-bs-scope-'))] } }); percy = new Percy({ token: 'PERCY_TOKEN', @@ -1915,6 +1915,135 @@ describe('API Server', () => { }); }); + // Mirrors the realmobile AAP-18965 shape: no sessionId segment and no + // _ prefix, so no tmp-root value composes it. Real-fs root for the + // same fast-glob binding-staleness reason as above. + describe('PERCY_MAESTRO_BS_SCOPE_ROOT override', () => { + const SCOPE_ROOT = path.join(os.tmpdir(), 'percy-bs-scope-real-root'); + const REALMOBILE_DIR = path.join(SCOPE_ROOT, 'maestro_debug_LoginFlow_LoginFlow_0'); + let priorScope, priorTmp; + + beforeEach(() => { + priorScope = process.env.PERCY_MAESTRO_BS_SCOPE_ROOT; + priorTmp = process.env.PERCY_APP_AUTOMATE_TMP_DIR; + process.env.PERCY_MAESTRO_BS_SCOPE_ROOT = SCOPE_ROOT; + fs.rmSync(SCOPE_ROOT, { recursive: true, force: true }); + fs.mkdirSync(REALMOBILE_DIR, { recursive: true }); + fs.writeFileSync(path.join(REALMOBILE_DIR, `${SS_NAME}.png`), 'PNGBYTES-SCOPE-ROOT'); + }); + + afterEach(() => { + if (priorScope === undefined) delete process.env.PERCY_MAESTRO_BS_SCOPE_ROOT; + else process.env.PERCY_MAESTRO_BS_SCOPE_ROOT = priorScope; + if (priorTmp === undefined) delete process.env.PERCY_APP_AUTOMATE_TMP_DIR; + else process.env.PERCY_APP_AUTOMATE_TMP_DIR = priorTmp; + fs.rmSync(SCOPE_ROOT, { recursive: true, force: true }); + }); + + it('finds a screenshot the platform convention cannot reach', async () => { + spyOn(percy, 'upload').and.resolveTo(); + await percy.start(); + + await expectAsync(postMaestro({ name: SS_NAME, sessionId: SID, platform: 'ios' })) + .toBeResolvedTo(jasmine.objectContaining({ success: true })); + + let [payload] = percy.upload.calls.mostRecent().args; + expect(payload.tiles[0].content).toBe(Buffer.from('PNGBYTES-SCOPE-ROOT').toString('base64')); + }); + + it('applies to android too — the root is the whole convention', async () => { + spyOn(percy, 'upload').and.resolveTo(); + await percy.start(); + + await expectAsync(postMaestro({ name: SS_NAME, sessionId: SID, platform: 'android' })) + .toBeResolvedTo(jasmine.objectContaining({ success: true })); + + let [payload] = percy.upload.calls.mostRecent().args; + expect(payload.tiles[0].content).toBe(Buffer.from('PNGBYTES-SCOPE-ROOT').toString('base64')); + }); + + it('wins over PERCY_APP_AUTOMATE_TMP_DIR when both are set', async () => { + process.env.PERCY_APP_AUTOMATE_TMP_DIR = '/tmp'; + spyOn(percy, 'upload').and.resolveTo(); + await percy.start(); + + await expectAsync(postMaestro({ name: SS_NAME, sessionId: SID, platform: 'ios' })) + .toBeResolvedTo(jasmine.objectContaining({ success: true })); + + let [payload] = percy.upload.calls.mostRecent().args; + // Not the /tmp IOS_DIR fixture the composed convention would have hit + expect(payload.tiles[0].content).toBe(Buffer.from('PNGBYTES-SCOPE-ROOT').toString('base64')); + }); + + it('tolerates a trailing slash on the override', async () => { + process.env.PERCY_MAESTRO_BS_SCOPE_ROOT = `${SCOPE_ROOT}/`; + spyOn(percy, 'upload').and.resolveTo(); + await percy.start(); + + await expectAsync(postMaestro({ name: SS_NAME, sessionId: SID, platform: 'ios' })) + .toBeResolvedTo(jasmine.objectContaining({ success: true })); + + let [payload] = percy.upload.calls.mostRecent().args; + expect(payload.tiles[0].content).toBe(Buffer.from('PNGBYTES-SCOPE-ROOT').toString('base64')); + }); + + it('ignores a non-absolute override and keeps the composed convention', async () => { + process.env.PERCY_MAESTRO_BS_SCOPE_ROOT = 'relative/path'; + spyOn(percy, 'upload').and.resolveTo(); + await percy.start(); + + await expectAsync(postMaestro({ name: SS_NAME, sessionId: SID, platform: 'android' })) + .toBeResolvedTo(jasmine.objectContaining({ success: true })); + + let [payload] = percy.upload.calls.mostRecent().args; + // Fell through to the default /tmp glob, not a cwd-relative root + expect(payload.tiles[0].content).toBe(Buffer.from('PNGBYTES-ANDROID').toString('base64')); + }); + + it('re-anchors filePath containment on the overridden root', async () => { + // In-root filePath resolves, out-of-root does not: boundary moved, not removed. + fs.writeFileSync(path.join(REALMOBILE_DIR, `${FILEPATH_NAME}.png`), 'PNGBYTES-SCOPE-FILEPATH'); + spyOn(percy, 'upload').and.resolveTo(); + await percy.start(); + + await expectAsync(postMaestro({ + name: FILEPATH_NAME, + sessionId: SID, + platform: 'ios', + filePath: path.join(REALMOBILE_DIR, `${FILEPATH_NAME}.png`) + })).toBeResolvedTo(jasmine.objectContaining({ success: true })); + + let [payload] = percy.upload.calls.mostRecent().args; + expect(payload.tiles[0].content).toBe(Buffer.from('PNGBYTES-SCOPE-FILEPATH').toString('base64')); + + await expectAsync(postMaestro({ + name: FILEPATH_NAME, + sessionId: SID, + platform: 'ios', + filePath: `${IOS_FILEPATH_DIR}/${FILEPATH_NAME}.png` + })).toBeRejectedWithError(/Screenshot not found/); + }); + + it('404s when the overridden root does not exist', async () => { + process.env.PERCY_MAESTRO_BS_SCOPE_ROOT = path.join(os.tmpdir(), 'percy-bs-scope-missing'); + await percy.start(); + + await expectAsync(postMaestro({ name: SS_NAME, sessionId: SID, platform: 'ios' })) + .toBeRejectedWithError(/Screenshot not found/); + }); + + it('pins the trim + null semantics of the exported helper', () => { + process.env.PERCY_MAESTRO_BS_SCOPE_ROOT = `${SCOPE_ROOT}///`; + expect(bsScopeRootOverride()).toBe(SCOPE_ROOT); + process.env.PERCY_MAESTRO_BS_SCOPE_ROOT = '/'; + expect(bsScopeRootOverride()).toBeNull(); + process.env.PERCY_MAESTRO_BS_SCOPE_ROOT = ''; + expect(bsScopeRootOverride()).toBeNull(); + delete process.env.PERCY_MAESTRO_BS_SCOPE_ROOT; + expect(bsScopeRootOverride()).toBeNull(); + }); + }); + // PNG-header fill: relay reads IHDR from the screenshot and populates // payload.tag.width / payload.tag.height when missing. Source of truth // for tag dims is the PNG bytes themselves — what Percy stores and