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
40 changes: 40 additions & 0 deletions src/platform/adapters/fs/cache/file-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
initializeFileCacheBackend,
isFileCacheDistributedEnabled,
} from "./file-cache.ts";
import { CacheBackends } from "#veryfront/cache/backend.ts";

describe("FileCache", () => {
let cache: FileCache;
Expand Down Expand Up @@ -329,6 +330,45 @@ describe("Distributed cache functions", () => {
assertEquals(typeof initializeFileCacheBackend, "function");
});

it("skips non-serializable synchronous writes to a distributed backend", async () => {
// A query-qualified import gives this regression its own module-scoped
// backend state, so the fake distributed backend cannot leak into other
// file-cache tests in the same Deno process.
const distributedModule = await import(
"./file-cache.ts?distributed-serialization-regression"
);
const descriptor = Object.getOwnPropertyDescriptor(CacheBackends, "file");
assertExists(descriptor);
let backendWrites = 0;
Object.defineProperty(CacheBackends, "file", {
...descriptor,
value: () =>
Promise.resolve({
type: "redis",
size: 0,
get: () => Promise.resolve(null),
set: () => {
backendWrites += 1;
return Promise.resolve();
},
del: () => Promise.resolve(false),
clear: () => Promise.resolve(),
} as never),
});

try {
assertEquals(await distributedModule.initializeFileCacheBackend(), true);
} finally {
Object.defineProperty(CacheBackends, "file", descriptor);
}

const distributedCache = new distributedModule.FileCache();
const circular: Record<string, unknown> = {};
circular.self = circular;
distributedCache.set("cyclic", circular);
assertEquals(backendWrites, 0);
});

it("should return boolean", async () => {
assertEquals(typeof (await initializeFileCacheBackend()), "boolean");
});
Expand Down
11 changes: 10 additions & 1 deletion src/platform/adapters/fs/cache/file-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,16 @@ export class FileCache {
// Note: key already includes the full prefix from buildFileCacheKeyPrefix (e.g., "file:env:project:...")
const backend = this.getBackend();
if (backend) {
const serialized = JSON.stringify(entry);
let serialized: string;
try {
serialized = JSON.stringify(entry);
} catch (error) {
logger.debug("Backend set skipped because the cache entry is not serializable", {
key,
error,
});
return;
}
// Update request-scoped cache so subsequent reads in same request see the new value
setInRequestCache(key, serialized);
backend.set(key, serialized, this.backendTtlSeconds).catch((error) => {
Expand Down
154 changes: 153 additions & 1 deletion src/platform/adapters/fs/github/github-api-client.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import "#veryfront/schemas/_test-setup.ts";
import { assertEquals, assertExists } from "#veryfront/testing/assert.ts";
import {
assertEquals,
assertExists,
assertRejects,
assertThrows,
} from "#veryfront/testing/assert.ts";
import { describe, it } from "#veryfront/testing/bdd.ts";
import { withMockFetch } from "#veryfront/testing/mock-fetch.ts";
import { GitHubApiClient } from "./github-api-client.ts";

const mockConfig = {
Expand Down Expand Up @@ -37,6 +43,29 @@ describe("GitHubApiClient", () => {
it("should be instantiable with config", () => {
assertExists(createClient());
});

it("rejects repository identity that could escape its URL segments", () => {
for (
const [field, value] of [
["owner", ".."],
["owner", "%2e%2e"],
["owner", "%25252e%25252e"],
["owner", "team/other"],
["owner", "team%2Fother"],
["repo", "."],
["repo", "..\\other"],
["repo", "repo%5Cother"],
["repo", "repo\u0000name"],
["repo", "r".repeat(257)],
] as const
) {
assertThrows(
() => new GitHubApiClient({ ...mockConfig, [field]: value }),
TypeError,
"GitHub",
);
}
});
});

describe("repoId", () => {
Expand Down Expand Up @@ -70,4 +99,127 @@ describe("GitHubApiClient", () => {
assertEquals(createClient().getRateLimitInfo(), null);
});
});

describe("getContents", () => {
it("encodes path segments and refs before URL construction", async () => {
const requestedUrls: string[] = [];
await withMockFetch(
(input) => {
requestedUrls.push(String(input));
return Promise.resolve(Response.json({
type: "file",
name: "file.ts",
path: "file.ts",
sha: "sha-1",
size: 0,
content: "",
encoding: "base64",
}));
},
async () => {
const client = createClient();
for (
const path of [
"%2e%2e/%2E%2E/user/repos",
"..\\..\\user/repos",
"docs/read me#draft?.md",
]
) {
await client.getContents(path, "feature/secure-paths");
}
},
);

assertEquals(requestedUrls.length, 3);
for (const requestedUrl of requestedUrls) {
const url = new URL(requestedUrl);
assertEquals(
url.pathname.startsWith("/repos/test-owner/test-repo/contents/"),
true,
);
assertEquals(url.searchParams.get("ref"), "feature/secure-paths");
}
assertEquals(
new URL(requestedUrls[0]!).pathname,
"/repos/test-owner/test-repo/contents/%252e%252e/%252E%252E/user/repos",
);
assertEquals(
new URL(requestedUrls[1]!).pathname,
"/repos/test-owner/test-repo/contents/..%5C..%5Cuser/repos",
);
assertEquals(
new URL(requestedUrls[2]!).pathname,
"/repos/test-owner/test-repo/contents/docs/read%20me%23draft%3F.md",
);
});

it("rejects literal traversal segments before fetching", async () => {
let fetchCalls = 0;
await withMockFetch(
() => {
fetchCalls++;
return Promise.resolve(Response.json({}));
},
async () => {
await assertRejects(
() => createClient().getContents("../secrets"),
TypeError,
"traversal",
);
},
);
assertEquals(fetchCalls, 0);
});
});

describe("endpoint construction", () => {
it("rejects dot-only endpoint values before fetching", async () => {
let fetchCalls = 0;
await withMockFetch(
() => {
fetchCalls++;
return Promise.resolve(Response.json({}));
},
async () => {
await assertRejects(() => createClient().getTree(".."), TypeError);
await assertRejects(() => createClient().getBlob("."), TypeError);
},
);
assertEquals(fetchCalls, 0);
});

it("encodes repository identity, refs, and blob identifiers as path segments", async () => {
const requestedUrls: string[] = [];
const client = new GitHubApiClient({
...mockConfig,
owner: "test owner",
repo: "repo#name",
});

await withMockFetch(
(input) => {
const url = String(input);
requestedUrls.push(url);
return Promise.resolve(
url.includes("/git/trees/")
? Response.json({ sha: "tree", tree: [], truncated: false })
: Response.json({ sha: "blob", size: 0, content: "", encoding: "base64" }),
);
},
async () => {
await client.getTree("feature/secure?recursive=0");
await client.getBlob("sha/../other");
},
);

assertEquals(
new URL(requestedUrls[0]!).pathname,
"/repos/test%20owner/repo%23name/git/trees/feature%2Fsecure%3Frecursive%3D0",
);
assertEquals(
new URL(requestedUrls[1]!).pathname,
"/repos/test%20owner/repo%23name/git/blobs/sha%2F..%2Fother",
);
});
});
});
98 changes: 89 additions & 9 deletions src/platform/adapters/fs/github/github-api-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,79 @@ const LOG_PREFIX = "[GitHubApiClient]";

const RATE_LIMIT_WARNING_THRESHOLD = 100;
const RETRY_JITTER_MAX_MS = 1_000;
const MAX_REPOSITORY_SEGMENT_LENGTH = 256;
const MAX_ENDPOINT_VALUE_LENGTH = 4_096;

function encodeRepositorySegment(value: string, label: string): string {
if (
typeof value !== "string" ||
value.length === 0 ||
value.length > MAX_REPOSITORY_SEGMENT_LENGTH ||
value.trim() !== value ||
value.normalize("NFC") !== value ||
/\p{Cc}/u.test(value)
) {
throw new TypeError(`GitHub ${label} must be a bounded canonical path segment`);
}

let decoded = value;
for (let depth = 0; depth <= value.length; depth++) {
if (
decoded === "." ||
decoded === ".." ||
decoded.includes("/") ||
decoded.includes("\\") ||
decoded.trim() !== decoded ||
/\p{Cc}/u.test(decoded)
) {
throw new TypeError(`GitHub ${label} must be a single non-traversal path segment`);
}

let next: string;
try {
next = decodeURIComponent(decoded);
} catch {
throw new TypeError(`GitHub ${label} contains malformed percent-encoding`);
}
if (next === decoded) return encodeURIComponent(value);
decoded = next;
}

throw new TypeError(`GitHub ${label} contains excessive percent-encoding`);
}

function encodeEndpointValue(value: string, label: string): string {
if (
typeof value !== "string" ||
value.length === 0 ||
value.length > MAX_ENDPOINT_VALUE_LENGTH ||
value === "." ||
value === ".." ||
/\p{Cc}/u.test(value)
) {
throw new TypeError(`GitHub ${label} must be bounded non-empty text`);
}
return encodeURIComponent(value);
}

function encodeContentsPath(path: string): { normalized: string; encoded: string } {
if (
typeof path !== "string" ||
path.length > MAX_ENDPOINT_VALUE_LENGTH ||
/\p{Cc}/u.test(path)
) {
throw new TypeError("GitHub contents path must be bounded text without control characters");
}
const normalized = path.replace(/^\/+/, "");
const segments = normalized.split("/");
if (segments.some((segment) => segment === "." || segment === "..")) {
throw new TypeError("GitHub contents path must not contain traversal segments");
}
return {
normalized,
encoded: segments.map(encodeURIComponent).join("/"),
};
}

class GitHubBlobIntegrityError extends Error {}

Expand All @@ -28,18 +101,24 @@ type APIError = Error & { statusCode?: number; endpoint?: string; repo?: string

export class GitHubApiClient {
private readonly baseUrl = "https://api.github.com";
private readonly repositoryEndpoint: string;
private rateLimitInfo: RateLimitInfo | null = null;

constructor(private readonly config: ResolvedGitHubConfig) {}
constructor(private readonly config: ResolvedGitHubConfig) {
const owner = encodeRepositorySegment(config.owner, "owner");
const repo = encodeRepositorySegment(config.repo, "repository");
this.repositoryEndpoint = `/repos/${owner}/${repo}`;
}

get repoId(): string {
return `${this.config.owner}/${this.config.repo}`;
}

async getTree(ref?: string): Promise<GitHubTreeResponse> {
const treeRef = ref ?? this.config.ref;
const endpoint =
`/repos/${this.config.owner}/${this.config.repo}/git/trees/${treeRef}?recursive=1`;
const endpoint = `${this.repositoryEndpoint}/git/trees/${
encodeEndpointValue(treeRef, "tree ref")
}?recursive=1`;

logger.debug(`${LOG_PREFIX} Fetching tree`, { ref: treeRef });

Expand All @@ -60,18 +139,19 @@ export class GitHubApiClient {
ref?: string,
): Promise<GitHubContentItem | GitHubContentItem[]> {
const contentRef = ref ?? this.config.ref;
const normalizedPath = path.replace(/^\/+/, "");
const endpoint =
`/repos/${this.config.owner}/${this.config.repo}/contents/${normalizedPath}?ref=${contentRef}`;
const { normalized, encoded } = encodeContentsPath(path);
const endpoint = `${this.repositoryEndpoint}/contents/${encoded}?ref=${
encodeEndpointValue(contentRef, "contents ref")
}`;

logger.debug(`${LOG_PREFIX} Fetching contents`, { path: normalizedPath });
logger.debug(`${LOG_PREFIX} Fetching contents`, { path: normalized });

const raw = await this.request(endpoint);
return getGitHubContentsResponseSchema().parse(raw);
}

async getBlob(sha: string): Promise<GitHubBlobResponse> {
const endpoint = `/repos/${this.config.owner}/${this.config.repo}/git/blobs/${sha}`;
const endpoint = `${this.repositoryEndpoint}/git/blobs/${encodeEndpointValue(sha, "blob SHA")}`;

logger.debug(`${LOG_PREFIX} Fetching blob`, { sha });

Expand All @@ -95,7 +175,7 @@ export class GitHubApiClient {
if (expectedSize > byteLimit) {
throw new RangeError(`GitHub blob exceeds ${byteLimit} bytes`);
}
const endpoint = `/repos/${this.config.owner}/${this.config.repo}/git/blobs/${sha}`;
const endpoint = `${this.repositoryEndpoint}/git/blobs/${encodeEndpointValue(sha, "blob SHA")}`;

logger.debug(`${LOG_PREFIX} Fetching bounded raw blob`, { sha, expectedSize });

Expand Down
Loading