Skip to content

Commit fdded59

Browse files
Hasitha9796claude
andcommitted
fix(wazuh_decoder_rule_tool): let a retrieved <order> contribute fields, verified
ml_order was routed through select_requested_fields(), which intersects it against what the local extractor already found — so a retrieved decoder's <order> could only ever reorder fields, never contribute one. The field most worth having was exactly the one dropped: for an sshd-shaped failed login the retrieved decoder says srcuser,srcip and the extractor finds only srcip. Output was byte-identical whether the suggestion was correct, absent, or nonsense: ml_order=None -> order=['srcip'] ml_order=['srcuser','srcip'] -> order=['srcip'] ml_order=['totally','made','up'] -> order=['srcip'] propose_ml_order_fields() locates a value for each retrieved field name by its label, and a proposal survives only if the regex the generator would actually emit captures, in *every* sample log, exactly the value located in that log. A pattern that fits only the first sample is overfitting — the failure mode this path has to avoid — so it is rejected. Proposals join the pool via common_fields, deliberately NOT requested_fields: adding a name there flips selection to "requested only" and drops every heuristic fallback field, so proposing srcuser would have cost us srcip. osregex_captures() is new because osregex_matches() escapes parens and so reports no match for any pattern with a capture group — it could never have verified what a <regex> extracts. osregex_to_python() gains keep_groups, off by default so prematch verification keeps its existing meaning. Verified against real Wazuh (temporary install, logtest, removed): Phase 2: name: 'mlfixprobe' srcip: '192.168.1.50' srcuser: 'admin' Phase 2: name: 'mlfixprobe' srcip: '10.20.30.40' srcuser: 'carol' Measured by scripts/eval_ml_order_proposals.py against logtest's own Phase 2 output for 1546 verified samples: 131 fields recovered, 57 additions the official decoder did not have, 79 samples improved (5.1%). The remaining additions are dominated by name normalisation where the captured value is correct — checkpoint dstip where the official decoder said dst, cisco user where it said username — and that 57 overcounts, since srcuser scores as an addition when the official decoder emitted user for the same token. Three guards kill the genuinely-wrong class, each pinned by a test: * bare-space matching only for labels where it is the convention. "Unescaped URL path matches" yielded url="path"; "dst outside:116.6.127.120" yielded a dstip still carrying its interface prefix. * quoted values honoured after `:` as well as `=`, so action:"Key Install" is not truncated to "Key". * structural words (from, for, invalid, user…) rejected for space matches only. `Failed password for user from 172.18.1.1` names no user at all and was offering "from" as the srcuser; `status=unknown` stays a valid value. Also, the fuzzy affix fallback was relabelling values when fed retrieved names: `timezone` matched `time`, `dstname` matched `dst`, `srcmac` matched `src`, each emitting a decoder that captures a real value under a field name the log never supported. select_requested_fields() gains allow_affix, off for the ML paths and on by default so a human typing `ip` still finds `srcip`. Genuine synonyms that had been working only through that fallback (proto/protocol, act/action, username/user) are now explicit in FIELD_ALIASES. score_ml_decoder_template() scores strictly too — it was crediting templates for fields selection would then refuse, so templates ranked on matches they never had. Costs 1.31ms on a worst-case 21-name order, against the ~950ms the ML model already spends per analyze. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent bed7ef8 commit fdded59

3 files changed

Lines changed: 600 additions & 5 deletions

File tree

integrations/wazuh_decoder_rule_tool/app/main.py

