fix(github): escapePath returns error on dot-segments, fix Description semantics
PR Ready Gate / clear-labels (pull_request) Successful in 1s
CI / test (pull_request) Successful in 23s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 33s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m24s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m44s
PR Ready Gate / clear-labels (pull_request) Successful in 1s
CI / test (pull_request) Successful in 23s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 33s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m24s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m44s
- escapePath now returns an error when paths contain dot-segments
(".", "..") instead of silently rewriting them. This prevents
subtle API misses where callers pass "foo/../bar" expecting to
hit "bar" but the old code produced "foo/bar".
- Uses path.Clean for canonical form after validation.
- CommitStatus.Description for check runs is now empty string
instead of the raw conclusion enum. The conclusion is already
captured in the Status field via mapCheckRunStatus; storing it
again in Description was semantically inconsistent with commit
statuses where Description carries a human-readable narrative.
- Removed unused derefString helper.
- Added tests for escapePath valid paths, dot-segment rejection,
and GetFileContentAtRef dot-segment error propagation.
This commit is contained in:
@@ -0,0 +1,79 @@
|
||||
package github
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestEscapePath_ValidPaths(t *testing.T) {
|
||||
t.Parallel()
|
||||
tests := []struct {
|
||||
name string
|
||||
path string
|
||||
want string
|
||||
}{
|
||||
{"simple file", "file.go", "file.go"},
|
||||
{"nested path", "path/to/file.go", "path/to/file.go"},
|
||||
{"special chars", "path/to/my file.go", "path/to/my%20file.go"},
|
||||
{"leading slash stripped", "/path/to/file.go", "path/to/file.go"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
got, err := escapePath(tt.path)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if got != tt.want {
|
||||
t.Errorf("escapePath(%q) = %q, want %q", tt.path, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestEscapePath_DotSegments(t *testing.T) {
|
||||
t.Parallel()
|
||||
tests := []struct {
|
||||
name string
|
||||
path string
|
||||
}{
|
||||
{"single dot", "./file.go"},
|
||||
{"double dot", "../file.go"},
|
||||
{"dot in middle", "path/./file.go"},
|
||||
{"parent traversal", "path/../file.go"},
|
||||
{"only dots", ".."},
|
||||
{"nested parent traversal", "a/b/../../c"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, err := escapePath(tt.path)
|
||||
if err == nil {
|
||||
t.Fatalf("expected error for path %q, got nil", tt.path)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "dot-segment") {
|
||||
t.Errorf("expected error about dot-segment, got: %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetFileContentAtRef_DotSegmentError(t *testing.T) {
|
||||
// Server should never be called — the error is caught before the request.
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
t.Fatal("server should not have been called")
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
c := NewClient(srv.URL, "token")
|
||||
_, err := c.GetFileContentAtRef(context.Background(), "owner", "repo", "foo/../bar.go", "main")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for path with dot-segments")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "invalid file path") {
|
||||
t.Errorf("expected 'invalid file path' error, got: %v", err)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user