feat/config_for_root_people #19
Conversation
```
// system administrators can define different constraints in how configurations are loaded
// this is a mechanism to require root to change these config options
"system": {
// do not allow users to tamper with settings at all
"disable_user_config": false,
// do not allow remote backend to tamper with settings at all
"disable_remote_config": false,
// protected keys are individual settings that can not be changed at remote/user level
// nested dictionary keys can be defined with "key1:key2" syntax,
// eg. {"a": {"b": True, "c": False}}
// to protect "c" you would enter "a:c" in the section below
"protected_keys": {
// NOTE: selene backend expects "opt_in" to be changeable in their web ui
// that effectively gives them a means to enable spying without your input
// Mycroft AI can be trusted, but you dont need to anymore!
// The other keys are not currently populated by the remote backend
// they are defined for protection against bugs and for future proofing
// (what if facebook buys mycroft tomorrow?)
"remote": [
"enclosure",
"server",
"system",
"websocket",
"gui_websocket",
"network_tests",
"listener:wake_word_upload:disable",
"skills:msm:disabled",
"skills:upload_skill_manifest",
"skills:auto_update",
"skills:priority_skills",
"skills:blacklisted_skills",
"opt_in"
],
"user": ["system"]
}
}
```
JarbasAl
force-pushed
the
fix/config_for_root_people
branch
from
8fa1db4
to
67d1d00
Compare
| def _get_system_constraints(): | ||
| # constraints must come from SYSTEM config | ||
| # if not defined then load the DEFAULT constraints | ||
| # these settings can not be set anywhere else! | ||
| return LocalConf(SYSTEM_CONFIG).get("system") or \ | ||
| LocalConf(DEFAULT_CONFIG).get("system") or \ | ||
| {} |
There was a problem hiding this comment.
link to key (below) for other reviewers' reference
| for cfg in configs: | ||
| # check for protected keys in remote config (changes blocked by system) | ||
| if isinstance(cfg, RemoteConf): | ||
| if skip_remote: # remote config disabled at system level | ||
| continue | ||
| # delete protected keys from remote config | ||
| flattened_delete(cfg, protected_remote) | ||
| # check for protected keys in user config (changes blocked by system) | ||
| elif isinstance(cfg, LocalConf) and cfg.path in xdg_locations + [OLD_USER_CONFIG]: | ||
| if skip_user: # user config disabled at system level | ||
| continue | ||
| # delete protected keys from user config | ||
| flattened_delete(cfg, protected_user) | ||
| merge_dict(base, cfg) |
There was a problem hiding this comment.
this is beautiful
should it be a function? it would pre-implement additional config layers, though i have no suggestions as to what those might be
There was a problem hiding this comment.
i can move it into a function if that feels cleaner, this method is already quite big anyway
There was a problem hiding this comment.
actually, moving this into a func is ugly, it needs get_xdg_config_locations and _get_system_constraints so those would need to be called twice or passed in args, regardless it would no longer be beautiful :P
There was a problem hiding this comment.
It would certainly need some params, but I don't see those
The interesting part would be creating a clean reference to skip_n and protected_n for like values of n
|
It wouldn't hurt to wait for another review, but my questions are answered |
|
All looking good to me except "the dictionary utils used in this PR can be found here OpenVoiceOS/zzz-old-ovos-utils#66" does this mean we need to depend on that archived util or is that ported over to this or to newer ovos utils? |
i was a github accident..... the PR was made in the old repo, but there is still only 1 ovos_utils package, the other repo should be ignored |
* feat/config_for_root_people
```
// system administrators can define different constraints in how configurations are loaded
// this is a mechanism to require root to change these config options
"system": {
// do not allow users to tamper with settings at all
"disable_user_config": false,
// do not allow remote backend to tamper with settings at all
"disable_remote_config": false,
// protected keys are individual settings that can not be changed at remote/user level
// nested dictionary keys can be defined with "key1:key2" syntax,
// eg. {"a": {"b": True, "c": False}}
// to protect "c" you would enter "a:c" in the section below
"protected_keys": {
// NOTE: selene backend expects "opt_in" to be changeable in their web ui
// that effectively gives them a means to enable spying without your input
// Mycroft AI can be trusted, but you dont need to anymore!
// The other keys are not currently populated by the remote backend
// they are defined for protection against bugs and for future proofing
// (what if facebook buys mycroft tomorrow?)
"remote": [
"enclosure",
"server",
"system",
"websocket",
"gui_websocket",
"network_tests",
"listener:wake_word_upload:disable",
"skills:msm:disabled",
"skills:upload_skill_manifest",
"skills:auto_update",
"skills:priority_skills",
"skills:blacklisted_skills",
"opt_in"
],
"user": []
}
}
```
authored-by: jarbasai <jarbasai@mailfence.com>
this PR changes some of the mycroft.conf default values to reflect #1
It also ports MycroftAI#2872 without changing the order of configs
SYSTEM still has precedence over REMOTE, this was accidentally changed in mycroft-core recently MycroftAI#3029 but i consider that a bug, if MycroftAI#2861 gets merged we can revisit this, for now i did not port that change
the dictionary utils used in this PR can be found here OpenVoiceOS/zzz-old-ovos-utils#66