diff --git a/.github/workflows/repo-hygiene.yml b/.github/workflows/repo-hygiene.yml index cac3921a18..f42afe599f 100644 --- a/.github/workflows/repo-hygiene.yml +++ b/.github/workflows/repo-hygiene.yml @@ -21,6 +21,7 @@ on: - "project.yml" - "TablePro.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved" - "TablePro/Resources/ThirdPartyLicenses/licenses.yml" + - ".swiftlint.yml" push: branches: [main] paths: @@ -36,6 +37,7 @@ on: - "project.yml" - "TablePro.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved" - "TablePro/Resources/ThirdPartyLicenses/licenses.yml" + - ".swiftlint.yml" workflow_dispatch: concurrency: @@ -106,6 +108,16 @@ jobs: - name: Check plugin sources that log import the module that defines it run: python3 scripts/ci/check-plugin-os-import.py + # SwiftLint runs in CI only on a release tag, and its included: never reaches Plugins/, + # where fifteen log lines published an error's text unseen. This applies that one rule to + # the app, the packages and the plugins on every pull request, with the regex read from + # .swiftlint.yml. + - name: Check no source publishes an error's text to the system log + run: python3 scripts/ci/check-log-privacy.py + + - name: Validate the log privacy check + run: python3 scripts/ci/test_check_log_privacy.py + # A plugin's String(localized:) resolves against Bundle.main, which is the host app, so its # strings live in the app's catalog. Xcode extracts per target and the plugin targets are not # the app target, so nothing puts them there: 537 strings had never reached a translator. diff --git a/.swiftlint.yml b/.swiftlint.yml index 77d241a167..569e8de0ab 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -196,6 +196,6 @@ custom_rules: severity: error public_error_text_in_log: name: "Error Text In The System Log" - regex: '\.localizedDescription\s*,\s*privacy:\s*\.public' - message: "A driver or Foundation error's text carries row values, paths and server messages. Publish `error.publicLogShape` and log the description at .private." + regex: '(\.(localizedDescription|errorDescription|failureReason)(\.prefix\(\d+\))?(\s*\?\?\s*"[^"]*")?|String\(describing:\s*\w*[eE]rr(or)?\)|\\\((?!(has|is|did|was|should)[A-Z])(\w*[eE]rr(or)?|e)(\.(message|description|debugDescription))?)\s*,\s*privacy:\s*\.public' + message: "A driver or Foundation error's text carries row values, paths and server messages. Publish `error.publicLogShape`, or `LogRedaction.publicDescription(of: error)` in a plugin, and log the description at .private." severity: error diff --git a/CHANGELOG.md b/CHANGELOG.md index 334f85a8da..bb2fc2dc72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -347,6 +347,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - A locked launch on iPhone and iPad connecting to the last session, and asking to trust a host key, before Face ID was answered. - Code inside a plugin bundle, and its resource envelope, were not verified before the bundle was loaded. - The system log carried query text, schema and table names, file paths and driver error messages, which can hold row values. +- Driver error messages and server replies published in the system log by database plugins. - A chat tool registered at runtime could take the name of a tool TablePro ships. - An open connection, a sheet and the app switcher preview left usable or visible behind the iOS app lock. - **Require Face ID** turned off on iPhone and iPad without authenticating. diff --git a/Packages/TableProCore/Package.swift b/Packages/TableProCore/Package.swift index f34203bc38..85a2016353 100644 --- a/Packages/TableProCore/Package.swift +++ b/Packages/TableProCore/Package.swift @@ -27,6 +27,7 @@ let package = Package( .library(name: "TableProWeaviateCore", targets: ["TableProWeaviateCore"]), .library(name: "TableProNumberFormatting", targets: ["TableProNumberFormatting"]), .library(name: "TableProDocumentPath", targets: ["TableProDocumentPath"]), + .library(name: "TableProLogRedaction", targets: ["TableProLogRedaction"]), .library(name: "TableProR2SQLCore", targets: ["TableProR2SQLCore"]), .library(name: "TableProConnectionLibrary", targets: ["TableProConnectionLibrary"]), .library(name: "TableProSQLGrammar", targets: ["TableProSQLGrammar"]), @@ -45,6 +46,11 @@ let package = Package( dependencies: [], path: "Sources/TableProDocumentPath" ), + .target( + name: "TableProLogRedaction", + dependencies: [], + path: "Sources/TableProLogRedaction" + ), .target( name: "TableProCoreTypes", dependencies: [], @@ -185,6 +191,11 @@ let package = Package( dependencies: ["TableProDocumentPath"], path: "Tests/TableProDocumentPathTests" ), + .testTarget( + name: "TableProLogRedactionTests", + dependencies: ["TableProLogRedaction"], + path: "Tests/TableProLogRedactionTests" + ), .testTarget( name: "TableProModelsTests", dependencies: ["TableProModels", "TableProPluginKit"], diff --git a/Packages/TableProCore/Sources/TableProLogRedaction/LogRedaction.swift b/Packages/TableProCore/Sources/TableProLogRedaction/LogRedaction.swift new file mode 100644 index 0000000000..eff36bcde7 --- /dev/null +++ b/Packages/TableProCore/Sources/TableProLogRedaction/LogRedaction.swift @@ -0,0 +1,52 @@ +import Foundation + +public protocol PubliclyLoggableError: Error { + var publicLogDescription: String { get } +} + +public enum LogRedaction { + private static let maximumDomainLength = 64 + private static let identifierPunctuation: Set = [".", "_", "-"] + + public static func publicDescription(of error: Error) -> String { + if let loggable = error as? PubliclyLoggableError { + return loggable.publicLogDescription + } + + let typeName = String(describing: type(of: error)) + let mirror = Mirror(reflecting: error) + + guard mirror.displayStyle == .enum else { + return bridgedShape(of: error as NSError, typeName: typeName) + } + if let caseName = mirror.children.first?.label { + return "\(typeName).\(caseName)" + } + guard !describesItself(type(of: error)) else { return typeName } + return "\(typeName).\(error)" + } + + private static func bridgedShape(of error: NSError, typeName: String) -> String { + let domain = error.domain + guard !domain.hasSuffix(typeName), isConstantIdentifier(domain) else { + return "\(typeName)(\(error.code))" + } + return "\(typeName)(\(domain), \(error.code))" + } + + private static func isConstantIdentifier(_ domain: String) -> Bool { + let scalars = domain.unicodeScalars + guard domain.utf8.count <= maximumDomainLength, let first = scalars.first, isASCIILetter(first) else { + return false + } + return scalars.allSatisfy { isASCIILetter($0) || ("0"..."9").contains($0) || identifierPunctuation.contains($0) } + } + + private static func isASCIILetter(_ scalar: Unicode.Scalar) -> Bool { + ("a"..."z").contains(scalar) || ("A"..."Z").contains(scalar) + } + + private static func describesItself(_ type: any Error.Type) -> Bool { + type is CustomStringConvertible.Type || type is CustomDebugStringConvertible.Type + } +} diff --git a/Packages/TableProCore/Tests/TableProLogRedactionTests/LogRedactionTests.swift b/Packages/TableProCore/Tests/TableProLogRedactionTests/LogRedactionTests.swift new file mode 100644 index 0000000000..48a9396596 --- /dev/null +++ b/Packages/TableProCore/Tests/TableProLogRedactionTests/LogRedactionTests.swift @@ -0,0 +1,91 @@ +import Foundation +import Testing + +@testable import TableProLogRedaction + +@Suite("Log redaction shapes") +struct LogRedactionShapeTests { + private static let serverText = + "ERROR: duplicate key value violates unique constraint \"users_email_key\" Key (email)=(a@b.com) already exists." + + private enum DriverError: Error { + case executionFailed(String) + case disconnected + } + + private enum DescribedError: Error, CustomStringConvertible { + case refused + + var description: String { LogRedactionShapeTests.serverText } + } + + private enum DebugDescribedError: Error, CustomDebugStringConvertible { + case refused + + var debugDescription: String { LogRedactionShapeTests.serverText } + } + + private struct BoxedError: Error { + let serverText: String + } + + private enum SafeError: PubliclyLoggableError { + case readOnlyConnection + + var publicLogDescription: String { "the connection is read-only" } + } + + @Test("An enum case's payload never reaches the public description") + func enumPayloadIsDropped() { + #expect(LogRedaction.publicDescription(of: DriverError.executionFailed(Self.serverText)) == "DriverError.executionFailed") + } + + @Test("A case with no payload keeps its name") + func payloadlessCaseKeepsItsName() { + #expect(LogRedaction.publicDescription(of: DriverError.disconnected) == "DriverError.disconnected") + } + + @Test("A case whose type writes its own description publishes the type alone") + func customDescriptionIsNotPublished() { + #expect(LogRedaction.publicDescription(of: DescribedError.refused) == "DescribedError") + #expect(LogRedaction.publicDescription(of: DebugDescribedError.refused) == "DebugDescribedError") + } + + @Test("A struct error publishes its type and bridged code, not its stored text") + func structErrorPublishesItsShape() { + let shape = LogRedaction.publicDescription(of: BoxedError(serverText: Self.serverText)) + + #expect(shape.hasPrefix("BoxedError(")) + #expect(!shape.contains("a@b.com")) + } + + @Test("A system error keeps the domain and code a reader can act on") + func systemDomainIsKept() { + let shape = LogRedaction.publicDescription(of: NSError(domain: NSPOSIXErrorDomain, code: 61)) + + #expect(shape == "NSError(\(NSPOSIXErrorDomain), 61)") + } + + @Test("A constant domain from another framework is published with its code") + func constantDomainIsKept() { + #expect(LogRedaction.publicDescription(of: NSError(domain: "CKErrorDomain", code: 3)) == "NSError(CKErrorDomain, 3)") + #expect(LogRedaction.publicDescription(of: NSError(domain: "com.example.sync-kit", code: 2)) == "NSError(com.example.sync-kit, 2)") + } + + @Test("A domain built from text is never published, whatever it holds", arguments: [ + serverText, + "a@b.com", + "/Users/someone/Library/db.sqlite", + "Key(email)", + String(repeating: "a", count: 65), + "" + ]) + func textDomainIsDropped(domain: String) { + #expect(LogRedaction.publicDescription(of: NSError(domain: domain, code: 7)) == "NSError(7)") + } + + @Test("An error that declares its description safe is published in full") + func publiclyLoggableErrorIsPublishedInFull() { + #expect(LogRedaction.publicDescription(of: SafeError.readOnlyConnection) == "the connection is read-only") + } +} diff --git a/Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreConnection.swift b/Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreConnection.swift index 588c86258b..ed04d45857 100644 --- a/Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreConnection.swift +++ b/Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreConnection.swift @@ -203,16 +203,14 @@ public final class OracleCoreConnection: @unchecked Sendable { throw connectError(from: sqlError) } catch let nioSslError as NIOSSLError { let detail = String(describing: nioSslError) - osLogger.error("Oracle TLS error: \(detail, privacy: .public)") - throw OracleCoreError.tlsHandshakeFailed( - kind: OracleSSLClassifier.classifyTLSFailure(detail) ?? .unknown, - serverMessage: detail - ) + let kind = OracleSSLClassifier.classifyTLSFailure(detail) ?? .unknown + osLogger.error("Oracle TLS error: \(String(describing: kind), privacy: .public) \(detail, privacy: .private)") + throw OracleCoreError.tlsHandshakeFailed(kind: kind, serverMessage: detail) } catch let coreError as OracleCoreError { throw coreError } catch { let detail = String(describing: error) - osLogger.error("Oracle connection failed: \(detail, privacy: .public)") + osLogger.error("Oracle connection failed: \(String(describing: type(of: error)), privacy: .public) \(detail, privacy: .private)") if let kind = OracleSSLClassifier.classifyTLSFailure(detail) { throw OracleCoreError.tlsHandshakeFailed(kind: kind, serverMessage: detail) } @@ -612,7 +610,7 @@ public final class OracleCoreConnection: @unchecked Sendable { code, serverErrorNumber: sqlError.serverInfo.map { Int($0.number) } ) else { guard let serverMessage = sqlError.serverInfo?.message else { - osLogger.error("Oracle statement failed: \(String(describing: sqlError), privacy: .public)") + osLogger.error("Oracle statement failed with \(code, privacy: .public): \(String(describing: sqlError), privacy: .private)") return .queryFailed(String(format: OracleCoreError.driverErrorFormat, code)) } return .queryFailed(serverMessage) @@ -655,7 +653,7 @@ public final class OracleCoreConnection: @unchecked Sendable { default: markConnectionDead(reason: .transportError) let detail = String(describing: error) - osLogger.error("Oracle connection reset after a transport error: \(detail, privacy: .public)") + osLogger.error("Oracle connection reset after a transport error: \(String(describing: type(of: error)), privacy: .public) \(detail, privacy: .private)") return OracleCoreError.queryFailed(detail) } } @@ -894,7 +892,7 @@ public final class OracleCoreConnection: @unchecked Sendable { return unsupportedPlaceholder(for: cell.dataType) } } catch { - osLogger.error("Oracle decode failed for column '\(cell.columnName, privacy: .public)': \(String(describing: error), privacy: .public)") + osLogger.error("Oracle decode failed for column '\(cell.columnName, privacy: .private(mask: .hash))': \(String(describing: type(of: error)), privacy: .public) \(String(describing: error), privacy: .private)") return "" } } diff --git a/Plugins/BigQueryDriverPlugin/BigQueryConnection.swift b/Plugins/BigQueryDriverPlugin/BigQueryConnection.swift index 96996c76e1..08365e2798 100644 --- a/Plugins/BigQueryDriverPlugin/BigQueryConnection.swift +++ b/Plugins/BigQueryDriverPlugin/BigQueryConnection.swift @@ -1,6 +1,7 @@ import Foundation import os import TableProGoogleCloud +import TableProLogRedaction import TableProPluginKit internal struct BQTableFieldSchema: Codable, Sendable { @@ -714,7 +715,7 @@ internal final class BigQueryConnection: @unchecked Sendable { do { try await cancelJob(jobId: jobId, location: location) } catch { - Self.logger.warning("BigQuery job cancel failed: \(error.localizedDescription, privacy: .public)") + Self.logger.warning("BigQuery job cancel failed: \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)") } } } diff --git a/Plugins/BigQueryDriverPlugin/BigQueryPluginDriver.swift b/Plugins/BigQueryDriverPlugin/BigQueryPluginDriver.swift index 7e34fac2ca..55e6a7d6cd 100644 --- a/Plugins/BigQueryDriverPlugin/BigQueryPluginDriver.swift +++ b/Plugins/BigQueryDriverPlugin/BigQueryPluginDriver.swift @@ -1,6 +1,7 @@ import Foundation import os import TableProGoogleCloud +import TableProLogRedaction import TableProPluginKit internal final class BigQueryPluginDriver: PluginDatabaseDriver, @unchecked Sendable { @@ -200,7 +201,7 @@ internal final class BigQueryPluginDriver: PluginDatabaseDriver, @unchecked Send } } } catch { - Self.logger.info("Could not auto-select a dataset: \(error.localizedDescription, privacy: .public)") + Self.logger.info("Could not auto-select a dataset: \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)") } } @@ -380,7 +381,7 @@ internal final class BigQueryPluginDriver: PluginDatabaseDriver, @unchecked Send return try await bulkColumns(datasetId: datasetId) } catch { Self.logger.info( - "Bulk column fetch failed, reading tables one by one: \(error.localizedDescription, privacy: .public)" + "Bulk column fetch failed, reading tables one by one: \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)" ) var columns: [String: [PluginColumnInfo]] = [:] for table in try await fetchTables(schema: schema) { diff --git a/Plugins/CSVInspectorPlugin/CSVDocument.swift b/Plugins/CSVInspectorPlugin/CSVDocument.swift index e5c67636fa..0af81b3c6d 100644 --- a/Plugins/CSVInspectorPlugin/CSVDocument.swift +++ b/Plugins/CSVInspectorPlugin/CSVDocument.swift @@ -1,6 +1,7 @@ import AppKit -import TableProPluginKit import os +import TableProLogRedaction +import TableProPluginKit public final class CSVDocument: NSDocument, CSVConfigurableDocument { static let logger = Logger(subsystem: "com.TablePro", category: "CSVInspector") @@ -105,7 +106,7 @@ public final class CSVDocument: NSDocument, CSVConfigurableDocument { do { try revert(toContentsOf: url, ofType: fileType ?? "public.comma-separated-values-text") } catch { - Self.logger.error("Auto-revert failed: \(error.localizedDescription, privacy: .public)") + Self.logger.error("Auto-revert failed: \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)") } } diff --git a/Plugins/ElasticsearchDriverPlugin/ElasticsearchConnection.swift b/Plugins/ElasticsearchDriverPlugin/ElasticsearchConnection.swift index 58f0df4067..fbde7b618f 100644 --- a/Plugins/ElasticsearchDriverPlugin/ElasticsearchConnection.swift +++ b/Plugins/ElasticsearchDriverPlugin/ElasticsearchConnection.swift @@ -145,15 +145,15 @@ internal final class ElasticsearchConnection: NSObject, @unchecked Sendable { let response = try await request(method: "GET", path: "/\(encode(index))/_mapping") guard response.statusCode == 200 else { throw mapError(response, fallback: "Failed to fetch mapping") } guard let json = response.json as? [String: Any] else { - Self.logger.error("mappingProperties \(index, privacy: .public): response.json not a dictionary; raw=\(response.rawText.prefix(300), privacy: .public)") + Self.logger.error("mappingProperties \(index, privacy: .private(mask: .hash)): response.json not a dictionary; raw=\(response.rawText.prefix(300), privacy: .private)") return [] } let properties = ElasticsearchMappingFlattener.properties(fromMappingResponse: json, index: index) let columns = ElasticsearchMappingFlattener.flattenMapping(properties: properties) Self.logger.debug(""" - mappingProperties \(index, privacy: .public): topKeys=[\(json.keys.joined(separator: ","), privacy: .public)] \ + mappingProperties \(index, privacy: .private(mask: .hash)): topKeys=[\(json.keys.joined(separator: ","), privacy: .private)] \ propertyCount=\(properties.count) columnCount=\(columns.count) \ - columns=[\(columns.map { "\($0.name):\($0.type)\($0.hasKeywordSubfield ? "+kw" : "")" }.joined(separator: ","), privacy: .public)] + columns=[\(columns.map { "\($0.name):\($0.type)\($0.hasKeywordSubfield ? "+kw" : "")" }.joined(separator: ","), privacy: .private)] """) return columns } diff --git a/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver+Execution.swift b/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver+Execution.swift index 01ff30a590..9848784a35 100644 --- a/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver+Execution.swift +++ b/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver+Execution.swift @@ -175,9 +175,9 @@ extension ElasticsearchPluginDriver { let fields = ElasticsearchMappingFlattener.fieldInfo(from: mappingColumns) Self.logger.debug(""" - executeSearch index=\(parsed.index, privacy: .public) from=\(parsed.from) size=\(parsed.size) \ + executeSearch index=\(parsed.index, privacy: .private(mask: .hash)) from=\(parsed.from) size=\(parsed.size) \ logic=\(parsed.logicMode, privacy: .public) \ - filters=\(parsed.filters.map { "\($0.column) \($0.op) \($0.value)" }.joined(separator: " | "), privacy: .public) \ + filters=\(parsed.filters.map { "\($0.column) \($0.op) \($0.value)" }.joined(separator: " | "), privacy: .private) \ sorts=\(parsed.sorts.map { "\($0.column) \($0.ascending ? "asc" : "desc")" }.joined(separator: " | "), privacy: .public) \ fieldInfoCount=\(fields.count) \ fields=\(fields.map { "\($0.key):\($0.value.type)\($0.value.hasKeywordSubfield ? "+kw" : "")" }.sorted().joined(separator: ","), privacy: .public) @@ -199,10 +199,10 @@ extension ElasticsearchPluginDriver { for: parsed, fields: fields, size: parsed.size, supportsCaseInsensitive: supportsCaseInsensitiveSearch ) body["from"] = parsed.from - Self.logger.debug("POST /\(index, privacy: .public)/_search body=\(Self.jsonString(body), privacy: .public)") + Self.logger.debug("POST /\(index, privacy: .private(mask: .hash))/_search body=\(Self.jsonString(body), privacy: .private)") let response = try await conn.search(index: index, body: body) let hits = extractHits(response) - Self.logger.debug("_search returned \(hits.count) hit(s) for index=\(index, privacy: .public)") + Self.logger.debug("_search returned \(hits.count) hit(s) for index=\(index, privacy: .private(mask: .hash))") return hits } return try await deepFetchHits(index: index, parsed: parsed, fields: fields, conn: conn) diff --git a/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver.swift b/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver.swift index 73d2da9834..f6daac652e 100644 --- a/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver.swift +++ b/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver.swift @@ -207,7 +207,7 @@ internal final class ElasticsearchPluginDriver: PluginDatabaseDriver, @unchecked Self.logger.debug(""" buildFilteredQuery table=\(table, privacy: .public) logic=\(logicMode, privacy: .public) limit=\(limit) offset=\(offset) \ columns=[\(columns.joined(separator: ","), privacy: .public)] \ - filters=\(filters.map { "\($0.column) \($0.op) '\($0.value)'" }.joined(separator: " | "), privacy: .public) \ + filters=\(filters.map { "\($0.column) \($0.op) '\($0.value)'" }.joined(separator: " | "), privacy: .private) \ sortColumns=\(sortColumns.map { "[\($0.columnIndex)]=\($0.ascending ? "asc" : "desc")" }.joined(separator: " "), privacy: .public) \ resolvedSorts=\(sorts.map { "\($0.column) \($0.ascending ? "asc" : "desc")" }.joined(separator: " | "), privacy: .public) """) diff --git a/Plugins/MSSQLDriverPlugin/MSSQLPluginDriver+Schema.swift b/Plugins/MSSQLDriverPlugin/MSSQLPluginDriver+Schema.swift index ba3b427ba9..a56a104f1d 100644 --- a/Plugins/MSSQLDriverPlugin/MSSQLPluginDriver+Schema.swift +++ b/Plugins/MSSQLDriverPlugin/MSSQLPluginDriver+Schema.swift @@ -5,6 +5,7 @@ import Foundation import os +import TableProLogRedaction import TableProMSSQLCore import TableProPluginKit @@ -433,7 +434,7 @@ extension MSSQLPluginDriver { metadata.append(try await fetchDatabaseMetadata(name)) } catch { Self.metadataLogger.debug( - "No metadata for database \(name, privacy: .public): \(error.localizedDescription, privacy: .public)" + "No metadata for database \(name, privacy: .private(mask: .hash)): \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)" ) unreadable.insert(name) metadata.append(PluginDatabaseMetadata(name: name)) @@ -458,7 +459,7 @@ extension MSSQLPluginDriver { } return sizes } catch { - Self.metadataLogger.debug("Server-wide database sizes unavailable: \(error.localizedDescription, privacy: .public)") + Self.metadataLogger.debug("Server-wide database sizes unavailable: \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)") return [:] } } diff --git a/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift b/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift index a8309676e6..8918247ec1 100644 --- a/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift +++ b/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift @@ -5,6 +5,7 @@ import Foundation import os +import TableProLogRedaction import TableProNumberFormatting import TableProPluginKit @@ -98,7 +99,7 @@ final class MongoDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable { let dbs = try await conn.listDatabases() currentDb = dbs.first { !Self.systemDatabases.contains($0) } ?? dbs.first ?? "" } catch { - Self.logger.warning("listDatabases failed during connect, continuing without default database: \(error.localizedDescription, privacy: .public)") + Self.logger.warning("listDatabases failed during connect, continuing without default database: \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)") } } diff --git a/Plugins/MongoDBDriverPlugin/MongoScriptRuntime.swift b/Plugins/MongoDBDriverPlugin/MongoScriptRuntime.swift index f11e2659e1..2d9fa2cf72 100644 --- a/Plugins/MongoDBDriverPlugin/MongoScriptRuntime.swift +++ b/Plugins/MongoDBDriverPlugin/MongoScriptRuntime.swift @@ -230,7 +230,7 @@ final class MongoScriptRuntime: @unchecked Sendable { context.setObject(execute, forKeyedSubscript: "__tp_exec" as NSString) context.setObject(emit, forKeyedSubscript: "__tp_print" as NSString) context.exceptionHandler = { _, exception in - Self.logger.debug("Script exception: \(exception?.toString() ?? "unknown", privacy: .public)") + Self.logger.debug("Script exception: \(exception?.toString() ?? "unknown", privacy: .private)") } context.evaluateScript(MongoScriptPrelude.source) diff --git a/Plugins/MySQLDriverPlugin/MariaDBCharacterSet.swift b/Plugins/MySQLDriverPlugin/MariaDBCharacterSet.swift index 81d266c104..94487085eb 100644 --- a/Plugins/MySQLDriverPlugin/MariaDBCharacterSet.swift +++ b/Plugins/MySQLDriverPlugin/MariaDBCharacterSet.swift @@ -10,9 +10,9 @@ nonisolated internal enum MariaDBCharacterSet { if mysql_set_character_set(mysql, MySQLConnectionEncoding.sessionCharacterSetName) != 0 { let refusal = errorSummary(of: mysql, encoding: encoding) if run(MySQLConnectionEncoding.sessionFallbackStatement, on: mysql) { - logger.notice("Server refused utf8mb4 (\(refusal, privacy: .public)), so the session uses utf8") + logger.notice("Server refused utf8mb4 (\(refusal, privacy: .private)), so the session uses utf8") } else { - logger.warning("Server refused a UTF-8 session (\(refusal, privacy: .public)); keeping its own") + logger.warning("Server refused a UTF-8 session (\(refusal, privacy: .private)); keeping its own") } } for statement in encoding.sessionStatements where !run(statement, on: mysql) { diff --git a/Plugins/MySQLDriverPlugin/MySQLPluginDriver+CreateDatabase.swift b/Plugins/MySQLDriverPlugin/MySQLPluginDriver+CreateDatabase.swift index d1d0bc42c1..3fda64cb3a 100644 --- a/Plugins/MySQLDriverPlugin/MySQLPluginDriver+CreateDatabase.swift +++ b/Plugins/MySQLDriverPlugin/MySQLPluginDriver+CreateDatabase.swift @@ -1,5 +1,6 @@ import Foundation import os +import TableProLogRedaction import TableProPluginKit extension MySQLPluginDriver { @@ -196,7 +197,7 @@ private extension MySQLPluginDriver { return value } catch { Self.logger.warning( - "Failed to read session variable \(variable.rawValue, privacy: .public): \(error.localizedDescription, privacy: .public)" + "Failed to read session variable \(variable.rawValue, privacy: .public): \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)" ) return nil } diff --git a/Plugins/OracleDriverPlugin/OraclePlugin.swift b/Plugins/OracleDriverPlugin/OraclePlugin.swift index 803371a436..9c77ddf68c 100644 --- a/Plugins/OracleDriverPlugin/OraclePlugin.swift +++ b/Plugins/OracleDriverPlugin/OraclePlugin.swift @@ -5,6 +5,7 @@ import Foundation import os +import TableProLogRedaction import TableProOracleCore import TableProPluginKit @@ -250,7 +251,7 @@ final class OraclePluginDriver: PluginDatabaseDriver, @unchecked Sendable { do { try await connection.captureServerOutput() } catch { - Self.logger.warning("DBMS_OUTPUT could not be enabled for this session: \(String(describing: error), privacy: .public)") + Self.logger.warning("DBMS_OUTPUT could not be enabled for this session: \(LogRedaction.publicDescription(of: error), privacy: .public) \(String(describing: error), privacy: .private)") } if let result = try? await connection.executeQuery(OracleSchemaQueries.currentSchema), diff --git a/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift b/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift index b9e9a695ec..958d569b1a 100644 --- a/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift +++ b/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift @@ -292,7 +292,7 @@ final class LibPQPluginConnection: @unchecked Sendable { guard PQresultStatus(result) != PGRES_COMMAND_OK else { return } let message = result.flatMap { PQresultErrorMessage($0) }.map { String(cString: $0) } ?? "" Self.logger.warning( - "Session setup statement failed: \(statement, privacy: .public) \(message, privacy: .public)" + "Session setup statement failed: \(statement, privacy: .public) \(message, privacy: .private)" ) } diff --git a/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+ServerSupport.swift b/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+ServerSupport.swift index 8f66c2fe66..3910755d61 100644 --- a/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+ServerSupport.swift +++ b/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+ServerSupport.swift @@ -5,6 +5,7 @@ import Foundation import os +import TableProLogRedaction import TableProPluginKit internal extension PostgreSQLPluginDriver { @@ -35,7 +36,7 @@ internal extension PostgreSQLPluginDriver { } catch { sessionFacts.withLock { $0 = .unknown } Self.sessionLogger.error( - "Session probe failed; DDL falls back to forms every server accepts: \(error.localizedDescription, privacy: .public)" + "Session probe failed; DDL falls back to forms every server accepts: \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)" ) } } diff --git a/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver.swift b/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver.swift index e571277e77..50571046ce 100644 --- a/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver.swift +++ b/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver.swift @@ -8,6 +8,7 @@ import Foundation import os +import TableProLogRedaction import TableProPluginKit class PostgreSQLPluginDriver: LibPQBackedDriver, @unchecked Sendable { @@ -839,7 +840,7 @@ class PostgreSQLPluginDriver: LibPQBackedDriver, @unchecked Sendable { ) } catch { Self.logger.error( - "Failed to read template1 defaults: \(error.localizedDescription, privacy: .public)" + "Failed to read template1 defaults: \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)" ) return nil } @@ -864,7 +865,7 @@ class PostgreSQLPluginDriver: LibPQBackedDriver, @unchecked Sendable { return (libc: libc, icu: icu) } catch { Self.logger.error( - "Failed to read pg_collation: \(error.localizedDescription, privacy: .public)" + "Failed to read pg_collation: \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)" ) return (libc: [], icu: []) } diff --git a/Plugins/PostgreSQLDriverPlugin/RedshiftPluginDriver.swift b/Plugins/PostgreSQLDriverPlugin/RedshiftPluginDriver.swift index 46c474d788..9222f54230 100644 --- a/Plugins/PostgreSQLDriverPlugin/RedshiftPluginDriver.swift +++ b/Plugins/PostgreSQLDriverPlugin/RedshiftPluginDriver.swift @@ -8,6 +8,7 @@ import Foundation import os +import TableProLogRedaction import TableProPluginKit final class RedshiftPluginDriver: LibPQBackedDriver, @unchecked Sendable { @@ -58,7 +59,7 @@ final class RedshiftPluginDriver: LibPQBackedDriver, @unchecked Sendable { externalSchemaCache = Set(result.rows.compactMap { $0.first?.asText }) } catch { Self.logger.warning( - "Could not read svv_external_schemas; external schemas stay unresolved: \(error.localizedDescription, privacy: .public)" + "Could not read svv_external_schemas; external schemas stay unresolved: \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)" ) } } @@ -117,7 +118,7 @@ final class RedshiftPluginDriver: LibPQBackedDriver, @unchecked Sendable { } } catch { Self.logger.warning( - "svv_external_tables failed for schema \(schema, privacy: .public); listing local tables only: \(error.localizedDescription, privacy: .public)" + "svv_external_tables failed for schema \(schema, privacy: .private(mask: .hash)); listing local tables only: \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)" ) return [] } @@ -147,7 +148,7 @@ final class RedshiftPluginDriver: LibPQBackedDriver, @unchecked Sendable { } } catch { Self.logger.warning( - "svv_external_columns failed for schema \(schema, privacy: .public): \(error.localizedDescription, privacy: .public)" + "svv_external_columns failed for schema \(schema, privacy: .private(mask: .hash)): \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)" ) return [] } @@ -250,7 +251,7 @@ final class RedshiftPluginDriver: LibPQBackedDriver, @unchecked Sendable { return allColumns } catch { Self.logger.warning( - "svv_external_columns failed for schema \(schema, privacy: .public): \(error.localizedDescription, privacy: .public)" + "svv_external_columns failed for schema \(schema, privacy: .private(mask: .hash)): \(LogRedaction.publicDescription(of: error), privacy: .public) \(error.localizedDescription, privacy: .private)" ) return [:] } diff --git a/Plugins/RedisDriverPlugin/HiredisSentinelTransport.swift b/Plugins/RedisDriverPlugin/HiredisSentinelTransport.swift index fc539ed411..e2ceda3774 100644 --- a/Plugins/RedisDriverPlugin/HiredisSentinelTransport.swift +++ b/Plugins/RedisDriverPlugin/HiredisSentinelTransport.swift @@ -60,14 +60,14 @@ struct HiredisSentinelTransport: RedisSentinelTransport { do { try await connection.connect() } catch let error as RedisPluginError where error.refusedByServer { - logger.debug("Sentinel \(sentinel.identifier, privacy: .public) refused: \(error.message, privacy: .public)") + logger.debug("Sentinel \(sentinel.identifier, privacy: .public) refused: \(RedisConnectProbe.errorClass(of: error.message), privacy: .public) \(error.message, privacy: .private)") throw RedisSentinelError.refused(sentinel, detail: error.message) } defer { connection.disconnect() } let reply = try await connection.executeCommand(command) if let message = reply.errorMessage { - logger.debug("Sentinel \(sentinel.identifier, privacy: .public) refused: \(message, privacy: .public)") + logger.debug("Sentinel \(sentinel.identifier, privacy: .public) refused: \(RedisConnectProbe.errorClass(of: message), privacy: .public) \(message, privacy: .private)") throw RedisSentinelError.refused(sentinel, detail: message) } return reply diff --git a/Plugins/SnowflakeDriverPlugin/SnowflakeConnection.swift b/Plugins/SnowflakeDriverPlugin/SnowflakeConnection.swift index 0f6b3f20d5..dd979a17de 100644 --- a/Plugins/SnowflakeDriverPlugin/SnowflakeConnection.swift +++ b/Plugins/SnowflakeDriverPlugin/SnowflakeConnection.swift @@ -830,7 +830,7 @@ final class SnowflakeConnection: @unchecked Sendable { guard (200..<300).contains(http.statusCode) else { let bodyText = String(data: data, encoding: .utf8) ?? "" Self.logger.error( - "HTTP \(http.statusCode, privacy: .public) from \(request.url?.path ?? "?", privacy: .public): \(String(bodyText.prefix(160)), privacy: .public)" + "HTTP \(http.statusCode, privacy: .public) from \(request.url?.path ?? "?", privacy: .public): \(String(bodyText.prefix(160)), privacy: .private)" ) throw SnowflakeError.invalidResponse("Snowflake returned HTTP \(http.statusCode) for \(request.url?.path ?? "request"): \(bodyText.prefix(300))") } diff --git a/TablePro/Core/Diagnostics/LogRedaction.swift b/TablePro/Core/Diagnostics/LogRedaction.swift index 6d3e7c6922..eff4b4e84c 100644 --- a/TablePro/Core/Diagnostics/LogRedaction.swift +++ b/TablePro/Core/Diagnostics/LogRedaction.swift @@ -4,52 +4,8 @@ // import Foundation - -/// An error type whose whole description is the app's own vocabulary, with nothing a server or a -/// person supplied inside it. Conforming says the description is safe to publish in the system log. -/// -/// Conform only when every case's text is a literal the app wrote. A case that interpolates a -/// server message, a table name, a path or a value is not publicly loggable, however app-owned the -/// type is. -internal protocol PubliclyLoggableError: Error { - var publicLogDescription: String { get } -} - -/// `Logger` redacts an interpolated value by default, and Apple's own reason is that it "prevents -/// the system from leaking potentially user-sensitive information in the log files". An error's -/// text is at once the most useful thing in a log line and the likeliest to carry someone's data: -/// PostgreSQL puts the offending value in a unique-violation message (`Key (email)=(a@b.com)`), -/// MySQL puts row data in its duplicate-key message, and a file error carries the person's paths. -/// -/// So an error contributes two things to a line: a shape that is safe to publish, and a description -/// that is not. This publishes the first; the call site logs the second at `.private`, where a -/// developer with a logging profile can still read it and a shared sysdiagnose cannot. -internal enum LogRedaction { - internal static func publicDescription(of error: Error) -> String { - if let loggable = error as? PubliclyLoggableError { - return loggable.publicLogDescription - } - - let typeName = String(describing: type(of: error)) - let mirror = Mirror(reflecting: error) - - guard mirror.displayStyle == .enum else { - let nsError = error as NSError - guard !nsError.domain.hasSuffix(typeName) else { return "\(typeName)(\(nsError.code))" } - return "\(typeName)(\(nsError.domain), \(nsError.code))" - } - - /// An enum case with a payload reports the case name as its single child's label, and the - /// payload is what would carry the text. A case without one prints as its own name. - if let caseName = mirror.children.first?.label { - return "\(typeName).\(caseName)" - } - return "\(typeName).\(error)" - } -} +import TableProLogRedaction internal extension Error { - /// The part of this error that is safe to publish in the system log. The description itself is - /// not: log it at `.private` beside this where a support workflow needs it. var publicLogShape: String { LogRedaction.publicDescription(of: self) } } diff --git a/TablePro/Core/MCP/Transport/MCPBridgeLogger.swift b/TablePro/Core/MCP/Transport/MCPBridgeLogger.swift index ad9ee8e83c..7238da22ab 100644 --- a/TablePro/Core/MCP/Transport/MCPBridgeLogger.swift +++ b/TablePro/Core/MCP/Transport/MCPBridgeLogger.swift @@ -22,13 +22,13 @@ public struct MCPOSBridgeLogger: MCPBridgeLogger { public func log(_ level: MCPBridgeLogLevel, _ message: String) { switch level { case .debug: - logger.debug("\(message, privacy: .public)") + logger.debug("\(message, privacy: .private)") case .info: - logger.info("\(message, privacy: .public)") + logger.info("\(message, privacy: .private)") case .warning: - logger.warning("\(message, privacy: .public)") + logger.warning("\(message, privacy: .private)") case .error: - logger.error("\(message, privacy: .public)") + logger.error("\(message, privacy: .private)") } } } diff --git a/TablePro/Core/Plugins/PluginBundleLoader.swift b/TablePro/Core/Plugins/PluginBundleLoader.swift index c83149f940..85ab24b9b3 100644 --- a/TablePro/Core/Plugins/PluginBundleLoader.swift +++ b/TablePro/Core/Plugins/PluginBundleLoader.swift @@ -17,7 +17,7 @@ enum PluginBundleLoader { let reason = describeLoadFailure(nsError) let detail = nsError.userInfo[NSDebugDescriptionErrorKey] as? String ?? nsError.localizedDescription logger.error( - "Bundle load failed for \(bundle.bundleURL.lastPathComponent, privacy: .public) [\(nsError.domain, privacy: .public) \(nsError.code, privacy: .public)]: \(reason, privacy: .public) [\(detail, privacy: .public)]" + "Bundle load failed for \(bundle.bundleURL.lastPathComponent, privacy: .public) [\(nsError.domain, privacy: .public) \(nsError.code, privacy: .public)]: \(reason, privacy: .public) [\(detail, privacy: .private)]" ) throw PluginError.invalidBundle(reason) } diff --git a/TablePro/Core/Scripting/ScriptCommand.swift b/TablePro/Core/Scripting/ScriptCommand.swift index b0c9869065..a098a7be02 100644 --- a/TablePro/Core/Scripting/ScriptCommand.swift +++ b/TablePro/Core/Scripting/ScriptCommand.swift @@ -70,7 +70,7 @@ internal class ScriptCommand: NSScriptCommand { } catch { let scripting = ScriptingError.from(error) Self.logger.error( - "\(type(of: command), privacy: .public) failed: \(scripting.errorDescription ?? "", privacy: .public)" + "\(type(of: command), privacy: .public) failed: \(error.publicLogShape, privacy: .public) \(scripting.errorDescription ?? "", privacy: .private)" ) command.scriptErrorNumber = scripting.number command.scriptErrorString = scripting.errorDescription diff --git a/TablePro/Core/Services/Infrastructure/LaunchIntentRouter.swift b/TablePro/Core/Services/Infrastructure/LaunchIntentRouter.swift index 77df2d6259..653b84f23c 100644 --- a/TablePro/Core/Services/Infrastructure/LaunchIntentRouter.swift +++ b/TablePro/Core/Services/Infrastructure/LaunchIntentRouter.swift @@ -66,7 +66,7 @@ internal final class LaunchIntentRouter { Self.logger.debug("LaunchIntentRouter.openInspectorDocument - calling NSDocumentController.shared (\(String(describing: Swift.type(of: NSDocumentController.shared)), privacy: .public)).openDocument for \(url.lastPathComponent, privacy: .private(mask: .hash))") try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in NSDocumentController.shared.openDocument(withContentsOf: url, display: true) { document, alreadyOpen, error in - Self.logger.debug("LaunchIntentRouter.openInspectorDocument completion - document=\(document == nil ? "nil" : "present", privacy: .public) alreadyOpen=\(alreadyOpen, privacy: .public) error=\(error?.localizedDescription ?? "nil", privacy: .public)") + Self.logger.debug("LaunchIntentRouter.openInspectorDocument completion - document=\(document == nil ? "nil" : "present", privacy: .public) alreadyOpen=\(alreadyOpen, privacy: .public) error=\(error?.publicLogShape ?? "nil", privacy: .public) \(error?.localizedDescription ?? "nil", privacy: .private)") if let error { continuation.resume(throwing: error) return diff --git a/TablePro/Core/Services/Infrastructure/PrivilegedShell.swift b/TablePro/Core/Services/Infrastructure/PrivilegedShell.swift index d2ecb9f36c..fcd82fb113 100644 --- a/TablePro/Core/Services/Infrastructure/PrivilegedShell.swift +++ b/TablePro/Core/Services/Infrastructure/PrivilegedShell.swift @@ -64,7 +64,7 @@ internal struct OSAScriptPrivilegedShell: PrivilegedShellRunning { if message.contains("-128") || message.localizedCaseInsensitiveContains("cancel") { throw PrivilegedShellError.cancelled } - Self.logger.error("Privileged command failed: \(message, privacy: .public)") + Self.logger.error("Privileged command failed: \(message, privacy: .private)") throw PrivilegedShellError.failed(message) } } diff --git a/TablePro/Core/Services/Query/ServerOutputCapture.swift b/TablePro/Core/Services/Query/ServerOutputCapture.swift index 1ca3091843..56a63258a2 100644 --- a/TablePro/Core/Services/Query/ServerOutputCapture.swift +++ b/TablePro/Core/Services/Query/ServerOutputCapture.swift @@ -70,7 +70,7 @@ enum ServerOutputCapture { do { return try await driver.fetchServerOutput() } catch { - logger.warning("Server output could not be read: \(String(describing: error), privacy: .public)") + logger.warning("Server output could not be read: \(error.publicLogShape, privacy: .public) \(String(describing: error), privacy: .private)") return .none } } diff --git a/TablePro/Core/Sync/SyncCoordinator.swift b/TablePro/Core/Sync/SyncCoordinator.swift index b4fe8b31bd..42d71abc0f 100644 --- a/TablePro/Core/Sync/SyncCoordinator.swift +++ b/TablePro/Core/Sync/SyncCoordinator.swift @@ -920,9 +920,8 @@ final class SyncCoordinator: ObservableObject { try applySettingsData(data, for: category) } catch { let recordName = record.recordID.recordName - let message = error.localizedDescription Self.logger.error( - "Skipping remote settings \(recordName, privacy: .public) (\(category, privacy: .public)): \(message, privacy: .public)" + "Skipping remote settings \(recordName, privacy: .private(mask: .hash)) (\(category, privacy: .private(mask: .hash))): \(error.publicLogShape, privacy: .public) \(error.localizedDescription, privacy: .private)" ) } } @@ -934,9 +933,8 @@ final class SyncCoordinator: ObservableObject { entry = try SyncRecordMapper.favoriteEntry(from: record) } catch { let recordName = record.recordID.recordName - let message = error.localizedDescription Self.logger.error( - "Skipping remote favorite table \(recordName, privacy: .public): \(message, privacy: .public)" + "Skipping remote favorite table \(recordName, privacy: .private(mask: .hash)): \(error.publicLogShape, privacy: .public) \(error.localizedDescription, privacy: .private)" ) return false } @@ -952,9 +950,8 @@ final class SyncCoordinator: ObservableObject { entry = try SyncRecordMapper.favoriteDatabase(from: record) } catch { let recordName = record.recordID.recordName - let message = error.localizedDescription Self.logger.error( - "Skipping remote favorite database \(recordName, privacy: .public): \(message, privacy: .public)" + "Skipping remote favorite database \(recordName, privacy: .private(mask: .hash)): \(error.publicLogShape, privacy: .public) \(error.localizedDescription, privacy: .private)" ) return } diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+SchemaLoading.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+SchemaLoading.swift index 6dbd6b8194..11a65c9c7d 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+SchemaLoading.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+SchemaLoading.swift @@ -88,7 +88,7 @@ extension MainContentCoordinator { armPostConnectSchemaLoad() case .surface(let message): Self.logger.error( - "[schema] initial load failed connId=\(self.connectionId, privacy: .public) error=\(message, privacy: .public)" + "[schema] initial load failed connId=\(self.connectionId, privacy: .public) error=\(error.publicLogShape, privacy: .public) \(message, privacy: .private)" ) services.schemaService.markLoadFailed(connectionId: connectionId, message: message, scope: scope) } diff --git a/TableProTests/Core/Diagnostics/LogRedactionTests.swift b/TableProTests/Core/Diagnostics/LogRedactionTests.swift index d91ab59c31..5fe50f2a41 100644 --- a/TableProTests/Core/Diagnostics/LogRedactionTests.swift +++ b/TableProTests/Core/Diagnostics/LogRedactionTests.swift @@ -7,8 +7,6 @@ import Foundation @testable import TablePro import Testing -/// The text these tests feed in is the shape of a real PostgreSQL unique-violation message, which -/// carries the offending value. Nothing derived from an error may carry it into the system log. @Suite("Log redaction") struct LogRedactionTests { private static let serverText = @@ -16,116 +14,19 @@ struct LogRedactionTests { private enum DriverError: LocalizedError { case executionFailed(String) - case disconnected var errorDescription: String? { switch self { case .executionFailed(let message): return message - case .disconnected: return "The connection closed." } } } - private struct BoxedError: Error { - let serverText: String - } - - private enum AppError: PubliclyLoggableError { - case readOnlyConnection - - var publicLogDescription: String { "AppError.readOnlyConnection" } - } - - @Test("An enum case's payload never reaches the public description") - func enumPayloadIsDropped() { - let shape = DriverError.executionFailed(Self.serverText).publicLogShape - - #expect(shape == "DriverError.executionFailed") - #expect(!shape.contains("a@b.com")) - } - - @Test("A case with no payload keeps its name, which is app vocabulary") - func payloadlessCaseKeepsItsName() { - #expect(DriverError.disconnected.publicLogShape == "DriverError.disconnected") - } - - @Test("A struct error publishes its type and bridged code, not its stored text") - func structErrorPublishesItsShape() { - let shape = BoxedError(serverText: Self.serverText).publicLogShape - - #expect(!shape.contains("a@b.com")) - #expect(shape.hasPrefix("BoxedError(")) - } - - @Test("An error that declares its description safe is published in full") - func publiclyLoggableErrorIsPublishedInFull() { - #expect(AppError.readOnlyConnection.publicLogShape == "AppError.readOnlyConnection") - } - - @Test("A Foundation error keeps the domain and code a reader can act on") - func foundationErrorKeepsDomainAndCode() { - let shape = (NSError(domain: NSPOSIXErrorDomain, code: 61) as Error).publicLogShape - - #expect(shape.contains(NSPOSIXErrorDomain)) - #expect(shape.contains("61")) - } - - @Test("A localizedDescription that embeds the server text is not what gets published") - func localizedDescriptionIsNotThePublishedValue() { + @Test("The app's public shape of an error never carries the text its description does") + func publicLogShapeDropsTheDescription() { let error = DriverError.executionFailed(Self.serverText) #expect(error.localizedDescription.contains("a@b.com")) - #expect(!error.publicLogShape.contains("a@b.com")) - } - - /// Scoped to the app. A plugin cannot reach `publicLogShape`, which is internal to the app - /// target, so the 16 sites under `Plugins/` need the helper in `TableProPluginKit` first, and - /// that is an ABI event with its own version bump. - @Test("No log call in the app publishes an error's description") - func noCallSitePublishesErrorText() throws { - let root = try repositoryRoot() - let offenders = try publicErrorSites(under: root.appendingPathComponent("TablePro"), root: root) - - #expect( - offenders.isEmpty, - """ - These sites publish an error's text to the system log, where any local process and any \ - sysdiagnose can read it: \(offenders.sorted()) - """ - ) - } - - private func publicErrorSites(under directory: URL, root: URL) throws -> [String] { - let enumerator = FileManager.default.enumerator(at: directory, includingPropertiesForKeys: nil) - var offenders: [String] = [] - - while let url = enumerator?.nextObject() as? URL { - guard url.pathExtension == "swift" else { continue } - guard !url.path.contains("/.build/"), !url.path.contains("/checkouts/") else { continue } - - let lines = try String(contentsOf: url, encoding: .utf8).components(separatedBy: .newlines) - for (index, line) in lines.enumerated() - where line.contains(".localizedDescription, privacy: .public") { - let relative = url.path.replacingOccurrences(of: root.path + "/", with: "") - offenders.append("\(relative):\(index + 1)") - } - } - - return offenders - } - - private func repositoryRoot(file: StaticString = #filePath) throws -> URL { - var directory = URL(fileURLWithPath: "\(file)").deletingLastPathComponent() - while directory.path != "/" { - if FileManager.default.fileExists(atPath: directory.appendingPathComponent("project.yml").path) { - return directory - } - directory = directory.deletingLastPathComponent() - } - throw RedactionTestError.repositoryRootNotFound - } - - private enum RedactionTestError: Error { - case repositoryRootNotFound + #expect(error.publicLogShape == "DriverError.executionFailed") } } diff --git a/project.yml b/project.yml index 357e2efec7..4ac82cd592 100644 --- a/project.yml +++ b/project.yml @@ -238,7 +238,7 @@ targets: - package: Sparkle - package: Yams - package: TableProCore - products: [TableProAnalytics, TableProConnectionLibrary, TableProGeometry, TableProGoogleCloud, TableProImport, TableProNumberFormatting, TableProSQLGrammar, TableProSSHTransport, TableProSyncTransport] + products: [TableProAnalytics, TableProConnectionLibrary, TableProGeometry, TableProGoogleCloud, TableProImport, TableProLogRedaction, TableProNumberFormatting, TableProSQLGrammar, TableProSSHTransport, TableProSyncTransport] settings: base: ASSETCATALOG_COMPILER_APPICON_NAME: AppIcon @@ -350,6 +350,9 @@ targets: - TablePro/Core/MCP/Transport/MCPUpstreamCredentials.swift - TablePro/Core/MCP/Wire - TablePro/Core/Services/Infrastructure/BackgroundLaunchFlag.swift + dependencies: + - package: TableProCore + product: TableProLogRedaction scheme: {} settings: base: @@ -713,7 +716,7 @@ targets: dependencies: - target: TablePro - package: TableProCore - products: [TableProConnectionLibrary, TableProDocumentPath, TableProGeometry, TableProMSSQLCore, TableProNumberFormatting, TableProSQLGrammar, TableProSSHTransport, TableProWeaviateCore] + products: [TableProConnectionLibrary, TableProDocumentPath, TableProGeometry, TableProLogRedaction, TableProMSSQLCore, TableProNumberFormatting, TableProSQLGrammar, TableProSSHTransport, TableProWeaviateCore] # The Kafka integration suite drives the real driver, so the test target links what # the plugin target links: zstd for decompression and NIO for the transport. - package: zstd @@ -769,6 +772,8 @@ targets: dependencies: - package: TableProCore product: TableProGeometry + - package: TableProCore + product: TableProLogRedaction settings: base: HEADER_SEARCH_PATHS: @@ -797,6 +802,9 @@ targets: templateAttributes: folder: PostgreSQLDriverPlugin principalClass: PostgreSQLPlugin + dependencies: + - package: TableProCore + product: TableProLogRedaction settings: base: HEADER_SEARCH_PATHS: @@ -1024,6 +1032,9 @@ targets: templateAttributes: folder: CSVInspectorPlugin principalClass: CSVInspectorPlugin + dependencies: + - package: TableProCore + product: TableProLogRedaction # ─── Plugins published to the registry ──────────────────────────────────────── @@ -1049,6 +1060,8 @@ targets: dependencies: - package: TableProCore product: TableProGoogleCloud + - package: TableProCore + product: TableProLogRedaction SpannerDriverPlugin: templates: [DriverPlugin] @@ -1247,7 +1260,7 @@ targets: principalClass: MSSQLPlugin dependencies: - package: TableProCore - products: [TableProCoreTypes, TableProMSSQLCore] + products: [TableProCoreTypes, TableProLogRedaction, TableProMSSQLCore] settings: base: HEADER_SEARCH_PATHS: @@ -1279,6 +1292,8 @@ targets: folder: MongoDBDriverPlugin principalClass: MongoDBPlugin dependencies: + - package: TableProCore + product: TableProLogRedaction - package: TableProCore product: TableProNumberFormatting settings: @@ -1314,6 +1329,8 @@ targets: folder: OracleDriverPlugin principalClass: OraclePlugin dependencies: + - package: TableProCore + product: TableProLogRedaction - package: TableProOracle product: TableProOracleCore diff --git a/scripts/ci/check-log-privacy.py b/scripts/ci/check-log-privacy.py new file mode 100644 index 0000000000..ebd2fe3f21 --- /dev/null +++ b/scripts/ci/check-log-privacy.py @@ -0,0 +1,96 @@ +#!/usr/bin/env python3 +"""Check no source publishes an error's text to the system log. + +`.swiftlint.yml` forbids it through the `public_error_text_in_log` custom rule, because a driver +error's text carries row values, paths and server messages: PostgreSQL puts the offending value in +a unique-violation message, and `.public` hands it to any process that reads the log and to every +sysdiagnose. Two things kept the rule from holding. SwiftLint's `included:` names `TablePro` and +`Packages` only, so it never read `Plugins/`, where fifteen log lines broke it. And SwiftLint runs +in CI only on a release tag, so nothing checked the rule on a pull request anywhere. + +Bringing all of `Plugins/` under SwiftLint is the fuller answer and a larger one: measured on +2026-09-23 with SwiftLint 0.65.1, `swiftlint lint --strict` over the 536 plugin sources outside +TableProPluginKit, passed as file paths, reports 175 violations across 23 plugins, and only 15 of +them were this rule. + +So this applies the one rule to every Swift file under `TablePro/`, `Packages/` and `Plugins/`, +across line breaks the way SwiftLint matches it. It reads the regex out of `.swiftlint.yml` rather +than repeating it, so the lint rule and this check cannot drift apart. It runs on Ubuntu in about a +second with no Xcode, and `test_check_log_privacy.py` pins what the regex must and must not match. + +A regex sees one interpolation, not where its text came from: an error's description bound to a +variable first and published under another name passes. Those sites are found by reading. + +`TableProMobile/` is not scanned yet: it still holds log lines this rule rejects. +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path + +RULE = "public_error_text_in_log" +SCANNED = ("TablePro", "Packages", "Plugins") +SKIPPED_DIRECTORIES = {".build", "checkouts", "DerivedData"} + + +def rule_pattern(config: Path) -> re.Pattern[str]: + lines = config.read_text(encoding="utf-8").splitlines() + try: + start = next(i for i, line in enumerate(lines) if line.strip() == f"{RULE}:") + except StopIteration: + raise SystemExit(f"error: {config.name} has no `{RULE}` custom rule") from None + + indent = len(lines[start]) - len(lines[start].lstrip()) + for line in lines[start + 1:]: + if line.strip() and len(line) - len(line.lstrip()) <= indent: + break + match = re.match(r"\s*regex:\s*'(?P(?:[^']|'')*)'\s*$", line) + if match: + return re.compile(match.group("body").replace("''", "'")) + raise SystemExit(f"error: the `{RULE}` rule in {config.name} has no single-quoted regex") + + +def sources(root: Path) -> list[Path]: + seen: set[Path] = set() + found = [] + for directory in SCANNED: + for path in sorted((root / directory).rglob("*.swift")): + if SKIPPED_DIRECTORIES.intersection(path.relative_to(root).parts): + continue + resolved = path.resolve() + if resolved in seen: + continue + seen.add(resolved) + found.append(path) + return found + + +def offenders(root: Path, pattern: re.Pattern[str]) -> list[str]: + found = [] + for path in sources(root): + text = path.read_text(encoding="utf-8", errors="replace") + for match in pattern.finditer(text): + number = text.count("\n", 0, match.start()) + 1 + found.append(f"{path.relative_to(root)}:{number}") + return found + + +def main() -> int: + root = Path(sys.argv[1]).resolve() if len(sys.argv) > 1 else Path(__file__).resolve().parents[2] + pattern = rule_pattern(root / ".swiftlint.yml") + found = offenders(root, pattern) + if not found: + return 0 + + print("These log lines publish an error's text, which can carry row values, paths and server") + print("messages. Publish `error.publicLogShape` in the app, or") + print("`LogRedaction.publicDescription(of: error)` elsewhere, and log the description at .private:") + for site in found: + print(f" {site}") + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/ci/test_check_log_privacy.py b/scripts/ci/test_check_log_privacy.py new file mode 100644 index 0000000000..6b55321c82 --- /dev/null +++ b/scripts/ci/test_check_log_privacy.py @@ -0,0 +1,80 @@ +#!/usr/bin/env python3 +"""Fixture tests for check-log-privacy.py, run against the real `.swiftlint.yml` regex.""" + +from __future__ import annotations + +import importlib.util +import shutil +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +SPEC = importlib.util.spec_from_file_location("check_log_privacy", Path(__file__).with_name("check-log-privacy.py")) +check = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(check) + +REJECTED = [ + 'logger.error("a \\(error.localizedDescription, privacy: .public)")', + 'logger.error("b \\(String(describing: error), privacy: .public)")', + 'logger.error("c \\(error, privacy: .public)")', + 'logger.error("d \\(String(describing: sqlError), privacy: .public)")', + 'logger.error("e \\(e, privacy: .public)")', + 'logger.error("f \\(error.message, privacy: .public)")', + 'logger.error("g \\(nioSslError.description, privacy: .public)")', + 'logger.error("h \\(error,\n privacy: .public)")', + 'logger.error("i \\(error?.localizedDescription ?? "nil", privacy: .public)")', + 'logger.error("j \\(error.localizedDescription.prefix(200), privacy: .public)")', + 'logger.error("k \\(scripting.errorDescription ?? "", privacy: .public)")', +] + +ACCEPTED = [ + 'logger.error("a \\(error.localizedDescription, privacy: .private)")', + 'logger.error("b \\(LogRedaction.publicDescription(of: error), privacy: .public)")', + 'logger.error("c \\(error.publicLogShape, privacy: .public)")', + 'logger.error("d \\(errorClass, privacy: .public)")', + 'logger.error("e \\(errors.count, privacy: .public)")', + 'logger.error("f \\(error.code, privacy: .public)")', + 'logger.error("g \\(String(describing: type(of: error)), privacy: .public)")', + 'logger.error("h \\(hasError, privacy: .public)")', + 'logger.error("i \\(isError, privacy: .public)")', +] + + +class CheckLogPrivacyTests(unittest.TestCase): + def setUp(self) -> None: + self.root = Path(tempfile.mkdtemp()) + shutil.copy(ROOT / ".swiftlint.yml", self.root / ".swiftlint.yml") + self.pattern = check.rule_pattern(self.root / ".swiftlint.yml") + + def tearDown(self) -> None: + shutil.rmtree(self.root) + + def write(self, directory: str, name: str, lines: list[str]) -> None: + folder = self.root / directory + folder.mkdir(parents=True, exist_ok=True) + (folder / name).write_text("\n".join(lines) + "\n", encoding="utf-8") + + def test_every_rejected_spelling_is_reported_on_its_own_line(self) -> None: + self.write("Plugins/Driver", "Rejected.swift", REJECTED) + found = check.offenders(self.root, self.pattern) + self.assertEqual(found, [f"Plugins/Driver/Rejected.swift:{number}" for number in [*range(1, 9), 10, 11, 12]]) + + def test_safe_spellings_are_not_reported(self) -> None: + self.write("Plugins/Driver", "Accepted.swift", ACCEPTED) + self.assertEqual(check.offenders(self.root, self.pattern), []) + + def test_app_and_packages_are_scanned_and_build_output_is_not(self) -> None: + self.write("TablePro/Core", "App.swift", REJECTED[:1]) + self.write("Packages/Core/Sources", "Package.swift", REJECTED[:1]) + self.write("Packages/Core/.build/checkouts/dep", "Vendored.swift", REJECTED[:1]) + found = check.offenders(self.root, self.pattern) + self.assertEqual(found, ["TablePro/Core/App.swift:1", "Packages/Core/Sources/Package.swift:1"]) + + def test_the_repository_itself_is_clean(self) -> None: + self.assertEqual(check.offenders(ROOT, check.rule_pattern(ROOT / ".swiftlint.yml")), []) + + +if __name__ == "__main__": + sys.exit(unittest.main())