Skip to content

Commit 49e17e6

Browse files
fix(adms): escape OData V4 string keys to prevent injection
User-supplied IDs (job_id, document_type_id, etc.) were f-string interpolated directly into single-quote-wrapped OData entity keys. A value containing a single quote would break the URL or alter query intent — e.g. an ID like ``x'); ...`` could in principle target a different entity than the caller intended. Add ``quote_odata_string_key`` helper (OData V4 §5.1.1.6.2 — single quotes inside string literals must be doubled) and apply it at all 8 call sites in client.py (sync + async). Also adds 4 unit tests covering simple values, embedded quotes, injection neutralisation, and the empty string.
1 parent da70d1c commit 49e17e6

3 files changed

Lines changed: 44 additions & 10 deletions

File tree

src/sap_cloud_sdk/adms/_http.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,20 @@
3232
_CSRF_FETCH_VALUE = "Fetch"
3333

3434

35+
def quote_odata_string_key(value: str) -> str:
36+
"""Quote and escape a string value for use in an OData V4 entity key segment.
37+
38+
OData V4 §5.1.1.6.2 requires single-quoted string literals with embedded
39+
single quotes doubled. Without escaping, a value like ``O'Brien`` (or a
40+
deliberately crafted ``'); ...``) breaks the URL or alters query intent.
41+
42+
Example::
43+
44+
path = f"Documents(DocID={quote_odata_string_key(doc_id)})"
45+
"""
46+
return "'" + value.replace("'", "''") + "'"
47+
48+
3549
# ---------------------------------------------------------------------------
3650
# Sync HTTP wrapper
3751
# ---------------------------------------------------------------------------

src/sap_cloud_sdk/adms/client.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
import httpx
3131

