fix(snmp-discovery): stub the device ref inside primary-IP snapshots

[leoparente]

Problem

One device appears twice in a single ingest payload with two different device_type.model values, and NetBox rejects the whole device changeset with an __all__ validation error because the merge cannot pick a winner.

device.device_type.model                                        = WS-C2960X-48LPS-L
device.primary_ip4.assigned_object_interface.device.device_type.model = cat29xxStack

The top-level value is the correct one: WS-C2960X-48LPS-L is the ENTITY-MIB product model, cat29xxStack is the CISCO-PRODUCTS-MIB label for the sysObjectID, i.e. a platform family. So the stack path is improving the model; it just fails to carry the improvement everywhere.

Root cause

Attaching the primary IP shallow-copies the Device into Device.PrimaryIp4/6.AssignedObject.(*Interface).Device to break the Device → IP → Interface → Device cycle. Later stages mutate the rich Device — the stack path replaces device_type, the annotators stamp source_match/run_id, name suppression clears Name. PruneNestedRefs rebuilds every other nested device ref into a matcher-only stub and runs after all of them, but skipped this one subtree:

case *diode.Device:
    // Top-level rich Devices stay rich.
    continue

That the machinery is already correct and only its coverage was wrong is visible in the failing payload: of 122 device refs, the 121 that pass through the prune carry 4-5 fields and the correct model; the 1 it skips carries 8 fields, the stale model, and no source_match.

What this fixes

Four defects share that one gap:

