From eca31afb4ad3ced654d645e607691497fe45d769 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Tue, 24 Feb 2026 11:01:40 +0100 Subject: [PATCH] [clsi] remove unused endpoints for downloading output files (#31692) GitOrigin-RevId: a0cac10f3585414779b026f38c2af2773c80082f --- services/clsi/app.js | 76 ----- services/clsi/app/js/ContentTypeMapper.js | 38 --- .../clsi/app/js/StaticServerForbidSymlinks.js | 89 ------ .../test/unit/js/ContentTypeMapper.test.js | 69 ----- .../js/StaticServerForbidSymlinks.test.js | 270 ------------------ 5 files changed, 542 deletions(-) delete mode 100644 services/clsi/app/js/ContentTypeMapper.js delete mode 100644 services/clsi/app/js/StaticServerForbidSymlinks.js delete mode 100644 services/clsi/test/unit/js/ContentTypeMapper.test.js delete mode 100644 services/clsi/test/unit/js/StaticServerForbidSymlinks.test.js diff --git a/services/clsi/app.js b/services/clsi/app.js index a8125be376..d8cc384721 100644 --- a/services/clsi/app.js +++ b/services/clsi/app.js @@ -2,27 +2,21 @@ import '@overleaf/metrics/initialize.js' import CompileController from './app/js/CompileController.js' -import ContentController from './app/js/ContentController.js' import Settings from '@overleaf/settings' import logger from '@overleaf/logger' import LoggerSerializers from './app/js/LoggerSerializers.js' import Metrics from '@overleaf/metrics' import smokeTest from './test/smoke/js/SmokeTests.js' -import ContentTypeMapper from './app/js/ContentTypeMapper.js' import Errors from './app/js/Errors.js' import OutputController from './app/js/OutputController.js' -import Path from 'node:path' import ProjectPersistenceManager from './app/js/ProjectPersistenceManager.js' import OutputCacheManager from './app/js/OutputCacheManager.js' -import ContentCacheManager from './app/js/ContentCacheManager.js' import express from 'express' import bodyParser from 'body-parser' -import ForbidSymlinks from './app/js/StaticServerForbidSymlinks.js' - import net from 'node:net' import os from 'node:os' import OError from '@overleaf/o-error' @@ -75,22 +69,6 @@ app.param('build_id', function (req, res, next, buildId) { } }) -app.param('contentId', function (req, res, next, contentId) { - if (contentId?.match(OutputCacheManager.CONTENT_REGEX)) { - next() - } else { - next(new OError('invalid content id', { contentId })) - } -}) - -app.param('hash', function (req, res, next, hash) { - if (hash?.match(ContentCacheManager.HASH_REGEX)) { - next() - } else { - next(new OError('invalid hash', { hash })) - } -}) - app.post( '/project/:project_id/compile', bodyParser.json({ limit: Settings.compileSizeLimit }), @@ -130,29 +108,6 @@ app.get( CompileController.wordcount ) -// create a static server which does not allow access to any symlinks -// avoids possible mismatch of root directory between middleware check -// and serving the files -const staticOutputServer = ForbidSymlinks( - express.static, - Settings.path.outputDir, - { - setHeaders(res, path, stat) { - if (Path.basename(path) === 'output.pdf') { - // Calculate an etag in the same way as nginx - // https://github.com/tj/send/issues/65 - const etag = (path, stat) => - `"${Math.ceil(+stat.mtime / 1000).toString(16)}` + - '-' + - Number(stat.size).toString(16) + - '"' - res.set('Etag', etag(path, stat)) - } - res.set('Content-Type', ContentTypeMapper.map(path)) - }, - } -) - // This needs to be before GET /project/:project_id/build/:build_id/output/* app.get( '/project/:project_id/build/:build_id/output/output.zip', @@ -167,37 +122,6 @@ app.get( OutputController.createOutputZip ) -app.get( - '/project/:project_id/user/:user_id/build/:build_id/output/*', - function (req, res, next) { - // for specific build get the path from the OutputCacheManager (e.g. .clsi/buildId) - req.url = - `/${req.params.project_id}-${req.params.user_id}/` + - OutputCacheManager.path(req.params.build_id, `/${req.params[0]}`) - staticOutputServer(req, res, next) - } -) - -app.get( - '/project/:projectId/content/:contentId/:hash', - ContentController.getPdfRange -) -app.get( - '/project/:projectId/user/:userId/content/:contentId/:hash', - ContentController.getPdfRange -) - -app.get( - '/project/:project_id/build/:build_id/output/*', - function (req, res, next) { - // for specific build get the path from the OutputCacheManager (e.g. .clsi/buildId) - req.url = - `/${req.params.project_id}/` + - OutputCacheManager.path(req.params.build_id, `/${req.params[0]}`) - staticOutputServer(req, res, next) - } -) - app.get('/status', (req, res, next) => res.send('CLSI is alive\n')) Settings.processTooOld = false diff --git a/services/clsi/app/js/ContentTypeMapper.js b/services/clsi/app/js/ContentTypeMapper.js deleted file mode 100644 index 5d0c5c113c..0000000000 --- a/services/clsi/app/js/ContentTypeMapper.js +++ /dev/null @@ -1,38 +0,0 @@ -/* eslint-disable - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -import Path from 'node:path' -let ContentTypeMapper - -// here we coerce html, css and js to text/plain, -// otherwise choose correct mime type based on file extension, -// falling back to octet-stream -export default ContentTypeMapper = { - map(path) { - switch (Path.extname(path)) { - case '.txt': - case '.html': - case '.js': - case '.css': - case '.svg': - return 'text/plain' - case '.csv': - return 'text/csv' - case '.pdf': - return 'application/pdf' - case '.png': - return 'image/png' - case '.jpg': - case '.jpeg': - return 'image/jpeg' - case '.tiff': - return 'image/tiff' - case '.gif': - return 'image/gif' - default: - return 'application/octet-stream' - } - }, -} diff --git a/services/clsi/app/js/StaticServerForbidSymlinks.js b/services/clsi/app/js/StaticServerForbidSymlinks.js deleted file mode 100644 index 5ba783d259..0000000000 --- a/services/clsi/app/js/StaticServerForbidSymlinks.js +++ /dev/null @@ -1,89 +0,0 @@ -/* eslint-disable - no-cond-assign, - no-unused-vars, - n/no-deprecated-api, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -import Path from 'node:path' -import fs from 'node:fs' -import Settings from '@overleaf/settings' -import logger from '@overleaf/logger' -let ForbidSymlinks - -export default ForbidSymlinks = function (staticFn, root, options) { - const expressStatic = staticFn(root, options) - const basePath = Path.resolve(root) - return function (req, res, next) { - let file, projectId, result - const path = req.url - // check that the path is of the form /project_id_or_name/path/to/file.log - if ((result = path.match(/^\/([a-zA-Z0-9_-]+)\/(.*)$/s))) { - projectId = result[1] - file = result[2] - if (path !== `/${projectId}/${file}`) { - logger.warn({ path }, 'unrecognized file request') - return res.sendStatus(404) - } - } else { - logger.warn({ path }, 'unrecognized file request') - return res.sendStatus(404) - } - // check that the file does not use a relative path - for (const dir of Array.from(file.split('/'))) { - if (dir === '..') { - logger.warn({ path }, 'attempt to use a relative path') - return res.sendStatus(404) - } - } - // check that the requested path is normalized - const requestedFsPath = `${basePath}/${projectId}/${file}` - if (requestedFsPath !== Path.normalize(requestedFsPath)) { - logger.error( - { path: requestedFsPath }, - 'requestedFsPath is not normalized' - ) - return res.sendStatus(404) - } - // check that the requested path is not a symlink - return fs.realpath(requestedFsPath, function (err, realFsPath) { - if (err != null) { - if (err.code === 'ENOENT') { - return res.sendStatus(404) - } else { - logger.error( - { - err, - requestedFsPath, - realFsPath, - path: req.params[0], - projectId: req.params.project_id, - }, - 'error checking file access' - ) - return res.sendStatus(500) - } - } else if (requestedFsPath !== realFsPath) { - logger.warn( - { - requestedFsPath, - realFsPath, - path: req.params[0], - projectId: req.params.project_id, - }, - 'trying to access a different file (symlink), aborting' - ) - return res.sendStatus(404) - } else { - return expressStatic(req, res, next) - } - }) - } -} diff --git a/services/clsi/test/unit/js/ContentTypeMapper.test.js b/services/clsi/test/unit/js/ContentTypeMapper.test.js deleted file mode 100644 index 0cc61c4928..0000000000 --- a/services/clsi/test/unit/js/ContentTypeMapper.test.js +++ /dev/null @@ -1,69 +0,0 @@ -import { describe, beforeEach, it } from 'vitest' -import path from 'node:path' - -const modulePath = path.join( - import.meta.dirname, - '../../../app/js/ContentTypeMapper' -) - -describe('ContentTypeMapper', function () { - beforeEach(async function (ctx) { - return (ctx.ContentTypeMapper = (await import(modulePath)).default) - }) - - return describe('map', function () { - it('should map .txt to text/plain', function (ctx) { - const contentType = ctx.ContentTypeMapper.map('example.txt') - return contentType.should.equal('text/plain') - }) - - it('should map .csv to text/csv', function (ctx) { - const contentType = ctx.ContentTypeMapper.map('example.csv') - return contentType.should.equal('text/csv') - }) - - it('should map .pdf to application/pdf', function (ctx) { - const contentType = ctx.ContentTypeMapper.map('example.pdf') - return contentType.should.equal('application/pdf') - }) - - it('should fall back to octet-stream', function (ctx) { - const contentType = ctx.ContentTypeMapper.map('example.unknown') - return contentType.should.equal('application/octet-stream') - }) - - describe('coercing web files to plain text', function () { - it('should map .js to plain text', function (ctx) { - const contentType = ctx.ContentTypeMapper.map('example.js') - return contentType.should.equal('text/plain') - }) - - it('should map .html to plain text', function (ctx) { - const contentType = ctx.ContentTypeMapper.map('example.html') - return contentType.should.equal('text/plain') - }) - - return it('should map .css to plain text', function (ctx) { - const contentType = ctx.ContentTypeMapper.map('example.css') - return contentType.should.equal('text/plain') - }) - }) - - return describe('image files', function () { - it('should map .png to image/png', function (ctx) { - const contentType = ctx.ContentTypeMapper.map('example.png') - return contentType.should.equal('image/png') - }) - - it('should map .jpeg to image/jpeg', function (ctx) { - const contentType = ctx.ContentTypeMapper.map('example.jpeg') - return contentType.should.equal('image/jpeg') - }) - - return it('should map .svg to text/plain to protect against XSS (SVG can execute JS)', function (ctx) { - const contentType = ctx.ContentTypeMapper.map('example.svg') - return contentType.should.equal('text/plain') - }) - }) - }) -}) diff --git a/services/clsi/test/unit/js/StaticServerForbidSymlinks.test.js b/services/clsi/test/unit/js/StaticServerForbidSymlinks.test.js deleted file mode 100644 index 81349b6c1f..0000000000 --- a/services/clsi/test/unit/js/StaticServerForbidSymlinks.test.js +++ /dev/null @@ -1,270 +0,0 @@ -import { vi, describe, beforeEach, it } from 'vitest' - -import path from 'node:path' -import sinon from 'sinon' -const modulePath = path.join( - import.meta.dirname, - '../../../app/js/StaticServerForbidSymlinks' -) - -describe('StaticServerForbidSymlinks', function () { - beforeEach(async function (ctx) { - ctx.settings = { - path: { - compilesDir: '/compiles/here', - }, - } - - ctx.fs = {} - - vi.doMock('@overleaf/settings', () => ({ - default: ctx.settings, - })) - - vi.doMock('fs', () => ({ - default: ctx.fs, - })) - - ctx.ForbidSymlinks = (await import(modulePath)).default - - ctx.dummyStatic = (rootDir, options) => (req, res, next) => - // console.log "dummyStatic serving file", rootDir, "called with", req.url - // serve it - next() - - ctx.StaticServerForbidSymlinks = ctx.ForbidSymlinks( - ctx.dummyStatic, - ctx.settings.path.compilesDir - ) - ctx.req = { - params: { - project_id: '12345', - }, - } - - ctx.res = {} - ctx.req.url = '/12345/output.pdf' - }) - - describe('sending a normal file through', function () { - beforeEach(function (ctx) { - ctx.fs.realpath = sinon - .stub() - .callsArgWith( - 1, - null, - `${ctx.settings.path.compilesDir}/${ctx.req.params.project_id}/output.pdf` - ) - }) - - it('should call next', async function (ctx) { - await new Promise((resolve, reject) => { - ctx.res.sendStatus = function (resCode) { - resCode.should.equal(200) - resolve() - } - ctx.StaticServerForbidSymlinks(ctx.req, ctx.res, err => { - if (err) reject(err) - resolve() - }) - }) - }) - }) - - describe('with a missing file', function () { - beforeEach(function (ctx) { - ctx.fs.realpath = sinon - .stub() - .callsArgWith( - 1, - { code: 'ENOENT' }, - `${ctx.settings.path.compilesDir}/${ctx.req.params.project_id}/unknown.pdf` - ) - }) - - it('should send a 404', async function (ctx) { - await new Promise((resolve, reject) => { - ctx.res.sendStatus = function (resCode) { - resCode.should.equal(404) - resolve() - } - ctx.StaticServerForbidSymlinks(ctx.req, ctx.res) - }) - }) - }) - - describe('with a new line', function () { - beforeEach(function (ctx) { - ctx.req.url = '/12345/output.pdf\nother file' - ctx.fs.realpath = sinon.stub().yields() - }) - - it('should process the correct file', async function (ctx) { - await new Promise((resolve, reject) => { - ctx.res.sendStatus = () => { - ctx.fs.realpath.should.have.been.calledWith( - `${ctx.settings.path.compilesDir}/12345/output.pdf\nother file` - ) - resolve() - } - ctx.StaticServerForbidSymlinks(ctx.req, ctx.res) - }) - }) - }) - - describe('with a symlink file', function () { - beforeEach(function (ctx) { - ctx.fs.realpath = sinon - .stub() - .callsArgWith(1, null, `/etc/${ctx.req.params.project_id}/output.pdf`) - }) - - it('should send a 404', async function (ctx) { - await new Promise((resolve, reject) => { - ctx.res.sendStatus = function (resCode) { - resCode.should.equal(404) - resolve() - } - ctx.StaticServerForbidSymlinks(ctx.req, ctx.res) - }) - }) - }) - - describe('with a relative file', function () { - beforeEach(function (ctx) { - ctx.req.url = '/12345/../67890/output.pdf' - }) - - it('should send a 404', async function (ctx) { - await new Promise((resolve, reject) => { - ctx.res.sendStatus = function (resCode) { - resCode.should.equal(404) - resolve() - } - ctx.StaticServerForbidSymlinks(ctx.req, ctx.res) - }) - }) - }) - - describe('with a unnormalized file containing .', function () { - beforeEach(function (ctx) { - ctx.req.url = '/12345/foo/./output.pdf' - }) - - it('should send a 404', async function (ctx) { - await new Promise((resolve, reject) => { - ctx.res.sendStatus = function (resCode) { - resCode.should.equal(404) - resolve() - } - ctx.StaticServerForbidSymlinks(ctx.req, ctx.res) - }) - }) - }) - - describe('with a file containing an empty path', function () { - beforeEach(function (ctx) { - ctx.req.url = '/12345/foo//output.pdf' - }) - - it('should send a 404', async function (ctx) { - await new Promise((resolve, reject) => { - ctx.res.sendStatus = function (resCode) { - resCode.should.equal(404) - resolve() - } - ctx.StaticServerForbidSymlinks(ctx.req, ctx.res) - }) - }) - }) - - describe('with a non-project file', function () { - beforeEach(function (ctx) { - ctx.req.url = '/.foo/output.pdf' - }) - - it('should send a 404', async function (ctx) { - await new Promise((resolve, reject) => { - ctx.res.sendStatus = function (resCode) { - resCode.should.equal(404) - resolve() - } - ctx.StaticServerForbidSymlinks(ctx.req, ctx.res) - }) - }) - }) - - describe('with a file outside the compiledir', function () { - beforeEach(function (ctx) { - ctx.req.url = '/../bar/output.pdf' - }) - - it('should send a 404', async function (ctx) { - await new Promise((resolve, reject) => { - ctx.res.sendStatus = function (resCode) { - resCode.should.equal(404) - resolve() - } - ctx.StaticServerForbidSymlinks(ctx.req, ctx.res) - }) - }) - }) - - describe('with a file with no leading /', function () { - beforeEach(function (ctx) { - ctx.req.url = './../bar/output.pdf' - }) - - it('should send a 404', async function (ctx) { - await new Promise((resolve, reject) => { - ctx.res.sendStatus = function (resCode) { - resCode.should.equal(404) - resolve() - } - ctx.StaticServerForbidSymlinks(ctx.req, ctx.res) - }) - }) - }) - - describe('with a github style path', function () { - beforeEach(function (ctx) { - ctx.req.url = '/henryoswald-latex_example/output/output.log' - ctx.fs.realpath = sinon - .stub() - .callsArgWith( - 1, - null, - `${ctx.settings.path.compilesDir}/henryoswald-latex_example/output/output.log` - ) - }) - - it('should call next', async function (ctx) { - await new Promise((resolve, reject) => { - ctx.res.sendStatus = function (resCode) { - resCode.should.equal(200) - resolve() - } - ctx.StaticServerForbidSymlinks(ctx.req, ctx.res, err => { - if (err) reject(err) - resolve() - }) - }) - }) - }) - - describe('with an error from fs.realpath', function () { - beforeEach(function (ctx) { - ctx.fs.realpath = sinon.stub().callsArgWith(1, 'error') - }) - - it('should send a 500', async function (ctx) { - await new Promise((resolve, reject) => { - ctx.res.sendStatus = function (resCode) { - resCode.should.equal(500) - resolve() - } - ctx.StaticServerForbidSymlinks(ctx.req, ctx.res) - }) - }) - }) -})