[PIDM-418] chore: ACA and Stand In flag in getCreditorInstitutionsAssociatedToBroker API #68
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const execSync = require('child_process').execSync; | ||
| for (const file of IGNORED_FILES.trim().split(',')) { | ||
|
|
||
| const ignored_additions_str = execSync('git --no-pager diff --numstat origin/main..origin/' + BRANCH_NAME + ' | grep ' + file + ' | cut -f 1', {encoding: 'utf-8'}) |
Check warning
Code scanning / CodeQL
Indirect uncontrolled command line Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
The best and most robust fix is to avoid shell string interpolation entirely and use the child_process APIs that accept command and arguments as arrays, e.g., spawnSync or execFileSync, which do not invoke a shell and safely avoid command injection issues. For the specific use case here, git diff --numstat origin/main..origin/BRANCH_NAME and filtering the results for particular files, the simplest approach is to construct the command as an array without using the shell, and replicate the original command-line pipeline (especially the use of grep and cut) in native JavaScript (using .filter, .includes, .split, etc.), rather than shelling out with pipes.
Thus, replace:
- The use of
execSyncwith a string command involving| grep ... | cut ...
with - Extracting the diff output once (or per file), parsing it in JS, and filtering/selecting the data using standard JS functions.
This avoids any shell invocation and ensures values like BRANCH_NAME and filenames are passed as safe arguments.
Specific changes:
- For each ignored file, construct and run a command like
git --no-pager diff --numstat origin/main..origin/BRANCH_NAME
usingexecFileSyncorspawnSync. - Parse the output of
git diff --numstat ...in JS, filter for the lines belonging to the ignored file, then extract the additions and deletions columns. - Remove all shell piping and string command construction.
Required additions:
- Use
execFileSyncfromchild_process. - Parse the output text in JS for
tab-separated values.
| @@ -8,18 +8,31 @@ | ||
| const ignored_files = IGNORED_FILES.trim().split(',').filter(word => word.length > 0); | ||
| if (ignored_files.length > 0) { | ||
| var ignored = 0 | ||
| const execSync = require('child_process').execSync; | ||
| const execFileSync = require('child_process').execFileSync; | ||
| for (const file of IGNORED_FILES.trim().split(',')) { | ||
|
|
||
| const ignored_additions_str = execSync('git --no-pager diff --numstat origin/main..origin/' + BRANCH_NAME + ' | grep ' + file + ' | cut -f 1', {encoding: 'utf-8'}) | ||
| const ignored_deletions_str = execSync('git --no-pager diff --numstat origin/main..origin/' + BRANCH_NAME + ' | grep ' + file + ' | cut -f 2', {encoding: 'utf-8'}) | ||
| // Get diff for this branch | ||
| const diffOutput = execFileSync( | ||
| 'git', | ||
| ['--no-pager', 'diff', '--numstat', `origin/main..origin/${BRANCH_NAME}`], | ||
| { encoding: 'utf-8' } | ||
| ); | ||
|
|
||
| const ignored_additions = ignored_additions_str.split('\n').map(elem => parseInt(elem || 0)).reduce( | ||
| (accumulator, currentValue) => accumulator + currentValue, | ||
| 0); | ||
| const ignored_deletions = ignored_deletions_str.split('\n').map(elem => parseInt(elem || 0)).reduce( | ||
| (accumulator, currentValue) => accumulator + currentValue, | ||
| 0); | ||
| // Each line: <additions>\t<deletions>\t<filename> | ||
| const lines = diffOutput.split('\n'); | ||
| let ignored_additions = 0; | ||
| let ignored_deletions = 0; | ||
| for (const line of lines) { | ||
| if (!line.trim()) continue; | ||
| const [addStr, delStr, filename] = line.split('\t'); | ||
| // If the filename matches the ignored file exactly | ||
| if (filename === file) { | ||
| const add = parseInt(addStr) || 0; | ||
| const del = parseInt(delStr) || 0; | ||
| ignored_additions += add; | ||
| ignored_deletions += del; | ||
| } | ||
| } | ||
|
|
||
| ignored += ignored_additions + ignored_deletions; | ||
| } |
| for (const file of IGNORED_FILES.trim().split(',')) { | ||
|
|
||
| const ignored_additions_str = execSync('git --no-pager diff --numstat origin/main..origin/' + BRANCH_NAME + ' | grep ' + file + ' | cut -f 1', {encoding: 'utf-8'}) | ||
| const ignored_deletions_str = execSync('git --no-pager diff --numstat origin/main..origin/' + BRANCH_NAME + ' | grep ' + file + ' | cut -f 2', {encoding: 'utf-8'}) |
Check warning
Code scanning / CodeQL
Indirect uncontrolled command line Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
To fix the problem, we should avoid concatenating untrusted environment variable values into a shell command string. Instead, we should use child_process.execFileSync, which takes the command to run and arguments as an array, thereby bypassing the shell and eliminating the risk of command injection from environment variables.
In detail, the commands on lines 14 and 15 run multiple commands piped together, which execFileSync does not support directly. To keep the same functionality, we can:
- Use
execFileSyncorspawnSyncfor the initialgit diff --numstat ...command to obtain the diff output. - Parse the output in JS to filter by file and extract the columns required, eliminating the need for
grepandcut. - Do all filtering and column extraction in JavaScript, using safe values.
This approach removes the use of shell metacharacters and makes all argument passing explicit, so untrusted data cannot inject commands.
You'll need to replace the code constructing and executing the shell command, and move the file filtering and column extraction logic into JavaScript.
| @@ -9,18 +9,27 @@ | ||
| if (ignored_files.length > 0) { | ||
| var ignored = 0 | ||
| const execSync = require('child_process').execSync; | ||
| // Get the diff output once and parse in JS | ||
| const diffOutput = execSync( | ||
| 'git --no-pager diff --numstat origin/main..origin/' + BRANCH_NAME, | ||
| { encoding: 'utf-8' } | ||
| ); | ||
| for (const file of IGNORED_FILES.trim().split(',')) { | ||
|
|
||
| const ignored_additions_str = execSync('git --no-pager diff --numstat origin/main..origin/' + BRANCH_NAME + ' | grep ' + file + ' | cut -f 1', {encoding: 'utf-8'}) | ||
| const ignored_deletions_str = execSync('git --no-pager diff --numstat origin/main..origin/' + BRANCH_NAME + ' | grep ' + file + ' | cut -f 2', {encoding: 'utf-8'}) | ||
|
|
||
| const ignored_additions = ignored_additions_str.split('\n').map(elem => parseInt(elem || 0)).reduce( | ||
| (accumulator, currentValue) => accumulator + currentValue, | ||
| 0); | ||
| const ignored_deletions = ignored_deletions_str.split('\n').map(elem => parseInt(elem || 0)).reduce( | ||
| (accumulator, currentValue) => accumulator + currentValue, | ||
| 0); | ||
|
|
||
| // Each line looks like: "<added>\t<deleted>\t<filename>" | ||
| const lines = diffOutput.split('\n').filter(line => { | ||
| const parts = line.split('\t'); | ||
| // The filename might be in parts[2] | ||
| return parts[2] && parts[2].trim() === file.trim(); | ||
| }); | ||
| // Sum additions and deletions for these lines | ||
| const ignored_additions = lines.map(line => { | ||
| const parts = line.split('\t'); | ||
| return parseInt(parts[0] || '0', 10); | ||
| }).reduce((acc, curr) => acc + curr, 0); | ||
| const ignored_deletions = lines.map(line => { | ||
| const parts = line.split('\t'); | ||
| return parseInt(parts[1] || '0', 10); | ||
| }).reduce((acc, curr) => acc + curr, 0); | ||
| ignored += ignored_additions + ignored_deletions; | ||
| } | ||
| changes -= ignored |
|
1 participant
List of Changes
acaandstandInflag ingetCreditorInstitutionsAssociatedToBrokerAPI responseMotivation and Context
PIDM-418
Expose
acaandstandInflag in Backoffice External Creditor Institution exportHow Has This Been Tested?
Screenshots (if appropriate):
Types of changes
expected)
Checklist: