Skip to content

Commit 03cb3a8

Browse files
authored
chore: type-aware lint, and the four things it found (#34)
Four rules, on src only: no-floating-promises, no-misused-promises, await-thenable, require-await. The broad recommendedTypeChecked preset is deliberately not used -- most of it duplicates what strict already enforces, at the cost of a much slower lint and a backlog of style findings. Five errors, of which one is a bug and two are traps: - express/captcha.ts was an async RequestHandler. Express 4 does not catch a rejected promise from a handler, so anything thrown inside endpoints.handle() -- crypto, JSON, a store -- left the request hanging until the client timed out, and logged an unhandled rejection instead of returning 500. Now synchronous, with the rejection routed to next(). - invisible.ts attached an ASYNC submit listener. e.preventDefault() only works because nothing had awaited yet; once the handler yields the browser has already submitted and cancelling is a no-op. It worked, but it was one added `await` away from silently breaking every protected form, with no test that would notice. The async half is now its own method, kicked off after the cancel. - environment.ts let audioCtx.close() float. The surrounding try/catch does not cover it, so a rejection surfaced as an unhandled rejection in the user's console. - violation-reporter's flush timer and the fastify plugin signature are both fine as they were; they now say so rather than reading as oversights. Closes WebDecoy/app#738
1 parent 1ac4046 commit 03cb3a8

7 files changed

Lines changed: 84 additions & 21 deletions

File tree

CONTRIBUTING.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ One flat config at the repo root (`eslint.config.mjs`) covers every package —
8282

8383
`@typescript-eslint/no-explicit-any` is a **warning** under a per-package budget, set in each package's lint script (`eslint src --max-warnings N`). CI fails if the count grows, so a new `any` needs either a real type or a deliberate decision to raise the number. Lower it when you remove one.
8484

85+
Four **type-aware** rules run on `src` (not on tests): `no-floating-promises`, `no-misused-promises`, `await-thenable`, `require-await`. They need a TypeScript program and are slower, so the set is deliberately small — these catch things `tsc` does not, and the rest of `recommendedTypeChecked` mostly duplicates `strict` at the cost of a large style backlog.
86+
87+
`no-floating-promises` is the one that earns its keep here. This SDK does a lot of deliberate fire-and-forget — violation reporting, honeytoken derivation, directory warmup — and an accidental one looks identical to an intentional one. Mark the deliberate ones with `void`, and say in a comment why the rejection is safe to drop.
88+
8589
Format code:
8690

8791
```bash

eslint.config.mjs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,36 @@ export default tseslint.config(
7070
},
7171
},
7272

73+
// Type-aware rules, on the source only.
74+
//
75+
// These need a TypeScript program, which costs real time — so they are scoped
76+
// to the rules that actually catch things `tsc` does not. The headline is
77+
// no-floating-promises: this SDK does a lot of deliberate fire-and-forget
78+
// (violation reporting, honeytoken derivation, directory warmup) where the
79+
// intentional ones are marked `void` and an accidental one would look
80+
// identical. A detection that silently never reported is the exact failure
81+
// this catches.
82+
//
83+
// The broad `recommendedTypeChecked` preset is deliberately NOT used: most of
84+
// it duplicates what `strict` already enforces, at the cost of a much slower
85+
// lint and a large backlog of findings that are style rather than defects.
86+
{
87+
files: ['**/src/**/*.ts'],
88+
ignores: ['**/*.test.ts', '**/*.spec.ts'],
89+
languageOptions: {
90+
parserOptions: {
91+
projectService: true,
92+
tsconfigRootDir: import.meta.dirname,
93+
},
94+
},
95+
rules: {
96+
'@typescript-eslint/no-floating-promises': 'error',
97+
'@typescript-eslint/no-misused-promises': 'error',
98+
'@typescript-eslint/await-thenable': 'error',
99+
'@typescript-eslint/require-await': 'error',
100+
},
101+
},
102+
73103
{
74104
files: ['**/*.test.ts', '**/*.spec.ts'],
75105
languageOptions: {

packages/client/src/collectors/environment.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,10 @@ export class EnvironmentalCollector {
265265
state: audioCtx.state,
266266
baseLatency: audioCtx.baseLatency
267267
};
268-
audioCtx.close();
268+
// Not awaited (the info is already gathered) but the rejection has to go
269+
// somewhere: `void` alone would leave an unhandled rejection logged in the
270+
// user's console, and the try/catch above does not cover it.
271+
audioCtx.close().catch(() => {});
269272
return info;
270273
} catch {
271274
return { supported: false, error: true };

packages/client/src/invisible.ts

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,12 @@ export class InvisibleSession {
108108
}
109109

110110
private _attachToForms(): void {
111-
document.addEventListener('submit', async (e) => {
111+
// Deliberately a SYNCHRONOUS listener. `e.preventDefault()` below only
112+
// works because nothing has awaited yet — once the handler yields, the
113+
// browser has already submitted the form and cancelling is a no-op. An
114+
// async listener made that a one-line change away from silently breaking,
115+
// with no test that would notice.
116+
document.addEventListener('submit', (e) => {
112117
const form = e.target as HTMLFormElement;
113118
if (form.dataset.webdecoyIgnore) return;
114119

@@ -122,28 +127,35 @@ export class InvisibleSession {
122127

123128
if (!this.lastScore || Date.now() - this.lastScore.timestamp > 60000) {
124129
e.preventDefault();
125-
126-
try {
127-
const result = await this.execute(form.dataset.webdecoyAction || 'form_submit');
128-
tokenField.value = result.token || '';
129-
130-
if (result.success) {
131-
form.submit();
132-
} else {
133-
document.dispatchEvent(
134-
new CustomEvent('webdecoy:blocked', { detail: { score: result.score, form } }),
135-
);
136-
}
137-
} catch (error) {
138-
console.error('WebDecoy captcha error:', error);
139-
form.submit(); // Fail open
140-
}
130+
void this._scoreThenSubmit(form, tokenField);
141131
} else {
142132
tokenField.value = this.lastScore.token || '';
143133
}
144134
});
145135
}
146136

137+
/** Score the session, then resubmit the form the listener cancelled. */
138+
private async _scoreThenSubmit(
139+
form: HTMLFormElement,
140+
tokenField: HTMLInputElement,
141+
): Promise<void> {
142+
try {
143+
const result = await this.execute(form.dataset.webdecoyAction || 'form_submit');
144+
tokenField.value = result.token || '';
145+
146+
if (result.success) {
147+
form.submit();
148+
} else {
149+
document.dispatchEvent(
150+
new CustomEvent('webdecoy:blocked', { detail: { score: result.score, form } }),
151+
);
152+
}
153+
} catch (error) {
154+
console.error('WebDecoy captcha error:', error);
155+
form.submit(); // Fail open
156+
}
157+
}
158+
147159
async execute(action = ''): Promise<VerifyResponse> {
148160
const elapsed = Date.now() - this.startTime;
149161
if (elapsed < this.options.minCollectionTime) {

packages/express/src/captcha.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,15 @@ function normalizeQuery(query: Request['query']): Record<string, string | undefi
6262
export function webdecoyCaptcha(options?: ExpressCaptchaOptions): RequestHandler {
6363
const endpoints = createCaptchaEndpoints(options);
6464

65-
return async (req: Request, res: Response, next: NextFunction): Promise<void> => {
65+
return (req: Request, res: Response, next: NextFunction): void => {
66+
// Express 4 does not catch a rejected promise from a handler, so an async
67+
// handler that throws leaves the request hanging until the client times out
68+
// and logs an unhandled rejection instead of a 500. Kept synchronous, with
69+
// the rejection routed to the error middleware explicitly.
70+
void handle(req, res, next).catch(next);
71+
};
72+
73+
async function handle(req: Request, res: Response, next: NextFunction): Promise<void> {
6674
const result = await endpoints.handle({
6775
method: req.method,
6876
pathname: req.path,
@@ -80,5 +88,5 @@ export function webdecoyCaptcha(options?: ExpressCaptchaOptions): RequestHandler
8088
res.status(result.status);
8189
for (const [k, v] of Object.entries(result.headers)) res.setHeader(k, v);
8290
res.json(result.body);
83-
};
91+
}
8492
}

packages/fastify/src/captcha.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ import type { FastifyInstance, FastifyRequest, FastifyReply } from 'fastify';
1818
import fp from 'fastify-plugin';
1919
import { createCaptchaEndpoints, type CaptchaEndpointsOptions } from '@webdecoy/node';
2020

21+
// fastify-plugin's async contract: the signature is what marks this a plugin,
22+
// not the body, so there is nothing here to await.
23+
// eslint-disable-next-line @typescript-eslint/require-await
2124
async function plugin(fastify: FastifyInstance, options: CaptchaEndpointsOptions): Promise<void> {
2225
const endpoints = createCaptchaEndpoints(options);
2326

packages/webdecoy/src/violation-reporter.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ export class ViolationReporter {
2929
this.debug = config.debug ?? false;
3030

3131
const flushInterval = config.flushInterval ?? 5000;
32-
this.flushTimer = setInterval(() => this.flush(), flushInterval);
32+
// flush() catches everything internally and never rejects, so `void` is the
33+
// whole handling. Said out loud because a timer whose callback rejects
34+
// keeps firing and every tick adds another unhandled rejection.
35+
this.flushTimer = setInterval(() => void this.flush(), flushInterval);
3336
if (this.flushTimer.unref) {
3437
this.flushTimer.unref();
3538
}

0 commit comments

Comments
 (0)