Add core BBS routes and utilities, update docs#4
Add core BBS routes and utilities, update docs#4PythonSmall-Q wants to merge 70 commits intomainfrom
Conversation
Implemented main BBS API routes for posts, replies, moderation, mentions, boards, mail, std, badges, images, and analytics. Added route generator script, scheduled cleanup plugin, and utility modules for authentication, captcha, mentions, output, and result handling. Updated middleware for unified authentication and analytics logging. Enhanced README with setup, route, and structure documentation. Updated dependencies in package.json.
Replaced for-in loops with for-of for better readability and reliability in EditReply, GetMailList, NewReply, and database utility functions. Removed unused variables and parameters in authentication, GetPost, and UploadImage routes. Also removed an unused import in mentions utility.
The Data property was destructured from the request body but never used. This commit removes it to clean up the code.
|
帮我测试一下吧,我不确定有没有问题 |
This comment was marked as resolved.
This comment was marked as resolved.
|
这个新的架构感觉挺不错,但是这种更改的量,难道真的是可以用人来审核的吗 |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
不到啊,我感觉也许?可以? |
|
@claude review |
This comment was marked as outdated.
This comment was marked as outdated.
Standardized the usage of the Result class across multiple routes to ensure consistent parameter order and error handling. Improved error logging by using Output.Error and clarified error messages. Fixed minor logic issues and improved code readability in database query construction and badge editing.
This comment was marked as outdated.
This comment was marked as outdated.
Introduces per-user rate limiting middleware, adds pagination support to board and post listing endpoints, and refactors database utilities to validate table and column names for security. Also extracts post deletion logic to a shared utility, improves output logging with sensitive data redaction, and adds initial Vitest test coverage for authentication, middleware, and database validation. Updates CI workflows and linter configuration.
This comment was marked as outdated.
This comment was marked as outdated.
Refactored GetPosts to use batch SQL queries for improved performance and reduced N+1 queries. Enhanced authentication middleware to better validate input and support distributed session cache via KV. Improved session token checking logic in utils/auth.ts, added support for distributed and in-memory caching, and fixed type handling. Added missing table/column whitelists for 'short_message'. Renamed checkPrams.ts to checkParams.ts and enabled strict mode in tsconfig. Removed redundant rate-limit middleware and improved in-memory rate limiter cleanup.
This comment was marked as outdated.
This comment was marked as outdated.
Enhances the GitHub Actions CI workflow to handle npm lockfile mismatches by attempting a fallback install and uploading the refreshed lockfile as an artifact. Also updates several dependencies and devDependencies in package.json to their latest versions.
Introduces htmlSanitizer and sanitize utilities to sanitize user-generated titles and rich text, preventing XSS and rendering issues. Updates routes to use these sanitizers for posts, replies, and badges. Improves rate limiting with KV support, strengthens error messages, and adds order column validation in database queries.
This comment was marked as outdated.
This comment was marked as outdated.
Added clamping for pagination parameters in GetBoards and GetPosts to prevent invalid values. Refactored GetPosts to avoid data mutation during reads. Enhanced session ID logging in auth to mask sensitive data and improved in-memory cache management with expiration and size limits. Minor type and variable improvements in EditBadge.
This comment was marked as outdated.
This comment was marked as outdated.
Replaces all imports of 'checkPrams' with 'checkParams' for consistency. Refactors CheckParams to support type specs, min/max, and enum validation. Enhances image upload route with stricter base64, filename, and size checks. Updates NewPost and NewReply routes for improved type safety and result handling. Improves in-memory rate limiting fallback logic.
This comment was marked as outdated.
This comment was marked as outdated.
…restoration in tests
This comment was marked as outdated.
This comment was marked as outdated.
…llowed columns in database schema
This comment was marked as outdated.
This comment was marked as outdated.
…bles; update AddMailMention to include MessageID; enhance UploadStd to prevent infinite loops with page limit
|
你先关一下additionalusage |
|
Disabled workflow |
|
我觉得差不多了,要不部署测试一下? |
…nd message; update AddMailMention to include MessageID in database update
|
Okay, but not to prod pls |
|
How about testing locally? |
…to support notifications in AddBBSMention and AddMailMention; update wrangler.toml for observability
| @@ -0,0 +1,19 @@ | |||
| /* Copyright header omitted */ | |||
| st.tokens = Math.min(CAPACITY, st.tokens + elapsed * REFILL_PER_SEC); | ||
| st.last = now; | ||
| if (st.tokens < 1) { | ||
| throw createError({ statusCode: 429, message: '请求过于频繁,请稍后重试' }); |
There was a problem hiding this comment.
Do we really need rate limiting? We can configure it at the Cloudflare level, and it would not count towards workers usage.
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| node: [18, 20, 22] |
There was a problem hiding this comment.
We only need to support one version, currently 20.
https://github.com/XMOJ-Script-dev/XMOJ-bbs/actions/runs/22307076070/job/64528953025
…HTTP polling endpoint for notifications
…ons; update build script and configuration for deployment
…pt compilation and update wrangler.toml for SQLite class migration; set default limit in GetPosts route
|
你要不帮帮我秀秀bug叭我没招了 |
| const jsSource = doSource | ||
| // Remove the entire copyright header block | ||
| .replace(/\/\*[\s\S]*?\*\/\s*\n*/g, '') | ||
| // Remove interface definitions | ||
| .replace(/interface\s+\w+\s*{[\s\S]*?}\n/g, '') | ||
| // Remove access modifiers (private, public, protected) | ||
| .replace(/\b(private|public|protected)\s+/g, '') | ||
| // Remove TypeScript type annotations on variables | ||
| .replace(/:\s*DurableObjectState\b/g, '') | ||
| .replace(/:\s*any\b/g, '') | ||
| .replace(/:\s*string\b/g, '') | ||
| .replace(/:\s*number\b/g, '') | ||
| .replace(/:\s*boolean\b/g, '') | ||
| .replace(/:\s*Record<[^>]*>\b/g, '') | ||
| .replace(/:\s*Notification\b/g, '') | ||
| .replace(/:\s*Map<[^>]*>\b/g, '') | ||
| .replace(/:\s*Promise<[^>]*>\b/g, '') | ||
| // Remove return type annotations on methods | ||
| .replace(/\):\s*Promise<Response>/g, ')') | ||
| .replace(/\):\s*Promise<void>/g, ')') | ||
| .replace(/\):\s*void/g, ')') | ||
| .replace(/\):\s*Response/g, ')') | ||
| .replace(/\):\s*Notification\[\]/g, ')') | ||
| // Remove type casts with 'as' | ||
| .replace(/\s+as\s+[A-Za-z<>,\s]*/g, '') | ||
| // Remove generic type parameters but keep the identifiers | ||
| .replace(/<[^>]*>/g, '') |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
Copilot Autofix
AI 29 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
|
|
||
| try { | ||
| // Use esbuild to transpile TypeScript to JavaScript | ||
| execSync(`npx esbuild ${sourceFile} --outfile=${tmpFile} --format=esm --target=es2020`, { |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 29 days ago
In general, to fix this kind of problem you should avoid interpolating dynamic values into a shell command string that is executed via a shell. Instead, call the program directly (execFileSync/spawnSync) and pass dynamic components as separate arguments, so they are not interpreted by the shell and are treated purely as data.
For this specific script, the best fix is to replace the execSync call that builds a command string:
execSync(`npx esbuild ${sourceFile} --outfile=${tmpFile} --format=esm --target=es2020`, {
stdio: 'pipe'
});with a call that invokes npx directly and provides esbuild, sourceFile, and its options as an argument array:
execSync('npx', [
'esbuild',
sourceFile,
`--outfile=${tmpFile}`,
'--format=esm',
'--target=es2020'
], {
stdio: 'pipe'
});Node’s child_process.execSync supports both a string (shell) form and a (command, args?, options?) form; switching to the latter avoids the shell entirely. No new imports are needed, and existing behavior (running npx esbuild with the same flags and capturing output) is preserved. The only code region to change is the execSync call around line 30 in scripts/export-durable-objects.js.
| @@ -27,7 +27,13 @@ | ||
|
|
||
| try { | ||
| // Use esbuild to transpile TypeScript to JavaScript | ||
| execSync(`npx esbuild ${sourceFile} --outfile=${tmpFile} --format=esm --target=es2020`, { | ||
| execSync('npx', [ | ||
| 'esbuild', | ||
| sourceFile, | ||
| `--outfile=${tmpFile}`, | ||
| '--format=esm', | ||
| '--target=es2020' | ||
| ], { | ||
| stdio: 'pipe' | ||
| }); | ||
|
|
There was a problem hiding this comment.
9 issues found across 21 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/durable-objects.ts">
<violation number="1" location="server/durable-objects.ts:133">
P2: `await request.json()` will throw on malformed/empty bodies, producing an unhandled 500. Wrap in try-catch and return a 400 on parse failure.</violation>
</file>
<file name="server/routes/NewReply.ts">
<violation number="1" location="server/routes/NewReply.ts:101">
P2: Duplicate DB queries and push notifications if the post author is explicitly mentioned in the reply. Check if the author is already in `MentionPeople` before calling `AddBBSMention` for them.</violation>
</file>
<file name="scripts/export-durable-objects.js">
<violation number="1" location="scripts/export-durable-objects.js:23">
P1: The exact string match for the export statement fails because `esbuild` includes whitespace and newlines in its output. This causes the code to be appended multiple times on subsequent runs, leading to duplicate export errors.</violation>
</file>
<file name="scripts/postbuild.js">
<violation number="1" location="scripts/postbuild.js:38">
P0: Bug: `readonly` keyword is not stripped during the TS→JS conversion, producing invalid JavaScript. The source file uses `private readonly` on multiple class fields. After removing `private`/`public`/`protected`, the `readonly` keyword remains (e.g., `readonly state`, `static readonly MAX_SESSIONS_PER_USER`), which is not valid JS syntax and will cause a runtime parse error.</violation>
<violation number="2" location="scripts/postbuild.js:56">
P0: Bug: The `as` type-cast removal regex has an incomplete character class that fails on union types and object type literals. For `value as WebSocket | undefined`, the `|` is not in `[A-Za-z<>,\s]` so the match stops early, leaving `value| undefined` (a bitwise OR producing `0` instead of the intended value). For `as { userId: string; ... }`, the `{` stops the match, leaving a dangling object literal that causes a parse error. Consider a more robust approach, e.g., matching to the end of the expression or using a proper TS transpiler.</violation>
<violation number="3" location="scripts/postbuild.js:58">
P1: Bug: Nested generic types like `Map<string, Set<WebSocket>>` are not properly handled. The `[^>]*` pattern in both the specific type removal (`:\s*Map<[^>]*>\b`) and the generic catch-all (`<[^>]*>`) only consumes up to the first `>`, leaving a stray `>` and producing invalid output like `Map>` or `: Map>`.</violation>
</file>
<file name="server/routes/LockPost.ts">
<violation number="1" location="server/routes/LockPost.ts:22">
P3: Dead code: the `cloudflare` check is an impossible branch since `auth` presence guarantees `cloudflare` is defined.</violation>
</file>
<file name="server/utils/mentions.ts">
<violation number="1" location="server/utils/mentions.ts:84">
P1: Race-condition fallback logic fails because thrown `Result` object evaluates to `[object Object]`.</violation>
<violation number="2" location="server/utils/mentions.ts:146">
P1: Race-condition fallback logic fails because thrown `Result` object evaluates to `[object Object]`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| .replace(/\):\s*Response/g, ')') | ||
| .replace(/\):\s*Notification\[\]/g, ')') | ||
| // Remove type casts with 'as' | ||
| .replace(/\s+as\s+[A-Za-z<>,\s]*/g, '') |
There was a problem hiding this comment.
P0: Bug: The as type-cast removal regex has an incomplete character class that fails on union types and object type literals. For value as WebSocket | undefined, the | is not in [A-Za-z<>,\s] so the match stops early, leaving value| undefined (a bitwise OR producing 0 instead of the intended value). For as { userId: string; ... }, the { stops the match, leaving a dangling object literal that causes a parse error. Consider a more robust approach, e.g., matching to the end of the expression or using a proper TS transpiler.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/postbuild.js, line 56:
<comment>Bug: The `as` type-cast removal regex has an incomplete character class that fails on union types and object type literals. For `value as WebSocket | undefined`, the `|` is not in `[A-Za-z<>,\s]` so the match stops early, leaving `value| undefined` (a bitwise OR producing `0` instead of the intended value). For `as { userId: string; ... }`, the `{` stops the match, leaving a dangling object literal that causes a parse error. Consider a more robust approach, e.g., matching to the end of the expression or using a proper TS transpiler.</comment>
<file context>
@@ -0,0 +1,67 @@
+ .replace(/\):\s*Response/g, ')')
+ .replace(/\):\s*Notification\[\]/g, ')')
+ // Remove type casts with 'as'
+ .replace(/\s+as\s+[A-Za-z<>,\s]*/g, '')
+ // Remove generic type parameters but keep the identifiers
+ .replace(/<[^>]*>/g, '')
</file context>
| // Remove interface definitions | ||
| .replace(/interface\s+\w+\s*{[\s\S]*?}\n/g, '') | ||
| // Remove access modifiers (private, public, protected) | ||
| .replace(/\b(private|public|protected)\s+/g, '') |
There was a problem hiding this comment.
P0: Bug: readonly keyword is not stripped during the TS→JS conversion, producing invalid JavaScript. The source file uses private readonly on multiple class fields. After removing private/public/protected, the readonly keyword remains (e.g., readonly state, static readonly MAX_SESSIONS_PER_USER), which is not valid JS syntax and will cause a runtime parse error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/postbuild.js, line 38:
<comment>Bug: `readonly` keyword is not stripped during the TS→JS conversion, producing invalid JavaScript. The source file uses `private readonly` on multiple class fields. After removing `private`/`public`/`protected`, the `readonly` keyword remains (e.g., `readonly state`, `static readonly MAX_SESSIONS_PER_USER`), which is not valid JS syntax and will cause a runtime parse error.</comment>
<file context>
@@ -0,0 +1,67 @@
+ // Remove interface definitions
+ .replace(/interface\s+\w+\s*{[\s\S]*?}\n/g, '')
+ // Remove access modifiers (private, public, protected)
+ .replace(/\b(private|public|protected)\s+/g, '')
+ // Remove TypeScript type annotations on variables
+ .replace(/:\s*DurableObjectState\b/g, '')
</file context>
|
|
||
| // Check if already exported | ||
| const currentIndex = fs.readFileSync(indexPath, 'utf8'); | ||
| if (currentIndex.includes('export{NotificationManager')) { |
There was a problem hiding this comment.
P1: The exact string match for the export statement fails because esbuild includes whitespace and newlines in its output. This causes the code to be appended multiple times on subsequent runs, leading to duplicate export errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/export-durable-objects.js, line 23:
<comment>The exact string match for the export statement fails because `esbuild` includes whitespace and newlines in its output. This causes the code to be appended multiple times on subsequent runs, leading to duplicate export errors.</comment>
<file context>
@@ -0,0 +1,48 @@
+
+// Check if already exported
+const currentIndex = fs.readFileSync(indexPath, 'utf8');
+if (currentIndex.includes('export{NotificationManager')) {
+ console.log('✅ NotificationManager already exported');
+ process.exit(0);
</file context>
| // Remove type casts with 'as' | ||
| .replace(/\s+as\s+[A-Za-z<>,\s]*/g, '') | ||
| // Remove generic type parameters but keep the identifiers | ||
| .replace(/<[^>]*>/g, '') |
There was a problem hiding this comment.
P1: Bug: Nested generic types like Map<string, Set<WebSocket>> are not properly handled. The [^>]* pattern in both the specific type removal (:\s*Map<[^>]*>\b) and the generic catch-all (<[^>]*>) only consumes up to the first >, leaving a stray > and producing invalid output like Map> or : Map>.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/postbuild.js, line 58:
<comment>Bug: Nested generic types like `Map<string, Set<WebSocket>>` are not properly handled. The `[^>]*` pattern in both the specific type removal (`:\s*Map<[^>]*>\b`) and the generic catch-all (`<[^>]*>`) only consumes up to the first `>`, leaving a stray `>` and producing invalid output like `Map>` or `: Map>`.</comment>
<file context>
@@ -0,0 +1,67 @@
+ // Remove type casts with 'as'
+ .replace(/\s+as\s+[A-Za-z<>,\s]*/g, '')
+ // Remove generic type parameters but keep the identifiers
+ .replace(/<[^>]*>/g, '')
+ // Clean up extra whitespace
+ .replace(/\s+/g, ' ')
</file context>
| ): Promise<void> { | ||
| // Use INSERT OR REPLACE pattern to handle race conditions atomically | ||
| try { | ||
| const result = ThrowErrorIfFailed(await XMOJDatabase.Insert("short_message_mention", { |
There was a problem hiding this comment.
P1: Race-condition fallback logic fails because thrown Result object evaluates to [object Object].
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/utils/mentions.ts, line 146:
<comment>Race-condition fallback logic fails because thrown `Result` object evaluates to `[object Object]`.</comment>
<file context>
@@ -65,28 +137,50 @@ export async function AddMailMention(
// Use INSERT OR REPLACE pattern to handle race conditions atomically
try {
- await XMOJDatabase.Insert("short_message_mention", {
+ const result = ThrowErrorIfFailed(await XMOJDatabase.Insert("short_message_mention", {
message_id: MessageID,
from_user_id: FromUserID,
</file context>
| } | ||
| // Use INSERT OR REPLACE pattern to handle race conditions atomically | ||
| try { | ||
| const result = ThrowErrorIfFailed(await XMOJDatabase.Insert("bbs_mention", { |
There was a problem hiding this comment.
P1: Race-condition fallback logic fails because thrown Result object evaluates to [object Object].
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/utils/mentions.ts, line 84:
<comment>Race-condition fallback logic fails because thrown `Result` object evaluates to `[object Object]`.</comment>
<file context>
@@ -19,25 +19,88 @@ import { ThrowErrorIfFailed } from "~/utils/resultUtils";
// Use INSERT OR REPLACE pattern to handle race conditions atomically
try {
- await XMOJDatabase.Insert("bbs_mention", {
+ const result = ThrowErrorIfFailed(await XMOJDatabase.Insert("bbs_mention", {
to_user_id: ToUserID,
from_user_id: FromUserID,
</file context>
| return new Response('Unauthorized', { status: 401 }) | ||
| } | ||
|
|
||
| const body = (await request.json()) as { userId: string; notification: object } |
There was a problem hiding this comment.
P2: await request.json() will throw on malformed/empty bodies, producing an unhandled 500. Wrap in try-catch and return a 400 on parse failure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/durable-objects.ts, line 133:
<comment>`await request.json()` will throw on malformed/empty bodies, producing an unhandled 500. Wrap in try-catch and return a 400 on parse failure.</comment>
<file context>
@@ -0,0 +1,207 @@
+ return new Response('Unauthorized', { status: 401 })
+ }
+
+ const body = (await request.json()) as { userId: string; notification: object }
+ const userSessions = this.sessions.get(body.userId)
+ if (userSessions) {
</file context>
| } | ||
|
|
||
| if ((Post as any[])[0]["user_id"] !== auth.username) { | ||
| await AddBBSMention((Post as any[])[0]["user_id"], auth.username, Data.PostID, ReplyID, auth.database, (auth as any).notificationNamespace, (auth as any).notificationToken); |
There was a problem hiding this comment.
P2: Duplicate DB queries and push notifications if the post author is explicitly mentioned in the reply. Check if the author is already in MentionPeople before calling AddBBSMention for them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/routes/NewReply.ts, line 101:
<comment>Duplicate DB queries and push notifications if the post author is explicitly mentioned in the reply. Check if the author is already in `MentionPeople` before calling `AddBBSMention` for them.</comment>
<file context>
@@ -85,11 +94,11 @@ export default eventHandler(async (event: any) => {
if ((Post as any[])[0]["user_id"] !== auth.username) {
- await AddBBSMention((Post as any[])[0]["user_id"], auth.username, Data.PostID, ReplyID, auth.database);
+ await AddBBSMention((Post as any[])[0]["user_id"], auth.username, Data.PostID, ReplyID, auth.database, (auth as any).notificationNamespace, (auth as any).notificationToken);
}
</file context>
| await AddBBSMention((Post as any[])[0]["user_id"], auth.username, Data.PostID, ReplyID, auth.database, (auth as any).notificationNamespace, (auth as any).notificationToken); | |
| if (!MentionPeople.includes((Post as any[])[0]["user_id"])) { | |
| await AddBBSMention((Post as any[])[0]["user_id"], auth.username, Data.PostID, ReplyID, auth.database, (auth as any).notificationNamespace, (auth as any).notificationToken); | |
| } |
| return new Result(false, "认证失败"); | ||
| } | ||
|
|
||
| if (!cloudflare) { |
There was a problem hiding this comment.
P3: Dead code: the cloudflare check is an impossible branch since auth presence guarantees cloudflare is defined.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/routes/LockPost.ts, line 22:
<comment>Dead code: the `cloudflare` check is an impossible branch since `auth` presence guarantees `cloudflare` is defined.</comment>
<file context>
@@ -14,6 +14,15 @@ export default eventHandler(async (event) => {
+ return new Result(false, "认证失败");
+ }
+
+ if (!cloudflare) {
+ return new Result(false, "服务器配置错误");
+ }
</file context>
Sadly I have a lot of homework to do... |




写了三个月,希望没有问题