From 1a044ae3117e694fe2abdf5273186391d4925c3f Mon Sep 17 00:00:00 2001 From: Lewis Date: Thu, 30 Jul 2026 11:59:23 +0300 Subject: [PATCH] cmd/zoekt-tngl-indexserver: validate branch names & object ids before running git Lewis: May this revision serve well! --- cmd/zoekt-tngl-indexserver/index.go | 40 ++++------- cmd/zoekt-tngl-indexserver/main.go | 5 +- cmd/zoekt-tngl-indexserver/server.go | 74 +++++++++++++++++--- cmd/zoekt-tngl-indexserver/server_test.go | 84 +++++++++++++++++++++++ 4 files changed, 164 insertions(+), 39 deletions(-) create mode 100644 cmd/zoekt-tngl-indexserver/server_test.go diff --git a/cmd/zoekt-tngl-indexserver/index.go b/cmd/zoekt-tngl-indexserver/index.go index da04667e..6b0e5a70 100644 --- a/cmd/zoekt-tngl-indexserver/index.go +++ b/cmd/zoekt-tngl-indexserver/index.go @@ -4,6 +4,7 @@ import ( "context" "crypto/sha1" "encoding/json" + "errors" "fmt" "io" "net/url" @@ -12,7 +13,7 @@ import ( "github.com/bluesky-social/indigo/atproto/identity" "github.com/bluesky-social/indigo/atproto/syntax" - "github.com/sourcegraph/zoekt" + "github.com/samber/lo" "tangled.org/core/repoident" "tangled.org/core/repoverify" ) @@ -71,7 +72,7 @@ func loadRepo(ctx context.Context, cfg *Config, dir identity.Directory, repoDID }, nil } -func fetchRepo(ctx context.Context, gitDir, cloneUrl string, branches []zoekt.RepositoryBranch) error { +func fetchRepo(ctx context.Context, gitDir, cloneUrl string, branches []indexBranch) error { // Create a repo to fetch into if err := executeCmd(ctx, "git", @@ -87,38 +88,27 @@ func fetchRepo(ctx context.Context, gitDir, cloneUrl string, branches []zoekt.Re return err } - fetchArgs := []string{ + fetchArgs := append([]string{ "-C", gitDir, "-c", "protocol.version=2", "fetch", "--depth=1", "--no-tags", - } - // Git's blob:limit filter excludes blobs whose size is >= the given limit, - // while zoekt indexes files up to and including FileLimit bytes. - fetchArgs = append(fetchArgs, fmt.Sprintf("--filter=blob:limit=%d", int64(MaxFileSize)+1)) - - fetchArgs = append(fetchArgs, cloneUrl) - - var commits []string - for _, b := range branches { - commits = append(commits, b.Version) - } - fetchArgs = append(fetchArgs, commits...) + // Git's blob:limit filter excludes blobs whose size is >= the given limit, + // while zoekt indexes files up to and including FileLimit bytes. + fmt.Sprintf("--filter=blob:limit=%d", int64(MaxFileSize)+1), + cloneUrl, + }, lo.Map(branches, func(b indexBranch, _ int) string { return string(b.Version) })...) if err := executeCmd(ctx, "git", fetchArgs...); err != nil { return err } - for _, b := range branches { - ref := b.Name - if ref != "HEAD" { - ref = "refs/heads/" + ref + return errors.Join(lo.FilterMap(branches, func(b indexBranch, _ int) (error, bool) { + err := executeCmd(ctx, "git", "-C", gitDir, "update-ref", b.Name.Ref(), string(b.Version)) + if err == nil { + return nil, false } - if err := executeCmd(ctx, "git", "-C", gitDir, "update-ref", ref, b.Version); err != nil { - return fmt.Errorf("failed update-ref %s to %s: %w", ref, b.Version, err) - } - } - - return nil + return fmt.Errorf("failed update-ref %s to %s: %w", b.Name.Ref(), b.Version, err), true + })...) } func indexRepo(ctx context.Context, cfg *Config, gitDir string, repo Repo) error { diff --git a/cmd/zoekt-tngl-indexserver/main.go b/cmd/zoekt-tngl-indexserver/main.go index 7ae942ce..4a197d05 100644 --- a/cmd/zoekt-tngl-indexserver/main.go +++ b/cmd/zoekt-tngl-indexserver/main.go @@ -21,7 +21,6 @@ import ( "github.com/bluesky-social/indigo/atproto/syntax" "github.com/carlmjohnson/versioninfo" "github.com/samber/lo" - "github.com/sourcegraph/zoekt" "github.com/sourcegraph/zoekt/gitindex" "github.com/sourcegraph/zoekt/index" "github.com/urfave/cli/v3" @@ -175,7 +174,7 @@ type Repo struct { Owner repoident.OwnerDid Slug syntax.RecordKey Knot repoident.KnotURL - Branches []zoekt.RepositoryBranch + Branches []indexBranch } func (r *Repo) CloneURL() string { @@ -227,7 +226,7 @@ func runIndex(ctx context.Context, cmd *cli.Command) error { return fmt.Errorf("repo is missing did, owner, or knot: %q", repoRaw) } - branches := lo.Map(repo.Branches, func(b zoekt.RepositoryBranch, _ int) string { return b.Name }) + branches := lo.Map(repo.Branches, func(b indexBranch, _ int) string { return string(b.Name) }) buildOpts := index.Options{} buildOpts.SetDefaults() diff --git a/cmd/zoekt-tngl-indexserver/server.go b/cmd/zoekt-tngl-indexserver/server.go index de983555..61ae6cf6 100644 --- a/cmd/zoekt-tngl-indexserver/server.go +++ b/cmd/zoekt-tngl-indexserver/server.go @@ -3,16 +3,20 @@ package main import ( "context" "encoding/json" + "errors" "fmt" "log" "net/http" + "regexp" "strconv" + "strings" "time" "github.com/bluesky-social/indigo/atproto/identity" + "github.com/go-git/go-git/v5/plumbing" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" - "github.com/sourcegraph/zoekt" + "github.com/samber/lo" "tangled.org/core/repoident" ) @@ -58,9 +62,60 @@ func (s *IndexServer) handleMetrics(w http.ResponseWriter, r *http.Request) { promhttp.Handler().ServeHTTP(w, r) } +type branchName string + +func (b branchName) Ref() string { + return lo.Ternary(b == "HEAD", "HEAD", "refs/heads/"+string(b)) +} + +func (b *branchName) UnmarshalText(text []byte) error { + name := branchName(text) + if strings.HasPrefix(string(name), "refs/") { + return fmt.Errorf("branch %q must be a short name, without the refs/ prefix", name) + } + if err := plumbing.ReferenceName(name.Ref()).Validate(); err != nil { + return fmt.Errorf("branch %q isn't a valid ref: %w", name, err) + } + *b = name + return nil +} + +type objectID string + +var objectIDPattern = regexp.MustCompile(`^([0-9a-fA-F]{40}|[0-9a-fA-F]{64})$`) + +func (o *objectID) UnmarshalText(text []byte) error { + if !objectIDPattern.Match(text) { + return fmt.Errorf("%q isn't a sha1 or sha256 object id", text) + } + *o = objectID(text) + return nil +} + +type indexBranch struct { + Name branchName `json:"name"` + Version objectID `json:"version"` +} + type indexRequest struct { - Repo repoident.RepoDid `json:"repo"` - Branches []zoekt.RepositoryBranch `json:"branches"` + Repo repoident.RepoDid `json:"repo"` + Branches []indexBranch `json:"branches"` +} + +func decodeIndexRequest(r *http.Request) (indexRequest, error) { + dec := json.NewDecoder(r.Body) + dec.DisallowUnknownFields() + var req indexRequest + if err := dec.Decode(&req); err != nil { + return indexRequest{}, err + } + if req.Repo == "" { + return indexRequest{}, errors.New("index request has no repo did") + } + if len(req.Branches) == 0 { + return indexRequest{}, fmt.Errorf("index request for %s has no branches", req.Repo) + } + return req, nil } func (s *IndexServer) handleDebugQueue(w http.ResponseWriter, r *http.Request) { @@ -74,10 +129,8 @@ func (s *IndexServer) handleDebugQueue(w http.ResponseWriter, r *http.Request) { func (s *IndexServer) handleEnqueueIndex(w http.ResponseWriter, r *http.Request) { route := "enqueueIndex" - dec := json.NewDecoder(r.Body) - dec.DisallowUnknownFields() - var req indexRequest - if err := dec.Decode(&req); err != nil { + req, err := decodeIndexRequest(r) + if err != nil { log.Printf("Error decoding index request: %v", err) http.Error(w, "JSON parser error", http.StatusBadRequest) s.incrementRequestsTotal(r.Method, route, http.StatusBadRequest) @@ -97,12 +150,11 @@ func (s *IndexServer) handleEnqueueIndex(w http.ResponseWriter, r *http.Request) func (s *IndexServer) handleForceIndex(w http.ResponseWriter, r *http.Request) { route := "index" - dec := json.NewDecoder(r.Body) - dec.DisallowUnknownFields() - var req indexRequest - if err := dec.Decode(&req); err != nil { + req, err := decodeIndexRequest(r) + if err != nil { log.Printf("Error decoding index request: %v", err) http.Error(w, "JSON parser error", http.StatusBadRequest) + s.incrementRequestsTotal(r.Method, route, http.StatusBadRequest) return } diff --git a/cmd/zoekt-tngl-indexserver/server_test.go b/cmd/zoekt-tngl-indexserver/server_test.go new file mode 100644 index 00000000..dc377015 --- /dev/null +++ b/cmd/zoekt-tngl-indexserver/server_test.go @@ -0,0 +1,84 @@ +package main + +import ( + "fmt" + "net/http/httptest" + "strings" + "testing" + + "github.com/bluesky-social/indigo/atproto/syntax" + "tangled.org/core/repoident" +) + +var ( + sha1Oid = strings.Repeat("a", 40) + sha256Oid = strings.Repeat("b", 64) +) + +func decodeBody(t *testing.T, body string) (indexRequest, error) { + t.Helper() + return decodeIndexRequest(httptest.NewRequest("POST", "/admin/enqueueIndex", strings.NewReader(body))) +} + +func TestDecodeIndexRequest_Accepts(t *testing.T) { + cases := map[string]struct { + name, oid, wantRef string + }{ + "a branch and a sha1": {name: "main", oid: sha1Oid, wantRef: "refs/heads/main"}, + "HEAD and a sha256": {name: "HEAD", oid: sha256Oid, wantRef: "HEAD"}, + } + for label, tc := range cases { + t.Run(label, func(t *testing.T) { + req, err := decodeBody(t, fmt.Sprintf(`{"repo":"did:plc:limpet","branches":[{"Name":%q,"Version":%q}]}`, tc.name, tc.oid)) + if err != nil { + t.Fatalf("decodeIndexRequest: %v", err) + } + if req.Repo.String() != "did:plc:limpet" { + t.Errorf("Repo = %q, want did:plc:limpet", req.Repo) + } + want := indexBranch{Name: branchName(tc.name), Version: objectID(tc.oid)} + if len(req.Branches) != 1 || req.Branches[0] != want { + t.Errorf("Branches = %v, want %v", req.Branches, want) + } + if got := req.Branches[0].Name.Ref(); got != tc.wantRef { + t.Errorf("Ref = %q, want %q", got, tc.wantRef) + } + }) + } +} + +func TestDecodeIndexRequest_RejectsBadRequests(t *testing.T) { + cases := map[string]string{ + "branch name is a git option": fmt.Sprintf(`{"repo":"did:plc:limpet","branches":[{"Name":"-d","Version":%q}]}`, sha1Oid), + "version is a git option": `{"repo":"did:plc:limpet","branches":[{"Name":"main","Version":"--upload-pack=touch /tmp/pwned"}]}`, + "version is a ref": `{"repo":"did:plc:limpet","branches":[{"Name":"main","Version":"refs/heads/main"}]}`, + "version is short hex": `{"repo":"did:plc:limpet","branches":[{"Name":"main","Version":"deadbeef"}]}`, + "branch name walks up": fmt.Sprintf(`{"repo":"did:plc:limpet","branches":[{"Name":"../../objects","Version":%q}]}`, sha1Oid), + "branch name is a full ref": fmt.Sprintf(`{"repo":"did:plc:limpet","branches":[{"Name":"refs/heads/main","Version":%q}]}`, sha1Oid), + "branch name is empty": fmt.Sprintf(`{"repo":"did:plc:limpet","branches":[{"Name":"","Version":%q}]}`, sha1Oid), + "repo isn't a did": fmt.Sprintf(`{"repo":"limpet","branches":[{"Name":"main","Version":%q}]}`, sha1Oid), + "repo is absent": fmt.Sprintf(`{"branches":[{"Name":"main","Version":%q}]}`, sha1Oid), + "branches are absent": `{"repo":"did:plc:limpet"}`, + "branches are empty": `{"repo":"did:plc:limpet","branches":[]}`, + "unknown field": fmt.Sprintf(`{"repo":"did:plc:limpet","branches":[{"Name":"main","Version":%q}],"shards":3}`, sha1Oid), + } + for name, body := range cases { + t.Run(name, func(t *testing.T) { + if _, err := decodeBody(t, body); err == nil { + t.Errorf("decodeIndexRequest accepted %s", body) + } + }) + } +} + +func TestRepoCloneURL(t *testing.T) { + knot, err := repoident.ParseKnotURL("https://knot.oyster.cafe", repoident.RequireHTTPS) + if err != nil { + t.Fatalf("ParseKnotURL: %v", err) + } + repo := Repo{Did: "did:plc:limpet", Owner: "did:plc:akshay", Slug: syntax.RecordKey("3kkkkkkkkkkkk"), Knot: knot} + const want = "https://knot.oyster.cafe/did:plc:limpet" + if got := repo.CloneURL(); got != want { + t.Errorf("CloneURL = %q, want %q", got, want) + } +} -- 2.51.2