Skip to content

Commit 51b29ef

Browse files
dsarnoclaude
andcommitted
harden(handlers): route every path-taking handler through McpPathValidator
The strong traversal validator from #347 was wired into only filesystem and script handlers; ~12 other path-taking handlers used a bare begins_with("res://") check (which does not reject "..") or no check at all (relying solely on ResourceLoader.exists/load). This unifies all of them on McpPathValidator and extends the validator with two checks. Validator (utils/path_validator.gd): - Reject embedded null bytes (truncation trap — the path written could differ from the one validated). - New for_write flag: write callers additionally refuse res://project.godot, the res://.godot/ metadata dir, and .import sidecars (overwriting these corrupts the project / import cache). Reads still permit inspecting them. Signature is backward-compatible (for_write defaults to false). Writers now validate with for_write=true (closes the traversal-write primitive): resource_io.save_to_disk (backs resource/environment/texture/curve saves), scene create_scene/save_scene_as, material/theme save helpers, filesystem write_text, script create/patch. Load/read sites now validate (closes the unvalidated-load surface, incl. the @tool-script-execution risk from open_scene/create_node): resource load/assign + nested object-property loads, scene open_scene, script attach_script, node create_node scene_path + set_property resource values, audio set_stream + list root, material assign/shader/list, ui theme + stylebox, autoload path, curve set_points, particle mesh/ material/texture, material_values.load_texture. Path-validation rejections report VALUE_OUT_OF_RANGE (the code these handlers already used for "must start with res://"), keeping the INVALID_PARAMS catch-all count under the audit-v2 #21 ceiling enforced by test_error_code_distribution. The four pre-existing validator sites that already wrapped errors as INVALID_PARAMS (filesystem, script create/read/patch/find, resource_io save, material _validate_material_path) are left unchanged. set_stream and the other load handlers now reject user:// (consistent with the validator's existing test_user_prefix_rejected policy); the audio test fixture moves from user:// to a res:// .tres registered via EditorFileSystem.update_file. Tests (run against live Godot 4.6.3): validator unit tests for null-byte + write blocklist + read-allows; scene traversal/manifest-overwrite rejection. All affected suites pass (path_validator/scene/resource/node/material/theme/ curve/particle/ui/audio/autoload/filesystem/script). test_error_code_distribution passes. Parse check clean. Addresses advisory GHSA-p5x8-v25q-qw69 (GH-1, GH-2, GH-3, GH-4). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent be40e1c commit 51b29ef

18 files changed

Lines changed: 212 additions & 41 deletions

plugin/addons/godot_ai/handlers/audio_handler.gd

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ func set_stream(params: Dictionary) -> Dictionary:
101101
if stream_path.is_empty():
102102
return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: stream_path")
103103

104+
var stream_path_err := McpPathValidator.validate_resource_path(stream_path)
105+
if not stream_path_err.is_empty():
106+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, stream_path_err)
107+
104108
var resolved := _resolve_player(player_path)
105109
if resolved.has("error"):
106110
return resolved
@@ -259,8 +263,9 @@ func list_streams(params: Dictionary) -> Dictionary:
259263
var root: String = params.get("root", "res://")
260264
var include_duration: bool = bool(params.get("include_duration", true))
261265

262-
if not root.begins_with("res://"):
263-
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "root must start with res://")
266+
var root_err := McpPathValidator.validate_resource_path(root)
267+
if not root_err.is_empty():
268+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, root_err)
264269

265270
var efs := EditorInterface.get_resource_filesystem()
266271
if efs == null:

plugin/addons/godot_ai/handlers/autoload_handler.gd

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@ func add_autoload(params: Dictionary) -> Dictionary:
3333
return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: name")
3434
if path.is_empty():
3535
return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: path")
36-
if not path.begins_with("res://"):
37-
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "Path must start with res:// (got: %s)" % path)
36+
var path_err := McpPathValidator.validate_resource_path(path)
37+
if not path_err.is_empty():
38+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, path_err)
3839
if not FileAccess.file_exists(path):
3940
return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "File not found: %s" % path)
4041

plugin/addons/godot_ai/handlers/curve_handler.gd

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ func set_points(params: Dictionary) -> Dictionary:
3838
var node: Node = null
3939
var curve_created := false
4040
if has_file_target:
41+
var rpath_err := McpPathValidator.validate_resource_path(resource_path, true)
42+
if not rpath_err.is_empty():
43+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, rpath_err)
4144
if not ResourceLoader.exists(resource_path):
4245
return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Resource not found: %s" % resource_path)
4346
# ResourceLoader.load() returns Godot's cached Resource. Duplicate

plugin/addons/godot_ai/handlers/filesystem_handler.gd

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func write_file(params: Dictionary) -> Dictionary:
3737
var path: String = params.get("path", "")
3838
var content: String = params.get("content", "")
3939

40-
var path_err := McpPathValidator.validate_resource_path(path)
40+
var path_err := McpPathValidator.validate_resource_path(path, true)
4141
if not path_err.is_empty():
4242
return ErrorCodes.make(ErrorCodes.INVALID_PARAMS, path_err)
4343

plugin/addons/godot_ai/handlers/material_handler.gd

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ func create_material(params: Dictionary) -> Dictionary:
6767
ErrorCodes.INVALID_PARAMS,
6868
"ShaderMaterial requires shader_path (res:// path to a .gdshader)"
6969
)
70+
var shader_path_err := McpPathValidator.validate_resource_path(shader_path)
71+
if not shader_path_err.is_empty():
72+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, shader_path_err)
7073
if not ResourceLoader.exists(shader_path):
7174
return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Shader not found: %s" % shader_path)
7275
var shader_res := ResourceLoader.load(shader_path)
@@ -297,8 +300,9 @@ func list_materials(params: Dictionary) -> Dictionary:
297300
var root: String = params.get("root", "res://")
298301
var type_filter: String = params.get("type", "")
299302

300-
if not root.begins_with("res://"):
301-
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "root must start with res://")
303+
var root_err := McpPathValidator.validate_resource_path(root)
304+
if not root_err.is_empty():
305+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, root_err)
302306

303307
var efs := EditorInterface.get_resource_filesystem()
304308
if efs == null:
@@ -366,6 +370,9 @@ func assign_material(params: Dictionary) -> Dictionary:
366370
var mat: Material = null
367371
var material_created := false
368372
if not resource_path.is_empty():
373+
var rpath_err := McpPathValidator.validate_resource_path(resource_path)
374+
if not rpath_err.is_empty():
375+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, rpath_err)
369376
if not ResourceLoader.exists(resource_path):
370377
if create_if_missing:
371378
# We'd need to create a new file here — refuse; callers should
@@ -668,8 +675,9 @@ static func _reverse_type_map() -> Dictionary:
668675
static func _validate_material_path(path: String, param_name: String) -> Variant:
669676
if path.is_empty():
670677
return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: %s" % param_name)
671-
if not path.begins_with("res://"):
672-
return ErrorCodes.make(ErrorCodes.INVALID_PARAMS, "%s must start with res:// (got %s)" % [param_name, path])
678+
var path_err := McpPathValidator.validate_resource_path(path, true)
679+
if not path_err.is_empty():
680+
return ErrorCodes.make(ErrorCodes.INVALID_PARAMS, "%s: %s" % [param_name, path_err])
673681
var has_suffix := false
674682
for s in _SUPPORTED_SUFFIXES:
675683
if path.ends_with(s):

plugin/addons/godot_ai/handlers/material_values.gd

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,13 @@ static func parse_gradient(value: Variant) -> Variant:
153153
return grad
154154

155155

