[Security] Vector store metadata uses pickle - trivial RCE if .pkl file is tampered

[GunaPalanivel]

Problem

vectorstore_utils.py serializes and deserializes FAISS metadata using pickle.dump() and pickle.load():

# rag/vectorstore/vectorstore_utils.py:57-78
def save_metadata(metadata, path):
    with open(path, "wb") as f:
        pickle.dump(metadata, f)

def load_metadata(path):
    with open(path, "rb") as f:
        return pickle.load(f)  # <-- arbitrary code execution

Python pickle executes arbitrary code during deserialization. If an attacker can replace the .pkl file on disk - via a supply chain attack on the data pipeline, a compromised scraping source, or a path traversal bug elsewhere - they get full RCE with the server process privileges.

The metadata being stored is just a list of dicts with string values (id, text, source, code_blocks). There is no reason to use pickle for this structure.

Impact

• Arbitrary code execution on the server if any .pkl file is modified
• The data pipeline pulls from external URLs (plugin docs, Discourse, etc.) and writes these files - any compromise upstream propagates
• pickle has been on the do-not-use-with-untrusted-data list since Python 2.x

Proposed Fix

Replace pickle with json in vectorstore_utils.py:

import json

def save_metadata(metadata, path):
    with open(path, "w", encoding="utf-8") as f:
        json.dump(metadata, f)

def load_metadata(path):
    with open(path, "r", encoding="utf-8") as f:
        return json.load(f)

Then update the embedding pipeline (store_embeddings.py) to write .json instead of .pkl, and update retriever_utils.py to read from the new path. Existing .pkl files need a one-time migration or rebuild.

Acceptance Criteria

• pickle.load and pickle.dump removed from vectorstore_utils.py
• Metadata saved as JSON (or MessagePack if JSON perf is a concern)
• store_embeddings.py writes .json metadata files
• retriever_utils.py reads .json metadata files
• Existing unit tests in tests/unit/rag/vectorstore/ updated and passing
• Migration note or rebuild script documented for existing deployments

References

• chatbot-core/rag/vectorstore/vectorstore_utils.py lines 57-78
• chatbot-core/rag/vectorstore/store_embeddings.py lines 14-15
• chatbot-core/rag/retriever/retriever_utils.py (reads the metadata)
• Python docs on pickle security: https://docs.python.org/3/library/pickle.html

[berviantoleo]

I'm still unsure if we can move it in one go.

Just get the reference:

https://github.com/facebookresearch/faiss/blob/796977a1abc1ebfc42ce27fda655593089bcc70e/tests/index_io_backward_compatibility/cmake_conda_io_utils.py#L600

[GunaPalanivel]

Thanks for the FAISS link — useful confirmation. Their write_metadata / read_metadata are just json.dump(..., indent=2) and json.load, so JSON is clearly the right call for a list-of-dicts payload like ours.

Here's how I'd approach it so we don't break anyone running the current build:

• Switch vectorstore_utils.save_metadata / load_metadata to json.dump / json.load (UTF-8, indent=2 to match the upstream style).
• Rename the artifact in store_embeddings.py (plugins_metadata.pkl → plugins_metadata.json) and the matching read in retriever_utils.py.
• Inside load_metadata, if the requested .json isn't there but a sibling .pkl exists, read it once via pickle.load and emit a DeprecationWarning + log line telling the operator to re-run store_embeddings.py. Pickle stays imported only inside that fallback branch, so the unsafe path is opt-in and goes away as soon as people rebuild.
• Update the existing tests in tests/unit/rag/vectorstore/ for the JSON round-trip and add one pytest.warns(DeprecationWarning) case for the fallback. Retriever test just gets the path suffix flipped.
• Short migration note in docs/chatbot-core/rag/vectorstore.md.

No new deps, no separate migration script — data/embeddings/ is gitignored so there's nothing to convert in-repo, and the deprecation-warned fallback covers existing deployments until the next reindex.

If that sounds reasonable I'll open a PR against main as fix/issue-348-pickle-to-json-metadata.