Repository navigation
Conversation
d7aa3c0 to
44bf0a4
Compare
✅ Elastic Docs Style Checker (Vale)No issues found on modified lines! The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale. |
🚀 Benchmarks reportTo see the full report comment with |
|
Pinging @elastic/integration-experience (Team:Integration-Experience) |
44bf0a4 to
8dc4174
Compare
|
#20030 has been merged, could you rebase? |
8dc4174 to
1bff832
Compare
|
Thanks @taylor-swanson. Rebased on main following the #20030 and #20031 merge, #20085 and #20088 draft were re-stacked accordingly. |
1bff832 to
f99deb3
Compare
Review summaryIssues found across earlier commits 4ce4236, 8dc4174 — 7 medium
Issues found across earlier commits 44bf0a4 — 7 medium, 1 low
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
|
7f67a83 to
4ed0f60
Compare
…metadata allow-list Derive `event.outcome` on the auth, server and xvpn families from the vendor `error` return code (`0` maps to success, other codes to failure). The official field documentation lists exactly these three families for `error`; on vpn logs the same key is a severity level and is left untouched. The vendor documentation pairs `0`-first numeric examples with `Success`-first display labels without stating the convention outright, so the mapping follows the strong implication of the field tables. Map `domain` to `user.domain` and `usergroup` to `user.group.name`, splitting the comma-separated multi-valued form that SNS 5.x emits. On web logs, map `op` to `http.request.method` and `result` to `http.response.status_code`; on other families these keys carry protocol-specific codes (FTP, Modbus) and are not promoted. Map the module name in `service` to `process.name` on the system, sandboxing and routing families only — on pvm logs the same key names the vulnerable product detected on a scanned host. Set `observer.product`. All mappings are copies; the vendor keys remain available under `stormshield.metadata`. Extend the `stormshield.metadata` allow-list with the documented vendor keys that previously leaked as undeclared `stormshield.*` fields: host and IP reputation (`srchostrep`, `dsthostrep`, `srciprep`, `dstiprep`), alarm packet captures (`pktlen`, `pktdump`, `pktdumplen`), Ethernet protocol (`etherproto`), SSO agent (`agentid`), IKE peer name (`peername`), and the industrial-protocol plugin fields (`unitid`/`unit_id`, `cipservicecode`, `cipclassid`, `requestmode`, `responsemode`, `UI`, `error_class`, `error_code`, `format`). The NAT reading of the pipeline was verified against the official field tables while preparing this change: `origdst`/`origdstport` are documented as the destination before translation and map to `destination.ip`/`destination.port`; `dst` is the translated destination and maps to `destination.nat.ip` when it differs. A connection sample with the documented example values now locks this in the pipeline tests, alongside new samples for the alarm reputation and packet-capture fields, a Modbus plugin line (whose `result` stays under metadata and does not produce `http.*`), a server line, a system line, and multi- and single-valued `usergroup` forms.
78bfed9 to
cb25778
Compare
|
✅ All changelog entries have the correct PR link. |
💚 Build Succeeded
History
|
taylor-swanson
left a comment
There was a problem hiding this comment.
A few minor comments, otherwise it's looking good to me.
| ignore_empty_value: true | ||
| if: ctx.stormshield?.domain != null |
There was a problem hiding this comment.
These two lines do the same thing (the if statement and ignore_empty_value), so only one of them is needed.
I don't have a strong preference on which one to keep. ignore_empty_value shows the intent better than an if statement, but the if statement tends to be more performant.
| ignore_empty_value: true | ||
| if: ctx.stormshield?.usergroup != null |
There was a problem hiding this comment.
These two lines do the same thing, so only one of them is needed.
| ignore_empty_value: true | ||
| if: ctx.stormshield?.service != null && ['system', 'sandboxing', 'routing'].contains(ctx.stormshield.logtype) |
There was a problem hiding this comment.
In this case, we can get rid of ignore_empty_value: true, since the if statement already does this (and does an additional check).
| ignore_empty_value: true | |
| if: ctx.stormshield?.service != null && ['system', 'sandboxing', 'routing'].contains(ctx.stormshield.logtype) | |
| if: ctx.stormshield?.service != null && ['system', 'sandboxing', 'routing'].contains(ctx.stormshield.logtype) |
|
Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as |
|
@arenard - This PR has merge conflicts with the base branch. Please resolve them. |
|
Hi! This PR has been stale for a while and we're going to close it as part of our cleanup procedure. We appreciate your contribution and would like to apologize if we have not been able to review it, due to the current heavy load of the team. Feel free to re-open this PR if you think it should stay open and is worth rebasing. Thank you for your contribution! |
Proposed commit message
Derive
event.outcomeon the auth, server and xvpn families from the vendorerrorreturn code (0maps to success, other codes to failure). The official field documentation lists exactly these three families forerror; on vpn logs the same key is a severity level and is left untouched. The vendor documentation pairs0-first numeric examples withSuccess-first display labels without stating the convention outright, so the mapping follows the strong implication of the field tables.Map
domaintouser.domainandusergrouptouser.group.name, splitting the comma-separated multi-valued form that SNS 5.x emits. On web logs, mapoptohttp.request.methodandresulttohttp.response.status_code; on other families these keys carry protocol-specific codes (FTP, Modbus) and are not promoted. Map the module name inservicetoprocess.nameon the system, sandboxing and routing families only; on pvm logs the same key names the vulnerable product detected on a scanned host. Setobserver.product. All mappings are copies; the vendor keys remain available understormshield.metadata.Extend the
stormshield.metadataallow-list with the documented vendor keys that previously leaked as undeclaredstormshield.*fields: host and IP reputation (srchostrep,dsthostrep,srciprep,dstiprep), alarm packet captures (pktlen,pktdump,pktdumplen), Ethernet protocol (etherproto), SSO agent (agentid), IKE peer name (peername), and the industrial-protocol plugin fields (unitid/unit_id,cipservicecode,cipclassid,requestmode,responsemode,UI,error_class,error_code,format).The NAT reading of the pipeline was verified against the official field tables while preparing this change:
origdst/origdstportare documented as the destination before translation and map todestination.ip/destination.port;dstis the translated destination and maps todestination.nat.ipwhen it differs. A connection sample with the documented example values now locks this in the pipeline tests, alongside new samples for the alarm reputation and packet-capture fields, a Modbus plugin line (whoseresultstays under metadata and does not producehttp.*), a server line, a system line, and multi- and single-valuedusergroupforms.Design notes
Affected logslists drive the family guards). The alarmclasssub-categorization was deliberately left out: the documentation gives example values without an exhaustive list or definitions.event.outcomeis only set where the log reports a return code, never inferred from message text.stormshield.*field changes location or type, so existing dashboards and saved queries are unaffected.Checklist
changelog.ymlfile.Author's Checklist
elastic-package check,test pipeline,test staticandtest systempass against stack 9.4.3result,service,error)How to test this PR locally
Related issues