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

Extension can delete all user settings.json #58

Open
TonyGravagno opened this issue Mar 20, 2024 · 5 comments
Open

Extension can delete all user settings.json #58

TonyGravagno opened this issue Mar 20, 2024 · 5 comments

Comments

@TonyGravagno
Copy link

On experimenting with this extension I think I created a condition that deletes the user settings.json file. I recovered it, others may not be so fortunate.

I think this happens when we set:

"syncSettings.resources": ["extensions"]

And not

"syncSettings.resources": ["extensions","user"]

Since user settings are not configured to sync outbound, when a sync is done and there are no user settings to pull in, the user settings are simply deleted - or replaced with the null settings that have been retrieved.

To reproduce this:

  1. Backup your settings.json and everything else you care about.
  2. Generate a new profile based on the current profile.
  3. Switch to that profile. Since the new profile has no user settings, the current profile is set to "no settings".
  4. OR ... perhaps it was on switching back to the original profile that does not specify that "user" is in "resources".

Preferred behaviour : If a resource isn't specified, don't process it at all.

@maxwowpow
Copy link

Yes it's a dreadful experience, thanks for opening the issue.
Just doing a destructive operation without any backup/confirmation can do a lot of damage.

@daiyam
Copy link
Member

daiyam commented May 18, 2024

@TonyGravagno I can't reproduce the bug...

Preferred behaviour : If a resource isn't specified, don't process it at all.

It's what it already does:

if(resources.includes(Resource.Settings)) {

Anyway, an extended profile never synchronizes the settings.json(https://github.com/zokugun/vscode-sync-settings?tab=readme-ov-file#profile-inheritance)

if(!profileSettings.extends) {

@daiyam
Copy link
Member

daiyam commented May 18, 2024

I was able to discover a bug with the same result as yours.
But, in the extended profile, the "settings"resource need to be added the "syncSettings.resources" property (while not present the base profile).

I think an extended profile shouldn't be able to change the resources property.
What do you think?

Are you still able the reproduce your bug?

@TonyGravagno
Copy link
Author

Sincere thanks for looking into this. I've been intensely working on other projects and just came back here. I will look into this soon.

@TonyGravagno
Copy link
Author

On creating a new syncSettings.resources value in the User settings.json, the attribute "user" isn't specified - "user" isn't a valid value for Resources:

export enum Resource {

I don't know why I had "user" there and I'm sorry that this might be an invalid report.
I can't reasonably ask for a fix to an issue that was caused by invalid data. 😢
However, when editing the settings file there is no intellisense that tells us a value is invalid:

  "syncSettings.resources": [
    "extensions",
    "keybindings",
    "settings",
    "snippets",
    "uiState",
    "user" // not valid but no indication from UI
  ]

Your code looks fine. If there is an invalid option, it looks like it is ignored.

public override async restoreProfile(): Promise<boolean> { // {{{

So now I think there might be a different trigger for that error. I have no idea what to check at the moment. I'll come back to this ticket if I see another pattern.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants