Add remote shell payload for remote REPL.

[mpereiraesaa]

Summary by CodeRabbit

• New Features
  • Interactive remote Python REPL over TCP with session controls, inspection commands, captured stdout/stderr, and clear result delimiters.
  • Companion interactive client supporting greetings/timeouts, .load addon uploads, and robust command/response handling.
  • Simple transfer protocol and receiver for streaming dumps (binary/text/kv/json) with local save helpers.
  • Utilities to stream memory and dictionary dumps from the remote and a small test addon to exercise dumping.

[coderabbitai]

Walkthrough

Adds a socket-backed Python REPL server and interactive client, plus a simple transfer/XFER protocol and dump helpers: server supports special commands, addon loading, stdout/stderr capture, and END_MARKER-delimited responses; client supports interactive REPL, local .load addon transfer; transfers and dumps modules handle XFER send/receive and mem/dict dumps.

Changes

Cohort / File(s)	Summary
Remote shell server payloads/remote_shell.py	New module: per-connection Python 2.7 REPL over a socket. Adds PROMPT, END_MARKER, WRITING; handle_special_commands(cmd, ctx), handle_command(cmd, ctx) (eval/exec with stdout/stderr capture), handle_addon(client_sock, cmd) to receive/execute addon until __LOAD_END__; outer loop builds commands byte-by-byte, sends greeting, handles .load, returns results + END_MARKER, and handles SystemExit for graceful close.
Interactive remote client remote_client.py	New interactive TCP client: END_MARKER, recv_until(sock, marker), send_addon(sock, name) (reads ./payloads/shell/<name>.py and transmits with load protocol), main(host, port) connects, optionally reads greeting, interactive loop supporting local .load interception to push addons, send commands, receive responses until END_MARKER, and exit handling.
Transfer/XFER receiver & helpers transfers.py	New module implementing XFER receiver: recv_xfer(sock) parses __XFER_BEGIN__ / __XFER_END__, validates size, returns (TYPE, NAME, raw); save helpers save_bin, save_text, save_kv, save_json, save_raw; handle_xfer(x_type, name, raw) dispatches to savers. Includes basic header/size error handling and progress feedback.
Dump helpers for shell payloads/shell/dumps.py	New module: XFER sender helpers using global client_sock and sc.syscalls.write: xfer_begin(x_type, name, size=-1), xfer_data(data), xfer_end(), memdump(addr, length) (reads via readbuf), dump_dict(d) (streams KV lines). No explicit error handling.
Shell test addon payloads/shell/test.py	New test addon: foo() builds a dict with test and name (from renpy.config.name) and calls dump_dict to send it via XFER.

Sequence Diagram

sequenceDiagram
    participant C as Client (remote_client)
    participant S as RemoteShell (payloads/remote_shell)
    participant T as Transfers (transfers.py)

    C->>S: TCP connect
    S-->>C: Greeting (optional)
    loop REPL
        S-->>C: PROMPT ("ps> ")
        C->>C: Read user input
        alt local .load (client)
            C->>S: ".load <name>"
            S-->>C: "__LOAD_REQUEST__ <name>"
            C->>C: send_addon reads ./payloads/shell/<name>.py
            C->>S: transmit addon lines... + "__LOAD_END__"
            S->>S: handle_addon reads until "__LOAD_END__" and execs in ctx
            S-->>C: addon load success/failure + END_MARKER
        else regular command
            C->>S: Send command
            alt special server command
                S->>S: handle_special_commands(cmd, ctx)
            else Python code
                S->>S: handle_command(cmd, ctx) — redirect stdout/stderr, eval/exec, capture output
            end
            S-->>C: Command output + END_MARKER
        end
    end

    opt XFER initiated from server
        S->>C: "__XFER_BEGIN__ TYPE NAME SIZE" + "__XFER_START__"
        S->>C: stream payload chunks (xfer_data)
        S->>C: "__XFER_END__"
        C->>T: recv_xfer(sock) parses & returns (TYPE,NAME,raw)
        T->>C: handle_xfer(...) saves file(s)
    end

    C->>S: Exit / close
    S->>S: Handle SystemExit, cleanup socket

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Inspect sandboxing and persistent ctx in payloads/remote_shell.py (security, surprising globals).
• Verify correct redirection/restoration of stdout/stderr and exception handling around eval/exec.
• Review addon protocol (__LOAD_REQUEST__ / __LOAD_END__), path handling in remote_client.py::send_addon, and encoding/UTF-8 handling in byte-wise read loop.
• Validate XFER framing/parsing logic in transfers.py and sender usage in payloads/shell/dumps.py (size headers, streaming, and mismatch handling).
• Check payloads/shell/test.py usage of dump_dict and any implicit global dependencies (e.g., renpy, sc).

