fix(controller): escape backslashes and newlines in Lua string encoding - #416
Open
nicknjpconsultingllc wants to merge 1 commit into
Open
nicknjpconsultingllc wants to merge 1 commit into
nicknjpconsultingllc wants to merge 1 commit into
Conversation
slpp.encode escapes double quotes but not backslashes or newlines, so
any string tool argument containing a backslash, \n or \r produces
invalid Lua and the RCON call fails ("invalid escape sequence",
"unfinished string"). A trailing odd backslash also closes the literal
early, so the remainder of the argument is parsed as Lua.
Fix: pre-escape at every string leaf before slpp sees it; slpp's own
quote escaping composes correctly on top. 26 tests run the emitted Lua
through lupa (already a dependency) in the exact
pcall(storage.actions.NAME, ...) shape the controller builds. Tests are
server-free (tests/unit).
Scope: correctness plus defense in depth, not a security boundary. The
agent's program already has `instance` in scope and can reach
rcon_client directly.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RNpy6PN7ttQrMRmNABEXK8
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
slpp.encode(the encoder every RCON tool call goes through inController._get_command/_execute_once/execute2) escapes double quotes but not backslashes or newlines. Any string tool argument containing\,\nor\rtherefore produces invalid Lua and the call fails:C:\path→invalid escape sequenceunfinished stringFix:
_lua_encode_safepre-escapes\,\n,\rat every string leaf (recursing into dict/list/tuple, since slpp reuses the same string branch for nested values) beforelua.encodesees it. slpp's own quote escaping composes correctly on top. No call site can currently rely on a backslash surviving, because none do.Tests:
tests/unit/test_lua_encoding_security.py, 26 cases, server-free undertests/unit/conftest.py. They run the emitted Lua throughlupa(already a dependency) in the exactpcall(storage.actions.NAME, ...)shape the controller builds, and check that string content can never escape its literal.Scope: correctness plus defense in depth, not a security boundary. The agent's program runs in-process with
instancein scope and can reachrcon_clientdirectly, so this closes no privilege gap today; it matters if a future frontend passes tool arguments without exposing Python.Not fixed here:
slpp.decode()on the response path has its own escaping limits, and slpp does no escaping of dict keys. Neither is reachable from agent input today.🤖 Generated with Claude Code
https://claude.ai/code/session_01RNpy6PN7ttQrMRmNABEXK8