fix: security, rate-limit and a11y hardening pass - #103
Merged
Merged
Conversation
Neither has passed since 2024. The test suite was removed in 86b3ae3 ('delay tests') and vitest exits 1 with no test files. The Pages deploy builds a Cloudflare Pages worker (_worker.js) that GitHub Pages cannot run, and also breaks on the npm cache step (repo uses bun.lock). Production deploys go through Cloudflare Pages' own Git integration.
Deploying petithub with
|
| Latest commit: |
0aaf04b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://20ca446f.petithub.pages.dev |
| Branch Preview URL: | https://petithub-hardening.petithub.pages.dev |
Bugs: - /api always returned 401 (apiAuth had no early return); also strip the "Bearer " prefix before using the header token and verify via the free GET /rate_limit endpoint - swagger UI/JSON were caught by apiAuth and unreachable; register them before the auth middleware so they stay public - OAuth state check passed when both cookie and param were undefined - OAuth tokens and client_secret travelled through URL query strings (browser history / Referer / CF logs). Callback now sets cookies directly and redirects to /; refresh happens inline; token endpoint gets POST body + Accept: application/json. Drops the /github/access_token bounce route. - open redirect via callback_url on /callback and /logout - cors advertised origin:* with credentials:true (spec-invalid) - malformed max_id cookie threw and 500'd the page Rate limiting: - getRandomRepository: ~3-6 API calls per load instead of up to ~1000 (random candidates, one detailed fetch each), self-correcting ceiling on empty pages, no per-repo console.log - optional GITHUB_TOKEN fallback so anonymous traffic isn't capped at 60/h - bump the stale hardcoded MAX_ID a11y / UX: - <html lang>, keyboard-accessible refresh link, decorative alt="", dead Watch/Fork/Star buttons become real GitHub links, no <a>-in-<button> - app.onError + a friendly "GitHub may be rate-limiting" card Tooling: - crypto.getRandomValues for the OAuth state token - hono/secure-headers (X-Frame-Options, Referrer-Policy, HSTS, nosniff) - typecheck script + CI (bun install --frozen-lockfile / typecheck / build) - wrangler compatibility_date bump, drop empty [vars] - .dev.vars.example, trimmed README, dependabot config - drop vitest + the dead vite.config test block
cletqui
force-pushed
the
petithub-hardening
branch
from
September 2, 2026 11:01
7c25736 to
0aaf04b
Compare
This was referenced Sep 2, 2026
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
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.
Full review of the project (like the portfolio pass). Two commits: the CI-workflow removal (already discussed) + this hardening pass. Local verification:
bun run typecheckclean,bun run buildOK,wrangler pages dev distboots and/,/api(401),/api/swagger(200),/github/login(302 w/ crypto state) all respond. The authenticated/api200 path and the live OAuth round-trip couldn't be exercised offline — worth watching the first deploy.Correctness bugs
/apialways returned 401 —apiAuthfell through to the 401 afterawait next(). Rewritten with an early return;Bearerprefix now stripped from the header token; verification uses the freeGET /rate_limit.apiAuthcovered/swaggertoo. Now registered before the auth middleware.statebypass —secret === statepassed when both wereundefined. Nowsecret && state && secret === state; state token usescrypto.getRandomValues.client_secretin URL query strings — the callback bounced tokens through/github/access_token?access_token=…. Now the callback sets cookies directly and redirects to/; refresh happens inline inhandleTokens; the token endpoint gets a POST body +Accept: application/json./github/access_tokenroute +handleAccessremoved.callback_urlon/callbackand/logout— sanitised to same-site paths.corsadvertisedorigin:*withcredentials:true(spec-invalid). DroppedcredentialsandpoweredBy().max_idcookie threw and 500'd the page — guarded parse.Rate limiting
getRandomRepository: ~3–6 API calls per load instead of up to ~1000 (random candidates, one detailed fetch each). Self-corrects a stale ceiling on empty pages. Per-repoconsole.logremoved.MAX_ID.a11y / UX
<html lang="en">; refresh control is a keyboard-accessible<a href="/">; dead Watch/Fork/Star/branch buttons are now real links to the GitHub sub-pages; removed<a>-inside-<button>; decorative icons getalt="".app.onError+ a "GitHub may be rate-limiting — sign in" card instead of a broken stream.Tooling
hono/secure-headers(X-Frame-Options: DENY, Referrer-Policy, HSTS, nosniff). CSP deliberately deferred — inlinestyle=""+ Swagger CDN need a live browser pass.typecheckscript +typescriptdev dep; new.github/workflows/ci.yml(bun install --frozen-lockfile / typecheck / build).wrangler.tomlcompat date → 2026-09-01, empty[vars]removed..dev.vars.example, dependabot config (npm grouped + github-actions).vitest+ the deadtestblock invite.config.ts.Not done (follow-ups)
MAX_IDrefresh job (Pages has no cron; needs a companion Worker).octokit→@octokit/coreto cut the ~550 KB bundle.schema.tsx(366 hand-written lines) → derive from@octokit/openapi-types.