Repository navigation
feat: add the Glitch Invaders challenge - #23
Conversation
theycallmeswift
left a comment
There was a problem hiding this comment.
Thanks for this; it's a substantial challenge and the game, server, and tests are in good shape. Two independent reviews ran on 51bd37a (one against the challenge contract and tests, one on security and network), with verify, test (206 pass), lint (18 files, 0 issues), and the hooks exercised by hand. Five things need to change before merge, one is a call for MLH, and the rest are nits.
Blocking
- Live Sentry config comes from a channel a station never provides.
server/live-config.mjsreadsGLITCH_*/VITE_*environment variables, a package.env, orlive.config.jsonnext tochallenge.json. The station passes hooks only the spec's Environment table, so on a station this is always demo mode, and a staffer who follows the header comment ends up with a real DSN in a file that is not git-ignored (only.envis), which is then served to the page at/runtime-config.json. Fix: ship demo-only for now, removelive-config.mjswith the env,.env, andVITE_*readers (setup.mjs:99), and bring live mode back once Sentry settings arrive through a station channel (a vault service or the event config). If you keep the file channel, addchallenges/*/live.config.jsonto.gitignoreand make the comment true. - No Content-Security-Policy.
curl -sI http://127.0.0.1:4373/returns only cache-control and x-content-type-options, so inline script and any external origin are allowed. Fix: send the bird server's policy (default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data:; connect-src 'self') fromserver/page-server.mjs, and replace the twostyle:attribute strings ingame/src/ui/incident-console.mjs(around lines 642 and 646) withelement.styleassignments sostyle-srcneeds no'unsafe-inline'. intro.mdsays each failure "sends telemetry to Sentry" and to "Open it in Sentry". In demo mode, which is every station today, nothing goes to Sentry. Fix: say the Incident Console shows the Sentry evidence (stack trace, breadcrumbs, trace) and drop "Open it in Sentry".- The start-challenge skill has no standalone branch. The spec requires the skill to tell the participant which step they finished when
report_stepis missing; onlydebriefis covered. Fix: whenreport_stepandsession_infoare absent, readglitch-progress.jsonand acknowledge each step whoselaunchedorstagereachedyesorrepaired. - The in-page inactivity reset wipes progress. After 120 s without page input the page warns, and 30 s later it
DELETEsglitch-progress.jsonand reloads at launch state (game/src/app/app.mjsaround line 520, defaults inshared/runtime-config.mjs). Time in the terminal talking to Pi counts as idle, so the debrief step trips it and the guide's "all three repaired" gate can no longer pass. The station already owns idle teardown and wipes everything. Fix: remove the in-page reset, or never clear progress oncelaunched.
For MLH to decide
PUT /api/progress accepts any stage for any bug in one write, and the page republishes it on load, so a participant can mark all three fixes done without playing (one curl, then reload, and #boothstrap-state reads all repaired). The spec's trust model accepts monitor reports as-is, and DevTools can edit any browser challenge's DOM, so this may be acceptable. If not, let the page POST events (encounter, diagnosis option, patch id) and have the server compute stages against the correct patch ids.
Nits
- Dead weight to delete:
game/src/sentry/reporter.mjsandgame/src/diagnostics/evidence.mjsare 0 bytes; thevite devfallbacks (MemoryProgressStore,createDemoRuntimeConfiginmain.mjs) serve a build tool the repo doesn't have;window.__GLITCH_STATE__/__GLITCH_WORLD__are read by nothing;server/setup.mjs:23mentions a nonexistentdist/. page-server.mjs: the GET handler follows a symlinkedglitch-progress.json(my probe read a file outside the workspace);lstatfirst and refuse anything that isn't a regular file with one link, and write the temp file withflag: 'wx', asbirdsbirdbirds/workspace-file.mjsdoes. A malformed percent-escape (/%E0%A4%A) returns 500; return 400.challenge.jsondone_whens name#boothstrap-state, which the monitor's accessibility snapshot doesn't expose; phrase them on the visible line, e.g.The page shows the line "glitch-state v1 …" containing "launched=yes".monitor/SKILL.md: atarget=_blanktab can leave the snapshot on the wrong page; add "if the snapshot is not http://127.0.0.1:4373/,list_pages,select_page, snapshot once."test/glitch-invaders.test.mjs: the hook test is the happy path plus a double cleanup. Add the house cases from the Mad Libs test: server in setup's process group, busy port fails naming it, cleanup after failed setup, after a crash with a foreign listener, with a missing workspace, and a run with noBOOTHSTRAP_*variables (all pass by hand today). Add server refusals for bad JSON, a malformed escape, and a symlinked progress file.README.md: rewrap to 80 columns and link tomonitor/SKILL.mdinstead of restating the marker rules.- PR body: give measured counts (206 tests, 18 lint files), drop the process narrative, and tick the Ready gate rows once they hold.
Generated by Claude Code
theycallmeswift
left a comment
There was a problem hiding this comment.
Re-reviewed at ecc41f4: every item from the first review is resolved or recorded under Decide. Live mode is gone, the server sends a CSP with no inline allowances, the progress and state files are read and written only through lstat-guarded paths, the inactivity reset is removed, the skill has its standalone branch, and the hook edge cases are tested (229 tests, 18 lint files, verify ok, CI green, Chromium load clean). Merging as agreed with the repo owner.
Generated by Claude Code
Adds
challenges/glitch-invaders, the repo's first browser challenge: a shooter where three bugs fight back and the participant fixes them from simulated Sentry evidence (stack trace, breadcrumbs, performance trace) in an Incident Console. It ships in demo mode only; live Sentry returns once boothstrap provides sponsor settings (MLH/boothstrap.sh#519).Decide
PUT /api/progressaccepts any stage for any bug, so one curl and a reload can mark all three fixes repaired. Recommend: keep it as is, because the spec's trust model accepts monitor reports as-is and DevTools can edit any browser challenge's DOM. If that changes, the page posts events and the server computes stages against the correct patch ids..env, orlive.config.json, none of which a station provides. Recommend: bring it back through station-provided sponsor settings, tracked in MLH/boothstrap.sh#519.Ready gate
npm run verify: 4 packages ok.npm test: 229 tests, 229 pass, 0 fail.npm run lint: 18 files, 0 issues./%E0%A4%A, and a GET following a symlinkedglitch-progress.jsonoutside the workspace; the new tests assert the CSP, 400, and 422. The directory-at-progress-path, symlinked.glitch/runtime-config.json, and symlinkedserver.pidtests fail with the setup and cleanup fixes reverted (the last with the port still accepting connections).test/forVITE_,live-config,live.config,inactivity,__GLITCH, andsentry.iofinds nothing. NodistDir,MemoryProgressStore, orcreateDemoRuntimeConfigremains.README.md,intro.md,challenge.jsondone_whens,monitor/SKILL.md(list_pages/select_pagefallback) andskills/start-challenge/SKILL.md(standalone branch readingglitch-progress.json) updated. Root README index entry unchanged and still accurate../setupin a temp workspace underenv -iwith onlyBOOTHSTRAP_*set, and with none set; curled the CSP header, traversal (404), malformed escape (400), oversized body (413), bad JSON (400), symlinked, hard-linked, and FIFO progress files (422, outside file untouched); cleanup twice, after a failed setup, with a missing workspace, and with a directory or symlinks planted under the workspace (exit 0, port refused afterwards, outside targets untouched). Loaded the page in Chromium: no CSP violations, theglitch-state v1line is visible and readslaunched=yesafter Start, and a refused save shows "Progress can't be saved; reload the page."docs/style/by a separate reviewer; cut live-mode text, the restated marker rules, and stale build wording.default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data:; connect-src 'self'. Setup and cleanup never write or read through a symlink planted in the workspace.🤖 Generated with Claude Code
https://claude.ai/code/session_01UwzZoAziJnZnizZ4cDr5Xc