Conversation
There was a problem hiding this comment.
Pull request overview
该 PR 在现有 NodeWarden(Bitwarden 兼容服务)中新增了 Send 能力,基于 D1(元数据/计数)+ R2(文件内容) 实现,包含鉴权端 API、公开访问端 API 以及收件人访问页面。
Changes:
- 新增 Send 数据模型/表结构(D1 schema + StorageService CRUD)并将 Send 纳入 Sync 响应
- 新增 Send 相关 API(创建/更新/删除/访问/文件上传/文件下载)与公开收件人页面
/send/... - 新增 Send 文件下载用的短期 JWT token 支持,并更新文档说明支持 Send
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/jwt.ts | 新增 Send 文件下载 JWT 的创建/校验方法 |
| src/types/index.ts | 新增 Send/SendType/SendResponse 类型,并将 SyncResponse.sends 具体化 |
| src/setup/pageTemplate.ts | 将 /#/send/... hash 路由重写到服务端 /send/... 页面 |
| src/services/storage.ts | 增加 sends 表 schema 与 Send 的 D1 CRUD 映射 |
| src/router.ts | 增加 Send 的鉴权端路由与公开访问端路由、公开页面路由 |
| src/handlers/sync.ts | Sync 响应中填充 sends 列表 |
| src/handlers/sends.ts | 新增 Send 全套 API handlers(含文件上传/下载、访问控制、密码等) |
| src/handlers/send-public.ts | 新增收件人公开访问页面(解密/下载前端逻辑) |
| src/config/limits.ts | 增加 Send 的文件大小/删除日期限制配置 |
| migrations/0001_init.sql | 初始迁移中新增 sends 表与索引 |
| README.md | 更新功能矩阵,标注支持 Send |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!contentType.includes('multipart/form-data')) { | ||
| return errorResponse('Content-Type must be multipart/form-data', 400); | ||
| } | ||
|
|
There was a problem hiding this comment.
handleUploadSendFile calls request.formData() without a Content-Length precheck (unlike attachment uploads). With LIMITS.send.maxFileSizeBytes set to ~550MB, this can cause large request bodies to be buffered/parsed before rejection, risking worker memory/time limits. Add a content-length early reject and consider a streaming upload approach (or reduce the configured limit) to avoid buffering huge multipart bodies.
| const contentLengthHeader = request.headers.get('content-length'); | |
| if (contentLengthHeader !== null) { | |
| const contentLength = Number(contentLengthHeader); | |
| if (Number.isFinite(contentLength) && contentLength > LIMITS.send.maxFileSizeBytes) { | |
| return errorResponse('Send storage limit exceeded with this file', 413); | |
| } | |
| } |
| export interface SendFileDownloadClaims { | ||
| sendId: string; | ||
| fileId: string; | ||
| exp: number; | ||
| } | ||
|
|
||
| export async function createSendFileDownloadToken( | ||
| sendId: string, | ||
| fileId: string, | ||
| secret: string | ||
| ): Promise<string> { | ||
| const header = { alg: 'HS256', typ: 'JWT' }; | ||
| const now = Math.floor(Date.now() / 1000); | ||
| const payload: SendFileDownloadClaims = { | ||
| sendId, | ||
| fileId, | ||
| exp: now + LIMITS.auth.fileDownloadTokenTtlSeconds, | ||
| }; |
There was a problem hiding this comment.
Unlike FileDownloadClaims, SendFileDownloadClaims has no jti, so Send download tokens can be reused until they expire. Since attachment download tokens are one-time (jti + consumeAttachmentDownloadToken), consider adding a jti to Send tokens and consuming it (or otherwise preventing reuse) to reduce token leakage impact and better enforce max access semantics.
| const maxMs = Date.now() + LIMITS.send.maxDeletionDays * 24 * 60 * 60 * 1000; | ||
| if (date.getTime() > maxMs) { | ||
| return errorResponse( | ||
| 'You cannot have a Send with a deletion date that far into the future. Adjust the Deletion Date to a value less than 31 days from now and try again.', |
There was a problem hiding this comment.
validateDeletionDate hardcodes "31 days" in the error message, but the actual limit is LIMITS.send.maxDeletionDays. If the config changes, the message becomes incorrect; interpolate the configured value in the message instead of hardcoding 31.
| 'You cannot have a Send with a deletion date that far into the future. Adjust the Deletion Date to a value less than 31 days from now and try again.', | |
| `You cannot have a Send with a deletion date that far into the future. Adjust the Deletion Date to a value less than ${LIMITS.send.maxDeletionDays} days from now and try again.`, |
| async function setSendPassword(send: Send, password: string | null): Promise<void> { | ||
| if (!password) { | ||
| send.passwordHash = null; | ||
| send.passwordSalt = null; | ||
| send.passwordIterations = null; | ||
| return; | ||
| } | ||
|
|
||
| const salt = crypto.getRandomValues(new Uint8Array(64)); | ||
| const hash = await deriveSendPasswordHash(password, salt, SEND_PASSWORD_ITERATIONS); | ||
|
|
||
| send.passwordSalt = base64UrlEncode(salt); | ||
| send.passwordHash = base64UrlEncode(hash); | ||
| send.passwordIterations = SEND_PASSWORD_ITERATIONS; | ||
| } | ||
|
|
||
| async function verifySendPassword(send: Send, password: string): Promise<boolean> { | ||
| if (!send.passwordHash || !send.passwordSalt || !send.passwordIterations) { | ||
| return false; | ||
| } | ||
|
|
||
| const salt = base64UrlDecode(send.passwordSalt); | ||
| const expected = base64UrlDecode(send.passwordHash); | ||
| if (!salt || !expected) return false; | ||
|
|
||
| const actual = await deriveSendPasswordHash(password, salt, send.passwordIterations); | ||
| return constantTimeEqual(actual, expected); | ||
| } |
There was a problem hiding this comment.
Password verification here expects the request to provide the raw password (server re-hashes with stored salt/iterations). However, the new public Send page hashes the password client-side before sending it, which will make password-protected Sends fail unless the creator also submitted that pre-hashed value. Align the contract: either accept raw passwords everywhere (update the public page to send raw) or store/verify the client-side hash directly (and drop server-side PBKDF2/salt).
| name: nameRaw.value.trim(), | ||
| notes: typeof notesRaw.value === 'string' ? notesRaw.value : null, | ||
| data: JSON.stringify(sendData), | ||
| key: keyRaw.value, |
There was a problem hiding this comment.
key is validated with .trim() but then the untrimmed value is stored (key: keyRaw.value). This can persist leading/trailing whitespace and break decryption for clients that treat the key as exact bytes. Store the trimmed value consistently (e.g., keyRaw.value.trim()).
| key: keyRaw.value, | |
| key: typeof keyRaw.value === 'string' ? keyRaw.value.trim() : keyRaw.value, |
| if (typeof keyRaw.value !== 'string' || !keyRaw.value.trim()) { | ||
| return errorResponse('Key is required', 400); | ||
| } | ||
| send.key = keyRaw.value; |
There was a problem hiding this comment.
On update, key is validated with .trim() but then saved without trimming (send.key = keyRaw.value). Persist the trimmed key value so updates can’t accidentally introduce whitespace that breaks recipients’ ability to decrypt.
| if (typeof keyRaw.value !== 'string' || !keyRaw.value.trim()) { | |
| return errorResponse('Key is required', 400); | |
| } | |
| send.key = keyRaw.value; | |
| if (typeof keyRaw.value !== 'string') { | |
| return errorResponse('Key is required', 400); | |
| } | |
| const trimmedKey = keyRaw.value.trim(); | |
| if (!trimmedKey) { | |
| return errorResponse('Key is required', 400); | |
| } | |
| send.key = trimmedKey; |
| if (send.type === SendType.Text) { | ||
| send.accessCount += 1; | ||
| send.updatedAt = new Date().toISOString(); | ||
| await storage.saveSend(send); | ||
| await storage.updateRevisionDate(send.userId); | ||
| } |
There was a problem hiding this comment.
For file Sends, /api/sends/access/:accessId does not increment accessCount, so maxAccessCount can be bypassed by repeatedly calling the access endpoint (and viewing metadata) without ever downloading. Consider incrementing accessCount for file Sends on access as well (or otherwise enforce maxAccessCount consistently across access flows).
| if (send.type === SendType.Text) { | |
| send.accessCount += 1; | |
| send.updatedAt = new Date().toISOString(); | |
| await storage.saveSend(send); | |
| await storage.updateRevisionDate(send.userId); | |
| } | |
| send.accessCount += 1; | |
| send.updatedAt = new Date().toISOString(); | |
| await storage.saveSend(send); | |
| await storage.updateRevisionDate(send.userId); |
| const claims = await verifySendFileDownloadToken(token, secret); | ||
| if (!claims) { | ||
| return errorResponse('Invalid or expired token', 401); | ||
| } | ||
| if (claims.sendId !== sendId || claims.fileId !== fileId) { | ||
| return errorResponse('Token mismatch', 401); | ||
| } | ||
|
|
||
| const object = await env.ATTACHMENTS.get(getSendFilePath(sendId, fileId)); | ||
| if (!object) { | ||
| return errorResponse('Send file not found', 404); | ||
| } |
There was a problem hiding this comment.
handleDownloadSendFile only validates the JWT and R2 object; it does not re-check that the Send is still available (disabled/expired/deleted/maxAccessCount) at download time. A token issued just before expiration/disablement (or reused) can still download the file. Load the Send and enforce isSendAvailable (and fileId match) here, or make tokens one-time use like attachment downloads.
| const pwKey = await crypto.subtle.importKey('raw', textEncoder.encode(rawPassword), 'PBKDF2', false, ['deriveBits']); | ||
| const bits = await crypto.subtle.deriveBits( | ||
| { | ||
| name: 'PBKDF2', | ||
| salt: keyMaterialBytes, | ||
| iterations: SEND_PASSWORD_ITERATIONS, | ||
| hash: 'SHA-256', | ||
| }, | ||
| pwKey, | ||
| 256 | ||
| ); | ||
| return bytesToBase64(new Uint8Array(bits)); |
There was a problem hiding this comment.
The public page hashes the entered password client-side (hashSendPassword) and sends that value as password to /api/sends/access/.... The server-side implementation verifies password by hashing it again with its own salt/iterations, which expects the raw password. Adjust this page to send the raw password (over HTTPS) or change the server contract to accept/verify the client-side hash directly; otherwise password-protected Sends will not unlock reliably across clients.
| const pwKey = await crypto.subtle.importKey('raw', textEncoder.encode(rawPassword), 'PBKDF2', false, ['deriveBits']); | |
| const bits = await crypto.subtle.deriveBits( | |
| { | |
| name: 'PBKDF2', | |
| salt: keyMaterialBytes, | |
| iterations: SEND_PASSWORD_ITERATIONS, | |
| hash: 'SHA-256', | |
| }, | |
| pwKey, | |
| 256 | |
| ); | |
| return bytesToBase64(new Uint8Array(bits)); | |
| // The server expects the raw password and applies its own hashing/salting. | |
| // Keep this function async to avoid changing existing call sites. | |
| return rawPassword; |
| name: nameRaw.value.trim(), | ||
| notes: typeof notesRaw.value === 'string' ? notesRaw.value : null, | ||
| data: JSON.stringify(fileData), | ||
| key: keyRaw.value, |
There was a problem hiding this comment.
Same issue as text Send creation: key is validated as non-empty after trimming, but the untrimmed value is stored (key: keyRaw.value). Persist the trimmed key value to avoid whitespace-related mismatches during decryption.
| key: keyRaw.value, | |
| key: typeof keyRaw.value === 'string' ? keyRaw.value.trim() : keyRaw.value, |
支持了Send功能 基于R2 和D1实现