Skip to content

Commit e747baa

Browse files
committed
Constrain OpenAPI server variable origins
1 parent 36facc5 commit e747baa

3 files changed

Lines changed: 186 additions & 46 deletions

File tree

packages/plugins/openapi/src/sdk/invoke.ts

Lines changed: 102 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
type MediaBinding,
1313
type OperationParameter,
1414
type ServerInfo,
15+
type ServerVariable,
1516
} from "./types";
1617

1718
// ---------------------------------------------------------------------------
@@ -745,31 +746,82 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* (
745746
});
746747
});
747748

749+
const urlOrigin = (value: string): string | null => {
750+
return URL.canParse(value) ? new URL(value).origin : null;
751+
};
752+
753+
const enumValues = (variable: ServerVariable | undefined): readonly string[] | undefined =>
754+
variable ? Option.getOrUndefined(variable.enum) : undefined;
755+
756+
const validateServerVariableOverrides = (
757+
templateUrl: string,
758+
variables: Record<string, ServerVariable>,
759+
overrides: Record<string, string>,
760+
): Effect.Effect<void, OpenApiInvocationError> =>
761+
Effect.gen(function* () {
762+
const defaultUrl = resolveServerUrl(templateUrl, variables, {});
763+
const defaultOrigin = urlOrigin(defaultUrl);
764+
const resolvedUrl = resolveServerUrl(templateUrl, variables, overrides);
765+
const resolvedOrigin = urlOrigin(resolvedUrl);
766+
767+
for (const [name, value] of Object.entries(overrides)) {
768+
const variable = variables[name];
769+
const allowed = enumValues(variable);
770+
if (allowed && !allowed.includes(value)) {
771+
return yield* new OpenApiInvocationError({
772+
message: `Server variable "${name}" must be one of: ${allowed.join(", ")}`,
773+
statusCode: Option.none(),
774+
});
775+
}
776+
}
777+
778+
if (!defaultOrigin || !resolvedOrigin || defaultOrigin === resolvedOrigin) return;
779+
780+
const unsafe = Object.keys(overrides).filter((name) => {
781+
const variable = variables[name];
782+
if (!variable || enumValues(variable)) return false;
783+
const singleOrigin = urlOrigin(
784+
resolveServerUrl(templateUrl, variables, { [name]: overrides[name]! }),
785+
);
786+
return singleOrigin !== null && singleOrigin !== defaultOrigin;
787+
});
788+
789+
if (unsafe.length > 0) {
790+
return yield* new OpenApiInvocationError({
791+
message: `Server variable override cannot change request origin: ${unsafe.join(", ")}`,
792+
statusCode: Option.none(),
793+
});
794+
}
795+
});
796+
748797
// Connection `baseUrl` wins; otherwise the call's chosen server (`server.url`, or
749798
// the first) resolved with its `{variables}` (call values, else spec defaults).
750799
const resolveRequestHost = (
751800
servers: readonly ServerInfo[],
752801
serverArg: unknown,
753802
baseUrl: string,
754-
): string => {
755-
if (baseUrl) return baseUrl;
756-
if (servers.length === 0) return "";
757-
758-
const arg = (
759-
typeof serverArg === "object" && serverArg !== null && !Array.isArray(serverArg)
760-
? serverArg
761-
: {}
762-
) as { url?: unknown; variables?: unknown };
763-
const chosen = servers.find((server) => server.url === arg.url) ?? servers[0]!;
764-
765-
const overrides: Record<string, string> = {};
766-
if (typeof arg.variables === "object" && arg.variables !== null) {
767-
for (const [name, value] of Object.entries(arg.variables as Record<string, unknown>)) {
768-
if (value != null && value !== "") overrides[name] = String(value);
803+
): Effect.Effect<string, OpenApiInvocationError> =>
804+
Effect.gen(function* () {
805+
if (baseUrl) return baseUrl;
806+
if (servers.length === 0) return "";
807+
808+
const arg = (
809+
typeof serverArg === "object" && serverArg !== null && !Array.isArray(serverArg)
810+
? serverArg
811+
: {}
812+
) as { url?: unknown; variables?: unknown };
813+
const chosen = servers.find((server) => server.url === arg.url) ?? servers[0]!;
814+
815+
const overrides: Record<string, string> = {};
816+
if (typeof arg.variables === "object" && arg.variables !== null) {
817+
for (const [name, value] of Object.entries(arg.variables as Record<string, unknown>)) {
818+
if (value != null && value !== "") overrides[name] = String(value);
819+
}
769820
}
770-
}
771-
return resolveServerUrl(chosen.url, Option.getOrUndefined(chosen.variables), overrides);
772-
};
821+
const variables = Option.getOrUndefined(chosen.variables) ?? {};
822+
yield* validateServerVariableOverrides(chosen.url, variables, overrides);
823+
return resolveServerUrl(chosen.url, variables, overrides);
824+
});
773825

