Skip to content

Commit ff624bc

Browse files
authored
Merge pull request #7 from erinlkolp/claude/review-dial-api-module-eP5F3
URL-encode API keys to prevent query string injection
2 parents 0498ce2 + a82dc24 commit ff624bc

File tree

3 files changed

+15
-3
lines changed

3 files changed

+15
-3
lines changed

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ jobs:
1616
strategy:
1717
fail-fast: false
1818
matrix:
19-
python-version: ["3.11", "3.12", "3.13"]
19+
python-version: ["3.11", "3.12", "3.13", "3.14"]
2020

2121
steps:
2222
- uses: actions/checkout@v4

src/vudials_client/vudialsclient.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ def get_uri(self, server_url: str, api_key: str, api_call: str, keyword_params:
1313
# This means it will appear in server access logs, proxy logs, and
1414
# HTTP client history. If the server adds header-based authentication
1515
# in the future, prefer an Authorization or X-API-Key header instead.
16-
return f'{server_url}/api/v0/{api_call}?key={api_key}{keyword_params}'
16+
return f'{server_url}/api/v0/{api_call}?key={quote(api_key)}{keyword_params}'
1717

1818
def send_http_request(self, path_uri: str, files: dict, timeout: int = 10) -> requests.Response:
1919
if files is not None:
@@ -27,7 +27,7 @@ def send_http_request(self, path_uri: str, files: dict, timeout: int = 10) -> re
2727
class VUAdminUtil:
2828
def get_uri(self, server_url: str, api_key: str, api_call: str, keyword_params: str) -> str:
2929
# Security note: See VUUtil.get_uri — same key-in-URL caveat applies.
30-
return f'{server_url}/api/v0/{api_call}?admin_key={api_key}{keyword_params}'
30+
return f'{server_url}/api/v0/{api_call}?admin_key={quote(api_key)}{keyword_params}'
3131

3232
def send_http_request(self, path_uri: str, method: str, timeout: int = 10) -> requests.Response:
3333
if method == "post":

tests/test_vudialsclient.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ def test_multiple_keyword_params(self):
4949
assert "green=128" in uri
5050
assert "blue=0" in uri
5151

52+
def test_key_with_special_chars_url_encoded(self):
53+
# A key containing & and = must not break the query string structure.
54+
uri = self.util.get_uri("http://localhost:5340", "k&admin_key=evil", "dial/list", "")
55+
assert "k%26admin_key%3Devil" in uri
56+
assert "admin_key=evil" not in uri
57+
5258

5359
class TestVUUtilSendHttpRequest:
5460
def setup_method(self):
@@ -138,6 +144,12 @@ def test_api_path_format(self):
138144
uri = self.util.get_uri("http://192.168.0.1:9000", "k", "admin/keys/list", "")
139145
assert uri.startswith("http://192.168.0.1:9000/api/v0/")
140146

147+
def test_admin_key_with_special_chars_url_encoded(self):
148+
# A key containing & and = must not inject extra query parameters.
149+
uri = self.util.get_uri("http://localhost:5340", "a&key=evil", "admin/keys/list", "")
150+
assert "a%26key%3Devil" in uri
151+
assert "key=evil" not in uri
152+
141153

142154
class TestVUAdminUtilSendHttpRequest:
143155
def setup_method(self):

0 commit comments

Comments
 (0)