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

feat: add a default security policy #12

Merged
merged 7 commits into from
Sep 29, 2023
Merged

feat: add a default security policy #12

merged 7 commits into from
Sep 29, 2023

Conversation

fgreinacher
Copy link
Contributor

No description provided.

@fgreinacher fgreinacher requested a review from nejch August 21, 2023 08:37
@fgreinacher
Copy link
Contributor Author

fgreinacher commented Aug 21, 2023

@jan-kiszka @StefanSchroeder Would you also have a look? This serves as a fallback only. Repo-level security policies are preferred.

SECURITY.md Outdated Show resolved Hide resolved
Copy link
Member

@nejch nejch left a comment

Choose a reason for hiding this comment

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

Looks nice and simple @fgreinacher.

As I said i think it might be nice to announce this to projects in https://github.com/siemens?q=&type=source before we merge if they don't have SECURITY.md / .github/SECURITY.md / docs/SECURITY.md, since this will become opt-out immediately after merge IIUC. WDYT? Even something silly like this.

import os

from github import Auth, Github

ISSUE_TITLE = "Default Siemens security policy"
ISSUE_DESCRIPTION = """Dear project maintainers,
The default Siemens organization-wide security poolicy will be applied on xx-xx-xxxx.
If you'd like to opt out, please provide your own [`SECURITY.md` policy](https://docs.github.com/en/code-security/getting-started/adding-a-security-policy-to-your-repository).

For more details, see https://github.com/siemens/.github/pull/12."""


auth = Auth.Token(os.getenv("GITHUB_TOKEN"))
gh = Github(auth=auth)
org = gh.get_organization("siemens")

for repo in org.get_repos(type="sources"):
    contents = repo.get_content() + repo.get_content("docs") + repo.get_content(".github")

    if any([f.path.endswith("SECURITY.md") for f in contents]):
        continue

    repo.create_issue(title=ISSUE_TITLE, body=ISSUE_DESCRIPTION)

(completely untested so might not work at all but you get teh idea 😸)

@fgreinacher
Copy link
Contributor Author

@nejch Great idea with the issue. I'll continue with this after my vacation and we can aim for enabling this in the end of September.

@fgreinacher fgreinacher self-assigned this Aug 21, 2023
@jan-kiszka
Copy link

We will also need a way to exclude forks like https://github.com/siemens/linux from this.

@nejch
Copy link
Member

nejch commented Aug 21, 2023

We will also need a way to exclude forks like https://github.com/siemens/linux from this.

Maybe we can check that with https://github.com/siemens-testing-main to see if it's applied at all to forks, or if there's a way to disable it. (Edit: seems like it is applied, just checked https://github.com/microsoft/openjdk-jdk/security which has no file).

Otherwise, worst case scenario maybe we can add a note this does not apply to forks with an explicit list? 🤔

@jan-kiszka
Copy link

jan-kiszka commented Aug 22, 2023

Forks might be used as PR sources, so I'd really like them to be clean, even "just" visually.

@nejch
Copy link
Member

nejch commented Aug 22, 2023

Forks might be used as PR sources, so I'd really like them to be clean, even "just" visually.

From what I can see, once we set an org-level SECURITY.md there's no way to disable displaying that on repo level - this is not supported by GitHub it seems at the moment.

So that might be a blocker unless we push local SECURITY.md for forks to override the org-level one.
Or create something like https://github.com/siemens-collaboration and move those projects there. This might actually be a bit cleaner to separate sources vs. forks.

/cc @fgreinacher @bufferoverflow

@fgreinacher
Copy link
Contributor Author

Hm, that's for sure a blocker. Maybe better to handle that from our internal project, pushing the security policy from there - would also be a bit more explicit and we could notify maintainers there.

@bufferoverflow
Copy link
Member

I just checked https://github.com/microsoft/openjdk-jdk the only effect is within the About section:

image

There is the Security policy which links to the default one, so I do not really see a problem with that. The git repo is of course untouched.

@nejch
Copy link
Member

nejch commented Aug 22, 2023

@bufferoverflow I was looking at this in the security tab:

https://github.com/microsoft/openjdk-jdk/security

image

But true, it's not in the repo, not sure if that's what @jan-kiszka meant.

@bufferoverflow
Copy link
Member

Yes, but security tab is no problem from my perspective. We could maybe mention within the policy that this is not valid for forks within the organization.

@jan-kiszka
Copy link

If there is no file visibly sneaked into the forked repo, just the security tab augmented, a statement that this policy does not apply to forks should be enough.

@nejch
Copy link
Member

nejch commented Aug 22, 2023

If there is no file visibly sneaked into the forked repo, just the security tab augmented, a statement that this policy does not apply to forks should be enough.

No, this was never going to be the case :) so this can be solved here in this PR.

Copy link
Contributor Author

@fgreinacher fgreinacher left a comment

Choose a reason for hiding this comment

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

@nejch @jan-kiszka I added some words regarding the scope of the policy in f4c8f67. PTAL 🙇

SECURITY.md Outdated Show resolved Hide resolved
SECURITY.md Outdated Show resolved Hide resolved
@fgreinacher
Copy link
Contributor Author

fgreinacher commented Sep 20, 2023

Announcements sent. Failures look alright, there are some repos where issues are disabled and a lot of archived repos.

@gernot-h
Copy link
Member

If there is no file visibly sneaked into the forked repo, just the security tab augmented, a statement that this policy does not apply to forks should be enough.

No, this was never going to be the case :) so this can be solved here in this PR.

So what should we do with the issues in the forks? Just close them?

@jan-kiszka, does it make sense to have outdated forks like https://github.com/siemens/linux-ipipe at all?

@nejch
Copy link
Member

nejch commented Sep 21, 2023

If there is no file visibly sneaked into the forked repo, just the security tab augmented, a statement that this policy does not apply to forks should be enough.

No, this was never going to be the case :) so this can be solved here in this PR.

So what should we do with the issues in the forks? Just close them?

@jan-kiszka, does it make sense to have outdated forks like https://github.com/siemens/linux-ipipe at all?

@gernot-h we're already discussing this in our community management repo, I'll ping you there so can join the discussion.

Regarding issues in forks - we actually filtered out forks on GitHub, but linux-ipipe is not technically a fork on GitHub as I guess it wasn't forked from the GitHub mirror. IMO it can be closed there and yes, probably archived if it's not active.

@nejch
Copy link
Member

nejch commented Sep 28, 2023

@fgreinacher should we merge this now? I had a reminder set for this, but we have this in draft still 🙇

@fgreinacher fgreinacher marked this pull request as ready for review September 29, 2023 07:30
@fgreinacher
Copy link
Contributor Author

@fgreinacher should we merge this now? I had a reminder set for this, but we have this in draft still 🙇

@nejch Yes, I haven't received any negative feedback. Feel free to do a last pass and merge 🙇

@nejch
Copy link
Member

nejch commented Sep 29, 2023

Thanks @fgreinacher & everyone for your feedback! LGTM.

@nejch nejch merged commit d3d097e into main Sep 29, 2023
1 check passed
@nejch nejch deleted the feat/security-policy branch September 29, 2023 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants