[LFXV2-932] Add CommitteeMemberSensitive enricher object #34
Conversation
…for sensitive data handling Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-932 Assisted by [Claude Code](https://claude.ai/code) Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
…cher Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-932 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-932 Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new CommitteeMemberSensitiveEnricher type and registers it with the indexer service, introduces an object-type constant, adds exhaustive unit/integration tests for the enricher, and bumps the Helm chart version. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces support for sensitive committee member data by adding a new committee_member_sensitive object type with a dedicated enricher that enforces stricter access control and extracts only email information for indexing.
Key changes:
- Added
ObjectTypeCommitteeMemberSensitiveconstant to support the new object type - Implemented
CommitteeMemberSensitiveEnricherwith auditor-level access control (instead of viewer-level) and email-only extraction for names/aliases - Registered the new enricher in the indexer service alongside existing enrichers
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/constants/messaging.go |
Added ObjectTypeCommitteeMemberSensitive constant for the new object type |
internal/enrichers/committee_member_sensitive_enricher.go |
Implemented the new enricher with auditor/writer access control and email-only extraction logic |
internal/enrichers/committee_member_sensitive_enricher_test.go |
Added comprehensive test coverage for access control, name/alias extraction, and sort name logic |
internal/domain/services/indexer_service.go |
Registered the new enricher in the service's enricher registry |
charts/lfx-v2-indexer-service/Chart.yaml |
Bumped chart version from 0.4.12 to 0.4.13 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal/enrichers/committee_member_sensitive_enricher.go (1)
100-101: Consider compiling regex at package level for efficiency.The regex is compiled on every method call. For better performance, consider moving the regex compilation to package level or to the enricher struct initialization.
🔎 Proposed refactor
+// Package-level compiled regex for email field matching +var emailFieldRegex = regexp.MustCompile(`(?i)^email$`) + // setExtractNameAndAliases extracts the name and aliases from the committee member data // overrides the default name and aliases extraction logic func (e *CommitteeMemberSensitiveEnricher) setExtractNameAndAliases(data map[string]any) []string { var nameAndAliases []string seen := make(map[string]bool) // Deduplicate names - // Compile regex pattern for name-like fields - aliasRegex := regexp.MustCompile(`(?i)^(email)$`) for key, value := range data { - if aliasRegex.MatchString(key) { + if emailFieldRegex.MatchString(key) {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
charts/lfx-v2-indexer-service/Chart.yamlinternal/domain/services/indexer_service.gointernal/enrichers/committee_member_sensitive_enricher.gointernal/enrichers/committee_member_sensitive_enricher_test.gopkg/constants/messaging.go
🧰 Additional context used
📓 Path-based instructions (8)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Run golangci-lint before committing code
Run fmt, vet, lint, and test together before committing
Files:
pkg/constants/messaging.gointernal/enrichers/committee_member_sensitive_enricher_test.gointernal/domain/services/indexer_service.gointernal/enrichers/committee_member_sensitive_enricher.go
pkg/constants/messaging.go
📄 CodeRabbit inference engine (CLAUDE.md)
Add new object type constants to pkg/constants/messaging.go
Files:
pkg/constants/messaging.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Run all tests with race detection and coverage
Mock external dependencies using interfaces in internal/mocks/
Include race detection in all test runs
Files:
internal/enrichers/committee_member_sensitive_enricher_test.go
internal/enrichers/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
internal/enrichers/**/*.go: Create new enrichers in internal/enrichers/ implementing the Enricher interface
Enricher System: Extensible data processing based on object type with registry pattern
Files:
internal/enrichers/committee_member_sensitive_enricher_test.gointernal/enrichers/committee_member_sensitive_enricher.go
internal/enrichers/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Add tests for new enrichers
Files:
internal/enrichers/committee_member_sensitive_enricher_test.go
internal/domain/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
internal/domain/**/*.go: Domain layer cannot import from infrastructure or presentation layers - enforce Clean Architecture dependency rule
All external dependencies must use repository interfaces
Files:
internal/domain/services/indexer_service.go
internal/domain/services/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Business logic stays in domain services
Files:
internal/domain/services/indexer_service.go
internal/domain/services/indexer_service.go
📄 CodeRabbit inference engine (CLAUDE.md)
Register new enrichers in IndexerService.NewIndexerService()
Files:
internal/domain/services/indexer_service.go
🧬 Code graph analysis (3)
internal/enrichers/committee_member_sensitive_enricher_test.go (3)
internal/enrichers/committee_member_sensitive_enricher.go (2)
CommitteeMemberSensitiveEnricher(17-19)NewCommitteeMemberSensitiveEnricher(130-140)internal/domain/contracts/transaction.go (2)
TransactionBody(13-45)LFXTransaction(48-87)pkg/constants/messaging.go (1)
ObjectTypeCommitteeMemberSensitive(50-50)
internal/domain/services/indexer_service.go (1)
internal/enrichers/committee_member_sensitive_enricher.go (1)
NewCommitteeMemberSensitiveEnricher(130-140)
internal/enrichers/committee_member_sensitive_enricher.go (4)
internal/enrichers/registry.go (1)
Enricher(12-18)internal/domain/contracts/transaction.go (2)
TransactionBody(13-45)LFXTransaction(48-87)pkg/constants/messaging.go (2)
ObjectTypeCommittee(47-47)ObjectTypeCommitteeMemberSensitive(50-50)internal/enrichers/default_enricher.go (3)
WithAccessControl(33-37)WithNameAndAliases(54-58)WithSortName(61-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: MegaLinter
🔇 Additional comments (10)
charts/lfx-v2-indexer-service/Chart.yaml (1)
9-9: LGTM!The version bump to 0.4.13 appropriately reflects the addition of the new CommitteeMemberSensitiveEnricher feature.
pkg/constants/messaging.go (1)
45-64: LGTM!The new
ObjectTypeCommitteeMemberSensitiveconstant follows the established naming conventions and is appropriately grouped with other committee-related object types. As per coding guidelines, new object type constants are correctly added topkg/constants/messaging.go.internal/enrichers/committee_member_sensitive_enricher.go (2)
36-43: Verify behavior whencommittee_uidis an empty string.When
committee_uidexists but is an empty string, the function will produce"committee:"as the permission object. This may be intentional (the test case at line 58-72 of the test file confirms this behavior), but consider whether an emptycommittee_uidshould fall back to object-level permissions instead, similar to whencommittee_uidis a non-string type.
118-127: LGTM!The
extractSortNameimplementation correctly extracts the email field with proper type checking and whitespace trimming.internal/domain/services/indexer_service.go (1)
83-83: LGTM!The new
CommitteeMemberSensitiveEnricheris correctly registered inNewIndexerService()following the established pattern and placed logically after the regularCommitteeMemberEnricher. As per coding guidelines, new enrichers are properly registered here.internal/enrichers/committee_member_sensitive_enricher_test.go (5)
15-187: LGTM!Excellent test coverage for
setAccessControlwith comprehensive edge cases including:
- Committee UID presence/absence
- Empty and non-string values
- Explicit overrides and partial overrides
- Empty value preservation
The table-driven test pattern is well-structured and maintainable.
189-340: LGTM!Thorough test coverage for
setExtractNameAndAliasesincluding edge cases for whitespace handling, type coercion, deduplication, and case-insensitive field matching. The use ofElementsMatchcorrectly handles the non-deterministic order from map iteration.
342-492: LGTM!Comprehensive test coverage for
extractSortNamewith excellent edge case handling including unicode characters, special characters, various whitespace scenarios, and type validation.
507-567: LGTM with a note on test data.The integration tests properly verify the access control behavior with and without
committee_uid. Note that the test data uses"uid"field while the default enricher typically uses"id"for ObjectID extraction - this is acceptable here as the tests focus on access control logic rather than ObjectID assignment.
569-740: LGTM!The remaining integration tests provide solid coverage for:
- Name and aliases extraction (email only)
- Sort name extraction
- Empty/missing email handling
These tests effectively validate that the sensitive enricher correctly restricts extracted data to only the email field, aligning with the PR's objective of controlling attribute-level visibility.
…nstructor Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-932 Reviewed with [GitHub Copilot](https://github.com/features/copilot) Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
internal/enrichers/committee_member_sensitive_enricher.go (1)
129-140: Remove extra blank line before closing brace.Line 139 contains an unnecessary blank line before the closing brace at line 140. This formatting issue was flagged in previous reviews.
🔎 Proposed fix
func NewCommitteeMemberSensitiveEnricher() Enricher { cme := &CommitteeMemberSensitiveEnricher{} cme.defaultEnricher = newDefaultEnricher( constants.ObjectTypeCommitteeMemberSensitive, WithAccessControl(cme.setAccessControl), WithNameAndAliases(cme.setExtractNameAndAliases), WithSortName(cme.extractSortName), ) return cme - }
🧹 Nitpick comments (1)
internal/enrichers/committee_member_sensitive_enricher.go (1)
101-101: Consider simplifying the regex pattern.The outer parentheses in
(?i)^(email)$create an unnecessary capture group. Since the captured value isn't used, simplify to(?i)^email$for clarity.🔎 Proposed simplification
- aliasRegex := regexp.MustCompile(`(?i)^(email)$`) + aliasRegex := regexp.MustCompile(`(?i)^email$`)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
internal/enrichers/committee_member_enricher.gointernal/enrichers/committee_member_sensitive_enricher.go
✅ Files skipped from review due to trivial changes (1)
- internal/enrichers/committee_member_enricher.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Run golangci-lint before committing code
Run fmt, vet, lint, and test together before committing
Files:
internal/enrichers/committee_member_sensitive_enricher.go
internal/enrichers/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
internal/enrichers/**/*.go: Create new enrichers in internal/enrichers/ implementing the Enricher interface
Enricher System: Extensible data processing based on object type with registry pattern
Files:
internal/enrichers/committee_member_sensitive_enricher.go
🧬 Code graph analysis (1)
internal/enrichers/committee_member_sensitive_enricher.go (4)
internal/enrichers/registry.go (1)
Enricher(12-18)internal/domain/contracts/transaction.go (2)
TransactionBody(13-45)LFXTransaction(48-87)pkg/constants/messaging.go (2)
ObjectTypeCommittee(47-47)ObjectTypeCommitteeMemberSensitive(50-50)internal/enrichers/default_enricher.go (3)
WithAccessControl(33-37)WithNameAndAliases(54-58)WithSortName(61-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: MegaLinter
🔇 Additional comments (5)
internal/enrichers/committee_member_sensitive_enricher.go (5)
1-14: LGTM: Clean imports and package structure.The package declaration and imports are well-organized and appropriate for the enricher's functionality.
16-29: LGTM: Clean delegation pattern.The struct design and delegation to the default enricher follow established patterns in the codebase. The comments accurately describe the functionality.
118-127: LGTM: Sort name extraction is clear and correct.The sort name extraction correctly retrieves and trims the "email" field, with appropriate handling of missing or non-string values.
95-116: Email field pattern is intentional and correct—no changes needed.The regex pattern
(?i)^(email)$correctly matches only the "email" field for committee member sensitive data. This design is confirmed by test cases that explicitly validate "only email extracted" behavior (seeTestCommitteeMemberSensitiveEnricher_Integration_OnlyEmailExtractedand test comment "should be ignored - only email is extracted"). The data model for committee members uses a single "email" field, not variations.
31-93: Verify the hardcoded relation names "auditor" and "writer" against your OpenFGA schema definition.The relations are consistently applied across the enricher system and thoroughly tested, but since the OpenFGA schema is not in the repository, confirm these relations exist and correctly represent the intended permissions at the committee level for sensitive member access control.
1 participant
Overview
Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-932
This pull request introduces a new object type,
committee_member_sensitive, and adds a corresponding enricher to handle its specific data enrichment and access control logic. The main changes are focused on supporting sensitive committee member data with customized access control and name extraction rules.Support for Sensitive Committee Member Data
ObjectTypeCommitteeMemberSensitivetopkg/constants/messaging.gofor handling sensitive committee member data.CommitteeMemberSensitiveEnricherininternal/enrichers/committee_member_sensitive_enricher.gowith custom logic for access control, name/aliases extraction, and sort name extraction tailored for sensitive committee member objects.internal/domain/services/indexer_service.go.Relates to linuxfoundation/lfx-v2-committee-service#53