doc: say which of the three guards on the review renderer is load-bearing
The file claimed that aligning the review renderer with the read one would let a
<script> run. Mutation says otherwise: putting html.WithUnsafe() back turns
nothing red, because the raw-HTML node renderer intercepts those node kinds and
the resolver strips a dangerous destination before goldmark's own check would.
Both of those ARE load-bearing — removing them turns six and three tests red
respectively, the wikilink among them. Unsafe stays off as the backstop, and the
comment now says that instead of overstating it.
scss: keep a flowed list item's bullet clear of the change bar
Measured in a headless render of the real corpus: the hanging indent pulled the
bullet back into the cell's left edge, where the change bar is painted, so every
list item on a new document had its bullet sitting on the green rule. A margin
moves the whole item right instead; the hanging indent still aligns a wrapped
line with the item's first word.
scss: draw the change as a bar on the block's edge, not as a fill
A proposal that adds a document makes every row an insertion, so a tint over the
whole text cell coloured 100% of the page — saying nothing, and costing the prose
its contrast for the whole review. The signal is a 3px bar on the block's left
edge instead: one continuous rule down the margin when everything is new, one
short mark when a paragraph was edited, and on a small edit the only coloured
thing on the page is the words that changed.
It is an inset box-shadow rather than a border because a border on a cell of a
border-collapse table collapses against the rail's hairline and takes its width
out of the text column; a shadow also paints above the opaque band a sticky
heading needs, which retires the two rules that used to re-assert the tint there.
Flowed prose gets a reading measure, since the page is full-bleed and prose set
across 1400px is unreadable, and keeps the structure its own text does not carry:
an indent and a bullet for a list item, a rule for a quote. An ordered item gets
no number, because prosediff records that the list was ordered and not which item
this is.
web: render a review block as a paragraph, not as its source lines
prosediff's package doc says prose reflows and that whitespace alone must never
produce a diff, and the web layer then emitted one row per SOURCE line: a
hard-wrapped paragraph arrived as six ragged rows with six numbers, which is the
line-differ artefact the engine exists to avoid. A prose block is now one row
holding the block's markdown rendered, with the word marks carried through as
<ins>/<del>, and the gutter states the line range the block occupies on each
side once. Code fences, frontmatter and raw HTML keep their lines, because there
the line is the unit and its whitespace is content.
That retires the recovery of per-line numbers for reflowed prose — the
WordsByLine spreading, its interleaving and the per-row pairing — and with it the
off-by-one they were defending against. A modification is one row of its own kind
rather than a delete row plus an insert row, so the edit reads in place.
The property test now checks the invariant per side rather than per row: one row
can carry both revisions' words, and the projection asserted is the one the
renderer uses.
diff.js restores a hash by extent overlap instead of membership, because a row
that states a range puts only its two ends in the cell.
doc: render a proposed document with unsafe HTML off
The read plane turns goldmark's unsafe mode on because its documents are
approved; on the review page approving them is what the page is for, so a
<script> would run in the reviewer's session while they decide. A second
renderer keeps unsafe off, shows raw HTML as escaped text rather than omitting
it (a review page must not hide part of what it asks to approve), refuses a
dangerous wikilink destination the package's own anchor writer would otherwise
emit unchecked, and carries a word diff's marks through a markdown rendering
with Private Use Area sentinels, refusing the rendering when a mark did not
survive it.
graph: pin the introspection schema to what the executor can serve
The introspection schema comes from gqlparser's built-in prelude; the
code that executes it is generated by gqlgen. Nothing tied the two
together, and gqlgen's generated executor answers a field it does not
know with panic("unknown field"), recovered into "internal system
error". A gqlparser that has learned a newer edition of the
introspection schema than the pinned gqlgen can execute therefore leaves
/query advertising fields that fail when a consumer selects them —
which matters here, because this endpoint is meant to be federated.
The new test derives the field list from whichever prelude is in use
rather than hard-coding one, and checks every declared field of the six
introspection types is selectable through a path this schema really has.
Against gqlparser v2.5.36 it fails on __Type and __InputValue, the two
types v2.5.22 added fields to; at the pinned v2.5.21 it passes.
Also corrects generate.go. Its note explained why the directive resolves
gqlgen through the module graph, which is true, but read as though that
made codegen work. It does not: v0.17.36 re-emits an existing resolver's
doc comment without its // markers and leaves schema.resolvers.go as
invalid Go. v0.17.94 fixes that and is blocked on sr-ht-core, so the
note now says so and says where the bump belongs.
go.mod: bump gqlparser past the GO-2024-2920 floor
v2.5.8 is below the fix for the parseDirectives denial of service, which
landed in v2.5.14. govulncheck reached it from two call sites here —
graph.mutationResolver.CreateUserWebhook via webhooks.Validate, and
api.init via gqlparser.MustLoadSchema — and reports it gone afterwards.
The version is v2.5.21 rather than the v2.5.36 the five sibling services
run, and the ceiling is deliberate. v2.5.22 updated the built-in prelude
for the oneOf-input-objects and deprecated-arguments spec changes: it
adds the @oneOf directive, __Type.isOneOf, __InputValue.isDeprecated and
deprecationReason, and includeDeprecated arguments on args and
inputFields. gqlgen v0.17.36's generated executor predates all of them
and answers an unknown introspection field with panic("unknown field"),
so on v2.5.36 the schema advertises fields that then fail to execute —
measured here as a 422 and "internal system error" for a query
selecting isDeprecated on an argument. The siblings can carry that
prelude because they run gqlgen v0.17.94, whose executor implements it.
v2.5.21's prelude is byte-identical to v2.5.8's, so this changes nothing
about what a consumer federating this /query sees.
mcpsrv: authenticate every call instead of trusting the session
The transport ran in the SDK's stateful mode, so a tool handler's context was
the context of the request that carried initialize rather than the one carrying
the call. Stateless connects a throwaway session per POST, which puts the
resolver middleware's principal back where the tools read it.
Measured here, not inferred from a sibling: one session, handshake as
claude-code/alpha, second call presenting claude-code/beta's credential.
stateless: attributed to beta, tokens.sr.ht #12
stateful: attributed to ALPHA, tokens.sr.ht #11
stateless: spec_read with a spec:propose-only token -> refused, spec:read missing
stateful: the same call -> served the document
The second pair is the one that hands out content. Gate cannot cover it: Gate
asks about identity, per HTTP request, and both credentials are agents; the
grant is read off the handler's context by requireRead, which stateful is the
handshake's. So the Mcp-Session-Id was a bearer credential for everything
decided inside the SDK.
What a client loses is the server->client half: the standalone SSE GET is now
405 with Allow: POST, so no server-initiated requests and no resumption of that
stream. No tool here samples, elicits, reports progress or subscribes, so
nothing gives anything up. The session id is still issued, and a forged one now
answers 200 rather than 404 because it is no longer an authority to validate.
The comments in proposeHandler and commentHandler claimed the principal came
from "this very request" and were false as written; they now name the constant
that makes them true.
The two tests that measure it were written against the stateful code and are red
without the flag. They are squashed in with the fix rather than landed as a red
commit of their own, so every commit here builds and passes.
doc: say what the anchors pass costs and what it buys, and pin the alert
doc: report a wikilink that names a section the document does not have
doc: slugify a same-page heading reference like every other one
doc: pin heading anchors against goldmark and against the fragment side
doc: slugify heading ids the way wikilink fragments are slugified
migrate: refuse a blank migrate-on-upgrade instead of panicking
The -a gate read the key through core-go's config.GetBool, which panics on
anything outside its eight spellings, while go-ini reports a bare
`migrate-on-upgrade=` as a key that is set — so the false default was never
reached and the empty string went straight to the panic. Measured before this
commit: exit status 2 and a Go stack trace out of `specsrht-migrate -a up`,
which the package upgrade hook runs unconditionally.
The key is now read here, through a parseBool matching core-go's set exactly so
that the daemon reading the same file cannot panic on a value accepted here. A
value that is not a boolean is a refusal naming the key and the spellings, at
exit 1, which is the status the hook reads. An absent key still means no.
migrate: install the instance's masking before brant can log a DSN
brant reports a migration directory it cannot open with slog.Error("failed to
create provider", "datasource", a.DataSourceName, ...) -- the connection
string, password and all, at ERR level. A missing directory is what an installed
package is in until its first migration ships, so this is an ordinary path
rather than an exotic one.
This binary installed no slog handler at all, so that record went to Go's
built-in stderr handler and no mask list of any kind applied -- while cmd/specsrht
had been masking a DSN correctly for as long as this one had been printing one.
It now installs the same handler from the same policy. The verbosity comes from
DefaultsWithoutDebugFlag, because -d here is brant's --dialect and takes a value.
Half a fix without the ecore bump: "datasource" was not a spelling the mask list
knew until 9a4d2ed. Both halves are pinned by a subprocess test, and removing
either turns it red.
The log package goes with it. slog.SetDefault reroutes the std logger through the
installed handler at info level, so a log.Fatalf left behind would be silently
dropped by an operator's log-level=warn -- an error message swallowed by a
logging preference.
Not fixable from here: brant's cli.initLogger runs ahead of that record and under
--json replaces the installed handler with a plain JSON one of its own. Nothing
passes --json; the default branch leaves the handler alone.
graph: accept a meta.sr.ht token, so /query can be federated
api.sr.ht forwards ONE client Authorization header to every service a
federated query touches — AuthMiddleware copies it verbatim into the
request context, and the Internal credential it can mint is used only to
fetch schemas at startup. So a federated caller arrives here holding
whatever credential the client had, and the only credential that works
across the whole instance is a meta.sr.ht personal access token. /query
refused those, which made federating it one `api-origin=` line that
would produce 401s.
api-meta.json was the stronger half of the same refusal: with an empty
scope list meta.sr.ht renders no checkbox for this service, so no PAT
could be scoped for it even in principle — no amount of code in graph/
could have worked around that. It now publishes SPECS, derived from
authn.ScopeRead rather than spelled twice, with a test on each side
asserting the two agree. A scope published and not checked admits what
should be refused, one checked and not published cannot be minted at
all, and neither is visible from inside one file.
The plane is built in cmd/specsrht beside the graph.New call and NOT on
the *authn.Resolver every surface shares, which is the whole scope of
the exception: the REST write plane, /mcp and the push hook resolve
through that resolver, which knows nothing of PlaneMeta, so a PAT is not
a way around the tokens.sr.ht grant they require. Routing is
metapat.PlaneOf on the presented credential rather than a fall-through
from a failed plane, because an instance with no [tokens.sr.ht] section
has no working-token validator to fail first.
Two things this service needs that the cov and bench donors did not.
MetaAuth is told the instance owner and refuses a PAT belonging to
anybody else with the same ErrNotInstanceOwner a foreign working token
gets: a PAT is the credential every account on the instance can mint for
itself, so without that check the widest credential in existence would
have been the one that skipped the narrowest identity rule, and any user
could have read the corpus. And a PAT resolves to KindAgent, never
KindOwner, so webhook management and proposal approval stay with the
human at a browser.
The sentinels are spec's own — ErrMissingScope, ErrInvalidPersonalToken,
ErrMetaUnavailable — rather than reuses of bearer's, because bearer's
prose names tokens.sr.ht and every word of it is wrong for this plane;
an operator reading "tokens.sr.ht could not be reached" while that
daemon is up goes looking in the wrong place. What must agree between
the planes is the status, not the wording, and StatusFor is the one
table that decides it for both.
The two vocabularies do not overlap and are refused differently, which
the tests pin: a PAT naming spec.sr.ht/spec:read is a well-formed OAuth
grant for a scope this service does not publish (403), while a working
token naming spec.sr.ht/SPECS does not parse as a grant string at all
(401). A 403 names the permission of the plane the caller actually used
— telling a PAT holder to obtain "spec:read" would send them looking for
a checkbox meta.sr.ht cannot render.
cmd/specsrht's api-meta assertion is not Postgres-gated here, unlike the
bench donor's: it ran locally and went red on the stale empty-list
assertion, rather than hiding until CI.
go.mod: take the shared libraries' current heads
ecore for mcphttp and the hot-path work, core for the errors fix: its seven
gqlerror sentinels were package-level pointers that gqlgen stamped with the
first field path they ever took, which was a wrong answer, a cross-request
leak and a data race.
graph: keep webhook management with the owner, not with any agent
Moving /query to the anonymous router admitted tokens.sr.ht working tokens, and
webhookAuthorized asked only whether the caller was AUTH_INTERNAL. coreauth maps
the owner and its agents alike to INTERNAL — deliberately, because INTERNAL is
what core-go's NewAuthConfig and FilterWebhooks demand of anyone at all — so a
token carrying nothing but spec:read could create and delete subscriptions and
read every subscription's URL, stored query and delivery bodies. A read grant
was buying a mutation, which is the one thing a grant vocabulary exists to
prevent. Measured before the fix: a spec:read token created two subscriptions
against a live daemon.
The guard now asks spec's own principal for IsOwner, which is the only value
left that still tells the owner from an agent. That restores exactly what the
daemon's ownerOnly wrapper enforced before the conversion, and it needs no new
grant string: naming a third grant beside spec:read and spec:propose is a
vocabulary decision for tokens.sr.ht to mint, not something to invent at a call
site.
One guard covers the whole webhook chapter. createUserWebhook,
deleteUserWebhook, userWebhooks and userWebhook all call it, and the two field
resolvers that carry webhook data are reachable only through an object one of
those four returned. Query.webhook has no ACL and needs none — outside a
delivery there is no payload, so it answers an error; asserted rather than
assumed.
The consequence, and it is not small: webhook management is now unreachable over
/query, because this endpoint accepts no credential that resolves to the owner —
authn produces KindOwner from the unified-login cookie alone and this endpoint
reads no cookie. The check is written against the right predicate anyway, so a
plane that does yield the owner works the moment it exists. Delivery is
untouched, and was verified end to end against a subscription inserted directly.
Also stop returning coreerrors.ErrAccessDenied itself. It is a package-level
*gqlerror.Error and gqlgen assigns the field path onto the error it is handed,
so the shared object carried the previous refusal's path: against a live daemon
a refused deleteUserWebhook, userWebhooks and userWebhook all reported
"path":["createUserWebhook"]. A wrong answer, a cross-request leak of which
field somebody else asked for, and a data race on a global. The bug predates
this endpoint but was latent while ownerOnly refused non-owners before any
resolver ran; refusing agents here makes it the common path.
graph: serve /query on the anonymous router with a bearer credential
The schema was mounted by core-go's server.WithSchema, on the authenticated
router, behind an ownerOnly middleware. That put it on meta.sr.ht's OAuth
vocabulary while every other surface of this service — the web UI, /mcp, the
REST write plane — authenticates with a tokens.sr.ht working token, so a
credential that reads through /mcp was refused by the endpoint meant to be the
instance-native read plane. dolt.sr.ht's graph package is the pattern; this
follows it.
/query is now mounted on the anonymous router and graph.Server installs its own
credential middleware: a working token owned by [sr.ht] owner-name and carrying
spec:read reads, one without that grant is 403, one belonging to anybody else is
403, and anything that does not verify is 401 with the bearer challenge. A
cookie is not a credential here — the principal is overwritten with the
anonymous one when no bearer token is presented, so no middleware above the
mount point can promote a browser session into read authority.
ownerOnly's rule survives the move: it compared auth.AuthContext.Username to the
owner, and authn's resolver refuses a foreign token's owner at the door with the
same 403. What it also did — remapping the owner to AUTH_INTERNAL so core-go's
webhook engine would accept them — is now coreauth's, which is what that package
was written for and had no caller for until today.
A service that mounts its own /query owes the instance api-meta.json, because
core-go serves that file only for the schemas it hosts itself. sr-ht-ecore's
apimeta serves it, with an empty scope list: spec.sr.ht defines no meta OAuth
scope and no @access directive to check one against, and a JSON null there is a
500 on meta's personal-token page for the whole instance.
Two consequences worth naming.
A meta.sr.ht personal access token no longer reaches /query. It did while
core-go's auth.Middleware stood in front of it. Accepting one again means giving
spec.sr.ht a meta scope first, and there is none to invent.
WebhookSubscription.sample cannot be rendered on this endpoint and says so.
corewebhooks.Exec reads the complexity bound off core-go's server context, which
only WithDefaultMiddleware installs and which cannot be built from outside that
package. Delivery is unaffected: the queue's context comes from WithQueues and
does carry it — but MaxComplexity must now be set by hand, because zero there
fails every delivery rather than imposing no limit.
mcpsrv: mark /mcp uncacheable, fail closed on origin, split tool errors from faults
Three gaps between this surface and the cov/dolt pattern the siblings settled
on. The write tools are untouched, the per-tool grant scheme is untouched, and
the transport stays stateful.
The endpoint set no Cache-Control and no Vary at all. Every answer here depends
entirely on the credential the request carried and says nothing about it in its
URL, and some of them are the whole approved corpus, so a shared cache was free
to keep one and replay it to the next caller. cache.go is a local copy of
ecore/mcphttp.PrivateCache, byte for byte on the header values so the retrofit
is a delete and an import once that commit is published. It commits the headers
on Write and Flush as well as WriteHeader: the SDK answers a POST with an event
stream that never calls WriteHeader, so a wrapper hooking only that one sets
nothing on the response an agent actually gets, while passing every other header
test. Measured — with only WriteHeader hooked the streamed answer leaves with
the SDK's own `no-cache, no-transform` and no Vary.
Vary names both planes, where dolt.sr.ht names Authorization alone. dolt is
right for dolt: its /mcp is bearer-only, so naming Cookie would promise a cache
a dependency the surface never reads. It is wrong here. authn.Resolver.Resolve
prefers a bearer token when one is present but falls through to
login.UsernameFromRequest when none is, and an owner cookie resolves to
KindOwner — which is exactly what Gate admits. On this service the cookie is
the difference between the whole corpus and a 401. This is the one string a
future mcphttp retrofit has to reconcile between the two services.
An origin with no host in it was a warning and then an unguarded endpoint. The
Host allowlist is the only thing protecting /mcp once the SDK's own rebinding
guard is disabled, so that path turned one unparseable config value into a
silently open endpoint indistinguishable in every functional test from a
correctly guarded one. It is a construction error now. The daemon cannot reach
it either way: service.Config.Validate already refuses to start unless the
origin parses and carries a host.
Errors from below travelled to the agent as tool results carrying their own
text, so a dead git object store and a missing document were the same kind of
answer. A tool result means "the call was understood and the thing you asked
for is not there", so an agent reading one for a store outage concludes the
document does not exist and rewrites a specification around a document that is
perfectly real — and the store's own words reached it. errors.go splits the
two: service.ErrNotFound is a tool result whose sentence is built from the
call's own arguments, and everything else is a jsonrpc protocol error saying
"internal server error" with the detail logged. The old tests asserted the
behaviour being removed — that the agent was shown the words "on fire" — and
are replaced by ones that pin the split in both directions.