fix: address the review of the contribution routes

README.md

@@ -60,8 +60,10 @@ Defined in [spec 003](docs/specs/003-edits-and-suggestions/spec.md) — the cont
 
 Writes need a signed-in session, an IPL origin and the CSRF token. Before any GitHub call, each identity is checked
 against a deny-list and two caps (20 contributions a day, 5 open at once → `429` with `Retry-After`); bodies are capped
-(edits 272 KB, suggestions 16 KB → `413`); a per-IP limit protects sign-in and writes. The open-PR slot is freed by the
-webhook when the pull request closes. To block an account (maintainers):
+(edits 272 KB, suggestions 16 KB → `413`); a per-IP limit protects sign-in and writes, and link checks for suggestions are capped at 50 a day per identity. The
+daily slot is reserved before GitHub is called and given back if the contribution fails; the open-PR slot is freed by
+the webhook when the pull request closes. KV has no atomic counters, so parallel requests can exceed a cap by a few
+(bounded by the per-IP limit). To block an account (maintainers):
 
 ```bash
 npx wrangler kv key put --binding CONTRIBUTE_KV --env production "deny:github:<numeric id>" "reason"   # or deny:orcid:<iD>

docs/specs/003-edits-and-suggestions/spec.md

@@ -15,7 +15,7 @@ All routes require a session (002) and an IPL origin; responses are JSON.
 | `GET /v1/source?url=<IPL page URL>` | — | `{ repository, path, branch, content, sha }` of the page's source file |
 | `POST /v1/edits` | `{ url, content, sha, summary }` | opens a PR replacing the file; `{ pullRequest: { number, url } }` |
 | `POST /v1/suggestions` | `{ lang, topicPath, type, name, url, description, access? }` | opens a PR appending to `_data/<lang>/<category>/<subcategory>/<type>.yaml`; `{ pullRequest }` |
-| `GET /v1/contributions` | — | the signed-in author's open/merged PRs |
+| `GET /v1/contributions` | — | the signed-in author's PRs (`open`, `closed` or `merged`) |
 
 Rules:
 1. Page → source mapping: `en-US`/`pt-BR` content repositories by URL prefix; generated pages map to

src/contrib/attribution.ts

@@ -1,5 +1,5 @@
 import type { Identity } from '../auth/identity';
-import { singleLine } from '../text';
+import { neutralizeGitHubText, singleLine } from '../text';
 
 /**
  * Credit built only from the verified session identity (issue #16, ADR 0003). The commit author is the App; people
@@ -27,7 +27,7 @@ export function coAuthorTrailer(identity: Identity): string {
 
 /** Commit message: a one-line subject (≤ 72 characters), a blank line and exactly one trailer. */
 export function commitMessage(subject: string, identity: Identity): string {
-  return `${singleLine(subject, 72)}\n\n${coAuthorTrailer(identity)}\n`;
+  return `${neutralizeGitHubText(singleLine(subject, 72))}\n\n${coAuthorTrailer(identity)}\n`;
 }
 
 /** Markdown credit for the PR body: name and profile link; GitHub logins are linked, not @mentioned. */

src/contrib/labels.ts

@@ -1,12 +1,12 @@
-import { singleLine } from '../text';
+import { neutralizeGitHubText, singleLine } from '../text';
 import type { Language } from './topics';
 
 /** Titles and labels of contribution pull requests (issue #9). */
 export const MAX_TITLE = 120;
 
 export function pullRequestTitle(kind: 'edit' | 'suggestion', subject: string): string {
   const prefix = kind === 'edit' ? 'Edit: ' : 'Resource suggestion: ';
-  return singleLine(`${prefix}${singleLine(subject, MAX_TITLE)}`, MAX_TITLE);
+  return singleLine(`${prefix}${neutralizeGitHubText(singleLine(subject, MAX_TITLE))}`, MAX_TITLE);
 }
 
 export interface LabelInput {

src/contrib/limits.test.ts

@@ -49,7 +49,7 @@ describe('per-identity limits (before any GitHub call)', () => {
   it('answers a 6th concurrently open contribution with 429', async () => {
     const identity = reader();
     const session = await signedIn(identity);
-    for (let n = 1; n <= 5; n += 1) await recordContribution(env.CONTRIBUTE_KV, identity, `o/r#${n}`, new Date(Date.UTC(2026, 0, 1)));
+    for (let n = 1; n <= 5; n += 1) await recordContribution(env.CONTRIBUTE_KV, identity, `o/r#${n}`);
     const fake = new FakeGitHub();
     const response = await postEdit(app(fake), session.headers);
 

