Update nft rules to block all protocols when no voucher

[vractal]

PR Type

Bug fix

---

Description

• Fix typo in table name from 'pirania' to 'pireania' in forward chain

• Remove commented-out legacy iptables code and unused rules

• Add default drop rule for unauthorized MAC addresses

• Clean up whitespace and formatting inconsistencies

---

Diagram Walkthrough

flowchart LR
  A["captive-portal script"] -->|Fix typo| B["forward chain table name"]
  A -->|Remove legacy code| C["Clean up commented rules"]
  A -->|Add default rule| D["Drop unauthorized MACs"]
  A -->|Format cleanup| E["Whitespace normalization"]

Loading

File Walkthrough

Relevant files

Bug fix

captive-portal Fix nftables typo and remove legacy code                                  packages/pirania/files/usr/bin/captive-portal Fixed typo in nftables chain definition: 'pirania' changed to 'pireania' for forward chain Removed multiple commented-out legacy iptables implementation code blocks Added new rule to drop all packets from unauthorized MAC addresses as default policy Cleaned up trailing whitespace and formatting throughout the file +8/-20

---

[devloai]

Unable to trigger custom agent "Code Reviewer"You have run out of credits 😔
Please upgrade your plan or buy additional credits from the subscription page.

[ai-coding-guardrails]

I've got 2 comments for you to consider

The PR title and description are not accurate. Here are my suggestions:

Proposed title: Fix nftables rules to block all protocols for unauthorized devices and remove legacy iptables code

Proposed description:

---

Updates the captive portal script to use nftables instead of iptables for blocking unauthorized devices. Adds a comprehensive drop rule to block all protocols when no voucher is present, and removes commented legacy iptables code. Note: Contains a critical typo in the nftables table name that needs to be addressed before merging.

---

Did you know we can integrate this feedback directly into 50+ IDEs? Get setup in just one command

[ai-coding-guardrails]

Typo in table name: 'pireania' should be 'pirania' to match the table created on line 25. This will cause the forward chain to be created in a non-existent table, breaking the firewall rules.

Why did I show this?

Category: bug
Comment Quality: high

Based on general best practices

Tools used:

1. search_in_file, {'pattern': 'def .*', 'file_path': 'packages/pirania/files/usr/bin/captive-portal'}
2. get_file_lines, {'start_line': 1, 'end_line': 114, 'file_path': '/etc/config/pirania'}
3. search_in_file, {'pattern': 'pirania.*uci', 'file_path': 'packages/pirania/files/usr/bin/captive-portal'}
4. search_in_file, {'pattern': 'pirania.*uci', 'file_path': 'packages/pirania/files/usr/bin/captive-portal'}