Poem

🐇 I hopped to the socket, ps> aglow,
I sent a little addon, watched it grow,
I streamed some bytes and dumped a map,
Markers wrapped results — a neat lil' wrap,
Carrot-coded REPL, hop, execute, and go.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 26.32% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding a remote shell payload that implements a remote REPL (Read-Eval-Print Loop) over a socket.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc04aed and 0ce8763.

📒 Files selected for processing (2)

• payloads/remote_shell.py (1 hunks)
• remote_client.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

payloads/remote_shell.py (2)

src/utils/rp.py (1)

• log (45-52)

payloads/updater_for_up_to_2.x.x.py (1)

• sc (16-34)

🪛 Ruff (0.14.4)

payloads/remote_shell.py

113-113: Use of possibly insecure function; consider using ast.literal_eval

(S307)

---

118-118: Use of exec detected

(S102)

---

121-121: Do not catch blind exception: Exception

(BLE001)

---

179-179: Comment contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?

(RUF003)

---

193-193: Do not catch blind exception: Exception

(BLE001)

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

payloads/remote_shell.py (1)

13-20: Consider documenting the purpose of the WRITING flag and external variable injection.

The WRITING flag is used to suppress editor warnings for variables (port, s, sockaddr_in, len_buf) that are injected externally before this payload executes. While functional, this pattern could be clearer with a comment explaining that these variables are expected to be present in the globals before the payload runs.

-WRITING = False
-
-if WRITING:  # solo para que el editor se calle
+# Development flag to suppress editor warnings for externally-injected variables
+# Variables s, sockaddr_in, len_buf are injected into globals before payload execution
+WRITING = False
+
+if WRITING:  # Editor hint: these variables exist at runtime
     port = 0
     s = 0
     sockaddr_in = bytearray()

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0ce8763 and 14ffeb8.

📒 Files selected for processing (1)

• payloads/remote_shell.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

payloads/remote_shell.py (2)

src/utils/rp.py (1)

• log (45-52)

payloads/updater_for_up_to_2.x.x.py (1)

• sc (16-34)

🪛 Ruff (0.14.4)

payloads/remote_shell.py

139-139: Simple statements must be separated by newlines or semicolons

(invalid-syntax)

---

200-200: Comment contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?

(RUF003)

🔇 Additional comments (5)

payloads/remote_shell.py (5)

1-11: LGTM! Imports are correct for Python 2.7 environment.

The imports are appropriate for the Python 2.7 target environment, including the use of StringIO.StringIO and from __future__ import print_function for compatibility.

---

27-38: LGTM! Syscall testing function is well-structured.

The function properly handles both syscall lookup and execution errors, returning clear success/failure indicators.

---

102-158: LGTM! Command execution properly handles Python 2.7 exec statement syntax.

The function correctly uses the Python 2 statement form exec code_obj in ctx, ctx on line 139, which addresses the previous review feedback. The static analysis warning from Ruff is a false positive since Ruff is a Python 3 linter analyzing Python 2.7 code. The I/O redirection and error handling are well-structured.

Note: This resolves the issue raised in the previous review comment about exec usage.

---

183-236: LGTM! REPL loop is well-structured with proper error handling.

