Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion evaluation_plans/evaluation-plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ var (
quality.InsightsListsRepositories,
},
"OSPS-QA-04.02": {
reusable_steps.NotImplemented,
reusable_steps.IsCodeRepo,
quality.SubprojectsEnforceSecurityRequirements,
},
"OSPS-QA-05.01": {
quality.NoBinariesInRepo,
Expand Down
94 changes: 94 additions & 0 deletions evaluation_plans/osps/quality/steps.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/gemaraproj/go-gemara"
"github.com/ossf/pvtr-github-repo-scanner/data"
"github.com/ossf/pvtr-github-repo-scanner/evaluation_plans/reusable_steps"
"github.com/ossf/si-tooling/v2/si"
sdkai "github.com/privateerproj/privateer-sdk/ai"
)

Expand Down Expand Up @@ -42,6 +43,99 @@ func InsightsListsRepositories(payload data.Payload) (result gemara.Result, mess
return gemara.Failed, "Insights does not contain a list of repositories", confidence
}

// SubprojectsEnforceSecurityRequirements assesses OSPS-QA-04.02: when the
// project has made a release comprising multiple source code repositories,
// all subprojects must enforce security requirements that are as strict or
// stricter than the primary codebase.
//
// The scanner evaluates a single repository per run, so it cannot compare
// security enforcement across repositories. This step instead narrows the
// result as far as the payload allows: it rules the requirement out when no
// release exists or when Security Insights lists no repositories beyond the
// one under evaluation, and otherwise degrades to manual review with the
// subproject repositories a reviewer needs to inspect named in the message.
func SubprojectsEnforceSecurityRequirements(payload data.Payload) (result gemara.Result, message string, confidence gemara.ConfidenceLevel) {
// The requirement only applies once a release exists.
released, observable := reusable_steps.HasPublishedRelease(payload)
if !observable {
return gemara.NeedsReview, "Release data is unavailable; manually review whether a release comprises multiple repositories and whether all subprojects enforce security requirements as strict as the primary codebase", gemara.Low
}
if !released {
return gemara.NotApplicable, "No published releases found; the subproject security requirement does not apply", gemara.High

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 (medium) This gate checks only the scanned repository's own releases, but the control text is project-scoped: "When the project has made a release comprising multiple source code repositories…". Subproject repos usually cut no releases of their own (the release comes from the primary repo), so scanning a subproject returns NotApplicable at High confidence for exactly the repositories this control targets, without ever consulting the SI repository list. If gating on the scanned repo's releases is a deliberate single-repo-scanner trade-off, it should be at most Medium confidence and called out in the doc comment; otherwise the SI repository list should be consulted before ruling the control out.

}

// Security Insights is the only observable source for the project's
// repository list (OSPS-QA-04.01 requires multi-repo projects to publish
// it there). Without it the scanner cannot tell whether subprojects exist.
if payload.InsightsError {
return gemara.NeedsReview, "Security Insights content could not be parsed, so the project's repository list is unknown; manually review whether release subprojects enforce security requirements as strict as the primary codebase", gemara.Low
}
if payload.Insights.Header.URL == "" {
return gemara.NeedsReview, "No Security Insights file was found, so the project's repository list is unknown; manually review whether release subprojects enforce security requirements as strict as the primary codebase", gemara.Low
}

subprojects := subprojectRepositories(payload.Insights)
if len(subprojects) == 0 {
return gemara.NotApplicable, "Security Insights lists no repositories beyond the one under evaluation, so no release subprojects are in scope", gemara.Medium

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 (medium) When the SI file exists but project.repositories is empty or missing, this returns NotApplicable — but the doc comment above is right that without the repository list "the scanner cannot tell whether subprojects exist". An absent list is a QA-04.01 violation, not evidence the project is single-repo, so a multi-repo project that just failed to document its repos gets silently exempted here. NeedsReview at Low confidence matches the missing-file branch above:

Suggested change
return gemara.NotApplicable, "Security Insights lists no repositories beyond the one under evaluation, so no release subprojects are in scope", gemara.Medium
return gemara.NeedsReview, "Security Insights does not list the project's repositories, so the scanner cannot tell whether release subprojects exist; manually review whether all subprojects enforce security requirements as strict or stricter than the primary codebase", gemara.Low

}

repoWord := "repositories"
if len(subprojects) == 1 {
repoWord = "repository"
}
return gemara.NeedsReview, fmt.Sprintf(
"Security Insights lists %d additional project %s (%s); the scanner evaluates one repository at a time, so manually verify each subproject enforces security requirements as strict or stricter than the primary codebase",
len(subprojects), repoWord, strings.Join(subprojects, ", ")), gemara.Low
}

// subprojectRepositories returns the URLs of repositories listed in Security
// Insights other than the repository under evaluation. The SI specification
// requires project.repositories to include the current repository, so entries
// matching repository.url are filtered out; when repository.url is absent,
// every listed entry counts. Results are deduplicated and reported in their
// original (non-normalized) form so messages stay recognizable.
func subprojectRepositories(insights si.SecurityInsights) []string {
if insights.Project == nil {
return nil
}

self := ""
if insights.Repository != nil {
self = normalizeRepoURL(string(insights.Repository.Url))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 (medium) The self-filter relies entirely on the SI-declared repository.url, but that section is optional in the SI v2 spec — ensureInsightsInitialized in data/rest-data.go substitutes an empty &si.Repository{} when it's absent (e.g. the header.project-si-source flow). Then self is "", nothing is filtered, and a project whose project.repositories lists only the scanned repo returns NeedsReview naming the repo itself as "1 additional project repository" instead of NotApplicable. The scanner already knows what it's evaluating (payload.Config.GetString("owner") / ("repo") are used later in this file), so a fallback self identity of github.com/{owner}/{repo} when repository.url is empty would close this.

}

var subprojects []string
seen := map[string]bool{}
for _, repo := range insights.Project.Repositories {
url := normalizeRepoURL(string(repo.Url))
if url == "" || url == self || seen[url] {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 (medium) Entries with an empty url are silently skipped here, and if every entry lacks one, the caller reports "Security Insights lists no repositories beyond the one under evaluation" and rules the control out. An SI file that plainly declares subprojects (by name, with urls omitted or blank) ends up NotApplicable instead of flagged. Worth counting url-less entries and degrading to NeedsReview when any exist.

continue
}
seen[url] = true
subprojects = append(subprojects, strings.TrimSpace(string(repo.Url)))
}
return subprojects
}

// normalizeRepoURL canonicalizes a repository URL for equality checks:
// lowercased, scheme and "www." stripped, and trailing slashes and a ".git"
// suffix removed. Git remote forms (git@host:owner/repo) reduce to
// host/owner/repo so they match their https equivalents.
func normalizeRepoURL(raw string) string {
url := strings.ToLower(strings.TrimSpace(raw))
url = strings.TrimPrefix(url, "http://")
url = strings.TrimPrefix(url, "https://")
url = strings.TrimPrefix(url, "ssh://")
url = strings.TrimPrefix(url, "git://")
if rest, ok := strings.CutPrefix(url, "git@"); ok {
url = strings.Replace(rest, ":", "/", 1)
}
url = strings.TrimPrefix(url, "www.")
url = strings.TrimSuffix(url, "/")
url = strings.TrimSuffix(url, ".git")
return url
}

func StatusChecksAreRequiredByRulesets(payload data.Payload) (result gemara.Result, message string, confidence gemara.ConfidenceLevel) {
// get the name of all status checks that were run
var statusChecks []string
Expand Down
182 changes: 182 additions & 0 deletions evaluation_plans/osps/quality/steps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1127,3 +1127,185 @@ func TestIsSignatureOrChecksumAsset(t *testing.T) {
}
}
}

