diff --git a/.planning/debug/buf-v1-graph-format.md b/.planning/debug/buf-v1-graph-format.md new file mode 100644 index 0000000..a67225d --- /dev/null +++ b/.planning/debug/buf-v1-graph-format.md @@ -0,0 +1,69 @@ +--- +status: resolved +trigger: "buf 1.69.0 crashes with nil pointer dereference in graph_provider.go:140 when running `buf dep update` against the proxy." +created: 2026-06-01 +updated: 2026-06-01 +--- + +# Debug Session: buf-v1-graph-format + +## Symptoms + +- **Expected behavior:** `buf dep update` succeeds when pointing at the local proxy +- **Actual behavior phase 1:** panic with nil pointer dereference at `bufmoduleapi/graph_provider.go:140` +- **Actual behavior phase 2:** after fixing phase 1, `invalid_argument: unmarshal message: string field contains invalid UTF-8` +- **Actual behavior phase 3:** after fixing phase 2, `*** Digest verification failed ***` +- **Final result:** `buf dep update` exits with code 0 + +## Root Cause + +Buf 1.69.0 uses `DigestTypeB5` (b5) as the default digest type. This causes it to use the **v1 API** (not v1beta1) for all module operations. The proxy was built for the v1beta1 API and had three separate issues. + +### Issue 1: v1 GraphService request format + +Buf's v1 `GetGraphRequest` uses `repeated ResourceRef` directly in field 1, while v1beta1 wraps each in `GetGraphRequest_ResourceRef { ResourceRef, Registry }`. The proxy's `ServeGraph` handler used a single parser (`parseGetGraphResourceRefs`) that only understood the v1beta1 format, causing v1 requests to return an empty graph → `response.Msg.Graph == nil` → nil pointer dereference. + +### Issue 2: v1 GraphService response format + +The v1 `Graph.commits` expects `repeated Commit` directly (no wrapper), while v1beta1 wraps each in `Graph_Commit { Commit, Registry }`. The proxy returned the v1beta1 format on both paths, causing buf to mis-parse the response → `invalid UTF-8`. + +### Issue 3: B5 digest computation + +The v1 `DigestType` enum has no B4 (`DIGEST_TYPE_B5 = 1` is the only valid type). Buf uses B5 digests, which are computed differently from B4: +- B4: `SHA3-Shake256(manifest_text)` +- B5: `SHA3-Shake256("shake256:" + hex(SHA3-Shake256(manifest_text)))` (wraps B4 hash as a string) + +The proxy returned B4 digest values tagged as wire-type 1 (= B5 in v1), causing verification failure. + +## Fix + +### Changed files: + +- `internal/connect/commits.go` — Three fixes: + 1. `ServeGraph`: detect v1 vs v1beta1 path, use appropriate request parser and response format + 2. `ServeHTTP` (CommitService): apply `toB5Digest()` on v1 path + 3. `ServeGraph`: apply `toB5Digest()` on v1 path + 4. `ServeDownload`: apply `toB5Digest()` on v1 path + 5. Added `toB5Digest()` helper function +- `internal/connect/commits_helpers.go` — Added `parseGetGraphResourceRefsV1` for v1 request format +- `internal/connect/api_test.go` — Updated tests to use correct request format for each path + +### How: + +**Request parsing:** +- v1beta1 path → uses `parseGetGraphResourceRefs` (existing, wrapped format) +- v1 path → uses `parseGetGraphResourceRefsV1` (new, direct ResourceRef format) + +**Response format:** +- v1beta1 path → wraps each commit in `Graph_Commit { Commit, Registry }` +- v1 path → places `Commit` directly into `Graph.commits` + +**Digest computation (`toB5Digest`):** +- Computes `SHA3-Shake256("shake256:" + hex(b4_hash))` to match buf's B5 algorithm +- Applied to all three v1 handler paths (CommitService, GraphService, DownloadService) + +## Verification + +- All existing tests pass +- `buf dep update` in `/Users/nil/DiskD/W/yadro/cyp-hardware-manager/api/proto` exits with code 0 +- Generated buf.lock contains all 4 deps with correct B5 digests diff --git a/internal/connect/api_test.go b/internal/connect/api_test.go index 2f3b6e6..1bd21d1 100644 --- a/internal/connect/api_test.go +++ b/internal/connect/api_test.go @@ -84,6 +84,28 @@ func buildGetGraphRequest(owner, module string) []byte { return req } +// buildV1GetGraphRequest builds a v1-format GetGraph request with ResourceRef directly. +// v1 GetGraphRequest: field 1 = repeated ResourceRef { name = 2; Name { owner=1; module=2 } } +// (no GetGraphRequest_ResourceRef wrapper) +func buildV1GetGraphRequest(owner, module string) []byte { + var name []byte + name = protowire.AppendTag(name, 1, protowire.BytesType) + name = protowire.AppendString(name, owner) + name = protowire.AppendTag(name, 2, protowire.BytesType) + name = protowire.AppendString(name, module) + + var resRef []byte + resRef = protowire.AppendTag(resRef, 2, protowire.BytesType) + resRef = append(resRef, protowire.AppendVarint(nil, uint64(len(name)))...) + resRef = append(resRef, name...) + + var req []byte + req = protowire.AppendTag(req, 1, protowire.BytesType) + req = append(req, protowire.AppendVarint(nil, uint64(len(resRef)))...) + req = append(req, resRef...) + return req +} + // buildDownloadRequest builds a protobuf-encoded Download request using a commit ID. func buildDownloadRequest(commitID string) []byte { // ResourceRef: id=1 @@ -166,7 +188,7 @@ func TestV1RoutesNotReachingRootHandler(t *testing.T) { body []byte }{ {"CommitService v1", "/buf.registry.module.v1.CommitService/GetCommits", buildGetCommitsRequest("owner", "repo")}, - {"GraphService v1", "/buf.registry.module.v1.GraphService/GetGraph", buildGetGraphRequest("owner", "repo")}, + {"GraphService v1", "/buf.registry.module.v1.GraphService/GetGraph", buildV1GetGraphRequest("owner", "repo")}, } for _, tc := range v1Paths { @@ -261,13 +283,16 @@ func TestGraphServiceV1ReturnsProtobuf(t *testing.T) { io.ReadAll(commitResp.Body) commitResp.Body.Close() - for _, path := range []string{ - "/buf.registry.module.v1.GraphService/GetGraph", - "/buf.registry.module.v1beta1.GraphService/GetGraph", - } { - t.Run(path, func(t *testing.T) { - body := buildGetGraphRequest("owner", "repo") - resp, err := http.Post(server.URL+path, "application/proto", bytes.NewReader(body)) + testPaths := []struct { + path string + body []byte + }{ + {"/buf.registry.module.v1.GraphService/GetGraph", buildV1GetGraphRequest("owner", "repo")}, + {"/buf.registry.module.v1beta1.GraphService/GetGraph", buildGetGraphRequest("owner", "repo")}, + } + for _, tc := range testPaths { + t.Run(tc.path, func(t *testing.T) { + resp, err := http.Post(server.URL+tc.path, "application/proto", bytes.NewReader(tc.body)) if err != nil { t.Fatalf("request failed: %v", err) } diff --git a/internal/connect/commits.go b/internal/connect/commits.go index 647b17c..76a268a 100644 --- a/internal/connect/commits.go +++ b/internal/connect/commits.go @@ -5,6 +5,8 @@ import ( "fmt" "io" "net/http" + "encoding/hex" + "strings" "sync" "github.com/easyp-tech/server/internal/providers/content" @@ -60,11 +62,19 @@ func (h *commitServiceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) return } cid := deterministicID(meta.Commit) + isV1 := !strings.Contains(r.URL.Path, "v1beta1") digest, err := h.computeB4Digest(r, ref, meta.Commit) if err != nil { http.Error(w, fmt.Sprintf("computing digest for %s/%s: %v", ref.owner, ref.module, err), http.StatusInternalServerError) return } + if isV1 { + digest, err = toB5Digest(digest) + if err != nil { + http.Error(w, fmt.Sprintf("wrapping digest for %s/%s: %v", ref.owner, ref.module, err), http.StatusInternalServerError) + return + } + } commits = append(commits, commitInfo{ ownerID: deterministicID(ref.owner), moduleID: deterministicID(ref.owner + "/" + ref.module), @@ -124,9 +134,15 @@ func (h *commitServiceHandler) ServeGraph(w http.ResponseWriter, r *http.Request return } - // Parse GetGraphRequest: field 1 (resource_refs) repeated GetGraphRequest_ResourceRef - // Each GetGraphRequest_ResourceRef has: field 1 (ResourceRef) + field 2 (Registry string) - refs := parseGetGraphResourceRefs(body) + // Parse GetGraphRequest - handle both v1 and v1beta1 formats: + // v1beta1: field 1 = repeated GetGraphRequest_ResourceRef { ResourceRef, Registry } + isV1 := !strings.Contains(r.URL.Path, "v1beta1") + var refs []moduleRef + if isV1 { + refs = parseGetGraphResourceRefsV1(body) + } else { + refs = parseGetGraphResourceRefs(body) + } if len(refs) == 0 { // Return empty graph w.Header().Set("Content-Type", "application/proto") @@ -170,6 +186,13 @@ func (h *commitServiceHandler) ServeGraph(w http.ResponseWriter, r *http.Request http.Error(w, fmt.Sprintf("computing digest for %s/%s: %v", ref.owner, ref.module, err), http.StatusInternalServerError) return } + if isV1 { + digest, err = toB5Digest(digest) + if err != nil { + http.Error(w, fmt.Sprintf("converting digest for %s/%s: %v", ref.owner, ref.module, err), http.StatusInternalServerError) + return + } + } commits = append(commits, commitInfo{ ownerID: deterministicID(ref.owner), moduleID: deterministicID(ref.owner + "/" + ref.module), @@ -180,25 +203,32 @@ func (h *commitServiceHandler) ServeGraph(w http.ResponseWriter, r *http.Request }) } - // Build Graph response: - // GetGraphResponse { field 1: Graph { field 1: [Graph_Commit...], field 2: [Graph_Edge...] } } - // Graph_Commit { field 1 (Commit), field 2 (Registry) } + // Build Graph response. + // v1: Graph.commits = repeated Commit (direct, no wrapper, no registry) + // v1beta1: Graph.commits = repeated Graph_Commit { Commit, Registry } + // Both: GetGraphResponse { field 1: Graph { field 1: commits, field 2: edges (empty) } } var graphMsg []byte for _, c := range commits { commit := buildCommitRaw(c.commitID, c.ownerID, c.moduleID, c.digest) - // Build Graph_Commit wrapper: field 1 (Commit), field 2 (Registry) - var graphCommit []byte - graphCommit = protowire.AppendTag(graphCommit, 1, protowire.BytesType) - graphCommit = append(graphCommit, protowire.AppendVarint(nil, uint64(len(commit)))...) - graphCommit = append(graphCommit, commit...) - graphCommit = protowire.AppendTag(graphCommit, 2, protowire.BytesType) - graphCommit = protowire.AppendString(graphCommit, h.api.domain) - - // Append to Graph.commits (field 1, repeated) - graphMsg = protowire.AppendTag(graphMsg, 1, protowire.BytesType) - graphMsg = append(graphMsg, protowire.AppendVarint(nil, uint64(len(graphCommit)))...) - graphMsg = append(graphMsg, graphCommit...) + if isV1 { + // v1: Commit goes directly into Graph.commits (field 1) + graphMsg = protowire.AppendTag(graphMsg, 1, protowire.BytesType) + graphMsg = append(graphMsg, protowire.AppendVarint(nil, uint64(len(commit)))...) + graphMsg = append(graphMsg, commit...) + } else { + // v1beta1: wrap in Graph_Commit { field 1 = Commit, field 2 = Registry } + var graphCommit []byte + graphCommit = protowire.AppendTag(graphCommit, 1, protowire.BytesType) + graphCommit = append(graphCommit, protowire.AppendVarint(nil, uint64(len(commit)))...) + graphCommit = append(graphCommit, commit...) + graphCommit = protowire.AppendTag(graphCommit, 2, protowire.BytesType) + graphCommit = protowire.AppendString(graphCommit, h.api.domain) + + graphMsg = protowire.AppendTag(graphMsg, 1, protowire.BytesType) + graphMsg = append(graphMsg, protowire.AppendVarint(nil, uint64(len(graphCommit)))...) + graphMsg = append(graphMsg, graphCommit...) + } } // Wrap Graph in GetGraphResponse: field 1 (Graph) @@ -263,6 +293,9 @@ func (h *commitServiceHandler) ServeDownload(w http.ResponseWriter, r *http.Requ } cid = deterministicID(meta.Commit) digest, _ = h.computeB4DigestFromFiles(files) + if !strings.Contains(r.URL.Path, "v1beta1") { + digest, _ = toB5Digest(digest) + } } commit := buildCommitRaw(cid, cached.ownerID, cached.moduleID, digest) @@ -296,6 +329,17 @@ func (h *commitServiceHandler) ServeDownload(w http.ResponseWriter, r *http.Requ w.Header().Set("Content-Type", "application/proto") _, _ = w.Write(respMsg) } +func toB5Digest(b4Digest []byte) ([]byte, error) { + // B5 digest wraps B4 (shake256) value: SHA3-Shake256("shake256:" + hex(b4_hash)) + // This matches buf's getB5DigestForBucketAndDepDigests with zero dependencies. + digestStr := "shake256:" + hex.EncodeToString(b4Digest) + hash, err := shake256.SHA3Shake256([]byte(digestStr)) + if err != nil { + return nil, err + } + return hash[:], nil +} + func (h *commitServiceHandler) computeB4Digest(r *http.Request, ref moduleRef, commit string) ([]byte, error) { files, err := h.api.repo.GetFiles(r.Context(), ref.owner, ref.module, commit) diff --git a/internal/connect/commits_helpers.go b/internal/connect/commits_helpers.go index 3afe8a4..82b14f7 100644 --- a/internal/connect/commits_helpers.go +++ b/internal/connect/commits_helpers.go @@ -127,6 +127,35 @@ func parseGetGraphResourceRefs(msg []byte) []moduleRef { return refs } +// parseGetGraphResourceRefsV1 parses v1 GetGraphRequest where field 1 contains ResourceRef directly. +// v1 GetGraphRequest: field 1 = repeated ResourceRef { Name { owner, module, ref } } +// (no GetGraphRequest_ResourceRef wrapper) +func parseGetGraphResourceRefsV1(msg []byte) []moduleRef { + var refs []moduleRef + for len(msg) > 0 { + num, typ, n := protowire.ConsumeTag(msg) + if n < 0 { + break + } + msg = msg[n:] + if num == 1 && typ == protowire.BytesType { + v, mLen := protowire.ConsumeBytes(msg) + msg = msg[mLen:] + // v is ResourceRef directly (not wrapped in GetGraphRequest_ResourceRef) + if ref := parseResourceRef(v); ref != nil { + refs = append(refs, *ref) + } + } else { + n = protowire.ConsumeFieldValue(num, typ, msg) + if n < 0 { + break + } + msg = msg[n:] + } + } + return refs +} + func extractField1(msg []byte) []byte { for len(msg) > 0 { num, typ, n := protowire.ConsumeTag(msg)