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

TAC Vote Needed - Enable GitHub Secret Scanning and Push Protection #333

Closed
Danajoyluck opened this issue May 23, 2024 · 22 comments
Closed
Labels
Content Updates/additions to TAC content/process. Must include a changelog entry. Needs 3 approvals. Next Meeting OpsModel vote

Comments

@Danajoyluck
Copy link
Contributor

Danajoyluck commented May 23, 2024

The issue is in response to BEST WG issue "Add secret scanning and push protection to SCM-BestPractices recommendations"

Secret scanning and push protection is added to the security baseline.

Will need TAC decision on enabling secret scanning and push protection at the enterprise level. Details of the features, settings at the enterprise level, how alert is raised, how to respond to the alert, and the approach to audit are capture in this document for TAC review and decision.

Summarized the responses from @SecurityCRob @sevansdell @steiza @lehors into TAC Decision section to make the remaining votes easier.

@lehors
Copy link
Contributor

lehors commented May 23, 2024

I don't think this should be a requirement to enter sandox but it could be to enter incubation, which should lead sandbox projects to want it in order to prepare for the next lifecycle stage.

@SecurityCRob
Copy link
Contributor

I agree with Arnaud. I think as an Incubating requirement would be more appropriate.

@SecurityCRob
Copy link
Contributor

SecurityCRob commented May 24, 2024

do we want to integrate #255 along with this and ensure branch protection is enabled?

@SecurityCRob SecurityCRob added vote Next Meeting OpsModel Content Updates/additions to TAC content/process. Must include a changelog entry. Needs 3 approvals. labels Jun 5, 2024
@Danajoyluck
Copy link
Contributor Author

do we want to integrate #255 along with this and ensure branch protection is enabled?

I feel we need to decouple these two issues to isolate the potential impacts of the changes. I would also like us to build a consensus on what rulesets should be at the enterprise level versus the organizational level.

@Danajoyluck
Copy link
Contributor Author

@lehors @SecurityCRob I have moved the secret scanning off sandbox baseline.

@sevansdell
Copy link
Contributor

Do we have a way to enable it at an enterprise level, but only apply it to incubating and graduated TIs?

@lehors
Copy link
Contributor

lehors commented Jun 7, 2024

Do we have a way to enable it at an enterprise level, but only apply it to incubating and graduated TIs?

For what it's worth I'm not worried about having it turned on for all projects within the ossf GitHub org. That becomes a benefit they get for free from having their repo in that org.

I was merely concerned about requiring it for all projects, including those projects that wouldn't get it for free because they are not part of that org (and which are quite numerous I believe).

@steiza
Copy link
Member

steiza commented Jun 7, 2024

Copying over the questions from the doc

Do we enable secret scanning at the enterprise level?

Yes, I think this is a good idea. It doesn't differentiate between sandbox projects, but using partner patterns is very unlikely to generate a false positive (and if I understand correctly, contributors can identify a false positive and still be able to push through a detection).

Do we enable secret scanning as a push protection at the enterprise level?

