Skip to content

Commit 63c2527

Browse files
tlmiiCopilotCopilotaaronpowell
authored
Accept nested extensions/<name>/extension.mjs in external-plugin canvas checks (#2403)
* Accept nested extensions/<name>/extension.mjs in external-plugin canvas checks The external-plugin canvas structure check (quality gate) and intake validation both hardcoded a flat extensions/extension.mjs entry point, falsely rejecting the documented nested extensions/<name>/extension.mjs layout that installs and runs fine. Scan the extensions/ directory for a nested subfolder containing extension.mjs while still accepting the flat form for backward compatibility. Applied to both runCanvasStructureGate (git-object lookups) and validateCanvasPluginMetadata (Contents API), keeping them behaviorally aligned. Added regression coverage for both paths. Fixes #2402 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Harden nested canvas extension detection after adversarial review Address multi-model review findings on the nested canvas extension fix: - Quality gate: enumerate extensions/ via 'git ls-tree -z' with spawnSync (NUL-delimited, untruncated) so large directories no longer drop the real entry past the 12KB output cap. - Intake: decouple the flat extensions/extension.mjs check from the directory listing, require an array listing (Array.isArray) before treating it as a directory, and surface an unverifiable (warning) result instead of a false rejection when the listing or a nested lookup hits a transient API error. - Add regression tests: nested entry beyond the legacy output cap (gate) and unverifiable/flat-still-accepted paths when the listing errors (intake). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Enumerate canvas extensions via a single recursive Git Trees call in intake Address PR review: the Contents API caps directory listings at 1,000 entries and required one request per extension subfolder, so a nested entry beyond the cap could be falsely rejected (the same truncation class the git gate avoids) and large repos risked latency / rate-limit exhaustion. Replace the per-subfolder Contents API enumeration with one recursive 'git/trees/<locator>?recursive=1' fetch and inspect 'extensions/extension.mjs' and immediate 'extensions/<name>/extension.mjs' paths locally. A truncated tree without a located entry point is reported as unverifiable (warning) rather than rejected, and refs are normalized so 'refs/tags/<tag>' resolves as a tree-ish. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Bound canvas extension discovery to plugin scope Address reviewer feedback on unbounded scaling for untrusted/large repos: - Quality gate: replace the per-candidate-directory git cat-file spawns in locateCanvasEntryPoint with a single recursive git ls-tree over the extensions subtree, classifying flat/nested entry points in memory. Process count is now constant regardless of how many folders live under extensions/. - Intake: stop fetching the recursive git tree from the repo root (which a large unrelated monorepo can push past the Trees API truncation limit and never validate). Walk to the plugin's extensions directory one level at a time to resolve its tree SHA, then fetch only that subtree recursively, so verifiability depends on the plugin's own size, not the whole repository. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
1 parent b34ac09 commit 63c2527

4 files changed

Lines changed: 560 additions & 34 deletions

eng/external-plugin-intake.mjs

Lines changed: 124 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,92 @@ async function validateRemoteRepository(repo, { ref, sha }, errors, warnings, to
380380
}
381381
}
382382