src/contrib/limits.ts

@@ -5,26 +5,36 @@ import { AppError } from '../errors';
 
 /**
  * Abuse limits (issue #6). Every write is tied to a signed-in identity and checked here **before** any GitHub call:
- * a maintainer deny-list, a daily cap and a cap on pull requests open at once. KV counters are eventually
- * consistent, so caps are approximate across locations; the per-IP Rate Limiting binding adds a coarse guard.
+ * a maintainer deny-list, a daily cap (reserved up front and given back if the contribution fails) and a cap on
+ * pull requests open at once. Workers KV has no atomic increment, so parallel requests from one identity can still
+ * slip past a cap by a few; the per-IP Rate Limiting binding bounds that, and a Durable Object is the upgrade path if
+ * exact counts are ever needed.
  */
 export const DEFAULT_MAX_PRS_PER_DAY = 20;
 export const DEFAULT_MAX_OPEN_PRS = 5;
+export const MAX_LINK_CHECKS_PER_DAY = 50;
 const DAY_TTL_S = 2 * 24 * 60 * 60;
 const OPEN_TTL_S = 30 * 24 * 60 * 60;
 const OPEN_RETRY_S = 60 * 60;
 
 type AppContext = Context<{ Bindings: Env }>;
-const who = (identity: Pick<Identity, 'provider' | 'id'>) => `${identity.provider}:${identity.id}`;
+type Who = Pick<Identity, 'provider' | 'id'>;
+const who = (identity: Who) => `${identity.provider}:${identity.id}`;
 const today = (now: Date) => now.toISOString().slice(0, 10);
-const dayKey = (identity: Pick<Identity, 'provider' | 'id'>, now: Date) => `quota:day:${who(identity)}:${today(now)}`;
-const openKey = (identity: Pick<Identity, 'provider' | 'id'>) => `quota:open:${who(identity)}`;
+const dayKey = (identity: Who, now: Date) => `quota:day:${who(identity)}:${today(now)}`;
+const probeKey = (identity: Who, now: Date) => `quota:probe:${who(identity)}:${today(now)}`;
+const openKey = (identity: Who) => `quota:open:${who(identity)}`;
+/** Repository names are case-insensitive on GitHub; keys must be too. */
+const pullKey = (pullRequest: string) => pullRequest.toLowerCase();
 
 function limit(value: string | undefined, fallback: number): number {
   const parsed = Number(value);
   return Number.isInteger(parsed) && parsed > 0 ? parsed : fallback;
 }
 
