fix: small fixes for perf and username checks (#3169)
Maintain Release Merge PR / update-release-pr (push) Has been cancelled
Notify HeyPuter / notify (push) Has been cancelled
release-please / release-please (push) Has been cancelled

This commit is contained in:
Daniel Salazar
2026-05-27 03:10:56 -07:00
committed by GitHub
parent 8f8efcb349
commit e95cf44fec
5 changed files with 149 additions and 3 deletions
@@ -0,0 +1,31 @@
-- Copyright (C) 2024-present Puter Technologies Inc.
--
-- This file is part of Puter.
--
-- Puter is free software: you can redistribute it and/or modify
-- it under the terms of the GNU Affero General Public License as published
-- by the Free Software Foundation, either version 3 of the License, or
-- (at your option) any later version.
--
-- This program is distributed in the hope that it will be useful,
-- but WITHOUT ANY WARRANTY; without even the implied warranty of
-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-- GNU Affero General Public License for more details.
--
-- You should have received a copy of the GNU Affero General Public License
-- along with this program. If not, see <https://www.gnu.org/licenses/>.
-- Enforce case-insensitive uniqueness on user.username.
--
-- SQLite defaults varchar columns to BINARY collation, so without this
-- index `Admin` and `admin` can coexist as separate rows even though the
-- reserved-name check and adminOnly gate both treat usernames as
-- case-insensitive. Prod MySQL already uses ascii_general_ci on this
-- column; this brings self-hosted SQLite to the same invariant.
--
-- If this CREATE fails because the DB already contains case-collision
-- duplicates, resolve them manually before re-running the migration —
-- there is no safe automatic merge of two user accounts.
CREATE UNIQUE INDEX IF NOT EXISTS idx_user_username_nocase
ON user(username COLLATE NOCASE)
WHERE username IS NOT NULL;
@@ -57,6 +57,16 @@ type RouterCache = Map<string, RequestHandler | null>;
const additionalRoutePaths: Record<string, string> = {};
// Legacy `/batch` multipart upload caps. Each file is buffered fully into
// memory before any quota / storage check runs, so without these limits an
// authenticated caller could grow the process heap proportional to whatever
// they sent. Streaming uploads go through `/writeFile`; this path is for
// pre-v2 clients only.
const BATCH_MAX_FILE_SIZE = 100 * 1024 * 1024; // 100 MiB per file
const BATCH_MAX_FILES = 64;
const BATCH_MAX_PARTS = 256;
const BATCH_MAX_FIELD_SIZE = 1 * 1024 * 1024; // 1 MiB per operation/fileinfo JSON
async function loadAdditionalRouter(
key: string,
): Promise<RequestHandler | null> {
@@ -2012,7 +2022,46 @@ export class LegacyFSController extends PuterController {
}> = [];
const fileinfos: Array<Record<string, unknown>> = [];
let parseError: Error | null = null;
const bb = Busboy({ headers: req.headers });
const bb = Busboy({
headers: req.headers,
limits: {
fileSize: BATCH_MAX_FILE_SIZE,
files: BATCH_MAX_FILES,
parts: BATCH_MAX_PARTS,
fieldSize: BATCH_MAX_FIELD_SIZE,
},
});
// Busboy emits these `*Limit` events when a configured cap is
// hit. Capture the first one as a 413 so callers get a clean
// signal instead of a silently-truncated upload.
bb.on('filesLimit', () => {
if (!parseError) {
parseError = new HttpError(
413,
`Too many files in batch (max ${BATCH_MAX_FILES})`,
{ legacyCode: 'too_large' as never },
);
}
});
bb.on('partsLimit', () => {
if (!parseError) {
parseError = new HttpError(
413,
`Too many parts in batch (max ${BATCH_MAX_PARTS})`,
{ legacyCode: 'too_large' as never },
);
}
});
bb.on('fieldsLimit', () => {
if (!parseError) {
parseError = new HttpError(
413,
'Too many fields in batch',
{ legacyCode: 'too_large' as never },
);
}
});
bb.on('field', (fieldName, value) => {
try {
@@ -2039,8 +2088,24 @@ export class LegacyFSController extends PuterController {
// For streaming uploads use the signed `/writeFile` endpoint.
bb.on('file', (_fieldName, stream, info) => {
const chunks: Buffer[] = [];
let truncated = false;
stream.on('data', (chunk: Buffer) => chunks.push(chunk));
// Busboy emits `limit` after writing the first byte past
// `fileSize`. The stream continues being drained so the
// multipart parser stays in sync, but we discard the (now
// truncated) buffer and mark the batch as failed.
stream.on('limit', () => {
truncated = true;
if (!parseError) {
parseError = new HttpError(
413,
`File in batch exceeds ${BATCH_MAX_FILE_SIZE} bytes`,
{ legacyCode: 'too_large' as never },
);
}
});
stream.on('end', () => {
if (truncated) return;
files.push({
content: Buffer.concat(chunks),
mimeType:
@@ -273,6 +273,35 @@ describe('adminOnlyGate', () => {
'forbidden',
);
});
it('admits the built-ins regardless of case', () => {
// On a SQLite self-host with case-sensitive (BINARY) collation, a
// user row could exist with `Admin`/`SYSTEM` capitalization. The
// gate must still admit those — case is normalized on both sides.
for (const username of ['Admin', 'ADMIN', 'sYsTeM']) {
const got = runGate(adminOnlyGate(), {
actor: { user: { uuid: 'u-1', username } },
});
expect(got).toBeUndefined();
}
});
it('admits case-mismatched extras (allowlist lowercased on construction)', () => {
// Pass an extra in mixed case; the gate must accept the same name
// in any case — the on-disk username row may be either, depending
// on the DB's column collation.
const gate = adminOnlyGate(['Daniel']);
expect(
runGate(gate, {
actor: { user: { uuid: 'u-1', username: 'daniel' } },
}),
).toBeUndefined();
expect(
runGate(gate, {
actor: { user: { uuid: 'u-1', username: 'DANIEL' } },
}),
).toBeUndefined();
});
});
// ── requireVerifiedGate ─────────────────────────────────────────────
+8 -2
View File
@@ -159,10 +159,16 @@ export const DEFAULT_ADMIN_USERNAMES = ['admin', 'system'] as const;
export const adminOnlyGate = (
extras: readonly string[] = [],
): RequestHandler => {
const allowList = new Set<string>([...DEFAULT_ADMIN_USERNAMES, ...extras]);
// Match the case-insensitivity guarantee of the username column
// (MySQL: ascii_general_ci; SQLite: idx_user_username_nocase). Comparing
// raw-case here would let a stored `Admin` bypass the lowercase allowlist
// on any backend that lets case-collision rows exist.
const allowList = new Set<string>(
[...DEFAULT_ADMIN_USERNAMES, ...extras].map((u) => u.toLowerCase()),
);
return (req, _res, next) => {
const username = req.actor?.user.username;
if (!username || !allowList.has(username)) {
if (!username || !allowList.has(username.toLowerCase())) {
next(
new HttpError(403, 'Only admins may request this resource', {
legacyCode: 'forbidden',
@@ -140,6 +140,21 @@ export class VoiceChangerDriver extends PuterDriver {
throw new HttpError(400, '`voice` is required', {
legacyCode: 'bad_request',
});
// `voiceId` lands in the request URL path; `modelId` lands in a
// multipart field. Both are forwarded to ElevenLabs with our
// long-lived API key, so anything other than a strict alphanumeric
// shape lets a caller steer the request at a different endpoint or
// inject parameters. ElevenLabs voice/model IDs are always
// `[A-Za-z0-9_-]+` in practice.
const ID_REGEX = /^[A-Za-z0-9_-]+$/;
if (!ID_REGEX.test(voiceId))
throw new HttpError(400, '`voice` must be alphanumeric', {
legacyCode: 'bad_request',
});
if (!ID_REGEX.test(modelId))
throw new HttpError(400, '`model` must be alphanumeric', {
legacyCode: 'bad_request',
});
// Metering: estimate duration from file size if we don't parse metadata.
// 16 kbit/s is a safe lower bound for speech audio; pre-check credits