From 67725add6d2b1f844141cb28baa61a70566dfaf2 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Wed, 12 Aug 2026 04:55:31 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20=EC=A0=80=EC=9E=A5=EB=90=9C=20SSRF(S?= =?UTF-8?q?tored=20SSRF)=20=EC=9C=84=ED=97=98=20=EC=99=84=ED=99=94=20?= =?UTF-8?q?=EB=B0=8F=20URL=20=ED=8C=8C=EC=8B=B1=EC=9D=98=20=ED=83=80?= =?UTF-8?q?=EC=9E=85=20=EC=97=90=EB=9F=AC=20=EB=B0=A9=EC=A7=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `appguardrail_core/controlplane.py`의 `/api/v1/webhook` POST 핸들러에서 전달된 `url`을 저장 전 명시적으로 검증 - `_is_safe_url()` 헬퍼(제어 평면과 CLI 모두)에 타입 체크(`if not isinstance(url, str)`)를 추가하여, 문자열 외 입력 시 `urllib.parse.urlparse`에서 `AttributeError` 예외(API 500 에러)가 발생하지 않도록 수정 - 400 Bad Request 등 관련된 단위 테스트를 수정하여 SSRF 방어 로직의 안전성 확인 --- .jules/sentinel.md | 5 +++++ appguardrail_core/controlplane.py | 12 +++++++++--- scanner/cli/appguardrail.py | 3 +++ tests/test_controlplane.py | 13 +++++++++++++ tests/test_ssrf_protection.py | 5 +++++ 5 files changed, 35 insertions(+), 3 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index f3e9114a..1600d432 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -122,3 +122,8 @@ **Vulnerability:** DOM XSS via unescaped `severity` string interpolated into `innerHTML` in `scanner/dashboard/index.html`. **Learning:** Even enum-like or seemingly safe meta-fields like `severity` can contain malicious payloads if sourced from user input (findings file) and directly injected into innerHTML. **Prevention:** Always use the `esc()` sanitizer for any dynamically rendered property from `findings.json`, regardless of expected schema types. + +## 2025-02-28 - Stored SSRF and Unhandled Parsing Exceptions Guardrail +**Vulnerability:** The `/api/v1/webhook` POST endpoint in `appguardrail_core/controlplane.py` failed to validate the `url` property when accepting it into the database, leading to Stored SSRF risks. In addition, the core SSRF validation logic (`_is_safe_url`) in both the CLI and control-plane did not verify the input type (e.g. `isinstance(url, str)`). Passing non-string types (like integers) resulted in unhandled `AttributeError` exceptions inside `urllib.parse.urlparse`, which led to API 500 crashes on malicious JSON payloads. +**Learning:** Network endpoints must explicitly validate the data type of user-provided configurations prior to execution or storage. Furthermore, webhooks configured by users should always be checked for SSRF when saved, as trusting them later assumes input has already been safely validated, bypassing downstream network guardrails. +**Prevention:** Apply `_is_safe_url` checks directly upon ingestion (e.g., in `/api/v1/webhook`) and enforce type checks `if not isinstance(url, str): return False` prior to using library parsing functions like `urlparse`. Always return gracefully failing responses (like `400 Bad Request`) for unsafe URLs instead of allowing unhandled 500 server errors. diff --git a/appguardrail_core/controlplane.py b/appguardrail_core/controlplane.py index bf74784e..35c8e253 100644 --- a/appguardrail_core/controlplane.py +++ b/appguardrail_core/controlplane.py @@ -221,6 +221,9 @@ def _is_safe_url(url: str) -> bool: import urllib.parse import socket + if not isinstance(url, str): + return False + try: parsed = urllib.parse.urlparse( url @@ -629,10 +632,13 @@ def do_POST(self): if not has_role(role, "owner"): return self._json(403, {"error": "owner role required"}) body = self._body() - if body is None: + if body is None or not isinstance(body, dict): return self._json(400, {"error": "invalid JSON body"}) - set_webhook(conn, org, (body or {}).get("url")) - return self._json(200, {"webhook_url": (body or {}).get("url")}) + webhook_url = body.get("url") + if webhook_url is not None and not _is_safe_url(webhook_url): + return self._json(400, {"error": "invalid webhook url"}) + set_webhook(conn, org, webhook_url) + return self._json(200, {"webhook_url": webhook_url}) if path == "/api/v1/keys": if not has_role(role, "owner"): diff --git a/scanner/cli/appguardrail.py b/scanner/cli/appguardrail.py index 9ca803e4..7dd60bec 100644 --- a/scanner/cli/appguardrail.py +++ b/scanner/cli/appguardrail.py @@ -1623,6 +1623,9 @@ def _is_safe_url(url: str) -> bool: import socket import urllib.parse + if not isinstance(url, str): + return False + try: parsed = urllib.parse.urlparse( url diff --git a/tests/test_controlplane.py b/tests/test_controlplane.py index 83f78f4b..80a7aaf9 100644 --- a/tests/test_controlplane.py +++ b/tests/test_controlplane.py @@ -230,6 +230,19 @@ def test_api_set_webhook(server): ) assert status == 200 and body["webhook_url"] == "http://hook.example/y" +def test_api_set_webhook_ssrf_protection(server): + base, key = server + # Invalid type + with pytest.raises(urllib.error.HTTPError) as exc: + _req("POST", f"{base}/api/v1/webhook", key, {"url": 1234}) + assert exc.value.code == 400 + assert json.loads(exc.value.read())["error"] == "invalid webhook url" + # Localhost SSRF attempt + with pytest.raises(urllib.error.HTTPError) as exc: + _req("POST", f"{base}/api/v1/webhook", key, {"url": "http://127.0.0.1/hook"}) + assert exc.value.code == 400 + assert json.loads(exc.value.read())["error"] == "invalid webhook url" + def test_roles_and_key_scoping(): conn = connect(":memory:") diff --git a/tests/test_ssrf_protection.py b/tests/test_ssrf_protection.py index 3462f72f..8c2f3fd7 100644 --- a/tests/test_ssrf_protection.py +++ b/tests/test_ssrf_protection.py @@ -10,6 +10,11 @@ def test_is_safe_url_public_domains(): assert _is_safe_url("http://google.com/") assert _is_safe_url("https://github.com/") +def test_is_safe_url_invalid_types(): + assert not _is_safe_url(None) + assert not _is_safe_url(123) + assert not _is_safe_url(True) + def test_is_safe_url_ipv4_localhost(): assert not _is_safe_url("http://127.0.0.1/") From 24a30834c14cbbf54ecff0076f62e2d33aa89364 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 14:07:57 +0900 Subject: [PATCH 2/2] fix(api): reject empty webhook updates --- appguardrail_core/controlplane.py | 5 ++++- tests/test_controlplane.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/appguardrail_core/controlplane.py b/appguardrail_core/controlplane.py index 35c8e253..576b990f 100644 --- a/appguardrail_core/controlplane.py +++ b/appguardrail_core/controlplane.py @@ -615,7 +615,10 @@ def _body(self): # Negative reads until EOF; oversized bodies exhaust memory. return None try: - return json.loads(self.rfile.read(length) or b"{}") + raw_body = self.rfile.read(length) + if not raw_body: + return None + return json.loads(raw_body) except (ValueError, TypeError): return None diff --git a/tests/test_controlplane.py b/tests/test_controlplane.py index 80a7aaf9..830f9693 100644 --- a/tests/test_controlplane.py +++ b/tests/test_controlplane.py @@ -230,6 +230,36 @@ def test_api_set_webhook(server): ) assert status == 200 and body["webhook_url"] == "http://hook.example/y" + +def test_api_empty_webhook_body_rejected(server): + import http.client + from urllib.parse import urlparse as _u + + base, key = server + _req("POST", f"{base}/api/v1/webhook", key, {"url": "http://hook.example/y"}) + + parsed = _u(base) + conn = http.client.HTTPConnection(parsed.hostname, parsed.port, timeout=5) + conn.putrequest("POST", "/api/v1/webhook") + conn.putheader("Authorization", f"Bearer {key}") + conn.putheader("Content-Type", "application/json") + conn.putheader("Content-Length", "0") + conn.endheaders() + response = conn.getresponse() + + assert response.status == 400 + assert json.loads(response.read()) == {"error": "invalid JSON body"} + conn.close() + + +def test_api_explicit_webhook_deletion(server): + base, key = server + status, body = _req("POST", f"{base}/api/v1/webhook", key, {"url": None}) + + assert status == 200 + assert body["webhook_url"] is None + + def test_api_set_webhook_ssrf_protection(server): base, key = server # Invalid type