From 2be4a422f87d6849e8990dd7438113755397b00e Mon Sep 17 00:00:00 2001 From: Security Fix PR Date: Thu, 16 Oct 2025 02:11:13 +0000 Subject: [PATCH] Security Fix: Eliminate unsafe quoting in network permissions hook generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Alert Number**: #16 **Severity**: Critical (security_severity_level) **Rule**: go/unsafe-quoting (CWE-78, CWE-89, CWE-94) ## Vulnerability Description The code was directly embedding JSON-serialized domain arrays into a Python script using fmt.Sprintf without proper escaping. While json.Marshal output is typically safe for []string types, CodeQL correctly identified this as a potential injection vector since the JSON content could theoretically contain quotes that break out of the string context. ## Fix Applied Changed the approach from embedding JSON as a Python literal to using Python's json.loads() to parse the JSON at runtime: 1. Added proper escaping for backslashes and single quotes before embedding 2. Changed from `ALLOWED_DOMAINS = %s` (direct literal) to `ALLOWED_DOMAINS = json.loads('%s')` (runtime parsing) 3. Updated the escapedJSON variable usage in fmt.Sprintf This eliminates any potential quoting vulnerabilities by: - Explicitly escaping special characters (\ and ') - Using json.loads() which safely handles all JSON content - Making the security intent clearer in the code ## Security Best Practices - Never embed user-controlled or serialized data directly into code literals - Always use proper escaping mechanisms for the target language - Prefer runtime parsing over literal embedding for complex data structures - Escape backslashes first to prevent escape sequence interference ## Testing Considerations - All existing tests pass with updated expectations - The generated Python script correctly parses domain lists - Empty domain lists (deny-all policy) are handled correctly - Domain patterns with special characters are properly escaped 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/workflow/engine_network_hooks.go | 18 +++++++++++------- pkg/workflow/engine_network_test.go | 4 ++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/workflow/engine_network_hooks.go b/pkg/workflow/engine_network_hooks.go index a2bd3dda1e6..15ce319b2cf 100644 --- a/pkg/workflow/engine_network_hooks.go +++ b/pkg/workflow/engine_network_hooks.go @@ -21,12 +21,16 @@ func (g *NetworkHookGenerator) GenerateNetworkHookScript(allowedDomains []string domainsJSON = string(jsonBytes) } - // Embed domain list JSON directly as a Python literal (safe for []string from json.Marshal) - // This prevents any quote-related injection vulnerabilities (CWE-78, CWE-89, CWE-94) - // Use domainsJSON directly for ALLOWED_DOMAINS assignment + // Use json.loads() to parse the JSON string at runtime instead of embedding as a literal + // This eliminates any potential quoting issues (CWE-78, CWE-89, CWE-94) + // The JSON string is embedded as a Python string literal with proper escaping + + // Escape backslashes and single quotes for safe embedding in Python string literal + escapedJSON := strings.ReplaceAll(domainsJSON, `\`, `\\`) + escapedJSON = strings.ReplaceAll(escapedJSON, `'`, `\'`) // Build the Python script using a safe template approach - // The JSON array is embedded directly as a Python list literal + // The JSON is parsed at runtime using json.loads() to avoid literal embedding issues return fmt.Sprintf(`#!/usr/bin/env python3 """ Network permissions validator for Claude Code engine. @@ -39,8 +43,8 @@ import urllib.parse import re # Domain allow-list (populated during generation) -# JSON array safely embedded as Python list literal -ALLOWED_DOMAINS = %s +# JSON string is parsed at runtime to avoid quoting vulnerabilities +ALLOWED_DOMAINS = json.loads('%s') def extract_domain(url_or_query): """Extract domain from URL or search query.""" @@ -109,7 +113,7 @@ try: except Exception as e: print(f"Network validation error: {e}", file=sys.stderr) sys.exit(2) # Block on errors -`, domainsJSON) +`, escapedJSON) } // GenerateNetworkHookWorkflowStep generates a GitHub Actions workflow step that creates the network permissions hook diff --git a/pkg/workflow/engine_network_test.go b/pkg/workflow/engine_network_test.go index dbd29faf4d6..4f071c5ca2d 100644 --- a/pkg/workflow/engine_network_test.go +++ b/pkg/workflow/engine_network_test.go @@ -68,8 +68,8 @@ func TestNetworkHookGenerator(t *testing.T) { allowedDomains := []string{} // Empty list means deny-all script := generator.GenerateNetworkHookScript(allowedDomains) - // Should still generate a valid script - if !strings.Contains(script, "ALLOWED_DOMAINS = []") { + // Should still generate a valid script with json.loads() parsing + if !strings.Contains(script, "ALLOWED_DOMAINS = json.loads('[]')") { t.Error("Script should handle empty domains list (deny-all policy)") } if !strings.Contains(script, "def is_domain_allowed") {