Copilot commented on code in PR #75215:
URL: https://github.com/apache/airflow/pull/75215#discussion_r4327867773
##########
airflow-core/tests/unit/cli/commands/test_connection_command.py:
##########
@@ -181,23 +181,124 @@ def
test_cli_connections_list_does_not_warn_by_default(self, monkeypatch):
class TestUriMasking:
"""Test URI credential masking functionality."""
+ @pytest.mark.parametrize(
+ ("conn", "expected"),
+ [
+ # Full credentials (user and password)
+ (
+ Connection(
+ conn_id="c1",
+ conn_type="postgresql",
+ host="host",
+ port=5432,
+ schema="db",
+ login="user",
+ password="pass",
+ ),
+ "postgresql://***:***@host:5432/db",
+ ),
+ (
+ Connection(
+ conn_id="c2",
+ conn_type="mysql",
+ host="localhost",
+ port=3306,
+ schema="test",
+ login="admin",
+ password="secret",
+ ),
+ "mysql://***:***@localhost:3306/test",
+ ),
+ (
+ Connection(
+ conn_id="c3",
+ conn_type="http",
+ host="api.example.com",
+ port=8080,
+ schema="v1",
+ login="api",
+ password="key123",
+ ),
+ "http://***:***@api.example.com:8080/v1";,
+ ),
+ (
+ Connection(
+ conn_id="c4",
+ conn_type="postgresql",
+ host="host",
+ port=5432,
+ schema="db",
+ login="user",
+ password="p@ssword",
+ ),
+ "postgresql://***:***@host:5432/db",
+ ),
+ # Username only
+ (
+ Connection(
+ conn_id="c5",
+ conn_type="ssh",
+ host="github.com",
+ port=22,
+ schema="repo",
+ login="git",
+ ),
+ "ssh://***@github.com:22/repo",
+ ),
+ # Password only (token)
+ (
+ Connection(
+ conn_id="c6",
+ conn_type="http",
+ host="api.example.com",
+ password="secret_token",
+ ),
+ "http://:***@api.example.com";,
+ ),
+ # Empty password (user:)
+ (
+ Connection(
+ conn_id="c7",
+ conn_type="http",
+ host="api.example.com",
+ login="user",
+ password="",
+ ),
+ "http://***:@api.example.com";,
Review Comment:
This expectation fails: `FernetFieldsMixin.set_password()` ignores empty
strings (`models/crypto.py:116–121`), leaving this connection's password as
`None`. `get_uri()` therefore emits `http://[email protected]`, which is
correctly masked without a colon. Expect the username-only result here;
preservation of an explicit empty password needs a literal URI case in the
URI-based tests.
##########
airflow-core/tests/unit/cli/commands/test_connection_command.py:
##########
@@ -181,23 +181,124 @@ def
test_cli_connections_list_does_not_warn_by_default(self, monkeypatch):
class TestUriMasking:
"""Test URI credential masking functionality."""
+ @pytest.mark.parametrize(
+ ("conn", "expected"),
+ [
+ # Full credentials (user and password)
+ (
+ Connection(
+ conn_id="c1",
+ conn_type="postgresql",
+ host="host",
+ port=5432,
+ schema="db",
+ login="user",
+ password="pass",
+ ),
+ "postgresql://***:***@host:5432/db",
+ ),
+ (
+ Connection(
+ conn_id="c2",
+ conn_type="mysql",
+ host="localhost",
+ port=3306,
+ schema="test",
+ login="admin",
+ password="secret",
+ ),
+ "mysql://***:***@localhost:3306/test",
+ ),
+ (
+ Connection(
+ conn_id="c3",
+ conn_type="http",
+ host="api.example.com",
+ port=8080,
+ schema="v1",
+ login="api",
+ password="key123",
+ ),
+ "http://***:***@api.example.com:8080/v1";,
+ ),
+ (
+ Connection(
+ conn_id="c4",
+ conn_type="postgresql",
+ host="host",
+ port=5432,
+ schema="db",
+ login="user",
+ password="p@ssword",
+ ),
+ "postgresql://***:***@host:5432/db",
+ ),
+ # Username only
+ (
+ Connection(
+ conn_id="c5",
+ conn_type="ssh",
+ host="github.com",
+ port=22,
+ schema="repo",
+ login="git",
+ ),
+ "ssh://***@github.com:22/repo",
+ ),
+ # Password only (token)
+ (
+ Connection(
+ conn_id="c6",
+ conn_type="http",
+ host="api.example.com",
+ password="secret_token",
+ ),
+ "http://:***@api.example.com";,
+ ),
+ # Empty password (user:)
+ (
+ Connection(
+ conn_id="c7",
+ conn_type="http",
+ host="api.example.com",
+ login="user",
+ password="",
+ ),
+ "http://***:@api.example.com";,
+ ),
+ # Connections without credentials
+ (
+ Connection(
+ conn_id="c8",
+ conn_type="http",
+ host="example.com",
+ ),
+ "http://example.com";,
+ ),
+ (
+ Connection(
+ conn_id="c9",
+ conn_type="redis",
+ host="localhost",
+ port=6379,
+ schema="0",
+ ),
+ "redis://localhost:6379/0",
+ ),
+ ],
+ )
+ def test_mask_uri_credentials_from_connection(self, conn, expected):
+ result = _mask_uri_credentials(conn.get_uri())
+ assert result == expected
+
@pytest.mark.parametrize(
("uri", "expected"),
[
- # URIs with credentials
- ("postgresql://user:pass@host:5432/db",
"postgresql://***:***@host:5432/db"),
- ("mysql://admin:secret@localhost:3306/test",
"mysql://***:***@localhost:3306/test"),
- ("http://api:[email protected]:8080/v1";,
"http://***:***@api.example.com:8080/v1";),
- # URIs without credentials
- ("sqlite:///tmp/test.db", "sqlite:///tmp/test.db"),
- ("filesystem://", "filesystem://"),
- ("redis://localhost:6379/0", "redis://localhost:6379/0"),
- # Edge cases
("", ""),
("invalid-uri", "***"), # Falls back to full masking on parse
error
Review Comment:
The last-`@` split and empty-userinfo return lack regression coverage.
`Connection.get_uri()` percent-encodes credential `@` characters, so the
`p@ssword` fixture also passes with the old first-`@` split. It also omits the
password separator for the empty-string fixture. Add literal URI cases for
multiple `@` characters, empty userinfo, and an explicit empty password to
exercise these changed paths.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
