harden(handlers): route every path-taking handler through McpPathValidator (#546)

* 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>

* harden(handlers): address review — uid:// loads, case-fold blocklist, shared helper

Follow-up to the McpPathValidator unification, fixing the code-review findings.

Correctness:
- Restore uid:// and user:// support on load handlers. ResourceLoader accepts
  both (uid:// is an opaque resource id that can't express traversal; user://
  is the user data sandbox), but routing every load through the strict res://
  validator rejected them — a regression for uid:// refs copied from .tscn/.uid
  and for user:// runtime assets. New validate_loadable_path() accepts res://
  (confined), uid:// (as-is), and user:// (confined under the user root), and
  load sites now use it: set_property/assign_resource/resource property dicts,
  open_scene, create_node, attach_script, set_stream, particle mesh/material/
  texture, material assign/shader, ui theme/stylebox, curve, material_values.
- Case-fold the write blocklist. macOS (APFS) and Windows (NTFS) are
  case-insensitive by default, so res://Project.godot resolved to the real
  project.godot and slipped past a case-sensitive compare.
- Add override.cfg to the write blocklist (applied over project.godot at
  startup — same takeover surface as the manifest).
- Reads no longer run the write blocklist: _validate_material_path /
  _validate_res_path / _load_material_from_path / _load_theme_from_params take
  for_write, passed true only by the create/save callers. get_material and
  apply_theme (pure reads) no longer return a spurious "Refusing to write".
- Stop blocking .import writes — editing import config then reimporting is a
  legitimate, recoverable workflow; the blocklist is the startup-execution
  surface (manifest, override.cfg, .godot/) only.

Cleanup / altitude:
- Single error-code choke point: McpPathValidator.path_error / loadable_error
  return a ready error dict (MISSING_REQUIRED_PARAM for empty, VALUE_OUT_OF_RANGE
  for invalid) so every handler reports the same code for the same failure.
  filesystem/script move off their old INVALID_PARAMS wrapping onto this.
- curve set_points validates the load path (the save layer, resource_io.save_to_disk,
  remains the authoritative write-confinement check) instead of double-running
  the write validator.
- Drop the redundant is_empty() pre-check in material_values.load_texture.
- Document the lexical-containment (no symlink resolution) limitation in the
  validator — GDScript has no realpath; the loopback boundary is the control.

Tests (live Godot 4.6.3, all suites 0 failures): uid:// / user:// acceptance,
user:// traversal + unknown-scheme rejection, case-insensitive blocklist,
override.cfg block, .import now allowed; audio fixture moved back to user://
(no repo/EFS pollution). test_path_traversal_guard counts any McpPathValidator
delegation; missing-path tests now assert MISSING_REQUIRED_PARAM.

Addresses review findings 1-10 on GHSA-p5x8-v25q-qw69 (path confinement).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* harden(handlers): address Copilot review on path validation

- Guard the null-byte check against an empty String.chr(0) sentinel in both
  validate_resource_path and validate_loadable_path. On builds that normalize
  embedded nulls away (e.g. 4.3), contains("") would be true and reject every
  path; a String that can't hold a null can't smuggle one, so skip the check.
- Update stale "res:// path" comments in ui_handler (stylebox override) and
  material_values.load_texture — both now accept uid:// / user:// via
  validate_loadable_path.
- Tighten test_path_traversal_guard: assert attach_script is present and require
  >=5 McpPathValidator delegations in script_handler, so a regression where
  attach_script stops validating its path is caught.

Live Godot 4.6.3: path_validator/filesystem/script/ui/material/resource/scene/
audio suites all pass; test_path_traversal_guard green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* style: wrap long func-name tuple in test_path_traversal_guard (ruff E501)

The 5-entry func list exceeded the 100-char line limit after adding
attach_script. Pure formatting; the assertion is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(handlers): align remaining res:// messages with uid:// / user:// support

Copilot re-review: now that the theme / stylebox / shader load paths accept
uid:// and user:// via loadable_error, the surrounding comments, the
build_layout docstring, the stylebox fallback error ("expects a res:// path"),
and the shader_path missing-arg error still said res:// only. Text-only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: dsarno

plugin/addons/godot_ai/handlers/audio_handler.gd

@@ -101,6 +101,10 @@ func set_stream(params: Dictionary) -> Dictionary:
 	if stream_path.is_empty():
 		return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: stream_path")
 
+	var stream_path_err = McpPathValidator.loadable_error(stream_path, "stream_path")
+	if stream_path_err != null:
+		return stream_path_err
+
 	var resolved := _resolve_player(player_path)
 	if resolved.has("error"):
 		return resolved
@@ -259,8 +263,9 @@ func list_streams(params: Dictionary) -> Dictionary:
 	var root: String = params.get("root", "res://")
 	var include_duration: bool = bool(params.get("include_duration", true))
 
-	if not root.begins_with("res://"):
-		return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "root must start with res://")
+	var root_err = McpPathValidator.path_error(root, "root")
+	if root_err != null:
+		return root_err
 
 	var efs := EditorInterface.get_resource_filesystem()
 	if efs == null:

plugin/addons/godot_ai/handlers/autoload_handler.gd

@@ -33,8 +33,9 @@ func add_autoload(params: Dictionary) -> Dictionary:
 		return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: name")
 	if path.is_empty():
 		return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: path")
-	if not path.begins_with("res://"):
-		return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "Path must start with res:// (got: %s)" % path)
+	var path_err = McpPathValidator.path_error(path, "path")
+	if path_err != null:
+		return path_err
 	if not FileAccess.file_exists(path):
 		return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "File not found: %s" % path)
 

plugin/addons/godot_ai/handlers/curve_handler.gd

@@ -38,6 +38,9 @@ func set_points(params: Dictionary) -> Dictionary:
 	var node: Node = null
 	var curve_created := false
 	if has_file_target:
+		var rpath_err = McpPathValidator.loadable_error(resource_path, "resource_path")
+		if rpath_err != null:
+			return rpath_err
 		if not ResourceLoader.exists(resource_path):
 			return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Resource not found: %s" % resource_path)
 		# ResourceLoader.load() returns Godot's cached Resource. Duplicate

plugin/addons/godot_ai/handlers/filesystem_handler.gd

@@ -9,9 +9,9 @@ const ErrorCodes := preload("res://addons/godot_ai/utils/error_codes.gd")
 func read_file(params: Dictionary) -> Dictionary:
 	var path: String = params.get("path", "")
 
-	var path_err := McpPathValidator.validate_resource_path(path)
-	if not path_err.is_empty():
-		return ErrorCodes.make(ErrorCodes.INVALID_PARAMS, path_err)
+	var path_err = McpPathValidator.path_error(path, "path")
+	if path_err != null:
+		return path_err
 
 	if not FileAccess.file_exists(path):
 		return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "File not found: %s" % path)
@@ -37,9 +37,9 @@ func write_file(params: Dictionary) -> Dictionary:
 	var path: String = params.get("path", "")
 	var content: String = params.get("content", "")
 
-	var path_err := McpPathValidator.validate_resource_path(path)
-	if not path_err.is_empty():
-		return ErrorCodes.make(ErrorCodes.INVALID_PARAMS, path_err)
+	var path_err = McpPathValidator.path_error(path, "path", true)
+	if path_err != null:
+		return path_err
 
 	# Ensure parent directory exists
 	var dir_path := path.get_base_dir()

plugin/addons/godot_ai/handlers/material_handler.gd

@@ -40,7 +40,7 @@ func create_material(params: Dictionary) -> Dictionary:
 	var shader_path: String = params.get("shader_path", "")
 	var overwrite: bool = params.get("overwrite", false)
 
-	var err := _validate_material_path(path, "path")
+	var err := _validate_material_path(path, "path", true)
 	if err != null:
 		return err
 
@@ -65,8 +65,11 @@ func create_material(params: Dictionary) -> Dictionary:
 		if shader_path.is_empty():
 			return ErrorCodes.make(
 				ErrorCodes.INVALID_PARAMS,
-				"ShaderMaterial requires shader_path (res:// path to a .gdshader)"
+				"ShaderMaterial requires shader_path (res:// / uid:// / user:// path to a .gdshader)"
 			)
+		var shader_path_err = McpPathValidator.loadable_error(shader_path, "shader_path")
+		if shader_path_err != null:
+			return shader_path_err
 		if not ResourceLoader.exists(shader_path):
 			return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Shader not found: %s" % shader_path)
 		var shader_res := ResourceLoader.load(shader_path)
@@ -111,7 +114,7 @@ func create_material(params: Dictionary) -> Dictionary:
 # ============================================================================
 
 func set_param(params: Dictionary) -> Dictionary:
-	var load_result := _load_material_from_path(params.get("path", ""))
+	var load_result := _load_material_from_path(params.get("path", ""), true)
 	if load_result.has("error"):
 		return load_result
 	var mat: Material = load_result.material
@@ -169,7 +172,7 @@ func set_param(params: Dictionary) -> Dictionary:
 # ============================================================================
 
 func set_shader_param(params: Dictionary) -> Dictionary:
-	var load_result := _load_material_from_path(params.get("path", ""))
+	var load_result := _load_material_from_path(params.get("path", ""), true)
 	if load_result.has("error"):
 		return load_result
 	var mat: Material = load_result.material
@@ -297,8 +300,9 @@ func list_materials(params: Dictionary) -> Dictionary:
 	var root: String = params.get("root", "res://")
 	var type_filter: String = params.get("type", "")
 
-	if not root.begins_with("res://"):
-		return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "root must start with res://")
+	var root_err = McpPathValidator.path_error(root, "root")
+	if root_err != null:
+		return root_err
 
 	var efs := EditorInterface.get_resource_filesystem()
 	if efs == null:
@@ -366,6 +370,9 @@ func assign_material(params: Dictionary) -> Dictionary:
 	var mat: Material = null
 	var material_created := false
 	if not resource_path.is_empty():
+		var rpath_err = McpPathValidator.loadable_error(resource_path, "resource_path")
+		if rpath_err != null:
+			return rpath_err
 		if not ResourceLoader.exists(resource_path):
 			if create_if_missing:
 				# We'd need to create a new file here — refuse; callers should
@@ -463,7 +470,7 @@ func apply_to_node(params: Dictionary) -> Dictionary:
 	var save_to: String = params.get("save_to", "")
 	var saved := false
 	if not save_to.is_empty():
-		var save_err_validation := _validate_material_path(save_to, "save_to")
+		var save_err_validation := _validate_material_path(save_to, "save_to", true)
 		if save_err_validation != null:
 			return save_err_validation
 		var dir_path := save_to.get_base_dir()
@@ -564,7 +571,7 @@ func apply_preset(params: Dictionary) -> Dictionary:
 			"Material already exists at %s (pass overwrite=true to replace)" % path
 		)
 
-	var path_err := _validate_material_path(path, "path")
+	var path_err := _validate_material_path(path, "path", true)
 	if path_err != null:
 		return path_err
 
@@ -665,11 +672,12 @@ static func _reverse_type_map() -> Dictionary:
 	return out
 
 
-static func _validate_material_path(path: String, param_name: String) -> Variant:
+static func _validate_material_path(path: String, param_name: String, for_write: bool = false) -> Variant:
 	if path.is_empty():
 		return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: %s" % param_name)
-	if not path.begins_with("res://"):
-		return ErrorCodes.make(ErrorCodes.INVALID_PARAMS, "%s must start with res:// (got %s)" % [param_name, path])
+	var path_err := McpPathValidator.validate_resource_path(path, for_write)
+	if not path_err.is_empty():
+		return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "%s: %s" % [param_name, path_err])
 	var has_suffix := false
 	for s in _SUPPORTED_SUFFIXES:
 		if path.ends_with(s):
