Fixes #29145: Prevent NPE when query tags are null#29249
Conversation
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
Hi maintainers, I addressed the review feedback by using It looks like the PR is currently blocked because:
Could someone please help with the project metadata and workflow approval? Thank you! |
harshach
added
the
safe to test
|
The Java checkstyle failed. Please run You can install the pre-commit hooks with |
🟡 Playwright Results — all passed (15 flaky)✅ 4305 passed · ❌ 0 failed · 🟡 15 flaky · ⏭️ 88 skipped
🟡 15 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Code Review ✅ ApprovedAdds a null guard to the PIIMasker query-tag stream to prevent NullPointerException and includes a regression test for unauthorized user access. The implementation correctly resolves the reported issue. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
2 participants
Describe your changes:
Fixes #29145
This PR fixes a NullPointerException in
PIIMaskerwhen query tags are not loaded andquery.getTags()returnsnull.The issue occurred when non-admin users accessed queries without requesting the
tagsfield (for example,GET /api/v1/queries?fields=queryUsedIn). The masking logic attempted to call.stream()on a null tag list, resulting in a server error.Changes made:
hasPiiSensitiveTag(Query)before streaming tags.Type of change:
High-level design:
N/A — small change.
Tests:
Use cases covered
query.getTags()isnullUnit tests
I added unit tests for the new/changed logic.
Files added/updated:
Backend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
query.setTags(null)Cannot invoke "java.util.List.stream()" because query.getTags() is nullhasPiiSensitiveTag(Query)PIIMaskerTestUI screen recording / screenshots:
Not applicable.
Checklist:
Fixes #29145: Prevent NPE when query tags are nullFixes #29145above.Greptile Summary
This PR adds a null guard to
hasPiiSensitiveTag(Query)inPIIMasker.java, preventing an NPE whenquery.getTags()returnsnull— a situation that arises when non-admin users access queries without requesting thetagsfield. A regression test is included that exercises the exact failure path.PIIMasker.java: Replaced the direct.getTags().stream()call withlistOrEmpty(query.getTags()).stream(), consistent with thelistOrEmptyutility already used elsewhere in the class.PIIMaskerTest.java: Added a test that setstagstonull, simulates an unauthorized user, and asserts no NPE is thrown and the query text passes through correctly.Confidence Score: 5/5
Safe to merge — the change is a single, well-scoped null guard that matches an existing utility pattern in the class.
The fix replaces one unsafe
.getTags().stream()call withlistOrEmpty(query.getTags()).stream(), which is exactly how every other nullable tag list in the class is handled. The regression test directly reproduces the reported failure path. No behavior changes for the happy path.No files require special attention.
Important Files Changed
.getTags().stream()withlistOrEmpty(query.getTags()).stream()to guard against null tags on Query; change is minimal, correct, and consistent with existing codebase patterns.Comments Outside Diff (1)
openmetadata-service/src/main/java/org/openmetadata/service/security/mask/PIIMasker.java, line 312-344 (link)Table,SearchIndex, andTopicoverloadshasPiiSensitiveTag(Table)(line 313),hasPiiSensitiveTag(SearchIndex)(line 324), andhasPiiSensitiveTag(Topic)(line 333) all call.getTags().stream()without a null guard. If any API path retrieves those entities without thetagsfield (just asGET /queries?fields=queryUsedInreproduced the original bug), the identical NPE will surface. Notably,hasPiiSensitiveTag(Container)already has the null check at line 317, confirming the pattern has been hit before — it just wasn't applied consistently.Reviews (3): Last reviewed commit: "Apply spotless formatting" | Re-trigger Greptile