From 82355138228f93b5c47516f8cc40e31c8a25840f Mon Sep 17 00:00:00 2001 From: hudson Date: Wed, 16 Sep 2026 15:15:26 -0400 Subject: [PATCH 01/10] Set trust proxy so NIP-98 URL matching works behind traefik (X-Forwarded-Proto) --- src/server.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/server.ts b/src/server.ts index 9e7aea1..c7acb43 100644 --- a/src/server.ts +++ b/src/server.ts @@ -83,6 +83,13 @@ const verifyNip98Auth = async (req: Request): Promise => { export const server: express.Application = express() +// Behind a TLS-terminating reverse proxy (traefik in the ansible deploy). +// Without this, req.protocol stays "http" and verifyNip98Auth builds an +// expectedUrl of http://…, which no browser client will ever sign (clients +// sign https://…). Trust one proxy hop so req.protocol honors +// X-Forwarded-Proto and NIP-98 URL matching works. +server.set('trust proxy', 1) + // CORS middleware for browser-facing routes only. // The browser hits /subscription with an Authorization header and Content-Type: // application/json, which triggers a CORS preflight. Answer it and allow the From 9fa1f73316b64f447a263ca609c55844f15db9a9 Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 17 Sep 2026 16:57:30 -0400 Subject: [PATCH 02/10] fix: distinguish already-confirmed tokens from invalid ones; prevent duplicate active subscriptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem ------- 1. Re-clicking an already-confirmed confirmation link (e.g. /confirm?token=…) returned from confirmSubscription because the SQL WHERE clause required . The caller then threw an ActionError('invalid or expired') which rendered the 'Email not confirmed' error page — misleading for someone who had already confirmed. 2. A second PUT /subscription/email with the same email+frequency could silently bypass the upsert path when getSubscriptionByPubkey found the active row but updateSubscription returned it unchanged (email and frequency matched). While the unique index prevented a true duplicate INSERT, the code path was fragile and the regression test was missing. Changes ------- database.ts: - confirmSubscription now returns { sub, alreadyConfirmed } | undefined. First it tries the existing UPDATE (unconfirmed tokens only). If that returns no rows, it looks up the key directly: if the row exists and is already confirmed, returns { sub, alreadyConfirmed: true }. If the row doesn't exist or is unsubscribed, returns undefined (invalid/expired). - Exported new ConfirmResult type for callers. actions.ts: - confirmSubscriptionAction destructures the new return type. - Only registers the cron job on fresh confirmation (not re-confirms). - Returns the ConfirmResult so the route can distinguish the two cases. server.ts: - /confirm route checks result.alreadyConfirmed and renders confirm-already.html instead of confirm-success.html. pages/confirm-already.html: - New page with title 'Email already confirmed' and an info message explaining the address was already confirmed. Tests: - test/confirm-already-confirmed.test.ts — NEW (3 tests): first confirm succeeds with alreadyConfirmed=false; second confirm returns alreadyConfirmed=true; nonexistent token returns undefined. - test/duplicate-subscription.test.ts — NEW (4 tests): full cycle of register → confirm → re-register → assert one active row with unchanged key, verifying the upsert is idempotent. - Adapted 3 existing test files to destructure the new ConfirmResult. --- src/actions.ts | 12 +++- src/database.ts | 25 ++++++++- src/pages/confirm-already.html | 62 +++++++++++++++++++++ src/server.ts | 8 ++- test/confirm-already-confirmed.test.ts | 44 +++++++++++++++ test/duplicate-subscription.test.ts | 50 +++++++++++++++++ test/event-arrival-race.test.ts | 2 +- test/notify-response-shape.test.ts | 2 +- test/reschedule-on-frequency-change.test.ts | 2 +- 9 files changed, 198 insertions(+), 9 deletions(-) create mode 100644 src/pages/confirm-already.html create mode 100644 test/confirm-already-confirmed.test.ts create mode 100644 test/duplicate-subscription.test.ts diff --git a/src/actions.ts b/src/actions.ts index 975accb..3235afd 100644 --- a/src/actions.ts +++ b/src/actions.ts @@ -41,13 +41,19 @@ export type ConfirmSubscriptionParams = { export const confirmSubscriptionAction = instrument( 'actions.confirmSubscription', async ({ token }: ConfirmSubscriptionParams) => { - const sub = await db.confirmSubscription(token) + const result = await db.confirmSubscription(token) - if (!sub) { + if (!result) { throw new ActionError('That confirmation code is invalid or has expired.') } - worker.registerSubscription(sub) + // Only register the cron job when this is a fresh confirmation. + // If already confirmed, the job is already running. + if (!result.alreadyConfirmed) { + worker.registerSubscription(result.sub) + } + + return result }, ) diff --git a/src/database.ts b/src/database.ts index 718e225..25d70d1 100644 --- a/src/database.ts +++ b/src/database.ts @@ -195,16 +195,37 @@ export const insertSubscription = instrument( } ) +export type ConfirmResult = { + sub: Subscription + alreadyConfirmed: boolean +} + export const confirmSubscription = instrument( 'database.confirmSubscription', - async (key: string) => { - return parseSubscription( + async (key: string): Promise => { + // Try to update an unconfirmed, active row + const updated = parseSubscription( await get( `UPDATE subscriptions SET confirmed_at = unixepoch() WHERE key = ? AND confirmed_at IS NULL AND unsubscribed_at IS NULL RETURNING *`, [key] ) ) + + if (updated) { + return { sub: updated, alreadyConfirmed: false } + } + + // No unconfirmed row was updated. Check if the key exists and is + // already confirmed — the user is re-clicking a used link. If the row + // is unsubscribed, treat it as invalid (expired). + const existing = await getSubscriptionByKey(key) + + if (!existing || existing.unsubscribed_at) { + return undefined + } + + return { sub: existing, alreadyConfirmed: true } } ) diff --git a/src/pages/confirm-already.html b/src/pages/confirm-already.html new file mode 100644 index 0000000..e511e1c --- /dev/null +++ b/src/pages/confirm-already.html @@ -0,0 +1,62 @@ + + + + + + Email Already Confirmed + + + +
+ {{#brandLogo}}{{/brandLogo}} +
{{brandName}}
+

Email already confirmed

+

+ This email address has already been confirmed. You're all set — no further action needed. + Visit {{brandName}} to manage your notification settings. +

+
+ + \ No newline at end of file diff --git a/src/server.ts b/src/server.ts index c7acb43..54b30ad 100644 --- a/src/server.ts +++ b/src/server.ts @@ -327,7 +327,13 @@ addRoute('get', '/confirm', async (req: Request, res: Response) => { } try { - await confirmSubscriptionAction({ token: req.query.token }) + const result = await confirmSubscriptionAction({ token: req.query.token }) + + if (result.alreadyConfirmed) { + return res.send(await render('pages/confirm-already.html', { + ...brandingVars(), + })) + } res.send(await render('pages/confirm-success.html', { ...brandingVars(), diff --git a/test/confirm-already-confirmed.test.ts b/test/confirm-already-confirmed.test.ts new file mode 100644 index 0000000..2d3cde3 --- /dev/null +++ b/test/confirm-already-confirmed.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect, beforeAll } from 'vitest' +import * as db from '../src/database.js' + +const pubkey = 'reconfirm-test-' + Date.now() +const email = 'reconfirm-test-' + Date.now() + '@example.com' +let token: string + +describe('Confirm link idempotency — already-confirmed token', () => { + beforeAll(async () => { + await db.migrate() + }) + + it('creates and confirms a subscription for the first time', async () => { + const sub = await db.insertSubscription(pubkey, email, 'daily') + expect(sub).toBeTruthy() + token = sub.key + + const result = await db.confirmSubscription(token) + expect(result).toBeTruthy() + expect(result!.sub.confirmed_at).toBeTruthy() + expect(result!.alreadyConfirmed).toBe(false) + }) + + it('returns a distinct result (not undefined) when confirming an already-confirmed token', async () => { + // BUG: confirmSubscription used `WHERE confirmed_at IS NULL`, so + // re-confirming an already-confirmed token matched zero rows and + // the UPDATE returned nothing → parseSubscription returned undefined. + // The caller then threw ActionError('invalid or expired') and the + // user saw "Email not confirmed" — which is misleading. + // + // FIX: confirmSubscription now detects the already-confirmed case + // and returns { sub, alreadyConfirmed: true } so the handler can + // render an "already confirmed" info page instead of an error page. + const result = await db.confirmSubscription(token) + expect(result).not.toBeUndefined() + expect(result!.alreadyConfirmed).toBe(true) + expect(result!.sub.confirmed_at).toBeTruthy() + }) + + it('returns undefined for a nonexistent token', async () => { + const result = await db.confirmSubscription('nonexistent-token-' + Date.now()) + expect(result).toBeUndefined() + }) +}) \ No newline at end of file diff --git a/test/duplicate-subscription.test.ts b/test/duplicate-subscription.test.ts new file mode 100644 index 0000000..e7b3371 --- /dev/null +++ b/test/duplicate-subscription.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect, beforeAll } from 'vitest' +import * as db from '../src/database.js' + +const pubkey = 'dup-test-' + Date.now() +const email = 'dup-test-' + Date.now() + '@example.com' +let token: string + +describe('Duplicate subscription prevention — idempotent re-register after confirm', () => { + beforeAll(async () => { + await db.migrate() + }) + + it('registers a subscription (fresh)', async () => { + const sub = await db.insertSubscription(pubkey, email, 'daily') + expect(sub).toBeTruthy() + expect(sub!.confirmed_at).toBeFalsy() + expect(sub!.unsubscribed_at).toBeFalsy() + token = sub!.key + }) + + it('confirms the subscription', async () => { + const result = await db.confirmSubscription(token) + expect(result).toBeTruthy() + expect(result!.alreadyConfirmed).toBe(false) + expect(result!.sub.confirmed_at).toBeTruthy() + }) + + it('re-registers with the same email and frequency (idempotent PUT)', async () => { + // This simulates a second PUT from the client with identical params. + // The bug would create a second active row + send a new confirmation email. + const sub = await db.insertSubscription(pubkey, email, 'daily') + expect(sub).toBeTruthy() + // Must return the existing confirmed row, NOT a fresh unconfirmed row + expect(sub!.confirmed_at).toBeTruthy() + expect(sub!.unsubscribed_at).toBeFalsy() + + // The key must remain unchanged — a new row would have a different key + expect(sub!.key).toBe(token) + }) + + it('has exactly one active row for this pubkey', async () => { + // getSubscriptionByPubkey filters by unsubscribed_at IS NULL and + // returns at most one row (enforced by the partial unique index). + // If a second active row exists, the index is absent or bypassed. + const active = await db.getSubscriptionByPubkey(pubkey) + expect(active).toBeTruthy() + expect(active!.confirmed_at).toBeTruthy() + expect(active!.key).toBe(token) + }) +}) \ No newline at end of file diff --git a/test/event-arrival-race.test.ts b/test/event-arrival-race.test.ts index 93e5c44..b24175a 100644 --- a/test/event-arrival-race.test.ts +++ b/test/event-arrival-race.test.ts @@ -24,7 +24,7 @@ describe('Event-arrival race in digest job', () => { expect(s).toBeTruthy() const confirmed = await db.confirmSubscription(s.key) expect(confirmed).toBeTruthy() - sub = confirmed + sub = confirmed!.sub // Set last_digest_at to 60 seconds ago (the "since" value runJob would use) since = Math.floor(Date.now() / 1000) - 60 diff --git a/test/notify-response-shape.test.ts b/test/notify-response-shape.test.ts index 9d229b8..01491d8 100644 --- a/test/notify-response-shape.test.ts +++ b/test/notify-response-shape.test.ts @@ -37,7 +37,7 @@ describe('notify_response_shape', () => { const email = 'shape-test-' + Date.now() + '@example.com' const sub = await db.insertSubscription(pubkey, email, 'daily') const confirmed = await db.confirmSubscription(sub.key) - subId = confirmed.id + subId = confirmed!.sub.id // Start the express server on a random available port await new Promise((resolve) => { diff --git a/test/reschedule-on-frequency-change.test.ts b/test/reschedule-on-frequency-change.test.ts index 208ff93..6e78919 100644 --- a/test/reschedule-on-frequency-change.test.ts +++ b/test/reschedule-on-frequency-change.test.ts @@ -20,7 +20,7 @@ describe('Frequency change reschedules cron job', () => { const confirmed = await db.confirmSubscription(sub.key) expect(confirmed).toBeTruthy() - sub = confirmed + sub = confirmed!.sub }) it('registers cron job with daily frequency', () => { From dfdc6d489c6902ee3025990a6440ebe2a09bebea Mon Sep 17 00:00:00 2001 From: Agent Date: Fri, 18 Sep 2026 10:30:52 -0400 Subject: [PATCH 03/10] hardening: run production container as non-root user Add a dedicated 'app' user/group in the production image stage so the application runs without root privileges. The build stage retains root for apk add of build-time dependencies. Changes: - Create 'app' user and group via addgroup/adduser - Change ownership of /data to app:app - Set USER app before EXPOSE and CMD Closes mailship-c3s --- Dockerfile | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Dockerfile b/Dockerfile index f5feb6e..d165a87 100644 --- a/Dockerfile +++ b/Dockerfile @@ -52,6 +52,12 @@ COPY --from=build /app/src/emails/ ./dist/emails/ # Create data directory for SQLite RUN mkdir -p /data +# Create non-root user for security hardening +RUN addgroup -S app && adduser -S -G app app +RUN chown -R app:app /data + +USER app + EXPOSE 4738 ENV NODE_ENV=production From 3d24260419cd347de133f8d7d380d318ddd0708e Mon Sep 17 00:00:00 2001 From: Agent Date: Fri, 18 Sep 2026 10:32:09 -0400 Subject: [PATCH 04/10] Remove unused/misplaced dependencies (bcrypt, express-ws, ts-node-dev, @types/node) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove bcrypt (^5.1.1) — no imports anywhere in source - Remove express-ws (^5.0.2) — no imports (server uses REST + welshman) - Remove ts-node-dev (^2.0.0) — deprecated, replaced by tsx, was in deps - Remove @types/express-ws (dev) — no longer needed without express-ws - Move @types/node (^22.18.1) from dependencies → devDependencies - Prune bcrypt from pnpm.onlyBuiltDependencies list All quality gates pass: tsc --noEmit, eslint, build, and 16 unit tests. --- package.json | 7 +------ pnpm-lock.yaml | Bin 172089 -> 160729 bytes 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/package.json b/package.json index 306a4b7..d470759 100644 --- a/package.json +++ b/package.json @@ -18,9 +18,9 @@ "@eslint/js": "^9.35.0", "@types/better-sqlite3": "^7.6.13", "@types/express": "^5.0.3", - "@types/express-ws": "^3.0.5", "@types/mjml": "^4.7.4", "@types/mustache": "^4.2.6", + "@types/node": "^22.18.1", "@types/nodemailer": "^8.0.1", "@types/sanitize-html": "^2.16.0", "@types/ws": "^8.18.1", @@ -34,7 +34,6 @@ "vitest": "^5.0.0" }, "dependencies": { - "@types/node": "^22.18.1", "@welshman/content": "^0.6.3", "@welshman/feeds": "^0.6.3", "@welshman/lib": "^0.6.3", @@ -43,13 +42,11 @@ "@welshman/signer": "^0.6.3", "@welshman/store": "^0.6.3", "@welshman/util": "^0.6.3", - "bcrypt": "^5.1.1", "cron": "^4.3.3", "cron-parser": "^5.3.1", "dotenv": "^16.6.1", "express": "^4.21.2", "express-rate-limit": "^7.5.1", - "express-ws": "^5.0.2", "localstorage-polyfill": "^1.0.1", "mjml": "^4.15.3", "mustache": "^4.2.0", @@ -58,12 +55,10 @@ "sanitize-html": "^2.17.0", "sqlite3": "^5.1.7", "succinct-async": "^1.0.4", - "ts-node-dev": "^2.0.0", "ws": "^8.18.3" }, "pnpm": { "onlyBuiltDependencies": [ - "bcrypt", "sqlite3" ] } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f444b0528ec9a5b99489fa6cd9b34333329db50d..95b78b8326b9a13cf33c646aa7ba4054dd9a807b 100644 GIT binary patch delta 776 zcmXAmSxA&&6oxrxw%-|@W`1Whhj0={uq?~g!D^7q$QCfNkmx2_q|!=7Tmr=?bJRll ztb-Rtwy6lEedWQRvQ4d_FpIKrXqUZ?=d&oiADnV3p?eP13__g9bi3vD zj5a-Wyn+bvJKZfGZyZo$AUo43pB%I)@?Ozr11+C*xbn+Fa-8nk}P2R+vo!gK_?t3YQWzaOmpu?DUpudj#KVO-w~!wO9iIz;Y_ zhrO4f+Qy#SkPEz=K_Gc9qMa-5z+VVLi(ZFHeJUoj&c6!^YxIv4*2wFSHDaj~fovQ1 zyoEj&-3(}u)#K1jB@@t2H_|YQ<0s)L(&$Qz7LN<<;$D%3l9JHL3x0qZSc_$cPK;HC zbH{J!Q-T{r>1jLp4R}8|MH4QJ6u!oAI&8#JMiVycLWN6a+$q+!#8CShbWo=S6M2sX zzauwT@u!)0xiFxF4rdHHhy$Qfc09)MaSwLsY40RN(TaKaKpYt{1grdK9%dOrUvFK6 zDM;&|X+0-X@D`UX!2$~@DyY20hwX`^=`ix_T-=7CO;PK>)>?4@k?eW+g{}G6pr?{z zoK4pX(8a?AxX<+8m5($-7MeMyy*!nRHhSYlmHH2ZgUicN=sB(uFNE@?APxDduElTL$e>OoH%>mbZJ|Q(Y0%3s=VZh##ddw11$-o)uc z#en7tUR^OYPVmU=(4(8fp`sbk{@EMZ6LgB}=Gz3H_Zvwi7f%hm!9tA8)|*(UL8x9L z7?#__Xpkt!Mr66t3fEepY%x8KaRW=^EQS!rl@Ti@6h)@!k?C(##*9|$%t{|Q_TUGg zPxu2r*=q+M1IIvUc5eUer}3~L8HB-eWUJVrj20*JMm{ZxT1~0Su~tCvbs`~uanQG7 z^-wQGM5rV!CZb zVH`Z2$#5Nm{=%@;la9+~FWu#;HM$b35u;GO)S-voq!0}AooQi8N@}h|>a@GpkEp@= zn9NPMKqH>cwqin1>X@Rp7EEAj7>lMS{bVI!X#yuy*eHQE&>~%$J^j7wH^R4k_vk^L zQwD9Zv#mCsHOOc4Af` z;GJ0Yc&_Oy#S)qNsNYOr(`c^BcCKK9e_;c@>3hcnW#&0@!|91$CFL$Jj_VmBY2hMP z^HD-sZ3gOXrh_H?xU4D-Dd!t-1WuB1HPGXji58H{x>^&nV{ane;FT=eYBN!CAjjR! zfj6aG$q&TB2LGitd^T~E8F%T4KTS##VImHaL24>tL!S5JiFCJ_^cqEj=Osh&p(|ydVM2cpiFzI;HY%mbi5@@88B{NoulpLWg|RRQiqkP zM7rg4nIoIYDQ-q(vQfh_dMMS+pu#XFRxOfE#kAHa87ijS1k+NxlT>Mf`w8D5rVoO2 zZQ3hsypm-Ot#~X}>aUYZa~(dm33D?~GVnt)Sdy->}Y4pMqZk{e zGU-;h$r#OE0`(`wOmJHB4vCr`3I%*QEho?^r#jl}Ua-ynxCQpjuI}Hl5$L_XeHpf! zp|KvYF%q1hsYEzgNCdjYMl--jp$s#n17b!E1u$X~lSh$Ew1W#=G0i>j%~bs_#g13wl#Qq zu-O~mIl;u-Oes>fDlL{3BVA)e^ogj|;oPmUJFF8UGm}mxr9dB6*)W@Fbwy^XmI_wJ z&lP;WNlZ!%eT9hJvufQco|MO(Nii`1jQ0JSmZK zKTsRz{ld{5Fca3W)s0gR=6CGlCj(X%IoQ~Fb11B%vvZH1Xz030<%Otw z8VbrnqoDEuR6zYrA)pTZt)i-sT%@1!SIfO-e@fAvRJ5uwHLOgQB77=86jSc5h56lD zgPo45Wxk|MQLh$v64UT*cv&CNe6TPwF-91)1T$sk%jt2W-s_ZFMKj*5Vt$Dl>GihS zrsRPonQg3K<$6;*lyNE$-16EdZ(QQx*?!{fz)g4XxKQ+=*}T=IvMnzW znG#XnANLJ9QvvNwgGOtrMST`kb1R{^kWUJJE$g0Sd@_%wy=}45#&i~=qFqK618${e z6>?r70oOYE31NAk%=QJ3plON*8yvny*pv-!r%Adi<%Qs=m{H_XH-f2gLCPt8&0FDc zs*`W&3O)$>%W{~`ci z_JF|b-k+Xm2wgPMX~+7*fl`mE?plK^Oh%1vJT?vYiC(JFDrfkVG^vUdld7f3;>0Nv zs5T5LQA6SR7Oy1hU6r&f&CN7PERJczJiq9icT&jV`8&PfrDeDj`0a@rr8bQyT`%M^ zI+pN~k~NWPJ&kX4RLakX1}53%+-PT79!t>{mcmeXJQ+^p7@c6nQZmj*Xj)Q}p-_&_ znc4QV9?eIM_Sl;3`|)9Wi3D$(uLZ&VU?;2VVrQ`K-z>XYx>GF;`t%@)>2ZskREb)J z#-~wSugfN(H@hiaXpl9=ohX}lk%$N5raPUFDondC#iUj?T^VqAoGvA6f}f)U-JUzl z(KGP4Yxd2vJLhu}ynD~?A*<=a0+s}4XRo`B$wt%DVl4)%OxZp0lhY7Rp)nqvFfDRO zg(r!+;_U|-4Y=`P!{_zI^n%{k)0!HT3dylH(Q1VOCiD`7LJdi_J1V3~QBsF^Xuc-~ z{^h_PU6Dr4az2;@vu8eeJDo3<9wAO8Nq`M=hI3V?5h znkQ;zz1LzhsSIAl3YFT>m@>hji>WPHQbDk1iur-gZ3{PAT=*Gz>aNzn7#3h6SZ(TSQ|wnwPeAq z`o`f{UGI-8L^K`4)bgMejE<+YrgU0CEK*l0ZIKI)8@fm(cy~rZWm)y+hFT&tp|o;; zqLs$g0>SihQ_6w%^TN9Txccf#*i&H5H+xjMoiW2n8qea~R8LJBOok!JHm@a=(U`A9 zlxiYcZW5ISV^qrHY?GcQJAtB?Ad&`QDzua|L$Rj9yTt+1%_z89>R1e2^-eG+dQ=}A zSYCp-ZiY?V^oSi%xiVAeglqXYF_^?+au$z zYh=b3R72q~rd4Bj85;(ZBXo*2Clp8OL~S%8qp3-~O^{fQGSKr?biUmLFRrXyV8-_T zoO4oq^K9 znf{equexl=Rv!R|_d-6oMc>yUq6*EPnXJ~@Xgoel@m#VqAg5fh(rROTSr}Oc>JK(@ z16EAuaMNe`%7UeZ-EF1ioeIS!rxhk8C7a6?f^0a&^I5Kxie&<+~ zR@Zfl^E!QP6`J>syI@YQ1<=(kF<^7lVs-hlQTyg6z_m-8o4IzIZ!NewySaX7=KA=a zT^HxG_dft`+3C#geDC}^a@Askn_PAsBAka$G)7HEo)%$~ipddw)$7-4Qq|YX*K&1; zlMDW6O04v7I@!Sru|PkPY4!bbTVptaZ}#>&O@Y!Q{Dx~>Zp+C(=$IT+fiqfbu&Jj&ZV5&lAA{7S{ z62)8bF6ADn;fCecaV%e<`u>WqG<8!MCKF=)R-f;23)loXj02m}I~EwpcRmvKhaUo! zx%_F+1IuGW81P$QHJfSB80@5JhVD?zAVimYqUp}nyAzs8>ZA5BD05~*4)GN_lR>?a zo}tlJoEk?-pI465TSit-^x`R&N3E2u@cyY*rDBdD+ac9iec$~14*?rl8m!}+&5s`Y zC!IuHH+@`C)Z=x5Y?xR>GSjkJ>yFiokIORo5*47h2wrNp68T{+*f!Ay3C}S`44Ld8 zK5jNf^)8RMyWvJCnw2Ajv)s%zS+@UCM-I&2^I7mFaQ#2R6lw2218%b4^)T3N-~TW; z2*sm&4xE@j{5f#j&Mh^`WfEf^kIf~MJDehxf&k-pomBy!X)J`bLr-|{`6 zBZ%ugv$fu}HuiJivVZgf@XXNOP4<)Lz`^+|FM!?PhAl>XW#RRn739d;Tk3vr3!BQ{ zTgTr~Z1*p!I?MCQz{3tj=pwuH9PrrtZbMc!<9qY)hWYPb1gDnvZc$`tiO&h@9Df@K;NV_lrLRckfy7QRr!Bve~Hz zZ?b=R3^`~Qe-4h%mwpa@jrpuiLRxJIbYf;f`@DjM==9bxp0M9f;X{%d!T&B_Xke(4Fe>`O6^B?~P{9vaO zk#GFNVf!b)2ROj(r(?)XHvT4%4k)djz!{!RuTWtezSF2n5So5(TS zMIp;a35QTS!tNh(9q2Us%K#jmf9iVV%<&aRm`4{;ffPbQ=3j~+*B~HZKXDG+X#XgN z9N6xxgWAnE!JW1%jx29?#B!f~XB@e~{_7a>@B7Yk0Oz1BCVDWAd}bSThRsI7!6S?4 zx?Drn=z1;z?~WM|Vjr_V(nAi;@5>GAj6{LYKJ_xVad$^jT291$FvKG#I{FFD)%l>n}rzw|DoiTnY?wV!zb+;Cv?^el3~SsNySeb-N* z|Bv5;d}L|wLV9#O?99Gvmwon8mzDWAn>Tnc({Q-8}N6ZM!c( zzja6^4ZPmjgikOovnTS`*{F!@uz&e2a6EAld7VdZ9(a9m9)Df>?Be9BPAXgq zIbjr@`!+b_thR8^>DBc=)mULgMAu-+-*U<7`g!xVZYnNR%qzU$`~O}z0abSBcNmW= z%s_07`yEVrqy5x>0$0nCcAaK~ulcvWVzthqz+9My(=IKi&Jyr%>t!BR(yfBH_2go) zaNdN<2n6xhEL?wTGwDWe0W+PJ51ahf6cbpQVCjLs;e+#DS`XT1{|#Eb?`mY{#eNBQ z6`~elY u2Hdfz=v#T_a>3HS=;HWh;<;Q~*b%Pjn}1P5j)2_|+nq0J_R~6Y Date: Fri, 18 Sep 2026 10:32:18 -0400 Subject: [PATCH 05/10] fix(README): repair empty NIP-9a links, add Node & single-instance constraints - Replace empty NIP-9a URL (first paragraph and API section header) with a link to https://github.com/nostr-protocol/nips/blob/master/9a.md - Add Node >= 22 requirement to Development section - Add single-instance constraint note about in-process cron jobs --- README.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 5c5c7d1..d92191f 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Mailship -A Nostr email notification server. Receives events pushed from relays via [NIP-9a](), stores them, and sends daily or weekly digest emails. +A Nostr email notification server. Receives events pushed from relays via [NIP-9a](https://github.com/nostr-protocol/nips/blob/master/9a.md), stores them, and sends daily or weekly digest emails. This repo is a fork of [anchor](https://github.com/coracle-social/anchor). It has been built primarily to support email notifications in [Flotilla](https://flotilla.social), but supports alternate branding and could be set up to work with any Nostr relay supporting NIP-9a. @@ -79,7 +79,7 @@ Response: { ok: true } ``` ### POST /notify/:id -NIP-9a relay push callback. Called by relays or NPB when matching events are found. +[NIP-9a](https://github.com/nostr-protocol/nips/blob/master/9a.md) relay push callback. Called by relays or NPB when matching events are found. ``` Body: { id, relay, event? } @@ -112,6 +112,11 @@ Unsubscribe via link from digest email. ## Development +**Requirements:** Node >= 22 + +> **Single instance:** Mailship uses in-process cron jobs for digest scheduling. +> Running more than one instance concurrently may cause duplicate or missed emails. + ```sh pnpm install pnpm run build From 0a5b7ad14d044cfb50fdc72a21fc1187aa87d178 Mon Sep 17 00:00:00 2001 From: Agent Date: Fri, 18 Sep 2026 10:39:04 -0400 Subject: [PATCH 06/10] fix: validate relay URL before calling load() and soften unhandledRejection handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /notify/:id accepted an attacker-supplied relay URL with a non-ws:// scheme (e.g. http://…). The URL was passed straight to @welshman/net's load(), whose internal batcher throws 'Invalid relay url' asynchronously (via setTimeout). That error escaped the route's try/catch as an unhandled rejection, triggering process.exit(1) in src/index.ts — the entire server died. Any registered subscriber could crash the service repeatedly. Two fixes applied (either alone breaks the attack): 1. Validate relay scheme in the route handler before calling load() (src/server.ts). Uses isRelayUrl() from @welshman/util, which accepts only wss:// and ws:// schemes. Returns 400 immediately for invalid URLs, preventing the bad URL from ever reaching the batcher. 2. Soften process.on('unhandledRejection') in src/index.ts to log and continue instead of calling process.exit(1). Defence in depth: if any other async error escapes a try/catch, the server stays up. Test: test/notify-non-wss-relay-crash.test.ts — starts an ephemeral server, creates a confirmed subscription, POSTs with http://127.0.0.1:9, expects 400 with an error message. --- src/index.ts | 5 +- src/server.ts | 11 +++- test/notify-non-wss-relay-crash.test.ts | 82 +++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 test/notify-non-wss-relay-crash.test.ts diff --git a/src/index.ts b/src/index.ts index 8f9dd20..4c92819 100644 --- a/src/index.ts +++ b/src/index.ts @@ -7,7 +7,9 @@ import { registerSubscription } from './worker/index.js' process.on('unhandledRejection', (error: Error) => { console.error('Unhandled rejection:', error.stack) - process.exit(1) + // Do not process.exit(1) — an async rejection from a library's internal + // timer (e.g. @welshman/net's batcher) would let an attacker crash the + // entire server with a single malformed request. Log and continue. }) process.on('uncaughtException', (error: Error) => { @@ -15,6 +17,7 @@ process.on('uncaughtException', (error: Error) => { process.exit(1) }) + migrate().then(async () => { server.listen(PORT, () => { console.log('Running on port', PORT) diff --git a/src/server.ts b/src/server.ts index c7acb43..7cf5fdb 100644 --- a/src/server.ts +++ b/src/server.ts @@ -6,7 +6,7 @@ import { render } from './templates.js' import { confirmSubscriptionAction, unsubscribeAction, registerSubscription, ActionError } from './actions.js' import { getSubscriptionById, insertEvent, getSubscriptionByKey, getSubscriptionByPubkey } from './database.js' import { load } from '@welshman/net' -import { getIdFilters } from '@welshman/util' +import { getIdFilters, isRelayUrl } from '@welshman/util' import crypto from 'crypto' import { verifyEvent } from 'nostr-tools/pure' @@ -263,6 +263,15 @@ addRoute('post', '/notify/:id', async (req: Request, res: Response) => { return res.status(400).json({ error: 'id and relay are required' }) } + // Reject non-ws:// relay URLs. load() from @welshman/net throws + // Invalid relay url asynchronously inside a batcher timer, which + // escapes the route's try/catch and becomes an unhandledRejection + // that would crash the server. Validate early to avoid calling + // load() with an unsupported scheme. + if (!isRelayUrl(relay)) { + return res.status(400).json({ error: 'Invalid relay URL. Only wss:// or ws:// relays are supported.' }) + } + const sub = await getSubscriptionById(req.params.id) if (!sub) { diff --git a/test/notify-non-wss-relay-crash.test.ts b/test/notify-non-wss-relay-crash.test.ts new file mode 100644 index 0000000..e6e87a5 --- /dev/null +++ b/test/notify-non-wss-relay-crash.test.ts @@ -0,0 +1,82 @@ +// POST /notify with non-wss relay URL crashes the server (remote DoS) +// +// Bug: When POST /notify/:id receives a relay URL with a non-wss scheme +// (e.g. http://…), the handler calls `load()` from @welshman/net. Inside +// `load`, the batcher schedules an async `_execute` via setTimeout(200ms). +// When `getAdapter` throws `Invalid relay url`, the error escapes as an +// unhandledPromiseRejection because the batcher's `_execute` async function +// is called from setTimeout with no `.catch()`. The global +// `process.on('unhandledRejection')` handler in `src/index.ts` then calls +// `process.exit(1)`, killing the entire server. +// +// Fix applied (2 of 3 fixes): +// 1. Validate relay scheme before calling load() — the route handler now +// checks isRelayUrl() and returns 400 for non-ws:// schemes. +// 2. Don't process.exit(1) on unhandledRejection — log and continue +// (defence in depth for any other async edge-case). + +import { describe, it, expect, beforeAll, afterAll } from 'vitest' +import * as db from '../src/database.js' +import { server } from '../src/server.js' +import { createServer, type Server } from 'http' + +// Partially mock @welshman/net so that `load()` returns an empty array, +// preventing real relay connections during the test, while preserving all +// other exports from the library. +vi.mock('@welshman/net', async (importOriginal) => { + const actual = await importOriginal() + return { + ...(actual as Record), + load: vi.fn().mockResolvedValue([]), + } +}) + +describe('notify_non_wss_relay', () => { + let httpServer: Server + let baseUrl: string + let subId: string + + beforeAll(async () => { + await db.migrate() + + // Create and confirm a subscription we can use for the notify call + const pubkey = 'nws-test-pk-' + Date.now() + const email = 'nws-test-' + Date.now() + '@example.com' + const sub = await db.insertSubscription(pubkey, email, 'daily') + const confirmed = await db.confirmSubscription(sub.key) + subId = confirmed.id + + // Start the express server on a random available port + await new Promise((resolve) => { + httpServer = createServer(server) + httpServer.listen(0, () => { + const addr = httpServer.address() + if (addr && typeof addr === 'object') { + baseUrl = `http://localhost:${addr.port}` + } + resolve() + }) + }) + }) + + afterAll(async () => { + httpServer?.close() + }) + + it('rejects non-wss relay URL with 400 instead of crashing the server', async () => { + const res = await fetch(`${baseUrl}/notify/${subId}`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + id: 'nonexistent-' + Date.now(), + relay: 'http://127.0.0.1:9', + }), + }) + + expect(res.status).toBe(400) + + const body = await res.json() + expect(body).toHaveProperty('error') + expect(body.error).toMatch(/Invalid relay/) + }) +}) \ No newline at end of file From ae836da03367a72b4d72d0bdacf79cc9d570b82f Mon Sep 17 00:00:00 2001 From: Agent Date: Fri, 18 Sep 2026 10:46:27 -0400 Subject: [PATCH 07/10] fix: load reply/reaction events from repository into digest context Instead of setting context = events (which made repliesByParentId always empty), query the repository for events that #e-tag our matched event IDs with kinds NOTE, COMMENT, or REACTION. This gives buildParameters real reply/reaction data to count. Fixes the bug where every event in the digest email showed '0 replies / 0 reactions'. Added test/digest-reply-stats.test.ts that: - Publishes reply and reaction events into a mock repository - Calls sendFromStoredEvents with only the parent event - Asserts that the resulting digest parameters show Replies >= 1 and Reactions >= 1 --- src/digest.ts | 9 +- test/digest-reply-stats.test.ts | 167 ++++++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 test/digest-reply-stats.test.ts diff --git a/src/digest.ts b/src/digest.ts index ef9850a..c9b2714 100644 --- a/src/digest.ts +++ b/src/digest.ts @@ -25,6 +25,7 @@ import { EVENT_VIEWER_URL } from './env.js' import { profilesByPubkey, loadProfile, + repository, } from './repository.js' type DigestData = { @@ -86,7 +87,6 @@ export class Digest { sendFromStoredEvents = async (storedEvents: { id: string; event: TrustedEvent; relay: string }[]) => { const events = storedEvents.map(se => se.event) - const context = [...events] // For now, context == events (no reply loading) const relayByEventId = new Map(storedEvents.map(se => [se.event.id, se.relay])) // Load profiles for event authors @@ -101,6 +101,13 @@ export class Digest { } } + // Load reply/reaction context: events that tag our matched events + const eventIds = events.map(e => e.id) + const replyEvents = repository.query([ + { '#e': eventIds, kinds: [NOTE, COMMENT, REACTION] }, + ]) + const context = [...events, ...replyEvents] + const data = { events, context, relayByEventId } as DigestData if (data.events.length > 0) { diff --git a/test/digest-reply-stats.test.ts b/test/digest-reply-stats.test.ts new file mode 100644 index 0000000..67032cf --- /dev/null +++ b/test/digest-reply-stats.test.ts @@ -0,0 +1,167 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import type { TrustedEvent } from '@welshman/util' +import { spec } from '@welshman/lib' + +// ── Shared state accessible from both vi.mock factories and test body ────── +const mockRepo = vi.hoisted(() => { + const events: TrustedEvent[] = [] + return { + events, // shared mutable array + query: vi.fn((filters: any[]) => { + return events.filter(e => { + for (const f of filters) { + // #e filter: event must have an 'e' tag whose value matches one of the filter values + if (f['#e']) { + const eTagVals = e.tags.filter(t => t[0] === 'e').map(t => t[1]) + if (!f['#e'].some((id: string) => eTagVals.includes(id))) return false + } + // kinds filter + if (f.kinds && !f.kinds.includes(e.kind)) return false + } + return true + }) + }), + publish: vi.fn((event: TrustedEvent) => { + events.push(event) + return true + }), + } +}) + +// ── Mocks (hoisted before imports) ───────────────────────────────────────── +// ── Mock profiles map (pre-populated so waitForProfile doesn't time out) ── +const mockProfilesMap = vi.hoisted(() => new Map()) + +vi.mock('../src/repository.js', () => ({ + profilesByPubkey: { get: () => mockProfilesMap }, + loadProfile: vi.fn().mockResolvedValue(undefined), + repository: mockRepo, +})) + +vi.mock('../src/util.js', () => { + const createElementMock = vi.fn().mockImplementation((tagName: string) => { + const el: any = { tagName, children: [], innerText: '' } + el.appendChild = (child: any) => { el.children.push(child) } + el.toString = () => + `<${tagName}>${el.innerText}${el.children.map((c: any) => c.toString()).join('')}` + return el + }) + return { + displayDuration: vi.fn().mockReturnValue('1 hour'), + createElement: createElementMock, + } +}) + +// Captured digest parameters (set by the mailer mock) +let lastDigestParams: Record | undefined + +vi.mock('../src/mailer.js', () => ({ + sendDigest: vi.fn((_sub: any, variables: Record) => { + lastDigestParams = variables + }), +})) + +// ── Imports (after mocks) ────────────────────────────────────────────────── +import { Digest } from '../src/digest.js' +import type { Subscription } from '../src/alert.js' + +// Valid 64-char hex strings for nostr IDs and pubkeys +const PARENT_ID = 'aaa' + 'a'.repeat(61) // 64 hex chars +const REPLY_ID = 'bbb' + 'b'.repeat(61) +const REACTION_ID = 'ccc' + 'c'.repeat(61) +const PARENT_PK = 'ddd' + 'd'.repeat(61) +const REPLIER_PK = 'eee' + 'e'.repeat(61) +const REACTER_PK = 'fff' + 'f'.repeat(61) + +function makeEvent(overrides: Partial): TrustedEvent { + const eid = overrides.id || 'a'.repeat(64) + return { + id: eid, + kind: 1, + pubkey: PARENT_PK, + created_at: Math.floor(Date.now() / 1000), + tags: [], + content: 'test content', + ...overrides, + id: eid, + } as TrustedEvent +} + +describe('digest reply/reaction stats', () => { + let sub: Subscription + + beforeEach(() => { + // Reset shared state + mockRepo.events.length = 0 + lastDigestParams = undefined + + sub = { + id: 'sub-1', + key: 'key-1', + pubkey: PARENT_PK, + email: 'test@example.com', + frequency: 'daily', + created_at: Math.floor(Date.now() / 1000) - 3600, + confirmed_at: Math.floor(Date.now() / 1000) - 3600, + } + }) + + it('should count replies and reactions loaded from repository context', async () => { + // Create a parent event + const parentEvent = makeEvent({ + id: PARENT_ID, + kind: 1, + pubkey: PARENT_PK, + content: 'Hello world, this is the parent event', + created_at: Math.floor(Date.now() / 1000) - 600, + }) + + // Create a reply event referencing the parent via an 'e' tag + const replyEvent = makeEvent({ + id: REPLY_ID, + kind: 1, + pubkey: REPLIER_PK, + content: 'This is a reply to the parent', + created_at: Math.floor(Date.now() / 1000) - 500, + tags: [['e', PARENT_ID, '', 'root']], + }) + + // Create a reaction event (kind 7 = REACTION) + const reactionEvent = makeEvent({ + id: REACTION_ID, + kind: 7, + pubkey: REACTER_PK, + content: '+', + created_at: Math.floor(Date.now() / 1000) - 400, + tags: [['e', PARENT_ID, '', 'root']], + }) + + // Publish reply/reaction events to the mock repository so the fix can find them + mockRepo.publish(replyEvent) + mockRepo.publish(reactionEvent) + + // Pre-populate profiles so waitForProfile resolves immediately + mockProfilesMap.set(PARENT_PK, { pubkey: PARENT_PK, name: 'parent-user', picture: '' }) + mockProfilesMap.set(REPLIER_PK, { pubkey: REPLIER_PK, name: 'replier', picture: '' }) + mockProfilesMap.set(REACTER_PK, { pubkey: REACTER_PK, name: 'reacter', picture: '' }) + + const storedEvents = [ + { id: 'se-1', event: parentEvent, relay: 'wss://relay.example.com' }, + ] + + const digest = new Digest(sub) + await digest.sendFromStoredEvents(storedEvents) + + // sendDigest should have been called with the template parameters + expect(lastDigestParams).toBeDefined() + + const latest = lastDigestParams!.Latest + expect(latest.length).toBeGreaterThanOrEqual(1) + + const parentEntry = latest[0] + // When context includes reply/reaction events loaded from the repository, + // Replies should be >= 1 and Reactions >= 1 + expect(parentEntry.Replies).toBeGreaterThanOrEqual(1) + expect(parentEntry.Reactions).toBeGreaterThanOrEqual(1) + }) +}) \ No newline at end of file From d55e3f941bb1e0c1c09a47ace14dcddbbb682461 Mon Sep 17 00:00:00 2001 From: Agent Date: Fri, 18 Sep 2026 12:22:31 -0400 Subject: [PATCH 08/10] gitignore .beads/interactions.jsonl (untrack beads audit sidecar) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit .beads/interactions.jsonl is beads' agent audit trail sidecar — bd writes one line per mutation (claim, assign, comment, status change). The July bd release's `bd init` created it as a git-tracked file and omitted it from .beads/.gitignore by design, so every fragua run that claims or comments dirtied the worktree with an 'M .beads/interactions.jsonl'. Newer beads made the sidecar opt-in (audit.enabled default false) and gitignore it. This commit backports that convention: 1. Adds 'interactions.jsonl' to .beads/.gitignore 2. git rm --cached to stop tracking (file kept on disk) --- .beads/.gitignore | 4 ++++ .beads/interactions.jsonl | 0 2 files changed, 4 insertions(+) delete mode 100644 .beads/interactions.jsonl diff --git a/.beads/.gitignore b/.beads/.gitignore index f773858..40db2e9 100644 --- a/.beads/.gitignore +++ b/.beads/.gitignore @@ -71,6 +71,10 @@ backup/ *.db-shm db.sqlite bd.db + +# Interactions log (runtime, not versioned) +interactions.jsonl + # NOTE: Do NOT add negation patterns here. # They would override fork protection in .git/info/exclude. # Config files (metadata.json, config.yaml) are tracked by git by default diff --git a/.beads/interactions.jsonl b/.beads/interactions.jsonl deleted file mode 100644 index e69de29..0000000 From 77ce571d52b12040b0f26e57d88b9e465bbd9bd6 Mon Sep 17 00:00:00 2001 From: Agent Date: Fri, 18 Sep 2026 12:33:43 -0400 Subject: [PATCH 09/10] Remove dead build-in-production.sh (anchor fork artifact) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This file was a leftover from the anchor/mailship fork. Its function (removing pnpm overrides, building) is already handled by the Dockerfile via remove-pnpm-overrides.js. The --no-frozen-lockfile workaround is a Render CI quirk irrelevant to Docker builds. Zero references anywhere in the codebase — confirmed via grep. --- build-in-production.sh | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 build-in-production.sh diff --git a/build-in-production.sh b/build-in-production.sh deleted file mode 100644 index 639fba3..0000000 --- a/build-in-production.sh +++ /dev/null @@ -1,10 +0,0 @@ -#!/usr/bin/env bash - -# Remove link overrides -node remove-pnpm-overrides.js package.json - -# When CI=true as it is on render.com, removing link overrides breaks the lockfile -pnpm i --no-frozen-lockfile - -# Build everything -pnpm run build From bcce525a83df891d0e530ae6de7a27d218f150cd Mon Sep 17 00:00:00 2001 From: Agent Date: Fri, 18 Sep 2026 12:59:38 -0400 Subject: [PATCH 10/10] =?UTF-8?q?README:=20link=20NIP-9a=20to=20the=20spec?= =?UTF-8?q?=20PR=20(2194)=20=E2=80=94=20not=20yet=20merged=20to=20master?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d92191f..53634a8 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Mailship -A Nostr email notification server. Receives events pushed from relays via [NIP-9a](https://github.com/nostr-protocol/nips/blob/master/9a.md), stores them, and sends daily or weekly digest emails. +A Nostr email notification server. Receives events pushed from relays via [NIP-9a](https://github.com/nostr-protocol/nips/pull/2194), stores them, and sends daily or weekly digest emails. This repo is a fork of [anchor](https://github.com/coracle-social/anchor). It has been built primarily to support email notifications in [Flotilla](https://flotilla.social), but supports alternate branding and could be set up to work with any Nostr relay supporting NIP-9a. @@ -79,7 +79,7 @@ Response: { ok: true } ``` ### POST /notify/:id -[NIP-9a](https://github.com/nostr-protocol/nips/blob/master/9a.md) relay push callback. Called by relays or NPB when matching events are found. +[NIP-9a](https://github.com/nostr-protocol/nips/pull/2194) relay push callback. Called by relays or NPB when matching events are found. ``` Body: { id, relay, event? }