fix: use two-pass validation in settings PUT to prevent partial writes

Validation now runs on all entries before any database writes.
Previously, clean entries could be written before a later malicious
entry triggered a 400 response.
This commit is contained in:
Siddharth Kumar Sah
2026-03-28 19:08:17 +08:00
parent 8a62093130
commit 813fa6b7e8
2 changed files with 34 additions and 5 deletions
+10 -5
View File
@@ -43,8 +43,8 @@ export async function settingsRoutes(app: FastifyInstance): Promise<void> {
});
}
const now = new Date();
let updatedCount = 0;
// Pass 1: validate all entries before writing any
const entries: Array<{ key: string; strValue: string }> = [];
for (const [key, value] of Object.entries(body)) {
if (typeof key !== "string" || key.length === 0) continue;
@@ -58,6 +58,13 @@ export async function settingsRoutes(app: FastifyInstance): Promise<void> {
});
}
entries.push({ key, strValue });
}
// Pass 2: write all entries now that all have passed validation
const now = new Date();
for (const { key, strValue } of entries) {
// Upsert: insert or update on conflict
const existing = db.select().from(schema.settings).where(eq(schema.settings.key, key)).get();
@@ -69,11 +76,9 @@ export async function settingsRoutes(app: FastifyInstance): Promise<void> {
} else {
db.insert(schema.settings).values({ key, value: strValue }).run();
}
updatedCount++;
}
return reply.send({ ok: true, updatedCount });
return reply.send({ ok: true, updatedCount: entries.length });
});
// GET /api/v1/settings/:key — Get a specific setting
+24
View File
@@ -1420,6 +1420,30 @@ describe("Settings", () => {
expect(body.code).toBe("VALIDATION_ERROR");
});
it("does not partially write entries when a later entry contains HTML tags", async () => {
const cleanKey = `atomicity_test_clean_${Date.now()}`;
const res = await app.inject({
method: "PUT",
url: "/api/v1/settings",
headers: { authorization: `Bearer ${adminToken}` },
payload: {
[cleanKey]: "safe_value",
"<script>xss</script>": "evil",
},
});
expect(res.statusCode).toBe(400);
const body = JSON.parse(res.body);
expect(body.code).toBe("VALIDATION_ERROR");
// The clean entry must NOT have been written
const getRes = await app.inject({
method: "GET",
url: `/api/v1/settings/${cleanKey}`,
headers: { authorization: `Bearer ${adminToken}` },
});
expect(getRes.statusCode).toBe(404);
});
it("allows normal setting values without HTML", async () => {
const res = await app.inject({
method: "PUT",