Commit `65ba14e8f6`

65ba14e8f62bad8bc6e66fef53534077c6cfb58c

parent: 7953e78178

Verified · cmc ci/build: success

cmc <hello@cleberg.net> · 2026-08-27T04:09:17Z

httpd: GET /api/v1/read for conditional reads

Every API call was a POST, so nothing could carry an ETag and a client
re-fetched a full body to re-render a screen it already had.

Reads get a GET surface over the same registry — no second
implementation — admitting only commands marked ReadOnly, so a GET can
never mutate. Responses carry an ETag salted per caller, since the same
question gets different answers per account, and Cache-Control:
private, no-cache: not storable by shared caches, but revalidatable,
which is what makes the 304 worth having.

Closes #38

e2e/api_test.go +107

		@@ -6,6 +6,7 @@ import (
6	6	"fmt"
7	7	"io"
8	8	"net/http"
	9	"net/url"
9	10	"strings"
10	11	"testing"
11	12	"time"
		@@ -242,3 +243,109 @@ func mintToken(t testing.T, inst instance, key, name string) string {
242	243	}
243	244	return env.Data.Token
244	245	}
	246
	247	// apiGet fetches one read command, optionally conditionally.
	248	func (i instance) apiGet(t testing.T, token string, argv []string, ifNoneMatch string) (int, string, string) {
	249	t.Helper()
	250	q := url.Values{}
	251	for _, a := range argv {
	252	q.Add("argv", a)
	253	}
	254	req, err := http.NewRequest("GET",
	255	fmt.Sprintf("http://127.0.0.1:%d/api/v1/read?%s", i.httpPort, q.Encode()), nil)
	256	if err != nil {
	257	t.Fatal(err)
	258	}
	259	if token != "" {
	260	req.Header.Set("Authorization", "Bearer "+token)
	261	}
	262	if ifNoneMatch != "" {
	263	req.Header.Set("If-None-Match", ifNoneMatch)
	264	}
	265	resp, err := http.DefaultClient.Do(req)
	266	if err != nil {
	267	t.Fatal(err)
	268	}
	269	defer resp.Body.Close()
	270	raw, _ := io.ReadAll(resp.Body)
	271	return resp.StatusCode, resp.Header.Get("ETag"), string(raw)
	272	}
	273
	274	// TestAPIReadGET covers the conditional-request surface: reads over GET
	275	// with an ETag, 304 on revalidation, writes refused, and one caller's ETag
	276	// never matching another's.
	277	func TestAPIReadGET(t *testing.T) {
	278	inst := startInstanceWith(t, "[api]\nenabled = true\n")
	279	aliceKey := inst.newKey(t, "alice")
	280	bobKey := inst.newKey(t, "bob")
	281	inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub")
	282	inst.admin(t, "admin", "user", "create", "bob", "--key", bobKey+".pub")
	283	aliceTok := mintToken(t, inst, aliceKey, "alice-get")
	284	bobTok := mintToken(t, inst, bobKey, "bob-get")
	285	if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/app"); code != 0 {
	286	t.Fatalf("repo create: %s", errOut)
	287	}
	288
	289	status, etag, body := inst.apiGet(t, aliceTok, []string{"repo", "show", "alice/app"}, "")
	290	if status != 200 {
	291	t.Fatalf("GET read: %d %s", status, body)
	292	}
	293	if etag == "" {
	294	t.Fatal("no ETag, so a client cannot revalidate")
	295	}
	296	if !strings.Contains(body, `"alice/app"`) {
	297	t.Errorf("body: %s", body)
	298	}
	299
	300	// Revalidation returns 304 with no body — the point of the surface.
	301	status, _, body = inst.apiGet(t, aliceTok, []string{"repo", "show", "alice/app"}, etag)
	302	if status != http.StatusNotModified {
	303	t.Fatalf("revalidation status %d, want 304", status)
	304	}
	305	if body != "" {
	306	t.Errorf("304 carried a body: %q", body)
	307	}
	308	// A weak validator from an intermediary still matches.
	309	if status, _, _ := inst.apiGet(t, aliceTok, []string{"repo", "show", "alice/app"}, "W/"+etag); status != http.StatusNotModified {
	310	t.Errorf("weak ETag not honoured: %d", status)
	311	}
	312
	313	// A stale ETag gets the real body back, not a 304.
	314	if status, _, body := inst.apiGet(t, aliceTok, []string{"repo", "show", "alice/app"}, `"stale"`); status != 200 \|\| body == "" {
	315	t.Errorf("stale ETag: %d %q", status, body)
	316	}
	317
	318	// The ETag is salted per caller, so one account can never be handed a
	319	// 304 for another account's cached answer.
	320	if status, _, _ := inst.apiGet(t, bobTok, []string{"repo", "show", "alice/app"}, etag); status == http.StatusNotModified {
	321	t.Error("another caller's ETag matched")
	322	}
	323
	324	// Responses must not be storable by shared caches.
	325	req, _ := http.NewRequest("GET",
	326	fmt.Sprintf("http://127.0.0.1:%d/api/v1/read?argv=whoami", inst.httpPort), nil)
	327	req.Header.Set("Authorization", "Bearer "+aliceTok)
	328	resp, err := http.DefaultClient.Do(req)
	329	if err != nil {
	330	t.Fatal(err)
	331	}
	332	resp.Body.Close()
	333	if cc := resp.Header.Get("Cache-Control"); !strings.Contains(cc, "private") {
	334	t.Errorf("Cache-Control = %q, want private", cc)
	335	}
	336
	337	// A GET can never mutate: writes are refused by the registry's own
	338	// ReadOnly flag rather than by a hand-kept list.
	339	status, _, body = inst.apiGet(t, aliceTok, []string{"repo", "create", "alice/sneaky"}, "")
	340	if status != http.StatusBadRequest \|\| !strings.Contains(body, "POST it") {
	341	t.Fatalf("write over GET: %d %s", status, body)
	342	}
	343	if _, _, code := inst.ssh(t, aliceKey, "", "repo", "show", "alice/sneaky"); code == 0 {
	344	t.Fatal("a GET created a repository")
	345	}
	346
	347	// Unauthenticated reads are refused like everywhere else.
	348	if status, _, _ := inst.apiGet(t, "", []string{"whoami"}, ""); status != http.StatusUnauthorized {
	349	t.Errorf("anonymous GET status %d, want 401", status)
	350	}
	351	}

