Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Security Review — PR #111
Result: No high-confidence vulnerabilities introduced by this PR.
This PR is a pure package-export refactoring: localAdapter moves from the @btst/stack/plugins/media/api barrel to its own entry point @btst/stack/plugins/media/api/adapters/local. The implementation in local.ts is byte-for-byte identical between the base and head commits. No authentication, authorization, serialization, secret-handling, or dependency changes are present in the diff.
Pre-existing observation (not introduced by this PR)
While reviewing local.ts in context, one pre-existing weakness is worth noting now that this adapter is a more visible standalone export.
Path traversal in delete() lacks an upload-directory boundary check.
The delete method reconstructs a file path by splitting the stored URL on /, decoding the last segment, and joining it with uploadDir:
const encodedFilename = url.split("/").pop();
const filename = decodeURIComponent(encodedFilename);
const filePath = path.join(uploadDir, filename); // no boundary check
await fs.unlink(filePath);If a URL such as /uploads/..%2F..%2Fetc%2Fpasswd were ever stored in the database, decoding it yields ../../etc/passwd and path.join resolves outside uploadDir. In practice the upload path generates filenames via path.basename() + random bytes so normal writes are safe — but there is no guard in delete() itself to assert filePath.startsWith(path.resolve(uploadDir)).
Exploitability: Low under normal operation (URLs are written by the trusted upload path). Becomes a real risk if records can be seeded through DB migrations, admin tooling, or a future code path that writes arbitrary URLs.
Suggested remediation (for a follow-up):
const resolvedUploadDir = path.resolve(uploadDir);
const filePath = path.join(resolvedUploadDir, filename);
if (!filePath.startsWith(resolvedUploadDir + path.sep)) {
throw new Error("Refusing to delete file outside upload directory");
}
await fs.unlink(filePath);This PR can be merged safely as written. The observation above is pre-existing and should be tracked as a separate hardening task.
Sent by Cursor Automation: Find vulnerabilities
|
✅ Shadcn registry validated — no registry changes detected. |
…ng file deletion paths
…o media backend plugin
olliethedev
changed the title
1 participant
Summary
Type of change
Checklist
pnpm buildpassespnpm typecheckpassespnpm lintpassesdocs/content/docs/) if consumer-facing types or behavior changedScreenshots
Note
Medium Risk
Medium risk because it changes the public package surface (new export path for
localAdapterand removal fromplugins/media/api) and adds stricter file-path validation in the local adapter delete flow, which could affect existing consumers relying on previous imports or permissive deletion behavior.Overview
Makes
localAdaptera standalone export by adding./plugins/media/api/adapters/localto build entries andpackage.jsonexports/typesVersions, and updating generated code templates to import it from the new path instead ofplugins/media/api.Hardens local file deletion in
localAdapter.delete()by resolvinguploadDirand rejecting decoded filenames that would escape the upload directory (path traversal), with a new unit test covering the malicious URL case.Expands the media plugin programmatic API surface by exposing
createAsset,updateAsset, andcreateFolderonmediaBackendPlugin().api(in addition to existing getters).Reviewed by Cursor Bugbot for commit 719dbc0. Bugbot is set up for automated code reviews on this repo. Configure here.