Fix SHOW QUERIES and KILL QUERY privilege filtering

[shuwenwei]

Summary

• Restrict tree-model SHOW QUERIES results to the current user when the user does not satisfy the MAINTAIN authorization check.
• Restrict tree-model KILL QUERY to the current user's queries under the same condition.
• Allow users with the SYSTEM privilege, which contains the deprecated MAINTAIN privilege, to view and kill queries from all users.
• Add IoTDBShowQueriesIT and IoTDBKillQueryIT with queries that remain active across Session fetches.

Root cause

TreeAccessCheckVisitor.visitShowQueries and visitKillQuery set allowedUsername when the MAINTAIN authorization check succeeded instead of when it failed. As a result, an unprivileged user could see or kill other users' queries, while a privileged user could be unnecessarily restricted.

Impact

Tree-model SHOW QUERIES and KILL QUERY now apply the intended visibility and control rules while retaining the existing MAINTAIN authorization check.

Validation

• Added integration tests that leave result sets unconsumed and verify query visibility and kill permissions before and after granting SYSTEM; the tests do not grant deprecated MAINTAIN directly.
• Tests were not run locally, following the repository instruction not to proactively run build or test checks unless requested.

[sonarqubecloud]

Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[codecov]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.39%. Comparing base (a8497be) to head (b1ce540).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...an/relational/security/TreeAccessCheckVisitor.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #18359      +/-   ##
============================================
- Coverage     43.40%   43.39%   -0.01%     
  Complexity      374      374              
============================================
  Files          5366     5366              
  Lines        382885   382885              
  Branches      49796    49796              
============================================
- Hits         166187   166171      -16     
- Misses       216698   216714      +16     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[PDGGK]

The inversion is right, and the root-cause line in the description is accurate: with the old condition the restriction was applied only to users who passed the MAINTAIN check, so the default for an unprivileged user was an unset allowedUsername, which is the see-everything case. Two things I checked that are worth recording, and one suggestion.

No audit-log regression. checkHasGlobalAuth is not side-effect-free — it calls AUDIT_LOGGER.log(...) with context.setResult(result) on every invocation. Inverting the branch does not change whether that call happens, only what runs inside it, so both the CONTROL audit event on KILL QUERY and the recorded pass/fail result are unchanged by this patch. Worth stating explicitly because inverting a condition around an audited check is the kind of change where a reviewer would reasonably worry about exactly that.

The SYSTEM claim holds. PrivilegeType.getAllPrivilegesContainingCurrentPrivilege() returns [MAINTAIN, SYSTEM] for MAINTAIN, so a user holding only SYSTEM still satisfies the MAINTAIN check and keeps cluster-wide visibility after the fix. That is the case I would most expect a fix in this direction to break — an operator with SYSTEM suddenly restricted to their own queries — and it does not break.

Suggestion, not a blocker: consider adopting the table-model shape. The table-model path for the same operation is written the other way round, in TableConfigTaskVisitor.visitKillQuery:

String allowedUsername = context.getSession().getUserName();
if (accessControl.hasGlobalPrivilege(context.getSession().getUserEntity(), PrivilegeType.SYSTEM)) {
  allowedUsername = null;
}

It starts restricted and widens only on a positive privilege result. The tree version, even after this patch, starts unrestricted and narrows inside an if — so the safe outcome depends on that branch being reached and correct. That is precisely how the original defect was able to exist, and it is why a single missing ! was a privilege escalation rather than a denial of service. If the tree sites were initialized to context.getUsername() and cleared on a successful check, the same class of mistake would fail closed instead of open.

I checked whether the table-model path had the mirror of this bug before suggesting this — it does not, for the reason above.

Two smaller notes:

• The description says SYSTEM "contains the deprecated MAINTAIN privilege". Since getReplacedPrivilegeType() maps MAINTAIN to SYSTEM, it may be worth a short comment at these two call sites recording that MAINTAIN is retained here for compatibility, so a future cleanup pass does not delete the check and silently widen visibility.
• The new ITs keep queries alive across Session fetches, which is the hard part of testing this — good. If it is cheap to add, the case I would most want pinned is a SYSTEM-only user (no explicit MAINTAIN grant) seeing another user's query, since that is the path that depends on the subsumption above rather than on the changed line itself.