Skip to content

Commit e679566

Browse files
zenglingbiaosteipete
zenglingbiao
and
steipete
authored
fix(sessions): reject non-UTF-8 session-state notice context keys (#111152)
* fix(sessions): reject non-UTF-8 session-state notice context keys * fix(sessions): preserve leading U+FEFF in notice context keys * fix(state): reject corrupt legacy watch markers Co-authored-by: zenglingbiao <[email protected]> --------- Co-authored-by: Peter Steinberger <[email protected]>
1 parent 4934957 commit e679566

4 files changed

Lines changed: 93 additions & 4 deletions

File tree

‎src/sessions/session-state-notices.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Session-state notice context key decoding: strict UTF-8 after hex validation.
2+
import { describe, expect, it } from "vitest";
3+
import { decodeSessionStateNoticeContextKey } from "./session-state-notices.js";
4+
5+
function encodeTarget(sessionKey: string): string {
6+
return `session-state:${Buffer.from(sessionKey, "utf8").toString("hex")}`;
7+
}
8+
9+
describe("decodeSessionStateNoticeContextKey", () => {
10+
it("round-trips a valid encoded session key", () => {
11+
const sessionKey = "agent:main:slack:channel:C01234567";
12+
expect(decodeSessionStateNoticeContextKey(encodeTarget(sessionKey))).toBe(sessionKey);
13+
});
14+
15+
it("round-trips a session key with a leading U+FEFF unchanged", () => {
16+
const sessionKey = "agent:main";
17+
expect(decodeSessionStateNoticeContextKey(encodeTarget(sessionKey))).toBe(sessionKey);
18+
});
19+
20+
it("rejects a context key whose hex payload is not valid UTF-8", () => {
21+
// 0xFF is not valid UTF-8; a forgiving decode would return U+FFFD and let a
22+
// corrupt context key collide with an unrelated watcher cursor.
23+
expect(decodeSessionStateNoticeContextKey("session-state:ff")).toBeUndefined();
24+
});
25+
26+
it("rejects malformed prefixes and hex payloads", () => {
27+
expect(decodeSessionStateNoticeContextKey("other:ff")).toBeUndefined();
28+
expect(decodeSessionStateNoticeContextKey("session-state:")).toBeUndefined();
29+
expect(decodeSessionStateNoticeContextKey("session-state:abc")).toBeUndefined();
30+
expect(decodeSessionStateNoticeContextKey("session-state:zz")).toBeUndefined();
31+
});
32+
});

‎src/sessions/session-state-notices.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,16 @@ export function decodeSessionStateNoticeContextKey(contextKey: string): string |
1717
if (!encoded || encoded.length % 2 !== 0 || !/^[0-9a-f]+$/.test(encoded)) {
1818
return undefined;
1919
}
20-
return Buffer.from(encoded, "hex").toString("utf8");
20+
// encodeNoticeTarget always writes the hex of a valid UTF-8 session key, so a
21+
// payload that fails strict UTF-8 decoding is corrupt: fail closed instead of
22+
// letting U+FFFD collisions acknowledge an unrelated watcher cursor.
23+
try {
24+
return new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }).decode(
25+
Buffer.from(encoded, "hex"),
26+
);
27+
} catch {
28+
return undefined;
29+
}
2130
}
2231

2332
// Terse on purpose: this line lands in model prompts, possibly repeatedly across

‎src/state/openclaw-state-db-session-watch-migration.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,13 @@ function decodeLegacyAmbientWatchMarkerKey(markerKey: string): string | undefine
6565
if (!encoded || encoded.length % 2 !== 0 || !/^[0-9a-f]+$/.test(encoded)) {
6666
return undefined;
6767
}
68-
return Buffer.from(encoded, "hex").toString("utf8");
68+
try {
69+
return new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }).decode(
70+
Buffer.from(encoded, "hex"),
71+
);
72+
} catch {
73+
return undefined;
74+
}
6975
}
7076

7177
export function migrateSessionWatchCursorProvenance(

‎src/state/openclaw-state-db.test.ts‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,12 @@ const LEGACY_AMBIENT_WATCH_PREFIX = "ambient-group-watch:";
137137

138138
function seedLegacySessionWatchCursorSchema(stateDir: string): {
139139
ambientTarget: string;
140+
bomTarget: string;
141+
bomWatcherSessionKey: string;
142+
corruptTarget: string;
140143
databasePath: string;
141144
explicitTarget: string;
145+
replacementWatcherSessionKey: string;
142146
watcherSessionKey: string;
143147
} {
144148
const options = { env: { OPENCLAW_STATE_DIR: stateDir } };
@@ -147,8 +151,13 @@ function seedLegacySessionWatchCursorSchema(stateDir: string): {
147151

148152
const watcherSessionKey = "agent:main:main";
149153
const ambientTarget = "agent:main:telegram:group:ambient";
154+
const bomTarget = "agent:main:telegram:group:bom";
155+
const bomWatcherSessionKey = "agent:main:bom-watcher";
156+
const corruptTarget = "agent:main:telegram:group:corrupt";
150157
const explicitTarget = "agent:main:subagent:explicit";
158+
const replacementWatcherSessionKey = "�";
151159
const markerKey = `${LEGACY_AMBIENT_WATCH_PREFIX}${Buffer.from(watcherSessionKey, "utf8").toString("hex")}`;
160+
const bomMarkerKey = `${LEGACY_AMBIENT_WATCH_PREFIX}${Buffer.from(bomWatcherSessionKey, "utf8").toString("hex")}`;
152161
const orphanMarkerKey = `${LEGACY_AMBIENT_WATCH_PREFIX}${Buffer.from("agent:main:orphan", "utf8").toString("hex")}`;
153162
const { DatabaseSync } = requireNodeSqlite();
154163
const legacy = new DatabaseSync(databasePath);
@@ -185,13 +194,26 @@ function seedLegacySessionWatchCursorSchema(stateDir: string): {
185194
`);
186195
insert.run(watcherSessionKey, ambientTarget, 7, 8, 9, 200);
187196
insert.run(watcherSessionKey, explicitTarget, 3, 4, 5, 300);
197+
insert.run(bomWatcherSessionKey, bomTarget, 10, 11, 12, 500);
198+
insert.run(replacementWatcherSessionKey, corruptTarget, 13, 14, 15, 600);
188199
insert.run(markerKey, ambientTarget, 7, 7, 7, 400);
200+
insert.run(bomMarkerKey, bomTarget, 10, 10, 10, 800);
201+
insert.run(`${LEGACY_AMBIENT_WATCH_PREFIX}ff`, corruptTarget, 13, 13, 13, 900);
189202
insert.run(orphanMarkerKey, "agent:main:telegram:group:orphan", 1, 1, 1, 100);
190203
insert.run(`${LEGACY_AMBIENT_WATCH_PREFIX}not-hex`, ambientTarget, 1, 1, 1, 100);
191204
} finally {
192205
legacy.close();
193206
}
194-
return { ambientTarget, databasePath, explicitTarget, watcherSessionKey };
207+
return {
208+
ambientTarget,
209+
bomTarget,
210+
bomWatcherSessionKey,
211+
corruptTarget,
212+
databasePath,
213+
explicitTarget,
214+
replacementWatcherSessionKey,
215+
watcherSessionKey,
216+
};
195217
}
196218

197219
type PlacementConstraintProbe = {
@@ -1118,7 +1140,7 @@ INSERT INTO macos_port_guardian_records VALUES (4242, 18789, '/usr/bin/ssh', 're
11181140
]);
11191141
expect(repairOpenClawStateDatabaseSchema(options)).toEqual({
11201142
changes: [
1121-
"Migrated shared state session watch cursors → provenance column (1 ambient, 3 sentinels removed)",
1143+
"Migrated shared state session watch cursors → provenance column (2 ambient, 5 sentinels removed)",
11221144
],
11231145
warnings: [],
11241146
});
@@ -1152,6 +1174,24 @@ INSERT INTO macos_port_guardian_records VALUES (4242, 18789, '/usr/bin/ssh', 're
11521174
provenance: "ambient-group",
11531175
updated_at: 400,
11541176
},
1177+
{
1178+
watcher_session_key: seeded.bomWatcherSessionKey,
1179+
target_session_key: seeded.bomTarget,
1180+
last_seen_sequence: 10,
1181+
notified_sequence: 11,
1182+
material_sequence: 12,
1183+
provenance: "ambient-group",
1184+
updated_at: 800,
1185+
},
1186+
{
1187+
watcher_session_key: seeded.replacementWatcherSessionKey,
1188+
target_session_key: seeded.corruptTarget,
1189+
last_seen_sequence: 13,
1190+
notified_sequence: 14,
1191+
material_sequence: 15,
1192+
provenance: "explicit",
1193+
updated_at: 600,
1194+
},
11551195
]);
11561196
expect(readSqliteNumberPragma(migrated.db, "user_version")).toBe(OPENCLAW_STATE_SCHEMA_VERSION);
11571197
expect(
@@ -1180,6 +1220,8 @@ INSERT INTO macos_port_guardian_records VALUES (4242, 18789, '/usr/bin/ssh', 're
11801220
).toEqual([
11811221
{ target_session_key: seeded.explicitTarget, provenance: "explicit" },
11821222
{ target_session_key: seeded.ambientTarget, provenance: "ambient-group" },
1223+
{ target_session_key: seeded.bomTarget, provenance: "ambient-group" },
1224+
{ target_session_key: seeded.corruptTarget, provenance: "explicit" },
11831225
]);
11841226
expect(readSqliteNumberPragma(migrated.db, "user_version")).toBe(OPENCLAW_STATE_SCHEMA_VERSION);
11851227
expect(detectOpenClawStateDatabaseSchemaMigrations(options)).toEqual([]);

0 commit comments

Comments
 (0)