internal/httpd/api.go +17 −9

		@@ -76,15 +76,7 @@ func (s Server) apiCmd(w http.ResponseWriter, r http.Request) {
76	76	}
77	77	code := control.Dispatch(ctx, req.Argv)
78	78
79		status := map[int]int{
80		protocol.ExitOK: http.StatusOK,
81		protocol.ExitUsage: http.StatusBadRequest,
82		protocol.ExitNotFound: http.StatusNotFound,
83		protocol.ExitDenied: http.StatusForbidden,
84		}[code]
85		if status == 0 {
86		status = http.StatusInternalServerError
87		}
	79	status := statusForExit(code)
88	80
89	81	// Commands normally emit exactly one JSON envelope; inject exit_code.
90	82	// A few (mr diff, help) write raw text instead — wrap those.
		@@ -104,6 +96,22 @@ func (s Server) apiCmd(w http.ResponseWriter, r http.Request) {
104	96	json.NewEncoder(w).Encode(body)
105	97	}
106	98
	99	// statusForExit maps a command's exit code onto an HTTP status, shared by
	100	// both API surfaces so they cannot answer the same failure differently.
	101	func statusForExit(code int) int {
	102	switch code {
	103	case protocol.ExitOK:
	104	return http.StatusOK
	105	case protocol.ExitUsage:
	106	return http.StatusBadRequest
	107	case protocol.ExitNotFound:
	108	return http.StatusNotFound
	109	case protocol.ExitDenied:
	110	return http.StatusForbidden
	111	}
	112	return http.StatusInternalServerError
	113	}
	114
