Discord.js: Experiment with private fields (Remove .token property from everything)

[AlexejheroYTB]

1. No one uses it - The token property is the same as the token used in bot.login, so that can just be used instead
2. It's problematic for open evals - Open evals are very hard to implement because every object has a .token property

Suggestion
Remove the token property on objects, or at least make it inaccessible from outside the class.

[DevYukine]

1. Thats for internal usage, d.js needs to use the token after you login for rest calls (such as fetching messages from a channel)

2. Don't ever have an open eval, eval is very insecure in itself and is not made to be open, dont even try to get around that even with "libraries" which allow to sandbox your enviroment you can never be sure that users can't escape it so you should not make an eval open and also d.js is not responsible for you having such a feature.

EDIT:

make it inaccessible from outside the class.

how is that supposed to work? in javascript there is no such thing as access modifiers (atleast not in the current nodejs v8 version) and the library is not supposed to "protect" you from your own mistakes, you should just not allow other people to execute code on your application

[tipakA]

client.token is used for every call to the api.
Removing it because it makes your open eval problematic does not solve the problem - you're giving access to most powerful tool your bot has. client.token is honestly one of least interesting things someone could get with open eval.
Javascript does not have a concept of private properties, so there is no way to make it inaccessible from outside

[kyranet]

how is that supposed to work? in javascript there is no such thing as access modifiers (atleast not in the current nodejs v8 version)

Private fields (supported since Node.js 12.4.0, check here):

class A {
    #thing;
}

We're targetting Node.js 12 soon in master, this is doable, though we'll need to change some things around since REST needs to access to the token.

[AlexejheroYTB]

For node 8, closures can simulate private properties

[AlexejheroYTB]

I do not have an open eval, but I have to rely on a Google library to disallow code injection... (which has its bugs...)

[kyranet]

For node 8, closures can simulate private properties

No. They're flawed, which is why most JS engines went for a WeakMap approach

// IIFE to "hide" the weakmap. In single-class structures this is not needed.
(function Client() {
    const weakmap = new WeakMap();

    return class Client {
        constructor(options) {
            this.options;
            // Do constructor's stuff
        }

        login(token) {
            // similar to `this.#token = token`
            weakmap.set(this, token);
            // Do login stuff
        }
    }
})();

Besides, we're targeting latest LTS, which currently is Node.js 12, if we go into private fields, we'll do with #token, not with WeakMaps or closures.

[iCrawl]

I do not have an open eval, but I have to rely on a Google library to disallow code injection... (which has its bugs...)

Sounds like a XY problem.

We are most likely going to experiment with private fields but certainly not anytime soon nor because of this specific use/edge case.

[AlexejheroYTB]

Not an XY problem. I'm using Google's Blockly library to allow users to make their own bot using puzzle pieces. Unfortunately, despite all my efforts, Blockly sometimes allows code injection due to bugs.

[tipakA]

That's exactly an XY problem.
You're trying to "secure" Blockly by modifying discord.js to not allow access to token.

[iCrawl]

That is exactly what makes it XY.

Another library has bugs/issues, hence we should change something to "fix" that library.

[AlexejheroYTB]

I'm not trying to "fix" the library, I'm trying to minimize the damage in case something unexpected happens. That's why I didn't even bring it up.

[DevYukine]

I dont think you exactly get what ICrawl and tipakA are trying to say, this library should not do changes for you because it could "damage" your application which relies on an external library that lets people inject code. This is not an issue with discord.js but with Google's Blockly library. This would also be a major change (atleast for the Client#token property remove).

[AlexejheroYTB]

This is not about my bot. _That's why I didn't bring it up in the first place..._ There are other bots out there which struggle with open evals sandboxing.
Plus, there is no reason to leave it public, when possible otherwise.