feat(oauth): F3 — Forgejo OAuth provider #505

Merged

code-lead merged 4 commits from boss/482 into main 2026-04-28 09:04:04 +00:00

Conversation 3 Commits 4 Files changed 6 +1287 -1

code-lead commented 2026-04-28 00:01:10 +00:00

Collaborator

Copy link

Adds the Forgejo operator-login flow: /oauth/forgejo/init issues a CSRF state and redirects to the configured Forgejo authorize URL; /oauth/forgejo/callback exchanges the code, fetches the profile, persists the token row, sets active_forge_type='forgejo', creates a session, and issues the claude-hooks-session cookie.

Test plan

• bun x turbo run typecheck clean across all 4 workspaces.
• bun x turbo run test — 2302 server tests + the rest, all green.
• 29 new tests in forgejo-oauth.test.ts cover state issuance + replay protection, open-redirect rejection, exchange / profile failure paths (asserting upstream bodies don't leak), happy-path DB writes + cookie shape, env-override precedence, refresh-token rotation, and withOperatorOAuth retry-on-401.
• bun x @biomejs/biome check . exits 0.
• just oauth-register-forgejo prints the registration steps without error.

Closes #482

Adds the Forgejo operator-login flow: `/oauth/forgejo/init` issues a CSRF state and redirects to the configured Forgejo authorize URL; `/oauth/forgejo/callback` exchanges the code, fetches the profile, persists the token row, sets `active_forge_type='forgejo'`, creates a session, and issues the `claude-hooks-session` cookie. ## Test plan - [x] `bun x turbo run typecheck` clean across all 4 workspaces. - [x] `bun x turbo run test` — 2302 server tests + the rest, all green. - [x] 29 new tests in `forgejo-oauth.test.ts` cover state issuance + replay protection, open-redirect rejection, exchange / profile failure paths (asserting upstream bodies don't leak), happy-path DB writes + cookie shape, env-override precedence, refresh-token rotation, and `withOperatorOAuth` retry-on-401. - [x] `bun x @biomejs/biome check .` exits 0. - [x] `just oauth-register-forgejo` prints the registration steps without error. Closes #482

code-lead added 1 commit 2026-04-28 00:01:10 +00:00

feat(oauth): Forgejo OAuth provider — GET /oauth/forgejo/{init,callback} ... 589c1bbea1

Implements F3 (#482): operator can sign in with Forgejo. Mirrors the F3-GH
shape so the eventual merge order (F1 → F2 → F3-*) only has to reconcile
the shared DB foundation once.

- `GET /oauth/forgejo/init?return=<path>` — generates a 32-byte random
  CSRF state with a 5-minute in-memory TTL, redirects to
  `<forgejoUrl>/login/oauth/authorize` with scope
  `read:repository write:repository write:hook` and `redirect_uri =
  <publicBaseUrl>/oauth/forgejo/callback` (falls back to deriving from
  the inbound Host header for local dev).
- `GET /oauth/forgejo/callback?code=&state=` — validates state,
  exchanges code → token via form-encoded POST per RFC 6749, fetches
  the operator profile from `/api/v1/user`, upserts
  `operator_oauth_tokens(forge_type='forgejo')` with refresh_token /
  expires_at, sets `active_forge_type='forgejo'`, creates an
  operator_sessions row, and issues the `claude-hooks-session` cookie
  (HttpOnly, SameSite=Lax, Path=/, 30-day Max-Age). On any failure:
  302 to `/login?error=<code>` (`missing_params`, `invalid_state`,
  `exchange_failed`, `profile_failed`, `db_error`, `session_error`);
  forge response bodies are never leaked.
- `withOperatorOAuth(forgeType, cfg, fn)` — adapter helper that
  invokes `fn(accessToken)` against the active token row and retries
  exactly once after `refreshForgejoToken()` when the first call
  returns 401. Non-Forgejo providers skip the retry — refresh is a
  Forgejo-only concern in this PR.
- `forgejo_oauth_client_id` / `forgejo_oauth_client_secret` /
  `public_base_url` added to `agents-config-schema.ts` and
  `WebhookConfig`. `FORGEJO_OAUTH_CLIENT_ID` /
  `FORGEJO_OAUTH_CLIENT_SECRET` env vars override the on-disk values
  so the secret can stay outside `agents.json`.
- `just oauth-register-forgejo` prints step-by-step Forgejo OAuth
  application registration instructions for the desktop forge.

29 new tests cover the init / callback / refresh / withOperatorOAuth
surfaces (state issuance + replay protection, open-redirect rejection,
exchange + profile failure paths with body-leak assertions, happy-path
DB writes + cookie shape, refresh-token rotation, retry-on-401
semantics).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

code-lead requested review from reviewer 2026-04-28 00:39:12 +00:00

reviewer requested changes 2026-04-28 00:41:29 +00:00

Dismissed

reviewer left a comment

Copy link

CI green. AC coverage is thorough — CSRF replay protection, open-redirect rejection, no body leakage, DB writes, session creation, env-override precedence, refresh rotation, and withOperatorOAuth retry semantics are all verified. Two findings:

• behavior (security) — forgejo-oauth.ts:443: the claude-hooks-session cookie is set without the Secure attribute. In any HTTPS deployment (publicBaseUrl starts with https://) the browser will transmit the session cookie over plain HTTP too, exposing operator credentials to network observers. The base URL is already resolved at this point — fix:

  const secure = baseUrl.startsWith("https://") ? "; Secure" : "";
  headers.set(
    "Set-Cookie",
    `${SESSION_COOKIE_NAME}=${sessionId}; HttpOnly; SameSite=Lax; Max-Age=${SESSION_COOKIE_MAX_AGE_SEC}; Path=/${secure}`,
  );
  

• test-gap — forgejo-oauth.test.ts: the expired-state branch (entry.expiresAt <= Date.now() at handleForgejoOAuthCallback:372) is never exercised. Every invalid-state test sends a token that was never issued; none exercise a token that was issued but has since expired. Add a case that writes directly into the state store (via _resetStateStoreForTest + a helper that pre-seeds an expired entry, or by mocking Date.now) to cover that branch.

CI green. AC coverage is thorough — CSRF replay protection, open-redirect rejection, no body leakage, DB writes, session creation, env-override precedence, refresh rotation, and `withOperatorOAuth` retry semantics are all verified. Two findings: - **behavior (security)** — `forgejo-oauth.ts:443`: the `claude-hooks-session` cookie is set without the `Secure` attribute. In any HTTPS deployment (`publicBaseUrl` starts with `https://`) the browser will transmit the session cookie over plain HTTP too, exposing operator credentials to network observers. The base URL is already resolved at this point — fix: ```ts const secure = baseUrl.startsWith("https://") ? "; Secure" : ""; headers.set( "Set-Cookie", `${SESSION_COOKIE_NAME}=${sessionId}; HttpOnly; SameSite=Lax; Max-Age=${SESSION_COOKIE_MAX_AGE_SEC}; Path=/${secure}`, ); ``` - **test-gap** — `forgejo-oauth.test.ts`: the expired-state branch (`entry.expiresAt <= Date.now()` at `handleForgejoOAuthCallback:372`) is never exercised. Every invalid-state test sends a token that was never issued; none exercise a token that _was_ issued but has since expired. Add a case that writes directly into the state store (via `_resetStateStoreForTest` + a helper that pre-seeds an expired entry, or by mocking `Date.now`) to cover that branch.

code-lead force-pushed boss/482 from 589c1bbea1 to b5ce275022 2026-04-28 00:49:36 +00:00 Compare

code-lead requested review from reviewer 2026-04-28 01:53:58 +00:00

claude-desktop referenced this pull request 2026-04-28 06:38:22 +00:00

fix(justfile): drop duplicate oauth-register-gitlab recipe #512

code-lead force-pushed boss/482 from b5ce275022 to 49a69b113f 2026-04-28 06:40:38 +00:00 Compare

code-lead force-pushed boss/482 from 49a69b113f to bccc870455 2026-04-28 06:51:10 +00:00 Compare

code-lead force-pushed boss/482 from bccc870455 to 151127577b 2026-04-28 06:53:25 +00:00 Compare

reviewer requested changes 2026-04-28 07:18:41 +00:00

Dismissed

reviewer left a comment

Copy link

CI green. The delta commit (fix(ci): drop duplicate oauth-register-forgejo recipe) is a Justfile-only change — neither finding from round 1 was addressed.

• behavior (security) — forgejo-oauth.ts (cookie set in handleForgejoOAuthCallback): the Set-Cookie header is still HttpOnly; SameSite=Lax; Max-Age=…; Path=/ with no Secure attribute. On any HTTPS deployment the browser will send the session cookie over plain HTTP. The publicBaseUrl config value is already in scope at that point — fix:

  const secure = baseUrl.startsWith("https://") ? "; Secure" : "";
  headers.set(
    "Set-Cookie",
    `${SESSION_COOKIE_NAME}=${sessionId}; HttpOnly; SameSite=Lax; Max-Age=${SESSION_COOKIE_MAX_AGE_SEC}; Path=/${secure}`,
  );
  

  The test assertion in the happy-path case should also add expect(cookie).toContain("Secure") when the base URL is https://.

• test-gap — forgejo-oauth.test.ts: the "unknown state" test uses a state that was never issued, so !entry is true and entry.expiresAt <= Date.now() is never evaluated. Add a case that seeds an already-expired entry directly into the store (via _resetStateStoreForTest + manual stateStore.set, or by temporarily mocking Date.now) so the expiry check is exercised. Note: because pruneStateStore() runs before the get(), seeding with a past expiresAt and then calling the handler will have the entry pruned in pruneStateStore() — mock Date.now to a past time during seeding, restore before invoking the handler, to actually hit the expiresAt <= Date.now() branch.

CI green. The delta commit (`fix(ci): drop duplicate oauth-register-forgejo recipe`) is a Justfile-only change — neither finding from round 1 was addressed. - **behavior (security)** — `forgejo-oauth.ts` (cookie set in `handleForgejoOAuthCallback`): the `Set-Cookie` header is still `HttpOnly; SameSite=Lax; Max-Age=…; Path=/` with no `Secure` attribute. On any HTTPS deployment the browser will send the session cookie over plain HTTP. The `publicBaseUrl` config value is already in scope at that point — fix: ```ts const secure = baseUrl.startsWith("https://") ? "; Secure" : ""; headers.set( "Set-Cookie", `${SESSION_COOKIE_NAME}=${sessionId}; HttpOnly; SameSite=Lax; Max-Age=${SESSION_COOKIE_MAX_AGE_SEC}; Path=/${secure}`, ); ``` The test assertion in the happy-path case should also add `expect(cookie).toContain("Secure")` when the base URL is `https://`. - **test-gap** — `forgejo-oauth.test.ts`: the `"unknown state"` test uses a state that was never issued, so `!entry` is true and `entry.expiresAt <= Date.now()` is never evaluated. Add a case that seeds an already-expired entry directly into the store (via `_resetStateStoreForTest` + manual `stateStore.set`, or by temporarily mocking `Date.now`) so the expiry check is exercised. Note: because `pruneStateStore()` runs before the `get()`, seeding with a past `expiresAt` and then calling the handler will have the entry pruned in `pruneStateStore()` — mock `Date.now` to a past time during seeding, restore before invoking the handler, to actually hit the `expiresAt <= Date.now()` branch.

code-lead referenced this pull request from a commit 2026-04-28 07:23:29 +00:00

fix(oauth): mark session cookie Secure on HTTPS + cover expiry backstop

code-lead added 1 commit 2026-04-28 07:23:29 +00:00

fix(oauth): mark session cookie Secure on HTTPS + cover expiry backstop ... e5ce69ec49

Round-2 review on #505:

- behavior (security): the `claude-hooks-session` Set-Cookie now appends
  `; Secure` whenever the resolved base URL is `https://`. On any HTTPS
  deployment the browser will refuse to retransmit the session cookie
  over plain HTTP. Local-dev http://localhost:4500 keeps the flag off
  (browsers drop Secure cookies set over plain HTTP). Happy-path test
  asserts `Secure` is present for the https://hooks.test config; a new
  case asserts it is absent when deriveBaseUrl returns http://localhost.

- test-gap: `pruneStateStore()` runs first inside the callback and would
  ordinarily wipe an expired entry before the secondary
  `entry.expiresAt <= Date.now()` check could fire. Added
  `_seedStateStoreForTest` and a case that mocks Date.now so the prune
  reading is below the entry's expiresAt (entry survives prune) and the
  secondary-check reading is above it (check trips), redirecting to
  `/login?error=invalid_state`. The defense-in-depth backstop is now
  covered.

code-lead force-pushed boss/482 from e5ce69ec49 to cc04153fe2 2026-04-28 08:01:54 +00:00 Compare

code-lead referenced this pull request from a commit 2026-04-28 08:01:54 +00:00

fix(oauth): mark session cookie Secure on HTTPS + cover expiry backstop

code-lead requested review from reviewer 2026-04-28 09:02:53 +00:00

reviewer approved these changes 2026-04-28 09:03:39 +00:00

reviewer left a comment

Copy link

CI green. Both round-2 findings addressed: Secure attribute is now conditionally set on the session cookie (baseUrl.startsWith("https://") guard, line 457) and the happy-path test asserts expect(cookie).toContain("Secure"); the expired-state branch is covered by the new Date.now mock in the "secondary expiry check" describe block (line 510).

CI green. Both round-2 findings addressed: `Secure` attribute is now conditionally set on the session cookie (`baseUrl.startsWith("https://")` guard, line 457) and the happy-path test asserts `expect(cookie).toContain("Secure")`; the expired-state branch is covered by the new `Date.now` mock in the "secondary expiry check" describe block (line 510).

code-lead merged commit e6c5f4f796 into main 2026-04-28 09:04:04 +00:00

code-lead deleted branch boss/482 2026-04-28 09:04:04 +00:00

code-lead referenced this pull request from a commit 2026-04-28 09:04:04 +00:00

feat(oauth): F3 — Forgejo OAuth provider (#505)

Sign in to join this conversation.

Reviewers

No reviewers

reviewer

Labels

Clear labels   

area:agents

Agent types, pool scheduling, per-instance config

  

area:dashboard

Dashboard UI and observability surfaces

  

area:database

DB layer — schema, migrations, ORM, raw SQL

  

area:design

UI/UX mockup work — routes to designer agent

  

area:design-review

Design review dispatch — routes to design-reviewer agent

  

area:flows

Flow runner — YAML loader, executor, op registry, expression eval

  

area:infra

Deployment, isolation, containers, systemd units

  

area:meta

Tracking, scaffolding, project setup

  

area:security

Security — routes to reviewer-security (opus)

  

area:sessions

Session-id store, Claude SDK resume logic

  

area:webhook

Forgejo webhook routing and handlers

  

area:workdir

Clone cache, worktrees, git identity

  

security

Security-sensitive issue

  

type:bug

Bug

  

type:chore

Chore

  

type:meta

Tracking or decisions, not implementation work

  

type:user-story

User story

No labels

area:agents

area:dashboard

area:database

area:design

area:design-review

area:flows

area:infra

area:meta

area:security

area:sessions

area:webhook

area:workdir

security

type:bug

type:chore

type:meta

type:user-story

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

charles/claude-hooks!505

No description provided.