The main loop correctly:

• Sends prompts before reading commands
• Handles byte-by-byte reading to detect newlines
• Decodes UTF-8 with error replacement for robustness
• Gracefully handles connection closure and SystemExit
• Sends END_MARKER to delineate responses for the client

---

163-178: No action required—the variable injection mechanism is properly implemented and reliable.

The variables s, sockaddr_in, and len_buf are correctly injected into the payload's execution context. In src/main.py (lines 175–178), the main orchestrator merges its globals and locals into a scope dictionary and passes it to exec(stage2_str, scope), making all initialized variables available to the payload code. The placeholder definitions in payloads/remote_shell.py (lines 15–19) serve an IDE/editor purpose. This is a well-established, reliable pattern in the codebase and works as intended.

[coderabbitai]

Actionable comments posted: 9

♻️ Duplicate comments (2)

payloads/remote_shell.py (2)

176-176: Fix Python 2 exec statement syntax.

The current syntax exec compile(code, "<addon:%s>" % name, "exec") in ctx, ctx is invalid for Python 2. The correct form to specify both globals and locals is exec code_obj in globals_dict, locals_dict, but you need to separate the compile call.

This issue was flagged in previous reviews and by static analysis.

Apply this diff:

     # Execute addon code
     try:
-        exec compile(code, "<addon:%s>" % name, "exec") in ctx, ctx
+        code_obj = compile(code, "<addon:%s>" % name, "exec")
+        exec code_obj in ctx, ctx
         msg = "Addon '%s' loaded successfully.\n" % name

---

80-80: Translate Spanish comment to English.

Per previous feedback, comments should be in English for consistency.

-    # .vars -> mostrar keys de ctx
+    # .vars -> show context keys

Based on learnings

🧹 Nitpick comments (1)

payloads/remote_shell.py (1)

11-17: Remove unused WRITING flag and related variables.

The WRITING flag is hardcoded to False, making the conditional block at lines 13-17 dead code. If these variables (port, s, sockaddr_in, len_buf) are needed elsewhere, initialize them unconditionally; otherwise, remove them.

-WRITING = False
-
-if WRITING:
-    port = 0
-    s = 0
-    sockaddr_in = bytearray()
-    len_buf = bytearray()

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 14ffeb8 and 64d5a6d.

📒 Files selected for processing (2)

• payloads/remote_shell.py (1 hunks)
• remote_client.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

payloads/remote_shell.py (2)

src/utils/rp.py (1)

• log (45-52)

payloads/updater_for_up_to_2.x.x.py (1)

• sc (16-34)

🪛 Ruff (0.14.4)

payloads/remote_shell.py

176-176: Simple statements must be separated by newlines or semicolons

(invalid-syntax)

🔇 Additional comments (3)

remote_client.py (1)

109-114: LGTM!

The argument parsing and entry point are clean and correctly validate the expected number of arguments.

payloads/remote_shell.py (2)

26-47: LGTM!

The kill_game function correctly retrieves the process ID, validates it, and terminates the process with proper error handling.

---

203-205: Translate Spanish comment to English.

Per previous feedback, all comments should be in English.

-    # -----------------------------------------------------
-    #  Inner REPL loop
-    # -----------------------------------------------------
+    # -----------------------------------------------------
+    #  Inner REPL loop
+    # -----------------------------------------------------

Wait, this comment is already in English. Let me check the Spanish comments.

Actually, looking at the code, I see Spanish comments at lines 80, 86, 96, 106, 118. I've already flagged those. Let me check if there are more in this section.

Line 229: "# Now decode" - English
Line 237: "# Load addon" - English
Line 246: "Decoded command" - English
Line 248: "# Execute command" - English
Line 257: "# Send results" - English

All comments in this section are in English. Marking as skip.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Translate Spanish comment to English.

Per previous feedback, comments should be in English for consistency.

-    # Ningún comando especial
+    # No special command matched

Based on learnings

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	# Ningún comando especial
	# No special command matched

