fix(aicore): prevent content filter bypass in multi-module and streaming modes

[NicoleMGomes]

Disclaimer: Do not include SAP-internal or customer-specific information in this PR (e.g. internal system URLs, customer names, tenant IDs, or confidential configurations). This is a public repository.

Description

Fixes two content-filtering bypass paths in sap_cloud_sdk.aicore:

1. Multi-module bypass — when fallback_sap_modules is configured, body["config"]["modules"] is a list rather than a dict. The previous code only injected filtering into modules[0]; all fallback entries were sent without any filter configuration. The patch now iterates over all entries.

2. Streaming output-filter bypass — LiteLLM never calls transform_response for streaming responses; it routes streams through a separate iterator path. This meant output-filter rejections (finish_reason == "content_filter" chunks) would be silently yielded to the caller rather than raising ContentFilteredError. The fix blocks stream=True upfront in transform_request when output filtering is active, raising ContentFilteredError before any request is sent. This guard runs at the LiteLLM transport layer and covers all callers uniformly — sap_cloud_sdk.aicore.completion, ChatLiteLLM, LangGraph, etc.

Input-only filtering is unaffected: stream=True with input filtering active continues to work normally — input rejections surface as ContentFilteredError through the HTTP 400 path regardless of streaming.

Related Issue

Closes #<issue_number>

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update
• Code refactoring
• Dependency update

How to Test

1. Multi-module fix — call set_filtering(ContentFiltering(...)) then invoke completion() with a fallback_sap_modules deployment. Confirm filtering is present in all modules list entries by inspecting the request body (or run TestTransformRequestMultiModule unit tests).

2. Streaming guard — activate output filtering via set_filtering(ContentFiltering(output_filtering=OutputFiltering(...))), then call completion(model=..., messages=..., stream=True). Confirm a ContentFilteredError with direction="output" is raised before any network request is made.

3. Input-only streaming still works — activate input-only filtering, call with stream=True, confirm no exception is raised upfront (filter rejections only arrive if the server returns HTTP 400).

4. Run the unit test suite: uv run python3 -m pytest tests/aicore/filtering/unit/test_filtering.py tests/aicore/unit/test_completion.py -v

Checklist

• I have read the Contributing Guidelines
• I have verified that my changes solve the issue
• I have added/updated automated tests to cover my changes
• All tests pass locally
• I have verified that my code follows the Code Guidelines
• I have updated documentation (if applicable)
• I have added type hints for all public APIs
• My code does not contain sensitive information (credentials, tokens, etc.)
• I have followed Conventional Commits for commit messages

Additional Notes

Security Review

• ⚠️ S7 — Key-path assumption: body["config"]["stream"]["enabled"] and body["config"]["modules"] are accessed without existence checks. If a future LiteLLM version changes this shape, the guard could silently fail. The current LiteLLM SAP handler (litellm/llms/sap/chat/handler.py:250-254) sets this path unconditionally, so the risk is limited to LiteLLM upgrades. Mitigation: the .get() chain on the stream key is already safe; modules access is protected by the enclosing if filtering_dict check.
• ⚠️ S9 — TOCTOU on _active_cfg: _active_cfg is read twice between the output-filtering check and the module injection. In a multi-threaded scenario, filtering could be disabled between the two reads. In practice, set_filtering / disable_filtering are called at startup, not concurrently with requests.

[NicoleMGomes]

PR Review: #410 — fix(aicore): prevent content filter bypass in multi-module and streaming modes

Author: NicoleMGomes Branch: fix/stream-filtering → main
Verdict: ⚠️ Needs Minor Work
Summary: Solid security fix with good test coverage; blocked only by a ruff-format failure in CI and two minor code-quality issues worth addressing before merge.

---

A: Process & Compliance

#	Criterion	Status	Finding
A1	PR template complete	⚠️	Two checklist items unchecked: "I have verified that my changes solve the issue" and "All tests pass locally" — CI shows tests pass, but the local-verification boxes are unticked. Issue placeholder Closes #<issue_number> was not replaced with a real issue number.
A2	Conventional Commits	⚠️	First commit fix: content filtering with stream is missing the (aicore) scope; subsequent commits are correctly scoped. PR title is correctly scoped — commit-validation CI passes because it checks the PR title. Recommend amending or squashing before merge.
A3	Issue linked	⚠️	Closes #<issue_number> is a literal placeholder — no real issue linked.
A4	AI-generated code disclosure	✅	No disclosure needed; the "Additional Notes / Code Review / Security Review" sections in the PR body are self-authored analysis, not AI-generated code.

B: Security & Sensitive Data

