Skip to content

Fix PR review bugs, add SetUserSettings/GetUserSettings, remove rate limiting #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PythonSmall-Q merged 1 commit into from
Mar 29, 2026
Merged

Fix PR review bugs, add SetUserSettings/GetUserSettings, remove rate limiting #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 0 additions & 59 deletions server/middleware/0.rate-limit.ts
View file
Open in desktop

This file was deleted.

9 changes: 7 additions & 2 deletions server/plugins/scheduled.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/

import { Database } from "~/utils/database";
import { Output } from "~/utils/output";

// Time constants matching auth.ts
const MILLISECONDS_PER_SECOND = 1000;
Expand All @@ -31,7 +32,7 @@ export default defineNitroPlugin((nitroApp: any) => {
const { env, context } = event;
let XMOJDatabase = new Database(env.DB);

context.waitUntil((async () => {
const cleanup = async () => {
await XMOJDatabase.Delete("short_message", {
"send_time": {
"Operator": "<=",
Expand All @@ -48,6 +49,10 @@ export default defineNitroPlugin((nitroApp: any) => {
"Value": new Date().getTime() - SESSION_EXPIRY_MS
}
});
})());
};

context.waitUntil(cleanup().catch((err: any) => {
Output.Error("Scheduled cleanup failed: " + (err?.message || String(err)));
}));
});
});
18 changes: 17 additions & 1 deletion server/routes/DeletePost.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
/* Copyright header omitted */
/*
* Copyright (C) 2023-2025 XMOJ-bbs contributors
* This file is part of XMOJ-bbs.
* XMOJ-bbs is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* XMOJ-bbs is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with XMOJ-bbs. If not, see <https://www.gnu.org/licenses/>.
*/

import { Result, ThrowErrorIfFailed } from "~/utils/resultUtils";
import { CheckParams } from "~/utils/checkParams";
import { IsAdminAsync } from "~/utils/auth";
Expand Down
18 changes: 17 additions & 1 deletion server/routes/EditBadge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
/* Copyright header omitted */
/*
* Copyright (C) 2023-2025 XMOJ-bbs contributors
* This file is part of XMOJ-bbs.
* XMOJ-bbs is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* XMOJ-bbs is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with XMOJ-bbs. If not, see <https://www.gnu.org/licenses/>.
*/

import { Result, ThrowErrorIfFailed } from "~/utils/resultUtils";
import { CheckParams } from "~/utils/checkParams";
import { IsAdminAsync, DenyEditAsync } from "~/utils/auth";
Expand Down
18 changes: 17 additions & 1 deletion server/routes/EditReply.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
/* Copyright header omitted */
/*
* Copyright (C) 2023-2025 XMOJ-bbs contributors
* This file is part of XMOJ-bbs.
* XMOJ-bbs is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* XMOJ-bbs is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with XMOJ-bbs. If not, see <https://www.gnu.org/licenses/>.
*/

import { Result, ThrowErrorIfFailed } from "~/utils/resultUtils";
import { CheckParams } from "~/utils/checkParams";
import { IsAdminAsync, IsSilencedAsync } from "~/utils/auth";
Expand Down
18 changes: 17 additions & 1 deletion server/routes/GetBBSMentionList.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
/* Copyright header omitted */
/*
* Copyright (C) 2023-2025 XMOJ-bbs contributors
* This file is part of XMOJ-bbs.
* XMOJ-bbs is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* XMOJ-bbs is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with XMOJ-bbs. If not, see <https://www.gnu.org/licenses/>.
*/

import { Result, ThrowErrorIfFailed } from "~/utils/resultUtils";

