Skip to content

feat(relay): zero-config remote access via tunnel#830

Closed
lucasygu wants to merge 1 commit intopaperclipai:masterfrom
lucasygu:feat/relay-tunnel-v3
Closed

feat(relay): zero-config remote access via tunnel#830
lucasygu wants to merge 1 commit intopaperclipai:masterfrom
lucasygu:feat/relay-tunnel-v3

Conversation

@lucasygu
Copy link

Summary

  • Adds relay tunnel client that connects Paperclip to a relay server for zero-config remote access via subdomain routing (<id>.relay-domain.com)
  • Auto-registers with the relay on first run when PAPERCLIP_RELAY_URL is set, persisting token to .env and adding the relay hostname to allowedHostnames
  • Includes HTTP forwarding, WebSocket bridging with reconnect backoff, and graceful shutdown on SIGINT/SIGTERM

Details

Server-side changes:

  • server/src/relay/ — new module with relay-client.ts, http-forwarder.ts, ws-bridge.ts, auto-register.ts, protocol.ts
  • server/src/index.ts — deployment mode guard, auto-registration, relay client startup, unconditional graceful shutdown
  • WebSocket close code validated via try/catch (RFC 6455 compliance)
  • Token sent via Authorization: Bearer header (not URL query string)
  • Relay URL trailing-slash normalized before /tunnel connection
  • WS bridge queues messages arriving before local handshake completes (no silent drops)
  • Registration response validated before new URL() parsing

Documentation:

  • doc/RELAY.md — full relay server architecture and Cloudflare Worker source
  • docs/deploy/relay-tunnel.md — user-facing Mintlify guide with setup steps, comparison table, troubleshooting
  • Updated docs/deploy/deployment-modes.md, overview.md, tailscale-private-access.md, environment-variables.md, local-development.md
  • Updated doc/DEVELOPING.md, doc/CLI.md

Closes #742

Test plan

  • End-to-end tested with deployed relay at paperclip-relay.com
  • Verified auto-registration flow persists token and hostname
  • Verified WebSocket bridge (real-time updates through tunnel)
  • Verified graceful shutdown closes tunnel on SIGINT
  • Verified deployment mode guard rejects relay in non-authenticated mode
  • Build passes with no type errors

🤖 Generated with Claude Code

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 13, 2026

Greptile Summary

This PR introduces a zero-config relay tunnel feature that gives any Paperclip instance a public HTTPS subdomain URL via an outbound WebSocket connection to a Cloudflare Workers-based relay server. The implementation is well-structured — the relay module is cleanly separated into protocol.ts, relay-client.ts, http-forwarder.ts, ws-bridge.ts, and auto-register.ts — and the security guard (requiring authenticated mode) is enforced at startup before any config mutation. Documentation is thorough and accurate.

Key issues found:

  • Binary WebSocket frame corruption (ws-bridge.ts): data.toString() silently corrupts binary frames from local WS endpoints rather than detecting and dropping/erroring on them. If any Paperclip WS endpoint emits binary frames, callers receive malformed data with no error signal.
  • Misleading public URL fallback (relay-client.ts): When a tunnel-ready message omits publicUrl (typed as optional), the fallback resolves to the relay's root URL rather than the instance subdomain, which would be displayed to the operator and result in a 404.
  • Missing HTTP request timeout (http-forwarder.ts): http.request() has no socket timeout; stalled local responses leak file descriptors since the relay DO times out at 30 s but the Node.js socket stays open.
  • Extra blank line in .env (auto-register.ts): Splitting an existing file that ends with \n produces a trailing empty string that is not filtered, inserting an unwanted blank line before the new token entry.
  • Unused path import (auto-register.ts): import path from "node:path" is never referenced.

Confidence Score: 3/5

  • Safe to merge for the relay feature itself, but the binary-frame silent corruption and misleading fallback URL are worth fixing before wide adoption.
  • The deployment-mode security guard, token persistence, reconnect backoff, and graceful shutdown logic are all correct. The two logic-level issues (binary frame corruption and fallback URL) are edge cases that won't affect typical JSON-only WebSocket usage but could produce confusing behaviour in other scenarios. The missing HTTP timeout and .env formatting issues are minor quality concerns that don't block correctness today.
  • server/src/relay/ws-bridge.ts (binary frame handling) and server/src/relay/relay-client.ts (public URL fallback) deserve the most attention before this feature is promoted beyond experimental.

