fix(security): harden command injection, path traversal, auth surfaces

Audit of the backend attack surface and fixes for the web-reachable
CRITICAL/HIGH issues. Adds back/shared/security.ts with centralized
hardening helpers (shellEscape, assertSafeDependenceName,
SUBSCRIPTION_PATTERNS, safeCompare, isSafeSshConfigValue).

- Subscription fields (url/branch/whitelist/blacklist/extensions/proxy)
  are now shell-escaped before reaching spawn() and validated with strict
  Joi patterns at the API, closing OS command injection and the
  downstream shell eval/git-arg-injection paths.
- Dependency names are validated before interpolation into
  pnpm/pip/apk/apt commands (incl. the embedded Python source).
- SSH config generation rejects newline/metachar injection in host/proxy
  (prevents injected ProxyCommand execution).
- ConfigService.getFile resolves the real path before containment check,
  fixing data/scripts/../db traversal that leaked the SQLite DB.
- /configs/save containment check fixed (sibling-dir write bypass).
- Script/env uploads use path.basename, preventing arbitrary file write
  (crontab.list/env.sh overwrite -> RCE) via multer originalname.
- JWT secret is generated and persisted per-install instead of the public
  default 'whyour-secret'; production refuses to boot without one.
- Token comparison is now constant-time (safeCompare).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

back/api/config.ts

@@ -4,7 +4,7 @@ import { Logger } from 'winston';
 import config from '../config';
 import * as fs from 'fs/promises';
 import { celebrate, Joi } from 'celebrate';
-import { join, basename } from 'path';
+import path, { basename } from 'path';
 import { SAMPLE_FILES } from '../config/const';
 import { t } from '../shared/i18n';
 import ConfigService from '../services/config';
