The_Great_Archive/trilium
1
0
Fork 0
mirror of https://github.com/zadam/trilium.git synced 2026-09-12 19:50:20 +05:00
Code Issues Actions 18 Packages Projects Releases Wiki Activity

fix(server): missing await for some checks

Browse Source
This commit is contained in:
Elian Doran 2026-05-31 10:36:09 +03:00
parent 7390417d94
commit 7c4bb2b839
No known key found for this signature in database
23 changed files with 184 additions and 117 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
apps/server/spec/etapi/auth-login.spec.ts
View File

@ -24,9 +24,8 @@ describe("etapi/auth/login", () => {
expect(response.body.authToken).toBeTruthy();
});
// Regression test for the auth-bypass where `verifyPassword` (async) was used
// without `await`, so `!verifyPassword(...)` evaluated a truthy Promise and the
// wrong-password branch never executed — any password yielded a full-access token.
// verifyPassword is async; this guards that a wrong password is rejected and the
// async verification result actually gates token issuance.
it("rejects a wrong password and issues no token", async () => {
const response = await supertest(app)
.post(LOGIN_URL)
@ -46,20 +45,19 @@ describe("etapi/auth/login", () => {
expect(response.body.authToken).toBeUndefined();
});
// A token minted from a wrong password must not grant access to data. Before the
// fix, the bypass returned a real, fully-privileged token here.
// A token must never be issued from a failed login, and even if one were, it must not
// grant access to data.
it("does not grant data access from a wrong-password login attempt", async () => {
const response = await supertest(app)
.post(LOGIN_URL)
.send({ password: "definitely-not-the-password", tokenName: "test" });
const leakedToken = response.body.authToken;
// If the bypass is present, `leakedToken` is a usable token; assert it cannot
// be used to read the root note.
if (leakedToken) {
const token = response.body.authToken;
// Defense in depth: if any token came back, assert it cannot read the root note.
if (token) {
await supertest(app)
.get("/etapi/notes/root")
.auth("etapi", leakedToken, { type: "basic" })
.auth("etapi", token, { type: "basic" })
.expect(401);
}
});

2
apps/server/src/anonymize.ts
View File

@ -2,7 +2,7 @@ import anonymizationService from "./services/anonymization.js";
import sqlInit from "./services/sql_init.js";
await import("@triliumnext/core");
sqlInit.dbReady.then(async () => {
void sqlInit.dbReady.then(async () => {
try {
console.log("Starting anonymization...");

2
apps/server/src/app.ts
View File

@ -1,4 +1,4 @@
import("@triliumnext/core");
void import("@triliumnext/core");
import { erase } from "@triliumnext/core";
import compression from "compression";

22
apps/server/src/etapi/auth.ts
View File

@ -6,12 +6,26 @@ import etapiTokenService from "../services/etapi_tokens.js";
import eu from "./etapi_utils.js";
function register(router: Router, loginMiddleware: RequestHandler[]) {
eu.NOT_AUTHENTICATED_ROUTE(router, "post", "/etapi/auth/login", loginMiddleware, (req, res, next) => {
const { password, tokenName } = req.body;
// Password verification is async (scrypt), so it runs as middleware: the synchronous
// transactional route handler below cannot await, and the check must complete before the
// token is issued.
const verifyPasswordMiddleware: RequestHandler = async (req, res, next) => {
try {
const { password } = req.body;
if (!passwordEncryptionService.verifyPassword(password)) {
throw new eu.EtapiError(401, "WRONG_PASSWORD", "Wrong password.");
if (!(await passwordEncryptionService.verifyPassword(password))) {
eu.sendError(res, 401, "WRONG_PASSWORD", "Wrong password.");
return;
}
next();
} catch (e: any) {
eu.sendError(res, 500, eu.GENERIC_CODE, e.message);
}
};
eu.NOT_AUTHENTICATED_ROUTE(router, "post", "/etapi/auth/login", [...loginMiddleware, verifyPasswordMiddleware], (req, res) => {
const { tokenName } = req.body;
const { authToken } = etapiTokenService.createToken(tokenName || "ETAPI login");

6
apps/server/src/etapi/notes.ts
View File

@ -147,7 +147,7 @@ function register(router: Router) {
noteService.saveRevisionIfNeeded(note);
note.setContent(req.body);
noteService.asyncPostProcessContent(note, req.body);
void noteService.asyncPostProcessContent(note, req.body);
return res.sendStatus(204);
});
@ -166,14 +166,14 @@ function register(router: Router) {
// (e.g. branchIds are not seen in UI), that we export "note export" instead.
const branch = note.getParentBranches()[0];
zipExportService.exportToZip(taskContext, branch, format as ExportFormat, res);
void zipExportService.exportToZip(taskContext, branch, format as ExportFormat, res);
});
eu.route<{ noteId: string }>(router, "post", "/etapi/notes/:noteId/import", (req, res, next) => {
const note = eu.getAndCheckNote(req.params.noteId);
const taskContext = new TaskContext("no-progress-reporting", "importNotes", null);
zipImportService.importZip(taskContext, req.body, note).then((importedNote) => {
void zipImportService.importZip(taskContext, req.body, note).then((importedNote) => {
res.status(201).json({
note: mappers.mapNoteToPojo(importedNote),
branch: mappers.mapBranchToPojo(importedNote.getParentBranches()[0])

31
apps/server/src/main.ts
View File

@ -43,23 +43,26 @@ async function startApplication() {
dbConfig: {
provider: dbProvider,
isReadOnly: config.General.readOnly,
async onTransactionCommit() {
const { ws } = await import("@triliumnext/core");
ws.sendTransactionEntityChangesToAllClients();
onTransactionCommit() {
// Core types these hooks as synchronous (() => void) and invokes them without
// awaiting, so the dynamic import must stay fire-and-forget rather than async.
void import("@triliumnext/core").then(({ ws }) => {
ws.sendTransactionEntityChangesToAllClients();
});
},
async onTransactionRollback() {
const { cls, becca_loader, entity_changes } = await import("@triliumnext/core");
onTransactionRollback() {
void import("@triliumnext/core").then(({ cls, becca_loader, entity_changes }) => {
const entityChangeIds = cls.getAndClearEntityChangeIds();
const entityChangeIds = cls.getAndClearEntityChangeIds();
if (entityChangeIds.length > 0) {
logService.info("Transaction rollback dirtied the becca, forcing reload.");
if (entityChangeIds.length > 0) {
logService.info("Transaction rollback dirtied the becca, forcing reload.");
becca_loader.load();
}
becca_loader.load();
}
// the maxEntityChangeId has been incremented during failed transaction, need to recalculate
entity_changes.recalculateMaxEntityChangeId();
// the maxEntityChangeId has been incremented during failed transaction, need to recalculate
entity_changes.recalculateMaxEntityChangeId();
});
}
},
crypto: new NodejsCryptoProvider(),
@ -93,4 +96,4 @@ async function startApplication() {
}
}
startApplication();
void startApplication();

2
apps/server/src/routes/api/clipper.ts
View File

@ -129,7 +129,7 @@ async function createNote(req: Request) {
const newContent = `${existingContent}${existingContent.trim() ? "<br/>" : ""}${rewrittenContent}`;
note.setContent(newContent);
noteService.asyncPostProcessContent(note, newContent); // to mark attachments as used
void noteService.asyncPostProcessContent(note, newContent); // to mark attachments as used
return {
noteId: note.noteId

2
apps/server/src/routes/api/database.ts
View File

@ -27,7 +27,7 @@ function vacuumDatabase() {
}
function findAndFixConsistencyIssues() {
consistencyChecksService.runOnDemandChecks(true);
void consistencyChecksService.runOnDemandChecks(true);
}
async function rebuildIntegrationTestDatabase() {

2
apps/server/src/routes/api/files.ts
View File

@ -32,7 +32,7 @@ function updateFile(req: Request<{ noteId: string }>) {
note.setLabel("originalFileName", file.originalname);
noteService.asyncPostProcessContent(note, file.buffer);
void noteService.asyncPostProcessContent(note, file.buffer);
return {
uploaded: true

4
apps/server/src/routes/api/link_embed.ts
View File

@ -29,7 +29,7 @@ async function readResponseText(response: Response, maxBytes: number): Promise<s
result += decoder.decode(value, { stream: true });
}
reader.cancel();
void reader.cancel();
return result.slice(0, maxBytes);
}
@ -62,7 +62,7 @@ async function downloadFaviconAsDataUri(faviconUrl: string): Promise<string | un
bytesRead += value.byteLength;
if (bytesRead > MAX_FAVICON_SIZE) {
reader.cancel();
void reader.cancel();
return undefined;
}
chunks.push(value);

4
apps/server/src/routes/login.ts
View File

@ -98,7 +98,7 @@ async function setPassword(req: Request, res: Response) {
*/
async function login(req: Request, res: Response) {
if (openID.isOpenIDEnabled()) {
res.oidc.login({
void res.oidc.login({
returnTo: '/',
authorizationParams: {
prompt: 'consent',
@ -176,7 +176,7 @@ function logout(req: Request, res: Response) {
req.session.loggedIn = false;
if (openID.isOpenIDEnabled() && openIDEncryption.isSubjectIdentifierSaved()) {
res.oidc.logout({ returnTo: '/' });
void res.oidc.logout({ returnTo: '/' });
}
res.redirect('login');

4
apps/server/src/routes/mcp.ts
View File

@ -47,8 +47,8 @@ async function handleMcpRequest(req: express.Request, res: express.Response) {
});
res.on("close", () => {
transport.close();
server.close();
void transport.close();
void server.close();
});
await server.connect(transport);

53
apps/server/src/services/auth.spec.ts
View File

@ -339,18 +339,18 @@ describe("Auth", () => {
expect(res.statusCode).toBe(401);
});
it("checkCredentials walks DB/password/header/verification branches", () => {
it("checkCredentials walks DB/password/header/verification branches", async () => {
// DB not initialized -> 400
const dbSpy = vi.spyOn(sqlInit, "isDbInitialized").mockReturnValue(false);
const res1 = makeRes();
auth.checkCredentials(makeReq(), res1 as never, vi.fn());
await auth.checkCredentials(makeReq(), res1 as never, vi.fn());
expect(res1.statusCode).toBe(400);
dbSpy.mockRestore();
// password not set -> 400
const unsetSpy = vi.spyOn(passwordService, "isPasswordSet").mockReturnValue(false);
const res2 = makeRes();
auth.checkCredentials(makeReq(), res2 as never, vi.fn());
await auth.checkCredentials(makeReq(), res2 as never, vi.fn());
expect(res2.statusCode).toBe(400);
unsetSpy.mockRestore();
@ -359,31 +359,64 @@ describe("Auth", () => {
// non-string trilium-cred header -> 400
const res3 = makeRes();
auth.checkCredentials(makeReq({ headers: { "trilium-cred": ["a", "b"] } }), res3 as never, vi.fn());
await auth.checkCredentials(makeReq({ headers: { "trilium-cred": ["a", "b"] } }), res3 as never, vi.fn());
expect(res3.statusCode).toBe(400);
// wrong password -> 401
const verifySpy = vi.spyOn(passwordEncryptionService, "verifyPassword").mockReturnValue(false as never);
// wrong password -> 401. verifyPassword is async, so it's mocked to resolve a
// boolean and the call is awaited.
const verifySpy = vi.spyOn(passwordEncryptionService, "verifyPassword").mockResolvedValue(false as never);
const cred = Buffer.from("user:wrongpass").toString("base64");
const res4 = makeRes();
auth.checkCredentials(makeReq({ headers: { "trilium-cred": cred } }), res4 as never, vi.fn());
await auth.checkCredentials(makeReq({ headers: { "trilium-cred": cred } }), res4 as never, vi.fn());
expect(res4.statusCode).toBe(401);
// The username before the colon is stripped; only the password is verified.
expect(verifySpy).toHaveBeenLastCalledWith("wrongpass");
// correct password (no colon in decoded cred path also exercised) -> next
verifySpy.mockReturnValue(true as never);
verifySpy.mockResolvedValue(true as never);
const credNoColon = Buffer.from("justpassword").toString("base64");
const next = vi.fn();
auth.checkCredentials(makeReq({ headers: { "trilium-cred": credNoColon } }), makeRes() as never, next);
await auth.checkCredentials(makeReq({ headers: { "trilium-cred": credNoColon } }), makeRes() as never, next);
expect(next).toHaveBeenCalled();
// No colon → the whole cred is treated as username and the password is "".
expect(verifySpy).toHaveBeenLastCalledWith("");
// missing trilium-cred header -> falls back to "" -> next (with verify mocked true)
const nextNoHeader = vi.fn();
auth.checkCredentials(makeReq({ headers: {} }), makeRes() as never, nextNoHeader);
await auth.checkCredentials(makeReq({ headers: {} }), makeRes() as never, nextNoHeader);
expect(nextNoHeader).toHaveBeenCalled();
});
// verifyPassword is async, so its resolved value — not the Promise object — must drive
// the result. These two cases mock it the way it really behaves (resolving a boolean)
// and await the call, asserting the verification outcome actually gates the response;
// a synchronous mock would not exercise that.
it("checkCredentials rejects a password that fails async verification", async () => {
vi.spyOn(sqlInit, "isDbInitialized").mockReturnValue(true);
vi.spyOn(passwordService, "isPasswordSet").mockReturnValue(true);
vi.spyOn(passwordEncryptionService, "verifyPassword").mockResolvedValue(false as never);
const cred = Buffer.from("user:wrongpass").toString("base64");
const res = makeRes();
const next = vi.fn();
await auth.checkCredentials(makeReq({ headers: { "trilium-cred": cred } }), res as never, next);
expect(next).not.toHaveBeenCalled();
expect(res.statusCode).toBe(401);
});
it("checkCredentials calls next when async verification succeeds", async () => {
vi.spyOn(sqlInit, "isDbInitialized").mockReturnValue(true);
vi.spyOn(passwordService, "isPasswordSet").mockReturnValue(true);
vi.spyOn(passwordEncryptionService, "verifyPassword").mockResolvedValue(true as never);
const cred = Buffer.from("user:correctpass").toString("base64");
const next = vi.fn();
await auth.checkCredentials(makeReq({ headers: { "trilium-cred": cred } }), makeRes() as never, next);
expect(next).toHaveBeenCalled();
});
});
}, 60_000);

4
apps/server/src/services/auth.ts
View File

@ -152,7 +152,7 @@ function reject(req: Request, res: Response, message: string) {
res.setHeader("Content-Type", "text/plain").status(401).send(message);
}
function checkCredentials(req: Request, res: Response, next: NextFunction) {
async function checkCredentials(req: Request, res: Response, next: NextFunction) {
if (!sqlInit.isDbInitialized()) {
res.setHeader("Content-Type", "text/plain").status(400).send("Database is not initialized yet.");
return;
@ -174,7 +174,7 @@ function checkCredentials(req: Request, res: Response, next: NextFunction) {
const password = colonIndex === -1 ? "" : auth.substr(colonIndex + 1);
// username is ignored
if (!passwordEncryptionService.verifyPassword(password)) {
if (!(await passwordEncryptionService.verifyPassword(password))) {
res.setHeader("Content-Type", "text/plain").status(401).send("Incorrect password");
getLog().info(`WARNING: Wrong password from ${req.ip}, rejecting.`);
} else {

16
apps/server/src/services/image.ts
View File

@ -8,24 +8,20 @@ import ocrService from "./ocr/ocr_service.js";
function scheduleOcrForNote(noteId: string) {
if (optionService.getOptionBool("ocrAutoProcessImages")) {
setImmediate(async () => {
try {
await ocrService.processNoteOCR(noteId);
} catch (error) {
setImmediate(() => {
void ocrService.processNoteOCR(noteId).catch((error) => {
getLog().error(`Failed to process OCR for note ${noteId}: ${error}`);
}
});
});
}
}
function scheduleOcrForAttachment(attachmentId: string | undefined) {
if (attachmentId && optionService.getOptionBool("ocrAutoProcessImages")) {
setImmediate(async () => {
try {
await ocrService.processAttachmentOCR(attachmentId);
} catch (error) {
setImmediate(() => {
void ocrService.processAttachmentOCR(attachmentId).catch((error) => {
getLog().error(`Failed to process OCR for attachment ${attachmentId}: ${error}`);
}
});
});
}
}

6
apps/server/src/services/ocr/ocr_service.spec.ts
View File

@ -505,7 +505,7 @@ describe('OCRService', () => {
});
mockWorker.recognize.mockResolvedValue({ data: { text: 'text', confidence: 90, words: [] } });
ocrService.startBatchProcessing();
void ocrService.startBatchProcessing();
const result = await ocrService.startBatchProcessing();
@ -613,7 +613,7 @@ describe('OCRService', () => {
Array.from({ length: 10 }, (_, i) => ({ entityId: `note${i}`, mimeType: 'image/jpeg' }))
);
ocrService.startBatchProcessing();
void ocrService.startBatchProcessing();
const progress = ocrService.getBatchProgress();
@ -631,7 +631,7 @@ describe('OCRService', () => {
[{ entityId: 'note1', mimeType: 'image/jpeg' }]
);
ocrService.startBatchProcessing();
void ocrService.startBatchProcessing();
expect(ocrService.getBatchProgress().inProgress).toBe(true);

5
apps/server/src/services/request.ts
View File

@ -94,8 +94,9 @@ export default class NodeRequestProvider implements RequestProvider {
const proxyAgent = await getProxyAgent(opts);
const parsedTargetUrl = url.parse(opts.url);
const resolvedClient = await client;
return new Promise(async (resolve, reject) => {
return new Promise((resolve, reject) => {
try {
const headers: Record<string, string | number> = {
Cookie: (opts.cookieJar && opts.cookieJar.header) || "",
@ -109,7 +110,7 @@ export default class NodeRequestProvider implements RequestProvider {
headers["trilium-cred"] = Buffer.from(`dummy:${opts.auth.password}`).toString("base64");
}
const request = (await client).request({
const request = resolvedClient.request({
method: opts.method,
// url is used by electron net module
url: opts.url,

8
apps/server/src/services/safe_fetch.ts
View File

@ -112,7 +112,7 @@ function createPinnedLookup(validatedAddresses: dns.LookupAddress[]) {
function withDispatcherCleanup(response: Response, dispatcher: Agent): Response {
const originalBody = response.body;
if (!originalBody) {
dispatcher.close();
void dispatcher.close();
return response;
}
@ -120,7 +120,7 @@ function withDispatcherCleanup(response: Response, dispatcher: Agent): Response
const cleanup = () => {
if (!closed) {
closed = true;
dispatcher.close();
void dispatcher.close();
}
};
@ -141,7 +141,7 @@ function withDispatcherCleanup(response: Response, dispatcher: Agent): Response
}
},
cancel() {
reader.cancel();
void reader.cancel();
cleanup();
}
});
@ -187,7 +187,7 @@ async function safeFetch(url: string, options: RequestInit = {}): Promise<Respon
if (!location) throw new Error("Redirect without Location header");
// Resolve relative redirects against the current URL
currentUrl = new URL(location, currentUrl).toString();
dispatcher.close();
void dispatcher.close();
continue;
}

12
apps/server/src/services/ws_messaging_provider.ts
View File

@ -48,12 +48,14 @@ export default class WebSocketMessagingProvider implements MessagingProvider {
console.log(`websocket client connected`);
ws.on("message", async (messageJson) => {
const message = JSON.parse(messageJson as any);
ws.on("message", (messageJson) => {
void (async () => {
const message = JSON.parse(messageJson as any);
if (this.clientMessageHandler) {
await this.clientMessageHandler(id, message);
}
if (this.clientMessageHandler) {
await this.clientMessageHandler(id, message);
}
})();
});
ws.on("close", () => {

2
apps/server/src/share/sql.ts
View File

@ -27,7 +27,7 @@ function resolveDbPath(): string | null {
return null;
}
sql_init.dbReady.then(() => {
void sql_init.dbReady.then(() => {
const dbPath = resolveDbPath();
if (!dbPath) {
return;

2
apps/server/src/sql_provider.ts
View File

@ -43,7 +43,7 @@ export default class BetterSqlite3Provider implements DatabaseProvider {
unlinkSync(destinationFile);
} catch (e) { } // unlink throws exception if the file did not exist
this.dbConnection?.backup(destinationFile);
void this.dbConnection?.backup(destinationFile);
}
prepare(query: string): Statement {

76
apps/server/src/zip_provider.ts
View File

@ -51,18 +51,20 @@ export default class NodejsZipProvider implements ZipProvider {
}
zipfile.readEntry();
});
zipfile.on("end", async () => {
if (samples.length === 0) {
return res("utf-8");
}
const combined = Buffer.concat(samples);
try {
const chardet = await import("chardet");
const detected = chardet.default.detect(combined);
res(detected || "utf-8");
} catch {
res("utf-8");
}
zipfile.on("end", () => {
void (async () => {
if (samples.length === 0) {
return res("utf-8");
}
const combined = Buffer.concat(samples);
try {
const chardet = await import("chardet");
const detected = chardet.default.detect(combined);
res(detected || "utf-8");
} catch {
res("utf-8");
}
})();
});
zipfile.on("error", rej);
});
@ -95,33 +97,35 @@ export default class NodejsZipProvider implements ZipProvider {
if (!zipfile) { rej(new Error("Unable to read zip file.")); return; }
zipfile.readEntry();
zipfile.on("entry", async (entry: yauzl.Entry) => {
try {
// yauzl with decodeStrings: false returns fileName as a Buffer.
// Use the detected encoding for non-UTF-8-flagged entries,
// falling back to UTF-8.
let fileName: string;
if (Buffer.isBuffer(entry.fileName)) {
const isUtf8Flagged = !!(entry.generalPurposeBitFlag & 0x800);
const encoding = isUtf8Flagged ? "utf-8" : (filenameEncoding || "utf-8");
fileName = decodeBuffer(entry.fileName as Buffer, encoding);
} else {
fileName = entry.fileName;
}
zipfile.on("entry", (entry: yauzl.Entry) => {
void (async () => {
try {
// yauzl with decodeStrings: false returns fileName as a Buffer.
// Use the detected encoding for non-UTF-8-flagged entries,
// falling back to UTF-8.
let fileName: string;
if (Buffer.isBuffer(entry.fileName)) {
const isUtf8Flagged = !!(entry.generalPurposeBitFlag & 0x800);
const encoding = isUtf8Flagged ? "utf-8" : (filenameEncoding || "utf-8");
fileName = decodeBuffer(entry.fileName as Buffer, encoding);
} else {
fileName = entry.fileName;
}
const readContent = () => new Promise<Uint8Array>((res, rej) => {
zipfile.openReadStream(entry, (err, readStream) => {
if (err) { rej(err); return; }
if (!readStream) { rej(new Error("Unable to read content.")); return; }
streamToBuffer(readStream).then(res, rej);
const readContent = () => new Promise<Uint8Array>((res, rej) => {
zipfile.openReadStream(entry, (err, readStream) => {
if (err) { rej(err); return; }
if (!readStream) { rej(new Error("Unable to read content.")); return; }
streamToBuffer(readStream).then(res, rej);
});
});
});
await processEntry({ fileName }, readContent);
} catch (e) {
rej(e);
}
zipfile.readEntry();
await processEntry({ fileName }, readContent);
} catch (e) {
rej(e);
}
zipfile.readEntry();
})();
});
zipfile.on("end", res);
zipfile.on("error", rej);

18
eslint.config.mjs
View File

@ -49,7 +49,23 @@ const mainConfig = [
argsIgnorePattern: "^_",
varsIgnorePattern: "^_"
}
]
],
// Catch mishandled promises at the type level. `no-misused-promises` flags a Promise used
// where a non-Promise is expected (e.g. a boolean conditional or a void-returning callback);
// `no-floating-promises` flags promises whose result/rejection is silently discarded.
// Warn repo-wide (large existing backlog, mostly in apps/client); errored on the server below.
"@typescript-eslint/no-misused-promises": "warn",
"@typescript-eslint/no-floating-promises": "warn"
}
},
{
// The server is the security-sensitive surface (auth, sync, API), so enforce the promise
// rules as errors here — an unawaited async call in a conditional or callback can silently
// change control flow.
files: ["apps/server/**/*.{js,ts,mjs,cjs,tsx}"],
rules: {
"@typescript-eslint/no-misused-promises": "error",
"@typescript-eslint/no-floating-promises": "error"
}
},
{