fix: improve zip upload handling

steipete · steipete · commit 2a2749f852d5 · 2026-01-04T16:45:56.000+01:00
diff --git a/src/__tests__/upload.route.test.tsx b/src/__tests__/upload.route.test.tsx
@@ -1,4 +1,5 @@
 import { fireEvent, render, screen, waitFor } from '@testing-library/react'
+import { strToU8, zipSync } from 'fflate'
 import { vi } from 'vitest'
 
 import { Upload } from '../routes/upload'
@@ -16,7 +17,6 @@ vi.mock('convex/react', () => ({
   useConvexAuth: () => ({ isAuthenticated: true }),
   useMutation: () => generateUploadUrl,
   useAction: () => publishVersion,
-  useQuery: () => null,
 }))
 
 describe('Upload route', () => {
@@ -83,6 +83,64 @@ describe('Upload route', () => {
     expect(await screen.findByText(/Add at least one file/i)).toBeTruthy()
   })
 
+  it('extracts zip uploads and unwraps top-level folders', async () => {
+    render(<Upload />)
+    fireEvent.change(screen.getByPlaceholderText('my-skill-pack'), {
+      target: { value: 'cool-skill' },
+    })
+    fireEvent.change(screen.getByPlaceholderText('My Skill Pack'), {
+      target: { value: 'Cool Skill' },
+    })
+    fireEvent.change(screen.getByPlaceholderText('1.0.0'), {
+      target: { value: '1.2.3' },
+    })
+    fireEvent.change(screen.getByPlaceholderText('latest, beta'), {
+      target: { value: 'latest' },
+    })
+
+    const zip = zipSync({
+      'hetzner-cloud-skill/SKILL.md': new Uint8Array(strToU8('hello')),
+      'hetzner-cloud-skill/notes.txt': new Uint8Array(strToU8('notes')),
+    })
+    const zipBytes = zip.buffer.slice(zip.byteOffset, zip.byteOffset + zip.byteLength)
+    const zipFile = new File([zipBytes], 'bundle.zip', { type: 'application/zip' })
+
+    const input = screen.getByTestId('upload-input') as HTMLInputElement
+    fireEvent.change(input, { target: { files: [zipFile] } })
+
+    expect(await screen.findByText('notes.txt', {}, { timeout: 3000 })).toBeTruthy()
+    expect(screen.getByText('SKILL.md')).toBeTruthy()
+    expect(await screen.findByText(/Ready to publish/i, {}, { timeout: 3000 })).toBeTruthy()
+  })
+
+  it('blocks non-text folder uploads (png)', async () => {
+    render(<Upload />)
+    fireEvent.change(screen.getByPlaceholderText('my-skill-pack'), {
+      target: { value: 'cool-skill' },
+    })
+    fireEvent.change(screen.getByPlaceholderText('My Skill Pack'), {
+      target: { value: 'Cool Skill' },
+    })
+    fireEvent.change(screen.getByPlaceholderText('1.0.0'), {
+      target: { value: '1.2.3' },
+    })
+    fireEvent.change(screen.getByPlaceholderText('latest, beta'), {
+      target: { value: 'latest' },
+    })
+
+    const skill = new File(['hello'], 'SKILL.md', { type: 'text/markdown' })
+    const png = new File([new Uint8Array([137, 80, 78, 71]).buffer], 'screenshot.png', {
+      type: 'image/png',
+    })
+    const input = screen.getByTestId('upload-input') as HTMLInputElement
+    fireEvent.change(input, { target: { files: [skill, png] } })
+
+    expect(await screen.findByText('screenshot.png')).toBeTruthy()
+    fireEvent.click(screen.getByRole('button', { name: /publish/i }))
+    expect(await screen.findByText(/Remove non-text files: screenshot\.png/i)).toBeTruthy()
+    expect(screen.getByText('screenshot.png')).toBeTruthy()
+  })
+
   it('surfaces publish errors and stays on page', async () => {
     publishVersion.mockRejectedValueOnce(new Error('Changelog is required'))
     generateUploadUrl.mockResolvedValue('https://upload.local')
diff --git a/src/lib/uploadFiles.jsdom.test.ts b/src/lib/uploadFiles.jsdom.test.ts
@@ -0,0 +1,33 @@
+import { strToU8, unzipSync, zipSync } from 'fflate'
+import { describe, expect, it } from 'vitest'
+
+import { expandFiles } from './uploadFiles'
+
+function readWithFileReader(blob: Blob) {
+  return new Promise<ArrayBuffer>((resolve, reject) => {
+    const reader = new FileReader()
+    reader.onerror = () => reject(reader.error ?? new Error('Could not read blob.'))
+    reader.onload = () => resolve(reader.result as ArrayBuffer)
+    reader.readAsArrayBuffer(blob)
+  })
+}
+
+describe('expandFiles (jsdom)', () => {
+  it('expands zip archives using FileReader fallback', async () => {
+    const zip = zipSync({
+      'hetzner-cloud-skill/SKILL.md': new Uint8Array(strToU8('hello')),
+      'hetzner-cloud-skill/notes.txt': new Uint8Array(strToU8('notes')),
+    })
+    const zipBytes = zip.buffer.slice(zip.byteOffset, zip.byteOffset + zip.byteLength)
+    const zipFile = new File([zipBytes], 'bundle.zip', { type: 'application/zip' })
+
+    const readerBuffer = await readWithFileReader(zipFile)
+    const entries = unzipSync(new Uint8Array(readerBuffer))
+    expect(Object.keys(entries)).toEqual(
+      expect.arrayContaining(['hetzner-cloud-skill/SKILL.md', 'hetzner-cloud-skill/notes.txt']),
+    )
+
+    const expanded = await expandFiles([zipFile])
+    expect(expanded.map((file) => file.name)).toEqual(['SKILL.md', 'notes.txt'])
+  })
+})
diff --git a/src/lib/uploadFiles.test.ts b/src/lib/uploadFiles.test.ts
@@ -71,6 +71,21 @@ describe('expandFiles', () => {
     expect(result.map((file) => file.name)).toEqual(['SKILL.md', 'docs/readme.txt'])
   })
 
+  it('unwraps top-level folders in zip archives', async () => {
+    const zip = zipSync({
+      'hetzner-cloud-skill/SKILL.md': strToU8('hello'),
+      'hetzner-cloud-skill/docs/readme.txt': strToU8('doc'),
+      '__MACOSX/._SKILL.md': strToU8('junk'),
+      'hetzner-cloud-skill/.DS_Store': strToU8('junk2'),
+      'hetzner-cloud-skill/screenshot.png': strToU8('not-really-a-png'),
+    })
+    const zipFile = new File([zip.buffer], 'pack.zip', { type: 'application/zip' })
+    const result = await expandFiles([zipFile])
+    expect(result.map((file) => file.name)).toEqual(['SKILL.md', 'docs/readme.txt'])
+    const png = result.find((file) => file.name.endsWith('.png'))
+    expect(png).toBeUndefined()
+  })
+
   it('expands gzipped tar archives into files', async () => {
     const tar = buildTar([
       { name: 'SKILL.md', content: 'hi' },
@@ -82,6 +97,17 @@ describe('expandFiles', () => {
     expect(result.map((file) => file.name)).toEqual(['SKILL.md', 'notes.txt'])
   })
 
+  it('unwraps top-level folders in tar.gz archives', async () => {
+    const tar = buildTar([
+      { name: 'skill-folder/SKILL.md', content: 'hi' },
+      { name: 'skill-folder/notes.txt', content: 'yo' },
+    ])
+    const tgz = gzipSync(tar)
+    const tgzFile = new File([tgz.buffer], 'bundle.tgz', { type: 'application/gzip' })
+    const result = await expandFiles([tgzFile])
+    expect(result.map((file) => file.name)).toEqual(['SKILL.md', 'notes.txt'])
+  })
+
   it('expands .gz single files', async () => {
     const gz = gzipSync(strToU8('content'))
     const gzFile = new File([gz.buffer], 'skill.md.gz', { type: 'application/gzip' })
diff --git a/src/lib/uploadFiles.ts b/src/lib/uploadFiles.ts
@@ -1,3 +1,4 @@
+import { TEXT_FILE_EXTENSION_SET } from 'clawdhub-schema'
 import { gunzipSync, unzipSync } from 'fflate'
 
 const TEXT_TYPES = new Map([
@@ -23,53 +24,77 @@ export async function expandFiles(selected: File[]) {
     const lower = file.name.toLowerCase()
     if (lower.endsWith('.zip')) {
       const entries = unzipSync(new Uint8Array(await readArrayBuffer(file)))
-      for (const [path, data] of Object.entries(entries)) {
-        if (!path || path.endsWith('/')) continue
-        expanded.push(
-          new File([data.buffer], normalizePath(path), {
-            type: guessContentType(path),
-          }),
-        )
-      }
+      pushArchiveEntries(
+        expanded,
+        Object.entries(entries).map(([path, data]) => ({ path, data })),
+      )
       continue
     }
     if (lower.endsWith('.tar.gz') || lower.endsWith('.tgz')) {
       const unpacked = gunzipSync(new Uint8Array(await readArrayBuffer(file)))
-      for (const entry of untar(unpacked)) {
-        expanded.push(
-          new File([entry.data.buffer], normalizePath(entry.path), {
-            type: guessContentType(entry.path),
-          }),
-        )
-      }
+      pushArchiveEntries(expanded, untar(unpacked))
       continue
     }
     if (lower.endsWith('.gz')) {
       const unpacked = gunzipSync(new Uint8Array(await readArrayBuffer(file)))
       const name = file.name.replace(/\.gz$/i, '')
-      expanded.push(new File([unpacked.buffer], name, { type: guessContentType(name) }))
+      expanded.push(new File([unpacked], name, { type: guessContentType(name) }))
       continue
     }
     expanded.push(file)
   }
   return expanded
 }
 
+function pushArchiveEntries(target: File[], entries: Array<{ path: string; data: Uint8Array }>) {
+  const normalized = entries
+    .map((entry) => ({ ...entry, path: normalizePath(entry.path) }))
+    .filter((entry) => entry.path && !entry.path.endsWith('/'))
+    .filter((entry) => !isJunkPath(entry.path))
+    .filter((entry) => isTextPath(entry.path))
+
+  const unwrapped = unwrapSingleTopLevelFolder(normalized)
+
+  for (const entry of unwrapped) {
+    target.push(
+      new File([entry.data], entry.path, {
+        type: guessContentType(entry.path),
+      }),
+    )
+  }
+}
+
 async function readArrayBuffer(file: Blob) {
   if (typeof file.arrayBuffer === 'function') {
     return file.arrayBuffer()
   }
-  return new Response(file).arrayBuffer()
+  if (typeof FileReader !== 'undefined') {
+    return new Promise<ArrayBuffer>((resolve, reject) => {
+      const reader = new FileReader()
+      reader.onerror = () => reject(reader.error ?? new Error('Could not read file.'))
+      reader.onload = () => resolve(reader.result as ArrayBuffer)
+      reader.readAsArrayBuffer(file)
+    })
+  }
+  return new Response(file as BodyInit).arrayBuffer()
 }
 
 function guessContentType(path: string) {
   const ext = path.split('.').pop()?.toLowerCase()
-  if (!ext) return 'text/plain'
-  return TEXT_TYPES.get(ext) ?? 'text/plain'
+  if (!ext) return 'application/octet-stream'
+  const known = TEXT_TYPES.get(ext)
+  if (known) return known
+  if (TEXT_FILE_EXTENSION_SET.has(ext)) return 'text/plain'
+  return 'application/octet-stream'
 }
 
 function normalizePath(path: string) {
-  return path.replace(/^\.\/+/, '').replace(/^\/+/, '')
+  return path
+    .replaceAll('\u0000', '')
+    .replaceAll('\\', '/')
+    .trim()
+    .replace(/^\.\/+/, '')
+    .replace(/^\/+/, '')
 }
 
 function untar(bytes: Uint8Array) {
@@ -100,3 +125,35 @@ function readOctal(bytes: Uint8Array) {
   const raw = readString(bytes)
   return raw ? Number.parseInt(raw, 8) : 0
 }
+
+function unwrapSingleTopLevelFolder<T extends { path: string }>(entries: T[]) {
+  if (entries.length === 0) return entries
+
+  const segments = entries.map((entry) => entry.path.split('/').filter(Boolean))
+  if (segments.some((parts) => parts.length < 2)) return entries
+
+  const first = segments[0]?.[0]
+  if (!first) return entries
+  if (!segments.every((parts) => parts[0] === first)) return entries
+
+  return entries.map((entry) => ({
+    ...entry,
+    path: entry.path.split('/').slice(1).join('/'),
+  }))
+}
+
+function isJunkPath(path: string) {
+  const normalized = path.toLowerCase()
+  if (normalized.startsWith('__macosx/')) return true
+  if (normalized.endsWith('/.ds_store')) return true
+  if (normalized === '.ds_store') return true
+  return false
+}
+
+function isTextPath(path: string) {
+  const normalized = path.trim().toLowerCase()
+  const parts = normalized.split('.')
+  const extension = parts.length > 1 ? (parts.at(-1) ?? '') : ''
+  if (!extension) return false
+  return TEXT_FILE_EXTENSION_SET.has(extension)
+}
diff --git a/src/routes/upload.tsx b/src/routes/upload.tsx
@@ -1,5 +1,6 @@
 import { createFileRoute, useNavigate } from '@tanstack/react-router'
-import { useAction, useConvexAuth, useMutation, useQuery } from 'convex/react'
+import { isTextContentType, TEXT_FILE_EXTENSION_SET } from 'clawdhub-schema'
+import { useAction, useConvexAuth, useMutation } from 'convex/react'
 import { useEffect, useMemo, useRef, useState } from 'react'
 import semver from 'semver'
 import { api } from '../../convex/_generated/api'
@@ -41,8 +42,6 @@ export function Upload() {
   const trimmedSlug = slug.trim()
   const trimmedName = displayName.trim()
   const trimmedChangelog = changelog.trim()
-  const lookupSlug = trimmedSlug && SLUG_PATTERN.test(trimmedSlug) ? trimmedSlug : ''
-  const existingSkill = useQuery(api.skills.getBySlug, lookupSlug ? { slug: lookupSlug } : 'skip')
   const parsedTags = useMemo(
     () =>
       tags
@@ -67,33 +66,29 @@ export function Upload() {
     if (parsedTags.length === 0) {
       issues.push('At least one tag is required.')
     }
-    if (existingSkill && !trimmedChangelog) {
-      issues.push('Changelog is required for updates.')
-    }
     if (files.length === 0) {
       issues.push('Add at least one file.')
     }
     if (!hasSkillFile) {
       issues.push('SKILL.md is required.')
     }
+    const invalidFiles = files.filter((file) => !isTextFile(file))
+    if (invalidFiles.length > 0) {
+      issues.push(
+        `Remove non-text files: ${invalidFiles
+          .slice(0, 3)
+          .map((file) => file.name)
+          .join(', ')}`,
+      )
+    }
     if (totalBytes > maxBytes) {
       issues.push('Total file size exceeds 50MB.')
     }
     return {
       issues,
       ready: issues.length === 0,
     }
-  }, [
-    trimmedSlug,
-    trimmedName,
-    version,
-    parsedTags.length,
-    trimmedChangelog,
-    existingSkill,
-    files.length,
-    hasSkillFile,
-    totalBytes,
-  ])
+  }, [trimmedSlug, trimmedName, version, parsedTags.length, files, hasSkillFile, totalBytes])
 
   useEffect(() => {
     if (!fileInputRef.current) return
@@ -444,3 +439,13 @@ function formatPublishError(error: unknown) {
   }
   return 'Publish failed. Please try again.'
 }
+
+function isTextFile(file: File) {
+  const path = (file.webkitRelativePath || file.name).trim().toLowerCase()
+  if (!path) return false
+  const parts = path.split('.')
+  const extension = parts.length > 1 ? (parts.at(-1) ?? '') : ''
+  if (file.type && isTextContentType(file.type)) return true
+  if (extension && TEXT_FILE_EXTENSION_SET.has(extension)) return true
+  return false
+}
