feat: Implement Custom Sensitivity Rules#52
Conversation
Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/0ae7fbcf-757d-49e5-a44f-db80e367ae06 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
…e readability Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/0ae7fbcf-757d-49e5-a44f-db80e367ae06 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
MaximumTrainer
requested review from
Copilot
and removed request for
MaximumTrainer
There was a problem hiding this comment.
Pull request overview
Implements a full-stack “Custom Sensitivity Rules” feature so users can define column-name + generic-type matching rules, preview matches against stored schema snapshots, and (optionally) link matches to generator presets for bulk recommendation application in the Privacy Hub.
Changes:
- Added backend entity/ports/service/controller for CRUD + preview of custom sensitivity rules, and integrated custom rule matching into sensitivity scans.
- Extended Privacy Hub recommendations to carry an optional
recommendedPresetIdand apply linked presets when present. - Added a new frontend Settings view (+ router/nav + API/types) to manage rules and preview matches.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/views/SensitivityRulesView.vue | New management UI with side-drawer CRUD and preview panel. |
| frontend/src/types/index.ts | Adds frontend enums/types for rules, matchers, and preview payloads/results. |
| frontend/src/router/index.ts | Registers /settings/sensitivity-rules route. |
| frontend/src/components/NavBar.vue | Adds “Sensitivity Rules” navigation link. |
| frontend/src/api/sensitivityRules.ts | Adds typed API client for rules CRUD + preview. |
| backend/src/main/kotlin/com/opendatamask/domain/port/output/CustomSensitivityRulePort.kt | New output port for rule persistence. |
| backend/src/main/kotlin/com/opendatamask/domain/port/input/dto/CustomSensitivityRuleDto.kt | New request/response/preview DTOs. |
| backend/src/main/kotlin/com/opendatamask/domain/port/input/CustomSensitivityRuleUseCase.kt | New use-case interface for rules. |
| backend/src/main/kotlin/com/opendatamask/domain/model/GenericDataType.kt | Adds generic data type enum for rule filtering. |
| backend/src/main/kotlin/com/opendatamask/domain/model/CustomSensitivityRule.kt | New JPA entity + matcher model/enums. |
| backend/src/main/kotlin/com/opendatamask/adapter/output/persistence/CustomSensitivityRuleRepository.kt | Spring Data repository implementing the new port. |
| backend/src/main/kotlin/com/opendatamask/application/service/CustomSensitivityRuleService.kt | Implements CRUD, preview, and matching helpers. |
| backend/src/main/kotlin/com/opendatamask/adapter/input/rest/CustomSensitivityRuleController.kt | REST endpoints for CRUD + preview. |
| backend/src/main/kotlin/com/opendatamask/application/service/SensitivityScanService.kt | Integrates custom rules into scanning and persists custom label/preset ID. |
| backend/src/main/kotlin/com/opendatamask/domain/model/ColumnSensitivity.kt | Adds customSensitivityLabel and recommendedPresetId fields. |
| backend/src/main/kotlin/com/opendatamask/domain/port/input/dto/PrivacyHubDto.kt | Extends PrivacyRecommendation with recommendedPresetId. |
| backend/src/main/kotlin/com/opendatamask/application/service/PrivacyHubService.kt | Applies preset-based recommendations and surfaces custom labels. |
| backend/src/test/kotlin/com/opendatamask/application/service/CustomSensitivityRuleServiceTest.kt | Adds unit coverage for matching + preview + error paths. |
| backend/src/test/kotlin/com/opendatamask/adapter/input/rest/CustomSensitivityRuleControllerTest.kt | Adds MVC test coverage for CRUD + preview endpoint. |
| backend/src/test/kotlin/com/opendatamask/application/service/SensitivityScanServiceTest.kt | Updates scan tests for new dependencies. |
| backend/src/test/kotlin/com/opendatamask/application/service/SensitivityScanLogServiceTest.kt | Updates scan/log tests for new dependencies. |
| backend/src/test/kotlin/com/opendatamask/application/service/PrivacyHubServiceTest.kt | Updates Privacy Hub tests for new preset dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { listSystemPresets, listWorkspacePresets } from '@/api/presets' | ||
| import type { GeneratorPresetResponse } from '@/api/presets' | ||
| import type { | ||
| CustomSensitivityRule, | ||
| CustomSensitivityRuleRequest, | ||
| CustomRuleMatcher, | ||
| CustomRulePreviewResult | ||
| } from '@/types' | ||
| import { GenericDataType, MatcherType } from '@/types' | ||
|
|
||
| const workspaceStore = useWorkspaceStore() | ||
| const workspaces = ref(workspaceStore.workspaces) | ||
|
|
There was a problem hiding this comment.
listWorkspacePresets, CustomRuleMatcher, and the local workspaces ref are imported/declared but never used. This will typically fail typecheck/lint in the frontend; either remove them or wire listWorkspacePresets/workspaces into the preset picker / workspace dropdown logic.
| async function fetchPresets() { | ||
| try { | ||
| systemPresets.value = await listSystemPresets() | ||
| } catch { | ||
| // non-critical | ||
| } | ||
| } |
There was a problem hiding this comment.
The linked preset picker only loads listSystemPresets(); workspace presets are never loaded, and presetName() only resolves against systemPresets. This prevents linking to (or even correctly displaying) workspace presets even though listWorkspacePresets(workspaceId) exists. Consider loading both system + workspace presets (scoped by selected workspace if needed) and resolving names from the combined list.
| function openCreateDrawer() { | ||
| editingRule.value = null | ||
| form.value = { | ||
| name: '', | ||
| description: null, | ||
| dataTypeFilter: GenericDataType.ANY, | ||
| matchers: [], | ||
| linkedPresetId: null, | ||
| isActive: true | ||
| } | ||
| previewResults.value = [] | ||
| previewError.value = null | ||
| saveError.value = null | ||
| drawerOpen.value = true | ||
| } | ||
|
|
||
| function openEditDrawer(rule: CustomSensitivityRule) { | ||
| editingRule.value = rule | ||
| form.value = { | ||
| name: rule.name, | ||
| description: rule.description, | ||
| dataTypeFilter: rule.dataTypeFilter, | ||
| matchers: rule.matchers.map((m) => ({ ...m })), | ||
| linkedPresetId: rule.linkedPresetId, | ||
| isActive: rule.isActive | ||
| } | ||
| previewResults.value = [] | ||
| previewError.value = null | ||
| saveError.value = null | ||
| drawerOpen.value = true | ||
| } |
There was a problem hiding this comment.
openCreateDrawer()/openEditDrawer() reset preview results/errors but do not reset previewWorkspaceId (or previewing). This can cause the drawer to reopen with a stale workspace selection and immediately show "No columns matched" for the previous selection. Consider resetting preview-related state (including previewWorkspaceId) when opening/closing the drawer.
| @@ -74,22 +93,67 @@ class SensitivityScanService( | |||
| columnName = column | |||
| ) | |||
| entity.isSensitive = true | |||
| entity.sensitivityType = result.sensitivityType | |||
| entity.confidenceLevel = result.confidence | |||
| entity.recommendedGeneratorType = result.recommendedGenerator | |||
| entity.sensitivityType = builtInResult.sensitivityType | |||
| entity.confidenceLevel = builtInResult.confidence | |||
| entity.recommendedGeneratorType = builtInResult.recommendedGenerator | |||
| entity.customSensitivityLabel = null | |||
| entity.recommendedPresetId = null | |||
| columnSensitivityRepository.save(entity) | |||
| } | |||
| sensitivityScanLogEntryRepository.save( | |||
| SensitivityScanLogEntry( | |||
| scanLogId = log.id!!, | |||
| tableName = table, | |||
| columnName = column, | |||
| detectedType = result?.sensitivityType?.name, | |||
| confidenceLevel = result?.confidence?.name, | |||
| recommendedGenerator = result?.recommendedGenerator?.name, | |||
| scannedAt = LocalDateTime.now() | |||
| sensitivityScanLogEntryRepository.save( | |||
| SensitivityScanLogEntry( | |||
| scanLogId = log.id!!, | |||
| tableName = table, | |||
| columnName = column, | |||
| detectedType = builtInResult.sensitivityType.name, | |||
| confidenceLevel = builtInResult.confidence.name, | |||
| recommendedGenerator = builtInResult.recommendedGenerator.name, | |||
| scannedAt = LocalDateTime.now() | |||
| ) | |||
| ) | |||
| ) | |||
| } else { | |||
| val customMatch = detectCustomRuleSensitivity(column, columnInfo.type, activeCustomRules) | |||
| if (customMatch != null) { | |||
| val (matchedRule, _) = customMatch | |||
There was a problem hiding this comment.
Custom rules are only evaluated in the else branch when builtInResult == null. This means a user cannot attach a linked preset via a custom rule if a built-in rule already detects the column (e.g., EMAIL), which appears to conflict with the stated behavior of applying custom rule matches after built-ins. Consider running custom rule evaluation after built-in detection as well, and define precedence (e.g., custom preset overrides built-in recommendedGeneratorType, while keeping the built-in sensitivityType).
| MatcherType.REGEX -> Regex( | ||
| matcher.value, | ||
| if (matcher.caseSensitive) emptySet() else setOf(RegexOption.IGNORE_CASE) | ||
| ).containsMatchIn(columnName) |
There was a problem hiding this comment.
matchesColumnName() constructs a Regex(...) directly from user-provided input. If the regex is invalid, this will throw and can fail preview requests and even mark an entire workspace scan as FAILED. Catch regex compilation/match exceptions per matcher (and/or validate regexes during create/update) so one bad rule doesn't break scans.
| MatcherType.REGEX -> Regex( | |
| matcher.value, | |
| if (matcher.caseSensitive) emptySet() else setOf(RegexOption.IGNORE_CASE) | |
| ).containsMatchIn(columnName) | |
| MatcherType.REGEX -> { | |
| try { | |
| Regex( | |
| matcher.value, | |
| if (matcher.caseSensitive) emptySet() else setOf(RegexOption.IGNORE_CASE) | |
| ).containsMatchIn(columnName) | |
| } catch (e: IllegalArgumentException) { | |
| false | |
| } | |
| } |
| data class CustomSensitivityRuleRequest( | ||
| val name: String, | ||
| val description: String? = null, | ||
| val dataTypeFilter: GenericDataType = GenericDataType.ANY, | ||
| val matchers: List<CustomRuleMatcherDto> = emptyList(), | ||
| val linkedPresetId: Long? = null, | ||
| val isActive: Boolean = true | ||
| ) |
There was a problem hiding this comment.
This request DTO lacks the bean-validation annotations used elsewhere (e.g., @field:NotBlank) and the controller doesn't use @Valid. As a result, invalid payloads (blank names, empty matcher values, etc.) can reach persistence/matching logic and cause runtime issues. Add validation constraints here and annotate controller @RequestBody parameters with @Valid for create/update/preview as appropriate.
| if (rec.recommendedPresetId != null) { | ||
| // Apply linked preset from a custom sensitivity rule | ||
| val preset = generatorPresetRepository.findById(rec.recommendedPresetId).orElse(null) | ||
| if (preset == null) { | ||
| logger.warn( | ||
| "Cannot apply recommendation for ${rec.tableName}.${rec.columnName}: " + | ||
| "linked preset id=${rec.recommendedPresetId} not found" | ||
| ) | ||
| continue | ||
| } | ||
| val existingGenerator = columnGeneratorRepository.findByTableConfigurationId(tableConfig.id) | ||
| .find { it.columnName == rec.columnName } | ||
| if (existingGenerator != null) { | ||
| existingGenerator.presetId = preset.id | ||
| existingGenerator.generatorType = preset.generatorType | ||
| existingGenerator.generatorParams = preset.generatorParams | ||
| columnGeneratorRepository.save(existingGenerator) |
There was a problem hiding this comment.
applyRecommendations() performs per-recommendation lookups (generatorPresetRepository.findById(...)) and per-recommendation generator scans (findByTableConfigurationId(...).find { ... }). For many recommendations this becomes an N+1 pattern. Consider preloading presets into a map and preloading existing generators per tableConfig (or extending the port with a targeted lookup) to reduce DB round-trips.
| @Mock private lateinit var EncryptionPort: EncryptionPort | ||
| @Mock private lateinit var customSensitivityRuleRepository: CustomSensitivityRulePort | ||
| @Mock private lateinit var customSensitivityRuleService: CustomSensitivityRuleService | ||
|
|
There was a problem hiding this comment.
SensitivityScanService.scanWorkspace() now calls customSensitivityRuleRepository.findByIsActiveTrue() unconditionally. In these tests, the new mock isn't stubbed, and Mockito defaults can return null for non-null Kotlin return types, causing an NPE when .map { ... } is invoked. Stub findByIsActiveTrue() to return emptyList() (or a specific list) in the test setup.
| @Mock private lateinit var connectorFactory: ConnectorFactory | ||
| @Mock private lateinit var EncryptionPort: EncryptionPort | ||
| @Mock private lateinit var customSensitivityRuleRepository: CustomSensitivityRuleRepository | ||
| @Mock private lateinit var customSensitivityRuleService: CustomSensitivityRuleService | ||
|
|
There was a problem hiding this comment.
Same as above: customSensitivityRuleRepository.findByIsActiveTrue() is invoked during scans. This test adds the new mock but never stubs it, which can lead to an NPE at runtime when the service does .map { ... } on the returned list. Stub it to emptyList() (or configure the desired custom-rule behavior).
| @PostMapping | ||
| fun createRule( | ||
| @RequestBody request: CustomSensitivityRuleRequest | ||
| ): ResponseEntity<CustomSensitivityRuleResponse> = | ||
| ResponseEntity.status(HttpStatus.CREATED) | ||
| .body(customSensitivityRuleService.createRule(request)) | ||
|
|
||
| @PutMapping("/{id}") | ||
| fun updateRule( | ||
| @PathVariable id: Long, | ||
| @RequestBody request: CustomSensitivityRuleRequest | ||
| ): ResponseEntity<CustomSensitivityRuleResponse> = | ||
| ResponseEntity.ok(customSensitivityRuleService.updateRule(id, request)) |
There was a problem hiding this comment.
Other controllers (e.g., GeneratorPresetController) use @Valid on request bodies to enforce DTO constraints. Here, create/update/preview accept request bodies without @Valid, so any added bean validation on the DTOs will not run. Consider adding @Valid to the @RequestBody parameters.
|
@copilot apply changes based on the comments in this thread |
… scan ordering, preview reset Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/5a92e1d9-53ef-44f8-b2b9-31addf85947e Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
All review feedback has been addressed in commit Backend:
Frontend:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
GenericDataTypeenumCustomSensitivityRuleJPA entityCustomSensitivityRulePort,CustomSensitivityRuleRepository@field:NotBlank/@field:NotNullbean validation constraintsCustomSensitivityRuleUseCase,CustomSensitivityRuleServiceCustomSensitivityRuleControllerwith@Validon all@RequestBodyparametersmatchesColumnNamecatchesIllegalArgumentExceptionfrom invalid regex patterns (returnsfalse, logs warning)previewRulelogs warning with workspaceId on schema JSON parse failuretoResponseusesrequireNotNull(id)instead ofid ?: 0LColumnSensitivitywithcustomSensitivityLabelandrecommendedPresetIdSensitivityScanServiceevaluates custom rules for ALL columns (including built-in matches) so a linked preset can be attachedPrivacyHubService.applyRecommendationspreloads presets and generators into maps (N+1 elimination)PrivacyRecommendationDTO withrecommendedPresetIdfindByIsActiveTrue()→emptyList()inSensitivityScanServiceTestandSensitivityScanLogServiceTestCustomSensitivityRuleControllerTest,CustomSensitivityRuleServiceTestlistWorkspacePresetsimport,CustomRuleMatchertype import, andworkspacesrefopenCreateDrawer/openEditDrawernow resets all preview state includingpreviewWorkspaceIdandpreviewingonPreviewWorkspaceChangeto reload workspace presets and clear stale results