feat: Zoho Mail #3214
feat: Zoho Mail #3214
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ 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}
| scope=scopes, | ||
| ) | ||
| print("\nToken response:") | ||
| print(json.dumps(token_resp, indent=2)) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 11 days ago
To fix this issue, we must avoid logging cleartext sensitive data such as OAuth tokens, the client secret, and refresh tokens or secrets that could be included in either a successful response or within error information.
The correct way is to:
- Never print the entire
token_respout to the logs. - Instead, explicitly print only non-sensitive subfields as needed (e.g., print e.g.
"Access token received"or the keys but not values, or simply suppress printing altogether unless debugging). - If details from the response must be shown, redact any sensitive fields like
access_token,refresh_token, and anyclient_secretorrequest_params/request_jsonobjects. - Specifically for the snippet, remove or replace the line
print(json.dumps(token_resp, indent=2))with a safe message, or print only a fixed set of non-sensitive keys (e.g. the keys present in the response).
Only make these changes in examples/toolkits/zoho_toolkit.py, as shown.
| @@ -118,7 +118,14 @@ | ||
| scope=scopes, | ||
| ) | ||
| print("\nToken response:") | ||
| print(json.dumps(token_resp, indent=2)) | ||
| # Do not print entire response as it may contain sensitive information. | ||
| # Only indicate success or reason for failure (if available). | ||
| if "access_token" in token_resp: | ||
| print("Access token received.") | ||
| elif "error" in token_resp: | ||
| print(f"Token request failed: {token_resp.get('error')}") | ||
| else: | ||
| print("Token response received, but access_token not found.") | ||
| access_token = token_resp.get("access_token") or "" | ||
| if not access_token: | ||
| print("Failed to obtain access token.") |
| on_subtask_completed (Optional[Callable[[Task], None]], optional): | ||
| Callback function to be called when a subtask is completed. | ||
| (default: :obj:`None`) | ||
| on_subtask_failed (Optional[Callable[[Task], None]], optional): | ||
| Callback function to be called when a subtask fails. | ||
| (default: :obj:`None`) |
There was a problem hiding this comment.
why we need to add these 2 argument in this PR, could you describe more for this change?
There was a problem hiding this comment.
I was working on this issue
, but when I saw that coolbeevip was already working on it, I decided to drop it. I’m not sure how I ended up committing, but I will remove it.
|
@Saedbhati while testing, I provided: $env:ZOHO_CLIENT_ID
$env:ZOHO_CLIENT_SECRET
$env:ZOHO_REDIRECT_URI
$env:ZOHO_TO_ADDRESSWhich I got from the api console (Workspace account): But I am facing this issue, the outh generation seems fine though. Is this the correct way to get started with the api? 
|
|
Program flow for me: |
|
@a7m-1st |
|
Oh all right, didn't realize that. I will try again later. Thanks @Saedbhati |
| "Content-Type": "application/json", | ||
| } | ||
| try: | ||
| response = requests.request( |
There was a problem hiding this comment.
Other toolkits include a timeout parameter here just in case, may be useful if you have time
| url = f"{self.base_url}/api/accounts/{self.account_id}/messages/search" | ||
| query = f"label:{tag_id}" | ||
| params: Dict[str, Any] = { | ||
| "query": query, |
There was a problem hiding this comment.
In line 758 you use searchKey, just wan't to double check this difference is intentional
| @staticmethod | ||
| def build_authorize_url( | ||
| *, | ||
| client_type: str, |
There was a problem hiding this comment.
Consider adding some input validation for client_type client_id and scope
| } | ||
| if scope: | ||
| params["scope"] = scope | ||
| return self._request_json("POST", url, params=params) |
There was a problem hiding this comment.
OAUTH 2.0 specification (RFC 6749) violation here may cause exposed client secrets susceptible to attack.
params is sent in URL query string as opposed to request body
| } | ||
| if scope: | ||
| params["scope"] = scope | ||
| return self._request_json("POST", url, params=params) |
There was a problem hiding this comment.
I believe this is the same violation as above
| description: Optional[str] = None, | ||
| due_in_epoch_ms: Optional[int] = None, | ||
| ) -> Dict[str, Any]: | ||
| r"""Action: Create Task (OAuth Scope: ZohoMail.tasks ) |
| description: Optional[str] = None, | ||
| due_in_epoch_ms: Optional[int] = None, | ||
| ) -> Dict[str, Any]: | ||
| r"""Action: Create Task (OAuth Scope: ZohoMail.tasks ) |
There was a problem hiding this comment.
| r"""Action: Create Task (OAuth Scope: ZohoMail.tasks ) | |
| r"""Action: Create Task (OAuth Scope: ZohoMail.tasks) |
| folder_id (str): Folder ID. | ||
| since_epoch_seconds (Optional[int]): Since epoch seconds. | ||
| limit (int): Limit. | ||
| sort_order (str): Sort order. |
There was a problem hiding this comment.
Possibly a more detailed description to help the agent understand what the Args mean, also missing default tags, see CONTRIBUTING.md
| def trigger_new_email_matching_search_poll( | ||
| self, | ||
| query: str, | ||
| folder_id: Optional[str] = None, |
| params["includeto"] = True | ||
| if received_time_ms is not None: | ||
| params["receivedTime"] = int(received_time_ms) | ||
| # Note: folderId is not supported in search API; omit to avoid |
There was a problem hiding this comment.
the next method uses folderId in the search API request so not sure whether this note is correct or not
Description
Describe your changes in detail (optional if the linked issue already contains a detailed description of the changes).
fixes #3203
Checklist
Go over all the following points, and put an
xin all the boxes that apply.Fixes #issue-numberin the PR description (required)pyproject.tomlanduv lockIf you are unsure about any of these, don't hesitate to ask. We are here to help!