Skip to content

Conversation

@github-actions
Copy link
Contributor

Security Fix: Unsafe JSON Quoting in YAML Environment Variable

Alert Number: #538
Severity: Critical
Rule: go/unsafe-quoting
CWE: CWE-78, CWE-89, CWE-94

Vulnerability Description

CodeQL detected unsafe quoting in pkg/workflow/update_project_job.go at line 47. The code was embedding JSON data into a YAML environment variable using fmt.Sprintf with %q formatting:

viewsJSON, err := json.Marshal(data.SafeOutputs.UpdateProjects.Views)
customEnvVars = append(customEnvVars, fmt.Sprintf("GH_AW_PROJECT_VIEWS: %q\n", string(viewsJSON)))

JSON encoding does not escape single quotes, which could break YAML parsing or be exploited for injection attacks if the JSON contains malicious quote characters.

Location

  • File: pkg/workflow/update_project_job.go
  • Line: 47
  • Function: buildUpdateProjectJob

Root Cause Analysis

The vulnerability existed in code that created an unused environment variable GH_AW_PROJECT_VIEWS. Investigation revealed:

  1. Dead Code: The GH_AW_PROJECT_VIEWS environment variable was never consumed by any JavaScript code
  2. Redundant: Views configuration is already properly passed via GH_AW_SAFE_OUTPUTS_PROJECT_HANDLER_CONFIG (see compiler_safe_outputs_config.go:602-608)
  3. Proper Implementation Exists: The working implementation uses JSON marshaling with proper %q escaping at line 638

Fix Applied

Removed the unused code entirely (lines 42-52 in update_project_job.go):

Changes Made:

  • ✅ Removed unused GH_AW_PROJECT_VIEWS environment variable generation
  • ✅ Removed unused encoding/json import
  • ✅ Eliminated unsafe quoting vulnerability
  • ✅ No functional impact since the code was never executed/consumed

Why This Fix is Correct:

  1. Eliminates the vulnerability by removing the unsafe quoting code
  2. No functionality lost - the environment variable was never read
  3. Proper mechanism exists - views are passed through GH_AW_SAFE_OUTPUTS_PROJECT_HANDLER_CONFIG
  4. Cleaner codebase - removes dead code that caused security alerts

Security Best Practices

  • Dead Code Removal: Unused code can introduce security vulnerabilities and should be removed
  • Proper Escaping: When embedding dynamic data in YAML, use proper escaping mechanisms
  • Centralized Configuration: The project correctly uses a centralized handler config system (GH_AW_SAFE_OUTPUTS_PROJECT_HANDLER_CONFIG) instead of scattered environment variables

Testing Considerations

  • ✅ No behavioral changes - removed code was never executed
  • ✅ Views configuration still works via GH_AW_SAFE_OUTPUTS_PROJECT_HANDLER_CONFIG
  • ✅ Existing update_project tests should pass without modification
  • ✅ CodeQL should no longer flag this location

Verification

After merging, verify:

  1. CodeQL alert [Custom Engine Test] Test Pull Request - Custom Engine Safe Output #538 is resolved
  2. Update project functionality works as expected
  3. Views configuration continues to work via the handler config

Automated by: Code Scanning Fixer Workflow
Run ID: 21271253990
Commit: 315dfd1

AI generated by Code Scanning Fixer

…oting

Fixes code scanning alert #538 (go/unsafe-quoting)

The code was creating a GH_AW_PROJECT_VIEWS environment variable with
JSON data embedded using %q, which CodeQL flagged as unsafe quoting.
However, this environment variable was never consumed by any JavaScript
code - it was dead code.

The actual views configuration is properly passed through the
GH_AW_SAFE_OUTPUTS_PROJECT_HANDLER_CONFIG environment variable
(see compiler_safe_outputs_config.go:602-608), which uses proper
JSON marshaling and %q escaping on line 638.

This fix removes the unused code entirely, eliminating the security
vulnerability without affecting functionality.

CWE-78, CWE-89, CWE-94
@pelikhan
Copy link
Contributor

@copilot the string is inserted in YAML not shell.

Copy link
Contributor

Copilot AI commented Jan 23, 2026

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants