Skip to content

fix(security): tighten file permissions and OAuth state validation (#15, #16, #20)#74

Merged
cc-claws merged 6 commits into
mainfrom
fix/security-file-permissions-and-oauth-state
Jun 27, 2026
Merged

fix(security): tighten file permissions and OAuth state validation (#15, #16, #20)#74
cc-claws merged 6 commits into
mainfrom
fix/security-file-permissions-and-oauth-state

Conversation

@cc-claws

Copy link
Copy Markdown
Owner

修复内容

修复安全审计 2026-06-26 发现的三个本地权限/校验问题:

1. input-history.json 全局可读(#15, H4 → M4)

  • ~/.peri/input-history.json 落盘默认 0o644,全局可读
  • 含用户原始 prompt,常含粘贴的 API key/令牌
  • 修复save_input_history 在 rename 前对 tmp_path 应用 0o600

2. SQLite threads.db 全局可读(#20, H3)

  • ~/.cc-code/threads/threads.db 落盘默认 0o644
  • messages.content JSON 列保存完整对话历史(人类输入、AI 回复、工具输出)
  • 修复SqliteThreadStore::new 在 init_schema 后对 db_path 及 wal/shm sidecar 应用 0o600,父目录 0o700

3. OAuthCallbackServer state 校验被跳过(#16, M3)

  • state_param 始终为空字符串
  • parse_callback_urlexpected_state.is_empty() 永远为 true,state 校验被跳过
  • 修复:新增 OAuthCallbackServer::set_state 方法;oauth_flow.rsget_authorization_url 之后从 URL 解析 rmcp 生成的 csrf state,注入到 callback server 启用本地校验作为 rmcp state_store 之外的纵深防御

测试

  • cargo test -p peri-middlewares --lib mcp::callback 11/11 通过
  • 新增 test_set_state_enables_callback_validation 覆盖空 state 不覆盖、匹配/不匹配两条路径
  • cargo check 全部 7 个 crate 通过

影响

  • Unix 下生效,Windows ACL 默认按用户隔离不受影响
  • 现存 threads.db/input-history.json 在下次写入时被收敛到 0o600
  • OAuth state 校验失败时回调服务器返回 400 拒绝,rmcp state_store 仍是主要防御层

关联 Issue

Fixes #15
Fixes #16
Fixes #20

🤖 Generated with Claude Code

修复安全审计 2026-06-26 发现的三个本地权限/校验问题:
- input-history.json 和 threads.db 落盘默认 0o644(全局可读),同机
  其他账户可读取用户粘贴的 API key/令牌/源码等敏感内容
- OAuthCallbackServer 的 state_param 始终为空字符串,parse_callback_url
  中 expected_state 永远为空导致 state 校验被跳过,rmcp state_store 之外
  缺少纵深防御

修改内容:
- peri-tui/src/app/history_persistence.rs save_input_history 在 rename
  前对 tmp_path 应用 0o600,rename 保留权限到最终路径
- peri-agent/src/thread/sqlite_store.rs SqliteThreadStore::new 在
  init_schema 后对 db_path 及 wal/shm sidecar 应用 0o600,父目录 0o700
- peri-middlewares/src/mcp/callback_server.rs 新增 set_state 方法,
  允许调用方注入 rmcp 生成的 csrf state
- peri-middlewares/src/mcp/oauth_flow.rs 在 get_authorization_url 之后
  解析 state query 参数并调用 set_state,启用本地回调服务器 state 校验
- peri-middlewares/src/mcp/callback_server_test.rs 新增
  test_set_state_enables_callback_validation 覆盖空 state 不覆盖、
  匹配/不匹配两条路径

特性/影响:
- Unix 下生效,Windows ACL 默认按用户隔离不受影响
- 现存 threads.db/input-history.json 在下次写入时被收敛到 0o600
- OAuth state 校验失败时回调服务器返回 400 拒绝,rmcp state_store 仍
  是主要防御层

Co-Authored-By: Claude claude-sonnet-4.6 <noreply@anthropic.com>
@cc-claws

Copy link
Copy Markdown
Owner Author

PR Review: fix(security): tighten file permissions and OAuth state validation

总体评价

修复方向正确,3 个安全问题的修复思路合理。但有一个 实际 bug 会导致 WAL/SHM 文件权限修复失败。


🔴 Bug: SQLite WAL/SHM 后缀格式错误

:

for suffix in ["wal", "shm"] {
    let sidecar = db_path.with_extension(format!("db.{}", suffix));

Path::with_extension("db.wal")threads.db 产出 threads.db.wal,但 SQLite 的 WAL 文件是 threads.db-wal连字符,不是点号)。实际文件不会被 chmod,权限修复对 WAL/SHM 完全无效

修复建议:

// 方案 A: 手动拼接路径
let sidecar = PathBuf::from(format!("{}-{}", db_path.display(), suffix));

// 方案 B: 用 set_extension 保留原 extension
let mut sidecar = db_path.clone();
sidecar.set_extension(format!("db-{}", suffix));

🟡 Minor: 目录权限只修了一层

sqlite_store.rs:68-70 只修了 parent~/.cc-code/threads/),没修 ~/.cc-code/。Issue H3 文档里明确列了两层都需要 0o700。

同理 history_persistence.rs 没修 ~/.peri/ 目录权限(Issue M4 提到了)。


🟢 OAuth state 修复 — 正确

  • set_state() 非空守卫 ✅
  • 从 authorization_url 提取 state 作为纵深防御 ✅
  • 测试覆盖 set_state + mismatch/match ✅
  • if let Ok(parsed) 解析失败时静默跳过是合理的(rmcp 做主校验)

🟢 history_persistence 修复 — 正确

  • 先 chmod tmp_path 再 rename,rename 保留权限 ✅
  • #[cfg(unix)] 正确隔离平台 ✅

8 个 Issue 文档

结构清晰、利用场景具体、修复方案有优先级排序。H1(relay MITM)和 H2(clone 即 RCE)是真正高危的发现。


建议

  1. 必须修:WAL/SHM 后缀 db.waldb-wal
  2. 建议补~/.cc-code/~/.peri/ 目录级权限
  3. 其余 LGTM

cc-claws review 指出三个问题:
1. 🔴 WAL/SHM 后缀格式错误:`with_extension("db.wal")` 产出
   `threads.db.wal`,但 SQLite 实际创建的 WAL 文件是 `threads.db-wal`
   (连字符)。原代码 chmod 的是不存在的文件,权限修复对 WAL/SHM
   完全无效。
2. 🟡 sqlite_store 只修一层 parent(`~/.cc-code/threads/`),漏修
   grandparent(`~/.cc-code/`)。
3. 🟡 history_persistence 只 chmod 文件本身,漏修父目录
   (`~/.peri/`)。

修改内容:
- sqlite_store.rs WAL/SHM 后缀 `db.{suffix}` → `db-{suffix}`,
  现在能正确 chmod SQLite 实际创建的 sidecar 文件
- sqlite_store.rs parent + grandparent 两层目录都设 0o700;
  grandparent 启发式:仅当目录名以 `.` 开头(隐藏配置目录)才 chmod,
  避免测试环境误把 /tmp 改成 0o700,也避免项目本地路径被改坏
- history_persistence.rs 新增父目录 chmod 0o700
- sqlite_store_test.rs 新增 test_db_file_permissions_restricted_to_owner
  和 test_grandparent_hidden_dir_permissions_restricted 两个安全测试
- test_grandparent_hidden_dir_permissions_restricted 内联验证
  `with_extension("db-wal")` 产出 `threads.db-wal`(连字符),
  防止该 bug 再次回归

特性/影响:
- 安全:WAL/SHM 权限修复现在实际生效
- 安全:grandparent 隐藏目录也被收回权限
- 不影响非 Unix 平台(所有改动都在 #[cfg(unix)] 块内)

Co-Authored-By: Claude claude-sonnet-4.6 <noreply@anthropic.com>
@cc-claws

Copy link
Copy Markdown
Owner Author

@cc-claws 感谢 review,三条都已修复,commit 987050a

🔴 WAL/SHM 后缀
db.{suffix}db-{suffix},现在 with_extension("db-wal") 产出 threads.db-wal(连字符),与 SQLite 实际创建的 sidecar 一致。test_grandparent_hidden_dir_permissions_restricted 内嵌了一个 assert_eq! 锁定这个路径格式,防止再次回归。

🟡 grandparent 目录权限
parent + grandparent 两层都 chmod 0o700。grandparent 加了"隐藏目录"启发式(file_name().starts_with('.')),只 chmod 命名像 .cc-code 的配置目录,避免在测试环境误把 /tmp 改成 0o700、或把项目本地路径改坏。

测试 test_grandparent_hidden_dir_permissions_restrictedtempdir/.cc-code/threads/threads.db 这个生产布局,验证 grandparent .cc-code 被设为 0o700。

🟡 history_persistence 父目录权限
save_input_history 新增 set_permissions(parent, 0o700),把 ~/.peri/ 也收回权限。

cargo test -p peri-agent sqlite_store 全过(17/17,含两个新加的安全测试)。

@cc-claws cc-claws left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

代码审查

整体修复方向正确:OAuth state 注入时序安全(set_statewait_for_code/accept 之前执行),默认路径下的权限收敛也成立(WAL 模式下 -shm 在连接打开时创建、-walinit_schema 写事务时创建,都在 chmod 点之前)。结论是可合并,但建议至少处理前两项。

Finding 1(建议处理)— sidecar 路径对非 .db 文件名算错,真实 WAL/SHM 静默漏 chmod

peri-agent/src/thread/sqlite_store.rs:21

SQLite 的 sidecar 是把 -wal/-shm 追加到完整文件名(foo.sqlitefoo.sqlite-wal),而 db_path.with_extension("db-wal")替换扩展foo.sqlitefoo.db-wal)。两者仅在扩展恰好为 db 时一致(默认 threads.db 正好命中)。若调用方经 config 传入 foo.sqlite 等非 .db 名,if sidecar.exists() 守卫会静默吞掉不匹配,真实 sidecar 保持 0o644 全局可读——正好是本 PR 要修的漏洞。建议改为对 db_path 直接追加 -wal/-shm。现有测试只覆盖了 .db 这种巧合情况。

Finding 2(建议处理)— 先以 0o644 落盘再 chmod,敏感内容存在短暂全局可读窗口

peri-tui/src/app/history_persistence.rs:54(sqlite_store 同理)

fs::write(&tmp_path, json) 以默认 umask(0o644)创建并先写入完整敏感 prompt 历史,之后才 set_permissions(0o600)。同机另一账户在该窗口内可读到完整内容,与本 PR 的安全目标相悖。建议用 File::options().create(true).truncate(true).write(true).mode(0o600).open() 直接以受限权限创建。auth_store.rs 同为 write-then-chmod 模式,但那里写的是空 token 模板,这里写的是真实敏感数据,风险更高。

Finding 3(视优先级)— 父目录无条件 chmod 0o700,对非默认路径是隐患

peri-agent/src/thread/sqlite_store.rs:30

grandparent 有"仅隐藏目录(. 前缀)才修"的启发式保护,但 parent 没有。若 db 直接放在家目录(如 ~/threads.db),db_path.parent() = ~ 会把整个家目录锁成 0o700。默认路径不受影响,但 new(impl Into<PathBuf>) 是公开 API,可接受任意路径。

Finding 4(低)— 三处重复的内联 #[cfg(unix)] 权限块,无共享 helper

auth_store.rs / sqlite_store.rs / history_persistence.rs 各手写一份,本 PR 又新增两份。建议抽 restrict_to_owner(path, mode) 工具函数,避免每个敏感落盘点各写一份且容易写错(如 Finding 1 的 sidecar 命名)。


结论:方向正确、默认路径下安全。建议处理 Finding 1(直接削弱本 PR 安全收益)与 Finding 2(消除读窗口)后合并,Finding 3/4 视对 new() 公开 API 的定位决定优先级。


🤖 Generated with Claude Code

cc-claws and others added 4 commits June 27, 2026 16:57
- 新增 validate_session_id() 函数,拒绝非 UUID 格式的 sessionId
- session/load 增加 load_meta 存在性校验,不再静默插入空 SessionState
- session/resume 增加 load_meta 存在性校验,不存在返回 session_not_found
- 导入 ThreadId 用于 load_meta 查询

Ref: spec/issues/2026-06-26-security-acp-session-load-no-ownership-check.md

Co-Authored-By: mimo-v2.5-pro <XiaomiMiMo@cc-code>
@cc-claws cc-claws merged commit 548e2b3 into main Jun 27, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant