[ananos]

A system uninstall deleted the brig0 bridge but left the two rules the CNI firewall plugin adds for it, for v0.1.0-rc19:

-A CNI-ISOLATION-STAGE-1 -i brig0 ! -o brig0 -m comment --comment "CNI firewall plugin rules (ingressPolicy: same-bridge)" -j CNI-ISOLATION-STAGE-2
-A CNI-ISOLATION-STAGE-2 -o brig0 -m comment --comment "CNI firewall plugin rules (ingressPolicy: same-bridge|isolated)" -j DROP

The plugin never removes these on CNI DEL. The chains themselves are shared with every other CNI network on the host, so the uninstaller must not flush or delete them.

Changes

• brig-uninstall.sh (generated by scripts/build-bundle.sh): new drop_bridge_rules. It reads iptables-save and ip6tables-save for the filter and nat tables and deletes every rule in a CNI-* chain that names brig0 with -i or -o. Each rule goes back to iptables-restore -w --noflush as -D, exactly as iptables-save printed it, so the shell never parses the quoted comment.
• The chains, other bridges' rules and rules outside CNI-* (eg. an admin's FORWARD -i brig0) stay. brig0x does not match brig0.
• The rules go whether or not the bridge is still up. When nothing matches, nothing is run or printed. A failed delete warns and the uninstall carries on.
• A user install skips this. Its CNI runs inside the rootlesskit network namespace, so its brig0 rules live there and go with it. On a test host, a rootless --network shared sandbox put 2 brig0 rules in that namespace and 0 on the host, and a restart of the user unit gave a fresh namespace with 0.
• tests/uninstall-cni-rules.sh (new): runs the generated uninstaller on a fake system install, with stub iptables-save/iptables-restore that answer from per-table rule files. It checks that exactly the brig0 rules go (IPv4 filter and nat, IPv6 filter), that a second run is quiet, that the rules go with the bridge already gone, that a failing iptables-restore does not stop the uninstall, that a user uninstall runs no iptables, and that a host without iptables still finishes. Against main's uninstaller it fails on the two leaked rules.
• README: item 6 of "What it touches outside the base directories" names the rules.

Tested

