Move sharedtokens #2350
Move sharedtokens #2350
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| function bootstrap() { | ||
| execSync(path.resolve('scripts/clean.js'), opts) |
Check warning
Code scanning / CodeQL
Shell command built from environment values
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 20 days ago
In general, to fix this problem you should not embed dynamic paths or other environment-derived values directly into a shell command string. Instead, invoke the target program in a way that bypasses shell interpretation, by either (a) using execFileSync with a command and an array of arguments, or (b) using fork (for Node scripts). This ensures that paths containing spaces or special characters are passed as literal arguments, not parsed by the shell.
For this specific script, the problematic line is:
execSync(path.resolve('scripts/clean.js'), opts)This runs a shell and asks it to execute whatever the resolved path string is, which is both unnecessary and brittle. The best fix that preserves existing functionality is to invoke the Node interpreter directly and pass the resolved script path as an argument, using execFileSync from child_process. We already import from child_process, so we can extend the destructuring to include execFileSync, and then change the call on line 68 to:
execFileSync(process.execPath, [path.resolve('scripts/clean.js')], opts)Here:
process.execPathis the absolute path to the Node executable running this script.- The Node script path is passed as a separate argument (in an array), so no shell is involved.
- We continue to use the same
opts({ stdio: 'inherit' }) so output behavior stays the same.
No other behavior in buildProject or bootstrap changes, and no new external dependencies are required.
| @@ -24,7 +24,7 @@ | ||
| * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
| * SOFTWARE. | ||
| */ | ||
| const { execSync, fork } = require('child_process') | ||
| const { execSync, execFileSync, fork } = require('child_process') | ||
| const path = require('path') | ||
|
|
||
| const opts = { stdio: 'inherit' } | ||
| @@ -65,7 +65,7 @@ | ||
| } | ||
|
|
||
| function bootstrap() { | ||
| execSync(path.resolve('scripts/clean.js'), opts) | ||
| execFileSync(process.execPath, [path.resolve('scripts/clean.js')], opts) | ||
| buildProject() | ||
| } | ||
|
|
|
…ost level of the themes and its type to the common theTypes INSTUI-4887
HerrTopi
force-pushed
the
move-sharedtokens
branch
from
6a0ec8d to
11cb993
Compare
matyasf
left a comment
matyasf
left a comment
There was a problem hiding this comment.
Looks good, I tested it with the components that use the shared tokens and found no issues
This should re-arrange the internals of the themes, but keep the interface mostly intact.
sharedTokens should go the same level as primitives and semantics are, its type should be in the commonTypes