From 224c3cb28565f8ab30ccbfae71344ad73a9c4812 Mon Sep 17 00:00:00 2001 From: Sungkyu Yoo Date: Sun, 19 Apr 2026 22:33:17 +0900 Subject: [PATCH 1/2] Potential fix for code scanning alert no. 2: Uncontrolled data used in path expression Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- internal/services/ecr/store.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/services/ecr/store.go b/internal/services/ecr/store.go index bffca9e..8b0ea64 100644 --- a/internal/services/ecr/store.go +++ b/internal/services/ecr/store.go @@ -12,6 +12,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "time" "github.com/skyoo2003/devcloud/internal/storage/sqlite" @@ -24,6 +25,8 @@ var ( ErrPolicyNotFound = errors.New("policy not found") ErrLifecyclePolicyNotFound = errors.New("lifecycle policy not found") ErrLayerUploadNotFound = errors.New("layer upload not found") + + uploadIDPattern = regexp.MustCompile(`^[a-f0-9]{32}$`) ) const region = "us-east-1" @@ -387,6 +390,12 @@ func (s *ECRStore) InitiateLayerUpload(accountID, repoName string) (string, erro // UploadLayerPart saves layer part blob to the filesystem and records the part metadata. func (s *ECRStore) UploadLayerPart(accountID, repoName, uploadID string, partFirst, partLast int64, blob []byte) error { + // uploadID is expected to be a 32-char lowercase hex value generated by InitiateLayerUpload. + // Reject anything else to prevent path traversal/path injection. + if !uploadIDPattern.MatchString(uploadID) { + return ErrLayerUploadNotFound + } + // Verify upload exists. var exists int _ = s.db().QueryRow(`SELECT COUNT(*) FROM layers WHERE upload_id=? AND repo_name=? AND account_id=?`, uploadID, repoName, accountID).Scan(&exists) From ea940ad6b8f4619bfc10aad0eb132bcfe0b9027c Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Sun, 19 Apr 2026 23:27:24 +0900 Subject: [PATCH 2/2] refactor: centralize upload ID validation, replace regex with manual check - Add shared.ValidateUploadID: simple length + hex check (no regex overhead) - Replace duplicate uploadIDPattern regexps in s3 and ecr with shared validator - Remove regexp import from both files --- internal/services/ecr/store.go | 6 ++---- internal/services/s3/provider.go | 13 ++++--------- internal/shared/validate.go | 21 +++++++++++++++++++++ 3 files changed, 27 insertions(+), 13 deletions(-) create mode 100644 internal/shared/validate.go diff --git a/internal/services/ecr/store.go b/internal/services/ecr/store.go index 8b0ea64..44e9237 100644 --- a/internal/services/ecr/store.go +++ b/internal/services/ecr/store.go @@ -12,9 +12,9 @@ import ( "fmt" "os" "path/filepath" - "regexp" "time" + "github.com/skyoo2003/devcloud/internal/shared" "github.com/skyoo2003/devcloud/internal/storage/sqlite" ) @@ -25,8 +25,6 @@ var ( ErrPolicyNotFound = errors.New("policy not found") ErrLifecyclePolicyNotFound = errors.New("lifecycle policy not found") ErrLayerUploadNotFound = errors.New("layer upload not found") - - uploadIDPattern = regexp.MustCompile(`^[a-f0-9]{32}$`) ) const region = "us-east-1" @@ -392,7 +390,7 @@ func (s *ECRStore) InitiateLayerUpload(accountID, repoName string) (string, erro func (s *ECRStore) UploadLayerPart(accountID, repoName, uploadID string, partFirst, partLast int64, blob []byte) error { // uploadID is expected to be a 32-char lowercase hex value generated by InitiateLayerUpload. // Reject anything else to prevent path traversal/path injection. - if !uploadIDPattern.MatchString(uploadID) { + if !shared.ValidateUploadID(uploadID) { return ErrLayerUploadNotFound } diff --git a/internal/services/s3/provider.go b/internal/services/s3/provider.go index ba44a90..951eaa1 100644 --- a/internal/services/s3/provider.go +++ b/internal/services/s3/provider.go @@ -16,7 +16,8 @@ import ( "net/url" "os" "path/filepath" - "regexp" + + "github.com/skyoo2003/devcloud/internal/shared" "sort" "strconv" "strings" @@ -461,12 +462,6 @@ func generateUploadID() (string, error) { return hex.EncodeToString(b), nil } -var uploadIDPattern = regexp.MustCompile(`^[a-f0-9]{32}$`) - -func isValidUploadID(uploadID string) bool { - return uploadIDPattern.MatchString(uploadID) -} - // multipartDir returns the directory used to store parts for an upload. func (p *S3Provider) multipartDir(uploadID string) string { return filepath.Join(p.fileStore.baseDir, "_multipart", filepath.Base(uploadID)) @@ -891,7 +886,7 @@ func (p *S3Provider) uploadPart(_ context.Context, bucket, key, uploadID, partNu } func (p *S3Provider) completeMultipartUpload(_ context.Context, bucket, key, uploadID string, req *http.Request) (*plugin.Response, error) { - if !isValidUploadID(uploadID) { + if !shared.ValidateUploadID(uploadID) { return xmlError("NoSuchUpload", "upload not found", http.StatusNotFound), nil } @@ -971,7 +966,7 @@ func (p *S3Provider) completeMultipartUpload(_ context.Context, bucket, key, upl } func (p *S3Provider) abortMultipartUpload(_ context.Context, bucket, key, uploadID string) (*plugin.Response, error) { - if !isValidUploadID(uploadID) { + if !shared.ValidateUploadID(uploadID) { return xmlError("InvalidRequest", "invalid uploadId", http.StatusBadRequest), nil } if _, err := p.metaStore.GetMultipartUpload(uploadID); err != nil { diff --git a/internal/shared/validate.go b/internal/shared/validate.go new file mode 100644 index 0000000..9324d62 --- /dev/null +++ b/internal/shared/validate.go @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: Apache-2.0 + +package shared + +// ValidateUploadID checks that id is a 32-character lowercase hex string, +// matching the format produced by InitiateLayerUpload in S3 and ECR. +func ValidateUploadID(id string) bool { + if len(id) != 32 { + return false + } + for _, c := range id { + if !isHexLower(c) { + return false + } + } + return true +} + +func isHexLower(c rune) bool { + return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') +}