Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
58 changes: 23 additions & 35 deletions docs/plans/universal-harness-access.md
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ func (h *Harness) Validate(orgAllowlist []string) error {

### 2. URL Detection and Classification

**File:** `internal/harness/url.go` (new)
**File:** `internal/harness/url.go`

```go
package harness
Expand All @@ -558,60 +558,44 @@ import (
"strings"
)

// IsURL returns true if s is a valid HTTPS URL.
// Only https:// URLs are accepted for remote resources. http:// URLs are rejected
// to avoid confusion and provide clear error messages.
// Rejects malformed URLs (empty host, userinfo, etc.)
func IsURL(s string) bool {
if s == "" {
return false
}
u, err := url.Parse(s)
if err != nil || u.Scheme != "https" {
return false
}
// Reject malformed URLs that url.Parse accepts but shouldn't be allowed:
// - Empty host (https:, https://, https:///path)
// - Userinfo (e.g., https://user:pass@host/ - credentials in URL)
// Note: url.Parse sets u.User for standard userinfo forms (https://user@host/), but may
// not catch all edge cases (e.g., https://@host/ on some Go versions). Production
// implementation should add strings.Contains(s, "@") check before hostname validation
// as belt-and-suspenders defense.
if u.Host == "" || u.User != nil {
return false
}
// Validate hostname is non-empty (u.Hostname() returns "" for malformed hosts)
if u.Hostname() == "" {
return false
}
if strings.Contains(u.Host, "@") {
return false
}
return true
}

// isAbsPath returns true if s is an absolute file path.
func isAbsPath(s string) bool {
func IsAbsPath(s string) bool {
return filepath.IsAbs(s)
}

// isRelPath returns true if s is a relative file path.
func isRelPath(s string) bool {
return !IsURL(s) && !isAbsPath(s)
func IsRelPath(s string) bool {
return s != "" && !IsURL(s) && !IsAbsPath(s)
}

// ParseIntegrityHash extracts the SHA256 hash from a URL fragment.
// Example: https://example.com/file.md#sha256=abc123... -> "abc123..."
// Returns an error if the hash is not a valid 64-character lowercase hex string.
func ParseIntegrityHash(rawURL string) (urlWithoutHash, hash string, hasHash bool) {
u, err := url.Parse(rawURL)
if err != nil {
func ParseIntegrityHash(rawURL string) (cleanURL, hash string, hasHash bool) {
idx := strings.LastIndex(rawURL, "#")
if idx == -1 {
return rawURL, "", false
}
if u.Fragment == "" {
fragment := rawURL[idx+1:]
if !strings.HasPrefix(fragment, "sha256=") {
return rawURL, "", false
}
if !strings.HasPrefix(u.Fragment, "sha256=") {
return rawURL, "", false
}
hash = strings.TrimPrefix(u.Fragment, "sha256=")

// Validate hash format: must be exactly 64 lowercase hex characters
// This prevents path traversal attacks like #sha256=../../etc/shadow
hash = strings.ToLower(strings.TrimPrefix(fragment, "sha256="))
if len(hash) != 64 {
return rawURL, "", false
}
Expand All @@ -620,12 +604,16 @@ func ParseIntegrityHash(rawURL string) (urlWithoutHash, hash string, hasHash boo
return rawURL, "", false
}
}

u.Fragment = ""
return u.String(), hash, true
return rawURL[:idx], hash, true
}
```

Key implementation details vs. original plan:
- `IsURL` guards against empty string and includes belt-and-suspenders `@` check on `u.Host`
- `IsRelPath` returns `false` for empty strings to prevent misclassifying missing values
- `ParseIntegrityHash` normalizes uppercase hex via `strings.ToLower` (SHA-256 hashes are commonly rendered with mixed case)
- `ParseIntegrityHash` uses `strings.LastIndex` for fragment extraction instead of `url.Parse` to correctly handle non-URL inputs (relative paths)

### 3. Resource Fetcher with SSRF Protection

**File:** `internal/fetch/fetch.go` (new)
Expand Down
63 changes: 63 additions & 0 deletions internal/harness/url.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package harness

import (
"net/url"
"path/filepath"
"strings"
)

// IsURL returns true if s is a valid HTTPS URL suitable for remote resource references.
func IsURL(s string) bool {
if s == "" {
return false
}
u, err := url.Parse(s)
if err != nil || u.Scheme != "https" {
return false
}
if u.Host == "" || u.User != nil {
return false
}
if u.Hostname() == "" {
return false
}
// Belt-and-suspenders: reject userinfo that url.Parse may not catch in all edge cases
if strings.Contains(u.Host, "@") {
return false
}
return true
}

// IsAbsPath returns true if s is an absolute file path.
func IsAbsPath(s string) bool {
return filepath.IsAbs(s)
}

// IsRelPath returns true if s is a non-empty relative file path (not a URL and not absolute).
func IsRelPath(s string) bool {
return s != "" && !IsURL(s) && !IsAbsPath(s)
}
Comment thread
ggallen marked this conversation as resolved.

// ParseIntegrityHash extracts the SHA256 hash from a URL fragment (#sha256=...).
// Returns the URL without the fragment, the hash value, and whether a valid hash was found.
// The hash is normalized to lowercase; both "sha256=ABC..." and "sha256=abc..." are accepted.
func ParseIntegrityHash(rawURL string) (cleanURL, hash string, hasHash bool) {
Comment thread
ggallen marked this conversation as resolved.
idx := strings.LastIndex(rawURL, "#")
if idx == -1 {
return rawURL, "", false
}
fragment := rawURL[idx+1:]
if !strings.HasPrefix(fragment, "sha256=") {
return rawURL, "", false
}
hash = strings.ToLower(strings.TrimPrefix(fragment, "sha256="))
if len(hash) != 64 {
return rawURL, "", false
}
for _, c := range hash {
if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f')) {
return rawURL, "", false
}
}
return rawURL[:idx], hash, true
}
176 changes: 176 additions & 0 deletions internal/harness/url_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
package harness

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestIsURL(t *testing.T) {
tests := []struct {
name string
input string
want bool
}{
{"valid https", "https://example.com/path/file.md", true},
{"valid https with port", "https://example.com:8443/path", true},
{"valid https with query", "https://example.com/path?q=1", true},
{"valid https with fragment", "https://example.com/path#sha256=abc", true},
{"http rejected", "http://example.com/path", false},
{"file scheme rejected", "file:///etc/passwd", false},
{"ftp rejected", "ftp://example.com/file", false},
{"empty string", "", false},
{"relative path", "agents/code.md", false},
{"relative path with dots", "../agents/code.md", false},
{"absolute path", "/opt/agents/code.md", false},
{"empty host", "https:///path", false},
{"scheme only", "https://", false},
{"userinfo", "https://user:pass@example.com/path", false},
{"userinfo user only", "https://user@example.com/path", false},
{"plain text", "not a url at all", false},
{"just a word", "https", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, IsURL(tt.input))
})
}
}

func TestIsAbsPath(t *testing.T) {
tests := []struct {
name string
input string
want bool
}{
{"absolute unix", "/opt/agents/code.md", true},
{"relative", "agents/code.md", false},
{"relative with dots", "../agents/code.md", false},
{"url", "https://example.com/path", false},
{"empty", "", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, IsAbsPath(tt.input))
})
}
}

func TestIsRelPath(t *testing.T) {
tests := []struct {
name string
input string
want bool
}{
{"relative", "agents/code.md", true},
{"relative with dots", "../agents/code.md", true},
{"dot slash", "./agents/code.md", true},
{"bare filename", "code.md", true},
{"empty string", "", false},
{"absolute path", "/opt/agents/code.md", false},
{"url", "https://example.com/path", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, IsRelPath(tt.input))
})
}
}

func TestParseIntegrityHash(t *testing.T) {
validHash := "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789"

tests := []struct {
name string
input string
wantURL string
wantHash string
wantHasHash bool
}{
{
name: "valid hash",
input: "https://example.com/file.md#sha256=" + validHash,
wantURL: "https://example.com/file.md",
wantHash: validHash,
wantHasHash: true,
},
{
name: "valid hash with query params",
input: "https://example.com/file.md?v=1#sha256=" + validHash,
wantURL: "https://example.com/file.md?v=1",
wantHash: validHash,
wantHasHash: true,
},
{
name: "no fragment",
input: "https://example.com/file.md",
wantURL: "https://example.com/file.md",
wantHash: "",
wantHasHash: false,
},
{
name: "non-sha256 fragment",
input: "https://example.com/file.md#section1",
wantURL: "https://example.com/file.md#section1",
wantHash: "",
wantHasHash: false,
},
{
name: "wrong prefix",
input: "https://example.com/file.md#md5=abc123",
wantURL: "https://example.com/file.md#md5=abc123",
wantHash: "",
wantHasHash: false,
},
{
name: "hash too short 63 chars",
input: "https://example.com/file.md#sha256=" + validHash[:63],
wantURL: "https://example.com/file.md#sha256=" + validHash[:63],
wantHash: "",
wantHasHash: false,
},
{
name: "hash too long 65 chars",
input: "https://example.com/file.md#sha256=" + validHash + "a",
wantURL: "https://example.com/file.md#sha256=" + validHash + "a",
wantHash: "",
wantHasHash: false,
},
{
name: "uppercase hex normalized",
input: "https://example.com/file.md#sha256=ABCDEF0123456789abcdef0123456789abcdef0123456789abcdef0123456789",
wantURL: "https://example.com/file.md",
wantHash: "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789",
wantHasHash: true,
},
{
name: "empty hash value",
input: "https://example.com/file.md#sha256=",
wantURL: "https://example.com/file.md#sha256=",
wantHash: "",
wantHasHash: false,
},
{
name: "path traversal in hash rejected",
input: "https://example.com/file.md#sha256=../../../../../../etc/shadow//////////////////////////////////",
wantURL: "https://example.com/file.md#sha256=../../../../../../etc/shadow//////////////////////////////////",
wantHash: "",
wantHasHash: false,
},
{
name: "relative path unchanged",
input: "agents/code.md",
wantURL: "agents/code.md",
wantHash: "",
wantHasHash: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotURL, gotHash, gotHasHash := ParseIntegrityHash(tt.input)
assert.Equal(t, tt.wantURL, gotURL)
assert.Equal(t, tt.wantHash, gotHash)
assert.Equal(t, tt.wantHasHash, gotHasHash)
})
}
}
Loading