Skip to content

net/fwblack: add new package - #30509

Open
EdgeBites wants to merge 1 commit into
openwrt:masterfrom
EdgeBites:net-fwblack-add
Open

EdgeBites wants to merge 1 commit into
openwrt:masterfrom
EdgeBites:net-fwblack-add

Conversation

@EdgeBites

Copy link
Copy Markdown

DNS-based nftables blocklist daemon for OpenWrt (IPv4+IPv6, firewall4/nftables, procd/UCI).

Upstream: https://github.com/EdgeBites/fw-black-luci, v1.0.2 tarball (codeload) with PKG_HASH verified. Daemon-only; LuCI app (luci-app-fwblack) to follow separately in openwrt/luci.

Tests:

  • ash -n clean on all scripts, nft -c -f OK
  • Upstream CI green (shellcheck -S warning -s sh, make stub parse, JSON/VERSION/hygiene, OpenWrt rootfs smoke with nft load + v4/v6 inserts + fw4 reload idempotent)
  • Functional test.sh / test-version.sh / pre-test.sh included (no grep -q, exit 1 on unknown package)

Maintainer: Calin Vlad calin@edgebites.com, MIT.

DNS-based nftables blocklist daemon for OpenWrt (IPv4+IPv6, firewall4, procd/UCI). Upstream v1.0.2 tarball with ash-clean scripts, nft ruleset, uci-defaults and functional test scripts.

Signed-off-by: Calin Vlad <calin@edgebites.com>
@openwrt

openwrt Bot commented Sep 12, 2026

Copy link
Copy Markdown

Formality Check: Failed

We checked this pull request against the contribution guidelines. Here is what needs your attention:

🛑 CRITICAL ERRORS

Commit 88bfd63 - net/fwblack: add new package:

  • Commit author email 'calin@edgebites.com' is not linked to any registered GitHub account. Please add and verify this email in your GitHub profile settings.
  • Line 3 in commit body exceeds max width (185/100 chars)

Commit 88bfd63 - net/fwblack: add new package:

  • Move 'PKGARCH:=all' into the 'define Package/' block: the build system resets 'PKGARCH' before reading the package definition, so a top-level assignment has no effect.

Tip

Do not close this pull request to make corrections. Instead, modify your existing commits (e.g. git commit --amend) and update the branch using git push --force-with-lease --force-if-includes. The checks will re-run automatically.


Something broken? Consider reporting an issue.
Running version 059e3de deployed on 2026-09-09 11:53:18 CEST

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 1 commit; findings inline.


Generated by Claude Code

Comment thread net/fwblack/Makefile
Comment on lines +19 to +20
define Build/Prepare
endef

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PKG_SOURCE is set here, so an empty Build/Prepare suppresses $(PKG_UNPACK) and $(PKG_BUILD_DIR) stays empty — every $(PKG_BUILD_DIR)/files/... path in Package/fwblack/install (and PKG_LICENSE_FILES) then has nothing to copy. The empty-Build/Prepare idiom is only for source-less packages that install from ./files/ (e.g. net/adblock). Remove this block and keep only the empty Build/Configure/Build/Compile.


Generated by Claude Code

Comment thread net/fwblack/Makefile
PKG_VERSION:=1.0.2
PKG_RELEASE:=1

PKG_SOURCE:=fw-black-luci-$(PKG_VERSION).tar.gz

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The codeload tarball's top-level directory is fw-black-luci-1.0.2, but PKG_UNPACK extracts into $(PKG_BUILD_DIR)/.. while PKG_BUILD_DIR defaults to $(BUILD_DIR)/fwblack-$(PKG_VERSION) — so the tree lands beside the build dir, not in it. An override is needed (must be before include $(INCLUDE_DIR)/package.mk).

Suggested change
PKG_SOURCE:=fw-black-luci-$(PKG_VERSION).tar.gz
PKG_SOURCE:=fw-black-luci-$(PKG_VERSION).tar.gz
PKG_BUILD_DIR:=$(BUILD_DIR)/fw-black-luci-$(PKG_VERSION)

Generated by Claude Code

Comment thread net/fwblack/Makefile
if [ -f /etc/nftables.d/ruleset-post/fwblack.nft ]; then
rm -f /etc/nftables.d/ruleset-post/fwblack.nft
fi
/etc/init.d/fwblack enable 2>/dev/null || true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

default_postinst already enables and starts every /etc/init.d/ file the package ships, and deliberately skips enable when PKG_UPGRADE=1; default_prerm does the mirror image for disable. The unconditional enable here plus the unconditional disable on line 104 bypass that guard, so every package upgrade re-enables the service for a user who had disabled it. Drop this line and the stop/disable pair on lines 103-104.


Generated by Claude Code

Comment thread net/fwblack/Makefile
Comment on lines +92 to +94
if [ -f /etc/nftables.d/ruleset-post/fwblack.nft ]; then
rm -f /etc/nftables.d/ruleset-post/fwblack.nft
fi

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

/etc/nftables.d/ is firewall4's user conffile directory, and this package never installs there — it installs to /usr/share/nftables.d/ruleset-post/. This runs outside the .migrated guard, so it silently deletes a hand-written user include on every install and upgrade; gate it behind the one-shot marker or drop it.


Generated by Claude Code

Comment thread net/fwblack/Makefile
Comment on lines +86 to +87
cp /etc/fwblack/blocklist.cfg /etc/fwblack/blocklist.cfg.ppkg-default
cp /etc/fw.black/blocklist.cfg /etc/fwblack/blocklist.cfg

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • /etc/fwblack/blocklist.cfg is declared in Package/fwblack/conffiles, so the package manager already preserves whatever the user has there. Overwriting it with the legacy /etc/fw.black copy discards the file the daemon is actually running with; the safer direction is to leave the existing conffile alone and stash the legacy one for the user to merge.
  • nit: .ppkg-default looks like a typo for .opkg-default.

Generated by Claude Code

Comment thread net/fwblack/test.sh
# (PKG_NAME/PKG_VERSION are also provided as environment variables).
# NOTE: plain grep without -q here - matches stay visible in CI logs.
name="${1:-$PKG_NAME}"
version="${2:-$PKG_VERSION}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Without set -e the cmd && echo ... lines below swallow their own failures: a broken ash -n or nft -c just skips the echo, and the script still exits with the status of the final grep, so CI reports a pass. Only the last command in the branch can actually fail the test.

Suggested change
version="${2:-$PKG_VERSION}"
version="${2:-$PKG_VERSION}"
set -e

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants