Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 16 additions & 9 deletions .github/workflows/all-ci-unsafe.yml
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,19 @@ jobs:
}
}

if (core.getOutput('should_run_deps_scan') === 'true') {
const files = await github.paginate(github.rest.pulls.listFiles, {
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: Number(prNumber),
});
const hasLockfileChange = files.some(f => f.filename === 'package-lock.json');
if (!hasLockfileChange) {
core.setOutput(`should_run_${output}`, 'false');
core.setOutput(`skip_reason_${output}`, 'package-lock.json not changed');
}
}
Comment on lines +161 to +172
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Bug: output variable is out of scope.

The variable output was declared inside the for...of loop (via destructuring in line 148), but this code block is outside the loop. After the loop ends at line 159, output is no longer in scope, causing this to use undefined in the template string.

🐛 Proposed fix
             if (core.getOutput('should_run_deps_scan') === 'true') {
               const files = await github.paginate(github.rest.pulls.listFiles, {
                 owner: context.repo.owner,
                 repo: context.repo.repo,
                 pull_number: Number(prNumber),
               });
               const hasLockfileChange = files.some(f => f.filename === 'package-lock.json');
               if (!hasLockfileChange) {
-                core.setOutput(`should_run_${output}`, 'false');
-                core.setOutput(`skip_reason_${output}`, 'package-lock.json not changed');
+                core.setOutput('should_run_deps_scan', 'false');
+                core.setOutput('skip_reason_deps_scan', 'package-lock.json not changed');
               }
             }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (core.getOutput('should_run_deps_scan') === 'true') {
const files = await github.paginate(github.rest.pulls.listFiles, {
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: Number(prNumber),
});
const hasLockfileChange = files.some(f => f.filename === 'package-lock.json');
if (!hasLockfileChange) {
core.setOutput(`should_run_${output}`, 'false');
core.setOutput(`skip_reason_${output}`, 'package-lock.json not changed');
}
}
if (core.getOutput('should_run_deps_scan') === 'true') {
const files = await github.paginate(github.rest.pulls.listFiles, {
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: Number(prNumber),
});
const hasLockfileChange = files.some(f => f.filename === 'package-lock.json');
if (!hasLockfileChange) {
core.setOutput('should_run_deps_scan', 'false');
core.setOutput('skip_reason_deps_scan', 'package-lock.json not changed');
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/all-ci-unsafe.yml around lines 161 - 172, The bug is that
the template string uses the loop variable `output` which is out of scope (it
was declared by destructuring inside the for...of loop), so replace the
out-of-scope reference by using a properly-scoped name (capture the loop's
output value into a variable in the outer scope or move this github.paginate +
core.setOutput block inside the same for...of loop where `output` is defined).
Specifically, ensure the code that calls
github.paginate(github.rest.pulls.listFiles, ...) and the subsequent
core.setOutput(`should_run_${output}`, ...) /
core.setOutput(`skip_reason_${output}`, ...) either runs inside the loop that
defines `output` or use a new variable (e.g., const outputName = output)
declared in the same scope before using it so `output` is defined when setting
outputs.


- name: Build matrix from changed files
if: steps.check_label.outputs.should_run_static_analyses == 'true'
id: matrix
Expand Down Expand Up @@ -187,7 +200,7 @@ jobs:

security-scan:
needs: [setup]
if: ${{ needs.setup.outputs.should_run_static_analyses == 'true' && needs.setup.outputs.matrix != '' }}
if: ${{ needs.setup.outputs.matrix != '' }}
strategy:
matrix: ${{ fromJson(needs.setup.outputs.matrix) }}
fail-fast: false
Expand All @@ -199,7 +212,8 @@ jobs:
path: apps/${{ matrix.workspace }}
pr_head_ref: ${{ needs.setup.outputs.pr_head_ref }}
pr_head_repo: ${{ needs.setup.outputs.pr_head_repo }}
skip_change_detection: true
run_static_analyses: ${{ needs.setup.outputs.should_run_static_analyses == 'true'}}
run_deps_scan: ${{ needs.setup.outputs.should_run_deps_scan == 'true' }}

apps-deps-scan:
needs: [setup]
Expand All @@ -212,13 +226,6 @@ jobs:
sparse-checkout: |
.github

- name: Check snyk status
uses: ./.github/actions/poll-check-status
with:
name: "security/snyk (corysturtevant)"
sha: ${{ github.event.workflow_run.head_sha }}
check_type: status

- name: Check socketio status
uses: ./.github/actions/poll-check-status
with:
Expand Down
41 changes: 27 additions & 14 deletions .github/workflows/reusable-validate-app-unsafe.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,13 @@
description: "PR head repository full name (optional, defaults to github.event.pull_request.head.repo.full_name)"
required: false
type: string
skip_change_detection:
description: "Skip change detection and always run (useful when called from workflow_run context)"
run_static_analyses:
description: "Run static analyses (Snyk Code)"
required: false
type: boolean
default: false
run_deps_scan:
description: "Run dependencies scan (Snyk)"
required: false
type: boolean
default: false
Expand All @@ -34,32 +39,40 @@
required: true

jobs:
should-validate-unsafe:
if: ${{ !inputs.skip_change_detection }}
uses: ./.github/workflows/reusable-should-validate.yml
secrets:
gh-token: ${{ secrets.gh-token }}
with:
path: ${{ inputs.path }}

validate-unsafe:
needs: should-validate-unsafe
static-analyses:
if: ${{ inputs.run_static_analyses }}
runs-on: ubuntu-latest
if: always() && (inputs.skip_change_detection || needs.should-validate-unsafe.outputs.has_changes == 'true')
steps:
- name: Checkout UNTRUSTED user's fork
uses: actions/checkout@v4
uses: actions/checkout@v6
with:
ref: ${{ inputs.pr_head_ref || github.event.pull_request.head.ref }}
repository: ${{ inputs.pr_head_repo || github.event.pull_request.head.repo.full_name }}
path: ./untrusted-user-fork
- name: Copy .dcignore
shell: bash
run: cp ./untrusted-user-fork/.dcignore ./untrusted-user-fork/${{ inputs.path }}/.dcignore
- name: Test code for security issues
uses: snyk/actions/node@4a528b5c534bb771b6e3772656a8e0e9dc902f8b
env:
SNYK_TOKEN: ${{ secrets.snyk-token }}
with:
command: code
args: test ./untrusted-user-fork/${{ inputs.path }} --severity-threshold=medium
apps-deps-scan:
Comment on lines 43 to +62

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}
if: ${{ inputs.run_deps_scan }}
runs-on: ubuntu-latest
steps:
- name: Checkout UNTRUSTED user's fork
uses: actions/checkout@v6
with:
ref: ${{ inputs.pr_head_ref || github.event.pull_request.head.ref }}
repository: ${{ inputs.pr_head_repo || github.event.pull_request.head.repo.full_name }}
path: ./untrusted-user-fork
- name: Validate dependencies for security issues
uses: snyk/actions/node@4a528b5c534bb771b6e3772656a8e0e9dc902f8b
env:
SNYK_TOKEN: ${{ secrets.snyk-token }}
with:
command: test
args: --file ./untrusted-user-fork/${{ inputs.path }}/package.json
Comment on lines 63 to 78

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}
Loading