Skip to content

fix: reject unknown built-in demo references locally - #3

Merged
renrenmimi merged 2 commits into
mainfrom
fix/reject-unknown-demo-references
Aug 25, 2026
Merged

renrenmimi merged 2 commits into
mainfrom
fix/reject-unknown-demo-references

Conversation

@renrenmimi

Copy link
Copy Markdown
Owner

Root cause

demo was treated as an ordinary GitHub owner. Only demo/learning-platform was
special-cased; every other demo/* reference parsed as a syntactically valid
owner/repo, missed the built-in check, and fell through to the GitHub adapter:

parseRepoRef('demo/not-a-real-demo')  → ok
isBuiltinRef(...)                     → false
fixtureModeEnabled()                  → false in production
                                      → GET https://github.com/ghapi/repos/demo/not-a-real-demo

So the 404 was correct but expensive: it spent a request from the 60/hour
anonymous allowance to be told what the application already knew. Reproduced
against production before the fix — the response carried
{"remaining":59} with rateLimitAgeMs: 0, which is a request that had just
happened.

There was a second, quieter problem in the same response. The rate-limit store is
module-level and holds whatever the last real GitHub response reported, so a
locally decided failure inherited a snapshot from an unrelated earlier request —
implying quota had been spent and dating the reading to now.

Where the rule lives

demo is now a reserved internal namespace holding exactly one reference. One
predicate, isUnknownBuiltinRef, is enforced at the two boundaries a reference
crosses:

Boundary Function Covers
Request parameters become a reference readRepoRef — src/lib/api/route-helpers.ts all six routes, which already call it
Fixture-or-GitHub decision servedFromBuiltin — src/lib/github/service.ts every service function, including non-route callers

servedFromBuiltin replaced the six separate isBuiltinRef gates, so the
"is this built-in?" question and the "then it cannot exist" answer are the same
call. Neither branch can be skipped for one endpoint.

Behaviour

  • demo/learning-platform — unchanged, still served from the built-in fixture.
  • Any other demo/* — local 404 not-found, on all six routes, naming what the
    namespace holds. Never silently swapped for the demo that does exist.
  • No GitHub request, no Authorization header, no quota consumed.
  • rateLimit: null and rateLimitAgeMs: null.
  • Unreachable to the test fixtures even with RTM_FIXTURE_MODE=1.
  • demos/… and my-demo/… are real accounts and keep going down the ordinary
    GitHub path.
  • Malformed input keeps its existing 400 invalid-input behaviour.

Locally decided failures in general now report no quota — that includes the
400s from parameter validation, which had the same leak. Genuine GitHub
failures still report theirs, including the rate-limited case, which carries the
snapshot from its own response.

Proof

Route-handler tests drive the real GET exports with fetch stubbed to throw on
any call, so a single GitHub request fails the test:

  • every route returns 404 for demo/not-a-real-demo with rateLimit and
    rateLimitAgeMs null, and zero adapter calls;
  • the valid demo answers all six routes with zero adapter calls;
  • eight unknown demo/* shapes refused, including in fixture mode.

The exact regression sequence asked for:

  1. a stubbed live GitHub response carrying x-ratelimit-* headers → 200, and
    the response reports {limit: 60, remaining: 42};
  2. demo/not-a-real-demo;
  3. 404;
  4. rateLimit: null;
  5. rateLimitAgeMs: null;
  6. fetch call count unchanged from step 1.

Two counter-tests keep the fix honest: a genuine GitHub 404 still reports
{remaining: 17}, and a live repository still produces exactly
https://github.com/ghapi/repos/octocat/hello-world.

E2E adds the same checks against the production build, plus one that a shared
?repo=demo/not-a-real-demo URL shows the not-found state, makes no GitHub
request, offers the demo as a choice, and does not show the built-in badge.

Verification

Check Result
tsc --noEmit clean
eslint . clean
vitest run 428 passed (was 397)
next build clean
playwright test 141 passed (was 138)
privacy audit 3/3 pass

Preserved

  • Badge wording is still 0 GitHub requests, in both the header and the landing
    card. 0 API requests was not reintroduced.
  • The CI workflow is untouched (0 files changed under .github/).
  • src/lib/github/client.ts is untouched, so the server-only token architecture
    is exactly as it was. A test asserts the browser still receives only
    tokenConfigured.
  • The built-in demo is unchanged: still exactly 16 commits, dataSource: builtin,
    htmlUrl: null.

🤖 Generated with Claude Code

renrenmimi and others added 2 commits August 25, 2026 09:36
`demo` is an internal namespace, not a GitHub account, but only
`demo/learning-platform` was treated as special. Every other `demo/*` reference
was syntactically valid, so it fell through to the GitHub adapter and spent a
request from the anonymous allowance to be told what was already known: the
repository does not exist.

The rule now lives in the two places a reference crosses a boundary, sharing one
predicate:

- `readRepoRef` in the route helpers, which all six routes already call, so no
  endpoint can be given the check separately and no direct API request escapes it;
- `servedFromBuiltin` in the service layer, which is the single question each
  service function asks before deciding between the built-in fixture and GitHub.

An unknown reference gets the ordinary 404 and names what the namespace holds. It
is never silently swapped for the demo that does exist, and it cannot reach the
test fixtures even with fixture mode on.

Owners that merely resemble the reserved one -- `demos`, `my-demo` -- are real
accounts and keep going down the ordinary path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The README still said unknown `demo/*` references go down the ordinary GitHub
path, which was the behaviour this change removes. It now states that the
namespace is reserved, where the rule is enforced, and why a locally decided
failure reports no quota.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Aug 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
repo-time-machine Ready Ready Preview Aug 25, 2026 4:38pm

@renrenmimi
renrenmimi merged commit 9cda0fb into main Aug 25, 2026
4 checks passed
@renrenmimi
renrenmimi deleted the fix/reject-unknown-demo-references branch August 25, 2026 16:42

This branch was successfully deployed

1 active deployment
Preview — ff89e03c Deployed Aug 25, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant