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
9 changes: 8 additions & 1 deletion cmd/claw/cli/auto.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,14 @@ func inspectBBHWorkspace(ctx context.Context, workspaceDir string) (string, erro
if err != nil {
return "", err
}
retrievalWS := sdkworkspace.Sub(sdkworkspace.Sub(ws, memoryRoot), "retrieval")
memoryWS, err := ws.Sub(memoryRoot)
if err != nil {
return "", fmt.Errorf("open memory workspace: %w", err)
}
retrievalWS, err := memoryWS.Sub("retrieval")
if err != nil {
return "", fmt.Errorf("open retrieval workspace: %w", err)
}
inspector, err := bbh.NewInspector(retrievalWS)
if err != nil {
return "", err
Expand Down
22 changes: 5 additions & 17 deletions memory/retrieval/bbh/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ const (
hnswGraphExt = ".graph"
)

type localRoot interface {
Root() string
}

// Index implements retrieval.Index using a shared Badger doc store plus
// per-namespace Bleve and HNSW indexes.
type Index struct {
Expand Down Expand Up @@ -71,22 +67,14 @@ type textDoc struct {
Content string `json:"content"`
}

// New constructs an Index rooted directly in a local workspace.
//
// BBH accepts the Workspace interface so callers can share the same
// constructor shape as other retrieval backends, but the current implementation
// requires a workspace that exposes a local Root() path because Bleve and
// Badger are path-backed embedded stores. Callers that need a common prefix
// should provide an already-prefixed workspace instead of configuring BBH.
func New(ws sdkworkspace.Workspace, opts ...Option) (*Index, error) {
// New constructs an Index rooted directly in a local workspace. Badger, Bleve,
// and HNSW are path-backed stores, so callers must provide an already-prefixed
// LocalWorkspace rather than a generic workspace implementation.
func New(ws *sdkworkspace.LocalWorkspace, opts ...Option) (*Index, error) {
if ws == nil {
return nil, errdefs.Validationf("retrieval/bbh: workspace is nil")
}
lr, ok := ws.(localRoot)
if !ok {
return nil, errdefs.Validationf("retrieval/bbh: workspace must expose local Root()")
}
root := lr.Root()
root := ws.Root()
cfg, err := resolveConfig(root, opts)
if err != nil {
return nil, err
Expand Down
16 changes: 3 additions & 13 deletions memory/retrieval/bbh/index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ func TestNewValidationAndClosedIndexErrors(t *testing.T) {
if _, err := New(nil); err == nil {
t.Fatal("nil workspace should fail")
}
if _, err := New(sdkworkspace.NewMemWorkspace()); err == nil {
t.Fatal("workspace without Root should fail")
}
if _, err := New(rootedWorkspace(t, filepath.Join(t.TempDir(), "missing-config")), WithConfigFilePath("missing.yaml")); err == nil {
t.Fatal("bad config in New should fail")
}
Expand Down Expand Up @@ -806,22 +803,15 @@ func openInternalIndex(t *testing.T, dir string, opts ...Option) *Index {
return idx
}

func rootedWorkspace(t *testing.T, root string) sdkworkspace.Workspace {
func rootedWorkspace(t *testing.T, root string) *sdkworkspace.LocalWorkspace {
t.Helper()
ws, err := sdkworkspace.NewLocalWorkspace(t.TempDir())
ws, err := sdkworkspace.NewLocalWorkspace(root)
if err != nil {
t.Fatal(err)
}
return localRootWorkspace{Workspace: ws, root: root}
}

type localRootWorkspace struct {
sdkworkspace.Workspace
root string
return ws
}

func (w localRootWorkspace) Root() string { return w.root }

func iterIDs(t *testing.T, idx *Index, ns, cursor string, batch int) []string {
t.Helper()
docs, next, err := idx.Iterate(context.Background(), ns, cursor, batch)
Expand Down
8 changes: 2 additions & 6 deletions memory/retrieval/bbh/inspector.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,11 @@ type NamespaceInspection struct {
// NewInspector opens an offline, read-only inspector rooted at a BBH workspace.
// It opens Badger with a read-only lock, so it cannot inspect a workspace while
// a writable Index is already open on the same path.
func NewInspector(ws sdkworkspace.Workspace) (*Inspector, error) {
func NewInspector(ws *sdkworkspace.LocalWorkspace) (*Inspector, error) {
if ws == nil {
return nil, errdefs.Validationf("retrieval/bbh: workspace is nil")
}
lr, ok := ws.(localRoot)
if !ok {
return nil, errdefs.Validationf("retrieval/bbh: workspace must expose local Root()")
}
root := lr.Root()
root := ws.Root()
in := &Inspector{root: root}
dbPath := filepath.Join(root, badgerDir)
info, err := os.Stat(dbPath)
Expand Down
9 changes: 3 additions & 6 deletions memory/retrieval/bbh/inspector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,13 @@ import (
"time"

"github.com/GizClaw/flowcraft/memory/retrieval"
sdkworkspace "github.com/GizClaw/flowcraft/sdk/workspace"
"github.com/dgraph-io/badger/v4"
)

func TestInspectorValidationAndEmptyWorkspace(t *testing.T) {
if _, err := NewInspector(nil); err == nil {
t.Fatal("nil workspace should fail")
}
if _, err := NewInspector(sdkworkspace.NewMemWorkspace()); err == nil {
t.Fatal("workspace without Root should fail")
}
dir := t.TempDir()
if err := os.WriteFile(filepath.Join(dir, badgerDir), []byte("not a dir"), 0o644); err != nil {
t.Fatal(err)
Expand All @@ -28,15 +24,16 @@ func TestInspectorValidationAndEmptyWorkspace(t *testing.T) {
}

empty := t.TempDir()
in, err := NewInspector(rootedWorkspace(t, empty))
emptyWS := rootedWorkspace(t, empty)
in, err := NewInspector(emptyWS)
if err != nil {
t.Fatalf("NewInspector empty: %v", err)
}
got, err := in.Inspect(context.Background())
if err != nil {
t.Fatalf("Inspect empty: %v", err)
}
if got.Root != empty || got.BadgerExists || got.BleveExists || got.HNSWExists {
if got.Root != emptyWS.Root() || got.BadgerExists || got.BleveExists || got.HNSWExists {
t.Fatalf("unexpected empty inspection: %+v", got)
}
if len(got.Namespaces) != 0 || got.TotalDocs != 0 {
Expand Down
22 changes: 5 additions & 17 deletions sdk/retrieval/bbh/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ const (
hnswGraphExt = ".graph"
)

type localRoot interface {
Root() string
}

// Index implements retrieval.Index using a shared Badger doc store plus
// per-namespace Bleve and HNSW indexes.
type Index struct {
Expand Down Expand Up @@ -73,22 +69,14 @@ type textDoc struct {
Content string `json:"content"`
}

// New constructs an Index rooted directly in a local workspace.
//
// BBH accepts the Workspace interface so callers can share the same
// constructor shape as other retrieval backends, but the current implementation
// requires a workspace that exposes a local Root() path because Bleve and
// Badger are path-backed embedded stores. Callers that need a common prefix
// should provide an already-prefixed workspace instead of configuring BBH.
func New(ws sdkworkspace.Workspace, opts ...Option) (*Index, error) {
// New constructs an Index rooted directly in a local workspace. Badger, Bleve,
// and HNSW are path-backed stores, so callers must provide an already-prefixed
// LocalWorkspace rather than a generic workspace implementation.
func New(ws *sdkworkspace.LocalWorkspace, opts ...Option) (*Index, error) {
if ws == nil {
return nil, errdefs.Validationf("retrieval/bbh: workspace is nil")
}
lr, ok := ws.(localRoot)
if !ok {
return nil, errdefs.Validationf("retrieval/bbh: workspace must expose local Root()")
}
root := lr.Root()
root := ws.Root()
cfg, err := resolveConfig(root, opts)
if err != nil {
return nil, err
Expand Down
16 changes: 3 additions & 13 deletions sdk/retrieval/bbh/index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ func TestNewValidationAndClosedIndexErrors(t *testing.T) {
if _, err := New(nil); err == nil {
t.Fatal("nil workspace should fail")
}
if _, err := New(sdkworkspace.NewMemWorkspace()); err == nil {
t.Fatal("workspace without Root should fail")
}
if _, err := New(rootedWorkspace(t, filepath.Join(t.TempDir(), "missing-config")), WithConfigFilePath("missing.yaml")); err == nil {
t.Fatal("bad config in New should fail")
}
Expand Down Expand Up @@ -807,22 +804,15 @@ func openInternalIndex(t *testing.T, dir string, opts ...Option) *Index {
return idx
}

func rootedWorkspace(t *testing.T, root string) sdkworkspace.Workspace {
func rootedWorkspace(t *testing.T, root string) *sdkworkspace.LocalWorkspace {
t.Helper()
ws, err := sdkworkspace.NewLocalWorkspace(t.TempDir())
ws, err := sdkworkspace.NewLocalWorkspace(root)
if err != nil {
t.Fatal(err)
}
return localRootWorkspace{Workspace: ws, root: root}
}

type localRootWorkspace struct {
sdkworkspace.Workspace
root string
return ws
}

func (w localRootWorkspace) Root() string { return w.root }

func iterIDs(t *testing.T, idx *Index, ns, cursor string, batch int) []string {
t.Helper()
docs, next, err := idx.Iterate(context.Background(), ns, cursor, batch)
Expand Down
30 changes: 30 additions & 0 deletions sdk/workspace/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,36 @@ func NewLocalWorkspace(root string) (*LocalWorkspace, error) {
// Root returns the absolute path of the workspace root.
func (w *LocalWorkspace) Root() string { return w.root }

// Sub returns a local workspace rooted under prefix.
//
// The resolved child root must remain inside the current workspace, including
// after symlink resolution performed by NewLocalWorkspace.
func (w *LocalWorkspace) Sub(prefix string) (*LocalWorkspace, error) {
if w == nil {
return nil, errdefs.Validationf("workspace: local workspace is nil")
}
cleaned, err := cleanPath(prefix)
if err != nil {
return nil, fmt.Errorf("workspace local sub: invalid prefix %q: %w", prefix, err)
}
if cleaned == "" {
return w, nil
}
full, err := w.resolve(cleaned)
if err != nil {
return nil, fmt.Errorf("workspace local sub: resolve root %q: %w", cleaned, err)
}
local, err := NewLocalWorkspace(full)
if err != nil {
return nil, fmt.Errorf("workspace local sub: open root %q: %w", cleaned, err)
}
root := local.Root()
if root != w.Root() && !strings.HasPrefix(root, w.Root()+string(filepath.Separator)) {
return nil, fmt.Errorf("%w: %s (symlink escape)", ErrPathTraversal, cleaned)
}
return local, nil
}

// Capabilities reports LocalWorkspace's storage characteristics:
// backed by the host filesystem, so Rename is atomic on the same
// device, writes are read-after-write consistent, and durability
Expand Down
47 changes: 47 additions & 0 deletions sdk/workspace/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,59 @@ package workspace

import (
"context"
"errors"
"os"
"path/filepath"
"runtime"
"testing"
)

func TestLocalWorkspace_Sub(t *testing.T) {
base, _ := newLocalWS(t)
child, err := base.Sub(filepath.Join("runtime-a", "memory"))
if err != nil {
t.Fatalf("Sub: %v", err)
}
want := filepath.Join(base.Root(), "runtime-a", "memory")
if child.Root() != want {
t.Fatalf("Root() = %q, want %q", child.Root(), want)
}
nested, err := child.Sub("retrieval")
if err != nil {
t.Fatalf("nested Sub: %v", err)
}
if want := filepath.Join(child.Root(), "retrieval"); nested.Root() != want {
t.Fatalf("nested Root() = %q, want %q", nested.Root(), want)
}
same, err := base.Sub(".")
if err != nil {
t.Fatalf("empty Sub: %v", err)
}
if same != base {
t.Fatal("empty Sub should return the receiver")
}
}

func TestLocalWorkspace_SubRejectsTraversalAndSymlinkEscape(t *testing.T) {
base, _ := newLocalWS(t)
if _, err := base.Sub("../escape"); !errors.Is(err, ErrPathTraversal) {
t.Fatalf("traversal Sub error = %v, want ErrPathTraversal", err)
}
if runtime.GOOS == "windows" {
return
}
outside := t.TempDir()
if err := os.Symlink(outside, filepath.Join(base.Root(), "escape")); err != nil {
t.Fatalf("Symlink: %v", err)
}
if _, err := base.Sub(filepath.Join("escape", "created")); !errors.Is(err, ErrPathTraversal) {
t.Fatalf("symlink Sub error = %v, want ErrPathTraversal", err)
}
if _, err := os.Stat(filepath.Join(outside, "created")); !os.IsNotExist(err) {
t.Fatalf("symlink Sub created an outside directory: %v", err)
}
}

func TestLocalWorkspace_ReadWrite(t *testing.T) {
ws, ctx := newLocalWS(t)

Expand Down
8 changes: 1 addition & 7 deletions sdk/workspace/sub.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"io/fs"
"path/filepath"
"strings"

"github.com/GizClaw/flowcraft/sdk/errdefs"
)
Expand Down Expand Up @@ -45,15 +44,10 @@ func Sub(inner Workspace, prefix string) Workspace {
sw := &subWorkspace{inner: inner, prefix: cleaned}
switch typed := inner.(type) {
case *LocalWorkspace:
local, err := NewLocalWorkspace(filepath.Join(typed.Root(), cleaned))
local, err := typed.Sub(cleaned)
if err != nil {
return &subWorkspace{inner: inner, initErr: fmt.Errorf("workspace sub: open local root %q: %w", cleaned, err)}
}
root := local.Root()
if root != typed.Root() && !strings.HasPrefix(root, typed.Root()+string(filepath.Separator)) {
err := fmt.Errorf("%w: %s (symlink escape)", ErrPathTraversal, cleaned)
return &subWorkspace{inner: inner, initErr: fmt.Errorf("workspace sub: open local root %q: %w", cleaned, err)}
}
return local
}
return sw
Expand Down
9 changes: 8 additions & 1 deletion sdkx/claw/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ func (c *Claw) buildMemory(ctx context.Context) (*memoryRuntime, error) {
opts := []recall.Option{recall.WithGraphEnabled(memCfg.Recall.GraphEnabled)}
memoryWS := sdkworkspace.Sub(c.ws, c.cfg.Workspace.MemoryRoot)
metadataWS := sdkworkspace.Sub(memoryWS, "metadata")
retrievalWS := sdkworkspace.Sub(memoryWS, "retrieval")
backend, err := recallworkspace.New(metadataWS)
if err != nil {
return nil, err
Expand All @@ -175,6 +174,14 @@ func (c *Claw) buildMemory(ctx context.Context) (*memoryRuntime, error) {
switch strings.TrimSpace(memCfg.Retrieval.Backend) {
case "", "memory":
case "bbh":
localMemoryWS, ok := memoryWS.(*sdkworkspace.LocalWorkspace)
if !ok {
return nil, fmt.Errorf("claw: retrieval backend bbh requires a local workspace")
}
retrievalWS, err := localMemoryWS.Sub("retrieval")
if err != nil {
return nil, fmt.Errorf("claw: create bbh retrieval workspace: %w", err)
}
index, err := bbh.New(retrievalWS, bbh.WithConfig(memCfg.Retrieval.BBH))
if err != nil {
return nil, err
Expand Down
16 changes: 16 additions & 0 deletions sdkx/claw/memory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,22 @@ func newMemoryEnabledTestClaw(t *testing.T) *Claw {
return newMemoryEnabledTestClawWith(t, nil)
}

func TestMemoryBBHRequiresLocalWorkspace(t *testing.T) {
ws := workspace.NewMemWorkspace()
cfg := testConfigForLLM(t, staticLLM{reply: "ok"})
cfg.Memory.Enabled = true
cfg.Memory.Retrieval.Backend = "bbh"
writeTestConfig(t, ws, cfg)

_, err := New(ws)
if err == nil {
t.Fatal("New should reject bbh with a non-local workspace")
}
if !strings.Contains(err.Error(), "bbh requires a local workspace") {
t.Fatalf("New error = %v, want local workspace requirement", err)
}
}

func newMemoryEnabledTestClawWith(t *testing.T, mutate func(*Config)) *Claw {
t.Helper()
ws, err := workspace.NewLocalWorkspace(t.TempDir())
Expand Down
Loading