From 197684727ea3af699da4d2d95daf73e9e37d696c Mon Sep 17 00:00:00 2001 From: Eddie Knight Date: Tue, 6 Oct 2026 11:22:22 -0500 Subject: [PATCH 1/5] feat(si): add Discover and Fetch, honour GITHUB_TOKEN and GITHUB_API_URL Discover lists the root and .github directories once each and returns the first match from DiscoveryPaths, covering the filename and location variants seen across OpenSSF repositories. Fetch exposes the raw-bytes read that Read already did internally, so callers can inspect a file that Load rejects. ErrNotFound distinguishes a missing file from a broken one. The GitHub client now authenticates with GITHUB_TOKEN when set and talks to GITHUB_API_URL when set, which also lets the tests run against a local server. Signed-off-by: Eddie Knight --- v2/si/doc.go | 1 + v2/si/github.go | 116 +++++++++++++++++++++++++++++++++++++ v2/si/github_test.go | 107 ++++++++++++++++++++++++++++++++++ v2/si/security_insights.go | 17 +----- 4 files changed, 225 insertions(+), 16 deletions(-) create mode 100644 v2/si/github.go create mode 100644 v2/si/github_test.go 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..b5677ea --- /dev/null +++ b/v2/si/github.go @@ -0,0 +1,116 @@ +package si + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/url" + "os" + "path" + "strings" + + "github.com/google/go-github/v71/github" +) + +// DiscoveryPaths lists, in priority order, the locations a repository may keep +// its Security Insights file. The spec names security-insights.yml at the root +// or under .github/; the other spellings are legacy variants seen in the wild. +var DiscoveryPaths = []string{ + "security-insights.yml", + "SECURITY-INSIGHTS.yml", + "SECURITY_INSIGHTS.yml", + "security_insights.yml", + ".github/security-insights.yml", + ".github/SECURITY-INSIGHTS.yml", + ".github/SECURITY_INSIGHTS.yml", + ".github/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 API call. 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. +func githubClient() *github.Client { + client := github.NewClient(http.DefaultClient) + if token := os.Getenv("GITHUB_TOKEN"); token != "" { + client = client.WithAuthToken(token) + } + if base, err := url.Parse(os.Getenv("GITHUB_API_URL")); err == nil && base.Host != "" { + base.Path = strings.TrimSuffix(base.Path, "/") + "/" + client.BaseURL = base + } + return client +} + +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. +func Fetch(owner, repo, filePath string) ([]byte, error) { + content, _, _, err := githubClient().Repositories.GetContents(context.Background(), 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, trying +// each entry of DiscoveryPaths in order. It lists the root and .github +// directories once each (two API calls) rather than probing every path. It +// returns ErrNotFound when none of the candidate paths exist. +func Discover(owner, repo string) (string, error) { + listed := map[string]map[string]bool{} + for _, candidate := range DiscoveryPaths { + dir := path.Dir(candidate) + if dir == "." { + dir = "" + } + names, seen := listed[dir] + if !seen { + var err error + names, err = listDir(owner, repo, dir) + if err != nil { + return "", err + } + listed[dir] = names + } + if names[path.Base(candidate)] { + return candidate, nil + } + } + return "", fmt.Errorf("%w: %s/%s", ErrNotFound, owner, repo) +} + +// listDir returns the set of entry names in a repository directory; a missing +// directory yields an empty set rather than an error. +func listDir(owner, repo, dir string) (map[string]bool, error) { + _, entries, _, err := githubClient().Repositories.GetContents(context.Background(), 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 { + 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..29935d8 --- /dev/null +++ b/v2/si/github_test.go @@ -0,0 +1,107 @@ +package si + +import ( + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeGitHub serves the subset of the contents API that Fetch and Discover use: +// files maps "path" => content; dirs maps "dir" => entry names. Anything else is a 404. +func fakeGitHub(t *testing.T, files map[string]string, dirs map[string][]string) { + t.Helper() + mux := http.NewServeMux() + mux.HandleFunc("/repos/o/r/contents/", func(w http.ResponseWriter, r *http.Request) { + p := r.URL.Path[len("/repos/o/r/contents/"):] + if body, ok := files[p]; ok { + _ = json.NewEncoder(w).Encode(map[string]string{ + "type": "file", "name": 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) +} + +func TestFetch(t *testing.T) { + fakeGitHub(t, map[string]string{"security-insights.yml": "header: {}"}, nil) + + got, err := Fetch("o", "r", "security-insights.yml") + require.NoError(t, err) + assert.Equal(t, "header: {}", string(got)) + + _, err = Fetch("o", "r", "missing.yml") + assert.ErrorIs(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: "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) { + fakeGitHub(t, nil, tt.dirs) + got, err := Discover("o", "r") + assert.ErrorIs(t, err, tt.err) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestReadUsesFetch(t *testing.T) { + fakeGitHub(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()) +} + +func TestGitHubClientEnv(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "x") + t.Setenv("GITHUB_API_URL", "https://ghe.example.com/api/v3") + assert.Equal(t, "https://ghe.example.com/api/v3/", githubClient().BaseURL.String()) + t.Setenv("GITHUB_API_URL", "") + assert.Equal(t, "https://api.github.com/", githubClient().BaseURL.String()) +} diff --git a/v2/si/security_insights.go b/v2/si/security_insights.go index 594ad0a..2a1d098 100644 --- a/v2/si/security_insights.go +++ b/v2/si/security_insights.go @@ -1,7 +1,6 @@ package si import ( - "context" "fmt" "io" "net/http" @@ -9,7 +8,6 @@ import ( "strings" "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. @@ -33,24 +31,11 @@ func fetchParentSecurityInsights(parentUrl string) (bytes []byte, err error) { return io.ReadAll(response.Body) } -func getGitHubSourceFile(owner, repo, path string) ([]byte, error) { - client := github.NewClient(http.DefaultClient) - content, _, _, err := client.Repositories.GetContents(context.Background(), owner, repo, path, nil) - if err != nil { - return nil, err - } - s, err := content.GetContent() - if err != nil { - return nil, err - } - 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(owner, repo, path) if err != nil { err = fmt.Errorf("error reading target SI: %s", err.Error()) return From 63ac2d9e5338f394b87f6c31bccbbc25af0a6293 Mon Sep 17 00:00:00 2001 From: Eddie Knight Date: Tue, 6 Oct 2026 11:22:22 -0500 Subject: [PATCH 2/5] feat(cmd/si): add 'si fetch' for machine-readable Security Insights Runs discovery and parsing over any number of targets (owner/repo, owner/repo/path, github.com blob URLs, raw.githubusercontent.com URLs) and prints a JSON array in input order. Outcomes are reported in-band as ok / not-found / invalid so one broken file does not fail the whole batch; schema-version is reported even for files Load rejects. Signed-off-by: Eddie Knight --- CHANGELOG.md | 9 +++ v2/README.md | 40 +++++++++++++ v2/cmd/si/fetch.go | 116 ++++++++++++++++++++++++++++++++++++ v2/cmd/si/fetch_test.go | 128 ++++++++++++++++++++++++++++++++++++++++ v2/cmd/si/main.go | 41 +++++++++++++ 5 files changed, 334 insertions(+) create mode 100644 v2/cmd/si/fetch.go create mode 100644 v2/cmd/si/fetch_test.go create mode 100644 v2/cmd/si/main.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 21afa40..66dba5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## Unreleased + +### Features + +- `si.Discover` finds a repository's Security Insights file among the known filename and location variants (`si.DiscoveryPaths`) using 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 +- GitHub API calls authenticate with `GITHUB_TOKEN` when set and honour `GITHUB_API_URL` + ## v2.1.0 This release is made in concert with the v2.1.0 release of [Security Insights](https://github.com/ossf/security-insights-spec) diff --git a/v2/README.md b/v2/README.md index b230c68..e6c9065 100644 --- a/v2/README.md +++ b/v2/README.md @@ -19,6 +19,46 @@ func main() { } ``` +### Discovering the file + +Repositories keep the file under a handful of names and locations. `si.Discover` lists the root and `.github/` directories (two API calls) and returns the first match from `si.DiscoveryPaths`; `si.Fetch` returns the raw bytes of a known path. + +```go +path, err := si.Discover("ossf", "scorecard") // ".github/security-insights.yml" +raw, err := si.Fetch("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. + +## 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. + +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 `ok`, `not-found` (no file at the path, or discovery found nothing) or `invalid` (the file exists but does not parse as Security Insights v2; `error` says why and `schema-version` is still reported when the header declares one). The exit code is 0 whenever every target was processed; only a usage error exits non-zero. 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..c02e9a2 --- /dev/null +++ b/v2/cmd/si/fetch.go @@ -0,0 +1,116 @@ +package main + +import ( + "errors" + "fmt" + "net/url" + "strings" + + "github.com/goccy/go-yaml" + "github.com/ossf/si-tooling/v2/si" +) + +const ( + statusOK = "ok" + statusNotFound = "not-found" + statusInvalid = "invalid" +) + +// 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"` +} + +// 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) { + var parts []string + switch { + case strings.HasPrefix(target, "https://github.com/"): + // https://github.com/owner/repo/blob// + u, perr := url.Parse(target) + if perr != nil { + return "", "", "", perr + } + parts = strings.Split(strings.Trim(u.Path, "/"), "/") + if len(parts) < 5 || parts[2] != "blob" { + return "", "", "", fmt.Errorf("expected https://github.com/owner/repo/blob//, got %s", target) + } + parts = append(parts[:2], parts[4:]...) + case strings.HasPrefix(target, "https://raw.githubusercontent.com/"): + // https://raw.githubusercontent.com/owner/repo// + u, perr := url.Parse(target) + if perr != nil { + return "", "", "", perr + } + parts = strings.Split(strings.Trim(u.Path, "/"), "/") + if len(parts) < 4 { + return "", "", "", fmt.Errorf("expected https://raw.githubusercontent.com/owner/repo//, got %s", target) + } + parts = append(parts[:2], parts[3:]...) + case strings.Contains(target, "://"): + return "", "", "", fmt.Errorf("unsupported URL %s: only github.com blob and raw.githubusercontent.com URLs are accepted", target) + default: + parts = strings.Split(strings.Trim(target, "/"), "/") + } + 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 +} + +func fetch(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(owner, repo); err != nil { + return fail(r, err) + } + } + raw, err := si.Fetch(owner, repo, r.Path) + if err != nil { + return fail(r, err) + } + r.SchemaVersion = schemaVersion(raw) + insights, err := si.Load(raw) + if err != nil { + r.Status, r.Error = statusInvalid, err.Error() + return r + } + r.Status, r.Insights = statusOK, insights + return r +} + +func fail(r result, err error) result { + r.Error = err.Error() + if errors.Is(err, si.ErrNotFound) { + r.Status = statusNotFound + } else { + r.Status = statusInvalid + } + 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. +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..396e779 --- /dev/null +++ b/v2/cmd/si/fetch_test.go @@ -0,0 +1,128 @@ +package main + +import ( + "encoding/base64" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "os" + "path" + "strings" + "testing" + + "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, statusInvalid, fail(result{}, errors.New("boom")).Status) +} + +func TestFetchBadTarget(t *testing.T) { + r := fetch("nonsense") + assert.Equal(t, statusInvalid, r.Status) + assert.Equal(t, "nonsense", r.Target) + assert.Nil(t, r.Insights) +} + +// fakeGitHub serves one repository, o/r, over the contents API: a root listing +// plus the given files. +func fakeGitHub(t *testing.T, files map[string]string) { + t.Helper() + mux := http.NewServeMux() + mux.HandleFunc("/repos/o/r/contents/", func(w http.ResponseWriter, r *http.Request) { + 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 p == "" { + var entries []map[string]string + for name := range files { + if !strings.Contains(name, "/") { + entries = append(entries, map[string]string{"type": "file", "name": name}) + } + } + _ = 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) +} + +func TestFetch(t *testing.T) { + minimal, err := os.ReadFile("../../si/test_data/minimal-v2.2.0.yml") + require.NoError(t, err) + fakeGitHub(t, map[string]string{ + "SECURITY-INSIGHTS.yml": string(minimal), + "v1.yml": "header:\n schema-version: 1.0.0\n commit-hash: abc\n", + }) + + discovered := fetch("o/r") + assert.Equal(t, statusOK, discovered.Status) + assert.Equal(t, "SECURITY-INSIGHTS.yml", discovered.Path) + assert.Equal(t, "2.2.0", discovered.SchemaVersion) + require.NotNil(t, discovered.Insights) + + exact := fetch("https://github.com/o/r/blob/main/SECURITY-INSIGHTS.yml") + assert.Equal(t, statusOK, exact.Status) + assert.Equal(t, "o", exact.Owner) + + invalid := fetch("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("o/r/nope.yml") + assert.Equal(t, statusNotFound, missing.Status) + assert.Equal(t, "nope.yml", missing.Path) + + undiscoverable := fetch("o/other") + 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..cd3d572 --- /dev/null +++ b/v2/cmd/si/main.go @@ -0,0 +1,41 @@ +// 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 (default branch is read; is ignored) +// https://raw.githubusercontent.com/owner/repo// +// +// Results are written to stdout as a JSON array in input order; diagnostics go +// to stderr. The exit code is 0 whenever every target was processed, even if +// some could not be found or parsed — that outcome is reported in each result's +// "status". Set GITHUB_TOKEN to raise the GitHub API rate limit. +package main + +import ( + "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) + } + results := make([]result, 0, len(os.Args)-2) + for _, target := range os.Args[2:] { + r := fetch(target) + fmt.Fprintf(os.Stderr, "%-9s %s\n", r.Status, target) + 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) + } +} From 884d93e4a125ca6e2a2f7e10be2cfda01ac74d88 Mon Sep 17 00:00:00 2001 From: Eddie Knight Date: Tue, 6 Oct 2026 11:24:15 -0500 Subject: [PATCH 3/5] fix(cmd/si): report transport failures as status error, exit 2 A rate limit, 5xx or network failure says nothing about the file, so it must not be reported as invalid: a consumer that hides invalid records would turn a GitHub outage into a content change. Such failures now get status "error" and the command exits 2 after printing the full array, so a workflow can refuse to commit a degraded snapshot. invalid is now strictly: fetched, but si.Load rejected it (or a malformed target). Signed-off-by: Eddie Knight --- CHANGELOG.md | 2 +- v2/README.md | 19 ++++++++++++++++++- v2/cmd/si/fetch.go | 12 ++++++++---- v2/cmd/si/fetch_test.go | 26 +++++++++++++++++++++++++- v2/cmd/si/main.go | 18 +++++++++++++++--- 5 files changed, 67 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66dba5e..ce9f157 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ - `si.Discover` finds a repository's Security Insights file among the known filename and location variants (`si.DiscoveryPaths`) using 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 +- `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 - GitHub API calls authenticate with `GITHUB_TOKEN` when set and honour `GITHUB_API_URL` ## v2.1.0 diff --git a/v2/README.md b/v2/README.md index e6c9065..dc01137 100644 --- a/v2/README.md +++ b/v2/README.md @@ -57,7 +57,24 @@ Each result looks like: } ``` -`status` is `ok`, `not-found` (no file at the path, or discovery found nothing) or `invalid` (the file exists but does not parse as Security Insights v2; `error` says why and `schema-version` is still reported when the header declares one). The exit code is 0 whenever every target was processed; only a usage error exits non-zero. Progress is written to stderr. +`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 could not be fetched at all (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 diff --git a/v2/cmd/si/fetch.go b/v2/cmd/si/fetch.go index c02e9a2..fc7111f 100644 --- a/v2/cmd/si/fetch.go +++ b/v2/cmd/si/fetch.go @@ -11,9 +11,10 @@ import ( ) const ( - statusOK = "ok" - statusNotFound = "not-found" - statusInvalid = "invalid" + 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): the file's state is unknown ) // result is one element of the JSON array `si fetch` prints. @@ -93,12 +94,15 @@ func fetch(target string) result { 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 = statusInvalid + r.Status = statusError } return r } diff --git a/v2/cmd/si/fetch_test.go b/v2/cmd/si/fetch_test.go index 396e779..bd717e0 100644 --- a/v2/cmd/si/fetch_test.go +++ b/v2/cmd/si/fetch_test.go @@ -53,7 +53,31 @@ func TestSchemaVersion(t *testing.T) { func TestFail(t *testing.T) { assert.Equal(t, statusNotFound, fail(result{}, si.ErrNotFound).Status) - assert.Equal(t, statusInvalid, fail(result{}, errors.New("boom")).Status) + assert.Equal(t, statusError, fail(result{}, errors.New("boom")).Status) +} + +func TestFetchTransportError(t *testing.T) { + 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(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("o/r").Status) } func TestFetchBadTarget(t *testing.T) { diff --git a/v2/cmd/si/main.go b/v2/cmd/si/main.go index cd3d572..ce1a1d8 100644 --- a/v2/cmd/si/main.go +++ b/v2/cmd/si/main.go @@ -10,9 +10,15 @@ // https://raw.githubusercontent.com/owner/repo// // // Results are written to stdout as a JSON array in input order; diagnostics go -// to stderr. The exit code is 0 whenever every target was processed, even if -// some could not be found or parsed — that outcome is reported in each result's -// "status". Set GITHUB_TOKEN to raise the GitHub API rate limit. +// 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, so the snapshot is incomplete and should not be trusted) +// +// Set GITHUB_TOKEN to raise the GitHub API rate limit. package main import ( @@ -27,9 +33,11 @@ func main() { os.Exit(2) } results := make([]result, 0, len(os.Args)-2) + degraded := false for _, target := range os.Args[2:] { r := fetch(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) @@ -38,4 +46,8 @@ func main() { 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) + } } From 0d070f75b962af8e965531dca042d215cc5c1e25 Mon Sep 17 00:00:00 2001 From: Eddie Knight Date: Tue, 6 Oct 2026 12:59:58 -0500 Subject: [PATCH 4/5] refactor(si): express discovery as dirs x names, drop DiscoveryPaths DiscoveryPaths spelled out every filename under every directory, and Discover then recovered the directories from it with path.Dir plus a listing cache. Two short lists and a nested loop do the same work with no cache; the spec filename comes from the existing SecurityInsightsFilename constant. Behaviour and call count are unchanged. The CLI test no longer rebuilds a root listing to re-test discovery, which the si package already covers; it keeps one empty-path case for the error mapping. Changelog gets the v2.1.1 heading. Signed-off-by: Eddie Knight --- CHANGELOG.md | 6 +++-- v2/README.md | 2 +- v2/cmd/si/fetch_test.go | 23 ++++++------------- v2/si/github.go | 49 ++++++++++++++--------------------------- 4 files changed, 29 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce9f157..3544e36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,12 @@ # Changelog -## Unreleased +## v2.1.1 + +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 (`si.DiscoveryPaths`) using two GitHub API calls +- `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 - GitHub API calls authenticate with `GITHUB_TOKEN` when set and honour `GITHUB_API_URL` diff --git a/v2/README.md b/v2/README.md index dc01137..c389a66 100644 --- a/v2/README.md +++ b/v2/README.md @@ -21,7 +21,7 @@ func main() { ### Discovering the file -Repositories keep the file under a handful of names and locations. `si.Discover` lists the root and `.github/` directories (two API calls) and returns the first match from `si.DiscoveryPaths`; `si.Fetch` returns the raw bytes of a known path. +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 path, err := si.Discover("ossf", "scorecard") // ".github/security-insights.yml" diff --git a/v2/cmd/si/fetch_test.go b/v2/cmd/si/fetch_test.go index bd717e0..1b2bac9 100644 --- a/v2/cmd/si/fetch_test.go +++ b/v2/cmd/si/fetch_test.go @@ -87,8 +87,8 @@ func TestFetchBadTarget(t *testing.T) { assert.Nil(t, r.Insights) } -// fakeGitHub serves one repository, o/r, over the contents API: a root listing -// plus the given files. +// fakeGitHub serves the files of one repository, o/r, over the contents API. +// Discovery itself is tested in package si; here the root listing is empty. func fakeGitHub(t *testing.T, files map[string]string) { t.Helper() mux := http.NewServeMux() @@ -102,13 +102,7 @@ func fakeGitHub(t *testing.T, files map[string]string) { return } if p == "" { - var entries []map[string]string - for name := range files { - if !strings.Contains(name, "/") { - entries = append(entries, map[string]string{"type": "file", "name": name}) - } - } - _ = json.NewEncoder(w).Encode(entries) + _, _ = w.Write([]byte(`[]`)) return } w.WriteHeader(http.StatusNotFound) @@ -127,15 +121,11 @@ func TestFetch(t *testing.T) { "v1.yml": "header:\n schema-version: 1.0.0\n commit-hash: abc\n", }) - discovered := fetch("o/r") - assert.Equal(t, statusOK, discovered.Status) - assert.Equal(t, "SECURITY-INSIGHTS.yml", discovered.Path) - assert.Equal(t, "2.2.0", discovered.SchemaVersion) - require.NotNil(t, discovered.Insights) - exact := fetch("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("o/r/v1.yml") assert.Equal(t, statusInvalid, invalid.Status) @@ -146,7 +136,8 @@ func TestFetch(t *testing.T) { assert.Equal(t, statusNotFound, missing.Status) assert.Equal(t, "nope.yml", missing.Path) - undiscoverable := fetch("o/other") + // Empty path runs discovery; the fake's root listing is empty. + undiscoverable := fetch("o/r") assert.Equal(t, statusNotFound, undiscoverable.Status) assert.Equal(t, "", undiscoverable.Path) } diff --git a/v2/si/github.go b/v2/si/github.go index b5677ea..f3b083c 100644 --- a/v2/si/github.go +++ b/v2/si/github.go @@ -13,19 +13,13 @@ import ( "github.com/google/go-github/v71/github" ) -// DiscoveryPaths lists, in priority order, the locations a repository may keep -// its Security Insights file. The spec names security-insights.yml at the root -// or under .github/; the other spellings are legacy variants seen in the wild. -var DiscoveryPaths = []string{ - "security-insights.yml", - "SECURITY-INSIGHTS.yml", - "SECURITY_INSIGHTS.yml", - "security_insights.yml", - ".github/security-insights.yml", - ".github/SECURITY-INSIGHTS.yml", - ".github/SECURITY_INSIGHTS.yml", - ".github/security_insights.yml", -} +// discoveryDirs and discoveryNames are the locations a repository may keep its +// Security Insights file, in priority order. The spec names security-insights.yml +// at the root or under .github/; the other spellings are legacy variants seen in the wild. +var ( + discoveryDirs = []string{"", ".github"} + discoveryNames = []string{SecurityInsightsFilename, "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") @@ -71,28 +65,19 @@ func Fetch(owner, repo, filePath string) ([]byte, error) { return []byte(s), nil } -// Discover returns the path of the repository's Security Insights file, trying -// each entry of DiscoveryPaths in order. It lists the root and .github -// directories once each (two API calls) rather than probing every path. It -// returns ErrNotFound when none of the candidate paths exist. +// 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(owner, repo string) (string, error) { - listed := map[string]map[string]bool{} - for _, candidate := range DiscoveryPaths { - dir := path.Dir(candidate) - if dir == "." { - dir = "" + for _, dir := range discoveryDirs { + names, err := listDir(owner, repo, dir) + if err != nil { + return "", err } - names, seen := listed[dir] - if !seen { - var err error - names, err = listDir(owner, repo, dir) - if err != nil { - return "", err + for _, name := range discoveryNames { + if names[name] { + return path.Join(dir, name), nil } - listed[dir] = names - } - if names[path.Base(candidate)] { - return candidate, nil } } return "", fmt.Errorf("%w: %s/%s", ErrNotFound, owner, repo) From 0b481257d5b8c01c62389d1424d756558d148299 Mon Sep 17 00:00:00 2001 From: Eddie Knight Date: Tue, 6 Oct 2026 21:29:47 -0500 Subject: [PATCH 5/5] fix(si): report unreachable parent as error, share test fake, add ctx Panel review of #69 found no duplication of existing logic, but three copies inside the PR and two bugs in what the PR itself set out to do. Load now returns ErrParentUnavailable when the project-si-source parent fails for a reason unrelated to its content (network error, or any status other than 200 and 404), and si fetch maps it to status "error" rather than "invalid", which is the case 884d93e was meant to prevent. The CHANGELOG heading is v2.3.0: v2.2.0 is already tagged and the new exported API is a feature release. Fetch and Discover take a context.Context before the API ships; Read keeps its signature. One http.Client with a 30s timeout serves GitHub and parent fetches, built once per Discover/Fetch. A GITHUB_API_URL that is not an absolute URL is an error instead of a silent fallback that would send the token to github.com. Read wraps with %w so ErrNotFound survives it. listDir matches files only. The contents-API fake shared by both test packages lives in internal/ghtest and records the Authorization header. parseTarget's two URL branches are one table. The lenient schemaVersion struct stays, with a comment: reusing si.Header loses the version when a sibling field is malformed. Tests cover the parent-unavailable path (sentinel and CLI mapping), the directory branch of Fetch, the token header and the bad GITHUB_API_URL. Coverage 83.1%. Signed-off-by: Eddie Knight --- CHANGELOG.md | 5 +- v2/README.md | 11 +++-- v2/cmd/si/fetch.go | 76 ++++++++++++++++++------------- v2/cmd/si/fetch_test.go | 70 ++++++++++++---------------- v2/cmd/si/main.go | 12 +++-- v2/internal/ghtest/ghtest.go | 53 +++++++++++++++++++++ v2/internal/ghtest/ghtest_test.go | 40 ++++++++++++++++ v2/si/github.go | 47 +++++++++++++------ v2/si/github_test.go | 73 ++++++++++++----------------- v2/si/security_insights.go | 44 +++++++++++------- v2/si/security_insights_test.go | 19 ++++++++ 11 files changed, 294 insertions(+), 156 deletions(-) create mode 100644 v2/internal/ghtest/ghtest.go create mode 100644 v2/internal/ghtest/ghtest_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 3544e36..a058945 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## v2.1.1 +## 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. @@ -9,7 +9,8 @@ This release expands on the Security Insights v2.1.0 specification by adding a n - `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 -- GitHub API calls authenticate with `GITHUB_TOKEN` when set and honour `GITHUB_API_URL` +- `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.1.0 diff --git a/v2/README.md b/v2/README.md index c389a66..093a9c6 100644 --- a/v2/README.md +++ b/v2/README.md @@ -24,12 +24,13 @@ func main() { 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 -path, err := si.Discover("ossf", "scorecard") // ".github/security-insights.yml" -raw, err := si.Fetch("ossf", "scorecard", path) +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. +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 @@ -41,7 +42,7 @@ 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. +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: @@ -64,7 +65,7 @@ Each result looks like: | `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 could not be fetched at all (rate limit, server error, network, bad token); nothing is known about it | +| `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: diff --git a/v2/cmd/si/fetch.go b/v2/cmd/si/fetch.go index fc7111f..cef19c3 100644 --- a/v2/cmd/si/fetch.go +++ b/v2/cmd/si/fetch.go @@ -1,6 +1,7 @@ package main import ( + "context" "errors" "fmt" "net/url" @@ -14,7 +15,7 @@ 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): the file's state is unknown + 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. @@ -29,37 +30,22 @@ type result struct { 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) { - var parts []string - switch { - case strings.HasPrefix(target, "https://github.com/"): - // https://github.com/owner/repo/blob// - u, perr := url.Parse(target) - if perr != nil { - return "", "", "", perr - } - parts = strings.Split(strings.Trim(u.Path, "/"), "/") - if len(parts) < 5 || parts[2] != "blob" { - return "", "", "", fmt.Errorf("expected https://github.com/owner/repo/blob//, got %s", target) - } - parts = append(parts[:2], parts[4:]...) - case strings.HasPrefix(target, "https://raw.githubusercontent.com/"): - // https://raw.githubusercontent.com/owner/repo// - u, perr := url.Parse(target) - if perr != nil { - return "", "", "", perr + parts := strings.Split(strings.Trim(target, "/"), "/") + if strings.Contains(target, "://") { + if parts, err = parseURLTarget(target); err != nil { + return "", "", "", err } - parts = strings.Split(strings.Trim(u.Path, "/"), "/") - if len(parts) < 4 { - return "", "", "", fmt.Errorf("expected https://raw.githubusercontent.com/owner/repo//, got %s", target) - } - parts = append(parts[:2], parts[3:]...) - case strings.Contains(target, "://"): - return "", "", "", fmt.Errorf("unsupported URL %s: only github.com blob and raw.githubusercontent.com URLs are accepted", target) - default: - parts = strings.Split(strings.Trim(target, "/"), "/") } if len(parts) < 2 || parts[0] == "" || parts[1] == "" { return "", "", "", fmt.Errorf("expected owner/repo[/path], got %s", target) @@ -67,7 +53,30 @@ func parseTarget(target string) (owner, repo, path string, err error) { return parts[0], parts[1], strings.Join(parts[2:], "/"), nil } -func fetch(target string) result { +// 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 { @@ -76,16 +85,19 @@ func fetch(target string) result { } r.Owner, r.Repo, r.Path = owner, repo, path if r.Path == "" { - if r.Path, err = si.Discover(owner, repo); err != nil { + if r.Path, err = si.Discover(ctx, owner, repo); err != nil { return fail(r, err) } } - raw, err := si.Fetch(owner, repo, r.Path) + 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 @@ -109,6 +121,8 @@ func fail(r result, err error) result { // 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 { diff --git a/v2/cmd/si/fetch_test.go b/v2/cmd/si/fetch_test.go index 1b2bac9..aa3fd02 100644 --- a/v2/cmd/si/fetch_test.go +++ b/v2/cmd/si/fetch_test.go @@ -1,16 +1,14 @@ package main import ( - "encoding/base64" - "encoding/json" + "context" "errors" "net/http" "net/http/httptest" "os" - "path" - "strings" "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" @@ -57,6 +55,7 @@ func TestFail(t *testing.T) { } 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) @@ -66,7 +65,7 @@ func TestFetchTransportError(t *testing.T) { t.Setenv("GITHUB_API_URL", srv.URL) for _, target := range []string{"o/r", "o/r/security-insights.yml"} { - r := fetch(target) + r := fetch(ctx, target) assert.Equal(t, statusError, r.Status, target) assert.Contains(t, r.Error, "rate limit", target) assert.Nil(t, r.Insights) @@ -77,67 +76,58 @@ func TestFetchTransportError(t *testing.T) { })) t.Cleanup(srv500.Close) t.Setenv("GITHUB_API_URL", srv500.URL) - assert.Equal(t, statusError, fetch("o/r").Status) + 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("nonsense") + r := fetch(context.Background(), "nonsense") assert.Equal(t, statusInvalid, r.Status) assert.Equal(t, "nonsense", r.Target) assert.Nil(t, r.Insights) } -// fakeGitHub serves the files of one repository, o/r, over the contents API. -// Discovery itself is tested in package si; here the root listing is empty. -func fakeGitHub(t *testing.T, files map[string]string) { - t.Helper() - mux := http.NewServeMux() - mux.HandleFunc("/repos/o/r/contents/", func(w http.ResponseWriter, r *http.Request) { - 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 p == "" { - _, _ = w.Write([]byte(`[]`)) - 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) -} - +// 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) - fakeGitHub(t, map[string]string{ + 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("https://github.com/o/r/blob/main/SECURITY-INSIGHTS.yml") + 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("o/r/v1.yml") + 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("o/r/nope.yml") + missing := fetch(ctx, "o/r/nope.yml") assert.Equal(t, statusNotFound, missing.Status) assert.Equal(t, "nope.yml", missing.Path) - // Empty path runs discovery; the fake's root listing is empty. - undiscoverable := fetch("o/r") + 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 index ce1a1d8..be782b5 100644 --- a/v2/cmd/si/main.go +++ b/v2/cmd/si/main.go @@ -6,9 +6,12 @@ // // owner/repo discover the file's location // owner/repo/path/to/security-insights.yml exact path -// https://github.com/owner/repo/blob// exact path (default branch is read; is ignored) +// 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: @@ -16,12 +19,14 @@ // 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, so the snapshot is incomplete and should not be trusted) +// 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" @@ -32,10 +37,11 @@ func main() { 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(target) + r := fetch(ctx, target) fmt.Fprintf(os.Stderr, "%-9s %s\n", r.Status, target) degraded = degraded || r.Status == statusError results = append(results, r) 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/github.go b/v2/si/github.go index f3b083c..4e083f6 100644 --- a/v2/si/github.go +++ b/v2/si/github.go @@ -24,20 +24,26 @@ var ( // 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 API call. It authenticates +// 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. -func githubClient() *github.Client { - client := github.NewClient(http.DefaultClient) +// 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 base, err := url.Parse(os.Getenv("GITHUB_API_URL")); err == nil && base.Host != "" { + 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 + return client, nil } func isNotFound(err error) bool { @@ -46,9 +52,14 @@ func isNotFound(err error) bool { } // 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. -func Fetch(owner, repo, filePath string) ([]byte, error) { - content, _, _, err := githubClient().Repositories.GetContents(context.Background(), owner, repo, filePath, nil) +// 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) @@ -68,9 +79,13 @@ func Fetch(owner, repo, filePath string) ([]byte, error) { // 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(owner, repo string) (string, error) { +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(owner, repo, dir) + names, err := listDir(ctx, client, owner, repo, dir) if err != nil { return "", err } @@ -83,10 +98,10 @@ func Discover(owner, repo string) (string, error) { return "", fmt.Errorf("%w: %s/%s", ErrNotFound, owner, repo) } -// listDir returns the set of entry names in a repository directory; a missing +// listDir returns the set of file names in a repository directory; a missing // directory yields an empty set rather than an error. -func listDir(owner, repo, dir string) (map[string]bool, error) { - _, entries, _, err := githubClient().Repositories.GetContents(context.Background(), owner, repo, dir, nil) +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 @@ -95,7 +110,9 @@ func listDir(owner, repo, dir string) (map[string]bool, error) { } names := make(map[string]bool, len(entries)) for _, e := range entries { - names[e.GetName()] = true + 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 index 29935d8..bcd82e8 100644 --- a/v2/si/github_test.go +++ b/v2/si/github_test.go @@ -1,56 +1,30 @@ package si import ( - "encoding/base64" - "encoding/json" - "net/http" - "net/http/httptest" + "context" "testing" + "github.com/ossf/si-tooling/v2/internal/ghtest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// fakeGitHub serves the subset of the contents API that Fetch and Discover use: -// files maps "path" => content; dirs maps "dir" => entry names. Anything else is a 404. -func fakeGitHub(t *testing.T, files map[string]string, dirs map[string][]string) { - t.Helper() - mux := http.NewServeMux() - mux.HandleFunc("/repos/o/r/contents/", func(w http.ResponseWriter, r *http.Request) { - p := r.URL.Path[len("/repos/o/r/contents/"):] - if body, ok := files[p]; ok { - _ = json.NewEncoder(w).Encode(map[string]string{ - "type": "file", "name": 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) -} - func TestFetch(t *testing.T) { - fakeGitHub(t, map[string]string{"security-insights.yml": "header: {}"}, nil) + 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("o", "r", "security-insights.yml") + 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("o", "r", "missing.yml") + _, 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) { @@ -83,8 +57,8 @@ func TestDiscover(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fakeGitHub(t, nil, tt.dirs) - got, err := Discover("o", "r") + 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) }) @@ -92,16 +66,27 @@ func TestDiscover(t *testing.T) { } func TestReadUsesFetch(t *testing.T) { - fakeGitHub(t, map[string]string{"security-insights.yml": string(minimalV220TestData())}, nil) + 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_TOKEN", "x") t.Setenv("GITHUB_API_URL", "https://ghe.example.com/api/v3") - assert.Equal(t, "https://ghe.example.com/api/v3/", githubClient().BaseURL.String()) + client, err := githubClient() + require.NoError(t, err) + assert.Equal(t, "https://ghe.example.com/api/v3/", client.BaseURL.String()) + t.Setenv("GITHUB_API_URL", "") - assert.Equal(t, "https://api.github.com/", githubClient().BaseURL.String()) + 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 2a1d098..a323cdb 100644 --- a/v2/si/security_insights.go +++ b/v2/si/security_insights.go @@ -1,11 +1,14 @@ package si import ( + "context" + "errors" "fmt" "io" "net/http" "strconv" "strings" + "time" "github.com/goccy/go-yaml" ) @@ -13,31 +16,40 @@ import ( // 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. const SecurityInsightsFilename = "security-insights.yml" -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) +// 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 fetchParentSecurityInsights(parentUrl string) ([]byte, error) { + response, err := httpClient.Get(parentUrl) if err != nil { - err = fmt.Errorf("error making http call: %s", err.Error()) - return + return nil, fmt.Errorf("%w: %w", ErrParentUnavailable, err) } - if response.StatusCode != http.StatusOK { - err = fmt.Errorf("unexpected response: %s", response.Status) - return + 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 io.ReadAll(response.Body) } // 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 := Fetch(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) @@ -69,7 +81,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 8719e71..147ad34 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" @@ -144,3 +146,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") +}