Node: Discussion: private fields and util.inspect

[jasnell]

Now that we have support for private class fields... we need to figure out how (and if) we want to support them with regards to util.inspect()...

Right now, those properties are hidden from the output even when showHidden is true...

class F { #foo = 1; }
const f = new F()
util.inspect(f)  // returns 'F {}'
util.inspect(f, { showHidden: true }) // returns 'F {}'

How should we handle these with inspect?

[devsnek]

https://chromium-review.googlesource.com/c/v8/v8/+/1546559

[BridgeAR]

@jasnell I definitely think we should add support for these behind the showHidden option. We can visualize them with the hash sign just as they are defined using a colon as separator as we do for all other properties as well.

But I guess your question was also (or mainly) in the direction of how to retrieve the information. I personally think the work by @joyeecheung looks very promising (while I agree that we could just extend the existing API as discussed in the comments of the reverenced PR).

[jasnell]

Yep, thank you for linking the V8 issue @devsnek .. I knew that conversation was happening but I had lost track of where it was. I opened this issue largely just so we could track the discussion and see if there were any actual objections to including those fields.

[cjihrig]

FWIW, I'm in favor of including private fields in util.inspect() output.

[joyeecheung]

From the feedback we've got from https://chromium-review.googlesource.com/c/v8/v8/+/1546559 I think we should try using the inspector to implement the inspection of private fields for a start. It could be built on top of https://github.com/nodejs/node/pull/27110 for more readable interactions with the inspector.

[domenic]

As a library author I would prefer if these were not exposed through util.inspect(). I am trying to hide my implementation details, and in the case of jsdom, emulate native classes. You do not get to see the private fields of Date, or ArrayBuffer, or Map, when you util.inspect() those.

But with this in place, people could read my private data easily at runtime:

class X {
  #secret = generateSecretNumber();
}

const x = new X();
const secret = /#secret: '([^']+)'/.exec(util.inspect(x))[1];

console.log("no longer a secret: ", secret);

Introducing this into Node.js would force me to avoid using private fields, and use WeakMaps instead, since those cannot be discovered by util.inspect() in this way.

In other words, this feature effectively makes private fields no longer useful, as they're just symbols that require different awkward contortions to access (grepping through util.inspect output, versus using indices into getOwnPropertySymbols).

I would prefer that private fields only be visible to users of the inspector, not to unprivileged Node.js code.

[devsnek]

I would prefer that private fields only be visible to users of the inspector, not to unprivileged Node.js code.

the inspector itself is also available to unprivileged node code, and returns the private fields as a nice object you don't need to use regex on.

[domenic]

My understanding is that you need to be started with a flag to include the inspector, per https://nodejs.org/en/docs/guides/debugging-getting-started/. That would be "privileged" code then, in my (admittedly imprecise) terminology.

[cjihrig]

You can start the inspector at runtime via inspector.open().

[domenic]

Oh :(. Then I guess Node.js is just not a feasible runtime for writing encapsulated code.

[joyeecheung]

@domenic I don't think we are going to implement what the snippet shows exactly - this is supposed to be only available when showHidden: true is passed to util.inspect(). Also, util.inspect() is capable of inspecting weak maps entries with showHidden: true as well - provided that you are able to get hold of the weak map, but then you may be able to use inspector to um, inspect that scope chain to get hold of it.

FWIW you can break encapsulation of any runtime that embeds v8 with post-mortem support by using the JS API of llnode - take a core dump via gcore (or something else that capture a core dump) and then iterate over the heap to find your object, then you can even break closure encapsulations by inspecting context that references your objects. It's just too slow to do this hack though.

[domenic]

I am just used to the browser where we have actual encapsulation from other code. It is sad and disappointing to find out that Node does not have the same level of encapsulation.

But given that it doesn't, I guess I don't have much more to add to this conversation.

[joyeecheung]

@domenic Node.js has a history of treating all user code (third party or not) as trusted and equal, I think that's what makes its privacy model different from the browser.

Would it help if we make inspector part of the experimental policy? Although whether it can be made opt-in or opt-out depends on how much breakage the ecosystem can take since inspector.open() has been out there since 8.x.

It would be quite hacky to use inspector to implement this (either in core or in the user land) though, as you need to inspect your own scope chain which means you need to tell the debugger to pause at your code and do a lot of stuff that depends on the exact shape of the functions in the call stack.

What's your primary concern about thing encapsulation being broken? That the users get to see the fields or that they get to use them programmatically? It would be quite difficult to use these programmatically if the private fields are not primitives, as the inspector would only give you a preview of them, and doing all these hacky things would bring a lot of overhead as well.

[ljharb]

Please do not show private fields anywhere, just like you wouldn't show closed-over variables when you log a function. This is the domain of debugger, not console.log.

[jasnell]

@domenic and @ljharb ... this is precisely the kind of feedback I was hoping to solicit when opening the issue so thank you for stepping in. Within core, we're always considered util.inspect() to be a debugging tool so the "This is the domain of debugger, not console.log" statement really highlights the difficulty here.

Perhaps a compromise here would be to show the private field names but not the actual values, specifically to make it clear that, yes, there is private state at play but you'll have to grab the inspector to get to the values.

[joyeecheung]

BTW, I just noticed that console.log does not use showHidden: true, to alter this you only have two choices:

1. -r ./options.js where options.js contains require("util").inspect.defaultOptions.showHidden=true
2. Get the symbol kGetInspectOptions from Reflect and patch the property behind it (which is a hack)

But I guess without console.log doing this by default, there is less value in showing the private fields in util.inspect().

[ljharb]

@jasnell i think it shouldn’t even show that. Private class fields are conceptually nothing more than closed-over variables for instances - if you wouldn’t show the names of closed-over variables used in a function when inspecting a function, you shouldn’t show the names of private fields when inspecting an instance.

[ljharb]

Put another way, it was a very explicit and critical design goal that it be impossible, modulo function toString, to expose even the existence of private fields - if node exposes it, it will be destroying a vital part of the design of the feature.

[jasnell]

Can definitely respect that and see the connection to closed over variables. Is there an approach here that Would make sense outside of stepping through with a debugger?

[ljharb]

No, that’s the only method i think should be possible - just like closed-over variables.

[joyeecheung]

BTW, Chrome (canary) does show the private fields in console.log(). (But then in the browser you can't really use the output of console.log programmatically)

[ljharb]

i also don’t think it should do that in chrome, and I’ll file a bug about it when i get the chance.

[joyeecheung]

I believe on the Chrome's side it is because the private field support in the DevTools are added through internalProperties returned by Runtime.getProperties which the console uses for displaying e.g. Promise statuses - to separate them from those kinds of "internal properties", private fields would need to be returned in e.g. another list . The original request for this was https://bugs.chromium.org/p/v8/issues/detail?id=8337

Note that showHidden: true basically returns the same thing (previews) as what's returned by Runtime.getProperties for e.g. Proxy targets, WeakMap entries. The difference is that Node.js currently uses raw embedder APIs for those, whereas Chrome uses the inspector protocol. If Node switches to the inspector protocol to implement that as well then this would be entirely an upstream issue.

[Jamesernator]

One problem with deciding whether or not to show private fields is that they can kinda serve two purposes. One is as a privacy mechanism, but the other is as a brand mechanism similar to internal slots. When being used as a brand mechanism they are arguably part of your API and would be worth showing to consumers of your object.

I'm not sure how would be the best way to surface these but I think opt-in would probably make sense. Maybe something like this with decorators:

class Stream {
  // Is provided by the user and probably needs to be inspected
  // by the user
  @util.expose('value')
  #queue;

  constructor(queue) {
    this.#queue = queue
  }
}

[ljharb]

The way that brand is exposed on builtins is with hardcoded type-specific checks; I think that if a type wants to expose that in a generic way, it should use the util.inspect.custom symbol and define a method.

If it doesn't want to expose it, it should be impossible to do so.

[legendecas]

As a note, since there is a inspector builtin module in Node.js, it is possible to use the inspector to get hands on private fields programmatically:

const inspector = require('inspector');
const session = new inspector.Session();
session.connect();
session.post('Runtime.evaluate', { expression: `class Foo { #bar = 'bar' }; new Foo` },
  (error, { result }) => {
    session.post('Runtime.getProperties', { objectId: result.objectId },
      (error, { privateProperties }) => {
        console.log('private properties', privateProperties);
      });
  });

[ljharb]

Can that also expose function variables? If not, then it shouldn’t be able to expose private fields.

[legendecas]

Can that also expose function variables?

I think it's possible since we could inspect scoped variables while paused on breakpoints on chrome devtools, but I haven't dig into it.

[joyeecheung]

@ljharb The variables can be inspected by looking up the scope chain (unless they are not referenced in the code and therefore not allocated - in that case they can be considered "optimized out"), and the scope chain of the call site can be inspected by setting Debugger.setSkipAllPauses, pausing the debugger through Debugger.pause, and then intercepting the event Debugger.paused which happens to be synchronous. However do note that the synchronicity of inspector sessions is not guaranteed so it would be tricky to use it in the synchronous util.inspect (see discussions in https://github.com/nodejs/node/pull/27110). Also, I don't think it's currently possible to inspect the scope of a function not currently on the call stack through the inspector protocol.

[ljharb]

It seems like a massively underappreciated security hole if node allows you to inspect a scope chain at runtime, and I’d consider private fields to be in the same category.

[joyeecheung]

@ljharb The bets are also off if one takes a heap snapshot at runtime, in that case they have access to every piece of data in JS (though that would likely cause a visible slowdown of the application) when they JSON.parse that snapshot.

For better or for worse, Node's security model already differs from the Web in that it assumes every code run in the application should be trusted, for example that's why Node does not enable v8_untrusted_code_mitigations.

[domenic]

FWIW the web's security model is the same. All information in the same process is readable. (And generally should be considered writable due to the prevalence of exploits inside the process sandbox.) I assume @ljharb is referring to encapsulation, not security.

[ljharb]

I guess i consider them intertwined, but sure, encapsulation.

[Jamesernator]

Given util.inspect can already inspect WeakMap for their keys using { showHidden: true }, I feel like the best option would be to be consistent with that.

As @joyeecheung has shown as long as Node code has access to inspector it can do anything it wants to break encapsulation anyway. And as mentioned also by @joyeecheung this could be solved with an extension to the new policy feature so that code isn't able to access these special features at all.

[ljharb]

That precedent (while equally worrying to me) does suggest that private fields should be hidden by default but accessible via an option.

[legendecas]

That precedent (while equally worrying to me) does suggest that private fields should be hidden by default but accessible via an option.

Then would it make sense to pick https://chromium-review.googlesource.com/c/v8/v8/+/1546559 up?

[joyeecheung]

@legendecas The conclusion in https://chromium-review.googlesource.com/c/v8/v8/+/1546559 was that Node should use the inspector protocol for inspecting private fields instead of using an embedder API.