diff --git a/CHANGELOG.md b/CHANGELOG.md index d46fb235..3c8d4d0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **`querya-ext-tester` sends a `--connect` password with `system.injectCredentials`, like the app** — a driver that reads its password only from the injected secret (the ClickHouse driver) failed authentication under the tester because the password was in the `db.connect` payload only. - **Demo Marketplace themes were named `${manifest.name}`** — the generated theme.json now carries the real name (the demo catalog is only used with `--dart-define=QUERYA_MARKETPLACE_MOCK=true`). - **In-app update of a package install (AUR, `.deb`, `.rpm`)** — the updater no longer downloads a zip and quits when the app lives in `/opt/querya-desktop` (owned by root): the update dialog says how to update it (`yay -Syu querya-desktop`, `apt`, `dnf`) with a *Copy update command* button, or offers the release page for any other folder the user cannot write. If an in-place replace fails anyway, the app is started again and the reason is logged to `~/.local/state/querya-update.log`, instead of staying closed. diff --git a/docs/extension-testing.md b/docs/extension-testing.md index e1c3789d..54f83ebf 100644 --- a/docs/extension-testing.md +++ b/docs/extension-testing.md @@ -13,6 +13,7 @@ server, Flutter or the Querya Desktop source tree. reported with its JSON path. 2. **Launch** — the `main` entry point exists and starts. 3. **RPC lifecycle**, in this order: `system.handshake`, `system.ping`, + `system.injectCredentials` (only when `--connect` has a `password`), `db.connect`, `db.getCapabilities`, `db.getServerStats`, `db.getSchemaTree`, `db.query` (must return `{"columns": [...], "rows": [...]}`), `db.getTableSchema` (only with `--table`), `db.disconnect`, @@ -20,8 +21,8 @@ server, Flutter or the Querya Desktop source tree. `system.handshake`, `system.shutdown` and the `db.connect` / `db.disconnect` / `db.query` / `db.getSchemaTree` calls are **required**: a failure fails the -run. `system.ping`, `db.getCapabilities`, `db.getServerStats` and -`db.getTableSchema` are **recommended**: the host falls back gracefully, so +run. `system.ping`, `system.injectCredentials`, `db.getCapabilities`, +`db.getServerStats` and `db.getTableSchema` are **recommended**: the host falls back gracefully, so the runner only warns. Methods the emulated host version does not send are skipped. @@ -78,9 +79,12 @@ querya-ext-tester ./my-extension \ Exit codes: `0` all checks passed (warnings and skips do not fail, unless `--strict`), `1` a check failed, `2` bad command line. -Credentials only travel inside the `db.connect` payload over the plugin's -stdio; they are never put on the command line of the plugin or into its -environment. Use a throwaway database in CI. +A `password` in `--connect` is sent the way the app sends it: with +`system.injectCredentials` before `db.connect`, and left out of the `db.connect` +payload. A driver that reads its password only from the injected secret (as the +ClickHouse driver does) therefore works with the tester. Credentials travel over +the plugin's stdio only; they are never put on the command line of the plugin or +into its environment. Use a throwaway database in CI. ## Following the data: `--trace` diff --git a/lib/core/extensions/harness/extension_harness.dart b/lib/core/extensions/harness/extension_harness.dart index ea179cdf..66638e6c 100644 --- a/lib/core/extensions/harness/extension_harness.dart +++ b/lib/core/extensions/harness/extension_harness.dart @@ -153,7 +153,8 @@ final class HarnessConnectionOptions { this.database, }); - /// Extra `db.connect` fields (`host`, `port`, `username`, `password`, ...). + /// Extra `db.connect` fields (`host`, `port`, `username`, ...). A `password` + /// is sent with `system.injectCredentials` instead, as the app does. final Map params; /// Statement for the `db.query` step. @@ -168,6 +169,7 @@ final class HarnessConnectionOptions { /// Querya Desktop does, without a GUI. /// /// Sequence: manifest check, launch, `system.handshake`, `system.ping`, +/// `system.injectCredentials` (only when the connection has a password), /// `db.connect`, `db.getCapabilities`, `db.getServerStats`, /// `db.getSchemaTree`, `db.query`, `db.getTableSchema` (when a table is /// given), `db.disconnect`, `system.shutdown`. Methods the emulated host @@ -381,10 +383,15 @@ class QueryaExtensionHarness { await step(name, body, level: level); } + // Like the app: the password goes to the plugin in memory with + // system.injectCredentials and is not part of the db.connect payload. + final password = connection.params['password']; + final injectPassword = password is String && password.isNotEmpty; final connectParams = { 'connectionId': _connectionId, if (connection.database != null) 'database': connection.database, - ...connection.params, + for (final e in connection.params.entries) + if (e.key != 'password') e.key: e.value, }; final idOnly = {'connectionId': _connectionId}; @@ -394,6 +401,14 @@ class QueryaExtensionHarness { await method('system.ping', () async { await call('system.ping'); }); + if (injectPassword) { + await method('system.injectCredentials', () async { + await call('system.injectCredentials', { + 'connectionId': _connectionId, + 'password': password, + }); + }); + } await method('db.connect', () async { await call('db.connect', connectParams); }); diff --git a/lib/core/extensions/harness/extension_protocol_profile.dart b/lib/core/extensions/harness/extension_protocol_profile.dart index b4290fa1..e1138657 100644 --- a/lib/core/extensions/harness/extension_protocol_profile.dart +++ b/lib/core/extensions/harness/extension_protocol_profile.dart @@ -80,6 +80,8 @@ final class ExtensionProtocolProfile { 'system.shutdown', minSupported, HarnessMethodLevel.required), HarnessProtocolMethod( 'system.ping', minSupported, HarnessMethodLevel.recommended), + HarnessProtocolMethod( + 'system.injectCredentials', minSupported, HarnessMethodLevel.recommended), HarnessProtocolMethod( 'db.connect', minSupported, HarnessMethodLevel.required), HarnessProtocolMethod( diff --git a/test/core/extensions/harness/extension_harness_test.dart b/test/core/extensions/harness/extension_harness_test.dart index fbd3e1c9..781e240a 100644 --- a/test/core/extensions/harness/extension_harness_test.dart +++ b/test/core/extensions/harness/extension_harness_test.dart @@ -227,6 +227,61 @@ void main() { expect(tableParams, containsPair('tableName', 'orders')); }); + group('credentials', () { + test('a password is injected before connect and is not in its payload', + () async { + writeExtension(manifest()); + Map? injected; + Map? connectParams; + final handlers = _healthy() + ..['system.injectCredentials'] = (p) { + injected = p; + return {'ok': true}; + } + ..['db.connect'] = (p) { + connectParams = p; + return {'serverVersion': '1'}; + }; + final (report, plugin) = await run( + handlers, + connection: const HarnessConnectionOptions( + params: {'host': 'db', 'username': 'u', 'password': 's3cret'}, + ), + ); + expect(report.isSuccess, isTrue, reason: report.toConsole()); + expect(injected, {'connectionId': 1, 'password': 's3cret'}); + expect(connectParams, containsPair('username', 'u')); + expect(connectParams, isNot(contains('password'))); + expect(plugin!.calls.indexOf('system.injectCredentials'), + lessThan(plugin.calls.indexOf('db.connect'))); + expect(check(report, 'system.injectCredentials').status, + HarnessCheckStatus.passed); + }); + + test('without a password nothing is injected', () async { + writeExtension(manifest()); + final (report, plugin) = await run( + _healthy(), + connection: const HarnessConnectionOptions(params: {'host': 'db'}), + ); + expect(report.isSuccess, isTrue, reason: report.toConsole()); + expect(plugin!.calls, isNot(contains('system.injectCredentials'))); + expect(report.checks.any((c) => c.name == 'system.injectCredentials'), + isFalse); + }); + + test('a plugin without injectCredentials only warns', () async { + writeExtension(manifest()); + final (report, _) = await run( + _healthy(), + connection: const HarnessConnectionOptions(params: {'password': 'p'}), + ); + expect(check(report, 'system.injectCredentials').status, + HarnessCheckStatus.warning); + expect(check(report, 'db.connect').status, HarnessCheckStatus.passed); + }); + }); + test('methods newer than the target host are skipped', () async { writeExtension(manifest()); final (report, plugin) = await run( @@ -380,9 +435,14 @@ void main() { ), ); + // The password travels in system.injectCredentials, not in db.connect. + final inject = + report.rpc.firstWhere((c) => c.method == 'system.injectCredentials'); + expect((inject.request! as Map)['password'], '***'); + final connect = report.rpc.firstWhere((c) => c.method == 'db.connect'); final request = connect.request! as Map; - expect(request['password'], '***'); + expect(request, isNot(contains('password'))); expect((request['ssh'] as Map)['passphrase'], '***'); expect(request['host'], 'db'); expect(report.toTraceJson(), isNot(contains('s3cret'))); diff --git a/test/core/extensions/harness/extension_protocol_profile_test.dart b/test/core/extensions/harness/extension_protocol_profile_test.dart index 2899a7be..ac89256d 100644 --- a/test/core/extensions/harness/extension_protocol_profile_test.dart +++ b/test/core/extensions/harness/extension_protocol_profile_test.dart @@ -25,6 +25,7 @@ void main() { final p = ExtensionProtocolProfile.forTarget('0.4.11'); expect(p.supports('system.handshake'), isTrue); expect(p.supports('db.query'), isTrue); + expect(p.supports('system.injectCredentials'), isTrue); expect(p.supports('db.getTableSchema'), isFalse); expect(p.supports('commands.execute'), isFalse); }); @@ -37,7 +38,8 @@ void main() { test('0.5.0 speaks everything the current host sends', () { final p = ExtensionProtocolProfile.forTarget('0.5.0'); - expect(p.methods.length, 11); + expect(p.methods.length, 12); + expect(p.supports('system.injectCredentials'), isTrue); }); test('levels distinguish required from recommended methods', () {