107	115	// limitKey buckets an authenticated caller by account, so rotating tokens
108	116	// buys no extra budget, and everyone else by peer address.
109	117	func limitKey(r *http.Request, user store.User) string {

internal/httpd/apiread.go added +116

		@@ -0,0 +1,116 @@
	1	package httpd
	2
	3	import (
	4	"bytes"
	5	"crypto/sha256"
	6	"encoding/hex"
	7	"encoding/json"
	8	"net/http"
	9	"strings"
	10
	11	"gitbay.org/gitbay/internal/control"
	12	"gitbay.org/gitbay/internal/protocol"
	13	)
	14
	15	// apiRead is the conditional-request half of the API: the same commands as
	16	// /api/v1/cmd, reached with GET so a response can carry an ETag and a
	17	// client can revalidate instead of refetching. A phone on a slow network
	18	// re-renders a screen for a 304 rather than a full body.
	19	//
	20	// It dispatches the same registry — no second implementation, no chance of
	21	// the two surfaces disagreeing — and admits only commands the registry
	22	// marks ReadOnly, so a GET can never mutate.
	23	//
	24	// GET /api/v1/read?argv=repo&argv=tree&argv=owner/name
	25	func (s Server) apiRead(w http.ResponseWriter, r http.Request) {
	26	user, _, ok := s.apiAuth(w, r)
	27	if !ok {
	28	return
	29	}
	30	argv := r.URL.Query()["argv"]
	31	if len(argv) == 0 {
	32	apiError(w, http.StatusBadRequest, "argv is required: ?argv=repo&argv=show&argv=owner/name")
	33	return
	34	}
	35	cmd, _, found := control.Lookup(argv)
	36	if !found {
	37	apiError(w, http.StatusNotFound, "unknown command "+argv[0])
	38	return
	39	}
	40	if !cmd.ReadOnly {
	41	// Not 405: the command exists, it is simply not a read. Saying so
	42	// is more useful than implying the URL is wrong.
	43	apiError(w, http.StatusBadRequest,
	44	joinArgv(cmd.Path)+" changes state; POST it to /api/v1/cmd")
	45	return
	46	}
	47	if allowed, wait := s.apiLimit.allow(limitKey(r, user), false); !allowed {
	48	tooManyRequests(w, wait)
	49	return
	50	}
	51
	52	var stdout, stderr bytes.Buffer
	53	ctx := &control.Ctx{
	54	User: user,
	55	Source: "api",
	56	Scope: "full",
	57	Store: s.st,
	58	Cfg: s.cfg,
	59	Stdin: strings.NewReader(""),
	60	Stdout: &stdout,
	61	Stderr: &stderr,
	62	JSON: true,
	63	ViaAPI: true,
	64	ReadOnly: true,
	65	}
	66	code := control.Dispatch(ctx, argv)
	67
	68	var body map[string]any
	69	if err := json.Unmarshal(stdout.Bytes(), &body); err != nil \|\| body == nil {
	70	body = map[string]any{"protocol_version": protocol.Version, "output": stdout.String()}
	71	}
	72	body["exit_code"] = code
	73	if msg := strings.TrimSpace(stderr.String()); msg != "" {
	74	body["stderr"] = msg
	75	}
	76	payload, err := json.Marshal(body)
	77	if err != nil {
	78	apiError(w, http.StatusInternalServerError, "internal error")
	79	return
	80	}
	81
	82	// Responses are authorized per account, so the ETag is salted with the
	83	// caller: two users asking the same question may get different answers,
	84	// and neither should ever be served the other's.
	85	sum := sha256.Sum256(append([]byte(limitKey(r, user)+"\x00"), payload...))
	86	etag := `"` + hex.EncodeToString(sum[:16]) + `"`
	87
	88	// private keeps this out of shared caches; no-cache requires a
	89	// revalidation rather than forbidding storage, which is what makes the
	90	// 304 worth having.
	91	w.Header().Set("Cache-Control", "private, no-cache")
	92	w.Header().Set("ETag", etag)
	93	w.Header().Set("Content-Type", "application/json")
	94	if match := r.Header.Get("If-None-Match"); match != "" && etagMatches(match, etag) {
	95	w.WriteHeader(http.StatusNotModified)
	96	return
	97	}
	98
	99	status := statusForExit(code)
	100	w.WriteHeader(status)
	101	w.Write(payload)
	102	}
	103
	104	// etagMatches handles the comma-separated If-None-Match list, and the weak
	105	// prefix a cache may add.
	106	func etagMatches(header, etag string) bool {
	107	for _, candidate := range strings.Split(header, ",") {
	108	candidate = strings.TrimSpace(candidate)
	109	if candidate == "*" \|\| strings.TrimPrefix(candidate, "W/") == etag {
	110	return true
	111	}
	112	}
	113	return false
	114	}
	115
	116	func joinArgv(path []string) string { return strings.Join(path, " ") }

internal/httpd/diff.go +6 −11

		@@ -155,8 +155,11 @@ func gitHeaderPaths(l string) (string, string, bool) {
155	155	}
156	156
157	157	// diffFormatter is the blob formatter without line numbers: the diff
158		// supplies its own gutters.
159		var diffFormatter = html.New(html.WithClasses(true))
	158	// supplies its own gutters. PreventSurroundingPre also drops chroma's
	159	// per-line <span class="line"> wrapper, which the generated CSS gives
	160	// display:flex — inside a diff row that breaks the +/- marker onto a line
	161	// of its own.
	162	var diffFormatter = html.New(html.WithClasses(true), html.PreventSurroundingPre(true))
160	163
161	164	// highlightFile syntax-highlights a file's diff content one hunk at a time,
162	165	// each side separately. A hunk's context+deletions are contiguous lines of
		@@ -230,15 +233,7 @@ func highlightLines(lexer chroma.Lexer, src string) []template.HTML {
230	233	if err := diffFormatter.Format(&buf, styles.Get(lightStyle), it); err != nil {
231	234	return nil
232	235	}
233		body := buf.String()
234		// Strip the wrapper chroma puts around the whole block.
235		if i := strings.Index(body, "<code"); i >= 0 {
236		if j := strings.IndexByte(body[i:], '>'); j >= 0 {
237		body = body[i+j+1:]
238		}
239		}
240		body = strings.TrimSuffix(strings.TrimSuffix(body, "</pre>"), "</code>")
241		body = strings.TrimSuffix(body, "\n")
	236	body := strings.TrimSuffix(buf.String(), "\n")
242	237
243	238	var out []template.HTML
244	239	for _, line := range splitHighlighted(body) {

internal/httpd/diff_test.go +11

		@@ -118,6 +118,17 @@ func TestParseDiffHighlighting(t *testing.T) {
118	118	}
119	119	}
120	120
	121	// chroma wraps each line in <span class="line">, and its stylesheet gives
	122	// that display:flex. A diff row is already one line, so the wrapper only
	123	// serves to break the +/- marker onto a row of its own.
	124	func TestParseDiffHasNoLineWrappers(t *testing.T) {
	125	for _, l := range parseDiff(samplePatch)[0].Lines {
	126	if strings.Contains(string(l.Code), `class="line"`) {
	127	t.Errorf("%s line %q kept chroma's line wrapper: %s", l.Class, l.Content, l.Code)
	128	}
	129	}
	130	}
	131
121	132	// A file whose type chroma does not know renders as plain text rather than
122	133	// being guessed at.
123	134	func TestParseDiffUnknownType(t *testing.T) {

internal/httpd/routes.go +1

		@@ -77,6 +77,7 @@ func (s *Server) Routes() []Route {
77	77	if s.cfg.API.Enabled {
78	78	routes = append(routes,
79	79	Route{Method: "POST", Pattern: "/api/v1/cmd", Mutating: true, Handler: s.apiCmd},
	80	Route{Method: "GET", Pattern: "/api/v1/read", Handler: s.apiRead},
80	81	)
81	82	}
82	83

Commit 65ba14e8f6

Commit `65ba14e8f6`