✨ feat: Refactor scorecard serve cmd #4665

Fix3dP0int · 2025-06-17T15:10:21Z

What kind of change does this PR introduce?

feature

PR title follows the guidelines defined in our pull request documentation

What is the current behavior?

The current serve uses Go’s built-in http package, which lacks modern features. And It fails to correctly aggregate the total score, and parameters and details cannot be retrieved properly.

What is the new behavior (if this is a feature change)?**

This PR refactors the serve component by migrating the original CLI-based parameter input to a RESTful API interface. Additionally, I replaced the native net/http logic with the chi router, which is lightweight yet expressive and well-suited for modular HTTP services in Go.

Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Related to #4627

spencerschrock · 2025-06-17T15:34:06Z

Don't have time to look in-depth, but wanted to make one comment:

The current serve uses Go’s built-in http package, which lacks modern features

What features specifically? Go 1.22 made good strides at least for the routing
https://go.dev/blog/routing-enhancements

Fix3dP0int · 2025-06-19T16:15:47Z

Sorry. I'm unfamiliar with new features of Go. It seems that chi isn't necessary. I will revert it. Thx.

Signed-off-by: fixedpoint <[email protected]>

spencerschrock

At a high level this looks like what we want, an http wrapper around the CLI. You said MCP will be built on top of this API, so is it matching what you need for that?

There are a few things that need changed, which I've left individual comments on. The linter also has some thoughts with this file.

spencerschrock · 2025-06-24T16:42:17Z

cmd/serve.go

+	PolicyFile  string   `json:"policy_file,omitempty"`
+}


In the CLI, policy file is a local file. How do we expect to pass a policy file to a server, with a URI?

Sorry, I'm late. I rarely use this policy file. I think that it should be a cmd parameter rather than a server parameter, so now I've temporarily removed it.

cmd/serve.go

spencerschrock · 2025-06-24T17:02:50Z

cmd/serve.go

+	if s.opts.LogLevel == "" {
+		s.opts.LogLevel = "info"


log.InfoLevel is a constant with this value.

Thanks. I have fixed it.

spencerschrock · 2025-06-24T17:12:35Z

cmd/serve.go

+	// Set options
+	s.opts.Repo = req.Repo
+	s.opts.Local = req.Local
+	s.opts.NPM = req.NPM
+	s.opts.PyPI = req.PyPI


Right now we have one options.Options, which is passed in serveCmd, and then this one copy is modified for each request. This seems like a race condition.

we should create a new struct for each request. either through options.New, or manually if we want to avoid the env var parsing.

Yeah. options.New would be a better choice. Have fixed it.

spencerschrock · 2025-06-24T17:14:34Z

cmd/serve.go

+	} else if s.opts.FileMode != options.FileModeArchive && s.opts.FileMode != options.FileModeGit {
+		http.Error(w, fmt.Sprintf("unsupported file mode: %s", s.opts.FileMode), http.StatusBadRequest)
+		return
+	}


this validation is already covered by the s.opts.Validate call below

OK. It has been removed.

spencerschrock · 2025-06-24T17:18:51Z

cmd/serve.go

