Skip to content

Commit 2b93731

Browse files
committed
fix: address Cursor Bot security and correctness issues
Fix critical bugs identified by Cursor Bot code review: 1. **Missing body tag handling in Vite plugin** - Added check for missing </body> tag (lastIndexOf returns -1) - Prevents malformed HTML with script at wrong position - Gracefully appends script at end if body tag not found - Added debug logging for missing body tag case 2. **Contradictory loading content suggestion** - Fixed confusing suggestion for loading selectors - Now suggests waiting for loading to disappear THEN get content - Provides alternative to wait for API request - Clarifies when user actually needs the loading element itself 3. **Unescaped quotes in generated code suggestions** - All selectors now properly escaped before interpolation - Prevents syntax errors like: cy.get('[data-test='value']') - Correctly generates: cy.get('[data-test=\\'value\\']') - Applied to all suggestion types (dynamic, complex, ID, general) Test coverage: - Added tests for quote escaping in selectors - Verified proper handling of special characters - All 19 tests passing - Lint checks passing Before: - cy.get('[data-test='value']') // SYNTAX ERROR After: - cy.get('[data-test=\\'value\\']') // VALID Related: Security and code quality improvements
1 parent 50d5920 commit 2b93731

File tree

3 files changed

+58
-10
lines changed

3 files changed

+58
-10
lines changed

