feat: implement global authentication mode and enhance vehicle initia…#161
feat: implement global authentication mode and enhance vehicle initia…#161DasBasti wants to merge 21 commits into
Conversation
…lization - Added support for global authentication mode in SmartAccount and SmartAuthentication classes. - Introduced methods for initializing vehicles from Smart Global servers. - Updated API session handling to accommodate global app requirements. - Enhanced logging and error handling for global API interactions. - Modified CLI to support new region options for global access.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a Global HMAC authentication mode alongside existing EU OAuth, introducing global endpoints, HMAC header generation, auth-mode dispatch in login/refresh flows, and routing vehicle operations through global or EU APIs based on inferred auth mode. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Authentication
participant GlobalAPI as Global API
participant EUAPI as EU OAuth API
participant Account
rect rgba(100, 200, 100, 0.5)
Note over Client,Authentication: Global HMAC Flow
Client->>Authentication: login()
Authentication->>GlobalAPI: POST /login (HMAC-signed)
GlobalAPI-->>Authentication: {id_token, api_user_id, ...}
Authentication-->>Client: auth_mode=GLOBAL_HMAC
Client->>Account: get_vehicles()
Account->>GlobalAPI: GET /vehicles (HMAC-signed)
GlobalAPI-->>Account: vehicle list
Account->>GlobalAPI: GET /vehicle/{vin}/abilities (HMAC-signed)
GlobalAPI-->>Account: vehicle details & abilities
Account-->>Client: merged vehicles
end
rect rgba(100, 150, 200, 0.5)
Note over Client,Authentication: EU OAuth Flow
Client->>Authentication: login()
Authentication->>EUAPI: OAuth token exchange
EUAPI-->>Authentication: {accessToken, idToken, refreshToken, ...}
Authentication-->>Client: auth_mode=EU_OAUTH
Client->>Account: get_vehicles()
Account->>EUAPI: GET /vehicles (Bearer token)
EUAPI-->>Account: vehicle list
Account->>EUAPI: GET /vehicle/{vin}/details
EUAPI-->>Account: vehicle details & abilities
Account-->>Client: merged vehicles
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… sanitization Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds support for Smart “Global app” authentication/endpoint selection and adjusts vehicle initialization to work with Global servers.
Changes:
- Introduces
SmartRegion.GLOBALandSmartAuthModeto drive endpoint selection and auth flow (EU OAuth vs Global HMAC). - Implements Global HMAC header signing plus Global login/refresh and Global vehicle initialization/detail fetching.
- Updates CLI region options and extends endpoint URL unit tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
pysmarthashtag/vehicle/vehicle.py |
Chooses vehicle API base URL based on auth mode and series/model code. |
pysmarthashtag/tests/test_endpoint_urls.py |
Adds coverage for SmartRegion.GLOBAL endpoint selection. |
pysmarthashtag/tests/common.py |
Adds mocked OAuth token endpoint route for refreshed-token flow. |
pysmarthashtag/const.py |
Adds Global constants, SmartAuthMode, Global endpoints, and extended EndpointUrls. |
pysmarthashtag/cli.py |
Adds global region CLI option and allows log level override via env var. |
pysmarthashtag/api/utils.py |
Adds Global HMAC request header/signature generation helpers. |
pysmarthashtag/api/authentication.py |
Adds auth-mode inference and implements Global login/refresh + EU refresh flow extraction. |
pysmarthashtag/account.py |
Adds Global vehicle init/details/abilities flows and switches behavior based on auth mode. |
pyproject.toml |
Updates Ruff config keys to lint.* tables and related sections. |
| self.id_token = token_data.get("id_token") | ||
| self.expires_at = token_data["expires_at"] | ||
| _LOGGER.debug("Login successful") | ||
| return True |
There was a problem hiding this comment.
SmartAuthentication.login is annotated to return None, but it returns True on success. Either change the return type to bool and document it, or keep it as None and remove the boolean return to avoid a misleading API/typing contract.
| return True |
| expires_at = datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta( | ||
| seconds=int(expires_in) if expires_in else HTTPX_TIMEOUT * 2 | ||
| ) |
There was a problem hiding this comment.
The fallback expires_at uses HTTPX_TIMEOUT * 2, but login() later subtracts EXPIRES_AT_OFFSET (also HTTPX_TIMEOUT * 2). If expiresIn is missing, the token will be considered expired immediately. Use a fallback that’s comfortably larger than the offset (e.g., a sane default TTL like 3600s) or avoid subtracting more than the computed TTL.
| return {} | ||
|
|
||
| expires_at = datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta( | ||
| seconds=int(expires_in) if expires_in else HTTPX_TIMEOUT * 2 |
There was a problem hiding this comment.
The fallback expires_at uses HTTPX_TIMEOUT * 2, but login() later subtracts EXPIRES_AT_OFFSET (also HTTPX_TIMEOUT * 2). If expiresIn is missing, the token will be considered expired immediately. Use a fallback that’s comfortably larger than the offset (e.g., a sane default TTL like 3600s) or avoid subtracting more than the computed TTL.
| seconds=int(expires_in) if expires_in else HTTPX_TIMEOUT * 2 | |
| seconds=int(expires_in) if expires_in else 3600 |
| async def get_vehicle_soc(self, vin) -> str: | ||
| """Get information about a vehicle.""" |
There was a problem hiding this comment.
get_vehicle_soc is annotated as returning str, but it returns a dict ({}) in the global-auth branch and a dict in the non-global branch as well. Update the return type hint to dict (and adjust the docstring) to match actual behavior.
| async def get_vehicle_soc(self, vin) -> str: | |
| """Get information about a vehicle.""" | |
| async def get_vehicle_soc(self, vin) -> dict: | |
| """Get state of charge (SOC) information for a vehicle.""" |
| GLOBAL_API_BASE_URL = "https://sg-app-api.smart.com" | ||
| GLOBAL_APP_KEY = "204587190" | ||
| GLOBAL_APP_SECRET = "vxnzkHbpQrkKKQKmFBZlOnL780rjXLFT" | ||
|
|
||
| EU_OAUTH_BASE_URL = "https://api.app-auth.srv.smart.com/v1/" | ||
| EU_OAUTH_API_KEY = "yHpsjnd9vzLq7GMowxBa" | ||
|
|
There was a problem hiding this comment.
GLOBAL_APP_SECRET (and similar credentials) are committed as constants. Even if they’re “app” credentials, hardcoding secrets makes rotation and downstream auditing difficult. Prefer loading these from environment/user configuration (e.g., require EndpointUrls.global_app_secret to be provided) and avoid shipping secrets in the library by default.
| async def _init_vehicles_global(self) -> None: | ||
| """Initialize vehicles from Smart Global servers.""" | ||
| _LOGGER.debug("Getting initial vehicle list (global)") | ||
| await self._ensure_ssl_context() | ||
|
|
||
| fetched_at = datetime.datetime.now(datetime.timezone.utc) | ||
| async with SmartLoginClient(ssl_context=self.config.ssl_context) as client: | ||
| path = "/vc/vehicle/v1/ownership/list" | ||
| body = json.dumps({}) | ||
| host = httpx.URL(self.endpoint_urls.get_api_base_url()).host | ||
| headers = utils.generate_global_header( | ||
| method="POST", | ||
| path=path, | ||
| host=host, | ||
| app_key=self.endpoint_urls.get_global_app_key(), | ||
| app_secret=self.endpoint_urls.get_global_app_secret(), | ||
| body=body, | ||
| access_token=self.config.authentication.access_token, | ||
| user_id=self.config.authentication.api_user_id, | ||
| id_token=self.config.authentication.id_token, | ||
| ) | ||
| vehicles_response = await client.post( | ||
| self.endpoint_urls.get_api_base_url() + path, | ||
| headers=headers, | ||
| content=body, | ||
| ) | ||
| data = vehicles_response.json() | ||
| vehicles = data.get("result") or data.get("data") or [] | ||
| for vehicle in vehicles: | ||
| _LOGGER.debug("Found vehicle %s", sanitize_log_data(vehicle)) | ||
| self.add_vehicle(vehicle, fetched_at) | ||
|
|
There was a problem hiding this comment.
Global vehicle initialization and follow-up detail/ability fetches introduce new network behavior, but there are no tests exercising the global flow. Add respx-based tests that set SmartRegion.GLOBAL (or EndpointUrls(api_base_url=GLOBAL_API_BASE_URL)) and assert _init_vehicles_global() parses vehicles and _update_global_vehicle_details() populates combine_data/abilities as expected.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| async def _login_global(self) -> dict: | ||
| """Login to Smart Global app services (HMAC flow).""" | ||
| ssl_ctx = await self.get_ssl_context() | ||
| async with SmartLoginClient(ssl_context=ssl_ctx) as client: | ||
| _LOGGER.info("Acquiring access token (global app).") | ||
|
|
||
| path = "/iam/service/api/v1/login" | ||
| payload = { | ||
| "email": self.username, | ||
| "password": self.password, | ||
| "imageSessionId": "", | ||
| "imageCode": "", | ||
| } | ||
| body = json.dumps(payload) | ||
| host = httpx.URL(self.endpoint_urls.get_api_base_url()).host | ||
| headers = utils.generate_global_header( | ||
| method="POST", | ||
| path=path, | ||
| host=host, | ||
| app_key=self.endpoint_urls.get_global_app_key(), | ||
| app_secret=self.endpoint_urls.get_global_app_secret(), | ||
| body=body, | ||
| ) | ||
|
|
||
| r_login = await client.post( | ||
| self.endpoint_urls.get_api_base_url() + path, | ||
| headers=headers, | ||
| content=body, | ||
| ) | ||
| login_result = r_login.json() | ||
| _LOGGER.debug("Login result: %s", sanitize_log_data(login_result)) | ||
| data = login_result.get("data") or login_result.get("result") or {} | ||
| if not data: | ||
| message = login_result.get("message", "Unknown error") | ||
| code = login_result.get("code", "unknown") | ||
| raise SmartAPIError(f"Could not get tokens from global login: {code} {message}") | ||
|
|
||
| access_token = data.get("accessToken") | ||
| refresh_token = data.get("refreshToken") | ||
| id_token = data.get("idToken") | ||
| api_user_id = data.get("userId") | ||
| expires_in = data.get("expiresIn") or data.get("expires_in") | ||
| if not access_token or not api_user_id: | ||
| raise SmartAPIError("Could not get access token from global login") | ||
|
|
||
| expires_at = datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta( | ||
| seconds=int(expires_in) if expires_in else HTTPX_TIMEOUT * 2 | ||
| ) | ||
|
|
||
| return { | ||
| "access_token": access_token, | ||
| "refresh_token": refresh_token, | ||
| "api_access_token": access_token, | ||
| "api_refresh_token": refresh_token, | ||
| "api_user_id": api_user_id, | ||
| "id_token": id_token, | ||
| "expires_at": expires_at, | ||
| } |
There was a problem hiding this comment.
The new Global HMAC login/refresh paths are untested. Add unit tests with mocked responses verifying that _login_global() and _refresh_access_token_global() correctly extract tokens/userId/expires and that auth_mode selection triggers these code paths for GLOBAL endpoints.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@pyproject.toml`:
- Around line 78-81: The per-file-ignores entry contains a typo in the package
name ("pysamrthashtag") so the ignore for the authentication module won't apply;
in pyproject.toml update the per-file-ignores key string to the correct package
name token ("pysmarthashtag/api/authentication.py") while keeping the ignore
codes ["D102","D107"] unchanged, then save and re-run lint/CI to verify the rule
is applied.
In `@pysmarthashtag/account.py`:
- Around line 122-153: The _init_vehicles_global method currently calls
vehicles_response.json() without checking HTTP status, which can raise or return
invalid data on non-2xx responses; update _init_vehicles_global to check
vehicles_response.status_code (or use vehicles_response.is_success) after the
client.post call and handle non-2xx responses by logging an error with status
and body (use sanitize_log_data for body) and returning/raising appropriately
before calling vehicles_response.json(); ensure subsequent logic that reads data
and calls self.add_vehicle(...) only runs when the response is successful.
- Around line 359-390: The _get_vehicle_details_global function currently
ignores HTTP error statuses because it calls response.json() without checking
the response; after the await client.post(...) call inside the SmartLoginClient
context, call response.raise_for_status() to raise on 4xx/5xx so errors are
surfaced (before parsing response.json()), leaving the rest of the logic
(parsing data, normalizing details, and calling
self.vehicles.get(vin).combine_data(details)) unchanged.
In `@pysmarthashtag/api/utils.py`:
- Around line 80-85: The _ensure_bytes function currently casts arbitrary
objects via str(), causing non-JSON reprs and HMAC mismatches; update
_ensure_bytes to explicitly handle bytes (return as-is), str (encode utf-8), and
dict/list (serialize with json.dumps(..., separators=(",", ":")) and encode
utf-8), raise a TypeError for any other types, and narrow the type hint
accordingly so callers cannot accidentally pass unsupported objects (refer to
_ensure_bytes function name to locate the change).
In `@pysmarthashtag/cli.py`:
- Around line 59-60: Validate the SMART_LOG_LEVEL value before assigning it into
logging_config to avoid ValueError during dictConfig: read and upper() the env
var as you already do, check it against the set of known logging level names
(e.g., via logging._nameToLevel or by using logging.getLevelName and confirming
it maps to a numeric level), and if it's invalid fall back to "INFO" (and emit a
warning via the pysmarthashtag logger or stderr) before setting
logging_config["loggers"]["pysmarthashtag"]["level"] = log_level; ensure this
validation logic is placed where log_level is computed so dictConfig never
receives an invalid level.
In `@pysmarthashtag/const.py`:
- Around line 18-24: Remove the hardcoded credentials and load them from
environment/config: replace the literal values for GLOBAL_APP_KEY,
GLOBAL_APP_SECRET, and EU_OAUTH_API_KEY with environment-based lookups (e.g.,
use os.environ.get or your config loader) and keep GLOBAL_API_BASE_URL and
EU_OAUTH_BASE_URL while allowing them to be overridden from env if desired; add
an import os at the top; ensure defaults are not secrets (raise/configure error
if required env vars are missing) and rotate the exposed keys after deploying
the change.
| async def _init_vehicles_global(self) -> None: | ||
| """Initialize vehicles from Smart Global servers.""" | ||
| _LOGGER.debug("Getting initial vehicle list (global)") | ||
| await self._ensure_ssl_context() | ||
|
|
||
| fetched_at = datetime.datetime.now(datetime.timezone.utc) | ||
| async with SmartLoginClient(ssl_context=self.config.ssl_context) as client: | ||
| path = "/vc/vehicle/v1/ownership/list" | ||
| body = json.dumps({}) | ||
| host = httpx.URL(self.endpoint_urls.get_api_base_url()).host | ||
| headers = utils.generate_global_header( | ||
| method="POST", | ||
| path=path, | ||
| host=host, | ||
| app_key=self.endpoint_urls.get_global_app_key(), | ||
| app_secret=self.endpoint_urls.get_global_app_secret(), | ||
| body=body, | ||
| access_token=self.config.authentication.access_token, | ||
| user_id=self.config.authentication.api_user_id, | ||
| id_token=self.config.authentication.id_token, | ||
| ) | ||
| vehicles_response = await client.post( | ||
| self.endpoint_urls.get_api_base_url() + path, | ||
| headers=headers, | ||
| content=body, | ||
| ) | ||
| data = vehicles_response.json() | ||
| vehicles = data.get("result") or data.get("data") or [] | ||
| for vehicle in vehicles: | ||
| _LOGGER.debug("Found vehicle %s", sanitize_log_data(vehicle)) | ||
| self.add_vehicle(vehicle, fetched_at) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.py" | head -20Repository: DasBasti/pySmartHashtag
Length of output: 763
🏁 Script executed:
fd "account.py" --type fRepository: DasBasti/pySmartHashtag
Length of output: 129
🏁 Script executed:
cat -n pysmarthashtag/account.py | sed -n '115,160p'Repository: DasBasti/pySmartHashtag
Length of output: 2506
🏁 Script executed:
fd ".*client.*" --type f -e py | grep -i "smart\|login\|client"Repository: DasBasti/pySmartHashtag
Length of output: 95
🏁 Script executed:
rg "class SmartLoginClient" -A 10 --type pyRepository: DasBasti/pySmartHashtag
Length of output: 944
🏁 Script executed:
cat -n pysmarthashtag/api/authentication.py | head -60Repository: DasBasti/pySmartHashtag
Length of output: 2466
🏁 Script executed:
cat -n pysmarthashtag/account.py | sed -n '109,121p'Repository: DasBasti/pySmartHashtag
Length of output: 743
🏁 Script executed:
rg "\.post\(" -A 5 pysmarthashtag/account.py | head -40Repository: DasBasti/pySmartHashtag
Length of output: 894
🏁 Script executed:
rg "response\.json\(" -B 3 -A 2 --type pyRepository: DasBasti/pySmartHashtag
Length of output: 3459
🏁 Script executed:
cat -n pysmarthashtag/api/authentication.py | grep -A 20 "class SmartLoginClient"Repository: DasBasti/pySmartHashtag
Length of output: 1096
🏁 Script executed:
rg "def post\(" pysmarthashtag/api/authentication.py -A 10Repository: DasBasti/pySmartHashtag
Length of output: 49
Add HTTP status checks for global vehicle list requests.
A non-2xx response will currently fall through to .json() and raise or produce misleading data.
🛠️ Suggested fix
vehicles_response = await client.post(
self.endpoint_urls.get_api_base_url() + path,
headers=headers,
content=body,
)
+ vehicles_response.raise_for_status()
data = vehicles_response.json()🤖 Prompt for AI Agents
In `@pysmarthashtag/account.py` around lines 122 - 153, The _init_vehicles_global
method currently calls vehicles_response.json() without checking HTTP status,
which can raise or return invalid data on non-2xx responses; update
_init_vehicles_global to check vehicles_response.status_code (or use
vehicles_response.is_success) after the client.post call and handle non-2xx
responses by logging an error with status and body (use sanitize_log_data for
body) and returning/raising appropriately before calling
vehicles_response.json(); ensure subsequent logic that reads data and calls
self.add_vehicle(...) only runs when the response is successful.
| async def _get_vehicle_details_global(self, vin) -> dict: | ||
| """Get global vehicle details.""" | ||
| _LOGGER.debug("Getting global vehicle details") | ||
| await self._ensure_ssl_context() | ||
| async with SmartLoginClient(ssl_context=self.config.ssl_context) as client: | ||
| path = "/vc/vehicle/v1/vehicleCustomerInfo" | ||
| body = json.dumps({"vin": vin}) | ||
| host = httpx.URL(self.endpoint_urls.get_api_base_url()).host | ||
| headers = utils.generate_global_header( | ||
| method="POST", | ||
| path=path, | ||
| host=host, | ||
| app_key=self.endpoint_urls.get_global_app_key(), | ||
| app_secret=self.endpoint_urls.get_global_app_secret(), | ||
| body=body, | ||
| access_token=self.config.authentication.access_token, | ||
| user_id=self.config.authentication.api_user_id, | ||
| id_token=self.config.authentication.id_token, | ||
| ) | ||
| response = await client.post( | ||
| self.endpoint_urls.get_api_base_url() + path, | ||
| headers=headers, | ||
| content=body, | ||
| ) | ||
| data = response.json() | ||
| details = data.get("result") or data.get("data") or [] | ||
| if isinstance(details, list): | ||
| details = details[0] if details else {} | ||
| if details: | ||
| self.vehicles.get(vin).combine_data(details) | ||
| return details or {} | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp/repo && find . -type f -name "*.py" | head -20Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && git ls-files "*.py" | head -30Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && wc -l pysmarthashtag/account.pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && sed -n '350,400p' pysmarthashtag/account.pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && find . -type f -name "*.py" -exec grep -l "class SmartLoginClient" {} \;Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "SmartLoginClient" --type pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "raise_for_status" --type pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "response\s*=\s*await.*\.post" --type py -A 5Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "response\s*\.json\(\)" --type py -B 3Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && grep -n "response\." pysmarthashtag/account.py | head -20Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "async def.*response\)|client\.post\(|client\.get\(" pysmarthashtag/account.py -A 2Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && sed -n '300,450p' pysmarthashtag/account.pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "async def _get.*vehicle" pysmarthashtag/account.py -A 15Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "\.post\(" pysmarthashtag/ --type py -A 5 -B 2Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "import httpx|from httpx" --type pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "class SmartLoginClient|SmartLoginClient\(" --type py -B 2 -A 10Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && find . -name "*.py" -type f -exec rg -l "SmartLoginClient" {} \;Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && git ls-files "*.py" | xargs grep -l "SmartLoginClient"Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && grep -r "class SmartLoginClient" .Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && head -50 pysmarthashtag/account.pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && find . -type f -name "*.py" | xargs grep "SmartLoginClient" 2>/dev/nullRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && ls -la pysmarthashtag/Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && cat pysmarthashtag/__init__.py | head -30Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && grep -n "from.*SmartLoginClient\|import.*SmartLoginClient" pysmarthashtag/account.pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && sed -n '1,100p' pysmarthashtag/account.pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && grep "SmartLoginClient" pysmarthashtag/*.pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "httpx\.(AsyncClient|Client)" --type py -B 2 -A 5Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "SmartLoginClient" -A 5Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && cat pysmarthashtag/account.py | grep -n "SmartLoginClient\|def _get_vehicle_details_global" -A 30 | head -50Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && cat pysmarthashtag/account.py | head -100Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && python3 << 'EOF'
import os
import re
# Find all Python files
for root, dirs, files in os.walk('.'):
for file in files:
if file.endswith('.py'):
filepath = os.path.join(root, file)
try:
with open(filepath, 'r') as f:
content = f.read()
if 'SmartLoginClient' in content:
print(f"Found in: {filepath}")
# Find the line number
lines = content.split('\n')
for i, line in enumerate(lines, 1):
if 'SmartLoginClient' in line:
print(f" Line {i}: {line.strip()}")
except:
pass
EOFRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && python3 << 'EOF'
with open('pysmarthashtag/account.py', 'r') as f:
lines = f.readlines()
# Print lines 350-400 with line numbers
for i in range(349, min(400, len(lines))):
print(f"{i+1:4d}: {lines[i]}", end='')
EOFRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && python3 << 'EOF'
import re
with open('pysmarthashtag/account.py', 'r') as f:
content = f.read()
# Find all response.json() calls
matches = re.finditer(r'response\s*=\s*await.*?(?=\n\s{8}(?:data|response|if|return|async|def|class))', content, re.DOTALL)
for match in matches:
# Check if response.raise_for_status() is called
text = match.group()
has_raise = 'raise_for_status' in text
if 'client.post' in text or 'client.get' in text:
lines = text.split('\n')
for line in lines[:10]:
print(line)
print(f" ... has raise_for_status: {has_raise}")
print("---")
EOFRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "await.*\.post\(|await.*\.get\(" pysmarthashtag/account.py -A 10 | head -80Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "response\.json\(\)" pysmarthashtag/account.py -B 10 -A 3Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "def _get_vehicle|def _fetch" pysmarthashtag/account.py -A 15Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "status_code|raise_for_status|HTTPError" pysmarthashtag/ --type pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && cat pyproject.toml | grep -i httpx -A 2 -B 2Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && cat requirements.txt 2>/dev/null || cat setup.py 2>/dev/null | grep -i httpx || echo "Not found in those files"Repository: DasBasti/pySmartHashtag
Length of output: 168
🏁 Script executed:
cd /tmp/repo && rg "\.status_code|is_success\(\)|is_error\(\)" --type pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "async def.*\(self" pysmarthashtag/account.py | head -20Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && python3 << 'EOF'
import re
with open('pysmarthashtag/account.py', 'r') as f:
lines = f.readlines()
# Find the method starting around line 359
in_method = False
for i in range(355, min(410, len(lines))):
line = lines[i]
if 'async def _get_vehicle_details_global' in line:
in_method = True
if in_method:
print(f"{i+1:4d}: {line}", end='')
if line.strip() and not line[0].isspace() and i > 355:
break
EOFRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "response\.status_code|response\.is_error\(\)|response\.is_success\(\)|response\.raise_for_status" pysmarthashtag/ --type py -B 2 -A 2Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && find . -name "*test*.py" -type f | head -10Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "raise_for_status|status_code" tests/ --type py 2>/dev/null | head -20Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && python3 << 'EOF'
import re
with open('pysmarthashtag/account.py', 'r') as f:
content = f.read()
# Find all async functions that use await and have response handling
pattern = r'async def\s+(\w+).*?(?=async def|\Z)'
matches = re.finditer(pattern, content, re.DOTALL)
for match in matches:
func_content = match.group()
func_name = match.group(1)
# Check if it has response.json() calls
if 'response.json()' in func_content or 'client.post' in func_content or 'client.get' in func_content:
# Check if it has any error handling
has_raise_for_status = 'raise_for_status' in func_content
has_status_check = 'status_code' in func_content or 'is_error' in func_content or 'is_success' in func_content
lines = func_content.split('\n')[:20] # First 20 lines
print(f"Function: {func_name}")
print(f" Has raise_for_status: {has_raise_for_status}")
print(f" Has status_code check: {has_status_check}")
if 'response.json()' in func_content:
print(f" Has response.json()")
print()
EOFRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && find . -name "requirements*.txt" -o -name "setup.py" -o -name "pyproject.toml" | xargs cat 2>/dev/null | grep -i httpx -A 1 -B 1Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && ls -la | grep -E "requirements|setup|pyproject"Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && cat pyproject.toml 2>/dev/null || cat setup.cfg 2>/dev/null | head -50Repository: DasBasti/pySmartHashtag
Length of output: 1124
🏁 Script executed:
cd /tmp/repo && find . -maxdepth 2 -name "*.toml" -o -name "*.cfg" -o -name "*.txt" | head -20Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && ls -laRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && cat pyproject.tomlRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && python3 << 'EOF'
with open('pysmarthashtag/account.py', 'r') as f:
lines = f.readlines()
# Show the method and a couple after it
start = None
for i, line in enumerate(lines):
if 'def _get_vehicle_details_global' in line:
start = i
break
if start:
# Print the method
indent_level = len(lines[start]) - len(lines[start].lstrip())
for i in range(start, min(start + 40, len(lines))):
print(f"{i+1:4d}: {lines[i]}", end='')
if i > start and lines[i].strip() and not lines[i].startswith(' ' * (indent_level + 1)) and 'def ' in lines[i]:
break
print("\n\n--- Next similar method ---\n")
# Find next similar method
for i in range(start + 35, min(start + 150, len(lines))):
if 'def _get_vehicle' in lines[i]:
for j in range(i, min(i + 40, len(lines))):
print(f"{j+1:4d}: {lines[j]}", end='')
if j > i and lines[j].strip() and not lines[j].startswith(' ' * (indent_level + 1)) and 'def ' in lines[j]:
break
break
EOFRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && find . -name "*.py" -path "*/test*" -type f | head -5Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "_get_vehicle_details_global" tests/ --type py -B 5 -A 10 2>/dev/null || echo "Method not found in tests"Repository: DasBasti/pySmartHashtag
Length of output: 169
🏁 Script executed:
cd /tmp/repo && rg "status.*=.*[0-9]{3}|status_code" tests/ --type py 2>/dev/null | head -20Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
python3 << 'EOF'
# Test what httpx does with error responses
import httpx
# Create a mock response with error status
response = httpx.Response(status_code=500, text='{"error": "server error"}')
print(f"Status code: {response.status_code}")
print(f"Is error: {response.is_error}")
print(f"Is success: {response.is_success}")
# Try to parse JSON from error response
try:
data = response.json()
print(f"JSON parsed successfully: {data}")
except Exception as e:
print(f"JSON parsing failed: {type(e).__name__}: {e}")
# Now try with a valid response
response_ok = httpx.Response(status_code=200, text='{"data": "test"}')
print(f"\nOK Status code: {response_ok.status_code}")
data_ok = response_ok.json()
print(f"OK JSON: {data_ok}")
EOFRepository: DasBasti/pySmartHashtag
Length of output: 215
🏁 Script executed:
cd /tmp/repo && grep -n "get_vehicle_details_global\|_get_vehicle_details_global" pysmarthashtag/account.pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && python3 << 'EOF'
with open('pysmarthashtag/account.py', 'r') as f:
content = f.read()
# Find where this method is called
import re
matches = list(re.finditer(r'_get_vehicle_details_global', content))
print(f"Found {len(matches)} occurrences of _get_vehicle_details_global")
# Get lines where it's called
lines = content.split('\n')
for i, line in enumerate(lines, 1):
if '_get_vehicle_details_global' in line and 'def _get_vehicle_details_global' not in line:
print(f"Line {i}: {line}")
EOFRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "_get_vehicle_details_global" pysmarthashtag/ -B 3 -A 3 --type pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
python3 << 'EOF'
# Test httpx behavior more thoroughly
import httpx
test_cases = [
{"status_code": 200, "content": b'{"result": "ok"}'},
{"status_code": 404, "content": b'{"error": "not found"}'},
{"status_code": 500, "content": b'{"error": "server error"}'},
{"status_code": 500, "content": b''}, # Empty response body
{"status_code": 403, "content": b'<html>Forbidden</html>'}, # HTML response
]
for case in test_cases:
response = httpx.Response(**case)
print(f"\nStatus {response.status_code}:")
# Try json()
try:
data = response.json()
print(f" json() succeeded: {data}")
except Exception as e:
print(f" json() failed: {type(e).__name__}: {str(e)[:50]}")
# Check if raise_for_status would raise
try:
response.raise_for_status()
print(f" raise_for_status(): OK")
except httpx.HTTPStatusError as e:
print(f" raise_for_status(): Would raise HTTPStatusError")
EOFRepository: DasBasti/pySmartHashtag
Length of output: 432
🏁 Script executed:
cd /tmp/repo && find . -name "*.py" -type f -path "*test*" -exec grep -l "get_vehicle_details_global\|_get_vehicle" {} \;Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && rg "status.*500|error.*response\|HTTPStatusError" --type py | head -20Repository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && sed -n '359,390p' pysmarthashtag/account.pyRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && python3 << 'EOF'
with open('pysmarthashtag/account.py', 'r') as f:
lines = f.readlines()
# Lines 359-390
for i in range(358, min(390, len(lines))):
print(f"{i+1:4d}: {lines[i]}", end='')
EOFRepository: DasBasti/pySmartHashtag
Length of output: 126
🏁 Script executed:
cd /tmp/repo && python3 << 'EOF'
# Simulate what happens when response has an error status but valid JSON
import json
# Case 1: Error response with empty result
error_response_data = {"result": None, "data": None}
details = error_response_data.get("result") or error_response_data.get("data") or []
if isinstance(details, list):
details = details[0] if details else {}
print(f"Case 1 (error with None/None): details = {details}")
print(f" Will be stored in vehicles.get(vin).combine_data(): {details == {}}")
# Case 2: Error response with error message
error_response_data2 = {"error": "Unauthorized", "result": None}
details2 = error_response_data2.get("result") or error_response_data2.get("data") or []
if isinstance(details2, list):
details2 = details2[0] if details2 else {}
print(f"\nCase 2 (error with error msg): details = {details2}")
print(f" Will be stored as: {details2 == {}}")
# Case 3: Success response
success_data = {"result": {"vin": "123", "name": "Car"}}
details3 = success_data.get("result") or success_data.get("data") or []
if isinstance(details3, list):
details3 = details3[0] if details3 else {}
print(f"\nCase 3 (success): details = {details3}")
EOFRepository: DasBasti/pySmartHashtag
Length of output: 126
Add status checks for global vehicle detail requests.
When the API returns an error status (4xx/5xx), the current code doesn't fail explicitly. Since response.json() succeeds on error responses, an error response gets processed as if no data was found, returning an empty dict. This masks the actual API failure.
Add response.raise_for_status() after the POST request to catch HTTP errors early and prevent silent failures.
🛠️ Suggested guard
response = await client.post(
self.endpoint_urls.get_api_base_url() + path,
headers=headers,
content=body,
)
+ response.raise_for_status()
data = response.json()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def _get_vehicle_details_global(self, vin) -> dict: | |
| """Get global vehicle details.""" | |
| _LOGGER.debug("Getting global vehicle details") | |
| await self._ensure_ssl_context() | |
| async with SmartLoginClient(ssl_context=self.config.ssl_context) as client: | |
| path = "/vc/vehicle/v1/vehicleCustomerInfo" | |
| body = json.dumps({"vin": vin}) | |
| host = httpx.URL(self.endpoint_urls.get_api_base_url()).host | |
| headers = utils.generate_global_header( | |
| method="POST", | |
| path=path, | |
| host=host, | |
| app_key=self.endpoint_urls.get_global_app_key(), | |
| app_secret=self.endpoint_urls.get_global_app_secret(), | |
| body=body, | |
| access_token=self.config.authentication.access_token, | |
| user_id=self.config.authentication.api_user_id, | |
| id_token=self.config.authentication.id_token, | |
| ) | |
| response = await client.post( | |
| self.endpoint_urls.get_api_base_url() + path, | |
| headers=headers, | |
| content=body, | |
| ) | |
| data = response.json() | |
| details = data.get("result") or data.get("data") or [] | |
| if isinstance(details, list): | |
| details = details[0] if details else {} | |
| if details: | |
| self.vehicles.get(vin).combine_data(details) | |
| return details or {} | |
| async def _get_vehicle_details_global(self, vin) -> dict: | |
| """Get global vehicle details.""" | |
| _LOGGER.debug("Getting global vehicle details") | |
| await self._ensure_ssl_context() | |
| async with SmartLoginClient(ssl_context=self.config.ssl_context) as client: | |
| path = "/vc/vehicle/v1/vehicleCustomerInfo" | |
| body = json.dumps({"vin": vin}) | |
| host = httpx.URL(self.endpoint_urls.get_api_base_url()).host | |
| headers = utils.generate_global_header( | |
| method="POST", | |
| path=path, | |
| host=host, | |
| app_key=self.endpoint_urls.get_global_app_key(), | |
| app_secret=self.endpoint_urls.get_global_app_secret(), | |
| body=body, | |
| access_token=self.config.authentication.access_token, | |
| user_id=self.config.authentication.api_user_id, | |
| id_token=self.config.authentication.id_token, | |
| ) | |
| response = await client.post( | |
| self.endpoint_urls.get_api_base_url() + path, | |
| headers=headers, | |
| content=body, | |
| ) | |
| response.raise_for_status() | |
| data = response.json() | |
| details = data.get("result") or data.get("data") or [] | |
| if isinstance(details, list): | |
| details = details[0] if details else {} | |
| if details: | |
| self.vehicles.get(vin).combine_data(details) | |
| return details or {} |
🤖 Prompt for AI Agents
In `@pysmarthashtag/account.py` around lines 359 - 390, The
_get_vehicle_details_global function currently ignores HTTP error statuses
because it calls response.json() without checking the response; after the await
client.post(...) call inside the SmartLoginClient context, call
response.raise_for_status() to raise on 4xx/5xx so errors are surfaced (before
parsing response.json()), leaving the rest of the logic (parsing data,
normalizing details, and calling self.vehicles.get(vin).combine_data(details))
unchanged.
| def _ensure_bytes(body: Optional[object]) -> Optional[bytes]: | ||
| if body is None: | ||
| return None | ||
| if isinstance(body, bytes): | ||
| return body | ||
| return str(body).encode("utf-8") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "utils.py" -path "*/api/*" | head -5Repository: DasBasti/pySmartHashtag
Length of output: 96
🏁 Script executed:
cat -n pysmarthashtag/api/utils.pyRepository: DasBasti/pySmartHashtag
Length of output: 7413
🏁 Script executed:
rg "_ensure_bytes" --context 10Repository: DasBasti/pySmartHashtag
Length of output: 2399
🏁 Script executed:
rg -i "hmac|signature|digest" -A 3 -B 3Repository: DasBasti/pySmartHashtag
Length of output: 12603
🏁 Script executed:
rg "generate_global_header" -B 3 -A 3Repository: DasBasti/pySmartHashtag
Length of output: 3006
🏁 Script executed:
rg -i "json\.dumps\|json\.loads" -B 2 -A 2Repository: DasBasti/pySmartHashtag
Length of output: 49
🏁 Script executed:
fd "\.py$" -x grep -l "generate_global_header" {} \; | head -10Repository: DasBasti/pySmartHashtag
Length of output: 163
🏁 Script executed:
cat -n pysmarthashtag/api/utils.py | sed -n '1,20p'Repository: DasBasti/pySmartHashtag
Length of output: 706
🏁 Script executed:
rg "body_bytes" -B 5 -A 5Repository: DasBasti/pySmartHashtag
Length of output: 905
🏁 Script executed:
rg "import json"Repository: DasBasti/pySmartHashtag
Length of output: 291
🏁 Script executed:
fd "test.*\.py$" -x grep -l "_ensure_bytes\|generate_global_header" {} \;Repository: DasBasti/pySmartHashtag
Length of output: 49
🏁 Script executed:
find . -name "test_*.py" -o -name "*_test.py" | xargs grep -l "ensure_bytes\|generate_global_header" 2>/dev/null || echo "No tests found"Repository: DasBasti/pySmartHashtag
Length of output: 81
🏁 Script executed:
cat -n pysmarthashtag/api/utils.py | sed -n '147,200p'Repository: DasBasti/pySmartHashtag
Length of output: 1963
Handle different body types explicitly in _ensure_bytes to prevent HMAC digest mismatches.
The current implementation accepts Optional[object] but uses str(body) to serialize non-bytes inputs. If callers pass dict/list objects directly (rather than pre-serialized JSON strings), str() produces Python repr syntax—not JSON—causing the MD5 digest in signature generation to mismatch the actual serialized request body. This breaks HMAC authentication silently.
All current callers pre-serialize using json.dumps(), but the permissive type hint allows misuse. Explicitly handle each type: strings encode directly, dict/list use json.dumps() with consistent separators, and unsupported types raise TypeError.
Suggested fix
+import json
def _ensure_bytes(body: Optional[object]) -> Optional[bytes]:
if body is None:
return None
if isinstance(body, bytes):
return body
- return str(body).encode("utf-8")
+ if isinstance(body, str):
+ return body.encode("utf-8")
+ if isinstance(body, (dict, list)):
+ return json.dumps(body, separators=(",", ":"), ensure_ascii=False).encode("utf-8")
+ raise TypeError(f"Unsupported body type for signing: {type(body)!r}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _ensure_bytes(body: Optional[object]) -> Optional[bytes]: | |
| if body is None: | |
| return None | |
| if isinstance(body, bytes): | |
| return body | |
| return str(body).encode("utf-8") | |
| import json | |
| def _ensure_bytes(body: Optional[object]) -> Optional[bytes]: | |
| if body is None: | |
| return None | |
| if isinstance(body, bytes): | |
| return body | |
| if isinstance(body, str): | |
| return body.encode("utf-8") | |
| if isinstance(body, (dict, list)): | |
| return json.dumps(body, separators=(",", ":"), ensure_ascii=False).encode("utf-8") | |
| raise TypeError(f"Unsupported body type for signing: {type(body)!r}") |
🤖 Prompt for AI Agents
In `@pysmarthashtag/api/utils.py` around lines 80 - 85, The _ensure_bytes function
currently casts arbitrary objects via str(), causing non-JSON reprs and HMAC
mismatches; update _ensure_bytes to explicitly handle bytes (return as-is), str
(encode utf-8), and dict/list (serialize with json.dumps(..., separators=(",",
":")) and encode utf-8), raise a TypeError for any other types, and narrow the
type hint accordingly so callers cannot accidentally pass unsupported objects
(refer to _ensure_bytes function name to locate the change).
| log_level = os.environ.get("SMART_LOG_LEVEL", "INFO").upper() | ||
| logging_config["loggers"]["pysmarthashtag"]["level"] = log_level |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the cli.py file around lines 59-60
wc -l pysmarthashtag/cli.pyRepository: DasBasti/pySmartHashtag
Length of output: 92
🏁 Script executed:
# Read the file with context around lines 59-60
head -100 pysmarthashtag/cli.py | tail -50Repository: DasBasti/pySmartHashtag
Length of output: 2057
🏁 Script executed:
# Search for dictConfig usage in the file
rg "dictConfig" -A 3 -B 3 pysmarthashtag/Repository: DasBasti/pySmartHashtag
Length of output: 525
🏁 Script executed:
# Look for any existing log level validation
rg -i "log.*level|SMART_LOG_LEVEL" -B 2 -A 2 pysmarthashtag/Repository: DasBasti/pySmartHashtag
Length of output: 6094
🏁 Script executed:
# Check Python logging behavior with invalid log levels
python3 << 'EOF'
import logging
import logging.config
# Test 1: Valid log level
try:
config = {
"version": 1,
"loggers": {
"test": {
"level": "DEBUG"
}
}
}
logging.config.dictConfig(config)
print("✓ Valid level (DEBUG) accepted")
except Exception as e:
print(f"✗ Valid level rejected: {e}")
# Test 2: Invalid log level string
try:
config = {
"version": 1,
"loggers": {
"test": {
"level": "INVALID_LEVEL"
}
}
}
logging.config.dictConfig(config)
print("✓ Invalid level (INVALID_LEVEL) accepted (silently)")
except Exception as e:
print(f"✗ Invalid level rejected: {type(e).__name__}: {e}")
# Test 3: Empty string
try:
config = {
"version": 1,
"loggers": {
"test": {
"level": ""
}
}
}
logging.config.dictConfig(config)
print("✓ Empty string accepted")
except Exception as e:
print(f"✗ Empty string rejected: {type(e).__name__}: {e}")
# Test 4: Numeric level string
try:
config = {
"version": 1,
"loggers": {
"test": {
"level": "20"
}
}
}
logging.config.dictConfig(config)
print("✓ Numeric string (20) accepted")
except Exception as e:
print(f"✗ Numeric string rejected: {type(e).__name__}: {e}")
EOFRepository: DasBasti/pySmartHashtag
Length of output: 313
🏁 Script executed:
# Check if there's any try-catch around dictConfig in the actual code
rg -B 10 "dictConfig" pysmarthashtag/cli.py | head -30Repository: DasBasti/pySmartHashtag
Length of output: 393
🏁 Script executed:
# Get the complete logging_config setup to understand the full context
rg -B 50 "log_level = os.environ.get" pysmarthashtag/cli.py | head -80Repository: DasBasti/pySmartHashtag
Length of output: 1680
Validate SMART_LOG_LEVEL to prevent startup failure on invalid values.
An invalid environment value will raise ValueError when dictConfig is called. Without validation, users setting invalid log levels (e.g., SMART_LOG_LEVEL=INVALID) will crash the application at startup.
🛠️ Suggested fix
log_level = os.environ.get("SMART_LOG_LEVEL", "INFO").upper()
+ if log_level not in {"CRITICAL", "ERROR", "WARNING", "INFO", "DEBUG", "NOTSET"}:
+ log_level = "INFO"
logging_config["loggers"]["pysmarthashtag"]["level"] = log_level
logging.config.dictConfig(logging_config)🤖 Prompt for AI Agents
In `@pysmarthashtag/cli.py` around lines 59 - 60, Validate the SMART_LOG_LEVEL
value before assigning it into logging_config to avoid ValueError during
dictConfig: read and upper() the env var as you already do, check it against the
set of known logging level names (e.g., via logging._nameToLevel or by using
logging.getLevelName and confirming it maps to a numeric level), and if it's
invalid fall back to "INFO" (and emit a warning via the pysmarthashtag logger or
stderr) before setting logging_config["loggers"]["pysmarthashtag"]["level"] =
log_level; ensure this validation logic is placed where log_level is computed so
dictConfig never receives an invalid level.
| GLOBAL_API_BASE_URL = "https://sg-app-api.smart.com" | ||
| GLOBAL_APP_KEY = "204587190" | ||
| GLOBAL_APP_SECRET = "vxnzkHbpQrkKKQKmFBZlOnL780rjXLFT" | ||
|
|
||
| EU_OAUTH_BASE_URL = "https://api.app-auth.srv.smart.com/v1/" | ||
| EU_OAUTH_API_KEY = "yHpsjnd9vzLq7GMowxBa" | ||
|
|
There was a problem hiding this comment.
Remove hardcoded Global/EU credentials from source.
The Global app secret, app key, and OAuth API key are embedded in code (Lines 18‑24). This is a security leak (also flagged by secret scanners). Move them to environment/config and rotate the exposed keys.
🔐 Proposed fix (move secrets to env)
-GLOBAL_APP_KEY = "204587190"
-GLOBAL_APP_SECRET = "vxnzkHbpQrkKKQKmFBZlOnL780rjXLFT"
+GLOBAL_APP_KEY = os.getenv("SMART_GLOBAL_APP_KEY")
+GLOBAL_APP_SECRET = os.getenv("SMART_GLOBAL_APP_SECRET")
-EU_OAUTH_API_KEY = "yHpsjnd9vzLq7GMowxBa"
+EU_OAUTH_API_KEY = os.getenv("SMART_EU_OAUTH_API_KEY")Also add:
import os🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 20-20: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 23-23: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Ruff (0.14.14)
[error] 20-20: Possible hardcoded password assigned to: "GLOBAL_APP_SECRET"
(S105)
🤖 Prompt for AI Agents
In `@pysmarthashtag/const.py` around lines 18 - 24, Remove the hardcoded
credentials and load them from environment/config: replace the literal values
for GLOBAL_APP_KEY, GLOBAL_APP_SECRET, and EU_OAUTH_API_KEY with
environment-based lookups (e.g., use os.environ.get or your config loader) and
keep GLOBAL_API_BASE_URL and EU_OAUTH_BASE_URL while allowing them to be
overridden from env if desired; add an import os at the top; ensure defaults are
not secrets (raise/configure error if required env vars are missing) and rotate
the exposed keys after deploying the change.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pysmarthashtag/const.py (1)
30-41:⚠️ Potential issue | 🟡 MinorDocument the new GLOBAL region in the SmartRegion docstring.
The docstring still enumerates only EU/INTL. Add GLOBAL so the public API docs stay accurate.
📝 Suggested doc update
@@ - - INTL: International region (Asia-Pacific) - for users with Hello Smart International app - (Australia, Singapore, and other international markets) + - INTL: International region (Asia-Pacific) - for users with Hello Smart International app + (Australia, Singapore, and other international markets) + - GLOBAL: Global app region (e.g., Australia/Israel) - uses global app endpoints
🧹 Nitpick comments (1)
pysmarthashtag/const.py (1)
172-188: Consider checking both v1 and v2 base URLs when inferring auth mode.If a caller overrides only
api_base_url_v2to the global endpoint, inference will still return EU OAuth and HMAC headers may be skipped. Checking both avoids that edge case.♻️ Suggested adjustment
def infer_auth_mode(self) -> SmartAuthMode: """Infer authentication mode based on endpoint URLs.""" - api_base_url = self.get_api_base_url() - if self._is_global_api_base_url(api_base_url): + api_base_url = self.get_api_base_url() + api_base_url_v2 = self.get_api_base_url_v2() + if self._is_global_api_base_url(api_base_url) or self._is_global_api_base_url(api_base_url_v2): return SmartAuthMode.GLOBAL_HMAC return SmartAuthMode.EU_OAUTH
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @DasBasti. * #161 (comment) The following files were modified: * `pysmarthashtag/account.py` * `pysmarthashtag/api/authentication.py` * `pysmarthashtag/api/utils.py` * `pysmarthashtag/cli.py` * `pysmarthashtag/const.py` * `pysmarthashtag/tests/common.py` * `pysmarthashtag/vehicle/vehicle.py`
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
📝 Add docstrings to `retry-international-auth`
Co-authored-by: DasBasti <1713093+DasBasti@users.noreply.github.com>
Co-authored-by: DasBasti <1713093+DasBasti@users.noreply.github.com>
- Created mock response files for global API endpoints - Added SmartGlobalMockRouter for global auth testing - Implemented comprehensive tests for _init_vehicles_global() and _update_global_vehicle_details() - All tests passing, no regression in existing tests Co-authored-by: DasBasti <1713093+DasBasti@users.noreply.github.com>
- Move json import to module level in common.py - Fix SmartMockRouter.add_global_routes to use side_effect handler - Rename prepare_global_account_with_vehicles to create_global_account_with_vehicles Co-authored-by: DasBasti <1713093+DasBasti@users.noreply.github.com>
- Remove trailing whitespace from blank lines - Apply ruff formatting Co-authored-by: DasBasti <1713093+DasBasti@users.noreply.github.com>
- Extract vehicle_details_handler to module-level _create_vehicle_details_handler function - Create shared _add_global_vehicle_routes helper function - Both SmartMockRouter and SmartGlobalMockRouter now use shared implementation Co-authored-by: DasBasti <1713093+DasBasti@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: DasBasti <1713093+DasBasti@users.noreply.github.com>
Add unit tests for Global HMAC authentication methods
Add test coverage for global authentication mode
…lization
Summary by CodeRabbit
New Features
Chores
Tests