fix(javascript-lang-security-detect-child-process-detect-child-process): Shell Injection via Unquoted xcframeworkPath in codesign Command - #33
Open
zepto-gaurav wants to merge 1 commit into
Conversation
…s): Shell Injection via Unquoted xcframeworkPath in codesign Command Fixes 1 finding(s): 9c9e0776 Auto-generated by Optimus AutoFix Agent
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Optimus AutoFix — Automated Security Fix
Findings Fixed
Fix Strategy
Shell Injection via Unquoted xcframeworkPath in codesign Command
Changes Made
require('child_process')imports and everyexecSynccall site. Then apply the following changes:Update the
child_processimport (wherever it appears near the top of the file): addexecFileSyncalongside any existingexecSyncimport. IfexecSyncis no longer used after the fixes below, remove it from the import.Fix
signXCFramework(lines 164–171): Replace the shell-string construction +execSynccall entirely withexecFileSync, which bypasses the shell interpreter:This eliminates shell interpretation for BOTH
identityandxcframeworkPath— double-quotingidentitywas insufficient because$(...)or embedded"could still break out. Passing all arguments as discrete array elements removes the shell from the execution path entirely.Fix
copyBundles(line 155): The same vulnerable pattern is present —execSyncwith a template-literal shell command. Even though these paths derive frompath.join(), they still pass through/bin/sh. Replace with:No shell, no quoting needed, no injection surface.
Do NOT change any other logic, call signatures, or surrounding code. The behavioral output of both functions is identical — only the execution mechanism changes (execFileSync bypasses /bin/sh).
AI Review
signXCFrameworkandcopyBundlesare migrated from template-literal shell strings passed toexecSyncto discrete argument arrays passed toexecFileSync, which bypasses/bin/shentirely. The root cause (shell interpretation) is eliminated rather than band-aided with additional quoting. Argument ordering, thestdio: 'inherit'option, and the trailing/for thecp -rcall are all preserved correctly. One minor cleanliness issue is noted below.Issues
execSyncimport is retained but appears to have no remaining uses in the file after both call sites were migrated toexecFileSync. If there are no otherexecSynccalls elsewhere in this file, the import should be cleaned up (const {execFileSync} = require('child_process')). This is a dead-import / code hygiene issue, not a security concern, but the fix plan explicitly called out removing it if unused.Risk Assessment
child_process.execSyncand exercisesignXCFrameworkorcopyBundleswill need to be updated to mockexecFileSyncinstead. Unit tests that only check return values or console output are unaffected.Auto-generated by Optimus AutoFix Agent. Review carefully before merging.