Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 6 additions & 0 deletions src/platform/adapters/fs/cache/file-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,12 @@ describe("Distributed cache functions", () => {
circular.self = circular;
distributedCache.set("cyclic", circular);
assertEquals(backendWrites, 0);

// Positive control: a serializable entry must reach the fake backend,
// proving the harness is live and the zero-write assertion above is not
// vacuously passing because the backend was never wired up.
distributedCache.set("serializable", { ok: true });
assertEquals(backendWrites, 1);
});

it("should return boolean", async () => {
Expand Down
8 changes: 6 additions & 2 deletions src/platform/adapters/fs/github/github-api-client.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 { withMockFetch } from "#veryfront/testing/mock-fetch.ts";
import { VeryfrontError } from "#veryfront/errors/types.ts";
import { GitHubApiClient } from "./github-api-client.ts";

const mockConfig = {
Expand Down Expand Up @@ -59,11 +60,14 @@ describe("GitHubApiClient", () => {
["repo", "r".repeat(257)],
] as const
) {
assertThrows(
// A CONFIG-category error keeps enhanceAdapterWithFS failing closed;
// a bare TypeError would silently fall back to the local filesystem.
const error = assertThrows(
() => new GitHubApiClient({ ...mockConfig, [field]: value }),
TypeError,
VeryfrontError,
"GitHub",
);
assertEquals(error.slug, "config-validation-failed");
Comment thread
kojiwakayama marked this conversation as resolved.
}
});
});
Expand Down
18 changes: 15 additions & 3 deletions src/platform/adapters/fs/github/github-api-client.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createError, retryWithBackoff, toError } from "#veryfront/errors";
import { CONFIG_VALIDATION_FAILED } from "#veryfront/errors/error-registry/config.ts";
import { logger } from "#veryfront/utils";
import type { ResolvedGitHubConfig } from "./types.ts";
import {
Expand Down Expand Up @@ -105,9 +106,20 @@ export class GitHubApiClient {
private rateLimitInfo: RateLimitInfo | null = null;

constructor(private readonly config: ResolvedGitHubConfig) {
const owner = encodeRepositorySegment(config.owner, "owner");
const repo = encodeRepositorySegment(config.repo, "repository");
this.repositoryEndpoint = `/repos/${owner}/${repo}`;
// Invalid repository identity is a configuration error: it must fail
// closed (surface as a CONFIG-category VeryfrontError) instead of a bare
// TypeError that enhanceAdapterWithFS would swallow before silently
// falling back to the host-local filesystem adapter.
try {
const owner = encodeRepositorySegment(config.owner, "owner");
const repo = encodeRepositorySegment(config.repo, "repository");
this.repositoryEndpoint = `/repos/${owner}/${repo}`;
} catch (cause) {
throw CONFIG_VALIDATION_FAILED.create({
detail: cause instanceof Error ? cause.message : "GitHub repository identity is invalid",
cause,
});
}
}

get repoId(): string {
Expand Down
26 changes: 11 additions & 15 deletions src/platform/adapters/fs/github/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createError, toError } from "#veryfront/errors";
import { CONFIG_INVALID } from "#veryfront/errors";

export type { DirectoryEntry } from "../shared-types.ts";

Expand Down Expand Up @@ -69,24 +69,20 @@ const DEFAULT_INITIAL_RETRY_DELAY_MS = 1_000;
const DEFAULT_MAX_RETRY_DELAY_MS = 10_000;

export function createGitHubConfig(config: GitHubConfig): ResolvedGitHubConfig {
// CONFIG-category registry errors fail closed in enhanceAdapterWithFS;
// legacy plain Errors would trigger its silent local-filesystem fallback.
if (!config.token) {
throw toError(
createError({
type: "config",
message:
"GitHub adapter requires a token. Set GITHUB_TOKEN environment variable or provide token in config.",
}),
);
throw CONFIG_INVALID.create({
detail:
"GitHub adapter requires a token. Set GITHUB_TOKEN environment variable or provide token in config.",
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

if (!config.owner || !config.repo) {
throw toError(
createError({
type: "config",
message:
"GitHub adapter requires owner and repo. Provide them in config or via GITHUB_OWNER and GITHUB_REPO environment variables.",
}),
);
throw CONFIG_INVALID.create({
detail:
"GitHub adapter requires owner and repo. Provide them in config or via GITHUB_OWNER and GITHUB_REPO environment variables.",
});
}

return {
Expand Down
34 changes: 34 additions & 0 deletions src/platform/adapters/fs/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,40 @@ describe("integration.ts", () => {
assertEquals(error.slug, "config-validation-failed");
});

it("should fail closed when GitHub repository identity is invalid", async () => {
const error = await assertRejects(
() =>
enhanceAdapterWithFS(denoAdapter, {
fs: {
type: "github",
github: { token: "test-token", owner: "team/other", repo: "repo" },
},
}),
VeryfrontError,
"GitHub owner",
);
assertInstanceOf(error, VeryfrontError);
assertEquals(error.slug, "config-validation-failed");
});

it("should fail closed when the GitHub adapter has no token", async () => {
// token: "" is explicit so the GITHUB_TOKEN environment variable cannot
// satisfy the requirement and mask the regression in CI.
const error = await assertRejects(
() =>
enhanceAdapterWithFS(denoAdapter, {
fs: {
type: "github",
github: { token: "", owner: "owner", repo: "repo" },
},
}),
VeryfrontError,
"token",
);
assertInstanceOf(error, VeryfrontError);
assertEquals(error.slug, "config-invalid");
});

it("should fall back to original adapter for unsupported type", async () => {
const adapter = await enhanceAdapterWithFS(denoAdapter, {
fs: { type: "unsupported-type" as any },
Expand Down
6 changes: 5 additions & 1 deletion src/platform/adapters/fs/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@ export function enhanceAdapterWithFS(

return enhancedAdapter;
} catch (error) {
if (error instanceof VeryfrontError && error.slug === "config-validation-failed") {
// Any configuration-class failure must fail closed: silently falling
// back to the host-local filesystem on a misconfigured remote adapter
// would serve the wrong filesystem. Only non-config runtime failures
// (network, transient init) may degrade to the local adapter.
if (error instanceof VeryfrontError && error.category === "CONFIG") {
throw error;
}
logger.error("Failed to initialize FSAdapter", {
Expand Down
12 changes: 12 additions & 0 deletions src/platform/adapters/fs/veryfront/path-normalizer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,18 @@ describe("PathNormalizer", () => {
);
});

it("should reject C1 control characters like the GitHub normalizer", () => {
const normalizer = new PathNormalizer();
// U+0080 and U+009F bound the C1 range; NUL is the C0 control anchor.
for (const codeUnit of [0x00, 0x80, 0x9f]) {
assertThrows(
() => normalizer.normalize(`src/${String.fromCharCode(codeUnit)}secrets.ts`),
TypeError,
"must not contain control characters",
);
}
});

it("should reject unbounded paths", () => {
const normalizer = new PathNormalizer();
assertThrows(
Expand Down
8 changes: 5 additions & 3 deletions src/platform/adapters/fs/veryfront/path-normalizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import { CONFIG_VALIDATION_FAILED } from "#veryfront/errors/error-registry/confi
const logger = baseLogger.component("path-normalizer");
const MAX_PATH_CODE_UNITS = 4_096;

function hasAsciiControlCharacter(value: string): boolean {
// Rejects the full non-printable control range (C0, DEL, and C1) — the same
// policy as the GitHub path normalizer in ../github/path-utils.ts.
function hasControlCharacter(value: string): boolean {
for (let index = 0; index < value.length; index++) {
const codeUnit = value.charCodeAt(index);
if (codeUnit <= 0x1f || codeUnit === 0x7f) return true;
if (codeUnit <= 0x1f || (codeUnit >= 0x7f && codeUnit <= 0x9f)) return true;
}
return false;
}
Expand Down Expand Up @@ -97,7 +99,7 @@ export class PathNormalizer {
`Filesystem ${label} exceeds the ${MAX_PATH_CODE_UNITS}-character limit`,
);
}
if (hasAsciiControlCharacter(path)) {
if (hasControlCharacter(path)) {
throw new TypeError(`Filesystem ${label} must not contain control characters`);
}
if (path.includes("\\")) {
Expand Down
Loading