Improved theme asset handling - permissible assets

refs: https://github.com/TryGhost/Team/issues/1633

- this makes /assets/ a more permissible folder - it can serve anything _except_ hbs files
- meanwhile the root folder becomes less permissible, and won't serve theme dev files commonly found in the root
This commit is contained in:
Hannah Wolfe 2022-05-16 18:54:44 +01:00
parent d9e6dfe97e
commit 5090d75d96
No known key found for this signature in database
GPG Key ID: AB586C3B5AE5C037
2 changed files with 76 additions and 7 deletions

View File

@ -5,17 +5,24 @@ const themeEngine = require('../../services/theme-engine');
const express = require('../../../shared/express');
function isDeniedFile(file) {
const deniedFileTypes = ['.hbs', '.md', '.json'];
const ext = path.extname(file);
const deniedFileTypes = ['.hbs', '.md', '.json', '.lock', '.log'];
const deniedFiles = ['gulpfile.js', 'gruntfile.js'];
return deniedFileTypes.includes(ext);
const ext = path.extname(file);
const base = path.basename(file);
return deniedFiles.includes(base) || deniedFileTypes.includes(ext);
}
function isAllowedFile(file) {
const allowedFiles = ['manifest.json'];
const allowedPath = '/assets/';
const alwaysDeny = ['.hbs'];
const ext = path.extname(file);
const base = path.basename(file);
return allowedFiles.includes(base);
return allowedFiles.includes(base) || (file.startsWith(allowedPath) && !alwaysDeny.includes(ext));
}
function forwardToExpressStatic(req, res, next) {
@ -25,14 +32,15 @@ function forwardToExpressStatic(req, res, next) {
const configMaxAge = config.get('caching:theme:maxAge');
express.static(themeEngine.getActive().path,
{maxAge: (configMaxAge || configMaxAge === 0) ? configMaxAge : constants.ONE_YEAR_MS}
express.static(themeEngine.getActive().path, {
maxAge: (configMaxAge || configMaxAge === 0) ? configMaxAge : constants.ONE_YEAR_MS
}
)(req, res, next);
}
function staticTheme() {
return function denyStatic(req, res, next) {
if (!isAllowedFile(req.path) && isDeniedFile(req.path)) {
if (!isAllowedFile(req.path.toLowerCase()) && isDeniedFile(req.path.toLowerCase())) {
return next();
}

View File

@ -59,6 +59,39 @@ describe('staticTheme', function () {
});
});
it('should skip for .lock file', function (done) {
req.path = 'yarn.lock';
staticTheme()(req, res, function next() {
activeThemeStub.called.should.be.false();
expressStaticStub.called.should.be.false();
done();
});
});
it('should skip for gulp file', function (done) {
req.path = 'gulpfile.js';
staticTheme()(req, res, function next() {
activeThemeStub.called.should.be.false();
expressStaticStub.called.should.be.false();
done();
});
});
it('should skip for Grunt file', function (done) {
req.path = 'Gulpfile.js';
staticTheme()(req, res, function next() {
activeThemeStub.called.should.be.false();
expressStaticStub.called.should.be.false();
done();
});
});
it('should call express.static for .css file', function (done) {
req.path = 'myvalidfile.css';
@ -123,4 +156,32 @@ describe('staticTheme', function () {
done();
});
});
it('should NOT skip if file is in assets', function (done) {
req.path = '/assets/whatever.json';
staticTheme()(req, res, function next() {
// Specifically gets called twice
activeThemeStub.calledTwice.should.be.true();
expressStaticStub.called.should.be.true();
// Check that express static gets called with the theme path + maxAge
should.exist(expressStaticStub.firstCall.args);
expressStaticStub.firstCall.args[0].should.eql('my/fake/path');
expressStaticStub.firstCall.args[1].should.be.an.Object().with.property('maxAge');
done();
});
});
it('should skip for .hbs file EVEN in /assets', function (done) {
req.path = '/assets/mytemplate.hbs';
staticTheme()(req, res, function next() {
activeThemeStub.called.should.be.false();
expressStaticStub.called.should.be.false();
done();
});
});
});