Skip to content
Merged
Changes from all commits
Commits
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
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 @@ -91,7 +92,7 @@ public JSONObject queryRange(
Request request = new Request.Builder().url(queryUrl).build();

logger.debug("Executing Prometheus request with headers: {}", request.headers().toString());
Response response = this.prometheusHttpClient.newCall(request).execute();
Response response = AccessController.doPrivilegedChecked(() -> this.prometheusHttpClient.newCall(request).execute());

logger.debug("Received Prometheus response for query_range: code={}", response);

Expand Down Expand Up @@ -126,7 +127,7 @@ public JSONObject query(String query, Long time, Integer limit, Integer timeout)
Request request = new Request.Builder().url(queryUrl).build();

logger.info("Executing Prometheus request with headers: {}", request.headers().toString());
Response response = this.prometheusHttpClient.newCall(request).execute();
Response response = AccessController.doPrivilegedChecked(() -> this.prometheusHttpClient.newCall(request).execute());

logger.info("Received Prometheus response for instant query: code={}", response);
// Return the full response object, not just the data field
Expand All @@ -146,7 +147,7 @@ public List<String> getLabels(Map<String, String> queryParams) throws IOExceptio
"%s/api/v1/labels%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());
JSONObject jsonObject = readResponse(response);
return toListOfLabels(jsonObject.getJSONArray("data"));
}
Expand All @@ -161,7 +162,7 @@ public List<String> getLabel(String labelName, Map<String, String> queryParams)
prometheusUri.toString().replaceAll("/$", ""), labelName, 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());
JSONObject jsonObject = readResponse(response);
return toListOfLabels(jsonObject.getJSONArray("data"));
}
Expand All @@ -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());
Copy link
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.

Copy link
Collaborator

@RyanL1997 RyanL1997 Jan 21, 2026

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

Copy link
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 All @@ -194,7 +195,7 @@ public List<Map<String, String>> getSeries(Map<String, String> queryParams) thro
"%s/api/v1/series%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());
JSONObject jsonObject = readResponse(response);
JSONArray dataArray = jsonObject.getJSONArray("data");
return toListOfSeries(dataArray);
Expand All @@ -211,7 +212,7 @@ public JSONArray queryExemplars(String query, Long start, Long end) throws IOExc
end);
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());
JSONObject jsonObject = readResponse(response);
return jsonObject.getJSONArray("data");
}
Expand All @@ -222,7 +223,7 @@ public JSONObject getAlerts() throws IOException {
String.format("%s/api/v1/alerts", prometheusUri.toString().replaceAll("/$", ""));
logger.debug("Making Prometheus alerts request: {}", 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());
JSONObject jsonObject = readResponse(response);
return jsonObject.getJSONObject("data");
}
Expand All @@ -235,7 +236,7 @@ public JSONObject getRules(Map<String, String> queryParams) throws IOException {
"%s/api/v1/rules%s", prometheusUri.toString().replaceAll("/$", ""), queryString);
logger.debug("Making Prometheus rules request: {}", 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());
JSONObject jsonObject = readResponse(response);
return jsonObject.getJSONObject("data");
}
Expand All @@ -248,7 +249,7 @@ public JSONArray getAlertmanagerAlerts(Map<String, String> queryParams) throws I

logger.debug("Making Alertmanager alerts request: {}", queryUrl);
Request request = new Request.Builder().url(queryUrl).build();
Response response = this.alertmanagerHttpClient.newCall(request).execute();
Response response = AccessController.doPrivilegedChecked(() -> this.alertmanagerHttpClient.newCall(request).execute());

return readAlertmanagerResponse(response);
}
Expand All @@ -261,7 +262,7 @@ public JSONArray getAlertmanagerAlertGroups(Map<String, String> queryParams) thr

logger.debug("Making Alertmanager alert groups request: {}", queryUrl);
Request request = new Request.Builder().url(queryUrl).build();
Response response = this.alertmanagerHttpClient.newCall(request).execute();
Response response = AccessController.doPrivilegedChecked(() -> this.alertmanagerHttpClient.newCall(request).execute());

return readAlertmanagerResponse(response);
}
Expand All @@ -273,7 +274,7 @@ public JSONArray getAlertmanagerReceivers() throws IOException {

logger.debug("Making Alertmanager receivers request: {}", queryUrl);
Request request = new Request.Builder().url(queryUrl).build();
Response response = this.alertmanagerHttpClient.newCall(request).execute();
Response response = AccessController.doPrivilegedChecked(() -> this.alertmanagerHttpClient.newCall(request).execute());

return readAlertmanagerResponse(response);
}
Expand All @@ -285,7 +286,7 @@ public JSONArray getAlertmanagerSilences() throws IOException {

logger.debug("Making Get Alertmanager silences request: {}", queryUrl);
Request request = new Request.Builder().url(queryUrl).build();
Response response = this.alertmanagerHttpClient.newCall(request).execute();
Response response = AccessController.doPrivilegedChecked(() -> this.alertmanagerHttpClient.newCall(request).execute());

return readAlertmanagerResponse(response);
}
Expand All @@ -301,7 +302,7 @@ public String createAlertmanagerSilences(String silenceJson) throws IOException
.header("Content-Type", "application/json")
.post(RequestBody.create(silenceJson.getBytes(StandardCharsets.UTF_8)))
.build();
Response response = this.alertmanagerHttpClient.newCall(request).execute();
Response response = AccessController.doPrivilegedChecked(() -> this.alertmanagerHttpClient.newCall(request).execute());

if (response.isSuccessful()) {
return Objects.requireNonNull(response.body()).string();
Expand Down
Loading