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

Feature request: Enforce per-app configuration and make global config explicitly global #123

Closed
exponentactivity opened this issue Dec 21, 2020 · 6 comments

Comments

@exponentactivity
Copy link
Contributor

exponentactivity commented Dec 21, 2020

EDIT: My original feature request was already done, see the next comments for discussion of the updated title.

I made a script that calls an API on my local network, and it needs a url and an api key to do that.

The problem is i would like to backup this script to git (with the rest of my config) and be able to share it with others in a usable format, but my url and api is hard coded in clear text.

I looked at Accessing YAML configuration and it seems like an okay solution, but it doesn't seem like it's supposed to contain any sensitive data, and putting it in .gitignore would mean the config is not backed up.

If i could read the api key from secrets.yaml all sensitive data would be kept in a single file.
I think it would be preferable to whitelist the secrets each pyscript script/app has access to in pyscript.config so a malicious app can't access everything.

Whitelisting could also double as place to map the (arbitrarily named) secrets to the variable inside the script.

pyscript.config could look something like this:

pyscript:
  allowed_secrets:
    - my_app:
      - my_whitelisted_secret: my_variable_the_secret_is_mapped_to
  apps:
    ...
@dlashua
Copy link
Contributor

dlashua commented Dec 21, 2020

This is already doable in almost exactly the way you specified:

secrets.yaml

some_secret: welcome

configuration.yaml

pyscript:
  secrets:
    some_secret: !secret some_secret

in a pyscript

@time_trigger('startup')
def startup():
  log.info(pyscript.config['secrets']['some_secret']

Keep in mind, in Home Assistant, secrets.yaml doesn't auto reload. So, if you add a secret, you'll need to restart Home Assistant before it's available.

This does not handle only allowing certain apps to read certain secrets. So I think that's the real core of what this feature request is asking for since everything else is already possible.

Make it so an APP can only read the config for THAT APP.

pyscript:
  apps:
    my_app:
      thing_a: this
      thing_b: that
      some_secret: !secret some_secret
    other_app:
      thing_c: another
      thing_d: and another
      another_secret: !secret my_gmail_password
  other_config: abc
  this_is_available_to_all_apps_and_scripts: thisthing
      

pyscript.config['app'] should be set to the config for that specific app (and only that app) when used inside of an app. and the rest of the config (not under apps) would be available to everything. Does that sound like what you need?

@exponentactivity
Copy link
Contributor Author

That totally solved my problem! Thanks!

I see the documentation for this feature now, i just didn't use the correct search terms. I imagine my use case isn't that out of the ordinary. If you agree i will look into expanding the documentation and send a pull-request? Maybe just some some examples that would make it pop up on search terms related to passwords and sensitive info.

My itch is scratched, but i do think enforcing the recommended use of per-app configuration, and having separate and explicit global configuration would be a great idea, especially if sharing apps becomes widespread, to avoid possible security issues.

Maybe something like:

pyscript:
  apps:
    my_app:
      thing_a: this
      thing_b: that
      some_secret: !secret some_secret
    other_app:
      thing_c: another
      thing_d: and another
      another_secret: !secret my_gmail_password
  global:
    other_config: abc
    this_is_available_to_all_apps_and_scripts: thisthing
    why_not_sublists:
      for_sake_of: readability
      and_to_make_organization: easier

Putting it in global would make it explicit that this configuration is also available to the "not_stealing_your_passwd" app and should only be used for things where that is okay.

@exponentactivity exponentactivity changed the title Feature request: Get variables from secrets.yaml in scripts Feature request: Enforce per-app configuration and make global config explicitly global Dec 21, 2020
@dlashua
Copy link
Contributor

dlashua commented Dec 21, 2020

Looks good. Now it's just up to @craigbarratt if he wants to make it work this way. It would be a large breaking change but, potentially worth it if this level of separation between apps is needed for safety. Handling the multitude of different use cases one might have for YAML configuration is a difficult task.

@craigbarratt
Copy link
Member

I do like the idea of partitioning each app's config, but as @dlashua mentions it's a significant breaking change. I've been trying to think of a graceful way to do this. Perhaps the app-specific config could be a different variable name (eg, pyscript.app_config) and we deprecate and eventually remove the apps entry in pyscript.config? That way pyscript.config would continue to include all the other non-app config (eg, the global settings in the example above).

@dlashua
Copy link
Contributor

dlashua commented Dec 31, 2020

I like the idea of pyscript.app_config. This has the added bonus of making it easier for an app to get it's own config since it no longer has to use its own name (pyscript.config['apps'][__name__.split('.')[-1]]) to get its config. Do this now, and then, a few releases from now, remove "app" from pyscript.config entirely. That seems pretty clean and easy to upgrade to.

@craigbarratt
Copy link
Member

This should be resolved with the recent commit.

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