[ti_eset] Add emailattachments data stream - #19138
Conversation
ReviewersBuildkite won't run for external contributors automatically; you need to add a comment:
NOTE: https://github.com/elastic/integrations/blob/main/.buildkite/pull-requests.json contains all those details. |
andrewkroh
added
Integration:ti_eset
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
6e98f63 to
d0e056a
Compare
|
Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as |
|
Tick the box to add this pull request to the merge queue (same as
|
|
👀 I have started reviewing the PR |
![vera-review-bot[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/3297022?s=60&v=4){kind=small}
|
This stream declares input:
|
@brijesh-elastic do we know why this PR (and other PRs I opened) are not passing automated review? |
|
@vera-review-bot review |
| size: 96x96 | ||
| type: image/svg+xml | ||
| vars: | ||
| - name: username |
There was a problem hiding this comment.
Severity: 🟠 High confidence: medium path: packages/ti_eset/manifest.yml:34
username/password move from the httpjson input to package-level vars, which existing 1.9.0 policies cannot carry over; treat this as a breaking change (major version + breaking-change changelog entry) or keep the input-level vars.
Details
This PR deletes the vars block from the httpjson input under policy_templates[0].inputs and re-declares username and password at package level. Fleet stores existing agent policy values against the scope they were declared in: 1.9.0 policies hold these values under inputs[].vars, and there is no package-level vars object to migrate them into. After upgrading to 1.10.0 the two package-level vars are required: true with no default, so upgraded policies come back with unset required credentials. The version bump is a minor one (1.9.0 -> 1.10.0) and the changelog entry does not mention it.
Recommendation:
If the move is intentional, ship it as a breaking change so users are warned and Fleet surfaces the reconfiguration:
- version: "2.0.0"
changes:
- description: Move the ETI username and password to package-level variables so they are shared by the httpjson and cel inputs. Existing agent policies must be reconfigured with the ETI credentials after upgrading.
type: breaking-change
link: https://github.com/elastic/integrations/pull/19138Otherwise keep the credentials on the httpjson input and declare the same two vars on the new cel input:
- type: cel
title: ETI New-Generation Feeds (TAXII 2.1)
description: Collection of ETI new-generation feeds
vars:
- name: username
type: text
title: ETI Username
required: true
show_user: true
- name: password
type: password
title: ETI Password
multi: false
required: true
secret: true
show_user: true🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
@polakovicp, can you please declare the same two vars on the new cel input instead of keeping it globally?
| target_field: eset.valid_until | ||
| formats: | ||
| - ISO8601 | ||
| if: ctx.eti?.valid_until != null |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: medium path: packages/ti_eset/data_stream/emailattachments/elasticsearch/ingest_pipeline/default.yml:79
threat.indicator.first_seen is declared in fields/ecs.yml but nothing populates it, while the STIX valid_from value is discarded; add a date processor mapping eti.valid_from to threat.indicator.first_seen.
Details
packages/ti_eset/data_stream/emailattachments/fields/ecs.yml declares threat.indicator.first_seen, but no processor in this pipeline writes it. The source objects do carry the value: the committed fixture _dev/test/pipeline/test-eset-ndjson.log contains "valid_from": "2024-03-18T14:15:42Z", and eti.valid_from is discarded by the remove_a0758d9e processor that drops the whole eti object. Consumers of the IOC index therefore never get a first-seen timestamp for these indicators.
Recommendation:
Parse valid_from next to the existing valid_until handling:
- date:
tag: date_eti_valid_from_to_threat_indicator_first_seen
field: eti.valid_from
target_field: threat.indicator.first_seen
formats:
- ISO8601
if: ctx.eti?.valid_from != null🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
correct. @polakovicp, can you populate the field instead of discarding? if its not the correct ECS match, then you can store it in custom field(maybe eset.valid_from).
There was a problem hiding this comment.
@brijesh-elastic since IoC does not contain information about first_seen, I populated eset.valid_from. threat.indicator.first_seen was removed from ecs
| allow_duplicates: false | ||
| if: ctx.error?.message != null | ||
| on_failure: | ||
| - append: |
There was a problem hiding this comment.
Severity: 🔵 Low confidence: high path: packages/ti_eset/data_stream/emailattachments/elasticsearch/ingest_pipeline/default.yml:178
Two of the three on_failure processors have no tag and the first one is indented with 6 spaces instead of 4; add tags and normalise the indentation.
Details
Every processor in the main processors list of this pipeline carries a tag, but in the on_failure block only the set has one; the append to error.message and the append to tags do not, which makes failures inside the failure handler harder to attribute. The first append also uses a 6-space body indent while the other two use 4, so elastic-package format will rewrite the block.
Recommendation:
Tag all three and use a consistent indent:
on_failure:
- append:
field: error.message
tag: append_error_message
value: |-
Processor '{{{ _ingest.on_failure_processor_type }}}'
{{{#_ingest.on_failure_processor_tag}}}with tag '{{{ _ingest.on_failure_processor_tag }}}'
{{{/_ingest.on_failure_processor_tag}}}failed with message '{{{ _ingest.on_failure_message }}}'
- set:
field: event.kind
tag: set_pipeline_error_to_event_kind
value: pipeline_error
- append:
field: tags
tag: append_preserve_original_event_tag
value: preserve_original_event
allow_duplicates: false🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
|
Hi @polakovicp, nice work on the PR! Could you run |
| size: 96x96 | ||
| type: image/svg+xml | ||
| vars: | ||
| - name: username |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: medium path: packages/ti_eset/manifest.yml:34
username/password move from the httpjson input's vars up to package-level vars, which changes where existing 1.9.x policies store them; either declare them per input instead, or record this as a breaking change in the changelog.
Details
This diff deletes the vars: block from the httpjson input of the eset policy template and re-declares username and password as package-level vars at the root of manifest.yml.
For policies already created on 1.9.x, these values are persisted against the httpjson input, not against the package policy. After upgrading to 1.10.0 the package schema no longer defines them at the input level and instead requires them at package level, where both are required: true with no default. The stored input-scoped values do not carry over to the new scope, so upgraded policies hit a required-but-unset variable rather than reusing the credentials the user already entered.
The changelog entry for 1.10.0 is typed enhancement and does not mention this relocation.
Recommendation:
Keep the vars scoped to each input so existing httpjson policies retain their values, and declare the same pair on the new cel input:
policy_templates:
- name: eset
title: ETI feeds (TAXII version 2.1)
description: Collect data from ETI feeds (TAXII version 2.1)
inputs:
- type: httpjson
title: ETI Legacy Feeds (TAXII 2.1)
description: Collection of ETI legacy feeds
vars: &eti_credentials
- name: username
type: text
title: ETI Username
required: true
show_user: true
- name: password
type: password
title: ETI Password
multi: false
required: true
secret: true
show_user: true
- type: cel
title: ETI New-Generation Feeds (TAXII 2.1)
description: Collection of ETI new-generation feeds
vars: *eti_credentialsIf the package-level move is intentional, keep it but record it explicitly:
- version: "1.10.0"
changes:
- description: Move `username` and `password` from the httpjson input to package-level variables. Existing policies must re-enter ETI credentials after upgrade.
type: breaking-change
link: https://github.com/elastic/integrations/pull/19138🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
@brijesh-elastic should this breaking change be addressed? Can I simply do it like this?
- version: "1.10.0"
changes:
- description: |-
PLACEHOLDER - specified once all related PRs will be merged to branch ti_set_polakovicp_enhancements
Move `username` and `password` from the httpjson input to package-level variables. Existing policies must re-enter ETI credentials after upgrade.
type: breaking-change, enhancement
link: TODOThere was a problem hiding this comment.
We should avoid introducing breaking-change as much as possible. You can avoid this by declaring the same variables in the cel input instead of keeping them globally in the manifest.
I updated description as requested |
| title: Sample logo | ||
| size: 96x96 | ||
| type: image/svg+xml | ||
| vars: |
There was a problem hiding this comment.
Severity: 🟠 High confidence: medium path: packages/ti_eset/manifest.yml:33
username/password are moved from the httpjson input's vars up to package-level vars, which changes where Fleet stores existing policy values; either keep them at the input level or ship this as a documented breaking change rather than a minor enhancement bump.
Details
This diff deletes the vars block from the httpjson input under policy_templates[0].inputs and re-declares username and password as package-level vars. Package-level vars are stored in a different place in the package policy than input-level vars, so the credential values configured by existing 1.9.x installations do not carry over to the new location. Both new vars are required: true with no default, so an upgraded policy has no value for them, and all seven existing httpjson data streams (apt, botnet, cc, domains, files, ip, url) render {{username}}/{{password}} from those vars. The package is nonetheless bumped 1.9.0 -> 1.10.0 with a changelog entry of type: enhancement, which does not signal to users that policy reconfiguration is needed. The new cel stream can read the credentials from either scope, so package-level placement is not required to add the emailattachments data stream.
Recommendation:
Either keep the credentials on the inputs that use them (and duplicate onto the new cel input):
policy_templates:
- name: eset
inputs:
- type: httpjson
title: ETI Legacy Feeds (TAXII 2.1)
vars: &eti_credentials
- name: username
type: text
title: ETI Username
required: true
show_user: true
- name: password
type: password
title: ETI Password
multi: false
required: true
secret: true
show_user: true
- type: cel
title: ETI New-Generation Feeds (TAXII 2.1)
vars: *eti_credentialsor, if the move to package scope is intentional, record it as a breaking change in changelog.yml:
- version: "2.0.0"
changes:
- description: Move ETI username and password to package-level variables. Existing agent policies must re-enter the credentials after upgrade.
type: breaking-change
link: https://github.com/elastic/integrations/pull/19138🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
0568535 to
d398f0e
Compare
d398f0e to
ce44d37
Compare
| name: ti_eset | ||
| title: "ESET Threat Intelligence" | ||
| version: "1.9.0" | ||
| version: "1.10.0" |
There was a problem hiding this comment.
Severity: 🟠 High confidence: medium path: packages/ti_eset/manifest.yml:12
Narrowing the Kibana constraint to ^9.4.0 drops support for 8.19/9.0/9.1 in a minor release; keep an 8.x branch in the range unless a 9.4-only feature is actually required.
Details
The constraint goes from ^8.19.4 || ~9.0.7 || ^9.1.4 to ^9.4.0, so every currently supported 8.19.x, 9.0.x, 9.1.x, 9.2.x and 9.3.x stack loses the ability to install or upgrade this integration, and the package version is only a minor bump (1.9.0 -> 1.10.0). The new agent features this PR introduces do not require it: run_as_cel, cel.program, cel.state and cel.redact are all documented for the httpjson input on the 8.19 branch (https://www.elastic.co/guide/en/beats/filebeat/8.19/filebeat-input-httpjson.html), and the cel input used by the new emailattachments stream predates 8.19 as well. If a specific 9.4-only fix is genuinely needed, that reason belongs in the changelog and the change must be recorded as a breaking change.
Recommendation:
Keep the 8.x branch in the constraint (raise the 8.x floor instead of removing it) unless a 9.4-only capability is documented:
conditions:
kibana:
version: "^8.19.4 || ~9.0.7 || ^9.1.4"
elastic:
subscription: "basic"🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
@polakovicp, Can you tell me what was the reason to keeping only ^9.4.0 constraint?
There was a problem hiding this comment.
Not sure, I reverted this back to "^8.19.4 || ~9.0.7 || ^9.1.4"
| size: 96x96 | ||
| type: image/svg+xml | ||
| vars: | ||
| - name: username |
There was a problem hiding this comment.
Severity: 🟠 High confidence: medium path: packages/ti_eset/manifest.yml:34
Moving username/password from the policy template input to package-level vars orphans the values stored in existing policies; keep them where they are or document the credential re-entry as a breaking change.
Details
Existing package policies persist these two vars under inputs[httpjson].vars. After this change the same names exist only at package scope, and Fleet copies var values by scope on upgrade - it has no aliasing between input-level and package-level vars. Both new vars are required: true with no default, so upgrading an existing 1.9.x policy leaves them unset: the policy fails validation on upgrade and, once resolved, the ETI feeds authenticate with empty credentials until the user re-enters them.
Recommendation:
Either declare the credentials on both inputs of the policy template so existing policies keep their values:
policy_templates:
- name: eset
inputs:
- type: httpjson
title: ETI Legacy Feeds (TAXII 2.1)
vars: &eti_credentials
- name: username
type: text
title: ETI Username
required: true
show_user: true
- name: password
type: password
title: ETI Password
multi: false
required: true
secret: true
show_user: true
- type: cel
title: ETI New-Generation Feeds (TAXII 2.1)
vars: *eti_credentialsor keep the package-level vars and record the migration in the changelog:
- version: "1.10.0"
changes:
- description: Move ETI credentials to package-level variables. Existing agent policies must re-enter the ETI username and password after upgrade.
type: breaking-change
link: https://github.com/elastic/integrations/pull/19138🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
| time: | ||
| field: eset.valid_until | ||
| max_age: 1m | ||
| source: |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: medium path: packages/ti_eset/elasticsearch/transform/emailattachments_latest_ioc/transform.yml:20
The transform source has no query, so ingest-error documents from the new pipeline flow into the latest IOC index; filter out error.message documents and cold/frozen tiers.
Details
The new ingest pipeline deliberately produces failure documents: its on_failure block sets error.message and event.kind: pipeline_error. With an unfiltered source those documents are picked up by the latest transform and surface under the logs-ti_eset_latest.emailattachments alias that consumers query for active IOCs. They also have no eset.id, so they all collapse onto a single entry of the event.dataset + eset.id unique key. Excluding cold and frozen tiers additionally keeps the transform from scanning expensive storage.
Recommendation:
Add a source query:
source:
index:
- logs-ti_eset.emailattachments-*
query:
bool:
must_not:
- exists:
field: error.message
- terms:
_tier:
- data_frozen
- data_cold🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
You could add the query below, which will prevent sending any agent error documents to the destination indices.
source:
index:
- logs-ti_eset.emailattachments-*
query:
bool:
must_not:
- exists:
field: error.message
| }, | ||
| "eset": { | ||
| "id": "indicator--00c42f20-62d2-4cb6-be87-2c451aaec4a4", | ||
| "labels": "malicious-activity", |
There was a problem hiding this comment.
Severity: 🔵 Low confidence: medium path: packages/ti_eset/data_stream/emailattachments/sample_event.json:25
sample_event.json stores eset.labels as a bare string while the pipeline always produces an array, so the committed sample (and the generated README) does not match real output; regenerate it with elastic-package test system -g.
Details
The foreach_eti_labels_ff89dc21 processor in elasticsearch/ingest_pipeline/default.yml uses an append sub-processor to build eset.labels, which always yields an array. This PR's own pipeline fixture agrees: _dev/test/pipeline/test-eset-ndjson.log-expected.json expects "labels": ["malicious-activity"] for the identical input document. Every sibling sample event stores the array form too, including data_stream/botnet/sample_event.json, which this branch regenerated with current tooling and which still has "labels": ["malicious-activity"] at the same position, so the tooling does not collapse single-element arrays. The scalar here therefore reflects a hand-edited file rather than one generated from a system-test document, and it is copied verbatim into the generated docs/README.md example event. There is no mapping impact because eset.labels is declared as keyword, but the published example misrepresents the shape users will query.
Recommendation:
Regenerate the file from a system test run (elastic-package test system -g -v) so it reflects the indexed document, and rebuild the docs. The eset block should read:
"eset": {
"id": "indicator--00c42f20-62d2-4cb6-be87-2c451aaec4a4",
"labels": [
"malicious-activity"
],
"valid_until": "2024-03-20T14:15:42.000Z"
},🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
@polakovicp, can you make sure that we don't change the auto-generated file?
| tag: set_threat_indicator_type_e912359f | ||
| field: threat.indicator.type | ||
| value: file | ||
| - set: |
There was a problem hiding this comment.
Severity: 🔵 Low confidence: medium path: packages/ti_eset/data_stream/emailattachments/elasticsearch/ingest_pipeline/default.yml:137
threat.indicator.name is populated with set + copy_from even though eti.name is discarded right afterwards; use a rename like the neighbouring eti.id and eti.description processors.
Details
This processor copies eti.name into threat.indicator.name and relies on the later remove_a0758d9e processor to delete the whole eti object. set with copy_from is the pattern for duplicating a value that must survive in both places (as set_threat_indicator_modified_at_5a74d882 correctly does for threat.indicator.last_seen), but here the source is thrown away, so the copy is a move. The two processors immediately above it, rename_eti_id_to_eset_id_e33c480b and rename_eti_description_to_threat_indicator_description_d13d7cb3, already use rename for exactly this situation. Using rename here removes the redundant if guard (ignore_missing covers it) and makes the pipeline's intent uniform. No behavioural change either way.
Recommendation:
Replace the set with a rename:
- rename:
tag: rename_eti_name_to_threat_indicator_name
field: eti.name
target_field: threat.indicator.name
ignore_missing: true🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
|
Tick the box to add this pull request to the merge queue (same as
|
cfd3bc6 to
e73fb1e
Compare
|
🟢 Reviewed the latest commits e73fb1e — nothing new beyond already posted comments. Review summaryIssues found across earlier commits d980032, ce44d37 — 3 low
Issues found across earlier commits d398f0e — 3 high, 2 medium, 1 low
Issues found across earlier commits 0568535 — 2 high, 1 medium, 1 low
Issues found across earlier commits 368dd6e, 70f0e2e — 3 high, 3 medium
Issues found across earlier commits d980032…12d3f09 (5 commits) — 2 high, 2 medium, 2 low
Issues found across earlier commits 6ac5a92…3bd470b (8 commits) — 1 high
Issues found across earlier commits 9262482 — 1 critical, 1 high
🤖 AI-Generated Review | Vera Review Bot - v0.2.7 | 📚 Knowledge base: integration-skills
|
3 participants
[ti_eset] Add emailattachments data stream
PR adds new Email attachments data stream to ti_eset integration. Documentation for this data stream is submitted as a separate PR
Related issues
Relates to PR #18709
Test result