fix: harden scaffold download safety

.github/workflows/ci.yml

@@ -73,7 +73,10 @@ jobs:
 
       - name: Check licenses
         if: matrix.node-version == 22
-        run: npx --yes license-checker --failOn "GPL-2.0;GPL-3.0;AGPL-3.0"
+        # Pin the top-level package version instead of resolving "latest" at CI
+        # time, matching the SHA/version-pinning discipline used for gitleaks and
+        # actions elsewhere in this template. Bump this alongside other deps.
+        run: npx --yes license-checker@25.0.1 --failOn "GPL-2.0;GPL-3.0;AGPL-3.0"
 
       - name: Verify EN/KO docs heading parity
         if: matrix.node-version == 22

.github/workflows/publish.yml

@@ -157,7 +157,7 @@ jobs:
 
       - name: Create GitHub Release
         if: github.event_name == 'push' || inputs.dry_run == false
-        uses: softprops/action-gh-release@v3
+        uses: softprops/action-gh-release@718ea10b132b3b2eba29c1007bb80653f286566b # v3.0.1
         with:
           tag_name: v${{ steps.pkg.outputs.version }}
           name: v${{ steps.pkg.outputs.version }}

src/audit-cd.ts

@@ -441,7 +441,15 @@ function probeGithubReleases(
     const out = execFileSync(
       "gh",
       ["release", "view", "--json", "tagName,publishedAt", "-R", id],
-      { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"] },
+      {
+        encoding: "utf-8",
+        stdio: ["ignore", "pipe", "pipe"],
+        // Bound the outbound GitHub API call so a slow/unreachable host or a
+        // gh auth prompt can't hang the single-process MCP server. On timeout
+        // execFileSync throws and the catch below degrades to an error status.
+        timeout: 10_000,
+        killSignal: "SIGKILL",
+      },
     );
     const data = JSON.parse(out) as { tagName?: string; publishedAt?: string };
     const tag = data.tagName ?? null;

src/audit-security.ts

@@ -251,7 +251,15 @@ function checkSecretScanning(
       const out = execFileSync(
         "gh",
         ["api", `repos/${remote}`, "--jq", ".security_and_analysis.secret_scanning.status // \"\""],
-        { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"] },
+        {
+          encoding: "utf-8",
+          stdio: ["ignore", "pipe", "pipe"],
+          // Bound the outbound GitHub API call so a slow/unreachable host or a
+          // gh auth prompt can't hang the single-process MCP server. On timeout
+          // execFileSync throws and the catch below falls through gracefully.
+          timeout: 10_000,
+          killSignal: "SIGKILL",
+        },
       ).trim();
       if (out === "enabled") {
         return {

src/download.ts

@@ -1,5 +1,5 @@
 import { mkdir } from "node:fs/promises";
-import { isAbsolute, normalize } from "node:path";
+import { resolve, sep } from "node:path";
 import { Readable } from "node:stream";
 import { pipeline } from "node:stream/promises";
 import { extract } from "tar";
@@ -115,6 +115,10 @@ async function fetchOnce(args: FetchOnceArgs): Promise<Buffer> {
       if (done) break;
       total += value.byteLength;
       if (total > maxSizeBytes) {
+        // Tear down the underlying fetch/socket the same way the timeout path
+        // does, so the response body isn't left undrained (fd/socket leak) on
+        // this DoS-defense branch.
+        controller.abort();
         throw new DownloadError(
           `download exceeded limit ${maxSizeBytes} bytes`,
           "SIZE_EXCEEDED",
@@ -147,26 +151,41 @@ async function fetchOnce(args: FetchOnceArgs): Promise<Buffer> {
 /**
  * Reject tar entries that would escape `destDir` (zip-slip / path-traversal).
  *
- * `tar.extract`'s `strip: 1` peels the leading "<repo>-<sha>/" segment off
- * GitHub archives, so by the time `filter` sees a path it should be a
- * relative path inside the project. Anything else — absolute paths, paths
- * containing `..`, Windows drive letters — is a malicious or corrupt entry.
+ * node-tar invokes the user `filter` on the RAW entry path — BEFORE the
+ * `strip: 1` that peels the leading "<repo>-<sha>/" segment off GitHub
+ * archives is applied. So this guard must replicate `strip: 1` itself: it
+ * drops the first raw segment, then resolves the remainder against `destDir`
+ * and requires the result to stay strictly inside `destDir`. Doing so on the
+ * post-strip path is what node-tar will actually extract to, and resolving
+ * against `destDir` is robust regardless of `..`, absolute paths, or Windows
+ * drive letters.
+ *
+ * IMPORTANT: the first raw segment is dropped BEFORE any normalization —
+ * normalizing first would let `repo/../../escape` collapse to `../escape` and
+ * then falsely drop `..` as the "first segment".
  *
  * `tar.extract` also rejects symlinks/hardlinks that target outside cwd by
  * default; we keep that behavior and add an explicit path check on top.
  */
-export function isSafeTarEntry(path: string): boolean {
-  if (!path) return false;
-  if (isAbsolute(path)) return false;
-  // Reject Windows drive letters that look relative on POSIX (e.g. "C:foo").
-  if (/^[A-Za-z]:/.test(path)) return false;
-  // `normalize` collapses `a/../b` → `b`. If the result still starts with
-  // `..` it means the entry escapes cwd.
-  const normalized = normalize(path);
-  if (normalized.startsWith("..") || normalized.includes(`${"/"}..${"/"}`)) {
-    return false;
-  }
-  return true;
+export function isSafeTarEntry(rawPath: string, destDir: string): boolean {
+  if (!rawPath) return false;
+  // Replicate node-tar's `strip: 1` on the RAW path first (drop the leading
+  // "<repo>-<sha>/" segment).
+  const stripped = stripTopLevelTarSegment(rawPath);
+  // Entry was just the top-level directory itself — nothing to extract.
+  if (!stripped) return false;
+  // Resolve against destDir and require the result to stay strictly inside it.
+  // Using `resolvedDest + sep` (not a bare prefix) prevents a sibling like
+  // "/dest-evil" from matching "/dest".
+  const resolvedDest = resolve(destDir);
+  const target = resolve(resolvedDest, stripped);
+  return target === resolvedDest || target.startsWith(resolvedDest + sep);
+}
+
+function stripTopLevelTarSegment(rawPath: string): string {
+  const parts = rawPath.replace(/\\/g, "/").split("/");
+  parts.shift();
+  return parts.join("/");
 }
 
 export async function extractTarball(
@@ -183,7 +202,10 @@ export async function extractTarball(
       cwd: destDir,
       strip: 1,
       filter: (path) => {
-        if (!isSafeTarEntry(path)) {
+        if (!stripTopLevelTarSegment(path)) {
+          return false;
+        }
+        if (!isSafeTarEntry(path, destDir)) {
           // Capture the first offender so we can surface it in the error
           // instead of silently skipping (which is what `filter` does).
           rejected ??= path;

tests/download.test.ts

@@ -114,39 +114,50 @@ describe("fetchTarball", () => {
 });
 
 describe("isSafeTarEntry (zip-slip / path-traversal guard)", () => {
+  // node-tar invokes `filter` on the RAW entry path (before `strip: 1`), so
+  // every entry from a GitHub archive carries a leading "<repo>-<sha>/"
+  // segment. isSafeTarEntry replicates strip:1 itself, then checks the result
+  // stays inside destDir. Tests therefore use raw (pre-strip) paths + a destDir.
+  const destDir = "/tmp/create-starter-dest";
   const safe = [
-    "package.json",
-    "src/index.ts",
-    "deeply/nested/file.txt",
-    "with-dashes/and_underscores.md",
-    "trailing/slash/", // tar can emit directory entries
+    "repo-abc/package.json",
+    "repo-abc/src/index.ts",
+    "repo-abc/deeply/nested/file.txt",
+    "repo-abc/with-dashes/and_underscores.md",
+    "repo-abc/trailing/slash/", // tar can emit directory entries
   ];
   const unsafe = [
     "",
-    "/etc/passwd",
-    "/absolute/path",
-    "../escape",
-    "ok/then/../../../escape", // collapses to ../escape — escapes cwd
-    "C:windows", // Windows drive letter
-    "C:\\Users\\Public",
-    "..",
-    "../",
-    "../../",
+    "repo-abc/../escape.txt", // after strip:1 → ../escape.txt — escapes destDir
+    "repo-abc/../../escape.txt", // after strip:1 → ../../escape.txt
+    "repo-abc/ok/then/../../../escape", // after strip:1 collapses to ../escape
+    "repo-abc", // top-level dir only — nothing to extract after strip
+    "repo-abc/", // ditto
+    "repo-abc/..",
+    "repo-abc/../",
   ];
 
-  it("accepts paths that resolve back inside cwd even with .. mid-path", () => {
-    // `ok/then/../../escape` → `escape` (still inside cwd) — safe.
-    assert.equal(isSafeTarEntry("ok/then/../../escape"), true);
+  it("accepts raw paths that resolve back inside destDir even with .. mid-path", () => {
+    // `repo-abc/ok/then/../../escape` → strip → `ok/then/../../escape`
+    // → resolves to `escape` (still inside destDir) — safe.
+    assert.equal(isSafeTarEntry("repo-abc/ok/then/../../escape", destDir), true);
+  });
+
+  it("strips the RAW first segment before normalizing (double-.. escape)", () => {
+    // If it normalized first, `repo-abc/../../escape` would collapse to
+    // `../escape`, then dropping `..` as the "first segment" would falsely
+    // accept `escape`. Stripping the raw segment first correctly rejects it.
+    assert.equal(isSafeTarEntry("repo-abc/../../escape", destDir), false);
   });
 
   for (const p of safe) {
     it(`accepts ${JSON.stringify(p)}`, () => {
-      assert.equal(isSafeTarEntry(p), true);
+      assert.equal(isSafeTarEntry(p, destDir), true);
     });
   }
   for (const p of unsafe) {
     it(`rejects ${JSON.stringify(p)}`, () => {
-      assert.equal(isSafeTarEntry(p), false);
+      assert.equal(isSafeTarEntry(p, destDir), false);
     });
   }
 });