diff --git a/.gitignore b/.gitignore index 3482954..9477fa0 100644 --- a/.gitignore +++ b/.gitignore @@ -3,7 +3,10 @@ # Templates (*.template) are safe and remain tracked. Configurations/host*.conf Configurations/master.conf -Configurations/*.bak +# *.bak alone missed conf_upgrade's real output — it writes host1.conf.bak-20260802, which does +# not end in .bak — so those sat untracked rather than ignored, one `git add -A` from being +# pushed. The glob has to cover the suffix. +Configurations/*.bak* .vscode # ── Personal scratch notes — dev-only, never pushed ─────────────────────────── diff --git a/Deployment/master.conf.template b/Deployment/master.conf.template index e835264..b0c3a3e 100644 --- a/Deployment/master.conf.template +++ b/Deployment/master.conf.template @@ -144,6 +144,8 @@ AI_DATA_DIR="${DATA_DIR}/ai" CACHE_BACKUP_DIR="${DATA_DIR}/cache" LOG_ARCHIVE_DIR="${DATA_DIR}/logs" + BACKUP_DIR="${DATA_DIR}/Backups" + CONF_BACKUP_DIR="${BACKUP_DIR}/Confs" # 0700 — holds credentials PERSISTENT_CONF_CACHE="${CACHE_BACKUP_DIR}/conf" ARR_CACHE_BACKUP_DIR="${CACHE_BACKUP_DIR}/arr" @@ -1659,6 +1661,13 @@ # ============================================================================================== # ── AI / RAG ────────────────────────────────────────────────────────────────────────────────── +# ━━━ Conf Backups ━━━ +# Every write through the plugin copies the conf aside first, to CONF_BACKUP_DIR with an +# ISO-8601 stamp. The confs are gitignored, so that directory is the whole recovery path — there +# is no history to revert to. A backup that cannot be taken cancels the write. +# Oldest are pruned past this count, per conf file. + CONF_BACKUP_RETAIN=30 + # ============================================================================================== # Varaverk works exactly as well with AI off as with it on. Nothing below is required for any # script to function — every feature that can lean on AI has a complete non-AI path, and the diff --git a/Plugin/unraid/include/confform.php b/Plugin/unraid/include/confform.php index 655fb50..1d0372f 100644 --- a/Plugin/unraid/include/confform.php +++ b/Plugin/unraid/include/confform.php @@ -18,7 +18,27 @@ // // Structure only; values are not validated. // Consistent with conf_upgrade.sh, this reconciles shape and leaves correctness to the -// consuming script. +// consuming script. Shape is enforced — the file must parse, source, and read back the +// value that was asked for. Whether 300 is a sensible timeout is still not this file's +// question. +// +// One guarded write path, not one set of guards per caller. +// vv_conf_edit() owns the lock, the backup, the validation and the audit line, and every +// conf write in the plugin goes through it — the Settings form, both raw editors, the +// flag toggles, orchestrator membership, script moves and reorders, the rsync window +// arrays, the docker folder map and first-run setup. vv_write_conf_raw() has exactly one +// caller left, inside vv_conf_edit() itself. +// +// A caller that reaches past it gets tmp + rename and nothing else: no backup, no bash -n, +// no read-back, no audit line. That is how this started — seven files each with their own +// partial idea of what a safe conf write was, two of them carrying a copy-pasted bash -n +// block that failed open. +// +// The rewrite happens inside the lock, or it proves nothing moved. +// $mutate receives the current contents, so a caller that can rebuild from them has no +// window at all. Callers that must assemble the result first — the ones driven by a form +// payload — compare against what they read and return null if it no longer matches, +// which abandons the write rather than reverting a concurrent edit. // // OPERATIONAL SAFEGUARDS // An unmatched section yields no fields rather than a wrong write. @@ -30,6 +50,33 @@ // Each field carries the exact line it came from, so a write cannot land outside the // subsection it was read from. // +// A value is shell source code, so command substitution is refused outright. +// Scalars are written inside double quotes and array values are spliced in verbatim, so +// $(...) or a backtick in a value runs as root in every script that sources the conf. No +// conf in this repo uses either, so rejecting them costs nothing. $VAR and ${VAR} stay +// legal — AI_DATA_DIR="${DATA_DIR}/ai" is the established idiom, and a reference resolves +// to a value where a substitution runs a program. +// +// Nothing is written without a backup in hand. +// The previous contents are copied to CONF_BACKUP_DIR/. first, and a backup +// that cannot be taken cancels the write. The confs are gitignored, so that directory is +// the entire recovery story — there is no history to revert to. +// +// Validated, then verified, then rolled back on failure. +// bash -n proves the candidate parses; sourcing the installed file and reading the keys +// back proves the values survived quoting. A scalar that does not read back as the value +// requested restores the backup. A conf that parses cleanly and holds the wrong string is +// the failure a syntax check cannot see. +// +// Held under an exclusive lock for the whole read-modify-write. +// Two concurrent savers would otherwise read the same original, and the second rename +// would discard the first one's change without either reporting a failure. +// +// Every outcome is logged, and secrets are logged by name only. +// LOG_DIR/conf_changes.log records applied, rejected, failed and rolled-back alike. A +// credential-shaped key logs value= — the log proves a change happened, it is +// not a second copy of the secret. +// // EXPORTS // vv_conf_has_sections() does this script have an editable conf section // vv_conf_parse_subsection() fields within one named subsection @@ -38,7 +85,10 @@ // vv_conf_write_changes() apply edits back to the conf file // // CONFIGURATION -// CONF_DIR master.conf and host*.conf are the read and write targets +// CONF_DIR master.conf and host*.conf are the read and write targets +// CONF_BACKUP_DIR DATA_DIR/Backups/Confs — pre-write copies, 0700 +// LOG_DIR conf_changes.log is written here +// CONF_BACKUP_RETAIN backups kept per conf file (default 30) // ═══════════════════════════════════════════════════════════════════════════════════════════════ require_once __DIR__ . '/config.php'; @@ -307,19 +357,132 @@ function vv_conf_fields_for_script(string $id): array { return $groups; } +// A conf key must be a plain shell identifier. Every downstream use — the replacement regex, +// the source-verification subshell, the audit line — treats the key as trusted text, so it is +// validated once here rather than escaped differently in three places. +function vv_conf_key_valid(string $key): bool { + return (bool) preg_match('/^[A-Za-z_][A-Za-z0-9_]*$/', $key); +} + +// Credential-shaped keys are logged by name only. The audit log is the record that a change +// happened, not a second copy of the secret that changed. +function vv_conf_key_is_secret(string $key): bool { + return (bool) preg_match('/(PASS|PASSWORD|SECRET|TOKEN|API_KEY|APIKEY|_KEY)$|(PASS|PASSWORD|SECRET|TOKEN|APIKEY)/i', $key); +} + +// Scalar values are written inside double quotes and array values are spliced in verbatim, so +// a value is shell source code the moment any script reads the conf. Command substitution in a +// value therefore executes on every load — in every script, as root. No conf in this repo uses +// it, so rejecting it costs nothing and closes the path. +// +// $VAR and ${VAR} stay legal on purpose: AI_DATA_DIR="${DATA_DIR}/ai" is the established idiom +// here, and a reference resolves to a value where a substitution runs a program. +function vv_conf_value_safe(string $value): bool { + return !preg_match('/\$\(|`|<\(|>\(/', $value); +} + +// Under DATA_DIR, not beside the confs. data/ is the one on-disk root and is gitignored whole, +// so a backup here cannot become a tracked file the way Configurations/*.bak did. 0700 because +// these are verbatim copies of files holding every credential on the host. +function vv_conf_backup_dir(): string { + if (!is_dir(CONF_BACKUP_DIR)) @mkdir(CONF_BACKUP_DIR, 0700, true); + return CONF_BACKUP_DIR; +} + +// Copy the current conf aside before it is touched. Returns the backup path, or null when no +// backup could be taken — which the caller treats as a reason not to write, because the whole +// recovery story for these files is this directory. +function vv_conf_backup(string $filename): ?string { + $src = CONF_DIR . '/' . $filename; + if (!is_file($src)) return null; + + $dest = vv_conf_backup_dir() . '/' . $filename . '.' . date('Y-m-d\THis'); + // Same second, second change: keep both rather than silently overwrite the older one. + if (file_exists($dest)) { + $n = 1; + while (file_exists($dest . '.' . $n)) $n++; + $dest .= '.' . $n; + } + if (!@copy($src, $dest)) return null; + @chmod($dest, 0600); + + vv_conf_prune_backups($filename); + return $dest; +} + +// Retain the newest CONF_BACKUP_RETAIN backups per conf file. Pruned by filename, which sorts +// chronologically because the stamp is ISO-8601 — no stat() per candidate. +function vv_conf_prune_backups(string $filename): void { + $keep = (int) (vv_conf_vars()['CONF_BACKUP_RETAIN'] ?? 30); + if ($keep < 1) $keep = 30; + + $found = glob(vv_conf_backup_dir() . '/' . $filename . '.*') ?: []; + if (count($found) <= $keep) return; + + sort($found); + foreach (array_slice($found, 0, count($found) - $keep) as $old) @unlink($old); +} + +// Source a candidate conf in a subshell and read back the keys that were just written. +// bash -n proves the file parses; this proves the values survived quoting and arrived as +// intended. Returns null when the file could not be sourced at all. +function vv_conf_read_back(string $path, array $keys): ?array { + $script = 'source ' . escapeshellarg($path) . ' >/dev/null 2>&1 || exit 90; '; + foreach ($keys as $k) { + if (!vv_conf_key_valid($k)) continue; + $script .= 'printf "%s\t%s\n" ' . escapeshellarg($k) . ' "${' . $k . '-}"; '; + } + + $out = []; $rc = 0; + exec('bash -c ' . escapeshellarg($script) . ' 2>/dev/null', $out, $rc); + if ($rc !== 0) return null; + + $vals = []; + foreach ($out as $line) { + $parts = explode("\t", $line, 2); + if (count($parts) === 2) $vals[$parts[0]] = $parts[1]; + } + return $vals; +} + +// One line per conf change, best-effort and never able to block the write itself. +function vv_conf_audit(string $file, string $key, string $outcome, string $detail = ''): void { + $line = date('Y-m-d H:i:s') + . " file={$file} key={$key} outcome={$outcome}" + . ($detail !== '' ? " {$detail}" : '') + . ' ip=' . ($_SERVER['REMOTE_ADDR'] ?? 'cli') + . "\n"; + @file_put_contents(LOG_DIR . '/conf_changes.log', $line, FILE_APPEND | LOCK_EX); +} + // Write a batch of field changes back to their respective conf files. // Each change: {file, key, value, type} function vv_conf_write_changes(array $changes): array { $byFile = []; foreach ($changes as $c) { - if (!empty($c['file']) && !empty($c['key'])) $byFile[$c['file']][] = $c; + if (empty($c['file']) || empty($c['key'])) continue; + if (!vv_conf_key_valid($c['key'])) { + vv_conf_audit((string) $c['file'], (string) $c['key'], 'rejected', 'reason=malformed-key'); + continue; + } + if (!vv_conf_value_safe((string) ($c['value'] ?? ''))) { + vv_conf_audit((string) $c['file'], (string) $c['key'], 'rejected', 'reason=command-substitution'); + continue; + } + $byFile[$c['file']][] = $c; } $results = []; foreach ($byFile as $file => $fileChanges) { - $raw = vv_read_conf_raw($file); - if ($raw === '') { $results[$file] = false; continue; } + $results[$file] = vv_conf_write_file($file, $fileChanges); + } + return $results; +} +// Read-modify-write for one conf file. The surgical replacement lives here; the guards that +// make installing it safe live in vv_conf_install(), which every conf writer shares. +function vv_conf_write_file(string $file, array $fileChanges): bool { + return vv_conf_edit($file, function (string $raw) use ($fileChanges): ?string { foreach ($fileChanges as $c) { $qKey = preg_quote($c['key'], '/'); $value = $c['value']; @@ -328,7 +491,10 @@ function vv_conf_write_changes(array $changes): array { if ($type === 'scalar') { $raw = preg_replace_callback( '/^(\s*' . $qKey . '\s*=\s*)("(?:[^"\\\\]|\\\\.)*"|\'(?:[^\'\\\\]|\\\\.)*\'|[^#\n]*?)(\s*(?:#[^\n]*)?)$/m', - fn($m) => $m[1] . '"' . str_replace(['"', '\\'], ['\\"', '\\\\'], $value) . '"' . $m[3], + // Backslashes first. Escaping quotes first meant the backslash just inserted + // was itself doubled on the next pass — " became \\" — which closed the + // string early and stored a truncated value that still parsed cleanly. + fn($m) => $m[1] . '"' . str_replace(['\\', '"'], ['\\\\', '\\"'], $value) . '"' . $m[3], $raw ) ?? $raw; @@ -354,23 +520,154 @@ function vv_conf_write_changes(array $changes): array { ) ?? $raw; } } - // Only the scalar path escapes its value; the array, array_single and assoc_array - // paths splice the caller's text into the file verbatim, and the type comes from the - // request. Every script sources these files, so the result is parsed before it is - // allowed to replace a working conf. - $results[$file] = vv_conf_syntax_ok($raw) && vv_write_conf_raw($file, $raw); + + return $raw; + + // Scalars are verified because their intended value is known exactly. The array types splice + // caller-supplied text whose sourced form is legitimately not equal to what was written, so + // for those a clean source is the whole assertion. + }, vv_conf_expected_scalars($fileChanges), array_column($fileChanges, 'key')); +} + +// The intended end state for the keys that can be checked against one. +function vv_conf_expected_scalars(array $fileChanges): array { + $expect = []; + foreach ($fileChanges as $c) { + if (($c['type'] ?? 'scalar') === 'scalar') $expect[$c['key']] = (string) $c['value']; } - return $results; + return $expect; +} + +// The one guarded way to install a changed conf. $mutate receives the current contents and +// returns the rewritten ones, or null to abort without touching anything. Every conf writer in +// the plugin goes through here, so the lock, the backup, the validation and the audit trail are +// written once and cannot be forgotten by a new caller. +// +// $expect key => intended value, verified by sourcing the installed file +// $subjects names for the audit line, when there is no key to assert (a toggled member) +// $allowCreate write a conf that does not exist yet. Off by default: for every caller except +// first-run setup, a missing target means the filename is wrong, and creating it +// would leave a stray conf that shadows nothing and is sourced by nobody. +function vv_conf_edit(string $file, callable $mutate, array $expect = [], array $subjects = [], + bool $allowCreate = false): bool { + $path = CONF_DIR . '/' . $file; + $subjects = $subjects ?: (array_keys($expect) ?: ['-']); + + $audit = function (string $outcome, string $detail = '') use ($file, $subjects): void { + foreach ($subjects as $s) vv_conf_audit($file, (string) $s, $outcome, $detail); + }; + + // Held across the whole read-modify-write. Two concurrent savers would otherwise each read + // the same original, and the second rename would silently discard the first one's change. + $lockFh = @fopen(CONF_DIR . '/.conf-write.lock', 'c'); + if ($lockFh === false || !flock($lockFh, LOCK_EX)) { + if ($lockFh) fclose($lockFh); + $audit('failed', 'reason=lock'); + return false; + } + + try { + // A conf that does not exist yet is a create, not an edit — first-run setup writes + // host*.conf before there is anything to read. There is no prior content to lose, so + // there is nothing to back up, and undoing a failed create means removing the file. + $exists = is_file($path); + if (!$exists && !$allowCreate) { $audit('failed', 'reason=unreadable'); return false; } + $raw = $exists ? vv_read_conf_raw($file) : ''; + if ($exists && $raw === '') { $audit('failed', 'reason=unreadable'); return false; } + + $before = $exists ? (vv_conf_read_back($path, $subjects) ?? []) : []; + + $new = $mutate($raw); + if ($new === null) { $audit('failed', 'reason=no-match'); return false; } + if ($exists && $new === $raw) { $audit('no-change'); return true; } + + $backup = null; + if ($exists) { + $backup = vv_conf_backup($file); + if ($backup === null) { + // No recovery path for this write means the write does not happen. These files + // are gitignored, so a backup not taken cannot be reconstructed afterwards. + $audit('failed', 'reason=backup'); + return false; + } + } + + $undo = function () use ($backup, $path): void { + if ($backup !== null) @copy($backup, $path); else @unlink($path); + }; + + if (!vv_conf_syntax_ok($new)) { $audit('rejected', 'reason=syntax'); return false; } + if (!vv_write_conf_raw($file, $new)) { $audit('failed', 'reason=write'); return false; } + + // bash -n proved the candidate parses. This proves the installed file still sources and + // that each value arrived intact — a quoting bug produces a file that parses perfectly + // and holds the wrong string, which is the failure the syntax check cannot see. + $after = vv_conf_read_back($path, $subjects); + if ($after === null) { + $undo(); + $audit('rolled-back', 'reason=source-failed'); + return false; + } + + foreach ($expect as $key => $want) { + if (($after[$key] ?? null) !== $want) { + $undo(); + vv_conf_audit($file, $key, 'rolled-back', 'reason=value-mismatch'); + return false; + } + } + + foreach ($subjects as $s) { + $s = (string) $s; + // A subject that is not a conf key — a script id, or a marker for a whole-file save + // — has no value to read back, so there is no before and after to report. + if (!vv_conf_key_valid($s)) { vv_conf_audit($file, $s, 'applied'); continue; } + if (vv_conf_key_is_secret($s)) { vv_conf_audit($file, $s, 'applied', 'value='); continue; } + vv_conf_audit($file, $s, 'applied', + 'from=' . vv_conf_audit_val($before[$s] ?? '') . ' to=' . vv_conf_audit_val($after[$s] ?? '')); + } + + return true; + + } finally { + flock($lockFh, LOCK_UN); + fclose($lockFh); + } +} + +// Audit values are single-line and bounded. A conf value can be a 40-line array; the log is a +// record of what changed, and an unbounded splat of it makes the log unreadable at the moment +// it is actually needed. +function vv_conf_audit_val(string $v): string { + $v = preg_replace('/\s+/', ' ', trim($v)); + if (strlen($v) > 120) $v = substr($v, 0, 117) . '...'; + return '"' . $v . '"'; } // bash -n against a private temp copy. Returns true when the content parses as a sourceable // conf, false otherwise — never writes anything itself. function vv_conf_syntax_ok(string $content): bool { + return vv_conf_syntax_error($content) === null; +} + +// The same check, with bash's own complaint when it fails — the raw editors show it to whoever +// is typing, where "conf does not parse" alone would mean hunting the line by hand. $label +// replaces the temp path in the message so the reader sees their own filename. +// +// Fails closed. This used to pass when the temp file could not be created, which was defensible +// while every write came from a human clicking Save on a form. The assistant writes through here +// too, so an unverified conf is not installed — a refused write is recoverable, a conf that no +// script can source is a system-wide outage. +function vv_conf_syntax_error(string $content, string $label = 'conf'): ?string { $tmp = tempnam(sys_get_temp_dir(), 'vvconf'); - if ($tmp === false) return true; // cannot check — do not block the write + if ($tmp === false) return 'cannot verify: no writable temp directory'; + file_put_contents($tmp, $content); $out = []; $rc = 0; exec('bash -n ' . escapeshellarg($tmp) . ' 2>&1', $out, $rc); @unlink($tmp); - return $rc === 0; + if ($rc === 0) return null; + + $msg = implode(' ', array_filter(array_map('trim', $out))); + return str_replace($tmp, $label, $msg ?: 'conf does not parse'); } diff --git a/Plugin/unraid/include/config.php b/Plugin/unraid/include/config.php index 8bdafe1..9316a45 100644 --- a/Plugin/unraid/include/config.php +++ b/Plugin/unraid/include/config.php @@ -117,6 +117,8 @@ define('STATE_DIR', DATA_DIR . '/state'); define('AI_DATA_DIR', DATA_DIR . '/ai'); define('CACHE_BACKUP_DIR', DATA_DIR . '/cache'); define('LOG_ARCHIVE_DIR', DATA_DIR . '/logs'); +define('BACKUP_DIR', DATA_DIR . '/Backups'); +define('CONF_BACKUP_DIR', BACKUP_DIR . '/Confs'); define('LOG_DIR', '/var/log/varaverk'); // User-authored custom scripts (scheduler page "+ Create Script") — kept outside the git // repo entirely, alongside the User Scripts plugin's own storage. Any *.sh file placed diff --git a/data/README.md b/data/README.md index b3b5342..a408631 100644 --- a/data/README.md +++ b/data/README.md @@ -19,6 +19,8 @@ data/ ├── cache/ persistent backups of the tmpfs caches — and only those │ ├── arr/ *_tracked_cache.json, restored into tmpfs on demand │ └── conf/ partner host*.conf snapshot (0700 — holds credentials) +├── Backups/ point-in-time copies kept so a bad write can be undone +│ └── Confs/ pre-write copies of this host's own confs (0700 — holds credentials) └── logs/ retained log output ``` @@ -31,6 +33,8 @@ variable is what a storage-mode migration rewrites. | `state/` | `STATE_DIR` | ALL state files must use this. No `/tmp`, no repo root. | | `ai/` | `AI_DATA_DIR` | | | `cache/` | `CACHE_BACKUP_DIR` | `ARR_CACHE_BACKUP_DIR`, `PERSISTENT_CONF_CACHE` sit under it | +| `Backups/` | `BACKUP_DIR` | parent only; each kind of backup gets a subdirectory | +| `Backups/Confs/` | `CONF_BACKUP_DIR` | retained per `CONF_BACKUP_RETAIN`; **not** a cache — see below | | `logs/` | `LOG_ARCHIVE_DIR` | live logging still goes to `LOG_DIR` (`/var/log/varaverk`) | --- @@ -74,6 +78,11 @@ Ask what happens if it is deleted. called — `lidarr_art_miss_cache.tsv` has "cache" in its name and lives in `db/` for exactly that reason. +`Backups/` is the same trap from the other side: it has "backup" in its name but backs up nothing +that exists elsewhere. The confs are gitignored, so a pre-write copy under `Backups/Confs/` is the +only prior version of that file anywhere. Deleting it loses something permanently, which is why it +is a root of its own and not a subdirectory of `cache/`. + --- ## History