ops: add uninstall.sh, fix firewall service path#1
Conversation
- bin/uninstall.sh: reverses everything setup.sh does (user, firewall, iptables, hidepid, sudoers, wrappers, procview group, pre-commit hook). Supports --dry-run, --keep-home, --yes flags. Idempotent. - setup.sh: add reminder to keep uninstall.sh in sync - hornet-firewall.service: use __REPO_DIR__ placeholder, setup.sh seds the actual path at install time (was hardcoded to old layout) - README: add uninstall commands to operations section
Greptile SummaryAdds comprehensive uninstall functionality that reverses all system changes made by
Confidence Score: 4/5
Important Files Changed
Flowchartflowchart TD
A[Run uninstall.sh] --> B{Check root?}
B -->|Not root| Z1[Exit: Must be root]
B -->|Root| C{Parse flags}
C --> D{--dry-run or --yes?}
D -->|No| E[Show warning + prompt]
E -->|User cancels| Z2[Exit: Aborted]
E -->|User confirms| F[Kill processes]
D -->|Yes| F
F --> G[Stop firewall service]
G --> H[Remove iptables rules]
H --> I[Restore /proc mount]
I --> J[Remove /etc/fstab entry]
J --> K[Remove sudoers]
K --> L[Remove wrappers]
L --> M[Delete procview group]
M --> N[Remove pre-commit hook]
N --> O{--keep-home?}
O -->|Yes| P[userdel hornet_agent]
O -->|No| Q[userdel -r hornet_agent]
P --> R[Show summary]
Q --> R
R --> S[Done]
Last reviewed commit: 3311981 |
| if iptables -w -L HORNET_OUTPUT -n &>/dev/null 2>&1; then | ||
| # Can't match by UID, but can still flush the chain | ||
| # Remove any OUTPUT jumps to HORNET_OUTPUT | ||
| while iptables -w -D OUTPUT -j HORNET_OUTPUT 2>/dev/null; do :; done |
There was a problem hiding this comment.
Bug: The while loop on line 139 executes iptables deletion commands directly, bypassing the --dry-run flag check and making unintended changes to the system's firewall rules.
Severity: MEDIUM
Suggested Fix
Wrap the while loop in a conditional check for the $DRY_RUN flag. If $DRY_RUN is true, print a message indicating what would happen. Otherwise, execute the while loop to delete the rules. This makes the behavior consistent with the rest of the script.
if $DRY_RUN; then
echo " [dry-run] while iptables -w -D OUTPUT -j HORNET_OUTPUT; do :; done"
else
while iptables -w -D OUTPUT -j HORNET_OUTPUT 2>/dev/null; do :; done
fiPrompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: bin/uninstall.sh#L139
Potential issue: In the uninstall script, a `while` loop on line 139 is used to delete
`iptables` rules. This command is executed directly in the loop's condition and is not
wrapped by the `run()` helper function. As a result, the `iptables` deletion command
will execute even when the script is run with the `--dry-run` flag, which is intended to
prevent any changes. This behavior is inconsistent with the rest of the script, where
destructive operations respect the dry-run mode, and it violates the user's expectation
that no firewall rules will be modified.
Did we get this right? 👍 / 👎 to inform future reviews.
* ops: add uninstall.sh, fix firewall service path - bin/uninstall.sh: reverses everything setup.sh does (user, firewall, iptables, hidepid, sudoers, wrappers, procview group, pre-commit hook). Supports --dry-run, --keep-home, --yes flags. Idempotent. - setup.sh: add reminder to keep uninstall.sh in sync - hornet-firewall.service: use __REPO_DIR__ placeholder, setup.sh seds the actual path at install time (was hardcoded to old layout) - README: add uninstall commands to operations section * fix: add -w flag to iptables in firewall service ExecStop --------- Co-authored-by: Ben Tossell <ben@modem.dev>
bin/uninstall.sh: reverses everything setup.sh doessetup.sh: add reminder to keep uninstall.sh in synchornet-firewall.service: use REPO_DIR placeholder