Add a character-bigram overlap-coefficient fallback to _calculate_decision_content_similarity so CJK scenarios (no whitespace tokenization) can match recorded decisions; the previous whitespace Jaccard was always 0 for CJK.
Rebuild _decisions/_decision_index/_entity_index/_temporal_index from persisted decision nodes at the end of load_from_file, otherwise find_precedents_by_scenario and decision_count break after a reload since save_to_file does not serialize the internal decision indexes.
Co-Authored-By: Claude <noreply@anthropic.com>
gensim (core dependency) has no prebuilt cp314 wheel, and the slim base image lacks gcc to build from source, so 'pip install .[explorer]' fails on python:3.14-slim. Pin to python:3.13-slim (still satisfies requires-python>=3.8) until gensim ships a cp314 wheel.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(context): clarify get_node_property not-found contract (#877)
Add default= param to get_node_property and get_node_attributes so
callers can distinguish node-missing from property-missing using a
sentinel. Fix add_node_attribute calling mutation_callback outside
the lock. Tests added for all cases.
* fix(context): address Qodo review findings (#877)
* fix(context): wrap add_node_attribute mutation_callback in try/except (#877)
The PR claimed to move the callback back inside `with self._lock`, but
the diff only dropped a stray blank line -- the call stayed outside the
lock, unchanged. That's actually correct: self._lock is an RLock, and
_add_internal_node/_add_internal_edge deliberately release the lock
before invoking the callback too, so a slow/misbehaving callback never
holds up other threads. The real gap was that, unlike those two
siblings, this call site didn't catch exceptions from the callback.
Wrapped it the same way, with a regression test.
---------
Co-authored-by: Mohd Kaif <98801504+KaifAhmad1@users.noreply.github.com>
Co-authored-by: KaifAhmad1 <kaifahmad087@gmail.com>
* fix(export): log DistanceExporter metric computation failures instead of swallowing them
The four private metric helpers in DistanceExporter (_betweenness,
_hop_distance, _weighted_distance, _semantic_similarity) each catch a bare
Exception and return None/{} with no signal. That makes an exported None
indistinguishable from a legitimate "no path exists" result, corrupting
downstream CSV/JSONL/DataFrame exports with no way to tell a real gap from a
swallowed error.
Log each caught exception at warning level with the offending source/target
before returning the existing sentinel. The exported row shape and values are
unchanged; only the observability of the failure changes.
Fixes#874
* fix(export): route DistanceExporter warnings through the semantica logger tree
get_logger(__name__) doubled the semantica. prefix (__name__ is already
semantica.export.distance_exporter), so the warnings this PR adds landed on
semantica.semantica.export.distance_exporter, a branch setup_logging() never
configures and does not reach the app's log handler. Also reworded the three
except-Exception log messages: they said "recording as no path", which
overclaims what a generic exception means.
Addresses review feedback from @KaifAhmad1 on #879.
* docs(changelog): add DistanceExporter logging fix entry
---------
Co-authored-by: Mohd Kaif <98801504+KaifAhmad1@users.noreply.github.com>
Co-authored-by: KaifAhmad1 <kaifahmad087@gmail.com>
* fix(kg): align GraphBuilder raw-text extraction defaults with the documented contract
_extract_from_text() defaulted ner_method, relation_method and
triplet_method to "llm" and ran relation extraction unconditionally,
contradicting the build() docstring ("ml"/"pattern"/False) and the
standalone extractor defaults. Any raw-text build() therefore required a
provider, an API key, and network access without saying so.
Defaults are now ml/pattern/pattern with extract_relations=False. LLM
extraction is unchanged and now opt-in via explicit kwargs.
Also documents relation_method and extract_triplets, which the docstring
never listed, and drops the stale "Default to LLM methods as per
requirement" comment.
Closes#930
* perf(kg): reuse extractors across texts instead of rebuilding per source
Addresses review feedback on #941. NERExtractor.__init__ loads its spaCy
model eagerly when the method includes "ml", so switching the default
from "llm" to "ml" made _extract_from_text() reload the model once per
source in a multi-document build.
Extractors are now cached per (kind, method) on the builder. Adds tests
asserting single construction across repeated texts, that distinct
methods still get distinct extractors, and that the default path runs
end to end without any provider call.
* fix(kg): keep fallback method lists working with the extractor cache
The extractor cache keyed directly on `method`, but all three extractors
accept a list for fallback ordering (e.g. ner_method=["pattern", "ml"]),
so a list argument raised TypeError: unhashable type: 'list' before
extraction started. Lists are now converted to tuples for the cache key
only; the extractor still receives the original value.
Also seeds _extraction_stats in __init__. It was previously created only
in build(), so calling _extract_from_text() directly — as the report's
repro does — raised an AttributeError that the broad except swallowed and
logged as "Entity extraction failed".
Adds coverage for list methods on all three extractors, cache reuse for
equal lists, and distinct entries for different orderings.
* fix(kg): forward extracted relations into triplet extraction
_extract_from_text() passed only entities= to extract_triplets(), so
TripletExtractor re-derived relations itself whenever relations is None,
using a method taken from triplet_method rather than relation_method.
That duplicated work and could yield triplets inconsistent with the
relations already extracted.
relations is now initialized to None, holds the extracted list when
extract_relations=True succeeds, and is forwarded to extract_triplets().
When extraction is disabled or fails, None is passed and
TripletExtractor's existing self-derivation is unchanged.
Folded in at maintainer request rather than tracked as #944.
* docs(changelog): note that #878 documented the LLM defaults before this landed
#878 merged while this was in review and resolved the same code/docstring
mismatch in the opposite direction. Records that #930's decision makes
the code the side that changes, and that #878's docstring formatting is
retained.
* docs(kg): document GraphBuilder public methods
* test(kg): skip module-level doctest to fix suite run
* docs(kg): restore GraphBuilder option documentation
* docs(kg): document default values for build() extraction options
extract_relations, extract_triplets, ner_method, relation_method, and
triplet_method all have concrete defaults in _extract_from_text(), but
the build() docstring only stated a default for extract, inconsistent
with CONTRIBUTING.md's docstring convention of noting parameter
defaults.
* docs: add changelog entry for GraphBuilder docstrings (#878, #876)
---------
Co-authored-by: Zohaib Hassnain <109234410+ZohaibHassan16@users.noreply.github.com>
Co-authored-by: Sameer Kadam <sskadam6305@gmail.com>
Co-authored-by: Mohd Kaif <98801504+KaifAhmad1@users.noreply.github.com>
Co-authored-by: KaifAhmad1 <kaifahmad087@gmail.com>
* fix(security): sanitize node_id in Content-Disposition to prevent header injection (CWE-113)
* fix(security): cap link prediction at 10k nodes with semaphore to prevent OOM DoS (CWE-770)
* fix(security): sanitize imported node IDs to prevent stored header injection chain (CWE-20)
* test(security): add self-contained PoC runner with real measured output
* test(security): add regression tests for header injection, DoS cap, import sanitization
* fix(security): comprehensive fix for header injection, DoS, and import ID sanitization
* fix: move semaphore to wrap entire data-load+scoring region, use node-specific edge queries (Qodo #2, #3)
* fix: sanitize edge source/target IDs to match sanitized node IDs (Qodo #4)
* fix: scope 999_999 check to predict_links function via AST (Qodo #1)
* fix: add explicit None guard to _sanitize_import_node_id
* fix(security): close import-sanitizer bypass, enforce link-prediction cap before the expensive scan
Follow-up to the fixes in this PR, found in review:
- export_import.py's "properties" in raw_node fast path stored the id
verbatim, completely skipping _sanitize_import_node_id() -- a node
payload of {"id": "<crlf>", "properties": {}} (the shape this app's
own /api/export produces) bypassed the VULN-3 fix entirely. That
branch now sanitizes id before storing.
- The link-prediction 10k-node cap checked `total` only after calling
session.get_nodes()/get_edges(), which normalize the graph's entire
matching set before applying `limit` -- so the DoS guard ran after
the expensive work it exists to prevent had already happened, on
every request regardless of graph size. Added
GraphSession.get_raw_counts(), an O(1) check against the raw
len(graph.nodes)/len(graph.edges), and moved the size check ahead of
the normalizing calls (also added an edge-count cap).
- 5 of the existing regression tests asserted that literal words like
"Set-Cookie"/"Content-Type" disappear from the sanitized value -- the
sanitizer strips \r\n\x00"\ , not letters, so those assertions failed
against this PR's own fix as submitted. Corrected to assert on the
actual security property (no \r/\n survives), and added end-to-end
tests that exercise the real /api/import -> /api/provenance/report
route chain so the properties-key bypass has regression coverage.
Full explorer suite: 241 passed. tests/test_security_regression_pr2.py: 30 passed.
---------
Co-authored-by: Zohaib Hassnain <109234410+ZohaibHassan16@users.noreply.github.com>
Co-authored-by: Mohd Kaif <98801504+KaifAhmad1@users.noreply.github.com>
Co-authored-by: KaifAhmad1 <kaifahmad087@gmail.com>
* fix(context): take the lock in ContextGraph.to_dict()
to_dict() iterated self.nodes.values() and self.edges without holding
self._lock, so a concurrent writer raised "RuntimeError: dictionary changed
size during iteration". It was the only reader on the class that did not take
the lock -- stats(), density(), find_nodes(), find_edges(), get_neighbors(),
get_nodes_by_label(), state_at() and save_to_file() all hold it.
Commit 1d1ae398 introduced the RLock and added 26 "with self._lock:" blocks;
to_dict already existed and was not among them. save_to_file is safe only
incidentally -- it holds the lock and builds its payload inline rather than
delegating to to_dict, so it never reaches the unguarded loops.
Beyond the RuntimeError, the unguarded body could also return a torn snapshot:
the statistics block reads len(self.nodes)/len(self.edges) after building the
node and edge lists, so a write landing in between yields counts that
contradict the payload they describe.
self._lock is an RLock, so this composes with the callers that already hold it
(build_from_conversation and build_from_documents both return self.to_dict()
from inside a locked block). Neither external caller -- agent_context's
_capture_checkpoint_state nor triplet_store's knowledge-graph conversion --
defines a lock of its own, so there is no ordering inversion.
Add tests/context/test_context_graph_thread_safety.py: a deterministic check
that to_dict() blocks while another thread holds _lock (no race window
needed), a reentrancy check, and three checks under concurrent writes covering
the RuntimeError, statistics/payload agreement, and duplicate node ids. Four
of the five fail against the unfixed method.
Closes#923
* test(context): make to_dict lock tests deterministic and hang-proof
Wait for the worker thread to actually start before asserting to_dict()
blocks on _lock, and run the reentrancy check in a joined worker so a
non-reentrant lock fails the test instead of hanging CI.
* test(context): assert worker threads actually stopped after timed joins
A join(timeout=...) on a daemon thread returns even if the thread is
still running, so a deadlock would leak a live thread into subsequent
tests instead of failing. Assert not is_alive() after each timed join.
---------
Co-authored-by: Pravit Ampapathini <pravitampapathini@Pravits-MacBook-Air-3.local>
Co-authored-by: Zohaib Hassnain <109234410+ZohaibHassan16@users.noreply.github.com>
* security(deps): bump fastapi floor to >=0.109.1 (PYSEC-2024-38)
The [explorer] extra declared fastapi>=0.100.0, which allows the
vulnerable 0.109.0 (PYSEC-2024-38, HTTP response splitting). Raise the
floor to 0.109.1, the patched release. One-line change, no functional
impact -- the 0.109.x API is stable and backward-compatible.
Fixes#869
* fix(deps): bump fastapi to >=0.109.2 and python-multipart to >=0.0.7 for PYSEC-2024-38
PYSEC-2024-38 (CVE-2024-24762 / GHSA-2jv5-9r88-3w3p) is a ReDoS in
python-multipart < 0.0.7: an attacker sends a crafted Content-Type header
that causes catastrophic backtracking in the multipart regex, stalling the
event loop and causing a DoS on any endpoint that parses form data.
The original PR bumped fastapi to >=0.109.1, but that version pins
starlette<0.36.0,>=0.35.0 and cannot install starlette 0.36.2+ (which
contains the fix via python-multipart>=0.0.7). FastAPI 0.109.2 is the
first version that pins starlette>=0.36.3 (verified against PyPI metadata).
Two changes are necessary:
1. fastapi>=0.109.1 -> fastapi>=0.109.2: ensures starlette>=0.36.3 is
installed as a transitive dependency, which in turn pulls the fixed
python-multipart>=0.0.7.
2. python-multipart>=0.0.6 -> python-multipart>=0.0.7: closes the direct
dependency path. python-multipart is listed explicitly in the explorer
extra, so without this floor a resolver could still install 0.0.6 and
leave the vulnerability present even with the fastapi bump.
The fix targets only the 'explorer' optional dependency group, which is
the only code surface where FastAPI and form-data parsing are used.
No functional API changes between 0.109.1 and 0.109.2; 239 Explorer tests
pass without modification.
* ci(security): gate pip-audit on explorer-extra dependency PRs, add changelog entry for PYSEC-2024-38
The Security workflow's pip-audit job ran weekly against a bare Python
env with none of Semantica's optional extras installed, and always
continue-on-error'd -- it would never have flagged the vulnerable
fastapi/python-multipart floors this PR fixes, or the first attempt at
the fix that left python-multipart>=0.0.6 in place. security-scan.yml's
Safety check has the same blind spot (only installs [llm-litellm]).
pip-audit now also runs on pull_request when pyproject.toml changes,
installs semantica[all] so it can actually see extras like [explorer],
and fails the build on findings for that trigger. Scheduled/dispatch
runs stay non-blocking pending a full pass over the [all] tree.
Also documents the fix (#871, closes#869) in CHANGELOG.md, including
the correction made during review after the original fastapi-only bump
turned out not to close the vulnerability.
* fix(deps): raise setuptools floor to >=83.0.0 (CVE-2026-59890), harden audit env
The new pull_request pip-audit gate (previous commit) caught this on its
first run: pip install -e ".[all]" resolved setuptools==79.0.1, vulnerable
to CVE-2026-59890 / GHSA-h35f-9h28-mq5c / PYSEC-2026-3447 (Unicode
normalization lets a MANIFEST.in exclude/prune pattern be bypassed on
macOS APFS/HFS+, leaking excluded files into a built sdist). Fixed in
setuptools 83.0.0.
[build-system] requires had the same too-permissive floor this whole PR
is about (setuptools>=61.0). Raised to >=83.0.0. Also upgrade pip/
setuptools explicitly in the Security workflow before running pip-audit,
since [build-system] requires only governs isolated build environments,
not the ambient one actions/setup-python provisions and pip-audit scans.
---------
Co-authored-by: Sameer Kadam <sskadam6305@gmail.com>
Co-authored-by: KaifAhmad1 <kaifahmad087@gmail.com>
semantica/mcp_server/__init__.py was fixed to stop hardcoding 0.4.0,
but the separate top-level mcp/ package (run via `python -m
mcp.server`, documented in mcp/__init__.py as a supported way to
configure Claude Desktop/Windsurf/etc. from a source checkout) still
hardcoded 0.4.0 in three places: mcp/__init__.py, mcp/server.py, and
mcp/resources/registry.py.
Reuses semantica.__version__ directly, matching the pattern just
adopted in semantica/mcp_server/__init__.py, so both implementations
stay in sync with the package version going forward.
The previous implementation used importlib.metadata.version('semantica') as
the primary version source with a PackageNotFoundError fallback to
semantica.__version__. This caused two of the three new regression tests to
fail in editable/development installs, where dist-info (egg-info) is written
at install time and is not automatically updated on subsequent version bumps.
In this repo, pyproject.toml declares version as a static field (not dynamic),
and semantica/__init__.py maintains __version__ in sync with it by convention.
semantica.__version__ is therefore the authoritative source of truth and is
always present whenever semantica.mcp_server is importable -- the importlib
.metadata indirection adds no value and can return a stale value.
Changes:
- semantica/mcp_server/__init__.py: replace the importlib.metadata try/except
block with a direct 'from semantica import __version__ as _SEMANTICA_VERSION'
- tests/test_mcp_server_version.py: rewrite tests to assert both MCP version
surfaces (SERVER_INFO['version'] and semantica://schema/info) against
semantica.__version__ as the single ground truth; add 0.4.0 regression
canaries and a cross-surface consistency assertion; remove the mirrored
importlib.metadata resolution that masked the staleness problem
The root-level mcp/ directory (a separate unpublished companion implementation
not included in the built package) is intentionally left unchanged -- it is
outside the scope of issue #863 which targets the semantica-mcp entry point.
- pinecone_store: call self.index.describe_index_stats() instead of the
nonexistent self.describe_index_stats(), and use a unit query vector
instead of an all-zero vector so filter_by_metadata() works on
cosine-metric indexes (the library's own default)
- pgvector_store: apply the existing lowercase true/false bool handling
to the list-filter branch too, and use the jsonb ?| operator so
list-valued metadata fields match on intersection instead of being
compared as a single JSON-text blob
- sqlite_vec_store: use json_each() with a json_type guard so list-valued
metadata fields match on intersection, mirroring the in-memory
backend's set-intersection semantics
- faiss_store: filter_by_metadata(limit=0) now returns [] instead of one
result
- milvus_store: reject NaN/Infinity filter values up front with a clear
ValidationError instead of building an invalid expression that gets
silently swallowed
- update the #848 FAISS NotImplementedError test to reflect that FAISS
now implements real filter_by_metadata() (this PR's whole point)
- add regression tests for each fix; sqlite tests run against the real
sqlite-vec extension
* fix(pipeline): resolve broken import and missing run() in PipelineWithProvenance
Fix two bugs in pipeline_provenance.py:
1. Wrong import path: `from .pipeline import Pipeline` fails because
`semantica/pipeline/pipeline.py` does not exist. Pipeline lives in
`pipeline_builder.py`. Fixed to `from .pipeline_builder import Pipeline`.
2. Pipeline dataclass has no run() method. PipelineWithProvenance.run()
now delegates to ExecutionEngine.execute_pipeline(), which is the
intended execution path for built pipelines.
Additional changes:
- Constructor now accepts a built Pipeline instance (breaking the previous
unusable API that tried to instantiate a dataclass with **config).
- Replace deprecated datetime.utcnow() with datetime.now(timezone.utc).
- Add test suite covering import, instantiation, execution, attribute
delegation, and provenance graceful degradation.
Fixes#858
* test: address Qodo review findings
- Remove redundant test_import_succeeds (module-level import already
guards against import regression at collection time).
- Fix test_provenance_disabled_when_import_fails to deterministically
simulate ImportError via sys.modules patch and assert provenance is
actually toggled off (runner.provenance is False).
* fix(pipeline): update provenance callers for Pipeline API
---------
Co-authored-by: Sameer Kadam <sskadam6305@gmail.com>
Co-authored-by: Russell Jurney <russell.jurney@gmail.com>
CORSMiddleware doesn't cover WebSocket handshakes at all (Starlette's CORS
support only wraps HTTP), so under SEMANTICA_ALLOW_ANONYMOUS=true --
the mode docker-compose.dev.yml ships -- is_valid_api_key's anonymous
bypass accepted a /ws/graph-updates connection from any origin. Loopback
binding isn't a boundary against a browser: any page the operator has
open can still reach ws://localhost:8000/ws/graph-updates directly, and
ConnectionManager.broadcast sends every graph_mutation to every
connected socket with no per-connection scoping. Combined with
/api/import accepting multipart/form-data (a CORS-safelisted content
type that skips preflight), a hostile page could write to the graph
over REST and read the result back over the unauthenticated WebSocket
-- demonstrated end-to-end in the report with a real client.
Not affected: any deployment with SEMANTICA_API_KEY configured -- the
handshake already rejects without a valid key in that mode. This is an
anonymous-mode-only, development-configuration exposure.
Fix: check the handshake's Origin header against
app.state.explorer_settings['allowed_origins'], the same list
CORSMiddleware already enforces for HTTP, before the key check. A
missing Origin (native/CLI clients, which never set the header --
only browsers do) is still allowed through, since the browser is the
only threat this closes.
4 new tests in test_explorer_auth.py: hostile Origin rejected under
anonymous mode; hostile Origin rejected even with a correct key
(Origin is checked before the key, so a leaked key alone can't
hijack the socket); an allowlisted Origin still connects under
anonymous mode; a missing Origin still connects under anonymous mode
(native clients keep working). Full explorer suite: 226 passed.
Co-authored-by: Sameer Kadam <sskadam6305@gmail.com>
Qodo's re-review confirmed the multi-IP fallback fix but kept the proxy
finding open: logging-and-falling-back when a proxy applies still let
the DNS-pinning protection be silently skipped under proxy
configuration, rather than enforcing a clear policy either way.
Implemented Qodo's preferred option: proxies are now disabled outright
for this SSRF-sensitive fetcher via session.trust_env = False, so
HTTP_PROXY/HTTPS_PROXY/NO_PROXY env vars are never consulted in the
first place (a configured proxy would perform its own DNS resolution
of the target host outside this process's control, reopening the
DNS check-then-use race pinning exists to close). The adapter also
keeps a fail-closed backstop: if a proxy is somehow still configured
despite trust_env=False (e.g. set explicitly by future code), it now
raises a clear 502 instead of silently connecting through the proxy
unpinned.
_validate_fetch_url's destination classification (blocking private/
internal targets) is unaffected either way — it runs before any of
this and doesn't depend on proxy configuration.
4 new tests: trust_env is disabled on every pinned session; an
HTTP_PROXY env var pointed at an address that would fail if contacted
is confirmed genuinely unused (real local-server fetch still succeeds
directly); and the fail-closed backstop actually raises when a proxy
is forced onto the session. Full explorer + triplet_store suite: 572
passed.
Four findings from PR #916's automated review, all addressed:
- CodeQL (HIGH): the test HTTPS server's SSLContext allowed TLSv1/TLSv1.1
by not setting a minimum version. Added
ssl_ctx.minimum_version = ssl.TLSVersion.TLSv1_2.
- github-code-quality: unused `cryptography` local in
_make_self_signed_cert — importorskip's return value was never used.
- Qodo (reliability): _validate_fetch_url() only returned the first
validated IP, and _make_pinned_session() pinned to just that one
address, so a fetch would fail outright if the first-returned A/AAAA
record happened to be unreachable even though a later one would work.
_validate_fetch_url() now returns every validated IP (deduplicated,
in resolution order); _make_pinned_session() takes the full list and
falls back through each one via a custom Connection._new_conn
override, matching the fallback behavior a normal DNS-resolving
connection would already get for free. Verified with a real test:
pin to an unreachable loopback address followed by a real one, confirm
the fetch still succeeds by falling back; and a real test confirming
it still raises (rather than silently re-resolving the hostname) when
every pinned address is unreachable.
- Qodo (security): when an HTTP(S) proxy applies, the adapter falls back
to the unpinned path rather than pinning. This is a real, but
architecturally unavoidable, limitation from the client side: for a
forward proxy, the *proxy* performs its own DNS resolution of the
target host on the application's behalf, a resolution this process
has no visibility into or control over — there's no client-side pin
that closes that race. _validate_fetch_url's destination
classification still fully applies either way; only the secondary
DNS-pinning hardening doesn't extend through a proxy. Added an info
log when this fallback path is taken so it's observable rather than
silent, and expanded the code comment to make the reasoning explicit
for the next reader/reviewer rather than looking like an oversight.
Tests: 3 new tests in test_ontology_dns_pinning.py (multi-IP fallback
success, all-unreachable failure, deduplicated multi-record resolution).
Full explorer + triplet_store suite: 569 passed.
Two follow-up hardening items flagged as secondary/deferred during
GHSA-8c7v-62gr-hj6g and GHSA-8vgg-8mr4-r236's fixes:
1. DNS check-then-use (TOCTOU) window in the ontology URL fetcher.
_validate_fetch_url() resolved and validated a hostname once, but
_fetch_url_sync() then let requests resolve the same hostname again
independently at connect time — a low-TTL or rebinding DNS answer
could differ between the two lookups, reopening the SSRF window the
validation exists to close.
_validate_fetch_url() now returns the validated IP, and a new
_make_pinned_session() builds a per-hop requests.Session whose
connection pool is pinned directly to that IP (bypassing DNS
resolution for the connection entirely), while explicitly restoring
the real hostname as the outgoing HTTP Host header and, for HTTPS,
the TLS SNI server_hostname/assert_hostname — so the connection
reaches the validated IP but still presents (and is verified
against) the real hostname's identity, keeping virtual hosting and
certificate validation correct.
Note: an earlier version of this fix set `_dns_host` post-construction
assuming it was decoupled from `host`, matching some other urllib3
releases; in the installed version (2.7.0), `host` is a property
that reads/writes `_dns_host` directly, so that approach silently
changed the Host header too. Verified with a real (non-mocked) local
HTTP server, a real local HTTPS server with a self-signed cert
(proving SNI/cert-hostname verification checks the real hostname,
not the pinned IP), and a negative control confirming a hostname/cert
mismatch is still correctly rejected — not silently bypassed.
2. Pre-wrapped object IRIs skipped full validation in
_format_object_for_sparql/_format_object_for_ntriples (Blazegraph,
RDF4J). A triplet object already wrapped in `<...>` only had its
inner content checked for a literal space or `>`, not run through
sparql_escaping.validate_uri() like the unwrapped-object branch —
flagged by automated review during GHSA-8vgg-8mr4-r236's fix. Both
branches now validate identically.
Tests: tests/explorer/test_ontology_dns_pinning.py (6 tests, including
2 real local-server end-to-end checks and 2 real-TLS checks with a
generated self-signed cert, gracefully skipped if `cryptography` isn't
installed); updated tests/explorer/test_ontology_ssrf.py for the new
per-hop session construction; 4 new tests in
tests/triplet_store/test_sparql_injection.py for the object-IRI fix.
Full explorer + triplet_store suite: 566 passed.
Two follow-up fixes to the initial ReDoS patch (CodeQL py/polynomial-redos
#1897), raised during code review:
--- Fix 1: _PREFIX_DECL regression — inline prologues and CRLF (#review-1) ---
The first ReDoS fix replaced the ambiguous trailing \s* with [ \t]*(?:\n|$),
but that introduced a behavioral regression:
* Inline prologues — PREFIX ex: <...> SELECT ... on a single line were no
longer stripped because the mandatory (?:\n|$) anchor never matched when
non-whitespace content followed the IRI on the same line.
* CRLF line endings — PREFIX ex: <...>\r\n failed because \r is not in
[ \t]* and the anchor expected a bare \n.
Root cause: the end-of-line anchor was unnecessary; the only thing needed
to eliminate backtracking ambiguity is ensuring the IRI body character class
and the trailing whitespace quantifier are disjoint.
Fix: change the IRI body from <[^>]*> to <[^>\r\n]*>, which:
- excludes CR and LF from the IRI match (semantically correct — SPARQL
IRIs cannot span line boundaries)
- makes [^>\r\n]* and the trailing [ \t]* have zero character overlap,
eliminating all backtracking ambiguity without any end-of-line anchor
No anchor is used, so both inline prologues and CRLF/LF endings work
naturally. ReDoS payloads (base< + !< x 10,000) still complete in <1 ms.
--- Fix 2: oversized-query length guard obscured error (#review-2) ---
The initial patch placed the _SPARQL_MAX_QUERY_LEN guard inside
_is_read_only_query(), which caused execute_sparql() to return the same
generic 'Only SELECT' error for both genuinely disallowed query types and
oversized inputs. Clients could not distinguish the two rejection reasons.
Fix: move the length check out of _is_read_only_query() and into
execute_sparql() as an explicit early gate, alongside the other resource
limits (_SPARQL_MAX_ROWS, _SPARQL_MAX_GRAPH_NODES). Oversized queries now
return a specific message naming the limit, the received length, and the
remediation step. _is_read_only_query() is documented to be length-agnostic.
_SPARQL_MAX_QUERY_LEN is relocated to the resource-limits block with the
other constants.
--- Tests added ---
tests/test_security_regression.py:
- test_inline_prefix_before_select_allowed (Fix 1 regression)
- test_crlf_line_endings_with_prefix (Fix 1 regression)
- test_crlf_multiple_prefixes_then_select (Fix 1 regression)
- test_inline_prefix_before_insert_still_blocked (Fix 1 security check)
- test_long_valid_query_not_rejected_by_is_read_only (Fix 2 separation)
tests/explorer/test_sparql_route.py:
- test_oversized_query_returns_distinct_length_error (Fix 2 error message)
- test_oversized_query_never_touches_the_graph (Fix 2 short-circuit)
- test_query_exactly_at_length_limit_is_accepted (Fix 2 boundary)
All 82 tests pass.
The _PREFIX_DECL pattern used \s* as a trailing quantifier after
<[^>]*>. On inputs that start with ase< but contain no closing >
(e.g. ase<!<<!<<!<...), the regex engine explores exponentially many
ways to split the match between [^>]* and \s*, causing polynomial
backtracking against user-controlled SPARQL query input.
Fix:
- Replace ^\s* / \s+ / \s* with ^[ \t]* / [ \t]+ / [ \t]*
so the leading/internal whitespace quantifiers only match horizontal
whitespace (no overlap with the <[^>]*> IRI part).
- Replace the ambiguous trailing \s* with [ \t]*(?:\n|$), which
matches only horizontal whitespace followed by a hard line boundary.
[^>]* and [ \t]* have disjoint character sets, eliminating the
backtracking ambiguity entirely.
- Add _SPARQL_MAX_QUERY_LEN = 10_000 guard at the top of
_is_read_only_query as defence-in-depth: rejects oversized input
before any regex work, bounding worst-case cost even if a future
pattern change reintroduces ambiguity.
Verified: ReDoS payload ase< + !< x 5000 completes in <1 ms.
Normal PREFIX/BASE stripping and read-only query detection unchanged.
Fixes: CodeQL py/polynomial-redos alert #1897
CWE: CWE-1333, CWE-730, CWE-400
* security: sanitize Cypher labels/relationship types/property keys (GHSA-482h-hw99-h62p)
Node labels and property keys passed to create_node/create_relationship
were interpolated directly into Cypher strings in the Neptune, Neo4j, and
FalkorDB graph stores. Property values are parameterized, but labels and
keys can't be bound as parameters, and nothing validated them, so a
document-derived entity type or property name could close the current
Cypher token early and append arbitrary statements (e.g. DETACH DELETE),
running with the application's database credentials.
- New shared semantica/graph_store/query_sanitize.py: sanitize_identifier()
generalizes age_store.py's existing _sanitize_label/_sanitize_rel_type
(the only backend that already validated this) into a helper the other
backends can import without an import cycle with graph_store.py/methods.py.
- Applied at every label/relationship-type/property-key interpolation site
in amazon_neptune.py, neo4j_store.py, falkordb_store.py, graph_store.py
(degree_centrality's own query builder), and methods.py
(update_relationship's own query builder) — create_node, create_nodes,
create_relationship, get_nodes, get_relationships, get_neighbors,
shortest_path, update_node, create_index, and all relationship-type
filters.
- depth/max_depth path-length parameters are also cast to int before
interpolation as defense-in-depth (they're already typed int, but
Python doesn't enforce that at runtime).
Added tests/graph_store/test_cypher_injection.py (12 tests covering the
sanitizer directly and reproducing the advisory's injection payload
against Neptune/Neo4j/FalkorDB create_node/create_relationship — asserts
the malicious query is never built or sent), plus regression tests for
graph_store.py's degree_centrality and methods.py's update_relationship.
Full graph_store test suite (224 tests) passes with no regressions.
* fix(graph-store): prevent depth-based Cypher injection
* test(graph-store): tighten injection regression assertions
* docs(changelog): add PR #910 (GHSA-482h Cypher injection) entry
---------
Co-authored-by: Sameer6305 <sskadam6305@gmail.com>
Two issues in the last round of commits:
1. explorer/auth.py added a new, opt-in APIKeyAuthMiddleware
(EXPLORER_API_KEY) and wired it into create_app(), but in doing so
removed the Depends(require_auth) dependency from every router and
deleted the /ws/graph-updates handshake check entirely. The new
middleware also fails OPEN (allows all requests) when its key is
unset, the opposite of require_auth's fail-closed design. Since
GHSA-j4mq-hprp-987v (the unauthenticated-Explorer-API advisory) is
already merged into main via require_auth, this would have reverted
a merged Critical fix the moment this branch merges. Removed
explorer/auth.py, restored the per-router dependencies and the
WebSocket auth check. Kept auth.py's one genuine improvement (adding
X-API-Key to the CORS allow_headers list) by folding it into the
existing CORS middleware config.
2. sparql.py's new _is_read_only_query() hardening (comment/PREFIX
stripping + forbidden-keyword scan) used `#[^\n]*` to strip SPARQL
comments, but a bare '#' also appears inside standard RDF namespace
IRIs (e.g. ".../1999/02/22-rdf-syntax-ns#") — the regex struck
everything after that '#' as a "comment", corrupting the query and
rejecting any legitimate SELECT using rdf:/rdfs:-style PREFIX
declarations. Confirmed by the fact the new hardening's own inlined
test copy failed against two of its own cases. Fixed by only
treating '#' as a comment-start at line-start or after whitespace,
which distinguishes ".../ns#" (preceded by a word character) from an
actual comment (preceded by whitespace/newline in every realistic
case, including the attacker's own comment-hiding PoC). Also fixed
the companion PREFIX/BASE regex, which required a prefix-name token
between the keyword and the IRI even for bare `BASE <...>`
declarations (which have none).
tests/test_security_regression.py's SPARQL section now imports the real
_is_read_only_query instead of maintaining a parallel inlined copy that
had silently drifted from — and shared the same bug as — the real
implementation; removed its TestAPIKeyAuth class (tested the now-deleted
auth.py) since equivalent, more thorough coverage already exists in
tests/explorer/test_explorer_auth.py. Updated tests/explorer/test_sparql_route.py's
multi-statement-injection test to reflect that the keyword scan now
catches "SELECT ... ; DROP ALL" itself rather than relying on rdflib's
parser, and added a new test confirming the parser still catches
multi-statement syntax that doesn't contain any forbidden keyword.
Full explorer/vector_store/security-regression/age_store suite: 543
passed (the only failures are 6 pre-existing, unrelated Pinecone-client
mocking issues).
The previous rework of the redirect loop closed the response on each
redirect hop but dropped the try/finally around the success path, so the
terminal response (the one actually read and returned) was left
unclosed, leaking the connection back to the pool unclosed under load.
