agw/overleaf-cep
1
0
Fork 0
mirror of https://github.com/yu-i-i/overleaf-cep.git synced 2026-11-12 13:13:45 +01:00
Code Issues Packages Projects Releases Wiki Activity

Validate id types in LinkedFiles getSourceProject (#35748)

Browse Source 
* Validate id types in LinkedFiles getSourceProject

Require source_project_id and v1_source_doc_id to be primitives before
querying Mongo, rejecting anything else with BadDataError.

* Validate refreshLinkedFile request params and body

Route refreshLinkedFile through parseReq with a schema requiring
project_id and file_id to be ObjectIds and typing the request body.

* Use well-formed ObjectIds in LinkedFilesController tests

Give the shared test setup valid ObjectId project_id/parent_folder_id
so parseReq passes and the controller logic is actually exercised, and
fail the tests if next is called unexpectedly.

* Use ObjectId file ids in v1 linked-file acceptance fixtures

The refresh route now validates file_id as an ObjectId, so the v1
not-imported fixtures need well-formed ids to reach the refuse-to-refresh
path (409) instead of failing id validation (404).

GitOrigin-RevId: d78b3664e825c6bb25ab2e363e5094b9212db307
This commit is contained in:
Brian Gough
2026-09-17 11:35:02 +00:00
committed by Copybot
parent 158d8a5839
commit 6c756eab73
5 changed files with 211 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
services/web/app/src/Features/LinkedFiles/LinkedFilesController.mjs
+15 -3
View File
@@ -60,6 +60,17 @@ const createLinkedFileSchema = z.object({
}),
})
const refreshLinkedFileSchema = z.object({
params: z.object({
project_id: zz.objectId(),
file_id: zz.objectId(),
}),
body: z.object({
clientId: z.string().optional(),
shouldReindexReferences: z.boolean().optional(),
}),
})
async function createLinkedFile(req, res, next) {
const { params, body } = parseReq(req, createLinkedFileSchema)
const { project_id: projectId } = params
@@ -94,8 +105,9 @@ async function createLinkedFile(req, res, next) {
}
async function refreshLinkedFile(req, res, next) {
const { project_id: projectId, file_id: fileId } = req.params
const { clientId } = req.body
const { params, body } = parseReq(req, refreshLinkedFileSchema)
const { project_id: projectId, file_id: fileId } = params
const { clientId, shouldReindexReferences } = body
const userId = SessionManager.getLoggedInUserId(req.session)
const { file, parentFolder } = await LinkedFilesHandler.promises.getFileById(
@@ -136,7 +148,7 @@ async function refreshLinkedFile(req, res, next) {
return LinkedFilesController.handleError(err, req, res, next)
}
if (req.body.shouldReindexReferences) {
if (shouldReindexReferences) {
// Signal to clients that they should re-index references
EditorRealTimeController.emitToRoom(
projectId,
services/web/app/src/Features/LinkedFiles/LinkedFilesHandler.mjs
+17
View File
@@ -1,4 +1,5 @@
import fs from 'node:fs'
import mongodb from 'mongodb-legacy'
import FileWriter from '../../infrastructure/FileWriter.mjs'
import EditorController from '../Editor/EditorController.mjs'
import ProjectLocator from '../Project/ProjectLocator.mjs'
@@ -7,6 +8,8 @@ import ProjectGetter from '../Project/ProjectGetter.mjs'
import LinkedFilesErrors from './LinkedFilesErrors.mjs'
import { callbackifyAll } from '@overleaf/promise-utils'
const { ObjectId } = mongodb
const { ProjectNotFoundError, V1ProjectNotFoundError, BadDataError } =
LinkedFilesErrors
@@ -25,6 +28,13 @@ const LinkedFilesHandler = {
async getSourceProject(data) {
const projection = { _id: 1, name: 1, overleaf: 1 } // include the historyId for future use
if (data.v1_source_doc_id != null) {
// v1_source_doc_id must be a string or a number.
if (
typeof data.v1_source_doc_id !== 'string' &&
typeof data.v1_source_doc_id !== 'number'
) {
throw new BadDataError('invalid v1_source_doc_id')
}
const project = await Project.findOne(
{ 'overleaf.id': data.v1_source_doc_id },
projection
@@ -36,6 +46,13 @@ const LinkedFilesHandler = {
return project
} else if (data.source_project_id != null) {
// source_project_id must be a plain ObjectId string.
if (
typeof data.source_project_id !== 'string' ||
!ObjectId.isValid(data.source_project_id)
) {
throw new BadDataError('invalid source_project_id')
}
const project = await ProjectGetter.promises.getProject(
data.source_project_id,
projection
services/web/test/acceptance/src/LinkedFilesTests.mjs
+4 -4
View File
@@ -222,7 +222,7 @@ describe('LinkedFiles', function () {
v1_source_doc_id: 9999999, // We won't find this id in the database
source_entity_path: 'example.jpeg',
},
_id: 'abcd',
_id: '000000000000000000000abc',
rev: 0,
created: new Date(),
name: 'example.jpeg',
@@ -232,7 +232,7 @@ describe('LinkedFiles', function () {
it('should refuse to refresh', async function () {
const { response, body } = await owner.doRequest('post', {
url: `/project/${projectOneId}/linked_file/abcd/refresh`,
url: `/project/${projectOneId}/linked_file/000000000000000000000abc/refresh`,
json: true,
})
expect(response.statusCode).to.equal(409)
@@ -505,7 +505,7 @@ describe('LinkedFiles', function () {
v1_source_doc_id: 9999999, // We won't find this id in the database
source_output_file_path: 'output.pdf',
},
_id: 'abcdef',
_id: '000000000000000000abcdef',
rev: 0,
created: new Date(),
name: 'whatever.pdf',
@@ -515,7 +515,7 @@ describe('LinkedFiles', function () {
it('should refuse to refresh', async function () {
const { response, body } = await owner.doRequest('post', {
url: `/project/${projectOneId}/linked_file/abcdef/refresh`,
url: `/project/${projectOneId}/linked_file/000000000000000000abcdef/refresh`,
json: true,
})
expect(response.statusCode).to.equal(409)
services/web/test/unit/src/LinkedFiles/LinkedFilesController.test.mjs
+48 -12
View File
@@ -21,7 +21,10 @@ describe('LinkedFilesController', function () {
refreshLinkedFile: sinon.stub().resolves(),
},
}
ctx.projectId = 'projectId'
// project_id, parent_folder_id and file_id are validated as Mongo
// ObjectIds, so use well-formed values throughout.
ctx.projectId = '507f1f77bcf86cd799439011'
ctx.parentFolderId = '507f191e810c19729de860eb'
ctx.provider = 'provider'
ctx.fileName = 'linked-file-name'
ctx.data = { customAgentData: 'foo' }
@@ -119,21 +122,30 @@ describe('LinkedFilesController', function () {
name: ctx.fileName,
provider: ctx.provider,
data: ctx.data,
parent_folder_id: ctx.parentFolderId,
},
}
ctx.next = sinon.stub()
})
it('sets importedAt timestamp on linkedFileData', async function (ctx) {
await new Promise(resolve => {
ctx.next = sinon.stub().callsFake(() => resolve('unexpected error'))
await new Promise((resolve, reject) => {
ctx.next = sinon
.stub()
.callsFake(err =>
reject(err || new Error('next called unexpectedly'))
)
ctx.res = {
json: () => {
expect(ctx.Agent.promises.createLinkedFile).to.have.been.calledWith(
ctx.projectId,
{ ...ctx.data, importedAt: ctx.fakeTime.toISOString() },
{
customAgentData: 'foo',
provider: ctx.provider,
importedAt: ctx.fakeTime.toISOString(),
},
ctx.fileName,
undefined,
ctx.parentFolderId,
ctx.userId
)
resolve()
@@ -145,6 +157,7 @@ describe('LinkedFilesController', function () {
})
describe('refreshLinkedFiles', function () {
beforeEach(function (ctx) {
ctx.fileId = '507f191e810c19729de860ea'
ctx.data.provider = ctx.provider
ctx.file = {
name: ctx.fileName,
@@ -154,7 +167,7 @@ describe('LinkedFilesController', function () {
},
}
ctx.LinkedFilesHandler.promises.getFileById
.withArgs(ctx.projectId, 'file-id')
.withArgs(ctx.projectId, ctx.fileId)
.resolves({
file: ctx.file,
path: 'fake-path',
@@ -163,15 +176,19 @@ describe('LinkedFilesController', function () {
},
})
ctx.req = {
params: { project_id: ctx.projectId, file_id: 'file-id' },
params: { project_id: ctx.projectId, file_id: ctx.fileId },
body: {},
}
ctx.next = sinon.stub()
})
it('resets importedAt timestamp on linkedFileData', async function (ctx) {
await new Promise(resolve => {
ctx.next = sinon.stub().callsFake(() => resolve('unexpected error'))
await new Promise((resolve, reject) => {
ctx.next = sinon
.stub()
.callsFake(err =>
reject(err || new Error('next called unexpectedly'))
)
ctx.res = {
json: () => {
expect(
@@ -182,7 +199,7 @@ describe('LinkedFilesController', function () {
...ctx.data,
importedAt: ctx.fakeTime.toISOString(),
},
ctx.name,
ctx.fileName,
'parent-folder-id',
ctx.userId
)
@@ -193,6 +210,21 @@ describe('LinkedFilesController', function () {
})
})
it('rejects invalid params without calling the agent', async function (ctx) {
ctx.req.params.file_id = 'not-an-object-id'
await new Promise(resolve => {
ctx.next = sinon.stub().callsFake(() => resolve())
ctx.res = {
json: () => resolve(),
sendStatus: () => resolve(),
}
ctx.LinkedFilesController.refreshLinkedFile(ctx.req, ctx.res, ctx.next)
})
expect(ctx.next).to.have.been.calledOnce
expect(ctx.next.firstCall.args[0]?.name).to.equal('InvalidParamsError')
expect(ctx.Agent.promises.refreshLinkedFile).to.not.have.been.called
})
describe('when bib file re-indexing is required', function () {
const clientId = 'client-id'
beforeEach(function (ctx) {
@@ -201,8 +233,12 @@ describe('LinkedFilesController', function () {
})
it('informs clients to re-index bib references', async function (ctx) {
await new Promise(resolve => {
ctx.next = sinon.stub().callsFake(() => resolve('unexpected error'))
await new Promise((resolve, reject) => {
ctx.next = sinon
.stub()
.callsFake(err =>
reject(err || new Error('next called unexpectedly'))
)
ctx.res = {
json: () => {
expect(
services/web/test/unit/src/LinkedFiles/LinkedFilesHandler.test.mjs
+127
View File
@@ -0,0 +1,127 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import sinon from 'sinon'
const modulePath =
'../../../../app/src/Features/LinkedFiles/LinkedFilesHandler.mjs'
// The module under test is imported dynamically in each test, so it resolves
// its own copy of the error classes. Assert on the stable `name` property
// rather than `instanceof`, which would compare against a different class
// instance.
async function getRejection(promise) {
try {
await promise
return null
} catch (error) {
return error
}
}
describe('LinkedFilesHandler', function () {
beforeEach(async function (ctx) {
ctx.projectId = '507f1f77bcf86cd799439011'
ctx.project = { _id: ctx.projectId, name: 'source project' }
ctx.Project = {
findOne: sinon.stub().returns({ exec: sinon.stub().resolves(null) }),
}
ctx.ProjectGetter = {
promises: {
getProject: sinon.stub().resolves(ctx.project),
},
}
vi.doMock('../../../../app/src/models/Project', () => ({
Project: ctx.Project,
}))
vi.doMock('../../../../app/src/Features/Project/ProjectGetter', () => ({
default: ctx.ProjectGetter,
}))
vi.doMock('../../../../app/src/infrastructure/FileWriter', () => ({
default: {},
}))
vi.doMock('../../../../app/src/Features/Editor/EditorController', () => ({
default: {},
}))
vi.doMock('../../../../app/src/Features/Project/ProjectLocator', () => ({
default: {},
}))
ctx.LinkedFilesHandler = (await import(modulePath)).default
})
describe('getSourceProject', function () {
describe('with a source_project_id', function () {
it('rejects a source_project_id that is not a string', async function (ctx) {
const error = await getRejection(
ctx.LinkedFilesHandler.promises.getSourceProject({
source_project_id: { _id: ctx.projectId, other: { $regex: '^6' } },
source_entity_path: 'main.tex',
})
)
expect(error?.name).to.equal('BadDataError')
expect(ctx.ProjectGetter.promises.getProject).to.not.have.been.called
})
it('rejects a non-ObjectId string', async function (ctx) {
const error = await getRejection(
ctx.LinkedFilesHandler.promises.getSourceProject({
source_project_id: 'not-an-object-id',
source_entity_path: 'main.tex',
})
)
expect(error?.name).to.equal('BadDataError')
expect(ctx.ProjectGetter.promises.getProject).to.not.have.been.called
})
it('passes a valid ObjectId string through to getProject', async function (ctx) {
const project = await ctx.LinkedFilesHandler.promises.getSourceProject({
source_project_id: ctx.projectId,
source_entity_path: 'main.tex',
})
expect(project).to.equal(ctx.project)
expect(ctx.ProjectGetter.promises.getProject).to.have.been.calledWith(
ctx.projectId
)
})
it('rejects with ProjectNotFoundError when no project matches', async function (ctx) {
ctx.ProjectGetter.promises.getProject.resolves(null)
const error = await getRejection(
ctx.LinkedFilesHandler.promises.getSourceProject({
source_project_id: ctx.projectId,
source_entity_path: 'main.tex',
})
)
expect(error?.name).to.equal('ProjectNotFoundError')
})
})
describe('with a v1_source_doc_id', function () {
it('rejects a v1_source_doc_id that is not a string or number', async function (ctx) {
const error = await getRejection(
ctx.LinkedFilesHandler.promises.getSourceProject({
v1_source_doc_id: { other: '1' },
source_entity_path: 'main.tex',
})
)
expect(error?.name).to.equal('BadDataError')
expect(ctx.Project.findOne).to.not.have.been.called
})
it('passes a numeric id through to Project.findOne', async function (ctx) {
ctx.Project.findOne.returns({
exec: sinon.stub().resolves(ctx.project),
})
const project = await ctx.LinkedFilesHandler.promises.getSourceProject({
v1_source_doc_id: 1234,
source_entity_path: 'main.tex',
})
expect(project).to.equal(ctx.project)
expect(ctx.Project.findOne).to.have.been.calledWith({
'overleaf.id': 1234,
})
})
})
})
})