Skip to content

fix: Relax inbound firewall and add tamper protection #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stranma wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: Relax inbound firewall and add tamper protection #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .claude/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@
"Bash(git clean *)", "Bash(git config *)",
"Bash(*git remote add *)", "Bash(*git remote set-url *)", "Bash(*git remote remove *)",
"Bash(*git remote rename *)", "Bash(*git remote set-head *)",
"Bash(uv self *)"
"Bash(uv self *)",
"Bash(*iptables *)", "Bash(*ip6tables *)", "Bash(*ipset *)",
"Bash(*nft *)", "Bash(*init-firewall*)"
],
"ask": [
"Bash(python *)", "Bash(uv run python *)",
Expand Down
3 changes: 2 additions & 1 deletion .devcontainer/devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
],
"containerEnv": {
"CLAUDE_CONFIG_DIR": "/home/vscode/.claude",
"POWERLEVEL9K_DISABLE_GITSTATUS": "true"
"POWERLEVEL9K_DISABLE_GITSTATUS": "true",
"FIREWALL_ALLOW_INBOUND": "${localEnv:FIREWALL_ALLOW_INBOUND:true}"
},
"workspaceMount": "source=${localWorkspaceFolder},target=/workspace,type=bind,consistency=delegated",
"workspaceFolder": "/workspace",
Expand Down
28 changes: 22 additions & 6 deletions .devcontainer/init-firewall.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ IFS=$'\n\t'
# Restricts egress to: PyPI, GitHub, Anthropic/Claude, VS Code, uv/Astral,
# plus any domains from WebFetch(domain:...) permission patterns.
# Uses ipset with aggregated CIDR ranges for reliable filtering.
# FIREWALL_ALLOW_INBOUND (default: true) controls inbound filtering.
# When true, INPUT chain is left permissive (Docker handles inbound isolation).
# When false, strict INPUT DROP policy is applied.

ALLOW_INBOUND="${FIREWALL_ALLOW_INBOUND:-true}"
echo "Inbound firewall mode: $([ "$ALLOW_INBOUND" = "true" ] && echo "permissive (Docker handles isolation)" || echo "strict (INPUT DROP)")"

echo "iptables version: $(iptables --version)"
if iptables_path="$(command -v iptables 2>/dev/null)"; then
Expand Down Expand Up @@ -49,10 +55,14 @@ fi

# Allow DNS and localhost before any restrictions
iptables -A OUTPUT -p udp --dport 53 -j ACCEPT
iptables -A INPUT -p udp --sport 53 -j ACCEPT
if [ "$ALLOW_INBOUND" != "true" ]; then
iptables -A INPUT -p udp --sport 53 -j ACCEPT
fi
iptables -A OUTPUT -p tcp --dport 53 -j ACCEPT
iptables -A OUTPUT -p tcp --dport 22 -j ACCEPT
iptables -A INPUT -p tcp --sport 22 -m state --state ESTABLISHED -j ACCEPT
if [ "$ALLOW_INBOUND" != "true" ]; then
iptables -A INPUT -p tcp --sport 22 -m state --state ESTABLISHED -j ACCEPT
fi
iptables -A INPUT -i lo -j ACCEPT
iptables -A OUTPUT -o lo -j ACCEPT

Expand Down Expand Up @@ -164,7 +174,9 @@ fi
HOST_NETWORK=$(echo "$HOST_IP" | sed "s/\.[0-9]*$/.0\/24/")
echo "Host network detected as: $HOST_NETWORK"

iptables -A INPUT -s "$HOST_NETWORK" -j ACCEPT
if [ "$ALLOW_INBOUND" != "true" ]; then
iptables -A INPUT -s "$HOST_NETWORK" -j ACCEPT
fi
iptables -A OUTPUT -d "$HOST_NETWORK" -j ACCEPT

# Block all IPv6 traffic (firewall is IPv4-only)
Expand All @@ -175,7 +187,9 @@ ip6tables -A INPUT -i lo -j ACCEPT 2>/dev/null || true
ip6tables -A OUTPUT -o lo -j ACCEPT 2>/dev/null || true

# Allow established connections
iptables -A INPUT -m state --state ESTABLISHED,RELATED -j ACCEPT
if [ "$ALLOW_INBOUND" != "true" ]; then
iptables -A INPUT -m state --state ESTABLISHED,RELATED -j ACCEPT
fi
iptables -A OUTPUT -m state --state ESTABLISHED,RELATED -j ACCEPT

# Allow traffic to whitelisted domains
Expand All @@ -185,11 +199,13 @@ iptables -A OUTPUT -m set --match-set allowed-domains dst -j ACCEPT
iptables -A OUTPUT -j REJECT --reject-with icmp-admin-prohibited

# Set default policies AFTER all ACCEPT rules (prevents lockout on partial failure)
iptables -P INPUT DROP
if [ "$ALLOW_INBOUND" != "true" ]; then
iptables -P INPUT DROP
fi
Comment on lines +202 to +204
Copy link

@coderabbitai coderabbitai bot Mar 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Permissive mode is not explicitly enforced (idempotency bug).

When ALLOW_INBOUND=true, INPUT policy is left unchanged. If a previous run set INPUT DROP, permissive mode can still behave as strict. Set INPUT ACCEPT explicitly in the permissive branch.

