Clear the heavier of what the audit left

An export wrote its temporary to a fixed name -- the target with ".tmp" on the
end. That is only safe while one writer exists at a time, and there is no such
guarantee: the window's export lock is in-process, so a command line export and
the window together, or two exports into one world, opened the same file with
O_TRUNC, interleaved their bytes, and both renamed the mixture over a region
file in somebody's save. The name is unique now and the contents are forced to
disk before the rename, because a region file that is a rename away from
complete but whose bytes never landed is a corrupt chunk rather than a missing
one. Checked on a copy of a real world: 6 chunks written, 374 kept, 552 before
and after, no temporary left behind.

Nothing stopped a RuntimeException reaching Minecraft. The mixins inject at the
tail of vanilla's packet handlers and the event registrations run inside the
client's own tick and connection handling, so anything escaping unwound into
code that has no idea what this mod is and took the client with it -- and
onBlockApplied, which calls into the level and two caches, had no guard at all.
The seam is the right place for one, so every dispatch point goes through it and
any future one will too. Errors are not caught: an OutOfMemoryError is not this
mod's to absorb, and pretending to carry on after one hides the only evidence.

A stray file in transactions/ made every Open fail forever, from every command,
with no way to reach fsck to find out why. A desktop.ini, a .DS_Store or a cloud
sync conflict copy was enough, and none of them says anything about whether the
transactions are sound. A file this package did not write is not a transaction.

transfer.Receive read a peer's bundle.json whole into memory whatever its size,
while internal/bundle caps four dimensions on the adapter path -- which is the
less untrusted of the two, since those bundles were written by a mod on the same
machine. Both it and each record are bounded now.

Three screens reported a failure as an absence. An unreadable recordings folder
came back as zero counts, so the play screen said "Nothing new since last time"
about a directory it had failed to open, while the import screen given the same
directory said so honestly. A failed list of moments read as "Nothing recorded
for this server. Play and bring in some recordings" -- advice for a situation
that was not theirs, about an answer that never arrived -- and on the world
screen the moment chooser simply vanished, taking the one thing that makes this
different from a world downloader with it. And a failed import left its button
disabled and still reading "Bringing it in...", because it lives in the screen's
footer and nothing else put it back.

Author: TonyLu786

adapters/fabric/src/client/java/org/worldledger/fabric/WorldLedgerRuntime.java

