diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 458bb67e0a..261aece1d1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -67,6 +67,14 @@ jobs: bump-dev-version: needs: publish if: ${{ inputs.dry-run != true }} + # A reusable-workflow CALL cannot grant the callee more than the calling job holds, + # and GitHub refuses the whole run at startup when the called workflow's own job + # declares permissions the caller did not pass down ("startup_failure", runs + # 33615174183 / 33615177849 — the first dispatches since #3129 wired this call). + # The callee's job declares exactly these two; nothing else in this file gains them. + permissions: + contents: write + pull-requests: write uses: ./.github/workflows/dev-version-bump.yml with: released-version: v${{ inputs.version }} diff --git a/src/client/hub-client.ts b/src/client/hub-client.ts index 524d9292ef..88a9082a78 100644 --- a/src/client/hub-client.ts +++ b/src/client/hub-client.ts @@ -1,14 +1,8 @@ import { MAX_REMOTE_CATALOG_BYTES } from "../server/catalog-download"; import { readBoundedResponseBytes } from "../lib/bounded-body"; -/** - * A pairing grant may cross loopback or authenticated HTTPS, and nothing else. - * - * Mirrors the hub-side rule in src/server/gui-session.ts. Checking here too is not - * redundant: it keeps the client from spending a single-use code on a request the hub is - * certain to refuse. - */ -function isPairingTransportPermitted(origin: string): boolean { +/** Hub traffic may cross loopback or authenticated HTTPS, and nothing else. */ +function isHubTransportPermitted(origin: string): boolean { let url: URL; try { url = new URL(origin); @@ -171,6 +165,12 @@ export function normalizeHubOrigin(input: string): string { "Hub URL must be an HTTP(S) origin without credentials, query, fragment, or non-/v1 path", ); } + if (!isHubTransportPermitted(parsed.origin)) { + throw new HubClientError( + "insecure_http_refused", + "Hub URLs require loopback or HTTPS; plaintext remote HTTP is not permitted", + ); + } return parsed.origin; } @@ -230,7 +230,7 @@ export async function exchangeConnectPairingGrant( // Deliberateness is not the control that matters: the grant is readable by anything on the // path and the session it mints is reusable. The hub refuses this exchange outright now, so // sending it would only burn a single-use code against a certain rejection. - if (!isPairingTransportPermitted(origin)) { + if (!isHubTransportPermitted(origin)) { throw new HubClientError("insecure_http_refused", "Pairing requires loopback or HTTPS; plaintext HTTP cannot carry a grant"); } const response = await fetchBounded(options.fetchImpl ?? fetch, `${origin}/opencodex-session`, { diff --git a/tests/client-connect.test.ts b/tests/client-connect.test.ts index 313e2d1d1b..422b6d6472 100644 --- a/tests/client-connect.test.ts +++ b/tests/client-connect.test.ts @@ -59,8 +59,12 @@ describe("remote hub client boundary", () => { test("canonicalizes origin and terminal /v1 only", () => { expect(normalizeHubOrigin("https://hub.example.test/v1")).toBe("https://hub.example.test"); expect(normalizeHubOrigin("https://hub.example.test/v1/")).toBe("https://hub.example.test"); + expect(normalizeHubOrigin("http://localhost:10100/v1")).toBe("http://localhost:10100"); + expect(normalizeHubOrigin("http://127.0.0.1:10100")).toBe("http://127.0.0.1:10100"); + expect(normalizeHubOrigin("http://[::1]:10100")).toBe("http://[::1]:10100"); for (const value of [ "ftp://hub.example.test", + "http://hub.example.test", "https://user@hub.example.test", "https://hub.example.test/private", "https://hub.example.test/?secret=1", @@ -85,9 +89,17 @@ describe("remote hub client boundary", () => { } }); + test("rejects plaintext remote discovery before sending a request", async () => { + let calls = 0; + await expect(fetchHubReady("http://hub.example.test", { + fetchImpl: async () => { calls += 1; return Response.json(readyBody()); }, + })).rejects.toThrow("plaintext remote HTTP is not permitted"); + expect(calls).toBe(0); + }); + test("admin key issuance is HTTPS-only and pairing exchanges into a full GUI session", async () => { let calls = 0; - await expect(issueClientKey("http://hub.example.test", { + await expect(issueClientKey("http://localhost:10100", { kind: "admin", value: new TextEncoder().encode("ocx_admin_secret"), }, "client", {