Skip to content

luci-app-fwlive: add firewall live view - #8992

Open
lucas-albers-lz4 wants to merge 6 commits into
openwrt:masterfrom
lucas-albers-lz4:luci-app-fwlive-add
Open

luci-app-fwlive: add firewall live view#8992
lucas-albers-lz4 wants to merge 6 commits into
openwrt:masterfrom
lucas-albers-lz4:luci-app-fwlive-add

Conversation

@lucas-albers-lz4

@lucas-albers-lz4 lucas-albers-lz4 commented Aug 30, 2026

Copy link
Copy Markdown

Summary

Add luci-app-fwlive (Firewall Live View): a Status menu view that polls
firewall-only log lines via a dedicated rpcd plugin (rules, poll,
resolve, logging_status, enable_wan_logging, disable_wan_logging).

License: Apache-2.0 (PKG_LICENSE). This PR is a snapshot of
https://github.com/lucas-albers-lz4/fwlive (signed binary feed remains for
non-snapshot users), not a move.

Maintenance

fwlive stays the development home. This tree is a snapshot. On conflict, the
next snapshot from that repository replaces this copy — land fixes there
first. A PKG_SOURCE tarball was considered and declined: LuCI applications
are in-tree under applications/, and Weblate owns po/<lang>/ after merge.

Design notes

  • Sessions never get ubus log.read. The plugin filters logs as root
    under a narrow ACL (read/write scopes kept separate).
  • WAN log=1 is opt-in from the UI; prerm restores the saved baseline
    (fail-open).
  • First commit ships po/templates only. Locales can come from Weblate
    after merge. Please do not hand-add .po files in follow-ups that would
    clobber Weblate.
  • PKG_VERSION / PKG_RELEASE kept in lockstep with the out-of-tree app.
  • Generated shell/JS snapshots come from the fwlive monorepo generators;
    Node/core/ are not vendored here.
  • Depends: luci-base, logd, jsonfilter. Optional hostnames use
    BusyBox nslookup (stock image).

Review follow-up (second commit)

Addresses the first-round findings: rules map assembled in the current shell
(nft/iptables labels reach the client); reverse DNS via nslookup;
jsonfilter declared and no longer a silent empty table; poll classifier is
one awk pass; json_escape is self-contained for prerm; UCI names with
whitespace are not word-split.

Test plan

  • Build luci-app-fwlive against a current OpenWrt SDK / image builder
  • Install; confirm Status → Firewall Live View menu appears
  • With fw4/nft, confirm rules with log prefix / !fw4: comments show labels
  • Poll updates; busy WAN-logging does not stall the 1s interval
  • Opt-in WAN logging; confirm events; disable / uninstall restores baseline
  • Confirm session ACL cannot call log.read directly
  • Show hostnames: a PTR-backed IP resolves on a stock image (no extra pkgs)

Add a Status menu view that polls firewall-only log lines via a dedicated
rpcd plugin (rules, poll, resolve, logging_status, enable/disable WAN
logging). Sessions never receive ubus log.read; the plugin filters as root
under a narrow ACL.

WAN zone log=1 is opt-in from the UI and restored on package removal
(prerm). First commit ships po/templates only; Weblate can add locales
after merge. License: Apache-2.0 (PKG_LICENSE). Maintained out-of-tree at
https://github.com/lucas-albers-lz4/fwlive with a signed binary feed for
non-snapshot users; this tree is a copy for the luci feed.

Signed-off-by: Lucas Albers <lucas.b.albers@gmail.com>
@openwrt openwrt Bot added the add package Introduces a new package Makefile build script label Aug 30, 2026

@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 new commit (91b4629). The commit message matches the diff; no commit-level issues.

Two of the inline findings look like they break advertised features on a stock image rather than being polish: the rules map is assembled inside pipeline subshells so the nft/iptables log-prefix labels never reach the client (the __rulesmap_iptables selftest uses a redirect and so passes), and resolve calls getent, which is not present anywhere in OpenWrt. Worth confirming both against a real device before merge, since the current code fails silently in each case.


Generated by Claude Code


case "$backend" in
nft)
nft_list_ruleset | map_from_nft_stream

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.

map_from_nft_stream runs as the right-hand side of a pipeline, so in POSIX sh (BusyBox ash / dash — this file is #!/bin/sh) it executes in a subshell. Every map_add it performs updates a copy of OUT, which is discarded when the subshell exits. By the time line 229 prints the result, OUT still holds only what map_uci_rule_names added — none of the nft log prefix / comment "!fw4: ..." mappings ever reach the client. Same for the iptables-save / ip6tables-save pipelines on lines 222 and 224.

Verified with dash:

$ OUT=''; add(){ OUT="${OUT}${OUT:+,}$1"; }; stream(){ while IFS= read -r l; do add "$l"; done; }
$ printf 'a\nb\n' | stream ; echo "[$OUT]"
[]
$ printf 'a\nb\n' > in ; stream < in ; echo "[$OUT]"
[a,b]

Note the selftest path does not hit this: rulesmap_from_iptables_file feeds the same function via a redirect (map_from_iptables_save_stream < "$file"), which stays in the current shell — so __rulesmap_iptables passes while the production path silently returns an almost-empty rules map.

Fix by feeding the stream functions through a redirect rather than a pipe (spool the nft list ruleset / iptables-save output to a temp file first, or have the stream functions emit fragments on stdout and accumulate with command substitution). Deliberate use of the pipeline subshell elsewhere (fwlive-log-filter.sh keeps sep local that way) makes this one look accidental rather than intended.


Generated by Claude Code

ip="$1"
[ -n "$ip" ] || return 1
is_resolvable_address "$ip" || return 1
line=$(run_with_timeout "$RESOLVE_TIMEOUT" getent hosts "$ip" | head -1)

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.

getent does not exist on a stock OpenWrt image, so resolve can never return a name and the "show hostnames" feature is permanently dead:

  • musl ships no getent (it is a glibc/NSS utility), and OpenWrt's glibc runtime package installs only ldd — see package/libs/toolchain/Makefile;
  • BusyBox has no getent applet (nothing in package/utils/busybox/config/ declares one);
  • a full-tree grep of openwrt/openwrt at 93cf01b and of openwrt/packages at master finds no package providing getent and no in-tree script that calls it.

run_with_timeout swallows the command not found, line comes back empty, resolve_hostname returns 1, and the method answers {"names":{}} on every call — indistinguishable from "no PTR record", so the client's hostnameFailed TTL cache just retries forever.

Please either switch to a tool that exists on the target (BusyBox nslookup <ip> does reverse lookups) or, if getent is genuinely intended, add the providing package to LUCI_DEPENDS in the Makefile and name it in the README's Dependencies section. A command -v probe surfaced through logging_status/blockers would also stop the UI from offering a toggle that cannot work.


Generated by Claude Code

Comment on lines +269 to +271
if [ "$lines" -gt "$POLL_LINES_MAX" ] 2>/dev/null; then
lines="$POLL_LINES_MAX"
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.

The clamp is bypassable with an over-long digit string. case "$first" in ''|*[!0-9]*) on line 265 accepts any number of digits, and test -gt then overflows instead of comparing — the 2>/dev/null hides the diagnostic and the failing test takes the else branch, leaving lines unclamped for the JSON interpolation on line 274:

$ dash -c 'lines=99999999999999999999; if [ "$lines" -gt 2000 ] 2>/dev/null; then echo clamp; else echo "NOCLAMP lines=$lines"; fi'
NOCLAMP lines=99999999999999999999

A read-ACL session can therefore drive ubus call log read '{"lines":<huge>}'. ({"addresses":["0"]} is also accepted and forwards "lines":0, which is not a value the caller can mean.) Rejecting anything longer than POLL_LINES_MAX before the numeric comparison closes both:

