Skip to content

Control: close the connection on input that isn't a request - #254

Merged
bflatastic merged 1 commit into
storytold:mainfrom
Luskish:fix/control-close-on-malformed-input
Oct 7, 2026
Merged

bflatastic merged 1 commit into
storytold:mainfrom
Luskish:fix/control-close-on-malformed-input

Conversation

@Luskish

@Luskish Luskish commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Same bug as storytold/filmcraft#119. The control server answers a non-JSON line with an error and keeps reading. So while --control is open, a web page's cross-origin POST to 127.0.0.1:<port> gets its JSON body run as a command.

Ports LightCraft's fix (storytold/lightcraft#129):

  • A line that isn't a JSON object with a string method gets one error reply, then the connection closes.
  • Lines over 4 MiB are refused, and at most 16 connections are served at once.
  • docs/control-protocol.md documents the rules.

Also comments the two intentional let _ = writes, per AGENTS.md.

vectorcraft-cli mcp --connect sends valid requests, so it's unaffected.

Testing: new tests in control_server.rs cover the browser POST and other junk input. They failed before the fix and pass now. cargo fmt --check, cargo clippy -p vectorcraft --all-targets -- -D warnings, cargo test -p vectorcraft, cargo xtask layers and cargo xtask assets all pass.

A line that is not a JSON object with a string `method` (an HTTP request
line, other non-JSON text, a JSON array, invalid UTF-8, a line over 4 MiB)
now gets one error reply and the connection is closed, so a browser's
cross-origin fetch can't smuggle a command in its body. Connections are
capped at 16. Same transport as LightCraft (storytold/lightcraft#129) and
FilmCraft (storytold/filmcraft#123).
@bflatastic

Copy link
Copy Markdown
Member

Thank you, merged! This closes a real hole. While --control was open, a web page's cross-origin POST to 127.0.0.1:<port> could get its JSON body run as a command.

Before merging I reviewed every line. I ran the full cargo xtask ci (all 7 steps, 3,415 tests) on it stacked with two other PRs. I also checked it against a live app: vectorcraft-cli mcp --connect still works through the server, and a browser-style POST / HTTP/1.1 with a file.new body now gets the error line and a closed connection, with nothing run.

Thank you for the contribution! If you haven't already, come join us on Discord: https://discord.gg/artcraft

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants