From fbff4ffa75bf5931a423b6a9a21f385f79d4df43 Mon Sep 17 00:00:00 2001 From: be-coder-07 Date: Wed, 5 Aug 2026 22:42:48 -0500 Subject: [PATCH] fix(#1045): close credential finalization races --- .../src/credentials/tea-login-store.spec.ts | 33 +++++- .../mosaic/src/credentials/tea-login-store.ts | 10 +- .../mosaic/src/credentials/team-grant.spec.ts | 111 ++++++++++++++++++ packages/mosaic/src/credentials/team-grant.ts | 9 +- 4 files changed, 157 insertions(+), 6 deletions(-) diff --git a/packages/mosaic/src/credentials/tea-login-store.spec.ts b/packages/mosaic/src/credentials/tea-login-store.spec.ts index 1598ef91..0a10ddd1 100644 --- a/packages/mosaic/src/credentials/tea-login-store.spec.ts +++ b/packages/mosaic/src/credentials/tea-login-store.spec.ts @@ -2,7 +2,7 @@ import { chmod, mkdtemp, readFile, readdir, rm, writeFile } from 'node:fs/promis import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; -import { TeaLoginStore } from './tea-login-store.js'; +import { TeaLoginStore, TeaLoginStoreError } from './tea-login-store.js'; let root: string | undefined; afterEach(async (): Promise => { @@ -106,6 +106,37 @@ describe('host-bound Tea login store', (): void => { prototypeNamedFieldChanged?.secret.fill(0); }); + it.each(['.nan', '.inf', '-.inf'] as const)( + 'rejects non-finite YAML metadata value %s while accepting explicit null', + async (nonFiniteValue): Promise => { + root = await mkdtemp(join(tmpdir(), 'mosaic-tea-store-')); + const configPath = join(root, 'tea', 'config.yml'); + const store = new TeaLoginStore(configPath); + await store.put('seat', 'git.one.invalid', new TextEncoder().encode('token-one')); + const baseline = await readFile(configPath, 'utf8'); + await writeFile( + configPath, + baseline.replace( + 'default: false', + `default: false\n extension-field: ${nonFiniteValue}`, + ), + { mode: 0o600 }, + ); + + expect(() => store.snapshot('seat', 'git.one.invalid')).toThrowError(TeaLoginStoreError); + expect(() => store.snapshot('seat', 'git.one.invalid')).toThrow(/code=tea-config-invalid/); + + await writeFile( + configPath, + baseline.replace('default: false', 'default: false\n extension-field: null'), + { mode: 0o600 }, + ); + const explicitNull = store.snapshot('seat', 'git.one.invalid'); + expect(explicitNull).toBeDefined(); + explicitNull?.secret.fill(0); + }, + ); + it.each(['put', 'remove'] as const)( 'removes secret-bearing temporary files when %s fails before rename', async (operation): Promise => { diff --git a/packages/mosaic/src/credentials/tea-login-store.ts b/packages/mosaic/src/credentials/tea-login-store.ts index 150cd93e..31f083b6 100644 --- a/packages/mosaic/src/credentials/tea-login-store.ts +++ b/packages/mosaic/src/credentials/tea-login-store.ts @@ -104,7 +104,15 @@ interface CanonicalJsonObject { function canonicalizeGenerationValue(value: unknown): CanonicalJsonValue { if (value === null || typeof value === 'string' || typeof value === 'boolean') return value; - if (typeof value === 'number') return Number.isFinite(value) ? value : null; + if (typeof value === 'number') { + if (!Number.isFinite(value)) { + throw new TeaLoginStoreError( + 'tea-config-invalid', + 'Tea login metadata contains a non-finite number', + ); + } + return value; + } if (Array.isArray(value)) { return value.map((entry: unknown): CanonicalJsonValue => canonicalizeGenerationValue(entry)); } diff --git a/packages/mosaic/src/credentials/team-grant.spec.ts b/packages/mosaic/src/credentials/team-grant.spec.ts index 2dd08c67..0c8bc1d5 100644 --- a/packages/mosaic/src/credentials/team-grant.spec.ts +++ b/packages/mosaic/src/credentials/team-grant.spec.ts @@ -350,6 +350,117 @@ describe('team repository grant', (): void => { expect(repositoryPresent).toBe(true); }); + it('never compensates successor state after lock release begins but release and audit sealing fail', async (): Promise => { + cleanup = await mkdtemp(join(tmpdir(), 'mosaic-team-grant-')); + let repositoryReads = 0; + let memberPresent = false; + let repositoryPresent = false; + let memberRemovals = 0; + let repositoryDetachments = 0; + const provider: GiteaTeamGrantProvider = { + async readBasicIdentity() { + return { + login: 'provisioner', + endpoint: 'GET /api/v1/user', + contentType: 'application/json', + }; + }, + async resolveTeam() { + return { + id: 7, + name: 'writers', + permission: 'write', + endpoint: 'GET /api/v1/orgs/owner/teams', + contentType: 'application/json', + }; + }, + async listTeamRepositories() { + repositoryReads += 1; + return { + repositories: repositoryReads === 1 ? [] : ['owner/repo'], + endpoint: 'GET /api/v1/teams/7/repos', + contentType: 'application/json', + }; + }, + async addTeamMember(): Promise { + memberPresent = true; + }, + async removeTeamMember(): Promise { + memberRemovals += 1; + memberPresent = false; + }, + async attachTeamRepository(): Promise { + repositoryPresent = true; + }, + async detachTeamRepository(): Promise { + repositoryDetachments += 1; + repositoryPresent = false; + }, + async readTeamMember() { + return { + state: memberPresent ? 'present' : 'absent', + endpoint: 'GET /api/v1/teams/7/members/seat-name', + contentType: 'application/json', + }; + }, + async readTeamRepository() { + return { + state: repositoryPresent ? 'present' : 'absent', + endpoint: 'GET /api/v1/teams/7/repos/owner/repo', + contentType: 'application/json', + }; + }, + async readOrganizationMembership() { + return { + state: 'present', + endpoint: 'GET /api/v1/users/seat-name/orgs', + contentType: 'application/json', + }; + }, + }; + const originalSeal = CredentialAuditJournal.prototype.seal; + vi.spyOn(CredentialAuditJournal.prototype, 'seal').mockImplementation(async function ( + this: CredentialAuditJournal, + outcome, + reasonCode, + ): Promise { + if (reasonCode === 'mutation-lock-release-failed') { + throw new CredentialJournalError('journal-unavailable', 'injected release audit failure'); + } + return originalSeal.call(this, outcome, reasonCode); + }); + + await expect( + grantTeamRepositoryPermission( + { + identity: 'seat-name', + estate: 'homelab', + host: 'git.example.invalid', + repo: 'owner/repo', + permission: 'write', + team: 'writers', + readOnlyControlIdentity: 'read-control', + }, + authority, + provider, + validation(), + { + stateRoot: join(cleanup, 'state'), + actor: 'provisioner', + acquireTeamLock: async (): Promise<() => Promise> => async (): Promise => { + memberPresent = true; + repositoryPresent = true; + throw new Error('injected release failure after successor mutation'); + }, + }, + ), + ).rejects.toMatchObject({ code: 'journal-unavailable', mutation: 'applied' }); + expect(memberRemovals).toBe(0); + expect(repositoryDetachments).toBe(0); + expect(memberPresent).toBe(true); + expect(repositoryPresent).toBe(true); + }); + it('refuses a shared team already attached to any repository outside the request', async (): Promise => { cleanup = await mkdtemp(join(tmpdir(), 'mosaic-team-grant-')); let mutated = false; diff --git a/packages/mosaic/src/credentials/team-grant.ts b/packages/mosaic/src/credentials/team-grant.ts index d6fffccb..769dbeb2 100644 --- a/packages/mosaic/src/credentials/team-grant.ts +++ b/packages/mosaic/src/credentials/team-grant.ts @@ -174,7 +174,7 @@ export async function grantTeamRepositoryPermission( await journal.recordIntent('provider-grant'); let mutation: 'none' | 'unknown' | 'applied' = 'none'; let releaseTeamLock: (() => Promise) | undefined; - let teamLockReleased = false; + let teamLockReleaseStarted = false; let rollbackTeam: TeamResolutionEvidence | undefined; let membershipBeforeMutation: PresenceEvidence | undefined; let repositoryAttachedBeforeMutation = false; @@ -187,9 +187,9 @@ export async function grantTeamRepositoryPermission( const release = releaseTeamLock; releaseTeamLock = undefined; if (release !== undefined) { + teamLockReleaseStarted = true; try { await release(); - teamLockReleased = true; } catch { await journal.seal('indeterminate', 'mutation-lock-release-failed'); return { outcome: 'indeterminate', reasonCode: 'mutation-lock-release-failed' }; @@ -427,7 +427,7 @@ export async function grantTeamRepositoryPermission( let compensationError: unknown; try { if ( - !teamLockReleased && + !teamLockReleaseStarted && rollbackTeam !== undefined && membershipBeforeMutation?.state === 'absent' && memberMutationAttempted @@ -446,7 +446,7 @@ export async function grantTeamRepositoryPermission( } } if ( - !teamLockReleased && + !teamLockReleaseStarted && rollbackTeam !== undefined && !repositoryAttachedBeforeMutation && repositoryMutationAttempted @@ -509,6 +509,7 @@ export async function grantTeamRepositoryPermission( const release = releaseTeamLock; releaseTeamLock = undefined; if (release !== undefined) { + teamLockReleaseStarted = true; try { await release(); } catch {