Skip to content

Commit cd9aedf

Browse files
authored
Merge pull request #7048 from galuis116/fix/oauth-device-flow-fetch-timeout
fix(miner): bound oauth-device-flow.js's GitHub fetches with a request timeout
2 parents fe49ac8 + 77ca20f commit cd9aedf

2 files changed

Lines changed: 57 additions & 9 deletions

File tree

packages/loopover-miner/lib/oauth-device-flow.js

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ const ACCESS_TOKEN_URL = "https://github.com/login/oauth/access_token";
1515
const DEFAULT_SCOPE = "repo";
1616
const DEFAULT_EXPIRES_IN_SECONDS = 900;
1717
const DEFAULT_INTERVAL_SECONDS = 5;
18+
// #miner-github-read-timeouts: matches github-token-resolution.js's GITHUB_TOKEN_FETCH_TIMEOUT_MS -- a stalled
19+
// connection can't hang forever, here or anywhere else this package talks to GitHub.
20+
const DEVICE_FLOW_FETCH_TIMEOUT_MS = 10_000;
1821

1922
/** The centrally-held loopover-ams App's OAuth client id -- public (not secret), so it's safe to read from a
2023
* plain env var. Empty/unset means device-flow authorization isn't available in this build/deployment. */
@@ -40,6 +43,7 @@ export async function requestDeviceCode({ clientId, scope = DEFAULT_SCOPE, fetch
4043
method: "POST",
4144
headers: { accept: "application/json", "content-type": "application/x-www-form-urlencoded" },
4245
body: new URLSearchParams({ client_id: clientId, scope }).toString(),
46+
signal: AbortSignal.timeout(DEVICE_FLOW_FETCH_TIMEOUT_MS),
4347
});
4448
if (!res.ok) throw new DeviceFlowError("device_code_request_failed", `GitHub returned HTTP ${res.status} requesting a device code`);
4549
const data = await res.json();
@@ -75,15 +79,23 @@ export async function pollForAccessToken({
7579
for (;;) {
7680
if (now() >= deadline) throw new DeviceFlowError("expired_token", "the device code expired before authorization completed");
7781
await sleepFn(interval * 1000);
78-
const res = await fetchFn(ACCESS_TOKEN_URL, {
79-
method: "POST",
80-
headers: { accept: "application/json", "content-type": "application/x-www-form-urlencoded" },
81-
body: new URLSearchParams({
82-
client_id: clientId,
83-
device_code: deviceCode,
84-
grant_type: "urn:ietf:params:oauth:grant-type:device_code",
85-
}).toString(),
86-
});
82+
let res;
83+
try {
84+
res = await fetchFn(ACCESS_TOKEN_URL, {
85+
method: "POST",
86+
headers: { accept: "application/json", "content-type": "application/x-www-form-urlencoded" },
87+
body: new URLSearchParams({
88+
client_id: clientId,
89+
device_code: deviceCode,
90+
grant_type: "urn:ietf:params:oauth:grant-type:device_code",
91+
}).toString(),
92+
signal: AbortSignal.timeout(DEVICE_FLOW_FETCH_TIMEOUT_MS),
93+
});
94+
} catch {
95+
// A stalled/timed-out attempt is a per-attempt failure, not a fatal one -- the existing deadline check
96+
// at the top of the loop still bounds total polling time, so this just costs one wasted interval.
97+
continue;
98+
}
8799
const data = await res.json().catch(() => ({}));
88100
if (data && typeof data.access_token === "string" && data.access_token) {
89101
return { accessToken: data.access_token, scope: typeof data.scope === "string" ? data.scope : "" };

test/unit/miner-oauth-device-flow.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,13 @@ describe("requestDeviceCode (#5682)", () => {
6969
await expect(requestDeviceCode({ clientId: "c", fetchFn })).rejects.toMatchObject({ code: "device_code_request_failed" });
7070
});
7171

72+
it("REGRESSION (#6988): bounds the fetch with a request timeout so a stalled connection can't hang forever", async () => {
73+
const fetchFn = vi.fn().mockResolvedValue(jsonResponse({ device_code: "dc1", user_code: "u", verification_uri: "v" }));
74+
await requestDeviceCode({ clientId: "c", fetchFn });
75+
const [, init] = fetchFn.mock.calls[0] as [string, RequestInit];
76+
expect(init.signal).toBeInstanceOf(AbortSignal);
77+
});
78+
7279
it("throws device_code_response_invalid when a required field is missing", async () => {
7380
const fetchFn = vi.fn().mockResolvedValue(jsonResponse({ device_code: "dc1" }));
7481
await expect(requestDeviceCode({ clientId: "c", fetchFn })).rejects.toMatchObject({ code: "device_code_response_invalid" });
@@ -112,6 +119,35 @@ describe("pollForAccessToken (#5682)", () => {
112119
expect(result.scope).toBe("");
113120
});
114121

122+
it("REGRESSION (#6988): bounds each poll's fetch with a request timeout so a stalled connection can't hang forever", async () => {
123+
const fetchFn = vi.fn().mockResolvedValue(jsonResponse({ access_token: "gho_xyz" }));
124+
await pollForAccessToken({ clientId: "c", deviceCode: "dc1", fetchFn, sleepFn: noSleep });
125+
const [, init] = fetchFn.mock.calls[0] as [string, RequestInit];
126+
expect(init.signal).toBeInstanceOf(AbortSignal);
127+
});
128+
129+
it("REGRESSION (#6988): a timed-out/rejected fetch is a per-attempt failure that still retries, not an unhandled crash", async () => {
130+
const fetchFn = vi
131+
.fn()
132+
.mockRejectedValueOnce(new DOMException("The operation was aborted", "TimeoutError"))
133+
.mockResolvedValueOnce(jsonResponse({ access_token: "gho_after_timeout" }));
134+
const result = await pollForAccessToken({ clientId: "c", deviceCode: "dc1", fetchFn, sleepFn: noSleep });
135+
expect(result.accessToken).toBe("gho_after_timeout");
136+
expect(fetchFn).toHaveBeenCalledTimes(2);
137+
});
138+
139+
it("REGRESSION (#6988): a permanently stalled connection still respects the deadline instead of hanging forever", async () => {
140+
let clock = 0;
141+
const now = () => clock;
142+
const sleepFn = vi.fn().mockImplementation(async (ms: number) => {
143+
clock += ms;
144+
});
145+
const fetchFn = vi.fn().mockRejectedValue(new DOMException("The operation was aborted", "TimeoutError"));
146+
await expect(
147+
pollForAccessToken({ clientId: "c", deviceCode: "dc1", intervalSeconds: 5, expiresInSeconds: 12, fetchFn, sleepFn, now }),
148+
).rejects.toMatchObject({ code: "expired_token" });
149+
});
150+
115151
it("keeps polling on authorization_pending until a token is granted", async () => {
116152
const fetchFn = vi
117153
.fn()

0 commit comments

Comments
 (0)