-
Notifications
You must be signed in to change notification settings - Fork 465
fix: replace ejs
with handlebars
due to security issue
#4970
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
Conversation
WalkthroughA new GitHub Actions workflow for CodeQL auditing was added. The project’s template engine was switched from EJS to Handlebars, with all references updated accordingly. The EJS and its type definitions were removed from dependencies, replaced by Handlebars equivalents in both runtime and development dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI_User as CLI User
participant CLI as CLI (init.ts)
participant Handlebars as Handlebars Engine
CLI_User->>CLI: Run config init
CLI->>Handlebars: Compile template with data
Handlebars-->>CLI: Rendered config content
CLI->>CLI_User: Output generated config file
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
package.json (1)
54-62
: Consider peer-pinning for downstream template engines.You added
handlebars
as a hard dependency. Because end-users rarely import it directly, bumping it is a risk-free semver-patch. You might loosen the caret version to~4.7.8
or leave as caret but add anengines
entry if you plan to move to v5 later.src/cli/config/init.ts (1)
106-112
: Optional: externalise template for readability.Maintaining 20-line template literals inline is brittle. Moving
JEST_CONFIG_TEMPLATE.hbs
tosrc/cli/templates/
would:• enable syntax-highlighting,
• ease future edits without TS recompile,
• allow pre-compilation if start-up perf matters..github/workflows/audit.yml (1)
11-14
: Path ignore list misses.test.ts
/.test.js
.If unit tests follow
*.test.ts[x]?
naming, CodeQL jobs will still run for doc-only changes. Add them if you want to save CI minutes.- - '**/*.spec.ts' + - '**/*.spec.ts' + - '**/*.test.ts' + - '**/*.test.js'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (3)
.github/workflows/audit.yml
(1 hunks)package.json
(2 hunks)src/cli/config/init.ts
(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ahnpnl
PR: kulshekhar/ts-jest#4950
File: .github/workflows/pr-docs-check.yml:19-21
Timestamp: 2025-07-21T14:13:24.979Z
Learning: In ts-jest repository, adding `pull-requests: write` permission to the pr-docs-check.yml workflow's deploy-preview job breaks the GitHub Pages deployment, so this permission should be avoided in this specific workflow.
📚 Learning: in the ts-jest project, dependencies are only installed at the root level using the root package-loc...
Learnt from: ahnpnl
PR: kulshekhar/ts-jest#4950
File: .github/actions/setup-workspace/action.yml:28-28
Timestamp: 2025-07-21T14:09:03.234Z
Learning: In the ts-jest project, dependencies are only installed at the root level using the root package-lock.json file. Subdirectory package-lock.json files in e2e/ and examples/ directories don't affect the main workspace dependency installation, so they should not be included in GitHub Actions cache keys for the main workspace.
Applied to files:
package.json
src/cli/config/init.ts
📚 Learning: in ts-jest repository, adding `pull-requests: write` permission to the pr-docs-check.yml workflow's ...
Learnt from: ahnpnl
PR: kulshekhar/ts-jest#4950
File: .github/workflows/pr-docs-check.yml:19-21
Timestamp: 2025-07-21T14:13:24.979Z
Learning: In ts-jest repository, adding `pull-requests: write` permission to the pr-docs-check.yml workflow's deploy-preview job breaks the GitHub Pages deployment, so this permission should be avoided in this specific workflow.
Applied to files:
.github/workflows/audit.yml
🔇 Additional comments (5)
package.json (1)
54-62
: Handlebars dependency verified – no outstanding CVEs
npm audit
reports 0 vulnerabilities for [email protected]npm view handlebars version
confirms 4.7.8 is the latest releaseNo further action needed.
src/cli/config/init.ts (2)
17-23
: Unescaped triple-stash may leak raw strings.
{{{ transformOpts }}}
outputs unescaped text. It is safe today because the source is machine-generated JSON, but if future options include user-provided paths it could theoretically inject arbitrary code into the generated config. Consider{{ transformOpts }}
plus pre-escaping withJSON.stringify
to keep control.
106-112
: All EJS references removedI searched the codebase for any remaining EJS usage—imports, requires,
.ejs
templates, or a dependency entry in package.json—and found none. It’s safe to remove EJS entirely.
- No
import
orrequire
statements referencing “ejs”- No
.ejs
template files in the repo- No
"ejs"
entry in package.json.github/workflows/audit.yml (2)
20-28
: Workflow permissions are minimal – nice.
security-events: write
plus read-only scopes follow GitHub’s recommendations. Nopull-requests: write
, so you avoided the pages deployment issue noted earlier. 👍
35-39
: Shallow fetch depth set to 20 – confirm CodeQL accuracy.CodeQL uses commit history for data-flow. Depth=20 is usually fine, but large refactors may exceed this window. If you see “history truncated” warnings, bump or drop the setting.
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/audit.yml (3)
11-14
: Broaden the test-file exclusions to avoid unnecessary CI runs
Only*.spec.ts
files are ignored. The repository also contains.test.ts
and possibly.spec.js
/.test.js
syllables that will still trigger the workflow even though they are pure test changes. Consider expanding the glob to:paths-ignore: - '**/*.md' - '**/*.txt' - '**/*.{spec,test}.{ts,js}'
35-39
: Considerfetch-depth: 0
for more accurate CodeQL analysis
Limiting history to the last 20 commits may hide taint-tracking flows or cause baseline-diff noise. CodeQL’s guidance is to fetch the full history unless the repository is huge.- fetch-depth: 20 + fetch-depth: 0 # full history for precise data-flow analysis
61-66
: Redundant lockfile arguments
-r ./
already scans all nestedpackage-lock.json
files. Explicitly listing them is harmless but unnecessary; dropping them simplifies the command.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/audit.yml
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ahnpnl
PR: kulshekhar/ts-jest#4950
File: .github/workflows/pr-docs-check.yml:19-21
Timestamp: 2025-07-21T14:13:24.979Z
Learning: In ts-jest repository, adding `pull-requests: write` permission to the pr-docs-check.yml workflow's deploy-preview job breaks the GitHub Pages deployment, so this permission should be avoided in this specific workflow.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test-windows (18) / Run tests on Node v18 on windows-latest
- GitHub Check: test-windows (22) / Run tests on Node v22 on windows-latest
- GitHub Check: test-windows (20) / Run tests on Node v20 on windows-latest
- GitHub Check: test-ubuntu (22) / Run tests on Node v22 on ubuntu-latest
- GitHub Check: test-ubuntu (20) / Run tests on Node v20 on ubuntu-latest
- GitHub Check: test-ubuntu (18) / Run tests on Node v18 on ubuntu-latest
🔇 Additional comments (1)
.github/workflows/audit.yml (1)
20-27
:security-events: write
will fail on PRs from forks
Forked PRs have no permission to write security events, causing the job to exit with HTTP 403. Guard the permission with a conditional or use:permissions: read-allfor PRs, and elevate only on
push
to the default branches.
|
Summary
Fixes #4969
Test plan
Green CI
Does this PR introduce a breaking change?
Other information
N.A.
Summary by CodeRabbit
Chores
New Features