[cgivre]

Potential fix for https://github.com/apache/drill/security/code-scanning/43

To fix this without changing intended behavior, enforce explicit hostname validation against the set of known Drillbit endpoints before constructing/sending the HTTP request.

Best approach:

• In DrillRoot.shutdownDrillbitByName(...), validate hostname using work.getContext().getAvailableBits() and only proceed if there is an exact match.
• Reject invalid hostnames with an exception (consistent with existing exception-driven flow).
• Keep using WebUtils.getDrillbitURL(...) and doHTTPRequest(...) after validation so functionality remains unchanged for valid Drillbit hosts.

This is a minimal, targeted fix in:

• exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
  • Method: shutdownDrillbitByName(...)
  • Add a pre-check for null/empty and allowlist membership.

No changes are required in WebUtils.java for this fix.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

[pjfanning]

I think this change is redundant. WebUtils.getDrillbitURL already validates the hostname against the registered Drillbits before it builds any URL:

int drillbitPort = work.getContext().getAvailableBits().stream()
    .filter(db -> db.getAddress().equals(hostname))
    .findAny()
    .map(DrillbitEndpoint::getHttpPort)
    .orElseThrow(() -> new RuntimeException("No such drillbit: " + hostname));

An unknown hostname throws before any request is sent, and the port comes from the endpoint record rather than from user input. The new block runs the same allowlist match a second time. Other notes:

• The null/empty check can't trigger. {hostname} is a path segment, so an empty value won't route to this method.
• The only behaviour change is the exception type (RuntimeException becomes IllegalArgumentException). Both return a 500.
• The endpoint is already @RolesAllowed(ADMIN_ROLE).

I'd suggest closing this and dismissing code-scanning alert 43 as a false positive, pointing to the allowlist lookup in getDrillbitURL. That lookup also covers the other callers of getDrillbitURL. If we want to make that clear to future readers or scanners, a short comment in getDrillbitURL would be better than duplicating the check at each call site.