🐛 Fixed perms error when building public assets

closes: https://github.com/TryGhost/Ghost/issues/13739

- Ghost cannot write to the core folder in correctly configured production installations
- Built assets therefore need to be written to the content directory
- Ghost does not overwrite anything in the content folder as part of an upgrade, therefore static files that are provided by Ghost
  must still live inside /core
- So as a result, we now have core/frontend/public and content/public

.gitignore

@@ -106,6 +106,7 @@ projectFilesBackup
 /content/images/**/*
 /content/media/**/*
 /content/files/**/*
+/content/public/*
 /content/adapters/storage/**/*
 /content/adapters/scheduling/**/*
 !/content/themes/casper
@@ -125,8 +126,6 @@ test/coverage
 # Built asset files
 /core/built
 /core/server/web/admin/views/*.html
-/core/frontend/public/*.min.css
-/core/frontend/public/*.min.js
 
 # Caddyfile - for local development with ssl + caddy
 Caddyfile

.npmignore

@@ -13,6 +13,8 @@ content/adapters/**
 !content/adapters/README.md
 content/apps/**
 !content/apps/README.md
+content/public/**
+!content/public/README.md
 content/data/**
 !content/data/README.md
 content/images/**

Gruntfile.js

@@ -75,7 +75,6 @@ module.exports = function (grunt) {
                     'core/shared/**/*.js',
                     'core/frontend/**/*.js',
                     'core/frontend/src/**/*.css',
-                    '!core/frontend/public/**',
                     'core/*.js',
                     'index.js',
                     'config.*.json',

content/public/README.md

@@ -0,0 +1,3 @@
+# Content / Public
+
+Ghost will store any built assets here. This goes hand in hand with core/frontend/public where static assets are stored.

core/frontend/services/card-assets/service.js

@@ -3,12 +3,12 @@ const _ = require('lodash');
 const path = require('path');
 const fs = require('fs').promises;
 const logging = require('@tryghost/logging');
