search

talaia-labs / rust-teos

The Eye of Satoshi - Lightning Watchtower
https://talaia.watch
MIT License
128 stars 62 forks source link

Disallow dangerous parameters in the config file #217

Open mariocynicys opened 1 year ago

mariocynicys commented 1 year ago

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.

sr-gi commented 1 year ago

Concept ACK

anipaul2 commented 1 year ago

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?

mariocynicys commented 1 year ago

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.

anipaul2 commented 1 year ago

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.

mariocynicys commented 11 months ago

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.

sr-gi commented 11 months ago

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