npm/vite-dev-server/src/plugins/cypress.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,21 @@ export const Cypress = (
126126
const loader = await getLoaderPromise()
127127

128128
// insert the script in the end of the body
129-
const newHtml = `
129+
let newHtml: string
130+
131+
if (endOfBody === -1) {
132+
// No closing body tag found, append at the end
133+
debug('No closing body tag found, appending script at end of HTML')
134+
newHtml = `${indexHtmlContent}
135+
<script>${loader}</script>`
136+
} else {
137+
// Insert before closing body tag
138+
newHtml = `
130139
${indexHtmlContent.substring(0, endOfBody)}
131140
<script>${loader}</script>
132141
${indexHtmlContent.substring(endOfBody)}
133142
`
143+
}
134144

135145
return newHtml
136146
},

packages/driver/src/cypress/timeout_diagnostics.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,38 +78,47 @@ export class TimeoutDiagnostics {
7878

7979
// Check for dynamic content indicators
8080
if (this.COMMON_PATTERNS.dynamicContent.test(selector)) {
81+
const escapedSelector = selector.replace(/'/g, "\\'");
82+
8183
suggestions.push({
82-
reason: 'The selector appears to target dynamic/loading content',
84+
reason: 'The selector appears to target dynamic/loading content that may not be ready yet',
8385
suggestions: [
84-
`Wait for the loading state to complete: cy.get('${selector}').should('not.exist')`,
86+
`If waiting for content to load, wait for the loading indicator to disappear first: cy.get('${escapedSelector}').should('not.exist').then(() => cy.get('[data-cy="content"]'))`,
87+
'Or wait for the API request: cy.intercept("GET", "/api/*").as("loadData"); cy.wait("@loadData")',
8588
'Consider using data-cy attributes instead of class names that indicate loading states',
86-
'Use cy.intercept() to wait for the API request that populates this content',
89+
`If you need the loading element itself, ensure it exists before trying to interact: cy.get('${escapedSelector}').should('exist')`,
8790
],
8891
docsUrl: 'https://on.cypress.io/best-practices#Selecting-Elements',
8992
})
9093
}
9194

9295
// Check for potentially incorrect selectors
9396
if (selector.includes(' ') && !selector.includes('[') && command === 'get') {
97+
const escapedFirst = selector.split(' ')[0].replace(/'/g, "\\'");
98+
const escapedRest = selector.split(' ').slice(1).join(' ').replace(/'/g, "\\'");
99+
94100
suggestions.push({
95101
reason: 'Complex selector detected - might be fragile or incorrect',
96102
suggestions: [
97103
'Verify the selector in DevTools: copy and paste it into the console',
98104
'Consider using data-cy attributes for more reliable selection',
99-
`Break down into multiple steps: cy.get('${selector.split(' ')[0]}').find('${selector.split(' ').slice(1).join(' ')}')`,
105+
`Break down into multiple steps: cy.get('${escapedFirst}').find('${escapedRest}')`,
100106
],
101107
docsUrl: 'https://on.cypress.io/best-practices#Selecting-Elements',
102108
})
103109
}
104110

105111
// Check for ID selectors that might be dynamic
106112
if (selector.startsWith('#') && /\d{3,}/.test(selector)) {
113+
const prefix = selector.split(/\d/)[0];
114+
const escapedPrefix = prefix.replace(/'/g, "\\'");
115+
107116
suggestions.push({
108117
reason: 'Selector uses an ID with numbers - might be dynamically generated',
109118
suggestions: [
110119
'Dynamic IDs change between sessions and will cause flaky tests',
111120
'Use a data-cy attribute or a more stable selector instead',
112-
'If the ID is dynamic, use a partial match: cy.get(\'[id^="prefix-"]\').first()',
121+
`If the ID is dynamic, use a partial match: cy.get('[id^="${escapedPrefix}"]').first()`,
113122
],
114123
})
115124
}
@@ -190,8 +199,10 @@ export class TimeoutDiagnostics {
190199

191200
// Add command-specific suggestions
192201
if (['get', 'contains'].includes(command) && selector) {
202+
const escapedSelector = selector.replace(/'/g, "\\'");
203+
193204
generalSuggestions.unshift(
194-
`Verify selector in DevTools: document.querySelector('${selector}')`,
205+
`Verify selector in DevTools: document.querySelector('${escapedSelector}')`,
195206
'Ensure the element is not hidden by CSS (display: none, visibility: hidden)',
196207
)
197208
}

packages/driver/test/unit/cypress/timeout_diagnostics.spec.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,20 @@ describe('TimeoutDiagnostics', () => {
1717

1818
expect(suggestions).toHaveLength(1)
1919
expect(suggestions[0].reason).toContain('dynamic/loading content')
20-
expect(suggestions[0].suggestions).toContain(
21-
'Wait for the loading state to complete: cy.get(\'.loading-spinner\').should(\'not.exist\')',
22-
)
20+
expect(suggestions[0].suggestions.some((s) => s.includes('wait for the loading indicator to disappear'))).toBe(true)
21+
})
22+
23+
it('escapes quotes in dynamic content selector suggestions', () => {
24+
const context = {
25+
command: 'get',
26+
selector: "[data-test='loading']",
27+
timeout: 4000,
28+
}
29+
30+
const suggestions = TimeoutDiagnostics.analyze(context)
31+
32+
expect(suggestions).toHaveLength(1)
33+
expect(suggestions[0].suggestions.some((s) => s.includes("\\'"))).toBe(true)
2334
})
2435

2536
it('detects complex selectors', () => {
@@ -228,6 +239,22 @@ describe('TimeoutDiagnostics', () => {
228239
expect(suggestions).toHaveLength(1)
229240
})
230241

242+
it('escapes quotes in code suggestions to prevent syntax errors', () => {
243+
const context = {
244+
command: 'get',
245+
selector: "[data-test='value']",
246+
timeout: 4000,
247+
}
248+
249+
const suggestions = TimeoutDiagnostics.analyze(context)
250+
const formatted = TimeoutDiagnostics.formatSuggestions(suggestions)
251+
252+
// Verify quotes are escaped in suggestions
253+
expect(formatted.includes("\\'")).toBe(true)
254+
// Verify no unescaped single quotes that would break JS
255+
expect(formatted.match(/cy\.get\('\[data-test='value'\]'\)/)).toBe(null)
256+
})
257+
231258
it('combines multiple diagnostic issues', () => {
232259
const context = {
233260
command: 'get',

0 commit comments

Comments
 (0)