Skip to content

Commit 1135228

Browse files
committed
fix(destination): add missing validate_resource_name to get_instance/subaccount read methods
get_instance_fragment, get_subaccount_fragment, get_instance_certificate, and get_subaccount_certificate were delegating directly to internal helpers that have an except Exception catch-all, causing invalid names to raise DestinationOperationError instead of ValueError. Add validate_resource_name before the try block in all four methods, consistent with every other public method on this branch. Also replace path-echo in _http.py traversal guard error message with a fixed generic message to avoid returning attacker-controlled strings in error responses. 843 tests green.
1 parent 9b041a9 commit 1135228

5 files changed

Lines changed: 33 additions & 3 deletions

File tree

‎src/sap_cloud_sdk/destination/_http.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,7 @@ def _request(
3636
# even if the client-layer validation is somehow bypassed.
3737
path_segments = normalized_path.split("/")
3838
if ".." in path_segments:
39-
raise HttpError(
40-
f"Path traversal detected: '..' segment in path {normalized_path!r}",
41-
)
39+
raise HttpError("Forbidden: path traversal detected")
4240
try:
4341
resp = http.request(
4442
method,

‎src/sap_cloud_sdk/destination/certificate_client.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ def get_instance_certificate(self, name: str) -> Optional[Certificate]:
173173
Raises:
174174
DestinationOperationError: If an HTTP error occurs or response parsing fails.
175175
"""
176+
validate_resource_name(name)
176177
try:
177178
return self._get_certificate(name=name, level=Level.SERVICE_INSTANCE)
178179
except HttpError as e:
@@ -207,6 +208,7 @@ def get_subaccount_certificate(
207208
DestinationOperationError: If tenant is missing for subscriber access strategies,
208209
on HTTP errors, or response parsing failures.
209210
"""
211+
validate_resource_name(name)
210212
try:
211213
return self._apply_access_strategy(
212214
access_strategy=access_strategy,

‎src/sap_cloud_sdk/destination/fragment_client.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ def get_instance_fragment(self, name: str) -> Optional[Fragment]:
9494
Raises:
9595
DestinationOperationError: If an HTTP error occurs or response parsing fails.
9696
"""
97+
validate_resource_name(name)
9798
try:
9899
return self._get_fragment(name=name, level=Level.SERVICE_INSTANCE)
99100
except HttpError as e:
@@ -126,6 +127,7 @@ def get_subaccount_fragment(
126127
DestinationOperationError: If tenant is missing for subscriber access strategies,
127128
on HTTP errors, or response parsing failures.
128129
"""
130+
validate_resource_name(name)
129131
try:
130132
return self._apply_access_strategy(
131133
access_strategy=access_strategy,

‎tests/destination/unit/test_certificate_client.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -634,3 +634,17 @@ def test_update_certificate_rejects_invalid_name(self, client):
634634
with pytest.raises(ValueError):
635635
c.update_certificate(cert)
636636
http.request.assert_not_called()
637+
638+
@pytest.mark.parametrize("name", _ATTACK_NAMES)
639+
def test_get_instance_certificate_rejects_traversal_names(self, client, name):
640+
c, http = client
641+
with pytest.raises(ValueError):
642+
c.get_instance_certificate(name)
643+
http.request.assert_not_called()
644+
645+
@pytest.mark.parametrize("name", _ATTACK_NAMES)
646+
def test_get_subaccount_certificate_rejects_traversal_names(self, client, name):
647+
c, http = client
648+
with pytest.raises(ValueError):
649+
c.get_subaccount_certificate(name)
650+
http.request.assert_not_called()

‎tests/destination/unit/test_fragment_client.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,3 +606,17 @@ def test_update_fragment_rejects_invalid_name(self, client):
606606
with pytest.raises(ValueError):
607607
c.update_fragment(frag)
608608
http.request.assert_not_called()
609+
610+
@pytest.mark.parametrize("name", _ATTACK_NAMES)
611+
def test_get_instance_fragment_rejects_traversal_names(self, client, name):
612+
c, http = client
613+
with pytest.raises(ValueError):
614+
c.get_instance_fragment(name)
615+
http.request.assert_not_called()
616+
617+
@pytest.mark.parametrize("name", _ATTACK_NAMES)
618+
def test_get_subaccount_fragment_rejects_traversal_names(self, client, name):
619+
c, http = client
620+
with pytest.raises(ValueError):
621+
c.get_subaccount_fragment(name)
622+
http.request.assert_not_called()

0 commit comments

Comments
 (0)