feat(connections): open a database from the terminal and DDEV (#1486)#1858
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f889db4fdd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| internal init(connection: DatabaseConnection, scopeName: String?) { | ||
| self.init( | ||
| databaseType: connection.type.rawValue, | ||
| host: connection.host, |
There was a problem hiding this comment.
Because the key and isLoopbackHost use only connection.host, SSH/tunneled URLs are treated as local whenever the database host on the remote side is 127.0.0.1. For example, after trusting a DDEV link like mysql://db:db@127.0.0.1/.../db?name=ddev-shop, a URL such as mysql+ssh://deploy:pw@evil.example/db:db@127.0.0.1/db?name=ddev-shop builds the same trust key while the actual SSH endpoint is stored separately in sshConfig, so ExternalConnectionGate can skip the confirmation for a remote connection. Include the tunnel/SSH identity in the trust decision or disallow Always Allow when a tunnel is enabled.
Useful? React with 👍 / 👎.
datlechin
force-pushed
the
feat/1486-open-db-from-terminal
branch
from
f889db4 to
ca7f771
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca7f771a4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for octet in octets { | ||
| guard !octet.isEmpty, | ||
| octet.allSatisfy({ $0.isASCII && $0.isNumber }), | ||
| let value = Int(octet), value <= 255 |
There was a problem hiding this comment.
Because the loopback check parses octets with Int, hosts such as 0127.0.0.1 are classified as 127/8 and get the Always Allow path. In environments where the database client/system resolver accepts legacy IPv4 notation, that same host can resolve as octal 87.0.0.1, so a remote connection can be persisted as trusted and later skip the external-link confirmation. Reject IPv4 octets with leading zeros (or resolve/canonicalize before comparing) before allowing loopback trust.
Useful? React with 👍 / 👎.
…pyable code block
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59ad55e2e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| internal init(connection: DatabaseConnection, scopeName: String?) { | ||
| self.init( | ||
| databaseType: connection.type.rawValue, | ||
| host: connection.host, |
There was a problem hiding this comment.
When a saved MongoDB connection has host == 127.0.0.1 but additionalFields["mongoHosts"] contains other seeds, this key records only connection.host, so key.isLoopbackHost offers and persists Always Allow. The driver later prefers additionalFields["mongoHosts"] over config.host (Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift:58), so a URL matching the first seed can skip the external-link prompt while the actual connection includes remote hosts. Include the effective Mongo host list in the trust check/key, or suppress Always Allow for MongoDB seed-list connections.
Useful? React with 👍 / 👎.
1 participant
Closes #1486.
Opens a project database in TablePro from the shell, and makes
ddev tableprowork.What ships
ddev tablepro: submitted upstream as a DDEV global host command, feat: add ddev tablepro host command for TablePro ddev/ddev#8580. DDEV creates it automatically for anyone with TablePro in/Applications, so there is nothing to copy or install. It readsDDEV_HOST_DB_PORTon every run, because DDEV reassigns the database port on eachddev start, and callsopen -b com.TableProso TablePro wins even when TablePlus also owns themysql://scheme. Nothing in the app had to change for it to work.tableprocommand: install it from Settings > General > Command Line. It writes a two-line shim to/usr/local/binthat always opens TablePro./usr/local/binis root-owned on a stock macOS install and often absent entirely, so when TablePro cannot write there it asks macOS for an administrator password (osascript ... with administrator privileges) and creates the folder and the command. Cancelling the prompt writes nothing. If even that fails, Settings falls back to showing the exactsudocommand.ddev tableproneeded a click every time. The alert now offers Always Allow, and Settings > General > Trusted Links lists and revokes what you trusted.Enviromentis accepted as an alias for theenvURL parameter. That is the misspelling TablePlus and DDEV actually emit, and it was being silently dropped.Security boundary
Always Allow is offered only for a database on your own machine (
localhost,127.0.0.1/8,::1), and that check is enforced on read as well as write, so a poisoned defaults blob cannot resurrect trust for a remote host. A link to a remote host asks every time, with no way to silence it. This kills the "a web page makes TablePro connect to the attacker's server" case.The trust key is database type + host + database + username + the URL's
nameparameter. It deliberately excludes the port, because DDEV hands out a new host port on every start and a port-bearing key would re-prompt forever. Because every DDEV project shares the same default credentials (db:db@127.0.0.1/db),name=ddev-<project>is what keeps one project's trust from covering another.Trust only skips the initial connect alert. SQL from a link (
?query=), a filter (?condition=), and a pre-connect script each still ask for their own confirmation, and a transient connection never carries a pre-connect script.Notes for review
ExternalConnectionGateholds the decision (trusted? prompt? persist?) as a plain unit behind two protocol seams, so it is unit-tested without anNSAlert.ExternalConnectionAlertPromptis the untestable UI half, matching howHostKeyStore/HostKeyVerifierare split.tableproshim is generated from a string and written at install time rather than bundled as a Resource. A script insideContents/MacOSwould be fed to the unguardedstrip -xloop inscripts/build-release.shand break every release build.OSAScriptPrivilegedShell.quote(POSIX single-quote escaping) and then escaped again for the AppleScript string literal. Both layers are unit-tested, including a directory name that tries to inject a second command (x'; touch canary; echo ') and asserts the canary is never created. Nothing user-supplied reaches the string: the only interpolated values are the tool path and a fixed directory.Tests
28 new tests across
ExternalConnectionTrustStoreTests,ExternalConnectionGateTests,CommandLineToolInstallerTests, plus two parser tests. They cover the loopback lookalikes (127.evil.com,0x7f.0.0.1), the poisoned-defaults case, port-independent trust across a DDEV restart, trust not leaking between projects, and the installer refusing to overwrite or delete a foreign file at/usr/local/bin/tablepro.swiftlint --strictclean. Build and tests green.Follow-ups, not in this PR
Localizable.xcstrings.xcodebuilddoes not extract them; one build in Xcode will.ddev tableprois missing. That wording holds once feat: add ddev tablepro host command for TablePro ddev/ddev#8580 ships in a DDEV release.