diff --git a/CHANGELOG.md b/CHANGELOG.md index 107edf2..35afdcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +## v2.3.0 + +This release expands on the Security Insights v2.1.0 specification by adding a new micro-CLI for non-library use cases. It also contains discovery and fetching improvements to the library. + +### Features + +- `si.Discover` finds a repository's Security Insights file among the known filename and location variants using at most two GitHub API calls +- `si.Fetch` returns the raw bytes of a file in a public GitHub repository; `si.ErrNotFound` identifies a missing file +- `si fetch` command (`v2/cmd/si`) runs discovery and parsing over many targets and prints machine-readable JSON, for catalogues and dashboards built on Security Insights data. Each result carries a status of `ok`, `not-found`, `invalid` or `error`; the command exits 2 when any target hit a transport `error`, so an incomplete batch is never mistaken for a clean one +- `si.Load` returns `si.ErrParentUnavailable` when the parent named in `project-si-source` cannot be fetched for a reason unrelated to its content; `si fetch` reports that as `error`, not `invalid` +- All GitHub API calls, including the existing `si.Read`, now authenticate with `GITHUB_TOKEN` when set and honour `GITHUB_API_URL`; all requests time out after 30 seconds + ## v2.2.1 ### Features diff --git a/v2/README.md b/v2/README.md index b230c68..093a9c6 100644 --- a/v2/README.md +++ b/v2/README.md @@ -19,6 +19,64 @@ func main() { } ``` +### Discovering the file + +Repositories keep the file under a handful of names and locations. `si.Discover` lists the root and `.github/` directories (at most two API calls) and returns the first known filename it finds; `si.Fetch` returns the raw bytes of a known path. + +```go +ctx := context.Background() +path, err := si.Discover(ctx, "ossf", "scorecard") // ".github/security-insights.yml" +raw, err := si.Fetch(ctx, "ossf", "scorecard", path) +insights, err := si.Load(raw) +``` + +Set `GITHUB_TOKEN` to authenticate GitHub API calls (unauthenticated requests are limited to 60 per hour). `GITHUB_API_URL`, which GitHub Actions exports, is honoured for GitHub Enterprise. Both apply to `si.Read` as well. Requests time out after 30 seconds. + +## Command line + +`si fetch` runs discovery and parsing over any number of targets and prints a JSON array, in input order, for other tools to consume: + +```sh +cd v2 +go run ./cmd/si fetch ossf/scorecard mindersec/minder \ + https://github.com/openbao/openbao/blob/main/.github/security-insights.yml +``` + +Targets may be `owner/repo` (discover the path), `owner/repo/`, a `github.com/.../blob/...` URL, or a `raw.githubusercontent.com` URL. URLs are read from the default branch; the ref in the URL is ignored and must be a single path segment. + +Each result looks like: + +```json +{ + "target": "ossf/scorecard", + "owner": "ossf", + "repo": "scorecard", + "path": ".github/security-insights.yml", + "status": "ok", + "schema-version": "2.0.0", + "insights": { "header": { ... }, "project": { ... }, "repository": { ... } } +} +``` + +`status` is one of: + +| status | meaning | +|---|---| +| `ok` | the file was fetched and parsed; `insights` is populated | +| `not-found` | no file at the path, or discovery found nothing | +| `invalid` | the file exists but does not parse as Security Insights v2 (`error` says why; `schema-version` is still reported when the header declares one), or the target string is malformed | +| `error` | the file, or the parent it names in `project-si-source`, could not be fetched (rate limit, server error, network, bad token); nothing is known about it | + +The full array is always printed. Exit codes: + +| exit | when | +|---|---| +| 0 | every target was processed; statuses are `ok`, `not-found` or `invalid` | +| 1 | the JSON could not be written | +| 2 | usage error, or at least one target has status `error`, so the output is incomplete and should not be committed as a snapshot | + +Progress is written to stderr. + ## Schema version support The module supports Security Insights schema version 2.x, including v2.2.0: diff --git a/v2/cmd/si/fetch.go b/v2/cmd/si/fetch.go new file mode 100644 index 0000000..cef19c3 --- /dev/null +++ b/v2/cmd/si/fetch.go @@ -0,0 +1,134 @@ +package main + +import ( + "context" + "errors" + "fmt" + "net/url" + "strings" + + "github.com/goccy/go-yaml" + "github.com/ossf/si-tooling/v2/si" +) + +const ( + statusOK = "ok" // file fetched and parsed + statusNotFound = "not-found" // no file at the path, or discovery found nothing + statusInvalid = "invalid" // file fetched but si.Load rejected it, or the target string is malformed + statusError = "error" // transport failure (rate limit, 5xx, network, bad token, unreachable parent): the file's state is unknown +) + +// result is one element of the JSON array `si fetch` prints. +type result struct { + Target string `json:"target"` + Owner string `json:"owner,omitempty"` + Repo string `json:"repo,omitempty"` + Path string `json:"path,omitempty"` + Status string `json:"status"` + Error string `json:"error,omitempty"` + SchemaVersion string `json:"schema-version,omitempty"` + Insights *si.SecurityInsights `json:"insights,omitempty"` +} + +// urlForms are the URL shapes parseTarget accepts. After owner and repo, a +// marker segment (if any) and a single ref segment precede the file path. The +// ref is ignored; the default branch is read. +var urlForms = []struct{ prefix, marker, shape string }{ + {"https://github.com/", "blob", "https://github.com/owner/repo/blob//"}, + {"https://raw.githubusercontent.com/", "", "https://raw.githubusercontent.com/owner/repo//"}, +} + +// parseTarget splits a target into owner, repo and path. An empty path means +// the caller should discover it. +func parseTarget(target string) (owner, repo, path string, err error) { + parts := strings.Split(strings.Trim(target, "/"), "/") + if strings.Contains(target, "://") { + if parts, err = parseURLTarget(target); err != nil { + return "", "", "", err + } + } + if len(parts) < 2 || parts[0] == "" || parts[1] == "" { + return "", "", "", fmt.Errorf("expected owner/repo[/path], got %s", target) + } + return parts[0], parts[1], strings.Join(parts[2:], "/"), nil +} + +// parseURLTarget reduces a supported URL to its owner, repo and path segments. +func parseURLTarget(target string) ([]string, error) { + for _, f := range urlForms { + if !strings.HasPrefix(target, f.prefix) { + continue + } + u, err := url.Parse(target) + if err != nil { + return nil, err + } + parts := strings.Split(strings.Trim(u.Path, "/"), "/") + skip := 1 // the ref + if f.marker != "" { + skip = 2 // the marker and the ref + } + if len(parts) < 3+skip || (f.marker != "" && parts[2] != f.marker) { + return nil, fmt.Errorf("expected %s, got %s", f.shape, target) + } + return append(parts[:2], parts[2+skip:]...), nil + } + return nil, fmt.Errorf("unsupported URL %s: only github.com blob and raw.githubusercontent.com URLs are accepted", target) +} + +func fetch(ctx context.Context, target string) result { + r := result{Target: target} + owner, repo, path, err := parseTarget(target) + if err != nil { + r.Status, r.Error = statusInvalid, err.Error() + return r + } + r.Owner, r.Repo, r.Path = owner, repo, path + if r.Path == "" { + if r.Path, err = si.Discover(ctx, owner, repo); err != nil { + return fail(r, err) + } + } + raw, err := si.Fetch(ctx, owner, repo, r.Path) + if err != nil { + return fail(r, err) + } + r.SchemaVersion = schemaVersion(raw) + insights, err := si.Load(raw) + if errors.Is(err, si.ErrParentUnavailable) { + return fail(r, err) + } + if err != nil { + r.Status, r.Error = statusInvalid, err.Error() + return r + } + r.Status, r.Insights = statusOK, insights + return r +} + +// fail records a Discover/Fetch failure. Only a 404 says anything about the +// file; every other failure is the transport's, and must not be mistaken for a +// bad or missing file by consumers that act on not-found/invalid. +func fail(r result, err error) result { + r.Error = err.Error() + if errors.Is(err, si.ErrNotFound) { + r.Status = statusNotFound + } else { + r.Status = statusError + } + return r +} + +// schemaVersion leniently reads header.schema-version so a file si.Load +// rejects (a v1 file, an unknown field) can still report which schema it claims. +// It deliberately does not reuse si.Header: a malformed sibling field would +// then lose the version too. +func schemaVersion(raw []byte) string { + var doc struct { + Header struct { + SchemaVersion string `yaml:"schema-version"` + } `yaml:"header"` + } + _ = yaml.Unmarshal(raw, &doc) + return doc.Header.SchemaVersion +} diff --git a/v2/cmd/si/fetch_test.go b/v2/cmd/si/fetch_test.go new file mode 100644 index 0000000..aa3fd02 --- /dev/null +++ b/v2/cmd/si/fetch_test.go @@ -0,0 +1,133 @@ +package main + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "os" + "testing" + + "github.com/ossf/si-tooling/v2/internal/ghtest" + "github.com/ossf/si-tooling/v2/si" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseTarget(t *testing.T) { + tests := []struct { + target string + owner, repo, path string + wantErr bool + }{ + {target: "ossf/scorecard", owner: "ossf", repo: "scorecard"}, + {target: "ossf/scorecard/.github/security-insights.yml", owner: "ossf", repo: "scorecard", path: ".github/security-insights.yml"}, + {target: "https://github.com/ossf/scorecard/blob/main/.github/security-insights.yml", owner: "ossf", repo: "scorecard", path: ".github/security-insights.yml"}, + {target: "https://raw.githubusercontent.com/ossf/scorecard/main/.github/security-insights.yml", owner: "ossf", repo: "scorecard", path: ".github/security-insights.yml"}, + {target: "https://github.com/ossf/scorecard", wantErr: true}, + {target: "https://github.com/ossf/scorecard/tree/main/.github", wantErr: true}, + {target: "https://raw.githubusercontent.com/ossf/scorecard", wantErr: true}, + {target: "https://gitlab.com/o/r/-/blob/main/x.yml", wantErr: true}, + {target: "scorecard", wantErr: true}, + {target: "", wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.target, func(t *testing.T) { + owner, repo, path, err := parseTarget(tt.target) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, [3]string{tt.owner, tt.repo, tt.path}, [3]string{owner, repo, path}) + }) + } +} + +func TestSchemaVersion(t *testing.T) { + assert.Equal(t, "1.0.0", schemaVersion([]byte("header:\n schema-version: 1.0.0\n commit-hash: abc\n"))) + assert.Equal(t, "", schemaVersion([]byte("not: yaml: at: all"))) +} + +func TestFail(t *testing.T) { + assert.Equal(t, statusNotFound, fail(result{}, si.ErrNotFound).Status) + assert.Equal(t, statusError, fail(result{}, errors.New("boom")).Status) +} + +func TestFetchTransportError(t *testing.T) { + ctx := context.Background() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-RateLimit-Remaining", "0") + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message":"API rate limit exceeded"}`)) + })) + t.Cleanup(srv.Close) + t.Setenv("GITHUB_API_URL", srv.URL) + + for _, target := range []string{"o/r", "o/r/security-insights.yml"} { + r := fetch(ctx, target) + assert.Equal(t, statusError, r.Status, target) + assert.Contains(t, r.Error, "rate limit", target) + assert.Nil(t, r.Insights) + } + + srv500 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + t.Cleanup(srv500.Close) + t.Setenv("GITHUB_API_URL", srv500.URL) + assert.Equal(t, statusError, fetch(ctx, "o/r").Status) +} + +func TestFetchParentUnavailable(t *testing.T) { + parent := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadGateway) + })) + t.Cleanup(parent.Close) + ghtest.Serve(t, map[string]string{ + "security-insights.yml": "header:\n schema-version: 2.0.0\n project-si-source: " + parent.URL + "\n", + }, nil) + + r := fetch(context.Background(), "o/r/security-insights.yml") + assert.Equal(t, statusError, r.Status, "an unreachable parent says nothing about the file") + assert.Equal(t, "2.0.0", r.SchemaVersion) + assert.Nil(t, r.Insights) +} + +func TestFetchBadTarget(t *testing.T) { + r := fetch(context.Background(), "nonsense") + assert.Equal(t, statusInvalid, r.Status) + assert.Equal(t, "nonsense", r.Target) + assert.Nil(t, r.Insights) +} + +// Discovery itself is tested in package si; here the fake has no root listing, +// so an owner/repo target is not-found. +func TestFetch(t *testing.T) { + ctx := context.Background() + minimal, err := os.ReadFile("../../si/test_data/minimal-v2.2.0.yml") + require.NoError(t, err) + ghtest.Serve(t, map[string]string{ + "SECURITY-INSIGHTS.yml": string(minimal), + "v1.yml": "header:\n schema-version: 1.0.0\n commit-hash: abc\n", + }, nil) + + exact := fetch(ctx, "https://github.com/o/r/blob/main/SECURITY-INSIGHTS.yml") + assert.Equal(t, statusOK, exact.Status) + assert.Equal(t, "o", exact.Owner) + assert.Equal(t, "2.2.0", exact.SchemaVersion) + require.NotNil(t, exact.Insights) + + invalid := fetch(ctx, "o/r/v1.yml") + assert.Equal(t, statusInvalid, invalid.Status) + assert.Equal(t, "1.0.0", invalid.SchemaVersion) + assert.Nil(t, invalid.Insights) + + missing := fetch(ctx, "o/r/nope.yml") + assert.Equal(t, statusNotFound, missing.Status) + assert.Equal(t, "nope.yml", missing.Path) + + undiscoverable := fetch(ctx, "o/r") + assert.Equal(t, statusNotFound, undiscoverable.Status) + assert.Equal(t, "", undiscoverable.Path) +} diff --git a/v2/cmd/si/main.go b/v2/cmd/si/main.go new file mode 100644 index 0000000..be782b5 --- /dev/null +++ b/v2/cmd/si/main.go @@ -0,0 +1,59 @@ +// Command si works with Security Insights files in public GitHub repositories. +// +// si fetch ... +// +// Each target is one of: +// +// owner/repo discover the file's location +// owner/repo/path/to/security-insights.yml exact path +// https://github.com/owner/repo/blob// exact path +// https://raw.githubusercontent.com/owner/repo// +// +// URLs are read from the default branch: is ignored and must be a single +// path segment (a ref containing "/" shifts the path). +// +// Results are written to stdout as a JSON array in input order; diagnostics go +// to stderr. A file that is missing or does not parse is reported in-band in +// its result's "status" and does not affect the exit code. Exit codes: +// +// 0 every target was processed (statuses ok, not-found or invalid) +// 1 the JSON could not be written +// 2 usage error, or at least one target has status "error" (a transport +// failure, including an unreachable parent file, so the snapshot is +// incomplete and should not be trusted) +// +// Set GITHUB_TOKEN to raise the GitHub API rate limit. +package main + +import ( + "context" + "encoding/json" + "fmt" + "os" +) + +func main() { + if len(os.Args) < 3 || os.Args[1] != "fetch" { + fmt.Fprintln(os.Stderr, "usage: si fetch ...\n\ntarget: owner/repo | owner/repo/ | GitHub blob URL | raw.githubusercontent.com URL") + os.Exit(2) + } + ctx := context.Background() + results := make([]result, 0, len(os.Args)-2) + degraded := false + for _, target := range os.Args[2:] { + r := fetch(ctx, target) + fmt.Fprintf(os.Stderr, "%-9s %s\n", r.Status, target) + degraded = degraded || r.Status == statusError + results = append(results, r) + } + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + if err := enc.Encode(results); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + if degraded { + fmt.Fprintln(os.Stderr, "si fetch: one or more targets could not be fetched; the output is incomplete") + os.Exit(2) + } +} diff --git a/v2/internal/ghtest/ghtest.go b/v2/internal/ghtest/ghtest.go new file mode 100644 index 0000000..e3975da --- /dev/null +++ b/v2/internal/ghtest/ghtest.go @@ -0,0 +1,53 @@ +// Package ghtest serves the subset of the GitHub contents API that package si +// uses, so tests in more than one package can share one fake. +package ghtest + +import ( + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "path" + "strings" + "testing" +) + +// Server is a running fake GitHub for the repository o/r. +type Server struct { + // Authorization is the Authorization header of the most recent request. + Authorization string +} + +// Serve starts a fake GitHub for repository o/r and points GITHUB_API_URL at +// it for the rest of the test. files maps a path to its content; dirs maps a +// directory to its entry names. Anything else is a 404. +func Serve(t *testing.T, files map[string]string, dirs map[string][]string) *Server { + t.Helper() + s := &Server{} + mux := http.NewServeMux() + mux.HandleFunc("/repos/o/r/contents/", func(w http.ResponseWriter, r *http.Request) { + s.Authorization = r.Header.Get("Authorization") + p := strings.TrimPrefix(r.URL.Path, "/repos/o/r/contents/") + if body, ok := files[p]; ok { + _ = json.NewEncoder(w).Encode(map[string]string{ + "type": "file", "name": path.Base(p), "encoding": "base64", + "content": base64.StdEncoding.EncodeToString([]byte(body)), + }) + return + } + if names, ok := dirs[p]; ok { + entries := make([]map[string]string, 0, len(names)) + for _, n := range names { + entries = append(entries, map[string]string{"type": "file", "name": n}) + } + _ = json.NewEncoder(w).Encode(entries) + return + } + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"Not Found"}`)) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + t.Setenv("GITHUB_API_URL", srv.URL) + return s +} diff --git a/v2/internal/ghtest/ghtest_test.go b/v2/internal/ghtest/ghtest_test.go new file mode 100644 index 0000000..160606c --- /dev/null +++ b/v2/internal/ghtest/ghtest_test.go @@ -0,0 +1,40 @@ +package ghtest + +import ( + "io" + "net/http" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestServe(t *testing.T) { + srv := Serve(t, map[string]string{"a.yml": "hi"}, map[string][]string{"": {"a.yml"}}) + base := os.Getenv("GITHUB_API_URL") + "/repos/o/r/contents/" + + get := func(p string) (int, string) { + req, err := http.NewRequest(http.MethodGet, base+p, nil) + require.NoError(t, err) + req.Header.Set("Authorization", "Bearer x") + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + return resp.StatusCode, string(body) + } + + code, body := get("a.yml") + assert.Equal(t, http.StatusOK, code) + assert.Contains(t, body, `"encoding":"base64"`) + assert.Equal(t, "Bearer x", srv.Authorization) + + code, body = get("") + assert.Equal(t, http.StatusOK, code) + assert.Contains(t, body, `"name":"a.yml"`) + + code, _ = get("nope") + assert.Equal(t, http.StatusNotFound, code) +} diff --git a/v2/si/doc.go b/v2/si/doc.go index 795b5ab..ecf3c0b 100644 --- a/v2/si/doc.go +++ b/v2/si/doc.go @@ -3,4 +3,5 @@ // Package si provides a Go API for reading and writing Security Insights data. See https://github.com/ossf/security-insights-spec?tab=readme-ov-file#readme for more details on the Security Insights specification. // // Read and Load are the two primary functions provided for unmarshaling Security Insights data. +// Discover locates a repository's file when its path is not known, and Fetch returns the raw bytes of a known path. package si diff --git a/v2/si/github.go b/v2/si/github.go new file mode 100644 index 0000000..01c13c1 --- /dev/null +++ b/v2/si/github.go @@ -0,0 +1,118 @@ +package si + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/url" + "os" + "path" + "strings" + + "github.com/google/go-github/v71/github" +) + +// discoveryDirs and discoveryNames are the locations a repository may keep its +// Security Insights file, in priority order. The spec names security-insights.yml +// or .yaml at the root or under .github/; the other spellings are legacy variants seen in the wild. +var ( + discoveryDirs = []string{"", ".github"} + discoveryNames = append(SecurityInsightsFilenames(), "SECURITY-INSIGHTS.yml", "SECURITY_INSIGHTS.yml", "security_insights.yml") +) + +// ErrNotFound is returned by Discover and Fetch when no Security Insights file exists at the requested location. +var ErrNotFound = errors.New("security insights file not found") + +// githubClient builds the client for one GitHub operation. It authenticates +// with GITHUB_TOKEN when set (unauthenticated requests are limited to 60/hour) +// and talks to GITHUB_API_URL when set, which GitHub Actions exports and which +// points tests at a local server. A GITHUB_API_URL that is not an absolute URL +// is an error rather than a silent fallback to github.com, so a token meant +// for one host is never sent to another. +func githubClient() (*github.Client, error) { + client := github.NewClient(httpClient) + if token := os.Getenv("GITHUB_TOKEN"); token != "" { + client = client.WithAuthToken(token) + } + if raw := os.Getenv("GITHUB_API_URL"); raw != "" { + base, err := url.Parse(raw) + if err != nil || base.Host == "" { + return nil, fmt.Errorf("GITHUB_API_URL %q is not an absolute URL", raw) + } + base.Path = strings.TrimSuffix(base.Path, "/") + "/" + client.BaseURL = base + } + return client, nil +} + +func isNotFound(err error) bool { + var ghErr *github.ErrorResponse + return errors.As(err, &ghErr) && ghErr.Response != nil && ghErr.Response.StatusCode == http.StatusNotFound +} + +// Fetch returns the raw contents of a file in a public GitHub repository's +// default branch. It returns ErrNotFound when the path does not exist. The +// contents API does not return files over 1 MB; those yield an error. +func Fetch(ctx context.Context, owner, repo, filePath string) ([]byte, error) { + client, err := githubClient() + if err != nil { + return nil, err + } + content, _, _, err := client.Repositories.GetContents(ctx, owner, repo, filePath, nil) + if err != nil { + if isNotFound(err) { + return nil, fmt.Errorf("%w: %s/%s/%s", ErrNotFound, owner, repo, filePath) + } + return nil, err + } + if content == nil { + return nil, fmt.Errorf("%s/%s/%s is a directory, not a file", owner, repo, filePath) + } + s, err := content.GetContent() + if err != nil { + return nil, err + } + return []byte(s), nil +} + +// Discover returns the path of the repository's Security Insights file. It +// lists the root and .github directories (at most two API calls) rather than +// probing every candidate path, and returns ErrNotFound when none exist. +func Discover(ctx context.Context, owner, repo string) (string, error) { + client, err := githubClient() + if err != nil { + return "", err + } + for _, dir := range discoveryDirs { + names, err := listDir(ctx, client, owner, repo, dir) + if err != nil { + return "", err + } + for _, name := range discoveryNames { + if names[name] { + return path.Join(dir, name), nil + } + } + } + return "", fmt.Errorf("%w: %s/%s", ErrNotFound, owner, repo) +} + +// listDir returns the set of file names in a repository directory; a missing +// directory yields an empty set rather than an error. +func listDir(ctx context.Context, client *github.Client, owner, repo, dir string) (map[string]bool, error) { + _, entries, _, err := client.Repositories.GetContents(ctx, owner, repo, dir, nil) + if err != nil { + if isNotFound(err) { + return map[string]bool{}, nil + } + return nil, err + } + names := make(map[string]bool, len(entries)) + for _, e := range entries { + if e.GetType() == "file" { + names[e.GetName()] = true + } + } + return names, nil +} diff --git a/v2/si/github_test.go b/v2/si/github_test.go new file mode 100644 index 0000000..0802a99 --- /dev/null +++ b/v2/si/github_test.go @@ -0,0 +1,97 @@ +package si + +import ( + "context" + "testing" + + "github.com/ossf/si-tooling/v2/internal/ghtest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFetch(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "x") + srv := ghtest.Serve(t, map[string]string{"security-insights.yml": "header: {}"}, map[string][]string{".github": {"workflows"}}) + ctx := context.Background() + + got, err := Fetch(ctx, "o", "r", "security-insights.yml") + require.NoError(t, err) + assert.Equal(t, "header: {}", string(got)) + assert.Equal(t, "Bearer x", srv.Authorization) + + _, err = Fetch(ctx, "o", "r", "missing.yml") + assert.ErrorIs(t, err, ErrNotFound) + + _, err = Fetch(ctx, "o", "r", ".github") + require.Error(t, err, "a directory is not a file") + assert.NotErrorIs(t, err, ErrNotFound) +} + +func TestDiscover(t *testing.T) { + tests := []struct { + name string + dirs map[string][]string + want string + err error + }{ + { + name: "root wins over .github", + dirs: map[string][]string{"": {"README.md", "SECURITY-INSIGHTS.yml"}, ".github": {"security-insights.yml"}}, + want: "SECURITY-INSIGHTS.yml", + }, + { + name: "falls back to .github", + dirs: map[string][]string{"": {"README.md"}, ".github": {"workflows", "security-insights.yml"}}, + want: ".github/security-insights.yml", + }, + { + name: "spec .yaml spelling", + dirs: map[string][]string{"": {"security-insights.yaml"}}, + want: "security-insights.yaml", + }, + { + name: "no .github directory", + dirs: map[string][]string{"": {"README.md"}}, + err: ErrNotFound, + }, + { + name: "empty repository", + dirs: map[string][]string{}, + err: ErrNotFound, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ghtest.Serve(t, nil, tt.dirs) + got, err := Discover(context.Background(), "o", "r") + assert.ErrorIs(t, err, tt.err) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestReadUsesFetch(t *testing.T) { + ghtest.Serve(t, map[string]string{"security-insights.yml": string(minimalV220TestData())}, nil) + insights, err := Read("o", "r", "security-insights.yml") + require.NoError(t, err) + assert.Equal(t, "2.2.0", insights.Header.SchemaVersion.String()) + + _, err = Read("o", "r", "missing.yml") + assert.ErrorIs(t, err, ErrNotFound) +} + +func TestGitHubClientEnv(t *testing.T) { + t.Setenv("GITHUB_API_URL", "https://ghe.example.com/api/v3") + client, err := githubClient() + require.NoError(t, err) + assert.Equal(t, "https://ghe.example.com/api/v3/", client.BaseURL.String()) + + t.Setenv("GITHUB_API_URL", "") + client, err = githubClient() + require.NoError(t, err) + assert.Equal(t, "https://api.github.com/", client.BaseURL.String()) + + t.Setenv("GITHUB_API_URL", "ghe.example.com/api/v3") + _, err = githubClient() + assert.Error(t, err, "a scheme-less URL must not fall back to github.com") +} diff --git a/v2/si/security_insights.go b/v2/si/security_insights.go index d77f793..a10f871 100644 --- a/v2/si/security_insights.go +++ b/v2/si/security_insights.go @@ -2,14 +2,15 @@ package si import ( "context" + "errors" "fmt" "io" "net/http" "strconv" "strings" + "time" "github.com/goccy/go-yaml" - "github.com/google/go-github/v71/github" ) // SecurityInsightsFilename is the expected name of the YAML file containing the insights data. See https://github.com/ossf/security-insights-spec?tab=readme-ov-file#usage for more details. @@ -24,44 +25,40 @@ func SecurityInsightsFilenames() []string { return []string{SecurityInsightsFilename, "security-insights.yaml"} } -func fetchParentSecurityInsights(parentUrl string) (bytes []byte, err error) { - request, err := http.NewRequest("GET", parentUrl, nil) - if err != nil { - return - } - client := &http.Client{} - response, err := client.Do(request) - if err != nil { - err = fmt.Errorf("error making http call: %s", err.Error()) - return - } - if response.StatusCode != http.StatusOK { - err = fmt.Errorf("unexpected response: %s", response.Status) - return - } - return io.ReadAll(response.Body) -} +// ErrParentUnavailable is returned by Load when the parent file named in +// header.project-si-source could not be fetched for a reason that says nothing +// about either file: a network failure, or any HTTP status other than 200 and +// 404. A 404 means the reference itself is broken and is reported as an +// ordinary error. +var ErrParentUnavailable = errors.New("parent security insights unavailable") + +// httpClient is shared by every request the package makes. +// ponytail: fixed 30s timeout; export a setter if a caller needs to tune it. +var httpClient = &http.Client{Timeout: 30 * time.Second} -func getGitHubSourceFile(owner, repo, path string) ([]byte, error) { - client := github.NewClient(http.DefaultClient) - content, _, _, err := client.Repositories.GetContents(context.Background(), owner, repo, path, nil) +func fetchParentSecurityInsights(parentUrl string) ([]byte, error) { + response, err := httpClient.Get(parentUrl) if err != nil { - return nil, err + return nil, fmt.Errorf("%w: %w", ErrParentUnavailable, err) } - s, err := content.GetContent() - if err != nil { - return nil, err + defer func() { _ = response.Body.Close() }() + switch response.StatusCode { + case http.StatusOK: + return io.ReadAll(response.Body) + case http.StatusNotFound: + return nil, fmt.Errorf("unexpected response: %s", response.Status) + default: + return nil, fmt.Errorf("%w: %s", ErrParentUnavailable, response.Status) } - return []byte(s), nil } // Read reads a SecurityInsights YAML file from a public GitHub repository // and returns an error if the file cannot be found or unmarshalled or returns // a SecurityInsights resulting from the unmarshalling. func Read(owner, repo, path string) (si SecurityInsights, err error) { - response, err := getGitHubSourceFile(owner, repo, path) + response, err := Fetch(context.Background(), owner, repo, path) if err != nil { - err = fmt.Errorf("error reading target SI: %s", err.Error()) + err = fmt.Errorf("error reading target SI: %w", err) return } insights, err := Load(response) @@ -93,7 +90,7 @@ func Load(contents []byte) (si *SecurityInsights, err error) { var raw []byte raw, err = fetchParentSecurityInsights(insights.Header.ProjectSISource.String()) if err != nil { - err = fmt.Errorf("error reading parent SI: %s", err.Error()) + err = fmt.Errorf("error reading parent SI: %w", err) return } parent := &SecurityInsights{} diff --git a/v2/si/security_insights_test.go b/v2/si/security_insights_test.go index aec17ab..7a10d7c 100644 --- a/v2/si/security_insights_test.go +++ b/v2/si/security_insights_test.go @@ -2,6 +2,8 @@ package si import ( "fmt" + "net/http" + "net/http/httptest" "os" "testing" @@ -155,3 +157,20 @@ func TestNewEmail(t *testing.T) { }) } } + +func TestLoadParentUnavailable(t *testing.T) { + status := http.StatusBadGateway + parent := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(status) + })) + t.Cleanup(parent.Close) + contents := []byte("header:\n schema-version: 2.0.0\n project-si-source: " + parent.URL + "\n") + + _, err := Load(contents) + assert.ErrorIs(t, err, ErrParentUnavailable, "a 5xx says nothing about either file") + + status = http.StatusNotFound + _, err = Load(contents) + require.Error(t, err) + assert.NotErrorIs(t, err, ErrParentUnavailable, "a 404 means the reference is broken") +}