Skip to content

Commit 7306c78

Browse files
yroblataskbot
andauthored
feat: only allow http urls for private registry urls (#1391)
Co-authored-by: taskbot <[email protected]>
1 parent 9142c96 commit 7306c78

File tree

2 files changed

+80
-3
lines changed

2 files changed

+80
-3
lines changed

pkg/api/v1/registry_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,76 @@ func TestGetRegistryInfo(t *testing.T) {
112112
}
113113
}
114114

115+
//nolint:paralleltest // uses MockConfig (env mutation)
116+
func TestSetRegistryURL_SchemeAndPrivateIPs(t *testing.T) {
117+
logger.Initialize()
118+
119+
// Isolate config (XDG) for each run
120+
cleanup := MockConfig(t, nil)
121+
t.Cleanup(cleanup)
122+
123+
tests := []struct {
124+
name string
125+
url string
126+
allowPrivateIPs bool
127+
wantErr bool
128+
}{
129+
// Scheme enforcement when NOT allowing private IPs
130+
{
131+
name: "reject http (public) when not allowing private IPs",
132+
url: "http://example.com/registry.json",
133+
allowPrivateIPs: false,
134+
wantErr: true,
135+
},
136+
{
137+
name: "accept https (public) when not allowing private IPs",
138+
url: "https://example.com/registry.json",
139+
allowPrivateIPs: false,
140+
wantErr: false,
141+
},
142+
143+
// When allowing private IPs, https is allowed to private hosts
144+
{
145+
name: "accept https to loopback when allowing private IPs",
146+
url: "https://127.0.0.1/registry.json",
147+
allowPrivateIPs: true,
148+
wantErr: false,
149+
},
150+
{
151+
name: "accept http to loopback when allowing private IPs",
152+
url: "http://127.0.0.1/registry.json",
153+
allowPrivateIPs: true,
154+
wantErr: false,
155+
},
156+
}
157+
158+
for _, tt := range tests {
159+
tt := tt
160+
t.Run(tt.name, func(t *testing.T) {
161+
// Start from a clean slate each case
162+
require.NoError(t, config.UnsetRegistry())
163+
164+
err := config.SetRegistryURL(tt.url, tt.allowPrivateIPs)
165+
if tt.wantErr {
166+
require.Error(t, err, "expected error but got nil")
167+
168+
// Verify nothing was persisted
169+
regType, src := getRegistryInfo()
170+
assert.Equal(t, RegistryTypeDefault, regType, "registry should remain default on error")
171+
assert.Equal(t, "", src, "source should be empty on error")
172+
return
173+
}
174+
175+
require.NoError(t, err, "unexpected error from SetRegistryURL")
176+
177+
// Confirm via the same helper used elsewhere
178+
regType, src := getRegistryInfo()
179+
assert.Equal(t, RegistryTypeURL, regType, "should be URL type after successful SetRegistryURL")
180+
assert.Equal(t, tt.url, src, "source should be the URL we set")
181+
})
182+
}
183+
}
184+
115185
func TestRegistryAPI_PutEndpoint(t *testing.T) {
116186
t.Parallel()
117187

pkg/config/registry.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,16 @@ func DetectRegistryType(input string) (registryType string, cleanPath string) {
3535

3636
// SetRegistryURL validates and sets a registry URL
3737
func SetRegistryURL(registryURL string, allowPrivateRegistryIp bool) error {
38-
// Basic URL validation - check if it starts with http:// or https://
39-
if registryURL != "" && !strings.HasPrefix(registryURL, "http://") && !strings.HasPrefix(registryURL, "https://") {
40-
return fmt.Errorf("registry URL must start with http:// or https://")
38+
if allowPrivateRegistryIp {
39+
// we validate either https or http URLs
40+
if !strings.HasPrefix(registryURL, "http://") && !strings.HasPrefix(registryURL, "https://") {
41+
return fmt.Errorf("registry URL must start with http:// or https:// when allowing private IPs")
42+
}
43+
} else {
44+
// we just allow https
45+
if !strings.HasPrefix(registryURL, "https://") {
46+
return fmt.Errorf("registry URL must start with https:// when not allowing private IPs")
47+
}
4148
}
4249

4350
if !allowPrivateRegistryIp {

0 commit comments

Comments
 (0)