PR26.4: DirectAdmin adapter consumes canonical conf.d#531
Merged
Conversation
PR26.4 — bridges the DirectAdmin panelfw adapter to the canonical
panel config at etc/nftban/conf.d/panels/directadmin/main.conf via
internal/ports/panel_loader.LoadPanelConfig("directadmin"). The
adapter no longer hardcodes a [2222]-only port list — RequiredPorts
returns the full conf.d-declared TCP_IN / UDP_IN port surface.
Per the panel architecture audit (V190_PANELS/PANEL_ARCHITECTURE_AUDIT.md):
- conf.d is the source-of-truth for panel ports.
- Conf.d wins over the legacy shell library (SSH port 22 is managed
separately by /etc/nftban/ports.d/00-ssh.conf and is intentionally
absent from panel TCP_IN).
- Adapter must not invent or duplicate a DirectAdmin port list.
Adapter (internal/installer/panelfw/adapters/directadmin/directadmin.go):
- imports internal/ports (new)
- panelConfDLoader / panelConfDDir package-level vars (production
values: ports.LoadPanelConfig + fhs.EtcDir; tests inject stubs)
- RequiredPorts: loads via panelConfDLoader; returns
(TCPIn, UDPIn, error). Defensive copies — caller cannot mutate
loader cache.
- Fail-closed: missing main.conf, nil PanelConfig, or empty TCP_IN
all return errors that PR26.2's panelfw.finalizeDetected
propagates as PanelResult.Fatal=true (PANEL-SURVIVAL-001 fires).
- ValidateReachability unchanged — still control-plane only
(default TCP 2222 with directadmin.conf port=N override).
- Detect unchanged.
- Doc-comment expanded to record PR26.4 scope and the resolved
four-truth source-of-truth choice (conf.d wins).
Tests (internal/installer/panelfw/adapters/directadmin/directadmin_test.go):
- withStubLoader / withFixtureConfD test helpers
- canonicalDA fixture matching shipped main.conf TCP_IN / UDP_IN
- RequiredPorts_ConfDLoaded_FullSurface — surface match
- RequiredPorts_ConfDLoaded_NotJust2222 — regression check
- RequiredPorts_MissingConfD_FailsClosed
- RequiredPorts_EmptyTCPIn_FailsClosed
- RequiredPorts_NilPanelConfig_FailsClosed
- RequiredPorts_DefensiveCopy
- RequiredPorts_ConfDLoaded_RealLoader_FixtureFile (integration via
real bash-subshell loader against tempdir-stamped main.conf)
- RequiredPorts_RealLoader_MissingConfD_FailsClosed
- ValidateReachability_ConfigOverride_HonoredByControlPlane (moved
here from RequiredPorts; control-plane is the right home for the
directadmin.conf port override)
- ValidateReachability_MalformedOverride_FallsBackToDefault
- Framework-integration tests stubbed with canonical DA loader so
tests are deterministic on the build host.
Hard exclusions preserved:
- No cPanel / Plesk / CyberPanel / CWP / InterWorx / Vesta /
generic adapter
- No shell decommission
- No Go-native env-var parser rewrite (panel_loader's bash
subshell stays — PR26.7's lane)
- No YAML/TOML migration
- No authority residue classifier change
- No restore semantics change
- No firewall mutation
- No new mutation surface (only added I/O is panel_loader's
existing read-only bash subshell sourcing the conf.d file)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
A. Explicit port-22 negative regression guard
- TestRequiredPorts_ConfDDoesNotIncludeSSHPort22: dedicated test
proving DirectAdmin RequiredPorts (TCP_IN + UDP_IN) does NOT
include port 22. Independent of full-surface identity test so a
future conf.d edit re-introducing 22 trips a clearly-named
failure. Comment cites the four-truth rule (conf.d wins, SSH
managed by /etc/nftban/ports.d/00-ssh.conf, shell-library port
22 inclusion is stale).
B. Fail-closed branches — no fallback to [2222] under any error
- assertNoControlPlaneFallback test helper added.
- TestRequiredPorts_MissingConfD_FailsClosed: now also asserts
returned tcp/udp slices are nil/empty (no [2222] fallback).
- TestRequiredPorts_EmptyTCPIn_FailsClosed: same.
- TestRequiredPorts_NilPanelConfig_FailsClosed: same.
- TestRequiredPorts_RealLoader_MissingConfD_FailsClosed (was
already present; the structural shape of fail-closed is now
covered uniformly).
C. Range-form (35000-35999) regression guard
- TestRequiredPorts_RealLoader_RangeExpansion_LengthAndEndpoints:
fixture conf.d with the canonical TCP_IN; asserts EXACT length
14 + 1000 = 1014, both endpoints (35000 and 35999) present, a
mid-range port (35500) present, every discrete declared port
present, and SSH 22 still excluded. Catches a future loader
change that drops range expansion or shifts the boundary.
D. Removed stale "PR26.4 follow-up" doc-comment from
ValidateReachability — replaced with PR26.4-current text noting
that RequiredPorts now loads the full conf.d surface but
ValidateReachability still probes only the control plane. The
ValidateReachability error message no longer says "validated in
PR26.4"; new wording: "loaded from conf.d via RequiredPorts but
not probed here".
E. Renamed misleading test
TestFrameworkIntegration_DA_Reason_DoesNotImplyFullPortSurvival
→
TestFrameworkIntegration_DA_ControlPlaneError_DoesNotClaimFullSurfaceReachability
The semantic is unchanged (control-plane error must not claim
full-surface probing), but the name now reflects post-PR26.4
reality where RequiredPorts does load the full surface declaratively.
No production code paths changed beyond the doc-comment + error
wording. All test additions/changes are test-file only.
Lab4 proof (post A–E, base 5366caf):
go vet ... clean
go test -v panelfw/ports/validate 100 sub-tests PASS, 0 FAIL
go test ./... 66 packages PASS, 0 FAIL
Hard exclusions preserved: no cPanel/Plesk/other adapters; no shell
decommission; no parser rewrite; no restore/firewall/authority
changes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
A–E patch applied —
|
| Branch | Test |
|---|---|
loader returns (nil, err) |
TestRequiredPorts_MissingConfD_FailsClosed |
loader returns (nil, nil) |
TestRequiredPorts_NilPanelConfig_FailsClosed |
cfg.TCPIn empty/nil |
TestRequiredPorts_EmptyTCPIn_FailsClosed |
conf.d directory missing (real loader) |
TestRequiredPorts_RealLoader_MissingConfD_FailsClosed |
All four assert len(tcp) == 0 && len(udp) == 0 so the historical [2222] fallback can never re-emerge.
C. Range-form (35000-35999) regression guard
Added TestRequiredPorts_RealLoader_RangeExpansion_LengthAndEndpoints:
- Fixture conf.d declares the canonical
TCP_IN(14 discrete + range) - Asserts exact length 1014 (14 + 1000-port range expansion)
- Asserts both endpoints present (
35000,35999) - Asserts mid-range port present (
35500) — catches "endpoints only" regressions - Asserts every discrete declared port present
- Asserts SSH port 22 still excluded even with the real bash-subshell loader
internal/ports/panel_loader.parsePortList confirmed to expand ranges (lines 347 in panel_loader.go); the test pins this contract.
D. Stale PR26.4 follow-up text removed
directadmin.go::ValidateReachability doc-comment and error message no longer reference "PR26.4 follow-up" or "validated in PR26.4". New wording reflects the post-PR26.4 reality:
- Doc-comment: "RequiredPorts (PR26.4) loads the full DirectAdmin service-port surface from the canonical conf.d via panel_loader, but this method deliberately does NOT probe each conf.d-declared port"
- Error:
"...the full DirectAdmin port surface is loaded from conf.d via RequiredPorts but not probed here"
grep "PR26.4 follow-up\|validated in PR26.4" returns zero hits across both adapter files.
E. Misleading test renamed
TestFrameworkIntegration_DA_Reason_DoesNotImplyFullPortSurvival
→
TestFrameworkIntegration_DA_ControlPlaneError_DoesNotClaimFullSurfaceReachability
Semantic unchanged (control-plane unreachable error must not claim full-surface probing). The name now matches post-PR26.4 reality where RequiredPorts declaratively reports the full conf.d surface but ValidateReachability still probes the control plane only.
Lab4 proof (post A–E, base 5366caf5)
go vet ./internal/installer/panelfw/... ./internal/ports/... ./internal/installer/validate/... ./cmd/nftban-installer/...
→ VET_EXIT=0 (clean)
go test -count=1 -v ./internal/installer/panelfw/... ./internal/ports/... ./internal/installer/validate/...
→ 100 sub-tests PASS, 0 FAIL (was 98 pre-patch; +2 explicit guards)
→ ok internal/installer/panelfw 0.003s
→ ok internal/installer/panelfw/adapters/directadmin 0.023s
→ ok internal/ports 0.006s
→ ok internal/installer/validate 0.007s
go test -count=1 ./...
→ 66 packages PASS, 0 FAIL
TMPDIR=/root/build-tmp honored.
Self-audit
- No fallback to
[2222]forRequiredPortsunder any conf.d failure (asserted in 4 tests viaassertNoControlPlaneFallback) - Control-plane reachability remains separate (
ValidateReachabilitybody unchanged from PR26.3 + clarified doc-comment + error wording) - conf.d is source-of-truth (
RequiredPortsreads exclusively viapanelConfDLoader; no embedded port list) - No other panels/adapters (
internal/installer/panelfw/adapters/still contains onlydirectadmin/) - No restore/firewall/authority/shell/parser changes (
panel_loader.gountouched; shell tree untouched; no restore-side or authority-classifier edits)
Requesting auditor final GO. PR26.4 not merged; PR26.5 / Gate B retry not started.
🤖 Generated with Claude Code
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bridges the DirectAdmin panelfw adapter to the canonical panel config at
etc/nftban/conf.d/panels/directadmin/main.confviainternal/ports/panel_loader.LoadPanelConfig("directadmin"). The adapter'sRequiredPortsno longer hardcodes a[2222]-only port list; it returns the full conf.d-declared TCP_IN / UDP_IN port surface.This is the consolidation step the auditor's PR26.3 disposition (CONDITIONAL-GO → Path A) flagged for follow-up. PR26.3 framed scope ("control-plane only"); PR26.4 implements the full-surface load.
Scope (per V190_PANELS/PANEL_ARCHITECTURE_AUDIT.md)
etc/nftban/ports.d/00-ssh.confand is intentionally absent from panel TCP_IN per the audit's four-truth resolution)Hard exclusions
What landed
internal/installer/panelfw/adapters/directadmin/directadmin.gointernal/ports; newpanelConfDLoader/panelConfDDirseam;RequiredPortsrewrites to callLoadPanelConfig; fail-closed on missing/nil/empty TCP_IN;ValidateReachabilityunchanged (control-plane only); doc-comment expandedinternal/installer/panelfw/adapters/directadmin/directadmin_test.gowithStubLoader/withFixtureConfDhelpers;canonicalDAfixture; new RequiredPorts tests covering full surface, fail-closed paths, and defensive copy; integration test via realLoadPanelConfigagainst tempdir fixture; ValidateReachability override tests moved here from RequiredPortsAdapter behavior changes
RequiredPortssource[2222]internal/ports/panel_loader.LoadPanelConfig("directadmin")RequiredPortsUDPnilcfg.UDPInfrom conf.dRequiredPortson missing config[2222]PanelResult.Fatal=trueRequiredPortson empty TCP_INFatal=trueValidateReachabilitydirectadmin.conf port=NoverrideDetectTests required by spec
TestRequiredPorts_ConfDLoaded_FullSurface(stub) +TestRequiredPorts_ConfDLoaded_RealLoader_FixtureFile(integration)[2222]-onlyTestRequiredPorts_ConfDLoaded_NotJust2222port=NoverrideTestValidateReachability_ConfigOverride_HonoredByControlPlaneTestRequiredPorts_MissingConfD_FailsClosed(stub) +TestRequiredPorts_RealLoader_MissingConfD_FailsClosed(integration)TestRequiredPorts_EmptyTCPIn_FailsClosed+TestRequiredPorts_NilPanelConfig_FailsClosedinternal/installer/panelfw/adapters/still contains onlydirectadmin/panel_loader.go::LoadPanelConfigLab proof (lab4, RHEL/cPanel, go1.25.8)
Branch base:
5366caf5(origin/main, post-PR26.3 merge).TMPDIR=/root/build-tmp(lab4: both/tmpand/var/tmpare noexec under cPanel/usr/tmpDSK).Self-audit
LoadPanelConfig; no embedded port listcanonicalDAfixture appears in tests onlyValidateReachabilityis still control-plane only (default 2222 +directadmin.confport=N)ls internal/installer/panelfw/adapters/→directadminonly)TestReadOnly_NoWrites_NoMutationCommandsupdated to stub the loader)Test plan
go test ./internal/installer/panelfw/...green on lab4go test ./internal/ports/...green on lab4go test ./internal/installer/validate/...green on lab4go vet ./internal/installer/panelfw/... ./internal/ports/... ./internal/installer/validate/... ./cmd/nftban-installer/...clean on lab4go test ./...green on lab4 (66 packages, 0 fail)🤖 Generated with Claude Code