Automatically credit remediation developers and reviewers - #61
Automatically credit remediation developers and reviewers#61sethmlarson wants to merge 3 commits into
Conversation
|
|
||
| # Find new credits for the security advisory. | ||
| if credits := get_security_advisory_credits(github, security_advisory): | ||
| patch_data["credits"] = credits |
There was a problem hiding this comment.
What about existing credits?
There was a problem hiding this comment.
I believe this is now handled in 27dd5d5 ? Although I believe the GitHub API already doesn't "clobber" in the PATCH method?
There was a problem hiding this comment.
Also, can we check that we aren't just patching the same credits?
| "cve_id": cve_id, | ||
| "collaborating_teams": [{"slug": team} for team in collaborating_teams], | ||
| "collaborating_users": [{"login": "octocat", "id": 1, "type": "User"}], | ||
| "private_fork": {"name": "repo-ghsa-xxxx-xxxx-xxxx", "owner": {"login": "owner"}}, |
There was a problem hiding this comment.
Won't we loose coverage?
There was a problem hiding this comment.
I removed this field to avoid needing to mock out the pull requests response for each test case that didn't process private_fork. All the test cases that use private_fork already set the value.
StanFromIreland
left a comment
StanFromIreland
left a comment
There was a problem hiding this comment.
Can you also please update the flow chart in the README for this, TiA!
|
|
||
| # Find new credits for the security advisory. | ||
| if credits := get_security_advisory_credits(github, security_advisory): | ||
| patch_data["credits"] = credits |
There was a problem hiding this comment.
Also, can we check that we aren't just patching the same credits?
| private_fork_repo = private_fork["name"] | ||
|
|
||
| try: | ||
| pull_requests = json.loads( |
There was a problem hiding this comment.
Can we please add a little comment re. the lack of pagination, just in case it somehow surprises someone down the line.
| ) | ||
| except RequestFailed: | ||
| capture_exception() | ||
| raise RuntimeError("Request to list pull requests failed") from None |
There was a problem hiding this comment.
We don't have coverage for these cases (to ensure we don't leak any other output), but that can be a follow up.
| raise RuntimeError("Request to list pull requests failed") from None | ||
|
|
||
| for pull_request in pull_requests: | ||
| # fmt: off |
There was a problem hiding this comment.
Do we really need these?
| ) | ||
| except RequestFailed: | ||
| capture_exception() | ||
| raise RuntimeError("Request to list pull requests reviews failed") from None |
There was a problem hiding this comment.
We could continue processing the next advisory instead?
2 participants
The bot can automatically credit people who participate in creating a patch or reviewing the patch privately. Does not override credits that already exist for an individual.