Skip to content

Conversation

@web3gaoyutang
Copy link
Contributor

Pull Request | PR 提交

📝 Description | 描述

English:
Enhanced the start.sh script to automatically generate strong passwords for NOFX_ADMIN_PASSWORD and jwt_secret when creating configuration files from templates. This improves security by ensuring that default credentials are never used in production environments.

中文:
增强了 start.sh 脚本,在从模板创建配置文件时自动为 NOFX_ADMIN_PASSWORDjwt_secret 生成强密码。这通过确保生产环境中永远不使用默认凭据来提高安全性。


🎯 Type of Change | 变更类型

  • 🐛 Bug fix | 修复 Bug
  • ✨ New feature | 新功能
  • 💥 Breaking change | 破坏性变更
  • 📝 Documentation update | 文档更新
  • 🎨 Code style update | 代码样式更新
  • ♻️ Refactoring | 重构
  • ⚡ Performance improvement | 性能优化
  • ✅ Test update | 测试更新
  • 🔧 Build/config change | 构建/配置变更
  • 🔒 Security fix | 安全修复

🔗 Related Issues | 相关 Issue

  • Closes # | 关闭 #
  • Related to # | 相关 #

📋 Changes Made | 具体变更

English:

  • Implemented generate_strong_password() utility function that generates 64-character passwords using OpenSSL (with /dev/urandom fallback)
  • Modified check_env() to automatically generate and replace NOFX_ADMIN_PASSWORD in .env file when created from template
  • Modified check_config() to automatically generate and replace jwt_secret in config.json file when created from template
  • Added cross-platform support (macOS and Linux) for sed commands
  • Added jq support for safer JSON manipulation with fallback to sed
  • Enhanced user feedback messages to inform about password generation

中文:

  • 实现了 generate_strong_password() 工具函数,使用 OpenSSL(带 /dev/urandom 回退方案)生成 64 字符强密码
  • 修改 check_env() 函数,在从模板创建 .env 文件时自动生成并替换 NOFX_ADMIN_PASSWORD
  • 修改 check_config() 函数,在从模板创建 config.json 文件时自动生成并替换 jwt_secret
  • 添加跨平台支持(macOS 和 Linux)的 sed 命令处理
  • 添加 jq 支持以更安全地操作 JSON,并提供 sed 回退方案
  • 增强用户反馈消息,告知密码生成情况

🧪 Testing | 测试

  • Tested locally | 本地测试通过
  • Tests pass | 测试通过
  • Verified no existing functionality broke | 确认没有破坏现有功能

测试步骤 | Testing Steps:

  1. 删除现有的 .envconfig.json 文件
  2. 运行 ./start.sh start 命令
  3. 验证 .env 文件中的 NOFX_ADMIN_PASSWORD 已被自动生成的强密码替换
  4. 验证 config.json 文件中的 jwt_secret 已被自动生成的强密码替换
  5. 验证生成的密码长度为 64 字符且包含随机字符

✅ Checklist | 检查清单

Code Quality | 代码质量

  • Code follows project style | 代码遵循项目风格
  • Self-review completed | 已完成代码自查
  • Comments added for complex logic | 已添加必要注释

Documentation | 文档

  • Updated relevant documentation | 已更新相关文档

Git

  • Commits follow conventional format | 提交遵循 Conventional Commits 格式
  • Rebased on latest dev branch | 已 rebase 到最新 dev 分支
  • No merge conflicts | 无合并冲突

📚 Additional Notes | 补充说明

English:

  • The password generation function uses OpenSSL's rand -base64 command as the primary method, which generates cryptographically secure random passwords
  • Falls back to /dev/urandom if OpenSSL is not available
  • The implementation supports both macOS (BSD sed) and Linux (GNU sed) for maximum compatibility
  • JSON manipulation prefers jq for safety but falls back to sed if jq is not installed
  • Generated passwords are 64 characters long, providing strong security for both admin passwords and JWT secrets