Proposed fix 
-if [ "$ALLOW_INBOUND" != "true" ]; then
-    iptables -P INPUT DROP
-fi
+if [ "$ALLOW_INBOUND" != "true" ]; then
+    iptables -P INPUT DROP
+else
+    iptables -P INPUT ACCEPT
+fi

Based on learnings: "Implement two-layer defense against data exfiltration: firewall (iptables whitelist in devcontainer) and exfiltration guard (dangerous-actions-blocker.sh hook)".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ "$ALLOW_INBOUND" != "true" ]; then
iptables -P INPUT DROP
fi
if [ "$ALLOW_INBOUND" != "true" ]; then
iptables -P INPUT DROP
else
iptables -P INPUT ACCEPT
fi
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.devcontainer/init-firewall.sh around lines 202 - 204, The current
init-firewall.sh branch only sets iptables -P INPUT DROP when ALLOW_INBOUND !=
"true", leaving INPUT policy unchanged when ALLOW_INBOUND=="true" which breaks
idempotency; update the conditional so the permissive branch explicitly runs
iptables -P INPUT ACCEPT (i.e., when ALLOW_INBOUND == "true" set the INPUT
policy to ACCEPT) so each run deterministically enforces the intended mode and
prevents leftover DROP policies from previous runs.

iptables -P FORWARD DROP
iptables -P OUTPUT DROP

echo "Firewall configuration complete"
echo "Firewall configuration complete (inbound: $([ "$ALLOW_INBOUND" = "true" ] && echo "permissive" || echo "strict"))"

# --- Verification ---
echo "Verifying firewall rules..."
Expand Down
9 changes: 9 additions & 0 deletions docs/DECISIONS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,12 @@ most hooks, commands, and niche agents. Refocus on exfiltration prevention.
- output-secrets-scanner removed -- conversation leaks to Anthropic are accepted
- Permission tiers removed -- single settings.json baseline for all environments
- unicode-injection-scanner removed -- exotic threat, low practical risk

## 2026-03-15: Devcontainer Firewall Inbound Relaxation

**Request**: Strict inbound filtering blocks legitimate dev server use cases unnecessarily.

**Decisions**:
- Default to permissive inbound (`FIREWALL_ALLOW_INBOUND=true`) -- the primary threat model is egress (data exfiltration), not inbound; Docker's network stack provides inbound isolation depending on port publishing and network mode
- Opt-in strict inbound via `FIREWALL_ALLOW_INBOUND=false` preserves the original INPUT DROP behavior for users who need it
- Firewall deny rules (iptables, ip6tables, ipset, nft, init-firewall) added to settings.json -- prevents Claude from tampering with the firewall, which is the primary security boundary
10 changes: 10 additions & 0 deletions tests/test_hooks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,13 @@ def test_checks_edit_and_write(self) -> None:
content = (HOOKS_DIR / "auto-format.sh").read_text(encoding="utf-8")
assert '"Edit"' in content, "auto-format should check Edit tool"
assert '"Write"' in content, "auto-format should check Write tool"


class TestFirewallDenyRules:
"""Verify firewall tampering is denied in settings.json."""

def test_settings_denies_firewall_commands(self) -> None:
settings_path = Path(__file__).parent.parent / ".claude" / "settings.json"
content = settings_path.read_text(encoding="utf-8")
for pattern in ["iptables", "ip6tables", "ipset", "nft", "init-firewall"]:
assert pattern in content, f"settings.json missing firewall deny pattern: {pattern}"
Comment on lines +175 to +179
Copy link

@coderabbitai coderabbitai bot Mar 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Test is too weak: it checks substrings, not actual deny-list entries.

This can pass even if firewall patterns are moved out of permissions.deny. Parse JSON and assert exact deny patterns in the deny array.

Proposed fix 
+import json
@@
 class TestFirewallDenyRules:
@@
     def test_settings_denies_firewall_commands(self) -> None:
         settings_path = Path(__file__).parent.parent / ".claude" / "settings.json"
-        content = settings_path.read_text(encoding="utf-8")
-        for pattern in ["iptables", "ip6tables", "ipset", "nft", "init-firewall"]:
-            assert pattern in content, f"settings.json missing firewall deny pattern: {pattern}"
+        data = json.loads(settings_path.read_text(encoding="utf-8"))
+        deny_rules = set(data.get("permissions", {}).get("deny", []))
+        expected = {
+            "Bash(*iptables *)",
+            "Bash(*ip6tables *)",
+            "Bash(*ipset *)",
+            "Bash(*nft *)",
+            "Bash(*init-firewall*)",
+        }
+        for pattern in expected:
+            assert pattern in deny_rules, f"settings.json missing firewall deny pattern: {pattern}"

Based on learnings: "Implement two-layer defense against data exfiltration: firewall (iptables whitelist in devcontainer) and exfiltration guard (dangerous-actions-blocker.sh hook)".

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/test_hooks.py` around lines 175 - 179, The
test_settings_denies_firewall_commands currently reads settings.json as a raw
string and asserts substrings; instead parse the JSON (use
settings_path.read_text + json.loads) and locate the permissions.deny array,
then assert that each expected pattern (iptables, ip6tables, ipset, nft,
init-firewall) is present as an element in that deny list (e.g., compare
membership or set equality) so the test fails if those patterns were moved out
of permissions.deny; update the test to raise a clear assertion if permissions
or deny are missing.

Loading