does source level 3 require a pull request?

[zachariahcox]

continued from:

• https://github.com/slsa-framework/slsa/pull/1097#discussion_r1714090149
• https://github.com/slsa-framework/slsa/pull/1097#discussion_r1714293116

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]

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]

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

[zachariahcox]

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]

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