debug icon indicating copy to clipboard operation
debug copied to clipboard

Problem when using a library that uses `debug` in Deno

Open krlwlfrt opened this issue 1 year ago • 9 comments

I use jsdom in a project in Deno. jsdom uses debug as a dependency:

└─┬ [email protected]
  ├─┬ [email protected]
  │ ├─┬ [email protected]
  │ │ └── [email protected] deduped
  │ └── [email protected]
  └─┬ [email protected]
    └── [email protected] deduped

Deno asks for permission for access to environment variables. Unfortunately debug accesses process.env directly and on requiring the module itself which leads to the following request by Deno.

┏ ⚠️  Deno requests env access.
┃  ├─ Object.toObject (ext:runtime/30_os.js:134:12)
┃  ├─ Object.ownKeys (ext:deno_node/_process/process.ts:54:40)
┃  ├─ Function.keys (<anonymous>)
┃  ├─ Object.<anonymous> (file:///home/krlwlfrt/work/krlwlfrt/calendar/node_modules/debug/src/node.js:124:30)
┃  ├─ Object.<anonymous> (file:///home/krlwlfrt/work/krlwlfrt/calendar/node_modules/debug/src/node.js:267:4)
┃  ├─ Module._compile (node:module:745:34)
┃  ├─ loadMaybeCjs (node:module:770:10)
┃  ├─ Object.Module._extensions..js (node:module:755:12)
┃  ├─ Module.load (node:module:662:32)
┃  └─ Function.Module._load (node:module:534:12)
┠─ Learn more at: https://docs.deno.com/go/--allow-env
┠─ Run again with --allow-env to bypass this prompt.
┗ Allow? [y/n/A] (y = yes, allow; n = no, deny; A = allow all env permissions) > 

This goes against the principle of least privilege. Denying the request leads to an exception, because the code can't handle a rejection of that request with Deno's API.

Long story short: Could you please change the behaviour of your library, so that the process.env is not accessed on module load?

krlwlfrt avatar Nov 28 '24 09:11 krlwlfrt

Er, No sorry. The whole point is that debug uses the environment to configure itself. Very common thing for logging libraries to do.

Qix- avatar Dec 01 '24 15:12 Qix-

Yes, sure. I get that. Totally valid point. I'm just asking if you could change it, so that the acccess to process.env does not happen on module load and rather on a function/method call.

Or that you access the variables that you need in process.env directly - like process.env.NODE_ENV or similar. And then parse the whole process.env when it is needed for debugging purposes.

Deno starts scripts without any permissions and then grants permissions as needed which is a huge security benefit. This is completely negated when I have to grant permission to access all environment variables.

krlwlfrt avatar Dec 01 '24 15:12 krlwlfrt

I'm not quite sure what the difference is; environment variables are still being accessed. How does delaying their access provide any security benefit?

Qix- avatar Dec 01 '24 19:12 Qix-

I'm not completely certain on how your module works, but I can see from glancing over the code, that inspectOpts, which contains the variables of process.env, is only accessed when certain functions are called. I assume that these some/most of these functions are only called when the developer of a module that wants to debug it enables a the debug mode (via environment variable). When I changed the code to inspectOpts = {}, JSDOM worked completely fine. So maybe it would only be necessary to access the full process.env when it is needed... Or you could build inspectOpts only when one of the referencing functions is called instead of on module load.

Deno is designed in a way, that the end user, who is running the script can grant access per environment variable, which is useful if you have data in your ENV that you don't want exposed to potentially thousands of node modules that might be installed in a project. If a module access process.env instaed of process.env.VARIABLE the end user has to grant access to all variables at once. Node.js is the complete opposite, where I/O, access to network, environment variables, etc. is unrestricted and any node module could do harmful things, without the end user even noticing.

I'm not sure if I'm able to communicate clearly, what I mean...

krlwlfrt avatar Dec 02 '24 12:12 krlwlfrt

I mean I'm not really sure that anything I do is going to fix Deno's case. There are still millions of packages that will never upgrade to the latest version. This will always be a problem. It's also a questionable way to achieve security, if you ask me.

Qix- avatar Dec 06 '24 13:12 Qix-

There are still millions of packages that will never upgrade to the latest version.

Valid point.

It's also a questionable way to achieve security, if you ask me. Yes, sure. It would be better to always run code you don't own in an unprivileged container. But it's not convenient and most people don't do it. And Deno's approach of asking for permissions is one layer of security.

I get that it's not that important to you. Would you consider it, if I'd open a PR with the changes I'm proposing?

krlwlfrt avatar Dec 08 '24 10:12 krlwlfrt

It really depends on the changes but sure, I can take a look.

Qix- avatar Dec 08 '24 14:12 Qix-

This would probably also solve another problem I'm encountering:

I'm running a Node.js application bundled by Vite and want to configure debug logging for the grammy library (which uses debug internally) based on runtime configuration. Currently, I set process.env.DEBUG right after startup when my config is parsed, but this doesn't work after bundling - likely because the env var is read during import, and the bundler hoists all imports to the top.

The proposed delayed loading would be a great solution for this use case, since it would allow setting the debug configuration before the actual environment variable reading happens.

eikowagenknecht avatar Dec 18 '24 11:12 eikowagenknecht

This package requesting access to env in Deno, how to disable it

Jobians avatar Jun 03 '25 09:06 Jobians