diff --git a/docs/ADRs/0070-portable-provider-profile-resolution.md b/docs/ADRs/0070-portable-provider-profile-resolution.md index 9e9b6380e..1851cfa6f 100644 --- a/docs/ADRs/0070-portable-provider-profile-resolution.md +++ b/docs/ADRs/0070-portable-provider-profile-resolution.md @@ -56,26 +56,30 @@ profiles. Today, providers and profiles are an exception. Extend the harness schema with two new fields: -1. **`openshell.profiles`** — A new list field accepting only HTTPS URLs with `#sha256=...` - integrity hashes. Profiles define openshell-level credential type schemas and - belong in shared repositories, not locally. No local-path form. - -2. **`providers`** — Extend the existing list field to accept both local provider - names (existing behavior) and remote HTTPS URLs with `#sha256=...` hashes. +1. **`openshell.profiles`** — A new list field accepting HTTPS URLs with `#sha256=...` + integrity hashes, and local file paths (resolved relative to the harness + directory or inherited as absolute cache paths from `base:` composition). + Profiles define openshell-level credential type schemas. + +2. **`providers`** — Extend the existing list field to accept local provider + names (existing behavior), local file paths (resolved relative to the + harness directory), and remote HTTPS URLs with `#sha256=...` hashes. Mixed forms allowed in the same list. ### Schema ```yaml -# New openshell.profiles field (URL-only) +# openshell.profiles field (local paths or URLs) openshell: profiles: + - profiles/claude-code.yaml # Local path (resolved relative to harness) - "https://github.com/org/profiles/tree/main/claude-code.yaml#sha256=abc..." - "https://github.com/org/profiles/tree/main/google-vertex-ai.yaml#sha256=def..." -# Extended providers field (mixed local names and URLs) +# Extended providers field (mixed local names, paths, and URLs) providers: - - "my-local-provider" # Local name (existing) + - "my-local-provider" # Local name: loaded from providers/my-local-provider.yaml + - providers/custom.yaml # Local path (resolved relative to harness) - "https://github.com/org/repo/tree/main/providers/my-provider.yaml#sha256=789..." # Remote ``` diff --git a/docs/guides/user/customizing-agents.md b/docs/guides/user/customizing-agents.md index bc0a4a883..ba8a0c2e5 100644 --- a/docs/guides/user/customizing-agents.md +++ b/docs/guides/user/customizing-agents.md @@ -59,8 +59,9 @@ providers: # Inference providers (local names or URLs) - vertex # Local name: references providers/vertex.yaml - "https://github.com/org/repo/tree/main/providers/claude.yaml#sha256=abc..." # Remote URL -openshell: # Openshell provider profiles (URL-only, with integrity hash) +openshell: # Openshell provider profiles (local paths or URLs) profiles: + - profiles/claude-code.yaml # Local path: resolved relative to harness - "https://github.com/org/profiles/tree/main/claude-code.yaml#sha256=def..." validation_loop: # script is required; these sub-fields are optional @@ -116,11 +117,12 @@ providers: - "https://github.com/org/repo/tree/main/providers/claude.yaml#sha256=abc..." # Remote ``` -**`openshell.profiles`** accepts only HTTPS URLs (profiles are always remote): +**`openshell.profiles`** accepts local paths and HTTPS URLs: ```yaml openshell: profiles: + - profiles/claude-code.yaml # Local path (resolved relative to harness) - "https://github.com/org/profiles/tree/main/claude-code.yaml#sha256=abc..." ``` diff --git a/internal/cli/lock.go b/internal/cli/lock.go index 1d58fc488..62352ff7e 100644 --- a/internal/cli/lock.go +++ b/internal/cli/lock.go @@ -779,17 +779,29 @@ func resolveFromLock(h *harness.Harness, entry *lock.HarnessLock, workspaceRoot } h.Skills = filtered - // Strip URL entries from providers — URL-resolved providers are now in - // the ResolvedProvider list, mirroring resolve.ResolveHarness behavior. + // Strip URL and absolute-path entries from providers — URL-resolved + // providers are in the ResolvedProvider list, and absolute-path entries + // (from base composition cache) will be resolved by the local-only + // ResolveHarness pass. Keep only bare provider names. remainingProviders := h.Providers[:0] for _, p := range h.Providers { - if !harness.IsURL(p) { + if !harness.IsURL(p) && !harness.IsProviderPath(p) { remainingProviders = append(remainingProviders, p) } } h.Providers = remainingProviders + // Strip URL and absolute-path profiles — URL-resolved profiles are in + // the ResolvedProfile list from lock deps, and absolute-path entries + // (from base composition cache) are reconstructed from lock deps too. + // Keep only relative-path profiles for the local-only ResolveHarness pass. if h.OpenShell != nil { - h.OpenShell.Profiles = nil + remaining := h.OpenShell.Profiles[:0] + for _, p := range h.OpenShell.Profiles { + if !harness.IsURL(p) && !filepath.IsAbs(p) { + remaining = append(remaining, p) + } + } + h.OpenShell.Profiles = remaining } return resolve.ResolveResult{ diff --git a/internal/cli/run.go b/internal/cli/run.go index 169e8e283..3a55681dd 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -478,6 +478,29 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep } } + // When profiles or providers use local paths (from ResolveRelativeTo or + // base composition), ResolveHarness must still run to parse them into + // ResolvedProfile/ResolvedProvider — even without URL references. + // The lock-file and URL-resolution paths strip entries they consume; + // this pass handles whatever remains. Outputs are merged and deduped. + if len(h.OpenShellProfiles()) > 0 || hasLocalProviders(h) { + prev := result + var resolveErr error + result, resolveErr = resolve.ResolveHarness(ctx, h, resolve.ResolveOpts{ + WorkspaceRoot: absFullsendDir, + }) + if resolveErr != nil { + return fmt.Errorf("resolving local profiles/providers: %w", resolveErr) + } + result.Deps = append(prev.Deps, result.Deps...) + result.Profiles = append(prev.Profiles, result.Profiles...) + result.Providers = append(prev.Providers, result.Providers...) + result.Warnings = append(prev.Warnings, result.Warnings...) + } + for _, w := range result.Warnings { + printer.StepWarn(w) + } + if resolved, overridden := applySandboxImageOverride(h.Image); overridden { printer.StepInfo(fmt.Sprintf("Image override via FULLSEND_SANDBOX_IMAGE: %s -> %s", h.Image, resolved)) h.Image = resolved @@ -3174,6 +3197,15 @@ func mergeProviderDefs(localDefs []harness.ProviderDef, urlProviders []resolve.R return allDefs, shadowed } +func hasLocalProviders(h *harness.Harness) bool { + for _, p := range h.Providers { + if !harness.IsURL(p) && harness.IsProviderPath(p) { + return true + } + } + return false +} + // sandboxProviderNames returns the provider names that should be attached to // the sandbox: harness-declared (local) names plus URL-resolved names. // Directory providers not declared in the harness are excluded — they may diff --git a/internal/harness/compose.go b/internal/harness/compose.go index 9270f5e06..0b551aca9 100644 --- a/internal/harness/compose.go +++ b/internal/harness/compose.go @@ -127,6 +127,18 @@ func LoadWithBase(ctx context.Context, path string, opts ComposeOpts) (*Harness, return nil, nil, fmt.Errorf("resolving URL-sourced host_files: %w", err) } deps = append(deps, hostFileDeps...) + + profileDeps, err := resolveBaseProfiles(ctx, child, opts.SourceURL, opts.OrgAllowlist, opts) + if err != nil { + return nil, nil, fmt.Errorf("resolving URL-sourced profiles: %w", err) + } + deps = append(deps, profileDeps...) + + providerDeps, err := resolveBaseProviders(ctx, child, opts.SourceURL, opts.OrgAllowlist, opts) + if err != nil { + return nil, nil, fmt.Errorf("resolving URL-sourced providers: %w", err) + } + deps = append(deps, providerDeps...) } if err := child.validateForge(); err != nil { @@ -260,6 +272,18 @@ func loadBaseChain( } deps = append(deps, hostFileDeps...) + profileDeps, err := resolveBaseProfiles(ctx, base, baseRef, allowlist, opts) + if err != nil { + return nil, nil, fmt.Errorf("resolving base profiles from %s: %w", cleanURL, err) + } + deps = append(deps, profileDeps...) + + providerDeps, err := resolveBaseProviders(ctx, base, baseRef, allowlist, opts) + if err != nil { + return nil, nil, fmt.Errorf("resolving base providers from %s: %w", cleanURL, err) + } + deps = append(deps, providerDeps...) + baseDir = childDir } else { // Local path base @@ -774,6 +798,82 @@ func resolveBaseHostFiles(ctx context.Context, base *Harness, baseURL string, al return deps, nil } +// resolveBaseProfiles fetches profile files with relative paths from a +// URL-referenced base harness. For each openshell.profiles entry that is a +// non-empty relative path (not a URL or absolute path), the file is fetched +// from the base URL's directory, cached content-addressed, and the entry is +// rewritten to the local cache path. This matches the resolution behavior +// for agent, policy, skills, scripts, and host_files in base composition. +func resolveBaseProfiles(ctx context.Context, base *Harness, baseURL string, allowlist []string, opts ComposeOpts) ([]Dependency, error) { + if base.OpenShell == nil || len(base.OpenShell.Profiles) == 0 { + return nil, nil + } + + baseURLDir := urlParentDirPrefix(baseURL) + if baseURLDir == "" { + return nil, fmt.Errorf("cannot determine directory from base URL") + } + + var deps []Dependency + + for i, p := range base.OpenShell.Profiles { + if p == "" || IsURL(p) || filepath.IsAbs(p) { + continue + } + fieldName := fmt.Sprintf("openshell.profiles[%d]", i) + if err := validateBaseRelPath(fieldName, p); err != nil { + return nil, err + } + dep, cachePath, err := fetchBaseFile(ctx, fieldName, baseURLDir, p, allowlist, opts, "resource", false) + if err != nil { + return nil, err + } + base.OpenShell.Profiles[i] = cachePath + deps = append(deps, dep) + } + + return deps, nil +} + +// resolveBaseProviders fetches provider files with relative paths from a +// URL-referenced base harness. For each providers entry that is a non-empty +// relative path (not a URL, absolute path, or bare provider name), the file +// is fetched from the base URL's directory, cached content-addressed, and +// the entry is rewritten to the local cache path. +func resolveBaseProviders(ctx context.Context, base *Harness, baseURL string, allowlist []string, opts ComposeOpts) ([]Dependency, error) { + if len(base.Providers) == 0 { + return nil, nil + } + + baseURLDir := urlParentDirPrefix(baseURL) + if baseURLDir == "" { + return nil, fmt.Errorf("cannot determine directory from base URL") + } + + var deps []Dependency + + for i, p := range base.Providers { + if p == "" || IsURL(p) || filepath.IsAbs(p) { + continue + } + if !IsProviderPath(p) { + continue + } + fieldName := fmt.Sprintf("providers[%d]", i) + if err := validateBaseRelPath(fieldName, p); err != nil { + return nil, err + } + dep, cachePath, err := fetchBaseFile(ctx, fieldName, baseURLDir, p, allowlist, opts, "resource", false) + if err != nil { + return nil, err + } + base.Providers[i] = cachePath + deps = append(deps, dep) + } + + return deps, nil +} + // validateBaseRelPath validates that a relative path inherited from a URL base // is safe to resolve. Rejects null bytes, query/fragment markers, URLs, // absolute paths, and path traversal segments. diff --git a/internal/harness/compose_test.go b/internal/harness/compose_test.go index b3556d66e..939b3a594 100644 --- a/internal/harness/compose_test.go +++ b/internal/harness/compose_test.go @@ -3950,3 +3950,233 @@ func TestIsTransientFetchError(t *testing.T) { }) } } + +func TestLoadWithBase_URLBase_ProfileResolution(t *testing.T) { + profileContent := []byte(`id: test-profile +network: + egress: + - host: "*.example.com" +`) + profileHash := computeHash(profileContent) + + baseContent := []byte(` +agent: agents/remote.md +role: test +openshell: + profiles: + - profiles/test-profile.yaml +`) + baseHash := computeHash(baseContent) + + agentContent := []byte("# test agent") + + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/base.yaml": + w.Write(baseContent) + case "/profiles/test-profile.yaml": + w.Write(profileContent) + case "/agents/remote.md": + w.Write(agentContent) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + dir := t.TempDir() + cacheDir := filepath.Join(dir, "cache") + + path := writeTestHarness(t, dir, "child.yaml", ` +agent: agents/child.md +role: test +base: `+server.URL+`/base.yaml#sha256=`+baseHash+` +`) + + policy := fetch.NewTestPolicy( + server.Client().Transport.(*http.Transport).TLSClientConfig, + []string{"127.0.0.1"}, + []string{server.Listener.Addr().String()[len("127.0.0.1:"):]}, + ) + + h, deps, err := LoadWithBase(context.Background(), path, ComposeOpts{ + WorkspaceRoot: cacheDir, + FetchPolicy: policy, + OrgAllowlist: []string{server.URL + "/"}, + }) + require.NoError(t, err) + + // Profile should be rewritten to a local cache path + require.NotNil(t, h.OpenShell) + require.Len(t, h.OpenShellProfiles(), 1) + assert.False(t, IsURL(h.OpenShellProfiles()[0]), "profile should be a local path, not a URL") + assert.True(t, filepath.IsAbs(h.OpenShellProfiles()[0]), "profile should be absolute (cache path)") + + // Verify cached content matches + content, err := os.ReadFile(h.OpenShellProfiles()[0]) + require.NoError(t, err) + assert.Equal(t, profileContent, content) + + // Dependencies: base + agent + profile = 3 + hasDep := false + for _, d := range deps { + if d.Field == "openshell.profiles[0]" { + hasDep = true + assert.Equal(t, server.URL+"/profiles/test-profile.yaml", d.URL) + assert.Equal(t, profileHash, d.SHA256) + } + } + assert.True(t, hasDep, "should have a dependency for the profile") +} + +func TestLoadWithBase_URLBase_ProviderResolution(t *testing.T) { + providerContent := []byte(`name: test-provider +type: custom +credentials: + TEST_KEY: "" +`) + providerHash := computeHash(providerContent) + + baseContent := []byte(` +agent: agents/remote.md +role: test +providers: + - providers/test-provider.yaml +`) + baseHash := computeHash(baseContent) + + agentContent := []byte("# test agent") + + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/base.yaml": + w.Write(baseContent) + case "/providers/test-provider.yaml": + w.Write(providerContent) + case "/agents/remote.md": + w.Write(agentContent) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + dir := t.TempDir() + cacheDir := filepath.Join(dir, "cache") + + path := writeTestHarness(t, dir, "child.yaml", ` +agent: agents/child.md +role: test +base: `+server.URL+`/base.yaml#sha256=`+baseHash+` +`) + + policy := fetch.NewTestPolicy( + server.Client().Transport.(*http.Transport).TLSClientConfig, + []string{"127.0.0.1"}, + []string{server.Listener.Addr().String()[len("127.0.0.1:"):]}, + ) + + h, deps, err := LoadWithBase(context.Background(), path, ComposeOpts{ + WorkspaceRoot: cacheDir, + FetchPolicy: policy, + OrgAllowlist: []string{server.URL + "/"}, + }) + require.NoError(t, err) + + // Provider should be rewritten to a local cache path + require.Len(t, h.Providers, 1) + assert.False(t, IsURL(h.Providers[0]), "provider should be a local path, not a URL") + assert.True(t, filepath.IsAbs(h.Providers[0]), "provider should be absolute (cache path)") + + // Verify cached content matches + content, err := os.ReadFile(h.Providers[0]) + require.NoError(t, err) + assert.Equal(t, providerContent, content) + + // Check dependency + hasDep := false + for _, d := range deps { + if d.Field == "providers[0]" { + hasDep = true + assert.Equal(t, server.URL+"/providers/test-provider.yaml", d.URL) + assert.Equal(t, providerHash, d.SHA256) + } + } + assert.True(t, hasDep, "should have a dependency for the provider") +} + +func TestLoadWithBase_URLBase_BareProviderNameSkipped(t *testing.T) { + // A URL-fetched base harness with a bare provider name like "fullsend-github" + // should NOT trigger a fetch attempt. Only relative paths should be fetched. + baseContent := []byte(` +agent: agents/remote.md +role: test +providers: + - fullsend-github + - providers/custom.yaml +`) + baseHash := computeHash(baseContent) + + providerContent := []byte(`name: custom-provider +type: custom +credentials: + CUSTOM_KEY: "" +`) + + agentContent := []byte("# test agent") + + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/base.yaml": + w.Write(baseContent) + case "/providers/custom.yaml": + w.Write(providerContent) + case "/agents/remote.md": + w.Write(agentContent) + case "/fullsend-github": + // If we hit this path, the bare name wasn't skipped + t.Error("bare provider name 'fullsend-github' was not skipped; tried to fetch as relative path") + w.WriteHeader(http.StatusNotFound) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + dir := t.TempDir() + cacheDir := filepath.Join(dir, "cache") + + path := writeTestHarness(t, dir, "child.yaml", ` +agent: agents/child.md +role: test +base: `+server.URL+`/base.yaml#sha256=`+baseHash+` +`) + + policy := fetch.NewTestPolicy( + server.Client().Transport.(*http.Transport).TLSClientConfig, + []string{"127.0.0.1"}, + []string{server.Listener.Addr().String()[len("127.0.0.1:"):]}, + ) + + h, deps, err := LoadWithBase(context.Background(), path, ComposeOpts{ + WorkspaceRoot: cacheDir, + FetchPolicy: policy, + OrgAllowlist: []string{server.URL + "/"}, + }) + require.NoError(t, err) + + // Should have two providers: the bare name (unchanged) and the resolved path + require.Len(t, h.Providers, 2) + assert.Equal(t, "fullsend-github", h.Providers[0], "bare provider name should remain unchanged") + assert.True(t, filepath.IsAbs(h.Providers[1]), "relative provider should be resolved to cache path") + + // Should have one dependency for the relative provider path, none for bare name + providerDeps := 0 + for _, d := range deps { + if strings.HasPrefix(d.Field, "providers[") { + providerDeps++ + assert.Equal(t, "providers[1]", d.Field, "only providers[1] (relative path) should be fetched") + } + } + assert.Equal(t, 1, providerDeps, "should have exactly one provider dependency (for relative path)") +} diff --git a/internal/harness/harness.go b/internal/harness/harness.go index e0ca02e0c..346e35cb2 100644 --- a/internal/harness/harness.go +++ b/internal/harness/harness.go @@ -413,8 +413,8 @@ func (h *Harness) Validate() error { } } for i, p := range h.Providers { - if IsURL(p) { - continue // validated by ValidateResourceTypes below + if IsURL(p) || filepath.IsAbs(p) || IsProviderPath(p) { + continue // URL or path — validated by ValidateResourceTypes below } if !validProviderName.MatchString(p) { return fmt.Errorf("providers[%d] name %q contains invalid characters (allowed: a-z, A-Z, 0-9, _, -)", i, p) @@ -575,6 +575,21 @@ func (h *Harness) ResolveRelativeTo(baseDir string) error { } } } + if h.OpenShell != nil { + for i := range h.OpenShell.Profiles { + if h.OpenShell.Profiles[i], err = resolve(fmt.Sprintf("openshell.profiles[%d]", i), h.OpenShell.Profiles[i]); err != nil { + return err + } + } + } + for i := range h.Providers { + p := h.Providers[i] + if IsProviderPath(p) { + if h.Providers[i], err = resolve(fmt.Sprintf("providers[%d]", i), p); err != nil { + return err + } + } + } return nil } @@ -687,6 +702,18 @@ func (h *Harness) ValidateFilesExist() error { return err } } + for i, p := range h.OpenShellProfiles() { + if err := check(fmt.Sprintf("openshell.profiles[%d]", i), p); err != nil { + return err + } + } + for i, p := range h.Providers { + if IsProviderPath(p) { + if err := check(fmt.Sprintf("providers[%d]", i), p); err != nil { + return err + } + } + } if h.ValidationLoop != nil { if err := check("validation_loop.script", h.ValidationLoop.Script); err != nil { return err @@ -836,11 +863,10 @@ func (h *Harness) ValidateResourceTypes() error { } } for i, p := range h.OpenShellProfiles() { - if !IsURL(p) { - return fmt.Errorf("openshell.profiles[%d] must be a URL (local profiles are not supported)", i) - } - if _, _, hasHash := ParseIntegrityHash(p); !hasHash { - return fmt.Errorf("openshell.profiles[%d] URL must include #sha256=... integrity hash", i) + if IsURL(p) { + if _, _, hasHash := ParseIntegrityHash(p); !hasHash { + return fmt.Errorf("openshell.profiles[%d] URL must include #sha256=... integrity hash", i) + } } } for i, p := range h.Providers { @@ -888,8 +914,10 @@ func (h *Harness) HasURLReferences() bool { return true } } - if len(h.OpenShellProfiles()) > 0 { // profiles are always URLs (enforced by ValidateResourceTypes) - return true + for _, profile := range h.OpenShellProfiles() { + if IsURL(profile) { + return true + } } for _, p := range h.Providers { if IsURL(p) { diff --git a/internal/harness/harness_test.go b/internal/harness/harness_test.go index a53ec7601..764a0ca88 100644 --- a/internal/harness/harness_test.go +++ b/internal/harness/harness_test.go @@ -831,6 +831,79 @@ func TestResolveRelativeTo_URLsUnchanged(t *testing.T) { assert.Equal(t, "/base/dir/skills/local-skill", h.Skills[0]) } +func TestResolveRelativeTo_Profiles(t *testing.T) { + h := &Harness{ + Agent: "agents/test.md", + OpenShell: &OpenShellConfig{ + Profiles: []string{"profiles/network.yaml"}, + }, + } + + require.NoError(t, h.ResolveRelativeTo("/base/dir")) + + assert.Equal(t, "/base/dir/profiles/network.yaml", h.OpenShellProfiles()[0]) +} + +func TestResolveRelativeTo_ProfilesUnchanged(t *testing.T) { + tests := []struct { + name string + profile string + }{ + {"absolute path", "/abs/cache/profiles/network.yaml"}, + {"URL", "https://example.com/profiles/net.yaml#sha256=abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := &Harness{ + Agent: "agents/test.md", + OpenShell: &OpenShellConfig{Profiles: []string{tt.profile}}, + } + require.NoError(t, h.ResolveRelativeTo("/base/dir")) + assert.Equal(t, tt.profile, h.OpenShellProfiles()[0]) + }) + } +} + +func TestResolveRelativeTo_ProfileProviderTraversalRejected(t *testing.T) { + tests := []struct { + name string + harness Harness + }{ + { + "profile traversal", + Harness{Agent: "agents/test.md", OpenShell: &OpenShellConfig{Profiles: []string{"../escape/profiles/net.yaml"}}}, + }, + { + "provider traversal", + Harness{Agent: "agents/test.md", Providers: []string{"../escape/providers/evil.yaml"}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.harness.ResolveRelativeTo("/base/dir") + require.Error(t, err) + assert.Contains(t, err.Error(), "resolves outside") + }) + } +} + +func TestResolveRelativeTo_Providers(t *testing.T) { + h := &Harness{ + Agent: "agents/test.md", + Providers: []string{ + "providers/custom.yaml", + "fullsend-github", + }, + } + + require.NoError(t, h.ResolveRelativeTo("/base/dir")) + + // File path gets resolved + assert.Equal(t, "/base/dir/providers/custom.yaml", h.Providers[0]) + // Bare provider name stays unchanged + assert.Equal(t, "fullsend-github", h.Providers[1]) +} + func TestValidateFilesExist_MissingPlugin(t *testing.T) { dir := t.TempDir() agentFile := filepath.Join(dir, "agent.md") @@ -860,6 +933,48 @@ func TestValidateFilesExist_SkipsOptionalPaths(t *testing.T) { require.NoError(t, h.ValidateFilesExist()) } +func TestValidateFilesExist_MissingProfile(t *testing.T) { + dir := t.TempDir() + agentFile := filepath.Join(dir, "agent.md") + require.NoError(t, os.WriteFile(agentFile, []byte("agent"), 0o644)) + + h := &Harness{ + Agent: agentFile, + OpenShell: &OpenShellConfig{ + Profiles: []string{"/nonexistent/profile.yaml"}, + }, + } + err := h.ValidateFilesExist() + require.Error(t, err) + assert.Contains(t, err.Error(), "openshell.profiles[0]") +} + +func TestValidateFilesExist_MissingProviderPath(t *testing.T) { + dir := t.TempDir() + agentFile := filepath.Join(dir, "agent.md") + require.NoError(t, os.WriteFile(agentFile, []byte("agent"), 0o644)) + + h := &Harness{ + Agent: agentFile, + Providers: []string{"/nonexistent/provider.yaml"}, + } + err := h.ValidateFilesExist() + require.Error(t, err) + assert.Contains(t, err.Error(), "providers[0]") +} + +func TestValidateFilesExist_BareProviderNameSkipped(t *testing.T) { + dir := t.TempDir() + agentFile := filepath.Join(dir, "agent.md") + require.NoError(t, os.WriteFile(agentFile, []byte("agent"), 0o644)) + + h := &Harness{ + Agent: agentFile, + Providers: []string{"fullsend-github"}, + } + require.NoError(t, h.ValidateFilesExist()) +} + // --- AllowedRemoteResources tests --- func TestHarness_AllowedRemoteResources_Parse(t *testing.T) { @@ -1206,6 +1321,11 @@ func TestHasURLReferences(t *testing.T) { h: Harness{Agent: "agents/test.md", Providers: []string{"my-provider"}}, want: false, }, + { + name: "local profile only", + h: Harness{Agent: "agents/test.md", OpenShell: &OpenShellConfig{Profiles: []string{"/cache/profiles/net.yaml"}}}, + want: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1711,6 +1831,8 @@ env: } func TestValidateResourceTypes_ProfilesRequireURL(t *testing.T) { + // Profiles can now be local paths (resolved from base composition) or URLs with hashes. + // This test verifies that local profile paths are accepted. h := &Harness{ Agent: "agents/test.md", Role: "test", @@ -1719,8 +1841,7 @@ func TestValidateResourceTypes_ProfilesRequireURL(t *testing.T) { }, } err := h.ValidateResourceTypes() - require.Error(t, err) - assert.Contains(t, err.Error(), "openshell.profiles[0] must be a URL") + require.NoError(t, err) } func TestValidateResourceTypes_ProfilesRequireIntegrityHash(t *testing.T) { diff --git a/internal/harness/url.go b/internal/harness/url.go index 78816c897..6c35c269b 100644 --- a/internal/harness/url.go +++ b/internal/harness/url.go @@ -2,6 +2,7 @@ package harness import ( "path/filepath" + "strings" "github.com/fullsend-ai/fullsend/internal/urlutil" ) @@ -21,6 +22,13 @@ func IsRelPath(s string) bool { return s != "" && !IsURL(s) && !IsAbsPath(s) } +// IsProviderPath returns true if s looks like a file path rather than a bare +// provider name. A provider string is a path if it contains a directory +// separator or ends with a YAML extension. +func IsProviderPath(s string) bool { + return strings.Contains(s, "/") || strings.HasSuffix(s, ".yaml") || strings.HasSuffix(s, ".yml") +} + // 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. diff --git a/internal/harness/url_test.go b/internal/harness/url_test.go index 12cb31ad4..f011ffec4 100644 --- a/internal/harness/url_test.go +++ b/internal/harness/url_test.go @@ -77,6 +77,29 @@ func TestIsRelPath(t *testing.T) { } } +func TestIsProviderPath(t *testing.T) { + tests := []struct { + name string + input string + want bool + }{ + {"bare name", "fullsend-github", false}, + {"bare name with underscore", "my_provider", false}, + {"relative path with slash", "providers/custom.yaml", true}, + {"yaml extension no slash", "custom.yaml", true}, + {"yml extension no slash", "custom.yml", true}, + {"absolute path", "/cache/providers/custom.yaml", true}, + {"nested path", "org/repo/providers/custom.yaml", true}, + {"dot prefix", "./providers/custom.yaml", true}, + {"empty string", "", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, IsProviderPath(tt.input)) + }) + } +} + func TestParseIntegrityHash(t *testing.T) { validHash := "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index 7bf27c5c1..97c95c95d 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -54,6 +54,7 @@ type ResolveResult struct { Deps []Dependency Profiles []ResolvedProfile Providers []ResolvedProvider + Warnings []string } // ProfileYAML is the subset of an openshell profile definition needed @@ -112,6 +113,27 @@ func WarnLiteralCredentials(providerName string, creds map[string]string) string providerName, strings.Join(bad, ", ")) } +func parseProviderDef(content []byte, index int, source string) (harness.ProviderDef, string, error) { + var def harness.ProviderDef + if err := yaml.Unmarshal(content, &def); err != nil { + return harness.ProviderDef{}, "", fmt.Errorf("parsing provider from %s: %w", source, err) + } + if def.Name == "" { + return harness.ProviderDef{}, "", fmt.Errorf("providers[%d]: provider name is required in %s", index, source) + } + if !validIdentifier.MatchString(def.Name) { + return harness.ProviderDef{}, "", fmt.Errorf("providers[%d]: provider name %q contains invalid characters (must match [a-zA-Z0-9][a-zA-Z0-9_-]*) in %s", index, def.Name, source) + } + if def.Type == "" { + return harness.ProviderDef{}, "", fmt.Errorf("providers[%d]: provider type is required in %s", index, source) + } + if !validIdentifier.MatchString(def.Type) { + return harness.ProviderDef{}, "", fmt.Errorf("providers[%d]: provider type %q contains invalid characters (must match [a-zA-Z0-9][a-zA-Z0-9_-]*) in %s", index, def.Type, source) + } + w := WarnLiteralCredentials(def.Name, def.Credentials) + return def, w, nil +} + // ResolveOpts controls how URL-referenced resources are resolved. type ResolveOpts struct { WorkspaceRoot string @@ -145,6 +167,7 @@ type resolveState struct { inDeps map[string]bool resourceCount int deps []Dependency + warnings []string maxDepth int maxResources int } @@ -240,48 +263,78 @@ func ResolveHarness(ctx context.Context, h *harness.Harness, opts ResolveOpts) ( } h.Skills = filtered - // Resolve profiles (all entries must be URLs — enforced by - // ValidateResourceTypes at load time). + // Resolve profiles: URL entries are fetched and cached; local paths + // (from ResolveRelativeTo or base composition cache) are used directly. var profiles []ResolvedProfile for i, p := range h.OpenShellProfiles() { - if !harness.IsURL(p) { - return ResolveResult{}, fmt.Errorf("openshell.profiles[%d]: expected URL, got local path %q", i, p) - } - dep, localPath, err := resolveFileURL(ctx, fmt.Sprintf("openshell.profiles[%d]", i), p, h, opts, state) - if err != nil { - return ResolveResult{}, fmt.Errorf("resolving openshell.profiles[%d]: %w", i, err) - } + var localPath string + if harness.IsURL(p) { + dep, lp, err := resolveFileURL(ctx, fmt.Sprintf("openshell.profiles[%d]", i), p, h, opts, state) + if err != nil { + return ResolveResult{}, fmt.Errorf("resolving openshell.profiles[%d]: %w", i, err) + } + localPath = lp - content, err := os.ReadFile(localPath) - if err != nil { - return ResolveResult{}, fmt.Errorf("reading resolved profile %s: %w", localPath, err) - } - id, err := ParseProfileID(content) - if err != nil { - return ResolveResult{}, fmt.Errorf("openshell.profiles[%d]: %w (from %s)", i, err, dep.URL) - } + content, err := os.ReadFile(localPath) + if err != nil { + return ResolveResult{}, fmt.Errorf("reading profile %s: %w", p, err) + } + id, err := ParseProfileID(content) + if err != nil { + return ResolveResult{}, fmt.Errorf("openshell.profiles[%d]: %w (from %s)", i, err, p) + } - // Create a named symlink so openshell sees a .yaml extension - // instead of the extensionless cache-internal "content" filename. - localPath, err = fetch.CacheNamedSymlink(localPath, id+".yaml") - if err != nil { - return ResolveResult{}, fmt.Errorf("naming cached profile for openshell.profiles[%d]: %w", i, err) - } - dep.LocalPath = localPath - // Keep the fetch-dedup cache in sync with the renamed path, so a - // second reference to the same profile URL (elsewhere in the same - // harness) doesn't resolve to the pre-rename cache path. - state.resolved[dep.URL] = dep + // Create a named symlink so openshell sees a .yaml extension + // instead of the extensionless cache-internal "content" filename. + localPath, err = fetch.CacheNamedSymlink(localPath, id+".yaml") + if err != nil { + return ResolveResult{}, fmt.Errorf("naming cached profile for openshell.profiles[%d]: %w", i, err) + } + dep.LocalPath = localPath + // Keep the fetch-dedup cache in sync with the renamed path, so a + // second reference to the same profile URL (elsewhere in the same + // harness) doesn't resolve to the pre-rename cache path. + state.resolved[dep.URL] = dep - state.appendDependency(dep) - profiles = append(profiles, ResolvedProfile{ID: id, LocalPath: localPath}) + state.appendDependency(dep) + profiles = append(profiles, ResolvedProfile{ID: id, LocalPath: localPath}) + } else { + localPath = p + + content, err := os.ReadFile(localPath) + if err != nil { + return ResolveResult{}, fmt.Errorf("reading profile %s: %w", localPath, err) + } + id, err := ParseProfileID(content) + if err != nil { + return ResolveResult{}, fmt.Errorf("openshell.profiles[%d]: %w (from %s)", i, err, localPath) + } + profiles = append(profiles, ResolvedProfile{ID: id, LocalPath: localPath}) + } } - // Resolve providers + // Resolve providers: URL entries are fetched and cached; absolute-path + // entries (from ResolveRelativeTo or base composition cache) are read + // directly; bare provider names are kept in h.Providers for LoadProviderDefs. var resolvedProviders []ResolvedProvider remaining := h.Providers[:0] for i, p := range h.Providers { if !harness.IsURL(p) { + if filepath.IsAbs(p) { + content, err := os.ReadFile(p) + if err != nil { + return ResolveResult{}, fmt.Errorf("reading provider %s: %w", p, err) + } + def, w, err := parseProviderDef(content, i, p) + if err != nil { + return ResolveResult{}, err + } + if w != "" { + state.warnings = append(state.warnings, w) + } + resolvedProviders = append(resolvedProviders, ResolvedProvider{Def: def, LocalPath: p}) + continue + } remaining = append(remaining, p) continue } @@ -294,24 +347,11 @@ func ResolveHarness(ctx context.Context, h *harness.Harness, opts ResolveOpts) ( if err != nil { return ResolveResult{}, fmt.Errorf("reading resolved provider %s: %w", localPath, err) } - var def harness.ProviderDef - if err := yaml.Unmarshal(content, &def); err != nil { - return ResolveResult{}, fmt.Errorf("parsing provider from %s: %w", dep.URL, err) - } - if def.Name == "" { - return ResolveResult{}, fmt.Errorf("providers[%d]: provider name is required in %s", i, dep.URL) - } - if !validIdentifier.MatchString(def.Name) { - return ResolveResult{}, fmt.Errorf("providers[%d]: provider name %q contains invalid characters (must match [a-zA-Z0-9][a-zA-Z0-9_-]*) in %s", i, def.Name, dep.URL) - } - if def.Type == "" { - return ResolveResult{}, fmt.Errorf("providers[%d]: provider type is required in %s", i, dep.URL) - } - if !validIdentifier.MatchString(def.Type) { - return ResolveResult{}, fmt.Errorf("providers[%d]: provider type %q contains invalid characters (must match [a-zA-Z0-9][a-zA-Z0-9_-]*) in %s", i, def.Type, dep.URL) + def, w, err := parseProviderDef(content, i, dep.URL) + if err != nil { + return ResolveResult{}, err } - - if w := WarnLiteralCredentials(def.Name, def.Credentials); w != "" { + if w != "" { dep.Warning = w } state.appendDependency(dep) @@ -326,6 +366,7 @@ func ResolveHarness(ctx context.Context, h *harness.Harness, opts ResolveOpts) ( Deps: state.deps, Profiles: profiles, Providers: resolvedProviders, + Warnings: state.warnings, }, nil } diff --git a/internal/resolve/resolve_test.go b/internal/resolve/resolve_test.go index a956df808..bbb2b66e5 100644 --- a/internal/resolve/resolve_test.go +++ b/internal/resolve/resolve_test.go @@ -1761,3 +1761,142 @@ func TestParseProfileID(t *testing.T) { }) } } + +func TestResolveHarness_LocalProfile(t *testing.T) { + root := t.TempDir() + + profileContent := []byte("id: test-profile\nnetwork:\n egress:\n - host: example.com\n") + profilePath := filepath.Join(root, "profiles", "test-profile.yaml") + require.NoError(t, os.MkdirAll(filepath.Dir(profilePath), 0755)) + require.NoError(t, os.WriteFile(profilePath, profileContent, 0644)) + + h := &harness.Harness{ + Agent: "/abs/path/agents/test.md", + OpenShell: &harness.OpenShellConfig{ + Profiles: []string{profilePath}, + }, + } + + result, err := ResolveHarness(context.Background(), h, ResolveOpts{ + WorkspaceRoot: root, + }) + require.NoError(t, err) + + require.Len(t, result.Profiles, 1) + assert.Equal(t, "test-profile", result.Profiles[0].ID) + assert.Equal(t, profilePath, result.Profiles[0].LocalPath) + + // Profiles slice should be cleared after resolution + assert.Nil(t, h.OpenShell.Profiles) +} + +func TestResolveHarness_LocalAbsProvider(t *testing.T) { + root := t.TempDir() + + providerContent := []byte("name: test-provider\ntype: custom\ncredentials:\n TEST_KEY: \"\"\n") + providerPath := filepath.Join(root, "providers", "test-provider.yaml") + require.NoError(t, os.MkdirAll(filepath.Dir(providerPath), 0755)) + require.NoError(t, os.WriteFile(providerPath, providerContent, 0644)) + + h := &harness.Harness{ + Agent: "/abs/path/agents/test.md", + Providers: []string{providerPath, "bare-provider-name"}, + } + + result, err := ResolveHarness(context.Background(), h, ResolveOpts{ + WorkspaceRoot: root, + }) + require.NoError(t, err) + + // Absolute path provider resolved + require.Len(t, result.Providers, 1) + assert.Equal(t, "test-provider", result.Providers[0].Def.Name) + assert.Equal(t, "custom", result.Providers[0].Def.Type) + assert.Equal(t, providerPath, result.Providers[0].LocalPath) + + // Bare provider name kept in h.Providers + require.Len(t, h.Providers, 1) + assert.Equal(t, "bare-provider-name", h.Providers[0]) +} + +func TestParseProviderDef(t *testing.T) { + tests := []struct { + name string + content string + wantErr string + }{ + { + "valid", + "name: my-provider\ntype: custom\n", + "", + }, + { + "missing name", + "type: custom\n", + "provider name is required", + }, + { + "missing type", + "name: my-provider\n", + "provider type is required", + }, + { + "invalid name chars", + "name: my.provider\ntype: custom\n", + "invalid characters", + }, + { + "invalid type chars", + "name: my-provider\ntype: my.type\n", + "invalid characters", + }, + { + "invalid yaml", + ":::not yaml", + "parsing provider", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + def, _, err := parseProviderDef([]byte(tt.content), 0, "test-source") + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + } else { + require.NoError(t, err) + assert.Equal(t, "my-provider", def.Name) + assert.Equal(t, "custom", def.Type) + } + }) + } +} + +func TestParseProviderDef_CredentialWarning(t *testing.T) { + content := []byte("name: my-provider\ntype: custom\ncredentials:\n API_KEY: hardcoded-secret\n") + _, w, err := parseProviderDef(content, 0, "test-source") + require.NoError(t, err) + assert.Contains(t, w, "API_KEY") + assert.Contains(t, w, "do not look like ${VAR} references") +} + +func TestResolveHarness_LocalProviderWarnings(t *testing.T) { + root := t.TempDir() + + providerContent := []byte("name: leaky\ntype: custom\ncredentials:\n SECRET: hardcoded-value\n") + providerPath := filepath.Join(root, "providers", "leaky.yaml") + require.NoError(t, os.MkdirAll(filepath.Dir(providerPath), 0755)) + require.NoError(t, os.WriteFile(providerPath, providerContent, 0644)) + + h := &harness.Harness{ + Agent: "/abs/agents/test.md", + Providers: []string{providerPath}, + } + + result, err := ResolveHarness(context.Background(), h, ResolveOpts{ + WorkspaceRoot: root, + }) + require.NoError(t, err) + require.Len(t, result.Providers, 1) + require.Len(t, result.Warnings, 1) + assert.Contains(t, result.Warnings[0], "SECRET") +}