From 76c566574aa6060074bbb4d0053aa3e643ebb67c Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 6 Jul 2026 12:08:00 +0100 Subject: [PATCH 1/2] Fix flaky 3PID inhibit error tests These tests could flake as the endpoints would intentionally wait for 100-1000ms in order to prevent a timing attack allowing one to deduce whether an email was already registered. The tests only waiting 1000ms for the endpoint to return, so a randomly-chosen jitter any higher than ~900ms would cause the test to fail. I'm unsure why this started failing recently, as the jitter has been in place for years. There was a recent change with the `Duration` work - we used `randrandom.randint(1, 10) / 10` and now `Duration(milliseconds=random.randint(100, 1000))`, but the math is technically equivalent. --- tests/rest/client/test_account.py | 10 +++++++++- tests/rest/client/test_register.py | 3 +++ tests/server.py | 5 ++++- tests/unittest.py | 4 ++++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/tests/rest/client/test_account.py b/tests/rest/client/test_account.py index 42102230f00..158d7f33f0d 100644 --- a/tests/rest/client/test_account.py +++ b/tests/rest/client/test_account.py @@ -325,7 +325,13 @@ def test_password_reset_bad_email_inhibit_error(self) -> None: email = "test@example.com" client_secret = "foobar" - session_id = self._request_token(email, client_secret) + session_id = self._request_token( + email, + client_secret, + # The endpoint intentionally adds up to 1000ms of jitter to avoid + # leaking whether the email address is bound to an account. + timeout_ms=3000, + ) self.assertIsNotNone(session_id) @@ -364,6 +370,7 @@ def _request_token( client_secret: str, ip: str = "127.0.0.1", next_link: str | None = None, + timeout_ms: int = 1000, ) -> str: body = {"client_secret": client_secret, "email": email, "send_attempt": 1} if next_link is not None: @@ -373,6 +380,7 @@ def _request_token( b"account/password/email/requestToken", body, client_ip=ip, + timeout_ms=timeout_ms, ) if channel.code != 200: diff --git a/tests/rest/client/test_register.py b/tests/rest/client/test_register.py index f66c56a6b19..a9f3ac24627 100644 --- a/tests/rest/client/test_register.py +++ b/tests/rest/client/test_register.py @@ -753,6 +753,9 @@ def test_request_token_existing_email_inhibit_error(self) -> None: "POST", b"register/email/requestToken", {"client_secret": "foobar", "email": email, "send_attempt": 1}, + # The endpoint intentionally adds up to 1000ms of jitter to avoid + # leaking whether the email address is already bound to an account. + timeout_ms=3000, ) self.assertEqual(200, channel.code, channel.result) diff --git a/tests/server.py b/tests/server.py index eea11b301f9..ab4aae02185 100644 --- a/tests/server.py +++ b/tests/server.py @@ -371,6 +371,7 @@ def make_request( await_result: bool = True, custom_headers: Iterable[CustomHeaderType] | None = None, client_ip: str = "127.0.0.1", + timeout_ms: int = 1000, ) -> FakeChannel: """ Make a web request using the given method, path and content, and render it @@ -399,6 +400,8 @@ def make_request( custom_headers: (name, value) pairs to add as request headers client_ip: The IP to use as the requesting IP. Useful for testing ratelimiting. + timeout_ms: if `await_result` is `True`, the amount of time to wait on + the request before timing out. Ignored otherwise. Returns: channel @@ -483,7 +486,7 @@ def make_request( req.requestReceived(method, path, b"1.1") if await_result: - channel.await_result() + channel.await_result(timeout_ms=timeout_ms) return channel diff --git a/tests/unittest.py b/tests/unittest.py index 5f7d0b3abf2..7dccff81c0f 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -571,6 +571,7 @@ def make_request( await_result: bool = True, custom_headers: Iterable[CustomHeaderType] | None = None, client_ip: str = "127.0.0.1", + timeout_ms: int = 1000, ) -> FakeChannel: """ Create a SynapseRequest at the path using the method and containing the @@ -599,6 +600,8 @@ def make_request( client_ip: The IP to use as the requesting IP. Useful for testing ratelimiting. + timeout_ms: if `await_result` is `True`, the amount of time to wait on + the request before timing out. Ignored otherwise. Returns: The FakeChannel object which stores the result of the request. @@ -618,6 +621,7 @@ def make_request( await_result, custom_headers, client_ip, + timeout_ms, ) def setup_test_homeserver( From 3664bdb1b58f4bfeb19c01f4dad2881c369c6641 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 6 Jul 2026 12:11:09 +0100 Subject: [PATCH 2/2] newsfile --- changelog.d/19913.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/19913.misc diff --git a/changelog.d/19913.misc b/changelog.d/19913.misc new file mode 100644 index 00000000000..31b5a4d37b9 --- /dev/null +++ b/changelog.d/19913.misc @@ -0,0 +1 @@ +Fix a flake in 3PID inhibit error unit tests, causing occasional failures in CI. \ No newline at end of file