chore: rework mobile visreg to run on new expo app set up to run cds v9#553
chore: rework mobile visreg to run on new expo app set up to run cds v9#553cb-ekuersch merged 1 commit intocds-v9from
Conversation
|
|
||
| // Replace assets/index.android.bundle in the APK (cd into patchDir so zip path is correct) | ||
| console.log(`\nPatching bundle into APK: ${apk}...`); | ||
| execSync(`zip -u ${apk} assets/index.android.bundle`, { cwd: patchDir, stdio: 'inherit' }); |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 15 days ago
In general, the way to fix this is to avoid interpolating environment- or config-derived values into a single shell command string interpreted by a shell. Instead, invoke the underlying tool (zip, zipalign, apksigner) by passing the executable name and its arguments as an array to a function that does not go through the shell, or by using an existing helper that already handles this safely.
In this file, the best targeted fix is to replace the unsafe execSync calls with the existing run helper imported from ./shell.mjs, which is already used elsewhere in the class and appears to accept the command and arguments as separate parameters. Specifically:
- Replace
execSync(\zip -u ${apk} assets/index.android.bundle`, { cwd: patchDir, stdio: 'inherit' });withawait run('zip', ['-u', apk, 'assets/index.android.bundle'], { cwd: patchDir });`. - Replace
execSync(\zipalign -f 4 ${apk} ${alignedApk}`, { stdio: 'inherit' });withawait run('zipalign', ['-f', '4', apk, alignedApk]);`. - Replace the
apksignerexecSynccall withawait run('apksigner', ['sign', '--ks', debugKeystore, '--ks-pass', 'pass:android', '--key-pass', 'pass:android', apk]);.
These changes:
- Stop using the shell for these commands, eliminating injection via shell metacharacters or spaces.
- Preserve current behavior (same tools, same arguments, same working directory and visible output), assuming
runcorrectly forwards stdio, as implied by its use ininstallandlaunch. - Only touch the shown file and do not require new imports or dependencies.
Because we’re switching to await run(...), the surrounding function applyBundle is already async, so no further structural changes are needed.
| @@ -135,17 +135,23 @@ | ||
|
|
||
| // Replace assets/index.android.bundle in the APK (cd into patchDir so zip path is correct) | ||
| console.log(`\nPatching bundle into APK: ${apk}...`); | ||
| execSync(`zip -u ${apk} assets/index.android.bundle`, { cwd: patchDir, stdio: 'inherit' }); | ||
| await run('zip', ['-u', apk, 'assets/index.android.bundle'], { cwd: patchDir }); | ||
|
|
||
| // Re-align (zip modification breaks alignment) then re-sign with debug keystore | ||
| execSync(`zipalign -f 4 ${apk} ${alignedApk}`, { stdio: 'inherit' }); | ||
| await run('zipalign', ['-f', '4', apk, alignedApk]); | ||
| await fs.rename(alignedApk, apk); | ||
|
|
||
| const debugKeystore = path.resolve(process.env.HOME, '.android/debug.keystore'); | ||
| execSync( | ||
| `apksigner sign --ks ${debugKeystore} --ks-pass pass:android --key-pass pass:android ${apk}`, | ||
| { stdio: 'inherit' }, | ||
| ); | ||
| await run('apksigner', [ | ||
| 'sign', | ||
| '--ks', | ||
| debugKeystore, | ||
| '--ks-pass', | ||
| 'pass:android', | ||
| '--key-pass', | ||
| 'pass:android', | ||
| apk, | ||
| ]); | ||
|
|
||
| await fs.rm(patchDir, { recursive: true }); | ||
|
|
| execSync(`zip -u ${apk} assets/index.android.bundle`, { cwd: patchDir, stdio: 'inherit' }); | ||
|
|
||
| // Re-align (zip modification breaks alignment) then re-sign with debug keystore | ||
| execSync(`zipalign -f 4 ${apk} ${alignedApk}`, { stdio: 'inherit' }); |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 15 days ago
In general, the safest way to fix this kind of issue is to avoid building a single shell command string that is interpreted by a shell. Instead, pass the program and its arguments separately so no shell parsing happens, or use argument arrays for the helper you already have that does this safely (like the existing run function). This way, even if a path contains spaces or metacharacters, it is treated as a literal argument, not as part of the command language.
For this specific case, we should stop using execSync with a template string for the zipalign invocation and instead use the existing run helper that is already imported from ./shell.mjs and used safely elsewhere in the file (for adb). We can replace:
execSync(`zipalign -f 4 ${apk} ${alignedApk}`, { stdio: 'inherit' });with:
await run('zipalign', ['-f', '4', apk, alignedApk]);This preserves behavior (same command, same arguments) but avoids shell interpolation. Since run already exists and is imported, no new imports or helper functions are required. The change is localized to applyBundle in apps/test-expo/scripts/utils/AndroidBuilder.mjs, and we should mark the containing function async use as-is (it already is async), so adding an await is valid. No other lines need to change.
| @@ -138,7 +138,7 @@ | ||
| execSync(`zip -u ${apk} assets/index.android.bundle`, { cwd: patchDir, stdio: 'inherit' }); | ||
|
|
||
| // Re-align (zip modification breaks alignment) then re-sign with debug keystore | ||
| execSync(`zipalign -f 4 ${apk} ${alignedApk}`, { stdio: 'inherit' }); | ||
| await run('zipalign', ['-f', '4', apk, alignedApk]); | ||
| await fs.rename(alignedApk, apk); | ||
|
|
||
| const debugKeystore = path.resolve(process.env.HOME, '.android/debug.keystore'); |
|
|
||
| const debugKeystore = path.resolve(process.env.HOME, '.android/debug.keystore'); | ||
| execSync( | ||
| `apksigner sign --ks ${debugKeystore} --ks-pass pass:android --key-pass pass:android ${apk}`, |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 15 days ago
In general, fix this by avoiding passing a single interpolated string to a shell. Instead, invoke the underlying executable directly (without going through a shell) and pass all dynamic paths and options as separate arguments. This prevents spaces or special characters in paths from altering how the command is parsed.
For this file, we should:
-
Replace the
execSynccall that signs the APK:execSync( `apksigner sign --ks ${debugKeystore} --ks-pass pass:android --key-pass pass:android ${apk}`, { stdio: 'inherit' }, );
with an
execFileSynccall that passes arguments as an array:execFileSync('apksigner', [ 'sign', '--ks', debugKeystore, '--ks-pass', 'pass:android', '--key-pass', 'pass:android', apk, ], { stdio: 'inherit' });
This removes shell interpolation of
debugKeystoreandapk. -
Add
execFileSyncto the existing import fromnode:child_processat the top ofAndroidBuilder.mjs. Do not change any other behavior: we keep the same options (stdio: 'inherit') and arguments; only the invocation mechanism changes.
No other lines need modification to address the reported alerts.
| @@ -1,4 +1,4 @@ | ||
| import { execSync, spawn } from 'node:child_process'; | ||
| import { execSync, execFileSync, spawn } from 'node:child_process'; | ||
| import fs from 'node:fs/promises'; | ||
| import path from 'node:path'; | ||
|
|
||
| @@ -142,8 +142,9 @@ | ||
| await fs.rename(alignedApk, apk); | ||
|
|
||
| const debugKeystore = path.resolve(process.env.HOME, '.android/debug.keystore'); | ||
| execSync( | ||
| `apksigner sign --ks ${debugKeystore} --ks-pass pass:android --key-pass pass:android ${apk}`, | ||
| execFileSync( | ||
| 'apksigner', | ||
| ['sign', '--ks', debugKeystore, '--ks-pass', 'pass:android', '--key-pass', 'pass:android', apk], | ||
| { stdio: 'inherit' }, | ||
| ); | ||
|
|
What changed? Why?
Root cause (required for bugfixes)
UI changes
Testing
How has it been tested?
Testing instructions
Illustrations/Icons Checklist
Required if this PR changes files under
packages/illustrations/**orpackages/icons/**Change management
type=routine
risk=low
impact=sev5
automerge=false