diff --git a/src/data.js b/src/data.js
index a74166a..0678f26 100644
--- a/src/data.js
+++ b/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);
diff --git a/src/frontend/leaderboard.js b/src/frontend/leaderboard.js
index 3c779b4..416a9b9 100644
--- a/src/frontend/leaderboard.js
+++ b/src/frontend/leaderboard.js
@@ -116,61 +116,143 @@ async function loadGlobalLeaderboard() {
} catch { if (board) board.innerHTML = '
Could not load global leaderboard
'; }
}
+// 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 = '
' +
+ '
Connect GitHub
' +
+ '
Copy this code and enter it at:
' +
+ '' +
+ 'Open GitHub' +
+ // aria-live so a screen reader hears "Code expired" / "Connection error"
+ // without the user having to go hunting for the status line.
+ '
Waiting for authorization...
' +
+ '' +
+ '
';
+ 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 = '
';
- 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() {
diff --git a/src/migrate.js b/src/migrate.js
index 05f5a7b..8569f9c 100644
--- a/src/migrate.js
+++ b/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;
}
}
}
diff --git a/test/auth-modal-and-exec-safety.test.js b/test/auth-modal-and-exec-safety.test.js
new file mode 100644
index 0000000..356b0ce
--- /dev/null
+++ b/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');
+});