Validate and sanitize string literals in runtime expression evaluation

[Copilot]

String literals in OR expressions (inputs.foo || 'default') were accepted without content validation, allowing potential injection through nested expressions, escape sequences, and zero-width characters.

Changes

String Literal Validation (isSafeExpression):

• Reject literals containing ${{ or }} markers
• Reject hex (\x), unicode (\u), and octal escape sequences that could obfuscate keywords
• Reject zero-width characters (U+200B-U+200D, U+FEFF)

Content Sanitization (evaluateExpression):

• Escape $ → \$ and { → \{ in extracted string literals

Example

// Previously allowed, now rejected:
isSafeExpression("inputs.repo || '${{ secrets.TOKEN }}'")  // → false
isSafeExpression("inputs.value || '\\x41'")                // → false
isSafeExpression("inputs.name || 'test\u200Bvalue'")       // → false

// Safe literals still work:
isSafeExpression("inputs.repo || 'default-value'")         // → true

// Extracted content is sanitized:
evaluateExpression("inputs.missing || 'test$value'")       // → "test\\$value"

Original prompt

Validate String Literal Content in runtime_imports.cjs

const isStringLiteral = /^(['"`]).*\1$/.test(rightExpr);
if (isStringLiteral) {
const contentMatch = rightExpr.match(/^(['"`])(.+)\1$/);
if (contentMatch) {
const content = contentMatch[2];

// Reject nested expressions
if (content.includes('${{') || content.includes('}}')) {
  return false;
}

// Reject escape sequences that could hide keywords
if (/\\[xu][\da-fA-F]/.test(content) || /\\[0-7]{1,3}/.test(content)) {
  return false;
}

// Reject zero-width characters
if (/[\u200B-\u200D\uFEFF]/.test(content)) {
  return false;
}

}
return true;
}
2. Escape Extracted Content

const stringLiteralMatch = rightExpr.match(/^(['"`])(.+)\1$/);
if (stringLiteralMatch) {
const content = stringLiteralMatch[2];
// Neutralize any expression markers
return content.replace(/$/g, '\$').replace(/{/g, '\{');
}

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[Copilot]

Pull request overview

This PR enhances the security of string literal validation in runtime_import.cjs by implementing content validation for string literals in OR expressions and escaping extracted literal values to prevent expression injection attacks.

Changes:

• Added validation to reject string literals containing nested expression markers (${{, }}), escape sequences (\x, \u, octal), and zero-width Unicode characters
• Added escaping of $ and { characters in extracted string literal content to neutralize expression markers
• Added comprehensive test coverage for the new validation and escaping rules

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
actions/setup/js/runtime_import.cjs	Implemented string literal content validation in isSafeExpression (lines 188-210) and content escaping in evaluateExpression (lines 257-259)
actions/setup/js/runtime_import.test.cjs	Added 38 new test cases covering nested expression rejection, escape sequence rejection, zero-width character rejection, safe literal acceptance, and content escaping

Comments suppressed due to low confidence (1)

actions/setup/js/runtime_import.cjs:190

• The regex pattern uses .+ which doesn't match empty strings, creating an inconsistency with line 187 which uses .* to detect string literals. While empty strings are correctly accepted (they skip validation and return true on line 209), this inconsistency could be confusing. Consider using (.*) instead of (.+) to make the intent explicit, even though both patterns currently produce the same result for empty strings.

      const contentMatch = rightExpr.match(/^(['"`])(.+)\1$/);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The regex pattern uses .+ which doesn't match empty string literals like '', "", or ``. This means expressions like inputs.value || '' will not extract the empty string and will instead fall through to line 268, eventually returning \${{ '' }} instead of an empty string. The pattern should use .* to correctly handle empty string literals.

This issue also appears on line 190 of the same file.

Suggested change

	// If right side is a literal, extract and return it
	const stringLiteralMatch = rightExpr.match(/^(['"`])(.+)\1$/);
	// If right side is a literal, extract and return it (including empty string literals)
	const stringLiteralMatch = rightExpr.match(/^(['"`])(.*)\1$/);

[Copilot]

Missing test coverage for empty string literals. Tests should verify that isSafeExpression("inputs.value || ''") returns true (currently passes but skips validation), and that evaluateExpression("inputs.missing || ''") correctly returns an empty string rather than a wrapped expression (currently fails due to regex bug).

[Copilot]

The test cases assume the regex will match single hex digits after \x or \u, but JavaScript escape sequences require exactly 2 hex digits for \x and exactly 4 for \u. Tests like '\\x41' and '\\u0041' have valid complete sequences, but the regex implementation /\\[xu][\da-fA-F]/ only checks for one hex digit. Consider adding tests for incomplete sequences like '\\x4' and '\\u004' to verify the intended validation behavior once the regex is corrected.

[Copilot]

The regex pattern /\\[xu][\da-fA-F]/ only matches one hex digit after \x or \u, but JavaScript escape sequences require exactly 2 hex digits for \x (e.g., \x41) and exactly 4 hex digits for \u (e.g., \u0041). The pattern should be /\\x[\da-fA-F]{2}|\\u[\da-fA-F]{4}/ to match complete hex and unicode escape sequences. As currently written, it may miss incomplete sequences or flag partial matches incorrectly.

Suggested change

	if (/\\[xu][\da-fA-F]/.test(content) || /\\[0-7]{1,3}/.test(content)) {
	if (/(?:\\x[\da-fA-F]{2}|\\u[\da-fA-F]{4})/.test(content) || /\\[0-7]{1,3}/.test(content)) {