fix: accessible GitHub auth dialog + argv-form exec at three sites

src/data.js

@@ -5954,10 +5954,16 @@ function findQwenSessionByPid(pid, cwd, allSessions) {
   const byCwd = [];
 
   try {
-    const lsofOut = execSync(`lsof -a -p ${pid} -Fn 2>/dev/null`, {
+    // argv form, not a shell string: `pid` reaches here from parsed `ps`
+    // output so it's numeric today, but the shell-interpolated version was
+    // one refactor away from being a real injection. (The sibling lsof call
+    // in getActiveSessions already uses execFileSync — this was the outlier.)
+    // stderr is ignored via stdio instead of a `2>/dev/null` redirect, which
+    // needed a shell in the first place.
+    const lsofOut = execFileSync('lsof', ['-a', '-p', String(pid), '-Fn'], {
       encoding: 'utf8',
       timeout: 2000,
-      stdio: ['pipe', 'pipe', 'pipe'],
+      stdio: ['pipe', 'pipe', 'ignore'],
     });
     for (const line of lsofOut.split('\n')) {
       const match = line.match(/(\/.*\.qwen\/projects\/.*\/(?:chats|sessions)\/([0-9a-f-]{36})\.jsonl)$/i);

src/frontend/leaderboard.js

@@ -116,61 +116,143 @@ async function loadGlobalLeaderboard() {
   } catch { if (board) board.innerHTML = '<div style="text-align:center;padding:20px;color:var(--text-muted)">Could not load global leaderboard</div>'; }
 }
 
+// The device-code modal is built once and reused. Kept self-contained (its own
+// Escape/Tab handling rather than app.js's _installModalFocusTrap) because that
+// helper dispatches Escape through a hardcoded if-chain of overlay ids — worth
+// generalizing into a shared close-callback, but that's a cross-cutting change
+// to a file several in-flight PRs already touch.
+function _lbBuildAuthModal() {
+  var modal = document.createElement('div');
+  modal.id = 'githubAuthModal';
+  modal.className = 'confirm-overlay';
+  modal.setAttribute('role', 'dialog');
+  modal.setAttribute('aria-modal', 'true');
+  modal.setAttribute('aria-labelledby', 'githubAuthTitle');
+  modal.innerHTML = '<div class="confirm-box" style="max-width:380px;text-align:center">' +
+    '<h3 id="githubAuthTitle">Connect GitHub</h3>' +
+    '<p style="font-size:13px;margin:12px 0">Copy this code and enter it at:</p>' +
+    '<div class="lb-auth-code" id="githubAuthCode"></div>' +
+    '<a id="githubAuthLink" href="" target="_blank" rel="noopener noreferrer" class="lb-github-btn" style="display:inline-flex;margin:12px 0">Open GitHub</a>' +
+    // aria-live so a screen reader hears "Code expired" / "Connection error"
+    // without the user having to go hunting for the status line.
+    '<p style="font-size:12px;color:var(--text-muted)" id="githubAuthStatus" role="status" aria-live="polite">Waiting for authorization...</p>' +
+    '<button class="btn-cancel" id="githubAuthCancel" style="margin-top:8px">Cancel</button>' +
+    '</div>';
+  document.body.appendChild(modal);
+  modal.querySelector('#githubAuthCancel').addEventListener('click', function () { _lbCloseAuthModal(); });
+  modal.addEventListener('keydown', function (e) {
+    if (e.key === 'Escape') { e.stopPropagation(); _lbCloseAuthModal(); return; }
+    if (e.key !== 'Tab') return;
+    // Keep Tab inside the dialog — it's aria-modal, so tabbing onto the page
+    // behind it would contradict what a screen reader just announced.
+    var nodes = Array.prototype.filter.call(
+      modal.querySelectorAll('a[href], button, [tabindex]:not([tabindex="-1"])'),
+      function (el) { return !el.disabled && el.offsetParent !== null; }
+    );
+    if (!nodes.length) return;
+    var first = nodes[0], last = nodes[nodes.length - 1];
+    if (e.shiftKey && document.activeElement === first) { e.preventDefault(); last.focus(); }
+    else if (!e.shiftKey && document.activeElement === last) { e.preventDefault(); first.focus(); }
+  });
+  return modal;
+}
+
+var _lbAuthFocusReturn = null;
+
+function _lbCloseAuthModal() {
+  var modal = document.getElementById('githubAuthModal');
+  if (modal) modal.style.display = 'none';
+  // Return focus to whatever opened the dialog, unless it's since been
+  // re-rendered away (render() replaces the leaderboard markup on success).
+  if (_lbAuthFocusReturn && _lbAuthFocusReturn.focus && document.body.contains(_lbAuthFocusReturn)) {
+    try { _lbAuthFocusReturn.focus(); } catch (e) {}
+  }
+  _lbAuthFocusReturn = null;
+}
+
+function _lbAuthStatus(text) {
+  var el = document.getElementById('githubAuthStatus');
+  if (el) el.textContent = text;
+}
+
 async function githubConnect() {
+  var modal = null;
   try {
     showToast('Starting GitHub auth...');
     var resp = await fetch('/api/github/device-code', { method: 'POST' });
     var data = await resp.json();
     if (data.error) { showToast('Error: ' + data.error); return; }
 
-    // Show modal with code
-    var modal = document.getElementById('githubAuthModal');
-    if (!modal) {
-      modal = document.createElement('div');
-      modal.id = 'githubAuthModal';
-      modal.className = 'confirm-overlay';
-      modal.style.display = 'flex';
-      modal.innerHTML = '<div class="confirm-box" style="max-width:380px;text-align:center">' +
-        '<h3>Connect GitHub</h3>' +
-        '<p style="font-size:13px;margin:12px 0">Copy this code and enter it at:</p>' +
-        '<div class="lb-auth-code" id="githubAuthCode"></div>' +
-        '<a id="githubAuthLink" href="" target="_blank" class="lb-github-btn" style="display:inline-flex;margin:12px 0">Open GitHub</a>' +
-        '<p style="font-size:12px;color:var(--text-muted)" id="githubAuthStatus">Waiting for authorization...</p>' +
-        '<button class="btn-cancel" onclick="this.parentElement.parentElement.style.display=\'none\'" style="margin-top:8px">Cancel</button>' +
-        '</div>';
-      document.body.appendChild(modal);
-    } else {
-      modal.style.display = 'flex';
-    }
+    _lbAuthFocusReturn = document.activeElement;
+    modal = document.getElementById('githubAuthModal') || _lbBuildAuthModal();
+    modal.style.display = 'flex';
     document.getElementById('githubAuthCode').textContent = data.user_code;
     document.getElementById('githubAuthLink').href = data.verification_uri;
+    _lbAuthStatus('Waiting for authorization...');
+    // Move focus into the dialog so keyboard users land inside it.
+    var link = document.getElementById('githubAuthLink');
+    if (link) link.focus();
 
     copyText(data.user_code, 'Copied GitHub code');
 
-    // Poll for token
+    // Poll for the token. Mirrors pollRepoScopeOnce() in app.js: honour
+    // slow_down per RFC 8628 §3.5, surface a persistent network failure
+    // instead of spinning silently, and never leave the user staring at
+    // "Waiting for authorization..." with no idea anything went wrong.
     var interval = (data.interval || 5) * 1000;
-    var maxTries = Math.ceil((data.expires_in || 900) / (interval / 1000));
-    for (var i = 0; i < maxTries; i++) {
-      await new Promise(function(r) { setTimeout(r, interval); });
+    var deadline = Date.now() + ((data.expires_in || 900) * 1000);
+    var errorStreak = 0;
+
+    while (Date.now() < deadline) {
+      await new Promise(function (r) { setTimeout(r, interval); });
       if (modal.style.display === 'none') return; // cancelled
+
+      var pollData;
       try {
         var pollResp = await fetch('/api/github/poll-token', {
-          method: 'POST', headers: {'Content-Type':'application/json'},
+          method: 'POST', headers: { 'Content-Type': 'application/json' },
           body: JSON.stringify({ device_code: data.device_code })
         });
-        var pollData = await pollResp.json();
-        if (pollData.status === 'ok') {
-          modal.style.display = 'none';
-          showToast('Connected as @' + pollData.profile.username);
-          render();
-          return;
-        } else if (pollData.status === 'expired') {
-          document.getElementById('githubAuthStatus').textContent = 'Code expired. Try again.';
+        pollData = await pollResp.json();
+      } catch (e) {
+        // Transient failures shouldn't kill the flow; persistent ones must
+        // not be swallowed (the old bare `catch {}` polled on in silence).
+        errorStreak += 1;
+        if (errorStreak >= 3) {
+          _lbAuthStatus('Connection error — check your network and try again.');
           return;
         }
-      } catch {}
+        _lbAuthStatus('Network hiccup, retrying…');
+        continue;
+      }
+      errorStreak = 0;
+
+      if (pollData.error) {
+        _lbAuthStatus('Error: ' + pollData.error);
+        return;
+      }
+      if (pollData.status === 'ok') {
+        _lbCloseAuthModal();
+        showToast('Connected as @' + pollData.profile.username);
+        render();
+        return;
+      }
+      if (pollData.status === 'expired') {
+        _lbAuthStatus('Code expired. Close this and click Connect to try again.');
+        return;
+      }
+      if (pollData.status === 'slow_down') {
+        interval += 5000;      // RFC 8628 §3.5
+        _lbAuthStatus('Waiting for authorization…');
+        continue;
+      }
+      _lbAuthStatus('Waiting for authorization...');
     }
-  } catch (e) { showToast('Auth error: ' + e.message); }
+    _lbAuthStatus('Code expired. Close this and click Connect to try again.');
+  } catch (e) {
+    if (modal) _lbAuthStatus('Auth error: ' + e.message);
+    showToast('Auth error: ' + e.message);
+  }
 }
 
 async function githubLogout() {

src/migrate.js

@@ -3,7 +3,7 @@
 const fs = require('fs');
 const path = require('path');
 const os = require('os');
-const { execSync, execFileSync } = require('child_process');
+const { execFileSync } = require('child_process');
 
 const CLAUDE_DIR = path.join(os.homedir(), '.claude');
 const CODEX_DIR = path.join(os.homedir(), '.codex');
@@ -96,21 +96,29 @@ function exportArchive(outPath) {
     return;
   }
 
-  // Calculate sizes
-  let totalSize = 0;
+  // Count files. execFileSync with an argument array (not a shell string) for
+  // the same reason the tar call below spells it out: a home path containing
+  // quotes/backticks/$ must not be able to break quoting. Counting lines here
+  // also drops the `| wc -l` pipe, which needed a shell to begin with.
+  //
+  // A `du -sb … || du -sk …` total used to be summed alongside this and then
+  // never printed. Dead, and wrong on macOS besides: BSD du has no -b, so it
+  // always fell through to -sk and added KILOBYTES to a byte total.
   let totalFiles = 0;
   for (const p of paths) {
     const full = path.join(os.homedir(), p);
     if (fs.existsSync(full)) {
-      const stat = fs.statSync(full);
-      if (stat.isDirectory()) {
-        const output = execSync(`find "${full}" -type f | wc -l`, { encoding: 'utf8' }).trim();
-        totalFiles += parseInt(output) || 0;
-        const sizeOut = execSync(`du -sb "${full}" 2>/dev/null || du -sk "${full}"`, { encoding: 'utf8' }).trim();
-        totalSize += parseInt(sizeOut) || 0;
+      if (fs.statSync(full).isDirectory()) {
+        try {
+          const output = execFileSync('find', [full, '-type', 'f'], {
+            encoding: 'utf8',
+            maxBuffer: 64 * 1024 * 1024,
+            stdio: ['pipe', 'pipe', 'ignore'],
+          });
+          totalFiles += output.split('\n').filter(Boolean).length;
+        } catch { /* unreadable subtree — the count is cosmetic, keep going */ }
       } else {
         totalFiles++;
-        totalSize += stat.size;
       }
     }
   }

test/auth-modal-and-exec-safety.test.js

@@ -0,0 +1,118 @@
+'use strict';
+
+// Two backlog items from the UX/server audit:
+//
+//  1. The leaderboard's GitHub device-code modal was a bare div — no dialog
+//     semantics, no Escape, no focus handling — and its poll loop swallowed
+//     every network error in a bare `catch {}`, leaving the user staring at
+//     "Waiting for authorization..." forever. app.js's repo-scope flow
+//     (pollRepoScopeOnce) already had the right shape; this brings parity.
+//
+//  2. Three exec sites interpolated values into a shell string instead of
+//     using the argv form the rest of the codebase documents as the
+//     injection-safe pattern (see the tar call in migrate.js).
+//
+// Source-level contract tests, matching the style of the other frontend
+// tests here (the browser files aren't importable modules).
+
+const test = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('fs');
+const path = require('path');
+
+function src(rel) {
+  return fs.readFileSync(path.join(__dirname, '..', rel), 'utf8');
+}
+
+function fn(source, name) {
+  const m = source.match(new RegExp('(?:async )?function ' + name + '\\([\\s\\S]*?\\n\\}'));
+  assert.ok(m, name + ' should exist');
+  return m[0];
+}
+
+// ── 1. GitHub auth modal ────────────────────────────────────────────────────
+
+test('the auth modal is a real dialog', () => {
+  const body = fn(src('src/frontend/leaderboard.js'), '_lbBuildAuthModal');
+  assert.match(body, /'role', 'dialog'/, 'must be role="dialog"');
+  assert.match(body, /'aria-modal', 'true'/, 'must be aria-modal');
+  assert.match(body, /aria-labelledby/, 'must point at its own title');
+});
+
+test('the auth modal handles Escape and traps Tab', () => {
+  const body = fn(src('src/frontend/leaderboard.js'), '_lbBuildAuthModal');
+  assert.match(body, /e\.key === 'Escape'/, 'Escape must close the dialog');
+  assert.match(body, /e\.key !== 'Tab'/, 'Tab must be handled for focus trapping');
+  assert.match(body, /shiftKey/, 'Shift+Tab must wrap backwards');
+});
+
+test('closing the modal returns focus to whatever opened it', () => {
+  const source = src('src/frontend/leaderboard.js');
+  const body = fn(source, '_lbCloseAuthModal');
+  assert.match(body, /_lbAuthFocusReturn/, 'must restore the saved focus target');
+  assert.match(body, /document\.body\.contains/,
+    'must not focus a node that render() has since replaced');
+  assert.match(fn(source, 'githubConnect'), /_lbAuthFocusReturn = document\.activeElement/,
+    'must capture the opener before showing the dialog');
+});
+
+test('the status line is announced to screen readers', () => {
+  const body = fn(src('src/frontend/leaderboard.js'), '_lbBuildAuthModal');
+  assert.match(body, /aria-live/, 'status updates must be announced, not just painted');
+});
+
+test('the poll loop surfaces failures instead of swallowing them', () => {
+  const body = fn(src('src/frontend/leaderboard.js'), 'githubConnect');
+  assert.doesNotMatch(body, /\}\s*catch\s*\{\s*\}/, 'no bare catch {} may remain');
+  assert.match(body, /errorStreak/, 'repeated network failures must be counted');
+  assert.match(body, /Connection error/, 'a persistent failure must be shown to the user');
+  assert.match(body, /pollData\.error/, 'an error field from the server must be surfaced');
+});
+
+test('the poll loop honours slow_down (RFC 8628 §3.5)', () => {
+  const body = fn(src('src/frontend/leaderboard.js'), 'githubConnect');
+  assert.match(body, /slow_down/, 'must handle the slow_down status');
+  assert.match(body, /interval \+= 5000/, 'must back off by at least 5s, per the RFC');
+});
+
+test('the cancel button is wired in JS, not by walking parentElement', () => {
+  const body = fn(src('src/frontend/leaderboard.js'), '_lbBuildAuthModal');
+  assert.doesNotMatch(body, /parentElement\.parentElement/,
+    'brittle DOM walking in an inline onclick should be a real handler');
+  assert.match(body, /githubAuthCancel/, 'the cancel button should be addressed by id');
+});
+
+// ── 2. No interpolated shell strings at the flagged exec sites ───────────────
+
+// Matches execSync(`...${x}...`) — a template literal carrying an
+// interpolation, i.e. a value being spliced into a shell command line.
+const INTERPOLATED_EXEC = /execSync\(\s*`[^`]*\$\{/;
+
+test('findQwenSessionByPid runs lsof via argv, not a shell string', () => {
+  const body = fn(src('src/data.js'), 'findQwenSessionByPid');
+  assert.doesNotMatch(body, INTERPOLATED_EXEC, 'must not interpolate the pid into a shell command');
+  assert.match(body, /execFileSync\('lsof', \['-a', '-p', String\(pid\), '-Fn'\]/,
+    'must pass the pid as a separate argv entry');
+});
+
+test('migrate.js no longer builds shell commands from home paths', () => {
+  const source = src('src/migrate.js');
+  assert.doesNotMatch(source, INTERPOLATED_EXEC, 'no interpolated shell command may remain');
+  assert.doesNotMatch(source, /execSync/, 'migrate.js should not need execSync at all now');
+  assert.match(source, /execFileSync\('find', \[full, '-type', 'f'\]/,
+    'find must take the path as an argv entry');
+});
+
+// Comments deliberately describe the removed `du` call and why — assert on
+// the code itself, not the explanation of it.
+function stripComments(source) {
+  return source.replace(/^\s*\/\/.*$/gm, '').replace(/\/\*[\s\S]*?\*\//g, '');
+}
+
+test('the dead du total is gone (it was also wrong on macOS)', () => {
+  const code = stripComments(src('src/migrate.js'));
+  assert.doesNotMatch(code, /du -s/, 'the du call was dead code');
+  assert.doesNotMatch(code, /totalSize/,
+    'totalSize was computed and never printed — and summed KB as bytes on BSD du');
+  assert.match(code, /totalFiles/, 'the file count is still reported');
+});