Skip to content

Commit 56040d6

Browse files
committed
More CSRF/XSS work
1 parent 9a45e49 commit 56040d6

10 files changed

Lines changed: 358 additions & 187 deletions

File tree

assets/app.js

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,7 @@ const App = (() => {
608608
return;
609609
}
610610

611-
User.updateUserDisplay(state, initiateLogin);
611+
User.updateUserDisplay(state, initiateLogin, logout);
612612
User.updateOrgFilter(state, parseURL, githubAPI);
613613
showMainContentWithLoading();
614614

@@ -684,7 +684,7 @@ const App = (() => {
684684
if (urlContext && urlContext.isStats) {
685685
updateSearchInputVisibility();
686686
await Stats.showStatsPage(state, githubAPI, loadCurrentUser,
687-
() => User.updateUserDisplay(state, initiateLogin),
687+
() => User.updateUserDisplay(state, initiateLogin, logout),
688688
setupHamburgerMenu,
689689
() => User.updateOrgFilter(state, parseURL, githubAPI),
690690
handleOrgChange, handleSearch, parseURL, User.loadUserOrganizations);
@@ -698,7 +698,7 @@ const App = (() => {
698698
if (token) {
699699
try {
700700
await loadCurrentUser();
701-
User.updateUserDisplay(state, initiateLogin);
701+
User.updateUserDisplay(state, initiateLogin, logout);
702702
setupHamburgerMenu();
703703
await User.updateOrgFilter(state, parseURL, githubAPI);
704704

@@ -725,7 +725,7 @@ const App = (() => {
725725
if (urlContext && urlContext.isLeaderboard) {
726726
updateSearchInputVisibility();
727727
await Leaderboard.showLeaderboardPage(state, githubAPI, loadCurrentUser,
728-
() => User.updateUserDisplay(state, initiateLogin),
728+
() => User.updateUserDisplay(state, initiateLogin, logout),
729729
setupHamburgerMenu,
730730
() => User.updateOrgFilter(state, parseURL, githubAPI),
731731
handleOrgChange, handleSearch, parseURL, User.loadUserOrganizations);
@@ -739,7 +739,7 @@ const App = (() => {
739739
if (token) {
740740
try {
741741
await loadCurrentUser();
742-
User.updateUserDisplay(state, initiateLogin);
742+
User.updateUserDisplay(state, initiateLogin, logout);
743743
setupHamburgerMenu();
744744
} catch (error) {
745745
console.error("Failed to load user for notifications:", error);
@@ -770,7 +770,7 @@ const App = (() => {
770770
}
771771
}
772772

773-
User.updateUserDisplay(state, initiateLogin);
773+
User.updateUserDisplay(state, initiateLogin, logout);
774774

775775
// Always update org filter to ensure dropdown is populated
776776
await User.updateOrgFilter(state, parseURL, githubAPI);
@@ -815,8 +815,32 @@ const App = (() => {
815815
await handlePRAction(action, prId);
816816
});
817817

818-
// Note: Modal interactions are handled via inline onclick handlers in HTML
819-
// No additional event listeners needed here
818+
// Modal event listeners
819+
const githubAppLoginBtn = $("githubAppLoginBtn");
820+
const patLoginBtn = $("patLoginBtn");
821+
const githubAppModalBackdrop = $("githubAppModalBackdrop");
822+
const closeGitHubAppModalBtn = $("closeGitHubAppModalBtn");
823+
const cancelGitHubAppBtn = $("cancelGitHubAppBtn");
824+
const proceedWithOAuthBtn = $("proceedWithOAuthBtn");
825+
const patModalBackdrop = $("patModalBackdrop");
826+
const closePATModalBtn = $("closePATModalBtn");
827+
const submitPATBtn = $("submitPATBtn");
828+
const yamlModalBackdrop = $("yamlModalBackdrop");
829+
const closeYAMLModalBtn = $("closeYAMLModalBtn");
830+
const copyYamlBtn = $("copyYaml");
831+
832+
if (githubAppLoginBtn) githubAppLoginBtn.addEventListener("click", showGitHubAppModal);
833+
if (patLoginBtn) patLoginBtn.addEventListener("click", initiatePATLogin);
834+
if (githubAppModalBackdrop) githubAppModalBackdrop.addEventListener("click", closeGitHubAppModal);
835+
if (closeGitHubAppModalBtn) closeGitHubAppModalBtn.addEventListener("click", closeGitHubAppModal);
836+
if (cancelGitHubAppBtn) cancelGitHubAppBtn.addEventListener("click", closeGitHubAppModal);
837+
if (proceedWithOAuthBtn) proceedWithOAuthBtn.addEventListener("click", proceedWithOAuth);
838+
if (patModalBackdrop) patModalBackdrop.addEventListener("click", closePATModal);
839+
if (closePATModalBtn) closePATModalBtn.addEventListener("click", closePATModal);
840+
if (submitPATBtn) submitPATBtn.addEventListener("click", submitPAT);
841+
if (yamlModalBackdrop) yamlModalBackdrop.addEventListener("click", closeYAMLModal);
842+
if (closeYAMLModalBtn) closeYAMLModalBtn.addEventListener("click", closeYAMLModal);
843+
if (copyYamlBtn) copyYamlBtn.addEventListener("click", copyYAML);
820844

821845
if ($("patInput")) {
822846
$("patInput").addEventListener("keypress", (e) => {
@@ -866,7 +890,7 @@ const App = (() => {
866890
state.viewingUser = await githubAPI(`/users/${urlContext.username}`);
867891

868892
showLoginPrompt();
869-
User.updateUserDisplay(state, initiateLogin);
893+
User.updateUserDisplay(state, initiateLogin, logout);
870894

871895
// Load public data
872896
await User.updateOrgFilter(state, parseURL, githubAPI);
@@ -909,7 +933,7 @@ const App = (() => {
909933
}
910934
}
911935

912-
User.updateUserDisplay(state, initiateLogin);
936+
User.updateUserDisplay(state, initiateLogin, logout);
913937
await User.updateOrgFilter(state, parseURL, githubAPI);
914938

915939
// Only load PRs if we're on the PR dashboard page

assets/auth.js

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
// Authentication Module for Ready To Review
22
console.log('[Auth Module] Loading...');
33

4-
// Log URL parameters on page load for debugging OAuth flow
4+
// Log URL parameters on page load for debugging OAuth flow (without exposing secrets)
55
const urlParams = new URLSearchParams(window.location.search);
66
if (urlParams.has('code') || urlParams.has('state') || urlParams.has('error')) {
77
console.log('[Auth] OAuth callback detected!');
8-
console.log('[Auth] URL:', window.location.href);
9-
console.log('[Auth] Code:', urlParams.get('code') ? 'present (length=' + urlParams.get('code').length + ')' : 'missing');
10-
console.log('[Auth] State:', urlParams.get('state') ? 'present' : 'missing');
11-
console.log('[Auth] Error:', urlParams.get('error'));
12-
console.log('[Auth] Error description:', urlParams.get('error_description'));
8+
console.log('[Auth] Code:', urlParams.has('code') ? 'present' : 'missing');
9+
console.log('[Auth] State:', urlParams.has('state') ? 'present' : 'missing');
10+
console.log('[Auth] Error:', urlParams.get('error')); // Error codes are not secret
11+
console.log('[Auth] Error description:', urlParams.get('error_description')); // GitHub's public error messages
1312
}
1413

1514
export const Auth = (() => {

assets/robots.js

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -755,15 +755,27 @@ export const Robots = (() => {
755755

756756
if (!config) return;
757757

758-
const mappingHtml = `
759-
<div class="robot-mapping" id="${mappingId}">
760-
<input type="text" placeholder="${config.placeholder1}">
761-
<input type="text" placeholder="${config.placeholder2}">
762-
<button onclick="window.App.removeMapping('${mappingId}')" aria-label="Remove mapping"></button>
763-
</div>
764-
`;
758+
const mappingDiv = document.createElement('div');
759+
mappingDiv.className = 'robot-mapping';
760+
mappingDiv.id = mappingId;
761+
762+
const input1 = document.createElement('input');
763+
input1.type = 'text';
764+
input1.placeholder = config.placeholder1;
765+
766+
const input2 = document.createElement('input');
767+
input2.type = 'text';
768+
input2.placeholder = config.placeholder2;
769+
770+
const button = document.createElement('button');
771+
button.setAttribute('aria-label', 'Remove mapping');
772+
button.addEventListener('click', () => removeMapping(mappingId));
773+
774+
mappingDiv.appendChild(input1);
775+
mappingDiv.appendChild(input2);
776+
mappingDiv.appendChild(button);
765777

766-
container.insertAdjacentHTML("beforeend", mappingHtml);
778+
container.appendChild(mappingDiv);
767779
};
768780

769781
const removeMapping = (mappingId) => {

assets/stats.js

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -477,46 +477,38 @@ export const Stats = (() => {
477477
<!-- Key Metrics Grid -->
478478
<div style="display: grid; grid-template-columns: repeat(auto-fit, minmax(220px, 1fr)); gap: 1rem; margin-bottom: 2rem;">
479479
<!-- Stuck PRs - Most Important -->
480-
<a href="#" id="openPRsLink-${org}" target="_blank" rel="noopener" style="text-decoration: none;">
481-
<div style="background: #ffffff; border-radius: 16px; padding: 2rem; box-shadow: 0 2px 12px rgba(0,0,0,0.06); transition: all 0.2s; cursor: pointer; border: 2px solid transparent;"
482-
onmouseover="this.style.borderColor='#007AFF'; this.style.transform='translateY(-2px)'; this.style.boxShadow='0 4px 20px rgba(0,122,255,0.15)';"
483-
onmouseout="this.style.borderColor='transparent'; this.style.transform='translateY(0)'; this.style.boxShadow='0 2px 12px rgba(0,0,0,0.06)';">
484-
<div style="font-size: 0.8125rem; color: #86868b; text-transform: uppercase; letter-spacing: 0.05em; margin-bottom: 0.5rem;">Forgotten Work</div>
485-
<div class="stat-value loading" id="openPRs-${org}" style="font-size: 3rem; font-weight: 300; color: #FF3B30; margin: 0.25rem 0;">-</div>
486-
<div style="font-size: 0.9375rem; color: #515154;">PRs stuck >10 days</div>
480+
<a href="#" id="openPRsLink-${org}" target="_blank" rel="noopener" class="stat-card-link">
481+
<div class="stat-card">
482+
<div class="stat-card-title">Forgotten Work</div>
483+
<div class="stat-card-value stat-value-danger stat-value loading" id="openPRs-${org}">-</div>
484+
<div class="stat-card-subtitle">PRs stuck >10 days</div>
487485
</div>
488486
</a>
489-
487+
490488
<!-- Average Wait Time -->
491-
<a href="#" id="avgOpenAgeLink-${org}" target="_blank" rel="noopener" style="text-decoration: none;">
492-
<div style="background: #ffffff; border-radius: 16px; padding: 2rem; box-shadow: 0 2px 12px rgba(0,0,0,0.06); transition: all 0.2s; cursor: pointer; border: 2px solid transparent;"
493-
onmouseover="this.style.borderColor='#007AFF'; this.style.transform='translateY(-2px)'; this.style.boxShadow='0 4px 20px rgba(0,122,255,0.15)';"
494-
onmouseout="this.style.borderColor='transparent'; this.style.transform='translateY(0)'; this.style.boxShadow='0 2px 12px rgba(0,0,0,0.06)';">
495-
<div style="font-size: 0.8125rem; color: #86868b; text-transform: uppercase; letter-spacing: 0.05em; margin-bottom: 0.5rem;">Wait Time</div>
496-
<div class="stat-value loading" id="avgOpenAge-${org}" style="font-size: 3rem; font-weight: 300; color: #1a1a1a; margin: 0.25rem 0;">-</div>
497-
<div style="font-size: 0.9375rem; color: #515154;">Avg age of open PRs</div>
489+
<a href="#" id="avgOpenAgeLink-${org}" target="_blank" rel="noopener" class="stat-card-link">
490+
<div class="stat-card">
491+
<div class="stat-card-title">Wait Time</div>
492+
<div class="stat-card-value stat-value-primary stat-value loading" id="avgOpenAge-${org}">-</div>
493+
<div class="stat-card-subtitle">Avg age of open PRs</div>
498494
</div>
499495
</a>
500-
496+
501497
<!-- Cycle Time -->
502-
<a href="#" id="avgMergeTimeLink-${org}" target="_blank" rel="noopener" style="text-decoration: none;">
503-
<div style="background: #ffffff; border-radius: 16px; padding: 2rem; box-shadow: 0 2px 12px rgba(0,0,0,0.06); transition: all 0.2s; cursor: pointer; border: 2px solid transparent;"
504-
onmouseover="this.style.borderColor='#007AFF'; this.style.transform='translateY(-2px)'; this.style.boxShadow='0 4px 20px rgba(0,122,255,0.15)';"
505-
onmouseout="this.style.borderColor='transparent'; this.style.transform='translateY(0)'; this.style.boxShadow='0 2px 12px rgba(0,0,0,0.06)';">
506-
<div style="font-size: 0.8125rem; color: #86868b; text-transform: uppercase; letter-spacing: 0.05em; margin-bottom: 0.5rem;">Cycle Time</div>
507-
<div class="stat-value loading" id="avgMergeTime-${org}" style="font-size: 3rem; font-weight: 300; color: #1a1a1a; margin: 0.25rem 0;">-</div>
508-
<div style="font-size: 0.9375rem; color: #515154;">Create → merge time</div>
498+
<a href="#" id="avgMergeTimeLink-${org}" target="_blank" rel="noopener" class="stat-card-link">
499+
<div class="stat-card">
500+
<div class="stat-card-title">Cycle Time</div>
501+
<div class="stat-card-value stat-value-primary stat-value loading" id="avgMergeTime-${org}">-</div>
502+
<div class="stat-card-subtitle">Create → merge time</div>
509503
</div>
510504
</a>
511-
505+
512506
<!-- Shipped -->
513-
<a href="#" id="mergedPRsLink-${org}" target="_blank" rel="noopener" style="text-decoration: none;">
514-
<div style="background: #ffffff; border-radius: 16px; padding: 2rem; box-shadow: 0 2px 12px rgba(0,0,0,0.06); transition: all 0.2s; cursor: pointer; border: 2px solid transparent;"
515-
onmouseover="this.style.borderColor='#007AFF'; this.style.transform='translateY(-2px)'; this.style.boxShadow='0 4px 20px rgba(0,122,255,0.15)';"
516-
onmouseout="this.style.borderColor='transparent'; this.style.transform='translateY(0)'; this.style.boxShadow='0 2px 12px rgba(0,0,0,0.06)';">
517-
<div style="font-size: 0.8125rem; color: #86868b; text-transform: uppercase; letter-spacing: 0.05em; margin-bottom: 0.5rem;">Shipped</div>
518-
<div class="stat-value loading" id="mergedPRs-${org}" style="font-size: 3rem; font-weight: 300; color: #34C759; margin: 0.25rem 0;">-</div>
519-
<div style="font-size: 0.9375rem; color: #515154;">Last 10 days</div>
507+
<a href="#" id="mergedPRsLink-${org}" target="_blank" rel="noopener" class="stat-card-link">
508+
<div class="stat-card">
509+
<div class="stat-card-title">Shipped</div>
510+
<div class="stat-card-value stat-value-success stat-value loading" id="mergedPRs-${org}">-</div>
511+
<div class="stat-card-subtitle">Last 10 days</div>
520512
</div>
521513
</a>
522514
</div>
@@ -1053,7 +1045,13 @@ export const Stats = (() => {
10531045
}
10541046
}
10551047

1056-
cacheAgeEl.innerHTML = `${cacheText} <button onclick="window.clearStatsCache('${org}')" style="background: none; border: none; color: #007AFF; cursor: pointer; font-size: 0.8125rem; padding: 0 0 0 0.5rem; text-decoration: underline;">[clear]</button>`;
1048+
const clearBtn = document.createElement('button');
1049+
clearBtn.className = 'cache-clear-btn';
1050+
clearBtn.textContent = '[clear]';
1051+
clearBtn.addEventListener('click', () => clearStatsCache(org));
1052+
1053+
cacheAgeEl.textContent = cacheText + ' ';
1054+
cacheAgeEl.appendChild(clearBtn);
10571055
cacheAgeEl.style.display = 'block';
10581056
}
10591057
};

assets/styles.css

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4015,3 +4015,125 @@ a {
40154015
font-family: var(--font-mono, monospace);
40164016
font-style: normal;
40174017
}
4018+
4019+
/* CSP-safe inline styles (moved from HTML) */
4020+
.modal-actions {
4021+
margin-top: 1.5rem;
4022+
display: flex;
4023+
gap: 1rem;
4024+
justify-content: flex-end;
4025+
}
4026+
4027+
.code-block {
4028+
background: #f3f4f6;
4029+
padding: 0.75rem;
4030+
border-radius: 0.375rem;
4031+
margin: 1rem 0;
4032+
}
4033+
4034+
.divider {
4035+
margin: 1.5rem 0;
4036+
border: none;
4037+
border-top: 1px solid #e5e7eb;
4038+
}
4039+
4040+
.permission-list {
4041+
margin: 0.5rem 0 1rem 1.5rem;
4042+
}
4043+
4044+
.error-message {
4045+
color: #dc2626;
4046+
margin-top: 0.5rem;
4047+
}
4048+
4049+
.header-logo-link {
4050+
text-decoration: none;
4051+
color: inherit;
4052+
}
4053+
4054+
.menu-emoji {
4055+
font-size: 20px;
4056+
}
4057+
4058+
.login-note {
4059+
margin-top: 1rem;
4060+
padding: 1rem;
4061+
background: #f8fafc;
4062+
border-radius: 0.5rem;
4063+
font-size: 0.8125rem;
4064+
color: #475569;
4065+
}
4066+
4067+
.subscription-note {
4068+
margin-top: 2rem;
4069+
padding: 1rem;
4070+
text-align: center;
4071+
color: var(--color-text-secondary);
4072+
font-size: 0.8125rem;
4073+
}
4074+
4075+
.cache-clear-btn {
4076+
background: none;
4077+
border: none;
4078+
color: #007AFF;
4079+
cursor: pointer;
4080+
font-size: 0.8125rem;
4081+
padding: 0 0 0 0.5rem;
4082+
text-decoration: underline;
4083+
}
4084+
4085+
.cache-clear-btn:hover {
4086+
opacity: 0.7;
4087+
}
4088+
4089+
/* Stats page metric cards */
4090+
.stat-card-link {
4091+
text-decoration: none;
4092+
}
4093+
4094+
.stat-card {
4095+
background: #ffffff;
4096+
border-radius: 16px;
4097+
padding: 2rem;
4098+
box-shadow: 0 2px 12px rgba(0,0,0,0.06);
4099+
transition: all 0.2s;
4100+
cursor: pointer;
4101+
border: 2px solid transparent;
4102+
}
4103+
4104+
.stat-card:hover {
4105+
border-color: #007AFF;
4106+
transform: translateY(-2px);
4107+
box-shadow: 0 4px 20px rgba(0,122,255,0.15);
4108+
}
4109+
4110+
.stat-card-title {
4111+
font-size: 0.8125rem;
4112+
color: #86868b;
4113+
text-transform: uppercase;
4114+
letter-spacing: 0.05em;
4115+
margin-bottom: 0.5rem;
4116+
}
4117+
4118+
.stat-card-value {
4119+
font-size: 3rem;
4120+
font-weight: 300;
4121+
margin: 0.25rem 0;
4122+
}
4123+
4124+
.stat-card-value.stat-value-danger {
4125+
color: #FF3B30;
4126+
}
4127+
4128+
.stat-card-value.stat-value-primary {
4129+
color: #1a1a1a;
4130+
}
4131+
4132+
.stat-card-value.stat-value-success {
4133+
color: #34C759;
4134+
}
4135+
4136+
.stat-card-subtitle {
4137+
font-size: 0.9375rem;
4138+
color: #515154;
4139+
}

0 commit comments

Comments
 (0)