+const config = require('../../../shared/config');
 
 class CardAssetService {
     constructor(options = {}) {
-        // @TODO: use our config paths concept
+        this.src = options.src || path.join(config.get('paths').assetSrc, 'cards');
-        this.src = options.src || path.join(__dirname, '../../src/cards');
+        this.dest = options.dest || config.getContentPath('public');
-        this.dest = options.dest || path.join(__dirname, '../../public');
         this.minifier = new Minifier({src: this.src, dest: this.dest});
 
         if ('config' in options) {
@@ -90,12 +90,12 @@ class CardAssetService {
     /**
      * A theme can declare which cards it supports, and we'll do the rest
      *
-     * @param {Array|boolean} config
+     * @param {Array|boolean} cardAssetConfig
      * @returns
      */
-    async load(config) {
+    async load(cardAssetConfig) {
-        if (config) {
+        if (cardAssetConfig) {
-            this.config = config;
+            this.config = cardAssetConfig;
         }
 
         await this.clearFiles();

core/frontend/web/middleware/serve-public-file.js

@@ -11,10 +11,16 @@ const messages = {
     fileNotFound: 'File not found'
 };
 
-function createPublicFileMiddleware(file, type, maxAge) {
+function createPublicFileMiddleware(location, file, mime, maxAge) {
     let content;
-    const publicFilePath = config.get('paths').publicFilePath;
+    // These files are provided by Ghost, and therefore live inside of the core folder
-    const filePath = file.match(/^public/) ? path.join(publicFilePath, file.replace(/^public/, '')) : path.join(publicFilePath, file);
+    const staticFilePath = config.get('paths').publicFilePath;
+    // These files are built on the fly, and must be saved in the content folder
+    const builtFilePath = config.getContentPath('public');
+
+    let locationPath = location === 'static' ? staticFilePath : builtFilePath;
+
+    const filePath = file.match(/^public/) ? path.join(locationPath, file.replace(/^public/, '')) : path.join(locationPath, file);
     const blogRegex = /(\{\{blog-url\}\})/g;
 
     return function servePublicFileMiddleware(req, res, next) {
@@ -24,7 +30,7 @@ function createPublicFileMiddleware(file, type, maxAge) {
         }
 
         // send image files directly and let express handle content-length, etag, etc
-        if (type.match(/^image/)) {
+        if (mime.match(/^image/)) {
             return res.sendFile(filePath, (err) => {
                 if (err && err.status === 404) {
                     // ensure we're triggering basic asset 404 and not a templated 404
@@ -57,13 +63,13 @@ function createPublicFileMiddleware(file, type, maxAge) {
 
             let str = buf.toString();
 
-            if (type === 'text/xsl' || type === 'text/plain' || type === 'application/javascript') {
+            if (mime === 'text/xsl' || mime === 'text/plain' || mime === 'application/javascript') {
                 str = str.replace(blogRegex, urlUtils.urlFor('home', true).replace(/\/$/, ''));
             }
 
             content = {
                 headers: {
-                    'Content-Type': type,
+                    'Content-Type': mime,
                     'Content-Length': Buffer.from(str).length,
                     ETag: `"${crypto.createHash('md5').update(str, 'utf8').digest('hex')}"`,
                     'Cache-Control': `public, max-age=${maxAge}`
@@ -78,8 +84,8 @@ function createPublicFileMiddleware(file, type, maxAge) {
 
 // ### servePublicFile Middleware
 // Handles requests to robots.txt and favicon.ico (and caches them)
-function servePublicFile(file, type, maxAge) {
+function servePublicFile(location, file, type, maxAge) {
-    const publicFileMiddleware = createPublicFileMiddleware(file, type, maxAge);
+    const publicFileMiddleware = createPublicFileMiddleware(location, file, type, maxAge);
 
     return function servePublicFileMiddleware(req, res, next) {
         if (req.path === '/' + file) {

core/frontend/web/site.js

@@ -104,15 +104,15 @@ module.exports = function setupSiteApp(options = {}) {
     siteApp.use(mw.serveFavicon());
 
     // Serve sitemap.xsl file
-    siteApp.use(mw.servePublicFile('sitemap.xsl', 'text/xsl', constants.ONE_DAY_S));
+    siteApp.use(mw.servePublicFile('static', 'sitemap.xsl', 'text/xsl', constants.ONE_DAY_S));
 
     // Serve stylesheets for default templates
-    siteApp.use(mw.servePublicFile('public/ghost.css', 'text/css', constants.ONE_HOUR_S));
+    siteApp.use(mw.servePublicFile('static', 'public/ghost.css', 'text/css', constants.ONE_HOUR_S));
-    siteApp.use(mw.servePublicFile('public/ghost.min.css', 'text/css', constants.ONE_YEAR_S));
+    siteApp.use(mw.servePublicFile('static', 'public/ghost.min.css', 'text/css', constants.ONE_YEAR_S));
 
     // Card assets
-    siteApp.use(mw.servePublicFile('public/cards.min.css', 'text/css', constants.ONE_YEAR_S));
+    siteApp.use(mw.servePublicFile('built', 'public/cards.min.css', 'text/css', constants.ONE_YEAR_S));
-    siteApp.use(mw.servePublicFile('public/cards.min.js', 'text/js', constants.ONE_YEAR_S));
+    siteApp.use(mw.servePublicFile('built', 'public/cards.min.js', 'text/js', constants.ONE_YEAR_S));
 
     // Serve blog images using the storage adapter
     siteApp.use(STATIC_IMAGE_URL_PREFIX, mw.handleImageSizes, storage.getStorage('images').serve());
@@ -147,7 +147,7 @@ module.exports = function setupSiteApp(options = {}) {
     debug('Themes done');
 
     // Serve robots.txt if not found in theme
-    siteApp.use(mw.servePublicFile('robots.txt', 'text/plain', constants.ONE_HOUR_S));
+    siteApp.use(mw.servePublicFile('static', 'robots.txt', 'text/plain', constants.ONE_HOUR_S));
 
     // site map - this should probably be refactored to be an internal app
     sitemapHandler(siteApp);

core/shared/config/defaults.json

@@ -16,7 +16,8 @@
     "useMinFiles": true,
     "paths": {
         "contentPath": "content/",
-        "fixtures": "core/server/data/schema/fixtures/fixtures"
+        "fixtures": "core/server/data/schema/fixtures/fixtures",
+        "assetSrc": "core/frontend/src"
     },
     "adapters": {
         "sso": {

core/shared/config/helpers.js

@@ -84,6 +84,8 @@ const getContentPath = function getContentPath(type) {
         return path.join(this.get('paths:contentPath'), 'data/');
     case 'settings':
         return path.join(this.get('paths:contentPath'), 'settings/');
+    case 'public':
+        return path.join(this.get('paths:contentPath'), 'public/');
     default:
         // new Error is allowed here, as we do not want config to depend on @tryghost/error
         // @TODO: revisit this decision when @tryghost/error is no longer dependent on all of ghost-ignition

test/unit/frontend/services/card-assets.test.js

@@ -73,6 +73,19 @@ describe('Card Asset Service', function () {
         cardAssets.files.should.eql(['cards.min.css']);
     });
 
+    it('can correctly load nothing when config is false', async function () {
+        const cardAssets = new CardAssetService({
+            src: srcDir,
+            dest: destDir
+        });
+
+        await fs.writeFile(path.join(srcDir, 'css', 'test.css'), '.test { color: #fff }');
+
+        await cardAssets.load(false);
+
+        cardAssets.files.should.eql([]);
+    });
+
     it('can clearFiles', async function () {
         const cardAssets = new CardAssetService({
             src: srcDir,

test/unit/frontend/web/middleware/serve-public-file.test.js

@@ -1,6 +1,7 @@
 const should = require('should');
 const sinon = require('sinon');
 const fs = require('fs-extra');
+const path = require('path');
 const servePublicFile = require('../../../../../core/frontend/web/middleware/serve-public-file');
 
 describe('servePublicFile', function () {
@@ -19,20 +20,20 @@ describe('servePublicFile', function () {
     });
 
     it('should return a middleware', function () {
-        const result = servePublicFile('robots.txt', 'text/plain', 3600);
+        const result = servePublicFile('static', 'robots.txt', 'text/plain', 3600);
 
         result.should.be.a.Function();
     });
 
     it('should skip if the request does NOT match the file', function () {
-        const middleware = servePublicFile('robots.txt', 'text/plain', 3600);
+        const middleware = servePublicFile('static', 'robots.txt', 'text/plain', 3600);
         req.path = '/favicon.ico';
         middleware(req, res, next);
         next.called.should.be.true();
     });
 
     it('should load the file and send it', function () {
-        const middleware = servePublicFile('robots.txt', 'text/plain', 3600);
+        const middleware = servePublicFile('static', 'robots.txt', 'text/plain', 3600);
         const body = 'User-agent: * Disallow: /';
         req.path = '/robots.txt';
 
@@ -58,11 +59,11 @@ describe('servePublicFile', function () {
     });
 
     it('should send the correct headers', function () {
-        const middleware = servePublicFile('robots.txt', 'text/plain', 3600);
+        const middleware = servePublicFile('static', 'robots.txt', 'text/plain', 3600);
         const body = 'User-agent: * Disallow: /';
         req.path = '/robots.txt';
 
-        sinon.stub(fs, 'readFile').callsFake(function (file, cb) {
+        let fileStub = sinon.stub(fs, 'readFile').callsFake(function (file, cb) {
             cb(null, body);
         });
 
@@ -72,7 +73,9 @@ describe('servePublicFile', function () {
         };
 
         middleware(req, res, next);
+
         next.called.should.be.false();
+        fileStub.firstCall.args[0].should.endWith('core/frontend/public/robots.txt');
         res.writeHead.called.should.be.true();
         res.writeHead.args[0][0].should.equal(200);
         res.writeHead.calledWith(200, sinon.match.has('Content-Type')).should.be.true();
@@ -82,7 +85,7 @@ describe('servePublicFile', function () {
     });
 
     it('should replace {{blog-url}} in text/plain', function () {
-        const middleware = servePublicFile('robots.txt', 'text/plain', 3600);
+        const middleware = servePublicFile('static', 'robots.txt', 'text/plain', 3600);
         const body = 'User-agent: {{blog-url}}';
         req.path = '/robots.txt';
 
@@ -103,7 +106,7 @@ describe('servePublicFile', function () {
     });
 
     it('should 404 for ENOENT on general files', function () {
-        const middleware = servePublicFile('robots.txt', 'text/plain', 3600);
+        const middleware = servePublicFile('static', 'robots.txt', 'text/plain', 3600);
         req.path = '/robots.txt';
 
         sinon.stub(fs, 'readFile').callsFake(function (file, cb) {
@@ -122,4 +125,27 @@ describe('servePublicFile', function () {
         next.called.should.be.true();
         next.calledWith(sinon.match({errorType: 'NotFoundError', code: 'PUBLIC_FILE_NOT_FOUND'})).should.be.true();
     });
+
+    it('can serve a built asset file as well as public files', function () {
+        const middleware = servePublicFile('built', 'something.css', 'text/css', 3600);
+        const body = '.foo {bar: baz}';
+        req.path = '/something.css';
+
+        let fileStub = sinon.stub(fs, 'readFile').callsFake(function (file, cb) {
+            cb(null, body);
+        });
+
+        res = {
+            writeHead: sinon.spy(),
+            end: sinon.spy()
+        };
+
+        middleware(req, res, next);
+
+        next.called.should.be.false();
+        res.writeHead.called.should.be.true();
+        res.writeHead.args[0][0].should.equal(200);
+
+        fileStub.firstCall.args[0].should.endWith('content/public/something.css');
+    });
 });

test/unit/shared/config/loader.test.js

@@ -96,6 +96,7 @@ describe('Config Loader', function () {
             Object.keys(pathConfig).should.eql([
                 'contentPath',
                 'fixtures',
+                'assetSrc',
                 'appRoot',
                 'corePath',
                 'clientAssets',