From 5090d75d963d865f34284e9d238f4b3afa4bc41f Mon Sep 17 00:00:00 2001
From: Hannah Wolfe <github.erisds@gmail.com>
Date: Mon, 16 May 2022 18:54:44 +0100
Subject: [PATCH] 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
---
 core/frontend/web/middleware/static-theme.js  | 22 ++++---
 .../web/middleware/static-theme.test.js       | 61 +++++++++++++++++++
 2 files changed, 76 insertions(+), 7 deletions(-)

diff --git a/core/frontend/web/middleware/static-theme.js b/core/frontend/web/middleware/static-theme.js
index 7c775ba000..af07bc3e6c 100644
--- a/core/frontend/web/middleware/static-theme.js
+++ b/core/frontend/web/middleware/static-theme.js
@@ -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();
         }
 
diff --git a/test/unit/frontend/web/middleware/static-theme.test.js b/test/unit/frontend/web/middleware/static-theme.test.js
index ab78eca119..24e7303c7c 100644
--- a/test/unit/frontend/web/middleware/static-theme.test.js
+++ b/test/unit/frontend/web/middleware/static-theme.test.js
@@ -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();
+        });
+    });
 });