Skip to content

Conversation

@cosmir17
Copy link
Contributor

Summary

This PR introduces a new Checkmarx action variant that enables fork PRs to run security scans on public repositories without requiring access to secrets.

Problem (PM-19178)

  • Fork PRs cannot access repository secrets (GitHub security feature)
  • Checkmarx requires authentication credentials to run
  • External contributors get failing CI, defeating the purpose of open source

Solution (PM-19431)

Created checkmarx-scan-public action that:

  • Does NOT checkout code (critical for security)
  • Uses Checkmarx CLI to scan repositories directly via URL
  • Works with pull_request_target to access secrets safely
  • Allows fork PRs to get green CI

Technical Approach

Instead of:

  1. Checkout code locally
  2. Scan local files

We do:

  1. Pass repo URL to Checkmarx CLI
  2. CLI fetches code using: cx scan create -s <repo-url> --branch <branch>
  3. Scan happens in Checkmarx environment

Security Model

  • pull_request_target provides secret access (runs with base branch context)
  • No code checkout = no risk of running untrusted code with elevated permissions
  • Checkmarx fetches and scans the code in their isolated environment

Changes

  • Added checkmarx-scan-public/action.yml - New composite action for fork-friendly scanning
  • Added checkmarx-scan-public/README.md - Documentation and usage examples

Testing Plan

  1. Merge this PR to main
  2. Update midnight-node-docker workflow to use pull_request_target with new action
  3. Create a fork PR to validate it works
  4. Roll out to other public repos

Usage Example

on:
  pull_request_target:  # Important: not pull_request
    types: [opened, synchronize, reopened]

jobs:
  checkmarx:
    steps:
      # NO checkout step - critical for security

      - uses: midnightntwrk/upload-sarif-github-action/checkmarx-scan-public@main
        with:
          cx-client-id: ${{ secrets.CX_CLIENT_ID }}
          cx-client-secret: ${{ secrets.CX_CLIENT_SECRET_EU }}
          cx-tenant: ${{ secrets.CX_TENANT }}