@@ -683,8 +691,8 @@ static func _validate_material_path(path: String, param_name: String) -> Variant
 	return null
 
 
-func _load_material_from_path(path: String) -> Dictionary:
-	var err := _validate_material_path(path, "path")
+func _load_material_from_path(path: String, for_write: bool = false) -> Dictionary:
+	var err := _validate_material_path(path, "path", for_write)
 	if err != null:
 		return err
 	if not ResourceLoader.exists(path):

plugin/addons/godot_ai/handlers/material_values.gd

@@ -153,9 +153,10 @@ static func parse_gradient(value: Variant) -> Variant:
 	return grad
 
 
-## Load a Texture2D from a res:// path. Returns null on failure.
+## Load a Texture2D from a res:// / uid:// / user:// path (validate_loadable_path).
+## Returns null on failure (including a path that fails confinement / traversal).
 static func load_texture(path: String) -> Texture2D:
-	if path.is_empty():
+	if not McpPathValidator.validate_loadable_path(path).is_empty():
 		return null
 	if not ResourceLoader.exists(path):
 		return null

plugin/addons/godot_ai/handlers/node_handler.gd

@@ -39,8 +39,9 @@ func create_node(params: Dictionary) -> Dictionary:
 		# scene instance (foldout icon, the .tscn stores a reference instead of
 		# an exploded subtree). Descendants remain owned by their sub-scene;
 		# setting their owner to our scene_root would break the instance link.
-		if not scene_path.begins_with("res://"):
-			return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "scene_path must start with res://")
+		var scene_path_err = McpPathValidator.loadable_error(scene_path, "scene_path")
+		if scene_path_err != null:
+			return scene_path_err
 		if not ResourceLoader.exists(scene_path):
 			return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Scene not found: %s" % scene_path)
 		var packed_scene = ResourceLoader.load(scene_path)
@@ -222,6 +223,9 @@ func set_property(params: Dictionary) -> Dictionary:
 		if value == "":
 			value = null
 		else:
+			var value_path_err = McpPathValidator.loadable_error(value, "value")
+			if value_path_err != null:
+				return value_path_err
 			if not ResourceLoader.exists(value):
 				return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Resource not found: %s" % value)
 			var loaded := ResourceLoader.load(value)

plugin/addons/godot_ai/handlers/particle_handler.gd

