From 0185ce345bc2770913738aec4b0a9524818c0059 Mon Sep 17 00:00:00 2001 From: Coding-Dev-Tools Date: Thu, 23 Jul 2026 15:28:36 -0400 Subject: [PATCH 1/5] security: require zero CodeQL findings --- .github/workflows/codeql.yml | 5 + CHANGELOG.md | 4 + demo/record_screen_demo.mjs | 20 ++-- engraphis/backends/embedder_api.py | 21 ++-- engraphis/backends/embedder_deterministic.py | 1 + engraphis/backends/graph_extractor.py | 80 +++++++++++--- engraphis/core/engine.py | 18 +++- engraphis/engines/ingest.py | 65 ++++++++---- engraphis/routes/memory.py | 5 +- engraphis/routes/v2_api.py | 59 ++++++++--- engraphis/routes/vault.py | 50 ++++++--- engraphis/service.py | 51 ++++++--- scripts/check_codeql_sarif.py | 67 ++++++++++++ scripts/externalize_dashboard_assets.py | 104 ++++++++++++++++--- tests/test_codeql_sarif_gate.py | 72 +++++++++++++ tests/test_dashboard_v2.py | 25 +++++ tests/test_engine.py | 18 ++++ tests/test_externalize_dashboard_assets.py | 54 ++++++++++ tests/test_graph_extractor.py | 27 +++++ tests/test_ingest_entities.py | 13 +++ tests/test_memory_routes_fixes.py | 27 +++++ tests/test_provider_error_redaction.py | 24 +++++ tests/test_release_infrastructure.py | 11 ++ tests/test_sync_dashboard.py | 22 ++++ 24 files changed, 720 insertions(+), 123 deletions(-) create mode 100644 scripts/check_codeql_sarif.py create mode 100644 tests/test_codeql_sarif_gate.py create mode 100644 tests/test_externalize_dashboard_assets.py diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 76a1d0c..02d53ae 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -28,4 +28,9 @@ jobs: languages: ${{ matrix.language }} build-mode: none - name: Analyze + id: analyze uses: github/codeql-action/analyze@3b0bd1d116c0bde30213346b22d4f634d96a2fb0 # v3 + with: + output: codeql-results + - name: Require clean CodeQL results + run: python scripts/check_codeql_sarif.py "${{ steps.analyze.outputs.sarif-output }}" diff --git a/CHANGELOG.md b/CHANGELOG.md index 783c0a2..aa76db4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,10 @@ Public 1.0.1 client reliability release. entitlement-gated managed-compute availability while preserving snapshot redaction and limits. - Commercial metadata now describes Pro as one owner account across that owner's local installations, matching the hosted entitlement model; Team remains billed per named seat. +- API error responses and provider logs no longer expose arbitrary exception or configuration + text; local folder and repository reads resolve and re-check filesystem boundaries. +- Entity extraction and dashboard asset migration avoid adversarial regular-expression + backtracking, and CI now fails when CodeQL emits any SARIF finding. ## [1.0.0] - 2026-07-23 diff --git a/demo/record_screen_demo.mjs b/demo/record_screen_demo.mjs index 1410027..1fba2b5 100644 --- a/demo/record_screen_demo.mjs +++ b/demo/record_screen_demo.mjs @@ -1,7 +1,7 @@ import { spawn, spawnSync } from "node:child_process"; import { createReadStream, mkdirSync, rmSync, statSync } from "node:fs"; import { createServer } from "node:http"; -import { join, resolve, sep } from "node:path"; +import { join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { chromium } from "playwright"; @@ -13,6 +13,11 @@ const payload = join(generatedDir, "screen_demo_payload.json"); const webm = join(outputDir, "engraphis-memory-demo.webm"); const mp4 = join(outputDir, "engraphis-memory-demo.mp4"); const port = 8790; +const demoAssets = new Map([ + ["/", join(demoDir, "engraphis_screen_demo.html")], + ["/engraphis_screen_demo.html", join(demoDir, "engraphis_screen_demo.html")], + ["/generated/screen_demo_payload.json", payload], +]); mkdirSync(generatedDir, { recursive: true }); mkdirSync(outputDir, { recursive: true }); @@ -22,17 +27,16 @@ const prepared = spawnSync(process.env.PYTHON || "python", [ if (prepared.status !== 0) process.exit(prepared.status || 1); const server = createServer((request, response) => { - let relative; + let pathname; try { - relative = decodeURIComponent((request.url || "/").split("?", 1)[0]); + pathname = new URL(request.url || "/", "http://127.0.0.1").pathname; } catch { response.writeHead(400); response.end("Bad request"); return; } - const requested = relative === "/" ? "engraphis_screen_demo.html" : relative.slice(1); - const file = resolve(demoDir, requested); - if (file !== demoDir && !file.startsWith(demoDir + sep)) { - response.writeHead(403); response.end("Forbidden"); return; - } + // This recorder only needs these generated demo assets. Map request paths to fixed + // files instead of deriving a filesystem path from a URL. + const file = demoAssets.get(pathname); + if (!file) { response.writeHead(404); response.end("Not found"); return; } try { const stat = statSync(file); response.writeHead(200, { "Content-Length": stat.size, "Content-Type": file.endsWith(".json") ? "application/json" : "text/html" }); diff --git a/engraphis/backends/embedder_api.py b/engraphis/backends/embedder_api.py index d7c10f4..fd93dd3 100644 --- a/engraphis/backends/embedder_api.py +++ b/engraphis/backends/embedder_api.py @@ -56,10 +56,9 @@ def __init__( # A custom endpoint can contain embedded credentials or signed query # parameters, while provider-controlled model identifiers are also untrusted # log input. Do not copy either into logs. - logger.info( - "ApiEmbedder(custom_endpoint=%s, dim=%s)", - bool(base_url), self._dim or "auto", - ) + # Do not log endpoint, model, dimensions, or any authentication-related state: + # provider configuration can contain account identifiers or signed parameters. + logger.info("API embedder initialized") @property def dim(self) -> int: @@ -89,10 +88,7 @@ def embed( import httpx if not self._api_key: - logger.error( - "No API key set — set %s env var or pass api_key", - _DEFAULT_API_KEY_ENV, - ) + logger.error("No API key set for API embedder") raise RuntimeError( f"ApiEmbedder requires an API key via {_DEFAULT_API_KEY_ENV} " "env var or the api_key parameter" @@ -114,9 +110,8 @@ def embed( ) resp.raise_for_status() data = resp.json() - except Exception as exc: - logger.warning("Batch embedding failed (%s), falling back per-item", - type(exc).__name__) + except Exception: + logger.warning("Batch embedding request failed; falling back per-item") # Fallback: embed one at a time vecs = [self._embed_one(t) for t in texts] return np.asarray(vecs, dtype=np.float32) @@ -172,8 +167,8 @@ def _embed_one(self, text: str) -> list[float]: ) resp.raise_for_status() data = resp.json() - except Exception as exc: - logger.error("Single embedding request failed (%s)", type(exc).__name__) + except Exception: + logger.error("Single embedding request failed") return [0.0] * (self._dim or 384) items = data.get("data", []) diff --git a/engraphis/backends/embedder_deterministic.py b/engraphis/backends/embedder_deterministic.py index 42d2a99..ff3fa49 100644 --- a/engraphis/backends/embedder_deterministic.py +++ b/engraphis/backends/embedder_deterministic.py @@ -32,6 +32,7 @@ def embed(self, texts: list[str], *, kind: Literal["text", "code"] = "text") -> # SHA-1 is retained only to keep the established offline embedding # mapping stable across upgrades. This is feature hashing, never a # password, signature, integrity check, or other security primitive. + # codeql[py/weak-sensitive-data-hashing] h = hashlib.sha1( token.encode("utf-8"), usedforsecurity=False ).digest() diff --git a/engraphis/backends/graph_extractor.py b/engraphis/backends/graph_extractor.py index 5d528f3..a38fe42 100644 --- a/engraphis/backends/graph_extractor.py +++ b/engraphis/backends/graph_extractor.py @@ -21,6 +21,7 @@ """ from __future__ import annotations +import heapq import re from dataclasses import dataclass, field from typing import Any, Optional @@ -28,13 +29,13 @@ from engraphis.core.interfaces import Edge, Node # ── Regex NER (ported from engraphis/engines/ingest.py — the v1 heuristic path) ── -# Capitalized multi-word sequences, emails, URLs/hashtags, quoted names/mentions. -_ENTITY_RE = re.compile( - r"\b([A-Z][a-z]+(?:-[A-Za-z]+)*(?:\s+[A-Z][a-z]+(?:-[A-Za-z]+)*){0,3})\b" - r"|([a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,})" - r"|(#[a-zA-Z][a-zA-Z0-9_-]+)" - r"|(@[a-zA-Z][a-zA-Z0-9_-]+)" -) +# Capitalized multi-word sequences, emails, hashtags, and mentions. Keep the +# individual recognizers unambiguous: the old all-in-one expression could take +# polynomial time while backtracking over a long email-like local part. +_CAPITALIZED_WORD_RE = re.compile(r"\b[A-Z][a-z]+(?:-[A-Za-z]+)*\b") +_EMAIL_RE = re.compile(r"(? list[tuple[str, str]]: + text = text or "" + + def capitalized_candidates(): + # Assemble at most four whitespace-separated capitalized words, matching the + # former heuristic without placing a nested repetition over untrusted text. + words = iter(_CAPITALIZED_WORD_RE.finditer(text)) + current = next(words, None) + while current is not None: + first = last = current + current = None + for _ in range(3): + following = next(words, None) + if following is None: + break + if text[last.end():following.start()].isspace(): + last = following + continue + current = following + break + else: + current = next(words, None) + yield ( + first.start(), + 0, + last.end(), + text[first.start():last.end()], + "person_or_concept", + ) + + # Merge four already-position-ordered iterators. This keeps memory bounded even + # for very large untrusted input and lets the entity fanout cap stop collection. + candidates = heapq.merge( + capitalized_candidates(), + ( + (match.start(), 1, match.end(), match.group(0), "email") + for match in _EMAIL_RE.finditer(text) + ), + ( + (match.start(), 2, match.end(), match.group(0), "hashtag") + for match in _HASHTAG_RE.finditer(text) + ), + ( + (match.start(), 3, match.end(), match.group(0), "mention") + for match in _MENTION_RE.finditer(text) + ), + ) + seen: set[str] = set() out: list[tuple[str, str]] = [] - for m in _ENTITY_RE.finditer(text or ""): - raw = (m.group(0) or "").strip() + matched_through = 0 + for start, _priority, end, candidate, etype in candidates: + if start < matched_through: + continue + matched_through = end + raw = candidate.strip() if not raw or raw.casefold() in _STOPWORD_KEYS or raw.casefold() in ( "user", "the user" ): continue - if raw.startswith("#"): - ent, etype = raw, "hashtag" - elif raw.startswith("@"): - ent, etype = raw, "mention" - elif "@" in raw and "." in raw: - ent, etype = raw, "email" - else: + if etype == "person_or_concept": ent, etype = _canon_concept(raw), "person_or_concept" if len(ent) < 2 or ent.casefold() in _STOPWORD_KEYS: continue + else: + ent = raw key = ent.lower() if key not in seen: seen.add(key) diff --git a/engraphis/core/engine.py b/engraphis/core/engine.py index ae38e79..8ca802e 100644 --- a/engraphis/core/engine.py +++ b/engraphis/core/engine.py @@ -1228,6 +1228,10 @@ def index_repo(self, repo_id: str, root_path: str, *, languages: Optional[set] = ) indexer = get_code_indexer(prefer=prefer) + # index_repo is an explicit local-filesystem capability: callers select the + # repository root to index. Every walked child is resolved and re-contained + # below before it is read. + # codeql[py/path-injection] root = Path(root_path).expanduser().resolve() if not root.exists(): raise ValueError(f"repo root not found: {root_path}") @@ -1257,10 +1261,14 @@ def index_repo(self, repo_id: str, root_path: str, *, languages: Optional[set] = if files_scanned >= max_files: scan_complete = False break - p = Path(file_path) + candidate = Path(file_path) try: - rel = p.resolve().relative_to(root).as_posix() - except ValueError: + # Resolve once, verify containment, then use this checked path for + # every filesystem operation. The walker skips symlinks, and this + # closes the remaining defense-in-depth gap if it ever changes. + source_file = candidate.resolve(strict=True) + rel = source_file.relative_to(root).as_posix() + except (OSError, ValueError): files_failed += 1 continue files_scanned += 1 @@ -1271,11 +1279,11 @@ def index_repo(self, repo_id: str, root_path: str, *, languages: Optional[set] = # of an otherwise complete scan. present.add(rel) try: - stat = p.stat() + stat = source_file.stat() if stat.st_size > max_file_bytes: files_skipped += 1 continue - raw = p.read_bytes() + raw = source_file.read_bytes() except OSError: files_failed += 1 continue diff --git a/engraphis/engines/ingest.py b/engraphis/engines/ingest.py index 9ee2fd4..ca32dd9 100644 --- a/engraphis/engines/ingest.py +++ b/engraphis/engines/ingest.py @@ -18,13 +18,12 @@ from engraphis.stores import vectors as mem_store # ── Lightweight entity extraction ──────────────────────────────────────────── -# Capitalized multi-word sequences, emails, URLs, hashtags, quoted names. -_ENTITY_RE = re.compile( - r"\b([A-Z][a-z]+(?:-[A-Za-z]+)*(?:\s+[A-Z][a-z]+(?:-[A-Za-z]+)*){0,3})\b" - r"|([a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,})" - r"|(#[a-zA-Z][a-zA-Z0-9_-]+)" - r"|(@[a-zA-Z][a-zA-Z0-9_-]+)" -) +# Keep each recognizer unambiguous. The previous combined expression nested a +# repeated, unanchored branches and could take polynomial time on adversarial input. +_CAPITALIZED_WORD_RE = re.compile(r"\b[A-Z][a-z]+(?:-[A-Za-z]+)*\b") +_EMAIL_RE = re.compile(r"(? dict[str, Any]: # ── Entity / relation extraction (heuristic, no LLM needed) ────────────────── def _extract_entities(text: str) -> list[tuple[str, str]]: + candidates: list[tuple[int, int, int, str, str]] = [] + + # Build the old maximum four-word capitalized sequence in Python rather than + # asking the backtracking engine to discover every possible word grouping. + words = list(_CAPITALIZED_WORD_RE.finditer(text)) + index = 0 + while index < len(words): + first = words[index] + last = first + index += 1 + for _ in range(3): + if index >= len(words) or not text[last.end():words[index].start()].isspace(): + break + last = words[index] + index += 1 + candidates.append((first.start(), last.end(), 0, text[first.start():last.end()], "person_or_concept")) + + candidates.extend( + (match.start(), match.end(), 1, match.group(0), "email") + for match in _EMAIL_RE.finditer(text) + ) + candidates.extend( + (match.start(), match.end(), 2, match.group(0), "hashtag") + for match in _HASHTAG_RE.finditer(text) + ) + candidates.extend( + (match.start(), match.end(), 3, match.group(0), "mention") + for match in _MENTION_RE.finditer(text) + ) + seen: set[str] = set() out: list[tuple[str, str]] = [] - for m in _ENTITY_RE.finditer(text): - raw = m.group(0).strip() + matched_through = 0 + for start, end, _priority, candidate, etype in sorted(candidates): + if start < matched_through: + continue + matched_through = end + raw = candidate.strip() if not raw or raw in _STOPWORDS: continue if raw.lower() in ("user", "the user"): continue - if raw.startswith("#"): - ent, etype = raw, "hashtag" - elif raw.startswith("@") and "@" in raw and "." in raw: - ent, etype = raw, "email" - elif raw.startswith("@"): - ent, etype = raw, "mention" - elif "@" in raw and "." in raw and not raw.startswith("@"): - ent, etype = raw, "email" - else: + if etype == "person_or_concept": ent, etype = raw, "person_or_concept" + else: + ent = raw key = ent.lower() if key not in seen: seen.add(key) @@ -175,7 +202,7 @@ def _extract_entities_from_doc(title: str, content: str) -> list[tuple[str, str] """Extract entities from title and content independently, then merge. Title and content must be processed as *separate* regex passes, never - concatenated first: the capitalized-word pattern in ``_ENTITY_RE`` has no notion + concatenated first: the capitalized-word recognizer has no notion of a title/content boundary, so matching it against ``f"{title}\\n\\n{content}"`` lets it bridge across that boundary — e.g. title "Meeting Notes" + content "Alice Johnson met..." previously produced one garbled entity "Meeting diff --git a/engraphis/routes/memory.py b/engraphis/routes/memory.py index 584ce8d..d3a0a0a 100644 --- a/engraphis/routes/memory.py +++ b/engraphis/routes/memory.py @@ -200,8 +200,9 @@ def _call(): ) answer = await asyncio.to_thread(_call) result["answer"] = answer - except Exception as e: - result["llm_error"] = str(e) + except Exception as exc: # noqa: BLE001 - provider libraries expose many exception types + logger.warning("LLM query error (%s)", type(exc).__name__) + result["llm_error"] = "LLM service unavailable" return _ok(result) diff --git a/engraphis/routes/v2_api.py b/engraphis/routes/v2_api.py index 5b0aab6..4cddbf5 100644 --- a/engraphis/routes/v2_api.py +++ b/engraphis/routes/v2_api.py @@ -67,12 +67,17 @@ def _run(fn, *a, **k): try: return fn(*a, **k) except GraphIndexRebuilding as exc: + logger.info("graph index unavailable (%s, job_id=%s)", type(exc).__name__, exc.job_id) raise HTTPException(status_code=409, detail={ - "error": str(exc), "index_state": "rebuilding", "job_id": exc.job_id, - }) + "error": f"graph index rebuilding (job {exc.job_id})", + "index_state": "rebuilding", + "job_id": exc.job_id, + }) from None except GraphSceneCapacityExceeded as exc: + logger.info("graph scene exceeds capacity (%s, resource=%s, count=%s, limit=%s)", + type(exc).__name__, exc.resource, exc.count, exc.limit) raise HTTPException(status_code=413, detail={ - "error": str(exc), + "error": "graph scene exceeds the safety limit", "safety_state": "capacity_exceeded", "degraded": True, "truncated": False, @@ -80,9 +85,13 @@ def _run(fn, *a, **k): "count": exc.count, "limit": exc.limit, "recommended_action": "narrow repository, time, type, or relation filters", - }) + }) from None except ValidationError as exc: - raise HTTPException(status_code=400, detail={"error": str(exc)}) + logger.info("dashboard request rejected (%s)", type(exc).__name__) + # ValidationError is the service layer's deliberately public, bounded input-error + # type. It is not a traceback or arbitrary provider exception. + # codeql[py/stack-trace-exposure] + raise HTTPException(status_code=400, detail={"error": str(exc)}) from None except HTTPException: raise except Exception as exc: # noqa: BLE001 @@ -105,10 +114,13 @@ def _managed_call(fn, *args, **kwargs): try: return fn(*args, **kwargs) except CloudFeatureError as exc: + logger.warning("managed cloud operation failed (%s, status=%s, transient=%s)", + type(exc).__name__, exc.status, exc.transient) raise HTTPException( status_code=exc.status or 503, - detail={"error": str(exc), "managed_cloud": True, "transient": exc.transient}, - ) + detail={"error": "managed cloud operation failed", "managed_cloud": True, + "transient": exc.transient}, + ) from None def _default_ws() -> Optional[str]: @@ -565,7 +577,9 @@ def llm_activity(workspace: Optional[str] = None, limit: int = 100): try: ws = service()._clean_ws(ws) except ValidationError as exc: - raise HTTPException(status_code=400, detail={"error": str(exc)}) + logger.info("LLM activity request rejected (%s)", type(exc).__name__) + # codeql[py/stack-trace-exposure] ValidationError is safe public input feedback. + raise HTTPException(status_code=400, detail={"error": str(exc)}) from None row = service().store.conn.execute( "SELECT id FROM workspaces WHERE name=?", (ws,) ).fetchone() @@ -823,7 +837,9 @@ def recall(q: str = Query(...), workspace: Optional[str] = None, k: int = 8, try: out = service().recall(q, workspace=ws, k=k, mtypes=mtypes, reinforce=False) except ValidationError as exc: - raise HTTPException(status_code=400, detail={"error": str(exc)}) + logger.info("dashboard recall request rejected (%s)", type(exc).__name__) + # codeql[py/stack-trace-exposure] ValidationError is safe public input feedback. + raise HTTPException(status_code=400, detail={"error": str(exc)}) from None except Exception as exc: # noqa: BLE001 if not _is_embedder_mismatch(exc): logger.error("dashboard recall failed (%s)", type(exc).__name__) @@ -851,7 +867,9 @@ def memories(workspace: Optional[str] = None, q: Optional[str] = None, limit: in try: ws = service()._clean_ws(ws) except ValidationError as exc: - raise HTTPException(status_code=400, detail={"error": str(exc)}) + logger.info("dashboard memories request rejected (%s)", type(exc).__name__) + # codeql[py/stack-trace-exposure] ValidationError is safe public input feedback. + raise HTTPException(status_code=400, detail={"error": str(exc)}) from None conn = _sql.connect("file:%s?mode=ro" % settings.db_path, uri=True) conn.row_factory = _sql.Row try: @@ -905,7 +923,9 @@ def why(q: str = Query(...), workspace: Optional[str] = None, k: int = 5): try: out = service().why(q, workspace=ws, k=k) except ValidationError as exc: - raise HTTPException(status_code=400, detail={"error": str(exc)}) + logger.info("dashboard why request rejected (%s)", type(exc).__name__) + # codeql[py/stack-trace-exposure] ValidationError is safe public input feedback. + raise HTTPException(status_code=400, detail={"error": str(exc)}) from None except Exception as exc: # noqa: BLE001 if not _is_embedder_mismatch(exc): logger.error("dashboard why failed (%s)", type(exc).__name__) @@ -925,7 +945,9 @@ def timeline(q: str = Query(...), workspace: Optional[str] = None, limit: int = try: out = service().timeline(q, workspace=ws, limit=limit) except ValidationError as exc: - raise HTTPException(status_code=400, detail={"error": str(exc)}) + logger.info("dashboard timeline request rejected (%s)", type(exc).__name__) + # codeql[py/stack-trace-exposure] ValidationError is safe public input feedback. + raise HTTPException(status_code=400, detail={"error": str(exc)}) from None except Exception as exc: # noqa: BLE001 if not _is_embedder_mismatch(exc): logger.error("dashboard timeline failed (%s)", type(exc).__name__) @@ -1773,12 +1795,17 @@ def _sync_all(svc) -> dict: read_only = sync_read_only() rep = syncer.sync(transport, row["id"], push=not read_only) except CloudSessionError as exc: - errors.append({"workspace": name, "error": str(exc), "status": 401}) + logger.warning("cloud sync session failed (%s)", type(exc).__name__) + errors.append({"workspace": name, "error": "cloud session authorization failed", + "status": 401}) continue except RelayError as exc: # Record the HTTP status (402 == cloud authorization denied) instead of raising, so # one workspace can't abort the sweep; sync_run() promotes a 402 to the button. - errors.append({"workspace": name, "error": str(exc), "status": exc.status}) + logger.warning("cloud sync relay failed (%s, status=%s)", type(exc).__name__, + exc.status) + errors.append({"workspace": name, "error": "cloud relay synchronization failed", + "status": exc.status}) continue except Exception as exc: # noqa: BLE001 — one bad workspace must not abort the rest logger.error("sync workspace failed (%s)", type(exc).__name__) @@ -1859,7 +1886,9 @@ def configure_sync_token(req: _SyncTokenReq): if not req.read_only: save_sync_read_only(False) except ValueError as exc: - raise HTTPException(status_code=400, detail={"error": str(exc)}) + logger.info("sync token configuration rejected (%s)", type(exc).__name__) + raise HTTPException(status_code=400, + detail={"error": "invalid sync token configuration"}) from None except OSError: raise HTTPException(status_code=503, detail={ "error": "sync token state could not be persisted"}) diff --git a/engraphis/routes/vault.py b/engraphis/routes/vault.py index 4c7143e..531df59 100644 --- a/engraphis/routes/vault.py +++ b/engraphis/routes/vault.py @@ -188,21 +188,31 @@ class FolderImportReq(BaseModel): @router.post("/vaults/import-folder") async def import_folder(req: FolderImportReq): """POST /memory/vaults/import-folder — import all .md files from a disk path.""" - folder = Path(req.path).resolve() - if not folder.exists(): - raise HTTPException(404, f"Path not found: {req.path}") - if not folder.is_dir(): - raise HTTPException(400, f"Not a directory: {req.path}") # Guard against path traversal: only allow import from directories that are # explicitly configured or under the user's home directory. import os - home = Path.home().resolve() + home = os.path.realpath(str(Path.home().expanduser())) allowed_roots = [home] env_roots = os.environ.get("ENGRAPHIS_IMPORT_ROOTS", "") if env_roots: - allowed_roots.extend(Path(r).resolve() for r in env_roots.split(os.pathsep) if r) - if not any(folder == r or folder.is_relative_to(r) for r in allowed_roots): + allowed_roots.extend( + os.path.realpath(os.path.expanduser(root)) + for root in env_roots.split(os.pathsep) + if root + ) + real_path = os.path.realpath(os.path.expanduser(req.path)) + comparable_path = os.path.normcase(real_path) + if not any( + comparable_path == os.path.normcase(root) + or comparable_path.startswith(os.path.normcase(root).rstrip(os.sep) + os.sep) + for root in allowed_roots + ): raise HTTPException(403, "Import path must be under an allowed root (home directory or ENGRAPHIS_IMPORT_ROOTS)") + folder = Path(real_path) + if not folder.exists(): + raise HTTPException(404, f"Path not found: {req.path}") + if not folder.is_dir(): + raise HTTPException(400, f"Not a directory: {req.path}") ns = req.namespace if not ns: @@ -216,19 +226,27 @@ async def import_folder(req: FolderImportReq): import fnmatch files = [] for f in folder.rglob("*"): - if f.is_file() and fnmatch.fnmatch(f.name, req.file_pattern): - if "node_modules" in str(f) or ".git" in str(f): - continue - files.append(f) + if not f.is_file() or not fnmatch.fnmatch(f.name, req.file_pattern): + continue + try: + # Read only the resolved, allowlisted file. In particular, do not let a + # symlink inside an import root redirect this legacy route outside it. + real = f.resolve(strict=True) + rel = real.relative_to(folder) + except (OSError, ValueError): + continue + if any(part in {"node_modules", ".git"} for part in rel.parts[:-1]): + continue + files.append((real, rel)) results = {"imported": 0, "errors": 0, "skipped": 0, "files": []} - for f in files: + for f, rel_path in files: try: content = f.read_text(encoding="utf-8", errors="replace") if not content.strip(): results["skipped"] += 1 continue - rel = f.relative_to(folder).as_posix() + rel = rel_path.as_posix() doc_id = rel.replace("/", "__").replace(".md", "").replace(".", "-") # Extract title from first H1 import re @@ -243,9 +261,9 @@ async def import_folder(req: FolderImportReq): ) results["imported"] += 1 results["files"].append({"path": rel, "title": title, "status": "ok"}) - except Exception as e: + except Exception: results["errors"] += 1 - results["files"].append({"path": str(f), "title": "", "status": "error", "error": str(e)}) + results["files"].append({"path": rel_path.as_posix(), "title": "", "status": "error"}) return _ok({"namespace": ns, "folder": req.path, **results}) diff --git a/engraphis/service.py b/engraphis/service.py index d5c4284..871de30 100644 --- a/engraphis/service.py +++ b/engraphis/service.py @@ -20,6 +20,7 @@ import json import hashlib import contextvars +import logging import math import copy import time @@ -43,6 +44,8 @@ from engraphis.core.store import _loads, _merge_edge_provenance, normalize_entity_name from engraphis.graphdata import build_graph_payload, empty_graph +logger = logging.getLogger("engraphis.service") + # ── validation limits (memory-poisoning / resource-exhaustion guards) ────────── MAX_CONTENT_CHARS = 100_000 MAX_TITLE_CHARS = 1_000 @@ -419,20 +422,30 @@ def _resolve_import_root(raw_path: str) -> Path: ``/memory/vaults/import-folder`` endpoint's convention — home directory by default, widened via ``ENGRAPHIS_IMPORT_ROOTS`` (``os.pathsep``-separated) for server deployments that keep content outside ``$HOME``.""" - folder = Path(raw_path).expanduser().resolve() - if not folder.exists(): - raise ValidationError(f"path not found: {raw_path}") - if not folder.is_dir(): - raise ValidationError(f"not a directory: {raw_path}") - home = Path.home().resolve() + home = os.path.realpath(str(Path.home().expanduser())) allowed_roots = [home] env_roots = os.environ.get("ENGRAPHIS_IMPORT_ROOTS", "") if env_roots: - allowed_roots.extend(Path(r).expanduser().resolve() for r in env_roots.split(os.pathsep) if r) - if not any(folder == r or folder.is_relative_to(r) for r in allowed_roots): + allowed_roots.extend( + os.path.realpath(os.path.expanduser(root)) + for root in env_roots.split(os.pathsep) + if root + ) + real_path = os.path.realpath(os.path.expanduser(raw_path)) + comparable_path = os.path.normcase(real_path) + if not any( + comparable_path == os.path.normcase(root) + or comparable_path.startswith(os.path.normcase(root).rstrip(os.sep) + os.sep) + for root in allowed_roots + ): raise ValidationError( "import path must be under an allowed root (your home directory, or " "ENGRAPHIS_IMPORT_ROOTS)") + folder = Path(real_path) + if not folder.exists(): + raise ValidationError(f"path not found: {raw_path}") + if not folder.is_dir(): + raise ValidationError(f"not a directory: {raw_path}") return folder @@ -452,16 +465,17 @@ def _iter_import_files(folder: Path, pattern: str, max_files: int) -> list: break if not f.is_file() or not fnmatch.fnmatch(f.name, pattern): continue - parts = f.relative_to(folder).parts - if any(p == "node_modules" or p == ".git" or p.startswith(".") for p in parts[:-1]): - continue try: - real = f.resolve() - except OSError: + # ``f`` came from a user-selected tree. Resolve and contain it before both + # deriving metadata and returning the path that ``import_folder`` will read. + real = f.resolve(strict=True) + rel = real.relative_to(folder) + except (OSError, ValueError): continue - if not (real == folder or real.is_relative_to(folder)): + parts = rel.parts + if any(p == "node_modules" or p == ".git" or p.startswith(".") for p in parts[:-1]): continue - files.append(f) + files.append(real) return files @@ -1206,8 +1220,9 @@ def import_folder(self, *, workspace: str, path: str, file_pattern: str = "*.md" if "no extractable text" in str(exc): skipped += 1 continue + logger.warning("folder import failed for one file (%s)", type(exc).__name__) errors += 1 - details.append({"file": f.name, "error": str(exc)}) + details.append({"file": f.name, "error": "file could not be imported"}) continue rel = f.relative_to(folder).as_posix() resource_meta = { @@ -1241,7 +1256,9 @@ def import_folder(self, *, workspace: str, path: str, file_pattern: str = "*.md" if note: file_warnings.append(note) except (OSError, ValueError) as exc: - file_warnings.append(f"fact derivation failed: {exc}") + logger.warning("fact derivation failed for one file (%s)", + type(exc).__name__) + file_warnings.append("fact derivation failed") if file_warnings: warnings.append({"file": rel, "warnings": file_warnings}) diff --git a/scripts/check_codeql_sarif.py b/scripts/check_codeql_sarif.py new file mode 100644 index 0000000..5780283 --- /dev/null +++ b/scripts/check_codeql_sarif.py @@ -0,0 +1,67 @@ +"""Fail a CI job when a CodeQL SARIF directory contains any findings.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path +from typing import Any + + +MAX_REPORTED_FINDINGS = 50 + + +def _location(result: dict[str, Any]) -> str: + locations = result.get("locations") + if not isinstance(locations, list) or not locations: + return "" + physical = locations[0].get("physicalLocation", {}) + artifact = physical.get("artifactLocation", {}) + region = physical.get("region", {}) + path = artifact.get("uri", "") + line = region.get("startLine") + return f"{path}:{line}" if isinstance(line, int) else str(path) + + +def findings_in(path: Path) -> list[str]: + """Return bounded, human-readable findings from one SARIF file.""" + + document = json.loads(path.read_text(encoding="utf-8")) + findings: list[str] = [] + for run in document.get("runs", []): + for result in run.get("results", []): + rule = result.get("ruleId", "") + message = result.get("message", {}).get("text", "") + findings.append(f"{rule} at {_location(result)}: {message}") + return findings + + +def main(argv: list[str] | None = None) -> int: + args = sys.argv[1:] if argv is None else argv + if len(args) != 1: + print("usage: check_codeql_sarif.py ", file=sys.stderr) + return 2 + directory = Path(args[0]) + sarif_files = sorted(directory.rglob("*.sarif")) + if not sarif_files: + print(f"CodeQL gate: no SARIF files found under {directory}", file=sys.stderr) + return 2 + findings = [ + finding + for sarif_file in sarif_files + for finding in findings_in(sarif_file) + ] + if findings: + print(f"CodeQL gate: {len(findings)} finding(s)", file=sys.stderr) + for finding in findings[:MAX_REPORTED_FINDINGS]: + print(f"- {finding}", file=sys.stderr) + hidden = len(findings) - MAX_REPORTED_FINDINGS + if hidden > 0: + print(f"- ... {hidden} additional finding(s) omitted", file=sys.stderr) + return 1 + print(f"CodeQL gate: clean ({len(sarif_files)} SARIF file(s))") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/externalize_dashboard_assets.py b/scripts/externalize_dashboard_assets.py index 2b35f8e..62b43b4 100644 --- a/scripts/externalize_dashboard_assets.py +++ b/scripts/externalize_dashboard_assets.py @@ -7,7 +7,10 @@ from __future__ import annotations import re +from dataclasses import dataclass +from html.parser import HTMLParser from pathlib import Path +from typing import Optional ROOT = Path(__file__).resolve().parents[1] @@ -18,8 +21,6 @@ STYLE_ATTR = re.compile(r"\sstyle=(?:\"([^\"]*)\"|'([^']*)')") EVENT_ATTR = re.compile(r"\s(on[a-z]+)=(?:\"([^\"]*)\"|'([^']*)')") -INLINE_SCRIPT = re.compile(r"]*\bsrc=)[^>]*>\r?\n?([\s\S]*?)") -INLINE_STYLE = re.compile(r"") STYLE_REF = re.compile(r'data-csp-style=["\'](s\d+)["\']') STYLE_RULE = re.compile(r'\[data-csp-style=["\'](s\d+)["\']\]\{') HANDLER_REF = re.compile(r'data-on([a-z]+)=["\'](h\d+)["\']') @@ -27,6 +28,82 @@ DELEGATED_EVENTS = re.compile(r"for\(const type of \[([^]]+)\]\)") +@dataclass(frozen=True) +class _InlineAsset: + start: int + end: int + content: str + + +class _InlineAssetParser(HTMLParser): + """Locate inline dashboard assets with an HTML parser, not a tag regex. + + Browser HTML parsing accepts malformed closing tags and mixed-case tag names; + using ``HTMLParser`` keeps the release gate aligned with that parsing model. + Offsets retain the source bytes exactly, so extraction remains mechanical. + """ + + def __init__(self, source: str) -> None: + super().__init__(convert_charrefs=False) + self.source = source + self._line_offsets = [0] + self._line_offsets.extend( + index + 1 for index, char in enumerate(source) if char == "\n" + ) + self._open: dict[str, tuple[int, int]] = {} + self.styles: list[_InlineAsset] = [] + self.scripts: list[_InlineAsset] = [] + + def _offset(self) -> int: + line, column = self.getpos() + return self._line_offsets[line - 1] + column + + def handle_starttag(self, tag: str, attrs: list[tuple[str, Optional[str]]]) -> None: + if tag not in {"style", "script"}: + return + if tag == "script" and any(name.lower() == "src" for name, _value in attrs): + return + start = self._offset() + self._open[tag] = (start, start + len(self.get_starttag_text())) + + def handle_endtag(self, tag: str) -> None: + opened = self._open.pop(tag, None) + if opened is None: + return + close_start = self._offset() + close_end = self.source.find(">", close_start) + if close_end < 0: + return + asset = _InlineAsset(opened[0], close_end + 1, self.source[opened[1]:close_start]) + (self.styles if tag == "style" else self.scripts).append(asset) + + def finish_unclosed(self) -> None: + """Treat an asset tag that reaches EOF as inline browser content.""" + + for tag, opened in self._open.items(): + asset = _InlineAsset( + opened[0], + len(self.source), + self.source[opened[1]:], + ) + (self.styles if tag == "style" else self.scripts).append(asset) + self._open.clear() + + +def _inline_assets(html: str) -> tuple[list[_InlineAsset], list[_InlineAsset]]: + parser = _InlineAssetParser(html) + parser.feed(html) + parser.close() + parser.finish_unclosed() + return parser.styles, parser.scripts + + +def _replace_assets(html: str, replacements: list[tuple[_InlineAsset, str]]) -> str: + for asset, replacement in sorted(replacements, key=lambda item: item[0].start, reverse=True): + html = html[:asset.start] + replacement + html[asset.end:] + return html + + def _add_generated_listeners(source: str, handlers: dict[tuple[str, str], str]) -> str: lines = [ "", @@ -52,7 +129,8 @@ def _add_generated_listeners(source: str, handlers: dict[tuple[str, str], str]) def migrate() -> None: html = INDEX.read_text(encoding="utf-8") - if not INLINE_STYLE.search(html) and not INLINE_SCRIPT.search(html): + style_assets, script_assets = _inline_assets(html) + if not style_assets and not script_assets: check() return @@ -84,20 +162,21 @@ def replace_handler(match: re.Match[str]) -> str: html = EVENT_ATTR.sub(replace_handler, html).replace("[onclick]", "[data-onclick]") - style_match = INLINE_STYLE.search(html) - script_matches = list(INLINE_SCRIPT.finditer(html)) - if style_match is None or len(script_matches) != 1: + style_assets, script_assets = _inline_assets(html) + if len(style_assets) != 1 or len(script_assets) != 1: raise RuntimeError("dashboard must contain exactly one inline style and script block") - css = style_match.group(1).rstrip() + css = style_assets[0].content.rstrip() css += "\n\n/* Generated from former static style attributes. */\n" css += "".join( f'[data-csp-style="{style_id}"]{{{value}}}\n' for value, style_id in styles.items() ) - js = _add_generated_listeners(script_matches[0].group(1), handlers) - html = INLINE_STYLE.sub('', html) - html = INLINE_SCRIPT.sub('', html) + js = _add_generated_listeners(script_assets[0].content, handlers) + html = _replace_assets(html, [ + (style_assets[0], ''), + (script_assets[0], ''), + ]) CSS.write_text(css, encoding="utf-8", newline="\n") JS.write_text(js, encoding="utf-8", newline="\n") @@ -111,9 +190,10 @@ def check() -> None: css = CSS.read_text(encoding="utf-8") if CSS.is_file() else "" js = JS.read_text(encoding="utf-8") if JS.is_file() else "" failures = [] - if INLINE_STYLE.search(html): + style_assets, script_assets = _inline_assets(html) + if style_assets: failures.append("inline style block") - if INLINE_SCRIPT.search(html): + if script_assets: failures.append("inline script block") if STYLE_ATTR.search(html): failures.append("inline style attribute") diff --git a/tests/test_codeql_sarif_gate.py b/tests/test_codeql_sarif_gate.py new file mode 100644 index 0000000..c941161 --- /dev/null +++ b/tests/test_codeql_sarif_gate.py @@ -0,0 +1,72 @@ +from __future__ import annotations + +import json + +from scripts.check_codeql_sarif import MAX_REPORTED_FINDINGS, findings_in, main + + +def _write_sarif(tmp_path, results): + path = tmp_path / "python.sarif" + path.write_text( + json.dumps({"version": "2.1.0", "runs": [{"results": results}]}), + encoding="utf-8", + ) + return path + + +def test_codeql_gate_accepts_clean_sarif(tmp_path, capsys) -> None: + _write_sarif(tmp_path, []) + + assert main([str(tmp_path)]) == 0 + assert "CodeQL gate: clean" in capsys.readouterr().out + + +def test_codeql_gate_reports_and_rejects_findings(tmp_path, capsys) -> None: + path = _write_sarif( + tmp_path, + [ + { + "ruleId": "py/example", + "message": {"text": "unsafe example"}, + "locations": [ + { + "physicalLocation": { + "artifactLocation": {"uri": "engraphis/example.py"}, + "region": {"startLine": 12}, + } + } + ], + } + ], + ) + + assert findings_in(path) == [ + "py/example at engraphis/example.py:12: unsafe example" + ] + assert main([str(tmp_path)]) == 1 + captured = capsys.readouterr() + assert "CodeQL gate: 1 finding(s)" in captured.err + assert "py/example at engraphis/example.py:12" in captured.err + + +def test_codeql_gate_rejects_missing_sarif(tmp_path, capsys) -> None: + assert main([str(tmp_path)]) == 2 + assert "no SARIF files found" in capsys.readouterr().err + + +def test_codeql_gate_bounds_finding_output(tmp_path, capsys) -> None: + _write_sarif( + tmp_path, + [ + { + "ruleId": f"py/example-{index}", + "message": {"text": "unsafe example"}, + } + for index in range(MAX_REPORTED_FINDINGS + 1) + ], + ) + + assert main([str(tmp_path)]) == 1 + captured = capsys.readouterr() + assert f"CodeQL gate: {MAX_REPORTED_FINDINGS + 1} finding(s)" in captured.err + assert "- ... 1 additional finding(s) omitted" in captured.err diff --git a/tests/test_dashboard_v2.py b/tests/test_dashboard_v2.py index 53a2161..9f0bf21 100644 --- a/tests/test_dashboard_v2.py +++ b/tests/test_dashboard_v2.py @@ -7,8 +7,11 @@ pytest.importorskip("httpx", reason="httpx not installed") from fastapi.testclient import TestClient # noqa: E402 +from fastapi import HTTPException # noqa: E402 from engraphis.config import settings # noqa: E402 +from engraphis.cloud_features import CloudFeatureError # noqa: E402 +from engraphis.routes import v2_api # noqa: E402 from engraphis.service import MemoryService # noqa: E402 @@ -263,3 +266,25 @@ def test_health_and_readiness_remain_public(monkeypatch, tmp_path): with _client(monkeypatch, tmp_path) as client: assert client.get("/api/health").status_code == 200 assert client.get("/api/ready").status_code == 200 + + +def test_dashboard_exception_responses_do_not_echo_untrusted_exception_text(): + secret = "https://provider.example/?api_key=do-not-return-this" + + def fail_with(exc): + raise exc + + with pytest.raises(HTTPException) as internal: + v2_api._run(fail_with, RuntimeError(secret)) + assert internal.value.status_code == 500 + assert internal.value.detail == {"error": "internal server error"} + assert secret not in repr(internal.value.detail) + + with pytest.raises(HTTPException) as managed: + v2_api._managed_call(fail_with, CloudFeatureError(secret, status=502)) + assert managed.value.status_code == 502 + assert managed.value.detail == { + "error": "managed cloud operation failed", "managed_cloud": True, + "transient": False, + } + assert secret not in repr(managed.value.detail) diff --git a/tests/test_engine.py b/tests/test_engine.py index 3d17a80..e7ba01c 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -550,6 +550,24 @@ def test_index_repo_skips_unsupported_files(tmp_path): assert report["files_indexed"] == 0 +def test_index_repo_never_reads_a_symlink_that_escapes_root(tmp_path): + outside = tmp_path.parent / (tmp_path.name + "-outside-indexed-source.py") + outside.write_text("def leaked_secret(): pass\n", encoding="utf-8") + link = tmp_path / "escape.py" + try: + link.symlink_to(outside) + except (OSError, NotImplementedError): + pytest.skip("symlinks not supported in this environment") + eng = MemoryEngine.create(":memory:") + wid = eng.store.get_or_create_workspace("w") + rid = eng.store.get_or_create_repo(wid, "sample") + + report = eng.index_repo(rid, str(tmp_path), prefer="regex") + + assert report["files_indexed"] == 0 + assert eng.search_code("leaked_secret", repo_id=rid)["symbols"] == [] + + def test_truncated_incremental_scan_does_not_delete_unseen_files(tmp_path): (tmp_path / "a.py").write_text("def alpha(): pass\n") (tmp_path / "b.py").write_text("def beta(): pass\n") diff --git a/tests/test_externalize_dashboard_assets.py b/tests/test_externalize_dashboard_assets.py new file mode 100644 index 0000000..95712a5 --- /dev/null +++ b/tests/test_externalize_dashboard_assets.py @@ -0,0 +1,54 @@ +"""Regression coverage for the dashboard CSP asset release gate.""" +from __future__ import annotations + +import pytest + +from scripts import externalize_dashboard_assets as assets + + +def test_inline_asset_parser_handles_case_and_malformed_closing_tag(): + styles, scripts = assets._inline_assets( + "" + ) + + assert [asset.content for asset in styles] == ["body{color:red}"] + assert [asset.content for asset in scripts] == ["alert(1)"] + + +def test_migrate_uses_parsed_asset_boundaries(tmp_path, monkeypatch): + index = tmp_path / "index.html" + css = tmp_path / "dashboard.css" + js = tmp_path / "dashboard.js" + index.write_text( + "" + "" + "", + encoding="utf-8", + ) + monkeypatch.setattr(assets, "INDEX", index) + monkeypatch.setattr(assets, "CSS", css) + monkeypatch.setattr(assets, "JS", js) + + assets.migrate() + + html = index.read_text(encoding="utf-8") + assert '' in html + assert '' in html + assert "body{color:red}" in css.read_text(encoding="utf-8") + assert "CSP_EVENT_HANDLERS" in js.read_text(encoding="utf-8") + + +@pytest.mark.parametrize("tag", ["script", "style"]) +def test_check_rejects_unclosed_inline_asset_at_eof(tmp_path, monkeypatch, tag): + index = tmp_path / "index.html" + css = tmp_path / "dashboard.css" + js = tmp_path / "dashboard.js" + index.write_text(f"<{tag}>unclosed", encoding="utf-8") + css.write_text("", encoding="utf-8") + js.write_text("", encoding="utf-8") + monkeypatch.setattr(assets, "INDEX", index) + monkeypatch.setattr(assets, "CSS", css) + monkeypatch.setattr(assets, "JS", js) + + with pytest.raises(SystemExit, match=f"inline {tag} block"): + assets.check() diff --git a/tests/test_graph_extractor.py b/tests/test_graph_extractor.py index 1558fd6..b6dbffd 100644 --- a/tests/test_graph_extractor.py +++ b/tests/test_graph_extractor.py @@ -9,6 +9,7 @@ """ from __future__ import annotations +import time from engraphis.backends import graph_extractor as graph_extractor_module from engraphis.backends.graph_extractor import ( @@ -77,6 +78,32 @@ def test_regex_and_feed_bound_per_memory_entity_fanout(): ).fetchone()["n"] == graph_extractor_module._MAX_ENTITIES +def test_regex_entity_extraction_rejects_long_non_email_in_linear_time(): + """A no-``@`` local-part candidate used to make the unanchored email arm retry + from every character. It must remain a fast, empty extraction on untrusted text.""" + start = time.perf_counter() + extraction = RegexGraphExtractor().extract("a" * 100_000) + elapsed = time.perf_counter() - start + + assert extraction.entities == [] + assert elapsed < 1.0 + + +def test_regex_entity_extraction_bounds_large_capitalized_candidate_set(): + # Four words form one candidate, so this input contains far more candidates than + # the public graph fanout limit while remaining deterministic and inexpensive. + text = " ".join( + "A" + chr(97 + (index // 26) % 26) + chr(97 + index % 26) + for index in range(10_000) + ) + start = time.perf_counter() + extraction = RegexGraphExtractor().extract(text) + elapsed = time.perf_counter() - start + + assert len(extraction.entities) == graph_extractor_module._MAX_ENTITIES + assert elapsed < 1.0 + + # ── the feed() writer (scoped graph) ────────────────────────────────────────── def test_feed_writes_scoped_entities_and_edges(): diff --git a/tests/test_ingest_entities.py b/tests/test_ingest_entities.py index f4e30ff..5bca70f 100644 --- a/tests/test_ingest_entities.py +++ b/tests/test_ingest_entities.py @@ -12,6 +12,8 @@ """ from __future__ import annotations +import time + import numpy as np import pytest @@ -91,3 +93,14 @@ def test_click_target_shape_has_namespace_and_documents(): ent = ents["Zephyr"] assert ent["documents"] == ["doc-1"] assert "preview_title" in ent and "preview_content" in ent + + +def test_entity_extraction_rejects_long_non_email_in_linear_time(): + """Keep ingestion responsive when untrusted text resembles an email local part + but never supplies an ``@`` separator.""" + start = time.perf_counter() + entities = ingest_engine._extract_entities("a" * 100_000) + elapsed = time.perf_counter() - start + + assert entities == [] + assert elapsed < 1.0 diff --git a/tests/test_memory_routes_fixes.py b/tests/test_memory_routes_fixes.py index a0506cb..a307155 100644 --- a/tests/test_memory_routes_fixes.py +++ b/tests/test_memory_routes_fixes.py @@ -110,3 +110,30 @@ def test_conversations_missing_content_is_400_not_500(monkeypatch, tmp_path): with _client(monkeypatch, tmp_path) as c: r = c.post("/memory/conversations", json={"messages": [{"role": "user"}]}) assert r.status_code == 400 + + +def test_query_context_does_not_echo_llm_exception_text(monkeypatch, tmp_path): + secret = "https://provider.example/?api_key=do-not-return-this" + + class _FailingLLM: + def __enter__(self): + return self + + def __exit__(self, *_): + return False + + def chat_with_context(self, **kwargs): + raise RuntimeError(secret) + + monkeypatch.setattr("engraphis.routes.memory.LLMClient", _FailingLLM) + monkeypatch.setattr( + "engraphis.routes.memory.recall_engine.recall", + lambda **kwargs: {"llmContextMessage": "", "count": 0, "chunks": []}, + ) + with _client(monkeypatch, tmp_path) as c: + response = c.post("/memory/queries", json={"query": "what is stored?"}) + + assert response.status_code == 200 + result = response.json()["data"] + assert result["llm_error"] == "LLM service unavailable" + assert secret not in repr(result) diff --git a/tests/test_provider_error_redaction.py b/tests/test_provider_error_redaction.py index f374b89..a6343e3 100644 --- a/tests/test_provider_error_redaction.py +++ b/tests/test_provider_error_redaction.py @@ -161,6 +161,30 @@ def post(self, *_args, **_kwargs): assert marker not in caplog.text +def test_api_embedder_failure_logs_do_not_include_api_key(monkeypatch, caplog): + api_key = "embedding-api-key-should-not-escape" + + class _Client: + def __init__(self, **_kwargs): + pass + + def __enter__(self): + return self + + def __exit__(self, *_args): + return False + + def post(self, *_args, **_kwargs): + raise RuntimeError(api_key) + + monkeypatch.setattr(httpx, "Client", _Client) + with caplog.at_level(logging.INFO, logger="engraphis.embedder_api"): + result = ApiEmbedder(model="safe-model", api_key=api_key, dim=2).embed(["hello"]) + + assert result.shape == (1, 2) + assert api_key not in caplog.text + + @pytest.mark.parametrize("status", [402, 500]) def test_relay_http_error_discards_response_body_and_request_url(monkeypatch, status): url_marker = "relay-customer@example.com" diff --git a/tests/test_release_infrastructure.py b/tests/test_release_infrastructure.py index e97e5d5..236364a 100644 --- a/tests/test_release_infrastructure.py +++ b/tests/test_release_infrastructure.py @@ -98,6 +98,17 @@ def test_ci_and_release_default_to_read_only_repository_permissions(): assert "\npermissions:\n contents: read\n" in header +def test_codeql_workflow_fails_when_sarif_contains_findings(): + workflow = _text(".github/workflows/codeql.yml") + + assert "id: analyze" in workflow + assert "output: codeql-results" in workflow + assert ( + 'python scripts/check_codeql_sarif.py ' + '"${{ steps.analyze.outputs.sarif-output }}"' + ) in workflow + + def test_release_repair_requires_tag_sha_successful_build_publish_and_pypi_identity(): repair = _text(".github/workflows/release.yml").split( "github-release-repair:", 1 diff --git a/tests/test_sync_dashboard.py b/tests/test_sync_dashboard.py index 7e3dc94..44da806 100644 --- a/tests/test_sync_dashboard.py +++ b/tests/test_sync_dashboard.py @@ -167,3 +167,25 @@ def fake_get_transport(kind="folder", **kw): error["workspace"] == "demo" and "visibility is invalid" in error["error"] for error in summary["errors"] ) + + +def test_sync_error_summary_does_not_echo_relay_exception_text(monkeypatch, tmp_path): + from engraphis.backends.sync_relay import RelayError + + secret = "https://relay.example/?token=do-not-return-this" + + def fail_transport(*args, **kwargs): + raise RelayError(secret, status=502) + + monkeypatch.setattr("engraphis.backends.sync_folder.get_transport", fail_transport) + with _client(monkeypatch, tmp_path, cloud=True) as c: + response = c.post("/api/sync/run", json={}) + + assert response.status_code == 200 + errors = response.json()["summary"]["errors"] + assert errors == [{ + "workspace": "demo", + "error": "cloud relay synchronization failed", + "status": 502, + }] + assert secret not in repr(errors) From 0188464818b9702c017749515f2966b7cf32805a Mon Sep 17 00:00:00 2001 From: Coding-Dev-Tools Date: Thu, 23 Jul 2026 15:31:27 -0400 Subject: [PATCH 2/5] ci: preserve Python 3.9 release gates --- pyproject.toml | 4 ++-- scripts/externalize_dashboard_assets.py | 13 ++++++++++--- tests/test_release_infrastructure.py | 6 ++++++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index b01d177..4efedf0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -124,13 +124,13 @@ all = [ "onnxruntime<1.24; python_version < '3.11'", "psycopg[binary]>=3.1", ] -dev = ["pytest>=8.0", "pytest-asyncio>=0.23", "ruff>=0.4"] +dev = ["pytest>=8.0", "pytest-asyncio>=0.23", "ruff>=0.15.22,<0.16"] # Everything needed to run the FULL offline gate in CI (lint + all extras-gated tests) # WITHOUT pulling torch/sentence-transformers — no test needs the real embedder. test = [ "pytest>=8.0", "pytest-asyncio>=0.23", - "ruff>=0.4", + "ruff>=0.15.22,<0.16", "python-dotenv>=1.0", "uvicorn[standard]>=0.29", "fastapi>=0.133.1,<1; python_version >= '3.10'", diff --git a/scripts/externalize_dashboard_assets.py b/scripts/externalize_dashboard_assets.py index 62b43b4..4c27b2d 100644 --- a/scripts/externalize_dashboard_assets.py +++ b/scripts/externalize_dashboard_assets.py @@ -104,6 +104,13 @@ def _replace_assets(html: str, replacements: list[tuple[_InlineAsset, str]]) -> return html +def _write_lf(path: Path, content: str) -> None: + # ``Path.write_text(..., newline=...)`` is Python 3.10+, while Engraphis keeps + # Python 3.9 as its package floor. + with path.open("w", encoding="utf-8", newline="\n") as handle: + handle.write(content) + + def _add_generated_listeners(source: str, handlers: dict[tuple[str, str], str]) -> str: lines = [ "", @@ -178,9 +185,9 @@ def replace_handler(match: re.Match[str]) -> str: (script_assets[0], ''), ]) - CSS.write_text(css, encoding="utf-8", newline="\n") - JS.write_text(js, encoding="utf-8", newline="\n") - INDEX.write_text(html, encoding="utf-8", newline="\n") + _write_lf(CSS, css) + _write_lf(JS, js) + _write_lf(INDEX, html) check() print(f"externalized {len(styles)} styles and {len(handlers)} event handlers") diff --git a/tests/test_release_infrastructure.py b/tests/test_release_infrastructure.py index 236364a..ec15b0d 100644 --- a/tests/test_release_infrastructure.py +++ b/tests/test_release_infrastructure.py @@ -109,6 +109,12 @@ def test_codeql_workflow_fails_when_sarif_contains_findings(): ) in workflow +def test_ci_linter_is_bounded_to_the_verified_release_series(): + pyproject = _text("pyproject.toml") + + assert pyproject.count('"ruff>=0.15.22,<0.16"') == 2 + + def test_release_repair_requires_tag_sha_successful_build_publish_and_pypi_identity(): repair = _text(".github/workflows/release.yml").split( "github-release-repair:", 1 From 664d4523d34dda258c32e0162bf9092a552b59d4 Mon Sep 17 00:00:00 2001 From: Coding-Dev-Tools Date: Thu, 23 Jul 2026 15:36:12 -0400 Subject: [PATCH 3/5] security: eliminate remaining CodeQL dataflow alerts --- engraphis/backends/graph_extractor.py | 43 +++++++++++++++++++++++-- engraphis/engines/ingest.py | 45 +++++++++++++++++++++++++-- engraphis/routes/vault.py | 3 ++ engraphis/service.py | 3 ++ tests/test_graph_extractor.py | 11 ++++--- tests/test_ingest_entities.py | 11 ++++--- 6 files changed, 100 insertions(+), 16 deletions(-) diff --git a/engraphis/backends/graph_extractor.py b/engraphis/backends/graph_extractor.py index a38fe42..ba99a35 100644 --- a/engraphis/backends/graph_extractor.py +++ b/engraphis/backends/graph_extractor.py @@ -33,9 +33,17 @@ # individual recognizers unambiguous: the old all-in-one expression could take # polynomial time while backtracking over a long email-like local part. _CAPITALIZED_WORD_RE = re.compile(r"\b[A-Z][a-z]+(?:-[A-Za-z]+)*\b") -_EMAIL_RE = re.compile(r"(? 0 and text[index - 1] in _EMAIL_LOCAL_CHARS: + index += 1 + continue + start = index + while index < length and text[index] in _EMAIL_LOCAL_CHARS: + index += 1 + if index >= length or text[index] != "@": + continue + domain_start = index + 1 + end = domain_start + while end < length and text[end] in _EMAIL_DOMAIN_CHARS: + end += 1 + domain = text[domain_start:end] + dot = domain.rfind(".") + suffix = domain[dot + 1:] if dot > 0 else "" + if len(suffix) >= 2 and all(char in _EMAIL_SUFFIX_CHARS for char in suffix): + yield start, end, text[start:end] + index = domain_start if end < length and text[end] == "@" else end + + def _defang(value: str) -> str: return _CONTROL_RE.sub("", value or "").strip()[:_MAX_NAME] @@ -140,8 +177,8 @@ def capitalized_candidates(): candidates = heapq.merge( capitalized_candidates(), ( - (match.start(), 1, match.end(), match.group(0), "email") - for match in _EMAIL_RE.finditer(text) + (start, 1, end, value, "email") + for start, end, value in _iter_emails(text) ), ( (match.start(), 2, match.end(), match.group(0), "hashtag") diff --git a/engraphis/engines/ingest.py b/engraphis/engines/ingest.py index ca32dd9..f9a7e89 100644 --- a/engraphis/engines/ingest.py +++ b/engraphis/engines/ingest.py @@ -21,9 +21,17 @@ # Keep each recognizer unambiguous. The previous combined expression nested a # repeated, unanchored branches and could take polynomial time on adversarial input. _CAPITALIZED_WORD_RE = re.compile(r"\b[A-Z][a-z]+(?:-[A-Za-z]+)*\b") -_EMAIL_RE = re.compile(r"(? 0 and text[index - 1] in _EMAIL_LOCAL_CHARS: + index += 1 + continue + start = index + while index < length and text[index] in _EMAIL_LOCAL_CHARS: + index += 1 + if index >= length or text[index] != "@": + continue + domain_start = index + 1 + end = domain_start + while end < length and text[end] in _EMAIL_DOMAIN_CHARS: + end += 1 + domain = text[domain_start:end] + dot = domain.rfind(".") + suffix = domain[dot + 1:] if dot > 0 else "" + if len(suffix) >= 2 and all(char in _EMAIL_SUFFIX_CHARS for char in suffix): + yield start, end, text[start:end] + # If another @ terminated an invalid domain, its domain run can be the local + # part of a later valid address (for example ``bad@name@valid.test``). + index = domain_start if end < length and text[end] == "@" else end + + def ingest_document( *, namespace: str, @@ -163,8 +202,8 @@ def _extract_entities(text: str) -> list[tuple[str, str]]: candidates.append((first.start(), last.end(), 0, text[first.start():last.end()], "person_or_concept")) candidates.extend( - (match.start(), match.end(), 1, match.group(0), "email") - for match in _EMAIL_RE.finditer(text) + (start, end, 1, value, "email") + for start, end, value in _iter_emails(text) ) candidates.extend( (match.start(), match.end(), 2, match.group(0), "hashtag") diff --git a/engraphis/routes/vault.py b/engraphis/routes/vault.py index 531df59..0d714ef 100644 --- a/engraphis/routes/vault.py +++ b/engraphis/routes/vault.py @@ -209,8 +209,11 @@ async def import_folder(req: FolderImportReq): ): raise HTTPException(403, "Import path must be under an allowed root (home directory or ENGRAPHIS_IMPORT_ROOTS)") folder = Path(real_path) + # real_path was canonicalized and checked against the configured roots above. + # codeql[py/path-injection] if not folder.exists(): raise HTTPException(404, f"Path not found: {req.path}") + # codeql[py/path-injection] if not folder.is_dir(): raise HTTPException(400, f"Not a directory: {req.path}") diff --git a/engraphis/service.py b/engraphis/service.py index 871de30..3555aa3 100644 --- a/engraphis/service.py +++ b/engraphis/service.py @@ -442,8 +442,11 @@ def _resolve_import_root(raw_path: str) -> Path: "import path must be under an allowed root (your home directory, or " "ENGRAPHIS_IMPORT_ROOTS)") folder = Path(real_path) + # real_path was canonicalized and checked against the configured roots above. + # codeql[py/path-injection] if not folder.exists(): raise ValidationError(f"path not found: {raw_path}") + # codeql[py/path-injection] if not folder.is_dir(): raise ValidationError(f"not a directory: {raw_path}") return folder diff --git a/tests/test_graph_extractor.py b/tests/test_graph_extractor.py index b6dbffd..6e36e6c 100644 --- a/tests/test_graph_extractor.py +++ b/tests/test_graph_extractor.py @@ -81,12 +81,13 @@ def test_regex_and_feed_bound_per_memory_entity_fanout(): def test_regex_entity_extraction_rejects_long_non_email_in_linear_time(): """A no-``@`` local-part candidate used to make the unanchored email arm retry from every character. It must remain a fast, empty extraction on untrusted text.""" - start = time.perf_counter() - extraction = RegexGraphExtractor().extract("a" * 100_000) - elapsed = time.perf_counter() - start + for text in ("a" * 100_000, "%" * 100_000): + start = time.perf_counter() + extraction = RegexGraphExtractor().extract(text) + elapsed = time.perf_counter() - start - assert extraction.entities == [] - assert elapsed < 1.0 + assert extraction.entities == [] + assert elapsed < 1.0 def test_regex_entity_extraction_bounds_large_capitalized_candidate_set(): diff --git a/tests/test_ingest_entities.py b/tests/test_ingest_entities.py index 5bca70f..5dee57b 100644 --- a/tests/test_ingest_entities.py +++ b/tests/test_ingest_entities.py @@ -98,9 +98,10 @@ def test_click_target_shape_has_namespace_and_documents(): def test_entity_extraction_rejects_long_non_email_in_linear_time(): """Keep ingestion responsive when untrusted text resembles an email local part but never supplies an ``@`` separator.""" - start = time.perf_counter() - entities = ingest_engine._extract_entities("a" * 100_000) - elapsed = time.perf_counter() - start + for text in ("a" * 100_000, "%" * 100_000): + start = time.perf_counter() + entities = ingest_engine._extract_entities(text) + elapsed = time.perf_counter() - start - assert entities == [] - assert elapsed < 1.0 + assert entities == [] + assert elapsed < 1.0 From 9c316c00ceef9d1c0859fd45d3a6d3c5095e976c Mon Sep 17 00:00:00 2001 From: Coding-Dev-Tools Date: Thu, 23 Jul 2026 15:39:31 -0400 Subject: [PATCH 4/5] security: scope path suppressions to checked sinks --- engraphis/routes/vault.py | 6 ++---- engraphis/service.py | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/engraphis/routes/vault.py b/engraphis/routes/vault.py index 0d714ef..c6bfc5d 100644 --- a/engraphis/routes/vault.py +++ b/engraphis/routes/vault.py @@ -210,11 +210,9 @@ async def import_folder(req: FolderImportReq): raise HTTPException(403, "Import path must be under an allowed root (home directory or ENGRAPHIS_IMPORT_ROOTS)") folder = Path(real_path) # real_path was canonicalized and checked against the configured roots above. - # codeql[py/path-injection] - if not folder.exists(): + if not folder.exists(): # codeql[py/path-injection] raise HTTPException(404, f"Path not found: {req.path}") - # codeql[py/path-injection] - if not folder.is_dir(): + if not folder.is_dir(): # codeql[py/path-injection] raise HTTPException(400, f"Not a directory: {req.path}") ns = req.namespace diff --git a/engraphis/service.py b/engraphis/service.py index 3555aa3..a45333d 100644 --- a/engraphis/service.py +++ b/engraphis/service.py @@ -443,11 +443,9 @@ def _resolve_import_root(raw_path: str) -> Path: "ENGRAPHIS_IMPORT_ROOTS)") folder = Path(real_path) # real_path was canonicalized and checked against the configured roots above. - # codeql[py/path-injection] - if not folder.exists(): + if not folder.exists(): # codeql[py/path-injection] raise ValidationError(f"path not found: {raw_path}") - # codeql[py/path-injection] - if not folder.is_dir(): + if not folder.is_dir(): # codeql[py/path-injection] raise ValidationError(f"not a directory: {raw_path}") return folder From e44d2ae68148588b88b343b115afc855202840d4 Mon Sep 17 00:00:00 2001 From: Coding-Dev-Tools Date: Thu, 23 Jul 2026 15:44:36 -0400 Subject: [PATCH 5/5] security: make path containment machine-verifiable --- engraphis/routes/vault.py | 23 ++++++++++++++--------- engraphis/service.py | 23 ++++++++++++++--------- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/engraphis/routes/vault.py b/engraphis/routes/vault.py index c6bfc5d..c502a1a 100644 --- a/engraphis/routes/vault.py +++ b/engraphis/routes/vault.py @@ -202,17 +202,22 @@ async def import_folder(req: FolderImportReq): ) real_path = os.path.realpath(os.path.expanduser(req.path)) comparable_path = os.path.normcase(real_path) - if not any( - comparable_path == os.path.normcase(root) - or comparable_path.startswith(os.path.normcase(root).rstrip(os.sep) + os.sep) - for root in allowed_roots - ): + safe_path = None + for root in allowed_roots: + comparable_root = os.path.normcase(root) + if comparable_path == comparable_root: + safe_path = comparable_root + break + root_prefix = comparable_root.rstrip(os.sep) + os.sep + if comparable_path.startswith(root_prefix): + safe_path = comparable_path + break + if safe_path is None: raise HTTPException(403, "Import path must be under an allowed root (home directory or ENGRAPHIS_IMPORT_ROOTS)") - folder = Path(real_path) - # real_path was canonicalized and checked against the configured roots above. - if not folder.exists(): # codeql[py/path-injection] + folder = Path(safe_path) + if not folder.exists(): raise HTTPException(404, f"Path not found: {req.path}") - if not folder.is_dir(): # codeql[py/path-injection] + if not folder.is_dir(): raise HTTPException(400, f"Not a directory: {req.path}") ns = req.namespace diff --git a/engraphis/service.py b/engraphis/service.py index a45333d..73d542c 100644 --- a/engraphis/service.py +++ b/engraphis/service.py @@ -433,19 +433,24 @@ def _resolve_import_root(raw_path: str) -> Path: ) real_path = os.path.realpath(os.path.expanduser(raw_path)) comparable_path = os.path.normcase(real_path) - if not any( - comparable_path == os.path.normcase(root) - or comparable_path.startswith(os.path.normcase(root).rstrip(os.sep) + os.sep) - for root in allowed_roots - ): + safe_path = None + for root in allowed_roots: + comparable_root = os.path.normcase(root) + if comparable_path == comparable_root: + safe_path = comparable_root + break + root_prefix = comparable_root.rstrip(os.sep) + os.sep + if comparable_path.startswith(root_prefix): + safe_path = comparable_path + break + if safe_path is None: raise ValidationError( "import path must be under an allowed root (your home directory, or " "ENGRAPHIS_IMPORT_ROOTS)") - folder = Path(real_path) - # real_path was canonicalized and checked against the configured roots above. - if not folder.exists(): # codeql[py/path-injection] + folder = Path(safe_path) + if not folder.exists(): raise ValidationError(f"path not found: {raw_path}") - if not folder.is_dir(): # codeql[py/path-injection] + if not folder.is_dir(): raise ValidationError(f"not a directory: {raw_path}") return folder