fix: anchor the https scheme downgrade in containerFetch

[mattjohnsonpint]

Note

Stacked on #249. Until that merges, the diff here also shows the two formatting commits; it will reduce to the single fix commit after a rebase. Remove this note then.

Summary

• anchor the scheme rewrite so it only matches the URL's scheme
• add regression coverage for https: in query strings and fragments
• pin the intended scheme downgrade so it can't be dropped by a future fix
• publish as a patch release

Why

containerFetch downgrades https to http before forwarding, because tcpPort.fetch opens a raw TCP connection to the container and nothing terminates TLS. That rewrite used an unanchored string replace:

const containerUrl = request.url.replace('https:', 'http:');

String.prototype.replace with a string pattern replaces only the first occurrence, and the match isn't anchored to the start. When the URL's scheme is already http:, the first https: in the string is somewhere in the path, query, or fragment — and that gets rewritten instead of the scheme.

So this:

await this.containerFetch('/callback?redirect=https://app.example.com');

arrives at the container as:

http://container/callback?redirect=http://app.example.com
                                        ^ silently downgraded

Any endpoint taking an unencoded absolute URL as a parameter is affected — OAuth redirect_uri, webhook callbacks, proxy targets, SSO RelayState. Percent-encoded values (https%3A%2F%2F…) are unaffected, since they don't contain the literal https:.

Absolute https:// URLs were never affected: the first match is the scheme, so the replacement did the right thing and the rest of the URL survived.

Relationship to #238

The bug is pre-existing, but #238 widened its reach considerably. Relative paths now resolve against http://container, which means every relative-path call has an http: scheme — precisely the condition that triggers this. Before #238, relative paths threw outright, so the corruption was largely unreachable.

Approach

Anchoring the regex is the whole fix. I also considered operating on the parsed URL:

const target = new URL(request.url);
if (target.protocol === 'https:') target.protocol = 'http:';

new URL(request.url).href round-trips request.url exactly (verified across default ports, empty paths, and percent-encoded characters), so the two are equivalent. The anchored regex wins on being a one-token change with no normalization risk and no extra allocation in the request-proxy hot path.

The comment above the line explains why the anchor matters, since the obvious "simplification" back to a string argument reintroduces the bug silently.

Tests

Two cases in src/tests/container.test.ts:

• preserves https: in query strings and fragments — the regression. Fails on main, passes here.
• downgrades only the scheme of an https URL — asserts the intended downgrade still happens and doesn't touch a https: query param on the same URL. This case was already correct; it's pinned so an over-eager future fix can't disable the downgrade entirely.

Only the first fails without the change, which is expected — the second documents behavior that already worked.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@cloudflare/containers@250

commit: 617c08f

[ask-bonk]

LGTM

github run