Feedback wanted: Hiding functionGlobalContext from Context Sidebar

Core Development
1

When we introduced Persistent Context in 0.19 we also added the new Context sidebar that lets you see the contents of context.

A concern has been raised that, in some circumstances, it is not desirable to expose the values of context to the editor. For example, where functionGlobalContext is used to provide API tokens to a flow.

This is probably of less interest to individual users of Node-RED who have full access to the system. But for those running hosted instances of Node-RED, where the user creating flows in the editor may not be the person who setup the instance and its environment, this could be an important issue to consider.

I'm looking at what options we can introduce to reduce the potential problem this brings and would like some feedback, as the approach would potential have consequences for all users.

Before describing the approach, some background to how some of this works.

  1. functionGlobalContext is used to seed the global context object in the runtime
  2. the global context object provides the get/set functions as well as a keys function
  3. the keys function returns the names of all variables held in that context object as a list. There is no indication if the value came from functionGlobalContext or was a value set at runtime by the flow.
  4. the context sidebar calls an admin HTTP api to get the values. That api uses the keys function to get a list of what it needs to return - it doesn't have access to any more information than that.

A propsoal for feedback

Here is what we're proposing to change:

  1. by default, global.keys() will no longer return the names of properties set in functionGlobalContext.
  2. they can still be retrieved by global.get
  3. with no further changes, that will hide the functionGlobalContext values from the context admin http api and therefore from the context sidebar in the editor
  4. a new setting will be introduced alongside functionGlobalContext that restores the behaviour of global.keys returning fGC's key names
  5. the name of this setting is to be decided... exportGlobalContext maybe...?

The reason I would like some feedback on this approach is because it will change the behaviour of global.keys for everyone. That is potentially disruptive if you have a flow that relies on global.keys returning everything.

We could have taken the approach that the new setting would be used to turn off the values appearing in global.keys. That would have less potential for disruption, but we'd rather be secure-by-default and for users to knowingly opt in to their functionGlobalContext values being visible in the editor.

There is a longer term approach around the finer-grained permissions model we have in the editor - where a user could be given flow edit permission, but not permission to read context. That would help address the more general problem of context beyond just that in functionGlobalContext - but isn't a quick change.

Please share your thoughts. As I said at the start, the issue this is trying to address is unlikely to be one many of you need to worry about. However the proposed way to solve it might have an impact.

2

Hi,

I would suggest to add a flag in settings.js to be able to hide/show the tabs in UI.

My issue:
All the global variables in "functionGlobalContext" like port, host etc which are used by different flows are being exposed via 'CONTEXT' tab.

so I would like to hide the tab in UI without changing any other functionality ( Enabling/Disabling the tab ).

Thank you

3

I'd vote having the option in settings.js be used to impliment the changes to hide things.
i.e. you have to edit settings.js to get the new feature.

4

Just to clarify, you'd vote for having to set the flag in order to hide functionGobalContext from calls to global.key, rather than that becoming the default behaviour?

5

Correct, you would have to take action to turn this new function on.

6

I personally don't mind if the default is to hide them as long as there's a way to restore current behaviour via the settings.

7

I think it would/should be safer by default - so should not be visible until you edit settings to turn them back on (so a change of default from today).

8

Just to confirm if my understanding is correct. Implementing this change will not prevent us from using global.keys() inside the function node to manipulate the values that are generated at runtime. Is this correct?

9

Correct. This is only about limiting whether global.keys includes the keys provided by functionGobalContext. Any values set by your flows at runtime would still be returned.

10

If by default they are hidden, can you overwrite them with a normal global.set?
If so you could potentially overwrite something you need without knowing it was set.

11

You can still global.get and global.set the values.

If someone has used global.keys to check if a particular value exists (rather than global.get('FOO') !== undefined ) then yes, they would be affected by this. To me, it's a very unnatural way of checking a value exists.

And that is the judgement we're trying to make. Is that type of code pattern actually used? The documentation doesn't highlight the existence of the keys function, nor do we use it in any examples. I think the risk is low.

12

@ukmoose's comment raises a good question. Do you want to hide the existence of properties set in functionGlobalContext or just their values? If the latter, why not just display the values as ●●●●●? Then the developer will know they have been set, but the user without access to settings.js will not be able to see them.

13

That is an option. I was hoping to avoid it because it complicates the implementation quite a lot. As I described in the original post, the HTTP Admin api uses the keys/get/set api that context provides. That api doesn't tell you where the value came from - whether it was from fGC or set at runtime. For the admin api to be able to censor values, it would need more access to the internals of context then it does today. Of course, the complexity of implementation isn't a primary factor in deciding the right approach - but it does have a bearing. This requirement has come up from one person, no-one else has chipped into this thread to support the need for it. That makes it hard to assess how much time and effort needs to be spent on it.

14

By all means, do the cost/benefit check -- your time is valuable. Perhaps the user's requirement could be met with the node-red-contrib-credentials node. I've used it happily but not recently. IIRC, it requires a message to flow through the node to pick up credentials instead of defining a global.

15

If the default behavior is to hide fGC, it would be good to provide a means to easily change the behavior to current, I think.
For example.

exportGlobalContext: 'all'  // or true
16

Hi @HirokiUchikawa

Yes, the proposal above says exportGobalContext would be used to restore the existing behaviour.