Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions auth/whitelist_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package auth

import (
"os"
"path/filepath"
"testing"

"ghproxy/config"
)

func TestSplitUserRepoWhitelist_PreservesHyphenatedOwner(t *testing.T) {
user, repo := splitUserRepoWhitelist("jow-/ucode")

if user != "jow-" {
t.Fatalf("user = %q, want %q", user, "jow-")
}
if repo != "ucode" {
t.Fatalf("repo = %q, want %q", repo, "ucode")
}
}

func TestInitWhitelist_AllowsHyphenatedOwnerRepo(t *testing.T) {
tmpDir := t.TempDir()
whitelistPath := filepath.Join(tmpDir, "whitelist.json")

content := []byte(`{"whitelist":["jow-/ucode"]}`)
if err := os.WriteFile(whitelistPath, content, 0o600); err != nil {
t.Fatalf("write whitelist file: %v", err)
}

whitelistInstance = nil
whitelistInitErr = nil

cfg := &config.Config{}
cfg.Whitelist.WhitelistFile = whitelistPath

if err := InitWhitelist(cfg); err != nil {
t.Fatalf("InitWhitelist() error = %v", err)
}

if !CheckWhitelist("jow-", "ucode") {
t.Fatal("CheckWhitelist() = false, want true for jow-/ucode")
}

if CheckWhitelist("jow", "ucode") {
t.Fatal("CheckWhitelist() = true, want false for non-hyphenated owner")
}

if CheckWhitelist("jow-", "other") {
t.Fatal("CheckWhitelist() = true, want false for unmatched repo")
}

t.Cleanup(func() {
whitelistInstance = nil
whitelistInitErr = nil
})
Comment on lines +31 to +56

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The t.Cleanup function should be registered immediately after the global variables are modified. In its current position at the end of the test, it will not be executed if InitWhitelist fails or if any assertion fails before the registration point. Moving it up ensures that the global state is properly reset even on test failure, preventing side effects on other tests in the same package.

	whitelistInstance = nil
	whitelistInitErr = nil
	t.Cleanup(func() {
		whitelistInstance = nil
		whitelistInitErr = nil
	})

	cfg := &config.Config{}
	cfg.Whitelist.WhitelistFile = whitelistPath

	if err := InitWhitelist(cfg); err != nil {
		t.Fatalf("InitWhitelist() error = %v", err)
	}

	if !CheckWhitelist("jow-", "ucode") {
		t.Fatal("CheckWhitelist() = false, want true for jow-/ucode")
	}

	if CheckWhitelist("jow", "ucode") {
		t.Fatal("CheckWhitelist() = true, want false for non-hyphenated owner")
	}

	if CheckWhitelist("jow-", "other") {
		t.Fatal("CheckWhitelist() = true, want false for unmatched repo")
	}

Comment on lines +31 to +56

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

t.Cleanup 前置并恢复旧状态,避免失败路径污染全局变量。

当前在 Line 53 才注册清理;如果 Line 37-50 之间触发 t.Fatalf,清理函数不会注册,whitelistInstance/whitelistInitErr 可能泄漏到后续测试。建议在重置前保存旧值并立即注册 cleanup。

🛠️ 建议修改
-	whitelistInstance = nil
-	whitelistInitErr = nil
+	prevWhitelistInstance := whitelistInstance
+	prevWhitelistInitErr := whitelistInitErr
+	t.Cleanup(func() {
+		whitelistInstance = prevWhitelistInstance
+		whitelistInitErr = prevWhitelistInitErr
+	})
+	whitelistInstance = nil
+	whitelistInitErr = nil
@@
-	t.Cleanup(func() {
-		whitelistInstance = nil
-		whitelistInitErr = nil
-	})

Based on learnings: The blacklist and whitelist functionality in auth/ package uses maps for O(1) lookups, separates user-level and repo-level controls, and ensures thread safety using sync.Once.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@auth/whitelist_test.go` around lines 31 - 56, Save the current globals
whitelistInstance and whitelistInitErr into local variables and immediately
register a t.Cleanup that restores those saved values before performing the test
actions (i.e. call t.Cleanup right after declaring cfg and before calling
InitWhitelist/CheckWhitelist); this ensures that if InitWhitelist or any
t.Fatalf runs the cleanup is still registered and the globals are restored.
Refer to symbols: whitelistInstance, whitelistInitErr, InitWhitelist,
CheckWhitelist, and t.Cleanup when making the change.

}
Loading