Authenticate /hook/*, and make every hook a command hook
/hook/record and /hook/approve accepted anything that reached the port. That is not "loopback only": `tailscale serve` proxies tailnet traffic to 127.0.0.1, so anyone who could reach the tunnel could forge timeline events and answer approval prompts. Both endpoints now require a 32-byte secret from $GLANCE_HOME/hook.secret (0600, created once, never rotated so nothing in flight is 403'd mid-session), compared in constant time before the body is read, as an x-glance-hook header or a ?k= parameter. Requests carrying x-forwarded-* are refused outright: a local hook process never sends them and a tunnelled caller always does. The check applies to /hook/* only, so the dashboard is unaffected. While wiring that up: the 13 passive `type: "http"` hooks could never have worked. Grok Build's http runner rejects every scheme but https, then resolves the host and blocks private/link-local/CGNAT addresses (validate_hook_url + is_blocked_ip), so neither loopback-over-http nor *.ts.net (100.64/10) can be a hook target - and it sends no header but Content-Type, so such a hook could not authenticate anyway. They were failing validation silently on every event. All of them are now command hooks running bin/glance-record.mjs, which costs a Node start and can present the secret. hooks.json is generated from hooks/hooks.template.json by scripts/gen-hooks.mjs (npm run build, glance sync-hooks). It creates the secret, derives the approval hook's timeout from approval.timeoutMs instead of hand-copying 125, and refuses to write a hook that cannot fire: bad type, non-positive timeout, non-https http URL, missing bin/ script, or a leftover placeholder. A template that embeds the token makes the output 0600 with a warning. Fail-open is unchanged: a missing, stale or rejected secret degrades to "no telemetry", and glance-approve.mjs still allows on every error path. glance status warns when the on-disk secret no longer matches the daemon's. Validated with the e2e suite (190 checks, including no-token/wrong-token/ same-length-token 403s, ?k= acceptance, x-forwarded-* refusal, and the recorder's fail-open paths) and a clean npm run build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
b3b6bf3f70
commit
5eec1940be
@@ -5,6 +5,10 @@ import type { IncomingMessage } from "node:http";
|
||||
|
||||
export const SESSION_COOKIE = "glance_session";
|
||||
export const CSRF_HEADER = "x-glance-csrf";
|
||||
/** Shared-secret header presented by the hook scripts on /hook/*. */
|
||||
export const HOOK_HEADER = "x-glance-hook";
|
||||
/** Query-string carrier for the same secret, for hooks that cannot set headers. */
|
||||
export const HOOK_QUERY_PARAM = "k";
|
||||
|
||||
export function parseCookies(header: string | undefined): Record<string, string> {
|
||||
const out: Record<string, string> = {};
|
||||
|
||||
@@ -43,6 +43,9 @@ export const paths = {
|
||||
get adminToken() {
|
||||
return path.join(glanceHome(), "admin.token");
|
||||
},
|
||||
get hookSecret() {
|
||||
return path.join(glanceHome(), "hook.secret");
|
||||
},
|
||||
get events() {
|
||||
return path.join(glanceHome(), "events.jsonl");
|
||||
},
|
||||
|
||||
+44
-6
@@ -3,10 +3,13 @@
|
||||
*
|
||||
* One small http server with three kinds of caller:
|
||||
*
|
||||
* /hook/* the plugin's hook scripts, on loopback. /hook/approve is the blocking one.
|
||||
* /hook/* the plugin's hook scripts, gated by the shared secret in hook.secret.
|
||||
* /api/* the web app, gated by a passkey-backed cookie session.
|
||||
* /local/* the `glance` CLI, gated by a rotating admin token on disk.
|
||||
*
|
||||
* None of the three trusts the source address: `tailscale serve` proxies tailnet traffic to
|
||||
* 127.0.0.1, so every caller looks local.
|
||||
*
|
||||
* Everything the hooks touch is written to fail open: if this process is confused, wedged, or
|
||||
* gone, Grok Build keeps working.
|
||||
*/
|
||||
@@ -28,6 +31,8 @@ import {
|
||||
import {
|
||||
CSRF_HEADER,
|
||||
EnrollmentCodes,
|
||||
HOOK_HEADER,
|
||||
HOOK_QUERY_PARAM,
|
||||
RateLimiter,
|
||||
SESSION_COOKIE,
|
||||
buildSessionCookie,
|
||||
@@ -47,7 +52,9 @@ import { SESSION_TTL_MS, WebAuthnService } from "./webauthn.js";
|
||||
import {
|
||||
destroyAuthSession,
|
||||
deviceList,
|
||||
hookSecret,
|
||||
lookupAuthSession,
|
||||
readHookSecretFromDisk,
|
||||
revokeCredentials,
|
||||
rotateAdminToken,
|
||||
sessionSecret,
|
||||
@@ -60,6 +67,7 @@ ensureHome();
|
||||
const cfg = loadConfig();
|
||||
const secret = sessionSecret();
|
||||
const adminToken = rotateAdminToken();
|
||||
const hookToken = hookSecret();
|
||||
const webauthn = new WebAuthnService(cfg);
|
||||
const codes = new EnrollmentCodes();
|
||||
|
||||
@@ -93,14 +101,36 @@ function currentSession(req: http.IncomingMessage): Session | null {
|
||||
return { token, credentialId: record.credentialId, label: record.label };
|
||||
}
|
||||
|
||||
function isAdmin(req: http.IncomingMessage): boolean {
|
||||
const provided = header(req, "x-glance-admin");
|
||||
function sameSecret(provided: string | undefined | null, expected: string): boolean {
|
||||
if (!provided) return false;
|
||||
const a = Buffer.from(provided);
|
||||
const b = Buffer.from(adminToken);
|
||||
const b = Buffer.from(expected);
|
||||
return a.length === b.length && crypto.timingSafeEqual(a, b);
|
||||
}
|
||||
|
||||
function isAdmin(req: http.IncomingMessage): boolean {
|
||||
return sameSecret(header(req, "x-glance-admin"), adminToken);
|
||||
}
|
||||
|
||||
/**
|
||||
* Is this really one of our hook scripts?
|
||||
*
|
||||
* "It came from 127.0.0.1" proves nothing: `tailscale serve` proxies tailnet traffic to
|
||||
* loopback, so without a check anyone on the tailnet could POST forged events into the
|
||||
* timeline and answer /hook/approve on your behalf. Two independent barriers:
|
||||
*
|
||||
* 1. A shared secret from $GLANCE_HOME/hook.secret (mode 0600), presented as a header or,
|
||||
* for hook types that cannot set one, as `?k=`. Compared in constant time.
|
||||
* 2. The request must not have been proxied. Tailscale stamps `x-forwarded-*` on anything
|
||||
* it tunnels, so their presence means the caller is not a local process — which no
|
||||
* real hook ever is. This keeps a leaked secret from being usable off-box.
|
||||
*/
|
||||
function isHookCaller(req: http.IncomingMessage, url: URL): boolean {
|
||||
if (header(req, "x-forwarded-for") || header(req, "x-forwarded-proto")) return false;
|
||||
const provided = header(req, HOOK_HEADER) ?? url.searchParams.get(HOOK_QUERY_PARAM);
|
||||
return sameSecret(provided, hookToken);
|
||||
}
|
||||
|
||||
/**
|
||||
* `application/json` is not a CORS-safelisted content type, so requiring it exactly means a
|
||||
* hostile page cannot post here without a preflight we never answer. The custom header on
|
||||
@@ -154,6 +184,11 @@ async function handle(req: http.IncomingMessage, res: http.ServerResponse): Prom
|
||||
out.json(405, { error: "post json" });
|
||||
return;
|
||||
}
|
||||
// Before reading a body: an unauthenticated caller gets to spend nothing here.
|
||||
if (!isHookCaller(req, url)) {
|
||||
out.json(403, { error: "hook token required" });
|
||||
return;
|
||||
}
|
||||
const payload = ((await readJson<HookPayload>(req)) ?? {}) as HookPayload;
|
||||
|
||||
if (p === "/hook/record") {
|
||||
@@ -163,8 +198,8 @@ async function handle(req: http.IncomingMessage, res: http.ServerResponse): Prom
|
||||
}
|
||||
|
||||
if (p === "/hook/approve") {
|
||||
// Note: this deliberately does not ingest an event. The PreToolUse http hook already
|
||||
// recorded the tool call; recording it here too would double every entry.
|
||||
// Note: this deliberately does not ingest an event. The PreToolUse recording hook
|
||||
// already logged the tool call; recording it here too would double every entry.
|
||||
const decision = await broker.request(payload);
|
||||
out.json(200, decision);
|
||||
return;
|
||||
@@ -194,6 +229,9 @@ async function handle(req: http.IncomingMessage, res: http.ServerResponse): Prom
|
||||
events: state.eventCount,
|
||||
webBuilt: webBuildExists(),
|
||||
home: paths.home,
|
||||
// Would a hook script authenticate right now? The daemon holds the token it read at
|
||||
// startup; if the file has since changed or gone, recording is silently dropping.
|
||||
hookAuthOk: sameSecret(readHookSecretFromDisk(), hookToken),
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -69,6 +69,47 @@ export function rotateAdminToken(): string {
|
||||
return token;
|
||||
}
|
||||
|
||||
/**
|
||||
* Token the hook scripts present on /hook/*. Unlike `admin.token` this is *not* rotated on
|
||||
* every start: hook scripts are separate short-lived processes that read the file per
|
||||
* invocation, and a rotation mid-session would 403 whatever was already in flight.
|
||||
*
|
||||
* It exists because `tailscale serve` proxies tailnet traffic to 127.0.0.1, so "the request
|
||||
* came from loopback" says nothing about who sent it. Without this, anyone on the tailnet
|
||||
* could forge timeline events and answer approval prompts.
|
||||
*
|
||||
* Created exclusively (`wx`) so two hooks racing on a fresh home cannot end up with
|
||||
* different values — the loser re-reads the winner's file.
|
||||
*/
|
||||
export function hookSecret(): string {
|
||||
ensureHome();
|
||||
for (let attempt = 0; attempt < 2; attempt++) {
|
||||
try {
|
||||
const existing = fs.readFileSync(paths.hookSecret, "utf8").trim();
|
||||
if (existing) return existing;
|
||||
} catch {
|
||||
/* create below */
|
||||
}
|
||||
const token = crypto.randomBytes(32).toString("base64url");
|
||||
try {
|
||||
fs.writeFileSync(paths.hookSecret, token + "\n", { mode: 0o600, flag: "wx" });
|
||||
return token;
|
||||
} catch {
|
||||
// Lost the race (or the file appeared between the read and the write): read it back.
|
||||
}
|
||||
}
|
||||
return fs.readFileSync(paths.hookSecret, "utf8").trim();
|
||||
}
|
||||
|
||||
/** Whatever is on disk right now, for diagnostics. Never creates the file. */
|
||||
export function readHookSecretFromDisk(): string | null {
|
||||
try {
|
||||
return fs.readFileSync(paths.hookSecret, "utf8").trim() || null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/* -------------------------------------------------------------- credentials */
|
||||
|
||||
export function listCredentials(): StoredCredential[] {
|
||||
|
||||
Reference in New Issue
Block a user