From a52c2422f46d0a7baeb0f6e746e09d30b40bd162 Mon Sep 17 00:00:00 2001 From: Ben Dalling Date: Fri, 3 Jan 2025 08:28:47 +0000 Subject: [PATCH 1/6] fix: dev: Reduce criticism from static checks. --- .gitignore | 1 + .qlty/qlty.toml | 103 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100644 .qlty/qlty.toml diff --git a/.gitignore b/.gitignore index b618a5c..f2fe15b 100644 --- a/.gitignore +++ b/.gitignore @@ -165,5 +165,6 @@ cython_debug/ .python-version .qlty/* +!.qlty/qlty.toml !.qlty/configs/ !.qlty/configs/** diff --git a/.qlty/qlty.toml b/.qlty/qlty.toml new file mode 100644 index 0000000..ad99b66 --- /dev/null +++ b/.qlty/qlty.toml @@ -0,0 +1,103 @@ +# This file was automatically generated by `qlty init`. +# You can modify it to suit your needs. +# We recommend you to commit this file to your repository. +# +# This configuration is used by both Qlty CLI and Qlty Cloud. +# +# Qlty CLI -- Code quality toolkit for developers +# Qlty Cloud -- Fully automated Code Health Platform +# +# Try Qlty Cloud: https://qlty.sh +# +# For a guide to configuration, visit https://qlty.sh/d/config +# Or for a full reference, visit https://qlty.sh/d/qlty-toml +config_version = "0" + +exclude_patterns = [ + "*_min.*", + "*-min.*", + "*.min.*", + "**/*.d.ts", + "**/.yarn/**", + "**/bower_components/**", + "**/build/**", + "**/cache/**", + "**/config/**", + "**/db/**", + "**/deps/**", + "**/dist/**", + "**/extern/**", + "**/external/**", + "**/generated/**", + "**/Godeps/**", + "**/gradlew/**", + "**/mvnw/**", + "**/node_modules/**", + "**/protos/**", + "**/seed/**", + "**/target/**", + "**/testdata/**", + "**/vendor/**", + "**/assets/**", + "**/tests/**", + "CHANGELOG.md", + ".devcontainer/devcontainer.json" +] + +test_patterns = [ + "**/test/**", + "**/spec/**", + "**/*.test.*", + "**/*.spec.*", + "**/*_test.*", + "**/*_spec.*", + "**/test_*.*", + "**/spec_*.*", +] + +[[source]] +name = "default" +default = true + +[[plugin]] +name = "actionlint" + +[[plugin]] +name = "bandit" +version = "1.7.9" + +[[plugin]] +name = "checkov" + +[[plugin]] +name = "golangci-lint" + +[[plugin]] +name = "markdownlint" +version = "0.41.0" + +[[plugin]] +name = "osv-scanner" + +[[plugin]] +name = "prettier" + +[[plugin]] +name = "ripgrep" + +[[plugin]] +name = "ruff" +version = "0.5.3" + +[[plugin]] +name = "trivy" +drivers = [ + "config", + "fs-vuln", +] + +[[plugin]] +name = "trufflehog" + +[[plugin]] +name = "yamllint" From 3791dc17da60e560ffab8aa1871800bac3804eeb Mon Sep 17 00:00:00 2001 From: Ben Dalling Date: Mon, 6 Jan 2025 07:18:38 +0000 Subject: [PATCH 2/6] fix: dev: Get tag directly from the script. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index c163254..5ac0181 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ .EXPORT_ALL_VARIABLES: -TAG = 0.2.1 +TAG = $$( python -c 'import router; print(router.__version__)' ) all: lint build clean test From a2fd0575a0662cdfe27605aedc25b00b97e9004d Mon Sep 17 00:00:00 2001 From: Ben Dalling Date: Mon, 6 Jan 2025 07:20:10 +0000 Subject: [PATCH 3/6] fix: Stop false negative security alert. --- replay_dlq.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/replay_dlq.py b/replay_dlq.py index 735de5c..094991e 100755 --- a/replay_dlq.py +++ b/replay_dlq.py @@ -148,7 +148,7 @@ def get_message_hash(self, message: Message) -> str: if type(body) is str: body = body.encode() - return hashlib.md5(body).hexdigest() + return hashlib.md5(body, usedforsecurity=False).hexdigest() def on_start(self, event): """Execute a start event.""" From cbcad85328bc8151c09b5a5579249d743d787883 Mon Sep 17 00:00:00 2001 From: Ben Dalling Date: Mon, 6 Jan 2025 08:59:57 +0000 Subject: [PATCH 4/6] fix: Handle non-URL compatible characters in namespace connection strings. --- router.py | 50 +++++++++++++++---- setup.cfg | 4 +- .../features/connection-string-helper.feature | 15 ++++-- tests/features/data-flow.feature | 1 + .../environment-config-parser.feature | 1 + tests/features/router-rule.feature | 1 + .../test_connection_string_helper.py | 21 ++++++++ 7 files changed, 76 insertions(+), 17 deletions(-) diff --git a/router.py b/router.py index 7ad9f21..afa5af5 100644 --- a/router.py +++ b/router.py @@ -101,6 +101,7 @@ def __init__(self, sbus_connection_string: str) -> None: self.port(5671) self.protocol('amqps') self.parse() + self.netloc(f'{self.protocol()}://{self.hostname()}:{self.port()}') url = f'{self.protocol()}://{self.key_name()}:{self.key_value()}' url += f'@{self.hostname()}:{self.port()}' self.amqp_url(url) @@ -177,6 +178,25 @@ def key_value(self, key_value: str = None) -> str: self._key_value = key_value return self._key_value + def netloc(self, netloc: str = None) -> str: + """ + Get or set the netloc. + + Parameters + ---------- + netloc : str, optional + The value to set. + + Returns + ------- + str + The currently set value. + """ + if netloc is not None: + self._netloc = netloc + + return self._netloc + def parse(self) -> None: """ Parse the connection string. @@ -502,8 +522,7 @@ def add(self, name: str, connection_string: str) -> None: connection_string : str The connection string for connecting to the namespace. """ - helper = ConnectionStringHelper(connection_string) - self._namespaces[name] = helper.amqp_url() + self._namespaces[name] = connection_string def count(self) -> int: """ @@ -536,8 +555,7 @@ def get(self, name: str) -> str: ValueError If the provided name is not known. """ - conn_str = self._namespaces[name] - return conn_str + return self._namespaces[name] class EnvironmentConfigParser: @@ -620,6 +638,10 @@ def get_rules(self) -> list[RouterRule]: return response + def get_source_connection_string(self) -> str: + """Get the connection string of the source namespace.""" + return self._environ['ROUTER_SOURCE_CONNECTION_STRING'] + def get_source_url(self) -> str: """ Get the URL of the source Service Bus Namespace. @@ -705,7 +727,7 @@ def __init__(self, config: EnvironmentConfigParser): self.rules = config.get_rules() self.senders = {} self.sources = [] - self.source_namespace_url = config.get_source_url() + self.source_namespace_connection_string = config.get_source_connection_string() self.parse_config_data() start_http_server(config.get_prometheus_port()) @@ -766,14 +788,20 @@ def on_start(self, event): """Respond to a start event.""" logger.debug('Start event.') - for url in self.connections.keys(): - hostname = urlparse(url).hostname + for connection_string in self.connections.keys(): + connection_details = ConnectionStringHelper(connection_string) + hostname = connection_details.netloc() logger.debug(f'Creating a connection for {hostname}...') - connection = event.container.connect(url, allowed_mechs='PLAIN') - self.connections[url] = connection + connection = event.container.connect( + url=connection_details.netloc(), + allowed_mechs='PLAIN', + password=connection_details.key_value(), + user=connection_details.key_name() + ) + self.connections[connection_string] = connection logger.info(f'Successfully created a connection for {hostname}.') - connection = self.connections[self.source_namespace_url] + connection = self.connections[self.source_namespace_connection_string] for source in self.sources: logger.debug(f'Creating a receiver for "{source}".') @@ -793,7 +821,7 @@ def on_start(self, event): def parse_config_data(self) -> dict: """Create a dict of connections with the URL as a key.""" - url_list = [self.source_namespace_url] + url_list = [self.source_namespace_connection_string] service_bus_namespaces = self.config.service_bus_namespaces() sources = [] diff --git a/setup.cfg b/setup.cfg index a7f06e9..49844ef 100644 --- a/setup.cfg +++ b/setup.cfg @@ -18,5 +18,5 @@ radon-max-cc = 5 addopts = --color=no --cov --durations 3 --verbose bdd_features_base_dir = tests/features/ markers = - system_tests: Marks tests as system tests. - unit_tests: Marks tests as unit tests. + system: Marks tests as system tests. + unit: Marks tests as unit tests. diff --git a/tests/features/connection-string-helper.feature b/tests/features/connection-string-helper.feature index 8eea561..82b8cbd 100644 --- a/tests/features/connection-string-helper.feature +++ b/tests/features/connection-string-helper.feature @@ -1,3 +1,4 @@ +@unit Feature: Connection String Helper In order to connect to Azure Service Bus As a developer @@ -7,12 +8,18 @@ Feature: Connection String Helper Given Azure Service Bus Connection String When the Azure Service Bus Connection String is parsed Then the AMQP URL is + And key_name is + And key_value is + And netloc is Examples: - | sbus_connection_string | amqp_url | - | Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE; | amqps://RootManageSharedAccessKey:SAS_KEY_VALUE@localhost:5671 | - | Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE=; | amqps://RootManageSharedAccessKey:SAS_KEY_VALUE=@localhost:5671 | - | Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE;UseDevelopmentEmulator=true; | amqp://RootManageSharedAccessKey:SAS_KEY_VALUE@localhost:5672 | + | sbus_connection_string | amqp_url | key_name | key_value | netloc | + | Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE; | amqps://RootManageSharedAccessKey:SAS_KEY_VALUE@localhost:5671 | RootManageSharedAccessKey | SAS_KEY_VALUE | amqps://localhost:5671 | + | Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE=; | amqps://RootManageSharedAccessKey:SAS_KEY_VALUE=@localhost:5671 | RootManageSharedAccessKey | SAS_KEY_VALUE= | amqps://localhost:5671 | + | Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=A+A+/aa= | amqps://RootManageSharedAccessKey:A+A+/aa=@localhost:5671 | RootManageSharedAccessKey | A+A+/aa= | amqps://localhost:5671 | + | Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=A+A+/aa=; | amqps://RootManageSharedAccessKey:A+A+/aa=@localhost:5671 | RootManageSharedAccessKey | A+A+/aa= | amqps://localhost:5671 | + | Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=A+A+/aa=;UseDevelopmentEmulator=true; | amqp://RootManageSharedAccessKey:A+A+/aa=@localhost:5672 | RootManageSharedAccessKey | A+A+/aa= | amqp://localhost:5672 | + | Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE;UseDevelopmentEmulator=true; | amqp://RootManageSharedAccessKey:SAS_KEY_VALUE@localhost:5672 | RootManageSharedAccessKey | SAS_KEY_VALUE | amqp://localhost:5672 | Scenario Outline: Invalid Connection Strings Given Azure Service Bus Connection String diff --git a/tests/features/data-flow.feature b/tests/features/data-flow.feature index d839055..1b271ac 100644 --- a/tests/features/data-flow.feature +++ b/tests/features/data-flow.feature @@ -1,3 +1,4 @@ +@system Feature: Data Flow In order to confirm rules As a Service Bus Router diff --git a/tests/features/environment-config-parser.feature b/tests/features/environment-config-parser.feature index 9e6b466..a02020f 100644 --- a/tests/features/environment-config-parser.feature +++ b/tests/features/environment-config-parser.feature @@ -1,3 +1,4 @@ +@unit Feature: Environment Configuration Parser Test that we can correctly extract configuration from diff --git a/tests/features/router-rule.feature b/tests/features/router-rule.feature index a4a0168..9b7f8ea 100644 --- a/tests/features/router-rule.feature +++ b/tests/features/router-rule.feature @@ -1,3 +1,4 @@ +@unit Feature: Router Rule In order to route a message As a Router diff --git a/tests/step_defs/test_connection_string_helper.py b/tests/step_defs/test_connection_string_helper.py index 1bebc61..59e5cc6 100644 --- a/tests/step_defs/test_connection_string_helper.py +++ b/tests/step_defs/test_connection_string_helper.py @@ -28,6 +28,27 @@ def _(): pass +@then(parsers.parse('key_name is {expected_key_name}')) +def _(expected_key_name: str, sbus_connection_string: str): + """key_name is .""" + widget = ConnectionStringHelper(sbus_connection_string) + assert widget.key_name() == expected_key_name + + +@then(parsers.parse('key_value is {expected_key_value}')) +def _(expected_key_value: str, sbus_connection_string: str): + """key_value is .""" + widget = ConnectionStringHelper(sbus_connection_string) + assert expected_key_value == widget.key_value() + + +@then(parsers.parse('netloc is {expected_netloc}')) +def _(expected_netloc: str, sbus_connection_string: str): + """netloc is .""" + widget = ConnectionStringHelper(sbus_connection_string) + assert widget.netloc() == expected_netloc + + @then(parsers.parse('the AMQP URL is {expected_url}')) def _(expected_url: str, sbus_connection_string): """the AMQP URL is .""" From 731b4ba7c72edd1f7eb41be833cd9155aafe59bd Mon Sep 17 00:00:00 2001 From: Ben Dalling Date: Mon, 6 Jan 2025 10:24:58 +0000 Subject: [PATCH 5/6] fix: dev: Correct how the replay script makes the initial connection. --- replay_dlq.py | 13 +++++++++---- tests/step_defs/test_data_flow.py | 18 +++++++++--------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/replay_dlq.py b/replay_dlq.py index 094991e..4290cdc 100755 --- a/replay_dlq.py +++ b/replay_dlq.py @@ -118,7 +118,7 @@ class DLQReplayHandler(MessagingHandler): How long to wait for messages to arrive before exiting. """ - def __init__(self, dlq_topic_name, connection, timeout, logger: logging.Logger): + def __init__(self, dlq_topic_name, connection: ConnectionStringHelper, timeout, logger: logging.Logger): super().__init__() self.dlq_topic_name = dlq_topic_name self.connection = connection @@ -153,8 +153,13 @@ def get_message_hash(self, message: Message) -> str: def on_start(self, event): """Execute a start event.""" self.reactor = event.reactor - self.logger.debug(f'Creating a connection for "{self.connection}".') - self.conn = event.container.connect(self.connection) + self.logger.debug(f'Creating a connection for "{self.connection.netloc()}".') + self.conn = event.container.connect( + url=self.connection.netloc(), + allowed_mechs='PLAIN', + password=self.connection.key_value(), + user=self.connection.key_name() + ) self.logger.debug(f'Creating a receiver for "{self.dlq_topic_name}...".') self.receiver = event.container.create_receiver(self.conn, source=self.dlq_topic_name) self.logger.debug(f'Receiver for "{self.dlq_topic_name}" created successfully.') @@ -227,7 +232,7 @@ def on_connection_close(self, event): helper = ConnectionStringHelper(runtime.connection_string) replayer = DLQReplayHandler( f'{runtime.dlq_topic_name}/Subscriptions/{runtime.subscription}', - helper.amqp_url(), + helper, runtime.timeout, runtime.logger ) diff --git a/tests/step_defs/test_data_flow.py b/tests/step_defs/test_data_flow.py index 48d758e..58bb1af 100644 --- a/tests/step_defs/test_data_flow.py +++ b/tests/step_defs/test_data_flow.py @@ -31,13 +31,13 @@ def _(input_topic: str): return input_topic -@given('the landing Service Bus Emulator', target_fixture='aqmp_url') +@given('the landing Service Bus Emulator', target_fixture='connection_details') def _(): """the landing Service Bus Emulator.""" conn_str = 'Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;' conn_str += 'SharedAccessKey=SAS_KEY_VALUE;UseDevelopmentEmulator=true;' conn_str = ConnectionStringHelper(conn_str) - return conn_str.amqp_url() + return conn_str @given(parsers.parse('the message contents is {input_data_file}'), target_fixture='message_body') @@ -58,18 +58,18 @@ def _(output_topic: str): @when('the DLQ messags are replayed') -def _(aqmp_url: str): +def _(connection_details: ConnectionStringHelper): """the DLQ messags are replayed.""" runtime = RuntimeParams( [ - '--connection-string', aqmp_url, + '--connection-string', connection_details.sbus_connection_string, '--name', 'dlq', '--subscription', 'dlq_replay' ] ) replayer = DLQReplayHandler( f'{runtime.dlq_topic_name}/Subscriptions/{runtime.subscription}', - aqmp_url, + connection_details, 5, logger ) @@ -77,10 +77,10 @@ def _(aqmp_url: str): @when('the input message is sent') -def _(aqmp_url: str, input_topic: str, output_topic: str, message_body: str): +def _(connection_details: ConnectionStringHelper, input_topic: str, output_topic: str, message_body: str): """the input message is sent.""" if output_topic == 'N/A': - conn = BlockingConnection(aqmp_url) + conn = BlockingConnection(connection_details.amqp_url()) sender = conn.create_sender(input_topic) sender.send(Message(body=message_body)) pytest.skip(f'Output topic is "{output_topic}".') @@ -128,9 +128,9 @@ def is_message_valid(message: Message, expected_body: str, topic_name: str) -> b @then('the expected output message is received') -def _(aqmp_url: str, input_topic: str, output_topic: str, message_body: str): +def _(connection_details: ConnectionStringHelper, input_topic: str, output_topic: str, message_body: str): """The expected output message is received.""" - conn = BlockingConnection(aqmp_url) + conn = BlockingConnection(connection_details.amqp_url()) receiver = conn.create_receiver(f'{output_topic}/Subscriptions/test') logger.debug(f'Receiver for "{output_topic}" created at {time.time()}.') From 1f97eb41207aea4ea43585243ffaaade33784e90 Mon Sep 17 00:00:00 2001 From: CBDQ Bot Account <136103132+cbdqbot@users.noreply.github.com> Date: Mon, 6 Jan 2025 09:00:17 +0000 Subject: [PATCH 6/6] chg: dev: Update change log. --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d200dbf..38c5c2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,15 @@ # Changelog +## $( python -c 'import router; print(router.__version__)' ) + +### Fix + +* Handle non-URL compatible characters in namespace connection strings. [Ben Dalling] + +* Stop false negative security alert. [Ben Dalling] + + ## 0.2.1 (2024-12-31) ### Fix