docker/harbor
1
0
Fork 0
mirror of https://github.com/goharbor/harbor.git synced 2026-10-23 10:08:57 +03:00
Code Activity

Fix webhook egress allow all (#24126)

Browse Source 
fix(webhook): default webhook egress policy level to allow_all

When the webhook egress policy is empty or unset, default to egressAllowAll
to preserve Harbor's default egress behavior and avoid regressions for internal
and test webhook targets.

Signed-off-by: stonezdj <stonezdj@gmail.com>
This commit is contained in:
stonezdj(Daojun Zhang)
2026-10-16 15:06:05 +08:00
committed by GitHub
parent 1edae4acdb
commit f1b4e3ccc6
3 changed files with 41 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/common/http/private_network.go
View File

@@ -33,7 +33,7 @@ import (
)
// webhookEgressPolicyEnv selects which destinations the webhook and Slack notification clients
// may reach: allow_all, block_restricted (the default) or public_only. See egressLevel.
// may reach: allow_all (the default), block_restricted or public_only. See egressLevel.
const webhookEgressPolicyEnv = "HARBOR_WEBHOOK_EGRESS_POLICY"
// webhookEgressAllowlistEnv lists destinations the notification clients may always reach, whatever
@@ -216,7 +216,7 @@ func parseEgressLevel(policyEnv, legacyEnv string) egressLevel {
}
value := strings.TrimSpace(legacyEnv)
if value == "" {
return egressBlockRestricted
return egressAllowAll
}
allowed, err := strconv.ParseBool(value)
switch {

47
src/common/http/private_network_test.go
View File

@@ -112,10 +112,32 @@ func TestValidateNetworkTarget(t *testing.T) {
}
}
// TestValidateNetworkTargetDefault covers the default policy: internal webhook receivers keep
// working, while targets that reach the host itself or cloud metadata stay blocked.
// TestValidateNetworkTargetDefault covers the default policy (allow_all): all targets are allowed.
func TestValidateNetworkTargetDefault(t *testing.T) {
setEgressEnv(t, "", "", "")
resolver := &stubNetworkResolver{answers: map[string][]netip.Addr{
"rebind.example.com": {netip.MustParseAddr("169.254.169.254")},
}}
for _, host := range []string{
"8.8.8.8",
"10.0.0.1",
"127.0.0.1",
"169.254.169.254",
"localhost",
"metadata.google.internal",
"rebind.example.com",
} {
t.Run("allows "+host, func(t *testing.T) {
require.NoError(t, ValidateNetworkTarget(context.Background(), resolver, host))
})
}
}
// TestValidateNetworkTargetBlockRestricted covers the block_restricted policy: internal webhook receivers keep
// working, while targets that reach the host itself or cloud metadata stay blocked.
func TestValidateNetworkTargetBlockRestricted(t *testing.T) {
setEgressEnv(t, "block_restricted", "", "")
resolver := &stubNetworkResolver{answers: map[string][]netip.Addr{
"receiver.ns.svc.cluster.local": {netip.MustParseAddr("10.96.12.7")},
"rebind.example.com": {netip.MustParseAddr("169.254.169.254")},
@@ -174,8 +196,8 @@ func TestEgressLevelPrecedence(t *testing.T) {
policy, legacy string
want egressLevel
}{
{"", "", egressBlockRestricted},
{" ", " ", egressBlockRestricted},
{"", "", egressAllowAll},
{" ", " ", egressAllowAll},
{"", "true", egressAllowAll},
{"", "1", egressAllowAll},
{"", "false", egressPublicOnly},
@@ -202,7 +224,7 @@ func TestValidateNetworkTargetPerLevel(t *testing.T) {
{"allow_all", "", true, true, true},
{"", "true", true, true, true},
{"block_restricted", "", true, true, false},
{"", "", true, true, false},
{"", "", true, true, true},
{"public_only", "", true, false, false},
{"", "false", true, false, false},
} {
@@ -271,7 +293,7 @@ func TestValidateNetworkTargetAllowlist(t *testing.T) {
}
func TestCheckEgress(t *testing.T) {
setEgressEnv(t, "", "", "")
setEgressEnv(t, "block_restricted", "", "")
require.NoError(t, checkEgress("tcp4", "10.0.0.1:443", nil))
require.NoError(t, checkEgress("tcp6", "[fd12::1]:443", nil))
require.ErrorContains(t, checkEgress("tcp4", "169.254.169.254:80", nil), "restricted network address")
@@ -280,10 +302,15 @@ func TestCheckEgress(t *testing.T) {
setEgressEnv(t, "public_only", "", "")
require.ErrorContains(t, checkEgress("tcp4", "10.0.0.1:443", nil), "private network address")
require.NoError(t, checkEgress("tcp4", "8.8.8.8:443", nil))
setEgressEnv(t, "", "", "")
require.NoError(t, checkEgress("tcp4", "10.0.0.1:443", nil))
require.NoError(t, checkEgress("tcp4", "169.254.169.254:80", nil))
require.NoError(t, checkEgress("tcp4", "127.0.0.1:80", nil))
}
func TestWithEgressGuard(t *testing.T) {
setEgressEnv(t, "", "", "")
setEgressEnv(t, "block_restricted", "", "")
listener, err := net.Listen("tcp", "127.0.0.1:0")
require.NoError(t, err)
t.Cleanup(func() { _ = listener.Close() })
@@ -396,7 +423,7 @@ func TestWithEgressGuardProxyBlocksPrivateTargets(t *testing.T) {
// proxy itself: a direct request to the proxy's address must not inherit the dial exemption the
// proxy gets once it has been used.
func TestWithEgressGuardProxyAddressNotReachableDirectly(t *testing.T) {
setEgressEnv(t, "", "", "")
setEgressEnv(t, "block_restricted", "", "")
proxy := newRecordingProxy(t)
transport := &http.Transport{Proxy: func(req *http.Request) (*url.URL, error) {
if req.URL.Host == proxy.url.Host {
@@ -419,7 +446,7 @@ func TestWithEgressGuardProxyAddressNotReachableDirectly(t *testing.T) {
// TestWithEgressGuardNoProxyStillGuardsDial covers NO_PROXY: when the proxy function declines a
// target, the direct dial keeps the resolved-address check.
func TestWithEgressGuardNoProxyStillGuardsDial(t *testing.T) {
setEgressEnv(t, "", "", "")
setEgressEnv(t, "block_restricted", "", "")
target := newRecordingProxy(t)
transport := &http.Transport{Proxy: func(*http.Request) (*url.URL, error) { return nil, nil }}
withEgressGuard(&stubNetworkResolver{})(transport)
@@ -449,7 +476,7 @@ func TestWithEgressGuardProxyPrivateAllowed(t *testing.T) {
func TestParseEgressPolicy(t *testing.T) {
policy := parseEgressPolicy("", "", "10.0.0.0/8 ::ffff:172.16.0.0/108, 192.168.1.10, fe80::1%eth0, *.svc.cluster.local, Hooks.Example.COM., bad host, -, *.")
assert.Equal(t, egressBlockRestricted, policy.level)
assert.Equal(t, egressAllowAll, policy.level)
assert.Equal(t, []netip.Prefix{
netip.MustParsePrefix("10.0.0.0/8"),
netip.MustParsePrefix("172.16.0.0/12"),

4
src/jobservice/job/impl/notification/ssrf_test.go
View File

@@ -123,10 +123,10 @@ func TestNotificationClientsKeepProxy(t *testing.T) {
}
// TestNotificationClientsGuardRestrictedTargets asserts the production notification clients carry
// the egress dial guard: with the default policy, which allows private networks, a loopback target
// the egress dial guard: under block_restricted, which allows private networks, a loopback target
// is still refused before any request reaches it.
func TestNotificationClientsGuardRestrictedTargets(t *testing.T) {
t.Setenv("HARBOR_WEBHOOK_EGRESS_POLICY", "")
t.Setenv("HARBOR_WEBHOOK_EGRESS_POLICY", "block_restricted")
t.Setenv("HARBOR_ALLOW_PRIVATE_NETWORK_ACCESS", "")
t.Setenv("HARBOR_WEBHOOK_EGRESS_ALLOWLIST", "")