fix: QQBot credential backups bypass gateway state isolation#84314
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 2, 2026, 6:58 AM ET / 10:58 UTC. Summary PR surface: Source +4, Tests +76. Total +80 across 3 files. Reproducibility: yes. The source path is clear: current main saves and loads QQBot credential backups through an OS-home path while documented multi-gateway isolation expects unique Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Merge this shape after required checks and maintainer acceptance of strict state isolation; do not add a runtime fallback that reads home-global QQBot credential backups for custom state roots. Do we have a high-confidence way to reproduce the issue? Yes. The source path is clear: current main saves and loads QQBot credential backups through an OS-home path while documented multi-gateway isolation expects unique Is this the best way to solve the issue? Yes. Moving only credential backups to the public AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 1db2c2a3e058. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +4, Tests +76. Total +80 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
Heads up: this PR needs to be updated against current |
…apply security fix Generated by staged fix workflow.
3ed8978 to
4e78163
Compare
|
Maintainer fixup pushed at 2e97f72. What changed:
Proof run locally on Node 24.15.0:
Result: 3 focused QQBot test files passed, 11 tests passed. Autoreview clean: no accepted/actionable findings reported. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Added a maintainer proof addendum to the PR body. New evidence:
Proof output included in the PR body: @clawsweeper re-review |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
|
Land-ready after maintainer fixup. What changed:
Proof:
Known status-check noise before merge:
Landing with contributor credit preserved in the squash body. |
Summary
~/.openclaw/qqbot/data/. Gateway B, started under the same OS account but a differentOPENCLAW_STATE_DIR, can silently import that backup and overwrite its own config with Gateway A'sappIdandclientSecret, collapsing two supposedly isolated gateway profiles into one shared QQBot identity.Motivation
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Real behavior proof (required for external PRs)
~/.openclaw/qqbot/data/. Gateway B, started under the same OS account but a differentOPENCLAW_STATE_DIR, can silently import that backup and overwrite its own config with Gateway A'sappIdandclientSecret, collapsing two supposedly isolated gateway profiles into one shared QQBot identity.node scripts/run-oxlint.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' 'extensions/qqbot/src/engine/utils/data-paths.ts' && node scripts/run-vitest.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' && pnpm build && pnpm checknode scripts/run-oxlint.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' 'extensions/qqbot/src/engine/utils/data-paths.ts' && node scripts/run-vitest.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' && pnpm build && pnpm checkreportedpassed.Root Cause (if applicable)
Regression Test Plan (if applicable)
node scripts/run-oxlint.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' 'extensions/qqbot/src/engine/utils/data-paths.ts' && node scripts/run-vitest.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' && pnpm build && pnpm check~/.openclaw/qqbot/data/. Gateway B, started under the same OS account but a differentOPENCLAW_STATE_DIR, can silently import that backup and overwrite its own config with Gateway A'sappIdandclientSecret, collapsing two supposedly isolated gateway profiles into one shared QQBot identity.User-visible / Behavior Changes
Diagram (if applicable)
N/A
Security Impact (required)
Yes/No): NoYes/No): YesYes/No): NoYes/No): YesYes/No): YesYes, explain risk + mitigation: The fix is focused, covers the actual exploit path, and includes a regression test delta.. Mitigation:node scripts/run-oxlint.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' 'extensions/qqbot/src/engine/utils/data-paths.ts' && node scripts/run-vitest.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' && pnpm build && pnpm checkreportedpassed.Repro + Verification
Environment
Steps
node scripts/run-oxlint.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' 'extensions/qqbot/src/engine/utils/data-paths.ts' && node scripts/run-vitest.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' && pnpm build && pnpm check.Expected
~/.openclaw/qqbot/data/. Gateway B, started under the same OS account but a differentOPENCLAW_STATE_DIR, can silently import that backup and overwrite its own config with Gateway A'sappIdandclientSecret, collapsing two supposedly isolated gateway profiles into one shared QQBot identity.Actual
Evidence
node scripts/run-oxlint.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' 'extensions/qqbot/src/engine/utils/data-paths.ts' && node scripts/run-vitest.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' && pnpm build && pnpm checkreportedpassed.extensions/qqbot/src/engine/utils/data-paths.test.ts(+49/-0)extensions/qqbot/src/engine/utils/data-paths.ts(+12/-3)Human Verification (required)
~/.openclaw/qqbot/data/. Gateway B, started under the same OS account but a differentOPENCLAW_STATE_DIR, can silently import that backup and overwrite its own config with Gateway A'sappIdandclientSecret, collapsing two supposedly isolated gateway profiles into one shared QQBot identity.node scripts/run-oxlint.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' 'extensions/qqbot/src/engine/utils/data-paths.ts' && node scripts/run-vitest.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' && pnpm build && pnpm checkcompleted with statuspassed.Review Conversations
Compatibility / Migration
Yes/No): No - strict state isolation intentionally stops custom-state gateways from reading old home-global QQBot backups.Yes/No): NoYes/No): NoRisks and Mitigations
node scripts/run-oxlint.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' 'extensions/qqbot/src/engine/utils/data-paths.ts' && node scripts/run-vitest.mjs 'extensions/qqbot/src/engine/utils/data-paths.test.ts' && pnpm build && pnpm checkreportedpassedand the changed-file scope stayed limited to the recorded diff.Maintainer Proof Addendum (June 2, 2026)
OPENCLAW_STATE_DIRvalues do not share restore data.saveCredentialBackup,loadCredentialBackup, andgetCredentialBackupFilehelpers with temp state roots and redacted credentials. No live QQBot network call or real secret was used.node --import tsx --input-type=modulewith a temp two-root script that saves Gateway A's backup, attempts to load it from Gateway B, then reloads it from Gateway A.