chore: upgrade Astro 6.4.8 → 7.0.2#838
Conversation
Moves to Astro 7's Rust compiler and Vite 8/Rolldown, updates Cloudflare and MDX integrations, fixes native-build.astro for stricter HTML parsing, and adds a Playwright visual-diff script for upgrade regression checks. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughBumps ChangesAstro v7 Upgrade and Visual Regression Tooling
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CLI as visual-diff.mjs CLI
participant Server as Static HTTP Server
participant Browser as Playwright Chromium
participant IM as ImageMagick compare
Dev->>CLI: node visual-diff.mjs capture before
CLI->>Server: startStaticServer(web dist, 4321)
CLI->>Server: startStaticServer(docs dist, 4322)
CLI->>Browser: launch, iterate viewports + routes
Browser->>Server: GET /route
Server-->>Browser: HTML/assets
Browser-->>CLI: full-page PNG saved to .visual-diff/before/
Dev->>CLI: node visual-diff.mjs capture after
Note over CLI,Browser: same capture flow → .visual-diff/after/
Dev->>CLI: node visual-diff.mjs compare
CLI->>IM: compare before.png after.png -metric AE
IM-->>CLI: pixel diff count
CLI-->>Dev: report.json + exit code (0=identical, 1=changes)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/visual-diff.mjs`:
- Around line 112-115: The capture function accepts a user-controlled label
parameter that is directly concatenated into a file path and used in a recursive
delete operation without validation. This allows path traversal attacks using
sequences like ".." to delete directories outside the intended OUT_DIR location.
Add validation at the beginning of the capture function to restrict the label
parameter and reject any values containing path separators (forward slashes,
backslashes) or parent directory references (".."). Apply the same validation to
the other instance of this vulnerability mentioned at lines 183-190.
- Around line 124-139: The compareImages function returns null when it cannot
parse a valid diff pixel value from stderr, which silently treats errors as
success. Instead of returning null on parsing failure, the function should
validate that the stderr output contains a valid parseable number and throw an
error if it does not. Modify the logic after Number.parseFloat(stderr) to check
if the result is finite, and if not (when isFinite returns false), throw an
error with a descriptive message that includes the actual stderr output. This
ensures that failures in the image comparison process (unreadable diffs, missing
files, or mismatched images) properly fail the regression test rather than being
treated as successful comparisons.
- Around line 63-67: The path traversal vulnerability exists because the
filePath constructed from joining distDir with the decoded pathname is not
validated to ensure it stays within the distDir boundary, allowing attackers to
use .. sequences to escape the directory. After constructing filePath with
path.join(distDir, pathname), use path.resolve() to get the absolute paths of
both filePath and distDir, then use path.relative() to verify that filePath is
actually within distDir by checking that the relative path does not start with
.., and reject the request if it does escape the boundary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 76493ab0-c53d-4f1b-83d4-e7b90ce818f9
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.gitignoreapps/docs/package.jsonapps/web/src/pages/native-build.astropackage.jsonscripts/visual-diff.mjs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
💤 Files with no reviewable changes (1)
- apps/web/src/pages/native-build.astro
| const url = new URL(req.url || '/', `http://127.0.0.1:${port}`) | ||
| let pathname = decodeURIComponent(url.pathname) | ||
| if (pathname.endsWith('/')) pathname += 'index.html' | ||
| const filePath = path.join(distDir, pathname) | ||
| const data = await readFile(filePath) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Prevent path traversal in the local static server.
The request path is decoded and joined directly into distDir without a root-boundary check, so .. traversal can escape the dist folder.
Proposed fix
const url = new URL(req.url || '/', `http://127.0.0.1:${port}`)
let pathname = decodeURIComponent(url.pathname)
if (pathname.endsWith('/')) pathname += 'index.html'
- const filePath = path.join(distDir, pathname)
+ const filePath = path.resolve(distDir, `.${pathname}`)
+ const distRoot = path.resolve(distDir) + path.sep
+ if (!filePath.startsWith(distRoot)) {
+ res.writeHead(403).end('Forbidden')
+ return
+ }
const data = await readFile(filePath)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/visual-diff.mjs` around lines 63 - 67, The path traversal
vulnerability exists because the filePath constructed from joining distDir with
the decoded pathname is not validated to ensure it stays within the distDir
boundary, allowing attackers to use .. sequences to escape the directory. After
constructing filePath with path.join(distDir, pathname), use path.resolve() to
get the absolute paths of both filePath and distDir, then use path.relative() to
verify that filePath is actually within distDir by checking that the relative
path does not start with .., and reject the request if it does escape the
boundary.
| async function capture(label) { | ||
| const outputDir = path.join(OUT_DIR, label) | ||
| await rm(outputDir, { recursive: true, force: true }) | ||
| await ensureDir(outputDir) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Restrict capture labels before recursive deletion.
label is user-controlled and is fed into rm(..., { recursive: true, force: true }) without validation. A crafted value can resolve outside .visual-diff and delete unintended directories.
Proposed fix
+const CAPTURE_LABELS = new Set(['before', 'after'])
+
async function capture(label) {
- const outputDir = path.join(OUT_DIR, label)
+ const outputDir = path.resolve(OUT_DIR, label)
+ if (!outputDir.startsWith(`${OUT_DIR}${path.sep}`)) {
+ throw new Error(`Invalid capture label: ${label}`)
+ }
await rm(outputDir, { recursive: true, force: true })
await ensureDir(outputDir)
@@
if (command === 'capture') {
- if (!label) {
+ if (!label || !CAPTURE_LABELS.has(label)) {
console.error('Usage: bun run scripts/visual-diff.mjs capture <before|after>')
process.exit(1)
}
await capture(label)Also applies to: 183-190
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/visual-diff.mjs` around lines 112 - 115, The capture function accepts
a user-controlled label parameter that is directly concatenated into a file path
and used in a recursive delete operation without validation. This allows path
traversal attacks using sequences like ".." to delete directories outside the
intended OUT_DIR location. Add validation at the beginning of the capture
function to restrict the label parameter and reject any values containing path
separators (forward slashes, backslashes) or parent directory references ("..").
Apply the same validation to the other instance of this vulnerability mentioned
at lines 183-190.
| async function compareImages(beforePath, afterPath) { | ||
| const proc = spawn('compare', ['-metric', 'AE', beforePath, afterPath, 'null:'], { | ||
| stdio: ['ignore', 'pipe', 'pipe'], | ||
| }) | ||
|
|
||
| const stderr = await new Promise((resolve) => { | ||
| let data = '' | ||
| proc.stderr.on('data', (chunk) => { | ||
| data += chunk.toString() | ||
| }) | ||
| proc.on('close', () => resolve(data.trim())) | ||
| }) | ||
|
|
||
| const diffPixels = Number.parseFloat(stderr) | ||
| return Number.isFinite(diffPixels) ? diffPixels : null | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail compare mode on invalid diffs and mismatched file sets.
Current logic can return success when diffs are unreadable or before/after screenshots don’t match, which weakens regression gating.
Proposed fix
async function compareImages(beforePath, afterPath) {
@@
- const diffPixels = Number.parseFloat(stderr)
- return Number.isFinite(diffPixels) ? diffPixels : null
+ const diffPixels = Number.parseFloat(stderr)
+ if (!Number.isFinite(diffPixels)) {
+ throw new Error(`Image comparison failed: ${stderr || 'no metric output'}`)
+ }
+ return diffPixels
}
@@
- const files = (await readdir(beforeDir)).filter((f) => f.endsWith('.png')).sort()
+ const beforeFiles = (await readdir(beforeDir)).filter((f) => f.endsWith('.png')).sort()
+ const afterFiles = (await readdir(afterDir)).filter((f) => f.endsWith('.png')).sort()
+ if (beforeFiles.length !== afterFiles.length || beforeFiles.some((f, i) => f !== afterFiles[i])) {
+ throw new Error('Before/after screenshot sets do not match. Re-run capture for both labels.')
+ }
+ const files = beforeFiles
@@
- const status = diffPixels === 0 ? 'identical' : diffPixels === null ? 'unknown' : 'changed'
+ const status = diffPixels === 0 ? 'identical' : 'changed'
@@
- const changed = results.filter((r) => r.status === 'changed').length
+ const changed = results.filter((r) => r.status === 'changed').length
+ const errored = results.filter((r) => r.status === 'error').length
@@
- return changed === 0
+ return changed === 0 && errored === 0Also applies to: 147-180
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/visual-diff.mjs` around lines 124 - 139, The compareImages function
returns null when it cannot parse a valid diff pixel value from stderr, which
silently treats errors as success. Instead of returning null on parsing failure,
the function should validate that the stderr output contains a valid parseable
number and throw an error if it does not. Modify the logic after
Number.parseFloat(stderr) to check if the result is finite, and if not (when
isFinite returns false), throw an error with a descriptive message that includes
the actual stderr output. This ensures that failures in the image comparison
process (unreadable diffs, missing files, or mismatched images) properly fail
the regression test rather than being treated as successful comparisons.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: medium. Did not approve — this Astro 6→7 monorepo upgrade exceeds the low-risk approval threshold. Cursor Bugbot did not run on this PR (signal skipped). Assigned rishi-raj-jain and WcaleNieWolny for human review.
Sent by Cursor Approval Agent: Pull Request Approver External
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| async function compareImages(beforePath, afterPath) { | ||
| const proc = spawn('compare', ['-metric', 'AE', beforePath, afterPath, 'null:'], { |
|
|
||
| if (status === 'changed') { | ||
| const diffPath = path.join(diffDir, file.replace('.png', '-diff.png')) | ||
| spawn('compare', [beforePath, afterPath, diffPath], { stdio: 'ignore' }) |
2 participants
Summary
astroto 7.0.2 (Rust compiler, Vite 8/Rolldown) and bumps@astrojs/cloudflareto 14.0.0 /@astrojs/mdxto 7.0.0 for docs.native-build.astrofor Astro 7’s stricter HTML parser by removing a multi-line HTML comment that broke the Rust compiler after a large<script>block.scripts/visual-diff.mjs(Playwright + ImageMagick) to capture/compare representative web + docs pages during framework upgrades.Build impact
bun run buildVisual diff (before → after)
Captured 18 screenshots (7 web + 3 docs routes × desktop + mobile) from production builds served statically.
/solutions/live-updates/,/docs/plugins/updater//about/,/plugins/,/pricing/,/docs/getting-started/quickstart/(mobile)/,/docs/, docs quickstart desktop/blog/(dynamic post listing), home mobile (full-page capture variance)No layout-breaking regressions observed; largest diffs align with dynamic blog content and subpixel/font rendering rather than broken pages.
Re-run locally:
Test plan
bun run build(docs + web)bun run ci:verify:web(check + build + SEO checker)bun run ci:verify:docs(check + build)/native-build/in preview (page touched for compiler fix)Made with Cursor
Summary by CodeRabbit
Release Notes
New Features
Chores