Mobile visreg by cb-ekuersch · Pull Request #517 · coinbase/cds

cb-ekuersch · 2026-03-18T03:41:18Z

What changed? Why?

Root cause (required for bugfixes)

UI changes

iOS Old	iOS New
old screenshot	new screenshot

Android Old	Android New
old screenshot	new screenshot

Web Old	Web New
old screenshot	new screenshot

Testing

How has it been tested?

Testing instructions

Illustrations/Icons Checklist

Required if this PR changes files under packages/illustrations/** or packages/icons/**

verified visreg changes with Terran (include link to visreg run/approval)
all illustration/icons names have been reviewed by Dom and/or Terran

Change management

type=routine
risk=low
impact=sev5

automerge=false

cb-heimdall · 2026-03-18T03:41:21Z

🟡 Heimdall Review Status

Requirement Status More Info

Reviews

🟡 0/1

Denominator calculation

Show calculation

1 if user is bot 0

1 if user is external 0

2 if repo is sensitive 0

From .codeflow.yml 1

Additional review requirements

Show calculation

Max	0

0

From CODEOWNERS 1

Global minimum 0

Max 1

1

1 if commit is unverified 0

Sum 1

CODEOWNERS 🟡 See below

🟡 `CODEOWNERS`

Code Owner Status Calculation

ui-systems-eng-team

🟡 0/1

Denominator calculation

Additional CODEOWNERS Requirement

Show calculation

Sum	0

0

From CODEOWNERS 1

Sum 1

packages/mobile-visreg/src/run.mjs

+const cmd = ['maestro', 'test', flowPath, ...envFlags.map((e) => `--env ${e}`)].join(' ');
+
+console.log(`\nRunning: ${cmd}\n`);
+execSync(cmd, { stdio: 'inherit', cwd: outputDir });


In general, the fix is to avoid constructing a single shell command string and instead call execFileSync (or spawnSync) with the command and its arguments as separate array elements. This way, Node passes arguments directly to the process without an intervening shell, so special characters in paths or environment values cannot change command structure. For this code, the best fix is to change the Maestro invocation so that maestro is the executable and the rest of the tokens (test, flowPath, --env, KEY=VALUE) are elements in an array, then use execFileSync instead of execSync. This preserves functionality but removes shell interpretation.

Concretely in packages/mobile-visreg/src/run.mjs:

Add execFileSync to the import from child_process.

Replace the string-building cmd line with an array of arguments: ['test', flowPath, ...envFlags.flatMap(e => ['--env', e])].

Replace the execSync(cmd, ...) call with execFileSync('maestro', args, { stdio: 'inherit', cwd: outputDir });.

Keep the existing console log of the human-readable command string (or reconstruct it from the args) for visibility if desired, but this log should not be executed by the shell.

The earlier execSync that runs node src/generate-flows.mjs ${platform} is technically similar, but it does not involve the tainted flowPath/absolute path and only injects platform (which is limited to 'ios' by default or whatever the caller supplies). Since CodeQL’s specific alerts here are about the Maestro command, we will leave that first execSync unchanged to avoid unrequested behavioral changes.

packages/mobile-visreg/src/upload.mjs

+const screenshotDir = resolve(dir);
+console.log(`Uploading screenshots from ${screenshotDir} to Percy...`);
+
+execSync(`npx percy upload ${screenshotDir}`, { stdio: 'inherit' });


In general, to fix shell command injection or misinterpretation issues, avoid constructing a single shell command string that includes untrusted values. Instead, invoke the underlying program directly (bypassing the shell) and pass untrusted values as separate arguments in an array, using child_process.execFile / execFileSync or spawn / spawnSync. This prevents the shell from interpreting special characters in the arguments.

For this specific case in packages/mobile-visreg/src/upload.mjs, replace execSync with execFileSync from child_process, and change the invocation from a template string to a command plus arguments array:

Import execFileSync instead of or in addition to execSync at the top of the file.

Replace execSync(\npx percy upload ${screenshotDir}`, { stdio: 'inherit' });withexecFileSync('npx', ['percy', 'upload', screenshotDir], { stdio: 'inherit' });`.

This keeps all existing behavior (still runs npx percy upload <dir> and inherits stdio) but avoids the shell entirely, so screenshotDir is treated as a literal argument. All changes are confined to packages/mobile-visreg/src/upload.mjs within the shown snippet, and no additional methods are needed beyond the new import.

github-actions · 2026-03-23T03:34:05Z

https://percy.io/d424b72f/web/cds-mobile-visreg-03c97426/builds/48124863

cb-ekuersch temporarily deployed to production March 18, 2026 03:41 — with GitHub Actions Inactive

github-actions bot added root apps:mobile-app labels Mar 18, 2026

github-advanced-security bot found potential problems Mar 18, 2026

View reviewed changes

cb-ekuersch force-pushed the mobile-visreg branch from 94dd9a3 to 30d2737 Compare March 18, 2026 14:53

cb-ekuersch temporarily deployed to production March 18, 2026 14:53 — with GitHub Actions Inactive

cb-ekuersch temporarily deployed to production March 18, 2026 15:15 — with GitHub Actions Inactive

cb-ekuersch had a problem deploying to production March 18, 2026 15:15 — with GitHub Actions Failure

cb-ekuersch temporarily deployed to production March 18, 2026 16:00 — with GitHub Actions Inactive

cb-ekuersch had a problem deploying to production March 18, 2026 16:02 — with GitHub Actions Failure

cb-ekuersch temporarily deployed to production March 18, 2026 16:30 — with GitHub Actions Inactive

cb-ekuersch had a problem deploying to production March 18, 2026 16:30 — with GitHub Actions Failure

cb-ekuersch temporarily deployed to production March 18, 2026 19:55 — with GitHub Actions Inactive

cb-ekuersch had a problem deploying to production March 18, 2026 19:56 — with GitHub Actions Failure

cb-ekuersch had a problem deploying to production March 18, 2026 20:44 — with GitHub Actions Failure

cb-ekuersch temporarily deployed to production March 18, 2026 20:50 — with GitHub Actions Inactive

cb-ekuersch had a problem deploying to production March 18, 2026 20:50 — with GitHub Actions Failure

cb-ekuersch temporarily deployed to production March 18, 2026 20:50 — with GitHub Actions Inactive

cb-ekuersch had a problem deploying to production March 18, 2026 21:03 — with GitHub Actions Error

cb-ekuersch force-pushed the mobile-visreg branch from 1d6940e to d0b8775 Compare March 18, 2026 21:04

cb-ekuersch temporarily deployed to production March 18, 2026 21:04 — with GitHub Actions Inactive

cb-ekuersch temporarily deployed to production March 18, 2026 21:23 — with GitHub Actions Inactive

cb-ekuersch temporarily deployed to production March 22, 2026 02:57 — with GitHub Actions Inactive

cb-ekuersch temporarily deployed to production March 22, 2026 16:13 — with GitHub Actions Inactive

cb-ekuersch had a problem deploying to production March 22, 2026 16:13 — with GitHub Actions Failure

cb-ekuersch temporarily deployed to production March 22, 2026 16:13 — with GitHub Actions Inactive

cb-ekuersch had a problem deploying to production March 22, 2026 17:06 — with GitHub Actions Failure

cb-ekuersch temporarily deployed to production March 22, 2026 17:06 — with GitHub Actions Inactive

cb-ekuersch had a problem deploying to production March 22, 2026 17:06 — with GitHub Actions Failure

cb-ekuersch force-pushed the mobile-visreg branch from 20d0147 to 1cebb84 Compare March 22, 2026 17:50

cb-ekuersch had a problem deploying to production March 22, 2026 17:50 — with GitHub Actions Failure

cb-ekuersch temporarily deployed to production March 22, 2026 17:50 — with GitHub Actions Inactive

cb-ekuersch temporarily deployed to production March 22, 2026 17:54 — with GitHub Actions Inactive

cb-ekuersch added 16 commits March 25, 2026 11:41

feat: add mobile-visreg package with maestro test suite

aec5ff5

clean up detox implementation

133bd5a

run on any pr to master

6335554

fix generated flow first assertion

f8be4cf

fix issues in workflow

03b5dc8

add flow to get around ios open link security modal

3350dbb

debug with two flows after tweak to configmr modal flow

e498305

remove legacy e2e testing setup in mobile-app

bebea63

trying something new

f1237c0

testing

d5a29c6

scorlling

c2b8eb6

remove limited on routes for visreg

bde969c

limited routes

39dcd8b

increase scroll timeout

aeaa949

remove slow pages and bring back percy PR step for testing:

215c217

remove more slow routes

4c49e68

@@ -1,4 +1,4 @@
-            import { execSync } from 'child_process';
+            import { execSync, execFileSync } from 'child_process';
             import { mkdirSync, readdirSync } from 'fs';
             import { resolve, dirname } from 'path';
             import { fileURLToPath } from 'url';
@@ -50,10 +50,11 @@
               envFlags.push(`ROUTE_NAME=${route}`);
             }
-            const cmd = ['maestro', 'test', flowPath, ...envFlags.map((e) => `--env ${e}`)].join(' ');
+            const maestroArgs = ['test', flowPath, ...envFlags.flatMap((e) => ['--env', e])];
+            const cmd = ['maestro', ...maestroArgs].join(' ');
             console.log(`\nRunning: ${cmd}\n`);
-            execSync(cmd, { stdio: 'inherit', cwd: outputDir });
+            execFileSync('maestro', maestroArgs, { stdio: 'inherit', cwd: outputDir });
             const screenshots = readdirSync(outputDir).filter((f) => f.endsWith('.png'));
             console.log(`\nCapture complete: ${screenshots.length} screenshots in ${outputDir}`);

@@ -1,4 +1,4 @@
-            import { execSync } from 'child_process';
+            import { execFileSync } from 'child_process';
             import { resolve } from 'path';
             function parseArgs() {
@@ -24,6 +24,6 @@
             const screenshotDir = resolve(dir);
             console.log(`Uploading screenshots from ${screenshotDir} to Percy...`);
-            execSync(`npx percy upload ${screenshotDir}`, { stdio: 'inherit' });
+            execFileSync('npx', ['percy', 'upload', screenshotDir], { stdio: 'inherit' });
             console.log('\nUpload complete. Visit percy.io to review the build.');

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mobile visreg#517

Mobile visreg#517
cb-ekuersch wants to merge 16 commits intomasterfrom
mobile-visreg

cb-ekuersch commented Mar 18, 2026

Uh oh!

cb-heimdall commented Mar 18, 2026 •

edited

Loading

Uh oh!

Check warning

Copilot Autofix

Check warning

Copilot Autofix

github-actions bot commented Mar 23, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Conversation

cb-ekuersch commented Mar 18, 2026

What changed? Why?

Root cause (required for bugfixes)

UI changes

Testing

How has it been tested?

Testing instructions

Illustrations/Icons Checklist

Change management

Uh oh!

cb-heimdall commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟡 Heimdall Review Status

🟡 CODEOWNERS

Uh oh!

Check warning

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot Autofix

Check warning

Uh oh!

Copilot Autofix

github-actions bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

cb-heimdall commented Mar 18, 2026 •

edited

Loading

🟡 `CODEOWNERS`

github-actions bot commented Mar 23, 2026 •

edited

Loading