fix(sensor-proxy): eliminate all uncoordinated config writers
Remove all code paths that manipulate config files without Phase 2 locking: 1. Installer: Remove ensure_allowed_nodes_file_reference() call (line 1674) - Migration now handled exclusively by config migrate-to-file 2. Installer: Make migration failures fatal in update_allowed_nodes() - Prevents fallback to unsafe Python manipulation 3. Daemon sanitizer: Remove os.WriteFile() call - Now only sanitizes in-memory copy, doesn't write back to disk - Logs warning instructing admin to run `config migrate-to-file` 4. Self-heal script: Replace 132 lines of Python with CLI call - sanitize_allowed_nodes() now calls `config migrate-to-file` - Eliminates uncoordinated Python-based config rewriting All config mutations now flow exclusively through Phase 2 CLI with atomic operations and file locking. No code paths remain that can create duplicate allowed_nodes blocks. Addresses Codex review feedback on Phase 2 gaps.
This commit is contained in:
parent
0e2559a6d4
commit
9d20acd35e
2 changed files with 21 additions and 140 deletions
|
|
@ -545,20 +545,15 @@ func sanitizeDuplicateAllowedNodesBlocks(path string, data []byte) (bool, []byte
|
||||||
buf.Write(data[last:])
|
buf.Write(data[last:])
|
||||||
|
|
||||||
cleaned := buf.Bytes()
|
cleaned := buf.Bytes()
|
||||||
|
|
||||||
|
// Phase 2: DO NOT write the file back - that would create an uncoordinated writer
|
||||||
|
// The sanitizer only fixes the in-memory copy for this startup
|
||||||
|
// Admins should run `pulse-sensor-proxy config migrate-to-file` to fix the file atomically
|
||||||
if path != "" {
|
if path != "" {
|
||||||
if err := os.WriteFile(path, cleaned, 0600); err != nil {
|
log.Warn().
|
||||||
log.Warn().
|
Str("config_file", path).
|
||||||
Err(err).
|
Int("removed_duplicate_blocks", len(matches)-1).
|
||||||
Str("config_file", path).
|
Msg("CONFIG SANITIZED (in-memory only) – duplicate allowed_nodes blocks detected; run 'pulse-sensor-proxy config migrate-to-file' to fix the file permanently")
|
||||||
Msg("Failed to rewrite sanitized configuration; using in-memory copy only")
|
|
||||||
} else {
|
|
||||||
// Phase 2: This sanitizer should never fire with the new config CLI
|
|
||||||
// If it does, it indicates a bug or manual file editing
|
|
||||||
log.Warn().
|
|
||||||
Str("config_file", path).
|
|
||||||
Int("removed_duplicate_blocks", len(matches)-1).
|
|
||||||
Msg("CONFIG SANITIZED – duplicate allowed_nodes blocks removed; this should not happen with Phase 2 config CLI – please report if you see this")
|
|
||||||
}
|
|
||||||
} else {
|
} else {
|
||||||
log.Warn().
|
log.Warn().
|
||||||
Int("removed_duplicate_blocks", len(matches)-1).
|
Int("removed_duplicate_blocks", len(matches)-1).
|
||||||
|
|
|
||||||
|
|
@ -612,9 +612,10 @@ update_allowed_nodes() {
|
||||||
local nodes=("$@")
|
local nodes=("$@")
|
||||||
|
|
||||||
# Phase 2: Use config CLI exclusively - no shell/Python manipulation
|
# Phase 2: Use config CLI exclusively - no shell/Python manipulation
|
||||||
# First, migrate any inline allowed_nodes to file mode
|
# First, migrate any inline allowed_nodes to file mode (failures are fatal)
|
||||||
if "$BINARY_PATH" config migrate-to-file --config "$CONFIG_FILE" --allowed-nodes "$ALLOWED_NODES_FILE" 2>/dev/null; then
|
if ! "$BINARY_PATH" config migrate-to-file --config "$CONFIG_FILE" --allowed-nodes "$ALLOWED_NODES_FILE"; then
|
||||||
print_info "Migrated inline allowed_nodes to file mode"
|
print_error "Failed to migrate config to file mode"
|
||||||
|
return 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
# Build --merge flags for the CLI
|
# Build --merge flags for the CLI
|
||||||
|
|
@ -1671,7 +1672,8 @@ EOF
|
||||||
chmod 0644 /etc/pulse-sensor-proxy/config.yaml
|
chmod 0644 /etc/pulse-sensor-proxy/config.yaml
|
||||||
fi
|
fi
|
||||||
|
|
||||||
ensure_allowed_nodes_file_reference
|
# Phase 2: Migration handled by update_allowed_nodes() -> config migrate-to-file
|
||||||
|
# No need to call ensure_allowed_nodes_file_reference anymore
|
||||||
|
|
||||||
# Register socket-mode proxy with Pulse if server provided
|
# Register socket-mode proxy with Pulse if server provided
|
||||||
if [[ "$HTTP_MODE" != true ]]; then
|
if [[ "$HTTP_MODE" != true ]]; then
|
||||||
|
|
@ -3104,135 +3106,19 @@ log() {
|
||||||
}
|
}
|
||||||
|
|
||||||
sanitize_allowed_nodes() {
|
sanitize_allowed_nodes() {
|
||||||
|
# Phase 2: Use config CLI instead of Python manipulation
|
||||||
if [[ ! -f "$CONFIG_FILE" ]]; then
|
if [[ ! -f "$CONFIG_FILE" ]]; then
|
||||||
return
|
return
|
||||||
fi
|
fi
|
||||||
if ! command -v python3 >/dev/null 2>&1; then
|
|
||||||
|
if [[ ! -x "$BINARY_PATH" ]]; then
|
||||||
|
log "Binary not available; skipping sanitization"
|
||||||
return
|
return
|
||||||
fi
|
fi
|
||||||
|
|
||||||
local result
|
# Use CLI to atomically migrate any inline blocks to file mode
|
||||||
if ! result=$(python3 - "$CONFIG_FILE" <<'PY'
|
if "$BINARY_PATH" config migrate-to-file --config "$CONFIG_FILE" 2>&1 | grep -q "Migration complete"; then
|
||||||
import sys
|
log "Migrated inline allowed_nodes to file mode"
|
||||||
from pathlib import Path
|
|
||||||
|
|
||||||
path = Path(sys.argv[1])
|
|
||||||
lines = path.read_text().splitlines()
|
|
||||||
blocks = []
|
|
||||||
i = 0
|
|
||||||
while i < len(lines):
|
|
||||||
line = lines[i]
|
|
||||||
stripped = line.lstrip()
|
|
||||||
if stripped.startswith("allowed_nodes:"):
|
|
||||||
start = i
|
|
||||||
entries = []
|
|
||||||
j = i + 1
|
|
||||||
while j < len(lines):
|
|
||||||
nxt = lines[j]
|
|
||||||
nxt_stripped = nxt.lstrip()
|
|
||||||
if nxt_stripped.startswith("-"):
|
|
||||||
entries.append(nxt_stripped[1:].strip())
|
|
||||||
j += 1
|
|
||||||
continue
|
|
||||||
if (
|
|
||||||
nxt_stripped.startswith("#")
|
|
||||||
or nxt_stripped == ""
|
|
||||||
or nxt.startswith((" ", "\t"))
|
|
||||||
):
|
|
||||||
j += 1
|
|
||||||
continue
|
|
||||||
break
|
|
||||||
|
|
||||||
comment_indices = set()
|
|
||||||
comment_text = []
|
|
||||||
k = start - 1
|
|
||||||
while k >= 0 and lines[k].strip() == "":
|
|
||||||
comment_indices.add(k)
|
|
||||||
k -= 1
|
|
||||||
while k >= 0 and lines[k].lstrip().startswith("#"):
|
|
||||||
comment_indices.add(k)
|
|
||||||
comment_text.append(lines[k])
|
|
||||||
k -= 1
|
|
||||||
comment_text.reverse()
|
|
||||||
|
|
||||||
blocks.append(
|
|
||||||
{
|
|
||||||
"start": start,
|
|
||||||
"end": j,
|
|
||||||
"comment_indices": comment_indices,
|
|
||||||
"comment_text": comment_text,
|
|
||||||
"entries": entries,
|
|
||||||
}
|
|
||||||
)
|
|
||||||
i = j
|
|
||||||
continue
|
|
||||||
i += 1
|
|
||||||
|
|
||||||
if len(blocks) <= 1:
|
|
||||||
sys.exit(0)
|
|
||||||
|
|
||||||
seen = set()
|
|
||||||
merged = []
|
|
||||||
for block in blocks:
|
|
||||||
for entry in block["entries"]:
|
|
||||||
key = entry.lower()
|
|
||||||
if not key or key in seen:
|
|
||||||
continue
|
|
||||||
seen.add(key)
|
|
||||||
merged.append(entry)
|
|
||||||
|
|
||||||
if not merged:
|
|
||||||
sys.exit(0)
|
|
||||||
|
|
||||||
first_block = blocks[0]
|
|
||||||
insert_at = min(
|
|
||||||
[first_block["start"]] + list(first_block["comment_indices"])
|
|
||||||
) if first_block["comment_indices"] else first_block["start"]
|
|
||||||
|
|
||||||
def build_comment():
|
|
||||||
if first_block["comment_text"]:
|
|
||||||
return first_block["comment_text"]
|
|
||||||
return ["# Cluster nodes (auto-discovered during installation)"]
|
|
||||||
|
|
||||||
comment_block = build_comment()
|
|
||||||
replacement = []
|
|
||||||
replacement.extend(comment_block)
|
|
||||||
if replacement and replacement[-1].strip() != "":
|
|
||||||
replacement.append("")
|
|
||||||
replacement.append("allowed_nodes:")
|
|
||||||
for entry in merged:
|
|
||||||
replacement.append(f" - {entry}")
|
|
||||||
replacement.append("")
|
|
||||||
|
|
||||||
indices_to_remove = set()
|
|
||||||
for block in blocks:
|
|
||||||
indices_to_remove.update(range(block["start"], block["end"]))
|
|
||||||
indices_to_remove.update(block["comment_indices"])
|
|
||||||
|
|
||||||
result = []
|
|
||||||
inserted = False
|
|
||||||
for idx, line in enumerate(lines):
|
|
||||||
if not inserted and idx == insert_at:
|
|
||||||
result.extend(replacement)
|
|
||||||
inserted = True
|
|
||||||
if idx in indices_to_remove:
|
|
||||||
continue
|
|
||||||
result.append(line)
|
|
||||||
|
|
||||||
if not inserted:
|
|
||||||
if result and result[-1].strip() != "":
|
|
||||||
result.append("")
|
|
||||||
result.extend(replacement)
|
|
||||||
|
|
||||||
path.write_text("\n".join(result).rstrip() + "\n")
|
|
||||||
print(f"Sanitized duplicate allowed_nodes blocks ({len(blocks) - 1} removed)")
|
|
||||||
PY
|
|
||||||
); then
|
|
||||||
return
|
|
||||||
fi
|
|
||||||
|
|
||||||
if [[ -n "$result" ]]; then
|
|
||||||
log "$result"
|
|
||||||
fi
|
fi
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue