Skip to content

fix(plugins): stop driver error text reaching the system log - #3081

Merged
datlechin merged 1 commit into
mainfrom
fix/plugin-public-error-text-in-log
Sep 23, 2026
Merged

datlechin merged 1 commit into
mainfrom
fix/plugin-public-error-text-in-log

Conversation

@datlechin

Copy link
Copy Markdown
Member

Defect

#2960 stopped the app publishing error text to the system log and added the public_error_text_in_log SwiftLint rule to keep it that way. SwiftLint's included: names TablePro and Packages only, so the rule never read Plugins/, and 15 plugin log lines kept writing error.localizedDescription at privacy: .public. The test #2960 added scanned TablePro/ alone, and said why: a plugin could not reach publicLogShape, which is internal to the app.

A driver error's text is where a server puts the data it is complaining about. PostgreSQL's unique-violation message carries the offending value (Key (email)=(a@b.com)), MySQL's duplicate-key message carries the row, and a file error carries the person's paths. At .public that text reaches every process that can read the log and every sysdiagnose a user attaches to a bug report. The 15 sites were in BigQuery, CSV Inspector, SQL Server, MongoDB, MySQL, PostgreSQL and Redshift, on paths such as the PostgreSQL create-database sheet reading template1 defaults and pg_collation, the Redshift external schema reads, and the MongoDB connect.

The same leak was spelled other ways the rule does not match, and those are fixed here too:

  • String(describing: error) at .public in the Oracle plugin, the app's ServerOutputCapture, and four places in TableProOracleCore (connect failure, TLS failure, statement failure, transport reset), one of which also published a column name.
  • A Redis Sentinel refusal published the server's whole error reply. It now publishes the error class, the way RedisClusterChannel already did.
  • mysql_error() text when a server refuses a UTF-8 session, a PostgreSQL session setup failure's PQresultErrorMessage, a MongoDB script exception, a Snowflake HTTP error body, and up to 300 characters of a raw Elasticsearch response.
  • Elasticsearch search request bodies and filter values, index names and mapping keys.
  • In the app: a failed initial schema load and three sync apply paths published error.localizedDescription through a message variable (the sync lines also published record names that embed table names and SQLite paths), a failed AppleScript command published the driver error through errorDescription ?? "", a document open failure published error?.localizedDescription ?? "nil", a failed privileged command published its stderr, a plugin bundle load failure published the NSError debug description, and the MCP bridge logger published every message, error text included.

Fix

  • LogRedaction and PubliclyLoggableError move out of the app into a new dependency-free SwiftPM target, TableProLogRedaction, beside TableProNumberFormatting and TableProDocumentPath, so the app and the plugins share one implementation. A plugin publishes LogRedaction.publicDescription(of: error). The app keeps Error.publicLogShape as a one-line forwarder, so the 113 app files that use it need no new import. The target is linked into the app, the mcp-server helper (which compiles LogRedaction.swift too), the test bundle, and the seven plugins that call it.
  • Why not TableProPluginKit: every public symbol there is permanent resilient ABI, for a helper that is not part of the plugin contract. Why not a public Error.publicLogShape in the new module: measured with swiftc, an app-internal extension member shadows a same-named public one inside the app, but a third module importing both (the test target) fails with ambiguous use of 'publicLogShape'.
  • Every site that now publishes the shape also logs the description at .private, so a developer with a logging profile keeps the detail.
  • The redaction fails closed in two places it did not before. An NSError domain is published only when it is identifier-shaped (ASCII letters, digits, ., _, -, at most 64 bytes), as every declared domain is; the old code published any domain that did not end in the type name, so NSError(domain: serverText, code: 1) put the server text in the public line. And a payload-free enum case whose type defines description or debugDescription publishes the type name, because "\(error)" would print that text.
  • Schema, database, index and record names on the lines touched go to .private(mask: .hash), the way fix(diagnostics): stop publishing user data to the system log #2960 treated them in the app.
  • The SwiftLint rule now also matches String(describing: <any error name>), a bare error interpolation, .message, .description, .debugDescription, errorDescription and failureReason, and a localizedDescription with .prefix(n) or a ?? "default", at .public. hasError and isError are not matched.

Closing the gap

Measured on main with SwiftLint 0.65.1: swiftlint lint --strict over the 536 Swift files under Plugins/ outside TableProPluginKit, passed as file paths, reports 175 violations across 23 plugins, 15 of them this rule. The rest are 53 identifier_name, 51 number_separator, 13 inclusive_language (Redis protocol terms) and 43 others. That is too much unrelated churn for this PR, and swiftlint lint --strict gates the release job, so adding Plugins to included: has to land with every one of them fixed. SwiftLint also runs in CI only on a release tag, so the rule was not checked on any pull request, for the app either.

