Skip to content

Potential fix for code scanning alert no. 8: Uncontrolled data used in path expression #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
VisualBean wants to merge 1 commit into from
Closed

Potential fix for code scanning alert no. 8: Uncontrolled data used in path expression #3

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 23 additions & 5 deletions internal/storage/filesystem.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
if err != nil {
return "", err
}
return fs.safeJoin("repositories", repoPath, "manifests", sanitizeReference(reference)), nil

Check failure on line 71 in internal/storage/filesystem.go

View workflow job for this annotation

GitHub Actions / Build, Test, OCI Conformance 

multiple-value sanitizeReference(reference) (value of type (string, error)) in single-value context
}

func (fs Filesystem) PutManifest(repository, reference, mediaType string, content []byte) (string, int64, error) {
Expand Down Expand Up @@ -130,7 +130,7 @@
}
digest := reference
if !strings.Contains(reference, ":") {
tagPath := fs.safeJoin("repositories", repoPath, "tags", sanitizeReference(reference))

Check failure on line 133 in internal/storage/filesystem.go

View workflow job for this annotation

GitHub Actions / Build, Test, OCI Conformance 

multiple-value sanitizeReference(reference) (value of type (string, error)) in single-value context
data, err := os.ReadFile(tagPath)
if err != nil {
if os.IsNotExist(err) {
Expand Down Expand Up @@ -168,7 +168,7 @@
}
digest := reference
if !strings.Contains(reference, ":") {
tagPath := fs.safeJoin("repositories", repoPath, "tags", sanitizeReference(reference))

Check failure on line 171 in internal/storage/filesystem.go

View workflow job for this annotation

GitHub Actions / Build, Test, OCI Conformance 

multiple-value sanitizeReference(reference) (value of type (string, error)) in single-value context
data, err := os.ReadFile(tagPath)
if err != nil {
if os.IsNotExist(err) {
Expand Down Expand Up @@ -512,7 +512,11 @@
}

func (fs Filesystem) linkManifestTag(repoPath, tag, digest string) error {
tagPath := fs.safeJoin("repositories", repoPath, "tags", sanitizeReference(tag))
safeTag, err := sanitizeReference(tag)
if err != nil {
return err
}
tagPath := fs.safeJoin("repositories", repoPath, "tags", safeTag)
if err := os.MkdirAll(filepath.Dir(tagPath), 0o750); err != nil {
return err
}
Expand Down Expand Up @@ -668,8 +672,22 @@
return repository, nil
}

func sanitizeReference(reference string) string {
reference = strings.ReplaceAll(reference, "/", "_")
reference = strings.ReplaceAll(reference, ":", "_")
return reference
func sanitizeReference(reference string) (string, error) {
reference = strings.TrimSpace(reference)
if reference == "" {
return "", fmt.Errorf("invalid reference")
}
if strings.Contains(reference, "/") || strings.Contains(reference, "\\") || strings.Contains(reference, "..") {
return "", fmt.Errorf("invalid reference")
}
for _, r := range reference {
if (r >= 'a' && r <= 'z') ||
(r >= 'A' && r <= 'Z') ||
(r >= '0' && r <= '9') ||
r == '.' || r == '-' || r == '_' {
continue
}
return "", fmt.Errorf("invalid reference")
}
return reference, nil
Comment on lines +675 to +692

@coderabbitai coderabbitai Bot Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Find all usages of sanitizeReference to ensure all call sites are updated.

rg -n -C 2 'sanitizeReference\(' internal/storage/

Repository: ByteBardOrg/simplecontainerregistry

Length of output: 1852

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '60,190p' internal/storage/filesystem.go | cat -n

Repository: ByteBardOrg/simplecontainerregistry

Length of output: 4758

Handle sanitizeReference's error at every call site

ManifestPath, GetManifest, and DeleteManifest still pass sanitizeReference(...) straight into safeJoin; each needs to unwrap the (string, error) result first or this file won't compile. The / and \ checks are redundant with the whitelist loop.

🤖 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 `@internal/storage/filesystem.go` around lines 675 - 692, Update every call to
sanitizeReference in ManifestPath, GetManifest, and DeleteManifest to handle its
(string, error) result before passing the sanitized value to safeJoin,
propagating or returning the validation error appropriately. Remove the
redundant slash and backslash checks from sanitizeReference, leaving the
whitelist validation to reject them.

}
Loading