Keep the profile private and write it all at once
Settings, bookmarks (server passwords) and identities (private keys) were written in place with the default permissions, readable by every user on the machine, and a crash mid-write left a truncated file that the next start silently replaced with defaults. Every profile file is now written to a private temporary file, synced and moved over the old one in one step. The profile folder itself is made accessible to its owner only, which also covers files written before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -36,10 +36,15 @@ public final class AppDirs {
|
||||
return new File(profile(), name);
|
||||
}
|
||||
|
||||
/** Creates the profile directory if it does not exist yet; call before writing into it. */
|
||||
/**
|
||||
* Creates the profile directory if it does not exist yet, readable by the user only; call
|
||||
* before writing into it.
|
||||
*/
|
||||
public static void createProfile() {
|
||||
File dir = profile();
|
||||
//noinspection ResultOfMethodCallIgnored
|
||||
profile().mkdirs();
|
||||
dir.mkdirs();
|
||||
ProfileFiles.makePrivate(dir);
|
||||
}
|
||||
|
||||
public static Path chatLogs() {
|
||||
|
||||
@@ -2,7 +2,6 @@ package com.ts3client.config;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FileInputStream;
|
||||
import java.io.FileOutputStream;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Properties;
|
||||
@@ -60,9 +59,7 @@ public final class AwayMessages {
|
||||
}
|
||||
try {
|
||||
AppDirs.createProfile();
|
||||
try (FileOutputStream out = new FileOutputStream(file())) {
|
||||
p.store(out, "TS3J client away messages");
|
||||
}
|
||||
ProfileFiles.write(file(), out -> p.store(out, "TS3J client away messages"));
|
||||
} catch (Exception ignored) {
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,7 +2,6 @@ package com.ts3client.config;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FileInputStream;
|
||||
import java.io.FileOutputStream;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
@@ -60,9 +59,7 @@ public final class BanReasonPresets {
|
||||
}
|
||||
try {
|
||||
AppDirs.createProfile();
|
||||
try (FileOutputStream out = new FileOutputStream(file())) {
|
||||
p.store(out, "TS3J client ban reason presets");
|
||||
}
|
||||
ProfileFiles.write(file(), out -> p.store(out, "TS3J client ban reason presets"));
|
||||
} catch (Exception ignored) {
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,7 +2,6 @@ package com.ts3client.config;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FileInputStream;
|
||||
import java.io.FileOutputStream;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Properties;
|
||||
@@ -85,9 +84,7 @@ public final class Bookmarks {
|
||||
}
|
||||
try {
|
||||
AppDirs.createProfile();
|
||||
try (FileOutputStream out = new FileOutputStream(file())) {
|
||||
p.store(out, "TS3J client bookmarks");
|
||||
}
|
||||
ProfileFiles.write(file(), out -> p.store(out, "TS3J client bookmarks"));
|
||||
} catch (Exception ignored) {
|
||||
}
|
||||
}
|
||||
|
||||
@@ -231,7 +231,7 @@ public final class IdentityStore {
|
||||
}
|
||||
Map<String, String> props = new HashMap<>();
|
||||
props.put("id", name == null ? "" : name);
|
||||
identity.save(file, props);
|
||||
ProfileFiles.write(file, out -> identity.save(out, props));
|
||||
}
|
||||
|
||||
private static IdentityEntry read(File file) {
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
package com.ts3client.config;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FilterOutputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.OutputStream;
|
||||
import java.nio.channels.Channels;
|
||||
import java.nio.channels.FileChannel;
|
||||
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.PosixFilePermissions;
|
||||
|
||||
/**
|
||||
* Writes the files in the profile, which hold private keys and server passwords: only the
|
||||
* user may read them, and a crash mid-write never leaves one cut short.
|
||||
*/
|
||||
public final class ProfileFiles {
|
||||
|
||||
/** Produces a file's content. */
|
||||
public interface Content {
|
||||
void writeTo(OutputStream out) throws IOException;
|
||||
}
|
||||
|
||||
private ProfileFiles() {
|
||||
}
|
||||
|
||||
/**
|
||||
* Replaces {@code file} with {@code content} in one step: it is written to a private
|
||||
* temporary file next to it, flushed to disk, then moved over the old one.
|
||||
*/
|
||||
public static void write(File file, Content content) throws IOException {
|
||||
Path target = file.toPath().toAbsolutePath();
|
||||
Path dir = target.getParent();
|
||||
Files.createDirectories(dir);
|
||||
// Created readable by the owner only, where the file system has permissions at all.
|
||||
Path temp = Files.createTempFile(dir, "." + file.getName() + ".", ".tmp");
|
||||
try {
|
||||
try (FileChannel channel = FileChannel.open(temp, StandardOpenOption.WRITE)) {
|
||||
content.writeTo(unclosable(Channels.newOutputStream(channel)));
|
||||
channel.force(true);
|
||||
}
|
||||
try {
|
||||
Files.move(temp, target, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE);
|
||||
} catch (AtomicMoveNotSupportedException e) {
|
||||
Files.move(temp, target, StandardCopyOption.REPLACE_EXISTING);
|
||||
}
|
||||
} finally {
|
||||
Files.deleteIfExists(temp);
|
||||
}
|
||||
}
|
||||
|
||||
/** Some writers close what they are given; the file still has to be synced after them. */
|
||||
private static OutputStream unclosable(OutputStream out) {
|
||||
return new FilterOutputStream(out) {
|
||||
@Override
|
||||
public void write(byte[] b, int off, int len) throws IOException {
|
||||
out.write(b, off, len);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void close() {
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
/** Makes a directory accessible to the user only, where the file system supports that. */
|
||||
static void makePrivate(File dir) {
|
||||
try {
|
||||
Files.setPosixFilePermissions(dir.toPath(), PosixFilePermissions.fromString("rwx------"));
|
||||
} catch (UnsupportedOperationException | IOException ignored) {
|
||||
// Not a POSIX file system (Windows): the user's profile folder is private already.
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -5,7 +5,6 @@ import com.ts3client.sound.NotificationSettings;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FileInputStream;
|
||||
import java.io.FileOutputStream;
|
||||
import java.util.Properties;
|
||||
|
||||
/**
|
||||
@@ -207,9 +206,7 @@ public final class Settings {
|
||||
try {
|
||||
AppDirs.createProfile();
|
||||
writeToProps();
|
||||
try (FileOutputStream out = new FileOutputStream(file())) {
|
||||
props.store(out, "TS3J Swing Client settings");
|
||||
}
|
||||
ProfileFiles.write(file(), out -> props.store(out, "TS3J Swing Client settings"));
|
||||
} catch (Exception ignored) {
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package com.ts3client.contacts;
|
||||
|
||||
import com.ts3client.config.AppDirs;
|
||||
import com.ts3client.config.ProfileFiles;
|
||||
import com.ts3client.teamspeak.TeamSpeakSettingsDb;
|
||||
|
||||
import java.io.File;
|
||||
@@ -94,7 +95,7 @@ public final class ContactStore {
|
||||
}
|
||||
try {
|
||||
AppDirs.createProfile();
|
||||
Files.writeString(file.toPath(), sb.toString(), StandardCharsets.UTF_8);
|
||||
ProfileFiles.write(file, out -> out.write(sb.toString().getBytes(StandardCharsets.UTF_8)));
|
||||
} catch (IOException ignored) {
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
package com.ts3client.hotkey;
|
||||
|
||||
import com.ts3client.config.AppDirs;
|
||||
import com.ts3client.config.ProfileFiles;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FileInputStream;
|
||||
import java.io.FileOutputStream;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
@@ -93,9 +93,7 @@ public final class Hotkeys {
|
||||
}
|
||||
try {
|
||||
AppDirs.createProfile();
|
||||
try (FileOutputStream out = new FileOutputStream(file())) {
|
||||
p.store(out, "TS3J client hotkeys");
|
||||
}
|
||||
ProfileFiles.write(file(), out -> p.store(out, "TS3J client hotkeys"));
|
||||
} catch (Exception ignored) {
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
package com.ts3client.config;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.io.TempDir;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.nio.file.attribute.PosixFilePermissions;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
|
||||
class ProfileFilesTest {
|
||||
|
||||
@TempDir
|
||||
Path dir;
|
||||
|
||||
@Test
|
||||
void replacesTheFileReadableByTheUserOnly() throws Exception {
|
||||
File file = dir.resolve("settings.properties").toFile();
|
||||
Files.writeString(file.toPath(), "old");
|
||||
|
||||
ProfileFiles.write(file, out -> out.write("new".getBytes()));
|
||||
|
||||
assertEquals("new", Files.readString(file.toPath()));
|
||||
assertEquals("rw-------", PosixFilePermissions.toString(Files.getPosixFilePermissions(file.toPath())));
|
||||
try (var left = Files.list(dir)) {
|
||||
assertEquals(1, left.count(), "no temporary file left behind");
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void aFailedWriteLeavesTheOldFileAlone() throws Exception {
|
||||
File file = dir.resolve("bookmarks.properties").toFile();
|
||||
Files.writeString(file.toPath(), "old");
|
||||
|
||||
assertThrows(IOException.class, () -> ProfileFiles.write(file, out -> {
|
||||
out.write("half".getBytes());
|
||||
throw new IOException("crashed");
|
||||
}));
|
||||
|
||||
assertEquals("old", Files.readString(file.toPath()));
|
||||
try (var left = Files.list(dir)) {
|
||||
assertEquals(1, left.count(), "no temporary file left behind");
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user