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

Add option for disabling file downloads #2426

Closed
code-asher opened this issue Dec 8, 2020 · 18 comments · Fixed by #5055
Closed

Add option for disabling file downloads #2426

code-asher opened this issue Dec 8, 2020 · 18 comments · Fixed by #5055
Assignees
Labels
feature New user visible feature high-priority This issue needs to be resolved ASAP security Security related
Milestone

Comments

@code-asher
Copy link
Member

Being able to easily download might be an issue in some situations. See some comments on #1386.

Maybe it also makes sense to prevent the HTTP server from serving these files as well so you can't just curl them.

@code-asher code-asher added the feature New user visible feature label Dec 8, 2020
@nhooyr nhooyr added the security Security related label Dec 8, 2020
@JammSpread
Copy link
Contributor

JammSpread commented Dec 9, 2020

If I'm correct we just have to patch this with if statements on lines 228 225 and 488 485?

@code-asher
Copy link
Member Author

code-asher commented Dec 10, 2020 via email

@bpmct
Copy link
Member

bpmct commented Dec 21, 2020

I confirmed two sections mentioned above will disable the download option :)

I was also able to modify the static endpoint, I believe we'll want to use something similar to this check to ensure the user can't curl or visit the page for these static files. For curl in particular, we can do

if (req.headers["user-agent"] && req.headers["user-agent"].includes("curl")) {
    throw new HttpError("Unauthorized", HttpCode.Unauthorized)
}

but I assume there is probably a better way to whitelist requests from code-server? Any ideas what headers we should be looking for?

Also, I've added the flag --disable-download in the CLI but am unsure how to check for that in fileActions.contribution.ts.

@lsc196
Copy link

lsc196 commented Dec 22, 2020

How to ban downloading files now?

@JammSpread
Copy link
Contributor

JammSpread commented Dec 26, 2020

Also, I've added the flag --disable-download in the CLI but am unsure how to check for that in fileActions.contribution.ts.

I think an easy way for you to check for --disable-download might be by passing the flag as an environment variable.

@bpmct
Copy link
Member

bpmct commented Jan 4, 2021

Yeah, I'd love to do it as both (code-server handles most options like this), but I haven't been able to find an example of it implemented somewhere like fileActions

@code-asher
Copy link
Member Author

code-asher commented Jan 4, 2021 via email

@jsjoeio jsjoeio added this to the Backlog milestone Apr 29, 2021
@metalshanked
Copy link

metalshanked commented Jun 4, 2021

@jsjoeio Will there be an option to disable file uploads as well ? Example:- To disable people uploading their own .vsix file and installing them manually.

@jsjoeio
Copy link
Contributor

jsjoeio commented Jun 7, 2021

I think we could definitely consider it!

If it's a global thing, like download scenario, then it might be possible to implement here as well.

To disable people uploading their own .vsix file and installing them manually.

For that specific use case, we could potentially add that.

It might help if you could expand on your use case (then we can see if it fits in this issue or should be a separate issue).

@metalshanked
Copy link

metalshanked commented Jun 8, 2021

I think we could definitely consider it!

If it's a global thing, like download scenario, then it might be possible to implement here as well.

To disable people uploading their own .vsix file and installing them manually.

For that specific use case, we could potentially add that.

It might help if you could expand on your use case (then we can see if it fits in this issue or should be a separate issue).

Thanks @jsjoeio. Yes, a global disable upload option would be awesome. The use case is similar to the original requester's disable download request. For Eg:- use cases where we want to disable certain users from uploading files to the code-server (running on Docker) so that they can only operate on the files and items preloaded into the container and nothing external can be uploaded (or downloaded)

@jsjoeio
Copy link
Contributor

jsjoeio commented Jun 8, 2021

Ahh okay. Thank you for elaborating! Sounds very similar in which case we can keep here in this issue :)

@songhanpoo
Copy link

Hi everyone, currently can i use these options ?
Do i need compile from src ?

@code-asher
Copy link
Member Author

code-asher commented Oct 12, 2021 via email

@belief-cyf
Copy link

Looking forward to this feature

@songhanpoo
Copy link

Looking forward to this feature which is can be use for external developer. intergrate LDAP login and code on web. no leak code.

hopeful anyone improve about that

@jsjoeio jsjoeio added the high-priority This issue needs to be resolved ASAP label Feb 23, 2022
@jsjoeio jsjoeio self-assigned this Mar 2, 2022
@greyscaled
Copy link
Contributor

[sc-16496]

@jsjoeio
Copy link
Contributor

jsjoeio commented Mar 16, 2022

@jsjoeio jsjoeio modified the milestones: March 2022, April 2022 Mar 22, 2022
@jsjoeio
Copy link
Contributor

jsjoeio commented Mar 30, 2022

Seems like all we need to do is:

  1. create new context key like: ContextKeyExpr.and(IsWebContext, HasDownloadFileEnabled)
  2. write expression that returns true by default, or false if CLI flag --disable-file-downloads or something

Might be worth looking into writing e2e tests.

Without CLI flag, should show Download File in menu.
WIth CLI flag, should not show Download File in menu.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New user visible feature high-priority This issue needs to be resolved ASAP security Security related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants