feat(rag)!: drop the deployment embedding key; add data-protection docs - #1604
Open
DEENUU1 wants to merge 4 commits into
Open
feat(rag)!: drop the deployment embedding key; add data-protection docs#1604DEENUU1 wants to merge 4 commits into
DEENUU1 wants to merge 4 commits into
Conversation
Map every store that holds personal data, every destination that receives it and the setting that decides each, the controls with the test or page that proves them, what deletion reaches and what it leaves, and the conditions a deployment has to decide for itself. A gap is named as the issue that tracks it rather than described around. Found while tracing deletion: no route removes a chat attachment's bytes, so a deleted conversation or account leaves its files under MEDIA_DIR. Recorded on the page and left to #1421, which owns erasure. Wire the page into the nav, the topic map, the rollout review table, SECURITY.md and the docs drift triggers. Refs #1596
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 33ca49a6d2
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
The transport row promised HTTPS to every provider, but the model profile service accepts an http:// base_url for an endpoint on the deployment's own network. Say so, and add the query that lists every such profile to the verification checklist.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33ca49a6d2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Lead with what a deployment that wants no third party configures: local chat models, pymupdf or a LiteParse sidecar, no Logfire token, no search, memory or MCP bindings. Name the one thing that cannot stay local - the embedding catalog holds OpenRouter and OpenAI only - and say precisely whose key embeds a collection, since OPENROUTER_API_KEY is a fallback for one provider rather than the embedding credential. Correct what the security review found against the code: event-triggered runs carry the event body in Prefect parameters; rate-limit keys hold the plain address for the window; audit details keep a few values, not only field names; the sandbox runtime can reach any host; web research offers DuckDuckGo, Tavily, Brave, Exa and native search; a collection's own llamaparse_secret_id bypasses an empty deployment key; an environment's Logfire token redirects runs too; the stale-run sweep is not retention; messages and attachments are scoped through their parent; context files are a store; a person's memory outlives their account; and the orphan check compares stored paths with the disk instead of counting files. Refs #1596
OPENROUTER_API_KEY was a deployment-wide fallback for one embedding
provider, left over from when openrouter.ai was hardcoded in the
embedding client. It was the only reason the catalog carried a
deployment_key flag, the resolver two fallback states and a
cross-provider rule, the embedding-models route a default_provider, and
the create form a "Deployment key" row that applied to one provider and
not the other.
Every collection now names the provider it embeds through, from
embedding_providers.json, and the organization vault key that pays - the
way a chat model is keyed per organization. A new personal or
organization collection without either is refused on that field; a
collection whose key is missing, unusable, of the wrong kind or never
chosen resolves to no key, refuses to index or search with a message
naming the collection and the reason, and the ingestion flow log says
so. A key can be replaced but no longer cleared, because there is
nothing to fall back to. App-scoped collections have no vault and stay
keyless; the legacy POST /rag/collections/{name} row records the first
catalog provider and waits for a key through PATCH /kb/{id}.
The form preselects the first catalogued provider, leaves the key empty
until one is chosen, and shows the server's refusal under the picker.
The docs, the env example, the changelog and the knowledge-base how-to
say the same thing.
BREAKING CHANGE: OPENROUTER_API_KEY is no longer read. A collection
created without a vault key refuses to embed until PATCH /kb/{id} gives
it one; clear_embedding_secret is removed from KnowledgeBaseUpdate, and
default_provider and deployment_key from GET /rag/embedding-models.
Refs #1596
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this delivers
Two things, both for #1596 (Refs — whether the issue closes on them or waits for the deployment-specific evidence is your call).
1.
docs/data-protection.md— the data-protection pack a DPO or security officer reads against a running deployment. It leads with the posture that matters for NFA-024/025: nothing leaves by default. Every external hop is a row or a setting somebody adds — a model profile, a parser choice, a search method, a Logfire token, a mem0 host, an MCP connection — and a deployment that wants no third party configures local chat models (ollama,litellm, or anybase_urlprovider behind a gateway),pymupdfor a LiteParse sidecar for parsing, no Logfire token, and no search/memory/MCP bindings. The one exception is named: the embedding catalog holds OpenRouter and OpenAI only, so a knowledge base always reaches one of them. Nothing on the page claims GDPR compliance; what only a deployment can decide is listed as evidence to obtain.2. Embeddings are paid for with the collection's vault key, and nothing else (breaking).
OPENROUTER_API_KEYwas a deployment-wide fallback for one provider, left over from whenopenrouter.aiwas hardcoded — and the only reason the catalog carried adeployment_keyflag, the resolver two fallback states and a cross-provider rule, the embedding-models route adefault_provider, and the create form a "Deployment key" row that applied to one provider and not the other. Every collection now names its provider fromembedding_providers.jsonand the organization vault key that pays, the way a chat model is keyed per organization.What changed
Docs
docs/data-protection.md— new: responsibilities; every store holding personal data, in the database and outside it (Redis rate-limit keys with the plain address, Prefect parameters carrying an event-triggered run'sevent_context, Logfire, SMTP, backups); every destination and the setting that decides it; a controls matrix mapping each control to its proof or its open issue; what deletion reaches and leaves; the evidence a deployment has to obtain; a reproducible verification checklist (doctor,vault-rotate --dry-run, an env sweep that masks keys, SQL over profiles / plain-HTTP endpoints / secrets / collections / connections / environment Logfire tokens, and an orphaned-attachment check comparing stored paths with the disk); the open conditions for a first rollout.mkdocs.yml,docs/rollout.md(security-review table row),SECURITY.md,CLAUDE.mdtopic map,scripts/docs_drift.pytriggers.docs/configuration.md,docs/file-processing.md,docs/howto/set-up-knowledge-base.md,backend/.env.example,CHANGELOG.md— the embedding key contract.Backend
config.py—OPENROUTER_API_KEYremoved.embedding_providers.json/embedding_providers.py— nodeployment_key, nodeployment_provider();require()takes a provider id.embedding_resolution.py—EmbeddingKeySourceisORGANIZATION | NONE_CHOSEN | SECRET_MISSING | SECRET_UNUSABLE | SECRET_WRONG_KIND; every non-organization source resolves to an empty key and is announced in the flow log. A provider the catalog no longer names keeps its key to itself.embeddings.py— no default key or endpoint; a service built outside any collection refuses on first use with a message that names no variable.knowledge_base.py— a new personal/org collection must nameembedding_providerandembedding_secret_id(refused on the field otherwise); app-scoped collections have no vault and stay keyless;clear_embedding_secretremoved from the update path, schema and repository; the legacyPOST /rag/collections/{name}row records the first catalog provider and waits for a key viaPATCH /kb/{id}.GET /rag/embedding-models— nodefault_provider, nodeployment_key.Frontend
embedding-picker.tsx— no "Deployment key" row; the key select is empty ("Choose a key") until one is chosen, controlled with""so switching provider really empties it.create-kb-dialog.tsx— preselects the first catalogued provider, always sends provider and key, shows the server's field refusal under the picker.embedding-dialog.tsx— a key is replaced, never cleared.types/knowledge-base.ts,messages/en.json(chooseKeyadded;deploymentKey,keyHereBillsEmbeddingsremoved;keyRequiredHere,modelsUnreadablereworded).Noticed, not fixed
chat_filesrows cascade with their message but nothing unlinks the file underMEDIA_DIR; the one caller ofFileUploadService.discardis the channel router. Deleting a conversation or an account leaves attachments on disk. Belongs to Erasure and export of one person's data: what UserService.delete leaves behind #1421.agent_memory_files.owner_keyis a string, not a foreign key, andUserService.deletenever callsMemoryService.forget_person. Also Erasure and export of one person's data: what UserService.delete leaves behind #1421.frontend/src/app/api/auth/password/change/route.tssets the refreshed cookies'securefromNODE_ENVinstead ofsecureCookies(request)as login/refresh do. One-line fix; kept out of this PR.Testing
make testagainst a real Postgres (dockerdb+redisfrom the repo's compose): 7539 passed, integration suite included, 100% coverage on the platform layer.make test-frontend-cov: 417 files / 6145 tests, 100% lines-statements-functions, 97.71% branches (floor 97.5).check:i18n, knip — clean.make docs-build(--strict), paragraph/backtick guards, codespell — clean; everypage.md#fragmenton the new page checked against the target's headings by script.Breaking
OPENROUTER_API_KEYis no longer read. A collection created without a vault key refuses to embed untilPATCH /kb/{id}gives it one;clear_embedding_secretis gone fromKnowledgeBaseUpdate;default_provideranddeployment_keyare gone fromGET /rag/embedding-models. No migration:knowledge_bases.embedding_secret_idstays nullable for app-scoped rows.