fix(manifest): reject unsafe paths and links

TODO.md

@@ -2,7 +2,7 @@
 
 ## Data loss and security
 
-- [ ] Validate every path read from `.ai-config-manifest.tsv` and reject links before backup, overwrite, or removal. Both manifest writers join unchecked recorded paths to the destination; a `../` entry or linked target can reach files outside the managed directory. (`scripts/lib/manifest.ps1:20`, `scripts/lib/manifest.ps1:136`, `scripts/lib/manifest.sh:20`, `scripts/lib/manifest.sh:170`)
+- [x] Validate every path read from `.ai-config-manifest.tsv` and reject links before backup, overwrite, or removal. Both manifest writers join unchecked recorded paths to the destination; a `../` entry or linked target can reach files outside the managed directory. (`scripts/lib/manifest.ps1:20`, `scripts/lib/manifest.ps1:136`, `scripts/lib/manifest.sh:20`, `scripts/lib/manifest.sh:170`)
 - [x] Keep an installed semantic retrieval runtime when an `auto` benchmark says no. `benchmark()` deletes the whole runtime and ownership state even when `state['clients']` still contains active clients. (`scripts/lib/semantic-retrieval.py:117`, `scripts/lib/semantic-retrieval.py:142`)
 - [ ] Check semantic retrieval ownership and links before copying or deleting its directory. An existing owned runtime's source files are overwritten without comparing recorded hashes; a replaced target symlink is followed on enable, and disabling the last client removes foreign files inside the runtime. (`scripts/lib/semantic-retrieval.py:82`, `scripts/lib/semantic-retrieval.py:100`)
 - [ ] Preserve pre-existing MCPorter server entries. `ensure_mcporter()` adds a server and claims ownership without checking existing configuration; if the add succeeds for an existing name, a later removal deletes that user's entry. (`scripts/lib/managed-extensions.py:395`, `scripts/lib/managed-extensions.py:263`)

scripts/lib/manifest.ps1

@@ -3,6 +3,24 @@ function Get-ManagedManifestPath {
     return (Join-Path $Destination '.ai-config-manifest.tsv')
 }
 
