Skip to content

fix(llmobs): stop recording hosted MCP credentials in OpenAI Agents spans - #21463

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits into
mainfrom
alberto.vara/APPSEC-71467-llmobs-openai-agents-mcp-credentials
Nov 4, 2026
Merged

gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits into
mainfrom
alberto.vara/APPSEC-71467-llmobs-openai-agents-mcp-credentials

Conversation

@avara1986

@avara1986 avara1986 commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Description

Jira: APPSEC-71467

The LLM Observability OpenAI Agents integration recorded credentials from agent tool and model configuration:

  1. Hosted MCP tools in the agent manifest (OpenAIAgentsIntegration._extract_tools_from_agent): the hosted_mcp tool's tool_config was copied verbatim. That config is the application's own, so it held headers, authorization and the full server_url in clear text.
  2. Code interpreter and image generation tools in the agent manifest: their tool_config was also copied verbatim, including container network_policy.domain_secrets[].value.
  3. Model settings in the agent manifest (_extract_model_settings_from_agent): ModelSettings was recorded in full, including extra_headers, extra_query, extra_body and extra_args, which can carry proxy or provider credentials.
  4. Response span metadata (OaiSpanAdapter.llmobs_metadata): tools was copied from response.tools. OpenAI redacts header values in that echo but returns server_url exactly as sent, so userinfo, path tokens (for example /api/mcp/s/<token>/mcp) and query keys were recorded.

Changes:

  • safe_tool_config() records tool configurations safely, in both the manifest and response metadata.
    • Hosted MCP tools keep only type, server_label, server_description, connector_id, require_approval, allowed_tools, and server_url reduced to its origin (scheme://host[:port]).
    • Every other tool keeps its configuration, but each container network policy domain secret keeps only its domain and name.
    • In response metadata this applies to every tool, so code_interpreter and shell containers are covered if OpenAI echoes their policies.
  • Model settings are filtered with the existing ALLOWED_MODEL_SETTINGS_KEYS and is_flat_scalar_value(), as the pydantic_ai integration already does.
  • A new shared helper, ddtrace.internal.utils.http.url_origin, provides the origin.
    • It drops userinfo, path, query and fragment, lowercases scheme and host, drops default ports, and returns None for URLs it cannot parse.
    • feat(aiguard): evaluate OpenAI hosted MCP tool calls #20850 adds the identical helper for AI Guard. Merging both PRs, in either order, is conflict-free and yields a single copy.

Testing

  • tests/contrib/openai_agents/test_openai_agents_llmobs.py, all built from real SDK types (agents.HostedMCPTool, agents.CodeInterpreterTool, agents.ModelSettings, and the OpenAI Mcp, CodeInterpreter and FunctionShellTool tools):

    • hosted MCP credentials (password in userinfo, path token, API key in the query, bearer header, OAuth token, cookie) never reach the manifest or response metadata;
    • container domain secret values never reach the manifest or response metadata;
    • extra_headers, extra_query, extra_body and extra_args never reach the manifest, while inference params do;
    • URLs that cannot be parsed are omitted.

    Reverting any of the fixes makes the matching test fail.

  • tests/tracer/test_utils.py: url_origin cases (path tokens, userinfo, query, fragment, default ports, IPv6, invalid input).

  • Locally:

    • llmobs::openai_agents passes on openai-agents 0.0.x (Python 3.9), 0.14 and latest: 44 passed, 2 skipped each.
    • llmobs::openai (openai 1.66): 282 passed.
    • tests/tracer/test_utils.py passes.
    • scripts/lint checks is clean.

Risks

  • Agent manifests and response metadata carry fewer fields:

    • hosted MCP entries lose headers, authorization and the URL path;
    • domain secrets lose their value;
    • model_settings keeps only inference params, so fields such as reasoning or truncation are no longer shown.

    Queries matching on those fields will no longer match.

Additional Notes

  • The plain OpenAI integration is not affected: hosted MCP tools produce no tool definition there and tools is not part of its metadata.
  • The llmobs-integrations skill guide now says to record tool and model configurations through these helpers rather than verbatim.

🤖 Generated with Claude Code

…pans

OpenAI Agents agent manifests copied the hosted MCP tool_config verbatim,
including headers, authorization and the full server URL, and response span
metadata copied the server_url OpenAI echoes back. Record an allowlisted view
of hosted MCP tools instead, with server_url reduced to its origin through
the new ddtrace.internal.utils.http.url_origin helper.

APPSEC-71467

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Dependency direction analysis

⚠️ Existing dependency direction violations

There are 102 dependency direction violations that already exist on the base branch and have not been changed by this PR.

Show existing violations (showing 5 of 102 highest severity)
ddtrace.internal.tracemethods -×-> ddtrace.trace  (internal-core -> product:tracing, score=122)
ddtrace.llmobs._integrations.bedrock_agents -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=120)
ddtrace.llmobs._integrations.openai_agents -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=120)
ddtrace.llmobs._integrations.crewai -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=120)
ddtrace.llmobs._integrations.google_adk -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=120)

