Skip to content

Commit 9a670da

Browse files
authored
feat: Add related computation to cron (#4704)
Make `related` vulns be regularly recomputed, so when `A` updates to related to `B`, `B` will get updated to relate to `A`. A withdrawn vuln will have non-withdrawn vulns added to its related field, but it will not add it's ID to the related fields of its own related vulns. (i.e. if `A` is withdrawn and has `related: ["C"]`, `C` will not get `A` added to its related field. If non-withdrawn `B` has `related: ["A"]`, `A` will end up with `related: ["B", "C"]`)
1 parent 0844879 commit 9a670da

11 files changed

Lines changed: 358 additions & 10 deletions

File tree

gcp/workers/recoverer/recoverer.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,22 @@ def transaction():
187187
else:
188188
modified = datetime.datetime.now(datetime.UTC)
189189

190+
elif f == 'related':
191+
related_group = osv.RelatedGroup.get_by_id(vuln_id)
192+
if related_group is None:
193+
related = []
194+
related_modified = datetime.datetime.now(datetime.UTC)
195+
else:
196+
related = related_group.related_ids
197+
related_modified = related_group.last_modified
198+
# Only update the modified time if it's actually being modified
199+
if vuln_proto.related != related:
200+
vuln_proto.related[:] = related
201+
if related_modified > modified:
202+
modified = related_modified
203+
else:
204+
modified = datetime.datetime.now(datetime.UTC)
205+
190206
vuln_proto.modified.FromDatetime(modified)
191207
osv.ListedVulnerability.from_vulnerability(vuln_proto).put()
192208
vuln.modified = modified

gcp/workers/worker/worker.py

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -612,14 +612,6 @@ def xact():
612612
ds_vuln = osv.Vulnerability.get_by_id(vulnerability.id)
613613
is_new_bug = ds_vuln is None
614614

615-
# Compute the related fields here first.
616-
# TODO(michaelkedar): Make a related computation in relations cron
617-
related_raw = vulnerability.related
618-
q = osv.Vulnerability.query(
619-
osv.Vulnerability.related_raw == vulnerability.id)
620-
related = set(vulnerability.related).union(set(r.key.id() for r in q))
621-
vulnerability.related[:] = sorted(related)
622-
623615
old_published = None
624616

625617
# Update the schema version
@@ -673,13 +665,15 @@ def xact():
673665
new_vulnerability.aliases.clear()
674666
old_vulnerability.upstream.clear()
675667
new_vulnerability.upstream.clear()
668+
old_vulnerability.related.clear()
669+
new_vulnerability.related.clear()
676670

677671
has_changed = old_vulnerability != new_vulnerability
678672

679673
ds_vuln.is_withdrawn = vulnerability.HasField('withdrawn')
680674
ds_vuln.modified_raw = orig_modified_date
681675
ds_vuln.alias_raw = list(vulnerability.aliases)
682-
ds_vuln.related_raw = list(related_raw)
676+
ds_vuln.related_raw = list(vulnerability.related)
683677
ds_vuln.upstream_raw = list(vulnerability.upstream)
684678
# Update the bug entity based on the comparison.
685679
if has_changed:
@@ -701,6 +695,10 @@ def xact():
701695
if upstream_group:
702696
vulnerability.upstream[:] = sorted(upstream_group.upstream_ids)
703697
ds_vuln.modified = max(upstream_group.last_modified, ds_vuln.modified)
698+
related_group = osv.RelatedGroup.get_by_id(vulnerability.id)
699+
if related_group:
700+
vulnerability.related[:] = sorted(related_group.related_ids)
701+
ds_vuln.modified = max(related_group.modified, ds_vuln.modified)
704702
# Make sure modified date is >= withdrawn date
705703
if ds_vuln.is_withdrawn and vulnerability.withdrawn.ToDatetime(
706704
datetime.UTC) > ds_vuln.modified:

go/cmd/relations/related.go

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
package main
2+
3+
import (
4+
"context"
5+
"errors"
6+
"fmt"
7+
"log/slog"
8+
"slices"
9+
"time"
10+
11+
"cloud.google.com/go/datastore"
12+
"github.com/google/osv.dev/go/logger"
13+
"github.com/google/osv.dev/go/osv/models"
14+
"google.golang.org/api/iterator"
15+
)
16+
17+
// computeRelated computes all related groups for the given vulns.
18+
// `groups` is a map of vuln IDs to their related IDs.
19+
// `withdrawnVulns` is a map of withdrawn vulns.
20+
// Returns a map of vuln IDs to their related IDs, with the inverse relation added.
21+
// `groups` is modified in place.
22+
func computeRelated(groups map[string][]string, withdrawnVulns map[string]struct{}) map[string][]string {
23+
// Add the inverse relation of the groups to the map
24+
for id, group := range groups {
25+
if _, ok := withdrawnVulns[id]; ok {
26+
// We want to prevent withdrawn vulns IDs from being added to related groups,
27+
// if the withdrawn vuln itself references other non-withdrawn vulns.
28+
// For example:
29+
// - If A (withdrawn) relates to B (valid), B should NOT list A.
30+
// - If A (valid) relates to B (withdrawn), B SHOULD list A.
31+
continue
32+
}
33+
for _, related := range group {
34+
if slices.Contains(groups[related], id) {
35+
continue
36+
}
37+
groups[related] = append(groups[related], id)
38+
slices.Sort(groups[related])
39+
}
40+
}
41+
42+
return groups
43+
}
44+
45+
func updateRelated(ctx context.Context, cl *datastore.Client, id string, relatedIDs []string, ch chan<- Update) error {
46+
if len(relatedIDs) == 0 {
47+
logger.Info("Deleting related group due to no related vulns", slog.String("id", id))
48+
if err := cl.Delete(ctx, datastore.NameKey("RelatedGroup", id, nil)); err != nil {
49+
return err
50+
}
51+
ch <- Update{
52+
ID: id,
53+
Timestamp: time.Now().UTC(),
54+
Field: updateFieldRelated,
55+
Value: nil,
56+
}
57+
58+
return nil
59+
}
60+
61+
group := models.RelatedGroup{
62+
RelatedIDs: relatedIDs,
63+
Modified: time.Now().UTC(),
64+
}
65+
if _, err := cl.Put(ctx, datastore.NameKey("RelatedGroup", id, nil), &group); err != nil {
66+
return err
67+
}
68+
ch <- Update{
69+
ID: id,
70+
Timestamp: group.Modified,
71+
Field: updateFieldRelated,
72+
Value: relatedIDs,
73+
}
74+
75+
return nil
76+
}
77+
78+
func ComputeRelatedGroups(ctx context.Context, cl *datastore.Client, ch chan<- Update) error {
79+
// Query for all vulns that have related.
80+
// It's easier to recompute all groups than to try and figure out which ones
81+
// need to be recomputed.
82+
logger.Info("Retrieving vulns for related computation...")
83+
q := datastore.NewQuery("Vulnerability").FilterField("related_raw", ">", "")
84+
85+
rawRelated := make(map[string][]string)
86+
withdrawnVulns := make(map[string]struct{})
87+
it := cl.Run(ctx, q)
88+
for {
89+
var v models.Vulnerability
90+
_, err := it.Next(&v)
91+
if errors.Is(err, iterator.Done) {
92+
break
93+
}
94+
if err != nil {
95+
return fmt.Errorf("failed to iterate vulnerabilities: %w", err)
96+
}
97+
if v.IsWithdrawn {
98+
withdrawnVulns[v.Key.Name] = struct{}{}
99+
}
100+
related := slices.Clone(v.RelatedRaw)
101+
slices.Sort(related)
102+
related = slices.Compact(related)
103+
rawRelated[v.Key.Name] = related
104+
}
105+
logger.Info("Retrieved vulns with related ids", slog.Int("count", len(rawRelated)))
106+
107+
logger.Info("Retrieving related groups...")
108+
q = datastore.NewQuery("RelatedGroup")
109+
it = cl.Run(ctx, q)
110+
relatedGroups := make(map[string]models.RelatedGroup)
111+
for {
112+
var group models.RelatedGroup
113+
_, err := it.Next(&group)
114+
if errors.Is(err, iterator.Done) {
115+
break
116+
}
117+
if err != nil {
118+
return fmt.Errorf("failed to iterate related groups: %w", err)
119+
}
120+
relatedGroups[group.Key.Name] = group
121+
}
122+
logger.Info("Related groups successfully retrieved", slog.Int("count", len(relatedGroups)))
123+
124+
related := computeRelated(rawRelated, withdrawnVulns)
125+
126+
for id, relatedIDs := range related {
127+
g, ok := relatedGroups[id]
128+
delete(relatedGroups, id)
129+
if !ok || !slices.Equal(g.RelatedIDs, relatedIDs) {
130+
if err := updateRelated(ctx, cl, id, relatedIDs, ch); err != nil {
131+
return fmt.Errorf("failed to update related group: %w", err)
132+
}
133+
}
134+
}
135+
136+
// The remaining groups in relatedGroups are the ones that are no longer
137+
// present in the vulns, so we delete them.
138+
for id := range relatedGroups {
139+
if err := updateRelated(ctx, cl, id, nil, ch); err != nil {
140+
return fmt.Errorf("failed to delete related group: %w", err)
141+
}
142+
}
143+
144+
return nil
145+
}

go/cmd/relations/related_test.go

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package main
2+
3+
import (
4+
"context"
5+
"slices"
6+
"testing"
7+
"time"
8+
9+
"cloud.google.com/go/datastore"
10+
"github.com/google/go-cmp/cmp"
11+
"github.com/google/osv.dev/go/osv/models"
12+
"github.com/google/osv.dev/go/testutils"
13+
)
14+
15+
func TestComputeRelated(t *testing.T) {
16+
tests := []struct {
17+
name string
18+
groups map[string][]string
19+
want map[string][]string
20+
}{
21+
{
22+
name: "Unrelated groups",
23+
groups: map[string][]string{"A": {"B"}, "C": {"D"}},
24+
want: map[string][]string{"A": {"B"}, "B": {"A"}, "C": {"D"}, "D": {"C"}},
25+
},
26+
{
27+
name: "Related groups",
28+
groups: map[string][]string{"A": {"B", "C"}, "B": {"A"}},
29+
want: map[string][]string{"A": {"B", "C"}, "B": {"A"}, "C": {"A"}},
30+
},
31+
{
32+
name: "Already computed",
33+
groups: map[string][]string{"A": {"B"}, "B": {"A"}},
34+
want: map[string][]string{"A": {"B"}, "B": {"A"}},
35+
},
36+
{
37+
name: "Circular",
38+
groups: map[string][]string{"A": {"B"}, "B": {"C"}, "C": {"A"}},
39+
want: map[string][]string{"A": {"B", "C"}, "B": {"A", "C"}, "C": {"A", "B"}},
40+
},
41+
{
42+
name: "Empty",
43+
groups: map[string][]string{},
44+
want: map[string][]string{},
45+
},
46+
}
47+
48+
for _, tt := range tests {
49+
t.Run(tt.name, func(t *testing.T) {
50+
got := computeRelated(tt.groups, map[string]struct{}{})
51+
if diff := cmp.Diff(tt.want, got); diff != "" {
52+
t.Errorf("computeRelated() mismatch (-want +got):\n%s", diff)
53+
}
54+
})
55+
}
56+
}
57+
58+
func TestComputeRelatedGroups(t *testing.T) {
59+
ctx := context.Background()
60+
dsClient := testutils.MustNewDatastoreClientForTesting(t)
61+
62+
// Setup Datastore
63+
vulns := []*models.Vulnerability{
64+
{
65+
Key: datastore.NameKey("Vulnerability", "A", nil),
66+
RelatedRaw: []string{"B"},
67+
Modified: time.Now().UTC(),
68+
},
69+
{
70+
Key: datastore.NameKey("Vulnerability", "B", nil),
71+
RelatedRaw: []string{"A"},
72+
Modified: time.Now().UTC(),
73+
},
74+
{
75+
Key: datastore.NameKey("Vulnerability", "C", nil),
76+
RelatedRaw: []string{"A", "D"},
77+
Modified: time.Now().UTC(),
78+
},
79+
{
80+
Key: datastore.NameKey("Vulnerability", "D", nil),
81+
RelatedRaw: []string{"E"}, // Withdrawn, should be ignored
82+
Modified: time.Now().UTC(),
83+
IsWithdrawn: true,
84+
},
85+
}
86+
keys := make([]*datastore.Key, len(vulns))
87+
for i, v := range vulns {
88+
keys[i] = v.Key
89+
}
90+
91+
if _, err := dsClient.PutMulti(ctx, keys, vulns); err != nil {
92+
t.Fatalf("failed to put vulns: %v", err)
93+
}
94+
95+
ch := make(chan Update, 100)
96+
if err := ComputeRelatedGroups(ctx, dsClient, ch); err != nil {
97+
t.Fatalf("ComputeRelatedGroups failed: %v", err)
98+
}
99+
close(ch)
100+
101+
// Check results
102+
var groups []models.RelatedGroup
103+
if _, err := dsClient.GetAll(ctx, datastore.NewQuery("RelatedGroup"), &groups); err != nil {
104+
t.Fatalf("failed to get related groups: %v", err)
105+
}
106+
107+
expected := map[string][]string{
108+
"A": {"B", "C"},
109+
"B": {"A"},
110+
"C": {"A", "D"},
111+
"D": {"C", "E"},
112+
}
113+
114+
got := make(map[string][]string)
115+
for _, g := range groups {
116+
slices.Sort(g.RelatedIDs)
117+
got[g.Key.Name] = g.RelatedIDs
118+
}
119+
120+
if diff := cmp.Diff(expected, got); diff != "" {
121+
t.Errorf("RelatedGroups mismatch (-want +got):\n%s", diff)
122+
}
123+
}

go/cmd/relations/relations.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ func main() {
6161
logger.Error("failed to compute upstream groups", slog.Any("err", err))
6262
}
6363
})
64+
wg.Go(func() {
65+
if err := ComputeRelatedGroups(ctx, gc.datastoreClient, updater.Ch); err != nil {
66+
logger.Error("failed to compute related groups", slog.Any("err", err))
67+
}
68+
})
6469
wg.Wait()
6570
updater.Finish()
6671
}

