[Fix] PR #478 latest Copilot review: _json_escape_string FreeBSD + multi-newline; post_scan_hook_timeout doc; deferred-item rationale comments

[Fix] _json_escape_string: replace sed pipeline with bash parameter expansion.
      Two defects fixed in one rewrite. (1) The quoted "$sed" invocation
      tried to execve a binary literally named "sed -E" on FreeBSD, where
      internals.conf sets sed="$sed -E", silently breaking post_scan_hook
      JSON payload construction. (2) The N;s/\n/\\n/;P;D sed cycle only
      escaped the first newline in multi-line input, leaving subsequent
      LF bytes raw in JSON output for paths with two or more embedded
      newlines. The bash parameter expansion form is portable, correct
      for any number of newlines, and removes an external process from
      the hook hot path. Backslash is escaped first so subsequent escape
      sequences do not get re-escaped.
[New] tests: J-01 through J-15 unit coverage for _json_escape_string
      including regression tests for the multi-newline bug and both a
      runtime (J-14) and structural (J-15) guard against $sed
      reintroduction. Expected values are pre-constructed using a BS
      local to avoid the Rocky 9 bash 5.1 BATS preprocessor gotcha for
      multi-backslash assertion strings.
[Change] conf.maldet / maldet.1: correct post_scan_hook_timeout docs.
      The prior text claimed a 5-second grace period before SIGKILL, but
      _scan_hook_exec_sync/_async invoke timeout(1) without --kill-after,
      so a hook that traps SIGTERM continues to run until it completes.
      Updated both conf.maldet and the man page to match actual behavior.
[Change] lmd_clamav, lmd_config, lmd_init: inline rationale comments for
      the three deferred-or-rejected items from the PR #478 Copilot
      review. lmd_clamav.sh mktemp -d failure is deferred to 2.0.2 with
      blast-radius documented. lmd_config.sh ps-u-iworx grep is a false
      positive because the user-filtered ps excludes the root-owned
      grep child. lmd_init.sh chown user:root is deferred to 2.0.2 with
      FreeBSD wheel-vs-root group divergence documented alongside
      identical class-usage in lmd_quarantine.sh; pr#478

Author: rfxn

CHANGELOG

@@ -124,6 +124,22 @@ v2.0.1 | Mar 25 2026:
 
   -- Bug Fixes --
 
+[Fix] _json_escape_string: replace sed pipeline with bash parameter expansion.
+      Two defects fixed in one rewrite: (1) the quoted "$sed" invocation tried
+      to execve a binary literally named "sed -E" on FreeBSD (where
+      internals.conf sets sed="$sed -E"), silently breaking post_scan_hook
+      JSON payload construction; (2) the N;s/\n/\\n/;P;D sed cycle only
+      escaped the first newline in multi-line input, emitting raw LF bytes
+      in JSON output for paths with two or more embedded newlines. The bash
+      parameter expansion form is portable, correct for any number of
+      newlines, and removes an external process from the hook hot path.
+      BATS coverage added (J-01 through J-15) including a structural guard
+      against sed reintroduction; pr#478
+[Change] conf.maldet / maldet.1: correct post_scan_hook_timeout documentation.
+      The prior text claimed a 5-second grace period before SIGKILL, but the
+      code invokes timeout(1) without --kill-after, so a hook that traps
+      SIGTERM will run indefinitely. Updated to match actual behavior;
+      pr#478
 [Fix] DEB: override_dh_fixperms target restores 640/750 modes after debhelper's
       dh_fixperms normalizes executables to 755 and non-executables to 644.
       Without this, 72 files shipped in the data.tar at the wrong mode including

CHANGELOG.RELEASE

@@ -124,6 +124,22 @@ v2.0.1 | Mar 25 2026:
 
   -- Bug Fixes --
 
