Conversation
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
There was a problem hiding this comment.
Pull request overview
This PR applies several risk-related fixes and feature additions across the worker, relational services, and API handlers, mainly standardizing risk↔evidence linking around evidence “stream UUIDs” and adding new scoped endpoints and suggestion flows.
Changes:
- Switch risk↔evidence links to store/use evidence stream UUIDs (and update related worker/service logic, tests, and docs).
- Update risk evidence worker violation filtering to use evidence labels (including legacy label name support) and adjust tests accordingly.
- Add new APIs and service support for SSP-scoped risk CRUD and statement-scoped component suggestions, with unit/integration test coverage.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/service/worker/risk_evidence_worker.go | Filters templates by violation IDs from evidence labels; logs/links use evidence stream UUID. |
| internal/service/worker/risk_evidence_worker_test.go | Updates fixtures/assertions to use labels for violations and evidence stream UUID for links. |
| internal/service/relational/risks/service.go | Resolves evidence references to stream UUID when adding/removing evidence links. |
| internal/service/relational/risks/service_test.go | Updates evidence/link expectations to reflect stream UUID semantics. |
| internal/service/relational/risks/queries_test.go | Adjusts test DB migration setup to include evidences table shape used by stream semantics. |
| internal/service/relational/risks/links.go | Documents that RiskEvidenceLink.EvidenceID is the evidence stream UUID. |
| internal/api/handler/risks.go | Adds SSP-scoped risk CRUD endpoints and shared create helper. |
| internal/api/handler/api.go | Registers /ssp/:sspId/risks routes. |
| internal/api/handler/risks_integration_test.go | Adds SSP-scoped CRUD integration test; updates evidence filter to use stream UUID. |
| internal/service/relational/templates/subject_template_service.go | Groups ComponentDefinitions by plugin; adds OSCAL metadata on auto-created component definitions. |
| internal/service/relational/templates/subject_template_service_test.go | Extends tests to verify component definition metadata and plugin grouping behavior. |
| internal/service/relational/system_component_suggestions.go | Adds statement-scoped suggestion/apply flows and shared apply helper. |
| internal/service/relational/system_component_suggestions_test.go | Adds unit tests for statement-scoped suggestions and apply behavior. |
| internal/api/handler/oscal/system_security_plans.go | Adds statement-scoped suggest/apply endpoints for component suggestions. |
| internal/api/handler/oscal/system_security_plan_suggestions_integration_test.go | Adds integration tests for statement-scoped suggest/apply endpoints. |
| docs/swagger.yaml / docs/swagger.json / docs/docs.go | Updates API schema docs for evidenceId semantics on risk-evidence links. |
Comments suppressed due to low confidence (1)
internal/service/relational/templates/subject_template_service.go:823
ComponentDefinitionIdentitylookup is keyed only by (entity_type, identity_hash). Sinceidentity_hashis built fromidentityPairsand does not incorporatepluginValue, two different plugins using the same identity label keys/values can collide and incorrectly reuse the other plugin’sDefinedComponentID. Consider including the normalized plugin value in the identity hash (or extending the identity table key to include plugin) so identities are plugin-scoped.
func (s *SubjectTemplateService) resolveOrCreateComponentDefinition(template SubjectTemplate, pluginValue string, identityPairs []identityLabelPair, schemaLabels []identityLabelPair, identityHash string) (*uuid.UUID, error) {
// Check if identity already exists.
var existingIdentity ComponentDefinitionIdentity
if err := s.db.Where("entity_type = ? AND identity_hash = ?", subjectTemplateTypeComponent, identityHash).First(&existingIdentity).Error; err == nil {
return &existingIdentity.DefinedComponentID, nil
} else if !errors.Is(err, gorm.ErrRecordNotFound) {
return nil, err
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
internal/service/relational/templates/subject_template_service.go:823
- ComponentDefinitionIdentity is looked up solely by (entity_type, identity_hash), but this PR also changes ComponentDefinition IDs to be grouped by plugin. If two different plugins produce the same identity label set (e.g., same asset_id/cluster), they will share the same identity_hash and the second plugin will incorrectly reuse the first plugin's DefinedComponentID/ComponentDefinitionID, defeating plugin scoping and potentially mixing data sources. To keep grouping-by-plugin correct, the identity key needs to be plugin-scoped as well (e.g., include normalized plugin in the identity hash input, or add a plugin/component_definition dimension to the identity table key).
func (s *SubjectTemplateService) resolveOrCreateComponentDefinition(template SubjectTemplate, pluginValue string, identityPairs []identityLabelPair, schemaLabels []identityLabelPair, identityHash string) (*uuid.UUID, error) {
// Check if identity already exists.
var existingIdentity ComponentDefinitionIdentity
if err := s.db.Where("entity_type = ? AND identity_hash = ?", subjectTemplateTypeComponent, identityHash).First(&existingIdentity).Error; err == nil {
return &existingIdentity.DefinedComponentID, nil
} else if !errors.Is(err, gorm.ErrRecordNotFound) {
return nil, err
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
internal/service/relational/risks/labels.go:41
- Adding
DefinedComponentIDas a non-pointeruuid.UUIDprimary key changes the schema ofcomponent_definition_labels(new NOT NULL PK column + different composite PK). BecauseMigrateUpuses GORM AutoMigrate for this model, this is likely to fail on existing Postgres databases (cannot add a NOT NULL column / alter PK automatically) and/or leave legacy rows unreadable. Consider shipping an explicit migration/backfill (and potentially keeping this field nullable during transition) rather than relying on AutoMigrate to reshape the primary key.
type ComponentDefinitionLabel struct {
DefinedComponentID uuid.UUID `json:"definedComponentId" gorm:"type:uuid;primaryKey;index"`
ComponentDefinitionID uuid.UUID `json:"componentDefinitionId" gorm:"type:uuid;primaryKey"`
Key string `json:"key" gorm:"type:text;primaryKey;index:idx_component_definition_label_key_value,priority:1"`
Value string `json:"value" gorm:"type:text;primaryKey;index:idx_component_definition_label_key_value,priority:2"`
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
internal/service/relational/templates/subject_template_service.go:822
- ComponentDefinitionIdentity lookup is keyed only by (entity_type, identity_hash). Since identity_hash does not include the plugin value, the first resolved component for a given identity could be incorrectly reused across different plugins (cross-plugin collision), preventing creation of the correct plugin-scoped ComponentDefinition/DefinedComponent. Consider scoping the lookup by plugin (e.g., also filter by component_definition_id derived from plugin) or incorporating normalized plugin into the identity hash for component-definition identities.
var existingIdentity ComponentDefinitionIdentity
if err := s.db.Where("entity_type = ? AND identity_hash = ?", subjectTemplateTypeComponent, identityHash).First(&existingIdentity).Error; err == nil {
return &existingIdentity.DefinedComponentID, nil
} else if !errors.Is(err, gorm.ErrRecordNotFound) {
return nil, err
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.