@@ -341,6 +341,9 @@ func _set_draw_pass_gpu_3d(node: GPUParticles3D, node_path: String, pass_idx: in
 	if int(node.draw_passes) >= pass_idx:
 		existing_mesh = node.get(property_name) as Mesh
 	if not mesh_path.is_empty():
+		var mesh_path_err = McpPathValidator.loadable_error(mesh_path, "mesh_path")
+		if mesh_path_err != null:
+			return mesh_path_err
 		if not ResourceLoader.exists(mesh_path):
 			return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Mesh not found: %s" % mesh_path)
 		var loaded := ResourceLoader.load(mesh_path)
@@ -357,6 +360,9 @@ func _set_draw_pass_gpu_3d(node: GPUParticles3D, node_path: String, pass_idx: in
 
 	var material: Material = null
 	if not material_path.is_empty():
+		var material_path_err = McpPathValidator.loadable_error(material_path, "material_path")
+		if material_path_err != null:
+			return material_path_err
 		if not ResourceLoader.exists(material_path):
 			return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Material not found: %s" % material_path)
 		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
 	var mesh: Mesh = node.mesh
 	var old_mesh: Mesh = mesh
 	if not mesh_path.is_empty():
+		var mesh_path_err = McpPathValidator.loadable_error(mesh_path, "mesh_path")
+		if mesh_path_err != null:
+			return mesh_path_err
 		if not ResourceLoader.exists(mesh_path):
 			return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Mesh not found: %s" % mesh_path)
 		var loaded := ResourceLoader.load(mesh_path)
@@ -417,6 +426,9 @@ func _set_draw_pass_cpu_3d(node: CPUParticles3D, node_path: String, mesh_path: S
 	var material: Material = null
 	var old_material: Material = node.material_override
 	if not material_path.is_empty():
+		var material_path_err = McpPathValidator.loadable_error(material_path, "material_path")
+		if material_path_err != null:
+			return material_path_err
 		if not ResourceLoader.exists(material_path):
 			return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Material not found: %s" % material_path)
 		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
 func _set_draw_pass_2d(node: Node, node_path: String, texture_path: String) -> Dictionary:
 	if texture_path.is_empty():
 		return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "2D particles require texture param")
+	var texture_path_err = McpPathValidator.loadable_error(texture_path, "texture_path")
+	if texture_path_err != null:
+		return texture_path_err
 	if not ResourceLoader.exists(texture_path):
 		return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Texture not found: %s" % texture_path)
 	var tex := ResourceLoader.load(texture_path)

plugin/addons/godot_ai/handlers/resource_handler.gd

@@ -64,8 +64,9 @@ func load_resource(params: Dictionary) -> Dictionary:
 	if path.is_empty():
 		return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: path")
 
-	if not path.begins_with("res://"):
-		return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "Path must start with res://")
+	var path_err = McpPathValidator.loadable_error(path, "path")
+	if path_err != null:
+		return path_err
 
 	if not ResourceLoader.exists(path):
 		return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Resource not found: %s" % path)
@@ -112,6 +113,10 @@ func assign_resource(params: Dictionary) -> Dictionary:
 	if resource_path.is_empty():
 		return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: resource_path")
 
+	var rpath_err = McpPathValidator.loadable_error(resource_path, "resource_path")
+	if rpath_err != null:
+		return rpath_err
+
 	var _resolved := McpNodeValidator.resolve_or_error(node_path, "node_path")
 	if _resolved.has("error"):
 		return _resolved
@@ -248,6 +253,9 @@ static func _apply_resource_properties(res: Resource, properties: Dictionary) ->
 			if v == "":
 				v = null
 			else:
+				var vpath_err = McpPathValidator.loadable_error(v, "property '%s'" % key)
+				if vpath_err != null:
+					return vpath_err
 				var loaded := ResourceLoader.load(v)
 				if loaded == null:
 					return ErrorCodes.make(

plugin/addons/godot_ai/handlers/scene_handler.gd

@@ -90,8 +90,9 @@ func create_scene(params: Dictionary) -> Dictionary:
 	if path.is_empty():
 		return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: path")
 
-	if not path.begins_with("res://"):
-		return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "Path must start with res://")
+	var path_err = McpPathValidator.path_error(path, "path", true)
+	if path_err != null:
+		return path_err
 
 	if not path.ends_with(".tscn") and not path.ends_with(".scn"):
 		path += ".tscn"
@@ -148,6 +149,10 @@ func open_scene(params: Dictionary) -> Dictionary:
 	if path.is_empty():
 		return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: path")
 
+	var path_err = McpPathValidator.loadable_error(path, "path")
+	if path_err != null:
+		return path_err
+
 	if not ResourceLoader.exists(path):
 		return ErrorCodes.make(ErrorCodes.RESOURCE_NOT_FOUND, "Scene not found: %s" % path)
 
@@ -202,8 +207,9 @@ func save_scene_as(params: Dictionary) -> Dictionary:
 	if path.is_empty():
 		return ErrorCodes.make(ErrorCodes.MISSING_REQUIRED_PARAM, "Missing required param: path")
 
-	if not path.begins_with("res://"):
-		return ErrorCodes.make(ErrorCodes.VALUE_OUT_OF_RANGE, "Path must start with res://")
+	var path_err = McpPathValidator.path_error(path, "path", true)
+	if path_err != null:
+		return path_err
 
 	if not path.ends_with(".tscn") and not path.ends_with(".scn"):
 		path += ".tscn"