-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Pr 6332 #6399
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
Pr 6332 #6399
Changes from 12 commits
aa70080
ac17db4
5ba8582
e727062
7b94267
d305671
bb15bf9
11f6472
111e9a3
a08c15c
d83b3e4
ba33729
a43b05e
462b9db
7244641
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,11 @@ | ||
| import fs from 'node:fs'; | ||
| import os from 'node:os'; | ||
| import path from 'node:path'; | ||
| import JSZip from 'jszip'; | ||
| import stream from 'node:stream'; | ||
| import util from 'node:util'; | ||
| import yauzl from 'yauzl'; | ||
|
|
||
| const pipeline = util.promisify(stream.pipeline); | ||
|
|
||
| export interface PluginInstallResult { | ||
| ok: boolean; | ||
|
|
@@ -109,22 +113,72 @@ export function safeUploadRelativePath(input: unknown) { | |
|
|
||
| export async function extractPluginZipToFolder(buffer: Buffer, stagedFolder: string, maxBytes: number) { | ||
| if (buffer.length > maxBytes) throw new Error('zip file too large'); | ||
| const zip = await JSZip.loadAsync(buffer); | ||
| let totalBytes = 0; | ||
| const entries = Object.values(zip.files); | ||
| if (entries.length === 0) throw new Error('zip contains no files'); | ||
| for (const entry of entries) { | ||
| if (entry.dir) continue; | ||
| const rel = safeUploadRelativePath(entry.name); | ||
| const unixMode = typeof entry.unixPermissions === 'number' ? entry.unixPermissions : 0; | ||
| if ((unixMode & 0o170000) === 0o120000) throw new Error(`zip entry is a symbolic link: ${entry.name}`); | ||
| const content = await entry.async('nodebuffer'); | ||
| totalBytes += content.length; | ||
| if (totalBytes > maxBytes) throw new Error('zip extracted size exceeds 50 MiB'); | ||
| const dest = path.join(stagedFolder, rel); | ||
| await fs.promises.mkdir(path.dirname(dest), { recursive: true }); | ||
| await fs.promises.writeFile(dest, content); | ||
| } | ||
|
|
||
| const zip = await new Promise<yauzl.ZipFile>((resolve, reject) => { | ||
| yauzl.fromBuffer(buffer, { lazyEntries: true }, (err, zipfile) => { | ||
| if (err || !zipfile) reject(err ?? new Error('failed to load zip')); | ||
| else resolve(zipfile); | ||
| }); | ||
| }); | ||
|
|
||
| return new Promise<void>((resolve, reject) => { | ||
| let totalBytes = 0; | ||
| let entryCount = 0; | ||
|
|
||
| zip.on('error', reject); | ||
| zip.on('end', () => { | ||
| if (entryCount === 0) reject(new Error('zip contains no files')); | ||
| else resolve(); | ||
| }); | ||
|
|
||
| zip.on('entry', (entry: yauzl.Entry) => { | ||
| if (entry.fileName.endsWith('/')) { | ||
| zip.readEntry(); | ||
| return; | ||
| } | ||
| entryCount++; | ||
|
|
||
| const unixMode = typeof entry.externalFileAttributes === 'number' ? (entry.externalFileAttributes >>> 16) : 0; | ||
| if ((unixMode & 0o170000) === 0o120000) { | ||
| reject(new Error(`zip entry is a symbolic link: ${entry.fileName}`)); | ||
| return; | ||
| } | ||
|
|
||
| let rel: string; | ||
| try { | ||
| rel = safeUploadRelativePath(entry.fileName); | ||
| } catch (err) { | ||
| reject(err); | ||
| return; | ||
| } | ||
|
|
||
| zip.openReadStream(entry, async (err, readStream) => { | ||
| if (err || !readStream) { | ||
| reject(err ?? new Error(`failed to read entry: ${entry.fileName}`)); | ||
| return; | ||
| } | ||
|
|
||
| const dest = path.join(stagedFolder, rel); | ||
| try { | ||
| await fs.promises.mkdir(path.dirname(dest), { recursive: true }); | ||
|
|
||
| readStream.on('data', (chunk: Buffer) => { | ||
| totalBytes += chunk.length; | ||
| if (totalBytes > maxBytes) { | ||
| readStream.destroy(new Error('zip extracted size exceeds 50 MiB')); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clean up the staging directory when streaming extraction fails. This new path writes each entry into |
||
| } | ||
| }); | ||
|
|
||
| await pipeline(readStream, fs.createWriteStream(dest)); | ||
| zip.readEntry(); | ||
| } catch (pipeErr) { | ||
| reject(pipeErr); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| zip.readEntry(); | ||
| }); | ||
| } | ||
|
|
||
| export function createPluginInstallationHelpers(deps: PluginInstallationHelpersDeps) { | ||
|
|
@@ -163,8 +217,13 @@ export function createPluginInstallationHelpers(deps: PluginInstallationHelpersD | |
|
|
||
| async function stageUploadedPluginZip(buffer: Buffer, source: string) { | ||
| const stagedFolder = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'od-plugin-zip-')); | ||
| await extractPluginZipToFolder(buffer, stagedFolder, deps.PLUGIN_UPLOAD_MAX_BYTES); | ||
| return finishUploadedPluginInstall(stagedFolder, source); | ||
| try { | ||
| await extractPluginZipToFolder(buffer, stagedFolder, deps.PLUGIN_UPLOAD_MAX_BYTES); | ||
| return await finishUploadedPluginInstall(stagedFolder, source); | ||
| } catch (err) { | ||
| await fs.promises.rm(stagedFolder, { recursive: true, force: true }).catch(() => undefined); | ||
| throw err; | ||
| } | ||
| } | ||
|
|
||
| async function stageUploadedPluginFolder(files: Array<{ buffer: Buffer; originalname: string }>, rawPaths: unknown) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import fs from 'node:fs'; | ||
| import os from 'node:os'; | ||
| import JSZip from 'jszip'; | ||
| import { createPluginInstallationHelpers } from '../src/services/plugin-installation.js'; | ||
|
|
||
| describe('plugin-installation zip extraction', () => { | ||
| it('cleans up staging directory on extraction failure', async () => { | ||
| // Generate a zip bomb: tiny compressed size, large decompressed size | ||
| const zip = new JSZip(); | ||
| zip.file('bomb.txt', Buffer.alloc(10000, 'A'), { compression: 'DEFLATE' }); | ||
| const buffer = await zip.generateAsync({ | ||
| type: 'nodebuffer', | ||
| compression: 'DEFLATE', | ||
| compressionOptions: { level: 9 } | ||
| }); | ||
|
|
||
| const deps = { | ||
| db: {} as any, | ||
| PLUGIN_UPLOAD_MAX_BYTES: buffer.length, // Buffer length is tiny (e.g. 200 bytes). Decompressed is 10000 bytes. | ||
| PLUGIN_REGISTRY_ROOTS: [], | ||
| PLUGIN_LOCKFILE_PATH: '', | ||
| installFromLocalFolder: async function* () { yield { kind: 'success' }; } | ||
| }; | ||
| const helpers = createPluginInstallationHelpers(deps); | ||
|
|
||
| const initialTmpCount = fs.readdirSync(os.tmpdir()).filter(n => n.startsWith('od-plugin-zip-')).length; | ||
|
|
||
| await expect(helpers.stageUploadedPluginZip(buffer, 'test')).rejects.toThrow('zip extracted size exceeds 50 MiB'); | ||
|
|
||
| const finalTmpCount = fs.readdirSync(os.tmpdir()).filter(n => n.startsWith('od-plugin-zip-')).length; | ||
| expect(finalTmpCount).toBe(initialTmpCount); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pin both new dependency specs to exact versions. This line adds
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.yauzlas^3.4.0, and the same changed dependency block adds@types/yauzlas^3.4.0; the repository guard reports both as violations because project dependencies must use exact versions orworkspace:*. As written,pnpm guardexits 1, so the branch cannot satisfy the required merge validation and future installs could resolve unreviewed releases. Change both specs to3.4.0and refresh the lockfile with the pinned workspace pnpm version.