+[Fix] _json_escape_string: replace sed pipeline with bash parameter expansion.
+      Two defects fixed in one rewrite: (1) the quoted "$sed" invocation tried
+      to execve a binary literally named "sed -E" on FreeBSD (where
+      internals.conf sets sed="$sed -E"), silently breaking post_scan_hook
+      JSON payload construction; (2) the N;s/\n/\\n/;P;D sed cycle only
+      escaped the first newline in multi-line input, emitting raw LF bytes
+      in JSON output for paths with two or more embedded newlines. The bash
+      parameter expansion form is portable, correct for any number of
+      newlines, and removes an external process from the hook hot path.
+      BATS coverage added (J-01 through J-15) including a structural guard
+      against sed reintroduction; pr#478
+[Change] conf.maldet / maldet.1: correct post_scan_hook_timeout documentation.
+      The prior text claimed a 5-second grace period before SIGKILL, but the
+      code invokes timeout(1) without --kill-after, so a hook that traps
+      SIGTERM will run indefinitely. Updated to match actual behavior;
+      pr#478
 [Fix] DEB: override_dh_fixperms target restores 640/750 modes after debhelper's
       dh_fixperms normalizes executables to 755 and non-executables to 644.
       Without this, 72 files shipped in the data.tar at the wrong mode including

files/conf.maldet

@@ -576,8 +576,9 @@ post_scan_hook_format="args"
 # [ async | sync ]
 post_scan_hook_exec="async"
 
-# Timeout in seconds before the hook is sent SIGTERM.
-# After SIGTERM, a 5-second grace period is given before SIGKILL.
+# Timeout in seconds before the hook is sent SIGTERM via timeout(1).
+# A well-behaved hook will exit on SIGTERM; a hook that traps and ignores
+# SIGTERM will continue to run until it completes (no SIGKILL escalation).
 # Set to 0 to disable timeout (hook runs indefinitely).
 # Minimum effective value is 5 (values 1-4 are clamped to 5).
 # [ 0 = disabled, default = 60 ]

files/internals/lmd_clamav.sh

