Skip to content

Commit 6284b5c

Browse files
authored
fix(connections): stop stale-wal replay, reuse misses, and orphaned copies on remote database files (#2842)
1 parent 060c254 commit 6284b5c

7 files changed

Lines changed: 256 additions & 6 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5555
- TiDB's `INFORMATION_SCHEMA` and `PERFORMANCE_SCHEMA` listed as user databases on a MySQL or MariaDB connection.
5656
- SQL Server database size and table count showing the current database's numbers, and no size at 2 GB or more.
5757
- ClickHouse databases with no tables missing from the database switcher and database statistics.
58+
- Stale write-ahead log replayed over a freshly fetched copy of a remote SQLite database.
59+
- Remote database copy reused after a commit that did not grow its write-ahead log.
60+
- Killed remote `VACUUM INTO` snapshot reported as a successful copy.
61+
- Interrupted remote snapshot files left on the server, now swept on the next fetch.
62+
- Local working copies of remote databases kept forever, now removed after 30 days unused.
5863

5964
### Security
6065

6166
- SQLite denies the `fts3_tokenizer` function, which could crash the app from a crafted query on any connection.
6267
- The AI assistant refuses statements that read or write files or run server-side code (ATTACH, LOAD, VACUUM INTO), matching the MCP server. (#2831)
68+
- Remote `VACUUM INTO` snapshot created world-readable beside a database with stricter permissions.
6369

6470
## [0.74.0] - 2026-09-13
6571

‎TablePro/AppDelegate.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ class AppDelegate: NSObject, NSApplicationDelegate {
102102
Task { await CloudflareTunnelManager.shared.sweepStalePidsIfNeeded() }
103103
Task { await CloudSQLProxyManager.shared.sweepStalePidsIfNeeded() }
104104
Task { await TunnelCommandManager.shared.sweepStalePidsIfNeeded() }
105+
Task { await RemoteDatabaseFileStore.shared.pruneAbandoned() }
105106

106107
NSWorkspace.shared.notificationCenter.addObserver(
107108
self, selector: #selector(handleSystemDidWake),

‎TablePro/Core/Database/RemoteDatabaseFileStore.swift‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,18 @@ struct RemoteFileManifest: Codable, Sendable, Equatable {
5555
/// report a conflict that was not there.
5656
var remoteWriteAheadLogSize: UInt64?
5757

58+
/// The log's modification time when the copy was taken. Optional, so a manifest written before
59+
/// this field existed decodes with nil and the copy is refetched once to gain a full baseline.
60+
var remoteWriteAheadLogModified: Date?
61+
5862
var downloadedSHA256: String
5963
let snapshotMethod: RemoteSnapshotMethod
6064
var fingerprint: RemoteFileFingerprint {
6165
RemoteFileFingerprint(
6266
mainSize: remoteSize,
6367
mainModified: remoteModified,
64-
writeAheadLogSize: remoteWriteAheadLogSize
68+
writeAheadLogSize: remoteWriteAheadLogSize,
69+
writeAheadLogModified: remoteWriteAheadLogModified
6570
)
6671
}
6772

@@ -184,6 +189,49 @@ actor RemoteDatabaseFileStore {
184189
try? FileManager.default.removeItem(at: directory(for: identity))
185190
Self.logger.info("Discarded the working copy for \(identity.displayOrigin, privacy: .public)")
186191
}
192+
193+
/// Marks a copy as used, so a reuse counts against the abandonment clock the same as a fresh
194+
/// fetch. Reading a file does not move a directory's modification time, so reuse would otherwise
195+
/// leave a copy that is opened daily looking abandoned once the server stopped changing.
196+
func touch(_ identity: RemoteFileIdentity) {
197+
try? FileManager.default.setAttributes(
198+
[.modificationDate: Date()], ofItemAtPath: directory(for: identity).path
199+
)
200+
}
201+
202+
/// Removes working copies nothing has used within `maxAge`, which is how a copy left behind by a
203+
/// deleted connection or a changed path is eventually reclaimed. Nothing else deletes them:
204+
/// `discard` has no routine caller, because a copy keyed by the resolved server path cannot be
205+
/// found from a connection's unresolved one at delete time.
206+
///
207+
/// Sweeping a copy is safe. A remote file connection is read-only and re-fetches on next open,
208+
/// so a removed copy costs one download and never loses data. This runs at launch, before any
209+
/// connection materializes a copy, so removing one that is about to be reopened only means it is
210+
/// fetched again. The directory's modification time is the last-used mark, moved forward by a
211+
/// fresh fetch and by `touch` on every reuse.
212+
func pruneAbandoned(olderThan maxAge: TimeInterval = 30 * 24 * 60 * 60, now: Date = Date()) {
213+
Self.pruneAbandoned(in: root, olderThan: maxAge, now: now)
214+
}
215+
216+
/// The filesystem half of `pruneAbandoned`, taking its root explicitly so a test can point it at
217+
/// a temporary directory rather than the app's real store.
218+
static func pruneAbandoned(in root: URL, olderThan maxAge: TimeInterval, now: Date) {
219+
let fileManager = FileManager.default
220+
guard let entries = try? fileManager.contentsOfDirectory(
221+
at: root,
222+
includingPropertiesForKeys: [.isDirectoryKey, .contentModificationDateKey],
223+
options: [.skipsHiddenFiles]
224+
) else { return }
225+
226+
for url in entries {
227+
let values = try? url.resourceValues(forKeys: [.isDirectoryKey, .contentModificationDateKey])
228+
guard values?.isDirectory == true else { continue }
229+
let modified = values?.contentModificationDate ?? .distantPast
230+
guard now.timeIntervalSince(modified) > maxAge else { continue }
231+
try? fileManager.removeItem(at: url)
232+
logger.info("Pruned a remote database working copy unused for over \(Int(maxAge / 86_400)) days")
233+
}
234+
}
187235
}
188236

189237
private extension JSONEncoder {

‎TablePro/Core/Database/RemoteDatabaseFileTransfer.swift‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,18 @@ struct RemoteFileFingerprint: Codable, Sendable, Equatable {
3737
let mainModified: Date
3838
let writeAheadLogSize: UInt64?
3939

40+
/// The log's modification time. A commit followed by a checkpoint can leave the main file and
41+
/// the log's size unchanged and move only this: measured on Linux with the default
42+
/// `journal_size_limit = -1`, three inserts took the row count up while `(mainSize, mainModified,
43+
/// walSize)` all stayed put and only the log's mtime advanced. Without it a busy database reads
44+
/// as untouched and the stale copy is reused.
45+
var writeAheadLogModified: Date?
46+
4047
func differs(from other: RemoteFileFingerprint) -> Bool {
4148
mainSize != other.mainSize
4249
|| mainModified != other.mainModified
4350
|| writeAheadLogSize != other.writeAheadLogSize
51+
|| writeAheadLogModified != other.writeAheadLogModified
4452
}
4553
}
4654

@@ -121,7 +129,8 @@ enum RemoteDatabaseFileTransfer {
121129
return RemoteFileFingerprint(
122130
mainSize: main.size,
123131
mainModified: main.modified,
124-
writeAheadLogSize: wal?.size
132+
writeAheadLogSize: wal?.size,
133+
writeAheadLogModified: wal?.modified
125134
)
126135
}
127136

@@ -184,6 +193,12 @@ enum RemoteDatabaseFileTransfer {
184193

185194
let workingCopy = destinationDirectory.appendingPathComponent(fileName)
186195
try replaceLocalItem(at: workingCopy, with: staging)
196+
clearStaleSidecars(
197+
layout: layout,
198+
plan: plan,
199+
destinationDirectory: destinationDirectory,
200+
fileName: fileName
201+
)
187202

188203
let manifest = RemoteFileManifest(
189204
origin: identity.displayOrigin,
@@ -195,6 +210,7 @@ enum RemoteDatabaseFileTransfer {
195210
remoteSize: before.mainSize,
196211
remoteModified: before.mainModified,
197212
remoteWriteAheadLogSize: before.writeAheadLogSize,
213+
remoteWriteAheadLogModified: before.writeAheadLogModified,
198214
downloadedSHA256: downloaded.sha256,
199215
snapshotMethod: plan.method
200216
)
@@ -208,6 +224,31 @@ enum RemoteDatabaseFileTransfer {
208224
return RemoteFetchResult(workingCopy: workingCopy, manifest: manifest, plan: plan)
209225
}
210226

227+
/// A reader that opens a working copy must not find a `-wal` or `-shm` left over from a previous
228+
/// copy of a different file, because SQLite would replay it against bytes it no longer matches.
229+
///
230+
/// A snapshot is fully checkpointed and carries no log, so every stale sidecar goes. A direct
231+
/// copy keeps the ones it just fetched (the server had them) and clears the rest, which is what
232+
/// removes a `-wal` that the server has since checkpointed away.
233+
static func clearStaleSidecars(
234+
layout: DatabaseFileLayout,
235+
plan: RemoteFetchPlan,
236+
destinationDirectory: URL,
237+
fileName: String
238+
) {
239+
let kept: Set<String>
240+
if case .directCopy(let sidecars) = plan {
241+
kept = Set(sidecars)
242+
} else {
243+
kept = []
244+
}
245+
for suffix in layout.staleAfterReplaceSuffixes where !kept.contains(suffix) {
246+
try? FileManager.default.removeItem(
247+
at: destinationDirectory.appendingPathComponent(fileName + suffix)
248+
)
249+
}
250+
}
251+
211252
/// Asks the server to write a consistent snapshot beside the database, fetches that, and removes
212253
/// it. The temp name carries a UUID so two windows fetching the same file never collide.
213254
private static func fetchViaRemoteSnapshot(
@@ -222,8 +263,15 @@ enum RemoteDatabaseFileTransfer {
222263
let snapshotPath = "\(remotePath).tablepro-snapshot-\(UUID().uuidString)"
223264
defer { session.remove(snapshotPath) }
224265

225-
let command = "\(executable) \(LibSSH2ExecChannel.shellQuoted(remotePath)) "
266+
/// `umask 077` makes `VACUUM INTO` create the snapshot `0600` from the start, so a full copy
267+
/// of a private database is never briefly world-readable beside it. The leading `rm` clears
268+
/// any snapshot a previous fetch left behind when its session died before the `defer` above
269+
/// could run; the glob is unquoted on purpose so the shell expands it, and errors are
270+
/// swallowed because a first run has nothing to remove.
271+
let quotedPath = LibSSH2ExecChannel.shellQuoted(remotePath)
272+
let vacuum = "\(executable) \(quotedPath) "
226273
+ LibSSH2ExecChannel.shellQuoted("VACUUM INTO \(sqlStringLiteral(snapshotPath))")
274+
let command = "umask 077; rm -f \(quotedPath).tablepro-snapshot-* 2>/dev/null; \(vacuum)"
227275
let result = try session.runRemoteCommand(command)
228276
guard result.succeeded else {
229277
throw SFTPError.remoteCommandFailed(

‎TablePro/Core/Database/RemoteFileTransportManager.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ actor RemoteFileTransportManager: TunnelManaging {
172172
Self.logger.info(
173173
"Reusing the working copy for \(identity.displayOrigin, privacy: .public): the server has not moved"
174174
)
175+
await store.touch(identity)
175176
return MaterializedRemoteFile(
176177
identity: identity,
177178
workingCopy: workingCopy,

‎TablePro/Core/SSH/LibSSH2ExecChannel.swift‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,13 @@ struct RemoteCommandResult: Sendable {
1313
let standardOutput: String
1414
let standardError: String
1515

16-
var succeeded: Bool { exitStatus == 0 }
16+
/// The name of the signal that killed the command, when one did. A process killed by a signal
17+
/// carries no exit status, so `libssh2_channel_get_exit_status` reports the stored `0`; without
18+
/// this, an OOM-killed or interrupted `VACUUM INTO` would read as success and its half-written
19+
/// snapshot would be adopted.
20+
let exitSignal: String?
21+
22+
var succeeded: Bool { exitStatus == 0 && exitSignal == nil }
1723

1824
var trimmedOutput: String {
1925
standardOutput.trimmingCharacters(in: .whitespacesAndNewlines)
@@ -69,14 +75,29 @@ enum LibSSH2ExecChannel {
6975
let err = drain(channel: channel, streamId: sshExtendedDataStderr)
7076
libssh2_channel_close(channel)
7177
let status = libssh2_channel_get_exit_status(channel)
78+
let signal = exitSignal(channel: channel, session: session)
7279

7380
Self.logger.debug(
74-
"remote command exited \(status, privacy: .public): \(command, privacy: .public)"
81+
"remote command exited \(status, privacy: .public) signal \(signal ?? "none", privacy: .public): \(command, privacy: .public)"
82+
)
83+
return RemoteCommandResult(
84+
exitStatus: status, standardOutput: out, standardError: err, exitSignal: signal
7585
)
76-
return RemoteCommandResult(exitStatus: status, standardOutput: out, standardError: err)
7786
}
7887
}
7988

89+
/// The signal name libssh2 recorded for the channel's process, or nil when it exited normally.
90+
/// libssh2 allocates the string, so it is freed here.
91+
private static func exitSignal(channel: OpaquePointer, session: OpaquePointer) -> String? {
92+
var signalPtr: UnsafeMutablePointer<CChar>?
93+
var signalLen = 0
94+
libssh2_channel_get_exit_signal(channel, &signalPtr, &signalLen, nil, nil, nil, nil)
95+
defer { if let signalPtr { libssh2_free(session, signalPtr) } }
96+
guard let signalPtr, signalLen > 0 else { return nil }
97+
let name = String(cString: signalPtr)
98+
return name.isEmpty ? nil : name
99+
}
100+
80101
private static func drain(channel: OpaquePointer, streamId: Int32) -> String {
81102
var collected = Data()
82103
var buffer = [CChar](repeating: 0, count: readChunk)
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
//
2+
// RemoteDatabaseFileCorrectnessTests.swift
3+
// TableProTests
4+
//
5+
6+
import Foundation
7+
import Testing
8+
9+
@testable import TablePro
10+
11+
struct RemoteDatabaseFileCorrectnessTests {
12+
private func temporaryDirectory() throws -> URL {
13+
let url = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString, isDirectory: true)
14+
try FileManager.default.createDirectory(at: url, withIntermediateDirectories: true)
15+
return url
16+
}
17+
18+
// MARK: - Fingerprint
19+
20+
@Test("A commit that moves only the write-ahead log's mtime is a change")
21+
func walModifiedIsAChange() {
22+
let base = Date(timeIntervalSince1970: 1_000_000)
23+
let recorded = RemoteFileFingerprint(
24+
mainSize: 32_768, mainModified: base, writeAheadLogSize: 234_872, writeAheadLogModified: base
25+
)
26+
let current = RemoteFileFingerprint(
27+
mainSize: 32_768, mainModified: base, writeAheadLogSize: 234_872, writeAheadLogModified: base.addingTimeInterval(5)
28+
)
29+
#expect(current.differs(from: recorded))
30+
}
31+
32+
@Test("An identical fingerprint including the log mtime is not a change")
33+
func identicalWithLogMtimeIsNotAChange() {
34+
let base = Date(timeIntervalSince1970: 1_000_000)
35+
let a = RemoteFileFingerprint(mainSize: 4_096, mainModified: base, writeAheadLogSize: 100, writeAheadLogModified: base)
36+
let b = RemoteFileFingerprint(mainSize: 4_096, mainModified: base, writeAheadLogSize: 100, writeAheadLogModified: base)
37+
#expect(!a.differs(from: b))
38+
}
39+
40+
// MARK: - Manifest codec
41+
42+
@Test("A manifest written before the log mtime field decodes with nil")
43+
func manifestWithoutLogMtimeDecodesNil() throws {
44+
let json = Data("""
45+
{"origin":"u@h:/p","username":"u","host":"h","port":22,"remotePath":"/p",
46+
"fetchedAt":"2026-01-01T00:00:00Z","remoteSize":4096,"remoteModified":"2026-01-01T00:00:00Z",
47+
"downloadedSHA256":"abc","snapshotMethod":"directCopy"}
48+
""".utf8)
49+
let decoder = JSONDecoder()
50+
decoder.dateDecodingStrategy = .iso8601
51+
let manifest = try decoder.decode(RemoteFileManifest.self, from: json)
52+
#expect(manifest.remoteWriteAheadLogModified == nil)
53+
#expect(manifest.fingerprint.writeAheadLogModified == nil)
54+
}
55+
56+
// MARK: - Stale sidecar clearing
57+
58+
@Test("A snapshot fetch clears a stale write-ahead log and shared-memory index")
59+
func snapshotClearsStaleSidecars() throws {
60+
let directory = try temporaryDirectory()
61+
let fileName = "app.db"
62+
for suffix in ["", "-wal", "-shm"] {
63+
try Data("x".utf8).write(to: directory.appendingPathComponent(fileName + suffix))
64+
}
65+
RemoteDatabaseFileTransfer.clearStaleSidecars(
66+
layout: .sqliteFamily,
67+
plan: .remoteSnapshot(executable: "sqlite3"),
68+
destinationDirectory: directory,
69+
fileName: fileName
70+
)
71+
#expect(FileManager.default.fileExists(atPath: directory.appendingPathComponent(fileName).path))
72+
#expect(!FileManager.default.fileExists(atPath: directory.appendingPathComponent(fileName + "-wal").path))
73+
#expect(!FileManager.default.fileExists(atPath: directory.appendingPathComponent(fileName + "-shm").path))
74+
}
75+
76+
@Test("A direct copy keeps the log it fetched and clears the shared-memory index")
77+
func directCopyKeepsFetchedLog() throws {
78+
let directory = try temporaryDirectory()
79+
let fileName = "app.db"
80+
for suffix in ["", "-wal", "-shm"] {
81+
try Data("x".utf8).write(to: directory.appendingPathComponent(fileName + suffix))
82+
}
83+
RemoteDatabaseFileTransfer.clearStaleSidecars(
84+
layout: .sqliteFamily,
85+
plan: .directCopy(sidecars: ["-wal"]),
86+
destinationDirectory: directory,
87+
fileName: fileName
88+
)
89+
#expect(FileManager.default.fileExists(atPath: directory.appendingPathComponent(fileName + "-wal").path))
90+
#expect(!FileManager.default.fileExists(atPath: directory.appendingPathComponent(fileName + "-shm").path))
91+
}
92+
93+
// MARK: - Killed remote command
94+
95+
@Test("A signal-killed remote command does not report success")
96+
func signalKilledCommandIsNotSuccess() {
97+
let killed = RemoteCommandResult(exitStatus: 0, standardOutput: "", standardError: "", exitSignal: "KILL")
98+
#expect(!killed.succeeded)
99+
let clean = RemoteCommandResult(exitStatus: 0, standardOutput: "ok", standardError: "", exitSignal: nil)
100+
#expect(clean.succeeded)
101+
}
102+
103+
// MARK: - Prune
104+
105+
@Test("An abandoned working copy is removed and a recently used one is kept")
106+
func pruneRemovesOnlyStaleCopies() throws {
107+
let root = try temporaryDirectory()
108+
let fresh = root.appendingPathComponent("fresh", isDirectory: true)
109+
let stale = root.appendingPathComponent("stale", isDirectory: true)
110+
try FileManager.default.createDirectory(at: fresh, withIntermediateDirectories: true)
111+
try FileManager.default.createDirectory(at: stale, withIntermediateDirectories: true)
112+
113+
let now = Date(timeIntervalSince1970: 2_000_000_000)
114+
let maxAge: TimeInterval = 30 * 24 * 60 * 60
115+
try FileManager.default.setAttributes([.modificationDate: now], ofItemAtPath: fresh.path)
116+
try FileManager.default.setAttributes(
117+
[.modificationDate: now.addingTimeInterval(-(maxAge + 86_400))], ofItemAtPath: stale.path
118+
)
119+
120+
RemoteDatabaseFileStore.pruneAbandoned(in: root, olderThan: maxAge, now: now)
121+
122+
#expect(FileManager.default.fileExists(atPath: fresh.path))
123+
#expect(!FileManager.default.fileExists(atPath: stale.path))
124+
}
125+
}

0 commit comments

Comments
 (0)