History

Filter GitHub source entries safely

Rhys Sullivan committed 4 months ago

Commit 9e77e341 parent bb155804 files · +159 −29Browse files

Unified · Split

4 files
  1. packages/plugins/apps/src/plugin/bindings.ts +2 −0
  2. packages/plugins/apps/src/plugin/tenant-isolation.test.ts +1 −0
  3. packages/plugins/apps/src/source/github-source.test.ts +101 −12
  4. packages/plugins/apps/src/source/github-source.ts +55 −17
packages/plugins/apps/src/plugin/bindings.ts+2−0
@@ -259,6 +259,8 @@ export const buildBridge = (context: BindingContext): HandleBridge => ({ · 258 unmodified lines
259 return Effect.fail(invokeErr(`no binding for role ${role}`));259 return Effect.fail(invokeErr(`no binding for role ${role}`));
260 }260 }
261 if (path.length === 1 && path[0] === "account") {261 if (path.length === 1 && path[0] === "account") {
262 // Connection-derived placeholders only; this is not verified upstream
263 // account identity.
262 return Effect.succeed({ email: connectionName, login: connectionName, name: connectionName });264 return Effect.succeed({ email: connectionName, login: connectionName, name: connectionName });
263 }265 }
264 266
packages/plugins/apps/src/plugin/tenant-isolation.test.ts+1−0
@@ -51,6 +51,7 @@ const makeGitHubFetch = (input: { readonly toolName: string; readonly upstreamSh · 50 unmodified lines
51 {51 {
52 path: `tools/${input.toolName}.ts`,52 path: `tools/${input.toolName}.ts`,
53 type: "blob",53 type: "blob",
54 mode: "100644",
54 sha: `${input.upstreamSha}-blob`,55 sha: `${input.upstreamSha}-blob`,
55 size: Buffer.byteLength(content, "utf8"),56 size: Buffer.byteLength(content, "utf8"),
56 },57 },
packages/plugins/apps/src/source/github-source.test.ts+101−12
@@ -8,6 +8,7 @@ import { Effect } from "effect"; · 7 unmodified lines
8import { makeSelfHostAppsRuntime } from "../plugin/self-host-runtime";8import { makeSelfHostAppsRuntime } from "../plugin/self-host-runtime";
9import { makeInMemoryAppsStore, makeTestResolver } from "../testing";9import { makeInMemoryAppsStore, makeTestResolver } from "../testing";
10import { PUBLISH_LIMITS } from "../pipeline/publish";10import { PUBLISH_LIMITS } from "../pipeline/publish";
11import { scopeAddress } from "../seams/scope-address";
11import { fetchGitHubSource, syncGitHubSource } from "./github-source";12import { fetchGitHubSource, syncGitHubSource } from "./github-source";
12 13
13const run = <A, E>(effect: Effect.Effect<A, E>): Promise<A> => Effect.runPromise(effect);14const run = <A, E>(effect: Effect.Effect<A, E>): Promise<A> => Effect.runPromise(effect);
@@ -34,8 +35,17 @@ const json = (value: unknown, status = 200): Response => · 20 unmodified lines
34 headers: { "content-type": "application/json" },35 headers: { "content-type": "application/json" },
35 });36 });
36 37
38interface GitHubTreeFixtureEntry {
39 readonly path: string;
40 readonly type: string;
41 readonly mode?: string;
42 readonly content?: string;
43 readonly size?: number;
44}
45
37const makeGitHubFetch = (input: {46const makeGitHubFetch = (input: {
38 readonly files: ReadonlyMap<string, string>;47 readonly files: ReadonlyMap<string, string>;
48 readonly entries?: readonly GitHubTreeFixtureEntry[];
39 readonly upstreamSha?: string;49 readonly upstreamSha?: string;
40 readonly treeSha?: string;50 readonly treeSha?: string;
41}) => {51}) => {
@@ -43,9 +53,22 @@ const makeGitHubFetch = (input: { · 1 unmodified line
43 const upstreamSha = input.upstreamSha ?? "commit-1";53 const upstreamSha = input.upstreamSha ?? "commit-1";
44 const treeSha = input.treeSha ?? "tree-1";54 const treeSha = input.treeSha ?? "tree-1";
45 const blobBySha = new Map<string, { path: string; content: string }>();55 const blobBySha = new Map<string, { path: string; content: string }>();
56 const treeEntries: readonly GitHubTreeFixtureEntry[] =
57 input.entries ??
58 [...input.files].map(([path, content]) => ({
59 path,
60 type: "blob",
61 mode: "100644",
62 content,
63 }));
46 let index = 0;64 let index = 0;
47 for (const [path, content] of input.files) {65 const shaByPath = new Map<string, string>();
48 blobBySha.set(`blob-${++index}`, { path, content });66 for (const entry of treeEntries) {
67 if (entry.content !== undefined) {
68 const sha = `blob-${++index}`;
69 shaByPath.set(entry.path, sha);
70 blobBySha.set(sha, { path: entry.path, content: entry.content });
71 }
49 }72 }
50 let blobCalls = 0;73 let blobCalls = 0;
51 const fetch = (async (rawUrl: string) => {74 const fetch = (async (rawUrl: string) => {
@@ -59,12 +82,16 @@ const makeGitHubFetch = (input: { · 7 unmodified lines
59 }82 }
60 if (url.pathname === `${repoPath}/git/trees/${treeSha}`) {83 if (url.pathname === `${repoPath}/git/trees/${treeSha}`) {
61 return json({84 return json({
62 tree: [...blobBySha.entries()].map(([sha, blob]) => ({85 tree: treeEntries.map((entry, entryIndex) => {
63 path: blob.path,86 const sha = shaByPath.get(entry.path) ?? `tree-${entryIndex}`;
64 type: "blob",87 return {
65 sha,88 path: entry.path,
66 size: Buffer.byteLength(blob.content, "utf8"),89 type: entry.type,
67 })),90 mode: entry.mode,
91 sha,
92 size: entry.size ?? Buffer.byteLength(entry.content ?? "", "utf8"),
93 };
94 }),
68 });95 });
69 }96 }
70 const blobPrefix = `${repoPath}/git/blobs/`;97 const blobPrefix = `${repoPath}/git/blobs/`;
@@ -103,10 +130,10 @@ describe("GitHub custom-tools source", () => { · 32 unmodified lines
103 ]);130 ]);
104 const github = makeGitHubFetch({ files, upstreamSha: "commit-a" });131 const github = makeGitHubFetch({ files, upstreamSha: "commit-a" });
105 const snapshot = await run(fetchGitHubSource({ repo: "acme/tools", fetch: github.fetch }));132 const snapshot = await run(fetchGitHubSource({ repo: "acme/tools", fetch: github.fetch }));
106 expect([...snapshot.files.keys()].sort()).toEqual([133 expect([...snapshot.files.keys()].sort()).toEqual(["executor.json", "tools/hello.ts"]);
107 "executor.json",134 expect(snapshot.skipped).toEqual([
108 "tools/hello.ts",135 { path: "workflows/deferred.ts", reason: "not supported yet" },
109 "workflows/deferred.ts",136 { path: "docs/readme.md", reason: "ignored" },
110 ]);137 ]);
111 expect(snapshot.description).toBe("Acme tools");138 expect(snapshot.description).toBe("Acme tools");
112 139
@@ -123,6 +150,7 @@ describe("GitHub custom-tools source", () => { · 10 unmodified lines
123 expect(result.tools).toEqual(["hello"]);150 expect(result.tools).toEqual(["hello"]);
124 expect(result.skipped).toEqual([151 expect(result.skipped).toEqual([
125 { path: "workflows/deferred.ts", reason: "not supported yet" },152 { path: "workflows/deferred.ts", reason: "not supported yet" },
153 { path: "docs/readme.md", reason: "ignored" },
126 ]);154 ]);
127 const descriptor = await run(host.runtime.getDescriptor("githubTools"));155 const descriptor = await run(host.runtime.getDescriptor("githubTools"));
128 expect(descriptor?.description).toBe("Acme tools");156 expect(descriptor?.description).toBe("Acme tools");
@@ -208,4 +236,65 @@ export default defineTool({ description: "bad", input: { type: "object" }, async · 79 unmodified lines
208 expect(result.errors?.[0]?.message).toContain('bare import "lodash" is not allowed');236 expect(result.errors?.[0]?.message).toContain('bare import "lodash" is not allowed');
209 await host.close();237 await host.close();
210 });238 });
239
240 it("skips unsupported tree entries and invalid paths without committing them", async () => {
241 const github = makeGitHubFetch({
242 files: new Map(),
243 entries: [
244 {
245 path: "tools/ok.ts",
246 type: "blob",
247 mode: "100644",
248 content: toolSource("ok"),
249 },
250 {
251 path: "tools/link.ts",
252 type: "blob",
253 mode: "120000",
254 content: "../outside.ts",
255 },
256 {
257 path: "tools/submodule.ts",
258 type: "commit",
259 mode: "160000",
260 },
261 {
262 path: "tools/Bad.Name.ts",
263 type: "blob",
264 mode: "100644",
265 content: toolSource("bad"),
266 },
267 ],
268 upstreamSha: "mixed-sha",
269 });
270 const host = makeRuntime();
271 const result = await run(
272 syncGitHubSource({
273 runtime: host.runtime,
274 scope: "githubTools",
275 repo: "acme/tools",
276 fetch: github.fetch,
277 }),
278 );
279
280 expect(result.status).toBe("published");
281 expect(result.tools).toEqual(["ok"]);
282 expect(result.skipped).toEqual([
283 { path: "tools/link.ts", reason: "unsupported file type" },
284 { path: "tools/submodule.ts", reason: "unsupported file type" },
285 { path: "tools/Bad.Name.ts", reason: "ignored" },
286 ]);
287
288 const descriptor = await run(host.runtime.getDescriptor("githubTools"));
289 expect(descriptor?.tools.map((tool) => tool.name)).toEqual(["ok"]);
290 const scopeStore = await run(
291 host.runtime.deps.artifactStore.forScope(scopeAddress("org", "githubTools")),
292 );
293 const snapshotPaths = await run(scopeStore.list(descriptor!.snapshotId as never));
294 expect(snapshotPaths).toContain("tools/ok.ts");
295 expect(snapshotPaths).not.toContain("tools/link.ts");
296 expect(snapshotPaths).not.toContain("tools/submodule.ts");
297 expect(snapshotPaths).not.toContain("tools/Bad.Name.ts");
298 await host.close();
299 });
211});300});
packages/plugins/apps/src/source/github-source.ts+55−17
@@ -1,6 +1,6 @@
1import { Data, Effect, Result } from "effect";1import { Data, Effect, Result } from "effect";
2 2
3import { PublishError, type FileDiagnostic, type SkippedArtifact } from "../pipeline/discover";3import { PublishError, type FileDiagnostic } from "../pipeline/discover";
4import { PUBLISH_LIMITS, enforcePublishLimits } from "../pipeline/publish";4import { PUBLISH_LIMITS, enforcePublishLimits } from "../pipeline/publish";
5import type { AppSourceRef } from "../pipeline/descriptor";5import type { AppSourceRef } from "../pipeline/descriptor";
6import type { AppsRuntime } from "../plugin/runtime";6import type { AppsRuntime } from "../plugin/runtime";
@@ -20,6 +20,7 @@ export interface GitHubSourceSnapshot { · 13 unmodified lines
20 readonly ref: string;20 readonly ref: string;
21 readonly upstreamSha: string;21 readonly upstreamSha: string;
22 readonly description?: string;22 readonly description?: string;
23 readonly skipped: readonly GitHubSkippedArtifact[];
23}24}
24 25
25export class GitHubSourceError extends Data.TaggedError("GitHubSourceError")<{26export class GitHubSourceError extends Data.TaggedError("GitHubSourceError")<{
@@ -35,27 +36,32 @@ export interface SyncErrorData { · 9 unmodified lines
35 readonly diagnostics?: readonly FileDiagnostic[];36 readonly diagnostics?: readonly FileDiagnostic[];
36}37}
37 38
39export interface GitHubSkippedArtifact {
40 readonly path: string;
41 readonly reason: "not supported yet" | "unsupported file type" | "ignored";
42}
43
38export type GitHubSyncResult =44export type GitHubSyncResult =
39 | {45 | {
40 readonly status: "published";46 readonly status: "published";
41 readonly snapshotId: SnapshotId;47 readonly snapshotId: SnapshotId;
42 readonly upstreamSha: string;48 readonly upstreamSha: string;
43 readonly tools: readonly string[];49 readonly tools: readonly string[];
44 readonly skipped: readonly SkippedArtifact[];50 readonly skipped: readonly GitHubSkippedArtifact[];
45 readonly errors?: undefined;51 readonly errors?: undefined;
46 }52 }
47 | {53 | {
48 readonly status: "up-to-date";54 readonly status: "up-to-date";
49 readonly upstreamSha: string;55 readonly upstreamSha: string;
50 readonly tools: readonly string[];56 readonly tools: readonly string[];
51 readonly skipped: readonly SkippedArtifact[];57 readonly skipped: readonly GitHubSkippedArtifact[];
52 readonly errors?: undefined;58 readonly errors?: undefined;
53 }59 }
54 | {60 | {
55 readonly status: "failed";61 readonly status: "failed";
56 readonly upstreamSha?: string;62 readonly upstreamSha?: string;
57 readonly tools: readonly string[];63 readonly tools: readonly string[];
58 readonly skipped: readonly SkippedArtifact[];64 readonly skipped: readonly GitHubSkippedArtifact[];
59 readonly errors: readonly SyncErrorData[];65 readonly errors: readonly SyncErrorData[];
60 };66 };
61 67
@@ -65,7 +71,9 @@ export interface SyncGitHubSourceInput extends GitHubSourceInput { · 3 unmodified lines
65 readonly scope: string;71 readonly scope: string;
66}72}
67 73
68const MATERIALIZED_RE = /^(tools|workflows|ui|skills)\//;74const TOOL_RE = /^tools\/([a-z0-9][a-z0-9-]*)\.(ts|tsx|js|jsx)$/;
75const DEFERRED_RE = /^(workflows|ui|skills)\//;
76const REGULAR_FILE_MODES = new Set(["100644", "100755"]);
69 77
70const trimBaseUrl = (baseUrl: string): string => baseUrl.replace(/\/+$/, "");78const trimBaseUrl = (baseUrl: string): string => baseUrl.replace(/\/+$/, "");
71 79
@@ -88,8 +96,7 @@ const encodedRepoPath = (repo: string): Effect.Effect<string, GitHubSourceError> · 16 unmodified lines
88 return Effect.succeed(`${encodeURIComponent(parsed.owner)}/${encodeURIComponent(parsed.name)}`);96 return Effect.succeed(`${encodeURIComponent(parsed.owner)}/${encodeURIComponent(parsed.name)}`);
89};97};
90 98
91const materializes = (path: string): boolean =>99const acceptedPath = (path: string): boolean => path === "executor.json" || TOOL_RE.test(path);
92 path === "executor.json" || MATERIALIZED_RE.test(path);
93 100
94const isRecord = (value: unknown): value is Record<string, unknown> =>101const isRecord = (value: unknown): value is Record<string, unknown> =>
95 value !== null && typeof value === "object" && !Array.isArray(value);102 value !== null && typeof value === "object" && !Array.isArray(value);
@@ -163,6 +170,7 @@ interface RefResponse { · 67 unmodified lines
163interface TreeEntry {170interface TreeEntry {
164 readonly path?: unknown;171 readonly path?: unknown;
165 readonly type?: unknown;172 readonly type?: unknown;
173 readonly mode?: unknown;
166 readonly sha?: unknown;174 readonly sha?: unknown;
167 readonly size?: unknown;175 readonly size?: unknown;
168}176}
@@ -219,6 +227,34 @@ const checkTreeLimits = (entries: readonly TreeEntry[]): PublishError | null => · 50 unmodified lines
219 return diagnostics.length === 0 ? null : limitError(diagnostics);227 return diagnostics.length === 0 ? null : limitError(diagnostics);
220};228};
221 229
230const classifyTreeEntry = (
231 entry: TreeEntry,
232):
233 | {
234 readonly kind: "fetch";
235 readonly entry: TreeEntry & { readonly path: string; readonly sha: string };
236 }
237 | { readonly kind: "skip"; readonly skipped: GitHubSkippedArtifact }
238 | null => {
239 const path = asString(entry.path);
240 if (!path) return null;
241
242 const type = asString(entry.type);
243 const mode = asString(entry.mode);
244 if (type === "tree") return null;
245 if (type !== "blob" || !mode || !REGULAR_FILE_MODES.has(mode)) {
246 return { kind: "skip", skipped: { path, reason: "unsupported file type" } };
247 }
248
249 const sha = asString(entry.sha);
250 if (!sha) return { kind: "skip", skipped: { path, reason: "unsupported file type" } };
251 if (acceptedPath(path)) return { kind: "fetch", entry: { ...entry, path, sha } };
252 if (DEFERRED_RE.test(path)) {
253 return { kind: "skip", skipped: { path, reason: "not supported yet" } };
254 }
255 return { kind: "skip", skipped: { path, reason: "ignored" } };
256};
257
222const decodeBlob = (path: string, blob: BlobResponse): Effect.Effect<string, GitHubSourceError> =>258const decodeBlob = (path: string, blob: BlobResponse): Effect.Effect<string, GitHubSourceError> =>
223 Effect.try({259 Effect.try({
224 try: () => {260 try: () => {
@@ -293,13 +329,14 @@ export const fetchGitHubSource = ( · 68 unmodified lines
293 path: `/repos/${repoPath}/git/trees/${treeSha}`,329 path: `/repos/${repoPath}/git/trees/${treeSha}`,
294 });330 });
295 }331 }
296 const entries = (tree.tree ?? []).filter(332 const entries: (TreeEntry & { readonly path: string; readonly sha: string })[] = [];
297 (entry) =>333 const skipped: GitHubSkippedArtifact[] = [];
298 entry.type === "blob" &&334 for (const entry of tree.tree ?? []) {
299 typeof entry.path === "string" &&335 const classified = classifyTreeEntry(entry);
300 typeof entry.sha === "string" &&336 if (!classified) continue;
301 materializes(entry.path),337 if (classified.kind === "fetch") entries.push(classified.entry);
302 );338 else skipped.push(classified.skipped);
339 }
303 const treeLimitError = checkTreeLimits(entries);340 const treeLimitError = checkTreeLimits(entries);
304 if (treeLimitError) return yield* Effect.fail(treeLimitError);341 if (treeLimitError) return yield* Effect.fail(treeLimitError);
305 342
@@ -321,6 +358,7 @@ export const fetchGitHubSource = ( · 15 unmodified lines
321 ref,358 ref,
322 upstreamSha,359 upstreamSha,
323 description: yield* executorDescription(files),360 description: yield* executorDescription(files),
361 skipped,
324 };362 };
325 });363 });
326 364
@@ -372,7 +410,7 @@ export const syncGitHubSource = (input: SyncGitHubSourceInput): Effect.Effect<Gi · 45 unmodified lines
372 status: "up-to-date",410 status: "up-to-date",
373 upstreamSha: snapshot.upstreamSha,411 upstreamSha: snapshot.upstreamSha,
374 tools: current.tools.map((tool) => tool.name),412 tools: current.tools.map((tool) => tool.name),
375 skipped: current.skipped ?? [],413 skipped: [...snapshot.skipped, ...(current.skipped ?? [])],
376 } satisfies GitHubSyncResult;414 } satisfies GitHubSyncResult;
377 }415 }
378 416
@@ -391,7 +429,7 @@ export const syncGitHubSource = (input: SyncGitHubSourceInput): Effect.Effect<Gi · 12 unmodified lines
391 status: "failed",429 status: "failed",
392 upstreamSha: snapshot.upstreamSha,430 upstreamSha: snapshot.upstreamSha,
393 tools: [],431 tools: [],
394 skipped: [],432 skipped: snapshot.skipped,
395 errors: [publishErrorToSyncError(published.failure)],433 errors: [publishErrorToSyncError(published.failure)],
396 } satisfies GitHubSyncResult;434 } satisfies GitHubSyncResult;
397 }435 }
@@ -401,6 +439,6 @@ export const syncGitHubSource = (input: SyncGitHubSourceInput): Effect.Effect<Gi · 3 unmodified lines
401 snapshotId: published.success.snapshotId,439 snapshotId: published.success.snapshotId,
402 upstreamSha: snapshot.upstreamSha,440 upstreamSha: snapshot.upstreamSha,
403 tools: published.success.descriptor.tools.map((tool) => tool.name),441 tools: published.success.descriptor.tools.map((tool) => tool.name),
404 skipped: published.success.skipped,442 skipped: [...snapshot.skipped, ...published.success.skipped],
405 } satisfies GitHubSyncResult;443 } satisfies GitHubSyncResult;
406 });444 });