Skip to content

Fix command injection in required-field-check workflow #3642

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
varadarajan-tw wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix command injection in required-field-check workflow #3642

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e8f0c78
Fix command injection in required-field-check workflow (CWE-78)
varadarajan-tw Mar 3, 2026
3a625c7
Skip required-field warnings for new destinations and new actions
varadarajan-tw Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 35 additions & 28 deletions .github/workflows/required-field-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,22 +39,30 @@ jobs:
pull_number: context.issue.number
})
const files = response.data.map(file => file.filename)
core.setOutput('files', files.join(' '))
core.setOutput('files', JSON.stringify(files))
if (files.length === 0) {
core.setOutput('no_files_changed', true)
}

- name: Find required fields on current branch
id: required-fields-curr-branch
if: steps.get_files.outputs.no_files_changed != 'true'
env:
CHANGED_FILES: ${{ steps.get_files.outputs.files }}
run: |
set -e
REQUIRED_FIELDS_CURR_BRANCH=$(./bin/run list-required-fields -p ${{ steps.get_files.outputs.files }} | jq -c .)
ARGS=()
while IFS= read -r file; do
ARGS+=(-p "$file")
done < <(echo "$CHANGED_FILES" | jq -r '.[]')
REQUIRED_FIELDS_CURR_BRANCH=$(./bin/run list-required-fields "${ARGS[@]}" | jq -c .)
echo "REQUIRED_FIELDS_CURR_BRANCH=$REQUIRED_FIELDS_CURR_BRANCH" >> $GITHUB_ENV

- name: Check for required fields on main branch
id: required-fields-main-branch
if: steps.get_files.outputs.no_files_changed != 'true'
env:
CHANGED_FILES: ${{ steps.get_files.outputs.files }}
run: |
set -e
git checkout main
Expand All @@ -65,7 +73,11 @@ jobs:
# checked against the correct version of the dependencies
yarn install --frozen-lockfile --silent
# Run the list-required-fields command on the main branch to get the required fields
REQUIRED_FIELDS_MAIN_BRANCH=$(./bin/run list-required-fields -p ${{ steps.get_files.outputs.files }} | jq -c .)
ARGS=()
while IFS= read -r file; do
ARGS+=(-p "$file")
done < <(echo "$CHANGED_FILES" | jq -r '.[]')
REQUIRED_FIELDS_MAIN_BRANCH=$(./bin/run list-required-fields "${ARGS[@]}" | jq -c .)
echo "REQUIRED_FIELDS_MAIN_BRANCH=$REQUIRED_FIELDS_MAIN_BRANCH" >> $GITHUB_ENV

- name: Add comment on PR if there is diff in required fields
Expand All @@ -82,34 +94,29 @@ jobs:
const requiredFieldsOnMain = JSON.parse(process.env.REQUIRED_FIELDS_MAIN_BRANCH)

Object.keys(requiredFieldsOnBranch).forEach(key => {
// Check if key is present in requiredFieldsOnMain
if(requiredFieldsOnMain[key]) {
const getActionKeys = Object.keys(requiredFieldsOnBranch[key])
for(const actionKey of getActionKeys) {
const branchRequiredFields = requiredFieldsOnBranch[key][actionKey]
const mainRequiredFields = requiredFieldsOnMain[key][actionKey]
const diff = branchRequiredFields.filter(field => !mainRequiredFields?.includes(field))
if(diff.length > 0) {
const isSettingsKey = actionKey === 'settings'
if (isSettingsKey) {
fieldsAdded.push(`- **Destination**: ${key}, **Settings**:${diff.join(',')}`)
} else {
fieldsAdded.push(`- **Destination**: ${key}, Action **Field(s)**:${diff.join(',')}`)
}
}
// Skip new destinations — no existing customers to break
if(!requiredFieldsOnMain[key]) {
return
}

const getActionKeys = Object.keys(requiredFieldsOnBranch[key])
for(const actionKey of getActionKeys) {
// Skip new actions — no existing configurations to break
if(!requiredFieldsOnMain[key][actionKey]) {
continue
}
} else {

// If key is not present in requiredFieldsOnMain, then all fields are added recently
const getActionKeys = Object.keys(requiredFieldsOnBranch[key])
for(const actionKey of getActionKeys) {
const branchRequiredFields = requiredFieldsOnBranch[key][actionKey]
if(actionKey === 'settings') {
fieldsAdded.push(`- **Destination**: ${key}, **Settings**:${branchRequiredFields.join(',')}`)

const branchRequiredFields = requiredFieldsOnBranch[key][actionKey]
const mainRequiredFields = requiredFieldsOnMain[key][actionKey]
const diff = branchRequiredFields.filter(field => !mainRequiredFields.includes(field))
if(diff.length > 0) {
const isSettingsKey = actionKey === 'settings'
if (isSettingsKey) {
fieldsAdded.push(`- **Destination**: ${key}, **Settings**:${diff.join(',')}`)
} else {
fieldsAdded.push(`- **Destination**: ${key}, **Action**:${actionKey}, **Fields**:${branchRequiredFields.join(',')}`)
fieldsAdded.push(`- **Destination**: ${key}, Action **Field(s)**:${diff.join(',')}`)
}
}
}
}
})
}
Expand Down
Loading