fix(notifications): keep the egress mark from clobbering a set action

The mark named its action unconditionally, so a job that had already
named its own (the low-stock underflow branch) or a settings route
whose test then failed at the relay lost that name on the wide event.
Set the action only when the event has none, and always land the
channel and origin meta pair, through a small hasAction() on the
builder. Note the parse-time floor on the Nightscout page, which shares
the grammar.

Refs #947
This commit is contained in:
Marc-André Bombeck committed 2026-09-10 16:51:57 +02:00
1 parent 9630b59973
commit 761a5a54c1
7 files changed
+93 -15

No files matched your search

+10
View File
@@ -150,6 +150,16 @@ export class WideEventBuilder {
return this;
}
/**
* Whether a caller has already named this event's action. A leaf that
* wants to mark the event (the private-origin egress mark in the
* notification senders) checks this first so it never overwrites the
* route's or job's own action name, which dashboards pin on.
*/
hasAction(): boolean {
return this.event.action !== undefined;
}
setBackground(bg: WideEvent["background"]): this {
this.event.background = bg;
return this;
@@ -12,7 +12,11 @@
*/
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { eventStorage } from "@/lib/logging/context";
import { WideEventBuilder } from "@/lib/logging/event-builder";
import {
annotatePrivateOriginEgress,
configuredNotificationPrivateOrigins,
evaluateNotificationTarget,
isAllowedNotificationTarget,
@@ -166,3 +170,42 @@ describe("evaluateNotificationTarget", () => {
});
});
});
describe("annotatePrivateOriginEgress", () => {
it("names the action when the event has none and pins the meta pair", () => {
const event = new WideEventBuilder("background");
eventStorage.run(event, () => {
annotatePrivateOriginEgress("webhook", "https://gotify.example.com");
});
const json = event.toJSON();
expect(json.action).toEqual({ name: "notification.egress.private_origin" });
expect(json.meta).toEqual({
channel: "webhook",
origin: "https://gotify.example.com",
});
});
it("keeps an action the route or job already set, and still lands the meta", () => {
// A settings route or a job names its own action; dashboards pin on it.
// The egress mark must not overwrite that name.
const event = new WideEventBuilder("http");
event.setAction({ name: "settings.webhook.test" });
eventStorage.run(event, () => {
annotatePrivateOriginEgress("ntfy", "http://ntfy.lan:8080");
});
const json = event.toJSON();
expect(json.action).toEqual({ name: "settings.webhook.test" });
expect(json.meta).toEqual({
channel: "ntfy",
origin: "http://ntfy.lan:8080",
});
});
it("is a no-op without an active event", () => {
expect(() =>
annotatePrivateOriginEgress("webhook", "https://gotify.example.com"),
).not.toThrow();
});
});
+11 -5
View File
@@ -6,7 +6,7 @@ import {
PRIVATE_ORIGIN_NOT_APPROVED,
type OriginVerdict,
} from "@/lib/private-origin-policy";
import { annotate } from "@/lib/logging/context";
import { annotate, getEvent } from "@/lib/logging/context";
/**
* `NOTIFICATION_PRIVATE_ORIGINS` (#947) — the operator grant that lets the
@@ -93,13 +93,19 @@ export { PRIVATE_ORIGIN_NOT_APPROVED };
* an operator can see which grant is actually in use. The origin is the
* canonical grant string, never the full target URL, which may carry a
* token in its path or query.
*
* The action name is set only when the event has none yet: a settings route
* or a job that already named its action keeps it, and the meta pair still
* lands on the same event, so the mark is never lost and never clobbers.
*/
export function annotatePrivateOriginEgress(
channel: "webhook" | "ntfy",
origin: string,
): void {
annotate({
action: { name: "notification.egress.private_origin" },
meta: { channel, origin },
});
const event = getEvent();
if (!event) return;
if (!event.hasAction()) {
annotate({ action: { name: "notification.egress.private_origin" } });
}
annotate({ meta: { channel, origin } });
}
@@ -25,7 +25,11 @@ vi.mock("@/lib/safe-fetch", () => ({
}));
vi.mock("@/lib/logging/context", () => ({
getEvent: () => ({ addWarning: vi.fn(), addExternalCall: vi.fn() }),
getEvent: () => ({
addWarning: vi.fn(),
addExternalCall: vi.fn(),
hasAction: () => false,
}),
annotate: (...args: unknown[]) => annotateMock(...args),
}));
@@ -165,6 +169,8 @@ describe("sendViaNtfy", () => {
expect(opts.operatorApprovedPrivateOrigin).toBe("http://ntfy.lan:8080");
expect(annotateMock).toHaveBeenCalledWith({
action: { name: "notification.egress.private_origin" },
});
expect(annotateMock).toHaveBeenCalledWith({
meta: { channel: "ntfy", origin: "http://ntfy.lan:8080" },
});
});
@@ -26,7 +26,11 @@ vi.mock("@/lib/safe-fetch", () => ({
const annotateMock = vi.fn();
vi.mock("@/lib/logging/context", () => ({
getEvent: () => ({ addWarning: vi.fn(), addExternalCall: vi.fn() }),
getEvent: () => ({
addWarning: vi.fn(),
addExternalCall: vi.fn(),
hasAction: () => false,
}),
annotate: (...args: unknown[]) => annotateMock(...args),
}));
@@ -198,6 +202,8 @@ describe("sendViaWebhook", () => {
);
expect(annotateMock).toHaveBeenCalledWith({
action: { name: "notification.egress.private_origin" },
});
expect(annotateMock).toHaveBeenCalledWith({
meta: { channel: "webhook", origin: "https://gotify.example.com" },
});
});