Skip to content

Commit f46a78f

Browse files
committed
Constrain OpenAPI server variable origins
1 parent 12d79c6 commit f46a78f

3 files changed

Lines changed: 190 additions & 46 deletions

File tree

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

Lines changed: 106 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,86 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* (
745746
});
746747
});
747748

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

774830
// ---------------------------------------------------------------------------
775831
// Invoke with a provided HttpClient layer + per-call host resolution
@@ -783,32 +839,38 @@ export const invokeWithLayer = (
783839
sourceQueryParams: Record<string, string>,
784840
httpClientLayer: Layer.Layer<HttpClient.HttpClient, never, never>,
785841
) => {
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-
);
842+
return Effect.gen(function* () {
843+
const effectiveBaseUrl = yield* resolveRequestHost(
844+
operation.servers ?? [],
845+
args.server,
846+
baseUrl,
847+
);
848+
const clientWithBaseUrl = effectiveBaseUrl
849+
? Layer.effect(
850+
HttpClient.HttpClient,
851+
Effect.map(
852+
Effect.service(HttpClient.HttpClient),
853+
HttpClient.mapRequest(HttpClientRequest.prependUrl(effectiveBaseUrl)),
854+
),
855+
).pipe(Layer.provide(httpClientLayer))
856+
: httpClientLayer;
857+
858+
return yield* invoke(operation, args, resolvedHeaders, sourceQueryParams).pipe(
859+
Effect.provide(clientWithBaseUrl),
860+
// `invoke` annotates http.status_code on ITS span (`OpenApi.invoke`,
861+
// via Effect.fn), annotateCurrentSpan inside it never reaches this
862+
// wrapper span. Stamp the status here too so queries against
863+
// `plugin.openapi.invoke` see the upstream outcome directly.
864+
Effect.tap((result) => Effect.annotateCurrentSpan({ "http.status_code": result.status })),
865+
Effect.withSpan("plugin.openapi.invoke", {
866+
attributes: {
867+
"plugin.openapi.method": operation.method.toUpperCase(),
868+
"plugin.openapi.path_template": operation.pathTemplate,
869+
"plugin.openapi.base_url": effectiveBaseUrl,
870+
},
871+
}),
872+
);
873+
});
812874
};
813875

814876
// ---------------------------------------------------------------------------

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)