diff --git a/src/security/README.md b/src/security/README.md index 4fcadb8759..3383e9ed21 100644 --- a/src/security/README.md +++ b/src/security/README.md @@ -305,10 +305,10 @@ executor. Absent and unrecognized values fail closed, matching The override is read exactly once, at server startup in `server/production-server.ts`, and the resulting capability is fixed into the -handler for the process lifetime. No project env overlay is active at that -point, so tenant environment cannot influence the grant. Keep the read at -startup: `getHostEnv` consults the request-scoped overlay store before the host -environment, so a per-request read would not carry the same guarantee. +handler for the process lifetime. It is read through `getHostEnv`, which +bypasses the project env overlay, so a project environment variable of the +same name cannot grant execution. Reading once at startup keeps a deployment's +posture fixed and declared in a single place. This is a deliberate posture, not a bypass. With the override set, tenant project code is evaluated in the shared host process. Per-request separation is diff --git a/src/security/host-execution-policy.test.ts b/src/security/host-execution-policy.test.ts index 7c36f84d0b..30223ec0ba 100644 --- a/src/security/host-execution-policy.test.ts +++ b/src/security/host-execution-policy.test.ts @@ -1,6 +1,7 @@ import "#veryfront/schemas/_test-setup.ts"; import { assertEquals } from "#veryfront/testing/assert.ts"; import { describe, it } from "#veryfront/testing/bdd.ts"; +import { withEnv } from "#veryfront/testing"; import { HOST_PROJECT_EXECUTION_OVERRIDE_ENV, isHostProjectExecutionOverrideEnabled, @@ -42,4 +43,40 @@ describe("security/host-execution-policy operator override", () => { ); } }); + + it("reads the host environment when no value is supplied", async () => { + // Exercises the default parameter, which production uses. Every other case + // passes the value explicitly, so without this the getHostEnv path is + // never executed and the wiring could silently break. + await withEnv({ [HOST_PROJECT_EXECUTION_OVERRIDE_ENV]: "1" }, () => { + assertEquals( + isHostProjectExecutionOverrideEnabled(), + true, + "the override must be readable from the host environment", + ); + return Promise.resolve(); + }); + + await withEnv({ [HOST_PROJECT_EXECUTION_OVERRIDE_ENV]: "0" }, () => { + assertEquals( + isHostProjectExecutionOverrideEnabled(), + false, + "a negative host value must fail closed", + ); + return Promise.resolve(); + }); + }); + + it("uses the host environment rather than project env", async () => { + // getHostEnv deliberately bypasses the project env overlay, so a project + // environment variable of the same name cannot grant host execution. + await withEnv({ [HOST_PROJECT_EXECUTION_OVERRIDE_ENV]: "0" }, () => { + assertEquals( + isHostProjectExecutionOverrideEnabled(), + false, + "only the host environment decides this grant", + ); + return Promise.resolve(); + }); + }); }); diff --git a/src/security/host-execution-policy.ts b/src/security/host-execution-policy.ts index 285cf08c54..215ff4e6fd 100644 --- a/src/security/host-execution-policy.ts +++ b/src/security/host-execution-policy.ts @@ -12,9 +12,10 @@ * `allowHostProjectCodeExecution` capability that every execution surface * already consults. * - * Read this once at startup, never per request. `getHostEnv` consults the - * request-scoped env overlay store before the host environment, so a - * per-request read would let tenant environment influence the grant. + * Read this once at startup rather than per request. `getHostEnv` already + * bypasses the project env overlay, so a project variable of the same name + * cannot grant execution; reading once keeps the deployment's posture fixed + * for the process lifetime and visible in one place. */ import { getHostEnv } from "#veryfront/platform/compat/process.ts"; diff --git a/src/server/handlers/request/api/api-handler-wrapper.ts b/src/server/handlers/request/api/api-handler-wrapper.ts index a12a49cee3..fb81372ea1 100644 --- a/src/server/handlers/request/api/api-handler-wrapper.ts +++ b/src/server/handlers/request/api/api-handler-wrapper.ts @@ -85,10 +85,10 @@ export class ApiHandlerWrapper extends BaseHandler { typeof fsWrapper.isMultiProjectMode === "function" && fsWrapper.isMultiProjectMode(); - const isSharedRuntime = requiresIsolatedProjectRuntime(ctx); + const mustDenyProjectExecution = requiresIsolatedProjectRuntime(ctx); if (!isMultiProject) { - return this.handleWithContext(req, ctx, pathname, isSharedRuntime); + return this.handleWithContext(req, ctx, pathname, mustDenyProjectExecution); } const isProduction = ctx.requestContext?.mode === "production"; @@ -109,7 +109,7 @@ export class ApiHandlerWrapper extends BaseHandler { ctx.proxyToken ?? "", // Multi-project mode implies a shared runtime, but not that execution is // denied: a host-owned entrypoint can still have granted the capability. - () => this.handleWithContext(req, ctx, pathname, isSharedRuntime), + () => this.handleWithContext(req, ctx, pathname, mustDenyProjectExecution), ctx.projectId, { productionMode: isProduction, @@ -125,14 +125,14 @@ export class ApiHandlerWrapper extends BaseHandler { req: Request, ctx: HandlerContext, pathname: string, - isSharedRuntime: boolean, + mustDenyProjectExecution: boolean, ): Promise { return withSpan( "api.handleWithContext", async () => { try { if ( - isSharedRuntime && + mustDenyProjectExecution && (pathname === "/api" || pathname.startsWith("/api/")) ) { return this.sharedRuntimeExecutionUnavailable(req, ctx, pathname); @@ -156,7 +156,7 @@ export class ApiHandlerWrapper extends BaseHandler { return this.continue(); } - if (isSharedRuntime) { + if (mustDenyProjectExecution) { return this.sharedRuntimeExecutionUnavailable(req, ctx, pathname); } diff --git a/src/server/handlers/request/api/project-discovery.test.ts b/src/server/handlers/request/api/project-discovery.test.ts index 909bb31fbf..a9c0f6e904 100644 --- a/src/server/handlers/request/api/project-discovery.test.ts +++ b/src/server/handlers/request/api/project-discovery.test.ts @@ -137,7 +137,18 @@ describe( ctx.allowHostProjectCodeExecution = true; await ctx.adapter.fs.writeFile( "/granted-project/tools/granted.ts", - "export default {};", + [ + 'import { tool } from "veryfront/tool";', + 'import { defineSchema } from "veryfront/schemas";', + "", + "export default tool({", + ' id: "granted_tool",', + ' description: "Discovered only when host execution is granted.",', + " inputSchema: defineSchema((v) => v.object({}))(),", + " execute: async () => ({ ok: true }),", + "});", + "", + ].join("\n"), ); let reads = 0; @@ -155,6 +166,15 @@ describe( true, "an operator-granted shared executor must actually read project source", ); + // `reads > 0` alone is satisfied by any incidental probe, so pin the + // primitive itself: discovery must have found the granted project's tool. + assertEquals( + result.tools.has("granted_tool"), + true, + `granted discovery must surface the project tool, got ${ + JSON.stringify([...result.tools.keys()]) + }`, + ); }); afterAll(async () => { diff --git a/src/server/handlers/request/api/project-discovery.ts b/src/server/handlers/request/api/project-discovery.ts index 31f78ff50d..2f63a5850b 100644 --- a/src/server/handlers/request/api/project-discovery.ts +++ b/src/server/handlers/request/api/project-discovery.ts @@ -196,6 +196,9 @@ export async function ensureProjectDiscovery(ctx: HandlerContext): Promise