Declaratively defining environment variables

Ok, at first I thought this is a bug and has an easy explanation - we do not do osutil.SubstituteEnv(..) for hooks in snap-exec. But I think it is actually by design and not an oversight (one could also say it’s a limitation): hooks are executed by snapd with euid=root (but are confined of course), so doing env var expansion for hooks would be a security issue.

@bloodearnest so, we don’t interpolate the variables from “environment:” section for hooks, but you can of course use $SNAP_DATA, $SNAP_COMMON etc. directly in hooks (no need for intermediate values in the yaml).

Hmm, wait, this makes no sense. We run daemons as root and we do expand their environment. Is this really by design?

@zyga-snapd I don’t know the original designs and I didn’t implement the hook machinery. All I can say is right now the env expansion happens in snap-exec and for some reason we don’t expand for hooks - fixing this is a one line change if that’s what we want.

I think @niemeyer and @mvo should review this. It certainly feels broken to me.

Thanks for checking this out.

I could see how full environment expansion likely has security issues, but @zyga-snapd’s question seems valid.

If there are security issues, would it be possible to create a clean environment with the appropriate $SNAP* variables in it, and use that to do expansion in snap-exec (I think this is fairly common pattern)? Or some similar mechanism where at least $SNAP* variables can be interpolated.

2 quick arguments for this idea

  1. simplicity and principle of least surprise. In general there seems to be a move to treat apps and hooks as similarly as possible (e.g. snapctl in apps, etc).

  2. My use case is to define a constant set of paths, based below SNAP/SNAP_DATA/SNAP_COMMON, in one place. Then my hooks and my apps can use that as the one source of truth, keeping it DRY. I did have an env file that I manually sourced in each and every app wrapper and in the configure hook wrapper, but I would prefer to have that in the snap.yaml, where it’s more visible and less manual.

Also, the issue of the appended new line is quite annoying. I don’t know where there is coming from, but it isn’t from the yaml.

Thanks

You’re right. We discussed this further in a hangout. Indeed, it doesn’t seem sensible to treat hooks any different. I’ll look at fixing this. Will report here when I have something for review.

2 Likes

I’m sorry it took so long… The PR that fixes it is here: https://github.com/snapcore/snapd/pull/3918

1 Like

After my PR above the variables from environment section should now be interpolated for hooks too. Also, @mvo fixed the case of command: foo $ENV_STRING. Now, I’m unclear if there’s anything else we want to do here?

Sorry to hijack this conversation, but it seems to be addressing something I am having difficulty with:

Do I understand correctly that the environment in which the snap executes is supposed to inherit from the user running the snap? E.g., in the case of rocketchat-server, where the root user runs the daemon, are the root user’s environment variables (possibly defined in .bashrc or .profile) inherited into the snap environment?

In my specific case, I need to set NODE_EXTRA_CA_CERTS so that nodejs can trust a corporate CA certificate, but I cannot see any way of setting this in the snap environment. Setting it for the root user has not helped either.

daemons do not inherit environment from root. You can use systemd’s snippets to do so. This isn’t particularly user-friendly but should work: use systemctl list-unit-files snap.\*.service to find the name of the unit, and then run systemd edit <unit>; for example

$ snap install test-snapd-python-webserver
test-snapd-python-webserver 16.04-3 from 'canonical' installed
$ systemctl list-units snap.\*.service
UNIT                                                                 LOAD   ACTIVE SUB     DESCRIPTION
snap.test-snapd-python-webserver.test-snapd-python-webserver.service loaded active running Service for snap application test-snapd-python-webserver.test-snapd-python-w

LOAD   = Reflects whether the unit definition was properly loaded.
ACTIVE = The high-level unit activation state, i.e. generalization of SUB.
SUB    = The low-level unit activation state, values depend on unit type.

1 loaded units listed. Pass --all to see loaded but inactive units, too.
To show all installed unit files use 'systemctl list-unit-files'.
$ sudo systemctl edit snap.test-snapd-python-webserver.test-snapd-python-webserver.service

you’re given a new file to edit; add the following:

[Service]
Environment=XYZZY=hello

you can use systemctl cat to see the changes have worked,

$ sudo systemctl cat snap.test-snapd-python-webserver.test-snapd-python-webserver.service
# /etc/systemd/system/snap.test-snapd-python-webserver.test-snapd-python-webserver.service
[Unit]
# Auto-generated, DO NOT EDIT
Description=Service for snap application test-snapd-python-webserver.test-snapd-python-webserver
Requires=snap-test\x2dsnapd\x2dpython\x2dwebserver-6.mount
Wants=network-online.target
After=snap-test\x2dsnapd\x2dpython\x2dwebserver-6.mount network-online.target
X-Snappy=yes

[Service]
ExecStart=/usr/bin/snap run test-snapd-python-webserver
SyslogIdentifier=test-snapd-python-webserver.test-snapd-python-webserver
Restart=on-failure
WorkingDirectory=/var/snap/test-snapd-python-webserver/6
TimeoutStopSec=30
Type=simple

[Install]
WantedBy=multi-user.target

# /etc/systemd/system/snap.test-snapd-python-webserver.test-snapd-python-webserver.service.d/override.conf
[Service]
Environment=XYZZY=hello

now snap restart test-snapd-python-webserver (or systemctl restart of the unit itself) and your service will have a XYZZY in its environment.

1 Like

I should’ve said: this is for manually tweaking an installed snap. As the snap’s author, I would suggest you instead look at making this kind of change work via configuration, so a user can just do snap set.

1 Like

Thank you very much @chipaca. I am going to try your solution above as a workaround and see if I can bring your alternative with snap set to the Rocket Chat snap maintainers’ attention. I am not familiar with snap set but I will read up on it. Thanks for the help!

Just double checking: this environment declaration is not yet documented on docs.snapcraft.io, right?

My question is: is this declaration for run-time, or for build-time, or for both?

Yes, the way this environment declaration looks too much like that in various CI configurations, e.g. environment: in appveyor.yml and .circleci/config.yml, as well as env in .travis.yml.

True, snapcraft is not a CI, but I think a clarification would be really nice in order to avoid confusion.
See https://discourse.gohugo.io/t/hugo-0-48-released/13908/9 for a real-life example.

Thank you!

1 Like

It should have been documented here, but it’s not, we need to fix that. We should also have an equivalent documentation for snapcraft.yaml, which is very similar but not quite the same thing.

@degville Can you please have a look into these two issues when you have a moment?

We’re overhauling the documentation format and location, so some of that will take a little while to get back into snapcraft.io.

Yep, I’ll add it to the updates we’re working on. Thanks for the heads-up.

2 Likes

It is for run-time only.

1 Like

Is there also a way to set an environment variable for a snap through the snapd api? This would enable a management system to set an env variable like a credential remotely, then trigger a snap to restart to use that env variable.

Even better, there’s explicit configuration support: https://docs.snapcraft.io/configuration-in-snaps/510 - see https://docs.snapcraft.io/supported-snap-hooks/3795 for how it works in the snap via the configure hook.

This is helpful if I’m the author of the snap. However, as a management system, I was looking for a way to solve this regardless of how the snap author creates their configure hooks. I think we can make use of the configure hook, though if there isn’t another way to dynamically set the environment variables for snaps through snapd.

Could you please open a new thread about the specific requirements your environment has and how the proposed changes would improve the situation? This thread is about declarative environment variable definitions and is a completed feature.