[qodo-free-for-open-source-projects]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🔴	Firewall table name typo Description: Typo in table name creates a forward chain in table 'pireania' instead of 'pirania', causing the forward chain to be orphaned and not cleaned up by clean_tables(), potentially leaving stale firewall rules. captive-portal [29-29] Referred Code nft add chain inet pireania forward { type filter hook forward priority 0 \; }
⚪	Overly broad drop rule Description: The new default drop rule for unauthorized MACs may block legitimate traffic that should be allowed through other protocols or services, and lacks proper logging to diagnose blocked connections, potentially causing denial of service for valid use cases. captive-portal [55-55] Referred Code nft add rule inet pirania prerouting ether saddr != @pirania-auth-macs drop }
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: MAC Address Logging: The code logs MAC addresses which may be considered PII in some jurisdictions, requiring verification of compliance requirements. Referred Code nft add rule inet pirania prerouting ether saddr @pirania-auth-macs ct state new,established,related counter log prefix "ValidSMAC" accept nft add rule inet pirania prerouting ip daddr @pirania-allowlist-ipv4 ct state new,established,related counter log prefix "ACCEPT-ipv4" accept nft add rule inet pirania prerouting ip6 daddr @pirania-allowlist-ipv6 ct state new,established,related counter log prefix "ACCEPT-ipv6" accept # send DNS requests, that are not from valid ips or macs, to our own captive portal DNS at 59053 nft add rule inet pirania prerouting meta l4proto udp udp dport 53 ether saddr != @pirania-auth-macs ct state new,established,related counter log prefix "SMACDNS" redirect to :59053 # redirect packets with dest port 80 to port 59080 of this host (the captive portal page). nft add rule inet pirania prerouting meta l4proto tcp tcp dport 80 ether saddr != @pirania-auth-macs ct state new,established,related counter log prefix "SMACHTTP" redirect to :59080 # drop packets with dest port 443 for unauthorized macs (block HTTPS) nft add rule inet pirania prerouting meta l4proto tcp tcp dport 443 ether saddr != @pirania-auth-macs ct state new,established,related counter log prefix "SMACHTTPS" drop # reject nft add rule inet pirania prerouting ether saddr != @pirania-auth-macs drop Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Input Validation Missing: MAC addresses and IP allowlist values from external sources are used without visible validation, which may allow injection attacks. Referred Code for mac in $(pirania_authorized_macs) ; do nft add element inet pirania pirania-auth-macs {$mac} echo "Adicionando enderecos:" $mac done # Update pirania-allowlist sets for ipv4 and ipv6 nft flush set inet pirania pirania-allowlist-ipv4 nft flush set inet pirania pirania-allowlist-ipv6 # Add allowed ip/prefixes # Get values from allowlist_ipvX and add to pirania-allowlist-ipvX set ipv4allowlist=$(uci get pirania.base_config.allowlist_ipv4 | sed 's/ /,/g') nft add element inet pirania pirania-allowlist-ipv4 {$ipv4allowlist} Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🔴	Firewall rule bypass Description: The new rule setup creates the forward chain under inet pireania (typo) instead of inet pirania, which can prevent intended forward-path filtering from being applied and may allow unintended traffic forwarding/bypass of captive-portal enforcement. captive-portal [24-30] Referred Code nft create table inet pirania # Create default tables and chains nft add table inet pirania nft add chain inet pirania prerouting { type nat hook prerouting priority 0 \; } nft add chain inet pirania input { type filter hook input priority 0 \; } nft add chain inet pireania forward { type filter hook forward priority 0 \; }
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Table name typo: A new chain is created under inet pireania while the rest of the rules use inet pirania, introducing a misleading/incorrect identifier that obscures intent and likely breaks the ruleset. Referred Code nft add chain inet pireania forward { type filter hook forward priority 0 \; } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Unhandled nft failure: The script adds an nft command targeting inet pireania without checking exit codes, so a runtime failure can silently prevent correct firewall setup and leave the system in an unintended state. Referred Code # Create pirania tables nft create table inet pirania # Create default tables and chains nft add table inet pirania nft add chain inet pirania prerouting { type nat hook prerouting priority 0 \; } nft add chain inet pirania input { type filter hook input priority 0 \; } nft add chain inet pireania forward { type filter hook forward priority 0 \; } Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Incomplete audit context: The newly added enforcement behavior (default drop for unauthorized MACs) is not clearly paired with audit logging that includes actor identity (e.g., user/voucher/session) needed to reconstruct authorization decisions. Referred Code # drop packets with dest port 443 for unauthorized macs (block HTTPS) nft add rule inet pirania prerouting meta l4proto tcp tcp dport 443 ether saddr != @pirania-auth-macs ct state new,established,related counter log prefix "SMACHTTPS" drop # reject nft add rule inet pirania prerouting ether saddr != @pirania-auth-macs drop Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Unstructured log output: The added/modified packet logging uses free-form log prefix strings rather than structured logs, so it may not meet structured logging requirements depending on downstream log processing expectations. Referred Code nft add rule inet pirania prerouting ether saddr @pirania-auth-macs ct state new,established,related counter log prefix "ValidSMAC" accept nft add rule inet pirania prerouting ip daddr @pirania-allowlist-ipv4 ct state new,established,related counter log prefix "ACCEPT-ipv4" accept nft add rule inet pirania prerouting ip6 daddr @pirania-allowlist-ipv6 ct state new,established,related counter log prefix "ACCEPT-ipv6" accept # send DNS requests, that are not from valid ips or macs, to our own captive portal DNS at 59053 nft add rule inet pirania prerouting meta l4proto udp udp dport 53 ether saddr != @pirania-auth-macs ct state new,established,related counter log prefix "SMACDNS" redirect to :59053 # redirect packets with dest port 80 to port 59080 of this host (the captive portal page). nft add rule inet pirania prerouting meta l4proto tcp tcp dport 80 ether saddr != @pirania-auth-macs ct state new,established,related counter log prefix "SMACHTTP" redirect to :59080 # drop packets with dest port 443 for unauthorized macs (block HTTPS) nft add rule inet pirania prerouting meta l4proto tcp tcp dport 443 ether saddr != @pirania-auth-macs ct state new,established,related counter log prefix "SMACHTTPS" drop # reject nft add rule inet pirania prerouting ether saddr != @pirania-auth-macs drop Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-free-for-open-source-projects]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Move drop rule to correct chain Move the new drop rule for unauthorized MAC addresses from the prerouting chain to the forward chain to ensure captive portal redirection rules are processed first. packages/pirania/files/usr/bin/captive-portal [54-55] # reject - nft add rule inet pirania prerouting ether saddr != @pirania-auth-macs drop + nft add rule inet pirania forward ether saddr != @pirania-auth-macs drop Apply / Chat Suggestion importance[1-10]: 10 __ Why: The suggestion correctly points out a critical logic error where the new drop rule in the prerouting chain would block traffic before it can be redirected to the captive portal, defeating its purpose. Moving the rule to the forward chain is the correct fix.	High
	✅ Fix typo in nftables chain Suggestion Impact: The suggestion was directly implemented. The typo 'pireania' on line 5 was corrected to 'pirania', exactly as suggested. code diff: - nft add chain inet pireania forward { type filter hook forward priority 0 \; } + nft add chain inet pirania forward { type filter hook forward priority 0 \; } Correct the typo in the nft command by changing the table name from pireania to pirania to match the table creation command. packages/pirania/files/usr/bin/captive-portal [29] -nft add chain inet pireania forward { type filter hook forward priority 0 \; } +nft add chain inet pirania forward { type filter hook forward priority 0 \; } [Suggestion processed] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a typo pireania introduced in the PR, which would cause the script to fail as the nftables table is named pirania. This is a critical bug that breaks the script's functionality.	High
