feat: migrate Hackweek to Cloudflare - #131
Merged
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+9
to
+25
| name: Verify | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout Repo | ||
| uses: actions/checkout@v3 | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Cache node modules | ||
| id: cache-npm | ||
| uses: actions/cache@v3 | ||
| env: | ||
| cache-name: cache-node-modules | ||
| - name: Set up Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| # npm cache files are stored in `~/.npm` on Linux/macOS | ||
| path: ~/.npm | ||
| key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-build-${{ env.cache-name }}- | ||
| ${{ runner.os }}-build- | ||
| ${{ runner.os }}- | ||
|
|
||
| - if: ${{ steps.cache-npm.outputs.cache-hit == 'false' }} | ||
| name: List the state of node modules | ||
| continue-on-error: true | ||
| run: npm list | ||
|
|
||
| - name: Install Dependencies | ||
| run: npm install | ||
| node-version: 24.19.0 | ||
| cache: npm | ||
|
|
||
| - name: Build | ||
| run: npm run build | ||
| - name: Install dependencies deterministically | ||
| run: npm ci --no-audit --no-fund | ||
|
|
||
| - name: Bind Config | ||
| run: npm run bind-version | ||
|
|
||
| - name: Test | ||
| run: npm test -- --testResultsProcessor="jest-junit" --coverage | ||
|
|
||
| - name: Publish Test Report | ||
| uses: mikepenz/action-junit-report@v3 | ||
| if: always() # always run even if the previous step fails | ||
| with: | ||
| report_paths: '**/junit.xml' | ||
| - name: Verify | ||
| run: npm run verify |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c4d6200. Configure here.
| - run: npx tsx scripts/measure-loudness.ts | ||
| env: | ||
| VIDEO_API_URL: ${{ secrets.VIDEO_API_URL }} | ||
| VIDEO_SERVICE_TOKEN: ${{ secrets.VIDEO_SERVICE_TOKEN }} |
There was a problem hiding this comment.
Measure workflow missing ffmpeg
Medium Severity
video-measure.yml runs scripts/measure-loudness.ts, which spawns ffmpeg, but the workflow never installs it. GitHub ubuntu-latest runners do not ship ffmpeg, unlike the archive workflow which installs rclone. When videos are queued, every measurement fails with a missing-command error and is reported as a measurement failure.
Reviewed by Cursor Bugbot for commit c4d6200. Configure here.
| </div> | ||
| )} | ||
|
|
||
| {canManage && !disabled && !isUploading && ( |
There was a problem hiding this comment.
Bug: A race condition allows deleting a video during server-side processing. The delete button appears before the server state is updated, creating a window for inconsistent deletion.
Severity: HIGH
Suggested Fix
Add a guard to the deleteProjectVideo function in src/worker/services/videos.ts to prevent deletion if the video's status is a transient one (e.g., 'uploading', 'processing', 'measuring'). The deletion should only be allowed for terminal states like 'ready' or 'failed'.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/app/video/ProjectVideoPanel.tsx#L140
Potential issue: A race condition exists where a user can delete a video while it is
still being processed on the server. The delete button in `ProjectVideoPanel.tsx`
becomes available immediately after the client-side upload completes, based on the
`isUploading` flag. However, the server-side video status is still 'uploading' and only
updates after a 5-second refetch interval. The `deleteProjectVideo` function lacks a
guard to prevent deletion of videos in transient states. A user clicking delete within
this 5-second window will cause the video to be removed from the database and Cloudflare
Stream, leading to data loss and inconsistent state.
Comment on lines
+68
to
+70
| async deleteVideo(uid: string): Promise<void> { | ||
| this.assertVideo(uid).deleted = true; | ||
| } |
There was a problem hiding this comment.
Bug: The FakeStreamGateway used in tests is a stateful singleton without a reset mechanism, causing state to leak between tests and potentially leading to test failures.
Severity: MEDIUM
Suggested Fix
Implement a reset mechanism for the FakeStreamGateway singleton. This could be an exported reset() function that is called in a beforeEach or afterEach hook within the test suite to ensure each test runs with a clean state.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/worker/integrations/stream/fake.ts#L68-L70
Potential issue: The `FakeStreamGateway` used for local development and testing is a
module-level singleton that persists state across requests and test runs. The
`deleteVideo` method permanently marks a video record as deleted within the singleton's
state. Since there is no mechanism to reset this state between tests, and tests run
sequentially sharing the same module cache (`maxWorkers: 1`), a test that deletes a
video could cause subsequent tests that rely on that video's UID to fail. This violates
test isolation and can lead to flaky and difficult-to-debug test failures.
Also affects:
src/worker/integrations/stream/index.ts:8~8
Comment on lines
+299
to
+304
| for (const video of results) { | ||
| if (!video.stream_uid) continue; | ||
| const download = await gateway.ensureDownload(video.stream_uid); | ||
| if (download.status === 'error') { | ||
| await markMeasurementFailure(db, video.id, 'Stream MP4 generation failed'); | ||
| continue; |
There was a problem hiding this comment.
Bug: An uncaught error in markMeasurementFailure within the listMeasurementQueue loop causes the entire endpoint to fail if a single video's status changes concurrently, halting queue processing.
Severity: HIGH
Suggested Fix
Wrap the call to markMeasurementFailure inside the loop within listMeasurementQueue in a try-catch block. This will allow the system to handle the ServiceError for a single video (e.g., by logging it) while continuing to process the remaining videos in the queue.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/worker/services/videos.ts#L299-L304
Potential issue: In the `listMeasurementQueue` function, videos with a download status
of 'error' trigger a call to `markMeasurementFailure`. If `markMeasurementFailure` fails
to update a video's status (e.g., due to a race condition where another process changes
the status first), it throws a `ServiceError`. This error is not caught within the
processing loop. Instead, it propagates up to the route handler, causing the entire
`/measurements` endpoint to fail with a 409 Conflict error. This halts the processing of
all videos in the queue, even if only a single video encounters the race condition.
Replace the CRA/Firebase runtime toolchain with a React and TypeScript SPA served by a single Hono Worker through Cloudflare Static Assets. Configure typed local D1 and R2 bindings, Vite+ quality gates, workerd integration tests, and CI verification.\n\nAdd the normalized initial D1 schema with source identifiers, relational constraints, vote integrity checks, Stream event idempotency, and indexes. Record the clean Vite+ compatibility spike and local development workflow without provisioning remote resources.
Validate Access application JWTs at the Worker boundary with explicit issuer, audience, signature, time, token-type, subject, and company-domain checks. Resolve validated identities to stable D1 profiles while preserving migrated source UIDs and keeping administrator roles database-backed.\n\nAdd typed session/profile contracts, centralized RBAC middleware, explicit signed local fixtures, authenticated frontend states, an identity-link migration, and security coverage for forged, expired, mis-scoped, out-of-domain, and client-role escalation attempts.
Add typed year, project, group, member, claim, withdrawal, and private attachment APIs backed by D1 and R2. Enforce open-submission, same-year group, membership, creator, and administrator rules exclusively at the Worker boundary, including transactional team and group-reference updates.\n\nReplace the foundation landing screen with accessible archive, project, idea, detail, editor, group-admin, and media routes using TanStack Query and a lightweight SPA router. Document intentional legacy changes and cover Worker invariants plus route-level loading, empty, filter, claim, and upload journeys.
Enforce voting, nomination, award, and screening-order invariants in D1 with server-authorized Hono routes and atomic writes. Add year, category, award, group, ballot, and screening administration alongside aggregate-only analytics APIs.\n\nProvide modern voting and admin interfaces, public award rendering, deterministic screening ordering before videos exist, and Worker/frontend coverage for invalid, duplicate, concurrent, cross-year, self-project, and authorization cases.
Add an operator-controlled CLI that validates and transforms Firebase database and Storage exports into deterministic D1 rows and private R2 keys. Imports use source-key upserts, explicit local or confirmed staging selection, path and checksum validation, and machine-readable reconciliation reports.\n\nInclude synthetic historical fixtures, migration invariant tests, a repeatable local rehearsal, ignored operator input/output paths, and the final operator-assisted cutover checklist. The tooling never reads live Firebase, deletes destination data, or performs DNS changes.
Add an explicit loopback-only authentication mode for ordinary local browser development while retaining fail-closed Access and signed fixture verification. Local identity configuration is validated strictly and continues through D1 synchronization so database roles remain authoritative. Replace the browser setup JWKS with safe placeholder identity values, document clean local import and admin-promotion steps, and cover loopback acceptance, remote-host rejection, contradictory configuration, and client role-escalation attempts.
Port the recognizable Sentry Hackweek visual system to the modern React application with the original masthead, self-hosted Rubik typography, ink and blurple palette, squiggle headers, compact controls, cards, forms, voting, and admin treatments. Reuse the tracked year banners and expose participant counts so the archive retains its legacy information hierarchy. Replace the unrelated control-room copy while preserving existing routes and mutations, and add focused semantic coverage for the masthead, archive imagery, project lists, and participant metadata.
Add typed fake and staging-only real Stream gateways for constrained direct tus uploads, selected historical promotion, protected playback, signed downloads, and deletion. Enforce the one-primary-video authorization and lifecycle model in D1 with verified replay-safe webhooks, exact -16 LUFS gain clamping, retryable failures, and archive state kept outside screening readiness.\n\nAdd service-authenticated measurement and Drive archive jobs, lifecycle tests, deterministic worker-test isolation, and an operations runbook documenting current Cloudflare contracts and staging validation.
Add project-bound resumable tus uploads with visible progress, pause, resume, retry, lifecycle status, replacement, and deletion controls. Integrate ready-video protected HLS playback and admin screening status/order preview into the restored Hackweek interface.\n\nBuild the unattended reel around two preloaded video elements, ended-event progression, title cards, keyboard and visible controls, and a single Web Audio graph with clamped per-clip gain and shared limiting. Cover fake upload/media events, accessibility states, ordering, shortcuts, and audio wiring without pretending local Stream can transcode or issue HLS.
Add an isolated seeded D1/R2/fake-Stream readiness journey and wire it into deterministic CI alongside schema, contract, frontend, migration, build, and staging dry-run checks. Gate staging deployment behind manual confirmation, protected environment credentials, reviewed placeholder replacement, and reusable verification.\n\nRemove the replaced Firebase hosting, rules, workflow, and legacy runtime source so Git history is the only reference instead of a compatibility path. Document the Cloudflare architecture, resource and secret setup, migration rehearsals, screening operations, staging evidence, legacy feature mapping, operator-assisted cutover, rollback, and decommission boundaries.
Normalize empty legacy collections and derive missing award labels from their categories so every valid historical award migrates. Explicitly report and omit five historical self-votes that conflict with the current eligibility rules, as selected during the production rehearsal.\n\nMove production-sized reconciliation SQL into private temporary files and chunk source-ID matching below SQLite statement limits. This keeps exact source-count verification reliable for hundreds of users and thousands of related records without weakening current application invariants.
Implement an application-owned Google authorization-code flow with PKCE, state, nonce, strict ID-token verification, exact sentry.io enforcement, and fixed callback navigation. Persist only short-lived login attempts and hashed opaque D1 sessions, rotate sessions on login, revoke them on logout, and enforce exact-origin mutation protection while retaining loopback-only local auth.\n\nReplace Access configuration and staging assumptions with one reviewed Cloudflare environment, Google Console and secret setup guidance, secure login/logout UI, deterministic OAuth fixtures, and comprehensive auth/session regression coverage.
Deploy the single Hackweek Worker environment with reviewed D1, private R2, Google OAuth, and Static Assets configuration for the Sentry Enterprise account. Add an explicit production-safe disabled Stream mode that fails closed without creating fake lifecycle records and presents clear unavailable-video UX while preserving non-video workflows. Adapt migrations and reconciliation to Wrangler's remote D1 behavior and retain deterministic local readiness coverage. Record the exact workers.dev origin, resource bindings, migration workflow, and deployment safeguards without committing secrets or production snapshot data.
Port the legacy multicolor Google G icon to the TypeScript frontend and pair it with the server-side OAuth login link. Style the control with the restored Hackweek palette and interactions, and cover the icon and navigation target in the auth test.
Remove the fixed local identity mode and make complete, origin-matched Google OAuth configuration mandatory for every Worker request. Preserve loopback browser support through normal OAuth sessions while continuing to reject forged identity headers and invalid configuration. Update local readiness, development variables, generated bindings, tests, and operator documentation for Google-only local development.
Promote the first API-ordered Hackweek year into a responsive landing hero with banner, participation metrics, and the appropriate projects call to action. Keep prior years in the existing archive card treatment and preserve the missing-banner fallback.\n\nDiscover year banner assets through Vite's eager glob so future conventionally named year images require no component changes. Cover the hero/archive split and fallback behavior in the app route tests.
Rename the archives menu entry to hackweek, stop the hero content card from overlapping the current-year banner by placing it in a box below the media, and center the archive banner timeline horizontally. Add the 2025 and 2026 year banner images, which the banner glob picks up automatically, and let the hero test accept a real banner as well as the fallback tile.
Persist an admin-only member view on each D1 session and derive the effective role during authentication so all existing authorization paths enforce it. Expose the underlying role for a subtle header switch, refresh frontend session state after changes, and cover role enforcement, recovery, and visibility.
Detach stdin from readiness subprocesses so wrangler auto-confirms its migration prompt instead of blocking; the full verify pipeline now runs unattended.
Derive effective year state from the maximum year id so archived years are always closed without rewriting stored administration flags. Enforce that state across project, media, and vote mutations, including vote creation which previously relied only on database triggers. Expose current-year status to the admin page, lock archived controls with an explanatory note, and cover archived/current behavior in worker and app tests.
Add an accessible segmented view preference to project and idea overviews, persisted locally across visits. Render a responsive compact row layout in list mode and clamp grid-card summaries to three lines while retaining their full text. Fall back to grid when localStorage is unavailable and keep view switching functional when preference writes fail. Cover the default, compact, persistence, and blocked-storage states in app route tests.
Replace the full collaborator checkbox list with selected-member chips and a client-side search combobox. Add bounded results, keyboard navigation, removal controls, and compact styling while preserving memberIds form submission semantics. Cover name and email filtering, selection, removal, empty-query behavior, keyboard interaction, and edit-mode prefill with app tests.
Remove the attachment count from project cards and rows, left-align the primary navigation next to the wordmark, reduce the oversized top padding on all pages, center the page-state card text, and finish the archives-to-hackweek rename on the year back link and forbidden state.
The year hero showed the vote link for archived years even though the server rejects every ballot there; gate it on the effective votingEnabled flag the API already returns.
Hide reel and project-video playback links unless Cloudflare Stream is running in real mode, while keeping the disabled core rollout explicit across the UI and operator documentation. Remove verified dead types, helpers, assets, formatter config, and stale design notes. Consolidate CI on the canonical verification command, keep video measurement manual until its separate rollout, and align the cutover runbooks with the reviewed workers.dev deployment.
Add an accessible search form to project and idea listings while retaining year, kind, group, and display preferences. Send submitted queries to the Worker so matching covers the filtered D1 result set before pagination rather than only visible client records. Match project titles and summaries with escaped, case-insensitive SQLite LIKE patterns. Bound search text to 100 characters and rank exact titles, title prefixes, title substrings, and description matches deterministically without requiring nonstandard D1 extensions. Cover relevance, title and description matches, pagination, filter composition, wildcard escaping, validation, accessible controls, clearing, and retained list view in focused Worker and frontend tests.
Replace the existing icon with the favicon currently served by sentry.io so the Hackweek rewrite carries the genuine Sentry mark instead of a mismatched local asset. Keep the ICO in Vite's public directory for local serving and declare its sizes and image/vnd.microsoft.icon MIME type explicitly in the application HTML.
Generate checked-in Worker bindings from the reviewed production Wrangler configuration rather than ignored local secrets. This keeps CI and clean worktrees reproducible while secret bindings remain explicitly typed in the application interfaces.
HazAT
force-pushed
the
cloudflare-migration
branch
from
August 11, 2026 08:22
fe0a41b to
c76ac1b
Compare
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
| {project.data.project.repository && ( | ||
| <a | ||
| className="repoLink" | ||
| href={project.data.project.repository} |
There was a problem hiding this comment.
User-controlled repository URL rendered as raw href enables javascript: XSS
The project repository field is user-editable free text and is rendered directly as an anchor href with no scheme validation, so a project editor can store a javascript: (or other dangerous-scheme) URL that executes when a viewer clicks 'Open repository'.
Evidence
worker/services/project-input.ts:14acceptsrepositoryviaoptionalText(..., 2_048)with no URL/scheme validation, andworker/repositories/projects.tsstores it verbatim.src/app/components/ProjectForm.tsx:113lets any user with edit permission setrepositoryto arbitrary text.ProjectDetailsPage.tsx:100-107rendershref={project.data.project.repository}directly in an<a>; React does not blockjavascript:hrefs, so a stored malicious URL executes in a viewer's browser on click.target="_blank"may limit execution context in some browsers, but scheme validation (e.g. http/https allowlist) is absent, so the sink is unmitigated.
Identified by Warden · security-review · EWG-K3U
Comment on lines
+80
to
+88
| function constraintError(error: unknown) { | ||
| if (error instanceof Error && error.message.includes('UNIQUE constraint failed')) { | ||
| return new ServiceError( | ||
| 'CONFLICT', | ||
| 'A group with this identifier already exists', | ||
| 409, | ||
| ); | ||
| } | ||
| return error; |
There was a problem hiding this comment.
Bug: The constraintError function returns non-ServiceError objects for database errors other than UNIQUE constraints, causing the worker to crash due to an unhandled exception.
Severity: CRITICAL
Suggested Fix
Modify constraintError to wrap all Error instances in a ServiceError, similar to the pattern in administrationConstraint. This ensures that all database errors are handled gracefully. For example, add a fallback case that returns a generic ServiceError for unexpected database issues.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/worker/repositories/groups.ts#L80-L88
Potential issue: The `constraintError` function only converts UNIQUE constraint
violation errors into a `ServiceError`. For any other database error, such as a foreign
key violation or timeout, it returns the original `Error` object. This error is then
re-thrown up to the `errorResponse` function, which does not handle generic `Error`
types. Consequently, the error is re-thrown again, causing an unhandled rejection that
crashes the worker instead of returning a structured JSON error response to the client.
| ); | ||
| } | ||
| await assertOpenYearAndReferences(db, input, user.id); | ||
| const memberIds = input.kind === 'project' ? unique(input.memberIds) : []; |
There was a problem hiding this comment.
Bug: The updateProject function can remove a creator from their project's member list, creating an inconsistent state where they have permissions but are not listed as a member.
Severity: LOW
Suggested Fix
Modify updateProject to align with createProject and claimProject. Ensure the project's creator ID is always included in the memberIds list during an update to maintain data consistency and prevent the creator from being inadvertently removed from the membership list.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/worker/repositories/projects.ts#L314
Potential issue: The `updateProject` function does not automatically include the
project's creator in the list of members when updating a project. This differs from
`createProject` and `claimProject`, which always ensure the acting user is a member.
While permissions are not affected because creator status is checked separately, this
can lead to an inconsistent state where a creator has full edit rights but is not listed
as a member of their own project. This could be triggered by an admin updating a project
via the API without including the creator in the `memberIds` list.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary