From 94ed8a0f8f0477bedfdca0c35c82347122011a39 Mon Sep 17 00:00:00 2001
From: AJ Isaacs <ajisaacs27@gmail.com>
Date: Sun, 11 Oct 2026 17:10:26 -0400
Subject: [PATCH] fix(backup): refuse uncertain or reused restore volumes

---
 docs/nest-storage.md                         | 22 ++++++++++++++------
 scripts/Server.Tests/test_revision_backup.py | 11 +++++++++-
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/docs/nest-storage.md b/docs/nest-storage.md
index 2192c6a..f8e3550 100644
--- a/docs/nest-storage.md
+++ b/docs/nest-storage.md
@@ -146,9 +146,15 @@ or guarantee of future disk capacity.
 
 Revision preconditions refuse stale edits; the database monitor is not a document
 edit lock. After a conflict, explicitly reload the newer revision or Save As a
-separate nest. After an ambiguous network failure, fetch current metadata before
-retrying; never blindly replay a write. Legacy desktops may read but must upgrade
-before updating/deleting. A version-aware client refuses writes to a legacy server.
+separate nest. After an ambiguous update failure, reload the current version before another save.
+For an ambiguous first upload (including Save As or a server switch), OpenNest
+keeps the client-generated pending ID and blocks another upload. The next Save
+checks that exact ID on the original server without writing: if it exists with
+the exact attempted archive and metadata, the document binds to its revision;
+if it is absent, a further explicit Save may retry with the same ID. If it
+differs, reopen the server record to review it; do not discard the pending
+identity or blindly create another record. Older servers remain read-only for
+existing nests until upgraded.
 
 ## Server integration tests
 
@@ -336,10 +342,14 @@ restore_check() (  # usage: restore_check opennest-data-YYYY-MM-DD <image> [new-
   fail() { echo "STOP: $*" >&2; exit 1; }
   [[ -r "$1.tar" && -r "$1.manifest" && -r "$1.metadata.json" ]] || fail "backup is incomplete"
   restore=${3:-opennest-data-restore-$(date +%Y%m%d-%H%M%S)}
-  ! docker volume inspect "$restore" > /dev/null 2>&1 || fail "volume $restore already exists"
-  docker volume create --label "com.opennest.restore.owner=$restore" "$restore" > /dev/null \
+  existing=$(docker volume ls --format '{{.Name}}') || fail "could not inspect Docker volumes"
+  while IFS= read -r volume; do
+    [[ "$volume" != "$restore" ]] || fail "volume $restore already exists"
+  done <<< "$existing"
+  owner=$(python3 -c 'import secrets; print(secrets.token_hex(16))') || fail "could not create ownership token"
+  docker volume create --label "com.opennest.restore.owner=$owner" "$restore" > /dev/null \
     || fail "could not create volume $restore"
-  [[ $(docker volume inspect --format '{{index .Labels "com.opennest.restore.owner"}}' "$restore") == "$restore" ]] \
+  [[ $(docker volume inspect --format '{{index .Labels "com.opennest.restore.owner"}}' "$restore") == "$owner" ]] \
     || fail "restore volume ownership differs"
   echo "Restoring into new volume $restore"
   docker run --rm -i --network none --entrypoint tar -v "$restore:/app/data" "$2" \
diff --git a/scripts/Server.Tests/test_revision_backup.py b/scripts/Server.Tests/test_revision_backup.py
index ee661e4..616c5dc 100644
--- a/scripts/Server.Tests/test_revision_backup.py
+++ b/scripts/Server.Tests/test_revision_backup.py
@@ -215,7 +215,7 @@ class DocumentationTests(unittest.TestCase):
             for volume in self.volumes:
                 labels = json.loads(self.docker('volume', 'inspect', '--format', '{{json .Labels}}', volume).stdout)
                 self.assertTrue(labels.get('com.opennest.doc-owner') == self.token
-                                or labels.get('com.opennest.restore.owner') == volume and volume.startswith(self.token + '-'))
+                                or bool(labels.get('com.opennest.restore.owner')) and volume.startswith(self.token + '-'))
                 self.assertEqual('', self.docker('container', 'ls', '-aq', '--filter', 'volume=' + volume).stdout.strip(),
                                  'A documentation check container was not removed')
                 self.docker('volume', 'rm', volume)
@@ -236,6 +236,15 @@ class DocumentationTests(unittest.TestCase):
         restored = self.token + '-restored'
         refused = self.procedure(f'if restore_check backup {self.image} {self.volume}; then exit 90; fi')
         self.assertIn('already exists', refused.stderr)
+        refused = self.procedure(
+            f'docker() {{ if [[ "$1 $2" == "volume ls" ]]; then return 47; fi; command docker "$@"; }}; '
+            f'if restore_check backup {self.image} {self.token}-lookup-failed; then exit 90; fi')
+        self.assertIn('could not inspect Docker volumes', refused.stderr)
+        raced = self.procedure(
+            f'docker() {{ if [[ "$1 $2" == "volume ls" ]]; then return 0; fi; command docker "$@"; }}; '
+            f'if restore_check backup {self.image} {self.volume}; then exit 90; fi')
+        self.assertIn('ownership differs', raced.stderr)
+        self.assertEqual(started, self.docker('inspect', '--format', '{{.State.StartedAt}}', self.cid).stdout)
         self.volumes.append(restored)
         self.procedure(f'restore_check backup {self.image} {restored}')
         tampered = self.token + '-tampered'