修复: 后端审查发现的 5 项问题

根据 Spec + Code Quality 双审查修复:

1. BatchCreate 事务保护(node_service.go/node_repository.go)
   原循环 Create 在 DB 约束失败时会残留半截数据。改为预先构造所有 Node
   再走 repo.BatchCreate 单一事务,任一失败整体回滚。

2. Peek 语义与 Consume 对齐(agent_install_token_repository.go)
   FindByToken 无条件返回任意记录,导致已消费/已过期的僵尸 token
   可通过 compose 端点的 mode 检查但必然 Consume 失败,出现 410 假错。
   新增 FindValidByToken,Peek 改用之。

3. MasterURL / AgentToken / AgentVersion 渲染前校验(installscript/renderer.go)
   防止 YAML 注入(换行/引号逃逸 compose 配置)、shell 注入($(...))、
   非法字符。加 TestRenderScriptRejects* 系列测试覆盖。

4. ipLimiter 无界增长修复(install_handler.go)
   新增 gc 方法 + startGC 后台协程,每 window 周期清理过期 IP 条目。
   RouterDependencies.Context 控制生命周期;app 传入 ctx,测试 t.Cleanup 取消。

5. CreateInstallToken 的 CreatedByID 从 JWT subject 解析(node_handler.go)
   原硬编码 0 导致审计不可追溯。新增 resolveCurrentUserID helper,
   借助 UserRepository 把 JWT subject(用户名)→ user.ID;失败退回 0。
This commit is contained in:
Awuqing
2026-04-19 17:14:17 +08:00
parent d7b2093b17
commit 34106556be
11 changed files with 289 additions and 18 deletions

View File

