From 2435727f0c6b02d56e254a5ba1d8e041c7c33c86 Mon Sep 17 00:00:00 2001 From: DanielZhangQD <36026334+DanielZhangQD@users.noreply.github.com> Date: Mon, 29 Jun 2026 14:59:48 +0800 Subject: [PATCH] [Core] Mark non-security MD5/SHA1 hashes with usedforsecurity=False (#9972) * [Core] Mark non-security MD5/SHA1 hashes with usedforsecurity=False These hashlib.md5/sha1 calls derive identifiers, cache keys, and resource names from non-secret inputs (usernames, paths, region/subscription ids, cluster/host names, wheel/catalog contents). None protects a secret or makes an authentication decision -- passwords use bcrypt via crypt_ctx and auth is enforced by oauth2-proxy / SSO. Pass usedforsecurity=False so weak-hash scanners (CWE-328) stop flagging them. The flag does not change any digest value, so all derived ids, cache keys and names are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) * [Core] Address review: avoid split hexdigest() call in oauth2_proxy Compute the digest first, then slice, instead of splitting the empty hexdigest() parentheses across lines. yapf-stable and more readable. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- sky/backends/backend_utils.py | 3 ++- sky/backends/wheel_utils.py | 3 ++- sky/batch/io_formats.py | 3 ++- sky/catalog/aws_catalog.py | 6 ++++-- sky/catalog/common.py | 4 +++- sky/clouds/aws.py | 2 +- sky/clouds/nebius.py | 3 ++- sky/data/mounting_utils.py | 3 ++- sky/data/storage.py | 6 ++++-- sky/provision/azure/config.py | 2 +- sky/provision/kubernetes/utils.py | 3 ++- sky/provision/scp/instance.py | 2 +- sky/server/auth/oauth2_proxy.py | 7 +++++-- sky/server/server.py | 5 ++++- sky/users/permission.py | 6 ++++-- sky/users/server.py | 13 ++++++++++--- sky/utils/command_runner.py | 7 ++++--- sky/utils/common_utils.py | 13 ++++++++++--- sky/utils/kubernetes/gpu_labeler.py | 3 ++- 19 files changed, 65 insertions(+), 29 deletions(-) diff --git a/sky/backends/backend_utils.py b/sky/backends/backend_utils.py index ff4662b4160..ec0b4bf74fb 100644 --- a/sky/backends/backend_utils.py +++ b/sky/backends/backend_utils.py @@ -707,7 +707,8 @@ def get_expirable_clouds( def _get_volume_name(path: str, cluster_name_on_cloud: str) -> str: - path_hash = hashlib.md5(path.encode()).hexdigest()[:6] + path_hash = hashlib.md5(path.encode(), + usedforsecurity=False).hexdigest()[:6] return f'{cluster_name_on_cloud}-{path_hash}' diff --git a/sky/backends/wheel_utils.py b/sky/backends/wheel_utils.py index 4cfd4c485a5..f142011bfe1 100644 --- a/sky/backends/wheel_utils.py +++ b/sky/backends/wheel_utils.py @@ -210,7 +210,8 @@ def _build_sky_wheel() -> pathlib.Path: # wheel content hash doesn't change. with open(wheel_path, 'rb') as f: contents = f.read() - hash_of_latest_wheel = hashlib.md5(contents).hexdigest() + hash_of_latest_wheel = hashlib.md5(contents, + usedforsecurity=False).hexdigest() wheel_dir = WHEEL_DIR / hash_of_latest_wheel wheel_dir.mkdir(parents=True, exist_ok=True) diff --git a/sky/catalog/aws_catalog.py b/sky/catalog/aws_catalog.py index c2480965708..e842068beaa 100644 --- a/sky/catalog/aws_catalog.py +++ b/sky/catalog/aws_catalog.py @@ -113,7 +113,8 @@ def _get_az_mappings(aws_user_hash: str) -> Optional['pd.DataFrame']: # Write md5 of the az_mapping file to a file so we can check it for # any changes when uploading to the controller with open(az_mapping_path, 'r', encoding='utf-8') as f: - az_mapping_hash = hashlib.md5(f.read().encode()).hexdigest() + az_mapping_hash = hashlib.md5(f.read().encode(), + usedforsecurity=False).hexdigest() with open(az_mapping_md5_path, 'w', encoding='utf-8') as f: f.write(az_mapping_hash) else: @@ -147,7 +148,8 @@ def _fetch_and_apply_az_mapping(df: common.LazyDataFrame) -> 'pd.DataFrame': user_identity_list = aws.AWS.get_active_user_identity() assert user_identity_list, user_identity_list user_identity = user_identity_list[0] - aws_user_hash = hashlib.md5(user_identity.encode()).hexdigest()[:8] + aws_user_hash = hashlib.md5(user_identity.encode(), + usedforsecurity=False).hexdigest()[:8] except (exceptions.CloudUserIdentityError, ImportError): # If failed to get user identity, or import aws dependencies, we use the # latest mapping file or the default mapping file. diff --git a/sky/catalog/common.py b/sky/catalog/common.py index 00829871e53..a0397e6301f 100644 --- a/sky/catalog/common.py +++ b/sky/catalog/common.py @@ -291,7 +291,9 @@ def _update_catalog(): tmp_path = f.name os.rename(tmp_path, catalog_path) with open(meta_path + '.md5', 'w', encoding='utf-8') as f: - f.write(hashlib.md5(r.text.encode()).hexdigest()) + f.write( + hashlib.md5(r.text.encode(), + usedforsecurity=False).hexdigest()) logger.debug(f'Updated {cloud} catalog {filename}.') return True diff --git a/sky/clouds/aws.py b/sky/clouds/aws.py index af4ca56e911..06607e0d802 100644 --- a/sky/clouds/aws.py +++ b/sky/clouds/aws.py @@ -1347,7 +1347,7 @@ def get_user_identities(cls) -> Optional[List[List[str]]]: # - aws credentials are not set, proceed anyway to get unified error # message for users return cls._sts_get_caller_identity() - config_hash = hashlib.md5(stdout).hexdigest()[:8] + config_hash = hashlib.md5(stdout, usedforsecurity=False).hexdigest()[:8] # Getting aws identity cost ~1s, so we cache the result with the output of # `aws configure list` as cache key. Different `aws configure list` output # can have same aws identity, our assumption is the output would be stable diff --git a/sky/data/mounting_utils.py b/sky/data/mounting_utils.py index f49a8db097f..6b97b2d6ef0 100644 --- a/sky/data/mounting_utils.py +++ b/sky/data/mounting_utils.py @@ -714,7 +714,8 @@ def get_mount_cached_cmd( # This is because the full path may be longer than # the filename length limit. # The hash is a non-negative integer in string form. - hashed_mount_path = hashlib.md5(mount_path.encode()).hexdigest() + hashed_mount_path = hashlib.md5(mount_path.encode(), + usedforsecurity=False).hexdigest() log_file_path = os.path.join(constants.RCLONE_MOUNT_CACHED_LOG_DIR, f'{hashed_mount_path}.log') create_log_cmd = (f'mkdir -p {constants.RCLONE_MOUNT_CACHED_LOG_DIR} && ' diff --git a/sky/data/storage.py b/sky/data/storage.py index 022fc471f41..13e068c5be5 100644 --- a/sky/data/storage.py +++ b/sky/data/storage.py @@ -3367,10 +3367,12 @@ def get_default_storage_account_name(region: Optional[str]) -> str: """ assert region is not None subscription_id = azure.get_subscription_id() - subscription_hash_obj = hashlib.md5(subscription_id.encode('utf-8')) + subscription_hash_obj = hashlib.md5(subscription_id.encode('utf-8'), + usedforsecurity=False) subscription_hash = subscription_hash_obj.hexdigest( )[:AzureBlobStore._SUBSCRIPTION_HASH_LENGTH] - region_hash_obj = hashlib.md5(region.encode('utf-8')) + region_hash_obj = hashlib.md5(region.encode('utf-8'), + usedforsecurity=False) region_hash = region_hash_obj.hexdigest()[:AzureBlobStore. _REGION_HASH_LENGTH] diff --git a/sky/provision/azure/config.py b/sky/provision/azure/config.py index 2dbf115807f..074bc801c3e 100644 --- a/sky/provision/azure/config.py +++ b/sky/provision/azure/config.py @@ -44,7 +44,7 @@ def get_azure_sdk_function(client: Any, function_name: str) -> Callable: def get_cluster_id_and_nsg_name(resource_group: str, cluster_name_on_cloud: str) -> Tuple[str, str]: - hasher = hashlib.md5(resource_group.encode('utf-8')) + hasher = hashlib.md5(resource_group.encode('utf-8'), usedforsecurity=False) unique_id = hasher.hexdigest()[:UNIQUE_ID_LEN] # We use the cluster name + resource group hash as the # unique ID for the cluster, as we need to make sure that diff --git a/sky/provision/kubernetes/utils.py b/sky/provision/kubernetes/utils.py index 1ec32ffb095..e3478c45608 100644 --- a/sky/provision/kubernetes/utils.py +++ b/sky/provision/kubernetes/utils.py @@ -5085,7 +5085,8 @@ def format_kubeconfig_exec_auth_with_cache(kubeconfig_path: str) -> str: with open(kubeconfig_path, 'r', encoding='utf-8') as file: config = yaml_utils.safe_load(file) normalized = yaml.dump(config, sort_keys=True) - hashed = hashlib.sha1(normalized.encode('utf-8')).hexdigest() + hashed = hashlib.sha1(normalized.encode('utf-8'), + usedforsecurity=False).hexdigest() path = os.path.expanduser( f'{kubernetes_constants.SKY_K8S_EXEC_AUTH_KUBECONFIG_CACHE}/{hashed}.yaml' ) diff --git a/sky/provision/scp/instance.py b/sky/provision/scp/instance.py index 9aae05c9196..5a03c308e3f 100644 --- a/sky/provision/scp/instance.py +++ b/sky/provision/scp/instance.py @@ -223,7 +223,7 @@ def _worker(cluster_name_on_cloud: str): def _suffix(name: str, n: int = 5): - return hashlib.sha1(name.encode()).hexdigest()[:n] + return hashlib.sha1(name.encode(), usedforsecurity=False).hexdigest()[:n] def _get_instance_id(instance_name, cluster_name_on_cloud): diff --git a/sky/server/auth/oauth2_proxy.py b/sky/server/auth/oauth2_proxy.py index dece4792fa9..7d117642cf7 100644 --- a/sky/server/auth/oauth2_proxy.py +++ b/sky/server/auth/oauth2_proxy.py @@ -204,8 +204,11 @@ def get_auth_user( """Extract user info from OAuth2 proxy response headers.""" email_header = response.headers.get('X-Auth-Request-Email') if email_header: - user_hash = hashlib.md5(email_header.encode()).hexdigest( - )[:common_utils.USER_HASH_LENGTH] + # MD5 only derives a stable user id from the (non-secret) SSO + # email; auth itself is done by oauth2-proxy. Not a security use. + email_hash = hashlib.md5(email_header.encode(), + usedforsecurity=False).hexdigest() + user_hash = email_hash[:common_utils.USER_HASH_LENGTH] return models.User(id=user_hash, name=email_header, user_type=models.UserType.SSO.value) diff --git a/sky/server/server.py b/sky/server/server.py index 4ec8ce1a86d..995ef89764c 100644 --- a/sky/server/server.py +++ b/sky/server/server.py @@ -282,8 +282,11 @@ def _extract_user_from_header( if not user_name: return None + # MD5 only derives a stable user id from the (non-secret) user name; + # not a security use. user_hash = hashlib.md5( - user_name.encode()).hexdigest()[:common_utils.USER_HASH_LENGTH] + user_name.encode(), + usedforsecurity=False).hexdigest()[:common_utils.USER_HASH_LENGTH] if proxy_config.enabled: return models.User(id=user_hash, name=user_name, diff --git a/sky/users/permission.py b/sky/users/permission.py index 82fb2877e50..a07be1c9b05 100644 --- a/sky/users/permission.py +++ b/sky/users/permission.py @@ -182,8 +182,10 @@ def _maybe_initialize_basic_auth_user(self) -> None: return username, password = basic_auth.split(':', 1) if username and password: - user_hash = hashlib.md5( - username.encode()).hexdigest()[:common_utils.USER_HASH_LENGTH] + # MD5 only derives a stable user id from the (non-secret) + # username; the password is checked separately. Not a security use. + user_hash = hashlib.md5(username.encode(), usedforsecurity=False + ).hexdigest()[:common_utils.USER_HASH_LENGTH] user_info = global_user_state.get_user(user_hash) if user_info: logger.debug(f'Basic auth user {username} already exists') diff --git a/sky/users/server.py b/sky/users/server.py index 7cd42ee1726..8258702a2ff 100644 --- a/sky/users/server.py +++ b/sky/users/server.py @@ -287,8 +287,12 @@ def user_create(user_create_body: payloads.UserCreateBody) -> None: # Create user password_hash = server_common.crypt_ctx.hash(password) + # MD5 here only derives a stable user identifier from the (non-secret) + # username; it is not used for any security purpose (passwords use bcrypt + # via crypt_ctx). user_hash = hashlib.md5( - username.encode()).hexdigest()[:common_utils.USER_HASH_LENGTH] + username.encode(), + usedforsecurity=False).hexdigest()[:common_utils.USER_HASH_LENGTH] with _user_lock(user_hash): # Check if user already exists if global_user_state.get_user_by_name(username): @@ -627,8 +631,11 @@ def user_import(user_import_body: payloads.UserImportBody) -> Dict[str, Any]: # Password is plain text, hash it password_hash = server_common.crypt_ctx.hash(password) - user_hash = hashlib.md5( - username.encode()).hexdigest()[:common_utils.USER_HASH_LENGTH] + # MD5 only derives a stable user identifier from the (non-secret) + # username; not a security use (passwords use bcrypt via crypt_ctx). + user_hash = hashlib.md5(username.encode(), + usedforsecurity=False).hexdigest() + user_hash = user_hash[:common_utils.USER_HASH_LENGTH] with _user_lock(user_hash): global_user_state.add_or_update_user( diff --git a/sky/utils/command_runner.py b/sky/utils/command_runner.py index 344a65eb789..44f981ac520 100644 --- a/sky/utils/command_runner.py +++ b/sky/utils/command_runner.py @@ -875,8 +875,8 @@ def git_clone( raise exceptions.CommandError(1, '', error_msg, None) # Remote script path (use a unique name to avoid conflicts) - script_hash = hashlib.md5( - f'{self.node_id}_{target_dir}'.encode()).hexdigest()[:8] + script_hash = hashlib.md5(f'{self.node_id}_{target_dir}'.encode(), + usedforsecurity=False).hexdigest()[:8] remote_script_path = f'/tmp/sky_git_clone_{script_hash}.sh' # Step 1: Transfer the script to remote machine using rsync @@ -992,7 +992,8 @@ def __init__( self.ssh_private_key = ssh_private_key self.ssh_control_name = ( None if ssh_control_name is None else hashlib.md5( - ssh_control_name.encode()).hexdigest()[:_HASH_MAX_LENGTH]) + ssh_control_name.encode(), + usedforsecurity=False).hexdigest()[:_HASH_MAX_LENGTH]) self._ssh_proxy_command = ssh_proxy_command self._ssh_proxy_jump = ssh_proxy_jump self.disable_control_master = ( diff --git a/sky/utils/common_utils.py b/sky/utils/common_utils.py index 027a59d5bca..71e7034f6a7 100644 --- a/sky/utils/common_utils.py +++ b/sky/utils/common_utils.py @@ -91,7 +91,10 @@ def is_valid_user_hash(user_hash: Optional[str]) -> bool: def generate_user_hash() -> str: """Generates a unique user-machine specific hash.""" hash_str = user_and_hostname_hash() - user_hash = hashlib.md5(hash_str.encode()).hexdigest()[:USER_HASH_LENGTH] + # MD5 only derives a stable machine-specific identifier, not a security + # use. + user_hash = hashlib.md5( + hash_str.encode(), usedforsecurity=False).hexdigest()[:USER_HASH_LENGTH] if not is_valid_user_hash(user_hash): # A fallback in case the hash is invalid. user_hash = uuid.uuid4().hex[:USER_HASH_LENGTH] @@ -297,7 +300,9 @@ def make_cluster_name_on_cloud(display_name: str, if truncate_cluster_name.endswith('-'): truncate_cluster_name = truncate_cluster_name.rstrip('-') assert truncate_cluster_name_length > 0, (cluster_name_on_cloud, max_length) - display_name_hash = hashlib.md5(display_name.encode()).hexdigest() + # MD5 only derives a short suffix for the cluster name, not a security use. + display_name_hash = hashlib.md5(display_name.encode(), + usedforsecurity=False).hexdigest() # Use base36 to reduce the length of the hash. display_name_hash = base36_encode(display_name_hash) return (f'{truncate_cluster_name}' @@ -650,7 +655,9 @@ def user_and_hostname_hash() -> str: The reason is AWS security group names are derived from this string, and thus changing the SG name makes these clusters unrecognizable. """ - hostname_hash = hashlib.md5(socket.gethostname().encode()).hexdigest()[-4:] + # MD5 only derives a short hostname suffix, not a security use. + hostname_hash = hashlib.md5(socket.gethostname().encode(), + usedforsecurity=False).hexdigest()[-4:] return f'{getpass.getuser()}-{hostname_hash}' diff --git a/sky/utils/kubernetes/gpu_labeler.py b/sky/utils/kubernetes/gpu_labeler.py index 227847cd50c..5de797139f9 100644 --- a/sky/utils/kubernetes/gpu_labeler.py +++ b/sky/utils/kubernetes/gpu_labeler.py @@ -71,7 +71,8 @@ def cleanup(context: Optional[str] = None) -> Tuple[bool, str]: def get_node_hash(node_name: str): # Generates a 32 character md5 hash from a string - md5_hash = hashlib.md5(node_name.encode()).hexdigest() + md5_hash = hashlib.md5(node_name.encode(), + usedforsecurity=False).hexdigest() return md5_hash[:32]