+function Resolve-ManagedPath {
+    param([Parameter(Mandatory=$true)][string]$Destination, [Parameter(Mandatory=$true)][string]$Relative)
+    $parts = $Relative -split '/', 0, 'SimpleMatch'
+    if ($Relative.StartsWith('/') -or $Relative.Contains('\') -or $Relative.Contains(':') -or $Relative.Contains("`t") -or $Relative.Contains("`r") -or $Relative.Contains("`n") -or @($parts | Where-Object { $_ -in @('', '.', '..') }).Count -gt 0) {
+        throw "Invalid managed path: $Relative"
+    }
+    $target = $Destination
+    foreach ($part in $parts) { $target = Join-Path $target $part }
+    $probe = $target
+    while ($true) {
+        $item = Get-Item -LiteralPath $probe -Force -ErrorAction SilentlyContinue
+        if ($item -and ($item.Attributes -band [IO.FileAttributes]::ReparsePoint)) { throw "Managed path contains a link: $probe" }
+        if ($probe -eq $Destination) { break }
+        $probe = Split-Path $probe -Parent
+    }
+    return $target
+}
+
 function Get-Sha256Hash {
     param([Parameter(Mandatory=$true)][string]$Path)
     # Lowercase to match sha256sum's output, since the manifest must be readable by
@@ -22,7 +40,10 @@ function Read-ManagedManifest {
     $entries = @{}
     if (Test-Path -LiteralPath $ManifestPath) {
         # Lowercase to tolerate an older manifest written before hashes were normalized.
-        Import-Csv -LiteralPath $ManifestPath -Delimiter ([char]9) | ForEach-Object { $entries[$_.path] = $_.sha256.ToLowerInvariant() }
+        Import-Csv -LiteralPath $ManifestPath -Delimiter ([char]9) | ForEach-Object {
+            $null = Resolve-ManagedPath -Destination (Split-Path $ManifestPath -Parent) -Relative $_.path
+            $entries[$_.path] = $_.sha256.ToLowerInvariant()
+        }
     }
     return $entries
 }
@@ -63,6 +84,7 @@ function Sync-ManagedDestination {
     )
     if (-not $Summary) { Write-Output "SOURCE $Source -> $Destination" }
     $manifestPath = Get-ManagedManifestPath $Destination
+    $null = Resolve-ManagedPath -Destination $Destination -Relative '.ai-config-manifest.tsv'
     $oldManifest = Read-ManagedManifest $manifestPath
     $newManifest = @{}
     $backupRoot = Join-Path $Destination "backups/$Stamp"
@@ -71,7 +93,7 @@ function Sync-ManagedDestination {
     Get-ChildItem $Source -File -Recurse | ForEach-Object {
         $relative = $_.FullName.Substring($Source.Length).TrimStart([char[]]@('\','/')).Replace('\','/')
         if ((Split-Path $Destination -Leaf) -eq '.codex' -and $relative.StartsWith('skills/')) { return }
-        $target = Join-Path $Destination $relative
+        $target = Resolve-ManagedPath -Destination $Destination -Relative $relative
         $rawContent = Get-Content -LiteralPath $_.FullName -Raw -Encoding UTF8
         $filterFlashbang = -not $FlashbangEnabled -and $relative -in @('settings.json', 'config.toml')
         if ($filterFlashbang) {
@@ -121,7 +143,7 @@ function Sync-ManagedDestination {
         if ($DryRun) { if (-not $Summary) { Write-Output "DRYRUN $action $target" }; if ($action -eq 'CREATE') { $tally.Created++ } else { $tally.Updated++ }; return }
         if ($exists) {
             $wasManaged = $oldManifest.ContainsKey($relative)
-            $backup = Join-Path $backupRoot $relative
+            $backup = Resolve-ManagedPath -Destination $Destination -Relative "backups/$Stamp/$relative"
             New-Item (Split-Path $backup -Parent) -ItemType Directory -Force | Out-Null
             Copy-Item -LiteralPath $target $backup -Force
             if (-not $Summary) { Write-Output "BACKUP $target -> $backup" }
@@ -134,10 +156,10 @@ function Sync-ManagedDestination {
     }
 
     foreach ($relative in @($oldManifest.Keys | Where-Object { -not $newManifest.ContainsKey($_) })) {
-        $target = Join-Path $Destination $relative
+        $target = Resolve-ManagedPath -Destination $Destination -Relative $relative
         if (-not (Test-Path -LiteralPath $target)) { continue }
         if ($DryRun) { if (-not $Summary) { Write-Output "DRYRUN REMOVE $target" }; $tally.Removed++; continue }
-        $backup = Join-Path $backupRoot $relative
+        $backup = Resolve-ManagedPath -Destination $Destination -Relative "backups/$Stamp/$relative"
         New-Item (Split-Path $backup -Parent) -ItemType Directory -Force | Out-Null
         Copy-Item -LiteralPath $target $backup -Force
         if (-not $Summary) { Write-Output "BACKUP $target -> $backup" }

scripts/lib/manifest.sh

@@ -16,6 +16,24 @@ manifest_path() {
   printf '%s/.ai-config-manifest.tsv' "$1"
 }
 
+managed_target() {
+  local destination=$1 relative=$2 component target=$destination probe
+  [[ -n "$relative" && "$relative" != /* && "$relative" != */ && "$relative" != *//* && "$relative" != *\\* && "$relative" != *:* && "$relative" != *$'\t'* && "$relative" != *$'\r'* && "$relative" != *$'\n'* ]] || { printf 'Invalid managed path: %s\n' "$relative" >&2; return 1; }
+  local -a parts
+  IFS=/ read -r -a parts <<< "$relative"
+  for component in "${parts[@]}"; do
+    [[ -n "$component" && "$component" != . && "$component" != .. ]] || { printf 'Invalid managed path: %s\n' "$relative" >&2; return 1; }
+    target+=/$component
+  done
+  probe=$target
+  while :; do
+    [ ! -L "$probe" ] || { printf 'Managed path contains a link: %s\n' "$probe" >&2; return 1; }
+    [ "$probe" != "$destination" ] || break
+    probe=$(dirname -- "$probe")
+  done
+  printf '%s' "$target"
+}
+
 # read_managed_manifest <manifest-file> <assoc-array-name>
 read_managed_manifest() {
   local path=$1
@@ -31,6 +49,7 @@ read_managed_manifest() {
       [ "$rel" = path ] && continue
     fi
     [ -n "$rel" ] || continue
+    managed_target "$(dirname -- "$path")" "$rel" > /dev/null || return 1
     hash=$(printf '%s' "$hash" | tr '[:upper:]' '[:lower:]') # tolerate uppercase hashes from older manifests
     # shellcheck disable=SC2034
     out_ref[$rel]=$hash
@@ -57,8 +76,9 @@ sync_managed_destination() {
   [ "$summary" = true ] || printf 'SOURCE %s -> %s\n' "$source" "$destination"
   local manifest
   manifest=$(manifest_path "$destination")
+  managed_target "$destination" '.ai-config-manifest.tsv' > /dev/null || return 1
   declare -A old_manifest
-  read_managed_manifest "$manifest" old_manifest
+  read_managed_manifest "$manifest" old_manifest || return 1
   declare -A new_manifest
   local backup_root="$destination/backups/$stamp"
   local created=0 updated=0 unchanged=0 removed=0 warned=0
@@ -67,7 +87,7 @@ sync_managed_destination() {
     local relative target content needs_sub new_hash exists current_hash action backup
     relative=${source_file#"$source/"}
     if [ "${destination##*/}" = .codex ] && [[ "$relative" = skills/* ]]; then continue; fi
-    target="$destination/$relative"
+    target=$(managed_target "$destination" "$relative") || return 1
     content=$(cat -- "$source_file" && printf '\034') || return
     content=${content%$'\034'}
     needs_sub=false
@@ -150,7 +170,7 @@ sync_managed_destination() {
     fi
 
     if [ "$exists" = true ]; then
-      backup="$backup_root/$relative"
+      backup=$(managed_target "$destination" "backups/$stamp/$relative") || return 1
       mkdir -p "$(dirname -- "$backup")"
       cp -- "$target" "$backup"
       [ "$summary" = true ] || printf 'BACKUP %s -> %s\n' "$target" "$backup"
@@ -172,14 +192,14 @@ sync_managed_destination() {
   local relative target current_hash backup
   for relative in "${!old_manifest[@]}"; do
     [ -z "${new_manifest[$relative]+x}" ] || continue
-    target="$destination/$relative"
+    target=$(managed_target "$destination" "$relative") || return 1
     [ -f "$target" ] || continue
     if [ "$dry_run" = true ]; then
       [ "$summary" = true ] || printf 'DRYRUN REMOVE %s\n' "$target"
       removed=$((removed + 1))
       continue
     fi
-    backup="$backup_root/$relative"
+    backup=$(managed_target "$destination" "backups/$stamp/$relative") || return 1
     mkdir -p "$(dirname -- "$backup")"
     cp -- "$target" "$backup"
     [ "$summary" = true ] || printf 'BACKUP %s -> %s\n' "$target" "$backup"