-
Notifications
You must be signed in to change notification settings - Fork 36
[code-scanning-fix] Fix go/unsafe-quoting: Escape single quotes in JSON data #11382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[code-scanning-fix] Fix go/unsafe-quoting: Escape single quotes in JSON data #11382
Conversation
- Add proper shell escaping for JSON data embedded in environment variables - Prevents potential command injection via single quotes in user data - Fixes CodeQL alert #538 (Critical severity) - Uses POSIX-compatible '\''escaping technique Security: Addresses CWE-78, CWE-89, CWE-94
pkg/workflow/update_project_job.go
Outdated
| return nil, fmt.Errorf("failed to marshal views configuration: %w", err) | ||
| } | ||
| customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PROJECT_VIEWS: '%s'\n", string(viewsJSON))) | ||
| // Escape single quotes in JSON to prevent shell injection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot we are writing a value in a yaml dictionary. The ' ' are not stricly needed. Encode viewsJSON using the YAML Marshal encoder insteead.
* Initial plan * Use %q for JSON escaping in YAML dictionary values Replaced manual single-quote escaping with Go's %q format specifier, which: - Automatically handles all special characters and escaping - Produces cleaner YAML with double quotes instead of single quotes - Follows existing codebase patterns (e.g., add_comment.go) - Simplifies code by removing manual string replacement logic This addresses the feedback to use simpler escaping for JSON data in YAML dictionary values. 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>
|
@copilot recompile |
…ing (#11392) * Initial plan * Recompile workflows after security fix 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>
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🎉 Yo ho ho! Changeset Generator found the treasure and completed successfully! ⚓💰 |
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Overall Status: PASS cc
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
GitHub MCP (last 2 merged PRs): ✅ Replace manual quote escaping with %q format specifier for YAML encoding; Replace manual quote escaping with %q format specifier
|
Smoke Test: PASSRetrieved PRs:
Test Results: ✅ GitHub MCP | ✅ Safe Inputs GH CLI | ✅ Serena MCP | ✅ Playwright | ✅ Tavily | ✅ File Write | ✅ Bash Overall Status: PASS
|
…2-00bd24b6b438b87e
Security Fix: 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
The code was constructing a shell environment variable by embedding JSON data directly into a single-quoted string without proper escaping. If the JSON data contained a single quote character, it could break out of the enclosing quotes and potentially allow command injection.
Vulnerable Code (Line 47):
If
viewsJSONcontains the character', it would prematurely close the string literal and potentially execute arbitrary shell commands.Location
pkg/workflow/update_project_job.gobuildUpdateProjectJobFix Applied
Added proper shell escaping for single quotes before embedding the JSON data into the environment variable string. The fix uses POSIX-compatible shell escaping:
\→\\(prevents interference with quote escaping)'→'\''(ends the quoted string, adds escaped quote, starts new quoted string)Changes Made:
stringsimport for string manipulation'\''(POSIX shell escape sequence)viewsJSONbefore embedding in the format stringSecure Code (Lines 48-52):
Security Best Practices
This fix follows CodeQL's recommendations for handling potentially untrusted data:
'\''technique works across all POSIX-compliant shells (bash, sh, zsh, etc.)Testing Considerations
Automated by: Code Scanning Fixer Workflow
Run ID: 21263052463
Changeset
go/unsafe-quoting).