Skip to content

Commit 74ca192

Browse files
PromoFauxclaude
andcommitted
refactor: replace eval with array in build.sh
Building a command string and calling eval is vulnerable to shell injection if branch names or fork names contain metacharacters. Replace with a bash array (DOCKER_BUILD_CMD) and execute directly via "${DOCKER_BUILD_CMD[@]}". USE_CACHE and TAG are now tracked as dedicated variables rather than via string substitution, which also removes the fragile TAG replacement logic. Signed-off-by: Adam Warner <me@adamwarner.co.uk> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 2f0aed6 commit 74ca192

1 file changed

Lines changed: 21 additions & 15 deletions

File tree

build.sh

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ usage() {
2020

2121
# Set default values
2222
TAG="pihole:local"
23-
DOCKER_BUILD_CMD="docker buildx build src/. --tag ${TAG} --load --no-cache"
2423
FTL_FLAG=false
24+
USE_CACHE=false
2525
CORE_FORK="pi-hole"
2626
WEB_FORK="pi-hole"
2727
PADD_FORK="pi-hole"
@@ -56,6 +56,9 @@ check_branch_exists() {
5656
fi
5757
}
5858

59+
# Collect extra --build-arg values from flags
60+
BUILD_ARGS=()
61+
5962
# Parse command line arguments
6063
while [[ $# -gt 0 ]]; do
6164
key="$1"
@@ -71,7 +74,7 @@ while [[ $# -gt 0 ]]; do
7174
usage
7275
fi
7376
FTL_FLAG=true
74-
DOCKER_BUILD_CMD+=" --build-arg FTL_SOURCE=local"
77+
BUILD_ARGS+=(--build-arg "FTL_SOURCE=local")
7578
shift
7679
;;
7780
-f | --ftlbranch)
@@ -82,58 +85,56 @@ while [[ $# -gt 0 ]]; do
8285
FTL_FLAG=true
8386
FTL_BRANCH="$2"
8487
check_branch_exists "ftl" "$FTL_BRANCH"
85-
DOCKER_BUILD_CMD+=" --build-arg FTL_BRANCH=$FTL_BRANCH"
88+
BUILD_ARGS+=(--build-arg "FTL_BRANCH=$FTL_BRANCH")
8689
shift
8790
shift
8891
;;
8992
-c | --corebranch)
9093
CORE_BRANCH="$2"
9194
check_branch_exists "pi-hole" "$CORE_BRANCH" "$CORE_FORK"
92-
DOCKER_BUILD_CMD+=" --build-arg CORE_BRANCH=$CORE_BRANCH"
95+
BUILD_ARGS+=(--build-arg "CORE_BRANCH=$CORE_BRANCH")
9396
shift
9497
shift
9598
;;
9699
-w | --webbranch)
97100
WEB_BRANCH="$2"
98101
check_branch_exists "web" "$WEB_BRANCH" "$WEB_FORK"
99-
DOCKER_BUILD_CMD+=" --build-arg WEB_BRANCH=$WEB_BRANCH"
102+
BUILD_ARGS+=(--build-arg "WEB_BRANCH=$WEB_BRANCH")
100103
shift
101104
shift
102105
;;
103106
-p | --paddbranch)
104107
PADD_BRANCH="$2"
105108
check_branch_exists "padd" "$PADD_BRANCH"
106-
DOCKER_BUILD_CMD+=" --build-arg PADD_BRANCH=$PADD_BRANCH"
109+
BUILD_ARGS+=(--build-arg "PADD_BRANCH=$PADD_BRANCH")
107110
shift
108111
shift
109112
;;
110113
-cf | --corefork)
111114
CORE_FORK="$2"
112-
DOCKER_BUILD_CMD+=" --build-arg CORE_FORK=$CORE_FORK"
115+
BUILD_ARGS+=(--build-arg "CORE_FORK=$CORE_FORK")
113116
shift
114117
shift
115118
;;
116119
-wf | --webfork)
117120
WEB_FORK="$2"
118-
DOCKER_BUILD_CMD+=" --build-arg WEB_FORK=$WEB_FORK"
121+
BUILD_ARGS+=(--build-arg "WEB_FORK=$WEB_FORK")
119122
shift
120123
shift
121124
;;
122125
-pf | --paddfork)
123126
PADD_FORK="$2"
124-
DOCKER_BUILD_CMD+=" --build-arg PADD_FORK=$PADD_FORK"
127+
BUILD_ARGS+=(--build-arg "PADD_FORK=$PADD_FORK")
125128
shift
126129
shift
127130
;;
128131
-t | --tag)
129-
CUSTOM_TAG="$2"
130-
DOCKER_BUILD_CMD=${DOCKER_BUILD_CMD/$TAG/$CUSTOM_TAG}
131-
TAG=$CUSTOM_TAG
132+
TAG="$2"
132133
shift
133134
shift
134135
;;
135136
use_cache)
136-
DOCKER_BUILD_CMD=${DOCKER_BUILD_CMD/--no-cache/}
137+
USE_CACHE=true
137138
shift
138139
;;
139140
*)
@@ -143,9 +144,14 @@ while [[ $# -gt 0 ]]; do
143144
esac
144145
done
145146

147+
# Build the command as an array to avoid eval and shell injection
148+
DOCKER_BUILD_CMD=(docker buildx build src/. --tag "${TAG}" --load)
149+
[ "$USE_CACHE" = false ] && DOCKER_BUILD_CMD+=(--no-cache)
150+
DOCKER_BUILD_CMD+=("${BUILD_ARGS[@]}")
151+
146152
# Execute the docker build command
147-
echo "Executing command: $DOCKER_BUILD_CMD"
148-
eval "${DOCKER_BUILD_CMD}"
153+
echo "Executing command: ${DOCKER_BUILD_CMD[*]}"
154+
"${DOCKER_BUILD_CMD[@]}"
149155

150156
# Check exit code of previous command
151157
if [ $? -ne 0 ]; then

0 commit comments

Comments
 (0)