-
Notifications
You must be signed in to change notification settings - Fork 40
Microsoft API Fix and general improvements #84
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
base: main
Are you sure you want to change the base?
Conversation
As the file is not used anywhere else, removed the output to it which simplifies (and presumably slightly speeds up) the method.
Instead of iterating over all malicious extensions (which can be a rather large list), iterate over all extensions that are to be downloaded.
This increases the runtime as the web request must be executed first. But the version will always be the latest. As it is only used for the user agent it shouldn't event matter what version it is.
Removes existing old extension versions that are no longer needed.
Existing extensions can be included in the update process.
Changes default execution to a "one-off task"
This does not include an updated certificate! The certificate still is valid for the old domain.
Most arguments / parameters now have a shorthand form. Eg. --sync can also be called with -s. This should simplify / shorten the command line in automations.
Only extensions that are beaing searched for, that are specified (on the allowlist) or that already exist, would be downloaded.
Fixes issue with Python 3 interpreting \d as Unicode character
Instead use timezone aware method (still with UTC time though) 'now'
|
Thanks for this comprehensive PR. I'll review it in more detail, but initially.. The malicious.json file is used by the vscode client directly and not by other code. And yes, the sync frequency will likely stay as this has historically been a set and forget process rather than a script which is run once off. With all CDS environments usually being slow to incorporate changes I'm hesitant to change the default behaviour now. I'd argue the default deployment case is in a container, usually Deployment in k8s. Cron like jobs are somewhat newer and have their own quirks. But absolutely adding a single run arg (can't remember if there is one) sounds good. |
As per the feedback from the PR readd the default frequency back as the standard should not change.
As per the feedback from the PR readd the output of malicious extensions to the "malicious.json". The file is used by VS Code itself so there are no ther references to the file in the source code.
Yes, I might have gone a bit overboard with this. I needed most of this for myself so I thought to add my work back to main repository.
Thank you for clearing this up! I didn't realize VS Code accessed this directly. I added this back with 1a3ad57.
I thought so. I added the default back in ba99907. I'll make another PR at some point with a new argument that would allow single execution. Or to be able to set the frequency to 0 in order to disable it. I'll also use this opportunity to thank you for this project. There doesn't seem to be any other solution (paid or not) that achieves an air gapped deployment of Visual Studio Code extensions. |
To simplify the process, I pulled two commits from this PR and put them in #85 |
malicious.json
as the file isn't used anywhere. I suspect this will speed up execution, though I haven't verified thiswin32
towin32-x64
. Thanks @AndreasAhlbeck (Failing to download extensions #81 (comment))