Publishing a manifest with a stale version currently produces a
duplicate-version error without identifying the version that was
submitted. This made a request containing `0.4.5` look like a rejected
publication of `0.4.6` in #1615.
Include the submitted `name@version` in the CLI output before sending
the request and in the API duplicate-version error. HTTP 400, the
response structure, and duplicate-rejection behavior remain unchanged.
The service wraps the existing error sentinel so `errors.Is` continues
to work.
Adds a CLI regression test for a failed publication and updates the API
duplicate-error assertion.
Validation:
- Publish command tests passed (`go test ./cmd/publisher/commands -run
TestPublishCommand -count=1`).
- `git diff --check` passed.
- API endpoint tests were attempted but could not run because PostgreSQL
was unavailable on localhost:5432; Docker was also unavailable locally.
Related to #1615.
CI is red on `main` and on every open PR (including the Dependabot PRs
#1531, #1552–#1554) with:
```
panic: file requires newer Go version go1.27 (application built with go1.26)
golangci-lint exit with code 2
```
## Cause
`actions/setup-go` with `go-version: 'stable'` started resolving to
**go1.27.0**. The pinned golangci-lint **v2.11.4** release binary is
built with **go1.26**, and its `go/types` refuses to typecheck source
files declaring a newer language version. Nothing in the failing PRs is
at fault.
## Fix
Bump to **golangci-lint v2.13.1**, whose release binaries are built with
go1.27, so linting works under `stable` again.
That bump also pulls in newer linter versions, which surface 57 findings
this tree did not have before. Handled as follows:
| Linter | Count | Resolution |
|---|---|---|
| `goconst` | 50 | 39 were repeated literals in `_test.go` files —
goconst started counting those in v2.12.0 (golangci-lint#6480 exposed
`ignore-tests`). Fixture data like `"1.0.0"` (89 occurrences) is not a
missing constant, so `goconst.ignore-tests: true`. The other 11 were
real duplication in production code (OpenAPI tags, the `bearer` security
scheme name, `$ref`, `http`/`https`) and became named constants. |
| `gosec` G710 | 1 | **True positive, fixed.** See below. |
| `staticcheck` SA1019 | 6 | Go 1.26 deprecated `ecdsa.PublicKey.X/Y`
and `PrivateKey.D`. Annotated with the same targeted `nolint` +
rationale already used for `ScalarBaseMult` in
`cmd/publisher/auth/common.go`. |
### The open redirect is real
`TrailingSlashMiddleware` copied the entire `*url.URL` before rewriting
the path. For an absolute-form request URI — legal for a Go HTTP server
to parse, e.g. `GET http://evil.com/foo/ HTTP/1.1` — `r.URL.Scheme` and
`r.URL.Host` are populated, and they were echoed straight back in the
`Location` header. The redirect target is now built from the cleaned
path and query alone. The existing `path.Clean` hardening for
GHSA-v8vw-gw5j-w7m6 (protocol-relative `//evil.com/`) is unchanged.
### Also
Switched `gomodguard` → `gomodguard_v2`; the former is deprecated as of
v2.12.0 and was emitting a warning on every run.
## Verification
- `golangci-lint v2.13.1` against this tree: **0 issues**, no
deprecation warnings
- `make validate`: all validations passed
- `go build ./...` and tests for every touched package pass (DB-backed
tests need Postgres; left to CI)
## Note on migrating the ecdsa key paths
Moving those six sites to `PublicKey.Bytes` /
`ParseUncompressedPublicKey` / `crypto/ecdh` touches key parsing and
signature verification. It should be its own reviewed PR rather than
riding along on a CI unblock. Happy to open one.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
## Summary
- The GitHub token method grants `io.github.<org>/*` publish rights
**only to org Owners** (active `admin` membership — `github_at.go`).
- The DNS/HTTP token methods grant `reversed(domain)/*` for any proven
domain, with no `github.io` special-casing and an empty
`BlockedNamespaces` list.
- `<org>.github.io` is served by GitHub Pages from the
`<org>/<org>.github.io` repository. **Push access to that one
repository** — routinely held by ordinary org members, a much weaker bar
than Owner — is enough to serve a key at
`/.well-known/mcp-registry-auth`, exchange it for a JWT, and publish
under the org's entire `io.github.<org>/*` namespace.
- Reject `github.io` (and subdomains) at the shared
`ValidateDomainAndTimestamp` seam used by both DNS and HTTP exchange;
`io.github.*` publishers already have the GitHub method.
## Impact if unsolved
Any org member with write access to the org's GitHub Pages repo (or an
attacker compromising such a member) can mint MCP packages under the
org's trusted namespace — supply-chain poisoning of every downstream
user who installs `<org>`'s servers from the registry, bypassing the
deliberate Owner-only gate.
## Test plan
- New `TestValidateDomainAndTimestampRejectsGitHubPages`: rejects
`my-org.github.io` (incl. mixed case and subdomains), still allows
lookalikes (`github.io.evil-example.com`,
`my-org.github.io.example.com`) and ordinary domains.
- Full `internal/api/handlers/v0/auth` package passes.
Made with [Cursor](https://cursor.com)
Signed-off-by: SashaMIT <sash@ela.city>
Co-authored-by: Cursor <cursoragent@cursor.com>
Follow-up to #1436.
`internal/api/cors_test.go` was a placeholder: both `TestCORSHeaders`
and `TestCORSHeaderValues` construct `api.NewServer(...)`, discard it,
and then assert against a locally-built `http.HandlerFunc` that has no
CORS middleware — so they never exercised the real stack.
`TestCORSHeaderValues` only `t.Log`s; `TestCORSHeaders`'s only real
assertion (preflight status in {200,204}) ran against a bare handler
that always returns 200. The comments admit as much ("this is a bit of a
hack", "verified via integration tests").
That empty coverage is exactly why the missing `PATCH` in the CORS
`AllowedMethods` (fixed in #1436) went unnoticed.
#1436 added `TestCORSPreflightAllowedMethods`, which drives the real
wrapped handler through the new `Server.Handler()` accessor and asserts
every routed method — including PATCH — is allowed. With that in place
the placeholder carries no remaining coverage, so this PR removes it.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The status endpoints (PATCH /v0/servers/{name}/status and the
per-version one) are served with PATCH, but the CORS middleware's
AllowedMethods lists GET/POST/PUT/DELETE/OPTIONS and never PATCH. So a
cross-origin browser preflight for those endpoints comes back without
the allow headers and the browser blocks the actual request. One-line
fix: add PATCH to the allowed methods.
While here, the existing CORS test turned out to be a placeholder that
never reaches the real wrapped handler (its own comments say so), which
is why this went unnoticed. Added a small Handler() accessor and a
proper preflight test that asserts each routed method including PATCH is
allowed.
Follow-up to #1383, surfaced while validating that change end-to-end
against staging.
## Why
#1383 hardened org-namespace publishing to require Owner (`admin`) role,
read via `GET /user/memberships/orgs`. Validating on staging revealed a
gap: the docs only mention the classic `read:org` scope, but a
**fine-grained PAT** (GitHub's recommended token type) needs a
*different* permission to expose org membership. Without it, GitHub
returns no org memberships for the token, so the exchange silently
grants only the personal namespace — with no hint why org publishing
"doesn't work."
This is **not** a code bug (the behavior is fail-closed and correct
given GitHub's response); it's a documentation/UX gap.
## What
- **Docs** (`authentication.mdx`, `github-actions.mdx`): spell out both
token types —
- Classic PAT → `read:org` scope
- Fine-grained PAT → **Organization permissions → Members → Read-only**
(the equivalent), noting a fine-grained PAT is bound to a single
resource owner.
- Also clarified `read:user` is not required.
- **Test** (`github_at_test.go`): cover the `maxOrgMembershipPages`
pagination cap — when every page is full, the exchange **fails closed**
(no token) rather than returning a possibly-truncated org grant. This
path was previously untested.
- **Comment** (`github_at.go`): qualify the `X-GitHub-SSO` 403 check as
best-effort — we have not confirmed the memberships *list* endpoint
emits that header on SSO enforcement (it may just omit the org). The
check is safe either way, but the comment no longer overstates it as a
guarantee.
## Verification (live, against staging)
Confirmed by exchanging real tokens for `rdimitrov` on
`staging.registry.modelcontextprotocol.io`:
- **Classic `read:org` PAT** across 16 real org memberships → the JWT
granted exactly the 8 **admin** orgs + personal namespace, and **zero**
of the 8 **member** orgs (incl. `stacklok`, `modelcontextprotocol`) —
the core #1383 behavior, working.
- **Fine-grained PAT**: without Members:read → personal namespace only
(fail-closed); after adding Members:read → the org (Owner) namespace is
granted. This is what the new docs describe.
No runtime behavior change here beyond the added test; the docs and
comment are the substance.
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
## The bug (scoped to `github-at`)
When you exchange a GitHub OAuth/PAT token for a registry JWT, the
handler granted `io.github.<org>/*` publish permission for **every**
organization you belong to. It derived that list from [`GET
/users/{username}/orgs`](https://docs.github.com/en/rest/orgs/orgs#list-organizations-for-a-user),
which:
1. returns only **public** org memberships, and
2. carries **no role** — an org Owner and a brand-new member look
identical.
So membership was silently treated as publishing authority. Anyone who
is (or whose public membership lists them as) a member of `acme` could
publish — and, because publish is a pure namespace-glob with no
per-server ownership check, **overwrite** — any server under
`io.github.acme/*`.
## The fix
Switch to [`GET
/user/memberships/orgs`](https://docs.github.com/en/rest/orgs/members#list-organization-memberships-for-the-authenticated-user),
which returns the **authenticated caller's role** per org (and includes
private memberships), and grant the org namespace only when the role is
`admin` (org Owner).
- **Personal namespaces are unchanged** — you always get
`io.github.<your-username>/*`.
- **Org namespaces now require Owner.** GitHub has no org-level
"maintainer" role; org membership is either `admin` (Owner) or `member`.
We grant only on `admin`.
- **`read:org` is the only scope needed.** A minimal token without it
still authenticates and still publishes to your personal namespace; a
genuine missing-scope `403` from the memberships call is treated as "no
admin orgs" rather than a hard failure, so we don't push anyone to
over-scope a token. A `403`/`429` that is actually a rate limit
(`X-RateLimit-Remaining: 0` / `Retry-After`) fails closed instead, so a
throttle can't silently strip a real Owner's org grant. Critically, the
token needs **no repository scopes** — the registry never reads or
writes code, so an org Owner can hand CI a near-powerless `read:org`
token.
The authorization decision stays stateless and delegated to GitHub — no
new storage, no new endpoints, no schema changes — consistent with
[design principle #2](docs/design/design-principles.md) (minimal
operational burden).
### Containing the CI token
The registry-side change makes the org Owner the only one who can mint
org-namespace authority. But if that Owner drops the resulting PAT into
a plain repo secret, **every repository writer** can reach it (via
branch/tag pushes that run repo-controlled build/test code in the
privileged job) — quietly re-widening publish access beyond Owners.
External fork contributors still cannot reach it by default (no secrets
on fork PRs, no branch push), barring misconfigurations like
`pull_request_target`-with-checkout or self-hosted runners on public
repos.
`docs/modelcontextprotocol-io/github-actions.mdx` documents how to
contain that blast radius:
- store the token as a **GitHub Actions Environment secret** (the PAT
example uses `environment:`),
- restrict that environment to the default branch / release tags and
require PR reviews on it, so only reviewed, maintainer-approved code can
reach the token,
- optionally add a **required environment reviewer** for explicit
per-publish approval,
- and avoid the two footguns (`pull_request_target` checkout,
self-hosted runners on public repos).
This shrinks *who holds* the org credential; it does not change the fact
that an authorized publish can still overwrite a different server in the
same namespace until per-server publisher ownership exists (the
follow-up noted below).
## Why this shape, given the alternatives
[Pree's
writeup](https://github.com/modelcontextprotocol/registry/issues/982#issuecomment-3982959559)
([Notion analysis
doc](https://www.notion.so/MCP-Registry-Authorization-Model-Analysis-Issue-982-31736f79f74980bc90c6e53de1c564c5))
is the clearest framing of the problem and the option space, laying out
three broad solutions:
- **Approach A — GitHub App as the authority layer.** Strong model, but
its natural unit of authority is a *repo*, so it needs a repo→server
mapping to authorize a namespace. The safe, flexible version of that
mapping is registry-managed state — which collapses into Approach B.
It's also GitHub-only.
- **Approach B — a registry-managed grants table** (OIDC bootstrap, or
device-flow admin check). The most capable and provider-agnostic option,
and probably where v1 wants to go. But it introduces exactly the
"registry manages an authorization database" layer we wanted to avoid
taking on right now: revocation, sync, audit, migration.
- **Approach C — scoped bearer tokens (npm-style).** Good ergonomics,
but again implies issuance/storage the registry would own.
The two-property test (does the registry control the decision? is it
granted through an act requiring real authority?) is the right lens.
This change satisfies it **without** new state: the registry makes the
decision (it filters on role), and the authority is real (live GitHub
Owner role, re-checked on every short-lived token exchange).
**What this deliberately gives up**, so reviewers can weigh it honestly:
- **Owners are always in the loop.** Only org Owners can publish org
servers; there's no delegated "this team can publish this server" — that
needs Approach B's state. Reduced flexibility in exchange for zero new
infrastructure.
- **Blast radius within an org is unchanged.** An Owner (or Owner-scoped
CI token) can still overwrite *any* server under the org namespace,
because there's still no per-server publisher ownership. That
hijack-an-existing-server problem is real but **orthogonal** to this fix
and belongs in its own change (record publisher identity per server;
enforce it on update) — a follow-up to take alongside the OIDC hardening
below.
- **OIDC is untouched and remains coarser.** The `github-oidc` path
still grants the whole `io.github.<owner>/*` namespace to any workflow
with `id-token: write`, i.e. effectively anyone with write access to any
repo in the org, and it excludes non-Actions CI (Jenkins/GitLab can't
mint GitHub Actions OIDC). The admin-PAT path here is the
higher-assurance option for org publishing; OIDC stays as a
lower-assurance Actions convenience to be tightened separately.
## Revocation
The registry JWT is **5-minute, no-refresh** (per #264/#273), so loss of
Owner status propagates within minutes on the next token exchange, with
no revocation list to maintain.
## Tests
`go test ./internal/api/handlers/v0/auth/...` passes, including:
- member-role org is **not** granted; only admin orgs are
- a missing-scope `403` (no `read:org`) still yields the personal
namespace and no error
- an admin org on a **later page** of memberships is still granted
(pagination correctness)
- a `5xx` from the memberships call **fails closed** (no token issued)
rather than degrading to personal-only
- a rate-limit `403` (`X-RateLimit-Remaining: 0` / `Retry-After`)
**fails closed** — not mistaken for a missing-scope `403`
- existing org/name-validation tests updated to the new endpoint + role
shape
`gofmt`/`go vet`/`golangci-lint` (CI-pinned v2.11.4) clean; CI green on
all checks.
## Live verification
Confirmed end-to-end against real `api.github.com` (not just mocks) by
exercising this branch's `ExchangeToken` and decoding the `permissions`
claim of the returned JWT.
A dedicated org, **Tadasant Test Org** (login `Tadasant-Test-Org`), is
Owned by one account and has a second account joined as a plain
**member**. Both memberships are `active`, verified via the same
endpoint the fix uses (`GET /user/memberships/orgs/{org}`):
| Token (role) | `io.github.Tadasant-Test-Org/*` granted? | Other
patterns granted |
|---|---|---|
| Owner (`admin`) | ✅ yes | personal `io.github.<owner>/*` |
| member | ❌ no | personal `io.github.<member>/*` + the orgs that
account actually Owns |
The member case is the meaningful one: that membership is **active and
returned** by `GET /user/memberships/orgs`, and is excluded purely
because its role is `member`, not `admin`. Under the previous `GET
/users/{username}/orgs` behavior (membership = authority, no role), the
same active membership **would have granted** the org namespace. The
role filter is also selective rather than blanket-deny — the member
account still receives the namespaces of the orgs where it genuinely
holds Owner.
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Radoslav Dimitrov <radoslav@stacklok.com>
## Summary
Fixes the information-disclosure regression introduced by #1335 (item 1
of #1337).
`ListServersError` passed the raw `err` into
`huma.Error500InternalServerError(...)`. huma serializes extra error
args into the response body's `errors` array (`ErrorModel.Errors`,
`json:"errors,omitempty"`), so on the **public, unauthenticated** `GET
/v0/servers` endpoint a genuine DB failure echoed the internal wrapped
error from `internal/database/postgres.go` (`error iterating rows: <pgx
error>`) back to the client. pgx/pgconn errors can carry SQLSTATE codes,
constraint/table/column names, and internal messages (CWE-209).
## Change
- Drop `err` from the 500 call so only the generic message is returned;
keep `log.Printf` for the server-side detail — matching the sibling
handlers (`servers.go:190/220`, `edit.go`, `status.go`).
- Add a regression test
(`TestListServersError_realFailureDoesNotLeakDetail`) that marshals the
500 error and asserts the internal detail is absent from the
client-visible body. Existing tests only asserted status codes, which is
why the leak slipped through.
The 499 client-cancelled path is unchanged (the client has already
disconnected, so nothing is leaked there).
## Test plan
- [x] `go test ./internal/api/handlers/v0/... -run ListServersError`
(3/3 pass)
- [x] `go build ./...`, `go vet`, `golangci-lint` (no new findings in
changed files)
## Note
This leaves the non-security items in #1337 (correcting the `superfluous
WriteHeader` claim, `statusClientClosed` const reuse, broader
cancellation-guard scope) for separate follow-up. Closing #1337 here
since the only security-impacting item is addressed; reopen/split if
you'd prefer to track the nits independently.
Closes#1337🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Maps context.Canceled from ListServers to 499 and skips the error-level log on client disconnect.
Merged as-is per maintainer decision; known follow-up tracked in #1337 (500-path error-detail leak + WriteHeader claim correction + nits).
## Summary
- Remap recorded status to `499` (NGINX-style "client closed") when
`ctx.Context().Err()` is `context.Canceled` after the handler returns
- Skip the `http_errors_total` increment for `499`s so the availability
metric reflects server-visible errors only
## Why
Under scraper-driven load, clients with short timeouts paginate
`/v0.1/servers`, give up while PG is busy, and close the TCP connection.
The handler's DB iteration returns `context canceled`, the handler
converts that to `huma.Error500InternalServerError`, and tries to write
a response to the now-closed socket. NGINX records `499` (client closed)
and never delivers anything to the client — but the registry's
middleware records `status_code=500` and bumps
`mcp_registry_http_errors_total`.
That's what's been firing the `Availability dropped below 95%` alert
during the daily 17:00 UTC bursts. Recent prod numbers from one pod
(~19h uptime):
```
mcp_registry_http_errors_total{path="/v0.1/servers", status_code="500"} 7396
mcp_registry_http_errors_total{path="/v0.1/servers/{serverName}/versions", status_code="500"} 283
mcp_registry_http_errors_total{path="/v0/servers", status_code="500"} 352
```
…against zero 5xx in NGINX ingress logs over the same window. The alert
was correct *given the data it had*; the data was misclassifying client
cancellations as server errors.
## What this does
In `internal/api/router/router.go` (the metric middleware): after
`next(ctx)`, check `ctx.Context().Err()`. If it's `context.Canceled`,
override `statusCode` to `499` and skip the error counter. Requests
counter and duration histogram still record so the cancellation rate
stays visible.
`context.DeadlineExceeded` is intentionally **not** remapped — that
would indicate a server-side timeout if we ever add per-request
deadlines, and should still count as a server error.
## What this does *not* do
- Doesn't reduce the underlying load (ServiceNow + notion catalog
scrapers driving the daily peak — separate work, see the `updated_since`
outreach we're doing).
- Doesn't change handler behaviour — handlers still call
`huma.Error500…` on `context.Canceled`. We could short-circuit earlier
(return without writing) to save the work, but that's a bigger refactor
and the metric fix unblocks the alert immediately.
## Companion change (no code, just a Grafana annotation tweak)
The `Availability dropped below 95%` alert (UID `bexrc60etvhmoa`)
currently has the annotation:
> No. of 5xx are increased from acceptable limit i.e. 5% of total
throughput
The query is on `mcp_registry_http_errors_total`, not 5xx specifically.
After this change the count is meaningful again, but the wording is
still misleading. Suggest updating to:
> Internal error rate exceeded 5% of total throughput
(no PromQL change required.)
## Test plan
- [x] `go test ./internal/api/router/...` — new test passes
- [x] `golangci-lint run ./internal/...` — clean
- [ ] After merge, watch a daily 17:00 UTC burst —
`mcp_registry_http_errors_total{status_code="500",
path="/v0.1/servers"}` should stay flat while
`mcp_registry_http_requests_total{status_code="499",
path="/v0.1/servers"}` grows. Availability alert should not fire on
cancellation-only bursts.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The HTTP namespace-verification fetcher's `isBlockedIP` composed its
blocklist from Go stdlib's `Is*` helpers plus a manual CGNAT range. That
covers IPv4 cloud metadata, RFC1918, ULA, and link-local — but Go's
classification helpers don't recognise IPv6 prefix families that embed
or tunnel to arbitrary IPv4:
- 2002::/16 RFC 3056 6to4 — bits 16-47 are an arbitrary IPv4
- 64:ff9b::/96 RFC 6052 NAT64 well-known — low 32 bits are IPv4
- 64:ff9b:1::/48 RFC 8215 NAT64 local-use — same IPv4-embedding shape
- fec0::/10 RFC 3879 site-local (deprecated, still routed by
some stacks)
On hosts with 6to4 routing or a NAT64 next-hop (the default on
IPv6-only GKE node pools, AWS IPv6-only EC2, Azure IPv6 VMs with
NAT64, and many corporate DNS64/NAT64 networks), an attacker who
controls DNS for a publisher domain can cause the registry to dial
`169.254.169.254`, `10.0.0.0/8`, etc. via the IPv6 wrapper. Same
SSRF class as GHSA-56c3-vfp2-5qqj on n8n-mcp.
Define the four prefixes via a new `mustCIDR` helper (panic on
malformed CIDR at init so a typo can't fail-open silently) and iterate
them in `isBlockedIP` after the stdlib gates. Refactor the existing
CGNAT IIFE to use the same helper for consistency. IPv4-mapped IPv6
(::ffff:0:0/96) needs no special-casing: Go's `Is*` helpers honour the
`To4()` fast-path, so e.g. `::ffff:10.0.0.1` is already classified as
`IsPrivate`.
Adds 17 test cases to `TestIsBlockedIP` covering each new prefix
family, IPv4-mapped IPv6 sanity checks, and just-outside-the-range
positive controls (`2001::1`, `2003::1`, `64:ff9c::1`).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The catalogue page interpolates publisher-supplied URL fields
(`server.repository.url`, `server.websiteUrl`) inside `href="..."`
attributes via `innerHTML`, escaping them with a
`textContent`→`innerHTML`
helper. Per the HTML5 fragment-serialisation algorithm, that round-trip
encodes only `&`, `<`, `>`, and U+00A0 inside text nodes — it does not
encode `"` or `'`. The helper is therefore safe in element-text
contexts (where it is used for `name`, `version`, `description`, etc.)
but unsafe inside an attribute value.
Add an attribute-safe `escapeAttr` helper that follows the OWASP
attribute-encoding rule (also encodes `"` and `'`) and use it for the
two `href` interpolations. Element-text usages keep the existing
`escapeHtml` and remain unchanged. A short comment on each helper
documents the contexts they cover so future fields are wired up
correctly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary
Follow-up to #1241. Two small tweaks to the `readBody` helper:
- **Rename `readBody` → `readErrorBody`.** The helper is only ever
called on the error path (after a non-2xx status check), where we want a
small string for diagnostics. The generic name made it look reusable on
the success path, where streaming JSON decoding is the right approach.
The new name pins the intended use down.
- **Cap the read with `io.LimitReader` (8 KiB).** Defense in depth —
error diagnostics never need more than a few KB, and we shouldn't buffer
an arbitrarily large body from an upstream just to format an error
message. 8 KiB is well above any realistic GitHub API or JWKS error
response.
No behavior change on the happy path. On the unhappy path, error
messages are now bounded.
## Test plan
- [x] `go build ./...`
- [x] `go test ./internal/api/handlers/v0/auth/...`
🤖 Generated with [Claude Code](https://claude.com/claude-code)
The same pattern for reading an error response body appeared in three
places across the auth handlers:
- `internal/api/handlers/v0/auth/github_at.go` (twice)
- `internal/api/handlers/v0/auth/github_oidc.go` (once)
A shared `readBody` helper function added to `common.go` now handles
this in one place.
The `io` import is removed from both files since it is no longer used
directly there.
---------
Co-authored-by: Radoslav Dimitrov <radoslav@stacklok.com>
## Summary
Follow-up to #1238 (closes#988). Adds direct unit-test coverage for the
`claimMatches` helper, which was the actual change in #1238.
`claimMatches` and `toAnySlice` are unexported, so the tests live in
`oidc_internal_test.go` (`package auth`) — same pattern as
`http_internal_test.go`. Cases covered:
- scalar/scalar match and mismatch
- scalar actual against array expected (both directions of overlap)
- array actual against scalar expected
- **array/array** — pre-#1238 this combination panicked with `comparing
uncomparable type []interface {}`
- `[]string` typed slice (exercises the typed-slice branch of
`toAnySlice`, which `[]any` skips)
- non-string scalars (int, bool)
- empty actual array
Test values use single letters (`x`, `y`, `p`, `q`) rather than role
labels — only the relationships matter, and short tokens keep `goconst`
quiet.
## Test plan
- [x] `go test ./internal/api/handlers/v0/auth/...` — all 13 subtests
pass
- [x] `golangci-lint run ./internal/api/handlers/v0/auth/...` — no new
issues vs main
Closes#988. Direct `actualValue != expectedValue` fails when the token
claim is an array (groups, roles, scp, aud). Switch to a slice-aware
any-of overlap so OIDC list claims are validated correctly.
## What this does
The publisher CLI now derives the OIDC audience to request from the
`--registry` URL instead of using a fixed constant, and the server
validates the token's `aud` claim against a new per-deployment env var
(`MCP_REGISTRY_GITHUB_OIDC_AUDIENCE`).
Production sets this env var to
`https://registry.modelcontextprotocol.io` (and per-environment URL
elsewhere), so the deployment only accepts tokens that were requested
for it.
## Compatibility
This is a breaking change for older `mcp-publisher` binaries that still
request the legacy hardcoded audience. After this rolls to a deployment,
users on old `mcp-publisher` will see a clear `invalid audience:
expected …, got […]` error and need to upgrade. Users following the
documented `releases/latest` install pattern get the new binary
automatically.
## Test plan
- [ ] CI green
- [ ] Run a synthetic publish from a real GitHub Actions workflow
against staging after deploy
- [ ] Confirm an older `mcp-publisher` against the same staging deploy
returns the expected `invalid audience` error
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## What this does
Fixes an open redirect plus a handful of small, mostly-invisible
hardening changes around it.
## What's in it
- **The fix**: trailing-slash redirects no longer let `//evil.com/`
redirect off-host.
- **SSRF block** on the DNS/HTTP namespace verification — refuses to
dial loopback, RFC1918, link-local, multicast, CGNAT, or
IP-literal/single-label \"domains\".
- **Slow-client guard**: `ReadTimeout` and `IdleTimeout` on the HTTP
server (no `WriteTimeout` — would risk cutting off legitimate
multi-package publishes).
- **Defence-in-depth headers** on the UI (`/`) and 404 responses.
- **5xx responses** stop leaking raw DB error text to clients (logged
server-side instead).
- **Search filter** stops treating `%` and `_` as wildcards.
- **Validators**: PyPI/NuGet identifiers URL-escaped before fetching;
MCPB no longer follows redirects.
- **Misc**: `Version` field bounded to 255 chars; logs quote user input;
`BlockedNamespaces` denylist now catches subdomain claimants.
Each change is its own commit so they can be reviewed (or reverted)
independently.
## Test plan
- [ ] CI green
- [ ] Synthetic publish in staging for each package type (npm, pypi,
nuget, oci, mcpb) — confirms validator changes don't regress
- [ ] Quick browser check of `/` after deploy to confirm UI still loads
with the new CSP
- [ ] `curl https://<host>//evil.com/` — confirm the open redirect is
closed
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary
DNS auth users have repeatedly hit two failure modes that both produce
the same opaque "Ed25519 signature verification failed" error, making
them indistinguishable to debug. This PR makes both diagnosable
end-to-end without changing the auth protocol or any successful path.
The two recurring failure modes:
1. **Wrong placement** — TXT record put under a selector
(`_mcp-auth.<domain>` or `_mcp-registry.<domain>`) instead of the apex.
DKIM-style intuition, but MCP DNS auth uses SPF-style apex placement.
See #385 (PR #388 fixed the docs but the failure recurred), #1103,
#1126.
2. **Stale apex record** — old TXT record left behind after a key
rotation, silently tried first and rejected.
## Changes
**`internal/api/handlers/v0/auth/dns.go`** — when the apex has no
`v=MCPv1` record, probe `_mcp-auth.<domain>` and
`_mcp-registry.<domain>` in parallel (2s individual timeout). If a
record is found there, return a targeted error pointing the user at the
apex. Costs zero extra DNS work in the success path.
**`internal/api/handlers/v0/auth/common.go`** — when verification fails
against records that *are* present, include short fingerprints of every
key the registry tried (`ed25519:YJLsHFhu`-style 8-char prefix). A new
`ErrSignatureMismatch` sentinel keeps structural errors (wrong signature
size, unsupported algorithm) passing through unchanged via `errors.Is`,
so existing assertions on those messages still hold.
**`docs/modelcontextprotocol-io/authentication.mdx`** — anti-pattern
`<Warning>` callout in the DNS section explicitly warning against
selector placement and stale records.
**Tests** — two new tests:
- `TestDNSAuthHandler_WrongSelectorProbe` — verifies the probe surfaces
records found at `_mcp-auth.` / `_mcp-registry.` and falls through to
the standard error when nothing is found anywhere
- `TestDNSAuthHandler_StaleKeyFingerprintInError` — verifies the
fingerprint hint appears in the failure message for #1126's exact
scenario
One existing test assertion was updated from a generic `"signature
verification failed"` substring to the more specific `"invalid signature
size for ECDSA P-384"` it should always have been asserting; the new
sentinel-based logic surfaces structural errors verbatim.
## Test plan
- [x] `make lint` — clean
- [x] `make test-unit` — all auth tests pass (existing + 2 new)
- [x] Verified existing single-key and multi-key paths preserve their
original (more specific) error messages where they were already specific
- [ ] Reviewer to sanity-check the docs callout renders well in the
Mintlify theme
## Out of scope
The publisher-side preflight DNS check (cross-link in #845) is
intentionally **not** in this PR — that belongs in the broader CLI UX
work tracked by #845.
Refs #845, #385, #1103Fixes#1126
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
<!-- Provide a brief summary of your changes -->
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
We keep bumping `go 1.26.x` across the repo every time a new Go patch
drops
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
- **`go.mod` / `deploy/go.mod`**: `go 1.25.8` → `go 1.26` (minor only —
"this module needs Go 1.26")
- **All CI workflows**: `go-version-file: 'go.mod'` → `go-version:
'stable'` — CI always uses the latest stable Go, no more
bump PRs
- **govulncheck**: removed hardcoded `go-version-input: '1.26.1'` →
`go-version-input: 'stable'`
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
(Updated auto-generated PR description from @tadasant / Claude on Feb
22, 2026)
## Motivation and Context
Fixes#637
Server lifecycle management (deprecation and yanking) is a standard
requirement for package registries. Use cases like rebranding, security
vulnerabilities, and end-of-life servers demand the ability to mark
servers as deprecated or deleted, with optional messaging to guide
consumers toward alternatives.
## Changes
### New API endpoints
- **`PATCH /v0/servers/{serverName}/versions/{version}/status`** —
Update the lifecycle status of a single server version
- **`PATCH /v0/servers/{serverName}/status`** — Update the lifecycle
status of all versions of a server in a single transaction
Both endpoints accept:
```json
{
"status": "active|deprecated|deleted",
"statusMessage": "Optional message (max 500 chars) explaining the change"
}
```
### New CLI command
- **`mcp-publisher status <serverName> [version]`** — Update server
lifecycle status from the command line
- `--status` (required): `active`, `deprecated`, or `deleted`
- `--message`: Optional status message
- `--all-versions`: Apply to all versions of the server (replaces the
`<version>` argument)
### API response changes
`RegistryExtensions` now includes two new fields:
- `statusChangedAt` — Timestamp of the last status change
- `statusMessage` — Optional free-form message (e.g., deprecation
reason, migration guidance)
### List endpoint changes
- New `include_deleted` query parameter on `GET /v0/servers` — defaults
to `false`, hiding deleted servers from listings
- When `updated_since` is provided, `include_deleted` is automatically
set to `true` for incremental sync
### Database
- Migration `013_add_status_fields.sql`: Adds `status_changed_at` (NOT
NULL, initialized from `published_at`) and `status_message` columns to
the `servers` table
### Design decisions (from review discussion)
- **Dedicated status endpoints** rather than overloading the existing
edit endpoint
- **Bidirectional status transitions** — all transitions between
`active`, `deprecated`, and `deleted` are allowed
- **`statusMessage` as free-form text** — no structured `nextName` or
`alternativeUrl` fields (per maintainer feedback, to avoid complexity
around chaining, trust transfer, etc.)
- **Publish permission** grants status change access (not limited to
admin/edit)
- **Status message auto-cleared** when transitioning back to `active`
Good, now I have the full picture. Here's the addendum:
### Consumer-facing API changes
The new status fields live inside
`_meta["io.modelcontextprotocol.registry/official"]` on every
`ServerResponse`. This is the existing `RegistryExtensions` object — two
new fields are added alongside the existing ones:
**Before:**
```json
{
"server": { "name": "com.example/my-server", "...": "..." },
"_meta": {
"io.modelcontextprotocol.registry/official": {
"status": "active",
"publishedAt": "2025-06-01T00:00:00Z",
"updatedAt": "2025-06-15T00:00:00Z",
"isLatest": true
}
}
}
```
**After:**
```json
{
"server": { "name": "com.example/my-server", "...": "..." },
"_meta": {
"io.modelcontextprotocol.registry/official": {
"status": "deprecated",
"statusChangedAt": "2026-02-01T12:00:00Z",
"statusMessage": "Deprecated in favor of com.example/my-server-v2",
"publishedAt": "2025-06-01T00:00:00Z",
"updatedAt": "2026-02-01T12:00:00Z",
"isLatest": true
}
}
}
```
| New field | Type | Present when |
|-----------|------|-------------|
| `statusChangedAt` | `string` (RFC3339) | Always — initialized to
`publishedAt` for existing records |
| `statusMessage` | `string` (max 500 chars) | Only when set —
`null`/omitted for active servers (auto-cleared on transition to active)
|
These fields appear on **every endpoint that returns a
`ServerResponse`**: `GET /v0/servers`, `GET /v0/servers/{name}`, `GET
/v0/servers/{name}/versions/{version}`, and the new PATCH status
endpoints.
### List endpoint filtering
`GET /v0/servers` gains an `include_deleted` query parameter:
| Scenario | Behavior |
|----------|----------|
| No `include_deleted` param | Deleted servers hidden (default `false`)
|
| `include_deleted=true` | Deleted servers included in results |
| `updated_since` provided | `include_deleted` forced to `true` for
incremental sync |
## How Has This Been Tested?
- Unit tests for status handler (`status_test.go`) covering status
transitions, permission checks, validation errors, and the all-versions
endpoint
- Unit tests for the CLI command (`status_test.go`)
- Integration with existing server/edit handler tests
- Database layer tests for the new status update and query methods
- Tested locally with `make dev-compose`
## Types of changes
- [x] New feature (non-breaking change which adds functionality)
## Checklist
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [x] New and existing tests pass locally
- [x] I have added appropriate error handling
- [x] I have added or updated documentation as needed
## Open items from review
- [ ] `include_deleted` silent override: When `updated_since` is
provided and `include_deleted=false` is explicitly passed, should return
a 400 error rather than silently overriding
([thread](https://github.com/modelcontextprotocol/registry/pull/893#discussion_r2100879360))
— agreed upon but may not yet be implemented
## Motivation and Context
Per discussion with @rdimitrov - all validation now happens in a new
`/validate` endpoint. For details, see below.
## Breaking Changes
None. Validation on /publish only validates schema version and semantic
checks (manual checks) as before. We can add schema validation by
changing a flag in the called to `ValidateJSONSchema` when we're ready.
## Types of changes
- [X] New feature (non-breaking change which adds functionality)
## Checklist
- [X] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [X] My code follows the repository's style guidelines
- [X] New and existing tests pass locally
- [X] I have added appropriate error handling
- [X] I have added or updated documentation as needed
## Additional context
## API Changes
### 1. New `/validate` Endpoint
A new public endpoint was added for validating `server.json` files
without publishing them.
**Details:**
- **Paths:** `/v0/validate` and `/v0.1/validate`
- **Method:** `POST`
- **Authentication:** None required (public endpoint)
- **Request:** JSON body containing `ServerJSON`
- **Response:** `200 OK` with `ValidationResult` containing:
- `valid` (boolean)
- `issues` (array of validation issues with type, path, message,
severity, reference)
**Implementation:**
- New handler file: `internal/api/handlers/v0/validate.go`
- Performs comprehensive validation using `ValidationAll` option:
- Schema version validation
- Full schema validation
- Semantic validation
- Always returns `200 OK` - validity is indicated in `result.Valid`
field
- Registered in router for both `/v0` and `/v0.1` prefixes
### 2. `/publish` Endpoint Validation Flow Changes
The publish endpoint now validates earlier in the request flow and
provides clearer error responses.
**Before:** Validation was mixed into `ValidatePublishRequest()` (which
also handled publisher extensions and registry ownership).
**After:**
1. Validates `ServerJSON` structure first using `ValidateServerJSON()`
(schema version + semantic validation)
2. Returns `422 Unprocessable Entity` with message: "Failed to publish
server, invalid schema: call /validate for details"
3. Only proceeds to publisher extensions/registry validation if schema
validation passes
4. No longer calls `ValidatePublishRequest()` internally for schema
validation
**Key Changes:**
- Validation happens before `CreateServer()` call
- Returns `422` on schema/semantic validation failures
- Error message directs users to `/validate` endpoint for details
- `ValidatePublishRequest()` no longer performs schema validation (added
note in comments)
### 3. `/edit` Endpoint Validation Flow Changes
Similar validation changes were applied to the edit endpoint.
**Changes:**
- Added `ValidateServerJSON()` call before server update
- Returns `422` with same error message format on validation failure
- Matches the `/publish` endpoint behavior
### 4. Validator Functions Refactoring
Updated validator functions to reflect that schema validation is now
done separately.
**Changes to `ValidatePublishRequest()`:**
- Removed internal `ValidateServerJSON()` call
- Added note: "ValidateServerJSON should be called separately before
this function"
- Now only validates publisher extensions and registry ownership
**Changes to `ValidateUpdateRequest()`:**
- Same change (removed schema validation, added note)
- Now only validates registry ownership
## CLI Changes
### 1. `validate` Command: Now Uses API Endpoint
**Before:** Performed local validation using
`validators.ValidateServerJSON()`.
**After:** Calls the `/v0/validate` endpoint on the registry.
**Details:**
- Reads `server.json` and sends it to the registry API
- Displays validation results returned by the server
- Uses the same error formatting as before
- Registry URL comes from token file (or default)
- No authentication required (public endpoint)
**Implementation:**
- Added `validateViaAPI()` function that POSTs to `/v0/validate`
- Parses `ValidationResult` response from server
- Uses shared `printValidationIssues()` for formatting
### 2. `publish` Command: Validation Only on 422 Errors
**Before:** Performed upfront local schema validation before publishing.
**After:** Attempts publish first, then validates only if server returns
422.
**Details:**
- Attempts publish immediately (no upfront validation)
- If publish returns 422 (Unprocessable Entity):
- Calls `/v0/validate` endpoint to get detailed validation errors
- Formats and displays them using same logic as `validate` command
- Returns error with migration links
- For non-422 errors (401, 403, etc.), returns original error (no
validation call)
**Implementation Changes:**
- Removed upfront `runValidationAndPrintIssues()` call
- Modified `publishToRegistry()` to return HTTP status code alongside
response/error
- Added 422 detection logic that calls `validateViaAPI()`
### 3. Shared Validation Error Formatting
**Before:** `runValidationAndPrintIssues()` function handled both
validation and printing together.
**After:** Refactored to separate printing logic.
**Details:**
- Created `printValidationIssues()` function - just prints validation
issues (no validation logic)
- Removed `runValidationAndPrintIssues()` function (no longer used)
- Both `validate` and `publish` commands use `printValidationIssues()`
for consistent output
**Behavior:**
- Prints schema validation errors with migration guidance
- Prints all other validation issues with formatting
- Returns formatted error message string for schema issues
## Summary
Both CLI commands now use the `/validate` API endpoint for validation
instead of local validation. The `publish` command validates only after
receiving a 422 error response, and both commands share the same error
formatting via `printValidationIssues()`.
### Benefits
- **Centralized validation logic** - All validation happens on the
server
- **Always up-to-date** - CLI always uses current server validation
rules
- **Consistent experience** - Same validation behavior whether using CLI
or API directly
- **Better separation of concerns** - Schema validation separated from
publisher-specific validation
<!-- Provide a brief summary of your changes -->
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
Follow up on #869
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
<!-- Provide a brief summary of your changes -->
Change the UI default to the locally hosted registry.
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
Was quite confusing at first :)
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [x] New and existing tests pass locally
- [x] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
The DNS auth endpoint was vulnerable to resource exhaustion if an
attacker controlled a domain with a slow/non-responding authoritative
DNS server. Without a timeout, handler goroutines would block on DNS
resolution indefinitely, allowing an attacker to pile up goroutines
until OOM.
Production is protected by NGINX ingress's default 60s proxy timeout,
but this adds defense-in-depth with a tighter 5s bound.
When users try to publish to a GitHub organization but don't have
permission, the error message now includes a helpful hint suggesting
they may need to make their organization membership public in GitHub
settings.
This addresses the issue where users couldn't understand why they were
unable to publish servers from organization repositories, even when they
were members of the organization.
Fixes#398
Generated with [Claude Code](https://claude.ai/code)
---------
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: adam jones <domdomegg@users.noreply.github.com>
## Summary
Adds a clean, minimal web interface at `/` for browsing MCP servers in
the registry. Built as a single self-contained HTML file embedded in the
Go binary.
## Features
- Responsive grid layout (1/2/3 columns based on screen size)
- Search servers by name
- Filter to show only latest versions (default on)
- "Recently Updated" section for servers updated in the last hour
- Expandable cards with full server metadata and JSON viewer
- Debug modal to switch between production/staging/local/custom API
endpoints
- Full URL parameter support and browser history navigation
## Implementation
- Single HTML file (~400 lines) with Tailwind CSS via CDN
- Zero build dependencies
- Embedded using `//go:embed`
- Served directly from router at root path
- Uses stable v0.1 API endpoints
Fixes#99
---
AI disclosure: this is 99% vibed with Claude. It's intended as a v0 to
get something up and working, and then maybe at a later date we can
replace it with something more real.
<!-- Provide a brief summary of your changes -->
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
The Repository field in ServerJSON was not properly omitting empty
values
when marshaling to JSON. Even when a server had no repository
configured,
the JSON would include `"repository": {"url": "", "source": ""}` due to:
1. Repository struct fields (URL, Source) lacking omitempty tags
2. Repository being a value type instead of pointer in ServerJSON
This caused validation warnings for servers without repository URLs.
Changes:
- Add omitempty tags to Repository.URL and Repository.Source fields
- Change ServerJSON.Repository from model.Repository to
*model.Repository
- Update validateRepository to handle nil pointers
- Update all code creating Repository instances to use pointers
Now, servers without repository information correctly omit the field
entirely from JSON output, eliminating spurious validation warnings.
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Fixes#710
This PR adds CORS middleware to enable browser-based clients and web
applications to access the registry API.
## Changes
- Add CORSMiddleware in internal/api/cors.go to handle preflight OPTIONS
requests and inject CORS headers
- Integrate CORS middleware into the server middleware stack
- Add CORS configuration options (CORS_ENABLED and CORS_ALLOWED_ORIGIN)
## Testing
- All existing tests pass
- Manual testing confirms CORS headers are present in responses
- OPTIONS preflight requests now return 204 with proper headers
## Configuration
New environment variables:
- MCP_REGISTRY_CORS_ENABLED (default: true)
- MCP_REGISTRY_CORS_ALLOWED_ORIGIN (default: *)
## Motivation and Context
See
https://github.com/modelcontextprotocol/registry/issues/482#issuecomment-3436993157
for full details.
This adds ECDSA P-384 (a popular NIST approved curve) in addition to
Ed25519.
## How Has This Been Tested?
I have tested with a local deployment against `mcp.joelverhagen.com`.
Also, I have added significant tests.
## Breaking Changes
None. (hopefully)
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [x] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [x] New and existing tests pass locally
- [x] I have added appropriate error handling
- [x] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
@domdomegg and I spoke a bit about this on Discord:
https://discord.com/channels/1358869848138059966/1369487942862504016/1423710918180606052
---------
Co-authored-by: adam jones <domdomegg+git@gmail.com>
Enhanced Go struct tags throughout `pkg/api/v0/types.go` and
`pkg/model/types.go` to better align with the OpenAPI specification.
## Changes
- Added comprehensive `doc` tags with detailed field descriptions
matching OpenAPI
- Added `example` tags showing valid values for fields (e.g.,
`"io.github.user/weather"`, `"npx"`)
- Added `format` tags (`uri`, `date-time`) for better type specification
- Added `pattern` and `enum` tags for validation constraints
- Added `placeholder` field to `Input` struct matching OpenAPI spec
- Removed redundant comments in favor of inline struct tags
These changes improve code clarity and make the types more
self-documenting, while maintaining compatibility with the OpenAPI
schema definitions in `docs/reference/api/openapi.yaml`.
---------
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Radoslav Dimitrov <radoslav@stacklok.com>
<!-- Provide a brief summary of your changes -->
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
The following PR introduces a `/version` API that serves the build
details along with the version of the API server being served. The
`/version` endpoint allows users to programmatically query which exact
version of the registry is running in production for debugging.
~~auditing, and compatibility checks~~.
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
<!-- Provide a brief summary of your changes -->
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
<!-- Provide a brief summary of your changes -->
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
Follow up from #658 addressing the feedback from @tadasant
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
<!-- Provide a brief summary of your changes -->
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
The following PR adds a new v0.1 API version.
The goal is to mark v0.1 as a "freeze" point while keeping v0 as the
moving target for new features so integrators can start building on top
of something stable(r).
Details:
* This change is quite simple - we re-register the same handlers twice,
once as `v0` and once as `v0.1`
* Going forward, we'll keep v0.1 frozen while we add changes either on
top of both (non-breaking) or we'll have to branch if breaking changes
are necessary
* We all acknowledge this is not exactly best-practice, but it gives us
a way to gather feedback and allow clients to start integrating with the
API without breaking them (**note** this implies a follow up PR that
would branch the handlers used in v0.1).
* For integrators/clients: please provide your feedback - what works
well, what doesn't so we can take this into account going towards GA.
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
No, this just adds a new API version alongside the existing one.
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
Fixes: #649
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Remove redundant endpoint as suggested in #617. Clients should now use
`GET /v0/servers/{serverName}/versions/latest` instead (using `latest`
as a special version value in the existing endpoint).
This simplifies the API surface and reduces implementation burden for
subregistries, while maintaining a clear migration path for consumers.
## Changes
- Removed `GET /v0/servers/{serverName}` endpoint
- Updated `GET /v0/servers/{serverName}/versions/{version}` to accept
`latest` as special version
- Updated all tests to use new endpoint pattern
- Updated documentation and OpenAPI spec
- Added breaking change notice to CHANGELOG
Fixes#617
Co-authored-by: Claude <noreply@anthropic.com>
<!-- Provide a brief summary of your changes -->
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
The following PR:
* Enforces schema validation on the API level (requests without schema
or older schema will fail)
* Updates the unit tests and examples to match that
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
**Note this can be considered breaking if clients have not included it
before.**
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
---------
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
## Summary
- Applied `gofmt -s -w` across the entire codebase to ensure consistent
Go formatting
- This will trigger a deployment with minimal code changes
## Test plan
- [x] Pre-commit hooks pass
- [x] No functional changes, formatting only
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude <noreply@anthropic.com>
- Add workflow_dispatch to deploy.yml for manual deployment triggers
- Document that releases don't auto-deploy to production
- Add instructions for deploying releases to production
- Add verification steps for deployment
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
<!-- Provide a brief summary of your changes -->
## Motivation and Context
Currently the `dns` and `http` based auth flows share code well in the
publisher tool. However there is a lot of duplication in the auth
endpoint. This on it's own is not a problem but it does make
enhancements to the proof record (the `v=MCPv1; ...` string) harder to
keep in sync.
This PR adds a `common.go` file to share logic between the two.
This supports work I am prototyping for
https://github.com/modelcontextprotocol/registry/issues/482.
## How Has This Been Tested?
Added new test cases for the permissions. Ran them before and after the
refactoring to ensure no behavior change.
Ran existing UTs.
Ran the integration tests.
Manually exercised the flows locally.
## Breaking Changes
None.
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [x] New and existing tests pass locally
- [x] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
<!-- Provide a brief summary of your changes -->
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
The following PR adds a simple unit test to ensure the served API
handlers are a superset of the reference OpenAPI schema.
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
Fixes: https://github.com/modelcontextprotocol/registry/issues/567
---------
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
## Motivation and Context
Replace server-only `http.MaxBytesReader(nil, ...)` with
`io.LimitReader` to cap the fetched body to 4KB in key fetcher. This
avoids passing a `nil` ResponseWriter and aligns with Go idioms. If more
than 4KB is received, return a clear error. Previously `MaxBytesReader`
returned a misleading `http: request body too large` error, which is
server-oriented. With this change the error becomes `HTTP auth key
response too large`.
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
Add tests covering success, oversized bodies, connection failure,
non-200 status, and read failures. This covers almost all lines of
`FetchKey` method of `DefaultHTTPKeyFetcher`. For tests, I also added a
new initialiser `NewDefaultHTTPKeyFetcherWithClient` so that the HTTP
client could be modified for test use.
Tests use `httptest.NewTLSServer` plus a helper to construct a client
pinned to the test server.
`auth` package test coverage increased from 56.8% to 59.3%.
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
None.
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [x] New and existing tests pass locally
- [x] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
<!-- Provide a brief summary of your changes -->
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
The follow PR removes the in-memory database option from the registry.
We had a chat about this with @domdomegg and although it is supposed to
be convenient, it isn't that useful in reality. We want to test and
develop the registry in a setup that is closer to the environment it is
supposed to be run. One of the main reasons are:
* We don't use it anywhere else besides unit testing and potentially
local development. Instead we should use postgresql for reliable results
(in-memory was actually giving false positive results)
* We have to keep maintaining it and make sure it mimics the behaviour
we have for postgresql which is not yielding a lot of value
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
Yes, I've updated the test unit make target to run in the postgresql
environment and now we can see the race condition issue happening.
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
No
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
---------
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
## Summary
Fixes a concurrent map access vulnerability in the OIDC handler that
could cause remote unauthenticated DoS.
## Changes
- Removed OIDC authorization flow endpoints ( and )
- Removed the unsynchronized map that caused the data race
- Kept only the OIDC token exchange endpoint ()
- Simplified by removing OAuth2 flow dependencies
The map was being accessed concurrently by the start (write) and
callback (read/delete) endpoints without synchronization, causing
runtime panics. Since the token exchange flow doesn't require session
state, removing the authorization flow eliminates the vulnerability
entirely.
