From c7b3c8195fc820efc7c256eea3ddb2dd17df1833 Mon Sep 17 00:00:00 2001 From: AndresL230 <190146319+AndresL230@users.noreply.github.com> Date: Fri, 31 Jul 2026 12:42:43 -0700 Subject: [PATCH] =?UTF-8?q?docs(adr):=200025=20=E2=80=94=20encrypt=20cours?= =?UTF-8?q?e=5Fchunks.chunk=5Ftext=20(#484)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the decision Andres already made, with the reasoning corrected. The issue's premise ("pgvector similarity can't run over ciphertext") is wrong: match_course_chunks ranks on 1 - (embedding <=> query_embedding) and only SELECTs chunk_text as payload, and nothing queries it by content. So encryption doesn't block retrieval and the decision is cheaper than stated. But the same fact makes it partial, which is the part worth recording: the embedding can't be encrypted (pgvector computes distance over it) and is partially invertible back to its source text. So this restores boundary consistency with documents.extracted_text — it does not make chunks confidential, and the ADR says so explicitly rather than letting a future reader assume otherwise. Decided uniform (document AND catalog chunks) so the invariant is assertable by the existing ciphertext oracle, and because decrypt_if_present's raw-value fallback would make a real decrypt failure indistinguishable from a legitimately-plaintext catalog row in a mixed table. Ids stay computed on plaintext: AES-GCM's random nonce means identical text encrypts differently every time, so ciphertext can never be a dedup key. Flagged that scripts/dedupe_course_chunks.py:70 re-derives ids from the STORED text and would destroy content-addressing if run against encrypted rows. Co-Authored-By: Claude Opus 5 --- docs/decisions/0025-encrypt-rag-chunk-text.md | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 docs/decisions/0025-encrypt-rag-chunk-text.md diff --git a/docs/decisions/0025-encrypt-rag-chunk-text.md b/docs/decisions/0025-encrypt-rag-chunk-text.md new file mode 100644 index 00000000..d76ed0c4 --- /dev/null +++ b/docs/decisions/0025-encrypt-rag-chunk-text.md @@ -0,0 +1,96 @@ +# 0025: Encrypt `course_chunks.chunk_text` — restore the encryption boundary, but the embedding stays plaintext + +- Status: accepted +- Date: 2026-07-31 +- Relates to: #484 (this decision), #483 (blocked on it), #482 (RAG hardening), + #231 (storage/RLS lockdown), migration 0030 (`documents.extracted_text`), + migration 0039 (the vector store) +- Supersedes: none + +## Context + +#484 names a real asymmetry. `documents.extracted_text` is encrypted (0030) +because it is student-uploaded content; `course_chunks.chunk_text` holds *the +same text, chunked*, in plaintext. One column is treated as PII and the other +isn't, for no reason anyone wrote down. + +**The issue's stated premise is wrong, and it matters.** It assumes "pgvector +similarity can't run over ciphertext." `match_course_chunks` (0039) computes +`1 - (c.embedding <=> query_embedding)`, orders by `c.embedding <=> +query_embedding`, and filters `WHERE c.embedding IS NOT NULL`. `chunk_text` is +only ever SELECTed as payload — the ranking never reads it. Nothing in the +codebase queries it by content either (no `ILIKE` / `LIKE` / FTS; verified +across `services/`, `routes/`, `scripts/`). So encryption does not block +retrieval at all, and this decision is far cheaper than the issue implies. + +But the correction cuts both ways, and this is the part worth recording: **the +reason it's cheap is the reason it's partial.** The `embedding` column cannot +be encrypted — pgvector must compute distance over it — and an embedding is a +lossy but real representation of its source text; embedding-inversion +techniques recover substantial content from vectors alone. Encrypting +`chunk_text` therefore does *not* make the row opaque. + +Threat model, for calibration: the backend connects with the service-role key +and RLS locks out `anon`/`authenticated` (#231), so direct table reads imply a +Supabase credential compromise or an insider. `ENCRYPTION_KEY` is a separate +secret held in the app environment, so column encryption genuinely raises the +bar against a database-only compromise — the same bar every other encrypted +column is already set at. + +## Decision + +Encrypt `chunk_text` for **every** row in `course_chunks` — document *and* +catalog — through the standard `encrypt_if_present` / `decrypt_if_present` +helpers, and compute content-addressed ids on the **plaintext, before +encryption**. + +Uniform rather than scoped to document chunks, even though catalog chunks are +public BU course-catalog text with nothing to protect: + +- One invariant — "`chunk_text` is always ciphertext" — is assertable by the + existing `ciphertext` e2e oracle. A per-category rule is not. +- `decrypt_if_present` returns the **raw value** when it cannot decrypt. In a + mixed table that makes a genuine decrypt failure indistinguishable from a + legitimately-plaintext catalog row, which is precisely the kind of silent + degradation #482 was about. +- The cost is ~5 AES-GCM decrypts per retrieval (`k=5`). Immaterial. + +Ids stay keyed on plaintext because AES-GCM uses a random nonce per call: the +same plaintext encrypts to different ciphertext every time, so ciphertext can +never be a dedup key. `chunk_id(course, text)` remains the merge key, and +re-uploads of identical content still converge on one row. + +**This ADR does not claim chunk confidentiality.** It restores boundary +consistency and removes trivially-readable plaintext. The embedding remains, +and it is the residual exposure. + +## Consequences + +- (+) The encryption boundary is consistent: the same student text is + protected in `documents.extracted_text` and in the chunks derived from it. +- (+) Retrieval is unaffected — ranking never touched `chunk_text`. +- (+) Unblocks #483. Notes indexing would otherwise write decrypted note + bodies (`notes.body` is encrypted) into a plaintext column, deepening the + very asymmetry this closes. +- (+) A uniform invariant the `ciphertext` oracle can assert, so a future + regression fails a lane instead of sitting unnoticed. +- (−) **Residual exposure: the embedding stays plaintext and is partially + invertible.** This is defense in depth, not confidentiality. Anyone reading + this ADR to answer "is chunk content protected?" must read this line. +- (−) `scripts/dedupe_course_chunks.py:70` re-derives ids from the **stored** + `chunk_text`. Run against encrypted rows it would hash ciphertext and + destroy content-addressing. It must decrypt before hashing, or be retired — + its own docstring calls it a one-time migration. +- (−) A backfill is required for existing rows (the + `db/backfill_encryption.py` precedent). Until it completes the table is + mixed, carried by `decrypt_if_present`'s raw-value fallback. +- (−) Catalog chunks pay encryption cost for no privacy benefit. Accepted as + the price of the uniform invariant. + +## Implementation (not done here) + +Write sites: `services/rag_service.py::index_document_chunks` and +`scripts/ingest_catalog.py`. Read site: +`services/rag_service.py::retrieve_chunks`, decrypting each returned chunk +before `format_rag_context`. Plus the dedupe-script fix above, a backfill, and +extending the `ciphertext` oracle to cover the column.