mirror of
https://github.com/safedep/pmg.git
synced 2026-08-03 07:24:09 +02:00
feat(sandbox): add network_via_proxy_only and allow_direct_dns policy fields (#370)
* feat(sandbox): add network_via_proxy_only and allow_direct_dns policy fields Config surface for network lockdown: network_via_proxy_only confines a sandboxed package manager's outbound network to the PMG proxy; allow_direct_dns is its escape hatch re-opening direct DNS. Both follow the existing pointer-bool inheritance pattern in MergeWithParent. Lint warns when allow_direct_dns is set without network_via_proxy_only, where it has no effect. The fields are declared and inherited but unread; enforcement lands with the ExecutionContext plumbing and Seatbelt lockdown translation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PqMU5GNBbQvQct9nxek1VS * fix(sandbox): deep-copy new pointer flags in profile resolution expandPolicyPaths re-points the older pointer booleans so callers cannot corrupt the registry-cached policy; the new NetworkViaProxyOnly and AllowDirectDNS fields need the same isolation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PqMU5GNBbQvQct9nxek1VS --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
e6b5157a2a
commit
22d6eabb6b
@@ -5,6 +5,7 @@ import (
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/safedep/dry/utils"
|
||||
"github.com/safedep/pmg/sandbox/util"
|
||||
)
|
||||
|
||||
@@ -119,6 +120,15 @@ func LintProfile(policy *SandboxPolicy) []LintIssue {
|
||||
}
|
||||
}
|
||||
|
||||
if utils.SafelyGetValue(policy.AllowDirectDNS) && !utils.SafelyGetValue(policy.NetworkViaProxyOnly) {
|
||||
warns = append(warns, LintIssue{
|
||||
Level: LintLevelWarn,
|
||||
Code: "allow-direct-dns-without-lockdown",
|
||||
Message: "allow_direct_dns has no effect unless network_via_proxy_only is true",
|
||||
Field: "allow_direct_dns",
|
||||
})
|
||||
}
|
||||
|
||||
conflictPairs := []struct {
|
||||
allowName string
|
||||
allow []string
|
||||
|
||||
@@ -3,7 +3,9 @@ package sandbox
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/safedep/dry/utils"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// cleanPolicy returns a minimally-valid policy with no lint issues.
|
||||
@@ -230,3 +232,51 @@ func TestLintProfile_OrderingErrorsBeforeWarnsBeforeInfo(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestLintProfile_AllowDirectDNSWithoutLockdown(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
allowDirectDNS *bool
|
||||
networkViaProxyOnly *bool
|
||||
wantWarn bool
|
||||
}{
|
||||
{
|
||||
name: "allow_direct_dns without lockdown warns",
|
||||
allowDirectDNS: utils.PtrTo(true),
|
||||
wantWarn: true,
|
||||
},
|
||||
{
|
||||
name: "allow_direct_dns with lockdown is clean",
|
||||
allowDirectDNS: utils.PtrTo(true),
|
||||
networkViaProxyOnly: utils.PtrTo(true),
|
||||
wantWarn: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
policy := cleanPolicy()
|
||||
policy.AllowDirectDNS = tc.allowDirectDNS
|
||||
policy.NetworkViaProxyOnly = tc.networkViaProxyOnly
|
||||
|
||||
var found *LintIssue
|
||||
for _, i := range LintProfile(policy) {
|
||||
if i.Code == "allow-direct-dns-without-lockdown" {
|
||||
issue := i
|
||||
found = &issue
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
if !tc.wantWarn {
|
||||
assert.Nil(t, found)
|
||||
return
|
||||
}
|
||||
|
||||
require.NotNil(t, found)
|
||||
assert.Equal(t, LintLevelWarn, found.Level)
|
||||
assert.Equal(t, "allow_direct_dns", found.Field)
|
||||
assert.Equal(t, "allow_direct_dns has no effect unless network_via_proxy_only is true", found.Message)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -38,6 +38,16 @@ type SandboxPolicy struct {
|
||||
|
||||
// AllowNetworkBind allows binding to localhost (127.0.0.1 / ::1) for listening.
|
||||
AllowNetworkBind *bool `yaml:"allow_network_bind" json:"allow_network_bind"`
|
||||
|
||||
// NetworkViaProxyOnly confines all outbound network access to the PMG
|
||||
// proxy. Requires the proxy flow; drivers fail closed without a running
|
||||
// proxy.
|
||||
NetworkViaProxyOnly *bool `yaml:"network_via_proxy_only" json:"network_via_proxy_only"`
|
||||
|
||||
// AllowDirectDNS re-opens direct DNS (mDNSResponder) under
|
||||
// NetworkViaProxyOnly. No effect otherwise. Default false: the proxy
|
||||
// resolves names and direct DNS is an exfiltration channel.
|
||||
AllowDirectDNS *bool `yaml:"allow_direct_dns" json:"allow_direct_dns"`
|
||||
}
|
||||
|
||||
// FilesystemPolicy defines allowed and denied filesystem access patterns.
|
||||
@@ -162,6 +172,14 @@ func (child *SandboxPolicy) MergeWithParent(parent *SandboxPolicy) {
|
||||
if child.AllowNetworkBind == nil {
|
||||
child.AllowNetworkBind = utils.PtrTo(utils.SafelyGetValue(parent.AllowNetworkBind))
|
||||
}
|
||||
|
||||
if child.NetworkViaProxyOnly == nil {
|
||||
child.NetworkViaProxyOnly = utils.PtrTo(utils.SafelyGetValue(parent.NetworkViaProxyOnly))
|
||||
}
|
||||
|
||||
if child.AllowDirectDNS == nil {
|
||||
child.AllowDirectDNS = utils.PtrTo(utils.SafelyGetValue(parent.AllowDirectDNS))
|
||||
}
|
||||
}
|
||||
|
||||
// unionStringSlices returns a new slice containing all unique elements from both slices.
|
||||
|
||||
@@ -286,3 +286,83 @@ func TestValidateResolved(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergeWithParentNetworkViaProxyOnly(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
parent *bool
|
||||
child *bool
|
||||
expected bool
|
||||
}{
|
||||
{
|
||||
name: "child overrides parent false with true",
|
||||
parent: utils.PtrTo(false),
|
||||
child: utils.PtrTo(true),
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "child nil inherits parent true",
|
||||
parent: utils.PtrTo(true),
|
||||
child: nil,
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "both nil defaults to false",
|
||||
parent: nil,
|
||||
child: nil,
|
||||
expected: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
parent := &SandboxPolicy{NetworkViaProxyOnly: tt.parent}
|
||||
child := &SandboxPolicy{NetworkViaProxyOnly: tt.child}
|
||||
|
||||
child.MergeWithParent(parent)
|
||||
|
||||
assert.NotNil(t, child.NetworkViaProxyOnly)
|
||||
assert.Equal(t, tt.expected, *child.NetworkViaProxyOnly)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergeWithParentAllowDirectDNS(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
parent *bool
|
||||
child *bool
|
||||
expected bool
|
||||
}{
|
||||
{
|
||||
name: "child overrides parent false with true",
|
||||
parent: utils.PtrTo(false),
|
||||
child: utils.PtrTo(true),
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "child nil inherits parent true",
|
||||
parent: utils.PtrTo(true),
|
||||
child: nil,
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "both nil defaults to false",
|
||||
parent: nil,
|
||||
child: nil,
|
||||
expected: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
parent := &SandboxPolicy{AllowDirectDNS: tt.parent}
|
||||
child := &SandboxPolicy{AllowDirectDNS: tt.child}
|
||||
|
||||
child.MergeWithParent(parent)
|
||||
|
||||
assert.NotNil(t, child.AllowDirectDNS)
|
||||
assert.Equal(t, tt.expected, *child.AllowDirectDNS)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -39,6 +39,12 @@ func expandPolicyPaths(p *SandboxPolicy, opts ResolveOptions) (*SandboxPolicy, e
|
||||
if p.AllowNetworkBind != nil {
|
||||
out.AllowNetworkBind = utils.PtrTo(*p.AllowNetworkBind)
|
||||
}
|
||||
if p.NetworkViaProxyOnly != nil {
|
||||
out.NetworkViaProxyOnly = utils.PtrTo(*p.NetworkViaProxyOnly)
|
||||
}
|
||||
if p.AllowDirectDNS != nil {
|
||||
out.AllowDirectDNS = utils.PtrTo(*p.AllowDirectDNS)
|
||||
}
|
||||
|
||||
allowRead, err := expandSlice(p.Filesystem.AllowRead, opts)
|
||||
if err != nil {
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/safedep/dry/utils"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
@@ -120,3 +121,34 @@ func TestResolveProfileDoesNotMutateRegistry(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, originalAllowRead, after.Filesystem.AllowRead, "registry profile must not be mutated")
|
||||
}
|
||||
|
||||
func TestExpandPolicyPathsIsolatesBoolPointers(t *testing.T) {
|
||||
source := &SandboxPolicy{
|
||||
Name: "ptr-isolation",
|
||||
PackageManagers: []string{"npm"},
|
||||
Filesystem: FilesystemPolicy{AllowRead: []string{"/tmp"}},
|
||||
AllowGitConfig: utils.PtrTo(true),
|
||||
AllowPTY: utils.PtrTo(true),
|
||||
AllowNetworkBind: utils.PtrTo(true),
|
||||
NetworkViaProxyOnly: utils.PtrTo(true),
|
||||
AllowDirectDNS: utils.PtrTo(true),
|
||||
}
|
||||
|
||||
resolved, err := expandPolicyPaths(source, ResolveOptions{CWD: "/x", Home: "/y"})
|
||||
require.NoError(t, err)
|
||||
|
||||
fields := []struct {
|
||||
name string
|
||||
source, resolved *bool
|
||||
}{
|
||||
{"AllowGitConfig", source.AllowGitConfig, resolved.AllowGitConfig},
|
||||
{"AllowPTY", source.AllowPTY, resolved.AllowPTY},
|
||||
{"AllowNetworkBind", source.AllowNetworkBind, resolved.AllowNetworkBind},
|
||||
{"NetworkViaProxyOnly", source.NetworkViaProxyOnly, resolved.NetworkViaProxyOnly},
|
||||
{"AllowDirectDNS", source.AllowDirectDNS, resolved.AllowDirectDNS},
|
||||
}
|
||||
for _, f := range fields {
|
||||
assert.NotSame(t, f.source, f.resolved, f.name)
|
||||
assert.Equal(t, *f.source, *f.resolved, f.name)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user