From ccc8a9eb5ce379799dd3151273977a926a68ab1f Mon Sep 17 00:00:00 2001 From: Alberto Vara Date: Mon, 14 Oct 2024 14:50:23 +0200 Subject: [PATCH] fix: enable fastapi header source --- .github/workflows/test_frameworks.yml | 68 +++++++++++++++++++ ddtrace/appsec/_iast/_patch.py | 44 +++--------- .../fastapi/test_fastapi_appsec_iast.py | 6 -- 3 files changed, 78 insertions(+), 40 deletions(-) diff --git a/.github/workflows/test_frameworks.yml b/.github/workflows/test_frameworks.yml index 6ea2513a76..294536970b 100644 --- a/.github/workflows/test_frameworks.yml +++ b/.github/workflows/test_frameworks.yml @@ -416,6 +416,74 @@ jobs: if: needs.needs-run.outputs.outcome == 'success' run: cat debugger-expl.txt + fastapi-testsuite-0_115: + strategy: + matrix: + include: + - suffix: Profiling + profiling: 1 + iast: 0 + appsec: 0 + - suffix: IAST + profiling: 0 + iast: 1 + appsec: 0 + - suffix: APPSEC + profiling: 0 + iast: 0 + appsec: 1 + - suffix: Tracer only + profiling: 0 + iast: 0 + appsec: 0 + name: FastAPI 0.115 (with ${{ matrix.suffix }}) + runs-on: ubuntu-latest + needs: needs-run + env: + DD_TESTING_RAISE: true + DD_PROFILING_ENABLED: ${{ matrix.profiling }} + DD_IAST_ENABLED: ${{ matrix.iast }} + DD_APPSEC_ENABLED: ${{ matrix.appsec }} + CMAKE_BUILD_PARALLEL_LEVEL: 12 + DD_DEBUGGER_EXPL_OUTPUT_FILE: debugger-expl.txt + defaults: + run: + working-directory: fastapi + steps: + - uses: actions/setup-python@v5 + if: needs.needs-run.outputs.outcome == 'success' + with: + python-version: '3.11' + - uses: actions/checkout@v4 + if: needs.needs-run.outputs.outcome == 'success' + with: + path: ddtrace + - uses: actions/checkout@v4 + if: needs.needs-run.outputs.outcome == 'success' + with: + repository: tiangolo/fastapi + ref: 0.115.2 + path: fastapi + - uses: actions/cache@v3.3.1 + if: needs.needs-run.outputs.outcome == 'success' + id: cache + with: + path: ${{ env.pythonLocation }} + key: ${{ runner.os }}-python-${{ env.pythonLocation }}-fastapi + - name: Install Dependencies + if: steps.cache.outputs.cache-hit != 'true' && needs.needs-run.outputs.outcome == 'success' + run: pip install -e .[all,dev,doc,test] + - name: Inject ddtrace + if: needs.needs-run.outputs.outcome == 'success' + run: pip install ../ddtrace + - name: Test + if: needs.needs-run.outputs.outcome == 'success' + # https://github.com/tiangolo/fastapi/pull/10876 + run: PYTHONPATH=../ddtrace/tests/debugging/exploration/ ddtrace-run pytest -p no:warnings tests -k 'not test_warn_duplicate_operation_id' + - name: Debugger exploration results + if: needs.needs-run.outputs.outcome == 'success' + run: cat debugger-expl.txt + flask-testsuite: strategy: matrix: diff --git a/ddtrace/appsec/_iast/_patch.py b/ddtrace/appsec/_iast/_patch.py index 214ae13efe..c796e3b850 100644 --- a/ddtrace/appsec/_iast/_patch.py +++ b/ddtrace/appsec/_iast/_patch.py @@ -91,25 +91,6 @@ def _patched_dictionary(origin_key, origin_value, original_func, instance, args, return taint_structure(result, origin_key, origin_value, override_pyobject_tainted=True) -def _patched_fastapi_function(origin, original_func, instance, args, kwargs): - result = original_func(*args, **kwargs) - - if _is_iast_enabled() and is_iast_request_enabled(): - try: - from ._taint_tracking import is_pyobject_tainted - - if not is_pyobject_tainted(result): - from ._taint_tracking import origin_to_str - from ._taint_tracking import taint_pyobject - - return taint_pyobject( - pyobject=result, source_name=origin_to_str(origin), source_value=result, source_origin=origin - ) - except Exception: - log.debug("Unexpected exception while tainting pyobject", exc_info=True) - return result - - def _on_iast_fastapi_patch(): from ddtrace.appsec._iast._taint_tracking import OriginType @@ -136,21 +117,16 @@ def _on_iast_fastapi_patch(): _set_metric_iast_instrumented_source(OriginType.PARAMETER) # Header sources - # try_wrap_function_wrapper( - # "starlette.datastructures", - # "Headers.__getitem__", - # functools.partial(if_iast_taint_returned_object_for, OriginType.HEADER), - # ) - # try_wrap_function_wrapper( - # "starlette.datastructures", - # "Headers.get", - # functools.partial(if_iast_taint_returned_object_for, OriginType.HEADER), - # ) - # try_wrap_function_wrapper( - # "fastapi", - # "Header", - # functools.partial(_patched_fastapi_function, OriginType.HEADER), - # ) + try_wrap_function_wrapper( + "starlette.datastructures", + "Headers.__getitem__", + functools.partial(if_iast_taint_returned_object_for, OriginType.HEADER), + ) + try_wrap_function_wrapper( + "starlette.datastructures", + "Headers.get", + functools.partial(if_iast_taint_returned_object_for, OriginType.HEADER), + ) # _set_metric_iast_instrumented_source(OriginType.HEADER) # Path source try_wrap_function_wrapper("starlette.datastructures", "URL.__init__", _iast_instrument_starlette_url) diff --git a/tests/contrib/fastapi/test_fastapi_appsec_iast.py b/tests/contrib/fastapi/test_fastapi_appsec_iast.py index 3856c9a8ac..63447ada71 100644 --- a/tests/contrib/fastapi/test_fastapi_appsec_iast.py +++ b/tests/contrib/fastapi/test_fastapi_appsec_iast.py @@ -104,9 +104,6 @@ async def test_route(request: Request): assert result["ranges_origin"] == "http.request.parameter" -@pytest.mark.skip( - reason="The refactor 10988 introduced (or disclosed) a FastAPI error in headers: https://github.com/DataDog/dd-trace-py/actions/runs/11290104662/job/31401282016" -) def test_header_value_source(fastapi_application, client, tracer, test_spans): @fastapi_application.get("/index.html") async def test_route(request: Request): @@ -142,9 +139,6 @@ async def test_route(request: Request): assert result["ranges_origin"] == "http.request.header" -@pytest.mark.skip( - reason="The refactor 10988 introduced (or disclosed) a FastAPI error in headers: https://github.com/DataDog/dd-trace-py/actions/runs/11290104662/job/31401282016" -) @pytest.mark.skipif(sys.version_info < (3, 9), reason="typing.Annotated was introduced on 3.9") @pytest.mark.skipif(fastapi_version < (0, 95, 0), reason="Header annotation doesn't work on fastapi 94 or lower") def test_header_value_source_typing_param(fastapi_application, client, tracer, test_spans):