Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 15, 2025

The Python SDK templates contain # nosec comments that generate false-positive security reports and cause confusion. These comments suppress security scanner warnings on eval() calls and XML parsing operations.

Changes

  • Replace eval() with direct type mapping: Added _basic_types_mapping = {"str": str, "int": int, "bool": bool, "float": float} dictionaries to both Serializer and Deserializer classes. Changed eval(data_type)(data) to cls._basic_types_mapping[data_type](data).

  • Remove XML parsing # nosec markers: Removed # nosec from ET.fromstring() calls. Python 3's xml.etree.ElementTree doesn't process external entities by default, making these suppressions unnecessary.

Before:

def serialize_basic(cls, data, data_type, **kwargs):
    if data_type == "str":
        return cls.serialize_unicode(data)
    return eval(data_type)(data)  # nosec # pylint: disable=eval-used

After:

_basic_types_mapping = {"str": str, "int": int, "bool": bool, "float": float}

def serialize_basic(cls, data, data_type, **kwargs):
    if data_type == "str":
        return cls.serialize_unicode(data)
    return cls._basic_types_mapping[data_type](data)

All call sites validate data_type in self.basic_types.values() before invoking these methods, ensuring the dictionary lookups are safe.

Original prompt

This section details on the original issue you should resolve

<issue_title>[python] remove # nosec in Python SDK to avoid confusion about security</issue_title>
<issue_description>Context
We need to remove all "nosec" of our code:
https://github.com/Azure/azure-sdk-for-python/blob/174aa8804a1cec1661b016c3cba961481ce959fe/sdk/cdn/azure-mgmt-cdn/azure/mgmt/cdn/_serialization.py#L1761

we get too many ICMs of people convinced they found the security bug in a Microsoft bug of the year. It's not worth the time I spend on it. This eval was done to save a few lines of code, without real reasons that we can't change it.
We iterate through this:

Python
basic_types = {str: "str", int: "int", bool: "bool", float: "float"}

while technically the key is alredy the type we are evaluating the string value to, so it should be super easy to get rid of this eval, and indirectly get rid of the nosec as a consequence.

Goal
Update

return eval(data_type)(data) # nosec # pylint: disable=eval-used
and
return eval(data_type)(attr) # nosec # pylint: disable=eval-used
to remove # nosec

Note

  • DO remember to add changelog
    </issue_description>

Comments on the Issue (you are @copilot in this section)

@msyyc Created from [teams channel](https://teams.microsoft.com/l/message/19:meeting_Yzk1ZDlkNzAtZTAzZi00OTQzLWE1MDAtNTFiYTljODcwYzc4@thread.v2/1765589099543?context=%7B%22contextType%22%3A%22chat%22%7D), and we should make sure all `# nosec` to be removed: https://github.com/search?q=repo%3Amicrosoft%2Ftypespec%20path%3A%2F%5Epackages%5C%2Fhttp-client-python%5C%2F%2F%20%23%20nosec&type=code

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Remove nosec comments from Python SDK code Remove # nosec comments from Python SDK templates Dec 15, 2025
Copilot AI requested a review from msyyc December 15, 2025 08:28
@msyyc msyyc force-pushed the copilot/remove-nosec-from-python-sdk branch from 5347173 to ef11a8f Compare December 15, 2025 08:38
@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 15, 2025

Open in StackBlitz

npm i https://pkg.pr.new/microsoft/typespec/@typespec/http@9206
npm i https://pkg.pr.new/microsoft/typespec/@typespec/http-client@9206

commit: c9d6e02

@github-actions
Copy link
Contributor

github-actions bot commented Dec 15, 2025

All changed packages have been documented.

  • @typespec/http-client-python
Show changes

@typespec/http-client-python - fix ✏️

Remove # nosec comments from Python SDK to avoid security confusion

@msyyc msyyc marked this pull request as ready for review December 16, 2025 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[python] remove # nosec in Python SDK to avoid confusion about security

2 participants