Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,38 @@ Human Review opens the file in your browser. Make direct edits, leave comments,

Note: For HTML files, direct edits and resizes save automatically. For Markdown and localhost pages, click Send so your agent can apply them to the source.

## Reviewing from another device (phone, LAN, Tailscale)

By default human-review binds to `127.0.0.1` and serves the reviewed app from the *other* loopback name (`localhost` ↔ `127.0.0.1`). The shell and the reviewed page are deliberately **two different origins** — that's what stops the reviewed app's scripts from reading the session token off the shell.

To review from a phone or another machine, configure both sides explicitly. **This is for private networks only (Tailscale / VPN / trusted LAN)** — see the exposure warning below.

| Env var | Effect | Default |
|---|---|---|
| `HUMAN_REVIEW_HOST` | Server bind address | `127.0.0.1` |
| `HUMAN_REVIEW_ALLOWED_HOSTS` | Comma-separated extra `Host`-header allowlist | loopback only |
| `HUMAN_REVIEW_PUBLIC_URL` | URL printed/used for the session (e.g. `http://100.x.y.z:8124`); when set, the CLI skips auto-opening the browser | — |
| `HUMAN_REVIEW_ARTIFACT_HOST` | Hostname the reviewed page's iframe loads from | the other loopback name |
| `HUMAN_REVIEW_CHROME_ORIGIN` | Origin the SDK posts messages to (the shell) | auto-derived |

⚠️ **You must use two distinct hostnames** for the shell and the artifact. If `HUMAN_REVIEW_ARTIFACT_HOST` resolves to the same origin as the shell, the reviewed app's scripts would gain same-origin access to the shell — including the session token. human-review refuses to serve the session in that case; the error page explains the requirement.

Example (Tailscale): shell on the tailnet IP, artifact on the MagicDNS name:

```sh
HUMAN_REVIEW_HOST=0.0.0.0
HUMAN_REVIEW_ALLOWED_HOSTS=100.101.102.103:8124,my-laptop.tailnet-name.ts.net:8124
HUMAN_REVIEW_PUBLIC_URL=http://100.101.102.103:8124
HUMAN_REVIEW_ARTIFACT_HOST=my-laptop.tailnet-name.ts.net
human-review my-page.html
```

Open `http://100.101.102.103:8124/s/<id>` on your phone.

> ⚠️ **Exposure model — private networks only.** With `HUMAN_REVIEW_HOST=0.0.0.0` the server is reachable by anyone who can reach the port *and* sends an allowed `Host` header. Token-free routes become network-readable: `/artifact/<key>/…` (keys are `sha256(realpath)` truncated to 16 hex chars, derivable from file paths) serves the reviewed page and sibling assets, and `/s/<id>` carries the session token over plain HTTP. On a private tailnet that's fine; on shared or public Wi-Fi it is **not** — anyone sniffing or on the network could read your reviewed files. Don't bind to `0.0.0.0` on untrusted networks.

> ℹ️ Env vars are read by the server when it **starts**. If you already have a server running, stop it first before changing these values, or the old settings stay in effect. The detached server exits on its own after being idle (default 45 min, `HUMAN_REVIEW_IDLE_MS`) — or kill the stale one via its PID in `server.json` / Task Manager.

## What this skill lets you do

- **Edit text directly and tweak basic formatting** (e.g., bold, italic).
Expand Down
26 changes: 19 additions & 7 deletions src/chrome-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,11 @@ async function api(path, options) {
return res.json();
}

// Keep the reviewed app on a different loopback origin from the review shell.
// This gives route-aware frameworks a real origin without exposing the parent UI.
const ARTIFACT_HOST = location.hostname === "127.0.0.1" ? "localhost" : "127.0.0.1";
// Keep the reviewed app on a different origin from the review shell (two
// loopback names on desktop; an env-provided host like a Tailscale DNS name
// when the shell is served over the network). This gives route-aware
// frameworks a real origin without exposing the parent UI.
const ARTIFACT_HOST = document.body.dataset.artifactHost || (location.hostname === "127.0.0.1" ? "localhost" : "127.0.0.1");
const ARTIFACT_ORIGIN = `${location.protocol}//${ARTIFACT_HOST}:${location.port}`;

