Conversation
Summary of ChangesHello @gnishant-gupta, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces several updates and enhancements to the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly expands the functionality of TIPCommon by adding numerous data models and API client methods. The changes are a positive step, particularly the move towards safer dictionary access using .get(). However, there are several areas that could be improved for robustness and maintainability. My review focuses on improving error handling by replacing broad except Exception blocks with more specific ones, correcting type hint mismatches, and addressing potential runtime errors to enhance code correctness. Addressing these points will make the new additions more reliable.
Signed-off-by: gnishant <106644073+gnishant-gupta@users.noreply.github.com>
Signed-off-by: gnishant <106644073+gnishant-gupta@users.noreply.github.com>
…arketplace into tipcommon_gitsync_1p
Signed-off-by: gnishant <106644073+gnishant-gupta@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5f59817. Configure here.
| ) -> SingleJson: | ||
| """Add or update company logo.""" | ||
| api_client = get_soar_client(chronicle_soar) | ||
| api_client.params.company_logo = company_logo |
There was a problem hiding this comment.
Attribute mismatch: company_logo vs logo_data param name
High Severity
The add_or_update_company_logo function sets api_client.params.company_logo, but both the legacy and 1P API implementations read self.params.logo_data. This means the logo payload is never sent — logo_data will be unset on the Container, resulting in None or an AttributeError at runtime.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 5f59817. Configure here.
| response = api_client.update_blocklist() | ||
| try: | ||
| response = validate_response(response, validate_json=False) | ||
| return response |
There was a problem hiding this comment.
validate_response returns None, assigned and returned as response
High Severity
In update_blocklist, validate_response returns None (it has no return value), but its result is reassigned to response and then returned. This means on success, the function always returns None instead of a meaningful response dict, breaking the SingleJson return type contract.
Reviewed by Cursor Bugbot for commit 5f59817. Configure here.
|
|
||
| def get_all_model_block_records(self) -> requests.Response: | ||
| """Get all model block records.""" | ||
| endpoint: str = "settings/GetAllModelBlockRecords" |
There was a problem hiding this comment.
Missing leading slash in legacy endpoint paths
High Severity
The endpoints for get_all_model_block_records ("settings/GetAllModelBlockRecords") and get_company_logo ("settings/GetCompanyLogo") are missing their leading /. Every other endpoint in this file uses a leading slash. This causes the endpoint to be concatenated directly to the base URL without a path separator, producing a malformed URL that will fail.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5f59817. Configure here.
| payload = { | ||
| "id": self.params.id, | ||
| "name": self.params.name, | ||
| "type": self.params.type, |
There was a problem hiding this comment.
Legacy add_dynamic_env_param uses wrong parameter name for type
Medium Severity
The legacy add_dynamic_env_param builds the payload with "type": self.params.type, but the caller in soar_api.py sets api_client.params.parameter_type (not params.type). This means the type field in the payload will be unset or None, and the actual parameter type value is lost.
Reviewed by Cursor Bugbot for commit 5f59817. Configure here.
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
4 participants
Checklist:
Please ensure you have completed the following items before submitting your PR.
This helps us review your contribution faster and more efficiently.
General Checks:
Open-Source Specific Checks:
For Google Team Members and Reviewers Only:
Note
Medium Risk
Large surface-area change adding many new SOAR client endpoints and response parsers across both legacy and 1P clients; risk is mainly runtime breakage from API shape/URL differences and inconsistent response types.
Overview
Expands TIPCommon’s Chronicle SOAR API surface by adding many new wrappers in
rest/soar_api.pyfor system/settings, environments, integrations (install/export/import/configure), playbooks, case metadata (tags/stages/close reasons), networks/custom lists/blocklists, ontology/visual families/mapping rules, and simulated cases.Adds/updates data models in
data_models.pyto normalize legacy vs 1P JSON shapes (notably newEnvironment,IntegrationSetting,VisualFamily,OntologyRecord,CaseTag,Domain,CaseStage,CaseCloseReasons,CustomList,Blacklist,Network,SlaDefinition,SoarBlockEntity,SimulatedCases,BlockRecord) and makes parsing more defensive (InstalledIntegrationInstance,DynamicParameter).Updates platform clients (
api_client_factory.py,legacy_soar_api.py,one_platform_soar_api.py) with the corresponding new API methods and adds URL rewriting inBaseSoarApi._make_requestto handle 1P download/export path differences; bumps package version to2.3.6.Reviewed by Cursor Bugbot for commit 5f59817. Bugbot is set up for automated code reviews on this repo. Configure here.