Repository navigation
Do not modify PATH when preparing services - #1087
stacyharper wants to merge 1 commit into
Conversation
d2776f9 to
6001a0a
Compare
stacyharper
changed the title
| if ((path = getenv("PATH"))) { | ||
| xasprintf(&p, "%s:%s", RC_PATH_PREFIX, path); | ||
| } else { | ||
| xasprintf(&p, "%s:%s", RC_PATH_PREFIX, RC_PATH_DEFAULT); |
There was a problem hiding this comment.
this ends up creating a path that has /bin /sbin /usr/bin /usr/sbin twice
would be better to fixup RC_PATH_PREFIX to be our default value asw
There was a problem hiding this comment.
Yes that is expected, because we will strip the prefix, and only the default value will remains. We could make the prefix the default, but then we have to detect this situation, and then not trim the prefix. And also this implies that the prefix become the default, with the local parts, and parts included in a correct order. Am not sure if this way is more simple
| } else { | ||
| xasprintf(&p, "%s:%s", RC_PATH_PREFIX, RC_PATH_DEFAULT); | ||
| } | ||
| setenv("PATH", p, 1); |
There was a problem hiding this comment.
xasprintf(&p, "PATH=%s", RC_PATH_PREFIX);
putenv(p);no need to double allocate (once here, once by libc inside setenv)
There was a problem hiding this comment.
ah right! thanks I was unaware that that string used with putenv become part of the env.
| len = strlen(RC_PATH_PREFIX ":"); | ||
| if (strncmp(token, RC_PATH_PREFIX ":", len) == 0) { | ||
| newpath = xmalloc(strlen(token) - len + 1); | ||
| strcpy(newpath, token + len); | ||
| } else { | ||
| newpath = xmalloc(strlen(token) + 1); | ||
| strcpy(newpath, token); | ||
| } | ||
| *np = '\0'; | ||
| unsetenv("PATH"); | ||
| setenv("PATH", newpath, 1); | ||
| free(newpath); |
There was a problem hiding this comment.
no need to allocate memory here, since we're no longer editing the path
if (matches_prefix)
setenv("PATH", &token[len], true);(and nothing to be done if token doesn't match, it is already the value of PATH, ditto for s-d)
There was a problem hiding this comment.
right, dunno why I made this that complicated
6001a0a to
0fc34e8
Compare
stacyharper
changed the title
0fc34e8 to
be6bd77
Compare
stacyharper
changed the title
Currently we re-order and add missing parts to the PATH. We do so for security reason, to prevent openrc to use binaries that are not installed on system locations. We also prefix the LIBEXECDIR to make the binaries available to scripts. Doing so modify the PATH, making services to run with an unexpected value. This can break openrc-user services. To solve this, instead of re-ordering and adding missing parts, we just prefix the value with RC_PATH_PREFIX in misc.c. Then on functions.sh, we drop all PATH additional manipulations, and we strip this whole prefix on start-stop-daemon, and supervise-daemon.
be6bd77 to
818a139
Compare
2 participants
Currently we re-order and add missing parts to the PATH. We do so for
security reason, to prevent openrc to use binaries that are not
installed on system locations. We also prefix the LIBEXECDIR to make
the binaries available to scripts.
Doing so modify the PATH, making services to run with an unexpected
value. This can break openrc-user services.
To solve this, instead of re-ordering and adding missing parts, we just
prefix the value with RC_PATH_PREFIX in misc.c. Then on functions.sh,
we drop all PATH additional manipulations, and we strip this whole
prefix on start-stop-daemon, and supervise-daemon.
it is an alternative to: #1083