[Threat Intel 3/11] HTML and article content parsing - #287199
[Threat Intel 3/11] HTML and article content parsing#287199stephmilovic wants to merge 61 commits into
Conversation
Third slice of the threat intel supply pipeline. Turns fetched feed content into the `content` shape a threat report stores. Nothing imports it yet and `threatIntelSupplyEnabled` defaults off, so it is inert. `text` strips markup, collapses whitespace, truncates to the stored-body cap, and builds the report `content` object. It also classifies section headers, which IOC extraction later uses to decide whether a value appeared in an indicators table or in prose. Script and style stripping matches end tags with attributes and arbitrary whitespace, since a regex that only handles `</script>` is not a sanitizer. `extract_article` picks the article body out of a full page, `canonicalize_url` normalizes URLs for dedup, and `severity` maps a level to the numeric score the report stores. One behaviour worth calling out: `buildReportContent` falls back to the title when the body is empty. Every enrichment route requires a non-empty text, so a report stored with no body can never be enriched, and it keeps getting picked up by the pending scan and failing. Title-only entries are common in feeds that carry a headline and a link, and a headline is thin but real input. The fallback lives here rather than in each adapter so all six get it. This layer sits below extraction and above the adapters, which is why it is its own directory rather than part of `adapters`: IOC extraction depends on it, and the adapters depend on both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
b19cfe2 to
38baf8c
Compare
|
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
kibanamachine
left a comment
There was a problem hiding this comment.
Libra found 4 issues.
Generated by Libra
Eight review findings. The article container was picked by first-match-of-first-selector, and two problems compounded: ARTICLE_SELECTORS puts `article` ahead of `main`, so any <article> beat a <main> holding the real report, and `.first()` then took the earliest of those. A page with an <article> teaser card above the body returned the teaser and every IOC in the report was missed. Candidates are now gathered across all selectors and the one with the most text wins, with selector order as a tie-break. Headings were classified before entity decoding, so `Indicators of Compromise` read as prose and its anchor hrefs were dropped rather than lifted. Every heading reset the section kind regardless of depth, so `<h2>Indicators of Compromise</h2><h3>Domains</h3>` fell back to prose at the subsection that actually holds the indicators. A deeper unclassified heading now inherits its parent section; a same-or-higher heading, or any explicitly classified one, still ends it. The href lift only matched quoted attributes. Unquoted href is valid HTML, and the generic tag stripper then removed the attribute, so an href-only IOC vanished. `ref` was stripped as a tracking param, but GitHub and GitLab use it for branch and commit refs and threat reports link to repos constantly, so it produced wrong canonical keys for exactly the URLs this pipeline sees most. Only unambiguous campaign values are stripped now. `www.com` is a registered domain and `slice(4)` turned it into the bare TLD `com`. Stripping now requires a dot after position 4. The three parser entry points took unbounded input from fetched pages inside a task worker. They cap at 10MB, matching the `body_html` bound the report API enforces. Truncating rather than throwing keeps a fat page degraded instead of failed. The title fallback for an empty body was silent, so the document was indistinguishable from one that genuinely repeats its title and enrichment paid to run inference over the same string twice. It sets `body_is_title_fallback` now. Also added the requested test coverage for `section_headers` and `severity`, and dropped an underscore prefix on a non-exported helper. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thanks @jonwalstedt, all eight addressed, details in the threads. Two worth surfacing here rather than leaving buried: The container-selection fix took two passes. My first attempt only picked the largest match within a selector, which misses the real problem: The |
kibanamachine
left a comment
There was a problem hiding this comment.
Libra found 3 issues.
Generated by Libra
…hor boundaries Three review findings, two of them gaps in fixes from the previous round. Container candidates were scored on raw text before chrome removal, so a teaser carrying a large inline script or style could outweigh the real report, win selection, and then be stripped to almost nothing. That drops the report and every IOC in it. Each candidate is now scored on a clone with the same chrome removed that the returned container gets. This is a gap in the cross-selector fix from the last round, which changed which candidates were compared but not how they were measured. `truncate` sliced to `maxLength` and then appended the ellipsis, so every truncated value came back one character over the cap. Callers pass the cap to satisfy a downstream length check, so being one over defeats the point. The ellipsis is now reserved inside the cap, with a zero or negative cap returning empty rather than a bare ellipsis. The anchor pattern had no attribute boundary before `href`, so the greedy prefix could run past `data-href="..."` and lift the tracker instead of the link. In an IOC section that both loses the real indicator and invents a false one. Requiring whitespace before `href` fixes it, and also allows whitespace around the equals sign, which is valid HTML. This one was made more reachable by accepting unquoted values in the previous round. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`NAMED_ENTITIES` was an object literal, so it inherited `Object.prototype` and a
lookup for `constructor`, `toString`, `valueOf`, `hasOwnProperty`, or `__proto__`
returned a real value. The `!== undefined` guard then accepted it, so feed HTML
containing `&constructor;` decoded to `function Object() { [native code] }` and
`&__proto__;` to `[object Object]`.
That text lands in `content.body_text`, which is what the LLM enrichment stages read
and what IOC extraction runs over, so untrusted feed markup could inject arbitrary
filler into a stored report with a nine-character string.
It is a `Map` now, which has no inherited keys. Found while re-reading the diff
rather than reported.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
All three addressed, details in the threads. Two of them were gaps in fixes I made in the previous round, which is worth naming: the candidate-scoring change compared the right candidates but still measured them wrong, and accepting unquoted hrefs widened what the greedy prefix could swallow, making the I also did another pass over the diff and found one more, unreported:
|
Mirror the elasticsearch-controller changes. Both _search_ai_lake_analyst and _search_ai_lake_soc_manager can run Detection Engine Indicator Match rules, which need read access to the per-space threat intel aliases.
Rules only need read access. Drop view_index_metadata for now, can be added in a follow-up if needed.
There was a problem hiding this comment.
Reviewed the content-parsing layer. The source modules are well-structured and thoroughly tested (including the Chromium fuzz oracle), and I found no concrete correctness, security, or coverage defects in them. One inline finding on accidentally committed local Bazel artifacts.
Generated by Claude Reviewer for #287199 · opus · 106.2 AIC · ⌖ 14.3 AIC · ⊞ 5.1K
Adapters in the RSS/STIX/TAXII (elastic#287205) and vendor_api (elastic#287206) slices import collapseWhitespace from content/text.ts. The content-parsing rewrite made it a private const, which breaks those imports once this PR merges. It's a plain string utility with real external callers, not part of the parsing-architecture change, so restore the export.
…ction_headers helper The content-parsing refactor (elastic#287199) internalized IOC_HEADER_TERMS, TERMINATOR_HEADER_TERMS, TERMINATOR_PREFIXES, and normalizeHeader behind a single classifyHeader() entry point that also added plural-aware and prefix-boundary-aware matching this file's own isIocHeader/isTerminatorHeader lacked. Those raw exports no longer exist, so this file's import breaks once elastic#287199 merges. Delegate to classifyHeader directly instead of re-implementing the same classification locally.
💛 Build succeeded, but was flaky
Failed CI Steps
Metrics [docs]
Test Failures
History
|
|
🤖 Jobs for this PR can be triggered through checkboxes. 🚧
ℹ️ To trigger the CI, please tick the checkbox below 👇
|
…ction_headers helper The content-parsing refactor (elastic#287199) internalized IOC_HEADER_TERMS, TERMINATOR_HEADER_TERMS, TERMINATOR_PREFIXES, and normalizeHeader behind a single classifyHeader() entry point that also added plural-aware and prefix-boundary-aware matching this file's own isIocHeader/isTerminatorHeader lacked. Those raw exports no longer exist, so this file's import breaks once elastic#287199 merges. Delegate to classifyHeader directly instead of re-implementing the same classification locally.
|
Closing per the TI MVP scope-narrowing decision. Arbitrary web-page / HTML article ingestion ( The small generic helpers that downstream PRs actually used have been transplanted onto their owning PRs (imports rewritten, no shared ancestry):
Nothing from this branch is lost — the pieces that remain in scope now live with their consumers. Closing rather than merging. |
…ction_headers helper The content-parsing refactor (elastic#287199) internalized IOC_HEADER_TERMS, TERMINATOR_HEADER_TERMS, TERMINATOR_PREFIXES, and normalizeHeader behind a single classifyHeader() entry point that also added plural-aware and prefix-boundary-aware matching this file's own isIocHeader/isTerminatorHeader lacked. Those raw exports no longer exist, so this file's import breaks once elastic#287199 merges. Delegate to classifyHeader directly instead of re-implementing the same classification locally.
…ction_headers helper The content-parsing refactor (elastic#287199) internalized IOC_HEADER_TERMS, TERMINATOR_HEADER_TERMS, TERMINATOR_PREFIXES, and normalizeHeader behind a single classifyHeader() entry point that also added plural-aware and prefix-boundary-aware matching this file's own isIocHeader/isTerminatorHeader lacked. Those raw exports no longer exist, so this file's import breaks once elastic#287199 merges. Delegate to classifyHeader directly instead of re-implementing the same classification locally.
…ction_headers helper The content-parsing refactor (elastic#287199) internalized IOC_HEADER_TERMS, TERMINATOR_HEADER_TERMS, TERMINATOR_PREFIXES, and normalizeHeader behind a single classifyHeader() entry point that also added plural-aware and prefix-boundary-aware matching this file's own isIocHeader/isTerminatorHeader lacked. Those raw exports no longer exist, so this file's import breaks once elastic#287199 merges. Delegate to classifyHeader directly instead of re-implementing the same classification locally.
…og API (#287204) ## Summary Adds LLM enrichment services and Threat Intelligence HTTP routes, including the fixed-catalog source list and enable/disable API. Manual report creation and provenance URLs go through the shared HTTP/HTTPS normalizer. Also wires the routes, inference features, and one-time bootstrap into `plugin.ts`, gated by `threatIntelSupplyEnabled`. This wiring is interim: it exists so the enrichment eval suite (see Evaluation below) has routes to call ahead of the full pipeline wiring. `#287207` replaces it with a proper `wiring.ts` module; that PR's description has a rebase note covering the `plugin.ts` conflict resolution. ## Where this sits | # | PR | Depends on | Status | |---|---|---|---| | 1 | #287197 workflow gate correction + fixtures | nothing | ✅ merged | | 2 | #287198 contracts, constants, shared libs | nothing | ✅ merged | | 3 | #287199 content parsing | #287198 | closed | | 4 | #287200 SSRF-guarded HTTP client | #287198 | ✅ merged | | 5 | #287201 index templates, seeding, inference features | #287198, #287200 | ✅ merged | | 6 | #287202 indicator alias | #287198, #287201 | ✅ merged | | 7 | #287203 IOC extraction | #287198, #287200 | ✅ merged | | 8 | #287204 LLM services, routes, source catalog API **← this PR** | #287198, #287200, #287202, #287203 | 👀 ready | | 9 | #287205 RSS adapter | #287198, #287200, #287203, #287204 | draft | | 10 | #287206 remaining adapters, dispatcher, fetch_source step | #287198, #287200, #287203, #287205 | draft | | 11 | #287207 promote/scrub tasks and plugin wiring | #287198–#287206 | draft | | 12 | #287479 generator fixture article URLs | nothing | ✅ merged | | 13 | #289343 Scout API tests for the deterministic routes | #287204 | draft | | 14 | #289345 enrichment eval suite | #287204 | draft | #287479 came first in the merge train and is already on `main`. The numbered series (#287197–#287207) stacks on top in dependency order. #287199 (content parsing) was closed; IOC extraction ships its own section-header classifier. ## Scope review follow-up - Shared `normalizeProvenanceUrl` helper (HTTP/HTTPS only, strips credentials, bounded length). - Source create/delete routes removed. Updates may change only `enabled`. - Lists and mutations reject catalog IDs outside the approved set. - Ensures the per-space indicator alias on `list_sources` (idempotent). ## Evaluation A `@kbn/evals` suite exercises all four enrichment stages against the BlackHat demo pack article text, posting each input directly to the internal route and scoring the structured response. It runs as a tracked baseline (scores recorded, no build-failing thresholds yet). Latest full run, EIS Claude Sonnet 4.6 as model and judge, 1 repetition, 20 examples: | Stage | Evaluator | Score | |---|---|---| | assess_relevance | IsIntelligenceMatch | 1.00 | | assess_relevance | RelevanceShapeValid | 1.00 | | classify_severity | SeverityExactMatch | 0.83 | | classify_severity | SeverityWithinOneLevel | 1.00 | | enrich_taxonomy | CategoryRecall | 0.80 | | enrich_taxonomy | RegionRecall | 1.00 | | extract_diamond | DiamondNoIocLeak | 1.00 | | extract_diamond | DiamondSignalCount | 1.00 | | extract_diamond | criteria (LLM judge, majority vote) | 1.00 | The suite ships in #289345, a separate eval-only PR that stacks on this one (it needs the routes and the flag-gated wiring added here). The four deterministic routes are covered separately by Scout API tests in #289343. Any prompt or calibration fix the evals surface lands by amending this PR, not the eval PR. ## To test ``` node scripts/jest --config x-pack/solutions/security/plugins/security_solution/server/threat_intel/jest.config.js routes/list_sources.test.ts services/provenance_url.test.ts ``` _PR developed with Cursor + Auto_ --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Jon Wålstedt <jon.walstedt@elastic.co> Co-authored-by: Cursor <cursoragent@cursor.com>
…d the flag (#287207) ## Summary Wires Threat Intelligence behind `threatIntelSupplyEnabled`: bootstrap, managed workflow installation, promote/scrub tasks, and route registration. Promote mirrors extracted IOCs into `.threat-intel-indicators` for Indicator Match rules. ## Where this sits | # | PR | Depends on | Status | |---|---|---|---| | 1 | #287197 workflow gate correction + fixtures | nothing | ✅ merged | | 2 | #287198 contracts, constants, shared libs | nothing | ✅ merged | | 3 | #287199 content parsing | #287198 | closed | | 4 | #287200 SSRF-guarded HTTP client | #287198 | ✅ merged | | 5 | #287201 index templates, seeding, inference features | #287198, #287200 | ✅ merged | | 6 | #287202 indicator alias | #287198, #287201 | ✅ merged | | 7 | #287203 IOC extraction | #287198, #287200 | ✅ merged | | 8 | #287204 LLM services, routes, source catalog API | #287198, #287200, #287202, #287203 | ✅ merged | | 9 | #287205 RSS adapter | #287198, #287200, #287203, #287204 | ✅ merged | | 10 | #287206 remaining adapters, dispatcher, fetch_source step | #287198, #287200, #287203, #287205 | ✅ merged | | 11 | #287207 promote/scrub tasks and plugin wiring **← this PR** | #287198–#287206 | ready for review | | 12 | #287479 generator fixture article URLs | nothing | ✅ merged | | 13 | #289343 Scout API tests for the deterministic routes | #287204 | ready for review | | 14 | #289345 enrichment eval suite | #287204 | ready for review | | 15 | #290144 read APIs, readiness, space-keyed attribution | #287207 | draft | #287479 came first in the merge train and is already on `main`. The numbered series (#287197–#287205) is merged; #287206 and #287207 now sit directly on `main`. #287199 (content parsing) was closed; IOC extraction ships its own section-header classifier. ## Scope review follow-up - Installs the managed workflows with the right space topology: `attribute_alerts_to_reports` installs per space (it queries `.alerts-security.alerts-*` and writes per-space hit totals, so a global install would clobber every space's totals onto one shared doc), while `ingest_threat_feeds` and `enrich_threat_report` install once globally. Alert analysis and threat intel share a single `ready()` call so neither install set is dropped, and spaces created after boot are reconciled on the promote task. All three ship `enabled: false`, and enrich routes its HTTP calls through a fixed space (`default`) instead of the install-time `workflow.spaceId` (which is `'*'` globally). - Ensures the default-space indicator alias on start. - Chunks promotion bulk writes and treats HTTP 408/500 as retryable bulk failures. - Sanitizes provenance and extracted IOC reference URLs during promotion. - Documents direct-index cross-space isolation as the blocker to enabling the feature. ## Bootstrap fix folded in from #289343 `seed_default_sources.ts` sorted the legacy-source disable scan on `_id`, which Elasticsearch rejects with `illegal_argument_exception: Fielddata access on the _id field is disallowed` unless `indices.id_field_data.enabled` is set, so it failed bootstrap on a stock cluster. Found this while getting #289343's Scout suite green and moved the fix here since this PR is the one wiring up bootstrap and already needs `security-threat-hunting` review, so it isn't adding a reviewer #289343 wouldn't otherwise need. ## Product boundary `threatIntelSupplyEnabled` defaults to false. Do not enable until direct-index cross-space isolation is hardened or the administrator trust model is explicitly accepted. ## To test ``` node scripts/jest --config x-pack/solutions/security/plugins/security_solution/server/threat_intel/jest.config.js wiring.test.ts tasks/promote_threat_indicators.test.ts node scripts/jest x-pack/solutions/security/plugins/security_solution/server/workflows/ node scripts/jest src/platform/packages/shared/kbn-workflows/managed/definitions/threat_intel/ ``` _PR developed with Cursor + Auto + Sonnet 5 + Opus 4.8_ --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Jon Wålstedt <jon.walstedt@elastic.co> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
DO NOT REVIEW! Considering alternative approach
Turns fetched feed content into the
contentshape a threat report stores. Nothing imports it yet.textparses markup through JSDOM, removes non-content and hidden subtrees, collapses whitespace, truncates to the stored-body cap, and builds the reportcontentobject. It also classifies section headers, which IOC extraction later uses to decide whether a value appeared in an indicators table or in prose. Inline visibility is evaluated withcss-tree, so malformed HTML and CSS follow parser behavior instead of custom lexical scanning.extract_articleuses Mozilla Readability to pick the article body out of a full page.canonicalize_urlnormalizes URLs for dedup, andseveritymaps a level to the numeric score the report stores.One behaviour worth a look:
buildReportContentfalls back to the title when the body is empty. Every enrichment route requires a non-empty text, so a report stored with no body can never be enriched, and it keeps getting picked up by the pending scan and failing. Title-only entries are common in feeds that carry a headline and a link, and a headline is thin but real input. The fallback lives here rather than in each adapter so all six get it.This layer sits below extraction and above the adapters, which is why it is its own directory rather than part of
adapters: IOC extraction depends on it, and the adapters depend on both.Where this sits
#287479 came first in the merge train and is already on
main. The numbered series (#287197–#287207) stacks on top in dependency order. #287199 (content parsing) was closed; IOC extraction ships its own section-header classifier.New third-party dependencies
css-tree@3.2.1display,visibility,all, priorities, escapes, or supportedvar()fallbacks.css-treealready appears transitively in the lockfile. This code imports it at server runtime, so it is declared directly and pinned to the repository-resolved version.@mozilla/readability@0.6.0@types/css-tree@2.3.10css-treeruntime API used by the inline-style evaluator.css-treedoes not ship the required TypeScript declarations. The direct runtime import needs checked types in Kibana's TypeScript build.css-tree; there is no overlapping type package. This is compile-time only.Inline CSS evaluation intentionally covers declarations and custom properties on the element itself. It does not load external stylesheets or inherit custom properties from ancestors. Unsupported CSS stays visible rather than being guessed as hidden.
To test
The content suites and scoped type check should pass.
PR developed with Claude Code + Claude Opus 5