Skip to content

Commit 1366373

Browse files
mack-erelclaude
andcommitted
fix(hyperdrive): seed remote bindings on every dev path and stop leaking bridges
Addresses three issues raised in review: - Seeding ran only in `LocalRuntimeController`, so `getPlatformProxy()`, the Vite plugin and multi-worker dev built the binding with placeholder credentials (and a `mysql` scheme even for Postgres) and failed to authenticate at the edge, silently. Seeding now happens inside `buildMiniflareOptions` and `getMiniflareOptionsFromConfig`, covering every async dev path; `buildMiniflareBindingOptions` takes the seeded values as an argument and warns when they are absent, so the remaining synchronous entry point fails visibly rather than silently. - `seedRemoteHyperdriveBindings` mutated binding objects that are shared by reference with the record the remote proxy session keeps for change detection, so every hot reload compared unequal and tore down the session. It now returns the seeded values instead of mutating. - The remote TCP bridge was re-registered under the same key on each reload without closing the previous listener, leaking it and its live edge relays for the session's lifetime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015GDyTfEa62t3bSe5wWn8Mt
1 parent 3103e01 commit 1366373

6 files changed

Lines changed: 110 additions & 35 deletions

File tree

packages/miniflare/src/plugins/hyperdrive/hyperdrive-proxy.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,10 @@ export class HyperdriveProxyController {
275275
}
276276
});
277277
});
278+
// `getServices` re-runs on every `setOptions` (i.e. every dev reload), so
279+
// close the bridge from the previous round before replacing it — otherwise
280+
// the old listener and its live edge relays leak for the session's lifetime.
281+
this.#servers.get(`remote:${name}`)?.close();
278282
this.#servers.set(`remote:${name}`, server);
279283
return port;
280284
}

packages/remote-bindings/src/seed-hyperdrive-bindings.ts

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,22 +44,32 @@ async function fetchEdgeConnectionString(
4444
}
4545

