Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion models/Award.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ export type Award = Record<
| 'reason'
| 'nominator'
| 'createdAt'
| 'votes',
| 'votes'
| 'walletAddress'
| 'transactionHash'
| 'tokenId',
TableCellValue
>;

Expand Down
161 changes: 161 additions & 0 deletions pages/api/Lark/award/issue.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
import { Context } from 'koa';
import { createKoaRouter, withKoaRouter } from 'next-ssr-middleware';
import { AwardModel } from '../../../../models/Award';
import { safeAPI, verifyJWT } from '../../core';

export const config = { api: { bodyParser: true } };

const router = createKoaRouter(import.meta.url);

const EthereumAddressPattern = /^0x[a-fA-F0-9]{40}$/;

const getUserIds = (field: any): string[] => {
if (!field) return [];
if (Array.isArray(field)) {
return field.map(u => (typeof u === 'object' && u ? u.id : String(u))).filter(Boolean);
}
if (typeof field === 'object' && field) {
return [field.id].filter(Boolean);
}
return [String(field)];
Comment on lines +12 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

getUserIds 的返回值真正归一化为字符串。

当前对象分支会返回原始 id,且 .filter(Boolean) 会丢掉数值 0;但 Line 64 后续用字符串 ID 做 includes,这会导致 ID 授权误判并落到展示名兜底。

建议修复
 const getUserIds = (field: any): string[] => {
   if (!field) return [];
   if (Array.isArray(field)) {
-    return field.map(u => (typeof u === 'object' && u ? u.id : String(u))).filter(Boolean);
+    return field
+      .map(u => (typeof u === 'object' && u ? u.id : u))
+      .filter(id => id !== undefined && id !== null && id !== '')
+      .map(String);
   }
   if (typeof field === 'object' && field) {
-    return [field.id].filter(Boolean);
+    return field.id === undefined || field.id === null || field.id === ''
+      ? []
+      : [String(field.id)];
   }
   return [String(field)];
 };

Also applies to: 64-69

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pages/api/Lark/award/issue.ts` around lines 12 - 20, getUserIds currently
does not fully normalize IDs to strings, and its Boolean filtering can drop
valid values like 0, which can break the later includes-based authorization
check in the award issue flow. Update getUserIds in issue.ts so every branch
returns stringified IDs consistently, and replace the truthy filter with an
explicit non-null/non-undefined check; make sure the downstream logic around the
includes check and display-name fallback continues to work with the normalized
string IDs.

};

const getUserNames = (field: any): string[] => {
if (!field) return [];
if (Array.isArray(field)) {
return field
.map(u => (typeof u === 'object' && u ? u.name || u.id : String(u)))
.filter(Boolean);
}
if (typeof field === 'object' && field) {
return [field.name || field.id].filter(Boolean);
}
return [String(field)];
};

router.post('/issue', safeAPI, verifyJWT, async (context: Context) => {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const { recordId, walletAddress } = (context.request as any).body;

if (!recordId || !walletAddress) {
context.throw(400, 'recordId and walletAddress are required');
}

if (typeof walletAddress !== 'string' || !EthereumAddressPattern.test(walletAddress)) {
context.throw(400, 'walletAddress must be a valid Ethereum address');
}

// 1. Fetch award and check authorization
const awardModel = new AwardModel();
const award = await awardModel.getOne(recordId);

if (!award) {
context.throw(404, 'Award record not found');
}

const currentUser = (context.state as any).user;
if (!currentUser) {
context.throw(401, 'Unauthorized');
}

const nominators = getUserIds(award.nominator);
const nomineeNames = getUserIds(award.nomineeName);
const nominatorNames = getUserNames(award.nominator);
const nomineeUserNames = getUserNames(award.nomineeName);
const currentUserIdStr = String(currentUser.id);

if (
currentUser.id !== 0 && // Robot bypass by stable ID 0
!nominators.includes(currentUserIdStr) &&
!nomineeNames.includes(currentUserIdStr) &&
!nominatorNames.includes(currentUser.name) &&
!nomineeUserNames.includes(currentUser.name)
) {
Comment on lines +66 to +72

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 验证 verifyJWT 写入 context.state.user 的字段,以及是否已有稳定 role/id 可复用。
rg -nP -C4 '\bverifyJWT\b|context\.state\.user|state\.user|isAdmin|role|userId|email|name' pages models --type=ts

Repository: Open-Source-Bazaar/Open-Source-Bazaar.github.io

Length of output: 47055


不要用展示名和 Robot 字符串做发放授权。

currentUser.name 是文本身份,存在重名或篡改风险。验证确认 JWT 负载中包含稳定字段 id(其中 Robot 的 ID 为 0)。请改用 currentUser.id 进行权限校验,并将奖项记录中的比对字段对应为内部 ID,确保认证逻辑的稳定性与安全性。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pages/api/Lark/award/issue.ts` around lines 36 - 40, 当前发放授权逻辑在 issueAward
相关校验里使用了 currentUser.name 与 “Robot” 展示名以及 award.nominator/award.nomineeName
做比较,存在重名和篡改风险。请改为基于 JWT 负载中的稳定字段 currentUser.id 做权限判断,并把奖项记录中参与比对的字段切换为内部 ID(包括
Robot 的 ID 0)。确保 issue.ts 中这段授权分支只依赖 ID 级别的身份校验,避免继续使用文本身份。

context.throw(403, 'You do not have permission to issue this award');
}

// 2. Concurrency Lock check
if (award.transactionHash === 'ISSUING') {
context.throw(409, 'An NFT issuance request is already in progress for this award');
}

// 3. Idempotency and Wallet binding check
if (award.transactionHash && award.tokenId) {
if (award.walletAddress && award.walletAddress !== walletAddress) {
context.throw(409, 'This award has already been issued to a different wallet address');
}
context.body = {
success: true,
transactionHash: award.transactionHash as string,
tokenId: award.tokenId as string,
};
return;
}

// 4. Acquire lock
await awardModel.updateOne(
{
transactionHash: 'ISSUING',
walletAddress,
},
recordId,
);

let transactionHash: string;
let tokenId: string;

try {
const mintApiUrl = process.env.NFT_MINT_API || 'https://api.octoken.org/mint';
const timeoutVal = parseInt(process.env.NFT_MINT_TIMEOUT || '10000', 10);
const controller = new AbortController();
const timeout = setTimeout(() => controller.abort(), timeoutVal);

const response = await fetch(mintApiUrl, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ walletAddress, recordId }),
signal: controller.signal,
}).finally(() => clearTimeout(timeout));

if (!response.ok) {
throw new Error(`NFT issuance failed with status ${response.status}`);
}

const data = await response.json();
transactionHash = data.transactionHash;
tokenId = data.tokenId;

if (!transactionHash || !tokenId) {
throw new Error('Invalid response from minting service');
}
} catch (error) {
// Revert the concurrency lock on failure
await awardModel
.updateOne(
{
transactionHash: '',
},
recordId,
)
.catch(e => console.error('Failed to revert transaction lock:', e));

const msg =
(error as Error).name === 'AbortError'
? 'NFT issuance request timed out'
: (error as Error).message || 'NFT issuance failed';
return context.throw(502, msg);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// 5. Finalize the record with actual transaction details
await awardModel.updateOne(
{
transactionHash,
tokenId,
walletAddress,
},
recordId,
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

context.body = { success: true, transactionHash, tokenId };
});

export default withKoaRouter(router);