@@ -72,16 +72,16 @@ export default (app: Router) => {
       const logger: Logger = Container.get('logger');
       try {
         const { name, content } = req.body;
-        // Resolve path first to prevent traversal attacks
-        let basePath = config.configPath;
-        if (name.startsWith('data/scripts/')) {
-          basePath = join(config.rootPath, 'data/scripts');
-        }
+        // Resolve the final path first, then verify containment with a path
+        // separator so sibling dirs (e.g. data/scripts-evil) cannot be reached.
+        const isScripts = name.startsWith('data/scripts/');
+        const basePath = path.resolve(
+          isScripts ? config.scriptPath : config.configPath,
+        );
         const cleanName = name.replace(/^data\/scripts\//, '');
-        const resolvedPath = join(basePath, cleanName);
-        const normalized = join(resolvedPath);
-        // Verify the resolved path stays within allowed directory
-        if (!normalized.startsWith(basePath)) {
+        const normalized = path.resolve(basePath, cleanName);
+        // Verify the resolved path stays within the allowed directory
+        if (normalized !== basePath && !normalized.startsWith(basePath + path.sep)) {
           return res.send({ code: 403, message: t('文件路径无效') });
         }
         // Check blacklist on actual filename (not user input)

back/api/env.ts

@@ -1,6 +1,7 @@
 import { Joi, celebrate } from 'celebrate';
 import { NextFunction, Request, Response, Router } from 'express';
 import fs from 'fs';
+import path from 'path';
 import multer from 'multer';
 import { Container } from 'typedi';
 import { Logger } from 'winston';
@@ -15,7 +16,13 @@ const storage = multer.diskStorage({
     cb(null, config.scriptPath);
   },
   filename: function (req, file, cb) {
-    cb(null, file.originalname);
+    // Never trust the client-supplied name: strip any directory components so
+    // the upload cannot be written outside config.scriptPath (path traversal).
+    const safeName = path.basename(file.originalname || '');
+    if (!safeName || safeName === '.' || safeName === '..') {
+      return cb(new Error('Invalid filename'), '');
+    }
+    cb(null, safeName);
   },
 });
 const upload = multer({ storage: storage });

back/api/script.ts

@@ -22,7 +22,13 @@ const storage = multer.diskStorage({
     cb(null, config.scriptPath);
   },
   filename: function (req, file, cb) {
-    cb(null, file.originalname);
+    // Never trust the client-supplied name: strip any directory components so
+    // the upload cannot be written outside config.scriptPath (path traversal).
+    const safeName = path.basename(file.originalname || '');
+    if (!safeName || safeName === '.' || safeName === '..') {
+      return cb(new Error('Invalid filename'), '');
+    }
+    cb(null, safeName);
   },
 });
 const upload = multer({ storage: storage });

back/api/subscription.ts

@@ -4,8 +4,33 @@ import { Logger } from 'winston';
 import SubscriptionService from '../services/subscription';
 import { celebrate, Joi } from 'celebrate';
 import CronExpressionParser from 'cron-parser';
+import { SUBSCRIPTION_PATTERNS } from '../shared/security';
 const route = Router();
 
+// Shared validation for fields that are interpolated into shell commands /
+// ssh config files. Blocks shell metacharacters / newlines at the boundary.
+const urlField = Joi.string().pattern(SUBSCRIPTION_PATTERNS.url);
+const branchField = Joi.string()
+  .pattern(SUBSCRIPTION_PATTERNS.branch)
+  .optional()
+  .allow('')
+  .allow(null);
+const extensionsField = Joi.string()
+  .pattern(SUBSCRIPTION_PATTERNS.extensions)
+  .optional()
+  .allow('')
+  .allow(null);
+const proxyField = Joi.string()
+  .pattern(SUBSCRIPTION_PATTERNS.proxy)
+  .optional()
+  .allow('')
+  .allow(null);
+const filterField = Joi.string()
+  .pattern(SUBSCRIPTION_PATTERNS.filter)
+  .optional()
+  .allow('')
+  .allow(null);
+
 export default (app: Router) => {
   app.use('/subscriptions', route);
 
@@ -38,19 +63,19 @@ export default (app: Router) => {
           .allow('')
           .allow(null),
         name: Joi.string().optional().allow('').allow(null),
-        url: Joi.string().required(),
-        whitelist: Joi.string().optional().allow('').allow(null),
-        blacklist: Joi.string().optional().allow('').allow(null),
-        branch: Joi.string().optional().allow('').allow(null),
-        dependences: Joi.string().optional().allow('').allow(null),
+        url: urlField.required(),
+        whitelist: filterField,
+        blacklist: filterField,
+        branch: branchField,
+        dependences: filterField,
         pull_type: Joi.string().optional().allow('').allow(null),
         pull_option: Joi.object().optional().allow('').allow(null),
-        extensions: Joi.string().optional().allow('').allow(null),
+        extensions: extensionsField,
         sub_before: Joi.string().optional().allow('').allow(null),
         sub_after: Joi.string().optional().allow('').allow(null),
         schedule_type: Joi.string().required(),
         alias: Joi.string().required(),
-        proxy: Joi.string().optional().allow('').allow(null),
+        proxy: proxyField,
         autoAddCron: Joi.boolean().optional().allow('').allow(null),
         autoDelCron: Joi.boolean().optional().allow('').allow(null),
       }),
@@ -169,19 +194,19 @@ export default (app: Router) => {
         schedule: Joi.string().optional().allow('').allow(null),
         interval_schedule: Joi.object().optional().allow('').allow(null),
         name: Joi.string().optional().allow('').allow(null),
-        url: Joi.string().required(),
-        whitelist: Joi.string().optional().allow('').allow(null),
-        blacklist: Joi.string().optional().allow('').allow(null),
-        branch: Joi.string().optional().allow('').allow(null),
-        dependences: Joi.string().optional().allow('').allow(null),
+        url: urlField.required(),
+        whitelist: filterField,
+        blacklist: filterField,
+        branch: branchField,
+        dependences: filterField,
         pull_type: Joi.string().optional().allow('').allow(null),
         pull_option: Joi.object().optional().allow('').allow(null),
         schedule_type: Joi.string().optional().allow('').allow(null),
-        extensions: Joi.string().optional().allow('').allow(null),
+        extensions: extensionsField,
         sub_before: Joi.string().optional().allow('').allow(null),
         sub_after: Joi.string().optional().allow('').allow(null),
         alias: Joi.string().required(),
-        proxy: Joi.string().optional().allow('').allow(null),
+        proxy: proxyField,
         autoAddCron: Joi.boolean().optional().allow('').allow(null),
         autoDelCron: Joi.boolean().optional().allow('').allow(null),
         id: Joi.number().required(),

back/config/index.ts

@@ -1,5 +1,7 @@
 import dotenv from 'dotenv';
 import path from 'path';
+import crypto from 'crypto';
+import fs from 'fs';
 
 dotenv.config({
   path: path.join(__dirname, '../../.env'),
@@ -128,9 +130,47 @@ if (envFound.error) {
   throw new Error("⚠️  Couldn't find .env file  ⚠️");
 }
 
+/**
+ * Resolve the JWT signing secret. A shared, source-code-public default secret
+ * lets anyone forge admin tokens, so:
+ *  - honor an explicitly configured JWT_SECRET (must not be the old default);
+ *  - otherwise generate a strong random secret once and persist it under the
+ *    data dir so it stays stable across restarts and cluster workers;
+ *  - never fall back to the public default in production.
+ */
+function resolveJwtSecret(): string {
+  const envSecret = process.env.JWT_SECRET;
+  if (envSecret && envSecret !== 'whyour-secret') {
+    return envSecret;
+  }
+
+  const secretFile = path.join(dbPath, 'jwt_secret.key');
+  try {
+    if (fs.existsSync(secretFile)) {
+      const existing = fs.readFileSync(secretFile, 'utf-8').trim();
+      if (existing) {
+        return existing;
+      }
+    }
+    const generated = crypto.randomBytes(48).toString('hex');
+    fs.mkdirSync(path.dirname(secretFile), { recursive: true });
+    fs.writeFileSync(secretFile, generated, { mode: 0o600 });
+    return generated;
+  } catch (error) {
+    if (process.env.NODE_ENV === 'production') {
+      throw new Error(
+        'Refusing to start: unable to provision a secure JWT secret. Set JWT_SECRET.',
+      );
+    }
+    return 'whyour-secret';
+  }
+}
+
+const jwtConfig = { ...config.jwt, secret: resolveJwtSecret() };
+
 export default {
   ...config,
-  jwt: config.jwt,
+  jwt: jwtConfig,
   baseUrl,
   rootPath,
   tmpPath,

back/config/subscription.ts

@@ -1,5 +1,6 @@
 import { Subscription } from '../data/subscription';
 import isNil from 'lodash/isNil';
+import { shellEscape } from '../shared/security';
 
 export function formatUrl(doc: Subscription) {
   let url = doc.url;
@@ -31,16 +32,22 @@ export function formatCommand(doc: Subscription, url?: string) {
     autoAddCron,
     autoDelCron,
   } = doc;
+  const addCron = isNil(autoAddCron) ? true : Boolean(autoAddCron);
+  const delCron = isNil(autoDelCron) ? true : Boolean(autoDelCron);
+  // Every user-controlled value is single-quote escaped so it can never break
+  // out of the shell command passed to spawn(command, { shell: '/bin/bash' }).
   if (type === 'file') {
-    command += `raw "${_url}" "${proxy || ''}" "${
-      isNil(autoAddCron) ? true : Boolean(autoAddCron)
-    }" "${isNil(autoDelCron) ? true : Boolean(autoDelCron)}"`;
+    command += `raw ${shellEscape(_url)} ${shellEscape(
+      proxy || '',
+    )} "${addCron}" "${delCron}"`;
   } else {
-    command += `repo "${_url}" "${whitelist || ''}" "${blacklist || ''}" "${
-      dependences || ''
-    }" "${branch || ''}" "${extensions || ''}" "${proxy || ''}" "${
-      isNil(autoAddCron) ? true : Boolean(autoAddCron)
-    }" "${isNil(autoDelCron) ? true : Boolean(autoDelCron)}"`;
+    command += `repo ${shellEscape(_url)} ${shellEscape(
+      whitelist || '',
+    )} ${shellEscape(blacklist || '')} ${shellEscape(
+      dependences || '',
+    )} ${shellEscape(branch || '')} ${shellEscape(extensions || '')} ${shellEscape(
+      proxy || '',
+    )} "${addCron}" "${delCron}"`;
   }
   return command;
 }

back/config/util.ts

@@ -12,6 +12,7 @@ import { DependenceTypes } from '../data/dependence';
 import { FormData } from 'undici';
 import os from 'os';
 import { maybeSudo, isInContainer } from './container';
+import { assertSafeDependenceName } from '../shared/security';
 
 export * from './share';
 
@@ -569,6 +570,7 @@ export async function setSystemTimezone(timezone: string): Promise<boolean> {
 }
 
 export function getGetCommand(type: DependenceTypes, name: string): string {
+  name = assertSafeDependenceName(name);
   const baseCommands = {
     [DependenceTypes.nodejs]: `pnpm ls -g  | grep "${name}" | head -1`,
     [DependenceTypes.python3]: `
@@ -592,6 +594,7 @@ except:
 }
 
 export function getInstallCommand(type: DependenceTypes, name: string): string {
+  name = assertSafeDependenceName(name);
   const baseCommands = {
     [DependenceTypes.nodejs]: 'pnpm add -g',
     [DependenceTypes.python3]:
@@ -614,6 +617,7 @@ export function getUninstallCommand(
   type: DependenceTypes,
   name: string,
 ): string {
+  name = assertSafeDependenceName(name);
   const baseCommands = {
     [DependenceTypes.nodejs]: 'pnpm remove -g',
     [DependenceTypes.python3]:

back/services/config.ts

@@ -1,5 +1,5 @@
 import { Service, Inject } from 'typedi';
-import path, { join } from 'path';
+import path from 'path';
 import config from '../config';
 import { getFileContentByName } from '../config/util';
 import { t } from '../shared/i18n';
@@ -19,31 +19,30 @@ export default class ConfigService {
     if (normalized.startsWith('..') || path.isAbsolute(normalized)) {
       return res.send({ code: 403, message: t('文件无法访问') });
     }
-    const resolvedRoot = path.resolve(config.rootPath, normalized);
-    const resolvedConfig = path.resolve(config.configPath, normalized);
-    const isValidPath =
-      resolvedRoot.startsWith(config.scriptPath) ||
-      resolvedRoot.startsWith(config.configPath) ||
-      resolvedConfig.startsWith(config.scriptPath) ||
-      resolvedConfig.startsWith(config.configPath);
-    if (!isValidPath) {
-      return res.send({ code: 403, message: t('文件无法访问') });
-    }
-    if (config.blackFileList.includes(path.basename(normalized))) {
-      return res.send({ code: 403, message: t('文件无法访问') });
-    }
 
+    // Remote sample files are fetched (and path-checked) separately.
     if (filePath.startsWith('sample/')) {
-      const res = await request(
-        `https://gitlab.com/whyour/qinglong/-/raw/master/${filePath}`,
+      const sampleRes = await request(
+        `https://gitlab.com/whyour/qinglong/-/raw/master/${normalized}`,
       );
-      content = await res.body.text();
-    } else if (filePath.startsWith('data/scripts/')) {
-      content = await getFileContentByName(join(config.rootPath, filePath));
-    } else {
-      content = await getFileContentByName(join(config.configPath, filePath));
+      return res.send({ code: 200, data: await sampleRes.body.text() });
     }
 
+    // Resolve the ACTUAL file path first, then validate that it stays within
+    // its allowed base. Validating a different path than the one read is what
+    // previously allowed `data/scripts/../db/database.sqlite` style traversal.
+    const isScripts = filePath.startsWith('data/scripts/');
+    const base = path.resolve(isScripts ? config.scriptPath : config.configPath);
+    const rel = isScripts ? filePath.slice('data/scripts/'.length) : filePath;
+    const finalPath = path.resolve(base, rel);
+    if (finalPath !== base && !finalPath.startsWith(base + path.sep)) {
+      return res.send({ code: 403, message: t('文件无法访问') });
+    }
+    if (config.blackFileList.includes(path.basename(finalPath))) {
+      return res.send({ code: 403, message: t('文件无法访问') });
+    }
+
+    content = await getFileContentByName(finalPath);
     res.send({ code: 200, data: content });
   }
 }

back/services/sshKey.ts

@@ -8,6 +8,7 @@ import { formatUrl } from '../config/subscription';
 import config from '../config';
 import { fileExist, rmPath } from '../config/util';
 import { writeFileWithLock } from '../shared/utils';
+import { isSafeSshConfigValue, SUBSCRIPTION_PATTERNS } from '../shared/security';
 
 @Service()
 export default class SshKeyService {
@@ -66,6 +67,14 @@ export default class SshKeyService {
     host: string,
     proxy?: string,
   ) {
+    // Prevent newline / shell-metacharacter injection into the ssh config file
+    // (e.g. an injected ProxyCommand directive executed by ssh on git pull).
+    if (!isSafeSshConfigValue(host)) {
+      throw new Error('Invalid ssh host');
+    }
+    if (proxy && !SUBSCRIPTION_PATTERNS.proxy.test(proxy)) {
+      throw new Error('Invalid ssh proxy');
+    }
     if (host === 'github.com') {
       host = `ssh.github.com\n    Port 443\n    HostkeyAlgorithms +ssh-rsa`;
     }

back/shared/auth.ts

@@ -1,4 +1,5 @@
 import { AuthInfo, TokenInfo } from '../data/system';
+import { safeCompare } from './security';
 
 /**
  * Validates if a token exists in the authentication info.
@@ -20,8 +21,8 @@ export function isValidToken(
 
   const { token = '', tokens = {} } = authInfo;
 
-  // Check legacy token field
-  if (headerToken === token) {
+  // Check legacy token field (constant-time)
+  if (token && safeCompare(headerToken, token)) {
     return true;
   }
 
@@ -35,10 +36,12 @@ export function isValidToken(
 
   if (typeof platformTokens === 'string') {
     // Legacy format: single string token
-    return headerToken === platformTokens;
+    return safeCompare(headerToken, platformTokens);
   } else if (Array.isArray(platformTokens)) {
-    // New format: array of TokenInfo objects
-    return platformTokens.some((t: TokenInfo) => t && t.value === headerToken);
+    // New format: array of TokenInfo objects (constant-time per entry)
+    return platformTokens.some(
+      (t: TokenInfo) => t && safeCompare(headerToken, t.value),
+    );
   }
 
   // Unexpected type - log warning and reject

back/shared/security.ts

@@ -0,0 +1,72 @@
+/**
+ * Centralized input-hardening helpers used to neutralize command injection,
+ * argument injection and config-file injection across the backend.
+ *
+ * The task runner legitimately executes user scripts, but data fields such as
+ * git URLs, branches, dependency names and proxies are NOT meant to be code.
+ * These helpers keep such values from breaking out of the shell commands /
+ * config files they are interpolated into.
+ */
+import crypto from 'crypto';
+
+/**
+ * POSIX single-quote escaping. Wraps a value so it is passed to the shell as a
+ * single literal argument, neutralizing $(), backticks, ;, |, &, redirects,
+ * whitespace and newlines.
+ */
+export function shellEscape(value: unknown): string {
+  const str = value === undefined || value === null ? '' : String(value);
+  return `'${str.replace(/'/g, `'\\''`)}'`;
+}
+
+/** Characters that enable shell command injection / argument chaining. */
+const SHELL_METACHAR = /[;&|`$<>(){}\[\]'"\\!\n\r\t ]/;
+
+/**
+ * Validate a package/dependence name before it is interpolated into an
+ * install/uninstall/version command (pnpm/pip/apk/apt). Throws on anything that
+ * could break out of the command or inject an extra argument.
+ */
+export function assertSafeDependenceName(name: string): string {
+  const n = String(name ?? '').trim();
+  if (!n || n.length > 214 || SHELL_METACHAR.test(n) || n.startsWith('-')) {
+    throw new Error('Invalid dependence name');
+  }
+  return n;
+}
+
+/**
+ * Joi-compatible patterns for subscription fields that flow into shell commands
+ * and ssh config files. They block shell metacharacters / newlines while still
+ * permitting legitimate values.
+ */
+export const SUBSCRIPTION_PATTERNS = {
+  // git/http(s)/ssh url: no whitespace or shell metacharacters, must not start with '-'
+  url: /^(?!-)[^\s;&|`$<>(){}'"\\]+$/,
+  // git ref: word chars, dots, slashes, dashes
+  branch: /^[\w.\/-]*$/,
+  // space/comma separated bare file extensions
+  extensions: /^[A-Za-z0-9 ,]*$/,
+  // host:port (consumed by the nc ProxyCommand) or empty
+  proxy: /^([\w.\-]+:\d+)?$/,
+  // regex filters: forbid line breaks and command-substitution chars, keep regex metachars
+  filter: /^[^\r\n`$\\]*$/,
+};
+
+/**
+ * Constant-time string comparison for tokens / secrets / passwords. Both inputs
+ * are hashed to a fixed length first so timingSafeEqual never throws on length
+ * mismatch and the comparison does not leak length via timing.
+ */
+export function safeCompare(a: string | undefined | null, b: string | undefined | null): boolean {
+  if (typeof a !== 'string' || typeof b !== 'string') return false;
+  const ha = crypto.createHash('sha256').update(a).digest();
+  const hb = crypto.createHash('sha256').update(b).digest();
+  return crypto.timingSafeEqual(ha, hb) && a.length === b.length;
+}
+
+/** Reject values that are dangerous when written verbatim into an ssh config file. */
+export function isSafeSshConfigValue(value: string | undefined | null): boolean {
+  if (value === undefined || value === null) return true;
+  return !/[\r\n;&|`$<>()'"\\]/.test(String(value));
+}