Slice 8: Server Module Boundary Hardening#55
Open
arndvs wants to merge 5 commits into
Open
Conversation
Move filesystem operations behind StorageAdapter: - detected-assets: use detectAssetFiles() instead of fs.access() - upload: use saveAssetFile() instead of fs.mkdir/unlink/writeFile() - outputs: use readOutputFile() instead of fs.readFile() readOutputFile validates individual path segments (absolute, .., null bytes) before delegating to the adapter, preserving the same rejection behavior as the former safeJoin call in the route handler. Acceptance: no node:fs/promises imports remain in app/api/.
Create lib/cast/server/index.ts that re-exports the entire public API surface. Documents the import boundary: app/api/* → lib/cast/server/* only No direct fs, Azure, or Qdrant calls in route handlers This barrel becomes the Fastify service API surface when the backend migrates off Next.js API routes.
There was a problem hiding this comment.
Pull request overview
This PR hardens the server/module boundary by moving filesystem I/O out of Next.js route handlers and into lib/cast/server storage helpers backed by StorageAdapter, plus adds a server barrel export intended to define the long-term service API surface.
Changes:
- Refactors API routes to remove direct
node:fs/promisesusage and delegate toStorageAdapter-backed helpers. - Adds new storage helpers: asset detection, asset upload save/replace, and outputs file reads with segment validation.
- Introduces
lib/cast/server/index.tsas a barrel export documenting import boundary rules.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/cast/server/storage.ts | Adds detectAssetFiles, saveAssetFile, and readOutputFile helpers that delegate to StorageAdapter. |
| lib/cast/server/index.ts | New barrel export defining/documenting the intended public server API surface. |
| app/api/upload/route.ts | Replaces direct FS write/delete operations with saveAssetFile(). |
| app/api/outputs/[...path]/route.ts | Replaces direct FS reads with readOutputFile() while keeping .png whitelist behavior in the route. |
| app/api/detected-assets/route.ts | Replaces per-extension FS probing with detectAssetFiles(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ents saveAssetFile: narrow ext param to AssetExt union type with runtime check, preventing arbitrary extensions from reaching the storage layer. readOutputFile: split segments on / and \ before validation so embedded separators cannot smuggle traversal components past per-segment checks. Pre-check rejects absolute and empty raw segments before normalization.
Replace `export *` from storage-adapter with explicit named exports, hiding _resetStorageAdapter and LocalFsAdapter test seams from the public API surface. Replace eager re-export of azure-blob-adapter with type-only export so @azure/storage-blob stays lazy-loaded via dynamic import().
Addresses Copilot review on PR #55 (round 2): - outputs/[...path]/route.ts — stale reference to safeJoin replaced with readOutputFile's segment validation description.
This was referenced May 9, 2026
Comment on lines
+10
to
+13
| * ✅ lib/cast/server/* → lib/cast/* (server uses shared schemas/types) | ||
| * ❌ app/api/* → node:fs/promises (no direct I/O in route handlers) | ||
| * ❌ app/api/* → @azure/* (no direct Azure calls in routes) | ||
| * ❌ components/ → lib/cast/server/* (client code never imports server) |
Comment on lines
+157
to
+163
| // Normalize: split on both / and \ so embedded separators can't smuggle | ||
| // traversal components past the per-segment check. | ||
| const parts = segments.flatMap((s) => s.split(/[/\\]/)).filter(Boolean) | ||
| for (const part of parts) { | ||
| if (part === "." || part === ".." || part.includes("\0")) { | ||
| throw new PathTraversalError(`invalid output path segment: "${part}"`) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Harden the server module boundary so route handlers are thin pass-throughs with no direct filesystem I/O.
Changes
Route refactoring — eliminate all
node:fs/promisesimports fromapp/api/:detected-assets/route.ts→ usesdetectAssetFiles()via StorageAdapter instead offs.access()upload/route.ts→ usessaveAssetFile()via StorageAdapter instead offs.mkdir/unlink/writeFile()outputs/[...path]/route.ts→ usesreadOutputFile()via StorageAdapter instead offs.readFile()New server functions in
lib/cast/server/storage.ts:detectAssetFiles(slugs)— delegates to existingfindLocalAsset()saveAssetFile(productSlug, ext, bytes)— delete-then-write via adapterreadOutputFile(...segments)— validates path segments then reads via adapterBarrel export —
lib/cast/server/index.ts:Acceptance Criteria
app/api/*→lib/cast/server/*onlyfs, Azure, or Qdrant calls in route handlersRelated