🤖 Prompt for AI Agents

In payloads/remote_shell.py around line 106, the inline comment "# Ningún
comando especial" is in Spanish; replace it with an English equivalent such as
"# No special command" (or "# No special commands" if plural) so all code
comments remain consistent in English.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Translate Spanish comment to English.

Per previous feedback, comments should be in English for consistency.

-    # Primero intentamos comandos especiales tipo .vars, etc.
+    # First try special commands like .vars, etc.

Based on learnings

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	# Primero intentamos comandos especiales tipo .vars, etc.
	# First try special commands like .vars, etc.

🤖 Prompt for AI Agents

In payloads/remote_shell.py around line 118, the inline comment is written in
Spanish ("# Primero intentamos comandos especiales tipo .vars, etc."); replace
it with an English equivalent such as "# First try special commands like .vars,
etc." so all source comments remain consistent in English, keeping punctuation
and intent the same.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Add fallback to exec mode for statement commands.

The function only compiles input as "eval", which fails for statements like assignments (x = 1) or imports (import os). The AI summary mentions "compiles input as eval with fallback to exec", but that fallback is missing. Statement commands will fail with a traceback instead of executing.

Apply this diff to add the exec fallback:

     try:
-        code_obj = compile(cmd, "<remote>", "eval")
-        log("Compiled as eval.")
-        result = eval(code_obj, ctx, ctx)
-        if result is not None:
-            print(repr(result))
+        try:
+            code_obj = compile(cmd, "<remote>", "eval")
+            log("Compiled as eval.")
+            result = eval(code_obj, ctx, ctx)
+            if result is not None:
+                print(repr(result))
+        except SyntaxError:
+            # Try as exec for statements
+            log("Eval failed, trying exec.")
+            code_obj = compile(cmd, "<remote>", "exec")
+            exec code_obj in ctx, ctx
     except SystemExit:
         raise
     except Exception:

🤖 Prompt for AI Agents

In payloads/remote_shell.py around lines 131 to 142, the code only compiles
input using "eval" so statement inputs (assignments, imports, etc.) raise
SyntaxError and fail; modify the try block to attempt compile(cmd, "<remote>",
"eval") and on a SyntaxError fall back to compile(cmd, "<remote>", "exec") and
run exec on that compiled object, preserving the existing logging and context
(ctx) and printing repr(result) only for eval results; keep SystemExit re-raised
and keep the existing exception handling and traceback printing for both eval
and exec paths.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Only break inner loop on read failure, not entire server.

When the client disconnects (read returns ≤ 0), the code raises SystemExit, which terminates both the inner and outer loops, shutting down the entire REPL server. Instead, close the client socket and break to the outer loop so the server can accept new connections.

Apply this diff:

             if n <= 0:
                 log("read <= 0, closing.")
                 sc.syscalls.close(client_sock)
-                raise SystemExit
+                break

Note: You'll need to add a check after the inner while loop to distinguish between normal exit and connection loss.

🤖 Prompt for AI Agents

In payloads/remote_shell.py around lines 217 to 220, the current logic closes
the client socket then raises SystemExit when read returns <= 0, which kills the
entire server; change this so the client socket is closed and execution breaks
out of the inner client-handling loop only (so the outer accept loop can
continue). Specifically, replace the SystemExit raise with a break, and add a
check immediately after the inner while loop to detect whether the loop exited
due to a connection loss (n <= 0) versus a normal client-initiated REPL exit so
you can continue to the next accept iteration without terminating the process.
Ensure the client socket is always closed in the connection-loss path.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Validate addon name and clarify working directory requirement.

The hardcoded path "./payloads/shell" assumes the client runs from the repository root. If invoked from elsewhere, addon loading will fail. Additionally, the addon name is not validated, which could allow path traversal attacks if an untrusted source controls the name.

Consider these improvements:

 def send_addon(sock, name):
     """
     Sends the contents of a local addon file from ./shell/<name>.py
     to the remote shell.
     """
