From 1c3ac66fd9d29b4d2ea73c5747e00fc01cd9e982 Mon Sep 17 00:00:00 2001 From: Sameer6305 Date: Tue, 11 Aug 2026 16:25:07 +0530 Subject: [PATCH] fix(rdf4j): preserve literal objects in delete_triplet --- semantica/triplet_store/rdf4j_store.py | 13 ++- tests/triplet_store/test_sparql_injection.py | 94 ++++++++++++++++++++ 2 files changed, 104 insertions(+), 3 deletions(-) diff --git a/semantica/triplet_store/rdf4j_store.py b/semantica/triplet_store/rdf4j_store.py index 594f6dd2..79a83320 100644 --- a/semantica/triplet_store/rdf4j_store.py +++ b/semantica/triplet_store/rdf4j_store.py @@ -484,11 +484,18 @@ class RDF4JStore: update_endpoint = self._get_update_endpoint() - # Use SPARQL DELETE + # Use SPARQL DELETE. + # subject/predicate must be IRIs — validate_uri enforces that and + # blocks injection through '>' or other SPARQL metacharacters. + # object can be an IRI *or* a literal, so it is routed through + # _format_object_for_ntriples (which internally calls validate_uri + # for URI-shaped values and escape_literal for strings), matching + # the same object-handling semantics used by the add path and by + # BlazegraphStore.delete_triplet (GHSA-8vgg-8mr4-r236 regression fix). subject = sparql_escaping.validate_uri(triplet.subject) predicate = sparql_escaping.validate_uri(triplet.predicate) - object_ = sparql_escaping.validate_uri(triplet.object) - query = f"DELETE DATA {{ <{subject}> <{predicate}> <{object_}> }}" + obj_str = self._format_object_for_ntriples(triplet) + query = f"DELETE DATA {{ <{subject}> <{predicate}> {obj_str} }}" try: response = requests.post( diff --git a/tests/triplet_store/test_sparql_injection.py b/tests/triplet_store/test_sparql_injection.py index d8bb0e48..30376804 100644 --- a/tests/triplet_store/test_sparql_injection.py +++ b/tests/triplet_store/test_sparql_injection.py @@ -101,6 +101,100 @@ class TestRDF4JSparqlInjection(unittest.TestCase): with self.assertRaises(ValidationError): store.delete_triplet(triplet) + # ------------------------------------------------------------------ + # Regression tests for the literal-object bug fixed after the + # adversarial review of PR #911: delete_triplet() previously called + # validate_uri(triplet.object) unconditionally, which rejected every + # non-URI object with ValidationError even though literal objects are + # perfectly legal in RDF. The fix routes the object through + # _format_object_for_ntriples so URI-valued objects are still validated + # while literal objects go through escape_literal unchanged. + # ------------------------------------------------------------------ + + def _make_connected_store_with_captured_query(self): + """Return (store, captured_dict) where captured['update'] is the + SPARQL update string passed to requests.post once delete_triplet + succeeds.""" + import requests as req_mod + from unittest.mock import MagicMock + + store = self._make_store() + store.connected = True + captured = {} + + mock_resp = MagicMock() + mock_resp.raise_for_status = MagicMock() + + def fake_post(url, **kwargs): + captured["update"] = kwargs.get("data", {}).get("update", "") + return mock_resp + + store._post = fake_post # not used directly; patch requests.post below + store._captured = captured + return store, captured + + def test_delete_triplet_literal_object_succeeds(self): + """A triplet with a plain-string literal object must delete without + raising ValidationError — the regression that prompted this fix.""" + store, captured = self._make_connected_store_with_captured_query() + + with patch("requests.post") as mock_post: + mock_post.return_value.__enter__ = lambda s: s + mock_post.return_value.raise_for_status = lambda: None + + result = store.delete_triplet( + Triplet(subject="http://s", predicate="http://p", object="Paris") + ) + + self.assertEqual(result, {"success": True}) + # Confirm query shape: object must be a quoted literal, not + query_sent = mock_post.call_args[1]["data"]["update"] + self.assertIn(" ", query_sent) + self.assertIn('"Paris"', query_sent) + self.assertNotIn("", query_sent) + self.assertNotIn("CLEAR ALL", query_sent) + + def test_delete_triplet_uri_object_still_works(self): + """A triplet whose object is a URI must still delete correctly.""" + store, _ = self._make_connected_store_with_captured_query() + + with patch("requests.post") as mock_post: + mock_post.return_value.raise_for_status = lambda: None + + result = store.delete_triplet( + Triplet(subject="http://s", predicate="http://p", object="http://o") + ) + + self.assertEqual(result, {"success": True}) + query_sent = mock_post.call_args[1]["data"]["update"] + self.assertIn(" ", query_sent) + self.assertNotIn("CLEAR ALL", query_sent) + + def test_delete_triplet_malicious_uri_object_rejected_before_post(self): + """A URI-shaped object containing '>' must be rejected by + _format_object_for_ntriples → validate_uri before requests.post + is ever called.""" + evil_obj = "http://evil.com/a>;CLEARALL" + store, _ = self._make_connected_store_with_captured_query() + + with patch("requests.post") as mock_post: + with self.assertRaises(ValidationError): + store.delete_triplet( + Triplet(subject="http://s", predicate="http://p", object=evil_obj) + ) + mock_post.assert_not_called() + + def test_delete_triplet_malicious_subject_still_rejected(self): + """Subject injection protection must remain intact after the fix.""" + store, _ = self._make_connected_store_with_captured_query() + + with patch("requests.post") as mock_post: + with self.assertRaises(ValidationError): + store.delete_triplet( + Triplet(subject=EVIL_SUBJECT, predicate="http://p", object="http://o") + ) + mock_post.assert_not_called() + def test_get_triplets_rejects_malicious_subject_filter(self): store = self._make_store() with self.assertRaises(ValidationError):