fix(google-dtm): guard optional fields when building the alert content (#7360) - #7361
Open
Romain GUIGNARD (romain-filigran) wants to merge 2 commits into
Open
fix(google-dtm): guard optional fields when building the alert content (#7360)#7361Romain GUIGNARD (romain-filigran) wants to merge 2 commits into
Romain GUIGNARD (romain-filigran) wants to merge 2 commits into
Conversation
|
The following packages appear to be unused:
|
Copilot started reviewing on behalf of
Romain GUIGNARD (romain-filigran)
August 22, 2026 14:02
View session
Romain GUIGNARD (romain-filigran)
added
the
filigran team
|
🔴 Connector Linter errors detected
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the external-import/google-dtm connector against missing optional fields in DTM alert documents so markdown generation and channel creation don’t raise exceptions that would drop the alert.md attachment or abort processing.
Changes:
- Added
{}defaults to nested.get()chains in multiple markdown builders and increate_channel(). - Made
create_incident()safer by guardingdocaccess (e.g.,doc_type,"channel" in doc) outside the markdown-generationtry/except. - Added a new pytest module covering missing optional fields and ensuring
create_channel()doesn’t abort runs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| external-import/google-dtm/src/google_dtm_connector/converter_to_stix.py | Adds defaults to chained .get() access (markdown + channel creation) and guards some doc reads outside the try/except. |
| external-import/google-dtm/tests/tests_connector/test_converter_to_stix.py | Adds regression tests for missing optional fields and for create_channel() behavior when channel metadata is absent. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+190
to
+199
| - **Source File**: {dtm_alert_doc.get("source_file", {}).get("filename")} | ||
| - **MD5**: {dtm_alert_doc.get("source_file", {}).get("hashes", {}).get("md5")} | ||
| - **SHA1**: {dtm_alert_doc.get("source_file", {}).get("hashes", {}).get("sha1")} | ||
| - **SHA256**: {dtm_alert_doc.get("source_file", {}).get("hashes", {}).get("sha256")} | ||
| ### Content | ||
| - **Service URL**: {dtm_alert_doc.get("service_account").get("service").get("inet_location").get("domain")} | ||
| - **Service Domain**: {dtm_alert_doc.get("service_account").get("service").get("inet_location").get("url")} | ||
| - **Email Domain**: {dtm_alert_doc.get("service_account").get("email_domain")} | ||
| - **Login**: {dtm_alert_doc.get("service_account").get("login")} | ||
| - **Password**: {dtm_alert_doc.get("service_account").get("password").get("plain_text")} | ||
| - **Service URL**: {dtm_alert_doc.get("service_account", {}).get("service", {}).get("inet_location", {}).get("domain")} | ||
| - **Service Domain**: {dtm_alert_doc.get("service_account", {}).get("service", {}).get("inet_location", {}).get("url")} | ||
| - **Email Domain**: {dtm_alert_doc.get("service_account", {}).get("email_domain")} | ||
| - **Login**: {dtm_alert_doc.get("service_account", {}).get("login")} | ||
| - **Password**: {dtm_alert_doc.get("service_account", {}).get("password", {}).get("plain_text")} |
Comment on lines
+73
to
+84
| def test_account_discovery_without_a_plaintext_password(converter): | ||
| """The reported bug: service_account.password is absent, .get() raised.""" | ||
| stix_objects = converter.create_incident(_ACCOUNT_DISCOVERY_ALERT) | ||
|
|
||
| markdown = _alert_markdown(stix_objects) | ||
| converter.helper.connector_logger.error.assert_not_called() | ||
| # The absent value is rendered empty, the ones that are there are kept. | ||
| assert "- **Password**: None\n" in markdown | ||
| assert "- **Login**: jdoe@acme.com\n" in markdown | ||
| assert "- **MD5**: d41d8cd98f00b204e9800998ecf8427e\n" in markdown | ||
| assert "- **SHA256**: None\n" in markdown | ||
|
|
Copilot started reviewing on behalf of
Romain GUIGNARD (romain-filigran)
August 24, 2026 06:46
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
external-import/google-dtm/src/google_dtm_connector/converter_to_stix.py:76
- When
messengeris missing,channel_typebecomes "" and the ExternalReferencesource_namebecomes" - <channel_name>"(leading separator). This makes the external reference harder to read/search and can create empty/oddsource_namevalues when name/type are missing. Consider buildingsource_namefrom the non-empty parts (fallbacking toformatted_channel_name) so it never includes a leading/trailing separator.
formatted_channel_name = f"[{channel_type}] - {channel_name}"
external_refs = []
if channel_url:
external_ref = stix2.ExternalReference(
source_name=f"{channel_type} - {channel_name}", url=channel_url
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
{}default, in the six markdown builders and increate_channel().account_discoverywas the reported case (service_account.passwordabsent, line 197), butsource_file.hashes.*,service_account.service.inet_location.*,channel.channel_info.description,sender.identity.nameandfile_hashes.*had the same defect, somessageanddocument_analysisalerts were affected too.create_channel()is called outsidecreate_incident()'stry/except, so itschannel_infochain did not just lose the attachment: the exception reachedprocess_message()and aborted the run — no bundle sent, state not advanced, and the same alert killing every subsequent run. Its"[" + channel_type + "] - " + channel_nameconcatenations are now f-strings, since they raised the same way on a missingmessenger, and both parts default to""so the Channel is not named[None] - leaks.document_analysisandpasteget thedtm_alert_doc = dtm_alert.get("doc", {})line the four other builders already had, instead of repeatingdtm_alert.get("doc").doc_typeand the"channel" in …test increate_incident()readdocwith a default too, both being outside thetry.tests/tests_connector/test_converter_to_stix.py(12 tests). The connector's suite goes from 8 to 20 tests.Related issues
Checklist
Further comments