Add support for new RepositoryAdvisories API :tada: by JLLeitschuh · Pull Request #2483 · PyGithub/PyGithub

JLLeitschuh · 2023-03-30T22:27:14Z

codecov-commenter · 2023-04-04T16:43:05Z

Codecov Report

Patch coverage: 95.11% and project coverage change: -0.13 ⚠️

Comparison is base (84912a6) 98.57% compared to head (1168ac3) 98.45%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2483      +/-   ##
==========================================
- Coverage   98.57%   98.45%   -0.13%     
==========================================
  Files         118      124       +6     
  Lines       11981    12407     +426     
==========================================
+ Hits        11810    12215     +405     
- Misses        171      192      +21

Impacted Files	Coverage Δ
github/Repository.py	`97.10% <93.02%> (-0.11%)`	⬇️
github/RepositoryAdvisoryCredit.py	`93.61% <93.61%> (ø)`
github/RepositoryAdvisory.py	`93.83% <93.83%> (ø)`
github/RepositoryAdvisoryCreditDetailed.py	`95.65% <95.65%> (ø)`
github/RepositoryAdvisoryVulnerability.py	`98.57% <98.57%> (ø)`
github/CWE.py	`100.00% <100.00%> (ø)`
github/GithubObject.py	`98.84% <100.00%> (+0.02%)`	⬆️
github/RepositoryAdvisoryVulnerabilityPackage.py	`100.00% <100.00%> (ø)`

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

JLLeitschuh · 2023-04-04T16:50:13Z

github/Repository.py

+        vulnerabilities: typing.Optional[
+            typing.Iterable[
+                github.RepositoryAdvisoryVulnerability.AdvisoryVulnerability
+            ]
+        ] = None,


According to the GitHub team I'm chatting with, this should not be optional. At least one must be supplied.

That GitHub's API currently supports passing an empty list, is a bug.

Turns out this isn't a bug. It's implemented as-designed

JLLeitschuh · 2023-04-04T17:39:44Z

CI passes. Linting passses. AFAIK, everything has solid unit test coverage.

Seeking code reviews!

rthorpeii

The structure of the API objects and calls looks good to me!

EnricoMi

LGTM, minor comments

EnricoMi · 2023-05-13T20:04:24Z

github/Repository.py

            self._requester, headers, data, completed=True
        )

+    def create_repository_advisory(


since this is on Repository, this could read:

Suggested change

def create_repository_advisory(

def create_advisory(

Or are there other methods that imply this naming convention?

EnricoMi · 2023-05-13T20:09:09Z

github/Repository.py

+            private_vulnerability_reporting=False,
+        )
+
+    def report_security_vulnerability(


I think this should read:

Suggested change

def report_security_vulnerability(

def report_security_advisory(

or

Suggested change

def report_security_vulnerability(

def create_security_advisory(

This API is used by reporters, not by maintainers, to report vulnerabilities.

I was trying to figure out a way to differentiate the create which is something only a repository admin can take, from the report which is an action any GitHub user can take.

right, the link to the docs confused me, I think it is linking to creating an advisory, where it should link to creating a vulnerability

EnricoMi · 2023-05-13T20:10:56Z

github/Repository.py

            ),
        }

+    def get_repository_advisories(


Here, repository could also be removed, but on the other hand, it returns a `RepositoryAdvisory.

So does the one above. Which naming convention would you like to see used here?

Unless you have a strong preference, I think we should opt to use the naming convention in the GitHub API docs here.

agreed, keep it

EnricoMi · 2023-05-23T14:10:46Z

github/Repository.py

+        ] = None,
+    ) -> github.RepositoryAdvisory.RepositoryAdvisory:
+        """
+        :calls: `POST /repos/{owner}/{repo}/security-advisories/reports <https://docs.github.com/en/rest/security-advisories/repository-advisories#create-a-repository-security-advisory>`_


I think the link should point to

https://docs.github.com/en/rest/security-advisories/repository-advisories#privately-report-a-security-vulnerability

EnricoMi · 2023-05-23T14:11:44Z

github/Repository.py

+            private_vulnerability_reporting=False,
+        )
+
+    def report_security_vulnerability(


right, the link to the docs confused me, I think it is linking to creating an advisory, where it should link to creating a vulnerability

EnricoMi · 2023-06-01T07:58:04Z

@JLLeitschuh happy to approve, please rebase and correct the doc links

JLLeitschuh · 2023-06-01T13:10:46Z

Will do, just been bogged down in other work

Related: - https://github.blog/changelog/2023-03-30-repository-security-advisories-rest-api/ - https://docs.github.com/en/rest/security-advisories/repository-advisories Signed-off-by: Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com>

EnricoMi

LGTM!

JLLeitschuh force-pushed the feat/JLL/support_repository_advisiories branch from 9cd7af4 to 5e1afde Compare March 31, 2023 14:58

JLLeitschuh mentioned this pull request Apr 4, 2023

Leverage PEP 484 inline types instead of .pyi files #2481

Closed

JLLeitschuh force-pushed the feat/JLL/support_repository_advisiories branch from 5e1afde to e7a5d60 Compare April 4, 2023 16:37

JLLeitschuh marked this pull request as ready for review April 4, 2023 16:38

JLLeitschuh commented Apr 4, 2023

View reviewed changes

JLLeitschuh force-pushed the feat/JLL/support_repository_advisiories branch 2 times, most recently from 48924fb to f4e1dc8 Compare April 4, 2023 17:35

rthorpeii reviewed Apr 5, 2023

View reviewed changes

JLLeitschuh requested a review from rthorpeii April 5, 2023 22:14

JLLeitschuh force-pushed the feat/JLL/support_repository_advisiories branch from 31820e2 to 884f1c4 Compare April 19, 2023 21:48

EnricoMi reviewed May 13, 2023

View reviewed changes

EnricoMi reviewed May 23, 2023

View reviewed changes

JLLeitschuh force-pushed the feat/JLL/support_repository_advisiories branch from 884f1c4 to 94d10ac Compare June 7, 2023 14:45

Add support for new RepositoryAdvisories API 🎉

c34ead0

Related: - https://github.blog/changelog/2023-03-30-repository-security-advisories-rest-api/ - https://docs.github.com/en/rest/security-advisories/repository-advisories Signed-off-by: Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com>

JLLeitschuh force-pushed the feat/JLL/support_repository_advisiories branch from 94d10ac to c34ead0 Compare June 7, 2023 15:24

EnricoMi approved these changes Jun 9, 2023

View reviewed changes

Merge branch 'master' into feat/JLL/support_repository_advisiories

1168ac3

JLLeitschuh merged commit daf62bd into PyGithub:master Jun 9, 2023

	def report_security_vulnerability(
	def report_security_advisory(

	def report_security_vulnerability(
	def create_security_advisory(

Uh oh!

Conversation

JLLeitschuh commented Mar 30, 2023

Uh oh!

codecov-commenter commented Apr 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JLLeitschuh commented Apr 4, 2023

Uh oh!

rthorpeii left a comment

Choose a reason for hiding this comment

Uh oh!

EnricoMi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EnricoMi commented Jun 1, 2023

Uh oh!

JLLeitschuh commented Jun 1, 2023

Uh oh!

EnricoMi left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Apr 4, 2023 •

edited

Loading