774826
// ---------------------------------------------------------------------------
775827
// Invoke with a provided HttpClient layer + per-call host resolution
@@ -783,32 +835,38 @@ export const invokeWithLayer = (
783835
sourceQueryParams: Record<string, string>,
784836
httpClientLayer: Layer.Layer<HttpClient.HttpClient, never, never>,
785837
) => {
786-
const effectiveBaseUrl = resolveRequestHost(operation.servers ?? [], args.server, baseUrl);
787-
const clientWithBaseUrl = effectiveBaseUrl
788-
? Layer.effect(
789-
HttpClient.HttpClient,
790-
Effect.map(
791-
Effect.service(HttpClient.HttpClient),
792-
HttpClient.mapRequest(HttpClientRequest.prependUrl(effectiveBaseUrl)),
793-
),
794-
).pipe(Layer.provide(httpClientLayer))
795-
: httpClientLayer;
796-
797-
return invoke(operation, args, resolvedHeaders, sourceQueryParams).pipe(
798-
Effect.provide(clientWithBaseUrl),
799-
// `invoke` annotates http.status_code on ITS span (`OpenApi.invoke`,
800-
// via Effect.fn) — annotateCurrentSpan inside it never reaches this
801-
// wrapper span. Stamp the status here too so queries against
802-
// `plugin.openapi.invoke` see the upstream outcome directly.
803-
Effect.tap((result) => Effect.annotateCurrentSpan({ "http.status_code": result.status })),
804-
Effect.withSpan("plugin.openapi.invoke", {
805-
attributes: {
806-
"plugin.openapi.method": operation.method.toUpperCase(),
807-
"plugin.openapi.path_template": operation.pathTemplate,
808-
"plugin.openapi.base_url": effectiveBaseUrl,
809-
},
810-
}),
811-
);
838+
return Effect.gen(function* () {
839+
const effectiveBaseUrl = yield* resolveRequestHost(
840+
operation.servers ?? [],
841+
args.server,
842+
baseUrl,
843+
);
844+
const clientWithBaseUrl = effectiveBaseUrl
845+
? Layer.effect(
846+
HttpClient.HttpClient,
847+
Effect.map(
848+
Effect.service(HttpClient.HttpClient),
849+
HttpClient.mapRequest(HttpClientRequest.prependUrl(effectiveBaseUrl)),
850+
),
851+
).pipe(Layer.provide(httpClientLayer))
852+
: httpClientLayer;
853+
854+
return yield* invoke(operation, args, resolvedHeaders, sourceQueryParams).pipe(
855+
Effect.provide(clientWithBaseUrl),
856+
// `invoke` annotates http.status_code on ITS span (`OpenApi.invoke`,
857+
// via Effect.fn), annotateCurrentSpan inside it never reaches this
858+
// wrapper span. Stamp the status here too so queries against
859+
// `plugin.openapi.invoke` see the upstream outcome directly.
860+
Effect.tap((result) => Effect.annotateCurrentSpan({ "http.status_code": result.status })),
861+
Effect.withSpan("plugin.openapi.invoke", {
862+
attributes: {
863+
"plugin.openapi.method": operation.method.toUpperCase(),
864+
"plugin.openapi.path_template": operation.pathTemplate,
865+
"plugin.openapi.base_url": effectiveBaseUrl,
866+
},
867+
}),
868+
);
869+
});
812870
};
813871