Suggested change
if [ "$lines" -gt "$POLL_LINES_MAX" ] 2>/dev/null; then
lines="$POLL_LINES_MAX"
fi
# 5+ digits necessarily exceeds POLL_LINES_MAX (2000) and would overflow
# `test -gt`, whose failure would otherwise skip the clamp entirely.
case "$lines" in
[0-9][0-9][0-9][0-9][0-9]*) lines="$POLL_LINES_MAX" ;;
0) lines=50 ;;
esac
if [ "$lines" -gt "$POLL_LINES_MAX" ] 2>/dev/null; then
lines="$POLL_LINES_MAX"
fi

Generated by Claude Code

if [ -z "$val" ]; then
printf 'null'
else
esc=$(printf '%s' "$val" | json_escape)

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.

json_escape is not defined in this file — it only exists in root/usr/libexec/rpcd/fwlive, which sources this one. Every other consumer sees an undefined command.

That is exactly what the package's own prerm does: Makefile:27-32 runs . /usr/libexec/fwlive-logging.sh standalone and calls restore_wan_log_baseline, which reaches json_null_or_stringjson_escape. On uninstall that emits json_escape: not found on stderr and silently yields an empty esc. Same for logging_blockers_append on line 249.

The value is only used in JSON error strings here, so nothing is corrupted today — but the dependency is inverted and will bite the first time a status field is derived from it. Move json_escape into this file (or a third shared fwlive-json.sh sourced by both) so fwlive-logging.sh is self-contained.


Generated by Claude Code

Comment on lines +28 to +34
while IFS= read -r entry || [ -n "$entry" ]; do
[ -n "$entry" ] || continue
msg=$(printf '%s' "$entry" | jsonfilter -e '@.msg' 2>/dev/null)
if is_firewall_event_msg "$msg"; then
printf '%s%s' "$sep" "$entry"
sep=','
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.

