From a0529754203af3a8a51228ee9c614378cfbe3497 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Mon, 31 Aug 2026 21:46:07 +0700 Subject: [PATCH] #206: pin the read-only open with a test that actually fails without it setReadOnly(true) is the whole thing keeping fleetd out of the operator's live 841MB opencode.db, and no test failed when it was removed. The obvious test does not work. Making the database file unwritable and checking the read still succeeds passes either way, because SQLite silently downgrades a read-write open of an unwritable file to read-only. I wrote that test, watched it pass with the flag removed, and threw it away. What works: extract a package-private openReadOnly(), then ask that connection to INSERT and require the refusal. Watched red with the flag removed, green with it restored. Also switches the test's INSERT helper to a PreparedStatement -- hand-escaped SQL in a test is a pattern that gets copied into main code. --- .../member/OpenCodeSessionDiscovery.java | 25 ++++++--- .../member/OpenCodeSessionDiscoveryTest.java | 51 ++++++++++++++++--- 2 files changed, 61 insertions(+), 15 deletions(-) diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeSessionDiscovery.java b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeSessionDiscovery.java index 2813999..0556871 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeSessionDiscovery.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeSessionDiscovery.java @@ -54,6 +54,23 @@ final class OpenCodeSessionDiscovery { this.databasePath = storageRoot.resolve("opencode.db"); } + /** + * A connection to {@link #databasePath} opened with SQLite's {@code SQLITE_OPEN_READONLY} + * flag: it never creates the file, never writes, and never touches WAL or journal mode. + * opencode may be running and writing this database concurrently, and this class must never + * disturb it. + * + *

Package-private so a test can hold the connection and prove it refuses a write. That is + * the only way to pin this property: making the file unwritable does not work, + * because SQLite silently downgrades a read-write open of an unwritable file to read-only, so + * such a test passes whether or not the flag is set. + */ + Connection openReadOnly() throws SQLException { + SQLiteConfig config = new SQLiteConfig(); + config.setReadOnly(true); + return config.createConnection("jdbc:sqlite:" + databasePath); + } + /** * The opencode session id whose row references {@code directory} (the worker's cwd), or * {@code null} when no row matches yet. When several rows share the directory — e.g. repeated @@ -81,14 +98,8 @@ final class OpenCodeSessionDiscovery { } return null; } - // SQLiteConfig.setReadOnly opens with the SQLITE_OPEN_READONLY flag: never creates the - // file, never writes, never touches WAL/journal mode. opencode may be running and - // writing this database concurrently (WAL mode) — this connection must never disturb it. - String url = "jdbc:sqlite:" + databasePath; - SQLiteConfig config = new SQLiteConfig(); - config.setReadOnly(true); String sql = "SELECT id FROM session WHERE directory = ? ORDER BY time_updated DESC LIMIT 1"; - try (Connection connection = config.createConnection(url); + try (Connection connection = openReadOnly(); PreparedStatement statement = connection.prepareStatement(sql)) { statement.setString(1, directory); try (ResultSet rows = statement.executeQuery()) { diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeSessionDiscoveryTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeSessionDiscoveryTest.java index de7fc0b..4e0ef83 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeSessionDiscoveryTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeSessionDiscoveryTest.java @@ -7,6 +7,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.sql.Connection; import java.sql.DriverManager; +import java.sql.PreparedStatement; +import java.sql.SQLException; import java.sql.Statement; import static org.junit.jupiter.api.Assertions.*; @@ -26,14 +28,20 @@ class OpenCodeSessionDiscoveryTest { */ static void writeRecord(Path root, String id, String directory, long timeUpdated) throws Exception { Path db = root.resolve("opencode.db"); - try (Connection connection = DriverManager.getConnection("jdbc:sqlite:" + db); - Statement statement = connection.createStatement()) { - statement.execute("CREATE TABLE IF NOT EXISTS session (" - + "id TEXT PRIMARY KEY, directory TEXT, time_updated INTEGER)"); - statement.execute("INSERT INTO session (id, directory, time_updated) VALUES (" - + "'" + id.replace("'", "''") + "', " - + "'" + directory.replace("'", "''") + "', " - + timeUpdated + ")"); + try (Connection connection = DriverManager.getConnection("jdbc:sqlite:" + db)) { + try (Statement statement = connection.createStatement()) { + statement.execute("CREATE TABLE IF NOT EXISTS session (" + + "id TEXT PRIMARY KEY, directory TEXT, time_updated INTEGER)"); + } + // Bound parameters, not string interpolation: the class under test uses a + // PreparedStatement, and a hand-escaped INSERT here is a pattern someone copies out. + try (PreparedStatement insert = connection.prepareStatement( + "INSERT INTO session (id, directory, time_updated) VALUES (?, ?, ?)")) { + insert.setString(1, id); + insert.setString(2, directory); + insert.setLong(3, timeUpdated); + insert.executeUpdate(); + } } } @@ -89,6 +97,33 @@ class OpenCodeSessionDiscoveryTest { assertNull(discovery.sessionIdForDirectory(" ")); } + /** + * The one line standing between fleetd and writing the operator's live {@code opencode.db} — + * 841MB, with a running opencode writing it — is {@code config.setReadOnly(true)} in + * {@link OpenCodeSessionDiscovery#openReadOnly()}. Delete it and every other test in this class + * still passes, so this is the test that guards it. + * + *

It asks the connection to write, and requires a refusal. The obvious alternative — make + * the database file unwritable and check the read still works — proves nothing: SQLite silently + * downgrades a read-write open of an unwritable file to read-only, so that test passes either + * way. It was tried and watched pass with the flag removed. + */ + @Test + void theDatabaseIsOpenedReadOnly(@TempDir Path root) throws Exception { + writeRecord(root, "ses_aaa", "/w/a", 1000L); + + try (Connection connection = new OpenCodeSessionDiscovery(root).openReadOnly(); + Statement statement = connection.createStatement()) { + SQLException refused = assertThrows(SQLException.class, + () -> statement.executeUpdate("INSERT INTO session (id, directory, time_updated) " + + "VALUES ('ses_zzz', '/w/z', 1)"), + "the connection must REFUSE a write — opencode is writing this database live"); + assertTrue(refused.getMessage().toLowerCase().contains("readonly") + || refused.getMessage().toLowerCase().contains("read-only"), + "the refusal must be about read-only, not some other error: " + refused.getMessage()); + } + } + @Test void aCorruptDatabaseFileYieldsNullWithoutThrowing(@TempDir Path root) throws Exception { // A file at opencode.db that is not a SQLite database at all — the open/query must fail