To see all violations, download the layers-base.json and layers-pr.json artifacts from this CI job and run:

uv run --script scripts/import-analysis/layers.py compare layers-base.json layers-pr.json

@cit-pr-commenter-54b7da

Copy link
Copy Markdown

Codeowners resolved as

Resolved from the full PR diff against main using the target branch CODEOWNERS file.
CODEOWNERS team requests not listed below are not required by the current file set.

.agents/skills/llmobs-integrations/references/implementation-guide.md   @DataDog/ml-observability
ddtrace/internal/utils/http.py                                          @DataDog/apm-core-python
ddtrace/llmobs/_integrations/openai_agents.py                           @DataDog/ml-observability
ddtrace/llmobs/_integrations/utils.py                                   @DataDog/ml-observability
releasenotes/notes/fix-llmobs-openai-agents-mcp-credentials-c6c2e23460dd3806.yaml  @DataDog/apm-python
tests/contrib/openai_agents/test_openai_agents_llmobs.py                @DataDog/ml-observability
tests/tracer/test_utils.py                                              @DataDog/apm-sdk-capabilities

@pr-commenter

pr-commenter Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-11-03 15:15:25

Comparing candidate commit 15747b7 in PR branch alberto.vara/APPSEC-71467-llmobs-openai-agents-mcp-credentials with baseline commit 11146fd in branch main.

📊 Benchmarking dashboard

Found 0 performance improvements and 0 performance regressions! Performance is the same for 600 metrics, 10 unstable metrics, 1 flaky benchmarks without significant changes.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

Unstable benchmarks

These benchmarks have a confidence interval too wide to call a change; treat them as noise rather than signal.

scenario:coreapiscenario-context_with_data_listeners

  • unstable execution_time [-706.070ns; +741.604ns] or [-6.590%; +6.922%]

scenario:coreapiscenario-core_dispatch_1_listener

  • unstable execution_time [-34.969ns; +36.166ns] or [-5.072%; +5.246%]

scenario:coreapiscenario-core_dispatch_50_listeners

  • unstable execution_time [-1903.201ns; +1903.777ns] or [-9.539%; +9.542%]

scenario:coreapiscenario-core_dispatch_exception_listeners

  • unstable execution_time [-1907.110ns; +1756.456ns] or [-9.879%; +9.099%]

scenario:coreapiscenario-core_dispatch_listeners

  • unstable execution_time [-376.531ns; +379.717ns] or [-8.634%; +8.707%]

scenario:coreapiscenario-core_dispatch_no_args_listeners

  • unstable execution_time [-227.119ns; +222.151ns] or [-8.396%; +8.212%]

scenario:coreapiscenario-core_dispatch_with_results_1_listener

  • unstable execution_time [-79.178ns; +97.995ns] or [-5.603%; +6.934%]

scenario:coreapiscenario-core_dispatch_with_results_50_listeners

  • unstable execution_time [-4633.383ns; +4919.965ns] or [-9.236%; +9.807%]

scenario:coreapiscenario-core_dispatch_with_results_listeners

  • unstable execution_time [-1056.548ns; +951.620ns] or [-9.894%; +8.911%]

scenario:packagesupdateimporteddependencies-import_many_stdlib_cached

  • unstable execution_time [-61.809µs; +51.505µs] or [-10.484%; +8.736%]

