Skip to content

Commit

Permalink
Add shellcheck
Browse files Browse the repository at this point in the history
Copy the `shellcheck.sh` script from the SecureDrop server repository
and add it to the central `make lint` target. I added SC2164 as a global
exclusion since we don't really care about `cd` potentially failing.

The following fixes are being made:
* client/.githooks/pre-commit: Use $() instead of backticks.
* debian/setup-venv.sh: Ignore quoting words, all the variables are a
  single word only.
* proxy/entrypoint.sh: Adjust shebang since we use bashisms.
* scripts/build-debs.sh: Set variable and then export it
* scripts/fixup-changelog.sh: Quote our variables, don't use `! -z`.

Fixes #814.
  • Loading branch information
legoktm committed Feb 22, 2024
1 parent db77515 commit 66f4118
Show file tree
Hide file tree
Showing 12 changed files with 64 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
container: debian:${{ matrix.debian_version }}
steps:
- run: |
apt-get update && apt-get install --yes git make
apt-get update && apt-get install --yes git make file
- uses: actions/checkout@v4
- name: Install dependencies
run: |
Expand Down
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ lint-desktop: ## Lint .desktop files
find . -name *.desktop -type f -not -path '*/\.git/*' | xargs desktop-file-validate

.PHONY: lint
lint: check-black check-isort bandit ## Run linters and formatters
lint: check-black check-isort bandit shellcheck ## Run linters and formatters

.PHONY: fix
fix: black isort ## Fix lint and formatting issues
Expand Down Expand Up @@ -43,6 +43,10 @@ safety: ## Run safety dependency checks on build dependencies
--ignore 62044 \
-r

.PHONY: shellcheck
shellcheck: ## Lint shell scripts
@poetry run ./scripts/shellcheck.sh

.PHONY: rust-lint
rust-lint: ## Lint Rust code
cargo fmt --check
Expand Down
2 changes: 1 addition & 1 deletion client/.githooks/pre-commit
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh
TOP=`git rev-parse --show-toplevel`
TOP=$(git rev-parse --show-toplevel)

make -C "$TOP" check-strings
3 changes: 1 addition & 2 deletions debian/securedrop-keyring.preinst
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#!/bin/bash

# shellcheck disable=SC2064
set -e

# Solution adapted from DKG's work on `deb.torproject.org-keyring` and
Expand Down Expand Up @@ -31,4 +31,3 @@ if [ -e /etc/apt/trusted.gpg ] && which gpg >/dev/null; then
fi

#DEBHELPER#

1 change: 1 addition & 0 deletions debian/setup-venv.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/bin/bash
# shellcheck disable=SC2086
set -euxo pipefail

NAME=$1
Expand Down
15 changes: 14 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion proxy/entrypoint.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh
#!/bin/bash

cd /home/user/projects/securedrop-proxy
virtualenv .venv
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ bandit = "*"
black = "*"
isort = "*"
safety = "*"
shellcheck-py = "*"

[tool.bandit]
exclude_dirs = ["client/tests", "export/tests", "log/tests", "proxy/tests"]
Expand Down
3 changes: 2 additions & 1 deletion scripts/build-debs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@ if test -t 0; then
fi

# Look for the builder repo with our local wheels
export BUILDER=$(realpath "${BUILDER:-../securedrop-builder}")
BUILDER=$(realpath "${BUILDER:-../securedrop-builder}")
if [[ ! -d $BUILDER ]]; then
echo "Cannot find securedrop-builder repository, please check it out \
to ${BUILDER} or set the BUILDER variable"
exit 1
fi

export BUILDER
export DEBIAN_VERSION="${DEBIAN_VERSION:-bullseye}"
export OCI_RUN_ARGUMENTS
export OCI_BIN
Expand Down
4 changes: 2 additions & 2 deletions scripts/fixup-changelog.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ source /etc/os-release
if [[ "$VERSION_CODENAME" == "" ]]; then
# PRETTY_NAME="Debian GNU/Linux bookworm/sid"
# Use awk to split on spaces and /
VERSION_CODENAME=$(echo $PRETTY_NAME | awk '{split($0, a, "[ /]"); print a[4]}')
VERSION_CODENAME=$(echo "$PRETTY_NAME" | awk '{split($0, a, "[ /]"); print a[4]}')
fi

VERSION=$(dpkg-parsechangelog -S Version)

NIGHTLY="${NIGHTLY:-}"
if [[ ! -z $NIGHTLY ]]; then
if [[ -n $NIGHTLY ]]; then
NEW_VERSION="${VERSION}.dev$(date +%Y%m%d%H%M%S)"
else
NEW_VERSION=$VERSION
Expand Down
34 changes: 34 additions & 0 deletions scripts/shellcheck.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#!/bin/bash

set -e

EXCLUDE_RULES="SC1091,SC2001,SC2164"

# Omitting:
# - the `.git/` directory since its hooks won't pass # validation, and
# we don't maintain those scripts.
# - Python, JavaScript, YAML, HTML, SASS, PNG files because they're not shell scripts.
# - Cache directories
# - test results
FILES=$(find "." \
\( \
-path '*.mo' \
-o -path '*.po' \
-o -path '*.py' \
-o -path '*.yml' \
-o -path '*/.mypy_cache/*' \
-o -path './.git' \
-o -path './build/*' \
-o -path './target' \
\) -prune \
-o -type f \
-exec file --mime {} + \
| awk '$2 ~ /x-shellscript/ { print $1 }' \
| sed 's/://')
# Turn the multiline find output into a single space-separated line
FILES=$(echo "$FILES" | tr '\n' ' ')

shellcheck --version
# $FILES intentionally unquoted so each file is passed as its own argument
# shellcheck disable=SC2086
shellcheck -x --exclude="$EXCLUDE_RULES" $FILES
2 changes: 1 addition & 1 deletion update_version.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ sed -i'' -r -e "s/^__version__ = \"(.*?)\"/__version__ = \"${NEW_VERSION}\"/" cl
sed -i'' -r -e "s/^__version__ = \"(.*?)\"/__version__ = \"${NEW_VERSION}\"/" export/securedrop_export/__init__.py

# Normalize version, convert any - to ~, e.g. 0.9.0-rc1 to 0.9.0~rc1
DEB_VERSION=$(echo $NEW_VERSION | sed 's/-/~/g')
DEB_VERSION=$(echo "$NEW_VERSION" | sed 's/-/~/g')

export DEBEMAIL="securedrop@freedom.press"
export DEBFULLNAME="SecureDrop Team"
Expand Down

0 comments on commit 66f4118

Please sign in to comment.