From c44194d09d9353b0bcf42d0f176ab8e00bb9ee4d Mon Sep 17 00:00:00 2001
From: SandObserver <260779319+SandObserver@users.noreply.github.com>
Date: Sun, 18 Oct 2026 18:38:18 -0400
Subject: [PATCH] Never overwrite an existing icon on upload

safeIconName stopped at the -999 suffix without checking it, so once a
name had 999 numbered copies the next upload overwrote the last one.
It now falls back to a random suffix, and the upload writes with the
exclusive-create flag.
---
 api/src/routes/icons.js           | 4 +++-
 api/test/icon-upload-name.test.js | 9 +++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/api/src/routes/icons.js b/api/src/routes/icons.js
index 0c1b6008..2b29bc7a 100644
--- a/api/src/routes/icons.js
+++ b/api/src/routes/icons.js
@@ -1,3 +1,4 @@
+const crypto = require('crypto');
 const fs = require('fs');
 const path = require('path');
 const { on, json, checkOrigin, getIp, readBodyCapped } = require('../router');
@@ -35,6 +36,7 @@ function safeIconName(dir, raw) {
   for (let n = 2; n <= 999 && fs.existsSync(path.join(dir, candidate)); n++) {
     candidate = `${stem}-${n}${ext}`;
   }
+  if (fs.existsSync(path.join(dir, candidate))) candidate = `${stem}-${crypto.randomBytes(6).toString('hex')}${ext}`;
   return candidate;
 }
 
@@ -219,7 +221,7 @@ on('POST', '/api/icons/upload', async (req, res) => {
     fs.mkdirSync(ICONS_PATH, { recursive: true });
     /* Never the submitted name directly. See safeIconName. */
     const saved = safeIconName(ICONS_PATH, filename);
-    fs.writeFileSync(path.join(ICONS_PATH, saved), fileData);
+    fs.writeFileSync(path.join(ICONS_PATH, saved), fileData, { flag: 'wx' });
     log.audit('icon uploaded', { filename: saved });
     json(res, 200, { ok: true, filename: saved });
   } catch (e) {
diff --git a/api/test/icon-upload-name.test.js b/api/test/icon-upload-name.test.js
index 3823c005..23733437 100644
--- a/api/test/icon-upload-name.test.js
+++ b/api/test/icon-upload-name.test.js
@@ -37,6 +37,15 @@ test('several uploads of the same name each get their own', () => {
   }
 });
 
+test('a name whose numbered copies are all taken still gets a free name', () => {
+  const d = dir();
+  touch(d, 'logo.svg');
+  for (let n = 2; n <= 999; n++) touch(d, `logo-${n}.svg`);
+  const got = safeIconName(d, 'logo.svg');
+  assert.match(got, /^logo-[0-9a-f]{12}\.svg$/);
+  assert.equal(fs.existsSync(path.join(d, got)), false);
+});
+
 test('the extension is preserved when a suffix is added', () => {
   const d = dir();
   touch(d, 'icon.png');