fix(milvus): escape document source values in delete filter (CWE-89)

[sebastiondev]

Summary

MilvusVDB.delete_documents builds Milvus boolean filter expressions by interpolating user-controlled document names directly into f-strings. A document name containing a single quote can break out of the surrounding string literal and inject arbitrary boolean clauses, broadening the delete predicate beyond the intended document.

• CWE: CWE-89 (Improper Neutralization of Special Elements used in a Query) — filter-expression injection
• File: src/nvidia_rag/utils/vdb/milvus/milvus_vdb.py, function delete_documents
• Severity: High — the affected route is the public DELETE /documents ingestor endpoint and the fault enables unintended / mass deletion of vectors and document metadata within a collection.

Data flow

DELETE /documents (FastAPI ingestor) → document_names: List[str] → MilvusVDB.delete_documents(..., source_values=document_names) → f-strings:

collection.delete(f"source['source_name'] == '{source_value}'")
collection.delete(f"source == '{source_value}'")  # legacy fallback
self._delete_entities(
    collection_name=DEFAULT_DOCUMENT_INFO_COLLECTION,
    filter=f"info_type == 'document' and collection_name == '{collection_name}' and document_name == '{doc_name}'",
)

A name like evil.pdf' or '1'=='1 rewrites the predicate into source['source_name'] == 'evil.pdf' or '1'=='1', which matches every row in the collection.

Fix

Milvus does not expose parameterised queries for Collection.delete / boolean expressions, so the correct mitigation is to escape values being interpolated into single-quoted string literals. The patch adds a small helper that escapes backslashes first and then single quotes, and uses it for every user-controlled value flowing into a filter expression in delete_documents (primary delete, document_info delete, and the legacy fallback path).

@staticmethod
def _escape_milvus_string_literal(value: str) -> str:
    if not isinstance(value, str):
        value = str(value)
    return value.replace("\\", "\\\\").replace("'", "\\'")

The change is minimal and behaviour-preserving for legitimate document names (which do not contain single quotes or backslashes).

Tests

Added test_delete_documents_escapes_filter_injection in tests/unit/test_utils/test_vdb/test_milvus_vdb.py. It feeds the classic payload evil.pdf' or '1'=='1 into delete_documents, captures the expression handed to Collection.delete, and asserts:

1. The unescaped injection fragment or '1'=='1' is not present in the filter expression.
2. The escaped form (with backslash-escaped quotes) is present.

The existing delete_documents tests continue to pass, confirming the escape is transparent for normal inputs.

Security analysis

• Reachability: the ingestor DELETE /documents route forwards document_names straight into delete_documents with no sanitisation in between; we traced the data flow through the route handler to the sink.
• Exploitability: an attacker who can reach the ingestor API and knows (or guesses) a collection_name — typically the default from environment configuration — can submit {"document_names": ["x' or '1'=='1"], "collection_name": "..."} and trigger deletion of every vector matching the broadened predicate, plus the corresponding document_info rows. There is no authentication on this route in the shipped configuration.
• Mitigation provided by the fix: quotes and backslashes in source_value and doc_name can no longer terminate the string literal, so the user-controlled portion of the expression is confined to a literal string compared by ==. The integrity-impacting primitive (mass delete) is removed.

Adversarial review

Before submitting we tried to disprove this finding. We checked whether Milvus might already escape filter-expression literals server-side (it does not — the expression language treats ' as a literal terminator), whether any upstream FastAPI validator constrains document_names to filename-safe characters (none does; the field is a free-form List[str]), and whether the route requires authentication (it does not in the default deployment). We also looked for parameterised-query support for Collection.delete to avoid manual escaping; the Milvus Python SDK does not provide one for boolean expressions, which is why escaping is the appropriate fix.

cc @lewiswigmore

[shubhadeepd]

Hi @sebastiondev thanks very much for your contribution! Please help rebase these changes on develop branch.

[sebastiondev]

Hi @shubhadeepd, thanks for the feedback!

I've rebased the fix onto develop locally and resolved the merge conflict in milvus_vdb.py. The rebased commit is clean — only the same 2 files changed (milvus_vdb.py + test file, 55 additions / 3 deletions).

However, I'm hitting a GitHub OAuth scope limitation when force-pushing: the develop branch includes .github/workflows/ changes in its history, and GitHub requires the workflow scope to push any branch that touches workflow files — even though my commit doesn't modify them.

I've already updated the PR base branch to develop via the API. I'll try pushing the rebased commit shortly — if the scope issue persists, I may open a fresh PR targeting develop with a new branch name. Apologies for the delay!

[sebastiondev]

Update: I've confirmed that the develop branch itself contains workflow file changes (.github/workflows/ci-pipeline.yml), which means any branch based on develop triggers GitHub's workflow scope requirement. My OAuth token doesn't have that scope, so I'm unable to push the rebased branch.

The rebase is done locally — the fix applies cleanly on develop (one merge conflict in milvus_vdb.py resolved). The commit touches only:

• src/nvidia_rag/utils/vdb/milvus/milvus_vdb.py (+30/-3)
• tests/unit/test_utils/test_vdb/test_milvus_vdb.py (+28/-0)

I've also updated the PR base to develop already. Once the token scope is sorted I'll push the rebased commit. Apologies for the friction!