Known flaky benchmarks

These benchmarks are marked as flaky and will not trigger a failure. Modify FLAKY_BENCHMARKS_REGEX to control which benchmarks are marked as flaky.

Known flaky benchmarks without significant changes:

  • scenario:packagespackageforrootmodulemapping-cache_off

@avara1986
avara1986 marked this pull request as ready for review November 3, 2026 11:12
@avara1986
avara1986 requested review from a team as code owners November 3, 2026 11:12
@avara1986
avara1986 requested review from juanjux, mtoffl01 and r1viollet and removed request for a team November 3, 2026 11:12
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T14:39:25.003895Z d87ea70 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@datadog-prod-us1-5 datadog-prod-us1-5 Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bits Code Review: PASS

More details

Static review and isolated execution checks support removal of MCP credentials from both manifests and response metadata while preserving non-MCP tools. Full integration execution remains unverified because dependencies were unavailable.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Bits Code Review · Commit dc65642

…rom OpenAI Agents spans

Code interpreter and image generation tool configs were copied verbatim into
the agent manifest, including container network policy domain secret values,
and model_settings recorded extra_headers, extra_query, extra_body and
extra_args. Record tool configs through safe_tool_config, which drops domain
secret values, apply it to every tool in response metadata, and filter model
settings with ALLOWED_MODEL_SETTINGS_KEYS.

APPSEC-71467

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@datadog-prod-us1-5

datadog-prod-us1-5 Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Pipelines  Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 15747b7 | Docs | View more details | Give us feedback!

@sabrenner sabrenner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

one suggestion but not blocking, thanks for doing this!

Comment thread ddtrace/llmobs/_integrations/utils.py Resolved
…igurable

Rename the hosted MCP field allowlist to _SAFE_OPENAI_MCP_TOOL_FIELDS and let
safe_mcp_tool_config, safe_tool_config and _safe_response_tools take a
keyword-only allowlist so other integrations can reuse them. server_url is
reduced to its origin whatever the allowlist.

APPSEC-71467

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@KowalskiThomas KowalskiThomas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Proposal to make the release not a bit more concise.

…c2e23460dd3806.yaml

Co-authored-by: Thomas Kowalski <thomas.kowalski@datadoghq.com>
@avara1986
avara1986 requested a review from mabdinur November 3, 2026 15:39
@avara1986

Copy link
Copy Markdown
Member Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-11-04 07:35:24 UTC ℹ️ Start processing command /merge


2026-11-04 07:35:28 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 1h (p90).


2026-11-04 08:08:35 UTC ❌ MergeQueue: The build pipeline failed for this merge request

Build pipeline has failing jobs for 666cf29:

What to do next?

  • Investigate the failures and when ready, re-add your pull request to the queue!
  • If your PR checks are green, try to rebase/merge. It might be because the CI run is a bit old.
  • Any question, go check the FAQ.

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!
If you want to reset everything, you can cancel your previous command by rerunning it with the -c flag.

@avara1986

Copy link
Copy Markdown
Member Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-11-04 08:13:36 UTC ℹ️ Start processing command /merge


2026-11-04 08:13:40 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 1h (p90).


2026-11-04 08:58:59 UTC ❌ MergeQueue: The build pipeline failed for this merge request

Build pipeline has failing jobs for ac15ac9:

What to do next?

  • Investigate the failures and when ready, re-add your pull request to the queue!
  • If your PR checks are green, try to rebase/merge. It might be because the CI run is a bit old.
  • Any question, go check the FAQ.

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!
If you want to reset everything, you can cancel your previous command by rerunning it with the -c flag.

@avara1986

Copy link
Copy Markdown
Member Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-11-04 08:59:32 UTC ℹ️ Start processing command /merge


2026-11-04 08:59:37 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 1h (p90).


2026-11-04 09:35:14 UTC ℹ️ MergeQueue: This merge request was merged

@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot merged commit 78bcc6a into main Nov 4, 2026
854 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot deleted the alberto.vara/APPSEC-71467-llmobs-openai-agents-mcp-credentials branch November 4, 2026 09:35
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.

6 participants