Notes

  • Only needed for public repos (private repos don't have fork issues)
  • Requires merging to main before we can fully test with real fork PRs
  • midnight-node-docker will be our first test case (per @gilescope)

Implements solution for fork PR scanning without secrets exposure:
- New checkmarx-scan-public action that doesn't checkout code
- Uses Checkmarx CLI to scan remote repositories directly
- Safe for use with pull_request_target in public repos
- Allows fork PRs to run Checkmarx scans

Solves the fork conundrum (PM-19178) where external contributors
couldn't get green CI due to missing secrets access.
@cosmir17 cosmir17 requested a review from gilescope September 22, 2025 22:48
@cosmir17 cosmir17 self-assigned this Sep 22, 2025
@cosmir17 cosmir17 requested a review from a team as a code owner September 22, 2025 22:48
@github-actions
Copy link

github-actions bot commented Sep 22, 2025

Logo
Checkmarx One – Scan Summary & Detailsb433ac69-5111-4b9a-b279-f1641a30fa22

Great job! No new security vulnerabilities introduced in this pull request

cosmir17 added a commit to midnightntwrk/midnight-node-docker that referenced this pull request Sep 22, 2025
Testing PM-19431 solution for fork PR scanning:
- Changed pull_request to pull_request_target
- Removed main code checkout (critical for security)
- Uses new checkmarx-scan-public action from sean/PM-19431-fork-friendly-checkmarx branch
- This will allow fork PRs to run Checkmarx scans

Note: Depends on midnightntwrk/upload-sarif-github-action#25
Signed-off-by: Squirrel <giles.cope@shielded.io>
Comment on lines +126 to +129
if [ -n "${{ inputs.additional-params }}" ]; then
SCAN_CMD="$SCAN_CMD ${{ inputs.additional-params }}"
fi

Copy link
Contributor

Choose a reason for hiding this comment

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

additional params default to '' so no need for the if:

Suggested change
if [ -n "${{ inputs.additional-params }}" ]; then
SCAN_CMD="$SCAN_CMD ${{ inputs.additional-params }}"
fi
SCAN_CMD="$SCAN_CMD ${{ inputs.additional-params }}"

gilescope
gilescope previously approved these changes Sep 23, 2025
gilescope pushed a commit to midnightntwrk/midnight-node-docker that referenced this pull request Sep 23, 2025
Testing PM-19431 solution for fork PR scanning:
- Changed pull_request to pull_request_target
- Removed main code checkout (critical for security)
- Uses new checkmarx-scan-public action from sean/PM-19431-fork-friendly-checkmarx branch
- This will allow fork PRs to run Checkmarx scans

Note: Depends on midnightntwrk/upload-sarif-github-action#25
MB-IOHK
MB-IOHK previously approved these changes Sep 23, 2025
Per Giles' standup feedback, the CLI was running different scan types
than the original action, resulting in fewer issues being detected.

Added --scan-types parameter with sast,sca,kics to ensure comprehensive
security scanning matching the original behavior.

This ensures fork PRs get the same security coverage as regular PRs.
@cosmir17 cosmir17 dismissed stale reviews from MB-IOHK and gilescope via f94b911 September 23, 2025 18:55
@cosmir17
Copy link
Contributor Author

Added fix for scan types issue mentioned in standup.

The CLI was using different defaults than the GitHub Action, resulting in fewer vulnerabilities being detected. Fixed by explicitly specifying --scan-types sast,sca,kics to ensure:

  • SAST (Static Application Security Testing)
  • SCA (Software Composition Analysis)
  • KICS (Infrastructure as Code Security)

This matches the original action's security coverage.

Note: Scorecard/SCS is still not included as it requires additional tokens that fork PRs wouldn't have access to. Per @gilescope's suggestion, we can add actionlint as an alternative validation tool in a follow-up PR.

@gilescope @MB-IOHK - This push has made your approvals stale. Could you please re-review? The only change is adding the scan types parameter to fix the issue Giles identified in standup.

cosmir17 added a commit that referenced this pull request Sep 23, 2025
Per Giles' suggestion in PR #25, adding actionlint as a lightweight
alternative to scorecard/SCS validation that works with fork PRs.

- Downloads and runs actionlint for workflow validation
- Integrates shellcheck and pyflakes for script validation
- Creates GitHub annotations for discovered issues
- No special tokens required (fork-friendly)
MB-IOHK
MB-IOHK previously approved these changes Sep 24, 2025
@cosmir17
Copy link
Contributor Author

cosmir17 commented Sep 25, 2025

Thanks for the approval @MB-IOHK!

@gilescope identified an issue with SCS/Scorecard for fork PRs:

  1. SCS fails because MIDNIGHTCI_REPO token doesn't have permissions for fork repositories
  2. It appears to check the fork's main branch instead of the PR branch (bug)

Giles suggested we try using GITHUB_TOKEN instead of MIDNIGHTCI_REPO for the SCS token before making it conditional.

Plan to fix in this PR:

  1. Add SCS parameters to checkmarx-scan-public action
  2. Try using --scs-repo-token ${{ github.token }} for public repos
  3. If that doesn't work, make SCS conditional (only for non-fork PRs)

I'll push these changes to complete PM-19431 properly. The core Checkmarx scanning works for forks, just need to fix the SCS component.

- Add scs-repo-token input parameter with github.token fallback
- Include SCS parameters in scan command when token is available
- Display SCS status in scan parameters output
- Use GITHUB_TOKEN for public fork PRs as suggested by Giles

This completes PM-19431 by enabling Supply Chain Security scanning
for fork PRs using the GitHub token which has read access to public
repositories.
Use the repo-url input (which points to the fork) instead of github.repository
(which always points to the base repo) for SCS scanning. This ensures SCS
analyzes the correct repository when scanning fork PRs.
@cosmir17
Copy link
Contributor Author

@MB-IOHK @gilescope, Thanks for your feedback about using github.token for SCS. You were right - it works for public fork PRs!

✅ Tested and Working

I've successfully tested this implementation with a real fork PR: midnightntwrk/midnight-node-docker#58

What's been fixed:

  1. Added SCS support with github.token fallback (commit 3afeaae)

    • Falls back to github.token when no explicit token provided
    • Enables SCS for fork PRs without permission errors
  2. Fixed SCS repo URL (commit baecb78)

    • Now correctly scans the fork repository instead of base repository
    • Uses ${{ inputs.repo-url }} which points to the actual fork

Test Results:

✅ Checkmarx scan: Completed successfully
✅ SCS enabled: "Using GitHub token for SCS..."
✅ Scanning correct fork: https://github.com/cosmir17/midnight-node-docker
✅ No permission errors

The SCS results show dashes, which is expected for midnight-node-docker (minimal dependencies, just Docker compose files).

Summary:

This PR now successfully implements fork-friendly Checkmarx scanning with SCS/Scorecard support, addressing the issue where MIDNIGHTCI_REPO token couldn't access fork repositories.

Add documentation for the new scs-repo-token input that enables SCS/Scorecard scanning
for fork PRs using github.token as default
@cosmir17 cosmir17 merged commit 2a471c1 into main Sep 25, 2025
1 check passed
@cosmir17 cosmir17 deleted the sean/PM-19431-fork-friendly-checkmarx branch September 25, 2025 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants