Skip to content

fix(admin): refuse a TCP allocation the server could never hand out - #12

Merged
nickolasdeluca merged 1 commit into
mainfrom
fix/tcp-port-range-validation
Aug 26, 2026
Merged

nickolasdeluca merged 1 commit into
mainfrom
fix/tcp-port-range-validation

Conversation

@nickolasdeluca

Copy link
Copy Markdown
Owner

Found while testing #11 against a local deployment.

The TCP port range lives in the server config and the reservation lives in SQLite, and nothing connected the two. The panel wrote whatever port it was given, and the deployment only disagreed later, when a client registered:

registration_failed - failed to allocate requested port 20052: requested port 20052 outside range 33000-33020

From the panel's side the allocation looked fine, so the failure reads as the allocation having been lost rather than as a port that was never valid in the first place.

The fix

  • admin.Deployment carries TCPPortMin/TCPPortMax, filled from the server config.
  • checkTCPPortRange guards both paths that write a reservation — handleCreateReservation and the Patch in builder.
  • The builder checks before resolving the allocation, so an out-of-range port is refused whether it would be created or adopted from an earlier reservation. A command promising an address the server cannot serve is worse than no command.
  • /api/server reports the range, so both port fields carry it as their min/max and the browser stops an out-of-range value at the control.

Deliberately left alone

  • A deployment that reports no range refuses nothing. Both bounds zero means the server did not say, and refusing an allocation over information it never gave would be worse than the symptom being fixed.
  • Pinning a live session is not checked. Its port came from the allocator, so it cannot be out of range.
  • drip admin reservation create still knows no range. It writes straight to the database — this is how the out-of-range rows in my test deployment got there. Closing that would mean teaching the CLI to read the server config, which is a separate decision. Recorded in AGENTS.md.

Testing

6 new tests: refusal and acceptance on both paths, the no-range escape hatch, refusing to adopt an out-of-range allocation, and /api/server reporting the range. go vet and go test -race ./... clean.

Checked in the panel too: the port placeholder becomes the server's actual minimum, and submitting a port outside the range is stopped at the field before it reaches the API — which refuses it as well.

The port range lives in the server config and the reservation lives in
SQLite, and nothing connected the two: the panel wrote any port it was
given, and the deployment only disagreed later, when a client registered
and got "requested port 20050 outside range 33000-33020". From the
panel's side the allocation looked fine, so the failure read as the
allocation having been lost rather than as a port that was never valid.

admin.Deployment now carries tcp_port_min/max, and checkTCPPortRange
guards both paths that write a reservation. The builder checks before
resolving the allocation, so an out-of-range port is refused whether it
would be created or adopted from an earlier one — a command promising an
address the server cannot serve is worse than no command.

A deployment that reports no range is left alone. Refusing an allocation
over information the server never gave would be worse than the symptom
being fixed, and a pinned session cannot be out of range anyway: its
port came from the allocator.

/api/server reports the range, so both port fields carry it as their
bounds and the browser stops an out-of-range value at the control.
`drip admin reservation create` still writes straight to the database
and knows no range.
@nickolasdeluca
nickolasdeluca merged commit 6bdc68e into main Aug 26, 2026
2 checks passed
@nickolasdeluca
nickolasdeluca deleted the fix/tcp-port-range-validation branch August 26, 2026 17:47
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.

1 participant