3232
from sap_cloud_sdk.adms._auth import IasTokenFetcher
33-
from sap_cloud_sdk.adms._http import AdmsHttp, AsyncAdmsHttp
33+
from sap_cloud_sdk.adms._http import AdmsHttp, AsyncAdmsHttp, quote_odata_string_key
3434
from sap_cloud_sdk.adms._models import (
3535
AllowedDomain,
3636
BusinessObjectNodeType,
@@ -201,7 +201,7 @@ def get_download_url(
201201

202202
fn_key = (
203203
f"{rel_key}/DownloadDocument("
204-
f"DocContentVersionID='{doc_content_version_id}')"
204+
f"DocContentVersionID={quote_odata_string_key(doc_content_version_id)})"
205205
)
206206
resp = self._http.get(fn_key, service_base=_SERVICE_PATH)
207207
return resp.json().get("value", "")
@@ -674,7 +674,7 @@ def create_document_type(self, payload: CreateDocumentTypeInput) -> DocumentType
674674
def delete_document_type(self, document_type_id: str) -> None:
675675
"""Delete a document type classification."""
676676
self._http.delete(
677-
f"DocumentType(DocumentTypeID='{document_type_id}')",
677+
f"DocumentType(DocumentTypeID={quote_odata_string_key(document_type_id)})",
678678
service_base=_CONFIG_SERVICE_PATH,
679679
)
680680

@@ -720,7 +720,7 @@ def delete_business_object_type(
720720
) -> None:
721721
"""Delete a business object node type registration."""
722722
self._http.delete(
723-
f"BusinessObjectNodeType(BusinessObjectNodeTypeUniqueID='{business_object_node_type_unique_id}')",
723+
f"BusinessObjectNodeType(BusinessObjectNodeTypeUniqueID={quote_odata_string_key(business_object_node_type_unique_id)})",
724724
service_base=_CONFIG_SERVICE_PATH,
725725
)
726726

@@ -839,7 +839,7 @@ def get_status(
839839
Current :class:`~sap_cloud_sdk.adms._models.JobOutput`.
840840
"""
841841
service = _ADMIN_SERVICE_PATH if use_admin_service else _SERVICE_PATH
842-
path = f"JobStatus(JobID='{job_id}')"
842+
path = f"JobStatus(JobID={quote_odata_string_key(job_id)})"
843843
resp = self._http.get(path, service_base=service)
844844
return JobOutput.from_dict(resp.json())
845845

@@ -941,7 +941,7 @@ async def get_download_url(
941941

942942
fn_key = (
943943
f"{rel_key}/DownloadDocument("
944-
f"DocContentVersionID='{doc_content_version_id}')"
944+
f"DocContentVersionID={quote_odata_string_key(doc_content_version_id)})"
945945
)
946946
resp = await self._http.get(fn_key, service_base=_SERVICE_PATH)
947947
return resp.json().get("value", "")
@@ -1316,7 +1316,7 @@ async def create_document_type(
13161316
async def delete_document_type(self, document_type_id: str) -> None:
13171317
"""Async variant of :meth:`_ConfigurationApi.delete_document_type` — same semantics."""
13181318
await self._http.delete(
1319-
f"DocumentType(DocumentTypeID='{document_type_id}')",
1319+
f"DocumentType(DocumentTypeID={quote_odata_string_key(document_type_id)})",
13201320
service_base=_CONFIG_SERVICE_PATH,
13211321
)
13221322

@@ -1362,7 +1362,7 @@ async def delete_business_object_type(
13621362
) -> None:
13631363
"""Async variant of :meth:`_ConfigurationApi.delete_business_object_type` — same semantics."""
13641364
await self._http.delete(
1365-
f"BusinessObjectNodeType(BusinessObjectNodeTypeUniqueID='{business_object_node_type_unique_id}')",
1365+
f"BusinessObjectNodeType(BusinessObjectNodeTypeUniqueID={quote_odata_string_key(business_object_node_type_unique_id)})",
13661366
service_base=_CONFIG_SERVICE_PATH,
13671367
)
13681368

@@ -1471,7 +1471,7 @@ async def get_status(
14711471
Current :class:`~sap_cloud_sdk.adms._models.JobOutput`.
14721472
"""
14731473
service = _ADMIN_SERVICE_PATH if use_admin_service else _SERVICE_PATH
1474-
path = f"JobStatus(JobID='{job_id}')"
1474+
path = f"JobStatus(JobID={quote_odata_string_key(job_id)})"
14751475
resp = await self._http.get(path, service_base=service)
14761476
return JobOutput.from_dict(resp.json())
14771477

tests/adms/unit/test_http.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import requests
88

99
from sap_cloud_sdk.adms._auth import IasTokenFetcher
10-
from sap_cloud_sdk.adms._http import AdmsHttp
10+
from sap_cloud_sdk.adms._http import AdmsHttp, quote_odata_string_key
1111
from sap_cloud_sdk.adms.config import AdmsConfig
1212
from sap_cloud_sdk.adms.exceptions import DocumentNotFoundError, HttpError
1313

@@ -200,3 +200,23 @@ def test_service_jwt_uses_get_token(self, config, token_fetcher):
200200

201201
token_fetcher.get_token.assert_called()
202202
token_fetcher.exchange_token.assert_not_called()
203+
204+
205+
class TestQuoteOdataStringKey:
206+
def test_simple_value(self):
207+
assert quote_odata_string_key("job-123") == "'job-123'"
208+
209+
def test_value_with_single_quote_is_doubled(self):
210+
# OData V4 §5.1.1.6.2 — single quotes inside string literals must be doubled.
211+
assert quote_odata_string_key("O'Brien") == "'O''Brien'"
212+
213+
def test_injection_attempt_is_neutralised(self):
214+
# An attacker-controlled value must not break out of the quoted segment.
215+
out = quote_odata_string_key("x'); DROP TABLE--")
216+
assert out == "'x''); DROP TABLE--'"
217+
# Result is one single-quoted literal, not two.
218+
assert out.count("'") % 2 == 0
219+
220+
def test_empty_string(self):
221+
assert quote_odata_string_key("") == "''"
222+

0 commit comments

Comments
 (0)