allow redundant security release alert suppression (#24461)

This commit is contained in:
Forrest 2023-04-17 12:57:49 -07:00 committed by GitHub
parent 0d1e625379
commit be0c659c9c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 108 additions and 2 deletions

View file

@ -980,7 +980,7 @@ func (a *Server) doReleaseAlertSync(ctx context.Context, current vc.Target, visi
}
}
if sp := visitor.NewestSecurityPatch(); sp.Ok() && sp.NewerThan(current) {
if sp := visitor.NewestSecurityPatch(); sp.Ok() && sp.NewerThan(current) && !sp.SecurityPatchAltOf(current) {
// explicit security patch alerts have a more limited audience, so we generate
// them as their own separate alert.
log.Warnf("A newer security patch has been detected. current=%s, patch=%s", current.Version(), sp.Version())

View file

@ -306,6 +306,14 @@ func TestLabelParse(t *testing.T) {
},
desc: "normalize caps and spaces",
},
{
notes: "labels: security-patch=yes, security-patch-alts=v1.2.3|v1.2.4",
expect: map[string]string{
vc.LabelSecurityPatch: "yes",
vc.LabelSecurityPatchAlts: "v1.2.3|v1.2.4",
},
desc: "real-world label set",
},
{
notes: "labels: hello=world, greeting='Hey there! how are you?', , count=7",
expect: map[string]string{

View file

@ -20,6 +20,7 @@ import (
"fmt"
"regexp"
"strconv"
"strings"
"golang.org/x/mod/semver"
)
@ -31,6 +32,10 @@ import (
// criterea for keys and values in the future.
var isValidLabel = regexp.MustCompile(`^[a-z0-9\.\-\/]+$`).MatchString
// isValidLabelVal is the same as isValidLabel except that it also permits '|' which is interpreted
// as a list delmeter for some labels.
var isValidLabelVal = regexp.MustCompile(`^[a-z0-9\.\-\/|]+$`).MatchString
// IsValidTargetKey checks if a string is a valid installation target key.
func IsValidTargetKey(key string) bool {
return isValidLabel(key)
@ -38,12 +43,15 @@ func IsValidTargetKey(key string) bool {
// IsValidTargetVal checks if a string is a valid installtion target value.
func IsValidTargetVal(val string) bool {
return isValidLabel(val)
return isValidLabelVal(val)
}
// LabelSecurityPatch indicates that a release is a security patch when set to 'yes'.
const LabelSecurityPatch = "security-patch"
// LabelSecurityPatchAlts lists alternative versions that provide the same security fixes.
const LabelSecurityPatchAlts = "security-patch-alts"
// LabelVersion is the only required label for an installation target and must be
// valid go-style semver.
const LabelVersion = "version"
@ -53,6 +61,7 @@ type TargetOption func(*targetOptions)
type targetOptions struct {
secPatch bool
secAlts []string
}
// SecurityPatch sets the security-patch=yes label if true.
@ -62,6 +71,13 @@ func SecurityPatch(sec bool) TargetOption {
}
}
// SecurityPatchAlts appends version strings to the security-patch-alts label.
func SecurityPatchAlts(alts ...string) TargetOption {
return func(opts *targetOptions) {
opts.secAlts = append(opts.secAlts, alts...)
}
}
// Target is a description of an available installation target. A given "release"
// (e.g. v1.2.3) may consist of one or more targets depending on how intallation
// targets are being modeled (e.g. TUF creates a separate target for each
@ -85,6 +101,11 @@ func NewTarget(version string, opts ...TargetOption) Target {
if options.secPatch {
target[LabelSecurityPatch] = "yes"
}
if len(options.secAlts) != 0 {
target[LabelSecurityPatchAlts] = strings.Join(options.secAlts, "|")
}
return target
}
@ -124,6 +145,43 @@ func (t Target) SecurityPatch() bool {
return t[LabelSecurityPatch] == "yes"
}
// SecurityPatchAltOf performs a bidirectional check to see if this target and another
// are security patch alternates (i.e. wether or not they provide the same security fix).
func (t Target) SecurityPatchAltOf(other Target) bool {
if !t.Ok() || !other.Ok() {
return false
}
var alt bool
t.iterSecAlts(func(v string) {
if semver.Compare(v, other.Version()) == 0 {
alt = true
}
})
other.iterSecAlts(func(v string) {
if semver.Compare(v, t.Version()) == 0 {
alt = true
}
})
return alt
}
// iterSecAlts is a helper for iterating the valide values of the
// security-patch-alts label.
func (t Target) iterSecAlts(fn func(v string)) {
for _, alt := range strings.Split(t[LabelSecurityPatchAlts], "|") {
alt = strings.TrimSpace(alt)
if !semver.IsValid(alt) {
continue
}
fn(alt)
}
}
// Prerelease checks if this target represents a prerelease installation target
// (e.g. v1.2.3-alpha.1).
func (t Target) Prerelease() bool {

View file

@ -22,6 +22,46 @@ import (
"github.com/stretchr/testify/require"
)
func TestSecurityPatchAlts(t *testing.T) {
tts := []struct {
desc string
a, b Target
alt bool
}{
{
desc: "basic alt case",
a: NewTarget("v1.2.3", SecurityPatch(true), SecurityPatchAlts("v1.2.2", "v1.2.1")),
b: NewTarget("v1.2.1"),
alt: true,
},
{
desc: "minimal alt case",
a: NewTarget("v1.2.3", SecurityPatchAlts("v1.2.1")),
b: NewTarget("v1.2.1"),
alt: true,
},
{
desc: "trivial non-alt case",
a: NewTarget("v1.2.3"),
b: NewTarget("v1.2.1"),
alt: false,
},
{
desc: "non-matching non-alt case case",
a: NewTarget("v1.2.3", SecurityPatchAlts("v1.2.2")),
b: NewTarget("v1.2.1"),
alt: false,
},
}
for _, tt := range tts {
// check alt status is expected
require.Equal(t, tt.alt, tt.a.SecurityPatchAltOf(tt.b), "desc=%q", tt.desc)
// check alt status is bidirectional
require.Equal(t, tt.alt, tt.b.SecurityPatchAltOf(tt.a), "desc=%q", tt.desc)
}
}
func TestVisitorBasics(t *testing.T) {
tts := []struct {
versions []string