Fix review findings: fail-closed merge, die ordering, sudo-path idempotency

- merge_policies: interpreter failure now aborts with the temp file
  removed instead of moving an empty tmp into place (errexit is
  suppressed when the function is called in a || context, so the
  status is checked explicitly); config stays untouched on failure
- move info/ok/warn/die above arg parsing so --allow-provider without
  a value prints the intended error, not 'die: command not found'
- install_enterprise_policy: compare content before writing on the
  sudo path (skip when already enforced, warn on drift), repair the
  no-root-with-existing-file case instead of pinning env vars, chmod
  644 the policy file in both paths
- tests: in-place merge, fail-closed on bad JSON, clean arg error
This commit is contained in:
c4ch3c4d3 committed 2026-10-06 10:45:56 -06:00
1 parent c926361ccc
commit 778341b7a5
1 file changed
+41 -12
+41 -12
View File
@@ -53,6 +53,10 @@ INVITE=0
INVITE_PORT=3000 INVITE_PORT=3000
NO_ENTERPRISE=0 NO_ENTERPRISE=0
ALLOW_PROVIDERS_STR="" ALLOW_PROVIDERS_STR=""
info() { printf '\033[1;34m[info]\033[0m %s\n' "$*"; }
ok() { printf '\033[1;32m[ ok ]\033[0m %s\n' "$*"; }
warn() { printf '\033[1;33m[warn]\033[0m %s\n' "$*"; }
die() { printf '\033[1;31m[fail]\033[0m %s\n' "$*" >&2; exit 1; }
while [ $# -gt 0 ]; do while [ $# -gt 0 ]; do
case "$1" in case "$1" in
--relay-url) RELAY_URL="$2"; shift 2 ;; --relay-url) RELAY_URL="$2"; shift 2 ;;
@@ -69,11 +73,6 @@ while [ $# -gt 0 ]; do
esac esac
done done
info() { printf '\033[1;34m[info]\033[0m %s\n' "$*"; }
ok() { printf '\033[1;32m[ ok ]\033[0m %s\n' "$*"; }
warn() { printf '\033[1;33m[warn]\033[0m %s\n' "$*"; }
die() { printf '\033[1;31m[fail]\033[0m %s\n' "$*" >&2; exit 1; }
# ---- helpers ---------------------------------------------------------------- # ---- helpers ----------------------------------------------------------------
# policy_json — emit the enterprise policy file content on stdout # policy_json — emit the enterprise policy file content on stdout
@@ -97,6 +96,7 @@ write_policy_file() {
warn "$(basename "$f"): differs from enforced policy — overwriting" warn "$(basename "$f"): differs from enforced policy — overwriting"
fi fi
mv "$tmp" "$f" mv "$tmp" "$f"
chmod 644 "$f"
ok "$(basename "$f"): installed" ok "$(basename "$f"): installed"
} }
@@ -104,10 +104,24 @@ write_policy_file() {
# (sudo when available); without root, pin OPENCHAMBER_ENTERPRISE_MODE=1 in # (sudo when available); without root, pin OPENCHAMBER_ENTERPRISE_MODE=1 in
# the rc files instead and say so loudly. # the rc files instead and say so loudly.
install_enterprise_policy() { install_enterprise_policy() {
local dir sudo="" local dir sudo="" tmp current
dir="$(dirname "$POLICY_FILE")" dir="$(dirname "$POLICY_FILE")"
tmp="$(mktemp)"
policy_json > "$tmp"
# Already correct? Then we're done regardless of root.
if [ -r "$POLICY_FILE" ]; then
current="$(cat "$POLICY_FILE" 2>/dev/null || true)"
if [ "$current" = "$(cat "$tmp")" ]; then
rm -f "$tmp"
ok "$(basename "$POLICY_FILE"): already enforced"
return 0
fi
fi
if [ -d "$dir" ] && [ -w "$dir" ]; then if [ -d "$dir" ] && [ -w "$dir" ]; then
write_policy_file "$POLICY_FILE" write_policy_file "$POLICY_FILE"
rm -f "$tmp"
return 0 return 0
fi fi
if sudo -n true 2>/dev/null; then if sudo -n true 2>/dev/null; then
@@ -115,9 +129,20 @@ install_enterprise_policy() {
elif [ -t 0 ]; then elif [ -t 0 ]; then
sudo="sudo" sudo="sudo"
fi fi
if [ -n "$sudo" ] && $sudo mkdir -p "$dir" 2>/dev/null && $sudo tee "$POLICY_FILE" >/dev/null <<< "$(policy_json)" 2>/dev/null; then if [ -n "$sudo" ]; then
ok "enterprise policy installed at $POLICY_FILE" if [ -f "$POLICY_FILE" ]; then
return 0 warn "$(basename "$POLICY_FILE"): differs from enforced policy — overwriting"
fi
if $sudo mkdir -p "$dir" 2>/dev/null && $sudo tee "$POLICY_FILE" >/dev/null < "$tmp" && $sudo chmod 644 "$POLICY_FILE" 2>/dev/null; then
rm -f "$tmp"
ok "enterprise policy installed at $POLICY_FILE"
return 0
fi
fi
rm -f "$tmp"
if [ -f "$POLICY_FILE" ]; then
warn "no root: $POLICY_FILE exists but differs from enforced policy and could not be repaired"
return 1
fi fi
warn "no root: cannot write $POLICY_FILE — falling back to env-var mode" warn "no root: cannot write $POLICY_FILE — falling back to env-var mode"
warn "env-var mode is a weaker lock (the user can edit their own rc files)" warn "env-var mode is a weaker lock (the user can edit their own rc files)"
@@ -142,11 +167,11 @@ pin_enterprise_env() {
# derived from the config's own providers block, the baseline list, and any # derived from the config's own providers block, the baseline list, and any
# extras (deduped, in that order). Uses python3, falls back to bun. # extras (deduped, in that order). Uses python3, falls back to bun.
merge_policies() { merge_policies() {
local in="$1" out="$2" tmp local in="$1" out="$2" tmp status=0
local extras="${*:3}" local extras="${*:3}"
tmp="$(mktemp)" tmp="$(mktemp)"
if command -v python3 >/dev/null 2>&1; then if command -v python3 >/dev/null 2>&1; then
python3 - "$in" "$tmp" "$BASELINE_ALLOWED_PROVIDERS" "$extras" <<'PY' python3 - "$in" "$tmp" "$BASELINE_ALLOWED_PROVIDERS" "$extras" <<'PY' || status=1
import json, sys import json, sys
src, dst, baseline, extras = sys.argv[1], sys.argv[2], sys.argv[3], sys.argv[4] src, dst, baseline, extras = sys.argv[1], sys.argv[2], sys.argv[3], sys.argv[4]
cfg = json.load(open(src)) cfg = json.load(open(src))
@@ -177,10 +202,14 @@ for (const p of [...derived,
cfg.experimental = cfg.experimental || {}; cfg.experimental = cfg.experimental || {};
cfg.experimental.policies = rules; cfg.experimental.policies = rules;
fs.writeFileSync(process.env.OUT, JSON.stringify(cfg, null, 2) + "\n"); fs.writeFileSync(process.env.OUT, JSON.stringify(cfg, null, 2) + "\n");
' ' || status=1
else else
die "need python3 or bun to inject provider policies" die "need python3 or bun to inject provider policies"
fi fi
if [ "$status" -ne 0 ]; then
rm -f "$tmp"
die "could not inject provider policies into $in (bad JSON or interpreter failure) — existing config left untouched"
fi
mv "$tmp" "$out" mv "$tmp" "$out"
} }