Skip to content

Conversation

@ReaNAiveD
Copy link
Member

@ReaNAiveD ReaNAiveD commented Dec 9, 2025

Related command

az login

Description

The pickle module poses a risk of arbitrary code execution if its data (~/.azure/msal_http_cache.bin) is tampered with by an attacker. This PR change the cache storage from pickle to json.

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change
[Component Name 2] az command b: Add some customer-facing feature


This checklist is used to make sure that common guidelines for a pull request are followed.

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Dec 9, 2025

️✔️AzureCLI-FullTest
️✔️acr
️✔️latest
️✔️3.12
️✔️3.13
️✔️acs
️✔️latest
️✔️3.12
️✔️3.13
️✔️advisor
️✔️latest
️✔️3.12
️✔️3.13
️✔️ams
️✔️latest
️✔️3.12
️✔️3.13
️✔️apim
️✔️latest
️✔️3.12
️✔️3.13
️✔️appconfig
️✔️latest
️✔️3.12
️✔️3.13
️✔️appservice
️✔️latest
️✔️3.12
️✔️3.13
️✔️aro
️✔️latest
️✔️3.12
️✔️3.13
️✔️backup
️✔️latest
️✔️3.12
️✔️3.13
️✔️batch
️✔️latest
️✔️3.12
️✔️3.13
️✔️batchai
️✔️latest
️✔️3.12
️✔️3.13
️✔️billing
️✔️latest
️✔️3.12
️✔️3.13
️✔️botservice
️✔️latest
️✔️3.12
️✔️3.13
️✔️cdn
️✔️latest
️✔️3.12
️✔️3.13
️✔️cloud
️✔️latest
️✔️3.12
️✔️3.13
️✔️cognitiveservices
️✔️latest
️✔️3.12
️✔️3.13
️✔️compute_recommender
️✔️latest
️✔️3.12
️✔️3.13
️✔️computefleet
️✔️latest
️✔️3.12
️✔️3.13
️✔️config
️✔️latest
️✔️3.12
️✔️3.13
️✔️configure
️✔️latest
️✔️3.12
️✔️3.13
️✔️consumption
️✔️latest
️✔️3.12
️✔️3.13
️✔️container
️✔️latest
️✔️3.12
️✔️3.13
️✔️containerapp
️✔️latest
️✔️3.12
️✔️3.13
️✔️core
️✔️latest
️✔️3.12
️✔️3.13
️✔️cosmosdb
️✔️latest
️✔️3.12
️✔️3.13
️✔️databoxedge
️✔️latest
️✔️3.12
️✔️3.13
️✔️dls
️✔️latest
️✔️3.12
️✔️3.13
️✔️dms
️✔️latest
️✔️3.12
️✔️3.13
️✔️eventgrid
️✔️latest
️✔️3.12
️✔️3.13
️✔️eventhubs
️✔️latest
️✔️3.12
️✔️3.13
️✔️feedback
️✔️latest
️✔️3.12
️✔️3.13
️✔️find
️✔️latest
️✔️3.12
️✔️3.13
️✔️hdinsight
️✔️latest
️✔️3.12
️✔️3.13
️✔️identity
️✔️latest
️✔️3.12
️✔️3.13
️✔️iot
️✔️latest
️✔️3.12
️✔️3.13
️✔️keyvault
️✔️latest
️✔️3.12
️✔️3.13
️✔️lab
️✔️latest
️✔️3.12
️✔️3.13
️✔️managedservices
️✔️latest
️✔️3.12
️✔️3.13
️✔️maps
️✔️latest
️✔️3.12
️✔️3.13
️✔️marketplaceordering
️✔️latest
️✔️3.12
️✔️3.13
️✔️monitor
️✔️latest
️✔️3.12
️✔️3.13
️✔️mysql
️✔️latest
️✔️3.12
️✔️3.13
️✔️netappfiles
️✔️latest
️✔️3.12
️✔️3.13
️✔️network
️✔️latest
️✔️3.12
️✔️3.13
️✔️policyinsights
️✔️latest
️✔️3.12
️✔️3.13
️✔️privatedns
️✔️latest
️✔️3.12
️✔️3.13
️✔️profile
️✔️latest
️✔️3.12
️✔️3.13
️✔️rdbms
️✔️latest
️✔️3.12
️✔️3.13
️✔️redis
️✔️latest
️✔️3.12
️✔️3.13
️✔️relay
️✔️latest
️✔️3.12
️✔️3.13
️✔️resource
️✔️latest
️✔️3.12
️✔️3.13
️✔️role
️✔️latest
️✔️3.12
️✔️3.13
️✔️search
️✔️latest
️✔️3.12
️✔️3.13
️✔️security
️✔️latest
️✔️3.12
️✔️3.13
️✔️servicebus
️✔️latest
️✔️3.12
️✔️3.13
️✔️serviceconnector
️✔️latest
️✔️3.12
️✔️3.13
️✔️servicefabric
️✔️latest
️✔️3.12
️✔️3.13
️✔️signalr
️✔️latest
️✔️3.12
️✔️3.13
️✔️sql
️✔️latest
️✔️3.12
️✔️3.13
️✔️sqlvm
️✔️latest
️✔️3.12
️✔️3.13
️✔️storage
️✔️latest
️✔️3.12
️✔️3.13
️✔️synapse
️✔️latest
️✔️3.12
️✔️3.13
️✔️telemetry
️✔️latest
️✔️3.12
️✔️3.13
️✔️util
️✔️latest
️✔️3.12
️✔️3.13
️✔️vm
️✔️latest
️✔️3.12
️✔️3.13

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Dec 9, 2025

️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes

@yonzhan
Copy link
Collaborator

yonzhan commented Dec 9, 2025

Thank you for your contribution! We will review the pull request and get back to you soon.

@github-actions
Copy link

github-actions bot commented Dec 9, 2025

The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR.

Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions).
After that please run the following commands to enable git hooks:

pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>

from .binary_cache import BinaryCache
http_cache = BinaryCache(self._msal_http_cache_file)
from .json_cache import JsonCache
http_cache = JsonCache(self._msal_http_cache_file)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The BinaryCache is designed for saving anything, so the name BinaryCache is accurate.

However, the new JsonCache can only handle NormalizedResponse, so the name is not accurate. It should be something like NormalizedResponseJsonCache.

Comment on lines +50 to +53
response = NormalizedResponse.__new__(NormalizedResponse)
response.status_code = response_dict["status_code"]
response.text = response_dict["text"]
response.headers = response_dict["headers"]
Copy link
Member

@jiasli jiasli Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel it is appropriate for Azure CLI as a downstream application of MSAL to serialize MSAL's HTTP cache as JSON. Azure CLI doesn't know which fields of the NormalizedResponse should be persisted. The HTTP cache should be treated as an opaque box.

MSAL itself should provide a serializable HTTP cache, similar to the token cache msal.token_cache.SerializableTokenCache or even msal_extensions.token_cache.PersistedTokenCache. Otherwise, both SDK and CLI, or even direct MSAL users need to implement their own serialization for MSAL HTTP cache.

Copy link
Member

@jiasli jiasli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see above comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Account az login/account Auto-Assign Auto assign by bot Core CLI core infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants