-
Notifications
You must be signed in to change notification settings - Fork 0
Fix timing leak, CORS enforcement, CI race condition, and frontend hardcoding #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
60f0a87
14abd92
011fd54
077ca89
46de2c0
bb8d55c
7fa6194
29038ce
29993fc
b98718a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,5 +2,6 @@ node_modules/ | |
| .debug-artifacts/ | ||
| .DS_Store | ||
| Thumbs.db | ||
| data/*.sqlite | ||
| data/*.sqlite-wal | ||
| data/*.sqlite-shm | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,16 +5,7 @@ | |
| <meta name="viewport" content="width=device-width, initial-scale=1.0, viewport-fit=cover"> | ||
| <title>Brialert redirect</title> | ||
| <meta http-equiv="refresh" content="0; url=/index.html"> | ||
| <script> | ||
| const canonicalTarget = location.pathname.toLowerCase().includes('/brialert') | ||
| ? '/Brialert/index.html' | ||
| : '/index.html'; | ||
| const query = location.search || ''; | ||
| const hash = location.hash || ''; | ||
| if (location.pathname.toLowerCase() !== canonicalTarget.toLowerCase()) { | ||
| location.replace(canonicalTarget + query + hash); | ||
| } | ||
| </script> | ||
| <script src="app/redirect.js"></script> | ||
|
Comment on lines
7
to
+8
|
||
| <style> | ||
| body { | ||
| margin: 0; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -95,8 +95,12 @@ function verifySignedToken(rawToken) { | |
| const expectedSignature = signToken(encodedPayload); | ||
| const expectedBuffer = Buffer.from(expectedSignature); | ||
| const receivedBuffer = Buffer.from(encodedSignature); | ||
| if (expectedBuffer.length !== receivedBuffer.length) return null; | ||
| if (!crypto.timingSafeEqual(expectedBuffer, receivedBuffer)) return null; | ||
| const maxLen = Math.max(expectedBuffer.length, receivedBuffer.length); | ||
| const paddedExpected = Buffer.alloc(maxLen); | ||
| const paddedReceived = Buffer.alloc(maxLen); | ||
| expectedBuffer.copy(paddedExpected); | ||
| receivedBuffer.copy(paddedReceived); | ||
| if (!crypto.timingSafeEqual(paddedExpected, paddedReceived) || expectedBuffer.length !== receivedBuffer.length) return null; | ||
| try { | ||
| const payload = JSON.parse(base64UrlDecode(encodedPayload)); | ||
| if (!payload || typeof payload !== 'object') return null; | ||
|
|
@@ -146,17 +150,15 @@ function serializeCookie(parts, value, maxAgeSeconds) { | |
| export function applyCorsHeaders(request, response, methods) { | ||
| const requestOrigin = normaliseOrigin(request?.headers?.origin); | ||
| const allowedOrigins = new Set(getAllowedOrigins()); | ||
| if (requestOrigin && allowedOrigins.has(requestOrigin)) { | ||
| const originAllowed = !requestOrigin || allowedOrigins.has(requestOrigin); | ||
| if (requestOrigin && originAllowed) { | ||
| response.setHeader('Access-Control-Allow-Origin', requestOrigin); | ||
| response.setHeader('Access-Control-Allow-Credentials', 'true'); | ||
| response.setHeader('Vary', 'Origin'); | ||
| } | ||
| response.setHeader('Access-Control-Allow-Methods', methods); | ||
| response.setHeader('Access-Control-Allow-Headers', 'Content-Type'); | ||
| if (request?.method === 'OPTIONS') { | ||
| return !requestOrigin || allowedOrigins.has(requestOrigin); | ||
| } | ||
| return true; | ||
| return originAllowed; | ||
|
Comment on lines
150
to
+161
|
||
| } | ||
|
|
||
| export function readAdminSession(request) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,7 +34,9 @@ function sendError(response, error) { | |
| } | ||
|
|
||
| export default async function handler(request, response) { | ||
| applyCorsHeaders(request, response, 'POST,OPTIONS'); | ||
| if (!applyCorsHeaders(request, response, 'POST,OPTIONS')) { | ||
| return response.status(403).json({ ok: false, error: 'origin-not-allowed', detail: 'Cross-origin request from disallowed origin.' }); | ||
| } | ||
| if (request.method === 'OPTIONS') { | ||
| response.setHeader('Allow', 'POST,OPTIONS'); | ||
| return response.status(204).end(); | ||
|
|
@@ -60,6 +62,11 @@ export default async function handler(request, response) { | |
| }); | ||
| } | ||
|
|
||
| // Claim the slot before the async dispatch so concurrent requests see | ||
| // the lock immediately (avoids check-then-act race). | ||
| const previousTriggerTime = lastTriggerTime; | ||
| lastTriggerTime = now; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Setting Useful? React with 👍 / 👎. |
||
|
|
||
| const config = getRepoConfig(); | ||
|
|
||
| const dispatchUrl = `${GITHUB_API_BASE}/repos/${config.owner}/${config.repo}/actions/workflows/${WORKFLOW_FILENAME}/dispatches`; | ||
|
|
@@ -79,26 +86,19 @@ export default async function handler(request, response) { | |
| }); | ||
|
|
||
| if (!dispatchResponse.ok) { | ||
| const errorText = await dispatchResponse.text().catch(() => ''); | ||
| // Roll back so the next request can retry after a real failure. | ||
| lastTriggerTime = previousTriggerTime; | ||
| let errorMessage = 'Failed to trigger workflow dispatch.'; | ||
| try { | ||
| const errorPayload = JSON.parse(errorText); | ||
| errorMessage = errorPayload.message || errorMessage; | ||
| } catch { | ||
| // Use default error message | ||
| } | ||
|
|
||
| if (dispatchResponse.status === 401 || dispatchResponse.status === 403) { | ||
| throw new ApiError('unauthorized', errorMessage, 503); | ||
| throw new ApiError('unauthorized', 'GitHub API authentication failed.', 503); | ||
| } | ||
| if (dispatchResponse.status === 404) { | ||
| throw new ApiError('workflow-not-found', `Workflow ${WORKFLOW_FILENAME} not found.`, 404); | ||
| throw new ApiError('workflow-not-found', 'Workflow not found.', 404); | ||
| } | ||
| throw new ApiError('trigger-failed', errorMessage, 500); | ||
| } | ||
|
|
||
| lastTriggerTime = now; | ||
|
|
||
| return response.status(200).json({ | ||
| ok: true, | ||
| detail: 'Live feed workflow triggered successfully. New feed data should appear within 2-5 minutes.', | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding
data/*.sqliteto.gitignoremakesgit add data/brialert.sqlitefail in thecommit-live-feedjob (.github/workflows/update-live-feed.yml), so scheduled feed updates will error whenever the builder outputs that file. This is a regression from the previous behavior where the SQLite artifact could be staged and committed; now the workflow exits non-zero with "path is ignored" instead of publishing refreshed data.Useful? React with 👍 / 👎.