Queryable firewall/flow claims + cross-server query surface (CLI + MCP)#38
Conversation
…d claims
network-operations now carries structured host_meta and outbound /
cross-cutting flow data (network-operations#<A>). Project all of it into
the OKF host concepts and compile it into exact-match-queryable claims.
extractors/ansible.py:
- ResolvedRef / FlowInfo dataclasses; os/role/summary/flows on HostInfo.
- resolve_ref() turns a `{{ peers.mon.ipv6 }}` / external-token / subnet
reference into a host name + address + kind, using the already-loaded
peers map (both cr1_nl1 and cr1-nl1 keys resolve to host cr1-nl1).
- extract_network_flows() + _host_flows() fold firewall_extra_rules
(inbound), network_flows_outbound, and network_flows.yml cross-cutting
flows into one per-host flow list. Missing port -> "any" (egress-accept
flows carry no port).
builder.py: render OS/Role/Summary, `# Outbound flows`, and
`# Cross-cutting flows` sections + os/role/summary/network_flows
frontmatter extras; host_connect_targets() emits connects-to edges to
peer hosts only (externals/wildcards make claims but not edges).
claims.py: keep the allows_inbound composite for back-compat and add
decomposed A0 claims — allows_port / allows_proto / allows_from and
connects_to / connects_port / connects_proto (distinct predicates so
inbound and outbound don't collide on object), plus runs_os / has_role.
Per-(host,peer) flows aggregate into a metadata `flows` list so claim
ids stay collision-free and deterministic (export --check reproducible).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Expose the decomposed flow claims as first-class queries so clients can ask "who allows port 22" or "what does loop connect out to" across every server — impossible before, when the only claim was an opaque proto/port/src blob queryable by exact match. - firewall.py: firewall_rules(host/port/proto/src/direction), flows_into, flows_from, server_registry — all reading the sqlite claim store, no live inventory access. src matches resolved peer host / label / addr. - cli.py: `hyrule-knowledge firewall [--host --port --proto --from --direction]` and `hyrule-knowledge servers` (registry dump). - mcp_server.py: knowledge_firewall_rules / knowledge_server_registry / knowledge_flows_into / knowledge_flows_from tools, following the existing nested @mcp.tool() + _service() + policy-gated pattern; they auto-appear in /health via _tool_names(). - tests/test_network_flows.py: extractor resolution (incl. duplicate router keys + cross-cutting self-loop exclusion), concept sections, decomposed-claim compilation + determinism, store cross-server port query, firewall_rules filtering, and the registry/flow helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04b48e32a8
ℹ️ 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".
| for entry in entries: | ||
| peer_host = entry.get("peer_host") | ||
| object_value = f"host:{peer_host}" if peer_host else str(entry.get("peer") or "unknown") | ||
| grouped.setdefault((object_value, str(peer_host or "")), []).append(entry) |
There was a problem hiding this comment.
Grouping only by object_value/peer_host collapses multiple rules to the same peer host that use different peer attributes or addresses, such as a router loopback versus underlay. Because the metadata later takes peer_addr from the first grouped entry, every returned row for that peer gets the sample address, and firewall_rules(src=<the other address>) will miss valid flows. Include peer_addr/peer_kind in the grouping key or keep those fields per flow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 97de716 — the flow grouping key is now (object_value, peer_host, peer_addr), so a router's loopback vs underlay address (same host, different address) no longer collapse onto one sampled address.
| "peer": sample.get("peer"), | ||
| "flows": flow_rows, | ||
| } | ||
| claims.append(_claim(concept, subject=subject, predicate=rel_pred, object_value=object_value, extracted_at=extracted_at, metadata=metadata)) |
There was a problem hiding this comment.
The relationship claims emitted here call _claim with the default source_ref_index=1, so all new allows_from/connects_to claims cite ansible/inventory/hosts.yml even when the flow came from firewall_extra_rules/network_flows_outbound in host vars or from repo-level network_flows.yml (which is not added as a source ref). Consumers following claim provenance or context packs will be sent to a file that does not contain the rule, despite these claims being A0 source truth. Please carry the flow origin/source ref into these claims.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 97de716 — added _ref_index(); each relation claim now cites network_flows.yml when every grouped flow is cross-cutting, otherwise host_vars/<host>.yml, instead of the default (hosts.yml) source_ref.
| candidates = [row.get("peer_host"), row.get("peer"), row.get("peer_addr")] | ||
| return any(value is not None and needle == str(value).lower() for value in candidates) |
There was a problem hiding this comment.
For rules whose source resolves to a wildcard such as any/all (for example an inbound firewall rule with src: any), the row's peer is the wildcard token, so filtering with a concrete host like --from mon drops a rule that actually permits that host. This makes source-specific firewall queries under-report broad allow rules; treat special peers as matching concrete sources or expand them before filtering.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 97de716 — _peer_matches now matches a wildcard peer (all/any/public) for a concrete --from <host> query, unless that host is listed in the row's excludes.
| host = _canonical_host(peer_key) | ||
| return ResolvedRef(raw=text, kind="host", label=host, host=host, address=str(address) if address else None) |
There was a problem hiding this comment.
When a peers.* reference points to an address-book entry that is not present in hosts.yml (for example peers used only as external or loopback addresses), this branch still sets kind="host" and peer_host, so _flow_claims emits relationships like allows_from host:<peer> even though no host concept or registry row exists. That pollutes cross-server claims with phantom hosts; only populate peer_host when the canonical name is in host_names, otherwise keep it as a non-host peer/literal with its address.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 97de716 — resolve_ref demotes a peers.* reference that isn't in hosts.yml to kind='peer' (host=None), so address-book-only peers never surface as phantom host:<name> claims.
Consume the review fixes from network-operations#430 so the compiled claims match the corrected flow data: - `_participants` subtracts a cross-cutting flow's `except: [host, ...]` from the `all` expansion, so the excepted hosts no longer get false per-host claims (noc->mail:22, mon->extmon/dom0:9100, off-net->rtr:53). - `_router_inbound_rules` synthesizes FirewallRuleInfo from `pf_bgp_peers` (tcp/179) and `pf_wg_ports` (udp) so router BGP/WG ingress compiles into allows_inbound/allows_port claims like every other host's inbound. - `family` is read from firewall_extra_rules onto FirewallRuleInfo/FlowInfo and carried into the flow-claim metadata, so a consumer reading structured fields sees that e.g. public mail:993 is IPv4-only (family ip) while the IPv6 IMAPS path is scoped — no longer indistinguishable. dom0 resolves as a host again (network-operations dropped its shadowing external token), so its except membership and dom0->log:6000 edge are correct. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f3356719d
ℹ️ 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".
| ) | ||
| if port is not None: | ||
| want_port = str(port) | ||
| rows = [row for row in rows if row["port"] == want_port] |
There was a problem hiding this comment.
any rules as matches for concrete ports
When a rule or flow omits dport, _host_flows stores the row port as "any", but this filter keeps only exact matches. In a query such as hyrule-knowledge firewall --port 22, a host with a catch-all allow is omitted even though that rule permits port 22; the same under-reporting applies to proto="any". Match concrete filters against catch-all rows before returning results.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 97de716 — added _value_matches; a concrete --port/--proto filter now also matches catch-all any rows, since a rule with no dport/proto genuinely permits the queried value.
|
|
||
| # Cross-cutting flows | ||
|
|
||
| {_markdown_list(cross_cutting, 'No cross-cutting flows reference this host.')} |
There was a problem hiding this comment.
When a cross-cutting flow is inbound for a host, _flow_bullet renders it in the same port ... from format as real firewall rules under this new section. _host_claims still scans the entire body with _FIREWALL_RE, so these repo-level flows also become legacy allows_inbound claims; existing consumers that treat allows_inbound as the firewall_extra_rules projection will see rules that never came from the host firewall. Render this section in a format the legacy regex cannot match or constrain the regex to the firewall section.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 97de716 — the legacy _FIREWALL_RE now scans only the # Inbound firewall rules section (via _section), so cross-cutting / outbound flow bullets (same port … from format) no longer leak in as phantom allows_inbound claims.
| claims.append(_claim(concept, subject=subject, predicate="member_of_group", object_value=str(group), extracted_at=extracted_at)) | ||
| os_value = str(fm.get("os") or "").strip() | ||
| if os_value: | ||
| claims.append(_claim(concept, subject=subject, predicate="runs_os", object_value=os_value, extracted_at=extracted_at)) |
There was a problem hiding this comment.
When host_meta supplies os or role, these new A0 claims are emitted with the default source_ref_index=1, which points at ansible/inventory/hosts.yml; the actual fact lives in ansible/inventory/host_vars/<host>.yml when that ref exists. Consumers following provenance for runs_os/has_role will be sent to the wrong source file, so choose the host-vars source ref for these claims.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 97de716 — runs_os / has_role (and allows_inbound) now pass source_ref_index=host_vars_ref, citing host_vars/<host>.yml where host_meta lives rather than the default hosts.yml ref.
| if ref.label in _SPECIAL_TOKENS and ref.kind == "special" and ref.label in {"all", "any"}: | ||
| return set(host_names) - counterpart |
There was a problem hiding this comment.
any peers from expanding to every host
For cross-cutting flows that use from: any to mean a wildcard/external source, this branch expands that endpoint into every inventory host, so a rule like any -> rtr:443 creates outbound flows and connects-to edges from all servers to rtr even though the peer should remain the wildcard. Only all should expand over host_names; keep any as a special peer like firewall rules do.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 97de716 — _participants now only expands the all token; any stays an opaque wildcard peer, so from: any no longer fabricates inbound flows from every host.
| """Cross-server firewall/flow rows filtered by host/port/proto/source.""" | ||
| store, _ = _service(db_path) | ||
| try: | ||
| rows = firewall_rules(store, host=host or None, port=port or None, proto=proto or None, src=from_ or None, direction=direction) |
There was a problem hiding this comment.
These new MCP tools return firewall and registry data without calling policy_decision_for, unlike the policy-gated search/claims/neighborhood tools in this file. In deployments where MCP is exposed to multiple agent roles, a client can bypass knowledge-policy.yml entirely for network topology/firewall data; add an explicit policy decision before returning rows, and add the corresponding read actions to the policy if they should be allowed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 60c8c53 — all four tools now call policy_decision_for(actor='engineering_loop', action='knowledge.<tool>'), surface the policy_decision in the response, and return empty rows on deny (matching knowledge_claims). The four read actions are registered under engineering_loop / noc_shadow / general in knowledge-policy.yml.
…d 2) Consume the round-2 fixes from network-operations#430: - `extract_network_flows` also returns `all_excludes` (hosts never part of an `all` expansion). `_host_flows` subtracts it — but ONLY from a side that is actually an `all`/special expansion, never from an explicit single-host endpoint, so the explicit mon->dom0 / dom0->log / xoa->dom0 flows survive while the spurious ci/noc/mon-to-dom0 SSH claims disappear. - Cross-cutting and declared-outbound flows now thread their `family:` field into FlowInfo, so the family-scoped rows (e.g. rtr's IPv4-only DNAT ingress) carry the restriction into the flow-claim metadata. Tests cover all_excludes (excluded from `all`, explicit flow preserved, non-excluded host still matches) and cross-cutting family propagation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3191905e7e
ℹ️ 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".
| "proto": str(flow.get("proto") or ""), | ||
| "port": str(flow.get("port") or ""), | ||
| "purpose": str(flow.get("purpose") or ""), | ||
| "origin": str(flow.get("origin") or ""), |
There was a problem hiding this comment.
When a flow carries family (for example the IPv4-only family: ip rule covered by the new tests), the claim metadata keeps it but this flattening step drops it from the CLI/MCP result rows. Consumers of hyrule-knowledge firewall or the MCP tools then cannot tell IPv4-only rules from IPv6/both-family rules and may treat a scoped allow as applying to all address families; include flow.get("family") in each returned row.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 97de716 — firewall query rows now carry family (from the flow), so the IPv4-only family: ipv4 DNAT / mgmt-scrape rules are distinguishable in query results.
| if peer_match: | ||
| peer_key, attr = peer_match.group(1), peer_match.group(2) | ||
| peer = _lookup_peer(peer_key, constants.peers) | ||
| address = peer.get(attr) or peer.get("ipv6") or peer.get("loopback") or peer.get("underlay") |
There was a problem hiding this comment.
For a templated peer reference, the requested attribute is part of the source truth; if {{ peers.cr1_nl1.underlay }} (or a typo like ipv4) is missing, falling back to ipv6/loopback/underlay silently records a different address than the rule named. In that scenario the generated flow claims point at the wrong peer address instead of leaving it unresolved or preserving the exact missing attribute.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 97de716 — resolve_ref uses the exact named attribute (peer.get(attr)); a missing {{ peers.x.underlay }} no longer silently falls back to ipv6 / loopback and records a peer the rule never named.
| if host_name in dst_hosts: | ||
| flows.append(FlowInfo(direction="inbound", proto=proto, port=port, purpose=purpose, origin="cross-cutting", peer=src_ref, family=family)) | ||
| if host_name in src_hosts: | ||
| flows.append(FlowInfo(direction="outbound", proto=proto, port=port, purpose=purpose, origin="cross-cutting", peer=dst_ref, family=family)) |
There was a problem hiding this comment.
When an all endpoint is narrowed by except, the emitted flow still stores the peer as the original wildcard. For a flow like from: mon, to: all, except: [dns], flows_from(mon) reports peer: all with no record that dns was removed, so downstream firewall/MCP consumers cannot distinguish “all hosts” from “all except dns”; expand the peer rows or preserve the exception set in metadata.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 97de716 — FlowInfo gained an excludes tuple; a narrowed all peer records the removed hosts (except + all_excludes), carried into the relation claim's metadata and honored by the firewall query layer.
| proto = rule.proto or "any" | ||
| port = str(rule.dport if rule.dport is not None else "any") |
There was a problem hiding this comment.
Some firewall rules use compound values such as proto: tcp+udp or list dports like [80, 443], but this records them as single flow values (tcp+udp and [80, 443]). The decomposed allows_proto/allows_port claims and firewall filters then only match those exact compound strings, so queries like --proto tcp --port 53 miss DNS rules and --port 80 misses the XOA allow; normalize these into per-protocol/per-port flows before emitting claims.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 97de716 — added _proto_atoms / _port_atoms; emit() expands tcp+udp → tcp, udp and [80, 443] → 80, 443 so allows_port / allows_proto claims and query filters match each atom individually.
…rce attribution Review round 3 (codex) — flow-model correctness in the OKF projection: - Split compound protos (`tcp+udp`) and list dports (`[80, 443]`) into atoms so `allows_port`/`allows_proto` claims and firewall query filters match each value individually instead of an opaque `tcp+udp` / `[80, 443]` string. - `_participants` now only expands the `all` token. `any` (a firewall `src: any` wildcard) stays an opaque peer — expanding it fabricated inbound flows from every host. - `resolve_ref` uses the exact named peer attribute (no silent ipv6/loopback/underlay fallback) and demotes address-book-only peers (in `peers` but not `hosts.yml`) to opaque `kind="peer"`, so they never surface as phantom `host:<name>` claims. - Carry `except:` / `all_excludes` as a flow `excludes` list on wildcard (`all`) peers, and teach the firewall query layer to match a wildcard/`any` row for a concrete host/port/proto unless that host was excepted. Rows now also carry `family`. - Attribute each claim to the file the fact came from: `allows_inbound` and the decomposed port/proto claims cite `host_vars/<host>.yml`; cross-cutting relation claims cite `network_flows.yml`. Scope the legacy firewall regex to the `# Inbound firewall rules` section so flow bullets don't leak in as phantom rules. Group flows by (object, peer_host, peer_addr) so a router's loopback vs underlay address don't collapse onto one sample. - builder: add `network_flows.yml` as a host-concept source ref. Regenerated exports: 92 `allows_inbound` claims now cite `host_vars/<host>.yml` (the file `firewall_extra_rules` actually live in) rather than `hosts.yml`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review round 2 (codex #236) — the four cross-server query tools (`knowledge_firewall_rules`, `knowledge_server_registry`, `knowledge_flows_into`, `knowledge_flows_from`) returned rows without consulting the policy engine, unlike every other read tool. Gate each on `policy_decision_for(actor="engineering_loop", action="knowledge.<tool>")` and surface the decision in the response (empty rows on deny), matching the `knowledge_claims` / `knowledge_neighborhood` pattern. Register the four new read actions in knowledge-policy.yml under the engineering_loop, noc_shadow, and general roles (read-only, alongside knowledge.claims / knowledge.neighborhood). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60c8c53f95
ℹ️ 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".
| if predicate is None: | ||
| raise ValueError(f"unknown direction: {direction!r} (expected 'inbound' or 'outbound')") | ||
| subject = f"host:{host}" if host else None | ||
| claims = store.claims(subject=subject, predicate=predicate, authority_min=authority_min, limit=limit) |
There was a problem hiding this comment.
In the current checkout, exports/knowledge.sqlite contains 0 rows for the new allows_from, connects_to, allows_port, and runs_os predicates, because the committed okf/generated host concepts still lack the new network_flows/host metadata frontmatter. Since this path reads only those shipped claims by default, hyrule-knowledge firewall and the MCP firewall tools return empty results from the committed database until the generated concepts/exports are regenerated together.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct — the committed exports/knowledge.sqlite has 0 rows for the new predicates because the committed okf/generated host concepts predate the flow-enrichment frontmatter (last ingest 2026-07-03, before net-ops#430 merged). Regenerating concepts+exports means a full hyrule-knowledge ingest, which re-clones every source repo and would drag ~8 days of unrelated cross-repo concept drift into this focused code PR (and the nightly ingest.yml bot has never merged to main). So I've split the catalog refresh out as a dedicated follow-up: #39. This PR lands the code, policy, and query surface; the DB populates on that refresh.
| excludes = sample.get("excludes") | ||
| if isinstance(excludes, list) and excludes: | ||
| metadata["excludes"] = list(excludes) | ||
| claims.append(_claim(concept, subject=subject, predicate=rel_pred, object_value=object_value, extracted_at=extracted_at, source_ref_index=_ref_for(grouped_entries), metadata=metadata)) |
There was a problem hiding this comment.
Fresh evidence after the grouping change: this now builds separate groups by peer_addr, but each group still calls _claim with the same subject/predicate/object/source URI when the same peer host appears on multiple addresses. KnowledgeClaim.build hashes only those fields, so compile_claims.add drops the later claim; a router peer with loopback and underlay flows still loses one address and firewall_rules(src=<lost address>) under-reports valid flows. Make the object/source URI unique per address or keep both addresses in one claim metadata entry.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6c81f35. Grouping by peer_addr minted two claims with the SAME id for a multi-address peer (KnowledgeClaim.build hashes only subject/predicate/object/source_uri, not metadata), so compile_claims dropped one. Now there's one aggregated claim per peer with every address preserved in the per-flow addr field (plus a sorted <src|dst>_addrs set when a peer has >1), and firewall.py emits one query row per flow reading its own addr — so firewall_rules(src=<any address>) matches. Regression test: test_multi_address_peer_keeps_every_address.
| """Cross-server firewall/flow rows filtered by host/port/proto/source.""" | ||
| store, _ = _service(db_path) | ||
| try: | ||
| decision = policy_decision_for(actor="engineering_loop", action="knowledge.firewall_rules") |
There was a problem hiding this comment.
When the MCP server is run from an installed package or any cwd without knowledge-policy.yml, load_policy() falls back to default_policy(), which this commit did not update with knowledge.firewall_rules, knowledge.server_registry, or the knowledge.flows_* actions. Because the new tools gate on those actions here, they return deny/empty payloads in that fallback mode even though the checked-in YAML allows them; update the built-in default policy alongside the YAML.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 89b6505. Added knowledge.firewall_rules, knowledge.server_registry, knowledge.flows_into, knowledge.flows_from to default_policy() (engineering_loop / noc_shadow / general) so the tools work in the no-YAML fallback, matching the checked-in knowledge-policy.yml. New parity test test_firewall_flow_actions_allowed_in_default_and_yaml_policy asserts all four are allowed under BOTH the default policy and the loaded YAML, so they can't drift apart again.
Review round 4 (codex) — the round-3 grouping-by-peer_addr change minted two claims with the SAME id for a peer reachable on more than one address (a router loopback + underlay): `KnowledgeClaim.build` hashes only subject/predicate/object/source_uri (not metadata), so `compile_claims` dropped the second and `firewall_rules(src=<lost address>)` under-reported valid flows. Group by peer object again (one claim per peer, no id collision) and preserve every address in the per-flow `addr` field; when a peer has >1 address, also record the sorted set in a `<src|dst>_addrs` metadata key. `firewall.py` builds one query row per flow, reading each row's own `addr`, so a query by any of the peer's addresses matches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review round 4 (codex) — `load_policy()` falls back to `default_policy()` when
no knowledge-policy.yml is on disk (installed package / foreign cwd), and that
built-in did not list `knowledge.firewall_rules`, `knowledge.server_registry`,
or `knowledge.flows_{into,from}`. The four MCP tools gate on those actions, so
in fallback mode they returned deny/empty even though the checked-in YAML allows
them. Add the four read actions to engineering_loop / noc_shadow / general in
default_policy(), and add a parity test asserting they are allowed under BOTH
the default policy and the checked-in YAML.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89b6505261
ℹ️ 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".
| if canonical in host_names or text in constants.peers or canonical in constants.peers: | ||
| peer = _lookup_peer(text, constants.peers) | ||
| address = peer.get("ipv6") or peer.get("loopback") or peer.get("underlay") | ||
| return ResolvedRef(raw=text, kind="host", label=canonical, host=canonical, address=str(address) if address else None) |
There was a problem hiding this comment.
Fresh evidence after the templated-peer fix: the bare-token path still treats any name present in peers as kind="host" even when the canonical name is not in hosts.yml. For an address-book-only peer used as to: loop or src: web, this emits host:<name> relationship claims and firewall rows with no corresponding host concept or registry entry; only set host=canonical when canonical in host_names, otherwise keep the peer as an opaque peer/literal with its address.
Useful? React with 👍 / 👎.
| excludes = sample.get("excludes") | ||
| if isinstance(excludes, list) and excludes: | ||
| metadata["excludes"] = list(excludes) |
There was a problem hiding this comment.
When multiple wildcard flows to the same peer have different except lists, the aggregated claim keeps only grouped_entries[0]'s exclusions while the individual flows rows do not carry their own exclusions. Because firewall_rules applies this claim-level excludes after port/proto filtering, a query for one port can be dropped or leaked based on a different port's exception set; split these groups by exclusions or store exclusions on each flow row.
Useful? React with 👍 / 👎.
| if host_name in dst_hosts: | ||
| excludes = tuple(sorted(src_removed)) if src_special else () | ||
| emit(direction="inbound", proto=proto, port=port, purpose=purpose, | ||
| origin="cross-cutting", peer=src_ref, family=family, excludes=excludes) |
There was a problem hiding this comment.
For a cross-cutting flow such as from: all, to: rtr, _participants deliberately removes rtr from the expanded source set to avoid a self-loop, but the wildcard peer metadata stored here only records src_removed and not the fixed destination that was subtracted. Since _peer_matches later treats all as matching any concrete --from not listed in excludes, firewall --host rtr --from rtr reports a self-allow that the expansion intentionally excluded.
Useful? React with 👍 / 👎.
| for port in ports: | ||
| claims.append(_claim(concept, subject=subject, predicate=port_pred, object_value=port, extracted_at=extracted_at, source_ref_index=host_vars_ref, metadata={"direction": direction})) | ||
| for proto in protos: | ||
| claims.append(_claim(concept, subject=subject, predicate=proto_pred, object_value=proto, extracted_at=extracted_at, source_ref_index=host_vars_ref, metadata={"direction": direction})) |
There was a problem hiding this comment.
Fresh evidence after the relationship-claim attribution fix: the decomposed index claims still always use host_vars_ref, so a host whose allows_port/connects_port value comes only from network_flows.yml is cited to host_vars/<host>.yml (or even hosts.yml) instead of the file that contains the rule. Consumers following provenance for a port/proto query will be sent to a source that does not define the cross-cutting flow; choose the source ref from the contributing entries as the relationship claims now do.
Useful? React with 👍 / 👎.
| def _firewall_rules(host_vars: dict[str, Any]) -> list[FirewallRuleInfo]: | ||
| rules: list[FirewallRuleInfo] = list(_router_inbound_rules(host_vars)) |
There was a problem hiding this comment.
Fresh evidence after scoping the legacy regex to # Inbound firewall rules: router-only pf_bgp_peers/pf_wg_ports are now seeded into host.firewall_rules, rendered in that same section, and therefore emitted as legacy allows_inbound claims even though that predicate is documented in claims.py as the firewall_extra_rules projection only. For routers with pf-managed ingress, existing consumers of allows_inbound will now see rules from a different source/template; keep these synthesized entries separate for the new flow claims or render them outside the legacy-scanned section.
Useful? React with 👍 / 👎.
1 participant
Context
The OKF ingest already projected a de-facto server registry and 92 A0
allows_inboundclaims from network-operations — but the claim object was anopaque
proto/port/srcblob with an unresolved{{ peers.* }}template,queryable by exact match only. You could not ask "which servers allow port
22" or "what does
loopconnect out to".This PR closes those gaps. It is the knowledge half of a two-repo change;
network-operations now ships the structured
host_meta+ outbound/cross-cuttingflow data this consumes (AS215932/network-operations#430).
What changed
Enrich the projection (
okf:commit)extractors/ansible.py:resolve_ref()turns a{{ peers.mon.ipv6 }}/external-token / subnet reference into a host name + address + kind (both
cr1_nl1andcr1-nl1keys resolve to hostcr1-nl1).extract_network_flows_host_flowsfold inbound + outbound + cross-cutting into one per-host flowlist; missing port →
"any".builder.py: renderOS/Role/Summary,# Outbound flows,# Cross-cutting flows+ frontmatter extras;connects-toedges to peerhosts only.
claims.py: keepallows_inboundfor back-compat; add decomposed A0claims —
allows_port/allows_proto/allows_from,connects_to/connects_port/connects_proto,runs_os/has_role. Per-(host,peer)flows aggregate into a metadata
flowslist so claim ids stay collision-freeand deterministic.
Query surface (
knowledge:commit)firewall.py:firewall_rules/flows_into/flows_from/server_registryover the sqlite claim store.hyrule-knowledge firewall [...]andhyrule-knowledge servers.knowledge_firewall_rules/knowledge_server_registry/knowledge_flows_into/knowledge_flows_from(existing nested@mcp.tool()+_service()+ policy-gated pattern; auto-listed in/health).Verification
ruffclean,mypy --strictclean (42 files),pytest168 passed (+11 new).export --checkbyte-reproducible — no regeneration.firewall_rules(port="22")→ 112 rows across 23 hosts, resolved peers onflows_into/flows_from, port-less egress flows land asany.Related
🤖 Generated with Claude Code