fix(scripts): validate destinations immediately before file writes

- Add explicit FileHandle type annotation for fileHandle variable
- Change fileHandle check from truthy to explicit undefined check
- Change release type from null to undefined for better type safety

Co-authored-by: whyour <22700758+whyour@users.noreply.github.com>

back/api/script.ts

@@ -35,6 +35,23 @@ function isPathAllowed(targetPath: string): boolean {
   );
 }
 
+async function writeScriptFile(
+  targetPath: string,
+  content: string,
+): Promise<boolean> {
+  const target = resolve(targetPath);
+  for (const writableRoot of config.writePathList) {
+    const root = resolve(writableRoot);
+    const prefix = root.endsWith(sep) ? root : root + sep;
+    // Check the actual write target, including filename, at the write boundary.
+    if (!target.startsWith(prefix)) continue;
+    if (!isPathAllowed(target)) return false;
+    await writeFileWithLock(target, content);
+    return true;
+  }
+  return false;
+}
+
 async function existingWritableRealPath(targetPath: string): Promise<string> {
   if (!isPathAllowed(targetPath)) {
     throw new ScriptHistoryError(403, '暂无权限');
@@ -373,7 +390,9 @@ export default (app: Router) => {
             await rmPath(originFilePath);
           }
         }
-        await writeFileWithLock(filePath, content);
+        if (!(await writeScriptFile(filePath, content))) {
+          return res.send({ code: 403, message: t('暂无权限') });
+        }
         return res.send({ code: 200 });
       } catch (e) {
         return historyError(e, res, next);
@@ -513,7 +532,9 @@ export default (app: Router) => {
         if (!isPathAllowed(filePath)) {
           return res.send({ code: 403, message: t('暂无权限') });
         }
-        await writeFileWithLock(filePath, content || '');
+        if (!(await writeScriptFile(filePath, content || ''))) {
+          return res.send({ code: 403, message: t('暂无权限') });
+        }
 
         const scriptService = Container.get(ScriptService);
         const result = await scriptService.runScript(filePath);

back/services/http.ts

@@ -12,7 +12,7 @@ export class HttpServerService {
   async initialize(expressApp: express.Application, port: number) {
     const hostsToTry = [
       config.bindHost,
-      ...(config.bindHost === '::' ? ['0.0.0.0'] : [])
+      ...(config.bindHost === '::' ? ['0.0.0.0'] : []),
     ];
 
     let lastError: Error | null = null;
@@ -20,16 +20,21 @@ export class HttpServerService {
     for (const host of hostsToTry) {
       try {
         const server = await this.tryListen(expressApp, port, host);
-        Logger.debug(`[boot] HTTP service started successfully on ${host}:${port}`);
+        Logger.debug(
+          `[boot] HTTP service started successfully on ${host}:${port}`,
+        );
         metricsService.record('http_service_start', 1, {
           port: port.toString(),
-          host
+          host,
         });
         this.server = server;
         return server;
       } catch (err) {
         lastError = err as Error;
-        Logger.warn(`Failed to bind HTTP on ${host}:${port}, trying next...`, err);
+        Logger.warn(
+          `Failed to bind HTTP on ${host}:${port}, trying next...`,
+          err,
+        );
       }
     }
 
@@ -37,17 +42,28 @@ export class HttpServerService {
     throw lastError || new Error('Failed to start HTTP service');
   }
 
-  private async tryListen(expressApp: express.Application, port: number, host: string): Promise<Server> {
+  private async tryListen(
+    expressApp: express.Application,
+    port: number,
+    host: string,
+  ): Promise<Server> {
     return new Promise((resolve, reject) => {
       // There is one HTTP worker; accepting here avoids primary IPC handoff
       // for every connection. Restore shared listening for custom clusters.
-      const server = expressApp.listen({
-        port,
-        host,
-        exclusive: process.env.QL_HTTP_SHARED_LISTEN !== 'true',
-      }, () => {
-        resolve(server);
-      });
+      const server = expressApp.listen(
+        {
+          port,
+          host,
+          exclusive: process.env.QL_HTTP_SHARED_LISTEN !== 'true',
+        },
+        () => {
+          resolve(server);
+        },
+      );
+
+      server.requestTimeout = 300000;
+      server.headersTimeout = 120000;
+      server.keepAliveTimeout = 65000;
 
       server.on('error', (err: Error) => {
         server.close();

back/shared/utils.ts

@@ -21,7 +21,7 @@ export async function writeFileWithLock(
   }
   if (!(await fileExist(filePath))) {
     const fileHandle = await open(filePath, 'w');
-    fileHandle.close();
+    await fileHandle.close();
   }
   const lockfilePath = getUniqueLockPath(filePath);
 
@@ -34,9 +34,12 @@ export async function writeFileWithLock(
     },
     lockfilePath,
   });
-  await writeFile(filePath, content, { encoding: 'utf8', ...options });
-  if (options?.mode) {
-    await chmod(filePath, options.mode);
+  try {
+    await writeFile(filePath, content, { encoding: 'utf8', ...options });
+    if (options?.mode) {
+      await chmod(filePath, options.mode);
+    }
+  } finally {
+    await release();
   }
-  await release();
 }

test/back/file-write-lock-release.test.cjs

@@ -0,0 +1,30 @@
+const test = require('node:test'),
+  assert = require('node:assert/strict'),
+  fs = require('node:fs'),
+  os = require('node:os'),
+  path = require('node:path');
+const load = require('../helpers/load-security-module.cjs');
+const { writeFileWithLock, getUniqueLockPath } = load('back/shared/utils.ts', {
+  '../config/util': { fileExist: async (p) => fs.existsSync(p) },
+});
+test('write failure releases the lock and preserves the original error', async (t) => {
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ql-lock-review-'));
+  t.after(() => fs.rmSync(dir, { recursive: true, force: true }));
+  const p = path.join(dir, 'script');
+  fs.writeFileSync(p, 'original');
+  await assert.rejects(
+    writeFileWithLock(p, 'bad', { encoding: 'invalid-encoding' }),
+    (e) => e.code === 'ERR_INVALID_ARG_VALUE',
+  );
+  assert.equal(fs.existsSync(getUniqueLockPath(p)), false);
+  await writeFileWithLock(p, 'retry');
+  assert.equal(fs.readFileSync(p, 'utf8'), 'retry');
+});
+test('first write closes its file handle and keeps requested permissions', async (t) => {
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ql-lock-review-'));
+  t.after(() => fs.rmSync(dir, { recursive: true, force: true }));
+  const p = path.join(dir, 'new');
+  await writeFileWithLock(p, 'created', { mode: 0o600 });
+  assert.equal(fs.readFileSync(p, 'utf8'), 'created');
+  assert.equal(fs.statSync(p).mode & 0o777, 0o600);
+});