+const secondsToMidnight = (now: Date) =>
+  Math.ceil((Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate() + 1) - now.getTime()) / 1000);
+
 /** Coarse per-IP limit (Workers Rate Limiting binding, configured for staging and production). */
 export async function assertIpAllowed(c: AppContext): Promise<void> {
   const limiter = c.env.IP_RATE_LIMITER;
@@ -34,40 +44,60 @@ export async function assertIpAllowed(c: AppContext): Promise<void> {
   if (!success) throw new AppError('RATE_LIMITED', 'Too many requests. Please wait a minute.', undefined, 60);
 }
 
-/** Deny-list and per-identity caps; 403 or 429 with Retry-After, before any GitHub call. */
-export async function assertContributionAllowed(c: AppContext, identity: Identity, now = new Date()): Promise<void> {
+export interface Reservation {
+  /** Give the daily slot back (the contribution failed before a pull request existed). */
+  rollback(): Promise<void>;
+}
+
+/** Deny-list and caps; reserves today's slot. 403 or 429 with Retry-After, before any GitHub call. */
+export async function reserveContribution(c: AppContext, identity: Identity, now = new Date()): Promise<Reservation> {
   const kv = c.env.CONTRIBUTE_KV;
   if (await kv.get(`deny:${who(identity)}`)) {
     throw new AppError('FORBIDDEN', 'This account cannot send contributions. Contact the maintainers if you think this is a mistake.');
   }
-  const perDay = limit(c.env.MAX_PRS_PER_DAY, DEFAULT_MAX_PRS_PER_DAY);
-  const sentToday = Number((await kv.get(dayKey(identity, now))) ?? 0);
-  if (sentToday >= perDay) {
-    const midnight = Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate() + 1);
-    throw new AppError('RATE_LIMITED', `You can send up to ${perDay} contributions a day.`, { limit: 'daily', max: perDay }, Math.ceil((midnight - now.getTime()) / 1000));
-  }
   const maxOpen = limit(c.env.MAX_OPEN_PRS, DEFAULT_MAX_OPEN_PRS);
   const open = (await kv.get<string[]>(openKey(identity), 'json')) ?? [];
   if (open.length >= maxOpen) {
     throw new AppError('RATE_LIMITED', `You already have ${open.length} contributions waiting for review.`, { limit: 'open', max: maxOpen }, OPEN_RETRY_S);
   }
+  const perDay = limit(c.env.MAX_PRS_PER_DAY, DEFAULT_MAX_PRS_PER_DAY);
+  const key = dayKey(identity, now);
+  const sentToday = Number((await kv.get(key)) ?? 0);
+  if (sentToday >= perDay) {
+    throw new AppError('RATE_LIMITED', `You can send up to ${perDay} contributions a day.`, { limit: 'daily', max: perDay }, secondsToMidnight(now));
+  }
+  await kv.put(key, String(sentToday + 1), { expirationTtl: DAY_TTL_S });
+  return {
+    async rollback() {
+      const current = Number((await kv.get(key)) ?? 0);
+      await kv.put(key, String(Math.max(0, current - 1)), { expirationTtl: DAY_TTL_S });
+    },
+  };
+}
+
+/** Link checks reach other servers on the reader's behalf, so they are capped per identity too. */
+export async function countLinkCheck(kv: KVNamespace, identity: Identity, now = new Date()): Promise<void> {
+  const key = probeKey(identity, now);
+  const used = Number((await kv.get(key)) ?? 0);
+  if (used >= MAX_LINK_CHECKS_PER_DAY) {
+    throw new AppError('RATE_LIMITED', 'Too many link checks today.', { limit: 'link-checks', max: MAX_LINK_CHECKS_PER_DAY }, secondsToMidnight(now));
+  }
+  await kv.put(key, String(used + 1), { expirationTtl: DAY_TTL_S });
 }
 
-/** Count a pull request that was opened. */
-export async function recordContribution(kv: KVNamespace, identity: Identity, pullRequest: string, now = new Date()): Promise<void> {
-  const sentToday = Number((await kv.get(dayKey(identity, now))) ?? 0);
-  await kv.put(dayKey(identity, now), String(sentToday + 1), { expirationTtl: DAY_TTL_S });
+/** Count a pull request that was opened and is waiting for review. */
+export async function recordContribution(kv: KVNamespace, identity: Who, pullRequest: string): Promise<void> {
   const open = (await kv.get<string[]>(openKey(identity), 'json')) ?? [];
-  await kv.put(openKey(identity), JSON.stringify([...new Set([...open, pullRequest])]), { expirationTtl: OPEN_TTL_S });
+  await kv.put(openKey(identity), JSON.stringify([...new Set([...open, pullKey(pullRequest)])]), { expirationTtl: OPEN_TTL_S });
 }
 
 /** Free an open slot when a contribution pull request closes (merged or not), using the body's identity marker. */
 export async function releaseContribution(kv: KVNamespace, body: unknown, pullRequest: string): Promise<boolean> {
-  const match = typeof body === 'string' ? /<!-- contribute-identity:(github|orcid):([\w-]{1,64}) -->/.exec(body) : null;
+  const match = typeof body === 'string' ? /<!-- contribute-identity:(github|orcid):([\w-]{1,64}) -->\s*$/.exec(body) : null;
   if (!match?.[1] || !match[2]) return false;
   const identity = { provider: match[1] as Identity['provider'], id: match[2] };
   const open = (await kv.get<string[]>(openKey(identity), 'json')) ?? [];
-  if (!open.includes(pullRequest)) return false;
-  await kv.put(openKey(identity), JSON.stringify(open.filter((item) => item !== pullRequest)), { expirationTtl: OPEN_TTL_S });
+  if (!open.includes(pullKey(pullRequest))) return false;
+  await kv.put(openKey(identity), JSON.stringify(open.filter((item) => item !== pullKey(pullRequest))), { expirationTtl: OPEN_TTL_S });
   return true;
 }

src/contrib/reachability.ts

@@ -1,20 +1,41 @@
 import { AppError } from '../errors';
+import { assertResourceUrl } from './safety';
+
+const MAX_REDIRECTS = 3;
 
 /**
  * Spec 003 rule 3: a suggested link must answer. Sites that refuse bots (401/403/429) still exist and pass; 404, 410,
- * 5xx, timeouts and network errors do not.
+ * 5xx, timeouts and network errors do not. Redirects are followed by hand and every hop must again be a public https
+ * address, so a link cannot bounce the probe to http, localhost or an IP address.
  */
 export async function assertReachable(url: string, fetcher: typeof fetch, timeoutMs = 5000): Promise<void> {
-  const attempt = (method: 'HEAD' | 'GET') =>
-    fetcher(url, { method, redirect: 'follow', headers: { 'user-agent': 'ipl-contribute (+https://github.com/InternetPublicLibrary/contribute)' }, signal: AbortSignal.timeout(timeoutMs) });
-  let status: number;
-  try {
-    let response = await attempt('HEAD');
-    if (response.status === 405 || response.status === 501) response = await attempt('GET');
-    status = response.status;
-  } catch {
-    status = 0;
+  const unreachable = (status: number) =>
+    new AppError('VALIDATION_FAILED', 'The link does not answer. Check the address and try again.', { fields: ['url'], status });
+  const attempt = (target: string, method: 'HEAD' | 'GET') =>
+    fetcher(target, {
+      method,
+      redirect: 'manual',
+      headers: { 'user-agent': 'ipl-contribute (+https://github.com/InternetPublicLibrary/contribute)' },
+      signal: AbortSignal.timeout(timeoutMs),
+    });
+
+  let target = url;
+  for (let hop = 0; hop <= MAX_REDIRECTS; hop += 1) {
+    assertResourceUrl(target);
+    let response: Response;
+    try {
+      response = await attempt(target, 'HEAD');
+      if (response.status === 405 || response.status === 501) response = await attempt(target, 'GET');
+    } catch {
+      throw unreachable(0);
+    }
+    const status = response.status;
+    if (status >= 300 && status < 400 && response.headers.has('location')) {
+      target = new URL(response.headers.get('location') ?? '', target).href;
+      continue;
+    }
+    if ((status >= 200 && status < 300) || [401, 403, 429].includes(status)) return;
+    throw unreachable(status);
   }
-  if ((status >= 200 && status < 400) || [401, 403, 429].includes(status)) return;
-  throw new AppError('VALIDATION_FAILED', 'The link does not answer. Check the address and try again.', { fields: ['url'], status });
+  throw unreachable(310);
 }