diff --git a/cloudflare/motorsignal-r2-worker/src/index.ts b/cloudflare/motorsignal-r2-worker/src/index.ts index 1523f28..33fc3ac 100644 --- a/cloudflare/motorsignal-r2-worker/src/index.ts +++ b/cloudflare/motorsignal-r2-worker/src/index.ts @@ -156,6 +156,7 @@ export default { if (!remoteRideId || !isValidRideId(remoteRideId)) return jsonResponse({ error: 'Invalid rideId parameter' }, 400); const ride = await fetchOwnedRide(projectId, authenticatedUid, remoteRideId, idToken); + if (ride === 'temporary-error') return jsonResponse({ error: 'Firestore temporary lookup error' }, 503); if (ride === 'unauthorized') return jsonResponse({ error: 'Ride ownership could not be authorized' }, 403); if (ride === 'not-found') return jsonResponse({ error: 'Ride not found' }, 404); if (ride === 'invalid') return jsonResponse({ error: 'Ride metadata is invalid' }, 409); @@ -181,6 +182,7 @@ export default { if (!remoteRideId || !isValidRideId(remoteRideId)) return jsonResponse({ error: 'Invalid rideId parameter' }, 400); const ride = await fetchOwnedRide(projectId, authenticatedUid, remoteRideId, idToken); + if (ride === 'temporary-error') return jsonResponse({ error: 'Firestore temporary lookup error', code: 'TEMPORARY_ERROR' }, 503); if (ride === 'unauthorized') return jsonResponse({ error: 'Ride ownership could not be authorized', code: 'UNAUTHORIZED' }, 403); if (ride === 'not-found') return jsonResponse({ error: 'Ride not found', code: 'NOT_FOUND' }, 404); if (ride === 'invalid') return jsonResponse({ error: 'Ride metadata is invalid', code: 'INVALID_METADATA' }, 409); @@ -204,7 +206,8 @@ export default { return new Response(null, { status: 200, headers }); } - return jsonResponse({ + headers.set('Content-Type', 'application/json'); + return new Response(JSON.stringify({ verified: true, remoteRideId, objectKey, @@ -213,7 +216,7 @@ export default { sha256, appVersion, uploadedAt, - }, 200); + }), { status: 200, headers }); } return jsonResponse({ error: 'Endpoint not found' }, 404); @@ -241,7 +244,7 @@ async function verifyFirebaseToken(token: string, expectedProjectId: string): Pr } interface OwnedRide { objectKey?: string; } -type RideAuthorizationResult = OwnedRide | 'unauthorized' | 'not-found' | 'invalid'; +type RideAuthorizationResult = OwnedRide | 'unauthorized' | 'not-found' | 'invalid' | 'temporary-error'; /** Reads the authenticated user's ride document through Firestore REST. */ async function fetchOwnedRide(projectId: string, uid: string, remoteRideId: string, idToken: string): Promise { @@ -251,6 +254,7 @@ async function fetchOwnedRide(projectId: string, uid: string, remoteRideId: stri if (response.status === 401 || response.status === 403) return 'unauthorized'; if (response.status === 404) return 'not-found'; + if (response.status === 429 || response.status >= 500) return 'temporary-error'; if (!response.ok) { console.error('Firestore ownership lookup failed', response.status); return 'unauthorized'; diff --git a/cloudflare/motorsignal-r2-worker/src/r2-verify.test.ts b/cloudflare/motorsignal-r2-worker/src/r2-verify.test.ts index f342a35..d14a9ec 100644 --- a/cloudflare/motorsignal-r2-worker/src/r2-verify.test.ts +++ b/cloudflare/motorsignal-r2-worker/src/r2-verify.test.ts @@ -337,4 +337,38 @@ describe('Worker /r2/verify endpoint', () => { const body = await res.json() as any; expect(body.error).toBe('Internal Worker Exception'); }); + + it('19. returns 503 TEMPORARY_ERROR when Firestore ownership lookup fails with 429 or 503', async () => { + mockFirestore(503, { error: 'Service Unavailable' }); + + const req = new Request('https://worker.local/r2/verify?rideId=ride_abc_123', { + method: 'GET', + headers: { Authorization: 'Bearer valid_token' }, + }); + const res = await worker.fetch(req, mockEnv); + expect(res.status).toBe(503); + const body = await res.json() as any; + expect(body.code).toBe('TEMPORARY_ERROR'); + expect(body.error).toContain('Firestore temporary lookup error'); + }); + + it('20. sets X-MotorSignal-* headers on GET as well as HEAD', async () => { + mockValidFirestore(); + mockR2Head.mockResolvedValue({ + size: 2048, + httpEtag: '"etag-2048"', + customMetadata: { sha256: 'sha256_get_test' }, + }); + + const req = new Request('https://worker.local/r2/verify?rideId=ride_abc_123', { + method: 'GET', + headers: { Authorization: 'Bearer valid_token' }, + }); + const res = await worker.fetch(req, mockEnv); + expect(res.status).toBe(200); + expect(res.headers.get('X-MotorSignal-Ride-Id')).toBe('ride_abc_123'); + expect(res.headers.get('X-MotorSignal-Sha256')).toBe('sha256_get_test'); + expect(res.headers.get('X-MotorSignal-Size')).toBe('2048'); + expect(res.headers.get('etag')).toBe('"etag-2048"'); + }); }); diff --git a/lib/services/sync/firebase_sync_provider.dart b/lib/services/sync/firebase_sync_provider.dart index 3a7db17..bacfbfa 100644 --- a/lib/services/sync/firebase_sync_provider.dart +++ b/lib/services/sync/firebase_sync_provider.dart @@ -209,13 +209,30 @@ class FirebaseSyncProvider implements SyncProvider { ); } - final r2Result = await _r2UploadService.verifyTelemetryBundle( + var r2Result = await _r2UploadService.verifyTelemetryBundle( idToken: idToken, remoteRideId: remoteRideId, expectedSha256: expectedSha256, expectedSize: expectedSize?.toInt(), ); + // If verification is rejected as unauthorized (e.g. expired cached token), + // force refresh the ID token once and retry before concluding unauthorized. + if (r2Result.status == CloudConfirmationStatus.unauthorized) { + final refreshedToken = await _authService.getIdToken(true); + if (refreshedToken != null && + refreshedToken.isNotEmpty && + refreshedToken != idToken) { + idToken = refreshedToken; + r2Result = await _r2UploadService.verifyTelemetryBundle( + idToken: idToken, + remoteRideId: remoteRideId, + expectedSha256: expectedSha256, + expectedSize: expectedSize?.toInt(), + ); + } + } + switch (r2Result.status) { case CloudConfirmationStatus.confirmed: return CloudConfirmationResult.confirmed( diff --git a/lib/services/sync/r2_upload_service.dart b/lib/services/sync/r2_upload_service.dart index 9013673..f32dd42 100644 --- a/lib/services/sync/r2_upload_service.dart +++ b/lib/services/sync/r2_upload_service.dart @@ -267,6 +267,16 @@ class R2UploadService { /// Lightweight verification of telemetry bundle storage and integrity in Cloudflare R2. /// Queries the worker via /r2/verify to verify existence, metadata, and hash match /// without downloading the full ZIP archive. + /// + /// CRITICAL ARCHITECTURAL DISTINCTION: + /// R2 HEAD / metadata checks prove that an object exists in Cloudflare R2 and that its stored + /// metadata (SHA-256 hash, byte size, object key, remote ride identity) matches the expected + /// Firestore and local contract. + /// They do NOT independently prove that the stored ZIP archive's binary payload is readable, + /// decompresses cleanly, or is uncorrupted at rest. Full bundle downloads on startup are + /// intentionally avoided to protect mobile bandwidth and startup latency. For future stronger + /// verification, background sampling or periodic full restore-integrity checks should be + /// scheduled rather than downloading every bundle on every launch or reconciliation pass. Future verifyTelemetryBundle({ required String idToken, required String remoteRideId, diff --git a/lib/services/sync/sync_manager.dart b/lib/services/sync/sync_manager.dart index b24f452..a3e1021 100644 --- a/lib/services/sync/sync_manager.dart +++ b/lib/services/sync/sync_manager.dart @@ -345,13 +345,6 @@ class SyncManager extends ChangeNotifier with WidgetsBindingObserver { continue; } - // Claim unclaimed ride if user is authenticated and target is firebase - if (ride.cloudOwnerUid == null && - isAuthenticated && - providerId == 'firebase') { - await _db.claimRideForCloudOwner(ride.id, user.uid); - } - // Stable cloud identity check final remoteRideId = await _db.ensureCloudRideId(ride.id); @@ -364,6 +357,11 @@ class SyncManager extends ChangeNotifier with WidgetsBindingObserver { switch (confirmation.status) { case CloudConfirmationStatus.confirmed: + if (ride.cloudOwnerUid == null && + isAuthenticated && + providerId == 'firebase') { + await _db.claimRideForCloudOwner(ride.id, user.uid); + } final syncedItem = SyncItem( localRideId: ride.id, providerId: providerId, @@ -377,6 +375,11 @@ class SyncManager extends ChangeNotifier with WidgetsBindingObserver { break; case CloudConfirmationStatus.legacyUnverified: + if (ride.cloudOwnerUid == null && + isAuthenticated && + providerId == 'firebase') { + await _db.claimRideForCloudOwner(ride.id, user.uid); + } final legacySyncedItem = SyncItem( localRideId: ride.id, providerId: providerId, @@ -409,6 +412,11 @@ class SyncManager extends ChangeNotifier with WidgetsBindingObserver { await _outbox.update(authReqItem); recoveredAuthRequired++; } else { + if (ride.cloudOwnerUid == null && + isAuthenticated && + providerId == 'firebase') { + await _db.claimRideForCloudOwner(ride.id, user.uid); + } final queuedItem = SyncItem( localRideId: ride.id, providerId: providerId, @@ -544,11 +552,11 @@ class SyncManager extends ChangeNotifier with WidgetsBindingObserver { } Future _performProcessQueue() async { - do { - _needsAnotherPass = false; - _isProcessing = true; - notifyListeners(); - try { + _isProcessing = true; + notifyListeners(); + try { + do { + _needsAnotherPass = false; final now = _now(); for (final item in _outbox.getEligibleItems(now)) { final provider = _providers[item.providerId]; @@ -684,7 +692,10 @@ class SyncManager extends ChangeNotifier with WidgetsBindingObserver { } // 4. In-flight cancellation protection: verify item was not cancelled while uploading - final currentItem = _outbox.getItem(item.localRideId, item.providerId); + final currentItem = _outbox.getItem( + item.localRideId, + item.providerId, + ); if (currentItem?.status == SyncStatus.cancelled) { continue; } @@ -727,11 +738,11 @@ class SyncManager extends ChangeNotifier with WidgetsBindingObserver { ); } } - } finally { - _isProcessing = false; - notifyListeners(); - } - } while (_needsAnotherPass); + } while (_needsAnotherPass); + } finally { + _isProcessing = false; + notifyListeners(); + } } Future cancelSync(int localRideId, String providerId) async { diff --git a/test/cloud_confirmation_contract_test.dart b/test/cloud_confirmation_contract_test.dart index dce373c..67205ca 100644 --- a/test/cloud_confirmation_contract_test.dart +++ b/test/cloud_confirmation_contract_test.dart @@ -18,6 +18,8 @@ class _MockAuthService implements AuthService { AuthUser? _currentUser; AuthState _authState = const AuthState.unauthenticated(); String? idTokenToReturn = 'valid_mock_jwt_token'; + String? refreshedIdToken; + int getIdTokenCalls = 0; void setAuthenticated(String uid, String email) { _currentUser = AuthUser(uid: uid, email: email, isAnonymous: false); @@ -45,8 +47,14 @@ class _MockAuthService implements AuthService { @override bool get isEmailVerified => true; @override - Future getIdToken([bool forceRefresh = false]) async => - idTokenToReturn; + Future getIdToken([bool forceRefresh = false]) async { + getIdTokenCalls++; + if (forceRefresh && refreshedIdToken != null) { + return refreshedIdToken; + } + return idTokenToReturn; + } + @override Future createUserWithEmailAndPassword( String email, @@ -624,7 +632,8 @@ void main() { jsonEncode({ 'verified': true, 'remoteRideId': actualRemoteRideId, - 'objectKey': 'users/$testUid/rides/$actualRemoteRideId/telemetry.zip', + 'objectKey': + 'users/$testUid/rides/$actualRemoteRideId/telemetry.zip', 'size': validSize, 'sha256': 'corrupted_hash_mismatch_123', }), @@ -657,7 +666,12 @@ void main() { expect(result.success, isFalse); expect(result.isRetryable, isFalse); expect(result.errorMessage, contains('inconsistent')); - expect(uploadAttempted, isFalse, reason: 'Must NEVER attempt R2 upload when cloud data is inconsistent'); + expect( + uploadAttempted, + isFalse, + reason: + 'Must NEVER attempt R2 upload when cloud data is inconsistent', + ); }, ); @@ -709,7 +723,11 @@ void main() { expect(result.success, isFalse); expect(result.isRetryable, isTrue); expect(result.errorMessage, contains('temporarily unavailable')); - expect(uploadAttempted, isFalse, reason: 'Must NOT upload bundle when verification is rate limited'); + expect( + uploadAttempted, + isFalse, + reason: 'Must NOT upload bundle when verification is rate limited', + ); }, ); @@ -728,7 +746,12 @@ void main() { .doc(testUid) .collection('rides') .doc(actualRemoteRideId) - .set(createFirestoreRideData(ownerId: 'different_owner_uid', rideId: actualRemoteRideId)); + .set( + createFirestoreRideData( + ownerId: 'different_owner_uid', + rideId: actualRemoteRideId, + ), + ); var uploadAttempted = false; final mockClient = http_testing.MockClient((request) async { @@ -781,7 +804,9 @@ void main() { }, }); - final mockClient = http_testing.MockClient((request) async => http.Response('{}', 200)); + final mockClient = http_testing.MockClient( + (request) async => http.Response('{}', 200), + ); final r2Service = R2UploadService(client: mockClient); final provider = FirebaseSyncProvider( db: db, @@ -820,7 +845,9 @@ void main() { }, }); - final mockClient = http_testing.MockClient((request) async => http.Response('{}', 200)); + final mockClient = http_testing.MockClient( + (request) async => http.Response('{}', 200), + ); final r2Service = R2UploadService(client: mockClient); final provider = FirebaseSyncProvider( db: db, @@ -931,7 +958,8 @@ void main() { 'ownerId': testUid, 'telemetryBundle': { 'provider': 'cloudflare-r2', - 'objectKey': 'users/$testUid/rides/$actualRemoteRideId/telemetry.zip', + 'objectKey': + 'users/$testUid/rides/$actualRemoteRideId/telemetry.zip', 'status': 'uploaded', }, }); @@ -943,7 +971,8 @@ void main() { jsonEncode({ 'verified': true, 'remoteRideId': actualRemoteRideId, - 'objectKey': 'users/$testUid/rides/$actualRemoteRideId/telemetry.zip', + 'objectKey': + 'users/$testUid/rides/$actualRemoteRideId/telemetry.zip', 'size': 1024, 'sha256': 'legacy_hash', }), @@ -974,7 +1003,105 @@ void main() { expect(result.success, isTrue); expect(result.remoteId, equals(actualRemoteRideId)); - expect(uploadAttempted, isFalse, reason: 'Must not re-upload existing historical bundle'); + expect( + uploadAttempted, + isFalse, + reason: 'Must not re-upload existing historical bundle', + ); + }, + ); + + test( + '23. Expired Firebase token triggers forceRefresh and confirms on retry', + () async { + await fakeFirestore + .collection('users') + .doc(testUid) + .collection('rides') + .doc(remoteRideId) + .set(createFirestoreRideData()); + + authService.idTokenToReturn = 'expired_old_token'; + authService.refreshedIdToken = 'fresh_valid_jwt_token'; + + var callCount = 0; + final mockClient = http_testing.MockClient((request) async { + callCount++; + final authHeader = request.headers['Authorization']; + if (authHeader == 'Bearer expired_old_token') { + return http.Response( + jsonEncode({'error': 'Token expired', 'code': 'UNAUTHORIZED'}), + 401, + ); + } + if (authHeader == 'Bearer fresh_valid_jwt_token') { + return http.Response( + jsonEncode({ + 'verified': true, + 'remoteRideId': remoteRideId, + 'objectKey': 'users/$testUid/rides/$remoteRideId/telemetry.zip', + 'size': validSize, + 'sha256': validSha256, + }), + 200, + ); + } + return http.Response('{}', 403); + }); + + final r2Service = R2UploadService(client: mockClient); + final provider = FirebaseSyncProvider( + db: db, + authService: authService, + firestore: fakeFirestore, + r2UploadService: r2Service, + ); + + final result = await provider.verifyCloudConfirmation(remoteRideId); + expect(result.status, equals(CloudConfirmationStatus.confirmed)); + expect(result.isConfirmed, isTrue); + expect( + callCount, + equals(2), + reason: 'Should have retried once with refreshed token', + ); + }, + ); + + test( + '24. Worker returns 503 TEMPORARY_ERROR when Firestore temporarily fails (maps to temporarilyUnverifiable)', + () async { + await fakeFirestore + .collection('users') + .doc(testUid) + .collection('rides') + .doc(remoteRideId) + .set(createFirestoreRideData()); + + final mockClient = http_testing.MockClient((request) async { + return http.Response( + jsonEncode({ + 'error': 'Firestore temporary lookup error', + 'code': 'TEMPORARY_ERROR', + }), + 503, + ); + }); + + final r2Service = R2UploadService(client: mockClient); + final provider = FirebaseSyncProvider( + db: db, + authService: authService, + firestore: fakeFirestore, + r2UploadService: r2Service, + ); + + final result = await provider.verifyCloudConfirmation(remoteRideId); + expect( + result.status, + equals(CloudConfirmationStatus.temporarilyUnverifiable), + ); + expect(result.isConfirmed, isFalse); }, ); }); diff --git a/test/completed_ride_sync_recovery_test.dart b/test/completed_ride_sync_recovery_test.dart index 7afaa72..6c256f3 100644 --- a/test/completed_ride_sync_recovery_test.dart +++ b/test/completed_ride_sync_recovery_test.dart @@ -676,8 +676,12 @@ void main() { await processFuture; final item = outbox.getItem(rideId, 'firebase'); - expect(item?.status, equals(SyncStatus.cancelled), - reason: 'Item must remain cancelled and not get overwritten with synced'); + expect( + item?.status, + equals(SyncStatus.cancelled), + reason: + 'Item must remain cancelled and not get overwritten with synced', + ); }, ); @@ -700,10 +704,7 @@ void main() { ); expect(managerWithProviders.activeProviderId, equals('custom_gateway')); - final managerNoProviders = SyncManager( - outbox, - autoStart: false, - ); + final managerNoProviders = SyncManager(outbox, autoStart: false); expect(managerNoProviders.activeProviderId, equals('fake')); }, ); @@ -737,7 +738,10 @@ void main() { final item = await manager.enqueueRide(rideId, autoProcess: false); expect(item.status, equals(SyncStatus.cancelled)); expect(item.lastError, contains('belongs to another account')); - expect(outbox.getItem(rideId, 'firebase')?.status, equals(SyncStatus.cancelled)); + expect( + outbox.getItem(rideId, 'firebase')?.status, + equals(SyncStatus.cancelled), + ); }, ); @@ -771,7 +775,10 @@ void main() { final report1 = await manager.reconcileCompletedRides(); expect(report1.unverifiablePendingCount, equals(1)); - expect(outbox.getItem(rideId, 'firebase')?.status, equals(SyncStatus.retryWait)); + expect( + outbox.getItem(rideId, 'firebase')?.status, + equals(SyncStatus.retryWait), + ); // Step 2: Backend recovers and verifies the bundle in R2 fakeProvider.confirmationResults[remoteRideId] = @@ -785,8 +792,12 @@ void main() { outbox.getItem(rideId, 'firebase')?.status, equals(SyncStatus.synced), ); - expect(fakeProvider.uploadedItems.isEmpty, isTrue, - reason: 'Must confirm and mark synced without re-uploading the bundle'); + expect( + fakeProvider.uploadedItems.isEmpty, + isTrue, + reason: + 'Must confirm and mark synced without re-uploading the bundle', + ); }, ); @@ -826,8 +837,53 @@ void main() { final item = outbox.getItem(rideId, 'firebase'); expect(item?.status, equals(SyncStatus.synced)); expect(item?.lastError, contains('without modern checksum')); - expect(fakeProvider.uploadedItems.isEmpty, isTrue, - reason: 'Must never re-upload existing legacy unverified bundle'); + expect( + fakeProvider.uploadedItems.isEmpty, + isTrue, + reason: 'Must never re-upload existing legacy unverified bundle', + ); + }, + ); + + test( + '18. Reconciliation Never Claims Unauthorized Rides: cloudOwnerUid remains null when verification returns unauthorized', + () async { + authService.setAuthenticatedUser('rider_eve', 'eve@example.com'); + + final rideId = await db.startRide( + startTime: DateTime.utc(2026, 10, 12, 15, 0, 0), + ); + await db.endRide(rideId, const RidesCompanion()); + final remoteRideId = await db.ensureCloudRideId(rideId); + + // Verification returns unauthorized (foreign-owned or permission denied) + fakeProvider.confirmationResults[remoteRideId] = + CloudConfirmationResult.unauthorized( + remoteRideId: remoteRideId, + message: 'Foreign owner in cloud metadata', + ); + + final outbox = SyncOutbox(prefs); + final manager = SyncManager( + outbox, + initialProviders: [fakeProvider], + defaultProviderId: 'firebase', + db: db, + authService: authService, + autoStart: false, + ); + + final report = await manager.reconcileCompletedRides(); + + // Must NOT claim ownership for rider_eve + final localOwner = await db.getCloudOwnerUid(rideId); + expect( + localOwner, + isNull, + reason: 'Must NEVER claim an unauthorized ride', + ); + expect(report.foreignOwnerSkippedCount, equals(1)); + expect(report.errors.isNotEmpty, isTrue); }, ); });