Reapply existing VEX rules after vulnerability reopening#2592
Reapply existing VEX rules after vulnerability reopening#2592nicksan222 wants to merge 6 commits into
Conversation
Signed-off-by: Nicholas Santi <nicksan222@gmail.com>
Signed-off-by: Nicholas Santi <nicksan222@gmail.com>
Signed-off-by: Nicholas Santi <nicksan222@gmail.com>
Signed-off-by: Nicholas Santi <nicksan222@gmail.com>
Signed-off-by: Nicholas Santi <nicksan222@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes the “Failed to create VEX rule” error when a user re-marks a dependency path as false-positive after reopening a vulnerability, by making manual VEX rule creation idempotent and reapplying existing rules to reopened vulnerabilities without overwriting stored rule metadata.
Changes:
- Add atomic “create-or-get” semantics for deterministic manual VEX rules (insert-or-load on conflict).
- Reapply existing rules to open vulnerabilities using a force path, while returning
201for new rules and200for reapplications. - Extend unit + integration coverage for reopening, reapplication, metadata preservation, and idempotent retries.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/scan_integration_test.go | Expands integration test to cover reopen + reapply behavior and idempotent retries. |
| shared/common_interfaces.go | Extends VEX rule repository/service interfaces with CreateOrGet. |
| services/vex_rule_service.go | Adds CreateOrGet with asset-version scope validation for deterministic-ID conflicts. |
| services/vex_rule_service_test.go | Adds unit tests for VEXRuleService.CreateOrGet behaviors. |
| database/repositories/vex_rule_repository.go | Implements atomic CreateOrGet via INSERT ... ON CONFLICT DO NOTHING + load. |
| controllers/vex_rule_controller.go | Uses CreateOrGet, forces reapply on existing rules, returns 200 vs 201, tightens reapply scope to asset version. |
| mocks/mock_VEXRuleService.go | Updates mock to support CreateOrGet. |
| mocks/mock_VEXRuleRepository.go | Updates mock to support CreateOrGet. |
Files not reviewed (2)
- mocks/mock_VEXRuleRepository.go: Generated file
- mocks/mock_VEXRuleService.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Nicholas Santi <nicksan222@gmail.com>
Why
|
| Field | Rule A | Rule B |
|---|---|---|
| Asset | backend-api |
backend-api |
| Asset version | main |
release |
| CVE | CVE-2026-1234 |
CVE-2026-1234 |
| Path | * → vulnerable-library |
* → vulnerable-library |
| Source | manual |
manual |
| ID | abc123… |
abc123… |
Why this matters for CreateOrGet
CreateOrGet performs an atomic insert:
INSERT ... ON CONFLICT (id) DO NOTHINGIf abc123… already belongs to main, a request under release conflicts and loads the persisted main row.
Without scope validation, the request could silently do this:
Create rule under release
↓
ID conflict loads rule from main
↓
Force-reapply persisted rule
↓
Vulnerabilities on main are modified
The service therefore enforces the full requested scope before returning the persisted rule:
if persistedRule.AssetID != rule.AssetID ||
persistedRule.AssetVersionName != rule.AssetVersionName {
return ErrVEXRuleAssetVersionConflict
}The outcomes are now explicit:
Same asset + same version → existing rule is reapplied
Same ID + different version → 409 Conflict; nothing is applied
The check lives in the service because the service owns the rule-identity invariant. The controller only maps the typed domain error to HTTP 409; it does not need to understand the hash composition.
The explicit /:ruleId/reapply endpoint separately verifies both route scopes because its rule ID is caller-controlled.
Why this PR does not change the ID formula
Adding AssetVersionName to the hash may ultimately be the correct model, but it requires a broader decision:
- Version-scoped rules: include the version in the ID, migrate every persisted rule ID, and make source synchronization consistently version-scoped.
- Asset-scoped rules: intentionally share rules across versions and reconsider the existing version-scoped routes, queries, exports, and application behavior.
This PR fixes #2115 without making that product-level decision or migrating persisted identities. It preserves the previous safe cross-version behavior while making the intended same-version duplicate flow idempotent:
Reopen a vulnerability, submit its existing path rule again, and reapply that persisted rule without overwriting its metadata.
Summary
201) from reapplication (200)Closes #2115
Design decisions
Insert or load atomically
Manual VEX rules use a deterministic primary key based on the asset, CVE, path pattern, and source. Repeating a dependency-graph selection therefore targets the same row.
The repository now uses
INSERT ... ON CONFLICT DO NOTHINGand inspectsRowsAffected. A successful insert returns the new rule; a conflict loads the persisted rule in the same transaction. This avoids a check-then-insert race while treating the expected duplicate as valid state rather than an internal error.Preserve the existing rule
The conflict path deliberately does not use the existing
UpdateAllupsert. Reapplying a rule should not replace its original justification, author, creation timestamp, enabled state, or other stored metadata with values from a repeated request.All subsequent application, counting, logging, and response generation therefore use the persisted rule returned by the repository.
Reapply only when the rule already exists
New rules continue through normal application and its duplicate-event protection. Existing rules use the force-reapply path so a historical VEX event does not suppress a new false-positive event after the vulnerability has explicitly been reopened.
The force path still considers only currently open vulnerabilities. Retrying the request after the vulnerability has already closed therefore creates neither another VEX rule nor another vulnerability event.
Enforce asset-version scope in the service
AssetVersionNameis not part of the historical deterministic rule ID. A conflict can consequently resolve to a rule stored for another version of the same asset.CreateOrGetvalidates the persisted rule against the full requested asset/version scope before returning it. This keeps hashing details out of the controller and preserves the previous safe behavior for cross-version collisions instead of applying a rule to the wrong version. The explicit reapply endpoint also validates both route scopes because its rule ID is caller-controlled.Changing the persisted ID format was intentionally kept out of scope: VEX rules began as asset-scoped records, and changing that identity would require a separate migration and a broader decision about cross-version rule semantics.
Keep creation and application transactional
Rule insertion/loading and vulnerability-event application remain in one transaction. Application failures now return immediately and rely on the existing deferred rollback instead of rolling back and then continuing to a commit attempt.
The endpoint returns:
201 Createdwhen a rule is inserted200 OKwhen the existing rule is reappliedThe web flow already treats both as success, refreshes the vulnerability and rule data, and displays the existing success feedback, so no client-side duplicate detection is needed.
Testing
go test ./...go test ./tests -run '^TestPathPatternVEXRules$' -count=1The integration test creates a path-based VEX rule, reopens the affected vulnerabilities, submits the same rule again with different request metadata, and verifies that: