diff --git a/docs/architecture/scripting_sandbox.md b/docs/architecture/scripting_sandbox.md new file mode 100644 index 0000000..c1ebc97 --- /dev/null +++ b/docs/architecture/scripting_sandbox.md @@ -0,0 +1,159 @@ +# GDScript Sandboxing for User-Written Scripts + +> **Status:** Proposed +> **Owner:** Platform team +> **Related:** Content moderation & age verification (scripts are one of the +> three moderated content types), avatar component system + +## 1. Overview + +This document evaluates how to safely run **user-written scripts** as part of +UGC (user-generated content) in **agp-game**, and recommends an approach. + +**Bottom line: Godot 4 has no built-in security sandbox for GDScript, and +none of the three options named in the original task hold up under testing.** +A script running anywhere inside the Godot process — however it's loaded, +parented, or wrapped — runs with full engine privileges: arbitrary process +execution, full filesystem read/write, and raw network access. Sandboxing +has to happen *outside* the scripting language, not through an in-engine +restriction mechanism, because no such mechanism exists. + +This was verified directly against Godot 4.6.2 (`godot --headless`, evidence +in `sandbox_probe/` on this branch) rather than assumed from documentation, +some of which (see §2.1) describes APIs that don't actually exist in this +version. + +## 2. Options Evaluated + +### 2.1 Godot's built-in "ScriptServer" restrictions — does not exist + +There is no `ScriptServer` class in Godot 4.6.2: + +``` +ClassDB.class_exists("ScriptServer") → false +``` + +There's also no project setting to disable engine subsystems for scripts — +none of `application/run/disable_os`, `disable_file_access`, or +`disable_network` exist as settings, and `ProjectSettings.get_property_list()` +returns nothing matching "sandbox", "restrict", or "security". Any plan that +assumes a flag like this exists needs to be dropped; it was likely conflated +with sandboxing features from a different engine/scripting ecosystem, or +with **export template stripping** (which controls what ships in a build, +not what a script can call at runtime). + +Directly probed capabilities of an ordinary GDScript running as a normal +node script, no special setup: + +| Capability | Available to any script? | +|---|---| +| `OS.execute(...)` — run arbitrary processes | ✅ Yes | +| `FileAccess.open(...)` — read/write any path the OS user can reach | ✅ Yes (read `/etc/hostname`, write `/tmp/...` both succeeded) | +| `TCPServer` / `HTTPClient` / `HTTPRequest` / `PacketPeerUDP` / `WebSocketPeer` | ✅ All instantiate and work | +| `ClassDB.instantiate(...)` — construct any engine class by name | ✅ Yes | +| Loading and running a second script from source text at runtime (`GDScript.new()` + `.reload()`) | ✅ Yes | + +**Verdict: rejected.** There is nothing to configure here — the restriction +surface this option assumed doesn't exist in the engine. + +### 2.2 Isolated SubViewport with a limited API — doesn't isolate scripts + +`SubViewport` is a **rendering and input isolation** primitive (it gives a +scene subtree its own render target and can gate input propagation) — it is +not a security or scripting boundary. A script attached to any node inside a +`SubViewport` still runs in the same process, on the same GDScript VM, with +the exact same access to `OS`, `FileAccess`, networking classes, and +`ClassDB` as a script anywhere else in the tree. Nothing about being +"inside" a SubViewport changes what API calls are reachable from a script. + +**Verdict: rejected.** This solves a rendering/UI-composition problem, not +the security problem the task is asking about. Worth keeping in mind for +other purposes (e.g. isolated preview rendering of user avatars), but it +provides zero script sandboxing. + +### 2.3 Custom GDScript interpreter with an allowlist — right idea, wrong layer for "custom interpreter" + +Two ways to read this option, tested separately: + +**(a) Write/embed a separate GDScript parser+interpreter.** Not +practical — this means re-implementing (and then indefinitely maintaining, +in lockstep with every Godot release) a full language interpreter outside +the engine. Far more engineering than the problem justifies. + +**(b) Use Godot's built-in `Expression` class as a restricted subset.** +`Expression` *looks* like a natural allowlist mechanism — it only evaluates +a single expression, no control flow, no arbitrary statements. But it isn't +safe by default: it can still reach through to unsafe engine methods if you +aren't careful about what object you hand it as a base instance: + +```gdscript +var expr = Expression.new() +expr.parse("get_environment('HOME')", ["OS"]) +expr.execute([], OS) # → "/home/daddy" (successfully called OS.get_environment) +``` + +So `Expression` is a viable building block only if the host **never passes +a base instance (or arguments) that exposes anything sensitive** — i.e. the +allowlisting work still has to happen at the call site, `Expression` itself +enforces none of it. + +**Static text pattern-matching (blocklisting dangerous substrings like +`"OS.execute"` before parsing) was also probed and is not a real +defense** — it's trivially defeated by string concatenation, `call()`, +`Callable`, or any other indirection, and produces false confidence. + +**Verdict: partially viable, but only as a foundation, not a solution on +its own.** A hand-built, narrow, data-driven DSL (not full GDScript, not +raw `Expression`) with an explicit allowlist of safe operations and *no +ability to reference engine singletons or arbitrary objects* is the only +version of "custom interpreter" that's actually safe. This is real +engineering effort, not a config flag. + +## 3. Recommendation + +**Don't run untrusted GDScript in-process at all.** Given none of the three +in-engine options provide a real boundary, the two approaches actually worth +building are, in order of recommended priority: + +1. **Primary: a narrow, non-Turing-complete content DSL**, not general + scripting. Most UGC scripting needs in a game like this (custom + interactions, simple triggers, item behaviors) don't need a general + programming language — they need a small set of declarative + verbs/conditions (e.g. `on_interact: give_item("torch")`, + `on_enter: play_sound("chime")`). Expressed as data (JSON/Resource, not + code) and interpreted by trusted, hand-written GDScript on the engine + side. This has *zero* attack surface by construction — there's no + interpreter to escape because there's no general execution, just a + fixed, reviewed set of verbs. This is almost certainly sufficient for + the actual UGC use cases in this game and should be scoped against real + feature requirements before assuming more is needed. + +2. **If genuinely general scripting becomes a hard requirement**, isolate it + at the OS level, not the engine level: run untrusted scripts in a + **separate process** (a stripped-down Godot instance or a dedicated + interpreter) under OS sandboxing (a restricted user account, a + container, or `firejail`/`bwrap`/seccomp), communicating with the main + game over a narrow IPC/RPC boundary that only exposes intentionally-safe + operations. This is the only approach that provides a real security + boundary for arbitrary code, because it doesn't depend on the scripting + language cooperating — the OS enforces it. Substantially more + infrastructure than option 1; only worth it if option 1's DSL genuinely + can't express what's needed. + +**Do not** attempt in-process GDScript sandboxing via SubViewport, +ScriptServer, or ad hoc blocklisting — all three were directly tested and +none provide a real boundary (§2). + +## 4. Open Questions + +- What do actual planned UGC scripts need to *do*? (Trigger conditions, + simple item/state effects, dialogue branching?) This determines whether + the DSL in §3.1 is sufficient or whether §3.2's process isolation is + needed. Needs a concrete feature list before implementation starts. +- If §3.2 is needed later, does it run per-user-session or shared/pooled? + Affects resource cost and isolation guarantees. +- Interaction with the content moderation pipeline (`content_moderation.md`): + scripts as a moderated content type still need a moderation flag path + regardless of which execution model is chosen — a DSL makes static + analysis of *what a script can possibly do* far easier than it would be + for general code, which is an additional point in its favor. diff --git a/project.godot b/project.godot new file mode 100644 index 0000000..8dddf44 --- /dev/null +++ b/project.godot @@ -0,0 +1,24 @@ +; Engine configuration file. +; It's best edited using the editor UI and not directly, +; but it can also be edited by hand. + +config_version=5 + +[application] + +config/name="AGP-Game" +run/main_scene="res://scenes/main_menu.tscn" +config/features=PackedStringArray("4.6.2") + +[autoload] + +[display] + +window/size/viewport_width=1920 +window/size/viewport_height=1080 +window/stretch/mode="canvas_items" + +[rendering] + +renderer/rendering_method="gl_compatibility" +renderer/rendering_method.mobile="gl_compatibility" diff --git a/sandbox_probe/.godot/.gdignore b/sandbox_probe/.godot/.gdignore new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/sandbox_probe/.godot/.gdignore @@ -0,0 +1 @@ + diff --git a/sandbox_probe/probe6.gd b/sandbox_probe/probe6.gd new file mode 100644 index 0000000..df96840 --- /dev/null +++ b/sandbox_probe/probe6.gd @@ -0,0 +1,174 @@ +extends Node + +func _ready(): + print("=== SANDBOX PROBE 6: Corrected Evidence Gathering (Godot 4.6.2) ===") + probe_script_loading() + probe_os_apis() + probe_file_access() + probe_network() + probe_classdb() + probe_expression_engine() + probe_engine_singletons() + probe_worker_threads() + probe_project_settings() + probe_allowlist() + print("=== SANDBOX PROBE 6 END ===") + get_tree().quit() + +func probe_script_loading(): + print("--- Script loading ---") + var gd = GDScript.new() + gd.source_code = "extends RefCounted\nfunc hello():\n\treturn 42\n" + var err = gd.reload() + print("GDScript.reload() from source_code err=", err) + if err == OK: + var inst = gd.new() + print(" instantiated, hello()=", inst.call("hello")) + var res = load("res://test_script.gd") + print("load('res://test_script.gd') = ", res) + if res: + print(" get_source_code() available: ", res.has_method("get_source_code")) + print(" GDScript instance has 'get_ast': ", gd.has_method("get_ast")) + print(" GDScript instance has 'get_parser': ", gd.has_method("get_parser")) + print(" GDScript instance has 'get_script_method_list': ", gd.has_method("get_script_method_list")) + print(" GDScript instance has 'get_script_property_list': ", gd.has_method("get_script_property_list")) + +func probe_os_apis(): + print("--- OS APIs ---") + var out = [] + var exit_code = OS.execute("echo", ["hello"], out) + print("OS.execute('echo') exit=", exit_code, " out=", out) + print("OS.get_environment('HOME') = ", OS.get_environment("HOME")) + print("OS has 'create_process': ", OS.has_method("create_process")) + print("OS.get_cmdline_args() = ", OS.get_cmdline_args()) + print("OS.get_user_data_dir() = ", OS.get_user_data_dir()) + print("OS.get_executable_path() = ", OS.get_executable_path()) + +func probe_file_access(): + print("--- File access ---") + var f = FileAccess.open("/etc/hostname", FileAccess.READ) + print("FileAccess.open('/etc/hostname') = ", f) + if f: + print(" content: ", f.get_as_text()) + f.close() + var w = FileAccess.open("/tmp/godot_probe_write.txt", FileAccess.WRITE) + print("FileAccess.open('/tmp/...', WRITE) = ", w) + if w: + w.store_string("probe") + w.close() + print(" wrote to /tmp OK") + var dir = DirAccess.open("/") + print("DirAccess.open('/') = ", dir) + if dir: + var names = dir.get_files() + print(" root files count: ", names.size()) + var res_dir = DirAccess.open("res://") + print("DirAccess.open('res://') = ", res_dir) + var user_dir = DirAccess.open("user://") + print("DirAccess.open('user://') = ", user_dir) + +func probe_network(): + print("--- Network ---") + var tcp = TCPServer.new() + var bind_err = tcp.listen(0) + print("TCPServer.listen(0) err=", bind_err) + var http = HTTPClient.new() + print("HTTPClient instantiated: ", http != null) + var http_req = HTTPRequest.new() + print("HTTPRequest instantiated: ", http_req != null) + var udp = PacketPeerUDP.new() + print("PacketPeerUDP instantiated: ", udp != null) + var ws = WebSocketPeer.new() + print("WebSocketPeer instantiated: ", ws != null) + +func probe_classdb(): + print("--- ClassDB ---") + print("ClassDB.class_exists('OS'): ", ClassDB.class_exists("OS")) + print("ClassDB.class_exists('FileAccess'): ", ClassDB.class_exists("FileAccess")) + print("ClassDB.class_exists('TCPServer'): ", ClassDB.class_exists("TCPServer")) + print("ClassDB.class_exists('HTTPClient'): ", ClassDB.class_exists("HTTPClient")) + var class_list = ClassDB.get_class_list() + print("ClassDB.get_class_list() count: ", class_list.size()) + var os_methods = ClassDB.class_get_method_list("OS") + print("OS method count: ", os_methods.size()) + var inst = ClassDB.instantiate("Node") + print("ClassDB.instantiate('Node') = ", inst) + print("ClassDB has 'class_get_singleton': ", ClassDB.has_method("class_get_singleton")) + +func probe_expression_engine(): + print("--- Expression engine ---") + var expr = Expression.new() + var err = expr.parse("1 + 2") + print("Expression.parse('1 + 2') err=", err) + if err == OK: + var result = expr.execute() + print(" Expression.execute() = ", result) + var expr2 = Expression.new() + err = expr2.parse("OS.get_environment('HOME')") + print("Expression.parse('OS.get_environment') err=", err) + if err == OK: + var result2 = expr2.execute() + print(" Expression.execute() = ", result2) + var expr3 = Expression.new() + err = expr3.parse("get_environment('HOME')", ["OS"]) + print("Expression.parse with base_instance err=", err) + if err == OK: + var result3 = expr3.execute([], OS) + print(" Expression.execute with OS base = ", result3) + var expr4 = Expression.new() + err = expr4.parse("FileAccess.open('/etc/hostname', 1)") + print("Expression.parse('FileAccess.open') err=", err) + if err == OK: + var result4 = expr4.execute() + print(" Expression.execute() = ", result4) + var expr5 = Expression.new() + err = expr5.parse("get_tree().quit()") + print("Expression.parse('get_tree().quit()') err=", err) + if err == OK: + var result5 = expr5.execute() + print(" Expression.execute() = ", result5) + +func probe_engine_singletons(): + print("--- Engine singletons ---") + print("Engine has 'remove_singleton': ", Engine.has_method("remove_singleton")) + print("Engine has 'add_singleton': ", Engine.has_method("add_singleton")) + print("Engine.get_main_loop() = ", Engine.get_main_loop()) + +func probe_worker_threads(): + print("--- Worker threads ---") + var thread = Thread.new() + print(" Thread.new() = ", thread) + print(" WorkerThreadPool has 'add_task': ", WorkerThreadPool.has_method("add_task")) + +func probe_project_settings(): + print("--- Project settings ---") + var settings = ProjectSettings.get_property_list() + var relevant = [] + for s in settings: + var name = String(s.name) + if "sandbox" in name.to_lower() or "restrict" in name.to_lower() or "security" in name.to_lower(): + relevant.append(name) + print(" Sandbox/security project settings: ", relevant) + print(" ProjectSettings has 'application/run/disable_os': ", ProjectSettings.has_setting("application/run/disable_os")) + print(" ProjectSettings has 'application/run/disable_file_access': ", ProjectSettings.has_setting("application/run/disable_file_access")) + print(" ProjectSettings has 'application/run/disable_network': ", ProjectSettings.has_setting("application/run/disable_network")) + print(" ProjectSettings has 'set_setting': ", ProjectSettings.has_method("set_setting")) + print(" ProjectSettings has 'save': ", ProjectSettings.has_method("save")) + +func probe_allowlist(): + print("--- Allowlist / static analysis ---") + var dangerous_patterns = [ + "OS.execute", "OS.create_process", "OS.get_environment", + "FileAccess.open", "DirAccess.open", "TCPServer", "HTTPClient", + "load(", "preload(", "ClassDB", "Engine.get_main_loop", + "get_tree().quit", "ProjectSettings" + ] + var test_script = """extends Node +func _ready(): + var f = FileAccess.open("/etc/passwd", FileAccess.READ) + OS.execute("rm", ["-rf", "/"]) +""" + print(" Test script contains dangerous patterns:") + for p in dangerous_patterns: + if p in test_script: + print(" DETECTED: ", p) diff --git a/sandbox_probe/probe6.tscn b/sandbox_probe/probe6.tscn new file mode 100644 index 0000000..27f675b --- /dev/null +++ b/sandbox_probe/probe6.tscn @@ -0,0 +1,6 @@ +[gd_scene load_steps=2 format=3] + +[ext_resource type="Script" path="res://probe6.gd" id="1"] + +[node name="Probe6" type="Node"] +script = ExtResource("1") diff --git a/sandbox_probe/project.godot b/sandbox_probe/project.godot new file mode 100644 index 0000000..79c5ba9 --- /dev/null +++ b/sandbox_probe/project.godot @@ -0,0 +1,8 @@ +; Engine configuration file. +config_version=5 + +[application] + +config/name="sandbox_probe" +run/main_scene="res://probe6.tscn" +config/features=PackedStringArray("4.6") diff --git a/sandbox_probe/test_script.gd b/sandbox_probe/test_script.gd new file mode 100644 index 0000000..47cd98a --- /dev/null +++ b/sandbox_probe/test_script.gd @@ -0,0 +1,4 @@ +extends RefCounted + +func hello(): + return "from test_script.gd"