From 85263ff1af65921d49a5638216822fc13cf6af53 Mon Sep 17 00:00:00 2001
From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com>
Date: Thu, 20 Mar 2025 09:02:42 -0400
Subject: [PATCH] Merge pull request #24390 from
 overleaf/em-enforce-content-hash-validation

Enforce content hash validation in history

GitOrigin-RevId: 90de21ea86ddc6548001059c41139a2af5b27060
---
 .../api/controllers/project_import.js         |  4 +++-
 services/history-v1/storage/index.js          |  3 +++
 .../history-v1/storage/lib/persist_changes.js | 23 +------------------
 .../js/storage/persist_changes.test.js        | 12 ++++------
 4 files changed, 11 insertions(+), 31 deletions(-)

diff --git a/services/history-v1/api/controllers/project_import.js b/services/history-v1/api/controllers/project_import.js
index 84bd9e0768..5dec84d843 100644
--- a/services/history-v1/api/controllers/project_import.js
+++ b/services/history-v1/api/controllers/project_import.js
@@ -22,6 +22,7 @@ const BlobStore = storage.BlobStore
 const chunkStore = storage.chunkStore
 const HashCheckBlobStore = storage.HashCheckBlobStore
 const persistChanges = storage.persistChanges
+const InvalidChangeError = storage.InvalidChangeError
 
 const render = require('./render')
 
@@ -113,7 +114,8 @@ async function importChanges(req, res, next) {
       err instanceof File.NotEditableError ||
       err instanceof FileMap.PathnameError ||
       err instanceof Snapshot.EditMissingFileError ||
-      err instanceof chunkStore.ChunkVersionConflictError
+      err instanceof chunkStore.ChunkVersionConflictError ||
+      err instanceof InvalidChangeError
     ) {
       // If we failed to apply operations, that's probably because they were
       // invalid.
diff --git a/services/history-v1/storage/index.js b/services/history-v1/storage/index.js
index 7fd1d589ea..238cd12a38 100644
--- a/services/history-v1/storage/index.js
+++ b/services/history-v1/storage/index.js
@@ -15,3 +15,6 @@ exports.zipStore = require('./lib/zip_store')
 const { BlobStore, loadGlobalBlobs } = require('./lib/blob_store')
 exports.BlobStore = BlobStore
 exports.loadGlobalBlobs = loadGlobalBlobs
+
+const { InvalidChangeError } = require('./lib/errors')
+exports.InvalidChangeError = InvalidChangeError
diff --git a/services/history-v1/storage/lib/persist_changes.js b/services/history-v1/storage/lib/persist_changes.js
index 6c21d4b294..8a848aa214 100644
--- a/services/history-v1/storage/lib/persist_changes.js
+++ b/services/history-v1/storage/lib/persist_changes.js
@@ -81,12 +81,6 @@ async function persistChanges(projectId, allChanges, limits, clientEndVersion) {
   let originalEndVersion
   let changesToPersist
 
-  /**
-   * It's only useful to log validation errors once per flush. When we enforce
-   * content hash validation, it will stop the flush right away anyway.
-   */
-  let validationErrorLogged = false
-
   limits = limits || {}
   _.defaults(limits, {
     changeBucketMinutes: 60,
@@ -131,22 +125,7 @@ async function persistChanges(projectId, allChanges, limits, clientEndVersion) {
       for (const operation of change.iterativelyApplyTo(currentSnapshot, {
         strict: true,
       })) {
-        try {
-          await validateContentHash(operation)
-        } catch (err) {
-          // Temporary: skip validation errors
-          if (err instanceof InvalidChangeError) {
-            if (!validationErrorLogged) {
-              logger.warn(
-                { err, projectId },
-                'content snapshot mismatch (ignored)'
-              )
-              validationErrorLogged = true
-            }
-          } else {
-            throw err
-          }
-        }
+        await validateContentHash(operation)
       }
 
       chunk.pushChanges([change])
diff --git a/services/history-v1/test/acceptance/js/storage/persist_changes.test.js b/services/history-v1/test/acceptance/js/storage/persist_changes.test.js
index 50eb505681..aa56dc8c2a 100644
--- a/services/history-v1/test/acceptance/js/storage/persist_changes.test.js
+++ b/services/history-v1/test/acceptance/js/storage/persist_changes.test.js
@@ -213,7 +213,7 @@ describe('persistChanges', function () {
       expect(result.numberOfChangesPersisted).to.equal(1)
     })
 
-    it('acccepts a change with an invalid hash (only logs for now)', async function () {
+    it('rejects a change with an invalid hash', async function () {
       const limitsToPersistImmediately = {
         minChangeTimestamp: farFuture,
         maxChangeTimestamp: farFuture,
@@ -235,13 +235,9 @@ describe('persistChanges', function () {
       )
       const changes = [change]
 
-      const result = await persistChanges(
-        projectId,
-        changes,
-        limitsToPersistImmediately,
-        0
-      )
-      expect(result.numberOfChangesPersisted).to.equal(1)
+      await expect(
+        persistChanges(projectId, changes, limitsToPersistImmediately, 0)
+      ).to.be.rejectedWith(storage.InvalidChangeError)
     })
   })
 })