Skip to content

Commit 292e1ed

Browse files
authored
security(config): project config should not override sandbox.network to allow (#379)
* security(config): project config should not override sandbox.network to allow * fix(config): use NormalizeNetworkMode in project config merge, per coderabbit review
1 parent 39e7100 commit 292e1ed

2 files changed

Lines changed: 49 additions & 1 deletion

File tree

‎internal/config/resolver.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,17 @@ func mergeProjectConfig(dst *FileConfig, src FileConfig) error {
226226
// config: a cloned repo's .zero/config.json must not be able to grant
227227
// itself write access outside the workspace. Global config and CLI flags
228228
// are the only config sources for write roots.
229+
//
230+
// Sandbox.Network from project config may only TIGHTEN (→ "deny"), never
231+
// WEAKEN (→ "allow"). A malicious repo must not be able to open network
232+
// access for shell commands. Matches the AdditionalWriteRoots posture.
229233
if network := strings.TrimSpace(src.Sandbox.Network); network != "" {
230-
dst.Sandbox.Network = network
234+
// Normalize: anything that isn't literally "allow" counts as deny.
235+
if sandbox.NormalizeNetworkMode(sandbox.NetworkMode(network)) != sandbox.NetworkAllow {
236+
dst.Sandbox.Network = network
237+
}
238+
// Silently ignore "allow" from project config — not an error, just
239+
// a privilege the project scope does not have.
231240
}
232241
if src.Sandbox.BlockUnixSockets {
233242
dst.Sandbox.BlockUnixSockets = true

‎internal/config/resolver_test.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,6 +1273,45 @@ func TestResolveSandboxBlockUnixSocketsFromFile(t *testing.T) {
12731273
}
12741274
}
12751275

1276+
func TestResolveSandboxNetworkProjectConfigCannotWeaken(t *testing.T) {
1277+
// 1. Project config tries to set network to "allow" (should be ignored).
1278+
userPath := writeConfig(t, `{}`)
1279+
projectPath := writeConfig(t, `{
1280+
"sandbox": {"network": "allow"}
1281+
}`)
1282+
1283+
resolved, err := Resolve(ResolveOptions{
1284+
UserConfigPath: userPath,
1285+
ProjectConfigPath: projectPath,
1286+
Env: map[string]string{},
1287+
})
1288+
if err != nil {
1289+
t.Fatalf("Resolve() error = %v", err)
1290+
}
1291+
if resolved.Sandbox.Network == "allow" {
1292+
t.Fatal("resolved.Sandbox.Network = allow, want empty/deny to prevent project config from weakening sandbox")
1293+
}
1294+
1295+
// 2. User config sets network to "allow", project config sets it to "deny" (tightening is allowed).
1296+
userPath2 := writeConfig(t, `{
1297+
"sandbox": {"network": "allow"}
1298+
}`)
1299+
projectPath2 := writeConfig(t, `{
1300+
"sandbox": {"network": "deny"}
1301+
}`)
1302+
resolved2, err := Resolve(ResolveOptions{
1303+
UserConfigPath: userPath2,
1304+
ProjectConfigPath: projectPath2,
1305+
Env: map[string]string{},
1306+
})
1307+
if err != nil {
1308+
t.Fatalf("Resolve() error = %v", err)
1309+
}
1310+
if resolved2.Sandbox.Network != "deny" {
1311+
t.Fatalf("resolved.Sandbox.Network = %q, want deny (project config allowed to tighten)", resolved2.Sandbox.Network)
1312+
}
1313+
}
1314+
12761315
func TestResolveNotifyValid(t *testing.T) {
12771316
path := writeConfig(t, `{"notify":{"mode":"both","focusMode":"always"}}`)
12781317
resolved, err := Resolve(ResolveOptions{UserConfigPath: path, Env: map[string]string{}})

0 commit comments

Comments
 (0)