Skip to content

Fix issue connecting with prometheus by wrapping with AccessController.doPrivilegedChecked #5061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Swiddis merged 2 commits into from
Feb 2, 2026
Merged

Fix issue connecting with prometheus by wrapping with AccessController.doPrivilegedChecked #5061

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15b2c78
Fix issue connecting with prometheus by wrapping with AccessControlle…
cwperks Jan 20, 2026
5cd034c
add wrapper for other Prometheus calls
joshuali925 Feb 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ...t-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
import org.opensearch.secure_sm.AccessController;
import org.opensearch.sql.prometheus.exception.PrometheusClientException;
import org.opensearch.sql.prometheus.model.MetricMetadata;

Expand Down Expand Up @@ -175,7 +176,7 @@ public Map<String, List<MetricMetadata>> getAllMetrics(Map<String, String> query
"%s/api/v1/metadata%s", prometheusUri.toString().replaceAll("/$", ""), queryString);
logger.debug("queryUrl: " + queryUrl);
Request request = new Request.Builder().url(queryUrl).build();
Response response = this.prometheusHttpClient.newCall(request).execute();
Response response = AccessController.doPrivilegedChecked(() -> this.prometheusHttpClient.newCall(request).execute());

@coderabbitai coderabbitai Bot Jan 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify all HTTP call sites in the file that may need the same fix
echo "=== HTTP calls using prometheusHttpClient ==="
rg -n "prometheusHttpClient\.newCall" direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java

echo ""
echo "=== HTTP calls using alertmanagerHttpClient ==="
rg -n "alertmanagerHttpClient\.newCall" direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java

echo ""
echo "=== Calls already wrapped with doPrivilegedChecked ==="
rg -n "doPrivilegedChecked" direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java

Repository: opensearch-project/sql

Length of output: 1543

🏁 Script executed:

#!/bin/bash
# Extract method signatures to confirm they are distinct methods
echo "=== Method signatures at the call sites ==="
rg -B 5 "prometheusHttpClient\.newCall|alertmanagerHttpClient\.newCall" direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java | grep -E "^\s*(public|private)\s+|prometheusHttpClient\.newCall|alertmanagerHttpClient\.newCall" | head -50

Repository: opensearch-project/sql

Length of output: 1308

🏁 Script executed:

#!/bin/bash
# Extract method signatures more clearly
echo "=== Method signatures with line numbers ==="
rg -B 10 "prometheusHttpClient\.newCall|alertmanagerHttpClient\.newCall" \
  direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java \
  | grep -E "^\s*[0-9]+-\s*(public|private)" | head -20

Repository: opensearch-project/sql

Length of output: 109

🏁 Script executed:

#!/bin/bash
# Read the file with line numbers to see the method context
head -330 direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java | tail -150 | cat -n

Repository: opensearch-project/sql

Length of output: 7759

🏁 Script executed:

#!/bin/bash
# Read the first part of the file to see queryRange and other methods
head -200 direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java | cat -n

Repository: opensearch-project/sql

Length of output: 9152

🏁 Script executed:

#!/bin/bash
# Check context around line 179 for any comments explaining the wrapper
sed -n '168,183p' direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java | cat -n

Repository: opensearch-project/sql

Length of output: 993

Inconsistent application of the security fix across HTTP call sites.

The AccessController.doPrivilegedChecked wrapper is applied only to getAllMetrics (line 179), but 13 other methods in this class make HTTP calls using the same pattern (prometheusHttpClient.newCall(request).execute() or alertmanagerHttpClient.newCall(request).execute()). If the SecurityException is triggered by socket connections in getAllMetrics, it will likely occur in other methods making identical HTTP calls:

  • queryRange (line 95)
  • query (line 130)
  • getLabels (line 150)
  • getLabel (line 165)
  • getSeries (line 198)
  • queryExemplars (line 215)
  • getAlerts (line 226)
  • getRules (line 239)
  • getAlertmanagerAlerts (line 252)
  • getAlertmanagerAlertGroups (line 265)
  • getAlertmanagerReceivers (line 277)
  • getAlertmanagerSilences (line 289)
  • createAlertmanagerSilences (line 305)

Extract a helper method to centralize the privileged execution and apply it consistently:

Suggested approach 
private Response executePrivileged(Request request, OkHttpClient client) throws IOException {
    return AccessController.doPrivilegedChecked(() -> client.newCall(request).execute());
}

Then use it consistently across all HTTP call sites:

-Response response = this.prometheusHttpClient.newCall(request).execute();
+Response response = executePrivileged(request, this.prometheusHttpClient);
🤖 Prompt for AI Agents 
In
`@direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java`
at line 179, Add a centralized helper in PrometheusClientImpl named something
like executePrivileged(Request request, OkHttpClient client) that wraps
AccessController.doPrivilegedChecked(() -> client.newCall(request).execute())
and throws IOException; then replace every direct call to
prometheusHttpClient.newCall(request).execute() and
alertmanagerHttpClient.newCall(request).execute() across methods queryRange,
query, getLabels, getLabel, getAllMetrics, getSeries, queryExemplars, getAlerts,
getRules, getAlertmanagerAlerts, getAlertmanagerAlertGroups,
getAlertmanagerReceivers, getAlertmanagerSilences, and
createAlertmanagerSilences to call executePrivileged(request,
prometheusHttpClient) or executePrivileged(request, alertmanagerHttpClient)
respectively so the security wrapper is applied consistently.

@RyanL1997 RyanL1997 Jan 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the change. The change itself looks good to me, however, I'm also curious the following:

  • among all these components in the above list, why only the Prometheus got impacted?
  • why this just be spotted by now?

cc @cwperks @joshuali925 @lezzago

@joshuali925 joshuali925 Feb 2, 2026

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.

among all these components in the above list, why only the Prometheus got impacted?

the components in the above list are also for prometheus. I just included them in this PR

I don't know why only Prometheus got impacted, or if any other components got impacted

why this just be spotted by now?

This issue does not occur with core + sql, it needs some other plugins that intercepts and filters requests to trigger. We found it when testing the distribution with all plugins installed

JSONObject jsonObject = readResponse(response);
TypeReference<HashMap<String, List<MetricMetadata>>> typeRef = new TypeReference<>() {};
return new ObjectMapper().readValue(jsonObject.getJSONObject("data").toString(), typeRef);
Expand Down
Loading