Skip to content

Conversation

@github-actions
Copy link
Contributor

Security Fix: Potentially Unsafe Quoting in Project Views Configuration

Alert Number: #538
Severity: Critical
Rule: go/unsafe-quoting
CWE: CWE-78 (OS Command Injection), CWE-89 (SQL Injection), CWE-94 (Code Injection)

Vulnerability Description

CodeQL identified a critical security issue where JSON data containing project views configuration was being directly embedded into a YAML environment variable declaration using Go's %q format specifier. While %q escapes characters for Go string literals, it doesn't eliminate the risk of quote-based injection attacks when the resulting YAML is processed.

The vulnerable code constructed a quoted string from potentially untrusted JSON data:

customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_PROJECT_VIEWS: %q\n", string(viewsJSON)))

If the JSON contained certain quote characters, they could potentially break out of the enclosing quotes and alter the structure of the YAML configuration, leading to injection vulnerabilities.

Location

  • File: pkg/workflow/update_project_job.go
  • Line: 47-51 (before fix)

Fix Applied

The fix uses base64 encoding to safely pass JSON data through YAML without any quoting concerns. Base64 encoding eliminates all special characters that could break YAML syntax.

Changes Made:

  • Added encoding/base64 import
  • Changed environment variable from GH_AW_PROJECT_VIEWS to GH_AW_PROJECT_VIEWS_BASE64 to indicate the encoding
  • Base64-encode the JSON data before embedding it in YAML
  • Removed the vulnerable direct string embedding
  • Removed the suppression comment that claimed the code was safe

Code Change:

// Before (vulnerable):
customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_PROJECT_VIEWS: %q\n", string(viewsJSON)))

// After (secure):
viewsBase64 := base64.StdEncoding.EncodeToString(viewsJSON)
customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_PROJECT_VIEWS_BASE64: %q\n", viewsBase64))

Security Best Practices

This fix follows several security best practices:

  1. Avoid String Interpolation for Structured Data: Never embed structured data (JSON, XML) directly into quoted strings
  2. Use Safe Encoding: Base64 encoding eliminates all special characters that could cause injection
  3. Defense in Depth: Even though the data comes from workflow configuration (trusted source), we treat it as potentially untrusted
  4. Clear Indication of Encoding: The new variable name GH_AW_PROJECT_VIEWS_BASE64 makes it clear that consumers must decode the value

Testing Considerations

Note: The environment variable GH_AW_PROJECT_VIEWS_BASE64 (or the previous GH_AW_PROJECT_VIEWS) is not currently consumed by any JavaScript code in the repository. This fix is preventative.

If this variable is used in the future, the consuming code should:

  1. Read process.env.GH_AW_PROJECT_VIEWS_BASE64
  2. Decode from base64: Buffer.from(viewsBase64, 'base64').toString('utf-8')
  3. Parse the JSON: JSON.parse(decodedString)

No breaking changes: Since the variable is not currently used, this change has no impact on existing functionality.

Why Previous Fixes Failed

This is the 7th attempt to fix alert #538. Previous attempts likely failed because:

  1. They may have tried to escape quotes manually (complex and error-prone)
  2. They may have used different quoting mechanisms (still vulnerable)
  3. They may have added suppression comments without actually fixing the root cause

This fix takes a different approach by eliminating the quoting problem entirely through base64 encoding, which is a proven secure method for passing binary or structured data through text-based configuration formats.


Automated by: Code Scanning Fixer Workflow
Run ID: 21272238432
Workflow: Alert #538 has been open since 2026-01-15 and had 6 previous fix attempts

AI generated by Code Scanning Fixer

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