Accept the URLs this project tells you to use

The URL check permitted a short list of characters that did not include "&", so
it refused any URL with two query parameters - including the push URL Uptime
Kuma hands out, which is the one value heartbeat_url is documented to hold. The
project rejected the setting the project recommends.

Refusing a legitimate URL is not the safe direction. It costs the heartbeat,
and the heartbeat is the one signal a compromised devbox cannot suppress, so an
over-strict rule removes the control that does not depend on the watched
machine cooperating - and removes it quietly, since a guard that never pings is
indistinguishable from one that was never configured to.

The set is RFC 3986's now: unreserved, gen-delims, sub-delims and %. What stays
refused is what cannot appear in a URL anyway - whitespace, control characters,
quotes, backslash and < > ^ ` { | }.

Whitespace and control characters are checked separately, because the test that
deletes permitted characters cannot see a trailing newline: command
substitution strips it, so the remainder reads as empty and the value passes.

The message names the characters it found. "contains a character that is not
allowed here" was the whole report, on values long enough that finding it meant
guessing.

The shipped example carried a path with no query, so it passed a check that
would have refused the real thing. It is the whole URL now.

bin/dejima-guard

@@ -65,14 +65,44 @@ cfg() {  # cfg KEY [DEFAULT]
 # The same restriction dejima puts on its own notify URL, for the same reason:
 # curl reads a leading -K/path as an option that takes the request from a file,
 # so a value without an explicit scheme is refused rather than guessed at.
+#
+# THE CHARACTER SET IS RFC 3986's, not a shorter list that looked sufficient.
+# The shorter list omitted "&", which every URL with two query parameters has -
+# including the push URL Uptime Kuma hands out, which is the one heartbeat_url
+# is documented to hold. So the setting this project recommends was rejected by
+# the project that recommends it.
+#
+# Refusing a legitimate URL is not the safe direction here. It costs the
+# heartbeat, and the heartbeat is the one signal a compromised devbox cannot
+# suppress - so an over-strict rule quietly removes the control that does not
+# depend on the watched machine cooperating.
 check_url() {  # check_url NAME VALUE
+    local bad
     case "$2" in
         http://*|https://*) ;;
         *) die "$1 must start with http:// or https:// - got '$2'" ;;
     esac
+
+    # Whitespace and control characters first, and separately, because the test
+    # below cannot see a trailing one: command substitution strips trailing
+    # newlines, so a value ending in one leaves an empty result and reads as
+    # clean. A newline is also the character worth being surest about, being the
+    # one that turns a single argument into two lines of something.
     case "$2" in
-        *[!A-Za-z0-9:/._~?=%@+-]*) die "$1 contains a character that is not allowed here" ;;
+        *[[:space:]]*)  die "$1 contains whitespace" ;;
+        *[[:cntrl:]]*)  die "$1 contains a control character" ;;
     esac
+
+    # Deleting the permitted characters leaves the offending ones, which is what
+    # the operator needs. Naming neither the character nor the setting's value
+    # left "contains a character that is not allowed here" as the whole of the
+    # report, and finding which one meant guessing.
+    #
+    # unreserved, gen-delims, sub-delims and % - everything a URI may contain.
+    # What remains banned is what cannot appear in one anyway: whitespace,
+    # control characters, quotes, backslash, and < > ^ ` { | }.
+    bad=$(printf '%s' "$2" | tr -d "A-Za-z0-9:/?#@[]!\$&'()*+,;=._~%-")
+    [ -z "$bad" ] || die "$1 contains characters that cannot appear in a URL: $bad"
 }
 
 # --- reaching the devbox ----------------------------------------------------

etc/config.example

@@ -99,7 +99,12 @@ ntfy_url = https://ntfy.example.invalid/devbox-alerts
 # itself, which is why the line is here rather than left to be discovered.
 #
 # Anything speaking the same "GET this URL to say you are alive" shape works.