Update

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Move drop rule to avoid blocking DHCP Move the default drop rule for unauthorized MACs from the prerouting chain to the forward chain to avoid blocking essential network services like DHCP. packages/pirania/files/usr/bin/captive-portal [54-55] # reject - nft add rule inet pirania prerouting ether saddr != @pirania-auth-macs drop + nft add rule inet pirania forward ether saddr != @pirania-auth-macs drop Apply / Chat Suggestion importance[1-10]: 10 __ Why: This suggestion identifies a critical logic flaw where dropping packets in the prerouting chain would block essential services like DHCP, making it impossible for new clients to connect and reach the captive portal.	High
	✅ Fix typo in nftables chain name Suggestion Impact: The suggestion was directly implemented. The typo "pireania" was corrected to "pirania" in line 6 of the diff, exactly as suggested. code diff: - nft add chain inet pireania forward { type filter hook forward priority 0 \; } + nft add chain inet pirania forward { type filter hook forward priority 0 \; } Correct the typo in the table name from pireania to pirania when adding the forward chain. packages/pirania/files/usr/bin/captive-portal [29] - nft add chain inet pireania forward { type filter hook forward priority 0 \; } + nft add chain inet pirania forward { type filter hook forward priority 0 \; } [Suggestion processed] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a typo (pireania instead of pirania) in the table name, which would cause the nft command to fail and prevent the firewall rules from being applied correctly.	High
More