feat(extension): configure PII entity types and custom regex from options page - #540
Open
yjouini wants to merge 5 commits into
Open
feat(extension): configure PII entity types and custom regex from options page#540yjouini wants to merge 5 commits into
yjouini wants to merge 5 commits into
Conversation
…ions page Wire the Chrome extension options page to the new server-side PII config endpoints introduced in dataiku#509/dataiku#513: - Entity types: GET/POST /api/pii/entities ({available, disabled}), unchecking a type adds it to the disabled (passthrough) set. - Custom patterns: GET/POST /api/pii/regexes, whole-set replacement on each add/remove; custom names appear as selectable entity types. Config is stored server-side, so /api/pii/check needs no per-request label list and background.js is unchanged. Closes dataiku#458, Closes dataiku#459 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 18, 2026
Davidnet
reviewed
Jun 30, 2026
Davidnet
left a comment
Davidnet
left a comment
Member
There was a problem hiding this comment.
Code review of the PII entity-type / custom-regex config wiring. Inline comments below cover the correctness issues found, ranked roughly most-severe first. Two of these (the empty-grid Save and the stale delete index) can silently lose user data.
Member
|
Hi @yjouini I have some proposal that cover a few of my comments, let me know if they make sense, and if you can bring to your PR.
Let me know if you have comments or questions |
Davidnet
requested changes
Jun 30, 2026
Collaborator
|
Hi @yjouini - just checking in if you had a chance to review David's comments. Thank you for your reply. |
Contributor
Author
|
Thanks for the ping @hanneshapke . I’m reviewing David’s comments and working through the proposed fixes now. I’ll push an update. |
- Guard the entity-types Save against an empty grid: when no checkboxes are loaded (still loading or backend unreachable), saving would POST an empty disabled set and silently re-enable every type the user had turned off. Now it surfaces an error and aborts. - Delete custom patterns by object identity instead of a render-time index. A concurrent delete could re-render and shift indices, causing the wrong pattern (or none) to be removed.
- Reject empty/whitespace-only names: the HTML `required` attribute only blocks an empty field, so a spaces-only value passed validation and then trimmed to an empty entity-type name. - Reject duplicate names: adding two patterns with the same name produced duplicate entity-type entries in the label grid with ambiguous toggling.
The live preview validates with the JS RegExp engine, but the backend compiles patterns with Go's regexp (RE2). Patterns using lookaround or backreferences passed the UI yet failed (or silently never matched) once saved. validateRegex now also flags these unsupported constructs, so both the preview and the add path reject them up front.
- Preserve unsaved entity-type toggles across the post-save grid refresh. savePatterns() reloads the label grid so new custom names appear, but the rebuild re-read the backend's saved state and discarded any check/uncheck the user had made but not yet saved. loadLabels() now carries the current checkbox states across the rebuild. - Guard the response parse: once the POST returns 2xx the backend has already persisted the set, so a malformed (non-JSON) body must not throw past that point and trip the caller's rollback, which would desync the UI.
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
3 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.
Closes #458, Closes #459
Wires the Chrome extension options page to the server-side PII config endpoints introduced in #509 / #513. The config UI from #470 is brought over and re-pointed at the new contract; the backend changes in #470 are superseded by #509/#513, so this PR is extension-only.
Entity types (
/api/pii/entities)GET({ available, disabled }). Checked = masked.disabled(passthrough) set viaPOST { disabled: [...] }. A "Disable all / Enable all" toggle is included.400and surfaced inline.Custom patterns (
/api/pii/regexes)name+pattern) with a live match preview before saving.POSTreplaces the whole set, so add/remove rebuild the list and re-POST. Each custom name then appears as a selectable entity type in the grid.Notes
GenericGenerator→[REDACTED_<NAME>_<hash>]. Demasking/substitution for custom types remains open.🤖 Generated with Claude Code