export default eventHandler(async (event) => {
Expand Down
18 changes: 17 additions & 1 deletion server/routes/GetBadge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
/* Copyright header omitted */
/*
* Copyright (C) 2023-2025 XMOJ-bbs contributors
* This file is part of XMOJ-bbs.
* XMOJ-bbs is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* XMOJ-bbs is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with XMOJ-bbs. If not, see <https://www.gnu.org/licenses/>.
*/

import { Result, ThrowErrorIfFailed } from "~/utils/resultUtils";
import { CheckParams } from "~/utils/checkParams";

Expand Down
18 changes: 17 additions & 1 deletion server/routes/GetBoards.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
/* Copyright header omitted */
/*
* Copyright (C) 2023-2025 XMOJ-bbs contributors
* This file is part of XMOJ-bbs.
* XMOJ-bbs is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* XMOJ-bbs is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with XMOJ-bbs. If not, see <https://www.gnu.org/licenses/>.
*/

import { Result, ThrowErrorIfFailed } from "~/utils/resultUtils";

const DEFAULT_LIMIT = 50;
Expand Down
18 changes: 17 additions & 1 deletion server/routes/GetMail.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
/* Copyright header omitted */
/*
* Copyright (C) 2023-2025 XMOJ-bbs contributors
* This file is part of XMOJ-bbs.
* XMOJ-bbs is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* XMOJ-bbs is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with XMOJ-bbs. If not, see <https://www.gnu.org/licenses/>.
*/

import { Result, ThrowErrorIfFailed } from "~/utils/resultUtils";
import { CheckParams } from "~/utils/checkParams";
import { sanitizeRichText } from "~/utils/sanitize";
Expand Down
18 changes: 17 additions & 1 deletion server/routes/GetMailList.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
/* Copyright header omitted */
/*
* Copyright (C) 2023-2025 XMOJ-bbs contributors
* This file is part of XMOJ-bbs.
* XMOJ-bbs is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* XMOJ-bbs is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with XMOJ-bbs. If not, see <https://www.gnu.org/licenses/>.
*/

import { Result, ThrowErrorIfFailed } from "~/utils/resultUtils";
import { decryptMessage } from "~/utils/messageEncryption";
import CryptoJS from "crypto-js";
Expand Down
52 changes: 52 additions & 0 deletions server/routes/GetUserSettings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Copyright (C) 2023-2025 XMOJ-bbs contributors
* This file is part of XMOJ-bbs.
* XMOJ-bbs is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* XMOJ-bbs is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with XMOJ-bbs. If not, see <https://www.gnu.org/licenses/>.
*/

import { Result, ThrowErrorIfFailed } from "~/utils/resultUtils";
import { Output } from "~/utils/output";

export default eventHandler(async (event) => {
try {
const { auth } = event.context;

const SettingsData: any[] = ThrowErrorIfFailed(
await auth.database.Select("user_settings", ["settings"], {
user_id: auth.username
})
);
Comment on lines +23 to +29

Copilot AI Mar 29, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This handler assumes event.context.auth is always set; if auth middleware returns early without populating it, auth.database.Select(...) will throw and the route will respond with an internal error message. Add an explicit if (!auth || !auth.database) return new Result(false, "身份验证失败"); guard before DB access.

Copilot uses AI. Check for mistakes.

if (SettingsData.length === 0) {
return new Result(true, "获得设置成功", { Settings: {} });
}

let SettingsObject: object;
try {
SettingsObject = JSON.parse(SettingsData[0]["settings"]);
Comment on lines +35 to +37

Copilot AI Mar 29, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If user_settings ever contains more than one row for a user (possible with the non-atomic insert/update in SetUserSettings), this will return an arbitrary row (SettingsData[0]). Consider enforcing uniqueness at the DB level and/or adding Limit: 1 with a deterministic ordering, or handling multiple rows explicitly (e.g., pick latest).

Suggested change
let SettingsObject: object;
try {
SettingsObject = JSON.parse(SettingsData[0]["settings"]);
// Handle potential multiple rows deterministically by selecting a specific row.
let selectedSettingsRow = SettingsData[0];
if (SettingsData.length > 1) {
const sortedSettingsData = [...SettingsData].sort((a, b) => {
const sa = typeof a.settings === "string" ? a.settings : JSON.stringify(a.settings);
const sb = typeof b.settings === "string" ? b.settings : JSON.stringify(b.settings);
return sa.localeCompare(sb);
});
selectedSettingsRow = sortedSettingsData[sortedSettingsData.length - 1];
}
let SettingsObject: object;
try {
SettingsObject = JSON.parse(selectedSettingsRow["settings"]);

Copilot uses AI. Check for mistakes.
} catch (_) {
return new Result(false, "设置数据损坏");
}
if (typeof SettingsObject !== "object" || Array.isArray(SettingsObject) || SettingsObject === null) {
Comment on lines +35 to +41

@sourcery-ai sourcery-ai Bot Mar 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Align the parsed settings type with a more precise structure instead of object.

Here you rely on SettingsObject being a non-null, non-array object, but it’s typed as object. Typing this as unknown (with runtime narrowing) or Record<string, unknown> would better reflect the checks you perform and keep the types aligned with the actual shape of the data.

Suggested implementation:

    let SettingsObject: unknown;
    try {
      SettingsObject = JSON.parse(SettingsData[0]["settings"]);
    } catch (_) {
      return new Result(false, "设置数据损坏");
    }
    if (typeof SettingsObject !== "object" || Array.isArray(SettingsObject) || SettingsObject === null) {
      return new Result(false, "设置数据损坏");
    }

    const TypedSettingsObject = SettingsObject as Record<string, unknown>;

Anywhere later in this function where SettingsObject was used as an object, switch to TypedSettingsObject so that you benefit from the more precise Record<string, unknown> type. If you prefer to keep the original variable name, you can instead reassign with a type assertion, e.g. SettingsObject = SettingsObject as Record<string, unknown>; and adjust the code accordingly.

return new Result(false, "设置数据损坏");
}

return new Result(true, "获得设置成功", { Settings: SettingsObject });
} catch (error) {
if (error instanceof Result) return error;
const errorMsg = error instanceof Error ? error.message : String(error);
Output.Error("GetUserSettings error: " + errorMsg);
return new Result(false, "获得设置失败: " + errorMsg);
Comment on lines +46 to +50

@sourcery-ai sourcery-ai Bot Mar 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): Return a generic error message instead of propagating internal error text.

The raw errorMsg is still included in the client-facing Result. To avoid leaking internal error details, keep the full message only in Output.Error and return a stable, user-friendly error string or code instead.

@cubic-dev-ai cubic-dev-ai Bot Mar 29, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Do not return raw internal exception messages to clients; return a generic failure message and keep details only in logs.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At server/routes/GetUserSettings.ts, line 50:

<comment>Do not return raw internal exception messages to clients; return a generic failure message and keep details only in logs.</comment>

<file context>
@@ -0,0 +1,52 @@
+    if (error instanceof Result) return error;
+    const errorMsg = error instanceof Error ? error.message : String(error);
+    Output.Error("GetUserSettings error: " + errorMsg);
+    return new Result(false, "获得设置失败: " + errorMsg);
+  }
+});
</file context>
Fix with Cubic

}
});
Loading
Loading