diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ebdc236..bbd7cfb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -176,6 +176,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security +- **Tarball restore path traversal, latent SQL injection, DNS-rebinding TOCTOU in the shared SSRF guard, stored XSS in report generation, and unvalidated SPARQL object IRIs in AnzoStore** (#1079) by @KaifAhmad1 + - `semantica backup restore`'s tar extraction (`cli.py`) stripped only the literal `semantica-backup/` prefix and called `tar.extract()` with no path-containment check, no symlink/hardlink validation, and (on Python <3.12) no extraction filter — a crafted archive member (`../../`, or a symlink pointing outside the restore root) could write arbitrary files above the restore directory. Every member is now validated for resolved-path containment before extraction, symlink/hardlink targets are rejected both lexically (absolute path, `..` segments) and by resolution, and `filter="data"` is applied on Python ≥3.12 + - `DataExporter.export_table_data()` (`db_ingestor.py`) was missing the `text` import from `sqlalchemy` — a `NameError` that made the method non-functional, but latently: the query it built from raw f-string interpolation of `table_name`/`schema`/`where`/`order_by` was already injectable, so fixing the import alone (without also fixing the injection) would have silently armed it. Both are fixed together: the import is restored, `table_name`/`schema` are now validated against a strict identifier allowlist, and `where`/`order_by` are checked against a blocklist (statement separators, comments, UNION, DDL/DML keywords, time-based blind-injection primitives, schema-enumeration terms). This is a blocklist, not a grammar — it closes the concrete UNION-exfiltration path and common injection primitives, but a boolean-blind subquery using none of the blocked keywords could still get through; `where`/`order_by` must be treated as trusted/operator input, not exposed to untrusted end users, and the docstrings now say so explicitly + - `request_with_ssrf_guard()` (`ssrf.py`) validated a hostname's resolved IPs, then let the underlying HTTP client re-resolve the same hostname independently at connect time — a low-TTL or DNS-rebinding answer could differ between the two lookups, so a hostname that validated as public could still connect to a private/internal address. Ported the IP-pinning pattern already used by `explorer/routes/ontology.py`'s `_make_pinned_session` into the shared ingest guard: the one resolution that decides accept/reject is now also the one the connection is pinned to, via a custom `HTTPAdapter` that presents the real hostname over TLS SNI / Host header while connecting only to the validated IPs. Also closes the RFC 6598 Carrier-Grade NAT gap noted as a known limitation in #905/#868: `100.64.0.0/10` is now in `BLOCKED_NETWORKS` + - `ReportGenerator._generate_html()` (`export/report_generator.py`) f-string-interpolated report title/summary/metrics into HTML with no escaping — an ingested entity or document whose content flowed into a report (e.g. ``) executed as stored XSS when the report was opened. All interpolated values are now `html.escape()`d + - `AnzoStore._format_object_for_sparql()` (`triplet_store/anzo_store.py`) validated the subject/predicate of a triplet via `sparql_escaping.validate_uri()` before interpolating them into a SPARQL `INSERT DATA` clause, but delegated the **object** position to a separate formatter that wrapped it as `<{obj}>` without the same validation — an object value containing `>`/`}`/`{`/`"` could close the intended `<...>` token early and inject additional SPARQL Update operations. The Blazegraph/RDF4J backends were hardened for the equivalent gap previously; Anzo's object position now goes through the same `validate_uri()` check + - Also hardened in the same pass: Apache AGE's `create_index()` `index_type` parameter is now allowlisted (was interpolated raw into a `USING` clause); Neo4j's `limit` is now explicitly validated (raises `ValidationError` for non-integer input instead of falling through to a generic `ProcessingError`); the `ffprobe` metadata-extraction subprocess call is guarded against a filename starting with `-` being parsed as an option; the MCP server no longer echoes raw exception text to JSON-RPC clients, logging full details server-side and returning a generic message plus the exception class name instead + - **Fixed during review** (@KaifAhmad1): the SSRF IP-pinning change introduced a connection-pool leak of its own — `requests.Session.mount()` silently drops whatever adapter it replaces without closing it, so a multi-hop redirect chain on a reused session leaked one pooled connection per hop. Pinned adapters are now tagged and explicitly closed before being replaced, both per-hop and on final restore + - **Fixed during review** (@KaifAhmad1): mounting a pinned adapter and setting a Host header on a caller-supplied `Session` is not inherently thread-safe — two guarded calls sharing the same session from different threads could interleave their mount/restore cycles. Added a per-session lock (`_get_session_lock`) so concurrent guarded calls on the same session now serialize instead of racing; verified with a two-thread test showing correct serialization and zero cross-contamination of per-request Host headers + - **Fixed during automated PR review** (Qodo): `export_table_data()`'s new identifier/fragment validation raised `ValidationError` from inside a `try` whose blanket `except Exception` re-wrapped it as `ProcessingError`, masking the distinction between "bad input" and "the export itself failed" that callers rely on elsewhere in this module. Added the `except ValidationError: raise` guard already used by its sibling methods + - **Fixed during automated PR review** (Qodo): on a hop where IP pinning doesn't apply (`allow_private_ips=True`), `_apply_connection_pin()` unconditionally popped the session's `Host` header instead of restoring whatever it was before pinning touched it — a caller-supplied session carrying its own legitimate `Host` override (e.g. fronting a private endpoint under a different name) had that override silently dropped for the in-flight request, only reappearing afterward via the outer `finally` restore. It now restores the session's own pre-call header state (set back if present, popped only if it was truly absent) instead of always popping + - **Fixed during automated PR review** (Qodo): the `where`/`order_by` blocklist matched keywords/punctuation inside properly quoted string literals and identifiers too, so legitimate data like `status = 'union'` or `name = 'a--b'` was rejected as if it were SQL syntax. The blocklist now runs against a copy with quoted-literal contents masked out (`_mask_sql_literals`) — a malformed/unterminated quote sequence doesn't match the masking pattern and is left fully exposed to the blocklist, so this closes false positives without opening a masking-based bypass; the fragment actually used in the query is unchanged + - Re-ran each finding's proof-of-concept (or an equivalent adversarial test) against the fix and confirmed it is blocked: tar path/symlink traversal (both lexical and resolved-path forms), SQL UNION exfiltration and identifier breakout, DNS-rebinding TOCTOU (including under a configured `HTTP_PROXY`, which the pinning adapter also rejects outright since a proxy would resolve DNS itself), stored XSS, and the AnzoStore SPARQL injection + - `pytest tests/ingest/`: 266 passed, 2 skipped (10 pre-existing failures unrelated to this change — identical failure set confirmed on unmodified `main`); full regression sweep across `graph_store`, `export`, `triplet_store`, `parse`, and backup/restore: 313 passed + - **`Authorization`/`Proxy-Authorization` credentials could leak to a different origin across HTTP redirects, and several ingest paths bypassed the shared SSRF/redirect guard entirely** (#1067, closes #947) by @Sameer6305, reviewed by @KaifAhmad1 - `request_with_ssrf_guard()` previously only stripped sensitive headers from per-request `kwargs["headers"]` on a cross-origin redirect; session-level `Authorization`/`Proxy-Authorization` headers, `session.auth`, and `session.trust_env` (`.netrc` lookup) could all still resurrect credentials on the hop to a foreign origin. All five credential sources are now stripped case-insensitively, kept stripped for the remainder of a multi-hop redirect chain (no resurrection even if a later hop returns to the original host), and unconditionally restored via `finally` — including on exceptions and redirect-limit errors - `MCPClient._send_request_http()` and `PublicAPIIngestor.detect_public_api()`/`ingest_public_api()` called `httpx.post()`/`requests.post()`/`session.request()` directly, bypassing `request_with_ssrf_guard()` entirely. Both now route through the shared guard, including when `validate_no_auth=False` diff --git a/README.md b/README.md index 8b89fd5f..49a4c57c 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,15 @@ Semantica -semantica-agi%2Fsemantica | Trendshift +
+ + semantica-agi/semantica | Trendshift + + + + semantica-agi/semantica | Trendshift + +
### Graph-Native Infrastructure for Context and Accountable AI Systems @@ -295,17 +303,10 @@ graph.add_causal_relationship(d1, d2, relationship_type="CAUSED") prov.track_entity("patient_P4821", source="ehr/medication_orders_2024.json", metadata={"extractor": "NamedEntityRecognizer"}) -# Export W3C PROV-O for regulator submission - RDFExporter expects -# {"entities": [...], "relationships": [...]}, so map ContextGraph.to_dict()'s -# {"nodes": [...], "edges": [...]} shape onto it first -graph_dict = graph.to_dict() -kg = { - "entities": [{"id": n["id"], "type": n["type"], "text": n["content"]} for n in graph_dict["nodes"]], - "relationships": [ - {"source_id": e["source"], "target_id": e["target"], "type": e["type"]} - for e in graph_dict["edges"] - ], -} +# Export W3C PROV-O for regulator submission - to_kg_dict() is the official +# adapter that emits the {"entities": [...], "relationships": [...]} / +# source_id shape RDFExporter expects, so no manual field mapping is needed +kg = graph.to_kg_dict() RDFExporter().export(kg, "audit_trail.ttl", format="turtle") ``` @@ -879,20 +880,14 @@ fact = BiTemporalFact( recorded_at=datetime(2024, 3, 5), ) -# Query facts valid within a time window - query_time_range() expects -# {"relationships": [...]} with source_id/target_id keys, which differs from -# ContextGraph.to_dict()'s {"nodes", "edges"} shape, so map it first -graph_dict = graph.to_dict() -kg_relationships = { - "relationships": [ - {**e, "source_id": e["source"], "target_id": e["target"]} - for e in graph_dict["edges"] - ] -} +# Query facts valid within a time window - to_kg_dict() is the official +# adapter that emits {"entities", "relationships"} with source_id/target_id +# keys, the shape query_time_range() expects (no manual mapping required) +kg = graph.to_kg_dict() tq = TemporalGraphQuery() facts_in_window = tq.query_time_range( - kg_relationships, query="valid_facts", start_time="2024-01-01", end_time="2024-12-31" + kg, query="valid_facts", start_time="2024-01-01", end_time="2024-12-31" ) # Normalize natural language temporal expressions - returns a (start, end) range diff --git a/mcp/server.py b/mcp/server.py index 3cd935be..bf84dd24 100644 --- a/mcp/server.py +++ b/mcp/server.py @@ -93,7 +93,14 @@ def _handle_tools_call(req_id: Any, params: dict) -> dict: result = tool["_handler"](args) except Exception as exc: log.exception("Tool %s raised an exception", name) - return _err(req_id, _INTERNAL_ERROR, str(exc)) + # The exception's class name (e.g. "ValidationError", "TimeoutError") + # is safe to surface — unlike str(exc), it never carries paths, + # connection strings, or other internal detail — and lets the + # client distinguish failure kinds without a full message. + return _err( + req_id, _INTERNAL_ERROR, + f"Tool '{name}' failed ({type(exc).__name__}). See server logs for details.", + ) # MCP spec: content must be a list of content items return _ok(req_id, { @@ -171,7 +178,10 @@ class SemanticaMCPServer: log.exception("Unhandled error in method %s", method) if req_id is None: return None - return _err(req_id, _INTERNAL_ERROR, str(exc)) + return _err( + req_id, _INTERNAL_ERROR, + f"Method '{method}' failed ({type(exc).__name__}). See server logs for details.", + ) # ------------------------------------------------------------------ def run(self) -> None: diff --git a/semantica/cli.py b/semantica/cli.py index 7b944dc6..403971e0 100644 --- a/semantica/cli.py +++ b/semantica/cli.py @@ -20,7 +20,7 @@ if sys.platform == "win32": sys.stderr.reconfigure(encoding="utf-8", errors="replace") from dataclasses import asdict, dataclass, field, is_dataclass -from pathlib import Path +from pathlib import Path, PurePosixPath, PureWindowsPath from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Sequence, Tuple import yaml @@ -3933,15 +3933,68 @@ def backup_restore(cli_ctx: CLIContext, source: str, local_dry: bool) -> None: try: if _tf.is_tarfile(str(work_path)): - restore_root = Path.cwd() + restore_root = Path.cwd().resolve() with _tf.open(str(work_path), "r:*") as tar: # Dry-run listing was already handled above; extract now for member in tar.getmembers(): # Strip the leading "semantica-backup/" prefix member.name = member.name.replace("semantica-backup/", "", 1) - if member.name: - tar.extract(member, path=str(restore_root)) - console.print(f" restored: {member.name}") + if not member.name: + continue + + # Reject members whose resolved path escapes the + # restore root (path traversal / absolute paths), + # regardless of the "semantica-backup/" prefix. + member_path = (restore_root / member.name).resolve() + try: + member_path.relative_to(restore_root) + except ValueError: + raise click.ClickException( + f"Refusing to restore '{member.name}': " + "path escapes the restore directory." + ) + + # Reject symlink/hardlink members whose target + # escapes the restore root. Checked two ways: + # lexically (linkname itself, so an absolute path or + # a literal ".." segment is rejected outright, with + # no dependence on what else does or doesn't already + # exist on disk) and by resolution (catches any + # remaining traversal the lexical check misses). + if member.issym() or member.islnk(): + linkname = member.linkname or "" + linkname_parts = PurePosixPath( + linkname.replace("\\", "/") + ).parts + if ( + not linkname + or os.path.isabs(linkname) + or PureWindowsPath(linkname).is_absolute() + or ".." in linkname_parts + ): + raise click.ClickException( + f"Refusing to restore '{member.name}': " + "link target is absolute or traverses " + "out of the archive." + ) + link_target = ( + member_path.parent / linkname + ).resolve() + try: + link_target.relative_to(restore_root) + except ValueError: + raise click.ClickException( + f"Refusing to restore '{member.name}': " + "link target escapes the restore directory." + ) + + extract_kwargs: Dict[str, Any] = {"path": str(restore_root)} + if hasattr(_tf, "data_filter"): + # Python >=3.12: also reject device files, and + # further harden the traversal/ownership checks. + extract_kwargs["filter"] = "data" + tar.extract(member, **extract_kwargs) + console.print(f" restored: {member.name}") elif src.is_dir(): restore_root = Path.cwd() for f in src.rglob("*"): diff --git a/semantica/context/context_graph.py b/semantica/context/context_graph.py index ad6ecf3f..ac3f134f 100644 --- a/semantica/context/context_graph.py +++ b/semantica/context/context_graph.py @@ -2449,6 +2449,97 @@ class ContextGraph: }, } + def to_kg_dict(self, entities_only: bool = False) -> Dict[str, Any]: + """Export graph in the canonical knowledge-graph shape. + + This is the official adapter that converts the ContextGraph's internal + ``{"nodes", "edges"}`` / ``source`` representation into the + ``{"entities", "relationships"}`` / ``source_id`` shape expected by + downstream consumers such as + :class:`~semantica.export.rdf_exporter.RDFExporter` and + :meth:`~semantica.kg.temporal_query.TemporalGraphQuery.query_time_range`. + + Users no longer need to hand-map field names between APIs. + + Args: + entities_only: If True, only nodes whose ``node_type`` is + ``"entity"`` are exported as entities. When False (default), + every node is exported. Relationships whose endpoints are not + in the exported entity set are dropped to avoid dangling + references in downstream consumers. + + Returns: + dict: A knowledge-graph dictionary with: + - ``entities``: list of ``{"id", "text", "type", "properties", + "metadata"}`` (plus ``valid_from`` / ``valid_until`` when set) + - ``relationships``: list of ``{"source_id", "target_id", + "type", "weight", "id", "familyId"}`` (plus ``metadata`` and + ``valid_from`` / ``valid_until`` when set) + - ``statistics``: ``{"entity_count", "relationship_count"}`` + """ + with self._lock: + entities_out = [] + for n in self.nodes.values(): + if entities_only and n.node_type != "entity": + continue + # Normalize the entity id to ``str`` so it matches ContextEdge, + # which coerces its endpoints to ``str`` in ``__post_init__``. + # Without this, non-string node ids (e.g. numeric ids loaded via + # ``from_dict``) would fail the ``valid_ids`` membership check + # below and silently drop otherwise-valid relationships. + entity_id = str(n.node_id) + entity: Dict[str, Any] = { + "id": entity_id, + "text": n.content, + "type": n.node_type, + # ``properties`` / ``metadata`` may be ``None`` when a node + # was loaded from JSON containing an explicit ``null``; + # guard with ``or {}`` so ``dict(...)`` never raises. + "properties": dict(n.properties or {}), + "metadata": dict(n.metadata or {}), + } + if n.valid_from is not None: + entity["valid_from"] = n.valid_from + if n.valid_until is not None: + entity["valid_until"] = n.valid_until + entities_out.append(entity) + + # When only entity nodes are exported, drop relationships whose + # endpoints were filtered out so downstream consumers never see a + # source_id/target_id that is absent from ``entities``. + valid_ids = {e["id"] for e in entities_out} if entities_only else None + + relationships_out = [] + for e in self.edges: + if valid_ids is not None and ( + e.source_id not in valid_ids or e.target_id not in valid_ids + ): + continue + rel: Dict[str, Any] = { + "id": e.edge_id, + "familyId": e.family_id or e.edge_id, + "source_id": e.source_id, + "target_id": e.target_id, + "type": e.edge_type, + "weight": e.weight, + } + if e.metadata: + rel["metadata"] = dict(e.metadata) + if e.valid_from is not None: + rel["valid_from"] = e.valid_from + if e.valid_until is not None: + rel["valid_until"] = e.valid_until + relationships_out.append(rel) + + return { + "entities": entities_out, + "relationships": relationships_out, + "statistics": { + "entity_count": len(entities_out), + "relationship_count": len(relationships_out), + }, + } + def from_dict(self, graph_dict: Dict[str, Any]) -> None: """Load graph from dictionary format.""" # Clear existing graph diff --git a/semantica/export/report_generator.py b/semantica/export/report_generator.py index d4b67920..fcee11a1 100644 --- a/semantica/export/report_generator.py +++ b/semantica/export/report_generator.py @@ -23,6 +23,7 @@ Author: Semantica Contributors License: MIT """ +import html import json from datetime import datetime from pathlib import Path @@ -362,7 +363,7 @@ class ReportGenerator: ' ' ) title = data.get("title", "Report") - lines.append(f" {title}") + lines.append(f" {html.escape(str(title))}") lines.append("