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

SECURITY.md updates and ownership #8455

Merged
merged 1 commit into from
Nov 2, 2023
Merged

SECURITY.md updates and ownership #8455

merged 1 commit into from
Nov 2, 2023

Conversation

raphdev
Copy link
Contributor

@raphdev raphdev commented Oct 13, 2023

closes: #8171

Description

This PR makes revisions to SECURITY.md to reflect currently supported versions.

We also add a CODEOWNERS file and make SECURITY.md owned by the security team (@Agoric/sec). Since the team is ultimately accountable for the expectations set in this doc, we want to make sure modifications are reviewed by security for awareness and input.

SECURITY.md Outdated
@@ -2,7 +2,7 @@

## Supported Versions

The current `master` and `release-pismo` branches are supported with security updates.
The current `master` and latest `agoric-upgrade-*` branches are supported with security updates.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what the correct descriptor should be, should we include latest and pre-release tagged releases in https://github.com/Agoric/agoric-sdk/releases ?

Copy link
Member

Choose a reason for hiding this comment

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

Branch of that shape is definitely not correct, it should be agoric-upgrade-* tags, but as you mention without restrictions it would include outdated releases that may have known issues. Maybe something like

The current master branch and the agoric-upgrade-* tags corresponding to the latest release and pre-release versions.

PS: We do have release-* branches, but those are mostly to stage the creation of these release tags, and may similarly become outdated like release-pismo now is.

@raphdev raphdev marked this pull request as ready for review October 13, 2023 21:45
CODEOWNERS Outdated
@@ -0,0 +1 @@
SECURITY.md @Agoric/sec
Copy link
Member

Choose a reason for hiding this comment

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

GH seem to be complaining about this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The permission changes are pending. This group needs write access to repo which will be done before merging.

SECURITY.md Outdated
@@ -2,7 +2,7 @@

## Supported Versions

The current `master` and `release-pismo` branches are supported with security updates.
The current `master` and latest `agoric-upgrade-*` branches are supported with security updates.
Copy link
Member

Choose a reason for hiding this comment

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

Branch of that shape is definitely not correct, it should be agoric-upgrade-* tags, but as you mention without restrictions it would include outdated releases that may have known issues. Maybe something like

The current master branch and the agoric-upgrade-* tags corresponding to the latest release and pre-release versions.

PS: We do have release-* branches, but those are mostly to stage the creation of these release tags, and may similarly become outdated like release-pismo now is.

@raphdev raphdev requested a review from mhofman November 1, 2023 13:52
Copy link
Contributor

@ivanlei ivanlei left a comment

Choose a reason for hiding this comment

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

LGTM

CODEOWNERS Outdated
@@ -0,0 +1 @@
SECURITY.md @Agoric/security
Copy link
Member

Choose a reason for hiding this comment

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

Nit: new line missing

@@ -2,7 +2,7 @@

## Supported Versions

The current `master` and `release-pismo` branches are supported with security updates.
The current `master` and only the latest `agoric-upgrade-*` tagged release and pre-release are supported with security updates.
Copy link
Member

Choose a reason for hiding this comment

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

Note: I don't expect us to ever have a latest pre-release tag that is older (aka ancestor commit) than the latest release tag, so this works, but it initially worried me about opening ourselves to reports on outdated pre-release tags. More precisely, given our release process, the latest pre-release tag should always be the same revision as the latest release tag, or newer when we're about to release an upgrade.

@raphdev raphdev added automerge:rebase Automatically rebase updates, then merge automerge:squash Automatically squash merge and removed automerge:rebase Automatically rebase updates, then merge labels Nov 2, 2023
@mergify mergify bot merged commit 0e22bfa into master Nov 2, 2023
88 of 96 checks passed
@mergify mergify bot deleted the raph/secmd-owners branch November 2, 2023 23:37
mhofman pushed a commit that referenced this pull request Nov 8, 2023
mhofman pushed a commit that referenced this pull request Nov 8, 2023
mhofman pushed a commit that referenced this pull request Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Review recent security.md edits & add codeowners to ensure future changes are reviewed
3 participants