From 011f5f1a3d29fc89a2fb9ff1b1a51efd2d00d339 Mon Sep 17 00:00:00 2001
From: viettranx <viettranx@gmail.com>
Date: Mon, 30 Mar 2026 14:11:42 +0700
Subject: [PATCH] fix(security): harden env file permissions and block NUL byte
 injection (#306, #44)

- prepare-env.sh: chmod 600 after .env creation to prevent world-readable secrets
- shell.go: reject commands containing NUL bytes before execution
- credentialed_exec.go: defense-in-depth NUL check in credentialed path
- shell_deny_test.go: add TestExecute_RejectsNULByte with 6 test cases
---
 internal/tools/credentialed_exec.go |  7 ++++++-
 internal/tools/shell.go             |  5 +++++
 internal/tools/shell_deny_test.go   | 32 +++++++++++++++++++++++++++++
 prepare-env.sh                      |  2 ++
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/internal/tools/credentialed_exec.go b/internal/tools/credentialed_exec.go
index 7bed0f73..d5de99fe 100644
--- a/internal/tools/credentialed_exec.go
+++ b/internal/tools/credentialed_exec.go
@@ -104,8 +104,13 @@ func matchesBinaryDeny(args []string, denyPatternsJSON json.RawMessage) string {
 func (t *ExecTool) executeCredentialed(ctx context.Context, cred *store.SecureCLIBinary,
 	binary string, args []string, cwd string, sandboxKey string) *Result {
 
-	// Step 1: Check for shell operators (early detection for clear error)
+	// Step 0: Reject NUL bytes (defense-in-depth — also checked in Execute()).
 	rawCommand := binary + " " + strings.Join(args, " ")
+	if strings.ContainsRune(rawCommand, '\x00') {
+		return ErrorResult("command contains invalid NUL byte")
+	}
+
+	// Step 1: Check for shell operators (early detection for clear error)
 	if ops := detectShellOperators(rawCommand); len(ops) > 0 {
 		return credentialedShellOperatorError(rawCommand, ops)
 	}
diff --git a/internal/tools/shell.go b/internal/tools/shell.go
index 73272357..9bc8f35f 100644
--- a/internal/tools/shell.go
+++ b/internal/tools/shell.go
@@ -113,6 +113,11 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *Result {
 		return ErrorResult("command is required")
 	}
 
+	// Reject NUL bytes — they cause silent shell truncation enabling injection.
+	if strings.ContainsRune(command, '\x00') {
+		return ErrorResult("command contains invalid NUL byte")
+	}
+
 	// Resolve deny patterns: per-agent overrides from context, fallback to all defaults.
 	denyOverrides := store.ShellDenyGroupsFromContext(ctx)
 	groupPatterns := ResolveDenyPatterns(denyOverrides)
diff --git a/internal/tools/shell_deny_test.go b/internal/tools/shell_deny_test.go
index c7c5de9f..eb3e18a3 100644
--- a/internal/tools/shell_deny_test.go
+++ b/internal/tools/shell_deny_test.go
@@ -1,7 +1,9 @@
 package tools
 
 import (
+	"context"
 	"regexp"
+	"strings"
 	"testing"
 )
 
@@ -138,6 +140,36 @@ func TestPrivilegeEscalationGaps(t *testing.T) {
 	)
 }
 
+func TestExecute_RejectsNULByte(t *testing.T) {
+	tool := &ExecTool{} // minimal instance, no sandbox/workspace needed
+
+	cases := []struct {
+		name    string
+		command string
+		reject  bool
+	}{
+		{"nul_mid", "ls\x00/etc/passwd", true},
+		{"nul_mid_echo", "echo hello\x00world", true},
+		{"nul_prefix", "\x00rm -rf /", true},
+		{"nul_only", "\x00", true},
+		{"normal_cmd", "echo normal", false},
+		{"empty_cmd", "", false}, // handled by "command is required"
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := tool.Execute(context.Background(), map[string]any{"command": tc.command})
+			hasNULError := result != nil && strings.Contains(result.ForLLM, "NUL byte")
+			if tc.reject && !hasNULError {
+				t.Errorf("expected NUL rejection for %q, got: %v", tc.name, result.ForLLM)
+			}
+			if !tc.reject && hasNULError {
+				t.Errorf("unexpected NUL rejection for %q", tc.name)
+			}
+		})
+	}
+}
+
 func TestLimitedBuffer(t *testing.T) {
 	t.Run("under limit", func(t *testing.T) {
 		lb := &limitedBuffer{max: 100}
diff --git a/prepare-env.sh b/prepare-env.sh
index 90ecebec..6c3058db 100755
--- a/prepare-env.sh
+++ b/prepare-env.sh
@@ -52,9 +52,11 @@ if [ ! -f "$ENV_FILE" ]; then
   if [ -f ".env.example" ]; then
     # Strip 'export ' prefix for Docker Compose compatibility
     sed 's/^export //' .env.example > "$ENV_FILE"
+    chmod 600 "$ENV_FILE"
     echo "  [created]   .env from .env.example"
   else
     touch "$ENV_FILE"
+    chmod 600 "$ENV_FILE"
     echo "  [created]   .env (empty)"
   fi
 else