fix(deps): move chromadb out of core install (CVE-2026-45830, CVE-2026-45833)

[TFT444]

Summary

• Remove chromadb==0.4.24, which is affected by CVE-2026-45830 and CVE-2026-45833, from every supported installation path.
• Replace the ChromaDB vector store with a dependency-free persisted BM25 JSON index.
• Preserve the existing retrieve() response contract used by the AI routes.
• Keep the existing route-level handling for a missing or unreadable index.

Changes

• requirements.txt: remove ChromaDB and NumPy; retain six>=1.16.0 for the runtime import used by azure-mgmt-rdbms.
• requirements-ai.txt: remove the obsolete optional vulnerable dependency file.
• ai/embed.py: tokenize repository knowledge, calculate BM25 corpus statistics, and atomically write ai/vectorstore/bm25_index.json.
• ai/retriever.py: validate the versioned JSON schema, reject malformed indexes through VectorStoreNotBuilt, and rank matching chunks using BM25.
• Dockerfile: use core requirements, build the BM25 index into every image, and ensure the file begins with a valid FROM instruction without a UTF-8 BOM.
• ai/README.md: document the dependency-free build and retrieval flow.
• ai/chunker.py: guarantee forward progress when a newline falls inside the overlap window.
• .github/workflows/ci.yml: retrieve OpenShield content inside the freshly built production image before runtime and Trivy checks.
• tests/test_rag_dependencies.py: cover dependency absence, Dockerfile validity, chunker progress, malformed indexes, BM25 scoring, index persistence, and retrieval.

Test plan

• python -m ruff check .
• python -m ruff format --check .
• python -m pytest (759 passed, 5 skipped locally on commit bfa95f0)
• ChromaDB absent from all requirements*.txt files
• BM25 index builds and retrieves relevant OpenShield content using core dependencies
• Required GitHub checks complete successfully
• Maintainer re-review confirms the requested changes

Security scope

OpenShield used ChromaDB as an embedded local dependency rather than exposing a standalone ChromaDB server. Removing the unpatched package eliminates the vulnerable component from supported installs without claiming that OpenShield previously exposed ChromaDB's network API.

Closes

Dependabot alerts #16 and #17

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Package	Version	Score	Details
pip/six	>= 1.16.0	Unknown	Unknown

Scanned Files

• requirements.txt

[ritiksah141]

Looks good to me approving it.

[parthrohit22]

