mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-10-27 07:52:03 +00:00
fix(semantic): delete indexes written before secrets stopped being indexed
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) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
This commit is contained in:
1 parent
fdbdb12596
commit
885cbe5665
8 files changed
+228
-23
No files matched your search
@@ -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
|
||||
|
||||
|
||||
@@ -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<Record<string, unknown>> {
|
||||
if (!existsSync(semanticManifestPath())) return {};
|
||||
try {
|
||||
return JSON.parse(await readFile(semanticManifestPath(), "utf8")) as Record<string, unknown>;
|
||||
} 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<void> {
|
||||
if (!isStaleSemanticIndex(await readSemanticManifest())) return;
|
||||
for (const path of [semanticRecordsPath(), semanticManifestPath()]) {
|
||||
if (existsSync(path)) await rm(path, { force: true });
|
||||
}
|
||||
}
|
||||
|
||||
async function readSemanticRecords(): Promise<SemanticRecord[]> {
|
||||
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<string, unknown> = {};
|
||||
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");
|
||||
|
||||
@@ -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<Record<string, unknown>> {
|
||||
if (!existsSync(path)) return {};
|
||||
try {
|
||||
return JSON.parse(await readFile(path, "utf8")) as Record<string, unknown>;
|
||||
} 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<boolean> {
|
||||
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<SemanticRecord[]> {
|
||||
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<string, unknown> = {};
|
||||
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");
|
||||
|
||||
@@ -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-"));
|
||||
|
||||
@@ -94,6 +94,7 @@ export {
|
||||
semanticText,
|
||||
semanticManifest,
|
||||
semanticStatus,
|
||||
isStaleSemanticIndex,
|
||||
tokenize,
|
||||
SEMANTIC_KINDS,
|
||||
SEMANTIC_MODEL_ID,
|
||||
|
||||
@@ -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<string, unknown>): boolean {
|
||||
const modelId = manifest["model_id"];
|
||||
if (modelId === undefined || modelId === null) return false;
|
||||
return String(modelId) !== SEMANTIC_MODEL_ID;
|
||||
}
|
||||
@@ -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 []
|
||||
|
||||
@@ -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()
|
||||
Reference in new issue
Block a user