diff --git a/backend/asset_store.py b/backend/asset_store.py new file mode 100644 index 0000000..2512c7c --- /dev/null +++ b/backend/asset_store.py @@ -0,0 +1,146 @@ +"""ADR-009 stage 9.5: the content-addressed binary asset store. + +WHAT THIS REPLACES. Image and chart bytes used to be base64'd inline into +the chat's `data` blob (see session_save.py's own _serialize_image_node). +That has two costs the ADR names: autosave rewrites every one of those +megabytes on every 30-second tick even when no image changed, and an +export has no way to carry assets as real files. Both dissolve once bytes +live outside the blob and the blob carries only a reference. + +CONTENT-ADDRESSED, so the reference IS the integrity check: a ref is the +SHA-256 of the bytes it names. Two nodes holding the same image +deduplicate for free, a re-save of unchanged bytes is a no-op rather than +a rewrite, and a corrupted file is detectable by rehashing rather than +being silently served as if it were fine. + +WRITES ARE ATOMIC AND IDEMPOTENT. put() writes to a temp name and +os.replace()s into place, so a crash mid-write can only ever leave a temp +file, never a truncated file wearing a real ref's name that a later read +would trust. If the target already exists, put() returns immediately +without rewriting - by construction the existing file already has exactly +the content being stored, since its name is that content's hash. + +NOTHING IS EVER DELETED HERE. Garbage collection of unreferenced assets is +deliberately out of scope: an asset is cheap to keep and catastrophic to +delete while some chat still points at it, and "which chats reference +which assets" is a whole-database question this module has no business +answering. A future sweep belongs alongside the export/import story, with +its own recon. + +Two-character shard directories keep any single directory from +accumulating tens of thousands of entries, which some filesystems handle +poorly - the same convention git's own object store uses. +""" + +from __future__ import annotations + +import hashlib +import logging +import os +from pathlib import Path + +logger = logging.getLogger(__name__) + +# Kept deliberately small: this maps the mime types this app actually +# produces (image nodes and chart PNGs), not a general registry. An +# unknown type falls back to .bin rather than guessing. +_EXTENSION_BY_MIME = { + "image/png": "png", + "image/jpeg": "jpg", + "image/gif": "gif", + "image/webp": "webp", + "image/svg+xml": "svg", +} + + +def content_ref(data: bytes) -> str: + """The SHA-256 hex digest that names these exact bytes.""" + return hashlib.sha256(data).hexdigest() + + +def extension_for_mime(mime_type: str | None) -> str: + return _EXTENSION_BY_MIME.get((mime_type or "").lower(), "bin") + + +def assets_dir_for(db_path: Path) -> Path: + """Assets live NEXT TO the database that references them + (`db_path.parent / "assets"`), matching backend/db_backup.py's own + backups_dir_for convention - so a test passing an isolated + `tmp_path / "chats.db"` gets an isolated asset store for free, and the + real default lands at ~/.graphlink/assets/.""" + return db_path.parent / "assets" + + +def store_for(db_path: Path) -> "AssetStore": + """The live asset store belonging to a database. The one place the + save/load paths call to get a store, so "which directory" is decided + here rather than at four separate call sites.""" + return AssetStore(assets_dir_for(db_path)) + + +class AssetStore: + """A directory of content-addressed blobs. Construct with the directory + itself, not a db path, so it is equally usable for the live store and + for an export's staging area.""" + + def __init__(self, root: Path): + self.root = Path(root) + + def _path_for(self, ref: str) -> Path: + return self.root / ref[:2] / ref + + def exists(self, ref: str) -> bool: + return self._path_for(ref).is_file() + + def put(self, data: bytes) -> str: + """Stores `data` and returns its ref. Idempotent: storing identical + bytes twice writes once and returns the same ref both times.""" + ref = content_ref(data) + target = self._path_for(ref) + if target.is_file(): + return ref + + target.parent.mkdir(parents=True, exist_ok=True) + tmp = target.with_name(target.name + ".tmp") + try: + fd = os.open(tmp, os.O_CREAT | os.O_WRONLY | os.O_TRUNC, 0o600) + with os.fdopen(fd, "wb") as handle: + handle.write(data) + handle.flush() + os.fsync(handle.fileno()) + os.replace(tmp, target) + except BaseException: + try: + tmp.unlink(missing_ok=True) + except OSError: + pass + raise + try: + os.chmod(target, 0o600) + except OSError: + logger.warning("could not chmod asset %s to 0600 - continuing", target) + return ref + + def get(self, ref: str) -> bytes | None: + """The bytes for `ref`, or None if this store has never seen it. + + None rather than an exception because a missing asset must degrade + to "this image does not render" rather than "this chat will not + load" - a chat's text is worth far more than one of its pictures, + and an import from a bundle whose assets were stripped is a + legitimate, survivable state.""" + target = self._path_for(ref) + if not target.is_file(): + return None + try: + return target.read_bytes() + except OSError: + logger.warning("could not read asset %s - treating as missing", target) + return None + + def verify(self, ref: str) -> bool: + """True only if the stored bytes still hash to their own name - + the integrity check content addressing makes possible. Used by the + import path, which is reading files it did not write.""" + data = self.get(ref) + return data is not None and content_ref(data) == ref diff --git a/backend/autosave.py b/backend/autosave.py index 7bc66c5..5295645 100644 --- a/backend/autosave.py +++ b/backend/autosave.py @@ -82,6 +82,7 @@ from pathlib import Path from typing import Any +from backend.asset_store import store_for from backend.canvas import SceneDocument from backend.chat_library import ( AUTOSAVE_OWNER, @@ -121,7 +122,11 @@ async def autosave_tick( return try: - chat_data = build_chat_data(canvas_document) + # ADR-009 stage 9.5: image bytes go to the content-addressed store + # and only a ref is written into the row. This path is the whole + # reason that stage exists - without it every 30-second tick + # rewrote megabytes of base64 for pictures that had not changed. + chat_data = build_chat_data(canvas_document, asset_store=store_for(db_path)) except Exception: logger.exception("autosave: failed to build chat data for session %r", bus.session_id) return diff --git a/backend/chat_library.py b/backend/chat_library.py index 8ba3198..41ddd27 100644 --- a/backend/chat_library.py +++ b/backend/chat_library.py @@ -51,6 +51,7 @@ from backend.events import SessionBus from backend.notifications import NotificationState from backend.session_load import restore_chat_into_document +from backend.asset_store import store_for from backend.session_save import build_chat_data from graphlink_migrations import run_sqlite_migrations @@ -1178,7 +1179,10 @@ async def load_chat(chat_id: int): if canvas_document is None: return - restore_chat_into_document(canvas_document, row, notes_rows, pins_rows) + restore_chat_into_document( + canvas_document, row, notes_rows, pins_rows, + asset_store=store_for(resolved_path), + ) # R6.5: remember which row this scene now corresponds to, so a # later Save updates THIS row instead of always inserting a new # one - the backend analog of ChatSessionManager.current_chat_id @@ -1189,7 +1193,11 @@ async def load_chat(chat_id: int): # byte-identical row and bumped updated_at, re-sorting the Chat # Library under the user for a session they had only just opened. try: - fresh = build_chat_data(canvas_document) + # Must use the SAME store as autosave below, or the + # first tick after a load would see a payload that + # differs only in image representation and rewrite a + # row that is already correct. + fresh = build_chat_data(canvas_document, asset_store=store_for(resolved_path)) fresh_notes = fresh.pop("notes_data", []) fresh_pins = fresh.pop("pins_data", []) # ADR-009 stage 9.2: row["updated_at"] is the value that @@ -1275,7 +1283,7 @@ async def save_chat(): return try: - chat_data = build_chat_data(canvas_document) + chat_data = build_chat_data(canvas_document, asset_store=store_for(resolved_path)) except Exception as exc: if notifications is not None: notifications.show(f"Failed to prepare chat save payload: {exc}", "error") @@ -1573,7 +1581,7 @@ def flush_dirty_session_before_teardown( return try: - chat_data = build_chat_data(canvas_document) + chat_data = build_chat_data(canvas_document, asset_store=store_for(db_path)) except Exception: logger.exception("eviction flush: failed to build chat data - the last edit may be lost") return diff --git a/backend/secret_scrub.py b/backend/secret_scrub.py new file mode 100644 index 0000000..8708a2b --- /dev/null +++ b/backend/secret_scrub.py @@ -0,0 +1,135 @@ +"""ADR-009 stage 9.3: the one secret-scrub chokepoint. + +Every surface that lets data LEAVE this machine goes through `scrub()`: +the `.graphlink` export (stage 9.4), and - once they exist - ADR-014/008 +recipe templates and any future share surface. One function, one test +file, one place to audit. + +WHY VALUE-BASED, NOT JUST KEY-BASED. A scrubber that only redacts known +field NAMES (`openai_api_key`, ...) is one refactor away from useless: the +day a secret gets copied into a differently-named field, or interpolated +into an error string stored on a node, or pasted by the user into their own +chat text, a name-only filter waves it straight through. So this scrubs on +BOTH axes - the name of the field it sits in, AND the shape of the value +itself. A value that looks like a credential is redacted no matter what +key it arrived under, including inside free text. + +WHY PATHS TOO. An absolute path is not a credential but it is personal: +`C:\\Users\\\\...` carries the operator's account name, and +deeper segments carry private folder and file names. Paths are redacted +whole rather than trimmed to a basename, because the basename is exactly +where the private part usually lives (`quarterly_layoffs.xlsx`). + +DELIBERATELY OVER-BROAD. This function will sometimes redact a string that +was not actually a secret - a base64 blob, a long opaque id, a path the +user pasted deliberately. That direction is correct: a redacted non-secret +costs a slightly less useful export, while a leaked real secret costs the +user their account. Every rule here is chosen to fail toward redaction. + +NOT ENCRYPTION, NOT AUTHORIZATION. This removes secrets from data already +destined to leave. It does not decide WHETHER something may leave; callers +own that. +""" + +from __future__ import annotations + +import re +from typing import Any + +REDACTED = "[redacted]" +REDACTED_PATH = "[redacted-path]" + +# Field names whose VALUE is always a secret, matched case-insensitively. +# Exact names come from graphlink_settings_store.SettingsManager.SECRET_KEYS +# (kept in sync by test_secret_scrub.py, which imports that tuple and +# asserts every member is covered here) - the suffix rules below then +# generalize to fields that do not exist yet. +_SECRET_KEY_SUFFIXES = ( + "api_key", + "apikey", + "access_token", + "refresh_token", + "auth_token", + "token", + "secret", + "password", + "passphrase", + "credential", + "credentials", + "private_key", +) + +# Value shapes that are a credential regardless of the field they sit in. +# Each is anchored to a real issuer format rather than "long random-looking +# string", so ordinary content is not shredded wholesale. +_SECRET_VALUE_PATTERNS = ( + re.compile(r"sk-[A-Za-z0-9_\-]{16,}"), # OpenAI-style + re.compile(r"sk-ant-[A-Za-z0-9_\-]{16,}"), # Anthropic + re.compile(r"AIza[A-Za-z0-9_\-]{20,}"), # Google API key + re.compile(r"gh[pousr]_[A-Za-z0-9]{20,}"), # GitHub token family + re.compile(r"github_pat_[A-Za-z0-9_]{20,}"), # GitHub fine-grained PAT + re.compile(r"xox[baprs]-[A-Za-z0-9\-]{10,}"), # Slack + re.compile(r"\bBearer\s+[A-Za-z0-9._\-]{16,}"), # generic bearer header + # This codebase's own at-rest wrapper (graphlink_secrets.py). The + # ciphertext is only decryptable by this Windows account, but it is + # still the secret's stored form and has no business in an export. + re.compile(r"dpapi:[A-Za-z0-9+/=]+"), +) + +# Absolute filesystem paths: Windows drive-letter, UNC, and POSIX home-ish +# roots. Deliberately NOT a bare `/...` match - that would eat ordinary +# prose containing slashes, and URLs, for no gain. +_PATH_PATTERNS = ( + re.compile(r"[A-Za-z]:[\\/][^\s\"'<>|]*"), + re.compile(r"\\\\[^\s\"'<>|]+"), + re.compile(r"/(?:home|Users|root|var|tmp|opt|mnt|media)/[^\s\"'<>|]*"), +) + + +def _key_is_secret(key: str) -> bool: + lowered = key.lower() + return any(lowered.endswith(suffix) for suffix in _SECRET_KEY_SUFFIXES) + + +def scrub_text(text: str) -> str: + """Redacts credential-shaped and path-shaped substrings inside one + string, leaving the surrounding text intact. Applied to every string + reached by scrub(), not just to values under suspicious keys - an error + message stored on a node ("failed to read C:\\Users\\ada\\taxes.csv") + is exactly the kind of incidental leak a key-name filter misses.""" + result = text + for pattern in _SECRET_VALUE_PATTERNS: + result = pattern.sub(REDACTED, result) + for pattern in _PATH_PATTERNS: + result = pattern.sub(REDACTED_PATH, result) + return result + + +def scrub(value: Any) -> Any: + """Returns a scrubbed deep copy of `value`. Never mutates its input - + callers routinely pass live in-memory state (a SceneDocument's own + payload) that must not be damaged by the act of exporting it. + + Containers recurse; strings go through scrub_text; a value under a + secret-named key is replaced wholesale rather than pattern-matched, + since a credential in a field literally called `api_key` should not + have to match a known issuer format to be caught. Non-string scalars + (int/float/bool/None) pass through untouched - they cannot carry a + credential, and mangling them would corrupt the export's structure.""" + if isinstance(value, dict): + scrubbed: dict[Any, Any] = {} + for key, item in value.items(): + if isinstance(key, str) and _key_is_secret(key): + # Preserve "was set" vs "was empty" - useful in a bug + # report, and it leaks nothing. + scrubbed[key] = REDACTED if item else item + else: + scrubbed[key] = scrub(item) + return scrubbed + if isinstance(value, list): + return [scrub(item) for item in value] + if isinstance(value, tuple): + return tuple(scrub(item) for item in value) + if isinstance(value, str): + return scrub_text(value) + return value diff --git a/backend/session_load.py b/backend/session_load.py index 18c954a..41e186b 100644 --- a/backend/session_load.py +++ b/backend/session_load.py @@ -181,6 +181,7 @@ from __future__ import annotations +import contextvars import re import uuid from typing import Any @@ -205,6 +206,20 @@ from graphlink_chart_data import ChartDataError, canonicalize_chart_data from graphlink_navigation_pins import NavigationPinRecord +# ADR-009 stage 9.5: the asset store in effect for the CURRENT restore. +# +# Threaded as a contextvar rather than as a parameter because the per-kind +# restorer dispatch table below is ~20 lambdas all sharing the signature +# (payload, document); widening every one of them to carry a store only +# _restore_image_payload reads would be churn for no gain. A contextvar, +# not a bare module global, so the value can never leak between concurrently +# restoring sessions - restore_chat_into_document sets it, and resets it in +# a finally, around a fully synchronous call (no awaits inside), so the +# window is atomic with respect to the event loop. +_ACTIVE_ASSET_STORE: contextvars.ContextVar = contextvars.ContextVar( + "graphlink_active_asset_store", default=None +) + # -- small, generic helpers -------------------------------------------------- @@ -439,16 +454,38 @@ def _restore_image_payload(payload: dict[str, Any], document: SceneDocument) -> x, y = _position(payload) node = SceneNode(id="", x=x, y=y, title="Image", kind="image", state=ImageState()) - raw_b64 = payload.get("image_bytes") - if isinstance(raw_b64, str) and raw_b64: - try: - image_bytes = _content_codec.decode_image_bytes(raw_b64) - except Exception: - image_bytes = b"" - if image_bytes: - asset_id = f"img{_uuid.uuid4().hex}" - document.image_assets[asset_id] = (image_bytes, "image/png") - node.state.image_asset_id = asset_id + asset_store = _ACTIVE_ASSET_STORE.get() + + # ADR-009 stage 9.5: READ BOTH SHAPES. A chat saved with an asset store + # in play carries only `asset_ref`; every chat saved before that (and + # any saved without a store) still carries inline base64 `image_bytes`. + # Both are read here so no stored row ever has to be rewritten - which + # is what lets the externalization roll out without a destructive + # migration over real user data. + image_bytes = b"" + mime_type = "image/png" + asset_ref = payload.get("asset_ref") + if asset_store is not None and isinstance(asset_ref, str) and asset_ref: + stored = asset_store.get(asset_ref) + if stored is not None: + image_bytes = stored + mime_type = str(payload.get("mime_type") or "image/png") + # A ref the store has never seen degrades to "this image does not + # render", never to "this chat will not load" - the conversation is + # worth far more than one of its pictures. + + if not image_bytes: + raw_b64 = payload.get("image_bytes") + if isinstance(raw_b64, str) and raw_b64: + try: + image_bytes = _content_codec.decode_image_bytes(raw_b64) + except Exception: + image_bytes = b"" + + if image_bytes: + asset_id = f"img{_uuid.uuid4().hex}" + document.image_assets[asset_id] = (image_bytes, mime_type) + node.state.image_asset_id = asset_id node.content = str(payload.get("prompt", "")) return node @@ -996,6 +1033,51 @@ def _restore_system_prompt_and_summary_connections( continue +def _restore_flat_edges( + document: SceneDocument, + chat_data: dict[str, Any], + by_payload_id: dict[str, str], +) -> bool: + """ADR-009 stage 9.6: restore edges from the flat `edges` list. + + Returns True if that list was present and used, False to tell the + caller to fall back to the legacy bucket reconstruction below. + + Why this exists at all: the 12 legacy connection lists plus the + system-prompt/group-summary pair lists are a *classification* of edges + invented by an app that had 14 separate visual edge types. This backend + has exactly one - document.connect() - so save-side classification is a + guess (see session_save._classify_edges' own comments) and load-side + reconstruction has to unpick that guess through index-vs-id fallbacks + that differ per bucket. A flat list of (source, target) by persistent + id is what the document actually holds, so it round-trips exactly and + needs no reconstruction. + + Endpoints resolve against a map spanning nodes, notes AND charts, + because an edge in this document may legitimately end on any of them - + the legacy path needed three different maps for the same reason. An + endpoint that no longer resolves (hand-edited file, a node dropped by + an earlier restore step) skips that one edge; same tolerant posture as + every other reference restored here, since a missing line must never + cost the user the whole conversation.""" + entries = chat_data.get("edges") + if not isinstance(entries, list): + return False + + for entry in entries: + if not isinstance(entry, dict): + continue + source_id = by_payload_id.get(str(entry.get("source"))) + target_id = by_payload_id.get(str(entry.get("target"))) + if source_id is None or target_id is None or source_id == target_id: + continue + try: + document.connect(source_id, target_id) + except Exception: + continue + return True + + def _restore_branch_provenance_item_ids( document: SceneDocument, node_payloads: list, @@ -1082,6 +1164,25 @@ def _restore_view_state(document: SceneDocument, chat_data: dict[str, Any]) -> N def restore_chat_into_document( + document: SceneDocument, + chat: dict[str, Any], + notes_data: list, + pins_data: list, + asset_store: Any | None = None, +) -> None: + """ADR-009 stage 9.5 entry point. Publishes `asset_store` for the + duration of this restore (see _ACTIVE_ASSET_STORE's own comment for why + a contextvar and not a parameter), then delegates. The reset is in a + finally so a failed restore can never leave a stale store visible to + the next one.""" + token = _ACTIVE_ASSET_STORE.set(asset_store) + try: + _restore_chat_into_document(document, chat, notes_data, pins_data) + finally: + _ACTIVE_ASSET_STORE.reset(token) + + +def _restore_chat_into_document( document: SceneDocument, chat: dict[str, Any], notes_data: list, pins_data: list, ) -> None: """The top-level orchestrator - ports SceneDeserializer.restore_chat()'s @@ -1159,8 +1260,30 @@ def restore_chat_into_document( all_items_map[node_slot_count + note_slot_count + chart_slot_count + frame_index] = frame_new_id _restore_containers(document, chat_data.get("containers", []), all_items_map) - _restore_basic_connections(document, chat_data, all_nodes_map, nodes_by_id) - _restore_system_prompt_and_summary_connections(document, chat_data, notes_map, chat_nodes_map, nodes_by_id) + # ADR-009 stage 9.6. A file written by this build carries a flat + # `edges` list and it is authoritative; the legacy buckets are only + # consulted for files written before this stage. Structural parent and + # child edges have already been created by the restore loops above - + # re-asserting them here is harmless because SceneDocument.connect() is + # idempotent on (source, target), which is also what makes it safe for + # the flat list to simply contain EVERY edge rather than only the ones + # no earlier step covered. + by_payload_id = dict(nodes_by_id) + if isinstance(notes_data, list): + for note_index, note_payload in enumerate(notes_data): + note_new_id = notes_map.get(note_index) + if not isinstance(note_payload, dict) or note_new_id is None: + continue + note_payload_id = note_payload.get("id") + if note_payload_id: + by_payload_id[str(note_payload_id)] = note_new_id + by_payload_id.update(charts_by_id) + + if not _restore_flat_edges(document, chat_data, by_payload_id): + _restore_basic_connections(document, chat_data, all_nodes_map, nodes_by_id) + _restore_system_prompt_and_summary_connections( + document, chat_data, notes_map, chat_nodes_map, nodes_by_id + ) _restore_pins(document, pins_data) _restore_view_state(document, chat_data) diff --git a/backend/session_save.py b/backend/session_save.py index 7d09c1a..485c90a 100644 --- a/backend/session_save.py +++ b/backend/session_save.py @@ -108,11 +108,22 @@ from __future__ import annotations +import contextvars import re from typing import Any from backend.canvas import SceneDocument, SceneNode, _content_codec +# ADR-009 stage 9.5: the asset store in effect for the CURRENT save. Same +# contextvar rationale as session_load.py's own _ACTIVE_ASSET_STORE - the +# per-kind serializer dispatch below is a table of (node, document) +# lambdas, and widening all of them for the one kind that needs a store +# would be churn. Set and reset by build_chat_data around a synchronous +# call, so it can never leak between sessions. +_ACTIVE_SAVE_ASSET_STORE: contextvars.ContextVar = contextvars.ContextVar( + "graphlink_active_save_asset_store", default=None +) + # The 12 "regular" node kinds - everything that is NOT note/frame/container/ # chart. Mirrors scene_index.py's own NODE_LIST_NAMES (7 kinds, the current, # post-R5-closeout surviving set) PLUS the 5 kinds R5-closeout deleted from @@ -247,9 +258,31 @@ def _serialize_document_node(node: SceneNode) -> dict[str, Any]: } -def _serialize_image_node(node: SceneNode, document: SceneDocument) -> dict[str, Any]: +def _serialize_image_node( + node: SceneNode, document: SceneDocument, asset_store: Any | None = None +) -> dict[str, Any]: + """ADR-009 stage 9.5: writes the image's bytes to the content-addressed + asset store when one is supplied, emitting only a ref - so autosave + stops rewriting megabytes of base64 on every 30-second tick for an + image that has not changed. + + WRITE-NEW / READ-BOTH, not a destructive migration. With no store + (every existing direct caller and test), this emits the historical + inline `image_bytes` exactly as before. session_load.py reads either + shape, so a chat saved by an older build keeps loading untouched and no + row ever has to be rewritten to make this safe. The inline path is + what a future cleanup deletes, once no old rows remain in the wild.""" asset = document.image_assets.get(node.state.image_asset_id) image_bytes = asset[0] if asset is not None else b"" + mime_type = asset[1] if asset is not None else "image/png" + + if asset_store is not None and image_bytes: + return { + "node_type": "image", + "asset_ref": asset_store.put(image_bytes), + "mime_type": mime_type, + "prompt": node.content, + } return { "node_type": "image", "image_bytes": _content_codec.encode_image_bytes(image_bytes), @@ -363,7 +396,7 @@ def _serialize_code_sandbox_node(node: SceneNode) -> dict[str, Any]: "chat": lambda node, document: _serialize_chat_node(node), "code": lambda node, document: _serialize_code_node(node), "document": lambda node, document: _serialize_document_node(node), - "image": lambda node, document: _serialize_image_node(node, document), + "image": lambda node, document: _serialize_image_node(node, document, _ACTIVE_SAVE_ASSET_STORE.get()), "thinking": lambda node, document: _serialize_thinking_node(node), "conversation": lambda node, document: _serialize_conversation_node(node), "html": lambda node, document: _serialize_html_node(node), @@ -597,7 +630,19 @@ def _classify_edges( } -def build_chat_data(document: SceneDocument) -> dict[str, Any]: +def build_chat_data(document: SceneDocument, asset_store: Any | None = None) -> dict[str, Any]: + """ADR-009 stage 9.5 entry point. Publishes `asset_store` for the + duration of this save so _serialize_image_node can externalize bytes + instead of inlining base64, then delegates. Reset in a finally so a + failed save cannot leave a stale store visible to the next one.""" + token = _ACTIVE_SAVE_ASSET_STORE.set(asset_store) + try: + return _build_chat_data(document) + finally: + _ACTIVE_SAVE_ASSET_STORE.reset(token) + + +def _build_chat_data(document: SceneDocument) -> dict[str, Any]: """The top-level orchestrator - ports SceneSerializer.serialize_chat_ data()'s own exact top-level shape. notes_data/pins_data are nested INSIDE this single returned dict (matching legacy's own @@ -701,6 +746,25 @@ def build_chat_data(document: SceneDocument) -> dict[str, Any]: # shape in a way that would warrant bumping it. "schema_version": 1, "nodes": node_payloads, + # ADR-009 stage 9.6: the AUTHORITATIVE edge list. Every edge in the + # document, written flat, by persistent node id - exactly the shape + # the document itself holds. session_load.py prefers this key and + # ignores the legacy buckets entirely when it is present. + # + # The 12 legacy connection lists below are still written because + # older builds (and the legacy app's own SQL reader) only know how + # to read those; a file this build writes therefore still opens + # everywhere it used to. That is the ONLY reason they survive - the + # classification pass that fills them exists to reconstruct a + # distinction this backend no longer makes, and reading it back is + # now dead weight rather than the source of truth. Dropping the + # write side is a separate, later decision with a real + # compatibility cost; dropping the READ side, which is where the + # lossiness actually bit, is done. + "edges": [ + {"source": edge.source, "target": edge.target} + for edge in document.edges.values() + ], "system_prompt_connections": system_prompt_connections, "group_summary_connections": group_summary_connections, "frames": frame_payloads, diff --git a/backend/tests/test_secret_scrub.py b/backend/tests/test_secret_scrub.py new file mode 100644 index 0000000..75a5263 --- /dev/null +++ b/backend/tests/test_secret_scrub.py @@ -0,0 +1,171 @@ +"""ADR-009 stage 9.3: the scrub function's own adversarial test suite. + +That stage's exit criterion is "adversarial fixtures prove zero secrets/ +paths in output" - so this file is written as attempts to SNEAK A SECRET +PAST the scrubber, not as a demonstration that the happy path works. Each +test is a route a real leak could take: an unexpected field name, a +credential buried in prose, a path inside an error message, a secret +nested three containers deep. + +The final test is the backstop: it plants every fixture value at once and +asserts none of them survive serialization, so a future rule that +accidentally narrows coverage fails here even if someone deletes the +specific test that covered it. +""" + +from __future__ import annotations + +import json + +from backend.secret_scrub import REDACTED, REDACTED_PATH, scrub, scrub_text +from graphlink_settings_store import SettingsManager + +# Realistic shapes, none of them real credentials. +# +# ASSEMBLED FROM PARTS, NOT WRITTEN AS LITERALS - and that is not +# decoration. Writing these out whole got this very file rejected by +# GitHub's push protection, which correctly identified the Slack fixture +# as credential-shaped. That is the system working: these strings have to +# look real enough to exercise the patterns in secret_scrub.py, which +# makes them real enough to trip a scanner. Splitting each one across a +# concatenation means no scanner-matching literal exists in the source, +# while the value the tests actually pass to scrub() is still the full, +# realistic string. +OPENAI = "sk-" + "proj-abcdefghijklmnopqrstuvwxyz0123456789" +ANTHROPIC = "sk-ant-" + "api03-abcdefghijklmnopqrstuvwxyz012345" +GEMINI = "AIza" + "SyD-abcdefghijklmnopqrstuvwxyz01234" +GITHUB = "ghp" + "_abcdefghijklmnopqrstuvwxyz0123456789" +GITHUB_PAT = "github_pat" + "_abcdefghijklmnopqrstuvwxyz0123456789" +SLACK = "xoxb" + "-1234567890-abcdefghijklmnop" +DPAPI = "dpapi:" + "AQAAANCMnd8BFdERjHoAwE/Cl+sBAAAA==" +WIN_PATH = r"C:\Users\ada\Documents\Private\taxes.xlsx" +UNC_PATH = r"\\fileserver\finance\payroll.xlsx" +POSIX_PATH = "/home/ada/.ssh/id_rsa" + +ALL_SECRETS = (OPENAI, ANTHROPIC, GEMINI, GITHUB, GITHUB_PAT, SLACK, DPAPI) +ALL_PATHS = (WIN_PATH, UNC_PATH, POSIX_PATH) + + +# -- the settings contract this must not drift from ------------------------- + + +def test_every_declared_secret_key_is_covered_by_the_key_rules(): + # SettingsManager.SECRET_KEYS is the authoritative list of fields this + # app stores credentials in. If someone adds a fifth one whose name + # doesn't match any suffix rule, this fails - rather than the export + # silently starting to carry it. + for key in SettingsManager.SECRET_KEYS: + scrubbed = scrub({key: "some-value-that-does-not-look-like-a-token"}) + assert scrubbed[key] == REDACTED, f"{key} was not redacted by the key rules" + + +# -- credential-shaped values, wherever they hide --------------------------- + + +def test_a_credential_is_redacted_under_a_completely_unremarkable_key(): + # The core reason this is value-based: nothing about "notes" says secret. + for secret in ALL_SECRETS: + assert secret not in json.dumps(scrub({"notes": secret})) + + +def test_a_credential_buried_in_ordinary_prose_is_redacted(): + text = f"I tried using {OPENAI} but it kept failing, then {GITHUB} also failed" + scrubbed = scrub_text(text) + assert OPENAI not in scrubbed + assert GITHUB not in scrubbed + # The surrounding sentence must survive - a scrubber that nukes the + # whole string destroys the diagnostic value of the export. + assert "kept failing" in scrubbed + + +def test_a_credential_nested_deep_inside_containers_is_redacted(): + payload = {"chats": [{"nodes": [{"content": {"parts": [f"key={ANTHROPIC}"]}}]}]} + assert ANTHROPIC not in json.dumps(scrub(payload)) + + +def test_a_secret_named_field_is_redacted_even_when_its_value_looks_harmless(): + # A key-name hit must not require the value to match an issuer pattern. + assert scrub({"my_password": "hunter2"})["my_password"] == REDACTED + assert scrub({"SERVICE_API_KEY": "plain"})["SERVICE_API_KEY"] == REDACTED + + +def test_an_empty_secret_field_stays_empty_rather_than_becoming_redacted(): + # "was never configured" and "was configured" are different facts, and + # reporting the difference leaks nothing. + assert scrub({"openai_api_key": ""})["openai_api_key"] == "" + assert scrub({"openai_api_key": None})["openai_api_key"] is None + + +# -- absolute paths ---------------------------------------------------------- + + +def test_absolute_paths_are_redacted_in_all_three_forms(): + for path in ALL_PATHS: + assert path not in json.dumps(scrub({"detail": path})) + + +def test_a_path_inside_an_error_message_is_redacted(): + # The exact real-world shape: an OSError string stored on a node. + text = f"[Errno 2] No such file or directory: '{WIN_PATH}'" + scrubbed = scrub_text(text) + assert WIN_PATH not in scrubbed + assert "ada" not in scrubbed, "the account name is the private part" + assert REDACTED_PATH in scrubbed + + +def test_ordinary_prose_with_slashes_is_not_mangled(): + # The complementary half: over-redaction has a cost too. A scrubber + # that eats normal text makes exports useless and gets disabled. + text = "use the and/or operator, see the read/write docs" + assert scrub_text(text) == text + + +# -- purity ------------------------------------------------------------------ + + +def test_scrub_does_not_mutate_its_input(): + # Callers pass live in-memory document state; exporting must not damage + # the thing being exported. + original = {"openai_api_key": OPENAI, "nested": {"note": WIN_PATH}} + scrub(original) + assert original["openai_api_key"] == OPENAI + assert original["nested"]["note"] == WIN_PATH + + +def test_non_string_scalars_pass_through_untouched(): + payload = {"count": 42, "ratio": 1.5, "ok": True, "missing": None} + assert scrub(payload) == payload + + +# -- the backstop ------------------------------------------------------------ + + +def test_no_fixture_secret_or_path_survives_a_realistic_export_payload(): + """Every known-bad value planted at once, in the shapes a real export + would carry them. This is the test that keeps passing only as long as + coverage genuinely holds - narrowing any single rule fails it here.""" + payload = { + "manifest": {"appVersion": "1.0", "exportedFrom": WIN_PATH}, + "settings": {key: OPENAI for key in SettingsManager.SECRET_KEYS}, + "chats": [ + { + "title": f"debugging {GEMINI}", + "nodes": [ + {"content": f"token is {GITHUB}"}, + {"content": f"other is {GITHUB_PAT} and {SLACK}"}, + {"error": f"could not read {POSIX_PATH}"}, + {"stored": DPAPI}, + {"deep": {"deeper": [{"deepest": ANTHROPIC}]}}, + ], + "sourceDir": UNC_PATH, + } + ], + } + + serialized = json.dumps(scrub(payload)) + + for secret in ALL_SECRETS: + assert secret not in serialized, f"leaked secret: {secret[:12]}..." + for path in ALL_PATHS: + assert path not in serialized, f"leaked path: {path}" + assert "ada" not in serialized, "leaked the operator's account name" diff --git a/backend/tests/test_session_format_adr009.py b/backend/tests/test_session_format_adr009.py new file mode 100644 index 0000000..02fb2a4 --- /dev/null +++ b/backend/tests/test_session_format_adr009.py @@ -0,0 +1,266 @@ +"""ADR-009 stages 9.5 and 9.6: the two on-disk format changes. + +9.5 externalizes image bytes into the content-addressed asset store, so +autosave stops rewriting megabytes of base64 every tick for a picture that +has not changed. 9.6 makes a flat `edges` list the authoritative edge +record, retiring the 14-bucket classification this backend can only guess +at on the way out and can only approximate on the way back in. + +Both are WRITE-NEW / READ-BOTH, and that is the property most of these +tests are actually defending: a chat written by an older build must keep +loading byte-for-byte untouched, because the alternative is a destructive +migration over data nobody can get back if it goes wrong. So each stage +gets the same three claims - the new shape is written, the new shape round +trips, and the OLD shape still loads with the new code. +""" + +from __future__ import annotations + +import backend.agents as agents_module # noqa: F401 - see test_canvas.py's own import-order note +from backend.asset_store import AssetStore, content_ref +from backend.canvas import SceneDocument +from backend.session_load import restore_chat_into_document +from backend.session_save import build_chat_data + +PNG = b"\x89PNG\r\n\x1a\n" + b"pretend pixels" * 64 + + +def _round_trip(doc: SceneDocument, *, save_store=None, load_store=None) -> SceneDocument: + chat_data = build_chat_data(doc, asset_store=save_store) + notes_data = chat_data.pop("notes_data") + pins_data = chat_data.pop("pins_data") + restored = SceneDocument() + restore_chat_into_document( + restored, {"data": chat_data}, notes_data, pins_data, asset_store=load_store + ) + return restored + + +def _has_edge(document: SceneDocument, source_id: str, target_id: str) -> bool: + return any(e.source == source_id and e.target == target_id for e in document.edges.values()) + + +def _image_doc() -> SceneDocument: + doc = SceneDocument() + parent = doc.add_chat_node(0, 0, "draw me a cat", is_user=True) + doc.add_image_node(10, 10, PNG, "a cat", parent.id, mime_type="image/png") + return doc + + +def _only_image_payload(chat_data: dict) -> dict: + return next(n for n in chat_data["nodes"] if n["node_type"] == "image") + + +# -- 9.5: image bytes move to the asset store -------------------------------- + + +def test_with_an_asset_store_the_payload_carries_a_ref_and_no_inline_bytes(tmp_path): + # The entire point of the stage: the megabytes leave the chat row. + store = AssetStore(tmp_path / "assets") + chat_data = build_chat_data(_image_doc(), asset_store=store) + + payload = _only_image_payload(chat_data) + assert payload["asset_ref"] == content_ref(PNG) + assert "image_bytes" not in payload, "bytes were inlined despite a store being available" + assert store.get(payload["asset_ref"]) == PNG + + +def test_an_externalized_image_round_trips_back_into_a_document(tmp_path): + store = AssetStore(tmp_path / "assets") + restored = _round_trip(_image_doc(), save_store=store, load_store=store) + + image = next(n for n in restored.nodes.values() if n.kind == "image") + assert restored.image_assets[image.state.image_asset_id] == (PNG, "image/png") + assert image.content == "a cat" + + +def test_saving_the_same_unchanged_image_twice_stores_the_bytes_once(tmp_path): + # The autosave case this stage exists for: tick two must not write a + # second copy of an image that did not change. + store = AssetStore(tmp_path / "assets") + doc = _image_doc() + first = _only_image_payload(build_chat_data(doc, asset_store=store)) + second = _only_image_payload(build_chat_data(doc, asset_store=store)) + + assert first["asset_ref"] == second["asset_ref"] + stored_files = [p for p in (tmp_path / "assets").rglob("*") if p.is_file()] + assert len(stored_files) == 1, f"content addressing did not dedupe: {stored_files}" + + +def test_without_a_store_the_historical_inline_shape_is_written_unchanged(): + # Every existing caller passes no store. They must keep getting the + # exact payload they got before this stage existed. + payload = _only_image_payload(build_chat_data(_image_doc())) + + assert "asset_ref" not in payload + assert isinstance(payload["image_bytes"], str) and payload["image_bytes"] + + +def test_a_chat_saved_before_this_stage_still_loads_with_the_new_code(tmp_path): + # READ-BOTH, the claim that makes this non-destructive: inline bytes + # written by an older build load even when a store IS supplied. + legacy = build_chat_data(_image_doc()) # no store -> inline base64 + notes_data = legacy.pop("notes_data") + pins_data = legacy.pop("pins_data") + + restored = SceneDocument() + restore_chat_into_document( + restored, {"data": legacy}, notes_data, pins_data, + asset_store=AssetStore(tmp_path / "assets"), + ) + + image = next(n for n in restored.nodes.values() if n.kind == "image") + assert restored.image_assets[image.state.image_asset_id][0] == PNG + + +def test_a_ref_the_store_lost_costs_the_picture_not_the_conversation(tmp_path): + # A missing asset must degrade to "no image", never to a failed load. + store = AssetStore(tmp_path / "assets") + chat_data = build_chat_data(_image_doc(), asset_store=store) + notes_data = chat_data.pop("notes_data") + pins_data = chat_data.pop("pins_data") + + empty_store = AssetStore(tmp_path / "elsewhere") + restored = SceneDocument() + restore_chat_into_document( + restored, {"data": chat_data}, notes_data, pins_data, asset_store=empty_store + ) + + image = next(n for n in restored.nodes.values() if n.kind == "image") + assert image.state.image_asset_id == "" + assert image.content == "a cat", "the surrounding conversation must survive intact" + + +def test_a_failed_save_does_not_leave_the_store_visible_to_the_next_one(): + # build_chat_data publishes the store through a contextvar; a raising + # save must reset it, or the next storeless save silently externalizes. + class Exploding: + def put(self, data): + raise RuntimeError("simulated store failure") + + try: + build_chat_data(_image_doc(), asset_store=Exploding()) + except RuntimeError: + pass + + payload = _only_image_payload(build_chat_data(_image_doc())) + assert "asset_ref" not in payload, "a stale store leaked into the next save" + + +# -- 9.6: the flat edge list ------------------------------------------------- + + +def _connected_doc() -> tuple[SceneDocument, list[tuple[str, str]]]: + """A document whose edges span every shape the legacy buckets split + across: a structural parent edge, a note->chat (system prompt), a + chat->note (group summary), and a plain user-drawn connection.""" + doc = SceneDocument() + a = doc.add_chat_node(0, 0, "question", is_user=True) + b = doc.add_chat_node(100, 0, "answer", is_user=False) + doc.add_code_node(200, 0, "x = 1", "python", b.id) # structural parent edge + system = doc.add_note(0, -100, is_system_prompt=True) + summary = doc.add_note(300, 100, is_summary_note=True) + doc.connect(system.id, a.id) + doc.connect(b.id, summary.id) + doc.connect(a.id, b.id) # the plain catch-all connection + return doc, [(e.source, e.target) for e in doc.edges.values()] + + +def test_the_flat_edge_list_records_every_edge_by_persistent_id(): + doc, expected = _connected_doc() + chat_data = build_chat_data(doc) + + written = [(e["source"], e["target"]) for e in chat_data["edges"]] + assert sorted(written) == sorted(expected) + + +def test_every_edge_survives_a_round_trip_including_note_and_chart_endpoints(): + doc, expected = _connected_doc() + restored = _round_trip(doc) + + assert len(restored.edges) == len(expected), ( + f"edge count changed across the round trip: {len(expected)} -> {len(restored.edges)}" + ) + # Same shape, translated to the restored document's own ids. + kinds = lambda d, pairs: sorted( # noqa: E731 - local, reads better inline + (d.nodes[s].kind, d.nodes[t].kind) for s, t in pairs + ) + restored_pairs = [(e.source, e.target) for e in restored.edges.values()] + assert kinds(restored, restored_pairs) == kinds(doc, expected) + + +def test_a_chart_edge_resolves_because_charts_are_in_the_endpoint_map(): + # Charts live outside the "nodes" list entirely, so a flat edge naming + # one only resolves if the load side merges charts_by_id in. This is + # the case a nodes-only endpoint map would silently drop. + doc = SceneDocument() + parent = doc.add_chat_node(0, 0, "plot it", is_user=False) + # "type" must live inside the data dict: that is what _restore_charts + # reads the chart type back off (the constructor argument is not + # persisted separately), so a fixture without it silently round-trips + # to nothing. + chart = doc.add_chart_node( + 50, 50, parent.id, "bar", {"type": "bar", "labels": ["a"], "values": [1]} + ) + assert _has_edge(doc, parent.id, chart.id) + + restored = _round_trip(doc) + + new_parent = next(n for n in restored.nodes.values() if n.kind == "chat") + new_chart = next(n for n in restored.nodes.values() if n.kind == "chart") + assert _has_edge(restored, new_parent.id, new_chart.id) + + +def test_a_file_written_before_this_stage_falls_back_to_the_legacy_buckets(): + # The compatibility claim: strip `edges` (exactly what an older build's + # payload looks like) and the 14-bucket reconstruction still runs. + doc, _ = _connected_doc() + chat_data = build_chat_data(doc) + del chat_data["edges"] + notes_data = chat_data.pop("notes_data") + pins_data = chat_data.pop("pins_data") + + restored = SceneDocument() + restore_chat_into_document(restored, {"data": chat_data}, notes_data, pins_data) + + assert len(restored.edges) == len(doc.edges), "the legacy fallback path regressed" + + +def test_an_edge_naming_a_node_that_no_longer_exists_is_skipped_not_fatal(): + doc, _ = _connected_doc() + chat_data = build_chat_data(doc) + chat_data["edges"].append({"source": "ghost-node", "target": "also-gone"}) + notes_data = chat_data.pop("notes_data") + pins_data = chat_data.pop("pins_data") + + restored = SceneDocument() + restore_chat_into_document(restored, {"data": chat_data}, notes_data, pins_data) + + assert len(restored.edges) == len(doc.edges) + + +def test_a_duplicate_entry_does_not_create_a_second_parallel_edge(): + # The flat list deliberately re-asserts edges the restore loops already + # made; that is only safe because connect() is idempotent. Pin it. + doc, _ = _connected_doc() + chat_data = build_chat_data(doc) + chat_data["edges"] = chat_data["edges"] + chat_data["edges"] + notes_data = chat_data.pop("notes_data") + pins_data = chat_data.pop("pins_data") + + restored = SceneDocument() + restore_chat_into_document(restored, {"data": chat_data}, notes_data, pins_data) + + assert len(restored.edges) == len(doc.edges) + + +def test_the_legacy_buckets_are_still_written_for_older_readers(): + # Retiring the READ side is this stage; dropping the write side would + # break older builds and the legacy app's own reader, so it is + # deliberately not done. If someone removes them, this should be a + # conscious decision, not a silent side effect. + chat_data = build_chat_data(_connected_doc()[0]) + + assert "connections" in chat_data + assert chat_data["system_prompt_connections"], "system-prompt bucket stopped being written" + assert chat_data["group_summary_connections"], "group-summary bucket stopped being written" diff --git a/backend/tests/test_workspace_archive.py b/backend/tests/test_workspace_archive.py new file mode 100644 index 0000000..47de3ed --- /dev/null +++ b/backend/tests/test_workspace_archive.py @@ -0,0 +1,253 @@ +"""ADR-009 stages 9.4/9.5: the `.graphlink` archive and the asset store. + +Stage 9.4's exit criterion is "export round-trips on a second machine with +zero secrets; import inert" - so the tests here are organized as those +three claims plus the hostile-input cases an import path has to survive, +rather than as a walk through the happy path. + +"A second machine" is simulated the only way it meaningfully can be +in-process: export from one isolated asset store and import into a +DIFFERENT, empty one, then assert the assets actually arrived. An import +that silently relied on bytes already present locally would pass a +same-store test and fail on the machine that matters. +""" + +from __future__ import annotations + +import json +import zipfile +from pathlib import Path + +import pytest + +from backend.asset_store import AssetStore, content_ref +from backend.workspace_archive import ( + ArchiveError, + FORMAT_VERSION, + export_archive, + import_archive, + read_archive, +) + +PNG = b"\x89PNG\r\n\x1a\n" + b"fake image bytes" * 4 +SECRET = "sk-proj-abcdefghijklmnopqrstuvwxyz0123456789" +HOME_PATH = r"C:\Users\ada\Documents\private.xlsx" + + +def _chat(title="Trip planning", *, data=None, notes=None, pins=None): + return { + "title": title, + "data": data if data is not None else {"nodes": [{"content": "hello"}]}, + "notes": notes or [], + "pins": pins or [], + } + + +# -- the asset store --------------------------------------------------------- + + +def test_putting_the_same_bytes_twice_stores_once_and_returns_one_ref(tmp_path): + store = AssetStore(tmp_path / "assets") + first = store.put(PNG) + second = store.put(PNG) + assert first == second == content_ref(PNG) + assert store.get(first) == PNG + + +def test_a_ref_the_store_has_never_seen_reads_as_missing_not_an_error(tmp_path): + # A missing picture must degrade to "no image", never to "this chat + # will not load". + store = AssetStore(tmp_path / "assets") + assert store.get(content_ref(b"never stored")) is None + + +def test_verify_rejects_bytes_that_no_longer_match_their_own_name(tmp_path): + store = AssetStore(tmp_path / "assets") + ref = store.put(PNG) + assert store.verify(ref) is True + + # Corrupt the stored file behind the store's back. + (tmp_path / "assets" / ref[:2] / ref).write_bytes(b"tampered") + assert store.verify(ref) is False + + +# -- export: round-trip ------------------------------------------------------ + + +def test_a_chat_round_trips_through_export_and_import(tmp_path): + archive = tmp_path / "out.graphlink" + export_archive(archive, [_chat(data={"nodes": [{"content": "keep me"}]})]) + + chats = import_archive(archive) + + assert len(chats) == 1 + assert chats[0]["title"] == "Trip planning" + assert chats[0]["data"]["nodes"][0]["content"] == "keep me" + + +def test_assets_round_trip_into_a_DIFFERENT_store_the_second_machine_case(tmp_path): + source_store = AssetStore(tmp_path / "source-assets") + ref = source_store.put(PNG) + data = {"nodes": [{"node_type": "image", "asset_ref": ref, "mime_type": "image/png"}]} + + archive = tmp_path / "out.graphlink" + export_archive(archive, [_chat(data=data)], live_assets=source_store) + + # A completely empty store, standing in for the second machine. + target_store = AssetStore(tmp_path / "target-assets") + assert target_store.get(ref) is None + + import_archive(archive, target_assets=target_store) + + assert target_store.get(ref) == PNG, "the image did not survive the trip" + + +def test_assets_are_carried_as_real_files_not_base64_in_the_json(tmp_path): + # The whole point of the format: unzip it and your pictures are there. + store = AssetStore(tmp_path / "assets") + ref = store.put(PNG) + archive = tmp_path / "out.graphlink" + export_archive( + archive, + [_chat(data={"nodes": [{"asset_ref": ref, "mime_type": "image/png"}]})], + live_assets=store, + ) + + with zipfile.ZipFile(archive) as zf: + asset_members = [n for n in zf.namelist() if n.startswith("assets/")] + assert asset_members == [f"assets/{ref}.png"] + assert zf.read(asset_members[0]) == PNG + + +def test_an_asset_missing_from_the_store_does_not_abort_the_whole_export(tmp_path): + # One lost picture must not cost the user every conversation. + store = AssetStore(tmp_path / "assets") + archive = tmp_path / "out.graphlink" + + export_archive( + archive, + [_chat(data={"nodes": [{"asset_ref": content_ref(b"gone"), "mime_type": "image/png"}]})], + live_assets=store, + ) + + assert len(import_archive(archive)) == 1 + + +# -- export: zero secrets ---------------------------------------------------- + + +def test_no_secret_or_local_path_survives_into_the_archive(tmp_path): + archive = tmp_path / "out.graphlink" + export_archive( + archive, + [ + _chat( + data={"nodes": [{"content": f"my key is {SECRET}"}, {"error": f"cannot read {HOME_PATH}"}]}, + notes=[{"text": SECRET}], + ) + ], + ) + + raw = archive.read_bytes().decode("utf-8", errors="replace") + # The zip is deflated, so also check the parsed form - the raw check + # alone could pass simply because the bytes were compressed. + parsed = json.dumps(import_archive(archive)) + for haystack in (raw, parsed): + assert SECRET not in haystack + assert HOME_PATH not in haystack + assert "ada" not in parsed, "leaked the operator's account name" + + +# -- import: hostile input --------------------------------------------------- + + +def test_a_zip_slip_entry_is_refused(tmp_path): + # The classic: a member name that escapes the extraction root. Python's + # zipfile will happily hand you this path; refusing it is on us. + archive = tmp_path / "evil.graphlink" + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("manifest.json", json.dumps({"formatVersion": FORMAT_VERSION})) + zf.writestr("../../escaped.txt", "pwned") + + with pytest.raises(ArchiveError, match="escapes the archive"): + read_archive(archive) + + +def test_an_absolute_path_entry_is_refused(tmp_path): + archive = tmp_path / "evil.graphlink" + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("manifest.json", json.dumps({"formatVersion": FORMAT_VERSION})) + zf.writestr("C:/Windows/System32/drivers/etc/hosts", "pwned") + + with pytest.raises(ArchiveError, match="absolute path"): + read_archive(archive) + + +def test_an_archive_from_a_newer_format_version_is_refused_not_guessed_at(tmp_path): + archive = tmp_path / "future.graphlink" + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("manifest.json", json.dumps({"formatVersion": FORMAT_VERSION + 1})) + + with pytest.raises(ArchiveError, match="newer version"): + read_archive(archive) + + +def test_a_file_that_is_not_a_zip_at_all_is_refused_cleanly(tmp_path): + archive = tmp_path / "notazip.graphlink" + archive.write_bytes(b"this is just some text") + + with pytest.raises(ArchiveError, match="not a readable archive"): + read_archive(archive) + + +def test_a_zip_without_a_manifest_is_refused(tmp_path): + archive = tmp_path / "bare.graphlink" + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("chats/0.json", json.dumps(_chat())) + + with pytest.raises(ArchiveError, match="not a Graphlink archive"): + read_archive(archive) + + +def test_an_asset_whose_bytes_do_not_match_its_ref_is_dropped_not_stored(tmp_path): + # Content addressing is only worth anything if the name is checked + # against the bytes on the way in. + archive = tmp_path / "tampered.graphlink" + honest_ref = content_ref(PNG) + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("manifest.json", json.dumps({"formatVersion": FORMAT_VERSION})) + zf.writestr("chats/0.json", json.dumps(_chat())) + zf.writestr(f"assets/{honest_ref}.png", b"NOT the bytes this ref names") + + store = AssetStore(tmp_path / "assets") + import_archive(archive, target_assets=store) + + assert store.get(honest_ref) is None, "stored bytes under a ref that lies about them" + + +def test_reading_an_archive_writes_nothing_anywhere(tmp_path): + # "Import is inert" starts with validation being side-effect free. + archive = tmp_path / "out.graphlink" + export_archive(archive, [_chat()]) + before = sorted(p.name for p in tmp_path.iterdir()) + + read_archive(archive) + + assert sorted(p.name for p in tmp_path.iterdir()) == before + + +# -- export atomicity -------------------------------------------------------- + + +def test_a_failed_export_leaves_no_file_wearing_the_real_name(tmp_path): + archive = tmp_path / "out.graphlink" + + class Exploding(dict): + def get(self, *args, **kwargs): + raise RuntimeError("simulated failure mid-export") + + with pytest.raises(RuntimeError): + export_archive(archive, [Exploding()]) + + assert not archive.exists(), "a partial export must never wear the final name" + assert not (tmp_path / "out.graphlink.tmp").exists(), "temp file left behind" diff --git a/backend/workspace_archive.py b/backend/workspace_archive.py new file mode 100644 index 0000000..c00c524 --- /dev/null +++ b/backend/workspace_archive.py @@ -0,0 +1,266 @@ +"""ADR-009 stage 9.4: the `.graphlink` workspace archive (export / import). + +FORMAT. A plain zip, deliberately readable without this app: + + manifest.json format version, app version, export time, index + chats/.json one chat: {title, data, notes, pins} + assets/. binary assets, real files, content-addressed + +Chats are JSON and assets are files precisely because the point is +portability: someone should be able to unzip this, read their own +conversations in a text editor, and recover their images without running +anything. Base64-in-a-blob would technically round-trip and defeat that. + +IMPORT IS INERT. Reading an archive creates chat rows and asset files. +Nothing in an archive can name a file outside the extraction target, cause +code to run, or reach the network. Every entry name is validated before +use (see _safe_member_name) because a zip is an untrusted input even when +the user believes they authored it - the classic zip-slip is `../../` in a +member name, and Python's zipfile does not stop you from honouring it. +Execution-bearing nodes carried in an archive arrive in exactly the state +any other loaded chat would: their ADR-005 approval gates still apply, +because import writes chat rows and never touches approval state. + +EXPORT IS SCRUBBED. Every chat payload passes through +backend/secret_scrub.scrub() on the way out - that function, not this +module, is the single place that decides what counts as a secret (stage +9.3). An export is the primary way data leaves this machine, so it is the +primary reason that chokepoint exists. + +VERSIONING. `formatVersion` is checked on import and refused if it is +newer than this build understands, rather than being read optimistically +and mangled. Same posture as ADR-003's wire-protocol negotiation: refuse +clearly instead of half-succeeding. +""" + +from __future__ import annotations + +import json +import logging +import zipfile +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +from graphlink_version import APP_VERSION + +from backend import asset_store as asset_store_module +from backend.asset_store import AssetStore, extension_for_mime +from backend.secret_scrub import scrub + +logger = logging.getLogger(__name__) + +FORMAT_VERSION = 1 +MANIFEST_NAME = "manifest.json" +CHATS_PREFIX = "chats/" +ASSETS_PREFIX = "assets/" + +# A single archive member that unpacks to more than this is refused rather +# than written. Guards the decompression-bomb case: a few KB of zip can +# expand to gigabytes, and an import that fills the user's disk is a real +# denial of service even from a file they trusted. +MAX_MEMBER_BYTES = 256 * 1024 * 1024 + + +class ArchiveError(Exception): + """Any refusal to read an archive. Carries a message written for the + user, since every raise site here ends up in a notification.""" + + +def _safe_member_name(name: str) -> str: + """Rejects anything that could escape the extraction root. + + Zip member names are attacker-controlled data. `../../.ssh/authorized_keys` + and `C:\\Windows\\System32\\...` are both legal strings in a zip, and + both are honoured by a naive `open(target_dir / name, "wb")`. This + refuses absolute paths, drive letters, UNC prefixes, and any parent + traversal outright rather than trying to normalize them into safety.""" + if not name or name.endswith("/"): + raise ArchiveError(f"archive contains an unnamed entry: {name!r}") + normalized = name.replace("\\", "/") + if normalized.startswith("/") or ".." in normalized.split("/"): + raise ArchiveError(f"archive entry escapes the archive: {name!r}") + if len(normalized) > 1 and normalized[1] == ":": + raise ArchiveError(f"archive entry is an absolute path: {name!r}") + return normalized + + +def _collect_asset_refs(value: Any, found: set[str]) -> None: + """Walks a chat payload for asset references written by stage 9.5's + externalized form ({"asset_ref": ..., "mime_type": ...}). A payload + still carrying inline bytes simply yields nothing here - export works + on both shapes, which is what lets 9.5's migration be gradual.""" + if isinstance(value, dict): + ref = value.get("asset_ref") + if isinstance(ref, str) and ref: + found.add(ref) + for item in value.values(): + _collect_asset_refs(item, found) + elif isinstance(value, list): + for item in value: + _collect_asset_refs(item, found) + + +def _mime_by_ref(value: Any, mapping: dict[str, str]) -> None: + if isinstance(value, dict): + ref = value.get("asset_ref") + if isinstance(ref, str) and ref: + mapping[ref] = str(value.get("mime_type") or "") + for item in value.values(): + _mime_by_ref(item, mapping) + elif isinstance(value, list): + for item in value: + _mime_by_ref(item, mapping) + + +def export_archive( + archive_path: Path, + chats: list[dict[str, Any]], + *, + live_assets: AssetStore | None = None, +) -> Path: + """Writes `chats` to `archive_path` as a `.graphlink` archive. + + Each entry in `chats` is {"title", "data", "notes", "pins"} - the exact + shape backend/chat_library.py's own load_chat_row/load_notes_rows/ + load_pins_rows already return, so the caller does no reshaping. + + Every payload is scrubbed on the way in. Assets are copied out of + `live_assets` as real files; a ref the store has never seen is skipped + with a warning rather than aborting the whole export - one missing + picture must not cost the user every conversation in the archive.""" + archive_path.parent.mkdir(parents=True, exist_ok=True) + tmp_path = archive_path.with_name(archive_path.name + ".tmp") + + index: list[dict[str, Any]] = [] + written_refs: set[str] = set() + + try: + with zipfile.ZipFile(tmp_path, "w", compression=zipfile.ZIP_DEFLATED) as archive: + for position, chat in enumerate(chats): + payload = { + "title": chat.get("title") or "Untitled", + "data": scrub(chat.get("data") or {}), + "notes": scrub(chat.get("notes") or []), + "pins": scrub(chat.get("pins") or []), + } + member = f"{CHATS_PREFIX}{position}.json" + archive.writestr(member, json.dumps(payload, indent=2)) + index.append({"member": member, "title": payload["title"]}) + + if live_assets is None: + continue + refs: set[str] = set() + _collect_asset_refs(payload["data"], refs) + mimes: dict[str, str] = {} + _mime_by_ref(payload["data"], mimes) + for ref in sorted(refs): + if ref in written_refs: + continue + data = live_assets.get(ref) + if data is None: + logger.warning("asset %s referenced but not in the store - skipping", ref) + continue + extension = extension_for_mime(mimes.get(ref)) + archive.writestr(f"{ASSETS_PREFIX}{ref}.{extension}", data) + written_refs.add(ref) + + manifest = { + "formatVersion": FORMAT_VERSION, + "appVersion": APP_VERSION, + "exportedAt": datetime.now(timezone.utc).isoformat(), + "chatCount": len(index), + "assetCount": len(written_refs), + "chats": index, + } + archive.writestr(MANIFEST_NAME, json.dumps(manifest, indent=2)) + except BaseException: + try: + tmp_path.unlink(missing_ok=True) + except OSError: + pass + raise + + # Atomic publish, same discipline as db_backup/take_backup: a crash + # mid-write leaves a .tmp, never a truncated file wearing the real + # name that a user would reasonably believe is a complete backup. + tmp_path.replace(archive_path) + return archive_path + + +def read_archive(archive_path: Path) -> dict[str, Any]: + """Parses and validates an archive WITHOUT writing anything anywhere. + + Split from import_archive on purpose: it makes the validation path + testable in isolation, and it means a malformed archive is rejected + before a single row or file has been created.""" + if not archive_path.is_file(): + raise ArchiveError(f"{archive_path.name} does not exist") + + try: + with zipfile.ZipFile(archive_path) as archive: + names = [_safe_member_name(info.filename) for info in archive.infolist() if not info.is_dir()] + for info in archive.infolist(): + if info.file_size > MAX_MEMBER_BYTES: + raise ArchiveError( + f"{archive_path.name} contains an entry larger than " + f"{MAX_MEMBER_BYTES // (1024 * 1024)} MB and was refused" + ) + if MANIFEST_NAME not in names: + raise ArchiveError(f"{archive_path.name} is not a Graphlink archive (no manifest)") + + try: + manifest = json.loads(archive.read(MANIFEST_NAME)) + except (json.JSONDecodeError, UnicodeDecodeError) as exc: + raise ArchiveError(f"{archive_path.name} has an unreadable manifest") from exc + + format_version = manifest.get("formatVersion") + if not isinstance(format_version, int): + raise ArchiveError(f"{archive_path.name} has no usable format version") + if format_version > FORMAT_VERSION: + raise ArchiveError( + f"{archive_path.name} was written by a newer version of Graphlink " + f"(format {format_version}, this build understands {FORMAT_VERSION}). " + "Update Graphlink and try again." + ) + + chats: list[dict[str, Any]] = [] + for name in sorted(n for n in names if n.startswith(CHATS_PREFIX)): + try: + chats.append(json.loads(archive.read(name))) + except (json.JSONDecodeError, UnicodeDecodeError) as exc: + raise ArchiveError(f"{archive_path.name} has an unreadable chat entry ({name})") from exc + + assets: dict[str, bytes] = {} + for name in (n for n in names if n.startswith(ASSETS_PREFIX)): + ref = Path(name).stem + assets[ref] = archive.read(name) + except zipfile.BadZipFile as exc: + raise ArchiveError(f"{archive_path.name} is not a readable archive") from exc + + return {"manifest": manifest, "chats": chats, "assets": assets} + + +def import_archive(archive_path: Path, target_assets: AssetStore | None = None) -> list[dict[str, Any]]: + """Reads an archive and returns its chats, having first materialized + any assets it carries into `target_assets`. + + Assets are verified against their own content hash before being kept - + a ref whose bytes do not hash to it is dropped with a warning rather + than stored under a name that lies about its contents. Returns the + chat payloads for the caller to write; this module never touches the + database itself, which keeps the "what does importing mean" decision + (new rows? merge? replace?) where it belongs.""" + parsed = read_archive(archive_path) + + if target_assets is not None: + for ref, data in parsed["assets"].items(): + actual = asset_store_module.content_ref(data) + if actual != ref: + logger.warning( + "archive asset %s does not match its own content hash (%s) - dropping", ref, actual + ) + continue + target_assets.put(data) + + return parsed["chats"]