Skip to content
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

Unsafe string replacement somewhere in `build-import-analysis #9790

Closed
7 tasks done
tcc-sejohnson opened this issue Aug 23, 2022 · 2 comments · Fixed by #11151
Closed
7 tasks done

Unsafe string replacement somewhere in `build-import-analysis #9790

tcc-sejohnson opened this issue Aug 23, 2022 · 2 comments · Fixed by #11151
Labels
duplicate This issue or pull request already exists

Comments

@tcc-sejohnson
Copy link

Describe the bug

Hey there. I'm one of the SvelteKit maintainers. First time filing an issue here.

I'm currently in the process of investigating sveltejs/kit#6102. It appears some sort of unsafe string replacement is going on somewhere in the vite:build-import-analysis plugin. Let me give a bit of background before moving on...

SvelteKit offers static environment variables from a module we call $env/static/{public|private}. For these modules (which are virtual and injected by our plugin), we generate code that looks like:

// $env/static/public
export const PUBLIC_FOO = 'asdf';
export const PUBLIC_BAR = 'jlk;';

These can be imported like so: import { PUBLIC_FOO } from '$env/static/public'. When generating the code, we load them using the Vite loadEnv function, so you're familiar with the semantics.

This has worked great so far, but it appears that if the content of one of the variables is import.meta.env, something bad happens:

// $env/static/public
export const PUBLIC_BAD = 'do NOT use import.meta.env in this string!!';

It results in these errors:

image

import.meta.env is definitely the problem, as changing it to anything else (even just import.meta.en) doesn't trigger the issue.

Steps to reproduce:

git clone https://github.com/tcc-sejohnson/kit-repro-unsafe-string-replacement
pnpm i
pnpm build

To satisfy your curiosity, you can edit the .env file to show a successful build with any value other than import.meta.env.

Reproduction

https://github.com/tcc-sejohnson/kit-repro-unsafe-string-replacement

System Info

System:
    OS: Linux 5.17 Pop!_OS 22.04 LTS
    CPU: (12) x64 AMD Ryzen 5 5600X 6-Core Processor
    Memory: 21.25 GB / 31.32 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 18.7.0 - ~/.nvm/versions/node/v18.7.0/bin/node
    Yarn: 1.22.17 - ~/.yarn/bin/yarn
    npm: 8.15.0 - ~/.nvm/versions/node/v18.7.0/bin/npm
  Browsers:
    Chrome: 104.0.5112.79
    Chromium: 83.0.4103.116
    Firefox: 101.0.1
  npmPackages:
    vite: ^3.0.9 => 3.0.9

Used Package Manager

pnpm

Logs

vite v3.0.9 building for production...
✓ 4 modules transformed.
[vite:build-import-analysis] Cannot destructure property 'line' of 'locate(...)' as it is undefined.
file: $env/static/public
error during build:
TypeError: Cannot destructure property 'line' of 'locate(...)' as it is undefined.
at augmentCodeLocation (file:///home/elliottjohnson/dev/github/sveltejs/repros/chungus-amongus/node_modules/.pnpm/rollup@2.77.3/node_modules/rollup/dist/es/shared/rollup.js:1868:17)
at Object.error (file:///home/elliottjohnson/dev/github/sveltejs/repros/chungus-amongus/node_modules/.pnpm/rollup@2.77.3/node_modules/rollup/dist/es/shared/rollup.js:21938:25)
at Object.transform (file:///home/elliottjohnson/dev/github/sveltejs/repros/chungus-amongus/node_modules/.pnpm/vite@3.0.9/node_modules/vite/dist/node/chunks/dep-0fc8e132.js:40776:22)
 ELIFECYCLE  Command failed with exit code 1.

Validations

@tony19
Copy link
Contributor

tony19 commented Aug 23, 2022

I think the problem is in the vite:define plugin, whose regex pattern matches import.meta.env even inside quotes.

A possible solution is to strip the literals from the code before testing the string for a match:

import { stripLiteral } from 'strip-literal'
//...
code = stripLiteral(code)
while ((match = pattern.exec(code))) {

@sapphi-red
Copy link
Member

Duplicate of #3304

@sapphi-red sapphi-red marked this as a duplicate of #3304 Oct 17, 2022
@sapphi-red sapphi-red closed this as not planned Won't fix, can't repro, duplicate, stale Oct 17, 2022
@sapphi-red sapphi-red added duplicate This issue or pull request already exists and removed pending triage labels Oct 17, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Nov 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
duplicate This issue or pull request already exists
Projects
None yet
3 participants