@@ -44,7 +44,7 @@ public static void initialize(CapturePaths paths) {
 	public static void onFullChunkPacket(ClientboundLevelChunkWithLightPacket packet) {
 		CaptureCoordinator coordinator = COORDINATOR.get();
 		if (coordinator != null) {
-			coordinator.onFullChunkPacket(packet);
+			guard("a chunk packet", () -> coordinator.onFullChunkPacket(packet));
 		}
 	}
 
@@ -70,30 +70,59 @@ public static String reload() {
 	public static void onBlockUpdate(ClientboundBlockUpdatePacket packet) {
 		CaptureCoordinator coordinator = COORDINATOR.get();
 		if (coordinator != null) {
-			coordinator.onBlockApplied(packet.getPos());
+			guard("a block update", () -> coordinator.onBlockApplied(packet.getPos()));
 		}
 	}
 
 	public static void onSectionBlocksUpdate(ClientboundSectionBlocksUpdatePacket packet) {
 		CaptureCoordinator coordinator = COORDINATOR.get();
 		if (coordinator != null) {
-			packet.runUpdates((position, state) -> coordinator.onBlockApplied(position));
+			guard("a section block update", () -> packet.runUpdates((position, state) -> coordinator.onBlockApplied(position)));
 		}
 	}
 
 	public static void onBlockEntityData(ClientboundBlockEntityDataPacket packet) {
 		CaptureCoordinator coordinator = COORDINATOR.get();
 		if (coordinator != null) {
-			coordinator.onBlockEntityPacket(packet);
+			guard("a block entity packet", () -> coordinator.onBlockEntityPacket(packet));
 		}
 	}
 
 	public static void onBiomeUpdate(ClientboundChunksBiomesPacket packet) {
 		CaptureCoordinator coordinator = COORDINATOR.get();
 		if (coordinator != null) {
-			for (ClientboundChunksBiomesPacket.ChunkBiomeData data : packet.chunkBiomeData()) {
-				coordinator.onBiomeApplied(data.pos().x(), data.pos().z());
-			}
+			guard("a biome update", () -> {
+				for (ClientboundChunksBiomesPacket.ChunkBiomeData data : packet.chunkBiomeData()) {
+					coordinator.onBiomeApplied(data.pos().x(), data.pos().z());
+				}
+			});
+		}
+	}
+
+	/**
+	 * Runs one piece of capture work, and never lets it reach Minecraft.
+	 *
+	 * <p>This class is the seam between the game and this mod: the mixins inject
+	 * at the tail of vanilla's own packet handlers, and the event registrations
+	 * below run inside the client's tick and connection handling. Anything that
+	 * escapes from here unwinds into code that has no idea what this mod is, and
+	 * takes the client down with it.
+	 *
+	 * <p>Nothing this mod does is worth that. Capture is a passenger: it records
+	 * what the client was shown, and a session that records nothing is a session
+	 * that recorded nothing. A player losing their connection -- or their
+	 * afternoon -- because a chunk could not be read is the one outcome that is
+	 * worse than not capturing at all.
+	 *
+	 * <p>Errors are not caught. An OutOfMemoryError or a linkage failure is not
+	 * this mod's to absorb, and pretending to carry on after one would hide the
+	 * only evidence of what happened.
+	 */
+	private static void guard(String what, Runnable body) {
+		try {
+			body.run();
+		} catch (RuntimeException exception) {
+			LOGGER.error("Capture failed during {}; the client is unaffected", what, exception);
 		}
 	}
 
@@ -103,45 +132,45 @@ private static void registerEvents() {
 			PENDING_JOIN.set(pending);
 			CaptureCoordinator coordinator = COORDINATOR.get();
 			if (coordinator != null && PENDING_JOIN.compareAndSet(pending, null)) {
-				coordinator.onJoin(client);
+				guard("joining a server", () -> coordinator.onJoin(client));
 			}
 		});
 		ClientPlayConnectionEvents.DISCONNECT.register((handler, client) -> {
 			CONNECTION_EPOCH.incrementAndGet();
 			PENDING_JOIN.set(null);
 			CaptureCoordinator coordinator = COORDINATOR.get();
 			if (coordinator != null) {
-				coordinator.onDisconnect();
+				guard("leaving a server", coordinator::onDisconnect);
 			}
 		});
 		ClientLevelEvents.AFTER_CLIENT_LEVEL_CHANGE.register((client, level) -> {
 			CaptureCoordinator coordinator = COORDINATOR.get();
 			if (coordinator != null) {
-				coordinator.onLevelChange(level);
+				guard("a dimension change", () -> coordinator.onLevelChange(level));
 			}
 		});
 		ClientChunkEvents.CHUNK_LOAD.register((level, chunk) -> {
 			CaptureCoordinator coordinator = COORDINATOR.get();
 			if (coordinator != null) {
-				coordinator.onChunkLoad(level, chunk);
+				guard("a chunk loading", () -> coordinator.onChunkLoad(level, chunk));
 			}
 		});
 		ClientChunkEvents.CHUNK_UNLOAD.register((level, chunk) -> {
 			CaptureCoordinator coordinator = COORDINATOR.get();
 			if (coordinator != null) {
-				coordinator.onChunkUnload(level, chunk);
+				guard("a chunk unloading", () -> coordinator.onChunkUnload(level, chunk));
 			}
 		});
 		ClientTickEvents.END_CLIENT_TICK.register(client -> {
 			CaptureCoordinator coordinator = COORDINATOR.get();
 			if (coordinator != null) {
-				coordinator.onEndTick();
+				guard("the end of a tick", coordinator::onEndTick);
 			}
 		});
 		ClientLifecycleEvents.CLIENT_STOPPING.register(client -> {
 			CaptureCoordinator coordinator = COORDINATOR.get();
 			if (coordinator != null) {
-				coordinator.onClientStopping();
+				guard("the client stopping", coordinator::onClientStopping);
 			}
 		});
 	}
