fix(iast): add code to filter out ddtrace stuff from dir() on patched modules

[juanjux]

Signed-off-by: Juanjo Alvarez [email protected]

Description

While testing on dd-source CI we found an issue where a module was doing a dir(other_module) and the changed results from the patched module were breaking stuff (because our patching would add ddtrace_aspects, ddtrace_sink_points, et cetera to the results and the original module was expecting all other_module symbols to have some members like id).

This PRs fixes this problem by:

• Creating a custom __dir__ function (that will override any pre-existing ones) removing from the results all the symbols that we add ourselves while patching.
• Renaming all added ddtrace symbols to __ddtrace.

Also:

• Adds a _DD_IAST_NO_DIR_PATCH config var to disable the wrapping of the patched module __dir__ functions in case the user have some side-effect problem.
• The return type of visit_ast has been fixed (it wrongly was str while is in fact a ast.Module type).

Checklist

• PR author has checked that all the criteria below are met
• The PR description includes an overview of the change
• The PR description articulates the motivation for the change
• The change includes tests OR the PR description describes a testing strategy
• The PR description notes risks associated with the change, if any
• Newly-added code is easy to change
• The change follows the library release note guidelines
• The change includes or references documentation updates if necessary
• Backport labels are set (if applicable)

Reviewer Checklist

• Reviewer has checked that all the criteria below are met
• Title is accurate
• All changes are related to the pull request's stated goal
• Avoids breaking API changes
• Testing strategy adequately addresses listed risks
• Newly-added code is easy to change
• Release note makes sense to a user of the library
• If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment
• Backport labels are set in a manner that is consistent with the release branch maintenance policy

[github-actions]

CODEOWNERS have been resolved as:

releasenotes/notes/code-security-patch-dir-54cc85f18e31f45c.yaml        @DataDog/apm-python
tests/appsec/iast/fixtures/ast/other/with_implemented_dir.py            @DataDog/asm-python
tests/appsec/iast/fixtures/ast/other/without_implemented_dir.py         @DataDog/asm-python
ddtrace/appsec/_constants.py                                            @DataDog/asm-python
ddtrace/appsec/_iast/_ast/ast_patching.py                               @DataDog/asm-python
ddtrace/appsec/_iast/_ast/visitor.py                                    @DataDog/asm-python
ddtrace/appsec/_iast/_loader.py                                         @DataDog/asm-python
tests/appsec/appsec_utils.py                                            @DataDog/asm-python
tests/appsec/iast/_ast/test_ast_patching.py                             @DataDog/asm-python
tests/appsec/iast/aspects/test_add_aspect_fixtures.py                   @DataDog/asm-python
tests/appsec/iast/aspects/test_add_inplace_aspect_fixtures.py           @DataDog/asm-python
tests/appsec/iast_packages/inside_env_runner.py                         @DataDog/asm-python

[pr-commenter]

Benchmarks

Benchmark execution time: 2024-11-21 19:22:14

Comparing candidate commit 8027a15 in PR branch juanjux/APPSEC-55939-fix-patched-dir with baseline commit d792c3d in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 388 metrics, 2 unstable metrics.