Symptom	Evidence
device_type diverges	the reported failure, reproduced from the customer payload
emit_device_name: false is defeated — the snapshot re-supplies the hostname the operator asked to suppress	reproduced by test
source_match missing, so two representations of one device resolve through different matcher paths	customer payload (snapshot carries no metadata)
the snapshot keeps the master as owner while the live interface is routed to a member — one port claimed by two devices, and two devices claiming one unique_primary_ip4 (matcher #2)	reproduced by test; latent in the customer data only because their primary sits on a Vlan that routes to the master

Design notes

The owner is resolved by address, not by interface name. IP-assigned interfaces are deliberately excluded from top-level emission, so the by-name index structurally cannot contain the interface carrying the primary — in the customer payload that interface is Vlan12, absent from all 118 top-level interfaces. An earlier revision resolved by name; it was rejected because a member exposing a same-named interface (per-member me0/mgmt0) could capture the lookup and bind the primary to an unrelated device. An address claimed by more than one interface is treated as ambiguous and left alone.

Tags is deliberately not carried on the interface stub. An earlier revision added it, on the reasoning that the stub becomes the only wire payload for an IP-assigned interface. Measured across all 52 captures, that framing was wrong: the primary-IP snapshot interface carries tags on 52/52 devices, but the other 80 nested IP-assigned refs carry them on 0/80. So adding Tags to the shared stub would start tagging 80 interfaces per run that are untagged today — a product behaviour change riding along in a device_type fix. Diode applies updates with partial=True, so omitting the field never strips tags from an interface that already has them; only first-time creation comes up untagged, which is already the behaviour for every other IP-assigned interface. The 0/80 gap is worth its own ticket.

Nulling the snapshot's device_type was considered and rejected. "Absent-in-one reconciles" only holds for nodes that share a fingerprint. Three reachable configurations produce disjoint matchers — emit_device_name: false with netbox_id, the same with discover_asset_tags: true, and any device with an empty sysName (common on cameras and printers) — and there the nested ref is planned independently, becoming a CREATE that NetBox rejects for a missing required device_type. Since the strip would be unconditional, that fails on every device in the policy every cycle: a wider blast radius than the bug. This change never removes a create-required field.

VirtualChassis.Master is deliberately out of scope. It is also a Device ref reached from a top-level Device and is pruned nowhere, but it is not stale in practice — buildMasterRef runs after the device_type replacement and after master.Serial, its name is handled explicitly, and the customer payload confirms it carries the correct model, a serial and metadata. Folding it in would need a masterRef-shaped rebuilder, since it intentionally carries a Serial that newDeviceStub drops.

Verification

Reproduced and fixed end to end. LibreNMS' ios_2960x recording — which the orb-test-lab already serves at 172.28.0.20 — is a close analogue of the reported device: a 3-member stack whose sysObjectID (.1.3.6.1.4.1.9.1.1208) resolves to the same cat29xxStack family label, while its chassis rows report WS-C2960X-48FPS-L / WS-C2960X-24PS-L. It lacks only an IP address table, which is why the conflict never surfaced there; adding ipAdEntAddr/ipAdEntIfIndex rows for the target on a VLAN interface completes the shape. A dry-run crawl against that simulator:

develop  entities=154  device refs=151  conflicting identities=1
           <name> -> {'WS-C2960X-48FPS-L' x66, 'cat29xxStack' x1}
                      at [0].device.primary_ip4.assigned_object_interface.device
fixed    entities=154  device refs=151  conflicting identities=0
           <name> -> WS-C2960X-48FPS-L x67

Same path, same stale label, same shape as the customer payload. The entity mix is byte-identical between the two runs, and the nested device goes from 6 rich fields to the 4-field stub.

All four symptoms were then driven end to end against that simulator, each by changing only configuration or which interface holds the address:

symptom	develop	with this change
device_type divergence	1 conflicting device identity	0
hostname leak under emit_device_name: false (+ netbox_id)	rich name suppressed, snapshot still carries it	snapshot name also nil
source_match matcher-path split	stamped on the rich device only	on both
primary IP on a member-owned port (Gi2/0/1)	snapshot says master, live entity says member 2	both say member 2

The last one matters most: it is the symptom the customer's own data cannot exercise, because their primary sits on a Vlan that routes to the master.

The reported error itself was then reproduced against the real plugin. Feeding the device entity from each payload through transform_proto_json in a local NetBox with the diode plugin installed:

develop (divergent): ValidationError
  {'__all__': ["Conflicting values for 'device_type' merging duplicate dcim.device,
               `new_object:dcim.devicetype:cdd2d438-…` != `new_object:dcim.devicetype:…`"]}
with fix:            changeset built OK -> 13 entities

Same error key, same message, and the same two new_object:dcim.devicetype refs the report shows. _merge_nodes and _fingerprint_dedupe are byte-identical between the plugin version the reporter is on (v1.13.0) and current HEAD, and v1.13.0 walks the circular subtree the same way, so this is not version-specific.

This also explains the reported symptom shape. _generate_changeset transforms one top-level entity at a time, so only the device entity's own changeset fails — the interfaces, IPs and prefixes land, and the device row itself still gets created from the nested device refs carried on interfaces. Hence a device that exists and looks broadly right while its own diff fails on every cycle.

go build, go vet clean · all 13 packages green · -race -count=2 green · golangci-lint 0 issues · repo-wide lint-all passes for every Go module.

Every guard is mutation-tested — each of these makes the suite fail:

mutation	killed by
revert the prune to continue	5 tests
prune v4 only (half-fix)	the v6 test
key the live index by name instead of address	the address-keying test
weaken the ambiguity guard len==1 → len>0	the ambiguity test
drop the self-cycle guard	the rich-writeback test

The v6 case and the address-keying case were added specifically because the suite was green without them: the customer capture has no IPv6 primary and no duplicate interface names, so neither CI nor the data would have caught a half-fix.

The table was re-run after Tags was reverted, so every row above reflects the diff as it stands.

Not verified

The customer's own device cannot be replayed — their files are dry-run output, not SNMP input. The end-to-end evidence above is a simulator analogue, matched on the properties that matter (stack, same sysObjectID family label, divergent entPhysicalModelName, primary IP on a VLAN interface) but not their exact hardware.

The IPv6 twin is covered by unit tests and by the "prune v4 only" mutation, but not end to end: driving it needs an IPv6 address table in the recording and an IPv6 listener, and no capture from the reporting customer has an IPv6 primary.

The simulator recording and the local responder tweaks are local only and not part of this PR.

Follow-up, not in this PR

What should a nested interface stub carry when it is the only wire representation? IP-assigned interfaces are excluded from top-level emission, so their nested stub is all NetBox ever sees. newInterfaceStub carries type and the plain attributes for exactly that reason, but not Tags (present on 52/52 primary-IP snapshots, 0/80 siblings) and not mode/untagged_vlan/tagged_vlans (0/132 — VlanMapper classifies switchports, and an IP-carrying interface is an SVI or routed port). Both were raised on this PR and both were declined here, because deciding either one inside a device_type fix would change tagging or switching-config behaviour for refs the ticket never touched. Worth answering once, for both fields, with a test each.

resolveIfaceOwner's by-name owner rewrite is provably a false hit for IP-assigned interfaces, since they are excluded from the index it consults — so any name match there is a different interface. On develop today that already mis-routes the live cycle-closer entity; this PR does not introduce it and makes the snapshot agree with it. A real fix must suppress the by-name rewrite on both paths at once. Unreachable in the customer data: zero duplicate top-level interface names across all 52 captures.

[leoparente]

@codex review

[github-actions]

Go test coverage

STATUS	ELAPSED	PACKAGE	COVER	PASS	FAIL	SKIP
🟢 PASS	1.05s	github.com/netboxlabs/orb-agent/orb-discovery/snmp-discovery/config	80.4%	69	0	0
🟢 PASS	26.54s	github.com/netboxlabs/orb-agent/orb-discovery/snmp-discovery/data	84.0%	12181	0	0
🟢 PASS	1.02s	github.com/netboxlabs/orb-agent/orb-discovery/snmp-discovery/env	85.7%	15	0	0
🟢 PASS	1.27s	github.com/netboxlabs/orb-agent/orb-discovery/snmp-discovery/ingest	87.7%	11	0	0
🟢 PASS	1.44s	github.com/netboxlabs/orb-agent/orb-discovery/snmp-discovery/mapping	90.1%	944	0	0
🟢 PASS	1.05s	github.com/netboxlabs/orb-agent/orb-discovery/snmp-discovery/mapping/qbridge	87.8%	66	0	0
🟢 PASS	1.03s	github.com/netboxlabs/orb-agent/orb-discovery/snmp-discovery/metrics	85.4%	27	0	0
🟢 PASS	27.78s	github.com/netboxlabs/orb-agent/orb-discovery/snmp-discovery/policy	86.6%	172	0	0
🟢 PASS	3.87s	github.com/netboxlabs/orb-agent/orb-discovery/snmp-discovery/server	84.4%	20	0	0
🟢 PASS	1.04s	github.com/netboxlabs/orb-agent/orb-discovery/snmp-discovery/snmp	82.2%	44	0	0
🟢 PASS	1.02s	github.com/netboxlabs/orb-agent/orb-discovery/snmp-discovery/targets	93.0%	30	0	0
🟢 PASS	1.02s	github.com/netboxlabs/orb-agent/orb-discovery/snmp-discovery/version	100.0%	1	0	0

Total coverage: 88.6%

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb4122bdc4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Vulnerability Scan: Failed — blocking vulnerabilities detected

Image: orb-agent:scan

Source	Library	CVE	Severity	Installed	Fixed	Title
Python	msgpack	GHSA-6v7p-g79w-8964	🟠 HIGH	1.1.2	1.2.1	MessagePack for Python: Out-of-bounds read / crash on Unpacker reuse after a cau
Python	setuptools	CVE-2025-47273	🟠 HIGH	70.3.0	78.1.1	setuptools: Path Traversal Vulnerability in setuptools PackageIndex
Python	setuptools	CVE-2026-59890	🟡 MEDIUM	70.3.0	83.0.0	setuptools: setuptools: MANIFEST.in exclusion bypass in sdist via Unicode normal

Commit: e0abb93

[leoparente]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 316a5a4fa1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".