fix(core): show assets stored in nested folders in the asset library#240
fix(core): show assets stored in nested folders in the asset library#240adawang1210 wants to merge 1 commit into
Conversation
The asset list endpoint read the scoped assets dir non-recursively, so files inside subfolders never appeared. Walk the directory recursively and support forward-slash nested paths end to end (list, serve, usages, upload, rename, delete), keeping the path-traversal containment guard. Rename keeps a nested asset in its directory. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@adawang1210 is attempting to deploy a commit to the Yiwei Ho Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds ChangesNested Asset Library Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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. 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: 1
🤖 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 `@packages/core/src/vite/routes/assets.ts`:
- Around line 95-99: The fs.stat() call within the entries loop does not handle
the case where a file may be deleted between the time it was enumerated and when
stat is attempted, causing the entire asset listing request to fail with a 500
error. Wrap the fs.stat() call in a try-catch block to catch ENOENT errors (or
other file system errors) and simply continue to the next entry when such an
error occurs, allowing the listing to complete despite transient file deletions.
🪄 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: CHILL
Plan: Pro
Run ID: c7e14464-a96a-469c-8fcd-8a2a9f44cf18
📒 Files selected for processing (4)
.changeset/asset-library-nested-folders.mdpackages/core/src/files/assets.test.tspackages/core/src/files/assets.tspackages/core/src/vite/routes/assets.ts
| for (const name of entries) { | ||
| if (!validateAssetName(name)) continue; | ||
| if (!validateAssetPath(name)) continue; | ||
| const stat = await fs.stat(path.join(scopedDir, name)); | ||
| if (!stat.isFile()) continue; | ||
| assets.push({ |
There was a problem hiding this comment.
Handle per-entry ENOENT during listing to prevent transient 500s.
At Line 97, the file can disappear between recursive enumeration and fs.stat(...); that currently aborts the entire list request instead of skipping the vanished file.
Suggested fix
for (const name of entries) {
if (!validateAssetPath(name)) continue;
- const stat = await fs.stat(path.join(scopedDir, name));
- if (!stat.isFile()) continue;
- assets.push({
- name,
- size: stat.size,
- mtime: stat.mtimeMs,
- mime: mimeForFilename(name),
- url: `/__assets/${slideId}/${encodeURIComponent(name)}`,
- unused: true,
- });
+ try {
+ const stat = await fs.stat(path.join(scopedDir, name));
+ if (!stat.isFile()) continue;
+ assets.push({
+ name,
+ size: stat.size,
+ mtime: stat.mtimeMs,
+ mime: mimeForFilename(name),
+ url: `/__assets/${slideId}/${encodeURIComponent(name)}`,
+ unused: true,
+ });
+ } catch (err) {
+ if ((err as NodeJS.ErrnoException).code === 'ENOENT') continue;
+ throw err;
+ }
}📝 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.
| for (const name of entries) { | |
| if (!validateAssetName(name)) continue; | |
| if (!validateAssetPath(name)) continue; | |
| const stat = await fs.stat(path.join(scopedDir, name)); | |
| if (!stat.isFile()) continue; | |
| assets.push({ | |
| for (const name of entries) { | |
| if (!validateAssetPath(name)) continue; | |
| try { | |
| const stat = await fs.stat(path.join(scopedDir, name)); | |
| if (!stat.isFile()) continue; | |
| assets.push({ | |
| name, | |
| size: stat.size, | |
| mtime: stat.mtimeMs, | |
| mime: mimeForFilename(name), | |
| url: `/__assets/${slideId}/${encodeURIComponent(name)}`, | |
| unused: true, | |
| }); | |
| } catch (err) { | |
| if ((err as NodeJS.ErrnoException).code === 'ENOENT') continue; | |
| throw err; | |
| } | |
| } |
🤖 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 `@packages/core/src/vite/routes/assets.ts` around lines 95 - 99, The fs.stat()
call within the entries loop does not handle the case where a file may be
deleted between the time it was enumerated and when stat is attempted, causing
the entire asset listing request to fail with a 500 error. Wrap the fs.stat()
call in a try-catch block to catch ENOENT errors (or other file system errors)
and simply continue to the next entry when such an error occurs, allowing the
listing to complete despite transient file deletions.
1 participant
Problem
Fixes #224. Files placed in subfolders of a slide's
assets/(or the global assets dir) never appear in the asset library — only top-level files are listed.Cause
In
packages/core/src/vite/routes/assets.ts, the list handler read the scoped assets directory with a single non-recursivefs.readdir, andvalidateAssetNamerejected any name containing a path separator. Nested files were therefore never enumerated, never served, and never matched for usage detection.Fix
listAssetFilesRecursive, which walks the scoped directory and returns forward-slash relative paths.validateAssetPath(nested-aware sibling ofvalidateAssetName): validates every segment, requires an extension only on the final segment, and rejects.., absolute, and backslash paths. The existingpath.resolve(...) + startsWith(dir + sep)containment check remains the traversal boundary.encodeURIComponent(/→%2F), so routing is unchanged.Tests
Added unit tests for
validateAssetPath(nested accept, traversal/absolute/backslash/hidden-segment reject) andlistAssetFilesRecursive(nested enumeration, missing dir).pnpm test,pnpm core typecheck, andbiome checkon the changed files all pass.🤖 Generated with Claude Code
Summary by CodeRabbit
logos/brand.svg. Upload and rename operations preserve nested folder structures, enabling improved asset organization and discoverability.