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
4 changes: 2 additions & 2 deletions docs/api-reference/veryfront/security.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ applySecurityHeaders(response.headers, false, generateNonce(), null);
| `cors` | Create CORS middleware. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/http/cors/middleware.ts#L10) |
| `corsSimple` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/http/cors/middleware.ts#L39) |
| `createResponseBuilder` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/http/response/builder.ts#L60) |
| `createSecureFs` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/secure-fs.ts#L1129) |
| `createSecureFs` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/secure-fs.ts#L1144) |
| `createValidatedHandler` | Create a validated API handler with bounded body/query validation. Bodies without a schema are preflighted through a clone, leaving the original request body available to the handler after its size is verified. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/input-validation/handler.ts#L163) |
| `createValidationError` | Create an input validation error. Convenience wrapper around INPUT_VALIDATION_FAILED.create(). | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/input-validation/errors.ts#L12) |
| `createValidator` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/path-validation/index.ts#L446) |
Expand Down Expand Up @@ -88,7 +88,7 @@ applySecurityHeaders(response.headers, false, generateNonce(), null);
| `validatePath` | Admit a path against the physical semantics of a runtime filesystem. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/path-validation/index.ts#L330) |
| `validatePathSync` | Validate lexical path containment without consulting a filesystem. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/path-validation/index.ts#L431) |
| `validateRequestLimits` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/input-validation/limits.ts#L106) |
| `wrapAdapterWithSecurity` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/secure-fs.ts#L1188) |
| `wrapAdapterWithSecurity` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/secure-fs.ts#L1203) |

### Classes

Expand Down
29 changes: 28 additions & 1 deletion src/platform/adapters/file-system-capabilities.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ const captureExclusiveCreateCapability = (
).captureExclusiveCreateCapability!;
const captureStaticReadCapabilities = (
capabilityModule as unknown as {
captureStaticReadCapabilities?: (value: unknown, label?: string) => StaticReaders;
captureStaticReadCapabilities?: (
value: unknown,
label?: string,
allowExplicitUndefined?: boolean,
) => StaticReaders;
}
).captureStaticReadCapabilities!;
const captureLegacyFileSystemCapabilitiesForSnapshot = (
Expand Down Expand Up @@ -236,6 +240,29 @@ describe("platform/adapters/file-system-capabilities", () => {
assertEquals(unrelatedReads, 0);
});

it("keeps virtual authority when a wrapper publishes absent slots as undefined", () => {
// FSAdapterWrapper freezes every optional capability slot, publishing
// `undefined` for the ones the adapter does not implement. Strict capture
// threw on that shape, and SecureFs swallows the throw, so wrapper-backed
// filesystems lost virtual snapshot authority silently rather than loudly.
const wrapperShaped = {
symlinkSemantics: "none",
getSourceSnapshotVersion: () => 7,
readFileBytes: () => Promise.resolve(new Uint8Array([1])),
readFileBytesWithinLimit: () => Promise.resolve(new Uint8Array([1])),
maxWholeFileReadBytes: 1024,
// Published but unimplemented, exactly as the wrapper does.
readFileSnapshotWithinLimit: undefined,
};

assertThrows(() => captureStaticReadCapabilities(wrapperShaped));

const captured = captureStaticReadCapabilities(wrapperShaped, "Filesystem", true);
assertEquals(captured.snapshot, undefined);
assertEquals(typeof captured.virtual?.generation, "function");
assertEquals(typeof captured.virtual?.exact, "function");
});

it("returns undefined only when a single-purpose raw method is absent", () => {
assertEquals(captureSnapshotReadCapability({}), undefined);
assertEquals(captureExclusiveCreateCapability({}), undefined);
Expand Down
20 changes: 17 additions & 3 deletions src/platform/adapters/file-system-capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -698,12 +698,26 @@ export function captureExclusiveCreateCapability(
return freezeObject(captured);
}

/**
* Capture snapshot and virtual read authority from a filesystem.
*
* `allowExplicitUndefined` stays strict by default, matching
* {@link captureSnapshotReadCapability}: for a raw adapter, a capability key
* present with the value `undefined` is a defect worth surfacing.
*
* Callers handed an `FSAdapterWrapper` must opt in, because that wrapper
* publishes every optional capability as a frozen own property, `undefined`
* included, so project code cannot inject one later. Without the opt-in this
* threw, and its only caller wraps it in a catch, so wrapper-backed
* filesystems silently lost virtual snapshot authority instead of failing.
*/
export function captureStaticReadCapabilities(
value: unknown,
label = "Filesystem",
allowExplicitUndefined = false,
): CapturedStaticReaders {
requireCapabilityObject(value, label);
const snapshot = captureSnapshotReadCapability(value, label);
const snapshot = captureSnapshotReadCapability(value, label, allowExplicitUndefined);
const captured = createObject(null) as CapturedStaticReaders;
if (snapshot !== undefined) captured.snapshot = snapshot;

Expand All @@ -727,7 +741,7 @@ export function captureStaticReadCapabilities(
generationProperties.getSourceSnapshotVersion,
label,
"getSourceSnapshotVersion",
false,
allowExplicitUndefined,
);
if (rawGeneration === undefined) return freezeObject(captured);

Expand All @@ -740,7 +754,7 @@ export function captureStaticReadCapabilities(
readerProperties.readFileBytesWithinLimit,
label,
"readFileBytesWithinLimit",
false,
allowExplicitUndefined,
);
const whole = captureWholeFileReader(value, readerProperties, label, true, false);
const virtual = createObject(null) as NonNullable<CapturedStaticReaders["virtual"]>;
Expand Down
16 changes: 16 additions & 0 deletions src/platform/adapters/fs/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import {
import { denoAdapter } from "../deno.ts";
import { VeryfrontError } from "#veryfront/errors/types.ts";
import { isProxyWithoutHooks } from "#veryfront/platform/compat/error-introspection.ts";
import { createSecureFs } from "#veryfront/security/secure-fs.ts";
import type { RuntimeAdapter } from "../base.ts";

describe("integration.ts", () => {
it("should export enhanceAdapterWithFS function", () => {
Expand Down Expand Up @@ -357,6 +359,20 @@ describe("integration.ts", () => {
assertEquals(descriptor.value === denoAdapter.fs, false);
});

it("produces an adapter SecureFs accepts end to end", async () => {
// The composed path is what broke in production, twice: SecureFs first
// rejected the Proxy adapter, then rejected the wrapped filesystem
// because an unimplemented optional capability was published as an own
// `undefined`. Each gate had a unit test, but nothing asserted the two
// together, so the second only surfaced after the first was deployed.
const enhanced = await enhanceWithRemoteFs();
const secureFs = createSecureFs({
baseDir: "/project",
adapter: enhanced as unknown as RuntimeAdapter,
});
assertExists(secureFs);
});

it("keeps the rest of the adapter, with methods bound to the original", async () => {
const enhanced = await enhanceWithRemoteFs();
assertEquals(enhanced.id, denoAdapter.id);
Expand Down
44 changes: 44 additions & 0 deletions src/security/secure-fs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
} from "#veryfront/testing/assert.ts";
import { describe, it } from "#veryfront/testing/bdd.ts";
import { createSecureFs, SecureFs, wrapAdapterWithSecurity } from "./secure-fs.ts";
import { wrapFSAdapter } from "#veryfront/platform/adapters/fs/wrapper.ts";
import { VeryfrontError } from "#veryfront/errors/types.ts";
import { DenoAdapter } from "#veryfront/platform/adapters/runtime/deno/adapter.ts";
import type { RuntimeAdapter, ServeOptions, Server } from "#veryfront/platform/adapters/base.ts";
Expand Down Expand Up @@ -1390,3 +1391,46 @@ describe("SecureFs", () => {
);
});
});

describe("SecureFs with the platform filesystem wrapper", () => {
it("accepts a wrapper whose unsupported capabilities are published as undefined", () => {
// FSAdapterWrapper publishes every optional capability as a frozen own
// property, including ones the underlying adapter lacks, so project code
// cannot inject one after construction. SecureFs previously read that
// shape as a malformed capability and rejected it with "SecureFs
// filesystem snapshot capability is invalid", which failed every hosted
// render on a remote filesystem.
const bareAdapter = {
readFile: (_path: string) => Promise.resolve("x"),
readFileBytes: (_path: string) => Promise.resolve(new Uint8Array()),
writeFile: (_path: string, _content: string) => Promise.resolve(),
exists: (_path: string) => Promise.resolve(true),
stat: (_path: string) =>
Promise.resolve({
size: 0,
isFile: true,
isDirectory: false,
isSymlink: false,
mtime: null,
}),
readDir: async function* (_path: string) {},
mkdir: (_path: string) => Promise.resolve(),
remove: (_path: string) => Promise.resolve(),
} as unknown as Parameters<typeof wrapFSAdapter>[0];

const wrapped = wrapFSAdapter(bareAdapter);
// Precondition: the capability really is published as an explicit undefined.
const descriptor = Object.getOwnPropertyDescriptor(
wrapped,
"readFileSnapshotWithinLimit",
);
assertEquals(descriptor !== undefined, true);
assertEquals(descriptor?.value, undefined);

const secureFs = createSecureFs({
baseDir: "/project",
adapter: { fs: wrapped } as unknown as RuntimeAdapter,
});
assertEquals(secureFs instanceof SecureFs, true);
});
});
15 changes: 15 additions & 0 deletions src/security/secure-fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,16 @@ export class SecureFs {
snapshotReader = captureSnapshotReadCapability(
suppliedFileSystem,
"SecureFs filesystem",
// Treat an explicitly undefined capability as unsupported rather than
// malformed. FSAdapterWrapper deliberately publishes every optional
// capability as a frozen own property, including the ones the
// underlying adapter does not provide, so that project code cannot
// inject one after construction. Rejecting that shape made SecureFs
// refuse the platform's own wrapper, and every hosted project on a
// remote filesystem failed its render. The wrapper itself captures with
// this same allowance. A present-but-non-function value is still
// rejected below.
true,
);
Comment thread
kojiwakayama marked this conversation as resolved.
} catch (_) {
invalidSecureFsOption("SecureFs filesystem snapshot capability is invalid");
Expand All @@ -717,6 +727,11 @@ export class SecureFs {
virtualSnapshotReader = captureStaticReadCapabilities(
suppliedFileSystem,
"SecureFs filesystem",
// Same wrapper shape as the snapshot capture above. Without this the
// capture threw on FSAdapterWrapper's frozen `undefined` slots, and
// the catch below turned that into a silent loss of virtual snapshot
// authority rather than an error.
true,
).virtual;
} catch {
// A malformed optional virtual publisher must not weaken otherwise
Expand Down