Feat/websub hub

[andrewshell]

Summary by CodeRabbit

Release Notes

• New Features

  • Added WebSub protocol support enabling callback-based content subscriptions
  • Implemented async intent verification with immediate 202 Accepted responses
  • Introduced configurable subscription lease management
  • Added HMAC-SHA256 signed content delivery for authenticated subscribers
  • Cross-protocol fan-out: single publisher ping now reaches both WebSub and rssCloud subscribers

• Documentation

  • Added architectural decision record for async verification design

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b38d2c0a-5e16-4ea3-8766-04cbb202ed6a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• ✅ Review completed - (🔄 Check again to review again)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 11

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/server/config.js`:
- Around line 19-20: The WEBSUB_PATH configuration variable is being used
without normalization, which causes issues when the value lacks a leading slash
(e.g., setting it to 'websub' results in malformed URLs like
'http://host:portwebsub'). Normalize the WEBSUB_PATH value at its point of
assignment to ensure it always has a leading slash prefix, so that both the
route mounting at the webSubPath variable and the subsequent hubUrl composition
at line 36 produce correctly formatted paths and URLs regardless of how
WEBSUB_PATH is configured.

In `@CONTEXT.md`:
- Line 210: There is a blank line inside the blockquote (quoted example) at line
210 in CONTEXT.md that violates markdownlint rule MD028. Remove this internal
blank line to keep the blockquote contiguous, ensuring all lines in the
blockquote are consecutive with no empty lines between them.

In `@packages/core/src/config.test.ts`:
- Around line 12-15: The default-config test in config.test.ts is checking
literal values but not validating the ordering invariant required by lease
clamping logic. Add an assertion after the configuration section that verifies
the relationship webSubLeaseMinSecs <= webSubLeaseDefaultSecs <=
webSubLeaseMaxSecs holds true. This will catch future misconfigurations of these
three lease-related settings earlier during testing.

In `@packages/core/src/engine/create-core.test.ts`:
- Around line 994-1028: Add three new test cases to the lease suite after the
existing tests to cover edge cases with non-finite and fractional lease values.
Create tests that call subscribeWebSub with leaseSeconds values of NaN,
Infinity, and a fractional number (like 123.456), and verify that each case
safely normalizes to a valid integer value within the allowed range, ensuring
sub?.details contains a valid integer leaseSeconds and sub?.whenExpires is a
valid Date object.

In `@packages/core/src/engine/create-core.ts`:
- Around line 109-116: The clampLease function does not validate that the
requested lease value is finite before clamping it, allowing NaN and Infinity to
pass through since typeof NaN returns 'number'. This causes invalid dates to be
persisted at the point where whenExpires is set. Fix the type check in
clampLease to also verify that the requested value is finite using
Number.isFinite(), returning config.webSubLeaseDefaultSecs if the value is not
finite, ensuring only valid numeric values proceed to the Math.min and Math.max
clamping logic.

In `@packages/core/src/engine/verification-scheduler.ts`:
- Around line 33-35: The current implementation in the queueMicrotask callback
only handles promise rejections via .catch(), but does not handle synchronous
throws that might occur before the task returns a promise. Although async
functions convert synchronous throws to rejections, the interface accepts any ()
=> Promise<void>, so synchronous exceptions could be thrown. Wrap the task()
invocation in a try-catch block to catch both synchronous throws and promise
rejections, ensuring both error types are passed to options.onError to fully
honor the non-throwing contract of the schedule method.

In `@packages/core/src/protocols/websub-dispatcher.test.ts`:
- Around line 50-58: Following the pattern of the existing test that validates
hub.callback rejection for invalid URLs, add additional test cases in the
parseSubscribe test suite to verify that malformed hub.topic and hub.url
parameters are also properly rejected with a 400 status code. Create test cases
similar to the shown test but with invalid non-absolute URLs for the hub.topic
and hub.url fields to ensure the parser consistently enforces URL shape
validation across all URL parameters.

In `@packages/core/src/protocols/websub-dispatcher.ts`:
- Around line 51-53: The validation for hub.topic and hub.url (around lines
51-53 and 63-71 respectively) currently only checks if they are non-empty
strings, but does not validate that they are valid URLs. This allows malformed
WebSub requests to pass initial parsing with a 202 response and fail later
during async verification instead of returning a synchronous 400 error. Add URL
validation logic for both the topic variable (after the non-empty string check)
and the url variable to ensure they are properly formatted URLs before returning
null, rejecting invalid requests immediately at parse time.

In `@packages/core/src/protocols/websub-plugin.test.ts`:
- Around line 394-427: The websub-plugin test suite is missing regression tests
for two critical invariants in the deliver functionality. Add two new test cases
to the deliver test suite in websub-plugin.test.ts: first, create a test that
verifies the redirect-loop cap is enforced (that redirect following has a
bounded limit and fails or stops when exceeded), and second, create a test that
verifies explicit failure occurs when the hubUrl is missing from the plugin
configuration. Both tests should follow the pattern of the existing 'follows a
3xx redirect' test case by setting up a fakeFetch mock and asserting on the
result behavior.

In `@packages/core/src/protocols/websub-plugin.ts`:
- Around line 93-97: The Link header construction in the websub-plugin.ts
headers object interpolates hubUrl without validating it is defined, which
produces invalid metadata like <undefined> when hubUrl is missing. Add a guard
check before the headers object initialization to validate that hubUrl exists
and is not null or undefined, and either throw an error or return early if it is
missing, ensuring the delivery path only proceeds with valid hub metadata.
- Around line 119-123: The distribute function recursively follows HTTP
redirects without any depth limit, which can cause infinite loops if a callback
URL redirects to itself. Add a redirect depth parameter to the distribute
function signature to track the number of hops, then before the recursive
distribute call (where it handles the location header), check if the current
depth has exceeded a maximum limit (e.g., 5 hops). If the limit is reached, log
a warning or error and return early instead of making the recursive call. This
bounds the redirect chain and prevents indefinite delivery task execution.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: cd787834-1333-4827-a6b8-a72efcb01c3b

📥 Commits

Reviewing files that changed from the base of the PR and between 783dac5 and cd0acf6.

📒 Files selected for processing (24)

• CONTEXT.md
• TODO.md
• apps/e2e/test/mock.js
• apps/e2e/test/websub.js
• apps/server/config.js
• apps/server/controllers/index.js
• apps/server/core.js
• docs/adr/0002-websub-async-intent-verification-seam.md
• packages/core/src/config.test.ts
• packages/core/src/config.ts
• packages/core/src/engine/core.ts
• packages/core/src/engine/create-core.test.ts
• packages/core/src/engine/create-core.ts
• packages/core/src/engine/plugin.ts
• packages/core/src/engine/verification-scheduler.test.ts
• packages/core/src/engine/verification-scheduler.ts
• packages/core/src/index.ts
• packages/core/src/protocols/websub-dispatcher.test.ts
• packages/core/src/protocols/websub-dispatcher.ts
• packages/core/src/protocols/websub-plugin.test.ts
• packages/core/src/protocols/websub-plugin.ts
• packages/express/src/index.ts
• packages/express/src/websub-middleware.test.ts
• packages/express/src/websub-middleware.ts

💤 Files with no reviewable changes (1)

• TODO.md

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Normalize WEBSUB_PATH before exporting and composing hubUrl.

At Line 19, WEBSUB_PATH is used verbatim. If it is set to websub (no leading /), the route mount becomes non-standard and the default URL at Line 36 is malformed (http://host:portwebsub). Normalize once in config (prefix /) to keep routing and advertised hub URL consistent.

Proposed fix

-const webSubPath = getConfig('WEBSUB_PATH', '/websub');
+const rawWebSubPath = getConfig('WEBSUB_PATH', '/websub');
+const webSubPath = rawWebSubPath.startsWith('/')
+    ? rawWebSubPath
+    : `/${rawWebSubPath}`;

Also applies to: 35-37

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/server/config.js` around lines 19 - 20, The WEBSUB_PATH configuration
variable is being used without normalization, which causes issues when the value
lacks a leading slash (e.g., setting it to 'websub' results in malformed URLs
like 'http://host:portwebsub'). Normalize the WEBSUB_PATH value at its point of
assignment to ensure it always has a leading slash prefix, so that both the
route mounting at the webSubPath variable and the subsequent hubUrl composition
at line 36 produce correctly formatted paths and URLs regardless of how
WEBSUB_PATH is configured.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove the blank line in the quoted example.