Important Files Changed

Filename Overview
server/src/relay/relay-client.ts New relay tunnel client with reconnect backoff; the tunnel-ready fallback URL logic can display the relay root instead of the instance subdomain when publicUrl is absent.
server/src/relay/ws-bridge.ts WebSocket bridge for the relay tunnel; binary frames are silently corrupted via .toString() rather than being detected and dropped/errored.
server/src/relay/auto-register.ts Auto-registration and token persistence; has an unused path import and a minor blank-line accumulation bug when writing to .env.
server/src/relay/http-forwarder.ts Forwards relay HTTP requests to the local server; missing a socket timeout on http.request, which can leak file descriptors when the local server stalls.
server/src/relay/protocol.ts Clean, well-typed union of all tunnel message types; no issues.
server/src/index.ts Startup wiring for relay client: deployment-mode guard, auto-registration, relay startup, and graceful shutdown on SIGINT/SIGTERM — logic looks correct.
server/src/config.ts Minimal change: adds relayUrl and relayToken fields, loaded from env vars; no issues.
doc/RELAY.md Comprehensive relay architecture doc including full Cloudflare Worker source; the handleHttpProxy DO method uses the new Promise(async (resolve) => {...}) anti-pattern which can mask errors thrown after an await.
docs/deploy/relay-tunnel.md New user-facing Mintlify guide; clear, accurate, and well-structured.

Comments Outside Diff (2)

  1. server/src/relay/http-forwarder.ts, line 1141-1199 (link)

    No socket timeout on the local HTTP request

    http.request() is created without a socket timeout. If the local Paperclip server stalls on a request (e.g. a long-running streaming response, a deadlocked handler, or an oversized upload), this Promise will never resolve or reject on the client side. Meanwhile the relay's Durable Object will time out at 30 s and return a 504 to the browser — but the Node.js http.IncomingMessage socket remains open, leaking a file descriptor per stalled request.

    Consider setting a reasonable socket timeout that matches or slightly exceeds the relay-side gateway timeout:

    Also add a proxyReq.on("timeout", () => { proxyReq.destroy(new Error("Local request timed out")); }) handler so the reject(err) path is triggered on timeout.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: server/src/relay/http-forwarder.ts
    Line: 1141-1199
    
    Comment:
    **No socket timeout on the local HTTP request**
    
    `http.request()` is created without a socket timeout. If the local Paperclip server stalls on a request (e.g. a long-running streaming response, a deadlocked handler, or an oversized upload), this `Promise` will never resolve or reject on the client side. Meanwhile the relay's Durable Object will time out at 30 s and return a 504 to the browser — but the Node.js `http.IncomingMessage` socket remains open, leaking a file descriptor per stalled request.
    
    Consider setting a reasonable socket timeout that matches or slightly exceeds the relay-side gateway timeout:
    
    
    
    Also add a `proxyReq.on("timeout", () => { proxyReq.destroy(new Error("Local request timed out")); })` handler so the `reject(err)` path is triggered on timeout.
    
    How can I resolve this? If you propose a fix, please make it concise.
  2. server/src/relay/relay-client.ts, line 1351-1354 (link)

    Misleading public URL printed to the user if relay omits publicUrl

    When a tunnel-ready message lacks publicUrl (which is typed as optional in protocol.ts), the fallback:

    const publicUrl = msg.publicUrl || relayOrigin;

    resolves to the relay's root URL (e.g. https://your-relay-server.com) rather than the instance subdomain. This URL is then displayed in the console and emitted via onReady, so the operator would navigate to the relay root and see a 404 or wrong instance.

    Since the tunnel-ready message is what conveys the actual subdomain, consider logging a warning and skipping the onReady callback rather than falling back to the root URL:

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: server/src/relay/relay-client.ts
    Line: 1351-1354
    
    Comment:
    **Misleading public URL printed to the user if relay omits `publicUrl`**
    
    When a `tunnel-ready` message lacks `publicUrl` (which is typed as `optional` in `protocol.ts`), the fallback:
    
    ```typescript
    const publicUrl = msg.publicUrl || relayOrigin;
    ```
    
    resolves to the relay's **root** URL (e.g. `https://your-relay-server.com`) rather than the instance subdomain. This URL is then displayed in the console and emitted via `onReady`, so the operator would navigate to the relay root and see a 404 or wrong instance.
    
    Since the `tunnel-ready` message is what conveys the actual subdomain, consider logging a warning and skipping the `onReady` callback rather than falling back to the root URL:
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: server/src/relay/auto-register.ts
Line: 8

Comment:
**Unused import**

`path` is imported but never referenced anywhere in the file. TypeScript strict-mode compilation may still pass, but this is dead code.

```suggestion
import fs from "node:fs";
import { resolvePaperclipConfigPath, resolvePaperclipEnvPath } from "../paths.js";
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: server/src/relay/ws-bridge.ts
Line: 54-59

Comment:
**Binary WebSocket frames are silently corrupted, not rejected**

The file header correctly documents that binary frames are unsupported and "will be corrupted," but the code calls `data.toString()` on whatever the `ws` library delivers. For binary frames the library delivers a `Buffer`, and calling `.toString()` decodes it as UTF-8 — which corrupts arbitrary binary payloads. The corruption is invisible to the sender and the relay consumer: both sides will receive malformed data with no error signal.

If Paperclip's local WebSocket endpoints ever emit binary frames (e.g. Uint8Array, protobuf payloads), this would produce hard-to-debug data corruption. At a minimum the handler should detect and drop/error on binary:

```suggestion
  localWs.on("message", (data: Buffer | string) => {
    if (Buffer.isBuffer(data)) {
      // Binary frames are not supported over the relay tunnel (JSON text only).
      // Drop and notify the relay so the client connection can be closed cleanly.
      send({ type: "ws-error", id: msg.id, message: "Binary WebSocket frames are not supported through the relay tunnel" });
      return;
    }
    send({
      type: "ws-message",
      id: msg.id,
      data: data.toString(),
    });
  });
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: server/src/relay/http-forwarder.ts
Line: 1141-1199

Comment:
**No socket timeout on the local HTTP request**

`http.request()` is created without a socket timeout. If the local Paperclip server stalls on a request (e.g. a long-running streaming response, a deadlocked handler, or an oversized upload), this `Promise` will never resolve or reject on the client side. Meanwhile the relay's Durable Object will time out at 30 s and return a 504 to the browser — but the Node.js `http.IncomingMessage` socket remains open, leaking a file descriptor per stalled request.

Consider setting a reasonable socket timeout that matches or slightly exceeds the relay-side gateway timeout:

```suggestion
    const opts: http.RequestOptions = {
      hostname: "127.0.0.1",
      port: localPort,
      path: req.path,
      method: req.method,
      headers: {
        ...req.headers,
        host: `127.0.0.1:${localPort}`,
      },
      timeout: 35_000, // slightly longer than relay's 30 s gateway timeout
    };

    const proxyReq = http.request(opts, (proxyRes) => {
```

Also add a `proxyReq.on("timeout", () => { proxyReq.destroy(new Error("Local request timed out")); })` handler so the `reject(err)` path is triggered on timeout.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: server/src/relay/relay-client.ts
Line: 1351-1354

Comment:
**Misleading public URL printed to the user if relay omits `publicUrl`**

When a `tunnel-ready` message lacks `publicUrl` (which is typed as `optional` in `protocol.ts`), the fallback:

```typescript
const publicUrl = msg.publicUrl || relayOrigin;
```

resolves to the relay's **root** URL (e.g. `https://your-relay-server.com`) rather than the instance subdomain. This URL is then displayed in the console and emitted via `onReady`, so the operator would navigate to the relay root and see a 404 or wrong instance.

Since the `tunnel-ready` message is what conveys the actual subdomain, consider logging a warning and skipping the `onReady` callback rather than falling back to the root URL:

```suggestion
        case "tunnel-ready": {
          if (!msg.publicUrl) {
            logger.warn({ instanceId: msg.instanceId }, "Relay tunnel ready but no publicUrl provided — cannot display access URL");
            break;
          }
          logger.info({ instanceId: msg.instanceId, publicUrl: msg.publicUrl }, "Relay tunnel ready");
          opts.onReady?.(msg.instanceId, msg.publicUrl);
          break;
        }
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: server/src/relay/auto-register.ts
Line: 70-79

Comment:
**Extra blank line inserted into `.env` on every registration**

When the existing `.env` file ends with a newline (the standard UNIX case), `content.split("\n")` produces a trailing empty string: `["FOO=bar", ""]`. The `filter` step does not remove this empty string, so after `lines.push(...)` the array becomes `["FOO=bar", "", "<KEY>=<value>"]`. Joining then adds `\n` at the end, producing an unwanted blank line between the existing entries and the newly appended key.

Consider filtering out empty strings before pushing the new entry:

```suggestion
    // Remove existing key if present (shouldn't be, but safe)
    const lines = content.split("\n").filter((l) => l !== "" && !l.startsWith(`${key}=`));
    lines.push(`${key}=${value}`);
    fs.writeFileSync(envPath, lines.join("\n") + "\n", "utf-8");
