Skip to content

feat(auth): add CLI RBAC, multi-token management, and audit#325

Open
yorkew-east8 wants to merge 3 commits into
mainfrom
feat/cli-rbac-multitoken-audit
Open

feat(auth): add CLI RBAC, multi-token management, and audit#325
yorkew-east8 wants to merge 3 commits into
mainfrom
feat/cli-rbac-multitoken-audit

Conversation

@yorkew-east8

Copy link
Copy Markdown
Contributor

Summary

  • add admin and read-only-admin authorization across all v2 Connect procedures and raw HTTP management routes
  • add client-generated multi-token lifecycle APIs and CLI commands; API responses never expose plaintext, hash, or fingerprint data
  • persist token hashes and operation audits in SQLite, including denied attempts, idempotent create retries, revocation cancellation, and environment-token rotation
  • store per-host CLI credentials in the existing mode-0600 config and add whoami, role, token, and audit commands
  • document the security model and add a real-daemon RBAC workflow task

Testing

Passed:

  • PATH=/Users/wangyue/go/bin:$PATH task generate:proto
  • go test ./pkg/auth ./pkg/storage/configstore ./pkg/agentcompose/api ./pkg/agentcompose/app ./cmd/agent-compose
  • go test -race ./pkg/auth ./pkg/storage/configstore
  • task build
  • task test:rbac
  • bash -n scripts/test-agent-compose-rbac.sh
  • task test:scripts (as part of task test)

Attempted but blocked by existing platform/baseline issues unrelated to this change:

  • task lint: unused boxliteVolumeBridgeIsMountPoint in pkg/driver/boxlite_volume_bridge.go:188
  • task docs:build: existing YAML manual misses schema fields commit, description, and display_name
  • task test: macOS install test requires GNU realpath -m and stops before coverage
  • ./scripts/test-coverage.sh: existing macOS /private path normalization failures in pkg/volumes

Checklist

  • Documentation updated when behavior or configuration changed.
  • Tests added or updated for user-visible behavior.
  • No secrets, private endpoints, internal certificates, or local runtime state included.

This PR intentionally excludes agent-compose-ui and must not be merged as part of this task.

@monkeyscan

monkeyscan Bot commented Jul 17, 2026

Copy link
Copy Markdown

PR Title: feat(auth): add CLI RBAC, multi-token management, ...

Commit: 0c731b4

本次 PR 引入了 daemon 控制平面的 RBAC Token 认证、Token 生命周期管理和操作审计功能。主要改动包括:

  1. 认证模型:新增 pkg/auth,定义了 adminread-only-admin 两种角色,支持环境变量启动的 default-admin token 和客户端生成的 issued token。
  2. 安全拦截层security_interceptor.go 为所有 Connect v2 RPC 增加了统一的鉴权与审计拦截;daemon_security.go 为原始 HTTP 路由(workspace upload、webhook source 等)增加了同样的 RBAC 检查。
  3. 审计存储auth_store.go 在 SQLite 中新增了 auth_tokenoperation_audit 表,支持 token 的幂等创建、撤销、环境 token 轮换以及审计记录的读写。
  4. CLIcli_security.go 提供了 auth token create/revoke/listauth role lsaudit ls 等命令,token 明文由客户端生成并通过 CreateToken 发送给服务端。
  5. 中间件链main.go 按顺序挂载了 Request ID、认证、安全三层 Echo 中间件;app.go 为所有 v2 handler 注入了 SecurityInterceptor

整体设计思路清晰,fail-closed 的默认策略、匿名引导模式、本地 Unix socket 免认证、token 撤销时取消活跃请求等机制都考虑到了。

observed := &auditStreamingConn{StreamingHandlerConn: conn}
callErr := next(ctx, observed)
status := controlauth.AuditStatusSucceeded
code, message := "", ""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Connect RPC 拦截器 DenyAudit 丢失请求 ID,导致审计记录无法关联

在 SecurityInterceptor 的 authorize 方法中,当角色权限不足需要记录拒绝审计时,调用的是 i.service.DenyAudit(context.WithoutCancel(ctx), identity, "", policy.Action),其中 requestID 参数固定传了空字符串。实际上 WrapUnary 和 WrapStreamingHandler 的调用方都可以通过 request.Header().Get("X-Request-ID")conn.RequestHeader().Get("X-Request-ID") 拿到真实的请求 ID,但 authorize 方法没有接收或传递这个值。结果导致所有被拒绝的 Connect RPC 操作在审计表中的 request_id 都不是客户端传入的 X-Request-ID,而会被 normalizeRequestID 替换成随机生成的 audit ID,严重削弱了审计日志与 HTTP 访问日志之间的可追踪性。

Problem code:

Changed code at pkg/agentcompose/api/security_interceptor.go:92-102

Recommendation:
修改 authorize 方法签名,增加 requestID 参数,并在 WrapUnary/WrapStreamingHandler 中将真实的 X-Request-ID 传递进去,确保 DenyAudit 能写入正确的请求 ID。

Comment thread pkg/auth/service.go
if r.items == nil {
r.items = make(map[string]map[uint64]context.CancelFunc)
}
r.nextID++

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SanitizeError 仅替换首次出现的 Bearer/Authorization 前缀,可能导致 token 泄露到审计日志

SanitizeError 用于在将错误信息写入审计表之前脱敏 Bearer Token 等敏感内容。但它对每个前缀(Bearer bearer Authorization:authorization:)仅使用 strings.Index 查找并替换首次出现的位置。如果错误消息中同一前缀出现多次(例如 failed: Bearer abc, retry with bearer xyz),第二次及之后的明文 token 不会被替换,会以原始形式存入 operation_audit.error_message。此外,当前替换逻辑 value[:index] + prefix + "[REDACTED]" 只是截断到前缀处并追加 [REDACTED],如果前缀后紧跟的并非 token 值而是其他文本,也可能留下残余信息。

Problem code:

Changed code at pkg/auth/service.go:236-247

Recommendation:
SanitizeError 改为使用 strings.ReplaceAll 或正则表达式,确保每个前缀的所有出现都被彻底替换为 [REDACTED],并考虑替换前缀后直到空白或字符串结尾的全部内容。

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.

1 participant