From 78f047c5cb22f57346d692a11cb51c95e60e31ba Mon Sep 17 00:00:00 2001 From: Ole-Kenneth Bratholt Date: Fri, 20 Oct 2023 15:38:41 +0200 Subject: [PATCH 01/10] Add AllowList to config --- server/config/config.go | 100 ++++++++++++++++++++++++++++------------ 1 file changed, 70 insertions(+), 30 deletions(-) diff --git a/server/config/config.go b/server/config/config.go index 388f84249..af05fb73d 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -16,26 +16,27 @@ import ( const MinBuildConcurrency = 4 type Config struct { - Port uint16 `json:"port,omitempty"` - TlsPort uint16 `json:"tlsPort,omitempty"` - NsPort uint16 `json:"nsPort,omitempty"` - BuildConcurrency uint16 `json:"buildConcurrency,omitempty"` - BanList BanList `json:"banList,omitempty"` - AuthSecret string `json:"authSecret,omitempty"` - WorkDir string `json:"workDir,omitempty"` - Cache string `json:"cache,omitempty"` - Database string `json:"database,omitempty"` - Storage string `json:"storage,omitempty"` - LogLevel string `json:"logLevel,omitempty"` - LogDir string `json:"logDir,omitempty"` - CdnOrigin string `json:"cdnOrigin,omitempty"` - CdnBasePath string `json:"cdnBasePath,omitempty"` - NpmRegistry string `json:"npmRegistry,omitempty"` - NpmToken string `json:"npmToken,omitempty"` - NpmRegistryScope string `json:"npmRegistryScope,omitempty"` - NpmUser string `json:"npmUser,omitempty"` - NpmPassword string `json:"npmPassword,omitempty"` - NoCompress bool `json:"noCompress,omitempty"` + Port uint16 `json:"port,omitempty"` + TlsPort uint16 `json:"tlsPort,omitempty"` + NsPort uint16 `json:"nsPort,omitempty"` + BuildConcurrency uint16 `json:"buildConcurrency,omitempty"` + BanList BanList `json:"banList,omitempty"` + AllowList AllowList `json:"allowList,omitempty"` + AuthSecret string `json:"authSecret,omitempty"` + WorkDir string `json:"workDir,omitempty"` + Cache string `json:"cache,omitempty"` + Database string `json:"database,omitempty"` + Storage string `json:"storage,omitempty"` + LogLevel string `json:"logLevel,omitempty"` + LogDir string `json:"logDir,omitempty"` + CdnOrigin string `json:"cdnOrigin,omitempty"` + CdnBasePath string `json:"cdnBasePath,omitempty"` + NpmRegistry string `json:"npmRegistry,omitempty"` + NpmToken string `json:"npmToken,omitempty"` + NpmRegistryScope string `json:"npmRegistryScope,omitempty"` + NpmUser string `json:"npmUser,omitempty"` + NpmPassword string `json:"npmPassword,omitempty"` + NoCompress bool `json:"noCompress,omitempty"` } type BanList struct { @@ -48,6 +49,16 @@ type BanScope struct { Excludes []string `json:"excludes"` } +type AllowList struct { + Packages []string `json:"packages"` + Scopes []AllowScope `json:"scopes"` +} + +type AllowScope struct { + Name string `json:"name"` + Includes []string `json:"includes"` +} + // Load loads config from the given file. Panic if failed to load. func Load(filename string) (*Config, error) { var ( @@ -188,16 +199,14 @@ func fixConfig(c *Config) *Config { return c } -// IsPackageBanned Checking if the package is banned. -// The `packages` list is the highest priority ban rule to match, -// so the `excludes` list in the `scopes` list won't take effect if the package is banned in `packages` list -func (banList *BanList) IsPackageBanned(fullName string) bool { - var ( - fullNameWithoutVersion string // e.g. @github/faker - scope string // e.g. @github - nameWithoutVersionScope string // e.g. faker - ) - paths := strings.Split(fullName, "/") +// extractPackageName Will take a packageName as input extract key +// parts and return them +// +// fullNameWithoutVersion e.g. @github/faker +// scope e.g. @github +// nameWithoutVersionScope e.g. faker +func extractPackageName(packageName string) (fullNameWithoutVersion string, scope string, nameWithoutVersionScope string) { + paths := strings.Split(packageName, "/") if len(paths) < 2 { // the package has no scope prefix nameWithoutVersionScope = strings.Split(paths[0], "@")[0] @@ -208,6 +217,15 @@ func (banList *BanList) IsPackageBanned(fullName string) bool { fullNameWithoutVersion = fmt.Sprintf("%s/%s", scope, nameWithoutVersionScope) } + return fullNameWithoutVersion, scope, nameWithoutVersionScope +} + +// IsPackageBanned Checking if the package is banned. +// The `packages` list is the highest priority ban rule to match, +// so the `excludes` list in the `scopes` list won't take effect if the package is banned in `packages` list +func (banList *BanList) IsPackageBanned(fullName string) bool { + fullNameWithoutVersion, scope, nameWithoutVersionScope := extractPackageName(fullName) + for _, p := range banList.Packages { if fullNameWithoutVersion == p { return true @@ -223,6 +241,28 @@ func (banList *BanList) IsPackageBanned(fullName string) bool { return false } +// IsPackageAllowed Checking if the package is allowed. +// The `packages` list is the highest priority allow rule to match, +// so the `includes` list in the `scopes` list won't take effect if the package is allowed in `packages` list +func (allowList *AllowList) IsPackageAllowed(fullName string) bool { + fullNameWithoutVersion, scope, nameWithoutVersionScope := extractPackageName(fullName) + + for _, p := range allowList.Packages { + if fullNameWithoutVersion == p { + return false + } + } + + for _, s := range allowList.Scopes { + if scope == s.Name { + return !isPackageExcluded(nameWithoutVersionScope, s.Includes) + } + } + + return true +} + + func isPackageExcluded(name string, excludes []string) bool { for _, exclude := range excludes { if name == exclude { From 3831257bde7deac08d780d9b53d90d2e06f51d1d Mon Sep 17 00:00:00 2001 From: Ole-Kenneth Bratholt Date: Fri, 20 Oct 2023 15:41:37 +0200 Subject: [PATCH 02/10] Rename isPackageExcluded to be generic --- server/config/config.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/config/config.go b/server/config/config.go index af05fb73d..13f01737b 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -234,7 +234,7 @@ func (banList *BanList) IsPackageBanned(fullName string) bool { for _, s := range banList.Scopes { if scope == s.Name { - return !isPackageExcluded(nameWithoutVersionScope, s.Excludes) + return !isPackageInList(nameWithoutVersionScope, s.Excludes) } } @@ -255,7 +255,7 @@ func (allowList *AllowList) IsPackageAllowed(fullName string) bool { for _, s := range allowList.Scopes { if scope == s.Name { - return !isPackageExcluded(nameWithoutVersionScope, s.Includes) + return !isPackageInList(nameWithoutVersionScope, s.Includes) } } @@ -263,9 +263,9 @@ func (allowList *AllowList) IsPackageAllowed(fullName string) bool { } -func isPackageExcluded(name string, excludes []string) bool { - for _, exclude := range excludes { - if name == exclude { +func isPackageInList(name string, list []string) bool { + for _, list := range list { + if name == list { return true } } From 6db30acbf7dc49cc6e59dc2a479f3522f083073e Mon Sep 17 00:00:00 2001 From: Ole-Kenneth Bratholt Date: Fri, 20 Oct 2023 15:42:49 +0200 Subject: [PATCH 03/10] Not ! in list --- server/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/config/config.go b/server/config/config.go index 13f01737b..ac907e600 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -255,7 +255,7 @@ func (allowList *AllowList) IsPackageAllowed(fullName string) bool { for _, s := range allowList.Scopes { if scope == s.Name { - return !isPackageInList(nameWithoutVersionScope, s.Includes) + return isPackageInList(nameWithoutVersionScope, s.Includes) } } From 07581c8de03d91732c8306bcb6f3691ee11737fe Mon Sep 17 00:00:00 2001 From: Ole-Kenneth Bratholt Date: Mon, 23 Oct 2023 13:42:25 +0200 Subject: [PATCH 04/10] Add tests and fix issues --- server/config/config.go | 12 +-- server/config/config_test.go | 163 +++++++++++++++++++++++++++++++++++ 2 files changed, 170 insertions(+), 5 deletions(-) diff --git a/server/config/config.go b/server/config/config.go index ac907e600..46e5cac48 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -56,7 +56,6 @@ type AllowList struct { type AllowScope struct { Name string `json:"name"` - Includes []string `json:"includes"` } // Load loads config from the given file. Panic if failed to load. @@ -245,21 +244,24 @@ func (banList *BanList) IsPackageBanned(fullName string) bool { // The `packages` list is the highest priority allow rule to match, // so the `includes` list in the `scopes` list won't take effect if the package is allowed in `packages` list func (allowList *AllowList) IsPackageAllowed(fullName string) bool { - fullNameWithoutVersion, scope, nameWithoutVersionScope := extractPackageName(fullName) + if len(allowList.Packages) == 0 && len(allowList.Scopes) == 0 { + return true + } + fullNameWithoutVersion, scope, _ := extractPackageName(fullName) for _, p := range allowList.Packages { if fullNameWithoutVersion == p { - return false + return true } } for _, s := range allowList.Scopes { if scope == s.Name { - return isPackageInList(nameWithoutVersionScope, s.Includes) + return true } } - return true + return false } diff --git a/server/config/config_test.go b/server/config/config_test.go index 128d59d7b..b2b2e76b3 100644 --- a/server/config/config_test.go +++ b/server/config/config_test.go @@ -4,6 +4,168 @@ import ( "testing" ) +func TestAllowListAndBanList_IsPackageNotAllowedOrBanned(t *testing.T) { + type args struct { + fullName string + } + tests := []struct { + name string + allowList AllowList + banList BanList + args args + want bool + }{ + { + name: "NoAllowOrBanListAllowAnything", + allowList: AllowList{}, + banList: BanList{}, + args: args{fullName: "faker@1.5.0"}, + want: false, + }, + { + name: "AllowedScopeBannedScope", + allowList: AllowList{ + Scopes: []AllowScope{{ + Name: "@github", + }}, + }, + banList: BanList{ + Scopes: []BanScope{{ + Name: "@github", + }}, + }, + args: args{fullName: "@github/faker"}, + want: true, + }, + { + name: "AllowedScopeBannedPackage", + allowList: AllowList{ + Scopes: []AllowScope{{ + Name: "@github", + }}, + }, + banList: BanList{ + Packages: []string{"@github/faker"}, + }, + args: args{fullName: "@github/faker"}, + want: true, + }, + { + name: "AllowedPackageBannedPackage", + allowList: AllowList{ + Packages: []string{"@github/faker"}, + }, + banList: BanList{ + Packages: []string{"faker"}, + }, + args: args{fullName: "faker"}, + want: true, + }, + { + name: "AllowedPackageBannedScope", + allowList: AllowList{ + Packages: []string{"faker"}, + }, + banList: BanList{ + Scopes: []BanScope{{ + Name: "@github", + }}, + }, + args: args{fullName: "@github/faker"}, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // to simulate: + // if !pkgAllowed || pkgBanned { + // return rex.Status(403, "forbidden") + // } + packageName := tt.args.fullName + + isAllowed := tt.allowList.IsPackageAllowed(packageName) + isBanned := tt.banList.IsPackageBanned(packageName) + + if got := !isAllowed || isBanned; got != tt.want { + t.Errorf("isPackageNotAllowedOrBanned() = %v, want %v. %v isAllowed %v, %v isBanned %v", got, tt.want, packageName, isAllowed, packageName, isBanned) + } + }) + } +} + + +func TestAllowList_IsPackageAllowed(t *testing.T) { + type args struct { + fullName string + } + tests := []struct { + name string + allowList AllowList + args args + want bool + }{ + { + name: "NoAllowListAllowAnything", + allowList: AllowList{}, + args: args{fullName: "faker@1.5.0"}, + want: true, + }, + { + name: "AllowedByPackages", + allowList: AllowList{ + Packages: []string{"faker"}, + }, + args: args{fullName: "faker"}, + want: true, + }, + { + name: "NotAllowedByPackages", + allowList: AllowList{ + Packages: []string{"allowedPackageName"}, + }, + args: args{fullName: "faker"}, + want: false, + }, + { + name: "AllowedByScope", + allowList: AllowList{ + Scopes: []AllowScope{{ + Name: "@github", + }}, + }, + args: args{fullName: "@github/perfect"}, + want: true, + }, + { + name: "NotAllowedByScope", + allowList: AllowList{ + Scopes: []AllowScope{{ + Name: "@github", + }}, + }, + args: args{fullName: "@faker/perfect"}, + want: false, + }, + { + name: "NotAllowedByScope", + allowList: AllowList{ + Scopes: []AllowScope{{ + Name: "@github", + }}, + }, + args: args{fullName: "@faker/perfect"}, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.allowList.IsPackageAllowed(tt.args.fullName); got != tt.want { + t.Errorf("IsPackageAllowed() = %v, want %v", got, tt.want) + } + }) + } +} + func TestBanList_IsPackageBanned(t *testing.T) { type args struct { fullName string @@ -70,4 +232,5 @@ func TestBanList_IsPackageBanned(t *testing.T) { } }) } + } From 6d5a207e675c4b76bde61ca642cdbe4fc63e24cd Mon Sep 17 00:00:00 2001 From: Ole-Kenneth Bratholt Date: Mon, 23 Oct 2023 13:43:00 +0200 Subject: [PATCH 05/10] Use allow package in server --- server/server_handler.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/server_handler.go b/server/server_handler.go index b34d8c9d5..7db77d9ff 100644 --- a/server/server_handler.go +++ b/server/server_handler.go @@ -524,8 +524,9 @@ func esmHandler() rex.Handle { // trim the leading `/` in pathname to get the package name // e.g. /@ORG/PKG -> @ORG/PKG packageFullName := pathname[1:] + pkgAllowed := cfg.BanList.IsAllowedBanned(packageFullName) pkgBanned := cfg.BanList.IsPackageBanned(packageFullName) - if pkgBanned { + if !pkgAllowed || pkgBanned { return rex.Status(403, "forbidden") } From 24f03fb91f77bbd8963e4092294f71a13688ad52 Mon Sep 17 00:00:00 2001 From: Ole-Kenneth Bratholt Date: Mon, 23 Oct 2023 13:50:31 +0200 Subject: [PATCH 06/10] Not rename method --- server/config/config.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/server/config/config.go b/server/config/config.go index 46e5cac48..17a87cd5f 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -233,7 +233,7 @@ func (banList *BanList) IsPackageBanned(fullName string) bool { for _, s := range banList.Scopes { if scope == s.Name { - return !isPackageInList(nameWithoutVersionScope, s.Excludes) + return !isPackageExcluded(nameWithoutVersionScope, s.Excludes) } } @@ -264,10 +264,9 @@ func (allowList *AllowList) IsPackageAllowed(fullName string) bool { return false } - -func isPackageInList(name string, list []string) bool { - for _, list := range list { - if name == list { +func isPackageExcluded(name string, excludes []string) bool { + for _, exclude := range excludes { + if name == exclude { return true } } From 84259c386323c5fc110877d3623307f1059e0237 Mon Sep 17 00:00:00 2001 From: Ole-Kenneth Bratholt Date: Mon, 23 Oct 2023 13:52:41 +0200 Subject: [PATCH 07/10] Remove lineshift --- server/config/config_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/server/config/config_test.go b/server/config/config_test.go index b2b2e76b3..1a02cde0b 100644 --- a/server/config/config_test.go +++ b/server/config/config_test.go @@ -232,5 +232,4 @@ func TestBanList_IsPackageBanned(t *testing.T) { } }) } - } From de404e3228889ed5688e93c266e0d0a20968b713 Mon Sep 17 00:00:00 2001 From: Ole-Kenneth Bratholt Date: Tue, 24 Oct 2023 09:49:39 +0200 Subject: [PATCH 08/10] Add allow list in config example --- config.example.jsonc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/config.example.jsonc b/config.example.jsonc index 9d51db32a..3df481fc4 100644 --- a/config.example.jsonc +++ b/config.example.jsonc @@ -74,5 +74,13 @@ "package_name" ] }] + }, + + // The list to only allow some packages or scopes. + "allowList": { + "packages": ["@some_scope/package_name"], + "scopes": [{ + "name": "@your_scope", + }] } } From fdd796a78fe22c621ec2a06de38c8505f7369acf Mon Sep 17 00:00:00 2001 From: Ole-Kenneth Date: Sun, 5 Nov 2023 14:27:45 +0100 Subject: [PATCH 09/10] Fix typo server_handler.go --- server/server_handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/server_handler.go b/server/server_handler.go index 7db77d9ff..24a587de6 100644 --- a/server/server_handler.go +++ b/server/server_handler.go @@ -524,7 +524,7 @@ func esmHandler() rex.Handle { // trim the leading `/` in pathname to get the package name // e.g. /@ORG/PKG -> @ORG/PKG packageFullName := pathname[1:] - pkgAllowed := cfg.BanList.IsAllowedBanned(packageFullName) + pkgAllowed := cfg.AllowList.isPackageAllowed(packageFullName) pkgBanned := cfg.BanList.IsPackageBanned(packageFullName) if !pkgAllowed || pkgBanned { return rex.Status(403, "forbidden") From 9d2c6e95ef66bfe55878d10e9ef54a3d0c886fb5 Mon Sep 17 00:00:00 2001 From: Ole-Kenneth Date: Sun, 5 Nov 2023 14:29:44 +0100 Subject: [PATCH 10/10] Fix typo --- server/server_handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/server_handler.go b/server/server_handler.go index 24a587de6..2b0736270 100644 --- a/server/server_handler.go +++ b/server/server_handler.go @@ -524,7 +524,7 @@ func esmHandler() rex.Handle { // trim the leading `/` in pathname to get the package name // e.g. /@ORG/PKG -> @ORG/PKG packageFullName := pathname[1:] - pkgAllowed := cfg.AllowList.isPackageAllowed(packageFullName) + pkgAllowed := cfg.AllowList.IsPackageAllowed(packageFullName) pkgBanned := cfg.BanList.IsPackageBanned(packageFullName) if !pkgAllowed || pkgBanned { return rex.Status(403, "forbidden")