// URL reviews keep a real origin. File reviews use an opaque sandbox origin,
Expand Down Expand Up @@ -713,15 +715,24 @@ $("note").addEventListener("input", (event) => {

$("handle").addEventListener("click", () => {
const collapsed = document.body.classList.toggle("collapsed");
const handle = $("handle");
handle.textContent = collapsed ? "‹" : "›";
handle.title = collapsed ? "Show comments panel" : "Hide comments panel";
handle.setAttribute("aria-label", handle.title);
syncHandle();
try {
localStorage.setItem("human-review:collapsed", collapsed ? "1" : "0");
} catch {}
});

/** Match the toggle glyph to the current layout: side arrows for the desktop
* rail, up/down arrows for the mobile bottom sheet. */
function syncHandle() {
const handle = $("handle");
const collapsed = document.body.classList.contains("collapsed");
const desktop = window.matchMedia("(min-width: 900px)").matches;
handle.textContent = collapsed ? (desktop ? "‹" : "▲") : (desktop ? "›" : "▼");
handle.title = collapsed ? "Show comments panel" : "Hide comments panel";
handle.setAttribute("aria-label", handle.title);
}
window.matchMedia("(min-width: 900px)").addEventListener("change", syncHandle);

$("theme").addEventListener("click", () => {
const dark = document.documentElement.dataset.theme !== "dark";
applyTheme(dark);
Expand Down Expand Up @@ -803,6 +814,7 @@ function connect() {
try {
applyTheme(localStorage.getItem("human-review:theme") === "dark");
if (localStorage.getItem("human-review:collapsed") === "1") $("handle").click();
syncHandle();
} catch {}

const bootstrap = await api(`/api/session/${state.sessionId}/page`).catch(() => null);
Expand Down
91 changes: 72 additions & 19 deletions src/chrome.css
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
--btn-bg: #1b1a16;
--btn-fg: #fbfaf7;
--rail-w: 352px;
--sheet-h: clamp(300px, 55vh, 520px);
}

:root[data-theme="dark"] {
Expand Down Expand Up @@ -74,48 +75,100 @@ body {

button, textarea { font: inherit; color: inherit; }

.app { display: flex; height: 100vh; }
.app { height: 100vh; }

/* The artifact takes all remaining width and styles itself. */
.stage { flex: 1; min-width: 0; background: var(--canvas); }
/* The artifact takes the whole viewport; the rail floats over it. */
.stage { height: 100%; width: 100%; background: var(--canvas); }
#frame { width: 100%; height: 100%; border: 0; display: block; background: #fff; }

/* Bottom-sheet toggle: a centered grabber tab that rides the sheet's top
edge when open, and drops to the bottom edge when closed. */
.handle {
position: fixed;
top: 50%;
right: var(--rail-w);
z-index: 40;
width: 22px;
height: 46px;
margin-top: -23px;
left: 50%;
bottom: var(--sheet-h);
transform: translateX(-50%);
z-index: 50;
width: 76px;
height: 28px;
display: flex;
align-items: center;
justify-content: center;
border: 1px solid var(--hair);
border-right: 0;
border-radius: 7px 0 0 7px;
border-bottom: 0;
border-radius: 12px 12px 0 0;
background: var(--rail);
color: var(--mute);
font-size: 11px;
line-height: 1;
cursor: pointer;
box-shadow: -2px 0 8px -6px rgba(0, 0, 0, 0.28);
transition: right 140ms ease-out;
box-shadow: 0 -4px 14px -8px rgba(0, 0, 0, 0.3);
transition: bottom 200ms ease-out;
}
.handle:hover { color: var(--strong-txt); background: var(--soft); }
body.collapsed .handle { bottom: 0; }

/* Bottom-sheet rail: floats over the artifact, never disturbs the page. */
.rail {
flex: none;
width: var(--rail-w);
position: fixed;
left: 50%;
bottom: 0;
width: min(720px, 100%);
height: var(--sheet-h);
display: flex;
flex-direction: column;
overflow: hidden;
border-left: 1px solid var(--hair);
border-top: 1px solid var(--hair);
border-radius: 14px 14px 0 0;
background: var(--rail);
transition: width 140ms ease-out;
box-shadow: 0 -10px 34px rgba(0, 0, 0, 0.16);
transform: translate(-50%, 105%);
transition: transform 200ms ease-out;
z-index: 40;
}
body:not(.collapsed) .rail { transform: translate(-50%, 0); }

/* Desktop: keep the classic right-side rail. The mobile bottom sheet applies
below this width; the handle returns to a right-edge tab and the rail
becomes a fixed-width flex column that the artifact shares the row with. */
@media (min-width: 900px) {
.app { display: flex; height: 100vh; }
.stage { flex: 1; min-width: 0; width: auto; }

.handle {
top: 50%;
bottom: auto;
left: auto;
right: var(--rail-w);
transform: none;
width: 22px;
height: 46px;
margin-top: -23px;
border: 1px solid var(--hair);
border-right: 0;
border-radius: 7px 0 0 7px;
box-shadow: -2px 0 8px -6px rgba(0, 0, 0, 0.28);
transition: right 140ms ease-out;
}
body.collapsed .handle { right: 0; }

.rail {
position: static;
left: auto;
bottom: auto;
width: var(--rail-w);
height: auto;
flex: none;
transform: none;
border-top: 0;
border-left: 1px solid var(--hair);
border-radius: 0;
box-shadow: none;
transition: width 140ms ease-out;
}
body:not(.collapsed) .rail { transform: none; }
body.collapsed .rail { width: 0; border-left-width: 0; }
}
body.collapsed .rail { width: 0; border-left-width: 0; }
body.collapsed .handle { right: 0; }

.rail-scroll { flex: 1; min-height: 0; overflow-y: auto; padding: 16px 12px 12px; }

Expand Down
4 changes: 2 additions & 2 deletions src/chrome.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<title>human-review</title>
<link rel="stylesheet" href="/chrome.css">
</head>
<body data-session="__SESSION_ID__" data-token="__TOKEN__">
<body data-session="__SESSION_ID__" data-token="__TOKEN__" data-artifact-host="__ARTIFACT_HOST__">
<div class="app">
<main class="stage">
<iframe
Expand All @@ -15,7 +15,7 @@
sandbox="allow-scripts allow-forms allow-modals allow-popups allow-downloads"></iframe>
</main>

<button type="button" id="handle" class="handle" title="Hide comments panel" aria-label="Hide comments panel"></button>
<button type="button" id="handle" class="handle" title="Hide comments panel" aria-label="Hide comments panel"></button>

<aside class="rail" id="rail">
<div class="rail-scroll">
Expand Down
8 changes: 6 additions & 2 deletions src/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,12 @@ async function openCommand(input) {
console.error(body.error || "Could not open that file.");
process.exit(1);
}
const url = `http://127.0.0.1:${server.port}${body.path}`;
openBrowser(url);
const publicUrl = process.env.HUMAN_REVIEW_PUBLIC_URL;
const publicHost = publicUrl || `http://127.0.0.1:${server.port}`;
const url = `${publicHost}${body.path}`;
// The local machine's browser can't reach the phone's view of the app, so
// opening it here is noise — print the URL and let the phone do the opening.
if (!publicUrl) openBrowser(url);
console.log(`Reviewing ${target.kind === "url" ? target.value : path.basename(target.value)}`);
console.log(url);
console.log(`\nWaiting for feedback? Run:\n human-review poll ${shellQuote(target.value)}`);
Expand Down
5 changes: 3 additions & 2 deletions src/html-transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ function absolutizeAssets(html, baseHref) {
* Add the review bootstrap tags. Everything else about the artifact is left
* byte-identical, so the saved file renders the same standalone.
*/
export function injectSdk(html, key, { src = `/sdk.js?key=${encodeURIComponent(key)}`, baseHref = "" } = {}) {
export function injectSdk(html, key, { src = `/sdk.js?key=${encodeURIComponent(key)}`, baseHref = "", chromeOrigin = "" } = {}) {
const clean = stripSdk(html);
const escapedSrc = String(src).replace(/&/g, "&amp;").replace(/"/g, "&quot;");
const tag = `<script data-eh-sdk type="module" src="${escapedSrc}"></script>`;
const originAttr = chromeOrigin ? ` data-chrome-origin="${String(chromeOrigin).replace(/&/g, "&amp;").replace(/"/g, "&quot;")}"` : "";
const tag = `<script data-eh-sdk${originAttr} type="module" src="${escapedSrc}"></script>`;
let prepared = clean;
if (baseHref) {
const url = new URL(baseHref);
Expand Down
9 changes: 6 additions & 3 deletions src/sdk.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@ const EDIT_FLUSH_MS = 500;
const MEDIA = /^(img|svg|canvas|video|picture|iframe|hr|figure)$/i;

// The chrome page lives on the other loopback hostname (a separate origin, so
// the reviewed document can never touch it directly). Address it explicitly so
// nothing we post can be read by any other embedder.
const CHROME_ORIGIN = `${location.protocol}//${location.hostname === "127.0.0.1" ? "localhost" : "127.0.0.1"}:${location.port}`;
// the reviewed document can never touch it directly). When the review is
// served over a real host (e.g. Tailscale), the server injects the chrome
// origin explicitly via data-chrome-origin on the SDK script tag.
const CHROME_ORIGIN =
document.querySelector("script[data-eh-sdk]")?.dataset.chromeOrigin ||
`${location.protocol}//${location.hostname === "127.0.0.1" ? "localhost" : "127.0.0.1"}:${location.port}`;

const post = (type, payload) => parent.postMessage({ ...payload, type }, CHROME_ORIGIN);

Expand Down
60 changes: 56 additions & 4 deletions src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,38 @@ const MAX_LOCAL_REDIRECTS = 5;
const LOCAL_FETCH_TIMEOUT_MS = 30000;
const MAX_LOCAL_PAGE_BYTES = 24 * 1024 * 1024;

/**
* Normalize a host[:port] (or scheme://host[:port]/path) to a lowercase
* host:port string for origin comparison. Missing ports fall back to the
* server's own port (the artifact iframe always uses the shell's port).
*/
function originHostPort(hostPort, defaultPort) {
let s = String(hostPort || "").trim().toLowerCase();
s = s.replace(/^https?:\/\//, "").replace(/\/.*$/, "");
const m = s.match(/^(\[[^\]]*\])(?::(\d+))?$/);
if (m) return `${m[1]}:${m[2] || defaultPort || ""}`;
const idx = s.lastIndexOf(":");
if (idx !== -1 && /^\d+$/.test(s.slice(idx + 1))) return `${s.slice(0, idx)}:${s.slice(idx + 1)}`;
return `${s}:${defaultPort || ""}`;
}

/** HTML error shown when the shell and artifact would share an origin. */
function sameOriginError(shellOrigin) {
return `<!doctype html>
<html><head><meta charset="utf-8"><meta name="viewport" content="width=device-width, initial-scale=1">
<title>human-review — same-origin config refused</title></head>
<body style="font-family:system-ui,sans-serif;max-width:640px;margin:48px auto;padding:0 20px;line-height:1.55">
<h2 style="margin-bottom:8px">⚠️ Refusing to open: shell and artifact share an origin</h2>
<p>The review shell is served from <code>${shellOrigin}</code> and <code>HUMAN_REVIEW_ARTIFACT_HOST</code> points at the <b>same</b> host. A URL-kind review keeps <code>allow-same-origin</code>, so the reviewed app's scripts would get full same-origin access to the parent shell — including the session token in <code>data-token</code>. The entire two-loopback design exists to prevent exactly this.</p>
<p>Remote reviews need <b>two distinct hostnames</b> that both reach this machine, e.g. the Tailscale IP for the shell and the MagicDNS name for the artifact (or vice versa):</p>
<pre style="background:#f6f6f6;padding:12px;border-radius:8px;overflow:auto">HUMAN_REVIEW_HOST=0.0.0.0
HUMAN_REVIEW_ALLOWED_HOSTS=100.101.102.103:8124,my-laptop.tailnet-name.ts.net:8124
HUMAN_REVIEW_PUBLIC_URL=http://100.101.102.103:8124
HUMAN_REVIEW_ARTIFACT_HOST=my-laptop.tailnet-name.ts.net</pre>
<p>Then open the shell via the <b>other</b> hostname. The localhost defaults (shell on <code>127.0.0.1</code>, artifact on <code>localhost</code>, or vice versa) are unaffected.</p>
</body></html>`;
}

const hash = (text) => crypto.createHash("sha1").update(text).digest("hex");
const uid = (prefix) => `${prefix}_${crypto.randomBytes(6).toString("hex")}`;

Expand Down Expand Up @@ -427,7 +459,11 @@ export function createServer() {
// it were same-origin.
const host = String(req.headers.host || "");
const port = req.socket.localPort;
if (host !== `127.0.0.1:${port}` && host !== `localhost:${port}`) {
const allowedHosts = (process.env.HUMAN_REVIEW_ALLOWED_HOSTS || "")
.split(",")
.map((s) => s.trim())
.filter(Boolean);
if (host !== `127.0.0.1:${port}` && host !== `localhost:${port}` && !allowedHosts.includes(host)) {
res.writeHead(403, { "content-type": "text/plain" });
return res.end("Forbidden");
}
Expand Down Expand Up @@ -486,8 +522,24 @@ export function createServer() {
}
seen(sessions.get(id));
const shell = fs.readFileSync(path.join(here, "chrome.html"), "utf8");
const artifactHost = process.env.HUMAN_REVIEW_ARTIFACT_HOST
|| (host.startsWith("127.0.0.1") ? "localhost" : "127.0.0.1");
// Security guard (review #1): the artifact iframe must live on a
// DIFFERENT origin from the shell. If both resolve to the same
// host:port, a URL-kind review (allow-same-origin) could read the
// session token off the shell — the two-loopback design exists to
// prevent exactly this. Refuse the token page with a clear error.
if (originHostPort(host, port) === originHostPort(artifactHost, port)) {
res.writeHead(500, { "content-type": "text/html; charset=utf-8", "cache-control": "no-store" });
return res.end(sameOriginError(originHostPort(host, port)));
}
res.writeHead(200, { "content-type": MIME[".html"], "cache-control": "no-store" });
return res.end(shell.replace("__SESSION_ID__", id).replace("__TOKEN__", token));
return res.end(
shell
.replace("__SESSION_ID__", id)
.replace("__TOKEN__", token)
.replace("__ARTIFACT_HOST__", artifactHost)
);
}

// --- the reviewed page itself, plus sibling assets for file targets
Expand Down Expand Up @@ -527,7 +579,7 @@ export function createServer() {
if (isMarkdown(page.file)) html = renderMarkdownPage(html, page.file);
}
res.writeHead(200, { "content-type": MIME[".html"], "cache-control": "no-store" });
return res.end(injectSdk(html, key, sdkOptions));
return res.end(injectSdk(html, key, { ...sdkOptions, chromeOrigin: process.env.HUMAN_REVIEW_CHROME_ORIGIN }));
}
if (page.kind === "url") {
res.writeHead(404, { "content-type": "text/plain" });
Expand Down Expand Up @@ -862,7 +914,7 @@ export function start(port = 0) {
console.error(`human-review server could not listen on port ${port}: ${err.message}`);
reject(err);
});
server.listen(port, "127.0.0.1", () => {
server.listen(port, process.env.HUMAN_REVIEW_HOST || "127.0.0.1", () => {
const actual = server.address().port;
ensureStateDir();
fs.writeFileSync(serverPath(), JSON.stringify({ port: actual, pid: process.pid, token, protocol: SERVER_PROTOCOL }));
Expand Down
Loading