From 34e2cbdc2d477e7458f48cdb5df5872233f27058 Mon Sep 17 00:00:00 2001 From: Chris Bowles Date: Wed, 8 Jul 2026 02:33:12 -0400 Subject: [PATCH] remove username as public input from /sponsors/github MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the previous commit's fix. The only caller (Donate.vue via apiService.getSponsors()) never sends a username and always relied on the server-side default, so the parameter had no legitimate use even after being parameterized — it just let anyone use deflock's server as an unauthenticated relay onto GitHub's GraphQL API under the server's own token. Drop it entirely: the route no longer accepts a username querystring param, and GithubClient.getSponsors() takes no argument, hardcoding the maintainer's login via MAINTAINER_USERNAME. Keeps the $username GraphQL-variable parameterization technique so reintroducing a variable input later can't reintroduce the injection. Co-Authored-By: Claude Sonnet 5 --- api/README.md | 2 +- api/server.ts | 10 +---- api/services/GithubClient.test.ts | 74 ++++++++++++++++++++----------- api/services/GithubClient.ts | 18 ++++---- 4 files changed, 61 insertions(+), 43 deletions(-) diff --git a/api/README.md b/api/README.md index d2281d1..2c8b6fb 100644 --- a/api/README.md +++ b/api/README.md @@ -4,7 +4,7 @@ A Fastify-based API service for DeFlock handling non-OSM related backend logic. ## Endpoints - `/geocode?query=...` — Geocode a location -- `/sponsors/github?username=...` — Get GitHub sponsors +- `/sponsors/github` — Get GitHub sponsors - `/healthcheck` — Health check ## Development diff --git a/api/server.ts b/api/server.ts index 6ece654..e899bd7 100644 --- a/api/server.ts +++ b/api/server.ts @@ -205,22 +205,14 @@ const start = async () => { server.get('/sponsors/github', { schema: { - querystring: { - type: 'object', - properties: { - // GitHub usernames are 1-39 chars of letters, digits, and hyphens. - username: { type: 'string', pattern: '^[A-Za-z0-9-]{1,39}$', default: 'frillweeman' }, - }, - }, response: { 200: SponsorsResponseSchema, 500: { type: 'object', properties: { error: { type: 'string' } } }, }, }, }, async (request, reply) => { - const { username } = request.query as { username?: string }; reply.header('Cache-Control', 'public, max-age=60, s-maxage=600'); - const result = await githubClient.getSponsors(username || 'frillweeman'); + const result = await githubClient.getSponsors(); return result; }); diff --git a/api/services/GithubClient.test.ts b/api/services/GithubClient.test.ts index b375557..7b3df30 100644 --- a/api/services/GithubClient.test.ts +++ b/api/services/GithubClient.test.ts @@ -1,36 +1,60 @@ -import { describe, it, expect } from 'bun:test'; -import { buildSponsorsQuery } from './GithubClient'; +import { describe, it, expect, afterEach, mock } from 'bun:test'; +import { GithubClient, buildSponsorsQuery } from './GithubClient'; describe('buildSponsorsQuery', () => { - it('parameterizes the username instead of interpolating it', () => { - const { query, variables } = buildSponsorsQuery('frillweeman'); - expect(query).toContain('query($login: String!)'); - expect(query).toContain('user(login: $login)'); - expect(variables).toEqual({ login: 'frillweeman' }); - }); - - it('never places the raw username inside the query string', () => { - const { query } = buildSponsorsQuery('octocat'); - // The query is fixed and only references the $login variable — the caller's - // value must not appear in the query text itself. - expect(query).not.toContain('octocat'); - }); - - it('keeps GraphQL-breaking input confined to the variables (injection guard)', () => { - // A username containing a quote used to break out of the interpolated - // string literal. It must now travel only in `variables.login`. - const hostile = '") { viewer { login } } #'; - const { query, variables } = buildSponsorsQuery(hostile); - expect(variables.login).toBe(hostile); - expect(query).not.toContain(hostile); - expect(query).not.toContain('viewer'); + it('parameterizes the username via a $username GraphQL variable instead of interpolating it', () => { + const { query, variables } = buildSponsorsQuery(); + expect(query).toContain('query($username: String!)'); + expect(query).toContain('user(login: $username)'); + expect(variables).toEqual({ username: 'frillweeman' }); }); it('requests sponsorships and the expected sponsor fields', () => { - const { query } = buildSponsorsQuery('frillweeman'); + const { query } = buildSponsorsQuery(); expect(query).toContain('sponsorshipsAsMaintainer(first: 100)'); for (const field of ['login', 'name', 'avatarUrl', 'url']) { expect(query).toContain(field); } }); }); + +describe('GithubClient.getSponsors', () => { + const originalFetch = global.fetch; + + afterEach(() => { + global.fetch = originalFetch; + }); + + it('sends exactly the query and variables built by buildSponsorsQuery', async () => { + let capturedBody: any; + global.fetch = mock(async (_url: string, init: RequestInit) => { + capturedBody = JSON.parse(init.body as string); + return new Response(JSON.stringify({ data: { user: { sponsorshipsAsMaintainer: { nodes: [] } } } })); + }) as unknown as typeof fetch; + + const client = new GithubClient(); + await client.getSponsors(); + + expect(capturedBody).toEqual(buildSponsorsQuery()); + }); + + it('returns the sponsor nodes from the response', async () => { + const nodes = [{ sponsor: { login: 'someone', name: 'Someone', avatarUrl: 'https://example.com/a.png', url: 'https://github.com/someone' } }]; + global.fetch = mock(async () => + new Response(JSON.stringify({ data: { user: { sponsorshipsAsMaintainer: { nodes } } } })) + ) as unknown as typeof fetch; + + const client = new GithubClient(); + const result = await client.getSponsors(); + + expect(result).toEqual(nodes); + }); + + it('throws when the GitHub API responds with a non-OK status', async () => { + global.fetch = mock(async () => new Response('', { status: 500 })) as unknown as typeof fetch; + + const client = new GithubClient(); + + await expect(client.getSponsors()).rejects.toThrow('Failed to get sponsors: 500'); + }); +}); \ No newline at end of file diff --git a/api/services/GithubClient.ts b/api/services/GithubClient.ts index cb5241e..f9ccaa1 100644 --- a/api/services/GithubClient.ts +++ b/api/services/GithubClient.ts @@ -3,6 +3,7 @@ import { Type, Static } from '@sinclair/typebox'; const GITHUB_TOKEN = process.env.GITHUB_TOKEN || ''; const graphQLEndpoint = 'https://api.github.com/graphql'; +const MAINTAINER_USERNAME = 'frillweeman'; export const SponsorSchema = Type.Object({ sponsor: Type.Object({ @@ -17,17 +18,18 @@ export type Sponsor = Static; export const SponsorsResponseSchema = Type.Array(SponsorSchema); -// Pass the username as a GraphQL variable rather than interpolating it into the -// query string, so a value containing quotes or other GraphQL syntax cannot -// break out of the string literal and alter the query. -export const buildSponsorsQuery = (username: string): { query: string; variables: { login: string } } => ({ - query: `query($login: String!) { user(login: $login) { sponsorshipsAsMaintainer(first: 100) { nodes { sponsor { login name avatarUrl url } } } } }`, - variables: { login: username }, +// The maintainer's username is passed as a GraphQL variable rather than +// interpolated into the query string. It's a hardcoded constant today, but +// keeping this indirection means a future caller-supplied value can never +// break out of the query text the way the old string-interpolated version did. +export const buildSponsorsQuery = (): { query: string; variables: { username: string } } => ({ + query: `query($username: String!) { user(login: $username) { sponsorshipsAsMaintainer(first: 100) { nodes { sponsor { login name avatarUrl url } } } } }`, + variables: { username: MAINTAINER_USERNAME }, }); export class GithubClient { - async getSponsors(username: string): Promise { - const { query, variables } = buildSponsorsQuery(username); + async getSponsors(): Promise { + const { query, variables } = buildSponsorsQuery(); const body = JSON.stringify({ query, variables }); const response = await fetch(graphQLEndpoint, { method: 'POST',