Skip to content

feat: add configurable sensitive word detection - #5

Closed
Timik232 wants to merge 1 commit into
KikuAI-Lab:mainfrom
Timik232:feature/sensitive-words
Closed

feat: add configurable sensitive word detection#5
Timik232 wants to merge 1 commit into
KikuAI-Lab:mainfrom
Timik232:feature/sensitive-words

Conversation

@Timik232

@Timik232 Timik232 commented May 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a new SENSITIVE_WORD entity type that allows detecting user-defined sensitive words via the MASKER_SENSITIVE_WORDS environment variable.

Changes

  • New entity type: SENSITIVE_WORD added to EntityType
  • Config: MASKER_SENSITIVE_WORDS env var (comma-separated word list)
  • Detection: Case-insensitive regex with \b word boundaries
  • Overlap handling: Lowest priority — loses to EMAIL, CARD, PHONE, PERSON
  • Masking/Redaction: Full support via existing endpoints
  • Policy: sensitive_word: mask in default policy

Usage

MASKER_SENSITIVE_WORDS="secret,confidential,password" python -m app.main
curl -X POST /api/v1/detect -d '{"text": "This is a secret document"}'
# Returns: {"entities": [{"type": "SENSITIVE_WORD", "value": "secret", ...}]}

Testing

  • Added tests/test_sensitive_words.py with class-based tests
  • All existing tests pass without modification
  • Tested with Cyrillic words (e.g., MASKER_SENSITIVE_WORDS="СВО")

Add SENSITIVE_WORD entity type that detects user-defined words
from MASKER_SENSITIVE_WORDS env var (comma-separated).

- Case-insensitive word boundary matching
- Lowest priority in overlap resolution (EMAIL/CARD/PHONE/PERSON win)
- Full support for detect, mask, and redact endpoints
- Policy entry in default.yaml

@kiku-jw kiku-jw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Blocking correctness/security issue in app/services/pii_detector.py:

SENSITIVE_WORD is documented as the lowest-priority entity, but _remove_overlaps() sorts primarily by start offset and then greedily accepts the first match. A configured phrase that starts before an overlapping EMAIL therefore suppresses the EMAIL match. Reproduction against the PR merged with current main:

settings.sensitive_words = "Contact test"
PIIDetector().detect("Contact test@example.com")
# => only SENSITIVE_WORD(0, 12)

POST /api/v1/mask then returns ***@example.com, leaking part of the email address. Please make overlap selection honor entity priority across the whole overlapping range (not only identical start positions) and add a regression test where a sensitive phrase starts before an overlapping EMAIL.

Verification performed on the clean merge with current main: ruff check app tests, ruff format --check app, and the full test suite passed (100 passed, 19 skipped); the targeted overlap reproduction above still fails the security expectation.

@kiku-jw

kiku-jw commented Jul 28, 2026

Copy link
Copy Markdown
Member

Thank you for the contribution and for taking the time to add tests. After reviewing the project status, Masker is currently parked as a portfolio/maintenance-only project, so we are not taking on new feature expansion.

The overlap issue in the review would require additional security-sensitive redesign, and it would not be fair to ask you to spend more time on that for a feature we do not plan to ship. Closing this as not planned rather than waiting for changes. The contribution remains preserved in the PR history.

@kiku-jw

kiku-jw commented Jul 28, 2026

Copy link
Copy Markdown
Member

I reconsidered this after reviewing the product value: an external contribution is itself useful demand evidence. Your implementation has now landed on main via #26, with your original commit and authorship preserved.

I added a safety fix so a configured phrase cannot partially override higher-priority PII (for example, expose part of an email), plus API/docs coverage. Thank you for contributing this feature.

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