mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-10-31 18:02:05 +00:00
feat(semantic): port the retrieval index — and stop it writing secrets to disk
The "semantic (vector search)" entry in not_yet_ported was a misnomer. It is
not vector search and needs no model: MODEL_ID is "seedpass-token-overlap-v1",
and it scores by Jaccard overlap over tokenized metadata. That is why it could
be ported at all — there is nothing to embed.
Porting it surfaced two defects in the Python implementation.
**Stored secrets were written to a plaintext, 0664 file.** `_extract_text`
appended a key_value entry's `value` — the stored secret itself — and
`build()` wrote it to semantic_index/records.json in the clear, along with a
tokenized copy that leaked it just as thoroughly. The file was created at the
process umask, so on a default install it was group- and world-readable, next
to a vault that is encrypted and 0600. Anything that could read the directory
— a backup, a sync client, another user on a shared machine — got the secret
without the master password.
Fixed in Python (index the key NAME, never the value; chmod 0600 on both
files) and never done in TypeScript. The rule is worth keeping simple: nobody
searches for a secret they do not already know, so indexing one is pure
downside. MODEL_ID is bumped to v2 so an index built by the old code is
distinguishable — a v1 index on disk still contains the secrets and should be
rebuilt.
**Entry 0 was silently unsearchable.** The check was
`int(entry.get("id", 0) or 0) <= 0`, conflating "no id" with "id 0" — and
entry ids start at 0, so the first entry any user ever created was absent from
every search result with nothing to indicate why. Found by writing a test that
queried for it. Fixed in both implementations: skip an id that is absent,
unparseable or negative, which is what the check was reaching for.
The TypeScript port is wired into the CLI (`semantic build/status/search`),
the API (the /api/v1/semantic routes are now real rather than 501), and
writes through atomicWrite for a genuine 0600 inode. Fixtures come from
scripts/generate_ts_port_fixtures.py, which asserts the leaked value is absent
from its own output, so the fixture-drift job proves provenance the way it
does for every other fixture.
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
51f1358da4
commit
39c4e45376
13 files changed
+1189
-11
No files matched your search
@@ -1,6 +1,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import time
|
||||
from dataclasses import dataclass
|
||||
@@ -26,7 +27,10 @@ class SemanticIndex:
|
||||
MANIFEST_FILENAME = "manifest.json"
|
||||
RECORDS_FILENAME = "records.json"
|
||||
SCHEMA_VERSION = 1
|
||||
MODEL_ID = "seedpass-token-overlap-v1"
|
||||
#: Bumped from v1 when secret values stopped being indexed. An index
|
||||
#: built by the old code still contains them, so `status()` reports the
|
||||
#: stored id and a mismatch means "rebuild this, it holds secrets".
|
||||
MODEL_ID = "seedpass-token-overlap-v2"
|
||||
|
||||
ALLOWED_KINDS = {
|
||||
"document",
|
||||
@@ -85,6 +89,10 @@ class SemanticIndex:
|
||||
json.dumps(serializable, ensure_ascii=True, sort_keys=True, indent=2),
|
||||
encoding="utf-8",
|
||||
)
|
||||
# Labels, notes, usernames and URLs are still vault contents even with
|
||||
# secret values excluded. Every other file in a profile is 0600; this
|
||||
# one was created at the process umask (0664 on a default install).
|
||||
os.chmod(self.records_path, 0o600)
|
||||
manifest = self._load_manifest()
|
||||
manifest.update(
|
||||
{
|
||||
@@ -157,8 +165,20 @@ class SemanticIndex:
|
||||
) -> list[SemanticRecord]:
|
||||
records: list[SemanticRecord] = []
|
||||
for entry in entries:
|
||||
entry_id = int(entry.get("id", 0) or 0)
|
||||
if entry_id <= 0:
|
||||
# `id` 0 is a real entry -- it is the FIRST one any profile
|
||||
# creates -- so it cannot double as "missing". The old test was
|
||||
# `int(entry.get("id", 0) or 0) <= 0`, which silently dropped it
|
||||
# along with genuinely absent ids, leaving every user's first
|
||||
# entry permanently unfindable by search with nothing to indicate
|
||||
# why.
|
||||
raw_id = entry.get("id")
|
||||
if raw_id is None:
|
||||
continue
|
||||
try:
|
||||
entry_id = int(raw_id)
|
||||
except (TypeError, ValueError):
|
||||
continue
|
||||
if entry_id < 0:
|
||||
continue
|
||||
kind = str(entry.get("kind") or entry.get("type") or "").strip().lower()
|
||||
if kind not in self.ALLOWED_KINDS:
|
||||
@@ -197,8 +217,15 @@ class SemanticIndex:
|
||||
parts.append(str(entry.get("username", "")).strip())
|
||||
parts.append(str(entry.get("url", "")).strip())
|
||||
elif kind == "key_value":
|
||||
# The key NAME only. `value` is the stored secret itself, and it
|
||||
# used to be indexed here -- which wrote it, in the clear, to
|
||||
# semantic_index/records.json alongside a tokenized copy. Nobody
|
||||
# searches for a secret they do not already know, so indexing it
|
||||
# was pure downside: every other file in the profile is encrypted
|
||||
# and 0600, and this one handed the secret to anything that could
|
||||
# read the directory (a backup, a sync client, another user on a
|
||||
# shared box) without the master password.
|
||||
parts.append(str(entry.get("key", "")).strip())
|
||||
parts.append(str(entry.get("value", "")).strip())
|
||||
elif kind == "totp":
|
||||
parts.append(str(entry.get("issuer", "")).strip())
|
||||
elif kind == "nostr":
|
||||
@@ -238,6 +265,7 @@ class SemanticIndex:
|
||||
json.dumps(manifest, ensure_ascii=True, sort_keys=True, indent=2),
|
||||
encoding="utf-8",
|
||||
)
|
||||
os.chmod(self.manifest_path, 0o600)
|
||||
|
||||
def _load_records(self) -> list[dict[str, Any]]:
|
||||
if not self.records_path.exists():
|
||||
|
||||
@@ -124,3 +124,101 @@ def test_semantic_index_service_build_and_status(tmp_path: Path) -> None:
|
||||
|
||||
hybrid = service.search("agent", k=3, mode="hybrid")
|
||||
assert len(hybrid) >= 1
|
||||
|
||||
|
||||
def test_stored_secrets_are_never_written_to_the_index(tmp_path):
|
||||
"""The index is a plaintext file; secrets must not be in it.
|
||||
|
||||
`_extract_text` used to append a key_value entry's `value` -- the stored
|
||||
secret itself -- which `build()` then wrote to
|
||||
semantic_index/records.json in the clear, alongside a tokenized copy that
|
||||
leaked it just as thoroughly. Nobody searches for a secret they do not
|
||||
already know, so indexing it was pure downside: it handed the value to
|
||||
anything that could read the profile directory (a backup, a sync client,
|
||||
another user on a shared machine) without the master password.
|
||||
"""
|
||||
from seedpass.core.semantic_index import SemanticIndex
|
||||
|
||||
index = SemanticIndex(tmp_path)
|
||||
index.build(
|
||||
[
|
||||
{
|
||||
"id": 1,
|
||||
"kind": "key_value",
|
||||
"label": "api-token",
|
||||
"key": "DEPLOY_TOKEN",
|
||||
"value": "SUPER-SECRET-VALUE-12345",
|
||||
"notes": "production credentials",
|
||||
"tags": ["ops"],
|
||||
}
|
||||
]
|
||||
)
|
||||
|
||||
raw = (tmp_path / "semantic_index" / "records.json").read_text(encoding="utf-8")
|
||||
assert "SUPER-SECRET-VALUE-12345" not in raw
|
||||
# Tokenized fragments leak it just as well as the whole string.
|
||||
for fragment in ("super", "12345"):
|
||||
assert fragment not in raw.lower()
|
||||
|
||||
# The entry is still findable by everything that is not the secret.
|
||||
assert index.search("token")
|
||||
assert index.search("production")
|
||||
assert "DEPLOY_TOKEN" in raw
|
||||
|
||||
|
||||
def test_index_files_are_not_readable_by_other_users(tmp_path):
|
||||
"""Every other file in a profile is 0600; these were created at the umask."""
|
||||
from seedpass.core.semantic_index import SemanticIndex
|
||||
|
||||
index = SemanticIndex(tmp_path)
|
||||
index.set_enabled(True)
|
||||
index.build([{"id": 1, "kind": "document", "label": "notes", "content": "hello"}])
|
||||
for name in ("records.json", "manifest.json"):
|
||||
mode = (tmp_path / "semantic_index" / name).stat().st_mode & 0o777
|
||||
assert mode == 0o600, f"{name} is {oct(mode)}"
|
||||
|
||||
|
||||
def test_model_id_marks_indexes_built_before_the_fix(tmp_path):
|
||||
"""A stale index still contains secrets, so it has to be distinguishable."""
|
||||
from seedpass.core.semantic_index import SemanticIndex
|
||||
|
||||
index = SemanticIndex(tmp_path)
|
||||
index.build([{"id": 1, "kind": "document", "label": "notes", "content": "hello"}])
|
||||
assert index.status()["model_id"] == "seedpass-token-overlap-v2"
|
||||
|
||||
|
||||
def test_the_first_entry_a_profile_creates_is_searchable(tmp_path):
|
||||
"""Entry 0 is a real entry, not a missing one.
|
||||
|
||||
The check was `int(entry.get("id", 0) or 0) <= 0`, which conflated "no
|
||||
id" with "id 0" — and entry ids start at 0, so the first entry any user
|
||||
ever created was silently absent from every search result, with nothing
|
||||
to indicate why.
|
||||
"""
|
||||
from seedpass.core.semantic_index import SemanticIndex
|
||||
|
||||
index = SemanticIndex(tmp_path)
|
||||
index.build(
|
||||
[
|
||||
{"id": 0, "kind": "password", "label": "bank.example", "notes": "first"},
|
||||
{"id": 1, "kind": "password", "label": "forum.example", "notes": "second"},
|
||||
]
|
||||
)
|
||||
assert [hit["entry_id"] for hit in index.search("bank")] == [0]
|
||||
assert [hit["entry_id"] for hit in index.search("forum")] == [1]
|
||||
|
||||
|
||||
def test_entries_without_a_usable_id_are_still_skipped(tmp_path):
|
||||
"""Which is what the old check was reaching for."""
|
||||
from seedpass.core.semantic_index import SemanticIndex
|
||||
|
||||
index = SemanticIndex(tmp_path)
|
||||
index.build(
|
||||
[
|
||||
{"kind": "document", "label": "no id", "content": "text"},
|
||||
{"id": None, "kind": "document", "label": "null id", "content": "text"},
|
||||
{"id": "abc", "kind": "document", "label": "bad id", "content": "text"},
|
||||
{"id": -1, "kind": "document", "label": "negative", "content": "text"},
|
||||
]
|
||||
)
|
||||
assert index.search("text") == []
|
||||
Reference in new issue
Block a user