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

Merge pull request #36223 from overleaf/jpa-zod-web-core-2

Browse Source 
[web] core-2: adopt strict zod schema and extend test coverage

GitOrigin-RevId: f26d349fefb6bf9e042cb39117395120c3f7c877
This commit is contained in:
Jakob Ackermann
2026-09-17 11:55:37 +00:00
committed by Copybot
parent e2564f356c
commit 55c23b6812
5 changed files with 233 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
services/web/app/src/Features/References/ReferencesController.mjs
+16 -2
View File
@@ -1,9 +1,23 @@
import EditorRealTimeController from '../Editor/EditorRealTimeController.mjs'
import { z, zz, parseReq } from '../../infrastructure/Validation.mjs'
const indexAllSchema = z.object({
params: z.strictObject({
Project_id: zz.objectId(),
}),
body: z.strictObject({
shouldBroadcast: z.boolean().optional(),
clientId: z.string().optional(),
}),
})
export default {
indexAll(req, res, next) {
const projectId = req.params.Project_id
const { shouldBroadcast, clientId } = req.body
const { params, body } = parseReq(req, indexAllSchema, {
logOnly: true,
})
const projectId = params.Project_id
const { shouldBroadcast, clientId } = body
// We've migrated to client side indexing, so we only use the message for
// broadcasting that the clients need to re-index.
if (shouldBroadcast) {
services/web/app/src/Features/ThirdPartyDataStore/TpdsController.mjs
+87 -13
View File
@@ -13,9 +13,28 @@ import HttpErrorHandler from '../Errors/HttpErrorHandler.mjs'
import TpdsQueueManager from './TpdsQueueManager.mjs'
import { parseReq, z, zz } from '@overleaf/validation-tools'
// project_id/projectId is either a valid Mongo ObjectId (look up an existing
// project) or absent/empty (fall back to resolving/creating by name) -- see
// TpdsUpdateHandler.getOrCreateProject. When present and non-empty it always
// reaches a Mongo lookup (ProjectGetter.getProject) before it can reach any
// sink, so a bare zz.objectId() union covers this safely.
const optionalProjectId = zz.objectId().or(z.literal('')).optional()
const createProjectSchema = z.object({
params: z.strictObject({
user_id: zz.objectId(),
}),
body: z.strictObject({
projectName: z.string().optional(),
}),
})
async function createProject(req, res) {
const { user_id: userId } = req.params
let { projectName } = req.body
const {
params: { user_id: userId },
body,
} = parseReq(req, createProjectSchema, { logOnly: true })
let { projectName } = body
projectName = await ProjectDetailsHandler.promises.generateUniqueName(
userId,
projectName
@@ -32,6 +51,23 @@ async function createProject(req, res) {
}
const resolveProjectSchema = z.object({
params: z.strictObject({
user_id: zz.objectId(),
}),
body: z
.strictObject({
projectId: zz.objectId(),
})
.or(
z.strictObject({
projectName: z.string().min(1),
})
),
})
// Rollout-temporary fallback (pre-refinement schema from main); delete
// when this route's REQ_VALIDATION_MODE instrumentation is removed.
const resolveProjectFallbackSchema = z.object({
params: z.object({
user_id: zz.objectId(),
}),
@@ -54,7 +90,9 @@ async function resolveProject(req, res) {
const {
params: { user_id: userId },
body: { projectId, projectName },
} = parseReq(req, resolveProjectSchema)
} = parseReq(req, resolveProjectSchema, {
fallbackSchema: resolveProjectFallbackSchema,
})
const project = await TpdsUpdateHandler.promises.getOrCreateProject(
userId,
projectId,
@@ -139,13 +177,22 @@ async function deleteUpdate(req, res) {
res.sendStatus(200)
}
const updateFolderSchema = z.object({
body: z.strictObject({
userId: zz.objectId(),
projectId: optionalProjectId,
path: z.string(),
}),
})
/**
* Update endpoint that accepts update details as JSON
*/
async function updateFolder(req, res) {
const userId = req.body.userId
const projectId = req.body.projectId
const { projectName, filePath } = splitPath(projectId, req.body.path)
const {
body: { userId, projectId, path },
} = parseReq(req, updateFolderSchema, { logOnly: true })
const { projectName, filePath } = splitPath(projectId, path)
const metadata = await TpdsUpdateHandler.promises.createFolder(
userId,
projectId,
@@ -173,9 +220,21 @@ async function updateFolder(req, res) {
// .gitignore, etc because people are generally more explicit with the files
// they want in git.
const projectContentsParamsSchema = z.object({
params: z.strictObject({
project_id: zz.objectId(),
// GitHub-sync repo file path; reaches UpdateMerger without any
// Path.join() normalization first, so it needs its own hardening.
path: zz.safePath(),
}),
})
async function updateProjectContents(req, res) {
const projectId = req.params.project_id
const path = `/${req.params[0]}` // UpdateMerger expects leading slash
const { params } = parseReq(req, projectContentsParamsSchema, {
logOnly: true,
})
const projectId = params.project_id
const path = `/${params.path}` // UpdateMerger expects leading slash
const source = req.headers['x-update-source'] || 'unknown'
try {
@@ -203,8 +262,11 @@ async function updateProjectContents(req, res) {
}
async function deleteProjectContents(req, res) {
const projectId = req.params.project_id
const path = `/${req.params[0]}` // UpdateMerger expects leading slash
const { params } = parseReq(req, projectContentsParamsSchema, {
logOnly: true,
})
const projectId = params.project_id
const path = `/${params.path}` // UpdateMerger expects leading slash
const source = req.headers['x-update-source'] || 'unknown'
const entityId = await UpdateMerger.promises.deleteUpdate(
@@ -221,10 +283,22 @@ async function getQueues(req, res) {
res.json(await TpdsQueueManager.promises.getQueues(userId))
}
const wildcardUpdateParamsSchema = z.object({
params: z.strictObject({
user_id: zz.objectId(),
project_id: optionalProjectId,
// Dropbox-supplied path (projectName/.../file, split up below). A path
// of exactly '/' is Dropbox's "the project folder itself was deleted"
// sentinel, which TpdsUpdateHandler.deleteUpdate acts on.
path: zz.safePath().or(z.literal('/')),
}),
})
function parseParams(req) {
const userId = req.params.user_id
const projectId = req.params.project_id
const { projectName, filePath } = splitPath(projectId, req.params[0])
const {
params: { user_id: userId, project_id: projectId, path },
} = parseReq(req, wildcardUpdateParamsSchema, { logOnly: true })
const { projectName, filePath } = splitPath(projectId, path)
return { filePath, userId, projectName, projectId }
}
services/web/test/acceptance/src/ReferencesTests.mjs
+40
View File
@@ -0,0 +1,40 @@
import { expect } from 'chai'
import UserHelper from './helpers/User.mjs'
import { expectValidationErrorRaw } from '@overleaf/validation-tools/testUtils.js'
const User = UserHelper.promises
describe('References', function () {
let owner, projectId
beforeEach(async function () {
owner = new User()
await owner.login()
projectId = await owner.createProject('references-test', {
template: 'blank',
})
})
describe('POST /project/:Project_id/references/indexAll', function () {
it('should return the projectId and empty keys', async function () {
const { response, body } = await owner.doRequest('post', {
url: `/project/${projectId}/references/indexAll`,
json: { shouldBroadcast: false },
})
expect(response.statusCode).to.equal(200)
expect(body).to.deep.equal({ projectId, keys: [] })
})
it('should reject a malformed project id with 404', async function () {
const { response, body } = await owner.doRequest('post', {
url: `/project/not-an-object-id/references/indexAll`,
json: { shouldBroadcast: false },
})
expectValidationErrorRaw(
{ statusCode: response.statusCode, body },
404,
'Project_id'
)
})
})
})
services/web/test/unit/src/References/ReferencesController.test.mjs
+26 -2
View File
@@ -1,5 +1,6 @@
import { beforeEach, describe, it, vi } from 'vitest'
import { afterEach, beforeEach, describe, it, vi } from 'vitest'
import sinon from 'sinon'
import { setReqValidationModeForTests } from '@overleaf/validation-tools'
import MockRequest from '../helpers/MockRequest.mjs'
import MockResponse from '../helpers/MockResponse.mjs'
const modulePath =
@@ -7,7 +8,8 @@ const modulePath =
describe('ReferencesController', function () {
beforeEach(async function (ctx) {
ctx.projectId = '2222'
// Project_id is validated as a Mongo ObjectId
ctx.projectId = '507f191e810c19729de860ea'
vi.doMock('@overleaf/settings', () => ({
default: (ctx.settings = {
@@ -142,5 +144,27 @@ describe('ReferencesController', function () {
})
})
})
describe('request validation', function () {
beforeEach(function () {
setReqValidationModeForTests('enforce')
})
afterEach(function () {
setReqValidationModeForTests(null)
})
it('should reject a malformed project id', function (ctx) {
ctx.req.params.Project_id = 'not-an-object-id'
;(() =>
ctx.controller.indexAll(ctx.req, ctx.res, ctx.next)).should.throw()
})
it('should reject an unrecognized body field', function (ctx) {
ctx.req.body.unexpectedField = 'nope'
;(() =>
ctx.controller.indexAll(ctx.req, ctx.res, ctx.next)).should.throw()
})
})
})
})
services/web/test/unit/src/ThirdPartyDataStore/TpdsController.test.mjs
+64 -15
View File
@@ -1,10 +1,14 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import mongodb from 'mongodb-legacy'
import sinon from 'sinon'
import Errors from '../../../../app/src/Features/Errors/Errors.js'
import MockResponse from '../helpers/MockResponse.mjs'
import MockRequest from '../helpers/MockRequest.mjs'
import { asZodError } from '@overleaf/validation-tools/testUtils.js'
import {
getRawReqInput,
setReqValidationModeForTests,
} from '@overleaf/validation-tools'
const ObjectId = mongodb.ObjectId
@@ -132,7 +136,7 @@ describe('TpdsController', function () {
await new Promise(resolve => {
const res = new MockResponse(vi)
const req = new MockRequest(vi)
req.params.user_id = ctx.user_id
req.params = { ...getRawReqInput(req).params, user_id: ctx.user_id }
req.body = { projectName: 'foo' }
res.callback = err => {
if (err) resolve(err)
@@ -165,6 +169,10 @@ describe('TpdsController', function () {
}
})
afterEach(function () {
setReqValidationModeForTests(null)
})
it('should throw without any input', async function (ctx) {
const next = sinon.stub()
await ctx.TpdsController.resolveProject(ctx.req, {}, next)
@@ -278,6 +286,41 @@ describe('TpdsController', function () {
ctx.TpdsController.resolveProject(ctx.req, res)
})
})
it('should still resolve a body with an unrecognized field in log mode', async function (ctx) {
setReqValidationModeForTests('log')
await new Promise(resolve => {
ctx.req.body = { projectName: 'projectName', extra: 'nope' }
const res = {
json: payload => {
expect(payload).to.deep.equal({
status: 'success',
projectId: ctx.resolvedProject._id.toString(),
historyId: 42,
otMigrationStage: 1,
})
resolve()
},
}
ctx.TpdsController.resolveProject(ctx.req, res)
})
})
describe('when enforced', function () {
afterEach(function () {
setReqValidationModeForTests(null)
})
it('should reject a body with an unrecognized field', async function (ctx) {
setReqValidationModeForTests('enforce')
ctx.req.body = { projectName: 'projectName', extra: 'nope' }
const next = sinon.stub()
await ctx.TpdsController.resolveProject(ctx.req, {}, next)
expect(next).to.have.been.calledWithMatch({
name: 'InvalidRequestError',
})
})
})
})
describe('getting an update', function () {
@@ -286,7 +329,7 @@ describe('TpdsController', function () {
ctx.path = '/here.txt'
ctx.req = {
params: {
0: `${ctx.projectName}${ctx.path}`,
path: `${ctx.projectName}${ctx.path}`,
user_id: ctx.user_id,
project_id: '',
},
@@ -341,9 +384,11 @@ describe('TpdsController', function () {
it('should process the update with the update receiver by id', async function (ctx) {
await new Promise(resolve => {
const path = '/here.txt'
// project_id is validated as a Mongo ObjectId
const projectId = new ObjectId().toString()
const req = {
pause() {},
params: { 0: path, user_id: ctx.user_id, project_id: '123' },
params: { path, user_id: ctx.user_id, project_id: projectId },
session: {
destroy() {},
},
@@ -355,7 +400,7 @@ describe('TpdsController', function () {
json: () => {
ctx.TpdsUpdateHandler.promises.newUpdate.should.have.been.calledWith(
ctx.user_id,
'123',
projectId,
'', // projectName
'/here.txt',
req,
@@ -400,7 +445,7 @@ describe('TpdsController', function () {
await new Promise(resolve => {
const path = '/projectName/here.txt'
const req = {
params: { 0: path, user_id: ctx.user_id, project_id: '' },
params: { path, user_id: ctx.user_id, project_id: '' },
session: {
destroy() {},
},
@@ -429,8 +474,10 @@ describe('TpdsController', function () {
it('should process the delete with the update receiver by id', async function (ctx) {
await new Promise(resolve => {
const path = '/here.txt'
// project_id is validated as a Mongo ObjectId
const projectId = new ObjectId().toString()
const req = {
params: { 0: path, user_id: ctx.user_id, project_id: '123' },
params: { path, user_id: ctx.user_id, project_id: projectId },
session: {
destroy() {},
},
@@ -442,7 +489,7 @@ describe('TpdsController', function () {
sendStatus: () => {
ctx.TpdsUpdateHandler.promises.deleteUpdate.should.have.been.calledWith(
ctx.user_id,
'123',
projectId,
'', // projectName
'/here.txt',
ctx.source
@@ -525,7 +572,7 @@ describe('TpdsController', function () {
describe('parseParams', function () {
it('should take the project name off the start and replace with slash', function (ctx) {
const path = 'noSlashHere'
const req = { params: { 0: path, user_id: ctx.user_id } }
const req = { params: { path, user_id: ctx.user_id } }
const result = ctx.TpdsController.parseParams(req)
result.userId.should.equal(ctx.user_id)
result.filePath.should.equal('/')
@@ -534,7 +581,7 @@ describe('TpdsController', function () {
it('should take the project name off the start and it with no slashes in', function (ctx) {
const path = '/project/file.tex'
const req = { params: { 0: path, user_id: ctx.user_id } }
const req = { params: { path, user_id: ctx.user_id } }
const result = ctx.TpdsController.parseParams(req)
result.userId.should.equal(ctx.user_id)
result.filePath.should.equal('/file.tex')
@@ -543,7 +590,7 @@ describe('TpdsController', function () {
it('should take the project name of and return a slash for the file path', function (ctx) {
const path = '/project_name'
const req = { params: { 0: path, user_id: ctx.user_id } }
const req = { params: { path, user_id: ctx.user_id } }
const result = ctx.TpdsController.parseParams(req)
result.projectName.should.equal('project_name')
result.filePath.should.equal('/')
@@ -554,8 +601,9 @@ describe('TpdsController', function () {
beforeEach(async function (ctx) {
ctx.req = {
params: {
0: (ctx.path = 'chapters/main.tex'),
project_id: (ctx.project_id = 'project-id-123'),
path: (ctx.path = 'chapters/main.tex'),
// project_id is validated as a Mongo ObjectId
project_id: (ctx.project_id = new ObjectId().toString()),
},
session: {
destroy: sinon.stub(),
@@ -595,8 +643,9 @@ describe('TpdsController', function () {
beforeEach(async function (ctx) {
ctx.req = {
params: {
0: (ctx.path = 'chapters/main.tex'),
project_id: (ctx.project_id = 'project-id-123'),
path: (ctx.path = 'chapters/main.tex'),
// project_id is validated as a Mongo ObjectId
project_id: (ctx.project_id = new ObjectId().toString()),
},
session: {
destroy: sinon.stub(),