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

content: draft: Harden 'safe-expunging-process' #1203

Merged
merged 15 commits into from
Oct 25, 2024

Conversation

TomHennen
Copy link
Contributor

fixes #1135

Hardens the 'safe-expunging-process' by:

  1. Suggesting that SCSs should document and log changes when possible.
  2. SCSs should use multi-party approval when possible

Also clarifies that some of these changes may need to be kept private to comply with local laws.

@TomHennen
Copy link
Contributor Author

@adityasaky I'd also like your input here.

Copy link

netlify bot commented Oct 16, 2024

Deploy Preview for slsa ready!

Name Link
🔨 Latest commit ab601b2
🔍 Latest deploy log https://app.netlify.com/sites/slsa/deploys/671abd38b224760008c87a71
😎 Deploy Preview https://deploy-preview-1203--slsa.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@trishankatdatadog trishankatdatadog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't see anything controversial

Copy link
Member

@adityasaky adityasaky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still a bit curious what this may end up looking like. In addition to the experiences @zachariahcox and others have had at GitHub, GitLab, etc., I wonder if something like this has ever come up within the LF for https://git.kernel.org/ or so? Is there a public guide for dealing with these scenarios that we can leverage for this?

docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
Copy link
Contributor

@marcelamelara marcelamelara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these revisions @TomHennen ! I agree with the other reviewers that the safe expunging process should be scoped to legal challenges.

docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
TomHennen and others added 5 commits October 23, 2024 17:19
fixes slsa-framework#1135

Hardens the 'safe-expunging-process' by:

1. Suggesting that SCSs should document and log changes when possible.
2. SCSs should use multi-party approval when possible

Also clarifies that some of these changes may need to be kept private
to comply with local laws.

Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Co-authored-by: Zachariah Cox <zachariahcox@github.com>
Signed-off-by: Tom Hennen <TomHennen@users.noreply.github.com>
Co-authored-by: Zachariah Cox <zachariahcox@github.com>
Signed-off-by: Tom Hennen <TomHennen@users.noreply.github.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
TomHennen and others added 3 commits October 24, 2024 13:05
Co-authored-by: Aditya Sirish <8928778+adityasaky@users.noreply.github.com>
Signed-off-by: Tom Hennen <TomHennen@users.noreply.github.com>
Co-authored-by: Aditya Sirish <8928778+adityasaky@users.noreply.github.com>
Signed-off-by: Tom Hennen <TomHennen@users.noreply.github.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Copy link
Member

@adityasaky adityasaky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor language suggestions but I think this looks good!

docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
TomHennen and others added 2 commits October 24, 2024 13:54
Co-authored-by: Aditya Sirish <8928778+adityasaky@users.noreply.github.com>
Signed-off-by: Tom Hennen <TomHennen@users.noreply.github.com>
Co-authored-by: Aditya Sirish <8928778+adityasaky@users.noreply.github.com>
Signed-off-by: Tom Hennen <TomHennen@users.noreply.github.com>
Copy link
Contributor

@marcelamelara marcelamelara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this process is much clearer now, thanks @TomHennen ! I have one remaining question about the multi-party approval.

docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
docs/spec/draft/source-requirements.md Outdated Show resolved Hide resolved
TomHennen and others added 4 commits October 24, 2024 21:30
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Co-authored-by: Marcela Melara <marcela.melara@intel.com>
Signed-off-by: Tom Hennen <TomHennen@users.noreply.github.com>
Copy link
Contributor

@marcelamelara marcelamelara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for these final wording changes @TomHennen ! LGTM

Copy link
Contributor

@zachariahcox zachariahcox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me and I recommend merging it.

We will need to come back at some point to review a real life expungement and see what can actually be accomplished to here. How smooth a process can we make it in git?

I'm also not sure the scs can be on the hook for enforcing that the data removals were for legal reasons.
Generally I think "the owner of the intellectual property in the repo (the root repo owner, in gh terms) can remove data. They should only do this for legal or privacy reasons due to the risk of severe reputational consequences (damage to artifact chain of custody)." Ie: it's indistinguishable from a repo hijack so you should have a good reason.

@TomHennen
Copy link
Contributor Author

This looks good to me and I recommend merging it.

Thanks!

We will need to come back at some point to review a real life expungement and see what can actually be accomplished to here. How smooth a process can we make it in git?

Agreed. We'll probably want similar feedback for the entire build track. Once we're in the RC phase (hopefully soon!) we can try to elicit that sort of feedback?

I'm also not sure the scs can be on the hook for enforcing that the data removals were for legal reasons. Generally I think "the owner of the intellectual property in the repo (the root repo owner, in gh terms) can remove data. They should only do this for legal or privacy reasons due to the risk of severe reputational consequences (damage to artifact chain of custody)." Ie: it's indistinguishable from a repo hijack so you should have a good reason.

Filed #1222 to track.

@TomHennen TomHennen merged commit 2941098 into slsa-framework:main Oct 25, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

Harden 'safe-expunging-process'
6 participants