Repository navigation
refactor: move components shared between aap and iast to internal.security - #21162
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🎉 All green!🧪 All tests passed 🔗 Commit SHA: dd3b972 | Docs | View more details | Give us feedback! |
Codeowners resolved asResolved from the full PR diff against |
Dependency direction analysis
|
There was a problem hiding this comment.
The architecture fixture excludes the new security namespace, making the relocated stacktrace extension’s loading assertion impossible to satisfy when IAST is enabled on a supported non-Lambda platform.
🤖 Bits Code Review · Commit 08ffb01
BenchmarksBenchmark execution time: 2026-10-28 16:53:51 Comparing candidate commit dd3b972 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 609 metrics, 10 unstable metrics, 1 flaky benchmarks without significant changes.
|
florentinl
left a comment
There was a problem hiding this comment.
That's awesome. Thank you for the refactor. Two small things:
- Can you add
ddtrace/internal/security/*to the security test and benchmark mappings. Changes limited to these files currently select only internal and detect_global_locks, skipping AAP/IAST/SCA coverage. - Would you mind adding us (@DataDog/asm-python) as the codeowners of the
ddtrace/internal/securityfolder so that we keep the ownership of this code ?
There was a problem hiding this comment.
More details
Entry-span lookup preserves its existing context and span fallbacks, and the relocated native extension’s imports match its build target. The specialist returned no actionable findings.
🤖 Bits Code Review · Commit dd943ea
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0205f01f5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
/merge |
|
View all feedbacks in Devflow UI.
It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.
The expected merge time in
Build pipeline has failing jobs for eac9adb: What to do next?
This build will be kept for 3h more. During that period, you can retry the pipeline or the failing jobs and then re-run the same command! |
Description
Removes the 10 dependency direction violations from IAST (
product:iast) into AAP (product:aap). Every one of them pointed at code both products use, so that code now lives in a newddtrace/internal/security/package, which both may import. Being part ofddtrace.internal, the package imports no product code.Extracted from AAP modules:
STACK_TRACEfromddtrace/appsec/_constants.py→ddtrace/internal/security/_constants.pyget_entry_spanand the_ASM_CONTEXTcore-context key fromddtrace/appsec/_asm_request_context.py→ddtrace/internal/security/_context.py.report_stackneeds the entry span; it now looks it up through the shared key instead of importing_asm_request_context._asm_manual_keepfromddtrace/appsec/_trace_utils.py→ddtrace/internal/security/_trace_utils.pyThe original AAP modules re-export the extracted names, so existing AAP call sites are unchanged. IAST now imports everything from
ddtrace.internal.security.Risks
ddtrace.appsec._shared._stacktracetoddtrace.internal.security._stacktrace. Packaging that removes the extension by its old path (as the AWS Lambda layer does to keep it out of serverless deployments) needs to remove it at the new path; otherwise the unused extension is shipped. Nothing imports it at serverless startup, so this affects package size only. There's a PR updating the exclusion rule in the Lambda layer here: chore: exclude stacktrace C extension's new location datadog-lambda-python#875