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

Remove initialCommand from args of terminal creation. #5916

Merged
merged 1 commit into from Jan 31, 2019

Conversation

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jan 28, 2019

I don't think we need to remove it from the terminal API, just from the command args.

@ian-r-rose ian-r-rose added this to the 1.0 milestone Jan 28, 2019
@ian-r-rose ian-r-rose requested a review from afshin Jan 28, 2019
@vidartf
Copy link
Member

@vidartf vidartf commented Jan 28, 2019

Just for reference: Why is it being removed?

@afshin
Copy link
Member

@afshin afshin commented Jan 31, 2019

@vidartf The rationale for removing this is that Phosphor command args were specifically designed to be serializable and loaded from many sources, so this opens a new vector to causing harm to the server by serializing a malicious shell command. While Jupyter is basically remote-execution-as-a-service, this still seems to offer a novel vector that we are seeking to avoid.

This is a preventative measure because we expect JupyterLab to be run in trusted environments, but it seems like a prudent one. What do you think?

afshin
afshin approved these changes Jan 31, 2019
Copy link
Member

@afshin afshin left a comment

👍

@jasongrout jasongrout merged commit 08449a8 into jupyterlab:master Jan 31, 2019
2 of 3 checks passed
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants