fix(3628)(security): whitelist bundled hook filenames in classifier (#3630)

#3610 added a `bundled-gsd-hook` classifier to classifyPromptUserAction
that auto-removes blocked hook files on first-time-baseline scan. The
match shape was a regex `/^hooks\/gsd-[^/]+\.(?:js|sh|cjs|mjs)$/` that
matched ANY file under hooks/ named gsd-<name>.{js,sh,cjs,mjs}, not only
the 13 hooks the npm package actually ships. As a result the classifier
silently auto-classified — and the resolver auto-removed — user-authored
custom hooks (hooks/gsd-personal-experiment.js) and retired bundled hooks
from prior versions (hooks/gsd-old-statusline.js).

Evidence the maintainer was already working around this: 0862df15 (the
#3610 follow-up) renamed the integration-test fixture
hooks/gsd-retired-hook.js -> hooks/gsd-retired-hook.txt specifically to
dodge the classifier so the test could exercise the "explicit block" path
it was written for.

Fix: replace the shape regex with an explicit Set of the 13 shipped hook
filenames (BUNDLED_GSD_HOOK_FILES). Files outside the whitelist fall
through to the existing block-or-prompt flow so users retain control.

A regression guard (tests/bug-3628-bundled-hook-classifier-whitelist.test.cjs)
fails CI if the whitelist drifts from the on-disk hooks/ directory in
either direction: whitelisted-but-missing OR shipped-but-not-whitelisted.
The latter check uses the SAME shape regex the buggy classifier used,
re-purposed as a lint that ensures every gsd-*-shaped file shipped in the
distribution IS in the whitelist.

Behavioural tests cover: every entry in BUNDLED_GSD_HOOK_FILES classifies
as bundled-gsd-hook -> remove; six user-owned / retired filenames return
null (proves the regression is fixed); nested hooks/gsd-*/ directories
still return null (the #3610 nested-directory boundary stays intact);
non-gsd hooks still return null (the user-hook-preservation boundary
stays intact).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

.changeset/3628-bundled-hook-whitelist.md

@@ -0,0 +1,5 @@
+---
+type: Security
+issue: 3628
+---
+**Installer no longer auto-removes user-authored or retired `hooks/gsd-*` files** — the `bundled-gsd-hook` classifier added in #3610 used a shape regex (`/^hooks\/gsd-[^/]+\.(?:js|sh|cjs|mjs)$/`) that matched any file with that naming shape, not only the 13 hooks actually shipped in the npm package. User-authored custom hooks (e.g. `hooks/gsd-personal-experiment.js`) and retired bundled hooks from prior versions were silently auto-classified and removed on first-time-baseline scan. The classifier now whitelists the explicit set of shipped hook filenames (`BUNDLED_GSD_HOOK_FILES`); files outside the whitelist fall through to the existing block-or-prompt flow so the user retains control. A drift guard in `tests/bug-3628-bundled-hook-classifier-whitelist.test.cjs` fails CI if the whitelist diverges from the on-disk `hooks/` directory in either direction.

get-shit-done/bin/lib/installer-migration-report.cjs

@@ -98,6 +98,32 @@ function summarizeInstallerMigrationResult(result) {
   };
 }
 
+// #3628: explicit whitelist of bundled hook files shipped in the npm
+// distribution under `hooks/`. The classifier-based auto-removal of these
+// files at first-time-baseline scan (added in #3610) is restricted to this
+// set — a shape regex like `^hooks/gsd-[^/]+\.(?:js|sh|cjs|mjs)$` also
+// matches user-authored custom hooks and retired bundled hooks from prior
+// versions, and auto-removing those is silent data loss.
+//
+// The bug-3628 regression guard asserts this Set stays aligned with the
+// on-disk `hooks/` directory in both directions: whitelist-but-missing
+// AND shipped-but-not-whitelisted both fail CI.
+const BUNDLED_GSD_HOOK_FILES = Object.freeze(new Set([
+  'hooks/gsd-check-update-worker.js',
+  'hooks/gsd-check-update.js',
+  'hooks/gsd-context-monitor.js',
+  'hooks/gsd-graphify-update.sh',
+  'hooks/gsd-phase-boundary.sh',
+  'hooks/gsd-prompt-guard.js',
+  'hooks/gsd-read-guard.js',
+  'hooks/gsd-read-injection-scanner.js',
+  'hooks/gsd-session-state.sh',
+  'hooks/gsd-statusline.js',
+  'hooks/gsd-update-banner.js',
+  'hooks/gsd-validate-commit.sh',
+  'hooks/gsd-workflow-guard.js',
+]));
+
 // Classify a blocked prompt-user action into one of the safe-default
 // categories. Returns null when no safe default applies — caller must
 // fall back to the hard assertion / interactive prompt for those.
@@ -115,15 +141,14 @@ function classifyPromptUserAction(action) {
   if (/^skills\/gsd-[^/]+\/SKILL\.md$/.test(relPath)) {
     return { category: 'user-facing-skill', choice: 'keep' };
   }
-  // #3610: bundled GSD hooks at hooks/gsd-<name>.<ext>. These are part of
-  // the npm distribution (`hooks/gsd-*.{js,sh,cjs,mjs}` shipped in the
-  // package), NOT user-owned files. When a first-time-baseline scan finds
-  // them on disk without manifest entries — the case for any upgrade from
-  // a pre-manifest-baseline release — the safe default is to remove them
-  // so the installer can write the fresh bundled versions in their place.
-  // Restricted to top-level files (`hooks/gsd-X.ext`) so nested user
-  // directories like `hooks/gsd-helpers/...` do NOT auto-classify.
-  if (/^hooks\/gsd-[^/]+\.(?:js|sh|cjs|mjs)$/.test(relPath)) {
+  // #3610 / #3628: bundled GSD hooks shipped under `hooks/`. The whitelist
+  // is the explicit set of filenames in the npm distribution — files that
+  // match the shape but are NOT in the whitelist (user-authored hooks,
+  // retired hooks from prior versions) fall through to the block-or-prompt
+  // flow so the user retains control. On a first-time-baseline scan the
+  // installer can safely remove whitelisted hooks because it is about to
+  // write the fresh bundled versions in their place.
+  if (BUNDLED_GSD_HOOK_FILES.has(relPath)) {
     return { category: 'bundled-gsd-hook', choice: 'remove' };
   }
   return null;
@@ -321,6 +346,7 @@ function assertInstallerMigrationsUnblocked(result) {
 
 module.exports = {
   RESOLUTION_ENV_VAR,
+  BUNDLED_GSD_HOOK_FILES,
   assertInstallerMigrationsUnblocked,
   classifyPromptUserAction,
   resolveInstallerMigrationPromptsForNonTty,

tests/bug-3628-bundled-hook-classifier-whitelist.test.cjs

@@ -0,0 +1,140 @@
+// allow-test-rule: architectural-invariant
+// classifyPromptUserAction returns a typed result object; this test asserts
+// on that typed surface (category + choice fields) for both the positive
+// (shipped) and negative (user-owned / retired) cases. There is no rendered
+// text or stdout under test — the classifier's structured return value IS
+// the contract.
+
+/**
+ * Bug #3628: `bundled-gsd-hook` classifier (added in #3610) uses a shape
+ * regex (`/^hooks\/gsd-[^/]+\.(?:js|sh|cjs|mjs)$/`) that matches ANY file
+ * named `hooks/gsd-<name>.{js,sh,cjs,mjs}`, not only the 13 hook files
+ * actually shipped in the npm distribution. The permissive shape regex
+ * silently auto-classifies — and on first-time-baseline scan auto-removes:
+ *
+ *   - User-authored custom hooks (e.g. `hooks/gsd-personal-experiment.js`)
+ *   - Retired bundled hooks from prior GSD versions
+ *
+ * Fix: the classifier must whitelist the explicit set of shipped hook
+ * filenames sourced from a single point of truth (`BUNDLED_GSD_HOOK_FILES`
+ * exported from the classifier module). Any `hooks/gsd-<name>` file NOT in
+ * that set must fall through to the existing block-or-prompt flow so the
+ * user retains control.
+ */
+
+'use strict';
+
+const { describe, test } = require('node:test');
+const assert = require('node:assert/strict');
+
+const {
+  classifyPromptUserAction,
+  BUNDLED_GSD_HOOK_FILES,
+} = require('../get-shit-done/bin/lib/installer-migration-report.cjs');
+const path = require('node:path');
+const fs = require('node:fs');
+
+describe('bug #3628: BUNDLED_GSD_HOOK_FILES is an explicit whitelist', () => {
+  test('exports a Set of shipped hook filenames', () => {
+    assert.ok(
+      BUNDLED_GSD_HOOK_FILES instanceof Set,
+      'BUNDLED_GSD_HOOK_FILES must be exported as a Set so callers can probe membership',
+    );
+    assert.ok(
+      BUNDLED_GSD_HOOK_FILES.size > 0,
+      'BUNDLED_GSD_HOOK_FILES must enumerate at least one shipped hook',
+    );
+  });
+
+  test('every entry is a hooks/-prefixed posix path', () => {
+    for (const relPath of BUNDLED_GSD_HOOK_FILES) {
+      assert.ok(
+        relPath.startsWith('hooks/'),
+        `entry ${JSON.stringify(relPath)} must be prefixed with "hooks/"`,
+      );
+      assert.ok(
+        !relPath.includes('\\'),
+        `entry ${JSON.stringify(relPath)} must use POSIX slashes`,
+      );
+      assert.ok(
+        relPath.includes('gsd-'),
+        `entry ${JSON.stringify(relPath)} must contain the "gsd-" prefix`,
+      );
+    }
+  });
+
+  test('every BUNDLED_GSD_HOOK_FILES entry corresponds to a real file in hooks/', () => {
+    // Sourcing the whitelist from a frozen constant is only durable if the
+    // constant stays aligned with the on-disk distribution. This guard
+    // fails the day someone removes a hook file but forgets to update the
+    // whitelist (or vice-versa).
+    const hooksDir = path.join(__dirname, '..', 'hooks');
+    for (const relPath of BUNDLED_GSD_HOOK_FILES) {
+      const fullPath = path.join(hooksDir, relPath.slice('hooks/'.length));
+      assert.ok(
+        fs.existsSync(fullPath),
+        `whitelisted ${relPath} is missing from hooks/ on disk — whitelist drifted`,
+      );
+    }
+  });
+
+  test('every gsd-*.{js,sh,cjs,mjs} file in hooks/ is in BUNDLED_GSD_HOOK_FILES (no shipping drift)', () => {
+    const hooksDir = path.join(__dirname, '..', 'hooks');
+    const onDisk = fs
+      .readdirSync(hooksDir, { withFileTypes: true })
+      .filter((e) => e.isFile() && /^gsd-[^/]+\.(?:js|sh|cjs|mjs)$/.test(e.name))
+      .map((e) => `hooks/${e.name}`);
+    for (const relPath of onDisk) {
+      assert.ok(
+        BUNDLED_GSD_HOOK_FILES.has(relPath),
+        `${relPath} ships in hooks/ but is missing from BUNDLED_GSD_HOOK_FILES — whitelist drifted`,
+      );
+    }
+  });
+});
+
+describe('bug #3628: classifyPromptUserAction whitelists shipped bundled hooks', () => {
+  test('classifies every entry in BUNDLED_GSD_HOOK_FILES as bundled-gsd-hook → remove', () => {
+    for (const relPath of BUNDLED_GSD_HOOK_FILES) {
+      const result = classifyPromptUserAction({ relPath });
+      assert.deepStrictEqual(
+        result,
+        { category: 'bundled-gsd-hook', choice: 'remove' },
+        `${relPath} should classify as bundled-gsd-hook`,
+      );
+    }
+  });
+
+  const USER_OWNED_OR_RETIRED = [
+    'hooks/gsd-personal-experiment.js',
+    'hooks/gsd-my-custom-guard.sh',
+    'hooks/gsd-team-policy.cjs',
+    'hooks/gsd-retired-hook.js',
+    'hooks/gsd-old-statusline.js',
+    'hooks/gsd-experimental.mjs',
+  ];
+
+  for (const relPath of USER_OWNED_OR_RETIRED) {
+    test(`does NOT classify ${relPath} (user-owned / retired)`, () => {
+      assert.strictEqual(
+        classifyPromptUserAction({ relPath }),
+        null,
+        `${relPath} must NOT auto-classify — falls through to block-or-prompt`,
+      );
+    });
+  }
+
+  test('still does NOT classify nested gsd-* directories (existing #3610 boundary preserved)', () => {
+    assert.strictEqual(
+      classifyPromptUserAction({ relPath: 'hooks/gsd-helpers/index.js' }),
+      null,
+    );
+  });
+
+  test('still does NOT classify non-gsd hooks (existing boundary preserved)', () => {
+    assert.strictEqual(
+      classifyPromptUserAction({ relPath: 'hooks/my-custom-hook.js' }),
+      null,
+    );
+  });
+});