Skip to content

Commit 9a92737

Browse files
dsarnoclaude
andauthored
harden(protocol): bound + charset-constrain the handshake session_id (#547)
The WebSocket handshake accepted an unbounded, arbitrary session_id from the peer, then used it as a registry key, logged it, and hashed it into telemetry. The plugin only ever produces "<slug>@<4hex>" (slug is [a-z0-9-]), so a pattern of ^[A-Za-z0-9._@-]{1,128}$ rejects only malformed or non-plugin clients while accepting every real id. A bad id now fails the handshake at the Pydantic boundary instead of populating the registry. Addresses review finding ST-4 (#527). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 5818173 commit 9a92737

2 files changed

Lines changed: 40 additions & 1 deletion

File tree

src/godot_ai/protocol/envelope.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,12 @@ class HandshakeMessage(BaseModel):
4646
"""Initial handshake sent by the Godot plugin on connection."""
4747

4848
type: str = "handshake"
49-
session_id: str
49+
## Bounded + charset-constrained: the plugin always produces "<slug>@<4hex>"
50+
## (slug is [a-z0-9-]; see connection.gd::_make_session_id), so this only
51+
## rejects a malformed or non-plugin peer. The value is used as a registry
52+
## key, logged, and hashed into telemetry, so an unbounded/arbitrary string
53+
## from an untrusted WS client shouldn't flow downstream unchecked (#527).
54+
session_id: str = Field(pattern=r"^[A-Za-z0-9._@-]{1,128}$")
5055
godot_version: str
5156
project_path: str
5257
plugin_version: str

tests/unit/test_protocol.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
import json
44

5+
import pytest
6+
from pydantic import ValidationError
7+
58
from godot_ai.protocol.envelope import (
69
CommandRequest,
710
CommandResponse,
@@ -134,3 +137,34 @@ def test_server_launch_mode_parsed_when_supplied(self):
134137
}
135138
)
136139
assert msg.server_launch_mode == "dev_venv"
140+
141+
def test_canonical_session_id_accepted(self):
142+
## The plugin's "<slug>@<4hex>" form must validate (connection.gd).
143+
msg = HandshakeMessage(
144+
session_id="my-game@a3f2",
145+
godot_version="4.4.1",
146+
project_path="/tmp/project",
147+
plugin_version="0.0.1",
148+
)
149+
assert msg.session_id == "my-game@a3f2"
150+
151+
@pytest.mark.parametrize(
152+
"bad_id",
153+
[
154+
"", # empty
155+
"has space", # whitespace
156+
"a/b/../etc", # path separators / traversal shape
157+
"x" * 129, # over the 128 bound
158+
"emoji😀", # non-ASCII control/payload
159+
],
160+
)
161+
def test_malformed_session_id_rejected(self, bad_id):
162+
## An untrusted WS peer can't register an arbitrary/oversized id that
163+
## then flows into the registry key, logs, and telemetry hash (#527).
164+
with pytest.raises(ValidationError):
165+
HandshakeMessage(
166+
session_id=bad_id,
167+
godot_version="4.4.1",
168+
project_path="/tmp/project",
169+
plugin_version="0.0.1",
170+
)

0 commit comments

Comments
 (0)