This loop is per-entry, and the multiplier upstream is large. The view registers poll.add(this.pollFn, 1) and every tick asks for the full ring — [`callFwlivePoll({ addresses: [ String(constants.FETCH_LINES_MAX) ] })`](https://github.com/openwrt/luci/blob/91b462957c8dfc3cec197b22124d7c26e59bbb92/applications/luci-app-fwlive/htdocs/luci-static/resources/view/status/fwlive.js#L734)`` with FETCH_LINES_MAX: 2000, regardless of the user's row limit.

Per entry this spawns one jsonfilter (line 30) plus, inside is_firewall_event_msg, one sed with 15 expressions, one tr, and up to eight grep invocations (_detect_action, _has_kv ×N, _has_firewall_hint). That is roughly ten fork+exec per log line, i.e. on the order of 10–20k processes per second on a busy WAN-logging router, once per second, for every open browser tab. On the low-end targets LuCI status pages are expected to run on, that will dominate the CPU.

Two changes would fold it to a near-constant cost per poll:

  • extract @.log[*].msg in the same jsonfilter pass (or emit entry+msg together) instead of re-invoking jsonfilter per entry;
  • reimplement is_firewall_event_msg as a single awk program that reads the whole batch on stdin and prints the passing entries — the classification is all regex/substring work that awk does natively, and it removes the sed/tr/grep chain entirely.

Sending only rowLimit lines rather than always 2000, and a poll interval above 1s, would reduce the multiplier as well.


Generated by Claude Code

Comment thread applications/luci-app-fwlive/Makefile Outdated
Comment on lines +12 to +16

# Reproducible build: honor SOURCE_DATE_EPOCH from the build environment (set by docker-sdk.sh / CI).
ifdef SOURCE_DATE_EPOCH
export SOURCE_DATE_EPOCH
endif

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.

nit: this block is a no-op here. SOURCE_DATE_EPOCH is already exported by the buildroot for every package build, and no other package Makefile in this tree (applications/, modules/, protocols/, themes/) carries such a block. The comment also refers to docker-sdk.sh, which is tooling from the out-of-tree fwlive repo and does not exist in openwrt/luci.

Suggested change
# Reproducible build: honor SOURCE_DATE_EPOCH from the build environment (set by docker-sdk.sh / CI).
ifdef SOURCE_DATE_EPOCH
export SOURCE_DATE_EPOCH
endif

Generated by Claude Code

Comment on lines +9 to +10
/* Keep in sync with Makefile PKG_VERSION. */
APP_VERSION: '0.1.36',

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.

nit: a hand-maintained "keep in sync" constant will drift. luci.mk's SubstituteVersion only rewrites PKG_VERSION markers in *.htm and *.utluci.mk:288-298 — so nothing in the build will catch APP_VERSION falling behind PKG_VERSION:=0.1.36, and with the app also versioned in the out-of-tree repo that is three places to bump. If the value is only shown in an "about" line, consider reading it from the installed package metadata (or dropping it) instead of duplicating the literal.


Generated by Claude Code

Comment on lines +39 to +42
## Documentation

- **Users:** [installation guide](https://github.com/lucas-albers-lz4/fwlive/blob/master/docs/user/installation.md)
- **Developers:** [developer documentation](https://github.com/lucas-albers-lz4/fwlive/blob/master/docs/developer/README.md)

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.

nit: the PR description states this is "a copy into the luci feed, not a move", with the canonical sources (and a separate signed binary feed) staying at lucas-albers-lz4/fwlive. That leaves two copies of ~5.4k lines that will diverge — and the generated artifacts here (fwlive-is-firewall-event.sh is marked "Snapshot from the fwlive monorepo … Do not edit by hand. … regenerate upstream of this tree", and constants.js mirrors the upstream PKG_VERSION) mean a LuCI maintainer cannot fix a bug in-tree without it being overwritten by the next snapshot.

Worth stating explicitly for the maintainers: which tree wins when they conflict, and are you committing to keeping this copy current in openwrt/luci? luci-app-https-dns-proxy is the cautionary precedent this feed already has for an app maintained in two places. If the answer is that upstream wins, a PKG_SOURCE-based package pulling the release tarball would avoid the duplication entirely.

Also, the README's Dependencies section lists only luci-base and logd; getent (see the resolve comment) and the assumption that jsonfilter is present belong there too.


Generated by Claude Code

Fix the eight openwrt-ai findings plus two local defects on the first
luci-app-fwlive PR.

The rules map is assembled in the current shell so nft/iptables labels
reach the client; duplicate keys are suppressed. Reverse DNS uses BusyBox
nslookup (getent is absent on stock images). jsonfilter is a declared
depend; a missing filter is logged instead of returning an empty table.
The poll classifier is one awk pass so a 2000-line batch stays inside
the 1s interval. json_escape lives in fwlive-logging.sh so prerm is
self-contained. UCI rule names with whitespace are not word-split.

This tree is a snapshot of lucas-albers-lz4/fwlive. On conflict, the next
snapshot from that repository replaces this copy. A PKG_SOURCE tarball
was considered and declined: LuCI apps are in-tree, and Weblate owns
po/<lang>/ after merge.

Signed-off-by: Lucas Albers <lucas.b.albers@gmail.com>
lucas-albers-lz4 added a commit to lucas-albers-lz4/fwlive that referenced this pull request Aug 31, 2026
Replace getent with nslookup, move json_escape into logging.sh, declare
jsonfilter, skip whitespace-split UCI names, and rewrite the classifier
to one awk pass so a 2000-line poll stays inside budget.

Closes #218, Closes #219, Closes #220, Closes #222, Closes #224,
Closes #225, Closes #226
Refs #216
lucas-albers-lz4 added a commit to lucas-albers-lz4/fwlive that referenced this pull request Aug 31, 2026
Replace getent with nslookup, move json_escape into logging.sh, declare
jsonfilter, skip whitespace-split UCI names, and rewrite the classifier
to one awk pass so a 2000-line poll stays inside budget.

Closes #218, Closes #219, Closes #220, Closes #222, Closes #224,
Closes #225, Closes #226
Refs #216
lucas-albers-lz4 added a commit to lucas-albers-lz4/fwlive that referenced this pull request Aug 31, 2026
Replace getent with nslookup, move json_escape into logging.sh, declare
jsonfilter, skip whitespace-split UCI names, and rewrite the classifier
to one awk pass so a 2000-line poll stays inside budget.

Closes #218, Closes #219, Closes #220, Closes #222, Closes #224,
Closes #225, Closes #226
Refs #216
lucas-albers-lz4 added a commit to lucas-albers-lz4/fwlive that referenced this pull request Aug 31, 2026
Merged after human sign-off. Covers #218#220, #222, #224#226 and review folds. Stacked on #227.
Re-cut from lucas-albers-lz4/fwlive after merging the openwrt#227/openwrt#228 review
wave: rules map delivery, poll clamp, nslookup resolver, jsonfilter declare,
classifier codegen, json_escape in logging.sh, and related host-test fixes.
Refresh the .pot via i18n-scan.pl.

Signed-off-by: Lucas Albers <lucas.b.albers@gmail.com>
lucas-albers-lz4 added a commit to lucas-albers-lz4/fwlive that referenced this pull request Aug 31, 2026
Fold the openwrt/luci#8992 review wave (#227/#228): rules map delivery,
poll clamp, nslookup resolver, jsonfilter declare, classifier codegen, and
related security fixes. Refresh the .pot after the upstream cut.
Re-cut from fwlive tip with BusyBox-safe WAN lock, jsonfilter stdin,
poll reply.error, !fw4: precedence, iptables-save timeout, and rules
map bounds. Refresh po template via i18n-scan.pl.

Signed-off-by: Lucas Albers <lucas.b.albers@gmail.com>
@lucas-albers-lz4

Copy link
Copy Markdown
Author

Re-cut from fwlive with the post-review follow-ups (BusyBox-safe WAN lock dir check, jsonfilter stdin for large rings, poll error surfaced in the UI, !fw4: label precedence, iptables-save timeout + rules map bounds).

Fork tip: lucas-albers-lz4/luci luci-app-fwlive-add @ 2defca544d.

@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 3 new commits since 91b4629.

The previous round's two functional findings are genuinely fixed: the rules map is now assembled in the current shell via <"$_rules_dump" redirects (and __rulesmap_iptables exercises the same two-pass path), and resolve uses BusyBox nslookup. poll_clamp_lines and the single-awk classifier both check out — I ran dash root/usr/libexec/rpcd/fwlive __selftest and _fwlive_filter_json_entries against a sample batch locally and they behave as documented. The stat -ctest -O + find -perm swap in #232 is correct for stock OpenWrt: BUSYBOX_DEFAULT_FEATURE_STAT_FORMAT is n while FEATURE_FIND_PERM, FEATURE_FIND_PAREN and FEATURE_FIND_PRUNE are all y in package/utils/busybox/Config-defaults.in.

One of the inline findings is a hard blocker for a feature the PR advertises: the nslookup resolver now works, but the client double-unwraps the reply (expect: { names: {} } and res.names), so hostnames are still never displayed. Worth confirming on a device before merge.

Commit checks

  • 05a223d2 "luci-app-fwlive: sync #8992 with fwlive master" — the message does not match the diff. It claims "Refresh the .pot via i18n-scan.pl", but the commit touches no po/ file (only fwlive-is-firewall-event.sh, fwlive-logging.sh, rpcd/fwlive). It also credits itself with "rules map delivery, poll clamp, nslookup resolver, jsonfilter declare, classifier codegen, json_escape in logging.sh" — all of which landed in the preceding commit 79019f0; jsonfilter declare is a Makefile change this commit does not contain. What it actually does is add the _fwlive_tmp_dir_ok sticky-bit gate on /tmp, strip the trailing dot in the Address N: branch of parse_nslookup_name, add the __tmp_dir_ok CLI hook, and reorder the selftest so run_logging_selftest runs before the jshn-gated early return. Please rewrite the body to describe that.

Generated by Claude Code

if (gen !== this.resolveGeneration)
return;

const names = (res && res.names) || {};

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 reply is unwrapped twice, so this is always {} and "Show hostnames" still cannot display a name.

callFwliveResolve declares expect: { names: {} },`` and rpc.js applies that before resolving the promise — ret = ret[key], then the type check falls back to the default. So `res` is the names map (`{"8.8.8.8":"dns.google"}`); `res.names` is `undefined`, and `names` is `{}` on every call.

Consequence: names[ip] is never truthy, every IP in need takes the else branch and gets hostname.failMarked, and updated stays false. The getent → BusyBox nslookup fix in 79019f0 makes the backend work, but the client discards the result, so the feature is still dead end-to-end.

Suggested change
const names = (res && res.names) || {};
const names = res || {};

The alternative is to drop expect from the declaration (like poll in this same commit) and keep res.names here — but then this path also has to handle the {"names":{},"error":"no_resolver"} reply that resolve_addresses returns, which the expect form currently collapses to {}.


Generated by Claude Code

chmod 0600 "$WAN_LOG_LOCK_FILE" 2>/dev/null || true
chown 0:0 "$WAN_LOG_LOCK_FILE" 2>/dev/null || true
[ -L "$WAN_LOG_LOCK_FILE" ] && return 1
exec 9>"$WAN_LOG_LOCK_FILE" 2>/dev/null || return 1

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.

This line does not do what the function's contract comment ("fails closed (return 1) only if the lock file cannot be opened") says, and it has a side effect on the success path.

Three things, all verified with dash:

$ dash -c 'acq(){ exec 9>"/nonexistent-dir/x.lock" 2>/dev/null || return 1; }; \
    if acq; then echo acquired; else echo "acq failed"; fi; echo "still alive"'
dash: 1: cannot create /nonexistent-dir/x.lock: Directory nonexistent
$ echo $?
2
  1. || return 1 never runs. A redirection error on exec with no command aborts a POSIX non-interactive shell (dash/BusyBox ash). Under rpcd that kills the plugin mid-call, so enable_wan_logging / disable_wan_logging return no body at all instead of {"ok":false,...,"error":"lock_failed"}; in prerm it aborts before exit 0.
  2. 2>/dev/null does not suppress that message — redirections are applied left to right, so 9> is attempted before 2 is reassigned (note the diagnostic above is still printed).
  3. On the success path 2>/dev/null is permanent. exec with no command makes both redirections stick, so every later uci set / uci commit / flock diagnostic in this process is discarded and never reaches rpcd's stderr handler. release_wan_log_lock does not restore it.

Probing in a subshell keeps the guard working and confines the 2>/dev/null:

Suggested change
exec 9>"$WAN_LOG_LOCK_FILE" 2>/dev/null || return 1
# Probe in a subshell first: a failed `exec` redirection aborts a POSIX
# non-interactive shell outright, so `|| return 1` on the real exec would
# never run — and `2>/dev/null` on the same exec would permanently
# silence this process's stderr on the success path.
( exec 9>>"$WAN_LOG_LOCK_FILE" ) 2>/dev/null || return 1
exec 9>>"$WAN_LOG_LOCK_FILE"

(>> rather than > also avoids truncating the lock file — pointless flash traffic for /etc/fwlive/logging.lock.)


Generated by Claude Code

async loadRulesMap() {
try {
const res = await callFwliveRules();
this.rulesMap = (res && res.rules) || {};

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.

nit: build_rules_map now emits an error field (mktemp_failed from _fwlive_mktemp, or rules_truncated once RULES_MAP_MAX_KEYS/RULES_MAP_MAX_BYTES trip — see rpcd/fwlive:324-330), but nothing here reads it: a truncated or empty map renders identically to a router with no labelled rules.

That is the inverse of what this same commit did for poll, where expect was dropped specifically so reply.error could reach the banner (#233). Truncation is reachable in practice — map_uci_rule_names adds up to two keys per UCI rule before any nft prefix is seen, so ~256 firewall rules exhaust the 512-key budget and every nft log prefix label is dropped silently. Worth surfacing the same way the poll error is, or at least logging it to the console.


Generated by Claude Code

Comment thread applications/luci-app-fwlive/Makefile Outdated
PKG_VERSION:=0.1.37
PKG_RELEASE:=1


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.

nit: leftover double blank line from removing the SOURCE_DATE_EPOCH block. No other applications/*/Makefile in the tree has a blank-line gap here.

Suggested change

Generated by Claude Code

lucas-albers-lz4 added a commit to lucas-albers-lz4/fwlive that referenced this pull request Aug 31, 2026
Stop double-unwrapping fwlive.resolve after rpc expect (hostnames display).
Probe WAN lock open in a subshell so failed exec cannot abort dash/ash or
silence stderr. Surface rules map reply.error in the status banner. Squeeze
the cut Makefile blank left by dropping SOURCE_DATE_EPOCH; extend lock tests
and refresh the pot.
Re-cut from lucas-albers-lz4/fwlive master after openwrt#238 and openwrt#247: fix resolve
double-unwrap so hostnames display, probe WAN lock open without aborting
POSIX sh or silencing stderr, surface rules map reply.error in the UI, and
squeeze the Makefile blank left by dropping SOURCE_DATE_EPOCH. Refresh the
.pot via i18n-scan.pl.

Signed-off-by: Lucas Albers <lucas.b.albers@gmail.com>
@lucas-albers-lz4

Copy link
Copy Markdown
Author

Re-cut from fwlive master after #238 + round-2 fixes (#243#246): resolve double-unwrap (hostnames), WAN lock probe (no POSIX abort / stderr silence), rules map error in the UI, cut Makefile blank squeeze. Refreshed .pot via i18n-scan.pl.

Fork tip: lucas-albers-lz4/luci luci-app-fwlive-add @ 593ce11d20.

@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 new commit since 2defca54.

The three findings from the previous round are correctly fixed. resolve no longer double-unwraps (expect: { names: {} } means res is the map, so const names = res || {} is right), the WAN lock now probes in a subshell so a failed exec redirection can't abort the shell or permanently silence its stderr, and the .pot really was refreshed — the three new Rule labels … strings are the only msgid additions in the template diff, matching i18n-scan.pl output.

The new rules-error UI has a side effect worth a second look: because loadRulesMap() runs once and lastRulesError is never cleared, a rules_truncated map (reachable at ~256 UCI rules) permanently replaces the live row counter for the rest of the session, unlike the self-clearing poll error it is modelled on. Inline.

CI is green on 593ce11d (build x86_64, eslint, CodeQL, FormalityCheck all pass).

Commit checks

  • 593ce11d "luci-app-fwlive: refresh snapshot for openwrt-ai round 2" — the body lists four changes plus the .pot refresh, and all four are present, but the diff also contains a fifth that the message does not mention: run_with_timeout in root/usr/libexec/rpcd/fwlive changes its no-timeout fallback from running the command unbounded to return 127. That is a behaviour change affecting backend detection, the rules map and hostname resolution — not a refactor — and it deserves a line in the body (the in-code comment cites #229, but the commit message does not).

Generated by Claude Code

Comment thread applications/luci-app-fwlive/htdocs/luci-static/resources/view/status/fwlive.js Outdated
Comment thread applications/luci-app-fwlive/htdocs/luci-static/resources/view/status/fwlive.js Outdated
Comment thread applications/luci-app-fwlive/root/usr/libexec/rpcd/fwlive
lucas-albers-lz4 added a commit to lucas-albers-lz4/fwlive that referenced this pull request Sep 1, 2026
…ing blocker (#253)

Three findings from the openwrt/luci#8992 round-3 review (fwlive #250, #251,
#252), plus the commit-hygiene notes.

Sticky lastRulesError no longer consumes the live counter (#250)
----------------------------------------------------------------
updateStatus() early-returned on lastRulesError, which is set exactly once in
loadRulesMap() (run once from load()) and never cleared — so the first
rules_truncated / mktemp_failed occurrence replaced the live match counter and
the paused class for the whole session, even though log capture kept working.
Unlike lastPollError, which resets after every successful poll.

The rules-map degradation now surfaces in the #fwlive-backend span next to the
title (backend label + " · " + the existing translated error), which already
carried persistent backend state, and updateStatus() always reaches the counter
branch. lastPollError precedence is unchanged. All three messages reuse the
existing .pot msgids — no new translation strings.

Also removed the dead updateStatus() call at the end of loadRulesMap(): load()
runs before render(), so the status element does not exist yet and the call
returned at its first guard. The status readout is driven by the poll/click
paths, which call updateStatus() with the element present.

timeout_missing blocker surfaces the fail-closed path (#251)
------------------------------------------------------------
run_with_timeout returns 127 when the timeout binary is absent (fail-closed,
#229). On slimmed builds without timeout that degraded silently: backend
detection fell through to unknown, the rules map returned no error field, and
resolve stayed empty — indistinguishable from "no PTR record".

collect_logging_blockers now appends a timeout_missing blocker via the existing
blockers channel (surfaced through logging_status), alongside
nf_log_ipv4_missing. No behaviour change when timeout is present (stock images;
BUSYBOX_DEFAULT_TIMEOUT=y).

upstream-cut.sh: verify-only (#252)
-----------------------------------
The SOURCE_DATE_EPOCH strip + double-blank squeeze already exist in the cut
script (#224/#246). Master's Makefile keeps the block intentionally
(verify-reproducible-build.sh exports it); only the luci-shaped copy drops it.
Comments amplified to state that; no logic change. Dry-run verified: single
blank after PKG_RELEASE, block gone, gate checks pass.

security-review.md updated with the two new controls.

Refs #216
Re-cut from lucas-albers-lz4/fwlive master after the round-2/3 reviews.
Two files changed; the .pot is unchanged (149 msgids, no new strings).

fwlive-logging.sh — WAN zone id mismatch fix and timeout_missing blocker
------------------------------------------------------------------------
- wan_firewall_zone_same / wan_log_staged_line_section /
  wan_log_staged_zone_ids: uci `changes` can reference a different section
  id for the same WAN zone (firewall.@zone[N] vs the cfg id); the staged-
  line filter now treats all ids of the same WAN zone as ours (fwlive openwrt#239).
- collect_logging_blockers appends a `timeout_missing` blocker when the
  `timeout` binary is absent. rpcd's run_with_timeout fail-closes to 127
  without it (fwlive openwrt#229), which previously degraded silently: backend
  detection fell through to unknown, the rules map returned no error field,
  and resolve stayed empty. Stock images are unaffected (BusyBox timeout is
  enabled by default); the blocker makes slimmed builds diagnosable through
  the existing logging_status blockers channel.

view/status/fwlive.js — non-sticky rules-map error state
--------------------------------------------------------
lastRulesError was set exactly once (loadRulesMap runs from load()) and
never cleared, so the first rules_truncated / mktemp_failed occurrence
replaced the live match counter and the paused class for the whole session.
The degradation now surfaces in the #fwlive-backend span next to the title
(backend label + " · " + the existing translated error), and updateStatus()
always reaches the counter branch; lastPollError precedence is unchanged.
Also removed a dead updateStatus() call in loadRulesMap(): load() runs
before render(), so the status element does not exist yet.

The maintenance-model README link (which tree wins, snapshot cadence, why
not PKG_SOURCE) points at the fwlive docs anchor.

Signed-off-by: Lucas Albers <lucas.b.albers@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

add package Introduces a new package Makefile build script

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants