Refuse a conf write that widens a path to its own ancestor

Several scripts delete inside a conf path — the orphan cleaner runs rm -rf under a download dir
and rsync runs --delete against a destination — so /mnt/user/Movies becoming /mnt/user is the
edit that turns a cleanup into a sweep. Depth cannot be the test, because /tv and /movies are
real container-internal values here; direction can. Clearing a path, making it relative and
'..' segments go with it, and autofix additionally requires a proposed path to exist, since
every probe it has is a network probe and proves nothing about a directory. Refusals now reach
the caller: a save whose only change was refused answered ok with no explanation.
This commit is contained in:
Gmer4Lfe
2026-08-09 21:21:52 -04:00
parent 77fb1abb85
commit a5be719261
3 changed files with 112 additions and 5 deletions
+85 -3
View File
@@ -383,6 +383,69 @@ function vv_conf_value_safe(string $value): bool {
return !preg_match('/\$\(|`|<\(|>\(/', $value);
}
// ── Path values ──────────────────────────────────────────────────────────────────────────────
// A conf path is not just a string: several scripts delete inside one. arr_download_orphan_cleaner
// runs rm -rf on entries under a download dir, rsync.sh runs --delete against a profile's
// destination, and the transcode ramdisk tears down its own mount point. Those are correct
// against /mnt/user/Movies and catastrophic against /mnt/user, and nothing between the form and
// the file could previously tell those apart — bash -n proves the file parses, and the read-back
// proves the value arrived, but both are perfectly happy with a value that will erase a share.
//
// Depth is not the test. /tv, /movies and "/kids movies" are all real values in host1.conf: they
// are paths inside the arr containers, and a rule requiring two segments would refuse the conf
// this host already runs. What separates a safe path edit from a dangerous one is not how deep
// the new value is but which direction it moved — widening a path to its own ancestor is the
// edit that turns a targeted cleanup into a sweep, and it is meaningful at every depth.
//
// Scalars only. Arrays are spliced in verbatim by a different branch of the writer, and the arr
// root-folder maps that live in them deserve their own pass; what matters here is that every
// value the repair subsystem writes is a scalar, so its whole surface is covered.
//
// Returns null when the write is allowed, or the reason it is not.
function vv_conf_path_write_ok(string $key, string $new, ?string $old): ?string {
$isPath = fn(?string $v): bool => $v !== null && $v !== '' && $v[0] === '/';
// A ${VAR} reference is the established idiom for composing paths here and cannot be
// resolved statically, so it is left to the read-back the way it always was.
if ($new !== '' && $new[0] === '$') return null;
// Not a path write at all unless one side of it is a path.
if (!$isPath($new) && !$isPath($old)) return null;
// A newline inside double quotes is legal bash, so this is one of the few malformations
// bash -n cannot catch. A path carrying one arrives at rsync or rm as two arguments.
if (preg_match('/[\r\n\x00]/', $new)) return 'newline-in-path';
// Blanking a path is the classic: rm -rf "$DIR"/* with an empty DIR is rm -rf /*. The raw
// conf editor is the way to empty one deliberately — this writer is the automated path.
if ($isPath($old) && $new === '') return 'path-cleared';
// Was a path, is now something that resolves against whatever directory the script happens
// to be in. There is no safe answer to "delete inside Movies" when Movies is relative.
if ($isPath($old) && !$isPath($new)) return 'path-became-relative';
if (!$isPath($new)) return null;
// '..' as a whole segment. Not a substring match — a share legitimately named "..old" is not
// traversal, and a rule that cannot tell them apart teaches people to work around it.
foreach (explode('/', $new) as $seg) {
if ($seg === '..') return 'path-traversal';
}
$norm = fn(string $p): string => rtrim(preg_replace('#/+#', '/', $p), '/');
$n = $norm($new);
if ($n === '') return 'path-is-root';
// The rule this whole function exists for. An ancestor is matched at the segment boundary so
// /mnt/user/Movies2 is not read as a parent of /mnt/user/Movies.
if ($isPath($old)) {
$o = $norm((string)$old);
if ($o !== $n && str_starts_with($o . '/', $n . '/')) return 'path-broadened';
}
return null;
}
// 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.
@@ -459,18 +522,37 @@ function vv_conf_audit(string $file, string $key, string $outcome, string $detai
// 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 {
//
// $rejected collects what was refused before any file was touched. A rejected change is dropped
// from the batch, so a request whose only change is refused produces no results at all — and
// "no results" and "nothing went wrong" were indistinguishable to a caller checking for false.
// The settings form reported a save that had silently not happened. Callers that care pass this
// and say so; the two that write assoc arrays are unaffected and do not.
function vv_conf_write_changes(array $changes, array &$rejected = []): array {
$rejected = [];
$refuse = function (array $c, string $reason) use (&$rejected): void {
vv_conf_audit((string) $c['file'], (string) $c['key'], 'rejected', 'reason=' . $reason);
$rejected[] = ['file' => (string) $c['file'], 'key' => (string) $c['key'], 'reason' => $reason];
};
$byFile = [];
foreach ($changes as $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');
$refuse($c, 'malformed-key');
continue;
}
if (!vv_conf_value_safe((string) ($c['value'] ?? ''))) {
vv_conf_audit((string) $c['file'], (string) $c['key'], 'rejected', 'reason=command-substitution');
$refuse($c, 'command-substitution');
continue;
}
// Compared against the value the conf holds now, which is what makes "broadened" a
// question this can answer at all. Scalars only — see vv_conf_path_write_ok().
if (($c['type'] ?? 'scalar') === 'scalar') {
$bad = vv_conf_path_write_ok((string) $c['key'], (string) ($c['value'] ?? ''),
vv_conf_vars()[$c['key']] ?? null);
if ($bad !== null) { $refuse($c, $bad); continue; }
}
$byFile[$c['file']][] = $c;
}