fix(install): cap redirects in fetchRawText to prevent loops

bin/install.js

@@ -319,15 +319,20 @@ async function promptGlobalOrLocal(c) {
 
 // ── Local install helpers ─────────────────────────────────────────────────
 
-// GET a raw text resource over HTTPS, following one level of redirects.
-// Resolves to the body string, or null on any non-200/error/timeout.
+// GET a raw text resource over HTTPS, following redirects up to MAX_REDIRECTS.
+// Resolves to the body string, or null on any non-200/error/timeout, on a
+// missing Location header, or when the redirect budget is exhausted (guards
+// against redirect loops / unbounded recursion).
+const MAX_REDIRECTS = 5;
 function fetchRawText(url) {
   return new Promise((resolve) => {
-    const get = (u) => {
+    const get = (u, redirectsLeft) => {
       const req = https.get(u, (res) => {
         if (res.statusCode === 301 || res.statusCode === 302) {
-          get(res.headers.location);
           res.resume();
+          const next = res.headers.location;
+          if (!next || redirectsLeft <= 0) { resolve(null); return; }
+          get(next, redirectsLeft - 1);
           return;
         }
         if (res.statusCode !== 200) { resolve(null); return; }
@@ -338,7 +343,7 @@ function fetchRawText(url) {
       req.on('error', () => resolve(null));
       req.setTimeout(10000, () => { req.destroy(new Error('timeout')); });
     };
-    get(url);
+    get(url, MAX_REDIRECTS);
   });
 }
 
@@ -641,4 +646,4 @@ if (require.main === module) {
 }
 
 // Exported for tests (tests/install.test.js, tests/test_installer.py) — kept minimal.
-module.exports = { loadTaskSkillSlugs, slugsFromTasksJson };
+module.exports = { loadTaskSkillSlugs, slugsFromTasksJson, fetchRawText };

tests/install.test.js

@@ -18,7 +18,36 @@ const { spawnSync } = require('node:child_process');
 const REPO_ROOT = path.resolve(__dirname, '..');
 const INSTALL_JS = path.join(REPO_ROOT, 'bin', 'install.js');
 
-const { slugsFromTasksJson, loadTaskSkillSlugs } = require(INSTALL_JS);
+const https = require('node:https');
+
+const { slugsFromTasksJson, loadTaskSkillSlugs, fetchRawText } = require(INSTALL_JS);
+
+// Swap https.get for a fake; returns a restore fn. install.js holds the same
+// cached https module object, so mutating .get here is visible to fetchRawText.
+function stubHttpsGet(handler) {
+  const real = https.get;
+  https.get = handler;
+  return () => { https.get = real; };
+}
+
+// Build a minimal response/request pair good enough for fetchRawText.
+function fakeExchange({ statusCode, location, body }) {
+  return (_url, cb) => {
+    const res = {
+      statusCode,
+      headers: location === undefined ? {} : { location },
+      resume() {},
+      on(ev, fn) {
+        if (ev === 'data' && body != null) process.nextTick(() => fn(body));
+        if (ev === 'end') process.nextTick(fn);
+        return res;
+      },
+    };
+    process.nextTick(() => cb(res));
+    const req = { on() { return req; }, setTimeout() { return req; }, destroy() {} };
+    return req;
+  };
+}
 
 test('slugsFromTasksJson lists the canonical skill slug first', () => {
   const slugs = slugsFromTasksJson(JSON.stringify({
@@ -75,6 +104,53 @@ test('slugsFromTasksJson throws when canonical_skill_slug is missing', () => {
   })));
 });
 
+test('fetchRawText resolves null on a redirect loop instead of recursing forever', async () => {
+  // Every request 301s back to the same URL. Without a cap this overflows the
+  // stack / fans out requests unbounded; with a cap it must resolve null.
+  const restore = stubHttpsGet(fakeExchange({ statusCode: 301, location: 'https://x/loop' }));
+  try {
+    const out = await fetchRawText('https://x/loop');
+    assert.equal(out, null);
+  } finally {
+    restore();
+  }
+});
+
+test('fetchRawText follows a finite redirect chain within the cap and returns the body', async () => {
+  // 3 hops (< cap of 5) then a 200 with a body.
+  let hops = 0;
+  const restore = stubHttpsGet((_url, cb) => {
+    const step = hops++;
+    const res = {
+      statusCode: step < 3 ? 302 : 200,
+      headers: step < 3 ? { location: `https://x/hop${step}` } : {},
+      resume() {},
+      on(ev, fn) {
+        if (ev === 'data' && step >= 3) process.nextTick(() => fn('PAYLOAD'));
+        if (ev === 'end') process.nextTick(fn);
+        return res;
+      },
+    };
+    process.nextTick(() => cb(res));
+    const req = { on() { return req; }, setTimeout() { return req; }, destroy() {} };
+    return req;
+  });
+  try {
+    assert.equal(await fetchRawText('https://x/start'), 'PAYLOAD');
+  } finally {
+    restore();
+  }
+});
+
+test('fetchRawText resolves null on a redirect with no Location header', async () => {
+  const restore = stubHttpsGet(fakeExchange({ statusCode: 302, location: undefined }));
+  try {
+    assert.equal(await fetchRawText('https://x/'), null);
+  } finally {
+    restore();
+  }
+});
+
 test('loadTaskSkillSlugs reads the real tasks.json from a local checkout', async () => {
   const slugs = await loadTaskSkillSlugs(REPO_ROOT);
   assert.ok(Array.isArray(slugs) && slugs.length > 0, 'expected a non-empty slug list');