mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-08-20 18:11:00 +00:00
3f1dd4bf5a
* fix: follow-ups from the post-merge reviews of #6221, #6227, #6230 and #6239 Six defects the automated reviews found after those PRs merged. Each is verified rather than taken on trust — two by experiment, the rest by reading the merged code. **Import restore never wrote an empty local value** (#6227). GORM builds the assignment map from the struct passed to Assign and drops zero-valued fields, so `Assign(model.Setting{Value: ""})` produced an empty Updates and the imported row survived. Empty is the normal state: UpdateAllSetting writes a row for every AllSetting field including the blank ones. That is exactly the case the PR existed for — a destination with no certificate inheriting the source machine's path. Confirmed with a throwaway test before changing anything: the value stayed "IMPORTED". Now uses saveSetting, which is not zero-filtered. **Import destroyed node mTLS material** (#6227). The "no local row means the default applied, so drop the import" branch fires for the five nodeMtls* keys, which are minted on demand and deliberately absent from AllSetting, so a fresh install has no row for them. Reinstall-then-restore therefore deleted the CA certificate and its private key — and the backup was the only copy, since neither is surfaced in the UI or the export. Those keys are now kept. **The clients-list enable toggle wiped renewal state** (#6239, #6238). setEnable hand-builds the update payload and carried reset but not resetDay or resetMax, so one click on the switch turned calendar mode off and lifted the renewal cap permanently. The form-modal tests could not catch it because that path does send both fields. **"Delete depleted clients" deleted calendar clients** (#6239). The predicate read `reset = 0` as "does not auto-renew", which is exactly the calendar shape, in two places. Both now share one constant that also requires `reset_day = 0`. **Allowlist validation and parsing disagreed** (#6230). Save used net, scan used netip, and they differ: `198.51.100.0/024` saves without complaint and is silently dropped at scan — the failure the PR set out to remove. Verified by running both parsers. An IPv4-mapped prefix parsed but could never match, because contains() unmaps the query while the prefix stayed 128-bit; it is unmapped at parse now. A test asserts the two acceptance sets agree. **A comment stated the opposite of the truth** (#6221). GetInbounds has no enable filter, so a node reports a disabled inbound normally; the row in that bug report was missing only because it was never delivered. Reworded to the real invariant. Also trims two comment blocks in ip_limit_allowlist.go to the repo's two-line maximum. Not included: the reviewer's suggestion to lift the node hand-off out of `if inbound.Enable` in AddInbound. It is the right root-cause fix, but it changes delivery behaviour on multi-node deployments and belongs in its own change with its own testing, not in a cleanup batch. One reported finding is not real: BulkCreate does call validateClientResetDay, validateClientResetMax and validateClientTrafficReset — verified in the merged tree. * fix(netsafe): wrap both errors so errorlint passes Unrelated to this PR's subject and in a file it does not otherwise touch. It is here only because CI lints the merge result, and `main` has been red since #6242 landed: `fmt.Errorf("%w; %v", ...)` wraps the first error and formats the second, which errorlint rejects. Go 1.20 allows more than one %w, so both are wrapped now and `errors.Is` works against either.
150 lines
5.0 KiB
Go
150 lines
5.0 KiB
Go
package service
|
|
|
|
import (
|
|
"testing"
|
|
|
|
"github.com/mhsanaei/3x-ui/v3/internal/database"
|
|
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
|
|
)
|
|
|
|
// An imported database carries the source machine's listen addresses,
|
|
// certificates and node identity. Keeping this machine's own values is what
|
|
// stops the panel from becoming unreachable on its own address after a restore.
|
|
func TestImportKeepsHostBoundSettings(t *testing.T) {
|
|
setupConflictDB(t)
|
|
db := database.GetDB()
|
|
|
|
mine := map[string]string{
|
|
"webPort": "8443",
|
|
"webCertFile": "/etc/ssl/this-host.pem",
|
|
"webBasePath": "/mine/",
|
|
"subURI": "https://this-host.example/sub/",
|
|
"panelGuid": "this-host-guid",
|
|
"nodeMtlsClientCertPem": "this-host-leaf",
|
|
}
|
|
for key, value := range mine {
|
|
if err := db.Create(&model.Setting{Key: key, Value: value}).Error; err != nil {
|
|
t.Fatalf("seed %s: %v", key, err)
|
|
}
|
|
}
|
|
// A setting that belongs to the configuration, not the machine.
|
|
if err := db.Create(&model.Setting{Key: "remarkTemplate", Value: "mine"}).Error; err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
kept := captureHostBoundSettings()
|
|
if len(kept.values) != len(mine) {
|
|
t.Fatalf("captured %d host settings, want %d: %v", len(kept.values), len(mine), kept.values)
|
|
}
|
|
|
|
// Stand in for the import: every row now holds the source machine's value.
|
|
for key := range mine {
|
|
if err := db.Model(&model.Setting{}).Where("key = ?", key).
|
|
Update("value", "from-imported-file").Error; err != nil {
|
|
t.Fatalf("overwrite %s: %v", key, err)
|
|
}
|
|
}
|
|
if err := db.Model(&model.Setting{}).Where("key = ?", "remarkTemplate").
|
|
Update("value", "from-imported-file").Error; err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
restoreHostBoundSettings(kept)
|
|
|
|
for key, want := range mine {
|
|
var got model.Setting
|
|
if err := db.Where("key = ?", key).First(&got).Error; err != nil {
|
|
t.Fatalf("read back %s: %v", key, err)
|
|
}
|
|
if got.Value != want {
|
|
t.Fatalf("setting %s = %q after import, want this machine's %q", key, got.Value, want)
|
|
}
|
|
}
|
|
|
|
var carried model.Setting
|
|
if err := db.Where("key = ?", "remarkTemplate").First(&carried).Error; err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if carried.Value != "from-imported-file" {
|
|
t.Fatalf("remarkTemplate = %q, want the imported value: only host-bound keys may survive", carried.Value)
|
|
}
|
|
}
|
|
|
|
// A certificate path this machine never set must not be inherited from the
|
|
// source. Lazily minted material is the opposite case and is covered below.
|
|
func TestImportDropsHostBoundSettingsThisMachineNeverHad(t *testing.T) {
|
|
setupConflictDB(t)
|
|
db := database.GetDB()
|
|
|
|
kept := captureHostBoundSettings()
|
|
|
|
for _, key := range []string{"webCertFile", "subCertFile"} {
|
|
if err := db.Create(&model.Setting{Key: key, Value: "from-imported-file"}).Error; err != nil {
|
|
t.Fatalf("seed imported %s: %v", key, err)
|
|
}
|
|
}
|
|
|
|
restoreHostBoundSettings(kept)
|
|
|
|
for _, key := range []string{"webCertFile", "subCertFile"} {
|
|
var count int64
|
|
if err := db.Model(&model.Setting{}).Where("key = ?", key).Count(&count).Error; err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if count != 0 {
|
|
t.Fatalf("imported %s survived although this machine had no row for it", key)
|
|
}
|
|
}
|
|
}
|
|
|
|
// Node mTLS material is minted on demand, so a fresh install has no row and the
|
|
// imported copy is the only one there is — including the CA private key (#6227).
|
|
func TestImportKeepsLazilyMintedMaterialThisMachineNeverHad(t *testing.T) {
|
|
setupConflictDB(t)
|
|
db := database.GetDB()
|
|
|
|
kept := captureHostBoundSettings()
|
|
|
|
for _, key := range []string{"nodeMtlsCaCertPem", "nodeMtlsCaKeyPem", "nodeMtlsClientCertPem", "nodeMtlsClientKeyPem", "nodeMtlsClientCAPem"} {
|
|
if err := db.Create(&model.Setting{Key: key, Value: "from-imported-file"}).Error; err != nil {
|
|
t.Fatalf("seed imported %s: %v", key, err)
|
|
}
|
|
}
|
|
|
|
restoreHostBoundSettings(kept)
|
|
|
|
for _, key := range []string{"nodeMtlsCaCertPem", "nodeMtlsCaKeyPem", "nodeMtlsClientCertPem", "nodeMtlsClientKeyPem", "nodeMtlsClientCAPem"} {
|
|
var got model.Setting
|
|
if err := db.Where("key = ?", key).First(&got).Error; err != nil {
|
|
t.Fatalf("%s was dropped; restoring a backup onto a reinstalled panel would lose it: %v", key, err)
|
|
}
|
|
}
|
|
}
|
|
|
|
// An empty local value is the normal state once Panel Settings has been saved:
|
|
// GORM's Assign(struct) dropped it, so the source machine's path survived.
|
|
func TestImportRestoresEmptyLocalValueOverImported(t *testing.T) {
|
|
setupConflictDB(t)
|
|
db := database.GetDB()
|
|
|
|
if err := db.Create(&model.Setting{Key: "webCertFile", Value: ""}).Error; err != nil {
|
|
t.Fatalf("seed local empty: %v", err)
|
|
}
|
|
kept := captureHostBoundSettings()
|
|
|
|
if err := db.Model(&model.Setting{}).Where("key = ?", "webCertFile").
|
|
Update("value", "/etc/ssl/source-host.pem").Error; err != nil {
|
|
t.Fatalf("seed imported: %v", err)
|
|
}
|
|
|
|
restoreHostBoundSettings(kept)
|
|
|
|
var got model.Setting
|
|
if err := db.Where("key = ?", "webCertFile").First(&got).Error; err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if got.Value != "" {
|
|
t.Fatalf("webCertFile = %q, want the empty local value back: the panel still points at the source machine's certificate", got.Value)
|
|
}
|
|
}
|