@@ -158,7 +187,9 @@ private static void bootstrap(CapturePaths paths) {
 			if (pendingJoin != null) {
 				pendingJoin.client().execute(() -> {
 					if (CONNECTION_EPOCH.get() == pendingJoin.epoch() && COORDINATOR.get() == coordinator) {
-						coordinator.onJoin(pendingJoin.client());
+						// This runs on the client thread, so it needs the same
+						// guard as the join it stands in for.
+						guard("joining a server", () -> coordinator.onJoin(pendingJoin.client()));
 					}
 				});
 			}

desktop/internal/api/status.go

@@ -84,6 +84,9 @@ type SpoolState struct {
 	// folder anyway, so the page can say the recordings still exist rather than
 	// leaving somebody to assume they were consumed.
 	Imported int `json:"imported"`
+	// Unreadable carries why the folder could not be read, when it could not be.
+	// Empty on the ordinary path.
+	Unreadable string `json:"unreadable,omitempty"`
 	// ImportedBytes is what those are costing inside the Minecraft directory.
 	//
 	// Keeping them is the safe default and stays the default. Not saying what
@@ -179,7 +182,12 @@ func readSpoolState() *SpoolState {
 		if os.IsNotExist(err) {
 			return nil
 		}
-		return &SpoolState{Dir: dir}
+		// A folder that cannot be read is not an empty folder. Returning zero
+		// counts made the play screen say "Nothing new since last time" about a
+		// folder it had failed to open -- while the import screen, given the
+		// same folder, said so honestly. Two screens disagreeing about one
+		// directory, and the reassuring one was wrong.
+		return &SpoolState{Dir: dir, Unreadable: err.Error()}
 	}
 	return &SpoolState{
 		Dir:           dir,

desktop/ui/assets/app.js

@@ -342,7 +342,12 @@ function renderCapture(status) {
   // or whose launcher replaced the mods folder, still has every recording they
   // ever made sitting here, and reading the folder's existence as "you are
   // recording" told them to go and play while nothing was being kept.
-  if (!status.capturing) {
+  if (status.spool.unreadable) {
+    // Said before anything else, because every number below it is zero for a
+    // reason that has nothing to do with what was recorded.
+    host.append(banner('todo', 'The recordings folder could not be read',
+      status.spool.unreadable + ' — nothing here is a count of what you have.'));
+  } else if (!status.capturing) {
     host.append(banner('todo', 'Nothing would be recorded if you played now',
       'Your past recordings are safe and listed below. Go to Set up to put the mod back.'));
   } else if (waiting > 0) {
@@ -508,6 +513,12 @@ document.getElementById('import-run').addEventListener('click', async () => {
       host.append(list);
     }
   } catch (err) {
+    // The button lives in the screen's footer, outside the body this replaces,
+    // so nothing else puts it back. Leaving it disabled and still reading
+    // "Bringing it in…" told somebody it was working underneath an error
+    // message, and only leaving the screen and returning ever fixed it.
+    button.disabled = false;
+    button.textContent = 'Try again';
     problem(host, err);
   }
 });
@@ -736,13 +747,24 @@ function whenText(iso) {
   });
 }
 
+// The last error from momentsFor, so a failure can be told apart from an empty
+// archive. Swallowing it made a damaged archive read as "Nothing recorded for
+// this server. Play and bring in some recordings." -- advice for a situation
+// that was not the one they were in, about an answer the application had never
+// received.
+let momentsFailure = null;
+
 async function momentsFor(server) {
+  momentsFailure = null;
   if (!server) return [];
   try {
     return (await call('/api/moments?server=' + encodeURIComponent(server) +
       '&dimension=' + encodeURIComponent(chosenDimension))).moments || [];
   } catch (err) {
-    // A missing list of moments costs the choice of one, not the screen.
+    // A failed list of moments costs the choice of one, not the screen. What it
+    // must not cost is the difference between "there are none" and "we could
+    // not find out".
+    momentsFailure = err;
     return [];
   }
 }
@@ -800,6 +822,14 @@ async function refreshWorld() {
       scope.append(field);
     }
     host.append(scope);
+    // Without this the chooser simply was not there, and the one thing that
+    // makes this different from a world downloader disappeared with no
+    // explanation. It still writes: "now" is a moment.
+    if (momentsFailure) {
+      host.append(banner('todo',
+        'The list of moments could not be read: ' + momentsFailure.message,
+        (momentsFailure.next || '') + ' You can still write the newest of everything recorded.'));
+    }
 
     const answer = await call('/api/worlds');
     if (!answer.worlds.length) {
@@ -952,6 +982,10 @@ async function refreshTravel() {
 
     const server = chosenServer;
     const moments = await momentsFor(server);
+    if (momentsFailure) {
+      problem(host, momentsFailure);
+      return;
+    }
     if (moments.length < 1) {
       host.append(banner('todo', 'Nothing recorded for ' + server, 'Play and bring in some recordings.'));
       return;

internal/anvil/export.go

@@ -360,13 +360,42 @@ func readObject(source ObjectSource, ref model.BlobRef) ([]byte, error) {
 	return data, nil
 }
 
+// writeFileAtomic replaces a file in one step, so a reader never sees half of
+// one.
+//
+// The temporary name is unique rather than the target's with ".tmp" on the end.
+// A fixed name is only safe while one writer exists at a time, and there is no
+// such guarantee here: the desktop application's export lock is in-process, so a
+// command line export and the window running together -- or two exports into the
+// same world -- opened the same temporary with O_TRUNC, interleaved their bytes,
+// and both renamed the mixture over a region file in somebody's save.
+//
+// The contents are forced to disk before the rename. This writes into a world
+// the player will open in Minecraft, and a region file that is a rename away
+// from complete but whose bytes never landed is a corrupt chunk rather than a
+// missing one.
 func writeFileAtomic(path string, data []byte) error {
-	temporary := path + ".tmp"
-	if err := os.WriteFile(temporary, data, 0o644); err != nil {
+	handle, err := os.CreateTemp(filepath.Dir(path), ".worldledger-tmp-*")
+	if err != nil {
+		return err
+	}
+	temporary := handle.Name()
+	if _, err := handle.Write(data); err != nil {
+		handle.Close()
+		os.Remove(temporary)
+		return err
+	}
+	if err := handle.Sync(); err != nil {
+		handle.Close()
+		os.Remove(temporary)
+		return err
+	}
+	if err := handle.Close(); err != nil {
+		os.Remove(temporary)
 		return err
 	}
 	if err := os.Rename(temporary, path); err != nil {
-		_ = os.Remove(temporary)
+		os.Remove(temporary)
 		return err
 	}
 	return nil

internal/archive/archive.go

@@ -257,7 +257,15 @@ func (a Archive) recoverTransactions() error {
 			continue
 		}
 		if filepath.Ext(entry.Name()) != ".json" {
-			return fmt.Errorf("unexpected transaction file %q", entry.Name())
+			// Refusing here refuses to open the archive at all, from every
+			// command, with no way to reach fsck to find out why. A desktop.ini,
+			// a .DS_Store, or a cloud sync service's conflict copy is enough --
+			// none of which says anything about whether the transactions are
+			// sound, and none of which anybody could act on from the message.
+			//
+			// A file this package did not write is not a transaction, so it is
+			// not replayed and not treated as damage.
+			continue
 		}
 		if err := a.recoverTransaction(filepath.Join(dir, entry.Name())); err != nil {
 			return err

internal/transfer/transfer.go

@@ -293,8 +293,40 @@ type Received struct {
 // own identity rules reject a record whose id does not match its contents. A
 // bundle from an untrusted peer therefore cannot introduce anything the archive
 // would not have accepted from its own adapter.
+// Sizes a peer does not get to choose.
+//
+// internal/bundle caps four dimensions on the adapter path, which is the less
+// untrusted of the two: the bundles it reads were written by a mod on the same
+// machine. This path reads a directory somebody else assembled and had no caps
+// at all, so a peer's bundle.json was read whole into memory whatever its size.
+// The numbers are generous against any real bundle and finite against a hostile
+// one.
+const (
+	maxTransferManifestBytes = 8 << 20
+	maxRecordBytes           = 1 << 20
+)
+
+func readAtMost(path string, max int64) ([]byte, error) {
+	handle, err := os.Open(path)
+	if err != nil {
+		return nil, err
+	}
+	defer handle.Close()
+	// One byte past the limit, so a file exactly at it is accepted and anything
+	// larger is refused rather than silently truncated into something that
+	// might still parse.
+	data, err := io.ReadAll(io.LimitReader(handle, max+1))
+	if err != nil {
+		return nil, err
+	}
+	if int64(len(data)) > max {
+		return nil, fmt.Errorf("%s is larger than %d bytes", filepath.Base(path), max)
+	}
+	return data, nil
+}
+
 func Receive(a archive.Archive, dir string) (Received, error) {
-	data, err := os.ReadFile(filepath.Join(dir, "bundle.json"))
+	data, err := readAtMost(filepath.Join(dir, "bundle.json"), maxTransferManifestBytes)
 	if err != nil {
 		return Received{}, err
 	}
@@ -338,7 +370,7 @@ func Receive(a archive.Archive, dir string) (Received, error) {
 		if err := validateDigest(id); err != nil {
 			return Received{}, err
 		}
-		raw, err := os.ReadFile(filepath.Join(dir, "observations", id+".json"))
+		raw, err := readAtMost(filepath.Join(dir, "observations", id+".json"), maxRecordBytes)
 		if err != nil {
 			return Received{}, fmt.Errorf("observation %s declared but missing: %w", id[:12], err)
 		}
@@ -396,7 +428,7 @@ func Receive(a archive.Archive, dir string) (Received, error) {
 		if strings.ContainsAny(name, `/\`) || strings.Contains(name, "..") {
 			return Received{}, fmt.Errorf("attestation %q: a name may not contain a path", name)
 		}
-		body, err := os.ReadFile(filepath.Join(dir, "attestations", name))
+		body, err := readAtMost(filepath.Join(dir, "attestations", name), maxRecordBytes)
 		if err != nil {
 			return Received{}, fmt.Errorf("attestation %s: %w", name, err)
 		}