4646
/**
47-
* Seeds every remote Hyperdrive binding's `localConnectionString` with the
48-
* connection string of its edge session, so that the local binding presents
49-
* credentials the edge Hyperdrive proxy will accept.
47+
* Fetches the edge session's connection string for every remote Hyperdrive
48+
* binding, so that the local binding can present credentials the edge
49+
* Hyperdrive proxy will accept.
5050
*
5151
* `buildMiniflareBindingOptions` is synchronous, so this async step must run
5252
* once the remote proxy session is ready and before miniflare options are
53-
* built. The passed `bindings` objects are mutated in place.
53+
* built.
5454
*
55-
* No-op when there is no remote proxy session or no remote Hyperdrive bindings.
55+
* The seeded values are *returned* rather than written onto the passed
56+
* `bindings`: those binding objects are shared by reference with the record the
57+
* remote proxy session keeps for change detection (`pickRemoteBindings` copies
58+
* the record, not the objects). Mutating them would make the next reload's
59+
* freshly-loaded config compare unequal to the stored, seeded one, tearing down
60+
* and re-establishing the remote session on every file save.
61+
*
62+
* Returns an empty map when there is no remote proxy session or no remote
63+
* Hyperdrive bindings.
5664
*/
5765
export async function seedRemoteHyperdriveBindings(
5866
bindings: Record<string, Binding> | undefined,
5967
remoteProxyConnectionString: RemoteProxyConnectionString | undefined
60-
): Promise<void> {
68+
): Promise<Map<string, string>> {
69+
const seeded = new Map<string, string>();
70+
6171
if (!remoteProxyConnectionString || !bindings) {
62-
return;
72+
return seeded;
6373
}
6474

6575
const remoteHyperdrives = Object.entries(bindings).filter(
@@ -74,13 +84,14 @@ export async function seedRemoteHyperdriveBindings(
7484
);
7585

7686
await Promise.all(
77-
remoteHyperdrives.map(async ([name, binding]) => {
87+
remoteHyperdrives.map(async ([name]) => {
7888
const connectionString = await fetchEdgeConnectionString(
7989
remoteProxyConnectionString,
8090
name
8191
);
82-
(binding as { localConnectionString?: string }).localConnectionString =
83-
connectionString;
92+
seeded.set(name, connectionString);
8493
})
8594
);
95+
96+
return seeded;
8697
}

packages/wrangler/src/__tests__/dev/miniflare-hyperdrive.test.ts

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ const remoteProxyConnectionString = new URL(
1010

1111
function buildHyperdriveOptions(
1212
binding: Extract<Binding, { type: "hyperdrive" }>,
13-
connectionString?: RemoteProxyConnectionString
13+
connectionString?: RemoteProxyConnectionString,
14+
seededConnectionStrings?: ReadonlyMap<string, string>
1415
) {
1516
const { bindingOptions } = buildMiniflareBindingOptions(
1617
{
@@ -26,11 +27,14 @@ function buildHyperdriveOptions(
2627
containerBuildId: undefined,
2728
enableContainers: false,
2829
},
29-
connectionString
30+
connectionString,
31+
seededConnectionStrings
3032
);
3133
return bindingOptions.hyperdrives;
3234
}
3335

36+
const SEEDED_CONNECTION_STRING = "postgres://edge-user:edge-pass@edge:5432/db";
37+
3438
describe("hyperdrive bindings in local dev", () => {
3539
const std = mockConsoleMethods();
3640

@@ -47,9 +51,35 @@ describe("hyperdrive bindings in local dev", () => {
4751
expect(std.warn).toBe("");
4852
});
4953

50-
it("hands miniflare the remote proxy connection string for a remote binding", ({
54+
it("hands miniflare the remote proxy connection string and the seeded edge credentials for a remote binding", ({
5155
expect,
5256
}) => {
57+
expect(
58+
buildHyperdriveOptions(
59+
{
60+
type: "hyperdrive",
61+
id: "hyperdrive-id",
62+
remote: true,
63+
},
64+
remoteProxyConnectionString,
65+
new Map([["HYPERDRIVE", SEEDED_CONNECTION_STRING]])
66+
)
67+
).toEqual({
68+
HYPERDRIVE: {
69+
localConnectionString: SEEDED_CONNECTION_STRING,
70+
remoteProxyConnectionString,
71+
},
72+
});
73+
expect(std.warn).toBe("");
74+
});
75+
76+
it("warns when a remote binding's edge credentials could not be seeded", ({
77+
expect,
78+
}) => {
79+
// Reached from dev entry points that cannot seed (the binding builder is
80+
// synchronous), e.g. `unstable_getMiniflareWorkerOptions`. Without the
81+
// warning the binding would silently fall back to placeholder credentials
82+
// and fail to authenticate at the edge.
5383
expect(
5484
buildHyperdriveOptions(
5585
{
@@ -65,7 +95,9 @@ describe("hyperdrive bindings in local dev", () => {
6595
remoteProxyConnectionString,
6696
},
6797
});
68-
expect(std.warn).toBe("");
98+
expect(std.warn).toContain(
99+
`The Hyperdrive binding "HYPERDRIVE" is configured with "remote": true, but its edge credentials could not be seeded in this context`
100+
);
69101
});
70102

71103
it("explains how to fix a remote binding that has neither a session nor a local database", ({

packages/wrangler/src/api/integrations/platform/index.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ import { logger } from "../../../logger";
2121
import { getSiteAssetPaths } from "../../../sites";
2222
import { dedent } from "../../../utils/dedent";
2323
import { getZoneFromRoute } from "../../../zones";
24-
import { maybeStartOrUpdateRemoteProxySession } from "../../remoteBindings";
24+
import {
25+
maybeStartOrUpdateRemoteProxySession,
26+
seedRemoteHyperdriveBindings,
27+
} from "../../remoteBindings";
2528
import { CacheStorage } from "./caches";
2629
import { ExecutionContext } from "./executionContext";
2730
// TODO: import from `@cloudflare/workers-utils` after migrating to `tsdown`
@@ -297,7 +300,10 @@ async function getMiniflareOptionsFromConfig(args: {
297300
containerBuildId: undefined,
298301
enableContainers: config.dev.enable_containers,
299302
},
300-
remoteProxyConnectionString
303+
remoteProxyConnectionString,
304+
// Remote Hyperdrive bindings authenticate with the edge session's
305+
// credentials, which have to be fetched before this synchronous builder.
306+
await seedRemoteHyperdriveBindings(bindings, remoteProxyConnectionString)
301307
);
302308

303309
let processedAssetOptions: AssetsOptions | undefined;

packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -310,11 +310,8 @@ export class LocalRuntimeController extends RuntimeController {
310310
if (data.config.dev?.remote !== false) {
311311
// note: remote bindings use (transitively) LocalRuntimeController, so we need to import
312312
// from the module lazily in order to avoid circular dependency issues
313-
const {
314-
maybeStartOrUpdateRemoteProxySession,
315-
pickRemoteBindings,
316-
seedRemoteHyperdriveBindings,
317-
} = await import("../remoteBindings");
313+
const { maybeStartOrUpdateRemoteProxySession, pickRemoteBindings } =
314+
await import("../remoteBindings");
318315

319316
const remoteBindings = pickRemoteBindings(configBundle.bindings ?? {});
320317

@@ -331,15 +328,6 @@ export class LocalRuntimeController extends RuntimeController {
331328
undefined
332329
: data.config.dev.auth
333330
);
334-
335-
// Remote Hyperdrive bindings need the edge session's connection
336-
// string seeded into their local config before miniflare options are
337-
// built (that step is synchronous). No-op unless a remote proxy
338-
// session is running and there are remote Hyperdrive bindings.
339-
await seedRemoteHyperdriveBindings(
340-
configBundle.bindings ?? undefined,
341-
this.#remoteProxySessionData?.session?.remoteProxyConnectionString
342-
);
343331
}
344332

345333
// Bail out if a newer bundle arrived while we were setting up

packages/wrangler/src/dev/miniflare/index.ts

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,8 @@ function pipelineEntry(
343343
}
344344
function hyperdriveEntry(
345345
hyperdrive: CfHyperdrive,
346-
remoteProxyConnectionString?: RemoteProxyConnectionString
346+
remoteProxyConnectionString?: RemoteProxyConnectionString,
347+
seededConnectionStrings?: ReadonlyMap<string, string>
347348
):
348349
| [string, string]
349350
| [
@@ -360,10 +361,25 @@ function hyperdriveEntry(
360361
// `connectionString`; miniflare uses `remoteProxyConnectionString` to stand
361362
// up the local TCP bridge.
362363
if (hyperdrive.remote && remoteProxyConnectionString) {
364+
const seededConnectionString = seededConnectionStrings?.get(
365+
hyperdrive.binding
366+
);
367+
if (seededConnectionString === undefined) {
368+
// Seeding is what makes a database client authenticate through the edge
369+
// proxy; without it miniflare falls back to placeholder credentials (and
370+
// a `mysql` scheme even for Postgres) and the login is rejected. Warn
371+
// rather than throw, so dev paths that cannot seed — the entry points
372+
// below are synchronous — stay usable and fail visibly instead of
373+
// silently.
374+
logger.once.warn(
375+
`The Hyperdrive binding "${hyperdrive.binding}" is configured with "remote": true, but its edge credentials could not be seeded in this context, so connections through it will likely fail to authenticate. Remote Hyperdrive bindings are currently supported in \`wrangler dev\` and \`getPlatformProxy()\`.`
376+
);
377+
}
363378
return [
364379
hyperdrive.binding,
365380
{
366-
localConnectionString: hyperdrive.localConnectionString,
381+
localConnectionString:
382+
seededConnectionString ?? hyperdrive.localConnectionString,
367383
remoteProxyConnectionString,
368384
},
369385
];
@@ -560,7 +576,11 @@ type MiniflareBindingsConfig = Pick<
560576
// each plugin options schema and use those
561577
export function buildMiniflareBindingOptions(
562578
config: MiniflareBindingsConfig,
563-
remoteProxyConnectionString: RemoteProxyConnectionString | undefined
579+
remoteProxyConnectionString: RemoteProxyConnectionString | undefined,
580+
// Edge connection strings for remote Hyperdrive bindings, keyed by binding
581+
// name (see `seedRemoteHyperdriveBindings`). Fetching them is async, so
582+
// callers seed before calling this synchronous builder.
583+
seededHyperdriveConnectionStrings?: ReadonlyMap<string, string>
564584
): {
565585
bindingOptions: WorkerOptionsBindings;
566586
externalWorkers: V4WorkerOptions[];
@@ -931,7 +951,11 @@ export function buildMiniflareBindingOptions(
931951
),
932952
hyperdrives: Object.fromEntries(
933953
hyperdrives.map((hyperdrive) =>
934-
hyperdriveEntry(hyperdrive, remoteProxyConnectionString)
954+
hyperdriveEntry(
955+
hyperdrive,
956+
remoteProxyConnectionString,
957+
seededHyperdriveConnectionStrings
958+
)
935959
)
936960
),
937961
analyticsEngineDatasets: Object.fromEntries(
@@ -1197,9 +1221,19 @@ export async function buildMiniflareOptions(
11971221
: undefined;
11981222

11991223
const { sourceOptions } = await buildSourceOptions(config);
1224+
// Remote Hyperdrive bindings need the edge session's credentials before the
1225+
// (synchronous) binding builder runs. Doing it here covers every dev path
1226+
// that goes through this function.
1227+
const { seedRemoteHyperdriveBindings } =
1228+
await import("../../api/remoteBindings");
1229+
const seededHyperdriveConnectionStrings = await seedRemoteHyperdriveBindings(
1230+
config.bindings ?? undefined,
1231+
remoteProxyConnectionString
1232+
);
12001233
const { bindingOptions, externalWorkers } = buildMiniflareBindingOptions(
12011234
config,
1202-
remoteProxyConnectionString
1235+
remoteProxyConnectionString,
1236+
seededHyperdriveConnectionStrings
12031237
);
12041238
if (bindingOptions.browserRendering && getBrowserRenderingHeadfulFromEnv()) {
12051239
bindingOptions.browserRendering.headful = true;

0 commit comments

Comments
 (0)