Feat/mobilization—closed loop finding lifecycle#28
Merged
Hidden character warning
The head ref may contain hidden characters: "feat/Mobilization\u2014Closed-LoopFindingLifecycle"
Conversation
- Add fix_applied status (dev marks fixed, scanner/security verifies) - Add resolution_method tracking (scan_verified, security_reviewed, admin_direct) - Add multi-dimension group view (7 dimensions: CVE, asset, owner, component, severity, source, type) - Add bulk fix-applied with related CVEs suggestion - Add bulk verify/reject by IDs or filter - Add auto-assign to asset owners - Add auto-verify in scanner ingest (fix_applied + vuln gone = resolved) - Add confirmed->resolved guard (requires findings:verify permission) - Add batch group assignment loading (avoid N+1) - Add data scope enforcement for verify/reject by filter - Add 3 indexes (cve_id, owner_id, fix_applied) for GROUP BY performance - Add 38 unit tests (status machine, transitions, lifecycle, backward compat) - Add 2 permissions: findings:fix_apply, findings:verify - Migration 000096: resolution_method column + indexes + permissions
CI/CD: - Re-enable lint job (go vet + staticcheck, replacing disabled golangci-lint) - Restore lint as build dependency Tests (10 new): - TestLifecycle_FixApplied_RequiresNote - TestLifecycle_FixApplied_RecordsActor - TestLifecycle_Verify_SetsResolutionMethod - TestLifecycle_Reject_ClearsResolution - TestLifecycle_DirectResolve_BlockedForNonAdmin - TestLifecycle_AdminEscapeHatch - TestLifecycle_Regression_ReopensResolved - TestLifecycle_MultiCycle_FixRejectFixVerify - TestLifecycle_PentestFindingsUnaffected - TestResolutionMethod_RejectsInvalid Total: 48 domain tests (38 existing + 10 new), all passing
…ymlink - Add test_e2e_fix_lifecycle.sh covering: - Status transitions (new → confirmed → in_progress) - in_progress → resolved BLOCKED (dev cannot self-close) - Groups view (7 dimensions) - Bulk fix applied (with/without note validation) - Verify by filter (security approve) - Related CVEs endpoint - Auto-assign to owners - Fix sdk-go symlink (was empty dir, now points to ../sdk-go)
Server wiring: - Add FindingLifecycle service to Services struct - Initialize in NewServices() with all dependencies - Add FindingLifecycle handler to Handlers struct - Wire in handlers.go Route fix: - Use /api/v1/finding-groups and /api/v1/finding-actions (avoids Chi mount conflict with existing /api/v1/findings group) SQL fixes: - Fix 'u.display_name' → 'u.name' (users table column) - Fix GROUP BY missing f.severity in CVE query - Fix UPDATE alias 'f' in BulkUpdateStatusByFilter E2E results: 16/18 pass (2 expected: bulk ops return 0 when finding not assigned to user — correct auth behavior)
…ilityRoutes - Rename FindingLifecycleService → FindingActionsService - Rename FindingLifecycleHandler → FindingActionsHandler - Rename finding_lifecycle_*.go → finding_actions_*.go - Delete separate route file, add routes inside registerVulnerabilityRoutes() - Fix SQL: u.display_name → u.name, add f.severity to GROUP BY, add alias to UPDATE
… paths)
All lifecycle routes now inside existing /api/v1/findings group:
GET /findings/groups
GET /findings/related-cves/{cveId}
POST /findings/actions/fix-applied
POST /findings/actions/verify
POST /findings/actions/reject-fix
POST /findings/actions/assign-to-owners
Removed separate /finding-groups and /finding-actions paths.
E2E: 16/18 pass (2 expected: auth behavior for unassigned findings).
7 test files, 8 mock structs updated to implement new accesscontrol.Repository interface method. All tests pass.
- Add BatchListFindingGroupIDs to 8 AC mock structs (7 files) - Add ListFindingGroups, BulkUpdateStatusByFilter, FindRelatedCVEs, ListByStatusAndAssets to 11 finding repo mock structs (9 files) - Fix HasVerifyPermission in 3 existing resolve tests - Add pagination import to 2 integration test files - All tests pass: pkg/* + tests/unit/*
sdk-go v0.2.0 published on GitHub doesn't contain pkg/adapters (added after v0.2.0 release). CI needs local checkout + replace directive. All 3 jobs (lint, test, build) now: 1. Checkout openctemio/sdk-go into ./sdk-go 2. Run go mod edit -replace + go mod tidy 3. Then execute their step Tested locally: go vet ✅, tests ✅, build ✅ with GOWORK=off + replace
Remove checkout sdk-go + replace directive hack. sdk-go v0.3.0 now published with pkg/adapters — go mod download works.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| clauses = append(clauses, fmt.Sprintf("f.finding_type = ANY($%d)", argOffset)) | ||
| args = append(args, pq.Array(types)) | ||
| argOffset++ |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
In general, a “useless assignment” should be removed when a variable is assigned a value that is never read. Here, argOffset++ at the end of the FindingTypes block modifies argOffset but its updated value is never used before the function returns, so it can be removed.
The best fix that does not change existing functionality is to delete the argOffset++ line in the if len(filter.FindingTypes) > 0 block, and leave the rest of the function untouched. All placeholders ($%d) already refer to the current argOffset before increment; removing the final increment has no effect on the generated SQL or arguments because nothing else relies on argOffset after that block.
Concretely, in internal/infra/postgres/finding_group_repository.go, within buildFilterWhere, remove line 112 (argOffset++) and keep lines 105–111 and 113–115 as they are. No new imports, methods, or definitions are required.
Suggested changeset
1
internal/infra/postgres/finding_group_repository.go
| @@ -109,7 +109,6 @@ | ||
| } | ||
| clauses = append(clauses, fmt.Sprintf("f.finding_type = ANY($%d)", argOffset)) | ||
| args = append(args, pq.Array(types)) | ||
| argOffset++ | ||
| } | ||
|
|
||
| return strings.Join(clauses, " AND "), args |
Copilot is powered by AI and may make mistakes. Always verify output.
- Replace deprecated Redis SetNX/ZRangeByScore with SetArgs/ZRangeArgs - Add SetNX method to internal redis.Client wrapper - Use struct type conversion instead of field-by-field copy (S1016) - Remove redundant nil check before len() (S1009) - Fix error return position in keycloak validator (ST1008) - Delete unused scanNotification, toPentestFindingResponse, unmarshalJSONBMap - Remove unused test mocks: mockAITriageFindingRepo, mockJobEnqueuer, mockTargetMappingRepo, wfExecMockNotificationHandler, wfExecMockConditionEvaluator, permCacheStore interface - Fix ssoMockUserRepo.Delete to use deleteErr field
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.