814872
// ---------------------------------------------------------------------------

packages/plugins/openapi/src/sdk/query-serialization.test.ts

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { expect, it } from "@effect/vitest";
2-
import { Effect, Option } from "effect";
2+
import { Cause, Effect, Exit, Option } from "effect";
33
import { FetchHttpClient } from "effect/unstable/http";
44
import { createServer, type Server } from "node:http";
55

66
import { invokeWithLayer } from "./invoke";
7-
import { OperationBinding, OperationParameter, ServerInfo } from "./types";
7+
import { OperationBinding, OperationParameter, ServerInfo, ServerVariable } from "./types";
88

99
const withServer = <A>(
1010
f: (input: { readonly baseUrl: string; readonly requests: string[] }) => Promise<A>,
@@ -221,3 +221,84 @@ it.effect("falls back to the base URL for bindings persisted without servers", (
221221
}),
222222
),
223223
);
224+
225+
it.effect("rejects server variable overrides that move an unbounded request origin", () =>
226+
Effect.gen(function* () {
227+
const operation = OperationBinding.make({
228+
method: "get",
229+
servers: [
230+
ServerInfo.make({
231+
url: "https://{host}/api",
232+
description: Option.none(),
233+
variables: Option.some({
234+
host: ServerVariable.make({
235+
default: "api.example.com",
236+
enum: Option.none(),
237+
description: Option.none(),
238+
}),
239+
}),
240+
}),
241+
],
242+
pathTemplate: "/ping",
243+
requestBody: Option.none(),
244+
responseBody: Option.none(),
245+
parameters: [],
246+
});
247+
248+
const exit = yield* invokeWithLayer(
249+
operation,
250+
{ server: { variables: { host: "169.254.169.254" } } },
251+
"",
252+
{},
253+
{},
254+
FetchHttpClient.layer,
255+
).pipe(Effect.exit);
256+
257+
expect(Exit.isFailure(exit)).toBe(true);
258+
const text = Exit.match(exit, {
259+
onFailure: (cause) => Cause.pretty(cause),
260+
onSuccess: () => "",
261+
});
262+
expect(text).toMatch(/cannot change request origin|host/i);
263+
}),
264+
);
265+
266+
it.effect("allows server variable origin changes when the value is enum-bounded", () =>
267+
Effect.promise(() =>
268+
withServer(async ({ baseUrl, requests }) => {
269+
const operation = OperationBinding.make({
270+
method: "get",
271+
servers: [
272+
ServerInfo.make({
273+
url: "{origin}",
274+
description: Option.none(),
275+
variables: Option.some({
276+
origin: ServerVariable.make({
277+
default: "https://api.example.com",
278+
enum: Option.some(["https://api.example.com", baseUrl]),
279+
description: Option.none(),
280+
}),
281+
}),
282+
}),
283+
],
284+
pathTemplate: "/ping",
285+
requestBody: Option.none(),
286+
responseBody: Option.none(),
287+
parameters: [],
288+
});
289+
290+
await Effect.runPromise(
291+
invokeWithLayer(
292+
operation,
293+
{ server: { variables: { origin: baseUrl } } },
294+
"",
295+
{},
296+
{},
297+
FetchHttpClient.layer,
298+
),
299+
);
300+
301+
expect(new URL(requests[0]!, "http://executor.test").pathname).toBe("/ping");
302+
}),
303+
),
304+
);

packages/plugins/openapi/src/sdk/spec-blob.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ describe("OpenAPI plugin — spec blob storage", () => {
161161
},
162162
template: null,
163163
storage,
164+
httpClientLayer: FetchHttpClient.layer,
164165
getValue: () => Effect.succeed(null),
165166
getValues: () => Effect.succeed({}),
166167
});

0 commit comments

Comments
 (0)