Pull request remediations engine + codeQL + dependabot remediations #1200
Conversation
|
I will add some comments to explain my thinking behind the PR. |
| // the file to patch | ||
| string path = 1; | ||
| // how to patch the file. For now, only replace is supported | ||
| string action = 2; |
There was a problem hiding this comment.
The idea here is to add several more actions. I think both JSON merge patch and JSON patch would be useful here, the former to patch YAML/JSON files to include some content in general and the latter to add an item to a JSON/YAML array which you can't do with a merge patch AFAIK.
The other attribute I think we should use would be a "condition" which might be a JQ condition that must pass in order for the action to be performed. We can have something like:
content:
- path: .github/dependabot.yaml
action: replace
condition: <if-empty>
- path. .github/dependabot.yaml
action: mergePatch
condition: <if-already-some-contents-but-not-this-ecosystem>
There was a problem hiding this comment.
tracked with #1210 for a future PR
proto/mediator/v1/mediator.proto
Outdated
| // the GIT mode of the file. Not UNIX mode! String because the GH API also uses strings | ||
| // the usual modes are: 100644 for regular files, 100755 for executable files and | ||
| // 040000 for submodules (which we don't use but now you know the meaning of the 1 in 100644) | ||
| string mode = 3; |
There was a problem hiding this comment.
The default is 10644 which translates to "a file with 0644 UNIX permissions" and it's handled in the code. Hmm maybe I should have made the mode optional?
There was a problem hiding this comment.
Optional would be better, yeah. Let's just try to have easy to use defaults
| // if no mode is specified, create a regular file with 0644 UNIX permissions | ||
| ghModeNonExecFile = "100644" | ||
| dflBranchBaseName = "mediator" | ||
| dflBranchFrom = "main" |
There was a problem hiding this comment.
These should be ultimately configurable in the rule_types and the remediator, but since the rule_types don't allow to configure the branches either, I was thinking of doing a follow-up.
There was a problem hiding this comment.
filed #1205 for future reference
jhrozek
force-pushed
the
pull_request_remediation
branch
from
bffb442
to
465eb26
Compare
| @@ -64,7 +64,7 @@ func NewOAuthConfig(provider string, cli bool) (*oauth2.Config, error) { | |||
| if provider == Google { | |||
| return []string{"profile", "email"} | |||
| } | |||
| return []string{"user:email", "repo", "read:packages", "write:packages", "read:org"} | |||
| return []string{"user:email", "repo", "read:packages", "write:packages", "workflow", "read:org"} | |||
There was a problem hiding this comment.
- the workflow permission is needed in order to write under
~/.github/workflows - should we change
repoforpublic_repo?
There was a problem hiding this comment.
What does changing to public_repo gives us?
There was a problem hiding this comment.
not being allowed to touch private repos (I know we can't register them as of now..and don't process them with web hooks at the moment).
jhrozek
force-pushed
the
pull_request_remediation
branch
from
465eb26
to
60f2b1a
Compare
|
rebased atop Radoslav's recent alerting work |
We'll need to use several methods from the GH API in order to open pull requests. This patch adds them to the provider interface we use.
Adds a new protobuf message that represents the pull request remediation.
Adds a new remediator whose job it is to open pull requests. Currently, the pull requests are only opened unless another PR with exactly the same contents exists.
In order to propose PRs that include files under the .github/workflows subtree, we need to grant Mediator the workflows scope as well.
Adds two usages of PR remediations, for dependabot and codeQL.
jhrozek
force-pushed
the
pull_request_remediation
branch
from
60f2b1a
to
e9891e9
Compare
|
rebased on current master & addressed the comments (which was really just making that one protobuf field optional). |
|
The test makes sense to me, let's get some tests |
thanks for the review, test added |
This PR adds a new remediations engine that is able to open pull requests.
At the moment, the only way the remediator can open PRs is to replace a file,
in subsequent PRs we might want to add patches (e.g. to support two dependabot
configurations, each for a different ecosystem), but the API support for that
should be there.
The PR is opened if no other PR with the same content hash exists. We might want
to revisit that to store the PR in the database instead to save API calls, but
this way was quiet straightforward and works fine.
The PR also amends the CodeQL and dependabot rule_types with remediations.
Fixes: #1064
Fixes: #1077
Fixes: #1078