fix: frontend environment variable type error#164
Conversation
WalkthroughVite proxy rewrite now replaces occurrences of the dynamic mock-host string (from VITE_MOCK_SERVER_HOST) with "/mock" instead of matching a fixed prefix. env.d.ts was restructured: ImportMetaEnv moved into a global augmentation and expanded with additional VITE_* keys; BUILD_TOOLS and NodeRequire declared globally and Vue i18n ComponentCustomProperties augmentation removed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
template/tinyvue/src/env.d.ts (2)
3-3: Remove unused import.The
I18nimport from 'vue-i18n' is not used anywhere in this file.🔎 Proposed fix
-import { I18n } from 'vue-i18n' -
26-28: Remove redundantdeclarekeywords insidedeclare globalblock.The
declarekeyword is redundant when declarations are already inside adeclare globalblock.🔎 Proposed fix
- declare const BUILD_TOOLS: string; - declare interface NodeRequire { + const BUILD_TOOLS: string; + interface NodeRequire { context: any; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
template/tinyvue/config/vite.config.dev.tstemplate/tinyvue/src/env.d.ts
🧰 Additional context used
🪛 ast-grep (0.40.3)
template/tinyvue/config/vite.config.dev.ts
[warning] 20-20: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${loadEnv('', process.cwd()).VITE_MOCK_SERVER_HOST})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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). (1)
- GitHub Check: e2e-test
| rewrite: (path: string) => { | ||
|
|
||
| return path.replace(new RegExp(`${loadEnv('', process.cwd()).VITE_MOCK_SERVER_HOST}`), '/mock') | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file
find . -name "vite.config.dev.ts" -type fRepository: opentiny/tiny-pro
Length of output: 105
🏁 Script executed:
# Read the file to see the actual code at the mentioned lines
cat -n template/tinyvue/config/vite.config.dev.ts | head -30Repository: opentiny/tiny-pro
Length of output: 1109
🏁 Script executed:
# Check if there are other similar patterns with loadEnv and RegExp
rg "loadEnv.*RegExp|new RegExp.*loadEnv" -A 2 -B 2Repository: opentiny/tiny-pro
Length of output: 780
🏁 Script executed:
# Check environment files to see typical values of these env vars
find . -name ".env*" -o -name "*.env" | head -10Repository: opentiny/tiny-pro
Length of output: 392
🏁 Script executed:
# Check if there are any documentation or comments about these env vars
rg "VITE_BASE_API|VITE_MOCK_SERVER_HOST" --type-list | head -20Repository: opentiny/tiny-pro
Length of output: 450
🏁 Script executed:
# Let me search for documentation or config examples
rg "VITE_BASE_API|VITE_MOCK_SERVER_HOST" -B 2 -A 2Repository: opentiny/tiny-pro
Length of output: 25975
🏁 Script executed:
# Check the actual .env file to see what values are being used
cat template/tinyvue/dev.envRepository: opentiny/tiny-pro
Length of output: 364
🏁 Script executed:
# Check if there are any other .env files that might be used
find . -path "*/tinyvue/.env*" -type f -exec cat {} \;Repository: opentiny/tiny-pro
Length of output: 390
🏁 Script executed:
# Verify that these environment variables could potentially contain special regex characters
# by checking all places where they're configured
rg "VITE_BASE_API|VITE_MOCK_SERVER_HOST" --type-list 2>/dev/null || rg "VITE_BASE_API|VITE_MOCK_SERVER_HOST" -B 1 -A 1 | grep -E "^[^:]+:[^:]*=" | head -20Repository: opentiny/tiny-pro
Length of output: 1227
🏁 Script executed:
# Let me verify the escaping pattern is correct for regex special characters
python3 << 'EOF'
import re
# Current regex metacharacters that need escaping in RegExp
special_chars = r'.*+?^${}()|[\]'
# Test what needs escaping
test_values = ['/api', '/mock', '/api.*', '/api[test]', '/api+test']
for val in test_values:
try:
# Without escaping
pattern_unescaped = val
regex_unescaped = re.compile(pattern_unescaped)
print(f"✓ '{val}' compiles as regex (unescaped)")
except re.error as e:
print(f"✗ '{val}' FAILS to compile as regex (unescaped): {e}")
# With escaping (proposed fix)
escaped = re.sub(r'[.*+?^${}()|[\]\\]', r'\\\g<0>', val)
try:
regex_escaped = re.compile(escaped)
print(f"✓ '{val}' -> '{escaped}' compiles as regex (escaped)")
except re.error as e:
print(f"✗ Escaping failed: {e}")
print()
EOFRepository: opentiny/tiny-pro
Length of output: 147
🏁 Script executed:
# Check if there are any other places in the project using similar patterns
rg "new RegExp.*loadEnv|new RegExp.*process.env" -A 2 -B 2Repository: opentiny/tiny-pro
Length of output: 780
🏁 Script executed:
# Verify the proposed fix approach is sound for JavaScript/TypeScript
cat > /tmp/test_regex_escape.js << 'EOF'
// Test the escaping approach from the review
const testValues = ['/api', '/mock', '/api.*test', '/api[123]', '/api+test'];
testValues.forEach(val => {
console.log(`\nTesting: ${val}`);
// Without escaping (current code pattern)
try {
const regex1 = new RegExp(val);
console.log(` Unescaped: OK - matches '/api' as string`);
console.log(` Test: '/api/users' matches? ${regex1.test('/api/users')}`);
} catch (e) {
console.log(` Unescaped: ERROR - ${e.message}`);
}
// With proposed escaping
const escaped = val.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
try {
const regex2 = new RegExp(escaped);
console.log(` Escaped ('${escaped}'): OK`);
console.log(` Test: '/api/users' matches? ${regex2.test('/api/users')}`);
} catch (e) {
console.log(` Escaped: ERROR - ${e.message}`);
}
});
EOF
node /tmp/test_regex_escape.jsRepository: opentiny/tiny-pro
Length of output: 862
Escape special characters in RegExp patterns constructed from environment variables.
Lines 12 and 21 create RegExp patterns from unescaped environment variables. While current values (/api, /mock) are safe, if these variables ever contain regex metacharacters, they will be interpreted as patterns rather than literal strings, causing unexpected behavior.
rewrite: (path: string) =>
path.replace(
- new RegExp(`${loadEnv('', process.cwd()).VITE_BASE_API}`),
+ new RegExp(loadEnv('', process.cwd()).VITE_BASE_API.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')),
'',
), rewrite: (path: string) => {
- return path.replace(new RegExp(`${loadEnv('', process.cwd()).VITE_MOCK_SERVER_HOST}`), '/mock')
+ const host = loadEnv('', process.cwd()).VITE_MOCK_SERVER_HOST;
+ return path.replace(new RegExp(host.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')), '/mock')
},Apply the same escaping to both locations to prevent ReDoS vulnerabilities.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rewrite: (path: string) => { | |
| return path.replace(new RegExp(`${loadEnv('', process.cwd()).VITE_MOCK_SERVER_HOST}`), '/mock') | |
| }, | |
| rewrite: (path: string) => { | |
| const host = loadEnv('', process.cwd()).VITE_MOCK_SERVER_HOST; | |
| return path.replace(new RegExp(host.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')), '/mock') | |
| }, |
🧰 Tools
🪛 ast-grep (0.40.3)
[warning] 20-20: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${loadEnv('', process.cwd()).VITE_MOCK_SERVER_HOST})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
In template/tinyvue/config/vite.config.dev.ts around lines 12 and 21, the code
builds RegExp objects directly from environment variable strings which may
contain regex metacharacters; update both sites to escape regex-special
characters from the env values before passing them to new RegExp (e.g.,
implement a small escape function that replaces [.*+?^${}()|[\]\\] with escaped
versions) and use the escaped string to construct the RegExp so the env value is
treated as a literal and avoids unexpected behavior or ReDoS risks.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
template/tinyvue/src/env.d.ts (1)
26-29: Remove redundantdeclarekeywords insidedeclare globalblock.Within a
declare globalblock, thedeclarekeyword is redundant for constants and interfaces. TypeScript already treats everything insidedeclare globalas ambient declarations.🔎 Proposed fix
} - declare const BUILD_TOOLS: string; - declare interface NodeRequire { + const BUILD_TOOLS: string; + interface NodeRequire { context: any; } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
template/tinyvue/src/env.d.ts
⏰ 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). (1)
- GitHub Check: e2e-test
🔇 Additional comments (1)
template/tinyvue/src/env.d.ts (1)
14-25: LGTM! Environment variables now correctly typed.The
ImportMetaEnvinterface correctly declares all Vite environment variables asstringtype, which properly addresses the critical type error flagged in the previous review. Line 18 specifically fixesVITE_LOWCODE_DESIGNER_ENABLEDby changing it frombooleantostring, matching Vite's runtime behavior where all environment variables are strings.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
template/tinyvue/src/env.d.ts (1)
24-27: Remove redundantdeclarekeywords insidedeclare globalblock.Inside a
declare globalblock, declarations are implicitly ambient, so thedeclarekeyword is redundant. While TypeScript accepts this syntax, removing it aligns with idiomatic TypeScript declaration patterns.🔎 Proposed refactor
- declare const BUILD_TOOLS: string; - declare interface NodeRequire { + const BUILD_TOOLS: string; + interface NodeRequire { context: any; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
template/tinyvue/src/env.d.ts
⏰ 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). (1)
- GitHub Check: e2e-test
🔇 Additional comments (1)
template/tinyvue/src/env.d.ts (1)
12-23: LGTM! Environment variable types are now correct.The
VITE_LOWCODE_DESIGNER_ENABLEDvariable is now correctly typed asstring(line 16), which resolves the type error mentioned in previous reviews. All Vite environment variables are properly typed asstring, which matches Vite's runtime behavior.
* fix: frontend environment variable type error * fix: lowcode designer enable type * refactor: remove unused import
2 participants
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.