Skip to content

Don't break docker networking#104

Merged
sjmiller609 merged 6 commits intomainfrom
fix-docker-networking
Mar 2, 2026
Merged

Don't break docker networking#104
sjmiller609 merged 6 commits intomainfrom
fix-docker-networking

Conversation

@sjmiller609
Copy link
Collaborator

@sjmiller609 sjmiller609 commented Feb 17, 2026

Note

Medium Risk
Touches host iptables FORWARD-chain manipulation, so a bug could impact Docker/host networking on Linux. Mitigated by only acting when DOCKER-FORWARD exists and the jump is missing, but still warrants careful review/testing on varied firewall setups.

Overview
Prevents host Docker networking from breaking when the FORWARD chain gets flushed/rebuilt by re-adding the missing jump to DOCKER-FORWARD during network initialization.

Adds ensureDockerForwardJump (with lastHypemanForwardRulePosition) to conditionally detect a present DOCKER-FORWARD chain but missing FORWARD jump and re-insert it after hypeman-managed rules.

Extends the Linux network integration test to delete the jump, re-run networkManager.Initialize, and assert the jump is restored (skipping when Docker isn’t running and cleaning up to avoid leaving the host misconfigured).

Written by Cursor Bugbot for commit fb37b52. This will update automatically on new commits. Configure here.

**Fixed (2):**
- **"Hypeman rule scan may miss comments"** — Added `-v` flag to the `iptables -L` call in `lastHypemanForwardRulePosition`, matching what `isForwardRuleCorrect` already does. Without `-v`, some iptables versions don't show comment text.
- **"Test can leave host iptables modified"** — Added a `t.Cleanup` that checks if the DOCKER-FORWARD jump is missing and restores it, so a mid-test failure won't leave the host broken.

**Dismissed with inline comments (2):**
- **"Docker jump may bypass DOCKER-USER rules"** — Added a comment on `ensureDockerForwardJump` explaining that it only inserts when the jump is completely absent (the `-C` check returns early otherwise), so it can't reorder existing Docker rules.
- **"Docker iptables test lacks privilege guard"** — Added a comment on the test explaining that `make test-linux` runs the entire suite under `sudo`, so iptables permissions are always available.
@sjmiller609 sjmiller609 enabled auto-merge (squash) February 17, 2026 15:44
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

Copy link
Contributor

@rgarcia rgarcia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@sjmiller609 sjmiller609 merged commit 50f4539 into main Mar 2, 2026
6 checks passed
@sjmiller609 sjmiller609 deleted the fix-docker-networking branch March 2, 2026 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants