feat(hermes): add config and gateway service - #1658
Conversation
Migrate from OpenClaw to Hermes Agent with secret hydration via env.template and config.yaml placeholders. Adds systemd gateway service for Kyber.
|
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
📝 WalkthroughWalkthroughThis PR introduces a complete Hermes agent system configuration to the dotfiles. It adds personality/behavior guidance (SOUL.md), runtime agent configuration (config.yaml), environment secret placeholders, a hydration script to inject secrets at runtime, and Home Manager service integration with a systemd user service for the Hermes gateway. ChangesHermes Agent System
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Mesa DescriptionTL;DRAdds Hermes Agent configuration with secret hydration and a Kyber-only user What changed?
Description generated by Mesa. Update settings |
- config.tpl.yaml (model placeholders) -> llm-update.sh -> config.template.yaml - config.template.yaml (secret placeholders) -> hydrate.sh -> ~/.hermes/config.yaml - Follows openclaw convention: .tpl -> .template -> runtime output
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="home-manager/services/hermes/activate.sh">
<violation number="1" location="home-manager/services/hermes/activate.sh:7">
P2: Avoid creating a shared `/tmp/hermes` directory; use a user-scoped directory with restrictive permissions to prevent cross-user access.</violation>
</file>
<file name="home-manager/services/hermes/default.nix">
<violation number="1" location="home-manager/services/hermes/default.nix:37">
P2: Avoid logging to `/tmp/hermes/...` without creating the parent directory first; the service can fail to start when `/tmp/hermes` is missing.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| set -euo pipefail | ||
| HOME_DIR="$1" | ||
|
|
||
| mkdir -p /tmp/hermes |
There was a problem hiding this comment.
P2: Avoid creating a shared /tmp/hermes directory; use a user-scoped directory with restrictive permissions to prevent cross-user access.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/hermes/activate.sh, line 7:
<comment>Avoid creating a shared `/tmp/hermes` directory; use a user-scoped directory with restrictive permissions to prevent cross-user access.</comment>
<file context>
@@ -0,0 +1,13 @@
+set -euo pipefail
+HOME_DIR="$1"
+
+mkdir -p /tmp/hermes
+mkdir -p "$HOME_DIR/.hermes"
+mkdir -p "$HOME_DIR/.hermes/sessions"
</file context>
| "PATH=${homeDir}/.local/bin:${homeDir}/.nix-profile/bin:/usr/local/bin:/usr/bin:/bin" | ||
| ]; | ||
| WorkingDirectory = "${homeDir}/.hermes"; | ||
| StandardOutput = "append:/tmp/hermes/hermes-gateway.log"; |
There was a problem hiding this comment.
P2: Avoid logging to /tmp/hermes/... without creating the parent directory first; the service can fail to start when /tmp/hermes is missing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/hermes/default.nix, line 37:
<comment>Avoid logging to `/tmp/hermes/...` without creating the parent directory first; the service can fail to start when `/tmp/hermes` is missing.</comment>
<file context>
@@ -0,0 +1,44 @@
+ "PATH=${homeDir}/.local/bin:${homeDir}/.nix-profile/bin:/usr/local/bin:/usr/bin:/bin"
+ ];
+ WorkingDirectory = "${homeDir}/.hermes";
+ StandardOutput = "append:/tmp/hermes/hermes-gateway.log";
+ StandardError = "append:/tmp/hermes/hermes-gateway.log";
+ };
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
home-manager/services/hermes/default.nix (1)
37-38: No log rotation for/tmp/hermes/hermes-gateway.logUsing
append:with a static path in/tmpaccumulates indefinitely. On a long-running Kyber host this will eventually fill/tmp. Consider routing to the systemd journal (StandardOutput = "journal") or adding alogrotateentry, particularly since the gateway is configured to restart-always.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@home-manager/services/hermes/default.nix` around lines 37 - 38, The service unit currently sets StandardOutput and StandardError to append:/tmp/hermes/hermes-gateway.log which will grow without bound; change StandardOutput and StandardError to "journal" to send logs to systemd journal (or alternatively create a logrotate entry for /tmp/hermes/hermes-gateway.log and ensure the file lives outside volatile /tmp), and if you choose logrotate update the package/service config to install a rotation rule for /tmp/hermes/hermes-gateway.log; update the lines that set StandardOutput and StandardError in the Hermes systemd unit accordingly (or add the logrotate config) so the gateway (restart-always) cannot fill disk.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/hermes/config.yaml`:
- Around line 164-165: The config currently disables PII and secret redaction;
change the default flags to enable redaction by setting privacy.redact_pii to
true and security.redact_secrets to true so identifiers and secrets are redacted
by default. Update any related comments or docs that reference these defaults
and ensure any code reading these keys (e.g., config parsing that references
privacy.redact_pii and security.redact_secrets) handles the true values
appropriately.
- Around line 266-270: The current config sets tirith_enabled: true but
tirith_fail_open: true which allows bypass on timeout/errors; change the
behavior so failures do not open enforcement by setting tirith_fail_open to
false (or add a conditional/fallback that enforces deny-on-error) and ensure
tirith_timeout remains tuned (tirith_timeout) so timeouts trigger the
closed/deny path; update any related logic that reads tirith_fail_open to treat
missing/invalid values as false.
- Line 304: The hydration script's sed replacement is fragile for API keys with
characters like &, |, or \; update hydrate.sh to avoid unescaped direct
substitution for the placeholder "api_key: __CLIPROXY_API_KEY__": either switch
to envsubst (export the variable and run envsubst on the template) or
sanitize/escape the replacement before passing it to sed (escape &, | and
backslashes in the API key) and then use a safe delimiter in the sed expression;
locate the sed invocation that targets "__CLIPROXY_API_KEY__" and replace it
with one of these safer approaches so special characters in the API key do not
break the substitution.
In `@config/hermes/default.nix`:
- Around line 31-33: The current activation step
home.activation.hydrateHermesConfig silences all errors by appending "|| true"
to the hydrateScript invocation, which hides real hydration failures; remove the
"|| true" and instead rely on hydrate.sh's explicit skip/ok exit behavior (or
change hydrate.sh to return a distinct skip exit code and handle only that code
as non-fatal here), ensuring genuine failures from hydrateScript (broken
templates, sed errors, permission issues) will propagate and fail activation; if
you intentionally want silent degradation, replace the blanket suppression with
a clear comment explaining that choice or explicitly check for the skip exit
value emitted by hydrate.sh.
In `@config/hermes/env.template`:
- Line 1: The env template uses TELEGRAM_BOT_TOKEN while hydrate.sh expects
TELEGRAM_TOKEN; update one to be consistent: either change hydrate.sh to
read/write TELEGRAM_BOT_TOKEN or change the template to TELEGRAM_TOKEN so both
match, and add a brief comment in the template or hydrate.sh noting the expected
override variable; reference the TELEGRAM_BOT_TOKEN and TELEGRAM_TOKEN names and
the hydrate.sh script when making the change.
In `@config/hermes/hydrate.sh`:
- Around line 66-69: The script currently exits early when GATEWAY_TOKEN is
unset (check around the GATEWAY_TOKEN guard in hydrate.sh), which prevents
hydrating config.yaml even though config.yaml only needs __CLIPROXY_API_KEY__;
change the logic so that config.yaml creation/hydration always runs (use the
existing config.yaml/template code paths) and only the .env / gateway-specific
hydration is skipped when GATEWAY_TOKEN is missing; update the conditional
around GATEWAY_TOKEN to gate only the .env/gateway steps and, if the original
“all-or-nothing” behavior was intentional, add a comment explaining that
requirement next to the GATEWAY_TOKEN check.
In `@home-manager/services/hermes/activate.sh`:
- Line 7: The directory creation in activate.sh uses "mkdir -p /tmp/hermes"
which creates a world-readable 0755 directory; change the creation to set
restrictive permissions and correct ownership (e.g., use install -d -m 0700 -o
hermes -g hermes /tmp/hermes or run mkdir -p /tmp/hermes && chown hermes:hermes
/tmp/hermes && chmod 0700 /tmp/hermes) so /tmp/hermes/hermes-gateway.log is not
world-readable; update the activate.sh mkdir invocation accordingly so the
systemd service runs with a directory owned and permissioned for the hermes
user/group.
---
Nitpick comments:
In `@home-manager/services/hermes/default.nix`:
- Around line 37-38: The service unit currently sets StandardOutput and
StandardError to append:/tmp/hermes/hermes-gateway.log which will grow without
bound; change StandardOutput and StandardError to "journal" to send logs to
systemd journal (or alternatively create a logrotate entry for
/tmp/hermes/hermes-gateway.log and ensure the file lives outside volatile /tmp),
and if you choose logrotate update the package/service config to install a
rotation rule for /tmp/hermes/hermes-gateway.log; update the lines that set
StandardOutput and StandardError in the Hermes systemd unit accordingly (or add
the logrotate config) so the gateway (restart-always) cannot fill disk.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97f002c4-4e21-4dd7-98f9-b54bd4483754
📒 Files selected for processing (9)
config/default.nixconfig/hermes/SOUL.mdconfig/hermes/config.yamlconfig/hermes/default.nixconfig/hermes/env.templateconfig/hermes/hydrate.shhome-manager/services/default.nixhome-manager/services/hermes/activate.shhome-manager/services/hermes/default.nix
| redact_pii: false | ||
| tts: |
There was a problem hiding this comment.
Enable redaction by default to avoid sensitive data leakage
privacy.redact_pii: false (Line 164) and security.redact_secrets: false (Line 265) together create a high risk of exposing identifiers/secrets in logs, UI output, and persisted session artifacts. For a shared dotfiles baseline, secure defaults should be on.
Suggested change
privacy:
- redact_pii: false
+ redact_pii: true
...
security:
allow_private_urls: false
- redact_secrets: false
+ redact_secrets: trueAlso applies to: 263-266
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config/hermes/config.yaml` around lines 164 - 165, The config currently
disables PII and secret redaction; change the default flags to enable redaction
by setting privacy.redact_pii to true and security.redact_secrets to true so
identifiers and secrets are redacted by default. Update any related comments or
docs that reference these defaults and ensure any code reading these keys (e.g.,
config parsing that references privacy.redact_pii and security.redact_secrets)
handles the true values appropriately.
| tirith_enabled: true | ||
| tirith_path: tirith | ||
| tirith_timeout: 5 | ||
| tirith_fail_open: true | ||
| website_blocklist: |
There was a problem hiding this comment.
tirith_fail_open: true weakens runtime protection
With tirith_enabled: true (Line 266) but tirith_fail_open: true (Line 269), timeout/error paths effectively bypass enforcement. That undermines the control exactly when the security component is unavailable.
Suggested change
security:
tirith_enabled: true
tirith_path: tirith
tirith_timeout: 5
- tirith_fail_open: true
+ tirith_fail_open: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tirith_enabled: true | |
| tirith_path: tirith | |
| tirith_timeout: 5 | |
| tirith_fail_open: true | |
| website_blocklist: | |
| tirith_enabled: true | |
| tirith_path: tirith | |
| tirith_timeout: 5 | |
| tirith_fail_open: false | |
| website_blocklist: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config/hermes/config.yaml` around lines 266 - 270, The current config sets
tirith_enabled: true but tirith_fail_open: true which allows bypass on
timeout/errors; change the behavior so failures do not open enforcement by
setting tirith_fail_open to false (or add a conditional/fallback that enforces
deny-on-error) and ensure tirith_timeout remains tuned (tirith_timeout) so
timeouts trigger the closed/deny path; update any related logic that reads
tirith_fail_open to treat missing/invalid values as false.
| custom_providers: | ||
| - name: cliproxy | ||
| base_url: http://localhost:8317/v1 | ||
| api_key: __CLIPROXY_API_KEY__ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect relevant hydrator block
fd -i 'hydrate.sh' | xargs -I{} sh -c 'echo "== {} =="; nl -ba "{}" | sed -n "66,82p"'
# Reproduce sed replacement edge case (no repository mutation)
TEST_KEY='abc&def|ghi\jkl'
printf 'api_key: __CLIPROXY_API_KEY__\n' > /tmp/hermes_tmpl.yaml
echo "Input key: $TEST_KEY"
echo "Hydrated output:"
sed -e "s|__CLIPROXY_API_KEY__|${TEST_KEY}|g" /tmp/hermes_tmpl.yamlRepository: shunkakinoki/dotfiles
Length of output: 471
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find and read the hermes hydrate.sh file
echo "=== config/hermes/hydrate.sh ==="
cat config/hermes/hydrate.sh
echo ""
echo "=== Checking for sed commands with CLIPROXY_API_KEY ==="
rg "CLIPROXY_API_KEY" config/hermes/hydrate.sh -A 2 -B 2Repository: shunkakinoki/dotfiles
Length of output: 3187
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Test the actual vulnerability more carefully
# using a simpler test without nl command
TEST_KEY='abc&def|ghi\jkl'
TEMPLATE_FILE="/tmp/test_hydrate.yaml"
# Create test file
cat > "$TEMPLATE_FILE" << 'EOF'
api_key: __CLIPROXY_API_KEY__
EOF
echo "Template content:"
cat "$TEMPLATE_FILE"
echo ""
echo "Test key: $TEST_KEY"
echo ""
echo "Attempting sed with | delimiter (should fail due to | in key):"
sed -e "s|__CLIPROXY_API_KEY__|${TEST_KEY}|g" "$TEMPLATE_FILE" 2>&1 || echo "FAILED as expected"
echo ""
echo "Alternative: Test with different delimiter to show it works:"
sed -e "s@__CLIPROXY_API_KEY__|${TEST_KEY}@g" "$TEMPLATE_FILE" 2>&1 || echo "This test format issue, trying different approach"Repository: shunkakinoki/dotfiles
Length of output: 446
🏁 Script executed:
#!/bin/bash
# Search for all hydrate.sh files and look for sed patterns
echo "=== All hydrate.sh files found ==="
find . -name "hydrate.sh" -type f
echo ""
echo "=== Checking all hydrate.sh for sed patterns with CLIPROXY_API_KEY ==="
rg "sed.*CLIPROXY_API_KEY" --multiline -A 3 -B 1Repository: shunkakinoki/dotfiles
Length of output: 337
Fix fragile sed substitution for secrets containing special characters
The hydration script (config/hermes/hydrate.sh) uses unescaped sed replacement with | as delimiter. API keys containing &, |, or \ will cause sed to fail or produce corrupted config files. Example failure:
- Key:
abc&def|ghi\jkl - Error:
sed: -e expressionfeat: ini nix home #1, char 33: unknown option to 's'
Replace unescaped variable substitution with a safer approach (e.g., sed's -e with escaped delimiters, or use a tool like envsubst that doesn't interpret special characters).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config/hermes/config.yaml` at line 304, The hydration script's sed
replacement is fragile for API keys with characters like &, |, or \; update
hydrate.sh to avoid unescaped direct substitution for the placeholder "api_key:
__CLIPROXY_API_KEY__": either switch to envsubst (export the variable and run
envsubst on the template) or sanitize/escape the replacement before passing it
to sed (escape &, | and backslashes in the API key) and then use a safe
delimiter in the sed expression; locate the sed invocation that targets
"__CLIPROXY_API_KEY__" and replace it with one of these safer approaches so
special characters in the API key do not break the substitution.
| home.activation.hydrateHermesConfig = lib.hm.dag.entryAfter [ "writeBoundary" ] '' | ||
| ${pkgs.bash}/bin/bash "${hydrateScript}" || true | ||
| ''; |
There was a problem hiding this comment.
|| true silences genuine hydration failures, not just the intentional early-exit
hydrate.sh already exits 0 when GATEWAY_TOKEN is absent (its intentional "skip" path). The || true therefore only fires on unexpected errors — broken template paths, sed failures, permission errors mid-write — and swallows them all, leaving stale or partially-written configs under ~/.hermes while activation reports success and the service starts against them.
Consider propagating real failures while still allowing the intentional skip:
♻️ Proposed fix
- ${pkgs.bash}/bin/bash "${hydrateScript}" || true
+ ${pkgs.bash}/bin/bash "${hydrateScript}" || echo "Warning: Hermes hydration failed — check ${hydrateScript}" >&2Or, if silent degradation is the desired behavior for this personal setup, at least add a comment explaining why || true is intentional here.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| home.activation.hydrateHermesConfig = lib.hm.dag.entryAfter [ "writeBoundary" ] '' | |
| ${pkgs.bash}/bin/bash "${hydrateScript}" || true | |
| ''; | |
| home.activation.hydrateHermesConfig = lib.hm.dag.entryAfter [ "writeBoundary" ] '' | |
| ${pkgs.bash}/bin/bash "${hydrateScript}" || echo "Warning: Hermes hydration failed — check ${hydrateScript}" >&2 | |
| ''; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config/hermes/default.nix` around lines 31 - 33, The current activation step
home.activation.hydrateHermesConfig silences all errors by appending "|| true"
to the hydrateScript invocation, which hides real hydration failures; remove the
"|| true" and instead rely on hydrate.sh's explicit skip/ok exit behavior (or
change hydrate.sh to return a distinct skip exit code and handle only that code
as non-fatal here), ensuring genuine failures from hydrateScript (broken
templates, sed errors, permission issues) will propagate and fail activation; if
you intentionally want silent degradation, replace the blanket suppression with
a clear comment explaining that choice or explicitly check for the skip exit
value emitted by hydrate.sh.
| @@ -0,0 +1,4 @@ | |||
| TELEGRAM_BOT_TOKEN=__TELEGRAM_TOKEN__ | |||
There was a problem hiding this comment.
TELEGRAM_BOT_TOKEN vs TELEGRAM_TOKEN naming inconsistency
The output env key is TELEGRAM_BOT_TOKEN, but hydrate.sh resolves the value using the internal variable TELEGRAM_TOKEN (line 62 of hydrate.sh). The substitution into the template is correct, but anyone attempting to override via ~/dotfiles/.env must set TELEGRAM_TOKEN=…, not TELEGRAM_BOT_TOKEN=…. The disconnect isn't obvious from reading the template alone and could cause silent failures if someone tries the intuitive override name.
Consider either:
- Aligning the hydrate.sh resolution variable to
TELEGRAM_BOT_TOKEN, or - Adding a comment here noting the override variable name.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config/hermes/env.template` at line 1, The env template uses
TELEGRAM_BOT_TOKEN while hydrate.sh expects TELEGRAM_TOKEN; update one to be
consistent: either change hydrate.sh to read/write TELEGRAM_BOT_TOKEN or change
the template to TELEGRAM_TOKEN so both match, and add a brief comment in the
template or hydrate.sh noting the expected override variable; reference the
TELEGRAM_BOT_TOKEN and TELEGRAM_TOKEN names and the hydrate.sh script when
making the change.
| if [ -z "${GATEWAY_TOKEN}" ]; then | ||
| echo "Warning: HERMES_GATEWAY_TOKEN not set, skipping Hermes hydration" >&2 | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
Early exit on missing GATEWAY_TOKEN also blocks config.yaml hydration
config.yaml only requires __CLIPROXY_API_KEY__ (it has no gateway token placeholder). A client-mode host with a valid CLIPROXY_API_KEY but no GATEWAY_TOKEN will silently skip writing config.yaml, leaving Hermes unconfigured even though it has everything it needs.
If this sentinel behaviour is intentional (require full secrets or nothing), add a comment explaining it. Otherwise, consider splitting the guard so config.yaml is always hydrated and only .env is gated on GATEWAY_TOKEN:
♻️ Proposed fix
+# Hydrate config.yaml (only needs CLIPROXY_API_KEY, no gateway token required)
+@sed@ \
+ -e "s|__CLIPROXY_API_KEY__|${CLIPROXY_API_KEY}|g" \
+ "$CONFIG_TEMPLATE" >"${STATE_DIR}/config.yaml"
+chmod 600 "${STATE_DIR}/config.yaml"
+
if [ -z "${GATEWAY_TOKEN}" ]; then
echo "Warning: HERMES_GATEWAY_TOKEN not set, skipping Hermes hydration" >&2
exit 0
fi
-# Hydrate config.yaml
-@sed@ \
- -e "s|__CLIPROXY_API_KEY__|${CLIPROXY_API_KEY}|g" \
- "$CONFIG_TEMPLATE" >"${STATE_DIR}/config.yaml"
-chmod 600 "${STATE_DIR}/config.yaml"
-
# Hydrate .env🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config/hermes/hydrate.sh` around lines 66 - 69, The script currently exits
early when GATEWAY_TOKEN is unset (check around the GATEWAY_TOKEN guard in
hydrate.sh), which prevents hydrating config.yaml even though config.yaml only
needs __CLIPROXY_API_KEY__; change the logic so that config.yaml
creation/hydration always runs (use the existing config.yaml/template code
paths) and only the .env / gateway-specific hydration is skipped when
GATEWAY_TOKEN is missing; update the conditional around GATEWAY_TOKEN to gate
only the .env/gateway steps and, if the original “all-or-nothing” behavior was
intentional, add a comment explaining that requirement next to the GATEWAY_TOKEN
check.
| set -euo pipefail | ||
| HOME_DIR="$1" | ||
|
|
||
| mkdir -p /tmp/hermes |
There was a problem hiding this comment.
/tmp/hermes is created world-accessible (0755)
Gateway logs (/tmp/hermes/hermes-gateway.log) appended by the systemd service will be readable by any user on the machine. If the gateway proxies messages or emits tokens in its output, this leaks sensitive data.
🛡️ Proposed fix
-mkdir -p /tmp/hermes
+mkdir -p /tmp/hermes
+chmod 700 /tmp/hermes🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@home-manager/services/hermes/activate.sh` at line 7, The directory creation
in activate.sh uses "mkdir -p /tmp/hermes" which creates a world-readable 0755
directory; change the creation to set restrictive permissions and correct
ownership (e.g., use install -d -m 0700 -o hermes -g hermes /tmp/hermes or run
mkdir -p /tmp/hermes && chown hermes:hermes /tmp/hermes && chmod 0700
/tmp/hermes) so /tmp/hermes/hermes-gateway.log is not world-readable; update the
activate.sh mkdir invocation accordingly so the systemd service runs with a
directory owned and permissioned for the hermes user/group.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the Hermes AI assistant, including its configuration, a personality guide, and a systemd service for the gateway. The feedback focuses on improving the robustness of the configuration hydration process by addressing risky sed delimiters, fragile YAML parsing with awk, and the suppression of errors during activation. Additionally, it is recommended to transition logging from /tmp files to the systemd journal for better reliability and security.
| @sed@ \ | ||
| -e "s|__CLIPROXY_API_KEY__|${CLIPROXY_API_KEY}|g" \ | ||
| "$CONFIG_TEMPLATE" >"${STATE_DIR}/config.yaml" |
There was a problem hiding this comment.
Using | as a delimiter for sed is risky when the replacement string contains secrets like API keys, which can frequently include special characters, including |. If a secret contains the delimiter, the sed command will fail with a syntax error. A more robust approach would be to use a delimiter that is guaranteed not to be in the secret (like a control character), or to escape the delimiter within the variable before passing it to sed (e.g., ${CLIPROXY_API_KEY//|/\\|}). This risk also applies to the .env hydration block starting at line 78. This aligns with the repository's preference for robust parsing and delimiter usage.
References
- To robustly parse command output in shell scripts, use a unique delimiter (e.g., tab) in the format string and read with a matching IFS. This is safer than splitting by spaces with cut, especially when data fields might contain spaces.
| StandardOutput = "append:/tmp/hermes/hermes-gateway.log"; | ||
| StandardError = "append:/tmp/hermes/hermes-gateway.log"; |
There was a problem hiding this comment.
Redirecting StandardOutput and StandardError to a file in /tmp is generally discouraged for systemd services. Files in /tmp are volatile, and this approach can lead to permission conflicts on multi-user systems. It is recommended to use the systemd journal (the default behavior) which provides better log management, rotation, and security. You can view the logs using journalctl --user -u hermes-gateway.
| set -euo pipefail | ||
| HOME_DIR="$1" | ||
|
|
||
| mkdir -p /tmp/hermes |
There was a problem hiding this comment.
Creating a shared directory in /tmp can lead to permission conflicts if multiple users run this script on the same machine. Since the recommendation is to use the systemd journal for logging instead of a file in /tmp, this directory is no longer needed.
| in | ||
| { | ||
| home.activation.hydrateHermesConfig = lib.hm.dag.entryAfter [ "writeBoundary" ] '' | ||
| ${pkgs.bash}/bin/bash "${hydrateScript}" || true |
There was a problem hiding this comment.
The use of || true masks all potential errors during the configuration hydration process. If the hydrate.sh script fails due to a syntax error, a missing template, or a failed sed command, the Home Manager activation will still report success, leaving the system in an inconsistent state. Since hydrate.sh already handles the case of missing optional secrets by exiting gracefully with 0 (at line 68), removing || true will ensure that real errors are correctly reported.
${pkgs.bash}/bin/bash "${hydrateScript}"
References
- Maintain consistency with established patterns for writing scripts that are extracted from Nix expressions, even if it involves suppressing linter warnings like ShellCheck SC2024.
| @awk@ ' | ||
| /^api-keys:/ { in_api_keys = 1; next } | ||
| in_api_keys && /^ - / { | ||
| value = $0 | ||
| sub(/^ - "/, "", value) | ||
| sub(/"$/, "", value) | ||
| print value | ||
| exit | ||
| } | ||
| in_api_keys && /^[^[:space:]]/ { exit } | ||
| ' "$config_file" |
There was a problem hiding this comment.
Parsing YAML using awk and regular expressions is fragile and prone to breaking if the format of the CLIPROXY_CONFIG file changes (e.g., different indentation, comments, or unquoted values). To ensure robust parsing as preferred in this repository, if yq is available in the environment, it would be a much more robust choice for extracting values from a YAML file.
References
- To robustly parse command output in shell scripts, use a unique delimiter (e.g., tab) in the format string and read with a matching IFS. This is safer than splitting by spaces with cut, especially when data fields might contain spaces.
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="config/hermes/config.tpl.yaml">
<violation number="1" location="config/hermes/config.tpl.yaml:2">
P2: `model.default` uses an unresolved placeholder (`__MINIMAX__`) that is never hydrated, so the runtime config keeps an invalid default model value.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -0,0 +1,305 @@ | |||
| model: | |||
| default: cliproxy/__MINIMAX__ | |||
There was a problem hiding this comment.
P2: model.default uses an unresolved placeholder (__MINIMAX__) that is never hydrated, so the runtime config keeps an invalid default model value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/hermes/config.tpl.yaml, line 2:
<comment>`model.default` uses an unresolved placeholder (`__MINIMAX__`) that is never hydrated, so the runtime config keeps an invalid default model value.</comment>
<file context>
@@ -0,0 +1,305 @@
+model:
+ default: cliproxy/__MINIMAX__
+providers: {}
+fallback_providers: []
</file context>
Add hermes_hydrate_spec.sh and activate tests. Register hermes scripts in coverage_spec.sh. Also adds hermes to llm-update.sh template map.
* feat(hermes): add config and gateway service Migrate from OpenClaw to Hermes Agent with secret hydration via env.template and config.yaml placeholders. Adds systemd gateway service for Kyber. * feat(hermes): add two-layer template hydration - config.tpl.yaml (model placeholders) -> llm-update.sh -> config.template.yaml - config.template.yaml (secret placeholders) -> hydrate.sh -> ~/.hermes/config.yaml - Follows openclaw convention: .tpl -> .template -> runtime output * test(hermes): add shell specs and coverage entries Add hermes_hydrate_spec.sh and activate tests. Register hermes scripts in coverage_spec.sh. Also adds hermes to llm-update.sh template map.
1 participant
Summary
__PLACEHOLDER__+ sed)config/hermes/: config.yaml template, env.template, hydrate.sh, SOUL.md, default.nixhome-manager/services/hermes/: systemd gateway service (Kyber-only)hermes claw migrateTest plan
nix buildsucceeds~/.hermes/config.yamland~/.hermes/.envwith correct secretssystemctl --user status hermes-gatewayruns on KyberSummary by cubic
Adds Hermes Agent config with secret hydration and a Kyber-only user
hermes-gatewayservice. Migrates the OpenClaw setup tohermeswith secure, on-activation config generation.New Features
config/hermes/with two-layer templating:config.tpl.yaml->scripts/llm-update.sh->config.template.yaml->hydrate.sh->~/.hermes/config.yaml; includes.envtemplate and SOUL guide.~/.config/hermes/*, or~/.cli-proxy-api/config.yaml(CLIPROXY_API_KEY).~/.hermesdirs; wired viaconfig/default.nixandhome-manager/services.systemduser servicehermes-gateway(Kyber-only) runshermes gatewaywith auto-restart; logs to/tmp/hermes/hermes-gateway.log.Migration
HERMES_GATEWAY_TOKEN(required),TELEGRAM_TOKEN,WHATSAPP_ALLOW_FROM,CLIPROXY_API_KEY(env or files in~/.config/hermes/).systemctl --user status hermes-gateway.hermes claw migrateto import OpenClaw settings.Written for commit 60d1f22. Summary will update on new commits.