diff --git a/Makefile b/Makefile index 4ebcaaa..82ccc8e 100644 --- a/Makefile +++ b/Makefile @@ -2,6 +2,10 @@ deploy: ./hacks/deploy.sh +.PHONY: test +test: + node --test assets/workspace.test.js assets/crypto.test.js assets/integration.test.js + # BEGIN: lint-install . # http://github.com/codeGROOVE-dev/lint-install diff --git a/assets/README.tests.md b/assets/README.tests.md new file mode 100644 index 0000000..cd1161a --- /dev/null +++ b/assets/README.tests.md @@ -0,0 +1,90 @@ +# Automated Tests + +## Running Tests + +```bash +make test +``` + +Or directly with Node.js: + +```bash +node --test assets/workspace.test.js assets/crypto.test.js +``` + +## Test Coverage + +### `integration.test.js` +Integration tests for app initialization and token handling: + +- **State initialization**: Validates null/undefined/non-string token handling + - Handles null accessToken gracefully (prevents TypeError) + - Detects PAT token type (ghp_ prefix) + - Detects OAuth token type (gho_ prefix) + - Handles undefined, empty string, and non-string tokens + - Validates OAuth warning check at user.js:325 (regression test) + +- **Async token loading**: Validates async token decryption flow + - Initializes with null token before async load completes + - Successfully sets token after async decryption + +### `crypto.test.js` +Tests for the crypto module (`assets/crypto.js`): + +- **encryptToken() and decryptToken()**: Validates encryption/decryption + - Successfully encrypts and decrypts tokens + - Produces different ciphertext for same token (random IV) + - Fails to decrypt with wrong username + - Fails to decrypt with wrong domain + - Fails to decrypt with wrong timestamp + - Handles various GitHub token formats (PAT, OAuth, fine-grained) + - Requires all parameters for encryption/decryption + - Handles realistic login scenarios + - Different timestamps produce different encryption keys + +### `workspace.test.js` +Tests for the workspace module (`assets/workspace.js`): + +- **currentWorkspace()**: Validates workspace detection from subdomain + - Base domain returns `null` + - Reserved subdomains (www, api, etc.) return `null` + - Org subdomains return the org name + - Localhost returns `null` + +- **hiddenOrgs()**: Validates reading hidden organizations from cookies + - Returns empty array when no preferences exist + - Reads from workspace-specific cookies + - Handles both personal and org workspaces + +- **setHiddenOrgs()**: Validates setting hidden organizations + - Creates workspace-specific cookies + - Handles empty arrays + - Works for both personal and org workspaces + +- **toggleOrgVisibility()**: Validates toggling org visibility + - Adds org when not present + - Removes org when present + - Works correctly across multiple toggles + +- **initializeDefaults()**: Validates default behavior for org workspaces + - Hides personal account PRs by default in org workspaces + - Does NOT set defaults in personal workspace + - Does NOT override existing preferences + - Requires username cookie to be set + - Maintains independent preferences per workspace + +- **Integration tests**: Validates full workflow scenarios + - Users can override defaults by toggling + - Preferences persist across page reloads + - Repeated toggles work correctly (idempotency) + - No duplicate orgs from rapid clicking (regression test) + +## Test Framework + +Tests use Node.js built-in test runner (available in Node.js 18+). No external dependencies required. + +The tests create a mock DOM environment with: +- `MockDocument`: Simulates `document` with cookie storage +- `MockWindow`: Simulates `window` with location/hostname + +This allows testing browser-specific code in a Node.js environment. diff --git a/assets/app.js b/assets/app.js index 8f434a6..b424c34 100644 --- a/assets/app.js +++ b/assets/app.js @@ -13,7 +13,7 @@ const App = (() => { const state = { currentUser: null, viewingUser: null, - accessToken: Auth.getStoredToken(), + accessToken: null, // Will be loaded async in init() organizations: [], pullRequests: { incoming: [], @@ -380,7 +380,10 @@ const App = (() => { const setCookie = (name, value, days) => { const expires = new Date(); expires.setTime(expires.getTime() + days * 24 * 60 * 60 * 1000); - document.cookie = `${name}=${value};expires=${expires.toUTCString()};path=/;SameSite=Strict`; + // Use domain cookie for cross-subdomain persistence + const isSecure = window.location.protocol === "https:"; + const securePart = isSecure ? ";Secure" : ""; + document.cookie = `${name}=${value};expires=${expires.toUTCString()};path=/;domain=.ready-to-review.dev;SameSite=Lax${securePart}`; }; const handlePRAction = async (action, prId) => { @@ -422,8 +425,29 @@ const App = (() => { // Load current user const loadCurrentUser = async () => { state.currentUser = await Auth.loadCurrentUser(); + // Store username and login timestamp in cookies + if (state.currentUser && state.currentUser.login) { + // Only set if not already set (preserve original timestamp) + const existingUsername = getCookie("username"); + if (!existingUsername) { + const timestamp = Date.now().toString(); + setCookie("username", state.currentUser.login, 365); + setCookie("login_ts", timestamp, 365); + } + } }; + function getCookie(name) { + const nameEQ = `${name}=`; + const ca = document.cookie.split(";"); + for (let i = 0; i < ca.length; i++) { + let c = ca[i]; + while (c.charAt(0) === " ") c = c.substring(1, c.length); + if (c.indexOf(nameEQ) === 0) return c.substring(nameEQ.length, c.length); + } + return null; + } + // GitHub API wrapper that uses Auth module const githubAPI = async (endpoint, options = {}) => { try { @@ -640,6 +664,9 @@ const App = (() => { // Initialize const init = async () => { + // Load access token first (async now due to encryption) + state.accessToken = await Auth.getStoredToken(); + const urlParams = new URLSearchParams(window.location.search); const demo = urlParams.get("demo"); const urlContext = parseURL(); @@ -677,7 +704,7 @@ const App = (() => { // Handle changelog page routing if (urlContext && urlContext.isChangelog) { updateSearchInputVisibility(); - const token = Auth.getStoredToken(); + const token = await Auth.getStoredToken(); if (token) { try { await loadCurrentUser(); @@ -721,7 +748,7 @@ const App = (() => { const path = window.location.pathname; if (path === "/notifications" || path.match(/^\/notifications\/gh\/[^/]+$/)) { updateSearchInputVisibility(); - const token = Auth.getStoredToken(); + const token = await Auth.getStoredToken(); if (token) { try { await loadCurrentUser(); @@ -738,7 +765,7 @@ const App = (() => { // Handle robots page routing if (path === "/robots" || path.match(/^\/robots\/gh\/[^/]+$/)) { updateSearchInputVisibility(); - const token = Auth.getStoredToken(); + const token = await Auth.getStoredToken(); if (!token) { showToast("Please login to configure Robot Army", "error"); window.location.href = "/"; @@ -855,7 +882,7 @@ const App = (() => { const authCodeExchanged = await Auth.handleAuthCodeCallback(); // Re-check for authentication token (it might have been set after module load or auth code exchange) - state.accessToken = Auth.getStoredToken(); + state.accessToken = await Auth.getStoredToken(); console.log("[App.init] Checked for access token:", state.accessToken ? "found" : "not found"); // If we just exchanged an auth code successfully, reload to start with fresh state @@ -906,6 +933,9 @@ const App = (() => { updateSearchInputVisibility(); await loadCurrentUser(); + // Initialize workspace defaults after user is loaded + Workspace.initializeDefaults(); + // If at root URL, redirect to user's page if (!urlContext && state.currentUser) { window.location.href = `/u/${state.currentUser.login}`; diff --git a/assets/auth.js b/assets/auth.js index 8c7b147..5f85830 100644 --- a/assets/auth.js +++ b/assets/auth.js @@ -1,6 +1,8 @@ // Authentication Module for Ready To Review console.log("[Auth Module] Loading..."); +import { Crypto } from "./crypto.js"; + // Log URL parameters on page load for debugging OAuth flow (without exposing secrets) const urlParams = new URLSearchParams(window.location.search); if (urlParams.has("code") || urlParams.has("state") || urlParams.has("error")) { @@ -18,8 +20,7 @@ export const Auth = (() => { const CONFIG = { CLIENT_ID: "Iv23liYmAKkBpvhHAnQQ", API_BASE: "https://api.github.com", - STORAGE_KEY: "github_token", // Legacy localStorage key (fallback) - COOKIE_KEY: "github_token", // Store all tokens in cookies for cross-subdomain support + COOKIE_KEY: "access_token", // Encrypted token in cookies OAUTH_REDIRECT_URI: "https://auth.ready-to-review.dev/oauth/callback", }; @@ -27,11 +28,21 @@ export const Auth = (() => { function setCookie(name, value, days) { const expires = new Date(); expires.setTime(expires.getTime() + days * 24 * 60 * 60 * 1000); + + // SECURITY: Always require HTTPS for cookies containing sensitive data + // Fail loudly on HTTP to catch development/deployment issues early + if (window.location.protocol !== "https:") { + console.error("[SECURITY] Cannot set cookie over insecure HTTP connection"); + console.error("[SECURITY] Cookie name:", name); + console.error("[SECURITY] Current protocol:", window.location.protocol); + throw new Error("Cookies can only be set over HTTPS for security"); + } + // Use domain cookie to share across all subdomains of ready-to-review.dev - // SameSite=Lax allows cookies to be sent on top-level navigation (OAuth redirects) - const isSecure = window.location.protocol === "https:"; - const securePart = isSecure ? ";Secure" : ""; - document.cookie = `${name}=${value};expires=${expires.toUTCString()};path=/;domain=.ready-to-review.dev;SameSite=Lax${securePart}`; + // SameSite=Lax: Required for OAuth callback (cross-site navigation with state cookie) + // Secure: Always set - enforced by protocol check above + // HttpOnly: Cannot use - JavaScript needs to read/decrypt tokens + document.cookie = `${name}=${value};expires=${expires.toUTCString()};path=/;domain=.ready-to-review.dev;SameSite=Lax;Secure`; } function getCookie(name) { @@ -50,35 +61,47 @@ export const Auth = (() => { document.cookie = `${name}=;expires=Thu, 01 Jan 1970 00:00:00 UTC;path=/;domain=.ready-to-review.dev;`; } - const getStoredToken = () => { - // Always check cookie first (works across subdomains) - const cookieToken = getCookie(CONFIG.COOKIE_KEY); - if (cookieToken) return cookieToken; - - // Fallback to localStorage for legacy tokens, then migrate to cookie - const localToken = localStorage.getItem(CONFIG.STORAGE_KEY); - if (localToken) { - console.log("[Auth] Migrating token from localStorage to cookie for cross-subdomain support"); - setCookie(CONFIG.COOKIE_KEY, localToken, 10); - localStorage.removeItem(CONFIG.STORAGE_KEY); - return localToken; + // Get encryption context (username, domain, and timestamp) + const getEncryptionContext = () => { + const username = getCookie("username"); + const timestamp = getCookie("login_ts"); + const domain = window.location.hostname.split(".").slice(-2).join("."); // Get base domain + + return { username, domain, timestamp }; + }; + + const getStoredToken = async () => { + const { username, domain, timestamp } = getEncryptionContext(); + + const encryptedToken = getCookie(CONFIG.COOKIE_KEY); + if (!encryptedToken || !username || !domain || !timestamp) { + return null; } - return null; + try { + console.log("[Auth] Decrypting stored token"); + return await Crypto.decryptToken(encryptedToken, username, domain, timestamp); + } catch (error) { + console.error("[Auth] Failed to decrypt token, clearing invalid token:", error); + deleteCookie(CONFIG.COOKIE_KEY); + return null; + } }; - const storeToken = (token) => { - // Always store in cookie for cross-subdomain support (10 days) - setCookie(CONFIG.COOKIE_KEY, token, 10); - // Clean up any legacy localStorage token - localStorage.removeItem(CONFIG.STORAGE_KEY); + const storeToken = async (token) => { + const { username, domain, timestamp } = getEncryptionContext(); + + if (!username || !domain || !timestamp) { + throw new Error("Cannot encrypt token: username, domain, or timestamp missing"); + } + + console.log("[Auth] Encrypting token for storage"); + const encrypted = await Crypto.encryptToken(token, username, domain, timestamp); + setCookie(CONFIG.COOKIE_KEY, encrypted, 10); }; const clearToken = () => { - // Clear cookie (primary storage) deleteCookie(CONFIG.COOKIE_KEY); - // Clear localStorage (legacy) - localStorage.removeItem(CONFIG.STORAGE_KEY); }; const initiateOAuthLogin = () => { @@ -196,7 +219,7 @@ export const Auth = (() => { }); if (response.ok) { - storeToken(token); // Store in cookie + await storeToken(token); // Store encrypted token in cookie closePATModal(); window.location.reload(); } else { @@ -244,8 +267,13 @@ export const Auth = (() => { const data = await response.json(); - // Store token in localStorage - storeToken(data.token); + // Store username and login timestamp (milliseconds since 1970) + const timestamp = Date.now().toString(); + setCookie("username", data.username, 365); + setCookie("login_ts", timestamp, 365); + + // Store encrypted token + await storeToken(data.token); console.log("[Auth] Successfully exchanged auth code for token, user:", data.username); @@ -279,7 +307,7 @@ export const Auth = (() => { ...options.headers, }; - const token = getStoredToken(); + const token = await getStoredToken(); if (token) { headers.Authorization = `token ${token}`; } @@ -350,7 +378,7 @@ export const Auth = (() => { // GraphQL API function with retry logic const githubGraphQL = async (query, variables = {}, retries = 5) => { - const token = getStoredToken(); + const token = await getStoredToken(); if (!token) { throw new Error("No authentication token available"); } diff --git a/assets/crypto.js b/assets/crypto.js new file mode 100644 index 0000000..3958259 --- /dev/null +++ b/assets/crypto.js @@ -0,0 +1,112 @@ +// Crypto Module for Ready To Review +// Encrypts sensitive data like GitHub tokens to prevent them from appearing raw on disk/memory +console.log("[Crypto Module] Loading..."); + +export const Crypto = (() => { + console.log("[Crypto Module] Initializing..."); + + // Derive encryption key from username, domain, and timestamp + // SECURITY: Uses PBKDF2 with 100,000 iterations to prevent brute-force attacks + // Even if attacker captures encrypted cookie, they cannot feasibly brute-force timestamp + const deriveKey = async (username, domain, timestamp) => { + const keyMaterial = `${username}@${domain}:${timestamp}`; + const encoder = new TextEncoder(); + const data = encoder.encode(keyMaterial); + + // Import raw key material for PBKDF2 + const baseKey = await crypto.subtle.importKey("raw", data, "PBKDF2", false, ["deriveKey"]); + + // Use domain as salt (public but unique per deployment) + // Salt doesn't need to be secret, just unique + const salt = encoder.encode(domain); + + // Derive AES key using PBKDF2 with 100,000 iterations + // This makes each key derivation take ~50ms, making brute-force infeasible + // Nation-state attackers would need years to test all timestamps for a single year + return await crypto.subtle.deriveKey( + { + name: "PBKDF2", + salt: salt, + iterations: 100000, + hash: "SHA-256", + }, + baseKey, + { name: "AES-GCM", length: 256 }, + false, + ["encrypt", "decrypt"] + ); + }; + + // Encrypt a token + const encryptToken = async (token, username, domain, timestamp) => { + if (!token || !username || !domain || !timestamp) { + throw new Error("Token, username, domain, and timestamp are required for encryption"); + } + + try { + const key = await deriveKey(username, domain, timestamp); + const encoder = new TextEncoder(); + const data = encoder.encode(token); + + // Generate random IV (initialization vector) + const iv = crypto.getRandomValues(new Uint8Array(12)); + + // Encrypt + const encrypted = await crypto.subtle.encrypt({ name: "AES-GCM", iv: iv }, key, data); + + // Combine IV + encrypted data and encode as base64 + const combined = new Uint8Array(iv.length + encrypted.byteLength); + combined.set(iv, 0); + combined.set(new Uint8Array(encrypted), iv.length); + + // Convert to base64 for cookie storage + return btoa(String.fromCharCode(...combined)); + } catch (error) { + console.error("[Crypto] Encryption failed:", error); + throw error; + } + }; + + // Decrypt a token + const decryptToken = async (encryptedToken, username, domain, timestamp) => { + if (!encryptedToken || !username || !domain || !timestamp) { + throw new Error( + "Encrypted token, username, domain, and timestamp are required for decryption" + ); + } + + try { + const key = await deriveKey(username, domain, timestamp); + + // Decode base64 + const combined = Uint8Array.from(atob(encryptedToken), (c) => c.charCodeAt(0)); + + // Extract IV (first 12 bytes) and encrypted data + const iv = combined.slice(0, 12); + const encryptedData = combined.slice(12); + + // Decrypt + const decrypted = await crypto.subtle.decrypt( + { name: "AES-GCM", iv: iv }, + key, + encryptedData + ); + + // Convert back to string + const decoder = new TextDecoder(); + return decoder.decode(decrypted); + } catch (error) { + console.error("[Crypto] Decryption failed:", error); + throw error; + } + }; + + console.log("[Crypto Module] Exporting functions..."); + const cryptoExports = { + encryptToken, + decryptToken, + }; + console.log("[Crypto Module] Exports:", cryptoExports); + return cryptoExports; +})(); +console.log("[Crypto Module] Module loaded, Crypto object:", Crypto); diff --git a/assets/crypto.test.js b/assets/crypto.test.js new file mode 100644 index 0000000..42271f4 --- /dev/null +++ b/assets/crypto.test.js @@ -0,0 +1,223 @@ +// Crypto module tests +// Run with: node --test assets/crypto.test.js + +import { describe, it } from "node:test"; +import assert from "node:assert"; + +// crypto is already available in Node.js global scope (no polyfill needed) + +// Import crypto module +import { Crypto } from "./crypto.js"; + +describe("Crypto Module", () => { + describe("encryptToken() and decryptToken()", () => { + it("should encrypt and decrypt a token successfully", async () => { + const token = "ghp_1234567890abcdefghijklmnopqrstuvwxyz"; + const username = "testuser"; + const domain = "ready-to-review.dev"; + const timestamp = "1699564800000"; + + // Encrypt + const encrypted = await Crypto.encryptToken(token, username, domain, timestamp); + + // Should be base64 encoded + assert.ok(encrypted.length > 0); + assert.ok(typeof encrypted === "string"); + assert.notStrictEqual(encrypted, token, "Encrypted token should not match plaintext"); + + // Decrypt + const decrypted = await Crypto.decryptToken(encrypted, username, domain, timestamp); + + // Should match original + assert.strictEqual(decrypted, token, "Decrypted token should match original"); + }); + + it("should produce different ciphertext for same token (random IV)", async () => { + const token = "ghp_1234567890abcdefghijklmnopqrstuvwxyz"; + const username = "testuser"; + const domain = "ready-to-review.dev"; + const timestamp = "1699564800000"; + + const encrypted1 = await Crypto.encryptToken(token, username, domain, timestamp); + const encrypted2 = await Crypto.encryptToken(token, username, domain, timestamp); + + // Should be different due to random IV + assert.notStrictEqual(encrypted1, encrypted2, "Each encryption should produce different ciphertext"); + + // But both should decrypt to same value + const decrypted1 = await Crypto.decryptToken(encrypted1, username, domain, timestamp); + const decrypted2 = await Crypto.decryptToken(encrypted2, username, domain, timestamp); + assert.strictEqual(decrypted1, token); + assert.strictEqual(decrypted2, token); + }); + + it("should fail to decrypt with wrong username", async () => { + const token = "ghp_1234567890abcdefghijklmnopqrstuvwxyz"; + const username = "testuser"; + const domain = "ready-to-review.dev"; + const timestamp = "1699564800000"; + + const encrypted = await Crypto.encryptToken(token, username, domain, timestamp); + + // Try to decrypt with different username - should throw + await assert.rejects(async () => { + await Crypto.decryptToken(encrypted, "wronguser", domain, timestamp); + }); + }); + + it("should fail to decrypt with wrong domain", async () => { + const token = "ghp_1234567890abcdefghijklmnopqrstuvwxyz"; + const username = "testuser"; + const domain = "ready-to-review.dev"; + const timestamp = "1699564800000"; + + const encrypted = await Crypto.encryptToken(token, username, domain, timestamp); + + // Try to decrypt with different domain - should throw + await assert.rejects(async () => { + await Crypto.decryptToken(encrypted, username, "wrong-domain.com", timestamp); + }); + }); + + it("should fail to decrypt with wrong timestamp", async () => { + const token = "ghp_1234567890abcdefghijklmnopqrstuvwxyz"; + const username = "testuser"; + const domain = "ready-to-review.dev"; + const timestamp = "1699564800000"; + + const encrypted = await Crypto.encryptToken(token, username, domain, timestamp); + + // Try to decrypt with different timestamp - should throw + await assert.rejects(async () => { + await Crypto.decryptToken(encrypted, username, domain, "1699564800001"); + }); + }); + + it("should handle various token formats", async () => { + const tokens = [ + "ghp_1234567890abcdefghijklmnopqrstuvwxyz", // PAT + "gho_1234567890abcdefghijklmnopqrstuvwxyz", // OAuth + "github_pat_11ABCDEFGHIJKLMNOPQRSTUVWXYZ123456789012345678901234567890", // Fine-grained PAT + ]; + const username = "testuser"; + const domain = "ready-to-review.dev"; + const timestamp = "1699564800000"; + + for (const token of tokens) { + const encrypted = await Crypto.encryptToken(token, username, domain, timestamp); + const decrypted = await Crypto.decryptToken(encrypted, username, domain, timestamp); + assert.strictEqual(decrypted, token, `Should handle token format: ${token.substring(0, 10)}...`); + } + }); + + it("should require all parameters for encryption", async () => { + await assert.rejects( + async () => { + await Crypto.encryptToken("token", null, "domain", "timestamp"); + }, + { + message: /Token, username, domain, and timestamp are required/, + } + ); + + await assert.rejects( + async () => { + await Crypto.encryptToken("token", "user", null, "timestamp"); + }, + { + message: /Token, username, domain, and timestamp are required/, + } + ); + + await assert.rejects( + async () => { + await Crypto.encryptToken("token", "user", "domain", null); + }, + { + message: /Token, username, domain, and timestamp are required/, + } + ); + }); + + it("should require all parameters for decryption", async () => { + await assert.rejects( + async () => { + await Crypto.decryptToken("encrypted", null, "domain", "timestamp"); + }, + { + message: /Encrypted token, username, domain, and timestamp are required/, + } + ); + + await assert.rejects( + async () => { + await Crypto.decryptToken("encrypted", "user", null, "timestamp"); + }, + { + message: /Encrypted token, username, domain, and timestamp are required/, + } + ); + + await assert.rejects( + async () => { + await Crypto.decryptToken("encrypted", "user", "domain", null); + }, + { + message: /Encrypted token, username, domain, and timestamp are required/, + } + ); + }); + + it("should handle realistic GitHub token scenario", async () => { + // Simulate a real login flow + const githubToken = "ghp_vT3xR9pL2kQ8mN4bV7wC1yZ5sA6fD0hJ"; + const username = "tstromberg"; + const domain = "ready-to-review.dev"; + const loginTimestamp = Date.now().toString(); + + // User logs in - token gets encrypted + const encryptedToken = await Crypto.encryptToken( + githubToken, + username, + domain, + loginTimestamp + ); + + // Store in cookie (simulated) + const cookieValue = encryptedToken; + + // Later, app needs to make API call - decrypt token + const decryptedToken = await Crypto.decryptToken( + cookieValue, + username, + domain, + loginTimestamp + ); + + // Should be able to use decrypted token + assert.strictEqual(decryptedToken, githubToken); + assert.ok(decryptedToken.startsWith("ghp_")); + }); + + it("should produce different keys for different timestamps", async () => { + const token = "ghp_test123"; + const username = "testuser"; + const domain = "ready-to-review.dev"; + + const ts1 = "1699564800000"; + const ts2 = "1699564800001"; // 1 millisecond later + + const encrypted1 = await Crypto.encryptToken(token, username, domain, ts1); + const encrypted2 = await Crypto.encryptToken(token, username, domain, ts2); + + // Can decrypt with correct timestamp + const decrypted1 = await Crypto.decryptToken(encrypted1, username, domain, ts1); + assert.strictEqual(decrypted1, token); + + // Cannot decrypt ts1's token with ts2's timestamp + await assert.rejects(async () => { + await Crypto.decryptToken(encrypted1, username, domain, ts2); + }); + }); + }); +}); diff --git a/assets/integration.test.js b/assets/integration.test.js new file mode 100644 index 0000000..c290697 --- /dev/null +++ b/assets/integration.test.js @@ -0,0 +1,482 @@ +// Integration tests for app initialization and token handling +// Run with: node --test assets/integration.test.js + +import assert from "node:assert"; +import { describe, it } from "node:test"; + +describe("App Integration", () => { + describe("State initialization", () => { + it("should handle null accessToken gracefully", () => { + // Simulate app state with null token (before async load completes) + const state = { + currentUser: null, + viewingUser: null, + accessToken: null, + organizations: [], + pullRequests: { incoming: [], outgoing: [] }, + isDemoMode: false, + }; + + // This simulates the logging code in user.js:292 + // Should not throw when accessToken is null + assert.doesNotThrow(() => { + const authType = + state.accessToken && typeof state.accessToken === "string" + ? state.accessToken.startsWith("ghp_") + ? "PAT" + : "OAuth" + : "none"; + assert.strictEqual(authType, "none"); + }); + }); + + it("should detect PAT token type", () => { + const state = { + accessToken: "ghp_1234567890abcdefghijklmnopqrstuvwxyz", + }; + + const authType = + state.accessToken && typeof state.accessToken === "string" + ? state.accessToken.startsWith("ghp_") + ? "PAT" + : "OAuth" + : "none"; + + assert.strictEqual(authType, "PAT"); + }); + + it("should detect OAuth token type", () => { + const state = { + accessToken: "gho_1234567890abcdefghijklmnopqrstuvwxyz", + }; + + const authType = + state.accessToken && typeof state.accessToken === "string" + ? state.accessToken.startsWith("ghp_") + ? "PAT" + : "OAuth" + : "none"; + + assert.strictEqual(authType, "OAuth"); + }); + + it("should handle undefined accessToken", () => { + const state = { + accessToken: undefined, + }; + + assert.doesNotThrow(() => { + const authType = + state.accessToken && typeof state.accessToken === "string" + ? state.accessToken.startsWith("ghp_") + ? "PAT" + : "OAuth" + : "none"; + assert.strictEqual(authType, "none"); + }); + }); + + it("should handle empty string accessToken", () => { + const state = { + accessToken: "", + }; + + assert.doesNotThrow(() => { + const authType = + state.accessToken && typeof state.accessToken === "string" + ? state.accessToken.startsWith("ghp_") + ? "PAT" + : "OAuth" + : "none"; + assert.strictEqual(authType, "none"); + }); + }); + + it("should handle non-string accessToken gracefully", () => { + const invalidTokens = [123, true, {}, [], NaN]; + + for (const invalidToken of invalidTokens) { + const state = { accessToken: invalidToken }; + + assert.doesNotThrow( + () => { + const authType = + state.accessToken && typeof state.accessToken === "string" + ? state.accessToken.startsWith("ghp_") + ? "PAT" + : "OAuth" + : "none"; + assert.strictEqual(authType, "none"); + }, + `Should handle ${typeof invalidToken} without throwing` + ); + } + }); + + it("should handle token type check for OAuth warning (user.js:325)", () => { + // This is the actual code from user.js:325 that was failing + const state = { accessToken: null }; + const totalCount = 100; + const allPRsLength = 50; + + assert.doesNotThrow(() => { + // This should not throw even with null token + if ( + state.accessToken && + typeof state.accessToken === "string" && + !state.accessToken.startsWith("ghp_") && + totalCount > allPRsLength + ) { + // Would show OAuth warning + } + }, "Should handle null token in OAuth warning check"); + + // Test with OAuth token - should enter the condition + state.accessToken = "gho_1234567890abcdefghijklmnopqrstuvwxyz"; + let warningTriggered = false; + if ( + state.accessToken && + typeof state.accessToken === "string" && + !state.accessToken.startsWith("ghp_") && + totalCount > allPRsLength + ) { + warningTriggered = true; + } + assert.strictEqual(warningTriggered, true, "Should trigger OAuth warning for OAuth tokens"); + + // Test with PAT token - should NOT enter the condition + state.accessToken = "ghp_1234567890abcdefghijklmnopqrstuvwxyz"; + warningTriggered = false; + if ( + state.accessToken && + typeof state.accessToken === "string" && + !state.accessToken.startsWith("ghp_") && + totalCount > allPRsLength + ) { + warningTriggered = true; + } + assert.strictEqual( + warningTriggered, + false, + "Should NOT trigger OAuth warning for PAT tokens" + ); + }); + }); + + describe("Async token loading", () => { + it("should initialize with null token before async load", () => { + // This is how the app initializes now + const state = { + accessToken: null, // Will be loaded async in init() + }; + + assert.strictEqual(state.accessToken, null); + }); + + it("should be able to set token after async load", async () => { + const state = { + accessToken: null, + }; + + // Simulate async token loading + await new Promise((resolve) => { + setTimeout(() => { + state.accessToken = "ghp_test123"; + resolve(); + }, 0); + }); + + assert.strictEqual(state.accessToken, "ghp_test123"); + }); + }); + + describe("buildWaitingOn", () => { + const escapeHtml = (text) => { + const map = { + "&": "&", + "<": "<", + ">": ">", + '"': """, + "'": "'", + }; + return text.replace(/[&<>"']/g, (m) => map[m]); + }; + + const buildWaitingOn = (pr, viewingUser, currentUser) => { + if (!pr.turnData?.analysis?.next_action) { + return ""; + } + + const nextAction = pr.turnData.analysis.next_action; + const usernames = Object.keys(nextAction); + + if (usernames.length === 0) { + return ""; + } + + // Group actions by kind + const actionsByKind = {}; + usernames.forEach((username) => { + const action = nextAction[username]; + const kind = action.kind || "action"; + + if (!actionsByKind[kind]) { + actionsByKind[kind] = []; + } + + const isViewingUser = viewingUser && username === viewingUser.login; + const isCurrentUser = currentUser && username === currentUser.login; + + let displayName = username; + let className = "pr-action-user"; + + // Special handling for _system + if (username === "_system") { + // Skip adding _system to the user list, we'll just show the action name + return; + } + + // If viewing someone else's dashboard, highlight their name + if (viewingUser && currentUser && viewingUser.login !== currentUser.login) { + if (isViewingUser) { + displayName = viewingUser.login; + className = "pr-action-you"; + } + } else { + // Normal behavior when viewing your own dashboard + if (isCurrentUser) { + displayName = "YOU"; + className = "pr-action-you"; + } + } + + const title = action.reason || "Waiting for action"; + actionsByKind[kind].push({ + html: `${escapeHtml(displayName)}`, + isYou: isCurrentUser || isViewingUser, + }); + }); + + // Build the action groups string + const actionGroups = Object.entries(actionsByKind) + .map(([kind, users]) => { + // Format action kind (replace underscores with spaces) + const actionName = kind.replace(/_/g, " "); + + if (users.length === 0) { + // _system action with no users + return actionName; + } + + // Join user names + const userList = users.map((u) => u.html).join(", "); + return `${actionName}: ${userList}`; + }) + .join("; "); + + return ` → ${actionGroups}`; + }; + + it("should return empty string when no turnData", () => { + const pr = {}; + const result = buildWaitingOn(pr, null, null); + assert.strictEqual(result, ""); + }); + + it("should return empty string when no next_action", () => { + const pr = { turnData: { analysis: {} } }; + const result = buildWaitingOn(pr, null, null); + assert.strictEqual(result, ""); + }); + + it("should return empty string when next_action is empty", () => { + const pr = { turnData: { analysis: { next_action: {} } } }; + const result = buildWaitingOn(pr, null, null); + assert.strictEqual(result, ""); + }); + + it("should show single review action for YOU", () => { + const pr = { + turnData: { + analysis: { + next_action: { + demo: { kind: "review", reason: "Please review this PR", critical: true }, + }, + }, + }, + }; + const currentUser = { login: "demo" }; + const result = buildWaitingOn(pr, currentUser, currentUser); + + assert.ok(result.includes("→"), "Should start with arrow"); + assert.ok(result.includes("review:"), "Should show action name"); + assert.ok(result.includes('pr-action-you'), "Should use YOU class for current user"); + assert.ok(result.includes("YOU"), "Should show YOU for current user"); + }); + + it("should group multiple users under same action", () => { + const pr = { + turnData: { + analysis: { + next_action: { + demo: { kind: "review", reason: "Please review" }, + alice: { kind: "review", reason: "Alice needs to review" }, + bob: { kind: "review", reason: "Bob needs to review" }, + }, + }, + }, + }; + const currentUser = { login: "demo" }; + const result = buildWaitingOn(pr, currentUser, currentUser); + + assert.ok(result.includes("review:"), "Should show review action"); + assert.ok(result.includes("YOU"), "Should show YOU"); + assert.ok(result.includes("alice"), "Should show alice"); + assert.ok(result.includes("bob"), "Should show bob"); + assert.ok(!result.includes(";"), "Should not have semicolon for single action type"); + }); + + it("should separate different action types with semicolons", () => { + const pr = { + turnData: { + analysis: { + next_action: { + demo: { kind: "review", reason: "Please review" }, + alice: { kind: "merge", reason: "Alice needs to merge" }, + bob: { kind: "fix_tests", reason: "Bob needs to fix tests" }, + }, + }, + }, + }; + const currentUser = { login: "demo" }; + const result = buildWaitingOn(pr, currentUser, currentUser); + + assert.ok(result.includes(";"), "Should have semicolons separating actions"); + assert.ok(result.includes("review:"), "Should show review action"); + assert.ok(result.includes("merge:"), "Should show merge action"); + assert.ok(result.includes("fix tests:"), "Should show fix tests with space"); + }); + + it("should handle _system user by showing only action name", () => { + const pr = { + turnData: { + analysis: { + next_action: { + _system: { kind: "fix_tests", reason: "Tests are failing" }, + }, + }, + }, + }; + const currentUser = { login: "demo" }; + const result = buildWaitingOn(pr, currentUser, currentUser); + + assert.ok(result.includes("fix tests"), "Should show action name"); + assert.ok(!result.includes("_system"), "Should not show _system username"); + assert.ok(!result.includes(":"), "Should not have colon when only _system"); + }); + + it("should replace underscores with spaces in action names", () => { + const pr = { + turnData: { + analysis: { + next_action: { + demo: { kind: "fix_merge_conflict", reason: "Fix conflict" }, + }, + }, + }, + }; + const currentUser = { login: "demo" }; + const result = buildWaitingOn(pr, currentUser, currentUser); + + assert.ok(result.includes("fix merge conflict:"), "Should replace underscores with spaces"); + assert.ok(!result.includes("fix_merge_conflict"), "Should not contain underscores"); + }); + + it("should handle mixed actions including _system", () => { + const pr = { + turnData: { + analysis: { + next_action: { + demo: { kind: "review", reason: "Please review" }, + alice: { kind: "add_comment", reason: "Alice needs to comment" }, + _system: { kind: "run_tests", reason: "Tests running" }, + }, + }, + }, + }; + const currentUser = { login: "demo" }; + const result = buildWaitingOn(pr, currentUser, currentUser); + + assert.ok(result.includes("review: "), "Should show review with users"); + assert.ok(result.includes("add comment: "), "Should show add comment"); + assert.ok(result.includes("run tests"), "Should show run tests from _system"); + assert.ok(!result.includes("_system"), "Should not show _system username"); + }); + + it("should escape HTML in usernames and reasons", () => { + const pr = { + turnData: { + analysis: { + next_action: { + 'user