From bdb1e8735e318ab4e465e24032511c13cb22454c Mon Sep 17 00:00:00 2001 From: McHersheys <285552078+McHersheys@users.noreply.github.com> Date: Sun, 25 Oct 2026 10:15:25 +0000 Subject: [PATCH 1/3] fix(relay): distinguish provider outages from access denials [H8,H16] [skip ci] Draft contribution: focused regressions and offline contract note. Product tests and gate are unrun; no runtime adoption or permission/ACK bypass. --- docs/rfcs/0005-relay-resilience.md | 96 ++++++++++++++++++++++++++++++ relay/github.js | 6 ++ src/api-http.js | 2 +- test/relay-fixture.js | 4 ++ test/relay-outbox.test.js | 31 ++++++++++ test/relay-provider-outage.test.js | 91 ++++++++++++++++++++++++++++ 6 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 docs/rfcs/0005-relay-resilience.md create mode 100644 test/relay-provider-outage.test.js diff --git a/docs/rfcs/0005-relay-resilience.md b/docs/rfcs/0005-relay-resilience.md new file mode 100644 index 000000000..56c29a655 --- /dev/null +++ b/docs/rfcs/0005-relay-resilience.md @@ -0,0 +1,96 @@ +# RFC 0005: Relay provider failures and offline claim boundaries + +- Status: draft; not a change to approved rows or permission to deploy +- Related: [issue #11](https://github.com/pullboard-dev/pullboard/issues/11) +- Rows: H3, H8, H10, H16; H21 remains draft + +## Small first increment + +Treat GitHub 403 responses with `x-ratelimit-remaining: 0` or an integer +`retry-after` as `GITHUB_UNAVAILABLE`, as for 429. Keep ordinary 401/403/404 +refusals unchanged. Return HTTP 503 for this code on the sealed API, matching +the auth API. A failed current permission check still authorizes no remote +write and appends no event. It must not poison the read-permission cache. + +This is error classification, not rate-limit prevention or an offline authority +implementation. It does not recognize every headerless secondary-limit response, +propagate a retry deadline, add backoff, or prove a hosted incident's cause. +A follow-up must honor provider reset/Retry-After and use bounded backoff without +turning an unavailable provider into an authorization success. + +## Reuse the existing outbox + +The current `relay.outbox: true` setting is opt-in and off by default. On an +accepted ordered baseline, transient failures can queue a move, seal its base +and identity before local application, and show it as pending. Replay restores +the base, catches up with the relay, then sends queued moves through the normal +credential, sequence and engine checks. Lost replies are reconciled by move ID +and receipt, not by inventing an ACK or executing the external effect again. +Conflicts can refuse a previously visible local move. See `docs/api.md`, +`docs/formats.md`, `src/relay-outbox.js` and `test/relay-outbox.test.js`. + +A queued claim is **provisional**, not a globally won lease. A dependent worker +must not interpret exit 0 alone as exclusive authority to publish or perform an +irreversible external action. Default linked behavior remains H10's refusal. +Changing these promises needs an explicit resolution of H3/H10/H16 and draft +H21, including how existing callers distinguish queued and committed results. + +## Offline contract before stronger claims + +| Boundary | Required decision or evidence | +| --- | --- | +| Authority | Keep relay ordering with provisional local work, or explicitly opt into one locally authoritative sequencer. Do not claim both independent partitioned writers and global single-winner claims. | +| Fencing | A locally authoritative mode needs a durable board epoch/owner and enforcement at every write ingress, including phones and older clients. A local file lock alone cannot fence another machine. Transfer only after reconciliation; no timeout-based unfenced promotion. | +| Authorization | Bind board/repository immutable IDs, principal, actor, operation scope and authority epoch. No anonymous mutation, forged sender, new seat/token mint, person action or approver decision through an offline fallback. | +| Revocation | State when locally delegated authority expires and what an offline node can know. Recheck current token/access at relay admission. Explicit denial/revocation must not be treated as transport failure. Rejected local work remains visible as a rejection, not silently accepted or discarded. | +| Identity and correlation | Persist the original unique move ID, intent, causal base and dependent item bindings before returning local success. Retain the ID across retry/reseal, correlate the canonical result/refusal, and reject an ID reused for different content. | +| Ordering | Local queue order is not global relay order. Catch up before sequencing each replay; preserve sequence-associated seal/nonces and record conflicts, renumbering and dependent refusals. Never reserve imaginary global sequence slots while disconnected. | +| Durability | Exercise process exits at every queue/apply/restore/send/ACK/prune boundary. Existing write-then-rename code is not evidence of power-loss durability: an explicit no-loss promise needs file and directory sync or an equivalent transactional journal, plus recovery tests. | +| Backlog | Define byte/entry bounds and backpressure before accepting more work. Never evict an unacknowledged move to make room; expose pending age/count/bytes and retained failures without plaintext/credential telemetry. | +| Acknowledgment | Distinguish locally durable intent, provisional application, relay acceptance and engine result. Clear only against a matching durable receipt; reconcile an unknown outcome before resend or destructive cleanup. | +| Side effects | Replaying board intent must not rerun Git publication, checks, messages or other external effects whose earlier outcome is uncertain. Keep their own correlation/receipts and require an authoritative claim where needed. | + +The first increment changes none of the outbox's storage, actor, seal, sequence, +revocation, deduplication or ACK code. It is not a certification of that code's +crash or power-loss guarantees. + +## GitHub API is not Git synchronization + +`createGitHubClient.access()` looks up the App installation, reuses its scoped +installation token, then reads repository metadata and the user's role. Writes +check afresh; reads have a bounded cache. This happens even if no Git objects +need synchronization. `git worktree add` uses local objects, but +`pullboard worktree` then registers a seat through the ordered board path. +Publication and runner-cache seeding have separate Git network operations. + +A five-minute fetch/push debounce can reduce **Git transport** traffic, not these +permission requests. It must retain exact publication receipts and must not +reuse a stale role to authorize a write. Measure sanitized request counts/status, +rate-limit resource/reset and request IDs at the relay before attributing load; +a different client's current allowance cannot establish the hosted App's past +budget. Do not stress GitHub to reproduce an outage. + +## Qualification still required + +The draft patch adds these real-loopback regressions, not a mocked success path: + +```sh +node --test test/relay-provider-outage.test.js +node --test --test-name-pattern='an opted-in outbox retains a move through a GitHub' test/relay-outbox.test.js +``` + +They cover header-evidenced primary/secondary limits and denial controls at +installation/metadata/permission endpoints; public-role fallback; fail-closed +HTTP 503 with no appended move; read-cache recovery versus genuine denial; and +stable queued identity followed by one replay after provider recovery. +**These tests have not been run for this draft.** There is no red/green result +or full gate claim. Run the tests against both unchanged main and the patch in +an admitted isolated environment before merge, then run the applicable existing +auth/revocation, lost-ACK, conflict, outbox and default-offline regressions. + +Stronger offline claims additionally need two-partition conflicting-claim tests, +credential expiry/revocation during replay, exact crash cuts, disk-full and +backlog limits, and an independent consumer test that never starts irreversible +work from a provisional claim. Those are open gates, not tests this patch claims +to implement or pass. Self-hosting the MIT-licensed `relay/` service gives +operational control; its current GitHub App permission dependency remains. diff --git a/relay/github.js b/relay/github.js index 23c9bb38c..66406091e 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 === 429 || 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-outbox.test.js b/test/relay-outbox.test.js index d44973598..7eb5515eb 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,H17]', 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-provider-outage.test.js b/test/relay-provider-outage.test.js new file mode 100644 index 000000000..2851098b4 --- /dev/null +++ b/test/relay-provider-outage.test.js @@ -0,0 +1,91 @@ +/** Provider failures stay distinct from permission loss; remote writes still fail closed [H8,H10,H16]. */ +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { test } from 'node:test'; +import { createGitHubClient } from '../relay/github.js'; +import { createRelayAuth, ACCESS_WINDOW_MS } from '../relay/auth.js'; +import { serveRelay } from '../relay/service.js'; +import { ENGINE_VERSION } from '../src/machine.js'; +import { githubFixture } from './relay-fixture.js'; + +const REPOSITORY = '/repos/fixture/repository'; +const PERMISSION = REPOSITORY + '/collaborators/fixture-user/permission'; +const USER = { id: '7', login: 'fixture-user' }; + +test('header-evidenced GitHub rate limits are unavailable, not access denials [H8]', async (t) => { + const provider = await githubFixture(t); + const client = createGitHubClient(provider.config); + const cases = [ + { status: 403, headers: { 'x-ratelimit-remaining': '0' }, code: 'GITHUB_UNAVAILABLE' }, + { status: 403, headers: { 'retry-after': '60', 'x-ratelimit-remaining': '17' }, code: 'GITHUB_UNAVAILABLE' }, + { status: 429, headers: {}, code: 'GITHUB_UNAVAILABLE' }, + { status: 503, headers: {}, code: 'GITHUB_UNAVAILABLE' }, + { status: 401, headers: {}, code: 'NO_REPO_ACCESS' }, + { status: 403, headers: { 'x-ratelimit-remaining': '17' }, code: 'NO_REPO_ACCESS' }, + { status: 403, headers: { 'retry-after': 'invalid' }, code: 'NO_REPO_ACCESS' }, + { status: 404, headers: {}, code: 'NO_REPO_ACCESS' }, + ]; + // Both the installation lookup and the public-repository permission fallback used to swallow a 403. + for (const path of [REPOSITORY + '/installation', REPOSITORY, PERMISSION]) { + for (const scenario of cases) await t.test(`${path}: ${scenario.status} ${JSON.stringify(scenario.headers)}`, async () => { + provider.state.public = false; + provider.state.providerFailure = { path, status: scenario.status, headers: scenario.headers }; + await assert.rejects(client.access(USER, 'fixture/repository'), { code: scenario.code }); + }); + } + provider.state.public = true; + provider.state.providerFailure = { path: PERMISSION, status: 403, headers: { 'retry-after': '60' } }; + await assert.rejects(client.access(USER, 'fixture/repository'), { code: 'GITHUB_UNAVAILABLE' }, + 'a public repository does not turn a transient permission failure into role none'); +}); + +test('GitHub outage refuses the sealed write with 503 and does not cache a denial [H8,H16]', async (t) => { + const provider = await githubFixture(t); + const root = mkdtempSync(join(tmpdir(), 'pullboard-provider-outage-')); + let now = Date.now(); + const auth = createRelayAuth({ database: join(root, 'auth.sqlite'), github: createGitHubClient(provider.config), now: () => now }); + let service; + t.after(async () => { await service?.close(); auth.close(); rmSync(root, { recursive: true, force: true }); }); + const grant = await auth.beginDevice(); + provider.state.deviceAuthorized = true; + now += grant.interval * 1000; + const person = await auth.pollDevice(grant.ticket); + const board = 'b'.repeat(32); + await auth.linkBoard(person.token, board, 'fixture/repository'); + service = await serveRelay({ directory: join(root, 'journal'), auth, port: 0, maintenanceMs: 0 }); + const origin = `http://127.0.0.1:${service.port}`; + const headers = { authorization: `Bearer ${person.token}`, 'x-pullboard-engine': String(ENGINE_VERSION), 'content-type': 'application/json' }; + const baseline = await fetch(`${origin}/api/v1/boards/${board}/state`, { + method: 'PUT', headers, body: JSON.stringify({ sequence: 0, sealed: 'AQ' }), + }); + assert.equal(baseline.status, 200); + await baseline.arrayBuffer(); + now += ACCESS_WINDOW_MS; + await auth.authenticate(person.token, { board }); + provider.state.providerFailure = { path: PERMISSION, status: 403, headers: { 'x-ratelimit-remaining': '0' } }; + const refused = await fetch(`${origin}/api/v1/boards/${board}/moves`, { + method: 'POST', headers, body: JSON.stringify({ sequence: 1, sealed: 'Ag' }), + }); + assert.equal(refused.status, 503); + assert.equal((await refused.json()).error.code, 'GITHUB_UNAVAILABLE'); + assert.equal((await auth.authenticate(person.token, { board })).permission, 'write', + 'the failed fresh write check does not poison the still-valid read cache'); + delete provider.state.providerFailure; + now += ACCESS_WINDOW_MS; + assert.equal((await auth.authenticate(person.token, { board })).permission, 'write', + 'provider recovery is observed without a false negative-cache delay'); + const events = await fetch(`${origin}/api/v1/boards/${board}/events?after=0`, { headers }); + assert.equal(events.status, 200); + assert.deepEqual((await events.json()).events, [], 'the unavailable permission check stores no move'); + const sent = await fetch(`${origin}/api/v1/boards/${board}/moves`, { + method: 'POST', headers, body: JSON.stringify({ sequence: 1, sealed: 'Ag' }), + }); + assert.equal(sent.status, 200); + await sent.arrayBuffer(); + provider.state.providerFailure = { path: PERMISSION, status: 403, headers: {} }; + await assert.rejects(auth.authenticate(person.token, { board, write: true }), { code: 'NO_REPO_ACCESS' }); + await assert.rejects(auth.authenticate(person.token, { board }), { code: 'NO_REPO_ACCESS' }, + 'a real denial still invalidates an allowed read'); +}); From b5a14f738a06877bc17eb849ff6d6ba1684f8394 Mon Sep 17 00:00:00 2001 From: McHerpes Date: Sun, 25 Oct 2026 22:38:04 +0000 Subject: [PATCH 2/3] refactor(relay): keep outage correction focused [H8,H16,H17] [skip ci] --- docs/rfcs/0005-relay-resilience.md | 96 ------------------------------ relay/github.js | 2 +- 2 files changed, 1 insertion(+), 97 deletions(-) delete mode 100644 docs/rfcs/0005-relay-resilience.md diff --git a/docs/rfcs/0005-relay-resilience.md b/docs/rfcs/0005-relay-resilience.md deleted file mode 100644 index 56c29a655..000000000 --- a/docs/rfcs/0005-relay-resilience.md +++ /dev/null @@ -1,96 +0,0 @@ -# RFC 0005: Relay provider failures and offline claim boundaries - -- Status: draft; not a change to approved rows or permission to deploy -- Related: [issue #11](https://github.com/pullboard-dev/pullboard/issues/11) -- Rows: H3, H8, H10, H16; H21 remains draft - -## Small first increment - -Treat GitHub 403 responses with `x-ratelimit-remaining: 0` or an integer -`retry-after` as `GITHUB_UNAVAILABLE`, as for 429. Keep ordinary 401/403/404 -refusals unchanged. Return HTTP 503 for this code on the sealed API, matching -the auth API. A failed current permission check still authorizes no remote -write and appends no event. It must not poison the read-permission cache. - -This is error classification, not rate-limit prevention or an offline authority -implementation. It does not recognize every headerless secondary-limit response, -propagate a retry deadline, add backoff, or prove a hosted incident's cause. -A follow-up must honor provider reset/Retry-After and use bounded backoff without -turning an unavailable provider into an authorization success. - -## Reuse the existing outbox - -The current `relay.outbox: true` setting is opt-in and off by default. On an -accepted ordered baseline, transient failures can queue a move, seal its base -and identity before local application, and show it as pending. Replay restores -the base, catches up with the relay, then sends queued moves through the normal -credential, sequence and engine checks. Lost replies are reconciled by move ID -and receipt, not by inventing an ACK or executing the external effect again. -Conflicts can refuse a previously visible local move. See `docs/api.md`, -`docs/formats.md`, `src/relay-outbox.js` and `test/relay-outbox.test.js`. - -A queued claim is **provisional**, not a globally won lease. A dependent worker -must not interpret exit 0 alone as exclusive authority to publish or perform an -irreversible external action. Default linked behavior remains H10's refusal. -Changing these promises needs an explicit resolution of H3/H10/H16 and draft -H21, including how existing callers distinguish queued and committed results. - -## Offline contract before stronger claims - -| Boundary | Required decision or evidence | -| --- | --- | -| Authority | Keep relay ordering with provisional local work, or explicitly opt into one locally authoritative sequencer. Do not claim both independent partitioned writers and global single-winner claims. | -| Fencing | A locally authoritative mode needs a durable board epoch/owner and enforcement at every write ingress, including phones and older clients. A local file lock alone cannot fence another machine. Transfer only after reconciliation; no timeout-based unfenced promotion. | -| Authorization | Bind board/repository immutable IDs, principal, actor, operation scope and authority epoch. No anonymous mutation, forged sender, new seat/token mint, person action or approver decision through an offline fallback. | -| Revocation | State when locally delegated authority expires and what an offline node can know. Recheck current token/access at relay admission. Explicit denial/revocation must not be treated as transport failure. Rejected local work remains visible as a rejection, not silently accepted or discarded. | -| Identity and correlation | Persist the original unique move ID, intent, causal base and dependent item bindings before returning local success. Retain the ID across retry/reseal, correlate the canonical result/refusal, and reject an ID reused for different content. | -| Ordering | Local queue order is not global relay order. Catch up before sequencing each replay; preserve sequence-associated seal/nonces and record conflicts, renumbering and dependent refusals. Never reserve imaginary global sequence slots while disconnected. | -| Durability | Exercise process exits at every queue/apply/restore/send/ACK/prune boundary. Existing write-then-rename code is not evidence of power-loss durability: an explicit no-loss promise needs file and directory sync or an equivalent transactional journal, plus recovery tests. | -| Backlog | Define byte/entry bounds and backpressure before accepting more work. Never evict an unacknowledged move to make room; expose pending age/count/bytes and retained failures without plaintext/credential telemetry. | -| Acknowledgment | Distinguish locally durable intent, provisional application, relay acceptance and engine result. Clear only against a matching durable receipt; reconcile an unknown outcome before resend or destructive cleanup. | -| Side effects | Replaying board intent must not rerun Git publication, checks, messages or other external effects whose earlier outcome is uncertain. Keep their own correlation/receipts and require an authoritative claim where needed. | - -The first increment changes none of the outbox's storage, actor, seal, sequence, -revocation, deduplication or ACK code. It is not a certification of that code's -crash or power-loss guarantees. - -## GitHub API is not Git synchronization - -`createGitHubClient.access()` looks up the App installation, reuses its scoped -installation token, then reads repository metadata and the user's role. Writes -check afresh; reads have a bounded cache. This happens even if no Git objects -need synchronization. `git worktree add` uses local objects, but -`pullboard worktree` then registers a seat through the ordered board path. -Publication and runner-cache seeding have separate Git network operations. - -A five-minute fetch/push debounce can reduce **Git transport** traffic, not these -permission requests. It must retain exact publication receipts and must not -reuse a stale role to authorize a write. Measure sanitized request counts/status, -rate-limit resource/reset and request IDs at the relay before attributing load; -a different client's current allowance cannot establish the hosted App's past -budget. Do not stress GitHub to reproduce an outage. - -## Qualification still required - -The draft patch adds these real-loopback regressions, not a mocked success path: - -```sh -node --test test/relay-provider-outage.test.js -node --test --test-name-pattern='an opted-in outbox retains a move through a GitHub' test/relay-outbox.test.js -``` - -They cover header-evidenced primary/secondary limits and denial controls at -installation/metadata/permission endpoints; public-role fallback; fail-closed -HTTP 503 with no appended move; read-cache recovery versus genuine denial; and -stable queued identity followed by one replay after provider recovery. -**These tests have not been run for this draft.** There is no red/green result -or full gate claim. Run the tests against both unchanged main and the patch in -an admitted isolated environment before merge, then run the applicable existing -auth/revocation, lost-ACK, conflict, outbox and default-offline regressions. - -Stronger offline claims additionally need two-partition conflicting-claim tests, -credential expiry/revocation during replay, exact crash cuts, disk-full and -backlog limits, and an independent consumer test that never starts irreversible -work from a provisional claim. Those are open gates, not tests this patch claims -to implement or pass. Self-hosting the MIT-licensed `relay/` service gives -operational control; its current GitHub App permission dependency remains. diff --git a/relay/github.js b/relay/github.js index 66406091e..8cab594f4 100644 --- a/relay/github.js +++ b/relay/github.js @@ -124,7 +124,7 @@ export function createGitHubClient({ clientId, clientSecret, callbackURL, oauthB continue; } // GitHub uses 403 for rate limits too; do not cache those as lost repository access. - if (response.status === 429 || response.status === 403 && (response.headers.get('x-ratelimit-remaining') === '0' + 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'); From b497d9b3424a461640fc5a1ff47e3840bc1891c2 Mon Sep 17 00:00:00 2001 From: McHerpes Date: Sun, 25 Oct 2026 23:27:29 +0000 Subject: [PATCH 3/3] test(relay): remove duplicate outage cases [H8,H16,H21] Signed-off-by: McHerpes --- test/relay-github.test.js | 22 ++++++++ test/relay-outbox.test.js | 2 +- test/relay-provider-outage.test.js | 91 ------------------------------ test/relay.test.js | 29 ++++++++++ 4 files changed, 52 insertions(+), 92 deletions(-) delete mode 100644 test/relay-provider-outage.test.js 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 7eb5515eb..e6cdc3196 100644 --- a/test/relay-outbox.test.js +++ b/test/relay-outbox.test.js @@ -108,7 +108,7 @@ 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,H17]', async (t) => { +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(); diff --git a/test/relay-provider-outage.test.js b/test/relay-provider-outage.test.js deleted file mode 100644 index 2851098b4..000000000 --- a/test/relay-provider-outage.test.js +++ /dev/null @@ -1,91 +0,0 @@ -/** Provider failures stay distinct from permission loss; remote writes still fail closed [H8,H10,H16]. */ -import assert from 'node:assert/strict'; -import { mkdtempSync, rmSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import { test } from 'node:test'; -import { createGitHubClient } from '../relay/github.js'; -import { createRelayAuth, ACCESS_WINDOW_MS } from '../relay/auth.js'; -import { serveRelay } from '../relay/service.js'; -import { ENGINE_VERSION } from '../src/machine.js'; -import { githubFixture } from './relay-fixture.js'; - -const REPOSITORY = '/repos/fixture/repository'; -const PERMISSION = REPOSITORY + '/collaborators/fixture-user/permission'; -const USER = { id: '7', login: 'fixture-user' }; - -test('header-evidenced GitHub rate limits are unavailable, not access denials [H8]', async (t) => { - const provider = await githubFixture(t); - const client = createGitHubClient(provider.config); - const cases = [ - { status: 403, headers: { 'x-ratelimit-remaining': '0' }, code: 'GITHUB_UNAVAILABLE' }, - { status: 403, headers: { 'retry-after': '60', 'x-ratelimit-remaining': '17' }, code: 'GITHUB_UNAVAILABLE' }, - { status: 429, headers: {}, code: 'GITHUB_UNAVAILABLE' }, - { status: 503, headers: {}, code: 'GITHUB_UNAVAILABLE' }, - { status: 401, headers: {}, code: 'NO_REPO_ACCESS' }, - { status: 403, headers: { 'x-ratelimit-remaining': '17' }, code: 'NO_REPO_ACCESS' }, - { status: 403, headers: { 'retry-after': 'invalid' }, code: 'NO_REPO_ACCESS' }, - { status: 404, headers: {}, code: 'NO_REPO_ACCESS' }, - ]; - // Both the installation lookup and the public-repository permission fallback used to swallow a 403. - for (const path of [REPOSITORY + '/installation', REPOSITORY, PERMISSION]) { - for (const scenario of cases) await t.test(`${path}: ${scenario.status} ${JSON.stringify(scenario.headers)}`, async () => { - provider.state.public = false; - provider.state.providerFailure = { path, status: scenario.status, headers: scenario.headers }; - await assert.rejects(client.access(USER, 'fixture/repository'), { code: scenario.code }); - }); - } - provider.state.public = true; - provider.state.providerFailure = { path: PERMISSION, status: 403, headers: { 'retry-after': '60' } }; - await assert.rejects(client.access(USER, 'fixture/repository'), { code: 'GITHUB_UNAVAILABLE' }, - 'a public repository does not turn a transient permission failure into role none'); -}); - -test('GitHub outage refuses the sealed write with 503 and does not cache a denial [H8,H16]', async (t) => { - const provider = await githubFixture(t); - const root = mkdtempSync(join(tmpdir(), 'pullboard-provider-outage-')); - let now = Date.now(); - const auth = createRelayAuth({ database: join(root, 'auth.sqlite'), github: createGitHubClient(provider.config), now: () => now }); - let service; - t.after(async () => { await service?.close(); auth.close(); rmSync(root, { recursive: true, force: true }); }); - const grant = await auth.beginDevice(); - provider.state.deviceAuthorized = true; - now += grant.interval * 1000; - const person = await auth.pollDevice(grant.ticket); - const board = 'b'.repeat(32); - await auth.linkBoard(person.token, board, 'fixture/repository'); - service = await serveRelay({ directory: join(root, 'journal'), auth, port: 0, maintenanceMs: 0 }); - const origin = `http://127.0.0.1:${service.port}`; - const headers = { authorization: `Bearer ${person.token}`, 'x-pullboard-engine': String(ENGINE_VERSION), 'content-type': 'application/json' }; - const baseline = await fetch(`${origin}/api/v1/boards/${board}/state`, { - method: 'PUT', headers, body: JSON.stringify({ sequence: 0, sealed: 'AQ' }), - }); - assert.equal(baseline.status, 200); - await baseline.arrayBuffer(); - now += ACCESS_WINDOW_MS; - await auth.authenticate(person.token, { board }); - provider.state.providerFailure = { path: PERMISSION, status: 403, headers: { 'x-ratelimit-remaining': '0' } }; - const refused = await fetch(`${origin}/api/v1/boards/${board}/moves`, { - method: 'POST', headers, body: JSON.stringify({ sequence: 1, sealed: 'Ag' }), - }); - assert.equal(refused.status, 503); - assert.equal((await refused.json()).error.code, 'GITHUB_UNAVAILABLE'); - assert.equal((await auth.authenticate(person.token, { board })).permission, 'write', - 'the failed fresh write check does not poison the still-valid read cache'); - delete provider.state.providerFailure; - now += ACCESS_WINDOW_MS; - assert.equal((await auth.authenticate(person.token, { board })).permission, 'write', - 'provider recovery is observed without a false negative-cache delay'); - const events = await fetch(`${origin}/api/v1/boards/${board}/events?after=0`, { headers }); - assert.equal(events.status, 200); - assert.deepEqual((await events.json()).events, [], 'the unavailable permission check stores no move'); - const sent = await fetch(`${origin}/api/v1/boards/${board}/moves`, { - method: 'POST', headers, body: JSON.stringify({ sequence: 1, sealed: 'Ag' }), - }); - assert.equal(sent.status, 200); - await sent.arrayBuffer(); - provider.state.providerFailure = { path: PERMISSION, status: 403, headers: {} }; - await assert.rejects(auth.authenticate(person.token, { board, write: true }), { code: 'NO_REPO_ACCESS' }); - await assert.rejects(auth.authenticate(person.token, { board }), { code: 'NO_REPO_ACCESS' }, - 'a real denial still invalidates an allowed read'); -}); 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);