diff --git a/crates/pgls_configuration/src/database.rs b/crates/pgls_configuration/src/database.rs index d32ff8222..de87a2296 100644 --- a/crates/pgls_configuration/src/database.rs +++ b/crates/pgls_configuration/src/database.rs @@ -43,6 +43,9 @@ pub struct DatabaseConfiguration { #[partial(bpaf(long("database")))] pub database: String, + /// Glob patterns of the databases that code actions may execute statements against, matched + /// against `host:port/database`. A pattern without a port, e.g. `localhost/*`, is matched + /// against `host/database` and allows every port. #[partial(bpaf(long("allow_statement_executions_against")))] pub allow_statement_executions_against: StringSet, diff --git a/crates/pgls_workspace/src/settings.rs b/crates/pgls_workspace/src/settings.rs index 8bce302eb..36fa0fd1b 100644 --- a/crates/pgls_workspace/src/settings.rs +++ b/crates/pgls_workspace/src/settings.rs @@ -785,9 +785,19 @@ impl DatabaseSettings { } let host = &self.host; + let port = self.port; let database = &self.database; + // A pattern can't name the port of an IPv6 address, see `pattern_names_port`. + let (target, pattern) = if host.contains(':') { + (format!("[{host}]:{port}/{database}"), format!("{host}/*")) + } else { + ( + format!("{host}:{port}/{database}"), + format!("{host}:{port}/*"), + ) + }; let mut reason = format!( - "Statement execution is disabled for `{host}/{database}`. Add a matching pattern to `db.allowStatementExecutionsAgainst`, e.g. [\"{host}/*\"]." + "Statement execution is disabled for `{target}`. Add a matching pattern to `db.allowStatementExecutionsAgainst`, e.g. [\"{pattern}\"]." ); if !self.invalid_statement_execution_patterns.is_empty() { let patterns = self @@ -840,7 +850,7 @@ impl Default for DatabaseSettings { password: None, database: "postgres".to_string(), conn_timeout_secs: Duration::from_secs(10), - allow_statement_executions: true, + allow_statement_executions: false, invalid_statement_execution_patterns: Vec::new(), connection_string_error: None, } @@ -903,8 +913,10 @@ impl From for DatabaseSettings { && connection_string_error.is_none(); // An invalid pattern is ignored rather than failing the whole configuration; the reason a - // statement cannot be executed names it. + // statement cannot be executed names it. A pattern that names a port only matches that + // port, any other pattern matches every port. let target = format!("{host}/{database}"); + let target_with_port = format!("{host}:{port}/{database}"); let mut allow_statement_executions = false; let mut invalid_statement_execution_patterns = Vec::new(); for pattern in value @@ -913,7 +925,14 @@ impl From for DatabaseSettings { .flat_map(|set| set.iter()) { match Glob::new(pattern) { - Ok(glob) => allow_statement_executions |= glob.compile_matcher().is_match(&target), + Ok(glob) => { + let target = if pattern_names_port(pattern) { + &target_with_port + } else { + &target + }; + allow_statement_executions |= glob.compile_matcher().is_match(target); + } Err(err) => { warn!( "Ignoring invalid pattern `{pattern}` in `db.allowStatementExecutionsAgainst`: {err}" @@ -946,6 +965,14 @@ impl From for DatabaseSettings { } } +/// Whether a pattern of `db.allowStatementExecutionsAgainst` names a port, as `localhost:54322/*` +/// does. That is the case if its host, the part before the first `/`, has exactly one `:`, so a +/// pattern for an IPv6 address like `::1/*` names no port. +fn pattern_names_port(pattern: &str) -> bool { + let host = pattern.split('/').next().unwrap_or(pattern); + host.matches(':').count() == 1 +} + /// Parses `db.connectionString`. The error says why it is invalid without repeating it, as it may /// contain a password. #[cfg(feature = "db")] @@ -1220,7 +1247,7 @@ mod tests { assert_eq!( config.statement_execution_disabled_reason().as_deref(), Some( - "Statement execution is disabled for `production/test-db`. Add a matching pattern to `db.allowStatementExecutionsAgainst`, e.g. [\"production/*\"]." + "Statement execution is disabled for `production:5432/test-db`. Add a matching pattern to `db.allowStatementExecutionsAgainst`, e.g. [\"production:5432/*\"]." ) ); } @@ -1345,4 +1372,89 @@ mod tests { assert!(!config.enable_connection); assert_eq!(config.connection_string_error, None); } + + fn allows(patterns: &[&str], host: &str, port: u16, database: &str) -> bool { + DatabaseSettings::from(PartialDatabaseConfiguration { + allow_statement_executions_against: Some(StringSet::from_iter( + patterns.iter().map(|pattern| pattern.to_string()), + )), + host: Some(host.into()), + port: Some(port), + database: Some(database.into()), + ..Default::default() + }) + .allow_statement_executions + } + + #[test] + fn patterns_with_a_port_only_match_that_port() { + assert!(allows( + &["127.0.0.1:54322/*"], + "127.0.0.1", + 54322, + "postgres" + )); + assert!(!allows( + &["127.0.0.1:54322/*"], + "127.0.0.1", + 5432, + "postgres" + )); + assert!(allows(&["localhost:*/dev"], "localhost", 6543, "dev")); + assert!(!allows(&["localhost:*/dev"], "localhost", 6543, "prod")); + assert!(allows(&["*:5432/*"], "db.internal", 5432, "app")); + } + + #[test] + fn patterns_without_a_port_match_every_port() { + assert!(allows(&["localhost/*"], "localhost", 5432, "postgres")); + assert!(allows(&["localhost/*"], "localhost", 15432, "postgres")); + assert!(allows(&["*"], "production", 5432, "postgres")); + assert!(allows(&["::1/*"], "::1", 5432, "postgres")); + } + + #[test] + fn disabled_reason_suggests_a_pattern_with_the_port() { + let config = DatabaseSettings::from(PartialDatabaseConfiguration { + host: Some("::1".into()), + port: Some(5433), + database: Some("postgres".into()), + ..Default::default() + }); + + assert_eq!( + config.statement_execution_disabled_reason().as_deref(), + Some( + "Statement execution is disabled for `[::1]:5433/postgres`. Add a matching pattern to `db.allowStatementExecutionsAgainst`, e.g. [\"::1/*\"]." + ) + ); + } + + #[cfg(feature = "db")] + #[test] + fn connection_string_port_is_matched() { + let allows = |connection_string: &str| { + DatabaseSettings::from(PartialDatabaseConfiguration { + connection_string: Some(connection_string.to_string()), + allow_statement_executions_against: Some(StringSet::from_iter([String::from( + "localhost:54322/*", + )])), + ..Default::default() + }) + .allow_statement_executions + }; + + assert!(allows( + "postgres://postgres:postgres@localhost:54322/postgres" + )); + // A production database reached through a tunnel on another local port. + assert!(!allows( + "postgres://postgres:postgres@localhost:15432/postgres" + )); + } + + #[test] + fn statement_executions_are_disallowed_by_default() { + assert!(!DatabaseSettings::default().allow_statement_executions); + } } diff --git a/crates/pgls_workspace/src/workspace/server.tests.rs b/crates/pgls_workspace/src/workspace/server.tests.rs index 553a194e3..ec6399acd 100644 --- a/crates/pgls_workspace/src/workspace/server.tests.rs +++ b/crates/pgls_workspace/src/workspace/server.tests.rs @@ -2084,7 +2084,7 @@ async fn execute_statement_is_disabled_without_matching_pattern(test_db: PgPool) .unwrap() .to_string(); let reason = format!( - "Statement execution is disabled for `{host}/{database}`. Add a matching pattern to `db.allowStatementExecutionsAgainst`, e.g. [\"{host}/*\"]." + "Statement execution is disabled for `{host}:5432/{database}`. Add a matching pattern to `db.allowStatementExecutionsAgainst`, e.g. [\"{host}:5432/*\"]." ); assert_eq!(disabled_reason.as_deref(), Some(reason.as_str())); assert_eq!( diff --git a/docs/configuration.md b/docs/configuration.md index cb683abee..5f62c72fc 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -178,7 +178,7 @@ Some tools that the Postgres Language Server provides are implemented as mere in "password": "postgres", "database": "postgres", "connTimeoutSecs": 10, - "allowStatementExecutionsAgainst": ["127.0.0.1/*", "localhost/*"] + "allowStatementExecutionsAgainst": ["127.0.0.1:5432/*", "localhost:5432/*"] } } ``` @@ -191,7 +191,7 @@ individual fields. The URI takes precedence over any other connection fields. { "db": { "connectionString": "postgres://postgres:postgres@localhost:5432/postgres?sslmode=disable", - "allowStatementExecutionsAgainst": ["localhost/*"] + "allowStatementExecutionsAgainst": ["localhost:5432/*"] } } ``` diff --git a/docs/guides/configure_database.md b/docs/guides/configure_database.md index aa921aa1f..9e03fd938 100644 --- a/docs/guides/configure_database.md +++ b/docs/guides/configure_database.md @@ -30,9 +30,9 @@ Configure database connection details in your `postgres-language-server.jsonc` f "database": "your_database_name", // Connection timeout in seconds (default: 10) "connTimeoutSecs": 10, - // Glob patterns matched against `host/database`; code actions may execute + // Glob patterns matched against `host:port/database`; code actions may execute // statements only when one of them matches (default: [], never) - "allowStatementExecutionsAgainst": ["localhost/*", "127.0.0.1/*"], + "allowStatementExecutionsAgainst": ["localhost:5432/*", "127.0.0.1:5432/*"], // Completely disable database features (default: false) "disableConnection": false } @@ -106,18 +106,20 @@ GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA public TO postgres_language_server; ``` ### Statement Execution Control -Code actions that execute the statement under the cursor are disabled by default. To enable them, list glob patterns in `allowStatementExecutionsAgainst`. Each pattern is matched against `host/database` of the connection (taken from the connection string when one is set), and statements can only be executed when one of them matches: +Code actions that execute the statement under the cursor are disabled by default. To enable them, list glob patterns in `allowStatementExecutionsAgainst`. Each pattern is matched against `host:port/database` of the connection (taken from the connection string when one is set), and statements can only be executed when one of them matches: ```json { "db": { - // any database on the local machine - "allowStatementExecutionsAgainst": ["localhost/*", "127.0.0.1/*"] + // any database of the local Postgres on port 5432, and of a local Supabase project + "allowStatementExecutionsAgainst": ["127.0.0.1:5432/*", "127.0.0.1:54322/*"] } } ``` -The patterns do not refer to schemas. To allow a single database only, name it, e.g. `"localhost/my_dev_db"`. +Name the port. A port forward, an SSH tunnel, `cloud-sql-proxy` or `fly proxy` makes a remote database reachable on `localhost` too, on another port. A pattern without a port, such as `localhost/*`, is matched against `host/database` and allows every port, including those. + +The patterns do not refer to schemas. To allow a single database only, name it, e.g. `"localhost:5432/my_dev_db"`. A pattern can't name the port of an IPv6 address: `::1/*` matches every port of `::1`. When no pattern matches, the code action is disabled and its reason names the setting. An invalid pattern is ignored with a warning in the log, and the reason lists it. diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 7e2e3a7ce..e5a00e257 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -179,7 +179,8 @@ Lints your database schema. The password to connect to the database. Can also be set via the `PGPASSWORD` environment variable. - **` --database`**=_`ARG`_ — The name of the database. Can also be set via the `PGDATABASE` environment variable. -- **` --allow_statement_executions_against`**=_`ARG`_ +- **` --allow_statement_executions_against`**=_`ARG`_ — + Glob patterns of the databases that code actions may execute statements against, matched against `host:port/database`. A pattern without a port, e.g. `localhost/*`, is matched against `host/database` and allows every port. - **` --conn_timeout_secs`**=_`ARG`_ — The connection timeout in seconds. @@ -305,7 +306,8 @@ Runs everything to the requested files. The password to connect to the database. Can also be set via the `PGPASSWORD` environment variable. - **` --database`**=_`ARG`_ — The name of the database. Can also be set via the `PGDATABASE` environment variable. -- **` --allow_statement_executions_against`**=_`ARG`_ +- **` --allow_statement_executions_against`**=_`ARG`_ — + Glob patterns of the databases that code actions may execute statements against, matched against `host:port/database`. A pattern without a port, e.g. `localhost/*`, is matched against `host/database` and allows every port. - **` --conn_timeout_secs`**=_`ARG`_ — The connection timeout in seconds. @@ -453,7 +455,8 @@ Formats the requested files. The password to connect to the database. Can also be set via the `PGPASSWORD` environment variable. - **` --database`**=_`ARG`_ — The name of the database. Can also be set via the `PGDATABASE` environment variable. -- **` --allow_statement_executions_against`**=_`ARG`_ +- **` --allow_statement_executions_against`**=_`ARG`_ — + Glob patterns of the databases that code actions may execute statements against, matched against `host:port/database`. A pattern without a port, e.g. `localhost/*`, is matched against `host/database` and allows every port. - **` --conn_timeout_secs`**=_`ARG`_ — The connection timeout in seconds. diff --git a/docs/schema.json b/docs/schema.json index 04d524540..ea7e8ec5f 100644 --- a/docs/schema.json +++ b/docs/schema.json @@ -394,6 +394,7 @@ "type": "object", "properties": { "allowStatementExecutionsAgainst": { + "description": "Glob patterns of the databases that code actions may execute statements against, matched against `host:port/database`. A pattern without a port, e.g. `localhost/*`, is matched against `host/database` and allows every port.", "anyOf": [ { "$ref": "#/definitions/StringSet" diff --git a/packages/@postgres-language-server/backend-jsonrpc/src/workspace.ts b/packages/@postgres-language-server/backend-jsonrpc/src/workspace.ts index 91da02005..9c0f9fa32 100644 --- a/packages/@postgres-language-server/backend-jsonrpc/src/workspace.ts +++ b/packages/@postgres-language-server/backend-jsonrpc/src/workspace.ts @@ -533,6 +533,9 @@ export interface PartialConfiguration { * The configuration of the database connection. */ export interface PartialDatabaseConfiguration { + /** + * Glob patterns of the databases that code actions may execute statements against, matched against `host:port/database`. A pattern without a port, e.g. `localhost/*`, is matched against `host/database` and allows every port. + */ allowStatementExecutionsAgainst?: StringSet; /** * The connection timeout in seconds. diff --git a/packages/@postgrestools/backend-jsonrpc/src/workspace.ts b/packages/@postgrestools/backend-jsonrpc/src/workspace.ts index 91da02005..9c0f9fa32 100644 --- a/packages/@postgrestools/backend-jsonrpc/src/workspace.ts +++ b/packages/@postgrestools/backend-jsonrpc/src/workspace.ts @@ -533,6 +533,9 @@ export interface PartialConfiguration { * The configuration of the database connection. */ export interface PartialDatabaseConfiguration { + /** + * Glob patterns of the databases that code actions may execute statements against, matched against `host:port/database`. A pattern without a port, e.g. `localhost/*`, is matched against `host/database` and allows every port. + */ allowStatementExecutionsAgainst?: StringSet; /** * The connection timeout in seconds.