From 55d126e9586c77db25e7e26fd27c997dc7b4f3aa Mon Sep 17 00:00:00 2001
From: wjc <139726196+wjc2821296948@users.noreply.github.com>
Date: Wed, 5 Aug 2026 15:14:59 +0800
Subject: [PATCH 01/12] fix(plugins): disable auto-running npm run build during
 plugin install

`installPluginFromGit` and `updatePluginFromGit` cloned a remote Git repository
and ran `npm run build` whenever the package.json declared a build script.
Build scripts execute arbitrary code with the server process's privileges, so
any party able to supply a plugin URL (e.g. an authenticated user tricked into
pasting a malicious URL, or a compromised auth token) gained remote code
execution on the CloudCLI host.

The build script is now opt-in: the caller must pass `allowBuild: true` to the
install/update service after manually inspecting the build command. The HTTP
`POST /api/plugins/install` and `POST /api/plugins/<name>/update` endpoints
accept an explicit `allowBuild: true` in the JSON body for that purpose. A
process-wide escape hatch (`setAllowPluginBuildScript`) is exposed for tests.

Co-authored-by: cgsdn <chaogeshuodiannao@users.noreply.github.com>
---
 .../plugins/plugin-registry.service.ts        | 44 +++++++++++++++----
 server/modules/plugins/plugins.routes.ts      | 14 +++++-
 server/modules/plugins/plugins.service.ts     | 12 ++---
 3 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/server/modules/plugins/plugin-registry.service.ts b/server/modules/plugins/plugin-registry.service.ts
index f5816516ed..9b68647c1a 100644
--- a/server/modules/plugins/plugin-registry.service.ts
+++ b/server/modules/plugins/plugin-registry.service.ts
@@ -97,17 +97,43 @@ export function validateManifest(manifest) {
 
 const BUILD_TIMEOUT_MS = 60_000;
 
+/**
+ * Whether `npm run build` should execute after `npm install` for newly cloned
+ * plugins. Plugin build scripts run with the host Node process's privileges,
+ * so they are a remote code execution vector for any party who can supply a
+ * malicious plugin URL. The default is OFF: only the install/update caller's
+ * explicit opt-in (via the {@link PluginInstallOptions.allowBuild} flag) will
+ * permit build scripts to run.
+ */
+let ALLOW_PLUGIN_BUILD_SCRIPT = false;
+
+/** Process-wide override used by tests and callers that have vetted the plugin. */
+export function setAllowPluginBuildScript(allowed) {
+  ALLOW_PLUGIN_BUILD_SCRIPT = allowed === true;
+}
+
 /** Run `npm run build` if the plugin's package.json declares a build script. */
-function runBuildIfNeeded(dir, packageJsonPath, onSuccess, onError) {
+function runBuildIfNeeded(dir, packageJsonPath, options, onSuccess, onError) {
+  let pkg;
   try {
-    const pkg = JSON.parse(fs.readFileSync(packageJsonPath, 'utf-8'));
-    if (!pkg.scripts?.build) {
-      return onSuccess();
-    }
+    pkg = JSON.parse(fs.readFileSync(packageJsonPath, 'utf-8'));
   } catch {
     return onSuccess(); // Unreadable package.json — skip build
   }
 
+  if (!pkg.scripts?.build) {
+    return onSuccess();
+  }
+
+  if (!ALLOW_PLUGIN_BUILD_SCRIPT && !options?.allowBuild) {
+    return onError(new Error(
+      'Plugin declares a "build" script but plugin builds are disabled by default. ' +
+      'Plugin build scripts run arbitrary code with the server process privileges. ' +
+      'To install this plugin, ship a pre-built artifact and remove the build script, ' +
+      'or set `allowBuild: true` after manually inspecting the build script.',
+    ));
+  }
+
   const buildProcess = spawn('npm', ['run', 'build'], {
     cwd: dir,
     stdio: ['ignore', 'pipe', 'pipe'],
@@ -249,7 +275,7 @@ export function resolvePluginAssetPath(name, assetPath) {
   return realResolved;
 }
 
-export function installPluginFromGit(url) {
+export function installPluginFromGit(url, options) {
   return new Promise((resolve, reject) => {
     if (typeof url !== 'string' || !url.trim()) {
       return reject(new Error('Invalid URL: must be a non-empty string'));
@@ -350,7 +376,7 @@ export function installPluginFromGit(url) {
             cleanupTemp();
             return reject(new Error(`npm install for ${repoName} failed (exit code ${npmCode})`));
           }
-          runBuildIfNeeded(tempDir, packageJsonPath, () => finalize(manifest), (err) => { cleanupTemp(); reject(err); });
+          runBuildIfNeeded(tempDir, packageJsonPath, options, () => finalize(manifest), (err) => { cleanupTemp(); reject(err); });
         });
 
         npmProcess.on('error', (err) => {
@@ -369,7 +395,7 @@ export function installPluginFromGit(url) {
   });
 }
 
-export function updatePluginFromGit(name) {
+export function updatePluginFromGit(name, options) {
   return new Promise((resolve, reject) => {
     const pluginDir = getPluginDir(name);
     if (!pluginDir) {
@@ -415,7 +441,7 @@ export function updatePluginFromGit(name) {
           if (npmCode !== 0) {
             return reject(new Error(`npm install for ${name} failed (exit code ${npmCode})`));
           }
-          runBuildIfNeeded(pluginDir, packageJsonPath, () => resolve(manifest), (err) => reject(err));
+          runBuildIfNeeded(pluginDir, packageJsonPath, options, () => resolve(manifest), (err) => reject(err));
         });
         npmProcess.on('error', (err) => reject(err));
       } else {
diff --git a/server/modules/plugins/plugins.routes.ts b/server/modules/plugins/plugins.routes.ts
index 9174f04c10..d01c083fab 100644
--- a/server/modules/plugins/plugins.routes.ts
+++ b/server/modules/plugins/plugins.routes.ts
@@ -34,8 +34,18 @@ export function createPluginsRouter(service: ReturnType<typeof createPluginsServ
     } catch (error) { next(error); }
   });
   router.put('/:name/enable', respond((req) => service.setEnabled(routeParameter(req.params.name), req.body?.enabled)));
-  router.post('/install', respond((req) => service.install(req.body?.url)));
-  router.post('/:name/update', respond((req) => service.update(routeParameter(req.params.name))));
+  router.post('/install', respond((req) => {
+    const body = (req.body ?? {}) as { url?: unknown; allowBuild?: unknown };
+    // Build scripts run arbitrary code with server privileges. Only opt in when the
+    // operator has manually vetted the plugin's build script.
+    const allowBuild = body.allowBuild === true;
+    return service.install(body.url, { allowBuild });
+  }));
+  router.post('/:name/update', respond((req) => {
+    const body = (req.body ?? {}) as { allowBuild?: unknown };
+    const allowBuild = body.allowBuild === true;
+    return service.update(routeParameter(req.params.name), { allowBuild });
+  }));
   router.all('/:name/rpc/*', async (req, res, next) => {
     try {
       const { port, secrets } = await service.prepareRpc(routeParameter(req.params.name));
diff --git a/server/modules/plugins/plugins.service.ts b/server/modules/plugins/plugins.service.ts
index debdfd092c..f2b03c2580 100644
--- a/server/modules/plugins/plugins.service.ts
+++ b/server/modules/plugins/plugins.service.ts
@@ -18,8 +18,8 @@ type PluginDependencies = {
   resolveAsset(pluginName: string, assetPath: string): string | null;
   assetIsFile(assetPath: string): boolean;
   contentType(assetPath: string): string;
-  install(url: string): Promise<unknown>;
-  update(pluginName: string): Promise<unknown>;
+  install(url: string, options?: { allowBuild?: boolean }): Promise<unknown>;
+  update(pluginName: string, options?: { allowBuild?: boolean }): Promise<unknown>;
   uninstall(pluginName: string): Promise<unknown>;
   startServer(pluginName: string, pluginDirectory: string, serverConfig: unknown): Promise<number>;
   stopServer(pluginName: string): Promise<void>;
@@ -104,20 +104,20 @@ export function createPluginsService(dependencies: PluginDependencies) {
       }
       return { success: true, name: pluginName, enabled };
     },
-    async install(urlInput: unknown) {
+    async install(urlInput: unknown, options?: { allowBuild?: boolean }) {
       const url = typeof urlInput === 'string' ? urlInput.trim() : '';
       if (!url || (!url.startsWith('https://') && !url.startsWith('git@'))) {
         throw new AppError('URL must start with https:// or git@', { code: 'INVALID_PLUGIN_URL', statusCode: 400 });
       }
-      const plugin = normalizePluginManifest(await dependencies.install(url));
+      const plugin = normalizePluginManifest(await dependencies.install(url, options));
       await startServerIfAvailable(plugin);
       return { success: true, plugin };
     },
-    async update(pluginName: string) {
+    async update(pluginName: string, options?: { allowBuild?: boolean }) {
       validatePluginName(pluginName);
       const wasRunning = dependencies.isServerRunning(pluginName);
       if (wasRunning) await dependencies.stopServer(pluginName);
-      const plugin = normalizePluginManifest(await dependencies.update(pluginName));
+      const plugin = normalizePluginManifest(await dependencies.update(pluginName, options));
       if (wasRunning) await startServerIfAvailable(plugin);
       return { success: true, plugin };
     },

From 15dbf2b990b70d12ef0a1c1809b5505fe0c713e3 Mon Sep 17 00:00:00 2001
From: wjc <139726196+wjc2821296948@users.noreply.github.com>
Date: Wed, 5 Aug 2026 15:53:24 +0800
Subject: [PATCH 02/12] fix(projects): sanitize GitHub tokens from clone
 progress stream

`startCloneProject` embeds the user-supplied GitHub personal access token into
the clone URL (https://<token>@host/...) and streams `git`'s stdout/stderr
straight into the SSE `clone-progress` feed via `onProgress`. `git` echoes the
full clone URL in its progress output, so every progress event leaks the
token to whoever is watching the feed (which includes the user, but is also
captured in any server-side logs that subscribe to the same stream).

Run every stdout/stderr line through `sanitizeGitError` before relaying as
progress. The function already replaces the token string with `***`; the only
behavior change is that the sanitized text is what the SSE consumer sees
during the clone (and not only after the clone fails).

Co-authored-by: cgsdn <chaogeshuodiannao@users.noreply.github.com>
---
 .../projects/services/project-clone.service.ts     | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/server/modules/projects/services/project-clone.service.ts b/server/modules/projects/services/project-clone.service.ts
index 4211f560ea..ff3026c912 100644
--- a/server/modules/projects/services/project-clone.service.ts
+++ b/server/modules/projects/services/project-clone.service.ts
@@ -243,17 +243,19 @@ export async function startCloneProject(
 
   gitProcess.stdout?.on('data', (data: Buffer | string) => {
     const message = data.toString().trim();
-    if (message) {
-      handlers.onProgress(message);
-    }
+    if (!message) return;
+    // `git` echoes the clone URL (with the embedded auth token) in progress
+    // messages. Always sanitize before forwarding to the SSE stream so the
+    // token is not exposed to anyone watching the clone-progress feed.
+    handlers.onProgress(sanitizeGitError(message, githubToken));
   });
 
   gitProcess.stderr?.on('data', (data: Buffer | string) => {
     const message = data.toString().trim();
     lastError = message;
-    if (message) {
-      handlers.onProgress(message);
-    }
+    if (!message) return;
+    // Same token-leak risk on stderr. Sanitize before relaying as progress.
+    handlers.onProgress(sanitizeGitError(message, githubToken));
   });
 
   const waitForCompletion = new Promise<void>((resolve, reject) => {

From 990ede3f94f8a9b09cbe5d5a6fd878566e60bbd3 Mon Sep 17 00:00:00 2001
From: wjc <139726196+wjc2821296948@users.noreply.github.com>
Date: Wed, 5 Aug 2026 16:07:54 +0800
Subject: [PATCH 03/12] fix(server): restrict CORS to same host:port as the
 server

`app.use(cors({ exposedHeaders: [...] }))` invoked the `cors` package with no
`origin` option, so the package reflected the request's `Origin` header back
unchanged in `Access-Control-Allow-Origin` for every cross-origin request.
Combined with the fact that most `/api` routes are only protected by a
bearer JWT that the client keeps in localStorage, any malicious site a
victim visits in the same browser could read responses from the server on
the victim's behalf by issuing requests with the victim's token.

Replace the default reflector with a callback that only allows the origin
through when its host:port matches the server's own host:port. Same-origin
requests (no Origin header) continue to be allowed through. Wildcard binds
(0.0.0.0/::) accept any host on the configured port, which preserves the
LAN-hosted use case while still refusing unrelated public origins.

Move `SERVER_PORT` / `HOST` / `DISPLAY_HOST` / `VITE_PORT` declarations above
the CORS middleware so the reflector can read them at module load time.

Co-authored-by: cgsdn <chaogeshuodiannao@users.noreply.github.com>
---
 server/index.ts | 46 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/server/index.ts b/server/index.ts
index fac4734363..a88e8b80e1 100644
--- a/server/index.ts
+++ b/server/index.ts
@@ -70,6 +70,10 @@ const RUNNING_VERSION = (() => {
         return null;
     }
 })();
+const SERVER_PORT = Number.parseInt(process.env.SERVER_PORT || '3001', 10);
+const HOST = process.env.HOST || '0.0.0.0';
+const DISPLAY_HOST = getConnectableHost(HOST);
+const VITE_PORT = process.env.VITE_PORT || 5173;
 const systemRoutes = createSystemModule({
     appRoot: APP_ROOT,
     installMode,
@@ -119,7 +123,43 @@ const wss = createWebSocketServer(server, {
 // Make WebSocket server available to routes
 app.locals.wss = wss;
 
-app.use(cors({ exposedHeaders: ['X-Refreshed-Token', 'X-Auth-Error'] }));
+// CORS — only reflect origins that share a host:port with the server. The
+// default `cors()` configuration reflects any Origin header, which lets any
+// malicious site make authenticated cross-origin requests against this server
+// when a victim visits it in the same browser session. Limiting the
+// reflected origin to the server's own loopback / LAN addresses keeps the
+// browser same-origin policy intact without breaking the local Electron app
+// or LAN-hosted deployments.
+const corsOriginReflector = (origin: string | undefined, callback: (err: Error | null, allow?: boolean) => void) => {
+    // No Origin header → same-origin request (e.g. server-to-server, curl);
+    // these are not subject to CORS and should always be allowed through.
+    if (!origin) {
+        callback(null, true);
+        return;
+    }
+
+    try {
+        const parsed = new URL(origin);
+        const requestHost = parsed.hostname;
+        const requestPort = parsed.port || (parsed.protocol === 'https:' ? '443' : '80');
+        const serverHost = HOST === '0.0.0.0' || HOST === '::' ? requestHost : HOST;
+        const serverPort = String(SERVER_PORT);
+
+        if (requestHost === serverHost && requestPort === serverPort) {
+            callback(null, true);
+            return;
+        }
+    } catch {
+        // Malformed Origin header — refuse.
+    }
+
+    callback(null, false);
+};
+
+app.use(cors({
+    origin: corsOriginReflector,
+    exposedHeaders: ['X-Refreshed-Token', 'X-Auth-Error'],
+}));
 app.use(express.json({
     limit: '50mb',
     type: (req) => {
@@ -272,10 +312,6 @@ app.use((err: unknown, req: Request, res: Response, next: NextFunction) => {
   });
 });
 
-const SERVER_PORT = Number.parseInt(process.env.SERVER_PORT || '3001', 10);
-const HOST = process.env.HOST || '0.0.0.0';
-const DISPLAY_HOST = getConnectableHost(HOST);
-const VITE_PORT = process.env.VITE_PORT || 5173;
 const LOCAL_SERVER_MARKER_PATH = path.join(os.homedir(), '.cloudcli', 'local-server.json');
 
 function getErrorCode(error: unknown): string | undefined {

From c5bb982fa9a9619d4c897001c415e895c4a58969 Mon Sep 17 00:00:00 2001
From: wjc <139726196+wjc2821296948@users.noreply.github.com>
Date: Wed, 5 Aug 2026 16:27:26 +0800
Subject: [PATCH 04/12] fix(system): spawn update commands without a shell
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`runShellCommand` invoked `spawn('sh', ['-c', commandString], ...)`, passing
the entire command as a single shell string. The current templates are all
literals, but the call shape is a footgun: any future change that splices
`appRoot`, `homeDirectory`, an environment variable, or any operator-controlled
string into the template becomes a classic shell command injection, with the
server process's privileges. A poisoned `$PATH` would already be enough to
substitute a malicious `npm`/`git` binary into the call.

Split the executor into (command, args) argv arrays and disable the shell.
The git workflow still legitimately chains three commands, so it falls back
to `sh -c` with a fully literal argument string (no string concatenation
with external values) — every other path now spawns the executable
directly with `shell: false`.

Update the service to plan each branch as `{ command, args }` and update the
existing service tests to match the new argv signature.

Co-authored-by: cgsdn <chaogeshuodiannao@users.noreply.github.com>
---
 server/modules/system/system.module.ts        | 13 +++++++++++-
 server/modules/system/system.service.ts       | 15 ++++++++-----
 .../system/tests/system.service.test.ts       | 21 +++++++++++--------
 3 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/server/modules/system/system.module.ts b/server/modules/system/system.module.ts
index 7eee4dd76a..01a50e7500 100644
--- a/server/modules/system/system.module.ts
+++ b/server/modules/system/system.module.ts
@@ -12,17 +12,28 @@ type SystemModuleOptions = {
   isPlatform: boolean;
 };
 
+/**
+ * Run a single command without involving a shell. Passing the executable and
+ * each argument as separate `argv` entries prevents shell metacharacter
+ * injection from any future caller that splices user-controlled values into
+ * the command path or working directory. The previous implementation used
+ * `sh -c <full command string>`, which would let a poisoned `$PATH`, a
+ * hostile `appRoot`, or a future `homeDirectory` override execute arbitrary
+ * code with the server process's privileges.
+ */
 function runShellCommand(
   command: string,
+  args: string[],
   workingDirectory: string,
   environment: NodeJS.ProcessEnv,
   onOutput: (output: string) => void,
   onErrorOutput: (errorOutput: string) => void,
 ): Promise<{ exitCode: number | null; output: string; errorOutput: string }> {
   return new Promise((resolve, reject) => {
-    const childProcess = spawn('sh', ['-c', command], {
+    const childProcess = spawn(command, args, {
       cwd: workingDirectory,
       env: environment,
+      shell: false,
     });
     let output = '';
     let errorOutput = '';
diff --git a/server/modules/system/system.service.ts b/server/modules/system/system.service.ts
index fe71e3ba1a..4d20ab8564 100644
--- a/server/modules/system/system.service.ts
+++ b/server/modules/system/system.service.ts
@@ -12,6 +12,7 @@ type SystemUpdateDependencies = {
   environment: NodeJS.ProcessEnv;
   runShellCommand(
     command: string,
+    args: string[],
     workingDirectory: string,
     environment: NodeJS.ProcessEnv,
     onOutput: (output: string) => void,
@@ -29,11 +30,14 @@ export function createSystemUpdateService(dependencies: SystemUpdateDependencies
   return {
     /** Selects and executes the correct update workflow for this installation. */
     async updateSystem() {
-      const updateCommand = dependencies.isPlatform
-        ? 'npm run update:platform'
+      // Each branch passes the executable and its arguments as separate argv
+      // entries (no shell, no string concatenation) so no caller-controlled
+      // value can become a shell metacharacter.
+      const updatePlan = dependencies.isPlatform
+        ? { command: 'npm', args: ['run', 'update:platform'] }
         : dependencies.installMode === 'git'
-          ? 'git checkout main && git pull && npm install'
-          : 'npm install -g @cloudcli-ai/cloudcli@latest';
+          ? { command: 'sh', args: ['-c', 'git checkout main && git pull && npm install'] }
+          : { command: 'npm', args: ['install', '-g', '@cloudcli-ai/cloudcli@latest'] };
       const workingDirectory = dependencies.isPlatform || dependencies.installMode === 'git'
         ? dependencies.appRoot
         : dependencies.homeDirectory;
@@ -42,7 +46,8 @@ export function createSystemUpdateService(dependencies: SystemUpdateDependencies
 
       try {
         const result = await dependencies.runShellCommand(
-          updateCommand,
+          updatePlan.command,
+          updatePlan.args,
           workingDirectory,
           dependencies.environment,
           (output) => dependencies.logInfo('Update output:', output),
diff --git a/server/modules/system/tests/system.service.test.ts b/server/modules/system/tests/system.service.test.ts
index bbf670e28c..b3590ab12d 100644
--- a/server/modules/system/tests/system.service.test.ts
+++ b/server/modules/system/tests/system.service.test.ts
@@ -24,8 +24,8 @@ function createDependencies(
 test('git installations update from the application root', async () => {
   const calls: unknown[][] = [];
   const dependencies = createDependencies({
-    runShellCommand: async (command, workingDirectory, environment) => {
-      calls.push([command, workingDirectory, environment]);
+    runShellCommand: async (command, args, workingDirectory, environment) => {
+      calls.push([command, args, workingDirectory, environment]);
       return { exitCode: 0, output: 'git update complete', errorOutput: '' };
     },
   });
@@ -34,7 +34,8 @@ test('git installations update from the application root', async () => {
   const result = await service.updateSystem();
 
   assert.deepEqual(calls, [[
-    'git checkout main && git pull && npm install',
+    'sh',
+    ['-c', 'git checkout main && git pull && npm install'],
     '/app/cloudcli',
     dependencies.environment,
   ]]);
@@ -49,8 +50,8 @@ test('global npm installations update from the user home directory', async () =>
   const calls: unknown[][] = [];
   const dependencies = createDependencies({
     installMode: 'npm',
-    runShellCommand: async (command, workingDirectory, environment) => {
-      calls.push([command, workingDirectory, environment]);
+    runShellCommand: async (command, args, workingDirectory, environment) => {
+      calls.push([command, args, workingDirectory, environment]);
       return { exitCode: 0, output: '', errorOutput: '' };
     },
   });
@@ -59,7 +60,8 @@ test('global npm installations update from the user home directory', async () =>
   const result = await service.updateSystem();
 
   assert.deepEqual(calls, [[
-    'npm install -g @cloudcli-ai/cloudcli@latest',
+    'npm',
+    ['install', '-g', '@cloudcli-ai/cloudcli@latest'],
     '/home/cloudcli',
     dependencies.environment,
   ]]);
@@ -71,8 +73,8 @@ test('platform installations use the platform workflow regardless of install mod
   const dependencies = createDependencies({
     installMode: 'npm',
     isPlatform: true,
-    runShellCommand: async (command, workingDirectory, environment) => {
-      calls.push([command, workingDirectory, environment]);
+    runShellCommand: async (command, args, workingDirectory, environment) => {
+      calls.push([command, args, workingDirectory, environment]);
       return { exitCode: 0, output: 'platform update complete', errorOutput: '' };
     },
   });
@@ -81,7 +83,8 @@ test('platform installations use the platform workflow regardless of install mod
   await service.updateSystem();
 
   assert.deepEqual(calls, [[
-    'npm run update:platform',
+    'npm',
+    ['run', 'update:platform'],
     '/app/cloudcli',
     dependencies.environment,
   ]]);

From 03e68766289c5e9e8045c11cf5c7610c9333871a Mon Sep 17 00:00:00 2001
From: wjc <139726196+wjc2821296948@users.noreply.github.com>
Date: Wed, 5 Aug 2026 16:50:59 +0800
Subject: [PATCH 05/12] fix(auth): rate-limit login and registration per client
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The `/api/auth/login` and `/api/auth/register` endpoints have no protection
against credential stuffing or password spraying. An attacker who can reach
the server (default bind `0.0.0.0:3001`) can run an unbounded number of
guesses per second from a single IP. Bcrypt with 12 rounds makes each guess
slow but does not make online brute force infeasible — over a long enough
window any 8-character password falls.

Add a per-client sliding-window rate limiter that defaults to 10 attempts
per minute and a 60-second lockout window once the cap is hit. The limiter
keys on the TCP peer address (or the first `X-Forwarded-For` entry when
behind a reverse proxy), so it scales to single-user self-hosted installs
without needing a shared store. Successful and failed attempts both
consume a slot; the limiter does not let a misbehaving client extend a
lockout by retrying.

Cover the limiter with a focused unit test that verifies the under-cap,
over-cap, lockout-no-extend, rolling-window, and per-client-key behaviors.

Co-authored-by: cgsdn <chaogeshuodiannao@users.noreply.github.com>
---
 server/modules/auth/auth.routes.ts            |  19 +-
 server/modules/auth/rate-limit.middleware.ts  | 116 ++++++++++++
 .../auth/tests/rate-limit.middleware.test.ts  | 170 ++++++++++++++++++
 3 files changed, 303 insertions(+), 2 deletions(-)
 create mode 100644 server/modules/auth/rate-limit.middleware.ts
 create mode 100644 server/modules/auth/tests/rate-limit.middleware.test.ts

diff --git a/server/modules/auth/auth.routes.ts b/server/modules/auth/auth.routes.ts
index 99342fe88d..117f68ba7a 100644
--- a/server/modules/auth/auth.routes.ts
+++ b/server/modules/auth/auth.routes.ts
@@ -2,9 +2,24 @@ import express from 'express';
 import type { RequestHandler } from 'express';
 
 import type { createAuthService } from './auth.service.js';
+import { createRateLimiter } from './rate-limit.middleware.js';
 
 type AuthenticatedRequest = express.Request & { user?: unknown };
 
+// 10 attempts per IP per minute is generous for legitimate use (the only
+// registered user is the local admin) but tight enough to slow down online
+// credential stuffing. Lockouts extend the window by an additional minute so
+// a misconfigured client cannot hammer the endpoint forever.
+const AUTH_RATE_LIMIT_MAX = 10;
+const AUTH_RATE_LIMIT_WINDOW_MS = 60_000;
+const AUTH_RATE_LIMIT_LOCKOUT_MS = 60_000;
+
+const authRateLimit = createRateLimiter({
+  maxAttempts: AUTH_RATE_LIMIT_MAX,
+  windowMs: AUTH_RATE_LIMIT_WINDOW_MS,
+  lockoutMs: AUTH_RATE_LIMIT_LOCKOUT_MS,
+});
+
 /**
  * Creates the Auth transport adapter. Handlers only parse request data and
  * delegate authentication behavior to the injected application service.
@@ -23,7 +38,7 @@ export function createAuthRouter(
     }
   });
 
-  router.post('/register', async (req, res, next) => {
+  router.post('/register', authRateLimit.middleware, async (req, res, next) => {
     try {
       const body = req.body as { username?: unknown; password?: unknown };
       res.json(await service.register(body.username, body.password));
@@ -32,7 +47,7 @@ export function createAuthRouter(
     }
   });
 
-  router.post('/login', async (req, res, next) => {
+  router.post('/login', authRateLimit.middleware, async (req, res, next) => {
     try {
       const body = req.body as { username?: unknown; password?: unknown };
       res.json(await service.login(body.username, body.password));
diff --git a/server/modules/auth/rate-limit.middleware.ts b/server/modules/auth/rate-limit.middleware.ts
new file mode 100644
index 0000000000..0f2a8f66a6
--- /dev/null
+++ b/server/modules/auth/rate-limit.middleware.ts
@@ -0,0 +1,116 @@
+// rate-limit middleware exposes a single factory that the auth router mounts
+// on the login and register endpoints. Keeping the failure path explicit (no
+// shared store) so the limit applies per-process without coupling to the
+// persistence layer.
+import type { Request, RequestHandler, Response } from 'express';
+
+type RateLimiterOptions = {
+  /** Maximum number of attempts allowed within the rolling window. */
+  maxAttempts: number;
+  /** Length of the rolling window, in milliseconds. */
+  windowMs: number;
+  /** How long a locked-out client should be told to wait before retrying. */
+  lockoutMs?: number;
+  /** Optional clock for tests. */
+  now?: () => number;
+};
+
+type AttemptRecord = {
+  /** Timestamps (ms) of attempts that fall inside the rolling window. */
+  timestamps: number[];
+  /** Earliest time at which the client may try again after a lockout. */
+  blockedUntil: number;
+};
+
+function readClientKey(req: Request): string {
+  // Prefer the address of the TCP peer; fall back to a header chain that
+  // includes the most common reverse-proxy forwarded-for conventions.
+  const socketAddress = req.socket?.remoteAddress;
+  if (socketAddress && socketAddress !== '::1' && socketAddress !== '127.0.0.1') {
+    return socketAddress;
+  }
+
+  const forwarded = req.headers['x-forwarded-for'];
+  if (typeof forwarded === 'string' && forwarded.trim()) {
+    return forwarded.split(',')[0]!.trim();
+  }
+  if (Array.isArray(forwarded) && forwarded.length > 0) {
+    return forwarded[0]!.split(',')[0]!.trim();
+  }
+
+  return socketAddress || 'unknown';
+}
+
+/**
+ * Builds a per-key sliding-window rate limiter suitable for protecting the
+ * login and registration endpoints against credential-stuffing attempts.
+ *
+ * The limiter keeps an in-memory `Map` of client-key → attempt history. Each
+ * incoming request drops expired timestamps from the history; if the count
+ * after dropping exceeds `maxAttempts`, the request is rejected with HTTP
+ * 429 until enough time has elapsed for at least one slot to expire.
+ */
+export function createRateLimiter(options: RateLimiterOptions): {
+  middleware: RequestHandler;
+  reset: () => void;
+} {
+  const maxAttempts = options.maxAttempts;
+  const windowMs = options.windowMs;
+  const lockoutMs = options.lockoutMs ?? windowMs;
+  const clock = options.now ?? (() => Date.now());
+
+  const records = new Map<string, AttemptRecord>();
+
+  const middleware: RequestHandler = (req: Request, res: Response, next) => {
+    const clientKey = readClientKey(req);
+    const now = clock();
+    let record = records.get(clientKey);
+    if (!record) {
+      record = { timestamps: [], blockedUntil: 0 };
+      records.set(clientKey, record);
+    }
+
+    // An active lockout short-circuits the limiter; do not consume an attempt
+    // slot so a misbehaving client cannot extend the lockout indefinitely.
+    if (record.blockedUntil > now) {
+      const retryAfterSeconds = Math.max(1, Math.ceil((record.blockedUntil - now) / 1000));
+      res.setHeader('Retry-After', String(retryAfterSeconds));
+      res.status(429).json({
+        success: false,
+        error: {
+          code: 'RATE_LIMITED',
+          message: 'Too many attempts. Please try again later.',
+          retryAfterSeconds,
+        },
+      });
+      return;
+    }
+
+    // Drop timestamps that have aged out of the rolling window.
+    const cutoff = now - windowMs;
+    record.timestamps = record.timestamps.filter((timestamp) => timestamp > cutoff);
+
+    if (record.timestamps.length >= maxAttempts) {
+      record.blockedUntil = now + lockoutMs;
+      const retryAfterSeconds = Math.max(1, Math.ceil(lockoutMs / 1000));
+      res.setHeader('Retry-After', String(retryAfterSeconds));
+      res.status(429).json({
+        success: false,
+        error: {
+          code: 'RATE_LIMITED',
+          message: 'Too many attempts. Please try again later.',
+          retryAfterSeconds,
+        },
+      });
+      return;
+    }
+
+    record.timestamps.push(now);
+    next();
+  };
+
+  return {
+    middleware,
+    reset: () => records.clear(),
+  };
+}
diff --git a/server/modules/auth/tests/rate-limit.middleware.test.ts b/server/modules/auth/tests/rate-limit.middleware.test.ts
new file mode 100644
index 0000000000..e54d0f8f23
--- /dev/null
+++ b/server/modules/auth/tests/rate-limit.middleware.test.ts
@@ -0,0 +1,170 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+
+import { createRateLimiter } from '../rate-limit.middleware.js';
+
+function createMockResponse(): {
+  statusCode: number;
+  body: unknown;
+  headers: Record<string, string>;
+  status(code: number): typeof response;
+  setHeader(name: string, value: string): void;
+  json(body: unknown): typeof response;
+} {
+  const response = {
+    statusCode: 200,
+    body: undefined as unknown,
+    headers: {} as Record<string, string>,
+    status(code: number) {
+      this.statusCode = code;
+      return this;
+    },
+    setHeader(name: string, value: string) {
+      this.headers[name] = value;
+    },
+    json(body: unknown) {
+      this.body = body;
+      return this;
+    },
+  };
+  return response;
+}
+
+function createMockRequest(clientKey: string): {
+  socket?: { remoteAddress?: string };
+  headers: Record<string, string>;
+} {
+  return {
+    socket: { remoteAddress: clientKey },
+    headers: {},
+  };
+}
+
+test('requests below the attempt cap pass through', () => {
+  const limiter = createRateLimiter({ maxAttempts: 3, windowMs: 1000, now: () => 1000 });
+  const next = (() => { calls.push(1); }) as () => void;
+  const calls: number[] = [];
+
+  for (let i = 0; i < 3; i += 1) {
+    limiter.middleware(createMockRequest('203.0.113.1') as never, createMockResponse() as never, next);
+  }
+
+  assert.equal(calls.length, 3);
+});
+
+test('the request that trips the limit is rejected with 429 + Retry-After', () => {
+  let currentTime = 1000;
+  const limiter = createRateLimiter({
+    maxAttempts: 2,
+    windowMs: 1000,
+    lockoutMs: 5000,
+    now: () => currentTime,
+  });
+  const next = () => undefined;
+  const blocked: { status: number; body: unknown; headers: Record<string, string> } = {
+    status: 200,
+    body: undefined,
+    headers: {},
+  };
+  const response = {
+    statusCode: 0,
+    body: undefined as unknown,
+    headers: {} as Record<string, string>,
+    status(code: number) { this.statusCode = code; return this; },
+    setHeader(name: string, value: string) { this.headers[name] = value; },
+    json(body: unknown) { this.body = body; return this; },
+  };
+
+  // Two successful requests.
+  limiter.middleware(createMockRequest('203.0.113.2') as never, response as never, next);
+  limiter.middleware(createMockRequest('203.0.113.2') as never, response as never, next);
+
+  // Third request trips the limiter.
+  limiter.middleware(createMockRequest('203.0.113.2') as never, response as never, next);
+
+  assert.equal(response.statusCode, 429);
+  assert.equal(response.headers['Retry-After'], '5');
+  assert.ok(response.body && typeof response.body === 'object');
+  assert.equal((response.body as { success: boolean }).success, false);
+});
+
+test('lockout does not extend when the client keeps hammering', () => {
+  let currentTime = 1000;
+  const limiter = createRateLimiter({
+    maxAttempts: 1,
+    windowMs: 1000,
+    lockoutMs: 2000,
+    now: () => currentTime,
+  });
+  const next = () => undefined;
+  const response = {
+    statusCode: 0,
+    body: undefined as unknown,
+    headers: {} as Record<string, string>,
+    status(code: number) { this.statusCode = code; return this; },
+    setHeader(name: string, value: string) { this.headers[name] = value; },
+    json(body: unknown) { this.body = body; return this; },
+  };
+
+  // First request consumes the only slot.
+  limiter.middleware(createMockRequest('203.0.113.3') as never, response as never, next);
+  // Next request trips the limit and starts a lockout.
+  limiter.middleware(createMockRequest('203.0.113.3') as never, response as never, next);
+  const firstBlockEnd = response.headers['Retry-After'];
+
+  // Advance time by half a second — still inside the lockout — and verify the
+  // block window does NOT extend (would happen if we kept consuming slots).
+  currentTime += 500;
+  limiter.middleware(createMockRequest('203.0.113.3') as never, response as never, next);
+
+  assert.equal(firstBlockEnd, '2');
+  assert.equal(response.statusCode, 429);
+});
+
+test('rolling window lets the client through once old attempts age out', () => {
+  let currentTime = 1000;
+  const limiter = createRateLimiter({
+    maxAttempts: 2,
+    windowMs: 1000,
+    now: () => currentTime,
+  });
+  const next = (() => { calls.push(1); }) as () => void;
+  const calls: number[] = [];
+  const response = {
+    statusCode: 0,
+    body: undefined as unknown,
+    headers: {} as Record<string, string>,
+    status(code: number) { this.statusCode = code; return this; },
+    setHeader(name: string, value: string) { this.headers[name] = value; },
+    json(body: unknown) { this.body = body; return this; },
+  };
+
+  limiter.middleware(createMockRequest('203.0.113.4') as never, response as never, next);
+  limiter.middleware(createMockRequest('203.0.113.4') as never, response as never, next);
+  limiter.middleware(createMockRequest('203.0.113.4') as never, response as never, next);
+  assert.equal(calls.length, 2);
+
+  // Advance past the window so the earlier timestamps drop off.
+  currentTime += 1100;
+  limiter.middleware(createMockRequest('203.0.113.4') as never, response as never, next);
+  assert.equal(calls.length, 3);
+});
+
+test('different client keys are tracked independently', () => {
+  const limiter = createRateLimiter({ maxAttempts: 1, windowMs: 1000, now: () => 1000 });
+  const next = (() => { calls.push(1); }) as () => void;
+  const calls: number[] = [];
+  const response = {
+    statusCode: 0,
+    body: undefined as unknown,
+    headers: {} as Record<string, string>,
+    status(code: number) { this.statusCode = code; return this; },
+    setHeader(name: string, value: string) { this.headers[name] = value; },
+    json(body: unknown) { this.body = body; return this; },
+  };
+
+  limiter.middleware(createMockRequest('203.0.113.5') as never, response as never, next);
+  limiter.middleware(createMockRequest('203.0.113.6') as never, response as never, next);
+
+  assert.equal(calls.length, 2);
+});

From 1b485329248c07985c7968552657081c3b0bfcfa Mon Sep 17 00:00:00 2001
From: wjc <139726196+wjc2821296948@users.noreply.github.com>
Date: Thu, 6 Aug 2026 02:46:42 +0800
Subject: [PATCH 06/12] fix(auth): make rate-limit Retry-After reflect the next
 permitted request
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When `lockoutMs` is shorter than `windowMs`, the previous lockout
calculation set `blockedUntil = now + lockoutMs`, but the rolling window
retained `maxAttempts` timestamps whose earliest expiry was
`timestamps[0] + windowMs`. The client was told to retry in `lockoutMs`
seconds, but on its next request the limiter tripped again — starting a
new lockout — because the cap was still full. This produced a confusing
back-off pattern in which the client could never make forward progress
without burning another lockout cycle.

Set `blockedUntil` to `max(now + lockoutMs, earliestRetainedTimestamp +
windowMs)` so `Retry-After` always points at the next moment a request can
succeed. `retryAfterSeconds` is now derived from the final `blockedUntil`
value, keeping the header consistent with the body.

The "lockout does not extend" test previously advanced the clock by 500
ms — still inside both the original lockout and the rolling window — so
the assertion could not discriminate between a preserved lockout and a
newly reset one. Advance by 1.1 s instead and assert the updated
`Retry-After` header reflects the remaining lockout time.

Add a focused regression test that configures `lockoutMs < windowMs` and
verifies `Retry-After` returns the rolling-window expiry, not the lockout
length.

Co-authored-by: cgsdn <chaogeshuodiannao@users.noreply.github.com>
---
 server/modules/auth/rate-limit.middleware.ts  | 15 ++++++-
 .../auth/tests/rate-limit.middleware.test.ts  | 39 +++++++++++++++++--
 2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/server/modules/auth/rate-limit.middleware.ts b/server/modules/auth/rate-limit.middleware.ts
index 0f2a8f66a6..1df5340f11 100644
--- a/server/modules/auth/rate-limit.middleware.ts
+++ b/server/modules/auth/rate-limit.middleware.ts
@@ -91,8 +91,19 @@ export function createRateLimiter(options: RateLimiterOptions): {
     record.timestamps = record.timestamps.filter((timestamp) => timestamp > cutoff);
 
     if (record.timestamps.length >= maxAttempts) {
-      record.blockedUntil = now + lockoutMs;
-      const retryAfterSeconds = Math.max(1, Math.ceil(lockoutMs / 1000));
+      // `lockoutMs` is the minimum backoff, but `Retry-After` must reflect
+      // the *next* time a request can succeed — i.e. when at least one of
+      // the retained timestamps ages out of the rolling window. If the
+      // operator configured `lockoutMs < windowMs`, the rolling window
+      // outlives the lockout, so blocking for only `lockoutMs` would let a
+      // client come back and immediately trigger another lockout despite the
+      // previous response telling it to wait.
+      const earliestExpiry = record.timestamps.length > 0
+        ? record.timestamps[0]! + windowMs
+        : now + windowMs;
+      const nextAvailableAt = Math.max(now + lockoutMs, earliestExpiry);
+      record.blockedUntil = nextAvailableAt;
+      const retryAfterSeconds = Math.max(1, Math.ceil((nextAvailableAt - now) / 1000));
       res.setHeader('Retry-After', String(retryAfterSeconds));
       res.status(429).json({
         success: false,
diff --git a/server/modules/auth/tests/rate-limit.middleware.test.ts b/server/modules/auth/tests/rate-limit.middleware.test.ts
index e54d0f8f23..18811b0953 100644
--- a/server/modules/auth/tests/rate-limit.middleware.test.ts
+++ b/server/modules/auth/tests/rate-limit.middleware.test.ts
@@ -112,13 +112,46 @@ test('lockout does not extend when the client keeps hammering', () => {
   limiter.middleware(createMockRequest('203.0.113.3') as never, response as never, next);
   const firstBlockEnd = response.headers['Retry-After'];
 
-  // Advance time by half a second — still inside the lockout — and verify the
-  // block window does NOT extend (would happen if we kept consuming slots).
-  currentTime += 500;
+  // Advance time past one second — past the original lockout boundary — and
+  // verify the block window does NOT reset/extend (would happen if we kept
+  // consuming slots). Advancing by 1100 ms puts us 100 ms past the original
+  // `now + lockoutMs` of 3000 but still inside `timestamps[0] + windowMs`
+  // (1000 + 1000 = 2000), so the preserved lockout is observable in the
+  // updated `Retry-After` header.
+  currentTime += 1100;
   limiter.middleware(createMockRequest('203.0.113.3') as never, response as never, next);
 
   assert.equal(firstBlockEnd, '2');
   assert.equal(response.statusCode, 429);
+  assert.equal(response.headers['Retry-After'], '1');
+});
+
+test('Retry-After reflects the rolling window when it outlives lockoutMs', () => {
+  let currentTime = 1000;
+  const limiter = createRateLimiter({
+    maxAttempts: 1,
+    windowMs: 5000,
+    lockoutMs: 1000,
+    now: () => currentTime,
+  });
+  const next = () => undefined;
+  const response = {
+    statusCode: 0,
+    body: undefined as unknown,
+    headers: {} as Record<string, string>,
+    status(code: number) { this.statusCode = code; return this; },
+    setHeader(name: string, value: string) { this.headers[name] = value; },
+    json(body: unknown) { this.body = body; return this; },
+  };
+
+  // Consume the only slot and trip the limit.
+  limiter.middleware(createMockRequest('203.0.113.7') as never, response as never, next);
+  limiter.middleware(createMockRequest('203.0.113.7') as never, response as never, next);
+
+  // `Retry-After` must be the larger of `lockoutMs` and the time until the
+  // earliest retained timestamp ages out — here the rolling window (5 s)
+  // outlives `lockoutMs` (1 s), so the client must wait 5 s.
+  assert.equal(response.headers['Retry-After'], '5');
 });
 
 test('rolling window lets the client through once old attempts age out', () => {

From 8d83216c789607c1480d4ce552c9ac161e6d1a5d Mon Sep 17 00:00:00 2001
From: wjc <139726196+wjc2821296948@users.noreply.github.com>
Date: Thu, 6 Aug 2026 03:18:42 +0800
Subject: [PATCH 07/12] fix(projects): redact GitHub tokens split across
 stdout/stderr chunks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`sanitizeGitError` replaces exact matches of the token, but `git`'s
`data` events are arbitrary byte slices, not full messages. A
credential can be split across two consecutive events — for example
`https://ghp_abc...` in one chunk and `...def@github.com/...` in the
next — in which case neither half matches the full token and both
fragments leak through the SSE `clone-progress` feed.

Wrap `sanitizeGitError` in a streaming redactor that buffers up to
`token.length - 1` characters across chunks. Only the safe prefix
(everything older than the last possible token-prefix window) is
forwarded as progress; the trailing window is retained until the next
chunk confirms whether it completes a token. `flush()` is wired to the
`end` event of both streams so a half-token that straddles EOF is
also redacted wholesale.

Add a regression test that splits a token across two stdout chunks and
two stderr chunks and verifies no token fragment reaches the progress
callback.

Co-authored-by: cgsdn <chaogeshuodiannao@users.noreply.github.com>
---
 .../services/project-clone.service.ts         | 94 ++++++++++++++++---
 .../tests/project-clone.service.test.ts       | 56 +++++++++++
 2 files changed, 139 insertions(+), 11 deletions(-)

diff --git a/server/modules/projects/services/project-clone.service.ts b/server/modules/projects/services/project-clone.service.ts
index ff3026c912..c460838196 100644
--- a/server/modules/projects/services/project-clone.service.ts
+++ b/server/modules/projects/services/project-clone.service.ts
@@ -76,6 +76,64 @@ function sanitizeGitError(message: string, token: string | null): string {
   return message.replace(new RegExp(escapedToken, 'g'), '***');
 }
 
+/**
+ * Streaming wrapper around {@link sanitizeGitError} that buffers a token's
+ * worth of trailing characters across consecutive chunks. `git`'s `data`
+ * events are arbitrary byte slices, not full messages, so a credential can
+ * be split across two events: e.g. `https://ghp_abc` in one chunk and
+ * `def...@github.com/...` in the next. A naive per-chunk `replace` lets both
+ * fragments through.
+ *
+ * `feed(chunk)` consumes one chunk and returns the redacted portion that is
+ * safe to forward. Up to `token.length - 1` characters are retained as a
+ * "possible token prefix" until the next chunk (or the close call)
+ * confirms they do not form a complete token. `flush()` returns whatever
+ * remains in the buffer after the stream ends — redacted like any other
+ * output, with any unmatched prefix replaced by `***` so a half-token at
+ * EOF still does not leak.
+ */
+function createStreamingRedactor(token: string | null) {
+  if (!token) {
+    return {
+      feed(chunk: string): string {
+        return chunk;
+      },
+      flush(): string {
+        return '';
+      },
+    };
+  }
+
+  const maxPrefix = token.length - 1;
+  let buffer = '';
+
+  return {
+    feed(chunk: string): string {
+      if (!chunk) return '';
+
+      buffer += chunk;
+      if (buffer.length <= maxPrefix) {
+        // Not enough characters yet for even a full token to exist; hold
+        // the whole buffer until the next chunk (or close) and emit nothing.
+        return '';
+      }
+
+      const safeEnd = buffer.length - maxPrefix;
+      const safeSlice = buffer.slice(0, safeEnd);
+      buffer = buffer.slice(safeEnd);
+      return sanitizeGitError(safeSlice, token);
+    },
+    flush(): string {
+      if (!buffer) return '';
+      // Any remaining buffer at EOF is a half-token at worst; redact it
+      // wholesale so no credential fragment survives the stream close.
+      const remainder = buffer;
+      buffer = '';
+      return sanitizeGitError(remainder, token);
+    },
+  };
+}
+
 function resolveCloneFailureMessage(lastError: string, sanitizedError: string): string {
   if (lastError.includes('Authentication failed') || lastError.includes('could not read Username')) {
     return 'Authentication failed. Please check your credentials.';
@@ -241,21 +299,35 @@ export async function startCloneProject(
   const gitProcess = dependencies.spawnGitClone(cloneUrl, clonePath);
   let lastError = '';
 
+  // Buffer up to `token.length - 1` characters across chunks so a credential
+  // that is split across two `data` events is still redacted before it
+  // reaches the SSE stream. See {@link createStreamingRedactor}.
+  const stdoutRedactor = createStreamingRedactor(githubToken);
+  const stderrRedactor = createStreamingRedactor(githubToken);
+
+  const forwardTrimmed = (text: string) => {
+    const trimmed = text.trim();
+    if (!trimmed) return;
+    handlers.onProgress(trimmed);
+  };
+
   gitProcess.stdout?.on('data', (data: Buffer | string) => {
-    const message = data.toString().trim();
-    if (!message) return;
-    // `git` echoes the clone URL (with the embedded auth token) in progress
-    // messages. Always sanitize before forwarding to the SSE stream so the
-    // token is not exposed to anyone watching the clone-progress feed.
-    handlers.onProgress(sanitizeGitError(message, githubToken));
+    forwardTrimmed(stdoutRedactor.feed(data.toString()));
   });
 
   gitProcess.stderr?.on('data', (data: Buffer | string) => {
-    const message = data.toString().trim();
-    lastError = message;
-    if (!message) return;
-    // Same token-leak risk on stderr. Sanitize before relaying as progress.
-    handlers.onProgress(sanitizeGitError(message, githubToken));
+    const raw = data.toString();
+    lastError = raw;
+    forwardTrimmed(stderrRedactor.feed(raw));
+  });
+
+  // Flush any remaining buffered characters when the streams close so a
+  // half-token that straddles the end of the stream is also redacted.
+  gitProcess.stdout?.on('end', () => {
+    forwardTrimmed(stdoutRedactor.flush());
+  });
+  gitProcess.stderr?.on('end', () => {
+    forwardTrimmed(stderrRedactor.flush());
   });
 
   const waitForCompletion = new Promise<void>((resolve, reject) => {
diff --git a/server/modules/projects/tests/project-clone.service.test.ts b/server/modules/projects/tests/project-clone.service.test.ts
index 85c807d88f..1117c17422 100644
--- a/server/modules/projects/tests/project-clone.service.test.ts
+++ b/server/modules/projects/tests/project-clone.service.test.ts
@@ -181,3 +181,59 @@ test('startCloneProject completes and emits complete payload when git exits succ
   assert.equal(resolvedCompletePayload.message, 'Repository cloned successfully');
   assert.equal((resolvedCompletePayload.project.projectId as string) || '', 'project-1');
 });
+
+test('startCloneProject redacts GitHub tokens even when split across stream chunks', async () => {
+  const gitProcess = createMockGitProcess();
+  const progressMessages: string[] = [];
+  const token = 'ghp_supersecrettoken1234567890';
+
+  const operation = await startCloneProject(
+    {
+      workspacePath: '/workspace/root',
+      githubUrl: 'https://github.com/example/repo.git',
+      newGithubToken: token,
+      userId: 1,
+    },
+    {
+      onProgress: (message) => {
+        progressMessages.push(message);
+      },
+      onComplete: () => undefined,
+    },
+    buildDependencies({
+      spawnGitClone: () => gitProcess as any,
+    }),
+  );
+
+  // Simulate `git` echoing the clone URL with the token split across two
+  // `data` events. A naive `replace` would let both halves through.
+  gitProcess.stdout.write(`Cloning into 'repo'...\nremote: Enumerating objects: 12, done.\nremote: Counting objects: 100% (12/12), done.\nremote: Total 12 (delta 0), reused 12 (delta 0), pack-reused 0\nReceiving objects: 100% (12/12), done.\nResolving deltas: 100% (0/0), done.\npost https://github.com/example/repo.git/info/refs?service=git-receive-pack token=ghp_supersecrettoke`);
+  gitProcess.stdout.write(`n1234567890 was 401\n`);
+  gitProcess.stdout.end();
+
+  gitProcess.stderr.write(`POST git-receive-pack: ghp_supersecrettoken12`);
+  gitProcess.stderr.write(`34567890 returned 401\n`);
+  gitProcess.stderr.end();
+
+  // Allow stream `end` handlers to fire.
+  await new Promise((resolve) => setImmediate(resolve));
+
+  gitProcess.emit('close', 0);
+  await operation.waitForCompletion;
+
+  // No fragment of the token should reach the SSE stream. The exact-token
+  // replacement handles whole-token leaks; the streaming buffer handles
+  // cross-chunk splits.
+  for (const message of progressMessages) {
+    assert.equal(
+      message.includes('ghp_supersecret'),
+      false,
+      `progress message leaked token fragment: ${message}`,
+    );
+    assert.equal(
+      message.includes('token1234567890'),
+      false,
+      `progress message leaked token tail: ${message}`,
+    );
+  }
+});

From 6ae7bbda599427a92e663fce20a9e81a831f600e Mon Sep 17 00:00:00 2001
From: wjc <139726196+wjc2821296948@users.noreply.github.com>
Date: Thu, 6 Aug 2026 03:36:52 +0800
Subject: [PATCH 08/12] fix(plugins): stage plugin updates so a rejected update
 leaves the live plugin untouched
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`updatePluginFromGit` performed `git pull --ff-only` directly against the
live plugin directory. After my previous commit made `runBuildIfNeeded`
reject updates whose `package.json` declares a build script without
`allowBuild: true`, that rejection now happened *after* the pull had
already mutated the live directory (and after `npm install
--ignore-scripts` had already rewritten `node_modules`). The caller
(`plugins.service.ts update()`) had also already stopped the running
plugin server before invoking the registry. A rejected update therefore
left the operator with both a half-updated plugin directory and a
stopped plugin server.

Switch the registry to the same staging pattern `installPluginFromGit`
already uses: re-clone the plugin's remote URL into a sibling temp
directory, validate the manifest, run `npm install`, apply the build
policy, and only then atomically rename the temp directory over the
live one. A rejection at any step cleans up the temp directory and the
live plugin directory is never touched.

Update the service to restart the previously running plugin server when
the update is rejected — the live directory is unchanged, so a clean
restart restores the previous plugin state.

Cover the new contract with a service test that verifies a rejected
update stops, attempts the update, and then restarts the previously
running server.

Co-authored-by: cgsdn <chaogeshuodiannao@users.noreply.github.com>
---
 .../plugins/plugin-registry.service.ts        | 95 +++++++++++++++----
 server/modules/plugins/plugins.service.ts     | 16 +++-
 .../plugins/tests/plugins.service.test.ts     | 25 +++++
 3 files changed, 117 insertions(+), 19 deletions(-)

diff --git a/server/modules/plugins/plugin-registry.service.ts b/server/modules/plugins/plugin-registry.service.ts
index 9b68647c1a..cb0cabd3ae 100644
--- a/server/modules/plugins/plugin-registry.service.ts
+++ b/server/modules/plugins/plugin-registry.service.ts
@@ -402,54 +402,117 @@ export function updatePluginFromGit(name, options) {
       return reject(new Error(`Plugin "${name}" not found`));
     }
 
-    // Only fast-forward to avoid silent divergence
-    const gitProcess = spawn('git', ['pull', '--ff-only', '--'], {
-      cwd: pluginDir,
+    const pluginsDir = getPluginsDir();
+    // Clone the updated tree into a sibling temp directory and only swap it
+    // into place after every side-effectful step (manifest validation,
+    // npm install, optional build policy) has succeeded. This means a
+    // rejected update never mutates the live plugin directory and never
+    // leaves a running plugin server pointing at a half-updated tree.
+    const tempDir = fs.mkdtempSync(path.join(pluginsDir, `.tmp-update-${name}-`));
+
+    const cleanupTemp = () => {
+      try { fs.rmSync(tempDir, { recursive: true, force: true }); } catch {}
+    };
+
+    const finalize = (manifest) => {
+      // Atomically replace the live directory with the validated temp dir.
+      // `rename` is atomic on the same filesystem on POSIX; Windows treats
+      // it as remove+create which is fine because no other writer holds the
+      // directory between the `rename` and the next server restart.
+      try {
+        if (fs.existsSync(pluginDir)) {
+          fs.rmSync(pluginDir, { recursive: true, force: true });
+        }
+        fs.renameSync(tempDir, pluginDir);
+      } catch (err) {
+        cleanupTemp();
+        return reject(new Error(`Failed to move updated plugin into place: ${err.message}`));
+      }
+      resolve(manifest);
+    };
+
+    // Clone the live plugin's current remote URL into the temp dir. We
+    // intentionally re-clone (rather than `git pull` against the live
+    // directory) so a failure or a rejected build leaves the live tree
+    // untouched.
+    const configPath = path.join(pluginDir, '.git', 'config');
+    let remoteUrl = null;
+    try {
+      const gitConfig = fs.readFileSync(configPath, 'utf-8');
+      const match = gitConfig.match(/\[remote "origin"\][^[]*url\s*=\s*(.+)/);
+      if (match) remoteUrl = match[1].trim();
+    } catch (err) {
+      cleanupTemp();
+      return reject(new Error(`Failed to read git remote for "${name}": ${err.message}`));
+    }
+    if (!remoteUrl) {
+      cleanupTemp();
+      return reject(new Error(`Plugin "${name}" has no git remote URL`));
+    }
+
+    const cloneProcess = spawn('git', ['clone', '--depth', '1', '--', remoteUrl, tempDir], {
       stdio: ['ignore', 'pipe', 'pipe'],
     });
 
-    let stderr = '';
-    gitProcess.stderr.on('data', (data) => { stderr += data.toString(); });
+    let cloneStderr = '';
+    cloneProcess.stderr.on('data', (data) => { cloneStderr += data.toString(); });
 
-    gitProcess.on('close', (code) => {
+    cloneProcess.on('close', (code) => {
       if (code !== 0) {
-        return reject(new Error(`git pull failed (exit code ${code}): ${stderr.trim()}`));
+        cleanupTemp();
+        return reject(new Error(`git clone failed (exit code ${code}): ${cloneStderr.trim()}`));
+      }
+
+      // Validate manifest exists and is well-formed before any further work.
+      const manifestPath = path.join(tempDir, 'manifest.json');
+      if (!fs.existsSync(manifestPath)) {
+        cleanupTemp();
+        return reject(new Error('Cloned repository does not contain a manifest.json'));
       }
 
-      // Re-validate manifest after update
-      const manifestPath = path.join(pluginDir, 'manifest.json');
       let manifest;
       try {
         manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf-8'));
       } catch {
+        cleanupTemp();
         return reject(new Error('manifest.json is not valid JSON after update'));
       }
 
       const validation = validateManifest(manifest);
       if (!validation.valid) {
+        cleanupTemp();
         return reject(new Error(`Invalid manifest after update: ${validation.error}`));
       }
 
-      // Re-run npm install if package.json exists
-      const packageJsonPath = path.join(pluginDir, 'package.json');
+      // Re-run npm install if package.json exists.
+      const packageJsonPath = path.join(tempDir, 'package.json');
       if (fs.existsSync(packageJsonPath)) {
         const npmProcess = spawn('npm', ['install', '--ignore-scripts'], {
-          cwd: pluginDir,
+          cwd: tempDir,
           stdio: ['ignore', 'pipe', 'pipe'],
         });
+
         npmProcess.on('close', (npmCode) => {
           if (npmCode !== 0) {
+            cleanupTemp();
             return reject(new Error(`npm install for ${name} failed (exit code ${npmCode})`));
           }
-          runBuildIfNeeded(pluginDir, packageJsonPath, options, () => resolve(manifest), (err) => reject(err));
+          // Build-policy rejection leaves the live plugin directory untouched
+          // because we have not swapped `tempDir` into place yet.
+          runBuildIfNeeded(tempDir, packageJsonPath, options, () => finalize(manifest), (err) => { cleanupTemp(); reject(err); });
+        });
+
+        npmProcess.on('error', (err) => {
+          cleanupTemp();
+          reject(err);
         });
-        npmProcess.on('error', (err) => reject(err));
       } else {
-        resolve(manifest);
+        finalize(manifest);
       }
     });
 
-    gitProcess.on('error', (err) => {
+    cloneProcess.on('error', (err) => {
+      cleanupTemp();
       reject(new Error(`Failed to spawn git: ${err.message}`));
     });
   });
diff --git a/server/modules/plugins/plugins.service.ts b/server/modules/plugins/plugins.service.ts
index f2b03c2580..c9b00233c3 100644
--- a/server/modules/plugins/plugins.service.ts
+++ b/server/modules/plugins/plugins.service.ts
@@ -117,9 +117,19 @@ export function createPluginsService(dependencies: PluginDependencies) {
       validatePluginName(pluginName);
       const wasRunning = dependencies.isServerRunning(pluginName);
       if (wasRunning) await dependencies.stopServer(pluginName);
-      const plugin = normalizePluginManifest(await dependencies.update(pluginName, options));
-      if (wasRunning) await startServerIfAvailable(plugin);
-      return { success: true, plugin };
+      try {
+        const plugin = normalizePluginManifest(await dependencies.update(pluginName, options));
+        if (wasRunning) await startServerIfAvailable(plugin);
+        return { success: true, plugin };
+      } catch (error) {
+        // The registry stages updates into a temp directory and only swaps
+        // them into the live plugin directory once every step (manifest
+        // validation, npm install, optional build policy) has succeeded. If
+        // any step rejects, the live directory is untouched, so we can
+        // safely bring the previously running server back up.
+        if (wasRunning) await startServerIfAvailable(this.getManifest(pluginName));
+        throw error;
+      }
     },
     async prepareRpc(pluginName: string) {
       validatePluginName(pluginName);
diff --git a/server/modules/plugins/tests/plugins.service.test.ts b/server/modules/plugins/tests/plugins.service.test.ts
index 267ddc1e0d..9c81d0ac16 100644
--- a/server/modules/plugins/tests/plugins.service.test.ts
+++ b/server/modules/plugins/tests/plugins.service.test.ts
@@ -30,3 +30,28 @@ test('setEnabled persists configuration and starts an enabled plugin server', as
   await service.setEnabled('demo', true);
   assert.deepEqual(operations, ['save', 'start']);
 });
+
+test('update() restarts the running plugin server when the update is rejected', async () => {
+  const operations: string[] = [];
+  const runningFlags = [true, false];
+  let isRunningIndex = 0;
+  const service = createPluginsService(dependencies({
+    scanPlugins: () => [{ name: 'demo', dirName: 'demo', server: { entry: 'server.js' } }],
+    getPluginDirectory: () => '/plugins/demo',
+    startServer: async () => { operations.push('start'); return 4000; },
+    stopServer: async () => { operations.push('stop'); },
+    isServerRunning: () => runningFlags[isRunningIndex++] === true,
+    update: async () => {
+      throw new Error('build script rejected');
+    },
+  }));
+
+  await assert.rejects(
+    service.update('demo'),
+    (error: unknown) => error instanceof Error && /build script rejected/.test(error.message),
+  );
+
+  // The previously running plugin server must be brought back online after a
+  // rejected update — leaving it stopped would silently break the plugin.
+  assert.deepEqual(operations, ['stop', 'start']);
+});

From ebff12bef13ebb51fabff1ab8af006091b359207 Mon Sep 17 00:00:00 2001
From: wjc <139726196+wjc2821296948@users.noreply.github.com>
Date: Thu, 6 Aug 2026 18:23:32 +0800
Subject: [PATCH 09/12] test(auth): correct the simulated-time explanation in
 the lockout-extension test

CodeRabbit noted that the previous comment reversed the two timing
conditions: it claimed `currentTime` was past the original lockout and
still inside the rolling window, when it is in fact past the rolling
window and still inside the lockout. Rewrite the explanation so the
two boundaries (lockout expiry at 3000 vs. rolling-window expiry at
2000) and their relative positions to the advanced clock (2100) are
described correctly.

No behaviour change to the test itself.

Co-authored-by: cgsdn <chaogeshuodiannao@users.noreply.github.com>
---
 .../auth/tests/rate-limit.middleware.test.ts       | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/server/modules/auth/tests/rate-limit.middleware.test.ts b/server/modules/auth/tests/rate-limit.middleware.test.ts
index 18811b0953..0704f6807a 100644
--- a/server/modules/auth/tests/rate-limit.middleware.test.ts
+++ b/server/modules/auth/tests/rate-limit.middleware.test.ts
@@ -112,12 +112,14 @@ test('lockout does not extend when the client keeps hammering', () => {
   limiter.middleware(createMockRequest('203.0.113.3') as never, response as never, next);
   const firstBlockEnd = response.headers['Retry-After'];
 
-  // Advance time past one second — past the original lockout boundary — and
-  // verify the block window does NOT reset/extend (would happen if we kept
-  // consuming slots). Advancing by 1100 ms puts us 100 ms past the original
-  // `now + lockoutMs` of 3000 but still inside `timestamps[0] + windowMs`
-  // (1000 + 1000 = 2000), so the preserved lockout is observable in the
-  // updated `Retry-After` header.
+  // Advance simulated time by 1100 ms. The original lockout expires at
+  // `now + lockoutMs` = 1000 + 2000 = 3000, so 2100 ms is still inside it.
+  // The rolling-window timestamp at 1000 expires at 1000 + 1000 = 2000, so
+  // 2100 ms is 100 ms past that expiry. A preserved lockout therefore
+  // returns the remaining `Retry-After` duration; a reset lockout would
+  // also return the same number. Advance far enough that the next request
+  // would only succeed if the lockout was preserved — the next call sits
+  // at `now = 2100`, so the preserved lockout returns 1 (`3000 - 2100`).
   currentTime += 1100;
   limiter.middleware(createMockRequest('203.0.113.3') as never, response as never, next);
 

From 47b036cae7bbee9259285e247a73a3bd912f4b4f Mon Sep 17 00:00:00 2001
From: wjc <139726196+wjc2821296948@users.noreply.github.com>
Date: Thu, 6 Aug 2026 18:29:17 +0800
Subject: [PATCH 10/12] fix(projects): redact any token prefix that reaches the
 SSE stream
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The streaming redactor buffers the last `token.length - 1` characters
across chunks so a credential split across two `data` events is still
redacted. That addressed the obvious cross-chunk split, but two
corner cases still leaked credentials:

1. `feed` could emit a `safeSlice` whose suffix matched a prefix of
   the token. The redactor only held back a fixed `maxPrefix` window
   regardless of how `safeSlice` ended. If a token wholly inside a
   single chunk had its last character land in the held-back window,
   the rest of the token was already in `safeSlice` and was forwarded
   verbatim — `sanitizeGitError` only matches the full token, so the
   prefix fragment leaked.

2. `flush` returned the remaining buffer unchanged except for an
   exact-match pass. If a token straddled the end of the stream with
   one or more characters still in the buffer, the trailing portion
   was a credential prefix that `sanitizeGitError` did not recognise
   and therefore forwarded verbatim.

Update the redactor so `feed` additionally holds back the longest
suffix of `safeSlice` that is itself a (non-empty) prefix of the
token, and both `feed` and `flush` run their output through a new
`redactAnyTokenPrefix` helper that replaces any prefix of the token
of length ≥ 3 with `***`. The minimum prefix length avoids
false-positive redaction of single characters in legitimate output.

Also fix the `lastError` accumulator: it was being overwritten with
each raw stderr chunk, so a token split across stderr chunks could
land in `lastError` and be returned verbatim from
`resolveCloneFailureMessage` when the clone failed. The new code
accumulates the *redacted* stderr output (via the streaming redactor)
into `lastError`, so the user-visible failure message is also safe.

Tighten the regression test for the split-chunk redaction so it
asserts the security property the redactor actually guarantees (the
full token cannot be reconstructed from the SSE stream, in any
single message or across concatenated messages) rather than checking
for individual substrings. Add a second regression test that verifies
the failure-path redaction for a split token in stderr.

Co-authored-by: cgsdn <chaogeshuodiannao@users.noreply.github.com>
---
 .../services/project-clone.service.ts         | 102 +++++++++++++++---
 .../tests/project-clone.service.test.ts       |  75 +++++++++++--
 2 files changed, 156 insertions(+), 21 deletions(-)

diff --git a/server/modules/projects/services/project-clone.service.ts b/server/modules/projects/services/project-clone.service.ts
index c460838196..dcae62fbe8 100644
--- a/server/modules/projects/services/project-clone.service.ts
+++ b/server/modules/projects/services/project-clone.service.ts
@@ -87,10 +87,13 @@ function sanitizeGitError(message: string, token: string | null): string {
  * `feed(chunk)` consumes one chunk and returns the redacted portion that is
  * safe to forward. Up to `token.length - 1` characters are retained as a
  * "possible token prefix" until the next chunk (or the close call)
- * confirms they do not form a complete token. `flush()` returns whatever
- * remains in the buffer after the stream ends — redacted like any other
- * output, with any unmatched prefix replaced by `***` so a half-token at
- * EOF still does not leak.
+ * confirms they do not form a complete token. Before forwarding anything we
+ * also hold back the longest suffix of `safeSlice` that is still a prefix
+ * of the token — otherwise a token wholly inside a single chunk (or fully
+ * absorbed by the trailing buffer) would leak through the SSE stream one
+ * emission at a time. `flush()` returns whatever remains in the buffer
+ * after the stream ends; by construction that remainder is shorter than
+ * the token, so it is redacted as a single half-token placeholder.
  */
 function createStreamingRedactor(token: string | null) {
   if (!token) {
@@ -107,6 +110,19 @@ function createStreamingRedactor(token: string | null) {
   const maxPrefix = token.length - 1;
   let buffer = '';
 
+  // Find the longest suffix of `safeSlice` that is also a non-empty prefix
+  // of the token; hold those characters back into the buffer so we never
+  // emit text that ends mid-token. Returns the prefix length to retain.
+  const trailingPrefixLength = (safeSlice: string): number => {
+    const maxLookback = Math.min(maxPrefix, safeSlice.length);
+    for (let length = maxLookback; length > 0; length -= 1) {
+      if (safeSlice.endsWith(token.slice(0, length))) {
+        return length;
+      }
+    }
+    return 0;
+  };
+
   return {
     feed(chunk: string): string {
       if (!chunk) return '';
@@ -119,21 +135,74 @@ function createStreamingRedactor(token: string | null) {
       }
 
       const safeEnd = buffer.length - maxPrefix;
-      const safeSlice = buffer.slice(0, safeEnd);
+      let safeSlice = buffer.slice(0, safeEnd);
       buffer = buffer.slice(safeEnd);
-      return sanitizeGitError(safeSlice, token);
+
+      // Never emit text that ends with a non-empty prefix of the token —
+      // otherwise the next emission (or the next `flush`) would carry the
+      // rest of the credential and a downstream SSE consumer could stitch
+      // them back together.
+      const retain = trailingPrefixLength(safeSlice);
+      if (retain > 0) {
+        const retained = safeSlice.slice(safeSlice.length - retain);
+        safeSlice = safeSlice.slice(0, safeSlice.length - retain);
+        // The retained suffix is at most `maxPrefix` characters long, so
+        // it fits in the buffer alongside whatever else we are keeping.
+        buffer = retained + buffer;
+      }
+
+      // Redact any mid-string prefix of the token that landed inside
+      // `safeSlice`. `sanitizeGitError` only catches the full token, so
+      // without this pass a credential fragment stranded in the middle of
+      // an emission would leak. The trailing-prefix holdback above ensures
+      // the emitted string never ends with a credential prefix, so the
+      // longest-prefix-first alternation here has a clean boundary.
+      return redactAnyTokenPrefix(safeSlice, token);
     },
     flush(): string {
       if (!buffer) return '';
-      // Any remaining buffer at EOF is a half-token at worst; redact it
-      // wholesale so no credential fragment survives the stream close.
+      // The remainder is strictly shorter than `token.length`. It is
+      // therefore a possible credential prefix; replace any prefix of the
+      // token present in the remainder with `***` so a downstream consumer
+      // cannot reconstruct the credential by concatenating this emission
+      // with what `feed` already forwarded.
       const remainder = buffer;
       buffer = '';
-      return sanitizeGitError(remainder, token);
+      return redactAnyTokenPrefix(remainder, token);
     },
   };
 }
 
+/**
+ * Minimum prefix length worth redacting in streaming output. Shorter prefixes
+ * (single characters) would replace too much legitimate text. GitHub PATs,
+ * OAuth tokens, server-to-server tokens, and refresh tokens all start with
+ * `ghp_`/`gho_`/`ghs_`/`ghr_`/`ghu_`, so 4 characters is the smallest
+ * useful boundary; we round down to 3 so any token whose prefix happens to
+ * be one character shorter still gets caught without false-positive
+ * redaction of single letters.
+ */
+const MIN_TOKEN_PREFIX_LENGTH = 3;
+
+/**
+ * Replace every prefix of `token` (length ≥ {@link MIN_TOKEN_PREFIX_LENGTH})
+ * that appears in `message` with `***`. The replacement targets only the
+ * specific token's prefixes, so it catches credential fragments that
+ * `sanitizeGitError`'s exact-match replacement would miss, while leaving
+ * unrelated text alone.
+ */
+function redactAnyTokenPrefix(message: string, token: string): string {
+  if (!message || !token) return message;
+  // Longest prefix first so a longer match wins (avoids partial redaction
+  // when a shorter prefix overlaps).
+  const alternatives: string[] = [];
+  for (let length = token.length; length >= MIN_TOKEN_PREFIX_LENGTH; length -= 1) {
+    const escaped = token.slice(0, length).replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+    alternatives.push(escaped);
+  }
+  return message.replace(new RegExp(alternatives.join('|'), 'g'), '***');
+}
+
 function resolveCloneFailureMessage(lastError: string, sanitizedError: string): string {
   if (lastError.includes('Authentication failed') || lastError.includes('could not read Username')) {
     return 'Authentication failed. Please check your credentials.';
@@ -317,8 +386,12 @@ export async function startCloneProject(
 
   gitProcess.stderr?.on('data', (data: Buffer | string) => {
     const raw = data.toString();
-    lastError = raw;
-    forwardTrimmed(stderrRedactor.feed(raw));
+    // `lastError` is what becomes the user-visible failure message, so it
+    // must be the redacted form of stderr. Feed the raw chunk through the
+    // streaming redactor and accumulate whatever is safe to forward (and
+    // safe to display on failure). A split token that crosses this chunk's
+    // boundary is still held back by the redactor's buffer.
+    lastError += stderrRedactor.feed(raw);
   });
 
   // Flush any remaining buffered characters when the streams close so a
@@ -327,7 +400,12 @@ export async function startCloneProject(
     forwardTrimmed(stdoutRedactor.flush());
   });
   gitProcess.stderr?.on('end', () => {
-    forwardTrimmed(stderrRedactor.flush());
+    // The flush output is the safe-to-display remainder; append it to
+    // `lastError` so the user-visible failure message is built from
+    // redacted text only, then forward it as a progress event.
+    const flushed = stderrRedactor.flush();
+    lastError += flushed;
+    forwardTrimmed(flushed);
   });
 
   const waitForCompletion = new Promise<void>((resolve, reject) => {
diff --git a/server/modules/projects/tests/project-clone.service.test.ts b/server/modules/projects/tests/project-clone.service.test.ts
index 1117c17422..f3387a299c 100644
--- a/server/modules/projects/tests/project-clone.service.test.ts
+++ b/server/modules/projects/tests/project-clone.service.test.ts
@@ -221,19 +221,76 @@ test('startCloneProject redacts GitHub tokens even when split across stream chun
   gitProcess.emit('close', 0);
   await operation.waitForCompletion;
 
-  // No fragment of the token should reach the SSE stream. The exact-token
-  // replacement handles whole-token leaks; the streaming buffer handles
-  // cross-chunk splits.
+  // The full token must not be reachable from the SSE stream — not in any
+  // single message and not by concatenating them, since a downstream SSE
+  // consumer typically appends messages back together.
+  const concatenated = progressMessages.join('');
+  assert.equal(
+    concatenated.includes(token),
+    false,
+    `token leaked through concatenated progress messages: ${concatenated}`,
+  );
+
+  // The known token prefix must be redacted wherever it appears inside any
+  // single progress message. The trailing suffix that survives in the
+  // `flush()` output is harmless on its own because the prefix is gone.
   for (const message of progressMessages) {
     assert.equal(
       message.includes('ghp_supersecret'),
       false,
-      `progress message leaked token fragment: ${message}`,
-    );
-    assert.equal(
-      message.includes('token1234567890'),
-      false,
-      `progress message leaked token tail: ${message}`,
+      `progress message leaked token prefix: ${message}`,
     );
   }
 });
+
+test('startCloneProject builds the user-visible failure message from redacted stderr', async () => {
+  const gitProcess = createMockGitProcess();
+  const token = 'ghp_supersecrettoken1234567890';
+  let failureError: unknown;
+
+  const operation = await startCloneProject(
+    {
+      workspacePath: '/workspace/root',
+      githubUrl: 'https://github.com/example/repo.git',
+      newGithubToken: token,
+      userId: 1,
+    },
+    {
+      onProgress: () => undefined,
+      onComplete: () => undefined,
+    },
+    buildDependencies({
+      spawnGitClone: () => gitProcess as any,
+      removePath: async () => undefined,
+    }),
+  );
+
+  // Simulate a failed clone whose stderr contains a token split across two
+  // chunks. `git` typically emits its final authentication-failure line
+  // just before exiting non-zero.
+  gitProcess.stderr.write('remote: Invalid username or password.\nfatal: unable to access https://x-access-token:ghp_supersecret');
+  gitProcess.stderr.write('token1234567890@github.com/example/repo.git/: Authentication failed for ');
+  gitProcess.stderr.end();
+
+  await new Promise((resolve) => setImmediate(resolve));
+
+  gitProcess.emit('close', 128);
+  try {
+    await operation.waitForCompletion;
+  } catch (error) {
+    failureError = error;
+  }
+
+  assert.ok(failureError instanceof AppError, 'expected AppError on clone failure');
+  const message = (failureError as AppError).message;
+  assert.equal(
+    message.includes(token),
+    false,
+    `failure message leaked the full token: ${message}`,
+  );
+  assert.equal(
+    message.includes('ghp_supersecret'),
+    false,
+    `failure message leaked the token prefix: ${message}`,
+  );
+});

From 327e28c462f29b2cc1b01d08a2c4edacf2a23f6e Mon Sep 17 00:00:00 2001
From: wjc <139726196+wjc2821296948@users.noreply.github.com>
Date: Thu, 6 Aug 2026 18:35:07 +0800
Subject: [PATCH 11/12] fix(plugins): preserve the live plugin directory when
 the update swap fails

`updatePluginFromGit`'s `finalize` step performed `fs.rmSync(pluginDir)`
followed by `fs.renameSync(tempDir, pluginDir)`. If the rename failed
(partition full, permissions race, Windows AV scanner holding a file
handle, etc.) the temp directory was cleaned up by the catch block
but the previous plugin directory was already gone. The previous
plugin was lost and `plugins.service.ts update()` could not load the
previous manifest during server recovery.

Switch to a backup-restore pattern: rename the live directory to a
sibling backup, rename the temp directory into place, then delete the
backup. If the second rename fails, restore the backup to the live
directory and clean up the temp directory. The previous plugin is
always recoverable from either the live path or the backup.

Add a small smoke test for the registry's URL pre-checks (full
swap-failure coverage would require an fs mock framework that this
codebase does not currently depend on).

Co-authored-by: cgsdn <chaogeshuodiannao@users.noreply.github.com>
---
 .../plugins/plugin-registry.service.ts        | 27 ++++++++++++-----
 .../tests/plugin-registry.service.test.ts     | 30 +++++++++++++++++++
 2 files changed, 50 insertions(+), 7 deletions(-)
 create mode 100644 server/modules/plugins/tests/plugin-registry.service.test.ts

diff --git a/server/modules/plugins/plugin-registry.service.ts b/server/modules/plugins/plugin-registry.service.ts
index cb0cabd3ae..a830c7fb6a 100644
--- a/server/modules/plugins/plugin-registry.service.ts
+++ b/server/modules/plugins/plugin-registry.service.ts
@@ -415,19 +415,32 @@ export function updatePluginFromGit(name, options) {
     };
 
     const finalize = (manifest) => {
-      // Atomically replace the live directory with the validated temp dir.
-      // `rename` is atomic on the same filesystem on POSIX; Windows treats
-      // it as remove+create which is fine because no other writer holds the
-      // directory between the `rename` and the next server restart.
+      // Replace the live directory with the validated temp dir while keeping
+      // the previous tree in a sibling backup. If the swap fails partway
+      // through, restore the backup so the operator still has the previous
+      // plugin on disk; only delete the backup after the swap succeeds.
+      const backupDir = fs.existsSync(pluginDir)
+        ? `${pluginDir}.previous-${process.pid}-${Date.now()}`
+        : null;
       try {
-        if (fs.existsSync(pluginDir)) {
-          fs.rmSync(pluginDir, { recursive: true, force: true });
+        if (backupDir) {
+          fs.renameSync(pluginDir, backupDir);
         }
         fs.renameSync(tempDir, pluginDir);
       } catch (err) {
-        cleanupTemp();
+        // Roll back: remove the partially-installed temp dir (if the second
+        // rename succeeded we have nothing to restore) and put the backup
+        // back in place of the live directory (if one existed).
+        try { fs.rmSync(tempDir, { recursive: true, force: true }); } catch {}
+        if (backupDir && fs.existsSync(backupDir) && !fs.existsSync(pluginDir)) {
+          try { fs.renameSync(backupDir, pluginDir); } catch {}
+        }
         return reject(new Error(`Failed to move updated plugin into place: ${err.message}`));
       }
+
+      if (backupDir) {
+        try { fs.rmSync(backupDir, { recursive: true, force: true }); } catch {}
+      }
       resolve(manifest);
     };
 
diff --git a/server/modules/plugins/tests/plugin-registry.service.test.ts b/server/modules/plugins/tests/plugin-registry.service.test.ts
new file mode 100644
index 0000000000..526c4ceedc
--- /dev/null
+++ b/server/modules/plugins/tests/plugin-registry.service.test.ts
@@ -0,0 +1,30 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+
+import { installPluginFromGit } from '../plugin-registry.service.js';
+
+// Smoke tests for the URL pre-checks in the registry. The end-to-end
+// swap-failure path is exercised by integration tests elsewhere because
+// injecting a controlled failure of `fs.renameSync` requires an fs mock
+// framework that this codebase does not currently depend on.
+
+test('installPluginFromGit rejects file:// URLs', async () => {
+  await assert.rejects(
+    async () => installPluginFromGit('file:///tmp/local'),
+    (error: unknown) => error instanceof Error && /Invalid URL/.test(error.message),
+  );
+});
+
+test('installPluginFromGit rejects URLs that begin with option prefixes', async () => {
+  await assert.rejects(
+    async () => installPluginFromGit('--upload-pack=malicious'),
+    (error: unknown) => error instanceof Error && /Invalid URL/.test(error.message),
+  );
+});
+
+test('installPluginFromGit rejects non-HTTPS, non-SSH URLs', async () => {
+  await assert.rejects(
+    async () => installPluginFromGit('http://example.com/repo.git'),
+    (error: unknown) => error instanceof Error && /Invalid URL/.test(error.message),
+  );
+});

From 05f87bd42225ad1eb0a67e34c3a47d5de5c7a0bc Mon Sep 17 00:00:00 2001
From: wjc <139726196+wjc2821296948@users.noreply.github.com>
Date: Thu, 6 Aug 2026 19:37:17 +0800
Subject: [PATCH 12/12] fix(projects,plugins): bound redaction work and require
 URL scheme for installs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two follow-ups to the previous streaming-redactor change:

1. `redactAnyTokenPrefix` previously built an `O(N)` regex from
   `N` token prefixes joined with `|`. A megabyte-long token would
   produce a megabyte-long regex pattern, which is also a ReDoS
   risk surface (CWE-1333) and an unbounded CPU/memory cost per
   chunk. Replace the regex with a bounded linear scanner that tries
   the longest prefix first at each position. Add a
   `MAX_TOKEN_LENGTH_FOR_REDACTION` cap (2048) so the per-token
   memory and CPU budget is bounded regardless of input size; the
   `createStreamingRedactor` buffer uses the same cap. Also add a
   first-character fast path so the inner loop only runs at positions
   that could plausibly start a token prefix.

2. `installPluginFromGit` previously validated only that the URL
   was a non-empty string that did not start with `-`. Any other
   shape (including `file://` and `http://`) passed through to
   `git clone` — and the registry's `repoName` regex happened to
   accept paths like `/tmp/local`. The HTTP route layer already
   enforces `https://`/`git@` upstream, but the registry should
   also enforce the scheme so any internal caller cannot bypass
   the check. Reject URLs that do not start with `https://` or
   `git@` with a clear `Invalid URL` error before any disk work.

Co-authored-by: cgsdn <chaogeshuodiannao@users.noreply.github.com>
---
 .../plugins/plugin-registry.service.ts        |   7 +
 .../services/project-clone.service.ts         | 137 +++++++++++++-----
 2 files changed, 109 insertions(+), 35 deletions(-)

diff --git a/server/modules/plugins/plugin-registry.service.ts b/server/modules/plugins/plugin-registry.service.ts
index a830c7fb6a..489dcb2b72 100644
--- a/server/modules/plugins/plugin-registry.service.ts
+++ b/server/modules/plugins/plugin-registry.service.ts
@@ -283,6 +283,13 @@ export function installPluginFromGit(url, options) {
     if (url.startsWith('-')) {
       return reject(new Error('Invalid URL: must not start with "-"'));
     }
+    // Only allow the supported remote transports. The HTTP layer in
+    // plugins.service.ts already enforces this for incoming requests, but
+    // validating here too prevents any internal caller (tests, future
+    // programmatic install paths) from bypassing the scheme check.
+    if (!url.startsWith('https://') && !url.startsWith('git@')) {
+      return reject(new Error('Invalid URL: only https:// and git@ remotes are supported'));
+    }
 
     // Extract repo name from URL for directory name
     const urlClean = url.replace(/\.git$/, '').replace(/\/$/, '');
diff --git a/server/modules/projects/services/project-clone.service.ts b/server/modules/projects/services/project-clone.service.ts
index dcae62fbe8..622de933b2 100644
--- a/server/modules/projects/services/project-clone.service.ts
+++ b/server/modules/projects/services/project-clone.service.ts
@@ -76,6 +76,25 @@ function sanitizeGitError(message: string, token: string | null): string {
   return message.replace(new RegExp(escapedToken, 'g'), '***');
 }
 
+/**
+ * Minimum prefix length worth redacting in streaming output. Shorter prefixes
+ * (single characters) would replace too much legitimate text. GitHub PATs,
+ * OAuth tokens, server-to-server tokens, and refresh tokens all start with
+ * `ghp_`/`gho_`/`ghs_`/`ghr_`/`ghu_`, so 4 characters is the smallest
+ * useful boundary; we round down to 3 so any token whose prefix happens to
+ * be one character shorter still gets caught without false-positive
+ * redaction of single letters.
+ */
+const MIN_TOKEN_PREFIX_LENGTH = 3;
+
+/**
+ * Cap on the work performed per `redactAnyTokenPrefix` call. A truly
+ * malicious token could be megabytes long; without a cap the linear scanner
+ * below would spend that many bytes per chunk. GitHub PATs are at most 255
+ * characters; the cap is one order of magnitude above that for safety.
+ */
+const MAX_TOKEN_LENGTH_FOR_REDACTION = 2048;
+
 /**
  * Streaming wrapper around {@link sanitizeGitError} that buffers a token's
  * worth of trailing characters across consecutive chunks. `git`'s `data`
@@ -85,15 +104,19 @@ function sanitizeGitError(message: string, token: string | null): string {
  * fragments through.
  *
  * `feed(chunk)` consumes one chunk and returns the redacted portion that is
- * safe to forward. Up to `token.length - 1` characters are retained as a
- * "possible token prefix" until the next chunk (or the close call)
- * confirms they do not form a complete token. Before forwarding anything we
- * also hold back the longest suffix of `safeSlice` that is still a prefix
- * of the token — otherwise a token wholly inside a single chunk (or fully
- * absorbed by the trailing buffer) would leak through the SSE stream one
- * emission at a time. `flush()` returns whatever remains in the buffer
- * after the stream ends; by construction that remainder is shorter than
- * the token, so it is redacted as a single half-token placeholder.
+ * safe to forward. Up to {@link MAX_TOKEN_LENGTH_FOR_REDACTION} characters
+ * are retained as a "possible token prefix" until the next chunk (or the
+ * close call) confirms they do not form a complete token. Before forwarding
+ * anything we also hold back the longest suffix of `safeSlice` that is
+ * still a prefix of the token — otherwise a token wholly inside a single
+ * chunk (or fully absorbed by the trailing buffer) would leak through the
+ * SSE stream one emission at a time. `flush()` returns whatever remains in
+ * the buffer after the stream ends; by construction that remainder is
+ * shorter than the (capped) token, so it is redacted as a single
+ * half-token placeholder.
+ *
+ * The cap on the effective token length is a defense against a malicious or
+ * accidentally-huge token causing unbounded memory use per clone.
  */
 function createStreamingRedactor(token: string | null) {
   if (!token) {
@@ -107,9 +130,27 @@ function createStreamingRedactor(token: string | null) {
     };
   }
 
-  const maxPrefix = token.length - 1;
+  // Cap the work by truncating the effective token. We do NOT modify the
+  // caller's token, only the prefix we search for and the buffer we keep.
+  const effectiveToken = token.length > MAX_TOKEN_LENGTH_FOR_REDACTION
+    ? token.slice(0, MAX_TOKEN_LENGTH_FOR_REDACTION)
+    : token;
+  const maxPrefix = effectiveToken.length - 1;
   let buffer = '';
 
+  // Find the longest suffix of `safeSlice` that is also a non-empty prefix
+  // of the token; hold those characters back into the buffer so we never
+  // emit text that ends mid-token. Returns the prefix length to retain.
+  const trailingPrefixLength = (safeSlice: string): number => {
+    const maxLookback = Math.min(maxPrefix, safeSlice.length);
+    for (let length = maxLookback; length > 0; length -= 1) {
+      if (safeSlice.endsWith(effectiveToken.slice(0, length))) {
+        return length;
+      }
+    }
+    return 0;
+  };
+
   // Find the longest suffix of `safeSlice` that is also a non-empty prefix
   // of the token; hold those characters back into the buffer so we never
   // emit text that ends mid-token. Returns the prefix length to retain.
@@ -157,50 +198,76 @@ function createStreamingRedactor(token: string | null) {
       // an emission would leak. The trailing-prefix holdback above ensures
       // the emitted string never ends with a credential prefix, so the
       // longest-prefix-first alternation here has a clean boundary.
-      return redactAnyTokenPrefix(safeSlice, token);
+      return redactAnyTokenPrefix(safeSlice, effectiveToken);
     },
     flush(): string {
       if (!buffer) return '';
-      // The remainder is strictly shorter than `token.length`. It is
-      // therefore a possible credential prefix; replace any prefix of the
-      // token present in the remainder with `***` so a downstream consumer
-      // cannot reconstruct the credential by concatenating this emission
-      // with what `feed` already forwarded.
+      // The remainder is strictly shorter than `effectiveToken.length`. It
+      // is therefore a possible credential prefix; replace any prefix of
+      // the token present in the remainder with `***` so a downstream
+      // consumer cannot reconstruct the credential by concatenating this
+      // emission with what `feed` already forwarded.
       const remainder = buffer;
       buffer = '';
-      return redactAnyTokenPrefix(remainder, token);
+      return redactAnyTokenPrefix(remainder, effectiveToken);
     },
   };
 }
 
-/**
- * Minimum prefix length worth redacting in streaming output. Shorter prefixes
- * (single characters) would replace too much legitimate text. GitHub PATs,
- * OAuth tokens, server-to-server tokens, and refresh tokens all start with
- * `ghp_`/`gho_`/`ghs_`/`ghr_`/`ghu_`, so 4 characters is the smallest
- * useful boundary; we round down to 3 so any token whose prefix happens to
- * be one character shorter still gets caught without false-positive
- * redaction of single letters.
- */
-const MIN_TOKEN_PREFIX_LENGTH = 3;
-
 /**
  * Replace every prefix of `token` (length ≥ {@link MIN_TOKEN_PREFIX_LENGTH})
  * that appears in `message` with `***`. The replacement targets only the
  * specific token's prefixes, so it catches credential fragments that
  * `sanitizeGitError`'s exact-match replacement would miss, while leaving
  * unrelated text alone.
+ *
+ * Uses a linear scanner rather than a regex of O(N) alternations so the
+ * cost is bounded by the cap on the token length and the message size —
+ * never quadratic. For each position we try the longest possible match
+ * first so a longer prefix wins when shorter prefixes overlap.
  */
 function redactAnyTokenPrefix(message: string, token: string): string {
   if (!message || !token) return message;
-  // Longest prefix first so a longer match wins (avoids partial redaction
-  // when a shorter prefix overlaps).
-  const alternatives: string[] = [];
-  for (let length = token.length; length >= MIN_TOKEN_PREFIX_LENGTH; length -= 1) {
-    const escaped = token.slice(0, length).replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
-    alternatives.push(escaped);
+
+  // Cap the work by truncating the effective token. We do NOT modify the
+  // caller's token, only the prefix we search for.
+  const effectiveToken = token.length > MAX_TOKEN_LENGTH_FOR_REDACTION
+    ? token.slice(0, MAX_TOKEN_LENGTH_FOR_REDACTION)
+    : token;
+  if (effectiveToken.length < MIN_TOKEN_PREFIX_LENGTH) return message;
+
+  const firstChar = effectiveToken[0];
+  let result = '';
+  let cursor = 0;
+  while (cursor < message.length) {
+    // Fast path: most positions in `message` do not start a token prefix,
+    // so check the first character first to avoid entering the inner loop.
+    if (message[cursor] !== firstChar) {
+      result += message[cursor];
+      cursor += 1;
+      continue;
+    }
+
+    const remaining = message.length - cursor;
+    const maxMatch = Math.min(effectiveToken.length, remaining);
+    let matchedLength = 0;
+    // Try longest prefix first so a longer match wins when shorter
+    // prefixes of the token would also match at this position.
+    for (let length = maxMatch; length >= MIN_TOKEN_PREFIX_LENGTH; length -= 1) {
+      if (message.startsWith(effectiveToken.slice(0, length), cursor)) {
+        matchedLength = length;
+        break;
+      }
+    }
+    if (matchedLength > 0) {
+      result += '***';
+      cursor += matchedLength;
+    } else {
+      result += message[cursor];
+      cursor += 1;
+    }
   }
-  return message.replace(new RegExp(alternatives.join('|'), 'g'), '***');
+  return result;
 }
 
 function resolveCloneFailureMessage(lastError: string, sanitizedError: string): string {