Merge pull request #281 from LeXwDeX/fix/http-hook-headers

fix(hook): apply http-hook headers and harden hookAdd validation

Author: LeXwDeX

packages/opencode/src/goal/loop.ts

@@ -474,24 +474,26 @@ const serviceLayer = Layer.effect(
           Effect.catchCause((cause) =>
             Effect.gen(function* () {
               // F1: Only pause for non-interrupt causes. An interrupt (user
-              // pressed ESC during continuation) is safe to drop because the
-              // session ALWAYS re-emits idle afterwards, which re-drives this
-              // loop: SessionRunState.cancel (run-state.ts) and the runner's
-              // onIdle callback both call status.set(idle), and
-              // SessionStatus.set (status.ts) publishes the Status+Idle event
-              // pair unconditionally — even when the session was already idle.
-              // That fresh idle event forks a new afterIdle fiber whose
-              // shouldPreempt guard detects the user's newer message and pauses
-              // there if needed. Pausing HERE would race that replacement
-              // afterIdle fiber and emit a spurious pause. Real dispatch
-              // failures (provider fault, session write error) still get the
-              // recoverable pause below.
+              // pressed ESC during continuation) is safe to drop because
+              // SessionPrompt.cancel pauses goal-driven turns SYNCHRONOUSLY via
+              // goal.pauseForUserCancel (prompt.ts) BEFORE state.cancel lets the
+              // interrupt propagate — by the time this catchCause observes the
+              // cause, the goal is already paused, and pausing again HERE would
+              // double-publish. The session still ALWAYS re-emits idle
+              // afterwards, which re-drives this loop: SessionRunState.cancel
+              // (run-state.ts) and the runner's onIdle callback both call
+              // status.set(idle), and SessionStatus.set (status.ts) publishes
+              // the Status+Idle event pair unconditionally — even when the
+              // session was already idle. On that next cycle shouldPreempt is
+              // only the DB-failure fallback for a pauseForUserCancel that could
+              // not persist. Real dispatch failures (provider fault, session
+              // write error) still get the recoverable pause below.
               // F1: hasInterrupts is a structural check; Cause.interruptors only
               // collects DEFINED fiber ids and silently ignores interrupts
               // carrying none (e.g. Cause.interrupt()), which would otherwise be
               // misclassified as a dispatch failure and spuriously paused here.
               if (Cause.hasInterrupts(cause)) {
-                yield* Effect.logInfo("goal continuation interrupted (likely user ESC) — not pausing; shouldPreempt handles next cycle")
+                yield* Effect.logInfo("goal continuation interrupted (likely user ESC) — not pausing; cancel path already paused the goal")
                 return Option.none()
               }
               const errMsg = `continuation dispatch failed: ${Cause.pretty(cause)}`

packages/opencode/src/hook/settings.ts

@@ -1432,10 +1432,11 @@ const mcpHandler: HookHandler = {
 
 /**
  * `type: "http"` handler. Per CC protocol, `entry.command` is the endpoint URL;
- * the envelope is POSTed as JSON. 2xx → body parsed via the same parseStdout path
- * as command stdout. Non-2xx → synthetic `exitBlock` so the trigger aggregator
- * surfaces it as a block. Network errors / timeouts → log.warn + silent allow,
- * mirroring commandHandler's spawnError behavior (hooks must never crash the host).
+ * the envelope is POSTed as JSON with `entry.headers` applied verbatim (auth
+ * tokens etc.). 2xx → body parsed via the same parseStdout path as command
+ * stdout. Non-2xx → synthetic `exitBlock` so the trigger aggregator surfaces it
+ * as a block. Network errors / timeouts → log.warn + silent allow, mirroring
+ * commandHandler's spawnError behavior (hooks must never crash the host).
  *
  * Factory takes the resolved HttpClient so the HookHandler.run signature stays
  * `R = never` (the WP-4A interface contract). Captures `http` in closure scope —
@@ -1454,6 +1455,7 @@ const httpHandler: HookHandler = {
 
     const url = httpUrl(entry)
     const exit = yield* HttpClientRequest.post(url).pipe(
+      HttpClientRequest.setHeaders(entry.headers ?? {}),
       HttpClientRequest.bodyJson(envelope),
       Effect.flatMap((req) => httpRead.execute(req)),
       Effect.flatMap((res) =>

packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts

@@ -440,6 +440,16 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session",
       // Non-empty hooks[] guard → 4xx. event/type membership is already enforced
       // by the payload's literal schemas; this completes the validation contract.
       if (ctx.payload.hooks.length === 0) return yield* new HttpApiError.BadRequest({})
+      // Per-entry guards: a command hook must carry a runnable command line
+      // (blank-only strings would spawn nothing), and timeout is a positive
+      // seconds multiplier — negative values time the request out immediately,
+      // and 0 silently falls back to the default instead of meaning "no timeout".
+      const invalidEntry = ctx.payload.hooks.some(
+        (hook) =>
+          (hook.type === "command" && !(hook.command ?? "").trim()) ||
+          (hook.timeout !== undefined && hook.timeout <= 0),
+      )
+      if (invalidEntry) return yield* new HttpApiError.BadRequest({})
       const id = yield* sessionHooks.add(ctx.params.sessionID, {
         event: ctx.payload.event as HookEvent,
         matcher: ctx.payload.matcher,

packages/opencode/test/hook/http-handler.test.ts

@@ -0,0 +1,94 @@
+import { describe, expect } from "bun:test"
+import { Effect, Layer } from "effect"
+import { FetchHttpClient } from "effect/unstable/http"
+import { SettingsHook } from "@/hook/settings"
+import { SessionHooks } from "@/hook/session-hooks"
+import { EventV2Bridge } from "@/event-v2-bridge"
+import { Database } from "@opencode-ai/core/database/database"
+import { SessionID } from "@/session/schema"
+import { testEffect } from "../lib/effect"
+
+// httpHandler runtime contract against a real local server: configured
+// entry.headers MUST reach the wire (auth tokens were silently dropped before
+// this fix), and non-2xx responses MUST surface as the synthetic exitBlock so
+// the trigger aggregator reports a block.
+
+const testLayer = SettingsHook.layer.pipe(
+  Layer.provide(EventV2Bridge.defaultLayer),
+  Layer.provide(Database.defaultLayer),
+  Layer.provideMerge(SessionHooks.defaultLayer),
+  Layer.provideMerge(FetchHttpClient.layer),
+)
+const it = testEffect(testLayer)
+
+const withFetch = <A, E, R>(
+  fetch: (req: Request) => Response | Promise<Response>,
+  fn: (url: string) => Effect.Effect<A, E, R>,
+) =>
+  Effect.acquireUseRelease(
+    Effect.sync(() => Bun.serve({ port: 0, fetch })),
+    (server) => fn(server.url.toString()),
+    (server) => Effect.sync(() => server.stop(true)),
+  )
+
+describe("SettingsHook http handler", () => {
+  it.instance("applies configured entry.headers to the outbound POST", () =>
+    Effect.gen(function* () {
+      const sessionHooks = yield* SessionHooks.Service
+      const hook = yield* SettingsHook.Service
+      const sessionID = SessionID.descending()
+      let seen: Headers | undefined
+      yield* withFetch(
+        (req) => {
+          seen = req.headers
+          return new Response("{}", { status: 200, headers: { "content-type": "application/json" } })
+        },
+        (url) =>
+          Effect.gen(function* () {
+            yield* sessionHooks.add(sessionID, {
+              event: "UserPromptSubmit",
+              hooks: [
+                {
+                  type: "http",
+                  url,
+                  headers: { authorization: "Bearer hook-secret", "x-hook-test": "present" },
+                },
+              ],
+            })
+            const r = yield* hook.trigger(
+              { event: "UserPromptSubmit", prompt: "hi" },
+              { sessionID, transcriptPath: "" },
+            )
+            expect(r.blocked).toBeUndefined()
+            expect(seen).toBeDefined()
+            expect(seen?.get("authorization")).toBe("Bearer hook-secret")
+            expect(seen?.get("x-hook-test")).toBe("present")
+          }),
+      )
+    }),
+  )
+
+  it.instance("non-2xx response surfaces as exitBlock", () =>
+    Effect.gen(function* () {
+      const sessionHooks = yield* SessionHooks.Service
+      const hook = yield* SettingsHook.Service
+      const sessionID = SessionID.descending()
+      yield* withFetch(
+        () => new Response("nope", { status: 500 }),
+        (url) =>
+          Effect.gen(function* () {
+            yield* sessionHooks.add(sessionID, {
+              event: "UserPromptSubmit",
+              hooks: [{ type: "http", url }],
+            })
+            const r = yield* hook.trigger(
+              { event: "UserPromptSubmit", prompt: "hi" },
+              { sessionID, transcriptPath: "" },
+            )
+            expect(r.blocked).toBeDefined()
+            expect(r.blocked?.reason).toContain("500")
+          }),
+      )
+    }),
+  )
+})

packages/opencode/test/server/httpapi-exercise/index.ts

@@ -1237,6 +1237,24 @@ const scenarios: Scenario[] = [
       body: { event: "NotAnEvent", hooks: [{ type: "command", command: "printf '%s' 'x'" }] },
     }))
     .status(400),
+  http.protected
+    .post("/session/{sessionID}/hook", "session.hook.add.empty_command")
+    .seeded((ctx) => ctx.session({ title: "Hook empty command session" }))
+    .at((ctx) => ({
+      path: route("/session/{sessionID}/hook", { sessionID: ctx.state.id }),
+      headers: ctx.headers(),
+      body: { event: "UserPromptSubmit", hooks: [{ type: "command", command: "" }] },
+    }))
+    .status(400),
+  http.protected
+    .post("/session/{sessionID}/hook", "session.hook.add.non_positive_timeout")
+    .seeded((ctx) => ctx.session({ title: "Hook bad timeout session" }))
+    .at((ctx) => ({
+      path: route("/session/{sessionID}/hook", { sessionID: ctx.state.id }),
+      headers: ctx.headers(),
+      body: { event: "UserPromptSubmit", hooks: [{ type: "command", command: "true", timeout: 0 }] },
+    }))
+    .status(400),
   http.protected
     .get("/session/{sessionID}/hook", "session.hook.list")
     .seeded((ctx) => ctx.session({ title: "Hook list session" }))

packages/opencode/test/server/session-hooks-api.test.ts

@@ -0,0 +1,74 @@
+import { afterEach, describe, expect } from "bun:test"
+import { Effect, Layer } from "effect"
+import { Session } from "@/session/session"
+import { disposeAllInstances, TestInstance } from "../fixture/fixture"
+import { testEffect } from "../lib/effect"
+import { httpApiLayer, requestInDirectory } from "./httpapi-layer"
+
+const it = testEffect(Layer.mergeAll(Session.defaultLayer, httpApiLayer))
+
+afterEach(() => disposeAllInstances())
+
+function addHook(directory: string, sessionID: string, hook: Record<string, unknown>) {
+  return requestInDirectory(`/session/${sessionID}/hook`, directory, {
+    method: "POST",
+    headers: { "Content-Type": "application/json" },
+    body: JSON.stringify({ event: "UserPromptSubmit", hooks: [hook] }),
+  })
+}
+
+describe("session hook add validation", () => {
+  it.instance(
+    "rejects command-type hooks with a missing or blank command",
+    () =>
+      Effect.gen(function* () {
+        const test = yield* TestInstance
+        const session = yield* Session.use.create({})
+
+        const missing = yield* addHook(test.directory, session.id, { type: "command" })
+        expect(missing.status).toBe(400)
+
+        const blank = yield* addHook(test.directory, session.id, { type: "command", command: "   " })
+        expect(blank.status).toBe(400)
+      }),
+    { git: true },
+  )
+
+  it.instance(
+    "rejects non-positive timeout",
+    () =>
+      Effect.gen(function* () {
+        const test = yield* TestInstance
+        const session = yield* Session.use.create({})
+
+        const zero = yield* addHook(test.directory, session.id, { type: "command", command: "true", timeout: 0 })
+        expect(zero.status).toBe(400)
+
+        const negative = yield* addHook(test.directory, session.id, { type: "command", command: "true", timeout: -5 })
+        expect(negative.status).toBe(400)
+      }),
+    { git: true },
+  )
+
+  it.instance(
+    "accepts valid command and http hooks",
+    () =>
+      Effect.gen(function* () {
+        const test = yield* TestInstance
+        const session = yield* Session.use.create({})
+
+        const command = yield* addHook(test.directory, session.id, { type: "command", command: "true" })
+        expect(command.status).toBe(200)
+        expect(typeof ((yield* command.json) as { id: string }).id).toBe("string")
+
+        const http = yield* addHook(test.directory, session.id, {
+          type: "http",
+          url: "https://hooks.example.com/endpoint",
+          timeout: 30,
+          headers: { authorization: "Bearer token" },
+        })
+        expect(http.status).toBe(200)
+      }),
+    { git: true },
+  )
+})