diff --git a/relay/github.js b/relay/github.js index 23c9bb38c..8cab594f4 100644 --- a/relay/github.js +++ b/relay/github.js @@ -123,6 +123,12 @@ export function createGitHubClient({ clientId, clientSecret, callbackURL, oauthB url = next; continue; } + // GitHub uses 403 for rate limits too; do not cache those as lost repository access. + if (response.status === 403 && (response.headers.get('x-ratelimit-remaining') === '0' + || /^\d+$/u.test(response.headers.get('retry-after') ?? ''))) { + await response.body?.cancel(); + throw new Refused('GITHUB_UNAVAILABLE', 'GitHub is rate limited; wait before retrying'); + } if ([401, 403, 404].includes(response.status)) { await response.body?.cancel(); throw new Refused('NO_REPO_ACCESS', 'GitHub cannot confirm access with this sign-in; sign in again or ask for repository access'); diff --git a/src/api-http.js b/src/api-http.js index 04aeca835..29f21c00c 100644 --- a/src/api-http.js +++ b/src/api-http.js @@ -373,7 +373,7 @@ export function applyStateDelta(state, delta) { /** Choose the same HTTP refusal class for local and sealed relay adapters. */ export function apiStatus(error) { if (error instanceof Refused && error.code === 'ACCOUNT_STORAGE_LIMIT') return 507; - return { RELAY_BOARD_DELETED: 410, AUTH_REQUIRED: 401, DEVICE_REVOKED: 403, TOKEN_BOARD: 403, API_ORIGIN: 403, BAD_ORIGIN: 403, WRITE_REQUIRED: 403, HUMAN_REQUIRED: 403, NO_REPO_ACCESS: 403, NO_BOARD: 404, NO_SHOUT: 404, BOARD_NOT_LINKED: 404, NO_SNAPSHOT: 404, BASELINE_EXISTS: 412, SNAPSHOT_REQUIRED: 409, SEQUENCE_REPEAT: 409, SEQUENCE_GAP: 409 }[error.code] ?? (error instanceof Refused ? 400 : 500); + return { GITHUB_UNAVAILABLE: 503, RELAY_BOARD_DELETED: 410, AUTH_REQUIRED: 401, DEVICE_REVOKED: 403, TOKEN_BOARD: 403, API_ORIGIN: 403, BAD_ORIGIN: 403, WRITE_REQUIRED: 403, HUMAN_REQUIRED: 403, NO_REPO_ACCESS: 403, NO_BOARD: 404, NO_SHOUT: 404, BOARD_NOT_LINKED: 404, NO_SNAPSHOT: 404, BASELINE_EXISTS: 412, SNAPSHOT_REQUIRED: 409, SEQUENCE_REPEAT: 409, SEQUENCE_GAP: 409 }[error.code] ?? (error instanceof Refused ? 400 : 500); } // Relay-only snapshot/delete routes reuse the exact shared bounded-body and refusal boundary. diff --git a/test/relay-fixture.js b/test/relay-fixture.js index 9af901769..c73755c51 100644 --- a/test/relay-fixture.js +++ b/test/relay-fixture.js @@ -51,6 +51,10 @@ export async function githubFixture(t) { for await (const chunk of req) raw += chunk; const form = new URLSearchParams(raw); calls.push({ method: req.method, path: url.pathname, user: isUser, app: isApp, installation: isInstallation, apiVersion: req.headers['x-github-api-version'], formKeys: [...form.keys()] }); + if (state.providerFailure?.path === url.pathname) { + res.writeHead(state.providerFailure.status, { 'Content-Type': 'application/json', ...state.providerFailure.headers }); + return res.end(JSON.stringify({ message: 'fixture provider refusal' })); + } if (url.pathname === '/login/oauth/authorize') { assert.equal(url.searchParams.get('scope'), null); assert.equal(url.searchParams.get('client_id'), 'fixture-app'); diff --git a/test/relay-github.test.js b/test/relay-github.test.js index 910051bb2..6d596d975 100644 --- a/test/relay-github.test.js +++ b/test/relay-github.test.js @@ -72,6 +72,28 @@ test('GitHub credentials never follow a redirect to a different origin [H8, H1]' assert.equal(foreignRequests, 0); }); +test('GitHub rate limits are unavailable, not access denials [H8]', async (t) => { + const provider = await githubFixture(t); + const client = createGitHubClient(provider.config); + const repository = '/repos/fixture/repository'; + const permission = repository + '/collaborators/fixture-user/permission'; + const user = { id: '7', login: 'fixture-user' }; + for (const [headers, code] of [ + [{ 'x-ratelimit-remaining': '0' }, 'GITHUB_UNAVAILABLE'], + [{ 'retry-after': '60', 'x-ratelimit-remaining': '17' }, 'GITHUB_UNAVAILABLE'], + [{ 'x-ratelimit-remaining': '17' }, 'NO_REPO_ACCESS'], + [{ 'retry-after': 'invalid' }, 'NO_REPO_ACCESS'], + ]) { + provider.state.providerFailure = { path: permission, status: 403, headers }; + await assert.rejects(client.access(user, 'fixture/repository'), { code }, JSON.stringify(headers)); + } + // Neither public-repository fallback may turn an outage into permission 'none'. + provider.state.public = true; + for (const path of [repository + '/installation', permission]) { + provider.state.providerFailure = { path, status: 403, headers: { 'retry-after': '60' } }; + await assert.rejects(client.access(user, 'fixture/repository'), { code: 'GITHUB_UNAVAILABLE' }, path); + } +}); test('GitHub roles use role_name, custom-role flags and legacy fallback [H13,H14,H8]', async (t) => { const fixture = await githubFixture(t); diff --git a/test/relay-outbox.test.js b/test/relay-outbox.test.js index d44973598..e6cdc3196 100644 --- a/test/relay-outbox.test.js +++ b/test/relay-outbox.test.js @@ -108,6 +108,37 @@ test('a move on an accepted relay link waits in the ordered outbox while the rel assert.equal(item(there.document).item_status, 'withdrawn'); }); +test('an opted-in outbox retains a move through a GitHub permission outage and replays it once [H8,H16,H21]', async (t) => { + const box = await relayClientFixture(t); + enableOutbox(box); + await box.link(); + await box.cli('status'); + assert.equal(JSON.parse(readFileSync(box.linkFile, 'utf8')).baselineAccepted, true); + const text = 'queued during the provider outage'; + box.providerState.providerFailure = { + path: '/repos/fixture/repository/collaborators/fixture-user/permission', + status: 403, headers: { 'x-ratelimit-remaining': '0' }, + }; + const queued = await box.cli('shout', 'all', text); + assert.equal(queued.code, 0, queued.failure); + const waiting = await box.cli('status'); + assert.equal(waiting.document.relay.outbox.length, 1); + assert.equal(waiting.document.relay.outbox[0].operation, 'shout'); + const id = waiting.document.relay.outbox[0].id; + assert.ok(id); + const again = await box.cli('status'); + assert.equal(again.document.relay.outbox[0].id, id, 'another failed sync retains the original move identity'); + delete box.providerState.providerFailure; + const replayed = await box.cli('status'); + assert.equal(replayed.code, 0, replayed.failure); + assert.equal(replayed.document.relay.outbox, undefined); + assert.equal(replayed.document.relay.outboxRefusals, undefined); + assert.equal((await box.cli('status')).document.relay.outbox, undefined); + const exported = await box.cli('export'); + assert.equal(shoutTexts(exported.document).filter(value => value === text).length, 1, + 'repeated syncs after recovery do not apply the queued move twice'); +}); + test('an item added while the relay is down keeps its queued moves when another machine adds one first [B26,H16,H17]', async (t) => { const { box, other } = await twoMachines(t, 'Added before the outage'); await box.stopRelay(); diff --git a/test/relay.test.js b/test/relay.test.js index 65a8e3f03..e65abaf70 100644 --- a/test/relay.test.js +++ b/test/relay.test.js @@ -731,6 +731,35 @@ test('lost GitHub read access takes effect at ten minutes and before every board assert.deepEqual(await box.auth.boardsFor(signed.token), [], 'a denied write invalidates an earlier allowed read'); }); +test('a provider outage returns 503 without caching a denial or storing the move [H8,H16]', async (t) => { + const box = await fixture(t); + const person = await login(box); + const board = 'b'.repeat(32); + await box.auth.linkBoard(person.token, board, 'fixture/repository'); + const service = await serveRelay({ directory: join(dirname(box.database), 'journal'), auth: box.auth, port: 0, maintenanceMs: 0 }); + t.after(() => service.close()); + const origin = `http://127.0.0.1:${service.port}/api/v1/boards/${board}`; + const headers = { authorization: `Bearer ${person.token}`, 'x-pullboard-engine': String(ENGINE_VERSION), 'content-type': 'application/json' }; + const baseline = await fetch(origin + '/state', { method: 'PUT', headers, body: JSON.stringify({ sequence: 0, sealed: 'AQ' }) }); + assert.equal(baseline.status, 200); + await baseline.arrayBuffer(); + box.state.providerFailure = { path: '/repos/fixture/repository/collaborators/fixture-user/permission', + status: 403, headers: { 'x-ratelimit-remaining': '0' } }; + const move = { method: 'POST', headers, body: JSON.stringify({ sequence: 1, sealed: 'Ag' }) }; + const refused = await fetch(origin + '/moves', move); + assert.equal(refused.status, 503); + assert.equal((await refused.json()).error.code, 'GITHUB_UNAVAILABLE'); + assert.equal((await box.auth.authenticate(person.token, { board })).permission, 'write', 'the valid read cache survives'); + delete box.state.providerFailure; + box.advance(ACCESS_WINDOW_MS); + const events = await fetch(origin + '/events?after=0', { headers }); + assert.equal(events.status, 200, 'recovery is not delayed by a cached denial'); + assert.deepEqual((await events.json()).events, [], 'the refused move was not stored'); + const sent = await fetch(origin + '/moves', move); + assert.equal(sent.status, 200); + await sent.arrayBuffer(); +}); + test('person token management lists only owned scoped metadata and preserves independent revocation [H2,H9]', async (t) => { const box = await fixture(t); const person = await login(box);