anordal / shellharden Goto Github PK
View Code? Open in Web Editor NEWThe corrective bash syntax highlighter
License: Mozilla Public License 2.0
The corrective bash syntax highlighter
License: Mozilla Public License 2.0
Support zsh, as it is supposedly more awesome compared to bash.
hi. i'm running 4.3.1 on arch linux. a file with just the following lines in it
if [[ $option =~ (\[((no|dont)-?)\]). ]]; then
option2=${option/"${BASH_REMATCH[1]}"/}
option2=${option2%%[<{().[]*}
options+=("${option2/=*/=}")
option=${option/"${BASH_REMATCH[1]}"/"${BASH_REMATCH[2]}"}
fi
gets
x.sh: Unexpected end of file
The file's end was reached without closing all sytactic scopes.
Either, the parser got lost, or the file is truncated or malformed.
cheers.
Firstly, thank you very much for shellharden
!
I have a small issue that is happening with version 4.1.1.
My (not properly quoted) line begins in this state:
SERVER_LOCATIONS[$location]="$zone"
shellharden --check --replace
changes it to:
SERVER_LOCATIONS["$location]=$zone"
with quotes only in front of the subscript and at the end of the value being assigned.
It should be changing it to something like this instead:
SERVER_LOCATIONS["$location"]="$zone"
to fully quote both the subscript and the value.
Running shellharden
or multiple files is confusing, since there's no clear delimiter between their contents. Current behavior is akin to cat
, while a bat
-like mode would be more appropriate.
I know there's a --help
flag, but almost every tool I've used has both a long and short form so you don't shoot yourself in the foot.
In reply to https://github.com/anordal/shellharden/blob/master/how_to_do_things_safely_in_bash.md#should-i-use-curly-braces :
Correct:
some_command "${arg1}" "${arg2}" "${arg3}"
Better:some_command "$arg1" "$arg2" "$arg3"
Curly braces should be preferred in order to limit the risk that a variable named a prefix of other variables does not alter other variables. An example:
arg="example"
some_command "$arg1" "$arg2" "$arg3"
some_command "${arg1}" "${arg2}" "${arg3}"
I really love shellharden so far, since that allows me to quickly rewrite scripts that I wrote too hastily 🙂 .
But when I ran it on a script I put a lot of effort in, it suggested to remove the braces ({
, }
), in places that do not strictly need them, which is something I prefer to do.
A variable in bash is like a hand grenade – take off its quotes, and it starts ticking.
For the same reason I prefer to always use the braces. Since I do not know how the line will change in the future.
Would it be possible to make shellharden do this? Perhaps as a configuration option?
A simple example which would have been prevented by always using braces:
my_file='abc.txt'
cp "$my_file"{,_copy}
ls "$my_file"
my_file='abc.txt'
cp "$my_file"{,_copy}
ls "$my_file_copy" # error, unset variable
I cloned current master and did cargo build
. Got this:
Compiling shellharden v3.1.0 (file:///H:/GitLocalRepos/shellharden)
error: expected item, found `..`
--> src\main.rs:1:1
|
1 | ../shellharden.rs
| ^^ expected item
error: aborting due to previous error
error: Could not compile `shellharden`.
To learn more, run the command again with --verbose.
I'm using
stable-i686-pc-windows-gnu (default)
rustc 1.26.1 (827013a31 2018-05-25)
What am I doing wrong?
Hi,
I've just tried installing this on my Docker Debian Bullseye container using cargo and get the following results: (specifically, I'm using the "debian:bullseye-slim" container with a few additional packages - such as coreutils, make, gcc, curl, rustc and cargo)
> cargo install shellharden --verbose
Updating crates.io index
Installing shellharden v4.3.0
Compiling shellharden v4.3.0
Running `rustc --crate-name shellharden /home/user/.cargo/registry/src/github.com-1ecc6299db9ec823/shellharden-4.3.0/src/main.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type bin --emit=dep-info,link -C opt-level=3 -Cembed-bitcode=no -C metadata=c67ba048abc62b54 -C extra-filename=-c67ba048abc62b54 --out-dir /tmp/cargo-installie2YhN/release/deps -L dependency=/tmp/cargo-installie2YhN/release/deps --cap-lints allow`
error[E0658]: or-patterns syntax is experimental
--> /home/user/.cargo/registry/src/github.com-1ecc6299db9ec823/shellharden-4.3.0/src/sitvarbrace.rs:63:6
|
63 | (State::Name | State::Index | State::Normal, b'}') => {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #54883 <https://github.com/rust-lang/rust/issues/54883> for more information
error: aborting due to previous error
For more information about this error, try `rustc --explain E0658`.
error: failed to compile `shellharden v4.3.0`, intermediate artifacts can be found at `/tmp/cargo-installie2YhN`
Caused by:
could not compile `shellharden`.
Caused by:
process didn't exit successfully: `rustc --crate-name shellharden /home/user/.cargo/registry/src/github.com-1ecc6299db9ec823/shellharden-4.3.0/src/main.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type bin --emit=dep-info,link -C opt-level=3 -Cembed-bitcode=no -C metadata=c67ba048abc62b54 -C extra-filename=-c67ba048abc62b54 --out-dir /tmp/cargo-installie2YhN/release/deps -L dependency=/tmp/cargo-installie2YhN/release/deps --cap-lints allow` (exit code: 1)
Here's some platform information which might help:
> cargo --version
cargo 1.46.0
> rustc --version
rustc 1.48.0
> cat /etc/debian_version
11.3
I've not really run a Rust script before which has failed with an error during install, so I'm not sure what information you may want/need.
Brilliant tool. Thanks. I just learned about it today, and I found it hard to figure out exactly what the tool was, or if there were multiple tools without downloading/installing it. I'd like to suggest adding a section to the README.md that shows some example calls, e.g.
$ shellharden script.sh
maybe immediately above each screenshot example.
Another thing that would help people "passing by" could be to add:
# Usage
` ```
$ shellharden --help
Shellharden: The corrective bash syntax highlighter.
Usage:
shellharden [options] [files]
cat files | shellharden [options] ''
Shellharden is a syntax highlighter and a tool to semi-automate the rewriting
of scripts to ShellCheck conformance, mainly focused on quoting.
The default mode of operation is like `cat`, but with syntax highlighting in
foreground colors and suggestive changes in background colors.
Options:
--suggest Output a colored diff suggesting changes.
--syntax Output syntax highlighting with ANSI colors.
--syntax-suggest Diff with syntax highlighting (default mode).
--transform Output suggested changes.
--check No output; exit with 2 if changes are suggested.
--replace Replace file contents with suggested changes.
-- Don't treat further arguments as options.
-h|--help Show help text.
--version Show version.
The changes suggested by Shellharden inhibits word splitting and indirect
pathname expansion. This will make your script ShellCheck compliant in terms of
quoting. Whether your script will work afterwards is a different question:
If your script was using those features on purpose, it obviously won't anymore!
Every script is possible to write without using word splitting or indirect
pathname expansion, but it may involve doing things differently.
See the accompanying file how_to_do_things_safely_in_bash.md or online:
https://github.com/anordal/shellharden/blob/master/how_to_do_things_safely_in_bash.md
` ```
## Usage advice
Don't apply `--transform` blindly; code review is still necessary: A script that relies on unquoted behavior
(implicit word splitting and glob expansion from variables and command substitutions) …
which of course requires more work, since you need to remember to update it whenever --help
output changes
Hi,
I just found that shellharden
has some difficulties with nested case statements like in this example input:
e=0
case $i in
a) : this ;;
b) : that ;;
*)
case $i in
c) j=3 ;;
d) j=2 ;;
*) e=1 ;;
esac
: use j
;;
esac
: use e
I get this unexpected diff transform:
@@ -7,7 +7,7 @@
c) j=3 ;;
d) j=2 ;;
*) e=1 ;;
- esac
+ ;; esac
: use j
;;
esac
Hi
I just saw your app and if you need a new logo or widget design i can help you
You can tell me what kind of a logo do you want
I am waiting for you reply
Hi!
If shellharden would support the same command line arguments as shellcheck, then it could act as a drop-in replacement for shellcheck, and be used in the projects/IDEs/scripts that currently use shellcheck.
Here is a quick overview of the current shellcheck arguments:
Usage: shellcheck [OPTIONS...] FILES...
-a --check-sourced Include warnings from sourced files
-C[WHEN] --color[=WHEN] Use color (auto, always, never)
-e CODE1,CODE2.. --exclude=CODE1,CODE2.. Exclude types of warnings
-f FORMAT --format=FORMAT Output format (checkstyle, gcc, json, tty)
-s SHELLNAME --shell=SHELLNAME Specify dialect (sh, bash, dash, ksh)
-V --version Print version information
-x --external-sources Allow 'source' outside of FILES
Supporting all different output formats and shell dialects would be a huge task, but just having support for -s bash
, would go a long way.
Cheers!
Shellcheck requires changing printf "Error: %s\n" "Bad parameters: $@"
to printf "Error: %s\n" "Bad parameters: $*"
. Shellharden requires the opposite.
Is there fundamental disagreement or is it a false positive?
Shellharden does not consider the quotes in a default value, which can lead to odd recommendations. There doesn't seem to be any actual problem with the suggestion though, other than looking odd.
Given:
#!/usr/bin/env bash
echo ${bar:-"two words"}
Shellharden produces:
#!/usr/bin/env bash
echo "${bar:-"two words"}"
The expected output should probably be as follows:
#!/usr/bin/env bash
echo "${bar:-two words}"
Would it be possible to release just the built binary files (i.e. output of ../cargo/bin/shellharden ) for major platforms? It's just a pain to install gcc (55M), rust and cargo (254M) and all their associated stuff/dependencies, just to generate the 3.8M shellharden file which seems to run fine and dandy without any of the others (at least on Linux).
shellcheck seems to offer prebuilt binaries for linux.x86_86, linux.aarch64, linux.arm6hf, darwin.x86_64 and windows.x86_64 (as per https://github.com/koalaman/shellcheck/blob/master/.github/workflows/build.yml )
(I'm trying to build a reproducible Dockerfile for my own usage for bash development - at least at the moment - and I want it to be as small and lightweight as possible. I'm having to use the fixed .tar.gz installers of rust/cargo as I can't depend on rustup returning the same file every time)
Build docker image
docker build -t shellgarden .
I'm assuming this is meant to be
docker build -t shellharden .
?
I've grown accustom to not quoting the right hand side of variable assignments,
var=$othervar
var=$(subshell)
var=value
This is perfectly safe in all POSIX shells (AFAIK). Reference.
Shellharden adds quotes to all of these. Do you consider this a bug?
the document currently contains:
The simplest is to go for single quotes, since these have the simplest escaping rules – only one: ' → '''.
this seems incorrect. backslashes do not escape anything inside single quotes::
\
that also means that this is an incomplete command, as can be seen from the continuation prompt:
$ echo '\''
>
> ^C
the correct approach is to replace '
with '"'"'
which looks horrible, but is correct:
'
closes the open single-quoted string"
opens a double-quoted string'
puts a single quote inside the double-quoted string"
closes the double-quoted string'
opens a new single-quoted stringwith the following minimal1 reproducible example:
$ cat file.sh
case $PATH in
:"$(dirname foo)")
esac
$ shellharden file.sh
file.sh: Unexpected end of file
The file's end was reached without closing all sytactic scopes.
Either, the parser got lost, or the file is truncated or malformed.
$ echo $?
1
Less minimally, I found this while creating a case
to ensure *:/usr/local/share/"$(command basename -- "${SHELL%%[0-9-]*}")"/site-functions:*
was in ":${FPATH-}:"
↩
hi, i tried both the monolithic bin from releases, and the cargo install on ubuntu, both just color the comments if called passing a bash script as argument (and doing the variable escaping etc etc, yes), but no syntax highlight in fg or mark of the "changing" parts in bg color... any help?
@anordal In echo → printf, the guide is trying to make a case that printf
should be preferred over echo
; however in a lot of other places within the guide (eg. Corollary: Use while loops to iterate strings and command output), echo
is being used extensively. Do we want to consolidate the guide and rewrite examples in printf
wherever possible?
We may want to remove the Build docker image
section from README.md entirely, as the Dockerfile
was removed in df5ff65.
Please add a LICENSE
or COPYING
file, for making shellharden
easier to package for Linux distros.
Cheers!
FYI. New versions of bash has set -E that 'If set, the ERR trap is inherited by shell functions'. That should make gotcha more and less annoying. By that I mean it is easy to avoid 'set -e' in every single function, but one still may have to go and update whole bunch of old scripts to use -E rather than -e.
Thank you for taking the time to write "Safe ways to do things in bash", this is extremely valuable information. I stumbled upon a surprising behavior in one of your suggestions.
The read
-based method for splitting strings into arrays described here breaks in combination with set -e
, since the return code of read
is nonzero when it encounters end-of-file. I am not sure if I have a good suggestion for how to fix it, short of specifying a long unique string for the -d
argument and appending it to the end of the printf
format string, or putting the call to read
within a compound expression.
Tested on Bash 4.4.19.
Quoting is generally not needed when variables are used between double brackets ([[
and ]]
).
Here is a test case that does not expand:
x='*'; if [[ $x == '*' ]]; then echo not expanded; fi
While with single brackets, this is the result:
x='*'; if [ $x == '*' ]; then echo not expanded; fi
bash: [: too many arguments
shellharden
wants to add double quotes around $x
in the first case, but this is not really needed.
Running shellharden erratic-script
on macOS (via Homebrew) simply outputs the script as-is. No syntax highlighting or no suggestions. For comparison, shellcheck erratic-script
outputs a bunch of errors. What could be wrong?
It would be nice if there was a way to filter input through STDIN and receive the transformed version back. This could be used for editor integrations (notably vim).
Version 4.1.1 is the most recent release according to https://github.com/anordal/shellharden/releases, but looking at the https://github.com/anordal/shellharden/tags, it looks like there are also 4.1.2, 4.1.3, and 4.2.0?
Is it an oversight that those versions are not "released"?
In the process of making changes to formulae in the Homebrew package manager, I noticed that shellharden was one of a handful of Rust binary projects without a Cargo.lock
file in version control. The Cargo book recommends the following (source):
If you’re building an end product, which are executable like command-line tool or an application, or a system library with crate-type of staticlib or cdylib, check
Cargo.lock
intogit
.
More information about the reasoning can be found in the "Why do binaries have Cargo.lock in version control, but not libraries?" section of the Cargo FAQ.
The Cargo.lock
file helps package managers to keep builds reproducible, since cargo install
simply uses the latest dependency versions unless the --locked
flag is added to the command, in which case it will use the versions outlined in Cargo.lock
. Without a Cargo.lock
file, there's a chance that a dependency update will break the build sometime in the future, which is something I've already encountered with other Rust binary projects.
I know shellharden currently doesn't have any dependencies but could you please consider checking Cargo.lock
into version control? There's a chance that Homebrew might roll out the --locked
flag as a default for building Rust projects from source, so it would be helpful to have the Cargo.lock
file available here to avoid errors.
Hello,
The section on using arrays could really do with an example of using an array in a loop.
https://github.com/anordal/shellharden/blob/master/how_to_do_things_safely_in_bash.md#use-arrays-ftw
maybe
xs=(
a
b
)
for x in "${xs[@]}" ; do
echo "$x"
end
But I did not submit a PR because I am not sure that is actually correct.
Thanks for the great resource!
Hey! First of all thank you for this project, it's really helpful and I'm going to integrate it into our CI pipeline(s).
What I'm thinking about is I'd also like to verify that our bash scripts have
shopt -s nullglob globstar
and
if test "$BASH" = "" || "$BASH" -uc "a=();true \"\${a[@]}\"" 2>/dev/null; then
# Bash 4.4, Zsh
set -euo pipefail
else
# Bash 4.3 and older chokes on empty arrays with set -u.
set -eo pipefail
fi
at the beginning. Do you think there's place for this in shellharden
(behind a flag, presumably)? I could open a PR if so.
shellharden --check test.sh
works one way, while shellharden test.sh --check
has a different behavior.
Please close if this is intentional.
Following seems incorrect since command's return value is overwritten by if statement.
if ! command; then
echo "Command returned $?"
fi
Use following instead:
command || echo "Command returned $?"
This only works if I remove the qoutes arround $(make_extension_volume_string)
.
make_extension_volume_string() {
extensions_volume_string=""
for extension_source_dir in "${EXTENSION_SOURCE_DIRS[@]}"; do
host_path=$(realpath "$extension_source_dir")
base=$(basename "$extension_source_dir")
extensions_volume_string+=" --volume {$host_path}:/usr/lib/ckanext/{$base}"
done
printf '%s' "$extensions_volume_string"
}
podman run -dt --pod "$pod_name" \
--name ckan-"$IMAGE_STAGE" \
--env-file "$env" \
--env "CKAN_REDIS_URL=redis://localhost:6379/1" \
--env "CKAN_SQLALCHEMY_URL=postgresql://ckan:$POSTGRES_PASSWORD@localhost:5432/ckan" \
--env "CKAN_SOLR_URL=http://localhost:8983/solr/ckan" \
--volume "$(realpath "$MERGE_INI")":/etc/ckan/merge.ini \
"$(make_extension_volume_string)" \
--volume ckan_home:/usr/lib/ckan \
--volume ckan_storage:/var/lib/ckan \
ckan-"$IMAGE_STAGE"
Otherwise this throws this error:
+ podman run -dt --pod ckan-develop --name ckan-development --env-file develop --env CKAN_REDIS_URL=redis://localhost:6379/1 --env CKAN_SQLALCHEMY_URL=postgresql://ckan:ckan@localhost:5432/ckan --env CKAN_SOLR_URL=http://localhost:8983/solr/ckan --volume /home/beavis/repositories/ckan-podman/deployment/ckan.ini:/etc/ckan/merge.ini ' --volume {/home/beavis/repositories/ckanext-test}:/usr/lib/ckanext/{ckanext-test}' --volume ckan_home:/usr/lib/ckan --volume ckan_storage:/var/lib/ckan ckan-development
Error: invalid reference format
What am I doing wrong?
If I save example.sh
with this content:
export variable=${one:-${two}}
and run shellharden --replace -- example.sh
, its content is changed to this:
export variable="${one:-${two}"}
instead of perhaps this:
export variable="${one:-${two}}"
or keeping the right side without quotation marks:
export variable=${one:-${two}} # no change from input
I was able to reproduce this using shellharden 4.1.2 on Ubuntu 20.10’s GNOME Terminal 3.38 and macOS 11.3’s Terminal 2.11 and iTerm2 3.4
I feel like I must be missing something obvious, my apologies if so. I want to do something like this:
function _get_contents_of_function {
local func=$1
# shellcheck disable=2086
declare -f $func # because "$func" doesn't work
}
Have I got the wrong code, or shellharden doesn't support any kind of ignoring? Or is this kind of meta programming just not supported by shellharden?
I used shellharden
to transform the script at https://github.com/richb-hanover/OpenWrtScripts/blob/master/betterspeedtest.sh
I got an error when trying to run the resulting file:
% shellharden --transform betterspeedtest.sh > junk.txt
% sh junk.txt -t 10 -H netperf-west.bufferbloat.net
junk.txt: line 210: syntax error near unexpected token `;;'
junk.txt: line 210: ` ;; esac ;;'
Removing the leading ';;' from line 210 allows the script to work as expected.
Is this expected? Should I tweak my source file to avoid this? Many thanks.
Version:
Steps that will reproduce the problem?
shellharden --check test.sh
(see below)What is the expected result?
What happens instead?
% shellharden --check test.sh
test.sh: Unexpected end of file
The file's end was reached without closing all sytactic scopes.
Either, the parser got lost, or the file is truncated or malformed.
Any additional information:
% ./test.sh
Expected to be 0 -> 0
Expected to be 1 -> 1
Expected to be 2 -> 2
#!/usr/bin/env bash
vercomp() {
if [[ "$1" == "$2" ]]; then
return 0
fi
local IFS=.
local i ver1 ver2
read -a ver1 <<<"$1"
read -a ver2 <<<"$2"
# fill empty fields in ver1 with zeros
for ((i = ${#ver1[@]}; i < ${#ver2[@]}; i++)); do
ver1[i]=0
done
for ((i = 0; i < ${#ver1[@]}; i++)); do
if [[ -z "${ver2[i]}" ]]; then
ver2[i]=0
fi
if ((10#${ver1[i]} > 10#${ver2[i]})); then
return 1
fi
if ((10#${ver1[i]} < 10#${ver2[i]})); then
return 2
fi
done
return 0
}
vercomp 3.0 3
echo "Expected to be 0 -> $?"
vercomp 2.2 0.5
echo "Expected to be 1 -> $?"
vercomp 1.4 1.5
echo "Expected to be 2 -> $?"
The module test (aka. integration test) doesn't know if cargo test
is invoked with --release
or not.
That is the fundamental problem. Furthermore, it doesn't really want to concern itself with where the various debug and release executables are located.
Currently, it searches for executables named "shellharden" and just tests every one of them. Care is taken to search CARGO_TARGET_DIR if set.
Why this is wrong: When you think you are testing the debug build, you are implicitly also testing a stale release build if it exists, and vice versa.
Problem:
rust-lang/cargo#3670
Solution:
rust-lang/cargo#7697
I have a script that (for some reason) wound up with curly quotes within some documentation comments. e.g.,
# “H” and “host” DNS or IP address of the netperf server host (default: netperf.bufferbloat.net)
shellharden appears to change these to ^D characters. Is this intended?
Thanks for a cool tool!
PS I could believe that I mis-diagnosed. And I am happy to get rid of the curly quotes...
I was wondering why you didn't use Cargo. Is it a matter of choice or it is because you're not comfortable with it?
Furthermore, using Cargo will allow you to publish your project on Crates.io and setup a test suite more easily.
Pacstall is a community-driven AUR-like package manager for Ubuntu. We have an ongoing pull request to add shellharden to our repository.
We have a few questions for you:
Hi there!
shellharden looks fantastic, and I was considering integrating it into some lint infrastructure we have. However, it looks like the only stable interface is a binary. Would you consider separating out the checks into a library and providing a Rust API as well?
A bonus would be if you could read files from memory rather than the file system -- perhaps via a trait that specifies how files should be read.
I do something similar for a tool I'm the primary author of, cargo-guppy, where there's both a Rust interface (guppy
) and a CLI tool (cargo-guppy
).
Thank you!
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.