Skip to content

Commit 30bfaf8

Browse files
vershwalvershwal
committed
Added file type validation and content-type control for file uploads
The files upload endpoint accepted any file type without validation, unlike images and media which check extension and MIME type. With the move to a shared CDN domain, we don't want to upload files with executable content types. This adds: - An allowlist of supported file extensions (based on Ghost Pro data) - Extension-only validation middleware on the upload route - Content-type resolution that serves browser-renderable types (images, PDF, JSON, audio, video, fonts) with their natural type, overrides formats (HTML, JS, CSS, XML) to text/plain, and defaults everything else to application/octet-stream (forced download) - Users can zip unsupported file types as an escape hatch
1 parent 29dc800 commit 30bfaf8

File tree

8 files changed

+425
-55
lines changed

8 files changed

+425
-55
lines changed

‎ghost/core/core/server/api/endpoints/files.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
const storage = require('../../adapters/storage');
2-
const mime = require('mime-types');
2+
const {getStorageContentType} = require('../../services/files/file-type-utils');
33

44
/** @type {import('@tryghost/api-framework').Controller} */
55
const controller = {
@@ -14,7 +14,7 @@ const controller = {
1414
const filePath = await storage.getStorage('files').save({
1515
name: frame.file.originalname,
1616
path: frame.file.path,
17-
type: mime.lookup(frame.file.originalname) || 'application/octet-stream'
17+
type: getStorageContentType(frame.file.originalname)
1818
});
1919

2020
return {

‎ghost/core/core/server/services/files/file-type-utils.js‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
const path = require('path');
2+
const mime = require('mime-types');
3+
4+
const CONTENT_TYPE_OVERRIDES = new Map([
5+
['.html', 'text/plain'],
6+
['.htm', 'text/plain'],
7+
['.js', 'text/plain'],
8+
['.css', 'text/plain'],
9+
['.xml', 'text/plain']
10+
]);
11+
12+
const BROWSER_RENDERABLE_TYPES = new Set([
13+
'image/jpeg',
14+
'image/png',
15+
'image/gif',
16+
'image/webp',
17+
'image/avif',
18+
'image/apng',
19+
'image/vnd.microsoft.icon',
20+
'application/pdf',
21+
'application/json',
22+
'audio/mpeg',
23+
'audio/wav',
24+
'audio/wave',
25+
'audio/ogg',
26+
'audio/mp4',
27+
'video/mp4',
28+
'video/webm',
29+
'font/otf',
30+
'font/woff',
31+
'font/woff2',
32+
'font/ttf',
33+
'text/plain'
34+
]);
35+
36+
/**
37+
* Determines the content type to store/serve for a given filename.
38+
*
39+
* Three-tier resolution:
40+
* 1. Override map — forced to text/plain to prevent browser execution
41+
* 2. Browser-renderable — type preserved as safe for browser rendering
42+
* 3. Fallback — application/octet-stream to force download
43+
*
44+
* @param {string} filename
45+
* @returns {string} content type to store
46+
*/
47+
function getStorageContentType(filename) {
48+
const ext = path.extname(filename).toLowerCase();
49+
50+
const override = CONTENT_TYPE_OVERRIDES.get(ext);
51+
if (override) {
52+
return override;
53+
}
54+
55+
const mimeType = mime.lookup(ext);
56+
if (mimeType && BROWSER_RENDERABLE_TYPES.has(mimeType)) {
57+
return mimeType;
58+
}
59+
60+
return 'application/octet-stream';
61+
}
62+
63+
module.exports = {
64+
getStorageContentType
65+
};

‎ghost/core/core/server/web/api/endpoints/admin/routes.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@ module.exports = function apiRoutes() {
323323
router.post('/files/upload',
324324
mw.authAdminApi,
325325
apiMw.upload.single('file'),
326+
apiMw.upload.fileValidation({type: 'files'}),
326327
http(api.files.upload)
327328
);
328329

‎ghost/core/core/server/web/api/middleware/upload.js‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ const messages = {
5252
thumbnail: {
5353
missingFile: 'Please select a thumbnail.',
5454
invalidFile: 'Please select a valid thumbnail.'
55+
},
56+
files: {
57+
missingFile: 'Please select a file.',
58+
invalidFile: 'The file type you uploaded is not supported. You can zip your file and upload it as a .zip.'
5559
}
5660
};
5761

@@ -356,11 +360,48 @@ const mediaValidation = function ({type}) {
356360
};
357361
};
358362

363+
/**
364+
* Extension-only validation for file uploads.
365+
* This validates the extension against the allowlist.
366+
* We are not validating the MIME type because it is unreliable and irrelevant as
367+
* we derive the storage content type from the extension via getStorageContentType().
368+
*
369+
* @param {Object} options
370+
* @param {String} options.type - config key under uploads (e.g. 'files')
371+
* @returns {import('express').RequestHandler}
372+
*/
373+
const fileValidation = function ({type}) {
374+
return function fileUploadValidation(req, res, next) {
375+
const extensions = (config.get('uploads')[type] && config.get('uploads')[type].extensions) || [];
376+
377+
req.file = req.file || {};
378+
req.file.name = req.file.originalname;
379+
req.file.type = req.file.mimetype;
380+
381+
if (!checkFileExists(req.file)) {
382+
return next(new errors.ValidationError({
383+
message: tpl(messages[type].missingFile)
384+
}));
385+
}
386+
387+
req.file.ext = path.extname(req.file.name).toLowerCase();
388+
389+
if (!extensions.includes(req.file.ext)) {
390+
return next(new errors.UnsupportedMediaTypeError({
391+
message: tpl(messages[type].invalidFile, {extensions: extensions})
392+
}));
393+
}
394+
395+
next();
396+
};
397+
};
398+
359399
module.exports = {
360400
single,
361401
media,
362402
validation,
363-
mediaValidation
403+
mediaValidation,
404+
fileValidation
364405
};
365406

366407
// Exports for testing only

‎ghost/core/core/shared/config/overrides.json‎

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -46,33 +46,52 @@
4646
"extensions": [
4747
".pdf",
4848
".json",
49-
".jsonld",
50-
".odp",
5149
".ods",
5250
".odt",
53-
".ppt",
5451
".pptx",
5552
".rtf",
5653
".txt",
5754
".xls",
5855
".xlsx",
59-
".xml"
60-
],
61-
"contentTypes": [
62-
"application/pdf",
63-
"application/json",
64-
"application/ld+json",
65-
"application/vnd.oasis.opendocument.presentation",
66-
"application/vnd.oasis.opendocument.spreadsheet",
67-
"application/vnd.oasis.opendocument.text",
68-
"application/vnd.ms-powerpoint",
69-
"application/vnd.openxmlformats-officedocument.presentationml.presentation",
70-
"application/rtf",
71-
"text/plain",
72-
"application/vnd.ms-excel",
73-
"application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
74-
"application/xml",
75-
"application/atom+xml"
56+
".xml",
57+
".apkg",
58+
".css",
59+
".csv",
60+
".doc",
61+
".docx",
62+
".epub",
63+
".gif",
64+
".gpx",
65+
".html",
66+
".ics",
67+
".ipynb",
68+
".jpeg",
69+
".jpg",
70+
".js",
71+
".key",
72+
".kml",
73+
".m4a",
74+
".md",
75+
".mobi",
76+
".mov",
77+
".mp3",
78+
".mp4",
79+
".otf",
80+
".pages",
81+
".paprikarecipes",
82+
".png",
83+
".psd",
84+
".py",
85+
".skp",
86+
".svg",
87+
".wav",
88+
".webp",
89+
".woff",
90+
".woff2",
91+
".xlsb",
92+
".xlsm",
93+
".yaml",
94+
".zip"
7695
]
7796
},
7897
"thumbnails": {

‎ghost/core/test/e2e-api/admin/files.test.js‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
const assert = require('node:assert/strict');
22
const path = require('path');
33
const fs = require('fs-extra');
4+
const os = require('os');
45
const supertest = require('supertest');
56
const sinon = require('sinon');
67
const localUtils = require('./utils');
@@ -94,4 +95,72 @@ describe('Files API', function () {
9495
const fileArg = saveSpy.firstCall.args[0];
9596
assert.equal(fileArg.type, 'application/json', 'save() should receive the correct content type for JSON files');
9697
});
98+
99+
it('Rejects an unsupported file extension with 415', async function () {
100+
const tempFile = path.join(os.tmpdir(), 'test.exe');
101+
fs.writeFileSync(tempFile, 'not a real executable');
102+
103+
try {
104+
const res = await request.post(localUtils.API.getApiQuery('files/upload'))
105+
.set('Origin', config.get('url'))
106+
.expect('Content-Type', /json/)
107+
.attach('file', tempFile)
108+
.expect(415);
109+
110+
assert.ok(res.body.errors, 'Response should contain errors');
111+
assert.match(res.body.errors[0].message, /not supported/i);
112+
} finally {
113+
fs.removeSync(tempFile);
114+
}
115+
});
116+
117+
it('Stores non-browser-renderable files with application/octet-stream', async function () {
118+
const store = storage.getStorage('files');
119+
const saveSpy = sinon.spy(store, 'save');
120+
121+
const tempFile = path.join(os.tmpdir(), 'spreadsheet.xlsx');
122+
fs.writeFileSync(tempFile, 'fake xlsx content');
123+
124+
try {
125+
const res = await request.post(localUtils.API.getApiQuery('files/upload'))
126+
.set('Origin', config.get('url'))
127+
.expect('Content-Type', /json/)
128+
.attach('file', tempFile)
129+
.expect(201);
130+
131+
assert.match(new URL(res.body.files[0].url).pathname, /\/content\/files\/\d+\/\d+\/spreadsheet\.xlsx/);
132+
files.push(new URL(res.body.files[0].url).pathname);
133+
134+
assert.ok(saveSpy.calledOnce, 'save() should have been called once');
135+
const fileArg = saveSpy.firstCall.args[0];
136+
assert.equal(fileArg.type, 'application/octet-stream', 'Non-browser-renderable files should be stored as application/octet-stream');
137+
} finally {
138+
fs.removeSync(tempFile);
139+
}
140+
});
141+
142+
it('Stores HTML files with text/plain content type to prevent execution', async function () {
143+
const store = storage.getStorage('files');
144+
const saveSpy = sinon.spy(store, 'save');
145+
146+
const tempFile = path.join(os.tmpdir(), 'page.html');
147+
fs.writeFileSync(tempFile, '<html><body><script>alert("xss")</script></body></html>');
148+
149+
try {
150+
const res = await request.post(localUtils.API.getApiQuery('files/upload'))
151+
.set('Origin', config.get('url'))
152+
.expect('Content-Type', /json/)
153+
.attach('file', tempFile)
154+
.expect(201);
155+
156+
assert.match(new URL(res.body.files[0].url).pathname, /\/content\/files\/\d+\/\d+\/page\.html/);
157+
files.push(new URL(res.body.files[0].url).pathname);
158+
159+
assert.ok(saveSpy.calledOnce, 'save() should have been called once');
160+
const fileArg = saveSpy.firstCall.args[0];
161+
assert.equal(fileArg.type, 'text/plain', 'HTML files should be stored as text/plain to prevent browser execution');
162+
} finally {
163+
fs.removeSync(tempFile);
164+
}
165+
});
97166
});

‎ghost/core/test/unit/server/data/importer/index.test.js‎

Lines changed: 13 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe('Importer', function () {
4242

4343
it('gets the correct extensions', function () {
4444
assert(Array.isArray(ImportManager.getExtensions()));
45-
assert.equal(ImportManager.getExtensions().length, 32);
45+
assert.equal(ImportManager.getExtensions().length, 54);
4646
assert(ImportManager.getExtensions().includes('.csv'));
4747
assert(ImportManager.getExtensions().includes('.json'));
4848
assert(ImportManager.getExtensions().includes('.zip'));
@@ -57,23 +57,20 @@ describe('Importer', function () {
5757
assert(ImportManager.getExtensions().includes('.m4a'));
5858

5959
assert(ImportManager.getExtensions().includes('.pdf'));
60-
assert(ImportManager.getExtensions().includes('.json'));
61-
assert(ImportManager.getExtensions().includes('.jsonld'));
62-
assert(ImportManager.getExtensions().includes('.odp'));
63-
assert(ImportManager.getExtensions().includes('.ods'));
64-
assert(ImportManager.getExtensions().includes('.odt'));
65-
assert(ImportManager.getExtensions().includes('.ppt'));
6660
assert(ImportManager.getExtensions().includes('.pptx'));
67-
assert(ImportManager.getExtensions().includes('.rtf'));
6861
assert(ImportManager.getExtensions().includes('.txt'));
69-
assert(ImportManager.getExtensions().includes('.xls'));
7062
assert(ImportManager.getExtensions().includes('.xlsx'));
7163
assert(ImportManager.getExtensions().includes('.xml'));
64+
assert(ImportManager.getExtensions().includes('.docx'));
65+
assert(ImportManager.getExtensions().includes('.html'));
66+
assert(ImportManager.getExtensions().includes('.epub'));
67+
assert(ImportManager.getExtensions().includes('.js'));
68+
assert(ImportManager.getExtensions().includes('.css'));
7269
});
7370

7471
it('gets the correct types', function () {
7572
assert(Array.isArray(ImportManager.getContentTypes()));
76-
assert.equal(ImportManager.getContentTypes().length, 35);
73+
assert.equal(ImportManager.getContentTypes().length, 24);
7774
assert(ImportManager.getContentTypes().includes('image/jpeg'));
7875
assert(ImportManager.getContentTypes().includes('image/png'));
7976
assert(ImportManager.getContentTypes().includes('image/gif'));
@@ -94,26 +91,9 @@ describe('Importer', function () {
9491
assert(ImportManager.getContentTypes().includes('audio/ogg'));
9592
assert(ImportManager.getContentTypes().includes('audio/x-m4a'));
9693

97-
assert(ImportManager.getContentTypes().includes('application/pdf'));
98-
assert(ImportManager.getContentTypes().includes('application/json'));
99-
assert(ImportManager.getContentTypes().includes('application/ld+json'));
100-
assert(ImportManager.getContentTypes().includes('application/vnd.oasis.opendocument.presentation'));
101-
assert(ImportManager.getContentTypes().includes('application/vnd.oasis.opendocument.spreadsheet'));
102-
assert(ImportManager.getContentTypes().includes('application/vnd.oasis.opendocument.text'));
103-
assert(ImportManager.getContentTypes().includes('application/vnd.ms-powerpoint'));
104-
assert(ImportManager.getContentTypes().includes('application/vnd.openxmlformats-officedocument.presentationml.presentation'));
105-
assert(ImportManager.getContentTypes().includes('application/rtf'));
106-
assert(ImportManager.getContentTypes().includes('text/plain'));
107-
assert(ImportManager.getContentTypes().includes('application/vnd.ms-excel'));
108-
assert(ImportManager.getContentTypes().includes('application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'));
109-
assert(ImportManager.getContentTypes().includes('application/xml'));
110-
assert(ImportManager.getContentTypes().includes('application/atom+xml'));
111-
11294
assert(ImportManager.getContentTypes().includes('application/octet-stream'));
11395
assert(ImportManager.getContentTypes().includes('application/json'));
114-
11596
assert(ImportManager.getContentTypes().includes('text/plain'));
116-
11797
assert(ImportManager.getContentTypes().includes('application/zip'));
11898
assert(ImportManager.getContentTypes().includes('application/x-zip-compressed'));
11999
});
@@ -128,17 +108,18 @@ describe('Importer', function () {
128108
});
129109

130110
it('globs extensions correctly', function () {
131-
assert.equal(ImportManager.getGlobPattern(ImportManager.getExtensions()), '+(.jpg|.jpeg|.gif|.png|.svg|.svgz|.ico|.webp|.mp4|.webm|.ogv|.mp3|.wav|.ogg|.m4a|.pdf|.json|.jsonld|.odp|.ods|.odt|.ppt|.pptx|.rtf|.txt|.xls|.xlsx|.xml|.csv|.md|.markdown|.zip)');
111+
const extGlob = '+(.jpg|.jpeg|.gif|.png|.svg|.svgz|.ico|.webp|.mp4|.webm|.ogv|.mp3|.wav|.ogg|.m4a|.pdf|.json|.ods|.odt|.pptx|.rtf|.txt|.xls|.xlsx|.xml|.apkg|.css|.csv|.doc|.docx|.epub|.gpx|.html|.ics|.ipynb|.js|.key|.kml|.md|.mobi|.mov|.otf|.pages|.paprikarecipes|.psd|.py|.skp|.woff|.woff2|.xlsb|.xlsm|.yaml|.zip|.markdown)';
112+
assert.equal(ImportManager.getGlobPattern(ImportManager.getExtensions()), extGlob);
132113
assert.equal(ImportManager.getGlobPattern(ImportManager.getDirectories()), '+(images|content|media|files)');
133114
assert.equal(ImportManager.getGlobPattern(JSONHandler.extensions), '+(.json)');
134115
assert.equal(ImportManager.getGlobPattern(ImageHandler.extensions), '+(.jpg|.jpeg|.gif|.png|.svg|.svgz|.ico|.webp)');
135-
assert.equal(ImportManager.getExtensionGlob(ImportManager.getExtensions()), '*+(.jpg|.jpeg|.gif|.png|.svg|.svgz|.ico|.webp|.mp4|.webm|.ogv|.mp3|.wav|.ogg|.m4a|.pdf|.json|.jsonld|.odp|.ods|.odt|.ppt|.pptx|.rtf|.txt|.xls|.xlsx|.xml|.csv|.md|.markdown|.zip)');
116+
assert.equal(ImportManager.getExtensionGlob(ImportManager.getExtensions()), '*' + extGlob);
136117
assert.equal(ImportManager.getDirectoryGlob(ImportManager.getDirectories()), '+(images|content|media|files)');
137-
assert.equal(ImportManager.getExtensionGlob(ImportManager.getExtensions(), 0), '*+(.jpg|.jpeg|.gif|.png|.svg|.svgz|.ico|.webp|.mp4|.webm|.ogv|.mp3|.wav|.ogg|.m4a|.pdf|.json|.jsonld|.odp|.ods|.odt|.ppt|.pptx|.rtf|.txt|.xls|.xlsx|.xml|.csv|.md|.markdown|.zip)');
118+
assert.equal(ImportManager.getExtensionGlob(ImportManager.getExtensions(), 0), '*' + extGlob);
138119
assert.equal(ImportManager.getDirectoryGlob(ImportManager.getDirectories(), 0), '+(images|content|media|files)');
139-
assert.equal(ImportManager.getExtensionGlob(ImportManager.getExtensions(), 1), '{*/*,*}+(.jpg|.jpeg|.gif|.png|.svg|.svgz|.ico|.webp|.mp4|.webm|.ogv|.mp3|.wav|.ogg|.m4a|.pdf|.json|.jsonld|.odp|.ods|.odt|.ppt|.pptx|.rtf|.txt|.xls|.xlsx|.xml|.csv|.md|.markdown|.zip)');
120+
assert.equal(ImportManager.getExtensionGlob(ImportManager.getExtensions(), 1), '{*/*,*}' + extGlob);
140121
assert.equal(ImportManager.getDirectoryGlob(ImportManager.getDirectories(), 1), '{*/,}+(images|content|media|files)');
141-
assert.equal(ImportManager.getExtensionGlob(ImportManager.getExtensions(), 2), '**/*+(.jpg|.jpeg|.gif|.png|.svg|.svgz|.ico|.webp|.mp4|.webm|.ogv|.mp3|.wav|.ogg|.m4a|.pdf|.json|.jsonld|.odp|.ods|.odt|.ppt|.pptx|.rtf|.txt|.xls|.xlsx|.xml|.csv|.md|.markdown|.zip)');
122+
assert.equal(ImportManager.getExtensionGlob(ImportManager.getExtensions(), 2), '**/*' + extGlob);
142123
assert.equal(ImportManager.getDirectoryGlob(ImportManager.getDirectories(), 2), '**/+(images|content|media|files)');
143124
});
144125

0 commit comments

Comments
 (0)