6
\$\begingroup\$

I was having problems setting my path the way I wanted, I turned to Apple's path_helper but found it appends system dirs to the beginning of the path. I found someone had made a start on a replacement and I forked it and made some updates.

I don't write a lot of shell scripts, and less so to replace system supplied programs. As such, I'm wondering 2 main things:

  • Does this follow best practices for a shell script?
  • Is there an obvious security flaw? I'm most reticent about the line that isn't in the script itself but that calls it

The suggestion is to put this in ~/.bash_profile or ~/.zshenv:

if [ -x /usr/local/libexec/path_helper ]; then
  eval `/usr/local/libexec/path_helper`
fi

Evals make me nervous. Is that okay in this context?

From https://github.com/yb66/path_helper

#!/bin/bash -

## A better path helper - don't put the standard bins first.

function path_helper {
  local -a path_dirs
  local envvar=$1
  local target=`echo $1 | /usr/bin/awk '{print tolower($1)}'`
  local etcpaths="/etc/${target}s"
  local etcpathsd="${etcpaths}.d/*"

  while read -r dir ; do
      if [ "${#path_dirs[@]}" == "0" ]; then
          path_dirs=${dir}
      else
          path_dirs+=":${dir}"
      fi
  done < <(/bin/cat ${etcpathsd} ${etcpaths})

  path_dirs=($(echo ${path_dirs[@]} | /usr/bin/tr [:space:] '\n' | /usr/bin/awk '!a[$0]++'))
  echo "${envvar}=\"\${${envvar}}:${path_dirs[@]}\"; export ${envvar};"
}

path_helper "PATH"

# Only handle MANPATH if the variable hasn't already been set.
if [ -z "$MANPATH" ]; then
  path_helper "MANPATH"
fi
# EOF
\$\endgroup\$
3
\$\begingroup\$

What's with the absolute paths?

I don't see much point in calling cat, tr, awk by absolute paths. What if you wanted to run the script on another system where these tools are installed somewhere else? You would have to replace all of them, carefully not miss one use.

Better alternatives:

  • Add PATH="/usr/bin:$PATH" near the top of the script
  • Define tr=/usr/bin/tr near the top, and use $tr throughout the script

Simplify

This is error-prone and overly complicated:

  while read -r dir ; do
    # ...
  done < <(/bin/cat ${etcpathsd} ${etcpaths})

  path_dirs=($(echo ${path_dirs[@]} | /usr/bin/tr [:space:] '\n' | /usr/bin/awk '!a[$0]++'))

You could achieve the same thing, safer and more efficiently by making the awk command provide the input for the loop:

  while read -r dir ; do
    # ...
  done < <(cat ${etcpathsd} ${etcpaths} | awk '!a[$0]++')

And instead of using an array to collect the paths, a normal variable could be just as good, and simpler to implement:

  local paths

  while read -r dir ; do
    paths=$paths:$dir
  done < <(cat ${etcpathsd} ${etcpaths} | awk '!a[$0]++')

  echo "${envvar}=\"\${${envvar}}:${paths:1}\"; export ${envvar};"

Misc

Use $(...) instead of the obsolete and error-prone `...`.

The preferred way to write function declarations is path_helper() { instead of function path_helper {.

Instead of echo something | awk ..., write awk ... <<< something to avoid a pointless echo command.

tr [:space:] '\n' is a bit dodgy. It won't work as intended in Bash 3, unless you write '[:space:]' or [[:space:]].

The indentation level is inconsistent. Sometimes you indented with 2 spaces, sometimes 4. It's easier to read when the indentation is consistent throughout the script.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Hehe, guilty as charged! The absolute paths were because I'm meddling with the path and don't want any strange outcomes because of it. I couldn't see OSX getting rid of or changing the location of these facilities so portability wasn't a consideration but I like the idea of loading them into vars. I think I kept the indentation that was originally there, I know shell programmers have their odd ways, I just want to fit in ;-) I'll make these changes and possibly add some tests for it. Thanks, I learnt a lot and appreciate the help. \$\endgroup\$ – iain Mar 12 '17 at 17:03
4
\$\begingroup\$

As to the security of this:

if [ -x /usr/local/libexec/path_helper ]; then
  eval `/usr/local/libexec/path_helper`
fi

This runs the path_helper script, which presumably outputs a string, and then the command represented by the output string is run in the local context. So this initially presumes you trust path_helper to run anything it wants. I don't think path_helper could much more damage by running something in your shell instead of what it already ran in a sub-shell.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ That's a good point :) \$\endgroup\$ – iain Mar 10 '17 at 17:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.