Skip to content

update miniupnpd to create chain without nft hooks#1806

Open
drahnieR wants to merge 1 commit intofirewalla:masterfrom
drahnieR:upnp
Open

update miniupnpd to create chain without nft hooks#1806
drahnieR wants to merge 1 commit intofirewalla:masterfrom
drahnieR:upnp

Conversation

@drahnieR
Copy link
Copy Markdown
Contributor

No description provided.

@drahnieR drahnieR requested a review from jasonlyc March 27, 2026 10:50
@MelvinTo
Copy link
Copy Markdown
Contributor

PR Review


Repo: firewalla/firerouter
PR: #1806
Head SHA: 965df349cee6abb2d1b7e3949599d00ba08cccd6
Checked at: 2026-03-27 19:01:51 CST

@j-sallyjin
Copy link
Copy Markdown

PR Review Summary

✅ What looks good

  • The change is narrowly scoped to a single artifact (platform/pse/bin/u22/miniupnpd.nft), which limits blast radius in terms of touched files.
  • Path naming suggests this is a platform-specific deliverable, which is reasonable for firmware/runtime packaging workflows.

⚠️ Issues found

  • High – opaque binary replacement with no auditable source diff.
    The PR only shows a binary delta, so correctness and security behavior changes are not reviewable from code.
  • High – supply-chain/provenance gap.
    No evidence in this diff of:
  • source commit/tag used to build,
  • build toolchain/version,
  • reproducible build steps,
  • artifact checksum/signature before/after.
  • Medium – missing validation evidence.
    No included test output demonstrating runtime compatibility/regression checks for UPnP/NAT behavior after replacing this binary.
  • Issue requirement coverage: No linked issue found.

💡 Suggestions

  • Attach release metadata for this binary update:
  • upstream source version/commit,
  • exact build command/toolchain/container,
  • SHA256 of produced artifact.
  • Include verification evidence:
  • smoke tests for miniupnpd startup + rule insertion/removal on target platform (u22),
  • regression checks for existing port-mapping flows.
  • Prefer a build-from-source path in CI (or at minimum a documented artifact pipeline) so future reviews can validate changes deterministically.

Verdict

REQUEST_CHANGES


Repo: firewalla/firerouter
PR: #1806
Head SHA: 965df349cee6abb2d1b7e3949599d00ba08cccd6
Checked at: 2026-03-28 00:11:39 CST

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