fix: bound the inputs on the grant and token routes

Three permission routes sized their work by the request body rather than by
the route, so one call could cost far more than its rate-limit slot implies.

- `extra` and `meta` were only type-checked. They ride every row a grant
  writes (up to 16) and are re-serialised into the per-(user, app) cache on
  each miss, so they are now capped at 4 KiB.
- `/auth/create-access-token` took a `permissions` array of any length: one
  permission check and one sequential INSERT each. It now uses the same
  16-per-request cap the grant routes already had, and runs every entry
  through the validator those routes use.
- Withdrawing an app's cross-app data grants looks them up by permission
  text, and no index on `user_to_app_permissions` led with `permission`.
  Indexed per engine, mirroring what mysql_mig_22 / postgres_mig_11 /
  sqlite 0067 did for `user_to_user_permissions`.

`grant-dev-app` validated none of its input — not even the type of `extra` —
so it now runs the same validator as its user-app sibling.

Two paths could also carry a permission wider than the `varchar(255)` column
it lands in. `grantUserAppPermission` already rejected that after rewriting;
`grantDevAppPermission` now does the same, and `createAccessToken` checks it
before the session row a failing INSERT would otherwise orphan.

Caps are published in rate-limits-and-quotas.md. Every in-tree caller sends
one permission and a small `extra`, so none of them change behaviour.
This commit is contained in:
Juan Castro committed 2026-10-01 16:51:39 -04:00
1 parent c6da95beb0
commit a67cdac380
11 files changed
+312 -10

No files matched your search

@@ -27,7 +27,7 @@ import { DatabaseClientFactory } from './index.js';
import { SqliteDatabaseClient } from './SqliteDatabaseClient.js';
/** Highest schema version the migration table can reach. */
const CURRENT_SCHEMA_VERSION = 85;
const CURRENT_SCHEMA_VERSION = 86;
/**
* These suites migrate real files on disk. Idle they finish in well under a
@@ -119,6 +119,7 @@ const AVAILABLE_MIGRATIONS: [number, string[]][] = [
[82, ['0087_feedback-attachments.sql']],
[83, ['0088_apps-index-url.sql']],
[84, ['0089_team-require-2fa.sql']],
[85, ['0090_user-app-permission-index.sql']],
];
export class SqliteDatabaseClient extends AbstractDatabaseClient {
@@ -0,0 +1,39 @@
-- 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/>.
-- Mirrors SQLite migration 0090. Guarded like mysql_mig_22, because there is
-- no per-file applied-state tracking and a replay must do nothing.
DROP PROCEDURE IF EXISTS _puter_add_user_app_permission_index;
DELIMITER //
CREATE PROCEDURE _puter_add_user_app_permission_index()
BEGIN
IF NOT EXISTS (
SELECT 1 FROM INFORMATION_SCHEMA.STATISTICS
WHERE TABLE_SCHEMA = DATABASE()
AND TABLE_NAME = 'user_to_app_permissions'
AND INDEX_NAME = 'idx_user_to_app_permissions_permission'
) THEN
ALTER TABLE `user_to_app_permissions`
ADD INDEX `idx_user_to_app_permissions_permission` (`permission`);
END IF;
END//
DELIMITER ;
CALL _puter_add_user_app_permission_index();
DROP PROCEDURE IF EXISTS _puter_add_user_app_permission_index;
@@ -0,0 +1,24 @@
-- 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/>.
-- Mirrors SQLite migration 0090, as postgres_mig_11 did for user_to_user.
-- text_pattern_ops is what makes the left-anchored LIKE a range scan under a
-- non-C collation -- the plain index dev_to_app already has does not.
CREATE INDEX IF NOT EXISTS idx_user_to_app_permissions_permission
ON user_to_app_permissions (permission text_pattern_ops);
CREATE INDEX IF NOT EXISTS idx_dev_to_app_permissions_permission_pattern
ON dev_to_app_permissions (permission text_pattern_ops);
@@ -0,0 +1,26 @@
-- 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/>.
-- Withdrawing an app's cross-app data grants looks them up by permission text,
-- and no existing index on either table leads with `permission`. Same fix as
-- `idx_user_to_user_permissions_permission` in 0067.
CREATE INDEX IF NOT EXISTS `idx_user_to_app_permissions_permission`
ON `user_to_app_permissions` (`permission`);
-- The sweep reads both tables; MySQL and Postgres already index this one.
CREATE INDEX IF NOT EXISTS `idx_dev_to_app_permissions_permission`
ON `dev_to_app_permissions` (`permission`);
@@ -2201,6 +2201,49 @@ describe('AuthController grant flows', () => {
expect(res.body).toEqual({});
});
it('grant-user-app: 400 on an `extra`/`meta` larger than the cap', async () => {
const appName = `tb-${uuidv4()}`;
const app = await server.stores.app.create(
{
name: appName,
title: 'TestBoundApp',
index_url: `https://${appName}.example.test/index.html`,
},
{ ownerUserId: issuer.id },
);
const permission = 'service:tb-app:ii:read';
const oversized = { blob: 'z'.repeat(5000) };
for (const body of [
{ app_uid: app.uid, permission, extra: oversized },
{ app_uid: app.uid, permission, meta: oversized },
]) {
await expect(
inCtx(issuerActor, () =>
controller.handleGrantUserApp(
makeReq(body, { actor: issuerActor }),
makeRes(),
),
),
).rejects.toMatchObject({ statusCode: 400 });
}
const res = makeRes();
await inCtx(issuerActor, () =>
controller.handleGrantUserApp(
makeReq(
{
app_uid: app.uid,
permission,
extra: { blob: 'z'.repeat(1000) },
},
{ actor: issuerActor },
),
res,
),
);
expect(res.body).toEqual({});
});
it('grant-user-app: refuses a key-value delegation over a whole namespace', async () => {
// The prompt for it would read "let this app show your app data to
// whoever it picks", which describes no bounded capability — so the
@@ -3515,6 +3558,75 @@ describe('AuthController.handleCreateAccessToken + handleRevokeAccessToken', ()
).rejects.toMatchObject({ statusCode: 400 });
});
it('rejects a permission wider than the column, leaving no session behind', async () => {
// Nothing rewrites these, so an over-wide one would reach the INSERT
// only after the session row was created and the JWT signed.
const before = (await server.clients.db.read(
"SELECT COUNT(*) AS n FROM sessions WHERE kind = 'access_token'",
[],
)) as Array<{ n: number }>;
await expect(
controller.handleCreateAccessToken(
makeReq(
{ permissions: [`service:${'a'.repeat(300)}:ii:read`] },
{ actor },
),
makeRes(),
),
).rejects.toMatchObject({ statusCode: 400 });
const after = (await server.clients.db.read(
"SELECT COUNT(*) AS n FROM sessions WHERE kind = 'access_token'",
[],
)) as Array<{ n: number }>;
expect(after[0].n).toBe(before[0].n);
});
it('rejects more permissions than one request may carry', async () => {
// Each entry costs a permission check and an INSERT.
const permissions = Array.from(
{ length: 17 },
(_, i) => `service:cap-${i}:ii:read`,
);
await expect(
controller.handleCreateAccessToken(
makeReq({ permissions }, { actor }),
makeRes(),
),
).rejects.toMatchObject({ statusCode: 400 });
});
it('still mints at the cap', async () => {
const permissions = Array.from(
{ length: 16 },
(_, i) => `service:atcap-${i}:ii:read`,
);
const res = makeRes();
await controller.handleCreateAccessToken(
makeReq({ permissions }, { actor }),
res,
);
expect((res.body as { token: string }).token).toEqual(
expect.any(String),
);
});
it.each([
['an oversized permission string', ['x'.repeat(4097)]],
[
'an oversized `extra`',
[['service:x:ii:read', { a: 'y'.repeat(5000) }]],
],
['a non-object `extra`', [['service:x:ii:read', 'nope']]],
['an empty permission string', ['']],
])('rejects %s with 400', async (_label, permissions) => {
await expect(
controller.handleCreateAccessToken(
makeReq({ permissions: permissions as never[] }, { actor }),
makeRes(),
),
).rejects.toMatchObject({ statusCode: 400 });
});
it('mints a verifiable access-token JWT for valid permissions', async () => {
const res = makeRes();
await controller.handleCreateAccessToken(
@@ -6779,6 +6891,34 @@ describe('AuthController dev-app permission flows', () => {
).rejects.toMatchObject({ statusCode: 400 });
});
it.each([
['a non-object `extra`', { extra: 'nope' }],
['an array `extra`', { extra: [1, 2] }],
['an oversized `extra`', { extra: { blob: 'z'.repeat(5000) } }],
['an oversized `meta`', { meta: { blob: 'z'.repeat(5000) } }],
['an oversized `permission`', { permission: 'x'.repeat(4097) }],
// Survives the route cap but not the column it lands in.
[
'a `permission` wider than the column',
{ permission: `service:${'a'.repeat(300)}:ii:read` },
],
])('grant-dev-app: 400 on %s', async (_label, patch) => {
const { actor } = await makeUserAndActor();
await expect(
controller.handleGrantDevApp(
makeReq(
{
app_uid: `app-${uuidv4()}`,
permission: 'fs:read',
...patch,
},
{ actor },
),
makeRes(),
),
).rejects.toMatchObject({ statusCode: 400 });
});
it('revoke-dev-app: 400 on missing app_uid/origin/permission', async () => {
const { actor } = await makeUserAndActor();
await expect(
+43 -7
View File
@@ -116,6 +116,8 @@ const FINGERPRINT_MAX_LENGTH = 128;
// One consent prompt covers a handful of scopes at most. The cap keeps a
// crafted request from turning a single grant call into a bulk write.
const MAX_PERMISSIONS_PER_REQUEST = 16;
// Rides every row a request writes, and the per-(user, app) cache after that.
const GRANT_EXTRA_MAX_BYTES = 4096;
const DISPATCH_ID_MAX_LENGTH = 128;
// One name for the flag, so the write and the read cannot drift apart.
const APP_AUTHENTICATED_FLAG = 'flag:app-is-authenticated';
@@ -3297,6 +3299,16 @@ export class AuthController extends PuterController {
legacyCode: 'bad_request',
});
}
if (
Buffer.byteLength(JSON.stringify(value), 'utf8') >
GRANT_EXTRA_MAX_BYTES
) {
throw new HttpError(
400,
`\`${key}\` may not exceed ${GRANT_EXTRA_MAX_BYTES} bytes`,
{ legacyCode: 'bad_request' },
);
}
}
}
@@ -3864,6 +3876,13 @@ export class AuthController extends PuterController {
async handleGrantDevApp(req: Request, res: Response): Promise<void> {
let { app_uid } = req.body ?? {};
const { origin, permission, extra, meta } = req.body ?? {};
this.#validateAppPermissionParams({
app_uid,
origin,
permission,
extra,
meta,
});
if (origin && !app_uid) {
// Registered apps only, for the same reason the user-app handlers
// insist on it: a synthesised `app-<uuidv5(origin)>` is resolved
@@ -4204,6 +4223,11 @@ export class AuthController extends PuterController {
legacyCode: 'bad_request',
});
}
if (permissions.length > MAX_PERMISSIONS_PER_REQUEST) {
throw new HttpError(400, 'Too many `permissions`', {
legacyCode: 'bad_request',
});
}
// Optional user-facing name for the manage-sessions UI. Trim and clamp
// to the same 64-char limit the rename endpoint enforces.
@@ -4237,13 +4261,25 @@ export class AuthController extends PuterController {
// Normalize specs: string → [string], [string] → [string, {}], [string, extra] → as-is
const normalized = permissions.map((spec) => {
if (typeof spec === 'string') return [spec];
if (Array.isArray(spec)) return spec;
throw new HttpError(
400,
'Each permission must be a string or [string, extra?]',
{ legacyCode: 'bad_request' },
);
const entry =
typeof spec === 'string'
? [spec]
: Array.isArray(spec)
? spec
: null;
if (!entry || typeof entry[0] !== 'string' || !entry[0]) {
throw new HttpError(
400,
'Each permission must be a string or [string, extra?]',
{ legacyCode: 'bad_request' },
);
}
// Same caps the grant routes apply, before a session or row exists.
this.#validateAppPermissionParams({
permission: entry[0],
extra: entry[1],
});
return entry;
});
const token = await this.services.auth.createAccessToken(
+13 -1
View File
@@ -36,7 +36,7 @@ import type { LayerInstances } from '../../types';
import { sessionCookieFlags } from '../../util/cookieFlags.js';
import { Span } from '../../util/span.js';
import type { puterServices } from '../index';
import { FULL_API_ACCESS } from '../permission/consts';
import { FULL_API_ACCESS, PERMISSION_MAX_LEN } from '../permission/consts';
import { PuterService } from '../types';
import type {
AccessTokenPayload,
@@ -1517,6 +1517,18 @@ export class AuthService extends PuterService {
);
}
// Unrewritten: the column width is the limit, and no row exists yet.
for (const [permission] of permissions) {
if (
typeof permission === 'string' &&
permission.length > PERMISSION_MAX_LEN
) {
throw new HttpError(400, 'Invalid `permission`', {
legacyCode: 'bad_request',
});
}
}
// Full-API-access sentinel: a token that may do anything its issuing
// user can do via the API (resolved against the issuer at check time —
// see PermissionService.#scanAccessToken). Only a plain user actor may
@@ -1553,6 +1553,12 @@ export class PermissionService extends PuterService {
): Promise<void> {
permission = await this.rewritePermission(permission);
this.assertGrantableFsPermission(permission);
// Post-rewrite, for the same reason as the user-app grant above.
if (permission.length > PERMISSION_MAX_LEN) {
throw new HttpError(400, 'Invalid `permission`', {
legacyCode: 'bad_request',
});
}
const app = await this.stores.app.resolveApp(appIdentifier);
if (!app)
throw new HttpError(404, `entity_not_found: app:${appIdentifier}`, {
@@ -232,6 +232,23 @@ describe('PermissionStore', () => {
// -- prefix deletion -----------------------------------------------
describe('deleteAppGrantsByPermissionPrefix', () => {
it.each(['user_to_app_permissions', 'dev_to_app_permissions'])(
'has an index leading with `permission` on %s',
async (table) => {
// Every other index on these tables leads with another column.
const postgres = server.clients.db.engineName === 'postgres';
const rows = (await server.clients.db.read(
postgres
? 'SELECT indexname AS name FROM pg_indexes WHERE tablename = ?'
: "SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = ?",
[table],
)) as Array<{ name: string }>;
expect(rows.map((r) => r.name)).toContain(
`idx_${table}_permission`,
);
},
);
it('removes the exact permission and its subtree, across both tables', async () => {
const user = await makeUser();
const app = await makeApp(user.id);
+2 -1
View File
@@ -171,7 +171,8 @@ The Puter desktop makes PDF upload thumbnails locally within these budgets. Goin
| Limit | Value |
| ----- | ----- |
| Grant / revoke calls | 60/min |
| Permissions per grant or revoke request | 16 |
| Permissions per grant, revoke, or access-token request | 16 |
| `extra` / `meta` on a grant or access-token permission | 4 KiB each |
| Filesystem entries one `create` grant may create per request | 4 |
| Path depth a `create` grant may create below the home directory | 16 components |