Skip to content

Commit cc1c3e1

Browse files
ericofericof
authored
Merge pull request #25 from kitconcept/issue-1-trim-remote-inputs
Strip whitespace from inputs that cannot contain any
2 parents c451a22 + eb93fb2 commit cc1c3e1

4 files changed

Lines changed: 156 additions & 0 deletions

File tree

‎README.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,23 @@ GitHub Action and Docker image used to deploy a Docker stack on a Docker Swarm.
4242
| `debug` | `DEBUG` | Verbose logging | | **0** |
4343
| `scale_after` | `SCALE_AFTER` | Scale a service after a deployment has converged successfully. Example: servicename=1 | | |
4444

45+
### A note on whitespace
46+
47+
`registry`, `username`, `remote_host`, `remote_port` and `remote_user` cannot
48+
contain whitespace, so any is removed before the value is used, and a line
49+
naming the input is written to the log when that happens:
50+
51+
```
52+
Input remote_host: removed whitespace from the value
53+
```
54+
55+
This exists because a stray newline in a GitHub secret is invisible in the
56+
repository UI, and used to surface much later as an opaque
57+
`ssh: Could not resolve hostname`.
58+
59+
`remote_private_key` and `password` are left exactly as given — newlines are
60+
structural in a PEM key, and whitespace can be a legitimate part of a token.
61+
4562

4663
## Using the GitHub Action
4764

‎news/1.bugfix‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed a stray newline or space in `remote_host` (or `registry`, `username`, `remote_port`, `remote_user`) breaking the deploy with an opaque `ssh: Could not resolve hostname`. Whitespace is now removed from those inputs, and the log says when it was. @ericof

‎scripts/docker-entrypoint.sh‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,29 @@ ENV_FILE_PATH="/root/.env"
1010
OPTS=("--with-registry-auth" "--resolve-image=${RESOLVE_IMAGE:-always}")
1111
[ "${PRUNE:-0}" = "1" ] && OPTS+=("--prune")
1212

13+
# Inputs whose values cannot legally contain whitespace. A newline pasted into
14+
# a GitHub secret is invisible in the repository UI and only surfaces much
15+
# later, as an opaque `ssh: Could not resolve hostname`, so strip it here.
16+
#
17+
# REMOTE_PRIVATE_KEY and PASSWORD are deliberately NOT in this list, and must
18+
# not be added: newlines are structural in a PEM key, and whitespace can be a
19+
# legitimate part of a registry token.
20+
TRIMMED_INPUTS=(REGISTRY USERNAME REMOTE_HOST REMOTE_PORT REMOTE_USER)
21+
22+
23+
trim_inputs() {
24+
local name original trimmed
25+
for name in "${TRIMMED_INPUTS[@]}"; do
26+
original="${!name-}"
27+
trimmed="${original//[[:space:]]/}"
28+
if [ "${trimmed}" != "${original}" ]; then
29+
# Reported unconditionally, not only under DEBUG: the whole problem with
30+
# this class of mistake is that it is invisible everywhere else.
31+
echo "Input ${name,,}: removed whitespace from the value"
32+
export "${name}=${trimmed}"
33+
fi
34+
done
35+
}
1336

