From a56e2ba7023a5ea433092ce45a51da05800b6d97 Mon Sep 17 00:00:00 2001 From: Jhonathan Abreu Date: Tue, 3 Nov 2026 13:50:15 -0400 Subject: [PATCH 1/8] feature: sanitize object store keys in the cloud object-store commands --- README.md | 18 ++-- lean/commands/cloud/object_store/delete.py | 4 +- lean/commands/cloud/object_store/get.py | 4 +- lean/commands/cloud/object_store/list.py | 4 +- .../commands/cloud/object_store/properties.py | 4 +- lean/commands/cloud/object_store/set.py | 4 +- lean/components/util/object_store_helper.py | 98 +++++++++++++++++++ .../cloud/object_store/test_delete.py | 12 +++ tests/commands/cloud/object_store/test_get.py | 21 ++++ .../commands/cloud/object_store/test_list.py | 13 +++ .../cloud/object_store/test_properties.py | 50 ++++++++++ tests/commands/cloud/object_store/test_set.py | 30 ++++++ .../util/test_object_store_helper.py | 86 ++++++++++++++++ 13 files changed, 337 insertions(+), 11 deletions(-) create mode 100644 tests/commands/cloud/object_store/test_properties.py create mode 100644 tests/components/util/test_object_store_helper.py diff --git a/README.md b/README.md index a11715de1..303db1488 100644 --- a/README.md +++ b/README.md @@ -619,7 +619,8 @@ Usage: lean cloud object-store delete [OPTIONS] KEY Delete a value from the organization's cloud object store. - :param key: The desired key name to delete. + :param key: The desired key name to delete. A key that does not follow the object store key format is sanitized, with + a warning. Options: --verbose Enable debug logging @@ -637,7 +638,8 @@ Usage: lean cloud object-store get [OPTIONS] [KEY]... Download an object store value to disk from the organization's cloud object store. - :param key: The desired key to fetch, multiple can be provided. + :param key: The desired key to fetch, multiple can be provided. A key that does not follow the object store key format + is sanitized, with a warning. Options: --destination-folder TEXT The destination folder to download the object store values, if not provided will use to @@ -657,7 +659,8 @@ Usage: lean cloud object-store list [OPTIONS] [KEY] List all values for the given root key in the organization's cloud object store. - :param key: The desired root key to list. + :param key: The desired root key to list. A key that does not follow the object store key format is sanitized, with a + warning. Options: --verbose Enable debug logging @@ -675,7 +678,8 @@ Usage: lean cloud object-store ls [OPTIONS] [KEY] List all values for the given root key in the organization's cloud object store. - :param key: The desired root key to list. + :param key: The desired root key to list. A key that does not follow the object store key format is sanitized, with a + warning. Options: --verbose Enable debug logging @@ -693,7 +697,8 @@ Usage: lean cloud object-store properties [OPTIONS] KEY Get a value properties from the organization's cloud object store. - :param key: The desired key to fetch the properties for. + :param key: The desired key to fetch the properties for. A key that does not follow the object store key format is + sanitized, with a warning. Options: --verbose Enable debug logging @@ -711,7 +716,8 @@ Usage: lean cloud object-store set [OPTIONS] KEY PATH Sets the data to the given key in the organization's cloud object store. - :param key: The key to set the data to. :param path: Path to the file containing the object data. + :param key: The key to set the data to. A key that does not follow the object store key format is sanitized, with a + warning. :param path: Path to the file containing the object data. Options: --verbose Enable debug logging diff --git a/lean/commands/cloud/object_store/delete.py b/lean/commands/cloud/object_store/delete.py index fa67d9ffe..c4b2fe705 100644 --- a/lean/commands/cloud/object_store/delete.py +++ b/lean/commands/cloud/object_store/delete.py @@ -15,6 +15,7 @@ from lean.click import LeanCommand from lean.container import container +from lean.components.util.object_store_helper import resolve_object_store_key @command(cls=LeanCommand) @@ -23,8 +24,9 @@ def delete(key: str) -> str: """ Delete a value from the organization's cloud object store. - :param key: The desired key name to delete. + :param key: The desired key name to delete. A key that does not follow the object store key format is sanitized, with a warning. """ + key = resolve_object_store_key(key, container.logger) organization_id = container.organization_manager.try_get_working_organization_id() api_client = container.api_client api_client.object_store.delete(key, organization_id) diff --git a/lean/commands/cloud/object_store/get.py b/lean/commands/cloud/object_store/get.py index b2a034774..242bbc8ae 100644 --- a/lean/commands/cloud/object_store/get.py +++ b/lean/commands/cloud/object_store/get.py @@ -17,6 +17,7 @@ from click import command, option, argument from lean.click import LeanCommand from lean.container import container +from lean.components.util.object_store_helper import resolve_object_store_key @command(cls=LeanCommand) @@ -28,11 +29,12 @@ def get(key: [str], destination_folder: str): """ Download an object store value to disk from the organization's cloud object store. - :param key: The desired key to fetch, multiple can be provided. + :param key: The desired key to fetch, multiple can be provided. A key that does not follow the object store key format is sanitized, with a warning. """ organization_id = container.organization_manager.try_get_working_organization_id() api_client = container.api_client logger = container.logger + key = tuple(resolve_object_store_key(k, logger) for k in key) logger.info(f"Fetching object store download url") url = api_client.object_store.get(key, organization_id, logger) diff --git a/lean/commands/cloud/object_store/list.py b/lean/commands/cloud/object_store/list.py index 5df3c99bd..0401f53ba 100644 --- a/lean/commands/cloud/object_store/list.py +++ b/lean/commands/cloud/object_store/list.py @@ -15,6 +15,7 @@ from lean.commands.cloud.object_store import object_store from lean.click import LeanCommand from lean.container import container +from lean.components.util.object_store_helper import resolve_object_store_key @object_store.command(cls=LeanCommand, name="list", aliases=["ls"]) @@ -23,11 +24,12 @@ def list(key: str): """ List all values for the given root key in the organization's cloud object store. - :param key: The desired root key to list. + :param key: The desired root key to list. A key that does not follow the object store key format is sanitized, with a warning. """ organization_id = container.organization_manager.try_get_working_organization_id() api_client = container.api_client logger = container.logger + key = resolve_object_store_key(key, logger) data = api_client.object_store.list(key, organization_id) try: diff --git a/lean/commands/cloud/object_store/properties.py b/lean/commands/cloud/object_store/properties.py index 3e42ac57f..fb6d91981 100644 --- a/lean/commands/cloud/object_store/properties.py +++ b/lean/commands/cloud/object_store/properties.py @@ -14,6 +14,7 @@ from click import command, argument from lean.click import LeanCommand from lean.container import container +from lean.components.util.object_store_helper import resolve_object_store_key @command(cls=LeanCommand) @@ -22,11 +23,12 @@ def properties(key: str): """ Get a value properties from the organization's cloud object store. - :param key: The desired key to fetch the properties for. + :param key: The desired key to fetch the properties for. A key that does not follow the object store key format is sanitized, with a warning. """ organization_id = container.organization_manager.try_get_working_organization_id() api_client = container.api_client logger = container.logger + key = resolve_object_store_key(key, logger) data = api_client.object_store.properties(key, organization_id) try: diff --git a/lean/commands/cloud/object_store/set.py b/lean/commands/cloud/object_store/set.py index 5794af051..6bf3be779 100644 --- a/lean/commands/cloud/object_store/set.py +++ b/lean/commands/cloud/object_store/set.py @@ -15,6 +15,7 @@ from pathlib import Path from lean.click import LeanCommand, PathParameter from lean.container import container +from lean.components.util.object_store_helper import resolve_object_store_key @command(cls=LeanCommand) @@ -23,9 +24,10 @@ def set(key: str, path: Path) -> None: """Sets the data to the given key in the organization's cloud object store. - :param key: The key to set the data to. + :param key: The key to set the data to. A key that does not follow the object store key format is sanitized, with a warning. :param path: Path to the file containing the object data. """ + key = resolve_object_store_key(key, container.logger) organization_id = container.organization_manager.try_get_working_organization_id() container.logger.info(f"Setting object {key} in organization {organization_id}") api_client = container.api_client diff --git a/lean/components/util/object_store_helper.py b/lean/components/util/object_store_helper.py index 549f0865f..08b0797ad 100644 --- a/lean/components/util/object_store_helper.py +++ b/lean/components/util/object_store_helper.py @@ -11,7 +11,105 @@ # See the License for the specific language governing permissions and # limitations under the License. +import re +from typing import List, Optional + from lean.components.config.lean_config_manager import LeanConfigManager +from lean.components.util.logger import Logger + +# The key rules of Lean's ObjectStore, so a key is stored under the name the engine would use for it +_SUPPORTED_KEY = re.compile(r"\.?[a-zA-Z0-9\\/_#\-$= ]+\.?[a-zA-Z0-9]*") +_UNSUPPORTED_CHARACTERS = re.compile(r"[^a-zA-Z0-9\\/_#\-$= .]") +_EXTENSION = re.compile(r"[a-zA-Z0-9]+") +_KEY_RULES = "Keys may only contain english letters, numbers, spaces and the characters '/', '\\', '_', '#', '-', '$' and '=', " \ + "plus at most one '.' followed by a letters-and-numbers-only extension, e.g. 'folder/trade_log-2024.csv'" + + +def resolve_object_store_key(key: str, logger: Logger) -> str: + """Returns the key the object store is asked for: the given one when it follows the key format, else its + sanitized version, with a warning stating the problems found and the key used instead. + + :param key: the key as given by the user + :param logger: the logger to warn with + :return: the key to send to the object store + """ + issues = get_key_issues(key) + if issues is None: + return key + + sanitized = sanitize_key(key) + logger.warn(f"Key '{key}' is not supported: {issues}. Using '{sanitized}' instead") + return sanitized + + +def get_key_issues(key: str) -> Optional[str]: + """Describes why a key does not follow the key format: the problems found and the rules. + + :param key: the key to check + :return: the issues with the key, or None when it is supported + """ + if _is_supported_key(key): + return None + return "; ".join(_find_key_issues(key)) + ". " + _KEY_RULES + + +def sanitize_key(key: str) -> str: + """Converts an unsupported key into a supported one by replacing every unsupported character with '_', + keeping at most the final '.extension': 'trade log: AI & Cloud.csv' becomes 'trade log_ AI _ Cloud.csv'. + + :param key: the key to sanitize + :return: a key following the key format + """ + sanitized = _UNSUPPORTED_CHARACTERS.sub("_", key) + + # a single trailing '.extension' of letters and numbers is allowed: keep the last dot if it + # introduces a valid extension and replace every other dot + last_dot = sanitized.rfind(".") + if last_dot > 0 and _EXTENSION.fullmatch(sanitized[last_dot + 1:]) is not None: + sanitized = sanitized[:last_dot].replace(".", "_") + sanitized[last_dot:] + elif last_dot >= 0: + sanitized = sanitized.replace(".", "_") + + issues = get_key_issues(sanitized) + if issues is not None: + raise RuntimeError(f"Key '{key}' is not supported: {issues}") + return sanitized + + +def _is_supported_key(key: str) -> bool: + return bool(key) and _SUPPORTED_KEY.fullmatch(key) is not None and key.count("/") <= 100 and key.count("\\") <= 100 + + +def _find_key_issues(key: str) -> List[str]: + """Lists why a key is not supported, one entry per problem found.""" + if not key: + return ["the key is empty"] + + issues = [] + unsupported = list(dict.fromkeys(f"'{char}'" for char in _UNSUPPORTED_CHARACTERS.findall(key))) + if unsupported: + issues.append(f"unsupported character{'s' if len(unsupported) > 1 else ''} {', '.join(unsupported)}") + + # a single leading '.' is allowed, every other '.' must introduce the final extension + name = key.lstrip(".") + dots = name.count(".") + if dots > 1: + issues.append(f"it has {dots} '.' but at most one '.extension' is allowed") + elif dots == 1: + extension = name[name.index(".") + 1:] + if extension and _EXTENSION.fullmatch(extension) is None: + issues.append(f"the extension '{extension}' may only contain letters and numbers") + if not name: + issues.append("it has no name before the '.'") + + if key.count("/") > 100 or key.count("\\") > 100: + issues.append("it has more than 100 '/' or '\\' separators") + + if not issues: + issues.append("it does not follow the key format") + return issues + + def open_storage_directory_in_explorer(lean_config_manager: LeanConfigManager): """Opens the storage directory in the file explorer.""" diff --git a/tests/commands/cloud/object_store/test_delete.py b/tests/commands/cloud/object_store/test_delete.py index 76ea1a1db..90a3ae84f 100644 --- a/tests/commands/cloud/object_store/test_delete.py +++ b/tests/commands/cloud/object_store/test_delete.py @@ -28,3 +28,15 @@ def test_delete_deletes_value_when_key_is_given() -> None: result = CliRunner().invoke(lean, ["cloud", "object-store", "delete", "test-key"]) assert result.exit_code == 0 container.api_client.object_store.delete.assert_called_once_with('test-key', 'abc') + + +def test_delete_sanitizes_an_unsupported_key_with_a_warning() -> None: + api_client = mock.Mock() + api_client.is_authenticated.return_value = True + initialize_container(api_client_to_use=api_client) + + result = CliRunner().invoke(lean, ["cloud", "object-store", "delete", "trade log: AI & Cloud.csv"]) + + assert result.exit_code == 0 + assert "Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. Keys may only contain" in result.output.replace("\n", "") + container.api_client.object_store.delete.assert_called_once_with('trade log_ AI _ Cloud.csv', 'abc') diff --git a/tests/commands/cloud/object_store/test_get.py b/tests/commands/cloud/object_store/test_get.py index bd5b0a4c5..9b80067b0 100644 --- a/tests/commands/cloud/object_store/test_get.py +++ b/tests/commands/cloud/object_store/test_get.py @@ -36,3 +36,24 @@ def test_get_gets_value_when_key_is_given(mock_zipfile) -> None: result = CliRunner().invoke(lean, ["cloud", "object-store", "get", "test-key"]) assert result.exit_code == 0 container.api_client.object_store.get.assert_called_once_with(('test-key',), 'abc', container.logger) + + +@mock.patch("zipfile.ZipFile") +def test_get_sanitizes_every_unsupported_key_with_a_warning(mock_zipfile) -> None: + archive = mock.Mock() + archive.return_value.read = mock.Mock() + mock_zipfile.return_value.__enter__ = archive + + api_client = mock.Mock() + api_client.is_authenticated.return_value = True + initialize_container(api_client_to_use=api_client) + + result = CliRunner().invoke(lean, ["cloud", "object-store", "get", "trade log: AI & Cloud.csv", "model.v2.pkl", "good.csv"]) + + assert result.exit_code == 0 + output = result.output.replace("\n", "") + assert "Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. Keys may only contain" in output + assert "Key 'model.v2.pkl' is not supported: it has 2 '.' but at most one '.extension' is allowed." in output + assert "Key 'good.csv'" not in output + container.api_client.object_store.get.assert_called_once_with( + ('trade log_ AI _ Cloud.csv', 'model_v2.pkl', 'good.csv'), 'abc', container.logger) diff --git a/tests/commands/cloud/object_store/test_list.py b/tests/commands/cloud/object_store/test_list.py index 87dadfea1..8c83aee8a 100644 --- a/tests/commands/cloud/object_store/test_list.py +++ b/tests/commands/cloud/object_store/test_list.py @@ -48,3 +48,16 @@ def test_list_handles_empty_or_none_objects(objects_value) -> None: container.api_client.object_store.list.assert_called_once_with('test', 'abc') + + +def test_list_sanitizes_an_unsupported_root_key_with_a_warning() -> None: + api_client = mock.Mock() + api_client.is_authenticated.return_value = True + api_client.object_store.list.return_value = {'objects': [], 'success': True, 'path': 'trade log_'} + initialize_container(api_client_to_use=api_client) + + result = CliRunner().invoke(lean, ["cloud", "object-store", "list", "trade log:"]) + + assert result.exit_code == 0 + assert "Key 'trade log:' is not supported: unsupported character ':'." in result.output.replace("\n", "") + container.api_client.object_store.list.assert_called_once_with('trade log_', 'abc') diff --git a/tests/commands/cloud/object_store/test_properties.py b/tests/commands/cloud/object_store/test_properties.py new file mode 100644 index 000000000..15bdbfd3b --- /dev/null +++ b/tests/commands/cloud/object_store/test_properties.py @@ -0,0 +1,50 @@ +# QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. +# Lean CLI v1.0. Copyright 2021 QuantConnect Corporation. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from unittest import mock + +from click.testing import CliRunner + +from lean.commands import lean +from lean.container import container +from tests.conftest import initialize_container + + +def test_properties_gets_value_when_key_is_given() -> None: + api_client = mock.Mock() + api_client.is_authenticated.return_value = True + api_client.object_store.properties.return_value = { + 'metadata': {'size': 10, 'modified': '2024-01-01', 'key': 'test-key', 'preview': 'abc'} + } + initialize_container(api_client_to_use=api_client) + + result = CliRunner().invoke(lean, ["cloud", "object-store", "properties", "test-key"]) + + assert result.exit_code == 0 + assert "test-key" in result.output + container.api_client.object_store.properties.assert_called_once_with('test-key', 'abc') + + +def test_properties_sanitizes_an_unsupported_key_with_a_warning() -> None: + api_client = mock.Mock() + api_client.is_authenticated.return_value = True + api_client.object_store.properties.return_value = { + 'metadata': {'size': 10, 'modified': '2024-01-01', 'key': 'trade log_ AI _ Cloud.csv', 'preview': 'abc'} + } + initialize_container(api_client_to_use=api_client) + + result = CliRunner().invoke(lean, ["cloud", "object-store", "properties", "trade log: AI & Cloud.csv"]) + + assert result.exit_code == 0 + assert "Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. Keys may only contain" in result.output.replace("\n", "") + container.api_client.object_store.properties.assert_called_once_with('trade log_ AI _ Cloud.csv', 'abc') diff --git a/tests/commands/cloud/object_store/test_set.py b/tests/commands/cloud/object_store/test_set.py index f282c14bc..6277ac312 100644 --- a/tests/commands/cloud/object_store/test_set.py +++ b/tests/commands/cloud/object_store/test_set.py @@ -35,3 +35,33 @@ def test_set_sets_value_when_path_is_given() -> None: with open(file_path, "rb") as file: bytes_data: bytes = file.read() container.api_client.object_store.set.assert_called_once_with('test-key', bytes_data, 'abc') + + +def test_set_sanitizes_an_unsupported_key_with_a_warning() -> None: + create_fake_lean_cli_project("test", "python") + api_client = mock.Mock() + api_client.is_authenticated.return_value = True + initialize_container(api_client_to_use=api_client) + file_path = str(Path.joinpath(Path.cwd(), "lean.json")) + + result = CliRunner().invoke(lean, ["cloud", "object-store", "set", "trade log: AI & Cloud.csv", file_path]) + + assert result.exit_code == 0 + assert "Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. Keys may only contain" in result.output.replace("\n", "") + assert "Using 'trade log_ AI _ Cloud.csv' instead" in result.output.replace("\n", "") + with open(file_path, "rb") as file: + container.api_client.object_store.set.assert_called_once_with('trade log_ AI _ Cloud.csv', file.read(), 'abc') + + +def test_set_keeps_a_supported_key_silently() -> None: + create_fake_lean_cli_project("test", "python") + api_client = mock.Mock() + api_client.is_authenticated.return_value = True + initialize_container(api_client_to_use=api_client) + file_path = str(Path.joinpath(Path.cwd(), "lean.json")) + + result = CliRunner().invoke(lean, ["cloud", "object-store", "set", "folder/trade_log-2024.csv", file_path]) + + assert result.exit_code == 0 + assert "is not supported" not in result.output + container.api_client.object_store.set.assert_called_once_with('folder/trade_log-2024.csv', mock.ANY, 'abc') diff --git a/tests/components/util/test_object_store_helper.py b/tests/components/util/test_object_store_helper.py new file mode 100644 index 000000000..2adce3609 --- /dev/null +++ b/tests/components/util/test_object_store_helper.py @@ -0,0 +1,86 @@ +# QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. +# Lean CLI v1.0. Copyright 2021 QuantConnect Corporation. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from unittest import mock + +import pytest + +from lean.components.util.object_store_helper import get_key_issues, resolve_object_store_key, sanitize_key + +RULES = "Keys may only contain english letters, numbers, spaces and the characters '/', '\\', '_', '#', '-', '$' and '=', " \ + "plus at most one '.' followed by a letters-and-numbers-only extension, e.g. 'folder/trade_log-2024.csv'" + + +@pytest.mark.parametrize("key", ["my_key", "./my_key/nested\\file.csv", "file with spaces #1-$=.txt", "folder/trade_log-2024.csv", "/"]) +def test_get_key_issues_returns_none_for_a_supported_key(key: str) -> None: + assert get_key_issues(key) is None + + +@pytest.mark.parametrize("key,expected_issue", [ + ("trade log: AI & Cloud.csv", "unsupported characters ':', '&'"), + ("résumé.pdf", "unsupported character 'é'"), + ("file.tar.gz", "it has 2 '.' but at most one '.extension' is allowed"), + ("file.name_v2", "the extension 'name_v2' may only contain letters and numbers"), + ("folder.v1/file", "the extension 'v1/file' may only contain letters and numbers"), + ("trade log: AI.v2.csv", "unsupported character ':'; it has 2 '.' but at most one '.extension' is allowed"), + (".", "it has no name before the '.'"), + ("", "the key is empty"), + ("a" + "/" * 101, "it has more than 100 '/' or '\\' separators"), +]) +def test_get_key_issues_states_the_issue_and_the_rules(key: str, expected_issue: str) -> None: + assert get_key_issues(key) == f"{expected_issue}. {RULES}" + + +@pytest.mark.parametrize("key,expected", [ + ("trade_log_ai_hardware_&_cloud.csv", "trade_log_ai_hardware___cloud.csv"), + ("trade log: AI & Cloud.csv", "trade log_ AI _ Cloud.csv"), + ("file.tar.gz", "file_tar.gz"), + ("résumé.pdf", "r_sum_.pdf"), + ("a..", "a__"), + ("100%.json", "100_.json"), + ("../prefix/file", "__/prefix/file"), + (".", "_"), +]) +def test_sanitize_key_returns_a_supported_key(key: str, expected: str) -> None: + sanitized = sanitize_key(key) + + assert sanitized == expected + assert get_key_issues(sanitized) is None + + +@pytest.mark.parametrize("key", ["", "a" + "/" * 101]) +def test_sanitize_key_raises_when_the_key_cannot_be_fixed(key: str) -> None: + with pytest.raises(RuntimeError, match="is not supported"): + sanitize_key(key) + + +def test_sanitize_key_is_lossy_and_distinct_keys_can_collide() -> None: + assert sanitize_key("trade_log_ai&cloud.csv") == sanitize_key("trade_log_ai:cloud.csv") == "trade_log_ai_cloud.csv" + + +def test_resolve_object_store_key_returns_a_supported_key_without_warning() -> None: + logger = mock.Mock() + + assert resolve_object_store_key("folder/trade_log-2024.csv", logger) == "folder/trade_log-2024.csv" + + logger.warn.assert_not_called() + + +def test_resolve_object_store_key_sanitizes_an_unsupported_key_with_a_warning() -> None: + logger = mock.Mock() + + assert resolve_object_store_key("trade log: AI & Cloud.csv", logger) == "trade log_ AI _ Cloud.csv" + + logger.warn.assert_called_once_with( + f"Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. {RULES}. " + "Using 'trade log_ AI _ Cloud.csv' instead") From a039b204dd8d5aabf646af5d8ce16a6f535ae5cc Mon Sep 17 00:00:00 2001 From: Jhonathan Abreu Date: Tue, 3 Nov 2026 15:04:12 -0400 Subject: [PATCH 2/8] fix: send keys as typed on get, delete, list and properties, warn before set overwrites --- README.md | 18 +++----- lean/commands/cloud/object_store/delete.py | 4 +- lean/commands/cloud/object_store/get.py | 4 +- lean/commands/cloud/object_store/list.py | 4 +- .../commands/cloud/object_store/properties.py | 4 +- lean/commands/cloud/object_store/set.py | 8 ++-- lean/components/api/object_store_client.py | 15 +++++++ lean/components/util/object_store_helper.py | 15 ++++--- .../cloud/object_store/test_delete.py | 9 ++-- tests/commands/cloud/object_store/test_get.py | 13 +++--- .../commands/cloud/object_store/test_list.py | 9 ++-- .../cloud/object_store/test_properties.py | 11 ++--- tests/commands/cloud/object_store/test_set.py | 23 +++++++++- .../api/test_object_store_client.py | 43 +++++++++++++++++++ .../util/test_object_store_helper.py | 22 +++++++--- 15 files changed, 142 insertions(+), 60 deletions(-) create mode 100644 tests/components/api/test_object_store_client.py diff --git a/README.md b/README.md index 303db1488..b345bb72d 100644 --- a/README.md +++ b/README.md @@ -619,8 +619,7 @@ Usage: lean cloud object-store delete [OPTIONS] KEY Delete a value from the organization's cloud object store. - :param key: The desired key name to delete. A key that does not follow the object store key format is sanitized, with - a warning. + :param key: The desired key name to delete. Options: --verbose Enable debug logging @@ -638,8 +637,7 @@ Usage: lean cloud object-store get [OPTIONS] [KEY]... Download an object store value to disk from the organization's cloud object store. - :param key: The desired key to fetch, multiple can be provided. A key that does not follow the object store key format - is sanitized, with a warning. + :param key: The desired key to fetch, multiple can be provided. Options: --destination-folder TEXT The destination folder to download the object store values, if not provided will use to @@ -659,8 +657,7 @@ Usage: lean cloud object-store list [OPTIONS] [KEY] List all values for the given root key in the organization's cloud object store. - :param key: The desired root key to list. A key that does not follow the object store key format is sanitized, with a - warning. + :param key: The desired root key to list. Options: --verbose Enable debug logging @@ -678,8 +675,7 @@ Usage: lean cloud object-store ls [OPTIONS] [KEY] List all values for the given root key in the organization's cloud object store. - :param key: The desired root key to list. A key that does not follow the object store key format is sanitized, with a - warning. + :param key: The desired root key to list. Options: --verbose Enable debug logging @@ -697,8 +693,7 @@ Usage: lean cloud object-store properties [OPTIONS] KEY Get a value properties from the organization's cloud object store. - :param key: The desired key to fetch the properties for. A key that does not follow the object store key format is - sanitized, with a warning. + :param key: The desired key to fetch the properties for. Options: --verbose Enable debug logging @@ -717,7 +712,8 @@ Usage: lean cloud object-store set [OPTIONS] KEY PATH Sets the data to the given key in the organization's cloud object store. :param key: The key to set the data to. A key that does not follow the object store key format is sanitized, with a - warning. :param path: Path to the file containing the object data. + warning naming the key to use from then on, also when that overwrites an existing object. :param path: Path to the + file containing the object data. Options: --verbose Enable debug logging diff --git a/lean/commands/cloud/object_store/delete.py b/lean/commands/cloud/object_store/delete.py index c4b2fe705..fa67d9ffe 100644 --- a/lean/commands/cloud/object_store/delete.py +++ b/lean/commands/cloud/object_store/delete.py @@ -15,7 +15,6 @@ from lean.click import LeanCommand from lean.container import container -from lean.components.util.object_store_helper import resolve_object_store_key @command(cls=LeanCommand) @@ -24,9 +23,8 @@ def delete(key: str) -> str: """ Delete a value from the organization's cloud object store. - :param key: The desired key name to delete. A key that does not follow the object store key format is sanitized, with a warning. + :param key: The desired key name to delete. """ - key = resolve_object_store_key(key, container.logger) organization_id = container.organization_manager.try_get_working_organization_id() api_client = container.api_client api_client.object_store.delete(key, organization_id) diff --git a/lean/commands/cloud/object_store/get.py b/lean/commands/cloud/object_store/get.py index 242bbc8ae..b2a034774 100644 --- a/lean/commands/cloud/object_store/get.py +++ b/lean/commands/cloud/object_store/get.py @@ -17,7 +17,6 @@ from click import command, option, argument from lean.click import LeanCommand from lean.container import container -from lean.components.util.object_store_helper import resolve_object_store_key @command(cls=LeanCommand) @@ -29,12 +28,11 @@ def get(key: [str], destination_folder: str): """ Download an object store value to disk from the organization's cloud object store. - :param key: The desired key to fetch, multiple can be provided. A key that does not follow the object store key format is sanitized, with a warning. + :param key: The desired key to fetch, multiple can be provided. """ organization_id = container.organization_manager.try_get_working_organization_id() api_client = container.api_client logger = container.logger - key = tuple(resolve_object_store_key(k, logger) for k in key) logger.info(f"Fetching object store download url") url = api_client.object_store.get(key, organization_id, logger) diff --git a/lean/commands/cloud/object_store/list.py b/lean/commands/cloud/object_store/list.py index 0401f53ba..5df3c99bd 100644 --- a/lean/commands/cloud/object_store/list.py +++ b/lean/commands/cloud/object_store/list.py @@ -15,7 +15,6 @@ from lean.commands.cloud.object_store import object_store from lean.click import LeanCommand from lean.container import container -from lean.components.util.object_store_helper import resolve_object_store_key @object_store.command(cls=LeanCommand, name="list", aliases=["ls"]) @@ -24,12 +23,11 @@ def list(key: str): """ List all values for the given root key in the organization's cloud object store. - :param key: The desired root key to list. A key that does not follow the object store key format is sanitized, with a warning. + :param key: The desired root key to list. """ organization_id = container.organization_manager.try_get_working_organization_id() api_client = container.api_client logger = container.logger - key = resolve_object_store_key(key, logger) data = api_client.object_store.list(key, organization_id) try: diff --git a/lean/commands/cloud/object_store/properties.py b/lean/commands/cloud/object_store/properties.py index fb6d91981..3e42ac57f 100644 --- a/lean/commands/cloud/object_store/properties.py +++ b/lean/commands/cloud/object_store/properties.py @@ -14,7 +14,6 @@ from click import command, argument from lean.click import LeanCommand from lean.container import container -from lean.components.util.object_store_helper import resolve_object_store_key @command(cls=LeanCommand) @@ -23,12 +22,11 @@ def properties(key: str): """ Get a value properties from the organization's cloud object store. - :param key: The desired key to fetch the properties for. A key that does not follow the object store key format is sanitized, with a warning. + :param key: The desired key to fetch the properties for. """ organization_id = container.organization_manager.try_get_working_organization_id() api_client = container.api_client logger = container.logger - key = resolve_object_store_key(key, logger) data = api_client.object_store.properties(key, organization_id) try: diff --git a/lean/commands/cloud/object_store/set.py b/lean/commands/cloud/object_store/set.py index 6bf3be779..7e5bd6dd9 100644 --- a/lean/commands/cloud/object_store/set.py +++ b/lean/commands/cloud/object_store/set.py @@ -15,7 +15,7 @@ from pathlib import Path from lean.click import LeanCommand, PathParameter from lean.container import container -from lean.components.util.object_store_helper import resolve_object_store_key +from lean.components.util.object_store_helper import key_to_save @command(cls=LeanCommand) @@ -24,13 +24,13 @@ def set(key: str, path: Path) -> None: """Sets the data to the given key in the organization's cloud object store. - :param key: The key to set the data to. A key that does not follow the object store key format is sanitized, with a warning. + :param key: The key to set the data to. A key that does not follow the object store key format is sanitized, with a warning naming the key to use from then on, also when that overwrites an existing object. :param path: Path to the file containing the object data. """ - key = resolve_object_store_key(key, container.logger) organization_id = container.organization_manager.try_get_working_organization_id() - container.logger.info(f"Setting object {key} in organization {organization_id}") api_client = container.api_client + key = key_to_save(key, lambda k: api_client.object_store.exists(k, organization_id), container.logger) + container.logger.info(f"Setting object {key} in organization {organization_id}") with open(path, "rb") as file: bytes_data: bytes = file.read() api_client.object_store.set(key, bytes_data, organization_id) diff --git a/lean/components/api/object_store_client.py b/lean/components/api/object_store_client.py index 76cd6bb5f..1932659c5 100644 --- a/lean/components/api/object_store_client.py +++ b/lean/components/api/object_store_client.py @@ -13,6 +13,7 @@ from lean.components.api.api_client import * from lean.components.util.logger import Logger +from lean.models.errors import RequestFailedError class ObjectStoreClient: @@ -38,6 +39,20 @@ def properties(self, key: str, organization_id: str) -> str: return self._api.post("object/properties", payload) + def exists(self, key: str, organization_id: str) -> bool: + """Returns whether an object is stored under the given key, as typed. + :param key: key of the object to look for + :param organization_id: the id of the organization who's object store to look in + :return: True if the object exists + """ + try: + self.properties(key, organization_id) + return True + except RequestFailedError as error: + if "not found" in str(error).lower(): + return False + raise + def get(self, keys: [str], organization_id: str, logger: Logger) -> str: """Will fetch an url to download the requested keys :param keys: keys of the object to retrieve diff --git a/lean/components/util/object_store_helper.py b/lean/components/util/object_store_helper.py index 08b0797ad..378b63e57 100644 --- a/lean/components/util/object_store_helper.py +++ b/lean/components/util/object_store_helper.py @@ -12,12 +12,12 @@ # limitations under the License. import re -from typing import List, Optional +from typing import Callable, List, Optional from lean.components.config.lean_config_manager import LeanConfigManager from lean.components.util.logger import Logger -# The key rules of Lean's ObjectStore, so a key is stored under the name the engine would use for it +# The key rules of LEAN's ObjectStore (QuantConnect/Lean#9905), mirrored so a key set here is the one an algorithm saves to _SUPPORTED_KEY = re.compile(r"\.?[a-zA-Z0-9\\/_#\-$= ]+\.?[a-zA-Z0-9]*") _UNSUPPORTED_CHARACTERS = re.compile(r"[^a-zA-Z0-9\\/_#\-$= .]") _EXTENSION = re.compile(r"[a-zA-Z0-9]+") @@ -25,11 +25,12 @@ "plus at most one '.' followed by a letters-and-numbers-only extension, e.g. 'folder/trade_log-2024.csv'" -def resolve_object_store_key(key: str, logger: Logger) -> str: - """Returns the key the object store is asked for: the given one when it follows the key format, else its - sanitized version, with a warning stating the problems found and the key used instead. +def key_to_save(key: str, exists: Callable[[str], bool], logger: Logger) -> str: + """Returns the key a set goes to: the given one when it follows the key format, else its sanitized version, + warning about the rewrite and about the existing object it overwrites, if any. :param key: the key as given by the user + :param exists: whether an object is stored under a key :param logger: the logger to warn with :return: the key to send to the object store """ @@ -39,6 +40,8 @@ def resolve_object_store_key(key: str, logger: Logger) -> str: sanitized = sanitize_key(key) logger.warn(f"Key '{key}' is not supported: {issues}. Using '{sanitized}' instead") + if exists(sanitized): + logger.warn(f"'{sanitized}' already exists and will be overwritten") return sanitized @@ -101,6 +104,8 @@ def _find_key_issues(key: str) -> List[str]: issues.append(f"the extension '{extension}' may only contain letters and numbers") if not name: issues.append("it has no name before the '.'") + elif key.startswith(".."): + issues.append("it starts with more than one '.'") if key.count("/") > 100 or key.count("\\") > 100: issues.append("it has more than 100 '/' or '\\' separators") diff --git a/tests/commands/cloud/object_store/test_delete.py b/tests/commands/cloud/object_store/test_delete.py index 90a3ae84f..5cc0d6ef1 100644 --- a/tests/commands/cloud/object_store/test_delete.py +++ b/tests/commands/cloud/object_store/test_delete.py @@ -30,13 +30,14 @@ def test_delete_deletes_value_when_key_is_given() -> None: container.api_client.object_store.delete.assert_called_once_with('test-key', 'abc') -def test_delete_sanitizes_an_unsupported_key_with_a_warning() -> None: +def test_delete_sends_an_unsupported_key_as_typed() -> None: api_client = mock.Mock() api_client.is_authenticated.return_value = True initialize_container(api_client_to_use=api_client) - result = CliRunner().invoke(lean, ["cloud", "object-store", "delete", "trade log: AI & Cloud.csv"]) + result = CliRunner().invoke(lean, ["cloud", "object-store", "delete", "model.v2.pkl"]) assert result.exit_code == 0 - assert "Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. Keys may only contain" in result.output.replace("\n", "") - container.api_client.object_store.delete.assert_called_once_with('trade log_ AI _ Cloud.csv', 'abc') + assert "is not supported" not in result.output + api_client.object_store.exists.assert_not_called() + container.api_client.object_store.delete.assert_called_once_with('model.v2.pkl', 'abc') diff --git a/tests/commands/cloud/object_store/test_get.py b/tests/commands/cloud/object_store/test_get.py index 9b80067b0..7020e57f3 100644 --- a/tests/commands/cloud/object_store/test_get.py +++ b/tests/commands/cloud/object_store/test_get.py @@ -39,7 +39,7 @@ def test_get_gets_value_when_key_is_given(mock_zipfile) -> None: @mock.patch("zipfile.ZipFile") -def test_get_sanitizes_every_unsupported_key_with_a_warning(mock_zipfile) -> None: +def test_get_sends_unsupported_keys_as_typed(mock_zipfile) -> None: archive = mock.Mock() archive.return_value.read = mock.Mock() mock_zipfile.return_value.__enter__ = archive @@ -48,12 +48,9 @@ def test_get_sanitizes_every_unsupported_key_with_a_warning(mock_zipfile) -> Non api_client.is_authenticated.return_value = True initialize_container(api_client_to_use=api_client) - result = CliRunner().invoke(lean, ["cloud", "object-store", "get", "trade log: AI & Cloud.csv", "model.v2.pkl", "good.csv"]) + result = CliRunner().invoke(lean, ["cloud", "object-store", "get", "trade log: AI & Cloud.csv", "model.v2.pkl"]) assert result.exit_code == 0 - output = result.output.replace("\n", "") - assert "Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. Keys may only contain" in output - assert "Key 'model.v2.pkl' is not supported: it has 2 '.' but at most one '.extension' is allowed." in output - assert "Key 'good.csv'" not in output - container.api_client.object_store.get.assert_called_once_with( - ('trade log_ AI _ Cloud.csv', 'model_v2.pkl', 'good.csv'), 'abc', container.logger) + assert "is not supported" not in result.output + api_client.object_store.exists.assert_not_called() + container.api_client.object_store.get.assert_called_once_with(('trade log: AI & Cloud.csv', 'model.v2.pkl'), 'abc', container.logger) diff --git a/tests/commands/cloud/object_store/test_list.py b/tests/commands/cloud/object_store/test_list.py index 8c83aee8a..73677212e 100644 --- a/tests/commands/cloud/object_store/test_list.py +++ b/tests/commands/cloud/object_store/test_list.py @@ -50,14 +50,15 @@ def test_list_handles_empty_or_none_objects(objects_value) -> None: -def test_list_sanitizes_an_unsupported_root_key_with_a_warning() -> None: +def test_list_sends_an_unsupported_root_key_as_typed() -> None: api_client = mock.Mock() api_client.is_authenticated.return_value = True - api_client.object_store.list.return_value = {'objects': [], 'success': True, 'path': 'trade log_'} + api_client.object_store.list.return_value = {'objects': [], 'success': True, 'path': 'trade log:'} initialize_container(api_client_to_use=api_client) result = CliRunner().invoke(lean, ["cloud", "object-store", "list", "trade log:"]) assert result.exit_code == 0 - assert "Key 'trade log:' is not supported: unsupported character ':'." in result.output.replace("\n", "") - container.api_client.object_store.list.assert_called_once_with('trade log_', 'abc') + assert "is not supported" not in result.output + assert "No objects found at 'trade log:'." in result.output + container.api_client.object_store.list.assert_called_once_with('trade log:', 'abc') diff --git a/tests/commands/cloud/object_store/test_properties.py b/tests/commands/cloud/object_store/test_properties.py index 15bdbfd3b..a30f82009 100644 --- a/tests/commands/cloud/object_store/test_properties.py +++ b/tests/commands/cloud/object_store/test_properties.py @@ -35,16 +35,17 @@ def test_properties_gets_value_when_key_is_given() -> None: container.api_client.object_store.properties.assert_called_once_with('test-key', 'abc') -def test_properties_sanitizes_an_unsupported_key_with_a_warning() -> None: +def test_properties_sends_an_unsupported_key_as_typed() -> None: api_client = mock.Mock() api_client.is_authenticated.return_value = True api_client.object_store.properties.return_value = { - 'metadata': {'size': 10, 'modified': '2024-01-01', 'key': 'trade log_ AI _ Cloud.csv', 'preview': 'abc'} + 'metadata': {'size': 10, 'modified': '2024-01-01', 'key': 'model.v2.pkl', 'preview': 'abc'} } initialize_container(api_client_to_use=api_client) - result = CliRunner().invoke(lean, ["cloud", "object-store", "properties", "trade log: AI & Cloud.csv"]) + result = CliRunner().invoke(lean, ["cloud", "object-store", "properties", "model.v2.pkl"]) assert result.exit_code == 0 - assert "Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. Keys may only contain" in result.output.replace("\n", "") - container.api_client.object_store.properties.assert_called_once_with('trade log_ AI _ Cloud.csv', 'abc') + assert "is not supported" not in result.output + api_client.object_store.exists.assert_not_called() + container.api_client.object_store.properties.assert_called_once_with('model.v2.pkl', 'abc') diff --git a/tests/commands/cloud/object_store/test_set.py b/tests/commands/cloud/object_store/test_set.py index 6277ac312..c323bf588 100644 --- a/tests/commands/cloud/object_store/test_set.py +++ b/tests/commands/cloud/object_store/test_set.py @@ -41,18 +41,36 @@ def test_set_sanitizes_an_unsupported_key_with_a_warning() -> None: create_fake_lean_cli_project("test", "python") api_client = mock.Mock() api_client.is_authenticated.return_value = True + api_client.object_store.exists.return_value = False initialize_container(api_client_to_use=api_client) file_path = str(Path.joinpath(Path.cwd(), "lean.json")) result = CliRunner().invoke(lean, ["cloud", "object-store", "set", "trade log: AI & Cloud.csv", file_path]) assert result.exit_code == 0 - assert "Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. Keys may only contain" in result.output.replace("\n", "") - assert "Using 'trade log_ AI _ Cloud.csv' instead" in result.output.replace("\n", "") + assert "Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. Keys may only contain" in " ".join(result.output.split()) + assert "Using 'trade log_ AI _ Cloud.csv' instead" in " ".join(result.output.split()) + assert "will be overwritten" not in result.output + api_client.object_store.exists.assert_called_once_with('trade log_ AI _ Cloud.csv', 'abc') with open(file_path, "rb") as file: container.api_client.object_store.set.assert_called_once_with('trade log_ AI _ Cloud.csv', file.read(), 'abc') +def test_set_warns_when_the_sanitized_key_overwrites_an_existing_object() -> None: + create_fake_lean_cli_project("test", "python") + api_client = mock.Mock() + api_client.is_authenticated.return_value = True + api_client.object_store.exists.return_value = True + initialize_container(api_client_to_use=api_client) + file_path = str(Path.joinpath(Path.cwd(), "lean.json")) + + result = CliRunner().invoke(lean, ["cloud", "object-store", "set", "report (1).csv", file_path]) + + assert result.exit_code == 0 + assert "'report _1_.csv' already exists and will be overwritten" in " ".join(result.output.split()) + container.api_client.object_store.set.assert_called_once_with('report _1_.csv', mock.ANY, 'abc') + + def test_set_keeps_a_supported_key_silently() -> None: create_fake_lean_cli_project("test", "python") api_client = mock.Mock() @@ -64,4 +82,5 @@ def test_set_keeps_a_supported_key_silently() -> None: assert result.exit_code == 0 assert "is not supported" not in result.output + api_client.object_store.exists.assert_not_called() container.api_client.object_store.set.assert_called_once_with('folder/trade_log-2024.csv', mock.ANY, 'abc') diff --git a/tests/components/api/test_object_store_client.py b/tests/components/api/test_object_store_client.py new file mode 100644 index 000000000..dba6d7796 --- /dev/null +++ b/tests/components/api/test_object_store_client.py @@ -0,0 +1,43 @@ +# QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. +# Lean CLI v1.0. Copyright 2021 QuantConnect Corporation. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from unittest import mock + +import pytest + +from lean.components.api.object_store_client import ObjectStoreClient +from lean.models.errors import RequestFailedError + + +def test_exists_returns_true_when_the_properties_are_returned() -> None: + api = mock.Mock() + api.post.return_value = {"metadata": {"key": "model.v2.pkl"}} + + assert ObjectStoreClient(api).exists("model.v2.pkl", "abc") + + api.post.assert_called_once_with("object/properties", {"organizationId": "abc", "key": "model.v2.pkl"}) + + +def test_exists_returns_false_when_the_object_is_not_found() -> None: + api = mock.Mock() + api.post.side_effect = RequestFailedError(mock.MagicMock(), "File not found") + + assert not ObjectStoreClient(api).exists("model.v2.pkl", "abc") + + +def test_exists_raises_any_other_failure() -> None: + api = mock.Mock() + api.post.side_effect = RequestFailedError(mock.MagicMock(), "Organization not authorized") + + with pytest.raises(RequestFailedError, match="not authorized"): + ObjectStoreClient(api).exists("model.v2.pkl", "abc") diff --git a/tests/components/util/test_object_store_helper.py b/tests/components/util/test_object_store_helper.py index 2adce3609..bd9b15d28 100644 --- a/tests/components/util/test_object_store_helper.py +++ b/tests/components/util/test_object_store_helper.py @@ -15,7 +15,7 @@ import pytest -from lean.components.util.object_store_helper import get_key_issues, resolve_object_store_key, sanitize_key +from lean.components.util.object_store_helper import get_key_issues, key_to_save, sanitize_key RULES = "Keys may only contain english letters, numbers, spaces and the characters '/', '\\', '_', '#', '-', '$' and '=', " \ "plus at most one '.' followed by a letters-and-numbers-only extension, e.g. 'folder/trade_log-2024.csv'" @@ -34,6 +34,7 @@ def test_get_key_issues_returns_none_for_a_supported_key(key: str) -> None: ("folder.v1/file", "the extension 'v1/file' may only contain letters and numbers"), ("trade log: AI.v2.csv", "unsupported character ':'; it has 2 '.' but at most one '.extension' is allowed"), (".", "it has no name before the '.'"), + ("..model.pkl", "it starts with more than one '.'"), ("", "the key is empty"), ("a" + "/" * 101, "it has more than 100 '/' or '\\' separators"), ]) @@ -50,6 +51,7 @@ def test_get_key_issues_states_the_issue_and_the_rules(key: str, expected_issue: ("100%.json", "100_.json"), ("../prefix/file", "__/prefix/file"), (".", "_"), + ("..model.pkl", "__model.pkl"), ]) def test_sanitize_key_returns_a_supported_key(key: str, expected: str) -> None: sanitized = sanitize_key(key) @@ -68,19 +70,29 @@ def test_sanitize_key_is_lossy_and_distinct_keys_can_collide() -> None: assert sanitize_key("trade_log_ai&cloud.csv") == sanitize_key("trade_log_ai:cloud.csv") == "trade_log_ai_cloud.csv" -def test_resolve_object_store_key_returns_a_supported_key_without_warning() -> None: +def test_key_to_save_keeps_a_supported_key_without_checking_the_store() -> None: logger = mock.Mock() + exists = mock.Mock() - assert resolve_object_store_key("folder/trade_log-2024.csv", logger) == "folder/trade_log-2024.csv" + assert key_to_save("folder/trade_log-2024.csv", exists, logger) == "folder/trade_log-2024.csv" + exists.assert_not_called() logger.warn.assert_not_called() -def test_resolve_object_store_key_sanitizes_an_unsupported_key_with_a_warning() -> None: +def test_key_to_save_sanitizes_an_unsupported_key_with_a_warning() -> None: logger = mock.Mock() - assert resolve_object_store_key("trade log: AI & Cloud.csv", logger) == "trade log_ AI _ Cloud.csv" + assert key_to_save("trade log: AI & Cloud.csv", lambda key: False, logger) == "trade log_ AI _ Cloud.csv" logger.warn.assert_called_once_with( f"Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. {RULES}. " "Using 'trade log_ AI _ Cloud.csv' instead") + + +def test_key_to_save_warns_when_the_sanitized_key_overwrites_an_existing_object() -> None: + logger = mock.Mock() + + assert key_to_save("report (1).csv", lambda key: key == "report _1_.csv", logger) == "report _1_.csv" + + assert logger.warn.call_args_list[-1] == mock.call("'report _1_.csv' already exists and will be overwritten") From 176e40c93a75e7dc2a700251b45363cf6cef8893 Mon Sep 17 00:00:00 2001 From: Jhonathan Abreu Date: Tue, 3 Nov 2026 17:07:24 -0400 Subject: [PATCH 3/8] docs: state that only set mirrors LEAN's key rules and pin the api not-found message --- README.md | 4 ++-- lean/commands/cloud/object_store/set.py | 2 +- lean/components/util/object_store_helper.py | 3 ++- tests/components/api/test_object_store_client.py | 5 +++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index b345bb72d..63b8623b0 100644 --- a/README.md +++ b/README.md @@ -712,8 +712,8 @@ Usage: lean cloud object-store set [OPTIONS] KEY PATH Sets the data to the given key in the organization's cloud object store. :param key: The key to set the data to. A key that does not follow the object store key format is sanitized, with a - warning naming the key to use from then on, also when that overwrites an existing object. :param path: Path to the - file containing the object data. + warning naming the key to use from then on, also when that overwrites an existing object. The other object-store + commands take the key as typed. :param path: Path to the file containing the object data. Options: --verbose Enable debug logging diff --git a/lean/commands/cloud/object_store/set.py b/lean/commands/cloud/object_store/set.py index 7e5bd6dd9..74edc4081 100644 --- a/lean/commands/cloud/object_store/set.py +++ b/lean/commands/cloud/object_store/set.py @@ -24,7 +24,7 @@ def set(key: str, path: Path) -> None: """Sets the data to the given key in the organization's cloud object store. - :param key: The key to set the data to. A key that does not follow the object store key format is sanitized, with a warning naming the key to use from then on, also when that overwrites an existing object. + :param key: The key to set the data to. A key that does not follow the object store key format is sanitized, with a warning naming the key to use from then on, also when that overwrites an existing object. The other object-store commands take the key as typed. :param path: Path to the file containing the object data. """ organization_id = container.organization_manager.try_get_working_organization_id() diff --git a/lean/components/util/object_store_helper.py b/lean/components/util/object_store_helper.py index 378b63e57..7d8a28b92 100644 --- a/lean/components/util/object_store_helper.py +++ b/lean/components/util/object_store_helper.py @@ -17,7 +17,8 @@ from lean.components.config.lean_config_manager import LeanConfigManager from lean.components.util.logger import Logger -# The key rules of LEAN's ObjectStore (QuantConnect/Lean#9905), mirrored so a key set here is the one an algorithm saves to +# The key rules of LEAN's ObjectStore (QuantConnect/Lean#9905), mirrored by set only so a key set here is the one an +# algorithm saves to. The other commands send the key as typed; LEAN's reads also fall back to the sanitized key _SUPPORTED_KEY = re.compile(r"\.?[a-zA-Z0-9\\/_#\-$= ]+\.?[a-zA-Z0-9]*") _UNSUPPORTED_CHARACTERS = re.compile(r"[^a-zA-Z0-9\\/_#\-$= .]") _EXTENSION = re.compile(r"[a-zA-Z0-9]+") diff --git a/tests/components/api/test_object_store_client.py b/tests/components/api/test_object_store_client.py index dba6d7796..6e47b5005 100644 --- a/tests/components/api/test_object_store_client.py +++ b/tests/components/api/test_object_store_client.py @@ -28,9 +28,10 @@ def test_exists_returns_true_when_the_properties_are_returned() -> None: api.post.assert_called_once_with("object/properties", {"organizationId": "abc", "key": "model.v2.pkl"}) -def test_exists_returns_false_when_the_object_is_not_found() -> None: +def test_exists_returns_false_on_the_api_not_found_message() -> None: api = mock.Mock() - api.post.side_effect = RequestFailedError(mock.MagicMock(), "File not found") + # the message object/properties answers for a missing key, as the api returns it + api.post.side_effect = RequestFailedError(mock.MagicMock(), "File not found model.v2.pkl") assert not ObjectStoreClient(api).exists("model.v2.pkl", "abc") From d0163d2d597e6ce92f58a5be49b29d96d4b1cacc Mon Sep 17 00:00:00 2001 From: Jhonathan Abreu Date: Tue, 3 Nov 2026 17:26:51 -0400 Subject: [PATCH 4/8] docs: regenerate the README for the Interactive Brokers (FIX) brokerage module --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 63b8623b0..10fe38484 100644 --- a/README.md +++ b/README.md @@ -420,7 +420,7 @@ Usage: lean cloud live deploy [OPTIONS] PROJECT --notify-insights. Options: - --brokerage [Paper Trading|Interactive Brokers|Tradier|Oanda|Bitfinex|Coinbase Advanced Trade|Binance|Zerodha|Samco|Quant Bridge|Trading Technologies|Kraken|CharlesSchwab|Bybit|TradeStation|Alpaca|Tastytrade|Eze|dYdX|Webull|Public|ClearStreet] + --brokerage [Paper Trading|Interactive Brokers|Tradier|Oanda|Bitfinex|Coinbase Advanced Trade|Binance|Zerodha|Samco|Quant Bridge|Trading Technologies|Kraken|CharlesSchwab|Bybit|TradeStation|Alpaca|Tastytrade|Eze|dYdX|Webull|Public|ClearStreet|Interactive Brokers (FIX)] The brokerage to use --data-provider-live [QuantConnect|Interactive Brokers|Tradier|Oanda|Bitfinex|Coinbase Advanced Trade|Binance|Zerodha|Samco|Quant Bridge|Trading Technologies|Kraken|CharlesSchwab|Polygon|CoinApi|Bybit|TradeStation|Alpaca|Tastytrade|Eze|dYdX] The live data provider to use @@ -544,6 +544,8 @@ Options: --clearstreet-access-token TEXT Your Clear Street API access token --clearstreet-account-id TEXT Your Clear Street account id, the numeric id of the account the algorithm trades + --ib-fix-user-name TEXT Your Interactive Brokers user name + --ib-fix-account TEXT Your Interactive Brokers account id --polygon-api-key TEXT Your Polygon.io API Key --polygon-license-type [Individual|Business] Select your Polygon.io subscription plan (Optional). From 71a9c989f62282581cce2aa024f4637692ce0760 Mon Sep 17 00:00:00 2001 From: Jhonathan Abreu Date: Tue, 3 Nov 2026 17:51:24 -0400 Subject: [PATCH 5/8] Revert "docs: regenerate the README for the Interactive Brokers (FIX) brokerage module" This reverts commit d0163d2d597e6ce92f58a5be49b29d96d4b1cacc. --- README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index 10fe38484..63b8623b0 100644 --- a/README.md +++ b/README.md @@ -420,7 +420,7 @@ Usage: lean cloud live deploy [OPTIONS] PROJECT --notify-insights. Options: - --brokerage [Paper Trading|Interactive Brokers|Tradier|Oanda|Bitfinex|Coinbase Advanced Trade|Binance|Zerodha|Samco|Quant Bridge|Trading Technologies|Kraken|CharlesSchwab|Bybit|TradeStation|Alpaca|Tastytrade|Eze|dYdX|Webull|Public|ClearStreet|Interactive Brokers (FIX)] + --brokerage [Paper Trading|Interactive Brokers|Tradier|Oanda|Bitfinex|Coinbase Advanced Trade|Binance|Zerodha|Samco|Quant Bridge|Trading Technologies|Kraken|CharlesSchwab|Bybit|TradeStation|Alpaca|Tastytrade|Eze|dYdX|Webull|Public|ClearStreet] The brokerage to use --data-provider-live [QuantConnect|Interactive Brokers|Tradier|Oanda|Bitfinex|Coinbase Advanced Trade|Binance|Zerodha|Samco|Quant Bridge|Trading Technologies|Kraken|CharlesSchwab|Polygon|CoinApi|Bybit|TradeStation|Alpaca|Tastytrade|Eze|dYdX] The live data provider to use @@ -544,8 +544,6 @@ Options: --clearstreet-access-token TEXT Your Clear Street API access token --clearstreet-account-id TEXT Your Clear Street account id, the numeric id of the account the algorithm trades - --ib-fix-user-name TEXT Your Interactive Brokers user name - --ib-fix-account TEXT Your Interactive Brokers account id --polygon-api-key TEXT Your Polygon.io API Key --polygon-license-type [Individual|Business] Select your Polygon.io subscription plan (Optional). From a5d18ff50155beb3d283e28011de106633d3c585 Mon Sep 17 00:00:00 2001 From: Jhonathan Abreu Date: Tue, 3 Nov 2026 18:26:58 -0400 Subject: [PATCH 6/8] fix: fail a set whose sanitized key already holds an object, as LEAN's save does Nothing records who wrote the object under the sanitized key, so it is only replaced through its exact key. The error is raised before any request. --- README.md | 5 +++-- lean/commands/cloud/object_store/set.py | 2 +- lean/components/util/object_store_helper.py | 6 ++++-- tests/commands/cloud/object_store/test_set.py | 9 +++++---- tests/components/util/test_object_store_helper.py | 8 +++++--- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 63b8623b0..baf5818c8 100644 --- a/README.md +++ b/README.md @@ -712,8 +712,9 @@ Usage: lean cloud object-store set [OPTIONS] KEY PATH Sets the data to the given key in the organization's cloud object store. :param key: The key to set the data to. A key that does not follow the object store key format is sanitized, with a - warning naming the key to use from then on, also when that overwrites an existing object. The other object-store - commands take the key as typed. :param path: Path to the file containing the object data. + warning naming the key to use from then on; the set fails when the sanitized key already holds an object, as LEAN's + save does. The other object-store commands take the key as typed. :param path: Path to the file containing the object + data. Options: --verbose Enable debug logging diff --git a/lean/commands/cloud/object_store/set.py b/lean/commands/cloud/object_store/set.py index 74edc4081..0b5cccd7e 100644 --- a/lean/commands/cloud/object_store/set.py +++ b/lean/commands/cloud/object_store/set.py @@ -24,7 +24,7 @@ def set(key: str, path: Path) -> None: """Sets the data to the given key in the organization's cloud object store. - :param key: The key to set the data to. A key that does not follow the object store key format is sanitized, with a warning naming the key to use from then on, also when that overwrites an existing object. The other object-store commands take the key as typed. + :param key: The key to set the data to. A key that does not follow the object store key format is sanitized, with a warning naming the key to use from then on; the set fails when the sanitized key already holds an object, as LEAN's save does. The other object-store commands take the key as typed. :param path: Path to the file containing the object data. """ organization_id = container.organization_manager.try_get_working_organization_id() diff --git a/lean/components/util/object_store_helper.py b/lean/components/util/object_store_helper.py index 7d8a28b92..7b2f8c7a2 100644 --- a/lean/components/util/object_store_helper.py +++ b/lean/components/util/object_store_helper.py @@ -28,7 +28,8 @@ def key_to_save(key: str, exists: Callable[[str], bool], logger: Logger) -> str: """Returns the key a set goes to: the given one when it follows the key format, else its sanitized version, - warning about the rewrite and about the existing object it overwrites, if any. + warning about the rewrite. Raises when the sanitized key already holds an object, as LEAN's save does: nothing + records who wrote it, so it is only replaced through its exact key. :param key: the key as given by the user :param exists: whether an object is stored under a key @@ -42,7 +43,8 @@ def key_to_save(key: str, exists: Callable[[str], bool], logger: Logger) -> str: sanitized = sanitize_key(key) logger.warn(f"Key '{key}' is not supported: {issues}. Using '{sanitized}' instead") if exists(sanitized): - logger.warn(f"'{sanitized}' already exists and will be overwritten") + raise RuntimeError(f"Key '{key}' is not supported and its sanitized name '{sanitized}' already holds another object. " + f"Delete '{sanitized}' or set it directly to replace it") return sanitized diff --git a/tests/commands/cloud/object_store/test_set.py b/tests/commands/cloud/object_store/test_set.py index c323bf588..3c4d52916 100644 --- a/tests/commands/cloud/object_store/test_set.py +++ b/tests/commands/cloud/object_store/test_set.py @@ -56,7 +56,7 @@ def test_set_sanitizes_an_unsupported_key_with_a_warning() -> None: container.api_client.object_store.set.assert_called_once_with('trade log_ AI _ Cloud.csv', file.read(), 'abc') -def test_set_warns_when_the_sanitized_key_overwrites_an_existing_object() -> None: +def test_set_fails_when_the_sanitized_key_already_holds_an_object() -> None: create_fake_lean_cli_project("test", "python") api_client = mock.Mock() api_client.is_authenticated.return_value = True @@ -66,9 +66,10 @@ def test_set_warns_when_the_sanitized_key_overwrites_an_existing_object() -> Non result = CliRunner().invoke(lean, ["cloud", "object-store", "set", "report (1).csv", file_path]) - assert result.exit_code == 0 - assert "'report _1_.csv' already exists and will be overwritten" in " ".join(result.output.split()) - container.api_client.object_store.set.assert_called_once_with('report _1_.csv', mock.ANY, 'abc') + assert result.exit_code != 0 + assert str(result.exception) == "Key 'report (1).csv' is not supported and its sanitized name 'report _1_.csv' already " \ + "holds another object. Delete 'report _1_.csv' or set it directly to replace it" + container.api_client.object_store.set.assert_not_called() def test_set_keeps_a_supported_key_silently() -> None: diff --git a/tests/components/util/test_object_store_helper.py b/tests/components/util/test_object_store_helper.py index bd9b15d28..88292b738 100644 --- a/tests/components/util/test_object_store_helper.py +++ b/tests/components/util/test_object_store_helper.py @@ -90,9 +90,11 @@ def test_key_to_save_sanitizes_an_unsupported_key_with_a_warning() -> None: "Using 'trade log_ AI _ Cloud.csv' instead") -def test_key_to_save_warns_when_the_sanitized_key_overwrites_an_existing_object() -> None: +def test_key_to_save_raises_when_the_sanitized_key_already_holds_an_object() -> None: logger = mock.Mock() - assert key_to_save("report (1).csv", lambda key: key == "report _1_.csv", logger) == "report _1_.csv" + with pytest.raises(RuntimeError) as error: + key_to_save("report (1).csv", lambda key: key == "report _1_.csv", logger) - assert logger.warn.call_args_list[-1] == mock.call("'report _1_.csv' already exists and will be overwritten") + assert str(error.value) == "Key 'report (1).csv' is not supported and its sanitized name 'report _1_.csv' already holds " \ + "another object. Delete 'report _1_.csv' or set it directly to replace it" From d3bdb2c0ae0da979acb7cf06a9d783e093949270 Mon Sep 17 00:00:00 2001 From: Jhonathan Abreu Date: Thu, 5 Nov 2026 09:41:11 -0400 Subject: [PATCH 7/8] Allow dots anywhere in object store keys and reject only '..' Keys may now carry dots in folder names and multiple extensions, as LEAN's ObjectStore does: only '..' is unsupported, since it would leave the storage root, along with a key that is nothing but dots and separators. A backslash is no longer a supported character and is sanitized to '/'. The supported check reuses the unsupported-characters regex, so the character set is written once. --- lean/components/util/object_store_helper.py | 58 ++++++++----------- .../util/test_object_store_helper.py | 48 ++++++++++----- 2 files changed, 59 insertions(+), 47 deletions(-) diff --git a/lean/components/util/object_store_helper.py b/lean/components/util/object_store_helper.py index 7b2f8c7a2..e5beaaa86 100644 --- a/lean/components/util/object_store_helper.py +++ b/lean/components/util/object_store_helper.py @@ -17,13 +17,13 @@ from lean.components.config.lean_config_manager import LeanConfigManager from lean.components.util.logger import Logger -# The key rules of LEAN's ObjectStore (QuantConnect/Lean#9905), mirrored by set only so a key set here is the one an +# The key rules of LEAN's ObjectStore (QuantConnect/Lean#10029), mirrored by set only so a key set here is the one an # algorithm saves to. The other commands send the key as typed; LEAN's reads also fall back to the sanitized key -_SUPPORTED_KEY = re.compile(r"\.?[a-zA-Z0-9\\/_#\-$= ]+\.?[a-zA-Z0-9]*") -_UNSUPPORTED_CHARACTERS = re.compile(r"[^a-zA-Z0-9\\/_#\-$= .]") -_EXTENSION = re.compile(r"[a-zA-Z0-9]+") -_KEY_RULES = "Keys may only contain english letters, numbers, spaces and the characters '/', '\\', '_', '#', '-', '$' and '=', " \ - "plus at most one '.' followed by a letters-and-numbers-only extension, e.g. 'folder/trade_log-2024.csv'" +_UNSUPPORTED_CHARACTERS = re.compile(r"[^a-zA-Z0-9/_#\-$= .]") +# '..' is never allowed: it would leave the storage root +_CONSECUTIVE_DOTS = re.compile(r"\.{2,}") +_KEY_RULES = "Keys may only contain english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', " \ + "never two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv'" def key_to_save(key: str, exists: Callable[[str], bool], logger: Logger) -> str: @@ -60,21 +60,15 @@ def get_key_issues(key: str) -> Optional[str]: def sanitize_key(key: str) -> str: - """Converts an unsupported key into a supported one by replacing every unsupported character with '_', - keeping at most the final '.extension': 'trade log: AI & Cloud.csv' becomes 'trade log_ AI _ Cloud.csv'. + """Converts an unsupported key into a supported one: '\' becomes '/', the separator LEAN's local store already + reads it as, every other unsupported character and every '.' of a '..' run become '_', so 'trade log: AI & Cloud.csv' + becomes 'trade log_ AI _ Cloud.csv'. :param key: the key to sanitize :return: a key following the key format """ - sanitized = _UNSUPPORTED_CHARACTERS.sub("_", key) - - # a single trailing '.extension' of letters and numbers is allowed: keep the last dot if it - # introduces a valid extension and replace every other dot - last_dot = sanitized.rfind(".") - if last_dot > 0 and _EXTENSION.fullmatch(sanitized[last_dot + 1:]) is not None: - sanitized = sanitized[:last_dot].replace(".", "_") + sanitized[last_dot:] - elif last_dot >= 0: - sanitized = sanitized.replace(".", "_") + sanitized = _UNSUPPORTED_CHARACTERS.sub("_", key.replace("\\", "/")) + sanitized = _CONSECUTIVE_DOTS.sub(lambda match: "_" * len(match.group()), sanitized) issues = get_key_issues(sanitized) if issues is not None: @@ -83,7 +77,12 @@ def sanitize_key(key: str) -> str: def _is_supported_key(key: str) -> bool: - return bool(key) and _SUPPORTED_KEY.fullmatch(key) is not None and key.count("/") <= 100 and key.count("\\") <= 100 + return bool(key) and _UNSUPPORTED_CHARACTERS.search(key) is None and ".." not in key and _has_name(key) and key.count("/") <= 100 + + +def _has_name(key: str) -> bool: + """Whether the key has something besides dots and separators, so it names an object and not a folder.""" + return any(char not in "./" for char in key) def _find_key_issues(key: str) -> List[str]: @@ -96,22 +95,13 @@ def _find_key_issues(key: str) -> List[str]: if unsupported: issues.append(f"unsupported character{'s' if len(unsupported) > 1 else ''} {', '.join(unsupported)}") - # a single leading '.' is allowed, every other '.' must introduce the final extension - name = key.lstrip(".") - dots = name.count(".") - if dots > 1: - issues.append(f"it has {dots} '.' but at most one '.extension' is allowed") - elif dots == 1: - extension = name[name.index(".") + 1:] - if extension and _EXTENSION.fullmatch(extension) is None: - issues.append(f"the extension '{extension}' may only contain letters and numbers") - if not name: - issues.append("it has no name before the '.'") - elif key.startswith(".."): - issues.append("it starts with more than one '.'") - - if key.count("/") > 100 or key.count("\\") > 100: - issues.append("it has more than 100 '/' or '\\' separators") + if ".." in key: + issues.append("it contains '..'") + if not _has_name(key): + issues.append("it has no name, only '.' and separators") + + if key.count("/") > 100: + issues.append("it has more than 100 '/' separators") if not issues: issues.append("it does not follow the key format") diff --git a/tests/components/util/test_object_store_helper.py b/tests/components/util/test_object_store_helper.py index 88292b738..97ca53b07 100644 --- a/tests/components/util/test_object_store_helper.py +++ b/tests/components/util/test_object_store_helper.py @@ -17,11 +17,12 @@ from lean.components.util.object_store_helper import get_key_issues, key_to_save, sanitize_key -RULES = "Keys may only contain english letters, numbers, spaces and the characters '/', '\\', '_', '#', '-', '$' and '=', " \ - "plus at most one '.' followed by a letters-and-numbers-only extension, e.g. 'folder/trade_log-2024.csv'" +RULES = "Keys may only contain english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', " \ + "never two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv'" -@pytest.mark.parametrize("key", ["my_key", "./my_key/nested\\file.csv", "file with spaces #1-$=.txt", "folder/trade_log-2024.csv", "/"]) +@pytest.mark.parametrize("key", ["my_key", "./my_key/nested/file.csv", "file with spaces #1-$=.txt", "folder/trade_log-2024.csv", + "folder.v1/file.tar.gz", ".hidden/file.name_v2", "trailing.", "a/"]) def test_get_key_issues_returns_none_for_a_supported_key(key: str) -> None: assert get_key_issues(key) is None @@ -29,14 +30,15 @@ def test_get_key_issues_returns_none_for_a_supported_key(key: str) -> None: @pytest.mark.parametrize("key,expected_issue", [ ("trade log: AI & Cloud.csv", "unsupported characters ':', '&'"), ("résumé.pdf", "unsupported character 'é'"), - ("file.tar.gz", "it has 2 '.' but at most one '.extension' is allowed"), - ("file.name_v2", "the extension 'name_v2' may only contain letters and numbers"), - ("folder.v1/file", "the extension 'v1/file' may only contain letters and numbers"), - ("trade log: AI.v2.csv", "unsupported character ':'; it has 2 '.' but at most one '.extension' is allowed"), - (".", "it has no name before the '.'"), - ("..model.pkl", "it starts with more than one '.'"), + ("folder\\file.csv", "unsupported character '\\'"), + ("..model.pkl", "it contains '..'"), + ("a/../b.csv", "it contains '..'"), + ("trade log: AI..v2.csv", "unsupported character ':'; it contains '..'"), + (".", "it has no name, only '.' and separators"), + ("./", "it has no name, only '.' and separators"), + ("/", "it has no name, only '.' and separators"), ("", "the key is empty"), - ("a" + "/" * 101, "it has more than 100 '/' or '\\' separators"), + ("a" + "/" * 101, "it has more than 100 '/' separators"), ]) def test_get_key_issues_states_the_issue_and_the_rules(key: str, expected_issue: str) -> None: assert get_key_issues(key) == f"{expected_issue}. {RULES}" @@ -45,13 +47,15 @@ def test_get_key_issues_states_the_issue_and_the_rules(key: str, expected_issue: @pytest.mark.parametrize("key,expected", [ ("trade_log_ai_hardware_&_cloud.csv", "trade_log_ai_hardware___cloud.csv"), ("trade log: AI & Cloud.csv", "trade log_ AI _ Cloud.csv"), - ("file.tar.gz", "file_tar.gz"), ("résumé.pdf", "r_sum_.pdf"), ("a..", "a__"), + ("a...b.csv", "a___b.csv"), ("100%.json", "100_.json"), ("../prefix/file", "__/prefix/file"), - (".", "_"), + ("a/../b.csv", "a/__/b.csv"), ("..model.pkl", "__model.pkl"), + ("folder\\file.csv", "folder/file.csv"), + ("file.csv\n", "file.csv_"), ]) def test_sanitize_key_returns_a_supported_key(key: str, expected: str) -> None: sanitized = sanitize_key(key) @@ -60,7 +64,25 @@ def test_sanitize_key_returns_a_supported_key(key: str, expected: str) -> None: assert get_key_issues(sanitized) is None -@pytest.mark.parametrize("key", ["", "a" + "/" * 101]) +@pytest.mark.parametrize("key,expected", [ + ("../secrets.json", "__/secrets.json"), + ("../../secrets.json", "__/__/secrets.json"), + ("..\\secrets.json", "__/secrets.json"), + ("./../secrets.json", "./__/secrets.json"), + ("x/../secrets.json", "x/__/secrets.json"), + ("x/../../secrets.json", "x/__/__/secrets.json"), + ("x\\..\\..\\secrets.json", "x/__/__/secrets.json"), + ("x/.../secrets.json", "x/___/secrets.json"), + ("x/..", "x/__"), + ("x/....", "x/____"), +]) +def test_traversal_keys_are_sanitized_so_they_cannot_leave_the_storage_root(key: str, expected: str) -> None: + assert "it contains '..'" in get_key_issues(key) + assert sanitize_key(key) == expected + assert ".." not in sanitize_key(key) + + +@pytest.mark.parametrize("key", ["", ".", "./", "/", "a" + "/" * 101]) def test_sanitize_key_raises_when_the_key_cannot_be_fixed(key: str) -> None: with pytest.raises(RuntimeError, match="is not supported"): sanitize_key(key) From 8e5b51df25d9f220f82336b966b02ca10331c7f4 Mon Sep 17 00:00:00 2001 From: Jhonathan Abreu Date: Thu, 5 Nov 2026 10:37:51 -0400 Subject: [PATCH 8/8] Rely on the cloud to sanitize object store keys and print its warnings on set The cloud now stores a key with unsupported characters under a sanitized name and returns the key used and a warning. The set command sends the key as typed and prints those warnings, so the CLI no longer carries its own copy of the key rules: the helper, the properties pre-check and their tests are removed. --- README.md | 7 +- lean/commands/cloud/object_store/set.py | 12 +- lean/components/api/object_store_client.py | 20 +-- lean/components/util/object_store_helper.py | 96 -------------- .../cloud/object_store/test_delete.py | 13 -- tests/commands/cloud/object_store/test_get.py | 18 --- .../commands/cloud/object_store/test_list.py | 14 -- .../cloud/object_store/test_properties.py | 51 -------- tests/commands/cloud/object_store/test_set.py | 38 ++---- .../api/test_object_store_client.py | 44 ------- .../util/test_object_store_helper.py | 122 ------------------ 11 files changed, 26 insertions(+), 409 deletions(-) delete mode 100644 tests/commands/cloud/object_store/test_properties.py delete mode 100644 tests/components/api/test_object_store_client.py delete mode 100644 tests/components/util/test_object_store_helper.py diff --git a/README.md b/README.md index baf5818c8..3d09ce0e8 100644 --- a/README.md +++ b/README.md @@ -711,10 +711,9 @@ Usage: lean cloud object-store set [OPTIONS] KEY PATH Sets the data to the given key in the organization's cloud object store. - :param key: The key to set the data to. A key that does not follow the object store key format is sanitized, with a - warning naming the key to use from then on; the set fails when the sanitized key already holds an object, as LEAN's - save does. The other object-store commands take the key as typed. :param path: Path to the file containing the object - data. + :param key: The key to set the data to. The cloud stores a key that does not follow the object store key format under + a sanitized name, with a warning naming the key to use from then on. :param path: Path to the file containing the + object data. Options: --verbose Enable debug logging diff --git a/lean/commands/cloud/object_store/set.py b/lean/commands/cloud/object_store/set.py index 0b5cccd7e..237fec67b 100644 --- a/lean/commands/cloud/object_store/set.py +++ b/lean/commands/cloud/object_store/set.py @@ -11,11 +11,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +from html import unescape from click import command, argument from pathlib import Path from lean.click import LeanCommand, PathParameter from lean.container import container -from lean.components.util.object_store_helper import key_to_save @command(cls=LeanCommand) @@ -24,13 +24,15 @@ def set(key: str, path: Path) -> None: """Sets the data to the given key in the organization's cloud object store. - :param key: The key to set the data to. A key that does not follow the object store key format is sanitized, with a warning naming the key to use from then on; the set fails when the sanitized key already holds an object, as LEAN's save does. The other object-store commands take the key as typed. + :param key: The key to set the data to. The cloud stores a key that does not follow the object store key format under a sanitized name, with a warning naming the key to use from then on. :param path: Path to the file containing the object data. """ organization_id = container.organization_manager.try_get_working_organization_id() - api_client = container.api_client - key = key_to_save(key, lambda k: api_client.object_store.exists(k, organization_id), container.logger) container.logger.info(f"Setting object {key} in organization {organization_id}") + api_client = container.api_client with open(path, "rb") as file: bytes_data: bytes = file.read() - api_client.object_store.set(key, bytes_data, organization_id) + response = api_client.object_store.set(key, bytes_data, organization_id) + # the cloud encodes the key in its messages for web pages + for warning in response.get("warnings", []) if isinstance(response, dict) else []: + container.logger.warn(unescape(warning)) diff --git a/lean/components/api/object_store_client.py b/lean/components/api/object_store_client.py index 1932659c5..e82f5fa53 100644 --- a/lean/components/api/object_store_client.py +++ b/lean/components/api/object_store_client.py @@ -13,7 +13,6 @@ from lean.components.api.api_client import * from lean.components.util.logger import Logger -from lean.models.errors import RequestFailedError class ObjectStoreClient: @@ -39,20 +38,6 @@ def properties(self, key: str, organization_id: str) -> str: return self._api.post("object/properties", payload) - def exists(self, key: str, organization_id: str) -> bool: - """Returns whether an object is stored under the given key, as typed. - :param key: key of the object to look for - :param organization_id: the id of the organization who's object store to look in - :return: True if the object exists - """ - try: - self.properties(key, organization_id) - return True - except RequestFailedError as error: - if "not found" in str(error).lower(): - return False - raise - def get(self, keys: [str], organization_id: str, logger: Logger) -> str: """Will fetch an url to download the requested keys :param keys: keys of the object to retrieve @@ -95,11 +80,12 @@ def get(self, keys: [str], organization_id: str, logger: Logger) -> str: return data["url"] - def set(self, key: str, object_data: bytes, organization_id: str) -> None: + def set(self, key: str, object_data: bytes, organization_id: str) -> dict: """Sets the given key in the Object Store. :param key: key of the object to set :param object_data: the data to set :param organization_id: the id of the organization who's object store to set data in + :return: the api response, with the key the object was stored under and any warnings about it """ payload = { "organizationId": organization_id, @@ -108,7 +94,7 @@ def set(self, key: str, object_data: bytes, organization_id: str) -> None: extra_options = { "files": {"objectData": object_data} } - self._api.post("object/set", payload, False, extra_options) + return self._api.post("object/set", payload, False, extra_options) def list(self, path: str, organization_id: str) -> str: """List all values for the given root key in the Object Store. diff --git a/lean/components/util/object_store_helper.py b/lean/components/util/object_store_helper.py index e5beaaa86..549f0865f 100644 --- a/lean/components/util/object_store_helper.py +++ b/lean/components/util/object_store_helper.py @@ -11,103 +11,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -import re -from typing import Callable, List, Optional - from lean.components.config.lean_config_manager import LeanConfigManager -from lean.components.util.logger import Logger - -# The key rules of LEAN's ObjectStore (QuantConnect/Lean#10029), mirrored by set only so a key set here is the one an -# algorithm saves to. The other commands send the key as typed; LEAN's reads also fall back to the sanitized key -_UNSUPPORTED_CHARACTERS = re.compile(r"[^a-zA-Z0-9/_#\-$= .]") -# '..' is never allowed: it would leave the storage root -_CONSECUTIVE_DOTS = re.compile(r"\.{2,}") -_KEY_RULES = "Keys may only contain english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', " \ - "never two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv'" - - -def key_to_save(key: str, exists: Callable[[str], bool], logger: Logger) -> str: - """Returns the key a set goes to: the given one when it follows the key format, else its sanitized version, - warning about the rewrite. Raises when the sanitized key already holds an object, as LEAN's save does: nothing - records who wrote it, so it is only replaced through its exact key. - - :param key: the key as given by the user - :param exists: whether an object is stored under a key - :param logger: the logger to warn with - :return: the key to send to the object store - """ - issues = get_key_issues(key) - if issues is None: - return key - - sanitized = sanitize_key(key) - logger.warn(f"Key '{key}' is not supported: {issues}. Using '{sanitized}' instead") - if exists(sanitized): - raise RuntimeError(f"Key '{key}' is not supported and its sanitized name '{sanitized}' already holds another object. " - f"Delete '{sanitized}' or set it directly to replace it") - return sanitized - - -def get_key_issues(key: str) -> Optional[str]: - """Describes why a key does not follow the key format: the problems found and the rules. - - :param key: the key to check - :return: the issues with the key, or None when it is supported - """ - if _is_supported_key(key): - return None - return "; ".join(_find_key_issues(key)) + ". " + _KEY_RULES - - -def sanitize_key(key: str) -> str: - """Converts an unsupported key into a supported one: '\' becomes '/', the separator LEAN's local store already - reads it as, every other unsupported character and every '.' of a '..' run become '_', so 'trade log: AI & Cloud.csv' - becomes 'trade log_ AI _ Cloud.csv'. - - :param key: the key to sanitize - :return: a key following the key format - """ - sanitized = _UNSUPPORTED_CHARACTERS.sub("_", key.replace("\\", "/")) - sanitized = _CONSECUTIVE_DOTS.sub(lambda match: "_" * len(match.group()), sanitized) - - issues = get_key_issues(sanitized) - if issues is not None: - raise RuntimeError(f"Key '{key}' is not supported: {issues}") - return sanitized - - -def _is_supported_key(key: str) -> bool: - return bool(key) and _UNSUPPORTED_CHARACTERS.search(key) is None and ".." not in key and _has_name(key) and key.count("/") <= 100 - - -def _has_name(key: str) -> bool: - """Whether the key has something besides dots and separators, so it names an object and not a folder.""" - return any(char not in "./" for char in key) - - -def _find_key_issues(key: str) -> List[str]: - """Lists why a key is not supported, one entry per problem found.""" - if not key: - return ["the key is empty"] - - issues = [] - unsupported = list(dict.fromkeys(f"'{char}'" for char in _UNSUPPORTED_CHARACTERS.findall(key))) - if unsupported: - issues.append(f"unsupported character{'s' if len(unsupported) > 1 else ''} {', '.join(unsupported)}") - - if ".." in key: - issues.append("it contains '..'") - if not _has_name(key): - issues.append("it has no name, only '.' and separators") - - if key.count("/") > 100: - issues.append("it has more than 100 '/' separators") - - if not issues: - issues.append("it does not follow the key format") - return issues - - def open_storage_directory_in_explorer(lean_config_manager: LeanConfigManager): """Opens the storage directory in the file explorer.""" diff --git a/tests/commands/cloud/object_store/test_delete.py b/tests/commands/cloud/object_store/test_delete.py index 5cc0d6ef1..76ea1a1db 100644 --- a/tests/commands/cloud/object_store/test_delete.py +++ b/tests/commands/cloud/object_store/test_delete.py @@ -28,16 +28,3 @@ def test_delete_deletes_value_when_key_is_given() -> None: result = CliRunner().invoke(lean, ["cloud", "object-store", "delete", "test-key"]) assert result.exit_code == 0 container.api_client.object_store.delete.assert_called_once_with('test-key', 'abc') - - -def test_delete_sends_an_unsupported_key_as_typed() -> None: - api_client = mock.Mock() - api_client.is_authenticated.return_value = True - initialize_container(api_client_to_use=api_client) - - result = CliRunner().invoke(lean, ["cloud", "object-store", "delete", "model.v2.pkl"]) - - assert result.exit_code == 0 - assert "is not supported" not in result.output - api_client.object_store.exists.assert_not_called() - container.api_client.object_store.delete.assert_called_once_with('model.v2.pkl', 'abc') diff --git a/tests/commands/cloud/object_store/test_get.py b/tests/commands/cloud/object_store/test_get.py index 7020e57f3..bd5b0a4c5 100644 --- a/tests/commands/cloud/object_store/test_get.py +++ b/tests/commands/cloud/object_store/test_get.py @@ -36,21 +36,3 @@ def test_get_gets_value_when_key_is_given(mock_zipfile) -> None: result = CliRunner().invoke(lean, ["cloud", "object-store", "get", "test-key"]) assert result.exit_code == 0 container.api_client.object_store.get.assert_called_once_with(('test-key',), 'abc', container.logger) - - -@mock.patch("zipfile.ZipFile") -def test_get_sends_unsupported_keys_as_typed(mock_zipfile) -> None: - archive = mock.Mock() - archive.return_value.read = mock.Mock() - mock_zipfile.return_value.__enter__ = archive - - api_client = mock.Mock() - api_client.is_authenticated.return_value = True - initialize_container(api_client_to_use=api_client) - - result = CliRunner().invoke(lean, ["cloud", "object-store", "get", "trade log: AI & Cloud.csv", "model.v2.pkl"]) - - assert result.exit_code == 0 - assert "is not supported" not in result.output - api_client.object_store.exists.assert_not_called() - container.api_client.object_store.get.assert_called_once_with(('trade log: AI & Cloud.csv', 'model.v2.pkl'), 'abc', container.logger) diff --git a/tests/commands/cloud/object_store/test_list.py b/tests/commands/cloud/object_store/test_list.py index 73677212e..87dadfea1 100644 --- a/tests/commands/cloud/object_store/test_list.py +++ b/tests/commands/cloud/object_store/test_list.py @@ -48,17 +48,3 @@ def test_list_handles_empty_or_none_objects(objects_value) -> None: container.api_client.object_store.list.assert_called_once_with('test', 'abc') - - -def test_list_sends_an_unsupported_root_key_as_typed() -> None: - api_client = mock.Mock() - api_client.is_authenticated.return_value = True - api_client.object_store.list.return_value = {'objects': [], 'success': True, 'path': 'trade log:'} - initialize_container(api_client_to_use=api_client) - - result = CliRunner().invoke(lean, ["cloud", "object-store", "list", "trade log:"]) - - assert result.exit_code == 0 - assert "is not supported" not in result.output - assert "No objects found at 'trade log:'." in result.output - container.api_client.object_store.list.assert_called_once_with('trade log:', 'abc') diff --git a/tests/commands/cloud/object_store/test_properties.py b/tests/commands/cloud/object_store/test_properties.py deleted file mode 100644 index a30f82009..000000000 --- a/tests/commands/cloud/object_store/test_properties.py +++ /dev/null @@ -1,51 +0,0 @@ -# QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. -# Lean CLI v1.0. Copyright 2021 QuantConnect Corporation. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from unittest import mock - -from click.testing import CliRunner - -from lean.commands import lean -from lean.container import container -from tests.conftest import initialize_container - - -def test_properties_gets_value_when_key_is_given() -> None: - api_client = mock.Mock() - api_client.is_authenticated.return_value = True - api_client.object_store.properties.return_value = { - 'metadata': {'size': 10, 'modified': '2024-01-01', 'key': 'test-key', 'preview': 'abc'} - } - initialize_container(api_client_to_use=api_client) - - result = CliRunner().invoke(lean, ["cloud", "object-store", "properties", "test-key"]) - - assert result.exit_code == 0 - assert "test-key" in result.output - container.api_client.object_store.properties.assert_called_once_with('test-key', 'abc') - - -def test_properties_sends_an_unsupported_key_as_typed() -> None: - api_client = mock.Mock() - api_client.is_authenticated.return_value = True - api_client.object_store.properties.return_value = { - 'metadata': {'size': 10, 'modified': '2024-01-01', 'key': 'model.v2.pkl', 'preview': 'abc'} - } - initialize_container(api_client_to_use=api_client) - - result = CliRunner().invoke(lean, ["cloud", "object-store", "properties", "model.v2.pkl"]) - - assert result.exit_code == 0 - assert "is not supported" not in result.output - api_client.object_store.exists.assert_not_called() - container.api_client.object_store.properties.assert_called_once_with('model.v2.pkl', 'abc') diff --git a/tests/commands/cloud/object_store/test_set.py b/tests/commands/cloud/object_store/test_set.py index 3c4d52916..72b884789 100644 --- a/tests/commands/cloud/object_store/test_set.py +++ b/tests/commands/cloud/object_store/test_set.py @@ -37,45 +37,34 @@ def test_set_sets_value_when_path_is_given() -> None: container.api_client.object_store.set.assert_called_once_with('test-key', bytes_data, 'abc') -def test_set_sanitizes_an_unsupported_key_with_a_warning() -> None: +def test_set_prints_the_warnings_the_api_returns() -> None: create_fake_lean_cli_project("test", "python") api_client = mock.Mock() api_client.is_authenticated.return_value = True - api_client.object_store.exists.return_value = False + api_client.object_store.set.return_value = { + "success": True, + "key": "trade log_ AI _ Cloud.csv", + "warnings": ["Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. " + "Using 'trade log_ AI _ Cloud.csv' instead"] + } initialize_container(api_client_to_use=api_client) file_path = str(Path.joinpath(Path.cwd(), "lean.json")) result = CliRunner().invoke(lean, ["cloud", "object-store", "set", "trade log: AI & Cloud.csv", file_path]) assert result.exit_code == 0 - assert "Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. Keys may only contain" in " ".join(result.output.split()) - assert "Using 'trade log_ AI _ Cloud.csv' instead" in " ".join(result.output.split()) - assert "will be overwritten" not in result.output - api_client.object_store.exists.assert_called_once_with('trade log_ AI _ Cloud.csv', 'abc') + output = " ".join(result.output.split()) + assert "Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. " \ + "Using 'trade log_ AI _ Cloud.csv' instead" in output with open(file_path, "rb") as file: - container.api_client.object_store.set.assert_called_once_with('trade log_ AI _ Cloud.csv', file.read(), 'abc') + container.api_client.object_store.set.assert_called_once_with('trade log: AI & Cloud.csv', file.read(), 'abc') -def test_set_fails_when_the_sanitized_key_already_holds_an_object() -> None: - create_fake_lean_cli_project("test", "python") - api_client = mock.Mock() - api_client.is_authenticated.return_value = True - api_client.object_store.exists.return_value = True - initialize_container(api_client_to_use=api_client) - file_path = str(Path.joinpath(Path.cwd(), "lean.json")) - - result = CliRunner().invoke(lean, ["cloud", "object-store", "set", "report (1).csv", file_path]) - - assert result.exit_code != 0 - assert str(result.exception) == "Key 'report (1).csv' is not supported and its sanitized name 'report _1_.csv' already " \ - "holds another object. Delete 'report _1_.csv' or set it directly to replace it" - container.api_client.object_store.set.assert_not_called() - - -def test_set_keeps_a_supported_key_silently() -> None: +def test_set_prints_no_warning_when_the_api_returns_none() -> None: create_fake_lean_cli_project("test", "python") api_client = mock.Mock() api_client.is_authenticated.return_value = True + api_client.object_store.set.return_value = {"success": True, "key": "folder/trade_log-2024.csv", "warnings": []} initialize_container(api_client_to_use=api_client) file_path = str(Path.joinpath(Path.cwd(), "lean.json")) @@ -83,5 +72,4 @@ def test_set_keeps_a_supported_key_silently() -> None: assert result.exit_code == 0 assert "is not supported" not in result.output - api_client.object_store.exists.assert_not_called() container.api_client.object_store.set.assert_called_once_with('folder/trade_log-2024.csv', mock.ANY, 'abc') diff --git a/tests/components/api/test_object_store_client.py b/tests/components/api/test_object_store_client.py deleted file mode 100644 index 6e47b5005..000000000 --- a/tests/components/api/test_object_store_client.py +++ /dev/null @@ -1,44 +0,0 @@ -# QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. -# Lean CLI v1.0. Copyright 2021 QuantConnect Corporation. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from unittest import mock - -import pytest - -from lean.components.api.object_store_client import ObjectStoreClient -from lean.models.errors import RequestFailedError - - -def test_exists_returns_true_when_the_properties_are_returned() -> None: - api = mock.Mock() - api.post.return_value = {"metadata": {"key": "model.v2.pkl"}} - - assert ObjectStoreClient(api).exists("model.v2.pkl", "abc") - - api.post.assert_called_once_with("object/properties", {"organizationId": "abc", "key": "model.v2.pkl"}) - - -def test_exists_returns_false_on_the_api_not_found_message() -> None: - api = mock.Mock() - # the message object/properties answers for a missing key, as the api returns it - api.post.side_effect = RequestFailedError(mock.MagicMock(), "File not found model.v2.pkl") - - assert not ObjectStoreClient(api).exists("model.v2.pkl", "abc") - - -def test_exists_raises_any_other_failure() -> None: - api = mock.Mock() - api.post.side_effect = RequestFailedError(mock.MagicMock(), "Organization not authorized") - - with pytest.raises(RequestFailedError, match="not authorized"): - ObjectStoreClient(api).exists("model.v2.pkl", "abc") diff --git a/tests/components/util/test_object_store_helper.py b/tests/components/util/test_object_store_helper.py deleted file mode 100644 index 97ca53b07..000000000 --- a/tests/components/util/test_object_store_helper.py +++ /dev/null @@ -1,122 +0,0 @@ -# QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. -# Lean CLI v1.0. Copyright 2021 QuantConnect Corporation. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from unittest import mock - -import pytest - -from lean.components.util.object_store_helper import get_key_issues, key_to_save, sanitize_key - -RULES = "Keys may only contain english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', " \ - "never two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv'" - - -@pytest.mark.parametrize("key", ["my_key", "./my_key/nested/file.csv", "file with spaces #1-$=.txt", "folder/trade_log-2024.csv", - "folder.v1/file.tar.gz", ".hidden/file.name_v2", "trailing.", "a/"]) -def test_get_key_issues_returns_none_for_a_supported_key(key: str) -> None: - assert get_key_issues(key) is None - - -@pytest.mark.parametrize("key,expected_issue", [ - ("trade log: AI & Cloud.csv", "unsupported characters ':', '&'"), - ("résumé.pdf", "unsupported character 'é'"), - ("folder\\file.csv", "unsupported character '\\'"), - ("..model.pkl", "it contains '..'"), - ("a/../b.csv", "it contains '..'"), - ("trade log: AI..v2.csv", "unsupported character ':'; it contains '..'"), - (".", "it has no name, only '.' and separators"), - ("./", "it has no name, only '.' and separators"), - ("/", "it has no name, only '.' and separators"), - ("", "the key is empty"), - ("a" + "/" * 101, "it has more than 100 '/' separators"), -]) -def test_get_key_issues_states_the_issue_and_the_rules(key: str, expected_issue: str) -> None: - assert get_key_issues(key) == f"{expected_issue}. {RULES}" - - -@pytest.mark.parametrize("key,expected", [ - ("trade_log_ai_hardware_&_cloud.csv", "trade_log_ai_hardware___cloud.csv"), - ("trade log: AI & Cloud.csv", "trade log_ AI _ Cloud.csv"), - ("résumé.pdf", "r_sum_.pdf"), - ("a..", "a__"), - ("a...b.csv", "a___b.csv"), - ("100%.json", "100_.json"), - ("../prefix/file", "__/prefix/file"), - ("a/../b.csv", "a/__/b.csv"), - ("..model.pkl", "__model.pkl"), - ("folder\\file.csv", "folder/file.csv"), - ("file.csv\n", "file.csv_"), -]) -def test_sanitize_key_returns_a_supported_key(key: str, expected: str) -> None: - sanitized = sanitize_key(key) - - assert sanitized == expected - assert get_key_issues(sanitized) is None - - -@pytest.mark.parametrize("key,expected", [ - ("../secrets.json", "__/secrets.json"), - ("../../secrets.json", "__/__/secrets.json"), - ("..\\secrets.json", "__/secrets.json"), - ("./../secrets.json", "./__/secrets.json"), - ("x/../secrets.json", "x/__/secrets.json"), - ("x/../../secrets.json", "x/__/__/secrets.json"), - ("x\\..\\..\\secrets.json", "x/__/__/secrets.json"), - ("x/.../secrets.json", "x/___/secrets.json"), - ("x/..", "x/__"), - ("x/....", "x/____"), -]) -def test_traversal_keys_are_sanitized_so_they_cannot_leave_the_storage_root(key: str, expected: str) -> None: - assert "it contains '..'" in get_key_issues(key) - assert sanitize_key(key) == expected - assert ".." not in sanitize_key(key) - - -@pytest.mark.parametrize("key", ["", ".", "./", "/", "a" + "/" * 101]) -def test_sanitize_key_raises_when_the_key_cannot_be_fixed(key: str) -> None: - with pytest.raises(RuntimeError, match="is not supported"): - sanitize_key(key) - - -def test_sanitize_key_is_lossy_and_distinct_keys_can_collide() -> None: - assert sanitize_key("trade_log_ai&cloud.csv") == sanitize_key("trade_log_ai:cloud.csv") == "trade_log_ai_cloud.csv" - - -def test_key_to_save_keeps_a_supported_key_without_checking_the_store() -> None: - logger = mock.Mock() - exists = mock.Mock() - - assert key_to_save("folder/trade_log-2024.csv", exists, logger) == "folder/trade_log-2024.csv" - - exists.assert_not_called() - logger.warn.assert_not_called() - - -def test_key_to_save_sanitizes_an_unsupported_key_with_a_warning() -> None: - logger = mock.Mock() - - assert key_to_save("trade log: AI & Cloud.csv", lambda key: False, logger) == "trade log_ AI _ Cloud.csv" - - logger.warn.assert_called_once_with( - f"Key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. {RULES}. " - "Using 'trade log_ AI _ Cloud.csv' instead") - - -def test_key_to_save_raises_when_the_sanitized_key_already_holds_an_object() -> None: - logger = mock.Mock() - - with pytest.raises(RuntimeError) as error: - key_to_save("report (1).csv", lambda key: key == "report _1_.csv", logger) - - assert str(error.value) == "Key 'report (1).csv' is not supported and its sanitized name 'report _1_.csv' already holds " \ - "another object. Delete 'report _1_.csv' or set it directly to replace it"