中文:

  • 密码生成函数使用 OpenSSL 的 rand -base64 命令作为主要方法,生成加密安全的随机密码
  • 如果 OpenSSL 不可用,则回退到 /dev/urandom
  • 实现支持 macOS(BSD sed)和 Linux(GNU sed),以确保最大兼容性
  • JSON 操作优先使用 jq 以确保安全,但如果未安装 jq 则回退到 sed
  • 生成的密码长度为 64 字符,为管理员密码和 JWT 密钥提供强大的安全性

By submitting this PR, I confirm | 提交此 PR,我确认:

  • I have read the Contributing Guidelines | 已阅读贡献指南
  • I agree to the Code of Conduct | 同意行为准则
  • My contribution is licensed under AGPL-3.0 | 贡献遵循 AGPL-3.0 许可证

🌟 Thank you for your contribution! | 感谢你的贡献!

- Implemented a utility function to generate a strong 64-character password using OpenSSL or /dev/urandom as a fallback.
- Automatically generate and replace NOFX_ADMIN_PASSWORD in .env and jwt_secret in config.json when these files are created from templates.
- Enhanced user experience by providing feedback on password generation during setup.
@xqliu
Copy link
Contributor

xqliu commented Nov 8, 2025

🔒 PR #722 代码审查报告

📊 审查结果:⚠️ 需要修复(BLOCKING 问题)


❌ BLOCKING 级别问题

1. 业务逻辑错误:替换不存在的字段

严重程度:🔴 CRITICAL

问题描述

  • 代码尝试替换 .env 文件中的 NOFX_ADMIN_PASSWORD,但该字段在 .env.example 模板中不存在
  • 执行 sed -i "s/NOFX_ADMIN_PASSWORD=.*/NOFX_ADMIN_PASSWORD=${ADMIN_PASSWORD}/" .env 时,由于没有匹配项,密码不会被写入文件
  • 后端代码中也没有使用 NOFX_ADMIN_PASSWORD 环境变量(Go 代码中搜索结果为空)

证据

# .env.example 内容(完整)
NOFX_BACKEND_PORT=8080
NOFX_FRONTEND_PORT=3000
NOFX_TIMEZONE=Asia/Shanghai
# ❌ 没有 NOFX_ADMIN_PASSWORD 字段!

影响

  • 用户运行 ./start.sh start 后,虽然提示"已自动生成强密码 NOFX_ADMIN_PASSWORD",但 .env 文件中实际没有这个密码
  • 功能完全失效,起不到任何安全保护作用
  • 误导用户认为密码已生成

修复建议

# 方案1:在模板中添加占位符(推荐)
echo "NOFX_ADMIN_PASSWORD=change_me_on_first_run" >> .env.example

# 方案2:使用追加而非替换
echo "NOFX_ADMIN_PASSWORD=${ADMIN_PASSWORD}" >> .env

2. 安全漏洞:Sed 正则转义不完整

严重程度:🟠 HIGH

