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).
This commit is contained in:
Johannes Millan 2026-07-11 13:11:49 +02:00
parent 826ea2ab95
commit 9ed669091f
2 changed files with 25 additions and 1 deletions

View file

@ -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)),

View file

@ -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')
);
}