From 9ed669091fbd312c18a024c99dffa2d8db94f46b Mon Sep 17 00:00:00 2001
From: Johannes Millan <johannes.millan@gmail.com>
Date: Sat, 11 Jul 2026 13:11:49 +0200
Subject: [PATCH] fix(sync): bound timeout heuristic 504 match to word
 boundaries

Follow-up hardening flagged in review of the tamper-error routing fix.
`_isTimeoutError` matched a bare '504' substring anywhere in String(error),
so any error whose stringified form embeds '504' in a longer token (a uuidv7
op id, a byte offset, a line number) was misclassified as a gateway timeout.
The prior commit neutralised this for OperationIntegrityError via branch
ordering; bounding '504' to word boundaries (\b504\b) removes the footgun
for every other error type too. A real HTTP 504 status still matches.

Adds a regression test (embedded-504 token routes to the generic ERROR
handler, not the timeout branch).
---
 src/app/imex/sync/sync-wrapper.service.spec.ts | 18 ++++++++++++++++++
 src/app/imex/sync/sync-wrapper.service.ts      |  8 +++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/app/imex/sync/sync-wrapper.service.spec.ts b/src/app/imex/sync/sync-wrapper.service.spec.ts
index 142fede4c4..989bd0f156 100644
--- a/src/app/imex/sync/sync-wrapper.service.spec.ts
+++ b/src/app/imex/sync/sync-wrapper.service.spec.ts
@@ -1253,6 +1253,24 @@ describe('SyncWrapperService', () => {
       expect(mockSnackService.open).not.toHaveBeenCalled();
     });
 
+    it('does not misclassify a non-timeout error with an embedded "504" token as a gateway timeout', async () => {
+      // _isTimeoutError bounds '504' to word boundaries, so a '504' buried inside
+      // a longer token (here a byte offset) is NOT read as an HTTP 504. Such an
+      // error must fall through to the generic ERROR handler, not the timeout
+      // branch (which would show the wrong "try again" message / silence it).
+      mockSyncService.downloadRemoteOps.and.returnValue(
+        Promise.reject(new Error('write failed at offset 1234504')),
+      );
+
+      const result = await service.sync(true);
+
+      expect(result).toBe('HANDLED_ERROR');
+      expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('ERROR');
+      expect(mockSnackService.open).not.toHaveBeenCalledWith(
+        jasmine.objectContaining({ msg: T.F.SYNC.S.TIMEOUT_ERROR }),
+      );
+    });
+
     it('should surface a lock-acquisition timeout for user-triggered syncs', async () => {
       mockSyncService.downloadRemoteOps.and.returnValue(
         Promise.reject(new LockAcquisitionTimeoutError('sp_op_log', 30000)),
diff --git a/src/app/imex/sync/sync-wrapper.service.ts b/src/app/imex/sync/sync-wrapper.service.ts
index a2c3978669..8ff7093799 100644
--- a/src/app/imex/sync/sync-wrapper.service.ts
+++ b/src/app/imex/sync/sync-wrapper.service.ts
@@ -1388,9 +1388,15 @@ export class SyncWrapperService {
 
   private _isTimeoutError(error: unknown): boolean {
     const errStr = String(error).toLowerCase();
+    // Bound '504' to word boundaries: an HTTP 504 status ("http 504 gateway
+    // timeout", "status 504") still matches, but a '504' buried inside a longer
+    // token — e.g. a uuidv7 op id like '01920504-…' in an OperationIntegrityError
+    // message — must NOT be read as a gateway timeout and misclassify an unrelated
+    // error. (The OperationIntegrityError branch is also ordered above this guard;
+    // this hardening removes the footgun for any other error type too.)
     return (
       errStr.includes('timeout') ||
-      errStr.includes('504') ||
+      /\b504\b/.test(errStr) ||
       errStr.includes('gateway timeout')
     );
   }