From 57115dba2fae263b8923cdc5f4a827f49fa680e4 Mon Sep 17 00:00:00 2001 From: Ruben Herold Date: Tue, 4 Aug 2026 13:17:20 +0200 Subject: nat: T9162: fix KeyError when show nat rules has no inbound-interface _get_formatted_output_rules() and _get_formatted_output_statistics() detected an interface match by checking whether *any* expr in the rule has a left.meta field, then unconditionally read expr[0]'s match.right as the interface. Both assumptions are wrong: meta is also used for non-interface matches (e.g. meta l4proto from a protocol match), and the interface match is not guaranteed to be first. For a rule with a protocol match but no inbound-interface, this read the address/port match's right-hand side (a dict) as if it were the interface string, raising KeyError: 0 on interface[0] instead of falling back to 'any'. Search expr entries explicitly for a left.meta.key == 'iifname' match instead, and guard the '@' set-name stripping with a type check. --- src/op_mode/nat.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/op_mode/nat.py b/src/op_mode/nat.py index 434273622..32b62481e 100755 --- a/src/op_mode/nat.py +++ b/src/op_mode/nat.py @@ -142,9 +142,13 @@ def _get_formatted_output_rules(data, direction, family): rule_number = comment.split('-')[-1] rule_number = rule_number.split(' ')[0] if 'expr' in rule['rule']: - interface = rule.get('rule').get('expr')[0].get('match').get('right') \ - if jmespath.search('rule.expr[*].match.left.meta', rule) else 'any' - if interface[0] == '@': + interface = 'any' + for expr in rule.get('rule').get('expr'): + match = expr.get('match') + if match and jmespath.search('left.meta.key', match) == 'iifname': + interface = match.get('right') + break + if isinstance(interface, str) and interface.startswith('@'): interface = interface[3:] for index, match in enumerate(jmespath.search('rule.expr[*].match', rule)): if 'payload' in match['left']: @@ -257,8 +261,14 @@ def _get_formatted_output_statistics(data, direction): rule_number = comment.split('-')[-1] rule_number = rule_number.split(' ')[0] if 'expr' in rule['rule']: - interface = rule.get('rule').get('expr')[0].get('match').get('right') \ - if jmespath.search('rule.expr[*].match.left.meta', rule) else 'any' + interface = 'any' + for expr in rule.get('rule').get('expr'): + match = expr.get('match') + if match and jmespath.search('left.meta.key', match) == 'iifname': + interface = match.get('right') + break + if isinstance(interface, str) and interface.startswith('@'): + interface = interface[3:] packets = jmespath.search('rule.expr[*].counter.packets | [0]', rule) _bytes = jmespath.search('rule.expr[*].counter.bytes | [0]', rule) data_entries.append([rule_number, packets, _bytes, interface]) -- cgit v1.2.3 From 072404da629f3d50c9f22eb24366e799759b8830 Mon Sep 17 00:00:00 2001 From: Ruben Herold Date: Tue, 4 Aug 2026 13:36:47 +0200 Subject: nat: T9162: extract duplicated interface-lookup into _get_interface() _get_formatted_output_rules() and _get_formatted_output_statistics() contained an identical block searching rule['rule']['expr'] for the iifname match and stripping the '@I_' set-name prefix. Extract it into a shared _get_interface(rule) helper so future fixes only need to land in one place. Addresses CodeRabbit nitpick on PR #5375. --- src/op_mode/nat.py | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) (limited to 'src') diff --git a/src/op_mode/nat.py b/src/op_mode/nat.py index 32b62481e..c65d93c36 100755 --- a/src/op_mode/nat.py +++ b/src/op_mode/nat.py @@ -99,6 +99,18 @@ def _get_raw_translation(direction, family, address=None): return _xml_to_dict(xml) +def _get_interface(rule): + interface = 'any' + for expr in rule.get('rule').get('expr'): + match = expr.get('match') + if match and jmespath.search('left.meta.key', match) == 'iifname': + interface = match.get('right') + break + if isinstance(interface, str) and interface.startswith('@'): + interface = interface[3:] + return interface + + def _get_formatted_output_rules(data, direction, family): @@ -142,14 +154,7 @@ def _get_formatted_output_rules(data, direction, family): rule_number = comment.split('-')[-1] rule_number = rule_number.split(' ')[0] if 'expr' in rule['rule']: - interface = 'any' - for expr in rule.get('rule').get('expr'): - match = expr.get('match') - if match and jmespath.search('left.meta.key', match) == 'iifname': - interface = match.get('right') - break - if isinstance(interface, str) and interface.startswith('@'): - interface = interface[3:] + interface = _get_interface(rule) for index, match in enumerate(jmespath.search('rule.expr[*].match', rule)): if 'payload' in match['left']: # Handle NAT rule containing comma-separated list of ports @@ -261,14 +266,7 @@ def _get_formatted_output_statistics(data, direction): rule_number = comment.split('-')[-1] rule_number = rule_number.split(' ')[0] if 'expr' in rule['rule']: - interface = 'any' - for expr in rule.get('rule').get('expr'): - match = expr.get('match') - if match and jmespath.search('left.meta.key', match) == 'iifname': - interface = match.get('right') - break - if isinstance(interface, str) and interface.startswith('@'): - interface = interface[3:] + interface = _get_interface(rule) packets = jmespath.search('rule.expr[*].counter.packets | [0]', rule) _bytes = jmespath.search('rule.expr[*].counter.bytes | [0]', rule) data_entries.append([rule_number, packets, _bytes, interface]) -- cgit v1.2.3 From 12caa797e52a2c11b7b609d9db3297f8e1e7c9c9 Mon Sep 17 00:00:00 2001 From: Ruben Herold Date: Tue, 4 Aug 2026 20:58:17 +0200 Subject: nat: T9162: fix _get_interface() to also match oifname (Copilot review) CodeRabbit's DRY refactor (072404da6) only matched iifname, causing source NAT rules with an outbound-interface to regress to displaying 'any' instead of the configured interface. Also guard against missing 'rule'/'expr' keys. --- src/op_mode/nat.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/op_mode/nat.py b/src/op_mode/nat.py index c65d93c36..9ac7bb83f 100755 --- a/src/op_mode/nat.py +++ b/src/op_mode/nat.py @@ -101,9 +101,9 @@ def _get_raw_translation(direction, family, address=None): def _get_interface(rule): interface = 'any' - for expr in rule.get('rule').get('expr'): + for expr in rule.get('rule', {}).get('expr', []): match = expr.get('match') - if match and jmespath.search('left.meta.key', match) == 'iifname': + if match and jmespath.search('left.meta.key', match) in ('iifname', 'oifname'): interface = match.get('right') break if isinstance(interface, str) and interface.startswith('@'): -- cgit v1.2.3 From 345a7bb0f506f93afded24f4df3c53914498d0fd Mon Sep 17 00:00:00 2001 From: Ruben Herold Date: Wed, 5 Aug 2026 14:43:15 +0200 Subject: nat: T9162: remove unused conn_id/state locals flagged by ruff Pre-existing dead extractions in _get_formatted_output_statistics(), never used in the assembled row or headers. Whole-file ruff check blocks CI on any PR touching this file, including this one. --- src/op_mode/nat.py | 2 -- 1 file changed, 2 deletions(-) (limited to 'src') diff --git a/src/op_mode/nat.py b/src/op_mode/nat.py index 9ac7bb83f..86cdb5f3a 100755 --- a/src/op_mode/nat.py +++ b/src/op_mode/nat.py @@ -306,13 +306,11 @@ def _get_formatted_translation(dict_data, nat_direction, family, verbose): reply_dport = meta['layer4']['dport'] proto = meta['layer4']['protoname'] if direction == 'independent': - conn_id = meta['id'] timeout = meta.get('timeout', 'n/a') orig_src = f'{orig_src}:{orig_sport}' if orig_sport else orig_src orig_dst = f'{orig_dst}:{orig_dport}' if orig_dport else orig_dst reply_src = f'{reply_src}:{reply_sport}' if reply_sport else reply_src reply_dst = f'{reply_dst}:{reply_dport}' if reply_dport else reply_dst - state = meta['state'] if 'state' in meta else '' mark = meta.get('mark', '') zone = meta['zone'] if 'zone' in meta else '' if nat_direction == 'source': -- cgit v1.2.3