So this adds scripts/ci/check-log-privacy.py to the Repo Hygiene job, which runs on every pull request that touches Swift. It reads the rule's regex out of .swiftlint.yml, so the rule and the check cannot drift, and applies it to every Swift file under TablePro/, Packages/ and Plugins/, across line breaks the way SwiftLint matches. On main it reports 19 sites; on this branch none. scripts/ci/test_check_log_privacy.py pins 11 spellings the rule must reject and 9 it must accept, and runs in the same job. The job now also runs when .swiftlint.yml changes. The app suite's old literal-substring scan is removed, since it had drifted from the rule and the check replaces it.

A regex cannot follow text through a variable, which is why the message, refusal, detail, bodyText and bridge-logger sites above were found by reading, not by the check.

Not in this PR

  • TableProMobile/ is outside SwiftLint's included: too, and has 29 log lines that publish error.localizedDescription, including a failed database switch and failed table, database and schema loads in ConnectionCoordinator. The same fix applies, but it needs an iOS build to verify, and the check leaves the directory out until then.
  • About 10 plugin log lines still publish table, schema or database names (PostgreSQL and MySQL fetchTriggers and fetchForeignKeys, Snowflake fetchColumns, Elasticsearch browse queries). fix(diagnostics): stop publishing user data to the system log #2960 moved names to .private(mask: .hash) in the app; the plugins need the same pass.
  • Five changed plugin files still carry 6 SwiftLint violations of other rules (sorted_imports, closure_parameter_position, number_separator, vertical_whitespace_closing_braces, inclusive_language), all present on main at the same lines.
  • BigQuery, Elasticsearch, SQL Server, MongoDB, Oracle and Snowflake are registry-only, so users get these fixes when those plugins are next released, not with the app.

Verification

Rebased on main at 5cff78b.

  • Build and app tests: verify.sh test over LogRedactionTests, ServerOutputCaptureTests, PluginBundleLoaderDescribeLoadFailureTests and CSVDocumentUndoTests, PASS, 16 cases executed, 0 failed. This build includes the mcp-server helper, which now links TableProLogRedaction.
  • Package tests: swift test --package-path Packages/TableProCore --filter LogRedactionShapeTests, 8 tests (one parameterized over 6 domains), all passed. The opt-in case returns text no fallback could produce, so removing the PubliclyLoggableError branch turns it red.
  • Every plugin: verify.sh plugins (AllPlugins), PASS.
  • Check: check-log-privacy.py exits 0 here, test_check_log_privacy.py passes its 4 tests, actionlint is clean. The widened rule was also run through SwiftLint itself on a fixture file and over TablePro and Packages: it flags every rejected spelling and nothing in the tree.
  • Lint: swiftlint lint --strict over the 32 changed Swift files as file paths, with this branch's .swiftlint.yml: only the 6 pre-existing findings above.
  • Review: Codex review and adversarial-review read an earlier version that put LogRedaction in TableProPluginKit. Their findings are addressed: the gate missed error.message and multi-line interpolation, mcp-server compiles LogRedaction.swift and needed a declared dependency, dynamic NSError domains were published, and more sites leaked. Codex then ran out of quota, so two Skill(code-review) passes (Claude) read the final design. Fixed from them: record names in sync logs, the Elasticsearch filter and body logs, the AppleScript, document-open and MCP bridge leaks, private descriptions beside every shape, a name-agnostic regex with fixture tests, a tautological test and the drifted app scan removed, comments removed from the new file.
  • Dismissed, with reasons: merging the CHANGELOG line into fix(diagnostics): stop publishing user data to the system log #2960's (it names a different gap, and rewording that line would move its credit); a public Error.publicLogShape in the module (ambiguous in the test target, measured); calling LogRedaction from TableProOracleCore (a separate package with no dependency on TableProCore, so it publishes the type or the TLS failure kind instead); hiding every domain outside a fixed list (identifier-shaped domains such as CKErrorDomain stay useful and cannot carry server text).

Found while investigating #3048 (see #3060).

@datlechin
datlechin merged commit 91a86c8 into main Sep 23, 2026
7 checks passed
@datlechin
datlechin deleted the fix/plugin-public-error-text-in-log branch September 23, 2026 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant