General
Always include a shebang, and set options to exit the script when a command fails or when we use a variable that's not set:
#!/bin/bash
set -eu
Test for existence with -e
, not -d
If wifi_pass_dir
exists, but is not a directory (perhaps it's a plain file), it will not be deleted, and the mkdir
will fail. It might be better not to check (rm -f
will succeed if the file doesn't exist):
# Ensure the wifi_pass_dir exists
rm -rf wifi_pass_dir
mkdir wifi_pass_dir
However, I don't believe it's necessary to copy the config files to an insecure location and make them world readable (and writeable) - see later.
sudo
is for interactive use, not scripts
#sudo
is for interactive use, not scripts
AvoidAvoid using sudo
in a script - it expects to be interactive, and won't necessarily do what you want if not connected to a terminal. It's easier to require that the script be run under sudo
(and we can then use su
to change to a non-privileged user where necessary).
Don't parse the output of ls
#Don't parse the output of ls
FilenamesFilenames you don't control can contain all sorts of characters, including newlines. Here, we don't need to do that, as we're reading from a directory. The while read
loop can simply be a for f in *
loop.
Don't over-exercise the cat
#Don't over-exercise the cat
ThisThis cat
isn't needed:
cat "$name" | grep psk=
Just tell grep
to take its input from $name
directly:
grep psk= "$name"
We don't even need a loop, as we can pass all the filenames to grep
in one go.
#Simplified code
Simplified code
#!/bin/bash
exec grep 'psk=' /etc/NetworkManager/system-connections/*
Yes, that's the whole program. Since we only execute one command, we don't need set -e
, and as we use no variables, neither do we need set -u
.