Skip to content

fix(v1.99): validator bin path — use constants.ValidatorBinPath single authority#478

Merged
itcmsgr merged 1 commit intomainfrom
fix/v1.99-validator-bin-path
Apr 19, 2026
Merged

fix(v1.99): validator bin path — use constants.ValidatorBinPath single authority#478
itcmsgr merged 1 commit intomainfrom
fix/v1.99-validator-bin-path

Conversation

@itcmsgr
Copy link
Copy Markdown
Owner

@itcmsgr itcmsgr commented Apr 19, 2026

Summary

PR-18/19/20 follow-up fix caught by the post-PR-20 G3-U-REBUILD-PARITY gate on lab2 (2026-04-19, evidence at /tmp/parity-lab2.tar.gz + mirror V1.90_AUDIT_WIKI_CODE/G3_U_REBUILD_PARITY_EVIDENCE.md §8).

The bug: runUpdateApply invoked exec.Run("nftban-validate", "--json") — bare name via $PATH. /usr/lib/nftban/bin is NOT in default $PATH on Ubuntu or AlmaLinux, so every operator-initiated apply failed at the validator phase and persisted StateFailedRebuild even when rebuild itself succeeded.

The gate's positive result: rebuild pipeline parity held — normalized kernel ruleset + protected filesystem sha256 set identical across baseline / Path A's rebuild phase / Path B. The divergence was purely in apply's validator invocation.

Fix shape (narrow, single-authority)

internal/constants/paths.go::ValidatorBinPath already defined the canonical path with 4 existing consumers (internal/metrics + internal/smoke). Two Go call sites had missed it:

  • cmd/nftban-installer/update_apply.goexec.Run("nftban-validate", …)
  • cmd/nftban-core/cmd_lifecycle.gorunCmdOutput("nftban-validate", …) + fileExists("/usr/lib/nftban/bin/nftban-validate")

All three now use constants.ValidatorBinPath. No new authority added.

Explicit non-addition

An earlier draft added fhs.NftbanValidateBin which would have created a second authority alongside constants.ValidatorBinPath — exactly the duplicate-authority pattern the review forbids. A marker comment now sits in internal/installer/fhs/paths.go pointing at the canonical location.

Scope boundaries

  • ✅ Go-side only
  • ✅ No state/exit semantics change
  • ✅ No mutation/recovery/authority path change
  • ❌ Shell-side hardcoded sites (5 exist) — out of scope; cosmetic hygiene for a separate PR if desired
  • ❌ No new abstractions, no new constants

Aligned surfaces

File Change
cmd/nftban-installer/update_apply.go use constants.ValidatorBinPath
cmd/nftban-core/cmd_lifecycle.go use constants.ValidatorBinPath at 2 sites
internal/installer/fhs/paths.go marker comment (intentionally NOT adding a duplicate constant)
internal/installer/update/apply_contract.go ApplyWhitelist entry updated to absolute path
internal/installer/update/apply_contract.md call graph updated
internal/installer/update/apply_contract_test.go harness happy-path string
cmd/nftban-installer/update_apply_test.go 6 mock keys
cmd/nftban-installer/update_apply_logging_test.go 1 mock key
.github/workflows/ci-update-canonization.yml stage validator at /usr/lib/nftban/bin/ in host prep

Required post-merge action (gates PR-21)

Re-run G3-U-REBUILD-PARITY on both lab2 AND lab4 per V1.90_AUDIT_WIKI_CODE/G3_U_REBUILD_PARITY_EVIDENCE.md. Template's §2 matrix requires both representative hosts before PR-21 can open.

PR-21 stays blocked until the parity gate signs off PASS on both hosts with captured artifacts.

🤖 Generated with Claude Code

…nPath

Fix discovered by post-PR-20 G3-U-REBUILD-PARITY gate FC-1 on 2026-04-19
(lab2 Ubuntu 24.04 DEB, evidence at /tmp/parity-lab2.tar.gz):

  runUpdateApply invoked bare "nftban-validate" which resolves via $PATH.
  /usr/lib/nftban/bin is NOT in default $PATH on Ubuntu or AlmaLinux, so
  every operator-initiated apply failed at the validator phase with:
      exec: "nftban-validate": executable file not found in $PATH
  and persisted StateFailedRebuild even though rebuild itself succeeded.

The parity gate caught this BEFORE PR-21 deleted the shell update path
(which still works because shell uses NFTBAN_LIB_DIR with canonical
fallback, not bare PATH lookup).

Fix — single authority:

  internal/constants/paths.go::ValidatorBinPath already defined this path
  and already had 4 consumers (metrics + smoke). Two Go call sites had
  missed it and were resolving the binary independently:

    cmd/nftban-installer/update_apply.go  — exec.Run("nftban-validate", …)
    cmd/nftban-core/cmd_lifecycle.go      — runCmdOutput("nftban-validate", …)
                                          — fileExists("/usr/lib/nftban/bin/nftban-validate")

  All three now use constants.ValidatorBinPath. No new authority added.

Explicit non-addition:

  An earlier draft of this commit added fhs.NftbanValidateBin which would
  have created a second path-authority alongside constants.ValidatorBinPath
  — exactly the "duplicate authority plumbing" the reviewer's hard rule
  forbids. A marker comment in internal/installer/fhs/paths.go records
  why the FHS constant is intentionally absent and points at the
  canonical location.

Shell call sites unchanged — 10 shell callers already use
${NFTBAN_LIB_DIR:-/usr/lib/nftban}/bin/nftban-validate (single shell-side
authority). 5 hardcoded shell sites exist but are out of scope for this
narrow PR (cosmetic hygiene, separate PR if desired).

Tests + whitelist + CI aligned to the absolute path (values unchanged —
they're literal string keys):
  cmd/nftban-installer/update_apply_test.go — mock keys
  cmd/nftban-installer/update_apply_logging_test.go — mock key
  internal/installer/update/apply_contract.go — ApplyWhitelist entry
  internal/installer/update/apply_contract_test.go — harness happy path
  internal/installer/update/apply_contract.md — call graph
  .github/workflows/ci-update-canonization.yml — stage validator binary
    at /usr/lib/nftban/bin/ in "Prepare host state for dry-run" so a
    future non-dry-run CI step would exercise the real invocation

Scope boundaries (explicit):
  - Go-side only (shell sites unchanged)
  - No behavior change other than removing the false PATH-lookup failure
  - No state/exit semantics change
  - No mutation/recovery/authority path change
  - PR-21 remains blocked until G3-U-REBUILD-PARITY re-runs GREEN on
    both lab2 AND lab4

Parity re-run required on BOTH hosts before PR-21 can open per the
filled evidence template in V1.90_AUDIT_WIKI_CODE/
G3_U_REBUILD_PARITY_EVIDENCE.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@itcmsgr itcmsgr marked this pull request as ready for review April 19, 2026 18:46
@itcmsgr itcmsgr merged commit cdbcb78 into main Apr 19, 2026
54 checks passed
@itcmsgr itcmsgr deleted the fix/v1.99-validator-bin-path branch April 19, 2026 18:46
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.

1 participant