Skip to content

fix(shared): escape backslashes before quotes in template preprocessor#1866

Merged
LeighFinegold merged 2 commits intofinos:mainfrom
rocketstack-matt:fix/shared-incomplete-string-escaping
Nov 27, 2025
Merged

fix(shared): escape backslashes before quotes in template preprocessor#1866
LeighFinegold merged 2 commits intofinos:mainfrom
rocketstack-matt:fix/shared-incomplete-string-escaping

Conversation

@rocketstack-matt
Copy link
Member

Description

Fixes CodeQL alert for incomplete string escaping (CWE-116). The path escaping now escapes backslash characters before double quotes to prevent bypass via backslash-escaped quotes.

https://github.com/finos/architecture-as-code/security/code-scanning/48

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🎨 Code style/formatting changes
  • ♻️ Refactoring (no functional changes)
  • ⚡ Performance improvements
  • ✅ Test additions or updates
  • 🔧 Chore (maintenance, dependencies, CI, etc.)

Affected Components

  • CLI (cli/)
  • Shared (shared/)
  • CALM Widgets (calm-widgets/)
  • CALM Hub (calm-hub/)
  • CALM Hub UI (calm-hub-ui/)
  • Documentation (docs/)
  • VS Code Extension (calm-plugins/vscode/)
  • Dependencies
  • CI/CD

Commit Message Format ✅

Testing

  • I have tested my changes locally
  • I have added/updated unit tests
  • All existing tests pass

Checklist

  • My commits follow the conventional commit format
  • I have updated documentation if necessary
  • I have added tests for my changes (if applicable)
  • My changes follow the project's coding standards

Fixes CodeQL alert for incomplete string escaping (CWE-116). The path
escaping now escapes backslash characters before double quotes to prevent
bypass via backslash-escaped quotes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a security vulnerability (CWE-116: Incomplete String Escaping) in the template preprocessor by ensuring backslashes are escaped before quotes when preparing path strings for injection into Handlebars templates. The fix prevents bypass attacks where an attacker could use \ to escape the escaping quote itself (e.g., foo\" becoming foo\\" which breaks the string boundary).

Key Changes

  • Modified the safePath escaping logic to escape backslashes first, then quotes, ensuring proper string sanitization
  • Maintains correct escaping order: replace(/\\/g, '\\\\') before replace(/"/g, '\\"')

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

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

}

const safePath = path.replace(/"/g, '\\"');
const safePath = path.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

This security fix for CWE-116 (incomplete string escaping) lacks test coverage. Consider adding a test case that verifies paths containing backslashes are properly escaped. For example:

it('escapes backslashes in paths to prevent quote bypass', () => {
    const input = '{{list foo\\bar}}';
    const output = TemplatePreprocessor.preprocessTemplate(input);
    expect(output).toBe('{{list (convertFromDotNotation this "foo\\\\bar")}}');
});

it('escapes backslashes before quotes in paths', () => {
    const input = '{{nodes[\\"test\\"]}}';
    const output = TemplatePreprocessor.preprocessTemplate(input);
    expect(output).toBe('{{convertFromDotNotation this "nodes[\\\\\\"test\\\\\\"]"}}');
});

Without tests, it's difficult to verify the fix works correctly and prevent regression in the future.

Copilot uses AI. Check for mistakes.
@LeighFinegold LeighFinegold merged commit 5548fa6 into finos:main Nov 27, 2025
13 checks passed
@rocketstack-matt rocketstack-matt deleted the fix/shared-incomplete-string-escaping branch December 8, 2025 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants