chore(brick-container): add --https flag for dev server#4879
Merged
Conversation
Walkthrough向CLI添加 ChangesHTTPS自签名证书生成
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/brick-container/serve/env.js (1)
136-153: 💤 Low value避免误把开发证书私钥提交进仓库。
dev-https.key/dev-https.cert默认生成在rootDir，若使用方未在.gitignore中忽略，私钥极易被一同提交。建议在文档/README 中说明，或在该流程中自动写入/校验项目的.gitignore，至少在生成时输出一行提示用户将其加入忽略列表。🤖 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 `@packages/brick-container/serve/env.js` around lines 136 - 153, The dev cert/key are written to rootDir as dev-https.key / dev-https.cert (keyPath/certPath) and may be accidentally committed; update the HTTPS generation flow in env.js to either (a) check the project's .gitignore and append "dev-https.key" and "dev-https.cert" if missing, or (b) at minimum print a clear one-line warning after generation advising the user to add those filenames to .gitignore; locate the logic around keyPath/certPath and the execSync generation block and implement the .gitignore check/append or the warning message there so users are prompted immediately when files are created.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@packages/brick-container/serve/env.js`:
- Around line 135-153: The SAN generation treats any non-"localhost" host as an
IP and interpolates it as IP:..., causing invalid SANs and potential command
injection; update the logic around the used variables keyPath, certPath and san
(where san is built from flags.host) to: validate and classify flags.host as
IPv4/IPv6 (use a regex or net.isIP) to choose "IP:" vs "DNS:", escape or
strictly validate hostnames (reject unsafe chars/wildcards), replace execSync
shell interpolation with execFileSync/execFile and pass openssl args as an array
(or otherwise escape paths) when calling openssl to generate the cert, and wrap
the child process call in try/catch to detect missing openssl and emit a clear
error message before exiting; leave reading keyPath/certPath and assigning
https.key/https.cert unchanged.
---
Nitpick comments:
In `@packages/brick-container/serve/env.js`:
- Around line 136-153: The dev cert/key are written to rootDir as dev-https.key
/ dev-https.cert (keyPath/certPath) and may be accidentally committed; update
the HTTPS generation flow in env.js to either (a) check the project's .gitignore
and append "dev-https.key" and "dev-https.cert" if missing, or (b) at minimum
print a clear one-line warning after generation advising the user to add those
filenames to .gitignore; locate the logic around keyPath/certPath and the
execSync generation block and implement the .gitignore check/append or the
warning message there so users are prompted immediately when files are created.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d3b36f65-7326-4ec0-b7a7-5d086b518f73
📒 Files selected for processing (1)
packages/brick-container/serve/env.js
Comment on lines
+135
to
+153
| if (!https && flags.https) { | ||
| const keyPath = path.join(rootDir, "dev-https.key"); | ||
| const certPath = path.join(rootDir, "dev-https.cert"); | ||
|
|
||
| if (!existsSync(keyPath) || !existsSync(certPath)) { | ||
| const { execSync } = await import("node:child_process"); | ||
| const san = `DNS:localhost${flags.host !== "localhost" ? ",IP:" + flags.host : ""}`; | ||
| console.log(chalk.cyan("Auto-generating self-signed certificate...")); | ||
| execSync( | ||
| `openssl req -x509 -newkey rsa:2048 -keyout "${keyPath}" -out "${certPath}" -days 365 -nodes -subj "/CN=localhost" -addext "subjectAltName=${san}"`, | ||
| { stdio: "inherit" } | ||
| ); | ||
| } | ||
|
|
||
| https = { | ||
| key: readFileSync(keyPath, "utf8"), | ||
| cert: readFileSync(certPath, "utf8"), | ||
| }; | ||
| } |
There was a problem hiding this comment.
SAN 生成逻辑对非 IP 主机名会产生无效证书。
第 141 行 DNS:localhost${flags.host !== "localhost" ? ",IP:" + flags.host : ""} 把所有非 localhost 的 --host 值都当成 IP 处理。但 --host 完全可能是一个 DNS 名（如 dev.local、mydev、*.local），此时生成的 SAN 是 IP:dev.local，openssl 会报错或生成的证书与浏览器实际访问的主机名不匹配，导致 HTTPS 校验失败。建议根据是否匹配 IPv4/IPv6 字面量来区分 DNS: 与 IP:。
此外该 openssl 命令将 flags.host、keyPath、certPath 直接拼接进 shell 字符串，若 rootDir 或 --host 含空格/shell 元字符，会出现命令注入或执行失败；建议用 execFileSync 配合参数数组，或对 host 做严格校验。同时缺少 openssl 不存在时的友好错误提示（Windows 上较常见）。
🛠️ 建议修复
if (!existsSync(keyPath) || !existsSync(certPath)) {
- const { execSync } = await import("node:child_process");
- const san = `DNS:localhost${flags.host !== "localhost" ? ",IP:" + flags.host : ""}`;
- console.log(chalk.cyan("Auto-generating self-signed certificate..."));
- execSync(
- `openssl req -x509 -newkey rsa:2048 -keyout "${keyPath}" -out "${certPath}" -days 365 -nodes -subj "/CN=localhost" -addext "subjectAltName=${san}"`,
- { stdio: "inherit" }
- );
+ const { execFileSync } = await import("node:child_process");
+ const isIPv4 = (h) => /^(\d{1,3}\.){3}\d{1,3}$/.test(h);
+ const isIPv6 = (h) => h.includes(":");
+ const sanEntries = ["DNS:localhost"];
+ if (flags.host && flags.host !== "localhost") {
+ sanEntries.push(
+ (isIPv4(flags.host) || isIPv6(flags.host) ? "IP:" : "DNS:") + flags.host
+ );
+ }
+ console.log(chalk.cyan("Auto-generating self-signed certificate..."));
+ try {
+ execFileSync(
+ "openssl",
+ [
+ "req", "-x509", "-newkey", "rsa:2048",
+ "-keyout", keyPath, "-out", certPath,
+ "-days", "365", "-nodes",
+ "-subj", "/CN=localhost",
+ "-addext", `subjectAltName=${sanEntries.join(",")}`,
+ ],
+ { stdio: "inherit" }
+ );
+ } catch (err) {
+ console.error(
+ chalk.red("Failed to generate self-signed certificate. Is `openssl` installed and on PATH?")
+ );
+ throw err;
+ }
}🤖 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 `@packages/brick-container/serve/env.js` around lines 135 - 153, The SAN
generation treats any non-"localhost" host as an IP and interpolates it as
IP:..., causing invalid SANs and potential command injection; update the logic
around the used variables keyPath, certPath and san (where san is built from
flags.host) to: validate and classify flags.host as IPv4/IPv6 (use a regex or
net.isIP) to choose "IP:" vs "DNS:", escape or strictly validate hostnames
(reject unsafe chars/wildcards), replace execSync shell interpolation with
execFileSync/execFile and pass openssl args as an array (or otherwise escape
paths) when calling openssl to generate the cert, and wrap the child process
call in try/catch to detect missing openssl and emit a clear error message
before exiting; leave reading keyPath/certPath and assigning
https.key/https.cert unchanged.
next-core
 |
||||||||||||||||||||||||||||
| Project |
next-core
|
| Branch Review |
chore/brick-container-https-flag-260513-1645
|
| Run status |
|
| Run duration | 00m 27s |
| Commit |
|
| Committer | 吃猫的鱼 |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
0
|
 |
0
|
 |
0
|
 |
17
|
| View all changes introduced in this branch ↗︎ | |
WHChen-Alex
approved these changes
May 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
依赖检查
组件之间的依赖声明，是微服务组件架构下的重要信息，请确保其正确性。
请勾选以下两组选项其中之一：
或者：
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG，请检查工作内容与提交信息是否相符，并在以下每组选项中都依次确认。
破坏性变更：
feat作为提交类型。BREAKING CHANGE: 你的变更说明。新特性：
feat作为提交类型。问题修复：
fix作为提交类型。杂项工作：
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动，例如修改注释、测试用例、开发文档等：
chore,docs,test等作为提交类型。Summary by CodeRabbit
--https命令行标志，支持启用 HTTPS 服务