Locally (macOS): sh -n and shellcheck 0.11.0 over the CI file list, and tests/uninstall-cni-rules.sh under sh and dash. On Ubuntu 24.04 (iptables 1.8.10 legacy) every tests/*.sh passes, as CI's shell job runs them.

Ubuntu 24.04, next to a rootless user install: system install of the published v0.1.0-rc18 linux-amd64 bundle with its install.sh, a root brig run -d --network shared ubuntu on hvi, brig rm, then this PR's brig-uninstall.sh copied over the installed one:

                 brig0 rules   CNI-* rules   all filter rules
sandbox up            2             15              73
after brig rm         2             11              68
after uninstall       0              9              66

The uninstall logged both deletes. Apart from the two brig0 rules, the rules and chains after the uninstall are identical to those before it, including two rules added by hand for a second bridge. The user install still boots (brig run -d ubuntu on hvi, then brig rm).

drop_bridge_rules also ran against iptables-nft and ip6tables-nft in a private network namespace. It removed the brig0 rules, kept br-other and FORWARD -i brig0, and a second run printed nothing.

Not in scope: brig's own per-sandbox networks (br-<id>, the default --network) leave the same two rules each after brig rm, and a system uninstall leaves the CNI IPAM state under /var/lib/cni. Both deserve their own follow-up.

Release sequence

For v0.1.0-rc19. Independent of #30: both touch scripts/build-bundle.sh, in different places. Merge either first, rebase the other onto main, merge it, then tag v0.1.0-rc19 on main.

🤖 Generated with Claude Code

[ananos]

@claude-fleet review

[ananos]

@claude-fleet review

[claude-fleet]

Review task queued.

Task: 4f8754aa-a851-4067-b2e9-dbefc04e6a10
Commit: 443ed7e
Model: claude-opus-5 (effort medium)
Executor: ci-16gb-fsn1-1
Status: done. Review posted.

[claude-fleet]

Approve: no blocking or should_fix findings. One nit.

Findings:

• nit, scripts/build-bundle.sh:896 — -w with no timeout can block forever, and 2>/dev/null discards the reason behind the warning

Verified:

• The awk filter matches only -A lines whose chain starts CNI-, with an exact -i/-o operand equal to the bridge, so brig0x and FORWARD -i brig0 survive — scripts/build-bundle.sh:891-894, asserted by tests/uninstall-cni-rules.sh:93,97 against the expected file built at 104-107.
• Deletion is by rule spec, not index, so deleting one rule does not invalidate the specs still queued from the same iptables-save snapshot — scripts/build-bundle.sh:890-896.
• The quoted --comment never reaches the shell as a word: the saved line goes to iptables-restore stdin verbatim after stripping -A — scripts/build-bundle.sh:895-896.
• "The rules go whether or not the bridge is still up": the outer condition dropped ip link show, which now only gates the link deletion — scripts/build-bundle.sh:906-913; covered by tests/uninstall-cni-rules.sh:198-203.
• A user install runs no iptables: INSTALL_MODE != user still gates the whole block — scripts/build-bundle.sh:906, asserted at tests/uninstall-cni-rules.sh:214-219.
• Shared chains survive: only -D lines are emitted, never -F/-X, and the chain-declaration lines are not reproduced — scripts/build-bundle.sh:895; asserted at tests/uninstall-cni-rules.sh:187.
• A failed delete warns and the uninstall completes — scripts/build-bundle.sh:897-899; tests/uninstall-cni-rules.sh:205-211 checks both the five warnings and that the rule files are unchanged.
• set -eu is on in the generated script and drop_bridge_rules cannot return non-zero: || continue yields 0, and an empty awk result leaves the while at 0 — generated uninstaller line 13, scripts/build-bundle.sh:886-902.
• drop_bridge_rules is the only bridge-teardown path; install.sh touches no iptables rules — only install.sh:240 (a presence check) and install.sh:894 (BRIDGE_NAME=brig0) mention either.
• CI picks the new test up automatically (for t in tests/*.sh) and shellchecks the extracted uninstaller — .github/workflows/ci.yml:53-55,67.

Checks run:

• sh tests/uninstall-cni-rules.sh (dash 0.5.12, mawk) → all 6 cases pass, including the no-iptables case.
• Revert check: replaced scripts/build-bundle.sh with 0d865d5's copy and reran the test → FAIL: the brig0 rules are not exactly the ones removed: iptables.filter, with the diff naming exactly the two leaked CNI-ISOLATION-STAGE-1/-2 rules. File restored; git status --porcelain shows only untracked node_modules.
• sh -n on the extracted uninstaller → clean. shellcheck is not installed here.
• iptables-restore -w --noflush --test and the ip6tables/iptables-legacy variants on 1.8.10 (nf_tables) → all reach "Permission denied (you must be root)", so -w with no value followed by --noflush parses as intended and --noflush is not eaten as -w's optional argument.

Smaller things:

• The match is on the operand alone, so a negated ! -i brig0 in a CNI-* chain would also be deleted. The firewall plugin never emits one standalone (its ! -o <br> is on the same rule as -i <br>), but CNI-ADMIN is admin-populated and is a CNI-* chain. Low risk, and removing a brig0 rule when brig0 is going away is defensible — worth one sentence in the function comment rather than a code change.
• On a system install whose bridge is already gone, this is now the first thing to run iptables-save -t nat, which can modprobe iptable_nat on a host that never loaded it. An uninstall loading a kernel module is odd but harmless.
• README:389 still opens the list with "all recorded in .install-stamp". The rules are not recorded there; BRIDGE_NAME is, and they are derived from it. The new item 6 reads fine on its own.
• tests/uninstall-cni-rules.sh:158-161 carries a good comment about why FAKE_* are re-exported by hand. The unset FAKE_RESTORE_FAIL FAKE_BRIDGE at line 212 is load-bearing for the two cases after it — the ${FAKE_BRIDGE:-up} defaults make that non-obvious.

Not checked:

• That the CNI firewall plugin adds exactly these two rules and never removes them on DEL — the plugin source is not in this repo and there is no network here. Taken from the PR description and the real-host numbers in it.
• The host runs reported in the description (Ubuntu 24.04, the rc18 bundle, the iptables-nft namespace run). The stub-based test reproduces the rule arithmetic but not the real binaries.
• scripts/build-bundle.sh outside the diffed hunk.

Not checked:

• The CNI firewall plugin's own source: that it adds exactly these two rules per bridge and never removes them on DEL. Not vendored here and no network access; taken from the PR description.
• The real-host verification runs described in the PR (Ubuntu 24.04 with the rc18 bundle, and the iptables-nft namespace run). Only the stubbed test was executed.
• scripts/build-bundle.sh outside the changed hunk at lines 879-914.

Task 4f8754aa-a851-4067-b2e9-dbefc04e6a10 on ci-16gb-fsn1-1, model claude-opus-5, effort medium.

[claude-fleet]

nit: iptables-restore -w with no value waits for the xtables lock indefinitely, and 2>/dev/null throws away the reason for a failure.
Failure: if another process (a container runtime, firewalld, a parallel brig rm) holds /run/xtables.lock, brig-uninstall.sh hangs at this line with no output and no timeout — the preceding info for the previous rule is the last thing the user sees. Separately, when a delete does fail, WARNING: could not remove iptables -t filter CNI-... gives the operator nothing to act on, since iptables' own message ("Bad rule (does a matching rule exist in that chain?)", a lock timeout, a missing module) went to /dev/null.
Evidence: scripts/build-bundle.sh:896 pipes into "$ipt-restore" -w --noflush 2>/dev/null; the warn branch at :898 prints only the rule text. Verified -w parses with a following --noflush on iptables 1.8.10 nf_tables, so the flag itself is fine.
Fix: -w 5 bounds the wait, and capturing stderr into the warning (err="$(... 2>&1)", or dropping the redirect so iptables' message lands next to the warning) makes a failed delete diagnosable.

[claude-fleet]

Review task queued.

Task: eb1d3b66-c41a-4bc5-876c-5148486da713
Commit: 443ed7e
Model: claude-opus-5 (effort medium)
Executor: rp1
Status: done. Review posted.

[claude-fleet]

Previous round at 443ed7e: 1 finding.

Previous finding	Classification	Cited line
scripts/build-bundle.sh:896 (nit): iptables-restore -w with no value waits for the xtables lock indefinitely, and 2>/dev/null throws away the reason for a failure. Failure: if another process (a container runtime, firewalld, a parallel brig rm) holds /run/xtables.lock, brig-uninstall.sh hangs at this line with no output and no timeout — the preceding info for the previous rule is the last thing the user sees. Separately, when a delete does fail, WARNING: could not remove iptables -t filter CNI-... gives the operator nothing to act on, since iptables' own message ("Bad rule (does a matching rule exist in that chain?)", a lock timeout, a missing module) went to /dev/null. Evidence: scripts/build-bundle.sh:896 pipes into "$ipt-restore" -w --noflush 2>/dev/null; the warn branch at :898 prints only the rule text. Verified -w parses with a following --noflush on iptables 1.8.10 nf_tables, so the flag itself is fine. Fix: -w 5 bounds the wait, and capturing stderr into the warning (err="$(... 2>&1)", or dropping the redirect so iptables' message lands next to the warning) makes a failed delete diagnosable.	open	scripts/build-bundle.sh:896

Approve: no new blocking or should_fix findings. Head is unchanged since the last review (git diff 443ed7e..HEAD empty), so the one earlier nit stands as written.

Findings:

• nit, scripts/build-bundle.sh:896 — unbounded -w and discarded iptables stderr (previous finding 1, still open)

Verified:

• The awk only matches -A lines whose chain starts with CNI-, so FORWARD -i brig0 -j ACCEPT survives — scripts/build-bundle.sh:890-893, fixture tests/uninstall-cni-rules.sh:93 vs want file at :104-107
• brig0x does not match: the comparison is $(i+1) == br, a whole-field equality — scripts/build-bundle.sh:892, covered by tests/uninstall-cni-rules.sh:97
• The rule text is never re-parsed by the shell: it goes back to iptables-restore byte-for-byte with only -A stripped — scripts/build-bundle.sh:895-896
• Rules go with the bridge already gone: drop_bridge_rules moved outside the ip link show branch — scripts/build-bundle.sh:903-911
• A user install runs no iptables at all: the mode guard is on the outer if — scripts/build-bundle.sh:903; asserted at tests/uninstall-cni-rules.sh:217
• The uninstaller heredoc is quoted (<<'UNEOF'), so $1, $i, $(i + 1) reach the generated script unexpanded — scripts/build-bundle.sh:741
• Chains are not flushed or deleted: only -D lines are emitted, --noflush is passed, and the test asserts :CNI-ISOLATION-STAGE-2 survives — scripts/build-bundle.sh:894-896, tests/uninstall-cni-rules.sh:187
• README item 6 matches the code (rules named, chains kept) — README.md:392-395

Checks run:

• sh tests/uninstall-cni-rules.sh → all 6 cases ok
• Revert check: ran the new test against main's scripts/build-bundle.sh (git show 0d865d5:scripts/build-bundle.sh) → FAIL: the brig0 rules are not exactly the ones removed: iptables.filter, with both leaked CNI-ISOLATION-STAGE-1/-2 rules in the diff. The PR's "fails against main" claim holds.
• for t in tests/*.sh; do sh "$t"; done → brig-ctl, fetch-hvi, source-pins, uninstall-cni-rules pass; the four install/rootless tests SKIP here for lack of a subuid range (CI adds one)
• Confirmed the 5 expected deletes in the fixture (IPv4 filter 2, IPv4 nat 1, IPv6 filter 2) match the assertion at tests/uninstall-cni-rules.sh:185

Smaller things:

• CNI-ADMIN matches ^CNI-, and the firewall plugin documents that chain as the operator's own. An admin rule like -A CNI-ADMIN -i brig0 -j ACCEPT is deleted by the uninstall, while the equivalent rule in FORWARD is kept. Defensible (the bridge is gone anyway), but it is narrower than the README's "only rules in the CNI chains that name brig0" framing suggests, since those are hand-written and not recreated.
• awk field-splits the whole saved line, so a rule in a CNI-* chain whose --comment text happens to contain -i brig0 would match and be deleted. Contrived; no such comment exists in practice.
• ip link show at :904 runs without a command -v ip guard, unlike the iptables path. Pre-existing, and a missing ip only yields 127 inside an if.

Not checked:

• shellcheck (not installed in this workspace; CI's shell job covers it)
• The host observations in the description (rc18 bundle on Ubuntu 24.04, rootless namespace rule counts, iptables-nft run) — no such host here.

Not checked:

• shellcheck over the CI file list (not installed in this workspace)
• The described runs on a real Ubuntu 24.04 host, including the rootless-namespace rule counts and the iptables-nft run

Task eb1d3b66-c41a-4bc5-876c-5148486da713 on rp1, model claude-opus-5, effort medium.

[claude-fleet]

nit: Still open since 443ed7e. iptables-restore -w with no value waits for the xtables lock indefinitely, and 2>/dev/null throws away the reason for a failure.
Failure: if another process (a container runtime, firewalld, a parallel brig rm) holds /run/xtables.lock, brig-uninstall.sh hangs at this line with no output and no timeout — the preceding info for the previous rule is the last thing the user sees. Separately, when a delete does fail, WARNING: could not remove iptables -t filter CNI-... gives the operator nothing to act on, since iptables' own message ("Bad rule (does a matching rule exist in that chain?)", a lock timeout, a missing module) went to /dev/null.
Evidence: scripts/build-bundle.sh:896 pipes into "$ipt-restore" -w --noflush 2>/dev/null; the warn branch at :898 prints only the rule text. Verified -w parses with a following --noflush on iptables 1.8.10 nf_tables, so the flag itself is fine.
Fix: -w 5 bounds the wait, and capturing stderr into the warning (err="$(... 2>&1)", or dropping the redirect so iptables' message lands next to the warning) makes a failed delete diagnosable.

[ananos]

@claude-fleet review

[claude-fleet]

Review task queued.

Task: e203ddf9-7800-4e3d-bafe-0e817ea1f9d2
Commit: f7878e8
Model: claude-opus-5 (effort medium)
Executor: rp2
Status: done. Review posted.

[claude-fleet]

Previous round at 443ed7e: 1 finding.

Previous finding	Classification	Cited line
scripts/build-bundle.sh:896 (nit): iptables-restore -w with no value waits for the xtables lock indefinitely, and 2>/dev/null throws away the reason for a failure. Failure: if another process (a container runtime, firewalld, a parallel brig rm) holds /run/xtables.lock, brig-uninstall.sh hangs at this line with no output and no timeout — the preceding info for the previous rule is the last thing the user sees. Separately, when a delete does fail, WARNING: could not remove iptables -t filter CNI-... gives the operator nothing to act on, since iptables' own message ("Bad rule (does a matching rule exist in that chain?)", a lock timeout, a missing module) went to /dev/null. Evidence: scripts/build-bundle.sh:896 pipes into "$ipt-restore" -w --noflush 2>/dev/null; the warn branch at :898 prints only the rule text. Verified -w parses with a following --noflush on iptables 1.8.10 nf_tables, so the flag itself is fine. Fix: -w 5 bounds the wait, and capturing stderr into the warning (err="$(... 2>&1)", or dropping the redirect so iptables' message lands next to the warning) makes a failed delete diagnosable.	fixed	scripts/build-bundle.sh:898

Approve.

Findings:

• nit, scripts/build-bundle.sh:893 — the CNI-* sweep also takes an operator's CNI-ADMIN rules for brig0

Verified:

• previous finding 1 fixed: -w 5 bounds the lock wait and err="$(... 2>&1)" carries iptables' reason into the warning — scripts/build-bundle.sh:897-901
• -w 5 --noflush parses on real iptables (1.8.9 legacy): deleted exactly the CNI-ISOLATION-STAGE-2 -o brig0 rule, kept FORWARD -i brig0 and -o br-other; a missing rule returns 1 with iptables-restore: line 2 failed on stderr, now captured
• the conflist really runs the firewall plugin with ingressPolicy: same-bridge, which is what adds the two leaked rules — scripts/build-bundle.sh:1418
• the rules go whether or not the bridge is up: drop_bridge_rules sits outside the ip link show branch — scripts/build-bundle.sh:908-916
• brig0x does not match: the awk compares $(i+1) == br exactly, and the fixture carries a brig0x rule that survives — tests/uninstall-cni-rules.sh:97,105-107
• user install runs no iptables: guarded by INSTALL_MODE != user at :908, asserted at tests/uninstall-cni-rules.sh:220
• the five expected deletes are IPv4 filter 2 + nat 1 + IPv6 filter 2, matching the fixtures at tests/uninstall-cni-rules.sh:96,100,116,121
• CI already picks the new test up: for t in tests/*.sh and the shellcheck list both glob it — .github/workflows/ci.yml:57,67

Checks run:

• sh tests/uninstall-cni-rules.sh → all 6 cases ok
• revert check: git checkout 0d865d5 -- scripts/build-bundle.sh then the same test → FAIL: the brig0 rules are not exactly the ones removed: iptables.filter, with the two leaked CNI-ISOLATION-STAGE-1/-2 rules in the diff. File restored, tree clean.
• real iptables in unshare -rnm with a tmpfs /run: delete path and failure path, as above

Smaller things:

• README:389 still opens with "Six things, all recorded in .install-stamp". The rules are not in the stamp; they are derived from BRIDGE_NAME, which is. Reword if the sentence is meant to be exact.
• The PR description's "Tested" section still describes the -w form, not -w 5. The namespace run above covers -w 5 on legacy iptables, but the text is stale.

Not checked:

• the iptables-nft backend (only iptables-legacy 1.8.9 is present here) and iptables older than 1.6.2, which has no -w on iptables-restore. All platforms README names (Ubuntu 20.04+, Debian 12) are above that.
• the on-host behaviour claims in the description (the rc18 bundle run on Ubuntu 24.04, the rootless namespace counts) — no such host here.

Not checked:

• the iptables-nft backend and iptables older than 1.6.2 (only iptables-legacy 1.8.9 available here)
• the on-host verification in the PR description: the rc18 bundle install/uninstall on Ubuntu 24.04 and the rootless namespace rule counts

Task e203ddf9-7800-4e3d-bafe-0e817ea1f9d2 on rp2, model claude-opus-5, effort medium.

[claude-fleet]

nit: The sweep matches any chain named CNI-*, which includes CNI-ADMIN — the chain the firewall plugin creates precisely so an operator can add their own rules. So the uninstaller deletes operator rules, not only the plugin's two.
Failure: an operator who added -A CNI-ADMIN -i brig0 -j ACCEPT (the documented override point; the jump to it is in your own fixture at tests/uninstall-cni-rules.sh:94) sees it removed, logged as removed iptables -t filter CNI-ADMIN -i brig0 -j ACCEPT. This contradicts the description's "rules outside CNI-* (eg. an admin's FORWARD -i brig0) stay", which reads as "admin rules stay" — here the location decides, not the author. Direction is fairly benign, since brig0 is going away in the same run.
Evidence: scripts/build-bundle.sh:893 is the only chain filter. Fed -A CNI-ADMIN -i brig0 -j ACCEPT and -A FORWARD -i brig0 -j ACCEPT to that exact awk program: it printed the CNI-ADMIN line. The fixture has a :CNI-ADMIN chain and the jump at tests/uninstall-cni-rules.sh:94 but no admin rule naming brig0, so nothing covers this.
Fix: either narrow to the chains the plugin owns (CNI-ISOLATION-STAGE-1|2, CNI-FORWARD), or keep the wide sweep and say in the comment at :879-886 and in README:389 that CNI-ADMIN overrides for this bridge go too — and add a fixture rule for it.