Skip to content

Fix critical and high vulns - #5066

Merged
lukaszgryglicki merged 3 commits into
devfrom
unicron-fix-vulns
May 26, 2026
Merged

Fix critical and high vulns#5066
lukaszgryglicki merged 3 commits into
devfrom
unicron-fix-vulns

Conversation

@lukaszgryglicki

Copy link
Copy Markdown
Member

Fixes critical vulns.

Assisted by OpenAI

Assisted by GitHub Copilot

Assisted by Claude

cc @mlehotskylf @ahmedomosanya

Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io>
@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 993a65cf-b543-4cad-9f39-1a28c1532f33

📥 Commits

Reviewing files that changed from the base of the PR and between 9039c28 and ce5dc92.

📒 Files selected for processing (1)
  • .github/workflows/security-scan-go.yml

Walkthrough

Bumps Go toolchains and many dependencies, replaces imroc/req with net/http in token and metrics code, updates Node dependency pins, enforces govulncheck in CI, clears yarn audit allowlist, and adds minor infra edits.

Changes

Maintenance and Upgrade

Layer / File(s) Summary
Token Service HTTP Client Migration
cla-backend-go/token/token.go
retrieveToken replaces imroc/req with net/http, adds JSON marshal/unmarshal, shared 2-minute httpClient, full response-body reads, and 2xx status validation.
Metrics Service HTTP Client Migration
cla-backend-go/v2/metrics/repository.go
cacheProjectMembership migrates from req to net/http, builds requests with Bearer auth and Accept headers, validates 200–299 responses, reads full body, and unmarshals member names into LfMembers.
Go Toolchain and Backend Dependencies
cla-backend-go/go.mod, cla-backend-legacy/go.mod
Go toolchain updated to 1.25.x; many direct and indirect modules refreshed, including gin-gonic/gin, golang.org/x/*, and google.golang.org/grpc bumped to v1.79.3.
Node.js Dependency Updates
cla-backend-go/package.json, cla-backend/package.json
simple-git upgraded to ^3.36.0; multiple transitive dependencies pinned/updated via resolutions (e.g., fast-xml-parser, axios, lodash).
Vulnerability Auditing and Infrastructure Edits
.github/workflows/go-audit.yml, .github/workflows/security-scan-go.yml, .yarn-audit-allowlist.json, .gitignore, cla-backend-go/cmd/server_standalone.go
CI govulncheck steps added/enforced without continue-on-error; .yarn-audit-allowlist.json cleared; .claude/ added to .gitignore; server signal channel changed to make(chan os.Signal, 1).

Sequence diagrams:

sequenceDiagram
  participant retrieveToken
  participant httpClient
  participant oauth_server
  retrieveToken->>httpClient: POST JSON (application/json)
  httpClient->>oauth_server: HTTP POST
  oauth_server->>httpClient: JSON response (200-299)
  httpClient->>retrieveToken: full response body -> unmarshal -> update token/expiry
Loading
sequenceDiagram
  participant cacheProjectMembership
  participant httpClient
  participant project_service
  participant LfMembers
  cacheProjectMembership->>httpClient: GET (Bearer auth, Accept: application/json)
  httpClient->>project_service: HTTP GET
  project_service->>httpClient: JSON response (200-299)
  httpClient->>cacheProjectMembership: full response body -> unmarshal -> store names in LfMembers
Loading

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix critical and high vulns' accurately describes the main objective of the changeset, which involves addressing vulnerabilities across multiple files including dependency updates, workflow adjustments, and code modifications.
Description check ✅ Passed The description 'Fixes critical vulns' is directly related to the changeset, which contains vulnerability fixes through dependency updates and security scanning improvements.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch unicron-fix-vulns

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread cla-backend-go/go.mod
Comment thread cla-backend-go/go.mod
Comment thread cla-backend-go/go.mod
Comment thread cla-backend-go/yarn.lock
Comment thread cla-backend-go/go.mod
Comment thread cla-backend-go/go.mod
Comment thread cla-backend-go/yarn.lock
Comment thread cla-backend-go/go.mod
Comment thread cla-backend-go/go.mod

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR focuses on remediating critical/high vulnerabilities across the Node (serverless backends) and Go services by upgrading dependency versions, removing a third-party HTTP client, and tightening CI vulnerability scanning so regressions fail builds.

Changes:

  • Upgraded Node dependency pins/resolutions (e.g., axios, basic-ftp, simple-git, fast-xml-*) and updated lockfile(s) to remove allowlisted high/critical advisories.
  • Upgraded Go toolchain/dependencies (including gin and golang.org/x/*) and removed github.com/imroc/req usage by switching to net/http.
  • Updated GitHub Actions security workflows to run govulncheck without continue-on-error and removed the Nancy step.

Reviewed changes

Copilot reviewed 10 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
cla-backend/yarn.lock Locks updated transitive dependency versions to incorporate vulnerability fixes (axios/basic-ftp/fast-xml-* etc.).
cla-backend/package.json Bumps simple-git and refreshes resolutions to force patched versions.
cla-backend-legacy/go.mod Updates Go toolchain and indirect Go deps to patched versions.
cla-backend-legacy/go.sum Syncs module checksums for upgraded dependencies.
cla-backend-go/v2/metrics/repository.go Replaces imroc/req HTTP usage with net/http to reduce vuln surface and keep timeouts explicit.
cla-backend-go/token/token.go Replaces imroc/req token fetch with net/http + JSON marshal/unmarshal.
cla-backend-go/package.json Updates simple-git and resolution pins for vulnerable JS transitive deps used in Go backend tooling.
cla-backend-go/go.sum Syncs module checksums after dependency upgrades/removals.
cla-backend-go/go.mod Upgrades Go version/toolchain and several dependencies (gin, x/*, grpc, validator, etc.) and drops imroc/req.
cla-backend-go/cmd/server_standalone.go Buffers the signal channel to avoid potential missed signals.
.yarn-audit-allowlist.json Clears allowlist/notes so high vulns are no longer suppressed.
.gitignore Ignores .claude/* artifacts.
.github/workflows/security-scan-go.yml Makes govulncheck failures block the workflow (removes continue-on-error).
.github/workflows/go-audit.yml Removes Nancy step and makes govulncheck failures block the workflow.

Comment thread cla-backend-go/token/token.go Outdated
Comment thread cla-backend-go/package.json

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
cla-backend-go/go.mod (1)

5-7: Confirm Go 1.25 toolchain validity in cla-backend-go/go.mod

Go 1.25.0 is officially released, and toolchain go1.25.10 matches the latest released 1.25 patch version—no change needed for release/toolchain correctness. Still do a quick compatibility and vulnerability scan given the language/runtime bump.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cla-backend-go/go.mod` around lines 5 - 7, The go.mod declares "go 1.25.0"
and "toolchain go1.25.10" which is valid; confirm this by verifying the
local/build CI Go binary version (run `go version` in CI or locally) and ensure
the toolchain string matches the desired patch (toolchain identifier
"go1.25.10") in the file; then run a quick compatibility and vulnerability scan
(e.g., `go vet`, `go test`, and `govulncheck`) against the module to surface any
issues and update the toolchain line only if you need a different patch release.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cla-backend-go/package.json`:
- Line 29: The package.json still allows a vulnerable minimatch range (>=10.0.0,
<10.2.3); update the minimatch dependency entry to version ">=10.2.3" (or a
specific patched version like "10.2.3" or newer) in cla-backend-go/package.json
and then regenerate the lockfile (npm install / npm ci or yarn install) so
package-lock.json/Yarn lock is updated; if minimatch is transitive, run an audit
fix or add a resolution/overrides for "minimatch" => "10.2.3" and verify with
npm audit that the HIGH-severity advisory is resolved.

In `@cla-backend-go/token/token.go`:
- Around line 114-119: The log entry built in the non-2xx response branch is
never emitted: replace the call to log.WithFields(f).WithError(err) with an
actual logging call (e.g. log.WithFields(f).WithError(err).Error("auth0 token
request failed") or .Errorf) so the constructed err (from oauthTokenURL,
resp.StatusCode, string(respBody)) is written to logs; update the block that
sets err and currently returns it to emit the log before returning.

In `@cla-backend/package.json`:
- Line 64: Update the inconsistent dependency entry for "minimatch" so both
projects use the same version (prefer the higher version ^10.2.3); edit the
"minimatch" entry in the other package.json to match the chosen version and then
regenerate lockfiles (npm/yarn) to ensure consistency across the repo; target
the "minimatch" dependency lines in both package.json files and update
package-lock.json / yarn.lock accordingly.

---

Nitpick comments:
In `@cla-backend-go/go.mod`:
- Around line 5-7: The go.mod declares "go 1.25.0" and "toolchain go1.25.10"
which is valid; confirm this by verifying the local/build CI Go binary version
(run `go version` in CI or locally) and ensure the toolchain string matches the
desired patch (toolchain identifier "go1.25.10") in the file; then run a quick
compatibility and vulnerability scan (e.g., `go vet`, `go test`, and
`govulncheck`) against the module to surface any issues and update the toolchain
line only if you need a different patch release.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 871e59c1-fb01-45f3-b213-25d60fb88c5a

📥 Commits

Reviewing files that changed from the base of the PR and between 355223d and db5df6f.

⛔ Files ignored due to path filters (4)
  • cla-backend-go/go.sum is excluded by !**/*.sum
  • cla-backend-go/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
  • cla-backend-legacy/go.sum is excluded by !**/*.sum
  • cla-backend/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (11)
  • .github/workflows/go-audit.yml
  • .github/workflows/security-scan-go.yml
  • .gitignore
  • .yarn-audit-allowlist.json
  • cla-backend-go/cmd/server_standalone.go
  • cla-backend-go/go.mod
  • cla-backend-go/package.json
  • cla-backend-go/token/token.go
  • cla-backend-go/v2/metrics/repository.go
  • cla-backend-legacy/go.mod
  • cla-backend/package.json
💤 Files with no reviewable changes (2)
  • .github/workflows/go-audit.yml
  • .github/workflows/security-scan-go.yml

Comment thread cla-backend-go/package.json
Comment thread cla-backend-go/token/token.go
Comment thread cla-backend/package.json
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io>

Assisted by [OpenAI](https://platform.openai.com/)

Assisted by [GitHub Copilot](https://github.com/features/copilot)

Assisted by [Claude](https://claude.ai)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 15 changed files in this pull request and generated 3 comments.

Comment thread cla-backend-go/token/token.go
Comment thread .github/workflows/security-scan-go.yml
Comment thread .github/workflows/go-audit.yml
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io>

Assisted by [OpenAI](https://platform.openai.com/)

Assisted by [GitHub Copilot](https://github.com/features/copilot)

Assisted by [Claude](https://claude.ai)
@lukaszgryglicki

Copy link
Copy Markdown
Member Author

@code PTAL agin.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 15 changed files in this pull request and generated no new comments.

@lukaszgryglicki

Copy link
Copy Markdown
Member Author

@copilot review all 15 files, not just 10.

Copilot AI commented May 26, 2026

Copy link
Copy Markdown

@copilot review all 15 files, not just 10.

I reviewed all 15 changed files in PR #5066 (head ce5dc92) and found no additional substantive issues to address beyond what’s already resolved.

@lukaszgryglicki

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@lukaszgryglicki
lukaszgryglicki merged commit cdfef47 into dev May 26, 2026
18 checks passed
@lukaszgryglicki
lukaszgryglicki deleted the unicron-fix-vulns branch May 26, 2026 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants