Skip to content

Commit ce570d1

Browse files
itcmsgrclaude
andcommitted
installer: tests read shipped conf.d directly — no Go port hardcoding
Operator feedback: hardcoding port lists in Go test files reproduces the four-truth drift PR26.4 was created to close. The conf.d files have 16 declarable port lists per panel (TCP/UDP × IN/OUT × IPv4/IPv6 + CUSTOM × 8) — any of them in Go is a maintenance burden and a drift risk. Operators should edit conf.d, not Go. Changes (test-file only — no production code change): - Removed canonicalDA Go fixture (mirrored shipped conf.d port list). - Removed expandRange() helper (used only by canonicalDA). - Replaced inline []int{20,21,25,...} discrete-port list with a shipped-conf.d read via locateRepoFile + os.ReadFile. - New `synthDA` synthetic stub fixture: tiny, arbitrary port set used ONLY by stub-loader tests that exercise the adapter contract (pass-through, defensive copy, fail-closed branches). Clearly marked "synthetic — NOT authoritative for DA". - TestRequiredPorts_ConfDDoesNotIncludeSSHPort22 now reads the shipped main.conf and verifies the actual file (not a Go mirror). - TestRequiredPorts_RealLoader_RangeExpansion_LengthAndEndpoints now reads the shipped main.conf; structural assertions only (length lower bound, range endpoints + mid-range, control-port presence, port-22 absence). NO discrete port enumeration in Go. - locateRepoFile helper (climbs to go.mod via runtime.Caller). - Big "FUTURE-AUDITOR DIRECTIVE" comment block at the top of the test file: 1. Stub-loader tests use synthDA (synthetic). 2. Real port content is verified against the shipped conf.d. 3. Do NOT add hardcoded port lists to this Go test file. Compatibility: - No production code touched in this commit. - Existing CLI surface (nftban panel directadmin {enable,disable, status,test,repair,report}, plus nftban_panel_detect() shell function) preserved 1:1. PR26.4's adapter is a read-only install- validation path; it does NOT replace the CLI verbs (those migrate to Go in PR26.9 and the shell libraries decommission in PR26.10 per the audit's locked sequence). Lab4 proof: go vet panelfw/... clean go test panelfw/adapters/directadmin: 30 sub-tests PASS go test panelfw/ports/validate: 100+ sub-tests PASS go test ./... 66 packages, 0 fail Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 84e9c28 commit ce570d1

1 file changed

Lines changed: 157 additions & 100 deletions

File tree

internal/installer/panelfw/adapters/directadmin/directadmin_test.go

Lines changed: 157 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"fmt"
2424
"os"
2525
"path/filepath"
26+
"runtime"
2627
"strings"
2728
"testing"
2829

@@ -187,29 +188,50 @@ func withFixtureConfD(t *testing.T, mainConf string) string {
187188
return tmp
188189
}
189190

