google / google/osv.dev

Worker retries oversized vulnerability updates indefinitely after importer drops message data

Open
#5,984 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
2.9k
Forks
369
Avg merge
1d 17h
Merged PRs (30d)
149

Description

**Describe the bug**

When `publishUpdate` produces a compressed vulnerability proto larger than `maxPubSubMessageSize`, it logs a warning and publishes the update with `Data = nil` while leaving `deleted=false`.

The Go worker's `parseVuln` treats empty message data as a valid nil vulnerability (documented there as expected for delete requests). `handleMessage` then constructs a `TaskUpdate`, and `Engine.handleUpdate` returns `vuln not provided`. The subscriber NACKs the message, so the same oversized update can be retried without a path to successful processing.

Relevant paths on current `master`:
- `go/internal/importer/importer.go` (`publishUpdate`)
- `go/internal/worker/subscriber.go` (`parseVuln`, `handleMessage`)
- `go/internal/worker/engine.go` (`handleUpdate`)

**To Reproduce**

1. Construct a non-deleted vulnerability whose zstd-compressed protobuf exceeds `maxPubSubMessageSize`.
2. Pass it through `publishUpdate`.
3. Observe that the published message has empty data and `deleted=false`.
4. Deliver that message to the worker subscriber.
5. `parseVuln` returns `nil, nil`, the task is classified as `TaskUpdate`, `handleUpdate` returns `vuln not provided`, and the message is NACKed.

**Expected behaviour**

Oversized vulnerability updates should have an explicit recoverable or terminal handling path rather than becoming an update message that deterministically fails and is retried.

A small fix could either avoid publishing this invalid update shape or make the worker/importer coordinate an explicit oversized-record path. The exact approach is probably best decided by maintainers because the importer comment currently says to let the worker decide what to do.

**Additional context**

I searched open/closed issues and PRs for the warning/error strings and `maxPubSubMessageSize` and did not find an existing report. I have not started implementation, per the repository's issue-first/assignment requirement. I am happy to work on a narrowly scoped fix and regression test if this issue is assigned to me.

Contributor guide

Open the contributing guide

Research direction

Trace the oversized-message path through go/internal/importer/importer.go, then follow parseVuln and handleMessage in go/internal/worker/subscriber.go into handleUpdate in go/internal/worker/engine.go. Reproduce the non-deleted empty-data case and add a regression test for an explicit recoverable or terminal handling path, with maintainers deciding the coordination approach.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.