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 #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 #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.

[jasnell]

Actually, we can't just remove the inspector module. It's a supported feature that would need to go through a full semver-major deprecation cycle which would take quite some time. There are systems actively using it in production for other purposes.

[ljharb]

I definitely don't think the inspector module should be removed. I think that modifying/wrapping Runtime.getProperties so that it does not produce the privateProperties property seems like it would be sufficient? (albeit, probably semver-major)

[ljharb]

Since this terrible thing is possible, I went ahead and made a terrible thing: https://www.npmjs.com/package/private-fields

I would gladly invite node to break it by removing privateProperties from the result of Runtime.getProperties :-) and if such a PR would be accepted, I'll be happy to give it a shot.

[ljharb]

Is there any willingness to accept such a PR for node 18?

[legendecas]

@ljharb there is previous work on this: #31817. It is abandoned because of some concerns about the inspector API synchronicity use patterns.

However, since the inspector API is being marked as stable now, I'm thinking maybe it is acceptable to implement the feature anyway?

[ljharb]

@legendecas that work seems like the opposite of my intent; private fields shouldn’t be observable whatsoever by the runtime.

[legendecas]

@ljharb sorry, what do you mean by "observable"? I was thinking that PR is similar to your work https://www.npmjs.com/package/private-fields. It doesn't get hands-on the private fields directly, only get a cloned shape of the values. Please correct me if I misunderstood.

[ljharb]

I want to make a PR that breaks that package :-) the entire point of the private fields feature is that it is impossible for the js runtime to observe them. It shouldn’t be possible to know that something has a private field or not.

[targos]

I want to make a PR that breaks that package :-) the entire point of the private fields feature is that it is impossible for the js runtime to observe them. It shouldn’t be possible to know that something has a private field or not.

I understand the point, but in Node.js, the most common way to debug values is to inspect them. If it becomes impossible, developers could not just console.log(obj) to see what's in the private fields.

[ljharb]

They shouldn’t be able to do that from outside the class, is the point. It’s not required for debugging.

[ljharb]

To be clear, i don’t mind console.log doing it; the facility i want to remove is getting structured data about it.

[targos]

i don’t mind console.log doing it; the facility i want to remove is getting structured data about it.

I think it's impossible to have both. If the Chrome developer tools are able to use a websocket connection to retrieve the structured data, so is a Node.js program.

[ljharb]

@targos it seems entirely possible to extract the ability from the inspector module, save it in an internal module that only node core can access, and then delete it so that userland code can't get access to it.

[Jamesernator]

@targos it seems entirely possible to extract the ability from the inspector module, save it in an internal module that only node core can access, and then delete it so that userland code can't get access to it.

I think what is meant is that as long as Node exposes an inspector port whatsoever then one can just connect to it with a generic websocket API e.g.:

const { w3cwebsocket: WebSocket } = require("websocket");

inspector.open();
const ws = new WebSocket(inspector.url());
ws.send(JSON.stringify({
  // Do whatever devtools protocol stuff to get private fields here
});
ws.addEventListener("message", () => {
  // Use the private fields here somehow
});

There's really nothing Node can do, like sure it could remove inspector.url(), but that url has to be stored somewhere for Chrome Devtools/VSCode inspector/etc to be able to access it so the Node program could just read those files/sockets/whateverisused.

Fundamentally denying access to debugger to the Node program is impossible, as long as Node is able to run arbitrary code on the machine it can always find some way of capturing the socket between Node and devtools somewhere.

If you really want to deny such power you can use something such as policy to deny access to the inspector module (and you'll need to deny access to fs, v8, child_process, and probably others).

[ljharb]

@Jamesernator i don’t understand. I’m saying nothing would have that access except the internal console.log, and maybe util.inspect - so there’d be no inspector object that provided private fields info, no matter how you connected to it.

[ljharb]

Or are you saying that node has no ability to alter the inspector object served by the protocol?

[domenic]

If console.log can display the fields, then anything with access to stdout can read them... This doesn't make a lot of sense to me.

[Jamesernator]

@Jamesernator i don’t understand. I’m saying nothing would have that access except the internal console.log, and maybe util.inspect - so there’d be no inspector object that provided private fields info, no matter how you connected to it.

Or are you saying that node has no ability to alter the inspector object served by the protocol?

Even if it does, I strongly don't think it should, it is a debugging protocol, people use Chrome Devtools and VSCode devtools with it already. If people are using it for things other than debugging that is their problem.

And again, you cannot deny access to private fields entirely, regardless of what you try, because as long as Node has general access to child processes, you cannot realistically prevent it from capturing its own memory (both internally with v8.getHeapSnapshot() or externally with OS APIs such as /proc/[pid]/maps on Linux or similar).

[ljharb]

@domenic that’s not structured data, though. I think there’s a distinct difference between directly providing the inspector API, and forcing someone to parse console output.

[domenic]

Feels like some seriously shifting goalposts to me... From "must not be exposed" to "only console.log can expose them" to "can be exposed as long as doing so requires parsing some strings".

[ljharb]

Being pragmatic isn't shifting goalposts. They must not be exposed at all, but there's still multiple "tiers" of exposing them, and the less they are exposed, the better.

[joyeecheung]

I wonder if it would make sense to make the private fields visible in console.log() behind a flag (also disabling privateProperties in the inspector protocol unless the flag is turned on). We could emit a warning for that flag if necessary, too.

EDIT: thinking about the idea in the parentheses again, it probably would be somewhat complicated if the private fields need to remain debuggable through DevTools by default at the same time...

[ghost]

If they are exposed in any way, I'd prefer they're behind a distinct option. It would be a pain if I want a full dump of just the public surface of an object but can't get it, because all my private fields are included.

util.inspect(instance, { showHidden: true })
util.inspect(instance, { showHidden: true, showPrivate: true })