[ti_opencti] Fix processing of externalReferences

[chrisberkhout]

Proposed commit message

[ti_opencti] Fix processing of externalReferences (#8556)

The contents of externalReferences.edges were being processed with the
append processor and mistakenly flattened into scalar values. To fix
that they are now handled in Painless, as objects.

Additional checks were added for collection fields that may be null,
after some were observed in example data added to test the
externalReferences fix.

Checklist

• I have reviewed tips for building integrations and this pull request is aligned with them.
• I have verified that all data streams collect metrics or logs.
• I have added an entry to my package's changelog.yml file.
• I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

• Merge of Fix for issue #8425 ti_opencti integration causes field conflict #8428

How to test this PR locally

cd packages/ti_opencti
elastic-package stack up -d
elastic-package test -v

Related issues

• Closes ti_opencti parsing failures lead to dropped events #8457

[elasticmachine]

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2023-12-01T11:19:34.366+0000

• Duration: 15 min 20 sec

Test stats 🧪

Test	Results
Failed	0
Passed	23
Skipped	0
Total	23

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

[elasticmachine]

🌐 Coverage report

Name	Metrics % (covered/total)	Diff
Packages	100.0% (1/1)	💚
Files	50.0% (5/10)	👎 -50.0
Classes	50.0% (5/10)	👎 -50.0
Methods	59.615% (31/52)	👎 -37.915
Lines	83.313% (669/803)	👍 0.785
Conditionals	100.0% (0/0)	💚

[efd6]

Changelog and manifest updates?

[chrisberkhout]

Changelog and manifest updates?

Now done. I should have opened this as a draft, since I wanted to wait until community PR #8428 was merged.

[efd6]

This is not safe; the ?. operator stops the derefs being unsafe, but then we get to a look up for the method on null, and bang!

Suggested change

	if (!ctx.opencti?.indicator?.containsKey('external_reference')) {
	if (ctx.opencti?.indicator != null && !ctx.opencti.indicator.containsKey('external_reference')) {

[chrisberkhout]

Here the last null safe operator replaces a plain .containsKey() method call. My rule of thumb is that ?. will make the thing immediately to its right safe - either a field access or a method call.

This script returns "safely got a null":

POST _scripts/painless/_execute
{
  "script": {
    "params": {
      "key": null
    },
    "source": """
      if (params.key?.nonKey?.nonMethod("arg")?.nonMethod2("arg") == null) {
        return "safely got a null";
      }
    """
  }
}

[chrisberkhout]

Actually, I see there's a problem. The ...?.containsKey('external_reference') is okay, but null as the result of an if condition is not okay. So I fixed that and other occurrences like this

if (ctx.opencti?.indicator?.containsKey('external_reference') == true) {
  ...
}

[efd6]

Thanks, that's very informative; I've learned something new.

[efd6]

Thanks

[elasticmachine]

Package ti_opencti - 0.3.2 containing this change is available at https://epr.elastic.co/search?package=ti_opencti