Yes, I would enable push protection and add a resource link (which will help someone identify what to do in case there's a false positive).

Do we allow or not allow repository admins to Enable or Disable Secret Scanning?

I could go either way, but I'm leaning towards no. I think it's very unlikely repository admins would need to disable it (since there's a bypass flow built-in), and we can always revisit this if needed.

How is alert going to be handled?

I think this should be the TI that maintains the repository - they have the most context on the repository contents. Any repository admin can get these alerts by either watching the entire repository (which is a good idea generally for admins), or by going Watch > Custom > Security alerts.

Audit of the actions taken on the alert: Can the audit be the next phase or a higher level of maturity to be prioritized after secret scanning is enabled?

That sounds fine to me!

@sevansdell
Copy link
Contributor

Copying over the questions from the doc

Do we enable secret scanning at the enterprise level?

Yes, I think this is a good idea. It doesn't differentiate between sandbox projects, but using partner patterns is very unlikely to generate a false positive (and if I understand correctly, contributors can identify a false positive and still be able to push through a detection).

Do we enable secret scanning as a push protection at the enterprise level?

Yes, I would enable push protection and add a resource link (which will help someone identify what to do in case there's a false positive).

Do we allow or not allow repository admins to Enable or Disable Secret Scanning?

I could go either way, but I'm leaning towards no. I think it's very unlikely repository admins would need to disable it (since there's a bypass flow built-in), and we can always revisit this if needed.

How is alert going to be handled?

I think this should be the TI that maintains the repository - they have the most context on the repository contents. Any repository admin can get these alerts by either watching the entire repository (which is a good idea generally for admins), or by going Watch > Custom > Security alerts.

Audit of the actions taken on the alert: Can the audit be the next phase or a higher level of maturity to be prioritized after secret scanning is enabled?

That sounds fine to me!

I concur! Thanks for the breakdown @steiza.

@SecurityCRob
Copy link
Contributor

I vote "yes" to enable and set as a requirement for incubating & above.

@Danajoyluck
Copy link
Contributor Author

Do we have a way to enable it at an enterprise level, but only apply it to incubating and graduated TIs?

For what it's worth I'm not worried about having it turned on for all projects within the ossf GitHub org. That becomes a benefit they get for free from having their repo in that org.

I was merely concerned about requiring it for all projects, including those projects that wouldn't get it for free because they are not part of that org (and which are quite numerous I believe).

thanks @lehors could you please elaborate more on "I was merely concerned about requiring it for all projects, including those projects that wouldn't get it for free because they are not part of that org (and which are quite numerous I believe).". By all projects I meant all the projects under OpenSSF enterprise account.

@Danajoyluck
Copy link
Contributor Author

Do we have a way to enable it at an enterprise level, but only apply it to incubating and graduated TIs?

Thanks @sevansdell . Two different concepts if I understand your question properly....

Enterprise means GitHub enterprise account. it's a GitHub construct and the root of OpenSSF GitHub account structure.

TI's are in the form of GitHub repositories that will be under a GitHub organization which is under the OpenSSF GitHub enterprise account.

@lehors
Copy link
Contributor

lehors commented Jun 14, 2024

So, what I meant is that if this is something that can be enabled by the staff on all repos hosted by the ossf GitHub account/org without requiring anything from the projects we might as well do it for all of them, independently of their maturity/lifecycle status.
For projects that are outside the ossf GitHub org let's make it a requirement for entering Incubation.
I hope that's clearer.

@Danajoyluck
Copy link
Contributor Author

Danajoyluck commented Jun 14, 2024

So, what I meant is that if this is something that can be enabled by the staff on all repos hosted by the ossf GitHub account/org without requiring anything from the projects we might as well do it for all of them, independently of their maturity/lifecycle status. For projects that are outside the ossf GitHub org let's make it a requirement for entering Incubation. I hope that's clearer.

@lehors I see....My note was flawed, yes, the setting is independent of project lifecycle. I updated the issue description.

@bobcallaway
Copy link
Contributor

I vote "yes" to enable and set as a requirement for incubating & above.

@Danajoyluck
Copy link
Contributor Author

Danajoyluck commented Jun 21, 2024

I vote "yes" to enable and set as a requirement for incubating & above.

Thanks @bobcallaway ....if we want to do it this way, we have two options:

  1. Opt out: Enforce scanning at enterprise level, and override it at repo level
  2. Opt in: Don't enforce at enterprise or level, each repo manages the scan based on the project life cycle

Either way requires repo admin to make changes. Enterprise or Org owner does not have to do anything with opt out approach.

Security wise, I cannot see difference between the two approaches, assuming projects will attest when they apply for life cycle changes

@bobcallaway
Copy link
Contributor

I vote "yes" to enable and set as a requirement for incubating & above.

Thanks @bobcallaway ....if we want to do it this way, we have two options:

  1. Opt out: Enforce scanning at org level, and override it at repo level
  2. Opt in: Don't enforce at org level, each repo manages the scan based on the project life cycle

Either way requires repo admin to make changes. Org owner does not have to do anything with opt out approach.

Security wise, I cannot see difference between the two approaches.

Opt out seems pragmatic to me.

@marcelamelara
Copy link
Contributor

After reviewing the doc and discussion, here are my votes for repos under the ossf enterprise org:

  1. Do we enable secret scanning at the enterprise level?

opt out

  1. Do we enable secret scanning as a push protection at the enterprise level and allow repo admin to add a resource link (which will help someone identify what to do in case there's a false positive)?

yes

  1. Do we allow or not allow repository admins to Enable or Disable Secret Scanning?

no

  1. Who and how to handle alerts?

I agree with Zach, it should be the admins of the affected TI's repo to handle alerts. Instructions that show repo admins how to handle alerts should be added to the SCM Best Practices docs.

In addition, I vote in favor of making the above a requirement for Incubating stage TIs that host repos outside the ossf org.

@mlieberman85
Copy link
Contributor

I vote yes

@torgo
Copy link
Contributor

torgo commented Jul 9, 2024

After reviewing the doc and discussion, here are my votes for repos under the ossf enterprise org:

  1. Do we enable secret scanning at the enterprise level?

opt out

  1. Do we enable secret scanning as a push protection at the enterprise level and allow repo admin to add a resource link (which will help someone identify what to do in case there's a false positive)?

yes

  1. Do we allow or not allow repository admins to Enable or Disable Secret Scanning?

no

  1. Who and how to handle alerts?

I agree with Zach, it should be the admins of the affected TI's repo to handle alerts. Instructions that show repo admins how to handle alerts should be added to the SCM Best Practices docs.

In addition, I vote in favor of making the above a requirement for Incubating stage TIs that host repos outside the ossf org.

I agree with Marcela. :)

@SecurityCRob
Copy link
Contributor

8 of 9 tac members expressed support in the 9july2024 call. merging

@SecurityCRob
Copy link
Contributor

closing (no pr to merge)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content Updates/additions to TAC content/process. Must include a changelog entry. Needs 3 approvals. Next Meeting OpsModel vote
Projects
None yet
Development

No branches or pull requests

9 participants