问题描述
转义表达式 s/[[\.*^$()+?{|]/\\&/g 存在语法错误和遗漏:

  • 字符类 [...] 中缺少对 ] 的转义
  • 未转义关键字符:&\/(sed 分隔符)、!=

测试结果

$ echo 'Test: $()[]{}|;:<>?' | sed 's/[[\.*^$()+?{|]/\\&/g'
# 输出:Test: \$\(\)\[]\{}\|;:<>\?
# ❌ 右括号 ] 未转义,{} 转义错误

潜在风险

  • 如果生成的密码包含 /&\ 等字符,sed 命令会执行失败
  • 例如:密码 a/b&c\d 会导致 JSON 更新失败

修复建议

# 正确的转义方式
ESCAPED_SECRET=$(printf '%s\n' "$JWT_SECRET" | sed -e 's/[]\/$*.^[]/\\&/g')

# 或使用 Perl 风格
ESCAPED_SECRET=$(echo "$JWT_SECRET" | sed 's/[&/\]/\\&/g')

3. 安全最佳实践缺失:密码明文存储在环境变量中

严重程度:🟡 MEDIUM

问题描述

  • 生成的强密码直接存储在 .env 文件中(明文)
  • Bash 变量 ADMIN_PASSWORDJWT_SECRET 在内存中明文存在
  • 未验证文件权限(.env 文件应该是 600 权限)

安全风险

  • .env 文件如果被意外提交到 Git,密码泄露
  • 其他用户/进程可能读取 .env 文件
  • 容器日志可能暴露这些变量

修复建议

# 1. 设置文件权限
chmod 600 .env
chmod 600 config.json

# 2. 添加 .env 到 .gitignore(验证已存在)
grep -q "^\.env$" .gitignore || echo ".env" >> .gitignore

# 3. 清理内存中的敏感变量
unset ADMIN_PASSWORD
unset JWT_SECRET

⚠️ 次要问题

4. 代码质量:密码强度不足

问题

openssl rand -base64 48 | tr -d "=+/" | cut -c1-64
  • Base64 编码后删除 =+/,实际字符集只有 [A-Za-z0-9](62 种字符)
  • 删除字符后可能导致长度不足 64 位(如果原始随机数包含很多 =+/

建议

# 方案1:使用更强的字符集
openssl rand -hex 32  # 生成 64 字符的十六进制(0-9a-f)

# 方案2:保留特殊字符
openssl rand -base64 64 | head -c 64  # 不删除特殊字符

5. 容错性问题:缺少错误处理

问题

  • generate_strong_password() 如果 OpenSSL 和 /dev/urandom 都失败,会返回空字符串
  • jq 命令失败后回退到 sed,但没有验证 sed 是否成功
  • 文件操作(cpmv)没有错误检查

建议

generate_strong_password() {
    local password
    if command -v openssl &> /dev/null; then
        password=$(openssl rand -base64 48 | tr -d "=+/" | cut -c1-64)
    elif [ -r /dev/urandom ]; then
        password=$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 64 | head -n 1)
    else
        print_error "无法生成随机密码:OpenSSL 和 /dev/urandom 均不可用"
        exit 1
    fi
    
    # 验证密码长度
    if [ ${#password} -lt 32 ]; then
        print_error "生成的密码长度不足(${#password} < 32)"
        exit 1
    fi
    
    echo "$password"
}

✅ 优点

  1. 安全意识强:主动解决默认密码问题,提升安全基线
  2. 跨平台兼容:正确处理 macOS(BSD sed)和 Linux(GNU sed)差异
  3. 降级方案:OpenSSL → /dev/urandom → sed/jq 的多重回退
  4. 用户体验:自动化密码生成,减少手动配置错误

🔍 安全审查(Web3 AI 交易系统专项)

✅ 通过的检查

  • 无私钥/助记词硬编码(grep 验证通过)
  • 密码不在日志中输出(仅输出提示消息,不输出实际密码)
  • 使用加密安全的随机源(OpenSSL、/dev/urandom)

⚠️ 需要改进

  • 密钥存储.env 明文存储 JWT_SECRET,建议使用加密密钥管理(如已有的 RSA 密钥体系)
  • 文件权限:未强制 .envconfig.json 的权限为 600
  • 审计日志:密码生成事件未记录(建议记录时间戳、操作者、文件哈希)

📋 修复清单(按优先级)

🔴 必须修复(BLOCKING)

  1. .env.example 中添加 NOFX_ADMIN_PASSWORD 占位符

    • 文件:.env.example
    • 行动:添加 NOFX_ADMIN_PASSWORD=CHANGE_ME
  2. 修复 sed 正则转义逻辑

    • 文件:start.sh:115-119
    • 行动:使用 printf '%s\n' "$JWT_SECRET" | sed -e 's/[]\/$*.^[]/\\&/g'
  3. 添加密码生成验证

    • 文件:start.sh:41-48
    • 行动:检查密码长度、非空、字符集

🟠 强烈建议修复

  1. 设置文件权限

    • check_env()check_config() 末尾添加 chmod 600
  2. 清理敏感变量

    • 使用后执行 unset ADMIN_PASSWORD JWT_SECRET

🎯 最小修复建议(Diff 格式)

# .env.example
+ NOFX_ADMIN_PASSWORD=CHANGE_ME_ON_FIRST_RUN

# start.sh:41-48
 generate_strong_password() {
-    if command -v openssl &> /dev/null; then
-        openssl rand -base64 48 | tr -d "=+/" | cut -c1-64
+    local password
+    if command -v openssl &> /dev/null; then
+        password=$(openssl rand -hex 32)
+    elif [ -r /dev/urandom ]; then
+        password=$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 64 | head -n 1)
     else
-        cat /dev/urandom | tr -dc 'a-zA-Z0-9!@#$%^&*()_+-=[]{}|;:,.<>?' | fold -w 64 | head -n 1
+        print_error "无法生成密码"
+        exit 1
     fi
+    [ ${#password} -ge 32 ] || exit 1
+    echo "$password"
 }

# start.sh:115
-        ESCAPED_SECRET=$(echo "$JWT_SECRET" | sed 's/[[\.*^$()+?{|]/\\&/g')
+        ESCAPED_SECRET=$(printf '%s\n' "$JWT_SECRET" | sed 's/[]\/$*.^[]/\\&/g')

# start.sh:97 (check_env 末尾)
+        chmod 600 .env
+        unset ADMIN_PASSWORD

# start.sh:129 (check_config 末尾)
+        chmod 600 config.json
+        unset JWT_SECRET

🚨 E2E 验证报告

✅ 用户流程验证

测试场景:新用户首次运行 ./start.sh start

预期流程:

  1. 用户运行 ./start.sh start
  2. 脚本检测到 .env 不存在
  3. 复制 .env.example.env
  4. 生成强密码并替换 NOFX_ADMIN_PASSWORD
  5. 用户启动服务,使用新密码登录

❌ 实际结果(当前代码):

  1. ✅ 脚本复制 .env.example.env
  2. sed 命令无匹配项(文件中无 NOFX_ADMIN_PASSWORD),替换失败
  3. .env 文件中没有管理员密码
  4. ✅ 显示提示消息"已自动生成强密码"(误导性)
  5. 用户无法使用密码登录(密码不存在)

验证命令

# 复现问题
rm .env
./start.sh start
grep NOFX_ADMIN_PASSWORD .env  # ❌ 输出为空!

🎓 根本原因分析

为什么会出现这个问题?

  1. 需求验证不足:未验证 NOFX_ADMIN_PASSWORD 是否在后端代码中使用
  2. 单元测试缺失:未测试 sed 替换是否成功
  3. E2E 验证缺失:未实际运行 ./start.sh 并检查生成的 .env 文件内容

如何预防?

# 添加集成测试
test_password_generation() {
    rm -f .env config.json
    ./start.sh start
    
    # 验证密码存在
    grep -q "^NOFX_ADMIN_PASSWORD=" .env || { echo "FAIL: Password not generated"; exit 1; }
    
    # 验证密码长度
    password=$(grep "^NOFX_ADMIN_PASSWORD=" .env | cut -d'=' -f2)
    [ ${#password} -ge 32 ] || { echo "FAIL: Password too short"; exit 1; }
    
    echo "PASS: Password generation test"
}

📚 参考资料

安全最佳实践

Bash 最佳实践


🏁 总结

当前状态⚠️ 不能合并 - 存在功能性 Bug

修复工作量:约 30 分钟(添加模板字段 + 修复转义逻辑 + 添加验证)

修复后价值:✅ 显著提升部署安全性,解决默认密码问题

下一步行动

  1. PR 作者修复 BLOCKING 问题(1-3)
  2. Code Review 通过后添加集成测试
  3. 重新审查并合并

感谢你的安全贡献!修复后这将是一个非常有价值的安全增强功能。如有疑问欢迎讨论。

审查人: Claude Code Agent
审查时间: 2025-11-08
审查标准: /20-code-review (业务逻辑、技术架构、安全性、E2E验证)

@xqliu
Copy link
Contributor

xqliu commented Nov 8, 2025

代码审查报告 - PR #722

审查结果:⚠️ 需要修复

问题清单

🔴 BLOCKING - 安全问题

问题1:.env.example 中不存在 NOFX_ADMIN_PASSWORD 字段

  • 位置start.sh:89-92

  • 问题描述

    • 代码尝试替换 NOFX_ADMIN_PASSWORD=.*
    • .env.example 文件中根本不存在这个字段
    • sed 命令不会报错,但也不会写入任何内容
    • 最终 .env 中缺少 NOFX_ADMIN_PASSWORD 配置
  • 验证证据

    $ grep NOFX_ADMIN_PASSWORD .env.example
    # (无结果)

问题2:密码生成后没有显示给用户

  • 位置start.sh:96

  • 问题描述

    • 生成的强密码只提示"已自动生成",但没有显示实际值
    • 用户无法知道管理员密码是什么
    • 需要手动查看 .envconfig.json 才能获取密码
  • 安全隐患

    • 用户可能不知道密码已生成,继续使用示例密码
    • 或者找不到密码,导致无法登录

问题3:sed 转义不完整可能导致注入

  • 位置start.sh:116-122

  • 问题描述

    • ESCAPED_SECRET 只转义了正则特殊字符 [\.*^$()+?{|
    • 但 sed 替换字符串中还需要转义 /&
    • 如果随机生成的密码包含 /&,sed 命令会失败或产生意外结果
  • 修复建议

    ESCAPED_SECRET=$(echo "$JWT_SECRET" | sed 's/[\/&]/\\&/g')
    # 或者使用不同的分隔符
    sed -i "s|\"jwt_secret\": \".*\"|\"jwt_secret\": \"${JWT_SECRET}\"|" config.json

🟡 技术改进建议

建议1:使用 jq 作为主要方案,sed 作为回退

  • 当前代码优先使用 jq(很好),但 sed 回退方案有转义问题
  • 建议:如果没有 jq,提示用户安装,而不是使用有风险的 sed

建议2:密码显示给用户

print_info "✓ 已自动生成强密码"
print_warning "⚠️  请保存以下密码(仅显示一次):"
print_info "   NOFX_ADMIN_PASSWORD=${ADMIN_PASSWORD}"
print_info "   jwt_secret=${JWT_SECRET}"

建议3:添加密码强度提示

print_info "✓ 密码已生成(64字符,包含大小写字母、数字和特殊字符)"
print_info "💡 密码已保存到 .env 和 config.json,请妥善保管"

建议4:检查 openssl/urandom 可用性

generate_strong_password() {
    if command -v openssl &> /dev/null; then
        openssl rand -base64 48 | tr -d "=+/" | cut -c1-64
    elif [ -r /dev/urandom ]; then
        cat /dev/urandom | tr -dc 'a-zA-Z0-9!@#$%^&*()_+-=[]{}|;:,.<>?' | fold -w 64 | head -n 1
    else
        print_error "无法生成安全密码:openssl 和 /dev/urandom 均不可用"
        exit 1
    fi
}

业务层面审查

需求验证:自动生成强密码是合理的安全需求
功能完整性:NOFX_ADMIN_PASSWORD 字段在 .env.example 中不存在,功能不完整
用户体验:密码生成后未显示,用户无法获知

技术层面审查

密码强度:64字符base64编码的密码强度足够
转义安全:sed 转义不完整,可能导致替换失败
⚠️ 错误处理:缺少对密码生成失败的处理

最小修复建议

修复1:在 .env.example 中添加字段

# .env.example
 NOFX_TIMEZONE=Asia/Shanghai
+
+# Admin Authentication
+# Default admin password (CHANGE THIS IN PRODUCTION!)
+NOFX_ADMIN_PASSWORD=change_me_in_production

修复2:修复 sed 转义问题

-            ESCAPED_SECRET=$(echo "$JWT_SECRET" | sed 's/[[\.*^$()+?{|]/\\&/g')
-            if [[ "$OSTYPE" == "darwin"* ]]; then
-                # macOS 使用 BSD sed
-                sed -i '' "s/\"jwt_secret\": \".*\"/\"jwt_secret\": \"${ESCAPED_SECRET}\"/" config.json
-            else
-                # Linux 使用 GNU sed
-                sed -i "s/\"jwt_secret\": \".*\"/\"jwt_secret\": \"${ESCAPED_SECRET}\"/" config.json
-            fi
+            # 使用 | 作为分隔符避免转义 /
+            if [[ "$OSTYPE" == "darwin"* ]]; then
+                sed -i '' "s|\"jwt_secret\": \".*\"|\"jwt_secret\": \"${JWT_SECRET}\"|" config.json
+            else
+                sed -i "s|\"jwt_secret\": \".*\"|\"jwt_secret\": \"${JWT_SECRET}\"|" config.json
+            fi

修复3:显示生成的密码

         print_info "✓ 已使用默认环境变量创建 .env"
-        print_info "✓ 已自动生成强密码 NOFX_ADMIN_PASSWORD"
+        print_warning "⚠️  管理员密码已自动生成(请保存):"
+        print_info "   NOFX_ADMIN_PASSWORD=${ADMIN_PASSWORD}"
         print_info "💡 如需修改端口等设置,可编辑 .env 文件"
         
         # ... (jwt_secret 同理)
-        print_info "✓ 已自动生成强密码 jwt_secret"
+        print_warning "⚠️  JWT密钥已自动生成(请保存):"
+        print_info "   jwt_secret=${JWT_SECRET}"

安全验证清单

✅ 密钥安全

  • 密码不以明文硬编码
  • 使用加密随机数生成器(openssl rand)
  • 密码长度足够(64字符)

❌ 功能完整性

  • NOFX_ADMIN_PASSWORD 在 .env.example 中存在
  • 生成的密码成功写入配置文件
  • 用户能够获知生成的密码

⚠️ 错误处理

  • 密码生成失败时有错误提示
  • sed/jq 操作失败时有回退方案
  • 文件写入失败时有提示

E2E 验证报告

测试场景1:全新安装

# 步骤1:删除现有配置
rm -f .env config.json

# 步骤2:运行启动脚本
./start.sh

# 预期结果:
# ❌ .env 中不包含 NOFX_ADMIN_PASSWORD(因为模板中没有)
# ✅ config.json 中包含随机 jwt_secret
# ❌ 用户不知道生成的密码是什么

测试场景2:密码包含特殊字符

# 模拟生成包含 / 和 & 的密码
JWT_SECRET="abc/def&ghi"

# 当前 sed 命令:
sed -i "s/\"jwt_secret\": \".*\"/\"jwt_secret\": \"${JWT_SECRET}\"/" config.json
# ❌ 会失败,因为 / 未转义

# 修复后:
sed -i "s|\"jwt_secret\": \".*\"|\"jwt_secret\": \"${JWT_SECRET}\"|" config.json
# ✅ 正常工作

总结

当前 PR 的安全目标很好,但实现有关键问题:

  1. BLOCKING.env.example 中缺少 NOFX_ADMIN_PASSWORD 字段,功能不完整
  2. BLOCKING:密码未显示给用户,可能导致无法登录
  3. ⚠️ 需要修复:sed 转义不完整,可能导致替换失败

建议

  • 必须添加 NOFX_ADMIN_PASSWORD.env.example
  • 在生成后显示密码给用户(带安全提示)
  • 修复 sed 转义问题(使用不同分隔符)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants