~bigbes/sr-ht-spec

ref: 9b5827b659a4c0524e7c213f5c0834821d35e783 sr-ht-spec/service/read.go -rw-r--r-- 9.3 KiB
eaa2b65f — bigbes 27 days ago
fix(service): separate the read-plane rev guard from the review path

The previous commit's guard broke two service tests, which were right and
the guard was too broad: rendering and diffing a proposal branch genuinely
needs to read one, so a blanket ban on ref names is not the shape.

ReadDocument keeps the strict object-name rule. ReadDocumentAtRef reaches
an arbitrary ref, deliberately as a separate awkwardly-named method rather
than a flag, so serving unreviewed content is something a caller asks for
by name and a reviewer can grep for.

A rejected rev now wraps ErrNotFound as well, preserving the existing
deliberate property that a crafted revision cannot distinguish malformed
from absent by probing.
8edca94e — bigbes 27 days ago
feat(mcpsrv): MCP read tools, with Host validation replacing the SDK guard

Adds the Phase 2 read tools (spec_search, spec_read, spec_list) over the
service layer.

Two security fixes came out of building them.

The read plane could serve proposal content. gitx resolves ref names, and
service.resolveRev passed any string through, so rev=proposals/42 made the
READ plane hand back unreviewed text — which would then flow into agent
context as though approved, the single failure this service exists to
prevent. ValidateReadRev now admits only the approved-head sentinel or a
full 40-character object name, at the layer all three surfaces share.
Abbreviations are refused too: one that is unique today can become
ambiguous later, so a pinned revision would silently stop meaning one
thing.

The MCP SDK's DNS-rebinding guard rejects a loopback listener whose Host
is not loopback, which is exactly nginx forwarding to 127.0.0.1 — it would
403 only in production, passing every local test. The SDK offers no
allowlist, so the guard is disabled and replaced by a stricter check: Host
must equal the configured origin, or a loopback name for development. A
rebinding attack carries the attacker's name in Host and fails it. An
unusable origin logs loudly rather than quietly unguarding the endpoint.
e1b3c64e — Eugene Blikh 27 days ago
feat: service — wiring, space lifecycle, read paths, push validation, reconciler

Phase 1 of the implementation plan: the orchestration layer's read, validate
and repair halves. The write plane (propose/merge) is Phase 3 and is absent.

- Config/New assemble a Service from the shared config.ini, reporting every
  missing key in one message; TokenStore adapts db/ to authn.TokenStore,
  mapping db.ErrNotFound onto authn.ErrUnknownToken so an unknown credential
  is a 401 rather than a 503.
- CreateSpace writes the repository first and the row second, removing the
  repository again if the insert fails; a crash between the two leaves content
  that is merely unlisted rather than a phantom space.
- ReadDocument/ListDocuments/Policy/ResolveRev resolve the approved head or a
  pinned revision through one code path.
- ValidatePush checks the refs rule first and unconditionally, then frontmatter
  and document-id uniqueness, which --push-option=skip-validation waives. The
  rejection is a structured, terminal-shaped message naming the document.
- PlanRepairs is the repair table as a pure, table-tested function; Reconcile
  gathers the facts and applies them, listing stale-index spaces for Phase 2.

Two departures from the design's repair table, both to stop the reconciler
destroying live state, documented at their definitions: an open row with no
branch is left alone inside a grace window (every propose passes through that
state), and a branch still sitting on its recorded base is never treated as
merged (its tip is trivially an ancestor of the approved head).