[quickfix] Add automated tests proving sandbox blacklist actually blocks each FORBIDDEN token #6

New issue

Closed

opened 2026-09-21 22:20:39 +00:00 by Mark · 1 comment

Mark commented

2026-09-21 22:20:39 +00:00

Owner

scripts/autoload/script_engine.gd declares a 24-token FORBIDDEN blacklist (OS., FileAccess, DirAccess, ClassDB, Engine., ProjectSettings, Input., ResourceLoader, ResourceSaver, load(, preload(, get_tree(, get_node(, root, HTTPRequest, TCPServer, PacketPeer, Expression., Script.new, GDScript.new, thread, Thread, set_script). The README discloses it as "blocks obvious escape hatches" but there is no automated proof that the rejection path actually fires for each token — GDScript drift would be silent.

What to do

Add tests/test_sandbox.gd (or tests/test_sandbox_blacklist.tscn driving a GDScript test rig) that:

For each of the 24 FORBIDDEN tokens, builds a tiny script source string that uses it (var x = OS.get_unix_time() etc.) and asserts ScriptEngine.compile(src) returns an error or a non-attached result.
Asserts a benign source (extends ScriptEngine.ScriptEntityAPI\nfunc me(): return null) compiles and attaches successfully.
Runs as part of CI's existing tests/e2e.tscn invocation (extend the gate or add a new tests/sandbox.tscn that the existing one runs first).

Place at tests/sandbox.tscn + tests/sandbox.gd to match the existing test layout.

Acceptance

bash .gsd/gates/verify.sh exits 0 AND removing any single token from the FORBIDDEN list causes the corresponding test case to fail (proving coverage of every entry). Sandbox baseline snapshot included in the test file's doc comment.

Labels

test, quickfix

`scripts/autoload/script_engine.gd` declares a 24-token FORBIDDEN blacklist (OS., FileAccess, DirAccess, ClassDB, Engine., ProjectSettings, Input., ResourceLoader, ResourceSaver, load(, preload(, get_tree(, get_node(, root, HTTPRequest, TCPServer, PacketPeer, Expression., Script.new, GDScript.new, thread, Thread, set_script). The README discloses it as "blocks obvious escape hatches" but there is no automated proof that the rejection path actually fires for each token — GDScript drift would be silent. ## What to do Add `tests/test_sandbox.gd` (or `tests/test_sandbox_blacklist.tscn` driving a GDScript test rig) that: 1. For each of the 24 FORBIDDEN tokens, builds a tiny script source string that uses it (`var x = OS.get_unix_time()` etc.) and asserts `ScriptEngine.compile(src)` returns an error or a non-attached result. 2. Asserts a benign source (`extends ScriptEngine.ScriptEntityAPI\nfunc me(): return null`) compiles and attaches successfully. 3. Runs as part of CI's existing `tests/e2e.tscn` invocation (extend the gate or add a new `tests/sandbox.tscn` that the existing one runs first). Place at `tests/sandbox.tscn` + `tests/sandbox.gd` to match the existing test layout. ## Acceptance `bash .gsd/gates/verify.sh` exits 0 AND removing any single token from the FORBIDDEN list causes the corresponding test case to fail (proving coverage of every entry). Sandbox baseline snapshot included in the test file's doc comment. ## Labels `test`, `quickfix`

Mark added the

test

quickfix

labels

2026-09-21 22:20:39 +00:00

Mark added the

triage-approved

label

2026-09-22 01:47:33 +00:00

Mark referenced this issue from a commit

2026-09-22 16:33:43 +00:00

test: add sandbox blacklist coverage test for Mark/veilwalkers#6

Mark closed this issue

2026-09-22 16:34:08 +00:00

Mark commented

2026-09-22 16:34:18 +00:00

Author

Owner

Closed via commits on main:

c19de96 — test: add sandbox blacklist coverage test (Mark/veilwalkers#6)
174d76f — style: gdformat sweep on tests/sandbox.gd

What landed:

tests/sandbox.gd + tests/sandbox.tscn drive ScriptEngine.compile() against
one source per FORBIDDEN token (string-literal samples so the source parses
cleanly when the blacklist is bypassed — otherwise removal of a token would
be masked by parse errors instead of failing the corresponding test).
Rejection check requires the error to start with forbidden token in script:,
proving the blacklist fired (not just any compile failure).
Substring-shadow relationships (load(/preload(, Script.new/GDScript.new,
thread/Thread case-folded) are exposed as documented findings: removing
GDScript.new or Thread from the FORBIDDEN array is operationally a no-op
because the shorter sibling catches the same samples first. The test
asserts "rejection names this token or its shadow" so this stays honest.
.gsd/gates/verify.sh now runs tests/sandbox.tscn alongside e2e.tscn.

Coverage verification (one test removed at a time, sandbox.tscn re-run):

21 / 23 tokens: removal -> 3+ failures in the corresponding test (PASS).
2 tokens (GDScript.new, Thread): 0 failures because their shorter
sibling is still in the list and catches the sample. Documented as a
redundancy finding in the test file's docstring + inline comments.

Closed via commits on main: - c19de96 — test: add sandbox blacklist coverage test (Mark/veilwalkers#6) - 174d76f — style: gdformat sweep on tests/sandbox.gd What landed: - tests/sandbox.gd + tests/sandbox.tscn drive ScriptEngine.compile() against one source per FORBIDDEN token (string-literal samples so the source parses cleanly when the blacklist is bypassed — otherwise removal of a token would be masked by parse errors instead of failing the corresponding test). - Rejection check requires the error to start with `forbidden token in script:`, proving the blacklist fired (not just any compile failure). - Substring-shadow relationships (load(/preload(, Script.new/GDScript.new, thread/Thread case-folded) are exposed as documented findings: removing `GDScript.new` or `Thread` from the FORBIDDEN array is operationally a no-op because the shorter sibling catches the same samples first. The test asserts "rejection names this token or its shadow" so this stays honest. - .gsd/gates/verify.sh now runs tests/sandbox.tscn alongside e2e.tscn. Coverage verification (one test removed at a time, sandbox.tscn re-run): - 21 / 23 tokens: removal -> 3+ failures in the corresponding test (PASS). - 2 tokens (`GDScript.new`, `Thread`): 0 failures because their shorter sibling is still in the list and catches the sample. Documented as a redundancy finding in the test file's docstring + inline comments.