fix: CLI/installer hardening — restart labels, key permissions, unit-secret escaping (#113)#120
Merged
Merged
Conversation
…secret escaping (#113) Three CLI/installer findings from the 2026-05-31 audit (no server.mjs): 1. ocp-plugin `cmdRestart` hardcoded uid 501 + the legacy `ai.openclaw.proxy` label, and fell through to a dangerous `pkill -f 'node.*server.mjs' && cd ~/.openclaw/projects/*/; node server.mjs &` that could kill an unrelated node process and relaunch an env-less ghost proxy from a glob-ambiguous dir. Now uses process.getuid() + the live labels (dev.ocp.proxy on macOS, ocp-proxy on Linux systemd; the OpenClaw gateway label is unchanged) and drops the pkill fallback entirely (returns a manual `ocp restart` message on failure). 2. ocp-connect wrote the quota key unquoted into rc files and a world-readable environment.d/ocp.conf. Now single-quotes the value and chmod 600s the rc files and ocp.conf (matching the existing auth-profiles.json 0o600 convention). 3. setup.mjs interpolated the injected service-unit secrets (CLAUDE_BIN, OCP_ADMIN_KEY, PROXY_ANONYMOUS_KEY) raw into plist <string> and systemd Environment= lines. Added xmlEscape() for all plist <string> values and assertSafeInjectValue() which rejects control characters (\x00-\x1f — newline, CR, tab) before any unit is written, blocking a newline-injected rogue Environment= directive. Spaces are intentionally allowed (CLAUDE_BIN paths may contain them). XML-escaping on write also resolves plist-merge.mjs's [^<]* regex concern transitively (no raw < reaches it) — comment added, logic unchanged. ALIGNMENT.md: CLI/installer scripts only, no Anthropic operation forwarded → cli.js citation N/A. No blacklisted tokens or port literals introduced; alignment.yml passes. Independent fresh-context reviewer (opus): APPROVE (Iron Rule 10) — verified the control-char regex byte-exact via od (no space-rejection regression), the pkill fallback fully removed, restart labels match setup.mjs ground truth, OCP keys (base64url) cannot break the single-quoting, and the validator runs before any write. Closes #113. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the three CLI/installer hardening findings from the 2026-05-31 audit (#113, P2). No
server.mjs.ocp-pluginrestart — wasgui/501/ai.openclaw.proxy(hardcoded uid + legacy label) with a dangerouspkill -f 'node.*server.mjs' && … node server.mjs &fallback (could kill an unrelated node process + relaunch an env-less ghost proxy). Nowprocess.getuid()+ live labels (dev.ocp.proxymacOS /ocp-proxyLinux systemd; OpenClaw gateway label unchanged); pkill fallback removed (returns a manualocp restartmessage).ocp-connectkey persistence — single-quotes the persistedOPENAI_API_KEY/OPENAI_BASE_URLandchmod 600s the rc files + the Linuxenvironment.d/ocp.conf(was world-readable; matches the existingauth-profiles.json0o600 convention).setup.mjsunit-secret escaping —xmlEscape()on all plist<string>values;assertSafeInjectValue()rejects control chars (\x00-\x1f) in the 3 injected secrets before any unit is written, blocking a newline-injected rogueEnvironment=directive. Spaces intentionally allowed (CLAUDE_BIN paths). XML-escaping on write transitively resolvesplist-merge.mjs's[^<]*regex concern (comment added, logic unchanged).ALIGNMENT.md
alignment.ymlpasses (server.mjsuntouched).od(no space-rejection regression — the one subtle risk here), thepkill/server.mjs &/shell:truefallback fully removed, restart labels matchsetup.mjsground truth, OCP keys (base64url) cannot break the single-quoting, and the validator runs before any write.npm test→ 163 passed, 0 failed. Verdict APPROVE.Notes
setup.mjs/ocp-connect/launchctlwere not executed during implementation or review (live-host isolation).--keycontaining a literal'would break the rc single-quoting — but that's not an OCP-minted key, and the change is strictly safer than the prior unquoted form.Closes #113.
🤖 Generated with Claude Code