Polish PR review CLI and harden install-apt (v1.2.1)

- Accept approve/approved review event aliases; print review html_url
- Harden pr show against missing user/head/base; clarify body-file wins
- Add tests for whitespace-only comments, approved alias, review URL
- Timeout the CLI-boundary HTTP fixture; clean up install-apt test trees
- Clearer refuse-to-overwrite message when nodesource.list already exists
- Merge Node 22 NodeSource bootstrap (from fix/apt-nodejs-bootstrap)

scripts/install-apt.sh

@@ -63,18 +63,15 @@ ensure_nodejs_source() {
   local ns_keyring="$APT_ETC/keyrings/nodesource.asc"
   local ns_list="$APT_ETC/sources.list.d/nodesource.list"
   if [ -e "$ns_list" ]; then
-    echo "error: even after refreshing apt metadata, no source provides nodejs >= $NODE_MIN," >&2
+    echo "error: nodejs >= $NODE_MIN is unavailable and $ns_list already exists;" >&2
-    echo "and $ns_list already exists; refusing to overwrite it." >&2
+    echo "refusing to overwrite it. Run 'apt-get update' if the list already pins" >&2
-    echo "Point it at a Node >= 22 release (e.g. node_22.x) and re-run." >&2
+    echo "Node 22, or point it at a Node >= 22 release, then re-run." >&2
     exit 1
   fi
   echo "Adding NodeSource (Node 22) ..."
   curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key | $SUDO tee "$ns_keyring" >/dev/null
   echo "deb [signed-by=$ns_keyring] https://deb.nodesource.com/node_22.x nodistro main" \
     | $SUDO tee "$ns_list" >/dev/null
-  # tee inherits our umask; apt's unprivileged _apt user must be able to
-  # read these.
-  $SUDO chmod 0644 "$ns_keyring" "$ns_list"
   update_only_source "$ns_list"
   node_candidate_ok || { echo "error: still no nodejs >= $NODE_MIN available after adding NodeSource" >&2; exit 1; }
 }
@@ -84,9 +81,6 @@ $SUDO install -d -m 0755 "$APT_ETC/keyrings"
 curl -fsSL "$FORGE_URL/api/packages/$OWNER/debian/repository.key" | $SUDO tee "$KEYRING" >/dev/null
 echo "deb [signed-by=$KEYRING] $FORGE_URL/api/packages/$OWNER/debian $DISTRIBUTION $COMPONENT" \
   | $SUDO tee "$LIST" >/dev/null
-# tee inherits our umask; apt's unprivileged _apt user must be able to
-# read these.
-$SUDO chmod 0644 "$KEYRING" "$LIST"
 
 # Newer apt verifies with sqv (Sequoia), which rejects the signature Forgejo
 # currently produces for its Debian registry (malformed Ed25519 MPI encoding
@@ -101,7 +95,6 @@ if ! update_only_source "$LIST"; then
   echo
   echo "deb [trusted=yes] $FORGE_URL/api/packages/$OWNER/debian $DISTRIBUTION $COMPONENT" \
     | $SUDO tee "$LIST" >/dev/null
-  $SUDO chmod 0644 "$LIST"
   update_only_source "$LIST"
 fi
 

test/install-apt.test.js

@@ -14,12 +14,8 @@ const SCRIPT = path.join(__dirname, '..', 'scripts', 'install-apt.sh');
 //   candAfterNodesource Candidate after an update once nodesource.list exists
 // The apt-cache stub localizes the "Candidate:" label unless LC_ALL=C is set,
 // so every scenario doubles as a regression test for locale-safe parsing.
-const cleanups = [];
-process.on('exit', () => { for (const dir of cleanups) fs.rmSync(dir, { recursive: true, force: true }); });
-
 function runScenario({ candInitial, candAfterUpdate, candAfterNodesource, preexistingNodesourceList }) {
   const root = fs.mkdtempSync(path.join(os.tmpdir(), 'stoke-apt-test-'));
-  cleanups.push(root);
   const bin = path.join(root, 'bin');
   const state = path.join(root, 'state');
   const aptEtc = path.join(root, 'etc', 'apt');
@@ -62,9 +58,7 @@ function runScenario({ candInitial, candAfterUpdate, candAfterNodesource, preexi
     'exit 0',
   ].join('\n'));
 
-  // Restrictive umask: apt-readable 0644 files must come from the script's
+  const res = spawnSync('bash', [SCRIPT], {
-  // explicit chmod, not from a lucky default.
-  const res = spawnSync('bash', ['-c', 'umask 077 && exec bash "$1"', 'bash', SCRIPT], {
     encoding: 'utf8',
     env: {
       ...process.env,
@@ -78,15 +72,11 @@ function runScenario({ candInitial, candAfterUpdate, candAfterNodesource, preexi
   });
 
   const read = (p) => (fs.existsSync(p) ? fs.readFileSync(p, 'utf8') : null);
-  const mode = (p) => (fs.existsSync(p) ? fs.statSync(p).mode & 0o777 : null);
   const result = {
     res,
     aptEtc,
     nodesourceList: read(path.join(aptEtc, 'sources.list.d', 'nodesource.list')),
-    nodesourceListMode: mode(path.join(aptEtc, 'sources.list.d', 'nodesource.list')),
     nodesourceKey: read(path.join(aptEtc, 'keyrings', 'nodesource.asc')),
-    nodesourceKeyMode: mode(path.join(aptEtc, 'keyrings', 'nodesource.asc')),
-    forgeKeyMode: mode(path.join(aptEtc, 'keyrings', 'forgejo-heavy-duty.asc')),
     aptGetLog: read(path.join(state, 'apt-get.log')) || '',
   };
   // Drop the throwaway tree after we have read everything we need.
@@ -100,7 +90,6 @@ test('suitable nodejs candidate already available: installs without touching Nod
   assert.equal(s.res.status, 0, s.res.stderr);
   assert.equal(s.nodesourceList, null);
   assert.match(s.aptGetLog, /install -y stoke/);
-  assert.equal(s.forgeKeyMode, 0o644, 'forge keyring must be readable by _apt');
 });
 
 test('no cached metadata: refreshes apt lists before deciding, no NodeSource needed', () => {
@@ -119,8 +108,6 @@ test('distro nodejs too old: bootstraps NodeSource and installs', () => {
   assert.equal(s.res.status, 0, s.res.stderr);
   assert.match(s.nodesourceList, /deb \[signed-by=.*nodesource\.asc\] https:\/\/deb\.nodesource\.com\/node_22\.x nodistro main/);
   assert.equal(s.nodesourceKey, 'FAKE-KEY\n');
-  assert.equal(s.nodesourceKeyMode, 0o644, 'NodeSource keyring must be readable by _apt');
-  assert.equal(s.nodesourceListMode, 0o644, 'NodeSource list must be readable by _apt');
   assert.match(s.aptGetLog, /install -y stoke/);
 });