Nice catch on this one. Both CVEs are real - I checked them independently rather than taking the numbers on faith (CVE-2026-45833 is RCE via UPDATE_COLLECTION plus a malicious embedding-function reference to an untrusted HuggingFace model, CVE-2026-45830 is a tenant-isolation bypass in ChromaDB's V1 HTTP API when a collection UUID is supplied), and the Python-side handling is genuinely solid: ai/embed.py/ai/retriever.py already wrap import chromadb in try/except ImportError, and every route in api/routes/ai.py that needs it (/ai/summary, /ai/prioritise, /ai/ask, /ai/threat-simulation) already catches VectorStoreNotBuilt and returns a clean 503 instead of crashing - that's not new code, but confirming it was already there and actually gets exercised is exactly the kind of check that matters here. Pulled this branch's actual requirements.txt and test_rag_dependencies.py into a local venv and ran it myself: genuinely 2 passed, 1 skipped, and the full suite (723 passed, 4 skipped) is otherwise unaffected.

One blocking issue though, plus a few worth fixing in the same push - left the specific ones inline.

Blocking: this silently breaks a live product feature. Dockerfile only ever runs pip install -r requirements.txt and isn't touched by this PR - I checked docker-compose.yml, every CI workflow, and the Terraform config too, and none of them reference requirements-ai.txt either. After this merges, the deployed image has no chromadb at all, and /api/ai/summary, /ai/prioritise, /ai/ask, /ai/threat-simulation will permanently 503 in production - not hypothetical, AILayer.jsx and ChatPanel.jsx are live frontend surfaces calling these routes today. The description frames this as a clean dependency move ("continues to work when requirements-ai.txt is installed separately") without disclosing that nothing in the actual deployment pipeline does that install. Either wire the Dockerfile/deploy step to install requirements-ai.txt, or say explicitly that this intentionally disables AI features in production until a follow-up lands - right now it reads as an oversight.

Worth softening: the exposure claim overstates what's actually running. The description says removing chromadb means "the API server and Docker image are no longer exposed." I grepped the whole codebase and infra for HttpClient, chroma run, or any standalone chromadb server and found nothing - ai/embed.py/ai/retriever.py only ever use chromadb.PersistentClient, which is embedded/local/file-based with no network listener and no multi-tenant HTTP API. Both CVEs are specifically about the server product's auth/tenant-isolation and its HTTP collection-update endpoint, neither of which this codebase runs. Still worth removing the dependency (keeps Dependabot/pip-audit quiet regardless), just worth describing accurately rather than implying an exposure that isn't actually there.

Left the other two (the six pin and the stale install messages in ai/embed.py/ai/retriever.py - the latter aren't touched by this diff so I couldn't comment inline on them, but worth a look since this PR is what makes them stale) inline and in the notes below.

[parthrohit22]

The commit message says azure-mgmt-rdbms==10.1.0 requires six, previously satisfied transitively by chromadb. I checked - azure-mgmt-rdbms's actual declared deps (azure-common, azure-mgmt-core, msrest) don't include six, and msrest's source doesn't reference it either. Then I did a full pip install --dry-run of this branch's real final requirements.txt with this line stripped back out - it resolves cleanly and six doesn't appear anywhere in the install list.

This pin looks unnecessary based on a misdiagnosed CI or local-environment issue. If there's a real reason six is needed that I'm missing, worth naming it in the commit message - otherwise this can probably come out.

[parthrohit22]

This line has a UTF-8 BOM prepended before the # heading (confirmed by inspecting the raw diff bytes - \xef\xbb\xbf sits right before # OpenShield RAG Pipeline). Harmless on GitHub's rendering, but it's inconsistent with the rest of the repo's plain-UTF8 files and looks like a Windows-editor artifact - worth stripping.

[parthrohit22]

One more from the review above, couldn't leave it inline since neither file is touched by this diff:

ai/embed.py:31 and ai/retriever.py:24 both still say "chromadb is not installed. Install it with 'pip install chromadb'." This PR is what makes that message stale - it should now point at requirements-ai.txt, since that's the install path this PR establishes. Small thing, but worth catching in the same push rather than a follow-up.

[ritiksah141]

Thanks for separating ChromaDB from the core install. I verified that this makes the current pip-audit and container checks green, but I do not think the current implementation makes OpenShield ChromaDB-safe yet.

The vulnerable chromadb==0.4.24 package is still published in requirements-ai.txt, and ai/README.md actively tells users to install it. Both reviewed advisories cover this version, and GitHub currently lists no patched release. Moving the dependency to an optional file changes who is exposed, but it does not provide a safe supported installation path.

There is also a production regression. The Docker image installs only requirements.txt, while the live AI routes depend on ai.retriever. After this change, those endpoints will consistently return 503 because no deployment path builds or installs the optional RAG stack.

Requested approach:

1. Remove ChromaDB from every supported requirements file and stop recommending its installation. Do not suppress either CVE in CI.
2. Preserve the deployed RAG feature with a safe in-process retrieval implementation. For this small, static OpenShield documentation corpus, a persisted JSON index with dependency-free BM25 or TF-IDF-style ranking is the lowest-risk option. It avoids a network service, tenant authorization, remote model loading, and another unpatched vector database dependency.
3. Update ai/embed.py and ai/retriever.py so the standard Docker/core installation can build and query that index without ChromaDB. Keep the existing route-level failure handling for a genuinely missing or corrupt index.
4. Replace the skipped Chroma embedding test with regression coverage that builds the new index and retrieves relevant OpenShield content using only core requirements. Add a test proving chromadb is absent from all requirements files and installation guidance.
5. Update ai/README.md and the runtime error messages to describe the new build path. Remove requirements-ai.txt unless it contains only audited, supported dependencies.
6. Rebase onto current dev containing #309 and rerun the full CI suite.

This keeps CI green by removing the vulnerable component, while preserving the product feature instead of relocating the risk or silently disabling the deployed AI endpoints.

[TFT444]

@parthrohit22 @ritiksah141 the Dockerfile blocker is resolved: INSTALL_AI_DEPS build arg (default false) controls whether requirements-ai.txt is installed, so the core image stays clean and the AI stack can be enabled at build time. The six pin is also restored as a floor constraint since azure-mgmt-rdbms imports it internally without declaring it.

On the BM25/TF-IDF replacement request: that is a solid idea but it is a new feature, not a fix for these two CVEs. I will open a separate issue to track it so it gets proper scope and testing. This PR's goal is to stop the vulnerable package from landing in the default install; the INSTALL_AI_DEPS gate achieves that. Would appreciate a re-review on what is here.

[parthrohit22]

Both items I flagged are properly fixed, and one of my other findings needs a correction on my end.

Dockerfile gap - fixed cleanly. ARG INSTALL_AI_DEPS=false plus the conditional requirements-ai.txt install, with a comment stating plainly that AI routes return 503 without it and everything else is unaffected. That's exactly the ask - either wire it up or say the tradeoff out loud, and this does both. One non-blocking observation: neither docker-compose.yml nor Terraform/Render sets INSTALL_AI_DEPS=true anywhere, so AI features stay off by default in every environment unless someone passes the flag at build time - consistent with the stated secure-by-default intent, just worth being aware of.

The six pin - I was wrong last time, and I want to be straight about it. I'd checked azure-mgmt-rdbms's declared metadata (no six) and run a pip install --dry-run that resolved cleanly without it, and concluded the pin was unnecessary. That was an incomplete check - a dry-run only verifies what pip's resolver would install from declared metadata, it can't catch a package that imports something at runtime without declaring it. I went back and tested that directly this time: blocked six from importing and tried from azure.mgmt.rdbms.postgresql import PostgreSQLManagementClient, the actual class this codebase uses - it fails, because .../postgresql/models/_postgre_sql_management_client_enums.py does from six import with_metaclass internally. The pin is genuinely necessary; my original finding was wrong and the fix (six>=1.16.0 as a floor constraint) is correct. Good catch, and thanks for pushing back on it with the more precise reasoning rather than just re-adding the line.

BOM in ai/README.md - fixed. Confirmed by checking the raw bytes, the file now starts clean with no \xef\xbb\xbf.

One thing still outstanding, not blocking this: ai/embed.py:31 and ai/retriever.py:24 still say "Install it with 'pip install chromadb'" instead of pointing at requirements-ai.txt. Worth a quick follow-up whenever, doesn't need to hold this up.

Ran the actual test suite fresh on this head in a clean clone rather than trusting the numbers: test_rag_dependencies.py - 2 passed, 1 skipped. Full suite - 720 passed, 6 skipped, 0 failed. CI 20/20 green.

Approving.

[TFT444]

The remaining production-readiness blockers are addressed on rebased head bfa95f0:

• Dockerfile now runs python -m ai.embed after COPY . ., so every successfully built image contains the BM25 index.
• CI builds the fresh image and executes a real retrieve() query inside it before startup and Trivy.
• The container log confirms 764 chunks indexed and retrieval returned analyzing-cloud-storage-access-patterns.
• _load_index() now validates the version, corpus statistics, IDF map, chunk structure, term frequencies, and document lengths. Malformed structures raise VectorStoreNotBuilt instead of leaking scoring errors as 500 responses.
• The chunker now guarantees forward progress when a newline falls inside the overlap window.
• The branch is rebased onto current dev at fcee748.

Verification: 759 passed, 5 skipped locally. All current GitHub checks pass, including Container Scan, backend tests, CodeQL, Semgrep, pip-audit, DCO, and CI Summary.

Please re-review the current head when available.

[ritiksah141]

Check throughly
Verified:

• ChromaDB is completely removed from every dependency path.
• No CVE suppression or vulnerable optional installation remains.
• Pure-Python BM25 retrieval preserves the existing API contract.
• Docker builds the BM25 index during image construction.
• CI performs a real retrieval inside the freshly built image.
• Malformed indexes raise VectorStoreNotBuilt, preserving safe 503 handling.
• Regression tests cover index creation, retrieval, malformed schemas, dependency absence, Docker configuration, and the chunker progress issue.
• PR is rebased exactly onto current dev at fcee748.
• All CI checks pass, including pip-audit, Trivy, backend tests, CodeQL, Semgrep, DCO, and external Semgrep.
  Approving it now

[parthrohit22]

Good work here - this is a real fix, not a workaround, and it's genuinely better than the version I approved two rounds ago.

I re-reviewed from scratch since GitHub correctly dismissed my prior approval - the PR changed materially after it (chromadb went from "optional, gated behind INSTALL_AI_DEPS" to "removed entirely, replaced with a pure-Python BM25 index"), so the old approval didn't cover what's actually here now. Full pass on the current head (bfa95f0):

What I verified myself, not just read:

• Built the real BM25 index from the actual repo docs (python -m ai.embed → 764 chunks from 489 documents) and ran real queries against it (storage account public access, network watcher flow logs, MFA for privileged identities, immutable backup policy) - results were genuinely relevant every time. For this domain (structured rule/control text with consistent terminology), lexical BM25 matching holds up well; I was ready to push back on retrieval-quality regression vs. the old embeddings approach, but the actual output doesn't support that concern.
• Ran tests/test_rag_dependencies.py myself: 16/16 pass. Full suite: 735 passed, 2 skipped, 0 failed.
• Confirmed chromadb, requirements-ai.txt, and INSTALL_AI_DEPS are completely gone - grepped the whole tree, no leftovers anywhere.
• Checked the actual CI run logs (not just the green checkmark) for the new "Verify BM25 retrieval in fresh image" step - it really does build the production Docker image, run a real query against the frozen artifact, and assert on the result. Confirmed the printed source value in the raw job log, not just that the step returned exit 0.
• ruff check/ruff format --check clean on all changed files.

One thing I checked precisely because the commit message claimed it, not because I doubted it: "ensure chunking always advances when newlines fall inside the overlap window." I reproduced the old bug directly - built a small repro of the old split_pos <= start condition against a document with a newline near the very start of a chunk followed by one long unbroken paragraph, and it genuinely infinite-loops (caught it at 1000 iterations in my repro). The new split_pos <= start + chunk_overlap guard fixes it: same input now terminates in 224 chunks and covers the whole document with correct overlap. That's a real correctness fix, not a defensive nit - the old code could have hung the index build on real-world markdown with a short heading line followed by a long paragraph.

Two non-blocking things, left inline - neither should hold this up.

Approving.

One more non-blocking nit, not inline since it's outside this diff's hunk: _split_text's early-return path for a document shorter than chunk_size (ai/chunker.py line 37) returns the raw text unstripped, unlike every chunk produced by the loop below it, which gets .strip()'d. Pre-existing, not touched by this PR - doesn't affect BM25 scoring, could leave stray whitespace in a single-chunk document's citation text. Worth a one-line fix whenever.

[parthrohit22]

Verified this independently rather than taking the commit message on faith: reproduced the old split_pos <= start condition against a document with a newline a few characters into a chunk window followed by one long unbroken paragraph, and it genuinely infinite-loops (start regresses to a value less than the previous iteration, gets clamped to 0, and the same state repeats forever). This + chunk_overlap guard fixes it - reran the same repro against the new code and it terminates cleanly (224 chunks, full coverage, correct overlap). Real bug, real fix.

[parthrohit22]

This closes the gap I flagged in my last review round on this PR - back when chromadb was optional behind INSTALL_AI_DEPS, neither docker-compose.yml nor the Terraform/Render config ever set that flag, so AI features would have silently 503'd in every real deployment by default. Baking the index into every image unconditionally (and now cheap to do, since there's no heavy dependency to gate) removes that failure mode entirely rather than just documenting it as a known tradeoff. Confirmed in the actual CI job log that this step runs and produces a real, non-trivial index (764 chunks) during the image build, and that the new 'Verify BM25 retrieval in fresh image' step below queries the frozen image afterward and gets a real result back - not just green-checkmark trust.

[parthrohit22]

Non-blocking suggestion: might be worth a line here noting the tradeoff versus the old embeddings-based retrieval - BM25 is lexical/keyword matching, so it won't catch a query and a chunk that mean the same thing but share little vocabulary (paraphrase/synonym gap), the way semantic embeddings would. I actually tested this codebase's retrieval quality directly and it holds up well because rule/control text and likely user queries share consistent technical vocabulary in this domain - but that's a property of this specific corpus, not something a future reader would know without being told, and it's worth being explicit about for whoever revisits this later.