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
2 changes: 1 addition & 1 deletion docs/plans/universal-harness-access-phase1.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ PRs 1, 2, 4, and 6 have no dependencies and can be developed/merged in parallel.
- For each declarative field (Agent, Policy, Skills):
- Local path: return as-is
- URL: extract/require integrity hash → validate against `AllowedRemoteResources` → check cache (with re-verification) → if miss and not offline: `fetch.FetchURL` → verify hash → `CachePut` → `AppendFetchAudit` → return cache content path
- Phase 1: single-level only (no transitive deps), security scanning deferred
- Single-level resolution; transitive deps added in Phase 2 (PR 2 of ADR-0038), security scanning deferred

**Create `internal/resolve/resolve_test.go`:**
- Tests using `httptest.NewTLSServer`: local pass-through, URL fetch+cache, cache hit, hash mismatch, URL not in allowlist, missing hash, offline+miss, offline+hit, security scan failure, mixed harness, audit entries
Expand Down
20 changes: 9 additions & 11 deletions docs/plans/universal-harness-access.md
Original file line number Diff line number Diff line change
Expand Up @@ -953,8 +953,7 @@ type ResolveOpts struct {

// ResolveHarness resolves URL-referenced declarative fields (Agent, Policy,
// Skills) in the harness to local cache paths. Local paths are left unchanged.
// The harness is modified in place.
// Phase 1: single-level resolution only (no transitive deps).
// The harness is modified in place. Transitive deps supported via MaxDepth.
func ResolveHarness(ctx context.Context, h *harness.Harness, opts ResolveOpts) ([]Dependency, error) {
var deps []Dependency

Expand All @@ -971,18 +970,17 @@ func ResolveHarness(ctx context.Context, h *harness.Harness, opts ResolveOpts) (
return deps, nil
}

// resolveResourceWithLimits resolves a single resource with depth and count limits.
// Phase 1: depth is always 0 (no transitive resolution), parentRef is unused
// Phase 2+: depth tracking prevents cycles and runaway recursion, parentRef enables relative path resolution
func resolveResourceWithLimits(ctx context.Context, workspaceRoot, ref string, allowedPrefixes []string, policy fetch.FetchPolicy, depth int, resourceCount *int, parentRef string) (string, error) {
// Phase 2+: Check depth limit (Phase 1 always passes since depth=0)
if depth > policy.MaxDepth {
return "", fmt.Errorf("exceeded maximum dependency depth of %d", policy.MaxDepth)
// Note: pseudocode below is illustrative. The implemented API uses resolveURL +
// resolveTransitiveDeps with explicit depth parameters and opts.MaxDepth/MaxResources.
// resolveResourceWithLimits was the design placeholder name; it was not shipped.
func resolveResourceWithLimits(ctx context.Context, workspaceRoot, ref string, allowedPrefixes []string, opts ResolveOpts, depth int, resourceCount *int, parentRef string) (string, error) {
if depth > opts.MaxDepth {
return "", fmt.Errorf("exceeded maximum dependency depth of %d", opts.MaxDepth)
}

// Check resource count limit (applies to all phases)
if *resourceCount >= policy.MaxResources {
return "", fmt.Errorf("exceeded maximum resource count of %d", policy.MaxResources)
if *resourceCount >= opts.MaxResources {
return "", fmt.Errorf("exceeded maximum resource count of %d", opts.MaxResources)
}

if harness.IsURL(ref) {
Expand Down
27 changes: 27 additions & 0 deletions internal/resolve/relurl.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package resolve

import (
"fmt"
"net/url"
)

// ResolveRelativeURL resolves a relative reference against a parent URL
// using RFC 3986 semantics. Absolute URLs are returned unchanged. The
// caller must validate the resolved URL against allowed prefixes.
func ResolveRelativeURL(parentURL, relRef string) (string, error) {
Comment thread
ggallen marked this conversation as resolved.
rel, err := url.Parse(relRef)
if err != nil {
return "", fmt.Errorf("parsing relative ref %q: %w", relRef, err)
}
if rel.IsAbs() {
Comment thread
ggallen marked this conversation as resolved.
return relRef, nil
}

parent, err := url.Parse(parentURL)
if err != nil {
return "", fmt.Errorf("parsing parent URL %q: %w", parentURL, err)
}

resolved := parent.ResolveReference(rel)
return resolved.String(), nil
}
103 changes: 103 additions & 0 deletions internal/resolve/relurl_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package resolve

import (
"testing"

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

func TestResolveRelativeURL(t *testing.T) {
tests := []struct {
name string
parentURL string
relRef string
want string
wantErr bool
}{
{
name: "sibling reference",
parentURL: "https://example.com/skills/rust/SKILL.md",
relRef: "../common/SKILL.md",
want: "https://example.com/skills/common/SKILL.md",
},
{
name: "child reference",
parentURL: "https://example.com/skills/rust/SKILL.md",
relRef: "policies/sandbox.yaml",
want: "https://example.com/skills/rust/policies/sandbox.yaml",
},
{
name: "absolute URL passthrough",
parentURL: "https://example.com/skills/rust/SKILL.md",
relRef: "https://other.com/skills/common/SKILL.md#sha256=abc",
want: "https://other.com/skills/common/SKILL.md#sha256=abc",
},
{
name: "path traversal resolves correctly",
parentURL: "https://github.com/org/skills/rust/SKILL.md",
relRef: "../../../../attacker/evil.md",
want: "https://github.com/attacker/evil.md",
},
{
name: "multiple parent segments",
parentURL: "https://example.com/a/b/c/d/SKILL.md",
relRef: "../../other/sub/SKILL.md",
want: "https://example.com/a/b/other/sub/SKILL.md",
},
{
name: "fragment preservation",
parentURL: "https://example.com/skills/rust/SKILL.md",
relRef: "../common/SKILL.md#sha256=abc123",
want: "https://example.com/skills/common/SKILL.md#sha256=abc123",
},
{
name: "bare fragment reference",
parentURL: "https://example.com/skills/rust/SKILL.md",
relRef: "#sha256=abc123",
want: "https://example.com/skills/rust/SKILL.md#sha256=abc123",
},
{
name: "invalid parent URL",
parentURL: "://bad-url",
relRef: "../sibling.md",
wantErr: true,
},
{
name: "invalid relRef percent-encoding",
parentURL: "https://example.com/skills/rust/SKILL.md",
relRef: "%xy/invalid.md",
wantErr: true,
},
{
name: "empty relRef resolves to parent URL",
parentURL: "https://example.com/skills/rust/SKILL.md",
relRef: "",
want: "https://example.com/skills/rust/SKILL.md",
},
{
name: "empty parentURL with relative ref",
parentURL: "",
relRef: "other/SKILL.md",
want: "/other/SKILL.md",
},
{
name: "parent URL with no path component",
parentURL: "https://example.com",
relRef: "../foo",
want: "https://example.com/foo",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := ResolveRelativeURL(tt.parentURL, tt.relRef)
if tt.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
assert.Equal(t, tt.want, got)
})
}
}
Loading
Loading