[imperva_cloud_waf] Tolerate no separator in log files

[chrisberkhout]

Proposed commit message

[imperva_cloud_waf] Tolerate no separator in log files

Make the CEL program tolerate log files that don't contain a
header/events separator (`|==|`).

The existing CEL program can fail with the error:

    failed evaluation: failed eval: ERROR: <input>:19:35: index out of
    bounds: 1 | ).as(v, v.next < size(v.worklist) ? |.................
    .................^

With this change, it will not fail, but no events will be returned.

Discussion

The observed error is for the index 1. The use of v.worklist[v.next] will never cause that, because those uses are all surrounded by the condition v.next < size(v.worklist), so it must be the access following split("|==|").

Log file structure documentation.

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.
• I have verified that any added dashboard complies with Kibana's Dashboard good practices

Screenshots

Here's a screenshot showing the problem:

[elasticmachine]

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

[elastic-vault-github-plugin-prod]

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: b62d4751abc1736e98509de22eb0e462cb5395fe

cc @chrisberkhout

[elastic-sonarqube]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[elastic-vault-github-plugin-prod]

Package imperva_cloud_waf - 1.5.1 containing this change is available at https://epr.elastic.co/package/imperva_cloud_waf/1.5.1/

[efd6]

Do we have a test instance for this (or a repro for the bug)?

I'd like to test this alternative:

diff --git a/packages/imperva_cloud_waf/data_stream/event/agent/stream/cel.yml.hbs b/packages/imperva_cloud_waf/data_stream/event/agent/stream/cel.yml.hbs
index b41d294a5d..fea7c770b0 100644
--- a/packages/imperva_cloud_waf/data_stream/event/agent/stream/cel.yml.hbs
+++ b/packages/imperva_cloud_waf/data_stream/event/agent/stream/cel.yml.hbs
@@ -30,7 +30,7 @@ program: |
         "worklist": (
           has(state.cursor) && has(state.cursor.log_file) && state.cursor.log_file != null
           ?
-            string(body).split(state.cursor.log_file)[1].split("\n").filter(x,x!="").map(x,{"filename":x})
+            string(body).split(state.cursor.log_file)[1].split("\n").map(x,x!="",{"filename":x})
           :
             string(body).split("\n").map(x,{"filename":x})
         ),
@@ -45,7 +45,9 @@ program: |
           "Authorization": ["Basic "+string(base64(state.user + ":" + state.password))],
         }
       }).do_request().as(resp, bytes(resp.Body).as(body, {
-        "events": string(body).split("|==|")[1].split("\n").filter(x,x!="").map(x,{"message":x}),
+        "events": string(body).as(body,
+            body.split("|==|")[?1].orValue(body).split("\n").map(x,x!="",{"message":x})
+        ),
         "cursor": {
           "log_file": (
             has(state.cursor) && has(state.cursor.log_file) && state.cursor.log_file != null

[chrisberkhout]

Do we have a test instance for this (or a repro for the bug)?
I'd like to test this alternative:

It could be tested with the system test by modifying the response data to have only the header (removing the separator and data lines), then asserting it runs error-free and returns no events.

We don't have the data from the live system, just the error message. Based on the code I think that the failed access of index 1 could only happen in the place I changed.

Your version looks nicer, except that the .orValue(body) should be .orValue(""), so the header data continues to be ignored.

[efd6]

It could be tested with the system test by modifying the response data to have only the header (removing the separator and data lines), then asserting it runs error-free and returns no events.

We currently have no system test. So I don't think this is possible this way, though I can temporarily set up a CEL input test.

Your version looks nicer, except that the .orValue(body) should be .orValue(""), so the header data continues to be ignored.

You are right.