@@ -135,6 +135,7 @@ func New(ctx context.Context, cfg config.Config, version string) (*Application,
nodeService.SetAgentRPC(agentService)
router := aphttp.NewRouter(aphttp.RouterDependencies{
Context: ctx,
Config: cfg,
Version: version,
Logger: appLogger,

View File

@@ -61,7 +61,13 @@ func setupInstallFlowRouter(t *testing.T) (http.Handler, string) {
auditLogRepo := repository.NewAuditLogRepository(db)
auditSvc := service.NewAuditService(auditLogRepo)
// 用 cancelable ctx测试结束时停掉 handler 启动的后台 GC 协程,
// 避免 goroutine 持有 map 导致 tempdir 清理失败。
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
router := NewRouter(RouterDependencies{
Context: ctx,
Config: cfg,
Version: "test",
Logger: log,

View File

@@ -1,6 +1,7 @@
package http
import (
"context"
stdhttp "net/http"
"strconv"
"strings"
@@ -21,12 +22,16 @@ type InstallHandler struct {
limiter *ipLimiter
}
func NewInstallHandler(tokenService *service.InstallTokenService, auditService *service.AuditService, externalURL string) *InstallHandler {
// NewInstallHandler 构造 handler 并启动限流器的后台 GC 协程。
// gcCtx 控制 GC 协程生命周期,建议传入 app context。
func NewInstallHandler(gcCtx context.Context, tokenService *service.InstallTokenService, auditService *service.AuditService, externalURL string) *InstallHandler {
limiter := newIPLimiter(20, time.Minute)
limiter.startGC(gcCtx)
return &InstallHandler{
tokenService: tokenService,
auditService: auditService,
externalURL: externalURL,
limiter: newIPLimiter(20, time.Minute),
limiter: limiter,
}
}
@@ -177,3 +182,40 @@ func (l *ipLimiter) allow(ip string) bool {
l.events[ip] = append(keep, now)
return true
}
// gc 清理窗口外所有过期的 IP 条目,防止公网扫描导致 map 无界增长。
// 由后台 goroutine 周期性调用。
func (l *ipLimiter) gc(now time.Time) {
l.mu.Lock()
defer l.mu.Unlock()
cutoff := now.Add(-l.window)
for k, v := range l.events {
stale := true
for _, t := range v {
if t.After(cutoff) {
stale = false
break
}
}
if stale {
delete(l.events, k)
}
}
}
// startGC 启动后台清理协程,每 window 周期清扫一次 map。
// ctx 取消时协程退出。
func (l *ipLimiter) startGC(ctx context.Context) {
go func() {
ticker := time.NewTicker(l.window)
defer ticker.Stop()
for {
select {
case <-ctx.Done():
return
case t := <-ticker.C:
l.gc(t)
}
}
}()
}

View File

@@ -7,6 +7,7 @@ import (
"backupx/server/internal/apperror"
"backupx/server/internal/installscript"
"backupx/server/internal/repository"
"backupx/server/internal/service"
"backupx/server/pkg/response"
"github.com/gin-gonic/gin"
@@ -16,23 +17,49 @@ type NodeHandler struct {
service *service.NodeService
auditService *service.AuditService
installTokenSvc *service.InstallTokenService
userRepo repository.UserRepository
externalURL string
}
// NewNodeHandler 构造 handler。
// userRepo 用于把 JWT subject用户名解析为 user.ID填入 install_token.created_by_id 做审计追溯;
// 传 nil 时 created_by_id 记为 0仍可用不阻断
func NewNodeHandler(
nodeService *service.NodeService,
auditService *service.AuditService,
installTokenSvc *service.InstallTokenService,
userRepo repository.UserRepository,
externalURL string,
) *NodeHandler {
return &NodeHandler{
service: nodeService,
auditService: auditService,
installTokenSvc: installTokenSvc,
userRepo: userRepo,
externalURL: externalURL,
}
}
// resolveCurrentUserID 从 JWT subject 解析出 user.ID失败返回 0。
func (h *NodeHandler) resolveCurrentUserID(c *gin.Context) uint {
if h.userRepo == nil {
return 0
}
subjectValue, ok := c.Get(contextUserSubjectKey)
if !ok {
return 0
}
subject, err := service.SubjectFromContextValue(subjectValue)
if err != nil || subject == "" {
return 0
}
user, err := h.userRepo.FindByUsername(c.Request.Context(), subject)
if err != nil || user == nil {
return 0
}
return user.ID
}
func (h *NodeHandler) List(c *gin.Context) {
items, err := h.service.List(c.Request.Context())
if err != nil {
@@ -224,7 +251,7 @@ func (h *NodeHandler) CreateInstallToken(c *gin.Context) {
AgentVersion: input.AgentVersion,
DownloadSrc: input.DownloadSrc,
TTLSeconds: input.TTLSeconds,
CreatedByID: 0, // 如需关联 userID后续可通过 auth 中间件注入
CreatedByID: h.resolveCurrentUserID(c),
})
if err != nil {
response.Error(c, err)

View File

@@ -1,6 +1,7 @@
package http
import (
"context"
"errors"
stdhttp "net/http"
@@ -15,6 +16,9 @@ import (
)
type RouterDependencies struct {
// Context 控制 handler 启动的后台协程(如 ipLimiter GC的生命周期。
// app 应传入随进程退出可取消的 ctx若为 nil 则退化为 context.Background()。
Context context.Context
Config config.Config
Version string
Logger *zap.Logger
@@ -143,7 +147,7 @@ func NewRouter(deps RouterDependencies) *gin.Engine {
database.POST("/discover", databaseHandler.Discover)
}
nodeHandler := NewNodeHandler(deps.NodeService, deps.AuditService, deps.InstallTokenService, deps.MasterExternalURL)
nodeHandler := NewNodeHandler(deps.NodeService, deps.AuditService, deps.InstallTokenService, deps.UserRepository, deps.MasterExternalURL)
nodes := api.Group("/nodes")
nodes.Use(AuthMiddleware(deps.JWTManager))
nodes.GET("", nodeHandler.List)
@@ -178,7 +182,11 @@ func NewRouter(deps RouterDependencies) *gin.Engine {
// 公开安装路由(不走 JWT 中间件)
if deps.InstallTokenService != nil {
installHandler := NewInstallHandler(deps.InstallTokenService, deps.AuditService, deps.MasterExternalURL)
gcCtx := deps.Context
if gcCtx == nil {
gcCtx = context.Background()
}
installHandler := NewInstallHandler(gcCtx, deps.InstallTokenService, deps.AuditService, deps.MasterExternalURL)
engine.GET("/install/:token", installHandler.Script)
engine.GET("/install/:token/compose.yml", installHandler.Compose)
}

View File

@@ -7,6 +7,8 @@ import (
"bytes"
_ "embed"
"fmt"
"net/url"
"strings"
"text/template"
"backupx/server/internal/model"
@@ -43,6 +45,9 @@ func DownloadBaseFor(src string) string {
// RenderScript 渲染目标机安装脚本。
func RenderScript(ctx Context) (string, error) {
ctx = withDefaults(ctx)
if err := validateContext(ctx); err != nil {
return "", err
}
tmpl, err := template.New("install").Parse(installScriptTmpl)
if err != nil {
return "", fmt.Errorf("parse template: %w", err)
@@ -57,6 +62,9 @@ func RenderScript(ctx Context) (string, error) {
// RenderComposeYaml 渲染 docker-compose.yml 片段。
func RenderComposeYaml(ctx Context) (string, error) {
ctx = withDefaults(ctx)
if err := validateContext(ctx); err != nil {
return "", err
}
tmpl, err := template.New("compose").Parse(composeYamlTmpl)
if err != nil {
return "", fmt.Errorf("parse template: %w", err)
@@ -68,6 +76,89 @@ func RenderComposeYaml(ctx Context) (string, error) {
return buf.String(), nil
}
// validateContext 对模板变量做安全校验,防止 YAML/shell 注入。
// - MasterURL必须是合法 http(s) URL无控制字符
// - AgentToken仅允许 hex 字符,最长 128
// - AgentVersion仅允许 tag 常见字符(字母数字、点、连字符、下划线、加号)
//
// 这些字段被直接写入 shell 双引号字符串和 YAML 双引号值;不做校验会带来
// 注入风险(如 MasterURL 含 `"\nCOMMAND:` 可逃逸 YAML 结构)。
func validateContext(ctx Context) error {
if err := validateMasterURL(ctx.MasterURL); err != nil {
return err
}
if err := validateAgentToken(ctx.AgentToken); err != nil {
return err
}
if err := validateAgentVersion(ctx.AgentVersion); err != nil {
return err
}
return nil
}
func validateMasterURL(raw string) error {
raw = strings.TrimSpace(raw)
if raw == "" {
return fmt.Errorf("master URL empty")
}
if strings.ContainsAny(raw, " \t\r\n\"'`$\\") {
return fmt.Errorf("master URL contains illegal characters")
}
u, err := url.Parse(raw)
if err != nil {
return fmt.Errorf("invalid master URL: %w", err)
}
if u.Scheme != "http" && u.Scheme != "https" {
return fmt.Errorf("master URL scheme must be http or https, got %q", u.Scheme)
}
if u.Host == "" {
return fmt.Errorf("master URL missing host")
}
return nil
}
// validateAgentToken 允许占位符 <AGENT_TOKEN>PreviewScript 使用),
// 或 32 字节 hex64 字符)+ 小幅兼容16-128 hex 字符)
func validateAgentToken(tok string) error {
if tok == "<AGENT_TOKEN>" {
return nil
}
if len(tok) < 8 || len(tok) > 128 {
return fmt.Errorf("agent token length out of range")
}
for _, c := range tok {
switch {
case c >= '0' && c <= '9':
case c >= 'a' && c <= 'f':
case c >= 'A' && c <= 'F':
default:
return fmt.Errorf("agent token must be hex")
}
}
return nil
}
func validateAgentVersion(v string) error {
v = strings.TrimSpace(v)
if v == "" {
return fmt.Errorf("agent version empty")
}
if len(v) > 64 {
return fmt.Errorf("agent version too long")
}
for _, c := range v {
switch {
case c >= '0' && c <= '9':
case c >= 'a' && c <= 'z':
case c >= 'A' && c <= 'Z':
case c == '.' || c == '-' || c == '_' || c == '+':
default:
return fmt.Errorf("agent version contains illegal char %q", c)
}
}
return nil
}
func withDefaults(ctx Context) Context {
if ctx.InstallPrefix == "" {
ctx.InstallPrefix = "/opt/backupx-agent"

View File

@@ -7,9 +7,10 @@ import (
"backupx/server/internal/model"
)
// 使用合法 hex token32 字节 = 64 字符)以通过 validateAgentToken 校验
var testCtx = Context{
MasterURL: "https://master.example.com",
AgentToken: "test-token-hex",
AgentToken: "deadbeefcafebabe0123456789abcdef0123456789abcdef0123456789abcdef",
AgentVersion: "v1.7.0",
Mode: model.InstallModeSystemd,
Arch: model.InstallArchAuto,
@@ -30,7 +31,7 @@ func TestRenderScriptSystemd(t *testing.T) {
"systemctl enable --now backupx-agent",
"X-Agent-Token: ${AGENT_TOKEN}",
"MASTER_URL=\"https://master.example.com\"",
"AGENT_TOKEN=\"test-token-hex\"",
"AGENT_TOKEN=\"deadbeefcafebabe0123456789abcdef0123456789abcdef0123456789abcdef\"",
}
for _, s := range mustContain {
if !strings.Contains(got, s) {
@@ -91,11 +92,60 @@ func TestRenderComposeYaml(t *testing.T) {
if !strings.Contains(got, "image: awuqing/backupx:v1.7.0") {
t.Errorf("compose missing image:\n%s", got)
}
if !strings.Contains(got, `BACKUPX_AGENT_TOKEN: "test-token-hex"`) {
if !strings.Contains(got, `BACKUPX_AGENT_TOKEN: "deadbeefcafebabe0123456789abcdef0123456789abcdef0123456789abcdef"`) {
t.Errorf("compose missing token env:\n%s", got)
}
}
func TestRenderScriptRejectsInjectedMasterURL(t *testing.T) {
bad := []string{
"https://example.com\" other: inject", // 含引号和空格
"javascript:alert(1)", // scheme 非法
"https://example.com\n- privileged", // 含换行YAML 注入经典 payload
"", // 空
}
for _, u := range bad {
ctx := testCtx
ctx.MasterURL = u
if _, err := RenderScript(ctx); err == nil {
t.Errorf("RenderScript should reject MasterURL %q", u)
}
}
}
func TestRenderComposeYamlRejectsInjectedMasterURL(t *testing.T) {
ctx := testCtx
ctx.Mode = model.InstallModeDocker
ctx.MasterURL = "https://example.com\n- privileged: true"
if _, err := RenderComposeYaml(ctx); err == nil {
t.Errorf("RenderComposeYaml should reject injected MasterURL")
}
}
func TestRenderScriptRejectsBadToken(t *testing.T) {
ctx := testCtx
ctx.AgentToken = "not-hex-token" // 非 hex
if _, err := RenderScript(ctx); err == nil {
t.Errorf("should reject non-hex agent token")
}
}
func TestRenderScriptAcceptsPlaceholderToken(t *testing.T) {
ctx := testCtx
ctx.AgentToken = "<AGENT_TOKEN>" // Preview 占位符
if _, err := RenderScript(ctx); err != nil {
t.Errorf("should accept placeholder token: %v", err)
}
}
func TestRenderScriptRejectsBadVersion(t *testing.T) {
ctx := testCtx
ctx.AgentVersion = "v1.7 && rm -rf /" // 含非法字符
if _, err := RenderScript(ctx); err == nil {
t.Errorf("should reject version with shell metacharacters")
}
}
func TestDownloadBaseMapping(t *testing.T) {
cases := map[string]string{
model.InstallSourceGitHub: "https://github.com/Awuqing/BackupX/releases/download",

View File

@@ -12,7 +12,11 @@ import (
// AgentInstallTokenRepository 一次性安装令牌仓储。
type AgentInstallTokenRepository interface {
Create(ctx context.Context, t *model.AgentInstallToken) error
// FindByToken 按 token 字符串查询(不过滤状态),用于管理工具或审计场景。
FindByToken(ctx context.Context, token string) (*model.AgentInstallToken, error)
// FindValidByToken 查询且要求 consumed_at IS NULL 且 expires_at > now
// 适用于 compose 端点预检 Mode 等"只读不消费但需有效"的场景。
FindValidByToken(ctx context.Context, token string) (*model.AgentInstallToken, error)
// ConsumeByToken 原子消费:仅当 token 存在、未过期、未消费时成功,返回消费后的记录。
// 其它情况(不存在/已过期/已消费)一律返回 (nil, nil)。
ConsumeByToken(ctx context.Context, token string) (*model.AgentInstallToken, error)
@@ -45,6 +49,22 @@ func (r *GormAgentInstallTokenRepository) FindByToken(ctx context.Context, token
return &item, nil
}
// FindValidByToken 仅返回未消费且未过期的记录,过滤条件与 ConsumeByToken 对齐。
func (r *GormAgentInstallTokenRepository) FindValidByToken(ctx context.Context, token string) (*model.AgentInstallToken, error) {
var item model.AgentInstallToken
now := time.Now().UTC()
err := r.db.WithContext(ctx).
Where("token = ? AND consumed_at IS NULL AND expires_at > ?", token, now).
First(&item).Error
if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return nil, nil
}
return nil, err
}
return &item, nil
}
// ConsumeByToken 使用条件 UPDATE + RowsAffected 实现原子消费。
// SQLite 不支持 SELECT FOR UPDATE但 UPDATE 本身在 SQLite 中是原子的。
func (r *GormAgentInstallTokenRepository) ConsumeByToken(ctx context.Context, token string) (*model.AgentInstallToken, error) {

View File

@@ -15,6 +15,8 @@ type NodeRepository interface {
FindByToken(context.Context, string) (*model.Node, error)
FindLocal(context.Context) (*model.Node, error)
Create(context.Context, *model.Node) error
// BatchCreate 在单一事务内批量创建节点,任一失败即全部回滚。
BatchCreate(ctx context.Context, nodes []*model.Node) error
Update(context.Context, *model.Node) error
Delete(context.Context, uint) error
MarkStaleOffline(ctx context.Context, threshold time.Time) (int64, error)
@@ -86,6 +88,22 @@ func (r *GormNodeRepository) Create(ctx context.Context, item *model.Node) error
return r.db.WithContext(ctx).Create(item).Error
}
// BatchCreate 在单一事务中批量创建节点。任一记录失败即事务回滚。
// 节点 ID 在事务提交后回填到入参切片元素上。
func (r *GormNodeRepository) BatchCreate(ctx context.Context, nodes []*model.Node) error {
if len(nodes) == 0 {
return nil
}
return r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
for _, n := range nodes {
if err := tx.Create(n).Error; err != nil {
return err
}
}
return nil
})
}
func (r *GormNodeRepository) Update(ctx context.Context, item *model.Node) error {
return r.db.WithContext(ctx).Save(item).Error
}

View File

@@ -128,12 +128,14 @@ func (s *InstallTokenService) Consume(ctx context.Context, token string) (*Consu
return &ConsumedInstallToken{Record: record, Node: node}, nil
}
// Peek 只读查询(不消费),供 compose 端点预检 Mode。
// Peek 只读查询(不消费)且仅返回有效 token未消费、未过期,供 compose 端点预检 Mode。
// 对已过期/已消费的 token 返回 (nil, nil),与 Consume 语义保持一致,
// 避免 compose handler 误放行"僵尸 token"造成后续 Consume 必然失败的迷惑链路。
func (s *InstallTokenService) Peek(ctx context.Context, token string) (*model.AgentInstallToken, error) {
if strings.TrimSpace(token) == "" {
return nil, nil
}
return s.repo.FindByToken(ctx, token)
return s.repo.FindValidByToken(ctx, token)
}
// StartGC 启动后台 GC按 interval 扫描并删 ExpiresAt < now-7d 的记录。

View File

@@ -402,23 +402,29 @@ func (s *NodeService) BatchCreate(ctx context.Context, names []string) ([]NodeCr
}
}
results := make([]NodeCreateResult, 0, len(cleaned))
// 预先构造所有 Nodetoken 生成在事务外完成(纯内存操作,失败不会影响 DB 状态)
nodes := make([]*model.Node, 0, len(cleaned))
now := time.Now().UTC()
for _, name := range cleaned {
tok, err := generateToken()
if err != nil {
return nil, fmt.Errorf("generate token: %w", err)
}
node := &model.Node{
nodes = append(nodes, &model.Node{
Name: name,
Token: tok,
Status: model.NodeStatusOffline,
IsLocal: false,
LastSeen: time.Now().UTC(),
}
if err := s.repo.Create(ctx, node); err != nil {
return nil, err
}
results = append(results, NodeCreateResult{ID: node.ID, Name: node.Name})
LastSeen: now,
})
}
// 事务内批量创建:任一失败整体回滚
if err := s.repo.BatchCreate(ctx, nodes); err != nil {
return nil, err
}
results := make([]NodeCreateResult, 0, len(nodes))
for _, n := range nodes {
results = append(results, NodeCreateResult{ID: n.ID, Name: n.Name})
}
return results, nil
}