rust-teos icon indicating copy to clipboard operation
rust-teos copied to clipboard

Disallow dangerous parameters in the config file

Open mariocynicys opened this issue 1 year ago • 6 comments

Right now, the parameters overwrite_key (overwrites* the tower's key and gives it a new identity) and force_update (introduced in #216) are allowed to be set in the configuration file.

Since they are more of a fail-safe and rather dangerous commands, we should assert that the user doesn't set them in the config file and only settable as command line flags.

mariocynicys avatar May 07 '23 05:05 mariocynicys

Concept ACK

sr-gi avatar May 07 '23 15:05 sr-gi

Right now, the parameters overwrite_key (overwrites* the tower's key and gives it a new identity) and force_update (introduced in #216) are allowed to be set in the configuration file.

Since they are more of a fail-safe and rather dangerous commands, we should assert that the user doesn't set them in the config file and only settable as command line flags.

So, removing the parameters from the config file and allowing the user to use it only in cli?

anipaul2 avatar May 11 '23 17:05 anipaul2

So, removing the command from the config file and allowing the user to use it only in cli?

Yeah something like that. You might only disallow it by sanity checking that it's not set. Not that we have to remove it from the Config struct.

mariocynicys avatar May 11 '23 18:05 mariocynicys

So, removing the command from the config file and allowing the user to use it only in cli?

Yeah something like that. You might only disallow it by sanity checking that it's not set. Not that we have to remove it from the Config struct.

Got it. I will raise a pr when it's done.

anipaul2 avatar May 11 '23 18:05 anipaul2

Just noticed there is actually nothing dangerous here because Config::patch_with_options completely ignores the config file value of overwrite_key & force_update and uses the command line options instead (which default to false).

I think we can just note that in a comment in the template config file so that users don't expect something when setting these ignored parameters to true.

Also, write a comment about it in https://github.com/talaia-labs/rust-teos/blob/1a89c5da70278ac5019a3bbb505b1923648a43da/teos/src/config.rs#L206-L207 since we all missed it xD.

mariocynicys avatar Jul 16 '23 09:07 mariocynicys

Oh damn 😮!

I still think it may be worth reporting this to the user though instead of just commenting this on the config file. In general, if something is not supported, I think the user should be made aware of it explicitly.

Even though I never finished it, a rework of the config to acknowledge where things come from may be good in the long run.

https://github.com/talaia-labs/rust-teos/compare/master...sr-gi:rust-teos:config-rework

sr-gi avatar Jul 17 '23 15:07 sr-gi