Skip to content

feat(rag)!: drop the deployment embedding key; add data-protection docs - #1604

Open
DEENUU1 wants to merge 4 commits into
mainfrom
docs/data-protection-controls
Open

feat(rag)!: drop the deployment embedding key; add data-protection docs#1604
DEENUU1 wants to merge 4 commits into
mainfrom
docs/data-protection-controls

Conversation

@DEENUU1

@DEENUU1 DEENUU1 commented Sep 12, 2026

Copy link
Copy Markdown
Member

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 any base_url provider behind a gateway), pymupdf or 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_KEY was a deployment-wide fallback for one provider, left over from when openrouter.ai was hardcoded — and 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 its provider from embedding_providers.json and 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's event_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.md topic map, scripts/docs_drift.py triggers.
  • 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.pyOPENROUTER_API_KEY removed.
  • embedding_providers.json / embedding_providers.py — no deployment_key, no deployment_provider(); require() takes a provider id.
  • embedding_resolution.pyEmbeddingKeySource is ORGANIZATION | 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 name embedding_provider and embedding_secret_id (refused on the field otherwise); app-scoped collections have no vault and stay keyless; clear_embedding_secret removed from the update path, schema and repository; the legacy POST /rag/collections/{name} row records the first catalog provider and waits for a key via PATCH /kb/{id}.
  • GET /rag/embedding-models — no default_provider, no deployment_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 (chooseKey added; deploymentKey, keyHereBillsEmbeddings removed; keyRequiredHere, modelsUnreadable reworded).

Noticed, not fixed

  • No code path removes a chat attachment's bytes. chat_files rows cascade with their message but nothing unlinks the file under MEDIA_DIR; the one caller of FileUploadService.discard is 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.
  • A deleted account keeps its memory. agent_memory_files.owner_key is a string, not a foreign key, and UserService.delete never calls MemoryService.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.ts sets the refreshed cookies' secure from NODE_ENV instead of secureCookies(request) as login/refresh do. One-line fix; kept out of this PR.
  • No self-hosted embedding provider in the catalog, so a fully on-premises deployment cannot use the knowledge base. Not tracked yet; worth its own issue if the city needs RAG without an external embedder.

Testing

  • make test against a real Postgres (docker db + redis from 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).
  • ruff, ty, vulture, deptry; tsc, eslint, prettier, check:i18n, knip — clean.
  • make docs-build (--strict), paragraph/backtick guards, codespell — clean; every page.md#fragment on the new page checked against the target's headings by script.
  • Codex security review: 15 threads, each verified against the code, fixed in the page and resolved.

Breaking

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 gone from KnowledgeBaseUpdate; default_provider and deployment_key are gone from GET /rag/embedding-models. No migration: knowledge_bases.embedding_secret_id stays nullable for app-scoped rows.

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
@DEENUU1 DEENUU1 added documentation Improvements or additions to documentation security Authorization, secrets, or an internet-facing surface labels Sep 12, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ 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.

Comment thread docs/data-protection.md Outdated
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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread docs/data-protection.md Outdated
Comment thread docs/data-protection.md Outdated
Comment thread docs/data-protection.md
Comment thread docs/data-protection.md Outdated
Comment thread docs/data-protection.md Outdated
Comment thread docs/data-protection.md Outdated
Comment thread docs/data-protection.md Outdated
Comment thread docs/data-protection.md Outdated
Comment thread docs/data-protection.md Outdated
Comment thread docs/data-protection.md Outdated
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
@DEENUU1 DEENUU1 changed the title docs(security): add the data-protection page for FA-108 feat(rag)!: drop the deployment embedding key; add data-protection docs Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation security Authorization, secrets, or an internet-facing surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant