Skip to content

Typo fix in common.sh - #2022

Merged
billz merged 2 commits into
RaspAP:masterfrom
1208nn:patch-1
Dec 7, 2025
Merged

Typo fix in common.sh#2022
billz merged 2 commits into
RaspAP:masterfrom
1208nn:patch-1

Conversation

@1208nn

@1208nn 1208nn commented Dec 5, 2025

Copy link
Copy Markdown
Contributor

This pull request makes a minor update to the networking configuration script. The change ensures that the grep command used to check for existing iptables rules is run with sudo, which is necessary for reading the rules file in environments with restricted permissions.

Copilot AI review requested due to automatic review settings December 5, 2025 15:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds sudo to the grep command used to check for existing iptables rules in the network configuration script. This ensures the command can read the iptables rules file (/etc/iptables/rules.v4) in restricted permission environments.

Key Changes

  • Modified the grep command on line 839 to run with sudo when checking for existing iptables rules

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread installers/common.sh Outdated
@billz

billz commented Dec 6, 2025

Copy link
Copy Markdown
Member

Co-pilot is wrong here: sudo isn't needed to grep the contents because the file is readable by group and others (octal 644):

-rw-r--r--   1 root root  319 Dec  6 09:55 rules.v4

the typo fix is valid but minor. please revise this or open a new PR

@1208nn

1208nn commented Dec 7, 2025

Copy link
Copy Markdown
Contributor Author

I deleted it and generated it with sudo netfilter-persistent save again and found on my system (deb 12 bookworm) the file default is -rw-r-----

So it cannot read if without sudo

@billz

billz commented Dec 7, 2025

Copy link
Copy Markdown
Member

Thanks for the additional context. sudo netfilter-persistent save does indeed save these rules with more restrictive permissions (deb 13 trixie). This is different from the method used in the installer:

if [ "$added" = true ]; then
echo "Persisting IP tables rules"
sudo iptables-save | sudo tee $rulesv4 > /dev/null || _install_status 1 "Unable to execute iptables-save"
fi

This isn't an issue on clean installs, or when custom OS images are generated with our pi-gen fork. However, if a user executes netfilter-persistent and then runs the installer, sudo will be necessary.

@1208nn 1208nn changed the title Use sudo for grep in common.sh Typo fix in common.sh Dec 7, 2025
@billz

billz commented Dec 7, 2025

Copy link
Copy Markdown
Member

@1208nn good to merge. thanks for the PR

@billz
billz merged commit f0dbb64 into RaspAP:master Dec 7, 2025
3 checks passed
@1208nn
1208nn deleted the patch-1 branch December 8, 2025 02:12
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.

3 participants