Skip to content
Merged
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
13 changes: 12 additions & 1 deletion packages/brick-container/serve/middlewares/getMiddlewares.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import jsYaml from "js-yaml";
import { serveBricks } from "@next-core/serve-helpers";
import bootstrapJson from "./bootstrapJson.js";
import mockAuth from "./mockAuth.js";
import singleAppBootstrapJson from "./singleAppBootstrapJson.js";
import standaloneBootstrapJson from "./standaloneBootstrapJson.js";
import serveBricksWithVersions from "./serveBricksWithVersions.js";
import { serveBricks } from "@next-core/serve-helpers";
import serveAppImages from "./serveAppImages.js";

const { safeDump, JSON_SCHEMA } = jsYaml;

Expand Down Expand Up @@ -101,6 +102,16 @@ export function getPreMiddlewares(env) {
}
},
});

middlewares.push({
path: `${baseHref}sa-static/${appId}/versions/0.0.0/webroot/-/images`,
middleware: serveAppImages(env, appId),
});

middlewares.push({
path: `${baseHref}sa-static/${appId}/versions/0.0.0/webroot/-/micro-apps/${appId}/images`,
middleware: serveAppImages(env, appId),
});
}

middlewares.push({
Expand Down
24 changes: 24 additions & 0 deletions packages/brick-container/serve/middlewares/serveAppImages.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import path from "node:path";
import { tryServeFiles } from "@next-core/serve-helpers";

export default function serveAppImages({ rootDir }, appId) {
/**
* @param {import("express").Request} req
* @param {import("express").Response} res
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The JSDoc comment is missing the next parameter. The returned async function takes three parameters (req, res, next), but the JSDoc only documents two. This should be:

/**
 * @param {import("express").Request} req
 * @param {import("express").Response} res
 * @param {import("express").NextFunction} next
 */
Suggested change
* @param {import("express").Response} res
* @param {import("express").Response} res
* @param {import("express").NextFunction} next

Copilot uses AI. Check for mistakes.
*/
return async function (req, res, next) {
if (req.method !== "GET") {
next();
return;
}

tryServeFiles(
["mock-micro-apps", "apps"].map((folder) =>
path.join(rootDir, folder, appId, "dist/images", req.path)
),
req,
res,
next
);
Comment on lines +15 to +22
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

验证路径遍历安全性。

req.path 直接用于构建文件路径。虽然 path.join() 会规范化路径,且 Express 的 res.sendFile() 提供了一定的保护,但建议验证 tryServeFiles 和底层文件服务逻辑是否充分防止了路径遍历攻击(例如 ../../ 序列)。

运行以下脚本检查 tryServeFiles 的实现细节:


🏁 Script executed:

#!/bin/bash
# 查看 tryServeFiles 的完整实现以确认安全性
ast-grep --pattern 'export function tryServeFiles($$$) {
  $$$
}'

# 查找是否有路径验证逻辑
rg -nP '(normalize|resolve|relative|\.\./)' packages/serve-helpers/src/ --type=js

Length of output: 1492


Now I need to check the tryFiles function implementation to see if there's any path validation:

验证路径遍历安全性 - 确认存在安全漏洞

您的原始评论准确指出了一个实际存在的安全问题。经验证,代码确实存在路径遍历风险:

虽然 Express 的 res.sendFile() 函数本身并非固有脆弱,但当用户输入用于确定文件路径时,它可能成为目录遍历攻击的来源。

关键问题是 path.normalize 并非独立的安全解决方案——它可能移除某些冗余路径元素,但不能防止遍历攻击。在此代码中,req.path 直接来自 HTTP 请求,且 path.join() 虽然规范化路径,但 攻击者仍可使用 "../" 访问根目录。

建议修复方案: 将规范化路径与硬编码或配置的可信根目录进行比较,若比较失败则表示检测到路径遍历,应拒绝请求。应在 tryServeFiles 前或内部添加验证,确保最终解析路径以受信基路径开头。

};
}
Loading