[apps] App attempts require() of module in same directory -> "ERROR: Unsafe App require"

[k-j-kleist]

My app works fine with all code inside ../content/apps/fayspook/index.js as in https://github.com/TryGhost/Ghost/wiki/Apps-Getting-Started-for-Ghost-Devs.

However, when I refactor the code by placing the app logic in a separate module client.js which I require('./client') from index.js:

ERROR: Unsafe App require: ..\..\..\..\..\content\apps\client
 The app will not be loaded
 Check with the app creator, or read the app documentation for more details on app requirements

This should work, shouldn't it? Or do these test cases fail, or aren't even performed?:

https://github.com/TryGhost/Ghost/blob/c7713c1d2730d548591ff6b68ac7d0d423516f7e/core/test/utils/fixtures/app/good.js

https://github.com/TryGhost/Ghost/blob/c7713c1d2730d548591ff6b68ac7d0d423516f7e/core/test/utils/fixtures/app/goodlib.js

Trying to find out what is going on here, I added some logging to https://github.com/TryGhost/Ghost/blob/stable/core/server/apps/sandbox.js#L19 ...

AppSandbox.prototype.loadModule = function loadModuleSandboxed(modulePath) {
    // Set loaded modules parent to this
    var self = this,
        // kleist: uses OLD modulePath
        moduleDir = path.dirname(modulePath),
        parentModulePath = self.opts.parent || module.parent,
        appRoot = self.opts.appRoot || moduleDir,
        currentModule,
        nodeRequire;

    // Resolve the modules path
    // kleist
    console.log('OLD modulePath: ' + modulePath);
    // kleist: note re-use of variable
    modulePath = Module._resolveFilename(modulePath, parentModulePath);
    // kleist
    console.log('NEW modulePath: ' + modulePath);

    // Instantiate a Node Module class
    currentModule = new Module(modulePath, parentModulePath);

    // Grab the original modules require function
    nodeRequire = currentModule.require;

    // Set a new proxy require function
    currentModule.require = function requireProxy(module) {
        // check whitelist, plugin config, etc.
        if (_.contains(self.opts.blacklist, module)) {
            throw new Error('Unsafe App require: ' + module);
        }

        var firstTwo = module.slice(0, 2),
            resolvedPath,
            relPath,
            innerBox,
            newOpts;

        // Load relative modules with their own sandbox
        if (firstTwo === './' || firstTwo === '..') {
            // Get the path relative to the modules directory
            resolvedPath = path.resolve(moduleDir, module);
            // kleist
            console.log('moduleDir: ' + moduleDir);
            console.log('appRoot: ' + appRoot);
            console.log('resolvedPath: ' + resolvedPath);

            // Check relative path from the appRoot for outside requires
            relPath = path.relative(appRoot, resolvedPath);
            if (relPath.slice(0, 2) === '..') {
                throw new Error('Unsafe App require: ' + relPath);
            }
[ - - - ]

... causing this output:

OLD modulePath: ..\..\..\content\apps\fayspook
NEW modulePath: C:\projects\Ghost\content\apps\fayspook\index.js
moduleDir: ..\..\..\content\apps
appRoot: C:\projects\Ghost\content\apps\fayspook
resolvedPath: C:\content\apps\client

ERROR: Unsafe App require: ..\..\..\..\..\content\apps\client

Maybe something fishy here with the variable modulePath?

• Finally, I'm very impressed by this sandbox approach in Ghost! Admittedly, my worn-out brain gets pretty dizzy reading the code. But reading "Hacking Node require" helps.

[ErisDS]

I think that you're looking along the right lines -

resolvedPath is achieved by doing path.resolve(moduleDir, module); - what is module in this context (that's not being console logged - is it ./client?). I'd suggest perhaps it should be resolved with appRoot, rather than moduleDir but it depends on what module is.

The tests are probably making an incorrect assumption somewhere to do with what the paths are like here, it would be interesting to see if you could recreate a failure case or put the console logging in the tests to see how it behaves differently.

I really recommend playing with it some more - will happily merge a fix, but this is not being actively worked on or really considered a working part of the codebase 😉

[k-j-kleist]

Yes, module is ./client.

Will stick to stuffing my code in index.js as for now. If and when the current app approach stabilizes, I'd be happy to give this some more love.

[ErisDS]

I don't think this needs to be closed - it is a bug & should be tracked. I was just trying to infer that it's not considered high priority, as most bugs are.

[program247365]

Wanted to dive in and help out on a 'beginner' issue. Spent some time on this, and using these files:

Index.js:

var App = require('ghost-app'),

    MyApp;

MyApp = App.extend({

    install: function () {},

    uninstall: function () {},

    activate: function () {
        this.ghost.api.posts.read(3).then(function (post) {
            console.log(post.title);
        }).catch(function(e) {
            console.log(e);
        });
    },

    deactivate: function () {}

});

module.exports = MyApp;

package.json

{
  "name": "example-app",
  "version": "0.0.1",
  "dependencies": {
    "ghost-app": "0.0.3" //note: the wiki page says 0.0.2 and I don't know how up-to-date that is
  }
}

Structure:

vagrant@vagrant-ubuntu-trusty-64:~/code/Ghost$ tree content/apps/example-ap
content/apps/example-app
├── index.js
└── package.json

I get:

vagrant@vagrant-ubuntu-trusty-64:~/code/Ghost$ npm start

> ghost@0.7.1 start /home/vagrant/code/Ghost
> node index

Migrations: Up to date at version 004
{ [InternalServerError: TypeError: Cannot read property 'status' of undefined]
  message: [TypeError: Cannot read property 'status' of undefined],
  stack: 'Error\n    at Error.InternalServerError (/home/vagrant/code/Ghost/core/server/errors/internal-server-error.js:6:18)\n    at Object.errors.handleAPIError (/home/vagrant/code/Ghost/core/server/err�
ors/index.js:234:33)\n    at handleError (/home/vagrant/code/Ghost/core/server/api/utils.js:186:31)\n    at tryCatcher (/home/vagrant/code/Ghost/node_modules/bluebird/js/main/util.js:24:31)\n    at Promis�
e._settlePromiseFromHandler (/home/vagrant/code/Ghost/node_modules/bluebird/js/main/promise.js:454:31)\n    at Promise._settlePromiseAt (/home/vagrant/code/Ghost/node_modules/bluebird/js/main/promise.js:5�
30:18)\n    at Promise._settlePromises (/home/vagrant/code/Ghost/node_modules/bluebird/js/main/promise.js:646:14)\n    at Async._drainQueue (/home/vagrant/code/Ghost/node_modules/bluebird/js/main/async.js�
:177:16)\n    at Async._drainQueues (/home/vagrant/code/Ghost/node_modules/bluebird/js/main/async.js:187:10)\n    at Async.drainQueues (/home/vagrant/code/Ghost/node_modules/bluebird/js/main/async.js:15:1�
4)\n    at process._tickDomainCallback (node.js:492:13)',
  code: 500,
  errorType: 'InternalServerError' }
Ghost is running in development...
Listening on 127.0.0.1:2368
Url configured as: http://localhost:2368
Ctrl+C to shut down

I'm willing to dig deeper, if someone can give me some pointers.

Or if there are other more important bugs/issues that a first-timer to the project can help out with, please direct me to those. I can and want to do both client and server work in ghost.

[ErisDS]

Hi there @program247365, sorry to send you off on a wild goose chase with this! Since it was raised, much of Ghost has changed and apps (which are not yet officially supported) no longer have permissions to access the API. That means requests through the api proxy fail with these big unhelpful errors.

I'm going to close this for now with the later label that all the apps issues have, so that we can revisit it later when we focus on bringing apps up to official support. For now I think this can be left.

As for other issues to work on, I've been going through the repo today closing as many of the issues that are stalled or not ready to be worked on as possible, and marking things with fix wanted and in progress where I can. I'm not quite finished and I'll do another pass-through looking for ones to tag beginner soon as well, but I hope it should now be clearer what needs doing!

[program247365]

Gotcha. No problem at all. Just looking to start contributing. Thanks for updating it. I'll keep an eye on the beginner label filter.

Thanks!

[nicolad]

Hi, I found a workaround using this:

const goodreads = require( path.resolve( path.dirname(module.parent.filename), '../../../../../content/apps/my-app/goodreads-api-node/index.js' ));