From dea58b85b8de3b11adf1c08ec313051f88145b68 Mon Sep 17 00:00:00 2001 From: erangel1 Date: Wed, 13 May 2026 01:08:19 +0200 Subject: [PATCH] fixed issues from opencode agent --- internal/api/handlers/oci.go | 6 +++++- internal/api/router.go | 4 ++-- internal/domain/oci/registry.go | 9 +++++---- internal/domain/sbom/generator.go | 10 ++++++++-- internal/domain/scanning/scanner.go | 11 +++++++---- internal/domain/vulnscan/scanner.go | 3 ++- 6 files changed, 29 insertions(+), 14 deletions(-) diff --git a/internal/api/handlers/oci.go b/internal/api/handlers/oci.go index 563e1e7..ee23293 100644 --- a/internal/api/handlers/oci.go +++ b/internal/api/handlers/oci.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "golang.org/x/crypto/bcrypt" "xorm.io/xorm" "github.com/forgeo/forgebucket/internal/domain/oci" @@ -513,7 +514,7 @@ func newOCIUploadID() string { } func (h *OCIRegistryHandler) basicAuthOCI(r *http.Request) string { - u, _, hasAuth := r.BasicAuth() + u, pass, hasAuth := r.BasicAuth() if !hasAuth { return "" } @@ -521,5 +522,8 @@ func (h *OCIRegistryHandler) basicAuthOCI(r *http.Request) string { if found, _ := h.db.Where("username = ?", u).Get(&user); !found { return "" } + if err := bcrypt.CompareHashAndPassword([]byte(user.PasswordHash), []byte(pass)); err != nil { + return "" + } return u } diff --git a/internal/api/router.go b/internal/api/router.go index b3227ce..4b0d846 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -211,8 +211,8 @@ func New(cfg *config.Config, engine *xorm.Engine, store sessions.Store, bus even }) }) r.Get("/artifacts/{artifactID}/download", artifactH.Download) - r.Get("/artifacts/{artifactID}/signature", artifactH.GetSignature) - r.Get("/artifacts/{artifactID}/verify", artifactH.VerifySignature) + r.Get("/artifacts/{artifactID}/signature", artifactH.GetSignature) + r.Get("/artifacts/{artifactID}/verify", artifactH.VerifySignature) r.Route("/members", func(r chi.Router) { r.Get("/", memberH.List) r.With(csrf).Post("/", memberH.Add) diff --git a/internal/domain/oci/registry.go b/internal/domain/oci/registry.go index 257785b..d48bbe7 100644 --- a/internal/domain/oci/registry.go +++ b/internal/domain/oci/registry.go @@ -27,7 +27,7 @@ type Registry struct { // New creates a Registry rooted at ociRoot, creating the directory tree if needed. func New(ociRoot string) (*Registry, error) { for _, sub := range []string{"blobs/sha256", "uploads"} { - if err := os.MkdirAll(filepath.Join(ociRoot, sub), 0755); err != nil { + if err := os.MkdirAll(filepath.Join(ociRoot, sub), 0700); err != nil { return nil, fmt.Errorf("oci: init storage %s: %w", sub, err) } } @@ -174,7 +174,7 @@ func (r *Registry) FinishUpload(uploadID, clientDigest string) (digest string, s // new total offset. func (r *Registry) AppendUpload(uploadID string, src io.Reader) (newOffset int64, err error) { path := r.UploadPath(uploadID) - f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) + f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600) if err != nil { return 0, fmt.Errorf("oci: open upload for append: %w", err) } @@ -291,9 +291,10 @@ func digestHex(digest string) (string, error) { return h, nil } -// sanitiseID strips any path separators from an upload ID. +// sanitiseID returns only the last path component of an upload ID, +// preventing any path traversal regardless of encoding. func sanitiseID(id string) string { - return strings.NewReplacer("/", "", "\\", "", "..", "").Replace(id) + return filepath.Base(id) } // ParseOCIPath extracts the image name and the operation kind from a path diff --git a/internal/domain/sbom/generator.go b/internal/domain/sbom/generator.go index 9204237..ce9351a 100644 --- a/internal/domain/sbom/generator.go +++ b/internal/domain/sbom/generator.go @@ -65,11 +65,17 @@ func (g *Generator) Start(ctx context.Context) { // generateForRun generates an SBOM for the pipeline run identified by runID. func (g *Generator) generateForRun(runID, repoID int64) { var run models.PipelineRun - if found, _ := g.db.ID(runID).Get(&run); !found { + if found, err := g.db.ID(runID).Get(&run); err != nil { + log.Printf("sbom: look up run %d: %v", runID, err) + return + } else if !found { return } var repo models.Repository - if found, _ := g.db.ID(repoID).Get(&repo); !found { + if found, err := g.db.ID(repoID).Get(&repo); err != nil { + log.Printf("sbom: look up repo %d: %v", repoID, err) + return + } else if !found { return } diff --git a/internal/domain/scanning/scanner.go b/internal/domain/scanning/scanner.go index d66665f..ef6f84e 100644 --- a/internal/domain/scanning/scanner.go +++ b/internal/domain/scanning/scanner.go @@ -69,7 +69,10 @@ func (s *Scanner) scanPush(evt events.PushEvent) { // Resolve repo. var repo models.Repository - if found, _ := s.db.ID(evt.RepoID).Get(&repo); !found { + if found, err := s.db.ID(evt.RepoID).Get(&repo); err != nil { + log.Printf("scanning: look up repo %d: %v", evt.RepoID, err) + return + } else if !found { return } @@ -112,11 +115,11 @@ func (s *Scanner) scanPush(evt events.PushEvent) { // getDiff returns the unified diff of all changes between two refs. func (s *Scanner) getDiff(repoPath, oldRef, newRef string) ([]byte, error) { - // If oldRef is the zero OID (new branch), just get the initial commit content. + // If oldRef is the zero OID (new branch), diff-tree against the empty tree so + // we get actual file contents rather than ls-tree metadata. zeroOID := "0000000000000000000000000000000000000000" if oldRef == zeroOID { - // Show the entire tree at the new ref. - out, err := gitdomain.Run(repoPath, "ls-tree", "-r", newRef) + out, err := gitdomain.Run(repoPath, "diff-tree", "--no-commit-id", "-r", "-p", newRef) if err != nil { return nil, err } diff --git a/internal/domain/vulnscan/scanner.go b/internal/domain/vulnscan/scanner.go index 057c160..521bf83 100644 --- a/internal/domain/vulnscan/scanner.go +++ b/internal/domain/vulnscan/scanner.go @@ -115,7 +115,8 @@ func (s *Scanner) DismissFindings(findingID int64, dismissedBy string) error { return nil } -func (s *Scanner) persistFindings(repoID int64, purl, version string, vulns []OSVVuln) []models.VulnerabilityFinding {var findings []models.VulnerabilityFinding +func (s *Scanner) persistFindings(repoID int64, purl, version string, vulns []OSVVuln) []models.VulnerabilityFinding { + var findings []models.VulnerabilityFinding for _, v := range vulns { // Check for duplicate before inserting. existing := &models.VulnerabilityFinding{}