+    # Validate addon name to prevent path traversal
+    if not name.replace("_", "").replace("-", "").isalnum():
+        print("Invalid addon name:", name)
+        return
+    
+    # Use script directory as base instead of cwd
+    script_dir = os.path.dirname(os.path.abspath(__file__))
+    path = os.path.join(script_dir, "payloads/shell", name + ".py")
-    path = os.path.join("./payloads/shell", name + ".py")

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	def send_addon(sock, name):
	"""
	Sends the contents of a local addon file from ./shell/<name>.py
	to the remote shell.
	"""
	path = os.path.join("./payloads/shell", name + ".py")
	if not os.path.exists(path):
	print("Addon not found:", path)
	return
	with open(path, "r") as f:
	contents = f.read()
	header = "__LOAD_BEGIN__:%s\n" % name
	sock.sendall(header.encode("utf-8"))
	sock.sendall(contents.encode("utf-8"))
	sock.sendall("__LOAD_END__".encode("utf-8"))
	# Wait for remote ack
	result = recv_until(sock, END_MARKER)
	sys.stdout.write(result)
	sys.stdout.flush()
	def send_addon(sock, name):
	"""
	Sends the contents of a local addon file from ./shell/<name>.py
	to the remote shell.
	"""
	# Validate addon name to prevent path traversal
	if not name.replace("_", "").replace("-", "").isalnum():
	print("Invalid addon name:", name)
	return
	# Use script directory as base instead of cwd
	script_dir = os.path.dirname(os.path.abspath(__file__))
	path = os.path.join(script_dir, "payloads/shell", name + ".py")
	if not os.path.exists(path):
	print("Addon not found:", path)
	return
	with open(path, "r") as f:
	contents = f.read()
	header = "__LOAD_BEGIN__:%s\n" % name
	sock.sendall(header.encode("utf-8"))
	sock.sendall(contents.encode("utf-8"))
	sock.sendall("__LOAD_END__".encode("utf-8"))
	# Wait for remote ack
	result = recv_until(sock, END_MARKER)
	sys.stdout.write(result)
	sys.stdout.flush()

🤖 Prompt for AI Agents

In remote_client.py around lines 21 to 43, the function send_addon uses a
hardcoded relative path "./payloads/shell" and does not validate the addon name,
which allows failures when run from a different CWD and enables path traversal;
fix by resolving the payloads directory relative to this file (use
os.path.dirname(__file__) to build an absolute base path), sanitize and validate
the name (reject if it contains os.path.sep, '..', or any characters outside a
safe whitelist like [A-Za-z0-9_\\-]), construct the full path with os.path.join,
call os.path.normpath and then verify the normalized path startswith the
absolute payloads base directory before opening, and keep the existing existence
check and error message if validation fails.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Check exit commands before sending to remote.

The client checks for exit commands after sending them to the remote and receiving a response. However, commands like .exit and .quit cause the server to close the connection (per handle_special_commands raising SystemExit), so recv_until on line 98 may fail or return incomplete data before reaching line 103. Move the exit check before sending to avoid this race.

Apply this diff:

+        # Check for exit commands before sending
+        if line.strip() in (".exit", "quit()", "exit()", ".quit"):
+            s.sendall(line.encode("utf-8"))
+            s.close()
+            break
+
         # Normal command, send to remote
         s.sendall(line.encode("utf-8"))
 
         # Read result
         result = recv_until(s, END_MARKER)
         if result:
             sys.stdout.write(result)
             sys.stdout.flush()
-
-        if line.strip() in (".exit", "quit()", "exit()", ".quit"):
-            break

🤖 Prompt for AI Agents

In remote_client.py around lines 98 to 104, the code checks for exit commands
only after sending the line and calling recv_until, which can race with the
server closing the connection; move the check for exit commands (".exit",
"quit()", "exit()", ".quit") to before sending so the client
breaks/returns/cleans up immediately without sending them and without calling
recv_until, ensuring you don't attempt to read from a connection the server has
already closed.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (3)

