Skip to content

Commit 88e4044

Browse files
committed
fix(sync): bound a CloudKit record name so a long database path cannot crash the app
Claude-Session: https://claude.ai/code/session_01KhHdFvjmq8f8cEFyx5WGiv
1 parent e6fd930 commit 88e4044

10 files changed

Lines changed: 403 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3737
- Cell editor opening off screen when `Tab` wrapped onto a row below the visible ones.
3838
- Cell cursor left on the old column after `Tab` carried the editor to the next one.
3939
- Every data grid switching to its accessibility layout after one `Tab` press, with no assistive app attached.
40+
- Crash loop on every launch after resizing a column on a database with a long file path, with iCloud sync on. (#2575)
4041

4142
## [0.69.0] - 2026-08-27
4243

Packages/TableProCore/Sources/TableProSyncTransport/SyncRecordType.swift

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import CryptoKit
12
import Foundation
23

34
public enum SyncRecordType: String, CaseIterable, Sendable {
@@ -25,10 +26,23 @@ public enum SyncRecordType: String, CaseIterable, Sendable {
2526
}
2627
}
2728

29+
/// A name longer than `SyncRecordName.maximumLength` carries a digest of the identifier in
30+
/// place of the identifier. `CKRecord.ID(recordName:)` raises `CKException` past that length,
31+
/// and an Objective-C exception raised inside a Swift task leaves the concurrency runtime
32+
/// unwound, which crashes the app seconds later from an unrelated call site. A settings
33+
/// category embeds a database name, so on SQLite it embeds a percent-encoded file path and
34+
/// has no bound at all (#2575).
35+
///
36+
/// Every name that already fits is returned unchanged, so records that reached iCloud keep
37+
/// their identity. A name that did not fit could never be written, so nothing is orphaned.
2838
public func recordName(for id: String) -> String {
29-
recordNamePrefix + id
39+
let name = recordNamePrefix + id
40+
guard (name as NSString).length > SyncRecordName.maximumLength else { return name }
41+
return recordNamePrefix + SyncRecordName.digestPrefix + SyncRecordName.digest(of: id)
3042
}
3143

44+
/// The identifier is only recoverable when `recordName(for:)` did not shorten it, so the push
45+
/// path resolves a saved record through the identifiers it sent rather than through this.
3246
public static func parse(recordName: String) -> (type: SyncRecordType, id: String)? {
3347
for type in longestPrefixFirst where recordName.hasPrefix(type.recordNamePrefix) {
3448
return (type, String(recordName.dropFirst(type.recordNamePrefix.count)))
@@ -39,3 +53,20 @@ public enum SyncRecordType: String, CaseIterable, Sendable {
3953
private static let longestPrefixFirst: [SyncRecordType] = allCases
4054
.sorted { $0.recordNamePrefix.count > $1.recordNamePrefix.count }
4155
}
56+
57+
/// CloudKit's own limit on a record name, and how a name that would exceed it is shortened.
58+
public enum SyncRecordName {
59+
/// Measured against the CloudKit framework: 255 UTF-16 code units pass and 256 raise, and the
60+
/// count is of UTF-16 units rather than characters or bytes (250 two-byte characters pass at
61+
/// 500 UTF-8 bytes; 128 emoji raise at 256 UTF-16 units). `scripts/check-cloudkit-record-name-limit.sh`
62+
/// re-measures it.
63+
public static let maximumLength = 255
64+
65+
/// Names the shortening in the CloudKit dashboard, and keeps a digest from colliding with an
66+
/// identifier that is genuinely 64 hexadecimal characters, such as a favorite's sync id.
67+
public static let digestPrefix = "sha256-"
68+
69+
public static func digest(of id: String) -> String {
70+
SHA256.hash(data: Data(id.utf8)).map { String(format: "%02x", $0) }.joined()
71+
}
72+
}
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import Foundation
2+
import Testing
3+
4+
@Suite("Record names are only built where CloudKit's length limit is enforced")
5+
struct SyncRecordNameConstructionTests {
6+
private static let repositoryRoot = URL(fileURLWithPath: #filePath)
7+
.deletingLastPathComponent()
8+
.deletingLastPathComponent()
9+
.deletingLastPathComponent()
10+
.deletingLastPathComponent()
11+
.deletingLastPathComponent()
12+
13+
private static let sourceRoots = [
14+
"TablePro",
15+
"TableProMobile/TableProMobile",
16+
"Packages/TableProCore/Sources"
17+
]
18+
19+
private static let permittedPaths: Set<String> = [
20+
"TablePro/Core/Sync/SyncRecordMapper.swift",
21+
"Packages/TableProCore/Sources/TableProSync/SyncRecordMapper.swift"
22+
]
23+
24+
@Test("Every source root the check covers is where the check expects it")
25+
func sourceRootsExist() {
26+
for path in Self.sourceRoots {
27+
let url = Self.repositoryRoot.appendingPathComponent(path)
28+
#expect(FileManager.default.fileExists(atPath: url.path), """
29+
\(path) has moved, so this check would pass vacuously. Update sourceRoots.
30+
""")
31+
}
32+
}
33+
34+
@Test("Every mapper the check permits is where the check expects it")
35+
func permittedMappersExist() {
36+
for path in Self.permittedPaths {
37+
let url = Self.repositoryRoot.appendingPathComponent(path)
38+
#expect(FileManager.default.fileExists(atPath: url.path), """
39+
\(path) has moved. Update permittedPaths.
40+
""")
41+
}
42+
}
43+
44+
@Test("No shipping source constructs a CKRecord.ID outside the mappers")
45+
func recordIdsComeFromTheMappers() {
46+
var offenders: [String] = []
47+
48+
for root in Self.sourceRoots {
49+
let rootURL = Self.repositoryRoot.appendingPathComponent(root)
50+
guard let files = FileManager.default.enumerator(
51+
at: rootURL,
52+
includingPropertiesForKeys: nil
53+
) else { continue }
54+
55+
for case let url as URL in files where url.pathExtension == "swift" {
56+
let path = url.path.replacingOccurrences(of: Self.repositoryRoot.path + "/", with: "")
57+
guard !Self.permittedPaths.contains(path),
58+
let source = try? String(contentsOf: url, encoding: .utf8) else { continue }
59+
60+
for (offset, line) in source.components(separatedBy: .newlines).enumerated() {
61+
let code = line.trimmingCharacters(in: .whitespaces)
62+
guard !code.hasPrefix("//"), code.contains("CKRecord.ID(recordName:") else { continue }
63+
offenders.append("\(path):\(offset + 1): \(code)")
64+
}
65+
}
66+
}
67+
68+
#expect(offenders.isEmpty, """
69+
CKRecord.ID(recordName:) raises CKException past 255 UTF-16 code units, and an \
70+
Objective-C exception raised inside a Swift task crashes the app from an unrelated call \
71+
site seconds later. SyncRecordType.recordName(for:) is the only thing that bounds the \
72+
name, so go through SyncRecordMapper.recordID(type:id:in:).
73+
\(offenders.joined(separator: "\n"))
74+
""")
75+
}
76+
}

Packages/TableProCore/Tests/TableProSyncTests/SyncRecordTypeTests.swift

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,63 @@ struct SyncRecordTypeTests {
6262
let prefixes = SyncRecordType.allCases.map(\.recordNamePrefix)
6363
#expect(Set(prefixes).count == prefixes.count)
6464
}
65+
66+
@Test("A name that already fits is returned unchanged", arguments: SyncRecordType.allCases)
67+
func namesThatFitAreUnchanged(_ type: SyncRecordType) {
68+
let id = String(repeating: "a", count: SyncRecordName.maximumLength - type.recordNamePrefix.count)
69+
let name = type.recordName(for: id)
70+
#expect(name == type.recordNamePrefix + id)
71+
#expect((name as NSString).length == SyncRecordName.maximumLength)
72+
}
73+
74+
@Test("A name one unit too long is shortened", arguments: SyncRecordType.allCases)
75+
func namesPastTheLimitAreShortened(_ type: SyncRecordType) {
76+
let id = String(repeating: "a", count: SyncRecordName.maximumLength - type.recordNamePrefix.count + 1)
77+
let name = type.recordName(for: id)
78+
#expect((name as NSString).length <= SyncRecordName.maximumLength)
79+
#expect(name.hasPrefix(type.recordNamePrefix + SyncRecordName.digestPrefix))
80+
}
81+
82+
/// The limit CloudKit enforces counts UTF-16 code units, so a name of 128 emoji is over it at
83+
/// 128 characters. `scripts/check-cloudkit-record-name-limit.sh` measures that.
84+
@Test("The limit counts UTF-16 code units, not characters")
85+
func theLimitCountsUTF16CodeUnits() {
86+
let id = String(repeating: "😀", count: 200)
87+
#expect(id.count < SyncRecordName.maximumLength)
88+
let name = SyncRecordType.settings.recordName(for: id)
89+
#expect((name as NSString).length <= SyncRecordName.maximumLength)
90+
#expect(name.hasPrefix("Settings_" + SyncRecordName.digestPrefix))
91+
}
92+
93+
@Test("Shortening is stable, so two devices agree on the record")
94+
func shorteningIsDeterministic() {
95+
let id = String(repeating: "path/to/database.sqlite", count: 40)
96+
#expect(SyncRecordType.settings.recordName(for: id) == SyncRecordType.settings.recordName(for: id))
97+
#expect(
98+
SyncRecordType.settings.recordName(for: id) == "Settings_sha256-"
99+
+ SyncRecordName.digest(of: id)
100+
)
101+
}
102+
103+
@Test("Two long identifiers do not collapse onto one record")
104+
func distinctLongIdentifiersStayDistinct() {
105+
let base = String(repeating: "a", count: 300)
106+
#expect(SyncRecordType.settings.recordName(for: base) != SyncRecordType.settings.recordName(for: base + "b"))
107+
}
108+
109+
/// The column layout category that produced #2575: a connection UUID, a percent-encoded
110+
/// SQLite file path, an empty schema and a table name.
111+
@Test("A long SQLite path produces a name CloudKit accepts")
112+
func aLongSQLitePathFits() {
113+
let path = "/Users/example/projects/acme/api/.wrangler/state/v3/d1"
114+
+ "/miniflare-D1DatabaseObject/"
115+
+ String(repeating: "f", count: 64) + ".sqlite"
116+
let parts = [UUID().uuidString, path, "", "d1_migrations"]
117+
.map { $0.addingPercentEncoding(withAllowedCharacters: .alphanumerics) ?? $0 }
118+
let category = "columnLayout." + parts.joined(separator: ".")
119+
120+
#expect((("Settings_" + category) as NSString).length > SyncRecordName.maximumLength)
121+
#expect((SyncRecordType.settings.recordName(for: category) as NSString).length
122+
<= SyncRecordName.maximumLength)
123+
}
65124
}

TablePro/Core/Sync/SyncCoordinator.swift

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -391,19 +391,20 @@ final class SyncCoordinator {
391391

392392
guard !recordsToSave.isEmpty || !uniqueDeletions.isEmpty else { return }
393393

394+
let identities = SyncRecordMapper.identities(for: pushedLocalIds(), in: zoneID)
394395
let outcome = try await engine.push(records: recordsToSave, deletions: uniqueDeletions)
395396

396397
recordCache.store(Array(outcome.savedRecords.values))
397398
recordCache.remove(Array(outcome.deletedRecordIDs))
398399

399400
for recordID in outcome.savedRecords.keys {
400-
guard let parsed = SyncRecordMapper.parse(recordName: recordID.recordName) else { continue }
401-
changeTracker.clearDirty(parsed.type, id: parsed.id)
401+
guard let identity = identities[recordID] else { continue }
402+
changeTracker.clearDirty(identity.type, id: identity.id)
402403
}
403404

404405
for recordID in outcome.deletedRecordIDs {
405-
guard let parsed = SyncRecordMapper.parse(recordName: recordID.recordName) else { continue }
406-
metadataStorage.removeTombstone(parsed.id, type: parsed.type)
406+
guard let identity = identities[recordID] else { continue }
407+
metadataStorage.removeTombstone(identity.id, type: identity.type)
407408
}
408409

409410
let savedCount = outcome.savedRecords.count
@@ -417,6 +418,20 @@ final class SyncCoordinator {
417418
throw SyncError.pushRejected(count: outcome.failures.count, detail: firstFailure.message)
418419
}
419420

421+
/// Every local identifier this push can have sent. `SyncChangeTracker` is not isolated to this
422+
/// actor, so the sets can move under an await; a record whose identifier is missing from the
423+
/// snapshot is left dirty and pushed again rather than cleared against the wrong entry.
424+
private func pushedLocalIds() -> [SyncRecordType: Set<String>] {
425+
var localIds: [SyncRecordType: Set<String>] = [:]
426+
for type in SyncRecordType.allCases {
427+
let ids = changeTracker.dirtyRecords(for: type)
428+
.union(metadataStorage.tombstones(for: type).map(\.id))
429+
guard !ids.isEmpty else { continue }
430+
localIds[type] = ids
431+
}
432+
return localIds
433+
}
434+
420435
// MARK: - Pull
421436

422437
nonisolated static func isTokenExpired(_ error: Error) -> Bool {
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
//
2+
// SyncRecordIdentity.swift
3+
// TablePro
4+
//
5+
6+
import Foundation
7+
import TableProSyncTransport
8+
9+
/// The local identifier behind a record the push sent, kept because a CloudKit record name is an
10+
/// identity rather than an encoding of that identifier.
11+
struct SyncRecordIdentity: Hashable, Sendable {
12+
let type: SyncRecordType
13+
let id: String
14+
}

TablePro/Core/Sync/SyncRecordMapper.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,23 @@ struct SyncRecordMapper {
4545
SyncRecordType.parse(recordName: recordName)
4646
}
4747

48+
/// Maps every record the push is about to send back onto the local identifier it was built
49+
/// from. `SyncRecordType.recordName(for:)` shortens an identifier that would take the name
50+
/// past CloudKit's limit, so a saved record cannot be read back through `parse(recordName:)`
51+
/// without clearing the wrong dirty entry and pushing the same record on every sync forever.
52+
static func identities(
53+
for localIds: [SyncRecordType: Set<String>],
54+
in zone: CKRecordZone.ID
55+
) -> [CKRecord.ID: SyncRecordIdentity] {
56+
var identities: [CKRecord.ID: SyncRecordIdentity] = [:]
57+
for (type, ids) in localIds {
58+
for id in ids {
59+
identities[recordID(type: type, id: id, in: zone)] = SyncRecordIdentity(type: type, id: id)
60+
}
61+
}
62+
return identities
63+
}
64+
4865
// MARK: - Connection
4966

5067
static func toCKRecord(

TableProTests/Core/Storage/ColumnLayoutSyncTests.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,25 @@ struct ColumnLayoutSyncTests {
5959
func categoryPrefix() {
6060
#expect(FileColumnLayoutPersister.syncCategory(for: "abc").hasPrefix(FileColumnLayoutPersister.syncCategoryPrefix))
6161
}
62+
63+
/// A SQLite database name is a file path, and the storage key percent-encodes every character
64+
/// that is not alphanumeric, so a wrangler path takes the record name past what CloudKit
65+
/// accepts. `CKRecord.ID(recordName:)` raised there, and the app crashed seconds later from an
66+
/// unrelated call site, on every launch (#2575).
67+
@Test("A long SQLite path still yields a record name CloudKit accepts")
68+
func longSQLitePathYieldsAcceptableRecordName() {
69+
let path = "/Users/example/projects/acme/api/.wrangler/state/v3/d1"
70+
+ "/miniflare-D1DatabaseObject/" + String(repeating: "f", count: 64) + ".sqlite"
71+
let tableKey = ColumnLayoutTableKey(
72+
connectionId: UUID(),
73+
databaseName: path,
74+
schemaName: nil,
75+
tableName: "d1_migrations"
76+
)
77+
let category = FileColumnLayoutPersister.syncCategory(for: tableKey.storageKey)
78+
79+
#expect((("Settings_" + category) as NSString).length > SyncRecordName.maximumLength)
80+
#expect((SyncRecordType.settings.recordName(for: category) as NSString).length
81+
<= SyncRecordName.maximumLength)
82+
}
6283
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
//
2+
// SyncRecordIdentityTests.swift
3+
// TableProTests
4+
//
5+
6+
import CloudKit
7+
import Foundation
8+
@testable import TablePro
9+
import TableProSyncTransport
10+
import Testing
11+
12+
@Suite("Push identities survive a shortened record name")
13+
@MainActor
14+
struct SyncRecordIdentityTests {
15+
private static let zone = CKRecordZone.ID(zoneName: "TableProZone", ownerName: CKCurrentUserDefaultName)
16+
17+
private static func longColumnLayoutCategory() -> String {
18+
let path = "/Users/example/projects/acme/api/.wrangler/state/v3/d1"
19+
+ "/miniflare-D1DatabaseObject/" + String(repeating: "f", count: 64) + ".sqlite"
20+
let key = ColumnLayoutTableKey(
21+
connectionId: UUID(),
22+
databaseName: path,
23+
schemaName: nil,
24+
tableName: "d1_migrations"
25+
)
26+
return FileColumnLayoutPersister.syncCategory(for: key.storageKey)
27+
}
28+
29+
@Test("A category too long for a record name still resolves back to itself")
30+
func longCategoryResolvesBack() {
31+
let category = Self.longColumnLayoutCategory()
32+
let identities = SyncRecordMapper.identities(for: [.settings: [category]], in: Self.zone)
33+
let recordID = SyncRecordMapper.recordID(type: .settings, id: category, in: Self.zone)
34+
35+
#expect(identities[recordID] == SyncRecordIdentity(type: .settings, id: category))
36+
}
37+
38+
/// The record name carries a digest once it is shortened, so the identifier the push needs
39+
/// back is not in it. Reading it out of the name clears the wrong dirty entry, which leaves
40+
/// the real one dirty and pushes the same record on every sync forever.
41+
@Test("The shortened record name no longer carries the category")
42+
func aShortenedNameDoesNotCarryTheCategory() {
43+
let category = Self.longColumnLayoutCategory()
44+
#expect((("Settings_" + category) as NSString).length > SyncRecordName.maximumLength, """
45+
The fixture no longer exceeds CloudKit's limit, so this check would pass vacuously.
46+
""")
47+
48+
let recordID = SyncRecordMapper.recordID(type: .settings, id: category, in: Self.zone)
49+
let parsed = SyncRecordMapper.parse(recordName: recordID.recordName)
50+
51+
#expect((recordID.recordName as NSString).length <= SyncRecordName.maximumLength)
52+
#expect(parsed?.type == .settings)
53+
#expect(parsed?.id != category)
54+
}
55+
56+
@Test("A short identifier resolves back without being shortened")
57+
func shortIdentifierResolvesBack() {
58+
let id = UUID().uuidString
59+
let identities = SyncRecordMapper.identities(for: [.connection: [id]], in: Self.zone)
60+
let recordID = SyncRecordMapper.recordID(type: .connection, id: id, in: Self.zone)
61+
62+
#expect(recordID.recordName == "Connection_" + id)
63+
#expect(identities[recordID] == SyncRecordIdentity(type: .connection, id: id))
64+
}
65+
66+
@Test("Identities keep every type apart")
67+
func identitiesKeepTypesApart() {
68+
let id = UUID().uuidString
69+
let identities = SyncRecordMapper.identities(
70+
for: [.connection: [id], .group: [id], .tag: [id]],
71+
in: Self.zone
72+
)
73+
74+
#expect(identities.count == 3)
75+
#expect(identities[SyncRecordMapper.recordID(type: .group, id: id, in: Self.zone)]?.type == .group)
76+
}
77+
}

0 commit comments

Comments
 (0)