Bind local Plannotator servers to loopback by default

[Aeg1sx]

Summary

• bind local Bun plan, review, and annotate servers to 127.0.0.1 instead of listening on all interfaces
• keep remote Bun and Pi sessions on 0.0.0.0 so Docker/SSH/devcontainer forwarding still works
• add Bun/Pi regression tests for hostname selection in local vs remote sessions

Why this patch

The local Plannotator servers expose review and annotation APIs, including the review agent job launcher. Local sessions should not listen on all interfaces by default.

This patch narrows local sessions to loopback while preserving the existing remote-session behavior needed for Docker bridge networking and remote forwarding workflows.

Testing

• /tmp/bun/bin/bun test packages/server/remote.test.ts apps/pi-extension/server/network.test.ts apps/pi-extension/server.test.ts

[backnotprop]

Running review workflows

[backnotprop]

Thanks for this PR. Two pieces of feedback:

1. The loopback binding: keep this. Changing all servers to bind to 127.0.0.1 instead of 0.0.0.0 is the right fix. Remote sessions were exposing the process-launching endpoint to the entire network. This closes that. Clean and effective.

2. The session token: I don't think this adds real protection. The token is returned by GET /api/diff, which has no authentication. So any process on the machine can call /api/diff, read the token, and then use it to launch jobs. It adds an extra request but not an actual barrier.

To put it another way: we are protecting the door with a password, but giving out the password at a window right next to the door.

I would suggest either removing the token for now and shipping just the loopback change, or redesigning how the token is delivered so it cannot be fetched from an open endpoint. Either way, the loopback binding is the real security improvement here and I would love to get that merged.

What do you think? Happy to discuss if you see a threat model where the token helps that I am missing.

[Aeg1sx]

I agree loopback is the primary fix. The token was intended as browser-origin CSRF friction, not local-process isolation. Since same-user local processes can still read /api/diff, I’m happy to drop it from this PR and keep the loopback change. If useful, I can follow up separately with Host/Origin validation for browser-side hardening.

[Aeg1sx]

Updated the PR to match that direction.

I removed the session-token gating changes and kept the loopback binding only. The branch is now rebased onto the latest main, and the remaining diff is just the Bun/Pi loopback change plus the hostname regression tests.

Re-ran:

• /tmp/bun/bin/bun test packages/server/remote.test.ts apps/pi-extension/server/network.test.ts apps/pi-extension/server.test.ts

[backnotprop]

Sorry for the extra round. One more thing we missed.

When Plannotator runs inside a Docker container with bridge networking and PLANNOTATOR_REMOTE=1, binding to 127.0.0.1 inside the container makes the published port unreachable from the host. Docker port forwarding connects to the container's network interface, not its loopback.

The old behavior was actually correct for this case. Suggested fix for both packages/server/remote.ts and apps/pi-extension/server/network.ts:

export function getServerHostname(): string {
  return isRemoteSession() ? "0.0.0.0" : LOOPBACK_HOST;
}

Local sessions get the loopback security fix. Remote/Docker sessions keep working on 0.0.0.0. The hostname tests would need updating so the remote case expects "0.0.0.0" instead of "127.0.0.1".

[Aeg1sx]

All right. I didn't expect this. I changed it again! Review please @backnotprop

Thank you