[security-fix] Security Fix: Unsafe Quoting in Network Hook Generation (Alert #9)

[github-actions]

Security Fix: Unsafe Quoting in Network Hook Generation

Alert Number: #9
Severity: Critical (security_severity_level: critical)
Rule: go/unsafe-quoting
File: pkg/workflow/engine_network_hooks.go:104
CWE: CWE-78 (OS Command Injection), CWE-89 (SQL Injection), CWE-94 (Code Injection)

Vulnerability Description

The GenerateNetworkHookScript function was embedding JSON-encoded domain data directly into a Python script template without proper quote escaping. The vulnerable pattern was:

ALLOWED_DOMAINS = %s

If the JSON data contained double quotes or other special characters, it could break out of the intended string context in the generated Python script. While JSON arrays are syntactically compatible with Python literals in most cases, this pattern is flagged as unsafe because:

1. It relies on implicit compatibility between JSON and Python syntax
2. Special characters in domain strings could potentially cause parsing issues
3. It doesn't follow secure coding practices for template generation

Fix Applied

The fix implements proper quote escaping using Go's standard library:

1. Added strconv import: Provides access to strconv.Quote() function
2. Applied strconv.Quote(): Safely escapes the JSON string for Python embedding
3. Updated Python template: Changed to use json.loads() with the properly quoted string
4. Added security documentation: Included comments explaining the security rationale

Before:

domainsJSON, _ := json.Marshal(allowedDomains)
// ...
ALLOWED_DOMAINS = %s
// ...
`, string(domainsJSON))

After:

jsonBytes, _ := json.Marshal(allowedDomains)
domainsJSON := string(jsonBytes)
quotedJSON := strconv.Quote(domainsJSON)
// ...
ALLOWED_DOMAINS = json.loads(%s)
// ...
`, quotedJSON)

Security Best Practices Applied

✅ Input Sanitization: All dynamic data is properly escaped before embedding
✅ Defense in Depth: Using json.loads() adds an additional parsing layer
✅ Standard Library Functions: Leveraging strconv.Quote() ensures correct escaping
✅ Code Documentation: Clear comments explain the security considerations

Testing Considerations

To validate this fix, please test:

• Network hook generation with standard domain lists
• Domain lists containing special characters (quotes, backslashes, unicode)
• Empty domain lists (deny-all policy)
• Generated Python scripts parse and execute correctly
• Network permissions enforcement works as expected
• No breaking changes to existing workflows

Impact Assessment

Risk Level: Low

• This is a preventive fix for a potential code injection vulnerability
• No known exploits in production
• Generated Python scripts are executed in controlled CI/CD environments
• Fix maintains backward compatibility

Functionality: No Changes

• The fix only changes how data is escaped during template generation
• Generated Python script behavior remains identical
• All existing tests should continue to pass

Related Security Alerts

This PR fixes CodeQL alert #9. There are 8 other open code scanning alerts in the repository:

• Alert Add workflow: githubnext/agentics/weekly-research #8: Similar unsafe quoting issue in pkg/parser/frontmatter.go
• Alerts add cli flag to guard dropping a agentic workflow instructinos file #6, Weekly Research Report: AI Workflow Automation Landscape and Market Opportunities - August 2025 #7: Allocation size overflow issues
• Alerts rejig docs #1-5: Test/informational alerts

---

🤖 Generated with Claude Code

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

AI generated by Security Fix PR

[pelikhan]

@copilot investigate the test failures

[Copilot]

@pelikhan I've opened a new pull request, #1522, to work on those changes. Once the pull request is ready, I'll request review from you.

[pelikhan]

@copilot format lint, recompile

[Copilot]

@pelikhan I've opened a new pull request, #1524, to work on those changes. Once the pull request is ready, I'll request review from you.

[github-actions]

Agentic Changeset Generator triggered by this pull request