From 885cbe5665498c442ad1d3bebabf5e010db77961 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN Date: Wed, 19 Aug 2026 10:02:31 -0400 Subject: [PATCH] fix(semantic): delete indexes written before secrets stopped being indexed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixing the writer does not rewrite files already on disk, so a profile that built an index before today still has one holding stored secrets in plaintext. Reporting a version string in a status field and expecting the user to notice is not a remedy for that. Both implementations now check the manifest's model_id on every read — status, search, and the API equivalents — and delete records + manifest when it predates the fix. Reading a stale index would also mean serving results derived from the secrets it should never have held, so search purges before it reads rather than after. Deleting is safe: the index is a derived cache, rebuilt from the vault in milliseconds, and the user is told to rebuild. A manifest that cannot be parsed is deliberately NOT treated as stale. "Cannot tell" must not mean "delete it", or every corrupt-manifest read would throw away a working index; that case is tested on both sides. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN --- TODO.md | 12 +++-- js/packages/cli/src/api/routes.ts | 38 +++++++++++---- js/packages/cli/src/program.ts | 51 ++++++++++++++++---- js/packages/cli/test/api.test.ts | 43 +++++++++++++++++ js/packages/core/src/index.ts | 1 + js/packages/core/src/vault/semanticIndex.ts | 18 +++++++ src/seedpass/core/semantic_index.py | 36 ++++++++++++++ src/tests/test_semantic_index.py | 52 +++++++++++++++++++++ 8 files changed, 228 insertions(+), 23 deletions(-) diff --git a/TODO.md b/TODO.md index 5389c509..75d5fc70 100644 --- a/TODO.md +++ b/TODO.md @@ -421,10 +421,14 @@ the fix does not rewrite existing files. `MODEL_ID` is bumped to `seedpass-token-overlap-v2` so a stale index is identifiable — `semantic status` reporting v1 means "rebuild this, it holds secrets". -- [ ] **Decide whether to auto-detect and delete v1 index files on upgrade**, - rather than relying on the user to notice the model id. Leaning yes: the - file has no value once stale, and leaving it is leaving plaintext - secrets on disk. +- [x] **(done 2026-08-19) Stale index files are deleted on first touch.** + Both implementations check the manifest's `model_id` whenever they read + an index, and remove records + manifest if it predates the fix, so the + plaintext does not sit there waiting for a user to notice a version + string. Safe because the index is a derived cache rebuilt in + milliseconds. A manifest that cannot be read is NOT treated as stale — + "cannot tell" must not mean "delete it", or a corrupt-manifest read + would throw away a good index. ### Unbuilt milestones diff --git a/js/packages/cli/src/api/routes.ts b/js/packages/cli/src/api/routes.ts index 78ca9e4f..04222c6b 100644 --- a/js/packages/cli/src/api/routes.ts +++ b/js/packages/cli/src/api/routes.ts @@ -8,7 +8,7 @@ * is the part nobody notices — which secret a given entry produces. */ -import { readFile, mkdir } from "node:fs/promises"; +import { readFile, mkdir, rm } from "node:fs/promises"; import { existsSync } from "node:fs"; import { basename, dirname, join, resolve as resolvePath } from "node:path"; import { @@ -42,6 +42,7 @@ import { searchSemanticRecords, semanticManifest, semanticStatus, + isStaleSemanticIndex, type SemanticRecord, totpCodeAt, Bip85, @@ -822,6 +823,30 @@ export function registerRoutes(server: ApiServer, ctx: ApiContext): void { const semanticRecordsPath = (): string => join(semanticDir(), "records.json"); const semanticManifestPath = (): string => join(semanticDir(), "manifest.json"); + async function readSemanticManifest(): Promise> { + if (!existsSync(semanticManifestPath())) return {}; + try { + return JSON.parse(await readFile(semanticManifestPath(), "utf8")) as Record; + } catch { + return {}; + } + } + + /** + * Delete an index written before secrets stopped being indexed. + * + * Such a file holds stored secrets in the clear, and fixing the writer does + * not rewrite what is already on disk. Deleting is safe — it is a derived + * cache — and serving results from it would mean serving results derived + * from secrets it should never have held. + */ + async function purgeStaleSemantic(): Promise { + if (!isStaleSemanticIndex(await readSemanticManifest())) return; + for (const path of [semanticRecordsPath(), semanticManifestPath()]) { + if (existsSync(path)) await rm(path, { force: true }); + } + } + async function readSemanticRecords(): Promise { if (!existsSync(semanticRecordsPath())) return []; try { @@ -862,14 +887,8 @@ export function registerRoutes(server: ApiServer, ctx: ApiContext): void { } server.route("GET", "/api/v1/semantic/status", async () => { - let manifest: Record = {}; - if (existsSync(semanticManifestPath())) { - try { - manifest = JSON.parse(await readFile(semanticManifestPath(), "utf8")); - } catch { - // Corrupt manifest reports as not-built; the fix is a rebuild. - } - } + await purgeStaleSemantic(); + const manifest = await readSemanticManifest(); return { json: semanticStatus(manifest, (await readSemanticRecords()).length) }; }); @@ -883,6 +902,7 @@ export function registerRoutes(server: ApiServer, ctx: ApiContext): void { server.route("POST", "/api/v1/semantic/search", async (req) => { const body = bodyObject(req); + await purgeStaleSemantic(); const records = await readSemanticRecords(); if (records.length === 0) { throw new HttpError(409, "no semantic index; POST /api/v1/semantic/build first"); diff --git a/js/packages/cli/src/program.ts b/js/packages/cli/src/program.ts index cc5c8c08..891074cf 100644 --- a/js/packages/cli/src/program.ts +++ b/js/packages/cli/src/program.ts @@ -30,6 +30,7 @@ import { searchSemanticRecords, semanticManifest, semanticStatus, + isStaleSemanticIndex, type SemanticRecord, generateFingerprint, assertValidMnemonic, @@ -75,7 +76,7 @@ import { sshPublicKeyOpenSsh, utf8, } from "@seedpass/core"; -import { readFile, writeFile, mkdir } from "node:fs/promises"; +import { readFile, writeFile, mkdir, rm } from "node:fs/promises"; import { existsSync, statSync } from "node:fs"; import { basename, dirname, extname, join } from "node:path"; import { homedir } from "node:os"; @@ -1690,6 +1691,40 @@ export function buildProgram(io: ProgramIo = defaultIo): Command { return { dir, records: join(dir, "records.json"), manifest: join(dir, "manifest.json") }; } + async function readManifest(path: string): Promise> { + if (!existsSync(path)) return {}; + try { + return JSON.parse(await readFile(path, "utf8")) as Record; + } catch { + // A corrupt manifest reads as "not built"; the fix is a rebuild, which + // is what status is meant to prompt. + return {}; + } + } + + /** + * Delete an index written before secrets stopped being indexed. + * + * Such a file holds stored secrets in the clear, and fixing the writer does + * not rewrite files already on disk. Deleting is safe — the index is a + * derived cache, rebuilt from the vault in milliseconds — and leaving it is + * leaving plaintext secrets next to an encrypted vault. + */ + async function purgeStaleSemantic(paths: { + records: string; + manifest: string; + }): Promise { + if (!isStaleSemanticIndex(await readManifest(paths.manifest))) return false; + for (const path of [paths.records, paths.manifest]) { + if (existsSync(path)) await rm(path, { force: true }); + } + io.err( + "Removed a semantic index built by an older version: it stored entry " + + "secrets in plaintext. Run 'semantic build' to search again.", + ); + return true; + } + async function readRecords(path: string): Promise { if (!existsSync(path)) return []; try { @@ -1744,15 +1779,8 @@ export function buildProgram(io: ProgramIo = defaultIo): Command { .action(async () => { const opts = program.opts() as GlobalOpts; const paths = await semanticPaths(opts); - let manifest: Record = {}; - if (existsSync(paths.manifest)) { - try { - manifest = JSON.parse(await readFile(paths.manifest, "utf8")); - } catch { - // A corrupt manifest reports as "not built" rather than failing: - // the fix is to rebuild, which the status is meant to prompt. - } - } + await purgeStaleSemantic(paths); + const manifest = await readManifest(paths.manifest); const records = await readRecords(paths.records); io.out(JSON.stringify(semanticStatus(manifest, records.length), null, 2)); }); @@ -1765,6 +1793,9 @@ export function buildProgram(io: ProgramIo = defaultIo): Command { .action(async (query: string, o: { limit: string; kind?: string }) => { const opts = program.opts() as GlobalOpts; const paths = await semanticPaths(opts); + // Reading a stale index would serve results derived from the secrets it + // should never have held. + await purgeStaleSemantic(paths); const records = await readRecords(paths.records); if (records.length === 0) { throw new Error("no semantic index; run 'semantic build' first"); diff --git a/js/packages/cli/test/api.test.ts b/js/packages/cli/test/api.test.ts index 7e2a815a..6025f057 100644 --- a/js/packages/cli/test/api.test.ts +++ b/js/packages/cli/test/api.test.ts @@ -11,6 +11,7 @@ import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { mkdtemp, mkdir, readFile, writeFile, stat } from "node:fs/promises"; +import { existsSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import process from "node:process"; @@ -467,6 +468,48 @@ describe("semantic index", () => { expect((await stat(path)).mode & 0o777).toBe(0o600); }); + it("deletes an index built before secrets stopped being indexed", async () => { + // Such a file holds stored secrets in the clear, and fixing the writer + // does not rewrite what is already on disk. Leaving it and reporting a + // version string in a status field is not a remedy. + await call("POST", "/api/v1/semantic/build"); + const dir = join(app.profileDir(FINGERPRINT), "semantic_index"); + const manifestPath = join(dir, "manifest.json"); + const recordsPath = join(dir, "records.json"); + + const manifest = JSON.parse(await readFile(manifestPath, "utf8")); + manifest.model_id = "seedpass-token-overlap-v1"; + await writeFile(manifestPath, JSON.stringify(manifest)); + await writeFile( + recordsPath, + JSON.stringify([ + { entry_id: 1, kind: "key_value", label: "x", text: "LEAKED-SECRET", tokens: ["leaked"] }, + ]), + ); + + // Touching it at all removes it, and the leaked content never comes back + // out through search. + const status = await call("GET", "/api/v1/semantic/status"); + expect(status.json.built).toBe(false); + expect(existsSync(recordsPath)).toBe(false); + + const hits = await call("POST", "/api/v1/semantic/search", { body: { query: "leaked" } }); + expect(hits.status).toBe(409); + }); + + it("keeps a current index rather than deleting anything it cannot identify", async () => { + // "Cannot tell" must not mean "delete it": that would throw away a good + // index on every corrupt-manifest read. + await call("POST", "/api/v1/semantic/build"); + const dir = join(app.profileDir(FINGERPRINT), "semantic_index"); + await writeFile(join(dir, "manifest.json"), "{ not json"); + const status = await call("GET", "/api/v1/semantic/status"); + expect(status.status).toBe(200); + expect(existsSync(join(dir, "records.json"))).toBe(true); + // Put it back for any later test. + await call("POST", "/api/v1/semantic/build"); + }); + it("says so rather than returning nothing when no index exists", async () => { const fresh = new ApiServer({ host: "127.0.0.1", port: 0, token: TOKEN }); const other = await mkdtemp(join(tmpdir(), "seedpass-nosem-")); diff --git a/js/packages/core/src/index.ts b/js/packages/core/src/index.ts index b07ca85b..188f425a 100644 --- a/js/packages/core/src/index.ts +++ b/js/packages/core/src/index.ts @@ -94,6 +94,7 @@ export { semanticText, semanticManifest, semanticStatus, + isStaleSemanticIndex, tokenize, SEMANTIC_KINDS, SEMANTIC_MODEL_ID, diff --git a/js/packages/core/src/vault/semanticIndex.ts b/js/packages/core/src/vault/semanticIndex.ts index 0fb8a64b..691f3f84 100644 --- a/js/packages/core/src/vault/semanticIndex.ts +++ b/js/packages/core/src/vault/semanticIndex.ts @@ -219,3 +219,21 @@ export function semanticStatus( updated_at: Number(manifest["updated_at"] ?? 0), }; } + +/** + * Is an index on disk one that was written before secrets stopped being + * indexed? + * + * Such a file holds stored secrets in the clear, and fixing the writer does + * not rewrite files already written — so anything that opens an index has to + * check, and callers delete rather than read. Expecting a user to notice a + * version string in a status field is not a remedy. + * + * A missing or unreadable manifest is NOT stale: there is nothing to purge, + * and treating "cannot tell" as "delete it" would throw away a good index. + */ +export function isStaleSemanticIndex(manifest: Record): boolean { + const modelId = manifest["model_id"]; + if (modelId === undefined || modelId === null) return false; + return String(modelId) !== SEMANTIC_MODEL_ID; +} diff --git a/src/seedpass/core/semantic_index.py b/src/seedpass/core/semantic_index.py index 4448674c..4c49bc34 100644 --- a/src/seedpass/core/semantic_index.py +++ b/src/seedpass/core/semantic_index.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import logging import os import re import time @@ -8,6 +9,8 @@ from dataclasses import dataclass from pathlib import Path from typing import Any +logger = logging.getLogger(__name__) + WORD_RE = re.compile(r"[a-z0-9_]+") @@ -50,7 +53,37 @@ class SemanticIndex: self.manifest_path = self.index_dir / self.MANIFEST_FILENAME self.records_path = self.index_dir / self.RECORDS_FILENAME + def purge_if_stale(self) -> bool: + """Delete an index built before secrets stopped being indexed. + + An index whose manifest records an older ``model_id`` was written by + code that put stored secrets into ``records.json`` in the clear. The + fix to that code does not rewrite files already on disk, so the + plaintext sits there until something removes it -- and expecting a + user to notice a version string in a status field is not a remedy. + + Deleting is safe: this index is a derived cache, rebuilt from the + vault in milliseconds. Returns True if anything was removed. + """ + manifest = self._load_manifest() + if not manifest: + return False + if str(manifest.get("model_id", "")) == self.MODEL_ID: + return False + removed = False + for path in (self.records_path, self.manifest_path): + if path.exists(): + path.unlink() + removed = True + if removed: + logger.warning( + "Removed a semantic index built by an older version: it stored " + "entry secrets in plaintext. Rebuild it to search again." + ) + return removed + def status(self) -> dict[str, Any]: + self.purge_if_stale() manifest = self._load_manifest() records = self._load_records() enabled = bool(manifest.get("enabled", False)) @@ -121,6 +154,9 @@ class SemanticIndex: k: int = 10, kind: str | None = None, ) -> list[dict[str, Any]]: + # Reading a stale index would also mean serving results derived from + # the secrets it should never have held. + self.purge_if_stale() records = self._load_records() if not records: return [] diff --git a/src/tests/test_semantic_index.py b/src/tests/test_semantic_index.py index 25aef367..f50d6822 100644 --- a/src/tests/test_semantic_index.py +++ b/src/tests/test_semantic_index.py @@ -222,3 +222,55 @@ def test_entries_without_a_usable_id_are_still_skipped(tmp_path): ] ) assert index.search("text") == [] + + +def test_an_index_built_before_the_fix_is_deleted_on_first_touch(tmp_path): + """A v1 index still holds plaintext secrets; the fix does not rewrite it. + + Leaving it on disk and reporting a version string in a status field is not + a remedy. Deleting is safe -- the index is a derived cache, rebuilt from + the vault in milliseconds. + """ + import json + + from seedpass.core.semantic_index import SemanticIndex + + index = SemanticIndex(tmp_path) + index.build([{"id": 1, "kind": "document", "label": "notes", "content": "hello"}]) + + manifest_path = tmp_path / "semantic_index" / "manifest.json" + records_path = tmp_path / "semantic_index" / "records.json" + manifest = json.loads(manifest_path.read_text()) + manifest["model_id"] = "seedpass-token-overlap-v1" + manifest_path.write_text(json.dumps(manifest)) + records_path.write_text( + json.dumps( + [ + { + "entry_id": 1, + "kind": "key_value", + "label": "x", + "text": "LEAKED-SECRET", + "tokens": ["leaked", "secret"], + } + ] + ) + ) + + # Searching it must neither return the leaked content nor leave it there. + assert index.search("leaked") == [] + assert not records_path.exists() + assert not manifest_path.exists() + assert index.status()["built"] is False + + +def test_an_unreadable_manifest_is_not_treated_as_stale(tmp_path): + """"Cannot tell" must not mean "delete it".""" + from seedpass.core.semantic_index import SemanticIndex + + index = SemanticIndex(tmp_path) + index.build([{"id": 1, "kind": "document", "label": "notes", "content": "hello"}]) + (tmp_path / "semantic_index" / "manifest.json").write_text("{ not json") + + index.status() + assert (tmp_path / "semantic_index" / "records.json").exists()