From 5781f8505b919267fe702ff803f7d6f6768126e7 Mon Sep 17 00:00:00 2001 From: suluyan Date: Mon, 20 Jul 2026 15:47:42 +0800 Subject: [PATCH] fix: reject empty model name in trusted owner cache paths check_model_from_owner_group treated paths like iic--/snapshots/v1 as trusted because split('--') still yields two parts. Require both owner and name segments to be non-empty. Co-authored-by: Cursor --- modelscope/utils/automodel_utils.py | 9 ++++--- tests/utils/test_owner_group_path_safety.py | 30 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 tests/utils/test_owner_group_path_safety.py diff --git a/modelscope/utils/automodel_utils.py b/modelscope/utils/automodel_utils.py index 73175dc6..1c46b296 100644 --- a/modelscope/utils/automodel_utils.py +++ b/modelscope/utils/automodel_utils.py @@ -147,12 +147,13 @@ def check_model_from_owner_group(model_dir: str, if group in owner_group: return True # Also check cache path pattern: {cache_root}/{owner}--{model_name}/snapshots/{revision} - # Require exactly "{owner}--{name}" format (2 segments split by --) - # to prevent spoofing via accounts like "iic--hacked" which would - # produce paths like "iic--hacked--evil" and bypass the check. + # Require exactly "{owner}--{name}" with both segments non-empty + # to prevent spoofing via accounts like "iic--hacked" (paths like + # "iic--hacked--evil") or empty names like "iic--". grandparent = os.path.basename(os.path.dirname(parent_dir)) if '--' in grandparent: parts = grandparent.split('--') - if len(parts) == 2 and parts[0] in owner_group: + # Both owner and name must be non-empty; reject "iic--" / "--name". + if len(parts) == 2 and all(parts) and parts[0] in owner_group: return True return False diff --git a/tests/utils/test_owner_group_path_safety.py b/tests/utils/test_owner_group_path_safety.py new file mode 100644 index 00000000..9a0bb5c6 --- /dev/null +++ b/tests/utils/test_owner_group_path_safety.py @@ -0,0 +1,30 @@ +# Copyright (c) Alibaba, Inc. and its affiliates. +import unittest + +from modelscope.utils.automodel_utils import check_model_from_owner_group + + +class OwnerGroupPathSafetyTest(unittest.TestCase): + """Safety checks for trusted-owner cache path recognition.""" + + def test_empty_name_cache_path_rejected(self): + # modelscope_hub layout: {cache}/{owner}--{name}/snapshots/{rev} + # Empty name ("iic--") must not be treated as a trusted owner path. + self.assertFalse( + check_model_from_owner_group('/cache/iic--/snapshots/v1')) + self.assertFalse( + check_model_from_owner_group('/cache/damo--/snapshots/v1')) + + def test_valid_and_spoof_cache_paths(self): + self.assertTrue( + check_model_from_owner_group('/cache/iic--x/snapshots/v1')) + self.assertFalse( + check_model_from_owner_group('/cache/--iic/snapshots/v1')) + self.assertFalse( + check_model_from_owner_group( + '/cache/iic--hacked--evil/snapshots/v1')) + self.assertTrue(check_model_from_owner_group('/cache/iic/some_model')) + + +if __name__ == '__main__': + unittest.main()