+	if s.opts.Local != "" {
+		repo, err = localdir.MakeLocalDirRepo(s.opts.Local)
+		if err != nil {
+			http.Error(w, fmt.Sprintf("making local dir: %v", err), http.StatusInternalServerError)
+			return
+		}


do we expect people to pass the serve command repos which are local to the server?

This error also stems from my naive approach of simply copying the logic from root.go. When the server starts up, the repo should be a parameter provided by the client's request. Now local would be not supported.

spencerschrock · 2025-06-24T17:20:19Z

cmd/serve.go

+	enabledChecks, err := policy.GetEnabled(pol, s.opts.Checks(), requiredRequestTypes)
+	stdlog.Printf("DEBUG: enabledChecks = %#v", enabledChecks)


should this be removed? was this for your testing?

Yeah. Sorry for my carelessness. Done removing.

spencerschrock · 2025-06-24T17:24:40Z

cmd/serve.go

+		Details:     s.opts.ShowDetails,
+		Annotations: false,


we would probably want ShowAnnotations as a query parameter.

Thanks for the suggestion! I've added ShowAnnotations as a query parameter now.

cmd/serve.go

+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		start := time.Now()
+		next.ServeHTTP(w, r)
+		stdlog.Printf("%s %s %s", r.Method, r.URL, time.Since(start))


codecov · 2025-06-24T17:42:11Z

Codecov Report

Attention: Patch coverage is 0% with 211 lines in your changes missing coverage. Please review.

Project coverage is 67.71%. Comparing base (353ed60) to head (3060b2a).
Report is 198 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4665      +/-   ##
==========================================
+ Coverage   66.80%   67.71%   +0.91%     
==========================================
  Files         230      249      +19     
  Lines       16602    19044    +2442     
==========================================
+ Hits        11091    12896    +1805     
- Misses       4808     5289     +481     
- Partials      703      859     +156

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions · 2025-07-05T02:10:50Z

This pull request has been marked stale because it has been open for 10 days with no activity

github-actions · 2025-07-17T02:13:44Z

This pull request has been marked stale because it has been open for 10 days with no activity

…ions, and remove some local parameters

github-actions · 2025-08-08T02:14:06Z

This pull request has been marked stale because it has been open for 10 days with no activity

spencerschrock · 2025-09-03T21:45:25Z

Back from vacation, going through my backlog from before and reopenning PRs that went stale waiting for me.

github-project-automation bot added this to OpenSSF Scorecard Jun 17, 2025

Fix3dP0int changed the title ~~✨ feat: Refactor scorecard serve cmd (#4627)~~ ✨ feat: Refactor scorecard serve cmd Jun 17, 2025

Fix3dP0int force-pushed the main branch from e3ae82a to 78ddc3e Compare June 22, 2025 07:25

✨ Refactor scorecard serve cmd

3060b2a

Signed-off-by: fixedpoint <[email protected]>

Fix3dP0int force-pushed the main branch from 78ddc3e to 3060b2a Compare June 22, 2025 16:45

Fix3dP0int marked this pull request as ready for review June 22, 2025 16:45

Fix3dP0int requested a review from a team as a code owner June 22, 2025 16:45

Fix3dP0int requested review from spencerschrock and raghavkaul and removed request for a team June 22, 2025 16:45

spencerschrock reviewed Jun 24, 2025

View reviewed changes

Fix3dP0int temporarily deployed to gitlab June 24, 2025 17:38 — with GitHub Actions Inactive

Fix3dP0int temporarily deployed to integration-test June 24, 2025 17:38 — with GitHub Actions Inactive

github-advanced-security bot found potential problems Jun 24, 2025

View reviewed changes

github-actions bot added Stale and removed Stale labels Jul 5, 2025

github-actions bot added the Stale label Jul 17, 2025

Fix3dP0int added 2 commits July 26, 2025 18:05

🐛 fix(serve): avoid race by per-request options, support show_annotat…

8768959

…ions, and remove some local parameters

🐛 fix(serve): sanitize user input in HTTP request logs

341ca36

Fix3dP0int force-pushed the main branch from 7e858d1 to 341ca36 Compare July 26, 2025 10:05

Merge branch 'main' into main

0180be5

github-actions bot removed the Stale label Jul 27, 2025

Fix3dP0int requested a review from spencerschrock July 28, 2025 06:11

github-actions bot added the Stale label Aug 8, 2025

github-actions bot closed this Aug 29, 2025

github-project-automation bot moved this to Done in OpenSSF Scorecard Aug 29, 2025

spencerschrock reopened this Sep 3, 2025

github-actions bot removed the Stale label Sep 4, 2025

		enabledChecks, err := policy.GetEnabled(pol, s.opts.Checks(), requiredRequestTypes)
		stdlog.Printf("DEBUG: enabledChecks = %#v", enabledChecks)

✨ feat: Refactor scorecard serve cmd #4665

Are you sure you want to change the base?

✨ feat: Refactor scorecard serve cmd #4665

Uh oh!

Conversation

Fix3dP0int commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior (if this is a feature change)?**

Which issue(s) this PR fixes

Uh oh!

spencerschrock commented Jun 17, 2025

Uh oh!

Fix3dP0int commented Jun 19, 2025

Uh oh!

spencerschrock left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Check failure

Uh oh!

codecov bot commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Jul 5, 2025

Uh oh!

github-actions bot commented Jul 17, 2025

Uh oh!

github-actions bot commented Aug 8, 2025

Uh oh!

spencerschrock commented Sep 3, 2025

Uh oh!

Uh oh!

Fix3dP0int commented Jun 17, 2025 •

edited

Loading

codecov bot commented Jun 24, 2025 •

edited

Loading