383-
async function validateCanvasPluginMetadata(plugin, errors, warnings, token) {
383+
function buildGitTreePath(repo, treeish, { recursive = false } = {}) {
384+
const encodedRepo = encodeRepoPath(repo);
385+
const query = recursive ? "?recursive=1" : "";
386+
return `/repos/${encodedRepo}/git/trees/${encodeURIComponent(treeish)}${query}`;
387+
}
388+
389+
function normalizeTreeish(locator) {
390+
const value = String(locator ?? "").trim();
391+
// The Git Trees API takes the tree-ish as a single path segment. A full "refs/tags/<tag>"
392+
// ref would break that, so reduce it to the bare tag name; commit SHAs and simple tag
393+
// names pass through unchanged.
394+
return value.startsWith("refs/tags/") ? value.slice("refs/tags/".length) : value;
395+
}
396+
397+
// Resolve the tree SHA of a directory by walking the path one level at a time. Each hop is a
398+
// non-recursive tree fetch of a single directory, so the work is bounded by the path depth and
399+
// is independent of the overall repository size — unlike a root recursive fetch, which a large
400+
// unrelated monorepo can push over the API's truncation limit and never validate.
401+
async function resolveDirectoryTreeSha(repo, treeish, segments, token) {
402+
let currentTreeish = treeish;
403+
for (const segment of segments) {
404+
const response = await fetchGitHubJson(buildGitTreePath(repo, currentTreeish), token);
405+
if (response.kind !== "found" || !Array.isArray(response.data?.tree)) {
406+
return { status: "apiError" };
407+
}
408+
if (response.data.truncated) {
409+
// A single directory level exceeded the response limit; presence is unverifiable.
410+
return { status: "apiError" };
411+
}
412+
413+
const match = response.data.tree.find((entry) => entry?.path === segment);
414+
if (!match) {
415+
return { status: "missing" };
416+
}
417+
if (match.type !== "tree") {
418+
return { status: "notDirectory" };
419+
}
420+
currentTreeish = match.sha;
421+
}
422+
423+
return { status: "found", treeSha: currentTreeish };
424+
}
425+
426+
// Inspect the (recursively fetched) "extensions" subtree for the plugin's canvas extension
427+
// entry point. Paths are relative to "extensions/", so the flat form is "extension.mjs" and a
428+
// nested form is "<name>/extension.mjs". Scoping the recursive fetch to this subtree keeps the
429+
// lookup complete without depending on the size of the rest of the repository.
430+
function analyzeCanvasExtensionSubtree(subtreeEntries) {
431+
let flatIsBlob = false;
432+
let flatIsTree = false;
433+
let nestedEntryPath = null;
434+
435+
for (const entry of subtreeEntries) {
436+
const entryPath = entry?.path;
437+
if (typeof entryPath !== "string") {
438+
continue;
439+
}
440+
441+
if (entryPath === "extension.mjs") {
442+
if (entry.type === "blob") {
443+
flatIsBlob = true;
444+
} else if (entry.type === "tree") {
445+
flatIsTree = true;
446+
}
447+
continue;
448+
}
449+
450+
const segments = entryPath.split("/");
451+
if (segments.length === 2 && segments[1] === "extension.mjs" && entry.type === "blob") {
452+
nestedEntryPath = nestedEntryPath ?? `extensions/${entryPath}`;
453+
}
454+
}
455+
456+
if (flatIsBlob) {
457+
return { status: "found", entryPath: "extensions/extension.mjs" };
458+
}
459+
if (nestedEntryPath) {
460+
return { status: "found", entryPath: nestedEntryPath };
461+
}
462+
if (flatIsTree) {
463+
return { status: "notFile" };
464+
}
465+
return { status: "notFound" };
466+
}
467+
468+
export async function validateCanvasPluginMetadata(plugin, errors, warnings, token) {
384469
const repo = plugin?.source?.repo;
385470
const sha = plugin?.source?.sha;
386471
const ref = plugin?.source?.ref;
@@ -471,41 +556,51 @@ async function validateCanvasPluginMetadata(plugin, errors, warnings, token) {
471556
);
472557
}
473558

474-
const extensionContainerPath = joinRepoPath(pluginRoot, "extensions");
475-
const extensionContainerResponse = await fetchGitHubFile(repo, extensionContainerPath, releaseLocator, token);
476-
if (extensionContainerResponse.kind === "notFound") {
559+
const unverifiableEntryPointWarning =
560+
`submission: could not verify the canvas extension entry point in GitHub repository "${repo}" at ${releaseLocatorDescription}; a maintainer should re-run intake`;
561+
const extensionsSegments = [...(pluginRoot ? pluginRoot.split("/") : []), "extensions"];
562+
const extensionsTree = await resolveDirectoryTreeSha(
563+
repo,
564+
normalizeTreeish(releaseLocator),
565+
extensionsSegments,
566+
token,
567+
);
568+
if (extensionsTree.status === "apiError") {
569+
warnings.push(unverifiableEntryPointWarning);
570+
} else if (extensionsTree.status === "missing") {
477571
errors.push(
478572
`submission: plugins tagged with "canvas" must include an "extensions" directory at ${releaseLocatorDescription}`,
479573
);
480-
} else if (extensionContainerResponse.kind === "apiError") {
481-
warnings.push(
482-
`submission: could not verify "extensions" directory in GitHub repository "${repo}" at ${releaseLocatorDescription}; a maintainer should re-run intake`,
483-
);
484-
} else if (
485-
!(
486-
extensionContainerResponse.data?.type === "dir"
487-
|| Array.isArray(extensionContainerResponse.data)
488-
)
489-
) {
574+
} else if (extensionsTree.status === "notDirectory") {
490575
errors.push(
491576
`submission: "extensions" must be a directory in ${releaseLocatorDescription}`,
492577
);
493-
}
494-
495-
const extensionEntryPath = joinRepoPath(pluginRoot, "extensions", "extension.mjs");
496-
const extensionEntryResponse = await fetchGitHubFile(repo, extensionEntryPath, releaseLocator, token);
497-
if (extensionEntryResponse.kind === "notFound") {
498-
errors.push(
499-
`submission: plugins tagged with "canvas" must include "extensions/extension.mjs" at ${releaseLocatorDescription}`,
500-
);
501-
} else if (extensionEntryResponse.kind === "apiError") {
502-
warnings.push(
503-
`submission: could not verify "extensions/extension.mjs" in GitHub repository "${repo}" at ${releaseLocatorDescription}; a maintainer should re-run intake`,
504-
);
505-
} else if (extensionEntryResponse.data?.type !== "file") {
506-
errors.push(
507-
`submission: "extensions/extension.mjs" must be a file in ${releaseLocatorDescription}`,
578+
} else {
579+
const subtreeResponse = await fetchGitHubJson(
580+
buildGitTreePath(repo, extensionsTree.treeSha, { recursive: true }),
581+
token,
508582
);
583+
if (subtreeResponse.kind !== "found" || !Array.isArray(subtreeResponse.data?.tree)) {
584+
warnings.push(unverifiableEntryPointWarning);
585+
} else {
586+
const canvasStructure = analyzeCanvasExtensionSubtree(subtreeResponse.data.tree);
587+
if (canvasStructure.status === "found") {
588+
// Entry point located (flat or nested); nothing to report.
589+
} else if (subtreeResponse.data.truncated) {
590+
// Absence is only inconclusive if the (already extensions-scoped) subtree itself is
591+
// truncated, which would take an implausibly large extensions directory; flag it as
592+
// unverifiable rather than falsely rejecting.
593+
warnings.push(unverifiableEntryPointWarning);
594+
} else if (canvasStructure.status === "notFile") {
595+
errors.push(
596+
`submission: "extensions/extension.mjs" must be a file in ${releaseLocatorDescription}`,
597+
);
598+
} else {
599+
errors.push(
600+
`submission: plugins tagged with "canvas" must include a canvas extension entry point at "extensions/extension.mjs" or "extensions/<extension>/extension.mjs" at ${releaseLocatorDescription}`,
601+
);
602+
}
603+
}
509604
}
510605

511606
const previewPath = joinRepoPath(pluginRoot, EXTERNAL_CANVAS_PREVIEW_PATH);

0 commit comments

Comments
 (0)