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

wandb token exposed to wandb users #1123

Open
bghira opened this issue Feb 16, 2024 · 13 comments
Open

wandb token exposed to wandb users #1123

bghira opened this issue Feb 16, 2024 · 13 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@bghira
Copy link

bghira commented Feb 16, 2024

hello, there exists a parameter to pass a wandb token into the training utility. unfortunately, this shows on the "Overview" tab of the training session, as a part of the complete commandline that is used to execute the trainer.

This exposes users to potentially nefarious activity, or at the very least, a sense of unease and the possibility of snooping/leaking internal data.

@bghira
Copy link
Author

bghira commented Mar 31, 2024

@bmaltais and @kohya-ss should probably do something about this :D does this project not care about security?

@bmaltais
Copy link
Contributor

This is not good. I guess the Wanda token need to be exempted from making it in the metadata. This is something @kohya-ss need to filter in the training scripts. There is not much I can do about the metadata…

@kohya-ss
Copy link
Owner

In my understanding, the metadata in the training model doesn't contain wandb token, but it would appear on the GUI screen.
I'm not sure because I don't personally use either wandb or GUI.

@bghira
Copy link
Author

bghira commented Mar 31, 2024

need to recommend users to go with wandb login command instead of a cmdline argument. just --report_to=wandb,tensorboard would work.

@kohya-ss kohya-ss added the documentation Improvements or additions to documentation label Apr 3, 2024
@kohya-ss
Copy link
Owner

kohya-ss commented Apr 3, 2024

That makes sense. I will update the doc in next release.

@bghira
Copy link
Author

bghira commented Apr 3, 2024

fwiw in Diffusers, there is simply a check whether wandb is enabled and the commandline args contain the Huggingface Hub token, because any secrets on cmdline will be exposed via wandb.

but in this case, its the wandb token itself on the cmdline. i am not sure that a documentation hint is enough to stop a user from doing this. the fact that it's on the cmdline at all means it will be exposed 100% of the time this is used.

@kohya-ss kohya-ss added the enhancement New feature or request label Apr 4, 2024
@kohya-ss
Copy link
Owner

kohya-ss commented Apr 4, 2024

You are correct. We must filter sensitive fields from command line args.

In my understanding, the repo doesn't expose either wandb token or command line args currently (the latter is discussed in #1231). If you find any code to do it, please let me know.

@bghira
Copy link
Author

bghira commented Apr 4, 2024

it's a part of the wandb module. it sends ALL args, and there is no way to filter them:

image

@kohya-ss
Copy link
Owner

kohya-ss commented Apr 4, 2024

Thank you for clarification. That's not good. I think it is undesirable to disclose not only wandb token but other information as well. Is there any way to control this?

@bghira
Copy link
Author

bghira commented Apr 4, 2024

none that i have found. i resorted in simpletuner to using json config files to hide cmdline args

@kohya-ss
Copy link
Owner

kohya-ss commented Apr 4, 2024

Thank you. That's nice. sd-scripts has .toml for configs, so I will show WARNING if the sensitive training args given in the command line.

@kohya-ss
Copy link
Owner

kohya-ss commented Apr 4, 2024

I have opened a PR for this #1240, If you have time to review it, I would greatly appreciate it.

@rafstahelin
Copy link

Would be great bowever to send parameters to Wanda. At the moment in kohya_ss, we don't see any training parameters in Wandb, only the project name (tracker name), and the run name set from kohya gui. Is it a problem to send all parameters through config?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants