Skip to content

Commit 9b0678f

Browse files
benvinegarbenvinegar
committed
refactor: shrink doctor/audit checks via shared node contracts
1 parent e004988 commit 9b0678f

8 files changed

Lines changed: 276 additions & 57 deletions

‎bin/checks/slack-allowed-users.mjs‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
#!/usr/bin/env node
2+
3+
import fs from "node:fs";
4+
5+
const envPath = process.argv[2] || "";
6+
7+
function countUsers(rawValue) {
8+
if (!rawValue) return 0;
9+
return rawValue
10+
.split(",")
11+
.filter((entry) => entry.length > 0).length;
12+
}
13+
14+
if (!envPath) {
15+
process.stdout.write(
16+
JSON.stringify({
17+
ok: "0",
18+
exists: "0",
19+
defined: "0",
20+
raw_non_empty: "0",
21+
count: "0",
22+
error: "missing_path_argument",
23+
}),
24+
);
25+
process.exit(0);
26+
}
27+
28+
if (!fs.existsSync(envPath)) {
29+
process.stdout.write(
30+
JSON.stringify({
31+
ok: "1",
32+
exists: "0",
33+
defined: "0",
34+
raw_non_empty: "0",
35+
count: "0",
36+
error: "",
37+
}),
38+
);
39+
process.exit(0);
40+
}
41+
42+
try {
43+
const lines = fs.readFileSync(envPath, "utf8").split(/\r?\n/);
44+
let value = "";
45+
let defined = false;
46+
47+
for (const line of lines) {
48+
if (!line.startsWith("SLACK_ALLOWED_USERS=")) continue;
49+
value = line.slice("SLACK_ALLOWED_USERS=".length);
50+
defined = true;
51+
}
52+
53+
const count = defined ? countUsers(value) : 0;
54+
55+
process.stdout.write(
56+
JSON.stringify({
57+
ok: "1",
58+
exists: "1",
59+
defined: defined ? "1" : "0",
60+
raw_non_empty: defined && value.length > 0 ? "1" : "0",
61+
count: String(count),
62+
error: "",
63+
}),
64+
);
65+
} catch {
66+
process.stdout.write(
67+
JSON.stringify({
68+
ok: "0",
69+
exists: "1",
70+
defined: "0",
71+
raw_non_empty: "0",
72+
count: "0",
73+
error: "parse_error",
74+
}),
75+
);
76+
}

‎bin/doctor.sh‎

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ source "$SCRIPT_DIR/lib/shell-common.sh"
1111
source "$SCRIPT_DIR/lib/paths-common.sh"
1212
# shellcheck source=bin/lib/doctor-common.sh
1313
source "$SCRIPT_DIR/lib/doctor-common.sh"
14+
# shellcheck source=bin/lib/json-common.sh
15+
source "$SCRIPT_DIR/lib/json-common.sh"
1416
# shellcheck source=bin/lib/runtime-node.sh
1517
source "$SCRIPT_DIR/lib/runtime-node.sh"
1618
bb_enable_strict_mode
@@ -64,6 +66,8 @@ else
6466
fail "Node.js not found (expected: $NODE_BIN)"
6567
fi
6668

69+
CHECK_NODE_BIN="$(bb_pick_node_bin "${NODE_BIN:-}" || true)"
70+
6771
PI_BIN="$(bb_resolve_runtime_node_bin_dir "$BAUDBOT_HOME")/pi"
6872
if [ -x "$PI_BIN" ] || [ -L "$PI_BIN" ]; then
6973
pass "pi is installed"
@@ -240,7 +244,10 @@ if [ -f "$ENV_FILE" ]; then
240244
fi
241245
fi
242246

243-
if grep -q '^SLACK_ALLOWED_USERS=.\+' "$ENV_FILE" 2>/dev/null; then
247+
SLACK_ALLOWED_USERS_CHECK_SCRIPT="$BAUDBOT_ROOT/bin/checks/slack-allowed-users.mjs"
248+
slack_allowed_users_payload="$(bb_run_node_check_payload "$CHECK_NODE_BIN" "$SLACK_ALLOWED_USERS_CHECK_SCRIPT" "$ENV_FILE")"
249+
slack_allowed_users_non_empty="$(bb_json_field_or_default "$slack_allowed_users_payload" "raw_non_empty" "0")"
250+
if [ "$slack_allowed_users_non_empty" = "1" ]; then
244251
pass "SLACK_ALLOWED_USERS is set"
245252
else
246253
warn "SLACK_ALLOWED_USERS is not set (all workspace members allowed)"
@@ -302,31 +309,14 @@ fi
302309

303310
INTEGRITY_STATUS_FILE="$BAUDBOT_INTEGRITY_STATUS_FILE"
304311
INTEGRITY_CHECK_SCRIPT="$BAUDBOT_ROOT/bin/checks/integrity-status.mjs"
305-
INTEGRITY_CHECK_NODE_BIN=""
306-
if [ -n "${NODE_BIN:-}" ] && [ -x "${NODE_BIN:-}" ]; then
307-
INTEGRITY_CHECK_NODE_BIN="$NODE_BIN"
308-
elif command -v node >/dev/null 2>&1; then
309-
INTEGRITY_CHECK_NODE_BIN="$(command -v node)"
310-
fi
311-
312-
if [ -n "$INTEGRITY_CHECK_NODE_BIN" ] && [ -f "$INTEGRITY_CHECK_SCRIPT" ]; then
313-
integrity_payload="$($INTEGRITY_CHECK_NODE_BIN "$INTEGRITY_CHECK_SCRIPT" "$INTEGRITY_STATUS_FILE" 2>/dev/null || true)"
314-
else
315-
integrity_payload=""
316-
fi
312+
integrity_payload="$(bb_run_node_check_payload "$CHECK_NODE_BIN" "$INTEGRITY_CHECK_SCRIPT" "$INTEGRITY_STATUS_FILE")"
317313

318314
if [ -n "$integrity_payload" ]; then
319-
integrity_exists="$(printf '%s' "$integrity_payload" | json_get_string_stdin "exists" 2>/dev/null || true)"
320-
integrity_status="$(printf '%s' "$integrity_payload" | json_get_string_stdin "status" 2>/dev/null || true)"
321-
integrity_checked_at="$(printf '%s' "$integrity_payload" | json_get_string_stdin "checked_at" 2>/dev/null || true)"
322-
integrity_missing="$(printf '%s' "$integrity_payload" | json_get_string_stdin "missing_files" 2>/dev/null || true)"
323-
integrity_mismatches="$(printf '%s' "$integrity_payload" | json_get_string_stdin "hash_mismatches" 2>/dev/null || true)"
324-
325-
[ -n "$integrity_exists" ] || integrity_exists="0"
326-
[ -n "$integrity_status" ] || integrity_status="unknown"
327-
[ -n "$integrity_checked_at" ] || integrity_checked_at="unknown"
328-
[ -n "$integrity_missing" ] || integrity_missing="0"
329-
[ -n "$integrity_mismatches" ] || integrity_mismatches="0"
315+
integrity_exists="$(bb_json_field_or_default "$integrity_payload" "exists" "0")"
316+
integrity_status="$(bb_json_field_or_default "$integrity_payload" "status" "unknown")"
317+
integrity_checked_at="$(bb_json_field_or_default "$integrity_payload" "checked_at" "unknown")"
318+
integrity_missing="$(bb_json_field_or_default "$integrity_payload" "missing_files" "0")"
319+
integrity_mismatches="$(bb_json_field_or_default "$integrity_payload" "hash_mismatches" "0")"
330320

331321
if [ "$integrity_exists" != "1" ]; then
332322
if [ "$IS_ROOT" -ne 1 ] && [ -d "$BAUDBOT_HOME/.pi/agent" ]; then

‎bin/lib/check-report-common.sh‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,54 @@ bb_summary_print_item() {
2727

2828
printf " %s %-9s %s\n" "$icon" "$label:" "$value"
2929
}
30+
31+
bb_json_field_or_default() {
32+
local payload="$1"
33+
local key="$2"
34+
local fallback="$3"
35+
local value=""
36+
37+
if [ -z "$payload" ]; then
38+
echo "$fallback"
39+
return 0
40+
fi
41+
42+
value="$(printf '%s' "$payload" | json_get_string_stdin "$key" 2>/dev/null || true)"
43+
if [ -n "$value" ]; then
44+
echo "$value"
45+
else
46+
echo "$fallback"
47+
fi
48+
}
49+
50+
bb_pick_node_bin() {
51+
local preferred_bin="${1:-}"
52+
53+
if [ -n "$preferred_bin" ] && [ -x "$preferred_bin" ]; then
54+
echo "$preferred_bin"
55+
return 0
56+
fi
57+
58+
if command -v node >/dev/null 2>&1; then
59+
command -v node
60+
return 0
61+
fi
62+
63+
return 1
64+
}
65+
66+
bb_run_node_check_payload() {
67+
local node_bin="${1:-}"
68+
local script_path="${2:-}"
69+
shift 2 || true
70+
71+
if [ -z "$node_bin" ] || [ ! -x "$node_bin" ]; then
72+
return 0
73+
fi
74+
75+
if [ -z "$script_path" ] || [ ! -f "$script_path" ]; then
76+
return 0
77+
fi
78+
79+
"$node_bin" "$script_path" "$@" 2>/dev/null || true
80+
}

‎bin/lib/check-report-common.test.sh‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
set -euo pipefail
55

66
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
7+
# shellcheck source=bin/lib/json-common.sh
8+
source "$SCRIPT_DIR/json-common.sh"
79
# shellcheck source=bin/lib/check-report-common.sh
810
source "$SCRIPT_DIR/check-report-common.sh"
911

@@ -71,12 +73,57 @@ test_summary_helpers_render_rows() {
7173
)
7274
}
7375

76+
test_json_field_or_default_reads_existing_key() {
77+
(
78+
set -euo pipefail
79+
payload='{"status":"pass"}'
80+
[ "$(bb_json_field_or_default "$payload" status unknown)" = "pass" ]
81+
)
82+
}
83+
84+
test_json_field_or_default_falls_back_when_missing() {
85+
(
86+
set -euo pipefail
87+
payload='{"exists":"1"}'
88+
[ "$(bb_json_field_or_default "$payload" status unknown)" = "unknown" ]
89+
)
90+
}
91+
92+
test_pick_node_bin_uses_preferred() {
93+
(
94+
set -euo pipefail
95+
fake_node="$(command -v node)"
96+
[ -n "$(bb_pick_node_bin "$fake_node")" ]
97+
)
98+
}
99+
100+
test_run_node_check_payload_executes_script() {
101+
(
102+
set -euo pipefail
103+
tmp_script="$(mktemp /tmp/check-report-node-check.XXXXXX.mjs)"
104+
trap 'rm -f "$tmp_script"' EXIT
105+
106+
cat >"$tmp_script" <<'EOF'
107+
#!/usr/bin/env node
108+
process.stdout.write(JSON.stringify({ ok: "1" }));
109+
EOF
110+
chmod +x "$tmp_script"
111+
112+
output="$(bb_run_node_check_payload "$(command -v node)" "$tmp_script")"
113+
[ "$(bb_json_field_or_default "$output" ok 0)" = "1" ]
114+
)
115+
}
116+
74117
echo "=== check-report-common tests ==="
75118
echo ""
76119

77120
run_test "reset many sets counters to zero" test_reset_many_sets_zero
78121
run_test "increment updates named counter" test_inc_increments_named_counter
79122
run_test "summary helpers render rows" test_summary_helpers_render_rows
123+
run_test "json field helper reads key" test_json_field_or_default_reads_existing_key
124+
run_test "json field helper falls back when missing" test_json_field_or_default_falls_back_when_missing
125+
run_test "node picker prefers runtime path" test_pick_node_bin_uses_preferred
126+
run_test "node check runner executes script" test_run_node_check_payload_executes_script
80127

81128
echo ""
82129
echo "=== $PASSED/$TOTAL passed, $FAILED failed ==="

‎bin/security-audit.sh‎

Lines changed: 23 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -289,30 +289,16 @@ else
289289
"Run deploy.sh to generate"
290290
fi
291291

292-
INTEGRITY_CHECK_SCRIPT="$BAUDBOT_SRC/bin/checks/integrity-status.mjs"
293-
INTEGRITY_CHECK_NODE_BIN="$(bb_resolve_runtime_node_bin "$BAUDBOT_HOME" 2>/dev/null || true)"
294-
if [ -z "$INTEGRITY_CHECK_NODE_BIN" ] && command -v node >/dev/null 2>&1; then
295-
INTEGRITY_CHECK_NODE_BIN="$(command -v node)"
296-
fi
297-
298-
if [ -n "$INTEGRITY_CHECK_NODE_BIN" ] && [ -f "$INTEGRITY_CHECK_SCRIPT" ]; then
299-
integrity_payload="$($INTEGRITY_CHECK_NODE_BIN "$INTEGRITY_CHECK_SCRIPT" "$BAUDBOT_INTEGRITY_STATUS_FILE" 2>/dev/null || true)"
300-
else
301-
integrity_payload=""
302-
fi
292+
CHECK_NODE_BIN="$(bb_pick_node_bin "$(bb_resolve_runtime_node_bin "$BAUDBOT_HOME" 2>/dev/null || true)" || true)"
293+
INTEGRITY_CHECK_SCRIPT="$SCRIPT_DIR/checks/integrity-status.mjs"
294+
integrity_payload="$(bb_run_node_check_payload "$CHECK_NODE_BIN" "$INTEGRITY_CHECK_SCRIPT" "$BAUDBOT_INTEGRITY_STATUS_FILE")"
303295

304296
if [ -n "$integrity_payload" ]; then
305-
status_exists="$(printf '%s' "$integrity_payload" | json_get_string_stdin "exists" 2>/dev/null || true)"
306-
status_value="$(printf '%s' "$integrity_payload" | json_get_string_stdin "status" 2>/dev/null || true)"
307-
status_checked_at="$(printf '%s' "$integrity_payload" | json_get_string_stdin "checked_at" 2>/dev/null || true)"
308-
status_missing="$(printf '%s' "$integrity_payload" | json_get_string_stdin "missing_files" 2>/dev/null || true)"
309-
status_mismatches="$(printf '%s' "$integrity_payload" | json_get_string_stdin "hash_mismatches" 2>/dev/null || true)"
310-
311-
[ -n "$status_exists" ] || status_exists="0"
312-
[ -n "$status_value" ] || status_value="unknown"
313-
[ -n "$status_checked_at" ] || status_checked_at="unknown"
314-
[ -n "$status_missing" ] || status_missing="0"
315-
[ -n "$status_mismatches" ] || status_mismatches="0"
297+
status_exists="$(bb_json_field_or_default "$integrity_payload" "exists" "0")"
298+
status_value="$(bb_json_field_or_default "$integrity_payload" "status" "unknown")"
299+
status_checked_at="$(bb_json_field_or_default "$integrity_payload" "checked_at" "unknown")"
300+
status_missing="$(bb_json_field_or_default "$integrity_payload" "missing_files" "0")"
301+
status_mismatches="$(bb_json_field_or_default "$integrity_payload" "hash_mismatches" "0")"
316302

317303
if [ "$status_exists" != "1" ]; then
318304
finding "WARN" "No startup integrity status found" \
@@ -731,18 +717,23 @@ echo ""
731717
echo "Bridge Configuration"
732718

733719
# Check SLACK_ALLOWED_USERS mode (without reading the actual value)
734-
if [ -f "$BAUDBOT_HOME/.config/.env" ]; then
735-
if grep -q '^SLACK_ALLOWED_USERS=' "$BAUDBOT_HOME/.config/.env" 2>/dev/null; then
736-
allowed_count=$(grep '^SLACK_ALLOWED_USERS=' "$BAUDBOT_HOME/.config/.env" 2>/dev/null | cut -d= -f2 | tr ',' '\n' | grep -c . || echo 0)
737-
if [ "$allowed_count" -gt 0 ]; then
738-
ok "SLACK_ALLOWED_USERS configured ($allowed_count user(s))"
739-
else
740-
finding "WARN" "SLACK_ALLOWED_USERS is empty" \
741-
"Bridge will allow all workspace members"
742-
fi
743-
else
720+
SLACK_ALLOWED_USERS_CHECK_SCRIPT="$SCRIPT_DIR/checks/slack-allowed-users.mjs"
721+
SLACK_ALLOWED_USERS_ENV_FILE="$BAUDBOT_HOME/.config/.env"
722+
slack_allowed_users_payload="$(bb_run_node_check_payload "$CHECK_NODE_BIN" "$SLACK_ALLOWED_USERS_CHECK_SCRIPT" "$SLACK_ALLOWED_USERS_ENV_FILE")"
723+
724+
slack_allowed_users_exists="$(bb_json_field_or_default "$slack_allowed_users_payload" "exists" "0")"
725+
slack_allowed_users_defined="$(bb_json_field_or_default "$slack_allowed_users_payload" "defined" "0")"
726+
slack_allowed_users_count="$(bb_json_field_or_default "$slack_allowed_users_payload" "count" "0")"
727+
728+
if [ "$slack_allowed_users_exists" = "1" ]; then
729+
if [ "$slack_allowed_users_defined" != "1" ]; then
744730
finding "WARN" "SLACK_ALLOWED_USERS not set in .env" \
745731
"Bridge will allow all workspace members"
732+
elif [ "$slack_allowed_users_count" -gt 0 ]; then
733+
ok "SLACK_ALLOWED_USERS configured ($slack_allowed_users_count user(s))"
734+
else
735+
finding "WARN" "SLACK_ALLOWED_USERS is empty" \
736+
"Bridge will allow all workspace members"
746737
fi
747738
fi
748739
echo ""

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"private": true,
55
"scripts": {
66
"test": "vitest run --config vitest.config.mjs",
7-
"test:js": "vitest run --config vitest.config.mjs pi/extensions/heartbeat.test.mjs pi/extensions/memory.test.mjs test/legacy-node-tests.test.mjs test/broker-bridge.integration.test.mjs test/integrity-status-check.test.mjs",
7+
"test:js": "vitest run --config vitest.config.mjs pi/extensions/heartbeat.test.mjs pi/extensions/memory.test.mjs test/legacy-node-tests.test.mjs test/broker-bridge.integration.test.mjs test/integrity-status-check.test.mjs test/slack-allowed-users-check.test.mjs",
88
"test:shell": "vitest run --config vitest.config.mjs test/shell-scripts.test.mjs test/security-audit.test.mjs",
99
"test:coverage": "vitest run --config vitest.config.mjs --coverage pi/extensions/heartbeat.test.mjs pi/extensions/memory.test.mjs test/legacy-node-tests.test.mjs",
1010
"lint": "npm run lint:js && npm run lint:shell",

0 commit comments

Comments
 (0)