diff --git a/back/loaders/express.ts b/back/loaders/express.ts index 20c05774..8ef1f06d 100644 --- a/back/loaders/express.ts +++ b/back/loaders/express.ts @@ -14,6 +14,7 @@ import { AuthInfo } from '../data/system'; import path from 'path'; import { t } from '../shared/i18n'; import { AppScope } from '../data/open'; +import uniqueConstraintError from '../middlewares/uniqueConstraintError'; import protectedPathCase from '../middlewares/protectedPathCase'; function resolveTrustProxy(value = process.env.QL_TRUST_PROXY) { @@ -190,6 +191,7 @@ export default ({ app }: { app: Application }) => { }); app.use(errors()); + app.use(uniqueConstraintError); app.use( ( diff --git a/back/middlewares/uniqueConstraintError.ts b/back/middlewares/uniqueConstraintError.ts new file mode 100644 index 00000000..8d4a6eda --- /dev/null +++ b/back/middlewares/uniqueConstraintError.ts @@ -0,0 +1,14 @@ +import { ErrorRequestHandler } from 'express'; +import { UniqueConstraintError } from 'sequelize'; +import { t } from '../shared/i18n'; + +// Handle the database constraint itself so concurrent writes are covered too. +const uniqueConstraintError: ErrorRequestHandler = (err, req, res, next) => { + if (!(err instanceof UniqueConstraintError)) return next(err); + // Constraint details can contain environment values or application credentials. + res + .status(409) + .json({ code: 409, message: t('资源已存在，请检查重复的名称或值') }); +}; + +export default uniqueConstraintError; diff --git a/back/shared/i18n.ts b/back/shared/i18n.ts index 6b05d86c..fbd83984 100644 --- a/back/shared/i18n.ts +++ b/back/shared/i18n.ts @@ -3,6 +3,7 @@ import { shareStore } from './store'; const messages: Record> = { zh: {}, en: { + '资源已存在，请检查重复的名称或值': 'Resource already exists; check for duplicate names or values', '暂无权限': 'Access denied', '参数错误': 'Invalid parameter', '参数不正确': 'Invalid parameter', diff --git a/cli/src/remote/api/client.ts b/cli/src/remote/api/client.ts index b63f0529..85790f4a 100644 --- a/cli/src/remote/api/client.ts +++ b/cli/src/remote/api/client.ts @@ -53,23 +53,12 @@ export async function request( // Check HTTP status before decoding (proxies may return HTML on denial). if (!response.ok) { await response.body?.cancel(); - const suffix = - method !== 'GET' && response.status >= 500 - ? translate( - process.env, - ' 执行结果可能未知，请先检查%s状态再考虑重试。', - resource, - ) - : ''; - fail( - translate( - process.env, - 'API 请求被拒绝（HTTP %s），请检查%s。%s', - response.status, - credentialsHint, - suffix, - ), - [401, 403].includes(response.status) ? 3 : 1, + rejectResponse( + response.status, + 'HTTP', + method, + resource, + credentialsHint, ); } if ( @@ -100,28 +89,63 @@ export async function request( ), ); } - if (endpoint.split('?')[0] === 'user/login' && isRecord(result) && result.code === 420) - fail(translate(process.env, '需要双因素验证，请调用 user two-factor-login。'), 3); + if ( + endpoint.split('?')[0] === 'user/login' && + isRecord(result) && + result.code === 420 + ) + fail( + translate(process.env, '需要双因素验证，请调用 user two-factor-login。'), + 3, + ); if (!isRecord(result) || result.code !== 200) { const status = isRecord(result) && typeof result.code === 'number' ? result.code : undefined; - fail( - translate( - process.env, - 'API 请求被拒绝（code %s），请检查%s。', - status ?? translate(process.env, '未知'), - credentialsHint, - ), - status === 401 || status === 403 ? 3 : 1, - ); + rejectResponse(status, 'code', method, resource, credentialsHint); } if (options.output) fail(translate(process.env, '接口返回 JSON，未保存下载文件。')); return result; } +function rejectResponse( + status: number | undefined, + kind: 'HTTP' | 'code', + method: string, + resource: string, + credentialsHint: string, +): never { + const denied = status === 401 || status === 403; + const reason = denied + ? translate(process.env, '请检查%s。', credentialsHint) + : status === 409 + ? translate(process.env, '资源冲突，请检查重复的名称或值。') + : status !== undefined && status >= 500 + ? translate(process.env, '服务端错误，请检查服务状态和日志。') + : translate(process.env, '请求未成功，请检查请求参数和服务日志。'); + const suffix = + method !== 'GET' && (status === undefined || status >= 500) + ? translate( + process.env, + ' 执行结果可能未知，请先检查%s状态再考虑重试。', + resource, + ) + : ''; + fail( + translate( + process.env, + 'API 请求失败（%s %s）。%s%s', + kind, + status ?? translate(process.env, '未知'), + reason, + suffix, + ), + denied ? 3 : 1, + ); +} + export async function authenticate(config: Credentials): Promise { const query = new URLSearchParams({ client_id: config.clientId, diff --git a/cli/src/shared/i18n/en.ts b/cli/src/shared/i18n/en.ts index 5a199372..f1cbdaae 100644 --- a/cli/src/shared/i18n/en.ts +++ b/cli/src/shared/i18n/en.ts @@ -151,6 +151,12 @@ export const english: Record = { 应用凭据: 'application credentials', '应用凭据和 %s 权限': 'application credentials and %s permission', 未知: 'unknown', + '请检查%s。': 'Check %s.', + '资源冲突，请检查重复的名称或值。': 'Resource conflict; check for duplicate names or values.', + '服务端错误，请检查服务状态和日志。': 'Server error; check service status and logs.', + '请求未成功，请检查请求参数和服务日志。': 'Request unsuccessful; check request parameters and service logs.', + 'API 请求失败（%s %s）。%s%s': 'API request failed (%s %s). %s%s', + ' 执行结果可能未知，请先检查%s状态再考虑重试。': ' Execution outcome may be unknown; check %s status before retrying.', 'API 请求被拒绝（HTTP %s），请检查%s。%s': diff --git a/cli/test/remote/apiDiagnostics.test.cjs b/cli/test/remote/apiDiagnostics.test.cjs index c5024a47..af000912 100644 --- a/cli/test/remote/apiDiagnostics.test.cjs +++ b/cli/test/remote/apiDiagnostics.test.cjs @@ -54,7 +54,7 @@ test('API diagnostics preserve resource scope, uncertainty, codes and secret sup language === 'en' ? /outcome.*unknown/ : /结果.*未知/, ); } - if (failure !== 'network') { + if (failure === 'permission' || failure === 'api') { assert.ok( error.message.includes( endpoint.startsWith('subscriptions') @@ -89,3 +89,54 @@ test('API diagnostics preserve resource scope, uncertainty, codes and secret sup ); } }); + +test('HTTP and body errors distinguish conflicts, authentication and server failures without leaking bodies', async (t) => { + const previous = process.env.QL_LANG; + t.after(() => { + if (previous === undefined) delete process.env.QL_LANG; + else process.env.QL_LANG = previous; + }); + for (const language of ['en', 'zh']) { + process.env.QL_LANG = language; + for (const kind of ['HTTP', 'code']) + for (const status of [400, 401, 403, 409, 500, 503]) + for (const method of ['GET', 'PUT']) { + t.mock.method( + global, + 'fetch', + async () => + new Response( + JSON.stringify({ code: status, message: 'secret-value' }), + { status: kind === 'HTTP' ? status : 200 }, + ), + ); + await assert.rejects( + request( + { url: 'https://fixture.invalid', token: 'secret-token' }, + 'envs', + { method }, + ), + (error) => { + assert.equal(error.exitCode, [401, 403].includes(status) ? 3 : 1); + assert.match(error.message, new RegExp(`${kind} ${status}`)); + assert.doesNotMatch(error.message, /secret-|fixture/); + if ([401, 403].includes(status)) + assert.match(error.message, /credentials|凭据/); + else + assert.doesNotMatch( + error.message, + /credentials|permission|凭据|权限/, + ); + if (status === 409) assert.match(error.message, /conflict|冲突/); + if (status >= 500) + assert.match(error.message, /Server error|服务端错误/); + if (status >= 500 && method === 'PUT') + assert.match(error.message, /unknown|未知/); + else assert.doesNotMatch(error.message, /unknown|未知/); + return true; + }, + ); + t.mock.restoreAll(); + } + } +}); diff --git a/test/back/unique-constraint-error.test.cjs b/test/back/unique-constraint-error.test.cjs new file mode 100644 index 00000000..69de47fb --- /dev/null +++ b/test/back/unique-constraint-error.test.cjs @@ -0,0 +1,72 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const { Sequelize, DataTypes } = require('sequelize'); +const i18n = require.resolve('../../back/shared/i18n'); +require.cache[i18n] = { + id: i18n, + filename: i18n, + loaded: true, + exports: { t: (x) => x }, +}; +const handler = require('../../back/middlewares/uniqueConstraintError').default; + +test('database uniqueness conflicts return 409 without values and preserve existing rows', async () => { + const db = new Sequelize('sqlite::memory:', { logging: false }); + try { + const App = db.define('App', { + name: { type: DataTypes.STRING, unique: true }, + }); + const Env = db.define('Env', { + name: { type: DataTypes.STRING, unique: 'pair' }, + value: { type: DataTypes.STRING, unique: 'pair' }, + }); + await db.sync(); + const first = await App.create({ name: 'private-app' }); + const second = await App.create({ name: 'other' }); + await Env.create({ name: 'TOKEN', value: 'private-value' }); + const env = await Env.create({ name: 'TOKEN', value: 'other-value' }); + for (const write of [ + () => App.create({ name: first.name }), + () => second.update({ name: first.name }), + () => env.update({ value: 'private-value' }), + ]) { + await assert.rejects(write, (err) => { + let status, body; + handler( + err, + {}, + { + status: (x) => { + status = x; + return { + json: (x) => { + body = x; + }, + }; + }, + }, + () => assert.fail('constraint passed through'), + ); + assert.equal(status, 409); + assert.equal(body.code, 409); + assert.doesNotMatch( + JSON.stringify(body), + /private-|TOKEN|INSERT|UPDATE/, + ); + return true; + }); + } + assert.equal((await second.reload()).name, 'other'); + assert.equal((await env.reload()).value, 'other-value'); + const writes = await Promise.allSettled([ + App.create({ name: 'race' }), + App.create({ name: 'race' }), + ]); + assert.equal(writes.filter((x) => x.status === 'fulfilled').length, 1); + assert.equal(await App.count({ where: { name: 'race' } }), 1); + const unrelated = new Error('unrelated'); + handler(unrelated, {}, {}, (err) => assert.equal(err, unrelated)); + } finally { + await db.close(); + } +});