Skip to content

fix: several fixes on risk#343

Merged
gusfcarvalho merged 7 commits intomainfrom
gc-fix-several-risk-fixes
Mar 10, 2026
Merged

fix: several fixes on risk#343
gusfcarvalho merged 7 commits intomainfrom
gc-fix-several-risk-fixes

Conversation

@gusfcarvalho
Copy link
Contributor

No description provided.

Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
Copilot AI review requested due to automatic review settings March 9, 2026 17:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  • ComponentDefinitionIdentity lookup is keyed only by (entity_type, identity_hash). Since identity_hash is built from identityPairs and does not incorporate pluginValue, two different plugins using the same identity label keys/values can collide and incorrectly reuse the other plugin’s DefinedComponentID. 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>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 DefinedComponentID as a non-pointer uuid.UUID primary key changes the schema of component_definition_labels (new NOT NULL PK column + different composite PK). Because MigrateUp uses 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>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@gusfcarvalho gusfcarvalho merged commit 0cfb541 into main Mar 10, 2026
8 checks passed
@gusfcarvalho gusfcarvalho deleted the gc-fix-several-risk-fixes branch March 10, 2026 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants