diff --git a/SECURITY.md b/SECURITY.md index 11dce15..35efe4d 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -66,7 +66,7 @@ The findings below describe the repository at the start of the 2026-07-12 assess | RISK-021 | Admin state transitions accept impossible combinations: cancel paid without refund decision, fulfill unpaid/cancelled orders, substitute without new waiver, or comp without proven waiver. | Financial, legal, fulfillment, and waiver records can be inconsistent. | Centralize allowed state transitions and authorization; require reason/evidence and test every forbidden edge. | | RISK-022 | Existing rate-limit documents contain raw IP or email keys and values; the profile-thumbnail slice instead uses a domain-separated SHA-256 account-key pseudonym, which is stable and not anonymous. Fixed windows are distributed only by those identifiers and cleanup depends on an externally configured TTL. The profile-thumbnail upload limiter charges every image-processing invocation, including an exact domain-command retry, because request ID alone cannot prove unchanged image bytes without retaining more image-derived data. | Additional PII or linkable pseudonymous storage, unbounded growth if TTL is absent, identifier-targeted denial of service, retry quota consumption, and easy distributed abuse. | HMAC identifiers with a rotating secret, verify TTL, add per-user/business budgets, monitor rejects/cost, and keep retry side effects limited to abuse-control bookkeeping rather than profile/photo/audit state. | | RISK-023 | The legacy `functions.config().api.key` endpoint uses a static shared key with no replay control, source restriction, App Check, authenticated operator, or request audit. The API is also scheduled for Firebase decommissioning in March 2027. | Key theft permits bulk role changes and future deployment failure. | Retire it in favor of authenticated admin workflow or narrowly authenticated scheduled import; migrate any remaining config to Secret Manager. | -| RISK-024 | OAuth tokens are stored plaintext and browser admins can read them under current rules; refresh writes have no concurrency control. | Token theft exposes member activity data; concurrent refresh can lose a rotated refresh token. | Make secrets server/IAM-only, encrypt where risk analysis requires, transact refresh versions, minimize scopes, and audit access. | +| RISK-024 | OAuth tokens remain plaintext. Repository Rules deny browser access, but protected deployment and live behavior are unproven; least-privilege runtime IAM and an encryption decision remain open. OAUTH-001A1H [#606](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/606) source transactionally rereads the exact access-token, refresh-token, and expiry tuple before persisting a refresh response. A retry rereads the tuple; changed, missing, or malformed state returns one fixed failure without a stale write or downstream bearer call. Synthetic tests use no real token, account, or provider call. Provider refresh calls are still not serialized; identical-tuple replacement, post-commit acknowledgement reconciliation, deployment, and live behavior remain unproven. | Token theft exposes member activity data; an undeployed boundary or an unhandled provider/acknowledgement race can still lose availability or leave local and provider state ambiguous. | Deploy and read back server/IAM-only secret access, decide encryption, serialize or otherwise reconcile refresh operations and lost acknowledgements, minimize scopes, audit access, and verify only with approved made-up staged accounts. | | RISK-025 | Admin authentication relies on password Auth plus a long-lived role token; no repository evidence of MFA, recent-auth checks, re-auth for refunds/role grants, or rapid revocation workflow. | A stolen admin session has broad durable impact. | Require MFA for privileged accounts, recent-auth for sensitive actions, capability roles, short sessions/forced refresh, and break-glass procedures. | | RISK-042 | Profile photos and an officer people finder have no approved privacy notice, retention/backup treatment, scoped capability, or production release path. The existing Admin Members page downloads complete account documents and cannot enforce an opt-out promise. #505/#506 source now supplies the private processed thumbnail, default-off preference, minimum server-only projection, and bounded audited verified-admin name search, but none is deployed. | A photo, name, query, preference, prefix digest, or stable entry reference could be exposed, retained, enumerated, correlated, or mistaken for membership evidence; automated face matching would add unapproved biometric processing. | Keep the feature default-hidden and separate from the official roster. Preserve current-source revalidation, direct Rules denials, fixed query/result bounds, no-store responses, pseudonymous rate keys, query-free audit, and synthetic race/opt-out/removal proof. Never store the original, persist raw queries, or add face recognition. Complete #110 policy inputs, AUTH-003 scoped authorization follow-up, #133 protected authority, and #507 backend-first release/readback before live use. | | RISK-026 | Firestore Admin SDK bypasses rules and the Functions runtime IAM scope is not documented. #135 removes the long-lived service-account JSON path from release source, but the least-privilege short-lived deploy identity is not configured yet. | Server or CI compromise may expose the entire project; missing provider configuration also blocks release. | Complete the private IAM inventory and #133 OIDC/WIF configuration; keep runtime and deploy identities separate; never restore a JSON key shortcut. | diff --git a/functions/strava.js b/functions/strava.js index 5aca757..10f12e6 100644 --- a/functions/strava.js +++ b/functions/strava.js @@ -715,6 +715,12 @@ function snapshotStoredTokenSecret(record) { return objectFreeze({ accessToken, refreshToken, expiresAt }); } +function storedTokenSecretsMatch(left, right) { + return left.accessToken === right.accessToken + && left.refreshToken === right.refreshToken + && left.expiresAt === right.expiresAt; +} + function snapshotStoredDisconnectAccessToken(record) { if (!isPlainJsonRecord(record)) return null; @@ -905,14 +911,24 @@ async function getFreshAccessToken(uid) { } const refreshed = await refreshToken(stored.refreshToken); try { - const secretRef = secretDocRef(uid); - const updatedAt = Timestamp.now(); - await secretRef.set({ - access_token: refreshed.accessToken, - refresh_token: refreshed.refreshToken, - expires_at: refreshed.expiresAt, - updatedAt, - }, { merge: true }); + const db = admin.firestore(); + const secretRef = secretDocRef(uid, db); + const persisted = await db.runTransaction(async (transaction) => { + const currentSnapshot = await transaction.get(secretRef); + if (!currentSnapshot.exists) return false; + const current = snapshotStoredTokenSecret(currentSnapshot.data()); + if (!current || !storedTokenSecretsMatch(current, stored)) return false; + + const updatedAt = Timestamp.now(); + transaction.set(secretRef, { + access_token: refreshed.accessToken, + refresh_token: refreshed.refreshToken, + expires_at: refreshed.expiresAt, + updatedAt, + }, { merge: true }); + return true; + }); + if (persisted !== true) throw stravaRefreshError('internal'); } catch (_error) { throw stravaRefreshError('internal'); } diff --git a/functions/strava.test.js b/functions/strava.test.js index f6bd4ac..eaa8d94 100644 --- a/functions/strava.test.js +++ b/functions/strava.test.js @@ -46,6 +46,9 @@ jest.mock('firebase-admin', () => { const transactionDeletes = []; const transactionGetFailures = new Map(); const transactionReads = []; + const transactionSetAttempts = []; + const transactionSetFailures = new Map(); + const transactionSets = []; let transactionBeforeCommitHook; let transactionRetryHook; let transactionRunAttempts = 0; @@ -70,6 +73,28 @@ jest.mock('firebase-admin', () => { } } + function applyTransactionSet(operation) { + transactionSets.push(operation); + writes.push(operation); + if (operation.options && operation.options.merge === true) { + const current = documents.get(operation.path); + const currentDescriptors = current && typeof current === 'object' + ? Object.getOwnPropertyDescriptors(current) + : {}; + const updateDescriptors = Object.getOwnPropertyDescriptors(operation.data); + Reflect.ownKeys(updateDescriptors).forEach((key) => { + delete currentDescriptors[key]; + }); + documents.set(operation.path, Object.create( + current && typeof current === 'object' ? Object.getPrototypeOf(current) : Object.prototype, + { ...currentDescriptors, ...updateDescriptors }, + )); + } else { + documents.set(operation.path, operation.data); + } + bumpVersion(operation.path); + } + function document(path) { if (documentReferenceFailures.has(path)) { throw documentReferenceFailures.get(path); @@ -181,6 +206,7 @@ jest.mock('firebase-admin', () => { for (let attempt = 0; attempt < 5; attempt += 1) { const readVersions = new Map(); const stagedDeletes = []; + const stagedSets = []; const transaction = { get: async (ref) => { transactionReads.push(ref.path); @@ -204,6 +230,15 @@ jest.mock('firebase-admin', () => { stagedDeletes.push(ref.path); return transaction; }, + set: (ref, data, options) => { + const operation = { path: ref.path, data, options }; + transactionSetAttempts.push(operation); + if (transactionSetFailures.has(ref.path)) { + throw transactionSetFailures.get(ref.path); + } + stagedSets.push(operation); + return transaction; + }, }; const result = await updateFunction(transaction); @@ -223,10 +258,17 @@ jest.mock('firebase-admin', () => { continue; } + for (const operation of stagedSets) { + if (writeFailures.has(operation.path)) { + throw writeFailures.get(operation.path); + } + } + for (const path of stagedDeletes) { transactionDeletes.push(path); applyDelete(path); } + stagedSets.forEach(applyTransactionSet); if (transactionCommitPostApplyFailure) { throw transactionCommitPostApplyFailure; } @@ -277,6 +319,9 @@ jest.mock('firebase-admin', () => { transactionDeletes.splice(0, transactionDeletes.length); transactionGetFailures.clear(); transactionReads.splice(0, transactionReads.length); + transactionSetAttempts.splice(0, transactionSetAttempts.length); + transactionSetFailures.clear(); + transactionSets.splice(0, transactionSets.length); transactionBeforeCommitHook = undefined; transactionRetryHook = undefined; transactionRunAttempts = 0; @@ -298,6 +343,8 @@ jest.mock('firebase-admin', () => { __getTransactionDeletes: () => [...transactionDeletes], __getTransactionReads: () => [...transactionReads], __getTransactionRunAttempts: () => transactionRunAttempts, + __getTransactionSetAttempts: () => [...transactionSetAttempts], + __getTransactionSets: () => [...transactionSets], __getWrites: () => [...writes], __hasDocument: (path) => documents.has(path), __setBatchCommitFailure: (error) => { @@ -344,6 +391,9 @@ jest.mock('firebase-admin', () => { __setTransactionRunFailure: (error) => { transactionRunFailure = error; }, + __setTransactionSetFailure: (path, error) => { + transactionSetFailures.set(path, error); + }, __setWriteFailure: (path, error) => writeFailures.set(path, error), }; }); @@ -2543,6 +2593,19 @@ describe('Strava token refresh failure boundary', () => { }; } + function expectedRefreshWrite() { + return { + path: SECRET_PATH, + data: { + access_token: 'refreshed_access_token_test', + refresh_token: 'refreshed_refresh_token_test', + expires_at: 1_900_000_000, + updatedAt: expect.any(Object), + }, + options: { merge: true }, + }; + } + function mockRefreshResponse(response) { const json = jest.fn().mockResolvedValue(response); fetchMock.mockResolvedValueOnce({ ok: true, json }); @@ -3056,6 +3119,191 @@ describe('Strava token refresh failure boundary', () => { }); }); + describe('OAUTH-001A1H stale refreshed-secret persistence boundary', () => { + function expectRefusedMutation(rejection) { + expect(publicError(rejection)).toEqual({ + code: 'internal', + message: FIXED_REFRESH_ERROR, + details: undefined, + cause: undefined, + }); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(admin.__getReads()).toEqual([CONNECTION_PATH, SECRET_PATH]); + expect(admin.__getDirectSetAttempts()).toEqual([]); + expect(admin.__getTransactionRunAttempts()).toBe(1); + expect(admin.__getTransactionReads()).toEqual([SECRET_PATH]); + expect(admin.__getTransactionSetAttempts()).toEqual([]); + expect(admin.__getTransactionSets()).toEqual([]); + expect(admin.__getWrites()).toEqual([]); + expect(Timestamp.now).not.toHaveBeenCalled(); + consoleSpies.forEach((spy) => expect(spy).not.toHaveBeenCalled()); + } + + test('refuses an older refresh response after the stored token tuple rotates', async () => { + const newerSecret = Object.freeze({ + access_token: 'newer_access_token_test', + refresh_token: 'newer_refresh_token_test', + expires_at: 1_950_000_000, + scope: 'read', + }); + seedExpiredConnection(); + const json = jest.fn().mockImplementation(async () => { + admin.__setDocument(SECRET_PATH, newerSecret); + return validRefreshResponse(); + }); + fetchMock + .mockResolvedValueOnce({ ok: true, json }) + .mockResolvedValueOnce({ + ok: true, + json: jest.fn().mockResolvedValue([]), + }) + .mockResolvedValueOnce({ + ok: true, + json: jest.fn().mockResolvedValue({ + ytd_run_totals: { distance: 0, count: 0 }, + ytd_ride_totals: { distance: 0, count: 0 }, + all_run_totals: { distance: 0, count: 0 }, + }), + }); + + const rejection = await captureFailure(() => stravaFetchStats({}, CONTEXT)); + + expectRefusedMutation(rejection); + expect(json).toHaveBeenCalledTimes(1); + expect(admin.__getDocument(SECRET_PATH)).toBe(newerSecret); + }); + + test.each([ + ['access token', 'access_token', 'different_access_token_test'], + ['refresh token', 'refresh_token', 'different_refresh_token_test'], + ['expiry', 'expires_at', 1_950_000_000], + ])('refuses when only the stored %s changes during provider refresh', async ( + _case, + field, + value, + ) => { + const changedSecret = Object.freeze({ ...EXPIRED_SECRET, [field]: value }); + seedExpiredConnection(); + const json = jest.fn().mockImplementation(async () => { + admin.__setDocument(SECRET_PATH, changedSecret); + return validRefreshResponse(); + }); + fetchMock.mockResolvedValueOnce({ ok: true, json }); + + const rejection = await captureFailure(() => stravaFetchStats({}, CONTEXT)); + + expectRefusedMutation(rejection); + expect(json).toHaveBeenCalledTimes(1); + expect(admin.__getDocument(SECRET_PATH)).toBe(changedSecret); + }); + + test('refuses when the stored secret is deleted during provider refresh', async () => { + seedExpiredConnection(); + const json = jest.fn().mockImplementation(async () => { + admin.__removeDocument(SECRET_PATH); + return validRefreshResponse(); + }); + fetchMock.mockResolvedValueOnce({ ok: true, json }); + + const rejection = await captureFailure(() => stravaFetchStats({}, CONTEXT)); + + expectRefusedMutation(rejection); + expect(json).toHaveBeenCalledTimes(1); + expect(admin.__hasDocument(SECRET_PATH)).toBe(false); + }); + + test('refuses malformed replacement state without inspecting unknown fields', async () => { + const unknownGetter = jest.fn(() => { + throw new Error('stale-refresh-malformed-unknown-canary'); + }); + const malformedSecret = { access_token: 'replacement_access_token_test' }; + Object.defineProperty(malformedSecret, 'provider_debug', { + configurable: true, + enumerable: true, + get: unknownGetter, + }); + seedExpiredConnection(); + const json = jest.fn().mockImplementation(async () => { + admin.__setDocument(SECRET_PATH, malformedSecret); + return validRefreshResponse(); + }); + fetchMock.mockResolvedValueOnce({ ok: true, json }); + + const rejection = await captureFailure(() => stravaFetchStats({}, CONTEXT)); + + expectRefusedMutation(rejection); + expect(json).toHaveBeenCalledTimes(1); + expect(unknownGetter).not.toHaveBeenCalled(); + expect(admin.__getDocument(SECRET_PATH)).toBe(malformedSecret); + }); + + test('rechecks and refuses changed state after a transaction conflict', async () => { + const newerSecret = Object.freeze({ + access_token: 'conflicting_access_token_test', + refresh_token: 'conflicting_refresh_token_test', + expires_at: 1_960_000_000, + scope: 'read', + }); + const retryHook = jest.fn(); + let beforeCommitCalls = 0; + seedExpiredConnection(); + mockRefreshResponse(validRefreshResponse()); + admin.__setTransactionBeforeCommitHook(() => { + beforeCommitCalls += 1; + if (beforeCommitCalls === 1) { + admin.__setDocument(SECRET_PATH, newerSecret); + } + }); + admin.__setTransactionRetryHook(retryHook); + + const rejection = await captureFailure(() => stravaFetchStats({}, CONTEXT)); + + expect(publicError(rejection)).toEqual({ + code: 'internal', + message: FIXED_REFRESH_ERROR, + details: undefined, + cause: undefined, + }); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(admin.__getDirectSetAttempts()).toEqual([]); + expect(admin.__getTransactionRunAttempts()).toBe(1); + expect(admin.__getTransactionReads()).toEqual([SECRET_PATH, SECRET_PATH]); + expect(admin.__getTransactionSetAttempts()).toEqual([expectedRefreshWrite()]); + expect(admin.__getTransactionSets()).toEqual([]); + expect(admin.__getWrites()).toEqual([]); + expect(admin.__getDocument(SECRET_PATH)).toBe(newerSecret); + expect(retryHook).toHaveBeenCalledTimes(1); + expect(Timestamp.now).toHaveBeenCalledTimes(1); + consoleSpies.forEach((spy) => expect(spy).not.toHaveBeenCalled()); + }); + + test('persists once and continues when the stored tuple is unchanged', async () => { + seedExpiredConnection(); + mockValidRefreshFlow(); + + const result = await stravaFetchStats({}, CONTEXT); + + const expectedWrite = expectedRefreshWrite(); + expect(result).toMatchObject({ connected: true }); + expect(fetchMock).toHaveBeenCalledTimes(3); + expect(admin.__getDirectSetAttempts()).toEqual([]); + expect(admin.__getTransactionRunAttempts()).toBe(1); + expect(admin.__getTransactionReads()).toEqual([SECRET_PATH]); + expect(admin.__getTransactionSetAttempts()).toEqual([expectedWrite]); + expect(admin.__getTransactionSets()).toEqual([expectedWrite]); + expect(admin.__getWrites()).toEqual([expectedWrite]); + expect(admin.__getDocument(SECRET_PATH)).toEqual({ + ...EXPIRED_SECRET, + access_token: 'refreshed_access_token_test', + refresh_token: 'refreshed_refresh_token_test', + expires_at: 1_900_000_000, + updatedAt: expect.any(Object), + }); + expect(Timestamp.now).toHaveBeenCalledTimes(1); + consoleSpies.forEach((spy) => expect(spy).not.toHaveBeenCalled()); + }); + }); + describe('OAUTH-001A1E refreshed-secret persistence boundary', () => { function hostilePersistenceFailure() { const probes = [ @@ -3080,19 +3328,6 @@ describe('Strava token refresh failure boundary', () => { return { failure, probes }; } - function expectedRefreshWrite() { - return { - path: SECRET_PATH, - data: { - access_token: 'refreshed_access_token_test', - refresh_token: 'refreshed_refresh_token_test', - expires_at: 1_900_000_000, - updatedAt: expect.any(Object), - }, - options: { merge: true }, - }; - } - function expectFixedPersistenceFailure(rejection, probes) { const exposed = publicError(rejection); expect(exposed).toEqual({ @@ -3128,8 +3363,7 @@ describe('Strava token refresh failure boundary', () => { expect(admin.__getBatchSetAttempts()).toEqual([]); expect(admin.__getBatchCommitAttempts()).toEqual([]); expect(admin.__getDeletes()).toEqual([]); - expect(admin.__getTransactionRunAttempts()).toBe(0); - expect(admin.__getTransactionReads()).toEqual([]); + expect(admin.__getDirectSetAttempts()).toEqual([]); expect(admin.__getTransactionDeleteAttempts()).toEqual([]); consoleSpies.forEach((spy) => expect(spy).not.toHaveBeenCalled()); } @@ -3146,7 +3380,10 @@ describe('Strava token refresh failure boundary', () => { expectFixedPersistenceFailure(rejection, probes); expect(Timestamp.now).toHaveBeenCalledTimes(1); - expect(admin.__getDirectSetAttempts()).toEqual([]); + expect(admin.__getTransactionRunAttempts()).toBe(1); + expect(admin.__getTransactionReads()).toEqual([SECRET_PATH]); + expect(admin.__getTransactionSetAttempts()).toEqual([]); + expect(admin.__getTransactionSets()).toEqual([]); expect(admin.__getWrites()).toEqual([]); expect(admin.__getDocument(CONNECTION_PATH)).toBe(CONNECTION); expect(admin.__getDocument(SECRET_PATH)).toBe(EXPIRED_SECRET); @@ -3166,12 +3403,87 @@ describe('Strava token refresh failure boundary', () => { expectFixedPersistenceFailure(rejection, probes); expect(json).toHaveBeenCalledTimes(1); expect(Timestamp.now).not.toHaveBeenCalled(); - expect(admin.__getDirectSetAttempts()).toEqual([]); + expect(admin.__getTransactionRunAttempts()).toBe(0); + expect(admin.__getTransactionReads()).toEqual([]); + expect(admin.__getTransactionSetAttempts()).toEqual([]); + expect(admin.__getTransactionSets()).toEqual([]); expect(admin.__getWrites()).toEqual([]); expect(admin.__getDocument(CONNECTION_PATH)).toBe(CONNECTION); expect(admin.__getDocument(SECRET_PATH)).toBe(EXPIRED_SECRET); }); + test('maps a transaction-start rejection to one fixed result', async () => { + const { failure, probes } = hostilePersistenceFailure(); + seedExpiredConnection(); + mockRefreshResponse(validRefreshResponse()); + admin.__setTransactionRunFailure(failure); + + const rejection = await captureFailure(() => stravaFetchStats({}, CONTEXT)); + + expectFixedPersistenceFailure(rejection, probes); + expect(Timestamp.now).not.toHaveBeenCalled(); + expect(admin.__getTransactionRunAttempts()).toBe(1); + expect(admin.__getTransactionReads()).toEqual([]); + expect(admin.__getTransactionSetAttempts()).toEqual([]); + expect(admin.__getTransactionSets()).toEqual([]); + expect(admin.__getWrites()).toEqual([]); + expect(admin.__getDocument(SECRET_PATH)).toBe(EXPIRED_SECRET); + }); + + test('maps a transaction-read rejection to one fixed result', async () => { + const { failure, probes } = hostilePersistenceFailure(); + seedExpiredConnection(); + mockRefreshResponse(validRefreshResponse()); + admin.__setTransactionGetFailure(SECRET_PATH, failure); + + const rejection = await captureFailure(() => stravaFetchStats({}, CONTEXT)); + + expectFixedPersistenceFailure(rejection, probes); + expect(Timestamp.now).not.toHaveBeenCalled(); + expect(admin.__getTransactionRunAttempts()).toBe(1); + expect(admin.__getTransactionReads()).toEqual([SECRET_PATH]); + expect(admin.__getTransactionSetAttempts()).toEqual([]); + expect(admin.__getTransactionSets()).toEqual([]); + expect(admin.__getWrites()).toEqual([]); + expect(admin.__getDocument(SECRET_PATH)).toBe(EXPIRED_SECRET); + }); + + test('maps a transaction-set staging rejection to one fixed result', async () => { + const { failure, probes } = hostilePersistenceFailure(); + seedExpiredConnection(); + mockRefreshResponse(validRefreshResponse()); + admin.__setTransactionSetFailure(SECRET_PATH, failure); + + const rejection = await captureFailure(() => stravaFetchStats({}, CONTEXT)); + + expectFixedPersistenceFailure(rejection, probes); + expect(Timestamp.now).toHaveBeenCalledTimes(1); + expect(admin.__getTransactionRunAttempts()).toBe(1); + expect(admin.__getTransactionReads()).toEqual([SECRET_PATH]); + expect(admin.__getTransactionSetAttempts()).toEqual([expectedRefreshWrite()]); + expect(admin.__getTransactionSets()).toEqual([]); + expect(admin.__getWrites()).toEqual([]); + expect(admin.__getDocument(SECRET_PATH)).toBe(EXPIRED_SECRET); + }); + + test('maps a transaction-commit rejection to one fixed result', async () => { + const { failure, probes } = hostilePersistenceFailure(); + seedExpiredConnection(); + mockRefreshResponse(validRefreshResponse()); + admin.__setTransactionCommitFailure(failure); + + const rejection = await captureFailure(() => stravaFetchStats({}, CONTEXT)); + + expectFixedPersistenceFailure(rejection, probes); + expect(Timestamp.now).toHaveBeenCalledTimes(1); + expect(admin.__getTransactionRunAttempts()).toBe(1); + expect(admin.__getTransactionReads()).toEqual([SECRET_PATH]); + expect(admin.__getTransactionSetAttempts()).toEqual([expectedRefreshWrite()]); + expect(admin.__getTransactionSets()).toEqual([]); + expect(admin.__getWrites()).toEqual([]); + expect(admin.__getDocument(SECRET_PATH)).toBe(EXPIRED_SECRET); + }); + test('maps a pre-apply secret-set rejection to one fixed result', async () => { const { failure, probes } = hostilePersistenceFailure(); seedExpiredConnection(); @@ -3182,7 +3494,10 @@ describe('Strava token refresh failure boundary', () => { expectFixedPersistenceFailure(rejection, probes); expect(Timestamp.now).toHaveBeenCalledTimes(1); - expect(admin.__getDirectSetAttempts()).toEqual([expectedRefreshWrite()]); + expect(admin.__getTransactionRunAttempts()).toBe(1); + expect(admin.__getTransactionReads()).toEqual([SECRET_PATH]); + expect(admin.__getTransactionSetAttempts()).toEqual([expectedRefreshWrite()]); + expect(admin.__getTransactionSets()).toEqual([]); expect(admin.__getWrites()).toEqual([]); expect(admin.__getDocument(CONNECTION_PATH)).toBe(CONNECTION); expect(admin.__getDocument(SECRET_PATH)).toBe(EXPIRED_SECRET); @@ -3192,14 +3507,17 @@ describe('Strava token refresh failure boundary', () => { const { failure, probes } = hostilePersistenceFailure(); seedExpiredConnection(); mockRefreshResponse(validRefreshResponse()); - admin.__setDirectSetPostApplyFailure(SECRET_PATH, failure); + admin.__setTransactionCommitPostApplyFailure(failure); const rejection = await captureFailure(() => stravaFetchStats({}, CONTEXT)); expectFixedPersistenceFailure(rejection, probes); const expectedWrite = expectedRefreshWrite(); expect(Timestamp.now).toHaveBeenCalledTimes(1); - expect(admin.__getDirectSetAttempts()).toEqual([expectedWrite]); + expect(admin.__getTransactionRunAttempts()).toBe(1); + expect(admin.__getTransactionReads()).toEqual([SECRET_PATH]); + expect(admin.__getTransactionSetAttempts()).toEqual([expectedWrite]); + expect(admin.__getTransactionSets()).toEqual([expectedWrite]); expect(admin.__getWrites()).toEqual([expectedWrite]); expect(admin.__getDocument(CONNECTION_PATH)).toBe(CONNECTION); expect(admin.__getDocument(SECRET_PATH)).toEqual({ @@ -3654,7 +3972,7 @@ describe('Strava token refresh failure boundary', () => { consoleSpies.forEach((spy) => expect(spy).not.toHaveBeenCalled()); }); - test('uses the refresh projection after the raw record mutates during clock access', async () => { + test('uses the refresh projection then refuses a raw stored tuple mutation', async () => { const secret = validStoredSecret(); secret.expires_at = NOW_SECONDS + 60; seedStoredSecret(secret); @@ -3666,8 +3984,14 @@ describe('Strava token refresh failure boundary', () => { }); mockValidRefreshFlow(); - await stravaFetchStats({}, CONTEXT); + const rejection = await captureFailure(() => stravaFetchStats({}, CONTEXT)); + expect(publicError(rejection)).toEqual({ + code: 'internal', + message: FIXED_REFRESH_ERROR, + details: undefined, + cause: undefined, + }); expect(fetchMock).toHaveBeenNthCalledWith( 1, 'https://www.strava.com/api/v3/oauth/token', @@ -3682,19 +4006,15 @@ describe('Strava token refresh failure boundary', () => { }), }, ); - expect(admin.__getWrites()).toEqual([{ - path: SECRET_PATH, - data: { - access_token: 'refreshed_access_token_test', - refresh_token: 'refreshed_refresh_token_test', - expires_at: 1_900_000_000, - updatedAt: expect.any(Object), - }, - options: { merge: true }, - }]); - expect(fetchMock).toHaveBeenCalledTimes(3); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(admin.__getDirectSetAttempts()).toEqual([]); + expect(admin.__getTransactionRunAttempts()).toBe(1); + expect(admin.__getTransactionReads()).toEqual([SECRET_PATH]); + expect(admin.__getTransactionSetAttempts()).toEqual([]); + expect(admin.__getTransactionSets()).toEqual([]); + expect(admin.__getWrites()).toEqual([]); expect(dateNowSpy).toHaveBeenCalledTimes(1); - expect(Timestamp.now).toHaveBeenCalledTimes(1); + expect(Timestamp.now).not.toHaveBeenCalled(); consoleSpies.forEach((spy) => expect(spy).not.toHaveBeenCalled()); });