From 93b8041c3df4c17745567b43f9f29a42cc2da823 Mon Sep 17 00:00:00 2001 From: Neal Shah <30693865+ProgrammerIn-wonderland@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:01:52 -0400 Subject: [PATCH] don't allow prototype defined method to be called in drivers (#3413) * don't allow prototype defined method to be called in drivers * fix test --- .../drivers/DriverController.test.ts | 24 +++++ .../controllers/drivers/DriverController.ts | 23 ++++- src/backend/drivers/callableMethods.test.ts | 94 +++++++++++++++++++ src/backend/drivers/meta.ts | 62 ++++++++++++ .../workers/WorkerDriver.hotreload.test.ts | 37 ++++++++ src/backend/drivers/workers/WorkerDriver.ts | 6 ++ 6 files changed, 244 insertions(+), 2 deletions(-) create mode 100644 src/backend/drivers/callableMethods.test.ts create mode 100644 src/backend/drivers/workers/WorkerDriver.hotreload.test.ts diff --git a/src/backend/controllers/drivers/DriverController.test.ts b/src/backend/controllers/drivers/DriverController.test.ts index 476220dd8..a1168ef3e 100644 --- a/src/backend/controllers/drivers/DriverController.test.ts +++ b/src/backend/controllers/drivers/DriverController.test.ts @@ -295,6 +295,30 @@ describe('DriverController.#handleCall (via captured router)', () => { ).rejects.toMatchObject({ statusCode: 404 }); }); + // A lifecycle hook *exists* on every driver (inherited from PuterDriver), + // so the old `typeof driver[method] === 'function'` dispatch would have + // invoked it. It must not be reachable over /drivers/call. + it.each([ + 'onServerStart', + 'onServerShutdown', + 'onServerPrepareShutdown', + 'getReportedCosts', + 'constructor', + 'toString', + ])('rejects the framework method %s with 404', async (method) => { + const actor = await makeUserActor(); + const req = makeReq({ interface: 'puter-kvstore', method }, actor); + await expect( + runWithContext({ actor }, () => + routes['POST /call']( + req, + makeRes() as unknown as Response, + () => {}, + ), + ), + ).rejects.toMatchObject({ statusCode: 404 }); + }); + it('rejects a bare session ("root" token) actor on a noUserSession driver with a helpful 403', async () => { // The AI drivers all set `noUserSession` — a session token must // not double as an AI credential. The message has to point the diff --git a/src/backend/controllers/drivers/DriverController.ts b/src/backend/controllers/drivers/DriverController.ts index 51ab409a0..caccdeb2b 100644 --- a/src/backend/controllers/drivers/DriverController.ts +++ b/src/backend/controllers/drivers/DriverController.ts @@ -31,6 +31,7 @@ import type { PuterRouter } from '../../core/http/PuterRouter.js'; import type { DriverMeta } from '../../drivers/meta.js'; import { isDriverStreamResult, + resolveCallableMethods, resolveDriverMeta, resolveDriverMethodConcurrent, resolveDriverMethodRateLimit, @@ -124,6 +125,14 @@ export class DriverController extends PuterController { * lookup doesn't have to walk prototype chains on every request. */ #meta = new WeakMap(); + /** + * driver instance → the set of method names callable via `/drivers/call`. + * Resolved once at registration (server startup) via + * `resolveCallableMethods`; the request path only does a `Set.has` lookup. + * This is what stops framework/lifecycle methods (`onServerStart`, etc.) + * and `Object.prototype` members from being invoked by remote callers. + */ + #callableMethods = new WeakMap>(); constructor(...args: ConstructorParameters) { super(...args); @@ -202,14 +211,19 @@ export class DriverController extends PuterController { ); } - const fn = driver[method]; - if (typeof fn !== 'function') { + // Only methods in the pre-resolved callable set are dispatchable. + // This excludes framework/lifecycle hooks (onServerStart, etc.), + // inherited base methods, and Object.prototype members, none of + // which are part of any interface's RPC contract. + const callable = this.#callableMethods.get(driver); + if (!callable?.has(method)) { throw new HttpError( 404, `Method '${method}' not found on driver '${ifaceName}'`, { legacyCode: 'not_found' }, ); } + const fn = driver[method]; // Resolve the concrete driver name for permission keys, falling // back through prototype metadata → instance field → requested name. @@ -513,6 +527,11 @@ export class DriverController extends PuterController { // Cache the resolved meta so the request hot-path can read the // per-method rate-limit spec without re-walking the prototype. this.#meta.set(instance, meta); + // Resolve the callable RPC surface once, at startup. The request + // path checks membership against this set instead of reflecting on + // the live instance, so lifecycle hooks / inherited framework + // methods can never be dispatched. + this.#callableMethods.set(instance, resolveCallableMethods(instance)); // Register each alias pointing at the same instance. Legacy puter-js // calls that pass a provider id in the `driver` slot (e.g. the TTS // module sends `aws-polly` / `openai-tts` / `elevenlabs-tts` instead diff --git a/src/backend/drivers/callableMethods.test.ts b/src/backend/drivers/callableMethods.test.ts new file mode 100644 index 000000000..af01bb424 --- /dev/null +++ b/src/backend/drivers/callableMethods.test.ts @@ -0,0 +1,94 @@ +/** + * 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 . + */ + +import { describe, expect, it } from 'vitest'; +import { puterDrivers } from './index.js'; +import { RESERVED_DRIVER_METHODS, resolveCallableMethods } from './meta.js'; + +// Guard on the exact set of methods each driver exposes over `/drivers/call`. +// The RPC surface is derived structurally (see `resolveCallableMethods`): a +// method is callable iff it is a novel public method on the concrete driver +// class. This test pins that surface so that ADDING a plain public method to a +// driver — which would silently make it a remote endpoint — fails CI until the +// expected list here is updated. It is the "fail loud" backstop for the +// otherwise fail-open structural gate. +// +// PuterDriver's constructor signature is (config, clients, stores, services); +// field initializers don't read them, so empty mocks are fine (same trick as +// driverPolicies.test.ts). +const fake = () => [{}, {}, {}, {}] as [any, any, any, any]; + +// Expected callable surface, keyed by the registry key in `puterDrivers`. +// Keep alphabetical within each list for easy diffing. +const EXPECTED: Record = { + kvStore: [ + 'add', 'batchPut', 'decr', 'del', 'expire', 'expireAt', 'flush', + 'get', 'incr', 'list', 'remove', 'set', 'update', + ], + aiChat: ['complete', 'list', 'models'], + aiImage: ['generate', 'list', 'models'], + aiTts: ['list', 'list_engines', 'list_voices', 'synthesize'], + aiVideo: ['generate', 'list', 'models'], + aiSpeech2Speech: ['convert'], + aiSpeech2Txt: ['list_models', 'transcribe', 'translate'], + aiSpeech2TxtXai: ['list_models', 'transcribe', 'translate'], + aiOcr: ['recognize'], + // AppDriver is the legacy `.js` driver: its non-RPC helpers are plain + // public methods (not `#`-private), so `isNameAvailable` (an AppController + // helper) and `toClientView` (a safe-field projection used by the homepage + // shell) sit on the callable surface. Both are (and always were, under the + // old reflection dispatch) remotely callable — pinned here rather than + // silently exposed. See the follow-up note: lock these down by making them + // `#`-private with dedicated call sites if the exposure is unwanted. + apps: [ + 'create', 'delete', 'isNameAvailable', 'read', 'select', + 'toClientView', 'update', 'upsert', + ], + subdomains: ['create', 'delete', 'read', 'select', 'update', 'upsert'], + notifications: ['create', 'mark_acknowledged', 'mark_shown', 'read', 'select'], + workers: ['create', 'destroy', 'getFilePaths', 'getLoggingUrl'], +}; + +describe('driver callable-method surface', () => { + for (const [key, DriverClass] of Object.entries(puterDrivers)) { + const instance = new (DriverClass as new ( + ...a: [any, any, any, any] + ) => object)(...fake()); + const callable = resolveCallableMethods(instance); + + it(`${key}: exposes exactly its declared RPC methods`, () => { + expect([...callable].sort()).toEqual(EXPECTED[key]); + }); + + it(`${key}: never exposes lifecycle/framework methods`, () => { + for (const reserved of RESERVED_DRIVER_METHODS) { + expect(callable.has(reserved)).toBe(false); + } + for (const framework of [ + 'constructor', + 'toString', + 'valueOf', + 'hasOwnProperty', + 'isPrototypeOf', + ]) { + expect(callable.has(framework)).toBe(false); + } + }); + } +}); diff --git a/src/backend/drivers/meta.ts b/src/backend/drivers/meta.ts index efa2f1949..8ed42c97e 100644 --- a/src/backend/drivers/meta.ts +++ b/src/backend/drivers/meta.ts @@ -411,3 +411,65 @@ export function resolveDriverMeta( noUserSession, }; } + +/** + * Framework/lifecycle method names that must never be reachable via + * `/drivers/call`. These live on `PuterDriver` (see `drivers/types.ts`) and + * are the machinery the dispatch surface must exclude. For class-based + * drivers a concrete `override` of one still carries the same name and is + * caught here; for plain-object drivers (registered by extensions — see + * `server.ts`, `typeof DriverClass === 'object'`) there is no base prototype + * to distinguish them, so this denylist is the *only* thing keeping a + * hook off the RPC surface. Any lifecycle hook added to `PuterDriver` must + * be added here too — the per-driver guard test (`callableMethods.test.ts`) + * fails loudly if a base method starts leaking into every driver's surface. + */ +export const RESERVED_DRIVER_METHODS: ReadonlySet = new Set([ + 'onServerStart', + 'onServerPrepareShutdown', + 'onServerShutdown', + 'getReportedCosts', +]); + +/** + * Compute the set of method names a driver exposes over `/drivers/call`. + * + * The RPC surface is defined structurally rather than by a hand-maintained + * per-method allow-list. Walking from the instance up to (but not including) + * `Object.prototype`, a name is callable iff it resolves to a function and is + * neither `constructor` nor a `RESERVED_DRIVER_METHODS` entry. This covers + * both driver shapes the server accepts (`server.ts`): class instances (RPC + * methods on the concrete prototype, config on the instance) and plain + * objects (everything own, used verbatim by extensions). It excludes all + * `Object.prototype` members (`toString`, `valueOf`, `__proto__`, …), the + * `constructor`, and the lifecycle hooks. + * + * `#`-private helpers need no handling: they are not real property keys, so + * `getOwnPropertyNames` never lists them and `driver['#x']` is `undefined`. + * Only *plain* public methods can appear here. + * + * Getters are excluded (we read the descriptor's `.value`, never access the + * property), so evaluating this set never runs driver code. Intended to be + * called once per driver at registration and cached — not on the hot path. + */ +export function resolveCallableMethods(driver: object): Set { + const callable = new Set(); + const seen = new Set(); + for ( + let o: object | null = driver; + o && o !== Object.prototype; + o = Object.getPrototypeOf(o) as object | null + ) { + for (const name of Object.getOwnPropertyNames(o)) { + // First (lowest) definition wins — a subclass override shadows + // the base, and we decide against the resolved descriptor. + if (seen.has(name)) continue; + seen.add(name); + if (name === 'constructor') continue; + if (RESERVED_DRIVER_METHODS.has(name)) continue; + const desc = Object.getOwnPropertyDescriptor(o, name); + if (desc && typeof desc.value === 'function') callable.add(name); + } + } + return callable; +} diff --git a/src/backend/drivers/workers/WorkerDriver.hotreload.test.ts b/src/backend/drivers/workers/WorkerDriver.hotreload.test.ts new file mode 100644 index 000000000..a84cc5819 --- /dev/null +++ b/src/backend/drivers/workers/WorkerDriver.hotreload.test.ts @@ -0,0 +1,37 @@ +// Focused unit coverage for hot-reload subscription idempotency. The main +// WorkerDriver.test.ts boots a full server where workers aren't Cloudflare- +// configured (so listeners never register); here we drive onServerStart +// directly with a local-server config and a spy event client. +// +// Regression guard for the /drivers/call exposure fix: onServerStart was +// remotely invokable, and each invocation used to stack another set of +// fs.* listeners — so a single caller could multiply every user's +// worker-source save into N edge redeploys. Dispatch is now gated, but the +// subscription must also be structurally idempotent regardless. +import { describe, expect, it, vi } from 'vitest'; +import { WorkerDriver } from './WorkerDriver.js'; + +const build = () => { + const on = vi.fn(); + const clients = { event: { on } } as any; + const config = { workers: { localServer: true } } as any; + const driver = new WorkerDriver(config, clients, {} as any, {} as any); + return { driver, on }; +}; + +describe('WorkerDriver hot-reload subscription', () => { + it('registers each fs listener exactly once across repeated onServerStart calls', () => { + const { driver, on } = build(); + + driver.onServerStart(); + driver.onServerStart(); + driver.onServerStart(); + + expect(on).toHaveBeenCalledTimes(3); + expect(on.mock.calls.map((c) => c[0]).sort()).toEqual([ + 'fs.move.node', + 'fs.remove.node', + 'fs.write.file', + ]); + }); +}); diff --git a/src/backend/drivers/workers/WorkerDriver.ts b/src/backend/drivers/workers/WorkerDriver.ts index f350c64b1..781db51c7 100644 --- a/src/backend/drivers/workers/WorkerDriver.ts +++ b/src/backend/drivers/workers/WorkerDriver.ts @@ -110,6 +110,7 @@ export class WorkerDriver extends PuterDriver { readonly isDefault = true; #cfBaseUrl = ''; + #hotReloadSubscribed = false; static currentPreambleVersion(): string | null { return preambleVersion; @@ -622,6 +623,11 @@ export class WorkerDriver extends PuterDriver { #subscribeHotReload(): void { if (!this.#cfBaseUrl && !USE_LOCAL_WORKERD) return; + // Idempotent: re-entry (e.g. a second onServerStart) must not stack + // duplicate listeners — each duplicate would multiply redeploys on + // every user's worker-source save. + if (this.#hotReloadSubscribed) return; + this.#hotReloadSubscribed = true; this.clients.event.on( 'fs.write.file',