Skip to content

Commit 87fb944

Browse files
fix(orb): reject a malformed health field even when real events are present (#7213)
handleOrbIngest only rejected a malformed `health` object when `events` was also empty -- a payload carrying both real outcome events AND a malformed health field had the malformed health silently treated as absent instead, masking a sender-side bug instead of surfacing it. Inconsistent with how every other field in this handler is validated (reject on malformed, never silently coerce). A `health` key that IS present must now always be well-formed, regardless of whether events is also non-empty. An absent health key (an older self-host build that doesn't send this field yet) is unaffected. Caught in review of #4933/PR #7208, after merge. Closes #7210 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 318ada3 commit 87fb944

2 files changed

Lines changed: 26 additions & 5 deletions

File tree

src/orb/ingest.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,21 @@ export async function handleOrbIngest(body: string, db: D1Database): Promise<Orb
9393
}
9494

9595
const { instance_id, events, health } = payload as OrbIngestPayload;
96-
// #4933: an empty batch is only valid when it's carrying a health-only ping (the hourly export still
97-
// has to report health even in a tick with nothing new to export) -- a truly empty, health-less payload
98-
// stays rejected exactly as before.
99-
const healthy = typeof health === "object" && health !== null && typeof health.ok === "boolean" ? (health.ok ? 1 : 0) : null;
96+
// #4933: a `health` key that IS present must be well-formed, whether or not `events` also carries real
97+
// outcome rows -- rejecting only when events is also empty would silently drop a malformed health report
98+
// from a sender that also has real events to export, instead of surfacing the sender's bug. An ABSENT
99+
// health key (an older self-host build that doesn't send this field yet) is fine and falls through as
100+
// healthy = null, exactly as before this field existed.
101+
let healthy: number | null = null;
102+
if (health !== undefined) {
103+
if (typeof health !== "object" || health === null || typeof health.ok !== "boolean") {
104+
return { error: "invalid_payload" };
105+
}
106+
healthy = health.ok ? 1 : 0;
107+
}
108+
// An empty batch is only valid when it's carrying a (well-formed) health-only ping (the hourly export
109+
// still has to report health even in a tick with nothing new to export) -- a truly empty, health-less
110+
// payload stays rejected exactly as before #4933.
100111
if (!instance_id || instance_id.length > MAX_INSTANCE_ID_CHARS || (events.length === 0 && healthy === null)) {
101112
return { error: "invalid_payload" };
102113
}

test/integration/orb-ingest.test.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,13 +181,23 @@ describe("handleOrbIngest() health ping (#4933)", () => {
181181
expect((await instanceRow(db, "h2"))?.healthy).toBe(0);
182182
});
183183

184-
it("a malformed health object (not an object / ok not boolean) is treated as absent", async () => {
184+
it("a malformed health object (not an object / ok not boolean) is rejected", async () => {
185185
const db = makeDb();
186186
expect(await handleOrbIngest(JSON.stringify({ instance_id: "h3", events: [], health: "bad" }), db)).toEqual({ error: "invalid_payload" });
187187
expect(await handleOrbIngest(JSON.stringify({ instance_id: "h4", events: [], health: { ok: "yes" } }), db)).toEqual({ error: "invalid_payload" });
188188
expect(await handleOrbIngest(JSON.stringify({ instance_id: "h5", events: [], health: null }), db)).toEqual({ error: "invalid_payload" });
189189
});
190190

191+
it("REGRESSION: a malformed health object is rejected even when real outcome events are ALSO present -- a present-but-invalid health key must never be silently dropped just because there's other work to do", async () => {
192+
const db = makeDb();
193+
const malformed = { instance_id: "h3b", events: [ev({ pr_hash: "h3b-p" })], health: { ok: "yes" } };
194+
expect(await handleOrbIngest(JSON.stringify(malformed), db)).toEqual({ error: "invalid_payload" });
195+
// Confirms the whole payload was rejected -- neither the event nor any instance row was persisted.
196+
expect(await instanceRow(db, "h3b")).toBeNull();
197+
const signalCount = await (db as unknown as TestD1Database).prepare("SELECT COUNT(*) AS n FROM orb_signals WHERE pr_hash='h3b-p'").first<{ n: number }>();
198+
expect(signalCount?.n).toBe(0);
199+
});
200+
191201
it("an outcome-only ingest (no health field) never overwrites a previously-reported health status", async () => {
192202
const db = makeDb();
193203
await handleOrbIngest(JSON.stringify({ instance_id: "h6", events: [], health: { ok: true } }), db);

0 commit comments

Comments
 (0)