Skip to content

Fix NPE in matchAnyCertification when entity is null#26904

Merged
sonika-shah merged 10 commits intomainfrom
fix-26875
Apr 2, 2026
Merged

Fix NPE in matchAnyCertification when entity is null#26904
sonika-shah merged 10 commits intomainfrom
fix-26875

Conversation

@sonika-shah
Copy link
Copy Markdown
Collaborator

Describe your changes:

Fixes #26875

Summary

  • matchAnyCertification in RuleEvaluator threw a NullPointerException when
    ResourceContext.getEntity() returned null (e.g. Settings, Team, or list operations).
    Optional.ofNullable(resourceContext.getEntity().getCertification()) NPE'd on the
    .getCertification() call before Optional.ofNullable could do anything — Java
    evaluates the argument first.
  • Added a null guard for getEntity(), consistent with isReviewer() and hasDomain()
    in the same class which already have this check.

Test plan

  • Unit tests: test_matchAnyCertification_nullEntity and
    test_matchAnyCertification_nullResourceContext in RuleEvaluatorTest
  • Integration test: test_matchAnyCertification_noCertificationOnEntity in
    QueryVisibilityPolicyIT — creates a policy with matchAnyCertification condition,
    assigns it to a user, then GETs a table with no certification (the exact repro path)
  • All 17 RuleEvaluatorTest tests pass

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Copilot AI review requested due to automatic review settings April 1, 2026 03:48
@sonika-shah sonika-shah added safe to test Add this label to run secure Github workflows on PRs backend labels Apr 1, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a NullPointerException in backend policy evaluation when matchAnyCertification is evaluated with a ResourceContext whose getEntity() is null, and adds regression coverage to prevent recurrence.

Changes:

  • Add an explicit null-guard for resourceContext.getEntity() in RuleEvaluator.matchAnyCertification.
  • Add unit tests covering null entity and null resource context scenarios for matchAnyCertification.
  • Add an integration test intended to reproduce the policy-evaluation NPE path.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/RuleEvaluator.java Adds a null check for resourceContext.getEntity() before accessing certification.
openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/RuleEvaluatorTest.java Adds unit tests ensuring matchAnyCertification returns false when entity/context is null.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/QueryVisibilityPolicyIT.java Adds an integration test for matchAnyCertification policy evaluation regression coverage.

Copilot AI review requested due to automatic review settings April 1, 2026 04:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Copilot AI review requested due to automatic review settings April 2, 2026 10:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings April 2, 2026 12:36
@sonika-shah sonika-shah enabled auto-merge (squash) April 2, 2026 12:37
@sonika-shah sonika-shah added the To release Will cherry-pick this PR into the release branch label Apr 2, 2026
@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Apr 2, 2026

Code Review ✅ Approved

Fixes null pointer exception in matchAnyCertification when entity parameter is null. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 2, 2026

@sonika-shah sonika-shah merged commit 7c7ba41 into main Apr 2, 2026
69 of 97 checks passed
@sonika-shah sonika-shah deleted the fix-26875 branch April 2, 2026 14:26
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

Changes have been cherry-picked to the 1.12.5 branch.

github-actions Bot pushed a commit that referenced this pull request Apr 2, 2026
* Fix NPE in matchAnyCertification when entity is null

* fix QueryVisibilityPolicyIT test

* fix QueryVisibilityPolicyIT test

* fix QueryVisibilityPolicyIT test - address feedback

* fix QueryVisibilityPolicyIT test - address feedback

* address feedback

---------

Co-authored-by: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com>
(cherry picked from commit 7c7ba41)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants