Skip to content

Commit ff64a00

Browse files
fix(credits): match the CoinPay status as a whole word, not a substring (#81)
The webhook guard tested /confirmed|completed|paid/i against the event name, which answers "does this name contain a success word anywhere" rather than "did the payment land". A negated status ends in one of those words - "payment.unpaid" ends in "paid", "payment.unconfirmed" ends in "confirmed" - so it was read as a confirmation and granted the full credit pack for money that never arrived. Anchor the status to the end of the event name and require a segment separator (or the start of the string) in front of it, so the last segment of a dotted event name has to be the status itself. Co-authored-by: clawedassistant26 <307253840+clawedassistant26@users.noreply.github.com>
1 parent f0702d9 commit ff64a00

2 files changed

Lines changed: 143 additions & 1 deletion

File tree

apps/pwa/src/routes/credits.mjs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,22 @@ creditsRouter.post("/credits/buy", requireAuth, async (req, res) => {
5050
}
5151
});
5252

53+
// The status is the last segment of a dotted event name ("payment.confirmed"),
54+
// so match it as a whole word. A bare substring test answers "does this name
55+
// contain a success word anywhere" rather than "did the payment land": a
56+
// negated status ends in one of them ("payment.unpaid" ends in "paid",
57+
// "payment.unconfirmed" ends in "confirmed") and would grant the full credit
58+
// pack for money that never arrived.
59+
const CONFIRMED_EVENT = /(?:^|[.\-_/:])(?:confirmed|completed|paid)$/i;
60+
5361
// CoinPay confirms payment → credit the balance (idempotent on purchase id).
5462
creditsRouter.post("/webhooks/coinpay", async (req, res) => {
5563
if (config.coinpay.webhookSecret && !verifySignature(req.get("x-coinpay-signature") || req.get("webhook-signature"), req.rawBody || "", config.coinpay.webhookSecret)) {
5664
return res.status(401).json({ error: "bad signature" });
5765
}
5866
const event = req.body?.type || req.body?.event;
5967
const payId = req.body?.data?.id || req.body?.payment_id || req.body?.id;
60-
if (event && /confirmed|completed|paid/i.test(event) && payId) {
68+
if (event && CONFIRMED_EVENT.test(event) && payId) {
6169
const p = await get(`SELECT * FROM credit_purchases WHERE id = ? AND status = 'pending'`, [payId]);
6270
if (p) {
6371
// Claim the purchase atomically, the same way /cli/token claims auth
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
// Regression tests for which CoinPay event types count as a payment
2+
// confirmation (POST /webhooks/coinpay).
3+
//
4+
// The guard used to be a substring test, so it answered "does this event name
5+
// contain a success word anywhere" rather than "is this a success event". Any
6+
// status that embeds one as a substring — including its own negation
7+
// ("payment.unpaid" contains "paid", "payment.unconfirmed" contains
8+
// "confirmed") — was read as a confirmation and granted the full credit pack
9+
// for money that never arrived.
10+
//
11+
// Same harness as credits-webhook.test.mjs: the real router against a
12+
// throwaway libsql file database, skipping cleanly when the PWA dependencies
13+
// are not installed. Run `npm install` in apps/pwa to enable them.
14+
import assert from "node:assert/strict";
15+
import http from "node:http";
16+
import fs from "node:fs";
17+
import { mkdtempSync } from "node:fs";
18+
import { tmpdir } from "node:os";
19+
import path from "node:path";
20+
import { createRequire } from "node:module";
21+
import test from "node:test";
22+
23+
const require = createRequire(import.meta.url);
24+
let deps = null;
25+
try {
26+
deps = { express: require("express"), cookieParser: require("cookie-parser") };
27+
} catch {
28+
deps = null; // pwa dependencies not installed — tests below skip
29+
}
30+
31+
// Point the app at a throwaway database BEFORE importing its modules (config
32+
// reads the environment once, at import time).
33+
const workdir = mkdtempSync(path.join(tmpdir(), "moshcode-pwa-event-match-test-"));
34+
process.env.DATABASE_URL = `file:${path.join(workdir, "test.db")}`;
35+
process.env.SESSION_SECRET = "test-secret";
36+
37+
async function boot() {
38+
const { migrate } = await import("../src/migrate.mjs");
39+
await migrate();
40+
const { run, all } = await import("../src/db.mjs");
41+
const { sessionMiddleware, csrfGuard } = await import("../src/lib/session.mjs");
42+
const { creditsRouter } = await import("../src/routes/credits.mjs");
43+
44+
const app = deps.express();
45+
app.use(deps.express.json({ verify: (req, _res, buf) => { req.rawBody = buf.toString("utf8"); } }));
46+
app.use(deps.express.urlencoded({ extended: false }));
47+
app.use(deps.cookieParser());
48+
app.use(sessionMiddleware);
49+
app.use(csrfGuard);
50+
app.use(creditsRouter);
51+
const server = await new Promise((resolve) => {
52+
const s = app.listen(0, "127.0.0.1", () => resolve(s));
53+
});
54+
const { port } = server.address();
55+
56+
const seedPurchase = async (payId, { userId = "u1", credits = 1000, usd = 5 } = {}) => {
57+
await run(`INSERT OR REPLACE INTO users (id, email, display_name, created_at) VALUES (?,?,?,1)`,
58+
[userId, `${userId}@b.c`, "demo"]);
59+
await run(
60+
`INSERT INTO credit_purchases (id,user_id,credits,amount_usd,status,created_at) VALUES (?,?,?,?,?,?)`,
61+
[payId, userId, credits, usd, "pending", Date.now()]
62+
);
63+
};
64+
65+
const granted = async (userId) => {
66+
const rows = await all(
67+
`SELECT delta FROM credit_ledger WHERE user_id = ? AND reason = 'topup.coinpay'`, [userId]);
68+
return { rows: rows.length, total: rows.reduce((s, r) => s + Number(r.delta), 0) };
69+
};
70+
71+
const deliver = (payId, type) => new Promise((resolve, reject) => {
72+
const req = http.request({
73+
host: "127.0.0.1", port, path: "/webhooks/coinpay", method: "POST", agent: false,
74+
headers: { "content-type": "application/json" },
75+
}, (res) => {
76+
let data = "";
77+
res.on("data", (chunk) => { data += chunk; });
78+
res.on("end", () => resolve({ status: res.statusCode, body: JSON.parse(data) }));
79+
});
80+
req.on("error", reject);
81+
req.end(JSON.stringify({ type, data: { id: payId } }));
82+
});
83+
84+
return { all, server, seedPurchase, granted, deliver };
85+
}
86+
87+
let booted = null;
88+
const app = () => (booted ||= boot());
89+
90+
test.after(() => {
91+
if (!booted) return;
92+
booted.then(({ server }) => server.close())
93+
.finally(() => { try { fs.rmSync(workdir, { recursive: true, force: true }); } catch { /* noop */ } });
94+
});
95+
96+
// A status that negates a success word must not be read as a confirmation.
97+
// "unpaid" ends in "paid" and "unconfirmed" ends in "confirmed", so a
98+
// substring test grants the pack for a payment that did not land.
99+
for (const type of ["payment.unpaid", "payment.unconfirmed"]) {
100+
test(`webhooks/coinpay: "${type}" credits nothing`, { skip: !deps && "apps/pwa deps not installed" }, async () => {
101+
const { seedPurchase, granted, deliver, all } = await app();
102+
const payId = `pay-${type.replace(/\W/g, "-")}`;
103+
const userId = `u-${type.replace(/\W/g, "-")}`;
104+
105+
await seedPurchase(payId, { userId });
106+
const res = await deliver(payId, type);
107+
assert.equal(res.status, 200);
108+
109+
assert.deepEqual(await granted(userId), { rows: 0, total: 0 },
110+
`${type} must not grant credits`);
111+
const [p] = await all(`SELECT status FROM credit_purchases WHERE id = '${payId}'`);
112+
assert.equal(p.status, "pending", `${type} must leave the purchase pending`);
113+
});
114+
}
115+
116+
// Controls: the confirmation paths the app actually depends on must keep
117+
// working. These pass both before and after the fix — they prove the fix
118+
// tightened the guard rather than disabling the webhook.
119+
for (const type of ["payment.confirmed", "payment.completed", "payment.paid", "PAYMENT.CONFIRMED"]) {
120+
test(`webhooks/coinpay: "${type}" still credits the balance`, { skip: !deps && "apps/pwa deps not installed" }, async () => {
121+
const { seedPurchase, granted, deliver, all } = await app();
122+
const payId = `pay-ok-${type.replace(/\W/g, "-")}`;
123+
const userId = `u-ok-${type.replace(/\W/g, "-")}`;
124+
125+
await seedPurchase(payId, { userId });
126+
const res = await deliver(payId, type);
127+
assert.equal(res.status, 200);
128+
129+
assert.deepEqual(await granted(userId), { rows: 1, total: 1000 },
130+
`${type} must grant the pack exactly once`);
131+
const [p] = await all(`SELECT status FROM credit_purchases WHERE id = '${payId}'`);
132+
assert.equal(p.status, "cleared");
133+
});
134+
}

0 commit comments

Comments
 (0)