func Test_SubprojectsEnforceSecurityRequirements(t *testing.T) {
insightsWithRepos := func(selfURL string, repoURLs ...string) si.SecurityInsights {
repos := make([]si.ProjectRepository, 0, len(repoURLs))
for _, url := range repoURLs {
repos = append(repos, si.ProjectRepository{Url: si.URL(url)})
}
return si.SecurityInsights{
Header: si.Header{URL: "https://github.com/org/repo/blob/main/security-insights.yml"},
Project: &si.Project{Repositories: repos},
Repository: &si.Repository{Url: si.URL(selfURL)},
}
}

tests := []struct {
name string
payload data.Payload
wantResult gemara.Result
wantMsgPart string
}{
{
name: "release data unobservable (nil RestData)",
payload: data.Payload{},
wantResult: gemara.NeedsReview,
wantMsgPart: "Release data is unavailable",
},
{
name: "release data unobservable (fetch error)",
payload: data.Payload{
RestData: &data.RestData{
ReleasesError: errors.New("boom"),
},
},
wantResult: gemara.NeedsReview,
wantMsgPart: "Release data is unavailable",
},
{
name: "no published releases",
payload: data.Payload{
RestData: &data.RestData{},
},
wantResult: gemara.NotApplicable,
wantMsgPart: "No published releases found",
},
{
name: "only draft releases",
payload: data.Payload{
RestData: &data.RestData{
Releases: []data.ReleaseData{{Draft: true}},
},
},
wantResult: gemara.NotApplicable,
wantMsgPart: "No published releases found",
},
{
name: "released but insights unparsable",
payload: data.Payload{
RestData: &data.RestData{
Releases: []data.ReleaseData{{Draft: false}},
InsightsError: true,
},
},
wantResult: gemara.NeedsReview,
wantMsgPart: "could not be parsed",
},
{
name: "released but no insights file",
payload: data.Payload{
RestData: &data.RestData{
Releases: []data.ReleaseData{{Draft: false}},
},
},
wantResult: gemara.NeedsReview,
wantMsgPart: "No Security Insights file was found",
},
{
name: "released, insights lists only the current repository",
payload: data.Payload{
RestData: &data.RestData{
Releases: []data.ReleaseData{{Draft: false}},
Insights: insightsWithRepos(
"https://github.com/org/repo",
"https://github.com/org/repo",
),
},
},
wantResult: gemara.NotApplicable,
wantMsgPart: "no repositories beyond the one under evaluation",
},
{
name: "released, current repo matched despite URL formatting differences",
payload: data.Payload{
RestData: &data.RestData{
Releases: []data.ReleaseData{{Draft: false}},
Insights: insightsWithRepos(
"https://github.com/org/repo",
"https://GitHub.com/Org/Repo.git/",
),
},
},
wantResult: gemara.NotApplicable,
wantMsgPart: "no repositories beyond the one under evaluation",
},
{
name: "released with subproject repositories",
payload: data.Payload{
RestData: &data.RestData{
Releases: []data.ReleaseData{{Draft: false}},
Insights: insightsWithRepos(
"https://github.com/org/repo",
"https://github.com/org/repo",
"https://github.com/org/subproject-a",
"https://github.com/org/subproject-b",
),
},
},
wantResult: gemara.NeedsReview,
wantMsgPart: "2 additional project repositories (https://github.com/org/subproject-a, https://github.com/org/subproject-b)",
},
{
name: "duplicate subproject entries are deduplicated",
payload: data.Payload{
RestData: &data.RestData{
Releases: []data.ReleaseData{{Draft: false}},
Insights: insightsWithRepos(
"https://github.com/org/repo",
"https://github.com/org/subproject-a",
"https://github.com/org/subproject-a.git",
),
},
},
wantResult: gemara.NeedsReview,
wantMsgPart: "1 additional project repository (https://github.com/org/subproject-a)",
},
{
name: "released with nil project section",
payload: data.Payload{
RestData: &data.RestData{
Releases: []data.ReleaseData{{Draft: false}},
Insights: si.SecurityInsights{
Header: si.Header{URL: "https://github.com/org/repo/blob/main/security-insights.yml"},
},
},
},
wantResult: gemara.NotApplicable,
wantMsgPart: "no repositories beyond the one under evaluation",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotResult, gotMsg, _ := SubprojectsEnforceSecurityRequirements(tt.payload)
if gotResult != tt.wantResult {
t.Errorf("result = %v, want %v", gotResult, tt.wantResult)
}
if !strings.Contains(gotMsg, tt.wantMsgPart) {
t.Errorf("message = %q, want it to contain %q", gotMsg, tt.wantMsgPart)
}
})
}
}

func Test_normalizeRepoURL(t *testing.T) {
tests := map[string]string{
"https://github.com/org/repo": "github.com/org/repo",
"http://github.com/org/repo/": "github.com/org/repo",
"https://www.github.com/Org/Repo": "github.com/org/repo",
"https://github.com/org/repo.git": "github.com/org/repo",
"git@github.com:org/repo.git": "github.com/org/repo",
"ssh://github.com/org/repo": "github.com/org/repo",
"git://github.com/org/repo": "github.com/org/repo",
" https://github.com/org/repo ": "github.com/org/repo",
"": "",
"https://gitlab.com/group/sub/repo": "gitlab.com/group/sub/repo",
}

for input, want := range tests {
if got := normalizeRepoURL(input); got != want {
t.Errorf("normalizeRepoURL(%q) = %q, want %q", input, got, want)
}
}
}
Loading