```

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: 35dc304

*/

import fs from "node:fs";
import path from "node:path";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import

path is imported but never referenced anywhere in the file. TypeScript strict-mode compilation may still pass, but this is dead code.

Suggested change
import path from "node:path";
import fs from "node:fs";
import { resolvePaperclipConfigPath, resolvePaperclipEnvPath } from "../paths.js";
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/relay/auto-register.ts
Line: 8

Comment:
**Unused import**

`path` is imported but never referenced anywhere in the file. TypeScript strict-mode compilation may still pass, but this is dead code.

```suggestion
import fs from "node:fs";
import { resolvePaperclipConfigPath, resolvePaperclipEnvPath } from "../paths.js";
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +54 to +59
localWs.on("message", (data: { toString(): string }) => {
send({
type: "ws-message",
id: msg.id,
data: data.toString(),
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Binary WebSocket frames are silently corrupted, not rejected

The file header correctly documents that binary frames are unsupported and "will be corrupted," but the code calls data.toString() on whatever the ws library delivers. For binary frames the library delivers a Buffer, and calling .toString() decodes it as UTF-8 — which corrupts arbitrary binary payloads. The corruption is invisible to the sender and the relay consumer: both sides will receive malformed data with no error signal.

If Paperclip's local WebSocket endpoints ever emit binary frames (e.g. Uint8Array, protobuf payloads), this would produce hard-to-debug data corruption. At a minimum the handler should detect and drop/error on binary:

Suggested change
localWs.on("message", (data: { toString(): string }) => {
send({
type: "ws-message",
id: msg.id,
data: data.toString(),
});
localWs.on("message", (data: Buffer | string) => {
if (Buffer.isBuffer(data)) {
// Binary frames are not supported over the relay tunnel (JSON text only).
// Drop and notify the relay so the client connection can be closed cleanly.
send({ type: "ws-error", id: msg.id, message: "Binary WebSocket frames are not supported through the relay tunnel" });
return;
}
send({
type: "ws-message",
id: msg.id,
data: data.toString(),
});
});
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/relay/ws-bridge.ts
Line: 54-59

Comment:
**Binary WebSocket frames are silently corrupted, not rejected**

The file header correctly documents that binary frames are unsupported and "will be corrupted," but the code calls `data.toString()` on whatever the `ws` library delivers. For binary frames the library delivers a `Buffer`, and calling `.toString()` decodes it as UTF-8 — which corrupts arbitrary binary payloads. The corruption is invisible to the sender and the relay consumer: both sides will receive malformed data with no error signal.

If Paperclip's local WebSocket endpoints ever emit binary frames (e.g. Uint8Array, protobuf payloads), this would produce hard-to-debug data corruption. At a minimum the handler should detect and drop/error on binary:

```suggestion
  localWs.on("message", (data: Buffer | string) => {
    if (Buffer.isBuffer(data)) {
      // Binary frames are not supported over the relay tunnel (JSON text only).
      // Drop and notify the relay so the client connection can be closed cleanly.
      send({ type: "ws-error", id: msg.id, message: "Binary WebSocket frames are not supported through the relay tunnel" });
      return;
    }
    send({
      type: "ws-message",
      id: msg.id,
      data: data.toString(),
    });
  });
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +70 to +79
try {
let content = "";
if (fs.existsSync(envPath)) {
content = fs.readFileSync(envPath, "utf-8");
if (!content.endsWith("\n")) content += "\n";
}
// Remove existing key if present (shouldn't be, but safe)
const lines = content.split("\n").filter((l) => !l.startsWith(`${key}=`));
lines.push(`${key}=${value}`);
fs.writeFileSync(envPath, lines.join("\n") + "\n", "utf-8");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra blank line inserted into .env on every registration

When the existing .env file ends with a newline (the standard UNIX case), content.split("\n") produces a trailing empty string: ["FOO=bar", ""]. The filter step does not remove this empty string, so after lines.push(...) the array becomes ["FOO=bar", "", "<KEY>=<value>"]. Joining then adds \n at the end, producing an unwanted blank line between the existing entries and the newly appended key.

Consider filtering out empty strings before pushing the new entry:

Suggested change
try {
let content = "";
if (fs.existsSync(envPath)) {
content = fs.readFileSync(envPath, "utf-8");
if (!content.endsWith("\n")) content += "\n";
}
// Remove existing key if present (shouldn't be, but safe)
const lines = content.split("\n").filter((l) => !l.startsWith(`${key}=`));
lines.push(`${key}=${value}`);
fs.writeFileSync(envPath, lines.join("\n") + "\n", "utf-8");
// Remove existing key if present (shouldn't be, but safe)
const lines = content.split("\n").filter((l) => l !== "" && !l.startsWith(`${key}=`));
lines.push(`${key}=${value}`);
fs.writeFileSync(envPath, lines.join("\n") + "\n", "utf-8");
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/relay/auto-register.ts
Line: 70-79

Comment:
**Extra blank line inserted into `.env` on every registration**

When the existing `.env` file ends with a newline (the standard UNIX case), `content.split("\n")` produces a trailing empty string: `["FOO=bar", ""]`. The `filter` step does not remove this empty string, so after `lines.push(...)` the array becomes `["FOO=bar", "", "<KEY>=<value>"]`. Joining then adds `\n` at the end, producing an unwanted blank line between the existing entries and the newly appended key.

Consider filtering out empty strings before pushing the new entry:

```suggestion
    // Remove existing key if present (shouldn't be, but safe)
    const lines = content.split("\n").filter((l) => l !== "" && !l.startsWith(`${key}=`));
    lines.push(`${key}=${value}`);
    fs.writeFileSync(envPath, lines.join("\n") + "\n", "utf-8");
```

How can I resolve this? If you propose a fix, please make it concise.

Adds a relay tunnel client that connects to a Cloudflare Workers-based
relay server, enabling remote access to Paperclip instances without port
forwarding. Each instance gets a unique subdomain (e.g. d4lsc.relay.com).

Server changes (server/src/relay/):
- Auto-registration: POST /register on first start, persist token to
  .env, add relay hostname to config.json allowedHostnames
- HTTP forwarder with proxyRes error handling and multi-value set-cookie
- WebSocket bridge with close code validation (RFC 6455)
- Relay client with exponential backoff reconnect and graceful shutdown
- Token sent via Authorization header (not URL query string)

Security:
- Relay requires authenticated deployment mode — guard fires before
  any registration to prevent orphaned credentials in local_trusted
- Follows existing Tailscale guard pattern in server/src/index.ts

Documentation:
- doc/RELAY.md: full relay server source, architecture, security model
- docs/deploy/relay-tunnel.md: user-facing setup guide (Mintlify)
- Updated deployment-modes, overview, env-vars, Tailscale, CLI,
  DEVELOPING, local-development, and README with relay references
- All examples use generic your-relay-server.com; paperclip-relay.com
  clearly labeled as community test relay (not affiliated with Paperclip AI)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@lucasygu lucasygu force-pushed the feat/relay-tunnel-v3 branch from 35dc304 to 95e8642 Compare March 13, 2026 23:23
@lucasygu
Copy link
Author

Superseded by a new PR with binary multiplexing protocol, isBinary fix, and board mutation guard fix.

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.

feat: Zero-config remote access — built-in tunnel so any device can reach a local Paperclip instance

1 participant