1437
login() {
1538
echo "${PASSWORD}" | docker login "${REGISTRY}" -u "${USERNAME}" --password-stdin
@@ -127,6 +150,12 @@ else
127150
SSH_VERBOSE=""
128151
fi
129152

153+
# NORMALISE INPUTS
154+
# Runs before the first use of any of them, and before the required-input
155+
# checks below, so an input that is nothing but whitespace is reported as
156+
# missing rather than being passed on to ssh.
157+
trim_inputs
158+
130159
# PROCEED WITH LOGIN
131160
if [ -z "${USERNAME}" ] || [ -z "${PASSWORD}" ]; then
132161
echo "Container Registry: No authentication provided"

‎tests/entrypoint.bats‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,115 @@ run_entrypoint() {
8080
[[ "$output" == *"Input stack_name is required!"* ]]
8181
}
8282

83+
# --- input whitespace (issue #1) ----------------------------------------------
84+
85+
# Source the script, run trim_inputs against a controlled environment, and
86+
# print the resulting value of one variable.
87+
#
88+
# The value is printed inside brackets on purpose: bats strips trailing
89+
# newlines from $output, so an untrimmed `host\n` would compare equal to `host`
90+
# and the assertion would pass whether the trimming happened or not.
91+
trim_and_print() {
92+
local var="$1"
93+
shift
94+
run env -i PATH="${PATH}" HOME="${BATS_TEST_TMPDIR}" WANT="${var}" "$@" \
95+
bash -c "
96+
source '${ENTRYPOINT}'
97+
trim_inputs >/dev/null
98+
printf '[%s]' \"\${!WANT}\"
99+
"
100+
}
101+
102+
@test "remote_host loses a trailing newline (issue #1)" {
103+
trim_and_print REMOTE_HOST REMOTE_HOST=$'swarm.example.com\n'
104+
[ "$status" -eq 0 ]
105+
[ "$output" = "[swarm.example.com]" ]
106+
}
107+
108+
@test "remote_host loses whitespace in the middle of the value (issue #1)" {
109+
trim_and_print REMOTE_HOST REMOTE_HOST='swarm .example.com'
110+
[ "$status" -eq 0 ]
111+
[ "$output" = "[swarm.example.com]" ]
112+
}
113+
114+
@test "remote_user is trimmed (issue #1)" {
115+
trim_and_print REMOTE_USER REMOTE_USER=$'deploy\n'
116+
[ "$status" -eq 0 ]
117+
[ "$output" = "[deploy]" ]
118+
}
119+
120+
@test "remote_port is trimmed (issue #1)" {
121+
trim_and_print REMOTE_PORT REMOTE_PORT=$' 2222\n'
122+
[ "$status" -eq 0 ]
123+
[ "$output" = "[2222]" ]
124+
}
125+
126+
@test "registry is trimmed (issue #1)" {
127+
trim_and_print REGISTRY REGISTRY=$'ghcr.io\n'
128+
[ "$status" -eq 0 ]
129+
[ "$output" = "[ghcr.io]" ]
130+
}
131+
132+
@test "username is trimmed (issue #1)" {
133+
trim_and_print USERNAME USERNAME=$'someone\n'
134+
[ "$status" -eq 0 ]
135+
[ "$output" = "[someone]" ]
136+
}
137+
138+
@test "trimming an input is reported on stdout (issue #1)" {
139+
# The point of the issue: the mistake is invisible everywhere else, so the
140+
# fix has to be visible without DEBUG.
141+
run env -i PATH="${PATH}" HOME="${BATS_TEST_TMPDIR}" \
142+
REMOTE_HOST=$'swarm.example.com\n' \
143+
bash -c "source '${ENTRYPOINT}'; trim_inputs"
144+
[ "$status" -eq 0 ]
145+
[[ "$output" == *"Input remote_host: removed whitespace from the value"* ]]
146+
}
147+
148+
@test "a clean input is left alone and reported silently (issue #1)" {
149+
run env -i PATH="${PATH}" HOME="${BATS_TEST_TMPDIR}" \
150+
REMOTE_HOST=swarm.example.com REMOTE_USER=deploy REMOTE_PORT=22 \
151+
bash -c "source '${ENTRYPOINT}'; trim_inputs"
152+
[ "$status" -eq 0 ]
153+
[ -z "$output" ]
154+
}
155+
156+
@test "remote_private_key is never trimmed (issue #1)" {
157+
# Newlines are structural in a PEM key. Counting them is what makes a
158+
# regression here fail loudly instead of silently mangling the key.
159+
run env -i PATH="${PATH}" HOME="${BATS_TEST_TMPDIR}" \
160+
REMOTE_PRIVATE_KEY=$'-----BEGIN-----\nabc\n-----END-----\n' \
161+
bash -c "
162+
source '${ENTRYPOINT}'
163+
trim_inputs >/dev/null
164+
printf '%s' \"\${REMOTE_PRIVATE_KEY}\" | wc -l
165+
"
166+
[ "$status" -eq 0 ]
167+
[ "${output// /}" = "3" ]
168+
}
169+
170+
@test "password is never trimmed (issue #1)" {
171+
trim_and_print PASSWORD PASSWORD='tok en '
172+
[ "$status" -eq 0 ]
173+
[ "$output" = "[tok en ]" ]
174+
}
175+
176+
@test "the trim runs as part of the deploy flow (issue #1)" {
177+
# Every other test in this section calls trim_inputs directly; this one is
178+
# what proves it is actually wired into the script's flow, and that it runs
179+
# before the required-input checks.
180+
run_entrypoint REMOTE_HOST=$'swarm.example.com\n'
181+
[ "$status" -eq 1 ]
182+
[[ "$output" == *"Input remote_host: removed whitespace from the value"* ]]
183+
[[ "$output" == *"Input remote_user is required!"* ]]
184+
}
185+
186+
@test "a remote_host of only whitespace is reported as missing (issue #1)" {
187+
run_entrypoint REMOTE_HOST=$' \n'
188+
[ "$status" -eq 1 ]
189+
[[ "$output" == *"Input remote_host is required!"* ]]
190+
}
191+
83192
# --- container registry login -------------------------------------------------
84193

85194
@test "skips login when no credentials are given (issue #4)" {

0 commit comments

Comments
 (0)