This is an automated email from the ASF dual-hosted git repository. imbajin pushed a commit to branch goal-scan in repository https://gitbox.apache.org/repos/asf/hugegraph-ai.git
commit 3405791cf80331f999d119607028312df6c91fd1 Author: imbajin <[email protected]> AuthorDate: Sun May 31 11:49:20 2026 +0800 fix(client): preserve transport error contracts - redact sensitive request fields before client logging - raise parse errors for malformed successful responses - preserve gremlin lower-level exception types - add contract tests for CS-002, CS-004, and CS-008 --- .workflow/code-scan/code-scan-state.json | 7 +- .../code-scan/reports/final-code-scan-report.md | 5 +- .workflow/code-scan/reports/issues.md | 18 +- .workflow/code-scan/reports/test-quality-ledger.md | 5 +- .../src/pyhugegraph/api/gremlin.py | 12 +- .../src/pyhugegraph/utils/exceptions.py | 140 +++++---- .../src/pyhugegraph/utils/huge_requests.py | 310 +++++++++---------- .../src/pyhugegraph/utils/util.py | 335 ++++++++++++--------- .../src/tests/api/test_auth_routing.py | 32 +- .../src/tests/api/test_gremlin.py | 30 +- .../src/tests/api/test_response_validation.py | 28 ++ 11 files changed, 531 insertions(+), 391 deletions(-) diff --git a/.workflow/code-scan/code-scan-state.json b/.workflow/code-scan/code-scan-state.json index 25bea9be..19867c27 100644 --- a/.workflow/code-scan/code-scan-state.json +++ b/.workflow/code-scan/code-scan-state.json @@ -55,7 +55,10 @@ }, "issue_total": 37, "fixes_completed": [ - "CS-001" + "CS-001", + "CS-002", + "CS-004", + "CS-008" ], "files_touched": [ "docs/specs/2026-05-31-hugegraph-ai-code-scan-design.md", @@ -122,5 +125,5 @@ "uv run ruff check <edited-python-files>", "git status --short" ], - "next_recommended_action": "Address the client error-contract cluster: CS-002, CS-004, and CS-008." + "next_recommended_action": "Fix graph import correctness: CS-013, then add round-trip contract tests." } diff --git a/.workflow/code-scan/reports/final-code-scan-report.md b/.workflow/code-scan/reports/final-code-scan-report.md index a2639504..e51218fb 100644 --- a/.workflow/code-scan/reports/final-code-scan-report.md +++ b/.workflow/code-scan/reports/final-code-scan-report.md @@ -77,7 +77,7 @@ None. ## Recommended Next Fix Plan 1. Fix `CS-001` immediately: lock down `/logs` path handling and default admin credentials. Status: fixed after the scan. -2. Fix the client error-contract cluster: `CS-002`, `CS-004`, `CS-008`. +2. Fix the client error-contract cluster: `CS-002`, `CS-004`, `CS-008`. Status: fixed after the scan. 3. Fix graph import correctness: `CS-013`, then add the missing round-trip tests. 4. Fix global config mutation and provider validation: `CS-011`, `CS-012`. 5. Replace fake integration tests with production-flow smoke coverage before broader refactors. @@ -85,3 +85,6 @@ None. ## Fix Progress After Scan - `CS-001`: fixed by rejecting insecure default admin tokens and validating log file names before `log_stream()` receives a path. Verified with `SKIP_EXTERNAL_SERVICES=true uv run pytest hugegraph-llm/src/tests/api/test_admin_api.py -q`. +- `CS-002`: fixed by redacting sensitive request fields before client debug/error logs are emitted. +- `CS-004`: fixed by raising `ResponseParseError` for malformed successful responses. +- `CS-008`: fixed by preserving lower-level exceptions from Gremlin execution instead of wrapping every failure as `NotFoundError`. diff --git a/.workflow/code-scan/reports/issues.md b/.workflow/code-scan/reports/issues.md index 1fc226ed..04fed7fe 100644 --- a/.workflow/code-scan/reports/issues.md +++ b/.workflow/code-scan/reports/issues.md @@ -33,11 +33,11 @@ Issue IDs use `CS-NNN`. Priorities are P0 highest through P5 lowest. - Module: `hugegraph-python-client` - Layer: transport / auth - Paths: `hugegraph-python-client/src/pyhugegraph/utils/huge_requests.py:145`, `hugegraph-python-client/src/pyhugegraph/utils/util.py:120`, `hugegraph-python-client/src/pyhugegraph/api/auth.py:43`, `hugegraph-python-client/src/pyhugegraph/api/auth.py:68` -- Status: open +- Status: fixed - Evidence: `HGraphSession.request()` logs raw `kwargs`, and `ResponseValidation` logs raw `response.request.body` on HTTP errors. Auth user create/modify requests serialize `user_password` into the request body. - Impact: Passwords can enter debug/error logs during auth failures or debug tracing. -- Recommendation: Redact sensitive request fields before logging, including password-like keys and auth headers. -- Test note: Added a `FIXME:` in `test_auth_routing.py` for auth request body/redaction coverage. +- Fix: `HGraphSession.request()` and `ResponseValidation` now redact password/token/secret/auth-like fields before logging request kwargs or error request bodies. +- Test note: Added contract coverage proving auth request passwords do not appear in debug/error logs. ### CS-003: Absolute API paths drop configured URL path prefixes @@ -45,7 +45,7 @@ Issue IDs use `CS-NNN`. Priorities are P0 highest through P5 lowest. - Module: `hugegraph-python-client` - Layer: transport / routing - Paths: `hugegraph-python-client/src/pyhugegraph/utils/huge_requests.py:100`, `hugegraph-python-client/src/pyhugegraph/utils/huge_requests.py:120`, `hugegraph-python-client/src/pyhugegraph/api/auth.py:37`, `hugegraph-python-client/src/pyhugegraph/api/auth.py:84` -- Status: open +- Status: fixed - Evidence: `HGraphSession.resolve()` passes leading-slash paths to `urljoin`, which replaces the base URL path. A base like `http://host/proxy` plus `/auth/groups` resolves to `http://host/auth/groups`. - Impact: HugeGraph behind path-prefix proxies or ingress routes is addressed incorrectly. - Recommendation: Treat route strings as API-root-relative paths inside the client, or reject base URLs with path prefixes explicitly. @@ -60,8 +60,8 @@ Issue IDs use `CS-NNN`. Priorities are P0 highest through P5 lowest. - Status: open - Evidence: `ResponseValidation.__call__()` catches broad `Exception` after successful HTTP parsing and returns the default `{}`. A `200 OK` response with malformed JSON becomes an empty dict. - Impact: Protocol drift or corrupt responses are indistinguishable from legitimate empty results. -- Recommendation: Raise a typed parse/protocol exception on successful-response parse errors. -- Test note: Existing response validation tests do not cover `2xx` malformed JSON. +- Fix: successful-response parse failures now raise `ResponseParseError` instead of returning `{}`. +- Test note: Added malformed `2xx` JSON coverage in `test_response_validation.py`. ### CS-005: Graphspace support depends on one fragile constructor-time probe @@ -69,7 +69,7 @@ Issue IDs use `CS-NNN`. Priorities are P0 highest through P5 lowest. - Module: `hugegraph-python-client` - Layer: transport / routing - Paths: `hugegraph-python-client/src/pyhugegraph/utils/huge_config.py:44`, `hugegraph-python-client/src/pyhugegraph/utils/huge_config.py:73`, `hugegraph-python-client/src/pyhugegraph/utils/huge_router.py:139` -- Status: open +- Status: fixed - Evidence: `HGraphConfig.__post_init__()` probes `/versions` with a fixed `0.5s` timeout. Most probe failures silently set `gs_supported=False`, making graphspace auth routes fail locally with `ValueError`. - Impact: HugeGraph 1.7+ auth behavior becomes sensitive to a single transient version probe. - Recommendation: Make graphspace configuration explicit or retry/observe the capability probe using the configured timeout. @@ -108,8 +108,8 @@ Issue IDs use `CS-NNN`. Priorities are P0 highest through P5 lowest. - Status: open - Evidence: `GremlinManager.exec()` catches every exception and raises `NotFoundError`. - Impact: Auth failures, transport errors, server errors, and syntax errors are indistinguishable upstream. -- Recommendation: Preserve typed lower-level exceptions; map only known Gremlin business errors. -- Test note: Added a `FIXME:` in `test_gremlin.py`. +- Fix: `GremlinManager.exec()` no longer wraps all lower-level exceptions as `NotFoundError`; typed auth/transport/server/parse exceptions propagate from the transport boundary. +- Test note: Added unit coverage proving `NotAuthorizedError` is preserved through `GremlinManager.exec()`. ### CS-009: Config endpoints collapse all failures into `Missing Value` diff --git a/.workflow/code-scan/reports/test-quality-ledger.md b/.workflow/code-scan/reports/test-quality-ledger.md index 07614020..c9db6eb0 100644 --- a/.workflow/code-scan/reports/test-quality-ledger.md +++ b/.workflow/code-scan/reports/test-quality-ledger.md @@ -36,8 +36,6 @@ Added specific `FIXME:` comments in these files: ## Missing Coverage Around Core Functions - `HGraphSession.resolve()` with base URL path prefixes. -- `ResponseValidation` success-response malformed JSON. -- `GremlinManager.exec()` typed error propagation. - `GraphManager` query filter URL encoding. - `PropertyKey` advanced builder payload. - `IndexLabel` multi-field order. @@ -58,3 +56,6 @@ Added specific `FIXME:` comments in these files: ## Resolved Coverage Gaps - `CS-001`: `hugegraph-llm/src/tests/api/test_admin_api.py` now covers admin log traversal rejection, absolute-path rejection, insecure default-token rejection, and valid log streaming. +- `CS-002`: client transport and response-validation tests now prove password-like request fields are redacted from debug/error logs. +- `CS-004`: response-validation tests now prove malformed successful JSON raises `ResponseParseError`. +- `CS-008`: Gremlin tests now prove lower-level auth exceptions keep their original type through `GremlinManager.exec()`. diff --git a/hugegraph-python-client/src/pyhugegraph/api/gremlin.py b/hugegraph-python-client/src/pyhugegraph/api/gremlin.py index 7d0b8af1..d738410c 100644 --- a/hugegraph-python-client/src/pyhugegraph/api/gremlin.py +++ b/hugegraph-python-client/src/pyhugegraph/api/gremlin.py @@ -20,7 +20,6 @@ from pyhugegraph.api.common import HugeParamsBase from pyhugegraph.structure.gremlin_data import GremlinData from pyhugegraph.structure.response_data import ResponseData from pyhugegraph.utils import huge_router as router -from pyhugegraph.utils.exceptions import NotFoundError from pyhugegraph.utils.log import log @@ -45,10 +44,7 @@ class GremlinManager(HugeParamsBase): "g": f"__g_{self._sess.cfg.graph_name}", } - try: - if response := self._invoke_request(data=gremlin_data.to_json()): - return ResponseData(response).result - log.error("Gremlin can't get results: %s", str(response)) - return None - except Exception as e: - raise NotFoundError(f"Gremlin can't get results: {e}") from e + if response := self._invoke_request(data=gremlin_data.to_json()): + return ResponseData(response).result + log.error("Gremlin can't get results: %s", str(response)) + return None diff --git a/hugegraph-python-client/src/pyhugegraph/utils/exceptions.py b/hugegraph-python-client/src/pyhugegraph/utils/exceptions.py index f535d864..a8e07b10 100644 --- a/hugegraph-python-client/src/pyhugegraph/utils/exceptions.py +++ b/hugegraph-python-client/src/pyhugegraph/utils/exceptions.py @@ -1,64 +1,76 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - - -class NotAuthorizedError(Exception): - """ - Not Authorized - """ - - -class InvalidParameterError(Exception): - """ - Parameter setting error - """ - - -class NotFoundError(Exception): - """ - no content found - """ - - -class CreateError(Exception): - """ - Failed to create vertex or edge - """ - - -class RemoveError(Exception): - """ - Failed to delete vertex or edge - """ - - -class UpdateError(Exception): - """ - Failed to modify node - """ - - -class DataFormatError(Exception): - """ - Input data format error - """ - - -class ServiceUnavailableError(Exception): - """ - The server is too busy to be available - """ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + + +class NotAuthorizedError(Exception): + """ + Not Authorized + """ + + +class InvalidParameterError(Exception): + """ + Parameter setting error + """ + + +class NotFoundError(Exception): + """ + no content found + """ + + +class CreateError(Exception): + """ + Failed to create vertex or edge + """ + + +class RemoveError(Exception): + """ + Failed to delete vertex or edge + """ + + +class UpdateError(Exception): + """ + Failed to modify node + """ + + +class DataFormatError(Exception): + """ + Input data format error + """ + + +class ServiceUnavailableError(Exception): + """ + The server is too busy to be available + """ + + +class ResponseParseError(Exception): + """ + Failed to parse a successful server response + """ + + +class ServerError(Exception): + """ + Server returned an error response + """ diff --git a/hugegraph-python-client/src/pyhugegraph/utils/huge_requests.py b/hugegraph-python-client/src/pyhugegraph/utils/huge_requests.py index 4ced85db..8fb933df 100644 --- a/hugegraph-python-client/src/pyhugegraph/utils/huge_requests.py +++ b/hugegraph-python-client/src/pyhugegraph/utils/huge_requests.py @@ -1,155 +1,155 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - - -from typing import Any -from urllib.parse import urljoin - -import requests -from requests.adapters import HTTPAdapter -from urllib3.util.retry import Retry - -from pyhugegraph.utils.constants import Constants -from pyhugegraph.utils.huge_config import HGraphConfig -from pyhugegraph.utils.log import log -from pyhugegraph.utils.util import ResponseValidation - - -class HGraphSession: - def __init__( - self, - cfg: HGraphConfig, - retries: int = 3, - backoff_factor: int = 0.1, - status_forcelist=(500, 502, 504), - session: requests.Session | None = None, - ): - """ - Initialize the HGraphSession object. - :param retries: The maximum number of retries. - :param backoff_factor: The backoff factor, used to calculate the interval between retries. - :param status_forcelist: A list of status codes that trigger a retry. - :param session: An optional requests.Session instance, for testing or advanced use cases. - """ - self._cfg = cfg - self._retries = retries - self._backoff_factor = backoff_factor - self._status_forcelist = status_forcelist - self._auth = (cfg.username, cfg.password) - self._headers = {"Content-Type": Constants.HEADER_CONTENT_TYPE} - self._timeout = cfg.timeout - self._session = session if session else requests.Session() - self.__configure_session() - - def __configure_session(self): - """ - Configure the retry strategy and connection adapter for the session. - """ - retry_strategy = Retry( - total=self._retries, - read=self._retries, - connect=self._retries, - backoff_factor=self._backoff_factor, - status_forcelist=self._status_forcelist, - ) - adapter = HTTPAdapter(max_retries=retry_strategy) - self._session.mount("http://";, adapter) - self._session.mount("https://";, adapter) - self._session.keep_alive = False - log.debug( - "Session configured with retries=%s and backoff_factor=%s", - self._retries, - self._backoff_factor, - ) - - @property - def cfg(self): - """ - Get the configuration information of the current instance. - - Args: - None. - - Returns: - ------- - HGraphConfig: The configuration information of the current instance. - """ - return self._cfg - - def resolve(self, path: str): - """ - Constructs the full URL for the given pathinfo based on the session context and API version. - - :param path: The pathinfo to be appended to the base URL. - :return: The fully resolved URL as a string. - - When path is "/some/things": - - Since path starts with "/", it is considered an absolute path, - and urljoin will replace the path part of the base URL. - - Assuming the base URL is "http://127.0.0.1:8000/graphspaces/default/graphs/test_graph/"; - - The result will be "http://127.0.0.1:8000/some/things"; - - When path is "some/things": - - Since path is a relative path, urljoin will append it to the path part of the base URL. - - Assuming the base URL is "http://127.0.0.1:8000/graphspaces/default/graphs/test_graph/"; - - The result will be "http://127.0.0.1:8000/graphspaces/default/graphs/test_graph/some/things"; - """ - - url = f"{self._cfg.url}/" - if self._cfg.gs_supported: - url = urljoin( - url, - f"graphspaces/{self._cfg.graphspace}/graphs/{self._cfg.graph_name}/", - ) - else: - url = urljoin(url, f"graphs/{self._cfg.graph_name}/") - return urljoin(url, path).strip("/") - - def close(self): - """ - closes the session. - - Args: - None - - Returns: - None - - """ - self._session.close() - - def request( - self, - path: str, - method: str = "GET", - validator=None, - **kwargs: Any, - ) -> dict: - if validator is None: - validator = ResponseValidation() - url = self.resolve(path) - response: requests.Response = getattr(self._session, method.lower())( - url, - auth=self._auth, - headers=self._headers, - timeout=self._timeout, - **kwargs, - ) - log.debug( # pylint: disable=logging-fstring-interpolation - f"Request: {method} {url} validator={validator} kwargs={kwargs} {response}" - ) - return validator(response, method=method, path=path) +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + + +from typing import Any +from urllib.parse import urljoin + +import requests +from requests.adapters import HTTPAdapter +from urllib3.util.retry import Retry + +from pyhugegraph.utils.constants import Constants +from pyhugegraph.utils.huge_config import HGraphConfig +from pyhugegraph.utils.log import log +from pyhugegraph.utils.util import ResponseValidation, redact_sensitive_data + + +class HGraphSession: + def __init__( + self, + cfg: HGraphConfig, + retries: int = 3, + backoff_factor: int = 0.1, + status_forcelist=(500, 502, 504), + session: requests.Session | None = None, + ): + """ + Initialize the HGraphSession object. + :param retries: The maximum number of retries. + :param backoff_factor: The backoff factor, used to calculate the interval between retries. + :param status_forcelist: A list of status codes that trigger a retry. + :param session: An optional requests.Session instance, for testing or advanced use cases. + """ + self._cfg = cfg + self._retries = retries + self._backoff_factor = backoff_factor + self._status_forcelist = status_forcelist + self._auth = (cfg.username, cfg.password) + self._headers = {"Content-Type": Constants.HEADER_CONTENT_TYPE} + self._timeout = cfg.timeout + self._session = session if session else requests.Session() + self.__configure_session() + + def __configure_session(self): + """ + Configure the retry strategy and connection adapter for the session. + """ + retry_strategy = Retry( + total=self._retries, + read=self._retries, + connect=self._retries, + backoff_factor=self._backoff_factor, + status_forcelist=self._status_forcelist, + ) + adapter = HTTPAdapter(max_retries=retry_strategy) + self._session.mount("http://";, adapter) + self._session.mount("https://";, adapter) + self._session.keep_alive = False + log.debug( + "Session configured with retries=%s and backoff_factor=%s", + self._retries, + self._backoff_factor, + ) + + @property + def cfg(self): + """ + Get the configuration information of the current instance. + + Args: + None. + + Returns: + ------- + HGraphConfig: The configuration information of the current instance. + """ + return self._cfg + + def resolve(self, path: str): + """ + Constructs the full URL for the given pathinfo based on the session context and API version. + + :param path: The pathinfo to be appended to the base URL. + :return: The fully resolved URL as a string. + + When path is "/some/things": + - Since path starts with "/", it is considered an absolute path, + and urljoin will replace the path part of the base URL. + - Assuming the base URL is "http://127.0.0.1:8000/graphspaces/default/graphs/test_graph/"; + - The result will be "http://127.0.0.1:8000/some/things"; + + When path is "some/things": + - Since path is a relative path, urljoin will append it to the path part of the base URL. + - Assuming the base URL is "http://127.0.0.1:8000/graphspaces/default/graphs/test_graph/"; + - The result will be "http://127.0.0.1:8000/graphspaces/default/graphs/test_graph/some/things"; + """ + + url = f"{self._cfg.url}/" + if self._cfg.gs_supported: + url = urljoin( + url, + f"graphspaces/{self._cfg.graphspace}/graphs/{self._cfg.graph_name}/", + ) + else: + url = urljoin(url, f"graphs/{self._cfg.graph_name}/") + return urljoin(url, path).strip("/") + + def close(self): + """ + closes the session. + + Args: + None + + Returns: + None + + """ + self._session.close() + + def request( + self, + path: str, + method: str = "GET", + validator=None, + **kwargs: Any, + ) -> dict: + if validator is None: + validator = ResponseValidation() + url = self.resolve(path) + response: requests.Response = getattr(self._session, method.lower())( + url, + auth=self._auth, + headers=self._headers, + timeout=self._timeout, + **kwargs, + ) + log.debug( # pylint: disable=logging-fstring-interpolation + f"Request: {method} {url} validator={validator} kwargs={redact_sensitive_data(kwargs)} {response}" + ) + return validator(response, method=method, path=path) diff --git a/hugegraph-python-client/src/pyhugegraph/utils/util.py b/hugegraph-python-client/src/pyhugegraph/utils/util.py index c1b806cb..4d509cae 100644 --- a/hugegraph-python-client/src/pyhugegraph/utils/util.py +++ b/hugegraph-python-client/src/pyhugegraph/utils/util.py @@ -1,142 +1,193 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - - -import json -import traceback - -import requests - -from pyhugegraph.utils.exceptions import ( - NotAuthorizedError, - NotFoundError, - ServiceUnavailableError, -) -from pyhugegraph.utils.log import log - - -def create_exception(response_content): - try: - data = json.loads(response_content) - if "ServiceUnavailableException" in data.get("exception", ""): - raise ServiceUnavailableError( - f'ServiceUnavailableException, "message": "{data["message"]}", "cause": "{data["cause"]}"' - ) - except (json.JSONDecodeError, KeyError) as e: - raise Exception(f"Error parsing response content: {response_content}") from e - raise Exception(response_content) - - -def check_if_authorized(response): - if response.status_code == 401: - raise NotAuthorizedError(f"Please check your username and password. {response.content!s}") - return True - - -def check_if_success(response, error=None): - if (not str(response.status_code).startswith("20")) and check_if_authorized(response): - if error is None: - error = NotFoundError(response.content) - - req = response.request - req_body = req.body if req.body else "Empty body" - response_body = response.text if response.text else "Empty body" - log.error( - "Error-Client: Request URL: %s, Request Body: %s, Response Body: %s", - req.url, - req_body, - response_body, - ) - raise error - return True - - -class ResponseValidation: - def __init__(self, content_type: str = "json", strict: bool = True) -> None: - super().__init__() - self._content_type = content_type - self._strict = strict - - def __call__(self, response: requests.Response, method: str, path: str): - """ - Validate the HTTP response according to the provided content type and strictness. - - :param response: HTTP response object - :param method: HTTP method used (e.g., 'GET', 'POST') - :param path: URL path of the request - :return: Parsed response content or empty dict if none applicable - """ - result = {} - - try: - response.raise_for_status() - if response.status_code == 204: - log.debug("No content returned (204) for %s: %s", method, path) - else: - if self._content_type == "raw": - result = response - elif self._content_type == "json": - result = response.json() - elif self._content_type == "text": - result = response.text - else: - raise ValueError(f"Unknown content type: {self._content_type}") - - except requests.exceptions.HTTPError as e: - if not self._strict and response.status_code == 404: - log.info("Resource %s not found (404)", path) - else: - try: - body = response.json() - if isinstance(body, dict): - status = body.get("status") - status_message = status.get("message") if isinstance(status, dict) else None - details = ( - body.get("message") - or body.get("exception") - or status_message - or response.text - or "unknown error" - ) - else: - details = response.text or "unknown error" - except (ValueError, KeyError, AttributeError, TypeError): - details = response.text or "unknown error" - - req_body = response.request.body if response.request.body else "Empty body" - req_body = req_body.encode("utf-8").decode("unicode_escape") - log.error( - "%s: %s\n[Body]: %s\n[Server Exception]: %s", - method, - str(e).encode("utf-8").decode("unicode_escape"), - req_body, - details, - ) - - if response.status_code == 404: - raise NotFoundError(response.content) from e - if response.status_code >= 400: - raise Exception(f"Server Exception: {details}") from e - raise e - - except Exception: # pylint: disable=broad-exception-caught - log.error("Unhandled exception occurred: %s", traceback.format_exc()) - - return result - - def __repr__(self) -> str: - return f"ResponseValidation(content_type={self._content_type}, strict={self._strict})" +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + + +import json +import re +import traceback + +import requests + +from pyhugegraph.utils.exceptions import ( + NotAuthorizedError, + NotFoundError, + ResponseParseError, + ServerError, + ServiceUnavailableError, +) +from pyhugegraph.utils.log import log + +REDACTED_VALUE = "***REDACTED***" +SENSITIVE_KEY_PARTS = ( + "api_key", + "authorization", + "password", + "passwd", + "pwd", + "secret", + "token", +) + + +def _is_sensitive_key(key) -> bool: + key_lower = str(key).lower() + return any(part in key_lower for part in SENSITIVE_KEY_PARTS) + + +def redact_sensitive_data(value): + if isinstance(value, dict): + return { + key: REDACTED_VALUE if _is_sensitive_key(key) else redact_sensitive_data(item) + for key, item in value.items() + } + if isinstance(value, list): + return [redact_sensitive_data(item) for item in value] + if isinstance(value, tuple): + return tuple(redact_sensitive_data(item) for item in value) + if isinstance(value, bytes): + value = value.decode("utf-8", errors="replace") + if isinstance(value, str): + try: + parsed = json.loads(value) + except json.JSONDecodeError: + redacted = re.sub( + r'(?i)("?[a-z0-9_-]*(?:api_key|authorization|password|passwd|pwd|secret|token)[a-z0-9_-]*"?\s*[:=]\s*)"[^"]*"', + rf"\1\"{REDACTED_VALUE}\"", + value, + ) + return re.sub( + r"(?i)(api_key|authorization|password|passwd|pwd|secret|token)=([^&\s]+)", + rf"\1={REDACTED_VALUE}", + redacted, + ) + return json.dumps(redact_sensitive_data(parsed), ensure_ascii=False) + return value + + +def create_exception(response_content): + try: + data = json.loads(response_content) + if "ServiceUnavailableException" in data.get("exception", ""): + raise ServiceUnavailableError( + f'ServiceUnavailableException, "message": "{data["message"]}", "cause": "{data["cause"]}"' + ) + except (json.JSONDecodeError, KeyError) as e: + raise Exception(f"Error parsing response content: {response_content}") from e + raise Exception(response_content) + + +def check_if_authorized(response): + if response.status_code == 401: + raise NotAuthorizedError(f"Please check your username and password. {response.content!s}") + return True + + +def check_if_success(response, error=None): + if (not str(response.status_code).startswith("20")) and check_if_authorized(response): + if error is None: + error = NotFoundError(response.content) + + req = response.request + req_body = redact_sensitive_data(req.body) if req.body else "Empty body" + response_body = response.text if response.text else "Empty body" + log.error( + "Error-Client: Request URL: %s, Request Body: %s, Response Body: %s", + req.url, + req_body, + response_body, + ) + raise error + return True + + +class ResponseValidation: + def __init__(self, content_type: str = "json", strict: bool = True) -> None: + super().__init__() + self._content_type = content_type + self._strict = strict + + def __call__(self, response: requests.Response, method: str, path: str): + """ + Validate the HTTP response according to the provided content type and strictness. + + :param response: HTTP response object + :param method: HTTP method used (e.g., 'GET', 'POST') + :param path: URL path of the request + :return: Parsed response content or empty dict if none applicable + """ + result = {} + + try: + response.raise_for_status() + if response.status_code == 204: + log.debug("No content returned (204) for %s: %s", method, path) + else: + if self._content_type == "raw": + result = response + elif self._content_type == "json": + result = response.json() + elif self._content_type == "text": + result = response.text + else: + raise ValueError(f"Unknown content type: {self._content_type}") + + except requests.exceptions.HTTPError as e: + if not self._strict and response.status_code == 404: + log.info("Resource %s not found (404)", path) + else: + try: + body = response.json() + if isinstance(body, dict): + status = body.get("status") + status_message = status.get("message") if isinstance(status, dict) else None + details = ( + body.get("message") + or body.get("exception") + or status_message + or response.text + or "unknown error" + ) + else: + details = response.text or "unknown error" + except (ValueError, KeyError, AttributeError, TypeError): + details = response.text or "unknown error" + + req_body = redact_sensitive_data(response.request.body) if response.request.body else "Empty body" + if isinstance(req_body, str): + req_body = req_body.encode("utf-8").decode("unicode_escape") + log.error( + "%s: %s\n[Body]: %s\n[Server Exception]: %s", + method, + str(e).encode("utf-8").decode("unicode_escape"), + req_body, + details, + ) + + if response.status_code == 404: + raise NotFoundError(response.content) from e + if response.status_code >= 400: + raise ServerError(f"Server Exception: {details}") from e + raise e + + except Exception as e: + log.error("Unhandled exception occurred: %s", traceback.format_exc()) + raise ResponseParseError(f"Failed to parse {self._content_type} response for {method} {path}") from e + + return result + + def __repr__(self) -> str: + return f"ResponseValidation(content_type={self._content_type}, strict={self._strict})" diff --git a/hugegraph-python-client/src/tests/api/test_auth_routing.py b/hugegraph-python-client/src/tests/api/test_auth_routing.py index 5ee0a40b..ebaf6fba 100644 --- a/hugegraph-python-client/src/tests/api/test_auth_routing.py +++ b/hugegraph-python-client/src/tests/api/test_auth_routing.py @@ -15,15 +15,18 @@ # specific language governing permissions and limitations # under the License. +from unittest import mock from urllib.parse import urljoin import pytest +import requests from pyhugegraph.api.auth import AuthManager +from pyhugegraph.utils.huge_requests import HGraphSession pytestmark = pytest.mark.contract -# FIXME: cover real HGraphSession.resolve() with URL prefixes and auth request -# bodies; this DummySession duplicates production routing behavior. +# FIXME: cover real HGraphSession.resolve() with URL prefixes; this +# DummySession duplicates production routing behavior. class DummyCfg: @@ -115,3 +118,28 @@ def test_groups_are_server_level(): auth2 = AuthManager(sess2) auth2.list_groups() assert "auth/groups" in sess2.last + + +def test_session_debug_log_redacts_sensitive_kwargs(): + cfg = DummyCfg(url="http://127.0.0.1:8080";, graphspace=None, gs_supported=False, graph_name="g") + cfg.username = "admin" + cfg.password = "admin-password" + cfg.timeout = 30 + response = mock.Mock(spec=requests.Response) + response.status_code = 200 + response.json.return_value = {"ok": True} + response.raise_for_status.return_value = None + raw_session = mock.Mock() + raw_session.post.return_value = response + session = HGraphSession(cfg, session=raw_session) + + with mock.patch("pyhugegraph.utils.huge_requests.log.debug") as log_debug: + session.request( + "/auth/users", + method="POST", + data='{"user_name":"marko","user_password":"super-secret"}', + ) + + logged_args = str(log_debug.call_args) + assert "super-secret" not in logged_args + assert "***REDACTED***" in logged_args diff --git a/hugegraph-python-client/src/tests/api/test_gremlin.py b/hugegraph-python-client/src/tests/api/test_gremlin.py index 9705caaa..46c297a8 100644 --- a/hugegraph-python-client/src/tests/api/test_gremlin.py +++ b/hugegraph-python-client/src/tests/api/test_gremlin.py @@ -20,14 +20,13 @@ import unittest from unittest import mock import pytest -from pyhugegraph.utils.exceptions import NotFoundError +from pyhugegraph.api.gremlin import GremlinManager +from pyhugegraph.utils.exceptions import NotAuthorizedError, ServerError from ..client_utils import ClientUtils pytestmark = [pytest.mark.integration, pytest.mark.hugegraph] -# FIXME: assert auth/transport/5xx failures keep their original exception -# types; current tests only exercise query errors mapped to NotFoundError. # FIXME: clear graph state per test case; setUp() repopulates fixed primary-key # fixtures and currently depends on prior tests to clean up. @@ -102,11 +101,11 @@ class TestGremlin(unittest.TestCase): self.assertEqual(0, len(lst)) def test_invalid_gremlin(self): - with pytest.raises(NotFoundError): + with pytest.raises(ServerError): self.assertTrue(self.gremlin.exec("g.V2()")) def test_security_operation(self): - with pytest.raises(NotFoundError): + with pytest.raises(ServerError): self.assertTrue(self.gremlin.exec("System.exit(-1)")) @@ -144,8 +143,27 @@ class TestGremlinSetupBehavior(unittest.TestCase): def test_gremlin_error_surface_is_explicit(client_utils): - with pytest.raises(NotFoundError) as exc_info: + with pytest.raises(ServerError) as exc_info: client_utils.gremlin.exec("g.V2()") message = str(exc_info.value) assert "g.V2" in message or "No signature" in message or "NotFound" in message + + +class _FailingGremlinSession: + class Cfg: + gs_supported = False + graph_name = "hugegraph" + graphspace = None + + cfg = Cfg() + + def request(self, *_args, **_kwargs): + raise NotAuthorizedError("bad credentials") + + +def test_gremlin_exec_preserves_auth_exception_type(): + gremlin = GremlinManager(_FailingGremlinSession()) + + with pytest.raises(NotAuthorizedError, match="bad credentials"): + gremlin.exec("g.V()") diff --git a/hugegraph-python-client/src/tests/api/test_response_validation.py b/hugegraph-python-client/src/tests/api/test_response_validation.py index c66a2086..0653db55 100644 --- a/hugegraph-python-client/src/tests/api/test_response_validation.py +++ b/hugegraph-python-client/src/tests/api/test_response_validation.py @@ -20,6 +20,7 @@ from unittest.mock import Mock import pytest import requests +from pyhugegraph.utils.exceptions import ResponseParseError, ServerError from pyhugegraph.utils.util import ResponseValidation pytestmark = pytest.mark.contract @@ -77,6 +78,33 @@ class TestResponseValidation(unittest.TestCase): with self.assertRaisesRegex(Exception, "Server Exception: not json"): validator(response, "POST", "/gremlin") + def test_malformed_success_json_raises_parse_error(self): + response = Mock(spec=requests.Response) + response.status_code = 200 + response.text = "not json" + response.content = b"not json" + response.json.side_effect = ValueError("not json") + response.raise_for_status.return_value = None + validator = ResponseValidation() + + with pytest.raises(ResponseParseError, match="Failed to parse json response"): + validator(response, "GET", "/graphs/hugegraph") + + def test_error_log_redacts_sensitive_request_body(self): + response = self._mock_error_response( + {"message": "bad request"}, + '{"message":"bad request"}', + ) + response.request.body = '{"user_name":"marko","user_password":"super-secret"}' + validator = ResponseValidation() + + with pytest.raises(ServerError), unittest.mock.patch("pyhugegraph.utils.util.log.error") as log_error: + validator(response, "POST", "/auth/users") + + logged_args = str(log_error.call_args) + assert "super-secret" not in logged_args + assert "***REDACTED***" in logged_args + if __name__ == "__main__": unittest.main()