Lines changed: 247 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -525,12 +525,19 @@ def prematch_osregex_from_current_logs(logs: List[str], *candidates: Optional[st
525525
}
526526

527527

528-
def osregex_to_python(pattern: str) -> str:
528+
def osregex_to_python(pattern: str, keep_groups: bool = False) -> str:
529529
"""Translate an OS_Regex pattern to a Python one, for verification only.
530530
531531
Wazuh's OS_Regex is not PCRE: `\\d` is a digit but `\\.` is *any char*, and
532532
`.` is a literal dot. Generated prematches are checked against the sample
533-
with this so a pattern that cannot match is never shipped."""
533+
with this so a pattern that cannot match is never shipped.
534+
535+
With keep_groups, bare `(`/`)` stay Python groups instead of becoming
536+
literal parens, so a <regex> with captures can be run to see what it would
537+
actually extract. Escaped `\\(` is still a literal paren either way, which
538+
matches OS_Regex. Default stays off: prematches carry no groups, and
539+
silently reinterpreting parens there would change what already-shipped
540+
verification means."""
534541
out: List[str] = []
535542
i = 0
536543
while i < len(pattern):
@@ -545,6 +552,8 @@ def osregex_to_python(pattern: str) -> str:
545552
out.append(char)
546553
elif char == "^":
547554
out.append("^" if not out else re.escape(char))
555+
elif keep_groups and char in "()":
556+
out.append(char)
548557
else:
549558
out.append(re.escape(char))
550559
i += 1
@@ -561,6 +570,23 @@ def osregex_matches(pattern: str, text: str) -> bool:
561570
return False
562571

563572

573+
def osregex_captures(pattern: str, text: str) -> Optional[Tuple[str, ...]]:
574+
"""What an OS_Regex <regex> would capture from text, or None if it misses.
575+
576+
Answers "would this decoder actually pull the value we think it would",
577+
which osregex_matches cannot: it escapes parens, so every pattern with a
578+
capture group reports no match."""
579+
if not pattern or text is None:
580+
return None
581+
try:
582+
match = re.search(osregex_to_python(pattern, keep_groups=True), text)
583+
except re.error:
584+
return None
585+
if match is None:
586+
return None
587+
return tuple(group if group is not None else "" for group in match.groups())
588+
589+
564590
# A vendor/product tag: LOGV3, APPAUTH, CEF, THREAT. Digits inside these are
565591
# part of the name (LOGV3 is not "LOGV" version 3), so they must survive
566592
# generalization or the prematch stops identifying the format.
@@ -1210,6 +1236,15 @@ def trailing_delimiter(end_idx: int) -> str:
12101236
"destinationport": ("destinationport", "dstport", "dpt"),
12111237
"dstport": ("destinationport", "dstport", "dpt", "port"),
12121238
"dvchost": ("dvchost",),
1239+
# True synonyms that were previously resolved only by the fuzzy affix
1240+
# fallback. Named explicitly so strict resolution (used for retrieved
1241+
# <order> names) still finds them, while `timezone`->`time` and
1242+
# `dstname`->`dst`, which that fallback also matched, stay rejected.
1243+
"protocol": ("protocol", "proto"),
1244+
"proto": ("proto", "protocol"),
1245+
"action": ("action", "act"),
1246+
"act": ("act", "action"),
1247+
"username": ("username", "user"),
12131248
}
12141249

12151250

@@ -1268,7 +1303,17 @@ def _affix_match(one: str, other: str) -> bool:
12681303
def select_requested_fields(
12691304
available_fields: Dict[str, str],
12701305
requested_fields: List[str],
1306+
allow_affix: bool = True,
12711307
) -> tuple[Dict[str, str], List[str]]:
1308+
"""Resolve requested field names against what the log actually offers.
1309+
1310+
allow_affix controls the last-resort fuzzy step. It is right for names a
1311+
*person* typed — `ip` should find `srcip` — but wrong for names that came
1312+
from a retrieved decoder, where it silently relabels values: a suggested
1313+
`timezone` affix-matched `time`, `dstname` matched `dst`, and `srcmac`
1314+
matched `src`, each emitting a decoder that captures a real value under a
1315+
field name the log never supported.
1316+
"""
12721317
selected: Dict[str, str] = {}
12731318
missing: List[str] = []
12741319
canonical_available = {canonicalize_field_name(name): name for name in available_fields}
@@ -1285,7 +1330,7 @@ def select_requested_fields(
12851330
if source_key and available_fields.get(source_key):
12861331
matched_key = source_key
12871332
break
1288-
if not matched_key:
1333+
if not matched_key and allow_affix:
12891334
for available_key in available_keys:
12901335
if _affix_match(canonical_name, available_key):
12911336
source_key = canonical_available.get(available_key)
@@ -1480,6 +1525,190 @@ def synthesize_requested_fields(
14801525
return synthesized
14811526

14821527

1528+
# Label spellings to look for in log text when a retrieved decoder names a
1529+
# field the local extractor missed. Keyed by canonical Wazuh field name; the
1530+
# field's own name and its FIELD_ALIASES are always tried too. Deliberately
1531+
# separate from FIELD_ALIASES, which drives selection semantics elsewhere —
1532+
# these are only ever used to *locate a value*, and every hit is verified
1533+
# against the generated regex before it can reach a decoder.
1534+
# Every label here must be a *noun that introduces its value*. Verbs and
1535+
# generic words look like labels and are not: "login" pulled `denied` out of
1536+
# `msg="Administrator login denied"` and offered it as srcuser. Words like
1537+
# "value", "info", "state" and "request" fail the same way, so none of them
1538+
# earn a place — a missed field costs nothing, a plausible wrong one ships.
1539+
_FIELD_LABEL_HINTS: Dict[str, Tuple[str, ...]] = {
1540+
"srcuser": ("srcuser", "username", "user", "logname", "account"),
1541+
"dstuser": ("dstuser", "username", "user", "account"),
1542+
"user": ("user", "username", "account", "logname"),
1543+
"srcip": ("srcip", "sourceip", "src", "client", "rhost"),
1544+
"dstip": ("dstip", "destinationip", "dst"),
1545+
"srcport": ("srcport", "sport", "spt"),
1546+
"dstport": ("dstport", "dport", "dpt"),
1547+
"action": ("action", "act"),
1548+
"status": ("status", "result", "outcome"),
1549+
"protocol": ("protocol", "proto"),
1550+
"url": ("url", "uri"),
1551+
"id": ("id", "sessionid"),
1552+
"command": ("command", "cmd"),
1553+
}
1554+
1555+
# A located value must look like a field value, not the rest of the line.
1556+
_MAX_LOCATED_VALUE_LEN = 120
1557+
1558+
# Labels where a bare space introduces the value ("invalid user admin", "port
1559+
# 54321"). Everywhere else a space is too weak to trust: "Unescaped URL path
1560+
# matches" yielded url="path", and "dst outside:116.6.127.120" yielded a dstip
1561+
# still carrying its interface prefix. Those need an explicit = or : separator.
1562+
_SPACE_SEPARATED_LABELS = frozenset({
1563+
"user", "username", "account", "logname", "srcuser", "dstuser",
1564+
"port", "srcport", "dstport", "sport", "dport", "spt", "dpt",
1565+
"from", "client", "rhost",
1566+
})
1567+
1568+
1569+
def _label_candidates(field_name: str) -> List[str]:
1570+
canonical = canonicalize_field_name(field_name)
1571+
labels: List[str] = []
1572+
for label in (
1573+
(field_name,)
1574+
+ FIELD_ALIASES.get(canonical, ())
1575+
+ _FIELD_LABEL_HINTS.get(canonical, ())
1576+
):
1577+
cleaned = (label or "").strip()
1578+
if cleaned and cleaned.lower() not in {existing.lower() for existing in labels}:
1579+
labels.append(cleaned)
1580+
# Longest first: `srcuser=` must win over the `user` substring inside it.
1581+
return sorted(labels, key=len, reverse=True)
1582+
1583+
1584+
# Words that structure a log line rather than carry a value. Only rejected for
1585+
# bare-space matches: `Failed password for user from 172.18.1.1` names no user
1586+
# at all, and the space form happily offered `from` as the srcuser. After an
1587+
# explicit `=` or `:` these are legitimate values (`status=unknown`).
1588+
_STRUCTURAL_WORDS = frozenset({
1589+
"a", "an", "and", "as", "at", "by", "for", "from", "in", "is", "of", "on",
1590+
"or", "the", "to", "via", "was", "with", "using", "invalid", "unknown",
1591+
"none", "null", "na", "user", "username", "account", "port", "host",
1592+
})
1593+
1594+
1595+
def _plausible_located_value(value: str, space_separated: bool = False) -> bool:
1596+
value = value.strip().strip("\"'")
1597+
if not value or len(value) > _MAX_LOCATED_VALUE_LEN:
1598+
return False
1599+
# Punctuation-only, or something that is plainly the next key rather than a
1600+
# value ("user action=deny" must not yield "action=deny").
1601+
if not re.search(r"[A-Za-z0-9]", value):
1602+
return False
1603+
if "=" in value or value.endswith(":"):
1604+
return False
1605+
if space_separated and value.lower() in _STRUCTURAL_WORDS:
1606+
return False
1607+
return True
1608+
1609+
1610+
def locate_field_value(log_line: str, field_name: str) -> Optional[str]:
1611+
"""Find the value a named field would take in this log, by its label.
1612+
1613+
Used only for field names proposed by retrieval — the log itself decides
1614+
whether the field exists at all. Returns None when no label in the log
1615+
plausibly introduces a value, which is the common case and must stay cheap.
1616+
"""
1617+
if not log_line or not field_name:
1618+
return None
1619+
1620+
for label in _label_candidates(field_name):
1621+
escaped = re.escape(label)
1622+
# Quoted forms first: `action:"Key Install"` must yield the whole value,
1623+
# not stop at the space and hand back "Key". Bare space is last and only
1624+
# for labels that conventionally use it.
1625+
templates = [
1626+
(rf'(?<![\w.]){escaped}\s*[=:]\s*"([^"]*)"', False),
1627+
(rf"(?<![\w.]){escaped}\s*[=:]\s*'([^']*)'", False),
1628+
(rf"(?<![\w.]){escaped}\s*=\s*([^\s,;]+)", False),
1629+
(rf"(?<![\w.]){escaped}\s*:\s*([^\s,;]+)", False),
1630+
]
1631+
if label.lower() in _SPACE_SEPARATED_LABELS:
1632+
templates.append((rf"(?<![\w.]){escaped}\s+([^\s,;]+)", True))
1633+
1634+
for template, space_separated in templates:
1635+
match = re.search(template, log_line, re.IGNORECASE)
1636+
if not match:
1637+
continue
1638+
value = match.group(1).strip().strip("\"'")
1639+
if _plausible_located_value(value, space_separated=space_separated):
1640+
return value
1641+
return None
1642+
1643+
1644+
def propose_ml_order_fields(
1645+
logs: List[str],
1646+
ml_order: Optional[List[str]],
1647+
common_fields: Dict[str, str],
1648+
) -> Dict[str, str]:
1649+
"""Fields a retrieved decoder names that the local extractor missed.
1650+
1651+
select_requested_fields() intersects ml_order against what the heuristics
1652+
already found, so a retrieved <order> could only ever reorder fields — it
1653+
could never contribute one. That silently dropped the field most worth
1654+
having: for an sshd failed-login the retrieved decoder says srcuser,srcip
1655+
and the extractor finds only srcip.
1656+
1657+
A proposal survives only if the regex the generator would actually emit for
1658+
it captures, in *every* sample log, exactly the value located in that log.
1659+
A pattern that only fits the first sample is overfitting, which is the
1660+
failure mode this whole path has to avoid, so it is rejected.
1661+
"""
1662+
proposals: Dict[str, str] = {}
1663+
sample_logs = [log for log in (logs or []) if log and log.strip()]
1664+
if not ml_order or not sample_logs:
1665+
return proposals
1666+
1667+
for raw_name in ml_order:
1668+
field_name = (raw_name or "").strip()
1669+
if not field_name:
1670+
continue
1671+
1672+
canonical = canonicalize_field_name(field_name)
1673+
if not canonical:
1674+
continue
1675+
# Already available, or already proposed under an equivalent spelling.
1676+
# Fuzzy matching stays ON here on purpose: this is the "don't capture
1677+
# the same value twice" guard, so an over-eager match suppresses a
1678+
# redundant proposal rather than inventing a field.
1679+
if select_requested_fields(common_fields, [field_name])[0]:
1680+
continue
1681+
if canonical in {canonicalize_field_name(name) for name in proposals}:
1682+
continue
1683+
1684+
located = [locate_field_value(log, field_name) for log in sample_logs]
1685+
if not all(located):
1686+
continue
1687+
1688+
candidate_pairs = build_split_regexes_from_fields(
1689+
sample_logs, {field_name: located[0]}
1690+
)
1691+
if len(candidate_pairs) != 1:
1692+
continue
1693+
regex, order = candidate_pairs[0]
1694+
if not regex or len(order) != 1:
1695+
continue
1696+
1697+
verified = True
1698+
for log, expected in zip(sample_logs, located):
1699+
# Phase 2 sees the post-pre-decode body, which is what
1700+
# build_split_regexes_from_fields anchored against.
1701+
body = (parse_phase1_predecode(log).get("body") or log).strip()
1702+
captures = osregex_captures(regex, body)
1703+
if not captures or captures[0] != expected:
1704+
verified = False
1705+
break
1706+
if verified:
1707+
proposals[field_name] = located[0]
1708+
1709+
return proposals
1710+
1711+
14831712
def choose_log_driven_fields(
14841713
logs: List[str],
14851714
requested_fields: List[str],
@@ -1503,8 +1732,16 @@ def choose_log_driven_fields(
15031732
if value and key not in common_fields:
15041733
common_fields[key] = value
15051734

1735+
# Verified retrieval proposals join the pool, but deliberately NOT
1736+
# requested_fields: adding a name there flips the branch below to
1737+
# "requested only" and drops every heuristic fallback field, so proposing
1738+
# srcuser would have cost us srcip. They enter as candidates that ml_order
1739+
# can then select, exactly like a field the extractor had found itself.
1740+
for name, value in propose_ml_order_fields(logs, ml_order, common_fields).items():
1741+
common_fields.setdefault(name, value)
1742+
15061743
selected_requested, missing_requested = select_requested_fields(common_fields, requested_fields)
1507-
ml_selected, _ = select_requested_fields(common_fields, ml_order or [])
1744+
ml_selected, _ = select_requested_fields(common_fields, ml_order or [], allow_affix=False)
15081745
fallback_fields = fields_excluding_noise(common_fields)
15091746

15101747
requested_canonical = {canonicalize_field_name(name) for name in (requested_fields or [])}
@@ -1854,7 +2091,12 @@ def score_ml_decoder_template(
18542091
if not ml_order:
18552092
return score
18562093

1857-
selected_ml_fields, _ = select_requested_fields(available_fields, ml_order)
2094+
# Strict, to match how these names are actually resolved during selection.
2095+
# Scoring them with affix matching credited a template for fields that
2096+
# selection would then refuse, so templates ranked on matches they never had.
2097+
selected_ml_fields, _ = select_requested_fields(
2098+
available_fields, ml_order, allow_affix=False
2099+
)
18582100
if not selected_ml_fields:
18592101
return 0.0
18602102

0 commit comments

Comments
 (0)