mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
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:
@@ -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
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user