Skip to content

Commit

Permalink
datastore: avoid update_operation timing window by creating in txn
Browse files Browse the repository at this point in the history
Previously the update_operation was created outside the transaction which updated all vulns. This meant other updaters could refresh the latest_update_operation materialized view, making it visible before any vulns were committed.

The upshot was a window where vulnerability reports could give false-negatives (they couldn't see any vulns associated with the most recent update_operation, because they hadn't been committed yet)

Signed-off-by: Mark Frost <[email protected]>
  • Loading branch information
frostmar authored and crozzy committed Apr 8, 2024
1 parent a19766a commit 813e7cc
Showing 1 changed file with 7 additions and 20 deletions.
27 changes: 7 additions & 20 deletions datastore/postgres/updatevulnerabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ func (s *MatcherStore) updateVulnerabilities(ctx context.Context, updater string
$3,
(SELECT id FROM vuln WHERE hash_kind = $1 AND hash = $2))
ON CONFLICT DO NOTHING;`
undo = `DELETE FROM update_operation WHERE id = $1;`
refreshView = `REFRESH MATERIALIZED VIEW CONCURRENTLY latest_update_operations;`
)

Expand All @@ -130,30 +129,19 @@ func (s *MatcherStore) updateVulnerabilities(ctx context.Context, updater string

start := time.Now()

if err := s.pool.QueryRow(ctx, create, updater, string(fingerprint)).Scan(&uoID, &ref); err != nil {
return uuid.Nil, fmt.Errorf("failed to create update_operation: %w", err)
}
var success bool
defer func() {
if !success {
if _, err := s.pool.Exec(ctx, undo, uoID); err != nil {
zlog.Error(ctx).
Err(err).
Stringer("ref", ref).
Msg("unable to remove update operation")
}
}
}()

updateVulnerabilitiesCounter.WithLabelValues("create", strconv.FormatBool(delta)).Add(1)
updateVulnerabilitiesDuration.WithLabelValues("create", strconv.FormatBool(delta)).Observe(time.Since(start).Seconds())

tx, err := s.pool.Begin(ctx)
if err != nil {
return uuid.Nil, fmt.Errorf("unable to start transaction: %w", err)
}
defer tx.Rollback(ctx)

if err := tx.QueryRow(ctx, create, updater, string(fingerprint)).Scan(&uoID, &ref); err != nil {
return uuid.Nil, fmt.Errorf("failed to create update_operation: %w", err)

Check warning on line 139 in datastore/postgres/updatevulnerabilities.go

View check run for this annotation

Codecov / codecov/patch

datastore/postgres/updatevulnerabilities.go#L139

Added line #L139 was not covered by tests
}

updateVulnerabilitiesCounter.WithLabelValues("create", strconv.FormatBool(delta)).Add(1)
updateVulnerabilitiesDuration.WithLabelValues("create", strconv.FormatBool(delta)).Observe(time.Since(start).Seconds())

zlog.Debug(ctx).
Str("ref", ref.String()).
Msg("update_operation created")
Expand Down Expand Up @@ -275,7 +263,6 @@ func (s *MatcherStore) updateVulnerabilities(ctx context.Context, updater string
return uuid.Nil, fmt.Errorf("could not refresh latest_update_operations: %w", err)
}

success = true
zlog.Debug(ctx).
Str("ref", ref.String()).
Int("skipped", skipCt).
Expand Down

0 comments on commit 813e7cc

Please sign in to comment.