Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/security/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines 306 to +311

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- README references ---'
rg -n -C 6 're-?arms|VERYFRONT_HOST_ALLOW_PROJECT_EXECUTION|process lifetime|restart|replace|startup' src/security/README.md

printf '%s\n' '--- startup and environment usage ---'
rg -n -C 5 'VERYFRONT_HOST_ALLOW_PROJECT_EXECUTION|getHostEnv|production-server' src server 2>/dev/null || true

Repository: veryfront/veryfront-code

Length of output: 50381


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- security README ---'
sed -n '295,325p' src/security/README.md

printf '%s\n' '--- production server files ---'
git ls-files | rg '(^|/)production-server\.ts$|(^|/)server\.ts$' | head -n 40

printf '%s\n' '--- exact override references ---'
rg -n -C 8 'VERYFRONT_HOST_ALLOW_PROJECT_EXECUTION' src/security src/server

Repository: veryfront/veryfront-code

Length of output: 6471


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- host execution policy ---'
cat -n src/security/host-execution-policy.ts

printf '%s\n' '--- production server call sites ---'
rg -n -C 12 'isHostProjectExecutionOverrideEnabled|HOST_PROJECT_EXECUTION_OVERRIDE_ENV|requiresIsolatedProjectRuntime|create.*Handler|handler' src/server/production-server.ts src/security

printf '%s\n' '--- runtime reload or environment refresh references ---'
rg -n -C 5 'refreshEnvironmentConfig|reload|hot.?reload|restart|replace|isHostProjectExecutionOverrideEnabled' src/server src/security

Repository: veryfront/veryfront-code

Length of output: 50383


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- policy helper ---'
sed -n '1,80p' src/security/host-execution-policy.ts

printf '%s\n' '--- exact policy symbol references ---'
rg -n -F 'isHostProjectExecutionOverrideEnabled' --glob '*.ts' src

printf '%s\n' '--- exact environment key references ---'
rg -n -F 'VERYFRONT_HOST_ALLOW_PROJECT_EXECUTION' --glob '*.ts' --glob '*.md' src

printf '%s\n' '--- production server imports and startup region ---'
rg -n -C 10 -F 'host-execution-policy' src/server/production-server.ts
sed -n '1,180p' src/server/production-server.ts

Repository: veryfront/veryfront-code

Length of output: 11088


Security Misconfiguration (CWE-16)

Reachability: Internal

State that a process restart is required.

If the operator unsets VERYFRONT_HOST_ALLOW_PROJECT_EXECUTION, the operator must restart or replace the process before the change takes effect. Update the re-arming statement to make this requirement explicit.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/security/README.md` around lines 306 - 311, Update the re-arming
statement in the security documentation to explicitly state that unsetting
VERYFRONT_HOST_ALLOW_PROJECT_EXECUTION requires restarting or replacing the
process before the change takes effect.


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
Expand Down
37 changes: 37 additions & 0 deletions src/security/host-execution-policy.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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();
});
});
Comment on lines +70 to +81
});
7 changes: 4 additions & 3 deletions src/security/host-execution-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
12 changes: 6 additions & 6 deletions src/server/handlers/request/api/api-handler-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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,
Expand All @@ -125,14 +125,14 @@ export class ApiHandlerWrapper extends BaseHandler {
req: Request,
ctx: HandlerContext,
pathname: string,
isSharedRuntime: boolean,
mustDenyProjectExecution: boolean,
): Promise<HandlerResult> {
return withSpan(
"api.handleWithContext",
async () => {
try {
if (
isSharedRuntime &&
mustDenyProjectExecution &&
(pathname === "/api" || pathname.startsWith("/api/"))
) {
return this.sharedRuntimeExecutionUnavailable(req, ctx, pathname);
Expand All @@ -156,7 +156,7 @@ export class ApiHandlerWrapper extends BaseHandler {
return this.continue();
}

if (isSharedRuntime) {
if (mustDenyProjectExecution) {
return this.sharedRuntimeExecutionUnavailable(req, ctx, pathname);
}

Expand Down
22 changes: 21 additions & 1 deletion src/server/handlers/request/api/project-discovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 () => {
Expand Down
3 changes: 3 additions & 0 deletions src/server/handlers/request/api/project-discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,9 @@ export async function ensureProjectDiscovery(ctx: HandlerContext): Promise<Disco
: `${key}:snapshot:${sourceSnapshotVersion}`,
config: ctx.config,
fsAdapter: ctx.adapter.fs,
// Correct by construction, unlike a bare literal elsewhere: the
// requiresIsolatedProjectRuntime guard at the top of this function
// means control cannot reach here without the capability.
Comment on lines +199 to +201
allowHostProjectCodeExecution: true,
});
const result = await discoverAll(discoveryOptions);
Expand Down