From 1c4e846dae5ea44faffcb64b7cfaa48cd0567e39 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 17:01:47 -0600 Subject: [PATCH 01/17] Add atomic YAML persistence helper --- .../api/misc/files/AtomicYamlWriter.java | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java new file mode 100644 index 0000000000..0dcc4c498e --- /dev/null +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java @@ -0,0 +1,56 @@ +package com.bencodez.advancedcore.api.misc.files; + +import java.io.File; +import java.io.IOException; +import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; + +import org.bukkit.configuration.file.FileConfiguration; + +/** + * Writes YAML data to a sibling temporary file before replacing the destination. + * This prevents a failed/partial serialization from truncating the last known-good + * configuration file. + */ +public final class AtomicYamlWriter { + + private AtomicYamlWriter() { + } + + public static void save(File file, FileConfiguration data) throws IOException { + if (file == null) { + throw new IllegalArgumentException("file cannot be null"); + } + if (data == null) { + throw new IllegalArgumentException("data cannot be null"); + } + + File parentFile = file.getAbsoluteFile().getParentFile(); + if (parentFile != null) { + Files.createDirectories(parentFile.toPath()); + } + + Path target = file.toPath(); + Path parent = target.toAbsolutePath().getParent(); + if (parent == null) { + parent = Path.of(".").toAbsolutePath(); + } + Path temp = Files.createTempFile(parent, "." + file.getName() + ".", ".tmp"); + boolean replaced = false; + try { + data.save(temp.toFile()); + try { + Files.move(temp, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + } catch (AtomicMoveNotSupportedException e) { + Files.move(temp, target, StandardCopyOption.REPLACE_EXISTING); + } + replaced = true; + } finally { + if (!replaced) { + Files.deleteIfExists(temp); + } + } + } +} From 422927ad4cff812727bd1fd6b0167698b7767728 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 17:03:13 -0600 Subject: [PATCH 02/17] Use atomic replacement for asynchronous YAML writes --- .../api/misc/files/FilesManager.java | 44 ++++++++----------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/FilesManager.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/FilesManager.java index d83a2bf043..b66312d6d6 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/FilesManager.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/FilesManager.java @@ -11,51 +11,43 @@ import net.md_5.bungee.api.ChatColor; -// TODO: Auto-generated Javadoc /** - * The Class FilesManager. + * Handles asynchronous file writes for AdvancedCore. */ public class FilesManager { - /** The instance. */ - static FilesManager instance = new FilesManager(); + private static final FilesManager instance = new FilesManager(); - /** - * Gets the single instance of FilesManager. - * - * @return single instance of FilesManager - */ public static FilesManager getInstance() { return instance; } - /** The plugin. */ - AdvancedCorePlugin plugin = AdvancedCorePlugin.getInstance(); - - /** - * Instantiates a new files manager. - */ private FilesManager() { } /** - * Edits the file. + * Saves the supplied configuration asynchronously. The replacement is written to + * a temporary sibling file before it replaces the destination, preserving the + * previous file if serialization fails. * * @param file the file * @param data the data */ public void editFile(File file, FileConfiguration data) { - FileThread.getInstance().run(new Runnable() { - - @Override - public void run() { - try { - data.save(file); - } catch (IOException e) { - Bukkit.getServer().getLogger().severe(ChatColor.RED + "Could not save " + file.getName()); + FileThread.getInstance().run(() -> { + try { + AtomicYamlWriter.save(file, data); + } catch (IOException | RuntimeException e) { + AdvancedCorePlugin plugin = AdvancedCorePlugin.getInstance(); + if (plugin != null) { + plugin.getLogger().severe("Could not save " + (file == null ? "null file" : file.getName()) + + ": " + e.getMessage()); + plugin.debug(e); + } else { + Bukkit.getServer().getLogger().severe(ChatColor.RED + "Could not save " + + (file == null ? "null file" : file.getName()) + ": " + e.getMessage()); } - } }); } -} \ No newline at end of file +} From 4190e34a06f122813c8da0b46fcdb5d47b1aa5a2 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 17:03:32 -0600 Subject: [PATCH 03/17] Persist ServerData with atomic replacement --- .../advancedcore/data/ServerData.java | 216 ++++++++---------- 1 file changed, 94 insertions(+), 122 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/data/ServerData.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/data/ServerData.java index 7eb06c2b43..7c05f21c01 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/data/ServerData.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/data/ServerData.java @@ -1,122 +1,94 @@ -package com.bencodez.advancedcore.data; - -import java.io.File; - -import org.bukkit.plugin.Plugin; - -import com.bencodez.advancedcore.AdvancedCorePlugin; -import com.bencodez.simpleapi.file.YMLFile; - -// TODO: Auto-generated Javadoc -/** - * The Class ServerData. - */ -public class ServerData extends YMLFile { - public ServerData(AdvancedCorePlugin plugin) { - super(plugin, new File(plugin.getDataFolder(), "ServerData.yml")); - } - - public long getLastUpdated() { - return getData().getLong("LastUpdated", -1); - } - - /** - * Gets the plugin version. - * - * @param plugin the plugin - * @return the plugin version - */ - public String getPluginVersion(Plugin plugin) { - return getData().getString("PluginVersions." + plugin.getName(), ""); - } - - /** - * Gets the prev day. - * - * @return the prev day - */ - public int getPrevDay() { - return getData().getInt("PrevDay", -1); - } - - /** - * Gets the prev month. - * - * @return the prev month - */ - public String getPrevMonth() { - return getData().getString("Month", ""); - } - - /** - * Gets the prev week day. - * - * @return the prev week day - */ - public int getPrevWeekDay() { - return getData().getInt("PrevWeek", -1); - } - - public boolean isIgnoreTime() { - return getData().getBoolean("IgnoreTime", false); - } - - @Override - public void onFileCreation() { - } - - public void setData(String path, Object value) { - getData().set(path, value); - saveData(); - } - - public void setIgnoreTime(boolean value) { - getData().set("IgnoreTime", value); - saveData(); - } - - public void setLastUpdated() { - getData().set("LastUpdated", System.currentTimeMillis()); - saveData(); - } - - /** - * Sets the plugin version. - * - * @param plugin the new plugin version - */ - public void setPluginVersion(Plugin plugin) { - getData().set("PluginVersions." + plugin.getName(), plugin.getDescription().getVersion()); - saveData(); - } - - /** - * Sets the prev day. - * - * @param day the new prev day - */ - public void setPrevDay(int day) { - getData().set("PrevDay", day); - saveData(); - } - - /** - * Sets the prev month. - * - * @param month the new prev month - */ - public void setPrevMonth(String month) { - getData().set("Month", month); - saveData(); - } - - /** - * Sets the prev week day. - * - * @param week the new prev week day - */ - public void setPrevWeekDay(int week) { - getData().set("PrevWeek", week); - saveData(); - } -} +package com.bencodez.advancedcore.data; + +import java.io.File; +import java.io.IOException; + +import org.bukkit.plugin.Plugin; + +import com.bencodez.advancedcore.AdvancedCorePlugin; +import com.bencodez.advancedcore.api.misc.files.AtomicYamlWriter; +import com.bencodez.simpleapi.file.YMLFile; + +/** + * Persistent server-level AdvancedCore data. + */ +public class ServerData extends YMLFile { + public ServerData(AdvancedCorePlugin plugin) { + super(plugin, new File(plugin.getDataFolder(), "ServerData.yml")); + } + + public long getLastUpdated() { + return getData().getLong("LastUpdated", -1); + } + + public String getPluginVersion(Plugin plugin) { + return getData().getString("PluginVersions." + plugin.getName(), ""); + } + + public int getPrevDay() { + return getData().getInt("PrevDay", -1); + } + + public String getPrevMonth() { + return getData().getString("Month", ""); + } + + public int getPrevWeekDay() { + return getData().getInt("PrevWeek", -1); + } + + public boolean isIgnoreTime() { + return getData().getBoolean("IgnoreTime", false); + } + + @Override + public void onFileCreation() { + } + + @Override + public void saveData() { + try { + AtomicYamlWriter.save(getdFile(), getData()); + } catch (IOException | RuntimeException e) { + getPlugin().getLogger().severe("Failed to save " + getdFile().getName() + ": " + e.getMessage()); + if (getPlugin() instanceof AdvancedCorePlugin) { + ((AdvancedCorePlugin) getPlugin()).debug(e); + } + } + } + + public void setData(String path, Object value) { + getData().set(path, value); + saveData(); + } + + public void setIgnoreTime(boolean value) { + getData().set("IgnoreTime", value); + saveData(); + } + + public void setLastUpdated() { + getData().set("LastUpdated", System.currentTimeMillis()); + saveData(); + } + + public void setPluginVersion(Plugin plugin) { + getData().set("PluginVersions." + plugin.getName(), plugin.getDescription().getVersion()); + saveData(); + } + + public void setPrevDay(int day) { + getData().set("PrevDay", day); + saveData(); + } + + public void setPrevMonth(String month) { + getData().set("Month", month); + saveData(); + } + + public void setPrevWeekDay(int week) { + getData().set("PrevWeek", week); + saveData(); + } +} From dec305803842012498200c77f751577d4016e4d0 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 17:03:43 -0600 Subject: [PATCH 04/17] Test atomic YAML persistence --- .../tests/files/AtomicYamlWriterTest.java | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java new file mode 100644 index 0000000000..37e65381f2 --- /dev/null +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java @@ -0,0 +1,50 @@ +package com.bencodez.advancedcore.tests.files; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.bukkit.configuration.file.YamlConfiguration; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import com.bencodez.advancedcore.api.misc.files.AtomicYamlWriter; + +public class AtomicYamlWriterTest { + + @TempDir + Path tempDir; + + @Test + public void replacesYamlWithoutLeavingTemporaryFiles() throws Exception { + File file = tempDir.resolve("ServerData.yml").toFile(); + YamlConfiguration oldData = new YamlConfiguration(); + oldData.set("Old", true); + oldData.save(file); + + YamlConfiguration replacement = new YamlConfiguration(); + replacement.set("New.Value", 42); + AtomicYamlWriter.save(file, replacement); + + YamlConfiguration loaded = YamlConfiguration.loadConfiguration(file); + assertFalse(loaded.contains("Old")); + assertEquals(42, loaded.getInt("New.Value")); + assertEquals(1L, Files.list(tempDir).count()); + } + + @Test + public void createsMissingParentDirectory() throws Exception { + File file = tempDir.resolve("nested/data.yml").toFile(); + YamlConfiguration data = new YamlConfiguration(); + data.set("Ready", true); + + AtomicYamlWriter.save(file, data); + + assertTrue(file.isFile()); + assertTrue(YamlConfiguration.loadConfiguration(file).getBoolean("Ready")); + } +} From fc4ec79a47bffa4ea875e259b79527ce9c0a2b11 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 18:07:00 -0600 Subject: [PATCH 05/17] Preserve symlinks and file permissions during atomic YAML writes --- .../api/misc/files/AtomicYamlWriter.java | 48 ++++++++++++++++--- 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java index 0dcc4c498e..0705898e66 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java @@ -6,6 +6,9 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.PosixFilePermission; +import java.util.HashSet; +import java.util.Set; import org.bukkit.configuration.file.FileConfiguration; @@ -16,6 +19,8 @@ */ public final class AtomicYamlWriter { + private static final int MAX_SYMLINK_DEPTH = 32; + private AtomicYamlWriter() { } @@ -27,20 +32,24 @@ public static void save(File file, FileConfiguration data) throws IOException { throw new IllegalArgumentException("data cannot be null"); } - File parentFile = file.getAbsoluteFile().getParentFile(); - if (parentFile != null) { - Files.createDirectories(parentFile.toPath()); + Path requestedTarget = file.toPath().toAbsolutePath().normalize(); + Path requestedParent = requestedTarget.getParent(); + if (requestedParent != null) { + Files.createDirectories(requestedParent); } - Path target = file.toPath(); - Path parent = target.toAbsolutePath().getParent(); + Path target = resolveWriteTarget(requestedTarget); + Path parent = target.getParent(); if (parent == null) { - parent = Path.of(".").toAbsolutePath(); + parent = Path.of(".").toAbsolutePath().normalize(); } - Path temp = Files.createTempFile(parent, "." + file.getName() + ".", ".tmp"); + Files.createDirectories(parent); + + Path temp = Files.createTempFile(parent, "." + target.getFileName() + ".", ".tmp"); boolean replaced = false; try { data.save(temp.toFile()); + preservePermissions(target, temp); try { Files.move(temp, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); } catch (AtomicMoveNotSupportedException e) { @@ -53,4 +62,29 @@ public static void save(File file, FileConfiguration data) throws IOException { } } } + + static Path resolveWriteTarget(Path requestedTarget) throws IOException { + Path current = requestedTarget.toAbsolutePath().normalize(); + Set visited = new HashSet<>(); + for (int depth = 0; Files.isSymbolicLink(current); depth++) { + if (depth >= MAX_SYMLINK_DEPTH || !visited.add(current)) { + throw new IOException("Too many symbolic links while resolving " + requestedTarget); + } + Path link = Files.readSymbolicLink(current); + current = link.isAbsolute() ? link.normalize() : current.getParent().resolve(link).normalize(); + } + return current; + } + + private static void preservePermissions(Path target, Path temp) throws IOException { + if (!Files.exists(target)) { + return; + } + try { + Set permissions = Files.getPosixFilePermissions(target); + Files.setPosixFilePermissions(temp, permissions); + } catch (UnsupportedOperationException ignored) { + // Non-POSIX providers do not expose Unix mode bits. + } + } } From 456d53ebc6fa8504cf5a8d0cbfb3ca6f7190ee63 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 18:07:16 -0600 Subject: [PATCH 06/17] Test symlink and permission preservation for atomic YAML writes --- .../tests/files/AtomicYamlWriterTest.java | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java index 37e65381f2..8707a5875d 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java @@ -3,10 +3,15 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import java.io.File; +import java.nio.file.FileStore; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.util.EnumSet; +import java.util.Set; import org.bukkit.configuration.file.YamlConfiguration; import org.junit.jupiter.api.Test; @@ -47,4 +52,51 @@ public void createsMissingParentDirectory() throws Exception { assertTrue(file.isFile()); assertTrue(YamlConfiguration.loadConfiguration(file).getBoolean("Ready")); } + + @Test + public void preservesDestinationSymlink() throws Exception { + Path sharedDir = tempDir.resolve("shared"); + Path localDir = tempDir.resolve("local"); + Files.createDirectories(sharedDir); + Files.createDirectories(localDir); + Path sharedTarget = sharedDir.resolve("ServerData.yml"); + YamlConfiguration original = new YamlConfiguration(); + original.set("Old", true); + original.save(sharedTarget.toFile()); + + Path link = localDir.resolve("ServerData.yml"); + try { + Files.createSymbolicLink(link, localDir.relativize(sharedTarget)); + } catch (UnsupportedOperationException | SecurityException e) { + assumeTrue(false, "symbolic links are not supported by this test environment"); + } + + YamlConfiguration replacement = new YamlConfiguration(); + replacement.set("New", true); + AtomicYamlWriter.save(link.toFile(), replacement); + + assertTrue(Files.isSymbolicLink(link), "atomic replacement must not replace the symlink itself"); + assertTrue(YamlConfiguration.loadConfiguration(sharedTarget.toFile()).getBoolean("New")); + assertFalse(YamlConfiguration.loadConfiguration(sharedTarget.toFile()).contains("Old")); + } + + @Test + public void preservesExistingPosixPermissions() throws Exception { + Path target = tempDir.resolve("permissions.yml"); + FileStore store = Files.getFileStore(tempDir); + assumeTrue(store.supportsFileAttributeView("posix"), "POSIX permissions are not available"); + + YamlConfiguration original = new YamlConfiguration(); + original.set("Old", true); + original.save(target.toFile()); + Set expected = EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, + PosixFilePermission.GROUP_READ); + Files.setPosixFilePermissions(target, expected); + + YamlConfiguration replacement = new YamlConfiguration(); + replacement.set("New", true); + AtomicYamlWriter.save(target.toFile(), replacement); + + assertEquals(expected, Files.getPosixFilePermissions(target)); + } } From 5886bf13d103c25440d7d3fcc191900b6cff93a1 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 18:29:17 -0600 Subject: [PATCH 07/17] Preserve POSIX ownership on atomic YAML writes --- .../api/misc/files/AtomicYamlWriter.java | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java index 0705898e66..208760129d 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java @@ -6,7 +6,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; -import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFileAttributes; import java.util.HashSet; import java.util.Set; @@ -49,7 +50,7 @@ public static void save(File file, FileConfiguration data) throws IOException { boolean replaced = false; try { data.save(temp.toFile()); - preservePermissions(target, temp); + preservePosixAttributes(target, temp); try { Files.move(temp, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); } catch (AtomicMoveNotSupportedException e) { @@ -76,15 +77,21 @@ static Path resolveWriteTarget(Path requestedTarget) throws IOException { return current; } - private static void preservePermissions(Path target, Path temp) throws IOException { + private static void preservePosixAttributes(Path target, Path temp) throws IOException { if (!Files.exists(target)) { return; } - try { - Set permissions = Files.getPosixFilePermissions(target); - Files.setPosixFilePermissions(temp, permissions); - } catch (UnsupportedOperationException ignored) { - // Non-POSIX providers do not expose Unix mode bits. + PosixFileAttributeView targetView = Files.getFileAttributeView(target, PosixFileAttributeView.class); + PosixFileAttributeView tempView = Files.getFileAttributeView(temp, PosixFileAttributeView.class); + if (targetView == null || tempView == null) { + return; } + + PosixFileAttributes attributes = targetView.readAttributes(); + // The temp inode becomes the destination inode after the move. Preserve all + // access-relevant POSIX metadata, not just mode bits. + tempView.setOwner(attributes.owner()); + tempView.setGroup(attributes.group()); + tempView.setPermissions(attributes.permissions()); } } From 75955cf147e44bbb47bac4a1b2c06475a5f4852f Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 18:29:38 -0600 Subject: [PATCH 08/17] Test POSIX ownership preservation --- .../tests/files/AtomicYamlWriterTest.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java index 8707a5875d..eb163e4bfd 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java @@ -9,6 +9,7 @@ import java.nio.file.FileStore; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.PosixFileAttributes; import java.nio.file.attribute.PosixFilePermission; import java.util.EnumSet; import java.util.Set; @@ -81,10 +82,10 @@ public void preservesDestinationSymlink() throws Exception { } @Test - public void preservesExistingPosixPermissions() throws Exception { + public void preservesExistingPosixPermissionsAndOwnership() throws Exception { Path target = tempDir.resolve("permissions.yml"); FileStore store = Files.getFileStore(tempDir); - assumeTrue(store.supportsFileAttributeView("posix"), "POSIX permissions are not available"); + assumeTrue(store.supportsFileAttributeView("posix"), "POSIX attributes are not available"); YamlConfiguration original = new YamlConfiguration(); original.set("Old", true); @@ -92,11 +93,15 @@ public void preservesExistingPosixPermissions() throws Exception { Set expected = EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.GROUP_READ); Files.setPosixFilePermissions(target, expected); + PosixFileAttributes before = Files.readAttributes(target, PosixFileAttributes.class); YamlConfiguration replacement = new YamlConfiguration(); replacement.set("New", true); AtomicYamlWriter.save(target.toFile(), replacement); - assertEquals(expected, Files.getPosixFilePermissions(target)); + PosixFileAttributes after = Files.readAttributes(target, PosixFileAttributes.class); + assertEquals(expected, after.permissions()); + assertEquals(before.owner(), after.owner(), "atomic replacement must preserve the file owner"); + assertEquals(before.group(), after.group(), "atomic replacement must preserve the configured POSIX group"); } } From 8762a6e81a7ed5bcd0fed9820befe69364d384e2 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 19:18:15 -0600 Subject: [PATCH 09/17] Preserve destination security metadata safely --- .../api/misc/files/AtomicYamlWriter.java | 73 ++++++++++++++----- 1 file changed, 56 insertions(+), 17 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java index 208760129d..4f7c81ee4d 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java @@ -2,12 +2,17 @@ import java.io.File; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.AclFileAttributeView; import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFileAttributes; +import java.nio.file.attribute.UserPrincipal; import java.util.HashSet; import java.util.Set; @@ -50,11 +55,15 @@ public static void save(File file, FileConfiguration data) throws IOException { boolean replaced = false; try { data.save(temp.toFile()); - preservePosixAttributes(target, temp); - try { - Files.move(temp, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); - } catch (AtomicMoveNotSupportedException e) { - Files.move(temp, target, StandardCopyOption.REPLACE_EXISTING); + if (prepareReplacementMetadata(target, temp)) { + try { + Files.move(temp, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + } catch (AtomicMoveNotSupportedException e) { + Files.move(temp, target, StandardCopyOption.REPLACE_EXISTING); + } + } else { + writeInPlacePreservingMetadata(temp, target); + Files.deleteIfExists(temp); } replaced = true; } finally { @@ -77,21 +86,51 @@ static Path resolveWriteTarget(Path requestedTarget) throws IOException { return current; } - private static void preservePosixAttributes(Path target, Path temp) throws IOException { + private static boolean prepareReplacementMetadata(Path target, Path temp) { if (!Files.exists(target)) { - return; + return true; + } + + PosixFileAttributeView targetPosix = Files.getFileAttributeView(target, PosixFileAttributeView.class); + PosixFileAttributeView tempPosix = Files.getFileAttributeView(temp, PosixFileAttributeView.class); + if (targetPosix != null && tempPosix != null) { + try { + PosixFileAttributes attributes = targetPosix.readAttributes(); + tempPosix.setPermissions(attributes.permissions()); + tempPosix.setGroup(attributes.group()); + UserPrincipal tempOwner = tempPosix.getOwner(); + if (!attributes.owner().equals(tempOwner)) { + tempPosix.setOwner(attributes.owner()); + } + return true; + } catch (IOException | SecurityException e) { + return false; + } } - PosixFileAttributeView targetView = Files.getFileAttributeView(target, PosixFileAttributeView.class); - PosixFileAttributeView tempView = Files.getFileAttributeView(temp, PosixFileAttributeView.class); - if (targetView == null || tempView == null) { - return; + + AclFileAttributeView targetAcl = Files.getFileAttributeView(target, AclFileAttributeView.class); + AclFileAttributeView tempAcl = Files.getFileAttributeView(temp, AclFileAttributeView.class); + if (targetAcl != null && tempAcl != null) { + try { + tempAcl.setAcl(targetAcl.getAcl()); + UserPrincipal targetOwner = targetAcl.getOwner(); + if (!targetOwner.equals(tempAcl.getOwner())) { + tempAcl.setOwner(targetOwner); + } + return true; + } catch (IOException | SecurityException e) { + return false; + } } - PosixFileAttributes attributes = targetView.readAttributes(); - // The temp inode becomes the destination inode after the move. Preserve all - // access-relevant POSIX metadata, not just mode bits. - tempView.setOwner(attributes.owner()); - tempView.setGroup(attributes.group()); - tempView.setPermissions(attributes.permissions()); + return true; + } + + private static void writeInPlacePreservingMetadata(Path temp, Path target) throws IOException { + try (InputStream input = Files.newInputStream(temp); + OutputStream output = Files.newOutputStream(target, StandardOpenOption.WRITE, + StandardOpenOption.TRUNCATE_EXISTING)) { + input.transferTo(output); + } } } From 0d02255705a476b5dbe1b4078d0b586d861e45d9 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 19:19:42 -0600 Subject: [PATCH 10/17] Cover atomic save metadata fallback --- .../AtomicYamlWriterSecurityMetadataTest.java | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterSecurityMetadataTest.java diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterSecurityMetadataTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterSecurityMetadataTest.java new file mode 100644 index 0000000000..549ea18f0b --- /dev/null +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterSecurityMetadataTest.java @@ -0,0 +1,80 @@ +package com.bencodez.advancedcore.tests.files; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.lang.reflect.Method; +import java.nio.file.FileStore; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.AclFileAttributeView; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.PosixFileAttributes; +import java.nio.file.attribute.PosixFilePermission; +import java.util.EnumSet; +import java.util.Set; + +import org.bukkit.configuration.file.YamlConfiguration; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import com.bencodez.advancedcore.api.misc.files.AtomicYamlWriter; + +public class AtomicYamlWriterSecurityMetadataTest { + + @TempDir + Path tempDir; + + @Test + public void inPlaceFallbackPreservesExistingPosixInodeMetadata() throws Exception { + FileStore store = Files.getFileStore(tempDir); + assumeTrue(store.supportsFileAttributeView("posix"), "POSIX attributes are not available"); + + Path target = tempDir.resolve("fallback.yml"); + Path serialized = tempDir.resolve("serialized.tmp"); + Files.writeString(target, "Old: true\n"); + Files.writeString(serialized, "New: true\n"); + Set expected = EnumSet.of(PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, PosixFilePermission.GROUP_READ); + Files.setPosixFilePermissions(target, expected); + PosixFileAttributes before = Files.readAttributes(target, PosixFileAttributes.class); + Object fileKeyBefore = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); + + Method fallback = AtomicYamlWriter.class.getDeclaredMethod("writeInPlacePreservingMetadata", Path.class, + Path.class); + fallback.setAccessible(true); + fallback.invoke(null, serialized, target); + + PosixFileAttributes after = Files.readAttributes(target, PosixFileAttributes.class); + Object fileKeyAfter = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); + assertEquals(expected, after.permissions()); + assertEquals(before.owner(), after.owner()); + assertEquals(before.group(), after.group()); + if (fileKeyBefore != null && fileKeyAfter != null) { + assertEquals(fileKeyBefore, fileKeyAfter, "fallback must preserve the existing destination inode"); + } + assertTrue(YamlConfiguration.loadConfiguration(target.toFile()).getBoolean("New")); + } + + @Test + public void savePreservesAclMetadataWhenProviderSupportsIt() throws Exception { + FileStore store = Files.getFileStore(tempDir); + assumeTrue(store.supportsFileAttributeView("acl"), "ACL attributes are not available"); + + Path target = tempDir.resolve("acl.yml"); + Files.writeString(target, "Old: true\n"); + AclFileAttributeView beforeView = Files.getFileAttributeView(target, AclFileAttributeView.class); + assumeTrue(beforeView != null, "ACL view is unavailable"); + var aclBefore = beforeView.getAcl(); + var ownerBefore = beforeView.getOwner(); + + YamlConfiguration replacement = new YamlConfiguration(); + replacement.set("New", true); + AtomicYamlWriter.save(target.toFile(), replacement); + + AclFileAttributeView afterView = Files.getFileAttributeView(target, AclFileAttributeView.class); + assertEquals(ownerBefore, afterView.getOwner()); + assertEquals(aclBefore, afterView.getAcl()); + } +} From 99ec2b80455940473bd9cbd509013cde78876e76 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 19:50:40 -0600 Subject: [PATCH 11/17] Preserve YAML creation and filesystem metadata semantics --- .../api/misc/files/AtomicYamlWriter.java | 91 ++++++++++++------- 1 file changed, 58 insertions(+), 33 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java index 4f7c81ee4d..a8d496b634 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java @@ -13,20 +13,19 @@ import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFileAttributes; import java.nio.file.attribute.UserPrincipal; -import java.util.HashSet; import java.util.Set; +import java.util.UUID; import org.bukkit.configuration.file.FileConfiguration; /** - * Writes YAML data to a sibling temporary file before replacing the destination. - * This prevents a failed/partial serialization from truncating the last known-good - * configuration file. + * Writes YAML data to a sibling temporary file before committing it to the + * destination. Serialization always completes before the existing file can be + * modified. Existing POSIX files keep their inode so extended ACLs and other + * inode metadata are not discarded by replacement. */ public final class AtomicYamlWriter { - private static final int MAX_SYMLINK_DEPTH = 32; - private AtomicYamlWriter() { } @@ -38,7 +37,7 @@ public static void save(File file, FileConfiguration data) throws IOException { throw new IllegalArgumentException("data cannot be null"); } - Path requestedTarget = file.toPath().toAbsolutePath().normalize(); + Path requestedTarget = file.toPath().toAbsolutePath(); Path requestedParent = requestedTarget.getParent(); if (requestedParent != null) { Files.createDirectories(requestedParent); @@ -47,12 +46,12 @@ public static void save(File file, FileConfiguration data) throws IOException { Path target = resolveWriteTarget(requestedTarget); Path parent = target.getParent(); if (parent == null) { - parent = Path.of(".").toAbsolutePath().normalize(); + parent = Path.of(".").toAbsolutePath(); } Files.createDirectories(parent); Path temp = Files.createTempFile(parent, "." + target.getFileName() + ".", ".tmp"); - boolean replaced = false; + boolean committed = false; try { data.save(temp.toFile()); if (prepareReplacementMetadata(target, temp)) { @@ -65,47 +64,52 @@ public static void save(File file, FileConfiguration data) throws IOException { writeInPlacePreservingMetadata(temp, target); Files.deleteIfExists(temp); } - replaced = true; + committed = true; } finally { - if (!replaced) { + if (!committed) { Files.deleteIfExists(temp); } } } static Path resolveWriteTarget(Path requestedTarget) throws IOException { - Path current = requestedTarget.toAbsolutePath().normalize(); - Set visited = new HashSet<>(); - for (int depth = 0; Files.isSymbolicLink(current); depth++) { - if (depth >= MAX_SYMLINK_DEPTH || !visited.add(current)) { - throw new IOException("Too many symbolic links while resolving " + requestedTarget); - } + Path current = requestedTarget.toAbsolutePath(); + + // If the complete path exists, let the filesystem resolve every component. + // This is important for paths such as dirlink/../target.yml: lexical + // normalization before following dirlink can select a different file. + if (Files.exists(current)) { + return current.toRealPath(); + } + + // A broken final symlink still needs write-through semantics. Follow that + // final link without normalizing its text, then resolve the longest existing + // parent through the filesystem. + if (Files.isSymbolicLink(current)) { Path link = Files.readSymbolicLink(current); - current = link.isAbsolute() ? link.normalize() : current.getParent().resolve(link).normalize(); + Path linkedTarget = link.isAbsolute() ? link : current.getParent().resolve(link); + return resolveWriteTarget(linkedTarget); + } + + Path parent = current.getParent(); + if (parent != null && Files.exists(parent)) { + return parent.toRealPath().resolve(current.getFileName()); } return current; } - private static boolean prepareReplacementMetadata(Path target, Path temp) { + private static boolean prepareReplacementMetadata(Path target, Path temp) throws IOException { if (!Files.exists(target)) { + preserveNormalCreationPermissions(target, temp); return true; } + // Java's POSIX view exposes owner/group/mode, but not Linux extended POSIX + // ACL entries. Replacing the inode can therefore silently discard setfacl + // grants. Commit into the existing inode after successful serialization. PosixFileAttributeView targetPosix = Files.getFileAttributeView(target, PosixFileAttributeView.class); - PosixFileAttributeView tempPosix = Files.getFileAttributeView(temp, PosixFileAttributeView.class); - if (targetPosix != null && tempPosix != null) { - try { - PosixFileAttributes attributes = targetPosix.readAttributes(); - tempPosix.setPermissions(attributes.permissions()); - tempPosix.setGroup(attributes.group()); - UserPrincipal tempOwner = tempPosix.getOwner(); - if (!attributes.owner().equals(tempOwner)) { - tempPosix.setOwner(attributes.owner()); - } - return true; - } catch (IOException | SecurityException e) { - return false; - } + if (targetPosix != null) { + return false; } AclFileAttributeView targetAcl = Files.getFileAttributeView(target, AclFileAttributeView.class); @@ -126,6 +130,27 @@ private static boolean prepareReplacementMetadata(Path target, Path temp) { return true; } + private static void preserveNormalCreationPermissions(Path target, Path temp) throws IOException { + PosixFileAttributeView tempPosix = Files.getFileAttributeView(temp, PosixFileAttributeView.class); + if (tempPosix == null) { + return; + } + + Path parent = target.getParent(); + if (parent == null) { + return; + } + Path probe = parent.resolve("." + target.getFileName() + "." + UUID.randomUUID() + ".mode-probe"); + try { + Files.createFile(probe); + Set permissions = Files + .readAttributes(probe, PosixFileAttributes.class).permissions(); + tempPosix.setPermissions(permissions); + } finally { + Files.deleteIfExists(probe); + } + } + private static void writeInPlacePreservingMetadata(Path temp, Path target) throws IOException { try (InputStream input = Files.newInputStream(temp); OutputStream output = Files.newOutputStream(target, StandardOpenOption.WRITE, From 0b8d1f16f5f6b526d9501d5b8c2a60d0588402a7 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 19:51:15 -0600 Subject: [PATCH 12/17] Test YAML creation permissions and symlink semantics --- .../tests/files/AtomicYamlWriterTest.java | 65 +++++++++++++++++-- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java index eb163e4bfd..0af7e2e53d 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java @@ -9,6 +9,7 @@ import java.nio.file.FileStore; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.attribute.PosixFileAttributes; import java.nio.file.attribute.PosixFilePermission; import java.util.EnumSet; @@ -54,6 +55,25 @@ public void createsMissingParentDirectory() throws Exception { assertTrue(YamlConfiguration.loadConfiguration(file).getBoolean("Ready")); } + @Test + public void newPosixFileUsesNormalCreationPermissions() throws Exception { + FileStore store = Files.getFileStore(tempDir); + assumeTrue(store.supportsFileAttributeView("posix"), "POSIX attributes are not available"); + + Path normalCreate = tempDir.resolve("normal-create.yml"); + Files.createFile(normalCreate); + Set expected = Files.getPosixFilePermissions(normalCreate); + Files.delete(normalCreate); + + Path target = tempDir.resolve("atomic-create.yml"); + YamlConfiguration data = new YamlConfiguration(); + data.set("Ready", true); + AtomicYamlWriter.save(target.toFile(), data); + + assertEquals(expected, Files.getPosixFilePermissions(target), + "first-time saves should honor the same process umask as a normal file creation"); + } + @Test public void preservesDestinationSymlink() throws Exception { Path sharedDir = tempDir.resolve("shared"); @@ -76,13 +96,44 @@ public void preservesDestinationSymlink() throws Exception { replacement.set("New", true); AtomicYamlWriter.save(link.toFile(), replacement); - assertTrue(Files.isSymbolicLink(link), "atomic replacement must not replace the symlink itself"); + assertTrue(Files.isSymbolicLink(link), "replacement must not replace the symlink itself"); assertTrue(YamlConfiguration.loadConfiguration(sharedTarget.toFile()).getBoolean("New")); assertFalse(YamlConfiguration.loadConfiguration(sharedTarget.toFile()).contains("Old")); } @Test - public void preservesExistingPosixPermissionsAndOwnership() throws Exception { + public void resolvesDotDotAfterIntermediateSymlinkUsingFilesystemSemantics() throws Exception { + Path local = tempDir.resolve("local"); + Path elsewhere = tempDir.resolve("elsewhere"); + Path nested = elsewhere.resolve("nested"); + Files.createDirectories(local); + Files.createDirectories(nested); + + Path directoryLink = local.resolve("dirlink"); + Path fileLink = local.resolve("ServerData.yml"); + try { + Files.createSymbolicLink(directoryLink, local.relativize(nested)); + Files.createSymbolicLink(fileLink, Path.of("dirlink/../target.yml")); + } catch (UnsupportedOperationException | SecurityException e) { + assumeTrue(false, "symbolic links are not supported by this test environment"); + } + + Path expectedTarget = elsewhere.resolve("target.yml"); + YamlConfiguration original = new YamlConfiguration(); + original.set("Old", true); + original.save(expectedTarget.toFile()); + + YamlConfiguration replacement = new YamlConfiguration(); + replacement.set("New", true); + AtomicYamlWriter.save(fileLink.toFile(), replacement); + + assertTrue(YamlConfiguration.loadConfiguration(expectedTarget.toFile()).getBoolean("New")); + assertFalse(Files.exists(local.resolve("target.yml")), + "lexical normalization must not redirect a symlink containing an intermediate link plus .."); + } + + @Test + public void preservesExistingPosixPermissionsOwnershipAndInode() throws Exception { Path target = tempDir.resolve("permissions.yml"); FileStore store = Files.getFileStore(tempDir); assumeTrue(store.supportsFileAttributeView("posix"), "POSIX attributes are not available"); @@ -94,14 +145,20 @@ public void preservesExistingPosixPermissionsAndOwnership() throws Exception { PosixFilePermission.GROUP_READ); Files.setPosixFilePermissions(target, expected); PosixFileAttributes before = Files.readAttributes(target, PosixFileAttributes.class); + Object beforeFileKey = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); YamlConfiguration replacement = new YamlConfiguration(); replacement.set("New", true); AtomicYamlWriter.save(target.toFile(), replacement); PosixFileAttributes after = Files.readAttributes(target, PosixFileAttributes.class); + Object afterFileKey = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); assertEquals(expected, after.permissions()); - assertEquals(before.owner(), after.owner(), "atomic replacement must preserve the file owner"); - assertEquals(before.group(), after.group(), "atomic replacement must preserve the configured POSIX group"); + assertEquals(before.owner(), after.owner(), "save must preserve the file owner"); + assertEquals(before.group(), after.group(), "save must preserve the configured POSIX group"); + if (beforeFileKey != null && afterFileKey != null) { + assertEquals(beforeFileKey, afterFileKey, + "existing POSIX files must retain their inode so extended POSIX ACLs are not discarded"); + } } } From a2b187a464fec4bbd14fedc2fa8c9e6f450062f1 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 20:08:56 -0600 Subject: [PATCH 13/17] Keep YAML commits atomic while preserving metadata --- .../api/misc/files/AtomicYamlWriter.java | 148 +++++++++++++++--- 1 file changed, 125 insertions(+), 23 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java index a8d496b634..d74cc804a5 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java @@ -2,8 +2,8 @@ import java.io.File; import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; +import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; import java.nio.file.Path; @@ -13,6 +13,7 @@ import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFileAttributes; import java.nio.file.attribute.UserPrincipal; +import java.util.HashSet; import java.util.Set; import java.util.UUID; @@ -21,11 +22,15 @@ /** * Writes YAML data to a sibling temporary file before committing it to the * destination. Serialization always completes before the existing file can be - * modified. Existing POSIX files keep their inode so extended ACLs and other - * inode metadata are not discarded by replacement. + * modified. Existing files whose security metadata cannot be recreated safely + * are committed through a preserved hard-link inode so the live path always + * points at a complete old or new file. */ public final class AtomicYamlWriter { + private static final int MAX_SYMLINK_DEPTH = 32; + private static final String PRESERVED_SUFFIX = ".advancedcore-preserved-inode"; + private AtomicYamlWriter() { } @@ -50,19 +55,17 @@ public static void save(File file, FileConfiguration data) throws IOException { } Files.createDirectories(parent); + Path preserved = preservedPath(target); + recoverPreservedInode(target, preserved); + Path temp = Files.createTempFile(parent, "." + target.getFileName() + ".", ".tmp"); boolean committed = false; try { data.save(temp.toFile()); if (prepareReplacementMetadata(target, temp)) { - try { - Files.move(temp, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); - } catch (AtomicMoveNotSupportedException e) { - Files.move(temp, target, StandardCopyOption.REPLACE_EXISTING); - } + moveReplacement(temp, target); } else { - writeInPlacePreservingMetadata(temp, target); - Files.deleteIfExists(temp); + commitThroughPreservedInode(temp, target, preserved); } committed = true; } finally { @@ -73,7 +76,13 @@ public static void save(File file, FileConfiguration data) throws IOException { } static Path resolveWriteTarget(Path requestedTarget) throws IOException { - Path current = requestedTarget.toAbsolutePath(); + return resolveWriteTarget(requestedTarget.toAbsolutePath(), new HashSet<>(), 0); + } + + private static Path resolveWriteTarget(Path current, Set visited, int depth) throws IOException { + if (depth > MAX_SYMLINK_DEPTH || !visited.add(current)) { + throw new IOException("Cyclic or excessively deep symbolic link while resolving " + current); + } // If the complete path exists, let the filesystem resolve every component. // This is important for paths such as dirlink/../target.yml: lexical @@ -82,13 +91,13 @@ static Path resolveWriteTarget(Path requestedTarget) throws IOException { return current.toRealPath(); } - // A broken final symlink still needs write-through semantics. Follow that - // final link without normalizing its text, then resolve the longest existing - // parent through the filesystem. + // Files.exists follows links and therefore reports false for both a broken + // final link and a symlink cycle. Inspect the directory entry itself before + // recursing, while keeping a visited/depth guard for cycles. if (Files.isSymbolicLink(current)) { Path link = Files.readSymbolicLink(current); Path linkedTarget = link.isAbsolute() ? link : current.getParent().resolve(link); - return resolveWriteTarget(linkedTarget); + return resolveWriteTarget(linkedTarget, visited, depth + 1); } Path parent = current.getParent(); @@ -105,10 +114,11 @@ private static boolean prepareReplacementMetadata(Path target, Path temp) throws } // Java's POSIX view exposes owner/group/mode, but not Linux extended POSIX - // ACL entries. Replacing the inode can therefore silently discard setfacl - // grants. Commit into the existing inode after successful serialization. + // ACL entries. Use the preserved-inode commit path so those ACLs and other + // inode metadata survive without truncating the live destination. PosixFileAttributeView targetPosix = Files.getFileAttributeView(target, PosixFileAttributeView.class); if (targetPosix != null) { + applyPosixMetadataBestEffort(target, temp); return false; } @@ -151,11 +161,103 @@ private static void preserveNormalCreationPermissions(Path target, Path temp) th } } - private static void writeInPlacePreservingMetadata(Path temp, Path target) throws IOException { - try (InputStream input = Files.newInputStream(temp); - OutputStream output = Files.newOutputStream(target, StandardOpenOption.WRITE, - StandardOpenOption.TRUNCATE_EXISTING)) { - input.transferTo(output); + private static void applyPosixMetadataBestEffort(Path target, Path temp) { + try { + PosixFileAttributes attributes = Files.readAttributes(target, PosixFileAttributes.class); + PosixFileAttributeView tempPosix = Files.getFileAttributeView(temp, PosixFileAttributeView.class); + if (tempPosix == null) { + return; + } + try { + tempPosix.setPermissions(attributes.permissions()); + } catch (IOException | SecurityException ignored) { + } + try { + tempPosix.setGroup(attributes.group()); + } catch (IOException | SecurityException ignored) { + } + try { + if (!attributes.owner().equals(tempPosix.getOwner())) { + tempPosix.setOwner(attributes.owner()); + } + } catch (IOException | SecurityException ignored) { + } + } catch (IOException | SecurityException ignored) { + } + } + + private static Path preservedPath(Path target) { + return target.getParent().resolve("." + target.getFileName() + PRESERVED_SUFFIX); + } + + private static void recoverPreservedInode(Path target, Path preserved) throws IOException { + if (!Files.exists(preserved)) { + return; + } + if (Files.isSymbolicLink(preserved)) { + throw new IOException("Refusing to follow preserved-inode symlink: " + preserved); + } + if (!Files.exists(target)) { + moveReplacement(preserved, target); + return; + } + if (Files.isSameFile(target, preserved)) { + Files.delete(preserved); + return; + } + + copyCompleteContents(target, preserved); + moveReplacement(preserved, target); + } + + private static void commitThroughPreservedInode(Path temp, Path target, Path preserved) throws IOException { + Files.deleteIfExists(preserved); + Files.createLink(preserved, target); + boolean livePathReplaced = false; + try { + // The fully serialized temp becomes the live path first. If any later copy + // into the preserved inode fails, the live path still contains a complete + // new YAML rather than a truncated file. + moveReplacement(temp, target); + livePathReplaced = true; + + // Refill the preserved original inode while it is off-path, then atomically + // restore that inode to the live path so POSIX ACLs/ownership/xattrs survive. + copyCompleteContents(target, preserved); + moveReplacement(preserved, target); + } catch (IOException | RuntimeException e) { + if (!livePathReplaced) { + Files.deleteIfExists(preserved); + } + // Once the live path has been replaced, leave the preserved inode in place + // on failure. A later save will repair it from the complete live file before + // attempting another commit. + throw e; + } + } + + private static void copyCompleteContents(Path source, Path destination) throws IOException { + try (FileChannel input = FileChannel.open(source, StandardOpenOption.READ); + FileChannel output = FileChannel.open(destination, StandardOpenOption.WRITE)) { + ByteBuffer buffer = ByteBuffer.allocateDirect(64 * 1024); + output.position(0L); + while (input.read(buffer) != -1) { + buffer.flip(); + while (buffer.hasRemaining()) { + output.write(buffer); + } + buffer.clear(); + } + output.truncate(output.position()); + output.force(true); + } + } + + private static void moveReplacement(Path source, Path target) throws IOException { + try { + Files.move(source, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + } catch (AtomicMoveNotSupportedException e) { + Files.move(source, target, StandardCopyOption.REPLACE_EXISTING); } } } From b16defd44d818b5bceaa3e1044765d7513b524e7 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 20:09:36 -0600 Subject: [PATCH 14/17] Test crash-safe POSIX commits and symlink cycles --- .../tests/files/AtomicYamlWriterTest.java | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java index 0af7e2e53d..7fd47d8673 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java @@ -2,13 +2,16 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; import java.io.File; +import java.io.IOException; import java.nio.file.FileStore; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.attribute.PosixFileAttributes; import java.nio.file.attribute.PosixFilePermission; @@ -132,6 +135,24 @@ public void resolvesDotDotAfterIntermediateSymlinkUsingFilesystemSemantics() thr "lexical normalization must not redirect a symlink containing an intermediate link plus .."); } + @Test + public void rejectsCyclicSymlinkChains() throws Exception { + Path first = tempDir.resolve("first.yml"); + Path second = tempDir.resolve("second.yml"); + try { + Files.createSymbolicLink(first, second.getFileName()); + Files.createSymbolicLink(second, first.getFileName()); + } catch (UnsupportedOperationException | SecurityException e) { + assumeTrue(false, "symbolic links are not supported by this test environment"); + } + + YamlConfiguration replacement = new YamlConfiguration(); + replacement.set("New", true); + + IOException error = assertThrows(IOException.class, () -> AtomicYamlWriter.save(first.toFile(), replacement)); + assertTrue(error.getMessage().contains("Cyclic") || error.getMessage().contains("deep")); + } + @Test public void preservesExistingPosixPermissionsOwnershipAndInode() throws Exception { Path target = tempDir.resolve("permissions.yml"); @@ -160,5 +181,43 @@ public void preservesExistingPosixPermissionsOwnershipAndInode() throws Exceptio assertEquals(beforeFileKey, afterFileKey, "existing POSIX files must retain their inode so extended POSIX ACLs are not discarded"); } + assertFalse(Files.exists(tempDir.resolve(".permissions.yml.advancedcore-preserved-inode"))); + } + + @Test + public void recoversPreservedInodeAfterInterruptedPosixCommit() throws Exception { + FileStore store = Files.getFileStore(tempDir); + assumeTrue(store.supportsFileAttributeView("posix"), "POSIX attributes are not available"); + + Path target = tempDir.resolve("recovery.yml"); + YamlConfiguration original = new YamlConfiguration(); + original.set("Old", true); + original.save(target.toFile()); + Object originalFileKey = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); + + Path preserved = tempDir.resolve(".recovery.yml.advancedcore-preserved-inode"); + try { + Files.createLink(preserved, target); + } catch (UnsupportedOperationException | SecurityException e) { + assumeTrue(false, "hard links are not supported by this test environment"); + } + + Path simulatedLiveReplacement = tempDir.resolve("simulated-new.yml"); + YamlConfiguration alreadyCommitted = new YamlConfiguration(); + alreadyCommitted.set("Committed", true); + alreadyCommitted.save(simulatedLiveReplacement.toFile()); + Files.move(simulatedLiveReplacement, target, StandardCopyOption.REPLACE_EXISTING); + + YamlConfiguration nextSave = new YamlConfiguration(); + nextSave.set("Final", true); + AtomicYamlWriter.save(target.toFile(), nextSave); + + assertTrue(YamlConfiguration.loadConfiguration(target.toFile()).getBoolean("Final")); + assertFalse(Files.exists(preserved)); + Object finalFileKey = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); + if (originalFileKey != null && finalFileKey != null) { + assertEquals(originalFileKey, finalFileKey, + "recovery should restore the preserved inode before the next successful commit"); + } } } From 7b77c6d8926b3129f48619d9f620d8d5f388472a Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 20:12:06 -0600 Subject: [PATCH 15/17] Update metadata test for preserved-inode commit --- .../AtomicYamlWriterSecurityMetadataTest.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterSecurityMetadataTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterSecurityMetadataTest.java index 549ea18f0b..2c372d30ae 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterSecurityMetadataTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterSecurityMetadataTest.java @@ -1,10 +1,10 @@ package com.bencodez.advancedcore.tests.files; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; -import java.lang.reflect.Method; import java.nio.file.FileStore; import java.nio.file.Files; import java.nio.file.Path; @@ -27,24 +27,21 @@ public class AtomicYamlWriterSecurityMetadataTest { Path tempDir; @Test - public void inPlaceFallbackPreservesExistingPosixInodeMetadata() throws Exception { + public void preservedInodeCommitKeepsExistingPosixMetadata() throws Exception { FileStore store = Files.getFileStore(tempDir); assumeTrue(store.supportsFileAttributeView("posix"), "POSIX attributes are not available"); Path target = tempDir.resolve("fallback.yml"); - Path serialized = tempDir.resolve("serialized.tmp"); Files.writeString(target, "Old: true\n"); - Files.writeString(serialized, "New: true\n"); Set expected = EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.GROUP_READ); Files.setPosixFilePermissions(target, expected); PosixFileAttributes before = Files.readAttributes(target, PosixFileAttributes.class); Object fileKeyBefore = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); - Method fallback = AtomicYamlWriter.class.getDeclaredMethod("writeInPlacePreservingMetadata", Path.class, - Path.class); - fallback.setAccessible(true); - fallback.invoke(null, serialized, target); + YamlConfiguration replacement = new YamlConfiguration(); + replacement.set("New", true); + AtomicYamlWriter.save(target.toFile(), replacement); PosixFileAttributes after = Files.readAttributes(target, PosixFileAttributes.class); Object fileKeyAfter = Files.readAttributes(target, BasicFileAttributes.class).fileKey(); @@ -52,9 +49,11 @@ public void inPlaceFallbackPreservesExistingPosixInodeMetadata() throws Exceptio assertEquals(before.owner(), after.owner()); assertEquals(before.group(), after.group()); if (fileKeyBefore != null && fileKeyAfter != null) { - assertEquals(fileKeyBefore, fileKeyAfter, "fallback must preserve the existing destination inode"); + assertEquals(fileKeyBefore, fileKeyAfter, + "the completed save must restore the original metadata-bearing inode"); } assertTrue(YamlConfiguration.loadConfiguration(target.toFile()).getBoolean("New")); + assertFalse(Files.exists(tempDir.resolve(".fallback.yml.advancedcore-preserved-inode"))); } @Test From de3397a10bb12742de1d31d34dd857e63f72cbd8 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 20:40:07 -0600 Subject: [PATCH 16/17] Handle writable files in non-writable directories --- .../api/misc/files/AtomicYamlWriter.java | 51 ++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java index d74cc804a5..78afe9945c 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/misc/files/AtomicYamlWriter.java @@ -4,6 +4,7 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.nio.channels.FileChannel; +import java.nio.charset.StandardCharsets; import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; import java.nio.file.Path; @@ -58,7 +59,17 @@ public static void save(File file, FileConfiguration data) throws IOException { Path preserved = preservedPath(target); recoverPreservedInode(target, preserved); - Path temp = Files.createTempFile(parent, "." + target.getFileName() + ".", ".tmp"); + Path temp; + try { + temp = Files.createTempFile(parent, "." + target.getFileName() + ".", ".tmp"); + } catch (IOException siblingCreationFailure) { + if (Files.exists(target) && Files.isWritable(target)) { + saveSerializedInPlace(target, data, siblingCreationFailure); + return; + } + throw siblingCreationFailure; + } + boolean committed = false; try { data.save(temp.toFile()); @@ -236,6 +247,44 @@ private static void commitThroughPreservedInode(Path temp, Path target, Path pre } } + private static void saveSerializedInPlace(Path target, FileConfiguration data, IOException siblingCreationFailure) + throws IOException { + byte[] replacement = data.saveToString().getBytes(StandardCharsets.UTF_8); + byte[] original = null; + try { + original = Files.readAllBytes(target); + } catch (IOException | SecurityException ignored) { + // The legacy save path required only write access. Preserve that compatibility + // even when this process cannot read the old file for rollback. + } + + try { + writeCompleteBytes(target, replacement); + } catch (IOException writeFailure) { + writeFailure.addSuppressed(siblingCreationFailure); + if (original != null) { + try { + writeCompleteBytes(target, original); + } catch (IOException rollbackFailure) { + writeFailure.addSuppressed(rollbackFailure); + } + } + throw writeFailure; + } + } + + private static void writeCompleteBytes(Path target, byte[] bytes) throws IOException { + try (FileChannel output = FileChannel.open(target, StandardOpenOption.WRITE)) { + ByteBuffer buffer = ByteBuffer.wrap(bytes); + output.position(0L); + while (buffer.hasRemaining()) { + output.write(buffer); + } + output.truncate(output.position()); + output.force(true); + } + } + private static void copyCompleteContents(Path source, Path destination) throws IOException { try (FileChannel input = FileChannel.open(source, StandardOpenOption.READ); FileChannel output = FileChannel.open(destination, StandardOpenOption.WRITE)) { From 1fd4a6229d0b6559ac4f662ad09413b3e5ac8457 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 24 Aug 2026 20:41:42 -0600 Subject: [PATCH 17/17] Test non-writable parent compatibility fallback --- .../tests/files/AtomicYamlWriterTest.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java index 7fd47d8673..1bed0982be 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/files/AtomicYamlWriterTest.java @@ -153,6 +153,40 @@ public void rejectsCyclicSymlinkChains() throws Exception { assertTrue(error.getMessage().contains("Cyclic") || error.getMessage().contains("deep")); } + @Test + public void writableExistingFileSavesWhenParentDirectoryIsNotWritable() throws Exception { + FileStore store = Files.getFileStore(tempDir); + assumeTrue(store.supportsFileAttributeView("posix"), "POSIX attributes are not available"); + + Path lockedDir = tempDir.resolve("locked"); + Files.createDirectory(lockedDir); + Path target = lockedDir.resolve("ServerData.yml"); + Files.writeString(target, "Old: true\n"); + + Set originalDirPermissions = Files.getPosixFilePermissions(lockedDir); + Files.setPosixFilePermissions(target, + EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE)); + Files.setPosixFilePermissions(lockedDir, + EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_EXECUTE)); + + try { + assumeTrue(!Files.isWritable(lockedDir), + "test environment still permits sibling creation in a non-writable directory"); + assumeTrue(Files.isWritable(target), + "existing target must remain writable for compatibility fallback"); + + YamlConfiguration replacement = new YamlConfiguration(); + replacement.set("New", true); + AtomicYamlWriter.save(target.toFile(), replacement); + + YamlConfiguration loaded = YamlConfiguration.loadConfiguration(target.toFile()); + assertTrue(loaded.getBoolean("New")); + assertFalse(loaded.contains("Old")); + } finally { + Files.setPosixFilePermissions(lockedDir, originalDirPermissions); + } + } + @Test public void preservesExistingPosixPermissionsOwnershipAndInode() throws Exception { Path target = tempDir.resolve("permissions.yml");