[security-fix] Security Fix: Prevent unsafe quoting in network hook generation (Alert #16)

[github-actions]

Security Fix: Prevent Unsafe Quoting in Network Hook Generation

Alert Number: #16
Severity: Critical (security_severity_level)
Rule: go/unsafe-quoting (CWE-78, CWE-89, CWE-94)

Vulnerability Description

CodeQL identified an unsafe quoting vulnerability in pkg/workflow/engine_network_hooks.go:112. 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 flagged this pattern as a potential injection vector. If the JSON content contains double quotes or other special characters, it could theoretically break out of the string context and alter the Python script's structure.

Vulnerable Code Pattern:

domainsJSON := string(jsonBytes)
// ...
return fmt.Sprintf(`...
ALLOWED_DOMAINS = %s
...`, domainsJSON)

The issue occurs at:

• pkg/workflow/engine_network_hooks.go:45 (ALLOWED_DOMAINS assignment)
• pkg/workflow/engine_network_hooks.go:114 (fmt.Sprintf call)

Fix Applied

Changed the approach from embedding JSON as a Python literal to using Python's json.loads() to parse the JSON string at runtime:

1. Added proper escaping: Escape backslashes and single quotes before embedding the JSON string
2. Runtime parsing: Changed from direct literal embedding to ALLOWED_DOMAINS = json.loads('...')
3. Clear security intent: Comments and code structure now make the security considerations explicit

Fixed Code:

// Security fix for CWE-78, CWE-89, CWE-94: Properly escape the JSON string
// before embedding it in Python code to prevent quote-injection vulnerabilities.
// We escape backslashes first, then single quotes, and use json.loads() at runtime.
escapedJSON := strings.ReplaceAll(domainsJSON, `\`, `\\`)
escapedJSON = strings.ReplaceAll(escapedJSON, `'`, `\'`)

return fmt.Sprintf(`...
ALLOWED_DOMAINS = json.loads('%s')
...`, escapedJSON)

Security Best Practices Applied

✅ Never embed serialized data directly into code literals
✅ Always use proper escaping for the target language
✅ Prefer runtime parsing over literal embedding for complex data
✅ Escape backslashes first to prevent escape sequence interference
✅ Make security considerations explicit in code comments

Testing

• ✅ All existing tests pass with updated expectations
• ✅ The generated Python script correctly parses domain lists using json.loads()
• ✅ Empty domain lists (deny-all policy) are handled correctly: ALLOWED_DOMAINS = json.loads('[]')
• ✅ Domain patterns with special characters are properly escaped

Changes Made

1. Modified pkg/workflow/engine_network_hooks.go:

   • Added escaping logic for backslashes and single quotes (lines 27-28)
   • Changed ALLOWED_DOMAINS assignment to use json.loads() (line 45)
   • Updated comments to reflect the security fix (lines 24-31)
   • Updated fmt.Sprintf call to use escapedJSON (line 114)

2. Updated pkg/workflow/engine_network_test.go:

   • Fixed test expectations to match new json.loads() pattern (line 72)
   • Updated test description to reflect the security fix (line 71)

Impact

This fix eliminates the CWE-78 (Command Injection), CWE-89 (SQL Injection), and CWE-94 (Code Injection) vulnerabilities identified by CodeQL without changing the functionality of the network permissions system.

---

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

AI generated by Security Fix Agent

AI generated by Security Fix PR