Skip to content
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

does source level 3 require a pull request? #1113

Closed
zachariahcox opened this issue Aug 15, 2024 · 4 comments
Closed

does source level 3 require a pull request? #1113

zachariahcox opened this issue Aug 15, 2024 · 4 comments

Comments

@zachariahcox
Copy link
Contributor

zachariahcox commented Aug 15, 2024

continued from:

Zac's thoughts:

I think this is what would need to be covered:

  • A target branch (EG: refs/heads/main) has at least minimal branch protections on it.
  • a user will push up a number of unshippable commits on some user-controlled topic branch
  • a pull request is opened to review the net delta between the tip of those commits and the tip of the target branch
  • tests, checks, reviews, comments, and votes will occur on the proposed merge commit in the context of the PR (this is sort of building up the final change proposal).
  • the SCP must record all security metadata related to the creation of the final commit representing the change managed by this pr. Depending on the merge type (which should generally be a squash merge for PRs), the SCP may need to record per-commit metadata for each commit that will become reachable by this merge.

I guess the critical question is whether you MUST have a pull request context to achieve this level. The PR is the context where the SCP can consider changes like this and might be willing to sign off on how the final commits were created.

I think yes, for tools like github pull requests (with or without required review) are necessary to achieve this level. Without them, the PR and SCP tools will not be involved in creation of the final merge commit and will have a hard time attesting anything about revision creation process.

@adityasaky
Copy link
Contributor

adityasaky commented Sep 20, 2024

My read is this is dependent on what the change management process (related: #1139) ends up being. If the requirement is only that force pushes and deletions are disallowed (related: #1136), and since level 3 does not prescribe review or automated testing requirements, I think a pull request may not be absolutely necessary as the SCP wouldn't be attesting to anything that's a requirement? It can still evaluate the commits that become reachable, which are the only bits that may need to be attested to, but still not with reviews or test results at intermediate revisions, etc.

@TomHennen
Copy link
Contributor

I agree with @adityasaky's take. Under the current definition PRs don't seem to be required. I think we can close this?

@zachariahcox
Copy link
Contributor Author

Yep. It's been a few months and I think I agree we can close it.

I think that's sort of saying "the scp might require extra process to issue attestations, either for security, mechanical necessity, or scale." But that's going to be true of most tools.

I'm pretty sure there are kinds of rules that require pr contexts (for the proposed merge commits), but it doesn't seem strictly relevant here now either.

@TomHennen
Copy link
Contributor

Perfect. We can reopen or start a new issue if we decide we need this after all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Status: Done
Development

No branches or pull requests

3 participants