190-
// canonicalDA is the conf.d port surface that ships with NFTBan
191-
// (etc/nftban/conf.d/panels/directadmin/main.conf as of PR26.4). The
192-
// adapter's RequiredPorts must return this set verbatim, no addition,
193-
// no truncation.
194-
var canonicalDA = struct {
191+
// =============================================================================
192+
// FUTURE-AUDITOR DIRECTIVE — port content lives in CONF.D, not in Go.
193+
// =============================================================================
194+
// The DirectAdmin port surface (TCP/UDP × IN/OUT × IPv4/IPv6 + CUSTOM
195+
// — 16 declarable lists per panel) lives ONLY in the shipped conf.d
196+
// file:
197+
//
198+
// etc/nftban/conf.d/panels/directadmin/main.conf
199+
//
200+
// That file is the single source of truth. Operators edit conf.d, not
201+
// Go. Reproducing any port list in Go (this test file or production
202+
// code) recreates the four-truth drift PR26.4 was created to close.
203+
//
204+
// RULES FOR FUTURE EDITS TO THIS TEST FILE:
205+
// 1. Stub-loader tests use the small `synthDA` synthetic fixture
206+
// below — clearly marked synthetic, not authoritative. Its only
207+
// job is to give the adapter SOMETHING to pass through so we can
208+
// test the contract (errors, defensive copies, fail-closed
209+
// branches). Its specific port values are arbitrary.
210+
// 2. Tests that verify ACTUAL DirectAdmin port content read the
211+
// shipped conf.d via the real loader (see real-loader tests).
212+
// Use `locateRepoFile(t, "etc/nftban/conf.d/panels/directadmin/main.conf")`.
213+
// 3. Do NOT add hardcoded port lists to Go. If you find yourself
214+
// typing a list of DirectAdmin ports in this file, stop and put
215+
// them in conf.d instead.
216+
// =============================================================================
217+
218+
// synthDA is a tiny synthetic PanelConfig used only by stub-loader
219+
// tests that test the adapter contract (pass-through, defensive copy,
220+
// non-trivial surface size). Its port values are arbitrary fixtures —
221+
// NOT the canonical DirectAdmin port surface. The canonical surface
222+
// lives in etc/nftban/conf.d/panels/directadmin/main.conf and is
223+
// verified by real-loader tests further down.
224+
var synthDA = struct {
195225
tcpIn []int
196226
udpIn []int
197227
}{
198-
// TCP_IN: 20,21,25,53,853,80,110,143,443,465,587,993,995,2222,35000-35999
199-
tcpIn: append([]int{20, 21, 25, 53, 853, 80, 110, 143, 443, 465, 587, 993, 995, 2222}, expandRange(35000, 35999)...),
200-
// UDP_IN: 20,21,53,853,80,443
201-
udpIn: []int{20, 21, 53, 853, 80, 443},
228+
tcpIn: []int{2222, 25, 80, 443, 35000, 35001},
229+
udpIn: []int{53, 443},
202230
}
203231

204-
func expandRange(lo, hi int) []int {
205-
out := make([]int, 0, hi-lo+1)
206-
for p := lo; p <= hi; p++ {
207-
out = append(out, p)
208-
}
209-
return out
210-
}
211-
212-
// PR26.4 R1: RequiredPorts equals DirectAdmin conf.d TCP/UDP declarations.
232+
// PR26.4 R1: adapter passes through the loader's PanelConfig
233+
// verbatim. Stub fixture (synthetic ports) — content correctness for
234+
// real DirectAdmin lives in the real-loader test below.
213235
func TestRequiredPorts_ConfDLoaded_FullSurface(t *testing.T) {
214236
withStubLoader(t, func(configDir, panelName string) (*ports.PanelConfig, error) {
215237
if panelName != "directadmin" {
@@ -219,32 +241,32 @@ func TestRequiredPorts_ConfDLoaded_FullSurface(t *testing.T) {
219241
Name: "directadmin",
220242
Enabled: true,
221243
ConfigFile: configDir + "/conf.d/panels/directadmin/main.conf",
222-
TCPIn: canonicalDA.tcpIn,
223-
UDPIn: canonicalDA.udpIn,
244+
TCPIn: synthDA.tcpIn,
245+
UDPIn: synthDA.udpIn,
224246
}, nil
225247
})
226248

227249
a := New()
228250
tcp, udp, err := a.RequiredPorts(context.Background(), executor.NewMockExecutor())
229251
if err != nil {
230-
t.Fatalf("RequiredPorts must not error on canonical DA conf.d: %v", err)
252+
t.Fatalf("RequiredPorts must not error on stub fixture: %v", err)
231253
}
232-
if !equalIntSlices(tcp, canonicalDA.tcpIn) {
233-
t.Errorf("TCP surface mismatch:\n got %v\n want %v", tcp, canonicalDA.tcpIn)
254+
if !equalIntSlices(tcp, synthDA.tcpIn) {
255+
t.Errorf("TCP pass-through mismatch:\n got %v\n want %v", tcp, synthDA.tcpIn)
234256
}
235-
if !equalIntSlices(udp, canonicalDA.udpIn) {
236-
t.Errorf("UDP surface mismatch:\n got %v\n want %v", udp, canonicalDA.udpIn)
257+
if !equalIntSlices(udp, synthDA.udpIn) {
258+
t.Errorf("UDP pass-through mismatch:\n got %v\n want %v", udp, synthDA.udpIn)
237259
}
238260
}
239261

240-
// PR26.4 R2: RequiredPorts is NOT [2222]-only. This is a structural
241-
// regression check that ensures the legacy hardcoded path is gone.
262+
// PR26.4 R2: RequiredPorts is NOT [2222]-only. Structural regression
263+
// guard — the legacy hardcoded path is gone.
242264
func TestRequiredPorts_ConfDLoaded_NotJust2222(t *testing.T) {
243265
withStubLoader(t, func(configDir, panelName string) (*ports.PanelConfig, error) {
244266
return &ports.PanelConfig{
245267
Name: "directadmin",
246-
TCPIn: canonicalDA.tcpIn,
247-
UDPIn: canonicalDA.udpIn,
268+
TCPIn: synthDA.tcpIn,
269+
UDPIn: synthDA.udpIn,
248270
}, nil
249271
})
250272

@@ -336,100 +358,131 @@ func TestRequiredPorts_NilPanelConfig_FailsClosed(t *testing.T) {
336358
}
337359

338360
// PR26.4 condition A: explicit regression guard — port 22 (SSH) must
339-
// NOT appear in DirectAdmin RequiredPorts output. The canonical
340-
// conf.d intentionally excludes 22 (managed separately by
341-
// /etc/nftban/ports.d/00-ssh.conf); the legacy shell library
342-
// historically included 22 (four-truth drift). Conf.d wins.
361+
// NOT appear in DirectAdmin RequiredPorts output. SSH is managed
362+
// separately by /etc/nftban/ports.d/00-ssh.conf; the legacy shell
363+
// library historically included 22 (four-truth drift). Conf.d wins.
343364
//
344-
// This test is independent of the full-surface identity test so a
345-
// future conf.d edit that re-introduces 22 trips a clearly-named
346-
// failure even if the surface-identity test has been amended.
365+
// Reads the SHIPPED conf.d file directly so the assertion verifies
366+
// the actual source of truth, not a Go-level mirror. A future
367+
// conf.d edit that re-introduces 22 trips this test by name.
347368
func TestRequiredPorts_ConfDDoesNotIncludeSSHPort22(t *testing.T) {
348-
withStubLoader(t, func(configDir, panelName string) (*ports.PanelConfig, error) {
349-
return &ports.PanelConfig{
350-
Name: "directadmin",
351-
Enabled: true,
352-
TCPIn: canonicalDA.tcpIn,
353-
UDPIn: canonicalDA.udpIn,
354-
}, nil
355-
})
369+
if _, err := os.Stat("/bin/bash"); err != nil {
370+
t.Skipf("/bin/bash unavailable on this host: %v", err)
371+
}
372+
shipped := locateRepoFile(t, "etc/nftban/conf.d/panels/directadmin/main.conf")
373+
data, err := os.ReadFile(shipped) // #nosec G304 -- fixed path under repo
374+
if err != nil {
375+
t.Fatalf("read shipped main.conf at %s: %v", shipped, err)
376+
}
377+
withFixtureConfD(t, string(data))
378+
356379
tcp, udp, err := New().RequiredPorts(context.Background(), executor.NewMockExecutor())
357380
if err != nil {
358-
t.Fatalf("unexpected error from canonical DA stub: %v", err)
381+
t.Fatalf("unexpected error loading shipped conf.d: %v", err)
359382
}
360383
if containsInt(tcp, 22) {
361-
t.Errorf("DirectAdmin RequiredPorts TCP_IN must NOT include port 22 — "+
362-
"SSH is managed by /etc/nftban/ports.d/00-ssh.conf, conf.d wins over shell library; got %v", tcp)
384+
t.Errorf("shipped DirectAdmin conf.d declares port 22 in TCP_IN — "+
385+
"SSH is managed by /etc/nftban/ports.d/00-ssh.conf; check %s", shipped)
363386
}
364387
if containsInt(udp, 22) {
365-
t.Errorf("DirectAdmin RequiredPorts UDP_IN must NOT include port 22; got %v", udp)
388+
t.Errorf("shipped DirectAdmin conf.d declares port 22 in UDP_IN; check %s", shipped)
366389
}
367390
}
368391

369392
// PR26.4 condition C: range-form (35000-35999) regression guard.
370393
// internal/ports/panel_loader.parsePortList expands ranges into
371394
// individual ints (35000..35999 = 1000 values). Verify the loader
372-
// integration produces the expected expanded length and both endpoints
373-
// so a future loader change that drops range expansion or shifts the
374-
// boundary surfaces here.
395+
// integration produces the expected expanded length and both endpoints.
396+
//
397+
// FUTURE-AUDITOR DIRECTIVE — DO NOT INVENT PORT LISTS HERE.
398+
// Source of truth for DirectAdmin ports is:
375399
//
376-
// Canonical conf.d declares:
400+
// etc/nftban/conf.d/panels/directadmin/main.conf
377401
//
378-
// TCP_IN: 14 discrete + 35000-35999 range = 14 + 1000 = 1014 ports
402+
// This test reads that shipped file directly and runs the real loader
403+
// against it. Structural assertions only (length range, range
404+
// endpoints, port-22 exclusion, control-port presence). NEVER add a
405+
// hardcoded port list to this test — if you need to
406+
// change DirectAdmin's port surface, edit the conf.d file and the
407+
// test will follow automatically.
379408
func TestRequiredPorts_RealLoader_RangeExpansion_LengthAndEndpoints(t *testing.T) {
380409
if _, err := os.Stat("/bin/bash"); err != nil {
381410
t.Skipf("/bin/bash unavailable on this host: %v", err)
382411
}
383-
const fixtureMain = `
384-
NFTBAN_DIRECTADMIN_PATH="/usr/local/directadmin"
385-
NFTBAN_DIRECTADMIN_PANEL_PORT="2222"
386-
NFTBAN_DIRECTADMIN_TCP_IN="20,21,25,53,853,80,110,143,443,465,587,993,995,2222,35000-35999"
387-
NFTBAN_DIRECTADMIN_UDP_IN="20,21,53,853,80,443"
388-
`
389-
withFixtureConfD(t, fixtureMain)
412+
413+
// Read the SHIPPED conf.d file (single source of truth). The
414+
// adapter, the loader, and this test all consume the same bytes.
415+
shipped := locateRepoFile(t, "etc/nftban/conf.d/panels/directadmin/main.conf")
416+
data, err := os.ReadFile(shipped) // #nosec G304 -- fixed path under repo
417+
if err != nil {
418+
t.Fatalf("read shipped main.conf at %s: %v", shipped, err)
419+
}
420+
withFixtureConfD(t, string(data))
390421

391422
tcp, udp, err := New().RequiredPorts(context.Background(), executor.NewMockExecutor())
392423
if err != nil {
393424
t.Fatalf("real-loader RequiredPorts: %v", err)
394425
}
395426

396-
// Exact length: 14 discrete + 1000 expanded range = 1014.
397-
const expectedTCP = 14 + 1000
398-
if len(tcp) != expectedTCP {
399-
t.Errorf("TCP_IN length = %d; want %d (14 discrete + 1000-port range expansion)",
400-
len(tcp), expectedTCP)
401-
}
402-
// Both range endpoints must be present.
403-
if !containsInt(tcp, 35000) {
404-
t.Errorf("TCP_IN must include range start 35000; got %v ports total", len(tcp))
427+
// Structural assertions — config-driven.
428+
//
429+
// Length: at minimum the range alone (35000..35999 = 1000) plus
430+
// the discrete declarations (>0). Use a sane lower bound rather
431+
// than an exact count so a future operator-edit of conf.d that
432+
// adds/removes a discrete port doesn't churn this test.
433+
if len(tcp) < 1000+1 {
434+
t.Errorf("TCP_IN length = %d; expected >= 1001 (1000 from range + at least one discrete); "+
435+
"loader may be dropping range expansion", len(tcp))
405436
}
406-
if !containsInt(tcp, 35999) {
407-
t.Errorf("TCP_IN must include range end 35999; got %v ports total", len(tcp))
408-
}
409-
// Spot-check one mid-range port to confirm the loader didn't only
410-
// keep endpoints.
411-
if !containsInt(tcp, 35500) {
412-
t.Errorf("TCP_IN must include mid-range port 35500 (loader range expansion broken?)")
413-
}
414-
// Every discrete declared port must be present.
415-
for _, p := range []int{20, 21, 25, 53, 853, 80, 110, 143, 443, 465, 587, 993, 995, 2222} {
437+
// Range endpoints + mid-range — protects against "endpoints only"
438+
// or "skip every Nth" loader bugs.
439+
for _, p := range []int{35000, 35500, 35999} {
416440
if !containsInt(tcp, p) {
417-
t.Errorf("TCP_IN missing discrete declared port %d", p)
441+
t.Errorf("TCP_IN missing range port %d (panel_loader range expansion broken?)", p)
418442
}
419443
}
420-
// SSH port 22 still excluded even with the real loader.
444+
// DirectAdmin control plane MUST be in the surface — without 2222
445+
// the panel itself is unreachable. This is the one literal
446+
// expectation the test makes; it's the architectural invariant,
447+
// not a port enumeration.
448+
if !containsInt(tcp, 2222) {
449+
t.Errorf("TCP_IN must include the DirectAdmin control port 2222; got len=%d", len(tcp))
450+
}
451+
// SSH port 22 must be absent (conf.d four-truth rule: SSH is
452+
// managed separately by /etc/nftban/ports.d/00-ssh.conf).
421453
if containsInt(tcp, 22) {
422-
t.Errorf("real loader produced TCP_IN containing port 22 — conf.d four-truth violation; got %v", tcp)
454+
t.Errorf("real loader produced TCP_IN containing port 22 — conf.d four-truth violation; "+
455+
"check %s for stale port-22 entry", shipped)
456+
}
457+
// UDP must be non-empty (DirectAdmin needs DNS at minimum).
458+
if len(udp) == 0 {
459+
t.Errorf("UDP_IN empty — conf.d should declare DirectAdmin's UDP surface (DNS, etc.)")
423460
}
424-
// UDP_IN exact length and contents.
425-
wantUDP := []int{20, 21, 53, 853, 80, 443}
426-
if len(udp) != len(wantUDP) {
427-
t.Errorf("UDP_IN length = %d; want %d", len(udp), len(wantUDP))
461+
// SSH port 22 also forbidden in UDP_IN.
462+
if containsInt(udp, 22) {
463+
t.Errorf("real loader produced UDP_IN containing port 22; check %s", shipped)
428464
}
429-
for _, p := range wantUDP {
430-
if !containsInt(udp, p) {
431-
t.Errorf("UDP_IN missing %d", p)
465+
}
466+
467+
// locateRepoFile climbs from the test file's directory until it finds
468+
// the repo's go.mod, then resolves relPath against that root. Used by
469+
// tests that read shipped config files (the source of truth for ports).
470+
func locateRepoFile(t *testing.T, relPath string) string {
471+
t.Helper()
472+
_, this, _, ok := runtime.Caller(0)
473+
if !ok {
474+
t.Fatalf("runtime.Caller failed")
475+
}
476+
dir := filepath.Dir(this)
477+
for {
478+
if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil {
479+
return filepath.Join(dir, relPath)
480+
}
481+
parent := filepath.Dir(dir)
482+
if parent == dir {
483+
t.Fatalf("could not locate go.mod above %s", filepath.Dir(this))
432484
}
485+
dir = parent
433486
}
434487
}
435488

@@ -691,16 +744,19 @@ func TestID(t *testing.T) {
691744
// Framework integration: registered adapter detected → policy fires
692745
// ----------------------------------------------------------------------------
693746

694-
// stubCanonicalDA installs a stub loader returning the canonical DA
695-
// conf.d port surface, so framework-integration tests are deterministic
696-
// regardless of whether /etc/nftban/conf.d/... exists on the build host.
747+
// stubCanonicalDA installs a stub loader returning the small
748+
// `synthDA` synthetic fixture so framework-integration tests are
749+
// deterministic regardless of whether the shipped conf.d exists on
750+
// the build host. Synthetic — NOT the canonical DirectAdmin port
751+
// surface; the canonical surface is verified separately by
752+
// real-loader tests reading the shipped main.conf.
697753
func stubCanonicalDA(t *testing.T) {
698754
withStubLoader(t, func(configDir, panelName string) (*ports.PanelConfig, error) {
699755
return &ports.PanelConfig{
700756
Name: "directadmin",
701757
Enabled: true,
702-
TCPIn: canonicalDA.tcpIn,
703-
UDPIn: canonicalDA.udpIn,
758+
TCPIn: synthDA.tcpIn,
759+
UDPIn: synthDA.udpIn,
704760
}, nil
705761
})
706762
}
@@ -724,13 +780,14 @@ func TestFrameworkIntegration_DA_Detected_Reachable_Passes(t *testing.T) {
724780
if !res.PortsApplied || !res.ReachableAfter {
725781
t.Errorf("expected PortsApplied+ReachableAfter true; got %#v", res)
726782
}
727-
// PR26.4: framework PanelResult must carry the full conf.d
728-
// port surface, not just the control plane.
729-
if !equalIntSlices(res.PortsTCP, canonicalDA.tcpIn) {
730-
t.Errorf("PortsTCP must equal canonical DA TCP_IN; got %v", res.PortsTCP)
783+
// PR26.4: framework PanelResult must carry the loaded surface
784+
// pass-through (synthetic here; full conf.d-content correctness
785+
// is verified by real-loader tests).
786+
if !equalIntSlices(res.PortsTCP, synthDA.tcpIn) {
787+
t.Errorf("PortsTCP pass-through mismatch; got %v want %v", res.PortsTCP, synthDA.tcpIn)
731788
}
732-
if !equalIntSlices(res.PortsUDP, canonicalDA.udpIn) {
733-
t.Errorf("PortsUDP must equal canonical DA UDP_IN; got %v", res.PortsUDP)
789+
if !equalIntSlices(res.PortsUDP, synthDA.udpIn) {
790+
t.Errorf("PortsUDP pass-through mismatch; got %v want %v", res.PortsUDP, synthDA.udpIn)
734791
}
735792
}
736793

0 commit comments

Comments
 (0)