Skip to content

Override the ref, sha and the manifests' source_location attributes in the JSON that will be submitted #249

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.

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

Closed
wants to merge 5 commits into from

Conversation

mkurz
Copy link
Contributor

@mkurz mkurz commented Mar 5, 2025

These inputs do not change how dependencies are processed. They only override certain attributes just before the JSON file is submitted.

By allowing these settings to be overridden, we can implement a workaround to submit dependencies from any branch and have them processed by GitHub - including receiving alerts for them - not just the main branch, which is currently the only officially supported branch.

@mkurz mkurz force-pushed the overrides-ref-sha-manifest branch 2 times, most recently from b6c1eca to 2e4a636 Compare March 6, 2025 17:15
@adpi2
Copy link
Member

adpi2 commented Mar 7, 2025

By allowing these settings to be overridden, we can implement a workaround to submit dependencies from any branch and have them processed by GitHub - including receiving alerts for them - not just the main branch, which is currently the only officially supported branch.

Could you give an example of what input you would use to have Github process the snapshot?

@mkurz
Copy link
Contributor Author

mkurz commented Mar 7, 2025

Could you give an example of what input you would use to have Github process the snapshot?

https://github.com/mkurz/playframework/blob/eb7591ec0fed5bfdf2b3f767d8d064b129f383a7/.github/workflows/dependency-graph.yml#L29-L41

Here is the thing:
GitHub only processes a JSON if the sha field value submitted is part of a repo's default branch (basically it checks git merge-base --is-ancestor <sha-from-the-json> <default-branch>), otherwise it will just ignore the json. In short that means GitHub only allows submitting the dependency graph for the default branch (this is even documented). (btw, the ref field does absolutely not matter, you can submit a random value and GitHub still processes the json - as long it passes GitHubs validation, I once got an error back: Invalid property /ref: does not match ``/^refs\\//``.","documentation_url":"https://docs.github.com/rest/dependency-graph/dependency-submission#create-a-snapshot-of-dependencies-for-a-repository","status":"422"} - but GitHub does not do anything with that attribute).

So what I do is I just hardcode the same sha (one that is on the default branch) for all workflows in all branches in a repo. This way GitHub thinks all json that it receives are from the default branch, which, in reality is not true.
As result the same snapshot gets updated each time a workflow runs. Now, to distinct different branches, we can just use the correlator - this way within the same snapshot multiple branches can coexist.
One problems remains: how to distinguish branches in the UI, because usually the root project always has the build.sbt in the root folder of the repo? This is what the manifest-override input is for. With this input we just override the name of the manifest GitHub will show in the UI. So for my test repo that means in https://github.com/mkurz/playframework/security/dependabot it displays:
image

So you can see, I changed the file name of the manifest: I added the timestamp when the workflow ran, the branch name and, like before the file name itself. This allows me to distinguish from which branch an alert comes from. (In theory we would not need the correlator anymore, because the file names do not override each other anymore).

If I run the referenced workflow scheduled, each night, now, we would be able to check not just the main branch, but also our stable branches for security vulnerabilities.

I have done lots of testing to understand how GitHub process the submitted dependency snapshots, when and how it opens new alerts, and when it closes them automatically:

I was also testing this PR thoroughly, hosting the artifacts on my github pages:

If something is unclear, let me know. I might have some more links that explain things a bit more, but I have to run now, will be back later.

Thanks!

@mkurz
Copy link
Contributor Author

mkurz commented Mar 7, 2025

(in case you merge this PR, it would be nice if yould push a new tag so I can make us if it, thanks!)

@mkurz
Copy link
Contributor Author

mkurz commented Mar 10, 2025

@mkurz mkurz force-pushed the overrides-ref-sha-manifest branch from 2e4a636 to dccb52a Compare March 10, 2025 15:16
@adpi2
Copy link
Member

adpi2 commented Mar 10, 2025

Hi @mkurz and thanks for opening this pull request.

After internal discussions at the Scala Center, we concluded not to merge it. The proposed changes are designed to bypass Github's built-in restrictions, which are part of their Advanced Security package:

Dependency review - Show the full impact of changes to dependencies and see details of any vulnerable versions before you merge a pull request. See About dependency review.

As maintainers of this repository, we don't want to open this door to other users.

@adpi2 adpi2 closed this Mar 10, 2025
@mkurz
Copy link
Contributor Author

mkurz commented Mar 10, 2025

Hmm.. OK. However, I do not understand why you refer to the "Dependency review" feature - what does that have to do with bypassing Github's built-in restrictions? Actually... (please don't get me wrong) - which restrictions?

I accept that you will not merge this PR.

