feat: ts-builder vscode-integration command#1429
Conversation
🦋 Changeset detectedLatest commit: ff9df8d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello @AStaroverov, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement to the Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new vscode-integration command to ts-builder for standardizing development environments within a monorepo. A high-severity Path Traversal vulnerability has been identified; the command's recursive scanning for packages does not correctly handle symbolic links, which could allow an attacker to write files outside the project directory via a malicious symlink. It is recommended to use lstatSync instead of statSync to mitigate this. Furthermore, high-severity issues were found with the logic for merging VSCode settings, which may not work for nested configurations, and a robustness issue in handling the current working directory that could lead to instability.
| if (statSync(fullPath).isDirectory()) { | ||
| walk(fullPath); | ||
| } |
There was a problem hiding this comment.
The walk function uses statSync which follows symbolic links. This can lead to a path traversal vulnerability if a malicious symlink is present in the repository. An attacker could trick the script into traversing directories outside the project root, and the ensurePackageConfigs function would then use process.chdir to move into those directories and write configuration files. This could allow writing files to arbitrary locations on the user's filesystem.
Remediation:
Replace statSync with lstatSync to get stats of the link itself instead of what it points to. Then, add a check to explicitly ignore symbolic links.
const stats = lstatSync(fullPath);
if (stats.isSymbolicLink()) continue;
if (stats.isDirectory()) {
walk(fullPath);
}| for (const [key, value] of Object.entries(VSCODE_SETTINGS)) { | ||
| if (!(key in settings)) { | ||
| settings[key] = value; | ||
| modified = true; | ||
| } |
There was a problem hiding this comment.
The current logic for updating VSCode settings is not robust enough for nested configurations. It only checks for the top-level key's existence. If a language-specific setting key like "[typescript]" already exists in settings.json (even as an empty object), the editor.defaultFormatter property will not be added. The logic should be changed to merge properties for object values to ensure settings are applied correctly without being destructive.
if (key.startsWith("[")) {
const currentGroup = (settings as any)[key];
if (typeof currentGroup !== "object" || currentGroup === null || Array.isArray(currentGroup)) {
(settings as any)[key] = value;
modified = true;
} else {
const valueGroup = value as Record<string, unknown>;
for (const prop in valueGroup) {
if (!(prop in currentGroup)) {
(currentGroup as Record<string, unknown>)[prop] = valueGroup[prop];
modified = true;
}
}
}
} else if (!(key in settings)) {
settings[key] = value;
modified = true;
}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1429 +/- ##
=======================================
Coverage 52.77% 52.78%
=======================================
Files 239 239
Lines 13472 13472
Branches 2787 2787
=======================================
+ Hits 7110 7111 +1
Misses 5459 5459
+ Partials 903 902 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
This pull request introduces a new CLI command to the
ts-buildertool for automating VSCode (and Cursor IDE) integration and configuration across a monorepo. The new command streamlines setup by ensuring recommended extensions, editor settings, and required dependencies are present, and by generating configuration files for all relevant packages. Additionally, a redundant oxfmt configuration file is removed.New VSCode/IDE integration feature:
vscode-integrationcommand intools/ts-builder/src/commands/vscode-integration.tsthat automates configuration of VSCode/Cursor IDE settings, extension recommendations, root dependencies, and per-package config files for all packages using@milaboratories/ts-builderin the monorepo.tools/ts-builder/src/cli.ts. [1] [2]Configuration cleanup:
.oxfmt.jsonconfig file fromtools/ts-builder, as configuration is now handled per-package by the new integration command.