Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions packages/loopover-miner/lib/oauth-device-flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ const ACCESS_TOKEN_URL = "https://github.com/login/oauth/access_token";
const DEFAULT_SCOPE = "repo";
const DEFAULT_EXPIRES_IN_SECONDS = 900;
const DEFAULT_INTERVAL_SECONDS = 5;
// #miner-github-read-timeouts: matches github-token-resolution.js's GITHUB_TOKEN_FETCH_TIMEOUT_MS -- a stalled
// connection can't hang forever, here or anywhere else this package talks to GitHub.
const DEVICE_FLOW_FETCH_TIMEOUT_MS = 10_000;

/** The centrally-held loopover-ams App's OAuth client id -- public (not secret), so it's safe to read from a
* plain env var. Empty/unset means device-flow authorization isn't available in this build/deployment. */
Expand All @@ -40,6 +43,7 @@ export async function requestDeviceCode({ clientId, scope = DEFAULT_SCOPE, fetch
method: "POST",
headers: { accept: "application/json", "content-type": "application/x-www-form-urlencoded" },
body: new URLSearchParams({ client_id: clientId, scope }).toString(),
signal: AbortSignal.timeout(DEVICE_FLOW_FETCH_TIMEOUT_MS),
});
if (!res.ok) throw new DeviceFlowError("device_code_request_failed", `GitHub returned HTTP ${res.status} requesting a device code`);
const data = await res.json();
Expand Down Expand Up @@ -75,15 +79,23 @@ export async function pollForAccessToken({
for (;;) {
if (now() >= deadline) throw new DeviceFlowError("expired_token", "the device code expired before authorization completed");
await sleepFn(interval * 1000);
const res = await fetchFn(ACCESS_TOKEN_URL, {
method: "POST",
headers: { accept: "application/json", "content-type": "application/x-www-form-urlencoded" },
body: new URLSearchParams({
client_id: clientId,
device_code: deviceCode,
grant_type: "urn:ietf:params:oauth:grant-type:device_code",
}).toString(),
});
let res;
try {
res = await fetchFn(ACCESS_TOKEN_URL, {
method: "POST",
headers: { accept: "application/json", "content-type": "application/x-www-form-urlencoded" },
body: new URLSearchParams({
client_id: clientId,
device_code: deviceCode,
grant_type: "urn:ietf:params:oauth:grant-type:device_code",
}).toString(),
signal: AbortSignal.timeout(DEVICE_FLOW_FETCH_TIMEOUT_MS),
});
} catch {
// A stalled/timed-out attempt is a per-attempt failure, not a fatal one -- the existing deadline check
// at the top of the loop still bounds total polling time, so this just costs one wasted interval.
continue;
}
const data = await res.json().catch(() => ({}));
if (data && typeof data.access_token === "string" && data.access_token) {
return { accessToken: data.access_token, scope: typeof data.scope === "string" ? data.scope : "" };
Expand Down
36 changes: 36 additions & 0 deletions test/unit/miner-oauth-device-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ describe("requestDeviceCode (#5682)", () => {
await expect(requestDeviceCode({ clientId: "c", fetchFn })).rejects.toMatchObject({ code: "device_code_request_failed" });
});

it("REGRESSION (#6988): bounds the fetch with a request timeout so a stalled connection can't hang forever", async () => {
const fetchFn = vi.fn().mockResolvedValue(jsonResponse({ device_code: "dc1", user_code: "u", verification_uri: "v" }));
await requestDeviceCode({ clientId: "c", fetchFn });
const [, init] = fetchFn.mock.calls[0] as [string, RequestInit];
expect(init.signal).toBeInstanceOf(AbortSignal);
});

it("throws device_code_response_invalid when a required field is missing", async () => {
const fetchFn = vi.fn().mockResolvedValue(jsonResponse({ device_code: "dc1" }));
await expect(requestDeviceCode({ clientId: "c", fetchFn })).rejects.toMatchObject({ code: "device_code_response_invalid" });
Expand Down Expand Up @@ -112,6 +119,35 @@ describe("pollForAccessToken (#5682)", () => {
expect(result.scope).toBe("");
});

it("REGRESSION (#6988): bounds each poll's fetch with a request timeout so a stalled connection can't hang forever", async () => {
const fetchFn = vi.fn().mockResolvedValue(jsonResponse({ access_token: "gho_xyz" }));
await pollForAccessToken({ clientId: "c", deviceCode: "dc1", fetchFn, sleepFn: noSleep });
const [, init] = fetchFn.mock.calls[0] as [string, RequestInit];
expect(init.signal).toBeInstanceOf(AbortSignal);
});

it("REGRESSION (#6988): a timed-out/rejected fetch is a per-attempt failure that still retries, not an unhandled crash", async () => {
const fetchFn = vi
.fn()
.mockRejectedValueOnce(new DOMException("The operation was aborted", "TimeoutError"))
.mockResolvedValueOnce(jsonResponse({ access_token: "gho_after_timeout" }));
const result = await pollForAccessToken({ clientId: "c", deviceCode: "dc1", fetchFn, sleepFn: noSleep });
expect(result.accessToken).toBe("gho_after_timeout");
expect(fetchFn).toHaveBeenCalledTimes(2);
});

it("REGRESSION (#6988): a permanently stalled connection still respects the deadline instead of hanging forever", async () => {
let clock = 0;
const now = () => clock;
const sleepFn = vi.fn().mockImplementation(async (ms: number) => {
clock += ms;
});
const fetchFn = vi.fn().mockRejectedValue(new DOMException("The operation was aborted", "TimeoutError"));
await expect(
pollForAccessToken({ clientId: "c", deviceCode: "dc1", intervalSeconds: 5, expiresInSeconds: 12, fetchFn, sleepFn, now }),
).rejects.toMatchObject({ code: "expired_token" });
});

it("keeps polling on authorization_pending until a token is granted", async () => {
const fetchFn = vi
.fn()
Expand Down