Skip to content

Conversation

@github-actions
Copy link
Contributor

Security Fix: Potentially Unsafe Quoting

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

Vulnerability Description

The code was using Go's %q format specifier to quote JSON data containing project views configuration before passing it via the GH_AW_PROJECT_VIEWS environment variable. While %q provides backslash escaping, it doesn't fully protect against quote injection attacks if the value is later used in shell commands, SQL queries, or other contexts where quotes have special meaning.

CodeQL flagged this as a critical security issue because if the JSON value contains a single quote, it could break out of the enclosing quotes in downstream processing, potentially enabling command injection or SQL injection attacks.

Location

  • File: pkg/workflow/update_project_job.go
  • Line: 48

Fix Applied

Replaced %q string formatting with base64 encoding for the JSON data. This approach:

  1. Eliminates quote-escaping vulnerabilities: Base64-encoded strings contain only alphanumeric characters and safe symbols (+, /, =), making it impossible for special characters like quotes to break out of string contexts
  2. Provides defense-in-depth: Even if the value is used unsafely downstream, base64 encoding prevents injection attacks
  3. Follows security best practices: Base64 encoding is the industry-standard approach for passing arbitrary data through environment variables

Changes Made:

  • Added encoding/base64 import
  • Replaced fmt.Sprintf("... %q\n", string(viewsJSON)) with base64 encoding
  • Updated comment to explain the security rationale for using base64 encoding
  • Added comprehensive documentation about preventing quote-breaking vulnerabilities

Before:

// Use %q to properly quote and escape the JSON for YAML
customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_PROJECT_VIEWS: %q\n", string(viewsJSON)))

After:

// Encode JSON as base64 to avoid quote-escaping vulnerabilities
// Base64 encoding ensures the value contains no special characters that could break out
// of quotes in downstream processing (shell commands, SQL queries, etc.)
viewsBase64 := base64.StdEncoding.EncodeToString(viewsJSON)
customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_PROJECT_VIEWS: %s\n", viewsBase64))

Security Best Practices

This fix implements defense-in-depth security by:

  1. Preventing injection attacks: Base64 encoding makes it impossible for malicious JSON content to inject commands or break SQL queries
  2. Eliminating quote-escaping complexity: No need to worry about properly escaping single quotes, double quotes, backslashes, etc.
  3. Providing a safe transport mechanism: Base64 is designed specifically for safely transmitting binary data through text-based channels

Testing Considerations

  • No breaking changes: The GH_AW_PROJECT_VIEWS environment variable is not currently consumed by JavaScript code, so this change has no immediate impact
  • Future JavaScript implementation: When JavaScript code is added to read this variable, it will need to base64-decode the value using Buffer.from(process.env.GH_AW_PROJECT_VIEWS, 'base64').toString() before parsing as JSON
  • CI validation: All existing tests should pass as this is a preventive security fix

References

  • [OWASP Command Injection]((redacted)
  • [CWE-78: Improper Neutralization of Special Elements in OS Commands]((redacted)
  • [CWE-89: Improper Neutralization of Special Elements in SQL Commands]((redacted)
  • [CWE-94: Improper Control of Generation of Code]((redacted)

Automated by: Code Scanning Fixer Workflow
Run ID: 21267994348

AI generated by Code Scanning Fixer

…ection

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

**Security Fix**: Replace %q string formatting with base64 encoding for JSON
data passed via environment variables to eliminate quote-escaping vulnerabilities.

**Root Cause**: The previous implementation used Go's %q format specifier to
quote JSON data containing project views configuration. While %q provides
backslash escaping, it doesn't fully protect against quote injection if the
value is used unsafely in downstream shell commands or SQL queries.

**Solution**: Encode the JSON data as base64 before passing it via the
GH_AW_PROJECT_VIEWS environment variable. Base64 encoding ensures the value
contains only alphanumeric characters and safe symbols (+, /, =), completely
eliminating the risk of quote-breaking characters.

**Impact**: This is a preventive fix - the environment variable is not
currently consumed by JavaScript code, so there is no breaking change. When
the JavaScript code is implemented to read this variable, it will need to
base64-decode the value before parsing as JSON.

**Security Best Practice**: Base64 encoding is the most robust solution for
passing arbitrary data through environment variables, as recommended by
security experts for preventing injection attacks.

Related: CWE-78, CWE-89, CWE-94
// Base64 encoding ensures the value contains no special characters that could break out
// of quotes in downstream processing (shell commands, SQL queries, etc.)
viewsBase64 := base64.StdEncoding.EncodeToString(viewsJSON)
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PROJECT_VIEWS: %s\n", viewsBase64))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot this is not a shell script, this is YAML. THERE IS NO SHELL INJECTION. Add a comment to disable codeql.

Copy link
Contributor

Copilot AI commented Jan 22, 2026

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

Copilot AI added a commit that referenced this pull request Jan 22, 2026
…ssion

Replace unnecessary base64 encoding with standard %q formatting for
GH_AW_PROJECT_VIEWS environment variable. The original security concern
was invalid - this code generates YAML environment variable declarations,
not shell scripts, so there is no shell injection risk.

Added lgtm[go/unsafe-quoting] suppression comment explaining that %q is
safe in this context because the value is set as a YAML environment
variable, not executed as shell code.

Addresses review feedback in #11401.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…#11402)

* Initial plan

* fix: replace base64 encoding with %q formatting and add CodeQL suppression

Replace unnecessary base64 encoding with standard %q formatting for
GH_AW_PROJECT_VIEWS environment variable. The original security concern
was invalid - this code generates YAML environment variable declarations,
not shell scripts, so there is no shell injection risk.

Added lgtm[go/unsafe-quoting] suppression comment explaining that %q is
safe in this context because the value is set as a YAML environment
variable, not executed as shell code.

Addresses review feedback in #11401.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review January 22, 2026 23:23
@pelikhan pelikhan merged commit 8b92ac3 into main Jan 22, 2026
@pelikhan pelikhan deleted the fix/code-scanning-alert-538-unsafe-quoting-base64-ce1c0eb9e9558c3c branch January 22, 2026 23:23
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