From 9b490073fcfb61621bfb91d0de709783da655a2f Mon Sep 17 00:00:00 2001
From: Andrew Rumble <andrew.rumble@overleaf.com>
Date: Thu, 12 Dec 2024 10:44:25 +0000
Subject: [PATCH] Merge pull request #22334 from
 overleaf/ar-guard-against-integer-like-strings-when-working-with-postgres

[history-v1] Guard against non-postgres projectIds

GitOrigin-RevId: 5bf75c67424297f52f2abd9d0f0f14a0f79f8921
---
 services/history-v1/storage/lib/assert.js     |  5 +++
 .../storage/lib/blob_store/postgres.js        | 10 ++---
 .../js/storage/blob_store_postgres.test.js    | 42 +++++++++++++++++++
 3 files changed, 52 insertions(+), 5 deletions(-)
 create mode 100644 services/history-v1/test/acceptance/js/storage/blob_store_postgres.test.js

diff --git a/services/history-v1/storage/lib/assert.js b/services/history-v1/storage/lib/assert.js
index 6f79086209..d0ce318b4d 100644
--- a/services/history-v1/storage/lib/assert.js
+++ b/services/history-v1/storage/lib/assert.js
@@ -40,6 +40,10 @@ function mongoId(arg, message) {
   assert.match(arg, MONGO_ID_REGEXP)
 }
 
+function postgresId(arg, message) {
+  assert.match(arg, POSTGRES_ID_REGEXP, message)
+}
+
 module.exports = {
   ...assert,
   transaction,
@@ -47,6 +51,7 @@ module.exports = {
   projectId,
   chunkId,
   mongoId,
+  postgresId,
   MONGO_ID_REGEXP,
   POSTGRES_ID_REGEXP,
 }
diff --git a/services/history-v1/storage/lib/blob_store/postgres.js b/services/history-v1/storage/lib/blob_store/postgres.js
index ee2cc6183f..7f66d2d24d 100644
--- a/services/history-v1/storage/lib/blob_store/postgres.js
+++ b/services/history-v1/storage/lib/blob_store/postgres.js
@@ -13,8 +13,8 @@ async function initialize(projectId) {
  * Return blob metadata for the given project and hash
  */
 async function findBlob(projectId, hash) {
+  assert.postgresId(projectId, `bad projectId ${projectId}`)
   projectId = parseInt(projectId, 10)
-  assert.integer(projectId, 'bad projectId')
   assert.blobHash(hash, 'bad hash')
 
   const binaryHash = hashToBuffer(hash)
@@ -35,8 +35,8 @@ async function findBlob(projectId, hash) {
  * @return {Promise.<Array.<Blob?>>} no guarantee on order
  */
 async function findBlobs(projectId, hashes) {
+  assert.postgresId(projectId, `bad projectId ${projectId}`)
   projectId = parseInt(projectId, 10)
-  assert.integer(projectId, 'bad projectId')
   assert.array(hashes, 'bad hashes: not array')
   hashes.forEach(function (hash) {
     assert.blobHash(hash, 'bad hash')
@@ -57,8 +57,8 @@ async function findBlobs(projectId, hashes) {
  * Return metadata for all blobs in the given project
  */
 async function getProjectBlobs(projectId) {
+  assert.postgresId(projectId, `bad projectId ${projectId}`)
   projectId = parseInt(projectId, 10)
-  assert.integer(projectId, 'bad projectId')
 
   const records = await knex('project_blobs')
     .select('hash_bytes', 'byte_length', 'string_length')
@@ -103,8 +103,8 @@ async function getProjectBlobsBatch(projectIds) {
  * Add a blob's metadata to the blobs table after it has been uploaded.
  */
 async function insertBlob(projectId, blob) {
+  assert.postgresId(projectId, `bad projectId ${projectId}`)
   projectId = parseInt(projectId, 10)
-  assert.integer(projectId, 'bad projectId')
 
   await knex('project_blobs')
     .insert(blobToRecord(projectId, blob))
@@ -116,8 +116,8 @@ async function insertBlob(projectId, blob) {
  * Deletes all blobs for a given project
  */
 async function deleteBlobs(projectId) {
+  assert.postgresId(projectId, `bad projectId ${projectId}`)
   projectId = parseInt(projectId, 10)
-  assert.integer(projectId, 'bad projectId')
 
   await knex('project_blobs').where('project_id', projectId).delete()
 }
diff --git a/services/history-v1/test/acceptance/js/storage/blob_store_postgres.test.js b/services/history-v1/test/acceptance/js/storage/blob_store_postgres.test.js
new file mode 100644
index 0000000000..0add4fa901
--- /dev/null
+++ b/services/history-v1/test/acceptance/js/storage/blob_store_postgres.test.js
@@ -0,0 +1,42 @@
+const postgresBackend = require('../../../../storage/lib/blob_store/postgres')
+const { ObjectId } = require('mongodb')
+const { expect } = require('chai')
+
+describe('BlobStore postgres backend', function () {
+  describe('projectId validation', function () {
+    it('insertBlob rejects when called with bad projectId', async function () {
+      const projectId = new ObjectId().toString()
+      await expect(
+        postgresBackend.insertBlob(projectId, 'hash', 123, 99)
+      ).to.be.rejectedWith(`bad projectId ${projectId}`)
+    })
+
+    it('deleteBlobs rejects when called with bad projectId', async function () {
+      const projectId = new ObjectId().toString()
+      await expect(postgresBackend.deleteBlobs(projectId)).to.be.rejectedWith(
+        `bad projectId ${projectId}`
+      )
+    })
+
+    it('findBlobs rejects when called with bad projectId', async function () {
+      const projectId = new ObjectId().toString()
+      await expect(postgresBackend.findBlobs(projectId)).to.be.rejectedWith(
+        `bad projectId ${projectId}`
+      )
+    })
+
+    it('findBlob rejects when called with bad projectId', async function () {
+      const projectId = new ObjectId().toString()
+      await expect(
+        postgresBackend.findBlob(projectId, 'hash')
+      ).to.be.rejectedWith(`bad projectId ${projectId}`)
+    })
+
+    it('getProjectBlobs rejects when called with bad projectId', async function () {
+      const projectId = new ObjectId().toString()
+      await expect(
+        postgresBackend.getProjectBlobs(projectId)
+      ).to.be.rejectedWith(`bad projectId ${projectId}`)
+    })
+  })
+})