go/cmd/relations/update.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,21 @@ func (u *Updater) run(ctx context.Context) {
160160
if u.Timestamp.After(modified) {
161161
modified = u.Timestamp
162162
}
163+
case updateFieldRelated:
164+
val, ok := u.Value.([]string)
165+
if !ok {
166+
logger.Error("updated related are not []string", slog.String("id", id))
167+
continue
168+
}
169+
if slices.Compare(v.GetRelated(), val) == 0 {
170+
continue
171+
}
172+
hasUpdates = true
173+
v.Related = val
174+
updatedFields = append(updatedFields, "related")
175+
if u.Timestamp.After(modified) {
176+
modified = u.Timestamp
177+
}
163178
default:
164179
logger.Error("unsupported update field", slog.Any("updateField", u.Field), slog.String("id", id))
165180
}

go/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
cloud.google.com/go/pubsub/v2 v2.3.0
99
cloud.google.com/go/storage v1.59.1
1010
github.com/charmbracelet/lipgloss v1.1.0
11+
github.com/google/go-cmp v0.7.0
1112
github.com/ossf/osv-schema/bindings/go v0.0.0-20260128001339-9d03e8f4632b
1213
golang.org/x/sync v0.19.0
1314
google.golang.org/api v0.263.0

0 commit comments

Comments
 (0)