#	Criterion	Status	Finding
B1	No sensitive data in code	✅	No credentials, SAP-internal URLs, tenant IDs, or customer data in the diff.
B2	No sensitive data in PR body	✅	PR body contains no account-specific URLs, real tenant IDs, or internal tooling references.

C: Code Quality

#	Criterion	Status	Finding
C1	CI checks passing	❌	Code Quality Checks fails: ruff-format requires the multi-line if condition at filtering/_patch.py:94–97 to be reformatted as a single logical line split differently. All other jobs (Tests, Build, CodeQL, Commit Validation, Version Bump) pass.
C2	Version bump	➖	No manual version bump — N/A.
C3	Type hints	⚠️	transform_request return type is dict (inherited, fine); _install param cfg is annotated Any instead of ContentFiltering | None — minor but public-facing.
C4	No hardcoded values	✅	No magic strings or magic numbers introduced; "content_filter" and "output" were pre-existing constants in context.
C5	Import organization	⚠️	from sap_cloud_sdk.aicore.filtering import _patch as _p at test_filtering.py:148 is imported but never used in _call_transform_request — dead import.
C6	Naming conventions	✅	No new enums or module-level lists introduced; existing conventions followed.
C7	No unused code	⚠️	Unused import _p at tests/aicore/filtering/unit/test_filtering.py:148 (see C5). The noqa: S7503 comment at test_completion.py:339 suppresses a non-existent Ruff rule — harmless but noisy.
C8	No unjustified new dependencies	✅	No new dependencies added.
C9	Proto code freshness	➖	No .proto files changed — N/A.

D: API & Design

#	Criterion	Status	Finding
D1	API future-proofing	✅	No new config params added; the guard is a behaviour change inside an existing class.
D2	Public API hygiene	✅	completion.py __all__ unchanged; no new public symbols exposed.
D3	Breaking changes marked	✅	stream=True with output filtering active now raises ContentFilteredError where it previously silently yielded chunks — this is a bug fix, not a breaking change, and the PR correctly does not tick the breaking-change checkbox.
D4	Pagination & tenant filtering	➖	Not applicable to this change.
D5	Telemetry instrumentation	➖	No new client methods added — N/A.
D6	Multi-tenancy support	✅	user-guide.md already contains a compliant ## Multi-tenancy section (line 554); no change required for this fix.

E: Tests & Documentation

#	Criterion	Status	Finding
E1	Tests added/updated	✅	TestTransformRequestMultiModule (3 tests in test_filtering.py) and TestStreamingGuard (7 tests in test_completion.py) cover the two new code paths. autouse fixture correctly restores _active_cfg after each test.
E2	Documentation quality	✅	completion.py module docstring and user-guide.md Streaming section both updated to describe the new stream=True + output-filter constraint. The prose is accurate and actionable.
E2a	Multi-tenancy documentation	✅	Pre-existing ## Multi-tenancy section in user-guide.md:554 has all four required fields (Supported / Authentication / How to use / Further reading). Not modified by this PR but remains compliant.
E3	Module structure compliance	➖	Not a new module — N/A.

---

❌ Blocking Issues

• [C1] src/sap_cloud_sdk/aicore/filtering/_patch.py:94–97: ruff-format requires the if condition to be reformatted. Run ruff format src/sap_cloud_sdk/aicore/filtering/_patch.py and push — Code Quality Checks CI job is currently failing because of this one file.

⚠️ Non-Blocking Suggestions

• [A1 / A3] Replace Closes #<issue_number> with the real issue number (or remove the line if no issue exists), and tick the two unchecked checklist items.
• [A2] Amend or squash the first commit (fix: content filtering with stream) to add the (aicore) scope for consistency with the rest of the repo history.
• [C5 / C7] Remove the unused from sap_cloud_sdk.aicore.filtering import _patch as _p import at tests/aicore/filtering/unit/test_filtering.py:148.
• [C3] Consider tightening _install's cfg: Any annotation to cfg: ContentFiltering | None for IDE support and static analysis clarity.

✅ Things Done Well

• The transport-layer approach (transform_request) is the right architectural choice: it is universal and caller-agnostic, covering ChatLiteLLM and LangGraph without any wrapper changes.
• The multi-module loop fix (for m in modules) is minimal and correct — the old modules[0]-only assignment was a clear bypass path.
• TestStreamingGuard correctly tests the boundary between input-only (allowed) and output-active (blocked), and the autouse fixture ensures no test state leaks.
• The updated user-guide.md Streaming section is clear and includes a runnable code example with the try/except ContentFilteredError pattern.
• The PR body's "Additional Notes" sections show thorough self-review including the thread-safety and choices[0]-only edge cases.