remote_client.py (3)

9-19: Handle unexpected connection closure.

If the remote closes the connection before sending END_MARKER, the function returns all accumulated data without indicating whether the response is complete or truncated. The caller cannot distinguish between a complete response and an incomplete one (e.g., server crash mid-response).

Consider returning a tuple (data, found_marker) or raising an exception:

 def recv_until(sock, marker):
     data = bytearray()
+    found = False
     while True:
         chunk = sock.recv(4096)
         if not chunk:
             break
         data.extend(chunk)
         if marker in data:
+            found = True
             break
+    if not found:
+        raise ConnectionError("Connection closed before END_MARKER received")
     parts = data.split(marker, 1)
     return parts[0].decode("utf-8", errors="ignore")

---

22-44: Validate addon name and use script-relative paths.

The hardcoded path "./payloads/shell" assumes the client runs from the repository root. The addon name is not validated, which could allow path traversal attacks.

Apply these improvements:

 def send_addon(sock, name):
     """
     Sends the contents of a local addon file from ./shell/<name>.py
     to the remote shell.
     """
+    # Validate addon name to prevent path traversal
+    if not name.replace("_", "").replace("-", "").isalnum():
+        print("Invalid addon name:", name)
+        return
+    
+    # Use script directory as base instead of cwd
+    script_dir = os.path.dirname(os.path.abspath(__file__))
+    path = os.path.join(script_dir, "payloads", "shell", name + ".py")
-    path = os.path.join("./payloads/shell", name + ".py")

---

120-121: Check exit commands before sending to avoid race condition.

The client checks for exit commands after sending them to the remote and receiving a response. However, commands like .exit cause the server to close the connection, so recv_until on line 115 may fail or return incomplete data before reaching line 120.

Move the exit check before sending:

+        # Check for exit commands before sending
+        if line.strip() in (".exit", "quit()", "exit()", ".quit"):
+            s.sendall(line.encode("utf-8"))
+            s.close()
+            break
+
         # Normal command, send to remote
         s.sendall(line.encode("utf-8"))

         # Read result
         result = recv_until(s, END_MARKER)
         if result:
             sys.stdout.write(result)
             sys.stdout.flush()
-
-        if line.strip() in (".exit", "quit()", "exit()", ".quit"):
-            break

🧹 Nitpick comments (6)

payloads/shell/test.py (1)

4-4: Use a more descriptive variable name.

The variable name l is ambiguous and easily confused with the number 1. Use a more descriptive name like data or result_dict.

Apply this diff:

 def foo():
-    l = { "test": True, "name": renpy.config.name }
-    dump_dict(l)
+    data = { "test": True, "name": renpy.config.name }
+    dump_dict(data)

remote_client.py (1)

106-111: Replace bare except with specific exception type.

The bare except clause on line 110 can mask unexpected errors. Specify socket.error or OSError to catch only socket-related failures.

Apply this diff:

-            except:
-                pass
+            except (socket.error, OSError):
+                # END_MARKER already consumed or not present
+                pass

payloads/shell/dumps.py (1)

7-21: Add error handling for socket write failures.

The xfer_* functions use sc.syscalls.write without error handling. If the socket is closed or write fails, the error will propagate uncaught, potentially leaving the transfer in an inconsistent state.

Consider wrapping writes with try-except:

def xfer_begin(x_type, name, size=-1):
    hdr  = "__XFER_BEGIN__\n"
    hdr += "TYPE:%s\n"  % x_type
    hdr += "NAME:%s\n"  % name
    hdr += "SIZE:%d\n"  % size
    hdr += "__XFER_START__\n"
    try:
        sc.syscalls.write(client_sock, hdr, len(hdr))
    except Exception as e:
        # Log or handle write failure
        raise RuntimeError("Failed to send XFER header") from e

transfers.py (3)

28-31: Replace bare except with specific exception type.

The bare except on line 30 is too broad and can mask unexpected errors. Catch UnicodeDecodeError specifically.