markdownlint flags this as MD028, so the docs lint step may fail until the blockquote is kept contiguous.

🧰 Tools

🪛 markdownlint-cli2 (0.22.1)

[warning] 210-210: Blank line inside blockquote

(MD028, no-blanks-blockquote)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CONTEXT.md` at line 210, There is a blank line inside the blockquote (quoted
example) at line 210 in CONTEXT.md that violates markdownlint rule MD028. Remove
this internal blank line to keep the blockquote contiguous, ensuring all lines
in the blockquote are consecutive with no empty lines between them.

Source: Linters/SAST tools

[coderabbitai]

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add a lease-bound invariant assertion in the default-config test.

This test checks literals, but not the ordering invariant used by lease clamping. Add an assertion that webSubLeaseMinSecs <= webSubLeaseDefaultSecs <= webSubLeaseMaxSecs to catch future misconfiguration edits earlier.

Suggested patch

 it('returns the documented defaults when given nothing', () => {
-    expect(resolveConfig()).toEqual({
+    const resolved = resolveConfig();
+    expect(resolved).toEqual({
         minSecsBetweenPings: 0,
         ctSecsResourceExpire: 90000,
         maxConsecutiveErrors: 3,
         maxResourceSize: 256000,
         requestTimeoutMs: 4000,
         feedsChangedWindowDays: 7,
         webSubLeaseDefaultSecs: 86400,
         webSubLeaseMinSecs: 300,
         webSubLeaseMaxSecs: 864000
     });
+    expect(resolved.webSubLeaseMinSecs).toBeLessThanOrEqual(
+        resolved.webSubLeaseDefaultSecs
+    );
+    expect(resolved.webSubLeaseDefaultSecs).toBeLessThanOrEqual(
+        resolved.webSubLeaseMaxSecs
+    );
 });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/config.test.ts` around lines 12 - 15, The default-config
test in config.test.ts is checking literal values but not validating the
ordering invariant required by lease clamping logic. Add an assertion after the
configuration section that verifies the relationship webSubLeaseMinSecs <=
webSubLeaseDefaultSecs <= webSubLeaseMaxSecs holds true. This will catch future
misconfigurations of these three lease-related settings earlier during testing.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add lease-edge regression cases (NaN, Infinity, fractional`).

The lease suite should assert that non-finite/fractional inputs are normalized safely so whenExpires is always valid and deterministic.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/engine/create-core.test.ts` around lines 994 - 1028, Add
three new test cases to the lease suite after the existing tests to cover edge
cases with non-finite and fractional lease values. Create tests that call
subscribeWebSub with leaseSeconds values of NaN, Infinity, and a fractional
number (like 123.456), and verify that each case safely normalizes to a valid
integer value within the allowed range, ensuring sub?.details contains a valid
integer leaseSeconds and sub?.whenExpires is a valid Date object.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Sanitize non-finite lease values before clamping.

Line 110 treats NaN as numeric; clampLease can return NaN, and Line 404 then persists whenExpires as an invalid date. That breaks expiry behavior and can leave subscriptions effectively unbounded.

Suggested fix

     function clampLease(requested: unknown): number {
-        if (typeof requested !== 'number') {
+        if (typeof requested !== 'number' || !Number.isFinite(requested)) {
             return config.webSubLeaseDefaultSecs;
         }
+        const normalized = Math.trunc(requested);
         return Math.min(
             config.webSubLeaseMaxSecs,
-            Math.max(config.webSubLeaseMinSecs, requested)
+            Math.max(config.webSubLeaseMinSecs, normalized)
         );
     }

Also applies to: 404-407

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/engine/create-core.ts` around lines 109 - 116, The
clampLease function does not validate that the requested lease value is finite
before clamping it, allowing NaN and Infinity to pass through since typeof NaN
returns 'number'. This causes invalid dates to be persisted at the point where
whenExpires is set. Fix the type check in clampLease to also verify that the
requested value is finite using Number.isFinite(), returning
config.webSubLeaseDefaultSecs if the value is not finite, ensuring only valid
numeric values proceed to the Math.min and Math.max clamping logic.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add negative URL-shape tests for hub.topic / hub.url.

Current coverage does not assert rejection of malformed topic URLs (e.g., non-absolute values). Add these cases so parser behavior stays pinned to 400 for invalid URLs.

Suggested test additions

 describe('parseSubscribe', () => {
+    it('rejects a non-absolute hub.topic as a 400', () => {
+        const result = parseSubscribe({
+            'hub.mode': 'subscribe',
+            'hub.callback': 'https://sub.example.com/listener',
+            'hub.topic': 'not-a-url'
+        });
+        expect(result).toEqual({ ok: false, status: 400 });
+    });
@@
 describe('parsePublish', () => {
+    it('rejects a non-absolute hub.url as a 400', () => {
+        const result = parsePublish({
+            'hub.mode': 'publish',
+            'hub.url': 'not-a-url'
+        });
+        expect(result).toEqual({ ok: false, status: 400 });
+    });

Also applies to: 194-233

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/protocols/websub-dispatcher.test.ts` around lines 50 - 58,
Following the pattern of the existing test that validates hub.callback rejection
for invalid URLs, add additional test cases in the parseSubscribe test suite to
verify that malformed hub.topic and hub.url parameters are also properly
rejected with a 400 status code. Create test cases similar to the shown test but
with invalid non-absolute URLs for the hub.topic and hub.url fields to ensure
the parser consistently enforces URL shape validation across all URL parameters.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reject non-URL topics at parse time.

Line 52 and Lines 65-70 only enforce non-empty strings for hub.topic/hub.url. That can accept malformed WebSub requests with 202 and fail later in async verification/fetch instead of returning a synchronous 400.

Suggested fix

 function parseHubCallbackTopic(
     body: Record<string, unknown>
 ): { callback: string; topic: string } | null {
@@
     const topic = body['hub.topic'];
-    if (typeof topic !== 'string' || topic === '') {
+    if (
+        typeof topic !== 'string' ||
+        topic === '' ||
+        !isAbsoluteUrl(topic)
+    ) {
         return null;
     }
     return { callback, topic };
 }
@@
 function publishTopic(body: Record<string, unknown>): string | null {
     const url = body['hub.url'];
-    if (typeof url === 'string' && url !== '') {
+    if (typeof url === 'string' && url !== '' && isAbsoluteUrl(url)) {
         return url;
     }
     const topic = body['hub.topic'];
-    if (typeof topic === 'string' && topic !== '') {
+    if (typeof topic === 'string' && topic !== '' && isAbsoluteUrl(topic)) {
         return topic;
     }
     return null;
 }

Also applies to: 63-71

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/protocols/websub-dispatcher.ts` around lines 51 - 53, The
validation for hub.topic and hub.url (around lines 51-53 and 63-71 respectively)
currently only checks if they are non-empty strings, but does not validate that
they are valid URLs. This allows malformed WebSub requests to pass initial
parsing with a 202 response and fail later during async verification instead of
returning a synchronous 400 error. Add URL validation logic for both the topic
variable (after the non-empty string check) and the url variable to ensure they
are properly formatted URLs before returning null, rejecting invalid requests
immediately at parse time.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add regression tests for redirect-loop cap and missing hubUrl.

The deliver suite should pin both invariants: bounded redirect following and explicit failure when hubUrl is absent.

Also applies to: 532-550

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/protocols/websub-plugin.test.ts` around lines 394 - 427,
The websub-plugin test suite is missing regression tests for two critical
invariants in the deliver functionality. Add two new test cases to the deliver
test suite in websub-plugin.test.ts: first, create a test that verifies the
redirect-loop cap is enforced (that redirect following has a bounded limit and
fails or stops when exceeded), and second, create a test that verifies explicit
failure occurs when the hubUrl is missing from the plugin configuration. Both
tests should follow the pattern of the existing 'follows a 3xx redirect' test
case by setting up a fakeFetch mock and asserting on the result behavior.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail fast when hubUrl is missing instead of sending Link: <undefined>.

Line 96 interpolates hubUrl even when unset, producing invalid hub metadata and non-compliant deliveries. Guard this in delivery path.

Suggested fix

     async function distribute(
         targetUrl: string,
         ctx: DeliveryContext
     ): Promise<void> {
+        if (!hubUrl) {
+            throw new Error(
+                'WebSub hubUrl is required for content distribution'
+            );
+        }
         const headers: Record<string, string> = {
             'Content-Type':
                 ctx.payload.contentType ?? 'application/octet-stream',
             Link: `<${hubUrl}>; rel="hub", <${ctx.resource.url}>; rel="self"`
         };

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const headers: Record<string, string> = {
	'Content-Type':
	ctx.payload.contentType ?? 'application/octet-stream',
	Link: `<${hubUrl}>; rel="hub", <${ctx.resource.url}>; rel="self"`
	};
	async function distribute(
	targetUrl: string,
	ctx: DeliveryContext
	): Promise<void> {
	if (!hubUrl) {
	throw new Error(
	'WebSub hubUrl is required for content distribution'
	);
	}
	const headers: Record<string, string> = {
	'Content-Type':
	ctx.payload.contentType ?? 'application/octet-stream',
	Link: `<${hubUrl}>; rel="hub", <${ctx.resource.url}>; rel="self"`
	};
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/protocols/websub-plugin.ts` around lines 93 - 97, The Link
header construction in the websub-plugin.ts headers object interpolates hubUrl
without validating it is defined, which produces invalid metadata like
<undefined> when hubUrl is missing. Add a guard check before the headers object
initialization to validate that hubUrl exists and is not null or undefined, and
either throw an error or return early if it is missing, ensuring the delivery
path only proceeds with valid hub metadata.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Bound redirect depth to avoid non-terminating delivery chains.

Line 122 recursively follows redirects with no hop limit. A self-redirecting callback can keep a delivery task running indefinitely and stall fan-out completion.

Suggested fix

+const MAX_REDIRECT_HOPS = 5;
@@
-    async function distribute(
-        targetUrl: string,
-        ctx: DeliveryContext
-    ): Promise<void> {
+    async function distribute(
+        targetUrl: string,
+        ctx: DeliveryContext,
+        redirectHops = 0
+    ): Promise<void> {
@@
         if (res.status >= 300 && res.status < 400) {
             const location = res.headers.get('location');
             if (location) {
-                await distribute(new URL(location, targetUrl).toString(), ctx);
+                if (redirectHops >= MAX_REDIRECT_HOPS) {
+                    throw new Error('WebSub content distribution failed: too many redirects');
+                }
+                await distribute(
+                    new URL(location, targetUrl).toString(),
+                    ctx,
+                    redirectHops + 1
+                );
                 return;
             }
         }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/protocols/websub-plugin.ts` around lines 119 - 123, The
distribute function recursively follows HTTP redirects without any depth limit,
which can cause infinite loops if a callback URL redirects to itself. Add a
redirect depth parameter to the distribute function signature to track the
number of hops, then before the recursive distribute call (where it handles the
location header), check if the current depth has exceeded a maximum limit (e.g.,
5 hops). If the limit is reached, log a warning or error and return early
instead of making the recursive call. This bounds the redirect chain and
prevents indefinite delivery task execution.