diff --git a/agents/megavolt/work/queue-236/BUILD.md b/agents/megavolt/work/queue-236/BUILD.md index 453c8a452..3c9f21b50 100644 --- a/agents/megavolt/work/queue-236/BUILD.md +++ b/agents/megavolt/work/queue-236/BUILD.md @@ -64,6 +64,30 @@ Each is a reading of the design where it left room. Dewey may rule otherwise. 17. X-G7 changes the binding with a fixture decision whose hash differs (row 234's G9 pattern), since the broker keeps its own copy of the business. The admin url check takes row 199's `parseGiteaUrl`, which accepts the origin with or without a trailing slash. +18. r2, Dewey B1 (dewey-0781): step 8 and both settle branches (forward, back) check for a `grant.revoked` + of the agent after the decision's raise, in the same transaction as their write. A revoke that came + while step 7's or the settle's delete awaited took the record and both tokens, so the grant ends + `revoked` with the human's notice, as a revoke of a running executor ends it (X-G11), and no record is + written back. N9's restart case stays silent: a restart never settles a revoked rotation. +19. r2, Dewey B2: a sweep retry of `tokens` or `login` checks the user's id first, as the revoke close does, + against the `gitea_users` row (it survives a fire). A 404 skips with `gitea-user-gone`, a 200 with + another id with `gitea-user-foreign`, any other status leaves the entry. Skips go in `grant.swept` and + the sweep answer as `skipped: [{agent, user, id, reason}]`; a skipped entry is not left, so the next + sweep does not retry it. + +## Named for DEFERRED (r2, Dewey N1-N4) + +None is a small local change, so none is in r2. + +- N1: an admin rotate cut between the mint and the slot write leaves a token at Gitea that no event names. + A `grant.admin rotate-begin {oldName}` before the mint would close it; that is a new event shape. +- N2: `adminTokenLeft` is `unknown` when `/api/v1/token` gives no name. Which 1.27 F-fact shows the name + needs a check against the container, not a code change. +- N3: admin set or remove records `adminTokenLeft: null`, so a left token drops out of status. Carrying it + forward lets `rotate --finish` delete by name under a slot of another account or url; refusing remove + while one is left blocks the human's remove when Gitea is down. Either needs a ruling. +- N4: rotate or remove while a gitea claim is held. Refusing it blocks the human's remove during a run; + the running executor keeps the api it began with. Needs a ruling or a line in the README. ## Q188 @@ -93,3 +117,12 @@ were added before any mutant run: before its Gitea delete", which holds the DELETE open and reads the file and state. - M37 (evidence without its kind filter): "X-G15: a look-alike spend under another kind with the grant writer pair is not an open grant". + +r2 tests, each red on 8bf442be9 by reading: + +- B1: "X-G11: a revoke during a rotation step 7 ends it revoked, and step 8 writes no record back" and + "X-G11: a revoke while a sweep settles a rotation, forward or back, ends it revoked with no record". + On 8bf442be9 step 8 and the settle write the record and `grant.applied` (forward) or `grant.failed + interrupted settled:back` after the revoke deleted it. +- B2: "X-G14: a sweep retries a fire only for the user it fired: a remade or gone user is skipped and + named". On 8bf442be9 the retry fires the remade user (token list, delete, PATCH) and has no `skipped`. diff --git a/packages/bus/README.md b/packages/bus/README.md index 796a0f9cb..21768d179 100644 --- a/packages/bus/README.md +++ b/packages/bus/README.md @@ -228,7 +228,7 @@ an operator inspect content and delivery evidence after an uncertain result. | `grants.status` | `{service:"gitea"}`; the human, the tester or the chief (row 236, #1742, #436b r5). The admin slot's url, account and rotate-by date with `rotationDue`, `adminTokenLeft`, the `gitea_users` and `gitea_grants` records, approved decisions not yet run (`spentAtNextStart`), grants `running` or `settlePending`, agents held as `tokenUnknown`, the `serviceLeft` entries a sweep retries, and revokes still `revoking`. No token. | | `grants.run` | `{decision}`; the human only. Starts the Gitea executor of a `credential.mint` decision with a gitea grant the human said yes to, as the yes itself does (X1). Step 0's refusals come back with no record: `not-authorized`, `decision-consumed`, `grant-token-unknown`, `grant-busy`, `gitea-not-configured`. Returns `{decision,agent,started:true}`; the outcome is a `grant.applied` or `grant.failed` and a notice. | | `grants.revoke` | `{service:"gitea",agent,reason}`; the human, the business's assistant, or the tester when `bus.tester` is exactly `true`. The token leaves memory and its file goes, an executor in flight aborts, then one transaction deletes the `gitea_grants` record and writes `grant.revoked` with the token ids to delete. The deletes run with the admin token after C1 to C4, and `grant.revoke.closed` records `serviceLeft`, a list. `credentials.revoke AGENT gitea` and `credentials.delete` do the same for a stack-made user; delete also deletes every token of the user and sets `prohibit_login` (fire). | -| `grants.sweep` | `{service:"gitea"}`; the human or the tester. Closes open revokes, settles open rotations again, and retries each `serviceLeft` entry. Writes `grant.sweep` and `grant.swept {left,settled,closed}`. | +| `grants.sweep` | `{service:"gitea"}`; the human or the tester. Closes open revokes, settles open rotations again, and retries each `serviceLeft` entry. A fire retry (`tokens`, `login`) first checks the user's id as the revoke close does, and a gone or remade user is skipped with `gitea-user-gone` or `gitea-user-foreign`. Writes `grant.sweep` and `grant.swept {left,skipped,settled,closed}`. | | `grants.admin.set`, `grants.admin.rotate`, `grants.admin.remove` | The human only (#436b r5 section 3). set takes `{service,url,account,rotateBy,token}`, checks C1 to C4 with the token, then writes the slot under `/secrets/grants//gitea`; a slot already there is `grant-admin-exists`. rotate `{service,rotateBy}` mints a successor admin token and names the old one in `adminTokenLeft`; `{service,finish:true}` deletes it. Rotation voids the approved decisions not yet run, as their `adminSha256` no longer matches. remove drops the slot. Each writes `grant.admin` with no secret. | Each of the three leaves one receipt in the organization's `changes.jsonl`: the actor and a diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index 0e01d4463..f1362f1f5 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -1703,12 +1703,16 @@ export class Broker { if (!previous) return 'applied'; // Step 7, after the new token is live. const serviceLeft = (await this.#giteaDelete(api, user, previous.tokenId).catch(() => false)) ? [] : [previous.tokenId]; - // Step 8. - this.#store.transaction(() => { + // Step 8. A revoke during step 7 took the record and both tokens: it ends revoked, with no write. + return this.#store.transaction(() => { + if (this.#giteaRevokedSince(business, decision, agent)) { + this.#giteaFailed(business, decision, agent, 'revoked', { user }); + return 'revoked'; + } write(); applied(serviceLeft.length ? { serviceLeft } : {}); + return 'applied'; }); - return 'applied'; } catch (e) { if (committed) { // Live already: a late abort is an ordinary revoke's to handle. Only a store write failed, so the @@ -1760,6 +1764,13 @@ export class Broker { 'ON CONFLICT(business,agent) DO UPDATE SET user=excluded.user,token_id=excluded.token_id,token_name=excluded.token_name,projects=excluded.projects,rotate_by=excluded.rotate_by,decision=excluded.decision', business, agent, user, t.tokenId, t.tokenName, JSON.stringify(t.projects), t.rotateBy, t.decision, ); + // A revoke while a branch below awaits took the record and both tokens: the grant ends revoked, as a + // revoke of a running executor ends it, and nothing is written back. Run inside the branch's transaction. + const revoked = () => { + if (!this.#giteaRevokedSince(business, decision, agent)) return false; + this.#giteaFailed(business, decision, agent, 'revoked', { user }); + return true; + }; if (who.status === 200 && who.id === s.tokenId) { try { this.#giteaKeys.reload(business, agent, g.rotateBy); @@ -1767,7 +1778,8 @@ export class Broker { return 'settle-pending'; } const serviceLeft = (await this.#giteaDelete(api, user, s.previousTokenId)) ? [] : [s.previousTokenId]; - this.#store.transaction(() => { + return this.#store.transaction(() => { + if (revoked()) return 'revoked'; record({ tokenId: s.tokenId, tokenName: s.tokenName, projects: g.target.projects, rotateBy: g.rotateBy, decision }); this.#grantEvent( business, @@ -1775,16 +1787,17 @@ export class Broker { agent, ); this.#grantNotice(business, decision, `The Gitea token rotation of ${agent} (decision ${decision}) settled forward: the new token is live.`); + return 'applied'; }); - return 'applied'; } if (who.status === 200 && who.id === s.previousTokenId) { const serviceLeft = (await this.#giteaDelete(api, user, s.tokenId)) ? [] : [s.tokenId]; - this.#store.transaction(() => { + return this.#store.transaction(() => { + if (revoked()) return 'revoked'; record(s.previous); this.#giteaFailed(business, decision, agent, 'interrupted', { user, settled: 'back', ...(serviceLeft.length ? { serviceLeft } : {}) }); + return 'interrupted'; }); - return 'interrupted'; } if (who.status === 401) { const serviceLeft = []; @@ -2099,6 +2112,7 @@ export class Broker { } const admin = await this.#giteaAdminChecked(business, log); const still = []; + const skipped = []; for (const e of left) { if (e.fresh) { still.push({ agent: e.agent, user: e.user, id: e.id }); @@ -2107,14 +2121,24 @@ export class Broker { let ok = false; if (admin && e.user) { if (e.id === 'tokens' || e.id === 'login') { - const r = await this.#giteaFire(admin.api, e.user); - ok = !r.includes(e.id); + // A fire retry checks the user's id first, as the revoke close does: the gitea_users row + // survives a fire, and a user remade under the same name is not the one that was fired. + const u = this.#store.get('SELECT id FROM gitea_users WHERE business=? AND agent=? AND user=?', business, e.agent, e.user); + const r = await admin.api.userGet(e.user); + const reason = r.status === 404 ? 'gitea-user-gone' : r.status === 200 && (!u || r.json?.id !== u.id) ? 'gitea-user-foreign' : null; + if (reason) { + skipped.push({ agent: e.agent, user: e.user, id: e.id, reason }); + continue; + } + if (r.status === 200) ok = !(await this.#giteaFire(admin.api, e.user)).includes(e.id); } else ok = await this.#giteaDelete(admin.api, e.user, e.id).catch(() => false); } if (!ok) still.push({ agent: e.agent, user: e.user, id: e.id }); } - this.#store.transaction(() => this.#grantEvent(business, { action: 'grant.swept', method: 'gitea', sweep, left: still, settled, closed, by })); - return { left: still, settled, closed }; + this.#store.transaction(() => + this.#grantEvent(business, { action: 'grant.swept', method: 'gitea', sweep, left: still, skipped, settled, closed, by }), + ); + return { left: still, skipped, settled, closed }; } // `mosaic grants status gitea`. #giteaStatus(business) { diff --git a/packages/bus/tests/gitea-grant.test.mjs b/packages/bus/tests/gitea-grant.test.mjs index c76d34d20..9c21f009e 100644 --- a/packages/bus/tests/gitea-grant.test.mjs +++ b/packages/bus/tests/gitea-grant.test.mjs @@ -939,6 +939,58 @@ test('X-G11: a revoke unloads memory and removes the file before its Gitea delet assert.deepEqual(f.userTokens(), []); }); +test('X-G11: a revoke during a rotation step 7 ends it revoked, and step 8 writes no record back', async (t) => { + const f = await setup(t); + await f.approve(); + const oldId = f.record().token_id; + let release; + const held = new Promise((r) => (release = r)); + f.gitea.rule((e) => e.method === 'DELETE' && e.path === `/api/v1/users/${USER}/tokens/${oldId}`, async () => { + await held; + return { pass: true }; + }); + const d = await f.raise(); + f.call('human', 'decision.resolve', { id: d.id, choice: 'yes' }); + await until(() => f.gitea.calls().includes(`DELETE /api/v1/users/${USER}/tokens/${oldId}`)); + const revoke = f.call('human', 'grants.revoke', { service: 'gitea', agent: 'coder', reason: 'stop' }); + release(); + await revoke; + await until(() => f.ended(d.id)); + await f.broker.giteaIdle(); + assert.equal(f.ended(d.id).code, 'revoked'); + assert.equal(f.grantEvents().some((e) => e.action === 'grant.applied' && e.grantDecision === d.id), false); + assert.equal(f.record(), undefined); + assert.deepEqual(f.userTokens(), []); +}); + +test('X-G11: a revoke while a sweep settles a rotation, forward or back, ends it revoked with no record', async (t) => { + for (const forward of [true, false]) { + const f = await setup(t); + const { live, next, id } = await openRotation(f); + if (forward) writeFileSync(f.tokenFile, next.sha1 + '\n'); + // Forward deletes the old token, back the new one: hold that delete. + const target = forward ? live.token_id : next.id; + let release; + const held = new Promise((r) => (release = r)); + f.gitea.rule((e) => e.method === 'DELETE' && e.path === `/api/v1/users/${USER}/tokens/${target}`, async () => { + await held; + return { pass: true }; + }); + const sweep = f.call('human', 'grants.sweep', { service: 'gitea' }); + await until(() => f.gitea.calls().includes(`DELETE /api/v1/users/${USER}/tokens/${target}`)); + const revoke = f.call('human', 'grants.revoke', { service: 'gitea', agent: 'coder', reason: 'stop' }); + release(); + await revoke; + const swept = await sweep; + await f.broker.giteaIdle(); + assert.deepEqual(swept.settled, [{ decision: id, outcome: 'revoked' }], forward ? 'forward' : 'back'); + assert.equal(f.ended(id).code, 'revoked'); + assert.equal(f.grantEvents().some((e) => e.grantDecision === id && e.settled), false); + assert.equal(f.record(), undefined); + assert.deepEqual(f.userTokens(), []); + } +}); + test('X-G12: a token that reaches wider than its forks fails with the repository named', async (t) => { const collab = await setup(t); collab.gitea.addRepo('other', 'secret', { collaborators: [USER] }); @@ -1068,6 +1120,36 @@ test('X-G14: a fire whose user is gone or remade skips the fire steps and says w assert.equal(g.gitea.users.get(USER).prohibit_login, false); }); +test('X-G14: a sweep retries a fire only for the user it fired: a remade or gone user is skipped and named', async (t) => { + for (const reason of ['gitea-user-foreign', 'gitea-user-gone']) { + const f = await setup(t); + await f.approve(); + // The fire's user check answers 500, so both fire steps are left for the sweep. + const off = f.gitea.rule((e) => e.method === 'GET' && e.path === `/api/v1/users/${USER}`, { status: 500, json: {} }); + f.call('human', 'credentials.delete', { agent: 'coder', reason: 'fired' }); + await until(() => f.grantEvents().some((e) => e.action === 'grant.revoke.closed')); + await f.broker.giteaIdle(); + off(); + assert.deepEqual(f.grantEvents().find((e) => e.action === 'grant.revoke.closed').serviceLeft, ['tokens', 'login']); + f.gitea.removeUser(USER); + if (reason === 'gitea-user-foreign') f.gitea.addUser(USER, { scopes: SEAT_SCOPES, tokenName: 'theirs' }); + const before = f.gitea.requests.length; + const swept = await f.call('human', 'grants.sweep', { service: 'gitea' }); + const after = f.gitea.requests.slice(before).map((r) => `${r.method} ${r.path.split('?')[0]}`); + assert.equal(after.some((c) => c.startsWith('PATCH ') || c.startsWith('DELETE ') || c.endsWith('/tokens')), false, reason); + assert.deepEqual(swept.left, []); + const skipped = [{ agent: 'coder', user: USER, id: 'tokens', reason }, { agent: 'coder', user: USER, id: 'login', reason }]; + assert.deepEqual(swept.skipped, skipped); + assert.deepEqual(f.grantEvents().findLast((e) => e.action === 'grant.swept').skipped, skipped); + if (reason === 'gitea-user-foreign') { + assert.equal(f.gitea.users.get(USER).prohibit_login, false); + assert.deepEqual(f.userTokens().map((x) => x.name), ['theirs']); + } + // Skipped is not left: the next sweep has nothing to retry. + assert.deepEqual((await f.call('human', 'grants.status', { service: 'gitea' })).serviceLeft, []); + } +}); + test('X-G14: a revoke Gitea cannot reach stays open, and a restart closes it', async (t) => { const f = await setup(t); await f.approve();