From 1cfc0c8e2e111ce6448a8de935dbb299c11ecffb Mon Sep 17 00:00:00 2001 From: whyour Date: Tue, 10 Nov 2026 01:11:54 +0800 Subject: [PATCH] fix(security): protect first-run notifications and server errors (#3126) * fix(security): close anonymous notification setup and mask server errors * fix: hide internal errors in default deployment * fix: route HTTP errors through application logger * test: mock logger in API fallback regression * fix: avoid sensitive SQL details in HTTP error logs --- back/api/health.ts | 1 - back/api/user.ts | 14 --- back/config/index.ts | 2 - back/loaders/express.ts | 73 ++++++++-------- cli/skills/qinglong-cli/references/openapi.md | 1 - cli/src/remote/api/openOperations.ts | 7 -- src/pages/initialization/index.tsx | 85 +------------------ src/utils/http.tsx | 1 - test/back/api-fallback.test.cjs | 1 + test/back/http-security.test.cjs | 52 ++++++++++++ test/back/scheduler-readiness.test.cjs | 13 +++ 11 files changed, 104 insertions(+), 146 deletions(-) diff --git a/back/api/health.ts b/back/api/health.ts index 738e8ec1..bd1a97c2 100644 --- a/back/api/health.ts +++ b/back/api/health.ts @@ -20,7 +20,6 @@ export default (app: Router) => { res.status(500).send({ code: 500, message: 'Health check failed', - error: err.message, }); } }); diff --git a/back/api/user.ts b/back/api/user.ts index 4e92b39b..b4e1aa70 100644 --- a/back/api/user.ts +++ b/back/api/user.ts @@ -310,20 +310,6 @@ export default (app: Router) => { }, ); - route.put( - '/notification/init', - async (req: Request, res: Response, next: NextFunction) => { - const logger: Logger = Container.get('logger'); - try { - const userService = Container.get(UserService); - const result = await userService.updateNotificationMode(req.body); - res.send(result); - } catch (e) { - return next(e); - } - }, - ); - route.put( '/avatar', upload.single('avatar'), diff --git a/back/config/index.ts b/back/config/index.ts index 00b2a7b8..7014fa6e 100644 --- a/back/config/index.ts +++ b/back/config/index.ts @@ -180,12 +180,10 @@ export default { '/api/user/two-factor/login', '/api/system', '/api/user/init', - '/api/user/notification/init', '/open/user/login', '/open/user/two-factor/login', '/open/system', '/open/user/init', - '/open/user/notification/init', ], versionFile, lastVersionFile, diff --git a/back/loaders/express.ts b/back/loaders/express.ts index b83184e0..62af7471 100644 --- a/back/loaders/express.ts +++ b/back/loaders/express.ts @@ -16,6 +16,7 @@ import { t } from '../shared/i18n'; import { AppScope } from '../data/open'; import uniqueConstraintError from '../middlewares/uniqueConstraintError'; import protectedPathCase from '../middlewares/protectedPathCase'; +import Logger from './logger'; function resolveTrustProxy(value = process.env.QL_TRUST_PROXY) { const setting = value?.trim(); @@ -191,14 +192,7 @@ export default ({ app }: { app: Application }) => { app.use(async (req, res, next) => { const pathLower = req.path.toLowerCase(); - if ( - ![ - '/api/user/init', - '/api/user/notification/init', - '/open/user/init', - '/open/user/notification/init', - ].includes(pathLower) - ) { + if (!['/api/user/init', '/open/user/init'].includes(pathLower)) { return next(); } const authInfo = (await shareStore.getAuthInfo()) || ({} as AuthInfo); @@ -238,37 +232,23 @@ export default ({ app }: { app: Application }) => { app.use( ( - err: Error & { status: number }, + err: Error & { status: number; code?: string }, req: Request, res: Response, next: NextFunction, ) => { if (err.name === 'UnauthorizedError') { - return res - .status(err.status) - .send({ code: 401, message: err.message }) - .end(); - } - return next(err); - }, - ); - - app.use( - ( - err: Error & { errors: any[] }, - req: Request, - res: Response, - next: NextFunction, - ) => { - if (err.name.includes('Sequelize')) { - return res - .status(500) - .send({ - code: 400, - message: `${err.message}`, - errors: err.errors, - }) - .end(); + let message = err.message; + if (!config.isDevelopment) { + if (err.code === 'credentials_bad_scheme') { + message = t('暂无权限'); + } else if (err.code === 'credentials_required') { + message = t('请先登录'); + } else { + message = t('Token 已失效'); + } + } + return res.status(401).send({ code: 401, message }).end(); } return next(err); }, @@ -281,10 +261,29 @@ export default ({ app }: { app: Application }) => { res: Response, next: NextFunction, ) => { - res.status(err.status || 500); + const status = + Number.isInteger(err.status) && err.status >= 400 && err.status < 600 + ? err.status + : 500; + if (status >= 500) { + const route = + typeof req.route?.path === 'string' ? req.route.path : '?'; + const errorName = /^[A-Za-z][A-Za-z0-9]{0,63}$/.test(err.name) + ? err.name + : 'Error'; + Logger.error( + 'HTTP request failed: status=%d method=%s route=%s type=%s', + status, + req.method, + route, + errorName, + ); + } + res.status(status); res.json({ - code: err.status || 500, - message: err.message, + code: status, + message: + !config.isDevelopment && status >= 500 ? t('未知错误') : err.message, }); }, ); diff --git a/cli/skills/qinglong-cli/references/openapi.md b/cli/skills/qinglong-cli/references/openapi.md index a836f479..e9e45088 100644 --- a/cli/skills/qinglong-cli/references/openapi.md +++ b/cli/skills/qinglong-cli/references/openapi.md @@ -224,7 +224,6 @@ System/user operations below execute remotely through the panel API. They do not | `ql user notification-get ` | GET | `user/notification` | --query | | `ql user notification-set ` | PUT | `user/notification` | --data, --query | | `ql user init ` | PUT | `user/init` | --data, --query | -| `ql user notification-init ` | PUT | `user/notification/init` | --data, --query | | `ql user avatar ` | PUT | `user/avatar` | --data, --file (avatar), --query | | `ql system apply-reload ` | PUT | `update/reload` | --query | | `ql system apply-system ` | PUT | `update/system` | --query | diff --git a/cli/src/remote/api/openOperations.ts b/cli/src/remote/api/openOperations.ts index 91624746..64f5fe5a 100644 --- a/cli/src/remote/api/openOperations.ts +++ b/cli/src/remote/api/openOperations.ts @@ -608,13 +608,6 @@ export const openOperations: readonly OpenOperation[] = [ body: 'json', anonymous: true, }, - { - name: 'user notification-init', - method: 'PUT', - path: 'user/notification/init', - body: 'json', - anonymous: true, - }, { name: 'user avatar', method: 'PUT', diff --git a/src/pages/initialization/index.tsx b/src/pages/initialization/index.tsx index 1fd9e64d..667e3de9 100644 --- a/src/pages/initialization/index.tsx +++ b/src/pages/initialization/index.tsx @@ -1,38 +1,21 @@ import intl from 'react-intl-universal'; -import React, { Fragment, useEffect, useState } from 'react'; -import { - Button, - Row, - Input, - Form, - message, - Typography, - Steps, - Select, -} from 'antd'; +import React, { useEffect, useState } from 'react'; +import { Button, Input, Form, Typography, Steps } from 'antd'; import config from '@/utils/config'; -import { history } from '@umijs/max'; import styles from './index.less'; import { request } from '@/utils/http'; -const FormItem = Form.Item; const { Step } = Steps; -const { Option } = Select; const { Link } = Typography; const Initialization = () => { const [loading, setLoading] = useState(false); const [current, setCurrent] = React.useState(0); - const [fields, setFields] = useState([]); const next = () => { setCurrent(current + 1); }; - const prev = () => { - setCurrent(current - 1); - }; - const submitAccountSetting = (values: any) => { setLoading(true); request @@ -48,23 +31,6 @@ const Initialization = () => { .finally(() => setLoading(false)); }; - const submitNotification = (values: any) => { - setLoading(true); - request - .put(`${config.apiPrefix}user/notification/init`, values) - .then(({ code, data }) => { - if (code === 200) { - next(); - } - }) - .finally(() => setLoading(false)); - }; - - const notificationModeChange = (value: string) => { - const _fields = (config.notificationModeMap as any)[value]; - setFields(_fields || []); - }; - useEffect(() => { localStorage.removeItem(config.authKey); }, []); @@ -95,53 +61,6 @@ const Initialization = () => { ), }, - { - title: intl.get('通知设置'), - content: ( -
- - - - {fields.map((x) => ( - - - - ))} - - -
- ), - }, { title: intl.get('账户设置'), content: ( diff --git a/src/utils/http.tsx b/src/utils/http.tsx index 466dc085..503cdb28 100644 --- a/src/utils/http.tsx +++ b/src/utils/http.tsx @@ -90,7 +90,6 @@ const apiWhiteList = [ `${config.baseUrl}api/user/two-factor/login`, `${config.baseUrl}api/system`, `${config.baseUrl}api/user/init`, - `${config.baseUrl}api/user/notification/init`, ]; _request.interceptors.request.use((_config) => { diff --git a/test/back/api-fallback.test.cjs b/test/back/api-fallback.test.cjs index d0e4888a..68abd332 100644 --- a/test/back/api-fallback.test.cjs +++ b/test/back/api-fallback.test.cjs @@ -13,6 +13,7 @@ test('unknown API requests return JSON 404 while env.js and SPA deep links work' const app = express(); load('back/loaders/express.ts', { '../api': () => express.Router(), + './logger': { error() {} }, '../config': { rootPath: dir, uploadPath: dir, diff --git a/test/back/http-security.test.cjs b/test/back/http-security.test.cjs index b405d76c..17e8b34c 100644 --- a/test/back/http-security.test.cjs +++ b/test/back/http-security.test.cjs @@ -3,6 +3,7 @@ const test = require('node:test'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); +const util = require('node:util'); const express = require('express'); const jwt = require('jsonwebtoken'); const load = require('../helpers/load-security-module.cjs'); @@ -105,6 +106,8 @@ test('HTTP authentication protects init, scopes, expired sessions and config sec let envWriteCalls = 0; let scopedMutationCalls = 0; let initialized = 0; + let notificationCalls = 0; + const loggedErrors = []; const User = class {}; const Script = class { getFile(folder, filename) { @@ -121,6 +124,10 @@ test('HTTP authentication protects init, scopes, expired sessions and config sec return { code: 200 }; }, getAuthInfo: async () => auth, + updateNotificationMode: async () => { + notificationCalls++; + return { code: 200 }; + }, }; const mocks = { '../config': config, @@ -136,6 +143,7 @@ test('HTTP authentication protects init, scopes, expired sessions and config sec shareStore: { getAuthInfo: async () => auth, getApps: async () => apps }, }, '../config/serverEnv': { serveEnv: (_req, res) => res.end() }, + './logger': { error: (...args) => loggedErrors.push(util.format(...args)) }, '../services/user': User, '../services/script': Script, '../data/open': {}, @@ -209,6 +217,17 @@ test('HTTP authentication protects init, scopes, expired sessions and config sec commandCalls++; res.json({ code: 200 }); }); + router.get('/security-test/error', (_req, _res, next) => + next(new Error('sensitive server detail')), + ); + router.get('/security-test/db-error', (_req, _res, next) => { + const error = new Error('sensitive database detail'); + error.name = 'SequelizeDatabaseError'; + error.errors = [{ message: 'sensitive row value' }]; + error.sql = "SELECT * FROM users WHERE password = 'sensitive password'"; + error.parameters = ['sensitive parameter']; + next(error); + }); return router; }; const app = express(); @@ -238,6 +257,39 @@ test('HTTP authentication protects init, scopes, expired sessions and config sec ); return { status: response.status, body: await response.json() }; }; + auth.username = 'admin'; + auth.password = 'admin'; + for (const prefix of ['/api', '/open', '/panel/api', '/panel/open']) { + const response = await request( + `${prefix}/user/notification/init`, + undefined, + 'PUT', + { type: 'webhook', webhookUrl: 'http://127.0.0.1/?title=$title' }, + ); + assert.equal(response.status, 401, prefix); + } + assert.equal(notificationCalls, 0); + auth.username = 'owner'; + auth.password = 'configured'; + assert.equal( + (await request('/api/user/notification', valid, 'PUT', {})).body.code, + 200, + ); + assert.equal(notificationCalls, 1); + for (const route of ['/security-test/error', '/security-test/db-error']) { + const response = await request(`/api${route}`, valid); + assert.equal(response.status, 500); + assert.deepEqual(response.body, { code: 500, message: '未知错误' }); + } + assert.equal(loggedErrors.length, 2); + assert.match(loggedErrors[1], /type=SequelizeDatabaseError/); + assert.ok(loggedErrors.every((line) => !line.includes('sensitive'))); + const malformedToken = await request('/api/envs', 'not-a-jwt'); + assert.equal(malformedToken.status, 401); + assert.deepEqual(malformedToken.body, { + code: 401, + message: 'Token 已失效', + }); for (const url of [ '/api/user/init', '/open/user/init', diff --git a/test/back/scheduler-readiness.test.cjs b/test/back/scheduler-readiness.test.cjs index 1fe68834..16bcf29f 100644 --- a/test/back/scheduler-readiness.test.cjs +++ b/test/back/scheduler-readiness.test.cjs @@ -52,6 +52,19 @@ test('health uses actual readiness and returns HTTP 503 until recovery',async(t) assert.equal(r.status,expected);assert.equal(b.code,expected);assert.equal(b.data.services.grpc,ready); } }); +test('health failure does not reveal internal error details',async(t)=>{ + const express=require('express'); + const HealthService=class{}; + const app=express();load('back/api/health.ts',{ + typedi:{get:()=>({check:async()=>{throw Error('sensitive health detail');}})}, + '../services/health':{HealthService},'../loaders/logger':{error(){}}, + }).default(app); + const server=app.listen(0,'127.0.0.1');await new Promise(r=>server.once('listening',r)); + t.after(()=>new Promise(r=>{server.close(r);server.closeAllConnections();})); + const response=await fetch(`http://127.0.0.1:${server.address().port}/health`); + assert.equal(response.status,500); + assert.deepEqual(await response.json(),{code:500,message:'Health check failed'}); +}); test('recovery registration errors propagate while ordinary autosave retains file synchronization',async()=>{ const source=fs.readFileSync('back/services/cron.ts','utf8');const a=source.indexOf(' public async autosave_crontab('),z=source.indexOf(' public async bootTask',a); const js=ts.transpileModule('class Fixture {\n'+source.slice(a,z)+'}\nmodule.exports=Fixture;', {compilerOptions:{target:ts.ScriptTarget.ES2020}}).outputText;