-heartbeat_url = https://kuma.example.invalid/api/push/abcdef
+#
+# Copied whole, including the query Uptime Kuma appends. An example trimmed to
+# the path is shorter and is not what you will paste, and the shorter one hid a
+# bug: the URL check refused "&", so the real thing was rejected while this
+# example passed.
+heartbeat_url = https://kuma.example.invalid/api/push/abcdef?status=up&msg=OK&ping=
 
 # ===========================================================================
 # IF YOU ALERT THROUGH SOMETHING ELSE

test/guard.bats

@@ -634,3 +634,68 @@ keyline() { awk '{print $1, $2}' "$BATS_TEST_TMPDIR/hk_$1.pub"; }
     [ -n "$uid" ]
     grep -q "chown $uid:$uid" "$REPO/README.md"
 }
+# --- what a URL may contain ---------------------------------------------------
+#
+# The rule refused "&", so it refused the push URL Uptime Kuma hands out - the
+# one value heartbeat_url is documented to hold. This project rejected the
+# setting this project recommends.
+#
+# Refusing a legitimate URL is not the safe direction. It costs the heartbeat,
+# which is the one signal a compromised devbox cannot suppress, so an
+# over-strict rule quietly removes the control that does not depend on the
+# watched machine cooperating.
+
+@test "an Uptime Kuma push URL is accepted, ampersands and all" {
+    export DEJIMA_GUARD_HEARTBEAT_URL='https://kuma.example.invalid/api/push/abc?status=up&msg=OK&ping='
+    stub_ssh "FILE /etc/passwd aaaa" 0
+    run_watch check
+    [[ "$output" != *"heartbeat_url contains"* ]]
+}
+
+@test "every character RFC 3986 permits is accepted" {
+    # Written out rather than sampled: the previous list looked complete too.
+    export DEJIMA_GUARD_NTFY_URL="https://h.example.invalid/p?a=1&b=2#f/x_y~z.w+q%20r,s;t=u!v\$w'x(y)z*"
+    stub_ssh "FILE /etc/passwd aaaa" 0
+    run_watch check
+    # Asserted as "not rejected at all" rather than as the absence of one
+    # message. Matching on the new wording passed against the old check, which
+    # rejected this URL saying something else.
+    [[ "$output" != *"ntfy_url"* ]]
+}
+
+@test "a URL with whitespace is still refused" {
+    export DEJIMA_GUARD_HEARTBEAT_URL='https://kuma.example.invalid/api/push/a b'
+    stub_ssh "" 0
+    run_watch check
+    [[ "$output" == *"whitespace"* ]]
+}
+
+@test "a URL with an embedded newline is refused" {
+    # The check that deletes permitted characters cannot see this one:
+    # command substitution strips a trailing newline, so what is left reads as
+    # empty and the value passes. It is also the character worth being surest
+    # about, being the one that turns one argument into two lines of something.
+    export DEJIMA_GUARD_HEARTBEAT_URL="$(printf 'https://kuma.example.invalid/a\nb')"
+    stub_ssh "" 0
+    run_watch check
+    [[ "$output" == *"whitespace"* ]] || [[ "$output" == *"control character"* ]]
+}
+
+@test "a refused URL names the character, not just the setting" {
+    # "contains a character that is not allowed here" was the whole report.
+    # Finding which one meant guessing, on a value that is usually long.
+    export DEJIMA_GUARD_HEARTBEAT_URL='https://kuma.example.invalid/a|b'
+    stub_ssh "" 0
+    run_watch check
+    [[ "$output" == *"|"* ]]
+}
+
+@test "the config example's own heartbeat_url would be accepted" {
+    # A shipped example the shipped check refuses is the failure this had.
+    url=$(sed -n 's/^[[:space:]]*heartbeat_url[[:space:]]*=[[:space:]]*//p' "$REPO/etc/config.example" | tail -1)
+    [ -n "$url" ]
+    export DEJIMA_GUARD_HEARTBEAT_URL="$url"
+    stub_ssh "FILE /etc/passwd aaaa" 0
+    run_watch check
+    [[ "$output" != *"heartbeat_url"* ]]
+}