diff --git a/docs/api-reference/veryfront/server.md b/docs/api-reference/veryfront/server.md index d98c1526e9..b3cbb1925c 100644 --- a/docs/api-reference/veryfront/server.md +++ b/docs/api-reference/veryfront/server.md @@ -52,7 +52,7 @@ await server.fetch(new Request("https://example.com/health")); | `parseProjectDomain` | Extract project slug and branch from domain/host header | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/utils/domain-parser.ts#L70) | | `startDevServer` | Starts dev server. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/dev-server/index.ts#L15) | | `startNodeVeryfrontServer` | Starts node veryfront server. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/service-server.ts#L575) | -| `startProductionServer` | Starts production server. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/production-server.ts#L167) | +| `startProductionServer` | Starts production server. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/production-server.ts#L168) | | `startServer` | Start a Veryfront server in development or production mode. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/index.ts#L520) | | `startVeryfrontServer` | Starts veryfront server. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/service-server.ts#L555) | | `toNodeHandler` | Convert a Web API request handler into a Node.js HTTP listener. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/node-handler.ts#L4) | @@ -72,16 +72,16 @@ await server.fetch(new Request("https://example.com/health")); | `BuildStats` | Public API contract for build stats. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/build-types.ts#L28) | | `CreateVeryfrontServerOptions` | Options accepted by create veryfront server. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/service-server.ts#L32) | | `DevServerOptions` | Options accepted by dev server. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/dev-server/types.ts#L2) | -| `DiscoveryOptions` | Configuration for AI primitives discovery during server startup | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/production-server.ts#L116) | +| `DiscoveryOptions` | Configuration for AI primitives discovery during server startup | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/production-server.ts#L117) | | `FileWatcherMetrics` | Public API contract for file watcher metrics. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/dev-server/types.ts#L33) | | `GracefulProductionShutdownOptions` | Inputs required to drain and stop a production server process. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/graceful-shutdown.ts#L25) | | `NodeVeryfrontServiceServer` | Public API contract for node veryfront service server. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/service-server.ts#L83) | | `RouteDirectory` | Public API contract for route directory. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/dev-server/types.ts#L27) | -| `ServerHandle` | Public API contract for server handle. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/production-server.ts#L153) | +| `ServerHandle` | Public API contract for server handle. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/production-server.ts#L154) | | `StartDevModeOptions` | Options accepted by start dev mode. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/index.ts#L113) | | `StartNodeVeryfrontServerOptions` | Options accepted by start node veryfront server. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/service-server.ts#L47) | | `StartProductionModeOptions` | Options accepted by start production mode. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/index.ts#L122) | -| `StartProductionServerOptions` | Options accepted by start production server. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/production-server.ts#L159) | +| `StartProductionServerOptions` | Options accepted by start production server. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/production-server.ts#L160) | | `StartServerOptions` | Server options. Defaults to development mode with HMR. Set `mode: "production"` for a production server. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/index.ts#L138) | | `StartVeryfrontServerOptions` | Options accepted by start veryfront server. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/service-server.ts#L59) | | `VeryfrontHandler` | Web API request handler with WebSocket upgrade and HMR helpers. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/server/index.ts#L153) | diff --git a/src/server/production-server.ts b/src/server/production-server.ts index 62c87ac2c7..19b0a4a63f 100644 --- a/src/server/production-server.ts +++ b/src/server/production-server.ts @@ -37,6 +37,7 @@ import { isHostProjectExecutionOverrideEnabled, } from "#veryfront/security/host-execution-policy.ts"; import { isSharedProjectRuntime } from "#veryfront/security/project-locality.ts"; +import { runStartupDiscovery } from "./startup-discovery.ts"; const serverLog = logger.component("server"); const globalLog = logger.component("global"); @@ -225,6 +226,18 @@ export function startProductionServer( // the actual data client-side after hydration. enableSSRClientOnlyFetching(); + // A dedicated single-project runtime carries the capability implicitly. + // A shared runtime intended to be the executor must be granted it by an + // operator, deliberately and visibly. + // + // Computed before discovery so startup and request handling share one + // value. They disagreed before issue-inbox#363: discovery hardcoded a + // grant while the handler computed the real posture. + const isolatedRuntimeGrant = bootstrap.config.fs?.veryfront?.proxyMode !== true && + !isSharedProjectRuntime({ adapter }); + const operatorGrant = isHostProjectExecutionOverrideEnabled(); + const allowHostProjectCodeExecution = isolatedRuntimeGrant || operatorGrant; + // Run primitive discovery before serving (registries must be populated before first request) if (discoveryConfig) { try { @@ -233,29 +246,15 @@ export function startProductionServer( "#veryfront/platform/adapters/fs/wrapper.ts" ); - if ( - discoveryConfig.projectSlug && discoveryConfig.apiToken && - discoveryConfig.fsAdapter && isExtendedFSAdapter(discoveryConfig.fsAdapter) && - discoveryConfig.fsAdapter.isMultiProjectMode() - ) { - // Multi-project proxy: scope discovery to specific project - await discoveryConfig.fsAdapter.runWithContext( - discoveryConfig.projectSlug, - discoveryConfig.apiToken, - () => - discoverAll({ - baseDir: discoveryConfig.baseDir, - fsAdapter: discoveryConfig.fsAdapter, - verbose: discoveryConfig.verbose ?? false, - }), - ); - } else { - await discoverAll({ - baseDir: discoveryConfig.baseDir, - fsAdapter: discoveryConfig.fsAdapter, - verbose: discoveryConfig.verbose ?? false, - allowHostProjectCodeExecution: true, - }); + const outcome = await runStartupDiscovery({ + config: discoveryConfig, + allowHostProjectCodeExecution, + discoverAll, + isExtendedFSAdapter, + }); + + if (!outcome.ran) { + serverLog.info("Primitive discovery skipped", { reason: outcome.reason }); } } catch (error) { serverLog.error("Primitive discovery failed", { @@ -266,13 +265,6 @@ export function startProductionServer( logger.info("Starting production server", { projectDir, port, bindAddress }); - // A dedicated single-project runtime carries the capability implicitly. - // A shared runtime intended to be the executor must be granted it by an - // operator, deliberately and visibly. - const isolatedRuntimeGrant = bootstrap.config.fs?.veryfront?.proxyMode !== true && - !isSharedProjectRuntime({ adapter }); - const operatorGrant = isHostProjectExecutionOverrideEnabled(); - if (operatorGrant && !isolatedRuntimeGrant) { logger.warn("Shared runtime is executing tenant project code by operator grant", { overrideEnv: HOST_PROJECT_EXECUTION_OVERRIDE_ENV, @@ -289,7 +281,7 @@ export function startProductionServer( defaultReleaseId, defaultEnvironment, localProjects, - allowHostProjectCodeExecution: isolatedRuntimeGrant || operatorGrant, + allowHostProjectCodeExecution, }); const coreHandler = baseHandler; diff --git a/src/server/startup-discovery.test.ts b/src/server/startup-discovery.test.ts new file mode 100644 index 0000000000..6b6ce9d6fa --- /dev/null +++ b/src/server/startup-discovery.test.ts @@ -0,0 +1,134 @@ +import "#veryfront/schemas/_test-setup.ts"; +import { assertEquals } from "#veryfront/testing/assert.ts"; +import { describe, it } from "#veryfront/testing/bdd.ts"; +import type { DiscoveryConfig, DiscoveryResult } from "#veryfront/discovery/types.ts"; +import type { FileSystemAdapter } from "#veryfront/platform/adapters/base.ts"; +import type { ExtendedFileSystemAdapter } from "#veryfront/platform/adapters/fs/wrapper.ts"; +import { runStartupDiscovery } from "./startup-discovery.ts"; + +function emptyResult(): DiscoveryResult { + return { + tools: new Map(), + agents: new Map(), + skills: new Map(), + resources: new Map(), + prompts: new Map(), + workflows: new Map(), + tasks: new Map(), + schedules: new Map(), + webhooks: new Map(), + evals: new Map(), + errors: [], + }; +} + +function recorder() { + const calls: DiscoveryConfig[] = []; + return { + calls, + discoverAll: (config: DiscoveryConfig) => { + calls.push(config); + return Promise.resolve(emptyResult()); + }, + }; +} + +/** Placeholder base dir: recorder() never touches the filesystem. */ +const PROJECT_DIR = ""; + +/** No adapter is extended, so discovery takes the unscoped branch. */ +const noExtendedAdapters = (_fs: FileSystemAdapter): _fs is ExtendedFileSystemAdapter => false; + +/** Every adapter is extended, so discovery takes the scoped branch. */ +const allExtendedAdapters = (_fs: FileSystemAdapter): _fs is ExtendedFileSystemAdapter => true; + +describe("server/startup-discovery", () => { + it("denies host execution when the deployment does not grant it", async () => { + const { calls, discoverAll } = recorder(); + + await runStartupDiscovery({ + config: { baseDir: PROJECT_DIR }, + allowHostProjectCodeExecution: false, + discoverAll, + isExtendedFSAdapter: noExtendedAdapters, + }); + + assertEquals(calls.length, 1); + assertEquals(calls[0]?.allowHostProjectCodeExecution, false); + }); + + it("grants host execution when the deployment does", async () => { + const { calls, discoverAll } = recorder(); + + await runStartupDiscovery({ + config: { baseDir: PROJECT_DIR }, + allowHostProjectCodeExecution: true, + discoverAll, + isExtendedFSAdapter: noExtendedAdapters, + }); + + assertEquals(calls[0]?.allowHostProjectCodeExecution, true); + }); + + it("skips the scoped multi-project path rather than calling discovery ungranted", async () => { + const { calls, discoverAll } = recorder(); + const fsAdapter = { + isMultiProjectMode: () => true, + runWithContext: (_slug: string, _token: string, fn: () => Promise) => fn(), + } as unknown as ExtendedFileSystemAdapter; + + const outcome = await runStartupDiscovery({ + // Even with the deployment granting, the scoped branch must not pass it: + // that path evaluates tenant source under a project context. + config: { baseDir: PROJECT_DIR, projectSlug: "p", apiToken: "t", fsAdapter }, + allowHostProjectCodeExecution: true, + discoverAll, + isExtendedFSAdapter: allExtendedAdapters, + }); + + assertEquals(outcome, { ran: false, reason: "scoped-multi-project" }); + assertEquals(calls.length, 0); + }); + + it("never calls discovery in a way the real implementation would reject", async () => { + // The stub above accepts any config, but the real `discoverAll` throws on + // an ungranted one (`discovery-engine.ts`, "Executable project discovery + // requires explicit trusted-local execution"). A permissive stub is why the + // scoped branch could call it ungranted on every startup while the suite + // stayed green, so this stub enforces the same rule the real one does. + const enforcing = (config: DiscoveryConfig) => { + if (config.allowHostProjectCodeExecution !== true) { + return Promise.reject( + new TypeError("Executable project discovery requires explicit trusted-local execution"), + ); + } + return Promise.resolve(emptyResult()); + }; + const fsAdapter = { + isMultiProjectMode: () => true, + runWithContext: (_slug: string, _token: string, fn: () => Promise) => fn(), + } as unknown as ExtendedFileSystemAdapter; + + // Scoped: must skip, so the enforcing stub is never reached. + assertEquals( + await runStartupDiscovery({ + config: { baseDir: PROJECT_DIR, projectSlug: "p", apiToken: "t", fsAdapter }, + allowHostProjectCodeExecution: true, + discoverAll: enforcing, + isExtendedFSAdapter: allExtendedAdapters, + }), + { ran: false, reason: "scoped-multi-project" }, + ); + + // Unscoped and granted: must reach discovery and be accepted. + assertEquals( + await runStartupDiscovery({ + config: { baseDir: PROJECT_DIR }, + allowHostProjectCodeExecution: true, + discoverAll: enforcing, + isExtendedFSAdapter: noExtendedAdapters, + }), + { ran: true }, + ); + }); +}); diff --git a/src/server/startup-discovery.ts b/src/server/startup-discovery.ts new file mode 100644 index 0000000000..3fbf4888a3 --- /dev/null +++ b/src/server/startup-discovery.ts @@ -0,0 +1,74 @@ +/** + * Primitive discovery run once at startup, before the server accepts requests. + * + * Extracted from `production-server.ts` so the host-execution grant it passes + * can be tested. The bug this addresses (issue-inbox#363) was that the fallback + * branch hardcoded `allowHostProjectCodeExecution: true` while the request + * handler computed the real answer fifteen lines below, so a deployment that + * denied execution at request time still granted it at startup. + * + * Same shape as veryfront-code#3364, where `api-handler-wrapper.ts` passed a + * hardcoded `true` and made the computed predicate dead code. A hardcoded + * capability sitting next to a computed one is the pattern to look for. + */ + +import type { DiscoveryConfig, DiscoveryResult } from "#veryfront/discovery/types.ts"; +import type { FileSystemAdapter } from "#veryfront/platform/adapters/base.ts"; +import type { ExtendedFileSystemAdapter } from "#veryfront/platform/adapters/fs/wrapper.ts"; +import type { DiscoveryOptions } from "./production-server.ts"; + +export interface RunStartupDiscoveryInput { + config: DiscoveryOptions; + /** + * The deployment's posture, computed once by the host-owned entrypoint and + * shared with the request handler. Never hardcoded here. + */ + allowHostProjectCodeExecution: boolean; + discoverAll: (config: DiscoveryConfig) => Promise; + isExtendedFSAdapter: (fs: FileSystemAdapter) => fs is ExtendedFileSystemAdapter; +} + +/** Whether discovery can be scoped to one project on a multi-project adapter. */ +function scopedAdapter( + input: RunStartupDiscoveryInput, +): ExtendedFileSystemAdapter | undefined { + const { config } = input; + if (!config.projectSlug || !config.apiToken || !config.fsAdapter) return undefined; + if (!input.isExtendedFSAdapter(config.fsAdapter)) return undefined; + return config.fsAdapter.isMultiProjectMode() ? config.fsAdapter : undefined; +} + +/** What startup discovery did, so the caller can report it accurately. */ +export type StartupDiscoveryOutcome = + | { ran: true } + | { ran: false; reason: "scoped-multi-project" }; + +export async function runStartupDiscovery( + input: RunStartupDiscoveryInput, +): Promise { + const { config } = input; + + if (scopedAdapter(input)) { + // Scoped to one project, so tenant source is in reach. This path stays + // ungranted whatever the deployment's posture: the capability is for a + // host-owned entrypoint evaluating its own project, not for discovery + // running inside a tenant's context. + // + // `discoverAll` refuses an ungranted config by throwing, so an ungranted + // call here cannot discover anything, it can only raise. Previously this + // branch called it anyway inside `runWithContext`, and every scoped + // multi-project startup logged "Primitive discovery failed" while the real + // meaning was "this deployment does not run startup discovery". Skipping + // is the same behaviour with an honest name and no exception as control + // flow. + return { ran: false, reason: "scoped-multi-project" }; + } + + await input.discoverAll({ + baseDir: config.baseDir, + fsAdapter: config.fsAdapter, + verbose: config.verbose ?? false, + allowHostProjectCodeExecution: input.allowHostProjectCodeExecution, + }); + return { ran: true }; +}