Skip to content

fix(guardrails): default CrowdStrike AIDR guardrail to run at end of stream - #43041

Open
kenany wants to merge 1 commit into
BerriAI:mainfrom
kenany:kenany/crowdstrike-aidr-streaming-end-of-stream-only
Open

kenany wants to merge 1 commit into
BerriAI:mainfrom
kenany:kenany/crowdstrike-aidr-streaming-end-of-stream-only

Conversation

@kenany

@kenany kenany commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

The CrowdStrike AIDR guardrail has always been intended to work with the
complete response, not with streamed chunks. The introduction of
streaming_end_of_stream_only and the like have caused the guardrail to begin
receiving chunked content, which then gets reflected in the CrowdStrike AIDR
platform as multiple incomplete findings/events, which looks like a regression
to customers. This patch updates the streaming_end_of_stream_only default to
True for this guardrail.

Affected release

Regressed at some point after v1.84.6?

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Type

🐛 Bug Fix

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

@kenany
kenany requested a review from a team September 24, 2026 20:32
@veria-ai

veria-ai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/10

@greptile-apps

greptile-apps Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The behavior appears safe to merge, but the updated test must satisfy the repository’s no-mutation requirement first

Findings

  1. P2 Mutable scan collection ▶

Summary

The PR defaults CrowdStrike AIDR streaming scans to the assembled response while preserving explicit periodic scanning. The latest test change checks the content sent for each scan rather than only counting vendor calls

Reviews (2) · Last reviewed commit: "fix(guardrails): default CrowdStrike AID..."

@codspeed

codspeed Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

1 warning

⚠️ 1 benchmark is dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

✅ 35 untouched benchmarks


Comparing kenany:kenany/crowdstrike-aidr-streaming-end-of-stream-only (0b7b79b) with main (295d79f)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (38b4d31) during the generation of this report, so 295d79f was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@kenany
kenany force-pushed the kenany/crowdstrike-aidr-streaming-end-of-stream-only branch 2 times, most recently from 31c03bc to a41fc2b Compare September 24, 2026 22:42
@kenany

kenany commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai

nonlocal calls
calls += 1
messages = json.loads(request.content)["guard_input"]["messages"]
scanned.append("".join(message["content"] for message in messages))

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.

P2 Mutable scan collection The updated test appends to scanned, violating the repository requirement to avoid mutation in updated code. Please resolve this before merging

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@VANDRANKI VANDRANKI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Community review, does not clear the merge gate.

I read the diff, the PR description, and the full parametrized test to check both the behavior-default change and the code pattern used to implement it.

The PR flips the CrowdStrike AIDR guardrail's default for streaming_end_of_stream_only from False to True, per the description because this guardrail was "always intended to work with the complete response," and mid-stream chunked scanning produces multiple incomplete findings on CrowdStrike's own platform that look like false regressions to customers. I flag the tradeoff up front since it's real and the updated docstring says so honestly: with the new default, a flagged/blocked response may already have reached the client before the guard runs, versus the old per-chunk mode which could cut a stream short mid-flight. That's a deliberate, documented tradeoff for this specific vendor integration, not a silent weakening.

The implementation detail I specifically checked: the old code used streaming_params.streaming_end_of_stream_only or False. That pattern only works when the default is falsy. Simply changing the fallback to or True would have been a real bug, since False or True evaluates to True, silently overriding a user's explicit streaming_end_of_stream_only=False back to True. The PR instead uses True if streaming_params.streaming_end_of_stream_only is None else streaming_params.streaming_end_of_stream_only, which correctly distinguishes "unset" (defaults to the new True) from "explicitly False" (stays False).

I traced the new parametrized test against this: {"streaming_end_of_stream_only": False} expects two scans ("ABCDE", "ABCDEFGHIJ", at the default streaming_sampling_rate=5), confirming explicit False still activates the old chunked-scan behavior rather than being coerced to True. {} (unset) expects exactly one scan of the full assembled text, confirming the new default. The two streaming_end_of_stream_only=True cases (with and without a sampling rate) both correctly ignore the sampling rate and scan once. Every case also asserts scanned[-1] == "ABCDEFGHIJ", i.e. the final scan always sees the complete accumulated output regardless of mode.

This is a narrowly scoped default change (only this guardrail's own _set_streaming_params, not a shared base-class default affecting other guardrail integrations), correctly avoids the truthy-default or footgun, and the tests exercise the actual edge case that pattern would have gotten wrong. Looks correct to me.

@kenany
kenany force-pushed the kenany/crowdstrike-aidr-streaming-end-of-stream-only branch 2 times, most recently from 981452e to 5990653 Compare October 5, 2026 12:45
@kenany
kenany force-pushed the kenany/crowdstrike-aidr-streaming-end-of-stream-only branch from 5990653 to 5b457f3 Compare October 18, 2026 00:42
@kenany
kenany force-pushed the kenany/crowdstrike-aidr-streaming-end-of-stream-only branch from 5b457f3 to 0b7b79b Compare October 28, 2026 14:25
…stream

The CrowdStrike AIDR guardrail has always been intended to work with the
complete response, not with streamed chunks. The introduction of
`streaming_end_of_stream_only` and the like have caused the guardrail to begin
receiving chunked content, which then gets reflected in the CrowdStrike AIDR
platform as multiple incomplete findings/events, which looks like a regression
to customers. This patch updates the `streaming_end_of_stream_only` default to
`True` for this guardrail.
@kenany
kenany force-pushed the kenany/crowdstrike-aidr-streaming-end-of-stream-only branch from 0b7b79b to d045668 Compare November 3, 2026 14:02

This branch has not been deployed

No deployments
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.

2 participants