However, I do not see it as strict as saying "bypass ... Github's built-in restrictions" - The dependency submission API is just a public REST api you can post to any dependencies you like. It's more or less independent of which dependency are actually contained in a repo's manifest file(s). I can right now start curl and post a json with any dependencies I like to a repo, no one can stop me from that, GitHub will not check the deps I submit and I don't think by doing that I would bypass GitHub's built-in restrictions (as there are no restrictions IMHO - because I actually can just post what I want).

The way I see it: I just know how the API works and make use of its features.

Side note: GitHub also just uses the same api via https://github.com/advanced-security/maven-dependency-submission-action to submit dependencies of maven projects - the only difference people don't have to set up their own workflows to run it, but just click a button in the settings.

The reason why I came up with this pull request is because I can just not see how it is possible to monitor the dependencies of non-default branches. I know my PR is a workaround, but for me personally, the benefit of having easy insight and getting notified about potential vulnerabilities in our stable branches outweighs that.

@adpi2
Copy link
Member

adpi2 commented Mar 11, 2025

Actually I was reading too fast, and it is true that Dependeny Review is fully available on public repositories. So I am not sure why Github is limiting dependency scans to the default branch only.

The reason why I came up with this pull request is because I can just not see how it is possible to monitor the dependencies of non-default branches.

As an alternative, have you considered using actions/dependency-review-action? You could have a long-living PR to get a dependency review comment each time you push to your branch.

@mkurz
Copy link
Contributor Author

mkurz commented Mar 18, 2025

Catching up here again...

I am not sure why Github is limiting dependency scans to the default branch only.

Because it's a limitation in GitHub. GitHub just way too much focuses on the default branch. For example, it's impossible to set up scheduled workflows for non-default branches in GitHub. I opened a ticket long time ago for that: https://github.com/orgs/community/discussions/16107 - I even talked with someone from GitHub engineering at a conference and they are aware of that limitation. I did not use Gitlab for a quite a while (except commenting on some random projects every now and then) but according to this comment, things are much easier to set up for non-default branches.

So GitHub not raising alerts for dependencies in non-default branches is just a limitation by GitHub, nothing more.

As an alternative, have you considered using actions/dependency-review-action? You could have a long-living PR to get a dependency review comment each time you push to your branch.

This is not an alternative. Just by looking at the response schema of the review api, I can see that the change_type can only be added or removed. That means when submitting a PR with a dependency update, GitHub will only check a dependency that got added (or upgraded) or removed and then will check a newly added dependency for vulnerabilities (or delete alerts for an vulnerability if a dependency gets removed).

So the review action/api does not cover:

  • check all dependencies when opening a pull request (only checks changed dependencies)
    • that alone is a showstopper already, because that means it can not be used to chronically check all dependencies on non-default branches (actually no matter if scheduled or when openend the PR, it's just not possible)
  • what if a dependency gets added via a merged pull request, that has no vulnerability yet
    • two days later a vulnerability gets reported
    • but because we are on a non-default branch, GitHub will not notify us
    • maybe scala steward will update the dependency one day, containing a fix, but then we will not be notified anymore, so we do not even know that we ever shipped a faulty dependency
    • Also what if the vulnerability gets fixed not in a patch release, but a minor release of the dependency and we have pinned the dependency in scala steward to update patch releases only. The dependency will never be updated and we will never be notified that it as a vulnerability. Worst case.

So the dependency review api is of no use for what I want to achieve.

What I want to achieve is to be able to get notified by GitHub for vulnerabilities in our stable, non-default branches. Either when openend a PR or when running scheduled - by running a scheduled job in the default branch, checking out the stable branch, running sbt-dependency-submission,... because, like written above, we can not even set up scheduled workflows ootb for non-default branches. This way we can run dependency checks for all our relevant branches every night and get notified immediately when a new vulnerability comes up.

After internal discussions at the Scala Center, we concluded not to merge it. The proposed changes are designed to bypass Github's built-in restrictions, which are part of their Advanced Security package

Hmm.. I think I realize now what you wanted to tell me with this sentence... Did you mean that, with this PR, I kind of replicated the functionality of the dependency review api, which you assumed was availabile for private repos only?
Turns out

  1. No, this PR does not replicate the dependency review api
  2. As you are aware now, the dep. review api is available for public repos anyway, so there is no need to replicat it anyway.
  3. The dependency review api is designed for a different purpose than for what I am trying to solve.

My PR just makes use of the submission api. I just make the api believe the dependencies I send to it are from the main (default) branch, so it will process them. That's basically all. By doing that I am not bypassing any GitHub restriction.
The dependency alert api/backend does not know which files (or manifests) are checked-in into the actual repo (otherwise we would not need an api).

Maybe you misunderstood the intention of this PR?

@mkurz
Copy link
Contributor Author

mkurz commented Mar 18, 2025

@mkurz
Copy link
Contributor Author

mkurz commented Mar 18, 2025

(side note: found, what I would say a bug, in GitHub security alerts: https://github.com/orgs/community/discussions/154281)

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