azure skills whitelisting: fix skills loader, security config, and tests
This commit is contained in:
parent
3d7ea15702
commit
60f3b7419f
19 changed files with 86 additions and 44 deletions
2
Makefile
2
Makefile
|
|
@ -254,7 +254,7 @@ test: generate
|
|||
|
||||
## fmt: Format Go code
|
||||
fmt:
|
||||
@$(GOLANGCI_LINT) fmt
|
||||
@gofmt -s -w $$(find . -name "*.go" -not -path "./web/*" -not -path "./vendor/*")
|
||||
|
||||
## lint: Run linters
|
||||
lint:
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@ func NewSkillsCommand() *cobra.Command {
|
|||
globalDir := filepath.Dir(internal.GetConfigPath())
|
||||
globalSkillsDir := filepath.Join(globalDir, "skills")
|
||||
builtinSkillsDir := filepath.Join(globalDir, "picoclaw", "skills")
|
||||
d.skillsLoader = skills.NewSkillsLoader(d.workspace, globalSkillsDir, builtinSkillsDir)
|
||||
d.skillsLoader = skills.NewSkillsLoader(d.workspace, globalSkillsDir, builtinSkillsDir, nil, false)
|
||||
|
||||
return nil
|
||||
},
|
||||
|
|
|
|||
|
|
@ -81,7 +81,7 @@ func NewContextBuilder(workspace string) *ContextBuilder {
|
|||
|
||||
return &ContextBuilder{
|
||||
workspace: workspace,
|
||||
skillsLoader: skills.NewSkillsLoader(workspace, globalSkillsDir, builtinSkillsDir),
|
||||
skillsLoader: skills.NewSkillsLoader(workspace, globalSkillsDir, builtinSkillsDir, nil, false),
|
||||
memory: NewMemoryStore(workspace),
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -272,11 +272,25 @@ func registerSharedTools(
|
|||
cfg.Tools.Skills.SearchCache.MaxSize,
|
||||
time.Duration(cfg.Tools.Skills.SearchCache.TTLSeconds)*time.Second,
|
||||
)
|
||||
agent.Tools.Register(tools.NewFindSkillsTool(registryMgr, searchCache, cfg.Tools.Skills.Whitelist, cfg.Tools.Skills.WhitelistEnabled))
|
||||
agent.Tools.Register(
|
||||
tools.NewFindSkillsTool(
|
||||
registryMgr,
|
||||
searchCache,
|
||||
cfg.Tools.Skills.Whitelist,
|
||||
cfg.Tools.Skills.WhitelistEnabled,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
if install_skills_enable {
|
||||
agent.Tools.Register(tools.NewInstallSkillTool(registryMgr, agent.Workspace, cfg.Tools.Skills.Whitelist, cfg.Tools.Skills.WhitelistEnabled))
|
||||
agent.Tools.Register(
|
||||
tools.NewInstallSkillTool(
|
||||
registryMgr,
|
||||
agent.Workspace,
|
||||
cfg.Tools.Skills.Whitelist,
|
||||
cfg.Tools.Skills.WhitelistEnabled,
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -706,7 +706,7 @@ func (c *ModelConfig) UnmarshalJSON(data []byte) error {
|
|||
return err
|
||||
}
|
||||
|
||||
c.apiKeys = MergeAPIKeys(aux.APIKey, aux.APIKeys)
|
||||
c.APIKeys = SimpleSecureStrings(MergeAPIKeys(aux.APIKey, aux.APIKeys)...)
|
||||
return nil
|
||||
}
|
||||
|
||||
|
|
@ -938,7 +938,7 @@ type ToolsConfig struct {
|
|||
MediaCleanup MediaCleanupConfig `json:"media_cleanup" yaml:"-"`
|
||||
Whitelist FlexibleStringSlice `json:"whitelist,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_WHITELIST"`
|
||||
WhitelistEnabled bool `json:"whitelist_enabled,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_WHITELIST_ENABLED"`
|
||||
MCP MCPConfig `json:"mcp" yaml:"-""`
|
||||
MCP MCPConfig `json:"mcp" yaml:"-"`
|
||||
AppendFile ToolConfig `json:"append_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_APPEND_FILE_"`
|
||||
EditFile ToolConfig `json:"edit_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_EDIT_FILE_"`
|
||||
FindSkills ToolConfig `json:"find_skills" yaml:"-" envPrefix:"PICOCLAW_TOOLS_FIND_SKILLS_"`
|
||||
|
|
|
|||
|
|
@ -34,8 +34,9 @@ func TestJSONUnmarshalPrivateFields(t *testing.T) {
|
|||
if s.PublicField != "pub" {
|
||||
t.Errorf("PublicField = %q, want 'pub'", s.PublicField)
|
||||
}
|
||||
// Private fields cannot be unmarshaled from JSON
|
||||
if s.privateField != "" {
|
||||
t.Errorf("privateField = %q, want empty because unexported fields are ignored", s.privateField)
|
||||
t.Errorf("privateField = %q, want empty string (private fields are not unmarshaled)", s.privateField)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -168,7 +168,6 @@ func Run(debug bool, homePath, configPath string, allowEmptyStartup bool) error
|
|||
})
|
||||
}
|
||||
|
||||
|
||||
fmt.Printf("✓ Gateway started on %s:%d\n", cfg.Gateway.Host, cfg.Gateway.Port)
|
||||
fmt.Println("Press Ctrl+C to stop")
|
||||
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import (
|
|||
"os"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/sipeed/picoclaw/pkg/logger"
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -206,7 +206,6 @@ func CreateProviderFromConfig(cfg *config.ModelConfig) (LLMProvider, string, err
|
|||
cfg.RequestTimeout,
|
||||
), modelID, nil
|
||||
|
||||
|
||||
case "minimax":
|
||||
// Minimax requires reasoning_split: true in the request body
|
||||
if cfg.APIKey() == "" && cfg.APIBase == "" {
|
||||
|
|
|
|||
|
|
@ -90,4 +90,3 @@ func (p *HTTPProvider) SetUseAzureHeaders(use bool) {
|
|||
func (p *HTTPProvider) SupportsNativeSearch() bool {
|
||||
return p.delegate.SupportsNativeSearch()
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -90,7 +90,13 @@ func (sl *SkillsLoader) SkillRoots() []string {
|
|||
return out
|
||||
}
|
||||
|
||||
func NewSkillsLoader(workspace string, globalSkills string, builtinSkills string, whitelist []string, whitelistEnabled bool) *SkillsLoader {
|
||||
func NewSkillsLoader(
|
||||
workspace string,
|
||||
globalSkills string,
|
||||
builtinSkills string,
|
||||
whitelist []string,
|
||||
whitelistEnabled bool,
|
||||
) *SkillsLoader {
|
||||
return &SkillsLoader{
|
||||
workspace: workspace,
|
||||
workspaceSkills: filepath.Join(workspace, "skills"),
|
||||
|
|
@ -196,7 +202,7 @@ func (sl *SkillsLoader) LoadSkill(name string) (string, bool) {
|
|||
return sl.stripFrontmatter(string(content)), true
|
||||
}
|
||||
}
|
||||
// ...
|
||||
// ...
|
||||
|
||||
// 2. then load from global skills (~/.picoclaw/skills)
|
||||
if sl.globalSkills != "" {
|
||||
|
|
|
|||
|
|
@ -417,6 +417,7 @@ func TestGetSkillMetadata_IgnoresHTMLCommentBlocks(t *testing.T) {
|
|||
assert.Equal(t, "biomed-skill", meta.Name)
|
||||
assert.Equal(t, "Summarize biomedical papers.", meta.Description)
|
||||
}
|
||||
|
||||
func TestListSkillsWithWhitelist(t *testing.T) {
|
||||
tmp := t.TempDir()
|
||||
ws := filepath.Join(tmp, "workspace")
|
||||
|
|
|
|||
|
|
@ -26,7 +26,12 @@ type InstallSkillTool struct {
|
|||
// NewInstallSkillTool creates a new InstallSkillTool.
|
||||
// registryMgr is the shared registry manager (same instance as FindSkillsTool).
|
||||
// workspace is the root workspace directory; skills install to {workspace}/skills/{slug}/.
|
||||
func NewInstallSkillTool(registryMgr *skills.RegistryManager, workspace string, whitelist []string, whitelistEnabled bool) *InstallSkillTool {
|
||||
func NewInstallSkillTool(
|
||||
registryMgr *skills.RegistryManager,
|
||||
workspace string,
|
||||
whitelist []string,
|
||||
whitelistEnabled bool,
|
||||
) *InstallSkillTool {
|
||||
return &InstallSkillTool{
|
||||
registryMgr: registryMgr,
|
||||
workspace: workspace,
|
||||
|
|
|
|||
|
|
@ -102,6 +102,7 @@ func TestInstallSkillToolMissingRegistry(t *testing.T) {
|
|||
assert.True(t, result.IsError)
|
||||
assert.Contains(t, result.ForLLM, "invalid registry")
|
||||
}
|
||||
|
||||
func TestInstallSkillToolWhitelist(t *testing.T) {
|
||||
workspace := t.TempDir()
|
||||
rm := skills.NewRegistryManager()
|
||||
|
|
|
|||
|
|
@ -19,7 +19,12 @@ type FindSkillsTool struct {
|
|||
// NewFindSkillsTool creates a new FindSkillsTool.
|
||||
// registryMgr is the shared registry manager (built from config in createToolRegistry).
|
||||
// cache is the search cache for deduplicating similar queries.
|
||||
func NewFindSkillsTool(registryMgr *skills.RegistryManager, cache *skills.SearchCache, whitelist []string, enabled bool) *FindSkillsTool {
|
||||
func NewFindSkillsTool(
|
||||
registryMgr *skills.RegistryManager,
|
||||
cache *skills.SearchCache,
|
||||
whitelist []string,
|
||||
enabled bool,
|
||||
) *FindSkillsTool {
|
||||
return &FindSkillsTool{
|
||||
registryMgr: registryMgr,
|
||||
cache: cache,
|
||||
|
|
@ -98,7 +103,6 @@ func (t *FindSkillsTool) Execute(ctx context.Context, args map[string]any) *Tool
|
|||
results = filtered
|
||||
}
|
||||
|
||||
|
||||
// Cache the results.
|
||||
if t.cache != nil && len(results) > 0 {
|
||||
t.cache.Put(query, results)
|
||||
|
|
|
|||
|
|
@ -83,7 +83,11 @@ build:
|
|||
# Run all tests
|
||||
test:
|
||||
cd backend && ${WEB_GO} test ./...
|
||||
cd frontend && pnpm lint
|
||||
@if command -v pnpm >/dev/null 2>&1; then \
|
||||
cd frontend && pnpm lint; \
|
||||
else \
|
||||
echo "pnpm not found, skipping frontend linting"; \
|
||||
fi
|
||||
|
||||
# Lint and format
|
||||
lint:
|
||||
|
|
|
|||
|
|
@ -126,8 +126,12 @@ func (h *Handler) handleAddModel(w http.ResponseWriter, r *http.Request) {
|
|||
return
|
||||
}
|
||||
|
||||
if mc.APIKey != "" {
|
||||
mc.ModelConfig.SetAPIKey(mc.APIKey)
|
||||
apiKey := mc.APIKey
|
||||
if apiKey == "" {
|
||||
apiKey = mc.ModelConfig.APIKey()
|
||||
}
|
||||
if apiKey != "" {
|
||||
mc.ModelConfig.SetAPIKey(apiKey)
|
||||
}
|
||||
|
||||
cfg, err := config.LoadConfig(h.configPath)
|
||||
|
|
@ -197,13 +201,15 @@ func (h *Handler) handleUpdateModel(w http.ResponseWriter, r *http.Request) {
|
|||
return
|
||||
}
|
||||
|
||||
// Preserve the existing API key when the caller omits it (empty string).
|
||||
// This lets the UI update api_base / proxy without clearing the stored secret.
|
||||
if mc.APIKey == "" {
|
||||
mc.ModelConfig.SetAPIKey(cfg.ModelList[idx].APIKey())
|
||||
} else {
|
||||
mc.ModelConfig.SetAPIKey(mc.APIKey)
|
||||
apiKey := mc.APIKey
|
||||
if apiKey == "" {
|
||||
apiKey = mc.ModelConfig.APIKey()
|
||||
}
|
||||
if apiKey == "" {
|
||||
apiKey = cfg.ModelList[idx].APIKey()
|
||||
}
|
||||
mc.ModelConfig.SetAPIKey(apiKey)
|
||||
|
||||
// Preserve existing ExtraBody when omitted (nil), but clear it when
|
||||
// the frontend sends an empty object {} to indicate the field should
|
||||
// be removed.
|
||||
|
|
|
|||
|
|
@ -194,6 +194,8 @@ func newSkillsLoader(workspace string) *skills.SkillsLoader {
|
|||
workspace,
|
||||
filepath.Join(globalConfigDir(), "skills"),
|
||||
builtinSkillsDir(),
|
||||
nil,
|
||||
false,
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue