fix: make hermes dashboard-auth failures honest and propagate them #386
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,9 +44,40 @@ MCP_ENTRY="$PROJECT_DIR/mcp/clawbox-mcp.ts" | |
| MCP_TOKEN_FILE="$PROJECT_DIR/data/.mcp-token" | ||
| EDITION_FILE="${CLAWBOX_EDITION_FILE:-/etc/clawbox/edition.env}" | ||
| API_BASE="${CLAWBOX_API_BASE:-http://127.0.0.1:80}" | ||
| # Shared with setup-hermes-dashboard-auth.sh: BOTH scripts read-modify-write | ||
| # ~/.hermes/config.yaml, and at install time they run seconds apart | ||
| # (production-server.js fire-and-forgets this script on the clawbox-setup | ||
| # restart in step_start_services; setup-hermes-edition.sh runs the auth script | ||
| # right after). Without a shared lock, whichever one snapshotted the file first | ||
| # and wrote last silently erased the other's block — the auth script's dashboard | ||
| # block vanished and its verify failed, blaming credentials that were correct. | ||
| # Same path derivation on both sides (HERMES_CONFIG + ".lock") so they collide. | ||
| CONFIG_LOCK="${HERMES_CONFIG}.lock" | ||
|
Comment on lines
+47
to
+55
There was a problem hiding this comment. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Compare HERMES_CONFIG resolution across the two cooperating writers and the unit that spawns register-mcp.sh.
set -euo pipefail
rg -n -C 3 'HERMES_CONFIG' --glob 'scripts/*.sh' --glob 'install.sh' || true
# How is register-mcp.sh spawned, and with which user/HOME?
rg -n -C 6 'register-mcp' --glob '!**/node_modules/**' || true
# Service user and environment of the units involved.
fd -e service . --exec rg -n -e '^User=' -e '^Environment=' -e '^WorkingDirectory=' {} +Repository: ID-Robots/clawbox Length of output: 155 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- relevant files ---'
git ls-files | rg '(^|/)(register-mcp\.sh|setup-hermes-dashboard-auth\.sh|production-server\.js|install\.sh|.*\.service)$' || true
printf '%s\n' '--- HERMES_CONFIG references ---'
rg -n -C 8 'HERMES_CONFIG|register-mcp|setup-hermes-dashboard-auth' --glob 'scripts/*.sh' --glob 'install.sh' --glob '*.js' --glob '*.service' . || true
printf '%s\n' '--- service identity and environment ---'
fd -e service . --exec sh -c 'printf "\n--- %s ---\n" "$1"; rg -n -e "^(User|Group|Environment|WorkingDirectory|ExecStart)=" "$1" || true' sh {} +Repository: ID-Robots/clawbox Length of output: 49287 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- install-time home and auth invocation ---'
sed -n '1,90p' install.sh
sed -n '3070,3120p' install.sh
printf '%s\n' '--- Hermes edition variables and cooperating invocations ---'
sed -n '1,75p' scripts/setup-hermes-edition.sh
sed -n '155,195p' scripts/setup-hermes-edition.sh
printf '%s\n' '--- setup service environment and install flow references ---'
sed -n '1,45p' config/clawbox-setup.service
rg -n -C 5 'step_start_services|setup-hermes-edition|CLAWBOX_HOME|HOME=' install.sh scripts production-server.js config
printf '%s\n' '--- shell expansion probe for the exact assignments ---'
python3 - <<'PY'
from pathlib import Path
import re
register = Path("scripts/register-mcp.sh").read_text()
auth = Path("scripts/setup-hermes-dashboard-auth.sh").read_text()
service = Path("config/clawbox-setup.service").read_text()
for name, text, patterns in [
("register-mcp.sh", register, [
r'^HOME_DIR=.*$',
r'^HERMES_CONFIG=.*$',
r'^CONFIG_LOCK=.*$',
]),
("setup-hermes-dashboard-auth.sh", auth, [
r'^HERMES_CONFIG=.*$',
r'^CONFIG_LOCK=.*$',
]),
("clawbox-setup.service", service, [
r'^User=.*$',
r'^Environment=HOME=.*$',
r'^ExecStart=.*$',
]),
]:
print(f"--- {name} ---")
for pattern in patterns:
for line in text.splitlines():
if re.match(pattern, line):
print(line)
PYRepository: ID-Robots/clawbox Length of output: 50373 🌐 Web query:
💡 Result: In systemd services, the HOME environment variable is controlled by the SetLoginEnvironment= directive within the unit file's [Service] section [1][2]. Key behaviors regarding the HOME environment variable: 1. Automatic Setting: By default, systemd automatically sets the $HOME, $LOGNAME, and $SHELL environment variables when User=, DynamicUser=, or PAMName= are configured for a system service [1][3][2]. 2. Configuration via SetLoginEnvironment=: You can explicitly control this behavior using the SetLoginEnvironment= boolean option [1][2]: - If set to true, $HOME, $LOGNAME, and $SHELL are set regardless of whether User=, DynamicUser=, or PAMName= are used (this includes the root user) [1][3]. - If set to false, these variables are not set by the service manager, even if User=, DynamicUser=, or PAMName= are specified [1][2]. 3. Per-User Service Manager: The SetLoginEnvironment= option generally has no effect on services run by the per-user service manager (systemd --user), as these variables are typically inherited from the user manager's own environment [1][2]. 4. User Home Directory Path: While systemd can set the $HOME environment variable to the user's home directory (as defined in the system's user database), it does not provide a direct specifier (like %h) that resolves to the home directory of the user specified by User= for use in other configuration lines (such as Environment=) [4]. The %h specifier in system unit files resolves to the home directory of the user running the service manager (typically root), not the user specified in the User= directive [4]. For services needing to store data in a user's home directory, it is often recommended to use systemd's built-in directory management directives—such as StateDirectory=, CacheDirectory=, or ConfigurationDirectory=—which are specifically designed to provide predictable paths for service data [3][5][6]. These directories are automatically created and associated with environment variables (e.g., $STATE_DIRECTORY) that point to the correct locations [3][6]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- environment-file loading and HERMES_CONFIG overrides ---'
rg -n -C 4 'source .*\.env|EnvironmentFile|dotenv|HERMES_CONFIG|HOME=' install.sh production-server.js config/clawbox-setup.service scripts/setup-hermes-edition.sh scripts/setup-hermes-dashboard-auth.sh scripts/register-mcp.sh
printf '%s\n' '--- tracked environment templates ---'
git ls-files | rg '(^|/)(\.env|.*\.env.*|environment.*)$' || true
printf '%s\n' '--- concise source facts ---'
python3 - <<'PY'
from pathlib import Path
for file, ranges in {
"config/clawbox-setup.service": [(1, 45)],
"scripts/register-mcp.sh": [(32, 58)],
"scripts/setup-hermes-edition.sh": [(10, 22), (160, 188)],
"scripts/setup-hermes-dashboard-auth.sh": [(48, 61)],
"install.sh": [(280, 305), (1318, 1331)],
}.items():
print(f"--- {file} ---")
lines = Path(file).read_text().splitlines()
for start, end in ranges:
for i in range(start, min(end, len(lines)) + 1):
print(f"{i}:{lines[i-1]}")
PY
printf '%s\n' '--- deterministic environment model ---'
python3 - <<'PY'
# Model the relevant precedence without executing repository code.
def register_path(env):
home_dir = env.get("HOME", "/home/clawbox")
return env.get("HERMES_CONFIG", f"{home_dir}/.hermes/config.yaml")
def auth_path(env):
return env.get("HERMES_CONFIG", f"{env.get('HOME', '')}/.hermes/config.yaml")
cases = [
("systemd default HOME", {"HOME": "/home/clawbox"}),
("custom HOME from .env", {"HOME": "/tmp/other"}),
("custom HERMES_CONFIG from .env", {
"HOME": "/home/clawbox",
"HERMES_CONFIG": "/tmp/other/config.yaml",
}),
]
for label, env in cases:
print(label)
print(" register:", register_path(env))
print(" auth:", auth_path(env))
print(" same:", register_path(env) == auth_path(env))
PYRepository: ID-Robots/clawbox Length of output: 36254 Set 🤖 Prompt for AI Agents |
||
|
|
||
| log() { echo "[register-mcp] $*"; } | ||
|
|
||
| # Take the exclusive config lock for the rest of the run (fd 9, released on | ||
| # exit). Covers BOTH the PyYAML reconcile below AND the `hermes tools disable` | ||
| # call — the Hermes CLI does its own wide load→save_config on the same file, so | ||
| # it has to be inside the same critical section. Best-effort: proceed without | ||
| # the lock rather than skip registering the device's tools if flock is missing. | ||
| acquire_config_lock() { | ||
| command -v flock >/dev/null 2>&1 || { | ||
| log "flock unavailable — proceeding without the config lock" | ||
| return 0 | ||
| } | ||
| mkdir -p "$(dirname "$CONFIG_LOCK")" 2>/dev/null || true | ||
| # Probe writability in a scoped subshell before opening fd 9; keep the `exec` | ||
| # redirect CLEAN (a `2>/dev/null` on it would silence the whole script, | ||
| # because redirections on exec are permanent). | ||
| if ! ( : > "$CONFIG_LOCK" ) 2>/dev/null; then | ||
| log "could not create $CONFIG_LOCK — proceeding without the config lock" | ||
| return 0 | ||
| fi | ||
| exec 9>"$CONFIG_LOCK" | ||
| flock -w 120 9 || log "could not acquire $CONFIG_LOCK within 120s — proceeding without it" | ||
| } | ||
|
|
||
| # ── 1. Which edition is this? ─────────────────────────────────────────────── | ||
| # Root-owned lock first, environment second, "openclaw" last — the same order | ||
| # and the same reasons as src/lib/edition-source.ts. Reading the file rather | ||
|
|
@@ -112,6 +143,11 @@ chmod 600 "$MCP_TOKEN_FILE" 2>/dev/null || true | |
| # NOT via `hermes mcp add`: that command performs live tool discovery and | ||
| # rewrites the whole config through Hermes' own save_config(), which is a much | ||
| # wider blast radius for a boot-time provisioning step, and it is slow. | ||
| # | ||
| # Everything from here to the end of the script touches config.yaml, so take the | ||
| # shared lock now and hold it until exit. | ||
| acquire_config_lock | ||
|
|
||
| export CLAWBOX_MCP_HERMES_CONFIG="$HERMES_CONFIG" | ||
| export CLAWBOX_MCP_BUN_BIN="$BUN_BIN" | ||
| export CLAWBOX_MCP_ENTRY="$MCP_ENTRY" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
A failed marker write leaves a stale
STATUS=okthat contradicts the exit code.mkdir -p, the redirect, andchmodall end in|| true. If the redirect fails, for example on a read-only/etcor without write permission, the previous file content survives unchanged. A device that installed successfully once and then fails keepsSTATUS=okon disk whileexit 1and the[provision-status] INCOMPLETEsentinel report the failure. The three signals must agree, as stated on Lines 3383-3385.Report the write failure so the operator sees that the marker is unreliable.
🐛 Proposed fix
{ echo "# Written by install.sh at the end of a full install. Machine-readable." echo "STATUS=$status" echo "FAILED_STEPS=$*" echo "TIMESTAMP=$(date -u +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || true)" - } > "$PROVISION_STATUS_FILE" 2>/dev/null || true + } > "$PROVISION_STATUS_FILE" 2>/dev/null || { + echo " Warning: could not write $PROVISION_STATUS_FILE — any existing marker there is STALE and does not describe this run (STATUS=$status)." + return 0 + } chmod 644 "$PROVISION_STATUS_FILE" 2>/dev/null || true📝 Committable suggestion
🤖 Prompt for AI Agents