Skip to content
Merged
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
7 changes: 6 additions & 1 deletion src/github/webhook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,20 @@ export async function enqueueWebhookByEnv(env: Env, deliveryId: string, eventNam
return "review_unavailable";
}

// #zero-trace-webhook-loss: hash the raw body (independent of whether it parses) BEFORE the parse attempt, so
// an unparseable delivery can still be durably recorded below instead of vanishing with no row anywhere.
const payloadHash = await sha256Hex(rawBody);
let payload: GitHubWebhookPayload;
try {
payload = JSON.parse(rawBody) as GitHubWebhookPayload;
} catch {
// installation/repository/action are unknown pre-parse; deliveryId + eventName + the hash are enough for an
// operator to trace this delivery instead of it being indistinguishable from "GitHub never sent it."
await recordWebhookEvent(env, { deliveryId, eventName, payloadHash, status: "error", errorSummary: "invalid_json" });
recordWebhookEnqueueMetric(eventName, undefined, "invalid_json");
return "invalid_json";
}

const payloadHash = await sha256Hex(rawBody);
const existingEvent = await getWebhookEvent(env, deliveryId);
// Suppress redelivery of an already-processed event (on success its payloadHash is overwritten to a
// "processed" sentinel, so a hash match alone misses it and the event re-runs its side effects) or one
Expand Down
15 changes: 15 additions & 0 deletions src/orb/broker-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
// The signal is the ENROLLMENT SECRET's presence: a brokered self-host sets ORB_ENROLLMENT_SECRET (issued by the
// operator), cloud never does — so this path is inert on cloud and the deploy is byte-identical there.

import { incr } from "../selfhost/metrics";

/** The Orb's hosted broker base; override (ORB_BROKER_URL) only to point at a private gittensory deployment. */
const DEFAULT_BROKER_URL = "https://gittensory-api.aethereal.dev";
// The broker's cold token mint can take many seconds when GitHub is throttling the App; allow headroom so the one
Expand Down Expand Up @@ -256,7 +258,20 @@ export async function drainOrbRelay(
for (const e of body.events ?? []) {
if (typeof e.deliveryId === "string" && typeof e.eventName === "string" && typeof e.rawBody === "string") {
out.push({ deliveryId: e.deliveryId, eventName: e.eventName, rawBody: e.rawBody });
continue;
}
// #zero-trace-webhook-loss: a batch entry missing/mistyping one of the three required fields was
// previously discarded with no record anywhere — indistinguishable from the Orb never having relayed it.
incr("gittensory_orb_relay_malformed_events_total");
console.error(
JSON.stringify({
level: "error",
event: "orb_relay_malformed_event_dropped",
hasDeliveryId: typeof e.deliveryId === "string",
hasEventName: typeof e.eventName === "string",
hasRawBody: typeof e.rawBody === "string",
}),
);
}
return out;
} catch (error) {
Expand Down
12 changes: 10 additions & 2 deletions test/unit/orb-broker-client.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { describe, expect, it } from "vitest";
import { describe, expect, it, vi } from "vitest";
import { counterValue, resetMetrics } from "../../src/selfhost/metrics";
import {
createOrbRelayRegistrationState,
drainOrbRelay,
Expand Down Expand Up @@ -362,7 +363,9 @@ describe("drainOrbRelay (pull-mode drain)", () => {
expect(await drainOrbRelay({})).toEqual([]);
});

it("POSTs the ack list, parses returned events, and filters malformed ones", async () => {
it("POSTs the ack list, parses returned events, and filters malformed ones (#zero-trace-webhook-loss: logs + counts the drop)", async () => {
resetMetrics();
const errors = vi.spyOn(console, "error").mockImplementation(() => undefined);
const { fetchImpl, calls } = captureFetch(
Response.json({
events: [
Expand All @@ -380,6 +383,11 @@ describe("drainOrbRelay (pull-mode drain)", () => {
expect(calls[0]?.url).toBe("https://gittensory-api.aethereal.dev/v1/orb/relay/pull");
expect((calls[0]?.init?.headers as Record<string, string>).authorization).toBe("Bearer s");
expect(JSON.parse(String(calls[0]?.init?.body))).toEqual({ ack: ["prev-1"] });
expect(counterValue("gittensory_orb_relay_malformed_events_total")).toBe(1);
const logged = errors.mock.calls.map((c) => String(c[0])).find((line) => line.includes("orb_relay_malformed_event_dropped"));
expect(logged).toBeDefined();
expect(JSON.parse(logged!)).toMatchObject({ level: "error", event: "orb_relay_malformed_event_dropped", hasDeliveryId: true, hasEventName: true, hasRawBody: false });
errors.mockRestore();
});

it("tolerates a missing events array (?? [] arm)", async () => {
Expand Down
11 changes: 11 additions & 0 deletions test/unit/webhook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,17 @@ describe("github webhook queue isolation (#audit-webhook-queue)", () => {
expect(metrics).toContain('gittensory_webhook_enqueue_total{action="other",event="other",result="queued"} 1');
});

it("REGRESSION (#zero-trace-webhook-loss): an unparseable delivery still gets a durable webhook_events row instead of vanishing with no trace", async () => {
const env = createTestEnv();
env.WEBHOOKS = { send: async () => undefined } as unknown as Queue;

await expect(enqueueWebhookByEnv(env, "invalid-json-trace", "pull_request", "{not json")).resolves.toBe("invalid_json");

const event = await getWebhookEvent(env, "invalid-json-trace");
expect(event).toMatchObject({ deliveryId: "invalid-json-trace", status: "error" });
expect(event?.payloadHash).toBeTruthy();
});

it("rejects retired direct review-app webhooks when the self-host review runtime is absent", async () => {
const env = createTestEnv();
delete env.SELFHOST_TRANSIENT_CACHE;
Expand Down