@@ -72,6 +72,13 @@ clamav_linksigs() {
 	local _log_ctx="${2:-sigup}"  # caller passes "scan" or "sigup"
 	if [ -d "$cpath" ]; then
 		local _staging
+		# NOTE (PR#478 copilot review, deferred to 2.0.2): if mktemp fails
+		# here, _staging is empty and the subsequent "cp -f ... $_staging/"
+		# resolves to "cp -f ... /", writing signature files into the
+		# filesystem root. The risk is bounded because $tmpdir is created
+		# and validated in _init_user_paths() during prerun, so a failure
+		# here only occurs if the tmpfs fills up between init and sigup.
+		# The correct fix is to gate the cp loop on mktemp success.
 		_staging=$(mktemp -d "$tmpdir/.clamval.XXXXXX")
 		# Stage sig files into temp dir (same conditional logic as before)
 		# HEX (.ndb) and YARA always seeded regardless of hash mode

files/internals/lmd_config.sh

@@ -18,6 +18,12 @@ LMD_CONFIG_VERSION="1.0.0"
 
 detect_control_panel() {
 	if [[ -d /usr/local/interworx ]]; then
+		# NOTE (PR#478 copilot review): these "ps | grep" patterns do not
+		# self-match. ps is filtered by user (-u iworx, -u iworx-web) and
+		# the grep child of maldet runs as root, so the grep process is
+		# not in the user-scoped ps output. No false positives possible
+		# unless maldet itself is invoked as the iworx user, which is not
+		# a supported invocation path.
 		iworx_db_ps=$(ps -u iworx | grep iworx-db)
 		iworx_web_ps=$(ps -u iworx-web | grep iworx-web)
 		pex_script=$(readlink -e /usr/local/interworx/bin/listaccounts.pex)

files/internals/lmd_hook.sh

@@ -20,20 +20,23 @@ LMD_HOOK_VERSION="1.0.0"
 # _json_escape_string(string)
 #   Escape a string for safe embedding inside a JSON double-quoted value.
 #   Handles: backslash, double-quote, tab, carriage return, and embedded
-#   newlines. Uses sed for portability. Outputs escaped string to stdout.
+#   newlines. Uses bash parameter expansion (no external process) so the
+#   implementation is portable across Linux and FreeBSD without depending
+#   on $sed, which is set to "sed -E" on FreeBSD and cannot be quoted as a
+#   single executable path. Also escapes every embedded newline (the prior
+#   sed N;P;D cycle only caught the first newline in multi-line input).
+#   Backslash must be escaped FIRST so the subsequent escape sequences
+#   (\t, \r, \n, \") do not get their own backslashes re-escaped.
+#   Outputs escaped string to stdout.
 # ---------------------------------------------------------------------------
 _json_escape_string() {
 	local _in="$1"
-	# shellcheck disable=SC2154
-	# $sed is discovered via command -v in internals.conf (globals from sourced config)
-	printf '%s' "$_in" \
-		| "$sed" \
-			-e 's/\\/\\\\/g' \
-			-e 's/"/\\"/g' \
-			-e 's/	/\\t/g' \
-			-e 's/\r/\\r/g' \
-			-e 'N;s/\n/\\n/;P;D' \
-			2>/dev/null # sed pipeline on trusted internal data; error means sed absent (FreeBSD alt path)
+	_in="${_in//\\/\\\\}"
+	_in="${_in//\"/\\\"}"
+	_in="${_in//$'\t'/\\t}"
+	_in="${_in//$'\r'/\\r}"
+	_in="${_in//$'\n'/\\n}"
+	printf '%s' "$_in"
 }
 
 # ---------------------------------------------------------------------------

files/internals/lmd_init.sh

@@ -165,6 +165,14 @@ prerun() {
 		mkdir -p "$quardir" "$sessdir" "$tmpdir" 2> /dev/null
 		chmod 711 "$userbasedir" 2> /dev/null
 		touch "$maldet_log" 2> /dev/null
+		# NOTE (PR#478 copilot review, deferred to 2.0.2): the literal
+		# "root" group name is correct on Linux but not on FreeBSD,
+		# where the root user's primary group is "wheel". On FreeBSD
+		# this chown fails silently (stderr is discarded) and the
+		# public-scan paths retain their default touch/mkdir group.
+		# lmd_quarantine.sh has the same class of "chown root:root"
+		# usage. Proper fix: derive the group once in internals.conf
+		# via os_freebsd and use a variable in both files.
 		chown -R "${user}:root" "$userbasedir/$user" 2> /dev/null
 		chmod 750 "$userbasedir/$user" "$quardir" "$sessdir" "$tmpdir" 2> /dev/null
 		chmod 640 "$maldet_log" 2> /dev/null

files/maldet.1

@@ -726,8 +726,11 @@ Monitor mode always forces async regardless of this setting.
 Default: async.
 .PP
 .B post_scan_hook_timeout
-sets the timeout in seconds before SIGTERM, followed by SIGKILL after
-a 5\-second grace period.
+sets the timeout in seconds before the hook is sent SIGTERM via
+.BR timeout (1).
+A well\-behaved hook will exit on SIGTERM; a hook that traps and
+ignores SIGTERM will continue to run until it completes
+(no SIGKILL escalation).
 Set to 0 to disable.
 Values 1\-4 are clamped to 5.
 Default: 60.

tests/46-post-scan-hook.bats

@@ -639,3 +639,184 @@ WWEOF
     [ "$status" -ne 1 ]
     [ -f "$HOOK_MARKER" ]
 }
+
+# ---------------------------------------------------------------------------
+# J-01 to J-14: _json_escape_string unit tests
+# Regression coverage for the bash-param-expansion rewrite (replaces the
+# sed pipeline that broke on FreeBSD ($sed set to "sed -E") and only
+# escaped the first newline in multi-line input).
+# ---------------------------------------------------------------------------
+
+# J-* expected values are pre-constructed outside the assertion lines to
+# avoid the Rocky 9 bash 5.1 BATS preprocessor corruption described in
+# CLAUDE.md (multi-backslash sequences in [ ... ] lines can be rewritten).
+# BS and DQ are the single-character literals that appear in the outputs.
+
+@test "J-01: _json_escape_string: plain ASCII pass-through" {
+    _source_lmd_stack
+    run _json_escape_string "hello world"
+    [ "$status" -eq 0 ]
+    [ "$output" = "hello world" ]
+}
+
+@test "J-02: _json_escape_string: empty input produces empty output" {
+    _source_lmd_stack
+    run _json_escape_string ""
+    [ "$status" -eq 0 ]
+    [ -z "$output" ]
+}
+
+@test "J-03: _json_escape_string: double-quote is escaped" {
+    _source_lmd_stack
+    local BS=$'\x5c' expected
+    # Expected: foo + BS + " + bar
+    expected="foo${BS}\"bar"
+    run _json_escape_string 'foo"bar'
+    [ "$status" -eq 0 ]
+    [ "$output" = "$expected" ]
+}
+
+@test "J-04: _json_escape_string: single backslash is doubled" {
+    _source_lmd_stack
+    local BS=$'\x5c' expected
+    # Expected: a + BS + BS + b
+    expected="a${BS}${BS}b"
+    run _json_escape_string "a${BS}b"
+    [ "$status" -eq 0 ]
+    [ "$output" = "$expected" ]
+}
+
+@test "J-05: _json_escape_string: backslash-first ordering preserves quote escape" {
+    _source_lmd_stack
+    local BS=$'\x5c' input expected
+    # Input: a + BS + " + b (4 chars)
+    input="a${BS}\"b"
+    # Expected: a + BS + BS + BS + " + b (6 chars)
+    # Backslash must be escaped first, otherwise the newly-inserted BS + "
+    # from step 2 would get re-escaped by step 1 into BS + BS + ".
+    expected="a${BS}${BS}${BS}\"b"
+    run _json_escape_string "$input"
+    [ "$status" -eq 0 ]
+    [ "$output" = "$expected" ]
+}
+
+@test "J-06: _json_escape_string: tab becomes literal backslash-t" {
+    _source_lmd_stack
+    local BS=$'\x5c' expected
+    expected="a${BS}tb"
+    run _json_escape_string "$(printf 'a\tb')"
+    [ "$status" -eq 0 ]
+    [ "$output" = "$expected" ]
+}
+
+@test "J-07: _json_escape_string: CR becomes literal backslash-r" {
+    _source_lmd_stack
+    local BS=$'\x5c' expected
+    expected="a${BS}rb"
+    run _json_escape_string "$(printf 'a\rb')"
+    [ "$status" -eq 0 ]
+    [ "$output" = "$expected" ]
+}
+
+@test "J-08: _json_escape_string: single LF becomes literal backslash-n" {
+    _source_lmd_stack
+    local BS=$'\x5c' expected
+    expected="a${BS}nb"
+    run _json_escape_string "$(printf 'a\nb')"
+    [ "$status" -eq 0 ]
+    [ "$output" = "$expected" ]
+}
+
+@test "J-09: _json_escape_string: multiple newlines all escaped (regression)" {
+    _source_lmd_stack
+    local BS=$'\x5c' expected
+    # Regression: old sed pipeline (N;s/\n/\\n/;P;D) only escaped the
+    # first newline, leaving subsequent LF bytes raw in JSON output.
+    expected="a${BS}nb${BS}nc${BS}nd"
+    run _json_escape_string "$(printf 'a\nb\nc\nd')"
+    [ "$status" -eq 0 ]
+    [ "$output" = "$expected" ]
+}
+
+@test "J-10: _json_escape_string: five newlines all escaped" {
+    _source_lmd_stack
+    local BS=$'\x5c' expected
+    expected="1${BS}n2${BS}n3${BS}n4${BS}n5${BS}n6"
+    run _json_escape_string "$(printf '1\n2\n3\n4\n5\n6')"
+    [ "$status" -eq 0 ]
+    [ "$output" = "$expected" ]
+}
+
+@test "J-11: _json_escape_string: CRLF escapes both CR and LF" {
+    _source_lmd_stack
+    local BS=$'\x5c' expected
+    expected="a${BS}r${BS}nb"
+    run _json_escape_string "$(printf 'a\r\nb')"
+    [ "$status" -eq 0 ]
+    [ "$output" = "$expected" ]
+}
+
+@test "J-12: _json_escape_string: realistic path with spaces and quotes" {
+    _source_lmd_stack
+    local BS=$'\x5c' expected
+    # /tmp/my \"odd\" folder/file.txt
+    expected="/tmp/my ${BS}\"odd${BS}\" folder/file.txt"
+    run _json_escape_string '/tmp/my "odd" folder/file.txt'
+    [ "$status" -eq 0 ]
+    [ "$output" = "$expected" ]
+}
+
+@test "J-13: _json_escape_string: all handled specials in one input" {
+    _source_lmd_stack
+    local BS=$'\x5c' TAB=$'\t' CR=$'\r' LF=$'\n' input expected
+    # NOTE: must use $'\n' for LF (command substitution strips trailing
+    # newlines, so $(printf '\n') gives an empty string).
+    # Input raw chars: a, BS, b, ", c, TAB, d, CR, e, LF, f  (11 chars)
+    input="a${BS}b\"c${TAB}d${CR}e${LF}f"
+    # Expected: a + BS + BS + b + BS + " + c + BS + t + d + BS + r + e + BS + n + f
+    expected="a${BS}${BS}b${BS}\"c${BS}td${BS}re${BS}nf"
+    run _json_escape_string "$input"
+    [ "$status" -eq 0 ]
+    [ "$output" = "$expected" ]
+}
+
+@test "J-14: _json_escape_string: does not require \$sed (FreeBSD regression)" {
+    _source_lmd_stack
+    local BS=$'\x5c' input expected result
+    input="hello/world with \"quote\" and ${BS}backslash"
+    expected="hello/world with ${BS}\"quote${BS}\" and ${BS}${BS}backslash"
+    # On FreeBSD, internals.conf sets sed to include "-E", so any future
+    # reintroduction of a quoted "\$sed" invocation would try to execve
+    # a binary literally named "sed -E" and fail. Scope the override in
+    # a subshell so the fake value does not leak into later tests, and
+    # pick a deliberately nonexistent path so any external-sed attempt
+    # would fail loudly rather than accidentally picking up /bin/sed.
+    result=$(
+        sed="/nonexistent/regression-sed-xyz"
+        _json_escape_string "$input"
+    )
+    [ "$result" = "$expected" ]
+}
+
+@test "J-15: _json_escape_string: function body does not reference sed" {
+    # Structural guard: the bash-parameter-expansion rewrite contains zero
+    # legitimate uses of the word "sed" in the function body, so any future
+    # reintroduction (quoted "$sed", unquoted $sed, "| sed", $($sed ...),
+    # eval "... sed ...", etc.) will contain the literal word "sed" and
+    # trip this guard. Simple word-boundary match is intentionally strict.
+    local hook_src="$LMD_INSTALL/internals/lmd_hook.sh"
+    [ -f "$hook_src" ]
+    # Extract body of _json_escape_string (from opening { to closing })
+    local body
+    body=$(awk '
+        /^_json_escape_string\(\) *\{/ { in_fn=1; next }
+        in_fn && /^\}/ { in_fn=0; exit }
+        in_fn { print }
+    ' "$hook_src")
+    [ -n "$body" ]
+    if printf '%s' "$body" | grep -qE '\bsed\b'; then
+        echo "FAIL: _json_escape_string body references sed:"
+        echo "$body"
+        return 1
+    fi
+}