mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 17:08:34 +08:00
fix: send automation API calls directly to the cloud host (#1309)
Co-authored-by: Rohit Malhotra <rohitvinodmalhotra@gmail.com>
This commit is contained in:
committed by
GitHub
co-authored by
Rohit Malhotra
parent
d20aa65228
commit
7537dab99d
@@ -335,9 +335,9 @@ describe("AutomationService", () => {
|
||||
});
|
||||
|
||||
// When the active backend is cloud the local axios instance must be
|
||||
// bypassed entirely; calls must route through `callCloudProxy` so the
|
||||
// bundled local agent-server forwards the request server-side to the
|
||||
// cloud host.
|
||||
// bypassed entirely; calls must route through `callCloudProxy`, which
|
||||
// sends them directly to the cloud host from the browser (the automation
|
||||
// service grants permissive CORS to API-key requests, automation#185).
|
||||
describe("cloud routing", () => {
|
||||
beforeEach(() => {
|
||||
mockGetActive.mockReturnValue({ backend: cloudBackend, orgId: null });
|
||||
@@ -358,9 +358,7 @@ describe("AutomationService", () => {
|
||||
expect(mockCallCloudProxy).toHaveBeenCalledWith({
|
||||
backend: cloudBackend,
|
||||
method: "GET",
|
||||
path: "/api/automation/v1?limit=10&offset=5",
|
||||
forceProxy: true,
|
||||
});
|
||||
path: "/api/automation/v1?limit=10&offset=5", });
|
||||
expect(mockGet).not.toHaveBeenCalled();
|
||||
expect(result).toEqual(response);
|
||||
});
|
||||
@@ -373,9 +371,7 @@ describe("AutomationService", () => {
|
||||
expect(mockCallCloudProxy).toHaveBeenCalledWith({
|
||||
backend: cloudBackend,
|
||||
method: "GET",
|
||||
path: "/api/automation/v1/abc",
|
||||
forceProxy: true,
|
||||
});
|
||||
path: "/api/automation/v1/abc", });
|
||||
expect(result).toEqual(mockAutomation);
|
||||
});
|
||||
|
||||
@@ -387,9 +383,7 @@ describe("AutomationService", () => {
|
||||
expect(mockCallCloudProxy).toHaveBeenCalledWith({
|
||||
backend: cloudBackend,
|
||||
method: "POST",
|
||||
path: "/api/automation/v1/abc/dispatch",
|
||||
forceProxy: true,
|
||||
});
|
||||
path: "/api/automation/v1/abc/dispatch", });
|
||||
expect(mockPost).not.toHaveBeenCalled();
|
||||
expect(result).toEqual(mockRun);
|
||||
});
|
||||
@@ -406,9 +400,7 @@ describe("AutomationService", () => {
|
||||
backend: cloudBackend,
|
||||
method: "PATCH",
|
||||
path: "/api/automation/v1/abc",
|
||||
body: { enabled: false },
|
||||
forceProxy: true,
|
||||
});
|
||||
body: { enabled: false }, });
|
||||
expect(mockPatch).not.toHaveBeenCalled();
|
||||
expect(result).toEqual(updated);
|
||||
});
|
||||
@@ -421,9 +413,7 @@ describe("AutomationService", () => {
|
||||
expect(mockCallCloudProxy).toHaveBeenCalledWith({
|
||||
backend: cloudBackend,
|
||||
method: "DELETE",
|
||||
path: "/api/automation/v1/abc",
|
||||
forceProxy: true,
|
||||
});
|
||||
path: "/api/automation/v1/abc", });
|
||||
expect(mockDelete).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -444,14 +434,12 @@ describe("AutomationService", () => {
|
||||
expect(mockCallCloudProxy).toHaveBeenCalledWith({
|
||||
backend: cloudBackend,
|
||||
method: "POST",
|
||||
path: "/api/automation/v1/abc/dispatch",
|
||||
forceProxy: true,
|
||||
});
|
||||
path: "/api/automation/v1/abc/dispatch", });
|
||||
expect(mockPost).not.toHaveBeenCalled();
|
||||
expect(result).toEqual(run);
|
||||
});
|
||||
|
||||
it("checkHealth tunnels through the cloud proxy with a fail-fast timeout and returns the upstream status", async () => {
|
||||
it("checkHealth calls the cloud host with a fail-fast timeout and returns the upstream status", async () => {
|
||||
mockCallCloudProxy.mockResolvedValue({ status: "ok" });
|
||||
|
||||
const result = await AutomationService.checkHealth();
|
||||
@@ -461,13 +449,12 @@ describe("AutomationService", () => {
|
||||
const call = mockCallCloudProxy.mock.calls[0]![0];
|
||||
expect(call.method).toBe("GET");
|
||||
expect(call.path).toBe("/api/automation/health");
|
||||
expect(call.forceProxy).toBe(true);
|
||||
expect(call.timeoutSeconds).toBe(5);
|
||||
expect(mockGet).not.toHaveBeenCalled();
|
||||
expect(result).toEqual({ status: "ok" });
|
||||
});
|
||||
|
||||
it("checkHealth resolves to an error status instead of throwing when the proxy call fails", async () => {
|
||||
it("checkHealth resolves to an error status instead of throwing when the cloud call fails", async () => {
|
||||
mockCallCloudProxy.mockRejectedValue(new Error("proxy unreachable"));
|
||||
|
||||
const result = await AutomationService.checkHealth();
|
||||
|
||||
@@ -99,21 +99,80 @@ describe("callCloudProxy X-Org-Id injection", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("callCloudProxy forceProxy routing", () => {
|
||||
it("routes through the local /api/cloud-proxy instead of the cloud host when forceProxy is set", async () => {
|
||||
// Arrange — automation endpoints opt into the proxy hop because the
|
||||
// standalone automation service's CORS allowlist rejects browser
|
||||
// requests from the local GUI origin.
|
||||
describe("callCloudProxy automation direct routing", () => {
|
||||
it("sends automation requests straight to the cloud host with the API key instead of the /api/cloud-proxy envelope", async () => {
|
||||
// Arrange — the automation service grants permissive CORS to API-key
|
||||
// requests (automation#185), so app-host automation calls no longer
|
||||
// need the same-origin proxy hop through the bundled agent-server.
|
||||
setRegisteredBackends([cloudPersonal]);
|
||||
setActiveSelection({ backendId: cloudPersonal.id, orgId: null });
|
||||
vi.mocked(axios.post).mockResolvedValue({ data: { status: "ok" } });
|
||||
const page = { automations: [], total: 0 };
|
||||
vi.mocked(axios.request).mockResolvedValue({ data: page });
|
||||
|
||||
// Act
|
||||
const result = await callCloudProxy({
|
||||
backend: cloudPersonal,
|
||||
method: "GET",
|
||||
path: "/api/automation/health",
|
||||
forceProxy: true,
|
||||
path: "/api/automation/v1?limit=50&offset=0",
|
||||
});
|
||||
|
||||
// Assert — the browser calls the automation API on the cloud host
|
||||
// directly, authenticated by the backend's API key, and no envelope
|
||||
// POST reaches /api/cloud-proxy.
|
||||
expect(axios.post).not.toHaveBeenCalled();
|
||||
const [config] = vi.mocked(axios.request).mock.calls[0]!;
|
||||
expect(config).toMatchObject({
|
||||
url: `${cloudPersonal.host}/api/automation/v1?limit=50&offset=0`,
|
||||
method: "GET",
|
||||
});
|
||||
expect(
|
||||
(config as { headers: Record<string, string> }).headers.Authorization,
|
||||
).toBe(`Bearer ${cloudPersonal.apiKey}`);
|
||||
expect(result).toEqual(page);
|
||||
});
|
||||
|
||||
it("forwards the blob responseType and fail-fast timeout to the direct request", async () => {
|
||||
// Arrange — tarball downloads and health probes rely on these
|
||||
// per-request options surviving the switch from the proxy envelope to
|
||||
// the direct call.
|
||||
setRegisteredBackends([cloudPersonal]);
|
||||
setActiveSelection({ backendId: cloudPersonal.id, orgId: null });
|
||||
|
||||
// Act
|
||||
await callCloudProxy({
|
||||
backend: cloudPersonal,
|
||||
method: "GET",
|
||||
path: "/api/automation/v1/auto-1/tarball",
|
||||
responseType: "blob",
|
||||
timeoutSeconds: 5,
|
||||
});
|
||||
|
||||
// Assert
|
||||
const [config] = vi.mocked(axios.request).mock.calls[0]!;
|
||||
expect(config).toMatchObject({
|
||||
responseType: "blob",
|
||||
timeout: 5000,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("callCloudProxy hostOverride routing", () => {
|
||||
const runtimeHost = "https://abc123.prod-runtime.all-hands.dev";
|
||||
|
||||
it("routes through the local /api/cloud-proxy instead of the upstream host when hostOverride is set", async () => {
|
||||
// Arrange — runtime-sandbox endpoints need the proxy hop because the
|
||||
// per-conversation runtime hosts reject browser requests from the
|
||||
// local GUI origin.
|
||||
setRegisteredBackends([cloudPersonal]);
|
||||
setActiveSelection({ backendId: cloudPersonal.id, orgId: null });
|
||||
vi.mocked(axios.post).mockResolvedValue({ data: { items: [] } });
|
||||
|
||||
// Act
|
||||
const result = await callCloudProxy({
|
||||
backend: cloudPersonal,
|
||||
method: "GET",
|
||||
path: "/api/bash/bash_events/search",
|
||||
hostOverride: runtimeHost,
|
||||
});
|
||||
|
||||
// Assert — the browser only makes a same-origin POST to the bundled
|
||||
@@ -123,11 +182,11 @@ describe("callCloudProxy forceProxy routing", () => {
|
||||
const [url, envelope] = vi.mocked(axios.post).mock.calls[0]!;
|
||||
expect(url).toMatch(/\/api\/cloud-proxy$/);
|
||||
expect(envelope).toMatchObject({
|
||||
host: cloudPersonal.host,
|
||||
host: runtimeHost,
|
||||
method: "GET",
|
||||
path: "/api/automation/health",
|
||||
path: "/api/bash/bash_events/search",
|
||||
});
|
||||
expect(result).toEqual({ status: "ok" });
|
||||
expect(result).toEqual({ items: [] });
|
||||
});
|
||||
|
||||
it("carries bearer auth and X-Org-Id inside the proxy envelope", async () => {
|
||||
@@ -144,8 +203,8 @@ describe("callCloudProxy forceProxy routing", () => {
|
||||
await callCloudProxy({
|
||||
backend: cloudPersonal,
|
||||
method: "GET",
|
||||
path: "/api/automation/health",
|
||||
forceProxy: true,
|
||||
path: "/api/bash/bash_events/search",
|
||||
hostOverride: runtimeHost,
|
||||
});
|
||||
|
||||
// Assert
|
||||
|
||||
@@ -10,7 +10,7 @@ import {
|
||||
getEffectiveLocalBackend,
|
||||
} from "../backend-registry/active-store";
|
||||
import { NoBackendAvailableError } from "../agent-server-client-options";
|
||||
import { callCloudProxy, type CloudProxyRequest } from "../cloud/proxy";
|
||||
import { callCloudProxy } from "../cloud/proxy";
|
||||
|
||||
const AUTOMATION_BASE_PATH = "/api/automation";
|
||||
|
||||
@@ -53,18 +53,6 @@ function buildPaginationQuery(limit: number, offset: number): string {
|
||||
return params.toString();
|
||||
}
|
||||
|
||||
// All /api/automation/* paths are served by the standalone automation
|
||||
// service, whose CORS allowlist (unlike the main cloud API's bearer-aware
|
||||
// CORS) excludes the local GUI origin — so cloud calls must tunnel through
|
||||
// the agent-server's /api/cloud-proxy instead of going direct from the
|
||||
// browser. Funnel every cloud branch through here so a future method can't
|
||||
// reintroduce the CORS failure.
|
||||
function callAutomationCloudProxy<TResponse>(
|
||||
req: Omit<CloudProxyRequest, "forceProxy">,
|
||||
): Promise<TResponse> {
|
||||
return callCloudProxy<TResponse>({ ...req, forceProxy: true });
|
||||
}
|
||||
|
||||
class AutomationService {
|
||||
static async listAutomations(
|
||||
params: { limit?: number; offset?: number } = {},
|
||||
@@ -73,7 +61,7 @@ class AutomationService {
|
||||
const active = getActiveBackend().backend;
|
||||
|
||||
if (active.kind === "cloud") {
|
||||
return callAutomationCloudProxy<AutomationsResponse>({
|
||||
return callCloudProxy<AutomationsResponse>({
|
||||
backend: active,
|
||||
method: "GET",
|
||||
path: `${AUTOMATION_BASE_PATH}/v1?${buildPaginationQuery(limit, offset)}`,
|
||||
@@ -99,7 +87,7 @@ class AutomationService {
|
||||
const path = `${AUTOMATION_BASE_PATH}/v1/${encodeURIComponent(id)}`;
|
||||
|
||||
if (active.kind === "cloud") {
|
||||
return callAutomationCloudProxy<Automation>({
|
||||
return callCloudProxy<Automation>({
|
||||
backend: active,
|
||||
method: "GET",
|
||||
path,
|
||||
@@ -118,7 +106,7 @@ class AutomationService {
|
||||
const path = `${AUTOMATION_BASE_PATH}/v1/${encodeURIComponent(id)}`;
|
||||
|
||||
if (active.kind === "cloud") {
|
||||
return callAutomationCloudProxy<Automation>({
|
||||
return callCloudProxy<Automation>({
|
||||
backend: active,
|
||||
method: "PATCH",
|
||||
path,
|
||||
@@ -135,7 +123,7 @@ class AutomationService {
|
||||
const path = `${AUTOMATION_BASE_PATH}/v1/${encodeURIComponent(id)}`;
|
||||
|
||||
if (active.kind === "cloud") {
|
||||
await callAutomationCloudProxy<unknown>({
|
||||
await callCloudProxy<unknown>({
|
||||
backend: active,
|
||||
method: "DELETE",
|
||||
path,
|
||||
@@ -151,7 +139,7 @@ class AutomationService {
|
||||
const path = `${AUTOMATION_BASE_PATH}/v1/${encodeURIComponent(id)}/dispatch`;
|
||||
|
||||
if (active.kind === "cloud") {
|
||||
return callAutomationCloudProxy<AutomationRun>({
|
||||
return callCloudProxy<AutomationRun>({
|
||||
backend: active,
|
||||
method: "POST",
|
||||
path,
|
||||
@@ -171,7 +159,7 @@ class AutomationService {
|
||||
const basePath = `${AUTOMATION_BASE_PATH}/v1/${encodeURIComponent(id)}/runs`;
|
||||
|
||||
if (active.kind === "cloud") {
|
||||
return callAutomationCloudProxy<AutomationRunsResponse>({
|
||||
return callCloudProxy<AutomationRunsResponse>({
|
||||
backend: active,
|
||||
method: "GET",
|
||||
path: `${basePath}?${buildPaginationQuery(limit, offset)}`,
|
||||
@@ -206,7 +194,7 @@ class AutomationService {
|
||||
|
||||
let blob: Blob;
|
||||
if (active.kind === "cloud") {
|
||||
blob = await callAutomationCloudProxy<Blob>({
|
||||
blob = await callCloudProxy<Blob>({
|
||||
backend: active,
|
||||
method: "GET",
|
||||
path,
|
||||
@@ -233,14 +221,13 @@ class AutomationService {
|
||||
|
||||
try {
|
||||
if (active.kind === "cloud") {
|
||||
const response =
|
||||
await callAutomationCloudProxy<AutomationHealthResponse>({
|
||||
backend: active,
|
||||
method: "GET",
|
||||
path,
|
||||
// Fail fast, matching the local branch's 5s timeout below.
|
||||
timeoutSeconds: 5,
|
||||
});
|
||||
const response = await callCloudProxy<AutomationHealthResponse>({
|
||||
backend: active,
|
||||
method: "GET",
|
||||
path,
|
||||
// Fail fast, matching the local branch's 5s timeout below.
|
||||
timeoutSeconds: 5,
|
||||
});
|
||||
return response;
|
||||
}
|
||||
|
||||
|
||||
+4
-17
@@ -49,18 +49,6 @@ export interface CloudProxyRequest {
|
||||
* payload (e.g. ZIP downloads); leave undefined for default JSON.
|
||||
*/
|
||||
responseType?: "blob";
|
||||
/**
|
||||
* Force this app-host call through the bundled agent-server's
|
||||
* `/api/cloud-proxy` instead of calling the cloud host directly from the
|
||||
* browser. App-host calls normally go direct because the main cloud API
|
||||
* loosens CORS for bearer-token requests (ApiKeyAwareCORSMiddleware →
|
||||
* `Access-Control-Allow-Origin: *`). The standalone automation service
|
||||
* (`/api/automation/*`) uses a strict origin allowlist instead, so direct
|
||||
* browser requests fail CORS preflight; the same-origin proxy hop avoids
|
||||
* cross-origin entirely while attaching the same auth and `X-Org-Id`
|
||||
* headers server-side.
|
||||
*/
|
||||
forceProxy?: boolean;
|
||||
}
|
||||
|
||||
function buildUpstreamAuthHeaders(
|
||||
@@ -76,10 +64,9 @@ function buildUpstreamAuthHeaders(
|
||||
|
||||
/**
|
||||
* Send a cloud request. App-host calls (`backend.host`) go directly to the
|
||||
* cloud API with the cloud backend's auth headers, unless `forceProxy` is
|
||||
* set. Runtime-sandbox calls pass `hostOverride`, and those still go through
|
||||
* `/api/cloud-proxy` because the per-conversation runtime hosts are not the
|
||||
* configured cloud app origin.
|
||||
* cloud API with the cloud backend's auth headers. Runtime-sandbox calls
|
||||
* pass `hostOverride`, and those go through `/api/cloud-proxy` because the
|
||||
* per-conversation runtime hosts are not the configured cloud app origin.
|
||||
*
|
||||
* App-host auth headers are sent directly to the cloud host. Proxied auth
|
||||
* headers are carried in the proxy envelope and attached server-side.
|
||||
@@ -106,7 +93,7 @@ export async function callCloudProxy<TResponse = unknown>(
|
||||
};
|
||||
const upstreamHost = req.hostOverride ?? req.backend.host;
|
||||
|
||||
if (!req.hostOverride && !req.forceProxy) {
|
||||
if (!req.hostOverride) {
|
||||
const response = await axios.request<TResponse>({
|
||||
url: `${upstreamHost.replace(/\/+$/, "")}${req.path}`,
|
||||
method: req.method,
|
||||
|
||||
Reference in New Issue
Block a user