Adds RFC 8414 (oauth-authorization-server) and RFC 9728 (oauth-
protected-resource) metadata documents.
Both URLs are derived from cfg.Issuer at construction time, never from
inbound request headers. Test TestDiscovery_IssuerIgnoresHostHeader
explicitly probes this — a malicious Host: evil.example.com value must
not leak into the published metadata. Defense against the OAuth
metadata-spoofing class starts at the discovery layer.
Capabilities published reflect the actual OAuth surface:
- response_types_supported = ["code"]
- grant_types_supported = ["authorization_code", "refresh_token"]
- code_challenge_methods_supported = ["S256"] (PKCE only, no plain)
- token_endpoint_auth_methods_supported = ["none"] (PKCE-only public clients)
Protected-resource metadata advertises /mcp as the resource; phase 5
will mount the gated MCP endpoint there.
Closes forgejo-mcp-broker-b2o.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements internal/oauth, the broker's OAuth 2.1 AS surface that
Claude.ai (and other MCP clients) talk to. User authentication is
delegated to upstream Forgejo via internal/forgejo.
Endpoints:
POST /oauth/register — RFC 7591 dynamic client registration
GET /oauth/authorize — RFC 6749 + 7636 PKCE (S256 only)
GET /oauth/callback — Forgejo redirects back here after consent
POST /oauth/token — authorization_code + refresh_token grants
POST /oauth/revoke — RFC 7009
Security model:
- PKCE required, S256 only — plain method rejected per OAuth 2.1
- Every broker-issued access/refresh token stored as hex(sha256(plain));
plaintext leaves the broker exactly once in the /token response body
- Refresh-token rotation: each refresh issues a new token pair and
revokes the old refresh (RFC 6749 §10.4)
- Auth-code single-use enforced atomically via UPDATE...WHERE used_at IS
NULL with rows-affected check, blocking the concurrent-replay race
- Issuer URL sourced from cfg.Issuer at construction time, never from
inbound headers — prevents host-header injection on /.well-known
metadata that ships in 2d
- redirect_uri must match a registered URI exactly (no prefix/wildcard)
Pending-authorization state (between /authorize and /callback) lives in
an in-memory sync.Map with a 1-minute reaper goroutine. A broker restart
drops them, forcing the user to re-authorize — acceptable trade-off
versus introducing a fifth table.
Tests: 81.0% coverage with ~20 cases across happy paths, every required-
field error, PKCE failure, code-replay, refresh expiry/revocation,
client-id and redirect-uri mismatches, Forgejo-side errors, and the
reaper logic itself (internal test).
Closes forgejo-mcp-broker-pur. The OAuth keystone is in place; phase 2c
unblocks discovery (2d) and security review (2e), and combined with the
existing supervisor + bridge it unblocks the session glue work in
phase 5.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Spawns a real forgejo-mcp --transport stdio subprocess, runs the
canonical MCP handshake (initialize → notifications/initialized →
tools/list → tools/call get_forgejo_mcp_server_version) through the
bridge, and verifies each step. Validates that the opaque-pipe
assumption holds end-to-end: every JSON-RPC message round-trips
correctly with no hand-rolled framing surprises.
Binary discovery (skipped if none found):
1. $FORGEJO_MCP_BIN
2. ../../../forgejo-mcp/forgejo-mcp (sibling-repo built binary)
3. go build of ../../../forgejo-mcp into a temp dir
Fake Forgejo (httptest.Server) covers the two probes the SDK and
forgejo-mcp's testConnection do at startup:
- GET /api/v1/version → returns 11.0.0 (>=1.11.0 satisfies SDK gate)
- GET /api/v1/user → returns a minimal authenticated user blob
Skipped under -short. Catch-all 404 handler logs unexpected probes so
new SDK or forgejo-mcp behaviour surfaces clearly in test output.
Closes forgejo-mcp-broker-xot. Phase 4 complete.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds two stress tests:
TestStress_NoLeaksAcross1000Cycles — spawns and reaps 1000 children
in sequence, asserts FD count, goroutine count, and zombie status are
all stable.
TestStress_StopMidLifecycle — 200 cycles that exercise the Stop path
(SIGTERM via Close+Signal) rather than relying on natural exit.
Bypassed by -short for the unit-test inner loop.
Notable findings:
* Using the helper-process pattern at this scale was a dead end. Each
spawn re-execs the test binary, which inherits the parent's open FDs
and runs Go's `testing` package init. Past a few hundred cycles the
inner test binaries drag delivery of EOF on their inherited stderr
pipe ends, leaving drainStderr goroutines blocked in bufio.ReadString
even after Wait returned. Replacing the helper with /bin/true (for
quick-exit) and /bin/cat (for echo-loop) sidesteps the recursion and
is closer to the production case anyway: the broker spawns
forgejo-mcp, not itself.
* Defensively close stdout/stderr handles in supervisor's reap goroutine
after cmd.Wait returns. cmd.StderrPipe is supposed to be closed by
Wait, but under load the kernel doesn't always deliver EOF promptly
through Go 1.26's pidfd-based wait path; an explicit Close ensures
drainStderr exits and FDs aren't held longer than needed.
Tests pass under -race with FD/goroutine deltas in single digits across
1000+200 cycles, and Wait4(-1) confirms no zombie children.
Closes forgejo-mcp-broker-31t. Phase 3 complete.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds internal/bridge: connects HTTP-side MCP clients to a stdio-side
child via JSON-RPC framing. Decoupled from internal/supervisor — takes
io.Writer + *bufio.Reader + done channel directly so it tests cleanly
with io.Pipe pairs and could later wrap something other than a child
process.
Routing model: one reader goroutine consumes child stdout line-by-line.
Each line is parsed only enough to extract the JSON-RPC `id` field
(string/number/null kept as raw JSON, so `1` and `"1"` don't collide).
HTTP requests register a per-id waiter channel before forwarding their
body to the child; the reader delivers the response to whichever waiter
matches. Concurrent in-flight requests are safe; a duplicate id while
the first is still pending returns 409.
HandleSSE response shapes:
- request with id + child reply → 200 text/event-stream, one
`event: message` SSE event carrying the JSON-RPC response
- request without id (notification) → 204 No Content (no waiter
needed; MCP notifications are fire-and-forget)
- empty body → 400
- duplicate in-flight id → 409
- send-to-child fails → 502
- client disconnect mid-wait → bridge cleans up its waiter; child
keeps running, other in-flight requests unaffected
- child exits before reply → SSE `error` event with reason=child_exited
Tests cover all of the above plus stale unsolicited replies, malformed
lines from the child, and reader robustness across both. 90.0%
coverage. The remaining gap is splitLines' empty-data branch (only
reachable if the child sends a literal `\n` line).
Closes forgejo-mcp-broker-am1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds internal/supervisor: a thin wrapper around os/exec that handles the
zombie/leak/escalation concerns once, so phase-4 (bridge) and phase-5
(session glue) don't each have to re-derive them.
Lifecycle (Stop):
1. Close stdin — well-behaved stdio servers exit on EOF
2. Send SIGTERM
3. Wait up to StopGrace (default 5s) for exit
4. SIGKILL if still alive
Reaping is mandatory: a goroutine calls cmd.Wait so the kernel actually
collects the child. Without it you accumulate zombies under N concurrent
sessions. Tests exercise this via the helper-process pattern (TestMain
re-execs the test binary in helper mode) — no shell or external binary
dependency.
Tests cover: empty Cmd validation, missing-binary error, echo round
trip via stdin/stdout, stderr drainer collecting lines, SIGTERM-friendly
graceful stop, SIGTERM-ignoring child escalating to SIGKILL (with a
ready-on-stdout sync barrier so the test isn't racing the helper's
signal.Notify), idempotent Stop, clean exit detection, non-zero exit
detection, env override propagation. 89.6% coverage; remaining gap is
unreachable-from-public-API defensive branches (pipe-creation failures
under FD exhaustion, post-release Pid).
Manual smoke test against a real `forgejo-mcp --transport stdio` is
deferred to phase 4b's integration test (where it adds the most value).
Closes forgejo-mcp-broker-zuq.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds internal/forgejo: a stateless OAuth 2.1 client for upstream Forgejo.
Covers what the broker AS needs:
- AuthorizeURL: builds the user-agent redirect to /login/oauth/authorize
- ExchangeCode: code → access+refresh tokens (PKCE verifier included)
- Refresh: refresh_token grant (Forgejo rotates the refresh token)
- FetchUserInfo: OIDC userinfo claims (sub, preferred_username, etc.)
OAuth errors come back as a structured *forgejo.Error so the AS can
distinguish "user must re-authenticate" (invalid_grant) from "transient
network problem" via errors.As. Forgejo doesn't currently expose a token
revocation endpoint, so revocation lives in the broker's own store —
upstream tokens expire naturally.
Defaults:
- 30s HTTP timeout (Forgejo OAuth is sub-second when healthy)
- User-Agent "fjmcp-broker" if not overridden
- 64 KiB cap on response bodies (these endpoints return ~kilobytes)
Tests: 95.1% coverage. httptest.Server fake Forgejo exercises every
public method, every error shape (OAuth-formatted, plain {"message":...},
malformed JSON, missing required fields, network failure), and verifies
form params hit the wire as expected.
Closes forgejo-mcp-broker-b9i.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds migrations/0002_oauth_tables.sql per design.md §4.2: clients,
auth_codes, access_tokens, refresh_tokens. Cascading foreign keys
guarantee that revoking a client tears down every dependent row, and
that a refresh token can never outlive its access token.
Storage choices:
- Broker access/refresh tokens stored as hex-encoded SHA-256 hashes;
plaintext leaves the broker exactly once (when handed to the MCP
client). Lookups by hash are O(log n) via the PK index.
- Forgejo tokens stored cleartext (subprocess spawning needs them).
At-rest protection is the volume permissions + optional encrypted
volume; application-layer encryption is tracked as backlog item -sd4.
- Timestamps are unix epoch INTEGERs, set by the application — keeps
deadline comparisons trivial and lets phase 5c inject a test clock.
- Tables are not STRICT to stay consistent with the phase-1 broker_meta
table; converting both is a future cleanup if we want it.
Tests verify column sets via PRAGMA table_info, expected indexes are
present, the FK CASCADE works in both directions (client → tokens, and
access_token → refresh_token), and the oauth_schema_version marker is
written. Existing migration-count assertions parameterised on
embeddedMigrationCount so adding a third migration only needs that
constant bumped.
Closes forgejo-mcp-broker-cpb.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Populate the Build & Test, Architecture Overview, and Conventions
& Patterns sections of CLAUDE.md with context captured during the
phase-1 implementation work. Highlights the non-obvious gotchas
(http.Server.Shutdown not interrupting active conns; bd link
direction; bd init auto-committing) so future sessions don't
re-discover them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Final phase-1 step: the broker now starts. run() parses config, opens
the store, builds the httpserver, and blocks on signal.NotifyContext
until SIGTERM/SIGINT fires, at which point it drains through
httpserver.Run's graceful-shutdown path and closes the store.
--version is handled before config.Load so operators can inspect a
binary without providing the rest of the config. flag.ErrHelp is passed
through so -h exits 0. Config failure exits 2; runtime failure exits 1.
Integration tests build the binary once in TestMain and exercise three
acceptance scenarios against it:
- --version: prints build info, exits 0
- no config: exits nonzero with stderr listing every missing field
- full startup: /healthz returns 200 with correct JSON; SIGTERM triggers
clean exit within 2s
Closes forgejo-mcp-broker-t37. Phase 1 complete.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements internal/httpserver and internal/log.
httpserver (forgejo-mcp-broker-8ei):
- Server struct owns the HTTP lifecycle; Run(ctx) blocks, Handler() returns
the composed handler for unit tests
- GET /healthz returns JSON with status, version, git_revision, build_date,
and store probe result. Returns 503 when the store reports unhealthy
- Signal handling delegated to the caller via ctx cancellation — main wires
signal.NotifyContext, httpserver just responds to Done()
- Graceful shutdown with a configurable deadline (default 10s). When the
deadline expires, falls back to http.Server.Close() so lingering
connections are forcibly terminated — http.Server.Shutdown alone never
interrupts active connections
- ExtraHandler extension point for the OAuth + MCP routes that land in
phase 2 and phase 5, so the server doesn't need to be re-plumbed later
log:
- Small slog wrapper: New(w, debug) returns a JSON logger that stamps every
record with service/version/git_rev for correlation across deployments
- Discard() helper for tests
Tests: 97.9% coverage on httpserver (all health states, wrong-method,
ExtraHandler dispatch, ctx-cancel shutdown, shutdown-deadline force-close
of hanging requests, missing-field errors), 100% on log.
Closes forgejo-mcp-broker-8ei.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements internal/store on top of modernc.org/sqlite (pure-Go, no CGO).
Open applies any pending migrations, Close releases the handle, Ping
underpins /healthz.
Migration design:
- Files embedded via embed.FS under migrations/NNNN_name.sql
- schema_migrations table tracks applied versions; re-open is a no-op
- Each migration runs in its own transaction: no partial commits
- loadMigrations takes an fs.FS so tests can inject synthetic migration
sets to exercise rollback and conflict paths
Connection pragmas (set via DSN so they apply to every pooled conn):
- journal_mode=WAL — better reader/writer concurrency
- foreign_keys=ON — off by default in SQLite, we always want them
- busy_timeout=5000 — absorb brief contention without surfacing SQLITE_BUSY
- synchronous=NORMAL — standard WAL pairing
Phase 1 schema (0001_initial.sql) is minimal: a broker_meta table with a
schema_version row. Real OAuth tables ship in phase 2.
Tests: 90.1% coverage across public API and internal migration runner,
including bad SQL rollback, PK-conflict record-step failure, and scan
errors on malformed schema_migrations rows.
Closes forgejo-mcp-broker-9jh.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements internal/config. Flags override env; empty env values are treated
as unset so exported-but-empty vars don't clobber defaults. Validation
aggregates every problem via errors.Join so operators see the full list
at once, not one at a time.
Notable decisions:
- Issuer URL validation requires http/https and rejects plain http on
non-loopback hosts (classic OAuth misconfig). Loopback http allowed so
local dev doesn't need TLS.
- Store-path writability probed via os.CreateTemp in the parent dir — a
real write test, not a string check. No side effects on the store file
itself (that's the storage layer's job).
- Public API: Load(args, out) returns *Config or error. flag.ErrHelp is
passed through so callers can exit 0 on -h.
Tests: 94.1% line coverage, covers env/flag precedence, empty-as-unset,
every required-field message, each URL failure mode, numeric bounds,
env-parse errors, and -help output.
Closes forgejo-mcp-broker-9nq.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Initialize go.mod with module path kode.naiv.no/olemd/forgejo-mcp-broker
- Create directory layout: cmd/broker + internal/{buildinfo,config,log,store,httpserver}
- Add Makefile with build/test/lint/tidy/clean targets and ldflags-injected build info
- Stub cmd/broker/main.go with --version support; real wiring follows in -t37
- Stub doc.go for each internal/* package, pointing to the issue that fills it in
Closes forgejo-mcp-broker-n84.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Establish project scope, architecture, and phased implementation plan
for an OAuth 2.1 broker that fronts forgejo-mcp, delegating user
authentication to Forgejo and spawning a per-session stdio
forgejo-mcp subprocess scoped to each authenticated user's token.
No code yet — planning only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
