Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--experimental-permission breaks REPL #48884

Closed
tniessen opened this issue Jul 22, 2023 · 5 comments · Fixed by #48920
Closed

--experimental-permission breaks REPL #48884

tniessen opened this issue Jul 22, 2023 · 5 comments · Fixed by #48920
Labels
confirmed-bug Issues with confirmed bugs. permission Issues and PRs related to the Permission Model repl Issues and PRs related to the REPL subsystem.

Comments

@tniessen
Copy link
Member

Version

20.5.0

Platform

any

Subsystem

permission model

What steps will reproduce the bug?

Open a REPL and start typing:

$ node --experimental-permission --allow-fs-read=/
> f

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior? Why is that the expected behavior?

No error.

What do you see instead?

Fatal error:

node:internal/readline/emitKeypressEvents:74
            throw err;
            ^

Error: Access to this API has been restricted
    at Session.connect (node:inspector:68:7)
    at sendInspectorCommand (node:internal/util/inspector:52:11)
    at getGlobalLexicalScopeNames (node:repl:1262:10)
    at REPLServer.complete (node:repl:1493:26)
    at REPLServer.completer (node:repl:783:5)
    at showCompletionPreview (node:internal/repl/utils:230:10)
    at showPreview (node:internal/repl/utils:384:7)
    at REPLServer.self._ttyWrite (node:repl:1020:9)
    at ReadStream.onkeypress (node:internal/readline/interface:264:20)
    at ReadStream.emit (node:events:514:28) {
  code: 'ERR_ACCESS_DENIED',
  permission: 'Inspector',
  resource: 'Connect'
}

Node.js v20.5.0

Additional information

Likely side effect of 34d92ed.

@tniessen tniessen added the repl Issues and PRs related to the REPL subsystem. label Jul 22, 2023
@RafaelGSS
Copy link
Member

Hi, thanks for reporting it. I'm aware of this behaviour.

@RafaelGSS RafaelGSS added the confirmed-bug Issues with confirmed bugs. label Jul 24, 2023
@targos
Copy link
Member

targos commented Jul 24, 2023

Question is: should we make the internal calls to the inspector possible/privileged, or do like when the inspector is unavailable (this would just be a new condition in

const { hasInspector } = internalBinding('config');
if (!hasInspector) return onError();
)

@RafaelGSS
Copy link
Member

I believe REPL shouldn't be available when the permission model is enabled. Please let me know if I'm wrong, but the REPL can create a new V8 isolate and bypass any permission in several ways. If we start supporting REPL, it would mean that we need to cover all those cases.

@targos
Copy link
Member

targos commented Jul 25, 2023

the REPL can create a new V8 isolate and bypass any permission in several ways

Can you elaborate?

@RafaelGSS
Copy link
Member

the REPL can create a new V8 isolate and bypass any permission in several ways

Can you elaborate?

Basically, the same as 34d92ed fixes. I don't have a reproducible example because I'm not sure if that's possible. I'm just wondering if REPL can create a new V8 isolate, it means it could create without the experimental permission rules, right?

Anyway, I created the #48920.

nodejs-github-bot pushed a commit that referenced this issue Jul 27, 2023
PR-URL: #48920
Fixes: #48884
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Jul 27, 2023
PR-URL: nodejs#48920
Fixes: nodejs#48884
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
rluvaton pushed a commit to rluvaton/node that referenced this issue Jul 28, 2023
PR-URL: nodejs#48920
Fixes: nodejs#48884
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
pluris pushed a commit to pluris/node that referenced this issue Aug 6, 2023
PR-URL: nodejs#48920
Fixes: nodejs#48884
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
pluris pushed a commit to pluris/node that referenced this issue Aug 7, 2023
PR-URL: nodejs#48920
Fixes: nodejs#48884
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@tniessen tniessen added the permission Issues and PRs related to the Permission Model label Aug 10, 2023
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
PR-URL: nodejs#48920
Fixes: nodejs#48884
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
PR-URL: nodejs#48920
Fixes: nodejs#48884
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
UlisesGascon pushed a commit to UlisesGascon/node that referenced this issue Aug 14, 2023
PR-URL: nodejs#48920
Fixes: nodejs#48884
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
RafaelGSS added a commit that referenced this issue Aug 15, 2023
PR-URL: #48920
Fixes: #48884
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
RafaelGSS added a commit that referenced this issue Aug 17, 2023
PR-URL: #48920
Fixes: #48884
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. permission Issues and PRs related to the Permission Model repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants