fix: treat access tokens as non-auth unless opted in (#3111)

* fix: treat access tokens as non-auth unless opted in

* fix: tests
This commit is contained in:
Daniel Salazar authored and GitHub committed 2026-05-13 14:20:15 -07:00
1 parent 6353a08fd5
commit 0276420213
14 files changed
+156 -152

No files matched your search

+1 -1
View File
@@ -90,7 +90,7 @@ const DEFAULT_BATCH_WRITE_SIDE_EFFECT_CONCURRENCY = 8;
@Controller('/fs')
export class FSController extends PuterController {
override getReportedCosts(): Record<string, unknown>[] {
override getReportedCosts() {
return Object.entries(FS_COSTS).map(([usageType, ucentsPerUnit]) => ({
usageType,
ucentsPerUnit,
@@ -43,6 +43,7 @@ import {
signingConfigFromAppConfig,
toLegacyEntry,
} from './legacyFsHelpers.js';
import { RouteOptions } from '../../core/http/index.js';
/**
* Legacy FS routes, implemented as thin shims over `FSService`.
@@ -79,11 +80,16 @@ export class LegacyFSController extends PuterController {
#additionalCache: RouterCache = new Map();
registerRoutes(router: PuterRouter): void {
const apiOptions = { subdomain: 'api', requireVerified: true } as const;
const apiOptions = {
subdomain: 'api',
requireVerified: true,
} as RouteOptions;
// Signed-URL routes: the handler validates the URL signature itself,
// so no auth gate is applied (matches v1, which mounted these routers
// with no middleware).
const signedOptions = { subdomain: 'api' } as const;
const signedOptions = {
subdomain: 'api',
} as RouteOptions;
// Core filesystem_api routes — direct handlers over the FS service.
router.post('/stat', apiOptions, this.stat);
@@ -98,7 +104,11 @@ export class LegacyFSController extends PuterController {
router.get('/read', apiOptions, this.read);
router.get(
'/token-read',
{ subdomain: 'api', requireVerified: false },
{
subdomain: 'api',
requireVerified: false,
allowAccessToken: true,
},
this.tokenRead,
);
@@ -23,7 +23,7 @@ import type { FSEntry } from '../../stores/fs/FSEntry.js';
import type { FSEntryStore } from '../../stores/fs/FSEntryStore.js';
import type { FSService } from '../../services/fs/FSService.js';
import type { ACLService, AclMode } from '../../services/acl/ACLService.js';
import type { Actor } from '../../core/actor.js';
import { isAppActor, type Actor } from '../../core/actor.js';
import { Context } from '../../core/context.js';
import type { EventClient } from '../../clients/event/EventClient.js';
import { HttpError } from '../../core/http/HttpError.js';
@@ -202,8 +202,8 @@ export async function assertAccess(
// App-under-user actors see denials as 404 "subject_does_not_exist"
// so existence of a sibling user's / other-app's files isn't leaked
// through the error code. User-actor denials keep the real 403.
const isAppActor = Boolean((actor as { app?: unknown })?.app);
if (isAppActor) {
if (isAppActor(actor)) {
throw new HttpError(404, `Entry not found: path=${path}`, {
legacyCode: 'subject_does_not_exist',
});
@@ -38,7 +38,7 @@ export class NotificationController extends PuterController {
* POST /notif/mark-ack — user dismissed a notification.
* Sets `acknowledged` timestamp; pushes ack event to sockets.
*/
@Post('/mark-ack', { subdomain: 'api', requireAuth: true })
@Post('/mark-ack', { subdomain: 'api', requireUserActor: true })
async markAck(req: Request, res: Response): Promise<void> {
const uid = req.body?.uid;
if (typeof uid !== 'string' || uid.length === 0) {
@@ -77,7 +77,7 @@ export class NotificationController extends PuterController {
* POST /notif/mark-read — user saw a notification.
* Sets `shown` timestamp; pushes ack event to sockets.
*/
@Post('/mark-read', { subdomain: 'api', requireAuth: true })
@Post('/mark-read', { subdomain: 'api', requireUserActor: true })
async markRead(req: Request, res: Response): Promise<void> {
const uid = req.body?.uid;
if (typeof uid !== 'string' || uid.length === 0) {
@@ -73,11 +73,6 @@ const makeUserActor = (): Actor => ({
user: { id: 7, uuid: 'u-7', username: 'alice' },
});
const makeAppActor = (): Actor => ({
user: { id: 7, uuid: 'u-7', username: 'alice' },
app: { id: 1, uid: 'app-uid' },
});
interface CapturedResponse {
statusCode: number;
body: unknown;
@@ -193,13 +188,23 @@ describe('PuterAIController.registerRoutes', () => {
]),
);
const chatRoute = calls.find(
(c) => c.path === '/puterai/openai/v1/chat/completions',
);
expect(chatRoute?.opts).toEqual({
subdomain: 'api',
requireAuth: true,
});
// The four upstream-proxy routes are user-only: app actors must
// call puter-chat-completion directly. The `requireUserActor`
// route option wires up the gate that enforces this, so each
// proxy route gets the assertion (not just chat/completions).
const userOnlyPaths = [
'/puterai/openai/v1/chat/completions',
'/puterai/openai/v1/completions',
'/puterai/openai/v1/responses',
'/puterai/anthropic/v1/messages',
];
for (const path of userOnlyPaths) {
const route = calls.find((c) => c.path === path);
expect(route?.opts).toEqual({
subdomain: 'api',
requireUserActor: true,
});
}
const modelsRoute = calls.find(
(c) => c.path === '/puterai/chat/models',
);
@@ -213,18 +218,10 @@ describe('PuterAIController.registerRoutes', () => {
// ── /openai/v1/chat/completions ─────────────────────────────────────
describe('PuterAIController.openaiChatCompletions', () => {
it('rejects app actors with HttpError 403', async () => {
const { res } = makeRes();
await expect(
controller.openaiChatCompletions(
makeReq({
body: { messages: [], model: 'gpt-test' },
actor: makeAppActor(),
}),
res,
),
).rejects.toMatchObject({ statusCode: 403 });
});
// Note: app-actor rejection (403) is enforced by the `requireUserActor`
// route gate and is asserted in the registerRoutes test above. The
// handler itself no longer does that check, so unit-testing it here
// would require invoking the gate stack.
it('rejects bodies missing a messages array with HttpError 400', async () => {
const { res } = makeRes();
@@ -272,13 +269,13 @@ describe('PuterAIController.openaiChatCompletions', () => {
const body = captured.body as Record<string, unknown>;
expect(body.object).toBe('chat.completion');
expect(body.model).toBe('gpt-test');
expect((body.choices as Array<Record<string, unknown>>)[0]).toMatchObject(
{
index: 0,
message: { role: 'assistant', content: 'hi there' },
finish_reason: 'stop',
},
);
expect(
(body.choices as Array<Record<string, unknown>>)[0],
).toMatchObject({
index: 0,
message: { role: 'assistant', content: 'hi there' },
finish_reason: 'stop',
});
// total_tokens is computed from prompt + completion.
expect(body.usage).toEqual({
prompt_tokens: 4,
@@ -392,18 +389,8 @@ describe('PuterAIController.openaiChatCompletions', () => {
// ── /openai/v1/completions ──────────────────────────────────────────
describe('PuterAIController.openaiCompletions', () => {
it('rejects app actors with HttpError 403', async () => {
const { res } = makeRes();
await expect(
controller.openaiCompletions(
makeReq({
body: { prompt: 'hi', model: 'gpt-test' },
actor: makeAppActor(),
}),
res,
),
).rejects.toMatchObject({ statusCode: 403 });
});
// App-actor rejection lives in the `requireUserActor` gate now;
// see the registerRoutes assertion.
it('rejects a non-string prompt with HttpError 400', async () => {
const { res } = makeRes();
@@ -443,13 +430,13 @@ describe('PuterAIController.openaiCompletions', () => {
const body = captured.body as Record<string, unknown>;
expect(body.object).toBe('text_completion');
expect((body.choices as Array<Record<string, unknown>>)[0]).toMatchObject(
{
text: 'response',
index: 0,
finish_reason: 'stop',
},
);
expect(
(body.choices as Array<Record<string, unknown>>)[0],
).toMatchObject({
text: 'response',
index: 0,
finish_reason: 'stop',
});
expect((body.id as string).startsWith('cmpl-')).toBe(true);
});
});
@@ -457,18 +444,8 @@ describe('PuterAIController.openaiCompletions', () => {
// ── /openai/v1/responses ────────────────────────────────────────────
describe('PuterAIController.openaiResponses', () => {
it('rejects app actors with HttpError 403', async () => {
const { res } = makeRes();
await expect(
controller.openaiResponses(
makeReq({
body: { input: 'hi', model: 'gpt-test' },
actor: makeAppActor(),
}),
res,
),
).rejects.toMatchObject({ statusCode: 403 });
});
// App-actor rejection lives in the `requireUserActor` gate now;
// see the registerRoutes assertion.
it('rejects providers other than openai-responses with HttpError 400', async () => {
const { res } = makeRes();
@@ -537,18 +514,8 @@ describe('PuterAIController.openaiResponses', () => {
// ── /anthropic/v1/messages ──────────────────────────────────────────
describe('PuterAIController.anthropicMessages', () => {
it('rejects app actors with HttpError 403', async () => {
const { res } = makeRes();
await expect(
controller.anthropicMessages(
makeReq({
body: { messages: [], model: 'claude-test' },
actor: makeAppActor(),
}),
res,
),
).rejects.toMatchObject({ statusCode: 403 });
});
// App-actor rejection lives in the `requireUserActor` gate now;
// see the registerRoutes assertion.
it('rejects bodies missing a messages array with HttpError 400', async () => {
const { res } = makeRes();
@@ -655,9 +622,8 @@ describe('PuterAIController model listing', () => {
const captureGetHandler = (
path: string,
): ((req: Request, res: Response) => Promise<void>) => {
let handler:
| ((req: Request, res: Response) => Promise<void>)
| null = null;
let handler: ((req: Request, res: Response) => Promise<void>) | null =
null;
const router = {
post: vi.fn(),
get: vi.fn((p: string, _opts: unknown, h: never) => {
@@ -719,9 +685,8 @@ describe('PuterAIController videoProxy', () => {
req: Request,
res: Response,
) => Promise<void>) => {
let handler:
| ((req: Request, res: Response) => Promise<void>)
| null = null;
let handler: ((req: Request, res: Response) => Promise<void>) | null =
null;
const router = {
post: vi.fn(),
get: vi.fn((path: string, _opts: unknown, h: never) => {
@@ -799,8 +764,9 @@ describe('PuterAIController videoProxy', () => {
it('500s when url_signature_secret is not configured', async () => {
// Temporarily blank the secret so the controller hits the
// 500 branch instead of the constant-time-compare gate.
const cfg = (controller as unknown as { config: Record<string, unknown> })
.config;
const cfg = (
controller as unknown as { config: Record<string, unknown> }
).config;
const orig = cfg.url_signature_secret;
cfg.url_signature_secret = undefined;
try {
@@ -826,8 +792,9 @@ describe('PuterAIController videoProxy', () => {
// Hit the post-signature `provider !== 'gemini'` branch by
// computing a valid signature for a known fileId/expires combo
// and then sending a different provider in the query.
const cfg = (controller as unknown as { config: Record<string, unknown> })
.config;
const cfg = (
controller as unknown as { config: Record<string, unknown> }
).config;
const secret = cfg.url_signature_secret as string;
const fileId = 'abc-123';
const expires = String(Math.floor(Date.now() / 1000) + 60);
@@ -854,11 +821,16 @@ describe('PuterAIController videoProxy', () => {
});
it('500s when provider=gemini but no Gemini API key is configured', async () => {
const cfg = (controller as unknown as {
config: Record<string, unknown> & {
providers?: Record<string, Record<string, unknown> | undefined>;
};
}).config;
const cfg = (
controller as unknown as {
config: Record<string, unknown> & {
providers?: Record<
string,
Record<string, unknown> | undefined
>;
};
}
).config;
const secret = cfg.url_signature_secret as string;
const fileId = 'gemini-no-key';
const expires = String(Math.floor(Date.now() / 1000) + 60);
@@ -1425,7 +1397,11 @@ describe('PuterAIController.anthropicMessages streaming + helpers', () => {
);
const tools = completeSpy.mock.calls[0]![0].tools as Array<{
type: string;
function: { name: string; description: string; parameters: unknown };
function: {
name: string;
description: string;
parameters: unknown;
};
}>;
expect(tools[0]?.type).toBe('function');
expect(tools[0]?.function.name).toBe('lookup');
@@ -1483,9 +1459,7 @@ describe('PuterAIController.anthropicMessages streaming + helpers', () => {
res,
);
const body = captured.body as { content: Array<{ text: string }> };
expect(body.content).toEqual([
{ type: 'text', text: 'hello world' },
]);
expect(body.content).toEqual([{ type: 'text', text: 'hello world' }]);
});
it('reads tool_use blocks from message.content (not just message.tool_calls)', async () => {
@@ -1536,9 +1510,8 @@ describe('PuterAIController model details', () => {
const captureGetHandler = (
path: string,
): ((req: Request, res: Response) => Promise<void>) => {
let handler:
| ((req: Request, res: Response) => Promise<void>)
| null = null;
let handler: ((req: Request, res: Response) => Promise<void>) | null =
null;
const router = {
post: vi.fn(),
get: vi.fn((p: string, _opts: unknown, h: never) => {
@@ -17,19 +17,19 @@
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
import type { Request, Response } from 'express';
import crypto from 'node:crypto';
import { Readable } from 'node:stream';
import type { Request, Response } from 'express';
import { HttpError } from '../../core/http/HttpError.js';
import { isAppActor } from '../../core/actor.js';
import { RouteOptions } from '../../core/http/index.js';
import type { PuterRouter } from '../../core/http/PuterRouter.js';
import { PuterController } from '../types.js';
import { isDriverStreamResult } from '../../drivers/meta.js';
import type { ChatCompletionDriver } from '../../drivers/ai-chat/ChatCompletionDriver.js';
import type {
ICompleteArguments,
IChatCompleteResult,
ICompleteArguments,
} from '../../drivers/ai-chat/types.js';
import { isDriverStreamResult } from '../../drivers/meta.js';
import { PuterController } from '../types.js';
const GEMINI_DOWNLOAD_BASE =
'https://generativelanguage.googleapis.com/download/v1beta/files';
@@ -49,7 +49,10 @@ const GEMINI_DOWNLOAD_BASE =
*/
export class PuterAIController extends PuterController {
registerRoutes(router: PuterRouter): void {
const apiAuthOpts = { subdomain: 'api', requireAuth: true } as const;
const apiAuthOpts = {
subdomain: 'api',
requireUserActor: true,
} as RouteOptions;
const publicOpts = { subdomain: 'api', requireAuth: false } as const;
// Every route below carries the `/puterai` prefix for wire
@@ -229,8 +232,6 @@ export class PuterAIController extends PuterController {
req: Request,
res: Response,
): Promise<void> => {
this.#rejectAppActor(req);
const body = asRecord(req.body);
const stream = !!body.stream;
@@ -396,8 +397,6 @@ export class PuterAIController extends PuterController {
// ── /openai/v1/completions ──────────────────────────────────────
openaiCompletions = async (req: Request, res: Response): Promise<void> => {
this.#rejectAppActor(req);
const body = asRecord(req.body);
const stream = !!body.stream;
@@ -526,8 +525,6 @@ export class PuterAIController extends PuterController {
// ── /openai/v1/responses ────────────────────────────────────────
openaiResponses = async (req: Request, res: Response): Promise<void> => {
this.#rejectAppActor(req);
const body = asRecord(req.body);
const stream = !!body.stream;
@@ -838,8 +835,6 @@ export class PuterAIController extends PuterController {
// ── /anthropic/v1/messages ──────────────────────────────────────
anthropicMessages = async (req: Request, res: Response): Promise<void> => {
this.#rejectAppActor(req);
const body = asRecord(req.body);
const stream = !!body.stream;
@@ -1052,17 +1047,6 @@ export class PuterAIController extends PuterController {
});
return driver;
}
#rejectAppActor(req: Request): void {
// Proxy routes are user-only; apps must call puter-chat-completion directly.
if (isAppActor(req.actor)) {
throw new HttpError(
403,
'App actors may not proxy to upstream AI APIs',
{ legacyCode: 'forbidden' },
);
}
}
}
// ── Shared helpers ──────────────────────────────────────────────────
+4 -1
View File
@@ -92,7 +92,10 @@ export const PuterController =
public onServerShutdown() {
return;
}
public async getReportedCosts() {
public getReportedCosts(): // eslint-disable-next-line @typescript-eslint/no-explicit-any
| Promise<Record<string, any>[]>
// eslint-disable-next-line @typescript-eslint/no-explicit-any
| Record<string, any>[] {
return [];
}
public registerRoutes(_router: PuterRouter) {}
+1 -1
View File
@@ -64,7 +64,7 @@ export const isSystemActor = (actor: Actor | undefined | null): boolean => {
};
export const isAppActor = (actor: Actor | undefined | null): boolean => {
return !!actor?.app && !actor?.accessToken;
return !!actor?.app && !isAccessTokenActor(actor);
};
export const isAccessTokenActor = (
+21
View File
@@ -114,6 +114,27 @@ export const requireUserActorGate = (): RequestHandler => {
};
};
export const requireNonAccessTokenGate = (): RequestHandler => {
return (req, _res, next) => {
const actor = req.actor;
if (!actor) {
next(rejectAuth(req));
return;
}
if (actor.accessToken) {
next(
new HttpError(
403,
'Access tokens are not allowed to access this resource',
{ legacyCode: 'forbidden' },
),
);
return;
}
next();
};
};
/** Built-in admin usernames that always pass `adminOnly`. */
export const DEFAULT_ADMIN_USERNAMES = ['admin', 'system'] as const;
+4 -1
View File
@@ -80,12 +80,15 @@ export interface RouteOptions {
*/
subdomain?: string | string[];
/** Reject anonymous + suspended-user requests with 401/403. */
/** Reject anonymous + suspended-user requests with 401/403. Only allows user and app actors */
requireAuth?: boolean;
/** Reject app/access-token actors. Implies `requireAuth`. */
requireUserActor?: boolean;
/** Allows access-tokens */
allowAccessToken?: boolean;
/**
* Reject unless the actor's username is `admin`, `system`, or one of the
* extras in this array. `true` means just `admin`/`system`; an array adds
+8 -8
View File
@@ -64,13 +64,13 @@ const meta = (
describe('KVStoreDriver — rate-limit policy', () => {
const m = meta(new KVStoreDriver(...fake()));
it('pins the kv tier values (registered 200 / 10s, temp 100 / 10s)', () => {
it('pins the kv tier values (registered 400 / 10s, temp 200 / 10s)', () => {
expect(m.rateLimit?.default).toEqual({
limit: 200,
limit: 400,
window: 10_000,
bySubscription: {
[DEFAULT_FREE_SUBSCRIPTION]: 200,
[DEFAULT_TEMP_SUBSCRIPTION]: 100,
[DEFAULT_FREE_SUBSCRIPTION]: 400,
[DEFAULT_TEMP_SUBSCRIPTION]: 200,
},
});
});
@@ -98,13 +98,13 @@ describe('AppDriver — rate-limit policy', () => {
describe('SubdomainDriver — rate-limit policy', () => {
const m = meta(new SubdomainDriver(...fake()));
it('matches AppDriver tier values (subdomain CRUD is similarly cheap)', () => {
it('pins the subdomain tier values (registered 200 / 10s, temp 100 / 10s)', () => {
expect(m.rateLimit?.default).toEqual({
limit: 100,
limit: 200,
window: 10_000,
bySubscription: {
[DEFAULT_FREE_SUBSCRIPTION]: 100,
[DEFAULT_TEMP_SUBSCRIPTION]: 50,
[DEFAULT_FREE_SUBSCRIPTION]: 200,
[DEFAULT_TEMP_SUBSCRIPTION]: 100,
},
});
});
+6 -6
View File
@@ -37,11 +37,11 @@ import { AI_CONCURRENT, AI_RATE_LIMIT } from './aiLimits.js';
describe('AI_RATE_LIMIT', () => {
it('pins the documented tier values', () => {
expect(AI_RATE_LIMIT.default).toEqual({
limit: 100,
limit: 200,
window: 10_000,
bySubscription: {
[DEFAULT_FREE_SUBSCRIPTION]: 30,
[DEFAULT_TEMP_SUBSCRIPTION]: 20,
[DEFAULT_FREE_SUBSCRIPTION]: 60,
[DEFAULT_TEMP_SUBSCRIPTION]: 40,
},
});
});
@@ -75,10 +75,10 @@ describe('AI_RATE_LIMIT', () => {
describe('AI_CONCURRENT', () => {
it('pins the documented tier values', () => {
expect(AI_CONCURRENT.default).toEqual({
limit: 10,
limit: 20,
bySubscription: {
[DEFAULT_FREE_SUBSCRIPTION]: 3,
[DEFAULT_TEMP_SUBSCRIPTION]: 2,
[DEFAULT_FREE_SUBSCRIPTION]: 6,
[DEFAULT_TEMP_SUBSCRIPTION]: 4,
},
});
});
+12 -2
View File
@@ -37,6 +37,7 @@ import {
allowedAppIdsGate,
requireAuthGate,
requireEmailConfirmedGate,
requireNonAccessTokenGate,
requireUserActorGate,
requireVerifiedGate,
subdomainGate,
@@ -791,7 +792,9 @@ export class PuterServer {
opts.allowedAppIds ||
opts.requireVerified,
);
if (needsAuth) mwChain.push(requireAuthGate());
if (needsAuth) {
mwChain.push(requireAuthGate());
}
// Default-on email confirmation gate. Every authenticated route
// rejects users pending confirmation unless `allowUnconfirmed`
@@ -802,6 +805,11 @@ export class PuterServer {
mwChain.push(requireEmailConfirmedGate());
}
// block access tokens by default
if (needsAuth && !opts.allowAccessToken) {
mwChain.push(requireNonAccessTokenGate());
}
// `requireVerified` intentionally does NOT imply `requireUserActor`:
// FS routes (and similar) want the user's email to be confirmed even
// when an app acts on the user's behalf. `requireVerifiedGate` reads
@@ -813,7 +821,9 @@ export class PuterServer {
// token, not only from browser sessions. `adminOnlyGate` gates on
// `actor.user.username`, which is populated for access-token and
// app-under-user actors alike.
if (opts.requireUserActor) mwChain.push(requireUserActorGate());
if (opts.requireUserActor) {
mwChain.push(requireUserActorGate());
}
if (opts.adminOnly) {
const extras = Array.isArray(opts.adminOnly) ? opts.adminOnly : [];
+3 -3
View File
@@ -17,13 +17,13 @@
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
import type { Server as HttpServer } from 'node:http';
import { createAdapter } from '@socket.io/redis-streams-adapter';
import type { Server as HttpServer } from 'node:http';
import { Server as SocketIOServer, type Socket } from 'socket.io';
import type { Actor } from '../../core/actor.js';
import { isAppActor, isAccessTokenActor } from '../../core/actor.js';
import { PuterService } from '../types.js';
import { isAccessTokenActor, isAppActor } from '../../core/actor.js';
import type { AuthService } from '../auth/AuthService.js';
import { PuterService } from '../types.js';
/**
* Socket push target. A `room` fans to every socket in that room; a