Skip to content

fix(dev-env): prevent non-safelisted EJS execution in repo config files#2938

Merged
rebeccahum merged 3 commits into
trunkfrom
fix/render-ejs-nope
Jul 6, 2026
Merged

fix(dev-env): prevent non-safelisted EJS execution in repo config files#2938
rebeccahum merged 3 commits into
trunkfrom
fix/render-ejs-nope

Conversation

@rebeccahum

@rebeccahum rebeccahum commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Description

This pull request updates the handling of template configuration files in the dev environment to restrict usage of EJS syntax, allowing only the <%= configDir %> placeholder. This change improves security and predictability by preventing arbitrary JavaScript execution in configuration templates.

Changelog Description

Changed

  • Dev-env: Limit vip-dev-env.yml.ejs to plain YAML with <%= configDir %> substitution.

Pull request checklist

New release checklist

Steps to Test

Outline the steps to test and verify the PR here.

Example:

  1. Check out PR.
  2. Run npm run build
  3. Run ./dist/bin/vip-cookies.js nom
  4. Verify cookies are delicious.

@rebeccahum rebeccahum requested a review from Copilot July 6, 2026 14:49
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 hardens dev-env repository configuration handling by removing full EJS rendering and restricting template processing to a safelisted configDir placeholder, reducing the risk of executing arbitrary template code when loading vip-dev-env.yml.ejs.

Changes:

  • Replaced ejs.render() with a strict <%= configDir %> string substitution for template config files.
  • Added validation to reject remaining EJS template syntax after substitution.
  • Added Jest tests covering placeholder substitution and rejection of arbitrary EJS usage.

Reviewed changes

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

File Description
src/lib/dev-environment/dev-environment-configuration-file.ts Removes full EJS rendering and enforces a safelisted placeholder-only template mechanism.
tests/lib/dev-environment/dev-environment-configuration-file.js Adds tests to verify allowed placeholder replacement and rejection of non-safelisted EJS syntax.

Comment thread src/lib/dev-environment/dev-environment-configuration-file.ts Outdated
Comment on lines +25 to +26
const ALLOWED_CONFIG_DIR_TEMPLATE = /<%=\s*configDir\s*%>/g;
const EJS_TEMPLATE_TAG = /<%[\s\S]*?%>/;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

n/a because it won't execute

@rebeccahum rebeccahum force-pushed the fix/render-ejs-nope branch from 943b53e to 23ada79 Compare July 6, 2026 15:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread src/lib/dev-environment/dev-environment-configuration-file.ts
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@sonarqubecloud

sonarqubecloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

@rebeccahum rebeccahum requested a review from a team July 6, 2026 15:14
@rebeccahum rebeccahum merged commit 8ebaaf3 into trunk Jul 6, 2026
20 checks passed
@rebeccahum rebeccahum deleted the fix/render-ejs-nope branch July 6, 2026 17:56
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.

3 participants