156-
## Load a Texture2D from a res:// path. Returns null on failure.
156+
## Load a Texture2D from a res:// path. Returns null on failure (including a
157+
## path that fails res:// confinement / traversal validation).
157158
static func load_texture(path: String) -> Texture2D:
158159
if path.is_empty():
159160
return null
161+
if not McpPathValidator.validate_resource_path(path).is_empty():
162+
return null
160163
if not ResourceLoader.exists(path):
161164
return null
162165
var res := ResourceLoader.load(path)

plugin/addons/godot_ai/handlers/node_handler.gd

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,9 @@ func create_node(params: Dictionary) -> Dictionary:
3939
# scene instance (foldout icon, the .tscn stores a reference instead of
4040
# an exploded subtree). Descendants remain owned by their sub-scene;
4141
# setting their owner to our scene_root would break the instance link.
42-
if not scene_path.begins_with("res://"):
43-
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "scene_path must start with res://")
42+
var scene_path_err := McpPathValidator.validate_resource_path(scene_path)
43+
if not scene_path_err.is_empty():
44+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, scene_path_err)
4445
if not ResourceLoader.exists(scene_path):
4546
return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Scene not found: %s" % scene_path)
4647
var packed_scene = ResourceLoader.load(scene_path)
@@ -222,6 +223,9 @@ func set_property(params: Dictionary) -> Dictionary:
222223
if value == "":
223224
value = null
224225
else:
226+
var value_path_err := McpPathValidator.validate_resource_path(value)
227+
if not value_path_err.is_empty():
228+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, value_path_err)
225229
if not ResourceLoader.exists(value):
226230
return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Resource not found: %s" % value)
227231
var loaded := ResourceLoader.load(value)

plugin/addons/godot_ai/handlers/particle_handler.gd

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,9 @@ func _set_draw_pass_gpu_3d(node: GPUParticles3D, node_path: String, pass_idx: in
341341
if int(node.draw_passes) >= pass_idx:
342342
existing_mesh = node.get(property_name) as Mesh
343343
if not mesh_path.is_empty():
344+
var mesh_path_err := McpPathValidator.validate_resource_path(mesh_path)
345+
if not mesh_path_err.is_empty():
346+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, mesh_path_err)
344347
if not ResourceLoader.exists(mesh_path):
345348
return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Mesh not found: %s" % mesh_path)
346349
var loaded := ResourceLoader.load(mesh_path)
@@ -357,6 +360,9 @@ func _set_draw_pass_gpu_3d(node: GPUParticles3D, node_path: String, pass_idx: in
357360

358361
var material: Material = null
359362
if not material_path.is_empty():
363+
var material_path_err := McpPathValidator.validate_resource_path(material_path)
364+
if not material_path_err.is_empty():
365+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, material_path_err)
360366
if not ResourceLoader.exists(material_path):
361367
return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Material not found: %s" % material_path)
362368
var loaded_mat := ResourceLoader.load(material_path)
@@ -407,6 +413,9 @@ func _set_draw_pass_cpu_3d(node: CPUParticles3D, node_path: String, mesh_path: S
407413
var mesh: Mesh = node.mesh
408414
var old_mesh: Mesh = mesh
409415
if not mesh_path.is_empty():
416+
var mesh_path_err := McpPathValidator.validate_resource_path(mesh_path)
417+
if not mesh_path_err.is_empty():
418+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, mesh_path_err)
410419
if not ResourceLoader.exists(mesh_path):
411420
return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Mesh not found: %s" % mesh_path)
412421
var loaded := ResourceLoader.load(mesh_path)
@@ -417,6 +426,9 @@ func _set_draw_pass_cpu_3d(node: CPUParticles3D, node_path: String, mesh_path: S
417426
var material: Material = null
418427
var old_material: Material = node.material_override
419428
if not material_path.is_empty():
429+
var material_path_err := McpPathValidator.validate_resource_path(material_path)
430+
if not material_path_err.is_empty():
431+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, material_path_err)
420432
if not ResourceLoader.exists(material_path):
421433
return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Material not found: %s" % material_path)
422434
var loaded_mat := ResourceLoader.load(material_path)
@@ -447,6 +459,9 @@ func _set_draw_pass_cpu_3d(node: CPUParticles3D, node_path: String, mesh_path: S
447459
func _set_draw_pass_2d(node: Node, node_path: String, texture_path: String) -> Dictionary:
448460
if texture_path.is_empty():
449461
return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "2D particles require texture param")
462+
var texture_path_err := McpPathValidator.validate_resource_path(texture_path)
463+
if not texture_path_err.is_empty():
464+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, texture_path_err)
450465
if not ResourceLoader.exists(texture_path):
451466
return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Texture not found: %s" % texture_path)
452467
var tex := ResourceLoader.load(texture_path)

plugin/addons/godot_ai/handlers/resource_handler.gd

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,9 @@ func load_resource(params: Dictionary) -> Dictionary:
6464
if path.is_empty():
6565
return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: path")
6666

67-
if not path.begins_with("res://"):
68-
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "Path must start with res://")
67+
var path_err := McpPathValidator.validate_resource_path(path)
68+
if not path_err.is_empty():
69+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, path_err)
6970

7071
if not ResourceLoader.exists(path):
7172
return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Resource not found: %s" % path)
@@ -112,6 +113,10 @@ func assign_resource(params: Dictionary) -> Dictionary:
112113
if resource_path.is_empty():
113114
return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: resource_path")
114115

116+
var rpath_err := McpPathValidator.validate_resource_path(resource_path)
117+
if not rpath_err.is_empty():
118+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, rpath_err)
119+
115120
var _resolved := McpNodeValidator.resolve_or_error(node_path, "node_path")
116121
if _resolved.has("error"):
117122
return _resolved
@@ -248,6 +253,12 @@ static func _apply_resource_properties(res: Resource, properties: Dictionary) ->
248253
if v == "":
249254
v = null
250255
else:
256+
var vpath_err := McpPathValidator.validate_resource_path(v)
257+
if not vpath_err.is_empty():
258+
return ErrorCodes.make(
259+
ErrorCodes.VALUE_OUT_OF_RANGE,
260+
"%s (property '%s')" % [vpath_err, key]
261+
)
251262
var loaded := ResourceLoader.load(v)
252263
if loaded == null:
253264
return ErrorCodes.make(

plugin/addons/godot_ai/handlers/scene_handler.gd

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,9 @@ func create_scene(params: Dictionary) -> Dictionary:
9090
if path.is_empty():
9191
return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: path")
9292

93-
if not path.begins_with("res://"):
94-
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "Path must start with res://")
93+
var path_err := McpPathValidator.validate_resource_path(path, true)
94+
if not path_err.is_empty():
95+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, path_err)
9596

9697
if not path.ends_with(".tscn") and not path.ends_with(".scn"):
9798
path += ".tscn"
@@ -148,6 +149,10 @@ func open_scene(params: Dictionary) -> Dictionary:
148149
if path.is_empty():
149150
return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: path")
150151

152+
var path_err := McpPathValidator.validate_resource_path(path)
153+
if not path_err.is_empty():
154+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, path_err)
155+
151156
if not ResourceLoader.exists(path):
152157
return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Scene not found: %s" % path)
153158

@@ -202,8 +207,9 @@ func save_scene_as(params: Dictionary) -> Dictionary:
202207
if path.is_empty():
203208
return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: path")
204209

205-
if not path.begins_with("res://"):
206-
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "Path must start with res://")
210+
var path_err := McpPathValidator.validate_resource_path(path, true)
211+
if not path_err.is_empty():
212+
return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, path_err)
207213

208214
if not path.ends_with(".tscn") and not path.ends_with(".scn"):
209215
path += ".tscn"

0 commit comments

Comments
 (0)