Apply this diff:

         try:
             header_buf += chunk.decode("utf-8", "ignore")
-        except:
+        except UnicodeDecodeError:
             return None, None, None

---

50-53: Replace bare except with specific exception type.

The bare except on line 52 should catch ValueError specifically to avoid masking unexpected errors.

Apply this diff:

             try:
                 declared_size = int(line[5:])
-            except:
+            except ValueError:
                 declared_size = -1

---

79-91: Progress bar displays misleading information when receiving excess data.

If more data is received than declared_size, the progress bar clamps the percentage to 100% but still displays the actual received count, which can exceed the declared size (e.g., "100% (5000/4096)"). This is confusing to users.

Consider breaking the loop or suppressing progress updates once received >= declared_size:

         # live progress bar
         if show_progress:
             received = len(raw)
+            # Stop showing progress if we've exceeded declared size
+            if received > declared_size:
+                continue
             if declared_size > 0:
                 pct = int((received * 100) / declared_size)
                 if pct > 100:
                     pct = 100

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 64d5a6d and c272fa4.

📒 Files selected for processing (4)

• payloads/shell/dumps.py (1 hunks)
• payloads/shell/test.py (1 hunks)
• remote_client.py (1 hunks)
• transfers.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

payloads/shell/test.py (1)

payloads/shell/dumps.py (1)

• dump_dict (30-35)

remote_client.py (1)

transfers.py (2)

• recv_xfer (5-105)
• handle_xfer (165-179)

payloads/shell/dumps.py (1)

payloads/updater_for_up_to_2.x.x.py (1)

• sc (16-34)

🪛 Ruff (0.14.4)

payloads/shell/test.py

4-4: Ambiguous variable name: l

(E741)

---

4-4: Undefined name renpy

(F821)

---

5-5: Undefined name dump_dict

(F821)

remote_client.py

110-110: Do not use bare except

(E722)

---

110-111: try-except-pass detected, consider logging the exception

(S110)

payloads/shell/dumps.py

13-13: Undefined name client_sock

(F821)

---

17-17: Undefined name client_sock

(F821)

---

21-21: Undefined name client_sock

(F821)

transfers.py

30-30: Do not use bare except

(E722)

---

52-52: Do not use bare except

(E722)

---

146-146: Use of possibly insecure function; consider using ast.literal_eval

(S307)

---

147-147: Use of possibly insecure function; consider using ast.literal_eval

(S307)

---

149-149: Do not use bare except

(E722)

---

149-150: try-except-pass detected, consider logging the exception

(S110)

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Document that client_sock must be initialized externally.

The global client_sock is declared but never initialized in this module. Calling any of the transfer functions before external code sets client_sock will result in a NameError. Consider adding a module-level docstring or an initialization check in the transfer functions.

Example check:

 def xfer_begin(x_type, name, size=-1):
+    if 'client_sock' not in globals() or client_sock is None:
+        raise RuntimeError("client_sock must be initialized before calling xfer functions")
     hdr  = "__XFER_BEGIN__\n"
     hdr += "TYPE:%s\n"  % x_type
     hdr += "NAME:%s\n"  % name
     hdr += "SIZE:%d\n"  % size
     hdr += "__XFER_START__\n"
     sc.syscalls.write(client_sock, hdr, len(hdr))

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In payloads/shell/dumps.py around line 4, the global client_sock is declared but
not initialized which can cause a NameError if transfer functions are called
before external code sets it; update the module by either assigning client_sock
= None at module scope and adding a short module-level docstring that states
"client_sock must be initialized by caller before use", and add a simple runtime
guard at the start of each transfer function that raises a clear RuntimeError if
client_sock is None (or provide instructions to initialize it), or alternatively
initialize client_sock via an explicit init_client_sock(sock) function and call
that from external code; implement one of these patterns so callers get a clear
error instead of a NameError.

[Helloyunho]

fix the coderabbit reviews and it'll be all good
(dont need to fix all of em, just do as much as you can)