fix(gateway): block webchat session compaction mutations#70716
fix(gateway): block webchat session compaction mutations#70716drobison00 merged 2 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR extends the existing Confidence Score: 5/5Safe to merge — the change is a targeted, consistent extension of an existing guard with matching test coverage. All findings are at most P2. The fix is consistent with the existing pattern, the error messages match the test assertions, and the createCheckpointFixture helper is a pre-existing utility already used in adjacent tests. No logic, security, or API-contract issues were found. No files require special attention. Reviews (1): Last reviewed commit: "fix(gateway): block webchat session comp..." | Re-trigger Greptile |
a0db036 to
52a7428
Compare
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟠 Webchat session-mutation guard bypass via spoofed client.id (CONTROL_UI) while in webchat mode
DescriptionThe
Vulnerable code: if (!params.client?.connect || !params.isWebchatConnect(params.client.connect)) {
return false;
}
if (params.client.connect.client.id === GATEWAY_CLIENT_IDS.CONTROL_UI) {
return false;
}This is an authorization bypass because RecommendationTreat Concrete fixes (pick one):
Example safer implementation: import { GATEWAY_CLIENT_MODES, normalizeGatewayClientMode, normalizeGatewayClientId } from "../protocol/client-info.js";
const mode = normalizeGatewayClientMode(params.client.connect.client.mode);
const id = normalizeGatewayClientId(params.client.connect.client.id);
if (!params.isWebchatConnect(params.client.connect)) return false;
// Never exempt webchat-mode clients.
if (mode === GATEWAY_CLIENT_MODES.WEBCHAT) {
params.respond(false, undefined, errorShape(...));
return true;
}
// (Optional) exempt only real control UI mode.
if (id === GATEWAY_CLIENT_IDS.CONTROL_UI && mode === GATEWAY_CLIENT_MODES.UI) return false;Also consider moving “client type” classification to server-side signals (origin checks, authenticated role/scopes, or a server-issued client attestation) rather than connect-declared fields. Analyzed PR: #70716 at commit Last updated on: 2026-04-23T18:51:31Z |
fix(gateway): block webchat session compaction mutations
Summary
Describe the problem and fix in 2-5 bullets:
WEBCHAT_UIclients were blocked fromsessions.patchandsessions.delete, but the sibling mutation handlerssessions.compactandsessions.compaction.restorestill ran.rejectWebchatSessionMutation(...)tosessions.compactandsessions.compaction.restore, and expanded the existing gateway regression test to cover both handlers.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
sessions.patchandsessions.delete, leaving the siblingcompactandrestoremutation paths unguarded.Regression Test Plan (if applicable)
For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write
N/A.src/gateway/server.sessions.gateway-server-sessions-a.test.tsWEBCHAT_UIclient cannot patch, delete, compact, or restore sessions, even when the client otherwise has admin-scoped access.User-visible / Behavior Changes
WEBCHAT_UIclients now receive the same invalid-request rejection forsessions.compactandsessions.compaction.restorethat already applied tosessions.patchandsessions.delete.Diagram (if applicable)
Security Impact (required)
Yes, explain risk + mitigation: N/ARepro + Verification
Environment
WEBCHAT_UISteps
WEBCHAT_UIclient connection.sessions.compactandsessions.compaction.restoreRPC requests against an existing session and checkpoint.pnpm test:gateway -- src/gateway/server.sessions.gateway-server-sessions-a.test.ts.Expected
Actual
Evidence
Attach at least one:
Validation command run:
Human Verification (required)
What you personally verified (not just CI), and how:
sessions.compactandsessions.compaction.restoregained the existing webchat guard; ranpnpm test:gateway -- src/gateway/server.sessions.gateway-server-sessions-a.test.ts; confirmed the branch diff only touches the session handler and its regression test.Review Conversations
Compatibility / Migration
Risks and Mitigations