Skip to content

Conversation

@Wolfgang-Romanowski
Copy link

Description

Adds revamp to automated Snyk security scanning for training runtime Dockerfiles.
Scans differentiate between inherited base image vulnerabilities and new vulnerabilities introduced by Dockerfile changes.

  • Fails builds on critical/high severity vulnerabilities introduced by Dockerfile
  • Allows builds to proceed with only inherited base image vulnerabilities

Trigger Conditions:

  • Push to main branch (when training Dockerfiles change)
  • Pull requests with changes to training Dockerfiles
  • Nightly scheduled runs at 03:00 UTC
  • Manual workflow dispatch with debug options
  • Fork PRs after applying run-snyk label
  • Tested on fork

will require:
SNYK_TOKEN secret
SLACK_WEBHOOK_URL secret

Tested
Tested on fork thoroughly

@openshift-ci openshift-ci bot requested review from Fiona-Waters and dimakis June 3, 2025 15:37
@openshift-ci
Copy link

openshift-ci bot commented Jun 3, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign laurafitzgerald for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Fiona-Waters Fiona-Waters requested review from ChughShilpa, abhijeet-dhumal and sutaakar and removed request for dimakis June 3, 2025 15:48
if: >
github.event_name != 'pull_request_target' ||
github.event.label.name == 'safe-to-test' ||
github.event.pull_request.head.repo.full_name == github.repository
Copy link
Contributor

Choose a reason for hiding this comment

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

When should this third condition be applied?
First condition applies for anything else apart from PRs from forks (I suppose), second covers pull_request_target with proper label.

Copy link
Author

Choose a reason for hiding this comment

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

1: should be applied for all events except for pull request target, this includes events from forks which are safe since they run in the forks context.

2: applies for events with manual labeling

3: events from within the same repository

fork PRs run safely under condition 1 via regular pull request events, not pull request target.

sorry for not making that clearer

Copy link
Contributor

@sutaakar sutaakar Jun 4, 2025

Choose a reason for hiding this comment

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

Thanks, makes me think, why do you have there both pull_request and pull_request_target?

Copy link
Author

Choose a reason for hiding this comment

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

No worries at all!
So the reason why is because pull_request automatically scans trusted PRs within the same reo and pull_request_target ends up allowing scanning fork PRs after manual approval via label.

Without the dual approach we'd either never scan fork PRs or always give fork PRs access to secrets

@sutaakar
Copy link
Contributor

sutaakar commented Jun 4, 2025

Ran the code in my fork, it seems that there are missing ${PYTHON_VERSION}:${IMAGE_TAG} placeholder values in Individual Dockerfile Analysis.
See https://github.com/sutaakar/distributed-workloads/actions/runs/15442769297

Edit: I suppose it is taken from the Dockerfile itself, where we have placeholder, right?
In that case I guess there is not much to do apart from simulating container engine itself (overkill).
Although in the GitHub logs I see Scanning base image: registry.access.redhat.com/ubi9/python-311:9.5-1741671866

@Wolfgang-Romanowski
Copy link
Author

Ran the code in my fork, it seems that there are missing ${PYTHON_VERSION}:${IMAGE_TAG} placeholder values in Individual Dockerfile Analysis. See https://github.com/sutaakar/distributed-workloads/actions/runs/15442769297

Edit: I suppose it is taken from the Dockerfile itself, where we have placeholder, right? In that case I guess there is not much to do apart from simulating container engine itself (overkill). Although in the GitHub logs I see Scanning base image: registry.access.redhat.com/ubi9/python-311:9.5-1741671866

i can try updating the extraction logic to use build time values or read from a config files where values are defined

but from what i gather the scan itself is fine and its just a confusing display

@Wolfgang-Romanowski Wolfgang-Romanowski force-pushed the feature/snyk-dockerfile-scan branch from ece56c1 to cb47c38 Compare June 4, 2025 13:57
@Wolfgang-Romanowski Wolfgang-Romanowski force-pushed the feature/snyk-dockerfile-scan branch 2 times, most recently from 5c5f99d to 171f8bb Compare June 5, 2025 02:10
@Wolfgang-Romanowski Wolfgang-Romanowski force-pushed the feature/snyk-dockerfile-scan branch from 171f8bb to f539730 Compare June 5, 2025 02:21
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.

2 participants